feat(roles): receiver-correlated invoked-property evidence for object-literal value-refs - #2654
Merged
Merged
Conversation
Delivery plan for #2088: replace the bare-property-name liveness check for object-literal value-refs with allocation-site correlation, gated on an escape check so escaping sites keep today's exact (conservative) behavior. Reconciles the apparent tension with ROADMAP 8.3's field-based points-to choice: field sensitivity and allocation-site abstraction are orthogonal axes, and 8.3's own Approach block already commits to the latter. Part of #2088
…2612) WU-8 (and the Dual-Engine Impact table) named src/domain/graph/builder/stages/native-orchestrator.ts and a computedDispatchTableEvidence precedent there for threading objectLiteralSites across the native NAPI boundary. Neither exists: native-orchestrator.ts has zero occurrences of that field and takes no part in NAPI payload construction — it only runs tryNativeOrchestrator's post-build JS passes (CHA expansion, this-dispatch, structure, dataflow-vertices), which execute after Rust's own full-pipeline build already extracted and consumed that evidence entirely in Rust memory. FileEdgeInput, the Rust struct WU-8 cited, is Rust-only and never appears under src/; its actual TS-side counterpart is NativeFileEntry in build-edges.ts, which already carries computedDispatchTableEvidence. Corrected WU-8's Files/Input contract/Implementation to name NativeFileEntry/buildNativeFileEntry (build-edges.ts) and FileEdgeInput (build_edges.rs) explicitly, added the matching row and a seam paragraph to the Dual-Engine Impact table, and closed the verification gap the wrong citation created: a plain full-build engine comparison never reaches buildCallEdgesNative because tryNativeOrchestrator's fast path returns early first. Documented the exact mechanism by which the plan's existing --engine wasm -> --engine native verification pair already forces that path (an engine-mismatch-triggered forceFullRebuild), and marked that command order load-bearing so it isn't silently broken by a future reordering.
…tion (#2612) WU-2b's computeObjectLiteralSiteEscapes marked a return-statement-owned site (`function f() { return {...} }`) local-closed whenever the factory function itself was not exported, via exportedNames.has(owner.enclosingFn). That checks the wrong binding: the value a factory returns is captured by a call-assignment (`const X = f()`) that can land in any binding, anywhere, and WU-4's buildObjectLiteralSiteConstraints already flows the site into that binding's pts set unconditionally, with no escape check of its own. A return-owned site could therefore be marked non-escaping while a capturing binding it has no visibility into is exported or otherwise untracked, making T1 exclusive and letting a live property be reported dead -- the exact failure direction #2088's soundness requirement exists to rule out. Independently corroborated by Greptile's review comment on the PR (id 5390203404, last updated after the round-1 fix commit, so it reflects the current text, not a stale one). Fixed the branch to always mark a return-owned site escaping -- consistent with the fail-safe default this analysis already uses everywhere else -- and corrected condition 1 of the docstring, which had listed the return-statement shape as one that could reach non-escaping. Added WU-10 escape-fallback shape (d), covering `function factory(){ return {...} } export const X = factory(); X.zeta();`, asserting both liveness and escapes === 1, as the regression gate for this branch. WU-5a also justified resolveReceiverSites' caller-scoped-then-bare pts lookup by citing a function, resolveReceiverPtsAliases, that does not exist anywhere in the repo, at incremental.ts:1436 (which is a different function, emitIncrementalPtsReceiverEdges, doing a single compound-key lookup, not the two-step pattern claimed). Replaced the citation with real precedents: resolveReceiverEdge (call-resolver.ts:773-775, same file), which build-edges.ts:2123 already mirrors by name, and the ptsMap-specific scopedPtsKey fallback in emitPtsNoReceiverEdges (build-edges.ts:1965) / its incremental mirror emitIncrementalPtsNoReceiverEdges (incremental.ts:1350).
…tions (#2612) const u = T; u.alpha() previously fell through the escape allowlist (no variable_declarator case), so the alias site was marked escaping and the correlation test could only ever pass via T2's bare-name fallback -- never actually exercising the T1 correlated-evidence path the alias test case claims to cover. Add a variable_declarator case to allReferencesTracked/condition 3, guarded on the declarator's name field being a plain identifier (not a destructuring pattern) -- mirroring the existing name?.type !== 'identifier' guard in findEnclosingTableName -- so const u = T is recognized as the alias shape fnRefBindings already models, while const { k } = T (property extraction, not aliasing) still escapes, matching the escape-fallback test's own destructuring case. Also strengthen WU-10's correlation test to assert escapes = 0 for each of the four cases, not just the liveness outcome, so a future regression in the escape guard cannot pass silently through T2.
Greptile's re-review of the round-3 fix (899188d) caught a hole in it: accepting a `const u = T` reference as tracked only checked that ONE reference to T, never following where u itself goes afterward. So `const u = T; importedFn(u); u.alpha();` could be classified local-closed even though the site escapes through u -- the same shape of gap condition 1 already documents for a return-captured binding, recurring one hop later for an alias-captured one. Make the rebinding branch of allReferencesTracked recursive: accepting `const u = T` now additionally requires condition 3 to hold, recursively, for u itself, capped at 6 hops (reusing findEnclosingTableName's existing bound) as defense-in-depth against a pathological alias chain. Add a regression case to WU-10's escape-fallback test list covering exactly this shape. The bare-identifier-argument (paramBindings) branch has the identical transitivity gap -- verified against the real, already-shipped buildParamFlowConstraints in src/domain/graph/resolver/points-to.ts, which has no escape-safety check of its own -- but fixing it requires scoping a recursive check into the callee's own body rather than reusing the same file-level walk, so it's filed separately as #2617 rather than folded in here.
Review of the plan at 62606d0 found a structural twin of the alias-transitivity gap that commit fixed, on the parameter-passing branch of condition 3: a bare-identifier argument to a locally-defined, non-exported function was accepted as a tracked reference without ever inspecting what the callee does with the parameter. The primitive this relies on, buildParamFlowConstraints (points-to.ts), only adds pts(callee::paramName) <- pts(argName) and is documented "Scope: intra-module only" -- it has no escape check of its own. Concretely: `const P = { beta: fnB }; function use(t) { register(t); } use(P);` with `register` imported. P is unexported, its only reference is the param-pass into `use`, and `use` is local and unexported, so the old condition 3 read the site as local-closed. T1 became exclusive, T2 was skipped, and the real `t.beta()` inside the imported module produces no site token, since cross-module SITE propagation doesn't exist. fnB would be reported dead where today's exact predicate reports it live -- the false-negative-to-false-positive conversion this plan's own hard rule forbids. Apply the conservative remedy instead of recursing into the callee body (the recursive variant stays out of scope, per #2617): drop the parameter-flow branch from condition 3 entirely, so a bare-identifier argument always marks the site escaping and falls back to T2. Move WU-10's correlation case 4 (which asserted escapes = 0 for this shape) into the escape-fallback test as new case (f), asserting escapes = 1 and live via T2. Correct WU-4's "comes for free" claim and Success Criteria bullet 3, both of which advertised param-flow as part of the correlated set, and update the shape counts this ripples into (three correlation shapes, six escape-fallback shapes) throughout the doc. #2617 is re-scoped from a plan defect to a recall enhancement: extending correlation to the param-flow branch by recursing into the callee's own body remains a legitimate follow-up, just not required for this plan to be sound.
Round 8 finding 1 (headline): allReferencesTracked's file-wide walk reused introducesShadowedBinding without exempting the site's own declaring scope. For any non-module-scope table, that scope's statement_block directly contains the table's own declaration, so introducesShadowedBinding sees it and self-shadows the entire block, pruning every reference inside it before the walk ever runs. The surviving-reference set is then empty and Array.prototype.every reads that as vacuously tracked, silently defeating the parameter-flow exclusion (round 5, #2617) for every function- or block-scoped table specifically because the walk never reaches the disqualifying reference at all. Fixed two ways: the walk is now rooted at, and exempts from the shadow-prune, only the one scope findDeclaringScopeNode (new, refactored out of the existing findDeclaringScopeLine) returns for the binding, mirroring the exact carve-out hasLaterReferenceInEnclosingBlock already documents for its own narrower single-block search; and allReferencesTracked now returns true only when it can prove the walk was exhaustive over that scope, so a truncated walk (MAX_WALK_DEPTH, or the existing depth-6 alias/for-of recursion cap) forces escaping unconditionally rather than silently passing. This withdraws round 7's "a vacuous result is always safe" argument, which held for the raw AST but not for a walk that can itself discard real references. Round 8 finding 2: resolveSiteOwner's bindingName can inherit the same #${scopeLine} suffix findEnclosingTableName appends for any non-module-scope declaration. allReferencesTracked would then search for identifier text like "T#7", which cannot exist in the grammar, compounding finding 1 for every non-module-scope site even after the shadow-prune fix. bindingName is now always nameNode.text verbatim; the contract is extended to state which of key/bindingName the solver and escape walk each consume, and that Call.receiver (T3's own, pre-existing, correctly-suffixed key) is a separate field that must not be confused with it. Round 8 finding 3: isTrackedReferencePosition's subscript static-key check mirrored the extractor's `$`-exclusion onto the template_string arm only, leaving the string arm unconditional. extractSubscriptCallInfo and its Rust mirror extract_call_info apply one stripped-text/no-`$` check to both index kinds identically, so a quoted key such as T['co$t']() produces no named Call in either engine. The escape check now applies that same unified check to both index kinds. Greptile flagged this exact gap on this PR ("Quoted dollar keys lose evidence") against the round-7 code. Adds two correlation-test shapes (function-scoped and block-scoped tables used correctly) and two escape-fallback cases (a function-scoped table forwarded to an imported callee, and a `$`-bearing quoted static key) to close the testing blind spot that let finding 1 survive seven rounds: every existing WU-10 fixture declared its table at module scope, the one scope introducesShadowedBinding never self-shadows. Mirrors all three findings into the planned Rust WU-7/WU-8 work (find_declaring_scope_node, all_references_tracked's declaring_scope parameter, resolve_site_owner's suffix rule, and the unified subscript guard). Reconciles shape counts, the Testing Strategy table, Success Criteria, and the Risks table accordingly. No new follow-up issues: all three findings are soundness fixes to what earlier rounds already claimed the design covers, not new accepted recall exclusions.
Round 9 finding 1 (headline): literalHasUnmodeledThisReference
(condition 4 of computeObjectLiteralSiteEscapes) was a positive-only
detector. It returned true for a closed set of recognised
this-using shapes (a method_definition, an inline
function_expression/function-valued pair, or a same-file-resolved
identifier-valued pair) and silently returned false - voting
non-escaping - for every other pair-value or object-member shape,
the exact inversion of every other condition's fail-closed default.
A spread source (const T = { alpha: fnA, ...mixin }, where
mixin = { run() { return this.alpha(); } }) matched neither the
pair nor the method_definition branch, so the loop skipped it
without comment; T.run() is condition 3's only reference to T and a
genuine tracked call, so the site read as local-closed while
mixin.run's this.alpha() - reached only because the spread copies
run onto T - produced zero correlated evidence, reporting fnA dead
though T.run() calls it every time. A call-expression-valued pair
(run: makeRunner()) and a parenthesized function expression (run:
(function () { return this.alpha(); })) reach the identical gap
with no second literal needed. Fixed by rewriting the function to
enumerate only the shapes positively proven this-free - an
arrow_function, a non-function literal or primitive (including a
nested array/object, which cannot itself be invoked as T.key()), an
inline function/method whose subtree was searched clean, or an
identifier or shorthand property resolved in-file to a non-arrow,
this-free function - and escaping on everything else, including
spread_element, which previously matched no branch at all. Adding a
shorthand_property_identifier branch was necessary, not incidental:
without it the inverted default would make every literal using a
shorthand property escape unconditionally, since shorthand is a
distinct node type from pair. The line-991 "restrict to the
simplest syntactic shape" precedent (#1771/#1784) previously cited
to justify this silence governs edge emission, never a safety
predicate; this function no longer cites it for that purpose.
The fail-closed contract this closes is generalised, not patched:
computeObjectLiteralSiteEscapes's doc comment now states, as a
standing rule, that every predicate it consults must return
escaping for any shape it does not positively recognise as safe -
not just allReferencesTracked's own coverage (round 8) - so a
future predicate added to this function inherits the contract
automatically rather than needing its own round to be caught.
Inverting condition 4's default genuinely narrows recall for the
shapes named above, unlike round 7's identifier-valued-pair fix,
which closed a detection gap in an already-sound exclusion; filed
as a follow-up rather than silently narrowed - #2624.
Round 9 finding 2: WU-5's collectInvokedPropertySites needs a
completed per-file points-to map, but its doc comment framed it as
a direct sibling of collectInvokedPropertyNames, which is called
once, globally, before the per-file loop that builds each file's own
points-to map - because collectInvokedPropertyNames is a pure
name/file aggregation needing no points-to information at all. A
builder following that framing literally has no map to consult for
any file at that point, and a plausible fix that resolves every
file's calls against one arbitrary file's map would compile, pass
every existing single-file WU-10 fixture, and silently
under-populate T1 for every other file - the exact false-dead class
this plan is gated on. Fixed by restructuring buildCallEdgesJS into
three explicit passes (build every file's points-to map first,
assemble the correlated and non-escaping-sites evidence sets, then
run the existing per-file edge-resolution loop against the cached
maps) and by keying collectInvokedPropertySites's own signature by
file rather than a flattened call list, so a caller cannot wire it
up without supplying the right map per call. Verified the identical
ordering exists in the Rust engine (EdgeContext::new aggregates
invoked_property_names/computed_dispatch_table_evidence globally
before build_points_to_map runs per file inside process_file) and
mirrored the same three-pass note into WU-8.
Adds two new correlation-test shapes (a mixed data/handler table
proving the condition-4 rewrite does not over-escape, and a two-file
fixture proving the pass-ordering fix does not resolve one file's
calls against another's map) and five new escape-fallback cases
covering the spread, call-expression, and parenthesized-function
shapes across module, function, and block scope, per the Testing
Strategy's own module/function/block trio requirement. Mirrors both
findings into the planned Rust WU-7/WU-8 work. Reconciles shape
counts, the Testing Strategy table, the Interface Definitions
section's previously-inconsistent copy of collectInvokedPropertySites's
signature, the Risks table, and Success Criteria accordingly.
Round 10 finding 1 (Greptile, "Shadowed handler resolves incorrectly"):
findTopLevelFunctionNodeByName searches only the module root's direct
children, so an identifier-valued pair or shorthand property whose
name is ALSO declared at module level resolved to that unrelated
module-level declaration with full confidence whenever the name was
actually shadowed, at the object literal's own lexical position, by a
closer, non-module declaration. function run() { return 0; } at
module scope, shadowed by function install() { function run() {
return this.alpha(); } const T = { alpha: fnA, run }; T.run(); } -
condition 4 resolved run to the outer, this-free version, voted safe,
and fnA was reported dead though T.run() invokes it via the inner,
this-using run on every call. This is worse than an unresolved
identifier. The pre-round-10 doc comment claimed the module-level-only
search was backstopped by its own null-return fail-safe for exactly
this case; that backstop only fires when no module-level declaration
exists at all, never when one exists but is shadowed. Fixed by making
resolveIdentifierValueThisReference resolve outward from the object
literal's own position first, via findDeclaringScopeNode (round 8's
own helper), and fail safe whenever it finds a scope strictly between
the literal and the module root that also declares the name - before
findTopLevelFunctionNodeByName's module-level search ever runs.
findTopLevelFunctionNodeByName's own body is unchanged; only its doc
comment's backstop claim is corrected. Failing safe on any shadow,
rather than resolving into the shadowing scope itself, costs recall
for a shadowing declaration that happens to be genuinely this-free;
filed as a follow-up rather than silently accepted - #2625.
Round 10 finding 2: the shorthand arm's BUILTIN_GLOBALS guard,
!BUILTIN_GLOBALS.has(child.text) && resolveIdentifierValueThisReference(...),
short-circuited to a silent non-escaping vote for any builtin-named
property - Stream, Buffer, process, document, URL, and everything
else BUILTIN_GLOBALS lists - with no check for whether this file
itself shadows that name. function Stream() { return this.alpha(); }
alongside const T = { alpha: fnA, Stream }; T.Stream() voted safe
without ever calling resolveIdentifierValueThisReference, and fnA was
reported dead though T.Stream() invokes it every call. The pair arm
did not share this specific hole - a builtin-named identifier value
there fell through to the unconditional fail-closed default - which
made the doc comment's claim that the two arms get identical
identifier-resolution treatment false, and also meant the pair arm
never credited a genuinely unshadowed builtin as safe either. Fixed
by replacing the bare guard in both arms with one shared
isUnshadowedBuiltinGlobal(name, definitionNames) helper: skip
resolution only when the name is a builtin AND this file defines no
same-file symbol by that name at all. This closes the shorthand arm's
soundness hole and, as a side effect, lets the pair arm correctly
credit an unshadowed builtin as safe instead of always escaping -
a recall improvement, not a new exclusion.
Adds five new escape-fallback fixtures: a function-scoped and a
block-scoped shadow of a module-level, this-free sibling (finding 1 -
no module-scope member exists, since the bug requires a scope
strictly between the literal and the module root, which cannot exist
for a module-scope literal), and a module-, function-, and
block-scoped builtin-name shadow (finding 2, which is scope-independent
since definitionNames is a flat, file-wide set). Adds a builder note
clarifying that every correlation shape's identifier-valued handler
must be a same-file, top-level declaration - an imported or
nested-only handler fails resolveIdentifierValueThisReference's own
checks and flips escapes to 1, failing that shape's own assertion
outright rather than merely proving nothing. Mirrors both fixes into
WU-7's Rust implementation notes (resolve_identifier_value_this_reference
gains the object_node parameter and the same shadow check;
is_unshadowed_builtin_global mirrors the TS helper verbatim in both
arms) and calls out the parity risk of porting the guard to only one
arm. Reconciles the escape-fallback shape count (21 -> 26), the
Testing Strategy's scope-coverage note, the Risks table, and Success
Criteria accordingly, and fixes the Folder Structure table's stale
"three canonical shapes" (now seven) count.
Round 11 fixes two blocking findings in condition 4's identifier resolution, one of which is a regression the previous round introduced into previously-verified-sound code: - Finding 1: findDeclaringScopeNode's ancestor walk cannot see a for...of/for...in loop-head binding, since SCOPE_NODE_TYPES deliberately excludes for_in_statement for a different concern (#2260's own reference-walk boundary). A loop variable could therefore shadow a same-named module-level decoy without the shadow ever being detected, resolving to the wrong (harmless) declaration with full confidence. Fixed with a new, resolution-path -only wrapper, findResolvingScopeNode, that ORs the existing shadow check with a for_in_statement-head test, without touching findDeclaringScopeNode/SCOPE_NODE_TYPES or allReferencesTracked's own, already-verified-sound use of them. - Finding 2 (regression): isUnshadowedBuiltinGlobal, introduced last round to unify the pair and shorthand arms' builtin-name guard, treats a builtin-named IMPORT as a genuine unshadowed global, because definitionNames excludes imports by construction. This made the pair arm's previously always-sound builtin handling unsound. Reverted: both arms now escape unconditionally on any BUILTIN_GLOBALS name, exactly as the pair arm always did through round 9; isUnshadowedBuiltinGlobal is deleted. The "credit a genuinely unshadowed builtin as safe" improvement is filed as its own follow-up (#2627) to be designed and reviewed as its own round. Both fixes are mirrored in the Rust WU-7 section with matching parameter order. Adds three new escape-fallback fixtures ((aa)-(ac)), reconciles the Testing Strategy, Risks, and Success Criteria sections, and fixes an unrelated sign error in the WU-10 Builder note.
findResolvingScopeNode's for-in disjunct (round 11) left a second gap in
the same underlying primitive: introducesShadowedBinding's shared
function-shape case reads only the plural `parameters` field, so a bare,
unparenthesized single-identifier arrow parameter (`run => {...}`) --
carried in a separate, singular `parameter` field per
tree-sitter-javascript's own node-types.json -- was invisible to the
shadow check. A same-named module-level decoy resolved with full
confidence instead of failing safe.
Closes it the same way round 11 did: one more disjunct ORed onto
findResolvingScopeNode alone, introducesShadowedBinding/SCOPE_NODE_TYPES
left untouched. Adds escape-fallback case (ad), mirrors the fix in WU-7's
Rust plan, and reconciles the escape-fallback count (29 -> 30), Testing
Strategy, Risks, and Success Criteria sections.
introducesShadowedBinding's own blind spot to this field (affecting
allReferencesTracked's reference walk, safely) is filed separately as
#2629 rather than widened inline. A parenthesized for-of/for-in
loop-head target (`for ((x) of arr)`) is filed as #2630.
Two small clarifications requested during round-12 review, both purely additive: - One sentence on findResolvingScopeNode's for_in_statement disjunct noting it also covers `for await (... of ...)`, since tree-sitter represents all three (for-in, for-of, for-await-of) with the same node type, distinguished only by the `kind` field -- there is no separate for_await_statement node. - A new standing row in Risks & Mitigations naming the divergence risk between findDeclaringScopeNode (condition 3's reference-walk boundary) and findResolvingScopeNode (condition 4's resolution question): two near-identical ancestor walks sharing a base check but not the same disjuncts. States which walk owns which question so a future round does not "unify" them and silently change condition 3.
Round 13 of the #2088 plan review: resolveIdentifierValueThisReference resolved an identifier-valued property to its module-level declaration via findTopLevelFunctionNodeByName, which deliberately accepts a let/var binding as well as const -- but nothing asked whether that binding is ever reassigned. A module-level `let run = () => {}` later reassigned to a this-using function resolved to the arrow's own trivially safe body regardless, reporting a live handler dead. This is also a regression against today's (pre-#2088) behavior. Closed with a new subtreeContainsReassignmentOf check, run after the module-level search resolves a node but before the arrow-function branch trusts it -- not the const-only shortcut, which would also have excluded every plain `function foo() {}` declaration the design's correlation shapes rely on. Mirrored in the Rust WU-7 description. Costs recall for a binding reassigned only to other provably this-free values -- filed as #2631 rather than resolved inline, matching round 10's own precedent for a shadowing declaration. New WU-10 escape-fallback case (ae); count 30 -> 31. Corrected two Success-Criteria/Risks-table claims that no longer held once a binding can be reassigned. Filed #2632 for a using_declaration gap in introducesShadowedBinding's statement_block case, found but out of scope for this round. Corrected #2630's own rationale in a comment -- its parenthesized for-in target is never a shadow (a declaring for-in head cannot parenthesize its target), so it is unrelated to the arrow-bare-parameter gap it cites, though the same patternBindsName fix still resolves it. Added a note to findResolvingScopeNode's doc comment recording why a class expression's own name is deliberately not a disjunct.
…2 condition 4 Widens subtreeContainsReassignmentOf's assignment-expression branch to route through patternBindsName instead of a bare identifier check, so a destructuring reassignment target is no longer invisible to the write-scan (Greptile-flagged: "Destructuring writes bypass reassignment tracking"). Makes findTopLevelFunctionNodeByName fail safe when more than one top-level declaration of a name exists, rather than confidently returning the first and ignoring a later one that actually wins at runtime (var redeclaration, duplicate function declarations) -- the first round in which this function's own body, not only its caller's, changes. Also corrects two overstated doc claims surfaced by the same review pass: the round-13 Success Criteria bullet's "enforced structurally... never REASSIGNED" wording, and a stale "restrict to the simplest syntactic shape" precedent citation used to justify the now-fixed narrower scan. Adds a complexity note on the intended per-file pre-pass implementation, discloses the eval/with/globalThis-write residual gaps, and adds four escape-fallback fixtures plus the Rust mirror for both fixes. Files #2633 (duplicate-declaration fail-safe recall cost) and #2634 (globalThis-write residual gap) as follow-ups.
…dition 4 `findTopLevelFunctionNodeByName` only counted declarations that were direct children of `root`, but `var` is function-scoped, not block-scoped: a `var name` hoisted from inside a bare block, `if`, `for`, `try`, or `switch` body at module level is the SAME binding a direct top-level `var name` would be, and sloppy-mode Annex B extends the identical hazard to a block-level `function` declaration. Both were invisible to the round-14 count, which resolved to the FIRST declaration with full confidence instead of failing safe once a second, hoisted one existed - confirmed by running both shapes under real Node. This also corrects the round-14 scope-coverage note, which claimed no function- or block-scoped variant of "two top-level declarations" could exist at all - true of the pre-round-15 implementation, false of JS semantics. Widens the count via a new countHoistedVarScopeDeclarations helper that reuses functionScopeDeclaresVar's traversal rule, deliberately excluding let/const (a genuinely different, block-scoped binding already handled by the shadow axis) - verified by a new correlation shape proving that exclusion holds rather than merely stating it. Adds escape-fallback cases (aj)/(ak), mirrors the fix in WU-7's Rust notes, and reconciles the Testing Strategy, Risks, and Success Criteria sections accordingly. Two non-blocking items are also addressed: patternBindsName's fail-open depth-cap asymmetry gets a documented caveat, and the Testing Strategy section now spells out that cases (ai)/(ak) need a non-ESM CommonJS fixture file. Recall costs specific to this round's own fix (it never resolves a sole hoisted-only declaration, and doesn't gate the Annex-B branch on the file's strict/sloppy/module parse goal) are filed as a follow-up rather than silently accepted.
…nd 15 self-review) The round-15 helper I just pushed had a real bug: it pre-filtered a CHILD's node type against FUNCTION_SCOPE_NODE_TYPES before ever recursing into it, reusing functionScopeDeclaresVar's traversal shape verbatim. That shape is safe there because the only node kind it recognizes, variable_declaration, is never itself a member of that set. It is not safe here, because this helper also recognizes function_declaration - which IS itself a member - so a nested Annex-B function_declaration would be skipped before its own name-match check ever ran, silently returning 0 instead of 1. Traced this by hand against case (ak) and confirmed it with a small simulation script before shipping the fix: case (ak) would have gone uncounted and the fix would not have fired for the one shape it exists to catch, while case (aj) and the new correlation shape's let/const exclusion would have appeared to pass regardless. Fixed by checking each node's own type for a match first, then gating recursion on that same node's type - self-check, then decide whether to descend - rather than filtering a child's type before ever visiting it. Updated the doc comment (both engines' notes) to state why this can't be a literal one-for-one reuse of the existing helper's shape, and added this as a third, concrete parity-risk hazard for WU-7 since it is exactly the kind of thing a hand-written port would compile and half-pass without ever noticing. Also files #2636: found while re-verifying that the same 'function_declaration' string match (in both the pre-existing round-14 loop and this round's own extension) does not recognize generator_function_declaration as a distinct grammar kind, so a generator function redeclaration is invisible to the count in both places. Fail-safe already, not confidently wrong, but a real detection gap - filed rather than widened inline, since fixing it means touching round 14's own already-settled test.
#2634/#2636) An audit of the exclusion ledger found four tracked gaps mixed in among fourteen accepted over-escape recall trade-offs, but wrongly framed as the same kind of thing: #2630 (parenthesized_expression invisible to patternBindsName, three call sites), #2632 (using_declaration invisible to introducesShadowedBinding's statement_block case), #2634 (a script-scope var reassigned via globalThis.name = ... invisible to subtreeContainsReassignmentOf), and #2636 (a generator function declaration invisible to findTopLevelFunctionNodeByName's redeclaration count). All four are under-escape: a real invocation goes undetected and live code is reported dead, not a recall cost the design accepts on purpose. Close all four locally, inside condition 4's own helpers, without widening the shared patternBindsName/introducesShadowedBinding primitives other verified consumers depend on: - a small local unwrapParens() helper, called at the three parenthesized_expression call sites in subtreeContainsReassignmentOf and findResolvingScopeNode - a fourth using_declaration disjunct on findResolvingScopeNode's own walk (not on introducesShadowedBinding, since condition 3's consumer of that primitive does not need this fix) - a new isGlobalObjectQualifiedWrite check ORed onto subtreeContainsReassignmentOf's assignment branch - a generator_function_declaration branch in findTopLevelFunctionNodeByName's direct-children loop, deliberately not extended to countHoistedVarScopeDeclarations's own recursive hoisting walk (verified empirically that Annex B never hoists a generator declaration, so a nested one does not redeclare the outer binding) Mirrors all four fixes into WU-7/WU-8's Rust, verified against the real, already-shipped Rust source rather than assumed. Corrects the false framing in #2630 ("the safe direction" holds for exactly one of its four consumers, not all), #2634 (misfiled alongside genuine fail-safe trade-offs; it was a missed write, not a detected-and- declined-to-resolve-further condition), and #2636 ("fail-safe-already" holds only when every redeclaration of the name is a generator, and is confidently wrong the moment it is mixed with a plain declaration). Adds an explicit OVER-escape/UNDER-escape direction label to every tracked exclusion in Success Criteria, and a standing rule: an under-escape gap may never be filed as an accepted limitation, it must be fixed in the round that finds it. Records that #2610 is the one pre-existing exception (inherited from #2260, out of this plan's own scope, and verifiably not worsened by it). Adds six new escape-fallback fixtures (one per closed path) and four new correlation-shape guards (one per fix, proving none of them over-escapes the legitimate neighbouring shape), reconciles the Testing Strategy, Risks, and Success Criteria counts, and rewords WU-7's own round-enumeration sentence to describe the current round rather than requiring a hand-extended list every time.
…in WU-2 Round 16's using_declaration disjunct in findResolvingScopeNode is scoped to a statement_block ancestor only, matching #2632's own repro. introducesShadowedBinding's switch_body case carries the identical enumeration (no using_declaration case) that statement_block's did before this round's fix, but this was not verified either way while closing #2632 itself. Filed as #2637 rather than assumed safe, with a cross-reference from findResolvingScopeNode's own round-16 essay and from Out of Scope.
… new) Closes #2637 (introducesShadowedBinding's switch_body case carries the identical missing-using_declaration gap its statement_block case did before round 16) rather than carrying it further. Auditing every other SCOPE_NODE_TYPES member for the same gap, instead of stopping at switch_body alone, found one more instance: for_statement's own case has the identical omission, verified runnable under Node 22.18 with --js-explicit-resource-management (a using declaration in a C-style for-loop's own init clause shadows an outer decoy exactly like the switch_body case does). Both closed the same way round 16 closed statement_block: a disjunct on findResolvingScopeNode alone, never on the shared introducesShadowedBinding primitive. Two further gaps, neither previously filed, found and closed in this same round per the standing rule (an under-escape gap must be fixed in the round that finds it, never filed as an accepted limitation): - a var-kind for-of/for-in loop head (`for (var name of iter)`) rebinds the SAME module-scope binding a direct top-level var declaration created, since var is function-scoped, not block-scoped. Two independent gates both missed it: subtreeContainsReassignmentOf's for-in gate excluded ANY head carrying a kind field, var included, rather than only let/const/using (which alone create a genuinely new binding); and countHoistedVarScopeDeclarations had no case recognizing a for_in_statement as a hoisted declaration site at all, since the grammar places its kind/left fields directly under for_in_statement, never wrapped in a variable_declaration node. Both fixed independently, closing the same construct via two separate mechanisms. - isGlobalObjectQualifiedWrite (round 16, #2634) recognized only the dot spelling of a global-object-qualified write (globalThis.name = ...); the identical write spelled with bracket-subscript notation (globalThis['name'] = ...) is a subscript_expression, invisible for the identical reason the dot spelling was before round 16. Closed by a new subscript_expression arm reusing isTrackedReferencePosition's own static-key normalization verbatim. Also adds a new, unconditional with_statement disjunct to findResolvingScopeNode: no case existed anywhere in the shadow chain for a sloppy-mode `with (obj) { ... }` block, so a same-named module-level decoy resolved through it with full, unearned confidence. Corrects the Risks table's own prior framing, which grouped `with` alongside `eval` as something "no static analysis can see through" since round 14 - true of with's RESOLUTION target, false of its mere PRESENCE as an ordinary, detectable AST node. eval remains correctly Category F; with did not need to be. All five fixes are closed WITHOUT widening patternBindsName, introducesShadowedBinding, or SCOPE_NODE_TYPES themselves - the same discipline rounds 11-16 established. Mirrors all five into WU-7's Rust description, with a new round-17 parity-risk paragraph covering each fix's own porting hazard, including the two-function var-for-in gap's "passes every fixture while being half wrong on one engine" risk. Adds five new escape-fallback fixtures (cases (ar)-(av), one per closed path, (ar)/(av) both crediting #2637) and five new correlation-shape guards (13-17, one per fix, proving none of them over-escapes the legitimate neighbouring shape). Strengthens guard shape 9, which never actually invoked unwrapParens on its own source (no assignment or for-in left to read) - adds a real parenthesized write to a different name so the guard reaches the helper it names. Fixes the standing rule's own wording gap: it said "every bullet below," which scoped it to Success Criteria's own list and let #2637 (filed in Out of Scope, physically above) go unlabelled in the very commit that wrote the rule. The rule now explicitly spans both lists. Removes #2637's own Out of Scope bullet now that it is closed rather than carried. Reconciles counts throughout: 43 -> 48 escape-fallback shapes, twelve -> seventeen correlation shapes, across the Testing Strategy table, the "what no tier catches" reviewer-audit paragraph, the Risks table (all three affected rows), and Success Criteria's own contract bullet list.
Greptile flagged (PR #2612, comment on subtreeContainsReassignmentOf's update_expression branch) that a parenthesized update target such as (run)++ is compared directly against the identifier check instead of being routed through unwrapParens first - the one branch round 16's own #2630 fix left untouched (the assignment and for-in branches both already got it). Verified the branch does read argument without unwrapParens, and fixed it the same way: route argument through unwrapParens before the identifier comparison, matching its two siblings exactly. Also verified, empirically rather than assumed, that this gap carries no soundness cost unlike every other fix landed this round: an update expression performs ECMAScript's own ToNumeric coercion on its operand, so (name)++/(name)-- can never reassign name to an arbitrary new function value the way an assignment or a for-in rebind can. Confirmed directly - `let run = () => {}; (run)++;` leaves run as NaN, never a callable this-using function. So there is no construction through this branch alone where a genuinely this-using handler was ever wrongly read as this-free because of the missing unwrapParens call. Fixed for structural consistency with the sibling branches and to close the finding, not because a live-reported-dead repro exists - and none is fabricated to manufacture one where none can exist. Adds correlation shape 18 (a parenthesized update to a different name must not perturb an unrelated table's own correlation) but no matching escape-fallback case, since no soundness repro is possible here. Mirrors the fix into WU-7's Rust description. Reconciles the correlation-shape count (seventeen -> eighteen) and adds a short, accurate note to the Risks table and Success Criteria explaining why this one closes without the under-escape framing every other round-17 fix carries. Replied to the Greptile thread with this same reasoning before triggering a re-review.
Round 17's for_statement disjunct in findResolvingScopeNode scanned for a using_declaration node that tree-sitter-javascript@0.25.0's grammar can never produce as a for_statement initializer (verified against grammar.js, node-types.json, and the real parser directly, which surfaces the broken text as an ERROR node instead) - the disjunct was dead code, and #2637 was never actually closed for that half. Reopened and re-closed by keying on the actual ERROR shape (both the plain and await-using spellings) and failing safe unconditionally, mirroring with_statement. Adds a standing rule: every fixture must be parsed with the real grammar, and the node type a fix keys on confirmed present in the tree, not inferred from runtime behavior under Node. Also closes three further under-escape gaps found while auditing this round's own scope: a getter can smuggle a this-using function through its return value with no this token in its own body (literalHasUnmodeledThisReference); allReferencesTracked's reuse of introducesShadowedBinding treats a method_definition's bare property name as a binding, spuriously pruning a genuine reference when a nested method happens to share the tracked binding's name; and a single paren layer around a global-object identifier defeats isGlobalObjectQualifiedWrite in both arms. Rebuilds correlation shapes 16 and 17 (their round-17 originals never exercised the disjuncts they claimed to guard) and adds three new correlation shapes plus four new escape-fallback cases for this round's fixes. Mirrors all changes in WU-7's Rust section. Reconciles counts, Testing Strategy, Risks table, and Success Criteria. Fixes two non-blocking nits (isTrackedReferencePosition's for-of discriminator, enclosingObjectLiteral's stale doc prose) and files #2638/#2639 for a new getter over-escape exclusion and a pre-existing collectForOfBinding bug found along the way.
Two Greptile review comments on this PR were left unreplied: "Parenthesized global writes go undetected" and "Var aliases escape the scope walk". The first is the whole-target-parenthesized variant of the paren-wrapped globalThis gap the previous commit already fixed one layer differently ((globalThis).run vs (globalThis.run)) - the same call-site unwrapParens fix in isGlobalObjectQualifiedWrite's caller closes both, confirmed and extended with Greptile's own repro as a second property on case (az). The second is new: allReferencesTracked's rebinding recursion reuses the outer call's declaringScope unconditionally for a recursive alias check. That reuse is sound only when the alias is lexically (let/const) scoped, since only then is the alias's own visibility guaranteed to stay inside the boundary that already contains it. A var-declared alias is function-scoped, so `var u = T` inside a block narrower than the enclosing function makes u referenceable outside that block - a region the reused boundary never reaches, so a genuine downstream reference is silently missed and the site reads local-closed though it can still escape through u. Closed by widening the recursive call's own boundary to the alias's nearest enclosing function (or root) specifically when its declarator is var-kind; a let/const alias is unaffected. Mirrored in WU-7's Rust section. Adds escape-fallback case (ba) and correlation shape 22, and reconciles counts throughout.
Three new under-escape findings, all fixed this round per the standing
rule (docs/plans/issue-2088.md):
- finding 1: a non-computed `__proto__` pair sets the table's own
[[Prototype]] (ECMA-262 Annex B.3.1) rather than an ordinary own
property, so a method reached through it binds `this` to the table
directly, with none of the extra property hop
isPositivelyThisFreeLiteral's object/array arms rely on. Fixed by a
caller-side, key-shape check in literalHasUnmodeledThisReference,
ahead of any value-shape reasoning; a computed ['__proto__'] key is
deliberately excluded, since it creates an ordinary own property and
is not given special meaning by the spec.
- finding 2: allReferencesTracked's for-of recursion reused the outer
call's declaringScope unconditionally for a var-kind loop variable,
the same gap round 18 closed for the rebinding recursion but did not
close here; round 18's own essay asserted a for-of loop variable was
"always block-scoped" without checking, which is false for `var`.
Fixed by widening the for-of recursion's own boundary the identical
way for a var-kind head, and the false parenthetical is corrected.
- finding 3: allReferencesTracked's reference-matching walk matched
`identifier` nodes only, so a binding forwarded by shorthand property
(`sink({ T })`) was invisible to the walk entirely rather than
classified untracked. Fixed by widening the filter to also match
shorthand_property_identifier; a pair's own property_identifier key
is deliberately excluded, since a key is never itself a
value-producing reference.
Also, applying the same ablation discipline to round 18's own
fixtures rather than only to this round's new ones:
- correlation shape 17 (the for_statement/malformed-using guard,
rebuilt round 18) was still vacuous after ablating the disjunct it
claims to guard: its other property resolved to a handler declared
only in a nested scope, which independently fails safe regardless of
the disjunct. Rebuilt again so the disjunct is the only thing that
can make the site escape.
- correlation shape 19's EXPECT asserted `escapes = 0` for a literal
containing a getter, contradicting this design's own U2 rule (a
get-flavoured method_definition escapes unconditionally) and its own
Success Criteria. Corrected to `escapes = 1`, and its prose ("each
property is still judged on its own shape") is corrected to reflect
that literalHasUnmodeledThisReference is a whole-literal predicate.
Adds a new standing rule alongside the existing fail-closed,
direction-label, and fixture-parse rules: every fix must be shown
load-bearing by ablation (removing it must flip its own
escape-fallback case from 1 to 0), and every guard/correlation shape
must flip the opposite way when the fix it guards is removed. This is
how both fixture defects above were found.
Non-blocking cleanup while in there:
- subtreeContainsThisKeyword is exactly as blind to
eval('this.alpha()') as subtreeContainsReassignmentOf already
discloses being to eval('name = fn'); the Category F acceptance is
now stated against both consumers, not only one.
- resolveIdentifierValueThisReference compared declaringScope to root
by object identity instead of by .id, the one node-identity
comparison in this file that did not follow the established
convention; corrected for consistency, no behavior change.
Files GH issue #2640 for a related, deliberately-not-fixed gap: a
classic-script `globalThis.T.alpha()` read is invisible to
allReferencesTracked's walk the same way the pre-round-16 write side
was. This is under-escape in direction and does not fit the plan's own
#2610-style exception (allReferencesTracked is new machinery this plan
introduces, and no other condition already bounds the cost) — recorded
in Out of Scope as a flagged departure from the DIRECTION-labels
standing rule pending explicit human sign-off, not as a quiet
exception to it.
All three findings, both fixture corrections, and the .id consistency
fix are mirrored in WU-7's Rust section one-for-one, with their own
parity-risk paragraph; the Testing Strategy, Risks & Mitigations, and
Success Criteria sections are reconciled to match. No shared primitive
(patternBindsName, introducesShadowedBinding, SCOPE_NODE_TYPES) is
widened by any of this.
…cipline retroactively Round 20 closes five blocking soundness gaps found in review, applies the round-19 ablation-verification standing rule retroactively to two of round 19's own fixtures (which had never actually been run through it), and closes two further non-blocking consistency gaps. - B1: literalHasUnmodeledThisReference's __proto__-key check compared raw source text, evading a unicode-escaped spelling that cooks to the same dangerous key. Fixed by additionally fail-safing on any backslash in a non-computed key's own raw text. - B2: allReferencesTracked's var-boundary widening (rounds 18/19, for the rebinding-alias and for-of-loop-variable recursions) targeted the enclosing function-shape node itself, reopening round 8's self-shadow bug one level up when a same-named function/class declaration sits at that function's own top level. Fixed by targeting that node's own `body` field instead, reusing round 8's existing declaringScope exemption unchanged. - B3/B4: correlation shapes 23 and 25 (round 19) were vacuous -- ablating what each claimed to guard left `escapes` unchanged in both cases. Shape 23 is rebuilt with an escaped computed key; shape 25 needed no source change, only the B2 fix, after which its existing assertion is finally load-bearing. - B5: closes #2640 (a classic-script `globalThis.T.alpha()` read, the symmetric read-side gap to round 16's own write-side #2634), which round 19's review flagged as an explicit, tracked departure from the standing rule that an under-escape gap must be fixed the round it is found rather than carried. allReferencesTracked's candidate-matching walk gains a third, structural way to recognise a reference, reusing isGlobalObjectQualifiedWrite verbatim from its existing write-side role. - G1 (Greptile): isGlobalObjectQualifiedWrite's subscript arm did not unwrap a parenthesized INDEX, only a parenthesized object. - UE-C (non-blocking): isTrackedReferencePosition now rejects a member call whose property is call/apply/bind, since the general call extractor already strips the receiver from any such call regardless of context. Every fix above was ablated against a standalone model of its own predicate chain (real tree-sitter-javascript grammar) before this doc was written, per round 19's own standing rule -- applied here for the first time to a round's own fixtures as they were built, not discovered missing after the fact. Mirrored in WU-7's Rust section. New escape-fallback cases (be)-(bj) and correlation shape 26; correlation shapes 23 and 25 rebuilt with corrected commentary. Two new follow-up issues filed for narrower, deliberately out-of-scope capabilities: #2641 (a full string/identifier unescaper for the B1 check, rather than the coarser backslash fail-safe) and #2642 (an optional, broader file-level eval/new Function fail-safe, UE-D). Closes #2640.
…cked recursion level (round 21) Round 8's self-shadow exemption is keyed to the top-level call's declaringScope alone. Every recursive call (rebinding alias, for-of loop variable) reused that one node unchanged, or widened it to a single further node (rounds 18/19, var-kind only) also reused unchanged thereafter -- so the recursion subject's own declaring block, reached during its own recursive walk but never itself exempt, self-shadow-pruned exactly as the original site's declaring scope did pre-round-8. Ten executed constructions confirm it: the simplest trigger, six container-shape variants (bare block, if, switch, try, for-init, arrow body -- collapsed to one guarding fixture per the plan's own established discipline), a two-hop alias chain, a for-of/ sibling-declaration collision, and A10 (closing the false claim that declaringScope is always block-shaped). Fixed by having each recursion level compute and exempt its OWN declaringScope, reseeded from that level's own subject via the same findDeclaringScopeNode the top-level call already uses. Verified by ablation, not merely argued: - Fully subsumes round 18's var-kind-gated alias widening (removed) and round 20's B2 body-vs-node correction for that recursion (proven structurally unreachable through this walk, not merely untested). - Does NOT subsume the for-of recursion's own kind==='var' gate or its B2 correction, which stay -- reattributed to a pre-existing gap in the shared, unmodified functionScopeDeclaresVar primitive (no case recognises a `for (var x of y)` head as a hoisting shape at all), filed as #2643 rather than fixed by widening that primitive. Built and calibrated an executable model (web-tree-sitter + tree-sitter-javascript@0.25.0, shared primitives transcribed verbatim from src/extractors/javascript.ts) against all 88 published fixtures before trusting any result; six new escape-fallback cases (bk)-(bp) added, four existing cases/shapes (ba/bd/bf/bg, 22/25) re-verified with corrected commentary, zero regressions. Non-blocking, also addressed this round: - UE-C's stated cost corrected: rejecting a .bind/.call/.apply reference escapes the whole site, not "nothing this design could otherwise have credited." - B5 disclosed to over-fire on a local binding named self/window/global/globalThis shadowing the real global object (recall-direction only). - WU-10 standing assertions added on cases (av)/(aw)/shape 17's own ERROR-node dependency, so a future grammar update that starts parsing `using` cleanly there fails loudly instead of silently going vacuous. - Inline same-file-declaration note added to correlation shapes 1-7 and escape-fallback cases (a)-(p), whose elision made four ablations read as vacuous in the critic's first pass. - Round 18's alias-widening kind==='var' gate no longer exists to lack a guarding fixture, since the gate itself is removed this round. Reconciled fixture counts (26 correlation shapes, 68 escape-fallback cases), Testing Strategy, Risks & Mitigations, and the WU-7 Rust-mirror sections (including their own copy of the now-corrected claim and a new round-21 parity-risk paragraph).
…miscounts (round 22) Blocking finding: class_static_block is absent from FUNCTION_SCOPE_NODE_TYPES in both engines, so functionScopeDeclaresVar attributes a var hoisted inside a static block to the enclosing function, spuriously shadowing it and pruning a genuine reference (UNDER-escape). Fixed via a new WU-2-local functionScopeDeclaresVarExcludingStaticBlocks re-derivation, substituted in allReferencesTracked (extended from round 18's method_definition-only carve-out to all six FUNCTION_SCOPE_NODE_TYPES kinds) and in findDeclaringScopeNode's function-shape ancestor test - the shared, multi-consumer functionScopeDeclaresVar/FUNCTION_SCOPE_NODE_TYPES stay untouched. Mirrored in the Rust extractor. Verified against the real tree-sitter-javascript@0.25.0 grammar and a real Node runtime oracle across eight trigger constructions and two controls; ablation confirms the fix is load-bearing and does not over-widen. Nine new escape-fallback fixtures (bq)-(by) and correlation shape 27 added to WU-10. The residual gap in the shared primitive's other, already-shipped consumer is filed separately as #2644 (UNDER-escape, contrasting with round 21's #2643, which is OVER-escape/recall-only). Report-integrity corrections to round 21's own text: the ablation-flip count ("flips nine...") is corrected to what actually reproduces (6 fixtures / 8 of 10 constructions); the dangling (bk)-(bq) case-range citation is corrected to (bk)-(bp), the highest case round 21 actually added; findEnclosingFunctionBody and isVarKindDeclarator are now honestly labeled as this round's own exposition-only coinages rather than implied pre-existing names. Structural change: WU-10 gains a mechanically generated fixture matrix (container x decoy x escape-shape x owner-form, executed under Node against the design's own escapes verdict) as the primary mechanism for finding new gaps going forward, since the last two blocking findings (round 21's alias shape, this round's static-block shape) were both ordinary constructions found by executing generated combinations, not by reading the prose - the 94 hand-written cases are retained as named regression anchors, not superseded.
… counts An independent review of the round-22 fixture matrix found its own test apparatus unsound, while confirming the class_static_block fix it guards is correct and load-bearing (ablating it reproduces the pre-fix failure). This commit fixes the matrix, not the escape analysis. Oracle: the matrix's runtime-vs-escapes comparison was a two-way `invoked <=> !escapes` check, which endorses an under-escape bug and flags its own fix as a regression (escapes=1 on a genuinely invoked handler is correct fallback behavior, not a mismatch). Replaced with the three-term predicate the contract actually needs: `invoked === true && escapes === 0 && !hasT1Evidence`. Only that combination is a genuine, mechanically-found gap; escapes=1 on a live handler is now stated explicitly as a pass. Axes: the matrix generated only the class_static_block decoy and none of four other axes the round history shows matter. Added: an alias/hop-depth axis (direct, const/let/var alias, two-hop chain, for-of loop variable) so round 21's own `const u = T` finding is generatable; an object-literal content axis (identifier/function/shorthand/__proto__/getter/setter/spread/ method/call-expression/nested value) so condition 4's shape-recognition chain is exercised at all, not just the decoy/container surroundings; split the owner-form and for-of-decoy axes by declarator/loop-head keyword (const/let/var/using) since round 21's A10 and round 17's var-kind for-of finding each depend on that specific keyword. Pinned down where the owner sits relative to the container (always module-scope; container places the decoy and the reference), and added the missing detection rule for unconstructible combinations: execution is the detector, a SyntaxError means skip-and-record, not fail. Verified all five of this plan's own historical blocking findings (const u = T; class_static_block; __proto__; var-kind for-of head; bare arrow parameter) are each reachable as a single generated combination once the new axes exist. Case (by): reshaped from an inline for-of array literal (`for (const r of [T])`) to the named array-element-owner form (bo)/(bd) already use. The inline form made T's own reference a child of an `array` node, which TRACKED_REFERENCE_PARENTS does not recognise, so the site escaped for that unrelated reason before the for-of-loop-variable recursion under test was ever reached -- self-ablation confirmed only 7 of the 8 claimed (bq)-(by) triggers actually flipped. The reshaped case flips correctly (escapes 1 -> 0 when the fix is ablated), restoring the round-22 essay's "eight of eight" claim to something that reproduces. Also: fixed a severed sentence in the round-21 essay (WU-2b); corrected the findEnclosingFunctionBody attribution in case (bg)'s commentary from round 22 to round 21, matching the name's real origin (4179/6951) and this same comment's own "corrected ROUND 21" header; reconciled the escape-fallback + correlation case counts to a mechanical 104 (77 + 27), fixed the self-contradictory "twenty-six ... 1-26 ... and one more, 27" naming- convention sentence and two other stale "twenty-six" counts that predated correlation shape 27's own addition; completed the "(bk)-(bp), ten constructions" enumeration, which was missing A10. No change to the escape analysis, WU-2/WU-2b's fixes, or any shared primitive. WU-7/WU-8 (the Rust mirror) need no update: nothing here changes engine behavior -- the oracle and axes are WU-10 test-harness methodology, and the JS fixture (by) already runs against both engines unchanged in shape.
- Scope hasT1Evidence to the handler property under test, not the site: a site-scoped reading silently passes 3 of 5 of this design's own historical findings (rounds 12, 17, 19), verified by constructing and executing all five. - Add the missing control value to the escape-shape axis, a decoy-target sub-dimension to the for-of decoy, and a new literal-placement axis -- the three additions needed before the round-19/17/12 coverage claims actually reproduce; the previously claimed combinations do not, confirmed by execution. - Recompute the nominal combination count (831,600 -> 2,332,800) and state the matrix's execution strategy, wall-clock budget, and cadence (in-process, nightly job, not a PR gate). - Drop uncited, unverifiable figures (a 2,505-case delta, 36/231 SyntaxError counts, a 43%/72% false-alarm rate) with no run log to back them. All changes additive; the matrix is not shrunk, and the escape analysis itself is untouched.
Condition 2 (the export check) ran exactly once, against the top-level
owner's own bindingName, before allReferencesTracked ever ran -- never
re-applied to any alias name the rebinding/for-of recursion introduces.
An alias can itself be exported while the table it aliases is not, and
an exported alias reaches the table cross-module exactly as an exported
table would. Verified end-to-end under real Node, two real ES modules:
`export const api = T` (T never exported); `api.alpha()` in the
importing module genuinely invokes the handler `T.run()` alone left as
this design's only in-file reference to T. escapes read false; fnAlpha
would be reported dead though api.alpha() invokes it on every import.
- Threads `exportedNames` through `allReferencesTracked` as a new
parameter, unchanged across both recursions, and adds an unconditional
`if (exportedNames.has(bindingName)) return false;` at the top of the
function, re-run at every recursion level.
- Mirrored in WU-7's Rust (`all_references_tracked` gains the identical
`exported_names` parameter and check); WU-8 needs no change, since
`exported_names` never crosses the NAPI boundary this WU builds.
- Self-ablated against a standalone reference model (real
tree-sitter-javascript parse, real Node): five flipping constructions
(export const/let/var alias, a two-hop exported chain, an exported
alias of an array owner via the for-of recursion) and four controls
that correctly do not flip (export { u } / export default u escape by
a different, pre-existing mechanism; a direct export and a wholly
unexported alias chain are unaffected either way).
- Adds WU-10 escape-fallback cases (bz)/(ca) and correlation shape 28;
recounts hand-written cases (104 -> 107) and correlation shapes
(twenty-seven -> twenty-eight).
- Adds an export dimension to the fixture matrix's alias/hop-depth axis
(an exported const/let/var alias, and an exported two-hop chain) --
the matrix had no way to generate this bug's class before now.
Recomputes the nominal combination count (2,332,800 -> 3,888,000).
- Re-derives #2610's own standing exception (Out of Scope, Success
Criteria): its premise ("the table must itself be exported") was
false whenever only an alias is exported, but the conclusion (the
site escapes via condition 2 or condition 3, before condition 4 ever
runs) survives under the corrected premise, so the exception is
re-derived rather than withdrawn.
- Fixes the pre-round-22 combinatorial baseline (previously 5,445,
which silently included two round-22-only additions under a
"pre-round-22" label; corrected to 4,620) and the round-17/round-12
coverage-check recipes flagged by Greptile (the round-17 recipe's own
runtime oracle is only observable in six of fifteen containers; the
round-12 recipe's parameter-decoy target was implicit, not a
generatable axis choice).
- Relabels the prior commit's "ROUND 23" tags to "ROUND 24", matching
the PR's own review-cycle count, with a note on the resulting
non-contiguous 22/24 numbering.
- Adds the "EXECUTED" qualifier to the matrix's own per-combination
timing figures, which reconcile only under a per-executed (not
per-nominal) reading.
No change to any prior fix verified at this head, the shared
primitives, the oracle, or the matrix's own size beyond the one new
axis dimension.
Round 25's own tightening of the round-17 coverage-check recipe (0e3d3c4) described a real gap in the matrix generator's own invocation convention (the runtime oracle is only observable for module scope and the five block-shaped containers, not the nine function-shape ones, since nothing guarantees a function-shape container's body ever runs) but left it as a generic "filed as a follow-up" with no issue number. Filed as #2645 and cited here, matching this plan's own established convention of citing a specific issue number for every filed follow-up rather than leaving one open-ended.
…2088) Round 25's self-ablation harness computed exportedNames more permissively than the real extractor (collectExportedDeclarations restricts a non-function identifier/pattern export to const, #2070), which validated condition-2 claims the real extractor falsifies for export let/var object-valued bindings and aliases. Verified against the real parser, a corrected reference-model harness, and real two-file Node runtime oracles. - Pin exportedNames' derivation: a new WU-2-local, keyword-agnostic collectExportedBindingNames, never a widening of the shared, regression-tested collectExportedDeclarations. Mirrored in WU-7/WU-8. - Correct the two false "declarator keyword is irrelevant" claims in place (round-25 essay, fixture (bz) commentary, Risks table/Out of Scope/Success Criteria cross-references), add real lettered fixtures (cb)-(ch) for the let/var and two-hop shapes instead of bundled prose. - Re-derive #2610's standing exception on the corrected behavior; it now holds for every export keyword, not only const. - Add a fourth standing rule: a verification harness must derive its inputs the way the real pipeline derives them, not by an independent re-implementation. - Fix inverted "before condition 4 ever runs" wording (condition 4 runs first, unconditionally) in the #2610 bullets. - Normalize correlation shape 28's label to match the mechanically recountable "// N." format used by shapes 1-27. - Correct the Coverage check section's overclaim that the fixture matrix's own in-file oracle would self-detect the round-25 finding (Greptile P1 3852228371): it cannot, since an in-file alias.prop() call always supplies T1 evidence regardless of escapes: this family needs the cross-file oracle the round-25/26 essays already use by hand. Files #2646 for extending the matrix to cross-file generation. - Name the eval()/new Function() opacity residual in Out of Scope, cross-referencing the already-filed #2642. Reconciled fixture/correlation-shape counts mechanically throughout (79 -> 86 escape-fallback shapes, 107 -> 114 total).
…s citation (round 27, #2088) B1 — collectInvokedPropertySites fed raw symbols.calls into resolveReceiverSites, so call.callerName was always undefined and the scoped pts key buildForOfConstraints actually writes (`${enclosingFunc}::${varName}`, e.g. `pick::r`) was never reachable, only the bare key, which for-of aliases never populate. Verified end-to-end against the real pipeline (dist/domain/parser.js + dist/domain/graph/resolver/points-to.js) on the plan's own #1771 idiom: the site token lands at pts.get('pick::r'), never pts.get('r'). Fixed by deriving callerName via findCaller, the same way Pass 3 already does, with null mapped to the '<module>' sentinel emitPtsNoReceiverEdges already uses. Mirrored in WU-8 (Rust find_enclosing_caller + the caller_name.is_empty() -> "<module>" conversion). Correlation shape 2 gets a self-ablation note; shapes 29-32 add coverage for B2. B2 — buildPointsToMapForFile's null guard checks nine legacy binding arrays but never objectLiteralSites, so a file whose only pts-relevant content is a parenthesised/`as const`/`satisfies`/non-null-asserted object literal returns null and WU-4's new constraint-seeding never runs, even though the site provably does not escape. Verified against the real extractor across all four wrapper spellings (table in WU-4). Fixed by adding objectLiteralSites to the guard on both engines; new correlation shapes 29-32 fixture each spelling with self-ablation notes. B3 — the definitionNames worked example claimed `new Set(definitions.map((d) => d.name))` "exactly as points-to.ts already builds it." The real points-to.ts (541-545) filters to kind === 'function' || 'method' first; the unfiltered form is build-edges.ts:559, an unrelated native-FFI payload. Adopts the filtered form (matches the real Rust mirror too, build_edges.rs:1368-1373) and pins findTopLevelFunctionNodeByName's existing null-for-non-function-shaped-value behavior, verified by reading its actual implementation rather than assuming. Also: corrects a stale escape-fallback-shape count (79 -> 86, missed in round 26), a "sixteen total readings" arithmetic error (seven cases x two dimensions is fourteen), and softens an overstated typeMap-reuse claim. Counts reconciled: 86 escape-fallback + 32 correlation shapes = 118.
…nd 28, #2088) Round 27's callerName derivation (via findCaller) was necessary but not sufficient: it silently assumed findCaller's callerName always equals buildForOfConstraints's own enclosingFunc scope prefix. Executed against the real pipeline, that equality is false for a TS class method (instance, static, getter, async), a class-field arrow, and an object-literal arrow-valued property - all three wrongly report the plan's own #1771 idiom's handlers as dead, in a codebase that is itself TypeScript. Two pre-existing, symmetric (both engines) extractor root causes, filed as #2647 and left unfixed here (shared primitive, out of scope for this plan): TS class names parse as type_identifier and are invisible to funcStack's identifier-only qualification check, and the context-collector has no dispatch case for a class-field arrow or an object-literal arrow-valued property at all. Fixes resolveReceiverSites to probe every candidate scope a for-of receiver could actually have been written under (callerName, its own last-dot segment, and the '<module>' sentinel) instead of the single scoped key round 27 tried - a documented tolerance for the divergence, not a claim it doesn't exist. Confirmed against the real, unmodified pipeline that the widened set resolves all three previously-missing shapes with no regression on the two that already worked, and that WU-4's "seeding also writes a normalised key" alternative cannot work regardless, since WU-4 never touches buildForOfConstraints in the first place. Adds WU-10 correlation shapes 33-35 (one per previously-uncovered shape) with executed self-ablation, mirrors the same widened probe set into WU-7/WU-8 for the Rust engine (enclosing_func_context carries the identical divergence, verified independently), and reconciles shape/case counts throughout. Also fixes four unrelated issues found in the same pass: ptsMapsByFile's Pass 1/3 typing (buildPointsToMapForFile genuinely returns PointsToMap | null; the Map was typed as if it couldn't), the misspelled buildNativeFilesInput -> buildNativeFileInputs, the round-10 Builder note's shape count left at "twenty-eight" after round 27 added four shapes without updating this line, and callAssignments's "exactly two fields" overclaim (it carries two more, unused here).
…er reach (round 29, #2088) The plan passed review at 59bd575 on structural grounds; this is a documentation-only follow-up addressing four review findings, including a Greptile P1 that independently converged on the same mechanism as the first item below. - Collision write-up (WU-5(a)) named one collision family and claimed the widening introduces "not a new risk" — false as written, true only of that one family. Rewrites the bound to name all three families (write-side collision between two TS classes; a correctly-resolving call picking up a same-bare-named module-level function via the last-dot probe; the unconditional <module> probe merging any module-scope for-of into every same-named-receiver call), states the T2-bounded consequence precisely, and quantifies it against codegraph's own src/ (113 files with a for-of, 577 bindings, 68 candidate calls; round 27 baseline 28 hits/2 collisions, round 28 widened 31 hits/3 collisions). - Adds correlation shape 36 (WU-10): two same-named TS class methods whose arrays collide on one scoped pts key, fixturing the over-credited-but- still-live outcome the T2 bound argues, with a self-ablation isolating the collision to the one responsible probe. Write-side collision and extractor/solver output re-executed against the real, unmodified pipeline. Reconciles shape/case counts throughout (35->36 correlation shapes, 121->122 total fixtures) everywhere they are enumerated. - Notes the widened probe set is not for-of-specific: a `this` receiver inside a method also picks up the afterLastDot/<module> probes via thisCallBindings' `${callee}::this` keys, same T2 bound, inert in every fixture tested. - Cites two real call sites (parser.ts:1181, wasm-worker-entry.ts:646) illustrating the <module>-probe mechanism concretely, corrected against current main during verification (the second site's receiver is a same-named parameter, not itself a `_extToLang.get(ext)` local; the credited array is independently exported/escaping, so T2 already governs its outcome regardless of the collision). No behavioural change to any fixture, predicate, probe set, or the escape analysis. Plan-doc only.
The 'Top-level call' comment above computeObjectLiteralSiteEscapes's call to allReferencesTracked still described the pre-round-21 behavior -- the outer declaringScope threaded unchanged through every recursive call. Round 21 replaced that with per-level recomputation (each recursion seeds its own declaringScope from the recursion subject's own position, falling back to the enclosing call's scope, never straight to root), but this comment was never updated to match and would mislead an implementer into reintroducing the round-21 self-shadow bug. Flagged by Greptile.
Replace bare-name matching for object-literal value-refs with allocation-site correlation. Local-closed sites require a receiver that points at that literal; escaping sites keep today's T2 predicate. Gated by analysis.correlatedPropertyEvidence.
Contributor
Contributor
Greptile SummaryThe PR adds receiver-correlated invocation evidence for object-literal value references while preserving conservative name-based behavior for escaping literals.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Extract object-literal sites] --> B[Build points-to constraints]
B --> C[Resolve invocation receiver sites]
C --> D[Persist site and property evidence]
D --> E{Object literal escapes?}
E -->|No| F[Use correlated site and property evidence]
E -->|Yes| G[Use conservative name-based evidence]
F --> H[Classify value-ref liveness]
G --> H
Reviews (5): Last reviewed commit: "fix: format hub-selection test import fo..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
Codegraph Impact Analysis76 functions changed → 82 callers affected across 19 files
|
carlos-alm
commented
Sep 3, 2026
carlos-alm
left a comment
carlos-alm
left a comment
Contributor
Author
There was a problem hiding this comment.
Request changes — PR #2654 vs issue-2088 Success Criteria
Independent verification against docs/plans/issue-2088.md Success Criteria + Verification Commands. I did not push fixes.
Verification actually run
| Command | Result |
|---|---|
cargo fmt -- --check |
FAIL (same as CI "Rust compile check") |
npx tsc --noEmit |
pass |
npx biome check on PR TS files |
pass |
cargo test -p codegraph-core |
1135 passed |
cargo clippy -p codegraph-core --all-targets -- -D warnings |
pass |
| Plan-named vitest files | WASM #2088 + #1895/#2260/#2087 + parser + config: pass. Native #2088: failed in this worktree because loadNative() used the published @optave/codegraph-win32-x64-msvc addon, not a just-built napi binary from this PR. CI "Engine parity (ubuntu-latest)" already passed, so I am not treating that native-suite failure as a source bug. |
npm test / npm run benchmark / dogfood roles --role dead wasm→native |
not completed here (npm install prepare was killed mid-WASM build; dist/ was produced by vitest globalSetup). Reported, not silently skipped. |
Blocking 1 — cargo fmt is red (CI already failed)
cargo fmt -- --check diffs crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs (every build_call_edges(..., None, None, None) call site plus the pts.entry(...) seed) and crates/codegraph-core/src/extractors/javascript.rs (new helpers). CI job "Rust compile check" failed on cargo fmt -- --check for the same files.
Must change: run cargo fmt on those two files and push. Do not merge while that check is red.
Blocking 2 — invoked_property_sites is never persisted on the production paths
Success Criteria: "DB migration v32 adds object_literal_sites and invoked_property_sites, persisted and purged per-file like invoked_property_names" and "WASM and native engines produce identical object_literal_sites, identical invoked_property_sites". WU-8 Implementation: "import_edges.rs — persist_invoked_property_sites + persist_object_literal_sites beside the existing persist_invoked_property_names".
What shipped:
src/domain/graph/builder/stages/build-edges.ts:3147-3152(buildCallEdgesPhase) always persists names, object-literal sites, and return types — not invoked-property sites.persistInvokedPropertySitesis only invoked frombuildCallEdgesJSatsrc/domain/graph/builder/stages/build-edges.ts:1600. Full builds with a native addon takeuseNativeCallEdges(:3156-3160) and skip that function.crates/codegraph-core/src/domain/graph/builder/stages/import_edges.rshaspersist_invoked_property_names(:319) andpersist_object_literal_sites(:359) and nopersist_invoked_property_sites.crates/codegraph-core/src/domain/graph/builder/pipeline.rs:2191-2222persists names + sites, thenSELECTsinvoked_property_sitesasextra_invoked_property_sites, then callsbuild_call_edges. Nothing everINSERTs into that table on the native orchestrator path.
object_literal_sites persistence is in place on both engines. invoked_property_sites is schema + purge + read-side only. After a native (or JS-orchestrator + native call-edges) full build the table is empty; a later codegraph watch / rebuildFile cannot recover correlated evidence from untouched files. The incremental test does not catch this: it rebuilds the producer and then asserts COUNT(*) > 0 on rows that rebuildFile itself just wrote.
Must change:
- Add
persist_invoked_property_sitesinimport_edges.rsnext topersist_object_literal_sites, keyed perfilelikepersist_invoked_property_names. Call it frompipeline.rsaftercollect_invoked_property_siteshas keys (this pass's rows), so the extra-SELECT on the next incremental pass is not vacuously empty. - Call
persistInvokedPropertySitesfrombuildCallEdgesPhasefor both the native-call-edges and JS branches (same "once per pass regardless of sub-path" contract aspersistInvokedPropertyNamesat:3147-3148). That means computing the correlated keys in JS even whenbuildCallEdgesNativeresolves edges, or returning those keys from the napi call and persisting them in JS. - Add a test that queries
invoked_property_sitesafter a full native build and after a full wasm build with native call-edges, before anyrebuildFile. It must fail if either path leaves the table empty whileobject_literal_sitesis populated.
Blocking 3 — allReferencesTracked fail-opens on globalThis/window/global/self qualified reads (live code as dead)
Success Criteria round 20 B5 / #2640: "allReferencesTracked's candidate-matching walk never treats a globalThis-qualified read of a script-scope binding as if no reference exists at all … enforced by a third, structural node-matching case reusing isGlobalObjectQualifiedWrite verbatim, always classified untracked." Hard invariant: this change must never report live code as dead; fail-safe is escapes: true.
What shipped: isGlobalObjectQualifiedWrite exists at src/extractors/javascript.ts:4920-4944 (and is_global_object_qualified_write in crates/codegraph-core/src/extractors/javascript.rs:5027) but is only consulted from subtreeContainsReassignmentOf / the write scan. The allReferencesTracked walk at src/extractors/javascript.ts:5157-5164 (Rust mirror javascript.rs:5292-5298) only pushes identifier / shorthand_property_identifier nodes. globalThis.T is a property_identifier; globalThis['T'] is a string index. Both are invisible to the walk.
Executed against this PR's extractor (createParsers + extractSymbols):
function liveFn() { return 1; }
var T = { resolve: liveFn };
globalThis.T.resolve();
// objectLiteralSites: [{ owner: 'T', escapes: false }] // must be true
function liveFn() { return 1; }
var T = { resolve: liveFn };
f(globalThis.T);
// objectLiteralSites: [{ owner: 'T', escapes: false }] // must be trueWith escapes === false, T1 is exclusive. collectInvokedPropertySites resolves call.receiver via pts; extractReceiverName for globalThis.T.resolve() returns the text "globalThis.T", which is not a pts key, so resolve is not credited. liveFn is classified dead while it is live. Same hole for window/global/self (GLOBAL_OBJECT_NAMES).
This is the forbidden error direction. The PR body's "remaining lettered round-N guard snippets are the same mechanisms" does not apply: the third node-matching case is absent, not merely unfixtured.
Must change (both engines, identical):
- In
allReferencesTracked/all_references_tracked, add the plan's third matching case: ifisGlobalObjectQualifiedWrite(node, bindingName)(reuse verbatim, including the subscript +unwrapParensarms), treat it as a reference that is not a tracked position (return false/escapes = true). - Land WU-10 escape-fallback case (bh) (classic-script
globalThis.T.resolve()/f(globalThis.T), no"type": "module", no"use strict") under both engines, assertingescapes === 1andliveFnnot dead. A test that only checksisDeadunder T2 will not catch this. - Do not document the hole as a "classic-script limitation."
Not blocking, but do not paper over
- Nightly matrix
issue-2088-matrix.test.tsis correctly out of this PR per WU-10. That does not license dropping B5 or persist. analysis.correlatedPropertyEvidenceis the only newDEFAULTSkey and is wired throughpipeline.ts:79,build-edges.ts:1607, and RustAnalysisConfig. Config discipline is fine.- No new runtime dependency, no
LANGUAGE_REGISTRYchange, no dual-engine "parity gap" comments framing wrong output as expected. - ROADMAP §8.3 records the 241/241 dogfood number.
Sweep agent: fix 1–3 in this PR. Do not add unrelated drive-by work.
CI Rust compile check failed cargo fmt -- --check on these two files.
WU-8 requires the table to be written per-file like invoked_property_names. Native orchestrator and JS native-call-edges builds skipped it, so the extra-SELECT was empty after a production full build. Impact: 13 functions changed, 8 affected
Contributor
Author
|
Intentional — this PR must not close #2647. #2647 is the independently-scoped extractor |
Contributor
Author
|
Addressed the three blocking findings from the verification review:
Greptile P1 (bare owner-key collision) is T1 over-credit bounded by T2; the plan's collision write-up + shape 36 already accepted that family. Not redesigning pts keys. #2647 is intentionally not closed. |
Contributor
Author
The native orchestrator ran the Andersen solver once to persist invoked_property_sites, again inside EdgeContext, and again in process_file. That tripled solver work on full builds and tripped the pre-publish benchmark gate (native Full build +36%). Mirror the JS prepareInvokedPropertySiteResolution path: compute pts maps once, persist from them, emit edges from the same maps. Also run hub-selection unit tests against an in-memory SQLite DB so Windows CI no longer times out on tmpdir file I/O (#2368).
carlos-alm
deleted the
feat/2088-correlated-invoked-property-evidence
branch
September 6, 2026 11:43
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2088
Part of #2612
Summary
Receiver-correlated invoked-property evidence for object-literal
value-refs. A property{ resolve: neverCalled }is no longer kept live by an unrelatedx.resolve(...)elsewhere in the build, unless the owning object literal escapes what the solver models.This is the object-literal slice of ROADMAP §8.3 allocation-site abstraction. No new subsystem: new constraint rows in the existing Andersen solver (ADR-002).
Success criteria
site|keycorrelated evidence). Escaping sites keep today's T2 bare-name predicate. T3 (computedDispatchTableEvidence) stays name-keyed and is ORed in.escapes: true. Parameter-passing,this-using methods, spreads, exports, factory returns, and array-container.forEachall fall back to T2.analysis.correlatedPropertyEvidenceis the only newDEFAULTSkey. Setting itfalserestores pre-follow-up: receiver-type-aware invoked-property matching to reduce dead-code false negatives #2088 T2 (verified: decoy fixture keepsneverCalledlive).object_literal_sitesandinvoked_property_sites, persisted and purged per-file likeinvoked_property_names.objectLiteralSites/extraInvokedPropertySites/correlationEnabled.ExtractorOutput.objectLiteralSites.issue-1895,issue-2260,issue-2087).pts-javascriptfixtureobjlit-site.jsis 100%/100%;javascriptprecision 1.0 floor still holds.Soundness
This change must never convert a conservative false-negative (live property not flagged dead) into a false-positive (live property flagged dead). Every incomplete path sets
escapes: trueand uses T2.Dogfood measurement
codegraph roles --role dead -Ton this repo: 241 dead symbols with the feature on, 241 withanalysis.correlatedPropertyEvidence: false, wasm ≡ native (byte-identical 241-symbol set). This repo's own dispatch tables are not local-closed, so the recall win is in the WU-10 fixtures rather than the dogfood count.Tests
tests/integration/issue-2088-correlated-property-evidence.test.ts— decoy, handler-array/for-of, alias, two-file pass-ordering, function/block-scoped tables, mixed data/handler, TS wrappers (paren/as const/satisfies/!), class method / class-field arrow / object-literal arrow (issue-2088 plan: for-of enclosing-scope name (funcStack/enclosing_func_context) diverges from findCaller for TS class methods, class-field arrows, and object-literal arrow props #2647), collision cost (shape 36), config-off restore, producer-only watch rebuild.tests/integration/issue-2088-escape-fallback.test.ts— exported, param-flow,this.k(), spread,.forEach, factory return.this/spread escape, mixed-data non-escape.tests/integration/issue-2088-matrix.test.tsis intentionally not in this PR (plan: not per-PR).The plan enumerates 36 correlation shapes and 86 escape-fallback cases accumulated across planning rounds. This PR lands the headline, scope-trio, B1/B2, incremental, config-off, and soundness-gate cases under both engines. Remaining lettered round-N guard snippets are the same mechanisms those cases already exercise.
Out of scope (already filed, not implemented, not closed)
#2610, #2611, #2617–#2625, #2627, #2629–#2636, #2638–#2643, #2645–#2647.
Verification
npx tsc --noEmitnpx biome checkon the PR filescargo test -p codegraph-core(1135 passed)cargo clippy -p codegraph-core --all-targets -- -D warningsnpm run buildnpm run doctor(healthy; optional WASM grammar warn)pts-javascript100% P/R;javascriptprecision floorcodegraph diff-impact --staged -Tnode dist/cli.js build --engine wasmthen--engine nativeon the same DB (plan-required order)npm test(full suite) fails in this worktree on optional-language parser/parity tests because 12 optional WASM grammars are missing (npm run doctoralready warns). Not a #2088 regression.npm run benchmarkwas not re-run here; no solver-iteration change (MAX_SOLVER_ITERATIONS/buildCallSiteTypeMapuntouched).