[PR #4243] [MERGED] fix(javascript): preserve incoming callback target uncertainty #6142

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4243
Author: @N0zoM1z0
Created: 10/15/2026
Status: ✅ Merged
Merged: 10/16/2026
Merged by: @N0zoM1z0

Base: main ← Head: fix/javascript-callback-entry-3880

📝 Commits (3)

2b26094 fix(javascript): preserve incoming callback target uncertainty
bd567c4 fix(javascript): distinguish callback initializer versions
f6b7bf4 fix(javascript): bound projected callback alias expansion

📊 Changes

8 files changed (+822 additions, -118 deletions)

View changed files

📝 docs/javascript-application-workflows.md (+11 -0)
➕ src/domain/javascript/javascriptSemanticCallResolution.test.ts (+233 -0)
📝 src/domain/javascript/javascriptSemanticCallResolution.ts (+69 -32)
📝 src/domain/javascript/javascriptSemanticCalls.ts (+6 -3)
📝 src/domain/javascript/javascriptSemanticMutationInitializers.ts (+87 -27)
📝 src/domain/javascript/javascriptSemanticReturnedReferences.ts (+122 -55)
📝 src/domain/javascript/javascriptSemanticState.ts (+1 -1)
➕ tests/acceptance/applications/javascriptCallbackTargets.test.ts (+293 -0)

📄 Description

Summary

Fixes #3880. if (flag) cb = fallback; return cb() previously produced a resolved edge to fallback, although Node calls the incoming callback when flag is false. It now retains a candidate edge and an ambiguous, partial default trace. A proven unconditional replacement remains resolved.

Problem and expected behavior

A known local candidate is not necessarily an exhaustive target set. Assignment timing also matters: later writes must not change an earlier callback read or captured alias.

Change and scope

Use the existing semanticBindingAtRead owner for callable resolution and returned-reference effects. Its definite-initializer proof distinguishes writes dominating a read from globally conditional writes. Cache by identifier read, and track active parsed initializer facts so saved/restored values are not mistaken for binding cycles. Memoize cycle-independent partial callable results. Returned references use one projected traversal: finish subgraphs before caching them, keep actual cycle blockers as dependencies, and propagate those dependencies through cache hits. The redundant preliminary callable traversal and its cache are removed.

Unproved branch/activation ordering stays conservative; this does not add a control-flow interpreter.

Contract and boundary impact

Semantic owner: JavaScript call resolution; returned-reference consumers share the same read-position facts.
Contribution scope: ordinary correctness/performance fix within existing static analysis; additional maintainer scope decision not applicable.
CLI/MCP: shared implementation; no schema or catalog changes.
Evidence: retains known candidates, source identity and uncertainty; complete parent Evidence remains queryable after summary replies.
Providers/platforms: Linux Node 24.18.0; no provider, installer or package changes.
Target authority/cleanup: analysis reads authored source files. Node execution is confined to the test oracle.

Expected effects:

Read existing files or captured data.

Evidence and regression coverage

Both public acceptance tests fail on pinned main 0cc56493b: the conditional callback is falsely resolved, and a later replacement falsely contaminates an earlier returned-reference effect. Both pass with this change through CLI and real stdio MCP.

Authored Node runtime controls cover conditional/logical replacement, catch/default parameters, delayed nested writes, captured callbacks, before/after reads and definite local replacement. Domain tests check read-cache isolation, read-specific initializer coverage, genuine cycle-context reuse and bounded reconvergent partial-alias work in both consumers (depth 8 versus 12).

Both actionable review findings were reproduced against original PR head 2b26094f4 and fixed in bd567c491: guarded calls now remain resolved, and restoring a saved callback retains its target/reference effects. Node returns 3 after mutating a returned shared object; the original head falsely reports literal 1, while the fix preserves unknown mutation evidence.

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

All JavaScript domain tests plus the new public acceptance file: 1,835 passed / 76 files.
npm run check: passed, including lint, module boundaries and architecture guards.
npm run docs:check: passed, including 5 documentation consistency tests.
npm run verify:test-discovery: all 1,363 test files discovered exactly once.

On the final head, three alternating fresh MCP sessions per version completed the same 10-call workflow: summary analysis, seven retained traces, explicit candidate traversal and full Evidence bundle verification. Median baseline → fix:

Measurement	Baseline	Fix
Completed workflow wall / CPU	10.045 / 11.714 s	9.881 / 11.597 s
Sampled process-family peak RSS	945 MiB	953 MiB
Cold completed goal	9.635 s	9.466 s
Warm retained trace	6.89 ms	5.88 ms
First goal token proxy, including catalog	808,379	806,848
Complete workflow token proxy	911,527	900,547

Tokens use cl100k_base over the full SDK catalog (798,856 tokens in both), all request/reply JSON including text/structured duplication, extra calls and full bundle. These are context proxies; initialization/wire framing/model billing are unmeasured. Correct candidate filtering changes returned content; these small timings do not establish a general speedup. Shared host; two pinned cores, 3 GiB Node heap, serial heavy jobs. RSS sampled every 200 ms may count shared pages twice or miss brief peaks; CPU covers reaped descendants. Largest local gate sample was 4.69 GiB, with no OOM or resource-limit stop. Logs, failed probes and measurements are retained locally under /tmp/rea-3880-20261015.

Separate branch-assignment growth controls (including an intrinsic cycle and projected method calls) protect the reference traversal itself. A regression against bd567c491 grows from 1,042 to 16,410 binding reads at depth 8→12; it fails before the fix. Empty-local ||=/??= controls also fail there and pass now, while initialized/incoming aliases stay conservative.

Cold public CLI plus Node-oracle verification at depth 12: 6.620→2.411 s wall, 7.503→3.021 s CPU, 447→448 MiB RSS against bd567c491. Full Evidence is exactly equal at depths 8/12. Fixed depths 20/32 complete in 2.501/2.545 s, about 445 MiB peak; the old version was not run at those sizes. These are single shared-host samples. All original-task SDK payloads also match the previously verified fix.

Compatibility, safety, and release

No new API, migration, provider or execution capability. Real-provider coverage is not applicable to this static JavaScript fix. Cross-branch/activation precision remains limited; uncertainty is preserved.

Author readiness

Human owner: @N0zoM1z0

The human owner explicitly authorized delegated review, proxy completion of these declarations, ready-for-review publication and normal merge for this batch. The agent reviewed the final diff and evidence; this records that delegation and does not assert a separate manual review occurred.

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.
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/4243 **Author:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Created:** 10/15/2026 **Status:** ✅ Merged **Merged:** 10/16/2026 **Merged by:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Base:** `main` ← **Head:** `fix/javascript-callback-entry-3880` --- ### 📝 Commits (3) - [`2b26094`](https://github.com/morluto/rea/commit/2b26094f4ddb3b5639be64a049ee6a26d0c8d58d) fix(javascript): preserve incoming callback target uncertainty - [`bd567c4`](https://github.com/morluto/rea/commit/bd567c4912b08fe3a45f812320a93780fd07e722) fix(javascript): distinguish callback initializer versions - [`f6b7bf4`](https://github.com/morluto/rea/commit/f6b7bf458c4a033fd1707ba729a4062accff1891) fix(javascript): bound projected callback alias expansion ### 📊 Changes **8 files changed** (+822 additions, -118 deletions) <details> <summary>View changed files</summary> 📝 `docs/javascript-application-workflows.md` (+11 -0) ➕ `src/domain/javascript/javascriptSemanticCallResolution.test.ts` (+233 -0) 📝 `src/domain/javascript/javascriptSemanticCallResolution.ts` (+69 -32) 📝 `src/domain/javascript/javascriptSemanticCalls.ts` (+6 -3) 📝 `src/domain/javascript/javascriptSemanticMutationInitializers.ts` (+87 -27) 📝 `src/domain/javascript/javascriptSemanticReturnedReferences.ts` (+122 -55) 📝 `src/domain/javascript/javascriptSemanticState.ts` (+1 -1) ➕ `tests/acceptance/applications/javascriptCallbackTargets.test.ts` (+293 -0) </details> ### 📄 Description ## Summary Fixes #3880. `if (flag) cb = fallback; return cb()` previously produced a resolved edge to `fallback`, although Node calls the incoming callback when `flag` is false. It now retains a candidate edge and an ambiguous, partial default trace. A proven unconditional replacement remains resolved. ## Problem and expected behavior A known local candidate is not necessarily an exhaustive target set. Assignment timing also matters: later writes must not change an earlier callback read or captured alias. ## Change and scope Use the existing `semanticBindingAtRead` owner for callable resolution and returned-reference effects. Its definite-initializer proof distinguishes writes dominating a read from globally conditional writes. Cache by identifier read, and track active parsed initializer facts so saved/restored values are not mistaken for binding cycles. Memoize cycle-independent partial callable results. Returned references use one projected traversal: finish subgraphs before caching them, keep actual cycle blockers as dependencies, and propagate those dependencies through cache hits. The redundant preliminary callable traversal and its cache are removed. Unproved branch/activation ordering stays conservative; this does not add a control-flow interpreter. ## Contract and boundary impact - Semantic owner: JavaScript call resolution; returned-reference consumers share the same read-position facts. - Contribution scope: ordinary correctness/performance fix within existing static analysis; additional maintainer scope decision not applicable. - CLI/MCP: shared implementation; no schema or catalog changes. - Evidence: retains known candidates, source identity and uncertainty; complete parent Evidence remains queryable after summary replies. - Providers/platforms: Linux Node 24.18.0; no provider, installer or package changes. - Target authority/cleanup: analysis reads authored source files. Node execution is confined to the test oracle. Expected effects: - [x] Read existing files or captured data. ## Evidence and regression coverage Both public acceptance tests fail on pinned main `0cc56493b`: the conditional callback is falsely resolved, and a later replacement falsely contaminates an earlier returned-reference effect. Both pass with this change through CLI and real stdio MCP. Authored Node runtime controls cover conditional/logical replacement, catch/default parameters, delayed nested writes, captured callbacks, before/after reads and definite local replacement. Domain tests check read-cache isolation, read-specific initializer coverage, genuine cycle-context reuse and bounded reconvergent partial-alias work in both consumers (depth 8 versus 12). Both actionable review findings were reproduced against original PR head `2b26094f4` and fixed in `bd567c491`: guarded calls now remain resolved, and restoring a saved callback retains its target/reference effects. Node returns 3 after mutating a returned shared object; the original head falsely reports literal 1, while the fix preserves unknown mutation evidence. - [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 - All JavaScript domain tests plus the new public acceptance file: **1,835 passed / 76 files**. - `npm run check`: passed, including lint, module boundaries and architecture guards. - `npm run docs:check`: passed, including 5 documentation consistency tests. - `npm run verify:test-discovery`: all **1,363** test files discovered exactly once. On the final head, three alternating fresh MCP sessions per version completed the same 10-call workflow: summary analysis, seven retained traces, explicit candidate traversal and full Evidence bundle verification. Median baseline → fix: | Measurement | Baseline | Fix | |---|---:|---:| | Completed workflow wall / CPU | 10.045 / 11.714 s | 9.881 / 11.597 s | | Sampled process-family peak RSS | 945 MiB | 953 MiB | | Cold completed goal | 9.635 s | 9.466 s | | Warm retained trace | 6.89 ms | 5.88 ms | | First goal token proxy, including catalog | 808,379 | 806,848 | | Complete workflow token proxy | 911,527 | 900,547 | Tokens use `cl100k_base` over the full SDK catalog (798,856 tokens in both), all request/reply JSON including text/structured duplication, extra calls and full bundle. These are context proxies; initialization/wire framing/model billing are unmeasured. Correct candidate filtering changes returned content; these small timings do not establish a general speedup. Shared host; two pinned cores, 3 GiB Node heap, serial heavy jobs. RSS sampled every 200 ms may count shared pages twice or miss brief peaks; CPU covers reaped descendants. Largest local gate sample was 4.69 GiB, with no OOM or resource-limit stop. Logs, failed probes and measurements are retained locally under `/tmp/rea-3880-20261015`. Separate branch-assignment growth controls (including an intrinsic cycle and projected method calls) protect the reference traversal itself. A regression against `bd567c491` grows from 1,042 to 16,410 binding reads at depth 8→12; it fails before the fix. Empty-local `||=`/`??=` controls also fail there and pass now, while initialized/incoming aliases stay conservative. Cold public CLI plus Node-oracle verification at depth 12: **6.620→2.411 s wall, 7.503→3.021 s CPU, 447→448 MiB RSS** against `bd567c491`. Full Evidence is exactly equal at depths 8/12. Fixed depths 20/32 complete in 2.501/2.545 s, about 445 MiB peak; the old version was not run at those sizes. These are single shared-host samples. All original-task SDK payloads also match the previously verified fix. ## Compatibility, safety, and release No new API, migration, provider or execution capability. Real-provider coverage is not applicable to this static JavaScript fix. Cross-branch/activation precision remains limited; uncertainty is preserved. ## Author readiness Human owner: @N0zoM1z0 The human owner explicitly authorized delegated review, proxy completion of these declarations, ready-for-review publication and normal merge for this batch. The agent reviewed the final diff and evidence; this records that delegation and does not assert a separate manual review occurred. - [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] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [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:12:11 +00:00