diff --git a/.github/workflows/ci-tests.yml b/.github/workflows/ci-tests.yml index b4fbb3d56..87d2c4377 100644 --- a/.github/workflows/ci-tests.yml +++ b/.github/workflows/ci-tests.yml @@ -500,6 +500,14 @@ jobs: run: node --import tsx bench/callable-value-flow/measure.mjs --check working-directory: gitnexus + - name: C++ qualified-namespace resolution guards (#2788) + # Build-free: asserts resolveCppQualifiedNamespaceMember resolves an + # unchanged symbol set (fingerprint) and that per-call-site cost stays + # independent of corpus size. Rationale and history: see the header of + # bench/cpp-qualified-ns/measure.mjs. + run: node --import tsx bench/cpp-qualified-ns/measure.mjs --check + working-directory: gitnexus + - name: Receiver-resolution drop guards # NOT build-free: this one runs the real pipeline, so it needs dist/ # (the setup action above builds). ~2m15s. @@ -566,12 +574,17 @@ jobs: working-directory: gitnexus - name: Cross-language pipeline benchmarks (GITNEXUS_BENCH, serial) + # cpp-adl-benchmark.test.ts is not a `*-pipeline-benchmark.test.ts` but + # belongs here for the same reason: it is skipIf-gated on GITNEXUS_BENCH, + # so it had never run in CI and the PR #1990 ADL emit-scaling guard it + # holds was dead. ~45s of test time. env: GITNEXUS_BENCH: '1' run: >- npx vitest run --no-file-parallelism test/integration/cobol-pipeline-benchmark.test.ts test/integration/csharp-pipeline-benchmark.test.ts + test/integration/cpp-adl-benchmark.test.ts test/integration/instance-ownership-pipeline-benchmark.test.ts test/integration/spring-bean-resource-benchmark.test.ts test/integration/rust-pipeline-benchmark.test.ts diff --git a/gitnexus/bench/cpp-qualified-ns/baselines.json b/gitnexus/bench/cpp-qualified-ns/baselines.json new file mode 100644 index 000000000..51ce5405d --- /dev/null +++ b/gitnexus/bench/cpp-qualified-ns/baselines.json @@ -0,0 +1,7 @@ +{ + "_comment": "Baselines for bench/cpp-qualified-ns/measure.mjs --check (#2788). `fingerprint` is a sha256 over every `receiver::member(arity|argumentTypes) -> outcome` the synthetic corpus resolves at the LARGE scale (hit nodeId, `` per #1564, or ``); it is a CORRECTNESS gate, so drift means C++ qualified `ns::member()` lookup started resolving a different symbol set and must be explained, never re-baselined to make CI green. THAT RULE IS UNCHANGED and applies to every future edit of inline-namespaces.ts. `scaling_budget` is a timing gate and carries deliberate headroom for shared CI runners.", + "_rebaseline_2788_review": "This fingerprint was moved ONCE, deliberately, during review of #2788 — because the bench CORPUS was expanded, not because a check failed. Do not read it as precedent. What changed: (1) receivers now mirror production — ~1 in 5 name a declared namespace, ~4 in 5 are plain identifiers naming none (`obj0`, `Widget3`, `buf12`). The previous corpus drew every receiver from `ns_${…}`, so the receiver lookup NEVER missed, while Case 1.5 in scope-resolution/passes/receiver-bound-calls.ts is reached by every plain-identifier receiver call and misses on the overwhelming majority. (2) A namespace reopened across two files (C++ namespaces are open — the cross-file merge property). (3) A same-name inline nest `namespace ns { inline namespace ns { … } }`, which is the only shape that observes `gatherQualifiedNsMember`'s `visited` dedup. (4) A member declared at BOTH the namespace level and in an inline child, selected apart by argument type, pinning both collection sources by nodeId. (5) Call sites carrying a real `Callsite`, without which narrowOverloadCandidates / cppConversionRank / isOverloadAmbiguousAfterNormalization were outside the fingerprinted surface entirely. Measured effect, same patched resolver, old bench vs new: removing the `visited` dedup — old PASS with a byte-identical fingerprint, new FAIL (fingerprint 1e6c51b9… != aba39c34…); resetting `visited` per root instead of across roots — old PASS, new FAIL on the same fingerprint. A PURE reorder of a candidate list still passes both, and correctly so: the resolver's return contract is order-blind by construction (see QualifiedNsMemberIndex's doc comment), so there is no behaviour there to gate.", + "fingerprint": "aba39c342ce536006bebded8b32260dc7807487be91f5c7ee9548f9e9283f9c9", + "scaling_budget": 1.8, + "_scaling_note": "(t_large/t_small)/(1600/400). ~1.0 is linear. OBSERVED BAND: 1.28-1.45 over ten unloaded runs on a 24-core dev box. The band this file previously claimed — 0.93-1.21 — did not reproduce and was an artifact: the small arm then measured ~1.7 ms, small enough that timer granularity and JIT warm-up, not scaling, set the number (the same ten-run sweep of that bench spanned 1.11-1.40). CALLS_PER_FILE is now sized so the small arm lands at ~14 ms; that halves the unloaded spread (0.30 -> 0.16) and costs ~2.0 s of wall time for the whole bench. The residual above 1.0 is real and not a defect: at LARGE the index and corpus are 4x the working set, so per-call-site locality is worse (~85 ns/site vs ~63 ns) while the algorithm stays linear. TRIAGE: a scaling failure is a TIMING signal — RE-RUN IT on an idle machine before investigating. Runner contention dominates everything above: pinned to 2 CPUs against 2 spinners the identical binary produced 1.16-2.18, i.e. a spurious FAIL, and the sibling bench/callable-value-flow drifts out of its own documented band the same way. The fingerprint arm is the opposite — it is deterministic; a re-run never changes it and must never be used to wish it away. Floor check: a per-call-site workspace rescan reintroduced ONLY on the receiver-bucket-absent path (the most plausible way #2788 returns) measures 4.538 at these same 400/1600 file scales — 812x slower on the small arm, 2850x on the large — while leaving the fingerprint byte-identical. The old always-hits corpus scored that same patch 1.279 and printed PASS. Resolution is timed alone; the fingerprint's outcome strings are built in a separate untimed pass because their allocation cost grows with the corpus and would otherwise show up as scaling." +} diff --git a/gitnexus/bench/cpp-qualified-ns/measure.mjs b/gitnexus/bench/cpp-qualified-ns/measure.mjs new file mode 100644 index 000000000..6ffbc18a0 --- /dev/null +++ b/gitnexus/bench/cpp-qualified-ns/measure.mjs @@ -0,0 +1,496 @@ +/** + * Build-free scaling + identity bench for `resolveCppQualifiedNamespaceMember`, + * the C++ qualified `ns::member()` receiver resolver (issue #2788). + * + * Before #2788 this function re-scanned EVERY parsed file — rebuilding a + * per-file `scopesById` map each time — once per qualified call site, so the + * scope-resolution emit phase cost O(callsites × 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. It is the same bug #1990 had already fixed in the sibling + * ADL path (`pickCppAdlCandidates` → `AdlCandidateIndex`). #1990 DID ship a + * scaling gate for that path — `test/integration/cpp-adl-benchmark.test.ts`, + * which asserts `emitRatio < fileRatio^1.5` — but it could never have caught + * this one, for two independent reasons: its corpus asserts + * `callsResolved === 0`, i.e. it generates only UNRESOLVED ADL sites, so it + * never drives the qualified-receiver path at all; and it is + * `describe.skipIf(!BENCH_ENABLED)` while the single CI step that sets + * `GITNEXUS_BENCH=1` names its test files explicitly and, until this PR wired + * it in, listed neither C++ bench — so it had never executed in CI. Even now + * that it runs, the `callsResolved === 0` half stands: it still cannot reach + * this path. Hence this bench, in an always-on step: a per-call-site workspace + * scan must not be reintroduced silently. + * + * For a synthetic corpus at two scales it reports: + * - `elapsed_ms` per scale (fastest of REPS, see `fastest`) for resolving + * every call site once, INCLUDING the one-time index build — that build is + * the work the per-site scan was traded for, so hiding it would let an + * index that is itself quadratic pass; + * - a scaling ratio `(t_large/t_small)/(LARGE/SMALL)`: ~1.0 linear, + * ~4.x quadratic at this scale gap; + * - a sha256 fingerprint over every `receiver::member(callsite) → outcome` + * the corpus resolves, as the correctness gate. A fingerprint change means + * qualified lookup started resolving different symbols — a behaviour + * change, never a performance one. + * + * A gate only covers the code path its corpus drives. Two properties below are + * therefore load-bearing and must not be "simplified" away: + * + * 1. **The receiver mix is production-shaped: ~1 in 5 receivers names a + * namespace, the other ~4 name nothing.** Case 1.5 in + * `scope-resolution/passes/receiver-bound-calls.ts` is reached by EVERY + * plain-identifier receiver call — `obj.size()`, `Widget::make()`, + * `buf.data()` — so in real source the overwhelming majority of calls into + * this resolver are receiver MISSES, not member misses inside a resolved + * receiver. An earlier revision of this bench drew every receiver from + * `ns_${…}`, i.e. always a namespace the corpus declared, so the receiver + * lookup never missed. Measured consequence: a "defensive full rescan when + * the receiver bucket is absent" regression — the single most plausible + * way this bug returns — scored 1.332 against the 1.8 budget and printed + * PASS, while costing 507× on a production-shaped corpus. + * 2. **The corpus contains every structural shape whose loss the fingerprint + * is supposed to catch** (see `buildCorpus`), including a batch of sites + * that pass a real `Callsite`. Without those, `narrowOverloadCandidates` / + * `cppConversionRank` / `isOverloadAmbiguousAfterNormalization` are not in + * the fingerprinted surface at all, and behaviour-only regressions there + * re-fingerprint byte-identically. + * + * Build-free: imports the `.ts` hotpath through tsx + * (`node --import tsx bench/cpp-qualified-ns/measure.mjs`). + * + * Without args: prints the JSON report. + * With `--check`: asserts the fingerprint == the committed baseline AND the + * scaling ratio is within budget; exits non-zero on drift/regression. + */ +import fs from 'node:fs'; +import path from 'node:path'; +import crypto from 'node:crypto'; +import { fileURLToPath } from 'node:url'; + +import { + clearCppInlineNamespaces, + markCppInlineNamespaceRange, + populateCppInlineNamespaceScopes, + resolveCppQualifiedNamespaceMember, +} from '../../src/core/ingestion/languages/cpp/inline-namespaces.ts'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const BASELINE_PATH = path.resolve(__dirname, 'baselines.json'); + +const SMALL = 400; +const LARGE = 1600; +/** Sized so the SMALL arm measures in the tens of ms rather than ~1.7 ms. + * Sub-2 ms samples are dominated by timer granularity and scheduler noise on + * a shared runner, which is what made the ratio drift out of its documented + * band under load; see `_scaling_note` in baselines.json. */ +const CALLS_PER_FILE = 480; +const REPS = 7; +const WARMUP = 3; + +/** 1 in N receivers names a declared namespace; the rest name nothing. Header + * property 1 is why this ratio, and not an always-hits corpus. */ +const NS_RECEIVER_IN = 5; + +const NO_SCOPES = {}; + +/** + * Deterministic 32-bit avalanche (murmur3 finalizer). Stands in for + * `Math.random()` — the corpus, the receiver mix and therefore the fingerprint + * must be byte-reproducible across machines and Node versions. + */ +function mix(n) { + let x = n >>> 0; + x = Math.imul(x ^ (x >>> 16), 0x85ebca6b) >>> 0; + x = Math.imul(x ^ (x >>> 13), 0xc2b2ae35) >>> 0; + return (x ^ (x >>> 16)) >>> 0; +} + +/** + * Deterministic synthetic corpus — no randomness, so the fingerprint is stable. + * + * Per file `f`, three top-level namespaces. Every shape here exists because + * some behaviour of `resolveCppQualifiedNamespaceMember` is unobservable + * without it; dropping one silently un-gates that behaviour. + * + * namespace ns_f { // ABI-versioning idiom (std::__1) + * void own0(); void own1(); // direct members → hit + * void both(int); // ALSO declared in v1 below + * void over(int); // overload set spanning levels + * inline namespace v1 { // transitively visible + * void inl0(); // → hit + * void dup(); + * void both(double); // the inline-child twin of `both` + * void over(int, int); void over(double); + * void same(int); + * } + * inline namespace v2 { + * void dup(); // two inline children → ambiguous + * void same(int); // identical signature → ambiguous + * } + * namespace detail { void hidden0(); } // NOT inline → invisible → miss + * } + * + * namespace twin_f { inline namespace twin_f { void twinned(); } } + * namespace shared_{f>>1} { void part{f&1}(); } + * + * What each shape gates: + * - `own0` / `inl0` / `hidden0` / `nosuch`: the three outcome classes (hit + * from the namespace's own defs, hit through an inline child, miss), each + * a different exit from the resolver. + * - `dup` across v1 and v2: `'ambiguous'` (#1564). + * - `twin_f`: the same-name inline nest — the only shape that observes + * `gatherQualifiedNsMember`'s `visited` dedup, without which the one + * `twinned` is collected twice and a resolved def flips to `'ambiguous'` + * (that function's comment explains why both scopes land on one receiver). + * - `shared_g` declared by files 2g and 2g+1: C++ namespaces are open, so one + * receiver's members are spread over however many files reopen it. The + * legacy per-call-site scan got this for free; the index has to merge + * across the whole `parsedFiles` array. `part0` and `part1` are declared in + * DIFFERENT files and both must resolve. + * - `both` at the namespace level and in the inline child: pins BOTH + * collection sources by nodeId, via the two `both` call sites that select + * between them on argument type. Drop own-def collection and the `int` + * probe moves; drop inline-child descent and the `double` probe moves. A + * pure REORDER of the two stays invisible, and correctly so: the return + * contract is order-blind — see `QualifiedNsMemberIndex.rootsByReceiver`. + * - `over` / `same` with a real `Callsite`: see `NS_PROBES`. + */ +function buildCorpus(fileCount) { + const parsedFiles = []; + for (let f = 0; f < fileCount; f++) { + const filePath = `src/file${f}.cpp`; + const scopes = []; + const inlineRanges = []; + let line = 1; + /** Push one Namespace scope with a range unique within this file, so + * `populateCppInlineNamespaceScopes` marks exactly the intended scopes. */ + const scope = (id, parent, ownedDefs, isInline = false) => { + const range = { startLine: line, startCol: 0, endLine: line + 1, endCol: 0 }; + line += 2; + scopes.push({ id, kind: 'Namespace', parent, ownedDefs, range }); + if (isInline) inlineRanges.push(range); + return id; + }; + const ns = (qualifiedName) => ({ + nodeId: `def:${filePath}#${qualifiedName}`, + type: 'Namespace', + qualifiedName, + }); + /** A callable def. `parameterTypes` are what makes overloads distinguishable + * both to `narrowOverloadCandidates` and — via the nodeId, exactly as the + * real C++ node keys do it — to the fingerprint. */ + const fn = (qualifiedName, parameterTypes) => + parameterTypes === undefined + ? { nodeId: `def:${filePath}#${qualifiedName}`, type: 'Function', qualifiedName } + : { + nodeId: `def:${filePath}#${qualifiedName}(${parameterTypes.join(',')})`, + type: 'Function', + qualifiedName, + parameterTypes, + parameterCount: parameterTypes.length, + requiredParameterCount: parameterTypes.length, + }; + + const nsId = scope(`sc:${f}:ns`, null, [ + ns(`ns_${f}`), + fn(`ns_${f}.own0`), + fn(`ns_${f}.own1`), + fn(`ns_${f}.both`, ['int']), + fn(`ns_${f}.over`, ['int']), + ]); + scope( + `sc:${f}:v1`, + nsId, + [ + ns(`ns_${f}.v1`), + fn(`ns_${f}.v1.inl0`), + fn(`ns_${f}.v1.dup`), + fn(`ns_${f}.v1.both`, ['double']), + fn(`ns_${f}.v1.over`, ['int', 'int']), + fn(`ns_${f}.v1.over`, ['double']), + fn(`ns_${f}.v1.same`, ['int']), + ], + true, + ); + scope( + `sc:${f}:v2`, + nsId, + [ns(`ns_${f}.v2`), fn(`ns_${f}.v2.dup`), fn(`ns_${f}.v2.same`, ['int'])], + true, + ); + scope(`sc:${f}:detail`, nsId, [ns(`ns_${f}.detail`), fn(`ns_${f}.detail.hidden0`)]); + + const twinId = scope(`sc:${f}:twin`, null, [ns(`twin_${f}`)]); + scope( + `sc:${f}:twin@inner`, + twinId, + [ns(`twin_${f}.twin_${f}`), fn(`twin_${f}.twin_${f}.twinned`)], + true, + ); + + const group = f >> 1; + scope(`sc:${f}:shared`, null, [ns(`shared_${group}`), fn(`shared_${group}.part${f & 1}`)]); + + parsedFiles.push({ filePath, scopes, inlineRanges }); + } + return parsedFiles; +} + +/** Capture-time inline marking + `populateOwners`-time scope-id resolution, in + * the same order the pipeline runs them. Must re-run after every + * `clearCppInlineNamespaces`, which drops both the marks and the index. */ +function populateInlineState(parsedFiles) { + clearCppInlineNamespaces(); + for (const parsed of parsedFiles) { + for (const range of parsed.inlineRanges) markCppInlineNamespaceRange(parsed.filePath, range); + populateCppInlineNamespaceScopes(parsed); + } +} + +/** + * Namespace-receiver probes: `[family, member, callsite]`. Drawn for the ~1 in + * `NS_RECEIVER_IN` call sites whose receiver actually names a namespace. + * + * The tail entries pass a real `Callsite`, which is the only way any of + * `narrowOverloadCandidates`, `cppConversionRank` or + * `isOverloadAmbiguousAfterNormalization` is reached — the resolver threads + * `callsite?.arity` / `callsite?.argumentTypes` into narrowing, and with no + * callsite those filters are pass-throughs. Each one is chosen to land on a + * DIFFERENT exit, so the fingerprint pins the whole narrowing ladder: + * - `over(int)` → exact-type filter, unique survivor (ns level) + * - `over(int,int)` → arity filter, unique survivor (inline child) + * - `over(double)` → exact-type filter, unique survivor (inline child) + * - `over(char)` → no exact match, `cppConversionRank` dominance + * picks `over(int)` (promotion 1) over + * `over(double)` (standard conversion 2) + * - `over(braced-init)` → conversion ranking rejects every candidate and + * `CPP_CONVERSION_ONLY_ARG_TYPE_PREFIXES` turns + * that into an empty set → `undefined` + * - `over` with arity 9 → arity filter empties an all-known-bounds set, + * the authoritative-empty branch → `undefined` + * - `same(int)` → two identical signatures survive narrowing → + * `isOverloadAmbiguousAfterNormalization` → `'ambiguous'` + * - `both(int)`/`both(double)` → select the namespace-level def and the + * inline-child def respectively, pinning both + * collection sources by nodeId + */ +const NS_PROBES = [ + ['ns', 'own0', undefined], + ['ns', 'own1', undefined], + ['ns', 'inl0', undefined], + ['ns', 'dup', undefined], + ['ns', 'hidden0', undefined], + ['ns', 'nosuch', undefined], + ['ns', 'both', undefined], + ['twin', 'twinned', undefined], + ['twin', 'nosuch', undefined], + ['shared', 'part0', undefined], + ['shared', 'part1', undefined], + ['ns', 'over', { arity: 1, argumentTypes: ['int'] }], + ['ns', 'over', { arity: 2, argumentTypes: ['int', 'int'] }], + ['ns', 'over', { arity: 1, argumentTypes: ['double'] }], + ['ns', 'over', { arity: 1, argumentTypes: ['char'] }], + ['ns', 'over', { arity: 1, argumentTypes: ['braced-init:int:3'] }], + ['ns', 'over', { arity: 9, argumentTypes: [] }], + ['ns', 'same', { arity: 1, argumentTypes: ['int'] }], + ['ns', 'both', { arity: 1, argumentTypes: ['int'] }], + ['ns', 'both', { arity: 1, argumentTypes: ['double'] }], +]; + +/** Members asked of the non-namespace receivers. Real-source member names, so + * the miss is a receiver miss and not a member miss. */ +const MISS_MEMBERS = ['size', 'begin', 'data', 'reset', 'own0', 'dup']; + +/** A plain identifier naming NO namespace in the corpus — a local, a type, a + * buffer. The ~4-in-5 majority of header property 1. */ +function missReceiver(key) { + const shape = key % 3; + if (shape === 0) return `obj${key % 97}`; + if (shape === 1) return `Widget${key % 31}`; + return `buf${key % 197}`; +} + +/** + * The call sites: `[receiver, member, callsite]`, deterministic, with the + * production receiver mix (~1 in `NS_RECEIVER_IN` names a namespace). + * + * Two independently mixed keys per site so the receiver class (`a`) and the + * probe choice (`b`) do not correlate — deriving both from one linear key made + * `key % NS_RECEIVER_IN === 0` imply `key % NS_PROBES.length ∈ {0, 5}`, which + * silently reduced the probe set to two entries. + */ +function callSites(fileCount) { + const sharedGroups = Math.ceil(fileCount / 2); + const sites = []; + let nsReceiverSites = 0; + /** The declared-namespace receiver a probe family asks for. */ + const nsReceiver = (family, key) => { + if (family === 'twin') return `twin_${key % fileCount}`; + if (family === 'shared') return `shared_${key % sharedGroups}`; + return `ns_${key % fileCount}`; + }; + const pushNs = (probe, key) => { + sites.push([nsReceiver(probe[0], key), probe[1], probe[2]]); + nsReceiverSites++; + }; + // Coverage prelude: every probe at least once at BOTH scales, so the + // fingerprinted outcome set never depends on how the mixer happens to spread. + for (let p = 0; p < NS_PROBES.length; p++) pushNs(NS_PROBES[p], p); + for (let f = 0; f < fileCount; f++) { + for (let c = 0; c < CALLS_PER_FILE; c++) { + const a = mix(f * 65599 + c); + const b = mix(a ^ 0x9e3779b9); + if (a % NS_RECEIVER_IN === 0) pushNs(NS_PROBES[b % NS_PROBES.length], b); + else sites.push([missReceiver(b), MISS_MEMBERS[a % MISS_MEMBERS.length], undefined]); + } + } + return { sites, nsReceiverSites }; +} + +/** The timed loop: resolution only. The outcome strings the fingerprint needs + * are built in a separate untimed pass (`outcomesOf`), so their allocation + * cost — which grows with the corpus and would inflate the scaling ratio on + * its own — never lands in the measurement. `sink` keeps the calls live. */ +function resolveAll(parsedFiles, sites) { + let sink = 0; + for (const [receiver, member, callsite] of sites) { + const hit = resolveCppQualifiedNamespaceMember( + receiver, + member, + parsedFiles, + NO_SCOPES, + callsite, + ); + if (hit !== undefined) sink++; + } + return sink; +} + +/** Fingerprint key for one site. The callsite is part of the key: `ns::over` + * resolves to a different def per arity/argument-type, and collapsing those + * onto one key would drop the whole narrowing ladder from the gate. */ +function siteKey(receiver, member, callsite) { + const args = + callsite === undefined ? '' : `${callsite.arity}|${callsite.argumentTypes.join(',')}`; + return `${receiver}::${member}(${args})`; +} + +/** Untimed identity pass, one resolve per DISTINCT `siteKey`. On a fixed corpus + * the resolver is a pure function of `(receiver, member, callsite)`, so a + * repeated site can only re-derive what the first occurrence already put in + * the Set — the same argument that makes collecting into a Set correct makes + * skipping the repeat correct. That is nearly the whole pass: the 192k/768k + * sites carry only 2,330/3,470 distinct outcomes. */ +function outcomesOf(parsedFiles, sites) { + const outcomes = new Set(); + const seen = new Set(); + for (const [receiver, member, callsite] of sites) { + const key = siteKey(receiver, member, callsite); + if (seen.has(key)) continue; + seen.add(key); + const hit = resolveCppQualifiedNamespaceMember( + receiver, + member, + parsedFiles, + NO_SCOPES, + callsite, + ); + outcomes.add( + `${key}\u0000${hit === undefined ? '' : hit === 'ambiguous' ? '' : hit.nodeId}`, + ); + } + return outcomes; +} + +/** + * MIN, not median — same rationale as bench/callable-value-flow: both scales + * are timed in one process and every error source (scheduler preemption, GC, a + * noisy neighbour on a shared CI runner) is additive, so the fastest observed + * run is the closest estimate of the uncontended cost and keeps the derived + * ratio comparable across machines. + */ +function fastest(values) { + return Math.min(...values); +} + +/** Time one full pass: index build (lazy, on the first call) + every call + * site. The corpus state is reset OUTSIDE the timer so the reset's own + * O(files) cost never lands in the measurement. */ +function timeResolution(parsedFiles, sites) { + for (let w = 0; w < WARMUP; w++) { + populateInlineState(parsedFiles); + resolveAll(parsedFiles, sites); + } + const samples = []; + for (let r = 0; r < REPS; r++) { + populateInlineState(parsedFiles); + const t0 = performance.now(); + resolveAll(parsedFiles, sites); + samples.push(performance.now() - t0); + } + return { ms: fastest(samples), outcomes: outcomesOf(parsedFiles, sites) }; +} + +function fingerprint(outcomes) { + return crypto + .createHash('sha256') + .update([...outcomes].sort().join('\n')) + .digest('hex'); +} + +const scales = {}; +for (const [name, fileCount] of [ + ['small', SMALL], + ['large', LARGE], +]) { + const parsedFiles = buildCorpus(fileCount); + const { sites, nsReceiverSites } = callSites(fileCount); + const { ms, outcomes } = timeResolution(parsedFiles, sites); + scales[name] = { + files: fileCount, + call_sites: sites.length, + // Reported, not asserted: `ns_receiver_sites` evidences header property 1's + // mix and `distinct_outcomes` the fingerprinted surface's size — a corpus + // edit collapsing either still yields a "valid" fingerprint over far less. + ns_receiver_sites: nsReceiverSites, + distinct_outcomes: outcomes.size, + ms: Number(ms.toFixed(3)), + fingerprint: fingerprint(outcomes), + }; +} + +const scalingRatio = scales.large.ms / scales.small.ms / (LARGE / SMALL); + +const report = { + small: scales.small, + large: scales.large, + scaling_ratio: Number(scalingRatio.toFixed(3)), + fingerprint: scales.large.fingerprint, +}; + +if (!process.argv.includes('--check')) { + console.log(JSON.stringify(report, null, 2)); + process.exit(0); +} + +const baseline = JSON.parse(fs.readFileSync(BASELINE_PATH, 'utf-8')); +const failures = []; +if (report.fingerprint !== baseline.fingerprint) { + failures.push( + `fingerprint drift: ${report.fingerprint} != ${baseline.fingerprint} — qualified ` + + `namespace lookup resolved a DIFFERENT symbol set. This is a behaviour change, not a perf one.`, + ); +} +if (report.scaling_ratio > baseline.scaling_budget) { + failures.push( + `scaling ${report.scaling_ratio} > budget ${baseline.scaling_budget} — per-call-site cost ` + + `now grows with corpus size again (#2788). Timing arm: re-run on an idle machine before ` + + `investigating (see _scaling_note in baselines.json); the fingerprint arm never warrants a re-run.`, + ); +} + +console.log(JSON.stringify(report, null, 2)); +if (failures.length > 0) { + console.error(`[cpp-qualified-ns --check] FAIL\n - ${failures.join('\n - ')}`); + process.exit(1); +} +console.log('[cpp-qualified-ns --check] PASS'); diff --git a/gitnexus/bench/schema-pairs/README.md b/gitnexus/bench/schema-pairs/README.md new file mode 100644 index 000000000..29594b9d7 --- /dev/null +++ b/gitnexus/bench/schema-pairs/README.md @@ -0,0 +1,100 @@ +# Schema pair-set bench (#2793) + +What a bigger `CodeRelation` FROM/TO pair set costs at query time, measured +against a real `@ladybugdb/core` database. + +```bash +# from gitnexus/ +node --import tsx bench/schema-pairs/measure.mjs # print one JSON line per size + a summary +node --import tsx bench/schema-pairs/measure.mjs --check # gate vs baselines.json +``` + +## Why it exists + +`src/core/lbug/schema.ts` generates its relation pairs from two cross products, +and declines to add a third one **on the strength of a number** — roughly 1.04× +at 450 declared pairs, 1.6× at 786, 2.1× at 1024. That measurement used to live +in a scratch directory, so nobody proposing a third rule could re-run it. This +harness is that measurement, committed — and it reproduces those figures. + +Run it before widening a rule, and quote the new ratio in the review. + +Observed on the reference box, **four runs** (ratios vs the 332-pair list): + +| pairs | untyped | typed (floor) | +| ----- | ---------- | ------------- | +| 332 | 1.00× | 1.00× | +| 450 | 0.93–1.05× | 0.98–1.17× | +| 641 | 1.22–1.43× | 1.11–1.23× | +| 786 | 1.52–1.75× | 1.19–1.31× | +| 1024 | 2.03–2.34× | 1.31–1.57× | + +Production's 450 came out _faster_ than 332 on three of the four runs, so at this +size the pair count is inside run-to-run noise. Everything past ~640 is not. +**Quote the range, not a single run** — one run is not evidence here. + +## What it measures + +For each pair-set size it builds a fresh database with all 32 node tables, a +`CodeRelation` table declaring exactly that many FROM/TO pairs, and **identical +data**, then times two query shapes over 40 anchors × 15 reps (median): + +- **`untyped_ms_`** — `MATCH (a {id: $id})-[r:CodeRelation]->(b)`. Neither + endpoint is labelled, so LadybugDB must treat every declared pair as a + candidate. This is the shape `impact`, `context` and `detect_changes` issue + when they walk out from one node id, and the only one whose plan depends on + how many pairs the table declares. +- **`typed_ms_`** — `MATCH (a:Function {…})-[r]->(b:Function)`, the lower + bound. Both endpoints labelled prunes the plan to a single pair, so this was + expected to be flat in the pair count. **It is not** — up to 1.17× at 450 and + 1.57× at 1024 — so a declared-but-unused pair costs something even when the + planner never considers it. `typed_ratio_*` is therefore the floor, not a noise + control; the real cost of widening sits between it and `ratio_*`. A run where + `typed_ratio` moves _more_ than `ratio` is noise-dominated and should be + rerun. +- **`ratio_`** — `untyped_ms_ / untyped_ms_332`. `ratio_450` is the + figure `schema.ts` quotes. + +### Sizes + +The pair set is a prefix of a fixed 32×32 (`NODE_TABLES`²) enumeration, so each +size is a strict superset of the smaller ones. The four pairs the synthetic data +uses are pinned to the front, so **the same rows are reachable by the same query +at every size** — the only variable is how many unused pairs are declared. The +harness fails if the row counts ever differ across sizes. + +| size | what it is | +| ---- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 332 | the pre-#2792 hand-written list — the reference for every ratio | +| 450 | production today (two cross products + 72 hand-declared pairs) | +| 641 | the third cross product `schema.ts` defers (`DEFINITION_ANCHOR_LABELS × {CodeElement, Section, Typedef, Union, Namespace, Impl, TypeAlias, Static, Template}`), which would leave ~29 hand-declared lines | +| 786 | the size an earlier revision of that comment attributed to the third rule — it is 641; kept as a measured waypoint | +| 1024 | the full cross product, the ceiling | + +## Correctness gate + +Before timing anything, the harness round-trips the **real** `SCHEMA_QUERIES` +through a real database and asserts that `CALL SHOW_CONNECTION('CodeRelation')` +reports exactly the pairs `parseRelationSchemaPairs` finds in `RELATION_SCHEMA`. + +No magic number is baked in: the invariant is that the DDL LadybugDB _accepted_ +carries the pair set our own parser believes it declares. The absolute count is +reported as `declared_pairs`. A pair declared twice would not reach this check at +all — LadybugDB rejects the `CREATE REL TABLE` outright, which is why a duplicate +kills every `analyze` rather than one repository's. + +## What it does NOT measure + +- **Ingest / `COPY` cost.** Pair-set size also multiplies the number of per-pair + CSVs the emitter routes to (`src/core/lbug/rel-pair-routing.ts`); that cost is + covered by `bench/emit-persistence`. +- **At-scale absolute numbers.** Row counts here are small and deliberately + constant. The ratios are the signal; the milliseconds are box-specific. + +## Regenerating the baseline + +`baselines.json` holds one budget, `ratio_450_budget` — the ceiling on what +production's own pair count may cost relative to the 332-pair hand-list it +replaced. Re-run without `--check` **several times** and copy the top of the +observed `ratio_450` range plus headroom — the spread between runs on this box +is wider than the effect being measured at 450, so a single run cannot set it. diff --git a/gitnexus/bench/schema-pairs/baselines.json b/gitnexus/bench/schema-pairs/baselines.json new file mode 100644 index 000000000..41a12d485 --- /dev/null +++ b/gitnexus/bench/schema-pairs/baselines.json @@ -0,0 +1,4 @@ +{ + "_comment": "ratio_450_budget — ceiling on what production's 450-pair set may cost on untyped-endpoint anchored queries, relative to the 332-pair hand-list it replaced. Observed 0.94x and 1.05x across two runs on the reference box (i.e. inside run-to-run noise; it came out faster than 332 once). The budget carries headroom for that spread — compare typed_ratio_450 (1.10-1.17x) for this box's floor. Raise it only with a measured range, never a single run.", + "ratio_450_budget": 1.3 +} diff --git a/gitnexus/bench/schema-pairs/measure.mjs b/gitnexus/bench/schema-pairs/measure.mjs new file mode 100644 index 000000000..59fe92db7 --- /dev/null +++ b/gitnexus/bench/schema-pairs/measure.mjs @@ -0,0 +1,357 @@ +/** + * What a bigger `CodeRelation` FROM/TO pair set costs at query time (#2793). + * + * `src/core/lbug/schema.ts` declares its relation pairs from two cross products + * plus a small hand-written remainder, and it justifies NOT adding a third cross + * product with a number: anchored queries cost ~1.04× at 450 declared pairs but + * 1.6× at 786 and 2.1× at 1024. That measurement previously lived in a scratch + * directory, so the claim could not be re-checked when someone proposed + * widening a rule. This is it, committed. + * + * WHAT IT MEASURES. Against a real `@ladybugdb/core` database, with byte-identical + * DATA at every size, it times the query shape whose plan actually depends on the + * declared pair set: + * + * MATCH (a {id: $id})-[r:CodeRelation]->(b) RETURN b.id + * + * Neither endpoint is labelled, so LadybugDB must consider every declared + * FROM/TO pair as a candidate — this is the shape `impact`, `context` and + * `detect_changes` all issue when they walk out from one node id. + * + * A LABEL-typed query (`MATCH (a:Function)-[r]->(b:Function)`) is measured + * alongside it as the LOWER BOUND. Its plan prunes to a single pair, so it was + * expected to be flat in the pair count — it is NOT. Measured here it reaches + * 1.17× at 450 and 1.57× at 1024 against the same 332-pair reference, i.e. a + * declared-but-unused pair costs something even when the planner never + * considers it (per-pair catalog/storage overhead the query pays regardless). + * So `typed_ratio_*` is not a noise control: it is the floor, and the true cost + * of a wider pair set lies between it and `ratio_*`. Treat any run where + * `typed_ratio` moves MORE than `ratio` as noise-dominated. + * + * SIZES. The pair set is a prefix of a fixed 32×32 (`NODE_TABLES`²) enumeration + * so every size is a strict SUPERSET of the smaller ones, and the four pairs the + * data actually uses are pinned first — so the same rows are reachable by the + * same query at every size, and the only variable is how many UNUSED pairs the + * table declares: + * - 332 — the pre-#2792 hand-written list (the historical baseline); + * - 450 — production today (two cross products + 72 hand-declared); + * - 641 — the third cross product schema.ts defers + * (`DEFINITION_ANCHOR_LABELS × {CodeElement, Section, Typedef, Union, + * Namespace, Impl, TypeAlias, Static, Template}`), which would leave + * only ~29 hand-declared lines; + * - 786 — the size an earlier revision of that comment attributed to the + * third rule (it is 641; 786 is kept as a measured waypoint); + * - 1024 — the full cross product, the ceiling. + * + * Ratios are reported against 332, the smallest size — `ratio_450` is the + * number schema.ts quotes. + * + * CORRECTNESS GATE. Before timing anything it round-trips the REAL + * `SCHEMA_QUERIES` through a real database and asserts that + * `CALL SHOW_CONNECTION('CodeRelation')` reports exactly the pairs + * `parseRelationSchemaPairs` finds in `RELATION_SCHEMA`. That is the invariant + * that matters and it needs no magic number: it proves the DDL LadybugDB + * ACCEPTED carries the pair set our own parser believes it declares. (A + * duplicated FROM/TO would not even get this far — LadybugDB rejects the + * `CREATE REL TABLE` outright, which is why that failure kills every `analyze`.) + * The absolute count is reported as `declared_pairs` for the record. + * + * Build-free: imports the `.ts` sources through tsx. + * + * node --import tsx bench/schema-pairs/measure.mjs # print JSON lines + * node --import tsx bench/schema-pairs/measure.mjs --check # gate vs baselines.json + * + * `--check` fails if the correctness gate breaks, or if `ratio_450` exceeds its + * budget — i.e. if production's own pair count starts costing materially more + * than the hand-written list it replaced. + */ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { NODE_TABLES } from 'gitnexus-shared'; +import { + NODE_SCHEMA_QUERIES, + RELATION_SCHEMA, + REL_TABLE_NAME, +} from '../../src/core/lbug/schema.ts'; +import { parseRelationSchemaPairs } from '../../src/core/lbug/rel-pair-routing.ts'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const BASELINE_PATH = path.resolve(__dirname, 'baselines.json'); + +const lbug = (await import('@ladybugdb/core')).default; + +// ---- sizes + the pair enumeration every size is a prefix of ---- + +const SIZES = [332, 450, 641, 786, 1024]; +const REFERENCE_SIZE = 332; // ratios are relative to this +const PRODUCTION_SIZE = 450; // the size schema.ts ships + +// The four pairs the synthetic data uses. Pinned to the FRONT of the +// enumeration so they are declared at every size — otherwise a smaller pair set +// would simply carry fewer rows and the comparison would measure data volume, +// not pair-set size. +const DATA_PAIRS = [ + ['File', 'Function'], + ['Function', 'Function'], + ['Function', 'Class'], + ['Class', 'Method'], +]; + +const pairKey = ([from, to]) => `${from}|${to}`; + +// NODE_TABLES² in declaration order, data pairs first, deduped. 32² = 1024. +const PAIR_UNIVERSE = (() => { + const seen = new Set(DATA_PAIRS.map(pairKey)); + const all = [...DATA_PAIRS]; + for (const from of NODE_TABLES) { + for (const to of NODE_TABLES) { + const key = `${from}|${to}`; + if (seen.has(key)) continue; + seen.add(key); + all.push([from, to]); + } + } + return all; +})(); + +if (PAIR_UNIVERSE.length !== NODE_TABLES.length ** 2) { + throw new Error( + `bench: pair universe is ${PAIR_UNIVERSE.length}, expected ${NODE_TABLES.length ** 2} ` + + `(NODE_TABLES changed — update SIZES, the 1024 ceiling is no longer the ceiling)`, + ); +} +for (const size of SIZES) { + if (size > PAIR_UNIVERSE.length) { + throw new Error(`bench: size ${size} exceeds the ${PAIR_UNIVERSE.length}-pair universe`); + } +} + +const relTableDdlFor = (size) => { + const pairs = PAIR_UNIVERSE.slice(0, size).map(([from, to]) => ` FROM \`${from}\` TO \`${to}\``); + return `CREATE REL TABLE ${REL_TABLE_NAME} (\n${pairs.join(',\n')},\n type STRING,\n confidence DOUBLE,\n reason STRING,\n step INT32\n)`; +}; + +// ---- synthetic data (identical at every size) ---- + +const FILES = 20; +const FNS_PER_FILE = 8; +const CLASSES = 40; +const METHODS_PER_CLASS = 4; +const CALLS_PER_FN = 3; +const REPS = 15; // median over reps +const ANCHORS = 40; // distinct anchor ids queried per rep + +// Batched with UNWIND rather than one statement per row: per-statement overhead +// dwarfs the insert itself here, and load time is not what this bench measures. +function dataStatements() { + const stmts = []; + const fnIds = []; + const classIds = []; + const methodIds = []; + const fileIds = []; + for (let f = 0; f < FILES; f++) fileIds.push(`file-${f}`); + for (let f = 0; f < FILES; f++) { + for (let i = 0; i < FNS_PER_FILE; i++) fnIds.push(`fn-${f}-${i}`); + } + for (let c = 0; c < CLASSES; c++) { + classIds.push(`cls-${c}`); + for (let m = 0; m < METHODS_PER_CLASS; m++) methodIds.push(`m-${c}-${m}`); + } + + const nodeBatch = (label, ids) => + `UNWIND [${ids.map((id) => `{id: '${id}'}`).join(', ')}] AS r ` + + `CREATE (:\`${label}\` {id: r.id, name: r.id, filePath: 'bench.ts'})`; + stmts.push(nodeBatch('File', fileIds)); + stmts.push(nodeBatch('Function', fnIds)); + stmts.push(nodeBatch('Class', classIds)); + stmts.push(nodeBatch('Method', methodIds)); + + const relBatch = (fromLabel, toLabel, type, edges) => + `UNWIND [${edges.map(([f, t]) => `{f: '${f}', t: '${t}'}`).join(', ')}] AS e ` + + `MATCH (a:\`${fromLabel}\` {id: e.f}), (b:\`${toLabel}\` {id: e.t}) ` + + `CREATE (a)-[:${REL_TABLE_NAME} {type: '${type}', confidence: 1.0, reason: 'bench', step: 0}]->(b)`; + + const contains = []; + for (let f = 0; f < FILES; f++) { + for (let i = 0; i < FNS_PER_FILE; i++) contains.push([`file-${f}`, `fn-${f}-${i}`]); + } + stmts.push(relBatch('File', 'Function', 'CONTAINS', contains)); + + // Function→Function calls: each fn calls the next CALLS_PER_FN, wrapping. + const calls = []; + for (let i = 0; i < fnIds.length; i++) { + for (let k = 1; k <= CALLS_PER_FN; k++) calls.push([fnIds[i], fnIds[(i + k) % fnIds.length]]); + } + stmts.push(relBatch('Function', 'Function', 'CALLS', calls)); + + const uses = fnIds.map((id, i) => [id, classIds[i % classIds.length]]); + stmts.push(relBatch('Function', 'Class', 'USES', uses)); + + const hasMethod = []; + for (let c = 0; c < CLASSES; c++) { + for (let m = 0; m < METHODS_PER_CLASS; m++) hasMethod.push([`cls-${c}`, `m-${c}-${m}`]); + } + stmts.push(relBatch('Class', 'Method', 'HAS_METHOD', hasMethod)); + + // Anchors: functions, which have out-edges on two distinct declared pairs. + return { stmts, anchors: fnIds.slice(0, ANCHORS) }; +} + +const { stmts: DATA_STATEMENTS, anchors: ANCHOR_IDS } = dataStatements(); + +// ---- timing ---- + +const median = (xs) => { + const s = [...xs].sort((a, b) => a - b); + const m = Math.floor(s.length / 2); + return s.length % 2 ? s[m] : (s[m - 1] + s[m]) / 2; +}; + +const withDb = async (fn) => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-bench-pairs-')); + const db = new lbug.Database(path.join(dir, 'db')); + const conn = new lbug.Connection(db); + try { + return await fn(conn); + } finally { + await conn.close().catch(() => {}); + await db.close?.().catch?.(() => {}); + fs.rmSync(dir, { recursive: true, force: true }); + } +}; + +async function runAll(conn, statements) { + for (const s of statements) await conn.query(s); +} + +// The measured shape: BOTH endpoints untyped, anchored by id. LadybugDB must +// consider every declared FROM/TO pair as a candidate. +const UNTYPED_QUERY = (id) => + `MATCH (a {id: '${id}'})-[r:${REL_TABLE_NAME}]->(b) RETURN b.id AS id, r.type AS type`; +// The lower bound: both endpoints labelled, so the planner prunes to one pair. +// Still not flat in the pair count (see the header) — an unused declared pair +// costs something even when the plan never touches it. +const TYPED_QUERY = (id) => + `MATCH (a:Function {id: '${id}'})-[r:${REL_TABLE_NAME}]->(b:Function) RETURN b.id AS id`; + +async function timeQueries(conn, build) { + // Warm: run the whole anchor sweep once uncounted (plan cache + page cache). + for (const id of ANCHOR_IDS) await (await conn.query(build(id))).getAll(); + const samples = []; + let rows = 0; + for (let rep = 0; rep < REPS; rep++) { + const start = process.hrtime.bigint(); + let n = 0; + for (const id of ANCHOR_IDS) n += (await (await conn.query(build(id))).getAll()).length; + samples.push(Number(process.hrtime.bigint() - start) / 1e6); + rows = n; + } + return { ms: median(samples), rows }; +} + +async function measureSize(size) { + return withDb(async (conn) => { + for (const q of NODE_SCHEMA_QUERIES) await conn.query(q); + await conn.query(relTableDdlFor(size)); + await runAll(conn, DATA_STATEMENTS); + const untyped = await timeQueries(conn, UNTYPED_QUERY); + const typed = await timeQueries(conn, TYPED_QUERY); + return { + pairs: size, + untyped_ms: Number(untyped.ms.toFixed(3)), + untyped_rows: untyped.rows, + typed_ms: Number(typed.ms.toFixed(3)), + typed_rows: typed.rows, + }; + }); +} + +// ---- correctness gate: the REAL schema, round-tripped ---- + +async function verifyRealSchema() { + return withDb(async (conn) => { + for (const q of NODE_SCHEMA_QUERIES) await conn.query(q); + // If RELATION_SCHEMA declared a pair twice, LadybugDB rejects this outright + // — the failure mode that kills every `analyze`, not just one repo's. + await conn.query(RELATION_SCHEMA); + const res = await conn.query(`CALL SHOW_CONNECTION('${REL_TABLE_NAME}') RETURN *`); + const rows = await res.getAll(); + const actual = new Set( + rows.map( + (r) => + `${r['source table name'] ?? r.source}|${r['destination table name'] ?? r.destination}`, + ), + ); + const expected = parseRelationSchemaPairs(RELATION_SCHEMA); + const missing = [...expected].filter((p) => !actual.has(p)).sort(); + const extra = [...actual].filter((p) => !expected.has(p)).sort(); + return { declared_pairs: expected.size, db_pairs: actual.size, missing, extra }; + }); +} + +// ---- run ---- + +const CHECK = process.argv.includes('--check'); +const failures = []; + +const verified = await verifyRealSchema(); +if (verified.missing.length > 0 || verified.extra.length > 0) { + failures.push( + `RELATION_SCHEMA round-trip mismatch: ${verified.missing.length} pair(s) parsed but absent ` + + `from SHOW_CONNECTION (${verified.missing.slice(0, 5).join(', ')}), ${verified.extra.length} ` + + `present in the DB but unparsed (${verified.extra.slice(0, 5).join(', ')})`, + ); +} + +const results = []; +for (const size of SIZES) results.push(await measureSize(size)); + +const reference = results.find((r) => r.pairs === REFERENCE_SIZE); +const summary = { + ...verified, + missing: undefined, + extra: undefined, + reference_pairs: REFERENCE_SIZE, +}; +for (const r of results) { + summary[`untyped_ms_${r.pairs}`] = r.untyped_ms; + summary[`typed_ms_${r.pairs}`] = r.typed_ms; + summary[`ratio_${r.pairs}`] = Number((r.untyped_ms / reference.untyped_ms).toFixed(3)); + summary[`typed_ratio_${r.pairs}`] = Number((r.typed_ms / reference.typed_ms).toFixed(3)); +} + +// Row counts must be identical at every size — otherwise the sizes are not +// carrying the same data and the ratios mean nothing. +const rowShapes = new Set(results.map((r) => `${r.untyped_rows}/${r.typed_rows}`)); +if (rowShapes.size !== 1) { + failures.push( + `row counts differ across pair-set sizes (${[...rowShapes].join(' vs ')}) — the data pins ` + + `in DATA_PAIRS are not holding, so the ratios compare different graphs`, + ); +} + +if (!CHECK) { + for (const r of results) process.stdout.write(JSON.stringify(r) + '\n'); + process.stdout.write(JSON.stringify(summary) + '\n'); +} else { + const baselines = JSON.parse(fs.readFileSync(BASELINE_PATH, 'utf8')); + const budget = baselines[`ratio_${PRODUCTION_SIZE}_budget`]; + if (budget !== undefined && summary[`ratio_${PRODUCTION_SIZE}`] >= budget) { + failures.push( + `production pair set (${PRODUCTION_SIZE}) costs ${summary[`ratio_${PRODUCTION_SIZE}`]}× vs ` + + `${REFERENCE_SIZE} pairs, >= budget ${budget} (untyped ${reference.untyped_ms}ms -> ` + + `${summary[`untyped_ms_${PRODUCTION_SIZE}`]}ms; typed control ` + + `${summary[`typed_ratio_${PRODUCTION_SIZE}`]}×)`, + ); + } + process.stdout.write(JSON.stringify(summary) + '\n'); +} + +if (failures.length > 0) { + for (const f of failures) process.stderr.write(`[schema-pairs] FAIL: ${f}\n`); + process.exit(1); +} +if (CHECK) process.stderr.write(`[schema-pairs --check] PASS (${results.length} sizes)\n`); diff --git a/gitnexus/bench/scope-capture/baselines.json b/gitnexus/bench/scope-capture/baselines.json index 8b712832c..84a235786 100644 --- a/gitnexus/bench/scope-capture/baselines.json +++ b/gitnexus/bench/scope-capture/baselines.json @@ -14,10 +14,11 @@ "_rebaselined_2766_callee_position_marker": "#2766 review fix: a call's callee selector is no longer DROPPED at capture. An earlier commit on this branch dropped it outright, which also deleted the genuine field read on a func-typed struct field (`h.dep.Work()` where `Work func() error`) - callback/hook/mock structs lost their only ACCESSES evidence. The match is now emitted carrying `@reference.callee-position`, and the phantom is suppressed at EMIT by the resolved target's kind instead. Go only: the other 14 languages' fingerprints are byte-identical, which is the check that this is not a cross-language capture change. Prior 7bb524a32a2eed57a15b454e3a33480e92a496c683e6856ef02179693c0e02e3 -> e47302079e17a5e73711bbed5416557b49327cb67e4932008700ec6b8fb468b3; scaling 1.001 < 1.5; fixtures 102 (unchanged), capture_groups_fp 2103." }, "cobol": { - "fingerprint": "d45bb091b0893d0de4fae2486b31ba21719c9377bf35a0908fd3a36fa1c3bf4e", + "fingerprint": "c8c00b56a7da24e04080eb885714fbbf45e3903324f0cf9df0754f5b5a92e3aa", "scaling_budget": 1.5, "_rebaselined_callable_flow_2522_followup": "PR #2522 follow-up: COBOL procedure-pointer callable flow facts; multi-topic extraction now consumes each grouped scope/declaration match once instead of requiring a duplicate declaration-only match. Prior 68ee0e95eb9f86f2d92ca35f730f4c2d4d83abc1b5241ae767ff3437780ec8d1 -> d45bb091b0893d0de4fae2486b31ba21719c9377bf35a0908fd3a36fa1c3bf4e; scaling 0.853 < 1.5.", - "_note": "Updated for F17-F23 fixes (P2: TIMES guard, ADD GIVING, SQL AS alias). See PR #1959." + "_note": "Updated for F17-F23 fixes (P2: TIMES guard, ADD GIVING, SQL AS alias). See PR #1959.", + "_rebaselined_2793_declaratives": "PR #2793: corpus-only re-baseline. `cobol-declaratives` was added to test/fixtures/lang-resolution to reproduce the `Namespace\u2192Record` analyze abort (DECLARATIVES / USE AFTER STANDARD ERROR ON ), and this bench globs `lang-resolution/cobol-*`, so the corpus grew 14 -> 15 files. Verified capture-neutral: with that one fixture moved aside the fingerprint is byte-identical to the prior d45bb091b0893d0de4fae2486b31ba21719c9377bf35a0908fd3a36fa1c3bf4e. No COBOL capture code changed in that PR. Scaling 0.677 < 1.5." }, "c": { "fingerprint": "3418cded9f7072152f68992f0a426f43ae7d9d553579a47075fc0cab185848a5", diff --git a/gitnexus/src/cli/analyze.ts b/gitnexus/src/cli/analyze.ts index 9475ee25f..6851c2478 100644 --- a/gitnexus/src/cli/analyze.ts +++ b/gitnexus/src/cli/analyze.ts @@ -15,6 +15,8 @@ import v8 from 'v8'; import cliProgress from 'cli-progress'; import { isLbugReady, LbugWipeError } from '../core/lbug/lbug-adapter.js'; import { boundedCheckpointBeforeExit } from '../core/lbug/shutdown-helpers.js'; +import { findUndeclaredRelationPairError } from '../core/lbug/rel-pair-routing.js'; +import { causeChain } from '../lib/utils.js'; import { getOsPageSize, isLbugCheckpointIoError, @@ -101,15 +103,13 @@ const writeFatalToStderr = (label: string, err: unknown): void => { // #2068) is only reachable via `.cause`. Without this the user sees the // wrapper's main-thread stack and never the real frame. `cause.stack` already // begins with the cause's message, so we print the stack alone (not message + - // stack) to avoid repeating it. Depth-bounded so a cyclic `cause` can't loop - // (the phase runner wraps one level; the bound leaves headroom for future - // nesting); uses realStderrWrite so the redirected console.error's ANSI - // clear-line wrapping can't erase it (#1169). - const MAX_CAUSE_DEPTH = 5; - let cause: unknown = isErr ? (err as { cause?: unknown }).cause : undefined; - for (let depth = 0; depth < MAX_CAUSE_DEPTH && cause instanceof Error; depth++) { + // stack) to avoid repeating it. `causeChain` owns the traversal and the depth + // bound that stops a cyclic `cause` looping — this used to be one of four + // hand-rolled copies that had already drifted apart on both. Uses + // realStderrWrite so the redirected console.error's ANSI clear-line wrapping + // can't erase it (#1169). The head is skipped: it was just printed above. + for (const cause of causeChain(isErr ? (err as { cause?: unknown }).cause : undefined)) { realStderrWrite(`\n Caused by: ${cause.stack ?? cause.message}\n`); - cause = (cause as { cause?: unknown }).cause; } }; @@ -1717,6 +1717,36 @@ const analyzeCommandImpl = async ( return; } + // An extracted edge whose FROM→TO label pair is missing from GitNexus's own + // relation DDL (#2789). `assertDeclaredPair` aborts the run rather than let + // the bulk COPY fail late and silently drop the edge, so the user sees a + // mid-run crash inside GitNexus internals with nothing to act on. Name the + // pair, the relationship and the file that produced it, and say plainly that + // a re-run cannot help — this is deterministic for the same input. + // Checked by TYPE (repo norm, #2385) BEFORE the message-text heuristics + // below, and through the `cause` chain because the ingestion phase runner + // rewraps every phase failure as `Phase 'X' failed: …`. + const undeclaredPair = findUndeclaredRelationPairError(err); + if (undeclaredPair !== undefined) { + // Render the error's OWN message indented — same idiom as the + // `LbugWipeError` and page-size branches below. `UndeclaredRelationPairError` + // builds a fully self-contained message (pair, relationship type, both node + // ids, source file, issue URL, `.gitnexusignore` workaround) precisely + // because `gitnexus serve` forwards only `err.message` over worker IPC. + // Re-rendering those fields here would be a second copy of one string, free + // to drift from the first — and the actionable half would reach CLI users + // only. `undeclaredPair.message`, not the outer `msg`: the real error may be + // several `cause` levels below the phase wrapper `msg` came from. + cliError(` ${undeclaredPair.message.replace(/\n/g, '\n ')}\n`, { + recoveryHint: 'undeclared-relation-pair', + labelPair: undeclaredPair.pairKey, + relationType: undeclaredPair.relationType, + sourceFile: undeclaredPair.sourceFile, + }); + process.exitCode = 1; + return; + } + // WAL corruption — the index file is unreadable. Give a clear recovery // path without a confusing stack trace (the native error message alone // is enough signal). diff --git a/gitnexus/src/cli/cli-message.ts b/gitnexus/src/cli/cli-message.ts index 61b65dd00..b9b509b52 100644 --- a/gitnexus/src/cli/cli-message.ts +++ b/gitnexus/src/cli/cli-message.ts @@ -60,7 +60,8 @@ export type RecoveryHint = | 'module-not-found' | 'gitnexusrc-invalid' | 'default-branch-invalid' - | 'index-lock-timeout'; + | 'index-lock-timeout' + | 'undeclared-relation-pair'; /** * Common shape for the optional structured-field bag passed to diff --git a/gitnexus/src/core/ingestion/languages/cpp/adl.ts b/gitnexus/src/core/ingestion/languages/cpp/adl.ts index 7a1bd9f66..4c9efb744 100644 --- a/gitnexus/src/core/ingestion/languages/cpp/adl.ts +++ b/gitnexus/src/core/ingestion/languages/cpp/adl.ts @@ -58,8 +58,8 @@ * - `noAdlSites` — call sites with parenthesized function (capture-time) * - `classToNamespaceQualifiedName` — class def → its enclosing namespace * qualified name (`populateCppAssociatedNamespaces` time) - * - `adlIndex` / `adlIndexSource` — the lazily-built candidate index and the - * `parsedFiles` reference it was built from (first-`pickCppAdlCandidates` + * - `adlIndexByPass` — the lazily-built candidate index, memoized weakly on + * the `parsedFiles` array it was built from (first-`pickCppAdlCandidates` * time; see `ensureAdlIndex`) * * The class→namespace map uses qualified names (not scope IDs) because @@ -176,8 +176,21 @@ export interface AdlCandidateIndex { readonly seqByNodeId: Map; } -let adlIndex: AdlCandidateIndex | undefined; -let adlIndexSource: readonly ParsedFile[] | undefined; +/** + * Per-pass memo: `parsedFiles` array identity → the built index. `WeakMap`-keyed + * so the entry lives only as long as the caller's `parsedFiles` array does, and + * is reclaimed with the pass — mirrors `qualifiedNsIndexByPass` in + * `inline-namespaces.ts` and `moduleScopeIndexByPass` in `file-local-linkage.ts`. + * + * Weak keying is load-bearing, not stylistic: the index holds + * `SymbolDefinition` references reaching into every ParsedFile's scopes, so the + * previous module-level strong `let` pair (index + source array) kept the whole + * C/C++ ParsedFile set alive past the point `scope-resolution/pipeline/phase.ts` + * evicts `files`/`contents`/`preExtractedByPath` and calls `forceGc()` to + * reclaim it — C++ is 7th of 16 `SCOPE_RESOLVERS` entries, so the retention + * survived nine later language passes plus emit. + */ +let adlIndexByPass = new WeakMap(); function siteKey(filePath: string, line: number, col: number): string { return `${filePath}:${line}:${col}`; @@ -373,25 +386,39 @@ export function validateAdlSeqCoverage(idx: AdlCandidateIndex): string[] { } /** Build the ADL index on first use of a given `parsedFiles` set; reuse it for - * all subsequent call sites in the same pipeline run. Reset by + * every subsequent call site in the same pipeline run. Reset by * `clearCppAdlState`. * + * Lifetime: the memoized entry is reachable only while the caller still holds + * the `parsedFiles` array. Once the pipeline drops it, the entry — and the + * `SymbolDefinition` references it holds into those files' scopes — becomes + * collectable; nothing here pins it (see {@link adlIndexByPass}). + * * Staleness is keyed on `parsedFiles` reference identity ONLY, but the index * is a function of THREE inputs: `parsedFiles` (namespace/friend candidates), * `scopes` (`classDefsBySimple`, read from `scopes.defs.byId`), and the - * module-level `classToNamespaceQualifiedName` (friend-candidate keys). This - * is sound for the current pipeline because all three are built together once - * per `runScopeResolution` pass and `clearCppAdlState` runs in - * `loadResolutionConfig` at the start of every pass. Callers MUST call - * `clearCppAdlState` between any two passes that change `scopes` or - * `classToNamespaceQualifiedName` while reusing the same `parsedFiles` array - * reference — otherwise a stale index would be served. (No such caller exists - * today; widening the guard to also key on `scopes` is deferred until one - * does.) */ -function ensureAdlIndex(scopes: ScopeResolutionIndexes, parsedFiles: readonly ParsedFile[]): void { - if (adlIndex !== undefined && adlIndexSource === parsedFiles) return; - adlIndex = buildAdlIndex(scopes, parsedFiles); - adlIndexSource = parsedFiles; + * module-level `classToNamespaceQualifiedName` (friend-candidate keys). The + * WeakMap key covers only the first, so the explicit clear is still REQUIRED + * for the other two: a later pass may hand back the same `parsedFiles` + * reference with different `scopes` / class→namespace state, and identity + * alone would serve the stale index. This is sound for the current pipeline + * because all three are built together once per `runScopeResolution` pass and + * `clearCppAdlState` runs in `loadResolutionConfig` at the start of every + * pass. Callers MUST call `clearCppAdlState` between any two passes that + * change `scopes` or `classToNamespaceQualifiedName` while reusing the same + * `parsedFiles` array reference — otherwise a stale index would be served. + * (No such caller exists today; widening the key to also cover `scopes` is + * deferred until one does.) */ +function ensureAdlIndex( + scopes: ScopeResolutionIndexes, + parsedFiles: readonly ParsedFile[], +): AdlCandidateIndex { + let index = adlIndexByPass.get(parsedFiles); + if (index === undefined) { + index = buildAdlIndex(scopes, parsedFiles); + adlIndexByPass.set(parsedFiles, index); + } + return index; } /** Record per-call-site argument info. Called once per call site from @@ -504,8 +531,14 @@ export function clearCppAdlState(): void { argInfoSiteKeysByFile.clear(); noAdlSiteKeysByFile.clear(); classToNamespaceQualifiedName.clear(); - adlIndex = undefined; - adlIndexSource = undefined; + // The candidate index is dropped by REASSIGNING a fresh `WeakMap`: `WeakMap` + // has no `.clear()`, and swapping the instance discards every memoized entry + // at once — the exact invalidation the previous `adlIndex = undefined` pair + // provided. Required even though the map is keyed on `parsedFiles` identity, + // because the index also depends on `scopes` and + // `classToNamespaceQualifiedName`, which the key cannot observe (see + // {@link ensureAdlIndex}). + adlIndexByPass = new WeakMap(); } /** @@ -572,15 +605,20 @@ export function pickCppAdlCandidates( if (args === undefined || args.length === 0) return undefined; // Build the workspace-wide ADL candidate index once; reuse for every site. - ensureAdlIndex(scopes, parsedFiles); + const idx = ensureAdlIndex(scopes, parsedFiles); // Collect associated namespace QNames from every participating class-typed arg // and from function-reference args. const associatedNamespaces = new Set(); for (const arg of args) { - collectAssociatedNamespacesForAdlArg(arg, scopes, associatedNamespaces); + collectAssociatedNamespacesForAdlArg(arg, scopes, idx, associatedNamespaces); if (arg.functionRefText !== undefined) { - collectFunctionTypeAssociatedNamespaces(arg.functionRefText, scopes, associatedNamespaces); + collectFunctionTypeAssociatedNamespaces( + arg.functionRefText, + scopes, + idx, + associatedNamespaces, + ); } } if (associatedNamespaces.size === 0) return undefined; @@ -593,8 +631,6 @@ export function pickCppAdlCandidates( // (`friendCandidates`, ISO C++ `[basic.lookup.argdep]` §2). // Dedup by nodeId and sort by visitation sequence so the candidate list is // byte-for-byte identical to the legacy file-major scan order. - const idx = adlIndex; - if (idx === undefined) return undefined; const bySeq = new Map(); const seenKey = new Set(); const collectFrom = (buckets: Map>): void => { @@ -621,12 +657,13 @@ export function pickCppAdlCandidates( function collectAssociatedNamespacesForAdlArg( arg: CppAdlArgInfo, scopes: ScopeResolutionIndexes, + idx: AdlCandidateIndex, associatedNamespaces: Set, ): void { // For template args this may be the template name itself (e.g. `vector`); // simple-name lookup can match project classes with the same name (known // V1/V2 simplification). - addAssociatedNamespaceForClassName(arg.simpleClassName, scopes, associatedNamespaces); + addAssociatedNamespaceForClassName(arg.simpleClassName, scopes, idx, associatedNamespaces); // Includes template-owner namespaces (e.g. `std` in std::vector). If // that surfaces extra candidates, merged-candidate overload narrowing in @@ -637,17 +674,18 @@ function collectAssociatedNamespacesForAdlArg( if (ns.length > 0) associatedNamespaces.add(ns); } for (const className of arg.templateArgClassNames) { - addAssociatedNamespaceForClassName(className, scopes, associatedNamespaces); + addAssociatedNamespaceForClassName(className, scopes, idx, associatedNamespaces); } } function addAssociatedNamespaceForClassName( simpleClassName: string, scopes: ScopeResolutionIndexes, + idx: AdlCandidateIndex, associatedNamespaces: Set, ): void { if (simpleClassName.length === 0) return; - const classLookup = findCppClassDefBySimpleName(simpleClassName); + const classLookup = findCppClassDefBySimpleName(idx, simpleClassName); if (classLookup === undefined) return; const { classDef, ambiguous } = classLookup; const nsQName = classToNamespaceQualifiedName.get(classDef.nodeId); @@ -762,11 +800,12 @@ function findNamespaceDefInScope(scope: { * ISO C++ `[basic.lookup.argdep]` §2: enumerations contribute their * enclosing namespace to the associated set, just like class types. */ function findCppClassDefBySimpleName( + idx: AdlCandidateIndex, simpleName: string, ): { classDef: SymbolDefinition; ambiguous: boolean } | undefined { // `classDefsBySimple` preserves `scopes.defs.byId` order, so `[0]` is the // legacy first-match and `length > 1` is the legacy `ambiguous` flag. - const matches = adlIndex?.classDefsBySimple.get(simpleName); + const matches = idx.classDefsBySimple.get(simpleName); if (matches === undefined) return undefined; const first = matches[0]; if (first === undefined) return undefined; @@ -780,10 +819,9 @@ function findCppClassDefBySimpleName( function collectFunctionTypeAssociatedNamespaces( refText: string, scopes: ScopeResolutionIndexes, + idx: AdlCandidateIndex, out: Set, ): void { - const idx = adlIndex; - if (idx === undefined) return; const colonIdx = refText.lastIndexOf('::'); if (colonIdx !== -1) { // Qualified ref: extract namespace prefix and normalise :: → dot notation. @@ -796,7 +834,7 @@ function collectFunctionTypeAssociatedNamespaces( // contributing `a` to the associated set (false-positive CALLS edge). const matches = idx.nsFunctionsByQName.get(nsText)?.get(simpleName); if (matches !== undefined) { - for (const def of matches) collectAssociatedNamespacesForFunctionDef(def, scopes, out); + for (const def of matches) collectAssociatedNamespacesForFunctionDef(def, scopes, idx, out); } return; } @@ -807,32 +845,34 @@ function collectFunctionTypeAssociatedNamespaces( // the function's own enclosing namespace. const matches = idx.nsFunctionsBySimple.get(refText); if (matches !== undefined) { - for (const def of matches) collectAssociatedNamespacesForFunctionDef(def, scopes, out); + for (const def of matches) collectAssociatedNamespacesForFunctionDef(def, scopes, idx, out); } } function collectAssociatedNamespacesForFunctionDef( def: SymbolDefinition, scopes: ScopeResolutionIndexes, + idx: AdlCandidateIndex, out: Set, ): void { const parameterTypes = def.parameterTypeClasses?.map((typeClass) => typeClass.base); for (const paramType of parameterTypes ?? def.parameterTypes ?? []) { - collectAssociatedNamespacesForFunctionTypeText(paramType, scopes, out); + collectAssociatedNamespacesForFunctionTypeText(paramType, scopes, idx, out); } if (def.returnType !== undefined) { - collectAssociatedNamespacesForFunctionTypeText(def.returnType, scopes, out); + collectAssociatedNamespacesForFunctionTypeText(def.returnType, scopes, idx, out); } } function collectAssociatedNamespacesForFunctionTypeText( typeText: string, scopes: ScopeResolutionIndexes, + idx: AdlCandidateIndex, out: Set, ): void { for (const token of extractCppTypeNameTokens(typeText)) { if (isIgnoredCppAdlNamespace(token.namespaceName)) continue; - addAssociatedNamespaceForClassName(token.simpleName, scopes, out); + addAssociatedNamespaceForClassName(token.simpleName, scopes, idx, out); if (token.namespaceName !== '') out.add(token.namespaceName); } } diff --git a/gitnexus/src/core/ingestion/languages/cpp/inline-namespaces.ts b/gitnexus/src/core/ingestion/languages/cpp/inline-namespaces.ts index 134738dfd..47f76d845 100644 --- a/gitnexus/src/core/ingestion/languages/cpp/inline-namespaces.ts +++ b/gitnexus/src/core/ingestion/languages/cpp/inline-namespaces.ts @@ -14,12 +14,14 @@ * 2. **Transitive qualified visibility.** `outer::foo()` resolves to * `outer::v1::foo()` when `v1` is inline. The qualified-namespace * receiver resolver (`resolveCppQualifiedNamespaceMember`) walks - * inline-namespace children transitively when collecting candidates. + * inline-namespace children transitively when collecting candidates — + * once per pipeline run, into {@link QualifiedNsMemberIndex} (#2788). * * State lifecycle: capture-time `markCppInlineNamespaceRange` records each * inline namespace's source range; `populateCppInlineNamespaceScopes` * resolves ranges to `ScopeId`s during `populateOwners`. Cleared via - * `clearCppInlineNamespaces`, called from `clearFileLocalNames`. + * `clearCppInlineNamespaces`, called from + * `cppScopeResolver.loadResolutionConfig` at the start of every pass. * * STL idiom this enables: `std::__1::vector` (libc++) and `std::__cxx11` * (libstdc++) are inline namespaces of `std`. With this support, @@ -32,7 +34,10 @@ import type { ScopeResolutionIndexes } from '../../model/scope-resolution-indexe import { isOverloadAmbiguousAfterNormalization, narrowOverloadCandidates, + type OverloadNarrowingHookCtx, } from '../../scope-resolution/passes/overload-narrowing.js'; +import { isOverloadableCallable } from '../../utils/callable-labels.js'; +import { isSemanticModelValidatorEnabled } from '../../utils/env.js'; import { CPP_CONVERSION_ONLY_ARG_TYPE_PREFIXES, cppConversionRank } from './conversion-rank.js'; interface RangeKey { @@ -44,6 +49,11 @@ interface RangeKey { const inlineNamespaceRangesByFile = new Map>(); const inlineNamespaceScopeIds = new Set(); +/** Bumped by every writer of {@link inlineNamespaceScopeIds}. The qualified-ns + * index is a function of both `parsedFiles` and that Set; its WeakMap key sees + * only the first, so the memo stamps this epoch and a missed + * {@link clearCppInlineNamespaces} degrades to a rebuild, not a stale answer. */ +let inlineNamespaceEpoch = 0; function rangeKey(r: RangeKey): string { return `${r.startLine}:${r.startCol}:${r.endLine}:${r.endCol}`; @@ -85,10 +95,19 @@ export function applyCppInlineNamespaceSideChannel( for (const r of ranges) set.add(r); } -/** Clear all inline-namespace state. Called from `clearFileLocalNames`. */ +/** Clear all inline-namespace state. Called from + * `cppScopeResolver.loadResolutionConfig` at the start of every pass. + * + * The qualified-namespace index is dropped by REASSIGNING a fresh `WeakMap` + * (`WeakMap` has no `.clear()`) so its entries — and the `SymbolDefinition` + * references they hold — are released here rather than waiting on the key. + * Correctness does not rest on that: {@link inlineNamespaceEpoch} invalidates + * any surviving entry. */ export function clearCppInlineNamespaces(): void { inlineNamespaceRangesByFile.clear(); inlineNamespaceScopeIds.clear(); + inlineNamespaceEpoch++; + qualifiedNsIndexByPass = new WeakMap(); } /** Resolve captured ranges to actual ScopeIds by matching scope ranges @@ -98,6 +117,7 @@ export function clearCppInlineNamespaces(): void { export function populateCppInlineNamespaceScopes(parsed: ParsedFile): void { const ranges = inlineNamespaceRangesByFile.get(parsed.filePath); if (ranges === undefined || ranges.size === 0) return; + inlineNamespaceEpoch++; for (const scope of parsed.scopes) { if (scope.kind !== 'Namespace') continue; if (ranges.has(rangeKey(scope.range))) { @@ -115,17 +135,316 @@ export function isCppInlineNamespaceScope(scopeId: ScopeId): boolean { } /** - * Walk every parsed file looking for a Namespace scope whose qualified - * name matches `receiverName`, collect its callable ownedDefs matching - * `memberName`, transitively descending into any inline-namespace - * children (since they're members of the enclosing namespace under ISO - * C++). + * Qualified-namespace member index — built **once** per pipeline run from + * `parsedFiles` and reused by every qualified call site. + * + * The legacy lookup re-scanned every parsed file (rebuilding a per-file + * `scopesById` map each time) once **per qualified call site**, making the + * scope-resolution emit phase O(callsites × scopes): 25.3 min of a 33-min + * analyze on a 1,473-file C++ repo, 75% of total self-time in this one + * function (#2788). Mirrors the same fix #1990 applied to ADL + * (`pickCppAdlCandidates` → {@link AdlCandidateIndex}); per-site cost drops + * to a Map lookup once a `(receiver, member)` pair has been resolved. + * + * **Why the index is a graph and not a flattened member table.** Every + * `Namespace` scope is a legal qualified receiver on its own — in + * `outer { inline v1 { inline v2 { … } } }`, `v2::foo()` is valid C++ — so an + * eager table has to record, for every namespace, every member of every inline + * descendant. On a chain of depth D that is D + (D−1) + … + 1 entries: quadratic + * in **both** time and memory, and materializing it recursively also recursed D + * deep. Neither bound was theoretical — a generated chain 6,000 deep with one + * function per level exhausted a 4 GB heap, and a member-less one 8,000 deep + * threw `RangeError: Maximum call stack size exceeded`. Nothing on the analyze + * path catches either (`run.ts` only wraps the CFG/PDG emit block; `phase.ts`'s + * `try` has a `finally` and no `catch`), so one pathological generated file + * aborted the whole `analyze` — the failure class of #2769. + * + * Storing the *shape* instead — each namespace's own members plus links to its + * inline children — makes the build linear in scopes+defs, and a call site pays + * one iterative pre-order walk of the receiver's inline subtree, memoized per + * `(receiver, member)`. Real inline nesting is 1–2 deep (`std::__1`, + * `std::__cxx11`), so that walk is 2–3 nodes. + */ +interface QualifiedNsMemberIndex { + /** + * Namespace simple name → the scopes declaring it, in the order the legacy + * linear scan visited them (file-major, then `parsed.scopes` declaration + * order). Nothing downstream observes that order — every multi-candidate + * path in {@link resolveCppQualifiedNamespaceMember} either narrows to a + * unique survivor or returns `'ambiguous'` — but it is free to preserve, and + * a future tie-break should inherit the legacy order rather than Map + * insertion happenstance. + */ + readonly rootsByReceiver: ReadonlyMap; + /** + * Memo of resolved candidate lists, receiver → member → defs. Filled on + * demand by {@link qualifiedNsMembers}; only pairs a call site actually asks + * for are ever materialized, which is what keeps the deep-chain case linear. + */ + readonly membersByReceiver: Map>; +} + +/** One `Namespace` scope as the qualified-lookup walk sees it. */ +interface QualifiedNsNode { + /** This scope's OWN callable defs bucketed by member simple name, in + * `ownedDefs` order. Computed once at build time, so a namespace with many + * members costs one pass no matter how many distinct members are queried. */ + readonly ownMembers: ReadonlyMap; + /** Inline-namespace children in `parsed.scopes` declaration order. Direct + * object links rather than `ScopeId` lookups, so the walk needs no scope + * table and ids that repeat across files can never splice one file's + * children under another file's namespace. */ + readonly inlineChildren: readonly QualifiedNsNode[]; +} + +/** Build-time view of {@link QualifiedNsNode}: `inlineChildren` is appended to + * as the build's second pass links each child, then only ever read. */ +interface MutableNsNode extends QualifiedNsNode { + readonly inlineChildren: QualifiedNsNode[]; +} + +type NsScope = ParsedFile['scopes'][number]; + +/** + * Dev/test-only mutation tripwire on the arrays this module shares across call + * sites ({@link NO_DEFS} and every `membersByReceiver` memo entry): an in-place + * `.sort()`/`.splice()` ever added to `overload-narrowing.ts` would corrupt + * every LATER resolution of the same pair, not just its own call, and freezing + * makes that a loud `TypeError` instead. DEV-ONLY because the types already + * reject it — the memo and `narrowOverloadCandidates`' parameter are both + * `readonly SymbolDefinition[]`, so only a cast gets past — while the freeze is + * not free: V8 moves a frozen array to `PACKED_FROZEN_ELEMENTS`, off the + * builtin fast path for the `.filter`/`.map`/`.some` narrowing runs over it at + * every multi-candidate call site (measured 4.6×; large bench arm 100.3 → 70.6ms + * with it off). Gated on `isSemanticModelValidatorEnabled()` — the OPT-IN form, + * not `adl.ts`'s opt-out `NODE_ENV !== 'production'`: `NODE_ENV` is unset in a + * CLI `analyze`, so the opt-out form would keep paying the freeze exactly where + * the cost lands. Read once at load because this one is per-call-site. + */ +const FREEZE_SHARED_CANDIDATES = isSemanticModelValidatorEnabled(); + +/** Shared empties: most namespace scopes declare no callables, and most + * qualified receivers name no namespace at all. `NO_DEFS` is process-wide, so + * it gets the same dev-only freeze the memoized arrays do. */ +const NO_MEMBERS: ReadonlyMap = new Map(); +const NO_DEFS: readonly SymbolDefinition[] = FREEZE_SHARED_CANDIDATES ? Object.freeze([]) : []; + +/** Simple (last-segment) name of a def, matching the legacy scan exactly — + * including the empty-string fallback for a def with no `qualifiedName`, and + * the empty last segment of a trailing-dot name. `lastIndexOf` + `slice` + * rather than `split('.').pop()`: same result, no intermediate array, and this + * runs per callable def and per namespace scope at build. */ +function simpleNameOf(def: SymbolDefinition): string { + const qualified = def.qualifiedName; + if (qualified == null) return ''; + const lastDot = qualified.lastIndexOf('.'); + return lastDot === -1 ? qualified : qualified.slice(lastDot + 1); +} + +/** + * Per-pass memo: `parsedFiles` array identity → the built index. `WeakMap`-keyed + * so the entry lives only as long as the caller's `parsedFiles` array does, and + * is reclaimed with the pass — mirrors `moduleScopeIndexByPass` in + * `file-local-linkage.ts`. + * + * Weak keying is load-bearing, not stylistic: the index holds + * `SymbolDefinition` references reaching into every ParsedFile's scopes, so a + * module-level strong `let` pair (index + source array) kept the whole C/C++ + * ParsedFile set alive past the point `scope-resolution/pipeline/phase.ts` + * evicts `files`/`contents`/`preExtractedByPath` and calls `forceGc()` to + * reclaim it — C++ is 7th of 16 `SCOPE_RESOLVERS` entries, so the retention + * survived nine later language passes plus emit (104.2 MB measured). + */ +let qualifiedNsIndexByPass = new WeakMap(); + +/** Memo cell: the index plus the {@link inlineNamespaceEpoch} it was built under. */ +interface MemoizedIndex { + readonly epoch: number; + readonly index: QualifiedNsMemberIndex; +} + +/** Build the index in two linear passes per file: one to make a node per + * `Namespace` scope, one to link inline children and register receivers. + * Linking needs both endpoints to exist and `parsed.scopes` does not promise + * parents precede children, hence two passes rather than one — but only the + * first pass filters `parsed.scopes`; it hands the second the `[scope, node]` + * pairs it already found. */ +function buildQualifiedNsMemberIndex(parsedFiles: readonly ParsedFile[]): QualifiedNsMemberIndex { + const rootsByReceiver = new Map(); + + for (const parsed of parsedFiles) { + const nodesByScope = new Map(); + const namespaces: [NsScope, MutableNsNode][] = []; + for (const sc of parsed.scopes) { + if (sc.kind !== 'Namespace') continue; + const node: MutableNsNode = { ownMembers: bucketOwnMembers(sc), inlineChildren: [] }; + nodesByScope.set(sc.id, node); + namespaces.push([sc, node]); + } + + for (const [sc, node] of namespaces) { + // A non-`Namespace` parent (a Module scope, say) has no node, so the + // inline child links to nothing — hence the `?.` below. + if (sc.parent !== null && inlineNamespaceScopeIds.has(sc.id)) { + nodesByScope.get(sc.parent)?.inlineChildren.push(node); + } + const nsDef = findNamespaceDefInScope(sc); + if (nsDef === undefined) continue; + const nsName = simpleNameOf(nsDef); + let roots = rootsByReceiver.get(nsName); + if (roots === undefined) { + roots = []; + rootsByReceiver.set(nsName, roots); + } + roots.push(node); + } + } + return { rootsByReceiver, membersByReceiver: new Map() }; +} + +/** Bucket a namespace scope's own callable `ownedDefs` by member simple name. + * No descent: inline children are separate nodes reached by the walk. */ +function bucketOwnMembers(scope: NsScope): ReadonlyMap { + let members: Map | undefined; + for (const def of scope.ownedDefs) { + if (!isOverloadableCallable(def.type)) continue; + const simple = simpleNameOf(def); + members ??= new Map(); + let arr = members.get(simple); + if (arr === undefined) { + arr = []; + members.set(simple, arr); + } + arr.push(def); + } + return members ?? NO_MEMBERS; +} + +/** Candidates for `receiver::member`, memoized on the index per pair so a + * repeated call site costs two Map lookups. */ +function qualifiedNsMembers( + index: QualifiedNsMemberIndex, + receiverName: string, + memberName: string, +): readonly SymbolDefinition[] { + const roots = index.rootsByReceiver.get(receiverName); + // No namespace by that name — the most common outcome in real source. Left + // unmemoized deliberately: it is already O(1), and memoizing would grow the + // index by an entry per unresolved receiver name in the workspace. + if (roots === undefined) return NO_DEFS; + let byMember = index.membersByReceiver.get(receiverName); + if (byMember === undefined) { + byMember = new Map(); + index.membersByReceiver.set(receiverName, byMember); + } + let hits = byMember.get(memberName); + if (hits === undefined) { + // Memoization makes this array shared by every call site in the pass rather + // than rebuilt per call, so it carries the dev-only mutation tripwire (see + // {@link FREEZE_SHARED_CANDIDATES}). + hits = gatherQualifiedNsMember(roots, memberName); + if (FREEZE_SHARED_CANDIDATES) hits = Object.freeze(hits); + byMember.set(memberName, hits); + } + return hits; +} + +/** Pre-order depth-first walk of every scope named `receiverName` and its + * transitive inline-namespace children, collecting callables named + * `memberName` — the index twin of the legacy + * `findMemberInNamespaceTransitive`. + * + * Iterative, not recursive: nesting depth is whatever the input file says, and + * a recursive descent threw `RangeError` past ~8k levels (see + * {@link QualifiedNsMemberIndex}). */ +function gatherQualifiedNsMember( + roots: readonly QualifiedNsNode[], + memberName: string, +): readonly SymbolDefinition[] { + const hits: SymbolDefinition[] = []; + const stack: QualifiedNsNode[] = []; + // `visited` spans ALL roots, reproducing the legacy per-receiver `seenNodeId` + // dedup: `namespace ns { inline namespace ns { … } }` registers both scopes + // under receiver `ns`, and the outer walk already collected the inner's defs. + // Node identity is equivalent to the legacy nodeId key because a node always + // contributes the same defs. It doubles as the guard that a malformed parent + // cycle terminates instead of spinning — the one regression an explicit stack + // could otherwise introduce over recursion's `RangeError`. + const visited = new Set(); + for (const root of roots) { + stack.push(root); + while (stack.length > 0) { + const node = stack.pop(); + if (visited.has(node)) continue; + visited.add(node); + const own = node.ownMembers.get(memberName); + if (own !== undefined) for (const def of own) hits.push(def); + // Reverse push so children pop in declaration order — the legacy walk's + // pre-order depth-first candidate order (see + // {@link QualifiedNsMemberIndex.rootsByReceiver}). + const kids = node.inlineChildren; + for (let i = kids.length - 1; i >= 0; i--) stack.push(kids[i]); + } + } + return hits.length === 0 ? NO_DEFS : hits; +} + +/** Build the index on first use of a given `parsedFiles` set; reuse it for + * every subsequent call site that passes the same array reference AND the same + * {@link inlineNamespaceEpoch}. + * + * Both keys are needed because the index is a function of TWO inputs: + * `parsedFiles` and the module-level `inlineNamespaceScopeIds` (which inline + * children get descended into). A later pass may hand back the same + * `parsedFiles` reference with different inline-namespace state, so identity + * alone would serve a stale index — the epoch is what makes that a checked + * rebuild instead of a documented obligation on the clear site. + * + * Lifetime: the entry is reachable only while the caller still holds the + * `parsedFiles` array; nothing here pins it (see {@link qualifiedNsIndexByPass}). + * + * Same STALENESS contract as `ensureAdlIndex` in `adl.ts`, minus that + * sibling's dev-gated `validateAdlSeqCoverage` guard: this index has no `?? 0` + * defaulting read to silently collide on, so there is nothing for such a guard + * to catch. */ +function qualifiedNsMemberIndex(parsedFiles: readonly ParsedFile[]): QualifiedNsMemberIndex { + const memo = qualifiedNsIndexByPass.get(parsedFiles); + if (memo !== undefined && memo.epoch === inlineNamespaceEpoch) return memo.index; + const index = buildQualifiedNsMemberIndex(parsedFiles); + qualifiedNsIndexByPass.set(parsedFiles, { epoch: inlineNamespaceEpoch, index }); + return index; +} + +/** Constant narrowing hooks for the C++ qualified-receiver path — hoisted so + * the literal is not reallocated at every multi-candidate call site. */ +const CPP_NARROWING_HOOKS: OverloadNarrowingHookCtx = { + conversionRankFn: cppConversionRank, + conversionOnlyArgTypePrefixes: CPP_CONVERSION_ONLY_ARG_TYPE_PREFIXES, +}; + +/** + * Find the Namespace scopes whose simple name matches `receiverName` and + * return their callable members matching `memberName`, transitively + * including inline-namespace children (since they're members of the + * enclosing namespace under ISO C++). Served from a per-pipeline index + * ({@link QualifiedNsMemberIndex}), not a per-call-site workspace scan. * * Returns the most specific (innermost) match — for `outer::foo()` * where `inline namespace v1` declares `foo`, returns `v1::foo`. When * multiple inline-namespace children declare the same name, ISO C++ * leaves the call ambiguous; returns `'ambiguous'` so the caller * suppresses edge emission rather than picking arbitrarily (#1564). + * + * Two production call sites, both in `scope-resolver.ts`: + * - `resolveQualifiedReceiverMember` — the `outer::foo()` qualified-receiver + * hook. Passes `callsite`, so a multi-candidate set can be narrowed by + * arity and argument types. + * - `resolveAdlCandidates` — resolving a `using ns::name;` named import back + * to its namespace member, for template-class method bodies where the + * lexical walk misses that visibility. Passes NO `callsite`, which makes + * the narrowing filters below pass-throughs: on that path any receiver + * whose member has more than one candidate returns `'ambiguous'` (and the + * caller then skips it) rather than being disambiguated. */ export function resolveCppQualifiedNamespaceMember( receiverName: string, @@ -134,27 +453,7 @@ export function resolveCppQualifiedNamespaceMember( _scopes: ScopeResolutionIndexes, callsite?: Callsite, ): SymbolDefinition | 'ambiguous' | undefined { - const allHits: SymbolDefinition[] = []; - const seenNodeId = new Set(); - for (const parsed of parsedFiles) { - const scopesById = new Map(); - for (const sc of parsed.scopes) scopesById.set(sc.id, sc); - for (const scope of parsed.scopes) { - if (scope.kind !== 'Namespace') continue; - const nsDef = findNamespaceDefInScope(scope); - if (nsDef === undefined) continue; - const nsName = nsDef.qualifiedName?.split('.').pop() ?? nsDef.qualifiedName ?? ''; - if (nsName !== receiverName) continue; - // Found a matching namespace scope in this file. Collect ALL - // members transitively through any inline-namespace children. - const hits = findMemberInNamespaceTransitive(scope, scopesById, memberName); - for (const hit of hits) { - if (seenNodeId.has(hit.nodeId)) continue; - seenNodeId.add(hit.nodeId); - allHits.push(hit); - } - } - } + const allHits = qualifiedNsMembers(qualifiedNsMemberIndex(parsedFiles), receiverName, memberName); if (allHits.length === 0) return undefined; if (allHits.length === 1) return allHits[0]; @@ -167,12 +466,7 @@ export function resolveCppQualifiedNamespaceMember( allHits, callsite?.arity, callsite?.argumentTypes, - callsite !== undefined - ? { - conversionRankFn: cppConversionRank, - conversionOnlyArgTypePrefixes: CPP_CONVERSION_ONLY_ARG_TYPE_PREFIXES, - } - : undefined, + callsite !== undefined ? CPP_NARROWING_HOOKS : undefined, ); if (narrowed.length === 1) return narrowed[0]; if (narrowed.length === 0) return undefined; @@ -182,46 +476,6 @@ export function resolveCppQualifiedNamespaceMember( return 'ambiguous'; } -/** Recursively search a namespace scope and any inline-namespace - * descendants for callable defs with the given simple name. Non-inline - * nested namespaces are NOT traversed — they require explicit - * qualification (`outer::nested::foo`). Returns ALL matches so the - * caller can detect same-name ambiguity across inline children (#1564). */ -function findMemberInNamespaceTransitive( - scope: { - readonly id: ScopeId; - readonly ownedDefs: readonly SymbolDefinition[]; - readonly parent: ScopeId | null; - }, - scopesById: ReadonlyMap< - ScopeId, - { - readonly id: ScopeId; - readonly kind: string; - readonly parent: ScopeId | null; - readonly ownedDefs: readonly SymbolDefinition[]; - } - >, - memberName: string, -): SymbolDefinition[] { - const results: SymbolDefinition[] = []; - // Check this scope's own ownedDefs first. - for (const def of scope.ownedDefs) { - if (def.type !== 'Function' && def.type !== 'Method' && def.type !== 'Constructor') continue; - const simple = def.qualifiedName?.split('.').pop() ?? def.qualifiedName ?? ''; - if (simple === memberName) results.push(def); - } - // Descend into inline-namespace children. - for (const childScope of scopesById.values()) { - if (childScope.parent !== scope.id) continue; - if (childScope.kind !== 'Namespace') continue; - if (!inlineNamespaceScopeIds.has(childScope.id)) continue; - const childHits = findMemberInNamespaceTransitive(childScope, scopesById, memberName); - for (const hit of childHits) results.push(hit); - } - return results; -} - function findNamespaceDefInScope(scope: { readonly ownedDefs: readonly SymbolDefinition[]; }): SymbolDefinition | undefined { diff --git a/gitnexus/src/core/ingestion/scope-resolution/graph-bridge/ids.ts b/gitnexus/src/core/ingestion/scope-resolution/graph-bridge/ids.ts index abe8b48e4..6f942d88b 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/graph-bridge/ids.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/graph-bridge/ids.ts @@ -54,17 +54,19 @@ import { definitionIdPosition } from '../utils/definition-id.js'; * restricted to function/class-likes, those calls correctly fall * through to the File-node fallback at the bottom of the walk. */ +export const CALLER_ANCHOR_LABELS: ReadonlySet = new Set([ + 'Function', + 'Method', + 'Constructor', + 'Module', + 'Class', + 'Interface', + 'Struct', + 'Enum', +]); + function isCallerAnchorLabel(label: NodeLabel): boolean { - return ( - label === 'Function' || - label === 'Method' || - label === 'Constructor' || - label === 'Module' || - label === 'Class' || - label === 'Interface' || - label === 'Struct' || - label === 'Enum' - ); + return CALLER_ANCHOR_LABELS.has(label); } function rangeContainsPoint( diff --git a/gitnexus/src/core/ingestion/scope-resolution/graph-bridge/node-lookup.ts b/gitnexus/src/core/ingestion/scope-resolution/graph-bridge/node-lookup.ts index 1c6b94410..a8f90393a 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/graph-bridge/node-lookup.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/graph-bridge/node-lookup.ts @@ -244,38 +244,53 @@ export function buildGraphNodeLookup(graph: KnowledgeGraph): GraphNodeLookup { return lookup; } +/** + * Every label {@link buildGraphNodeLookup} registers — and therefore the ONLY + * labels `resolveDefGraphId` can ever return an id for. Both endpoints of every + * scope-resolution edge come from that lookup (the one exception is the File + * fallback in `resolveCallerGraphId`), so this set defines the whole FROM/TO + * surface those edges can produce. + * + * That makes it load-bearing for the LadybugDB relation DDL: a label added here + * without the matching `FROM x TO y` pairs in `RELATION_SCHEMA` crashes + * `analyze` at `assertDeclaredPair` on whichever codebase first emits the pair + * (#2792). `test/unit/schema-pair-coverage.test.ts` derives the required pairs + * from this set and fails in CI instead. + */ +export const LINKABLE_LABELS: ReadonlySet = new Set([ + 'Function', + 'Method', + 'Constructor', + // Program-like module declarations are provider-gated callable-value + // targets and need the same def→graph bridge. + 'Module', + 'Class', + 'Interface', + 'Struct', + 'Enum', + // Trait nodes are linkable so MRO builders can bridge PHP/Rust trait + // defs between scope-resolution DefIds and the graph's node ids. + // IMPLEMENTS edges from classes to traits are otherwise invisible to + // the scope-resolution MRO pass. + 'Trait', + // Variable / Property are linkable too — receiver-bound write/read + // ACCESSES edges target field nodes (e.g. `user.name = "x"` → + // ACCESSES edge to User's `name` Variable/Property node). + 'Variable', + 'Property', + // Const is linkable so the value-receiver-owner bridge in + // `receiver-bound-calls.ts` Case 5 can translate the scope-resolution + // `Variable` def for `export const fooService = {...}` to the canonical + // `Const:filePath:name` graph node id, against which object-literal + // method symbols register their `ownerId` (PR #1718 / issue #1358). + 'Const', + // Macro nodes are linkable so a macro invocation (`log!(…)`) resolved + // via `MacroRegistry` can bridge its scope-resolution `Macro` def to + // the legacy `@definition.macro` graph node and emit the `USES` edge + // (Rust #1934 F72; also covers C/C++ `#define` macro defs). + 'Macro', +]); + export function isLinkableLabel(label: NodeLabel): boolean { - return ( - label === 'Function' || - label === 'Method' || - label === 'Constructor' || - // Program-like module declarations are provider-gated callable-value - // targets and need the same def→graph bridge. - label === 'Module' || - label === 'Class' || - label === 'Interface' || - label === 'Struct' || - label === 'Enum' || - // Trait nodes are linkable so MRO builders can bridge PHP/Rust trait - // defs between scope-resolution DefIds and the graph's node ids. - // IMPLEMENTS edges from classes to traits are otherwise invisible to - // the scope-resolution MRO pass. - label === 'Trait' || - // Variable / Property are linkable too — receiver-bound write/read - // ACCESSES edges target field nodes (e.g. `user.name = "x"` → - // ACCESSES edge to User's `name` Variable/Property node). - label === 'Variable' || - label === 'Property' || - // Const is linkable so the value-receiver-owner bridge in - // `receiver-bound-calls.ts` Case 5 can translate the scope-resolution - // `Variable` def for `export const fooService = {...}` to the canonical - // `Const:filePath:name` graph node id, against which object-literal - // method symbols register their `ownerId` (PR #1718 / issue #1358). - label === 'Const' || - // Macro nodes are linkable so a macro invocation (`log!(…)`) resolved - // via `MacroRegistry` can bridge its scope-resolution `Macro` def to - // the legacy `@definition.macro` graph node and emit the `USES` edge - // (Rust #1934 F72; also covers C/C++ `#define` macro defs). - label === 'Macro' - ); + return LINKABLE_LABELS.has(label); } diff --git a/gitnexus/src/core/lbug/csv-generator.ts b/gitnexus/src/core/lbug/csv-generator.ts index 68a66aa0e..4444a9687 100644 --- a/gitnexus/src/core/lbug/csv-generator.ts +++ b/gitnexus/src/core/lbug/csv-generator.ts @@ -17,8 +17,8 @@ import { createWriteStream, WriteStream } from 'fs'; import path from 'path'; import type { GraphNode, GraphRelationship } from 'gitnexus-shared'; import { KnowledgeGraph } from '../graph/types.js'; -import { NodeTableName, NODE_TABLES, RELATION_SCHEMA } from './schema.js'; -import { parseRelationSchemaPairs, RelPairRouter } from './rel-pair-routing.js'; +import { NodeTableName, RELATION_SCHEMA } from './schema.js'; +import { VALID_NODE_TABLES, parseRelationSchemaPairs, RelPairRouter } from './rel-pair-routing.js'; import { parseTruthyEnv } from '../ingestion/utils/env.js'; import { SYMBOL_NODE_LABELS } from '../ingestion/utils/symbol-labels.js'; import { applyCjkSegmentationIfEnabled } from '../search/cjk-segmentation.js'; @@ -793,13 +793,13 @@ export const streamAllCSVsToDisk = async ( const relRouter = new RelPairRouter( csvDir, REL_CSV_HEADER, - new Set(NODE_TABLES), + VALID_NODE_TABLES, DECLARED_RELATION_PAIRS, ); try { let emitted = 0; for (const rel of orderedRelationships(graph, sortOutput)) { - const pending = relRouter.route(rel.sourceId, rel.targetId, buildRelRow(rel)); + const pending = relRouter.route(rel.sourceId, rel.targetId, buildRelRow(rel), rel.type); if (pending) await pending; // Periodically hand the event loop back so the overlapped node COPY and // write-stream drains run instead of starving behind this synchronous diff --git a/gitnexus/src/core/lbug/graph-emit-sink.ts b/gitnexus/src/core/lbug/graph-emit-sink.ts index a0491ab14..f1527c6d2 100644 --- a/gitnexus/src/core/lbug/graph-emit-sink.ts +++ b/gitnexus/src/core/lbug/graph-emit-sink.ts @@ -85,9 +85,10 @@ * ## Correctness contract * * Structural sibling of {@link PdgEmitSink}, and reuses its row builder - * (`buildRelRow`), header (`REL_CSV_HEADER`), label derivation (`getNodeLabel`) - * and `RelPairRouter` validity check, so the streamed row SET equals the - * whole-graph emit's and the bulk COPY loads the same rows. Set-level, not + * (`buildRelRow`), header (`REL_CSV_HEADER`) and pair classification + * (`relPairKeyFor`, which is also what `RelPairRouter` routes and skips by), so + * the streamed row SET equals the whole-graph emit's and the bulk COPY loads + * the same rows. Set-level, not * byte-level: rows stream in emit order and are not re-sorted under * `GITNEXUS_SORT_GRAPH_OUTPUT`. */ @@ -96,8 +97,12 @@ import path from 'path'; import type { GraphNode, GraphRelationship, RelationshipType } from 'gitnexus-shared'; import type { KnowledgeGraph } from '../graph/types.js'; import { DECLARED_RELATION_PAIRS, REL_CSV_HEADER, buildRelRow } from './csv-generator.js'; -import { assertDeclaredPair, getNodeLabel } from './rel-pair-routing.js'; -import { NODE_TABLES } from './schema.js'; +import { + VALID_NODE_TABLES, + assertDeclaredPair, + relPairKeyFor, + splitRelPairKey, +} from './rel-pair-routing.js'; import { DEFAULT_EMIT_CHUNK_ROWS, SyncCsvWriter } from './sync-csv-writer.js'; /** @@ -229,7 +234,6 @@ export class StreamedRelationshipRemovalError extends Error { * {@link finalize} once after the pipeline, before `loadGraphToLbug`. */ export class GraphEmitSink implements KnowledgeGraph, GraphEmitControl { - private readonly validTables: Set; private readonly relWriters = new Map(); /** * Ids of relationships already streamed. `KnowledgeGraph.addRelationship` @@ -303,7 +307,6 @@ export class GraphEmitSink implements KnowledgeGraph, GraphEmitControl { private readonly csvDir: string, private readonly chunkRows: number = DEFAULT_EMIT_CHUNK_ROWS, ) { - this.validTables = new Set(NODE_TABLES as readonly string[]); // Own directory, distinct from the PDG sink's: PdgEmitSink wipes and // recreates its dir on construction and opens with O_EXCL, so a shared dir // would destroy the other sink's manifest on a combined --pdg run. @@ -407,16 +410,24 @@ export class GraphEmitSink implements KnowledgeGraph, GraphEmitControl { } // Mirror KnowledgeGraph.addRelationship's first-writer-wins dedup. - const fromLabel = getNodeLabel(relationship.sourceId); - const toLabel = getNodeLabel(relationship.targetId); - // Skip edges whose endpoint labels are not valid node tables — mirrors - // `RelPairRouter` exactly so the streamed set matches the whole-graph set. - if (!this.validTables.has(fromLabel) || !this.validTables.has(toLabel)) return; + // Classify + skip via the SHARED `relPairKeyFor`, not a local copy of its + // three lines, so the streamed set cannot drift from the whole-graph set + // `RelPairRouter` produces. `undefined` = an endpoint label is not a node + // table, so the edge is dropped exactly as the router drops it. + const pairKey = relPairKeyFor(relationship.sourceId, relationship.targetId, VALID_NODE_TABLES); + if (pairKey === undefined) return; - const pairKey = `${fromLabel}|${toLabel}`; - assertDeclaredPair(pairKey, DECLARED_RELATION_PAIRS); + assertDeclaredPair( + pairKey, + DECLARED_RELATION_PAIRS, + relationship.type, + relationship.sourceId, + relationship.targetId, + ); let writer = this.relWriters.get(pairKey); if (writer === undefined) { + // Cold: once per pair, so decoding the key back into its labels is free. + const [fromLabel, toLabel] = splitRelPairKey(pairKey); try { writer = new SyncCsvWriter( path.join(this.csvDir, `rel_${fromLabel}_${toLabel}.csv`), diff --git a/gitnexus/src/core/lbug/pdg-emit-sink.ts b/gitnexus/src/core/lbug/pdg-emit-sink.ts index cc5560d2a..78d337816 100644 --- a/gitnexus/src/core/lbug/pdg-emit-sink.ts +++ b/gitnexus/src/core/lbug/pdg-emit-sink.ts @@ -24,8 +24,8 @@ * `storage/parsedfile-store.ts`. * * Byte-identity (issue acceptance): the sink reuses the SAME shared row - * builders (`buildBasicBlockRow`, `buildRelRow`) and label derivation - * (`getNodeLabel`) as `streamAllCSVsToDisk`, so the streamed CSV line SET is + * builders (`buildBasicBlockRow`, `buildRelRow`) and pair classification + * (`relPairKeyFor`) as `streamAllCSVsToDisk`, so the streamed CSV line SET is * identical to the whole-graph emit's, and the bulk COPY loads the same rows → * the persisted graph is SET-identical and DB-identical. The guarantee is * set-level, not byte-level on the CSV file: the sink streams rows in emit @@ -54,9 +54,14 @@ import { buildBasicBlockRow, buildRelRow, } from './csv-generator.js'; -import { assertDeclaredPair, getNodeLabel } from './rel-pair-routing.js'; +import { + VALID_NODE_TABLES, + assertDeclaredPair, + relPairKeyFor, + splitRelPairKey, +} from './rel-pair-routing.js'; import { DEFAULT_EMIT_CHUNK_ROWS, SyncCsvWriter } from './sync-csv-writer.js'; -import { NODE_TABLES, type NodeTableName } from './schema.js'; +import { type NodeTableName } from './schema.js'; /** * PDG edge types streamed per-file (all intra-block BasicBlock→BasicBlock). @@ -98,7 +103,6 @@ export interface PdgEmitManifest { * `--pdg` emit, then {@link finalize} once after the last language. */ export class PdgEmitSink implements KnowledgeGraph { - private readonly validTables: Set; private bbWriter: SyncCsvWriter | undefined; /** pairKey (`From|To`) → writer. PDG edges are all `BasicBlock|BasicBlock`, * but the map keeps the sink general and the manifest pair-keyed. */ @@ -129,7 +133,6 @@ export class PdgEmitSink implements KnowledgeGraph { private readonly pdgCsvDir: string, private readonly chunkRows: number = DEFAULT_PDG_EMIT_CHUNK_ROWS, ) { - this.validTables = new Set(NODE_TABLES as readonly string[]); // Clear any streamed CSVs left by a previous (possibly crashed) run so a // later COPY never picks up stale rows. fs.rmSync(pdgCsvDir, { recursive: true, force: true }); @@ -160,15 +163,27 @@ export class PdgEmitSink implements KnowledgeGraph { addRelationship(relationship: GraphRelationship): void { if (PDG_EDGE_TYPES.has(relationship.type)) { - const fromLabel = getNodeLabel(relationship.sourceId); - const toLabel = getNodeLabel(relationship.targetId); - // Skip edges whose endpoint labels are not valid node tables — mirrors - // `RelPairRouter` exactly so the streamed set matches the whole-graph set. - if (!this.validTables.has(fromLabel) || !this.validTables.has(toLabel)) return; - const pairKey = `${fromLabel}|${toLabel}`; - assertDeclaredPair(pairKey, DECLARED_RELATION_PAIRS); + // Classify + skip via the SHARED `relPairKeyFor`, not a local copy of its + // three lines, so the streamed set cannot drift from the whole-graph set + // `RelPairRouter` produces. `undefined` = an endpoint label is not a node + // table, so the edge is dropped exactly as the router drops it. + const pairKey = relPairKeyFor( + relationship.sourceId, + relationship.targetId, + VALID_NODE_TABLES, + ); + if (pairKey === undefined) return; + assertDeclaredPair( + pairKey, + DECLARED_RELATION_PAIRS, + relationship.type, + relationship.sourceId, + relationship.targetId, + ); let writer = this.relWriters.get(pairKey); if (writer === undefined) { + // Cold: once per pair, so decoding the key back into labels is free. + const [fromLabel, toLabel] = splitRelPairKey(pairKey); try { writer = new SyncCsvWriter( path.join(this.pdgCsvDir, `rel_${fromLabel}_${toLabel}.csv`), diff --git a/gitnexus/src/core/lbug/rel-pair-routing.ts b/gitnexus/src/core/lbug/rel-pair-routing.ts index 5a77cec0b..56b95d0aa 100644 --- a/gitnexus/src/core/lbug/rel-pair-routing.ts +++ b/gitnexus/src/core/lbug/rel-pair-routing.ts @@ -31,10 +31,29 @@ import path from 'path'; import { createWriteStream, type WriteStream } from 'fs'; import { once } from 'events'; import { finished } from 'stream/promises'; +import { NODE_TABLES } from 'gitnexus-shared'; +import { findInCauseChain } from '../../lib/utils.js'; /** Injectable for tests (backpressure/error simulation), mirroring split. */ export type WriteStreamFactory = (filePath: string) => WriteStream; +/** + * Every label LadybugDB has a node table for — the filter that decides whether + * an edge is routable at all. + * + * ONE shared instance, deliberately. `RelPairRouter`, `GraphEmitSink` and + * `PdgEmitSink` each used to build their own `new Set(NODE_TABLES)`; three + * copies of the same immutable set are three chances to seed one of them from + * a different source. Typed `ReadonlySet` because that — not `Object.freeze`, + * which does not touch a Set's internal slots — is what actually stops a + * consumer mutating the shared instance. + * + * Imported straight from `gitnexus-shared` rather than `./schema.js`: schema.ts + * imports `parseRelationSchemaPairs` from this module, so the reverse import + * would close a cycle. + */ +export const VALID_NODE_TABLES: ReadonlySet = new Set(NODE_TABLES); + /** * Derive a node's table label from its graph id. Matches the legacy * `getNodeLabel` that lived inline in `loadGraphToLbug`: @@ -48,20 +67,92 @@ export const getNodeLabel = (nodeId: string): string => { return nodeId.split(':')[0]; }; +/** + * Classify one edge into its `From|To` pair key, or `undefined` when the edge + * must be SKIPPED because an endpoint's label is not a real node table. + * + * THE single definition of "which pair does this edge belong to, and is it + * routable at all". `RelPairRouter.route`, `GraphEmitSink.addRelationship`, + * `PdgEmitSink.addRelationship` and the `structural-pair-coverage` corpus guard + * each used to inline the same three lines (label both ends → drop if either + * label is not a node table → join with `|`). The corpus guard's docblock said + * it "mirrors `RelPairRouter.route`" — a mirror is a drift marker: change the + * skip rule here and the guard would keep classifying by the old one, report + * green, and let `analyze` abort on a pair it had already declared covered. + * + * HOT PATH — called once per edge (~1M on a large repo). Returns the key + * string (which every caller needs anyway for its own Map lookup) rather than + * a `{ pairKey, fromLabel, toLabel }` object or a tuple, so the success path + * allocates nothing beyond what `getNodeLabel` already did. Callers that need + * the two labels back — only when opening a new pair's CSV, once per pair — + * decode the key with {@link splitRelPairKey}. + */ +export const relPairKeyFor = ( + fromId: string, + toId: string, + validTables: ReadonlySet, +): string | undefined => { + const fromLabel = getNodeLabel(fromId); + const toLabel = getNodeLabel(toId); + if (!validTables.has(fromLabel) || !validTables.has(toLabel)) return undefined; + return `${fromLabel}|${toLabel}`; +}; + +/** + * Decode a `From|To` pair key back into its two labels. + * + * Safe because `|` cannot occur inside a node label: every label is a + * `NODE_TABLES` identifier (`[A-Za-z][A-Za-z0-9_]*`), so the FIRST `|` is + * always the separator. That invariant was documented in one comment and + * enforced nowhere while every consumer re-derived it with a bare + * `key.split('|')`. + * + * DECODE ONLY — there is deliberately no matching `encode` helper. The key is + * built once per edge inside {@link relPairKeyFor} (~1M edges on a large + * repo), where a function call is a real regression risk; every decode site is + * cold by construction (once per pair when its CSV is opened, or on the + * throw path of {@link assertDeclaredPair}). + */ +export const splitRelPairKey = (key: string): readonly [from: string, to: string] => { + const sep = key.indexOf('|'); + return sep < 0 ? [key, ''] : [key.slice(0, sep), key.slice(sep + 1)]; +}; + +/** + * Build a fresh matcher for the `FROM