GitNexus

mirror of https://github.com/abhigyanpatwari/GitNexus.git synced 2026-08-28 05:25:25 +00:00

Author	SHA1	Message	Date
azizur100389	1fa751d76d	fix(spring): extract method-level RequestMapping routes (#2857 ) * fix(spring): extract RequestMapping route methods Co-authored-by: Cursor <cursoragent@cursor.com> * fix(spring): address RequestMapping review findings * fix(spring): accept trivia in request methods --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>	2026-08-07 11:43:19 +01:00
Gergő Magyar	e69d3c49c4	fix(analyze): gate FTS-indexed DML before the incremental writeback (#2841 ) (#2854 ) * fix(lbug): never report a drop that could not happen, and gate FTS-indexed DML `CALL DROP_FTS_INDEX` is itself an FTS-extension function, so with the extension unloaded it fails with `Catalog exception: function DROP_FTS_INDEX is not defined`. `isBenignDropFtsIndexError` classifies that as "nothing to drop" — correct when the index does not exist, wrong when it does: the drop silently no-ops and the next write to that table dies at bind time with an engine message that never mentions FTS (#2841). The classifier stays pure (a message cannot tell you whether an index is live). Instead `dropFTSIndex` settles liveness with a catalog read on the ERROR path only and raises an FTS-named, remedy-bearing error when the index is present but undroppable. Adds `ensureFtsRowDmlSafe`, the FTS twin of `ensureEmbeddingRowDmlSafe` (#2623): catalog first, load FTS with the analyze policy only when an index actually gates DML. LadybugDB refuses that DML at BIND time — a DETACH DELETE matching zero rows fails exactly as hard as one matching thousands — and the indexes cannot be cleared in place, so a verdict is the only useful answer. Both gates now share one `SHOW_INDEXES` read via `readIndexCatalogRows`, so adding the FTS check costs no extra catalog round-trip. Refs #2841 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CxokkpfssUCxvBwRZMtRCB * fix(analyze): escalate instead of crashing when FTS blocks incremental DML The incremental writeback decided its write plan without ever asking whether row-level DML was legal. On a DB carrying FTS indexes with an unloadable FTS extension, `deleteNodesForFiles` then died mid-writeback: Binder exception: Trying to delete from an index on table File but its extension is not loaded. with no mention of FTS anywhere in the run — the only install-capable load happened in Phase 3, long after the writes (#2841). The incremental branch now reads the index catalog once and derives both extension verdicts before any DML. When FTS (or VECTOR) blocks in-place writes, the run falls through to the existing wipe-and-bulk-COPY escalation — the same answer #2623 gave for VECTOR, and the only one available, since the indexes cannot be dropped without the extension. Every blocked extension is named in the reason log, not just the first one checked: a DB can carry both a vector index and FTS indexes, and reporting half the cause is how this failure stayed mis-diagnosed. Refs #2841 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CxokkpfssUCxvBwRZMtRCB * test(analyze): cover the FTS DML gate, both-blocked escalation, and the drop guard New `incremental-index-extension-dml-gate.test.ts` drives the real `runFullAnalysis` against a real mini-repo and a real LadybugDB: - a DB carrying FTS indexes with FTS made unloadable escalates to a full DB write, names FTS in the log, ends with zero FTS indexes, and still has the newly committed content in the graph (pre-fix: Binder exception, exit 1); - FTS available keeps the surgical plan and the indexes; - a DB that never carried FTS indexes is not escalated (the catalog-first check must not tax FTS-less machines); - FTS and VECTOR both blocked produce ONE escalation naming both. `drop-fts-index-error-classification.test.ts` gains the two `dropFTSIndex` cases the #2841 guard turns on: live index + unloaded extension rejects with an FTS-named error, absent index still resolves. The existing classifier assertions are unchanged — it stays pure. The CLI e2e reproduces the reporter's exact journey (analyze with the extension, remove it, touch a file, analyze again) and asserts exit 0 plus an FTS-named reason. It skips visibly when the seeded extension cannot load on the host, so it can never report a false red about the fix. Mutation-verified: reverting the run-analyze gate fails the first scenario; reverting the dropFTSIndex guard fails the live-index case. Refs #2841 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CxokkpfssUCxvBwRZMtRCB * fix(lbug): make every catalog-gated path fail closed, and classify the drop remedy Review findings on #2854 (two-engine, 17 lanes). H3 — `ftsIndexExistsInCatalog` returned `false` when the catalog could not be read, i.e. "index absent", so `dropFTSIndex` swallowed the error and the caller proceeded as if the index were gone. That is the #2841 symptom the guard exists to make loud, and it contradicted the contract `readIndexCatalogRows` states two functions above. It now fails closed. §6.A — `ensureFtsRowDmlSafe` keyed on `index_type === 'FTS'`, which answers `undefined === 'FTS'` → false → no gate for a row whose shape cannot be read: fail-open, in the gate whose only job is preventing an unsafe write, while the VECTOR twin fails closed on the same input. Now only a positively-identified non-FTS index is waved through. Deliberately NOT the twin's `!== 'HASH'`: that is safe there only because it is scoped to the embedding table first, and this gate is table-agnostic — `!== 'HASH'` would let the HNSW index gate FTS DML. §5.A — `undefined` was overloaded: "caller passed nothing" and "caller tried and could not prove anything" shared one value, so a failed shared read silently became three reads and the two gates could decide from different snapshots. The failed snapshot is now representable (`INDEX_CATALOG_UNREADABLE`), leaving one unambiguous `??` in `resolveGateRows`. §5.B — both gates regained the unconditional null-connection precondition the refactor moved into the reader. §5.G — the throw's remedy now routes through `diagnoseExtensionLoad`, like `--repair-fts` and `ftsDegradedWarning`, so a missing runtime dependency is not told to reinstall. The message stays path-free (#2374/#2375). The dead positional row fallbacks are kept and marked `LADYBUGDB-CONTRACT`: removing them would turn a proven-inert hedge into a fail-open gate if a future engine returns unnamed tuples. Refs #2841 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CxokkpfssUCxvBwRZMtRCB * fix(analyze): never undo an explicit wipe, stage extension-forced rebuilds, report honestly Review findings on #2854 (two-engine, 17 lanes). H1 (P1, both engines) — `analyze --drop-embeddings` was silently reverted. The `--drop-embeddings` → `force` conversion sits inside the `embeddingCheckpoint` branch, so without a checkpoint the run stays incremental and reaches the gate; the flag then deliberately leaves `cachedEmbeddings` empty, which is exactly the rescue's trigger, so every row the operator asked to destroy was read back and restored, exit 0. Widening the rescue from `!embeddingRowDmlSafe` to `extensionForcedRebuild` moved that latent bug onto the dominant path, because every analyzed DB carries FTS indexes. Guarded on the flag itself — NOT on `shouldLoadCache`, which is false in the meta-under-reports case the rescue exists for and would have deleted the safeguard while fixing the wipe. The `--drop-embeddings --embeddings` variant is covered by the same guard. H2 — an extension-forced escalation wiped the LIVE index in place: `buildPath` was frozen ~440 lines earlier while the run was still classified incremental, so an interrupt or ENOSPC left no complete index, where main failed at bind time with it intact. Extension-forced rebuilds now build into a staging file and publish via the existing atomic swap; size-forced ones stay in place, since that trigger is the repo's own churn rather than a machine condition. H5 — the escalation log asserted a vector index "exists" and that the store "carries FTS indexes" in exactly the case the catalog read proved nothing, while the only truthful signal went to stderr rather than the IPC log. It now emits a distinct unreadable-catalog cause, and "this index carries" (which pointed at the vector index just named) reads "the graph store carries". §5.D — the write-set cause was dropped whenever an extension cause co-occurred; causes are appended now, not selected between. §5.C — after an FTS-forced rebuild stamped lastCommit, a plain rerun on the same commit hit the alreadyUpToDate fast path before Phase 3, so the CLI's "install … then rerun" advice could never restore FTS. The fast path is now bypassed when meta records FTS unavailable and the extension can load again, keyed on the persisted capabilities stamp rather than new state. §5.F (skip the escalation for a zero-change commit) is deliberately NOT implemented: `deleteSpringAutoConfigurationSyntheticClasses` and `deleteSpringAopEvidenceNodes` run unconditionally on the surgical branch and bind against FTS-indexed `Class`/`CodeElement`, and a zero-row DETACH DELETE fails at bind time exactly as hard as a large one — so the skip would restore the original crash. Refs #2841 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CxokkpfssUCxvBwRZMtRCB * perf(search): read the index catalog once per drop sweep, and state the real contract Review findings on #2854. H4 — on a machine where FTS cannot load and the DB carries no FTS index, the gate correctly returned early without loading the extension, but the surgical path still ran the full 20-entry drop sweep: every `CALL DROP_FTS_INDEX` raised "function DROP_FTS_INDEX is not defined", and the new liveness guard then fired a fresh catalog read per table — 20 reads every run, forever, for exactly the offline/load-only population, contradicting the "healthy path costs nothing" claim shipped with the guard. The sweep now reads the catalog once and skips entirely when no FTS-typed index exists. An unreadable catalog runs the sweep, so an unprovable catalog never skips real work. H8 — the docstring still promised `dropFTSIndex` "tolerates" an unloadable extension. Post-#2854 a live index plus an unloadable extension throws, and safety rests on caller ordering discipline rather than the type system — which is what would have talked the next caller out of that ordering. GUARDRAILS — the "switching to a full DB write" sign described exactly one trigger (write set >~50%). Since #2623 and #2841 an unloadable extension escalates regardless of write-set size; documented with its recovery steps. Refs #2841 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CxokkpfssUCxvBwRZMtRCB * test(analyze): cover the wipe guard, the staged rebuild, and the fail-closed branches Review findings on #2854. H1/H2 mutation-verified: removing `!options.dropEmbeddings` fails the new drop-embeddings case ("expected true to be false"); disabling the staging upgrade fails the staging case ("expected 0 to be greater than 0"), so both assert behaviour rather than describe it. Gate suite (7 cases): `--drop-embeddings` under an FTS-forced escalation ends at zero embedding rows and logs no "Preserving"; the escalation is one-shot — a third run on a healthy host returns to surgery and rebuilds every FTS index; an extension-forced rebuild is observed building into `lbug.staging.` and leaves none behind; the rescue complement still preserves un-stamped rows when no wipe was requested; the never-built case now asserts the commit reached the graph. H6 — the both-blocked case hard-asserted `createVectorIndex()` while the suite probed FTS only, so it went red on any FTS-yes/VECTOR-no host. VECTOR is probed now and gates only that case, with a GITNEXUS_REQUIRE_VECTOR hard-fail. H7 — the fail-closed branches had no coverage although the VECTOR twin's test and interception technique were ready to copy: `ensureFtsRowDmlSafe` under an unreadable catalog now proves it routes to the load, and `dropFTSIndex` proves it rejects rather than silently tolerating. Plus a redaction case that forces a real path-bearing load failure — under policy `never` the assertion would have been vacuous, since that reason carries no path. §5.E/§6.B — the suite is registered in the cross-platform matrix (its sibling was; it wasn't, and GITNEXUS_REQUIRE_VECTOR is set only on that job) and moved into the sequential lbug-db project per TESTING.md:68, verified not to drop it from the sharded ubuntu job. A Windows shard weight is added as a labelled estimate — the 8s floor would skew the split it exists to protect. Refs #2841 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CxokkpfssUCxvBwRZMtRCB refactor(analyze): make the FTS gate's fast path cheap, its claims provable, and its remedies classified Cleanup review of the #2841 work (four parallel angles: reuse, simplification, efficiency, altitude). Behaviour-preserving except where the previous behaviour was wrong. Correctness the review caught: - The fast-path probe keyed on `capabilities.fts.status === 'unavailable'`, which collapses "extension unavailable" and "index build failed". A deterministic build failure (an un-tokenizable row, #2544) therefore bypassed `alreadyUpToDate` on EVERY subsequent run, re-analyzed the whole repo, failed the same way, and restamped — a permanent loop where the run used to be one `stat`. Phase 3 already computes the discriminator; it is now persisted as `fts.skipReason` and the probe only runs for `extension-unavailable`. Metas written before this carry no field and keep today's behaviour. - `dropSearchFTSIndexes` skipped its sweep when no row read `index_type === 'FTS'`, while `ensureFtsRowDmlSafe` treats an unreadable type as "might be FTS". Opposite polarity, under a comment claiming they matched: a row-shape change would let the gate wave the surgical plan through while the sweep dropped nothing, putting DELETEs back on tables carrying live FTS indexes — #2589 again. The sweep now decides per configured index on identity, which is also strictly more precise. Its old justification (leftover indexes under other names) was unreachable — the loop only ever drops configured entries. - `dropFTSIndex` threw "FTS index X on table Y exists" on the one path where the catalog could not be read — a fabricated claim, on a DB the same run had just shown carries no FTS index. Presence is now `present \| absent \| unverifiable` and the message says which. - The remedy was hand-written for three of the four load-failure classes, discarding `missingFileRemedy`/`corruptFileRemedy`, so a corrupt extension file was told to retry an install — the misdirection #2383 fixed. Both the drop error and the escalation log now use the classified remedy. Cost, measured on a 391 MB index (cold open ~1 s, SHOW_INDEXES ~4 ms): - The probe opened the live index WRITABLE on the millisecond fast path, dragging in schema DDL, the cross-process write lock, sidecar reclaim and a CHECKPOINT on close. It is read-only now. That also closes an install trap: `doInitLbug`'s pre-load resolves the env policy on the writable branch, so an operator following our own `GITNEXUS_LBUG_EXTENSION_INSTALL=auto` advice paid a forked 15 s installer on every up-to-date run (memoized per process; the CLI is a fresh process each time). The read-only branch pins `load-only`. - A failed staged rebuild orphaned a full index-sized copy until the next lock sweep; the failure path now reclaims it. - The sweep re-read a catalog the run already held, defeating the invariant the snapshot type exists to enforce. Structure: row-shape accessors have one home, so the LADYBUGDB-CONTRACT grep claim is true by construction; staging now applies to both escalation causes, since recoverability is a property of the wipe-then-COPY plan, not of the trigger; `getExtensionCapability`/`getFtsCapability` replace hand-spelled lookups where the seam allows. Two lookups in run-analyze.ts deliberately keep the exported `getExtensionCapabilities()` form: the #2383 tests stub that export, and an ESM module mock does not intercept a helper's internal call — routing through it silently degraded the classified remedy to generic text. Recorded in-comment. Not taken, deliberately: extracting the escalation message and replacing the snapshot protocol with a connection-scoped catalog memo (both sound, both restructure code this PR just stabilised — they belong in their own change); an extension registry (premature at two instances, and the FTS/VECTOR polarity difference is exactly what it would have to parameterize back out). Refs #2841 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CxokkpfssUCxvBwRZMtRCB * test(analyze): pin both sides of the degraded-FTS fast-path bypass `healDegradedFts` (§5.C) had zero coverage — three separate review angles flagged it, and the cleanup pass then found it sat one conjunct away from a permanent full-re-analyze loop. Both sides are pinned now: - it re-analyzes past `alreadyUpToDate` when the stored meta says FTS is degraded and the extension loads again: run 1 analyzes with loads blocked (asserting the precondition — `status: 'unavailable'`, `skipReason: 'extension-unavailable'` — rather than assuming it), then a same-commit clean-tree rerun rebuilds every FTS index without a file changing; - it stands down when the degradation was a BUILD failure: the stored `skipReason` is rewritten to 'build-failed' and the rerun must take the fast path, because that rebuild would fail identically on every run forever. The build-failed state is reached by rewriting the stamped discriminator, not by provoking a real tokenizer failure: a genuine one needs a stored row the native tokenizer rejects (#2544/#2546), which is neither portable across the CI matrix nor deterministic, and §5.C reads only that field. Also folds the first escalation case into the one-shot case. The claim that it was fully subsumed did not hold on audit: `logs` containing 'FTS' was unique as expected, but so was the duplicate-File-node row count — every other reader goes through a Map keyed by path, which collapses a stale twin an appending rebuild would leave. Both assertions moved rather than one being dropped. Net suite runtime goes UP (two cycles removed, four added), against the cross-platform-matrix argument that motivated the dedup — recorded here because the shard weight is an estimate pending a real Windows measurement. Refs #2841 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CxokkpfssUCxvBwRZMtRCB * test(search): keep the whole-module adapter mock in step with the row accessors The cleanup pass moved the LadybugDB row-shape reads behind named accessors so the column contract has one home. `fts-indexes.test.ts` mocks the entire adapter module with a hand-written factory, which still exposed only the three exports the file imported before — so `verifySearchFTSIndexes` failed with "No `indexRowName` export is defined on the mock" while production was fine. The added accessors mirror the real implementations rather than returning stubs. A stub would have read `undefined` out of every catalog row and let the suite pass for the wrong reason — the failure mode a whole-module mock invites whenever the module under test grows an import. Refs #2841 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CxokkpfssUCxvBwRZMtRCB * revert(analyze): drop the degraded-FTS auto-heal, fix the advice it existed to justify §5.C's complaint was that the CLI tells users to "install the extension … then rerun" when a rerun lands on the up-to-date fast path and rebuilds nothing. The answer shipped for it was a probe that bypasses that fast path. Four independent problems later, the sentence is cheaper to fix than to make true: - it could not tell "extension was missing" from "index build failed" without a stamped discriminator, so a deterministic build failure (#2544/#2546) re-analyzed the entire repo on every invocation, forever, where the run used to be one `stat`; - it opened the live index on the millisecond fast path — writable at first, dragging in DDL, the cross-process lock and a CHECKPOINT (~1 s on a 391 MB index), and even read-only it is a full open; - `doInitLbug`'s pre-load resolves the env policy, so an operator following our own `GITNEXUS_LBUG_EXTENSION_INSTALL=auto` advice paid a forked 15 s installer per up-to-date run; - and it turns the fast path into a full re-analysis whenever an index authored where FTS was unavailable is later read where it loads — a legitimate, common state, and the invariant `analyzer-identity-cli.test.ts` pins. So: no probe. The degraded-search warning now points at `gitnexus analyze --repair-fts`, which rebuilds the search indexes without re-parsing the repo, instead of "then rerun". One line, no new failure modes, and it is what the issue actually asked for. `capabilities.fts.skipReason` stays in the meta stamp: it costs three lines, makes the two degradation causes distinguishable for support, and is what any future correct answer here would key on. Also gates the H2 staging assertion on the production predicate. It asserted staging unconditionally while the upgrade requires `posixSwap \|\| windowsSwapOk`, and `windowsSwapOk` is opt-in (#2614) — so it failed on the Windows matrix for a reason unrelated to #2841. Registering this suite cross-platform is what exposed it; the assertion now mirrors the condition it is testing. Refs #2841 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CxokkpfssUCxvBwRZMtRCB * fix(analyze): never stage around a damaged index — escalate in place when the catalog is unreadable CI caught this on ubuntu and macOS: `analyze-wal-checkpoint-failure` stopped failing, which is worse than it sounds. That test plants a directory at `.gitnexus/lbug.wal.checkpoint` so the auto-checkpoint's rename target is blocked, and asserts analyze exits non-zero with the `--wal-checkpoint-threshold` hint. But LadybugDB cannot open that path at all, so `CALL SHOW_INDEXES()` now fails with `IO exception: … Is a directory`. The catalog read returns UNREADABLE, both DML gates correctly fail closed, both extension loads fail with the same IO error, and the run escalates — and since the escalation stages, it built a fresh index at `lbug.staging.<uuid>`, swapped it in, and exited 0. The blocked path was never touched. The run "succeeded" while the damage sat untouched on disk, waiting to break the next in-place writeback. So the staging upgrade is now conditional on the catalog having been READ. Staging exists to protect a healthy live index from a machine-level cause (an extension that will not load); it must not be used to route around a damaged one. When we are escalating out of ignorance, build in place so the underlying IO fault lands on the failure path where the operator gets a diagnosis. Verified against the real CLI, not just the suite: with a directory planted at the checkpoint path, analyze now exits 1 and prints `gitnexus analyze --wal-checkpoint-threshold 67108864`. The healthy extension-forced case still stages (gate suite 6/6). Refs #2841 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CxokkpfssUCxvBwRZMtRCB --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-07 09:44:44 +01:00
drdave	021ac30376	feat(cli): add a bunx lane so bun-only machines can run gitnexus (#2765 ) Some checks failed CodeQL / Analyze (javascript-typescript) (push) Waiting to run Details CodeQL / Analyze (python) (push) Waiting to run Details Gitleaks / gitleaks (push) Waiting to run Details Publish / Classify release event (push) Waiting to run Details Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions Details Publish / ci (push) Blocked by required conditions Details Publish / Publish to npm (push) Blocked by required conditions Details Publish / Build & Push RC Docker images (push) Blocked by required conditions Details Scorecard / Scorecard analysis (push) Waiting to run Details Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run Details Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run Details Skill copy sync / shipped skills drift guard (push) Has been cancelled Details * feat(cli): add a bunx lane to the runner ladder The ladder assumed a Node toolchain: global gitnexus, then pnpm dlx or npx in some order, with npx as the last resort. On a bun-only machine npm, npx and pnpm are all absent, so every rung fell through to npx and both the emitted hint and the generated .gitnexus/run.cjs produced a command the machine could not run at all. Add bun as a fourth mode, invoked as an install-free bunx one-shot, on two rungs: - npm 11+ with no pnpm to fall back on — bunx dodges the same arborist install crash the pnpm rung exists for (#1939); - npm and pnpm both absent — previously the dead end described above. Every pre-existing outcome is preserved: pnpm still wins on npm 11+, npx still wins on npm < 11, and pnpm still wins over bunx when npm is absent. Regression tests pin each of those. The bun PATH probe is lazy, so a machine with a Node toolchain pays no extra scan and the stale-index hook budget is unchanged. bunx takes no allow-build equivalent: bun's --trust is a bun add/install flag that writes trustedDependencies into a project package.json, which a one-shot has none of, so the argv stays flag-free. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016R9psS9gJ73MRyquoBoPKg * fix(lbug): restore the prebuilt native binary when install scripts were skipped Without this the new bunx lane resolves to a command that still fails: bun skips lifecycle scripts for a bunx fetch, so @ladybugdb/core's install script never copies lbugjs.node up from its per-platform sub-package and every native command dead-ends on 'LadybugDB native binary (lbugjs.node) is missing'. The existing guidance cannot rescue that case. It offers pnpm --allow-build, a global install, or adding trustedDependencies to a project package.json — bunx has no project package.json to add to, no per-invocation opt-in, and re-extracts the package on every run, so an out-of-band repair is wiped before the next invocation. In-process recovery is the only thing that can work. Recovery is cheap because nothing is actually absent: the binary is already on disk in @ladybugdb/core-<platform>-<arch>, and the skipped script only copied it up. Redo that copy (prebuilt only — never a source build, never a network fetch) before reporting failure. Best-effort by construction: read-only node_modules, an absent sub-package or an unsupported platform all fall through to the existing diagnostics unchanged, which a test pins. Also covers pnpm dlx without --allow-build and npm --ignore-scripts. Declare trustedDependencies so a plain `bun install` in this repo produces a working native binary too — the remedy the error message already prescribes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016R9psS9gJ73MRyquoBoPKg * fix(ai-context): name every install-free runner in the generated bootstrap note The emitted gitnexus:start block told a reader with no runner yet to run `npx gitnexus analyze`, falling back to a global npm install. Both name binaries a bun-only machine does not have, so the generated AGENTS.md and CLAUDE.md offered it no reachable bootstrap path. List npx, bunx and pnpm dlx instead of resolving one. The block is committed, so emitting the command this machine happens to resolve would make two contributors on different package managers rewrite it at each other on every analyze — the per-machine churn #1706 removed. Naming all three keeps the note machine-independent and correct everywhere. Regenerates this repo's own committed block to match. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016R9psS9gJ73MRyquoBoPKg * fix(cli): address PR #2765 review — bunx liveness, restore diagnostics, docs Addresses all five review comments on #2765. P1 — `hasBun()` was a PATH-existence check only, so a present-but-broken `bunx` shim (partial uninstall, failed `bun upgrade`) was selected with no functional validation. Because selecting `bun` also suppresses the npm-11 npx-crash warning, the result was a silent dead end: no diagnostic, and a `bunx gitnexus@latest analyze` command that only fails at execution time. Add `probeRuns()` — a real `bunx --version` liveness probe, gated behind the cheap spawn-free PATH scan so machines with npm/pnpm still pay nothing. It ignores the output on purpose (a banner or unparseable version still counts as alive); only a spawn failure, non-zero exit, or timeout rejects. Injectable via a new `bunRuns` dep so the mode tests stay host-independent. P2 — the `gitnexus-cli` skill (and both shipped mirrors) still described the pre-bunx ladder, stranding exactly this PR's audience: a bun-only machine whose agent bootstraps from that file was told to use npx/npm/pnpm, none of which exist there. All three copies now name `bunx` in the ladder and the bootstrap fallback, with a `shipped-skills-sync` fragment assertion so the gap is CI-caught (these copies are not byte-compared, only the engineering family is). P2 — `restorePrebuiltNativeBinary` collapsed every failure into `false`, so an EACCES/EROFS from `copyFileSync` was indistinguishable from "no prebuilt sub-package exists". Users on a read-only `node_modules` layer (a baked container image mounted read-only — a common CI pattern) got the generic lifecycle-script advice, which cannot fix a non-writable filesystem. Return a `RestoreOutcome` instead and route `copy-failed` to its own message. P2 — document that `trustedDependencies` only takes effect for `bun install` / `pnpm install` run inside this repo: it does nothing for a `bunx` one-shot or for a consumer's `bun add gitnexus`. The note sits on `restorePrebuiltNativeBinary` so a future maintainer cannot mistake that function for redundant and delete the thing the bunx path actually relies on. P3 — the `binary_missing` bun advice told `bunx` one-shot users to edit a package.json they do not have, and listed 1 of the 3 packages this package now trusts. Both repair messages now share one `BUN_REPAIR_LINES` const with the full package list and a `bun install -g gitnexus` alternative. Also: shortened the bootstrap note and raised the CLAUDE.md block budget 2900 -> 2950. The note has to name every install-free runner (that is the point of the bun lane), and main's own growth since this PR's last green CI had already pushed the generated block over the old ceiling. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015epfxkEMsmHFNVSFQqAkB4 * refactor(cli): simplify the #2765 review fixes Cleanup pass over the previous commit — no intended behavior change except the doctor status line noted below. Reuse: `probeRuns()` duplicated `probeVersion()`'s entire spawn setup — same argv, timeout, `windowsHide`, and the CVE-2024-27980 Windows-shim workaround — in a file with two byte-identical committed copies, so the shim rule lived at four sites. Its docstring's own objection was to the RETURN SHAPE, not to reuse, so `probeVersion` now returns `{ ran, major, minor }` and `hasBun` reads `.ran`. Existing callers only read `major`/`minor`, so nothing else changes. Also dropped a pointless `const runs = () => …` thunk (`&&` already short-circuits), and deleted a new test that was a character-for-character duplicate of `falls back to npx when npm is null-absent and pnpm is also absent` — its cheapest-first-gate rationale moved into that test's comment. Correctness in the budget comment: the claim that the bun rung is free because "pnpm is absent there, so its probe never ran" was wrong. `formatAnalyzeCommand` spawns `pnpm --version` unconditionally when no global `gitnexus` is on PATH — that spawn IS how pnpm presence is discovered. Real worst case is 5 subprocesses / ~8s, and the 8s needs Windows (`shell: true` spawns cmd.exe for an absent pnpm); on POSIX an absent pnpm ENOENTs in ~1ms. Comment now says that. Likewise "a machine with npm or pnpm never pays" was wrong for npm 11+ without pnpm — that IS the rung that pays. Altitude: `copy-failed` changed only the message text while still returning `kind: 'binary_missing'`, so `doctor` would have printed "✗ lbugjs.node missing" directly above a message saying the binary IS present — exactly the contradiction #2672 removed. Added a `binary_unwritable` kind, a doctor case, and a `nativeStatusCases` row. The binary-missing message construction moved out of `checkLbugNative` into `unrestorableBinaryFailure`, typed `Exclude<RestoreOutcome, 'restored'>` so a new outcome forces a decision instead of silently inheriting the lifecycle-script advice. Drift: the trusted-package list was hand-spelled in five places in native-check.ts, with "matches gitnexus/package.json" asserted only in a comment. All five now render from one `NATIVE_BUILD_PACKAGES` const (rendered output is byte-identical), and the test reads the list out of package.json instead of restating it, so a fourth native package fails the test rather than silently shipping stale advice. Finally, replaced the absolute CLAUDE.md block cap with the ratio the two prior justifications actually appealed to (`< 5465 * 0.55`). Raising 2700 -> 2900 -> 2950 was a ratchet with no ratchet: an absolute cap can only fail on the PR that adds the character, and the fix is always to nudge the number. Also fixed a stale runner ladder in skills-steering.test.ts that still omitted bunx. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015epfxkEMsmHFNVSFQqAkB4 --------- Co-authored-by: drdave-flexnteos <revenaugh.david@gmail.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>	2026-08-06 08:40:36 +00:00
dependabot[bot]	a55763feb3	chore(deps)(deps): bump @tailwindcss/vite in /gitnexus-web (#2845 ) Bumps [@tailwindcss/vite](https://github.com/tailwindlabs/tailwindcss/tree/HEAD/packages/@tailwindcss-vite) from 4.3.2 to 4.3.3. - [Release notes](https://github.com/tailwindlabs/tailwindcss/releases) - [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md) - [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.3.3/packages/@tailwindcss-vite) --- updated-dependencies: - dependency-name: "@tailwindcss/vite" dependency-version: 4.3.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: Abhigyan Patwari <126312502+abhigyanpatwari@users.noreply.github.com>	2026-08-06 09:24:16 +01:00
dependabot[bot]	817893df97	chore(deps)(deps-dev): bump wait-on in /gitnexus-web (#2844 ) Bumps [wait-on](https://github.com/jeffbski/wait-on) from 9.0.10 to 9.1.0. - [Release notes](https://github.com/jeffbski/wait-on/releases) - [Commits](https://github.com/jeffbski/wait-on/compare/v9.0.10...v9.1.0) --- updated-dependencies: - dependency-name: wait-on dependency-version: 9.1.0 dependency-type: direct:development 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>	2026-08-06 08:20:57 +01:00
dependabot[bot]	3e959aefcf	chore(deps)(deps-dev): bump @playwright/test in /gitnexus-web (#2846 ) Bumps [@playwright/test](https://github.com/microsoft/playwright) from 1.61.1 to 1.62.0. - [Release notes](https://github.com/microsoft/playwright/releases) - [Commits](https://github.com/microsoft/playwright/compare/v1.61.1...v1.62.0) --- updated-dependencies: - dependency-name: "@playwright/test" dependency-version: 1.62.0 dependency-type: direct:development 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>	2026-08-06 08:20:40 +01:00
dependabot[bot]	c99ed24aed	chore(deps)(deps): bump react-i18next in /gitnexus-web (#2847 ) Bumps [react-i18next](https://github.com/i18next/react-i18next) from 17.0.10 to 17.0.11. - [Changelog](https://github.com/i18next/react-i18next/blob/master/CHANGELOG.md) - [Commits](https://github.com/i18next/react-i18next/compare/v17.0.10...v17.0.11) --- updated-dependencies: - dependency-name: react-i18next dependency-version: 17.0.11 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>	2026-08-06 08:20:19 +01:00
dependabot[bot]	b529e65b98	chore(deps)(deps): bump tailwindcss from 4.3.2 to 4.3.3 in /gitnexus-web (#2849 ) Bumps [tailwindcss](https://github.com/tailwindlabs/tailwindcss/tree/HEAD/packages/tailwindcss) from 4.3.2 to 4.3.3. - [Release notes](https://github.com/tailwindlabs/tailwindcss/releases) - [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md) - [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.3.3/packages/tailwindcss) --- updated-dependencies: - dependency-name: tailwindcss dependency-version: 4.3.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>	2026-08-06 08:20:02 +01:00
dependabot[bot]	60d244b726	chore(deps)(deps-dev): bump tsx from 4.23.1 to 4.23.4 in /gitnexus (#2850 ) Bumps [tsx](https://github.com/privatenumber/tsx) from 4.23.1 to 4.23.4. - [Release notes](https://github.com/privatenumber/tsx/releases) - [Changelog](https://github.com/privatenumber/tsx/blob/master/release.config.cjs) - [Commits](https://github.com/privatenumber/tsx/compare/v4.23.1...v4.23.4) --- updated-dependencies: - dependency-name: tsx dependency-version: 4.23.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>	2026-08-06 08:19:46 +01:00
dependabot[bot]	29929b7488	chore(deps): bump docker/login-action from 4.4.0 to 4.6.0 (#2851 ) Bumps [docker/login-action](https://github.com/docker/login-action) from 4.4.0 to 4.6.0. - [Release notes](https://github.com/docker/login-action/releases) - [Commits](`af1e73f918...dbcb813823`) --- updated-dependencies: - dependency-name: docker/login-action dependency-version: 4.6.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>	2026-08-06 08:19:19 +01:00
dependabot[bot]	911fdb1ae1	chore(deps): bump ossf/scorecard-action from 2.4.3 to 2.4.4 (#2852 ) Bumps [ossf/scorecard-action](https://github.com/ossf/scorecard-action) from 2.4.3 to 2.4.4. - [Release notes](https://github.com/ossf/scorecard-action/releases) - [Changelog](https://github.com/ossf/scorecard-action/blob/main/RELEASE.md) - [Commits](`4eaacf0543...2d1146689b`) --- updated-dependencies: - dependency-name: ossf/scorecard-action dependency-version: 2.4.4 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>	2026-08-06 08:19:01 +01:00
dependabot[bot]	665e7bb44a	chore(deps): bump release-drafter/release-drafter from 7.6.0 to 7.7.0 (#2853 ) Bumps [release-drafter/release-drafter](https://github.com/release-drafter/release-drafter) from 7.6.0 to 7.7.0. - [Release notes](https://github.com/release-drafter/release-drafter/releases) - [Commits](`eada3c96a6...34d80673e0`) --- updated-dependencies: - dependency-name: release-drafter/release-drafter dependency-version: 7.7.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>	2026-08-06 08:18:01 +01:00
Shifra Williams	f2717c6a7c	feat(render): add one-click deploy to render support (#2804 ) Some checks are pending Scorecard / Scorecard analysis (push) Waiting to run Details CodeQL / Analyze (javascript-typescript) (push) Waiting to run Details CodeQL / Analyze (python) (push) Waiting to run Details Gitleaks / gitleaks (push) Waiting to run Details Publish / Classify release event (push) Waiting to run Details Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions Details Publish / ci (push) Blocked by required conditions Details Publish / Publish to npm (push) Blocked by required conditions Details Publish / Build & Push RC Docker images (push) Blocked by required conditions Details Skill copy sync / shipped skills drift guard (push) Waiting to run Details Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run Details Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run Details	2026-08-06 00:19:44 +00:00
Gergő Magyar	a033b04c46	fix(go): scope and define each type_spec, not the type_declaration (#2837 ) (#2843 )	2026-08-06 00:55:48 +01:00
Gergő Magyar	aaa78f9590	fix(scope-resolution): fan out interface dispatch from Case 3b receivers (#2832 ) (#2842 ) * fix(scope-resolution): fan out interface dispatch from Case 3b receivers (#2832) Case 3b (chain-typebinding) folds a receiver through the same `resolveCompoundReceiverClass` call and the same `[owner, ...mroFor(owner)]` walk Case 0 uses, but emitted its edge without calling `emitInterfaceDispatchFor`. When that fold landed on an Interface the site got one edge to the interface's bodiless declaration and none to any implementation — the defect #2813 reported for field receivers, in the half #2829 did not cover. The gap was a property of how a receiver was SPELLED rather than of what it resolved to. `d.repo.save()` contains a dot, so it took Case 0 and fanned out; binding the identical field to a local first — `const r = d.repo; r.save()` — made the receiver a bare name with a dotted typeBinding, which is Case 3b, and lost every implementation edge. `ownerDef` is the receiver's own folded type, matching Case 0's `currentClass` and Case 4's `ownerDef`, not the owner of the member the MRO walk settled on: a receiver folding to a concrete class that merely inherits an interface method must not fan out, because its runtime type is that class. The closure self-gates on `ownerDef.type !== 'Interface'`, so the call is inert for every concrete receiver and needs no language check. Confidence is the 0.85 literal this case's own primary emits, so dispatch edges never outrank the edge they hang off; Case 4's site.kind-dependent value has no counterpart here because Case 3b's primary does not vary that way. The new fixture pins the route as well as the fix. `const r = d.repo` reaches Case 3b and nothing else can take the site: Case 0 needs a `.`/`(` in the receiver name or a minted receiver chain, and `encodeReceiverChain` returns undefined for the empty step list a bare identifier produces; Case 4 excludes itself on the dot. Before the fix the primary assertion passed while the fan-out came back empty — the exact "reached Case 3b and stopped at the declaration" signature. Resolution-side only: this changes what the resolver produces, not how it is stored, so no SCHEMA_BUMP applies. An existing index must be re-analyzed to show the new edges. Follow-up from #2829. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(scope-resolution): record Case 3b's interface-dispatch fan-out in I4 (#2832) Invariant I4 documented the fan-out as something "Cases 0 and 4 both perform" and spelled out Case 0.5's exclusion, while saying nothing about Case 3b — which is what made 3b's missing fan-out an undocumented asymmetry rather than a deliberate exclusion someone could defend or point at. With the fan-out added, Case 0.5 is the only case that folds or walks to a receiver type without dispatching to implementations, and its exclusion is gated behind `resolveThisViaEnclosingClass`. Saying so explicitly keeps the next reader from having to re-derive which cases fan out by reading the pass. Comment-only; `detect-changes --scope staged` reports no graph change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(scope-resolution): add the concrete-implementor control for Case 3b (#2832) The Case 3b fan-out shipped with one negative control — a chain folding to PlainCache, a class that implements nothing. That proves only the weak claim: no interface anywhere near the site, no fan-out. Add the stronger negative. SqlRepo implements Repo, so an interface IS in scope and `save` is a name Repo declares, yet the receiver's folded type is the concrete class and nothing may fan out. This is the control that fails if a later change fans out from the interface a member is DECLARED in rather than from the receiver's own folded type. The comment says what the control cannot do, too: it cannot catch "member owner passed instead of folded type" in TypeScript, because an implementing class always declares the member itself, so the MRO walk never settles on the interface's bodiless declaration. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NdZtWXQJUGB1ZGLH2YNw3o * docs(scope-resolution): correct four overclaims the review found (#2832) A multi-lane review of this PR reproduced, against the real pipeline, that several claims in the comments and one test name assert more than the code delivers. No behavior changes here — only the text, and one test rename. 1. The test comment gave the WRONG REASON why the concrete-implementor control cannot catch "member owner passed instead of folded type". It said an implementing class always declares the member itself; `class C extends Base implements I {}` is valid TypeScript and inherits it. The real reason is that TypeScript's MRO chain never contains an implemented interface, so the walk cannot settle on an interface declaration for a concrete receiver. The mutation IS expressible where a concrete class inherits a `default` interface method (Java, Kotlin) — reproduced during review — so this is language-scoped, not inherent, and a follow-up fixture is tracked. 2. "fans out to every implementation of the folded interface" certified a completeness that does not exist. TypeScript emits heritage edges for `class_declaration` only (languages/typescript/captures.ts:749, stated in its own docstring at :732-733), so `abstract class X implements I` and `interface B extends A` produce no heritage edge and still dead-end on the bodiless declaration. Renamed to name the shape actually covered, with a KNOWN GAP note. The gap is in the capture layer and predates this fan-out. 3. Invariant I4 said Case 0.5 is the ONLY case that resolves a receiver type without fanning out. Cases 3 and 5 do too, by direct lookup rather than a fold or MRO walk. The sentence now says which distinction it means and states the reachability argument (no known language reaches Case 3 with an Interface — every one that could strips the namespace qualifier first, sending it to Case 4) instead of implying a completed audit. 4. The gate's rationale claimed the `ownerDef.type !== 'Interface'` test is right for every non-Interface receiver. An abstract-class receiver also dead-ends on a declaration-only member and does not fan out. Noted, with why widening the gate belongs to Cases 0 and 4 across all languages rather than to #2832. Also completes the module-level case ladder, which still credited the fan-out to Case 0 alone and omitted it from the Case 3b entry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NdZtWXQJUGB1ZGLH2YNw3o * docs(scope-resolution): name Case 2 in the I4 exclusion list too (#2832) The first pass at this correction listed Cases 3 and 5 as the other cases that resolve a receiver type without fanning out, and was itself incomplete: Case 2 also walks an MRO and its binding admits `Interface`. It is excluded for a different reason than 3 and 5 — its receiver IS the type name, so the site is static dispatch and a fan-out would be wrong, whereas 3 and 5 resolve by direct lookup rather than a fold or MRO walk. Say both rather than enumerate one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NdZtWXQJUGB1ZGLH2YNw3o * fix(scope-resolution): close the two gaps the #2842 review left open Both were pre-existing and reached by Cases 0 and 4 as well; the Case 3b fan-out only widened the population of sites that hit them. Researched against the real TypeScript compiler and the language service before choosing semantics, plus how comparable tools draw the same lines. 1. THE FAN-OUT COULD TARGET A STATIC MEMBER `class C implements I { static save() {} }` does not satisfy `I` — TypeScript rejects it as TS2420, "Property 'save' is missing" — so an edge from an `I`-typed receiver to a static member names a target dispatch can never produce. The closure picked targets with `pickOverload`, which applies no static filter, while the surrounding cases pick their own primary with `pickFirstNonStaticOnly`: the speculative edges were picked with weaker rules than the certain edge they hang off. A same-name static+instance pair also made `pickOverload` return OVERLOAD_AMBIGUOUS, suppressing the CORRECT edge too, so this was a false negative as well as a false positive. Every comparable tool draws this line: tsserver partitions static from instance results, clangd gates on `isVirtual()` (C++ forbids virtual statics), jdtls filters abstract-or-static, and class-hierarchy analysis expands only VIRTUAL call sites. The guard prefers `provider.isStaticOnly` where a language declares it and falls back to the graph node's `isStatic`. That order is load-bearing, not stylistic: the method extractor derives `isStatic` from the OWNER type as well as the member (`staticOwnerTypes`), and the JVM config lists `object_declaration` — so reading the flag first would delete Kotlin `object` implementations, which are singleton INSTANCES and genuinely reachable. Kotlin is the only hook implementor and marks exactly the companion-promoted set; Ruby's `singleton_class` (`def self.foo`) is correctly filtered by the fallback. 2. TYPESCRIPT HERITAGE WAS CLASS-ONLY `interface B extends A` and `abstract class X implements I` emitted no heritage edge at all, so the subtype closure had nothing to descend and both shapes dead-ended on a bodiless declaration — including the very example the closure's own docstring cites as the reason it exists. Since Case 3b's dotted-alias binding survives qualifier-stripping only in TS/JS, this was the language that actually reaches the new path. The two shapes reach their bases differently: an abstract class carries the same `class_heritage` child a concrete one does, while an interface's bases hang off `extends_type_clause` directly. That clause's `type` field is `multiple: true`, so `childForFieldName('type')` would silently drop `C` from `interface B extends A, C` — hence iterating named children. Deliberately NOT structural matching. TypeScript is structurally typed, so a class satisfies an interface without `implements`, but tsc's own navigation is declaration-only and says why: "users are typically only interested in explicit implementations... The type checker doesn't let us make the distinction between structurally compatible implementations and explicit implementations, so we must use the AST." scip-typescript reached the same design independently. gopls does match structurally, but only because Go has no `implements` keyword to prefer. Abstract declarations are still walked THROUGH rather than targeted — the rule everywhere is "does it have a body?", which is what `isDeclarationOnly` already tests. VERSIONING. The capture change is parse-time, so a v43 warm cache would serve entries missing the new matches: SCHEMA_BUMP 43 -> 44 with its pin test moved in the same commit, verified against origin/main at `a857f4c5a` (still 43). Rebaselined only the `typescript` scope-capture fingerprint, justified by a capture-name histogram diff over the same 145-file corpus: the only deltas are @reference.inherits 17 -> 20 and its paired @reference.name 245 -> 248, emitted together by `emitTsInheritanceBase`. Every other capture count is byte-identical and javascript is unchanged, the language having no interfaces. Tests: the fan-out now covers a static-shadowing subclass, a concrete class below an abstract intermediate, and an implementor of an extending interface. Resolvers 3176 passed; all five bench gates pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NdZtWXQJUGB1ZGLH2YNw3o --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-05 15:50:08 +01:00
Parafee41	905a1e191a	fix(mcp): ignore CR-only line ending diffs (#2839 ) Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>	2026-08-05 12:17:44 +01:00
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>	2026-08-05 11:35:27 +01:00
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>	2026-08-05 09:15:32 +01:00
Shifra Williams	a6a8aa788c	feat(serve): validate and port-scope the origin/proxy configuration surface (#2820 )	2026-08-05 06:52:39 +01:00
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 Details Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run Details CodeQL / Analyze (javascript-typescript) (push) Waiting to run Details CodeQL / Analyze (python) (push) Waiting to run Details Gitleaks / gitleaks (push) Waiting to run Details Publish / Classify release event (push) Waiting to run Details Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions Details Publish / ci (push) Blocked by required conditions Details Publish / Publish to npm (push) Blocked by required conditions Details Publish / Build & Push RC Docker images (push) Blocked by required conditions Details Scorecard / Scorecard analysis (push) Waiting to run Details 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>	2026-08-04 21:16:34 +00:00
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>	2026-08-04 21:52:37 +01:00
Gergő Magyar	c1103f38f2	fix: type an inference-typed class field so it can act as a call receiver (#2807 ) (#2810 ) * test(helpers): add the shared temp-repo lifecycle helper `createTempDirPool` gives a suite one owner for its temp fixture repos — create on demand, remove them all in one `afterAll` — instead of a hand-rolled mkdtemp/rmSync pair per file. The PDG receiver pin added in the next commit uses it. Cherry-picked verbatim from `ec36c6dda` on the #2802 branch, where it was extracted to collapse five hand-rolled cleanups. Identical content, so if both branches land the add resolves as a duplicate rather than a divergence. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(typescript): type a class field from its initializer so it can be a receiver A field whose type had to be inferred from its initializer produced no CALLS edge at all — not a truncated chain, nothing. `this.p.inner().compute(x)` lost `Outer.inner` too, an ordinary named-receiver call, because `typeOfMemberOnClass` found no `typeBindings` entry for `p` and `foldReceiverChain` declines at its first untypeable step rather than folding on a guessed owner. The initializer was never invisible: `new Outer()` emitted its own constructor edge exactly as the annotated twin does. What was missing was the step turning that initializer into a TYPE BINDING, i.e. capture patterns for the two shapes the query never covered: private p = new Outer(); // public_field_definition value: private p; constructor() { this.p = new … } // this.<field> = new … Both are `@type-binding.constructor`, so `annotation` still outranks them in `typeBindingStrength` and an annotated field keeps resolving through its annotation. The assignment form carries a narrow `@type-binding.this-field` marker on its `(this)` node — anchorCaptureFor takes the broadest range, so the statement stays the anchor — which `tsBindingScopeFor` reads to hoist the binding onto the Class scope, the only place `typeOfMemberOnClass` looks. The marker must stay specific to that pattern: hoisting every constructor-inferred binding would move method-local `const o = new Outer()` out of its own scope. Kotlin and Swift needed no such pattern for the initializer form because one grammar node (property_declaration) covers both a local and a stored property; TypeScript splits them, and only the local half was ever covered. Both self-diffing pins flip and gain rows: a method-assigned field, and a deliberately mistyped `private p: Mismatch = new Outer()` that asserts the source-strength tie-break executably. That row also pins a pre-existing artifact — `Inner.compute` still resolves through the hoisted module-level return-type binding — verified byte-identical on the pre-fix tree. Fixes #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(javascript): type a class field from its initializer so it can be a receiver JavaScript has no field annotations at all, so a class field's type can only ever come from its initializer — which made this the strictly worse half of #2807: `class C { p = new Outer(); }` gave `this.p` no type, and `this.p.inner()` emitted nothing. `synthesizeConstructorFieldBindings` in captures.ts already covered the sibling shape, `this.p = new Outer()`, which is why THAT row resolved — but it only walks `constructor` bodies, so a field initialized at its declaration matched no pattern anywhere. Adds the `field_definition` + `value: (new_expression)` patterns (the JS grammar names the field `property:`, not `name:`), anchored so the binding lands in the class body scope where `typeOfMemberOnClass` reads it. No hook change needed: `jsBindingScopeFor` already delegates to `tsBindingScopeFor`, so it inherits the `@type-binding.this-field` branch too. Measured: `InferredField.run` now emits `Outer.inner`, exact parity with both the local-const control and the constructor-assigned row. The second chain link (`Inner.compute`) stays absent in ALL THREE rows — that is JavaScript's separate return-type-inference gap, not this one. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(python): infer an instance field's type from the constructor it calls `self.outer = Outer()` in `__init__` bound nothing, so `self.outer.inner()` had no receiver type and the fold declined the whole chain — the Python half of #2807. An annotated field (`self.outer: Outer = ...`) or one assigned from an annotated parameter already worked. `synthesizeConstructorFieldTypeBindings` deliberately refused to infer "from arbitrary unannotated RHS expressions ... not a name-only guess". A CALL is not that: Python has no `new`, so a call to a plain (or dotted) name is the only syntactic construction form there is, and it is the same positive evidence every other language reads from `= new X()`. A bare name, subscript, await or comprehension is still refused. Adds it as a THIRD and weakest tier. The existing explicit/parameter boolean becomes a rank, so precedence is now explicit annotation > parameter annotation > construction, and a later same-tier assignment still wins (the last write in `__init__` is the live one). `interpretPythonTypeBinding` maps the new marker to `constructor-inferred` (strength 1) — checked before the parameter branch, which would otherwise have read the absent parameter marker as `annotation` and promoted a guess to the strongest tier. The Class-scope hoist needed no change: `@type-binding.instance-field` already carries it in `pythonBindingScopeFor`. Measured: `AssignedField.run` now emits `Outer.inner`, exact parity with the annotated-field and local-const rows. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(ruby): infer an instance variable's type from the constructor it calls `@service = UserService.new` in `initialize` bound nothing, so `@service.inner` had no receiver type and the fold declined the whole chain — the Ruby half of #2807. An instance variable is the ONLY way a Ruby object gets a field, and Ruby has no annotations, so this was the single shape that could have worked and did not: the existing constructor-inferred patterns bind a local (`x = Foo.new`) and a constant (`SERVICE = Foo.new`), never an ivar. Adds the plain and `Foo::Bar` qualified ivar forms. `@type-binding.name` is captured on the `instance_variable` node so the bound name keeps its `@` sigil and matches the receiver text at the call site verbatim — the resolver compares spellings, and `service` would never have matched `@service`. `rubyBindingScopeFor` gains a Class hoist gated on a narrow `@type-binding.ivar-field` marker riding the same node: an ivar declares a field of the enclosing class, so the binding must live on the Class scope or no other method can see it. Gated on the dedicated marker, never on `@type-binding.constructor` at large, which also fires for `x = Foo.new` locals that must stay in their own method. Measured: `AssignedField.run` now emits BOTH chain links, exact parity with the local-const control. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(resolvers): pin inference-typed field receivers across eight languages #2807 was filed against TypeScript, but the defect class is cross-language: "can a field whose type is inferred act as a call receiver". This measures all eight languages where the shape exists at all, in one table. The metric is parity with EACH LANGUAGE'S OWN CONTROL ROW, not "both chain links present". JavaScript, Python, Dart and PHP lose the second link (`Inner.compute`) even for a plain local, because nothing annotates `inner()`'s return type — a separate return-type-inference gap. Scoring against "both links" would have accused those four of a bug they do not have; scoring against their own control isolates the field-typing question cleanly. Recorded state: TypeScript, JavaScript, Python and Ruby now match their controls. Kotlin and PHP already did before #2807 and are pinned so the shared fold cannot regress them unnoticed — the languages that got receiver typing for free are precisely the ones nobody re-checks. Two rows stay pinned BROKEN, at their exact current value: Dart — real and narrow: the annotated control resolves, the inferred one does not. Its bindings are synthesized in dart/captures.ts rather than by a query, so the fix is its own change. Swift — blocked by a different defect found while measuring: with several classes each defining `run`, every `run`'s edges are attributed to the FIRST-declared one, which collects duplicates while its siblings — including the ANNOTATED control — collect none. Receiver typing cannot be measured there until that is fixed, and "fixing" it against this observable would be fitting to a broken measurement. Both gap rows carry a `callerExists` probe in the same assertion object, so an empty list can never read as "resolved fine, wrong node id", plus a whole-matrix guard that every language keeps a resolving control — that is what makes a gap row mean "broken" instead of "fixture never worked". Targets are deduplicated before comparison: Swift emits one edge more than once per call site, and edge multiplicity is a different question from whether the receiver typed at all. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(python): a method call on the receiver is not a construction Review finding on f4e1ead0d. `constructorCallTypeName` accepted ANY call with an identifier or attribute callee, so `self.p = self.build()` bound `p` to the non-type `"self.build"` — and because that shares the weakest tier with a real construction, a later such assignment DISPLACED an earlier `self.p = Outer()` and left the field untyped again. Measured before the fix: `self.p = Outer()` followed by `self.p = self.rebuild()` emitted no CALLS edge at all from a method chaining off `self.p`, and `self.q = self.make()` bound a type name that resolves to nothing. After: the real construction survives the reassignment, and a pure method call binds nothing rather than something wrong. Rejects a callee rooted at the receiver name. `models.Outer()` still binds — only `self`-rooted callees are refused, which is exactly the method-call shape. The matrix gains a `reassigned-from-method-call` row that fails without this rejection; that discrimination is the only reason the row exists. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(swift): resolve a method def to its own node when the labels disagree Two classes in one Swift file each declaring `func run` collapsed onto one node: every call in BOTH bodies was attributed to whichever `run` registered first, which collected duplicate edges while its twin collected none. Renaming one method fixed it; moving it to another file fixed it; so the collision was name-keyed and per-file, not positional. Root cause is a LABEL split, not a name. Swift's structure phase emits a type's methods as `Function` nodes, while the scope extractor derives `Method` from the `@declaration.method` anchor. Every key in `resolveDefGraphId` — qualified, parameter-types, arity, shape — is label-scoped, so such a pair misses all of them and lands on the bottom fallback, `simpleKey(filePath, name)`, which is deliberately label-agnostic and first-write-wins. Fixed at both ends: - Swift qualifies a method def as `<Type>.<method>`, matching the qualifier the structure phase already encoded in the node id. `class`, `struct` and `extension` all parse to `class_declaration`, so one ancestor walk covers them; a generic `class Box<T>` and an `extension Foo` wrapping a `user_type` both reduce to the bare owner name. - The bridge retries the qualified keys under the sibling callable label. Gated on the name containing a dot: `A.run` names one construct whatever the label, while a bare `run` is exactly the top-level-vs-method aliasing the label was added to prevent, so the original guarantee is untouched. This also unmasked Swift's #2807 row. `let p = Outer()` had always bound correctly — its edges were being credited to the wrong caller, so the inference-typed receiver looked broken when it was not. `InferredField.run` now emits `Outer.inner`, matching its control. Verified on the full resolver + CFG suite: 3165 passed, 0 failed, against a 3164-passing baseline. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(dart): declare inference-typed class fields so they can be receivers `var b = Outer();` produced no `@declaration.property` capture at all — no Property node, and nothing for the capture layer to hang a type binding on — so `b.inner()` could not type its receiver while the annotated twin `Outer b = Outer();` resolved fine (#2807). The gap was in the query, one layer below where the binding is emitted: both class-field patterns require a leading `(type_identifier)` or `(nullable_type)`, i.e. a WRITTEN type. Dart puts the keyword there instead for an inferred field, and spells it two ways — `inferred_type` for `var`, `final_builtin` for `final` and `late final`. Covering only `var` would have left the more idiomatic Dart style broken, so both are matched. With the field declared, the capture layer types it from the constructor its initializer calls, as `constructor-inferred` — the weakest source, and the annotated branch returns before it, so an annotated field is untouched. Only a direct construction is accepted (a bare identifier followed by a `selector` carrying an `argument_part`, the same shape `findDirectCallValue` accepts for locals); a literal, member call or await is left alone rather than guessed at. Note this is the LOCAL/field split that made the gap invisible: `emitVarTypeBinding` already handled `initialized_variable_definition`, but a class field is `declaration(<keyword>, initialized_identifier_list(initialized_identifier))`. `InferredField.run` now emits `Outer.inner`, matching its control. Dart's `var r; C() { r = Outer(); }` shape stays pinned as a known gap: Dart writes the field with no receiver prefix, so binding it means treating assignment to a bare identifier as a field write, indistinguishable from a constructor-local. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(resolvers): record Swift and Dart reaching parity in the matrix Both languages' inference-typed field rows move from KNOWN GAP to resolving, which is the self-diffing signal this file was built to produce: closing either gap failed it with the newly resolved ids in the diff. The header table and prose are corrected together with the rows, as the file's own instructions require — including WHY Swift moved. Its `let p = Outer()` binding had always been correct; a separate label-split defect attributed the second same-named method's calls to the first, which masked this row entirely. Recording that is the point: a future reader comparing the table against the code needs to know the row was never a receiver-typing failure. One row stays pinned: Dart's `var r; C() { r = Outer(); }`. Dart writes fields without a receiver prefix, so binding it means treating assignment to a bare identifier as a field write — indistinguishable from a constructor-local until the field set is known. Idiomatic Dart writes `final r = Outer();`, which the inferred-field row now covers. Every language keeps its resolving control row, so the remaining gap still means "broken" rather than "fixture never worked". Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(dart): type a field from a constructor assigned to it `var r; C() { r = Outer(); }` bound nothing, so `r.inner()` had no receiver type — the last inference-typed field shape still failing after the initializer form was fixed (#2807). Dart is the one language here that writes a field with NO receiver prefix, so `r = Outer()` inside a constructor is syntactically identical to assigning a constructor-local. That ambiguity is why this was initially left pinned — but the field set IS knowable: the class body declares `var r`, which the initializer fix already turned into a property declaration. So a bare name binds exactly when Dart itself resolves it to the field: the enclosing class declares it AND the enclosing body declares no local of that name. A `this.`-prefixed write is unambiguous and needs neither test. The shadowing case is asserted, not assumed: with a body-local `var s = Outer()` in scope, the field stays unbound while the local still resolves on its own. Binds `constructor-inferred` (weakest source, so an annotation still wins), and only for a direct construction — an identifier followed by a `selector` carrying an `argument_part`, the same shape accepted for locals. The narrow `@type-binding.dart-field` marker drives the Class-scope hoist in `dartBindingScopeFor`; gating on it rather than on `@type-binding.constructor` at large is what keeps genuine locals in their own scope. All three shapes now match their control: bare `r = Outer()`, `this.s = …`, and a non-constructor `setUp()` assignment. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(swift): type an optional field and read through its force-unwrap Swift cannot declare a stored property with neither a type nor an initializer, so its "declare now, assign in init" idiom is an OPTIONAL field read back through a force-unwrap. That shape resolved nothing, and it was broken in two independent places — each alone leaves it broken: 1. `var a: Outer?` parses as `type_annotation(optional_type(user_type(…)))`, but the property-annotation pattern required the `user_type` to be a DIRECT child, so an optional field was never typed at all. The pattern added here captures the INNER `type_identifier`, so the binding is `Outer` without relying on `stripOptional` reducing an `Outer?` spelling. 2. `self.a!` is a `postfix_expression`, which the receiver walk did not peel, so even a typed field could not be read through the unwrap. For (2), `postfix_expression` is NOT added to `TRANSPARENT_RECEIVER_WRAPPERS` outright: unlike TypeScript's `non_null_expression` — which is only ever `!` — Swift's node also carries user-defined postfix operators, which can return anything. Peeling those would type the receiver as the operand and mint a confidently WRONG owner, the failure mode compound-receiver.ts calls strictly worse than no edge. So the peel is operator-gated: transparent only when the node's text ends in `!`, which is provably type-preserving. Verified: force-unwrap `self.a!.inner()`, optional chain `self.b?.inner()`, and the plain annotated field all resolve; previously only the plain one did. The gate keeps this off every other language — `postfix_expression` is not a node type the other grammars produce here — and the full resolver + CFG suite is green at 3166 passed / 0 failed, against a 3165 baseline. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(resolvers): close the last two matrix gaps Dart's `assigned-field` and Swift's new `optional-assigned-field` rows now resolve, leaving no known-gap row in the matrix: every language reaches parity with its own control on both the initializer and the assigned shape it can express. The Swift row is new because the shape it covers did not exist in the fixture: Swift cannot declare a stored property with neither type nor initializer, so its assigned form is an optional field written in `init` and read through a force-unwrap — a shape that needed both an optional-annotation pattern and an operator-gated receiver peel, which is why the row's comment names both. The header records how the two hard cases were fixed, including the Dart shadowing rule the fix depends on: a bare `r = Outer()` binds only when the class declares that field and the body declares no local of the same name. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(bench): rebaseline the receiver-resolution and scope-capture gates Both gates are exact-match, so the improvements in this branch fail CI until the baselines move and the movement is explained. Caught by running the CI gates locally — the resolver and CFG suites are green throughout and never see these. receiver-resolution — three shapes moved to RESOLVES, no drop-count changed: ruby.fieldReceiverCall INVISIBLE-GAP -> RESOLVES (`@ivar = Foo.new`) swift.decoratedFieldType INVISIBLE-GAP -> RESOLVES (`var a: Outer?`) kotlin.nonNullAssert VISIBLE-GAP -> RESOLVES (`x!!` receiver) scope-capture — swift and typescript fingerprints, both ADD captures and remove none; the per-language `_rebaselined_inferred_field_receiver_2807` notes carry the detail and the prior digests. The other 13 languages are unchanged, which is the check that this is the intended emission and not a capture regression. CORRECTION to d5d878033's message, which claimed the operator-gated `postfix_expression` peel "keeps this off every other language — postfix_expression is not a node type the other grammars produce here". That is wrong: Kotlin's grammar produces it too, and `kotlin.nonNullAssert` moving to RESOLVES is the proof. The peel is still correct there — Kotlin `!!` is a non-null assertion with exactly the type-preserving semantics the `!` gate tests for — but it is a BEHAVIOUR CHANGE IN KOTLIN, not Swift-only as stated. The gate is what surfaced it; the claim should have been verified rather than asserted. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(cache): bump SCHEMA_BUMP for the six-language capture change, + review fixes SCHEMA_BUMP 39 -> 40. THIS IS THE MERGE-BLOCKER of the review: every language change in this PR is PARSE-TIME capture emission, and `analyze` skips tree-sitter dispatch for byte-unchanged chunks (GUARDRAILS.md:34), so a warm cache replays the pre-fix capture set verbatim and the new receiver edges never appear — silently, no error. Exactly the v27/v30 failure mode this file already documents. The PR description's claim that "no schema or version constant applies" was wrong on both counts: a bump IS required, and a plain re-analyze does NOT surface the captures without it. Re-check against origin/main before merging — main was also at 39 when 40 was allocated, and this file records eight prior collisions. Also from the review: - dart/simple-hooks.ts hand-rolled a 9-line parent walk byte-identical to the shared `walkToScope(innermost, tree, 'Class')` that TypeScript and Ruby call in one line in this same PR. Now uses the helper. - utils/call-analysis.ts: the doc framed the postfix-`!` peel as Swift-only. It is not — Kotlin `!!` parses as the same node and is peeled too, which the receiver-resolution bench proved (kotlin.nonNullAssert VISIBLE-GAP -> RESOLVES). The comment now says so, and names the `!` gate rather than the language as the bound. - test/helpers/temp-dir-pool.ts: its doc claimed four consumers; on THIS branch only `pdg-chained-receiver-callees` uses it (the other three convert on #2802). Corrected, and the byte-identical-to-#2802 intent recorded. - inferred-field-receiver-matrix: adds the Dart shadowing assertion the header comment already CLAIMED to make but never did. First attempt was vacuous — `var s = Outer()` is a declaration, so it never produced the bare `assignment_expression` the guard inspects; removing the guard did not fail the row. Fixture corrected to `var s; s = Outer();`, and mutation-verified: guard present 35 pass, guard removed the row goes red. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(cache): move the SCHEMA_BUMP pin to 40 The pin at incremental-parse-cache.test.ts asserts the exact value on purpose — it exists to catch two branches claiming one number, and it has earned that eight times. Bumping the constant to 40 without moving the pin turned it red. Found by the Codex (gpt-5.6-sol) review leg, which flagged it as a deterministic committed-test failure. The Claude lanes could not have caught it: they were dispatched before the bump landed. The comment now records the 39 -> 40 movement and its reason, matching the existing convention in that block. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(dart): treat every binder as a field shadow, not just local declarations Review P1, reproduced by two independent reviewers. `emitDartFieldAssignmentBindings` binds a bare `r = Outer()` to the FIELD when the class declares `r` and the body declares no local `r` — but the shadow set was built by walking for `initialized_variable_definition` only. That is one binder form out of many, so a formal PARAMETER named like a field slipped through: void reset(Alpha r) { r = Alpha(); } // r is the PARAMETER retyped the FIELD to `Alpha`, fabricating an edge AND destroying the correct `Beta` binding the constructor had established. The mutation test shows exactly that: the pre-fix result is not a missing edge but a WRONG one (`Other.inner#0` instead of `Outer.inner#0`) — the failure mode compound-receiver.ts:519-537 calls strictly worse than no edge. The node types were chosen from real grammar output, not assumed. Two facts drove the design: formal parameters live on the SIBLING `method_signature`, never inside `function_body`, so no walk of the body could ever have seen them; and `formal_parameter` carries a `name` field only when typed — untyped, `this.` and `super.` forms do not. `collectDartBodyShadows` therefore walks the signature AND the body, collecting formal/closure/local-function/named/optional params, `this.`/`super.` constructor params, catch bindings, for-in variables, and both local-declarator forms. A parameter shape whose name cannot be read contributes nothing — declining to bind is the safe direction. A 27-case binder sweep passes: 26 shadow shapes bind nothing, the no-binder control still binds. Four new matrix rows (param, closure param, catch, loop var) assert a surviving POSITIVE target rather than an empty list — deliberately, because the pre-fix value is a different non-empty target, so these rows cannot pass vacuously the way an empty-assert row can. Mutation-verified: reverting captures.ts turns exactly those four red and leaves every pre-existing row green. SCHEMA_BUMP is already at 40 on this branch for the six-language capture change and has not shipped, so it covers this too; re-check against origin/main before merge. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(ruby): don't bind a class-object @ivar as an instance field Review P1. The `@ivar = Foo.new` patterns added on this branch hoist to the enclosing Class scope without asking WHOSE `self` owns the ivar. In Ruby an ivar written in singleton context belongs to the class object, not to instances, so def self.build; @pool = Alpha.new; end class << self; def make; @cache = Alpha.new; end; end bound `@pool`/`@cache` as INSTANCE fields, fabricating edges from instance methods that read an ivar which is never assigned on an instance. Three corrections came out of fixing it: 1. The detection premise was wrong. `def self.build` is NOT a `method` node with a `self` receiver — it is its own node type, `singleton_method`, and `childForFieldName('receiver')` returns NONE on it. Matching on a receiver field would have detected nothing, silently. Detection is by node type: `singleton_method` / `singleton_class`. 2. A THIRD form exists that the review did not name: a class-body-level `class C; @shared = Outer.new;` is the same defect (self is the class object), and is likewise new on this branch — before it, `left: (instance_variable)` matched nothing at all. 3. Dropping only the `@type-binding.ivar-field` marker is NOT sufficient, and the class-body case is what proves it: with the marker gone the binding falls back to its innermost scope, which at class-body level ALREADY IS the Class scope, so it still lands in the wrong place. The whole match is therefore discarded. The check lives in `languages/ruby/captures.ts` because `Capture` carries only `{name, range, text}` — no AST node — so `rubyBindingScopeFor` structurally cannot ask whose `self` owns the ivar. All Ruby logic stays under `languages/ruby/`. `method` alone is not a sufficient "instance" signal, since a `def` inside `class << self` is reached through a `method` node first. Cost relative to main is zero: a class-object ivar goes back to binding nothing, exactly as before these patterns existed. The three new rows are structurally two-sided, not just mutation-checked: each empty row is paired with a non-empty `-instance-ivar` row on the SAME fixture class, so breaking the hoist entirely turns the partner red while an unconditional hoist turns the empty row red. Mutation-verified in both directions. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> fix(typescript,javascript): bind `this.field = new X()` only inside a class method Review P0, the most serious finding of the tri-review and reproduced by two independent reviewers. The `this.<field> = new X()` patterns added on this branch were CONTEXT-FREE: they matched anywhere in the file, and `tsBindingScopeFor` hoisted to the nearest enclosing Class without asking whose `this` that was. Since the binding lands on the same Class scope with the same `constructor-inferred` source as the field-initializer pattern, and pass4CollectTypeBindings prefers the later match on `>=`, it OVERWROTE the class's real field type. Reproduced from a non-arrow callback, an object-literal method, a static method, and module level. Fixed STRUCTURALLY, in the query: both patterns are now nested under `class_body -> method_definition -> body: (statement_block) -> (expression_statement)`, which kills the callback, object-literal and top-level triggers with no runtime code and mirrors JavaScript's `synthesizeConstructorFieldBindings` discipline. TypeScript still accepts ANY method, not just `constructor`, so the setter case this branch deliberately supports keeps working. `static` needed one emit-side guard: it is an ANONYMOUS token on `method_definition` with no field name, and tree-sitter patterns cannot negate an anonymous token (checked against node-types.json), so `isStaticMethodThis` drops it in captures.ts. `simple-hooks.ts` is comment-only — the unconditional Class hoist is now documented as safe BECAUSE the marker's producers are bounded, with a note that widening them means re-establishing that. Also fixes a `.ts`/`.js` disagreement the narrowing itself created: JavaScript's synthesis matched `method_definition` ANYWHERE, so an object literal containing a method named `constructor` still typed the enclosing class's field. Measured on identical source — JS emitted `p -> Alien`, narrowed TS emitted nothing — and closed with a `node.parent?.type !== 'class_body'` guard in javascript/captures.ts. The two languages must not disagree about the same source. Deliberately NOT matched (a missing binding, never a wrong one — JS declines these too): an assignment in a nested block, or inside an arrow where `this` genuinely IS the instance. Evidence the narrowing removed nothing legitimate: `bench/scope-capture --check` passes with the TypeScript AND JavaScript fingerprints BYTE-IDENTICAL. The five new matrix rows use an `Alien` class that also declares `inner()`, so a regression SWAPS the target rather than emptying the set — they cannot pass vacuously. Mutation test: reverting the source turns exactly those rows red (`+ "Alien.inner#0"`, `- "Outer.inner#0"`). SCHEMA_BUMP stays at 40 — this PR's existing bump covers the capture change being narrowed, and the buggy variant never shipped outside this branch. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(resolution): consult the sibling callable label in the position key too Review P1. This branch added a sibling Method<->Function retry to the qualified keys in `resolveDefGraphId`, but not to the #2699 POSITION key or its fail-closed guard, which both stayed scoped to `def.type`. Since the premise of the whole fix is that Swift defs are `Method` while nodes are `Function`, the position lookup missed and the fail-closed guard could NEVER FIRE for exactly the case the retry serves — so a function-local `func helper` inside `Host.run` was deterministically aliased onto the class method `Host.helper`, even with differing arity. Two earlier reviewers REFUTED this by arguing the guard runs before `lookupTagged`. That is true and irrelevant: the guard is scoped to `def.type`, so in the label-split case it is unreachable. Recording it because two independent lanes agreeing on a refutation is not proof. `siblingCallableLabel(label)` is now the single definition, consulted by all three key families: - position key: retried under the sibling label, gated on `posHit === undefined` so an AMBIGUOUS_POSITION tombstone still falls through to the name keys rather than being resolved by relabelling. Deliberately NOT dot-gated — a position key is not a name, so the aliasing risk the dot gate exists for does not apply. - fail-closed guard: mirrored unconditionally (it only ever returns undefined). - qualified retry: dot gate untouched. Measured before -> after on a Swift fixture: `Host.helper#1 -> sink` (the local body's call credited to the public 1-arg method) becomes `Host.run.helper@8:8#2 -> sink`, with the local's own node no longer edgeless. SCOPE CORRECTION to the P1 report: only the first consequence is a bridge defect. The second — "`run`'s call to the local resolves to the method" — is NOT reachable from ids.ts. Both defs carry qualifiedName `Host.helper` and label `Method`, and the binding hands the target side the class-member def, so the scope walk in free-call-fallback picks the member. No def->node mapping can change that; it is pinned as an explicitly labelled KNOWN GAP rather than left implied. Verification, on shared code so the full bar: resolvers+cfg 3170 passed / 1 skipped / 0 failed; `bench/receiver-resolution --check` OK; `bench/scope-capture --check` PASS (15 languages, Swift fingerprint unchanged) — i.e. the bridge change altered no capture output. The 3170 reconciles against the 3167 pre-existing at `a5bf4c2da` plus exactly 3 new tests; 3167 differs from the older 3166 baseline because `0418b0aac` added the matrix's only known-gap row, which emits one extra `it`. Mutation test: with both arms reverted, 3 of the 5 new cases go red, each arm pinned independently — the guard case registers no position key, the position case registers no local-name key. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor: apply cleanup-review findings across the receiver-typing change Four parallel quality lanes (reuse, simplification, efficiency, altitude) over `origin/main...HEAD`. Eleven fixes; both exact-match bench gates hold with every capture fingerprint BYTE-IDENTICAL, so none of this changed what the analyser emits. Reuse — stop re-rolling helpers that already exist: - `walkToScope` moved out of the TypeScript provider into a language-neutral `utils/scope-tree-walk.ts`. Ruby and Dart had begun importing it FROM `languages/typescript/`, which made three unrelated providers depend on the TS module for a generic `Scope`/`ScopeTree` walk. Python's hand-rolled copy — the one this PR's new `self.x = Outer()` path routes through — is folded in, so all six languages now share one traversal. - Swift stops string-parsing a type name. `swiftEnclosingTypeName` split on `<` and `.`; `swiftBaseTypeIdentifier` + `swiftQualifiedBaseTail` do it structurally and correctly skip the sibling `type_arguments` node, which the string form only guessed at. `findEnclosingTypeDeclaration` replaces the inlined ancestor walk. - TypeScript uses the canonical `hasKeyword(method, 'static')`. The previous `child.type === 'static'` is the exact form `isStaticMember` documents as grammar-version-fragile: "`static` can appear as an unnamed token or as a keyword node depending on grammar version; check text." - Both new test suites use `cleanupTempDirSync`, which exists because a pipeline test's open handle surfaces as EBUSY/EPERM on Windows and `force` does not suppress it. This repo shards Windows CI. LATENT DEFECT, found by the reuse lane and fixed: `var a = X(), b = Y();` parses as ONE `declaration` with two declarators, and the query matches it once per declarator with the SAME node — so the first-descendant search handed every declarator the FIRST one's initializer. `b` resolved as `X`. Now reads `nameNode.nextNamedSibling`, which is both correct and free. Pinned by a `multi-declarator-inferred-field` row ordered so the declarator under test is the second; reverting the fix turns exactly that row red with the wrong edge. Efficiency — measured, not asserted: - Dart's shadow set was built eagerly for EVERY method body and discarded 87-100% of the time (a `this.`-prefixed write never reads it). Now lazy and memoised per body, gated on `fields.has()`. Semantics are unchanged: the set is body-wide, so deferring construction cannot change its contents. Worth recording WHY CI could never have caught this: `bench/scope-capture` gates the SCALING RATIO, and the work is linear — ratio stays 1.0 against a 1.5 budget while a constant-factor regression passes straight through. - `isTransparentReceiverWrapper` crossed the `node.type` native getter twice on the common path. One hoisted read, and — since absent and ungated are distinguishable — one `get` replaces `has`+`get`. Simplification: - One `Map<string, string \| null>` replaces the parallel Set + Map that both expressed "this wrapper is transparent", with `null` meaning unconditional. - `ids.ts` computed `siblingCallableLabel` twice under two names. The three retry blocks are deliberately NOT collapsed — they use different key builders and materially different gates. - Python's `interpret.ts` nesting was only a consequence of arm ORDER; swapping the arms is unconditionally equivalent (the two differ only when both markers are present, and both orders then yield `constructor-inferred`). - One `isDirectConstruction` predicate replaces the construction-shape test that had been written four times in dart/captures.ts. Deliberately NOT done, each needing a fingerprint rebaseline or new node ids: unifying the six `@type-binding.-field` markers into one canonical capture (it would change Python's anchor semantics, which must be verified not assumed); a Swift `labelOverride` mirroring Kotlin's four-line fix, which is the real cure for the Method/Function split the bridge currently compensates for; generalising the Swift optional-annotation pattern to `(type_annotation (_))` so the existing strippers handle every wrapper; and merging the TS query with the JS walker, which also carries a JSDoc branch no query can express. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> test(swift): regenerate the Swift captures golden for the optional-annotation pattern CI caught what my local runs did not: `swift-captures-golden.test.ts` pins `emitSwiftScopeCaptures` output across every `swift-` fixture, and this branch changes that output. It is a THIRD capture gate, separate from the two exact-match benches already rebaselined here — `bench/scope-capture` hashes a different corpus, so its Swift fingerprint moving did not imply this one, and passing it was not evidence this was clean. The drift is digest-only: 37 changed lines, 37 in each direction, no capture entry added or removed. That is the expected shape for `(type_annotation (optional_type (user_type …)))` making optional properties emit an annotation binding they previously did not, plus the `@declaration.qualified_name` now carried on Swift method declarations. Regenerated with the mechanism the test itself prescribes (`UPDATE_GOLDEN=1`), not by relaxing the assertion. Verified after: all Swift unit + resolver suites green (4 files, 124 tests), and `bench/receiver-resolution --check` still exactly matches its baseline. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> fix: three more wrong-owner defects, found by a second review round A second tri-review of this PR found THREE new P1 wrong-owner defects — every one of them in code the FIRST round had already fixed. All three are the same root shape: an incomplete ENUMERATION of binder or scope forms. That class has now bitten this branch four times (formal parameters, then these), so two of the three fixes below deliberately attack the class rather than the instance. 1. DART 3 PATTERN BINDERS (P1, reproduced by two independent lanes). `addDartBinderName` enumerated five binder node types, and every Dart 3 pattern form parses into node types in NONE of them — so a pattern-bound local did not count as a shadow and its write retyped the CLASS FIELD: class Host { var session = Cache(); void load() { final (session, count) = (Session(), 2); } void use() { session.ping(); } // resolved Session.ping, not Cache.ping } The grammar hides every rule that would carry a binder (`_pattern_field`, `_list_pattern_element`, `_guarded_pattern`, …), inlining children onto the enclosing visible pattern node, so binders land as direct `identifier` children of just two leaf types. Covers all 10 pattern types that can hold one; the eight container types are defence, since the grammar demonstrably inlines identifiers onto containers already. THE COMPOUNDING PART: a grammar-derived coverage guard reads `nodeTypeInfo` and fails if the grammar declares a `pattern` type the fixtures do not exercise. A grammar bump adding an 11th type now turns the suite red instead of silently reopening this bug a third time. 2. RUBY BLOCK-RECEIVER `self` REBINDING (P1 here, both Claude lanes + Codex, which rated it P2 — the engines agreed the defect is real and disagreed on severity). `isRubyInstanceIvarWrite` enumerated `singleton_method`/`singleton_class` as the ways `self` gets rebound. A `def` inside a BLOCK attaches to the block's receiver, so `Struct.new(:x) do def warm; @a = Beta.new; end end`, `Class.new do … end`, `class_eval`, and `other.instance_eval { @a = … }` all published onto the nearest LEXICAL class. Deliberately NOT fixed by listing rebinding call names: that set is OPEN — `def helper(&blk) = Foo.class_eval(&blk)` rebinds a block it merely receives, and nothing in the block's own syntax reveals it. An allow-list of "safe" iterators would be the same defect one level down. The rule is structural: crossing ANY block boundary makes ownership unprovable, so discard. Complete by construction rather than by enumeration. ACCEPTED COST, asserted not hidden: `[1].each { @shared = X.new }` in an instance method really is the instance's `self`, and this drops it — that block is syntactically identical to the `instance_eval` one. It has its own row (`plain-block-self-ivar`) so the loss is visible rather than discovered later. 3. STATIC FIELD INITIALIZERS (P1, found by Codex/gpt-5.6-sol, corroborated). A `static` field initializer was captured as an ordinary instance binding, and since both land on one Class scope at the same `constructor-inferred` strength, the later wins the `>=` tie-break — so a static field retyped the instance field of the same name (`this.p.hit()` -> `Wrong.hit`). Unguarded in BOTH `javascript/query.ts` and `typescript/query.ts`; the existing `isStaticMethodThis` only ever covered the `this.x =` assignment form. Two things surfaced while fixing it: the TS `annotation` pattern collides identically and is PRE-EXISTING, not introduced here; and JS `static constructor(){}` had no guard where TS did — the .ts/.js divergence this PR's own comment claimed could not happen. Dart has no same-name twin (the language forbids it), but a static method's receiver-less write named a library-level variable and DISPLACED the constructor's binding. Fixed narrowly, with a counterweight row (`static-field-declaration-still-types-its-receiver`) that goes red if anyone widens the guard into "drop every static binding" — reading a static by bare name from an instance method is ordinary Dart and must keep working. ACCEPTED COST: `typeBindings` has one map per Class scope with no static/ instance split, so a static field is dropped rather than recorded separately, losing typing on a TS/JS `Host.p.hit()` static receiver chain. Missed edge over wrong edge, per compound-receiver.ts:519-537. Every new row asserts a SURVIVING POSITIVE target, never an empty set: the pre-fix value in each case is a DIFFERENT non-empty target, so none can pass vacuously — the trap this branch already fell into once. Mutation-verified per fix: reverting each turns exactly its own rows red (17 Dart, 6 Ruby blocks, 5 static) with every pre-existing row green. Matrix 49 -> 80 tests. Siblings 510 passed. tsc clean. scope-capture PASS (15 languages, all ratios within gate). Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(dart): mask a shadowed field on the READ side, not just the write side The review critic refused to pass this PR while this was open, and it was right to: this is the same wrong-owner shape as the three defects fixed in the previous commit, except this one is introduced BY this PR rather than merely missed by it. THE DEFECT. Typing an unannotated field from a constructor assignment (`var conn; Host() { conn = Alpha(); }` binds `conn` on the CLASS scope) is this PR's whole point. `emitDartFieldAssignmentBindings` correctly declines to WRITE that binding when a member body rebinds the name — but the shadow set gated writes ONLY. `collectDartBodyShadows` had exactly one call site, inside the bare-name write branch. Nothing consulted it on the read side, so a bare-name READ of a shadowing binder the resolver cannot type walked straight past the local and hit the field binding this feature mints: class Host { var conn; Host() { conn = Alpha(); } void probe(List<Beta> xs) { for (final conn in xs) { conn.inner(); } // conn is a Beta element } } // resolved Alpha.inner, not Beta.inner Reproduced in SEVEN binder shapes, not the one the review reported: for-in (`final` and `var`), untyped formal parameter, plain local `var`, catch binding, closure parameter, and record pattern. Delete the constructor and the same read emits NOTHING — which is what proves this PR introduced it. "No edge" became "wrong edge", the one failure mode compound-receiver.ts:519-537 exists to prevent. THE FIX uses `Scope.ownsReceivers` (#2701), the primitive that already exists for exactly this, rather than inventing a mechanism. `scope/walkers.ts` consults `typeBindings` FIRST at every scope and only then honours the mask, so a shadow the resolver CAN type still wins — an annotated `void probe(Beta conn)` keeps `Beta`, because `synthesizeDartSignatureBindings` anchors parameter bindings on the same body node and they land on the same Function scope. The mask fires only where the alternative was a fabricated type. Plumbing follows TypeScript's `@receiver-owner.this` precedent: the marker rides the same synthesized match as `@scope.function` and sits outside the `@scope.` namespace so `anchorCaptureFor` cannot mistake it for the anchor. Dart differs only in that its function scopes are synthesized in captures.ts rather than declared in the .scm, so the names travel as capture TEXT — a `CaptureMatch` carries no AST node, so the reader cannot re-derive them. SCOPE, and the costs taken knowingly rather than hidden. The mask is `shadows ∩ fields` and nothing wider. Masking every locally bound name would also fix a library-level `var logger = Logger();` shadowed by a loop variable, but it changes resolution for code this PR never touched. Three consequences are documented on `dartShadowedFieldsCapture`, not buried: the wider case is left open; an ANNOTATED field shadowed by a binder is masked too (correct Dart, but it touches resolution predating #2807); and `mixin` bodies are reached, since the grammar gives them a `class_body`. PERFORMANCE, measured rather than asserted. The mask is emitted eagerly in Pass A, where `collectDartBodyShadows` used to be lazy — the replaced comment recorded 87-100% of eagerly built sets being discarded, ~15% of Dart emission. Actual cost on the scope-capture large corpus, median of 3: 405.6ms with the mask vs 390.0ms without, ≈ +4%. Fingerprint and capture_groups are byte-identical across both arms, so no corpus fixture emits a mask at all — that 4% is the cost of the CHECK alone. Not visible to `bench/scope-capture`, which gates the scaling RATIO and is blind to a linear constant factor; stated here because the gate cannot state it. (3 samples per arm, blocked not interleaved — an estimate, not a rigorous number.) A per-file memo keyed by node span makes both passes share one walk per body, so the write side no longer pays a second one. SCHEMA_BUMP 40 -> 41 with its exact-value pin, since capture emission changed. Re-check against origin/main immediately before merge — main was 39 at commit time. Mutation-verified both directions, which is the part that matters: - unwire `scopeOwnsReceivers`, rebuild -> exactly 2 rows red (`loop-var-read-does-not-see-the-field`, `pattern-read-does-not-see-the-field`), 83/85 green. - over-widen the mask (drop the `shadows.has` test) -> 28 Dart rows red, including `unshadowed-read-in-a-shadowing-class-still-resolves`. The three control rows stay green under the first mutation BY DESIGN — they guard overreach, not the defect; the second mutation is what proves they are live. Pre/post on the trigger row: `{Class:Alien, Alien.inner#0, Outer.inner#0}` -> `{Class:Alien, Alien.inner#0}`, so no row can pass vacuously. Matrix 80 -> 85 tests. Sweep 3220 passed (was 3215; exactly +5). tsc clean. All four capture gates green: receiver-resolution OK, scope-capture PASS (15 languages, no fingerprint moved, nothing rebaselined), callable-value-flow PASS, swift golden 9. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(ts,js): let each language name its own class-field node type CI caught a defect the whole review round missed. `grammar-literal-validation`: 1 dead grammar literal(s) found: - node-type "field_definition" — languages/typescript/captures.ts:0 — not valid in [typescript] `isStaticClassFieldBinding` held BOTH spellings in one set — `public_field_definition` (TypeScript) and `field_definition` (JavaScript) — so that one predicate could serve both languages. But the predicate lives in `typescript/captures.ts`, and the gate checks every literal against the grammar of the FILE it appears in. `field_definition` is not a TypeScript node type. The literal was NOT dead code: `javascript/captures.ts:42` imports the predicate and calls it against real JS nodes, so the guard worked. The gate is still right to fail it, and for exactly the reason this predicate's own docblock gives for preferring `hasKeyword` over a node-type test — "a node-type test silently stops firing on a grammar bump and every static field starts retyping its instance twin again". A literal already dead in its own file is that failure shipped pre-broken: nothing in the TypeScript file would ever have told us. Each language now names its own node type and passes it in (`TS_CLASS_FIELD_DEFINITION_TYPES` / `JS_CLASS_FIELD_DEFINITION_TYPES`), so every literal is checked against the grammar it belongs to. The `hasKeyword` logic and the static/instance reasoning stay shared and unchanged — only the node-type set moves to the caller. WHY THE LOCAL SWEEP DID NOT CATCH IT: I ran `test/integration/resolvers` and `test/integration/cfg`. The gate is `test/integration/grammar-literal-validation. test.ts`, in the parent directory. Scoping a sweep to the subdirectories a change touches is precisely how a cross-cutting gate gets skipped. grammar-literal-validation 4 passed. tsc clean. Full `test/integration` + `test/unit/scope-resolution`: 6305 passed, 14 failed — all 14 in e2e/environment suites (fts-extension-e2e 9, analyze-heap-oom-e2e, cli-e2e, analyze-wal-checkpoint-failure, plus interproc-taint and parse-impl-env-reads, which BOTH pass in isolation and fail only under 28-worker load). CI runs the same files green. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(dart,ts): pin all seven read-side binder shapes; correct a wrong accepted-cost claim Two review findings, one of which turned out to be a documentation defect rather than the design defect it was filed as. S8 — THE READ-SIDE FIX PINNED 2 OF THE 7 SHAPES IT REPORTED REPRODUCING. `ab2f48c17` reported the wrong-edge defect reproducing in seven binder shapes and landed rows for two. The stated mitigation was that all seven route through one `collectDartBodyShadows` enumeration whose completeness the grammar-derived coverage guard protects. That mitigation is NARROWER THAN CLAIMED: the guard filters `nodeTypeInfo` on `type.includes('pattern')`, so it covers the pattern family and NOT catch bindings, closure parameters, plain locals, or formal parameters. Narrowing `addDartBinderName`'s catch arm would have turned no row red. All seven were re-measured by unwiring `dartScopeOwnsReceivers` and rebuilding. Every one gained the wrong edge `Outer.inner#0` — none had to be dropped as non-reproducing. Five new rows: formal parameter, plain local `var`, catch binding, closure parameter, for-in `var`. Non-vacuity established structurally, not by assertion: the Dart AST was dumped first to confirm each fixture produces the node `addDartBinderName` actually inspects (`formal_parameter`, `initialized_variable_definition`, `catch_parameters`, `for_loop_parts`). The catch row uses a bare `catch (zf)` rather than `on Err catch` deliberately — an `on` clause names a type, which would make the row measure type resolution instead of the mask. S7 — THE ACCEPTED-COST COMMENT WAS WRONG, AND THAT IS THE FINDING. It claimed dropping a static field's binding trades a wrong edge for a missed one. Measured on a same-name twin, that is false: read with the drop without it this.p (instance twin) Outer correct Alien wrong Host.p (static twin) Outer WRONG Alien correct Host.q (static, no twin) none — missed Alien correct The wrong edge did not disappear. It MOVED to the static read, which now picks up the instance twin's type. Only the no-twin case is a genuine missed edge. The trade is still right — `this.p` is far more common than `Host.p` — but it was documented as safer than it is, and a reader deciding whether to revisit it was being given the wrong picture. NAMESPACING WAS EVALUATED AND DELIBERATELY NOT DONE. `Host.p.hit()` resolves through `foldReceiverChain` in shared `compound-receiver.ts`, which explicitly discards whether a chain's base was a class reference or a value (:519-527). The class-constant bit exists only on the text-cascade path (`currentIsClassConstant`) and is consumed solely by `isConstructionSelectorHop`; TS/JS take the fold, not the cascade. `Scope.typeBindings` is `ReadonlyMap<string, TypeRef>` with no static field. `ownsReceivers` cannot help — it is a suppressor that can only REMOVE a binding, never route to a second one. A real fix needs `FoldState` to carry the bit plus a key convention in shared code (an AGENTS.md:42 hook if not language-neutral), it crosses the worker boundary so it needs a SCHEMA_BUMP, and `compound-receiver.ts:826` iterates every binding for `fieldFallback` so a namespaced key would leak straight back in as an ordinary field. Not a cheap or safe change — and it would have been made with ZERO existing tests pinning static-read behaviour. So: smallest safe step instead. Two rows pin the measured behaviour (`static-read-of-a-same-name-twin-picks-up-the-instance-type` asserts the positive wrong target, not an empty set; `static-read-without-a-twin-loses-its-type` is a known-gap), and the comment now says what actually happens. Anyone who revisits this starts from measurements rather than from a claim. No SCHEMA_BUMP: the `captures.ts` change is comment-only — verified, the diff has no non-comment added lines. Mutation red-rows 2/85 -> 7/90; each new row fails with a strictly larger set (`+Outer.inner#0`), so none can pass vacuously. Overreach control still live: dropping `shadows.has` turns 28 rows red including `unshadowed-read-in-a-shadowing-class-still-resolves`. Matrix 85 -> 92 tests. Sweep 3231 passed, 0 failed. tsc clean. All four gates green, no fingerprint moved, nothing rebaselined. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(python): stop a dotted callee from fabricating a constructor type S4 and S5 from the review round. They are ONE defect, not two, and the real one is wider than the review described. Both live in a 32-line block THIS PR adds (`@@ -116,0 +117,32 @@` — a pure addition), so neither is pre-existing. THE DEFECT. `constructorCallTypeName` rejected only a callee rooted at the receiver and returned every other dotted callee whole to `resolveTypeRef`, which resolves dotted names through `QualifiedNameIndex` — and that index matches the TRAILING SEGMENT against a class of that name even when the callee is a method on an unrelated object: class Alpha: def ping(self): return 1 class Factory: def Alpha(self): return "not an Alpha" # a METHOD class Host: def __init__(self, f): self.svc = f.Alpha() # svc is a str def run(self): return self.svc.ping() # measured: Host.run -> Alpha.ping, fabricated WIDER THAN FILED: the review framed the trigger as a callee rooted at an `__init__` PARAMETER. Measured, the root's binding form is irrelevant — a module-level variable (`shared_factory.Alpha()`) fabricates identically. Any rule written against what the root binds to would have fixed half the defect and left the other half looking fixed. Both fabrications now have rows. S5 IS A SYMPTOM, NOT A SECOND DEFECT. `self.conn = Outer()` then `self.conn = Registry.get()` typed the field as `"Registry.get"` (resolving to nothing, so the edge vanished) only because the dotted arm accepted `Registry.get` as a constructor in the first place. Once dotted callees yield no candidate, there is nothing weak left to displace with and `Outer` survives. So `>=` is untouched and NO second mechanism was added: between two REAL constructions last-write-wins is correct, and the existing `ReassignedField` matrix row depends on it. Tightening the tie-break would have been the wrong fix to a symptom. THE FIX: accept a bare `identifier` callee only. Refusing ambiguous evidence at CAPTURE time rather than resolving-then-rejecting is deliberate — the target-kind route is not reachable from this file (`resolveTypeRef` already filters `TYPE_KINDS`; the fabrication comes from a trailing-segment match in `scope/walkers.ts`), and the root-alias route would collide with PR #2828, which is rewriting exactly how an unaliased dotted namespace import resolves. This change is orthogonal to #2828 by construction: it changes what is CAPTURED, never how a name is looked up, and touches none of its files. WHAT THE DOTTED ARM WAS ACTUALLY BUYING: nothing. The review (and this PR's own docblock) justified it with `self.u = models.User()`. Measured, that shape emits NO edge before or after this change — an instance field's binding lands in CLASS scope, which never reaches the namespace split. The shape that really resolves is the module-level local `u = models.User()`, which comes from `query.ts` and is untouched here. The arm's entire measured contribution was fabrications, which is what made the fix cheap. #2828 COMPATIBILITY, checked not assumed: `import pkg.user` -> `self.u = pkg.user.User()` resolves to nothing both before and after, so this cannot stop it resolving. No test row pins that shape ON PURPOSE — asserting its current empty state would plant a tripwire that goes red the moment #2828 lands. If #2828 also teaches the FIELD path the namespace split, re-enabling dotted field callees becomes a live option; the docblock says so, and says why redoing it capture-side would re-open the fabrication. SCHEMA_BUMP 41 -> 42 with its pin. This is parse-time capture emission: after the fix `self.svc = f.Alpha()` emits no `@type-binding.constructor` capture at all, so a v41 warm cache replays the pre-fix capture set for byte-unchanged files and keeps serving the fabricated edge (GUARDRAILS.md:34). A within-PR re-bump, not a collision fix — 40/41/42 are all this unmerged branch's, and `origin/main` is at 39. Re-check against origin/main immediately before merging. Mutation-verified in BOTH directions, which is what shows the fix is placed at the right width rather than merely working: - revert the fix -> exactly 3 red: both S4 fabrication rows + the S5 displacement row (8 green) - reject EVERY callee -> exactly 3 red: the three positive-typing rows (8 green); the S4 rows correctly stay green The two mutations hit DISJOINT row sets — too loose and too tight each break a different half. No row asserts an empty set: the three "must not type" rows call `Alien.ping()` as a witness so a regression SWAPS a target in rather than emptying. Non-vacuity is asserted in the test itself — one guard checks every caller node is live, another asserts the `Alpha` class / `Factory.Alpha` method name collision the fabrication NEEDS is actually present, so the rows cannot rot into passing for the wrong reason. Sweep 3268 passed, 0 failed. Python unit + python.test.ts 342 passed. tsc clean. All four gates green — no bench cell moved, nothing rebaselined. Refs #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-04 19:31:25 +01:00
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>	2026-08-04 11:29:51 +01:00
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>	2026-08-04 09:55:59 +00:00
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>	2026-08-04 08:53:24 +00:00
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>	2026-08-04 09:33:12 +01:00
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>	2026-08-04 09:32:20 +01:00
dependabot[bot]	4a2f4c8ddd	chore(deps)(deps): bump the npm_and_yarn group across 1 directory with 1 update (#2817 )	2026-08-04 07:48:05 +00:00
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>	2026-08-04 08:23:53 +01:00
dependabot[bot]	38be4c6bd2	chore(deps)(deps): bump node-addon-api from 8.9.0 to 8.9.1 in /gitnexus (#2816 )	2026-08-04 06:48:55 +00:00
dependabot[bot]	ca294e8cdb	chore(deps)(deps): bump ip-address from 10.2.0 to 10.4.0 in /gitnexus (#2818 )	2026-08-04 07:24:37 +01:00
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 Details CodeQL / Analyze (python) (push) Has been cancelled Details Gitleaks / gitleaks (push) Has been cancelled Details Publish / Classify release event (push) Has been cancelled Details Scorecard / Scorecard analysis (push) Has been cancelled Details Skill copy sync / shipped skills drift guard (push) Has been cancelled Details Trivy Image Scan / Trivy (gitnexus-cli) (push) Has been cancelled Details Trivy Image Scan / Trivy (gitnexus-web) (push) Has been cancelled Details Publish / RC guard (marker + release-PR skip) (push) Has been cancelled Details Publish / ci (push) Has been cancelled Details Publish / Publish to npm (push) Has been cancelled Details Publish / Build & Push RC Docker images (push) Has been cancelled Details * fix(mcp): key the empty-ascent note on CALL_SUMMARY data, not language (#2802) `pdg-impact.ts` decided whether to append a "return-value ascent is TypeScript/JavaScript-only" caveat to the `impact(mode:'pdg')` note by looking up the criterion file's language. That put language-specific logic in a layer that must be language-agnostic, and it was a lossy proxy for a fact the graph already holds. Whether the ascent can fire is a property of the persisted CALL_SUMMARY edges. The descent already computes it, so thread the resolved-callee and return-flowing counts out of `interproceduralDescent` and key the note on those instead. Three defects the language proxy carried, all gone: - Wrong for `.mjs`/`.cjs`/`.mts`/`.cts`: the provider registry's extension arrays omit them while the ingestion pipeline parses them as TS/JS, so those files were harvested but the note claimed their ascent was empty. - Silently stale: any language whose harvester started recording formal indices would keep getting the caveat until someone edited the list. - Wrong in reverse: a TS/JS callee with no return-flow got no caveat, so an ascent that found nothing read like one that covered the slice. `pdg-impact.ts` now names no language and imports nothing from the language layer, which also drops the analyze-only provider closure from MCP server startup. Measured on overlayfs against a full build: import mcp/local/local-backend.js before 565-648 ms / 548 modules import mcp/local/local-backend.js after 458-463 ms / 170 modules Tests hold CALL_SUMMARY content fixed while varying the file extension across nine languages and assert the note text is identical, then hold the extension fixed and vary the summary to show the note tracks the data. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(mcp): guard MCP startup against the language-provider closure returning The eager `pdg-impact.ts -> core/ingestion/languages` edge was found and lost once already during #2793 before #2802 re-derived it, so it gets a test rather than a comment. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(lbug): record why csv-generator is not lazy-imported #2802 proposed cutting `csv-generator.js` out of the adapter chain to shorten MCP server startup. Measured on a native filesystem, the marginal cost is small relative to the siblings this module already imports, and `core/search/bm25-index.ts` statically imports `normalizeFtsText` from the same module on a path `local-backend.ts` reaches dynamically for FTS — so deferring would relocate the cost to first query, not remove it. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(pdg): pin chained receiver calls reaching BasicBlock.calleeIds The PDG inter-procedural descent hops through `BasicBlock.calleeIds`, so it can only cross a call boundary the resolver resolved. Chained receiver calls reach `calleeIds` through the receiver-typing pass's own `calleeIdSink` — a separate path from plain calls. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(analyze): drop the stale per-language cross-reference (#2802 review P3-4) `pdgModeMismatch`'s comment told readers to keep "the diagnostic per-language refinement in the impact CONSUMER (see pdg-impact.ts assemblePdgImpactResult)". That refinement is no longer per-language — removing it is the point of #2802, which now keys the empty-ascent note on the persisted CALL_SUMMARY data instead. The comment's real invariant is untouched and still correct: the values in `resolvePdgConfig` must stay scalar, because the comparison below is a shallow `!==` and an object would compare by reference. Only the cross-reference was stale. Comment-only; no executable line changes. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(mcp): probe the real module loader for the startup language closure (#2802 review P1-2) The previous guard hand-rolled a regex walk over TypeScript source to assert `core/ingestion/languages` was not statically reachable from MCP startup. Four bypasses were reproduced against it, any one of which let the exact 226-module regression return while the test stayed green: a. Wrong entry root. It walked from `mcp/local/local-backend.ts`, but the server module is `mcp/server.ts` — which imports LocalBackend as `import type`, so the guard's anchor was not even on server.ts's runtime closure. Ten real startup modules sat outside it. b. A top-level `await import(...)` executes during module evaluation, so it is eager at startup — but the walker skipped every `import(...)` by construction. c. The `import type` strip deleted a 16,445-character window of `pdg-impact.ts`: an `export type X =` matched lazily to the next `from "…"`, which lives inside a string literal. Any import in that window was invisible. d. The comment strip treated a `/` inside a string literal as a comment opener. Replace the approximation with a real module-load probe: spawn a child node process per entry, import the built `dist/` entry, and report what the loader actually pulled in. Rooted at `dist/mcp/server.js` and `dist/cli/mcp.js` (the real startup entries) plus `dist/mcp/local/local-backend.js`. Syntax cannot fool it. One deviation from the two existing sibling probes is load-bearing: `dist/` is ESM, so a `require.cache` diff alone cannot see the first-party `dist/` graph — it only catches CJS and native modules, which is why `import-closure.test.ts` gets away with it (it asserts on `@ladybugdb/core`). A pure cache diff here would have reported zero language modules unconditionally, i.e. a new vacuous guard. This probe unions `module.registerHooks({ load })` with the cache diff, and each entry carries a non-vacuity anchor and a module floor so an empty result fails loudly. Verified load-bearing: adding a top-level `await import('../core/ingestion/languages/index.js')` to `src/mcp/resources.ts` and rebuilding turns `dist/mcp/server.js` red with 70+ named offenders, while the `local-backend` and `cli/mcp` cases stay green — which is bypass (a) demonstrated directly. The old guard passed that poisoned tree entirely. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> docs(lbug): drop the unreproducible 9p multiplier from the csv-generator note (#2802 review P3-2) The comment justifying why `csv-generator.js` is NOT lazy-imported carried a hard "~40x" figure for how much a 9p mount inflates per-file ESM resolve. Three independent measurements during review produced ~40x, ~7.3x and ~30x, so the multiplier is not a reproducible quantity and had no business being stated as one in a durable comment. Reworked so the STRUCTURAL argument leads and the numbers only support it. That argument is what actually settles the question and it does not rot: `core/search/bm25-index.ts` statically imports `normalizeFtsText` from `csv-generator.js`, and `local-backend.ts` reaches bm25-index through a dynamic import on the FTS query path — so deferring here relocates the cost to first query rather than removing it. Both verified again at `bm25-index.ts:15` and `local-backend.ts:2756`. Remaining figures are re-measured, attributed to a date and issue, and labelled by filesystem: ~1.6 ms marginal (median of 45 cold imports on local disk) versus ~50 ms for the same import on a network mount, stated as environment-bound rather than as a property of the module. The provider-registry cost is given as "several hundred modules" — the static walk, the runtime hook, and the reviewer's probe each counted it differently (375 / 439 / 407), so no single number was picked to go stale. The old "226 modules" was real but counted only the `languages/` subtree and undercounted the win. Also repoints the trailing reference to the guard's new home at `test/integration/mcp/startup-language-closure.test.ts` (same comment block, inseparable from this rewrite). Comment-only; no executable line changes. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(mcp): stop the empty-ascent note asserting a fact an undecodable summary contradicts (#2802 review P2-2) The note claimed "this is a property of the persisted summaries" whenever the descent resolved callees and none carried a return-flow. But `decodeCallSummary` never throws by design: a version-skewed (`2\|r:1`), corrupt (`1\|r:zz`), or NULL `reason` yields no entry, which was indistinguishable from a cleanly-decoded empty summary. So the note could assert "no formal parameter is recorded as flowing to its return value" about a callee whose CALL_SUMMARY actually records `p0 -> return`. `meta.pdg.hasCallSummary` is a plain boolean and stores no codec version, so nothing else caught it. `calleesWithReturnFlow` now reports three outcomes instead of two — flowing, decoded-empty, and undecodable — and the undecodable count is threaded through the descent to the note. When it is non-zero the note says so and points at a re-index; when every summary decoded, the persisted-summaries claim is kept and now explicitly conditioned on that. Soundness is unchanged: an undecodable summary still licenses no ascent and never enters the return-flowing set, so the ascent path is byte-identical. Only the note's wording moves. Tests drive all three undecodable forms through the mock and assert the false claim is gone, the remedy is reported, and the ascent is still withheld. A companion assertion pins that the all-decoded case KEEPS the persisted-summaries claim, so the fix cannot degenerate into deleting the sentence. Verified load-bearing: reverting the source alone fails 6 of 34. Impact analysis: `calleesWithReturnFlow` upstream LOW (2 callers, both in this file); `assemblePdgImpactResult` upstream LOW (1 caller). Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(pdg): cover every chained-receiver shape and pin the inference gap (#2802 review P2-1, P3-1) The fixture proved chained receiver calls reach `BasicBlock.calleeIds` using exactly one receiver form — a local `const`. That is the shape that works, so a single-shape fixture implied general support the resolver does not have. This repo has been burned by that before: a drop-count gate blind to fixed shapes. Measuring nine forms against the real pipeline also corrects how the gap was originally characterised. It is NOT local-versus-field. An annotated field resolves fine, including the constructor-assigned variant: private p: Outer = new Outer(); -> both links private p: Outer; this.p = new Outer(); -> both links private p = new Outer(); -> EMPTY CELL private p; this.p = new Outer(); -> EMPTY CELL The discriminator is the type ANNOTATION. When a field's type must be inferred from its initializer the whole `calleeIds` cell empties — so even `Outer.inner`, an ordinary named-receiver call, is lost, and the inter-procedural descent cannot cross the boundary at all. Pre-existing; independent of #2802, which does not touch receiver resolution. The fixture is now table-driven over seven working forms (local const, local in a method, annotated field, ctor-assigned annotated, ctor-param assigned, call-result receiver, three-link chain) plus the two inference-typed forms, each row carrying its expected chain-link ids. Assertions moved from substring to exact id membership, split with the production `splitCalleeIds` reader — so `Inner.compute` can no longer be satisfied by `Inner.computeExtra` or `OtherInner.compute`, which matters because the descent keys on exact ids for span and CALL_SUMMARY lookup. The two known-gap rows are pinned with `it.fails` plus a hard assertion on the exact gap-row set, so a resolver fix turns them red instead of passing silently, and an anti-vacuity guard requires every shape to match exactly one block — without it a drifted fixture matching zero blocks would let `it.fails` pass for the wrong reason. Proven by mutation: relabelling a working row as a known gap fails both pins. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(mcp): qualify the empty-ascent note when the examined callee set is incomplete (#2802 review P2-4) The note asserted "none of the N resolved callees carry a CALL_SUMMARY return-flow", and on the all-decoded path that this is "a property of the persisted summaries". Both are universal claims over the callees the descent actually examined, and two mechanisms can leave that set incomplete without the note saying so: 1. Budget truncation. The descent stops on depth/limit/node-cap, so a callee that DOES carry a return-flow can sit in a hop never reached. A 4-deep chain reported "none of the 3 resolved callees" while link 4 held the only summary. 2. Emit-time capping. When a block's `calleeIds` cell was capped, `splitCalleeIds` strips CALLEES_TRUNCATED_SENTINEL, so the dropped callees are invisible to both the scan and the counters — even though the callgraph bridge in this same file already treats such a block as callee-incomplete. Add `calleeIdsWereTruncated`, the counterpart to the sentinel strip, read from the raw cell before splitting so a block whose entire list was capped away still raises the flag. Thread it through the descent to the note. Case 1 needs no new plumbing — the aggregate `truncated` is already on the input object. Using the aggregate rather than a descent-only flag is deliberate: seed truncation and intra-BFS depth truncation also shrink the initial slice, so their callees are never gathered either. It is a sound superset that never under-hedges. When either mechanism fired, one clause naming the reasons is appended and the whole-slice assertion softens to "every summary examined decoded … a property of those summaries". When the set is complete both branches stay byte-identical to before, so this does not become a blanket hedge. Tests pin truncated, untruncated, emit-capped-alone, both-mechanisms, and undecodable+truncated, asserting the truncation premise rather than assuming it. Verified load-bearing: reverting the source alone fails 6 of 42, and the HEAD note printed in those failures is the bug verbatim. Impact analysis: `assemblePdgImpactResult`, `calleeIdsByBlock`, `interproceduralDescent` all upstream LOW; every caller is in this file and `runImpactPDG`'s exported signature is unchanged. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(mcp): stop the empty-ascent note calling call-site references "resolved callees" (#2802 review P3-7) The note printed "none of the N resolved callees carry a CALL_SUMMARY return-flow (no formal parameter is recorded as flowing to its return value)". N counted the raw `BasicBlock.calleeIds` cell, which carries ids `resolveCalleeSpans` never enters — out-of-repo targets, interface methods, and the `Class:` id a `new X()` emits. On the chained-receiver fixture that inflated N from 1 to 3. Two defects, both in the wording rather than the arithmetic: "resolved" implies a symbol-table lookup that did not happen for those ids, and the parenthetical asserted a FORMALS-level property about symbols never resolved to a body. Reworded rather than re-seeded, deliberately. `calleesWithReturnFlow` scans the RAW id set, so the claim "none of these carries a return-flow" is exactly established for all N — the scan really did check the `Class:` id. Re-seeding N from the resolved spans would make the sentence quantify over a strict SUBSET of what was checked, silently dropping the un-enterable references from a claim that genuinely covers them, and would desync N from `calleesUndecodable`, which is derived from the same scan population. none of the N resolved callees carry ... none of the N call-site callee references carry ... and the formals parenthetical is dropped. The note gets shorter, not longer. `calleesResolved` is renamed `calleeReferences` end-to-end (file-local; nothing outside referenced it), and the descent's return-type doc — which called them "callee symbols the descent resolved" and reinforced the wrong reading — now states that un-enterable ids ride the same cell, are scanned, and are never entered. The `> 0` gate is unchanged, so no slice that previously produced the note stops producing one. A test pins that explicitly: an all-un-enterable cell resolves no span, takes no hop, and emits no ascent sentence despite a non-zero count — so a future re-seeding cannot silently move when the note fires. Tests also pin the quoted number and singular/plural against a mixed cell, with a discriminator asserting `reachableBlocks` is byte-identical while the count moves 1 -> 3. Verified load-bearing: reverting the source alone fails 6 of 7 new tests, printing the finding verbatim. Impact analysis: `assemblePdgImpactResult` and `interproceduralDescent` upstream LOW, sole caller `runImpactPDG` in the same file; exported signature unchanged. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(mcp): pin cross-hop callee accumulation and the mixed return-flow contract (#2802 review P2-5) Every case in this file drove a single hop, so the Set union the descent performs across hops (`calleeReferencesSeen` / `calleesReturnFlowingSeen`) was never proven to accumulate rather than overwrite — a one-hop descent cannot tell the two apart. And although a sibling commit added a three-id cell, none of those ids return-flowed, so the "some callees flow, some do not" boundary was entirely unpinned. Extends the mock with a `secondSummary` knob that drives a genuine second hop: `helper2` is named only in `helper`'s own body block, so the descent must cross a second boundary to reach it. Three mock handlers are made faithful to the parameters they already bind — `calleeIdsByBlock` now routes on the asked `$ids`, and the CALL_SUMMARY scan and span resolve answer per asked id — which is what makes a second callee answerable at all. Existing cases are behavior-identical. Five tests: the union count across two hops; a return-flow on hop 0 surviving a later empty hop; a return-flow found only on hop 1; mixed callees in one examined set going silent rather than partial; and a flowing callee alongside an undecodable sibling staying silent including the decode remedy. The mixed case pins a deliberate contract rather than proposing one. The production condition is `calleesReturnFlowing === 0`, so partial coverage is reported as silence. A reviewer considered and dropped "report partial coverage" as a product change; this makes flipping it a conscious edit instead of an accident. Verified load-bearing against three separate source mutations: accumulating only on hop 0 (2 fail), each hop overwriting instead of unioning (3 fail), and flipping the gate to partial-coverage reporting (4 fail). In all three every PRE-EXISTING test still passed — which is the finding restated as evidence. Test-only; `pdg-impact.ts` is byte-identical to HEAD. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(mcp): consolidate the empty-ascent rationale to one canonical site (#2802 review P3-6) The "keyed on observed CALL_SUMMARY data, never on the criterion's language" rationale was restated in full at four comment sites. It exists because a reviewer asked "why not just look up the language?", so it has to stay findable — but not four times. The canonical explanation now lives in `interproceduralDescent`'s return-type doc, where the counters are actually computed, organised as POPULATION (why the raw `calleeIds` tally is the right set to quantify over) and OBSERVED DATA, NEVER THE CRITERION'S LANGUAGE (the full answer, including the producer-change argument and the no-language-naming rule). The other three sites keep only what is locally load-bearing and point here. Deliberately preserved, because each carries a non-obvious fact: why an undecodable summary licenses no ascent, why the aggregate `truncated` is used rather than a descent-only flag, and the raw-id-tally population argument. Net comment delta -11 lines. The reviewer also flagged the local/field naming asymmetry (`calleeReferencesSeen` vs `calleeReferences`). Keeping the suffix, with a comment recording why so it is not re-raised: the premise that every other local matches its field is true, but those locals are identity-returned, whereas these are `Set<string>` accumulators returned as `.size`. Dropping the suffix would give one identifier two types in one file — a `Set` at the accumulation site and a `number` where the note does arithmetic and pluralisation on it ~900 lines away. The Set-ness is also load-bearing: the dedup is why a callee invoked from two hops is not double-counted, which is what makes the note's count correct. Comment-only. Verified mechanically: every added and removed line in `git diff -U0` matches a comment pattern, so the note's template literals are untouched and its rendered text is byte-identical. 89 tests unchanged. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(mcp): collapse the ascent plumbing accreted across 13 fix commits Quality cleanup, no behavior change. Four independent review passes converged on the same root cause: thirteen commits each fixed one review finding in isolation, and the ascent facts grew one loose field at a time until 62% of the changed region was comments explaining plumbing. Five changes: - `calleeIdsFromBlocks` deleted. Zero call sites anywhere in src/ or test/ — already dead on main, and this branch had edited it to keep it compiling. Its only reference was a stale `{@link}` in a neighbour's doc, now rewritten to stand alone. - `parseCalleeIdsCell` replaces the two-pass read. `calleeIdsWereTruncated` and `splitCalleeIds` were splitting the same cell on adjacent lines, which measured ~2x the parse cost (0.82 -> 1.59 ms at a realistic hop, 57.7 -> 92.7 ms at the per-statement site cap) and was a second independent encoding of the sentinel format — exactly what `splitCalleeIds` was extracted to prevent. One pass classifies as it walks; `splitCalleeIds` stays as a wrapper so its two external callers are untouched. The single-use `export` is gone. - `AscentCoverage` replaces four fields threaded through three signatures. ~12 declaration sites become 3, and the canonical rationale now lives on the type by construction — which is why the earlier doc-consolidation commit was needed at all. - `calleesReturnFlowing` becomes a boolean. Its only reads were `=== 0`, twice; it cost a Set sized to every callee in the slice plus a per-hop union loop. The flag is set inside the existing `returnFlowing.size > 0` branch — equivalent, since the cross-hop union is non-empty iff some hop's was. - The duplicated empty-ascent note head is collapsed to one gate and one head with per-arm tails. Both arms had been edited in lockstep twice in this branch's own history. The rendered note text is byte-identical. Verified structurally and then empirically: both expressions reconstructed standalone and diffed across the full cross product of references x returnFlowing x undecodable x truncated x listTruncated — 288 combinations, 0 mismatches. Net -53 lines. 102 tests pass unedited; the unused-symbol lint warning is gone. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(mcp): parallelise the startup probes, drop a redundant pin, name the mock knobs Quality cleanup from the same review passes. The set of verified behaviors is unchanged except where noted. Startup probes run concurrently. `spawnSync` blocks the event loop and vitest runs a file's tests in order, so the three probes strictly serialised. Launching all three with async `spawn` in `beforeAll` and asserting over the collected outcomes cuts the file from ~12.7 s to ~3.9 s wall (-69%). Every promise is caught before `Promise.all`, so all three children are reaped and failures report per entry rather than surfacing only the first rejection. Preserved and each proven by mutation: the missing-dist error names its entry, a raised module floor fails only its own row, and a bogus anchor still reports the loaded-module count. The two `it.fails` rows are removed. They pinned the inference-typed receiver gap that the strict `toEqual` pin beside them already covers — and they were the weaker of the two, because `it.fails` passes when the body throws for ANY reason, including `idsFor`'s own non-vacuity guard. A renamed fixture marker would have kept them green on a rotted premise. The strict pin is self-diffing and was verified load-bearing on its own: pointing a known-gap marker at a resolving shape fails it with the two newly-present ids listed. The file header now carries the gap's durable description. The ascent-note mock takes options objects. `descentExec` and `run` had grown to five and seven positional parameters in the order five agents added them, so call sites read `run(FILE, true, null, 3, false, undefined, null)` — several carrying `undefined` purely to reach a later argument. All 34 call sites are converted; nine that used only defaults are now bare `run(file)`. No knob renamed — they are orthogonal and correctly named. Code lines are exactly neutral (353 -> 353); the win is at the call sites. Also refreshes five comments that still described `calleesReturnFlowingSeen` and the two-branch note, both of which the preceding commit replaced. 102 unit and 10 integration tests pass; test count moves 9 -> 7 in the chained-receiver file, exactly the two redundant rows. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(mcp): publish return-value-ascent coverage on the PDG impact result `impact(mode:'pdg')` computed four facts about ascent coverage and used them exactly once — to interpolate an English sentence. They never reached the result object, so an agent consuming this MCP output could only ask "was the ascent complete, and if not why" by regexing prose. The cost was already demonstrated: a pure rewording commit earlier in this branch broke ~30 assertions and would have silently broken any consumer keying on the old phrase. Adds `pdgEvidence.ascent`: referencesScanned how many call-site callee references were scanned returnFlowFound did the ascent fire anywhere in this slice undecodableSummaryCount summaries the codec could not decode examinedComplete was the examined set the whole callee list incompleteReasons 'traversal-truncated' \| 'callee-list-capped' callSummaryLayerPresent false => pre-FU-C (v3) index Nested under `pdgEvidence` because that is the established counts-and- classification namespace, and `composeUnifiedPdgImpactResult` already spreads it, so the member survives the unified compose untouched. `incompleteReasons` carries CODES, following the existing `truncatedByReasons: ('depth'\|'limit')[]` precedent. The prose clause and the structured field now render from one array computed once, so an agent branching on codes and a human reading the note cannot disagree, and a third reason becomes a rendering decision rather than a contract change. Two shape decisions worth recording. `callSummaryLayerPresent` exists because without it a v3 index publishes `referencesScanned: N, returnFlowFound: false`, which reads as "these callees record no return-flow" when the truth is "the layer that records it is absent" — the note already distinguishes those, and the structured surface must not be less honest than the prose. And the field is ABSENT rather than zeroed when the descent never ran (upstream slices): "nothing was scanned" is a different fact from "we scanned and found nothing". `pdgResultVersion` stays 2. The documented trigger is a BREAKING change to the result shape; this removes nothing, renames nothing, and changes no existing field's meaning. Confirmed mechanically: zero top-level key drift across 2304 cases. The historical v2 bump was for changing an existing field's semantics (startLine 0- to 1-based). The note prose is byte-identical, proven across the same 2304 cases with a negative control — perturbing one character of the phrase table produces 60 drifts, so the harness demonstrably detects what it asserts. 14 new tests cover the structured surface and all 14 fail when the source is reverted, while the 54 prose tests pass unchanged. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(helpers): share one module-load probe, and fix two guards that passed on broken builds Three tests independently spawned a child node process to inspect what a built `dist/` entry loads, duplicating the REPO_ROOT derivation, the probe source, the missing-dist guard, the spawn with NODE_OPTIONS cleared, the status-vs-signal rendering, and the payload parse. The newest copy was also the only correct one, so the next author had 2-in-3 odds of copying a weaker probe. The two older probes diff `require.cache` only, which is structurally blind to the first-party ESM `dist/*` graph. That is not theoretical — both were demonstrated passing on genuinely broken builds: - Severing `dist/cli/mcp.js -> stdio-context.js` (a pure ESM change) leaves the require.cache diff EMPTY, so `import-closure.test.ts`'s two assertions reduce to `[].filter(...) === []`. It reported 2 passed on a severed graph. - Severing `registry -> swift/query.js` leaves 76 unrelated CJS entries, which satisfied `registry-import-closure.test.ts`'s indirect guard. The Swift half of its headline had gone vacuous and it reported 1 passed. Both now fail on those same builds, naming the missing anchor. `test/helpers/module-load-probe.ts` unions the ESM `registerHooks({ load })` channel with the cache diff, probes entries concurrently, and makes non-vacuity STRUCTURAL: `anchor` and `minModules` are required fields and the helper throws when either fails. A vacuous probe is a harness failure, not a silently green test, so it cannot be forgotten. Forbidden patterns and remedy text stay per-test — the harness is the shared part, the policy is not. Also fixes `toRepoRelativePosix` resolving non-absolute specifiers against `process.cwd()`, and dedupes modules a CJS-from-ESM import reported once per channel. Faster despite doing more: the registry file goes 12.4s -> 6.75s, because `spawnSync` burned the parent thread polling while the child loaded native grammars. `import-closure` drops to one spawn from two. The `local-backend.js` entry is kept although its closure is currently a strict subset of `server.js`'s: that is an observation, not an invariant. If `server.js` ever stops eagerly reaching the local backend, the server probe stays green while the module #2802 actually changed goes unobserved — and now that anchors are mandatory, that entry is what pins `pdg-impact.js`. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> docs(lbug): trim the csv-generator note and fix the claim it got wrong Two reviewers split on this comment: one wanted it cut to the structural argument, the other said a comment is the right depth for documenting a rejected change since there is no invariant to guard. Both are right, so it stays a comment and gets shorter — 13 lines to 6. Trimmed because it had already taken two corrections (an unreproducible "~40x" figure, and a pointer to a test file that no longer exists), and its tail had drifted from its own guard: the comment said "several hundred modules, ~150 ms" where `startup-language-closure.test.ts` says "~226 extra modules and ~130 ms". Two numbers for one fact. That tail is documented better in the guard's own header, so deleting it loses nothing. It also stated the load-bearing claim inaccurately. The old text said bm25-index imports `normalizeFtsText` "from here" — but `lbug-adapter.ts` neither exports nor re-exports it; the only occurrence of the identifier in this file WAS the comment. Anyone verifying would have grepped, found nothing, and concluded the note was stale. Now names `csv-generator.js` explicitly, re-verified at `bm25-index.ts:15` (static) and `local-backend.ts:2756` (dynamic, on the FTS query path). Comment-only, proven two ways: every changed line matches a comment pattern, and stripping all `//` lines from HEAD and from the working tree yields byte-identical text. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(helpers): extract the temp-repo lifecycle, collapsing five hand-rolled cleanups into one Four cfg integration tests each hand-rolled a `tmpDirs` array, a mkdtemp-and-register step, and an `afterAll` rmSync. It is actually five registrations across six creation sites — `pipeline-pdg.test.ts` keeps a second pool for its C-family fixtures. Seeding genuinely varies four ways (recursive cpSync, single copyFileSync, inline mkdir+writeFile, and nothing at all), so a fixture-copier helper would have fitted about half the sites and made things worse. Extracted the LIFECYCLE instead — mkdtemp, register, afterAll cleanup — which is byte-identical at all five registrations and is the correctness-critical part. `dir()` returns an empty registered directory for callers that seed themselves; `fromFixture()` covers the common case. That fits 6/6. The duplication had already produced a latent defect: `cFamilyTmpDirs` was cleaned by TWO `afterAll` blocks, harmless only because `rmSync` was called with `force: true`. Now one hook. `createTempDirPool` is a function called from each test file's module scope rather than a top-level hook in the helper, because under ESM caching a module-level `afterAll` would register once, against whichever file imported it first. That hazard is documented in the helper. Raw line count is roughly neutral (-44 across the tests, +62 for the helper, 29 of which are the rationale). The win is that a cleanup invariant went from five copies to one. Cleanup verified empirically, including the failure path: a throwaway suite whose `beforeAll` throws still has its directory removed, and every temp directory created by the four migrated files is gone after a run. 46 tests pass across the four files. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(resolvers): pin the inference-typed field receiver gap at the resolver level The gap was pinned only in a PDG test, asserting on `BasicBlock.calleeIds` behind the full `--pdg` pipeline. But it is a resolver fact: when a class field's type must be inferred from its initializer, chained receiver calls resolve to nothing. Whoever closes it will be working in the resolver suite and would have got a red CFG/PDG test with no resolver-side signal. Asserts CALLS edges directly, alongside `python-constructor-field-receiver.test.ts`. Nine receiver shapes run the identical statement; seven resolve, two do not: const o = new Outer() resolves private p: Outer = new Outer() resolves private p: Outer; this.p = new Outer() resolves private p: Outer; this.p = p (ctor arg) resolves constructor(private p: Outer) {} resolves makeOuter().inner().compute() resolves o.inner().mid().compute() (three links) resolves private p = new Outer() NO EDGES private p; this.p = new Outer() NO EDGES Two things the fixture establishes that the PDG-side pin could not. The discriminator is the type ANNOTATION, not local-versus-field — the parameter-property form resolves fine. And the initializer is NOT invisible to the resolver: `new Outer()` still emits its own constructor CALLS edge, byte-identical to the annotated twin. Only the initializer-to-field-type binding is missing, which narrows where a fix belongs. Assertions key on exact node ids rather than names, because `compute` is ambiguous across two classes and keying on the source name collides with `Object.prototype.constructor`. No `describe.skip` and no `it.fails` — the latter passes when the body throws for ANY reason, so it can go green on a rotted premise. The gap is pinned as its explicit current value, which self-diffs: simulating the fix fails one test showing the two newly-resolved ids, and renaming a fixture symbol fails the non-vacuity guard. Runtime is comparable to the PDG-side pin (~9-11s, both dominated by worker startup), so this is an altitude and scope win, not a speed one. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(mcp): replace the extension sweeps with a stronger language-agnosticism pin Two `it.each` sweeps over nine file extensions asserted that the empty-ascent caveat was present (or absent) for each. They looked like the pin for the property the whole change exists for — `pdg-impact.ts` must name no language and its output must not vary by extension — but they were the weakest available form of it. They asserted substring presence/absence, so a language dependence that ADDS text while leaving the caveat intact passes them. Demonstrated, not assumed: injecting a `.py`-only hedge inside the caveat sentence and replaying the two sweeps verbatim against that source gives 18 passed. The byte-identity test beside them caught it. So the sweeps are deleted and the identity test carries the property alone, hardened in two ways: - Two rows instead of one, covering BOTH sides of the caveat gate. The silent (return-flow present) branch previously had no identity counterpart at all — nine runs proving one fact, with nothing checking that its rendering was extension-invariant. - The fingerprint spans the note AND the reachable blocks, not just the note. Strictly more than the sweeps verified. Entailment is exact: identity across the extension set, plus the two existing single-extension content assertions, gives "every extension gets the caveat" and "no extension gets it". Reducing a sweep to one extension was rejected because it reproduces an assertion already present verbatim. Also converts the incompleteness block from six near-identical bodies to a 3-row premise table crossed with two assertions. Each row now names the exact phrase set its clause must contain, so presence and absence are asserted together — which adds three checks the longhand version lacked (the budget row now also proves the emit-cap phrase is absent). And three tests that re-rendered one fixture to make one assertion each are hoisted to a single render. 97 tests, down from 116: -18 sweep cases, -2 from the hoist, +1 identity row. No assertion was lost; several were added. Verified by injection: a `.py`-only note change fails the identity pin, and a dependence in the shared hop sentence fails BOTH rows, confirming the second row is load-bearing rather than decorative. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * perf(mcp): lazy-import syncGroup so MCP startup skips the group extractor closure `core/group/service.ts` statically imported `./sync.js`, which pulls all six contract extractors, five of which statically import the native `tree-sitter` binding. That put the whole parser stack on every MCP server start, for a server that never syncs. Only `groupSync` needs it. The other seven group tools — `group_list`, `group_impact`, `group_query`, `group_contracts`, `group_status`, `group_trace`, `group_context` — do not, and now never load it. `syncGroup` has a single call site, already inside an `async` method, so this is a lazy `await import(...)` at that call site and nothing else: no signature change, no async ripple, no change to `local-backend.ts`. The pattern is already established on this exact module — `cli/group.ts`'s sync command lazy-imports `sync.js` the same way. `service.ts` was the outlier. Measured on a native filesystem (overlayfs; /workspace is a 9p mount that inflates ESM resolve, so it is not a valid measurement surface), 5 cold runs, medians: dist/mcp/server.js 521 ms -> 133 ms (-75%) dist/mcp/local/local-backend.js 453 ms -> 66 ms (-85%) tree-sitter modules at both entries: 11 -> 0 Same defect class as #2802, which cut the language-provider registry from the same startup path; this is what remained. The cost is moved rather than deleted: the first `group_sync` call now pays the module load. That is the right trade — `group_sync` is already a long-running operation, and sessions that never sync pay nothing. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(mcp): guard MCP startup against the group extractor closure returning Sibling forbidden-pattern case in the #2802 startup guard, reusing the concurrent probes it already collects — no new spawn, no new harness. Asserts that none of `dist/mcp/server.js`, `dist/cli/mcp.js`, or `dist/mcp/local/local-backend.js` loads a `core/group/extractors/` module or the native `tree-sitter` package. The parser is matched by package prefix rather than a bare substring, so a source file that merely mentions the word can neither satisfy nor trip it. Verified load-bearing rather than assumed: restoring the static `import { syncGroup }` in `core/group/service.ts` and rebuilding turns `dist/mcp/server.js` red and names all seven offenders — http-route, grpc, thrift, topic, include, manifest and workspace extractors. Reverted and re-confirmed green. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * perf(mcp): keep the analyze-only CFG closure off MCP server startup (#2802 review) `mcp/local/pdg-impact.ts` imported `CALLEES_TRUNCATED_SENTINEL` and `CALLEE_ID_SEP` from `core/ingestion/cfg/emit.ts`. ESM evaluates a module to import any binding from it, so those two strings dragged the whole analyze-only CFG closure into every MCP server start. Measured against a clean build, per entry point: 8 modules — `emit`, `reaching-defs`, `reaching-defs-graph`, `control-dependence`, `post-dominators`, `synthetic-escape`, `call-site-harvest`, `reaching-def-reason-codec` — present at `dist/mcp/server.js`, `dist/mcp/local/local-backend.js` and `dist/mcp/http-transport.js`. Same defect class as the language-provider closure this branch already removed, and the guard could not see it: `FORBIDDEN_RE` covers `core/ingestion/languages/` and `FORBIDDEN_GROUP_RE` covers `core/group/extractors/\|node_modules/tree-sitter`, neither of which matches `core/ingestion/cfg/`. The format constants move to a new LEAF module `cfg/callee-cell-format.ts` that imports nothing; `emit.ts` re-exports both names so every existing importer is untouched, and producer and consumer still resolve to one definition — the drift the shared constant exists to prevent stays impossible. Deleted, not deferred — the same bar #2802 held its own csv-generator proposal to. After: cfg modules at startup 8 -> 2, and both survivors (`callee-cell-format`, `reaching-def-reason-codec`) are leaves that import nothing. Totals: `server.js` 387 -> 380, `local-backend.js` 163 -> 156, `http-transport.js` 523 -> 516. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(mcp): stop pdgEvidence.ascent claiming a completeness it cannot have (#2802 review) `examinedComplete` is the field a consumer reads to decide whether `returnFlowFound: false` is a whole-slice claim. It could be published `true` over a callee set the descent never finished examining — the exact false all-clear the field was added to prevent. Root cause: `bfsReachableBlocks` sets `truncatedByDepth` when its frontier is still non-empty at the budget, but both call sites inside `interproceduralDescent` folded only the row-limit flag and dropped the depth flag. The top-level intra BFS's copy of that same flag was already propagated, so the asymmetry was unintended — one `if`-pair folding limit-but-not-depth, within a merge that already folds the node cap too. Reproduced at `maxDepth: 3`, the shipped default: a criterion calling a helper whose body is a 5-block dependence chain, with the return-flowing callee on the block past the clamp. Result reported `truncated: undefined`, `examinedComplete: true`, `incompleteReasons: []` and an unqualified universal note sentence. Fixed by propagating the dropped flags rather than inventing a parallel channel: `intraDepthBudget` is documented in-file as the SAME clamp the top-level intra BFS applies, and that one's depth truncation is already result-level. So the result's own `truncated`/`truncatedBy` were under-reporting for the same reason, and both surfaces are corrected together. Four further honesty fixes to the same published record: - Blocks reached only by the U-C4 ascent went into `reachable` but never `hopReached`, so their `calleeIds` cells were never scanned, never counted, and could not raise `callee-list-capped`. They are slice blocks; they now enter the hop set and get the same treatment as every other one. - `pdgEvidence.ascent` was absent on the empty-slice early return even though the descent had already run and scanned, contradicting the "present iff the descent ran" contract this branch itself added to `tools.ts`. Both exits now classify through one shared helper so they cannot disagree. - A block carrying call sites in `callees` but no resolved ids in `calleeIds` (the whole-file case where `emit.ts` has no fileMap) silently shrank the population while `examinedComplete` still reported `true`. That now raises a third reason, `callee-ids-unrecorded`. - `referencesScanned` is a distinct-callee tally and both surfaces described it as a call-site count. Field name kept — a rename is breaking at `pdgResultVersion: 2` — and the prose corrected instead. `PdgAscentIncompleteReason` gains a member, which is additive, so `pdgResultVersion` stays 2. Visible output change worth knowing: slices whose callee chain outruns `maxDepth` now report `truncatedBy: 'depth'` where they previously reported none, and a repo with id-less call sites now reports `examinedComplete: false`. Both are strictly more honest. Every behavioural change carries a mutation proof — revert the source, watch the new test go red, restore. One exception is documented inline rather than faked: the ascent-side fold cannot be observed independently, because the re-seed shares the caller's `visited` set and so can only reach past the budget when the traversal that covered that closure was already cut and had already raised a flag. Suite: 49 -> 59 tests. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(mcp): anchor each import-closure policy on the edge it polices (#2802 review) `module-load-probe.ts` makes non-vacuity structural via a required `anchor` — but the anchor was one per ENTRY while `startup-language-closure.test.ts` now runs TWO independent policies. The group-extractor policy added in `83e8cf7c5` therefore had no anchor of its own, and one of its three rows was already vacuous: `cli/mcp.js` loads four leaf modules and reaches no `core/group/` module at all, so its group assertion could not fail for any policy-related reason while its `dist/mcp/stdio-context.js` anchor stayed green. Proven, not argued. `dist/mcp/local/local-backend.js` is the only static importer of `core/group/service.js` in the whole build; severing that one edge — the exact next lazy-load step — and re-probing: OLD shape (anchor per entry): server 385, http-transport 521, local-backend 161 reaches group/service = false, group offenders 0 -> GREEN on all three NEW shape (anchor per policy): -> RED on all three, each naming the missing dist/core/group/service.js Counts fell only 387->385 and 163->161, so `minModules` was structurally blind to the severance; the anchor is the only thing that catches it. `anchor` accepts `string \| readonly string[]` and every listed anchor must load. Existing single-anchor call sites are unchanged. `anchorsOf()` lets the group `it.each` DERIVE its entries by filtering on the group anchor, with a test pinning that derivation, so the policy cannot silently register zero cases. `cli/mcp.js` is dropped from the group policy — it cannot honestly carry that anchor — and the doc-comment now states the invariant: an anchor is per-POLICY, not per-entry. Also: - `mcp/http-transport.js` gets a row. It is the largest startup entry (516 modules) and `src/cli/mcp.ts` imports it directly rather than through `server.js`, so nothing about the server row constrained it. Measured clean today; the gap was coverage, not a broken claim. - The three spawn-based closure tests are registered in `SPAWN_CLI`, so the Windows-safety plumbing this branch wrote for them (POSIX normalisation, `pathToFileURL`, `NODE_OPTIONS` clearing, array-form `spawn`) is finally exercised on the Windows/macOS matrix. Measured cost ~11.7s on Linux; budget ~60s on Windows against a 25-minute job. - `PROBE_TARGET` now wins over `extraEnv`, which was spread last and could have silently redirected a probe while `anchor`/`minModules` stayed keyed on `entry`. - The child's JSON payload is validated through a type predicate instead of a bare `as string[]`, and the spawn timeout escalates SIGTERM to SIGKILL so a child stalled in native code is reaped rather than orphaned. - Recorded baselines re-measured (server 380, local-backend 156, cli/mcp 4) and relabelled a snapshot rather than a contract — they moved twice inside this branch alone. The subset claim was re-verified exactly: 0 of local-backend's 156 modules are absent from server's 380. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(helpers): survive a failing temp-dir removal instead of leaking the rest (#2802 review) `createTempDirPool`'s `afterAll` ran a bare `for (const d of created) fs.rmSync(d, {recursive, force})`. `force` suppresses only `ENOENT` — not the `EBUSY`/`EPERM`/`ENOTEMPTY` class a Windows runner produces when a pipeline test still holds a handle — so the FIRST failure threw out of the loop and leaked every directory registered after it. Pre-existing: all four hand-rolled cleanups this helper consolidated had the same shape. But the blast radius is now shared across four consumers, which is exactly why it is worth fixing at the point of consolidation. Cleanup is now per-directory best-effort via `removeTempDirs`, plus Node's own documented mitigation for that error class (`maxRetries: 3, retryDelay: 50`), which costs nothing on the happy path. Warn rather than swallow or rethrow, and the reasoning is in the doc comment, not just here: rethrowing would fail an otherwise green suite from `afterAll` over housekeeping the OS reclaims anyway, where it reads as a test failure and buries the real result — a Windows EBUSY on a temp dir is not a defect in the code under test. Silence is the opposite hazard: a systematic leak would be invisible with nothing naming the responsible suite. The warning carries the path, and the `mkdtemp` prefix is per-pool, so it names the suite that made it. Failure is injected through a scripted remover keyed by path (a Map lookup, so no `if` in a test body and no dependence on producing a real locked handle). Beyond the three behavioural pins there is a wiring pin — a nested `describe` creates a real pool and a sibling `it` declared after it asserts the dirs are gone — so the tested function cannot drift into "tested helper plus an untested copy of the loop". Mutation proof: restoring the abort-on-first-failure loop turns 3 of the 5 tests red, the throw escaping `removeTempDirs` outright so the third real directory is never attempted. With the fix, `[first, blocked, last].map(existsSync)` is `[false, true, false]` — the injected failure survives and the directory after it is really gone, through the remover that actually ships. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(pdg): point the self-diffing receiver pins at #2807, not at this PR (#2802 review) Both pins named the gap "(#2802 follow-up)". The gap has its own tracking issue — #2807, "Inference-typed field receivers resolve to no CALLS edges at all" (open, labeled bug) — and PR #2810 is already open against it. As written, after merge the gap was discoverable only by reading a KNOWN GAP marker inside a test file, not from the issue tracker. Both describe names now read "(known gap: #2807)" and both KNOWN GAP test names carry the number. #2802 is kept only as provenance: the gap was FOUND during #2802 work but is pre-existing and independent of it. Each header gains an explicit "this pin is self-diffing: it will go red on purpose" section naming #2807 with its exact title, noting #2810 is open against it at the time of writing, and stating that the pin asserts the gap EXISTS — so closing #2807 fails it by design, and the correct response is to update the expected value, not to relax the assertion. The same note is repeated inline above each KNOWN GAP test, where a maintainer editing it will actually see it. No pin is weakened. Both deliberately reject `it.fails` in favour of exact `toEqual` assertions with a non-vacuity probe, and that design is left untouched. Refs #2802, #2807 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(group): cover the lazy syncGroup import that no test reached (#2802 review) `9ea9676dc` turned `GroupService.groupSync`'s `syncGroup` into `await import('./sync.js')` — this branch's one changed control-flow line in production code — and nothing exercised it. Every existing test stopped short: `service.test.ts` returns at the empty-name guard; `group-service-not-found.test.ts` mocks `loadGroupConfig` to reject and never invokes its `syncGroupMock`; `group-sync.test.ts` imports `syncGroup` directly, bypassing `GroupService`; and the startup guard asserts only the negative, that `sync.js` is absent at startup. `tsc` catches a path typo, but nothing verified the import resolves and hands off correctly — while every production `group_sync` call goes through that line. No production change was needed; the reviewed design was sound. This is the missing coverage. The happy-path test mocks nothing: it points `GITNEXUS_HOME` at a pool temp dir, seeds a real `group.yaml`, and calls `groupSync`, so `loadGroupConfig` resolves, `groupDir` is found, and execution falls through into the REAL `syncGroup`. What makes a real sync reachable with no indexed repo: an empty registry puts both members in `missingRepos`, but one declared manifest link still yields synthetic-UID contracts. It asserts the returned counts AND reads back the `contracts.json` that real `syncGroup` wrote into `groupDir` via the production `readContractRegistry`, which pins the option handoff too. Two further tests use `vi.doMock` to re-evaluate the service against a `sync.js` whose load throws: one asserts the call rejects with the load failure in its `cause` chain — so the caller gets a catchable rejection, not a floating unhandled one — and one asserts both pre-import guards still answer with `sync.js` unloadable, which is also a structural pin that the module has no STATIC import of it (a static one would throw at re-import, before any call). Mutation proofs: pointing the specifier at `./sync-nope.js` turns 2 of 3 red ("Cannot find module .../sync-nope.js ... at GroupService.groupSync service.ts:349"); aliasing a real-but-wrong export turns 1 red. Restored, all 3 green, and `service.ts` verified byte-identical to HEAD. Out of scope, stated rather than glossed: the final `isError: true` MCP envelope is produced above `GroupService` and needs a full `LocalBackend`; the rejection test is the in-scope half of that claim. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(mcp): close the gaps a cleanup pass found in the #2802 review fixes Quality pass over the review-response series (reuse / simplification / efficiency / altitude). No behaviour change except where noted. The two that mattered: - The cfg/emit fix had no guard. `FORBIDDEN_RE` covers `core/ingestion/languages/` and `FORBIDDEN_GROUP_RE` covers `core/group/extractors/\|tree-sitter`; neither matches `core/ingestion/cfg/`. Because `emit.ts` re-exports the constants, pointing `pdg-impact.ts` back at `cfg/emit.js` typechecks identically and silently restores all 7 modules. Verified: with the import reverted, `tsc --noEmit` still exits 0 and every test stayed green before this commit; after it, 3 rows go red naming the offenders. Written as an ALLOWLIST of genuine leaves rather than a denylist of the 7 already-suffered modules, because the next regression is a module nobody has thought of yet. - `FORBIDDEN_GROUP_RE`'s parser matcher was forward-slash only while both sibling probe regexes spell the separator `[\\/]`. Native bindings arrive via the `require.cache` channel as absolute paths and `toRepoRelativePosix` only normalises paths inside the repo root, so a hoisted `node_modules` renders as `…\node_modules\tree-sitter\…` on Windows and matched nothing. The same series put this file on the Windows matrix, where that half of the assertion would have been vacuous. Reuse — three re-implementations of existing helpers: - `removeTempDirRecursive` re-rolled `fs.rmSync` retries; it now delegates to `cleanupTempDirSync` (`test-db.ts`), the repo's Windows-lock-aware remover. The copy had already drifted on both knobs that matter — 3 retries at 50 ms vs 5 at 100–400 ms, and warn-on-everything vs swallow-lock-codes-rethrow-rest — which is how one half of a suite goes green-with-a-warning on the same `EBUSY` the other half fails on. The per-directory try/warn loop, which is the actual fix, is unchanged. - `errorChainText` re-rolled the cause-chain walk that `causeChain` (`src/lib/utils.ts`) exists to be the single copy of — its own doc asks callers not to. - The SIGKILL escalation (a timer, an `unref`, and two `clearTimeout`s) is `spawn`'s own `killSignal` option, which Node's `timeout` already delivers. Simplification and altitude: - `'callee-ids-unrecorded'` documented ONE of its three producer paths. The unnamed common one is a call site that did not RESOLVE — exactly the receiver gaps this repo pins (#2807) — so on a real index the reason fires broadly, driven by resolution quality rather than a missing `--pdg` layer, and "re-run analyze --pdg" is the wrong remedy for it. Doc now names all three and states the consequence: `examinedComplete: true` is the strong, rare signal. - The derived policy-entry list was re-pinned against a hand-written 3-element literal, reinstating one layer down the list the derivation removes. Now asserts the properties that are actually at risk — non-emptiness (a policy going silent) and `cli/mcp.js` staying excluded (a row that cannot fail). - A test fixture spread `ascentBlockCell: 'idless'` and then overrode it to `'capped'` in both runs, so the id-less shape never reached the mock while reading as though it did. - `idlessCallSites` is sticky, so its per-row string allocation now short-circuits once set. - Dropped an unused `export` on `CleanupWarner`. Refs #2802 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * revert(ci): unregister the module-load closure guards from the Windows matrix Registering the three `dist/` closure guards in `SPAWN_CLI` turned the Windows `platform-sensitive 1/3` shard red at the 20-minute watchdog. Baseline `83e8cf7c5` was green on all three shards; `a4245119c` (which added them) failed 1/3; `d0b201442` failed the same way. It is not the files themselves. On the Windows runner they are among the cheapest in the suite — `registry-import-closure` 448 ms, `import-closure` 53 ms — and both passed. vitest shards this list by file COUNT, not runtime, so adding three files RESHUFFLED the split: shard 1 went to 32 files against 26 and 29, concentrating the heavy CLI e2e suites. It timed out with `cli-e2e`, `group/cross-trace-e2e`, `lbug-orphan-sidecar-recovery` and `server-http-startup` still queued — `cli-e2e` being the ~50-spawn suite whose setup flakiness already needed fixing once (PR #2000). That clustering fragility is pre-existing and this file's own header documents it (#2449: "the heaviest spawn suites can cluster on one shard", busiest Windows shard already at 14m57s against the old watchdog). These three files only tipped it over, and unblocking the PR beats holding it for a CI-infra fix that belongs in its own change. Reverted rather than worked around: raising the shard count would keep the coverage but is a repo-wide CI change made on a 25-minute feedback loop with no guarantee the reshuffle balances, and this PR is about MCP startup. The removed entries are replaced by a comment recording WHY they are absent, what they were measured to cost, and the precondition for re-landing them — so the gap is documented at the point someone would otherwise re-add them blind. Verified: the emitted file list is byte-identical to 83e8cf7c5's, so the shard split returns to the configuration that was green. The Windows-specific bug this series found is unaffected — `FORBIDDEN_GROUP_RE` now spells its separator `[\\/]` like its siblings, which was a real forward-slash-only vacuity, and that fix stays. Refs #2802, #2449 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(ci): shard the cross-platform matrix by measured weight, not file count Restores the three `dist/` module-load closure guards to the Windows/macOS matrix, and fixes the reason they could not stay there. They must run on every OS — the shared probe in `test/helpers/module-load-probe.ts` IS the platform-varying code (array-form `process.execPath` spawn, cleared NODE_OPTIONS, `pathToFileURL` because Windows rejects a bare absolute path as an ESM specifier, and a `path.sep`→POSIX normalisation the anchors and offender regexes depend on). Ubuntu-only coverage of a platform guard is no coverage. The earlier attempt turned Windows `platform-sensitive 1/3` red at the 20-minute watchdog, and the reflex fix — unregistering them — treated the symptom. The files are among the cheapest in the suite (measured 448 ms, 53 ms, sub-second, and both that completed passed). The defect is that `run-cross-platform.ts` handed vitest all 84 files plus `--shard=i/n`, and vitest partitions by file COUNT. Runtimes here span three orders of magnitude, so a count-split is blind to the thing that decides the budget, AND re-partitions on every insertion: adding three free files reshuffled the list and happened to co-locate `cli-e2e` (361 s) with `cli-limit-e2e` (75 s) and `analyze-heap-oom-e2e` (23 s) — 32 files against 26 and 29 — which timed out with four still queued. The split now happens in `scripts/cross-platform-shard.ts`, longest-processing- time first over measured Windows runtimes, and only the chosen shard's files are passed to vitest (`--shard` is consumed, never forwarded — forwarding would re-partition the slice a second time and silently drop most of it). Weights are measured, from the last green matrix run plus the timed files of the failing one, and every file also carries an 8 s per-file floor. That floor is calibrated, not guessed: the last green busiest shard ran 736 s of wall clock over ~511 s of attributed file time. Without it the balancer isolates the two monsters and then piles every light file onto the remaining shards — trading a runtime imbalance for a count imbalance that costs the same. Result at TOTAL=3, with the three guards back in: 521 s / 527 s / 519 s across 20 / 33 / 34 files. The previous green configuration's busiest shard was 736 s, so this is better balanced than the state before any of this, and the busiest shard is now bounded by construction rather than by sort-order luck. `test/unit/cross-platform-shard.test.ts` pins the properties, and the load-bearing one is not "the split is even" — it is "adding a cheap file cannot move a heavy one", the property whose absence caused the outage. Two details in that test are themselves load-bearing, and earlier drafts got both wrong and were vacuous: the inserted names must sort EARLY (names sorting last disturb nothing under any scheme) and the count must not be a multiple of the shard total (adding exactly `total` files leaves an equal-weight round-robin in the same rotation). Mutation-proved: replacing `weightOf` with a constant — i.e. count-based sharding — turns that test and the per-file-floor test red; restored, all 8 pass. Refs #2802, #2449 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 21:26:13 +01:00
Gergő Magyar	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>	2026-08-03 15:04:30 +01:00
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 Details CodeQL / Analyze (python) (push) Waiting to run Details Gitleaks / gitleaks (push) Waiting to run Details Publish / Classify release event (push) Waiting to run Details Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions Details Publish / ci (push) Blocked by required conditions Details Publish / Publish to npm (push) Blocked by required conditions Details Publish / Build & Push RC Docker images (push) Blocked by required conditions Details Scorecard / Scorecard analysis (push) Waiting to run Details Skill copy sync / shipped skills drift guard (push) Waiting to run Details Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run Details Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run Details * 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>	2026-08-02 20:43:59 +00:00
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>	2026-08-02 20:52:44 +01:00
Gergő Magyar	990d79ba8c	fix(mcp): make impact/context reproducible — deterministic ordering on every capped query (#2787 ) (#2796 )	2026-08-02 17:03:15 +00:00
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 `81daf370e` (to reproduce the `Namespace→Record` analyze abort) joined that corpus and shifted the fingerprint — 14 → 15 files. Verified corpus-only, not a capture change: with that one fixture moved aside the fingerprint is byte-identical to the prior baseline (d45bb091…), and `81daf370e` touches no COBOL capture code. The new value reproduces CI's reported hash exactly. Scaling 0.677 < 1.5 budget. `bench/scope-capture/measure.mjs --check` → PASS (15 languages). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0182jkjQqzACkJKYw4MLDnhX --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-02 16:26:25 +01:00
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` (`f1b843838`). Corrected in the bench header and the CI step comment. The accurate point is narrower and stronger: that bench asserts `callsResolved === 0`, so it never drives the qualified-receiver path, and it is `skipIf(!GITNEXUS_BENCH)` while the only step setting that variable lists neither C++ bench — so it has never run in CI. Wiring it in is a follow-up. - "Ordering is load-bearing" was not a live property: `allHits[0]` is only reached at length 1, and both tail branches return `'ambiguous'`. Order is still preserved for byte-identity with the pre-#2788 walker; the comment now says that instead, and the test named for ordering is renamed to the parent/inline-child visibility it actually asserts. - "Same contract as `ensureAdlIndex`" overstated parity — the sibling ships a `validateAdlSeqCoverage` guard because it reads `seqByNodeId.get(...) ?? 0`, which can silently collapse candidates. This index has no analogous defaulting read, so no guard is added; the comment now says why. - Two coverage gaps closed, both mutation-verified: cross-file merge of one namespace reopened in two files (no existing test covered it — confirmed by making each file clobber the previous and watching only the new test fail), and the same-name inline nest whose dedup, when defeated, flips a resolved def to `'ambiguous'`. - `resolveCppQualifiedNamespaceMember`'s JSDoc now names both production call sites, including the callsite-less `resolveAdlCandidates` path. - Memoized candidate buckets are frozen, so a future in-place sort in `overload-narrowing.ts` throws instead of silently corrupting later resolutions now that the array is shared across call sites. - `_scaling_note`'s "measured 0.93-1.21" band did not reproduce; it is now the honestly observed 1.28-1.45, with the small arm widened to ~14ms (halves the spread) and a triage line saying a scaling failure is a timing signal to re-run, unlike the deterministic fingerprint arm. Verification: 840,000 differential probes against the pre-#2788 walker extracted from base, 0 mismatches, plus 48,000 candidate-order comparisons, 0 mismatches. cpp resolver integration suite 334/334. Unit suite 10/10. tsc, eslint, prettier clean. Bench --check PASS. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(bench): escape the NUL separator so measure.mjs stays a text file The fingerprint key separator was written as a literal NUL byte instead of the `\u0000` escape. Git classifies any file containing a NUL as binary, so the whole bench showed as `Bin` with no diff on GitHub and could not be reviewed — the same defect this branch already fixed once before the tri-review. Escaping it is byte-for-byte equivalent at runtime (both produce U+0000), so the committed fingerprint is unchanged and `--check` still passes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(cpp): quality cleanups from a four-angle review of the #2788 series Reuse, simplification, efficiency and altitude passes over the diff. No resolution behaviour changes: the bench fingerprint is unchanged and the C++ resolver integration suite still passes in full. Efficiency - The `Object.freeze` added in the review-fix commit to close an aliasing residual costs 4.6x on the narrowing path — V8 moves frozen arrays to PACKED_FROZEN_ELEMENTS, off the fast path for the `.filter`/`.map`/`.some` runs `narrowOverloadCandidates` does at every multi-candidate call site. The hazard it guards is already a compile error (both the memo and the parameter are `readonly SymbolDefinition[]`), so it is now a dev-only tripwire. Gated on `isSemanticModelValidatorEnabled()` — the repo's opt-IN form used by `phase.ts` and `validate-bindings-immutability.ts` — not `adl.ts`'s opt-out `NODE_ENV !== 'production'`, which would keep paying the cost in CLI runs where `NODE_ENV` is unset. Large bench arm 100.3 -> 70.6ms. - The index build scanned every scope in every file twice; pass 1 now collects `[scope, node]` pairs for pass 2 to iterate. 800k scopes 14.40 -> 8.21ms. - `simpleNameOf` uses `lastIndexOf('.')` + `slice` instead of `split('.').pop()` (83 -> 24 ns/call), semantics verified byte-equivalent over 12 edge cases including `undefined`, `''`, `'a.'`, `'.b'` and `'a..b'`. - The per-call `hookCtx` literal is hoisted to a module const. - The bench's fingerprint pass resolved 960k call sites to produce 5,800 distinct outcomes; it now dedups on the key it already builds. Bench wall time 2.4 -> 1.9s, fingerprint byte-identical. Reuse - `bucketOwnMembers` used an inlined `Function \| Method \| Constructor` compare; it now calls the canonical `isOverloadableCallable`. `graph-bridge/ids.ts` already carries a note that inlining this list recreated twin-list drift once. Altitude - `adl.ts` had the same retention defect this series just fixed next door: a module-level `let adlIndex` + `let adlIndexSource` strongly pinning the whole `parsedFiles` array until the next C++ pass, which in a single analyze never comes. Converted to the same `WeakMap` shape. Measured with `--expose-gc`: 89.11MB retained after the caller drops the array before, 0.17MB after. Six file-local helpers now take the index as a parameter; no exported signature changed. - The index's freshness depended on `clearCppInlineNamespaces()` being called from another file, guarded only by a warning paragraph. An epoch bumped in both `populateCppInlineNamespaceScopes` and the clear is now stored with the memo, so a missed clear degrades to a rebuild instead of a stale answer — confirmed by driving inline state mid-pass without the clear. Roughly line neutral, since it replaces most of the paragraph. - `test/integration/cpp-adl-benchmark.test.ts` is wired into the `GITNEXUS_BENCH` step. It is `skipIf`-gated and was absent from that step's explicit file list, so #1990's ADL emit-scaling guard had never executed in CI. It passes; ~50s added to a 25-minute job. `cpp-pipeline-benchmark.test.ts` is deliberately NOT wired: it costs 115s for guards covering generic per-language pipeline scaling that nothing here touches. Simplification - Deleted a comment referencing a `inlineChildrenByParent` map that only ever existed inside this branch's own first commit, so "the legacy map" pointed a reader at code that never shipped. - Dropped three unreachable `undefined` guards (`strict: false`, no `noUncheckedIndexedAccess`), keeping the load-bearing `visited` check. - Compressed the `rootsByReceiver` doc from 17 lines to 7 — it was the longest comment in the file and guarded the least consequential property — the `validateAdlSeqCoverage` paragraph from 7 lines to 3, and turned three restatements of the uncaught-throw and dedup arguments into pointers. - Test fixtures: dropped the dead `'Module'` union arm, added a one-line `ns()` builder for the nine hand-written scope literals, and moved the file to `test/unit/scope-resolution/cpp/` where every other C++ scope-resolution unit test lives. 403 -> 337 lines, same 10 tests, and the cross-file mutation check still fails exactly one test. Not done, and why: merging this index into `AdlCandidateIndex` (they key on different names with different inline-transparency depth — a refactor with correctness risk, not a cleanup); `ScopeTree.getChildren` (trades in-memory `parsed.scopes` for store hits on a hot path); a shared `cpp/` util for the five pre-existing `simpleName` copies; sharing fixtures across the bench/test boundary (no precedent in this repo); and an exact-count arm for the bench, which is a gate redesign worth its own change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-02 15:52:33 +01:00
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 Details CodeQL / Analyze (python) (push) Waiting to run Details Gitleaks / gitleaks (push) Waiting to run Details Publish / Classify release event (push) Waiting to run Details Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions Details Publish / ci (push) Blocked by required conditions Details Publish / Publish to npm (push) Blocked by required conditions Details Publish / Build & Push RC Docker images (push) Blocked by required conditions Details Scorecard / Scorecard analysis (push) Waiting to run Details Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run Details Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run Details	2026-08-01 22:42:18 +01:00
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>	2026-08-01 20:10:58 +00:00
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>	2026-08-01 20:24:06 +01:00
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>	2026-08-01 18:00:21 +00:00
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>	2026-08-01 18:31:49 +01:00
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>	2026-08-01 17:22:12 +01:00
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>	2026-08-01 12:42:36 +01:00
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>	2026-08-01 12:35:37 +01:00
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>	2026-08-01 12:18:07 +01:00
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>	2026-08-01 09:38:10 +01:00
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>	2026-08-01 08:47:14 +01:00
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>	2026-08-01 08:42:51 +01:00

1 2 3 4 5 ...

1826 commits