morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #4827] [CLOSED] fix(javascript): keep a defaulted nested pattern's destructuring dependency #6429

New issue

Closed

opened 2026-10-31 01:14:23 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:14:23 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4827
Author: @XiaoBinGan
Created: 10/17/2026
Status: ❌ Closed

Base: main ← Head: fix/rest-and-default-destructuring

📝 Commits (1)

c635bfd fix(javascript): keep a defaulted nested pattern's destructuring dependency

📊 Changes

2 files changed (+58 additions, -5 deletions)

View changed files

➕ src/domain/javascript/javascriptSemanticAnalysis.destructuringDefaults.test.ts (+49 -0)
📝 src/domain/javascript/javascriptSemanticObjects.ts (+9 -5)

📄 Description

Summary

analyze-javascript-application lost a statically known destructuring dependency when the nested pattern carried a default value. For the module below, plain kept its obj.a[0] destructuring operation and destructures relation while fallback kept only its lexical binding — even though the default = [] cannot apply while obj.a is present, so the known obj.a[0] dependency is unchanged (morluto/rea#4604).

const obj = {a: [1], b: 2};
const {a: [plain]} = obj;
const {a: [fallback] = []} = obj;

A default value wraps the pattern it defaults, so {a: [fallback] = []} carries an AssignmentPattern whose left is the array pattern [fallback]. Destructuring collection recognized only a bare identifier inside an AssignmentPattern (bindingIdentifier), so a defaulted pattern was neither dispatched into nor projected, and the target silently lost its static evidence.

Problem and expected behavior

Pattern	Before	After
`{a: [plain]}` (nested control)	`destructures` relation, path `a[0]`	unchanged
`{a: [fallback] = []}` (defaulted nested pattern)	no operation, no relation	`destructures` relation, path `a[0]`
`{b: scalar = 0}` (defaulted identifier)	`destructures` relation, path `b`	unchanged
computed dynamic key `{[key]: x}`	no operation for `x`	unchanged

End-to-end on the issue's fixture, with a fresh isolated HOME per run and node scripts/rea.mjs analyze-javascript-application <dir> --json (exit 0 in both runs):

before: 4 destructure operations, relation targets ['a', 'plain', 'scalar', 'x'];
after: 5 destructure operations, relation targets ['a', 'fallback', 'plain', 'scalar', 'x'].

The overall coverage status and the object-flow family status are unchanged by this fix; no false complete-coverage result is introduced, because the added relation only restates a slot that was already resolved through the same collector for the plain control.

Change and scope

src/domain/javascript/javascriptSemanticObjects.ts — collectDestructuring's visit unwraps an AssignmentPattern to its left before choosing the pattern shape, so object patterns, array patterns, and identifiers are all reached through a default. The operation's reported node stays the original pattern, so existing defaulted-identifier operations keep their identity, location, and path.
src/domain/javascript/javascriptSemanticAnalysis.destructuringDefaults.test.ts (new) — pins the known obj.a[0] slot for both the plain and the defaulted nested pattern, and the defaulted identifier control.

Out of scope, deliberately: object-rest ({a, ...rest}) and array-rest ([x, ...ys]) targets still receive no operation. A rest target has no single static property path, so inventing one would fabricate evidence; the issue's own impact section states that the rest omissions are related observations and does not require an exact rest path. That work needs its own contract decision (for example an explicit partial/unknown projection) and is left to a follow-up.

Contract and boundary impact

Semantic owner and earliest changed stage: the static destructuring collector (collectDestructuring) in the domain IR; the graph projection is unchanged and consumes the operation as before.
Contribution scope fit and linked maintainer decision: the issue is a direct, supported P2 report with a fixed fixture and no designated intentional omission; no new scope decision is required for the defaulted-pattern case.
CLI and MCP/tool-catalog contract: destructures relations gain the previously missing target for defaulted nested patterns; relation names, kinds, resolutions, schemas, and tool schemas are unchanged.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: no change to evidence hashing or artifact output beyond the added relation in the semantic graph.
Target selection, authority, upstream behavior, and cleanup details: not applicable; the fixture is inert and is neither imported nor executed.
Generated metadata / package / installation impact: none.

Expected effects:

Read existing files or captured data.
Launch or attach to a process.
Inject instrumentation or run scripts/code.
Change process memory, registers, files, or configuration.
Contact network services.
Recover protected code or work around protections.
Documentation or usability only; no target interaction.
Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

Checks actually run on Node 24.18.0 (c635bfd8c, based on main d74e72406):

npx vitest run src/domain/javascript/javascriptSemanticAnalysis.destructuringDefaults.test.ts → 1 passed with the change, 1 failed with the parent revision of the changed source file (AssertionError: expected undefined to deeply equal { propertyPath: [ 'a', '0' ] … }), i.e. the new test pins the reported loss.
npx vitest run src/domain/javascript/ → 2086 passed, 0 failed (81 files).
CLI before/after on the issue fixture (npm run build:cached before each run, fresh HOME): 4 → 5 destructure operations and relation targets ['a', 'plain', 'scalar', 'x'] → ['a', 'fallback', 'plain', 'scalar', 'x'].
npm run check:fast → typecheck + lint + verify:module-boundaries pass.
npm run format:check, npm run verify:test-discovery → pass.

Unverified gaps: the report's published-package run (rea-agents@6.5.0) was not repeated; the comparison here is source-build only, and the fixed output was verified through the source build. Rest-target behavior is intentionally unchanged and untested here (see scope note).

Compatibility, safety, and release

Breaking changes or migration steps: consumers of destructures relations receive additional relations for defaulted nested patterns; no existing relation, node, or field is removed or renamed.
Real Hopper/Ghidra, browser, or OS coverage: none required; no native provider is involved.
Package or release metadata impact: none.
Security, privacy, process, or containment review: none applicable.

Author readiness

Human owner: @XiaoBinGan

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

Before-merge:

I will respond to review feedback and keep the PR current with main until it merges.
This PR addresses a concrete problem or an agreed enhancement.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/4827 **Author:** [@XiaoBinGan](https://github.com/XiaoBinGan) **Created:** 10/17/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/rest-and-default-destructuring` --- ### 📝 Commits (1) - [`c635bfd`](https://github.com/morluto/rea/commit/c635bfd8c24f6dfba2b510e36cd882cb5519bfda) fix(javascript): keep a defaulted nested pattern's destructuring dependency ### 📊 Changes **2 files changed** (+58 additions, -5 deletions) <details> <summary>View changed files</summary> ➕ `src/domain/javascript/javascriptSemanticAnalysis.destructuringDefaults.test.ts` (+49 -0) 📝 `src/domain/javascript/javascriptSemanticObjects.ts` (+9 -5) </details> ### 📄 Description ## Summary `analyze-javascript-application` lost a statically known destructuring dependency when the nested pattern carried a default value. For the module below, `plain` kept its `obj.a[0]` destructuring operation and `destructures` relation while `fallback` kept only its lexical binding — even though the default `= []` cannot apply while `obj.a` is present, so the known `obj.a[0]` dependency is unchanged (morluto/rea#4604). ```js const obj = {a: [1], b: 2}; const {a: [plain]} = obj; const {a: [fallback] = []} = obj; ``` A default value wraps the pattern it defaults, so `{a: [fallback] = []}` carries an `AssignmentPattern` whose `left` is the array pattern `[fallback]`. Destructuring collection recognized only a bare identifier inside an `AssignmentPattern` (`bindingIdentifier`), so a defaulted *pattern* was neither dispatched into nor projected, and the target silently lost its static evidence. ## Problem and expected behavior | Pattern | Before | After | | --- | --- | --- | | `{a: [plain]}` (nested control) | `destructures` relation, path `a[0]` | unchanged | | `{a: [fallback] = []}` (defaulted nested pattern) | **no operation, no relation** | `destructures` relation, path `a[0]` | | `{b: scalar = 0}` (defaulted identifier) | `destructures` relation, path `b` | unchanged | | computed dynamic key `{[key]: x}` | no operation for `x` | unchanged | End-to-end on the issue's fixture, with a fresh isolated `HOME` per run and `node scripts/rea.mjs analyze-javascript-application <dir> --json` (exit 0 in both runs): - before: `4` destructure operations, relation targets `['a', 'plain', 'scalar', 'x']`; - after: `5` destructure operations, relation targets `['a', 'fallback', 'plain', 'scalar', 'x']`. The overall coverage status and the `object-flow` family status are unchanged by this fix; no false complete-coverage result is introduced, because the added relation only restates a slot that was already resolved through the same collector for the plain control. ## Change and scope - `src/domain/javascript/javascriptSemanticObjects.ts` — `collectDestructuring`'s `visit` unwraps an `AssignmentPattern` to its `left` before choosing the pattern shape, so object patterns, array patterns, and identifiers are all reached through a default. The operation's reported node stays the original pattern, so existing defaulted-identifier operations keep their identity, location, and path. - `src/domain/javascript/javascriptSemanticAnalysis.destructuringDefaults.test.ts` (new) — pins the known `obj.a[0]` slot for both the plain and the defaulted nested pattern, and the defaulted identifier control. Out of scope, deliberately: object-rest (`{a, ...rest}`) and array-rest (`[x, ...ys]`) targets still receive no operation. A rest target has no single static property path, so inventing one would fabricate evidence; the issue's own impact section states that the rest omissions are related observations and does not require an exact rest path. That work needs its own contract decision (for example an explicit partial/unknown projection) and is left to a follow-up. ## Contract and boundary impact - Semantic owner and earliest changed stage: the static destructuring collector (`collectDestructuring`) in the domain IR; the graph projection is unchanged and consumes the operation as before. - Contribution scope fit and linked maintainer decision: the issue is a direct, supported P2 report with a fixed fixture and no designated intentional omission; no new scope decision is required for the defaulted-pattern case. - CLI and MCP/tool-catalog contract: `destructures` relations gain the previously missing target for defaulted nested patterns; relation names, kinds, resolutions, schemas, and tool schemas are unchanged. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: no change to evidence hashing or artifact output beyond the added relation in the semantic graph. - Target selection, authority, upstream behavior, and cleanup details: not applicable; the fixture is inert and is neither imported nor executed. - Generated metadata / package / installation impact: none. Expected effects: - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage Checks actually run on Node 24.18.0 (`c635bfd8c`, based on `main` `d74e72406`): - `npx vitest run src/domain/javascript/javascriptSemanticAnalysis.destructuringDefaults.test.ts` → **1 passed** with the change, **1 failed** with the parent revision of the changed source file (`AssertionError: expected undefined to deeply equal { propertyPath: [ 'a', '0' ] … }`), i.e. the new test pins the reported loss. - `npx vitest run src/domain/javascript/` → **2086 passed**, 0 failed (81 files). - CLI before/after on the issue fixture (`npm run build:cached` before each run, fresh `HOME`): `4` → `5` destructure operations and relation targets `['a', 'plain', 'scalar', 'x']` → `['a', 'fallback', 'plain', 'scalar', 'x']`. - `npm run check:fast` → typecheck + lint + `verify:module-boundaries` pass. - `npm run format:check`, `npm run verify:test-discovery` → pass. Unverified gaps: the report's published-package run (`rea-agents@6.5.0`) was not repeated; the comparison here is source-build only, and the fixed output was verified through the source build. Rest-target behavior is intentionally unchanged and untested here (see scope note). ## Compatibility, safety, and release - Breaking changes or migration steps: consumers of `destructures` relations receive additional relations for defaulted nested patterns; no existing relation, node, or field is removed or renamed. - Real Hopper/Ghidra, browser, or OS coverage: none required; no native provider is involved. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none applicable. ## Author readiness  Human owner: @XiaoBinGan - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. Before-merge: - [x] I will respond to review feedback and keep the PR current with `main` until it merges. - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:14:23 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:47:16 +00:00

[PR #6429] [CLOSED] fix(mcp): publish portable Windows absolute path patterns #7354

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#6429

No description provided.

Rows
Columns