morluto/rea

Fork 0

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

[PR #4363] [CLOSED] fix(javascript): grow a binding's returned-reference projection once per chain #6205

New issue

Closed

opened 2026-10-31 01:12:39 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:12:39 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4363
Author: @Ambar-13
Created: 10/16/2026
Status: ❌ Closed

Base: main ← Head: fix/returned-reference-growth

📝 Commits (6)

8b40b2a fix(javascript): grow a binding's returned-reference projection once per chain
8c5f726 fix(javascript): split a grown binding's writes per branch and record alias cuts
280700f fix(javascript): keep member keys on branches split after a self-walk
a6ce37b fix(javascript): cut container writes that read the chain after a self-walk
9a16bc9 fix(javascript): scan only the selected member of a container write
1f1fab7 fix(javascript): cut a grown binding on its second deeper read, not by predicting writes

📊 Changes

3 files changed (+204 additions, -14 deletions)

View changed files

📝 docs/javascript-application-workflows.md (+6 -0)
📝 src/domain/javascript/javascriptSemanticCallResolution.test.ts (+136 -0)
📝 src/domain/javascript/javascriptSemanticReturnedReferences.ts (+62 -14)

📄 Description

Summary

Part 2 of #4348 (the returned-reference expansion). Part 1, call resolution, is #4349.

Since #4243, the member-mutation pass runs out of heap on React bundles: both playwright-core 1.58.2 Vite bundles named in the issue, and per the issue's follow-up eight Notion renderer bundles. The cause is a local like React's fiber t, reassigned from its own members in many branches (t = t.child, t = t.return, t = t.sibling, ...). The returned-reference search follows each write once per chain, but it follows them in every order, and each order yields a different projection, so the work is factorial in the number of such writes. On the recorder bundle a single call site reaches 1.5 million visits with projections 22 to 25 keys deep before the heap goes.

This change lets a binding deepen its projection through its own writes once per chain. Both bundles now finish the returned-reference phase in about 3.9 s in-process and the CLI completes them.

Problem and expected behavior

Smallest trigger (also the new regression test):

function walk(root, flag){ let t = root; switch (flag) {
  case 0: t = t.key0; break; case 1: t = t.key1; break; /* ... n cases */
} return t.make(); }

Scope lookups during createSemanticReturnedReferences on main: 194 (n=4), 5,870 (n=6), 328,802 (n=8), 2,959,229 (n=9, 4 s). With this change: 38, 80, 138, 173 (and 822 at n=20).

Real inputs, this Mac (macOS 27.0.1 arm64, Node 26.7.0), rea analyze-javascript-application <dir> with defaults:

Input	main `1b97b0f`	this branch
`recorder/assets/index-BhTWtUlo.js` (363,108 B, SHA-256 `c909272a…`)	`Owned JavaScript worker exhausted its heap … during static`, 9.8 s, 1.23 GB RSS	completes, 19.5 s wall, 844 MB RSS
`traceViewer/assets/defaultSettingsView-CJSZINFr.js` (640,870 B)	heap exhausted (same message)	completes, 21.7 s wall, 1.09 GB RSS

In-process, the returned-reference phase alone over all 6,178 call sites of the recorder bundle: heap exhausted at 3 GB on main; 3.9 s with this change (an earlier design that predicted which writes to cut ran in 2.0 s but could not be made to match the expansion's own selection rules; see Review history).

Expected: the pass terminates in time polynomial in the number of writes, and the result stays the conservative set of references it was before #4243 in every case that does not need two consecutive self-walks of the same binding.

Change and scope

src/domain/javascript/javascriptSemanticReturnedReferences.ts:

The chain context now records, besides each followed initializer, each binding the chain read and the projection depth at that first read (Chain = ReadonlyMap<ChainEntry, number>).
When a binding is re-read with a deeper projection than at its first read, the chain has grown it through one of its own writes. That first deeper re-read still follows every write the read selects, so x = root resolves root.next, and the ordinary expansion rules decide what a conditional, object, array or spread write selects. The re-read is recorded, and a later re-read deeper than the first read is a cycle at the binding: nothing is followed. So a binding grows its projection once per chain, and a mesh of N self-walking writes costs about N² steps instead of every order.
Re-reads at the same depth keep every write, so alias cycles and the position-narrowed reads #4243 introduced behave as before. The bounds separate branch assignments and bounds shared partial callback alias expansion tests from #4243 pass unchanged.
The completed-expansion cache keys its blockers on exact (entry, depth) pairs, so a cached cut is reused only by a chain that would cut the same way.

What changes in results: a reference reachable only through two or more consecutive self-walks of the same binding is no longer reported. let x = root; x = x.next; x = x.next; x.make() with root = {next:{next:{make}}} returned one reference on main and returns none now; a test pins that so a future change is deliberate. Mutation analysis then does not mark such a value escaped, which is the under-approximating direction the issue's follow-up comment describes. I measured the alternative of allowing two self-walks on an earlier design: 10 s per bundle versus 2 s, and three did not finish.

Not included: the call-resolution part (#4349), and a structured "incomplete" signal from createSemanticReturnedReferences; its output has no such channel today.

Contribution scope: ordinary performance fix inside the existing static analysis; no target execution.

Review history

Earlier commits cut a grown binding's writes before following them, which meant re-implementing how the expansion selects through conditionals, member chains, object properties, array indices and spreads; Codex and Greptile found a divergence in each attempt (whole conditionals cut, member keys dropped, unselected properties hiding selected ones, trailing spreads bypassing the bound, the alias behind a cut not recorded for the cache). Commit six removes that prediction: the first deeper re-read follows everything and only the second deeper re-read is cut, so the expansion's own rules apply and the helper functions are gone. Every reviewer example is a test case, and the measurements below are from the final rule.

Contract and boundary impact

Semantic owner and earliest changed stage: createSemanticReturnedReferences in the semantic domain layer, consumed by member-mutation analysis.
Contribution scope fit and linked maintainer decision: not applicable (the issue's follow-up notes a polynomial bound has to pick a direction; this PR picks under-approximation after one self-walk and documents it).
CLI and MCP/tool-catalog contract: no schema change. Graph output can lose escape effects reachable only through repeated self-walks, as described above.
Provider, bridge, target-format, or platform compatibility: static JavaScript analysis only.
Evidence, artifact, provenance, or reconstruction contract: unchanged shapes; see "what changes in results".
Generated metadata, package, or 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

Tests added or updated (src/domain/javascript/javascriptSemanticCallResolution.test.ts, beside the other createSemanticReturnedReferences bound tests); the bound tests use 8 vs 12 writes and assert lookups grow less than 3x, for direct self-walks and for self-walks hidden behind ([root, ...t.key])[0]; the behaviour test covers one self-walk, an alias cycle, conditional and member-wrapped writes, containers with unselected or alternative chain reads, and the two-self-walk case that is dropped:
- bounds self-walking writes of one binding by their count: 8 vs 12 self-walking writes, asserts lookups grow less than 3x. On main this test does not finish in reasonable time (8 writes already take 328,802 lookups, 12 would be millions more).
- grows a binding's projection once per chain through its own writes: one self-walk resolves (1 reference), an alias cycle at equal depth resolves (1), two consecutive self-walks do not (0).
Base reproduction: see the tables above; executed on main 1b97b0f and this branch with the same files.
Corpus comparison: full semantic IR (analyzeJavaScriptSemantics, which includes the member-mutation pass) digest for every .js file under playwright-core@1.58.2/lib (313 files), main build vs this branch, each in its own 1 GB process: 311 of 313 files produce byte-identical IR (total 24.3 s on main vs 23.7 s here across them); the remaining two are the recorder and traceViewer bundles above, which exhaust a 1 GB heap on main and complete here. No file changed output. Harness: analyzeJavaScriptSemantics on each file, SHA-256 of JSON.stringify(ir).
Remaining proof gaps: Notion's bundles were not re-run here (the issue's follow-up measured them; same cause, part 2). Linux and Windows not run. The pre-#4243 timing for the recorder bundle was 1.25 s for the whole semantic phase; this branch spends 3.9 s in returned references alone, so it is slower than before #4243 while no longer exponential.

For evidence-bearing changes:

Observed, derived, and inferred claims remain distinguishable.
Artifact identity, source provenance, and failed attempts remain preserved.
Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

macOS 27.0.1 arm64, Node 26.7.0, npm 11.19.0, on top of main 1b97b0f.

npm ci --ignore-scripts — ok
npm run test:focused -- src/domain/javascript/javascriptSemanticCallResolution.test.ts — 13 passed
npm run test:focused over every src/domain/javascript/*.test.ts, tests/boundary/javascript/**/*.test.ts and tests/**/*lectron*.test.ts — 102 files, 2200 passed, 1 skipped
npm run check — ok (typecheck, lint, format, knip, metadata, architecture guards across 1451 source files)
npm run docs:check — ok; npm run verify:test-discovery — all 1412 test files found once
npm run build:cached, then the CLI and in-process runs above
Not run: full suite, Linux, Windows, Notion bundles

Compatibility, safety, and release

Breaking changes or migration steps: none in shape; see the result change above.
Real Hopper/Ghidra, browser, or OS coverage: not applicable (static JavaScript).
Package or release metadata impact: none.
Security, privacy, process, or containment review: none; pure analysis change.

Author readiness

Human owner: @Ambar-13

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

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.

🤖 Generated with Claude Code

_{🔄 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/4363 **Author:** [@Ambar-13](https://github.com/Ambar-13) **Created:** 10/16/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/returned-reference-growth` --- ### 📝 Commits (6) - [`8b40b2a`](https://github.com/morluto/rea/commit/8b40b2a34a56597add3e9d712e3a877e9e7f659d) fix(javascript): grow a binding's returned-reference projection once per chain - [`8c5f726`](https://github.com/morluto/rea/commit/8c5f7266cdef0bf965c1e1cff8cbb9df76a4d2ab) fix(javascript): split a grown binding's writes per branch and record alias cuts - [`280700f`](https://github.com/morluto/rea/commit/280700fd05f265253cf755a7594ffc639d00224c) fix(javascript): keep member keys on branches split after a self-walk - [`a6ce37b`](https://github.com/morluto/rea/commit/a6ce37b2020d1c936eda6ad23cfcfaeaddebb4c0) fix(javascript): cut container writes that read the chain after a self-walk - [`9a16bc9`](https://github.com/morluto/rea/commit/9a16bc92f074cb1e363be7034daeae4ca4500a84) fix(javascript): scan only the selected member of a container write - [`1f1fab7`](https://github.com/morluto/rea/commit/1f1fab7793edc5b74dfb0ed51ccf827cc133a26b) fix(javascript): cut a grown binding on its second deeper read, not by predicting writes ### 📊 Changes **3 files changed** (+204 additions, -14 deletions) <details> <summary>View changed files</summary> 📝 `docs/javascript-application-workflows.md` (+6 -0) 📝 `src/domain/javascript/javascriptSemanticCallResolution.test.ts` (+136 -0) 📝 `src/domain/javascript/javascriptSemanticReturnedReferences.ts` (+62 -14) </details> ### 📄 Description ## Summary Part 2 of #4348 (the returned-reference expansion). Part 1, call resolution, is #4349. Since #4243, the member-mutation pass runs out of heap on React bundles: both playwright-core 1.58.2 Vite bundles named in the issue, and per the issue's follow-up eight Notion renderer bundles. The cause is a local like React's fiber `t`, reassigned from its own members in many branches (`t = t.child`, `t = t.return`, `t = t.sibling`, ...). The returned-reference search follows each write once per chain, but it follows them in every order, and each order yields a different projection, so the work is factorial in the number of such writes. On the recorder bundle a single call site reaches 1.5 million visits with projections 22 to 25 keys deep before the heap goes. This change lets a binding deepen its projection through its own writes once per chain. Both bundles now finish the returned-reference phase in about 3.9 s in-process and the CLI completes them. ## Problem and expected behavior Smallest trigger (also the new regression test): ```js function walk(root, flag){ let t = root; switch (flag) { case 0: t = t.key0; break; case 1: t = t.key1; break; /* ... n cases */ } return t.make(); } ``` Scope lookups during `createSemanticReturnedReferences` on main: 194 (n=4), 5,870 (n=6), 328,802 (n=8), 2,959,229 (n=9, 4 s). With this change: 38, 80, 138, 173 (and 822 at n=20). Real inputs, this Mac (macOS 27.0.1 arm64, Node 26.7.0), `rea analyze-javascript-application <dir>` with defaults: | Input | main `1b97b0f` | this branch | | --- | --- | --- | | `recorder/assets/index-BhTWtUlo.js` (363,108 B, SHA-256 `c909272a…`) | `Owned JavaScript worker exhausted its heap … during static`, 9.8 s, 1.23 GB RSS | completes, 19.5 s wall, 844 MB RSS | | `traceViewer/assets/defaultSettingsView-CJSZINFr.js` (640,870 B) | heap exhausted (same message) | completes, 21.7 s wall, 1.09 GB RSS | In-process, the returned-reference phase alone over all 6,178 call sites of the recorder bundle: heap exhausted at 3 GB on main; 3.9 s with this change (an earlier design that predicted which writes to cut ran in 2.0 s but could not be made to match the expansion's own selection rules; see Review history). Expected: the pass terminates in time polynomial in the number of writes, and the result stays the conservative set of references it was before #4243 in every case that does not need two consecutive self-walks of the same binding. ## Change and scope `src/domain/javascript/javascriptSemanticReturnedReferences.ts`: - The chain context now records, besides each followed initializer, each binding the chain read and the projection depth at that first read (`Chain = ReadonlyMap<ChainEntry, number>`). - When a binding is re-read with a deeper projection than at its first read, the chain has grown it through one of its own writes. That first deeper re-read still follows every write the read selects, so `x = root` resolves `root.next`, and the ordinary expansion rules decide what a conditional, object, array or spread write selects. The re-read is recorded, and a later re-read deeper than the first read is a cycle at the binding: nothing is followed. So a binding grows its projection once per chain, and a mesh of N self-walking writes costs about N² steps instead of every order. - Re-reads at the same depth keep every write, so alias cycles and the position-narrowed reads #4243 introduced behave as before. The `bounds separate branch assignments` and `bounds shared partial callback alias expansion` tests from #4243 pass unchanged. - The completed-expansion cache keys its blockers on exact `(entry, depth)` pairs, so a cached cut is reused only by a chain that would cut the same way. What changes in results: a reference reachable only through two or more consecutive self-walks of the same binding is no longer reported. `let x = root; x = x.next; x = x.next; x.make()` with `root = {next:{next:{make}}}` returned one reference on main and returns none now; a test pins that so a future change is deliberate. Mutation analysis then does not mark such a value escaped, which is the under-approximating direction the issue's follow-up comment describes. I measured the alternative of allowing two self-walks on an earlier design: 10 s per bundle versus 2 s, and three did not finish. Not included: the call-resolution part (#4349), and a structured "incomplete" signal from `createSemanticReturnedReferences`; its output has no such channel today. Contribution scope: ordinary performance fix inside the existing static analysis; no target execution. ## Review history Earlier commits cut a grown binding's writes before following them, which meant re-implementing how the expansion selects through conditionals, member chains, object properties, array indices and spreads; Codex and Greptile found a divergence in each attempt (whole conditionals cut, member keys dropped, unselected properties hiding selected ones, trailing spreads bypassing the bound, the alias behind a cut not recorded for the cache). Commit six removes that prediction: the first deeper re-read follows everything and only the second deeper re-read is cut, so the expansion's own rules apply and the helper functions are gone. Every reviewer example is a test case, and the measurements below are from the final rule. ## Contract and boundary impact - Semantic owner and earliest changed stage: `createSemanticReturnedReferences` in the semantic domain layer, consumed by member-mutation analysis. - Contribution scope fit and linked maintainer decision: not applicable (the issue's follow-up notes a polynomial bound has to pick a direction; this PR picks under-approximation after one self-walk and documents it). - CLI and MCP/tool-catalog contract: no schema change. Graph output can lose `escape` effects reachable only through repeated self-walks, as described above. - Provider, bridge, target-format, or platform compatibility: static JavaScript analysis only. - Evidence, artifact, provenance, or reconstruction contract: unchanged shapes; see "what changes in results". - Generated metadata, package, or 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 - Tests added or updated (`src/domain/javascript/javascriptSemanticCallResolution.test.ts`, beside the other `createSemanticReturnedReferences` bound tests); the bound tests use 8 vs 12 writes and assert lookups grow less than 3x, for direct self-walks and for self-walks hidden behind `([root, ...t.key])[0]`; the behaviour test covers one self-walk, an alias cycle, conditional and member-wrapped writes, containers with unselected or alternative chain reads, and the two-self-walk case that is dropped: - `bounds self-walking writes of one binding by their count`: 8 vs 12 self-walking writes, asserts lookups grow less than 3x. On main this test does not finish in reasonable time (8 writes already take 328,802 lookups, 12 would be millions more). - `grows a binding's projection once per chain through its own writes`: one self-walk resolves (1 reference), an alias cycle at equal depth resolves (1), two consecutive self-walks do not (0). - Base reproduction: see the tables above; executed on main `1b97b0f` and this branch with the same files. - Corpus comparison: full semantic IR (`analyzeJavaScriptSemantics`, which includes the member-mutation pass) digest for every `.js` file under `playwright-core@1.58.2/lib` (313 files), main build vs this branch, each in its own 1 GB process: 311 of 313 files produce byte-identical IR (total 24.3 s on main vs 23.7 s here across them); the remaining two are the recorder and traceViewer bundles above, which exhaust a 1 GB heap on main and complete here. No file changed output. Harness: `analyzeJavaScriptSemantics` on each file, SHA-256 of `JSON.stringify(ir)`. - Remaining proof gaps: Notion's bundles were not re-run here (the issue's follow-up measured them; same cause, part 2). Linux and Windows not run. The pre-#4243 timing for the recorder bundle was 1.25 s for the whole semantic phase; this branch spends 3.9 s in returned references alone, so it is slower than before #4243 while no longer exponential. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed macOS 27.0.1 arm64, Node 26.7.0, npm 11.19.0, on top of main `1b97b0f`. - `npm ci --ignore-scripts` — ok - `npm run test:focused -- src/domain/javascript/javascriptSemanticCallResolution.test.ts` — 13 passed - `npm run test:focused` over every `src/domain/javascript/*.test.ts`, `tests/boundary/javascript/**/*.test.ts` and `tests/**/*lectron*.test.ts` — 102 files, 2200 passed, 1 skipped - `npm run check` — ok (typecheck, lint, format, knip, metadata, architecture guards across 1451 source files) - `npm run docs:check` — ok; `npm run verify:test-discovery` — all 1412 test files found once - `npm run build:cached`, then the CLI and in-process runs above - Not run: full suite, Linux, Windows, Notion bundles ## Compatibility, safety, and release - Breaking changes or migration steps: none in shape; see the result change above. - Real Hopper/Ghidra, browser, or OS coverage: not applicable (static JavaScript). - Package or release metadata impact: none. - Security, privacy, process, or containment review: none; pure analysis change. ## Author readiness Human owner: @Ambar-13 - [ ] 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 - [ ] This PR addresses a concrete problem or an agreed enhancement. - [ ] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:12:39 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:46:06 +00:00

[PR #6205] [MERGED] fix(process): settle owned cleanup with the launcher's own exit proof #7202

bsakaguchi changed title from ~~[PR #4363] [CLOSED] fix(javascript): grow a binding's returned-reference projection once per chain~~ to [PR #4363] fix(javascript): grow a binding's returned-reference projection once per chain

2026-11-03 13:32:12 +00:00

bsakaguchi changed title from ~~[PR #4363] fix(javascript): grow a binding's returned-reference projection once per chain~~ to [PR #4363] [CLOSED] fix(javascript): grow a binding's returned-reference projection once per chain

2026-11-03 14:33:56 +00:00

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#6205

No description provided.

Rows
Columns