mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-08-28 05:25:25 +00:00
52 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
DuduPhudu
|
223ac7010d
|
feat: close reported graph blind spots in reference resolution, analyze and storage (#2856)
* fix(mcp): report UNKNOWN risk when an upstream impact walk finds no callers `risk: LOW` asserts "safe to change" — a claim ABOUT callers. An upstream walk that resolved none has nothing to base it on: the symbol may be genuinely unused, or reached only through a reference class the index does not record (a property access on a plain object, a bare-identifier read of a module-scope const). Seeding LOW from an empty result is the false-safe signal `anyKnownRisk` already refuses to emit on the ambiguous-candidate path, and that #2687 removed by making an undetermined impactedCount `null` rather than `0`. Zero-caller upstream results now report risk UNKNOWN with a riskNote saying absence of edges is not evidence of disuse. Downstream is untouched: an empty downstream walk reports resolved callees, not safety. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(javascript): emit ACCESSES for bare-identifier reads of module-scope consts A constant read only as a bare identifier — `Math.max(LIMIT, n)`, a default parameter value, `return LIMIT` — minted no reference site at all, because JS captured only `@reference.read.member`, which requires a receiver a bare identifier does not have. So "who uses this constant?", the question behind every dead-code trim and constants refactor, answered with a confident zero in both directions. The rest of the machinery was already in place: `FIELD_KINDS` accepts `Const`, the scope query already declares it via `@declaration.const`, and `read` maps to ACCESSES for any resolved target. This adds the missing capture in VALUE POSITIONS ONLY (call arguments, default-parameter values, return statements) — a blanket `(identifier)` rule would mint a site for every token in the file, which is unaffordable at repo scale and would keep alive the block-local symbols `pruneLocalSymbols` exists to drop. Cross-file readers are NOT yet covered: the site exists and a call through the same import statement resolves, but a value-kind def does not link across the import edge. Recorded as a todo with the investigation. PARSE_CACHE_VERSION bumped 44 -> 45: this is parse-time capture emission, so a warm cache replays the pre-change capture set and the new edges never appear — observed directly, a full `analyze --force` produced a byte-identical graph until the cache was cleared by hand. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(javascript): pin A1/A5 plain-object property acceptance criteria Fixture plus todo specs for the four shapes plain-object property access has to answer: object-literal keys indexed as Property nodes, a read through the holding variable, a property WRITE, and a read through an untyped param. Records the investigation so the work is resumable: the parse-query pattern scoped to literals bound to a variable matches correctly (verified against the raw JAVASCRIPT_QUERIES), but no Property node reaches the graph and local-symbol-pruner is not the cause — it drops only Const/Variable/Static. The remaining gate is in the parse worker's node-creation path. No production code — specs only, so the suite stays green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(javascript): index object-literal keys of a named object as Property nodes Idiomatic JS models configuration as an object literal, not a class, but Property definition nodes existed only for DECLARED CLASS FIELDS. A config field therefore had no symbol at all: `context({name: 'exitMinAtrMult'})` answered "not found" for a field read and written throughout a live code path, and ACCESSES had no target to point at. Both halves are added for keys of a literal BOUND TO A VARIABLE — the parse query mints the graph node, the scope query mints the def the resolver can aim at. Unbound literals are deliberately excluded: an inline call argument or a JSX prop bag is call-site data, not a named surface other code references, so a node per key there would add volume without adding an answerable question. This lands the definition-node half only. The ACCESSES edges still require receiver resolution — typing the const that holds the literal to the literal's scope for the precise case, and name-based matching at reduced confidence for the untyped-param (option bag) case. Both are recorded as todos with the mechanism each needs. Also records a trap that cost a wrong conclusion: under vitest the parse worker runs the BUILT dist code (parse-impl resolves parse-worker.js, absent under src/, and falls back to dist), so parse-query changes are invisible to tests until `npm run build`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(cache): move the SCHEMA_BUMP pin to 45 The pin is the guard that makes two branches claiming one cache-schema number fail loudly instead of silently serving each other's entries, so a bump is only half-done until the pin moves with it. The bump itself landed with the JavaScript bare-identifier captures; this is the other half. Caught by the guard working exactly as designed — the suite failed with "expected 45 to be 44" rather than letting a mismatched pair through. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(scope-resolution): resolve plain-object property access by unique name Idiomatic JS reads configuration off an object whose receiver cannot be typed — an options bag passed as a parameter, a destructured handle, an imported literal. No precise pass resolves those, so a field read and written across a live code path produced no ACCESSES edge at all and "who reads this setting?" answered a confident zero. A last-resort pass runs after every precise pass and sees only what they left behind. For each still-unresolved read/write site it asks whether exactly ONE Property in the workspace carries that name. If so the read almost certainly means it. If two or more do, nothing is emitted and the site is COUNTED as ambiguous — a guess between them would be a coin flip, and a wrong edge in the pre-edit safety gate is worse than a missing one. Uniqueness is the right gate because it recovers exactly the names worth recovering: distinctive domain fields (exitMinAtrMult, bookNotionalUsdt) are unique in a repo and resolve, while generic keys (id, name, data) are not and are skipped — which is where name matching would over-connect. Bounded four ways: - Confidence 0.5, the global tier, with the inference named in the reason, so a consumer can filter inferences without losing scope-resolved edges. - Never second-guesses a precise result: sites already resolved are excluded, because first-write-wins stops a duplicate but NOT a second edge to a different target. - Honors `fieldFallbackOnMethodLookup`. A statically-typed language opts out of name matching precisely because it over-connects; inferring an ACCESSES edge by name is the same claim and must obey the same opt-out. - Requires an explicit receiver — a bare identifier is not a property access, and matching one by name would link a local to an unrelated key. Indexes graph nodes rather than scope defs because an object-literal key mints a Property NODE but no scope-resolution DEF: `localDefs` and `scope.bindings` are both empty for exactly the population this serves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(analyze): record a collapsed graph write instead of reporting fresh The dangerous half of a broken refresh: metadata IS written, so the index reads as fresh, hooks re-arm, and every tool answers from a graph missing most of its edges — indistinguishable from a codebase that genuinely has no such relationships. Reported in the field as edges collapsing 23009 -> 2170 and as a CodeRelation table that never materialized. `analyze` now compares the relationship count the pipeline PRODUCED against what the DB hands back after the write. Both numbers are already in scope at the same point, so the shortfall is provable rather than inferred — no comparison against the previous index, which cannot distinguish a failed write from a repo that legitimately shrank. A missing relation table needs no special case: it reads back as a persisted count of zero. On a collapse the run records `graphWriteCollapsed` in metadata, which `getIndexIncompleteReasons` turns into `graph-write-collapsed` so status and the MCP resources report the index INCOMPLETE rather than fresh. A ratio, not equality: some relationship types do not round-trip one-for-one and `--pdg` writes MORE rows into the same table, so demanding equality would fire on healthy runs. Only a collapse is a defect. Fail-safe when the expected count is unavailable — an implementation that offloads relationships out of memory may not be able to report a total, and a false "your index is broken" is worse than a missed one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(ingestion): qualify object-literal Property ids by their owning object Two config objects in one file that share a key name generated the same `Property:<file>:<key>` id and COLLAPSED INTO ONE node, so two distinct settings became a single symbol. Worse, the merged name then looked workspace-unique to name inference, which happily resolved reads of it to a node representing both — a wrong edge in the pre-edit safety gate, which is precisely what the unique-name pass is bounded to avoid. `objectLiteralOwnerInfo` already existed for exactly this ("so two constructors in one file that both define `bar` stay distinct nodes") but was gated to `Method`. `Property` now opts in. `findObjectLiteralBindingInfo` returns `ownerName` only when asked. Its `Method` ids must stay byte-identical — qualifying them would rewrite every object-literal method id in every indexed repo — while object-literal KEYS, indexed only since A1/A5, have no such history to preserve. Found by a test written for the ambiguity path rather than by review: the suite reported one node where two were expected, and an edge where none should exist. Both are now pinned, along with the detection boundaries of the B2 collapse check, which was previously an untestable inline expression and is now a pure function. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(typescript): index type aliases and shape members as symbols A TS frontend models its API contracts as `type X = { … }` and `interface`, so a field on one is exactly what "who breaks if I remove this?" is asked about. Three gaps made that unanswerable, all in the TypeScript queries: 1. No `type_alias_declaration` -> `@definition.type`, so an alias minted NO NODE AT ALL and a context() lookup on an exported contract type answered "Symbol not found". TypeScript was the ONLY language missing this — Rust (type_item), Kotlin (type_alias), Swift (typealias_declaration) and Dart all emit it. The alias was declared for scope resolution but never became a graph symbol. 2. No `property_signature` in the parse query, so INTERFACE members minted no Property nodes either — the upstream report's "class/interface index fine" holds only for the type, not its fields. 3. No `property_signature` in the scope query, so even with nodes present the resolver had no member declaration to aim at. Its sibling `method_signature` -> `@declaration.method` already existed; only properties were missing. Interface bodies and object-type aliases both spell members as property_signature, so one pattern per query covers both shapes. Lands the SYMBOLS, not yet the ACCESSES edges: the shape is already a class-like scope and now has member declarations, but no edge forms — the remaining link is owner/type-binding, recorded as todos with the diagnosis. Note TypeScript sets fieldFallbackOnMethodLookup:false, so unlike JavaScript there is deliberately no name-based fallback here; the precise path is the only route by design. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(golden): accept interface members in the mini-repo snapshot Drift is entirely the new TypeScript shape-member indexing: the fixture's three interfaces (ValidationResult 2, DbRecord 3, LogEntry 3) contribute exactly 8 Property nodes, each with exactly one HAS_PROPERTY owner edge. Verified before regenerating rather than after: every pre-existing count is untouched (CALLS 9, IMPORTS 12, DEFINES 16, HAS_METHOD 1, MEMBER_OF 12, STEP_IN_PROCESS 12), so nothing was rewired — the digest moved only because 8 edges were added. The fixture's inline `return { valid: false, … }` literals correctly produced nothing, confirming the object-literal rule stays scoped to variable-bound literals. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(analyze): never report a collapse from a non-numeric count The B2 check reported healthy runs as total graph-write collapses. A non-numeric `expected` (a graph implementation reporting no total, a lightweight pipeline result) does not skip the guards — it INVERTS them: `undefined < 100` is false, so the small-repo exemption never fires, and `0 >= undefined * 0.5` is `0 >= NaN`, also false, so the ratio check "passes" as well. Both bounds silently evaporate and every such run is flagged. That is precisely the failure this check was written to catch, reproduced inside the check itself: an unmeasurable quantity treated as a measured zero. Both sides are now validated as finite numbers before any comparison. `persisted` is also passed as UNKNOWN rather than zero when the DB was not demonstrably readable: `getLbugStats` flattens "no connection", "query threw" and "empty table" all into `edges: 0`, so `stats.nodes > 0` is used as independent evidence the read happened at all. Caught by the existing run-analyze suites, not by the new unit tests — those exercised the pure function with well-formed numbers and were blind to the integration's actual inputs. Both cases are now pinned. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(typescript): make object-type aliases own their members A TS object-type alias declares the same `property_signature` members as the interface beside it and answers the same question, but was not a member owner: its fields were minted with bare ids and no owner edge, so two aliases in one file sharing a field name collapsed onto one node, while the identical interface resolved normally. `type_alias_declaration` joins CLASS_CONTAINER_TYPES (and CONTAINER_TYPE_TO_LABEL, as that set's invariant requires — a container missing there gets orphaned member edges or a wrong owner label). Aliases with no object type (`type Id = string`) declare no members, so they own nothing and are unaffected. This also lands the INTERFACE field -> consumer edges, verified on the mini-repo fixture rather than only on a purpose-built one: `saveToDb` now links to `ValidationResult.value`, and `formatLogEntry` to `LogEntry.level` and `LogEntry.message` — three real contract-field reads that previously had no graph path at all. Golden updated: +3 ACCESSES, no node changes. The ALIAS field -> consumer edge is still not linked and is recorded as a todo with the exact blocker: resolving a receiver typed as the alias needs the NAME to resolve to a class-like def, and `isClassLike` is Class|Interface|Struct|Record|Enum|Trait. That predicate is read from ~12 sites including MRO and heritage, and every language mints TypeAlias, so widening it would enrol aliases in linearizations where they do not belong. Widening only the scope index was tried and reverted — the type-name walkers gate on it independently, so it fixed nothing and left dead code. That needs a deliberate "shape-like" concept, not more call-site widening. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(test): record the traced diagnosis for the unlinked alias field edge Traced to the end rather than left as "needs investigation", so the next attempt starts from facts: 1. Graph side is COMPLETE and symmetric with the interface — Property:...:LiveModeConfig.bookSlots is owner-qualified and carries HAS_PROPERTY. 2. Resolution DOES reach resolveClassBindingForName('LiveModeConfig') (instrumented) and misses. 3. It misses because the module scope binds LiveModeIface:Interface, renderAlias, renderIface — and not LiveModeConfig. The alias has no binding on the receiver's scope chain at all. 4. The TS scope query tags aliases @declaration.type, but normalizeNodeLabel accepts only typealias / type_alias and has no "type" case, so it returns undefined. Kotlin and Dart use @declaration.type_alias; TypeScript is alone on the dead tag. 5. Retagging is NECESSARY BUT NOT SUFFICIENT — tried, and the binding still does not appear, so a second gate exists in how a declaration anchored on a node that is ALSO a @scope.class anchor is attached: the alias appears to bind inside its own scope rather than hoisting to Module, where interface_declaration evidently does hoist. An isShapeLike predicate (the nominal-vs-structural split: shapes declare members, nominal types participate in MRO) plus a mirrored findShapeBindingInScope were built and REVERTED along with the retag. With no binding on the chain they never fire, and shipping inert widening is worse than shipping none — the same standard applied to the earlier scope-index attempt. The design is recorded here; it is worth doing once step 5 is fixed, and it also unblocks Rust's parked union_item, which the MEMBER_OWNER_NODE_TYPES comment documents as the same gap in another language. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(scope-resolution): resolve cross-file value references, skip block-locals Two halves of the same question, "who uses this constant?". CROSS-FILE. `resolveReferenceSites` runs against the registries and, as its own comment says, "imports live in finalized bindings the registries can't see" — which is why free CALLS need `emitFreeCallFallback`. Reads had no counterpart, so `import { LIMIT }` followed by a bare use resolved to nothing while a CALL through the very same import statement resolved fine. This adds the read/write counterpart, reusing `findValueBindingInScope` (which walks the FINALIZED chain) rather than inventing a lookup. Confidence 0.9: the import names the def, so this is precise resolution, not inference. BLOCK-LOCALS. Bare-identifier capture also matches a read of a block-local `const`, and an edge to one keeps alive exactly the inert locals `pruneLocalSymbols` exists to drop — a pruned node becomes a retained node plus an edge, in every function of every indexed repo. Emission now takes the set of value defs bound at MODULE scope and drops ACCESSES to Const/Variable/Static outside it. The cross-file pass carries the same guarantee structurally: a def in another file cannot be a block-local of this one, so it skips same-file hits entirely. The block-local leak was already shipped in the intra-file A2 commit and was found only because a test was written for the guard rather than the feature — the same way the object-literal id collision surfaced. Verified on the full resolver matrix: 3172 tests, golden unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(lbug): diagnose a vanished staging CSV instead of surfacing a Binder error A forced rebuild could fail with "COPY failed for File: Binder exception: No file found that matches the pattern .gitnexus/csv/file.csv" and then an ENOENT on .gitnexus/csv/rel_Folder_File.csv — two engine-level messages that name neither a cause nor a remedy, which is where several field reports end. Only tables with rows > 0 enter the COPY manifest (csv-generator.ts), so an absent file was WRITTEN during this run and removed since. Both COPY loops now preflight and say exactly that, with the row count, both causes the reports point at (a second `gitnexus analyze` on the same repo — they share .gitnexus/csv — or an external cleanup of .gitnexus/), and the action to take. Scope note, deliberately narrow: this does not attempt to fix WAL corruption or checkpoint rotation. Those already have detection and recovery hints (isWalCorruptionError, WAL_RECOVERY_SUGGESTION, the configurable wal-checkpoint-threshold), and the ~6000 lines added to lbug/ + storage/ since v1.6.9 — index-lock.ts most of all, which serializes writers and plausibly closes the concurrent-run class outright — postdate every report in the window. Guessing at unreproducible durability faults would be speculation; making the one failure with NO handling legible is not. An existing overlap test induced this exact scenario (a manifest entry pointing at a missing csv) and asserted on the engine's wording. Its intent — that a node-COPY failure is rethrown at the FK barrier rather than swallowed — is unchanged and still asserted; only the message it matches moved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(scope-resolution): split shape-like from class-like, linking alias fields Completes A4: a field on a TypeScript object-type alias now links to the code that reads it, the last unanswerable half of "who breaks if I remove this?" for a TS frontend that models contracts as `type X = { … }`. `isClassLike` answered two questions that only coincide for classes: 1. does this declare MEMBERS I can look up? — a SHAPE (structural) 2. does this participate in inheritance / MRO? — a NOMINAL TYPE An object-type alias is (1) and emphatically not (2) — it has no supertypes and no place in a linearization. Widening `isClassLike` to buy (1) would have enrolled every language's aliases (Rust type_item, Kotlin/Swift/Dart typealias, C typedef) into MRO and heritage, so the two questions now get two predicates. Call sites split by which they ask, and their names already said which: `resolveInheritanceBaseInScope` and `resolveQualifiedInheritanceBase` keep `isClassLike`; receiver typing and member OWNERSHIP take `isShapeLike`. Three parts, each necessary and none sufficient alone: - `findShapeBindingInScope`, mirroring `findValueBindingInScope`'s established relationship to `findClassBindingInScope` (same walker, different accepted def-type), consulted only AFTER the class lookup misses so a class of the same name always wins. - `populateClassOwnedMembers` uses it, so alias members get an `ownerId` and are registered under the alias. Without this the receiver resolved to the alias and then found no members under it. - The TS scope query tags aliases `@declaration.type_alias`, not `@declaration.type`: `normalizeNodeLabel` accepts typealias / type_alias and has no "type" case, so the old tag mapped to NO label and TypeScript aliases produced no scope-resolution def at all. Kotlin and Dart already spelled it this way; TypeScript alone was on the dead tag. An earlier attempt concluded a further "scope-attachment gate" existed. That was wrong and is worth recording: scope extraction runs in the parse WORKER, which loads built `dist`, so the retag was never executed. Rebuilt, the alias hoists to Module scope exactly as the interface does. Same trap as the parse query — `src` edits to anything the worker runs are invisible until `npm run build`. Typedef and Union stay out of `isShapeLike` deliberately: they belong conceptually (the union_item note on MEMBER_OWNER_NODE_TYPES records the same gap) but neither is wired as a member container, so including them would widen a predicate nothing exercises. Verified on the full resolver matrix: 3173 tests, golden unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(typescript): pin the type-alias capture to a tag that maps to a label The capture test asserted `@declaration.type`, the tag that `normalizeNodeLabel` does not recognize (it accepts typealias / type_alias and has no "type" case). So the test passed for as long as the tag was broken: it checked only that the capture FIRED, never that it resolved to anything, while TypeScript aliases produced no scope-resolution def at all. Updated to the working tag and given a second assertion that the derived kind string is one the label mapper accepts — the property that actually matters, and the one whose absence let a dead tag sit pinned. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(lbug): declare TypeAlias member pairs so analyze does not abort Making object-type aliases member owners emits HAS_PROPERTY from a `TypeAlias`, and the relation schema declared no such pair. The emit therefore threw `UndeclaredRelationPairError` and the ENTIRE analyze died on any repo containing `type X = { ... }` — a hard stop, not a dropped edge. Found by running the analyzer over a real 16k-node TypeScript repo, not by a test. `Method` is declared alongside `Property`: a member written `type Handler = { onClick(): void }` is a method_signature and would fail in exactly the same way. Why every existing test missed it: the resolver suites build an in-memory graph via `runPipelineFromRepo` and never write to LadybugDB, so the schema constraint was never exercised. `structural-pair-coverage.test.ts` is the one suite that does run the emitters against the declared pairs — and its own docstring names the gap: coverage is bounded by NON_BRIDGE_CORPUS, "a new structural emitter should land with an entry here". This adds that entry, pinning TypeAlias|Property and Interface|Property as sentinels. Verified the guard is not vacuous: removing the pair again makes the suite fail with undeclaredPairs: ["TypeAlias|Property"]. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(processes): trace depth-first so multi-hop flows are detected D1 ("query ranks frontend components above the backend module that owns the concept") and D2 ("processes is dominated by trivial mechanical chains") are the same defect, and neither is about ranking or selection. The walk stops after a fixed NUMBER of traces, so traversal order decides which traces those are. Breadth-first reaches every shallow terminal before any deep one, so the quota filled with the shortest paths in the graph and the walk stopped — `maxTraceDepth: 10` was never approached. Measured on a real repo before the fix: of 300 processes NONE exceeded 7 steps and 90% were 3-4. A multi-hop business flow (signal → order → exit) therefore had no process that could represent it, and `query` could only rank the mechanical pairs that did exist. Step 4 of the caller already sorts by length and dedupes by endpoint — it was always asking for the deepest traces this walk could give it. Depth-first descends to a terminal first, so the same quota is spent on paths worth keeping. Cost is unchanged: same budget, same cycle guard, same depth ceiling — only the order differs. Measured on the same 16k-node repo, same build and flags, BFS vs DFS (an earlier comparison was discarded as confounded — it crossed builds and --pdg): steps 6-8: 50 → 168 (3.4x) totals: 844 → 806 and the reported query moved from `LiveSetupView → Cn` (a React component) to `ReconcilePositions → IsTpInProfit / WithHeld / ShouldNotify` — server-side exit management, which is what was asked for. `traceFromEntryPoint` is exported for the test. Traversal order is unobservable through `processProcesses`: `findEntryPoints` supplies several starting points, so a deep chain is traced from inside it whatever the order does. A test at that level passes under BOTH traversals — the first version of this test did exactly that and guarded nothing. Driving the walk directly, it fails under breadth-first with "expected 3 to be greater than 3". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(test): correct a stale status note left behind by a later fix The A1/A5 header still said "edge resolution REMAINING ... neither is implemented". Both shapes resolve — the typeable receiver precisely, the untyped one by workspace-unique name — and the tests below assert exactly that, so the note contradicted the file it sat on. It was accurate when written and went stale when the work continued past it. Left as-is it would tell a reviewer that a landed feature is missing. The TRAP note is kept: the parse worker still runs built dist under vitest, and that is still the trap it describes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(scope-resolution): index literals behind identity-preserving wrappers `export const INERT_EXIT_CONTRACT = Object.freeze({ ... })` minted no `Property` node for any of its keys. The object-literal rule matches `variable_declarator > value: (object)` as a DIRECT child, and freezing puts a call expression in between — so the shape whose fields are most worth querying was the one shape the rule could not see. Freezing a config object is how JS publishes an immutable contract, which is why this reads as a confident zero on exactly the fields a reader cares about. The allowlist is three functions, not "any call". `Object.freeze`, `seal` and `preventExtensions` RETURN THE ARGUMENT THEY WERE GIVEN, which is what makes the literal's keys members of the bound name. For `const x = compute({ a: 1 })` the literal is an argument and `x` holds compute's return value, so attributing `a` to `x` would be a fabrication. Two negative controls, because the obvious one is vacuous: a bare-identifier callee is rejected structurally and would pass with no allowlist at all, so the assertion that actually pins the predicate uses `Object.entries` — identical shape, differing only by name. Verified load-bearing by adding `entries` to the allowlist and watching that test alone fail. SCHEMA_BUMP 46 -> 47: parse-time emission, so a warm cache replays the pre-fix capture set. Observed as a false negative first — `analyze --force` returned the old node set until the on-disk cache was removed by hand. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(scope-resolution): narrow multi-candidate property names by scope Workspace uniqueness was the wrong denominator. Measured on the reporting repo: `exitMinAtrMult` has 26 `Property` definitions — 16 in one-off `scripts/`, 7 in the frontend, one in a test, and exactly ONE in the backend that reads it. Every backend read was refused because of competitors the reader cannot see. The gate was not too permissive or too strict, it was scope-blind. A name with several definitions is now narrowed before being abandoned: same-file first, then files the reading file directly imports, using the finalized import graph rather than a path-shape heuristic. Exactly one survivor at the first non-empty tier resolves; anything else stays refused. A tier holding several candidates stops the walk instead of falling through — local evidence that is itself ambiguous still contradicts reaching further out. Confidence stays 0.5 at every tier. Narrowing changes which candidate is chosen, not the kind of claim: it is still a name match, and the round-1 contract is that filtering on confidence drops all name inference at once. The reason string now names the tier that fired. Ambiguity reporting goes from a count to the actual names (capped), because a count says a gap exists while the names say which fields are unanswerable. Measured on that repo, backend readers of `exitMinAtrMult` go 0 -> 24 and total readers 9 -> 45, including the two call sites in `oppositeSignalExitManager.js` the report singled out. Both narrowing tests were mutation-checked by dropping the import evidence and confirming they, and only they, fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(scope-resolution): capture destructured parameter keys as property reads `function exit({ exitMinAtrMult = 0 })` reads that property off whatever the caller passes, exactly as `cfg.exitMinAtrMult` would. It never appears in a member_expression, so it had no reference site at all — and this is the shape the function that IMPLEMENTS a behaviour uses, so the most relevant reader was the one systematically missing from "who reads this setting?". Uses a distinct `@reference.read.destructured` anchor rather than `@reference.read.member`. The latter is filtered emit-side to matches with a member_expression ancestor, because calls and writes share its shape, and a destructuring pattern has none — reusing the tag would have been silently dropped by that filter. The `read.` head already maps to a read kind, so no mapping change is needed. Scoped to formal_parameters. A destructuring binding elsewhere (`const { x } = require('m')`) is frequently an import rather than a field read, and minting a property read there would attribute module bindings to unrelated same-named keys. All three cases (default value, bare shorthand, renamed key) mutation-checked by removing the patterns and confirming those three tests, and only those, fail. The renamed case also asserts the edge points at the KEY and that the local alias mints nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(scope-resolution): link type consumers to the type they name An exported contract type owned its members after round 1 and still answered `incoming: {}`, so "what breaks if I remove this field?" — the question a contract type exists to answer — had no edge to walk. Measured on the reporting repo: all 324 TypeAlias nodes AND every Interface node had DEFINES as their only incoming edge. Two independent causes, and the second is why the first was not enough. TypeScript captured no type references at all — only cpp and csharp did — so an annotation naming a declared type minted no reference site. Added for annotations, generic arguments and `as` assertions, anchored to those contexts rather than a bare `(type_identifier)`, which would also match the name in `type X = …` and make every declaration a consumer of itself. That alone fixed interfaces and left aliases still empty. `TypeAlias` was missing from `LINKABLE_LABELS`, so alias graph nodes were never indexed in `nodeLookup` and `resolveDefGraphId` could not bridge a def to its node — the edge was dropped AFTER a successful lookup. `CLASS_KINDS` has always listed TypeAlias and the ClassRegistry returned the def correctly, which is what made this read as a resolution failure; instrumenting the lookup showed it returning the right def all along and moved the search one table over. Exactly the bug already documented two entries above it for Trait. Fixes every language that spells an alias this way — TypeScript, Kotlin, Dart and Rust all emit `@declaration.type_alias`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(scope-resolution): capture record construction as property writes The read side answered well after the narrowing work while "who SETS this field?" still missed the code that stamps the value. A record built inline — `return { exitContract: { exitMinAtrMult: settings.x } }` — is bound to no variable, so it minted no definition and its keys referenced nothing. Modelled as WRITE REFERENCES, deliberately not definitions. The round-1 rule already mints Property nodes for literals bound to a variable; minting more for anonymous records would add same-named competitors to the very name-narrowing that makes these fields resolvable — measured at 26 competing definitions for one field on the reporting repo, which is what made every backend read unanswerable in the first place. A construction site is a USE of a field, not another declaration of it. Two positions only: nested under a key, and returned. Both are records with a name attached (the key, or the function). An inline call argument (`doThing({ id: 1 })`) stays excluded for the same reason round 1 excluded it from definitions — it is call-site data, not a named surface — and is asserted as such. The enclosing literal is the receiver and it is anonymous, so these route through the same narrowing and the same refusal-to-guess as every other untyped receiver. Verified on the reporting repo: `entryPlan.js` went from no rows to `selectExitEnvelope` as a writer of `exitMinAtrMult`. Both captures mutation-checked. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(processes): select round-robin by terminal so the list is not one flow repeated Ranking was `sort by length` alone, so the top of the list was one behaviour described many ways: eleven of the top fourteen processes on the reporting repo were four entry points crossed with three terminals of the SAME date-window utility cluster. Genuine call chains, but a reader learns one thing from fourteen entries, and the repo's own domain flows sat below them. Selection now round-robins across TERMINALS, deepest first. Depth still orders within a terminal and still leads the list; what changes is that no terminal takes a second slot until every other has had a first. Keying on the entry point was tried first and made it worse — many files declare a `main`, so each was a distinct entry that round-robin then awarded its own slot, and `Main -> AlignWindowEnd` went from one row to eight. The repetition was never in where a flow starts. Measured on that repo: distinct terminals in the top 20 went 3 -> 20, and its domain flows (`ReconcilePositions -> ...`) moved into the top 4%. Two things this deliberately does not claim. The reported cause — ranking rewarding fan-in, promoting chains ending in widely-called helpers — measured FALSE: those terminals have one caller each (`alignWindowStart` 1, `validateSymbol` 1). A fan-in discount was implemented against that hypothesis, measured, and reverted for moving nothing. And a business flow still cannot be a process in its own right: the walk only emits at a leaf, at max depth, or on a cycle, so a flow whose meaningful endpoint calls onward survives only as whatever leaf it bottoms out in. Both are recorded in the code so neither reads as settled. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(structural-pairs): pin the type-annotation USES pair R2-2 emits USES INTO a `TypeAlias`, so the pair is `Function|TypeAlias` — a different table from the `TypeAlias|Property` entry added in round 1, and one that entry stays green without. `TypeAlias` is on the eleven-table list this suite exists for, and an undeclared pair does not degrade: it throws `UndeclaredRelationPairError` and kills the entire analyze on any repo containing an annotated type. Every resolver suite still passes, because they build an in-memory graph and never write to the DB. That exact failure shipped once in this PR already. Two emitters into the same label, each with its own way to reach a released build. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(scope-resolution): build the module-level set before the out-of-core seal Review blocker. Under `GITNEXUS_DISK_SCOPE_INDEX=1` the seal replaces every ParsedFile with a scope-STRIPPED copy, and the block-local filter's set was built after it — so it walked `scopes: []` for every file, came out empty, and the filter read that as "no def is module-level" and dropped EVERY `Const`/`Variable`/`Static` ACCESSES edge in the repo. All languages, all files, including the module-scope-const edges this PR exists to add. Nothing threw and nothing logged, on the path the largest repos take: the exact confident-empty answer the PR is about. Built above the seal now, from `parsedFiles`, and passed as `undefined` rather than an empty set when no scope was inspectable — an empty set is a legitimate answer ("this repo has no module-level value defs") and must not be indistinguishable from "could not look". Fails open; the block-local exclusion is still asserted under the seal, since that is correctness rather than optimization. Also widens module level past `kind === 'Module'`. A `Namespace` scope (TS `namespace`, Rust `mod`, C++/C# `namespace`) holds importable values too, and treating its consts as function-locals dropped their reads. Included only when the whole chain to the root is Module/Namespace, so a namespace declared inside a function body stays local — asserted both ways. That fixture then failed for a third reason: `@reference.read.identifier` existed ONLY in the JavaScript query, so A2 did not work for TypeScript at all. Added there, and both languages widened to `variable_declarator value:` and `binary_expression` operands — the gaps review named between what A2 claimed and what it matched. Nothing covered `GITNEXUS_DISK_SCOPE_INDEX`. The new parity test asserts the seal changes no edge, and was verified against an emulation of the original bug: same-file readers vanish and only the cross-file reader survives. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(typescript): anchor property_signature to declared shapes Review blocker, and it reproduces end to end. `property_signature` occurs in EVERY object_type in the TS grammar, not only in an interface body or an alias's object type, so inline parameter types, inline return types and nested object types all matched — and the enclosing-container walk hung each one off the nearest class, interface or alias. Measured against the unanchored rule, all four appeared as members of shapes that do not have them: Property:contracts.ts:Svc.inlineParamOnlyKey Property:contracts.ts:Repo.inlineQueryOnlyKey Property:contracts.ts:NestedConfig.nestedOnlyKey Property:contracts.ts:buildInline.inlineReturnOnlyKey@46:33 When the inline member shares a name with a real one — `run(opts: { retries: number })` inside a class that declares `retries` — `addNode` is first-write-wins and the two distinct symbols merge onto one node, so every context()/impact()/rename() answer about that field describes the merge. The sibling JS object-literal rule in this same PR is anchored for exactly this reason; this is the TypeScript half of the same fix. `(A (B))` matches DIRECT children, so nested object types are excluded by the same anchor rather than by a second rule. The first version of these tests was VACUOUS and is recorded here because the reason generalizes: a collision and a correct exclusion both leave exactly one node behind, so counting ids cannot distinguish them. Every inline member in the fixture is now uniquely named, which is the only thing that discriminates — verified by restoring the unanchored rule and watching exactly those four assertions fail. A fifth test asserts anchoring costs no real member. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(analyze): correct the numbers feeding the graph-write-collapse guard Review blocker. The predicate itself held under adversarial probing; every defect was in what it was handed and what happened after it fired. (a) `expected` was wrong twice. Under `GraphEmitSink` streaming the bulk types leave the heap at parse time and never enter `relationshipCount`, so the count understated the real volume by most of it and the ratio passed trivially — on `force === true` runs, which include crash recovery AND the `analyze --force` retry this check's own warning tells the operator to run. Adds the manifest totals, the same correction the buffer-pool hint in this file already makes for the same reason. Separately, an incremental run persists only the changed subgraph while both counts are whole-scope: a 10,000-edge index that lost 200 replacements reads 9,800 and is certified complete. The check is skipped on that path rather than answered wrongly. (b) A throwing edge count became a measured zero. `getLbugStats` initialised its total to 0 and ran the query in a swallowing catch, so WAL/lock contention during finalize — documented on this exact call — reported a healthy index as a total collapse. It now returns `number | undefined`, and the caller requires both a readable node count and a defined edge count. (c) A total loss was exempted for being small. The min-edges rule tested `expected` before looking at `persisted` at all, so `expected = 99, persisted = 0` — every edge gone — stayed fresh and reported success. Total loss is now decided first. The existing test asserted the defect; it now asserts a PARTIAL shortfall, which is the case the exemption was written for. (d) A detected collapse reported success and exited 0. It is different in kind from the other incomplete reasons: those describe a run that did what it said and left work for later, this one means most of your edges are gone and every query answers a confident empty. The CLI now prints INCOMPLETE with the counts and sets a non-zero exit code, and the flag crosses IPC so the worker cannot send a clean `complete` either. Nothing exercised this wiring — only the pure helper. Adds tests for all four, each written so the pre-fix arithmetic fails it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(scope-resolution): keep unique-name property inference inside one language The pass indexed `Property` nodes from the whole shared graph. Per-language gating decides whether it RUNS for a language; it never restricted which nodes could be TARGETS. So the only carrier of a name could be in another language entirely, and a read here resolved to it on name uniqueness alone — no owner, no file, no call path. Reproduced: a Java class declaring `private int loyaltyPointsBalance` and a JS `cfg.loyaltyPointsBalance` on an untyped parameter produced an ACCESSES edge from the JS function to the Java private field. Confidence does not mitigate it, because `minConfidence` defaults to 0 — the tier is only a filter for consumers who ask for one. Candidates are now restricted to files in the language's own `parsedFiles`, which is a precise restriction rather than a heuristic and needs no new node property. Every other fixture in the suite is single-language, so this could not be caught anywhere by construction. The new fixture is deliberately polyglot and asserts both halves: no cross-language edge, and a same-language unique name still resolves. Known and not addressed here: the index is still O(total graph nodes) and is rebuilt once per qualifying language, the per-language whole-graph-scan pattern `phase.ts` hoisted out for `sharedNodeLookup`. Hoisting it belongs with that machinery rather than in this fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(processes): explore siblings in source order, log the exhausted budget `slice(0, maxBranching)` selected the FIRST N callees while `pop()` explored them LAST-first, so the trace budget went to the last-declared branch. For `main() { init(); loadConfig(); run(); shutdown(); }` the walk spends itself on `shutdown` and can drop `init` — the earliest steps of a flow, which is the opposite of what a process describes. Selecting first-N and exploring last-first was simply inconsistent; pushing in reverse makes the stack pop in source order. Measured on the reporting repo, this costs depth: 6-8 step processes go 168 -> 146 of 816. Still roughly three times the pre-PR baseline of 50, and the right trade — a deep branch is no longer reached by accident of being declared last. The remaining limit is the BUDGET, not the traversal: with a fixed quota a deep branch declared after enough shallow ones is not reached at all. That is now asserted in both directions rather than left implicit, and the walk logs when it stops with branches unexplored — a silently truncating cap reads as "this is everything", the same confident-empty answer this work is about, and the repo already sets that precedent for `dispatchFanoutSkipped`. Removes the second depth test, which was vacuous: the note twelve lines above it already said a `processProcesses`-level depth assertion passes under BOTH traversals, and measured it does — breadth-first yields the same deepest stepCount of 8, so it passed with the production change reverted. Traversal order is asserted against `traceFromEntryPoint` directly; what is observable at the pipeline level is which traces survive selection, which the diversity tests cover. Also renames `queue` to `stack` and corrects the BFS references in the module docstring and the function's own JSDoc, which is what an IDE hover shows. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(impact): carry riskNote onto ambiguous candidates and separate UNKNOWN's two meanings Two problems on the ambiguous fan-out, which builds its own candidate object rather than returning the single-symbol shape. The narrowed type had no `riskNote` field and never read one, so a candidate that resolved and found no callers reported `risk: UNKNOWN` with nothing attached — losing the entire point of the change on the path where the reader has the least context, since the name is ambiguous there by definition. And `UNKNOWN` used to mean exactly one thing on this path: the probe threw. The zero-caller branch gives it a second meaning, so an all-UNKNOWN fan-out could no longer be told apart from a broken one. Candidates now carry `probeFailed`, and the comment asserting the old reading is corrected. Also aligns `gitnexus-web`, which review flagged as giving a different verdict for the same symbol. That surface answers in prose rather than an enum, and its message said the symbol "appears to be unused (not called by anything)" — the identical false certainty in words. It now carries the same MEANING rather than the same field. Downstream wording is unchanged: no outgoing dependencies really is a fact about the symbol itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test: replace assertions that cannot fail Four from review, each satisfied by the defect it was meant to catch. `new Set(props).size === 2` over two different literal strings can only ever be 2, so it could not detect the node merge its title promises — that is a difference in COUNT, now asserted on the raw array. The ambiguity test asserted only an empty edge set, which is satisfied equally by "the gate fired" and "the name was never looked up". It now also requires the ambiguity counter to have moved. `Interface|Property` was listed as a structural-pair sentinel beside `TypeAlias|Property`, but both its labels are in the SCOPE_BRIDGE cross-product so the pair is generated by construction and the sentinel cannot fail. Dropped rather than left reading as coverage; `TypeAlias|Property` is the load-bearing one. `TypeAlias|Method` was declared in the schema with no fixture emitting it — a declared pair no emitter exercises is indistinguishable from a missing one until an analyze aborts on a real repo. Adds a method-shaped alias member, and the suite requires sentinels to actually appear, so it is not vacuous. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: document the new incomplete reason, the UNKNOWN verdict and the id churn Review found the code changes landed without the guidance around them, and an agent following this repo's own rules would have been told the wrong thing. `graph-write-collapsed` joined `INDEX_INCOMPLETE_REASONS` with no Sign block and no recovery section, while the precedent it cites (`embedding-checkpoint-pending`) has both — so `gitnexus status` would surface a new string naming silent wrong answers with nothing explaining trigger or remedy. Added to RUNBOOK and GUARDRAILS, including why this reason alone also fails the exit code. `AGENTS.md` said MUST warn on HIGH or CRITICAL and never mentioned UNKNOWN, and the shipped impact skill's risk table had no UNKNOWN row and still implied few-callers ⇒ LOW. An agent obeying those rules literally sees `risk: UNKNOWN` and proceeds, which negates the change the verdict exists to make. Both copies of both skills updated. `MIGRATION.md` now records that process ids do not survive this release — positional ids plus depth-first tracing, source-order siblings and round-robin selection mean `proc_7_handle` is a different flow afterwards. Bounded honestly: nothing in-repo joins on a raw process id, so it is index churn, not a broken consumer. `ARCHITECTURE.md`'s scope-resolution stage list gains the two new stages. The guide skill's node list gains `Property` and `TypeAlias` — the two node types this work most prominently creates. Also, on the pair-CSV preflight review asked to confirm: the hard abort IS deliberate, because a fallback recovering zero rows is the confident-empty failure this work targets. But the transient the message itself names — a second concurrent analyze sharing `.gitnexus/csv` — is a race, so the check now re-looks three times over ~150ms before declaring the file gone. Long enough to ride out a rename, far too short to mask a file that is genuinely missing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: drop redundant TypeAlias pairs and keep bare identifiers off class members Two regressions the full suite caught after the review fixes, both real. `schema-pair-coverage` failed with eleven hand-declared pairs that a rule now generates. Adding `TypeAlias` to `LINKABLE_LABELS` — needed so `resolveDefGraphId` can bridge an alias def to its node — also makes it a SCOPE_BRIDGE source and target, so the cross-product produces `File|TypeAlias`, `TypeAlias|Property` and nine others that round 1 had declared by hand. Removed; the invariant is that no pair is both generated and hand-declared. This also changes what the structural-pair sentinel means, and the comment is corrected rather than left overstating it: `TypeAlias|Property` is no longer load-bearing because the label is off the generated grid — it is load-bearing because it now depends on `TypeAlias` being IN `LINKABLE_LABELS`. Remove it and the pair stops being generated while the hand declaration is gone, which is the same state that silently breaks alias consumer edges. `block-scope-shadowing` failed because a bare identifier resolved to a class `Property`. `class Box { baseUrl = '...'; pick() { const baseUrl = ...; return baseUrl; } }` linked the block-local read to `Box.baseUrl`, duplicating the legitimate `this.baseUrl` edge. A bare identifier is not a member access: with no receiver there is no object whose property it could be, and in JS/TS a field read needs `this.`. Receiver-less read/write sites no longer accept `Property` hits; callables stay reachable, so `cb = save` naming a top-level function is unaffected. That defect PREDATES this branch's TypeScript captures — JavaScript has emitted bare-identifier reads since A2 and no class fixture exercised the shadow. The TS parity added here is what surfaced it. Golden snapshot regenerated after verifying the drift line by line: exactly +5 USES from type annotations in the mini-repo, every pre-existing count unchanged, so nothing was rewired. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * perf(scope-resolution): share the property-name index across language passes Review follow-up. `indexPropertyNodesByName` scanned every node in the graph and was rebuilt inside each qualifying language pass, reintroducing exactly the pattern `phase.ts` hoisted out for `sharedNodeLookup` — whose comment records why it matters: "the previous per-language rebuild burned that CPU+heap N times and, on a huge repo, a tiny language's full-graph copy overlapped the next language's — a real contributor to the scope-resolution memory peak." Built once in `phase.ts` beside `sharedNodeLookup` and `sharedFnNodeIndex`, and threaded through the same `prebuilt*` seam, so tests and isolated calls still build their own. Sharing is only safe because the per-language restriction MOVED rather than disappeared: the shared index is whole-graph, and candidates are filtered to the language's own files at lookup time. That also fixes a subtlety the per-language build had backwards — the cap now applies to the FILTERED set, so a name carried by forty properties across a polyglot monorepo but only two in the language being resolved is still answerable, where a global cap would have refused it. The tri-state at the lookup boundary is deliberate and the three outcomes are not interchangeable: no property of this name in this language (nothing to say, and NOT an ambiguity), too many to choose between (reportable), or a list to narrow. Caught mid-change by the polyglot fixture: an intermediate state shared the index without moving the filter, and the cross-language edge came straight back. That test earning its keep twice is the reason it exists. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(scope-resolution): report when a field's only anchor is another language Round 3, found OUT-OF-SAMPLE — six field names appearing in no prior report, so nothing here was tuned against them. All six answered 0 backend ACCESSES while their definitions sat in `apps/research-dashboard/**`: TypeScript only. The in-sample set scored 5/5 and the out-of-sample set 0/6, and the gap is entirely this. Per-language inference (`3c5eadc7`) is right and stays. What was wrong is that declining is INVISIBLE: an empty result for a field anchored only in TypeScript is byte-identical to an empty result for a field nobody reads. One says "look in the other language or grep"; the other says "delete it". That is the same confident-empty failure this series exists to remove, one surface over — and this time the missing fact is about the ANALYZER's reach rather than the code. Declines are now counted and named, with the languages the anchors actually live in, kept SEPARATE from ambiguity because the remedies differ: ambiguity wants better receiver typing, this wants an anchor in the reading language. Collapsing them would tell a reader the wrong thing to do. A non-zero count warns at analyze time regardless of dev mode. The facts are published as `PipelineResult.propertyInference`, which they had to be for any of this to be testable — and that exposed a second defect. The round-2 ambiguity assertion, which I told the reviewer of #2856 I had strengthened, read its stat off a `scopeResolution` field that does not exist on PipelineResult: the `if (undefined) return` guard swallowed it and the test passed with the production code deleted. Both that assertion and the new ones now read the published field, and the guard is an assertion rather than an escape. Verified by deleting the counter and watching them fail. Reported by the same round-3 method note that caught it: verifying a fix against the cases it was written for only proves those cases pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(context): explain an empty property result caused by a cross-language anchor The other half of R3-1. The analyze pass now knows which fields it declined to link because every definition of the name lives in another language; this puts that fact where it is actually read. `context()` on such a field previously returned an incoming list byte-identical to a genuinely unread field. The two demand opposite actions — "look in the other language, or grep" versus "delete it" — so the difference has to travel with the answer: unresolved: property reads of this name were NOT linked: every definition of it is typescript, and name inference does not cross languages. An empty or short incoming list here is not evidence the field is unused — confirm with a text search, or give it an anchor in the reading language. anchorLanguages: ['typescript'] Carried through repo meta because the graph cannot answer it: the unlinked reads mint no edge and no node, so the only record is the pass that declined them. Keyed on the NAME, not on the resolved label. Gating on `=== 'Property'` was tried first and is wrong — the label reads `''` on this path for a plain Property node, so the gate silently suppressed the entire feature while every test still passed. Caught by asserting the field is DEFINED rather than guarding on it, which is the same anti-pattern that made two earlier assertions vacuous. The meta list only ever contains property names, so matching the name is itself the type check. Cached per (index, indexedAt): `ensureInitialized` deliberately avoids a per-call `loadMeta` because every tool routes through it, so this re-reads exactly when a re-analyze could have changed the answer and never otherwise. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(scope-resolution): report declined property reads for opt-out languages too Generalizing R3-1 rather than waiting for it to be re-reported in the other direction. The reported case was a JavaScript read whose only anchor was TypeScript; the mirror — a TypeScript read anchored only in JavaScript — was still silent, because a language that sets `fieldFallbackOnMethodLookup: false` had the whole pass skipped, and skipping emission also skipped REPORTING. Detection is not inference. Counting what could not be linked asserts nothing about what it means, so `reportOnly` runs the pass for its facts while emitting no edge, and the opt-out keeps protecting exactly what it protected before. Two things this turned up that a single-instance fix would have missed: The cross-language fixture could NOT prove `reportOnly` is load-bearing — the per-language candidate filter already blocks those edges, so the assertion passed with the flag forced off. The case that discriminates is a SAME-language TypeScript read that name inference could legitimately link and the opt-out forbids; forcing the flag off there emits `readsTsOnly -> tsOnlyBudget`, which is the violation. Getting to that case surfaced a sibling gap, recorded but NOT fixed here: the object-literal `Property` rule is JavaScript-only, so `const CONFIG = { ... }` in a `.ts` file mints no node and its keys are invisible. The first draft of this fixture used exactly that shape and could not discriminate for that reason. It is the TypeScript half of R2-1a and wants its own change, not a rider on this one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(typescript): index object-literal keys, as JavaScript already did The sibling recorded in `0c5a4f64` and deliberately left out of it. Both the named object-literal rule (A1/A5) and the identity-wrapper rule (R2-1a) lived only in JAVASCRIPT_QUERIES, so the single most common config idiom in TypeScript — export const tsRuntimeConfig = { tsConfigRetries: 3 }; — minted no node for any key. `context()` answered "Symbol not found" and a precise read through the holding variable had nothing to resolve to. TypeScript sets `fieldFallbackOnMethodLookup: false`, so these gain no name-based inference. What they gain is the PRECISE path, which is the route TypeScript is meant to use: `tsRuntimeConfig.tsConfigRetries` has a typeable receiver and now resolves. A read through an untyped receiver stays unresolved and, since `0c5a4f64`, is reported as such rather than answering an empty set. Scoped exactly as the JavaScript rules are — bound to a variable, and for the wrapper only the three functions that return the argument they were given — with the same `Object.entries` negative control pinning the allowlist. Found by fixture, not by report: the first draft of the `reportOnly` test used a TS `const CONFIG = { ... }` as its discriminator and could not discriminate, because the shape mints nothing. That is the whole argument for sweeping a class instead of waiting for each instance to be filed. SCHEMA_BUMP 47 -> 48: parse-time, so a warm cache replays ParsedFiles carrying none of these matches and the keys stay invisible. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(scope-resolution): anchor anonymous returned object literals to their function The last gap round 3 named, and the dominant shape in idiomatic JS: 437 `return {` sites in a single backend directory of the reporting repo, including the ~25-field payload of its entire signal pipeline. The literal binds to nothing, so its keys could not even be named — "who reads wickRatio?" had no symbol to ask about. The enclosing FUNCTION is the owner: the literal is that function's return shape, a contract its callers consume. Keys qualify as `<function>.<key>`, so two functions returning the same name stay two shapes rather than one merged symbol, and multiple returns in one function stay distinct by position. RECONCILING THIS WITH R2-1b, which deliberately modelled returned keys as WRITES to avoid adding same-named competitors to narrowing. These are definitions, but narrowing now ranks DECLARED anchors — named literals, class fields, interface and alias members — strictly above return shapes. A name that already resolved keeps resolving to what it resolved to before, so the competitor problem R2-1b was avoiding cannot come back. Mutation-checked: dropping that ranking breaks five pre-existing R2 resolutions. That also required an R2-1b assertion to change, and the change is a strengthening rather than a concession. It asserted `toHaveLength(1)` — no new definition — as a proxy for "adding definitions must not move an existing answer". The proxy is now false while the property still holds, so the property itself is asserted directly. No `HAS_PROPERTY` edge from the function: that would be a `Function|Property` relation pair the schema does not declare, and an undeclared pair does not degrade — it throws and kills the whole analyze. That already shipped once in this PR. Two things found by dumping rather than assuming, both fixed here: SHORTHAND keys were not matched at all. `return { symbol, interval, score }` is the commonest spelling and the reporting repo's own payload is mostly this form, but tree-sitter models it as `shorthand_property_identifier`, which `(pair)` does not match. Caught by dumping the golden fixture and seeing a literal returning `{ level, message, timestamp: Date.now() }` had indexed only `timestamp`. Now covered in return position AND in the variable-bound rule, which had the same gap. Provenance was flagged by owner-presence, which mislabelled the anonymous case: a callback's return shape yields no name to qualify by, so it looked like a DECLARED anchor and would have outranked real declarations. Flagged by position now — a different question from whether a name could be derived. SCHEMA_BUMP 48 -> 49. Within one PR the version only has to differ from main's, but a build stamped 48 was installed and used to analyze before these captures existed, so caches stamped 48 carry none of them — the intermediate-build hazard this ledger already records for 33/34. Golden regenerated after verifying the drift: exactly +10 Property and +10 DEFINES, every pre-existing count unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(scope-resolution): rank production anchors above test fixtures Found by testing R3-4 on the reporting repo instead of on its fixtures. Anchoring returned literals took `wickRatio` from 6 definitions to 13 — and backend reads still resolved to nothing, because SEVEN of the new JavaScript anchors compete and four of them are in `tests/`. A test constructs throwaway shapes carrying production field names; a read in shipped code cannot mean one of them. Applied before the declared/return-shape split, because "is this the shipped program" is the stronger signal — a declaration inside a test fixture is still a test fixture. Skipped when the READER is itself a test, since a read there legitimately means the test's own shape. The first version of this test was vacuous and the mutation check caught it: the reader sat in the same file as the production anchor, so the same-file tier resolved it whether or not this tier existed. The reader now lives in a file that imports neither anchor, which leaves production-vs-test as the only thing that can decide. Honest about what this does NOT do: it narrows `wickRatio` from seven candidates to three, and three functions in different files each returning that field is GENUINELY ambiguous — refusing is correct, and the ambiguity is now counted and named rather than silent. The reported question ("who reads wickRatio?") is answerable only where one producer exists; where several do, the honest answer is the list of producers, which R3-4 made nameable for the first time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(scope-resolution): resolve members through a call result's return shape The question three rounds of reports could not answer, and the one narrowing must refuse by design: a field produced by SEVERAL functions. A read of `spike.wickRatio` could mean any producer, so name inference correctly declines and no amount of tier-tuning changes that. It needs evidence, not inference. The evidence existed in two halves that had never been joined. The call-result type binding (`const alert = formatSpikeAlert(row)` binds `alert` to a TypeRef whose rawName is the callee) predates all of this work; it simply had nothing to resolve to when the callee returned an anonymous literal, because an anonymous literal named nothing. R3-4 gave it a name. Joining them: const alert = formatSpikeAlert(row); alert.wickRatio -> Property:...:formatSpikeAlert.wickRatio Precise, at ordinary emission confidence, and it works EXACTLY where narrowing cannot: several producers sharing a field name stop being competitors because the receiver says which one. Runs before the name fallback and claims its sites, so a precise answer is never second-guessed by a name match. Measured on the reporting repo: 1,410 precise edges, and all six fields round 3 verified OUT-OF-SAMPLE go from 0 backend readers to 7, 11, 10, 7, 6 and 14. Round 3 scored 0/6 on that set; this is 6/6. The bound is asserted, not just documented: a read off a BARE PARAMETER has no binding here, because typing it needs the caller's type to flow in — that is inter-procedural and genuinely larger. Those reads still fall through to name inference and are still reported when it declines. The fixture has two producers sharing a field name precisely so the test cannot pass by name matching, and mutation-checking the owner lookup fails it. No SCHEMA_BUMP: this is scope resolution, not parse-time capture, so a warm cache already carries everything it reads. Noted in the ledger because the reflex on this branch has been to bump, and an unnecessary bump costs every user a full re-parse. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Revert "return-shape anchoring" (R3-4/R3-5): it degrades query Reverts |
||
|
Gergő Magyar
|
9eaf2e6c4e
|
perf(mcp): cut the analyze-only language-provider closure out of MCP server startup (#2802) (#2806)
Some checks failed
CodeQL / Analyze (javascript-typescript) (push) Has been cancelled
CodeQL / Analyze (python) (push) Has been cancelled
Gitleaks / gitleaks (push) Has been cancelled
Publish / Classify release event (push) Has been cancelled
Scorecard / Scorecard analysis (push) Has been cancelled
Skill copy sync / shipped skills drift guard (push) Has been cancelled
Trivy Image Scan / Trivy (gitnexus-cli) (push) Has been cancelled
Trivy Image Scan / Trivy (gitnexus-web) (push) Has been cancelled
Publish / RC guard (marker + release-PR skip) (push) Has been cancelled
Publish / ci (push) Has been cancelled
Publish / Publish to npm (push) Has been cancelled
Publish / Build & Push RC Docker images (push) Has been cancelled
* 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 |
||
|
Gergő Magyar
|
990d79ba8c
|
fix(mcp): make impact/context reproducible — deterministic ordering on every capped query (#2787) (#2796) | ||
|
Gergő Magyar
|
911151e230
|
fix(resolution): resolve Go pointer-receiver calls, and report the program boundary instead of hedging (#2766) (#2782)
Some checks are pending
CodeQL / Analyze (javascript-typescript) (push) Waiting to run
CodeQL / Analyze (python) (push) Waiting to run
Gitleaks / gitleaks (push) Waiting to run
Publish / Classify release event (push) Waiting to run
Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions
Publish / ci (push) Blocked by required conditions
Publish / Publish to npm (push) Blocked by required conditions
Publish / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
|
||
|
azizur100389
|
d268f351d3
|
fix(group): preserve manifest-only impact crossings (#2784)
* fix(group): preserve manifest-only impact crossings Keep proven manifest cross-repo hits when the far endpoint has no concrete graph symbol, avoiding a guaranteed failed UID fan-out. * fix(group): verify manifest-only neighbor repos Keep manifest-only crossings from bypassing neighbor repository resolution so unavailable repos still surface as truncated fan-out. * fix(group): distinguish boundary-only impact crossings Keep manifest-only boundaries visible without treating unattempted fan-out as completed impact or escalating risk, and cover service scope, deduplication, and real bridge persistence. --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> |
||
|
MyShining
|
1147646518
|
feat(spring): model AOP transactions, caching, and security (#2783)
* feat(spring): model AOP advice and proxy behavior * fix(spring): address AOP review findings --------- Co-authored-by: Shining <xuenning@qiyi.com> |
||
|
MyShining
|
de84ad6297
|
feat(spring): index @Bean factories and @Resource injection (#2740)
* feat(spring): index Bean factories and Resource injection * fix(spring): address Bean and Resource review findings * refactor(lbug): keep relation pair parsing in router * test(lbug): preserve schema exports in WAL mocks * test(cache): align schema bump pin --------- Co-authored-by: Shining <xuenning@qiyi.com> Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> |
||
|
Void Freud
|
7be6d29ca0
|
fix: require repo in multi-repo MCP tool schemas (#2717)
* fix: require repo in multi-repo MCP schemas * style(mcp): fix server test formatting * chore(autofix): apply prettier + eslint fixes via /autofix command * test(mcp): cover repository schema policy --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
|
azizur100389
|
24584297d2
|
fix(trace): add file disambiguator alias (#2705) | ||
|
Parafee41
|
b85f1ace7a
|
fix(mcp): avoid api impact schema combinators (#2489)
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> |
||
|
Gergo Magyar
|
c836801c4a |
feat(mcp): add deterministic response budgets (#2460)
Composed with the read-only and repository policies in the CallTool handler: read-only assert, then budget resolution, then scoped dispatch with the transport arg stripped. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
Eva
|
3f3494fd32 | fix(mcp): validate aliases without schema combinators | ||
|
Eva
|
a75844b692 | feat(mcp): normalize impact and context aliases | ||
|
Eva
|
627ec5a5aa | feat(mcp): add deterministic output budgets | ||
|
Gergő Magyar
|
fbffa96554
|
fix(lbug/mcp): exact symbol content + 0-based line storage with 1-based MCP display (#2377, #2379) (#2380)
* fix(lbug): store exact symbol content snippets * fix(ingestion): emit 0-based line numbers for COBOL/JCL/scope/markdown nodes COBOL/JCL processors, the scope-graph emitter, and the markdown Section emitter stored 1-based startLine/endLine, unlike every tree-sitter node (0-based). The exact-content slice (#2379) then dropped each symbol's declaration line for those languages. Convert to 0-based at the graph-node emission boundary via toZeroBasedLine — leaving parser-internal .line values, L${line} node/edge IDs, and containment checks untouched. Refs #2377, #2379 * refactor(lbug): single source of truth for symbol-content labels Extract SYMBOL_NODE_LABELS so the exact-content label set can't drift the way the inline copy did in #2379. csv-generator derives EXACT_SYMBOL_CONTENT_LABELS from it; manifest-extractor's near-identical allowlist is left behavior-unchanged (intentional subset, #2325-test-locked) with a documented cross-reference. Refs #2379 * test(ingestion): cover 0-based emitter output and pin exact-content slicing - csv-pipeline: replace the blank-buffer fixture (a +/-1 shift silently passed) with directly-adjacent neighbors; add one-line-symbol and Section (+/-2 fallback) cases. - cobol resolver: assert COBOL Module and JCL job/step emit 0-based startLine. - markdown CRLF: update Section startLine/endLine expectations to 0-based. Refs #2377, #2379 * feat(mcp): present 1-based line numbers in context/query/impact tools GraphNode startLine/endLine are stored 0-based (tree-sitter rows), which surprised users querying them (they don't line up with editors/sed). Add toDisplayLine and apply it at the context/query/impact response boundaries so line numbers are editor/sed-aligned. Raw cypher stays 0-based (documented in the schema resource); BasicBlock/PDG statement lines (already 1-based) and internal join params are left untouched. Refs #2377 * test(mcp): assert 1-based tool exposure with raw cypher staying 0-based context() reports startLine+1 (editor/sed aligned); a raw cypher RETURN of the same node keeps the stored 0-based value. Guards against double-conversion and leaking the display shift into raw results. Refs #2377 * fix(mcp): stop query() double-converting BM25 line numbers bm25Search applied toDisplayLine to its result rows, and query()'s aggregation loop applied it again, so BM25-matched symbols reported lines shifted +2 (stored 0-based 41 read as 43, not 42) while semantic-matched symbols were correct. bm25Search is called only from query(); return raw 0-based rows and let the single aggregation-loop conversion handle both retrievers. Adds a query() BM25 regression test asserting stored 41 -> 42 (would be 43 if double-converted), which the prior mcp-line-display test — covering only context()+cypher — never exercised. (#2380, #2377) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): use ?? not || so first-line symbols keep their line number `sym.startLine || sym[4]` treated a legitimate 0-based startLine of 0 as absent, so context()/query() dropped startLine/endLine for every symbol on line 1 of its file — every COBOL Module (toZeroBasedLine(1) = 0) and markdown h1. `??` only falls through to the positional fallback on null/undefined, preserving a real 0. This also repairs the rename definition-edit path, which consumes context()'s value. Adds a context() first-line (startLine:0 -> 1) assertion. (#2380, #2377) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): make group/cross-repo trace line numbers 1-based consistently A group/cross-repo trace presented 1-based endpoints (via resolveSymbolForGroup) but 0-based hops (tagHops copies port.trace output verbatim), so one response mixed bases. Wrap the trace port adapter (traceForGroup) to convert hop lines to 1-based too, matching the endpoints. Single-repo trace dispatches directly (not through this port) and stays 0-based — full single-repo parity is a tracked follow-up. core/group stays display-agnostic (no mcp import). Extends the cross-trace e2e test to assert hops share the endpoints' base (checkout 10 -> 11, getUsers 1 -> 2). (#2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): present explain/pdg_query anchor line 1-based resolveBlockAnchor converted its ambiguous-candidate lines to 1-based but left the resolved-target anchor raw 0-based, so the same tool reported two bases depending on whether the target was ambiguous. Convert the display anchor to 1-based via toDisplayLine. The BasicBlock join param (symStart: sym.startLine + 1) is untouched — it targets the 1-based BasicBlock id space, not display. Asserts the resolved anchor is 1-based (targetFn stored 10 -> 11). (#2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): bump schema + PDG result versions for the line-number change The 0-based storage flip for COBOL/JCL/markdown/scope (#2377/#2379) changed on-disk line semantics, and the PDG result startLine is now 1-based (#2380). Neither shipped a version bump, so an incremental re-analyze would preserve old 1-based rows (mixed-base index rendered one line too high) and PDG consumers got no signal. - INCREMENTAL_SCHEMA_VERSION 5 -> 6 (forces a one-time full re-analyze) - PDG_RESULT_VERSION 1 -> 2 (result-shape discriminator) Updates the version-pinning tests, the pdgResultVersion result type, and the tools.ts PDG output-contract doc. (#2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(group): guard manifest label list against SYMBOL_NODE_LABELS drift manifest-extractor's CUSTOM_CONTRACT_RESOLVE_QUERY hand-lists the contract-resolvable labels as a deliberate subset of the shared SYMBOL_NODE_LABELS, guarded only by a comment — the same drift class (#2379) the shared-set refactor eliminated elsewhere. Derive the query's label set and assert it is a strict subset whose difference is exactly {Namespace, Variable, Module}, so adding a symbol label without a conscious manifest decision fails. Query string stays literal (#2325-test-locked). (#2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(mcp): document which tools present 1-based vs 0-based line numbers The schema-resource note listed only context/query/impact as 1-based. After the trace/anchor fixes it now enumerates the full set — context, query, impact, group/cross-repo trace, and explain/pdg_query anchors are 1-based; raw Cypher and single-repo trace stay 0-based (full single-repo-trace parity is a tracked follow-up); BasicBlock/PDG statement lines are separately 1-based. (#2377, #2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mcp): pin impact() line-value display (close the coverage gap) The prior mcp-line-display test only asserted context() + raw cypher, which is why the query() double-conversion (#2380) shipped green. Adds an impact() line-value assertion via the ambiguous-candidate path (the only impact response that surfaces a per-candidate line): two same-name symbols force ambiguity and the candidate at stored 0-based 41 must read 42. (#2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mcp): fix stale rename #2283 mock after 1-based context display rename resolves its symbol via context(), which now presents startLine 1-based (#2377), then subtracts 1 to recover the 0-based file index. The #2283 mock stored startLine:1 but put `oldName` on the file's line 0, so after the 1-based shift the definition edit no longer matched and the write-failure path never fired — the test read 'success' instead of 'partial'. Align the mock content to its stored line (oldName on 0-based line 1). Pre-existing failure surfaced once ubuntu/coverage completed on this branch. (#2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mcp): consolidate line-display tests into one shared DB block The query()/BM25 case had spun up a second full LadybugDB + FTS setup; fold it into the single existing block (adding FTS + the Zqxwvbm seed there) so the file builds one DB, not two. Trims per-file setup cost — relevant to the Windows platform-sensitive suite's under-load 15-minute timeout. Same five assertions, all green. (#2380) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: kigland <shuaizhicheng336@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergő Magyar
|
e46b87f291
|
feat: flat workspace index follows the checked-out branch (#2364)
Some checks failed
Devcontainer Smoke / Config-transform unit tests (push) Has been cancelled
Devcontainer Smoke / Build devcontainer image (push) Has been cancelled
CodeQL / Analyze (javascript-typescript) (push) Waiting to run
CodeQL / Analyze (python) (push) Waiting to run
Gitleaks / gitleaks (push) Waiting to run
Publish / Classify release event (push) Waiting to run
Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions
Publish / ci (push) Blocked by required conditions
Publish / Publish to npm (push) Blocked by required conditions
Publish / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
* feat: flat workspace index follows the checked-out branch (#2354) A plain `gitnexus analyze` now always targets the flat workspace slot, updating it incrementally across branch switches instead of auto-routing non-owner branches into `branches/<slug>/` sub-indexes (disk bloat) or nagging with the primary-inversion "run gitnexus clean" warning. No new CLI flag or config key: the smart behavior is the default. - Placement: only explicit `--branch` consults resolveBranchPlacement; plain runs resolve to the flat slot, `meta.branch` becomes an informational "last analyzed branch" label restamped each run. - Fast path: a same-commit clean-tree branch flip restamps the label and registry entry (adoptFlatBranchLabel, no-op for unregistered repos). - Shadow cleanup: when the flat slot adopts a label that has a pinned sub-index, the now-unreachable `branches/<slug>/` dir and its registry summary are removed together. - MCP: applyBranchScope always falls back to the on-disk flat meta before throwing "not indexed", so long-lived servers resolve a freshly restamped workspace branch. - status: no more "current branch not indexed" dead end — falls through to the workspace index with an informational line and the usual commit-based staleness verdict. - Deleted primaryInversionWarning; explicit `--branch` pinning, the checkout-mismatch guard, detached-HEAD/CI behavior, and `clean --branch` are unchanged. Supersedes the flag-based approaches in #2358/#2359. Closes #2354. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(storage): check registry before deleting shadowed sub-index (#2364 review F2) adoptFlatBranchLabel ran the branches/<slug>/ rm before its own unregistered-repo no-op check, so a repo in the #2264 half-finalized state (up to date but unregistered) lost its pinned sub-index on a same-commit branch flip while the run still failed. The registry lookup now precedes the deletion, making the no-self-heal rule (#2264/#1169) cover disk as well as registry state. The 'never self-heals' unit test now materializes a sub-index dir and asserts it survives; the run-analyze #2354 fast-path test registers its repo under an isolated GITNEXUS_HOME (deletion is only legitimate for registered repos) with a new unregistered variant pinning dir survival. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(storage): keep branch summary when sub-index rm fails (#2364 review F4) The shadow-cleanup fs.rm swallowed every error while the registry summary was dropped unconditionally. On Windows an lbug held open by a live MCP server fails the rm with EBUSY/EPERM, and once the summary is gone 'clean --branch' can never target the leftover dir (it resolves solely via the recorded summary) — stranding the exact un-cleanable disk bloat adoptFlatBranchLabel exists to prevent. The summary is now dropped only when the directory is verifiably gone (post-rm existence check); on failure the summary is retained, a warning names the path and errno, and the informational branch label still restamps. Later adopts retry the rm. New repo-manager-rm-failure.test.ts uses the delegating fs/promises mock idiom (vi.spyOn cannot intercept ESM namespace exports). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(core): restamp fast path adopt-first and tolerate read-only storage (#2364 review F3) The fast-path label sync stamped meta before adoptFlatBranchLabel, so a crash or adopt failure between the two flipped the retry guard (existingMeta.branch !== branchLabel) and locked in the partial state: every subsequent same-commit run skipped the cleanup and branch-scoped queries kept routing to the stale pinned sub-index. The block also sat outside any try/catch, so a same-commit branch flip on a read-only .gitnexus mount (the documented Docker :ro workflow, #1549) failed a byte-for-byte-current analyze over a purely informational label sync. Adopt now runs first and saveMeta last — any partial failure leaves the guard true and the next run self-heals — and the whole sync is best-effort: read-only errors warn citing #1549, anything else warns and retries next run. Safe because the block only fires on a same-commit clean tree, where the flat DB content is byte-valid for both labels. isReadOnlyFilesystemError is now exported. New run-analyze-adopt-failure.test.ts covers retry-after-partial- failure, adopt-before-stamp ordering, and EROFS/EACCES/EPERM (gaps 4 and 7); a detached-HEAD fast-path pin lands in run-analyze.test.ts (gap 6). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(mcp): make flat meta authoritative in applyBranchScope (#2364 review F1) applyBranchScope trusted two pieces of cached state before its flat- meta disk fallback, and the handle cache only refreshes on a resolve miss — never on a hit. Post-#2354 that stale window is the routine case: (i) the handle.branch early-return served the flat handle under the OLD label after a workspace flip, silently returning the new branch's content as the old branch (the pool staleness reinit hot- swaps content without updating handle.branch); (ii) a stale cached branches[] summary routed to a branches/<slug>/ dir that adoptFlatBranchLabel had already deleted (raw 'LadybugDB not found' or POSIX ghost reads with staleness detection blinded). The on-disk flat meta is now read before any cached-state trust. A branches[] summary is served only when its sub-index lbug actually exists (the lbug is what the pool opens — serviceability truth); the cached label is trusted only when no readable flat meta contradicts it (#2106 R4 legacy shapes preserved). One refreshRepos() fires on detected staleness so subsequent calls see fresh handles. Safe against mid-analyze reads: dirty stamps spread the existing meta, preserving the old label until the end-of-run atomic write. Fixtures now materialize the pinned sub-index lbug; new regressions cover the stale-old-label error, adopted-summary fall-through to flat, and the dangling-summary partial-failure window (test gaps 1-2). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(core): make end-of-run branch-label sync best-effort (#2364 review F5) The end-of-run adoptFlatBranchLabel sat inside the pipeline try whose catch rethrows, so a registry write failure (ENOSPC, ~/.gitnexus perms) after a successful multi-minute analyze failed the whole run — even though the index was complete and registered, the neighbouring parse-cache save is deliberately wrapped for exactly this reason, and adopt retries unconditionally on the next plain analyze. It now warns and continues, mirroring the parse-cache wrapper. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(mcp): correct branch-not-indexed guidance for workspace index (#2364 review F6) The error told users to 'Run: gitnexus analyze --branch <X>', but post-#2354 that command hard-errors unless X is checked out — and this message is now the common goodbye for a formerly-indexed branch whose sub-index the workspace slot adopted. The guidance now explains that the workspace index follows the checked-out branch and leads with the checkout; the '(primary only)' fallback becomes '(workspace only)'. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: align primary/workspace vocabulary with the #2354 inversion (#2364 review F7) The review flagged pre-inversion 'primary/non-primary' wording that now misleads readers about the placement model: the isPrimaryBranch JSDoc (field name kept — public API surface), the two branches? JSDoc comments in local-backend, the base_ref gate comment in cli/analyze, and four branch-scope test names. Comment/JSDoc/test-name edits only; 'Registry-primary' and 'primary key' senses untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(cli): clarify workspace index status wording (#2364 review F8) 'gitnexus analyze follows this branch' was ambiguous about WHICH branch analyze follows — the recorded one on the line or the current checkout. Both locales now say a re-run follows the current branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(storage): re-read registry after the shadow rm in adoptFlatBranchLabel The F2 reorder moved the registry read to the top of the function, so the whole-file writeRegistry at the bottom persisted a snapshot taken BEFORE the recursive rm of an entire sub-index — widening the unlocked read-modify-write window from microseconds to the duration of a multi- hundred-MB delete. A concurrent registerRepo/removeBranchIndex writer in that window was silently clobbered (the #2106 R9 lost-update class; registerRepo re-reads before writing for exactly this reason). The top read is now a cheap membership gate only (the F2 no-op guarantee); the mutate re-reads its own fresh snapshot after the rm. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: treat only provably-absent errno as gone in the new existence probes Both probes added by this series inverted the codebase's provably- absent polarity (listRegisteredRepos validate prunes only on ENOENT/ENOTDIR): adoptFlatBranchLabel's dirGone check read ANY fs.access failure — including a transient EACCES/EIO on a surviving dir — as 'verifiably gone' and dropped the summary, recreating exactly the stranded-bloat bug F4 fixed; applyBranchScope's sub-index check read the same transient errors on a healthy pinned lbug as 'adopted/ deleted', producing a false 'not indexed' error. A resolved force:true rm now proves absence without a probe; on failure the probe treats only ENOENT/ENOTDIR as gone, and a non-missing lbug serves the handle so the pool open surfaces the real error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(mcp): harden applyBranchScope stale-state coherence Four residual gaps in the new arm structure, found by post-fix review: - The stale-label error listed the just-contradicted cached label as indexed ('not indexed: main. Indexed branches: main'). The message now derives the flat label from the authoritative meta and excludes the requested branch from the hint list. - A branch pinned AFTER the server cached its handle never triggered a refresh (resolve hits skip the miss-refresh), erroring until restart. Every miss now fires exactly one best-effort refreshRepos() before the error, so the next call resolves; a refresh-once guard keeps doubly-stale resolutions to a single registry re-scan. - A registry entry claiming the branch both as flat label and pinned summary (the rm-failed adopt-degraded state) could serve the stale- vintage pin under a label the flat slot owns; the summary arm now requires handle.branch !== branch and the degraded state errors honestly. - The flat-meta match path returned the cached handle's pre-restamp branch/commit/stats; the meta that decided routing now also supplies the metadata. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(core): keep the real error visible in restamp warnings; correct the end-of-run retry claim The fast-path catch replaced the actual error with 'storage is read-only (#1549)' for any EACCES/EPERM — mislabeling ownership problems and transient Windows locks and discarding the only diagnostic signal. The warning now carries the real message with the #1549 hint appended. The end-of-run best-effort comment claimed adopt 'retries unconditionally on the next plain analyze'; same-commit runs take the fast path whose guard compares the already-stamped meta label, so the retry actually lands on the next content-changing run. The comment now states the true retry semantics and why the interim state is safe (flat meta stamped first; applyBranchScope trusts it). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test: unique tmpdir for the branch-scope fixture; drop redundant dynamic imports The branch-scope describe materialized its sub-index stub under a FIXED os.tmpdir()/gnx-2106-multi path — concurrent vitest runs on one host (the documented parallel-agents workflow) could rm each other's stub between beforeEach and the resolve under test, flaking the pinned-branch tests. The fixture root is now mkdtemp-unique per run with afterAll cleanup. run-analyze.test.ts dynamically imported repo-manager inside test bodies despite the module being statically imported at the top of the file (no vi.mock exists there to justify it); the three call sites now use the static import. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
ChunxueLi
|
1029a8ddd7
|
feat: add Spring DI resolver for @Autowired List<T> injection (#2200)
Some checks are pending
CodeQL / Analyze (javascript-typescript) (push) Waiting to run
CodeQL / Analyze (python) (push) Waiting to run
Gitleaks / gitleaks (push) Waiting to run
Publish / Classify release event (push) Waiting to run
Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions
Publish / ci (push) Blocked by required conditions
Publish / Publish to npm (push) Blocked by required conditions
Publish / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
* feat: add Spring DI resolver for @Autowired List<T> injection Addresses all P0/P1 findings from tri-review (#2200): - P0: Register INJECTS in RelationshipType union (compiles) - P0: Rewrite execute() to emit consumer→implementation edges from graph data only - P1: Register in VALID_RELATION_TYPES, single-pass O(N) indexes - P1: Java-only gate with early exit on non-Java repos - P1: Update FULL_ORDER golden test - 8 unit tests covering all edge cases * test: make VALID_RELATION_TYPES size assertion array-driven (no hardcoded count) The security test hardcoded toBe(16) for the relation type count, but PR #2200 added INJECTS, bumping it to 17. Replace the magic number with an EXPECTED_RELATION_TYPES array whose .length drives the size assertion, so future additions only need to append to the list. Fixes CI failure on PR #2200. * fix(ingestion): thread raw generic field types onto Property nodes so Spring DI matching works (review 4616076037 P0) Production declaredType is generics-stripped by design (extractSimpleTypeName: List<Shape> -> "List"), so the spring-di phase's anchored regexes could never match real extraction output — the phase was a silent no-op on every real Java repository, while its unit tests passed against hand-built node shapes. Add FieldInfo.rawDeclaredType captured verbatim from the field's type node (.text, generics and qualifiers preserved — same precedent as the JVM method extractor), thread it through both parse-worker Property sites, add it to the shared NodeProperties contract, and match on rawDeclaredType ONLY (no declaredType fallback: it can never match real data and would mask future plumbing regressions as quiet no-ops). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ingestion): gate Spring DI on real injection annotations, honest edge reason (review 4616076037 P1) Extract Java field annotations (shared extractAnnotations helper, moved verbatim from the method extractor) onto Property nodes and require @Autowired or @Inject before a collection field becomes an INJECTS candidate. Previously every edge's reason string fabricated "@Autowired" without any annotation ever being checked, and any plain collection field would have fanned out false edges once matching worked. @Resource is deliberately excluded: JSR-250 resolves by bean name first (defaulting to the field name), injecting a single named collection bean — the opposite of the collect-all-implementers fan-out INJECTS models. Pinned by a test. An annotated candidate missing rawDeclaredType now logs an isDev warning (plumbing-contract breach signal) instead of vanishing silently. SCHEMA_BUMP 9 -> 10: Property nodes gained rawDeclaredType + annotations; warm parse caches must invalidate or the DI phase silently no-ops on replayed pre-upgrade nodes (the #2038 trap). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(ingestion): framework-neutral di phase + language-scoped Spring matcher registry (review 4616076037 P1) spring-di was the only pipeline phase naming a language in shared core/ingestion code (DoD.md language rule; the maintainer's direction is a generic DI solution). Split it: - di-extractors/spring.ts: the Spring matcher (annotation gate, collection type parse, @Resource exclusion rationale, framework-specific reason payload) — language-scoped home, mirroring route-extractors/. - di-extractors/index.ts: DI_MATCHERS, a single-valued ReadonlyMap<SupportedLanguages, DiFieldMatcher> mirroring the SCOPE_RESOLVERS registry shape sanctioned by AGENTS.md. Constructor injection deliberately out of scope; widen to arrays only when a second same-language framework lands. - pipeline-phases/di.ts (renamed from spring-di.ts): framework-neutral — routes Property nodes to registered matchers by node language via a typed guard, then runs the unchanged reverse-index fan-out. Zero language or framework names remain (grep-verified). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ingestion): language- and qualified-name-scoped interface resolution for DI fan-out (review 4616076037 P2) The interface index was built from ALL Interface nodes regardless of language, keyed by bare simple name with last-writer-wins overwrite — a polyglot repo with a TS and a Java 'Shape' could fan Java INJECTS edges into TypeScript classes, and two same-named Java interfaces in different packages silently collapsed to whichever parsed last (documented GitNexus bug class: #2054, PR #1956). Resolution is now per-language with qualifiedName as the primary key (Interface nodes already carry package-qualified qualifiedName); dotted element types resolve via qualifiedName, bare names via a per-language simple-name index that records ambiguity and fails CLOSED. Ambiguity skips are observable: DIOutput.ambiguousSkipped + an aggregated isDev debug log, so 'no DI fields' is distinguishable from 'all candidates ambiguous'. Same-package tiebreaking is a pinned, documented follow-up. Order-independence pinned by running collision tests in both insertion orders. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ingestion): depth-aware Spring collection-type parser for idiomatic generics (review 4616076037 P3) The two anchored regexes silently skipped idiomatic Spring shapes: Map<Pair<A,B>, IFoo> (nested-generic key broke the [^,]+ split), List<? extends IFoo> / List<? super IFoo> (bounded wildcards), java.util.List<IFoo> (qualified wrapper), and whitespace/multi-line declarations. Replace them with a small scanner: whitespace normalization, wrapper matched by last dotted segment, depth-aware top-level-comma split, wildcard bound stripping, and a final plain-dotted-type-name gate so anything else (nested-generic elements, arrays, unbounded wildcards, embedded comments, unbalanced brackets) fails closed. Every accept and reject is documented in the module docstring and pinned by 27 table-driven cases. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(integration): prove Spring DI end-to-end through the real pipeline (review 4616076037 P1) Both no-op incarnations of this feature shipped with a green unit suite because every test hand-built the exact graph shape the phase expected — no test ever ran real Java source through the actual extraction pipeline. Add test/integration/spring-di-pipeline.test.ts: real .java fixtures via runPipelineFromRepo, pinning (a) the extraction contract on the annotated field's Property node (declaredType 'List', rawDeclaredType 'List<IFoo>', annotations ['@Autowired']), (b) set-equality on ALL INJECTS edges (exactly Consumer->FooA and Consumer->FooB; the non-annotated 'plain' field of the same type contributes nothing; no self-edges), and (c) a negative-control fixture with no injection annotations producing zero INJECTS edges. Either historical regression fails at least one of these. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(incremental): register INJECTS across product surfaces + delete-before-writeback (review 4616076037 P2) INJECTS was allowlisted in VALID_RELATION_TYPES but invisible or unhandled everywhere else. Register it deliberately: - REL_TYPES (gitnexus-shared schema-constants): web-side validRelType() otherwise silently rejects INJECTS filters (CLI/web single source of truth). - mcp/tools.ts cypher edge list (agent-facing schema discovery). - isGraphWideRelType: INJECTS validity is a whole-program property — a change to a THIRD file (the interface, or a new/removed implementer) creates/invalidates edges between two untouched files (the TAINT_PATH / #2084 M4 U6 class), so incremental extraction must always re-include the full fresh set. - deleteAllInjects (lbug-adapter): mirrors deleteAllInterprocTaintPaths — COUNT-then-DELETE under withConnLock, benign missing-table carve-out, re-throw otherwise (CodeRelation has no PK and there is no read-side dedup; a fail-soft delete + re-add would silently duplicate rows). - run-analyze.ts: the delete is UNCONDITIONAL, next to the Communities delete — deliberately NOT inside the options.pdg block: the di phase runs on every persisting analyze while the graph-wide re-include is unconditional, so a pdg-gated delete would append without deleting on every non-pdg incremental run (N runs = N copies). - local-backend.ts comment: opt-in traversal by design (not in default impact()/context() lists; no IMPACT_RELATION_CONFIDENCE entry per the WRAPS/FETCHES precedent — edges carry their own 0.8). - ARCHITECTURE.md: 14 -> 15 phases, DAG diagram, phase table, skip-list. Note: the tools.ts edge list also predates WRAPS/QUERIES/USES — that drift is pre-existing and left for a follow-up. Idempotency pinned end-to-end: two successive incremental runs (real runFullAnalysis + real LadybugDB, unrelated-file touches) leave the INJECTS row count stable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: describe INJECTS' actual precondition; drop stale fixed-at-16 comments (review 4616076037 P3) The shared-schema doc for INJECTS claimed an @Autowired precondition the code (pre-fix) never checked, and hardwired Spring semantics into what is now a framework-neutral edge type. Reword: precondition is an injection annotation recognized by a per-language matcher in di-extractors/; framework specifics live in the reason payload, not the type contract. security.test.ts comments still said the allow-list size 'stays fixed at 16' (it is 17 and the assertion derives from EXPECTED_RELATION_TYPES). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor: simplify DI surfaces — narrow matcher contract, dedup delete-alls, derive tools edge list Post-implementation simplification pass (4 review angles): - DiFieldMatch/CandidateField carried collectionType + matchedAnnotation that no consumer read (the matcher bakes both into reason) — narrowed to {elementTypeName, reason}. - parseElementTypeName had two guard branches fully subsumed by the final plain-dotted-type-name gate — deleted, rationale folded into the regex comment. - The three byte-identical delete-all-by-rel-type functions in lbug-adapter (TAINT_PATH / CALL_SUMMARY / INJECTS) are now one parameterized helper + thin wrappers with identical names, signatures, and message text (character-diff verified) — the missing-table regex and abort policy now live in exactly one place. - The cypher tool's hand-maintained edge-type list (already missing WRAPS/QUERIES/USES) is now derived from the canonical REL_TYPES — the drift class is gone rather than patched. - di phase: interface indexes are built only for languages that actually have candidates; test builder gained a rawDeclaredType opt-out replacing a hand-rolled node. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: apply Tier-2 review findings — qualified-name fail-closed, honest cypher docs, pinned delete contract, hook isolation - byQualifiedName was last-writer-wins on duplicate qualified names (reproduced: order-dependent INJECTS edges with ambiguousSkipped 0 — same package+interface duplicated across monorepo modules/source roots; Java qualifiedName has no file-path component). Both indexes now share the AMBIGUOUS fail-closed sentinel; order-flip test added. - The REL_TYPES-derived cypher edge list advertised pdg-gated types with no caveat (LLM queries on them silently return zero rows on default indexes) — caveat appended, INJECTS example added, impact relationTypes description now names the DI fan-out opt-in. - The delete-all re-throw contract (only defense against duplicate CodeRelation rows) was untested — error classification extracted to a pure classifyDeleteAllError and pinned exhaustively. - extractRawType/extractAnnotations hooks lacked the per-hook try/catch the pipeline applies elsewhere (#2286 pattern): a throwing hook would silently drop every remaining file in the language group. Hardened, degradation tested. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Gergő Magyar
|
57e4afa4c8
|
fix(mcp): stabilize api_impact response shape for same-URL multi-verb routes (#2308) (#2309)
Some checks failed
Scorecard / Scorecard analysis (push) Has been cancelled
CodeQL / Analyze (javascript-typescript) (push) Has been cancelled
CodeQL / Analyze (python) (push) Has been cancelled
Gitleaks / gitleaks (push) Has been cancelled
Publish / Classify release event (push) Has been cancelled
Trivy Image Scan / Trivy (gitnexus-cli) (push) Has been cancelled
Trivy Image Scan / Trivy (gitnexus-web) (push) Has been cancelled
Publish / RC guard (marker + release-PR skip) (push) Has been cancelled
Publish / ci (push) Has been cancelled
Publish / Publish to npm (push) Has been cancelled
Publish / Build & Push RC Docker images (push) Has been cancelled
* fix(mcp): stabilize api_impact response shape for same-URL multi-verb routes After #2302 made Route identity method-aware, a same URL exposes one Route node per HTTP verb, so a bare-URL api_impact lookup could silently flip from a direct route object to the wrapped { routes, total } envelope. Surface each route's `method` (via the shared fetch) so multi-verb results are distinguishable, and add an optional `method` selector that narrows a multi-verb URL/file to one verb and forces the singular shape. A verb that matches no route returns a clear error. Document the match-count contract in the tool schema. Refs #2308 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mcp): cover same-URL multi-verb api_impact contract Regression coverage for #2308: bare-URL and bare-file lookups of a same-URL GET+POST pair return the wrapped form with distinct per-route methods; the method selector collapses to the singular shape (case-insensitively); an unmatched verb returns a verb-not-found error; and verbless routes surface a null method. Refs #2308 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(review): apply autofix feedback - tools.ts: correct api_impact contract docs — `method` narrows to one verb but the singular shape only holds when exactly one route remains after filtering (substring route/file matches can still wrap); cover file lookups; enumerate verbs. - local-backend.ts: surface `method` in route_map and shape_check output (the shared fetch already returns it; agents discover verbs there before api_impact). - local-backend.ts: compute routeCountByHandler from the unfiltered match so a method-scoped api_impact still flags a multi-verb handler's partial middleware. - tests: add file+method and verbless-exclusion cases; assert unconditionally via toMatchObject; lowercase the verb-not-found input to exercise error uppercasing. Refs #2308 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): treat wildcard '*' routes as matching any api_impact method selector (#2308) Method-agnostic routes (Django function views) persist with Route method '*', not null. The api_impact method selector used exact verb equality, so '*' routes were excluded and api_impact({route, method:'POST'}) falsely reported 'No routes found' for a route that handles every verb. Treat '*' as matching any requested verb, and correct the comment + tool-description strings that wrongly grouped Django wildcards with null/verbless routes. * fix(mcp): harden api_impact method input against non-string and empty values (#2308) The MCP envelope is not schema-validated, so a non-string `method` reached `.toUpperCase()` and threw a TypeError. Widen the param to `unknown` and guard it with a typeof check that returns a structured error (mirroring the resolveAliasString pattern from #2175), and collapse empty/whitespace verbs to no selector. * fix(mcp): distinguish url-not-found from verb-not-found in api_impact error (#2308) The verb-not-found error appended 'with method "X"' even when the URL/file itself did not exist, implying the URL exists with other verbs. Gate the verb clause on matched.length > 0 so a non-existent URL/file gets the plain message. * fix(mcp): clarify api_impact middlewareNote wording for verbless siblings (#2308) The partial-middleware note claimed 'other methods in this handler' even when the co-located sibling is a verbless (null) route rather than another HTTP verb. Refer to 'other route exports' instead, which covers both cases. * docs(mcp): document and test the method field on route_map and shape_check (#2308) The shared fetchRoutesWithConsumers change surfaced a method key on route_map and shape_check responses too, but their tool descriptions never mentioned it and no test covered it. Document the field on both descriptions and add unit tests asserting it (shape_check rows carry responseKeys + a consumer so they survive shape_check's keys-and-consumers filter). * test(mcp): cover middlewareDetection 'partial' survival under a method filter (#2308) The diff's core behavioral line counts verbs-per-handler from the unfiltered match set so a method-scoped query still flags a multi-verb handler's partial middleware, but no test exercised it (every verbRow hardcoded middleware:null). Add a middleware param to verbRow and a test that fails if the count is taken from the post-filter set instead. Verified via mutation: matched->routes fails it. * test(mcp): add live-LadybugDB integration coverage for route method round-trip (#2308) The new n.method query column was only unit-mocked. Add a self-contained integration suite that seeds GET+POST /api/orders and a method-agnostic '*' Django route, then asserts api_impact surfaces method, narrows by verb, and matches the '*' route end-to-end (the U1 fix), plus route_map surfacing. Own seed + no FTS so it neither perturbs api-impact-e2e nor silently skips. * refactor(mcp): type the api_impact response shape instead of Promise<any> (#2308) Replace apiImpact's Promise<any> with an explicit ApiImpactResult union (single route | wrapped { routes, total } | { error }) and a typed ApiImpactRoute. The results.map is annotated so the response builder is checked against the declared shape. Behavior unchanged; sibling MCP methods keep their Promise<any> convention. * fix(mcp): express the route-or-file requirement in the api_impact schema (#2308) The inputSchema left route/file as bare optionals, so the 'at least one of route/file' rule the handler enforces was invisible to clients. Add an optional anyOf to ToolDefinition (forwarded verbatim by the ListTools handler) and an anyOf:[{required:[route]},{required:[file]}] on api_impact. Matches runtime (both allowed, route wins); 'at least one' not 'exactly one'. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergő Magyar
|
47477e5554
|
fix(mcp): tolerate adapter-materialized line:0 in impact callgraph mode (#2279) (#2283)
* fix(mcp): tolerate adapter-materialized line:0 in impact callgraph mode (#2279) Some MCP client/agent adapters serialize an omitted optional numeric field as `0` rather than dropping it, so callgraph `impact` calls arrive carrying a spurious `line: 0`. `line` is a PDG-only statement anchor and is meaningless on the callgraph path, so the backend rejected the call ("'line' is only supported with mode:'pdg'") and strict clients rejected it client-side against the advertised `minimum: 1`. Treat a literal `line: 0` as omitted in `_impactImpl` when mode !== 'pdg' and let the normal symbol→symbol BFS run. The coercion is deliberately narrow: only the literal 0, only on the callgraph path. A genuine positive `line` on callgraph still errors (real mode mistake), negative/ fractional values still error, and pdg mode is untouched — `line: 0` there is still rejected (there is no 1-based source line 0 to anchor on). Regression tests pin the full matrix: callgraph + line:0 runs the BFS and is byte-identical to omitting line; pdg + line:0 still errors; positive line on callgraph still errors. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): log swallowed best-effort query degradations at warn, not error `logQueryError` is the shared handler for query failures that every caller catches and degrades past with a safe fallback (the operation still returns a result). It logged all of them at `logger.error` (level 50) — the same severity as fatal failures — so a gracefully-handled degradation raised a false alarm and drowned genuine errors. This surfaced as an ERROR-level log firing during a passing unit test that intentionally injects a slice-callees query failure to verify the degrade path. Make the severity match reality: - benign missing optional table/label/column (a repo analyzed without processes/communities, or a pre-v3 PDG index lacking the `calleeIds` column — a query that fails on every pdg-downstream impact for such an index) → debug, the normal-configuration case. - any other swallowed failure → warn (handled degradation, still observable). - error is reserved for failures that actually abort an operation, which log directly rather than through this helper. Also fix the sibling bm25/FTS fallback, which logged its swallowed "FTS indexes may not exist" degradation at error while its own import-failure fallback already used warn. The slice-callees degradation test now captures the log and asserts it lands at warn (40), not error (50), pinning the severity against regression. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): relax impact `line` schema minimum to 0 for adapter compatibility (#2279) Strict MCP clients/agents validate against the advertised input schema and reject a request before sending it. With `line` declaring `minimum: 1`, a client that materializes the omitted optional `line` as `0` rejects a perfectly valid callgraph impact call client-side — so the backend tolerance added in the previous commit never gets a chance to run. Lower the advertised `line.minimum` to 0 and document that 0 (or omission) means "no statement anchor" while mode:'pdg' still requires a positive line. The advertised schema is advisory (the backend self-validates and is the real gate), so this cannot loosen any enforced contract — it only stops strict clients from pre-rejecting `line: 0`. Negative lines are still rejected at the client boundary. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(review): apply autofix feedback Code-review autofix pass on the #2279 branch: - Replace a newly-introduced `mode as any` cast in the #2279 it.each with the narrow `mode as 'callgraph' | undefined` (strict-typing-no-any). - Add a degradation test for the new logQueryError benign-missing-table → debug branch (asserts no warn/error record surfaces, i.e. it routed to debug). - Pin the bm25/FTS error→warn severity change with a _captureLogger assertion in the existing #1489 test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): make swallowed-failure callers surface degradation; narrow benign-error match (#2283) Tri-review (#2283) found the `error → {debug|warn}` rework reduced telemetry for `logQueryError` callers that do NOT degrade safely, while the docstring over-claimed "every caller degrades to a safe fallback". Address the substance rather than only the log level: - rename apply-edit: track failed writes and return status:'partial' with `failed_files` instead of reporting `status:'success'` when a write was swallowed. A partial rename is no longer indistinguishable from a clean one. - detect_changes: a swallowed symbol/process query failure now sets `partial:true` (rendered by the existing eval-server partial path) so the pre-commit safety gate can't return a false-clean `risk_level:'low'` no-op. - isBenignMissingTableError: scope the `not (defined|found)` arm to a schema object (table/label/rel/column/property), mirroring lbug-adapter's isMissingColumnError. An unscoped "not found" matched operation failures like `rg: not found` / `Symbol not found` and silently demoted them to debug. - logQueryError docstring: state the contract honestly — level reflects telemetry severity, and mutating/safety-critical callers MUST also surface a result-level degradation signal; `warn` alone is not a substitute. - pdg dispatch: pass the normalized `effectiveLine` (not raw params.line) so the validation gate and engine share one source of truth (identity today). Tests: - _captureLogger(level?) lets tests capture below info; the benign-missing-table test now asserts the record IS emitted at debug (20), not merely absent — no longer a vacuous pass if the call were deleted. - new: a non-schema "not found" failure logs at warn (regex-narrowing guard); rename write-failure degrades to status:'partial'+failed_files; line:-1 on the callgraph path still errors (line:0 coercion is narrow); typed the it.each tuple to drop a `mode as` cast. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(mcp): fix impact `line` description contradiction for whole-symbol pdg (#2283) The new `line` schema description said "mode:'pdg' requires a positive line", which contradicted the top-level impact description ("Without 'line', pdg returns whole-symbol inter-procedural reach plus local whole-symbol PDG diagnostics"). A pdg call without a line is a valid (degraded whole-symbol) call, not an error — the old wording could push an agent to avoid valid no-line pdg calls or synthesize line:0 (which then hard-errors). Reword to: omit line for whole-symbol pdg; a positive line anchors a statement slice; literal 0 is tolerated only as an omitted-line compatibility sentinel on the callgraph path and is rejected for mode:'pdg'. Update the schema test to pin the new, non-contradictory wording and assert "requires a positive line" is gone. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergő Magyar
|
1a03c8527a
|
feat(group): cross-repo call trace using PDG (#2269)
* refactor(group): extract shared resolveBridgeNeighbors from cross-impact
Lift the uid-filtered consumer<->provider ContractLink join (direction +
queryBridge + row normalization + confidence sort) out of runGroupImpact's
inline Phase-2 block into an exported resolveBridgeNeighbors helper. Behavior
is unchanged for impact; the helper becomes the single shared bridge join so
the upcoming cross-repo trace path never forks its own copy of the neighbor
Cypher. Empty uid sets short-circuit without a DB round-trip.
Adds direct coverage (real bridge via writeBridge/openBridgeDbReadOnly) for
both directions plus the empty-set and unknown-uid edges.
* feat(group): cross-repo trace stitching (groupTrace + runGroupTrace)
Add GroupService.groupTrace and the pure runGroupTrace engine that stitches
per-repo CALLS/HAS_METHOD trace segments across one ContractLink boundary in
the group bridge:
from --(local trace)--> consumer --(ContractLink)--> provider --(local trace)--> to
- Resolves from/to across all members (symbol node id == bridge symbolUid);
same-repo endpoints delegate to a single local trace with no crossing.
- Single boundary crossing (MAX_SUPPORTED_CROSS_DEPTH); deeper crossDepth is
clamped with a note, mirroring cross-impact.
- Discriminated GroupTraceResult union (ok|not_found|ambiguous|error) with
per-hop repo tags, a typed crossings[] entry, and centralized degraded-state
note constants (TRACE_NOTES). No .
- Trace-specific pair query (keeps BOTH crossing endpoints) lives in this
module; the uid-filtered neighbor join (resolveBridgeNeighbors) is reused
where it fits. ensureBridgeReady exported for reuse.
- New GroupToolPort methods (trace/resolveSymbol/pdgFlows) are optional so
existing port mocks keep type-checking; runGroupTrace guards on presence.
PDG enrichment is wired as an opt-in hook (enrichSegment) — the port method is
stubbed until U4. Covered by unit tests over a real bridge + mocked port.
* feat(group): route trace tool to groupTrace on @group syntax
Wire the cross-repo trace through the existing @group dispatch:
- callTool routes trace with an @-prefixed repo to callToolAtGroupRepo, which
forwards from/to/uid/file/maxDepth/includeTests plus the experimental
pdg/crossDepth flags to GroupService.groupTrace. Member path in @group/path
is advisory for trace (resolution is whole-group).
- Port gains trace/resolveSymbol/pdgFlows adapters. resolveSymbolForGroup wraps
the shared resolveSymbolCandidates so groupTrace can locate the member repo
and recover each endpoint node id (== bridge symbolUid). pdgFlowsForGroup is
a degraded stub here (call-level only); U4 implements the REACHING_DEF walk.
- trace tool schema documents the @group entry point, pdg, and crossDepth.
Single-repo trace is untouched. Covered by dispatch-routing tests (@group ->
groupTrace, non-group stays local) and tool-schema assertions.
* feat(group): opt-in PDG data-flow enrichment for cross-repo trace
Implement _pdgFlowsForGroupImpl: the real REACHING_DEF anchor walk that backs
the port pdgFlows adapter (replacing the U3 call-level stub). When pdg:true and
the segment repo has a flows PDG layer, the boundary-adjacent segments carry
their intra-procedural def->use hops:
- Anchors by the boundary symbol UID (precise; avoids the by-name ambiguity the
resolveBlockAnchor path can hit), then reuses the same span-anchored,
bind-param-only flows query as pdg_query (BasicBlock id-prefix + [start+1,
end+1] line window; no rel-property index, so the anchor IS the bound).
- Stays intra-procedural: data flow never crosses the repo boundary.
- pdgStampForMode probe: false -> available:false (degrade with note); the
trace stays ok. Any query failure is swallowed (enrichment is auxiliary).
Covered by runGroupTrace enrichment tests: dataFlow attached on opt-in,
degraded note when no layer, and no pdgFlows call when pdg is omitted.
* test(group): evaluation-first cross-repo trace e2e (two real indexes)
End-to-end gate for the cross-repo trace: stands up two real LadybugDB indexes
(consumer 'frontend' + provider 'backend'), a real ContractLink bridge, and a
real LocalBackend with both repos registered, then drives the public
callTool('trace', { repo: '@grp', pdg: true }) and asserts:
- the stitched checkout -> callUsers -(CONTRACT_LINK)-> handleUsers -> getUsers
path, each hop tagged with its member repo
- real REACHING_DEF data-flow enrichment of the consumer segment (userId)
- a degraded 'No PDG layer in app/backend' note (provider has no PDG layer)
- single-repo trace against one member is unchanged (no crossings)
Hand-persists the minimal real graph (deterministic; a full two-repo analyze is
heavier than this gate needs) and exercises real Cypher across
resolveSymbolCandidates, _traceImpl, the bridge pair query, and
_pdgFlowsForGroupImpl. Windows-skipped (describeReopen) and registered in the
cross-platform native-lbug set.
Scoped to a single @group call: opening bridge.lbug read-only a SECOND time in
one process currently fails (shared bridge open/close lifecycle, also affects
impact @group) — the pdg-omitted/clamp variants are unit-covered.
* docs(group): document cross-repo trace + PDG enrichment
ARCHITECTURE.md: trace is now group-aware; describe the @group cross-repo
stitch over a single ContractLink boundary (CONTRACT_LINK hop, crossings[],
crossDepth clamp), the opt-in experimental PDG REACHING_DEF enrichment of
boundary-adjacent segments, the symbolUid-grain join between the two stores,
and the deferred full cross-program (SDG-like) data flow. PIPELINE.md: add the
cross-trace consumer of the bridge with its pair-query rationale.
Does not touch gitnexus/CHANGELOG.md (release-owned).
* fix(review): apply autofix feedback
Apply safe_auto findings from ce-code-review (run 20260622-094243):
- local-backend.ts: drop (r: any) in _pdgFlowsForGroupImpl row map; coerce
hop line via Number() so a nullish LadybugDB cell can't surface NaN.
- tools.ts: advertise the forwarded param in the trace schema and add
crossDepth maximum:10 (schema now matches what groupTrace reads).
- cross-trace.ts: parallelize per-member resolveSymbol/resolveRepo with
order-preserving Promise.all (matches groupContext/groupQuery); add a note
when pdg:true is passed to a same-repo trace (PDG only enriches at a
cross-repo boundary).
- tests: remove / tighten (no-any rule).
Residual gated_auto/manual findings (unbounded crossing query + loop,
whole-file PDG widening on absent span, error-vs-no_path masking, top-level
try/catch parity, helper dedupe, branch-coverage gaps) are recorded in the run
artifact for the PR body.
* fix(group): skip CHECKPOINT on read-only bridge close so it can reopen
Root cause of the in-process bridge.lbug reopen failure (which broke repeated
@group impact/trace calls in a long-lived MCP server): closeBridgeDb issued
CHECKPOINT on EVERY handle, including read-only ones. A CHECKPOINT on a
read-only connection has nothing to flush but leaves a WAL/shadow lock artifact
that makes the next read-only open of the same path fail (openBridgeDbReadOnly
returns null -> 'Could not open bridge.lbug read-only'). Reproduced: open ->
query -> closeBridgeDb -> open again returned null only when the close ran
CHECKPOINT; a non-checkpoint close reopened fine, and the raw native
open/close cycle was never the problem.
Fix: tag read-only handles (BridgeHandle._readOnly, set by openBridgeDbReadOnly)
and skip CHECKPOINT for them in closeBridgeDb. Writable handles are unchanged
(they still flush before close). This is the shared bridge-db close path, so
impact @group benefits identically.
- Regression test in bridge-db.test.ts: open/query/close/open/query/open in one
process now succeeds.
- Re-enabled the second @group call in cross-trace-e2e.test.ts (was scoped to a
single call for this very limitation).
* fix(group): bring bridge-db close to parity with the core adapter safeClose
The bridge open/close cycle was less robust than the main graph DB's: closeBridgeDb
closed the connection/database but skipped the post-close steps the core adapter's
safeClose performs, so a rapid in-process reopen could race the OS handle release
(Windows) or an orphaned WAL sidecar. That gap is why the close-then-reopen tests
had to skip Windows.
closeBridgeDb now mirrors safeClose after closing the handle:
- waitForWindowsHandleRelease(dbPath): probe the file (+ .wal) until the residual
Windows lock clears, so the next open does not race (warns if the budget is
exhausted, matching the core adapter).
- finalizeLbugSidecarsAfterClose(dbPath): quarantine an orphaned WAL (shadow
missing) so the next open replays a consistent file.
Both helpers are the same ones safeClose uses (Windows-proven via the core adapter
CI), and the bridge read open already retries transient locks. Combined with the
read-only CHECKPOINT skip, the bridge reopen is now robust on every platform, so
the close-then-reopen tests run on all platforms (Windows CI exercises them via the
cross-platform subset). No write-path behavior change; Linux/macOS unaffected.
* fix(group): bound cross-repo crossing fan-out (LIMIT + segment memoization)
Address the top review residual: the bridge crossing query was unbounded and the
crossing-selection loop could run an O(2*N) sequential trace-BFS over every
ContractLink between a repo pair.
- CY_CROSSINGS_BETWEEN now ORDERs BY confidence DESC and LIMITs to
MAX_CROSSINGS_TO_TRY + 1; listCrossingsBetween slices to the cap and reports
truncation. Exceeding the cap surfaces a note (no silent truncation), keeping
the highest-confidence crossings. Aligns with the repo's anchored+LIMIT-bounded
query discipline (LadybugDB has no rel-property index).
- The home-repo segment (from -> consumer) depends only on the consumer uid and
the target-repo segment (provider -> to) only on the provider uid, so each is
memoized by that uid. Many crossings sharing a consumer/provider (one client
call linked to several providers) now cost one trace per distinct endpoint
instead of one per crossing. A consumer whose segment already failed is skipped
for every later crossing that shares it.
Test: two links sharing a consumer (first provider unreachable, second reachable)
assert the from->consumer segment is traced exactly once and the second crossing
wins.
* fix(group): restore Windows skip for bridge reopen tests; drop ineffective close-side probe
The previous commit flipped the bridge close-then-reopen tests to run on Windows,
betting that a close-side waitForWindowsHandleRelease + finalizeLbugSidecarsAfterClose
probe (mirroring the core adapter safeClose) would make the in-process reopen work
there. Windows CI proved otherwise: 4 writeBridge->openBridgeDbReadOnly tests fail
('expected null not to be null' — the read open returns null). The writable-close ->
read-open handoff plus writeBridge's atomic sidecar rename does not release the OS
file handle before the read open races, and the existing open-side LBUG_OPEN_RETRY
only retries lock-pattern errors, not the post-rename sidecar database-id mismatch.
macOS passes; the core adapter's own reopen also passes — this is bridge+Windows
specific.
- Revert itLbugReopen to the Windows skip (the pre-existing, correct state).
- Remove the close-side probe + finalize from closeBridgeDb: it did NOT close the
Windows gap, and reviewers flagged it for hot-path latency (finalize ran on every
close, all platforms) and safeClose duplication.
- KEEP the load-bearing fix — skipping CHECKPOINT on read-only handles — which fixed
the reproduced Linux/macOS in-process reopen artifact (the real bug).
Net: Linux/macOS repeated @group impact/trace works in-process; Windows in-process
bridge reopen remains a documented limitation (unchanged from before this PR).
* fix(group): surface degraded members + cap truncation; honest crossDepth schema
Address the cross-engine-corroborated tri-review findings (Codex + Claude):
- resolveAcrossMembers / runGroupTrace now track member repos that could NOT be
queried (resolveRepo or resolveSymbol threw) and, when the result is not_found,
attach a degraded-member note. A transient/corrupt member DB is no longer
silently reported as a clean 'symbol absent' not_found. (Codex B1+B3 + ce-reliability.)
- The cross-repo not_found now carries a programmatic truncated:true flag (and a
clearer suggestion) when the MAX_CROSSINGS_TO_TRY cap was hit, so a consumer can
distinguish 'no path' from 'cap may have hidden a connecting ContractLink'.
(Codex B3 + ce-adversarial + ce-api-contract.)
- trace tool schema: crossDepth maximum 10 -> 1 to match the implementation's
single-hop clamp (the schema previously advertised an unsupported 2-10 range).
(ce-api-contract, conf 100.)
Test: a member whose resolveSymbol throws yields not_found WITH a degraded note
naming the unreachable repo (if-free responder map).
* docs(group): clarify trace @group/memberPath is advisory (resolves all members)
Tri-review (Codex ce, conf 100) caught a doc/impl inconsistency: ARCHITECTURE.md
lumped trace with query/context/impact as honoring @group/memberPath member
scoping, but cross-repo trace resolves from/to across ALL members (the member
path is advisory). Clarify the behavior and point to from_uid/to_uid for
disambiguating same-named symbols across members.
* feat(group): file-level boundary fallback so cross-repo trace works on HTTP contracts
Benchmark (bench/cross-repo-trace/) running the REAL pipeline (runFullAnalysis
--pdg -> real syncGroup -> trace @group) found that cross-repo trace returned
not_found for real HTTP links even though sync built the correct ContractLinks:
HTTP (and other source-scan) contracts hardcode symbolUid:'' (http-route-extractor),
and both cross-trace AND cross-impact join crossings by Contract.symbolUid, which
never matches an empty uid. (Pre-existing — impact @group has the same gap.)
Fix: when a crossing's symbolUid is empty, fall back to the contract's FILE — if
the user's from/to resolves into the contract file, that endpoint anchors the
boundary. CY_CROSSINGS_BETWEEN now returns consumer/provider filePath; a crossing
is kept if it can be anchored by uid OR file on each side; a fileBoundaryFallback
note flags that the boundary is file-level, not symbol-precise. This makes the
common 'trace from=<calling fn> to=<handler fn>' case work end-to-end (verified:
fetchUsers -> listUsers stitches with a CONTRACT_LINK hop + PDG enrichment, 2/2).
Limits (documented in the bench README + the note): anonymous handlers have no
named target; when several contracts share files the file fallback may attach the
wrong contractId to a correct path. The proper upstream fix is to populate
symbolUid in the HTTP extraction (benefits impact too) — the bench is its gate.
Adds a unit test pinning the empty-symbolUid file-fallback stitch.
* fix(group): resolve HTTP contract symbolUid by containment (fixes cross-repo trace + impact)
Addresses the root cause behind the cross-repo trace file-fallback: HTTP
contracts hardcoded symbolUid:'' (http-route-extractor), so both cross-trace and
cross-impact — which join crossings on Contract.symbolUid — could not traverse
HTTP links. (Also found: the pre-existing graph-assisted resolution queried the
wrong edge, CONTAINS instead of DEFINES, so it never resolved a uid either.)
Now the extractor resolves each detection to a real symbol:
- HttpDetection carries the call-site line (node.ts sets it on every express/
fetch/axios/jquery/nest detection; express also captures the handler arg).
- resolveDetectionSymbol resolves the named handler first, else the innermost
Function/Method whose line span encloses the call (consumer = the function
containing the fetch; provider = the named/inline handler), over the correct
File-[DEFINES]->symbol edge. Base-tolerant (0- vs 1-based startLine).
- Wired into both source-scan and graph-assisted provider/consumer paths.
Verified end-to-end (bench/cross-repo-trace): all 4 contracts now carry real
uids, trace is symbol-precise (GET pair -> http::GET, POST -> http::POST, no
file-fallback note), and impact @group fans out (cross_repo_hits 0 -> 1). The
cross-trace file-level fallback remains as the secondary path for truly
anonymous handlers. Adds 2 containment unit tests; 738 group/integration pass.
Languages other than JS/TS still resolve providers by handler name; their
consumers fall through to the file fallback until their plugins set the line.
* fix(group): extend HTTP symbolUid containment to all languages + nested methods
Completes the symbolUid resolution across every bundled HTTP plugin: Python, Go,
PHP, Kotlin and Java now set the call-site line on their consumer (and Feign/
named) detections, so their HTTP contracts resolve to the containing function
the same way Node/TS already did.
Also generalizes the containment query: it now matches Function/Method/CodeElement
by filePath (UNION ALL) instead of File-[DEFINES]->symbol. The DEFINES edge only
reaches a file's TOP-LEVEL symbols, so methods nested in classes (Java/Kotlin —
File defines the class, the class defines the method) were invisible; matching by
filePath reaches them. Verified against a real index (LadybugDB supports the
UNION); JS/TS still fully symbol-precise (bench 2/2), 709 group tests pass.
Residual is now only the inherent case — a fully anonymous handler with no named
callee — which keeps the cross-trace file-level fallback.
* feat(group): destination trace — follow a consumer to an anonymous handler
Handles the one inherent residual: an anonymous route handler
(`router.get('/x', (req,res) => …)`) has no symbol node at all (the file holds
only a Const + PDG BasicBlocks), so it can never be named as a trace `to`.
Adds a DESTINATION TRACE: omit to/to_uid/to_file on an @group trace and
`trace from=<consumer>` follows the consumer's outgoing HTTP call across the
bridge and reports where it lands — by route + file:line, with a notes[] entry
flagging the handler as anonymous. Implemented as a new branch in runGroupTrace
(p.destination) backed by CY_CROSSINGS_FROM (all ContractLinks leaving the
consumer repo) + stitchToDestination; the provider endpoint is labelled
'<METHOD /path handler>' when its symbolName is a generic token/file basename.
The MCP routing already omitted an absent `to`, so only the schema docs changed.
parseTraceParams now treats a missing `to` as a destination trace instead of an
error. Verified end-to-end: anonymous fixture reports
'app/frontend:fetchUsers -> app/backend:<http::GET::/api/users handler>'; named
fixture lands at the real function. Adds 2 unit tests; 915 group tests pass.
* fix(group): tri-review fixes for cross-repo trace + symbolUid resolution
Two-engine tri-review (Claude swarm+ce + Codex GPT-5.5 swarm+ce+adversarial)
surfaced these; cross-engine-corroborated unless noted.
Correctness (P1, all four lanes): destination trace reported the WRONG endpoint
— an empty-uid consumer made trace(from->from) trivially succeed, so the highest-
confidence same-file crossing won regardless of which call `from` makes.
stitchToDestination now collects ALL connecting crossings, prefers symbol-precise
hits, and returns `ambiguous` (with candidates) when it cannot disambiguate.
Correctness (P1, Codex): resolveDetectionSymbol early-returned null when
d.line==null, blocking NAME resolution for named providers that set no line
(Spring/Go/etc.). Name resolution now runs first; only containment needs a line.
Correctness (P2): resolveContainingSymbol OR-ed `line` and `line-1`, which could
mis-pick a one-line sibling. It now probes the base-correct `line-1` first and
falls back to `line` only if nothing matches.
Correctness (Codex): anonymous Express handlers emitted name:'handler' and could
attach to an unrelated fn literally named `handler`. node.ts now emits name:null
for non-identifier handlers (containment-only).
Robustness: drop the first-symbol-in-file pickSymbolUid guess from the graph
consumer/provider paths (a wrong uid would win the contractId merge); remove the
dead CONTAINS_QUERY fallback (CONTAINS is File->Folder, never a symbol) + the now
-unused pickSymbolUid/handlerName; seed destination notes with degraded-member
notes so a successful trace still surfaces them; providerLabel takes providerUid
so a resolved fn named `handler` is not mislabeled anonymous, and only true file
basenames (known extensions) — not any dotted name — count as anonymous.
API contract: a single-repo trace with no `to` now returns an actionable error
(destination trace is @group-only) instead of "symbol 'undefined' not found".
Maintainability/tests: narrow asLocalTrace per-field (drop as-unknown-as); fix the
PR's lone as-any (vi.mocked); if-free e2e teardown; qualify the bench README.
Adds ambiguous-destination, anonymous-handler-no-false-name, and single-repo-no-to
tests; redirects graph mocks CONTAINS->UNION ALL. 918 group/integration pass.
* fix(group): carry degraded-member notes through SUCCESSFUL group traces
A reviewer (koriyoshi2041, PR #2269) correctly flagged that degraded-member
resolution was surfaced only on not_found, not on a successful ok result. Group
trace resolves names across ALL members, so an ok is 'unique among the members
we could query' — if a member that threw during resolveSymbol also holds from/to,
the real answer could be ambiguous. The destination path already seeded the note
(prior commit); this extends it to the same-repo and cross-repo success paths by
seeding the dispatch notes with degradedNotes([...fromRes.degraded, ...toRes.degraded]).
Adds a regression test: reg-be throws while a same-repo trace succeeds in reg-fe;
the ok result now carries the 'could not be queried' degraded note (app/backend).
* test(bench): cover all implemented cross-repo trace cases in one runner
Replace the single named-handler script with a self-contained verify.mjs that
generates each fixture inline and exercises every implemented end-to-end case
against the real analyze -> sync -> trace/impact pipeline, asserting PASS/FAIL
(exit non-zero on failure). 10 checks across 4 scenarios:
- named handlers: 4/4 symbolUid resolved; symbol-precise GET vs POST crossing
selection; destination trace lands at the named handler.
- anonymous handler: empty symbolUid; destination trace reports it by route with
the anonymous note.
- impact @group fan-out (cross_repo_hits >= 1).
- multi-language (Python Flask + requests): link built, cross-repo trace stitches,
and the file-level boundary fallback is exercised when the provider has no uid.
Ambiguous-destination and degraded-member paths need synthetic inputs the real
analyzer cannot produce, so they stay in the unit suite (documented in the README
+ script header). Removes verify-named.mjs + fixtures-named/ (folded inline).
* test(group): pin destination degraded-success + precise-tier ambiguity
Adds the two regression guards koriyoshi2041 requested on PR #2269 after the
degraded-on-success fix:
- destination trace success with a degraded member: reg-fe resolves from and
follows the link to an anonymous handler while reg-be throws; the ok result
carries the anonymous endpoint AND the 'could not be queried' degraded note, so
the no-to path stays aligned with explicit to traces.
- multiple PRECISE destination hits: one from reaches two consumers with resolved
uids linked to different routes; the result is ambiguous (role: to) with both
route candidates. Distinct from the existing file-level ambiguous test, this
pins the stronger precise tier against a future change silently picking the
highest-confidence destination.
Both already pass against current behavior; 716 group tests pass.
|
||
|
Gergő Magyar
|
78b4077d8a
|
feat(impact): opt-in PDG-backed impact mode - statement + inter-procedural slicing, resolved-callee-id soundness, mutation-oracle validated (#2227)
Some checks are pending
CodeQL / Analyze (javascript-typescript) (push) Waiting to run
CodeQL / Analyze (python) (push) Waiting to run
Gitleaks / gitleaks (push) Waiting to run
Publish / Classify release event (push) Waiting to run
Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions
Publish / ci (push) Blocked by required conditions
Publish / Publish to npm (push) Blocked by required conditions
Publish / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
|
||
|
Goutham Krishna Mandati
|
4c73b18387
|
feat(mcp): add trace tool for shortest call path between symbols (#2173)
Some checks are pending
CodeQL / Analyze (javascript-typescript) (push) Waiting to run
CodeQL / Analyze (python) (push) Waiting to run
Gitleaks / gitleaks (push) Waiting to run
Publish / Classify release event (push) Waiting to run
Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions
Publish / ci (push) Blocked by required conditions
Publish / Publish to npm (push) Blocked by required conditions
Publish / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
* feat(mcp): add trace tool for shortest call path between symbols (#1821) Implement the \ race\ MCP tool and \gitnexus trace\ CLI command that finds the shortest directed call path between two symbols using BFS over CALLS + HAS_METHOD edges. - MCP tool definition in tools.ts with READ_ONLY annotations - Directed BFS in local-backend.ts with parent-map path reconstruction - Symbol resolution via resolveSymbolCandidates (name/UID/file-hint) - Gap reporting with furthest reachable node and depth tracking - CLI wiring: gitnexus trace <from> <to> [--from-uid] [--to-uid] [--depth] - i18n keys in en.ts and zh-CN.ts + help-i18n.ts registration - ARCHITECTURE.md tools table entry - 16 unit tests (11 BFS core + 5 CLI wiring) * test(mcp): account for trace tool in tools.test.ts count The trace tool makes GITNEXUS_TOOLS length 15; update the hardcoded count, add 'trace' to the expected-names list, and refresh the stale "13 tools" comment and it() title. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): sanitize trace maxDepth to reject 0/NaN/negative `Math.min(params.maxDepth ?? 10, 30)` had no lower bound and `??` does not recover 0 or NaN, so `--depth 0|-5|abc` made the BFS loop run zero iterations and return a false `no_path`. Clamp at the real boundary with a `Number.isInteger && > 0` guard (the MCP inputSchema minimum is advisory only), and reject a non-numeric `--depth` in the CLI up front rather than forwarding NaN. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): check trace target before applying test-file filter The `isTestFilePath` filter ran before the target-equality check, but resolveSymbolCandidates does not exclude test-file symbols. A target (or a required hop) that lives in a test file was therefore skipped under the default includeTests=false and produced a false no_path with a misleading dynamic-dispatch suggestion. Match the explicitly-requested target first; non-target test-file nodes are still filtered. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(mcp): bound trace BFS with per-level LIMIT and visited cap The per-level query had no LIMIT and the visited set was uncapped, so a high-fanout hub could materialize an unbounded frontier. Cap per-level rows (interpolated LIMIT — Kuzu does not bind LIMIT) and the total visited set; either cap sets a `truncated` flag so a resulting no_path reports that the search was cut short rather than implying the graph was exhausted. Note: the sibling impact BFS shares the same unbounded pattern; applying the cap there is deferred (out of scope for this PR). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(mcp): clarify trace traverses call + class-member edges trace was advertised as a "shortest call path" but also traverses HAS_METHOD (class→member) containment edges so a class-rooted trace can descend into its methods. Keep that capability (consistent with impact/ context) and make the docs honest: rename EDGE_TYPES→TRAVERSAL_EDGE_TYPES, state the call + class-member traversal in the MCP/CLI/i18n/ARCHITECTURE descriptions, and note each hop's edge type is reported in edges[]. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): set status:'error' on trace failure responses Every trace return path sets a `status` discriminator except the caught-error path, so a consumer switching on `result.status` saw undefined on failure. Add status:'error' to both the backend trace() catch and the CLI traceCommand catch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): return a friendly error for non-string trace from/to A non-string from/to reaching resolveSymbolCandidates surfaced a low-level "x.includes is not a function" via name.includes. Guard the four name/uid params at the top of _traceImpl and return a structured status:'error' with a clear message instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(mcp): single row-decode + drop dead field in trace BFS Decode each BFS row once into named locals instead of repeating `(row.x ?? row[N])` across the two parent.set calls and the furthest-tracking. Drop the `type` field from the parent map value (it was written but never read), and rename the internal `deepestInfo` to `lastReached` for accuracy (the output field `furthest` is unchanged). Pure refactor — no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(cli): dedicated trace includeTests i18n key + guard coverage `trace|--include-tests` reused the impact help key, so rewording the impact option would silently change trace's help text. Add a dedicated help.option.trace.includeTests key in en + zh-CN and repoint it. Add CLI coverage for the (already symmetric) --from-uid/--to-uid flag-value guard and for --include-tests forwarding. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mcp): faithful BFS mock + expand trace coverage Fix makeResolveMock: concatenate neighbors across ALL frontier ids (it returned only the first node's, so a multi-node frontier was unmodelled) and key the UID branch on params.uid (the old query-text match never fired). Add coverage: shortest path through the second frontier node (proves the mock fix), confidence floor fallback, HAS_METHOD traversal with a mixed edge-type chain, no_path furthest:null, and from_file disambiguation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * style(trace): apply root prettier formatting to trace files The root `quality / format` gate (prettier --check, printWidth 100) runs on the full repo and flagged the trace sources/tests (the local config masks it). Reformat to root style — no behavior change; trace + tools suites and tsc stay green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(skills): document the trace tool for AI agents Add `trace` to the GitNexus skill docs so agents reach for it instead of hand-chaining context/impact hops. The guide gains a Tools Reference row and a "shortest path between two symbols" subsection (params, result shape, status/furthest/truncated semantics); the debugging skill gains a "how does A reach B?" pattern row and a trace tool example. Mirrored to the .claude and claude-plugin copies (byte-identical) and the cursor copy (compact style). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mcp): drop unused trace test fixtures (CodeQL js/unused-local-variable) CodeQL flagged two unused locals in the trace BFS tests: the top-level SYMBOL_C and a SYMBOL_D inside the maxDepth test (both defined, never referenced). Remove them. No behavior change — 58 trace/tools tests stay green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
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> |
||
|
Gergő Magyar
|
9ff7337f1e
|
fix(mcp): rename query/cypher params so Claude Code can call them (#2186)
* fix(mcp): advertise search_query/statement params for query/cypher tools (#2175) Claude Code drops a tool-call argument named exactly 'query', making the query and cypher tools unusable from it. Rename the advertised required parameters to search_query and statement so the client transmits them. Handler-side backward-compat for the legacy 'query' key follows in the next commit. * fix(mcp): accept search_query/statement with legacy query fallback (#2175) Resolve the new advertised param names in the backend while still accepting the legacy 'query' key, so curl/HTTP, other MCP clients, the CLI, the group path, and the internal executeCypher() all keep working. Alias is normalized once at the callTool chokepoint (covers group-forward + search alias); query() and cypher() dual-read defensively. New name wins when both are supplied. Updates the required-error message and adds dual-accept unit + integration coverage. * fix(cli): pass canonical search_query/statement params to query/cypher tools (#2175) Stop the CLI from depending on the deprecated 'query' alias. No user-facing change — the positional args are unchanged and the backend accepts both keys. * fix(mcp): generators advertise search_query in query() examples (#2175) Update the three doc/example generators (ai-context AGENTS/CLAUDE block, skill-gen community skills, resources repo hint) so future analyze runs emit query({search_query: ...}) — the param name Claude Code actually transmits. Tests assert the new form is present and the legacy query({query: form is absent (the #2059 generator-test pattern). * docs(mcp): advertise search_query/statement in skill & guidance examples (#2175) Sync the committed agent-facing docs to the renamed params so a Claude Code agent following them emits the transmittable key: AGENTS.md/CLAUDE.md gitnexus block, the canonical gitnexus/skills/* source and its installed/plugin/cursor mirrors, and the README examples. Scoped rewrite of the two call prefixes only (query({query: -> search_query, cypher({query: -> statement). * style(mcp): prettier line-wrap for #2175 alias-resolution edits * fix(review): uniform search_query precedence + cypher empty guard (#2175) Code-review findings (correctness/adversarial/api-contract/maintainability consensus): - Group-mode query inverted the 'new name wins' rule: the callTool chokepoint backfilled params.query only when empty and the @group-forward read params.query directly, so a both-keys (or whitespace-legacy) group call let the legacy value win — unlike the local path. Replace the hidden param mutation with a self-contained 'search_query ?? query' resolve at the group-forward; precedence is now uniformly new-wins at every consumer site. - cypher() now returns the same friendly required-param error as query() when neither statement nor query is supplied, instead of a raw DB prepare error. - Document the legacy alias as permanent (third-party clients may send query=). Adds group-forward alias tests (both-keys + legacy-only), empty/whitespace search_query, the search-alias path, and the cypher empty-statement guard. * fix(review): non-string alias safety + drop stale chokepoint comment (#2175) Tri-review findings (correctness/adversarial/security + maintainability): - Non-string statement/search_query/query (the MCP envelope is not schema-validated) hit .trim() and threw TypeError to the server boundary instead of a friendly required-param error. Introduce resolveAliasString() (new name wins; non-string -> undefined) used by query(), cypher(), and the group-forward, so all three return the structured error. Empirically verified (123 ?? '' -> 123, (123).trim() throws) — this overrides a critic refutation that mis-read ?? as a string coercion. - Remove the stale query() comment claiming alias resolution happens at a callTool chokepoint; that mutation was removed earlier in this PR — each site resolves the alias itself. - Document GroupToolPort.query's intentionally-narrower required type vs the wider LocalBackend impl. Adds non-string and empty-new-key precedence tests. * fix(mcp): alias falls back to legacy value when new key is blank (#2175) PR #2186 review finding: resolveAliasString used `canonical ?? legacy` (nullish), so an explicitly empty/whitespace new-name value (e.g. {search_query:'', query:'real'}) won and was rejected — discarding a valid legacy value, contradicting the 'new name wins when both supplied' intent. Resolve to the first NON-BLANK string instead (new preferred when it carries a real value, else legacy). Covers query(), cypher(), and the group-forward (all route through the helper); non-string still resolves to a friendly error. Flips the presence-based test and adds whitespace/cypher/group fallback cases. * fix(mcp): drop legacy "query" mention from query/cypher schema descriptions (#2175) PR #2186 review finding: the search_query/statement inputSchema descriptions named the legacy "query" key — the exact arg Claude Code drops — and description text is read by an LLM choosing arguments, weakly nudging it to send "query". Trim the descriptions to their clean form and move the legacy-alias note to a code comment next to the schema (preserved for maintainers / non-CC clients). properties/required unchanged (no `query`). |
||
|
Gergő Magyar
|
129bc84c0d
|
feat(taint): interprocedural taint via function summaries over resolved CALLS (#2084) (#2179) | ||
|
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. |
||
|
azizur100389
|
bdb824cfe4
|
feat(cli): add circular import cycle check (#2166) | ||
|
Gergő Magyar
|
7eaeb0a0c4
|
feat: multi-branch indexing and branch-scoped querying (#2106) (#2137)
Some checks are pending
Devcontainer Smoke / Config-transform unit tests (push) Waiting to run
Devcontainer Smoke / Build devcontainer image (push) Waiting to run
CodeQL / Analyze (javascript-typescript) (push) Waiting to run
CodeQL / Analyze (python) (push) Waiting to run
Gitleaks / gitleaks (push) Waiting to run
Publish / Classify release event (push) Waiting to run
Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions
Publish / ci (push) Blocked by required conditions
Publish / Publish to npm (push) Blocked by required conditions
Publish / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
* feat(git): add getCurrentBranch + resolveRefToCommit helpers (#2106) * feat(storage): branch-scoped getStoragePaths + branchSlug + resolveBranchPlacement (#2106) * feat(analyze): branch-aware indexing — per-branch slot, no overwrite (#2106) * feat(registry): nest non-primary branches under one path entry (#2106) * feat(mcp): optional branch scope on query tools + list_repos branches (#2106) * feat(cli): --branch on analyze + query/context/impact/cypher/detect-changes (#2106) * feat(cli): branch-aware list/status + per-branch staleness meta (#2106) * fix(review): apply autofix feedback - guard analyze against --branch != checked-out branch (prevents writing one branch's working tree into another branch's index slot) - fix branch-handle pool reinit thrash (track observed indexedAt by lbugPath, since applyBranchScope returns fresh handles) - remove dead resolveRefToCommit helper (staleness uses HEAD vs branch meta) - RepoListing.branches -> Omit<BranchSummary,'stats'> for type cohesion - add tests: branchSlug traversal containment, --branch mismatch reject, callTool branch threading, legacy-entry branch routing, status detached/stale * fix(review): address tri-review findings (#2106) - P1 data-loss: a detached-HEAD re-analyze (CI's actions/checkout default) no longer strips the primary's meta.branch stamp; preserve it so a later branch analyze cannot claim & overwrite the flat/primary index. +cascade integration test - P2: capture validateBranchName's trimmed return for --branch so a whitespace-padded value no longer false-rejects on-branch or ghosts an index - F1: on a lost/rebuilt registry, a branch run reconstructs the primary top-level entry from the flat meta, not the feature branch's meta * fix(storage): only trust a non-empty-string flatMeta.branch (#2106 R5) * fix(analyze): warn when the default branch is not the primary index (#2106 R8) * fix(mcp): resolve --branch <primary> on a legacy unstamped flat index (#2106 R4) * feat(cli): gitnexus clean --branch to remove a single branch index (#2106 R7) * fix(mcp): evict orphaned branch pools on unregister/clean (#2106 R3) * fix(analyze): union per-branch cache keys so a branch switch keeps shards (#2106 R6) * fix(analyze): normalize the auto-detected branch label via sanitizeDetectedBranch (#2106 R1) * fix(cli): skip AGENTS.md base_ref refresh for a non-primary branch fast path (#2106 R2) * fix(storage): atomic writeRegistry + re-read-before-write to narrow the registry race (#2106 R9) * refactor(storage): extract branch primitives to branch-index.ts (#2106 R10) |
||
|
Gergő Magyar
|
4682a477d8
|
feat(mcp): paginate list_repos to avoid client token truncation (#2119) (#2120)
* feat(mcp): paginate list_repos to avoid client token truncation (#2119) list_repos returned every indexed repository in one unpaginated array, which large/LLM MCP clients truncate by token limit — so agents with hundreds of indexed repos could not enumerate them all (the data transmits fully; the consuming client drops it). Add bounded limit/offset pagination to the list_repos tool: - result changes from a bare array to { repositories, pagination: { total, limit, offset, returned, hasMore, nextOffset } }; default page 50, max 200 (shared constants) - reject malformed limit/offset; clamp limit above the max - deterministic order (lower-cased name, then path) over one registry snapshot per call, so paging never skips or duplicates an entry - covers both stdio and remote /api/mcp (shared createMCPServer/callTool) The internal listRepos() method (5 callers), GET /api/repos, and the `gitnexus list` CLI are unchanged. The array->object tool-result shape is a deliberate contract change, documented in CHANGELOG. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): reject list_repos limit above the max instead of clamping (#2119) parseListReposPagination silently clamped limit>max to the maximum while throwing on every other out-of-bounds value (limit<1, offset<0, non-integer, NaN). A client that advanced offset by its requested limit (rather than pagination.nextOffset) then silently skipped repositories and saw hasMore:false — defeating the "never skips" guarantee. Reject an over-max limit too, so validation is symmetric and a caller never gets a smaller page than it asked for without a clear error. Updates the schema/description, the helper + ListReposPagination JSDoc, the guide note, and the two clamp tests. Resolves the cross-engine-corroborated P2 (Codex + adversarial lane) and the maintainability lane's clamp-vs-throw inconsistency from the PR #2120 review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(mcp): name the list_repos return type and mark the parser @internal Extract the inline listRepos() element shape into an exported RepoListing interface and use it for both listRepos() and listReposPage().repositories, replacing the opaque Awaited<ReturnType<LocalBackend['listRepos']>> expression the maintainability review flagged. Tag parseListReposPagination @internal (it is exported only for unit testing). Pure type/JSDoc change; no behavior. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(eval-server): type formatListReposResult to the paginated shape Narrow formatListReposResult's parameter from `any` to { repositories: RepoListing[]; pagination?: ListReposPagination } and drop the dead bare-array branch — after #2119 callTool('list_repos') always returns the paginated object, so the Array.isArray shim was unreachable. Add a list_repos continuation hint to the eval-server's getNextStepHint (parity with the MCP server), and cover the previously-untested non-empty + hasMore:false formatter branch. Migrates the two bare-array formatter tests to the object shape. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mcp): harden list_repos pagination coverage - Exercise the #2054 sibling-clone guarantee through the real callTool tool path (in the #2054 describe, which has temp-dir cleanup), proving siblings and remoteUrl survive listReposPage's sort+slice — not only listRepos(). - Assert total + limit on the middle-page test (a total miscalculation at a non-zero offset would otherwise slip past it). - Cover the benign boundaries: negative-zero offset (accepted as page 0) and a MAX_SAFE_INTEGER offset (empty page). - Replace the integration test's '\n\n---' split with a string-aware brace scan, so a repo path containing braces can never truncate the JSON parse. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(skills): sync the list_repos pagination example to the guide mirrors The .claude and gitnexus-claude-plugin guide mirrors only carried the one-line table note; add the full "Paginating list_repos" section (shape + multi-page traversal example + notes) so all three guide copies are byte-consistent with the canonical gitnexus/skills/gitnexus-guide.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: drop list_repos CHANGELOG entries from this PR Restore gitnexus/CHANGELOG.md to match main so this PR contributes no changelog change; the changelog is curated separately from feature PRs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
jelsco
|
11fc43b425
|
feat(impact): per-symbol processes field on byDepth items (#1867)
* feat(impact): per-symbol processes field on byDepth items
Today `impact` returns aggregated `affected_processes` at the top level
but the per-symbol `byDepth` items don't say which processes each caller
participates in. Consumers planning a deploy want to know if a given
caller is hit by a daily cron, a webhook, or a user-facing route - each
is a different deploy-risk profile - and that information requires a
follow-up cypher query per symbol today.
This change attaches `processes: [...]` to every `byDepth[depth][i]`
item, listing the processes that symbol participates in:
byDepth: {
"1": [
{
depth: 1,
id: "Function:src/foo.ts:doStuff",
name: "doStuff",
...
processes: [
{ id: "proc:cron_daily", label: "Daily cron",
processType: "cron", step: 12 }
]
}
]
}
The list is empty for symbols not in any process. Additive change, no
breaking modifications to existing fields.
Implementation:
- A second chunked Cypher pass runs after the existing per-process
aggregation pass, returning per-(symbol, process) rows. Same chunk
size and MAX_CHUNKS as the aggregation pass, so worst-case adds 10
extra round-trips bounded by the same env var.
- The enrichment pass is skipped entirely when `affectedProcesses.length
=== 0` (nothing to enrich) or `summaryOnly === true` (byDepth not
returned anyway).
- The aggregation query is unchanged - the new query has a distinct
RETURN shape (`RETURN s.id AS sid, ...`) so an existing unit test that
counts STEP_IN_PROCESS chunks was narrowed to match only the
aggregation pattern.
Tests:
- New: byDepth items always have a `processes` field (default empty
when no STEP_IN_PROCESS edges exist).
- New: when STEP_IN_PROCESS rows exist, the matching byDepth item
carries the right `{id, label, processType, step}` entry.
- Updated: impact-batching-grouping test mock narrowed to count only
aggregation chunks (the new per-symbol pass is covered separately).
* style: apply prettier to gitnexus/src/mcp/local/local-backend.ts
Pure line-wrap fix flagged by quality / format CI on PR #1867. Zero
semantic change: prettier broke a chained .slice().map() across three
lines instead of one. No test changes, no logic changes.
* fix(impact): address PR review findings on per-symbol process enrichment
- byDepth.processes doc now states each item carries processes (Finding 1)
- move per-symbol STEP_IN_PROCESS enrichment post-pagination so symbols
beyond the pre-pagination cap no longer get false-empty processes:[]
(Finding 2); hoist CHUNK_SIZE/MAX_CHUNKS to function scope so the
post-pagination pass can reference them
- dedup per-symbol query with DISTINCT + MIN(r.step) per (symbol,process)
pair (Finding 3)
- suppress the per-symbol pass under summaryOnly, incl. impactByUid group
fan-out, plus a test asserting the query never fires (Findings 4, 6)
* fix(impact): address second-round review findings A-E
Finding A (blocker): impactByUid passed summaryOnly:true, which drops the
entire byDepth field. cross-impact.ts reads fan.byDepth to build the group
by_depth output, so cross-repo by_depth was always {}. Replace with a new
skipPerSymbolEnrichment option on _runImpactBFS that suppresses only the
per-symbol STEP_IN_PROCESS pass while preserving byDepth.
Finding B+D (blocker): rewrite the byDepth.processes tool description. Drop
the stale "enrichment cap" wording (no longer true post-pagination), document
the {id,label,processType,step} entry shape, and tell agents to cross-check
affected_processes when partial:true.
Finding C: bound the post-pagination per-symbol enrichment loop to
MAX_CHUNKS*CHUNK_SIZE page IDs and surface partial:true when capped, so a
large page cannot trigger unbounded DB round-trips (DoD 2.6).
Finding E: add a test exercising the real impactByUid -> _runImpactBFS path
asserting byDepth survives and the per-symbol query never fires.
---------
Co-authored-by: scotjelinski <58397194+scotjelinski@users.noreply.github.com>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
|
||
|
Gergő Magyar
|
c916c88361
|
feat(mcp): add limit/offset/summaryOnly pagination to impact tool (#1818)
* feat(mcp): add limit/offset/summaryOnly pagination to impact tool (#414) The impact tool returns unbounded byDepth arrays for hub symbols (base error classes, shared utilities), producing 140KB+ responses that get truncated by MCP clients. maxDepth alone does not help when most dependents are at depth 1. Add three new parameters: - summaryOnly: returns counts/risk/processes/modules without byDepth - limit: caps symbols per depth level (default 100) - offset: skips symbols for pagination Also adds byDepthCounts to all responses so agents can see total counts even when the symbol list is paginated or omitted. Closes #414 * fix(mcp): prevent pagination from silently truncating cross-repo impact Address review findings on #1818: - F1 (blocker): _runImpactBFS no longer defaults to limit 100 when limit is not set — only _impactImpl (MCP entry) applies the default. Internal callers (impactByUid, group impact) get complete results. GroupToolPort.impact interface gains optional limit param, and cross-impact.ts passes limit: 10000 for local UID collection. - F2 (blocker): tool description updated — byDepth is now documented as paginated, not 'all affected symbols'. - F3: impactByUid calls _runImpactBFS without limit, so Phase-2 neighbor results are no longer capped at 100. - F4: pagination metadata now appears when offset > 0 (head truncation), not just tail truncation. Pagination.limit is null when uncapped. - F5: limit/offset schema types changed from number to integer; Math.trunc applied in implementation as defense-in-depth. - F6: 7 new tests — multi-depth pagination, offset-only truncation, offset past end, float inputs, _runImpactBFS internal uncapped path, collectImpactSymbolUids with paginated vs complete data. * fix(mcp): NaN guard on pagination params, complete GroupToolPort interface - Add Number.isFinite guard to limit/offset in _runImpactBFS so NaN inputs fall through to uncapped/zero defaults instead of producing silent empty byDepth with no truncation signal. - Add offset and summaryOnly to GroupToolPort.impact interface to match the implementation and prevent silent param loss at the port boundary. - Replace bounds-only toBeLessThan assertion with exact byDepthCounts and pagination assertions per DoD §2.7. * fix(mcp): address remaining review findings for impact pagination - #3: Forward limit/offset/summaryOnly through callToolAtGroupRepo so group-mode MCP callers can use the new pagination params. - #4: Extract GROUP_LOCAL_PHASE_LIMIT constant from magic 10000 in cross-impact.ts with a comment explaining the intent. - #7: eval-server formatImpactResult uses byDepthCounts[depth] for the 'and N more' suffix instead of paginated slice length. - #8: Extract ImpactParams interface from duplicate inline type definitions in impact() and _impactImpl(). - #9: Add --limit, --offset, --summary-only CLI flags to the impact command with i18n help strings (en + zh-CN). - #10: Clarify in tool description that limit/offset apply per depth level, not per total result set. * chore(autofix): apply prettier + eslint fixes via /autofix command * @ fix(mcp): address Copilot review feedback on impact pagination - Sanitize limit/offset with Number.isFinite in _impactImpl to prevent NaN passthrough from bypassing the default limit of 100 - Omit pagination.limit field instead of emitting null when paginationLimit is Infinity, keeping the response schema consistent - Move GROUP_LOCAL_PHASE_LIMIT after all imports in cross-impact.ts - Stop forwarding limit/offset/summaryOnly to group-mode impact since runGroupImpact overrides limit with GROUP_LOCAL_PHASE_LIMIT for UID collection and does not re-paginate - Validate CLI parseInt results with Number.isFinite before passing to the backend, falling back to undefined so defaults apply - Use byDepthCounts to decide whether to render depth sections in formatImpactResult, handling empty pages from offset past end @ * @ fix(mcp): address code review findings on impact pagination - Fix formatImpactResult "N more" count: use Math.min(items.length, 12) instead of hardcoded 12, so paginated pages with <12 items show the correct remaining count - Detect summaryOnly responses (byDepth absent, byDepthCounts present) and show a summary-mode message instead of misleading "(0 items on this page — adjust offset)" per depth level - Document that limit/offset/summaryOnly are single-repo only and ignored in group mode (@groupName) in MCP tool schema descriptions - List byDepthCounts in summaryOnly description and note byDepth absence when summaryOnly is true - Remove unused limit/offset/summaryOnly from GroupToolPort.impact interface since they are never forwarded to group impact - Deduplicate parseInt calls in CLI tool.ts: extract to local variables with consistent optional-chain usage @ * chore(autofix): apply prettier + eslint fixes via /autofix command * @ fix(group): restore limit in GroupToolPort.impact interface cross-impact.ts passes limit: GROUP_LOCAL_PHASE_LIMIT through the GroupToolPort.impact interface for UID collection. Only offset and summaryOnly were truly unused — limit must stay. @ * @ docs: add limit/offset/summaryOnly to impact tool options in README @ --------- Co-authored-by: Test <test@example.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
|
Copilot
|
7d500390b9
|
fix: Use Ladybug native read-only enforcement and prepared statement execution for Cypher query paths (#1655)
Some checks are pending
CodeQL / Analyze (python) (push) Waiting to run
Gitleaks / gitleaks (push) Waiting to run
Publish / Classify release event (push) Waiting to run
Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions
Publish / Publish to npm (push) Blocked by required conditions
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
CodeQL / Analyze (javascript-typescript) (push) Waiting to run
Publish / ci (push) Blocked by required conditions
Publish / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (push) Waiting to run
|
||
|
Nilotpal Kashyap
|
bdc0439a10
|
feat(detect-changes): support git worktrees (#1654) | ||
|
Temirkhan
|
1fe3bf9399
|
feat(mcp): add tool safety annotations (#1127)
* feat(mcp): add tool safety annotations * test(mcp): address PR #1127 review follow-ups - Replace private `_requestHandlers` SDK access in server.test.ts with `Client` + `InMemoryTransport.createLinkedPair()` for the tools/list annotation propagation test. The new path uses supported public APIs and surfaces SDK changes loudly instead of silently degrading. - Extract `OPEN_WORLD_READ_ONLY_TOOLS` set in tools.test.ts so future read-only open-world tools can be added without rewriting the invariant; preserves the current "only `query` is open-world" guard. - Add inline rationale on `group_sync` annotations explaining the conservative `idempotentHint: false` (writes contracts.json on every call even when output is deterministic). No runtime behavior change. Annotations themselves and tools/list shape are unchanged. --------- Co-authored-by: Gergo Magyar <gergomagyar@icloud.com> |
||
|
ivkond
|
00966630c4
|
feat: cross-repo impact analysis (#794) — @repo MCP routing + group resources (#984) | ||
|
azizur100389
|
131d411ae4
|
feat(mcp): rank context/impact disambiguation candidates and expose kind/file_path hints (#888)
* feat(mcp): rank context/impact disambiguation candidates and expose kind/file_path hints
The `context` MCP tool already returned `{ status: 'ambiguous', candidates }`
when a name hit multiple symbols, but the candidates were returned in
arbitrary DB order and the only hint it accepted was file_path. The
`impact` tool was worse: when its name resolver found multiple viable
matches it silently picked the first one from a priority UNION, with no
signal back to the caller that a different symbol might have been
intended.
Both failure modes were flagged in issue #470 and reconfirmed in the
comments by a second user who described impact as returning "incorrect
parsing results and meaningless tool calls" in the multi-match case.
Changes:
* Add `resolveSymbolCandidates(repo, query, hints)` private helper on
LocalBackend. Single place that:
- Short-circuits on direct uid (zero-ambiguity)
- Runs the same name-or-qualified-id match as before, with LIMIT 20
(was 10) so the ranker has headroom instead of arbitrary truncation
- Preserves the #480 Class/Constructor preference -- when the only
ambiguity is a Class and its own Constructor, the Class wins
silently
- Scores each candidate (pure TS, no extra DB round-trip): base 0.50,
+0.40 for file_path match, +0.20 for kind match, plus a small
kind-priority tiebreaker (Class > Interface > Function > Method >
Constructor) when no explicit kind hint is given
- Sorts desc by score with stable tiebreakers (shorter filePath,
then lex uid)
- Promotes to a single confident resolve when the top score is
>= 0.95 AND beats the runner-up by >= 0.10 -- lets a strong hint
cut through without forcing the caller through a disambiguation
round-trip
* Rewire `context()` to use the shared helper. Response shape is a
strict superset of today's: candidates gain a `score` field, the
existing `{ uid, name, kind, filePath, line }` keys are preserved so
every downstream consumer (rename, eval-server formatter, etc.) keeps
working. New `kind` input hint accepted.
* Rewire `impact()` to use the shared helper. Now emits the same
`{ status: 'ambiguous', candidates, impactedCount: 0, risk: 'UNKNOWN' }`
shape instead of silent first-pick. New inputs accepted:
`target_uid`, `file_path`, `kind`.
* Update tool schemas in mcp/tools.ts to advertise the new inputs and
describe ranked disambiguation.
Backward compatibility:
The #480 Class/Constructor collapse is preserved and covered by the
existing java-class-impact integration test (still green). The
ambiguous response shape is a strict superset -- `eval-formatters`
unit test that parses the old shape is unchanged and still passes.
`impact` going from silent-first-pick to structured ambiguous is a
semantic improvement that is the entire point of the issue; callers
relying on silent first-pick now get an actionable response.
Scope declined for v1:
module/community hint -- the issue lists it as one of several hints,
but kind + file_path cover the vast majority of disambiguation needs
in practice, and a community-label filter requires an extra graph
query per candidate. Natural v2 follow-up.
Tests: calltool-dispatch.test.ts gains 5 new cases covering file_path
boost, kind hint boost, impact ambiguous shape, impact target_uid
short-circuit, and score field presence on the existing ambiguous
test. Plus the extended assertions on the existing
`context tool returns disambiguation for multiple matches`.
Verification:
npx vitest run test/unit/calltool-dispatch.test.ts -> 64 pass
npx vitest run test/integration/java-class-impact.test.ts -> pass
npm run test:unit -> 3642 pass
(4 pre-existing env failures unchanged: skip-git-cli needs built
dist/, git-utils tmpdir on Windows worktree -- same on main)
npx tsc --noEmit -> clean
Closes #470
* fix(mcp): enrich labels from UNION when labels(n)[0] is empty; address review findings
CI on PR #888 caught 13 integration-test failures I did not cover locally:
my resolver refactor collected candidates via `labels(n)[0] AS type`, but
LadybugDB returns an empty string for that projection on certain node
types (most importantly Class). With an empty `type`, impact's downstream
`_runImpactBFS` no longer recognised `symType === 'Class' | 'Interface'`
and stopped seeding Constructor + File nodes into the frontier, so the
"impact(upstream) surfaces the file importer" assertion broke across 11
language fixtures plus 2 OVERRIDES filter tests.
The original impact resolver worked around this by running a prioritised
UNION across Class/Interface/Function/Method/Constructor and picking the
first hit. My refactor dropped that. Fix: keep the simple candidate MATCH
but enrich types afterward via a single scoped UNION query, so every
candidate carries an accurate label for both scoring and downstream
BFS seeding. The UID direct-lookup path is patched the same way.
Also addresses the findings from the senior reviewer on PR #888:
* MIGRATION.md: document the `impact` behavioural change (silent first-
pick → structured `{ status: 'ambiguous', candidates }`) so downstream
callers know to branch on `result.status` before reading byDepth/
summary. `context` is unchanged shape-wise (strict superset).
* New test: `context tool promotes top candidate via scoring when
multiple rows survive DB pre-filter`. The review flagged that the
existing file_path test works only because the mock ignores WHERE
parameters -- the scored-promotion path (top ≥ 0.95 AND gap > 0.09)
wasn't directly exercised. The new test uses two candidates both in
App.tsx-containing paths plus a kind hint so promotion is decided by
scoring, not DB pre-filtering. Also tightened the comment on the
earlier file_path test to describe the mock vs production divergence
honestly.
* NIT: added a paragraph explaining why `scored.length >= 2` is kept as
a defensive guard even though the `normalized.length === 1` early
return already covers the single-candidate path.
* Integration: two tests in `local-backend-calltool.test.ts` targeted
`'authenticate'`, which now correctly resolves as ambiguous (two
Method nodes: AuthService.authenticate and BaseService.authenticate).
Updated both to pass `file_path: 'src/auth.ts'` so they exercise the
new disambiguation API and still assert the METHOD_OVERRIDES filtering
they were originally about.
Edge case fix in the promotion gap check: IEEE754 makes 0.50 + 0.40 +
0.20 - 0.90 = 0.09999999999999998 instead of exactly 0.10, which would
otherwise break the "winner clearly dominates" intent for legitimate
1.00 vs 0.90 cases. Changed `>= 0.10` to `> 0.09`; same user-facing
intent, no floating-point sensitivity.
Verification (all from gitnexus/):
npx vitest run test/integration/class-impact-all-languages.test.ts
-> 52 pass (was 11 FAIL on CI before this fix)
npx vitest run test/integration/local-backend-calltool.test.ts
-> 18 pass (was 2 FAIL on CI before this fix)
npx vitest run test/integration/java-class-impact.test.ts
-> 10 pass (regression guard for #480 preserved)
npx vitest run test/unit/calltool-dispatch.test.ts
-> 65 pass (1 new test + 4 from original #470 PR)
npm run test:unit
-> 3626 pass, 4 pre-existing env failures unchanged
npx tsc --noEmit
-> clean
|
||
|
Gergő Magyar
|
0561d24efd
|
feat: METHOD_IMPLEMENTS edges, overload disambiguation, MethodExtractor unification (#574) (#642) | ||
|
ivkond
|
4fed097abb |
feat(group): add sync pipeline, CLI, MCP tools, and monorepo fixture
Wire extractors into the sync pipeline with service boundary detection. GroupService provides high-level API for all group operations. - Sync pipeline: orchestrates extraction (HTTP, gRPC, topics) with service boundary assignment and exact matching - GroupService: groupList, groupSync, groupContracts, groupQuery, groupStatus (groupImpact deferred to cross-repo follow-up PR) - CLI: group create/add/remove/list/sync/contracts/query/status - MCP tools: group_list, group_sync, group_contracts, group_query, group_status - Monorepo fixture: 3 services (auth/orders/gateway) connected via gRPC + Kafka + HTTP — all intra-repo cross-links discovered - Documentation: CLI commands and MCP tools added to both READMEs Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
Gergő Magyar
|
bf09eab95b
|
feat: configure prettier with pre-commit hook (#563)
* feat: configure prettier with pre-commit hook integration Add prettier, lint-staged, and prettier-plugin-tailwindcss at the repo root with husky pre-commit hook integration. Moves husky from gitnexus/ to root package.json for reliable hook installation. - Root package.json with prepare/format/format:check scripts - .prettierrc with endOfLine:lf and tailwindStylesheet for TW v4 - .prettierignore excluding fixtures, vendor, generated, *.d.ts, *.md - .gitattributes enforcing LF line endings for Windows consistency - Pre-commit hook uses direct node_modules/.bin/ paths (no npx) * style: apply prettier formatting to entire codebase One-time bulk format. No logic changes. Use .git-blame-ignore-revs to skip this commit in git blame. * chore: add .git-blame-ignore-revs for prettier format commit * perf: pre-commit hook runs only tests related to staged files Use vitest --related to scope test execution to tests that import the changed files, instead of running the full suite on every commit. * perf: remove vitest from pre-commit hook, keep in CI only Pre-commit now runs lint-staged + tsc only. Tests run in CI (ci-tests.yml) where they belong — keeps commits fast. * ci: add prettier format check to quality workflow PRs will now fail if code isn't formatted with prettier. |
||
|
marxo126
|
c437acf6bb
|
feat: deep flow detection — consumer access tracking, middleware chains, error shapes, api_impact tool (#482) | ||
|
Shunsuke Hayashi
|
b272c6864c |
docs(schema): add Community and Process node properties to cypher tool description (#411)
The cypher tool description and schema resource omit Community and Process node properties, causing agents to write failing queries on first attempt. Added property listings sourced from the actual LadybugDB schema definitions: - Community: heuristicLabel, cohesion, symbolCount, keywords, description, enrichedBy - Process: heuristicLabel, processType, stepCount, communities, entryPointId, terminalId Closes #411 |
||
|
Gergo Magyar
|
7b71b64427 |
fix(mcp): update tool descriptions for Phase 9C capabilities
- context tool: remove outdated "Phase 2" ACCESSES reference, document that CALLS edges resolve through field/method chains - cypher tool: fix Property query example to use declaredType (not description) - schema resource: add node_properties section documenting Method returnType, Property declaredType, Function parameterCount etc. - schema resource: clarify ACCESSES edge read/write coverage |
||
|
Gergő Magyar
|
973c7bfbf0
|
feat: ACCESSES edge type with read/write field access tracking (#372)
* feat: Phase 1 ACCESSES edge type — read tracking from chain resolution Add ACCESSES relationship type to track field read access during call chain resolution. When walkMixedChain resolves a field access (e.g., user.address.save()), an ACCESSES edge with reason 'read' is emitted from the calling function to the Property node. Schema: ACCESSES added to RelationshipType, REL_TYPES, VALID_RELATION_TYPES, context queries, tools/resources descriptions. Excluded from default impact BFS to prevent traversal explosion. Implementation: resolveFieldAccessType now returns FieldResolution with fieldNodeId. walkMixedChain accepts optional onFieldResolved callback. makeAccessEmitter factory provides Set-based dedup per source node. Bug fix: Added Java 'field_access' to FIELD_ACCESS_NODE_TYPES — was missing, causing extractMixedChain to fail for Java member access. * feat: Phase 2 ACCESSES write edges — assignment detection across 12 languages Add tree-sitter query patterns for field write detection (obj.field = value) across all supported languages: TS/JS, Python, Java, Go, C++, C#, Rust, PHP, Ruby (setter syntax), Kotlin, Swift. Processing: Sequential path handles assignment captures inline. Worker path extracts ExtractedAssignment data for deferred resolution via new processAssignmentsFromExtracted function. Bug fix: Kotlin/Swift assignment queries used invalid navigation_expression wrapper — fixed to match actual directly_assignable_expression AST structure. Tests: Write access integration tests for TS, Java, Python, Go with dedicated fixtures. All use strict toBe() assertions. * test: add unit tests for call-routing, shared type extractors, and symbol-table branches Add 215 new unit tests across 3 files to increase branch coverage toward the 23% global threshold (was 21.49%): - call-routing.test.ts (49 tests): Ruby call routing — require/require_relative, include/extend/prepend heritage, attr_accessor properties with YARD types - shared-type-extractors.test.ts (108 tests): pure string functions — extractElementTypeFromString, stripNullable, extractReturnTypeName, methodToTypeArgPosition, getContainerDescriptor - symbol-table.test.ts (+29 tests): Property/fieldByOwner index, metadata spread branches, lazy callable index, lookupExactFull shape * fix: defer write-access resolution to fix Ruby cross-file property timing Ruby attr_accessor properties are registered during processCalls (not the parsing phase), so lookupFieldByOwner fails when service.rb is processed before models.rb. Fix by collecting pending write-access edges during the file loop and resolving them after all files are done. Also adds write-access integration tests and fixtures for 7 languages (C++, C#, JS, Kotlin, PHP, Ruby, Rust), Ruby compound assignment query, PHP static property write query, and Kotlin property type extraction. * fix: address PR #372 review — write-access constructor bindings parity and docs - Add verified constructor bindings fallback to write-access resolution in both sequential path (receiverIndex lookup) and worker path (constructorBindings param for processAssignmentsFromExtracted), closing the read/write ACCESSES edge asymmetry for factory-returned receivers - Clarify inner guard control flow comment in processCalls match loop - Document Go inc_statement/dec_statement gap in roadmap - Clarify PHP nullsafe write footnote (invalid syntax, not just untracked) - Update symbol-table tests for intentional fieldByOwner behavior change (Properties without declaredType now indexed for dynamic language write-access tracking) |
||
|
Gergő Magyar
|
11a3d0515c
|
feat: Phase 8 field/property type resolution (#354)
* feat: Phase 8 field/property type resolution — resolve chained member access
Add field/property type extraction to the type resolution system so that
chained member access like `user.address.save()` resolves the intermediate
receiver type (`address → Address`) through Property symbols in SymbolTable.
Key changes:
- SymbolTable: add `declaredType` field, `fieldByOwner` O(1) index,
`lookupFieldByOwner()` method, P0 conditional callableIndex invalidation,
P2 exclude Properties from globalIndex to prevent namespace pollution
- tree-sitter queries: add `definition.property` for TypeScript, Java, Go
- parse-worker: extract declared types for Property nodes via
`extractPropertyDeclaredType()`, capture field-access receiver info
- call-processor: add `resolveFieldAccessType()` helper and field-access
branch in both sequential and worker receiver resolution paths
- Integration tests: new field-types test suite verifying end-to-end
`user.address.save() → Address#save` resolution
* fix: Go tree-sitter query captures field_declaration not field_declaration_list
Post-review fix: the Go struct field query incorrectly put @definition.property
on field_declaration_list (the list container) instead of field_declaration
(the individual field). Also removed unused `language` parameter from
extractPropertyDeclaredType.
* feat: expand field-type tests to 6 languages, fix Go ownerId and Kotlin navigation_expression
- Add integration test fixtures for Java, C#, Go, Kotlin, PHP (alongside existing TS)
- Fix Go: add type_declaration handling in findEnclosingClassId for struct fields
(field_declaration → field_declaration_list → struct_type → type_spec → type_declaration)
- Fix Kotlin: add navigation_expression handling in field-access resolution
(Kotlin uses navigation_expression + navigation_suffix, not member_expression)
- Add extractMemberAccessParts helper in call-processor for cross-language member access
- All 24 field-type tests pass across 6 languages, 181 Go+Kotlin tests pass with no regressions
* refactor: split HAS_METHOD into HAS_METHOD + HAS_PROPERTY edge types
Property nodes now use HAS_PROPERTY edges instead of HAS_METHOD, giving
the graph schema proper semantic separation between methods and fields.
- HAS_METHOD: Method, Constructor, Function (when inside a class)
- HAS_PROPERTY: Property nodes (class fields, struct fields, attributes)
MRO processor only reads HAS_METHOD — properties correctly excluded from
method resolution order. Impact analysis accepts both edge types.
Updated 12 files: graph types, schema, tools docs, parse-worker,
parsing-processor, call-processor, and 6 test files.
* fix(test): update security test to expect 7 VALID_RELATION_TYPES (added HAS_PROPERTY)
* test: add unit tests for Phase 8 SymbolTable features (39 tests, up from 19)
Cover all new branches: declaredType metadata, Property exclusion from
globalIndex, conditional callableIndex invalidation, lookupFieldByOwner
(happy path + edge cases), lookupFuzzyCallable filtering, and clear()
with fieldByOwner. Fixes branch coverage threshold (21.8% → 23%+).
* feat: Phase 8B mixed field+method chain resolution, C++/Rust chain fixes
Unify field and method chain resolution into a single `extractMixedChain`
walker that handles interleaved patterns like `svc.getUser().address.save()`.
Fix C++ chain calls (tree-sitter-cpp `field_expression` uses `argument` not
`object`), Rust unit struct instantiation (`let svc = TypeName;`), and add
stdlib passthrough for `unwrap()`/`clone()`/`expect()` in chain loops.
Key changes:
- Replace `receiverCallChain` + `receiverFieldAccess` with unified
`receiverMixedChain: MixedChainStep[]` on ExtractedCall
- Add `extractMixedChain` in utils.ts (handles both call_expression and
field_expression nodes, including C++ `argument` field)
- Add `TYPE_PRESERVING_METHODS` set for stdlib identity operations
- Add C++ inline method double-indexing guard in parsing-processor.ts
and parse-worker.ts
- Add Rust unit struct recognition in type-extractors/rust.ts
- Split field-types.test.ts into per-language test files
- Add ts-mixed-chain fixture and integration tests
- Resolve rust.test.ts todo: Option<T>.unwrap().save() now works
- Update roadmap: Phases 7+8 complete, Phase 9 is next
* fix: Python declaredType extraction and sequential-path property registration
- Move @definition.property capture from expression_statement to assignment
node in Python queries so Strategy 1 childForFieldName('type') succeeds
- Pass item.declaredType through ctx.symbols.add in sequential call-processor
path, matching worker path behavior (fixes Ruby YARD declaredType drop)
- Add Python chain resolution integration test (user.address.save → Address#save)
- Update Rust/Python status in roadmap and system docs to reflect actual coverage
* fix: Python/Ruby field type disambiguation and Rust chain test
Three fixes from PR #354 third review:
1. Python typed_parameter name extraction: tree-sitter-python's
typed_parameter uses positional children for the name, not a named
field. TypeEnv and extractParameter now fall back to firstNamedChild.
2. Ruby/Python call-step field resolution: Ruby's AST uses `call` nodes
for both property access and method calls. The chain walker now tries
resolveFieldAccessType before resolveCallTarget for call steps, so
attr_accessor properties resolve via declaredType.
3. Rust chain resolution test: added missing integration test asserting
user.address.save() resolves to Address#save.
Also splits C/C++ and TS/JS columns in type-resolution-system.md
language matrix with footnotes for accuracy.
1062 resolver integration tests passing, 0 failures.
* refactor: Phase 8 code review cleanup — extract walkMixedChain, fix MCP agent gaps
- Extract duplicated chain resolution loop into shared walkMixedChain() helper,
eliminating ~60 lines of copy-pasted code between sequential and worker paths
- Add returnType to ResolveResult, removing redundant lookupFuzzy+find per chain step
- Fix context() tool to include HAS_METHOD, HAS_PROPERTY, OVERRIDES in queries
so agents can discover class members
- Fix p.declaredType Cypher example (column doesn't exist) → p.description
- Add HAS_METHOD, HAS_PROPERTY, OVERRIDES to schema resource
- Document HAS_METHOD/HAS_PROPERTY in impact tool description
- Delete dead code extractMemberAccessParts (superseded by extractMixedChain)
- Replace any with SyntaxNode on extractPropertyDeclaredType
- Add Rust deep-field-chain test (5 tests), Java mixed-chain (4), Go mixed-chain (4)
- All 1075 tests pass (13 new, 0 regressions)
* refactor: type SymbolDefinition.type as NodeLabel, add O(1) receiver index
- Change SymbolDefinition.type from string to NodeLabel union (35 members)
across symbol-table.ts, parse-worker.ts, parsing-processor.ts — compiler
now enforces correctness at all comparison/assignment sites
- Replace O(N*M) linear scan in lookupReceiverType with pre-built
ReceiverTypeIndex (Map<funcName, Map<varName, Entry>>) for O(1) lookups
with proper ambiguity handling and file-level fallback
- All 1075 tests pass, 0 regressions
* fix: capture C++ pointer/ref fields, Kotlin data class props, PHP constructor promotion
Add tree-sitter query patterns for three previously missed property declaration
forms: C++ pointer/reference member fields (Address* addr; Address& ref;),
Kotlin primary constructor val/var parameters (data class User(val name: String)),
and PHP 8.0+ constructor property promotion (public Address $address).
Fix "10 languages" off-by-one in docs (Ruby is single-level only, not deep chain).
Update Python feature matrix cell from No* to Yes* after
|
||
|
Gergő Magyar
|
1afe9166aa
|
feat: language-aware code intelligence — symbol resolution, MRO, constructor discrimination (#238)
* feat: add Method Resolution Order (MRO) with language-specific rules
Implement full MRO computation for multi-language inheritance hierarchies:
- HAS_METHOD edges: Class→Method ownership edges emitted during parsing
(both worker pool and sequential fallback paths)
- Method signatures: extract parameterCount and returnType from AST nodes
- C# heritage fix: distinguish EXTENDS vs IMPLEMENTS for base_list captures
using symbol table lookup + I[A-Z] naming heuristic fallback
- MRO processor (Phase 4.5): walks inheritance DAG, detects method-name
collisions across parents, applies language-specific resolution:
- C++: leftmost base class in declaration order wins
- C#/Java: class method wins over interface default
- Python: C3 linearization with cycle detection
- Rust: no auto-resolution (requires qualified syntax)
- Default: first definition in BFS order wins
- OVERRIDES edges emitted for resolved method collisions
- KuzuDB schema: Method table extended with parameterCount/returnType;
dedicated CSV writer and COPY query for 10-column Method rows
- MCP tools: updated Cypher examples for HAS_METHOD, OVERRIDES, diamond
72 tests across 5 test files covering MRO resolution, HAS_METHOD edges,
method signature extraction, C# heritage resolution, and integration
tests across C#/Rust/Python/TS/Java/C++.
* feat: add scope-based symbol resolution replacing raw lookupFuzzy
Introduces a shared 3-tier resolveSymbol function used by both
heritage-processor and call-processor:
1. Same-file (lookupExactFull — authoritative)
2. Import-scoped (filtered by ImportMap — high confidence)
3. Global fuzzy (first match — low confidence fallback)
Adds lookupExactFull to SymbolTable returning full SymbolDefinition
with type info needed for heritage Class/Interface disambiguation.
* refactor: tighten symbol resolution — Tier 3 refuses ambiguous matches
- lookupExactFull now O(1) via direct SymbolDefinition storage in fileIndex
(shared object references with globalIndex — zero additional memory)
- Added resolveSymbolInternal() preserving { definition, tier, candidateCount }
for test assertions and logging
- Tier 3 now returns null when multiple global candidates exist instead of
arbitrary allDefs[0] — a wrong edge is worse than no edge
- call-processor: renamed fuzzy-global → unique-global, removed dead branch
- 12 new tests: tier assertions, ambiguous refusal per language family,
heritage false-positive guard, O(1) shared reference verification
* fix: critical language support bugs in import resolution and MRO
Phase 5 critical fixes from all-language analysis:
- Python: add relative_import query capture (PEP 328) — `.models`, `..utils`
were silently dropped, producing zero ImportMap entries
- Rust: extract prefix from grouped imports `crate::module::{A, B}` — brace
groups previously failed resolution entirely
- Swift: use normalizedFileList for Windows path compatibility in module
import resolution (matches Go's resolveGoPackage pattern)
- MRO: fix c_sharp → csharp language name mismatch (enum is 'csharp'),
add Kotlin to C#/Java resolution rules (class method wins over interface)
* feat: add strict multi-language integration tests + fix C/C++ import resolution
Add 32 integration tests across 6 language fixtures (TypeScript, C#, C++,
Java, Python, Rust) with exact toBe/toEqual assertions validating heritage
edges, import resolution, and trait implementations.
Fix C/C++ import resolution bug where dot-to-slash conversion mangled
include paths (e.g. "animal.h" became "animal/h"). Now skips conversion
for C/C++ languages which use actual file paths in #include directives.
* fix: language-gate heritage heuristic, add Swift extension heritage, handle Rust grouped imports
- Gate I[A-Z] naming heuristic to C#/Java only (was firing for all languages)
- Swift unresolved types default to IMPLEMENTS (protocol conformance is the norm)
- Add tree-sitter query for Swift extension protocol conformance (extension Foo: Protocol)
- Handle Rust top-level grouped imports (use {crate::a, crate::b}) in both import loops
- Add 4 new heritage-processor tests (TypeScript refusal, Swift default, Swift Tier 1)
* feat: add Go struct embedding heritage + PackageMap optimization
Add Go struct embedding detection (anonymous fields → EXTENDS edges) via
new tree-sitter heritage query with named-field filtering in both
parse-worker and heritage-processor paths.
Implement PackageMap optimization for Go cross-package resolution:
replace O(N) file-level ImportMap expansion with directory-level suffix
matching (Tier 2b in symbol resolver). Graph IMPORTS edges are preserved
via addImportGraphEdge split.
Remove overly broad @definition.type from GO_QUERIES that was
double-matching structs/interfaces as TypeAlias nodes, breaking Tier 3
unique-global resolution.
Add Go fixture (go-pkg) with Admin→User embedding, cross-package calls,
and 7 integration tests covering structs, functions, imports, calls,
and heritage edges.
* test: add Kotlin heritage integration tests
Adds a kotlin-heritage fixture and 7 integration tests validating
class inheritance, interface implementation, JVM-style import
resolution, and symbol-table-driven EXTENDS/IMPLEMENTS disambiguation
via Kotlin delegation specifiers.
* feat: extract resolvers, add PHP tests, ambiguous tests for all languages
- Extract language-specific resolvers from import-processor.ts into
resolvers/ directory (P7): jvm, go, csharp, php, rust, standard, utils
- import-processor.ts reduced from 1412 to 711 lines (50% reduction)
- Add comprehensive PHP integration tests: PSR-4 imports, traits, enums,
heritage edges, method calls, MRO overrides
- Add ambiguous symbol resolution tests for all 9 languages verifying
correct disambiguation via import chains
- Split monolithic lang-resolution.test.ts (1080 lines) into 9 per-language
files under test/integration/resolvers/ with shared helpers
* feat: update integration tests to include resolver tests for multiple languages
* fix: address code review — schema gap, Rust impl name, Property OVERRIDES
Bugs fixed:
- Add 13 missing FROM/TO pairs in RELATION_SCHEMA for HAS_METHOD edges
(Class/Interface/Struct/Trait/Impl/Record to Method/Constructor/Property)
- Fix findEnclosingClassId to pick implementing type for Rust
impl Trait for Struct blocks (was picking trait name)
- Exclude Property nodes from MRO OVERRIDES collision detection
- Change MRO language fallback from typescript to unknown
Tests added:
- Unit: Property OVERRIDES exclusion (2 tests), Rust impl Trait for
Struct name resolution (2 tests), schema HAS_METHOD pair coverage
- Integration: no OVERRIDES targets Property nodes across all 9 languages
- PHP fixture: added shared $status property to both traits to create
real collision scenario for Property OVERRIDES exclusion test
Documentation:
- OVERRIDES edge direction (Class to Method), Go return type gap,
BFS first-reach heuristic limitation
* feat: harden CALLS-edge resolution — Phase 0 validation
- Fix same-file confidence (0.85 → 0.95) to correctly outrank import-scoped (0.9)
- Fix Tier 1 overload preservation: use globalIndex filter instead of fileIndex lookup
- Add callable-kind guard: refuse CALLS edges to Interface and Enum symbols
- Fix Kotlin countCallArguments: handle call_suffix → value_arguments nesting
- Fix Kotlin extractFunctionName: add simple_identifier to fallback search
- Strictly type findParameterList and countCallArguments (remove all `any`)
- Add arity-based call resolution integration tests for 9 languages
- Add unit regression tests for Interface/Enum CALLS refusal
* chore: remove C# build artifacts from fixtures
* feat: add call-form discrimination and ownerId to symbol table (Phase 1)
Add inferCallForm() and extractReceiverName() to distinguish free/member/constructor
calls at the AST level across all 9 languages. Add ownerId field to SymbolDefinition
linking Method/Constructor/Property to their owning class. Includes 36 unit tests
and member-call integration tests for all 9 languages (132 tests, 0 failures).
* feat: constructor/struct-literal resolution across all languages (Phase 2)
Add constructor discrimination to CALLS-edge resolution: new Foo(),
User{...} struct literals, and C# primary constructors now resolve to
Constructor/Class/Struct/Record nodes instead of being filtered out.
Queries: new_expression (C++), object_creation_expression (PHP),
composite_literal (Go), struct_expression (Rust), primary constructor
and implicit_object_creation_expression (C#).
Relaxes global tier in collectTieredCandidates to pass all candidates
through filterCallableCandidates, allowing kind/arity narrowing to
disambiguate at lower confidence.
* feat: receiver-constrained resolution with integration tests for all 9 languages
Add receiver-type filtering (Phase 3): when a member call like `user.save()`
has a known receiver type from TypeEnv, filter candidates by ownerId to
disambiguate methods with the same name across different classes.
Key changes:
- call-processor: build per-file TypeEnv, pass receiverTypeName to resolveCallTarget
- parse-worker: extract receiverTypeName from TypeEnv in worker thread
- resolveCallTarget: new step D filters by ownerId matching receiver type
- utils: extractReceiverName supports C++ field_expression (argument field)
- utils: findEnclosingClassId extracts Go method receiver types
- type-env: handle Go qualified_type, Kotlin user_type/variable_declaration
- parse-worker + parsing-processor: Function added to needsOwner for
Kotlin/Rust/Python class methods captured as Function nodes
Integration tests added for receiver-constrained resolution across all 9
languages: TypeScript, Java, Python, Go, Rust, C++, C#, Kotlin, PHP.
* feat: NamedImportMap, scoped TypeEnv, broadened signatures + TS rest-param variadic fix
Address all 4 PR #238 review items:
1. Remove redundant lookupFuzzy in processRoutesFromExtracted
2. Add NamedImportMap for TS/Python symbol-level import tracking (Tier 2a)
3. Make TypeEnv scope-aware (Map<scopeKey, Map<varName, type>>) to fix
non-deterministic receiver resolution across functions
4. Broaden extractMethodSignature: Go/Rust/C++ return types, variadic
detection for Go/Java/Python/C++/Kotlin/TypeScript rest params
Discovered and fixed: TS rest params (...args) were not detected as
variadic — added rest_pattern detection inside required_parameter nodes.
Integration tests added: scoped receiver, named import disambiguation,
and variadic call resolution for both TypeScript and Python.
* fix: alias import resolution, Go multi-assign TypeEnv, dead code removal
- NamedImportMap now stores {sourcePath, exportedName} so aliased imports
(import { User as U }) resolve U → User in the source file
- Named binding check moved before empty-allDefs early return in both
call-processor and symbol-resolver, fixing constructor calls via aliases
- Go extractFromGoShortVarDeclaration iterates all LHS/RHS pairs for
multi-assignment (user, repo := User{}, Repo{}) instead of only first
- Remove unused TYPED_DECLARATION_TYPES set (TYPED_PARAMETER_TYPES kept)
- Integration tests for both fixes (go-multi-assign, typescript-alias-imports)
* feat: alias import extraction for Kotlin, Rust, PHP, C# + integration tests
Add named import alias extraction to both pipeline paths
(import-processor.ts and parse-worker.ts) for Kotlin, Rust, PHP,
and C#. Add integration test fixtures and tests for all 5 languages
(Python alias extraction already worked, just needed the test).
Each test verifies: class detection, member call resolution through
aliases to correct target files, and IMPORTS edge emission.
* refactor: use SupportedLanguages enum everywhere instead of raw strings
Replace all raw language string literals and `language: string` types
with the SupportedLanguages enum across 10 files. This ensures
compile-time safety for language dispatch and eliminates dead
`language === 'tsx'` checks (tsx maps to TypeScript in the enum).
* fix: tier-ordering bug, re-export chains, PHP grouped imports, Java named imports
- Fix collectTieredCandidates tier-ordering: same-file now checked before
named bindings, preventing imports from shadowing local definitions
(matches resolveSymbolInternal priority order)
- Add re-export chain resolution for TypeScript/JavaScript barrel files:
export { X } from './base' and export type { X } from './base' now
followed up to 5 hops through NamedImportMap
- Fix PHP grouped import alias extraction: use App\Models\{User, Repo as R}
now correctly handled in both parse-worker and import-processor
- Add Java NamedImportMap support: import com.example.models.User now
records User as a named binding for precise disambiguation
- Add 16 new integration tests across TypeScript, PHP, and Java resolvers
(220 total resolver tests, all passing)
* refactor: consolidate alias extraction + add variadic/constructor/shadow integration tests
- Extract shared named-binding-extraction.ts from duplicate logic in
import-processor.ts and parse-worker.ts (net -200 lines)
- Deduplicate appendKotlinWildcard (now imported from resolvers/index.ts)
- Add integration tests: constructor calls (Kotlin, Python), variadic
resolution (Go, Java, C#, C++, Kotlin), re-export chains (Python),
local definition shadowing (Python, Go)
- Add TODO(stack-graph) for TypeEnv scope key collision
- 225 integration tests passing (was 223)
* fix: PHP non-aliased imports, Python node identity, re-export chain dedup + local-shadow tests
- PHP flat non-aliased imports (use App\Models\User) now stored in NamedImportMap
- PHP grouped non-aliased imports ({User} in {User, Repo as R}) now stored in NamedImportMap
- Python: replace non-public child.id with child.startIndex for node identity
- Extract shared walkBindingChain() from symbol-resolver and call-processor
- Add PHP variadic resolution fixture + test (variadic_parameter already covers PHP)
- Add local-shadow integration tests for Java, C#, Kotlin, Rust, PHP, C++ (6 languages)
* feat: Rust non-aliased use bindings, Kotlin non-aliased imports, re-export chain resolution
Extend NamedImportMap coverage for Rust and Kotlin non-aliased imports:
- Rust: rename collectUseAsClauses → collectRustBindings, extract terminal
scoped_identifier (use crate::models::User) and identifier in use_list
(use crate::models::{User, Repo}) into NamedImportMap. This also enables
pub use re-export chain following via walkBindingChain.
- Kotlin: extend extractKotlinNamedBindings to handle non-aliased imports
(import com.example.User), skipping wildcard imports.
- Add rust-reexport-chain fixture + 3 integration tests verifying Handler{}
resolves through mod.rs pub use to handler.rs.
- Add Kotlin heritage + constructor-calls reason assertions for non-aliased
import-resolved resolution.
- Add C# heritage test documenting namespace import tier behavior.
* fix: skip Kotlin lowercase member imports in NamedImportMap
Member imports like `import util.OneArg.writeAudit` (lowercase last
segment) must not populate NamedImportMap — same-named function imports
from different classes collide, breaking arity-based disambiguation.
Apply the same guard Java already uses: skip lowercase last segments.
* fix: skip spurious path-prefix bindings in Rust grouped imports
collectRustBindings was extracting the path segment (e.g. "models") from
`use crate::models::{User, Repo}` as a spurious NamedImportMap entry.
Skip scoped_identifier nodes that are direct children of scoped_use_list
since they are path prefixes, not importable symbols.
Adds rust-grouped-imports fixture and 4 integration tests verifying both
symbols resolve correctly and no spurious binding leaks through.
* fix: use startIndex in TypeEnv scope key to prevent same-name method collision
Two methods named identically in different classes within the same file
previously shared a scope key, causing non-deterministic type resolution.
Now keys use funcName@startIndex for uniqueness.
Also adds tests documenting destructuring assignment extraction gap.
* test: document C# namespace-level import limitation in named binding extraction
* test: document same-arity overload discrimination limitation in call processor
* perf: parallelize calls/heritage/routes processing in worker path
Worker path now runs processCallsFromExtracted, processHeritageFromExtracted,
and processRoutesFromExtracted via Promise.all instead of sequentially.
Safe because all three only read shared state and write via addRelationship's
dedup guard. Sequential fallback path stays sequential (shared LRU astCache).
Also fixes Rust collectRustBindings spurious path-prefix bindings for 3+ level
grouped imports, and adds @param JSDoc for walkBindingChain's allDefs invariant.
* docs: improve Promise.all safety comment and walkBindingChain JSDoc
Clarify that the parallelization safety comes from disjoint relationship
types + idempotent id-keyed Maps, not from lack of shared state (the
graph is shared). Strengthen allDefs JSDoc to describe silent-miss
consequence of passing pre-filtered results.
* refactor: extract language-specific processing into modular dispatch tables
Phase 1: Extract type binding logic from type-env.ts (635→125 LOC) into
type-extractors/ directory with per-language files and Record<SupportedLanguages,
LanguageTypeConfig> + satisfies dispatch.
Phase 2: Extract 5 config loaders from import-processor.ts into
language-config.ts (removed ~196 LOC of inline loaders).
Phase 3: Convert export-detection.ts switch/case to exhaustive
Record<SupportedLanguages, ExportChecker> + satisfies dispatch table,
fix node: any → SyntaxNode.
Also adds language feature matrix to README.
All 1146 unit tests and 433 integration tests pass.
* refactor: extract type binding logic into type-extractors/ directory (Phase 1)
Extract per-language type extraction from type-env.ts (635→125 LOC) into
type-extractors/ with Record<SupportedLanguages, LanguageTypeConfig> + satisfies
dispatch. 9 per-language files, shared helpers, and barrel index.
* refactor: extract config loaders to language-config.ts (Phase 2)
Move 5 language-specific config loaders and their type interfaces from
import-processor.ts into standalone language-config.ts module.
|
||
|
abhigyanpatwari
|
91289404c2 |
feat(gitnexus): v1.2.9 — impact enrichment, cypher markdown, Windows setup fix
- Impact tool now returns risk score, affected processes/modules, and summary - Cypher tool formats results as markdown tables for LLM readability - Context tool includes module (functional area) field - Semantic search skips model init when embeddings are disabled - Setup: wrap npx in cmd /c on Windows for .cmd script compatibility - Embedder: silence stderr during ONNX model load to protect MCP stdio - API: use executeCypher directly to avoid double formatting - Add community integrations section to READMEs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> |
||
|
abhigyanpatwari
|
eca55aacd7 | fixed resource count multiplying issue ( using resource templates now ) | ||
|
abhigyanpatwari
|
96e1d799c8 | agent md experiments | ||
|
abhigyanpatwari
|
dda8de41a3 | MCP and cli fixes | ||
|
abhigyanpatwari
|
1735c0ffcd | gitnexus wal cleanup preventing reindexing issue fixed |