15 commits

Author	SHA1	Message	Date
Gergő Magyar	72edf40087	perf(store): V8 sidecars plus hardlinked ParsedFile restore (#3099) ... * perf(store): add best-effort V8 sidecars beside canonical JSON caches Warm ParsedFile and parse-cache loads skip JSON.parse when a sidecar is present. JSON remains authoritative: envelope validation plus v8.deserialize decide the hit, and any failure falls back without reparsing. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(store): require generation bind or sidecar drop before cache overwrite A same-length JSON rewrite could accept a leftover V8 sidecar if both generation rotation and unlink failed. Refuse the new generation unless at least one of those invalidations succeeds; skip publishing a sidecar when only the drop succeeded. detect_changes --scope all: 7 files, risk low, no affected processes. tsc --noEmit clean; 115/115 relevant unit tests; cache-related integration tests pass. parse-impl-env-reads worker-ready timeout is pre-existing (same 5 failures with this change set stashed). ESLint 0 errors; remaining warnings are pre-existing and not on changed lines. Co-authored-by: Cursor <cursoragent@cursor.com> * chore(autofix): apply prettier + eslint fixes via /autofix command * refactor(store): share V8 overwrite invalidation across persist paths The bind-or-drop gate lived in five writers. One helper keeps the protocol in a single place and lets bind/drop run together on the async path. detect_changes --scope all: 3 files, risk low, no affected processes. Co-authored-by: Cursor <cursoragent@cursor.com> * perf(store): hardlink durable ParsedFile shards into the run store Warm restore of parsedfile-cache into parsedfile-store now publishes all four shard files via fs.link, falling back to copy-into-tmp + rename so a leftover dest hardlink can never be written through. JSON remains the canonical cache; V8 sidecars ride the same path. Co-authored-by: Cursor <cursoragent@cursor.com> * perf(store): load immutable V8 shards in place, drop JSON fallback Warm analyze was still paying JSON.parse plus a restore copy. One .v8 envelope per shard and SCHEMA_BUMP 81 make a miss re-extract instead of serving a stale JSON twin. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(store): validate durable V8 warm-cache restores Reject incomplete or corrupt durable generations and snapshot valid shards before skipping parse workers, preserving ParsedFiles when persistence fails. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor(store): drop unused durable load path Load ParsedFiles only from the run-store snapshot and share one checksummed payload reader so inspect and deserialize stay consistent. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>	2026-08-30 20:50:20 +00:00
Gergő Magyar	c1103f38f2	fix: type an inference-typed class field so it can act as a call receiver (#2807) (#2810) ... * test(helpers): add the shared temp-repo lifecycle helper `createTempDirPool` gives a suite one owner for its temp fixture repos — create on demand, remove them all in one `afterAll` — instead of a hand-rolled mkdtemp/rmSync pair per file. The PDG receiver pin added in the next commit uses it. Cherry-picked verbatim from ec36c6dda on the #2802 branch, where it was extracted to collapse five hand-rolled cleanups. Identical content, so if both branches land the add resolves as a duplicate rather than a divergence. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(typescript): type a class field from its initializer so it can be a receiver A field whose type had to be inferred from its initializer produced no CALLS edge at all — not a truncated chain, nothing. `this.p.inner().compute(x)` lost `Outer.inner` too, an ordinary named-receiver call, because `typeOfMemberOnClass` found no `typeBindings` entry for `p` and `foldReceiverChain` declines at its first untypeable step rather than folding on a guessed owner. The initializer was never invisible: `new Outer()` emitted its own constructor edge exactly as the annotated twin does. What was missing was the step turning that initializer into a TYPE BINDING, i.e. capture patterns for the two shapes the query never covered: private p = new Outer(); // public_field_definition value: private p; constructor() { this.p = new … } // this.<field> = new … Both are `@type-binding.constructor`, so `annotation` still outranks them in `typeBindingStrength` and an annotated field keeps resolving through its annotation. The assignment form carries a narrow `@type-binding.this-field` marker on its `(this)` node — anchorCaptureFor takes the broadest range, so the statement stays the anchor — which `tsBindingScopeFor` reads to hoist the binding onto the Class scope, the only place `typeOfMemberOnClass` looks. The marker must stay specific to that pattern: hoisting every constructor-inferred binding would move method-local `const o = new Outer()` out of its own scope. Kotlin and Swift needed no such pattern for the initializer form because one grammar node (property_declaration) covers both a local and a stored property; TypeScript splits them, and only the local half was ever covered. Both self-diffing pins flip and gain rows: a method-assigned field, and a deliberately mistyped `private p: Mismatch = new Outer()` that asserts the source-strength tie-break executably. That row also pins a pre-existing artifact — `Inner.compute` still resolves through the hoisted module-level return-type binding — verified byte-identical on the pre-fix tree. Fixes #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(javascript): type a class field from its initializer so it can be a receiver JavaScript has no field annotations at all, so a class field's type can only ever come from its initializer — which made this the strictly worse half of #2807: `class C { p = new Outer(); }` gave `this.p` no type, and `this.p.inner()` emitted nothing. `synthesizeConstructorFieldBindings` in captures.ts already covered the sibling shape, `this.p = new Outer()`, which is why THAT row resolved — but it only walks `constructor` bodies, so a field initialized at its declaration matched no pattern anywhere. Adds the `field_definition` + `value: (new_expression)` patterns (the JS grammar names the field `property:`, not `name:`), anchored so the binding lands in the class body scope where `typeOfMemberOnClass` reads it. No hook change needed: `jsBindingScopeFor` already delegates to `tsBindingScopeFor`, so it inherits the `@type-binding.this-field` branch too. Measured: `InferredField.run` now emits `Outer.inner`, exact parity with both the local-const control and the constructor-assigned row. The second chain link (`Inner.compute`) stays absent in ALL THREE rows — that is JavaScript's separate return-type-inference gap, not this one. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(python): infer an instance field's type from the constructor it calls `self.outer = Outer()` in `__init__` bound nothing, so `self.outer.inner()` had no receiver type and the fold declined the whole chain — the Python half of #2807. An annotated field (`self.outer: Outer = ...`) or one assigned from an annotated parameter already worked. `synthesizeConstructorFieldTypeBindings` deliberately refused to infer "from arbitrary unannotated RHS expressions ... not a name-only guess". A CALL is not that: Python has no `new`, so a call to a plain (or dotted) name is the only syntactic construction form there is, and it is the same positive evidence every other language reads from `= new X()`. A bare name, subscript, await or comprehension is still refused. Adds it as a THIRD and weakest tier. The existing explicit/parameter boolean becomes a rank, so precedence is now explicit annotation > parameter annotation > construction, and a later same-tier assignment still wins (the last write in `__init__` is the live one). `interpretPythonTypeBinding` maps the new marker to `constructor-inferred` (strength 1) — checked before the parameter branch, which would otherwise have read the absent parameter marker as `annotation` and promoted a guess to the strongest tier. The Class-scope hoist needed no change: `@type-binding.instance-field` already carries it in `pythonBindingScopeFor`. Measured: `AssignedField.run` now emits `Outer.inner`, exact parity with the annotated-field and local-const rows. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(ruby): infer an instance variable's type from the constructor it calls `@service = UserService.new` in `initialize` bound nothing, so `@service.inner` had no receiver type and the fold declined the whole chain — the Ruby half of #2807. An instance variable is the ONLY way a Ruby object gets a field, and Ruby has no annotations, so this was the single shape that could have worked and did not: the existing constructor-inferred patterns bind a local (`x = Foo.new`) and a constant (`SERVICE = Foo.new`), never an ivar. Adds the plain and `Foo::Bar` qualified ivar forms. `@type-binding.name` is captured on the `instance_variable` node so the bound name keeps its `@` sigil and matches the receiver text at the call site verbatim — the resolver compares spellings, and `service` would never have matched `@service`. `rubyBindingScopeFor` gains a Class hoist gated on a narrow `@type-binding.ivar-field` marker riding the same node: an ivar declares a field of the enclosing class, so the binding must live on the Class scope or no other method can see it. Gated on the dedicated marker, never on `@type-binding.constructor` at large, which also fires for `x = Foo.new` locals that must stay in their own method. Measured: `AssignedField.run` now emits BOTH chain links, exact parity with the local-const control. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(resolvers): pin inference-typed field receivers across eight languages #2807 was filed against TypeScript, but the defect class is cross-language: "can a field whose type is inferred act as a call receiver". This measures all eight languages where the shape exists at all, in one table. The metric is parity with EACH LANGUAGE'S OWN CONTROL ROW, not "both chain links present". JavaScript, Python, Dart and PHP lose the second link (`Inner.compute`) even for a plain local, because nothing annotates `inner()`'s return type — a separate return-type-inference gap. Scoring against "both links" would have accused those four of a bug they do not have; scoring against their own control isolates the field-typing question cleanly. Recorded state: TypeScript, JavaScript, Python and Ruby now match their controls. Kotlin and PHP already did before #2807 and are pinned so the shared fold cannot regress them unnoticed — the languages that got receiver typing for free are precisely the ones nobody re-checks. Two rows stay pinned BROKEN, at their exact current value: Dart — real and narrow: the annotated control resolves, the inferred one does not. Its bindings are synthesized in dart/captures.ts rather than by a query, so the fix is its own change. Swift — blocked by a different defect found while measuring: with several classes each defining `run`, every `run`'s edges are attributed to the FIRST-declared one, which collects duplicates while its siblings — including the ANNOTATED control — collect none. Receiver typing cannot be measured there until that is fixed, and "fixing" it against this observable would be fitting to a broken measurement. Both gap rows carry a `callerExists` probe in the same assertion object, so an empty list can never read as "resolved fine, wrong node id", plus a whole-matrix guard that every language keeps a resolving control — that is what makes a gap row mean "broken" instead of "fixture never worked". Targets are deduplicated before comparison: Swift emits one edge more than once per call site, and edge multiplicity is a different question from whether the receiver typed at all. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(python): a method call on the receiver is not a construction Review finding on f4e1ead0d. `constructorCallTypeName` accepted ANY call with an identifier or attribute callee, so `self.p = self.build()` bound `p` to the non-type `"self.build"` — and because that shares the weakest tier with a real construction, a later such assignment DISPLACED an earlier `self.p = Outer()` and left the field untyped again. Measured before the fix: `self.p = Outer()` followed by `self.p = self.rebuild()` emitted no CALLS edge at all from a method chaining off `self.p`, and `self.q = self.make()` bound a type name that resolves to nothing. After: the real construction survives the reassignment, and a pure method call binds nothing rather than something wrong. Rejects a callee rooted at the receiver name. `models.Outer()` still binds — only `self`-rooted callees are refused, which is exactly the method-call shape. The matrix gains a `reassigned-from-method-call` row that fails without this rejection; that discrimination is the only reason the row exists. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(swift): resolve a method def to its own node when the labels disagree Two classes in one Swift file each declaring `func run` collapsed onto one node: every call in BOTH bodies was attributed to whichever `run` registered first, which collected duplicate edges while its twin collected none. Renaming one method fixed it; moving it to another file fixed it; so the collision was name-keyed and per-file, not positional. Root cause is a LABEL split, not a name. Swift's structure phase emits a type's methods as `Function` nodes, while the scope extractor derives `Method` from the `@declaration.method` anchor. Every key in `resolveDefGraphId` — qualified, parameter-types, arity, shape — is label-scoped, so such a pair misses all of them and lands on the bottom fallback, `simpleKey(filePath, name)`, which is deliberately label-agnostic and first-write-wins. Fixed at both ends: - Swift qualifies a method def as `<Type>.<method>`, matching the qualifier the structure phase already encoded in the node id. `class`, `struct` and `extension` all parse to `class_declaration`, so one ancestor walk covers them; a generic `class Box<T>` and an `extension Foo` wrapping a `user_type` both reduce to the bare owner name. - The bridge retries the qualified keys under the sibling callable label. Gated on the name containing a dot: `A.run` names one construct whatever the label, while a bare `run` is exactly the top-level-vs-method aliasing the label was added to prevent, so the original guarantee is untouched. This also unmasked Swift's #2807 row. `let p = Outer()` had always bound correctly — its edges were being credited to the wrong caller, so the inference-typed receiver looked broken when it was not. `InferredField.run` now emits `Outer.inner`, matching its control. Verified on the full resolver + CFG suite: 3165 passed, 0 failed, against a 3164-passing baseline. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(dart): declare inference-typed class fields so they can be receivers `var b = Outer();` produced no `@declaration.property` capture at all — no Property node, and nothing for the capture layer to hang a type binding on — so `b.inner()` could not type its receiver while the annotated twin `Outer b = Outer();` resolved fine (#2807). The gap was in the query, one layer below where the binding is emitted: both class-field patterns require a leading `(type_identifier)` or `(nullable_type)`, i.e. a WRITTEN type. Dart puts the keyword there instead for an inferred field, and spells it two ways — `inferred_type` for `var`, `final_builtin` for `final` and `late final`. Covering only `var` would have left the more idiomatic Dart style broken, so both are matched. With the field declared, the capture layer types it from the constructor its initializer calls, as `constructor-inferred` — the weakest source, and the annotated branch returns before it, so an annotated field is untouched. Only a direct construction is accepted (a bare identifier followed by a `selector` carrying an `argument_part`, the same shape `findDirectCallValue` accepts for locals); a literal, member call or await is left alone rather than guessed at. Note this is the LOCAL/field split that made the gap invisible: `emitVarTypeBinding` already handled `initialized_variable_definition`, but a class field is `declaration(<keyword>, initialized_identifier_list(initialized_identifier))`. `InferredField.run` now emits `Outer.inner`, matching its control. Dart's `var r; C() { r = Outer(); }` shape stays pinned as a known gap: Dart writes the field with no receiver prefix, so binding it means treating assignment to a bare identifier as a field write, indistinguishable from a constructor-local. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(resolvers): record Swift and Dart reaching parity in the matrix Both languages' inference-typed field rows move from KNOWN GAP to resolving, which is the self-diffing signal this file was built to produce: closing either gap failed it with the newly resolved ids in the diff. The header table and prose are corrected together with the rows, as the file's own instructions require — including WHY Swift moved. Its `let p = Outer()` binding had always been correct; a separate label-split defect attributed the second same-named method's calls to the first, which masked this row entirely. Recording that is the point: a future reader comparing the table against the code needs to know the row was never a receiver-typing failure. One row stays pinned: Dart's `var r; C() { r = Outer(); }`. Dart writes fields without a receiver prefix, so binding it means treating assignment to a bare identifier as a field write — indistinguishable from a constructor-local until the field set is known. Idiomatic Dart writes `final r = Outer();`, which the inferred-field row now covers. Every language keeps its resolving control row, so the remaining gap still means "broken" rather than "fixture never worked". Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(dart): type a field from a constructor assigned to it `var r; C() { r = Outer(); }` bound nothing, so `r.inner()` had no receiver type — the last inference-typed field shape still failing after the initializer form was fixed (#2807). Dart is the one language here that writes a field with NO receiver prefix, so `r = Outer()` inside a constructor is syntactically identical to assigning a constructor-local. That ambiguity is why this was initially left pinned — but the field set IS knowable: the class body declares `var r`, which the initializer fix already turned into a property declaration. So a bare name binds exactly when Dart itself resolves it to the field: the enclosing class declares it AND the enclosing body declares no local of that name. A `this.`-prefixed write is unambiguous and needs neither test. The shadowing case is asserted, not assumed: with a body-local `var s = Outer()` in scope, the field stays unbound while the local still resolves on its own. Binds `constructor-inferred` (weakest source, so an annotation still wins), and only for a direct construction — an identifier followed by a `selector` carrying an `argument_part`, the same shape accepted for locals. The narrow `@type-binding.dart-field` marker drives the Class-scope hoist in `dartBindingScopeFor`; gating on it rather than on `@type-binding.constructor` at large is what keeps genuine locals in their own scope. All three shapes now match their control: bare `r = Outer()`, `this.s = …`, and a non-constructor `setUp()` assignment. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(swift): type an optional field and read through its force-unwrap Swift cannot declare a stored property with neither a type nor an initializer, so its "declare now, assign in init" idiom is an OPTIONAL field read back through a force-unwrap. That shape resolved nothing, and it was broken in two independent places — each alone leaves it broken: 1. `var a: Outer?` parses as `type_annotation(optional_type(user_type(…)))`, but the property-annotation pattern required the `user_type` to be a DIRECT child, so an optional field was never typed at all. The pattern added here captures the INNER `type_identifier`, so the binding is `Outer` without relying on `stripOptional` reducing an `Outer?` spelling. 2. `self.a!` is a `postfix_expression`, which the receiver walk did not peel, so even a typed field could not be read through the unwrap. For (2), `postfix_expression` is NOT added to `TRANSPARENT_RECEIVER_WRAPPERS` outright: unlike TypeScript's `non_null_expression` — which is only ever `!` — Swift's node also carries user-defined postfix operators, which can return anything. Peeling those would type the receiver as the operand and mint a confidently WRONG owner, the failure mode compound-receiver.ts calls strictly worse than no edge. So the peel is operator-gated: transparent only when the node's text ends in `!`, which is provably type-preserving. Verified: force-unwrap `self.a!.inner()`, optional chain `self.b?.inner()`, and the plain annotated field all resolve; previously only the plain one did. The gate keeps this off every other language — `postfix_expression` is not a node type the other grammars produce here — and the full resolver + CFG suite is green at 3166 passed / 0 failed, against a 3165 baseline. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(resolvers): close the last two matrix gaps Dart's `assigned-field` and Swift's new `optional-assigned-field` rows now resolve, leaving no known-gap row in the matrix: every language reaches parity with its own control on both the initializer and the assigned shape it can express. The Swift row is new because the shape it covers did not exist in the fixture: Swift cannot declare a stored property with neither type nor initializer, so its assigned form is an optional field written in `init` and read through a force-unwrap — a shape that needed both an optional-annotation pattern and an operator-gated receiver peel, which is why the row's comment names both. The header records how the two hard cases were fixed, including the Dart shadowing rule the fix depends on: a bare `r = Outer()` binds only when the class declares that field and the body declares no local of the same name. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(bench): rebaseline the receiver-resolution and scope-capture gates Both gates are exact-match, so the improvements in this branch fail CI until the baselines move and the movement is explained. Caught by running the CI gates locally — the resolver and CFG suites are green throughout and never see these. receiver-resolution — three shapes moved to RESOLVES, no drop-count changed: ruby.fieldReceiverCall INVISIBLE-GAP -> RESOLVES (`@ivar = Foo.new`) swift.decoratedFieldType INVISIBLE-GAP -> RESOLVES (`var a: Outer?`) kotlin.nonNullAssert VISIBLE-GAP -> RESOLVES (`x!!` receiver) scope-capture — swift and typescript fingerprints, both ADD captures and remove none; the per-language `_rebaselined_inferred_field_receiver_2807` notes carry the detail and the prior digests. The other 13 languages are unchanged, which is the check that this is the intended emission and not a capture regression. CORRECTION to d5d878033's message, which claimed the operator-gated `postfix_expression` peel "keeps this off every other language — postfix_expression is not a node type the other grammars produce here". That is wrong: Kotlin's grammar produces it too, and `kotlin.nonNullAssert` moving to RESOLVES is the proof. The peel is still correct there — Kotlin `!!` is a non-null assertion with exactly the type-preserving semantics the `!` gate tests for — but it is a BEHAVIOUR CHANGE IN KOTLIN, not Swift-only as stated. The gate is what surfaced it; the claim should have been verified rather than asserted. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(cache): bump SCHEMA_BUMP for the six-language capture change, + review fixes SCHEMA_BUMP 39 -> 40. THIS IS THE MERGE-BLOCKER of the review: every language change in this PR is PARSE-TIME capture emission, and `analyze` skips tree-sitter dispatch for byte-unchanged chunks (GUARDRAILS.md:34), so a warm cache replays the pre-fix capture set verbatim and the new receiver edges never appear — silently, no error. Exactly the v27/v30 failure mode this file already documents. The PR description's claim that "no schema or version constant applies" was wrong on both counts: a bump IS required, and a plain re-analyze does NOT surface the captures without it. Re-check against origin/main before merging — main was also at 39 when 40 was allocated, and this file records eight prior collisions. Also from the review: - dart/simple-hooks.ts hand-rolled a 9-line parent walk byte-identical to the shared `walkToScope(innermost, tree, 'Class')` that TypeScript and Ruby call in one line in this same PR. Now uses the helper. - utils/call-analysis.ts: the doc framed the postfix-`!` peel as Swift-only. It is not — Kotlin `!!` parses as the same node and is peeled too, which the receiver-resolution bench proved (kotlin.nonNullAssert VISIBLE-GAP -> RESOLVES). The comment now says so, and names the `!` gate rather than the language as the bound. - test/helpers/temp-dir-pool.ts: its doc claimed four consumers; on THIS branch only `pdg-chained-receiver-callees` uses it (the other three convert on #2802). Corrected, and the byte-identical-to-#2802 intent recorded. - inferred-field-receiver-matrix: adds the Dart shadowing assertion the header comment already CLAIMED to make but never did. First attempt was vacuous — `var s = Outer()` is a declaration, so it never produced the bare `assignment_expression` the guard inspects; removing the guard did not fail the row. Fixture corrected to `var s; s = Outer();`, and mutation-verified: guard present 35 pass, guard removed the row goes red. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(cache): move the SCHEMA_BUMP pin to 40 The pin at incremental-parse-cache.test.ts asserts the exact value on purpose — it exists to catch two branches claiming one number, and it has earned that eight times. Bumping the constant to 40 without moving the pin turned it red. Found by the Codex (gpt-5.6-sol) review leg, which flagged it as a deterministic committed-test failure. The Claude lanes could not have caught it: they were dispatched before the bump landed. The comment now records the 39 -> 40 movement and its reason, matching the existing convention in that block. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(dart): treat every binder as a field shadow, not just local declarations Review P1, reproduced by two independent reviewers. `emitDartFieldAssignmentBindings` binds a bare `r = Outer()` to the FIELD when the class declares `r` and the body declares no local `r` — but the shadow set was built by walking for `initialized_variable_definition` only. That is one binder form out of many, so a formal PARAMETER named like a field slipped through: void reset(Alpha r) { r = Alpha(); } // r is the PARAMETER retyped the FIELD to `Alpha`, fabricating an edge AND destroying the correct `Beta` binding the constructor had established. The mutation test shows exactly that: the pre-fix result is not a missing edge but a WRONG one (`Other.inner#0` instead of `Outer.inner#0`) — the failure mode compound-receiver.ts:519-537 calls strictly worse than no edge. The node types were chosen from real grammar output, not assumed. Two facts drove the design: formal parameters live on the SIBLING `method_signature`, never inside `function_body`, so no walk of the body could ever have seen them; and `formal_parameter` carries a `name` field only when typed — untyped, `this.` and `super.` forms do not. `collectDartBodyShadows` therefore walks the signature AND the body, collecting formal/closure/local-function/named/optional params, `this.`/`super.` constructor params, catch bindings, for-in variables, and both local-declarator forms. A parameter shape whose name cannot be read contributes nothing — declining to bind is the safe direction. A 27-case binder sweep passes: 26 shadow shapes bind nothing, the no-binder control still binds. Four new matrix rows (param, closure param, catch, loop var) assert a surviving POSITIVE target rather than an empty list — deliberately, because the pre-fix value is a different non-empty target, so these rows cannot pass vacuously the way an empty-assert row can. Mutation-verified: reverting captures.ts turns exactly those four red and leaves every pre-existing row green. SCHEMA_BUMP is already at 40 on this branch for the six-language capture change and has not shipped, so it covers this too; re-check against origin/main before merge. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(ruby): don't bind a class-object @ivar as an instance field Review P1. The `@ivar = Foo.new` patterns added on this branch hoist to the enclosing Class scope without asking WHOSE `self` owns the ivar. In Ruby an ivar written in singleton context belongs to the class object, not to instances, so def self.build; @pool = Alpha.new; end class << self; def make; @cache = Alpha.new; end; end bound `@pool`/`@cache` as INSTANCE fields, fabricating edges from instance methods that read an ivar which is never assigned on an instance. Three corrections came out of fixing it: 1. The detection premise was wrong. `def self.build` is NOT a `method` node with a `self` receiver — it is its own node type, `singleton_method`, and `childForFieldName('receiver')` returns NONE on it. Matching on a receiver field would have detected nothing, silently. Detection is by node type: `singleton_method` / `singleton_class`. 2. A THIRD form exists that the review did not name: a class-body-level `class C; @shared = Outer.new;` is the same defect (self is the class object), and is likewise new on this branch — before it, `left: (instance_variable)` matched nothing at all. 3. Dropping only the `@type-binding.ivar-field` marker is NOT sufficient, and the class-body case is what proves it: with the marker gone the binding falls back to its innermost scope, which at class-body level ALREADY IS the Class scope, so it still lands in the wrong place. The whole match is therefore discarded. The check lives in `languages/ruby/captures.ts` because `Capture` carries only `{name, range, text}` — no AST node — so `rubyBindingScopeFor` structurally cannot ask whose `self` owns the ivar. All Ruby logic stays under `languages/ruby/`. `method` alone is not a sufficient "instance" signal, since a `def` inside `class << self` is reached through a `method` node first. Cost relative to main is zero: a class-object ivar goes back to binding nothing, exactly as before these patterns existed. The three new rows are structurally two-sided, not just mutation-checked: each empty row is paired with a non-empty `*-instance-ivar` row on the SAME fixture class, so breaking the hoist entirely turns the partner red while an unconditional hoist turns the empty row red. Mutation-verified in both directions. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(typescript,javascript): bind `this.field = new X()` only inside a class method Review P0, the most serious finding of the tri-review and reproduced by two independent reviewers. The `this.<field> = new X()` patterns added on this branch were CONTEXT-FREE: they matched anywhere in the file, and `tsBindingScopeFor` hoisted to the nearest enclosing Class without asking whose `this` that was. Since the binding lands on the same Class scope with the same `constructor-inferred` source as the field-initializer pattern, and pass4CollectTypeBindings prefers the later match on `>=`, it OVERWROTE the class's real field type. Reproduced from a non-arrow callback, an object-literal method, a static method, and module level. Fixed STRUCTURALLY, in the query: both patterns are now nested under `class_body -> method_definition -> body: (statement_block) -> (expression_statement)`, which kills the callback, object-literal and top-level triggers with no runtime code and mirrors JavaScript's `synthesizeConstructorFieldBindings` discipline. TypeScript still accepts ANY method, not just `constructor`, so the setter case this branch deliberately supports keeps working. `static` needed one emit-side guard: it is an ANONYMOUS token on `method_definition` with no field name, and tree-sitter patterns cannot negate an anonymous token (checked against node-types.json), so `isStaticMethodThis` drops it in captures.ts. `simple-hooks.ts` is comment-only — the unconditional Class hoist is now documented as safe BECAUSE the marker's producers are bounded, with a note that widening them means re-establishing that. Also fixes a `.ts`/`.js` disagreement the narrowing itself created: JavaScript's synthesis matched `method_definition` ANYWHERE, so an object literal containing a method named `constructor` still typed the enclosing class's field. Measured on identical source — JS emitted `p -> Alien`, narrowed TS emitted nothing — and closed with a `node.parent?.type !== 'class_body'` guard in javascript/captures.ts. The two languages must not disagree about the same source. Deliberately NOT matched (a missing binding, never a wrong one — JS declines these too): an assignment in a nested block, or inside an arrow where `this` genuinely IS the instance. Evidence the narrowing removed nothing legitimate: `bench/scope-capture --check` passes with the TypeScript AND JavaScript fingerprints BYTE-IDENTICAL. The five new matrix rows use an `Alien` class that also declares `inner()`, so a regression SWAPS the target rather than emptying the set — they cannot pass vacuously. Mutation test: reverting the source turns exactly those rows red (`+ "Alien.inner#0"`, `- "Outer.inner#0"`). SCHEMA_BUMP stays at 40 — this PR's existing bump covers the capture change being narrowed, and the buggy variant never shipped outside this branch. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(resolution): consult the sibling callable label in the position key too Review P1. This branch added a sibling Method<->Function retry to the qualified keys in `resolveDefGraphId`, but not to the #2699 POSITION key or its fail-closed guard, which both stayed scoped to `def.type`. Since the premise of the whole fix is that Swift defs are `Method` while nodes are `Function`, the position lookup missed and the fail-closed guard could NEVER FIRE for exactly the case the retry serves — so a function-local `func helper` inside `Host.run` was deterministically aliased onto the class method `Host.helper`, even with differing arity. Two earlier reviewers REFUTED this by arguing the guard runs before `lookupTagged`. That is true and irrelevant: the guard is scoped to `def.type`, so in the label-split case it is unreachable. Recording it because two independent lanes agreeing on a refutation is not proof. `siblingCallableLabel(label)` is now the single definition, consulted by all three key families: - position key: retried under the sibling label, gated on `posHit === undefined` so an AMBIGUOUS_POSITION tombstone still falls through to the name keys rather than being resolved by relabelling. Deliberately NOT dot-gated — a position key is not a name, so the aliasing risk the dot gate exists for does not apply. - fail-closed guard: mirrored unconditionally (it only ever returns undefined). - qualified retry: dot gate untouched. Measured before -> after on a Swift fixture: `Host.helper#1 -> sink` (the local body's call credited to the public 1-arg method) becomes `Host.run.helper@8:8#2 -> sink`, with the local's own node no longer edgeless. SCOPE CORRECTION to the P1 report: only the first consequence is a bridge defect. The second — "`run`'s call to the local resolves to the method" — is NOT reachable from ids.ts. Both defs carry qualifiedName `Host.helper` and label `Method`, and the binding hands the target side the class-member def, so the scope walk in free-call-fallback picks the member. No def->node mapping can change that; it is pinned as an explicitly labelled KNOWN GAP rather than left implied. Verification, on shared code so the full bar: resolvers+cfg 3170 passed / 1 skipped / 0 failed; `bench/receiver-resolution --check` OK; `bench/scope-capture --check` PASS (15 languages, Swift fingerprint unchanged) — i.e. the bridge change altered no capture output. The 3170 reconciles against the 3167 pre-existing at a5bf4c2da plus exactly 3 new tests; 3167 differs from the older 3166 baseline because 0418b0aac added the matrix's only known-gap row, which emits one extra `it`. Mutation test: with both arms reverted, 3 of the 5 new cases go red, each arm pinned independently — the guard case registers no position key, the position case registers no local-name key. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor: apply cleanup-review findings across the receiver-typing change Four parallel quality lanes (reuse, simplification, efficiency, altitude) over `origin/main...HEAD`. Eleven fixes; both exact-match bench gates hold with every capture fingerprint BYTE-IDENTICAL, so none of this changed what the analyser emits. Reuse — stop re-rolling helpers that already exist: - `walkToScope` moved out of the TypeScript provider into a language-neutral `utils/scope-tree-walk.ts`. Ruby and Dart had begun importing it FROM `languages/typescript/`, which made three unrelated providers depend on the TS module for a generic `Scope`/`ScopeTree` walk. Python's hand-rolled copy — the one this PR's new `self.x = Outer()` path routes through — is folded in, so all six languages now share one traversal. - Swift stops string-parsing a type name. `swiftEnclosingTypeName` split on `<` and `.`; `swiftBaseTypeIdentifier` + `swiftQualifiedBaseTail` do it structurally and correctly skip the sibling `type_arguments` node, which the string form only guessed at. `findEnclosingTypeDeclaration` replaces the inlined ancestor walk. - TypeScript uses the canonical `hasKeyword(method, 'static')`. The previous `child.type === 'static'` is the exact form `isStaticMember` documents as grammar-version-fragile: "`static` can appear as an unnamed token or as a keyword node depending on grammar version; check text." - Both new test suites use `cleanupTempDirSync`, which exists because a pipeline test's open handle surfaces as EBUSY/EPERM on Windows and `force` does not suppress it. This repo shards Windows CI. LATENT DEFECT, found by the reuse lane and fixed: `var a = X(), b = Y();` parses as ONE `declaration` with two declarators, and the query matches it once per declarator with the SAME node — so the first-descendant search handed every declarator the FIRST one's initializer. `b` resolved as `X`. Now reads `nameNode.nextNamedSibling`, which is both correct and free. Pinned by a `multi-declarator-inferred-field` row ordered so the declarator under test is the second; reverting the fix turns exactly that row red with the wrong edge. Efficiency — measured, not asserted: - Dart's shadow set was built eagerly for EVERY method body and discarded 87-100% of the time (a `this.`-prefixed write never reads it). Now lazy and memoised per body, gated on `fields.has()`. Semantics are unchanged: the set is body-wide, so deferring construction cannot change its contents. Worth recording WHY CI could never have caught this: `bench/scope-capture` gates the SCALING RATIO, and the work is linear — ratio stays 1.0 against a 1.5 budget while a constant-factor regression passes straight through. - `isTransparentReceiverWrapper` crossed the `node.type` native getter twice on the common path. One hoisted read, and — since absent and ungated are distinguishable — one `get` replaces `has`+`get`. Simplification: - One `Map<string, string | null>` replaces the parallel Set + Map that both expressed "this wrapper is transparent", with `null` meaning unconditional. - `ids.ts` computed `siblingCallableLabel` twice under two names. The three retry blocks are deliberately NOT collapsed — they use different key builders and materially different gates. - Python's `interpret.ts` nesting was only a consequence of arm ORDER; swapping the arms is unconditionally equivalent (the two differ only when both markers are present, and both orders then yield `constructor-inferred`). - One `isDirectConstruction` predicate replaces the construction-shape test that had been written four times in dart/captures.ts. Deliberately NOT done, each needing a fingerprint rebaseline or new node ids: unifying the six `@type-binding.*-field` markers into one canonical capture (it would change Python's anchor semantics, which must be verified not assumed); a Swift `labelOverride` mirroring Kotlin's four-line fix, which is the real cure for the Method/Function split the bridge currently compensates for; generalising the Swift optional-annotation pattern to `(type_annotation (_))` so the existing strippers handle every wrapper; and merging the TS query with the JS walker, which also carries a JSDoc branch no query can express. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(swift): regenerate the Swift captures golden for the optional-annotation pattern CI caught what my local runs did not: `swift-captures-golden.test.ts` pins `emitSwiftScopeCaptures` output across every `swift-*` fixture, and this branch changes that output. It is a THIRD capture gate, separate from the two exact-match benches already rebaselined here — `bench/scope-capture` hashes a different corpus, so its Swift fingerprint moving did not imply this one, and passing it was not evidence this was clean. The drift is digest-only: 37 changed lines, 37 in each direction, no capture entry added or removed. That is the expected shape for `(type_annotation (optional_type (user_type …)))` making optional properties emit an annotation binding they previously did not, plus the `@declaration.qualified_name` now carried on Swift method declarations. Regenerated with the mechanism the test itself prescribes (`UPDATE_GOLDEN=1`), not by relaxing the assertion. Verified after: all Swift unit + resolver suites green (4 files, 124 tests), and `bench/receiver-resolution --check` still exactly matches its baseline. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: three more wrong-owner defects, found by a second review round A second tri-review of this PR found THREE new P1 wrong-owner defects — every one of them in code the FIRST round had already fixed. All three are the same root shape: an incomplete ENUMERATION of binder or scope forms. That class has now bitten this branch four times (formal parameters, then these), so two of the three fixes below deliberately attack the class rather than the instance. 1. DART 3 PATTERN BINDERS (P1, reproduced by two independent lanes). `addDartBinderName` enumerated five binder node types, and every Dart 3 pattern form parses into node types in NONE of them — so a pattern-bound local did not count as a shadow and its write retyped the CLASS FIELD: class Host { var session = Cache(); void load() { final (session, count) = (Session(), 2); } void use() { session.ping(); } // resolved Session.ping, not Cache.ping } The grammar hides every rule that would carry a binder (`_pattern_field`, `_list_pattern_element`, `_guarded_pattern`, …), inlining children onto the enclosing visible pattern node, so binders land as direct `identifier` children of just two leaf types. Covers all 10 pattern types that can hold one; the eight container types are defence, since the grammar demonstrably inlines identifiers onto containers already. THE COMPOUNDING PART: a grammar-derived coverage guard reads `nodeTypeInfo` and fails if the grammar declares a `*pattern*` type the fixtures do not exercise. A grammar bump adding an 11th type now turns the suite red instead of silently reopening this bug a third time. 2. RUBY BLOCK-RECEIVER `self` REBINDING (P1 here, both Claude lanes + Codex, which rated it P2 — the engines agreed the defect is real and disagreed on severity). `isRubyInstanceIvarWrite` enumerated `singleton_method`/`singleton_class` as the ways `self` gets rebound. A `def` inside a BLOCK attaches to the block's receiver, so `Struct.new(:x) do def warm; @a = Beta.new; end end`, `Class.new do … end`, `class_eval`, and `other.instance_eval { @a = … }` all published onto the nearest LEXICAL class. Deliberately NOT fixed by listing rebinding call names: that set is OPEN — `def helper(&blk) = Foo.class_eval(&blk)` rebinds a block it merely receives, and nothing in the block's own syntax reveals it. An allow-list of "safe" iterators would be the same defect one level down. The rule is structural: crossing ANY block boundary makes ownership unprovable, so discard. Complete by construction rather than by enumeration. ACCEPTED COST, asserted not hidden: `[1].each { @shared = X.new }` in an instance method really is the instance's `self`, and this drops it — that block is syntactically identical to the `instance_eval` one. It has its own row (`plain-block-self-ivar`) so the loss is visible rather than discovered later. 3. STATIC FIELD INITIALIZERS (P1, found by Codex/gpt-5.6-sol, corroborated). A `static` field initializer was captured as an ordinary instance binding, and since both land on one Class scope at the same `constructor-inferred` strength, the later wins the `>=` tie-break — so a static field retyped the instance field of the same name (`this.p.hit()` -> `Wrong.hit`). Unguarded in BOTH `javascript/query.ts` and `typescript/query.ts`; the existing `isStaticMethodThis` only ever covered the `this.x =` assignment form. Two things surfaced while fixing it: the TS `annotation` pattern collides identically and is PRE-EXISTING, not introduced here; and JS `static constructor(){}` had no guard where TS did — the .ts/.js divergence this PR's own comment claimed could not happen. Dart has no same-name twin (the language forbids it), but a static method's receiver-less write named a library-level variable and DISPLACED the constructor's binding. Fixed narrowly, with a counterweight row (`static-field-declaration-still-types-its-receiver`) that goes red if anyone widens the guard into "drop every static binding" — reading a static by bare name from an instance method is ordinary Dart and must keep working. ACCEPTED COST: `typeBindings` has one map per Class scope with no static/ instance split, so a static field is dropped rather than recorded separately, losing typing on a TS/JS `Host.p.hit()` static receiver chain. Missed edge over wrong edge, per compound-receiver.ts:519-537. Every new row asserts a SURVIVING POSITIVE target, never an empty set: the pre-fix value in each case is a DIFFERENT non-empty target, so none can pass vacuously — the trap this branch already fell into once. Mutation-verified per fix: reverting each turns exactly its own rows red (17 Dart, 6 Ruby blocks, 5 static) with every pre-existing row green. Matrix 49 -> 80 tests. Siblings 510 passed. tsc clean. scope-capture PASS (15 languages, all ratios within gate). Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(dart): mask a shadowed field on the READ side, not just the write side The review critic refused to pass this PR while this was open, and it was right to: this is the same wrong-owner shape as the three defects fixed in the previous commit, except this one is introduced BY this PR rather than merely missed by it. THE DEFECT. Typing an unannotated field from a constructor assignment (`var conn; Host() { conn = Alpha(); }` binds `conn` on the CLASS scope) is this PR's whole point. `emitDartFieldAssignmentBindings` correctly declines to WRITE that binding when a member body rebinds the name — but the shadow set gated writes ONLY. `collectDartBodyShadows` had exactly one call site, inside the bare-name write branch. Nothing consulted it on the read side, so a bare-name READ of a shadowing binder the resolver cannot type walked straight past the local and hit the field binding this feature mints: class Host { var conn; Host() { conn = Alpha(); } void probe(List<Beta> xs) { for (final conn in xs) { conn.inner(); } // conn is a Beta element } } // resolved Alpha.inner, not Beta.inner Reproduced in SEVEN binder shapes, not the one the review reported: for-in (`final` and `var`), untyped formal parameter, plain local `var`, catch binding, closure parameter, and record pattern. Delete the constructor and the same read emits NOTHING — which is what proves this PR introduced it. "No edge" became "wrong edge", the one failure mode compound-receiver.ts:519-537 exists to prevent. THE FIX uses `Scope.ownsReceivers` (#2701), the primitive that already exists for exactly this, rather than inventing a mechanism. `scope/walkers.ts` consults `typeBindings` FIRST at every scope and only then honours the mask, so a shadow the resolver CAN type still wins — an annotated `void probe(Beta conn)` keeps `Beta`, because `synthesizeDartSignatureBindings` anchors parameter bindings on the same body node and they land on the same Function scope. The mask fires only where the alternative was a fabricated type. Plumbing follows TypeScript's `@receiver-owner.this` precedent: the marker rides the same synthesized match as `@scope.function` and sits outside the `@scope.` namespace so `anchorCaptureFor` cannot mistake it for the anchor. Dart differs only in that its function scopes are synthesized in captures.ts rather than declared in the .scm, so the names travel as capture TEXT — a `CaptureMatch` carries no AST node, so the reader cannot re-derive them. SCOPE, and the costs taken knowingly rather than hidden. The mask is `shadows ∩ fields` and nothing wider. Masking every locally bound name would also fix a library-level `var logger = Logger();` shadowed by a loop variable, but it changes resolution for code this PR never touched. Three consequences are documented on `dartShadowedFieldsCapture`, not buried: the wider case is left open; an ANNOTATED field shadowed by a binder is masked too (correct Dart, but it touches resolution predating #2807); and `mixin` bodies are reached, since the grammar gives them a `class_body`. PERFORMANCE, measured rather than asserted. The mask is emitted eagerly in Pass A, where `collectDartBodyShadows` used to be lazy — the replaced comment recorded 87-100% of eagerly built sets being discarded, ~15% of Dart emission. Actual cost on the scope-capture large corpus, median of 3: 405.6ms with the mask vs 390.0ms without, ≈ +4%. Fingerprint and capture_groups are byte-identical across both arms, so no corpus fixture emits a mask at all — that 4% is the cost of the CHECK alone. Not visible to `bench/scope-capture`, which gates the scaling RATIO and is blind to a linear constant factor; stated here because the gate cannot state it. (3 samples per arm, blocked not interleaved — an estimate, not a rigorous number.) A per-file memo keyed by node span makes both passes share one walk per body, so the write side no longer pays a second one. SCHEMA_BUMP 40 -> 41 with its exact-value pin, since capture emission changed. Re-check against origin/main immediately before merge — main was 39 at commit time. Mutation-verified both directions, which is the part that matters: - unwire `scopeOwnsReceivers`, rebuild -> exactly 2 rows red (`loop-var-read-does-not-see-the-field`, `pattern-read-does-not-see-the-field`), 83/85 green. - over-widen the mask (drop the `shadows.has` test) -> 28 Dart rows red, including `unshadowed-read-in-a-shadowing-class-still-resolves`. The three control rows stay green under the first mutation BY DESIGN — they guard overreach, not the defect; the second mutation is what proves they are live. Pre/post on the trigger row: `{Class:Alien, Alien.inner#0, Outer.inner#0}` -> `{Class:Alien, Alien.inner#0}`, so no row can pass vacuously. Matrix 80 -> 85 tests. Sweep 3220 passed (was 3215; exactly +5). tsc clean. All four capture gates green: receiver-resolution OK, scope-capture PASS (15 languages, no fingerprint moved, nothing rebaselined), callable-value-flow PASS, swift golden 9. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(ts,js): let each language name its own class-field node type CI caught a defect the whole review round missed. `grammar-literal-validation`: 1 dead grammar literal(s) found: - node-type "field_definition" — languages/typescript/captures.ts:0 — not valid in [typescript] `isStaticClassFieldBinding` held BOTH spellings in one set — `public_field_definition` (TypeScript) and `field_definition` (JavaScript) — so that one predicate could serve both languages. But the predicate lives in `typescript/captures.ts`, and the gate checks every literal against the grammar of the FILE it appears in. `field_definition` is not a TypeScript node type. The literal was NOT dead code: `javascript/captures.ts:42` imports the predicate and calls it against real JS nodes, so the guard worked. The gate is still right to fail it, and for exactly the reason this predicate's own docblock gives for preferring `hasKeyword` over a node-type test — "a node-type test silently stops firing on a grammar bump and every static field starts retyping its instance twin again". A literal already dead in its own file is that failure shipped pre-broken: nothing in the TypeScript file would ever have told us. Each language now names its own node type and passes it in (`TS_CLASS_FIELD_DEFINITION_TYPES` / `JS_CLASS_FIELD_DEFINITION_TYPES`), so every literal is checked against the grammar it belongs to. The `hasKeyword` logic and the static/instance reasoning stay shared and unchanged — only the node-type set moves to the caller. WHY THE LOCAL SWEEP DID NOT CATCH IT: I ran `test/integration/resolvers` and `test/integration/cfg`. The gate is `test/integration/grammar-literal-validation. test.ts`, in the parent directory. Scoping a sweep to the subdirectories a change touches is precisely how a cross-cutting gate gets skipped. grammar-literal-validation 4 passed. tsc clean. Full `test/integration` + `test/unit/scope-resolution`: 6305 passed, 14 failed — all 14 in e2e/environment suites (fts-extension-e2e 9, analyze-heap-oom-e2e, cli-e2e, analyze-wal-checkpoint-failure, plus interproc-taint and parse-impl-env-reads, which BOTH pass in isolation and fail only under 28-worker load). CI runs the same files green. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(dart,ts): pin all seven read-side binder shapes; correct a wrong accepted-cost claim Two review findings, one of which turned out to be a documentation defect rather than the design defect it was filed as. S8 — THE READ-SIDE FIX PINNED 2 OF THE 7 SHAPES IT REPORTED REPRODUCING. `ab2f48c17` reported the wrong-edge defect reproducing in seven binder shapes and landed rows for two. The stated mitigation was that all seven route through one `collectDartBodyShadows` enumeration whose completeness the grammar-derived coverage guard protects. That mitigation is NARROWER THAN CLAIMED: the guard filters `nodeTypeInfo` on `type.includes('pattern')`, so it covers the pattern family and NOT catch bindings, closure parameters, plain locals, or formal parameters. Narrowing `addDartBinderName`'s catch arm would have turned no row red. All seven were re-measured by unwiring `dartScopeOwnsReceivers` and rebuilding. Every one gained the wrong edge `Outer.inner#0` — none had to be dropped as non-reproducing. Five new rows: formal parameter, plain local `var`, catch binding, closure parameter, for-in `var`. Non-vacuity established structurally, not by assertion: the Dart AST was dumped first to confirm each fixture produces the node `addDartBinderName` actually inspects (`formal_parameter`, `initialized_variable_definition`, `catch_parameters`, `for_loop_parts`). The catch row uses a bare `catch (zf)` rather than `on Err catch` deliberately — an `on` clause names a type, which would make the row measure type resolution instead of the mask. S7 — THE ACCEPTED-COST COMMENT WAS WRONG, AND THAT IS THE FINDING. It claimed dropping a static field's binding trades a wrong edge for a missed one. Measured on a same-name twin, that is false: read with the drop without it this.p (instance twin) Outer correct Alien wrong Host.p (static twin) Outer WRONG Alien correct Host.q (static, no twin) none — missed Alien correct The wrong edge did not disappear. It MOVED to the static read, which now picks up the instance twin's type. Only the no-twin case is a genuine missed edge. The trade is still right — `this.p` is far more common than `Host.p` — but it was documented as safer than it is, and a reader deciding whether to revisit it was being given the wrong picture. NAMESPACING WAS EVALUATED AND DELIBERATELY NOT DONE. `Host.p.hit()` resolves through `foldReceiverChain` in shared `compound-receiver.ts`, which explicitly discards whether a chain's base was a class reference or a value (:519-527). The class-constant bit exists only on the text-cascade path (`currentIsClassConstant`) and is consumed solely by `isConstructionSelectorHop`; TS/JS take the fold, not the cascade. `Scope.typeBindings` is `ReadonlyMap<string, TypeRef>` with no static field. `ownsReceivers` cannot help — it is a suppressor that can only REMOVE a binding, never route to a second one. A real fix needs `FoldState` to carry the bit plus a key convention in shared code (an AGENTS.md:42 hook if not language-neutral), it crosses the worker boundary so it needs a SCHEMA_BUMP, and `compound-receiver.ts:826` iterates every binding for `fieldFallback` so a namespaced key would leak straight back in as an ordinary field. Not a cheap or safe change — and it would have been made with ZERO existing tests pinning static-read behaviour. So: smallest safe step instead. Two rows pin the measured behaviour (`static-read-of-a-same-name-twin-picks-up-the-instance-type` asserts the positive wrong target, not an empty set; `static-read-without-a-twin-loses-its-type` is a known-gap), and the comment now says what actually happens. Anyone who revisits this starts from measurements rather than from a claim. No SCHEMA_BUMP: the `captures.ts` change is comment-only — verified, the diff has no non-comment added lines. Mutation red-rows 2/85 -> 7/90; each new row fails with a strictly larger set (`+Outer.inner#0`), so none can pass vacuously. Overreach control still live: dropping `shadows.has` turns 28 rows red including `unshadowed-read-in-a-shadowing-class-still-resolves`. Matrix 85 -> 92 tests. Sweep 3231 passed, 0 failed. tsc clean. All four gates green, no fingerprint moved, nothing rebaselined. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(python): stop a dotted callee from fabricating a constructor type S4 and S5 from the review round. They are ONE defect, not two, and the real one is wider than the review described. Both live in a 32-line block THIS PR adds (`@@ -116,0 +117,32 @@` — a pure addition), so neither is pre-existing. THE DEFECT. `constructorCallTypeName` rejected only a callee rooted at the receiver and returned every other dotted callee whole to `resolveTypeRef`, which resolves dotted names through `QualifiedNameIndex` — and that index matches the TRAILING SEGMENT against a class of that name even when the callee is a method on an unrelated object: class Alpha: def ping(self): return 1 class Factory: def Alpha(self): return "not an Alpha" # a METHOD class Host: def __init__(self, f): self.svc = f.Alpha() # svc is a str def run(self): return self.svc.ping() # measured: Host.run -> Alpha.ping, fabricated WIDER THAN FILED: the review framed the trigger as a callee rooted at an `__init__` PARAMETER. Measured, the root's binding form is irrelevant — a module-level variable (`shared_factory.Alpha()`) fabricates identically. Any rule written against what the root binds to would have fixed half the defect and left the other half looking fixed. Both fabrications now have rows. S5 IS A SYMPTOM, NOT A SECOND DEFECT. `self.conn = Outer()` then `self.conn = Registry.get()` typed the field as `"Registry.get"` (resolving to nothing, so the edge vanished) only because the dotted arm accepted `Registry.get` as a constructor in the first place. Once dotted callees yield no candidate, there is nothing weak left to displace with and `Outer` survives. So `>=` is untouched and NO second mechanism was added: between two REAL constructions last-write-wins is correct, and the existing `ReassignedField` matrix row depends on it. Tightening the tie-break would have been the wrong fix to a symptom. THE FIX: accept a bare `identifier` callee only. Refusing ambiguous evidence at CAPTURE time rather than resolving-then-rejecting is deliberate — the target-kind route is not reachable from this file (`resolveTypeRef` already filters `TYPE_KINDS`; the fabrication comes from a trailing-segment match in `scope/walkers.ts`), and the root-alias route would collide with PR #2828, which is rewriting exactly how an unaliased dotted namespace import resolves. This change is orthogonal to #2828 by construction: it changes what is CAPTURED, never how a name is looked up, and touches none of its files. WHAT THE DOTTED ARM WAS ACTUALLY BUYING: nothing. The review (and this PR's own docblock) justified it with `self.u = models.User()`. Measured, that shape emits NO edge before or after this change — an instance field's binding lands in CLASS scope, which never reaches the namespace split. The shape that really resolves is the module-level local `u = models.User()`, which comes from `query.ts` and is untouched here. The arm's entire measured contribution was fabrications, which is what made the fix cheap. #2828 COMPATIBILITY, checked not assumed: `import pkg.user` -> `self.u = pkg.user.User()` resolves to nothing both before and after, so this cannot stop it resolving. No test row pins that shape ON PURPOSE — asserting its current empty state would plant a tripwire that goes red the moment #2828 lands. If #2828 also teaches the FIELD path the namespace split, re-enabling dotted field callees becomes a live option; the docblock says so, and says why redoing it capture-side would re-open the fabrication. SCHEMA_BUMP 41 -> 42 with its pin. This is parse-time capture emission: after the fix `self.svc = f.Alpha()` emits no `@type-binding.constructor` capture at all, so a v41 warm cache replays the pre-fix capture set for byte-unchanged files and keeps serving the fabricated edge (GUARDRAILS.md:34). A within-PR re-bump, not a collision fix — 40/41/42 are all this unmerged branch's, and `origin/main` is at 39. Re-check against origin/main immediately before merging. Mutation-verified in BOTH directions, which is what shows the fix is placed at the right width rather than merely working: - revert the fix -> exactly 3 red: both S4 fabrication rows + the S5 displacement row (8 green) - reject EVERY callee -> exactly 3 red: the three positive-typing rows (8 green); the S4 rows correctly stay green The two mutations hit DISJOINT row sets — too loose and too tight each break a different half. No row asserts an empty set: the three "must not type" rows call `Alien.ping()` as a witness so a regression SWAPS a target in rather than emptying. Non-vacuity is asserted in the test itself — one guard checks every caller node is live, another asserts the `Alpha` class / `Factory.Alpha` method name collision the fabrication NEEDS is actually present, so the rows cannot rot into passing for the wrong reason. Sweep 3268 passed, 0 failed. Python unit + python.test.ts 342 passed. tsc clean. All four gates green — no bench cell moved, nothing rebaselined. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-04 19:31:25 +01:00
Gergő Magyar	9eaf2e6c4e	perf(mcp): cut the analyze-only language-provider closure out of MCP server startup (#2802) (#2806) ... * fix(mcp): key the empty-ascent note on CALL_SUMMARY data, not language (#2802) `pdg-impact.ts` decided whether to append a "return-value ascent is TypeScript/JavaScript-only" caveat to the `impact(mode:'pdg')` note by looking up the criterion file's language. That put language-specific logic in a layer that must be language-agnostic, and it was a lossy proxy for a fact the graph already holds. Whether the ascent can fire is a property of the persisted CALL_SUMMARY edges. The descent already computes it, so thread the resolved-callee and return-flowing counts out of `interproceduralDescent` and key the note on those instead. Three defects the language proxy carried, all gone: - Wrong for `.mjs`/`.cjs`/`.mts`/`.cts`: the provider registry's extension arrays omit them while the ingestion pipeline parses them as TS/JS, so those files were harvested but the note claimed their ascent was empty. - Silently stale: any language whose harvester started recording formal indices would keep getting the caveat until someone edited the list. - Wrong in reverse: a TS/JS callee with no return-flow got no caveat, so an ascent that found nothing read like one that covered the slice. `pdg-impact.ts` now names no language and imports nothing from the language layer, which also drops the analyze-only provider closure from MCP server startup. Measured on overlayfs against a full build: import mcp/local/local-backend.js before 565-648 ms / 548 modules import mcp/local/local-backend.js after 458-463 ms / 170 modules Tests hold CALL_SUMMARY content fixed while varying the file extension across nine languages and assert the note text is identical, then hold the extension fixed and vary the summary to show the note tracks the data. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(mcp): guard MCP startup against the language-provider closure returning The eager `pdg-impact.ts -> core/ingestion/languages` edge was found and lost once already during #2793 before #2802 re-derived it, so it gets a test rather than a comment. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(lbug): record why csv-generator is not lazy-imported #2802 proposed cutting `csv-generator.js` out of the adapter chain to shorten MCP server startup. Measured on a native filesystem, the marginal cost is small relative to the siblings this module already imports, and `core/search/bm25-index.ts` statically imports `normalizeFtsText` from the same module on a path `local-backend.ts` reaches dynamically for FTS — so deferring would relocate the cost to first query, not remove it. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(pdg): pin chained receiver calls reaching BasicBlock.calleeIds The PDG inter-procedural descent hops through `BasicBlock.calleeIds`, so it can only cross a call boundary the resolver resolved. Chained receiver calls reach `calleeIds` through the receiver-typing pass's own `calleeIdSink` — a separate path from plain calls. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(analyze): drop the stale per-language cross-reference (#2802 review P3-4) `pdgModeMismatch`'s comment told readers to keep "the diagnostic per-language refinement in the impact CONSUMER (see pdg-impact.ts assemblePdgImpactResult)". That refinement is no longer per-language — removing it is the point of #2802, which now keys the empty-ascent note on the persisted CALL_SUMMARY data instead. The comment's real invariant is untouched and still correct: the values in `resolvePdgConfig` must stay scalar, because the comparison below is a shallow `!==` and an object would compare by reference. Only the cross-reference was stale. Comment-only; no executable line changes. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(mcp): probe the real module loader for the startup language closure (#2802 review P1-2) The previous guard hand-rolled a regex walk over TypeScript source to assert `core/ingestion/languages` was not statically reachable from MCP startup. Four bypasses were reproduced against it, any one of which let the exact 226-module regression return while the test stayed green: a. Wrong entry root. It walked from `mcp/local/local-backend.ts`, but the server module is `mcp/server.ts` — which imports LocalBackend as `import type`, so the guard's anchor was not even on server.ts's runtime closure. Ten real startup modules sat outside it. b. A top-level `await import(...)` executes during module evaluation, so it is eager at startup — but the walker skipped every `import(...)` by construction. c. The `import type` strip deleted a 16,445-character window of `pdg-impact.ts`: an `export type X =` matched lazily to the next `from "…"`, which lives inside a string literal. Any import in that window was invisible. d. The comment strip treated a `/*` inside a string literal as a comment opener. Replace the approximation with a real module-load probe: spawn a child node process per entry, import the built `dist/` entry, and report what the loader actually pulled in. Rooted at `dist/mcp/server.js` and `dist/cli/mcp.js` (the real startup entries) plus `dist/mcp/local/local-backend.js`. Syntax cannot fool it. One deviation from the two existing sibling probes is load-bearing: `dist/` is ESM, so a `require.cache` diff alone cannot see the first-party `dist/**` graph — it only catches CJS and native modules, which is why `import-closure.test.ts` gets away with it (it asserts on `@ladybugdb/core`). A pure cache diff here would have reported zero language modules unconditionally, i.e. a new vacuous guard. This probe unions `module.registerHooks({ load })` with the cache diff, and each entry carries a non-vacuity anchor and a module floor so an empty result fails loudly. Verified load-bearing: adding a top-level `await import('../core/ingestion/languages/index.js')` to `src/mcp/resources.ts` and rebuilding turns `dist/mcp/server.js` red with 70+ named offenders, while the `local-backend` and `cli/mcp` cases stay green — which is bypass (a) demonstrated directly. The old guard passed that poisoned tree entirely. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(lbug): drop the unreproducible 9p multiplier from the csv-generator note (#2802 review P3-2) The comment justifying why `csv-generator.js` is NOT lazy-imported carried a hard "~40x" figure for how much a 9p mount inflates per-file ESM resolve. Three independent measurements during review produced ~40x, ~7.3x and ~30x, so the multiplier is not a reproducible quantity and had no business being stated as one in a durable comment. Reworked so the STRUCTURAL argument leads and the numbers only support it. That argument is what actually settles the question and it does not rot: `core/search/bm25-index.ts` statically imports `normalizeFtsText` from `csv-generator.js`, and `local-backend.ts` reaches bm25-index through a dynamic import on the FTS query path — so deferring here relocates the cost to first query rather than removing it. Both verified again at `bm25-index.ts:15` and `local-backend.ts:2756`. Remaining figures are re-measured, attributed to a date and issue, and labelled by filesystem: ~1.6 ms marginal (median of 45 cold imports on local disk) versus ~50 ms for the same import on a network mount, stated as environment-bound rather than as a property of the module. The provider-registry cost is given as "several hundred modules" — the static walk, the runtime hook, and the reviewer's probe each counted it differently (375 / 439 / 407), so no single number was picked to go stale. The old "226 modules" was real but counted only the `languages/` subtree and undercounted the win. Also repoints the trailing reference to the guard's new home at `test/integration/mcp/startup-language-closure.test.ts` (same comment block, inseparable from this rewrite). Comment-only; no executable line changes. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(mcp): stop the empty-ascent note asserting a fact an undecodable summary contradicts (#2802 review P2-2) The note claimed "this is a property of the persisted summaries" whenever the descent resolved callees and none carried a return-flow. But `decodeCallSummary` never throws by design: a version-skewed (`2|r:1`), corrupt (`1|r:zz`), or NULL `reason` yields no entry, which was indistinguishable from a cleanly-decoded empty summary. So the note could assert "no formal parameter is recorded as flowing to its return value" about a callee whose CALL_SUMMARY actually records `p0 -> return`. `meta.pdg.hasCallSummary` is a plain boolean and stores no codec version, so nothing else caught it. `calleesWithReturnFlow` now reports three outcomes instead of two — flowing, decoded-empty, and undecodable — and the undecodable count is threaded through the descent to the note. When it is non-zero the note says so and points at a re-index; when every summary decoded, the persisted-summaries claim is kept and now explicitly conditioned on that. Soundness is unchanged: an undecodable summary still licenses no ascent and never enters the return-flowing set, so the ascent path is byte-identical. Only the note's wording moves. Tests drive all three undecodable forms through the mock and assert the false claim is gone, the remedy is reported, and the ascent is still withheld. A companion assertion pins that the all-decoded case KEEPS the persisted-summaries claim, so the fix cannot degenerate into deleting the sentence. Verified load-bearing: reverting the source alone fails 6 of 34. Impact analysis: `calleesWithReturnFlow` upstream LOW (2 callers, both in this file); `assemblePdgImpactResult` upstream LOW (1 caller). Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(pdg): cover every chained-receiver shape and pin the inference gap (#2802 review P2-1, P3-1) The fixture proved chained receiver calls reach `BasicBlock.calleeIds` using exactly one receiver form — a local `const`. That is the shape that works, so a single-shape fixture implied general support the resolver does not have. This repo has been burned by that before: a drop-count gate blind to fixed shapes. Measuring nine forms against the real pipeline also corrects how the gap was originally characterised. It is NOT local-versus-field. An annotated field resolves fine, including the constructor-assigned variant: private p: Outer = new Outer(); -> both links private p: Outer; this.p = new Outer(); -> both links private p = new Outer(); -> EMPTY CELL private p; this.p = new Outer(); -> EMPTY CELL The discriminator is the type ANNOTATION. When a field's type must be inferred from its initializer the whole `calleeIds` cell empties — so even `Outer.inner`, an ordinary named-receiver call, is lost, and the inter-procedural descent cannot cross the boundary at all. Pre-existing; independent of #2802, which does not touch receiver resolution. The fixture is now table-driven over seven working forms (local const, local in a method, annotated field, ctor-assigned annotated, ctor-param assigned, call-result receiver, three-link chain) plus the two inference-typed forms, each row carrying its expected chain-link ids. Assertions moved from substring to exact id membership, split with the production `splitCalleeIds` reader — so `Inner.compute` can no longer be satisfied by `Inner.computeExtra` or `OtherInner.compute`, which matters because the descent keys on exact ids for span and CALL_SUMMARY lookup. The two known-gap rows are pinned with `it.fails` plus a hard assertion on the exact gap-row set, so a resolver fix turns them red instead of passing silently, and an anti-vacuity guard requires every shape to match exactly one block — without it a drifted fixture matching zero blocks would let `it.fails` pass for the wrong reason. Proven by mutation: relabelling a working row as a known gap fails both pins. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(mcp): qualify the empty-ascent note when the examined callee set is incomplete (#2802 review P2-4) The note asserted "none of the N resolved callees carry a CALL_SUMMARY return-flow", and on the all-decoded path that this is "a property of the persisted summaries". Both are universal claims over the callees the descent actually examined, and two mechanisms can leave that set incomplete without the note saying so: 1. Budget truncation. The descent stops on depth/limit/node-cap, so a callee that DOES carry a return-flow can sit in a hop never reached. A 4-deep chain reported "none of the 3 resolved callees" while link 4 held the only summary. 2. Emit-time capping. When a block's `calleeIds` cell was capped, `splitCalleeIds` strips CALLEES_TRUNCATED_SENTINEL, so the dropped callees are invisible to both the scan and the counters — even though the callgraph bridge in this same file already treats such a block as callee-incomplete. Add `calleeIdsWereTruncated`, the counterpart to the sentinel strip, read from the raw cell before splitting so a block whose entire list was capped away still raises the flag. Thread it through the descent to the note. Case 1 needs no new plumbing — the aggregate `truncated` is already on the input object. Using the aggregate rather than a descent-only flag is deliberate: seed truncation and intra-BFS depth truncation also shrink the initial slice, so their callees are never gathered either. It is a sound superset that never under-hedges. When either mechanism fired, one clause naming the reasons is appended and the whole-slice assertion softens to "every summary examined decoded … a property of those summaries". When the set is complete both branches stay byte-identical to before, so this does not become a blanket hedge. Tests pin truncated, untruncated, emit-capped-alone, both-mechanisms, and undecodable+truncated, asserting the truncation premise rather than assuming it. Verified load-bearing: reverting the source alone fails 6 of 42, and the HEAD note printed in those failures is the bug verbatim. Impact analysis: `assemblePdgImpactResult`, `calleeIdsByBlock`, `interproceduralDescent` all upstream LOW; every caller is in this file and `runImpactPDG`'s exported signature is unchanged. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(mcp): stop the empty-ascent note calling call-site references "resolved callees" (#2802 review P3-7) The note printed "none of the N resolved callees carry a CALL_SUMMARY return-flow (no formal parameter is recorded as flowing to its return value)". N counted the raw `BasicBlock.calleeIds` cell, which carries ids `resolveCalleeSpans` never enters — out-of-repo targets, interface methods, and the `Class:` id a `new X()` emits. On the chained-receiver fixture that inflated N from 1 to 3. Two defects, both in the wording rather than the arithmetic: "resolved" implies a symbol-table lookup that did not happen for those ids, and the parenthetical asserted a FORMALS-level property about symbols never resolved to a body. Reworded rather than re-seeded, deliberately. `calleesWithReturnFlow` scans the RAW id set, so the claim "none of these carries a return-flow" is exactly established for all N — the scan really did check the `Class:` id. Re-seeding N from the resolved spans would make the sentence quantify over a strict SUBSET of what was checked, silently dropping the un-enterable references from a claim that genuinely covers them, and would desync N from `calleesUndecodable`, which is derived from the same scan population. none of the N resolved callees carry ... none of the N call-site callee references carry ... and the formals parenthetical is dropped. The note gets shorter, not longer. `calleesResolved` is renamed `calleeReferences` end-to-end (file-local; nothing outside referenced it), and the descent's return-type doc — which called them "callee symbols the descent resolved" and reinforced the wrong reading — now states that un-enterable ids ride the same cell, are scanned, and are never entered. The `> 0` gate is unchanged, so no slice that previously produced the note stops producing one. A test pins that explicitly: an all-un-enterable cell resolves no span, takes no hop, and emits no ascent sentence despite a non-zero count — so a future re-seeding cannot silently move when the note fires. Tests also pin the quoted number and singular/plural against a mixed cell, with a discriminator asserting `reachableBlocks` is byte-identical while the count moves 1 -> 3. Verified load-bearing: reverting the source alone fails 6 of 7 new tests, printing the finding verbatim. Impact analysis: `assemblePdgImpactResult` and `interproceduralDescent` upstream LOW, sole caller `runImpactPDG` in the same file; exported signature unchanged. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(mcp): pin cross-hop callee accumulation and the mixed return-flow contract (#2802 review P2-5) Every case in this file drove a single hop, so the Set union the descent performs across hops (`calleeReferencesSeen` / `calleesReturnFlowingSeen`) was never proven to accumulate rather than overwrite — a one-hop descent cannot tell the two apart. And although a sibling commit added a three-id cell, none of those ids return-flowed, so the "some callees flow, some do not" boundary was entirely unpinned. Extends the mock with a `secondSummary` knob that drives a genuine second hop: `helper2` is named only in `helper`'s own body block, so the descent must cross a second boundary to reach it. Three mock handlers are made faithful to the parameters they already bind — `calleeIdsByBlock` now routes on the asked `$ids`, and the CALL_SUMMARY scan and span resolve answer per asked id — which is what makes a second callee answerable at all. Existing cases are behavior-identical. Five tests: the union count across two hops; a return-flow on hop 0 surviving a later empty hop; a return-flow found only on hop 1; mixed callees in one examined set going silent rather than partial; and a flowing callee alongside an undecodable sibling staying silent including the decode remedy. The mixed case pins a deliberate contract rather than proposing one. The production condition is `calleesReturnFlowing === 0`, so partial coverage is reported as silence. A reviewer considered and dropped "report partial coverage" as a product change; this makes flipping it a conscious edit instead of an accident. Verified load-bearing against three separate source mutations: accumulating only on hop 0 (2 fail), each hop overwriting instead of unioning (3 fail), and flipping the gate to partial-coverage reporting (4 fail). In all three every PRE-EXISTING test still passed — which is the finding restated as evidence. Test-only; `pdg-impact.ts` is byte-identical to HEAD. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(mcp): consolidate the empty-ascent rationale to one canonical site (#2802 review P3-6) The "keyed on observed CALL_SUMMARY data, never on the criterion's language" rationale was restated in full at four comment sites. It exists because a reviewer asked "why not just look up the language?", so it has to stay findable — but not four times. The canonical explanation now lives in `interproceduralDescent`'s return-type doc, where the counters are actually computed, organised as POPULATION (why the raw `calleeIds` tally is the right set to quantify over) and OBSERVED DATA, NEVER THE CRITERION'S LANGUAGE (the full answer, including the producer-change argument and the no-language-naming rule). The other three sites keep only what is locally load-bearing and point here. Deliberately preserved, because each carries a non-obvious fact: why an undecodable summary licenses no ascent, why the aggregate `truncated` is used rather than a descent-only flag, and the raw-id-tally population argument. Net comment delta -11 lines. The reviewer also flagged the local/field naming asymmetry (`calleeReferencesSeen` vs `calleeReferences`). Keeping the suffix, with a comment recording why so it is not re-raised: the premise that every other local matches its field is true, but those locals are identity-returned, whereas these are `Set<string>` accumulators returned as `.size`. Dropping the suffix would give one identifier two types in one file — a `Set` at the accumulation site and a `number` where the note does arithmetic and pluralisation on it ~900 lines away. The Set-ness is also load-bearing: the dedup is why a callee invoked from two hops is not double-counted, which is what makes the note's count correct. Comment-only. Verified mechanically: every added and removed line in `git diff -U0` matches a comment pattern, so the note's template literals are untouched and its rendered text is byte-identical. 89 tests unchanged. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(mcp): collapse the ascent plumbing accreted across 13 fix commits Quality cleanup, no behavior change. Four independent review passes converged on the same root cause: thirteen commits each fixed one review finding in isolation, and the ascent facts grew one loose field at a time until 62% of the changed region was comments explaining plumbing. Five changes: - `calleeIdsFromBlocks` deleted. Zero call sites anywhere in src/ or test/ — already dead on main, and this branch had edited it to keep it compiling. Its only reference was a stale `{@link}` in a neighbour's doc, now rewritten to stand alone. - `parseCalleeIdsCell` replaces the two-pass read. `calleeIdsWereTruncated` and `splitCalleeIds` were splitting the same cell on adjacent lines, which measured ~2x the parse cost (0.82 -> 1.59 ms at a realistic hop, 57.7 -> 92.7 ms at the per-statement site cap) and was a second independent encoding of the sentinel format — exactly what `splitCalleeIds` was extracted to prevent. One pass classifies as it walks; `splitCalleeIds` stays as a wrapper so its two external callers are untouched. The single-use `export` is gone. - `AscentCoverage` replaces four fields threaded through three signatures. ~12 declaration sites become 3, and the canonical rationale now lives on the type by construction — which is why the earlier doc-consolidation commit was needed at all. - `calleesReturnFlowing` becomes a boolean. Its only reads were `=== 0`, twice; it cost a Set sized to every callee in the slice plus a per-hop union loop. The flag is set inside the existing `returnFlowing.size > 0` branch — equivalent, since the cross-hop union is non-empty iff some hop's was. - The duplicated empty-ascent note head is collapsed to one gate and one head with per-arm tails. Both arms had been edited in lockstep twice in this branch's own history. The rendered note text is byte-identical. Verified structurally and then empirically: both expressions reconstructed standalone and diffed across the full cross product of references x returnFlowing x undecodable x truncated x listTruncated — 288 combinations, 0 mismatches. Net -53 lines. 102 tests pass unedited; the unused-symbol lint warning is gone. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(mcp): parallelise the startup probes, drop a redundant pin, name the mock knobs Quality cleanup from the same review passes. The set of verified behaviors is unchanged except where noted. **Startup probes run concurrently.** `spawnSync` blocks the event loop and vitest runs a file's tests in order, so the three probes strictly serialised. Launching all three with async `spawn` in `beforeAll` and asserting over the collected outcomes cuts the file from ~12.7 s to ~3.9 s wall (-69%). Every promise is caught before `Promise.all`, so all three children are reaped and failures report per entry rather than surfacing only the first rejection. Preserved and each proven by mutation: the missing-dist error names its entry, a raised module floor fails only its own row, and a bogus anchor still reports the loaded-module count. **The two `it.fails` rows are removed.** They pinned the inference-typed receiver gap that the strict `toEqual` pin beside them already covers — and they were the weaker of the two, because `it.fails` passes when the body throws for ANY reason, including `idsFor`'s own non-vacuity guard. A renamed fixture marker would have kept them green on a rotted premise. The strict pin is self-diffing and was verified load-bearing on its own: pointing a known-gap marker at a resolving shape fails it with the two newly-present ids listed. The file header now carries the gap's durable description. **The ascent-note mock takes options objects.** `descentExec` and `run` had grown to five and seven positional parameters in the order five agents added them, so call sites read `run(FILE, true, null, 3, false, undefined, null)` — several carrying `undefined` purely to reach a later argument. All 34 call sites are converted; nine that used only defaults are now bare `run(file)`. No knob renamed — they are orthogonal and correctly named. Code lines are exactly neutral (353 -> 353); the win is at the call sites. Also refreshes five comments that still described `calleesReturnFlowingSeen` and the two-branch note, both of which the preceding commit replaced. 102 unit and 10 integration tests pass; test count moves 9 -> 7 in the chained-receiver file, exactly the two redundant rows. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(mcp): publish return-value-ascent coverage on the PDG impact result `impact(mode:'pdg')` computed four facts about ascent coverage and used them exactly once — to interpolate an English sentence. They never reached the result object, so an agent consuming this MCP output could only ask "was the ascent complete, and if not why" by regexing prose. The cost was already demonstrated: a pure rewording commit earlier in this branch broke ~30 assertions and would have silently broken any consumer keying on the old phrase. Adds `pdgEvidence.ascent`: referencesScanned how many call-site callee references were scanned returnFlowFound did the ascent fire anywhere in this slice undecodableSummaryCount summaries the codec could not decode examinedComplete was the examined set the whole callee list incompleteReasons 'traversal-truncated' | 'callee-list-capped' callSummaryLayerPresent false => pre-FU-C (v3) index Nested under `pdgEvidence` because that is the established counts-and- classification namespace, and `composeUnifiedPdgImpactResult` already spreads it, so the member survives the unified compose untouched. `incompleteReasons` carries CODES, following the existing `truncatedByReasons: ('depth'|'limit')[]` precedent. The prose clause and the structured field now render from one array computed once, so an agent branching on codes and a human reading the note cannot disagree, and a third reason becomes a rendering decision rather than a contract change. Two shape decisions worth recording. `callSummaryLayerPresent` exists because without it a v3 index publishes `referencesScanned: N, returnFlowFound: false`, which reads as "these callees record no return-flow" when the truth is "the layer that records it is absent" — the note already distinguishes those, and the structured surface must not be less honest than the prose. And the field is ABSENT rather than zeroed when the descent never ran (upstream slices): "nothing was scanned" is a different fact from "we scanned and found nothing". `pdgResultVersion` stays 2. The documented trigger is a BREAKING change to the result shape; this removes nothing, renames nothing, and changes no existing field's meaning. Confirmed mechanically: zero top-level key drift across 2304 cases. The historical v2 bump was for changing an existing field's semantics (startLine 0- to 1-based). The note prose is byte-identical, proven across the same 2304 cases with a negative control — perturbing one character of the phrase table produces 60 drifts, so the harness demonstrably detects what it asserts. 14 new tests cover the structured surface and all 14 fail when the source is reverted, while the 54 prose tests pass unchanged. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(helpers): share one module-load probe, and fix two guards that passed on broken builds Three tests independently spawned a child node process to inspect what a built `dist/` entry loads, duplicating the REPO_ROOT derivation, the probe source, the missing-dist guard, the spawn with NODE_OPTIONS cleared, the status-vs-signal rendering, and the payload parse. The newest copy was also the only correct one, so the next author had 2-in-3 odds of copying a weaker probe. The two older probes diff `require.cache` only, which is structurally blind to the first-party ESM `dist/**` graph. That is not theoretical — both were demonstrated passing on genuinely broken builds: - Severing `dist/cli/mcp.js -> stdio-context.js` (a pure ESM change) leaves the require.cache diff EMPTY, so `import-closure.test.ts`'s two assertions reduce to `[].filter(...) === []`. It reported 2 passed on a severed graph. - Severing `registry -> swift/query.js` leaves 76 unrelated CJS entries, which satisfied `registry-import-closure.test.ts`'s indirect guard. The Swift half of its headline had gone vacuous and it reported 1 passed. Both now fail on those same builds, naming the missing anchor. `test/helpers/module-load-probe.ts` unions the ESM `registerHooks({ load })` channel with the cache diff, probes entries concurrently, and makes non-vacuity STRUCTURAL: `anchor` and `minModules` are required fields and the helper throws when either fails. A vacuous probe is a harness failure, not a silently green test, so it cannot be forgotten. Forbidden patterns and remedy text stay per-test — the harness is the shared part, the policy is not. Also fixes `toRepoRelativePosix` resolving non-absolute specifiers against `process.cwd()`, and dedupes modules a CJS-from-ESM import reported once per channel. Faster despite doing more: the registry file goes 12.4s -> 6.75s, because `spawnSync` burned the parent thread polling while the child loaded native grammars. `import-closure` drops to one spawn from two. The `local-backend.js` entry is kept although its closure is currently a strict subset of `server.js`'s: that is an observation, not an invariant. If `server.js` ever stops eagerly reaching the local backend, the server probe stays green while the module #2802 actually changed goes unobserved — and now that anchors are mandatory, that entry is what pins `pdg-impact.js`. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(lbug): trim the csv-generator note and fix the claim it got wrong Two reviewers split on this comment: one wanted it cut to the structural argument, the other said a comment is the right depth for documenting a rejected change since there is no invariant to guard. Both are right, so it stays a comment and gets shorter — 13 lines to 6. Trimmed because it had already taken two corrections (an unreproducible "~40x" figure, and a pointer to a test file that no longer exists), and its tail had drifted from its own guard: the comment said "several hundred modules, ~150 ms" where `startup-language-closure.test.ts` says "~226 extra modules and ~130 ms". Two numbers for one fact. That tail is documented better in the guard's own header, so deleting it loses nothing. It also stated the load-bearing claim inaccurately. The old text said bm25-index imports `normalizeFtsText` "from here" — but `lbug-adapter.ts` neither exports nor re-exports it; the only occurrence of the identifier in this file WAS the comment. Anyone verifying would have grepped, found nothing, and concluded the note was stale. Now names `csv-generator.js` explicitly, re-verified at `bm25-index.ts:15` (static) and `local-backend.ts:2756` (dynamic, on the FTS query path). Comment-only, proven two ways: every changed line matches a comment pattern, and stripping all `//` lines from HEAD and from the working tree yields byte-identical text. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(helpers): extract the temp-repo lifecycle, collapsing five hand-rolled cleanups into one Four cfg integration tests each hand-rolled a `tmpDirs` array, a mkdtemp-and-register step, and an `afterAll` rmSync. It is actually five registrations across six creation sites — `pipeline-pdg.test.ts` keeps a second pool for its C-family fixtures. Seeding genuinely varies four ways (recursive cpSync, single copyFileSync, inline mkdir+writeFile, and nothing at all), so a fixture-copier helper would have fitted about half the sites and made things worse. Extracted the LIFECYCLE instead — mkdtemp, register, afterAll cleanup — which is byte-identical at all five registrations and is the correctness-critical part. `dir()` returns an empty registered directory for callers that seed themselves; `fromFixture()` covers the common case. That fits 6/6. The duplication had already produced a latent defect: `cFamilyTmpDirs` was cleaned by TWO `afterAll` blocks, harmless only because `rmSync` was called with `force: true`. Now one hook. `createTempDirPool` is a function called from each test file's module scope rather than a top-level hook in the helper, because under ESM caching a module-level `afterAll` would register once, against whichever file imported it first. That hazard is documented in the helper. Raw line count is roughly neutral (-44 across the tests, +62 for the helper, 29 of which are the rationale). The win is that a cleanup invariant went from five copies to one. Cleanup verified empirically, including the failure path: a throwaway suite whose `beforeAll` throws still has its directory removed, and every temp directory created by the four migrated files is gone after a run. 46 tests pass across the four files. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(resolvers): pin the inference-typed field receiver gap at the resolver level The gap was pinned only in a PDG test, asserting on `BasicBlock.calleeIds` behind the full `--pdg` pipeline. But it is a resolver fact: when a class field's type must be inferred from its initializer, chained receiver calls resolve to nothing. Whoever closes it will be working in the resolver suite and would have got a red CFG/PDG test with no resolver-side signal. Asserts CALLS edges directly, alongside `python-constructor-field-receiver.test.ts`. Nine receiver shapes run the identical statement; seven resolve, two do not: const o = new Outer() resolves private p: Outer = new Outer() resolves private p: Outer; this.p = new Outer() resolves private p: Outer; this.p = p (ctor arg) resolves constructor(private p: Outer) {} resolves makeOuter().inner().compute() resolves o.inner().mid().compute() (three links) resolves private p = new Outer() NO EDGES private p; this.p = new Outer() NO EDGES Two things the fixture establishes that the PDG-side pin could not. The discriminator is the type ANNOTATION, not local-versus-field — the parameter-property form resolves fine. And the initializer is NOT invisible to the resolver: `new Outer()` still emits its own constructor CALLS edge, byte-identical to the annotated twin. Only the initializer-to-field-type binding is missing, which narrows where a fix belongs. Assertions key on exact node ids rather than names, because `compute` is ambiguous across two classes and keying on the source name collides with `Object.prototype.constructor`. No `describe.skip` and no `it.fails` — the latter passes when the body throws for ANY reason, so it can go green on a rotted premise. The gap is pinned as its explicit current value, which self-diffs: simulating the fix fails one test showing the two newly-resolved ids, and renaming a fixture symbol fails the non-vacuity guard. Runtime is comparable to the PDG-side pin (~9-11s, both dominated by worker startup), so this is an altitude and scope win, not a speed one. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(mcp): replace the extension sweeps with a stronger language-agnosticism pin Two `it.each` sweeps over nine file extensions asserted that the empty-ascent caveat was present (or absent) for each. They looked like the pin for the property the whole change exists for — `pdg-impact.ts` must name no language and its output must not vary by extension — but they were the weakest available form of it. They asserted substring presence/absence, so a language dependence that ADDS text while leaving the caveat intact passes them. Demonstrated, not assumed: injecting a `.py`-only hedge inside the caveat sentence and replaying the two sweeps verbatim against that source gives 18 passed. The byte-identity test beside them caught it. So the sweeps are deleted and the identity test carries the property alone, hardened in two ways: - Two rows instead of one, covering BOTH sides of the caveat gate. The silent (return-flow present) branch previously had no identity counterpart at all — nine runs proving one fact, with nothing checking that its rendering was extension-invariant. - The fingerprint spans the note AND the reachable blocks, not just the note. Strictly more than the sweeps verified. Entailment is exact: identity across the extension set, plus the two existing single-extension content assertions, gives "every extension gets the caveat" and "no extension gets it". Reducing a sweep to one extension was rejected because it reproduces an assertion already present verbatim. Also converts the incompleteness block from six near-identical bodies to a 3-row premise table crossed with two assertions. Each row now names the exact phrase set its clause must contain, so presence and absence are asserted together — which adds three checks the longhand version lacked (the budget row now also proves the emit-cap phrase is absent). And three tests that re-rendered one fixture to make one assertion each are hoisted to a single render. 97 tests, down from 116: -18 sweep cases, -2 from the hoist, +1 identity row. No assertion was lost; several were added. Verified by injection: a `.py`-only note change fails the identity pin, and a dependence in the shared hop sentence fails BOTH rows, confirming the second row is load-bearing rather than decorative. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * perf(mcp): lazy-import syncGroup so MCP startup skips the group extractor closure `core/group/service.ts` statically imported `./sync.js`, which pulls all six contract extractors, five of which statically import the native `tree-sitter` binding. That put the whole parser stack on every MCP server start, for a server that never syncs. Only `groupSync` needs it. The other seven group tools — `group_list`, `group_impact`, `group_query`, `group_contracts`, `group_status`, `group_trace`, `group_context` — do not, and now never load it. `syncGroup` has a single call site, already inside an `async` method, so this is a lazy `await import(...)` at that call site and nothing else: no signature change, no async ripple, no change to `local-backend.ts`. The pattern is already established on this exact module — `cli/group.ts`'s sync command lazy-imports `sync.js` the same way. `service.ts` was the outlier. Measured on a native filesystem (overlayfs; /workspace is a 9p mount that inflates ESM resolve, so it is not a valid measurement surface), 5 cold runs, medians: dist/mcp/server.js 521 ms -> 133 ms (-75%) dist/mcp/local/local-backend.js 453 ms -> 66 ms (-85%) tree-sitter modules at both entries: 11 -> 0 Same defect class as #2802, which cut the language-provider registry from the same startup path; this is what remained. The cost is moved rather than deleted: the first `group_sync` call now pays the module load. That is the right trade — `group_sync` is already a long-running operation, and sessions that never sync pay nothing. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(mcp): guard MCP startup against the group extractor closure returning Sibling forbidden-pattern case in the #2802 startup guard, reusing the concurrent probes it already collects — no new spawn, no new harness. Asserts that none of `dist/mcp/server.js`, `dist/cli/mcp.js`, or `dist/mcp/local/local-backend.js` loads a `core/group/extractors/` module or the native `tree-sitter` package. The parser is matched by package prefix rather than a bare substring, so a source file that merely mentions the word can neither satisfy nor trip it. Verified load-bearing rather than assumed: restoring the static `import { syncGroup }` in `core/group/service.ts` and rebuilding turns `dist/mcp/server.js` red and names all seven offenders — http-route, grpc, thrift, topic, include, manifest and workspace extractors. Reverted and re-confirmed green. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * perf(mcp): keep the analyze-only CFG closure off MCP server startup (#2802 review) `mcp/local/pdg-impact.ts` imported `CALLEES_TRUNCATED_SENTINEL` and `CALLEE_ID_SEP` from `core/ingestion/cfg/emit.ts`. ESM evaluates a module to import any binding from it, so those two strings dragged the whole analyze-only CFG closure into every MCP server start. Measured against a clean build, per entry point: 8 modules — `emit`, `reaching-defs`, `reaching-defs-graph`, `control-dependence`, `post-dominators`, `synthetic-escape`, `call-site-harvest`, `reaching-def-reason-codec` — present at `dist/mcp/server.js`, `dist/mcp/local/local-backend.js` and `dist/mcp/http-transport.js`. Same defect class as the language-provider closure this branch already removed, and the guard could not see it: `FORBIDDEN_RE` covers `core/ingestion/languages/` and `FORBIDDEN_GROUP_RE` covers `core/group/extractors/|node_modules/tree-sitter`, neither of which matches `core/ingestion/cfg/`. The format constants move to a new LEAF module `cfg/callee-cell-format.ts` that imports nothing; `emit.ts` re-exports both names so every existing importer is untouched, and producer and consumer still resolve to one definition — the drift the shared constant exists to prevent stays impossible. Deleted, not deferred — the same bar #2802 held its own csv-generator proposal to. After: cfg modules at startup 8 -> 2, and both survivors (`callee-cell-format`, `reaching-def-reason-codec`) are leaves that import nothing. Totals: `server.js` 387 -> 380, `local-backend.js` 163 -> 156, `http-transport.js` 523 -> 516. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(mcp): stop pdgEvidence.ascent claiming a completeness it cannot have (#2802 review) `examinedComplete` is the field a consumer reads to decide whether `returnFlowFound: false` is a whole-slice claim. It could be published `true` over a callee set the descent never finished examining — the exact false all-clear the field was added to prevent. Root cause: `bfsReachableBlocks` sets `truncatedByDepth` when its frontier is still non-empty at the budget, but both call sites inside `interproceduralDescent` folded only the row-limit flag and dropped the depth flag. The top-level intra BFS's copy of that same flag was already propagated, so the asymmetry was unintended — one `if`-pair folding limit-but-not-depth, within a merge that already folds the node cap too. Reproduced at `maxDepth: 3`, the shipped default: a criterion calling a helper whose body is a 5-block dependence chain, with the return-flowing callee on the block past the clamp. Result reported `truncated: undefined`, `examinedComplete: true`, `incompleteReasons: []` and an unqualified universal note sentence. Fixed by propagating the dropped flags rather than inventing a parallel channel: `intraDepthBudget` is documented in-file as the SAME clamp the top-level intra BFS applies, and that one's depth truncation is already result-level. So the result's own `truncated`/`truncatedBy` were under-reporting for the same reason, and both surfaces are corrected together. Four further honesty fixes to the same published record: - Blocks reached only by the U-C4 ascent went into `reachable` but never `hopReached`, so their `calleeIds` cells were never scanned, never counted, and could not raise `callee-list-capped`. They are slice blocks; they now enter the hop set and get the same treatment as every other one. - `pdgEvidence.ascent` was absent on the empty-slice early return even though the descent had already run and scanned, contradicting the "present iff the descent ran" contract this branch itself added to `tools.ts`. Both exits now classify through one shared helper so they cannot disagree. - A block carrying call sites in `callees` but no resolved ids in `calleeIds` (the whole-file case where `emit.ts` has no fileMap) silently shrank the population while `examinedComplete` still reported `true`. That now raises a third reason, `callee-ids-unrecorded`. - `referencesScanned` is a distinct-callee tally and both surfaces described it as a call-site count. Field name kept — a rename is breaking at `pdgResultVersion: 2` — and the prose corrected instead. `PdgAscentIncompleteReason` gains a member, which is additive, so `pdgResultVersion` stays 2. Visible output change worth knowing: slices whose callee chain outruns `maxDepth` now report `truncatedBy: 'depth'` where they previously reported none, and a repo with id-less call sites now reports `examinedComplete: false`. Both are strictly more honest. Every behavioural change carries a mutation proof — revert the source, watch the new test go red, restore. One exception is documented inline rather than faked: the ascent-side fold cannot be observed independently, because the re-seed shares the caller's `visited` set and so can only reach past the budget when the traversal that covered that closure was already cut and had already raised a flag. Suite: 49 -> 59 tests. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(mcp): anchor each import-closure policy on the edge it polices (#2802 review) `module-load-probe.ts` makes non-vacuity structural via a required `anchor` — but the anchor was one per ENTRY while `startup-language-closure.test.ts` now runs TWO independent policies. The group-extractor policy added in 83e8cf7c5 therefore had no anchor of its own, and one of its three rows was already vacuous: `cli/mcp.js` loads four leaf modules and reaches no `core/group/` module at all, so its group assertion could not fail for any policy-related reason while its `dist/mcp/stdio-context.js` anchor stayed green. Proven, not argued. `dist/mcp/local/local-backend.js` is the only static importer of `core/group/service.js` in the whole build; severing that one edge — the exact next lazy-load step — and re-probing: OLD shape (anchor per entry): server 385, http-transport 521, local-backend 161 reaches group/service = false, group offenders 0 -> GREEN on all three NEW shape (anchor per policy): -> RED on all three, each naming the missing dist/core/group/service.js Counts fell only 387->385 and 163->161, so `minModules` was structurally blind to the severance; the anchor is the only thing that catches it. `anchor` accepts `string | readonly string[]` and every listed anchor must load. Existing single-anchor call sites are unchanged. `anchorsOf()` lets the group `it.each` DERIVE its entries by filtering on the group anchor, with a test pinning that derivation, so the policy cannot silently register zero cases. `cli/mcp.js` is dropped from the group policy — it cannot honestly carry that anchor — and the doc-comment now states the invariant: an anchor is per-POLICY, not per-entry. Also: - `mcp/http-transport.js` gets a row. It is the largest startup entry (516 modules) and `src/cli/mcp.ts` imports it directly rather than through `server.js`, so nothing about the server row constrained it. Measured clean today; the gap was coverage, not a broken claim. - The three spawn-based closure tests are registered in `SPAWN_CLI`, so the Windows-safety plumbing this branch wrote for them (POSIX normalisation, `pathToFileURL`, `NODE_OPTIONS` clearing, array-form `spawn`) is finally exercised on the Windows/macOS matrix. Measured cost ~11.7s on Linux; budget ~60s on Windows against a 25-minute job. - `PROBE_TARGET` now wins over `extraEnv`, which was spread last and could have silently redirected a probe while `anchor`/`minModules` stayed keyed on `entry`. - The child's JSON payload is validated through a type predicate instead of a bare `as string[]`, and the spawn timeout escalates SIGTERM to SIGKILL so a child stalled in native code is reaped rather than orphaned. - Recorded baselines re-measured (server 380, local-backend 156, cli/mcp 4) and relabelled a snapshot rather than a contract — they moved twice inside this branch alone. The subset claim was re-verified exactly: 0 of local-backend's 156 modules are absent from server's 380. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(helpers): survive a failing temp-dir removal instead of leaking the rest (#2802 review) `createTempDirPool`'s `afterAll` ran a bare `for (const d of created) fs.rmSync(d, {recursive, force})`. `force` suppresses only `ENOENT` — not the `EBUSY`/`EPERM`/`ENOTEMPTY` class a Windows runner produces when a pipeline test still holds a handle — so the FIRST failure threw out of the loop and leaked every directory registered after it. Pre-existing: all four hand-rolled cleanups this helper consolidated had the same shape. But the blast radius is now shared across four consumers, which is exactly why it is worth fixing at the point of consolidation. Cleanup is now per-directory best-effort via `removeTempDirs`, plus Node's own documented mitigation for that error class (`maxRetries: 3, retryDelay: 50`), which costs nothing on the happy path. Warn rather than swallow or rethrow, and the reasoning is in the doc comment, not just here: rethrowing would fail an otherwise green suite from `afterAll` over housekeeping the OS reclaims anyway, where it reads as a test failure and buries the real result — a Windows EBUSY on a temp dir is not a defect in the code under test. Silence is the opposite hazard: a systematic leak would be invisible with nothing naming the responsible suite. The warning carries the path, and the `mkdtemp` prefix is per-pool, so it names the suite that made it. Failure is injected through a scripted remover keyed by path (a Map lookup, so no `if` in a test body and no dependence on producing a real locked handle). Beyond the three behavioural pins there is a wiring pin — a nested `describe` creates a real pool and a sibling `it` declared after it asserts the dirs are gone — so the tested function cannot drift into "tested helper plus an untested copy of the loop". Mutation proof: restoring the abort-on-first-failure loop turns 3 of the 5 tests red, the throw escaping `removeTempDirs` outright so the third real directory is never attempted. With the fix, `[first, blocked, last].map(existsSync)` is `[false, true, false]` — the injected failure survives and the directory after it is really gone, through the remover that actually ships. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(pdg): point the self-diffing receiver pins at #2807, not at this PR (#2802 review) Both pins named the gap "(#2802 follow-up)". The gap has its own tracking issue — #2807, "Inference-typed field receivers resolve to no CALLS edges at all" (open, labeled bug) — and PR #2810 is already open against it. As written, after merge the gap was discoverable only by reading a KNOWN GAP marker inside a test file, not from the issue tracker. Both describe names now read "(known gap: #2807)" and both KNOWN GAP test names carry the number. #2802 is kept only as provenance: the gap was FOUND during #2802 work but is pre-existing and independent of it. Each header gains an explicit "this pin is self-diffing: it will go red on purpose" section naming #2807 with its exact title, noting #2810 is open against it at the time of writing, and stating that the pin asserts the gap EXISTS — so closing #2807 fails it by design, and the correct response is to update the expected value, not to relax the assertion. The same note is repeated inline above each KNOWN GAP test, where a maintainer editing it will actually see it. No pin is weakened. Both deliberately reject `it.fails` in favour of exact `toEqual` assertions with a non-vacuity probe, and that design is left untouched. Refs #2802, #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(group): cover the lazy syncGroup import that no test reached (#2802 review) 9ea9676dc turned `GroupService.groupSync`'s `syncGroup` into `await import('./sync.js')` — this branch's one changed control-flow line in production code — and nothing exercised it. Every existing test stopped short: `service.test.ts` returns at the empty-name guard; `group-service-not-found.test.ts` mocks `loadGroupConfig` to reject and never invokes its `syncGroupMock`; `group-sync.test.ts` imports `syncGroup` directly, bypassing `GroupService`; and the startup guard asserts only the negative, that `sync.js` is absent at startup. `tsc` catches a path typo, but nothing verified the import resolves and hands off correctly — while every production `group_sync` call goes through that line. No production change was needed; the reviewed design was sound. This is the missing coverage. The happy-path test mocks nothing: it points `GITNEXUS_HOME` at a pool temp dir, seeds a real `group.yaml`, and calls `groupSync`, so `loadGroupConfig` resolves, `groupDir` is found, and execution falls through into the REAL `syncGroup`. What makes a real sync reachable with no indexed repo: an empty registry puts both members in `missingRepos`, but one declared manifest link still yields synthetic-UID contracts. It asserts the returned counts AND reads back the `contracts.json` that real `syncGroup` wrote into `groupDir` via the production `readContractRegistry`, which pins the option handoff too. Two further tests use `vi.doMock` to re-evaluate the service against a `sync.js` whose load throws: one asserts the call rejects with the load failure in its `cause` chain — so the caller gets a catchable rejection, not a floating unhandled one — and one asserts both pre-import guards still answer with `sync.js` unloadable, which is also a structural pin that the module has no STATIC import of it (a static one would throw at re-import, before any call). Mutation proofs: pointing the specifier at `./sync-nope.js` turns 2 of 3 red ("Cannot find module .../sync-nope.js ... at GroupService.groupSync service.ts:349"); aliasing a real-but-wrong export turns 1 red. Restored, all 3 green, and `service.ts` verified byte-identical to HEAD. Out of scope, stated rather than glossed: the final `isError: true` MCP envelope is produced above `GroupService` and needs a full `LocalBackend`; the rejection test is the in-scope half of that claim. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(mcp): close the gaps a cleanup pass found in the #2802 review fixes Quality pass over the review-response series (reuse / simplification / efficiency / altitude). No behaviour change except where noted. The two that mattered: - **The cfg/emit fix had no guard.** `FORBIDDEN_RE` covers `core/ingestion/languages/` and `FORBIDDEN_GROUP_RE` covers `core/group/extractors/|tree-sitter`; neither matches `core/ingestion/cfg/`. Because `emit.ts` re-exports the constants, pointing `pdg-impact.ts` back at `cfg/emit.js` typechecks identically and silently restores all 7 modules. Verified: with the import reverted, `tsc --noEmit` still exits 0 and every test stayed green before this commit; after it, 3 rows go red naming the offenders. Written as an ALLOWLIST of genuine leaves rather than a denylist of the 7 already-suffered modules, because the next regression is a module nobody has thought of yet. - **`FORBIDDEN_GROUP_RE`'s parser matcher was forward-slash only** while both sibling probe regexes spell the separator `[\\/]`. Native bindings arrive via the `require.cache` channel as absolute paths and `toRepoRelativePosix` only normalises paths inside the repo root, so a hoisted `node_modules` renders as `…\node_modules\tree-sitter\…` on Windows and matched nothing. The same series put this file on the Windows matrix, where that half of the assertion would have been vacuous. Reuse — three re-implementations of existing helpers: - `removeTempDirRecursive` re-rolled `fs.rmSync` retries; it now delegates to `cleanupTempDirSync` (`test-db.ts`), the repo's Windows-lock-aware remover. The copy had already drifted on both knobs that matter — 3 retries at 50 ms vs 5 at 100–400 ms, and warn-on-everything vs swallow-lock-codes-rethrow-rest — which is how one half of a suite goes green-with-a-warning on the same `EBUSY` the other half fails on. The per-directory try/warn loop, which is the actual fix, is unchanged. - `errorChainText` re-rolled the cause-chain walk that `causeChain` (`src/lib/utils.ts`) exists to be the single copy of — its own doc asks callers not to. - The SIGKILL escalation (a timer, an `unref`, and two `clearTimeout`s) is `spawn`'s own `killSignal` option, which Node's `timeout` already delivers. Simplification and altitude: - `'callee-ids-unrecorded'` documented ONE of its three producer paths. The unnamed common one is a call site that did not RESOLVE — exactly the receiver gaps this repo pins (#2807) — so on a real index the reason fires broadly, driven by resolution quality rather than a missing `--pdg` layer, and "re-run analyze --pdg" is the wrong remedy for it. Doc now names all three and states the consequence: `examinedComplete: true` is the strong, rare signal. - The derived policy-entry list was re-pinned against a hand-written 3-element literal, reinstating one layer down the list the derivation removes. Now asserts the properties that are actually at risk — non-emptiness (a policy going silent) and `cli/mcp.js` staying excluded (a row that cannot fail). - A test fixture spread `ascentBlockCell: 'idless'` and then overrode it to `'capped'` in both runs, so the id-less shape never reached the mock while reading as though it did. - `idlessCallSites` is sticky, so its per-row string allocation now short-circuits once set. - Dropped an unused `export` on `CleanupWarner`. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * revert(ci): unregister the module-load closure guards from the Windows matrix Registering the three `dist/` closure guards in `SPAWN_CLI` turned the Windows `platform-sensitive 1/3` shard red at the 20-minute watchdog. Baseline 83e8cf7c5 was green on all three shards; a4245119c (which added them) failed 1/3; d0b201442 failed the same way. It is not the files themselves. On the Windows runner they are among the cheapest in the suite — `registry-import-closure` 448 ms, `import-closure` 53 ms — and both passed. vitest shards this list by file COUNT, not runtime, so adding three files RESHUFFLED the split: shard 1 went to 32 files against 26 and 29, concentrating the heavy CLI e2e suites. It timed out with `cli-e2e`, `group/cross-trace-e2e`, `lbug-orphan-sidecar-recovery` and `server-http-startup` still queued — `cli-e2e` being the ~50-spawn suite whose setup flakiness already needed fixing once (PR #2000). That clustering fragility is pre-existing and this file's own header documents it (#2449: "the heaviest spawn suites can cluster on one shard", busiest Windows shard already at 14m57s against the old watchdog). These three files only tipped it over, and unblocking the PR beats holding it for a CI-infra fix that belongs in its own change. Reverted rather than worked around: raising the shard count would keep the coverage but is a repo-wide CI change made on a 25-minute feedback loop with no guarantee the reshuffle balances, and this PR is about MCP startup. The removed entries are replaced by a comment recording WHY they are absent, what they were measured to cost, and the precondition for re-landing them — so the gap is documented at the point someone would otherwise re-add them blind. Verified: the emitted file list is byte-identical to 83e8cf7c5's, so the shard split returns to the configuration that was green. The Windows-specific bug this series found is unaffected — `FORBIDDEN_GROUP_RE` now spells its separator `[\\/]` like its siblings, which was a real forward-slash-only vacuity, and that fix stays. Refs #2802, #2449 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(ci): shard the cross-platform matrix by measured weight, not file count Restores the three `dist/` module-load closure guards to the Windows/macOS matrix, and fixes the reason they could not stay there. They must run on every OS — the shared probe in `test/helpers/module-load-probe.ts` IS the platform-varying code (array-form `process.execPath` spawn, cleared NODE_OPTIONS, `pathToFileURL` because Windows rejects a bare absolute path as an ESM specifier, and a `path.sep`→POSIX normalisation the anchors and offender regexes depend on). Ubuntu-only coverage of a platform guard is no coverage. The earlier attempt turned Windows `platform-sensitive 1/3` red at the 20-minute watchdog, and the reflex fix — unregistering them — treated the symptom. The files are among the cheapest in the suite (measured 448 ms, 53 ms, sub-second, and both that completed passed). The defect is that `run-cross-platform.ts` handed vitest all 84 files plus `--shard=i/n`, and vitest partitions by file COUNT. Runtimes here span three orders of magnitude, so a count-split is blind to the thing that decides the budget, AND re-partitions on every insertion: adding three free files reshuffled the list and happened to co-locate `cli-e2e` (361 s) with `cli-limit-e2e` (75 s) and `analyze-heap-oom-e2e` (23 s) — 32 files against 26 and 29 — which timed out with four still queued. The split now happens in `scripts/cross-platform-shard.ts`, longest-processing- time first over measured Windows runtimes, and only the chosen shard's files are passed to vitest (`--shard` is consumed, never forwarded — forwarding would re-partition the slice a second time and silently drop most of it). Weights are measured, from the last green matrix run plus the timed files of the failing one, and every file also carries an 8 s per-file floor. That floor is calibrated, not guessed: the last green busiest shard ran 736 s of wall clock over ~511 s of attributed file time. Without it the balancer isolates the two monsters and then piles every light file onto the remaining shards — trading a runtime imbalance for a count imbalance that costs the same. Result at TOTAL=3, with the three guards back in: 521 s / 527 s / 519 s across 20 / 33 / 34 files. The previous green configuration's busiest shard was 736 s, so this is better balanced than the state before any of this, and the busiest shard is now bounded by construction rather than by sort-order luck. `test/unit/cross-platform-shard.test.ts` pins the properties, and the load-bearing one is not "the split is even" — it is "adding a cheap file cannot move a heavy one", the property whose absence caused the outage. Two details in that test are themselves load-bearing, and earlier drafts got both wrong and were vacuous: the inserted names must sort EARLY (names sorting last disturb nothing under any scheme) and the count must not be a multiple of the shard total (adding exactly `total` files leaves an equal-weight round-robin in the same rotation). Mutation-proved: replacing `weightOf` with a constant — i.e. count-based sharding — turns that test and the per-file-floor test red; restored, all 8 pass. Refs #2802, #2449 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 21:26:13 +01:00
Gergő Magyar	e307286d52	fix(scope-resolution): a named receiver's member never resolves lexically, + two #2695 follow-ups (#2714) ... * fix(scope-resolution): a named receiver's member never resolves lexically (#2699) `lookupCore` Step 1 walked the lexical scope chain for every lookup, including explicit-receiver property reads. So `options.baseUrl` could bind to an unrelated function-local `const baseUrl` in the same file, and `config.extractVisibility(node)` to the enclosing class's own method. This is the residual half of the defect JS/TS block scopes narrowed in #2695. Blocks moved nested-block locals off the chain of a reference outside the block, which removed 114 false edges; a local declared directly in the function body stayed on it, and no amount of extra scopes reaches that case. Fixed at the cause instead: `recv.name` names a member of whatever `recv` denotes, so a binding of the bare tail name in an enclosing scope is never the right answer. Steps 2 and 3 (receiver type / owner members) are the legitimate routes. `this` and `self` are EXEMPT, and that exemption was measured, not assumed. Skipping Step 1 for every explicit receiver removed 711 edges on a 762-file corpus — but 2 of those were genuine: `self.srcIx` and `self.streamedAt(...)` after `const self = this`, reaching their own class's members through the class-body scope. For a self-receiver the members and the lexical chain legitimately overlap; for a named receiver they never do. Exempting the self names keeps both true edges and still removes 709 false ones, adding none. The removals were classified by reading source at the site, not by pattern- matching ids — an "is the target a member of the source's owner?" heuristic labelled 43 of them plausible and every one I then read was false: language = config.language; -> the class's own `language` dirMap.get(...) / exactMap.get(...) -> a sibling object-literal `get` return config.extractVisibility(n); -> the class's own method (self-edge) writer.close(); -> GraphEmitSink.close Residual, deliberately kept: a `this.x` read can still bind lexically to a same-named local. That is the price of the two true self-alias edges above. `INCREMENTAL_SCHEMA_VERSION` 19 -> 20: a v19 index holds these false CALLS/ACCESSES on every unchanged file and would keep serving them through the reuse gate. Test confirmed discriminating: it fails with the guard reverted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0184RmD24KFJidYqpM7v3XjR * fix(typescript,javascript): a generator expression binding is a Function node (#2693) `const g = function* () {}` matched none of the closure-binding definition rules — they covered `arrow_function` and `function_expression` only — so the binding emitted a `Const` node. `buildGraphTargetIndex` admits callable nodes only, so `g()` resolved to nothing. Same defect shape as the `var` case #2693 already fixed: a different grammar node for the same construct, and the resulting graph node was not callable. Adds the four variable-binding shapes in both languages: `const`/`let` and `var`, each plain and exported. Purely additive — no existing pattern is reordered or rewritten, because the #2687 pre-scan dedup is order-dependent and collapsing the value/callable pair depends on which match wins. Deliberately NOT covered, and the query comment says so: a generator in an object-literal pair or a HOC wrapper still falls through anonymous. Those are rarer, and each additional pattern is another chance to disturb the dedup. `SCHEMA_BUMP` 26 -> 27: definition captures are parse-time, so a warm parse cache would replay the old ones verbatim — `--force` does not clear it. Two tests confirmed discriminating (they fail with the patterns reverted), plus a guard that the already-working generator DECLARATION form is unaffected, since it shares the emit path these were inserted beside. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0184RmD24KFJidYqpM7v3XjR * fix(ingestion): keep caller attribution in lockstep with definition ids (#2699) The definition phase appends `localIdentity` to a nested callable's own name segment (`run.save@3:2`); `findEnclosingFunctionId` did not, so the two phases derived different ids for the same callable. The failure mode is silent — the caller id names a node that does not exist, so the edge is dropped rather than reported — which is why the parse-worker docblock calls this pair a lockstep guarantee and asks that both phases derive the prefix from one place. The condition is now byte-identical to the definition phase's (`nestedPrefix !== undefined`), so the two cannot diverge again. Scope of the claim, stated plainly: no reproducing case was found, and this changes nothing measurable on a 762-file TypeScript corpus. TS/JS resolve callers through `resolveCallerGraphId` in the graph bridge, not this path; `findEnclosingFunctionId` serves the `callExtractor` languages, and the corpus does not exercise a nested callable there. The review that raised it (P3) observed zero dangling edges, and "zero dangling" is also what silently dropped edges look like — so this closes a documented contract rather than a demonstrated bug, and carries no test of its own. Rides the `SCHEMA_BUMP` 26 -> 27 in the preceding commit: caller attribution runs in the worker, so a warm parse cache would replay the old ids. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0184RmD24KFJidYqpM7v3XjR * docs(test): correct the block-scope header that this PR made false (#2699) Review finding (MEDIUM). The file header still described `lookupCore` Step 1 as walking the lexical chain for EVERY lookup, and called the function-body-local case "unchanged and still mis-resolves ... pre-existing and tracked separately". Commit 59b892ca in this same PR falsified both, and the describe block added ~80 lines lower in this same file asserts the opposite — a reader scoping future work from the header would have concluded the case was still open. Rewritten to state what the code does: Step 1 is skipped for a NAMED explicit receiver, the function-body case is fixed here, and the surviving residual is that a `this`/`self` read can still bind lexically to a same-named local — with the reason those two names are exempt (they keep the genuine `const self = this; self.member` reads that Step 1 resolves correctly). Also corrects a PRE-EXISTING staleness inherited from #2695 in the same paragraph block: "the genuine bare read of that same local must still emit its edge" describes a test that no longer exists, because TypeScript emits no `@reference.read` for bare identifiers at all. Fixed here rather than left adjacent to a freshly corrected sentence. Comments only — `detect_changes` reports 0 changed symbols across 1 file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0184RmD24KFJidYqpM7v3XjR * refactor(ingestion): give the nested-callable id rule one definition (#2699) Review finding (LOW): the lockstep change in this PR shipped without a test. The plan called for a unit test asserting the two id-derivation phases agree. Two things changed that plan during execution, both recorded here. FIRST — there are THREE phases, not two. Re-verifying the plan's assumption (`grep -n localIdentity`) found a third call site: the worker-path node-id derivation in `processFileGroup` (parse-worker.ts:2316), whose own comment already acknowledged the coupling. `impact` on `localIdentity` corroborates: three direct dependents, all in the Workers module. So the invariant three phases must agree on is now ONE function, `nestedCallableQualifiedName`, and divergence requires deleting a call rather than editing a duplicated expression. SECOND — the planned `_forTest` alias seam does not work for this module. `parse-worker.ts` posts a `ready` message to `parentPort` at module scope, so value-importing it from a unit test throws before any test runs; the existing unit tests that reference it use `import type` only, which erases. The rules therefore move to a new pure module, `workers/callable-id.ts`. That is what makes them testable at all, rather than merely commented. Pure refactor — no id changes. Verified by the suites that assert exact node ids (`Function:svc.ts:run.save@7:2`, `Function:c.php:run.$save@3:2`): 74/74 green, and `detect_changes` reports only the three expected symbols and the two `processFileGroup` flows `impact` predicted. The test pins both halves: the rule's contract, and a structural assertion that no site has re-inlined `${prefix}.${localIdentity(...)}` — the unit assertions alone would still pass if a fourth phase spelled the rule out by hand, which is exactly how the divergence arose. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0184RmD24KFJidYqpM7v3XjR * fix(scope-resolution): give PHP's `$this` the same self-receiver exemption (#2699) Review finding (LOW). The Step-1 skip added in this PR exempts `this`/`self`, but the receiver name arrives as the reference node's RAW SOURCE TEXT — `extractExplicitReceiver` returns `cap.text` verbatim — so PHP's `$this->x` presents as the string "$this" and matched neither entry. PHP was the one supported language whose self-receiver got no exemption at all. Measured, and the measurement is why this is framed as consistency rather than a bug fix: - Corpus delta ZERO. 762-file TypeScript corpus, CALLS+ACCESSES set diff: 13179 -> 13179, added 0, removed 0. So no INCREMENTAL_SCHEMA_VERSION bump (stays 20), per the plan's decision rule. - No PHP shape found that DISCRIMINATES. Both the simple `$this->prop` / `$this->helper()` shapes and a closure reading `$this->…` inside a method that also declares a same-named local produce byte-identical edge sets with `$this` present and absent — Step 2 resolves the receiver's type first. The added test is therefore labelled a COMPANION INVARIANT, exactly as the `this.baseUrl` case beside it is, and does not claim to prove the fix. It is still worth making: the exemption is protective, and the 709-removed / 0-true-lost measurement that justified the narrow guard was TypeScript-only, so PHP's safety was never established by evidence. This closes that by construction. Two corrections to what the plan assumed, both found by checking: - The plan (and my first draft of this comment) claimed the codebase had no precedent for handling a sigil'd receiver name. FALSE: `THIS_RECEIVERS` in `core/ingestion/type-env.ts:244` has always listed `$this`, and it is the ingestion-side twin of this very list. The precedent does not merely exist, it validates the approach chosen here — list the spelling as data, do not strip sigils. - That twin also lists `Me`. Deliberately NOT mirrored: no entry in `SupportedLanguages` is Visual Basic, so it could only ever exempt a variable that happens to be called `Me`. The two lists are otherwise the same set with nothing enforcing it — a fifth instance of the twin-list drift class this PR keeps meeting. A drift guard is the right fix and is out of scope here; noted for follow-up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0184RmD24KFJidYqpM7v3XjR * fix(rust): resolve `Self` in scope-resolution type bindings (#2699) CI regression, caught by `tests / ubuntu / coverage` on 13d5e738 and traced to the named-receiver Step-1 skip earlier in this PR (59b892ca), not to the three commits above it — verified by reverting those three and reproducing the failure unchanged. `test/integration/resolvers/rust.test.ts > resolves fresh.validate() inside impl User via Self {} inference` failed: 192/192 on main, 191/192 on this branch. The fixture calls `fresh.validate()` where `let fresh = Self { .. }` inside `impl User` — a genuine call to `User::validate`, and a TRUE edge that the skip deleted. Root cause is a twin-channel disagreement, not the skip: - `type-extractors/rust.ts:142` substitutes `Self` -> the enclosing impl type into the TYPE-ENV channel via `findEnclosingImplType`. - `languages/rust/interpret.ts` recorded `@type-binding.type` verbatim, so the SCOPE-RESOLUTION channel bound `fresh: Self` — a type that does not exist, leaving the receiver's type unknown and Step 2 unable to resolve. `main` passed only because Step 1 still walked the lexical chain for named receivers: the impl scope binds `validate` by name, so the call resolved BY ACCIDENT. Stopping that walk turned a latent gap into a lost edge. The fix closes the gap rather than restoring the accident — `Self` is now substituted at capture-emit time in `languages/rust/captures.ts`, where the impl node is reachable, reusing the `findEnclosingImpl` + `syntheticCapture` idiom already in that file. CORRECTION to this PR's central claim. "709 removed / 0 added / 0 true edges lost" was measured on a 762-file TYPESCRIPT corpus and stated without that qualifier. Rust lost one true edge. The measurement stands for TypeScript; it did not generalise, and the PR body is being updated to say so. Scope of the breakage, measured rather than assumed: 1 failure in 2927 tests across all 51 resolver files. Every other language — Go, Java, C#, Kotlin, Swift, Python, PHP, Ruby, Dart, C++ — passes, which is why this is a targeted fix and not a revert of the skip. Re-baselined `bench/scope-capture` for RUST ONLY (655aed01 -> 7f1240b3); the other 14 language fingerprints are byte-identical. The drift is the intended output change and the reason is recorded in the baseline entry, per that file's own "explain, never re-baseline to make CI green" rule. Verified: rust resolvers 192/192; all 51 resolver files 2926 passed / 1 skipped / 0 failed; the 8 targeted suites 96/96; all 8 CI bench gates PASS; `tsc --noEmit` clean; `detect_changes` reports one touched symbol (`emitRustScopeCaptures`) and no affected flows. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0184RmD24KFJidYqpM7v3XjR * test(golden): refresh the Rust capture golden and the C# PDG snapshot (#2699) The two committed artifacts CI flagged after 5f55fe46. They drifted for OPPOSITE reasons, so each was inspected before regenerating rather than refreshed on sight. RUST GOLDEN — drifted because 5f55fe46 CORRECTS the output. A `Self` type binding now records the enclosing impl's type instead of the literal `Self`, in both the `let x = Self { .. }` and `fn new() -> Self` forms. Blast radius verified exact: 5 fixtures drifted, all 5 contain `Self`, and every `Self`-bearing rust fixture is among them (rust-self-struct-literal, rust-constructor-type-inference, rust-default-constructor, rust-method-enrichment, rust-scoped-multi-file). C# PDG SNAPSHOT — drifted because the named-receiver Step-1 skip (59b892ca) REMOVED A FALSE EDGE. CALLS 7 -> 6, and the edge that went is: Demo.Resolve.Parse@142:12#1 -> Demo.Resolve.Parse@142:12#1 a self-call, from `int Parse(string v) => int.Parse(v);`. `int.Parse(v)` is System.Int32.Parse; the lexical chain was binding it to the enclosing local function that happens to also be called `Parse`. Same defect class as `writer.close()` -> GraphEmitSink.close. The snapshot's own comment says it exists so "a future refactor that silently rewires the C-family graph trips this gate" — it tripped correctly, and the rewiring is an improvement. Both failures were PRE-EXISTING on this PR from 59b892ca, not from the three commits above it — verified by reverting those and reproducing unchanged. They went unseen because this PR's CI was never watched after its first push. Verified after regeneration, WITHOUT update flags so they must genuinely pass: rust-captures-golden 9/9; pipeline-pdg 31/31. The snapshot diff is 3 lines, all inside the C# entry — no other language's snapshot moved. `detect_changes` reports 0 changed symbols (test artifacts only). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0184RmD24KFJidYqpM7v3XjR --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-27 17:56:38 +01:00
Gergő Magyar	89bbdcf566	fix(ingestion): stop double-indexing const X = () => {} as Function + edgeless Const twin (#2687) (#2691)	2026-07-25 16:56:17 +01:00
Gergő Magyar	ed8ab1c246	fix(scope-resolution): resolve callable reference flows (#2437) (#2522) ... * docs(plans): add provider-hook value-refs plan (#2437) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(plans): deepen #2437 plan to USES + property-dispatch design Design revised after prior-art research (Kythe ref vs ref/call, Joern METHOD_REF, Feldthaus field-based call graphs, CodeQL impliedReceiverStep): registration sites emit reference-class USES, invocation is recovered by a field-based property-dispatch pass synthesizing CALLS at member-call sites. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(scope-resolution): model provider-hook value references (#2437) Functions referenced as object-literal property values (provider hooks like emitScopeCaptures: emitCppScopeCaptures) previously produced no edge at all, so impact/context reported a false-safe 0 upstream dependents. Two coordinated halves, per prior art (Kythe ref vs ref/call, Joern METHOD_REF, Feldthaus ICSE'13 field-based call graphs, CodeQL impliedReceiverStep): - Registration -> USES: new ReferenceKind 'value-ref'; TS/JS queries capture pair values and shorthand properties (with @reference.property-key); emitted as a reference-class USES edge, reason 'scope-resolution: value-ref'. Resolution is callable-gated so plain values emit nothing. - Dispatch -> CALLS: new shared pass emitPropertyDispatchCalls synthesizes CALLS (reason 'property-dispatch', confidence 0.7, per-key fan-out cap 32 calibrated on this repo's 16-provider hook tables) from member-call sites to every function registered under the same property key. Deviation from plan: the pass owns value-ref resolution entirely via the post-finalize findCallableBindingInScope walker — the shared registries only see pre-finalize local bindings, so imported hooks (the c-cpp.ts case) were unresolvable through lookupForSite; Reference.propertyKey passthrough dropped as unnecessary. SCHEMA_BUMP 13 -> 14: ParsedFile gains value-ref sites + propertyKey. Verified end-to-end: impact(emitCppScopeCaptures, upstream) now reports 8 impacted / HIGH with extractParsedFile (true dispatch caller) at d=1 via property-dispatch and the c-cpp.ts registration via USES. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(scope-resolution): cover value-ref registration and property dispatch (#2437) Integration: same-file/cross-file/aliased/shorthand registrations emit USES; non-callable and destructuring values emit nothing; dispatch sites gain property-dispatch CALLS (incl. JS twins and per-language partitioning); fan-out-capped keys are dropped entirely; factory-call values unchanged. Unit: capture-shape pins for @reference.value-ref + @reference.property-key. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(scope-resolution): surface dropped property-dispatch keys in stats (#2437) Review finding: skippedKeys was returned but discarded — a hook table larger than the fan-out cap silently reopened the #2437 gap for those keys. Log dropped keys and fold value-ref USES + dispatch CALLS into referenceEdgesEmitted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(plans): add callable reference-flow implementation plan * fix(scope-resolution): close property-dispatch review gaps * feat(scope-resolution): add callable flow facts * feat(scope-resolution): resolve callable value flow * feat(scope-resolution): resolve callable references across providers * fix: harden callable reference flow resolution * fix(scope-resolution): preserve callable binding semantics * docs(plans): add pr-2522-review-fixes plan Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(storage): bump INCREMENTAL_SCHEMA_VERSION for callable-value-flow edges Callable-value-flow CALLS/USES edges (#2437) can connect two files whose content did not change, but the incremental write set only covers changed files — a top-up against a pre-v7 index would silently omit the new edges for every unchanged file pair, indefinitely. Force the one-time full re-analyze (review finding 1, #2522). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(storage): sanitize callable-flow sites per-site at load, log drops The load-time validator rejected the WHOLE ParsedFile when one site was malformed or over-bound, with no logging — and C++ legitimately emits empty-string parameterTypes entries ('' = unknown, the ReferenceSite.argumentTypes convention) for cv-only/ERROR-recovered types, so real repos fell into a permanent, silent warm-cache-miss reparse loop through the #1983-sensitive main-thread path (review finding 7, #2522). Now: '' entries are valid in type arrays; a malformed/over-bound site drops only itself (counted, warned once per load); only non-array garbage — evidence the serialization itself is untrustworthy — rejects the file. Deviation from plan §6 wording: validator-side tolerance replaces emit-side clamps — smaller diff, same asymmetry closed at the single chokepoint. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(scope-resolution): keep declarations in the union for reassigned callable cells The binding-lookup suppression for fact-constrained cells was wholesale: reassigning a declared function through its own name (greet = other; greet()) deferred the call to the solver, which then refused the lexical lookup that resolves the declaration — an unresolvable RHS yielded zero CALLS for a call that resolved pre-flow (review finding 8, #2522). Suppression now applies only to cells bound by FORMAL facts — its actual purpose (a parameter whose grammar emits no declaration binding must not adopt a same-named outer function). Copy/alias/store/load destinations keep their declaration as an inclusion seed (Andersen-style union). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(scope-resolution): count forfeited deferred sites in the budget-bailout warning On work-budget exhaustion the deferred invoke sites end the run with zero CALLS — free-call fallback and reference emission already skipped them — but the warning said 'ordinary graph emission remains untouched', which is false for exactly those sites. The warning context now carries the unresolved deferred-site count and the comment states the real cost (review finding: budget-bailout honesty, #2522). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(scope-resolution): surface dropped property-dispatch keys in stats and warn payload The over-cap warning carried only a count; the dropped key NAMES were discarded and RunScopeResolutionStats had no field, so the PR-body claim 'includes them in resolver statistics' was unimplemented (review finding, #2522; reviewer ask on the fan-out cap). The warn payload now names up to 20 dropped keys and the stats carry propertyDispatchSkippedKeys. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(scope-resolution): drop producer-less ownerQualifiedName from formal sites No capture emitter anywhere produces @callable-flow.owner-qualified-name — the solver branch consuming it was unreachable in production, yet the field was typed, parsed, validated, and unit-tested with hand-built input (review finding 16, #2522; YAGNI). Re-add with a real producer if C++ qualified member declarators ever need it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(scope-resolution): drop dead callable-flow knobs CallableFlowPassingMode 'callable-object' had no producer and no consumer distinguishing it, and CallableFlowCaptureOptions.extractCallArguments had no language providing it (unlike its live sibling extractCallCallee) — review finding 17, #2522 (YAGNI). The invocation-kind 'callable-object' is a different, live concept and stays. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ingestion): bind subscripted callable cells to the container, not the index terminalIdentifier iterates children in reverse, so tbl[i] = handler seeded the INDEX variable's cell (polluting a same-named formal) and tbl[i](7) looked up the callee under i in a different scope — no join, no CALLS edge for the classic function-pointer-array dispatch (review finding 12, #2522). Subscript nodes now recurse into their container field only, in both bindingIdentifier and terminalIdentifier, across the fielded grammars (C/C++/JS/TS/Python/Go/Java). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ingestion): make cross-function file-scope callable bindings resolvable Two stacked gaps killed the canonical C callback-registration pattern (fp assigned in init(), called in run()) — the exact #2437 false-safe this PR exists to fix (review finding H1, #2522): 1. isVisibleValueBinding only consulted assignment regions and formals, so a call in a function OTHER than the assigning one emitted no invoke fact. A declared callable-typed binding is now a value binding wherever its declaration is visible (visibleCallableSignature). 2. The C scope query had no @declaration.variable pattern for function- pointer declarators — void (*fp)(int); created no scope-tree binding, so the seed (init) and invoke (run) cells canonicalized to different keys and never joined. Both bare and initialized forms now bind. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(c): detect variadic parameters via the named variadic_parameter node tree-sitter-c materializes '...' as a named variadic_parameter node; the anonymous-token checks never matched, so variadic function-pointer signatures were emitted with a wrong fixed arity and no '...' sentinel (review finding, #2522). C++ is unaffected ('...' stays an anonymous token there); the token checks remain for such grammars. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ingestion): emit invoke facts for field-stored callable member calls The C ops-vtable pattern (o->run = handler; o->run(1)) captured the store but never the call — the member path in emitCallFacts bailed for languages without protocol methods, and the value-binding index recorded the member store under the OBJECT's name ('o'), not the member's ('run') (review finding 11/M3, #2522). Member destinations now also record their terminal member name, and a member call whose name-cell has a visible store emits an indirect invoke — gated on the store so plain accessor calls (map.get) stay inert. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(cpp): disambiguate (obj->*ptr)() ERROR recovery by token order tree-sitter-cpp groups the recovered '->*' two ways depending on error-recovery cost (identifier lengths): [identifier, ERROR '->*m'] or [ERROR 'obj->*', identifier]. The recovery assumed the first shape, so the second silently swapped receiver/member and dropped the call site — the committed test passed only by name luck (review finding H2, #2522). The identifier's position relative to '->*' inside the ERROR now decides roles. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(cpp): class members are never file-local in hasFileLocalCallableLinkage The name-keyed file-local set is populated from every static declaration, so an in-class 'static void make();' (external linkage — in-class static means no-instance) and any member sharing a name with a static free function were over-marked, refusing legitimate cross-file declaration/definition joins (review finding 13/M2, #2522). Method and Constructor defs now bypass the name-set, per the hook's own linkage-only contract. Deviation from plan step 13: the regression is a unit-level contract pin rather than an end-to-end join test — C++ merges out-of-line member definitions onto the member node by qualified identity, so the graph shape cannot discriminate the join refusal for members. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(cpp): classify parameter passing mode from the declarator chain only A whole-subtree scan for reference_declarator inverted copy vs alias: void reg(void (*cb)(int& out)) marked the by-value pointer cb as 'reference' because of the NESTED parameter's int&, making the solver back-propagate formal targets into every caller's argument cell — alias semantics for a copy (review finding 14/M5, #2522). The chain walk never descends into nested parameter lists; a reference anywhere ON the chain (int& x, void (*&cb)(int)) still aliases. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ruby): bare identifiers are calls, not callable references Ruby parses a receiver-less zero-arg method call identically to a variable read, so 'action = process' — which CALLS process and stores its return — seeded action with the callable and minted a wrong CALLS edge from any dispatch through it, confirmed end-to-end (review finding 15/HIGH, #2522). New provider knob bareNamesAreCalls: a bare name that is not a provably local value binding and not an explicit reference form (method(:x), lambda/proc) emits no flow fact, on both the assignment and argument paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(go): pair multi-value := positionally instead of cross-wiring The shared field fallback took the FIRST LHS identifier and the LAST RHS identifier of Go's expression_list pair, cross-wiring 'a, b := f, g' and synthesizing a garbage comma-joined qualified name — the real relationships were silently dropped (review finding 16, #2522). extractAssignment may now return multiple pairs; Go pairs list entries positionally and emits nothing for a length mismatch (multi-return call RHS). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(java): drop get/test from callableProtocolMethods 'get' and 'test' collide with ubiquitous non-functional-interface APIs (Map/List/Optional/Future.get), so every ordinary container access emitted a spurious callable-object invoke fact — high-volume misleading graph facts with a cross-wiring risk on receiver-name reuse (review finding 17, #2522). Supplier.get/Predicate.test dispatch is deliberately traded away until the check can gate on the receiver's declared type. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(rust): pin the qualified-name no-degrade guard as a hard invariant Rust's scoped_identifier callable-reference capture over-includes unit enum variants and associated constants (Shape::Square seeds as if callable); they stay edge-free only because resolveSeedCandidates refuses to degrade an unresolved qualified name to a simple-name lookup (review finding 18, #2522). Capture-side type filtering would false-negative on tuple-variant constructors, so the guard IS the contract: documented as a hard invariant (Go's mis-shaped multi-value forms also rely on it) and pinned end-to-end. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(php): remove nonexistent optional_parameter node type tree-sitter-php has no 'optional_parameter' — defaults ride on simple_parameter — so the entry was dead weight the #1920 literal gate does not cover for capture-option Sets (review finding 19, #2522). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(cobol): detect procedure pointers on fixed-format sources Two stacked defects made the feature a no-op on classic sequence-numbered fixed format (review finding 20/H3, #2522): 1. parseDataItemClauses' USAGE alternation knew POINTER but not PROCEDURE-POINTER/FUNCTION-POINTER, so the dataItems filter was dead. 2. The raw-line fallback scanned UNCLEANED text, where the sequence number satisfied the leading digits and the LEVEL NUMBER got captured as the pointer name. It now scans preprocessed lines and requires a letter- initial name (COBOL data names must contain a letter). 161 COBOL preprocessor/copy-expander tests stay green; free-format matrix case unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(cobol): skip comment lines in SET seed/copy scans A commented-out SET (indicator-column '*'/'/' or free-format '*>') produced a live seed and a false CALLS edge from dead code (review finding 21/M1, #2522). The scan now skips indicator-column comment lines and strips inline '*>' tails before matching. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(architecture): document callable-flow-only mode and skipped-key reporting The Callable-value flow section omitted scopeResolutionEdgeMode: 'callable-flow-only' — a real emit-pipeline branch that suppresses all ordinary emission for standalone providers (review finding 22, #2522) — and predated the skipped-key names/stats surfacing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(scope-resolution): correct value-ref resolution attribution and stale pdg-gating comments The value-ref contract comment claimed MethodRegistry resolution — the mechanism is the post-finalize findCallableBindingInScope walker owned by emitPropertyDispatchCalls (resolveReferenceSites skips these sites). Three 'only under --pdg' calleeIdSink comments were falsified by the #2437 gating change (callee-id-sink.ts's header was updated; these copies were missed). Review finding 23, #2522. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(ingestion): direct unit coverage for synthesizeCallableFlowCaptures The 1,100-line shared synthesizer had no test naming it — only downstream consumers were covered (review finding 24, #2522). Pins seed/invoke/ formal/argument emission, subscript container binding, store-gated member invokes, produced-value guards, and the bareNamesAreCalls knob over a minimal options object so assertions target the synthesizer's own semantics. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(resolvers): deepen shallow-language coverage; fix Kotlin/Swift reassignment gaps it exposed Adds the COBOL SET x TO y copy-branch scenario and conditional-assignment scenarios for Kotlin, C#, Swift, and Dart (10 languages previously had one generic case each — review finding 25, #2522). The new scenarios exposed two real capture gaps, fixed here: - tree-sitter-kotlin's 'assignment' node is fieldless, so nested reassignments (chosen = ::target inside a block) produced no flow facts; Kotlin's extractAssignment now decomposes it positionally. - tree-sitter-swift fields its assignment as target:/result:, neither in the shared fallback's field lists; both added. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(infra): literal-validation gate for callable-capture option Sets The #1920 gate validates query literals and exported configs but not the module-private *_CALLABLE_CAPTURE_OPTIONS Sets consumed by the shared synthesizer — a typo'd node type silently captures nothing (PHP shipped a dead 'optional_parameter'; review finding 26, #2522). Every <key>NodeTypes Set literal is now validated against its language's grammar; name-carrying sets (callableProtocolMethods, memberPointerOperators) are deliberately outside the contract. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(storage): centralize corrupt-fixture casts into makeStoreEntry The callable-flow store tests scattered 'as unknown as' double-casts per fixture (review finding 27, #2522; standing no-as-any rule). One typed helper now owns the single controlled escape hatch for building malformed serialization-boundary payloads. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(bench): refresh capture fingerprints after review fixes python-scope: the committed baseline (8d5c3699) never matched this branch's code — CI's benchmarks arm was red on the PR head (review finding 2/HIGH, #2522); regenerated (a99e69ab), scaling 1.04 in budget. scope-capture: ruby/cpp/swift/java/kotlin drifted from the review-fix commits (bare-name suppression, passing modes + ->* recovery, assignment fields, protocol narrowing, positional assignment); all 14 languages re-verified PASS with ratios <= 1.18 against the 1.5 budget. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(docs): untrack docs/plans working documents docs/ is gitignored (local working docs); the plan files were force-added past the ignore. Untracked from the index only — they stay on disk. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(golden): regenerate captures goldens after callable-flow review fixes The per-language digest guards (csharp/go/php/python/ruby/rust/swift) locked the pre-fix capture output; the review-fix series intentionally changed it — store-gated member invokes, subscript container binding, Ruby bare-name suppression, Swift assignment fields, positional pairing. Regenerated with UPDATE_GOLDEN=1; clean verification run 59/59; all other parity/golden guards (pipeline-graph, spring-route, python parity) pass untouched at 33/33. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ingestion): prototypes are callees, not callable value cells The cross-function visibility fix indexed EVERY signature-bearing declaration as a value binding — including plain function/method prototypes (void f(int);). Every call to a declared function then became an indirect invoke, and with emitCanonicalInvokeReference (C/C++) minted a free-call reference that resolved through the registry, bypassing the precise passes' two-phase/ambiguity/subobject suppression — eight phantom CALLS edges in the cpp resolver suite on CI. Only declarations whose binding identifier sits under a pointer/ parenthesized declarator (callable-typed variables like void (*fp)(int);) create value cells now. cpp resolver suite 331/331; callable-value-flow + C/C++ suites 181/181 (the cross-function fp regression still passes); cpp fingerprint rebaselined, both bench gates PASS. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-07-17 17:20:02 +01:00
Gergo Magyar	b685ba4cc8	test(communities): rebaseline pipeline-pdg goldens for projection order ... The C#, Java and Go flag-off digests shift with the canonical community projection from #2478 stacked on the walker sort from #2482. Regenerated via vitest -u; mini-repo pipeline golden was already correct. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>	2026-07-16 08:55:09 +00:00
Gergő Magyar	78b4077d8a	feat(impact): opt-in PDG-backed impact mode - statement + inter-procedural slicing, resolved-callee-id soundness, mutation-oracle validated (#2227)	2026-06-20 12:04:32 +01:00
Gergő Magyar	3c82361b66	perf(cfg): streaming/chunked PDG graph emit for full-kernel-scale repos (#2202) (#2216)	2026-06-16 05:04:10 +01:00
Gergő Magyar	6932e7a9fd	feat(cfg): PDG/CFG visitors for all supported languages (#2195) (#2197) ... * test(cfg): validate cfg/visitors literals + drop 3 dead TS node types Extend the grammar-literal CI gate (test/helpers/literal-collectors.ts) to scan cfg/visitors/*.ts, mapping each visitor file to its grammar via the existing basename rule (c-cpp -> C/C++, csharp -> C#, java -> Java, go -> Go, typescript -> TS). Closes the gap where the gate never validated CFG visitor node-type literals -- the prerequisite for adding C-family visitors safely (#2195 U1). The newly-scanned TS visitor surfaced 3 dead literals absent from every grammar it serves (typescript/javascript/tsx all = 0): for_of_statement (for-of parses as for_in_statement), async_function_declaration and async_arrow_function (async functions are function_declaration / arrow_function + an async child). Removed them; behavior-preserving -- the cases never matched, bench --check fingerprints unchanged, TS visitor unit tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(cfg): language-agnostic CFG unit-test harness (#2195 U1) Extract the grammar-agnostic engine from ts-cfg-harness into makeCfgHarness(grammar, visitor, filePath) at test/helpers/cfg-harness.ts. Function discovery delegates to visitor.isFunction, so the harness carries no language-specific node-type knowledge -- each C-family visitor's unit tests can drive the real worker-side builder against real source. ts-cfg-harness becomes a thin TS binding re-exporting the same parse/collectFunctions/cfgOf/cfgsOf (behavior-preserving: all 5 existing consumers -- taint propagate/model-match/summary-harvest/taint-emit + cfg harvest -- pass unchanged, 223 tests green). New harness.test.ts proves TS-faithfulness and isFunction-delegation via a stub visitor. The bench parameterization (measure.mjs) is sequenced into U7, where the first C-family scaling scenario makes the {grammar, visitorFactory} seam validatable against a real non-TS language. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(cfg): C and C++ CFG visitor + def/use harvest (#2195 U2) Add createCCfgVisitor/createCppCfgVisitor over a shared CCfgWalk core. Grammar introspection confirmed tree-sitter-c and tree-sitter-cpp share every control-flow node type/field, so CppCfgWalk extends CCfgWalk with only the C++-only nodes (try/catch/throw/for_range_loop/lambda) via a visitExtra hook -- no language conditionals (AGENTS no-language-naming). Wire both into c-cpp.ts providers. Harvest (c-cpp-harvest.ts): two-phase binding table + per-statement defs/uses/mayDefs (no sites[] yet -- U6). Edge kinds match the TS contract; functionStartColumn populated; non-terminating loops (for(;;), while(1)) emit the structural exit-escape edge so EXIT stays reverse-reachable and CDG is not silently skipped -- verified against the production post-dominator + control-dependence solvers (for(;;) -> 3 CDG edges). buildFunctionCfg returns undefined rather than throwing. 23 real-parser regression tests; grammar-literal gate green (literals validated against both grammars). Documented gaps: C++ RAII destructors, setjmp/longjmp, computed goto (route to EXIT + warn). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(cfg): C# CFG visitor + def/use harvest (#2195 U3) Add createCsharpCfgVisitor + csharp-harvest over the shared CfgBuilder / ControlFlowContext, modeling the C# statement taxonomy: if/else, for/foreach/while/do, switch_section (+ switch_expression arms), try/catch/catch_filter/finally, using + lock (deterministic finalizers -- dispose/release runs on normal AND exception exit, finally-* completion edges on crossing jumps), goto/labeled, yield (surface only), return/ throw/break/continue. Wire into csharpProvider. Every literal validated against tree-sitter-c-sharp via the introspection probe (record_declaration, no else_clause, switch_section, positional access where no field exists). Edge kinds match the contract; functionStartColumn populated; while(true) keeps EXIT reverse-reachable (production CDG probe: 3 edges). buildFunctionCfg returns undefined rather than throwing. 34 real-parser regression tests; grammar-literal gate green; no regression (cfg unit dir 256/256, tsc clean). Documented gaps: yield iterator state machine, goto case/default, async suspension points. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(cfg): Java CFG visitor + def/use harvest (#2195 U4) Add createJavaCfgVisitor + java-harvest over the shared CfgBuilder / ControlFlowContext: if/else, classic for, enhanced-for, while, do-while, classic-vs-arrow switch (switch_block_statement_group fallthrough vs switch_rule no-fallthrough), try/catch/finally + try-with-resources (auto-close synthesized as a finalizer, closes on normal AND exception exit) + synchronized (monitor-release finalizer), labeled break/continue to the labeled frame, yield, return/throw/break/continue. Wire into javaProvider. Every literal validated against tree-sitter-java via the probe (switch_expression covers both switch forms, generic_type, line_comment, for init field). Edge kinds match the contract; functionStartColumn populated; while(true)/for(;;) keep EXIT reverse-reachable (production CDG probe: 3 edges; hazard fixture: 34 CDG edges). buildFunctionCfg returns undefined rather than throwing. 43 real-parser regression tests; grammar-literal gate green; no regression (cfg unit suite 304, tsc clean). Documented gaps: switch-as- expression-value inline, yield state machine, async/field-write defs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(cfg): Go CFG visitor + def/use harvest (#2195 U5) Add createGoCfgVisitor + go-harvest, the highest-divergence target: for_statement (all four shapes -- for_clause C-style, while-style, range_clause, bare for{}), expression/type switch (no implicit fallthrough) + explicit fallthrough_statement, select_statement, defer (LIFO finalizer legs at function exit), go (call is straight-line; the closure body is its own CFG via isFunction), labeled break/continue/goto, multiple-return assigns (a, b := f() defines each LHS). Wire into goProvider. CRITICAL (review A2): every non-terminating shape -- for{}, for cond{}, select{} with no default -- emits a structural exit-escape edge so EXIT stays reverse-reachable and the production CDG is not silently skipped. Verified: for{} -> CDG=3, select{} -> CDG=1, for-range -> CDG=2, all exitReachable=true. Every literal validated against tree-sitter-go via the probe. 32 real-parser regression tests; grammar-literal gate green; no regression (186 across all 5 visitors + gate, full cfg unit 331, tsc clean). Documented gaps: panic/recover unwind, goroutine happens-before. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(cfg): call-site sites[] taint substrate for C-family (#2195 U6) Extend the C/C++/C#/Java/Go harvests with the call-site sites[] taint substrate (SiteRecord/SiteArgOccurrence), mirroring the TS shape so the shared taint matcher consumes all languages uniformly. Extract the grammar-agnostic site machinery into cfg/visitors/call-site-harvest.ts (CallSiteFactAccumulator -- names no language); each harvest adds only its per-grammar visitCall/walkChain over its call node (C/C++ call_expression, C# invocation_expression, Java method_invocation, Go call_expression). INERT BY DESIGN: no C-family taint model exists (registerBuiltinTaintModels is TS/JS only), so getSourceSinkConfig returns undefined for these languages and the harvested sites produce ZERO TAINTED edges -- the positive source->sink->TAINTED path is deferred with the model authoring. sites emitted only when non-empty; facts-only attachment, block/edge topology unchanged (pre-existing topology + def/use tests byte-identical). 23 new substrate tests; 574 green across the cfg/taint/emit suites; gate green; tsc clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(cfg): worker-mode PDG integration + bench parameterization (#2195 U7) Prove the five C-family visitors build PDG through the REAL worker pipeline. pipeline-pdg.test.ts: per-language (C/C++/C#/Java/Go) temp repo run with pdg:true asserts BasicBlock+CFG+REACHING_DEF+CDG all > 0 (CDG>0 proves EXIT stays reverse-reachable end-to-end through the worker, incl. each fixture's non-terminating loop/select); a paired run with pdg off asserts == 0, the two flag-off graphs byte-identical (R3), no PDG types leak, pinned by a golden snapshot. Counts e.g. Go 151 BB / 56 CDG. Parameterize bench/cfg/measure.mjs by a per-language LANGS registry resolved generically via getLanguageGrammar + getProvider(X).cfgVisitor (no static import table). Default TS byte-identical -- all 6 TS fingerprints unchanged under --check; taint-dense stays TS-only (TS_JS_TAINT_MODEL never runs against model-less C-family CFGs). Add a go:branchy scenario+baseline (namespaced) -- its fingerprint shape (32 blocks/46 edges) matches TS branchy, cross-validating the Go visitor. 15 pipeline tests + bench --check PASS (7 scenarios); 354 unit cfg green; dist rebuilt clean. Absorbs the bench parameterization deferred from U1. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(cfg): Python CFG visitor + def/use harvest (#2195 U8) Add createPythonCfgVisitor + python-harvest -- the most structurally divergent target (indentation blocks, elif, for/while-else, with, try/ except/except-group/else/finally, match/case, comprehensions, walrus), confirming the shared CfgBuilder/ControlFlowContext core carries no brace-family assumptions. for/while else-clause sits on the normal- completion edge (not break); with modeled as try/finally dispose; match has no fallthrough. Wire into pythonProvider. Every literal validated against tree-sitter-python via the probe. while True: keeps EXIT reverse-reachable (production CDG probe: 3 edges; fixture: 42 CDG edges). 37 real-parser tests; gate green; no regression (cfg unit 391, tsc clean). Gaps: async/generator suspension, comprehension scope over-approximation. No sites[] (taint substrate, separate). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(cfg): PHP CFG visitor + def/use harvest (#2195 U9) Add createPhpCfgVisitor + php-harvest: if/elseif/else (+ alt colon syntax), for/foreach/while/do-while, switch (fallthrough) + match (no fallthrough), try/catch/finally, break N/continue N (N-th enclosing loop), goto, return/throw. Wire into phpProvider. Every literal validated against tree-sitter-php (php_only) via the probe (for_statement initialize/condition/update; throw_expression not throw_statement; break/continue integer child). while(true) keeps EXIT reverse-reachable (production CDG probe: 3 edges; break 2 escapes the outer loop). 35 real-parser tests. Also repoint worker-roundtrip's "non-CFG language" gate test from Python (which now has a cfgVisitor) to COBOL (the permanent non-goal of the rollout) -- a stale assertion the Python commit invalidated. Full in-process sweep green (452 across 18 files). Gaps: match inline value, goto plain-block. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(cfg): Ruby CFG visitor + def/use harvest (#2195 U10) Add createRubyCfgVisitor + ruby-harvest: if/unless/elsif/else + statement-modifier forms (x if c, x while c), while/until/for (until inverts the sense), case/when + case/in (pattern, no fallthrough), begin/rescue/else/ensure (ensure=finally, rescue=catch) + retry (loop-back into begin), return/break/next/redo, blocks/lambdas as their own closure CFGs. Wire into rubyProvider. Every literal validated against tree-sitter-ruby via the probe (case vs case_match, modifier nodes, typed rescue/ensure children). loop do / while true keep EXIT reverse-reachable (production CDG probe: 3 edges). 34 real-parser tests; comprehensive sweep green (486). Gaps: yield, expression-position if/case/begin inline, ivar/gvar non-local defs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(cfg): Rust CFG visitor + def/use harvest (#2195 U11) Add createRustCfgVisitor + rust-harvest for the expression-oriented Rust: if/else + if-let, loop (infinite -- structural escape edge), while/ while-let/for, match (no fallthrough) + guards, labeled break/continue ('outer), break-with-value, ? operator (try_expression) as an early-return throw edge to EXIT, let-else (diverging else). visitLet handles control-flow in value position (let x = loop/if/match). Wire into rustProvider. Every literal validated against tree-sitter-rust via the probe (label is a named child not a field; line_comment; _ pattern). loop {} keeps EXIT reverse-reachable (production CDG probe: 3 edges). 33 real-parser tests; comprehensive sweep green (519). Gaps: panic, async/.await, macro bodies. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(cfg): Swift CFG visitor + def/use harvest (#2195 U12) Add createSwiftCfgVisitor + swift-harvest (vendored tree-sitter-swift via requireVendoredGrammar): if/else + optional binding (if let), guard...else (diverging early exit), for-in/while/repeat-while (bottom-test), switch (no implicit fallthrough; explicit fallthrough keyword; where guards), do/catch + try/try?/try!, defer (LIFO finalizer at scope exit), labeled break/continue, control_transfer_statement (one node for break/continue/ return/throw). Wire into swiftProvider. Every literal validated against the vendored grammar via the probe (no block node; if-let folds into condition+bound_identifier; defer parses as a call_expression with trailing closure). while true keeps EXIT reverse-reachable (production CDG probe: 3 edges). 24 real-parser tests; comprehensive sweep green (543). Gaps: computed properties, defer block-scope approx, fatalError traps. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(cfg): Kotlin CFG visitor + def/use harvest (#2195 U13) Add createKotlinCfgVisitor + kotlin-harvest (vendored tree-sitter-kotlin): if/else, when (subject + subjectless, no fallthrough), for/while/do-while, try/catch/finally, jump_expression (return/return@/break/break@/continue/ continue@/throw), labeled loops, control_structure_body unwrapping, expression-body functions. The grammar is field-less for control flow, so the visitor navigates by child type+position. Wire into kotlinProvider. Every literal validated against the vendored grammar via the probe (line_comment/multiline_comment, not comment). while (true) keeps EXIT reverse-reachable (production CDG probe: 3 edges; worker-mode fixture: BB=82, CDG=41). 28 real-parser tests; comprehensive sweep green (571). Gaps: value-position if/when/try inline, inline-fun non-local return, getters/setters. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(cfg): Dart CFG visitor + def/use harvest (#2195 U14) Add createDartCfgVisitor + dart-harvest (vendored tree-sitter-dart): if/else, C-for/for-in/while/do-while, switch (empty-case fallthrough + explicit continue-label) + switch_expression, try/on/catch/finally + rethrow + assert (throw edges), return/break/continue/throw, labeled loops, arrow bodies, closures. Dart splits a function into sibling signature + function_body nodes, so the body (or function_expression) is the CFG-bearing node. Wire into dartProvider. Every literal validated against the vendored grammar via the probe (only constant_pattern exists; removed speculative relational/logical pattern names). while (true) keeps EXIT reverse-reachable (production CDG probe: 3 edges). 34 real-parser tests; comprehensive sweep green (605). Gaps: labeled-loop grammar quirk (read via ERROR sibling), async straight-line, value-position if/switch inline. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(cfg): Vue (reuse TS visitor) + worker-mode proof for all langs (#2195 U15) Vue SFC <script> blocks are extracted and parsed with the TS grammar (parse-worker languageMap[Vue] = TypeScript.typescript), so wire vueProvider.cfgVisitor = createTypeScriptCfgVisitor() -- pure reuse, no Vue-specific visitor. vue-visitor.test.ts replicates the worker path (extractVueScript -> TS parse -> CFG) and confirms branch edges + EXIT reverse-reachable + CDG>0. Extend pipeline-pdg.test.ts with a worker-mode block covering all eight remaining languages (Python/PHP/Ruby/Rust/Swift/Kotlin/Dart/Vue): per- language temp repo, real worker pool, BasicBlock+CFG+REACHING_DEF+CDG all > 0 with --pdg (CDG>0 proves EXIT reverse-reachable end-to-end through the worker despite each fixture's non-terminating loop), == 0 without. Counts e.g. Ruby 122 BB/45 CDG, Vue 49 BB/11 CDG. 30 pipeline tests green. COBOL: documented as the deliberate PDG non-goal (no grammar, exotic PERFORM/GO-TO control flow) in cobol.ts + the worker-roundtrip gate. This completes PDG language coverage: every supported language except COBOL now builds CFG/REACHING_DEF/CDG under --pdg. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): surface skippedUnsoundFunctions in per-language stats (#2195 U2) emitFileCdg computes skippedUnsoundFunctions (functions whose CDG is withheld because EXIT isn't reverse-reachable from all blocks) but run.ts dropped it on the floor — only cdgEdges/cdgDropped were aggregated. Add the aggregation + a stats-line segment so CDG coverage gaps are an explicit signal, not silent. Establishes the baseline skip count that makes the U1 synthetic-escape pass's effect (the drop to genuine anomalies only) measurable. Additive; no emit-logic change. The emit-side field is covered by cfg-emit.test.ts (asserts skippedUnsoundFunctions===1 + the warn on a disconnected-block CFG); the run.ts aggregation is a thin pass-through. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): synthetic-escape pass restores CDG for exit-unreachable cycles (#2195 U1) Unconditional goto-cycles (C/C++/C#/Go) wire a backward seq edge with no structural exit-escape edge, so EXIT becomes non-reverse-reachable and emitFileCdg silently skipped ALL control-dependence for the function. New cfg/synthetic-escape.ts: a pure deterministic SCC routine (iterative Tarjan, sorted adjacency) + augmentForPostDom(cfg). No-op when EXIT is already reverse-reachable (terminating fns + visitor-escaped loops are byte-identical — returns the same object). Otherwise it batch-bridges every exit-less SCC by adding an ANALYSIS-ONLY escape edge from the SCC's controlling block (highest out-degree branch; lowest-index tie-break) to EXIT, on a shallow-cloned FunctionCfg — never mutating persisted cfg.edges. emitFileCdg threads that augmented view through BOTH isExitReachableFromAllBlocks AND computeControlDependence (the Ferrante walk re-reads cfg.edges, so a tree-only augmentation would be wrong). Precision (anti-masking): only a trapped region containing a control point (>=2-successor block) is bridged — a branch-less trapped region carries no recoverable control-dependence and is indistinguishable from a genuine construction anomaly, so it stays on the skip path (the existing disconnected-block skip test still skips, skippedUnsoundFunctions===1). A residual non-cycle dangling block is never bridged. repro `void handler(int a){ start: if(a>0){work();} goto start; }`: before exitReachable=false/CDG=0 → after one synthetic 2->1 edge, exitReachable=true, exact CDG = {2->2:T,2->2:F,2->3:T,2->4:T,2->4:F} (pinned exactly, not CDG>0 — catches a wrong representative). AC2 property test extended to the augmented graph; per-language goto-cycle regressions (C/C++/C#/Go). 199 cfg tests green; bench --check fingerprints unchanged (analysis-only, zero persisted drift). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(cfg): isolate the non-terminating-loop hazard in worker CDG asserts (#2195 U3) The pipeline-pdg worker-mode blocks asserted a whole-fixture cdg>0 aggregate (satisfied by any branching fn) while the comment claimed it proved the non-terminating-loop EXIT-reachability end-to-end. Add a per- language `hazard` marker + isolate the assertion: locate the hazard function's BasicBlocks by its anchor and assert >=1 CDG edge is sourced within it (a marker mutation now fails the test — non-vacuous). C# keeps the aggregate (its fixture has no infinite loop). Comments corrected. Switch the 7 visitor unit tests (java/csharp/dart/kotlin/php/swift/c-cpp) from the local exitReachableFromAll CFG-shape helper to the production isExitReachableFromAllBlocks + computeControlDependence on the hazard function, matching go/python/ruby/rust/vue. 241 unit + 30 pipeline green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(cfg): gate vendored-grammar worker assertions on isLanguageAvailable (#2195 U4) The Swift/Kotlin/Dart worker-mode pipeline-pdg cases require a vendored grammar prebuild that may be absent on a CI platform — they'd go red there. Mark those three REMAINING_LANGS entries `vendored` and gate both the --pdg-on and --pdg-off `it`s on isLanguageAvailable(SupportedLanguages [lang]) → it.skip when the grammar can't load. Installed-grammar languages stay unconditional. Grammars present here, so all 30 run green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(cfg): remove dead useCount() from swift + rust harvests (#2195 U5) useCount() was declared on the local FactAccumulator in swift-harvest.ts and rust-harvest.ts but never called (a copy-paste artifact; ruby's copy IS used in an emit guard, so it stays). Pure deletion — the swift/rust visitor suites stay green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(cfg): standardize harvester API table()->bindingTable() (#2195 U9) The binding-table accessor was named table() in the C/C++/C#/Go harvests but bindingTable() in the other 7. Rename the 4 (definitions + their visitor call sites) to the majority name bindingTable(). Pure rename; the 4 visitor suites stay green and tsc confirms no call site was missed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(cfg): consolidate scope-tree substrate into ScopeTreeHarvester (#2195 U6) The Go/Java/C#/C-C++ def/use harvesters each carried a byte-identical copy of the lexical scope-tree machinery (Scope record, two-phase resolution cache, openScope/nearestScopeOf/resolve/def/use/conditional/bindingTable, ~270 lines total). Extract it into an abstract ScopeTreeHarvester base; the four harvesters now extend it and supply only their genuine per-language variation (the prescan switch, plus Go's _-blank-identifier overrides of declare/def/use). Net -422 lines. Mechanical and byte-equivalent: cfg unit suite 613 passed, bench --check fingerprints unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(cfg): consolidate no-site def/use accumulator into DefUseAccumulator (#2195 U7) The Kotlin/Python/Ruby/Rust/Dart/Swift harvesters each carried a byte-identical copy of the no-site def/use accumulator (~270 lines total; only Ruby's adds the live useCount() emit-guard helper). Extract it as an exported DefUseAccumulator beside CallSiteFactAccumulator in call-site-harvest.ts (the PR's own model for the with-site superset); the six harvesters import it under their existing local FactAccumulator name. Pure byte-equivalent move, no logic change: cfg unit suite 613 passed, tsc clean, bench --check fingerprints unchanged (TS/Go paths untouched). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(cfg): consolidate copied visitor-test helpers into cfg-harness (#2195 U8) The 13 *-visitor.test.ts files each copied a byte-identical set of CFG-shape helpers (edgeKinds/block/reaches/reachable/bindingIdx/allSites/hasAnySites, ~380 lines total). Export them once from test/helpers/cfg-harness.ts and import per file (only the subset each references). Also drop each file's local exitReachableFromAll — a re-implementation of the production isExitReachableFromAllBlocks (semantically identical: false iff some entry-reachable non-EXIT block can't reach EXIT) — and point its live call sites at the already-imported production function. Pure test-only mechanical move, behavior-preserving: tsc clean, test/unit/cfg/ 613 passed unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(cfg): pin *-harvest.ts literals to their own grammar in the gate (#2195 U10) The grammar-literal validation gate scans cfg/visitors/, but a <lang>-harvest.ts basename was not in BASENAME_LANGS, so fileLanguages() fell it through to the weak ALL_LANGS valid-if-any bucket — a node-type literal dead in its own grammar but valid in some other grammar would pass undetected. Strip the -harvest suffix and reuse the visitor basename map so go-harvest -> Go, c-cpp-harvest -> C+C++, typescript-harvest -> TS, etc. The two language-agnostic harvesters (call-site-harvest, scope-tree-harvest) name no grammar and stay valid-if-any. Also corrects the now-inaccurate mode2Files comment. Adds a fileLanguages unit test; the existing gate stays green (no harvest file has a dead literal), and a scratch probe confirmed a bogus go-harvest literal is now caught. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(cfg): defensive per-statement cap on harvested taint sites (#2195 U11) A statement's harvested sites[] had no explicit bound — a pathological or machine-generated statement (hundreds of nested calls) could grow it without limit. Add DEFAULT_PDG_MAX_SITES_PER_STATEMENT (512, mirroring the PDG edge/fact cap style): openCallSite/addMemberRead check-before-push and stop at the cap, keeping the first 512 sites fully intact and setting an observable sitesTruncated flag. A cap-dropped openCallSite returns a -1 sentinel that pushFrame/setSite*/the occurrence fan-out all tolerate (no dangling parent/via, no clobber of kept sites). Generous enough that no real statement is affected: bench --check fingerprints unchanged, cfg unit suite 617 passed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci(codeql): exclude nested test fixtures from the CodeQL gate (#2195) The CodeQL results gate failed on test/integration/cfg/fixtures/python-hazards.py ('total' may be used before init, unused vars) — but that file is an intentional CFG/PDG hazard fixture, exactly the synthetic broken-code the existing '**/test/fixtures/**' exclusion is meant to skip. That glob does not match the deeper test/integration/cfg/fixtures/ path, so the hazard fixtures leaked into the scan. Add '**/test/**/fixtures/**' to cover fixtures nested anywhere under a test tree. Analyze (python) and Analyze (javascript-typescript) both already pass — production code is clean; this only silences fixture noise. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * style(cfg): apply prettier + drop unused imports across the PDG files (#2195) The merge of main into this branch pulled in the stricter quality gates (prettier --check . and eslint .), which surfaced pre-existing formatting in the PDG/CFG rollout (line-width wrapping across the visitor + harvest files, bench, tests) plus 9 no-unused-imports errors. Mechanical autofix only — npm run format + lint:fix equivalent, scoped to gitnexus/: removes unused FunctionCfg/ SiteRecord type imports left by the U8 helper consolidation and stale FinalizerFrame imports in python.ts/ruby.ts. No behavior change: tsc clean, cfg unit suite 617 passed, eslint 0 errors. (gitnexus-web class-order noise is a local tailwind-plugin artifact CI does not flag — left untouched.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): harvest C++ structured-binding defs (auto [a,b]=e) (#2195) The C++ def/use harvester only recorded a def when an init_declarator's declarator was a plain identifier, so a structured binding (auto [a,b] = mk(), incl. the auto& reference form whose binding sits under a reference_declarator) declared only the first name in phase 1 and emitted ZERO defs in phase 2 — a,b were walked as spurious uses and later use(a)/use(b) resolved to a synthetic module binding, silently corrupting REACHING_DEF/taint for an idiomatic C++17 shape. Unwrap the structured_binding_declarator in both phases and def every identifier leaf; result-of-initializer flows to the whole list. Inert for C (no structured bindings). Characterization tests added (plain + reference form). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): model C++ co_return as a return terminator to EXIT (#2195) co_return_statement was neither in CPP_CONTROL_FLOW_TYPES nor dispatched, so a coroutine's co_return coalesced into a straight-line block and emitted a spurious seq fallthrough to the following statement instead of an edge to EXIT — statements after co_return looked reachable and the terminator edge was missing, corrupting CFG/CDG for coroutines. Add the node type to the C++ control-flow set and dispatch it through visitReturn (block -> EXIT 'return', no fallthrough). C path untouched; co_await/co_yield remain plain expressions. Characterization test added; c-cpp suite + grammar-literal gate green, bench --check unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): harvest C# out-var and deconstruction-declaration defs (#2195) Two idiomatic C# write shapes recorded ZERO defs, silently breaking REACHING_DEF/taint: - out-var (G(out var n) / G(out int n)) parses as a declaration_expression; it was neither declared (phase 1) nor def'd (phase 2), so n resolved to a synthetic module binding and the callee-written value had no reaching def. - deconstruction declaration (var (a, b) = T()) has a variable_declarator whose name slot is a tuple_pattern (null name field), so declareVariableDeclaration + the variable_declaration walk skipped it entirely (only the assignment form (a,b)=T() was handled). Both a and b were dropped. Declare + def the declaration_expression's identifier (must-def: out params are definitely-assigned), and route a null-name variable_declarator through the tuple_pattern via the existing declareForeachTarget/defTupleTargets helpers. Characterization tests added; csharp suite 42 passed, grammar gate + bench --check green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): put embedded-script CFGs in file coordinates via lineOffset (#2195) A Vue SFC <script> block parses at row 0 but lives at lineOffset in the .vue file. Every other worker-emitted graph node adds lineOffset to reach file coordinates, but collectFunctionCfgs built FunctionCfgs from the extracted script's raw rows and never offset them. Two consequences for .vue files: - inter-procedural taint silently resolved NOTHING — the summary-harvest join keys graph Function/Method nodes by their (offset) startLine but looked up the CFG's (unoffset) functionStartLine, missing by exactly lineOffset, so no FunctionSummary was ever produced; - persisted BasicBlock startLine/endLine (and the id's functionStartLine segment) pointed at the wrong .vue line, breaking source mapping. Thread lineOffset into collectFunctionCfgs and shift every CFG source-line field (functionStartLine/End, block start/end, statement + non-synthetic binding lines) into file coordinates at the one production chokepoint. A 0 offset returns the CFG unchanged, so .ts/.js/etc. stay byte-identical (bench --check fingerprints unchanged; worker-roundtrip + pipeline-pdg green). Unit tests for the shift + the 0-offset no-op added. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(cfg): surface CDG soundness skips at warn, not just debug (#2195) skippedUnsoundFunctions (a function whose EXIT is not reverse-reachable from all blocks, so control dependence is withheld) was only reported inside the per-language logger.debug stats line — while the taint/RD coverage-gap and cap-drop counts surface unconditionally at warn. A language that systematically trapped EXIT (an unmodeled non-terminating / multi-terminal shape the synthetic-escape pass can't bridge) would silently lose all CDG. Add a parallel unconditional warn (R8) alongside the R4 taint-gap warn. Observability only — no graph change; emit-layer skip counting stays covered by cfg-emit's skippedUnsoundFunctions test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): harvest Kotlin x++/--x as a def (#2195) The Kotlin harvester had no postfix_expression/prefix_expression case, so an increment/decrement fell to the default descent and recorded its operand as a use only — never a def. Every sibling harvester (Java/C#/C++/Dart/TS/PHP) models inc/dec, so a Kotlin counting loop (while/for using i++) silently dropped the loop-carried reaching-def of the counter. Add the case: def AND use the operand when it is a plain simple_identifier and the operator is ++/-- (other pre/postfix forms — -x, !x, x!!, x? — stay pure reads, byte-identical to the old descent). Characterization tests for postfix + prefix added; kotlin suite 30 passed, grammar gate + bench --check green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): harvest Go select channel-receive binding as a def (#2195) walkValue had no receive_statement case, so a select receive (case v := <-ch:) fell to the default descent: v was recorded as a USE of an uninitialized var and the channel-sourced definition was invisible to REACHING_DEF/taint — channels are a primary taint source in Go. Add the case mirroring short_var_declaration: def each left identifier, use the <-ch right, attach resultDefs for the := short form. prescan already declared the binding; this completes the phase-2 fact. go:branchy bench fingerprint unchanged; go suite 40 passed, grammar gate green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): harvest all names of a Dart multi-variable declaration (#2195) `var a = 1, b = 2;` is one initialized_variable_definition whose first binding is the name/value field pair and whose subsequent bindings are trailing initialized_identifier children. Both prescan (declareInitializedVar) and the walkValue case read only the name/value fields, so every name after the first was never declared or def'd — `b` resolved to a synthetic module binding and its REACHING_DEF/taint flow was lost. Iterate the trailing initialized_identifier nodes in both phases. (Dart-3 record/list pattern declarations `var (a,b)=pair` remain a separate follow-up.) dart suite 35 passed, tsc + grammar gate green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): bind Swift switch-case value patterns (case let n) (#2195) A switch value-binding (case let n where …, case .some(let v)) was never declared in prescan, so n/v resolved to a synthetic module binding and a body use(n) did not link to any def — a very common Swift idiom silently lost its data dependence. Declare the switch_pattern's bindings (prescan, reusing declarePattern) and emit them as MAY-defs on the dispatch block (a case may not match) via a new switchPatternFacts, propagated into the case body. swift suite 25 passed, tsc + grammar gate green. (The rare ?? / ternary-arm may-def — Swift assignment-as-expression — remains a separate follow-up.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): wire Swift multi-catch throw edges to every handler (#2195) visitDo routed the protected body's throw edge only to handlerEntries[0], so a do { try r() } catch A {} catch {} left the 2nd..Nth catch handlers UNREACHABLE from ENTRY — orphaned blocks whose error bindings + def/use facts were stranded in a dead component (a soundness gap for idiomatic Swift typed multi-catch). Swift tries the catch clauses in order and the thrown type is unknown at CFG time, so every protected block may reach ANY clause: edge each protected block to every handlerEntry. Found by the per-language CFG/CDG verification swarm (reproduced: 2-catch=1, 3-catch=3 unreachable blocks). swift suite 26 passed, tsc clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): synthesize a protected block for an empty Kotlin try {} (#2195) An empty `try {}` body produced zero protected blocks, so visitTry's throw-edge loop wired nothing to the catch and the try's entry fell through to the finally — leaving the catch handler block + its error binding orphaned (unreachable from ENTRY), a malformed CFG with stranded def/use facts. Mirror the existing empty-`catch` synthesis: when the try body is empty and there is a catch or finally, synthesize one protected block so the catch handler(s) are wired and the try entry is the body, not the finally. Found by the per-language CFG verification swarm. Non-empty try is byte-identical; kotlin suite 31 passed, tsc clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(cfg): bound the reaching-defs fixpoint with a per-block visit ceiling (#2195) The per-language verification swarm reproduced, AT PRODUCTION DEFAULTS, a reaching-defs blow-up: a machine-generated ~2000-line all-loops function (under DEFAULT_PDG_MAX_FUNCTION_LINES) reaches ~10k basic blocks because loops emit ~5 blocks/line, and the dataflow fixpoint is O(blocks^2.3) on deep loop nests — measured 62s (C/C++) and 2.05s + 810MB (Go) for ONE function. maxFacts does not help: the fact count stays LINEAR, so it never fires. Iterative reaching-defs on a reducible CFG converges in O(loop-nesting-depth) passes, so a worklist re-visits each block a small multiple of times for real code. Add a maxBlockVisits ceiling (emit passes blocks.length × 64 — far beyond any hand-written nesting depth, ~15) that bails when the fixpoint has not converged. An unconverged fixpoint's in/out sets are not sound, so it returns NO facts (status 'truncated', like the existing 'overflow' guard) — a per- function coverage gap, never wrong facts. Real code is byte-identical: full cfg suites 725 passed, bench --check fingerprints unchanged. NOTE: computeControlDependence's O(N²) up-walk on deep post-dom chains is the sibling concern but stays ~13ms in production (bounded by the line cap + the CDG materialization cap); a CDG work-budget is a documented follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): raise the parse-worker stack limit for deep CFG recursion (#2195) The CFG visitors build per-function control-flow graphs by recursive descent over the tree-sitter AST, so deeply-nested source overflows the worker thread's call stack (~1.5k nesting levels) — caught per-function (R4 try/catch) but the function silently gets no PDG. A worker thread's stack is governed by resourceLimits.stackSizeMb (Node default 4 MB); the main process's --stack-size=4096 flag does NOT propagate to worker threads (confirmed by prior- art research on Node worker_threads). Raise it to 16 MB, pushing the overflow threshold to several-thousand nesting levels — far beyond any hand-written code, so only machine-generated/obfuscated nesting can still hit it (and that stays a caught per-function skip, never a crash). Complements a future proactive depth guard. pipeline-pdg worker tests 30 passed, tsc clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(cfg): compute control dependence as a reverse-CFG dominance frontier (#2195) The Ferrante §3.1.1 up-walk re-climbed the ipdom chain once per CFG edge, which is Θ(N²) on a deep post-dom chain (a single branch fanning into a shared spine took ~7.1s at 16k blocks). Replace it with the reverse-CFG post-dominance-frontier formulation (Cytron, Ferrante, Rosen, Wegman & Zadeck 1991): control dependence IS the dominance frontier of the reverse CFG, computed bottom-up over the post-dom tree (PDF_local from a node's CFG in-edges + PDF_up from its post-dom-tree children) in O(N + E + output). LLVM (ReverseIDFCalculator), Joern (CdgPass) and WALA use the same form. Output is the IDENTICAL deduped/sorted (controller, dependent, label) set: verified byte-identical across all cfg unit+integration suites, the cdg-snapshot oracle, and bench --check fingerprints (unchanged). The PDF unions a label SET per (controller, dependent) pair, preserving the multi-label rows the old per-row dedup kept on opposite-sense (goto-cycle) arms. buildArmSenses, labelFor, the final sort and the maxEdges truncation cap are kept verbatim; the post-order walk is iterative so a chain-deep post-dom forest cannot overflow the stack. Adds three regressions: multi-label-per-pair preservation, the literal self-edge / NO_IPDOM seed guard (a !== x), and a fan-into-chain perf tripwire (linear vs the former quadratic up-walk). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(cfg): record the reaching-defs WTO no-go decision (#2195) Weak-topological-order / loop-aware iteration (Bourdoncle 1993) was evaluated as the fix for the O(blocks²) deep-loop-nest blow-up and rejected: a faithful WTO solver was 104/104 byte-identical to the RPO worklist but 0% faster — the cost is inherent dense-set propagation + lattice merges, not visitation order, and the loop-body-skip shortcut is unsound on irreducible (goto) CFGs. Document this at the RPO-order site and the emit.ts revisit-ceiling constant so the shipped blocks×64 bound reads as the sound backstop it is, with SSA-sparse reaching-defs named as the deferred real fix. Comment-only; no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(cfg): proactive visitor nesting-depth guard + observable CFG skips (#2195) The CFG visitors are recursive-descent with no shared base, so a pathologically nested function (machine-generated / adversarial) could overflow the worker's native stack — a nondeterministic RangeError that escaped to the language-group catch and silently dropped EVERY remaining file's CFG. Guard it proactively: CfgBuilder tracks live recursive-descent nesting depth via enterNesting/exitNesting, called at each visitor's visitBody and visitSeq choke points (visitBody covers nested control constructs incl. else-if ladders; visitSeq covers deeply-nested bare blocks). Exceeding MAX_CFG_NESTING_DEPTH (500, far below the ~1.2k+ native limit and far above real code's ≤~50) throws a typed, DETERMINISTIC CfgNestingDepthError instead of waiting for the engine's nondeterministic overflow. collectFunctionCfgs now isolates the build PER FUNCTION: the depth bail or any other throw is caught, counted, and skipped — one bad function no longer loses the whole file's CFGs. CollectedCfgs.skipped widens from a bare number to reason-counted buckets (tooManyLines / tooDeeplyNested / buildError). The worker stops discarding that count (parse-worker.ts), aggregates it per-language onto ParseWorkerResult.cfgSkipped (survives the parse cache via slim's `...result`), and mergeChunkResults merges + warns per-language so a CFG coverage gap is observable, not silent. Behavior-preserving on normal code: the guard never fires below 500 nesting, so the cfg unit+integration suites (731), the CDG/RD/CFG snapshots and the bench --check fingerprints are all byte-identical. The worker stackSizeMb 4→16MB bump shipped earlier (de4c43a4). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(cfg): unify the nesting guard behind CfgBuilder.withNesting (#2195) Tri-review flagged the visitBody/visitSeq guard asymmetry (visitBody used try/finally; visitSeq placed a bare exitNesting before its tail return). It is not a bug today — the CfgBuilder is per-function and discarded on a bail, so a leaked counter is never read — but a future mid-loop return in visitSeq would silently corrupt the depth count. Replace both hand-paired sites in all 12 visitors with a single `CfgBuilder.withNesting(fn)` helper that enters on the way in and exits in a finally, so the pair can never drift. Also document that block-bodied constructs pass through BOTH choke points, so the effective lexical ceiling is ~MAX_CFG_NESTING_DEPTH/2 (~250). Behavior-preserving: 733 cfg tests + bench --check fingerprints byte-identical. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): include cfgSkipped in all parse-worker result initializers (#2195) Tri-review found cfgSkipped omitted from the two reset/fallback ParseWorkerResult initializers (only the main one carried it). The field is optional with `?? {}` reads so there is no runtime bug, but the zero-state initializers should be complete and consistent. Also correct the field's doc-comment: the per-language merge + warn lives in `dispatchChunkParse` (alongside skippedLanguages), not `mergeChunkResults` — and, like that sibling telemetry, the warn fires for freshly-parsed chunks, not on a warm cache hit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(cfg): scope the CDG byte-identical claim to the untruncated output (#2195) Tri-review noted the rewrite's `maxEdges` truncation now trims the SORTED edge set, whereas the old up-walk broke mid-walk in CFG-edge-iteration order — so a truncated PREFIX can differ at the cap boundary (only when a single function exceeds maxEdges; the FULL untruncated set is byte-identical, now also confirmed by ~1M-case differential fuzz). Clarify the module doc and the maxEdges param doc: the cap bounds OUTPUT count (peak working set ≈ output in the DF formulation, not the old pre-dedup spike), and the byte-identical guarantee is scoped to the untruncated output. Comments only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(cfg): strengthen depth-guard, perf-tripwire and buildError coverage (#2195) Tri-review test-quality findings: - cfg-builder: capture the thrown CfgNestingDepthError unconditionally (a catch-only assertion silently passes if a future change stops throwing); add a withNesting test that the counter balances on the THROW path too. - control-dependence perf tripwire: assert controller IDENTITY (every edge controlled by block 0, distinct dependents in range), not just length, so a fast-but-wrong reimplementation can't pass on the M-1 count alone. - worker-roundtrip: add the missing buildError test — a generic (non-depth) buildFunctionCfg throw is caught per function, counted under buildError, and does NOT drop the file's sibling CFGs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): model Kotlin value-position when/if as control dependence (#2205) Idiomatic Kotlin uses `if`/`when` as EXPRESSIONS (`val x = when (k) { … }`, `return if (c) a else b`, `fun f() = when (k) { … }`), but the visitor only modeled them as control flow in statement position (the `isStatementPosition` gate) — value-position branches collapsed into one straight-line block, so their arms emitted no control dependence. Cross-language PDG validation (#2195, PR #2197) measured the result: two Kotlin repos at 3% / 6% CDG per BasicBlock vs 18–76% for every other language (incl. the other expression-conditional languages, Rust 30% / Swift 24%). Model value-position `when` (≥2 arms) and `if`/`else` as control flow in the three dominant carriers — `property_declaration` (rejoin the arms at a binding continuation carrying the bound name's def), `return`, and the `fun f() = …` expression body (each arm returns) — mirroring the Rust visitor's value-position `let` handling. `visitWhen`/`visitIf` are reused unchanged; `isControlFlow` now routes a value-branch `val`/`var` decl to the branch handler instead of coalescing it. Measured: Exposed CDG 3636→5644 (+55%, 6%→9% of BasicBlocks), turbine 67→82 (+22%). Argument-position branches, assignment RHS, and value-position `try` are left inline — a remaining gap tracked on #2205. Behavior-preserving for non-Kotlin (bench --check byte-identical; 739 cfg tests incl. 6 new value-position regressions). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): model Ruby value-position if/case on assignment RHS as control dependence (#2195) `if`/`case` are expressions in Ruby; as an `assignment` RHS (`x = if c then a else b end`, `x = case k … end`) the visitor previously left the arms INLINE in one coalesced block (a gap documented at ruby.ts §"if/case/begin are EXPRESSIONS"), emitting no control dependence. Model the RHS branch as control flow and bind the LHS at the rejoin: `assignmentBranch` detects the carrier, `visitSeq` routes it out of the coalescing path, and `visitBindBranch` reuses `visitIf`/`visitCase` + a facts-only continuation carrying the LHS def (new `harvest.assignmentDefFacts`). Mirrors the Kotlin (#2205) and Rust value-position handling. Honest impact: SMALL in practice — rack CDG 1269→1303, sinatra 1212→1232 (~+2–3%). `x = if/case` is far rarer in idiomatic Ruby than the Kotlin analog (Ruby favors ternary / `||=` / guard modifiers), and Ruby's low CDG/BB is mostly structural (micro-branches `&&`/`||`/`?:`/`&.` excluded by design, plus many straight-line `.each`/`.map` block CFGs). This closes the documented gap correctly; it is not a large ratio mover. Explicit `return if … end` is NOT a carrier — tree-sitter-ruby drops that value; the idiomatic implicit-last-expression conditional was already modeled. Behavior-preserving for non-Ruby (bench --check byte-identical; 744 cfg tests incl. 5 new value-position regressions). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): keep an empty-arm Kotlin when wired to the join (#2195) An all-empty-arm `when` with an `else` — `when(k){0->{};else->{}}`, idiomatic `else -> {}` "do nothing" — left the dispatch block with ZERO successors: empty arms got no `switch-case` edge, and the `else` suppressed the no-match edge. The dispatch and its join then became orphaned, so isExitReachableFromAllBlocks returned false and emitFileCdg silently dropped the ENTIRE function's control dependence (counted cdgSkippedUnsound). The #2205 value-position fix newly routes `val x = when(…)` / `return when(…)` / `fun f() = when(…)` through visitWhen, exposing it in those carriers too. Wire every arm (empty or not) to the join, so the dispatch always has a successor. Found by the per-language CFG verification swarm. Behavior- preserving elsewhere (bench --check byte-identical; cfg suite green). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): wire C/C++ throw edges to every catch handler, not just the first (#2195) `visitTry` edged every protected-region block only to `handlerEntries[0]`, so in a multi-`catch` (`catch(int e){…} catch(double d){…} catch(...){…}`) the 2nd..Nth handlers were orphaned — unreachable from ENTRY, their catch-param binding and body control/data flow silently lost. The runtime catch that matches a thrown type is not statically known, so over-approximate: edge each protected block to EVERY handler entry (mirrors the Swift multi-catch handling). Found by the per-language CFG verification swarm; the two existing exception tests both used a single catch, so it was never exercised. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): keep a bare continue in a Dart switch case — it targets the loop (#2195) `caseStatements` stripped EVERY `continue_statement` from a case body, but only a LABELED `continue LABEL;` is a switch fallthrough-spill (handled via caseContinueLabel). A bare `continue;` targets the ENCLOSING LOOP (valid Dart); dropping it removed the jump and fabricated a false case → next-statement fall-through edge (e.g. `case 1: tainted(); continue; default: sink();` made tainted() flow directly into sink()). Only strip the labeled form; a bare `continue;` stays in the body and routes to the loop. Found by the per-language CFG verification swarm. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): harvest Rust match-arm pattern bindings as defs (taint propagation) (#2206) `visitMatch` visited arm bodies but never harvested the arm PATTERN's bindings, so `match x { Some(n) => sink(n) }` left `n` with a use and no def/may-def — taint from the matched subject could not propagate into the arm. Add `matchArmPatternFacts` (the binders as MAY-defs, since only the matching arm binds) and attach it to the dispatch block, co-located with the subject's use. Found by the per-language CFG verification swarm; the match tests asserted `hasUse` but never `hasDef`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): harvest Swift guard case / if case enum-pattern bindings as locals (#2206) `guard case .some(let v) = e` / `if case let .x(n) = e` nest the binder inside a `pattern` condition child, not a direct `bound_identifier`. Both the declaration (declareOptionalBindings) and the def-facts (conditionFacts, which ran walkValue = a USE on the pattern) missed it, so the binding resolved to a synthetic `@module` global with a use and no def — breaking taint propagation from the subject. Declare the `pattern` child and def its leaves (may-def when conditional). Found by the per-language CFG verification swarm. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): treat Dart switch-expression arm writes as may-defs, not hard kills (#2206) `DartHarvester.walkValue` had no `switch_expression` case, so an assignment in an arm value — `var y = switch(x){ 1 => z = 10, _ => z = 20 }` — became an unconditional def that KILLED the prior `z`, even though only one arm runs (the module docstring claimed it was a may-def, but the code didn't implement it). Walk the subject always and each `switch_expression_case` under `conditional(…)`, so arm writes are may-defs — mirroring `conditional_expression`. Found by the per-language CFG verification swarm. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): model the C# `using var` declaration-form dispose finalizer (#2206) `using var f = Open();` (C# 8) parses as a `local_declaration_statement` with a leading `using` keyword, not a `using_statement`, so visitUsing never ran — no dispose block, and a `return`/`break`/`continue` in its scope got no `finally-*` completion edge. Unlike the delimited block form, its dispose runs at ENCLOSING-SCOPE exit, so visitSeq now treats the REST of the sequence as the protected body: the acquisition (`var f = e`) is a normal block outside the dispose region (a throw there means the resource was never acquired), and `buildUsingDeclScope` wraps the remainder in a synthetic dispose finalizer (normal + exception exit, early exits thread through) — mirroring buildProtectedSynthetic. Closes the last #2206 item. Found by the per-language CFG verification swarm. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-15 12:31:04 +01:00
Gergő Magyar	7c3d4e6862	feat(pdg): control dependence — post-dominators + CDG (Ferrante) [M5 #2085] (#2188) ... * feat(pdg): add CDG + POST_DOMINATE edge types (M5 #2085) * feat(pdg): post-dominator tree on reverse CFG (M5 #2085) * feat(pdg): Ferrante control-dependence over the post-dom tree (M5 #2085) * feat(pdg): emitFileCdg + optional POST_DOMINATE debug edges (M5 #2085) * feat(pdg): wire CDG emission in-phase + pdgModeMismatch CDG-cap stamp (M5 #2085) * test(pdg): CDG snapshot + end-to-end pipeline answerability (M5 #2085) * fix(review): apply autofix feedback (M5 #2085) * fix(pdg): label CDG edges by controller arm sense, not edge kind (#2188 F1/F2/F4) Tri-review (with Codex as the independent engine) found the CDG 'T'/'F' label was wrong for the commonest control flow: the M1 TS visitor wires a condition's fall-through FALSE arm as `seq`/`loop-back`, but `branchSense` mapped both to 'T', so guard clauses, if-no-else, and loop `break` got 'T' instead of 'F' (F1, P1). The structural CDG edges were correct; only the label — the AC3 "under what condition does X run?" answer — was wrong. - F1: replace edge-kind `branchSense` with controller-arm-sense `labelFor`. An ambiguous fall-through edge (seq/loop-back) takes the COMPLEMENT of its source block's explicit cond-true/cond-false sibling arm. This correctly handles do/while (loop-back = TRUE arm) and inner-if-in-loop (loop-back = FALSE arm) — the ambiguity a kind→label table cannot resolve. Adds real-parser regression tests (the hand-built tests used a fictional cond-false edge and missed it). - F2: correct the false "sound over-approximation that never drops a real dependence" claim in post-dominators.ts — exit-unreachable regions both drop and invent control dependences (latent for the current TS visitor, which keeps EXIT reverse-reachable). Reframe the exit-less-loop test to characterize, not bless, the degenerate behavior. - F4: make the AC2 property-test reference compute post-dominance INDEPENDENTLY (node-removal reachability, no shared code with post-dominators.ts), so a post-dom direction bug can no longer pass both the impl and the reference. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ci): root-prettier format + run-analyze pdg stamp gains maxCdgEdgesPerFunction (#2085) Two deterministic CI failures from the M5 CDG work: - quality/format: basicblock-roundtrip.test.ts failed CI's root `prettier --check .` (the pre-commit hook uses the gitnexus-local prettier config, which differs); reformatted with the root config. - tests/ubuntu/coverage: run-analyze.test.ts pinned the resolved RepoMeta.pdg shape (DEFAULTS) and the all-zero cap override without the new maxCdgEdgesPerFunction key (default 5000); added it so resolvePdgConfig toEqual and pdgModeMismatch(DEFAULTS) pass. (The stale-test sweep missed this file in PR #2188 — same trap M2 hit.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(mcp): add pdg_query tool definition (controls/flows modes) [M6 #2086] * feat(mcp): pdg_query backend — controls (CDG) + flows (REACHING_DEF) + e2e test [M6 #2086] * feat(mcp): document PDG edges + pdg_query (schema, cypher, skill, --pdg-gated ai-context) [M6 #2086] * fix(mcp): correct pdg_query symbol-anchor lower bound + harden inputs [PR #2188 review] Tri-review (Codex + adversarial + correctness lanes) of the M6 pdg_query surface found the symbol-anchor window over-includes a neighbor function's block. The upper bound was widened to the 1-based BasicBlock basis (symEnd+1) but the lower bound was left 0-based, so a block on the line directly above the target function leaked into the result. Shift both bounds +1 ([symStart+1, symEnd+1]) so the window is the function's true block span. Also from the same review: - pdg_query no longer throws on a no-arguments MCP call: the dispatch passes raw `params`, so default it to {} → a clean mode-validation error instead of a TypeError. (`explain` shares this latent pattern — pre-existing follow-up.) - tools.ts: the controls-mode description no longer hard-codes the 'F' branch sense for guards — `if (!ok) return;` rides the predicate's 'T' arm; the guard:true flag is label-agnostic (regex on the dependent block text). Tests: a hand-seeded adjacency regression (verified failing without the lower-bound +1) + a no-arguments validation test. Skill doc updated to document the two-sided [symStart+1, symEnd+1] window. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): drop always-true anchor conditional in pdg_query [CodeQL #2188] CodeQL alert 756 flagged `...(anchor ? { anchor } : {})` in _pdgQueryImpl as a useless conditional: `anchor` is unconditionally assigned in both the file-path and symbol branches before the return (the not-found/ambiguous/no-layer paths return earlier), so it is always truthy. Drop `| undefined` from the declaration (TypeScript definite-assignment holds across both branches) and emit `anchor` directly. No runtime change — the `anchor` field was already present on every result. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(cli): add hasPdg to the noStats bridge expectation [#2188] The M6 work threaded `hasPdg: options.pdg === true` into the AIContextOptions passed to generateAIContextFiles on the --skills regeneration path, but this test's strict .toEqual expectation predated it (4 keys vs 3 → CI failure). Add `hasPdg: false` (the value on this non---pdg path). The assertion stays strict; the #1477 noStats bridging it guards is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(cli): collapse generateGitNexusContent params to an options bag [#2188] The function had grown to 9 positional params; reaching `hasPdg` meant passing six `undefined`s (the M6 review's maintainability flag). Collapse params 3-9 (generatedSkills, groupNames, noStats, skipSkills, runnerPath, defaultBranch, hasPdg) into a `GitNexusContentOptions` object with the defaults moved to destructuring. The body is unchanged (same local names); the single production caller and the test calls become self-documenting named fields. Pure refactor — generated AGENTS.md/CLAUDE.md content is byte-identical. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): skip CDG for exit-unreachable CFGs (unsound post-dominance) [#2188] M5 review P2: computePostDominators roots only at cfg.exitIndex and nothing enforced that EXIT is reachable from every block. For an entry-reachable region that cannot reach EXIT (a non-terminating loop, or a multi-terminal CFG a future visitor might emit) the EXIT-rooted reverse walk degenerates — it both drops real control dependences and invents spurious ones. Add a pure precondition predicate `isExitReachableFromAllBlocks` (co-located with the algorithm it guards) and gate it in emitFileCdg: a CFG that violates it is skipped for CDG (counted as skippedUnsoundFunctions + one onWarn), while its CFG and REACHING_DEF projections — which do not depend on post-dominance — are kept. A CDG-specific gate, not a widening of isEmitSafeCfg, so the blast radius is exactly the unsound CDG. The current TS visitor always satisfies the precondition (every loop gets a structural header→loopExit edge), so CDG output for real fixtures is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): bound computeControlDependence materialization (heap parity) [#2188] M5 review P2: unlike computeReachingDefs (maxFacts) and the emit-side edge cap, computeControlDependence materialized the full deduped seen/out before emitFileCdg's per-function cap could trim it — O(edges × post-dom depth) heap for a deeply nested function. Add a `maxEdges` ceiling (default 0 = unbounded) returning {edges, truncated}, mirroring computeReachingDefs's {facts, truncated}. The ceiling is checked before pushing a new unique edge, so `truncated` means a genuine overflow (not merely "reached cap"). emitFileCdg passes a FIXED materialization ceiling (8× the default edge cap) — deliberately NOT derived from the runtime edge cap, because CDG's materialization IS the deduped-edge quantity the cap reports on (deriving it would pre-truncate that set and lose the exact dropped count). A ceiling hit is surfaced via onWarn + the truncated flag — never silent. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(mcp): share resolveBlockAnchor; fix explain's anchor off-by-one [#2188] M6 review P2 (duplication) + the flagged pre-existing _explainImpl correctness follow-up. _pdgQueryImpl and _explainImpl each carried a near-identical symbol↔block anchor resolver that had DRIFTED: pdg_query used the corrected [symStart+1, symEnd+1] window (BasicBlock startLine is 1-based, the symbol span 0-based) while _explainImpl still used [symStart, symEnd] — dropping a taint source on the function's final line AND leaking a neighbor's block on the line directly above. Extract one `resolveBlockAnchor` helper, used by both, that applies the correct window and a single (bare) clause convention (callers compose their own WHERE). This removes ~50 duplicated lines and fixes explain's anchor in one place. A hand-seeded characterization test (taint-explain Block 4) pins both bounds — verified to FAIL on the pre-fix window (it returned the line-10 neighbor instead of the line-15 final-line source). Existing taint-explain + pdg-query suites are unchanged (their fixtures have interior sources/sinks). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): pdg_query reports "status unknown" when the layer can't be confirmed [#2188] M6 review P3 (Codex): when meta is UNREADABLE and the bounded global existence probe returns zero rows of the edge type, _pdgQueryImpl asserted "no PDG layer" — but a genuinely edge-free layer (all-linear functions) is indistinguishable from a missing one via that probe. Soften only that fallback path to an inconclusive "PDG layer status unknown — was this repo indexed with --pdg?" note. The meta-stamped path (stamp present, cap absent ⇒ layer truly missing) keeps the definitive "no PDG layer" wording. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mcp): cover pdg_query ambiguous / pagination / Windows-path gaps [#2188] M6 review test-gap follow-ups, all hand-seeded with controlled data: - ambiguous symbol name → status:'ambiguous' + ranked candidates shape (uid/name/filePath/score), never a silent guess; - total/truncated page boundary in both directions (limit below the match count sets truncated with the full total; limit above it omits truncated); - a Windows-style filePath containing ':' resolves and fnLineOf decodes the function-line segment correctly (split-from-right past the drive letter). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(skills): ship gitnexus-pdg-query skill mirrors + add pdg_query to the guide [#2086] M6 bundled pdg_query into this PR, but the skill shipped only in the canonical gitnexus/skills/ root. Mirror it (byte-identical) to the two hand-maintained roots the sibling taint skill uses — .claude/skills/gitnexus/ and the plugin — so Claude Code + plugin users get it too. Also extend the gitnexus-guide tool reference (all 3 copies, now byte-identical): add a `pdg_query` row + a "Control & data dependence" section mirroring the taint/`explain` section, and reconcile the pre-existing drift where only the .claude copy carried the `check` tool row (a real registered tool) — all three now list it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(architecture): refresh CFG/PDG section for the full M1–M6 stack [#2086] The PR body had deferred the "ARCHITECTURE docs refresh" to #2086; now that M6 ships here, do it: - MCP tools table gains `explain` and `pdg_query` (were absent). - "Optional CFG/PDG emission" was M1-only; rewrite to cover the whole opt-in stack — M1 CFG, M2 REACHING_DEF, M3/M4 taint, M5 CDG (Ferrante over CHK post-dominators, with the exit-unreachable skip), M6 read surface (pdg_query + explain, anchored + LIMIT-bounded, shared resolveBlockAnchor) — and note the no-Function→BasicBlock-edge join. - LadybugDB schema notes the `--pdg` additions: the `BasicBlock` node table and the CFG/REACHING_DEF/CDG/TAINTED/SANITIZES/TAINT_PATH relation types, kept out of the default VALID_RELATION_TYPES / web schema. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-13 18:49:03 +01:00
Gergő Magyar	129bc84c0d	feat(taint): interprocedural taint via function summaries over resolved CALLS (#2084) (#2179)	2026-06-13 07:04:14 +01:00
Gergő Magyar	14397dd4aa	feat(taint): intra-procedural taint analysis (#2083) (#2164) ... * feat(taint): harvest occurrence-tagged call/member sites on StatementFacts (#2083 U1) Worker-side site harvest in TsHarvester: call/new/member-read records with dotted callee paths, receiver slots, per-argument occurrence tagging with nested-site links, per-declarator resultDefs, spread/template/require-literal markers. hasTaintSafeSites validation seam. The pdg parse-cache chunk-key namespace is versioned (pdg:1 -> pdg:2) instead of a global SCHEMA_BUMP so flag-off users keep warm caches; bench fingerprints re-baselined for the three call-bearing scenarios (straight-line/dense-bindings byte-unchanged). * feat(taint): built-in TS/JS source/sink/sanitizer model + site matcher (#2083 U2) Typed spec (kind taxonomy; sanitizers carry neutralizes-kinds), the canonical Express/Node model, and matchFunctionSites: ESM alias/namespace + require- literal callee resolution, bare-name fallback restricted to true globals, sanitizers module-or-global only (never user-shadowable by name), spread/ template arg-position rules, deterministic taintModelVersion. * feat(taint): pure intra-procedural taint propagation engine (#2083 U3) Two-rule model (statement-local + du-fact worklist) with per-taint neutralized-kind exclusion sets: sanitizers exclude only the sink kinds they neutralize (escape(req.body) suppresses res.send but still fires db.query; exec(path.basename(t)) fires), intersection-over-paths so a bypass occurrence keeps the taint live, kill locality on resultDefs, propagate-through args+receiver with viaCall hops, one path per finding, deterministic caps, coverage-gap statuses. Test-first: 38 scenarios on real harvested CFGs. * feat(taint): thread taint caps + model version through pdg config/meta (#2083 U5) resolvePdgConfig gains maxTaintFindingsPerFunction (200), maxTaintHops (32), and the taintModelVersion digest; RepoMeta.pdg + RunScopeResolutionInput surfaces added. The key-union comparator trips full writeback on M2->M3 upgrade and on model-version change without --force (mode-flip tested). No CLI flags or rc keys (programmatic parity with the other caps). * feat(taint): in-phase taint emit with sparse TAINTED/SANITIZES edges (#2083 U4) run.ts pdg window: match-first fast path (solver only when a function has both a matched source and sink) -> computeReachingDefs with the shared RD fact derivation -> computeTaintFlows -> per-finding TAINTED (versioned hop-encoded reason via the shared path codec, statement-level occurrence identity) + per-kill SANITIZES, dedup-before-budget, truncate-and-warn. All emit counters surfaced (aggregate warn for gaps/drops, debug for volume); PROF gains taint=. Flag-off golden untouched. * feat(mcp): explain tool for persisted taint findings (#2083 U6) Anchorless calls enumerate the sparse TAINTED table (bounded, deterministic, limit-clamped); anchored calls (file or symbol via resolveSymbolCandidates) return full decoded hop detail. sinkKind rides a version-1 codec header (1;<kind>|hops — no other persisted channel exists; U4/U6 ship together). RepoMeta.pdg probe yields a no-taint-layer note instead of an error. TAINTED/SANITIZES pinned OUT of VALID_RELATION_TYPES (KTD9a negative- membership tests); generators + canonical skill docs + mirrors updated. * test(taint): acceptance fixture battery, snapshots, and bench gates (#2083 U7) pdg-repo taint-cases fixtures complete the six plan shapes; committed findings/kills snapshot via a shared pure-path harness that also feeds the AE2 exact-equality assertion (stored TAINTED == pure-path findings, the no-explosion gate). New taint-dense bench scenario with four --check gates: per-function findings pinned AT the cap, absolute reason-byte + site-bytes disk ceilings (the load-bearing R10 gate), zero-match pass < 0.5x match- dense, N-linearity. Pre-existing scenario baselines untouched. * refactor(taint): share one pointKey helper across propagate + emit (#2083 review) Extract pointKey(ProgramPoint) to cfg/reaching-defs.ts (colon-separated, matching the codebase block:stmt id convention) and import it in both propagate.ts and emit.ts, replacing the two divergent locals (':' vs '.'). Edge-id material now uses the colon form; ids are in-memory only and no test asserts the pointKey segment shape. * fix(taint): discriminate taint state by source occurrence (#2083 review) Two distinct sources flowing into one variable at one def point no longer collapse to a single TAINTED edge: the taint-state key gains a root source-occurrence discriminator ({point, siteIndex} — the same fields recordFinding's identity uses, excluding kind). Def->use fact lookup keys on the source-independent (binding, def-point) portion. Same-source multi-path flows still share one state so their exclusion sets intersect (the raw arm soundly wins); termination holds (finite keys, monotone shrink, no cross-source ping-pong). Restores the KTD6 identity contract. * fix(mcp): route dotted symbol names in explain to symbol resolution (#2083 review) The fileish classifier matched any dotted name (UserController.create) as a file via its extension-like suffix, so symbol resolution never ran and the tool returned a silent empty file-anchored result. Tighten the classifier to require a path separator or a real source extension (derived from the resolver's EXTENSIONS list, multi-language), so dotted/bare names route to resolveSymbolCandidates (found / ambiguous / not-found). * fix(mcp): gate explain no-taint-layer note on taintModelVersion (#2083 review) An M1/M2-era --pdg index has meta.pdg defined (BasicBlock/REACHING_DEF recorded) but no taintModelVersion and zero TAINTED rows. The probe keyed on generic meta.pdg presence, so explain returned the generic empty note instead of the actionable 'no taint layer — run analyze' hint. Gate on meta.pdg?.taintModelVersion (the field M3 stamps) so an M2-era index gets the layer hint; a taint-stamped index with no findings still gets the generic note. * fix(taint): sequence-expression value flows only the final operand (#2083 review) A comma expression in value position (exec((log(x), 'safe'))) default- descended, fanning every operand's occurrences into the enclosing sink argument — over-tainting exec's arg 0 with x. Add an explicit walkValue case that records earlier operands' uses with occurrence fan-out suppressed (new FactAccumulator.suppressOccurrences) and routes only the last operand through the value path. Sites-layer only; defs/uses/mayDefs byte-identical (cfg + reaching-defs snapshots unchanged). * perf(taint): FIFO head-cursor worklist + dedup before chainHops (#2083 review) Replace queue.shift() (O(N) dequeue) with a strict-FIFO head cursor plus order-preserving prefix reclamation; FIFO is load-bearing because chainHops reads the live taints map whose parent/source/viaCall are rewritten order-sensitively on monotone shrink, so hop determinism is dequeue-order contingent. Extract findingKey() and dedup-check before chainHops in the justify branch — already-recorded identities discard their hop chain (first write wins), so the ancestry walk was pure waste. The else kill branch is untouched. Findings + hops byte-identical (snapshot unchanged). * perf(taint): O(1) member-read dedup via composite-key set (#2083 review) addMemberRead rescanned the whole per-statement sites array per call to dedup by (object, property, parent) — O(n^2) on member-read-dense statements. Track a composite-key Set alongside sites for O(1) dedup. (The require-literal join is already O(sites) with a no-op body on non-require sites, so no early-exit is needed there.) Behavior identical: harvest + model-match + taint snapshots unchanged. * refactor(taint): drop test-only export; source taint caps via emit.ts (#2083 review) Remove the sanitizerNeutralizes export (its only consumers were two test assertions — inlined to entry.neutralizes membership). Re-export the DEFAULT_PDG_MAX_TAINT_* caps from emit.ts and point run.ts at emit.ts, so the pipeline's taint dependency surface is the single orchestration module rather than reaching into propagate.ts. * test(taint): extract the shared TS CFG/taint test harness (#2083 review) The parse/collectFunctions/cfgOf/cfgsOf/importsFor harness was copied byte-for-byte across four suites (harvest, model-match, propagate, taint-emit). Promote it to test/helpers/ts-cfg-harness.ts and import it. site-safety/reaching-defs carry a structurally different inlined builder and are left as-is. Pure extraction, no assertion changes. * test(mcp): harden explain limit-rejection battery (#2083 review) Add NaN, Infinity, -Infinity, and a numeric string to the out-of-bounds limit cases — a regression fence over the interpolated LIMIT, confirming the Number.isInteger guard rejects every non-integer/non-finite/string input before it reaches the query.	2026-06-12 07:35:09 +01:00
Gergő Magyar	bde340a5b4	feat(cfg): intra-procedural REACHING_DEF data-dependence layer (#2082) (#2160) ... * fix(cfg): route early exits through finally with target-relative threading (#2082 U2) * feat(cfg): harvest per-statement def/use facts into the side channel (#2082 U1) * feat(cfg): add reaching-definitions solver with GEN/KILL fixpoint + statement sweep (#2082 U3) * feat(cfg): persist budgeted REACHING_DEF projection with RepoMeta coherence (#2082 U4) * test(cfg): REACHING_DEF snapshot, pipeline both-sinks, and cache-seam coverage (#2082 U5) * bench(cfg): reaching-defs scaling gates — dense-bindings + fact-fanout scenarios (#2082 U6) * fix(mcp): exclude BasicBlock pseudo-symbols from detect_changes on pdg indexes (#2082 U7) * style: prettier pass over M2 files * fix(cfg): review-pass fixes — defKey overflow guard, catch-param block, class defs, intra-statement reads, graceful fact degradation (#2082) - reaching-defs: STMT_STRIDE 2^16→2^21 + upfront aliasing bail-out; a use that shares its statement with a def now also sees the same-statement def (assign-and-test idiom was a taint false negative); drop dead posInOrder - visitor: catch-param def gets its own once-executed block (prepending into a loop-header entry re-genned per iteration and killed loop-carried redefs); unresolved-label jumps now thread all active finallys; the finalizer-threading protocol moved to control-flow-context as shared helpers for future language visitors - harvest: class declarations def their name (was a bogus use in JS, silent skip in TS); class-expression names stay internal - emit: isEmitSafeCfg adds index==position contiguity; fact validation split into hasEmitSafeFacts so malformed facts degrade to CFG-only instead of dropping the function's whole CFG layer; facts-per-edge multiplier single source; lazy top-binding tally; dead solveMs removed - run-analyze: pdgModeMismatch compares the key union structurally — new resolved knobs join the comparison automatically - mcp: BasicBlock exclusion via id prefix (NULL-name rows of real symbols are no longer dropped) + same filter on the BM25 filePath fallback - bench: rd ratio denominator clamped (gate no longer self-disables at fast small-N); PROF-gated pdg timing in run.ts * test(run-analyze): model the M2 RepoMeta.pdg stamp in resolvePdgConfig defaults The DEFAULTS constant lacked the maxReachingDefEdgesPerFunction field that resolvePdgConfig resolves since the M2 stamp landed, failing two strict toEqual expectations (the CI 'tests' job failures). Models M2 steady-state equality; the M1-era-stamp upgrade path stays pinned in pdg-mode-flip.test.ts. Finding P1-4 of review 4471987625 (#2160). * test(cfg): reassign the shadowing fixture's bindings — fixes prefer-const CI errors Both withShadowing let bindings now genuinely reassign (s = s + 1 per scope), clearing the two prefer-const errors that failed quality/lint. Plain const would change the binding kind the harvest test exercises; reassignment keeps the let semantics and enriches the reaching-defs facts the snapshot pins (snapshot + per-binding assertion updated accordingly). Finding P2-6 of review 4471987625 (#2160). * fix(cfg): validate entry/exit indices in the emit-safety guard A corrupted side-channel element with an out-of-range entryIndex passed isEmitSafeCfg and threw inside the reaching-defs RPO walk — caught by the per-FILE try/catch, costing every sibling function's REACHING_DEF projection instead of the one element (and logging a misleading message). entry/exit join the guard's id-anchor checks. Finding P3 (entryIndex) of review 4471987625 (#2160). * fix(cfg): report the def-key stride bail-out as a distinct 'overflow' status The STMT_STRIDE aliasing guard reused status 'truncated', so the emit warn misnamed it as the fact-materialization limit (printing an unrelated maxFacts value, including '(0)' when unlimited) and telemetry conflated the two. A distinct 'overflow' status gets its own warn naming the actual cause; the function's CFG layer is explicitly unaffected. Finding P3 (stride-bail diagnosis) of review 4471987625 (#2160). * perf(cfg): cache the nearest enclosing scope per node during the prescan resolve() walked the AST parent chain per identifier — O(expression nesting depth), quadratic on deeply-chained single-statement expressions in generated code (not caught by any bench scenario, which scale blocks/bindings, not expression depth). The prescan already visits every node once, so caching its innermost scope makes phase-2 resolution O(scope-chain). Behavior-identical; the parent-chain walk survives as fallback for prescan-unvisited nodes. Finding P2 (resolve depth walk) of review 4471987625 (#2160). * fix(cfg): stop harvesting initializer-less var declarators as defs A bare `var x;` mid-function is hoisted and writes nothing at runtime, but the harvester recorded a def — fabricating a kill of the live def in the same block: `x = source(); var x; sink(x)` lost the source→sink fact (a reaching-defs false negative). Defs now require an initializer for variable_declaration declarators; let/const genuinely initialize and keep their def. Finding P2-5 of review 4471987625 (#2160). * fix(cfg): unwrap parenthesized/non-null lvalue wrappers before def detection `(x) += 1` and `(x)++` gated the def on the node type being exactly 'identifier', so the parenthesized form fell to the uses-only branch — the def (and its kill) silently vanished. Wrappers that don't change the lvalue (parenthesized_expression, TS non_null_expression) now unwrap at all three lvalue sites. Finding P3 (parenthesized lvalues) of review 4471987625 (#2160). * fix(cfg): conditionally-evaluated defs are MAY-defs — gen without kill A def inside a short-circuit right operand, ternary arm, logical assignment, or switch case test was harvested as a must-def; the solver's total kill then erased the prior def on the not-taken path — a taint false negative on core idioms (`if (a && (x = clean())) {} sink(x)` lost source→sink; `cached ?? (cached = load())` likewise). StatementFacts gains an optional mayDefs field (conditional-context tracking in the harvester); the solver's per-block GEN carries {set, kills} so a may-def UNIONS into the binding's set instead of replacing it, in both the transfer and the statement sweep; the emit fact-guard validates mayDefs indices; switch case tests harvest via the conditional path. Finding P1-1 of review 4471987625 (#2160). * fix(cfg): model labeled statements generically — break keeps its real continuation A break to a label the visitor didn't model (labeled non-loop block, the OUTER label of a doubly-labeled construct) routed to EXIT, REMOVING the only path that kept the pre-jump def live — a reaching-defs false kill the in-code comment wrongly called sound. Loop/switch frames now carry their full label LIST (`outer: inner: for` resolves both); a labeled non-loop statement gets a break-target frame whose target is a synthesized join after the body; an unlabeled break never matches a block frame; labels compose with finalizer threading (a labeled break crossing a finally still threads it). Finding P1-2 of review 4471987625 (#2160). * fix(cfg): throw edges deliver ALL of a block's defs to the handler The throw contribution was IN ∪ OUT — entry and final states only. The intermediate defs of a multi-def coalesced block were invisible to the handler, though they are exactly what the catch observes when a later statement throws: `try { x = parse(a); x = normalize(x); } catch { sink(x) }` lost the parse→sink fact (normalize throwing delivers parse's value). Throw predecessors now contribute IN(from) ∪ allDefs(from) — a static per-block all-def-sites map — which subsumes OUT; monotone and deterministic. Finding P1-3 of review 4471987625 (#2160).	2026-06-11 05:49:39 +01:00
Gergő Magyar	5bf8a17cd5	feat(ingestion): add control-flow-graph layer for TS/JS (#2081) (#2099) ... * feat(cfg): language-agnostic CFG construction core (#2081) U1 of M1 (CFG layer). Plain JSON-serializable CFG data model (BasicBlockData/ CfgEdgeData/FunctionCfg — must survive the worker→main boundary + ParsedFile store), a CfgBuilder accumulator (leaders→blocks→edges, synthetic ENTRY/EXIT, idempotent edges), a ControlFlowContext (break/continue/switch + labeled-jump target stacks), and a TraversalResult ({entry, dangling exits}). AST-agnostic and unit-tested on the classic control-flow topologies (if/else, while back-edge, mid-block return, labeled break/continue) the S2 spike validated; reachability helper backs the R9 property test. * feat(ingestion): U2 — TS/JS CFG visitor over tree-sitter AST (#2081) Add the TS/JS CfgVisitor that walks a function's tree-sitter AST and drives the U1 CfgBuilder to produce a serializable FunctionCfg. One visitor covers both languages (shared grammar family). Handles the classic CFG hazards explicitly (R2, R10): - loops allocate a dedicated loop-exit block so `break` has a concrete target before the loop's successor is known; `continue`/back-edge close the loop (while, do-while, C-for with init-once + increment-as-continue-target, for-in, for-of) - switch fallthrough falls out naturally: a non-breaking case yields exits we wire to the next case as `fallthrough`; a breaking case wires to the switch exit via ControlFlowContext - try/catch/finally: normal completion AND exceptional flow both route through finally (post-domination); a conservative exceptional edge models that the protected region may raise to its handler (not just explicit `throw`) - labeled break/continue resolve against the labeled loop's frame - early return/throw wire to EXIT/handler and terminate their block 19 hazard tests (one per construct) + AC1 10-function fixture; all green. No change to the committed U1 core or ControlFlowContext. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(ingestion): U3 — worker CFG build + cfgSideChannel + cache coherence (#2081) Run the CFG visitor in the parse worker (where the AST lives), serialize the per-function CFG onto a new ParsedFile.cfgSideChannel, and keep it coherent across the disk-backed store and the warm/durable parse cache (R3, R4). - gitnexus-shared parsed-file.ts: add `cfgSideChannel?: unknown` as a DISTINCT field from captureSideChannel (different producer/consumer/lifecycle; plain JSON data — blocks/edges deliberately lack the `nodeId` the store's interning reviver keys on, so no mis-interning). - cfg/types.ts + visitors/typescript.ts: add CfgVisitor.isFunction so the worker enumerates functions (and applies the line budget) by a cheap node-type test. - cfg/collect.ts (new): collectFunctionCfgs walks the tree, builds one CFG per function (nested included), applies maxFunctionLines (over-cap = skipped). - language-provider.ts: add `cfgVisitor?: CfgVisitor<SyntaxNode>` hook; typescript.ts attaches it to both the TS and JS providers (shared grammar). - parse-worker.ts: read pdg + pdgMaxFunctionLines from workerData (read once at init — the worker never sees PipelineOptions), gate the build, attach cfgSideChannel alongside captureSideChannel. - parse-cache.ts: bump SCHEMA_BUMP 4→5 (ParsedFile shape changed) and fold the pdg flag into computeChunkHash so a pdg-off cached chunk is NOT reused on a --pdg run (the #2038-class warm-cache trap). Default path keeps its keys. - worker-pool.ts + parse-impl.ts + pipeline.ts: thread pdg/pdgMaxFunctionLines PipelineOptions → WorkerPoolOptions → workerData, and into the chunk-hash key. 9 boundary tests: collect contract, JSON round-trip identity (no AST leakage), the pdg cache-key guard, the line-cap skip, and the no-visitor gate. Full CFG suite (U1+U2+U3) green; build clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(ingestion): U4 — emit BasicBlock + CFG within scope-resolution (#2081) Emit persisted BasicBlock nodes + CFG edges from each ParsedFile's worker-built cfgSideChannel, INSIDE scope-resolution's Phase-4 graph emission — the last point where the worker-built CFGs are loaded (emitParsedFiles carries the channel; the disk store is cleared right after the orchestrator returns). This is the architecture the doc-review corrected to: a standalone post-`mro` phase (the issue's literal subtask) provably reads empty data (KTD1). - cfg/emit.ts (new): pure emitFileCfgs(graph, cfgs, maxEdgesPerFunction, onWarn). BasicBlock id = `BasicBlock:<filePath>:<functionStartLine>:<blockIndex>` (KTD3 — funcStart disambiguates blocks across functions in one file; no `name` column). CFG edge = CodeRelation type 'CFG' with the edge KIND (seq/cond-true/…) in `reason` (kinds can't be their own edge type). Per- function edge cap stops at the cap and warns with the dropped count — no silent truncation (R6/KTD6). - run.ts: pdg-gated emit pass over emitParsedFiles after emitPostResolutionEdges (store still live); RunScopeResolutionInput gains pdg + pdgMaxEdgesPerFunction. - phase.ts: thread ctx.options.pdg / pdgMaxEdgesPerFunction into the call. - pipeline.ts: PipelineOptions.pdgMaxEdgesPerFunction. 6 tests: node/edge shape (KTD3 id, no name, type='CFG', kind in reason), cross-function id uniqueness, AC2 reachability-from-ENTRY property, the edge cap's no-silent-truncation contract, and empty-input no-op. Flag-off byte-identity + full runPipelineFromRepo round-trip land in U7. Build clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(cli): U5 — `--pdg` opt-in plumbing (CLI + .gitnexusrc → both sinks) (#2081) Expose the CFG/PDG substrate as an opt-in and thread it from CLI/.gitnexusrc to the single source of truth (PipelineOptions.pdg), which fans out to BOTH sinks already wired in U3/U4: the worker build gate (workerData.pdg) and the scope-resolution emit gate. Off by default (R7). - cli/index.ts: `--pdg` commander flag. - cli/analyze.ts: AnalyzeOptions.pdg + pass `pdg` into runFullAnalysis options. - cli/analyze-config.ts: KEY_SPECS `pdg` (boolean) so `.gitnexusrc { "pdg": true }` normalizes and a non-boolean value fails closed with GitNexusRcError. - core/run-analyze.ts: AnalyzeOptions.pdg → runPipelineFromRepo({ pdg }). (The internal PipelineOptions/WorkerPoolOptions/workerData fields + the parse-cache key fold landed in U3/U4; this unit adds the user-facing surface. The budget knobs stay at internal defaults for M1.) Tests: analyze-config pdg normalization + non-boolean rejection; opt-in.test.ts covers the CLI/file merge precedence and that pdg perturbs the chunk-dispatch key. The full worker-build + main-emit round-trip is the U7 integration test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(ingestion): U7 — CFG acceptance fixtures, parity, end-to-end + docs (#2081) Acceptance criteria for the M1 CFG layer: - AC1: a 10-function TS fixture's CFG node/edge set matches a committed snapshot (cfg-snapshot.test.ts). - AC2: every BasicBlock is reachable from its function ENTRY (property test over the emitted graph; the fixture has no dead code). - AC3: hazard fixtures lock the classic-bug coverage — try/throw/finally post-domination + labeled break/continue resolution. - AC4: the existing pipeline-graph-golden test stays byte-identical with --pdg off (verified; no UPDATE_GOLDEN), proving the opt-in adds zero default-run drift. - End-to-end (pipeline-pdg.test.ts): runPipelineFromRepo({ pdg: true }) on a tiny repo emits BasicBlock nodes + CFG edges with both endpoints present — the true both-sinks proof (worker builds → store → scope-resolution emits); the default run emits zero. Docs: CHANGELOG M1 entry, ARCHITECTURE "Optional CFG/PDG emission" subsection (why emit is in-phase, not post-mro), README CFG language-support note. Full CFG suite (U1–U7): 56 tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(ingestion): drop unused helper in cfg-snapshot test (#2081) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(review): apply ce-code-review autofix feedback (#2081) Review (10 reviewers) confirmed OFF-path byte-identity (adversarial + golden) and found defects all within the --pdg path. Fixes: - P1 same-line BasicBlock id collision: add a start-column disambiguator to FunctionCfg + the id (`BasicBlock:<file>:<line>:<col>:<idx>`) so two functions sharing a start line no longer collide under first-writer-wins addNode. - P1 worker crash-cascade: per-file try/catch around collectFunctionCfgs so a CFG-build throw cannot escape to the language-group catch and silently drop every remaining file in the group. - P2 edge-cap drop now logs unconditionally (input.onWarn is validator-gated/ silent in prod) — upholds the no-silent-truncation guarantee. - P2 Array.isArray guard before the cfgSideChannel cast in run.ts. - P2 maxFunctionLines default: worker applies DEFAULT_PDG_MAX_FUNCTION_LINES=2000 when unset; caps forwarded through run-analyze AnalyzeOptions (closes the server-path drop). - P3 README duplicate paragraph removed; `0`-vs-default docstrings corrected; CLI --pdg flag made language-neutral; reachableBlocks JSDoc corrected. - Documented the break-through-finally + stacked-label CFG limitations. - Tests: same-line id-collision regression, standalone throw→EXIT, dead-code- after-return, async/generator/method coverage, strengthened labeled-continue. Refuted: the HTTP-500 getNodeQuery finding — M0 already shipped the BasicBlock branch + name-floor (R12/web-safety handled). CFG + analyze-config suites: 95 tests green; golden parity (AC4) byte-identical. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(ingestion): benchmark CFG construction + O(n) block-text accumulation (#2081) Closes the M1 review's requires_verification perf gap ("no benchmark for collectFunctionCfgs; a wall-time + cfgSideChannel byte-size regression gate would catch the extendBlock concatenation before kernel scale"). - bench/cfg/measure.mjs (new): build-free tsx harness timing collectFunctionCfgs (parse once, reuse the tree) across three scaling scenarios — straight-line (extendBlock path), many-functions (collect walk), branchy (block/edge growth) — at 500→2000. Reports a wall-time scaling ratio AND a cfgSideChannel byte-size ratio, plus an order-independent sha256 over the emitted blocks/edges as the behavior gate. `--check` compares both ratios + the fingerprint against bench/cfg/baselines.json; mirrors the scope-capture / python-scope harnesses. - .github/workflows/ci-tests.yml: run the gate on every test job (build-free, alongside the existing scope-capture guards) so an O(n^2) re-regression fails CI. - cfg-builder.ts: structural fix for the one real hotspot the bench surfaced — accumulate basic-block text as fragments joined once in finish(), instead of concatenating onto a growing string per coalesced statement (O(n^2) → O(n)). Behavior-identical (the CFG fingerprint + the AC1 snapshot are unchanged). Measured (post-fix): time ratios straight-line ~1.3, many-functions ~1.0, branchy ~1.1 (all sub-quadratic; a true O(n^2) would be ~4.0). cfgSideChannel bytes scale linearly (~1.0-1.04). 60 CFG tests green; build clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(ingestion): add memory + disk growth gates to the CFG benchmark (#2081) Extend bench/cfg/measure.mjs beyond wall-time to the two other scalability dimensions that matter at kernel scale: - DISK growth: utf8 byte size of the serialized cfgSideChannel — exactly what a --pdg run writes onto every ParsedFile shard (durable store + parse cache). - MEMORY growth: retained JS heap of the cfgSideChannel payload, measured by the release-delta method (heap held minus heap after dropping it) — robust to pre-existing garbage and dead-stable run-to-run. Needs `node --expose-gc`; without it the heap metric is null and its gate is skipped (local runs still work). ci-tests.yml now passes --expose-gc so the heap gate runs in CI. Both gated on linear scaling in baselines.json (disk_bytes_budget / heap_budget 1.2-1.3). Measured: disk ~1.0-1.04, retained heap ~0.87-1.0 — both linear (~1KB/function each; ~2MB heap / 1.6MB disk at 2000 functions, --pdg only). Bumped REPS 7->15 to stabilize the noisier time signal and widened the coarse time tripwire budgets (the disk/heap gates carry the tight regression detection). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ingestion): address tri-review + CFG-expert findings (#2081) Corroborated findings from the tri-review (Codex + CE personas + GitNexus swarm + a CFG/program-analysis domain-expert lane). The OFF-path stays byte-identical; all fixes are within the --pdg path or the benchmark. - [Codex+CFG-expert] Exceptional `throw` edges now wire EVERY block in a try's protected region to the handler, not just the body ENTRY. A branched try body (`try { if (x) { use(t); } } catch`) previously left interior blocks with no path to `catch` — a taint false-negative into the handler for the M2 PDG pass. - [Codex+CFG-expert] An unresolved labeled jump (a stacked outer label or a labeled non-loop block) now routes to the function EXIT instead of leaving a dangling sink — restores the single-exit invariant post-dominator/PDG computation needs. - [Codex] computeChunkHash now folds pdgMaxFunctionLines/pdgMaxEdgesPerFunction into the chunk key (not just the pdg boolean), so a warm cache built under one cap is never served to a run with a different cap (#2038 class, extended to the budgets). Adds PdgCacheKey; boolean form kept for back-compat. - [perf] visitTry resolves catch/finally in a single namedChild pass (the double `namedChildren.find` allocated two throwaway arrays). - [adversarial] The bench `straight-line` scenario now runs at 2000->8000: output is a constant 4 blocks so disk/heap can't see the concat path, and at the old N a genuine O(n²) was masked by V8 cons-strings. Verified at the new N: the array-join impl ~1.0, a rope-optimized `+=` ~1.0 (correctly not flagged), a real O(n²) (re-join-every-append) ~3.8 — budget tightened 2.0->1.5. - [adversarial+Codex] The bench `--check` now FAILS LOUDLY when run without `--expose-gc` instead of silently skipping the retained-heap gate. - Doc: re-labeled the finally-bypass as a SOUNDNESS (false-negative) limitation tracked for M2, not mere "precision." 3 new regression tests (branched-try interior→handler, stacked-label→EXIT, cap-fold key). 99 CFG tests pass; build clean; bench gate green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(parse-cache): clarify that SCHEMA_BUMP still invalidates caches once (#2099 F6) The computeChunkHash comment claimed pdg-off warm caches "survive this change untouched" — true for the key FORMAT, but misleading as an upgrade-behavior promise: SCHEMA_BUMP 4→5 changes PARSE_CACHE_VERSION and both stores hard-invalidate on it. Separate the two facts so the next cache change isn't reasoned about from a false premise. Review finding F6 (P3) of PR #2099 tri-review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): correct for-loop back-edge kinds when no increment clause (#2099 F5) A for with a body but no increment emitted an unconditional header→header 'loop-back' self-edge (a path that never executes the body) while the real back-edge body→header was labeled 'seq'. Any consumer identifying loops via reason='loop-back' picked the phantom edge and excluded the body from the natural loop. Gate the self-edge on the body being absent (the one case where the header genuinely re-tests itself) and carry 'loop-back' on the body's exits when they ARE the back-edge, matching visitWhile/visitForIn. Review finding F5 (P3) of PR #2099 tri-review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): treat an empty catch clause as a real handler (#2099 F2) visitTry keyed handler semantics off the traversal result — null for an empty body, since visitSeq([]) returns null — instead of the syntactic clause. An empty `catch {}` was therefore treated as NO catch: the swallowed exception escaped to the outer handler/EXIT, the no-catch re-propagation misfired past finally, and code after a try whose body always throws became unreachable from ENTRY — a hard false-negative source for the M2 taint pass, on an extremely common pattern. Synthesize one empty block spanning the clause (entry == sole exit) when the catch body traverses to null, before the protected region is walked. Exception flow lands in it and rejoins the normal continuation; all downstream wiring (handler selection, finally routing, the !catchRes re-propagation gate) operates on the syntactically-correct shape. Review finding F2 (P2, reproduced) of PR #2099 tri-review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): guard CFG emission per element, not just per outer array (#2099 F4) The cfgSideChannel guard checked only Array.isArray before casting to FunctionCfg[] — its own comment promised a wrong-shape value would 'skip emission, not throw a TypeError mid-graph-build', but a malformed ELEMENT sailed through. Worse, the obvious-looking failure shape never throws at all: emitFileCfgs string-templates any edge endpoint into the BasicBlock id and graph inserts are no-throw, so a non-integer endpoint silently became a dangling 'BasicBlock:…:undefined' edge that degrades the DB rel-pair COPY to row-by-row fallback inserts much later. Layered fix matching house precedents (parsedfile-store reviver, worker-side per-file catch): a per-element shape+content predicate (arrays + integer edge endpoints) that warns and skips malformed elements while valid siblings still emit, plus a per-file try/catch backstop for shapes that genuinely throw (e.g. a null inside blocks). Review finding F4 (P3) of PR #2099 tri-review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(parse-cache): drop emit-time edge cap from the pdg chunk key (#2099 F3) pdgMaxEdgesPerFunction is applied exclusively in emitFileCfgs during scope-resolution on the main thread — the worker never receives it (workerData carries only pdg + pdgMaxFunctionLines), so the cached worker output is byte-identical across cap values. Folding it into the chunk key (added by a prior review round) only converted a free knob into a repo-sized cost: every cap change forced a full re-parse and a durable-store rewrite of unchanged data. Keep pdg + maxFunctionLines (genuinely worker-visible, shape the cached cfgSideChannel) and document the classification test in the PdgCacheKey doc comment so the next option gets sorted deliberately: worker-shard inputs go in this key; persisted-graph-only inputs belong in the RepoMeta pdg stamp (F1). Chunks written under the old ns string miss once and prune — no migration needed. Review finding F3 (P2) of PR #2099 tri-review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(analyze): record pdg config in RepoMeta; force full writeback on mode flip (#2099 F1) Running --pdg against an already-indexed repo silently persisted ~zero CFG: incremental eligibility had no pdg term, RepoMeta recorded no mode, and extractChangedSubgraph keeps only changed-file nodes — on a no-change --pdg re-run every freshly built BasicBlock was dropped from the written subgraph ('Incremental: changed=0', run succeeds, zero rows). The converse flip left zombie mixed-coverage blocks only --force could clean. Worse, a clean-tree flip hit the alreadyUpToDate fast path and never ran the pipeline at all. - RepoMeta gains an additive-optional pdg stamp ({maxFunctionLines, maxEdgesPerFunction}, resolved values; absent ≡ pdg-off, which covers every legacy meta). No INCREMENTAL_SCHEMA_VERSION bump — that would force a one-time full rebuild for everyone. The end-of-run meta is a fresh literal, so omitting the field on a pdg-off run is what clears the stamp after an on→off flip. - pdgModeMismatch (pure, exported) compares the resolved triple; the flip check sits before the fast path and always logs its notice (not gated on options.force — --skills implies force with no message of its own), naming the .gitnexusrc pdg key that pins the mode. - The full-rebuild branch now writes the incrementalInProgress dirty flag (toWriteCount: 0 sentinel) before the wipe whenever a prior meta exists, mirroring the incremental branch. This closes the crash window where a rebuild dying between the bulk load and saveMeta left meta/DB inconsistent and the fast path certified zombie (or missing) CFG rows indefinitely — and incidentally closes the same pre-existing hole for user --force runs. Recovery log reworded accordingly. Tests: pdg-mode-flip.test.ts (real git + LadybugDB; primary assertion is a direct BasicBlock table count — meta.stats aggregates nondeterministic Community/Process rows) covering off→on, steady-state fast path, on→off zombie cleanup, cap-change rebuild, and dirty-flag + flip composition; pure-helper tests for default resolution and the 0=unlimited carve-out. Review finding F1 (P1) of PR #2099 tri-review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-10 19:26:45 +01:00