mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-08-28 05:25:25 +00:00
1760 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Gergő Magyar
|
9372b17049
|
fix(python): resolve calls through an unaliased dotted namespace import (#2826) (#2828)
* fix(python): resolve calls through an unaliased dotted namespace import (#2826) `import pkg.db` followed by `pkg.db.session_scope()` emitted no CALLS edge, while all three sibling spellings resolved. In a codebase whose style guide mandates absolute imports this is close to the only cross-module call form used, so `impact()` reported `impactedCount: 0, risk: LOW, epistemic: exact` for functions with dozens of real callers — a dropped caller reading as a verified all-clear. The resolution path was never missing; one map was keyed on the wrong half of the import. `interpretPythonImport`'s plain arm splits `import pkg.db` into `localName: 'pkg'` (the name Python actually binds) and `importedName: 'pkg.db'`, and finalize carries both onto the edge as `localName` / `targetExportedName`. `collectNamespaceTargets` keyed only on `localName`, but the receiver text captured at the call site is the whole dotted path — Python's query binds the attribute's `object` field with a wildcard, so `pkg.db.session_scope()` yields the receiver `pkg.db`. Case 0 declines it (a module is not a class) and falls through, Case 1 looks up `pkg.db` and misses, and Case 1.5 needs `resolveQualifiedReceiverMember`, which only the C++ provider implements. The site drops silently. Key the map on the dotted import path as well — gated on a provider opt-in, not on the edge shape. The shape alone cannot decide it: Swift's `import Foo.Bar` produces the identical pair (`localName: 'Foo'`, `targetExportedName: 'Foo.Bar'`), but there the FIRST segment is the resolved target and `Foo.Bar` names a nested type. Minting a key for it would hand `resolveConstructionExpressionClass` an authoritative namespace — that branch deliberately does not fall through on a miss — and break `Foo.Bar(x)` construction that resolves correctly today. Hence `ScopeResolver.namespaceReceiverIncludesImportPath`, which only Python sets. The root-segment check on the added key does real work: `import pkg.db as pdb` binds only `pdb`, so writing `pkg.db.f()` there is a NameError, and its edge (localName `pdb`, path `pkg.db`) is correctly rejected. Two same-package imports stay separate — `import pkg.db` + `import pkg.cache` key `pkg.db` and `pkg.cache` independently, so neither call can land in the other's module; the shared `pkg` bucket keeps its existing ambiguity rather than gaining any. Tests: five integration rows (the issue's own repro, the three sibling spellings as controls, non-crossing two-package imports, a three-segment receiver, and dotted construction) plus a unit pin on the keying rule that asserts a Swift-shaped edge mints nothing. All five integration rows fail on the pre-fix tree; the controls pass on both, which is what makes them controls. Resolver integration suite 3024 passed / 1 skipped / 0 failed; scope-resolution unit suite 1446 passed. This changes what the resolver produces, not how it is stored — no schema or version constant applies, and an existing index needs a re-analyze to show the new edges. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(resolution): shadow-test a dotted namespace key by its root segment (#2826) `isNamespaceNameShadowed` walks the scope chain looking for a binding, type binding, lexical name, or owned def named exactly `namespaceName`. Once a namespace key can be a dotted import path, that string never matches anything: `import pkg.db` binds `pkg`, so a local `pkg = Decoy()` shadows the import, but the guard was asked about `pkg.db` and answered "not shadowed". The consequence is not a missed edge but a wrong one. The caller treats a verified namespace as authoritative and deliberately does not fall through to the workspace-wide simple-name heuristics, so an unguarded shadowed receiver resolves construction against the imported module instead of the local value. Test the first dot-separated segment instead. Single-segment names are unaffected — their root is themselves — so every pre-existing row keeps its behaviour. This ships with the key that first routes a dotted name into the guard rather than after it: the previous commit is what makes the defect reachable. The new pin fails on the pre-fix guard (verified by reverting the four comparisons and re-running: 1 failed / 5 passed), so it discriminates rather than merely passing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(python): pin the callee name on the dotted-construction row (#2826) The row asserted only that `builds` reached `pkg/db.py`. That module also exports `session_scope`, so a regression that resolved the construction to the wrong member of the right module would have kept the test green — it pinned the file, not the answer. Assert the exact edge set for the caller instead. Verified against the current tree with a scratch probe: `builds -> Model@pkg/db.py` is the only edge the file produces. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(plans): include the #2826 engineering plan in the PR `.gitignore` keeps `docs/*` local because planning output is normally throwaway. Force-added here at the reviewer's request so the plan travels with the work it drove: it records the evidence chain behind the fix, the two places the plan turned out to be wrong, and the follow-ups deliberately left out of scope. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(resolution): make the namespace shadow guard shared (#2826) `isNamespaceNameShadowed` lived module-private in `compound-receiver.ts` with a single caller. The namespace map it guards has three consumers, and the next commit adds the guard to a second one, so it moves to `scope/walkers.ts` alongside the other scope-chain primitives rather than being duplicated. Behaviour is unchanged — this is a move plus documentation. Two notes were added because both are easy to get wrong later: - Fails closed on a missing scope or a parent cycle. For every caller, suppressing costs a missing edge while trusting a corrupt scope chain costs a wrong one, so the bias is deliberate. - It reads `scope.bindings` DIRECTLY rather than through `lookupBindingsAt`, which is the opposite of the fix #2745 applied to Rust's `headBoundLocally`. There the question was "is this name bound at all?", so missing finalize's import channels lost real bindings. Here the question is "does something LOCAL shadow the import?", and the import's own finalized binding is exactly what must not count — routing this through `lookupBindingsAt` would find every namespace import shadowing itself and suppress the lot. Verified against a target module carrying a self-named def, which still resolves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(python): close the three remaining namespace-receiver gaps (#2826) Three defects the first fix left behind. All three were confirmed by probe before being touched, and a fourth suspected gap was disproved the same way. ## 1. Case 1 resolved through an import a local had shadowed `namespaceTargets` is collected per FILE, but Case 1 in `receiver-bound-calls` consulted it with no lexical guard at all, so import pkg.db def f(pkg): # parameter shadows the package return pkg.db.session_scope() emitted an edge to pkg/db.py. That is a WRONG edge, and it predates the dotted key: the single-segment spelling (`import single` + `def f(single)`) failed identically. The compound-receiver construction path has applied this guard since #2770; Case 1 simply never did. Now both use the shared guard. ## 2 + 3. The root key named the leaf module, not the package These read as two gaps and are one. `import a.b.c` binds ONE name — `a` — but makes three attribute paths callable, naming three different files: a → a/__init__.py a.b → a/b/__init__.py a.b.c → a/b/c.py The map keyed only `a`, pointed at the LEAF. So `a.helper()` resolved into a/b/c.py whenever that module happened to export `helper` — silently preferring a decoy over the real definition in the package — and `a.b.mid()` resolved to nothing at all. One wrong edge and one missing edge from a single mis-keying. Fixing it needs per-language knowledge the shared collector cannot have: which prefixes are reachable, and which file each names. The `__init__.py` convention is Python's alone, and the edge shape is ambiguous across languages — Swift's `import Foo.Bar` produces an identical `localName`/`targetExportedName` pair that means the opposite thing. So the previous commit's boolean opt-in is replaced by `ScopeResolver.namespaceReceiverPaths`, which returns every spelling with the file it names; absent or declining, the shared default (bound name → own target) is unchanged for every other language. Prefix files are proposed, not asserted — `moduleFileExists` drops any the workspace never parsed, so a PEP-420 namespace package contributes no key rather than one pointing at a missing file. ## Disproved: C# was not a fourth gap The plan listed C# `using System.Collections.Generic` + `System.Collections.Generic.List` as the same class of bug. It is not: a probe shows `My.Deep.Space.Helpers.Work()` already resolves through the FQN namespace bindings in `walkers.ts`. No change made, and the claim is withdrawn rather than carried forward as a known gap. ## Testing Integration: the shadow block asserts the exact surviving edge set (an absence-only assertion would also pass if the guard over-suppressed and killed the clean rows); the prefix block asserts all three spellings land on their own file, with `helper` defined in BOTH package and leaf so a wrong edge is visible rather than merely possible. Unit: 16 rows on the keying contract, including that a Swift-shaped edge mints nothing and an alias import keys neither the path nor the root. Resolver integration 3024 passed / 1 skipped / 0 failed; scope-resolution unit 1452 passed; tsc clean in both packages. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(python): probe both path separators when resolving a prefix package (#2826) Workspace file paths are not normalized to POSIX at ingestion — `import-target` already re-normalizes at five other comparison points, and `moduleScopeByFile` is keyed by the raw `ParsedFile.filePath`. The prefix probe built only the `/` spelling, so on Windows it would compare `a/b/__init__.py` against an `a\b\__init__.py` key, find nothing, and mint no prefix keys at all. That fails quietly, which is the worst shape for it: `a.b.mid()` simply goes back to unresolved on one platform, with no drop recorded and every test on POSIX still green. Probe both spellings and key whichever the workspace actually holds. The new row is mutation-tested — reverting to the `/`-only probe turns it red (1 failed / 10 passed), so it pins the behaviour rather than passing alongside it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(python): correct three defects a multi-lane review found in this PR (#2826) All three were introduced by this PR's own earlier commits, and none was found by re-reading the diff — each came from a lane attacking an angle the author had not. ## 1. The shadow guard ran BEFORE the map lookup it gates Case 1 evaluated `isNamespaceNameShadowed` unconditionally, then consulted `namespaceTargets`. So every call/read/write site with an explicit receiver, in every language, paid a scope-chain walk (a Set allocation, three Map lookups and a linear `ownedDefs` scan per level) ahead of an O(1) hash miss that was going to decline it anyway. The proof it was an oversight rather than a decision sits in this same PR: the sibling guard in `compound-receiver.ts` reads the map first and only guards on a hit. Two call sites of one shared function, opposite order. Semantics are identical either way — a miss yields `undefined` regardless — which is exactly why it survived several readings. ## 2. Prefix packages were anchored on the import spelling, not the resolved leaf `pythonNamespaceReceiverPaths` built `a/__init__.py` from the dotted path joined at the workspace root, never consulting the file the import actually resolved to. But `resolvePythonImportTarget` resolves off-root in two of its three tiers, so `import utils.db` can land on `libs/common/utils/db.py`. That produced a wrong edge where a same-named `utils/` package exists at the root, and produced NOTHING in a `src/` layout — the prefix feature was inert for the most common Python project shape, silently. Now the prefix directories are derived by walking back from the resolved leaf, which is exact for root, `src/` and off-root layouts alike. It also inherits the leaf's own separator, which subsumes the previous dual-separator probe: that probe was dead code anyway, because `filesystem-walker.ts` normalizes `\` to `/` before a path ever becomes a `ParsedFile.filePath`. Its test row is removed rather than left asserting an unreachable state. ## 3. Keying the root at `__init__.py` INSTEAD of the leaf lost re-exports `findExportedDef` accepts only a binding whose `origin === 'local'`. The canonical Python package re-exports from its submodules — `from .b.c import helper` in `__init__.py` — which is an IMPORT binding, so it is rejected. Keying the prefix solely at the package therefore turned `a.helper()` from a correct edge into no edge at all for the most common package shape. Every fixture in this PR defined its members locally in `__init__.py`, which is precisely the one layout where that mistake is invisible. The prefix now keys the package FIRST and the leaf behind it. A real definition in `__init__.py` still wins over a same-named decoy deeper in the package, and a name merely re-exported there still resolves through the leaf. Ordering is the contract, so the unit rows assert the exact arrays rather than membership. ## Testing New rows: off-root layout with a decoy `utils/` at the root, and a `src/` layout. Both mutation-tested — reverting to the spelling-anchored build turns them red. The re-export case was verified end-to-end with a scratch fixture whose `__init__.py` only re-exports (`uses -> helper@a/b/c.py`). Resolver integration 3131 passed / 1 skipped / 0 failed — unchanged from before these fixes, so they regress nothing. Scope-resolution unit 1459 passed. tsc clean in both packages. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(resolution): stop the namespace shadow guard AT the module scope (#2826) CI caught a regression this PR introduced: `cjs-exports-assignment.test.ts` lost both of its cross-file rows — cross-file require() member call resolves expected [] to deeply equal [ 'handle' ] an `exports` parameter does not hijack the module (UMD factory) expected [] to deeply equal [ 'publicApi' ] — i.e. `const svc = require('./svc'); svc.handle()` stopped resolving in JavaScript. Cause: in CommonJS the namespace import IS a variable declaration. One statement produces both the ImportEdge and a module-scope `const` binding, so the guard, by inspecting the module scope, found the import's own name there and read it as a shadow of itself — suppressing exactly the receivers it exists to enable. The guard's own contract sentence already said the right thing: "a declaration BETWEEN the call site and its module scope". The module scope is the floor of that walk, not a rung on it. It now returns at Module without inspecting it. Nothing is lost on the suppression side: a genuine shadow is a parameter, a local, or a nested declaration, and all of those live in scopes strictly inside the module. The Python rows that pin suppression (`def f(pkg): pkg.db.f()` and its single-segment `import single` twin) still pass, because a parameter is an inner scope. Worth recording for the next reader: two independent review lanes examined this exact scenario and both REFUTED it, reasoning that `require()` yields an ImportEdge in `scope.imports` rather than a local binding. That is true for Python's `import x` and false for CommonJS, where one statement is both. My own probe used a Python fixture and so could not surface it either. Agreement between reviewers was not evidence; the test corpus was. Verified: cjs-exports-assignment 36/36, the #2826 integration rows 7/7, scope-resolution unit 126/126. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Parafee41
|
a857f4c5a6
|
docs(taint): document per-language model files (#2809)
* docs(taint): document per-language model files * docs(taint): link language-specific model tests --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> |
||
|
Shifra Williams
|
a6a8aa788c
|
feat(serve): validate and port-scope the origin/proxy configuration surface (#2820) | ||
|
dependabot[bot]
|
f36c3eb678
|
chore(deps)(deps): bump js-yaml from 5.2.2 to 5.2.3 in /gitnexus (#2831)
Some checks are pending
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
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
Bumps [js-yaml](https://github.com/nodeca/js-yaml) from 5.2.2 to 5.2.3. - [Changelog](https://github.com/nodeca/js-yaml/blob/master/CHANGELOG.md) - [Commits](https://github.com/nodeca/js-yaml/compare/5.2.2...5.2.3) --- updated-dependencies: - dependency-name: js-yaml dependency-version: 5.2.3 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
Gergő Magyar
|
cabd5b82f9
|
fix(go): model Go method sets exactly so interface satisfaction is decidable (#2813) (#2829)
* test(go): pin calls through an interface-typed struct field (#2813) A call through an interface-typed struct field never reaches the implementation: the CALLS edge stops at the interface DECLARATION, so `impact()` on the implementing method reports 0 callers. This commit adds the executable statement of that defect; the fixes follow. Two stacked defects produce it, and either alone is enough to reproduce — which is why no existing fixture could observe it: D1 `buildDetectionIndexes` skips every POINTER-receiver method, so a struct whose methods are all `func (r *T)` has an empty method set, structurally satisfies nothing, and gets no IMPLEMENTS edge. Go's rule is that the method set of *T includes pointer-receiver methods, and idiomatic Go stores *T in an interface-typed field. D2 Case 0 (compound receiver) emits its primary edge and short-circuits without the interface-dispatch fan-out Case 4 performs. A struct field receiver `s.orderRepo` contains a dot and so always takes Case 0; a local or parameter receiver is a bare name and reaches Case 4. Every implementor in both pre-existing structural-dispatch fixtures uses a VALUE receiver, and the one pointer-receiver type is pinned as a negative (`not.toContain('PointerOnlyThing -> PointerOnly')`), so the corpus could not see D1 by construction. The new fixture is pointer-receiver throughout, cross-package, and carries concrete-field controls in the same structs. Failing-first, verified against this tree: 7 of the 11 new assertions fail and 4 pass. The 4 that pass are exactly the controls that must not regress — the primary edge to the interface declaration, the concrete-field call, the absence of fan-out on a concrete field, and the partial-signature negative — so the suite discriminates rather than merely failing. Two recorded artifacts move here because the FIXTURE was added, not because capture output changed: - test/fixtures/go-captures-golden/expected-captures.json — regenerated additively (32 insertions, 0 deletions). - bench/scope-capture/baselines.json — go fingerprint, fixture_count 102 -> 110. Both are regenerated in this commit rather than deferred to the end of the series: the fixture is their only cause, no later commit touches capture emission, so they cannot re-drift and every commit stays green. The check that this is corpus growth and not a capture regression is that go was the only one of 15 language fingerprints to move on the same run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(go): count pointer-receiver methods toward structural interface satisfaction (#2813) D1 of two stacked defects. `buildDetectionIndexes` skipped every method whose receiver is a pointer, so a struct declaring `func (r *OrderRepo) DeleteItem(...)` had an EMPTY method set, structurally satisfied nothing, and produced no IMPLEMENTS edge at all. Go's method-set rule is per-type, and there are two types involved: the method set of `T` holds only value-receiver methods, while the method set of `*T` holds both. #1966 implemented the `T` reading, which is exactly right for `T` — and leaves `*T` permanently empty. GitNexus models one Struct node per type with no separate `*T` node, so only one of the two can be represented, and the `T` reading is the one idiomatic Go almost never uses: methods take pointer receivers so they can mutate, and `*T` is what gets stored in an interface-typed field. The cost was silence rather than caution. With no IMPLEMENTS edge, a call through an interface-typed field resolved to the interface DECLARATION and `impact()` on the implementing method returned 0 callers — byte-identical to a symbol that genuinely has none, which is what made the reporter's blast-radius check unusable rather than merely incomplete. This picks the `*T` reading: the graph now answers "which types provide this interface's behaviour", and no longer proves `var x I = T{}` invalid. The trade is deliberate and was checked against every consumer of IMPLEMENTS before being made — MRO/METHOD_IMPLEMENTS derivation, community clustering, the receiver-dispatch fan-out index, and the epistemic heritage probe. None performs value-assignability checking. Two negative pins encoded the #1966 decision and are REVERSED here rather than deleted, each keeping a comment that explains why the polarity moved: - go.test.ts: `PointerOnlyThing -> PointerOnly` now expected to be emitted. - go-hooks.test.ts: the pointer-receiver-only unit case now expects the implementor instead of `undefined`. `goReceiverKind` is still stamped in method-owners.ts — it is the hook a future value/pointer-aware model would read — but is deliberately no longer a filter. Its now-dead local predicate and type alias are removed so the file no longer carries a helper asserting the reverted rule. Measured on the #2813 fixture, this commit alone: the two IMPLEMENTS assertions flip to passing (6 pass, up from 4) while the five interface-dispatch fan-out assertions still fail — those are D2, fixed in the next commit. Keeping the two commits separate is what makes that attribution visible. Go unit suite: 91 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(resolution): fan out interface dispatch from a compound receiver (#2813) D2 of two stacked defects, and the one that closes the issue. Case 0 (compound receiver) emitted its primary edge and short-circuited without the interface-dispatch fan-out that Case 4 performs, so a call whose receiver is a struct FIELD stopped at the interface's method DECLARATION and never reached any implementation. The gap was a property of receiver SYNTAX rather than of types. Case 0 is selected by `receiverName.includes('.')`, so a field receiver (`s.orderRepo`) always lands there, while the very same interface reached through a local or a parameter is a bare name and falls through to Case 4 — which fans out correctly. Field-held interfaces, i.e. dependency injection, were the half that silently lost every implementation edge; the pre-existing fixtures exercise the local and parameter forms only, which is why the suite was green. The fix is the call Case 4 already makes, placed after Case 0's primary `tryEmitEdge` and before its `handledSites.add`. It stays language-agnostic (AGENTS.md section 42): `emitInterfaceDispatchFor` self-gates on `ownerDef.type !== 'Interface'`, so a receiver that folds to a Struct emits nothing extra and no language check is needed. Confidence is Case 0's own 0.85 literal, not Case 4's site.kind-dependent value — Case 0 has no read/write arm to mirror. The case ladder itself is untouched: invariant I4 in contract/scope-resolver.ts makes the ordering a contract, so the fan-out is added INSIDE Case 0 rather than by reordering or merging cases. Also flips a second, previously unnoticed encoding of the #1966 value-only reading that the full sweep surfaced: the exact-set assertion at go.test.ts:361 enumerates every structural IMPLEMENTS edge, and D1 correctly adds `PointerOnlyThing -> PointerOnly` to it. It is D1 fallout rather than D2's, but D1 had already landed; recording it here with its reason beats amending a commit whose separate measurability is the point. Measured: - #2813 suite: 11 of 11 pass (was 7 failing after D1 alone, which fixed only the two IMPLEMENTS rows). - go.test.ts: 160 passed. - Full cross-language sweep, test/integration/resolvers: 3027 passed, 1 skipped, across 52 files. The single failure in that run was the exact-set assertion above, fixed here; no other language regressed. `detect_changes` rates this HIGH (6 affected flows, all EmitReceiverBoundCalls at step 1) — inherent to editing a hub symbol in the resolution pipeline. The sweep above is the empirical answer to that label. An existing index must be re-analyzed to show the new edges; this changes what the resolver produces, not how it is stored, so no SCHEMA_BUMP applies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(go): pin the heritage edges that make impact() hedge an interface-bound count (#2813) The epistemic half of the issue, resolved by MEASUREMENT rather than by new code, and pinned at its mechanism. The reporter's disqualifying complaint was that `impact()` reported `impactedCount 0, epistemic "exact", risk LOW` for a method reachable only through an interface-typed field — byte-identical to what it reports for a symbol that genuinely has no callers. A zero therefore could not be used defensively, which was the entire use case. That verdict comes from `computeEpistemicBoundary`, which has two producers and neither fired: the call sites were not DROPPED (they resolved, just to the interface declaration, so the #2744 receiver-typing producer saw nothing), and its heritage probe walks IMPLEMENTS/METHOD_IMPLEMENTS edges out of the queried symbol — of which there were none, because the pointer-receiver exclusion (D1) meant no such edge was ever emitted. Restoring those edges fixes the epistemics as a side effect, so the planned conditional change to local-backend.ts is NOT needed. Measured on this fixture against the fixed tree: impact(OrderRepo.DeleteItem, upstream) before: impactedCount 0, epistemic "exact" after: impactedCount 3, epistemic "lower-bound", with an interface boundary note; the three callers are OrderHandlers.Delete, PickService.StartSession and WaveService.Release — all correct. impact(CartRepo.Get, upstream) [concrete receiver, no interface] after: impactedCount 1, epistemic "exact" The second row is the one that matters for trust: the hedge discriminates instead of firing on everything, so "exact" still means exact. This test asserts the METHOD_IMPLEMENTS edges the probe walks. Pinning the mechanism keeps the resolver suite from reaching into the MCP layer while still failing loudly if the edges regress; the impact() numbers above are recorded in the commit message and PR body rather than re-asserted here. #2813 suite: 12 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(go): model Go method sets exactly so interface satisfaction is decidable (#2813) Replaces the approximate structural-interface model with the rules the Go spec actually defines, so the graph answers what the compiler answers instead of a useful-but-wrong summary of it. Three answers were provably wrong before; all three are now exact and covered. Method sets (go.dev/ref/spec#Method_sets): MS(T) = methods declared with receiver T MS(*T) = methods declared with receiver *T OR T Promotion (#Struct_types): S embeds T -> MS(S) and MS(*S) get promoted methods with receiver T; MS(*S) ALSO gets those with receiver *T S embeds *T -> MS(S) AND MS(*S) both get receiver T or *T Identifier identity (#Uniqueness_of_identifiers): "Two identifiers are different if they are spelled differently, OR IF THEY APPEAR IN DIFFERENT PACKAGES AND ARE NOT EXPORTED." func (b *Base) Ping() // pointer receiver type ByValue struct{ Base } type ByPointer struct{ *Base } type before exact answer Base IMPLEMENTS only *Base implements ByValue IMPLEMENTS only *ByValue implements ByPointer IMPLEMENTS the VALUE type implements All three were the same edge. Two of the three were wrong, and nothing in the graph could tell them apart. Worse, in a different direction: package sealed; type Sealed interface { seal() } package foreign; func (t *T) seal() {} `foreign.T` cannot implement `sealed.Sealed` in Go — `seal` is unexported, so the two identifiers are DIFFERENT. Matching on the bare name emitted a FALSE IMPLEMENTS edge, and the interface-dispatch fan-out then turned it into an impossible CALLS edge. That is the entire basis of the sealed-interface idiom. - `methodSetKey` qualifies UNEXPORTED method names with their declaring package, leaving exported names unqualified (which is what makes cross-package satisfaction work at all). Exactness, not a heuristic: the sealed case now emits no edge, while the legitimate same-package implementor is retained. - `collectStructMethodEntries` builds MS(T) and MS(*T) together and applies the promotion table above. The embed FORM is load-bearing, so it is now captured: `@reference.embedded-pointer` records `*T` versus `T`, which the parser previously discarded (the `*` is an unnamed token). - Detection returns `{ structDefId, receiverForm }`. `receiverForm: 'pointer'` means the value type does NOT implement and only `*T` does — the fact `var x I = T{}` turns on. - The form rides in the edge `reason` (`-structural-implements-pointer`). Relationships carry no arbitrary properties, so a new field would change the relation DDL, move SCHEMA_FINGERPRINT and force a rebuild for a fact a string already expresses. Value-form implementors keep the ORIGINAL unsuffixed reason, so a consumer matching the old string now sees exactly the assignable set — which is what that string always claimed to mean. - `emitInterfaceDispatchFor` walks the SUBTYPE CLOSURE (IMPLEMENTS + EXTENDS) and skips bodiless declarations, instead of stopping at depth 1. Two reproduced Java shapes emitted an edge to a second abstract declaration while the only class with a body got nothing: a sub-interface that re-declares the method, and an abstract base between interface and implementation. Both now reach the implementation and neither emits the declaration edge. - The fan-out is bounded by `MAX_INTERFACE_DISPATCH_FANOUT` (32, `GITNEXUS_MAX_INTERFACE_DISPATCH_FANOUT`) and reports what it dropped, mirroring `MAX_PROPERTY_DISPATCH_FANOUT`. A bare cap would silently discard valid dispatch targets, which is the same false-safe silence this issue is about. - Corrects a rationale comment that was factually wrong about the code 70 lines above it (Case 0 DOES branch on `site.kind`, at :713-716; what it lacks is a read/write branch in its reason/confidence computation). - Updates both copies of the case-ladder contract, which still described the fan-out as Case-4-exclusive. The embed-pointer marker is PARSE-TIME capture emission, so a warm cache would replay the pre-marker capture set and the distinction would never appear — silently, the v27/v30 failure mode. 43 and not 40 because origin/main allocated 40, 41 and 42 while this branch was in review, which is exactly the window this file's history records both prior EXACT clashes landing in. Pin moved with it. RE-CHECK AGAINST origin/main IMMEDIATELY BEFORE MERGE. - Go unit: 93 passed, including new rows pinning that `populateGoOwners` stamps `goReceiverKind` (previously the field had no reader and could rot silently) and that a pointer-receiver-only type implements in POINTER form only. - Cross-language sweep, test/integration/resolvers: 3034 passed, 1 skipped, 52 files, zero regressions. - scope-capture bench: PASS (15 languages). Go is the ONLY fingerprint that moved, which is the check that this is a Go capture change and not a cross-language regression; rebaselined with rationale. - Also closes review gaps in this PR's own tests: the concrete-field control was vacuous with respect to the type gate (repointed at a struct that IS an implementor), the two-service-file row could not distinguish the two files it is named for (both ends now file-qualified), plus new rows for signature mismatch, emitted confidence, and an exact N-by-M fan-out bound. An existing index must be re-analyzed; the schema bump forces it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Gergő Magyar
|
c1103f38f2
|
fix: type an inference-typed class field so it can act as a call receiver (#2807) (#2810)
* test(helpers): add the shared temp-repo lifecycle helper `createTempDirPool` gives a suite one owner for its temp fixture repos — create on demand, remove them all in one `afterAll` — instead of a hand-rolled mkdtemp/rmSync pair per file. The PDG receiver pin added in the next commit uses it. Cherry-picked verbatim from |
||
|
dependabot[bot]
|
b2cd1c2ad6
|
chore(deps)(deps): bump @hono/node-server in /gitnexus (#2827)
Bumps [@hono/node-server](https://github.com/honojs/node-server) from 1.19.14 to 2.1.0. - [Release notes](https://github.com/honojs/node-server/releases) - [Commits](https://github.com/honojs/node-server/compare/v1.19.14...v2.1.0) --- updated-dependencies: - dependency-name: "@hono/node-server" dependency-version: 2.1.0 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
dependabot[bot]
|
6ae35f1e71
|
chore(deps): bump aiohttp in /eval in the uv group across 1 directory (#2825)
--- updated-dependencies: - dependency-name: aiohttp dependency-version: 3.14.3 dependency-type: indirect dependency-group: uv ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> |
||
|
dependabot[bot]
|
f3b4806389
|
chore(deps)(deps): bump fast-uri from 3.1.4 to 3.1.5 in /gitnexus (#2821)
Bumps [fast-uri](https://github.com/fastify/fast-uri) from 3.1.4 to 3.1.5. - [Release notes](https://github.com/fastify/fast-uri/releases) - [Commits](https://github.com/fastify/fast-uri/compare/v3.1.4...v3.1.5) --- updated-dependencies: - dependency-name: fast-uri dependency-version: 3.1.5 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> |
||
|
dependabot[bot]
|
e7503b6ea5
|
chore(deps)(deps): bump @modelcontextprotocol/sdk in /gitnexus (#2815)
Bumps [@modelcontextprotocol/sdk](https://github.com/modelcontextprotocol/typescript-sdk) from 1.29.0 to 1.30.0. - [Release notes](https://github.com/modelcontextprotocol/typescript-sdk/releases) - [Commits](https://github.com/modelcontextprotocol/typescript-sdk/compare/v1.29.0...1.30.0) --- updated-dependencies: - dependency-name: "@modelcontextprotocol/sdk" dependency-version: 1.30.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> |
||
|
dependabot[bot]
|
62d07cca5d
|
chore(deps)(deps): bump the npm_and_yarn group across 1 directory with 2 updates (#2823)
Bumps the npm_and_yarn group with 2 updates in the /gitnexus-web directory: [fast-uri](https://github.com/fastify/fast-uri) and [postcss](https://github.com/postcss/postcss). Updates `fast-uri` from 3.1.4 to 3.1.5 - [Release notes](https://github.com/fastify/fast-uri/releases) - [Commits](https://github.com/fastify/fast-uri/compare/v3.1.4...v3.1.5) Updates `postcss` from 8.5.22 to 8.5.25 - [Release notes](https://github.com/postcss/postcss/releases) - [Changelog](https://github.com/postcss/postcss/blob/main/CHANGELOG.md) - [Commits](https://github.com/postcss/postcss/compare/8.5.22...8.5.25) --- updated-dependencies: - dependency-name: fast-uri dependency-version: 3.1.5 dependency-type: indirect dependency-group: npm_and_yarn - dependency-name: postcss dependency-version: 8.5.25 dependency-type: indirect dependency-group: npm_and_yarn ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
dependabot[bot]
|
4a2f4c8ddd
|
chore(deps)(deps): bump the npm_and_yarn group across 1 directory with 1 update (#2817) | ||
|
dependabot[bot]
|
4c0a78fcfb
|
chore(deps)(deps): bump hono from 4.12.31 to 4.13.0 in /gitnexus (#2822)
Bumps [hono](https://github.com/honojs/hono) from 4.12.31 to 4.13.0. - [Release notes](https://github.com/honojs/hono/releases) - [Commits](https://github.com/honojs/hono/compare/v4.12.31...v4.13.0) --- updated-dependencies: - dependency-name: hono dependency-version: 4.13.0 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
dependabot[bot]
|
38be4c6bd2
|
chore(deps)(deps): bump node-addon-api from 8.9.0 to 8.9.1 in /gitnexus (#2816) | ||
|
dependabot[bot]
|
ca294e8cdb
|
chore(deps)(deps): bump ip-address from 10.2.0 to 10.4.0 in /gitnexus (#2818) | ||
|
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
|
7468cc915b
|
fix(analyze): replace the hand-incremented schema version with a derived DDL fingerprint (#2798) (#2808)
* feat(schema): derive a fingerprint from the DDL this build creates `SCHEMA_FINGERPRINT` is a sha256 digest of the node and relation DDL that `runSchemaCreationQueries` actually executes, in the same shape as the existing `taintModelVersion` stamp (hex, sliced to 12). It exists because `INCREMENTAL_SCHEMA_VERSION` is hand-picked and has to *predict* whether an on-disk database matches this build's DDL. That number has collided with `main` eight times, twice exactly — and an exact clash is the quiet one, because the reuse gate is a strict `===`. `EMBEDDING_SCHEMA` is deliberately excluded: its `FLOAT[N]` width comes from `GITNEXUS_EMBEDDING_DIMS` at module load, so folding it in would make the digest a function of the environment rather than of code, and two runs of the same build under different env would thrash full rebuilds. Refs #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(storage): record the DDL fingerprint in RepoMeta `RepoMeta.schemaFingerprint` stores the digest of the DDL an index's tables were actually created from. It is the derived companion to `schemaVersion`, not its replacement: both are compared, and both must match. Absent means mismatch, deliberately. Grandfathering a missing fingerprint would let an incremental top-up stamp a fresh one onto a database whose DDL was never verified, permanently certifying exactly the wrong-shaped index the field exists to catch. The cost is one full rebuild per pre-existing index. The version ladder gains a note that its "re-check against origin/main before merge" ritual now only guards *semantic* bumps. v25, v26, v30, v31 and v34 all changed emitted ids, edges or wire formats while leaving the DDL byte-identical, and the fingerprint cannot see any of them — but DDL collisions no longer need renumbering. Refs #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(analyze): gate index reuse on the DDL fingerprint, not just the version (#2798) `INCREMENTAL_SCHEMA_VERSION` is a hand-incremented integer that has to predict a derived fact: whether the on-disk DDL matches the code's DDL. It has collided with `main` eight times, and twice the collision was *exact*. An exact clash is the silent one. Two builds stamp the same number over different DDL, the `===` reuse gate reads the index as current, every `CREATE ... TABLE` is then skipped as "already exists" (suppressed in `runSchemaCreationQueries`), and the edges whose endpoint pair the live database cannot hold are dropped by `fallbackRelationshipInserts`' bare `catch`. The result is a wrong graph, with no error anywhere. Reuse now requires the version AND the DDL fingerprint to match, in both the pre-pipeline force-rebuild guard and the `isIncremental` predicate, and the fingerprint is stamped alongside the version at the end of a run. Both conditions are necessary. The fingerprint does not replace the integer: most entries in the version ladder change emitted ids, edges or wire formats while the DDL stays byte-identical, and a fingerprint-only gate would stop forcing rebuilds for all of them. What it does buy is that two branches picking the same number no longer need renumbering. The new branch sits above the `alreadyUpToDate` fast path for the same reason the version guard does — a clean tree at an unchanged commit would otherwise early-return before either check ran. Closes #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(analyze): pin the DDL fingerprint gate and its two failure cases `schema-fingerprint.test.ts` pins the properties the gate rests on: the digest covers exactly the node and relation DDL that gets executed (recomputed from the exported lists, so adding a table or a FROM/TO pair without the fingerprint moving is impossible), it excludes the environment-derived embedding DDL, and it moves when any covered string moves. The two `incremental-orchestration` cases exercise the production path rather than modelling it: an index carrying the *current* version with a foreign fingerprint, and one with no fingerprint at all. Both were run against the pre-fix tree first and both failed there with `alreadyUpToDate === true` — the fast path swallowing the mismatch, which is the #2798 symptom exactly. `call-summary-schema-version.test.ts` widens its gate model to two equalities. The second argument defaults to the current fingerprint so all 33 existing version cases read unchanged, and a new case covers the collision, the legacy absence, and the semantic bump the fingerprint cannot see. Refs #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(review-skill): point the schema-constant check at the fingerprint, not the deleted integer All four `gitnexus-review` SKILL.md mirrors told reviewers to verify `INCREMENTAL_SCHEMA_VERSION` "was bumped or regenerated". That constant no longer exists, so the instruction sent every future reviewer looking for something they could not find — and, worse, past its replacement. The check for graph DDL is now derived: `SCHEMA_FINGERPRINT` moves on its own, so the question is whether the diff changed a string in `NODE_SCHEMA_QUERIES` / `REL_SCHEMA_QUERIES`, and whether a newly added DDL array was folded into the fingerprint at all — the one way the derived gate can still be bypassed. What did NOT change is called out explicitly: the parse-store `SCHEMA_BUMP` and the bench fingerprint sets are still hand-maintained and still need the re-check-against-base ritual, and semantic changes that leave the DDL untouched fall outside the fingerprint entirely — those rely on the analyzer runner-identity receipt. Found by the review swarm's docs lane. The original plan for #2798 claimed no documentation mentioned the constant; that sweep covered five root docs and never looked at `.claude/skills/**` or the three mirrors. Refs #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(migration): record the one-time rebuild the fingerprint switch costs Replacing `schemaVersion` with `schemaFingerprint` means every index written by an earlier GitNexus carries no fingerprint, reads as a mismatch, and is rebuilt once. That is deliberate — grandfathering absence would stamp a fresh fingerprint onto a database whose DDL was never verified — but until now it was undocumented, so a user's first post-upgrade analyze would announce a full re-analyze with nothing to explain it. MIGRATION.md already sets the precedent: PR #2363's meta.json → gitnexus.json rename was equally automatic and equally in need of an entry. This follows that shape, and is explicit about the parts that are easy to undersell: - the cost is per INDEX, and branch-scoped slots (#2106) each pay separately; on a large repository a full re-analyze is substantial, not a blip; - rollback is safe — an older binary sees no `schemaVersion` and forces its own rebuild, which is a cost, never a stale graph; - alternating between an old and a new binary rebuilds on every switch, because the end-of-run meta is written as a fresh literal so neither field survives the other's run. The retired ladder's per-version rationale is pointed at in git history rather than reproduced: `git show 561f913a3:.../repo-manager.ts`. That commit is an ancestor of origin/main, so the pointer survives this branch being squash-merged. Refs #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(identity): cover workspace-linked packages in the analyzer dependency digest `dependencyNames` enumerated `dependencies`, `optionalDependencies` and `peerDependencies` only. `gitnexus-shared` is declared as a devDependency (`file:../gitnexus-shared`), and in a source-mode run the build root is the gitnexus package tree, which does not contain that sibling. So a change to gitnexus-shared moved neither `build.digest` nor `dependencyRuntime.digest`. That gap matters more since #2798 deleted `INCREMENTAL_SCHEMA_VERSION`. A DDL-affecting edit there is still caught by `SCHEMA_FINGERPRINT`, but a SEMANTIC-only edit — a new `REL_TYPES` member, say, where the relation table carries a bare `type STRING` column so no CREATE statement moves — was covered by nothing at all. Roughly thirty of the retired ladder's entries were exactly that change class, and the runner-identity receipt is what now carries them. Only checkout-local specifiers are added: `file:`, `link:`, `workspace:`, `portal:` and npm's bare local-path shorthands. Pulling in every devDependency was rejected — vitest, eslint and typescript would enter the digest and force a full re-analyze on unrelated tool bumps, which is worse than the hole. Scanning the linked sibling for the first time exposed a latent throw: `collectArtifacts` honoured `PRUNED_RUNTIME_DIRECTORIES` only for a real directory, so a SYMLINKED `node_modules` fell through to the payload branch and died with "Analyzer identity input is not a file". Worktree-style dev layouts and pnpm shared stores hit this immediately — verified in this worktree, where `gitnexus-shared/node_modules` is such a symlink. Pruning it loses nothing: packages beneath are still reached through `resolveDependencyPackageRoot`. Verified: a real `analyze` in this worktree succeeds with `packageCount` 259; editing the linked package's source moves the digest, bumping an installed registry devDependency does not, and removing the link moves it. `DEPENDENCY_RUNTIME_CANONICALIZATION` is deliberately not bumped — freshness compares digests, not the label, and the input-set change already moves them. Follow-up worth having: no fixture in the suite declares `devDependencies`, so this has no regression test yet. Refs #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(analyze)!: delete INCREMENTAL_SCHEMA_VERSION, gate reuse on the DDL fingerprint alone The integer and its ~180-line version ladder are gone, along with `RepoMeta.schemaVersion`. Index reuse is now decided solely by `SCHEMA_FINGERPRINT`; a mismatch — including the absent stamp every pre-existing index carries — warns and forces a full re-analyze, which wipes and recreates the database so the tables are built from the current DDL. Deleting the integer is safe because it was already redundant: the runner-identity guard deep-compares the whole schema-v4 receipt, including a digest over the build tree, and forces a rebuild on ANY analyzer delta. Verified empirically — a comment-only edit to logger.ts, with the fingerprint byte identical, produced "runner identity changed ... forcing a full rebuild". The fingerprint is not thereby redundant. It fires where that guard cannot: a DDL-affecting change in `gitnexus-shared`, which is a workspace-linked devDependency and so sat outside both digests until the companion commit closed that gap. Review findings folded in, each correcting a line this rewrite itself introduced and never published: - B1: two assertions matched a log string the rewrite had renamed; both tests failed. They now assert what production emits. - B2: the pre-existing downgrade test perturbed `schemaVersion: 7`, a field this change deletes, so the spread carried a valid fingerprint, every guard passed, and the run legitimately took the fast path. It perturbs the fingerprint now, restoring the only integration coverage of the gate-above-the-fast-path ordering invariant. - N5: duplicate `schemaFingerprint` keys silently collapsed two assertions into one (TS1117). - N6: the absent-stamp message told non-git repositories their index was "built by an older GitNexus version" — on every run, about an index this exact build had just written. Non-git repos never record a fingerprint, and now the message says so. - N9: the on-disk stamp is shape-checked before being echoed, so a crafted gitnexus.json cannot push ANSI escapes through the CLI log. - N7: a test case that re-computed the same digest expression with its operands swapped, mislabelled as a randomness check on a module-level const. - N10: comments claiming the digest "cannot collide" (it is 48 bits), pointing at a vector-column gate that does not exist, and asserting storage/ is free of a core/ dependency two lines below a core/ value import. None of these were caught by `tsc -p tsconfig.json`, which covers src only, nor by eslint, where no-dupe-keys is off. `tsconfig.test.json` reports all three test defects and is not currently wired into CI. Refs #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(schema): pin that the fingerprint covers every DDL statement init executes `SCHEMA_QUERIES` is the list `runSchemaCreationQueries` iterates — the DDL that actually runs. The fingerprint hashes only two of its three members, and until now no test imported `SCHEMA_QUERIES` at all, so nothing tied the two together. A fourth member appended to that array — the one literally named for what init executes — would have been invisible to the gate. Every existing test would still pass, because they all recompute the digest from the same two arrays the fingerprint already uses. An index whose gate passed would then run `initLbug` over the old database, where `runSchemaCreationQueries` suppresses "already exists", so the new table would never be created and its edges would be dropped by `fallbackRelationshipInserts`' bare catch. A wrong graph, no error — exactly the failure #2798 exists to end. The check is a pure predicate over (executed, fingerprinted, documented exclusions) rather than a positional `toEqual`, so `EMBEDDING_SCHEMA` is named as an exclusion with its reason — its FLOAT[N] width is environment-derived — rather than sitting in a list where a future reader might "fix" it by folding it in. It asserts both directions and is order-insensitive, leaving ordering to the digest assertion that already pins it. The negative case is pinned in CI rather than checked by hand once: the same predicate over a synthetic fourth member must report it. If a refactor ever makes the predicate vacuous, that case fails even though the positive one would not. Refs #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(analyze): name the invariant the version deletion now rests on Deleting `INCREMENTAL_SCHEMA_VERSION` moved a load-bearing guarantee into an implicit one. Roughly thirty of the retired ladder's entries changed no DDL at all — node ids, wire formats, resolution tiers — and the fingerprint is structurally incapable of firing on any of them. Their only remaining cover is the analyzer runner-identity receipt, and nothing in the suite said so. This adds a table over the real `analyzerRunnerIdentitiesEqual` with a well-formed schema-v4 receipt: byte-identical reuses; an entrypoint-only difference reuses (CLI vs analyze worker); a moved build digest with unchanged DDL forces — that case IS the invariant, commented as such; and a dependency change, an ABI change, undefined, null, a schema-v3 legacy receipt, a missing build section and a non-sha256 digest all fail closed. The deleted `expect(INCREMENTAL_SCHEMA_VERSION).toBe(35)` pin is also worth naming: it failed CI on every bump by design, which is what made an author stop and think. Nothing replaced it. This does not restore that — a digest has no literal to pin — but it does make the mechanism that took over the job visible to the next person who reads the file. Refs #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(spring): pin CLASS_SCHEMA's membership in the fingerprinted DDL set When `INCREMENTAL_SCHEMA_VERSION` went away, its sibling in basicblock-callee-ids-schema.test.ts got a replacement assertion tying BASICBLOCK_SCHEMA to the fingerprint's input set. This file's `>= 23` floor was deleted with nothing put in its place. The file still asserts CLASS_SCHEMA's CONTENT — that the `frameworkAnnotations` column exists — but not that CLASS_SCHEMA is part of what the digest covers, and the second is what makes an index built before that column carry a different fingerprint and get rebuilt. Mirrors the sibling so the two read the same way. Refs #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(identity): stop a symlinked directory from aborting the whole analyze `collectArtifacts` fused two orthogonal facts into one condition: that four directory names never carry runtime payload, and that a symlink where a real directory was assumed falls through to the payload branch, where `snapshotReadableFile` stats the target, sees a directory, and throws "Analyzer identity input is not a file". The second was only fixed for those four names. Every other symlinked directory in a scanned package root still aborted the run — `dist -> build`, a vendored grammar link, anything inside a linked sibling checkout. Newly reachable, because making workspace-linked packages scannable pointed the scanner at a live checkout instead of an immutable registry tarball for the first time. Split along the actual seam: prune on the NAME alone, and give symlinks their own branch in the type dispatch, ahead of the payload branch. Link text is recorded rather than followed. Following was rejected on three grounds, each checked in source: the traversal is a stack with no visited set, so a self-referential link would recurse to `runtimeDepth` — which throws, trading one hard abort for another; `snapshotDirectory` rejects a symlink outright, so the directory guard could not accept one without a realpath rewrite of its canonical-path identity; and a link into an already-scanned tree double-counts against `runtimeEntries`/`runtimeBytes`, which also throw. The cost is stated in code: a link out of the package contributes its text, not its target's content. Links resolving to a regular file keep the existing content digest. The new `'unfollowed-symlink'` kind is threaded through every consumer, including the cache validator — which re-probes with `mode: 'link'`, since the readable-file probe resolves the target and would return null for exactly this kind, silently failing every warm validation. No canonicalization or cache-schema bump. Digest content changes only for trees that previously crashed: a delta scan over all 258 scanned roots of this install found no regular file bearing a pruned name and no symlink failing to resolve to a file, so `dependencyRuntime.digest` is byte-identical here. Six of the eight new tests fail against the unfixed tree with the exact production error; all eight pass after. Refs #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(analyze): give the reuse gate a real seam and sanitize logs at the funnel Cleanup pass over the #2798 branch. Net -183 lines. The gate had no extracted predicate, so its own test asserted it by regex-matching run-analyze.ts SOURCE TEXT. That pinned production formatting: one pattern froze three back-to-back single-name imports from './lbug/schema.js', so merging them — the obvious tidy-up — failed a test named "still imports the DDL digest itself". `schemaFingerprintMismatch` and `isSchemaFingerprintShaped` now live in core/lbug/schema.ts beside the constant. Not in run-analyze.ts next to `pdgModeMismatch`, because storage/ must stay off the analyze pipeline and mcp/resources.ts is a plausible second consumer — the same reasoning that puts `cjkSegmentationModeMismatch` in core/search/. The regex block is gone; the test calls the predicate. The three imports are merged. ANSI sanitation moved from one field to the funnel. The per-field guard's own comment stated the general hazard — gitnexus.json is parsed with no runtime shape validation and the notice reaches console.log — while two sibling guards twelve lines away echoed `runnerIdentity.schemaVersion` and `cjkSegmentation` from that same file raw into the same log. `log()` now strips C0/C1 controls, covering all seven guard messages and any written later. Also: - Deleted a duplicate integration test. After the downgrade test was repointed at `schemaFingerprint` it became the same scenario as the new one, differing only by an extra log assertion — which is now folded into the survivor. Saves a fixture and two full pipeline runs per CI pass. - Replaced a 3-parameter set-difference helper with one set equality. Its doc was false at one call site (arguments semantically swapped) and it needed a fourth test purely to prove itself non-vacuous; set equality cannot go vacuous. - Removed ~115 lines of runner-identity table that duplicated analyzer-identity.test.ts. The three genuinely uncovered cases moved there, and the #2798 invariant — build digest moved while the DDL did not — now asserts against a REAL analyzer-build-tree edit rather than a hand-built literal, which is strictly stronger than what it replaces. - MIGRATION.md quoted a log line the code cannot emit; it was written before the placeholder changed. - Restored the rationale on the `capabilities` docstring, which a previous pass replaced with its consequence — leaving a maintainer reading "duplicated by hand" as a wart to fix by importing, which is what the original forbade. - Marked the `isIncremental` conjunct as belt-and-braces: `!options.force` short-circuits before it, so it cannot decide anything. Refs #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(analyze): force a rebuild when the vector column width changes `CodeEmbedding.embedding` is declared `FLOAT[EMBEDDING_DIMS]`, resolved from `GITNEXUS_EMBEDDING_DIMS` at module load. Nothing gated it. Flip the variable on a same-commit clean tree and no guard fired at all: `alreadyUpToDate` returned over a `FLOAT[384]` table while the process embedded at 768. The only reaction anywhere discards the embedding CACHE and re-embeds — into a column whose type it never revisits. This predates #2798; `INCREMENTAL_SCHEMA_VERSION` never covered dims either. It surfaced because the fingerprint work had to reason about why `EMBEDDING_SCHEMA` must stay OUT of the digest: its width is environment-derived, so folding it in would make the same build disagree with itself and thrash rebuilds. That exclusion is correct, and it leaves the width needing its own guard. Modelled on `cjkSegmentation`, the closest sibling: an env-resolved scalar stamped at write time and compared by a small exported predicate that forces on mismatch. `embeddingDimsMismatch` sits in core/lbug/schema.ts beside `EMBEDDING_DIMS`, so the query side can adopt it without importing the analyze pipeline — mcp/local/local-backend.ts already warns on a cjkSegmentation disagreement and has the identical claim here, since the query path embeds at the live width against a table of unknown width with no validation at all today. ABSENCE IS NOT A MISMATCH, deliberately. Forcing on it would be dead code: `embeddingDims` and `schemaFingerprint` ship together, and a missing fingerprint already forces exactly one rebuild — which is where this stamp lands. Absence also carries no signal here, unlike the fingerprint: a missing fingerprint means "DDL this build cannot vouch for" and ships WITH a DDL change, whereas a missing dims stamp means only "written before the field existed", and that run's table agreed with that run's width. Drift requires the env to change, which absence says nothing about. The `cjkSegmentation` trick of folding absence into the default was unavailable — there is no width that is safe to assume for an existing table — so the stamp is instead written unconditionally, giving absence exactly one meaning. Malformed values are not grandfathered: null, '384', NaN and objects all read as a mismatch and err toward a rebuild. Refs #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(mcp): warn when the served index's vector width differs from the query embedder's The analyze side now forces a rebuild when the vector column width changes. The query side had no equivalent: a serving process embeds a query at its own width and searches a table whose width was fixed when the index was built. Disagree and the user gets wrong or missing semantic results with nothing explaining why. Mirrors the cjkSegmentation drift warning immediately above it — same warnings[] array, same per-query recomputation, agent-visible in the tool response, and it warns rather than refuses. A width mismatch degrades the semantic lane only; keyword results are unaffected, so `partial` is deliberately not set. Compares against `getEmbeddingDims()` — the width the query embedder actually produces — NOT schema.ts's `EMBEDDING_DIMS`. The two diverge exactly when GITNEXUS_EMBEDDING_DIMS is set on a server that embeds LOCALLY: the query path ignores that variable and embeds at 384, so comparing against the env-derived constant would report drift on a lane that works fine. The recorded width is what the vector CAST actually binds. `embeddingDimsMismatch` is imported from core/lbug/schema.js rather than restated, so "absent is not a mismatch" cannot drift between the analyze and query sides. That predicate was placed in schema.ts precisely so this consumer could reach it without importing the analyze pipeline. Two gates keep it quiet when it would be noise: it fires only for a repo where this process actually produced a query vector, so an index analyzed without --embeddings (or a server whose embedder is unavailable) never carries it. An untrusted recorded value — meta.json is schema-less JSON — is reported as "an unrecognized width" rather than echoed. `loadMeta` is hoisted out of the neighbouring try so both diagnostics share one read and an invalid GITNEXUS_FTS_CJK_SEGMENTATION cannot take this one down with it. Refs #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(identity): detect an npm-linked dev dependency the specifier cannot see `isLocallyLinkedSpecifier` admits a devDependency whose SPECIFIER is checkout-local. `npm link <pkg>` leaves the specifier a registry range while the node_modules entry symlinks to a checkout — locally linked, invisible to a specifier check, so a semantic-only edit there still moves neither digest. The obvious placement is unaffordable, measured rather than assumed: probing every dev-only name inside collectRuntimePackages costs 1998 resolutions, not the ~8 it looks like, because dependencyNames runs for every package in the BFS and published tarballs retain their devDependencies. Persisted path guards go 2221 -> 11050 (+398%), and every guard is re-probed on each warm validation — the path `status` takes. Scoped to the root package instead. The declared-intent half is untouched and still enumerated everywhere: it alone can emit the `<missing>` edge for a declared link whose checkout is absent, where resolution returns null and cannot distinguish that from an uninstalled dev tool. The new resolved-location half runs only when `parent.root === packageRoot`, resolves through the existing resolver so its path guards are recorded, and admits a name iff the realpath'd root carries no node_modules segment. Bounded against mis-fire by EXPANSION. "Not under node_modules" is a proxy for "checkout-local"; under a relocated pnpm virtual store every dev dep passes it and the whole dev tree folds into the receipt — against limits that THROW, so a legitimate install would abort. Measured here: uncapped, that shape takes 259 -> 347 packages and 2250 -> 3786 guards. The cap admits at most four and DROPS THE WHOLE CHANNEL on overflow rather than an arbitrary prefix, because the abort comes from the transitive payload of whichever trees get folded in — four of a mis-fired thirteen is still unbounded, and a sorted-prefix receipt would be arbitrary. Overflow falls back to the specifier-only receipt that ships today. Cost on this install: 259 packages unchanged, 13 dev names resolved, guards 2221 -> 2250 (+29, +1.3%). Verified against the real implementation, not just a replay: validation guards 16295 -> 16324, packageCount and artifactCount unchanged, and `dependencyRuntime.digest` byte-identical — so this forces no re-analysis for anyone. Each test fails on the defect it targets: disabling the channel kills the npm-link and cap cases; dropping the root-only scope makes the differential guard-count case fail at 2.8x guards; removing the specifier half kills the `<missing>` case. Refs #2798 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Gergő Magyar
|
561f913a32
|
fix(embeddings): retry unparseable 200 responses and survive partial embedding failures (#2790) (#2795)
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
Skill copy sync / shipped skills drift guard (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
* fix(embeddings): retry unparseable 200 responses and survive partial embedding failures (#2790) A long-running embedding job against an OpenAI-compatible endpoint could lose hours of work to a single transient glitch, then refuse to recover on the next run. Four defects compounded: 1. An HTTP 200 carrying a truncated or non-JSON body was never retried. `classifyOutcome` treats any 2xx as success, and the `resp.json()` parse ran after `resilientFetch` had already returned, so the parse failure surfaced as a terminal error. Measured: a 503 got 3 attempts, a garbage 200 got 1. The parse and the response-shape check now run inside the `fetchImpl` callback, so a bad body is classified as a retryable failure and gets the same backoff as a 5xx. This also stops a garbage 200 from calling the circuit breaker's `recordSuccess()`, which previously erased accumulated failures and meant an endpoint alternating 5xx and garbage-200 could never trip it. 2. One failed `embedBatch` sub-batch aborted the entire pipeline. Failures are now tolerated: the sub-batch's node ids are collected and all of their embedding rows are deleted, so those nodes hold zero rows and are re-embedded later. Deleting rather than keeping partial rows is deliberate — chunk arrays are flat over a 16-node batch and sliced by 8, so a node's chunks can straddle a sub-batch boundary, and surviving rows carry the current content hash. The hash maps collapse per-chunk rows last-row-wins, so a partially embedded node would read as fresh forever and never regenerate its missing chunks. A run that fails 5 sub-batches in a row still aborts, and rethrows the first error of the streak rather than the last: after 3 failures the circuit breaker opens, so later errors degrade into "circuit open, retry in 30s" while the first still names the real defect. 3. The Phase 5 `embeddingCount === 0` fail-fast could not tell "wrote nothing" from "could not ask" — the count query's catch was silent. The count is now tri-state and only a known zero after real work is fatal. A non-numeric count previously bypassed the gate entirely, because `Number()` returns NaN and `NaN === 0` is false, and then serialized as `embeddings: null`. An unverified count no longer certifies `capabilities.vectorSearch.status`. 4. `saveEmbeddingCheckpoint` wrote a completion-shaped meta: it advanced `lastCommit`, wrote the new `fileHashes` and cleared `incrementalInProgress`. The first checkpoint window fires before a single embedding exists, and on a full rebuild the graph is still in a staging database that a crash discards. The next run then diffed against the advanced hashes, saw no changes and preserved the old graph — the "skipping wipe" symptom in the report. It now re-reads meta and replaces only the checkpoint, matching what the server endpoint already did. A partially failed run keeps its checkpoint with the failed ids in `pendingNodeIds`, so the next plain `analyze` regenerates them through the existing resume path. Clearing it would have been silent data loss: a plain run derives `shouldGenerateEmbeddings: false` once embeddings exist, so the pipeline would never have run again. The old crash-and-abort self-healed only by accident, via the checkpoint its crash left behind. `gitnexus status` reports the index incomplete until the nodes recover, and `--drop-embeddings` still abandons them. `POST /api/embed` is the pipeline's other caller and was discarding the result, reporting "Embeddings complete" for a partial run. It now persists the pending ids and reports the run as failed with the underlying endpoint error. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(embeddings): abort a run whose sub-batch failure ratio is too high (#2790) The consecutive-failure ceiling only catches a total outage, because any successful sub-batch resets it. An endpoint under load shedding that alternates success and failure never trips it, so the run walks the whole corpus, deletes every failed node's rows and exits 0 having dropped a large fraction of the index. The retained checkpoint made that visible in `gitnexus status`, but a run that drops a quarter of the corpus should tell the operator to fix their endpoint, not leave them to notice a status flag. Adds a cumulative guard: abort once more than 25% of attempted sub-batches have failed, evaluated as the run progresses and gated behind a floor of 20 attempted sub-batches. The shape follows Resilience4j's circuit breaker (failure rate plus a minimum-sample floor) because it is the only one of the surveyed designs that answers the small-repo case — a three node repo can fail one sub-batch and never accumulate enough sample for a ratio to mean anything. The rate sits below a live traffic breaker's 50% because a batch indexer's job is to index the whole corpus rather than serve degraded traffic, and above Hadoop's single-digit `failures.maxpercent` because tolerating transient hiccups is the point of the change this follows. The guard reuses the existing break-then-cleanup path, so the failed batch's DELETE still runs before the rethrow, and it wraps the retained first-error-of- streak rather than inventing a new one, so the message names both the ratio and the underlying endpoint failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(server): record the embedding count after /api/embed so the next analyze cannot wipe it `POST /api/embed` generated embeddings and wrote them to the database but never wrote `stats.embeddings` into meta.json. Its checkpoint writer replaced only `embeddingCheckpoint`, and the finalize write folded in nothing else. So a repo embedded purely through the server kept whatever count the last CLI `analyze` stamped, which is 0 for a repo analyzed without embeddings. The next CLI run read `existingEmbeddingCount = 0`, `deriveEmbeddingMode` returned `shouldLoadCache: false`, and `gitnexus analyze --force` wiped the database with no cache load. Every server generated embedding was silently destroyed, with no warning — the user just lost semantic search. The route now measures the live count with the same query the CLI uses and folds it into both meta writes. The measurement is tri-state and deliberately never falls back to 0: an unverified count is written as absent rather than as zero, because a wrong-low value is exactly what arms the wipe. It is taken after `flushWAL()` and inside `withLbugDb`, so it describes durable rows and the connection is still open. A partial run records its honest count too, alongside the retained checkpoint, so the next CLI run preserves the partial index instead of discarding it. Found while working #2790; not part of that issue. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(embeddings): retry short 200 bodies and stop laundering body-phase timeouts Two gaps in the #2790 retry fix, both found by review. A 200 carrying `{"data": []}` or fewer vectors than inputs passed the in-`fetchImpl` shape check, because `every(isEmbeddingItem)` is vacuously true for an empty array. `resilientFetch` then classified it `success` and called `recordSuccess()`, erasing the outage signal, and the cardinality check in `httpEmbed` threw terminally one attempt later. That is exactly the pair of properties #2790 was filed about, still broken for this body shape — and worse than before the fix, since the pipeline now tolerates the error by deleting those nodes' rows instead of aborting loudly. The count check moves inside the retried callback; the outer one stays as a backstop. The `.json()` catch also swallowed every rejection, not just parse errors. `AbortSignal.any([caller, timeout])` is wired to the body stream, so a stalled body rejects with a DOMException — which, wrapped in a plain Error, defeated `classifyOutcome`'s terminal-network test. Measured: the same TimeoutError got 3 attempts and "unparseable response" when raised during the body read, but 1 attempt and "timed out after 180000ms" when raised by fetch itself, and three such sub-batches opened the process-global breaker that `recordNeutral()` exists to protect. Abort-like DOMExceptions are now re-raised unchanged. The dimension check stays outside the loop deliberately: it validates against `config.dimensions ?? DEFAULT_DIMS`, not the request-dimensions argument, and a width mismatch is a configuration error where retrying only triples latency and books failures against a healthy endpoint. Adds the negative assertion the review found missing: response body text must never reach the user-facing error string. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(embeddings): scale the sub-batch failure-ratio floor to the run The cumulative guard needed 20 attempted sub-batches before a failure rate could abort anything — roughly 160 chunks, or ~80 embeddable nodes at the default subBatchSize of 8. A 50-node repo whose endpoint sheds every other sub-batch fails half of them and still exits 0: the ratio guard is below its floor, and every intervening success resets the consecutive ceiling. The floor was a good choice for a first run over a small repo, where one failure out of one sub-batch is 100% and means nothing. The defect is that every resume run has that shape by construction — its node set is only the pending ids — so the guard was structurally off in the one run whose entire purpose is retrying against the endpoint that already failed. The floor is now sized to the run: clamp(ceil(totalNodes / 16), 5, 20). The lower bound keeps the case the flat floor protected; the upper bound preserves today's behavior above 320 nodes and avoids a proportional-only floor perversely weakening the guard at scale, where a sixteenth of a 20k-node repo would be 1250 sub-batches of damage before a rate could fire. Resilience4j can use a constant minimumNumberOfCalls because a breaker sits on an unbounded call stream; a batch indexer has a finite budget, so a constant can exceed the whole run. The ratio is still evaluated only inside the catch. That is already its local maximum — both counters have just incremented — so sampling more often would only ever observe lower ratios. Also: a failing cleanup DELETE no longer swallows the abort, which was discarding the retained first-error-of-the-streak that names the real endpoint fault; `ceilingError` is renamed `abortError` since it carries the ratio abort too; and three `{ error }` log keys become `{ err }` (#2114 — an arbitrary key serializes to `{}`, losing message and stack). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(analyze): one tri-state embedding counter, and stop partial runs wedging later runs The tri-state count doctrine this branch introduced was applied at two of its three CLI sites, and the two implementations that were meant to mirror each other had already drifted. `measurePersistedEmbeddingCount` moves to `core/embedding-count.ts` — beside `embedding-mode.ts`, with the same no-native-imports property, and outside `core/embeddings/` so the lazy-embeddings convention (#2370) still holds. All three call sites now share it. - The mid-run `onCheckpoint` counter ran the query bare. A throw there — DB busy, connection closed, read-only, the VECTOR DML lock (#2623) — rejected the callback out of `runEmbeddingPipeline` and killed the analyze before Phase 5 could apply the tri-state that exists for exactly this case. A non-numeric cell wrote `stats.embeddings: null` to disk mid-run. - Phase 5 used `?? 0` while the server used `?? Number.NaN`, under a comment asserting both measured the field the same way. `Number.isFinite(0)` is true, so a no-row answer became a *measured* zero and hard-failed a run whose embeddings had all persisted. - The unknown-count fallback read `existingMeta`, assigned once at run start, so it republished the pre-run figure over the fresher count the terminal checkpoint had already written. With a prior count of 0 that armed the wipe chain: hasExisting false, shouldLoadCache false, and the next --force discards live embeddings. It now re-reads the latest on-disk meta, and an unverifiable count retains a recovery marker instead of clearing it. A completed-but-partial run also planted a landmine. Its checkpoint is stamped with the run's embedding identity, so a later plain `gitnexus analyze` from a hook, a CI job, or a shell without GITNEXUS_EMBEDDING_URL resolved provider 'local' and threw before any phase ran — after an exit-0 run, where previously only a visible crash left that state. `--force` did not help: the resume gate inspected only `--drop-embeddings`. `RepoMeta.embeddingCheckpoint` gains `kind` to tell the two situations apart. An 'interrupted' marker (or one with no kind, so markers already on disk keep the stricter path) still fails closed — its nodes may be half-written, and resuming under a foreign model would mix vector spaces. A 'partial' marker names nodes the pipeline already deleted to zero rows, so nothing is at risk: an identity mismatch drops the pending set with a warning and continues. `--force` now discards a checkpoint, and `attempts` bounds the retry at EMBEDDING_RESUME_MAX_ATTEMPTS (3, matching the HTTP embedder's and the WAL driver's existing per-operation budgets) so a node the endpoint deterministically rejects converges instead of keeping the repo incomplete forever. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(server): close the SSE stream on terminal job status, not a progress phase A tolerated partial run reached SSE clients as a clean success — a regression in this branch's own claim that /api/embed reports a partial run as failed. The pipeline emits `phase:'ready'` unconditionally before returning, including when it dropped nodes. The route mapped that to `'complete'`, and `mountSSEProgress` treated a terminal-looking *progress phase* as terminal: write the event, `res.end()`, `unsubscribe()`. The route's own `updateJob({status:'failed'})` then fired into a stream with no listener, and the web app had already shown "ready". Before this branch the pipeline threw, which produced `phase:'error'` and did reach the client. Pollers on GET /api/embed/:jobId were unaffected, so the two consumers disagreed. Terminality is a property of the job, so the relay now asks the job. Remapping `ready` alone would have left the trap armed: the `error -> 'failed'` mapping has the identical shape and would emit `event: failed` with `error: undefined` before the catch block fills the message in. `ready` is additionally remapped to `finalizing` so a poller no longer sees `status:'analyzing'` next to `progress.phase:'complete'`. The single-terminal-event property (#2264) is preserved on both the clean and partial paths, and /api/analyze is unaffected — its terminal progress phase is 'done', never 'complete'. `AnalyzeJob` gains an optional `partial` payload so a client can tell a partial run from a total failure without a new status member; it is absent on every other job, so existing payloads stay byte-identical. Consuming it in gitnexus-web is left to that app's owner — today it renders both as the same red retry chip. `resolveEmbedRunOutcome` moves to `embed-run-outcome.ts` and `mountSSEProgress` to `sse-progress.ts`, both free of Express/LadybugDB/MCP imports, and the local count copy is replaced by the shared `core/embedding-count.ts`. Reaching three pure functions previously meant importing the whole server: measured at ~20s against a 30s test timeout, with one observed timeout failure. That file is now 1.6s. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: document the partial embedding index and its recovery A run can now finish exit 0 with a partial embedding index, which neither operator doc described. GUARDRAILS' "Embeddings vanished after analyze" Sign keys its trigger on `stats.embeddings` being 0 and lists "the only ways to end up at zero". A partial run stamps an honest non-zero count and sets `embeddingCheckpoint`, so the operator's actual symptom is `incompleteReasons: ["embedding-checkpoint-pending"]` — a state that Sign cannot match. Adds a Sign for it and drops the exhaustive framing from the existing one. RUNBOOK gains the recovery path: a plain `gitnexus analyze` is correct and needs no flag, because a retained checkpoint forces generation for the pending nodes regardless of flags. Also corrects two stale claims — that `stats.embeddings` is always freshly measured (it can carry forward when the count query cannot answer, which is why `capabilities.vectorSearch.status` is the certified read), and that later analyzes must always pass `--embeddings` or lose their vectors, which contradicts Non-negotiable 5. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(embeddings): one owner for the checkpoint record and the abort predicate Cleanup pass over the #2790 review fixes. No behavior change except where noted; the two exceptions are both cases where the code was lying to the operator or to the other half of itself. The previous pass extracted `core/embedding-count.ts` because two hand-copied bodies of "measure the embedding count" had drifted inside a single change. It then created a second pair of hand-copied publishers — of `RepoMeta.embeddingCheckpoint` — and those had drifted too: the CLI armed the attempt counter only after clearing its identity gate, the server derived it from the resumed marker alone. Only one of the two READERS implemented `kind` at all, so a 'partial' marker written by `gitnexus analyze` and resumed through POST /api/embed still hit the permanent wedge `kind` exists to remove. `core/embedding-checkpoint.ts` now owns the record: `checkpointKind` (the one home for absent-means-interrupted), the three minters, `nextAttemptCount`, and `decideEmbeddingResume`, which both gates route through. Five mint sites and two resume gates become one implementation each. `resilient-fetch.ts` exports `isTerminalNetworkError` and `classifyOutcome` calls it, replacing a caller-side copy of the same DOMException test whose docstring promised it "mirrors classifyOutcome exactly" — an invariant enforced by prose, where a divergence silently reverts body-phase timeouts to being retried three times and charged to the shared breaker. The ratio-guard floor now divides by the run's actual `subBatchSize` instead of a constant 16 that assumed the default of 8. At `subBatchSize: 32` the old formula demanded more sub-batches than the run contains, leaving the guard structurally off — the exact failure the scaled floor was introduced to fix, and sub-batch size is tuned mainly for the flaky endpoints it protects. Two operator-facing corrections: - The count-recovery marker was stamped `kind: 'partial'` with an empty pending set, so `gitnexus status` reported "N node(s) lost their embeddings" where N is zero. It gets its own kind and its own incomplete reason. - `decideEmbeddingResume` initially keyed its skip-the-identity-gate branch on an empty pending set, assuming that meant the count-recovery marker. It does not: `onCheckpoint` mints an 'interrupted' marker with no pending nodes after every post-window save. That silently cleared an interrupted marker under a foreign provider instead of failing closed. Keyed on `kind` now, with a regression test. Also: `isTerminalJobStatus` adopted at the seven sites that still hand-copied it, including the one gating the single-terminal-event emit; `mountSSEProgress` re-export dropped and `server-sse-payload.test.ts` repointed at the extracted module, which takes it from 24.60s to 0.408s — the test that motivated the extraction was still paying the cost it was meant to remove; the count-mismatch message and the SSE test harness deduplicated; per-batch error strings made lazy (~75k needless `new URL()` per large run); `retryable: true` dropped as a field that can never be false; ~110 lines of restated rationale reduced to pointers at their canonical home. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
azizur100389
|
797e4ef8f6
|
fix(ai-context): document CLI graph fallbacks (#2803)
* fix(ai-context): document CLI graph fallbacks Teach generated GitNexus guidance to pair mandatory MCP graph checks with repo-scoped CLI fallbacks so agents can keep working when MCP is unavailable. Co-authored-by: Cursor <cursoragent@cursor.com> * style(ai-context): satisfy Prettier --------- Co-authored-by: Cursor <cursoragent@cursor.com> |
||
|
Gergő Magyar
|
990d79ba8c
|
fix(mcp): make impact/context reproducible — deterministic ordering on every capped query (#2787) (#2796) | ||
|
Gergő Magyar
|
010a7d806a
|
fix(schema): declare the full scope-resolution relation cross product (#2792) (#2793)
* fix(schema): declare the full scope-resolution relation cross product (#2792) `RELATION_SCHEMA` was hand-listed, and every prior fix added only the FROM/TO pair named in a crash report — `Const→Method` in #2769, the Swift/Rust member pairs before it. So `analyze` kept aborting at `assertDeclaredPair` on the next codebase whose edges happened to land on a different pair; #2792 reports `Class→Variable` on Java. Audit the surface instead of the symptom. `buildGraphNodeLookup` skips any node whose label is not in `isLinkableLabel`, so the lookup holds only linkable-labelled nodes — and both endpoints of every graph-bridge edge resolve through that lookup. The emittable surface is therefore exactly: FROM LINKABLE_LABELS + File (the module-level caller fallback) TO LINKABLE_LABELS + CALL_TARGET_TYPES `isCallerAnchorLabel` is a strict subset of linkable and contributes nothing on top. `CALL_TARGET_TYPES` contributes `Delegate`, which `tryEmitEdgeWithExplicitTargetId` can emit without going through the lookup at all. Generate that 14x14 block into the DDL rather than listing it: 223 -> 322 declared pairs, and no future pair from these sets can be missing by construction. The containment/inheritance/DI/route/cluster/PDG pairs stay hand-declared — no single predicate describes them. Both label sets live in the ingestion layer, which `core/lbug` must not import, so schema.ts carries twin lists. test/unit/schema-pair-coverage.ts derives the requirement from the originals and fails CI when either set grows without the pairs landing here — the piecemeal loop this fix ends. Measured before widening: at 322 pairs the cost is inside noise (1.09s vs 1.12s per 300 anchored queries on a 32-table DB), but the full 32x32 cross product is ~1.8x on untyped-endpoint anchored queries. The audited subset is the right scope, not "declare everything". INCREMENTAL_SCHEMA_VERSION 34 -> 35: LadybugDB fixes endpoint pairs when the rel table is created, so a pre-v35 database physically cannot store these edges. Closes #2792 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(schema): declare the non-bridge structural pairs COBOL and Vue emit The generated scope-resolution block closed the half of RELATION_SCHEMA a label predicate can describe. The hand-declared half was still stale: with #2791's Function->Variable fix applied, `analyze` continued to abort on this repo's own test/fixtures/lang-resolution with Relationship label pair Module→Property is not declared A full sweep (assertDeclaredPair patched to log-and-skip, run over the whole fixture corpus) found 13 undeclared pairs over 106 edges. This branch already covered 3 of them via the cross product; the remaining 10 come from emitters outside the graph bridge: - cobol-processor.ts mints Module / Namespace / Record / Property / CodeElement and wires them with CONTAINS, CALLS and ACCESSES (9 pairs) - vue-sfc-extractor.ts emits BINDS_EVENT_HANDLER from a handler Function to the child component's File, the only edge whose target is a File (1 pair) CodeElement, Namespace, Record and File are in neither scope-bridge label set, so neither the generated block nor schema-pair-coverage.test.ts can reach them. Adds test/integration/structural-pair-coverage.test.ts, which derives the requirement from a corpus instead of a predicate: it runs the real pipeline over the non-bridge fixtures and requires every FROM/TO pair they produce to be declared. Mutation-checked — dropping `FROM Function TO File` fails it with exactly Function|File. Verified: cobol-app, vue-basic and php-transitive-traits now index instead of aborting; the full lang-resolution corpus completes at 10,876 nodes / 18,517 edges; scrypster/muninndb at 0b7a4272 (the #2789 repro) completes at 20,069 nodes / 71,580 edges, matching #2791 exactly, so this supersedes that PR. * refactor(test): simplify the structural pair coverage guard Cleanup pass over the previous commit. No behaviour change to the schema. - reuse `FIXTURES` and `runPipelineFromRepo` from resolvers/helpers.ts instead of re-deriving the fixture root and importing pipeline.js directly - gate on `distWorkerExists()` like every other integration test that passes `workerUrlForTest`, so a missing dist skips rather than fails - run the three fixtures with `it.concurrent.each`; they share nothing and the cost is almost all worker spawn plus grammar load, which overlaps well (tests phase 21-24s -> 5.6s measured) - replace the sentinel-in-a-Set filter with a plain `.filter()` chain, matching the sibling unit test, and move the declared/table lookups off the per-edge path onto the deduped set - move the pure string pin out of the integration tier into schema-pair-coverage.test.ts, where the identical construct already lives, so it needs no build and survives fixture deletion - trim the schema and test prose that restated the code, and correct the BINDS_EVENT_HANDLER attribution: it is emitted by languages/vue/scope-resolver.ts, not vue-sfc-extractor.ts - amend the v35 comment to mention the 10 structural pairs it now also stamps Still mutation-checked: dropping `FROM Function TO File` now fails both the integration sweep and the unit pin with exactly Function|File. 89 tests green. * fix(schema): generate the attachment pair surface and close four analyze aborts Review of the generated scope-bridge cross product found four `analyze` hard-aborts still live at head, each reproduced end-to-end on the default user path (`analyze --index-only --skip-git`): Method→Annotation Spring `@Bean` + `@ConditionalOnMissingBean` (Java + Kotlin) Method→File Vue Options-API `methods:` handler bound to a child event Namespace→Record COBOL `DECLARATIVES` / `USE AFTER STANDARD ERROR ON <file>` Class→Tool `@mcp.tool()` applied to a class All four are pre-existing on main, and both existing guards were structurally blind to them: the unit guard derives from LINKABLE_LABELS ∪ CALL_TARGET_TYPES (none of Annotation/Tool/Record/File-as-target is a member) and the corpus guard ran three fixtures that exercise none of these emitters. All 16 tests passed while all four crashes were live. The PR's model — "bridge endpoint × structural endpoint" — does not fit: Namespace→Record is structural on both sides. The property that does hold is that the ANCHOR is a lookup result, not a literal at the emit site, so the emitter cannot constrain its label. That gives a second closed-form rule: DEFINITION_ANCHOR_LABELS × ATTACHMENT_TARGET_LABELS DEFINITION_ANCHOR_LABELS is derived from NODE_TABLES by subtraction, so a new node table joins automatically. 332 → 450 declared pairs. Sized against a committed harness (gitnexus/bench/schema-pairs), real @ladybugdb/core, identical data: 450 costs 0.93–1.05× of 332 on untyped-endpoint anchored queries — inside noise — versus 1.22–1.43× at 641 and 2.03–2.34× at 1024. The harness reproduces the known #2792 cliff, which is what makes the 450 figure trustworthy. Also in this change: - Delete the 161 hand-declared pairs the rules already generate (233 → 72). The declared set is byte-identical at 450; those lines were load-bearing shadow, because the generator suppresses anything already declared structurally, so narrowing a rule later would silently keep pairs alive. A new guard fails CI if a hand-declared pair is ever re-added inside a rule. - Import LINKABLE_LABELS / CALL_TARGET_TYPES instead of hand-copying them. The twins' stated justification ("the ingestion layer must not be imported here") is false: csv-generator.ts and lbug-adapter.ts, siblings in the same directory, already do, and no rule in AGENTS.md / ARCHITECTURE.md / CONTRIBUTING.md / GUARDRAILS.md states otherwise. - Resolve `resolveStreamGraphEmit` after the guards that rebind `options.force`, not at function entry. It gates on `force`, and every freshness guard runs ~360 lines later, so the v34→v35 bump would have pushed every existing index down the non-streamed emit path — losing the #2680 memory streaming added for the #2649 kernel-scale OOM, for exactly the population most likely to be memory-constrained. - `UndeclaredRelationPairError` now carries the relationship type, both node ids and the source file, with a matching CLI branch. The old message named only the abstract label pair, which a user could not act on. Found through the cause chain, since pipeline-phases/runner.ts rewraps every phase failure. - Share one classifier (`relPairKeyFor`) across the router, both emit sinks and the corpus guard, which previously hand-mirrored the router's skip rule; one cause-chain walker in lib/utils.ts; one exported pair-matching regex. - Corpus guard: four new fixtures reproducing the aborts, per-fixture sentinel pairs so a fixture that stops emitting fails loudly instead of passing vacuously on an empty graph. The per-edge path stays allocation-free: the failure context is passed positionally and the message is built only inside the throw. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0182jkjQqzACkJKYw4MLDnhX * test(bench): re-baseline the COBOL capture fingerprint for the new fixture `bench/scope-capture` globs `lang-resolution/cobol-*`, so the `cobol-declaratives` fixture added in |
||
|
Gergő Magyar
|
74409a37f6
|
perf(cpp): index qualified namespace members once per pipeline run (#2788) (#2794)
* perf(cpp): index qualified namespace members once per pipeline run (#2788)
`resolveCppQualifiedNamespaceMember` walked every parsed file — rebuilding a
per-file `scopesById` map each time — once per qualified `ns::member()` call
site, so the scope-resolution emit phase cost O(callsites x scopes). On a
1,473-file C++ repo that was 25.3 min of a 33-min analyze, with 75% of total
self-time in this one function. Its inner `findMemberInNamespaceTransitive`
compounded it: each recursion step filtered `scopesById.values()` by parent,
O(scopes^2) per file on its own.
This is the same bug #1990 fixed in the sibling ADL path (`pickCppAdlCandidates`
-> `AdlCandidateIndex`), so it gets the same fix: a `QualifiedNsMemberIndex`
(receiver simple name -> member simple name -> callable defs) built lazily once
per `parsedFiles` identity and reset by `clearCppInlineNamespaces`, which runs
from `cppScopeResolver.loadResolutionConfig` at the start of every pass. Per
call site the work drops to two Map lookups.
Ordering is preserved exactly — file-major, `parsed.scopes` declaration order,
a namespace's own `ownedDefs` before its inline-namespace children, depth-first
— because the caller takes `allHits[0]` for the single-hit case and
`narrowOverloadCandidates` is first-wins. Non-inline nested namespaces are
still not descended into, and same-name hits across inline children still
report `'ambiguous'` (#1564).
Measured with `PROF_SCOPE_RESOLUTION=1 analyze --force --index-only` on a
synthetic corpus (`namespace ns_i { inline namespace v1 { ... } }` plus 20
`ns_j::fn()` call sites per file):
| files | emit before | emit after |
|-------|-------------|------------|
| 100 | 153ms | 16ms |
| 200 | 704ms | 24ms |
| 400 | 3,293ms | 42ms |
| 800 | 16,898ms | 78ms |
Before, doubling the file count quadrupled emit; now it doubles. At 800 files
total scope resolution goes 17.2s -> 394ms.
Output is unchanged, verified rather than assumed: a full graph dump (sorted
nodes + relationships) from a baseline build at the parent commit and from this
one are byte-identical on all 134 `cpp-*` fixtures merged into a single repo
(1573 nodes / 1997 relationships) and on the 400-file synthetic corpus.
`test/integration/resolvers/cpp.test.ts` passes 334/334.
#1990 shipped its ADL fix without a scaling gate, which is how the bug class
came straight back here, so this adds one: `bench/cpp-qualified-ns` measures
`(t_large/t_small)/(1600/400)` — 0.93-1.21 indexed versus 3.45 for the old
per-call-site scan — alongside a fingerprint over every
`receiver::member -> outcome` the corpus resolves, and CI runs it with
`--check`. `test/unit/cpp-qualified-ns-index.test.ts` covers the cache
invalidation the index introduces.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(cpp): address tri-review findings on the qualified-namespace index (#2788)
Multi-engine review of this PR (Claude swarm + ce-code-review, Codex
gpt-5.6-sol swarm + ce + adversarial) returned two P1s and five smaller
findings. All are fixed here.
P1 — the index defeated the pipeline's post-language memory release.
`scope-resolution/pipeline/phase.ts` evicts each language's files and then
calls `forceGc()`, on a stated premise that "This language's ParsedFiles are
now unreachable", sizing C/C++ at ~17-20GB on the Linux kernel. The
module-level `let qualifiedNsIndexSource` falsified that: it pinned the whole
`parsedFiles` array, and the index held defs reaching into those files'
scopes, until the *next* C++ pass cleared it — which in a single analyze never
comes. C++ is 7th of 16 in SCOPE_RESOLVERS, so the set survived nine later
language passes plus emit. Replaced with a
`WeakMap<readonly ParsedFile[], QualifiedNsMemberIndex>`, the pattern already
used by `moduleScopeIndexByPass` in `cpp/file-local-linkage.ts`.
`clearCppInlineNamespaces` still swaps in a fresh WeakMap, because the index
has a second input (`inlineNamespaceScopeIds`) the key cannot observe.
Measured with `--expose-gc`: 61.2MB retained after the caller drops the array
before, 0.1MB after.
The ADL twin (`adl.ts`) has the same pattern, so the hazard predates this PR —
but `pickCppAdlCandidates` returns early before `ensureAdlIndex` on
`noAdlSites`/empty `argInfoBySite`, so it rarely arms, whereas a qualified
`ns::member()` index arms on almost every C++ workspace. Moving the ADL twin
to a WeakMap is left as a follow-up.
P1 — the new bench could not see the regression class it exists to gate.
`callSites()` drew every receiver from `ns_${...}`, so the receiver lookup
never missed; production is the opposite, since Case 1.5 in
`receiver-bound-calls.ts` is reached by every plain-identifier receiver call
and misses on most. A rescan reintroduced only on the receiver-bucket-absent
path scored 1.279 and PASSED the old bench. The corpus now mirrors production
(~1 in 5 receivers name a declared namespace) and adds a namespace reopened
across files, a same-name inline nest, a member declared at both namespace and
inline-child level, and call sites carrying a real `Callsite` so
`narrowOverloadCandidates`/`cppConversionRank`/
`isOverloadAmbiguousAfterNormalization` are inside the fingerprinted surface at
all. That same rescan now measures 4.538 and FAILS; defeating the dedup now
fails the fingerprint arm where it previously passed byte-identical. The
fingerprint moved once, deliberately, for the corpus expansion — recorded in
`_rebaseline_2788_review`, explicitly not precedent.
Also fixed:
- Unbounded recursion aborted analyze. `collectNamespaceMembers` recursed per
inline child with no bound and threw an uncontained `RangeError` at inline
depth 8000 (`phase.ts`'s try has a `finally`, no `catch`), and a receiver
*miss* paid full recursion where the deleted walker skipped on a name
mismatch. An explicit work-stack alone would only have converted that into
an OOM at depth 6000, because the eager table was quadratic in memory too:
for a depth-D chain it legitimately holds D(D+1)/2 entries, since `v2::foo()`
is a valid receiver at every level. Replaced with a lazily-queried node graph
(per-scope own-member buckets plus direct child links, resolved on demand and
memoized per receiver+member). Build is now linear; depth 100000 costs 133ms
where 8000 previously threw.
- "#1990 shipped without a scaling gate" was false. #1990 did ship
`test/integration/cpp-adl-benchmark.test.ts` (
|
||
|
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
|
||
|
Karl Lehenbauer
|
639eb04b31
|
fix(swift): preprocess indented conditional directives so class bodies survive parsing (#2771)
* fix(swift): preprocess indented conditional directives so class bodies survive parsing * fix(swift): make conditional-directive blanking comment-, string- and brace-aware (#2771) Addresses the review findings on PR #2771. The transform fired unconditionally, which turned valid Swift into parse errors while missing the most common shape it was written for. - The blank/keep decision now consults `blockCommentDepth`, so ` #endif */` — the result of commenting out a conditional block — keeps its comment terminator. Previously `hasError` went raw=false -> preprocessed=true and the rest of the file was swallowed. - The decision keys on the scanner's brace depth instead of indentation. A column-0 `#if` inside a class body is blanked (6 of 7 body shapes previously still lost the enclosing declaration) and an indented file-scope directive is not — matching what the doc comment already claimed. Bare-CR line endings, NBSP/ideographic indentation and a leading BOM are recognized too. - A group is blanked only when every branch is brace-balanced. An `#if`/`#else` that splits a declaration header leaves one unmatched `{` once both branches survive, which collapsed five top-level nodes into one and gave unrelated types fabricated `NetworkClient.` qualified names. Such a group now degrades to the pre-fix behavior. - Multiline strings honour `\"""` escapes, and a plain `"""` closes even when a `#` follows it, so the scanner no longer wedges in string state and silently stops blanking for the rest of the file. - The pound run is counted once per position and skipped. It was quadratic: 10.6s for one 64k-`#` line, well inside the 512 KB walker limit. - Extended regex literals (`#/.../#`) no longer open a phantom block comment. - Directive-free files return early, matching `stripUeMacros`. Worker parity: `emitSwiftScopeCaptures` and `emitCppScopeCaptures` re-apply their provider's `preprocessSource` on the parse-cache-miss path — Dart already did this — and the embedding parse in `ensureAndParse` applies the hook as well. Before this the worker and the scope-capture/embedding halves analyzed different programs, turning a consistent degradation into cold-run/warm-run non-determinism. A new parity test pins the equivalence for every provider that defines the hook. SCHEMA_BUMP 37 -> 38: this changes parse semantics, the chunk key hashes raw on-disk bytes, and `preprocessSource` runs after the key is computed — so a same-package-version warm cache would replay pre-fix Swift results verbatim, including across `--force`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(ingestion): apply preprocessSource once in the scope bridge (#2771) Follow-up cleanup on the review fixes. The previous commit re-applied each provider's `preprocessSource` inside `emitSwiftScopeCaptures` and `emitCppScopeCaptures`, mirroring what Dart already did — three copies of the same rule, and a contract that asked every future emitter to remember it. `extractParsedFile` is the single funnel every `emitScopeCaptures` caller passes through (parse worker, scope-resolution run, Vue script extraction), and it already receives the provider. Applying the hook there on the cache-miss path covers all three languages and every future one, names no language in shared code, and drops Dart's unconditional transform on the cache-hit path. Verified the three emitters use `sourceText` for nothing but the parse, so the substitution is output-identical — which the parity test asserts directly. Also from the cleanup pass: - the parity test derives its language list from the provider registry, so a new provider adopting the hook fails until it adds a fixture - `ensureAndParse` resolves the provider from the language it already computed, instead of a second extension table (`getProviderForFile`) - the preprocessor returns `sourceText` unchanged when no group was blanked, which is the common case for files whose only directives are top-level - `split(/(\r\n|\n|\r)/)` replaces the hand-rolled line splitter, and the per-group brace bookkeeping is two scalars instead of an array - the hint regex is derived from the line regex so the two cannot drift - unit assertions compare the WHOLE preprocessed file against the expected blanking, replacing per-line spot checks; the pipeline tests share one `runFixture` helper and `getNodesForFile` in the resolver test helpers - `LanguageProvider.preprocessSource` documents the real call sites and says plainly that the set is not closed — `populateRangeBindings` still hands language helpers raw text Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(autofix): apply prettier + eslint fixes via /autofix command --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
|
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> |
||
|
dependabot[bot]
|
a6aae8142d
|
chore(deps)(deps): bump brace-expansion from 5.0.7 to 5.0.9 in /gitnexus (#2786)
Bumps [brace-expansion](https://github.com/juliangruber/brace-expansion) from 5.0.7 to 5.0.9. - [Release notes](https://github.com/juliangruber/brace-expansion/releases) - [Commits](https://github.com/juliangruber/brace-expansion/compare/v5.0.7...v5.0.9) --- updated-dependencies: - dependency-name: brace-expansion dependency-version: 5.0.9 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
ivangegovdve-sudo
|
565287528d
|
fix(ci): stop CI Report dying silently when the tests job fails (#2728)
* fix(ci): stop CI Report dying silently when the tests job fails
The "Build report" step in ci-report.yml runs under
`bash --noprofile --norc -e -o pipefail`. It located its inputs with
UNIT_SUMMARY=$(find "$DIR/test-reports" -name ... 2>/dev/null | head -1)
`coverage-merge` in ci-tests.yml is `needs: tests` with no `if: always()`,
so any failing shard skips it and the `test-reports` artifact is never
uploaded. `find` then runs against a directory that does not exist and
exits 1; `-o pipefail` carries that status through `| head -1`, the
command substitution hands it to the assignment, and `-e` kills the step.
The death is invisible: `2>/dev/null` discards find's error and the whole
report is built into `$GITHUB_OUTPUT`, so the step logs nothing and just
reports "Process completed with exit code 1". "Comment on PR" is then
skipped, so the CI Report workflow fails and posts nothing on exactly the
PRs whose tests failed — when the report is most useful. The
"Coverage data unavailable" fallback already existed for this case but
was unreachable, because the script died ~160 lines before it.
Route the four lookups through a `find_first` helper that returns empty
when the root is absent. Verified by extracting the step body and running
it against both artifact layouts: with `test-reports` present the output
is byte-identical to the previous script (1335 bytes), and with it absent
the step now exits 0 and emits the coverage-unavailable report instead of
exiting 1 with an empty $GITHUB_OUTPUT.
Observed on 32 of the last 100 failed runs; correlation with the tests
job's conclusion was 6/6 failure and 4/4 success in the sampled runs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(ci): let the prebuild assertion report a missing .node
`Build prebuild` deletes `$pkgdir/prebuilds` before running prebuildify,
so a run that emits nothing without failing leaves `find` searching a path
that no longer exists. Under the step's `shell: bash` (`-e -o pipefail`)
that `find` exits 1 and kills the step before the `test -n "$out"` guard
below it — the guard written to explain exactly this case never runs, and
the job dies with a bare "Process completed with exit code 1".
Same shape as the `ci-report.yml` fix in this PR: a lookup that exits
non-zero on an absent root pre-empts the fallback beneath it. `|| true`
hands the empty result to the guard, which still fails the build, now with
`::error::prebuildify produced no .node`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
Co-authored-by: Gergo Magyar <gergomagyar0@gmail.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> |
||
|
ChunxueLi
|
99291891b7
|
feat: make MAX_CALLABLE_VALUE_TARGETS configurable via env (#2725)
* feat(scope-resolution): make MAX_CALLABLE_VALUE_TARGETS configurable via env The branch's original commit was a whole-file snapshot taken at a stale base and never touched the constant, so the env read was missing and the branch's own test failed. Implemented here, matching the sibling GITNEXUS_MAX_PROPERTY_DISPATCH_FANOUT knob. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(callable-value-flow): add env override tests * docs(callable-value-flow): document GITNEXUS_MAX_CALLABLE_VALUE_TARGETS env Adds a Troubleshooting subsection to README.md and a commented entry to gitnexus/.env.example for the new per-callable-site dispatch-target cap (default 32), following the maintainer's review request to document the knob alongside its implementation. --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Ubuntu <ubuntu@localhost.localdomain> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> |
||
|
Karl Lehenbauer
|
bebb1d2367
|
fix(schema): declare Swift member-containment pairs in CONTAINS DDL (#2769)
* fix(schema): declare Swift member-containment pairs in CONTAINS DDL * fix(schema): declare remaining Rust impl/trait and JS/TS object-literal HAS_METHOD pairs; guard streamed emit sinks against undeclared pairs (PR #2769 review) * refactor(schema): share one declared-pairs constant across router and sinks DECLARED_REL_PAIRS was being computed independently in three places (csv-generator.ts, graph-emit-sink.ts, pdg-emit-sink.ts) from the same static RELATION_SCHEMA parse. Export the existing constant from csv-generator.ts (already imported by both sinks) instead. assertDeclaredPair now takes the pre-built pairKey rather than the two labels, since every caller (RelPairRouter.route, both sinks' addRelationship) needs that same key immediately after for its own Map/stream lookup on the per-streamed-edge hot path — avoids rebuilding the template string twice per edge. Also drops two schema.test.ts assertions that duplicated coverage already in the more narrowly-named regression tests below them, and trims the v32 ladder comment to point at assertDeclaredPair's docstring instead of re-explaining the same failure mechanism. * fix(schema): use replaceAll for the pair-arrow error message (CodeQL) .replace(str, ...) only touches the first match; CodeQL flags that as incomplete string escaping regardless of the caller's invariant that pairKey contains exactly one '|'. replaceAll is equivalent here and silences the alert. --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> |
||
|
ChunxueLi
|
c238085676
|
feat: make MAX_PROPERTY_DISPATCH_FANOUT configurable via env (#2726)
* feat(scope-resolution): make MAX_PROPERTY_DISPATCH_FANOUT configurable via env * test(property-dispatch): add env override tests * docs(scope-resolution): document GITNEXUS_MAX_PROPERTY_DISPATCH_FANOUT in README and .env.example Add a dedicated troubleshooting subsection and .env.example entry for the GITNEXUS_MAX_PROPERTY_DISPATCH_FANOUT environment variable, matching the format used by the sibling GITNEXUS_MAX_CALLABLE_VALUE_TARGETS knob. Closes maintainer request: "Could you please document this in the readme plus the .env.example?" --------- Co-authored-by: Ubuntu <ubuntu@localhost.localdomain> |
||
|
Void Freud
|
0f78179e7f
|
fix(jvm): enforce proximity-bounded sibling injection (#2732)
* fix(jvm): bound same-package sibling injection * docs(jvm): document sibling injection cap * fix(jvm): mark truncated sibling sets incomplete and bound the merge Review follow-ups on the sibling injection cap (#2732): - The cap silently produced a third visibility state. Before it, a file was either fully visible (package under 500 files) or fully incomplete; once `injectedIds.size` hit the cap, real siblings were dropped while `isVisibilityIncomplete` still returned `false`. That flag gates wildcard attribution in seven Spring passes (`bean-candidates.ts:199` and the java/ kotlin bean-metadata, conditionals, config-bindings and DI resolvers), so 201-500-file packages — exactly this cap's population — resolved wildcard annotations against a truncated sibling set with no log signal. Truncation now marks the file incomplete and analyze warns once with the affected file count. - The cap only bounded `bindingAugmentations`; the two `typeBindings` merges below it still absorbed every sibling, so a class excluded from the binding set could still steer receiver/variable type inference through `scope.typeBindings`. Both halves now use the same bounded sibling set, and the merge iterates that set directly rather than filtering a full rescan, so the cap bounds the work as well as the result. - Path segments are split once per bucket instead of on every pairwise proximity comparison — that comparison runs O(files²) per package. - `JvmPackageFact` was re-declared locally instead of imported from `package-facts.js`, where the canonical declaration still serves both languages' facades and capture side-channels. Nothing kept the copies in sync. Restored the import. - README/.env.example: `GITNEXUS_MAX_INJECTED_SIBLINGS` does not lift the fixed 500-file package skip (including at `0`), and truncation disables wildcard attribution for the affected files. Both are now stated. * test(jvm): restore the language-facade coverage and pin the cap's behaviour The cap rewrite replaced the per-language harness with generic fixtures, dropping the Java/Kotlin capture-side-channel and facade coverage (package fact extraction, the 500-file skip, fail-closed on a file that produced no ParsedFile) and leaving a proximity fixture whose candidates were already in order — so it could not tell a working sort from plain truncation of the input. Restores that harness and adds cap-specific cases on top, driven through the shared JVM factory. The fixture interleaves near and distant siblings, so the retained set is only reachable by a working proximity sort. Covers: the exact capped set, truncation marking the file visibility-incomplete, type bindings bounded by the same sibling set, the unbounded `0` override staying complete, and the documented default of 200 applying when the variable is unset. Each new case fails against the pre-fix implementation. --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> |
||
|
Void Freud
|
7ee0df9e55
|
fix: serialize global registry transactions across processes (#2716)
* fix: serialize global registry mutations * fix: serialize global registry mutations * fix: keep registry reads lock-free * test(registry): document cross-platform lock coverage * fix(registry): isolate the registry lock's namespace, timeout and diagnostics Review follow-ups on the global registry lock (#2716): - The lock took `getGlobalDir()` itself, which is byte-identical to a repo's index slot when that repo is rooted at the user's home directory (a real dotfiles layout). `runFullAnalysis` holds the per-repo lock across its whole pipeline and `acquireIndexLock` is not reentrant, so `registerRepo` / `adoptFlatBranchLabel` self-deadlocked until the wait ceiling and then failed the analyze. The registry now locks a private `<globalDir>/registry-lock` namespace no index slot can ever resolve to. - The lock inherited the index lock's 10-minute default timeout, sized for multi-minute analyze runs. `gitnexus augment` — documented to cold-start in under 500ms and shelled out from editor tool-use hooks — reaches it through `listRegisteredRepos({ validate: true })`. Registry transactions are sub-second, so they now get their own 5s ceiling. - Contention was silent: no `log`/`onWaitStart` was wired, and the primitive's own texts attribute a wait to "another gitnexus analyze", which misnames a registry holder. A registry-specific line is emitted on wait start instead. - On timeout the transaction now proceeds unlocked with a warning rather than throwing. The lost-update race it guards was unguarded before this branch, so degrading to the old best-effort behaviour beats failing `analyze`/`list`/ `index` outright — none of which wrap these calls in a handler — on a wedged lock. - `adoptFlatBranchLabel`'s recursive `fs.rm` no longer runs inside the lock; only the closing re-read/mutate/write does, mirroring `clean.ts`, which deletes the branch directory before calling the locked `removeBranchIndex`. A slow delete no longer blocks every registry operation on the machine. * test(registry): cover the remaining locked mutators and the colliding layout Three of the five functions the registry lock wraps had no overlap coverage, so a future narrowing of the lock would go unnoticed. Adds: - overlapping `removeBranchIndex` calls on two branches of one entry, - an overlapping `unregisterRepo` / `registerRepo` pair on distinct repos, - a registration issued while an index lock is held on the global directory, which reproduces the home-rooted self-contention the lock namespace fix addresses. Each fails without the corresponding fix: the two overlap tests lose an update when `withRegistryLock` is bypassed, and the collision test sees the wait announcement and the degraded-write warning once the lock namespace is reverted to `getGlobalDir()`. The collision test asserts on those log records rather than on elapsed time, so it stays deterministic on a slow runner. * perf(registry): keep the validation walk out of the registry lock `listRegisteredRepos({ validate: true })` held the global lock across its read-only validation walk — an `fs.access` per entry, slow on a network mount or a large registry — even though the common case prunes nothing and writes nothing. That is the same lock `gitnexus augment` takes on every editor tool call, so unrelated registry work serialized behind a walk that never touched the file. The walk now runs unlocked; the lock is taken only when an entry is provably gone, and the prune is applied to a snapshot re-read inside it, so a registration that lands during the walk is no longer clobbered by a stale write. Same shape as the `adoptFlatBranchLabel` split. --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> |
||
|
Gergő Magyar
|
064832f50c
|
fix(mcp): resolve false FTS-missing warnings in the query tool (#2773)
* fix(mcp): surface resolved repo/branch/indexed-at in the FTS-degraded warning Turns the generic "FTS indexes missing" message into a diagnostic that reveals what this MCP session actually resolved, so a CLI/MCP mismatch or stale-connection theory is visible in the warning text itself instead of requiring a separate debugging round-trip (#2767). * fix(mcp): stop swallowing real FTS query errors behind the missing-index message queryFTSViaExecutor previously collapsed 'index genuinely missing' and 'a real query/connection error occurred' into the same silent null, so a real failure could masquerade as the generic FTS-degraded message with no diagnostic trail — even when it happened on only some of the per-table queries while others succeeded. Classifies the failure (mirroring queryFTS's own check for this exact cypher call), always logs a non-benign error server-side regardless of overall outcome, and surfaces it (redacted) in the client warning only when every table failed (#2767). * fix(mcp): give --repair-fts a dedicated freshness signal for warm readers --repair-fts intentionally never restamps indexedAt (it doesn't regenerate the graph), so a long-lived MCP session's pool staleness check had no explicit signal that a repair happened, only the incidental file-identity delta. Reuses the existing (forensic-only) capabilities.fts.status field: repair-fts now stamps just that sub-field (everything else byte-identical), and ensureInitialized compares it as a third, independent reinit trigger alongside the existing stamp/identity checks, seeded at cold init too so a fresh process's first warm check doesn't false-trigger (#2767). * test(mcp): warm session picks up an out-of-band --repair-fts rebuild (#2767) New end-to-end integration test: a real writable LadybugDB session builds an index WITHOUT FTS, a real LocalBackend observes 'FTS indexes missing' through the real pool, a separate writable session performs the exact repair-fts writes (real createSearchFTSIndexes + the #2767 capability-only meta stamp), and the SAME still-warm backend re-queries successfully without a restart — closing the one end-to-end gap no existing test covered. Running this against the real engine surfaced a second real message shape for a missing FTS index ("doesn't have an index with name X", not just "does not exist") that the U2 classifier didn't recognize — fixed classifyFtsQueryError to match both, with a regression test pinning the exact observed string. * fix(review): address code-review findings on the #2767 FTS fix - Anchor classifyFtsQueryError to the exception class (mirroring isBenignDropFtsIndexError) instead of a bare substring search, so a real, differently-classed error that happens to echo the benign phrase in its body (e.g. an echoed user query) can't be misclassified as a benign missing-index (adversarial review). - Re-read the on-disk meta immediately before the --repair-fts capability stamp write instead of reusing the pre-rebuild snapshot, so a concurrent writer (e.g. the HTTP server's background embedding checkpoint job) landing mid-repair isn't silently reverted. - Surface a client-facing partial-result warning (mirroring the existing enrichmentDegraded convention) when some FTS tables succeed but at least one hits a real error, instead of only logging it server-side. - Update RepoMeta.capabilities' stale 'no programmatic readers' docstring now that ensureInitialized reads capabilities.fts.status. - Widen the warm-session integration test's polling deadline for more margin over the production 5s staleness-check throttle. * fix(ci): drop the cold-init loadMeta call ensureInitialized never needed It stole the mocked loadMeta call an unrelated upstream PDG test depends on (test/integration/impact-pdg-statement-precise.test.ts queues a single mockResolvedValueOnce for its own PDG-config read; the extra call consumed that slot before the PDG code ran, so it fell through to the mock's null default and epistemic came back undefined instead of 'pdg-intra-procedural'). Cold init now leaves lastObservedFtsStatus unseeded — the cost is at most one redundant initLbug call on the first warm check, which no-ops via a single fs.stat when nothing actually changed, not a real reopen. * fix(review): address tri-review findings on the #2767 FTS fix Fixes two P1s (misleading repair-fts advice on real query errors; embedding-checkpoint job silently reverting the capabilities.fts stamp for up to its 30-minute lifetime), five P2/P3s (stale indexedAt in warnings, extension-unavailable noise, mismatched log severity, a table-missing vs index-missing conflation confirmed against a live LadybugDB, and a reinit-watermark latching bug), and the four residual items already self-disclosed in this PR's description (shared FTS error classifier, consolidated per-pool observed-state map, a redactPaths whitespace gap, and an isolated ftsCapsChanged test). A /simplify pass afterward caught one more real bug: the extension-unavailable short-circuit only guarded the MCP pool path, so the CLI-path fix above it started surfacing spurious non-benign errors for the same expected degraded state the pool path stays silent on — now both paths agree. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
azizur100389
|
84f584449d
|
fix(python): resolve classes through module imports (#2770) | ||
|
dependabot[bot]
|
51095c19f8
|
chore(deps): bump actions/setup-python from 6.3.0 to 7.0.0 (#2757)
Some checks are pending
Gitleaks / gitleaks (push) Waiting to run
CodeQL / Analyze (javascript-typescript) (push) Waiting to run
CodeQL / Analyze (python) (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
Bumps [actions/setup-python](https://github.com/actions/setup-python) from 6.3.0 to 7.0.0.
- [Release notes](https://github.com/actions/setup-python/releases)
- [Commits](
|
||
|
azizur100389
|
454d383416
|
fix(ingestion): join multi-line closure bindings on initializer startLine (#2735) (#2762)
* fix(ingestion): join multi-line closure bindings on initializer startLine Graph-node captures sit on the outer binding wrapper while scope-resolution anchors on the inner callable; the line-only position join missed when those split across lines and fail-closed dropped the real CALLS edge (#2735). * fix(ingestion): unwrap Ruby call+block for multi-line lambda joins Cover Kotlin/Ruby/Dart multi-line closure CALLS in integration tests, and dig Ruby's call/block field so do-end bindings join on the block start line. * style(ingestion): format closure join changes * fix(ingestion): make closure position join language agnostic |
||
|
dependabot[bot]
|
d99d828a52
|
chore(deps)(deps): bump express-rate-limit in /gitnexus (#2764)
Bumps [express-rate-limit](https://github.com/express-rate-limit/express-rate-limit) from 8.6.0 to 8.6.1. - [Release notes](https://github.com/express-rate-limit/express-rate-limit/releases) - [Commits](https://github.com/express-rate-limit/express-rate-limit/compare/v8.6.0...v8.6.1) --- updated-dependencies: - dependency-name: express-rate-limit dependency-version: 8.6.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> |
||
|
dependabot[bot]
|
e0dc0c2d5e
|
chore(deps): bump release-drafter/release-drafter from 7.5.1 to 7.6.0 (#2756)
Bumps [release-drafter/release-drafter](https://github.com/release-drafter/release-drafter) from 7.5.1 to 7.6.0.
- [Release notes](https://github.com/release-drafter/release-drafter/releases)
- [Commits](
|
||
|
dependabot[bot]
|
909f2f85b6
|
chore(deps): bump the codeql-action group across 1 directory with 3 updates (#2755)
Bumps the codeql-action group with 3 updates in the / directory: [github/codeql-action/init](https://github.com/github/codeql-action), [github/codeql-action/analyze](https://github.com/github/codeql-action) and [github/codeql-action/upload-sarif](https://github.com/github/codeql-action). Updates `github/codeql-action/init` from 4.37.0 to 4.37.3 - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits]( |
||
|
dependabot[bot]
|
e8e572fbff
|
chore(deps)(deps): bump i18next from 26.3.0 to 26.3.6 in /gitnexus-web (#2751)
Bumps [i18next](https://github.com/i18next/i18next) from 26.3.0 to 26.3.6. - [Release notes](https://github.com/i18next/i18next/releases) - [Changelog](https://github.com/i18next/i18next/blob/master/CHANGELOG.md) - [Commits](https://github.com/i18next/i18next/compare/v26.3.0...v26.3.6) --- updated-dependencies: - dependency-name: i18next dependency-version: 26.3.6 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.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> |
||
|
dependabot[bot]
|
5ed9617ff4
|
chore(deps)(deps): bump @langchain/core in /gitnexus-web (#2749)
Bumps [@langchain/core](https://github.com/langchain-ai/langchainjs) from 1.2.2 to 1.2.3. - [Release notes](https://github.com/langchain-ai/langchainjs/releases) - [Commits](https://github.com/langchain-ai/langchainjs/compare/@langchain/core@1.2.2...@langchain/core@1.2.3) --- updated-dependencies: - dependency-name: "@langchain/core" dependency-version: 1.2.3 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: Abhigyan Patwari <126312502+abhigyanpatwari@users.noreply.github.com> |
||
|
dependabot[bot]
|
c1ee62854a
|
chore(deps)(deps-dev): bump @babel/types in /gitnexus-web (#2750)
Bumps [@babel/types](https://github.com/babel/babel/tree/HEAD/packages/babel-types) from 8.0.0 to 8.0.4. - [Release notes](https://github.com/babel/babel/releases) - [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md) - [Commits](https://github.com/babel/babel/commits/v8.0.4/packages/babel-types) --- updated-dependencies: - dependency-name: "@babel/types" dependency-version: 8.0.4 dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> |
||
|
dependabot[bot]
|
59ea1ce2c8
|
chore(deps)(deps-dev): bump @types/node in /gitnexus (#2763)
Bumps [@types/node](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/node) from 26.1.1 to 26.1.2. - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/node) --- updated-dependencies: - dependency-name: "@types/node" dependency-version: 26.1.2 dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
dependabot[bot]
|
c0f2eb594e
|
chore(deps)(deps-dev): bump @vitejs/plugin-react in /gitnexus-web (#2753)
Bumps [@vitejs/plugin-react](https://github.com/vitejs/vite-plugin-react/tree/HEAD/packages/plugin-react) from 6.0.2 to 6.0.4. - [Release notes](https://github.com/vitejs/vite-plugin-react/releases) - [Changelog](https://github.com/vitejs/vite-plugin-react/blob/main/packages/plugin-react/CHANGELOG.md) - [Commits](https://github.com/vitejs/vite-plugin-react/commits/plugin-react@6.0.4/packages/plugin-react) --- updated-dependencies: - dependency-name: "@vitejs/plugin-react" dependency-version: 6.0.4 dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
dependabot[bot]
|
7890798192
|
chore(deps)(deps-dev): bump @types/node in /gitnexus-web (#2752)
Bumps [@types/node](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/node) from 25.9.5 to 26.0.1. - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/node) --- updated-dependencies: - dependency-name: "@types/node" dependency-version: 26.0.1 dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
Gergő Magyar
|
27ab37c432
|
feat(resolution): type receiver chains from AST structure across all 14 languages (#2708) + epistemic lower-bound (#2744) (#2747) | ||
|
Gergő Magyar
|
9c24e3459e
|
fix(rust): qualify items by their enclosing mod chain so same-named ones stay distinct (#2742) (#2745)
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
* fix(rust): let the qualified-call filter see inline modules The negative filter added in #2741 builds its set of known module names from FILE PATHS, so an inline `mod x { … }` — which appears in no path — was absent from it. Every module-qualified call into an inline module was therefore rejected before any candidate channel ran, which is a hole in that optimisation rather than in the resolution logic it guards. The per-pass index now unions the file-derived names with inline module names taken from the scope model: a `mod` declaration binds a `Namespace` def locally in the declaring scope, and that binding is the only place an inline module's name exists. Collected in the same walk that already builds the module → scope map, so it costs no extra pass. Found while fixing #2742, where a correctly resolved call into `mod inner { … }` still could not reach its target. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J5nWP4BFeBSjbQ3uqAjtxV * fix(scope-resolution): try the namespace-prefixed node key before the bare one `resolveDefGraphId` looked up the plain `qualifiedName` key first and only retried with the `namespacePrefix`-qualified key afterwards. For the defs that carry a prefix the qualified name is a bare TAIL, so the plain key happily matched a same-named item at a different namespace depth in the same file and returned it before the more specific retry was ever reached. The namespace-prefixed key is strictly the more specific of the two, so it is now tried first. Where no such node exists the lookup falls through to exactly the previous order, which keeps the #1982 behaviour this retry was added for. Without this, a call into `mod inner { fn dispatch }` resolved to the correct definition and then mapped it onto the crate-root `fn dispatch` node — the self-loop #2742 describes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J5nWP4BFeBSjbQ3uqAjtxV * fix(rust): qualify items by their enclosing mod chain so same-named ones stay distinct (#2742) Node identity is `<label>:<file>:<qualifiedName>` and carried no module path, so an inline `mod inner { fn dispatch }` and a crate-root `fn dispatch` in the same file collapsed onto `Function:<file>:dispatch`, first-wins. Resolution already picked the right definition — the target simply was not representable, so a correct resolution still rendered as a self-loop and `impact` reported the real callee as unreached. The mechanism already existed: `qualifyRustImplTargetByModScope` has walked `mod_item` ancestors for impl targets since #1982. Generalised to `qualifyByEnclosingModScope` and applied to free items, so `mod inner { fn dispatch }` becomes `Function:<file>:inner.dispatch`. Keyed purely on the `mod_item` node type, exactly as the impl qualifier already was, so it is a no-op for every language whose grammar has no such node. Two constraints found by tests rather than by reading, both now encoded: - The helper normalised `::` to `.` unconditionally. With no enclosing `mod` that rewrote a top-level `impl a::Inner` from `a::Inner` to `a.Inner` and moved its node id away from the one the HAS_METHOD owner edge emits, breaking the #1975 scoped-impl ownership. It now returns raw text untouched when there are no mod segments, which also makes the change strictly additive for every id that has no enclosing module. - Qualification is scoped to items with no enclosing class/impl. A method already carries its owner's name, and that owner's id is mod-scoped by the impl qualifier, so qualifying the method again breaks the same byte-for-byte agreement. Same-named methods on same-named types in sibling modules therefore still collapse — a narrower residual than the free-item case fixed here, and one belonging to the owner edge rather than to this path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J5nWP4BFeBSjbQ3uqAjtxV * fix(storage): bump schema versions for the mod-qualified Rust node ids (#2742) `INCREMENTAL_SCHEMA_VERSION` 24 -> 25 and `SCHEMA_BUMP` 31 -> 32. Node ids change for every Rust item inside any `mod` block, and `#[cfg(test)] mod tests` makes that close to every Rust repository. A pre-v25 index therefore holds ids an incremental top-up cannot reconcile — the old nodes would simply be stranded — so the reuse gate has to force a full re-analyze. The qualified name is computed in the parse worker, so a warm parse cache would likewise replay the old unqualified ids and keep the collapse. This branch originally claimed v24; #2708 took that number and merged first, so it is renumbered to v25 here. That is exactly the collision the v29 note in parse-cache.ts warns about, and re-checking against origin/main at rebase time rather than at branch time is what caught it. #2708 did not touch `SCHEMA_BUMP`, so 32 is free. The version-pin test moves with the bump by design, including the new pre-v25 row in the reuse-gate table. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J5nWP4BFeBSjbQ3uqAjtxV * fix(rust): stop mod-qualifying container ids while their owner edges stay bare (#2745 review) #2742 re-keyed Rust node ids by the enclosing `mod` chain. The mint moved; the owner-edge anchor did not. `findEnclosingClassInfo` mints a member's owner id from the container's BARE `nameNode.text` and only follows a qualified shape when the provider sets `classExtractor.qualifiedNodeId`, which Rust does not. So every `struct` / `trait` / `enum` / `impl` declared directly inside a `mod` got a node id that none of its member edges pointed at. Five lines of idiomatic Rust were enough: pub mod engine { pub struct Config { pub retries: usize } } NODE Struct:src/lib.rs:engine.Config DANGLING HAS_PROPERTY Struct:src/lib.rs:Config -> Property:src/lib.rs:Config.retries The rows are discarded by the IGNORE_ERRORS COPY retry, so the struct silently lost every field. A trait impl inside a `mod` additionally dropped its METHOD_IMPLEMENTS edge outright. The same gap put `impl a::Inner` inside a `mod` back on the #1975 rake that `qualifyByEnclosingModScope`'s own docblock warns about. The impl-target branch deliberately fires only for an UNSCOPED `type_identifier`; the new gate had no such restriction and picked up the scoped targets that branch had just excluded, minting `Impl:<file>:outer.a.Inner` against an anchor still reading `Impl:<file>🅰️:Inner`. The member side was already excluded via `!enclosingClassInfo`. This adds the owner side, gated on `MEMBER_OWNER_NODE_TYPES` — derived from `CLASS_CONTAINER_TYPES`, which is already the single source of "this node type owns member edges" and already carries an INVARIANT note binding it to `CONTAINER_TYPE_TO_LABEL`. A language adding a container therefore cannot gain a mismatched id shape here without also failing that invariant. Keyed purely on tree-sitter node types, so no language name enters shared ingestion. `union_item` is listed too: its fields are captured as Property but it is not a recognized owner, so they carry no HAS_PROPERTY edge and cannot dangle — it is here so a union's id keeps the same shape as the struct beside it. Containers still collapse across sibling modules, exactly as before this fix. That residual belongs to the owner edge, and is not worked around here. Regression tests use the UNFILTERED `findDanglingEdges(result)`. Every other dangling assertion in `rust.test.ts` passes `['HAS_METHOD']`, which is precisely why the HAS_PROPERTY breakage shipped with a green suite. They assert the NODE id rather than only the edge's anchor, because the anchor was already bare while the bug was live — an edge-only assertion passes in both builds. All four fail when the new gate clause alone is reverted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019GvVxWt1ShhEP8CiMj3b6Z * fix(rust): resolve modules nested inside an inline mod (#2745 review) The #2730 self-loop survived one `mod` deeper: pub mod outer { pub mod tools { pub fn dispatch() {} } pub fn dispatch() { tools::dispatch(); } } CALLS outer.dispatch -> outer.dispatch <- the #2730 symptom NODE outer.tools.dispatch <- correct target, unlinked Two gates were blind to a nested inline module, so the hook refused and the shared lexical tier bound the call to the enclosing same-name `dispatch`: `knownModuleNames` was collected by walking `moduleScopeByFile`, which maps a file to its ROOT `Module` scope only. A `mod` nested inside an inline `mod` binds in the parent module's scope, so the walk saw depth-1 inline modules and missed every nested one — `tools` never entered the set and the negative filter rejected the qualifier before any candidate ran. `declaresSubmodule` had the same root-only assumption, so even with the name known the candidate `outer::tools` was never yielded. Both now read the def index. Names come from every `Namespace` def; inline module PATHS are derived from the members' `namespacePrefix` rather than from the `mod` defs, because a `mod` def carries no nesting information of its own — inside `mod outer { mod tools { … } }` the inner def is `qualifiedName: 'tools'` with NO `namespacePrefix`, while every def within it is stamped `outer.tools`. A `Namespace` scope also owns its OWN def rather than its children's, so the scope tree cannot answer this either: the `mod outer` scope lists `outer`, never `tools`. Restricted to non-empty prefixes, so this stays a DECLARATION check. Including file-derived modules would let an undeclared or `cfg`-gated file on disk outrank a real `use` binding — the regression #2741's review already fixed once. File-backed submodules therefore keep going through the binding check. A module with no defs at all is absent from the set, which is harmless: it has no member for a qualified call to resolve to. Cost is one pass over an already-resident def index, memoized per resolution pass on the existing WeakMap — the same order of work as the binding walk it replaces, and it subsumes it. `isLocalNamespaceBinding` was going to single-source the duplicated "locally declared submodule" predicate the review flagged; deriving paths from members removed the second copy outright instead. Regression fixture covers depth 2 and depth 3, so the fix is depth-agnostic rather than depth-2 special-cased. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019GvVxWt1ShhEP8CiMj3b6Z * fix(rust): let an imported type outrank a same-named module (#2745 review) Widening the negative filter to inline `mod` names let a type-qualified call through whenever a module happened to share the type's name. The crate-root-relative candidate then captured it: // src/lib.rs pub mod Buffer { pub fn with_capacity() -> usize { 111 } } // src/b.rs use crate::c::Buffer; // the real target lives in c.rs pub fn call() -> usize { Buffer::with_capacity() } base: (no CALLS edge — unresolved) PR: CALLS b::call -> Function:src/lib.rs:Buffer.with_capacity <- fabricated `ids.ts` states the doctrine this broke: a missing edge is the correct failure direction for a graph whose consumers include `impact`; a fabricated caller is not. The base produced the missing edge and the PR produced the fabricated one. That third candidate is the loosest of the three — a guess at a crate-root-relative path the caller never wrote, kept for 2015-edition style. In Rust 2018 a bare first segment resolves in the CALLER's module, so a local binding for that segment settles the question: it is now skipped when the head names anything non-module in the caller's own module. Candidates 1 and 2 are untouched, and they run first, so the legitimate `use crate::tools;` path is unaffected. The binding lookup goes through `lookupBindingsAt`. A first attempt read `Scope.bindings` directly and the guard never fired: a `use` binding is finalize OUTPUT and absent from the scope's own local table, which is exactly the imported-type case being guarded. Contract I8 in `contract/scope-resolver.ts` requires that channel anyway. The regression test asserts the forbidden TARGET rather than an empty edge set, and separately asserts the module member still exists as a node — otherwise the test would pass just as well if the call went unresolved for some unrelated reason, or if the module node disappeared entirely. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019GvVxWt1ShhEP8CiMj3b6Z * fix(scope-resolution): try the namespace-prefixed name on the TAGGED keys too (#2745 review) `resolveDefGraphId` gained a namespace-prefixed retry for the plain qualified key in this PR, but the five tagged keys above it — template constraints, parameter types, parameter shape, arity, template arguments — kept composing from the bare `qualifiedName`. For a namespace- or `mod`-qualified def those keys are simply dead: `node-lookup.ts` registers them under the QUALIFIED name (`inner.dispatch#0`) while this side built `dispatch#0`. The keys exist to separate overloads, so a mod-scoped overload set was relying on whichever later key happened to catch it. Verified as a miss rather than a mis-hit before changing anything — an end-to-end run with a crate-root decoy of the same name and arity binds correctly — so this is hygiene, not a live bug. Worth doing while the code is open rather than leaving five keys dead and the behaviour dependent on fallback order. Both name forms now go through one `lookupTagged` helper, most specific first, so a sixth tagged key cannot be added with the bare form only. That also removes the five hand-repeated `qualifiedKey(...)` / `nodeLookup.get(...)` pairs. Also pins the C++ `EXTENDS` retarget this PR's reorder produces. `cpp-two-phase-dependent-base-cross-ns-deep` declares a global `Inner` decoy alongside `ns:🅰️🅱️:Inner`; the base's `qualifiedName` is a bare `Inner` with the path on `namespacePrefix`, so only the prefixed key separates them, and only if it runs first. The improvement was riding unasserted in a Rust-scoped PR. The captures golden covers every `rust-*` fixture, so the three fixtures added by this review series drift it; regenerated with UPDATE_GOLDEN=1. Verified: 785 tests across cpp / csharp / rust resolvers and the callable-id-lockstep unit test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019GvVxWt1ShhEP8CiMj3b6Z * fix(rust): keep a mod declared inside a fn from hoisting above the callable (#2745 review) `fn wrapper() { mod helper { fn dispatch } }` minted `Function:<file>:helper.wrapper.dispatch@2:8` — the mod segment composed OUTSIDE the enclosing-callable prefix, inverting the real nesting. Nothing dangled: the `@line:col` suffix already makes a function-local callable's id unique, which is also why the mod segment adds no identity in this position. The path simply read as a lie about the source. It is now skipped rather than reordered — interleaving two qualifier passes to fix the order would be real machinery for a shape whose ids are already unique. Also folds in the three documentation and structure findings from the same review: - The 4-clause gate is extracted to a named `qualifiesByEnclosingModScope`, matching the two conditions directly above it in the same function, which were already named consts. - `qualifyByEnclosingModScope`'s docblock documented only the impl-target contract even though the generalized name has had a second, looser caller since #2742. It now states both, and says which gate belongs to which — that gap is what let the #1975 scoped-impl regression through in the first place. - The "cheap rejection BEFORE any index work" comment was no longer true: `passIndexFor` walks the def index on its first call in a pass. Corrected rather than left to mislead the next reader into thinking the filter is free. What it still buys — skipping the per-site candidate search, the part that scales with the workspace — is stated instead. Verified: 279 tests across the Rust resolver suite and the Rust scope-resolution unit tests. Captures golden regenerated for the extended fixture. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019GvVxWt1ShhEP8CiMj3b6Z * test(storage): move main's SCHEMA_BUMP pin to 32 for the mod-qualified ids (#2745 review) `#2736` added a pin asserting `SCHEMA_BUMP === 31` on main, which arrived on this branch through the merge of main while `0062a5c2` had already bumped the constant to 32. Neither side conflicted textually — the pin and the constant live in different files — so the merge was clean and the test failed instead. That is the pin working as designed: it exists so a bump cannot ride along unnoticed, and this is the fifth time a SCHEMA_BUMP collision has been caught by a guard rather than by review. Updated to 32 with the reason recorded inline. `INCREMENTAL_SCHEMA_VERSION` needs no second bump: 25 was introduced by this unmerged branch, so no released index carries it, and its own pin in `call-summary-schema-version.test.ts` is already consistent. Verified: 119 tests across the parse-cache, schema-version, incremental-orchestration and the two identity suites that arrived with the merge. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019GvVxWt1ShhEP8CiMj3b6Z * test(bench): rebaseline the Rust capture fingerprint for the three new fixtures (#2745 review) CI caught what I missed: [scope-capture --check] FAIL: rust: capture fingerprint drift (got 05acbaca..., expected 90fda086...) fixture_count 202 `bench/scope-capture` fingerprints the whole `rust-*` fixture corpus, so the three fixtures added by this review series drift it. I rebaselined the `rust-captures-golden` snapshot and stopped there — a new fixture is a call site of BOTH, and updating only one is how this reached CI red. This is the same class as PR #2743's headline finding, from the other direction: an id-shape change makes every synthetic corpus a call site, and the author fixed the unit-test fixture and missed the bench. Here it is a fixture-count change rather than an id-shape change, and the review that flagged the #2743 lead as "REFUTED, bench/ has no Rust node-id corpus" was right about node ids and wrong about the corpus fingerprint. Noted for the next author in the baseline entry itself. Verified as pure corpus growth rather than a capture-logic shift: removing ONLY the three new fixture directories and re-running reproduces the prior fingerprint exactly (196 fixtures, capture_groups_fp 3432), and restoring them gives the new one (202, 3556). `emitRustScopeCaptures` is untouched by this series. Scaling 1.022 local / 1.057 CI, well inside the 1.5 budget. `bench/python-scope` globs `python-*` only and is unaffected; no other bench walks the Rust corpus. `--check` now PASSes for all 15 languages. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019GvVxWt1ShhEP8CiMj3b6Z --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
azizur100389
|
e723f3c2ee
|
fix(scope-resolution): parse def coordinates after file paths (#2743)
* fix(scope-resolution): parse def coordinates after file paths Anchor coordinate parsing to the known file path so coordinate-like path fragments and private symbol names cannot corrupt closure attribution. * fix(bench): use production definition ids |