Skip to content

test(javascript): expect provable precision for the short-circuit mutation - #1385

Closed
BaaRai wants to merge 1 commit into
morluto:mainfrom
BaaRai:test/short-circuit-mutation-provable-presence
Closed

BaaRai wants to merge 1 commit into
morluto:mainfrom
BaaRai:test/short-circuit-mutation-provable-presence

Conversation

@BaaRai

@BaaRai BaaRai commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

Summary

main has been red on Test shard 1/4 since 001851f ("preserve capture-time guards and ordering"): JavaScriptSemanticGraphBuilder.test.ts > preserves possible alias mutation uncertainty in queries: short-circuit mutation expects unknown-coverage but the graph now reports present for the shared slot. The new presence is the correct outcome of that commit's capture-time ordering: the reassignment alias = { mode: "other" } and the guarded mutation flag && (alias.mode = "updated") share one statement list, so the analysis proves the mutation cannot reach the earlier alias target. The test table now states each row's expected shared presence explicitly and additionally asserts the mutation itself stays recorded on the alias binding in every row.

Problem and expected behavior

Smallest trigger: npm run test on main at 001851f and later (reproduced deterministically on Linux and Windows, 15/15 and 8/8 local runs; main CI run 38003459598 failed the same test at 08ec2bf, and PR #1382's shard hit it too).

Root cause chain, from bisecting 39dabd0 (passes) to 001851f (fails):

  • directMutationPosition now climbs from the assignment to its enclosing statement, so a mutation nested inside a logical expression receives a position in the program's statement list instead of no position at all.
  • positionPrecedes then proves the preceding reassignment dominates the mutation, and the member mutation is attributed only to the reassigned object; the shared slot keeps its captured present value.
  • The if-guarded form keeps no such proof because its mutation's position lives in the consequent block body, so it correctly stays unknown-coverage. Both outcomes match the semantics documented in the reference_lifetimes tests added by the same commit.

Violated invariant: the test table's single hardcoded unknown-coverage expectation encoded the pre-001851fd conservative behavior and was not updated with that change.

Change and scope

src/application/javascript/JavaScriptSemanticGraphBuilder.test.ts only: the seven-row table gains an explicit expected-shared-presence column (unknown-coverage for the six rows whose reassignment or proof is conditional, present for the short-circuit row), and the assertions add the alias slot staying unknown-coverage in every row, proving the guarded mutation is still recorded on the current alias target. No product code changes.

Contract and boundary impact

  • Semantic owner and earliest changed stage: test only; no runtime stage changes.
  • CLI and MCP/tool-catalog contract: none.
  • Provider, bridge, target-format, or platform compatibility: none.
  • Evidence, artifact, provenance, or reconstruction contract: not applicable.
  • Process execution, authorization, cleanup, or containment impact: not applicable.
  • Generated metadata, package, or installation impact: not applicable.

Evidence and regression coverage

  • Tests added or updated: the corrected table (explicit per-row presence plus the alias-slot recording assertion).
  • Base reproduction and other evidence:
    • Deterministic local failures at 001851f and later on both platforms (Windows 15/15, Linux 8/8); 39dabd0 passes; bisect isolates 001851f.
    • Presence matrix dumped for all seven rows before writing expectations: shared is unknown-coverage in six rows and present only in the short-circuit row; the alias slot is unknown-coverage in all seven.
    • With this change the file is 54/54 green and javascriptSemanticAnalysis.reference_lifetimes.test.ts stays 79/79 green; the four remaining JavaScript failures in my local full-directory run are the known Windows-only ?/# filename NTFS artifacts, untouched by this change.
    • npm run check:fast passes.
  • User-visible CLI/MCP output: not applicable.

…ation

001851f threads capture-time ordering through member-mutation
references, so a guarded mutation nested in a logical expression
shares its statement list with the preceding reassignment and the
analysis proves the mutation cannot reach the earlier alias target.
The if-guarded form keeps its own block body, retains no same-list
proof, and stays unknown-coverage, so the table now states each row's
expected shared presence explicitly and asserts the mutation itself
stays recorded on the alias binding in every case.
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

@BaaRai

BaaRai commented Oct 10, 2026

Copy link
Copy Markdown
Contributor Author

Closing in favor of b8a8190, which covers this correctly and picks the better direction. I had read the short-circuit precision as the intended outcome of 001851f and updated the expectation to match; the guarded-mutation rule in b8a8190 (no statement position for logical-RHS and conditional-branch mutations) is the right call: it keeps presence stable across semantically equivalent guarded forms (if (flag) m() versus flag && m() versus flag ? m() : noop()), which matters for evidence comparison, and it preserves the documented uncertainty contract the original test encodes. Verified the file is green on main with your fix. Thanks!

@BaaRai BaaRai closed this Oct 10, 2026
@morluto morluto added area: tests Test suite and verification area: native Native analysis and decompilation, including Ghidra, IDA, and Hopper analysis behavior labels Oct 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: native Native analysis and decompilation, including Ghidra, IDA, and Hopper analysis behavior area: tests Test suite and verification

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants