From 26894d835bc29251e3273ff770a6703954901ee8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Fri, 29 May 2026 20:24:58 +0100 Subject: [PATCH] fix(csharp): eliminate namespace-siblings OOM and worker-path re-parse (#1905) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(csharp): eliminate O(S·D) BindingRef OOM in namespace siblings Types declared in the C# global (default) namespace are visible from every file, so the previous per-scope augmentation materialized O(scopes × defs) BindingRefs — on large Unity solutions (tens of thousands of global types) this caused severe slowness and OOM. Route global-namespace types through a single workspace-level binding channel (workspaceFqnBindings, consulted by lookupBindingsAt) for O(D) memory. Also fix quadratic costs in the non-global path: append defs in place instead of copying (was O(D²) per bucket), pre-index the first scope per file (was O(S²·D)), and seed de-dup sets instead of repeated .some scans. Add csharp-pipeline-benchmark.test.ts (mirrors the PHP benchmark) with spread and concentrated-global-namespace scenarios to track elapsedMs, peakHeapMB, nodeCount, and edgeCount. Post-fix runs show linear scaling and stable heap. Co-authored-by: Cursor * perf(csharp): scanner fallback for namespace siblings on the worker path Worker threads can't return tree-sitter Trees across MessageChannels, so the cross-phase tree cache is empty for worker-parsed files. The C# same-namespace pass (populateCsharpNamespaceSiblings -> extractFileStructure) then re-parsed every file with tree-sitter to find namespace / using-static nodes — effectively parsing a large solution a second time during scope resolution. Add a line-scanner fallback (extractCsharpStructureViaScanner) used only when no cached Tree is available, mirroring PHP's fix for issue #1741. It extracts the same namespaces / usingStaticPaths the AST walk produces for the common line-anchored forms (file-scoped + block namespaces, plain / global / aliased `using static`). The AST walk stays authoritative on the sequential / warm-cache path. Micro-benchmark over 3000 synthetic files: scanner is ~188x faster than parse+walk (0.001 vs 0.251 ms/file) with identical output on the parity spot-check; real-world files are larger, so the worker-path saving is bigger. Adds csharp-namespace-extraction.test.ts (12 cases) covering all declaration forms plus negative cases (using var, plain using, comments). Co-authored-by: Cursor * chore(autofix): apply prettier + eslint fixes via /autofix command * fix(csharp): cover global-namespace workspaceFqnBindings path + doc + using-static perf Addresses the production-readiness review of the namespace-siblings OOM fix. - Add a unit test proving global-(default-)namespace C# types route to indexes.workspaceFqnBindings (one entry per simple name) with ZERO bindingAugmentations — pinning the O(D) invariant behind the #1871 Unity-scale OOM fix and guarding against a revert to per-scope O(scopes x defs) augmentation. (The csharp-hooks mock now supplies workspaceFqnBindings, which the global fast path reads directly.) - Correct the workspaceFqnBindings doc comment: it is shared by PHP (backslash-FQN keys) and C# (global-namespace simple-name keys); the two key formats are disjoint. - Pre-index parsedFiles by path before the `using static` member-injection loop, replacing an O(files) find-per-import with an O(1) Map lookup. Verified: tsc --noEmit clean; csharp-hooks + csharp-namespace-extraction suites pass (38 tests); prettier clean; eslint 0 errors. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(csharp): apply PR-review polish to namespace-siblings (tests, types, docs) Addresses the multi-agent code review of this PR — the concrete, defensible findings. Two items intentionally deferred (below). - namespace-siblings.ts: couple the augmentation bucket + its de-dup set into one nullable lifecycle, removing the seen!/bucketArr! non-null assertions (identical runtime, still lazy). - validate-bindings-immutability.ts: extend the dev-mode immutability validator to the third channel (workspaceFqnBindings) + a test; complete the validator test mock with workspaceFqnBindings. - walkers.ts: document that namesAtScope deliberately excludes the scope-independent workspaceFqnBindings channel (enumerating workspace names at every scope would flood per-scope callers; lookupBindingsAt still consults it when resolving a specific name). - scope-resolution-indexes.ts: reframe the workspaceFqnBindings doc to describe the key-format contract language-neutrally (examples, not language branching). - csharp-hooks.test.ts: assert workspace entries carry origin:'namespace'; add a partial-class test (same simple name, distinct nodeIds across global files → both kept); rename the stale "parses" cache-miss test to "scans". - csharp-pipeline-benchmark.test.ts: clearTimeout the Promise.race budget timer (dangling handle when the pipeline won the race). - csharp.test.ts: correct the #1066 comment — extractFileStructure no longer re-parses on cache miss (line scanner); only emitCsharpScopeCaptures re-parses. Deferred (surfaced, not applied): (1) worker-path scanner mis-reads namespace/using-static inside block comments and verbatim/raw strings — an explicitly documented trade-off mirroring the PHP scanner; hardening it to track comment/string state is a separate decision. (2) workspaceFqnBindings is read via an `as Map` cast; a type-safe mutable handle from finalize-orchestrator is a cross-module contract change. Verified: tsc --noEmit clean; 49 unit tests pass (incl. 3 new); prettier clean; eslint 0 errors. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(csharp): harden worker-path scanner + localize workspace-map cast Addresses the two deferred PR-review findings plus the remaining test gap. #1 — Worker-path scanner false positives: the line scanner now tracks block- comment and string state across lines (advanceCsScanState), so a `namespace` / `using static` keyword at the start of a line inside a block comment, verbatim string (@"..."), or raw string literal ("""...""") is no longer mistaken for a declaration on the worker cache-miss path. It matches only at code-state line starts. 5 new scanner tests cover the block-comment / raw / verbatim cases. #4 — workspaceFqnBindings type safety: the ReadonlyMap->Map cast is localized to one documented line, and global-namespace writes go through a new getWorkspaceBucket helper (mirroring getAugmentationBucket) rather than an inline `.set()` at the mutation site. #2 — lookupBindingsAt workspace-channel coverage: walkers-augmentations.test.ts now exercises the third (workspace) channel: workspace-only, append-after- finalized/augmented, and dedup-loses-to-finalized/augmented precedence. #5 — OOM CI guard: the deterministic O(D) invariant (zero per-scope augmentation for global types) is already asserted by the always-on csharp-hooks unit tests added earlier; the scale/time benchmark stays appropriately opt-in (skipIf). Verified: tsc --noEmit clean; 69 unit tests (4 suites) + 210 C# integration resolver tests pass; prettier clean; eslint 0 errors. Co-Authored-By: Claude Opus 4.8 (1M context) * perf(csharp): replace remaining O(A) .some dedup scans with seeded Sets The using-static member-injection loop and the cross-namespace import loop both de-duped via `bucketArr.some((b) => b.def.nodeId === ...)` — O(A) per item. Both now use a per-file `Map>`, seeded lazily from the augmentation bucket (capturing entries from earlier passes), matching the global and named-namespace paths. Same dedup semantics, O(1) amortized. Verified: tsc --noEmit clean; csharp-hooks unit (27) + C# integration resolver (210) tests pass; prettier + eslint clean. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Cursor Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) --- .../languages/csharp/namespace-siblings.ts | 318 ++++++++++++++++-- .../model/scope-resolution-indexes.ts | 14 +- .../validate-bindings-immutability.ts | 20 +- .../scope-resolution/scope/walkers.ts | 8 + .../csharp-pipeline-benchmark.test.ts | 253 ++++++++++++++ .../test/integration/resolvers/csharp.test.ts | 9 +- .../unit/csharp-namespace-extraction.test.ts | 99 ++++++ .../csharp/csharp-hooks.test.ts | 124 ++++++- .../validate-bindings-immutability.test.ts | 20 ++ .../walkers-augmentations.test.ts | 37 +- 10 files changed, 862 insertions(+), 40 deletions(-) create mode 100644 gitnexus/test/integration/csharp-pipeline-benchmark.test.ts create mode 100644 gitnexus/test/unit/csharp-namespace-extraction.test.ts diff --git a/gitnexus/src/core/ingestion/languages/csharp/namespace-siblings.ts b/gitnexus/src/core/ingestion/languages/csharp/namespace-siblings.ts index b6de589e2..f4d29d7e7 100644 --- a/gitnexus/src/core/ingestion/languages/csharp/namespace-siblings.ts +++ b/gitnexus/src/core/ingestion/languages/csharp/namespace-siblings.ts @@ -28,17 +28,19 @@ * aliased `using static X = Y.Z;`, attributed namespace declarations, * and preprocessor-guarded declarations correctly because the * tree-sitter grammar parses them as real nodes (not textual - * coincidences). + * coincidences). When the orchestrator's `treeCache` has no Tree for a + * file — the worker path, where native Trees can't cross MessageChannels + * — `extractFileStructure` falls back to a line scanner rather than + * re-parsing every file from scratch (that re-parse dominated worker-mode + * scope-resolution time). See `extractCsharpStructureViaScanner`. */ import type { SyntaxNode } from 'tree-sitter'; import type { BindingRef, ParsedFile, Scope, ScopeId, SymbolDefinition } from 'gitnexus-shared'; import type { ScopeResolutionIndexes } from '../../model/scope-resolution-indexes.js'; import { getCsharpParser } from './query.js'; -import { getTreeSitterBufferSize } from '../../constants.js'; -import { parseSourceSafe } from '../../../tree-sitter/safe-parse.js'; -interface CsharpFileStructure { +export interface CsharpFileStructure { /** Declared namespace names in file source order. Empty array means * the file has no `namespace X;` / `namespace X { }` declaration * and sits in the default (global) namespace. */ @@ -48,18 +50,174 @@ interface CsharpFileStructure { readonly usingStaticPaths: readonly string[]; } -/** Build a structural view of a C# file by walking the tree-sitter - * AST. Prefers `cachedTree` (handed in via `treeCache`) so we don't - * re-parse files the orchestrator already parsed for `extractParsedFile`; - * falls back to a fresh parse on cache miss. Parser singleton is - * shared across calls. */ +// Line-anchored matchers for the worker-path fallback (see +// `extractCsharpStructureViaScanner`). Anchored at line start (after +// indentation); the scanner additionally tracks block-comment / string +// state across lines so a keyword at the start of a line inside one of +// those regions is skipped. +const CS_NAMESPACE_RE = /^[ \t]*namespace[ \t]+([A-Za-z_@][A-Za-z0-9_.]*)/; +// `global using static`, plain `using static`, and the aliased +// `using static Alias = NS.Type;` form (the AST keeps the RHS path, so +// the optional `Alias =` is skipped and only the dotted path captured). +const CS_USING_STATIC_RE = + /^[ \t]*(?:global[ \t]+)?using[ \t]+static[ \t]+(?:[A-Za-z_@][A-Za-z0-9_]*[ \t]*=[ \t]*)?([A-Za-z_@][A-Za-z0-9_.]*)/; + +/** Multi-line lexical state carried line-to-line by the scanner. */ +type CsScanState = 'code' | 'block' | 'verbatim' | 'raw'; + +/** Advance the scanner's lexical state across one line, consuming block + * comments (slash-star), line comments (`//`), single-line regular / + * interpolated strings, verbatim strings (`@"…"`), and raw string literals + * (`"""…"""`, fence length tracked in `rawFence`). Returns the state and + * raw-fence length in effect at the START of the next line. Single-line + * strings and `//` comments resolve back to `code` before end of line; only + * block comments and multi-line strings carry state forward. */ +function advanceCsScanState( + line: string, + state: CsScanState, + rawFence: number, +): [CsScanState, number] { + const n = line.length; + let i = 0; + while (i < n) { + if (state === 'block') { + const end = line.indexOf('*/', i); + if (end === -1) return ['block', rawFence]; + i = end + 2; + state = 'code'; + } else if (state === 'verbatim') { + // Ends at a `"` that is not doubled (`""` is an escaped quote). + while (i < n) { + if (line[i] === '"') { + if (line[i + 1] === '"') { + i += 2; + continue; + } + break; + } + i++; + } + if (i >= n) return ['verbatim', rawFence]; + i += 1; + state = 'code'; + } else if (state === 'raw') { + // Ends at a run of `"` at least `rawFence` long. + let closed = false; + while (i < n) { + if (line[i] === '"') { + let k = i; + while (k < n && line[k] === '"') k++; + if (k - i >= rawFence) { + i = k; + state = 'code'; + rawFence = 0; + closed = true; + break; + } + i = k; + } else { + i++; + } + } + if (!closed) return ['raw', rawFence]; + } else { + const c = line[i]; + const next = line[i + 1]; + if (c === '/' && next === '/') return ['code', rawFence]; // line comment to EOL + if (c === '/' && next === '*') { + state = 'block'; + i += 2; + } else if (c === '@' && next === '"') { + state = 'verbatim'; + i += 2; + } else if ((c === '$' && next === '@') || (c === '@' && next === '$')) { + if (line[i + 2] === '"') { + state = 'verbatim'; // interpolated verbatim ($@"…" / @$"…") + i += 3; + } else { + i++; + } + } else if (c === '"') { + let k = i; + while (k < n && line[k] === '"') k++; + const run = k - i; + if (run >= 3) { + state = 'raw'; + rawFence = run; + i = k; + } else if (run === 2) { + i = k; // "" — empty string + } else { + // single-line regular / interpolated string; consume to closer + let j = i + 1; + while (j < n) { + if (line[j] === '\\') { + j += 2; + continue; + } + if (line[j] === '"') break; + j++; + } + i = j >= n ? n : j + 1; + } + } else { + i++; + } + } + } + return [state, rawFence]; +} + +/** Line-scanner used when no cached tree is available (worker-parsed files + * can't transfer native tree-sitter Trees across MessageChannels, so + * `treeCache` is empty for them). Re-parsing every C# file here with + * tree-sitter was the dominant scope-resolution cost on large worker-mode + * runs — for a multi-thousand-file solution this loop alone re-parsed the + * whole repo a second time. The scanner extracts the same `namespaces` / + * `usingStaticPaths` the AST walk produces for line-anchored declarations, + * while tracking block-comment and string state across lines (via + * `advanceCsScanState`) so a `namespace` / `using static` keyword at the + * start of a line inside a block comment, verbatim string, or raw string + * literal is NOT mistaken for a declaration. The remaining trade-off vs the + * AST is a declaration whose keyword is not at the start of a code line + * (split across lines, or sharing a line with a comment/string closer). + * Mirrors PHP's `extractNamespaceViaScanner` (issue #1741). */ +export function extractCsharpStructureViaScanner(content: string): CsharpFileStructure { + const namespaces: string[] = []; + const usingStaticPaths: string[] = []; + let state: CsScanState = 'code'; + let rawFence = 0; + for (const line of content.split('\n')) { + // Only match when the line START is real code — keywords reached while + // inside a block comment / multi-line string are skipped. + if (state === 'code') { + const ns = CS_NAMESPACE_RE.exec(line); + if (ns !== null) { + namespaces.push(ns[1]!); + } else { + const us = CS_USING_STATIC_RE.exec(line); + if (us !== null) usingStaticPaths.push(us[1]!); + } + } + [state, rawFence] = advanceCsScanState(line, state, rawFence); + } + return { namespaces, usingStaticPaths }; +} + +/** Build a structural view of a C# file. Prefers `cachedTree` (handed in + * via `treeCache`) and walks the tree-sitter AST — the authoritative + * path that sees `global using static`, aliased `using static X = Y.Z;`, + * attributed namespace declarations, and preprocessor-guarded nodes + * correctly. On cache miss (worker-parsed files, whose native Trees + * can't cross MessageChannels) it falls back to the line scanner instead + * of a fresh tree-sitter parse — the parse here dominated worker-mode + * scope-resolution time. Parser singleton is shared across calls. */ function extractFileStructure(content: string, cachedTree: unknown): CsharpFileStructure { + if (!cachedTree) { + return extractCsharpStructureViaScanner(content); + } type CsharpTree = ReturnType['parse']>; - const tree = - (cachedTree as CsharpTree | undefined) ?? - parseSourceSafe(getCsharpParser(), content, undefined, { - bufferSize: getTreeSitterBufferSize(content), - }); + const tree = cachedTree as CsharpTree; const namespaces: string[] = []; const usingStaticPaths: string[] = []; @@ -277,11 +435,17 @@ export function populateCsharpNamespaceSiblings( // scope, so `Record(...)` (without `Logger.` qualifier) resolves // to `Logger.Record`. AST walk above captured these (including // `global using static` and aliased forms). + // Pre-index files by path once: the member-injection lookup below would + // otherwise be an O(files) scan per `using static` import. + const fileByPath = new Map(parsedFiles.map((p) => [p.filePath, p])); for (const parsed of parsedFiles) { const struct = structureByFile.get(parsed.filePath); if (struct === undefined) continue; const moduleScope = parsed.scopes.find((s) => s.kind === 'Module'); if (moduleScope === undefined) continue; + // Per-file de-dup sets keyed by simple name, seeded lazily from the + // augmentation bucket — replaces the per-member O(A) `.some` scan below. + const seenByName = new Map>(); for (const fullPath of struct.usingStaticPaths) { const lastDot = fullPath.lastIndexOf('.'); @@ -302,7 +466,7 @@ export function populateCsharpNamespaceSiblings( // Inject the class's member methods into the importer's module // scope. `memberByOwner` wasn't built yet here, so we walk the // file's localDefs to find members with `ownerId === targetDef.nodeId`. - const targetFile = parsedFiles.find((p) => p.filePath === targetDef.filePath); + const targetFile = fileByPath.get(targetDef.filePath); if (targetFile === undefined) continue; for (const memberDef of targetFile.localDefs) { if ((memberDef as { ownerId?: string }).ownerId !== targetDef.nodeId) continue; @@ -316,7 +480,14 @@ export function populateCsharpNamespaceSiblings( // `lookupBindingsAt`, which fans out across `bindings` + // `bindingAugmentations`. const bucketArr = getAugmentationBucket(augmentations, moduleScope.id, simpleName); - if (bucketArr.some((b) => b.def.nodeId === memberDef.nodeId)) continue; + let seen = seenByName.get(simpleName); + if (seen === undefined) { + seen = new Set(); + for (const b of bucketArr) seen.add(b.def.nodeId); + seenByName.set(simpleName, seen); + } + if (seen.has(memberDef.nodeId)) continue; + seen.add(memberDef.nodeId); bucketArr.push({ def: memberDef, origin: 'import' }); } } @@ -332,6 +503,9 @@ export function populateCsharpNamespaceSiblings( for (const parsed of parsedFiles) { const moduleScope = parsed.scopes.find((s) => s.kind === 'Module'); if (moduleScope === undefined) continue; + // Per-file de-dup sets keyed by simple name, seeded lazily from the + // augmentation bucket — replaces the per-def O(A) `.some` scan below. + const seenByName = new Map>(); for (const imp of parsed.parsedImports) { if (imp.kind !== 'namespace') continue; const targetNs = imp.targetRaw; @@ -344,41 +518,113 @@ export function populateCsharpNamespaceSiblings( const simpleName = q.includes('.') ? q.slice(q.lastIndexOf('.') + 1) : q; if (simpleName === '') continue; const bucketArr = getAugmentationBucket(augmentations, moduleScope.id, simpleName); - if (bucketArr.some((b) => b.def.nodeId === def.nodeId)) continue; + let seen = seenByName.get(simpleName); + if (seen === undefined) { + seen = new Set(); + for (const b of bucketArr) seen.add(b.def.nodeId); + seenByName.set(simpleName, seen); + } + if (seen.has(def.nodeId)) continue; + seen.add(def.nodeId); bucketArr.push({ def, origin: 'namespace' }); } } } - for (const [, bucket] of buckets) { - // De-dup by (nodeId, filePath) across multiple declarations (e.g. - // partial classes declaring the same name in two files — we take - // both and leave de-dup to downstream consumers of bindings). + // Workspace-level binding channel for global-namespace types (see the + // global fast-path below). `lookupBindingsAt` consults this as a third + // source after finalized + per-scope augmented bindings. Its inner arrays + // are mutable by contract (append-only, like `bindingAugmentations` — see + // the ScopeResolutionIndexes doc + validateBindingsImmutability), so the + // ReadonlyMap→Map cast is localized to this one line and all writes go + // through `getWorkspaceBucket`. + const workspace = indexes.workspaceFqnBindings as Map; + + for (const [nsName, bucket] of buckets) { + // Group sibling defs by simple name. Append in place — the previous + // `[...prev, def]` copy made this O(D²) per bucket, which on the + // global (`''`) namespace bucket of a large Unity solution (tens of + // thousands of type defs) was a primary slowness/OOM source. We keep + // every declaration (e.g. partial classes across files) and leave + // de-dup to downstream consumers. const defsByName = new Map(); for (const def of bucket.classDefs) { // Simple name = last segment of qualifiedName (e.g. `App.User` → `User`). const q = def.qualifiedName ?? ''; const key = q.includes('.') ? q.slice(q.lastIndexOf('.') + 1) : q; if (key === '') continue; - const arr = [...(defsByName.get(key) ?? [])]; + let arr = defsByName.get(key); + if (arr === undefined) { + arr = []; + defsByName.set(key, arr); + } arr.push(def); - defsByName.set(key, arr); + } + + // Global-namespace fast path (Unity OOM guard). Types declared in the + // default (global) namespace are visible from EVERY file in C# — the + // global namespace is always implicitly in scope — so one workspace- + // level entry per simple name is both semantically correct and O(D) + // instead of the O(S·D) per-scope augmentation that materialized + // billions of BindingRefs on large Unity solutions (tens of thousands + // of global types × tens of thousands of scopes). `walkScopeChain` + // checks local `scope.bindings` first, so local declarations still + // shadow these workspace entries; a file resolving its own global type + // hits the local binding before this map. Dedup by `def.nodeId` keeps + // partial-class / duplicate declarations from double-emitting. + if (nsName === '') { + for (const [name, defs] of defsByName) { + const bucket = getWorkspaceBucket(workspace, name); + const seen = new Set(); + for (const b of bucket) seen.add(b.def.nodeId); + for (const def of defs) { + if (seen.has(def.nodeId)) continue; // dedup by nodeId (keeps partials, drops re-emits) + seen.add(def.nodeId); + bucket.push({ def, origin: 'namespace' }); + } + } + continue; + } + + // Pre-index the first scope per file once (O(S)) instead of an + // O(S) `.find` re-run for every (scope, name) pair, which made the + // injection loop O(S²·D) and was the dominant cost on large buckets. + // Multiple scopes share a filePath (Module + Namespace); the local + // shadow check only needs that file's lexical `Scope.bindings`, which + // is identical regardless of which of those scopes we read. + const firstScopeByFile = new Map(); + for (const s of bucket.scopes) { + if (!firstScopeByFile.has(s.filePath)) firstScopeByFile.set(s.filePath, s.scope); } for (const { scopeId, filePath } of bucket.scopes) { + const localScope = firstScopeByFile.get(filePath); for (const [name, defs] of defsByName) { // Skip names already present locally — `origin: 'local'` in // scope.bindings would naturally shadow the cross-file // namespace entry, but we also keep this index lean. - const local = bucket.scopes.find((s) => s.filePath === filePath)?.scope.bindings.get(name); + const local = localScope?.bindings.get(name); if (local !== undefined && local.some((b) => b.origin === 'local')) continue; - let bucketArr: BindingRef[] | null = null; + // Bind the augmentation bucket and its seeded de-dup set together + // under one nullable lifecycle, so neither needs a non-null + // assertion (they are always set or unset as a pair). Stays lazy: + // nothing is allocated for a name with no cross-file defs. + let inject: { bucket: BindingRef[]; seen: Set } | null = null; for (const def of defs) { if (def.filePath === filePath) continue; // don't self-reference - if (bucketArr === null) bucketArr = getAugmentationBucket(augmentations, scopeId, name); - if (bucketArr.some((b) => b.def.nodeId === def.nodeId)) continue; - bucketArr.push({ def, origin: 'namespace' }); + if (inject === null) { + const bucket = getAugmentationBucket(augmentations, scopeId, name); + // Seed the de-dup set from any entries an earlier pass + // (using-static / cross-namespace imports) already added, + // replacing the per-def O(A) `.some` scan. + const seen = new Set(); + for (const b of bucket) seen.add(b.def.nodeId); + inject = { bucket, seen }; + } + if (inject.seen.has(def.nodeId)) continue; + inject.seen.add(def.nodeId); + inject.bucket.push({ def, origin: 'namespace' }); } } } @@ -409,6 +655,22 @@ function getAugmentationBucket( return bucketArr; } +/** Get-or-create a mutable inner bucket inside the `workspaceFqnBindings` + * channel (the scope-independent third channel; see + * `ScopeResolutionIndexes.workspaceFqnBindings`). Like + * `getAugmentationBucket`, the inner arrays are mutable by contract — + * callers `push` directly. Keeping the get-or-create here means the one + * ReadonlyMap→Map cast at the call site is the only place the mutable + * view is taken. */ +function getWorkspaceBucket(workspace: Map, name: string): BindingRef[] { + let bucketArr = workspace.get(name); + if (bucketArr === undefined) { + bucketArr = []; + workspace.set(name, bucketArr); + } + return bucketArr; +} + function isTypeDef(def: SymbolDefinition): boolean { return ( def.type === 'Class' || diff --git a/gitnexus/src/core/ingestion/model/scope-resolution-indexes.ts b/gitnexus/src/core/ingestion/model/scope-resolution-indexes.ts index 561595ac5..71e78f567 100644 --- a/gitnexus/src/core/ingestion/model/scope-resolution-indexes.ts +++ b/gitnexus/src/core/ingestion/model/scope-resolution-indexes.ts @@ -77,11 +77,15 @@ export interface ScopeResolutionIndexes { * are returned first and win duplicate `def.nodeId` metadata, with * unique augmentations appended after. See I8. */ readonly bindingAugmentations: ReadonlyMap>; - /** Workspace-level FQN binding lookup. Populated by PHP namespace- - * siblings Step 3b as a shared map instead of per-scope duplication. - * Consulted by `lookupBindingsAt` as a third source after finalized - * and per-scope augmented bindings. Keys are backslash-separated FQNs - * (e.g. `App\Models\User`). */ + /** Workspace-level binding lookup, shared instead of per-scope + * duplication. Consulted by `lookupBindingsAt` as a third source after + * finalized and per-scope augmented bindings. Language-specific + * namespace-sibling hooks populate it with disjoint key formats that + * never collide — e.g. backslash-separated FQNs (`App\Models\User`) for + * backslash-namespace languages, and bare simple names (`User`) for + * global-/default-namespace types that are visible from every file. The + * shared map gives those workspace-wide names one entry each instead of + * O(scopes × defs) per-scope augmentation. */ readonly workspaceFqnBindings: ReadonlyMap; /** Pre-resolution usage facts; consumed by the resolution phase. */ readonly referenceSites: readonly ReferenceSite[]; diff --git a/gitnexus/src/core/ingestion/scope-resolution/pipeline/validate-bindings-immutability.ts b/gitnexus/src/core/ingestion/scope-resolution/pipeline/validate-bindings-immutability.ts index 60e5fe179..5ccba91ac 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/pipeline/validate-bindings-immutability.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/pipeline/validate-bindings-immutability.ts @@ -1,6 +1,6 @@ /** - * Dev-mode runtime validator for the two-channel binding lifecycle - * (Contract Invariant I8 in `contract/scope-resolver.ts`). + * Dev-mode runtime validator for the post-finalize binding-channel + * lifecycle (Contract Invariant I8 in `contract/scope-resolver.ts`). * * The two channels: * - `indexes.bindings` — finalize-output channel. After @@ -74,5 +74,21 @@ export function validateBindingsImmutability( } } + // Third channel: `workspaceFqnBindings` (scope-independent, shared map + // populated by language namespace-sibling hooks — PHP FQN keys, C# + // global-namespace simple names). Like bindingAugmentations its inner + // arrays are mutable by contract (hooks `push()` directly), so freezing + // one is the same defect as freezing an augmentation bucket. + for (const [name, bucket] of indexes.workspaceFqnBindings) { + if (Object.isFrozen(bucket)) { + onWarn( + `binding-immutability: indexes.workspaceFqnBindings[${name}] is FROZEN — ` + + `the workspace channel is mutable by contract; freezing it defeats the ` + + `append-only purpose. See ScopeResolver Invariant I8.`, + ); + violations++; + } + } + return violations; } diff --git a/gitnexus/src/core/ingestion/scope-resolution/scope/walkers.ts b/gitnexus/src/core/ingestion/scope-resolution/scope/walkers.ts index df6d259b5..6bf967fff 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/scope/walkers.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/scope/walkers.ts @@ -99,6 +99,14 @@ const EMPTY_NAMES: Iterable = Object.freeze([]) as readonly string[]; * Fast paths (zero allocation) when at most one channel is populated: * returns the underlying `Map.keys()` iterator directly. Only when both * channels carry names do we materialize a `Set` for deduplication. + * + * Scope: enumerates only the per-scope `bindings` and `bindingAugmentations` + * channels. It deliberately EXCLUDES the scope-independent + * `workspaceFqnBindings` channel (PHP FQN keys, C# global-namespace simple + * names). `lookupBindingsAt` consults that third channel when resolving a + * specific name, but name *enumeration* here does not — those names apply at + * every scope and would flood per-scope callers. Callers that need + * workspace-level names must read `workspaceFqnBindings` directly. */ export function namesAtScope(scopeId: ScopeId, scopes: ScopeResolutionIndexes): Iterable { const finalized = scopes.bindings.get(scopeId); diff --git a/gitnexus/test/integration/csharp-pipeline-benchmark.test.ts b/gitnexus/test/integration/csharp-pipeline-benchmark.test.ts new file mode 100644 index 000000000..998c3c3e9 --- /dev/null +++ b/gitnexus/test/integration/csharp-pipeline-benchmark.test.ts @@ -0,0 +1,253 @@ +/** + * C# ingestion pipeline benchmark. + * + * Generates synthetic C# codebases at increasing scales and measures + * wall-clock time and peak heap through the full pipeline — parsing, + * scope extraction, C# namespace-siblings (same-namespace cross-file + * visibility, using-static, cross-namespace imports), and call + * resolution. + * + * Mirrors test/integration/php-pipeline-benchmark.test.ts. Two shapes: + * 1. "spread" — files distributed across many namespaces (the common + * case; each namespace bucket stays small). + * 2. "concentrated" — every file in the SAME (or global/no) namespace, + * so a single namespace bucket holds all type defs. This is the + * shape that drove the Unity-solution OOM: `populateCsharpNamespaceSiblings` + * materialises O(scopes × defs) BindingRefs into that one bucket. + * The concentrated test is the regression guard for that path. + * + * Run: GITNEXUS_BENCH=1 npx vitest run test/integration/csharp-pipeline-benchmark.test.ts + */ +import { describe, it, expect } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { runPipelineFromRepo } from '../../src/core/ingestion/pipeline.js'; + +const BENCH_ENABLED = process.env.GITNEXUS_BENCH === '1'; + +interface BenchResult { + fileCount: number; + classCount: number; + namespaceCount: number; + elapsedMs: number; + peakHeapMB: number; + nodeCount: number; + edgeCount: number; +} + +type FixtureShape = 'spread' | 'concentrated'; + +function generateCsharpFixture( + fileCount: number, + namespacesPerLevel: number, + shape: FixtureShape, +): { dir: string; classCount: number; namespaceCount: number } { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), `csharp-bench-${shape}-${fileCount}-`)); + + // "spread": square grid of namespaces. "concentrated": a single + // global (no-namespace) bucket so every type lands in the `''` bucket + // — the OOM-prone path. + const namespaces: string[] = []; + if (shape === 'spread') { + for (let i = 0; i < namespacesPerLevel; i++) { + for (let j = 0; j < namespacesPerLevel; j++) { + namespaces.push(`App.Module${i}.Sub${j}`); + } + } + } else { + namespaces.push(''); // global / no namespace declaration + } + + const classCount = fileCount; + const namespaceCount = namespaces.length; + + for (let f = 0; f < fileCount; f++) { + const ns = namespaces[f % namespaces.length]!; + const className = `Class${f}`; + // Concentrated files share a flat directory; spread files mirror the + // namespace as a directory tree (matches typical C# project layout). + const targetDir = ns === '' ? dir : path.join(dir, ns.replace(/\./g, '/')); + fs.mkdirSync(targetDir, { recursive: true }); + + const siblingIdx = (f + 1) % fileCount; + const siblingClass = `Class${siblingIdx}`; + + const crossNsIdx = (f + Math.floor(fileCount / 3)) % fileCount; + const crossNs = namespaces[crossNsIdx % namespaces.length]!; + const crossClass = `Class${crossNsIdx}`; + const usesCross = ns !== '' && ns !== crossNs; + + const body = [ + ns !== '' ? `namespace ${ns};` : '', + usesCross ? `using ${crossNs};` : '', + '', + `public class ${className}`, + '{', + ' private int id;', + ' private string name;', + '', + ' public int GetId()', + ' {', + ' return this.id;', + ' }', + '', + ` public ${siblingClass} Process()`, + ' {', + ` var sibling = new ${siblingClass}();`, + ' return sibling;', + ' }', + usesCross + ? [ + '', + ` public ${crossClass} CrossCall()`, + ' {', + ` var cross = new ${crossClass}();`, + ' cross.GetId();', + ' return cross;', + ' }', + ].join('\n') + : '', + '}', + '', + ] + .filter(Boolean) + .join('\n'); + + fs.writeFileSync(path.join(targetDir, `${className}.cs`), body); + } + + // Minimal SDK-style csproj so the C# project-loading phase engages + // (matches the real-world Unity/.NET solution path). + const csproj = [ + '', + ' ', + ' net8.0', + ' enable', + ' ', + '', + '', + ].join('\n'); + fs.writeFileSync(path.join(dir, 'Bench.csproj'), csproj); + + return { dir, classCount, namespaceCount }; +} + +async function runBenchmark( + fileCount: number, + nsLevels: number, + shape: FixtureShape, + budgetMs: number, +): Promise { + const { dir, classCount, namespaceCount } = generateCsharpFixture(fileCount, nsLevels, shape); + + let peakHeapMB = 0; + const heapSampler = setInterval(() => { + const heap = process.memoryUsage().heapUsed / 1024 / 1024; + if (heap > peakHeapMB) peakHeapMB = heap; + }, 50); + + let budgetTimer: ReturnType | undefined; + try { + const start = Date.now(); + const result = await Promise.race([ + runPipelineFromRepo(dir, () => {}, { skipGraphPhases: true }), + new Promise((_, reject) => { + budgetTimer = setTimeout( + () => + reject(new Error(`Pipeline exceeded ${budgetMs}ms at ${fileCount} files (${shape})`)), + budgetMs, + ); + }), + ]); + const elapsedMs = Date.now() - start; + + return { + fileCount, + classCount, + namespaceCount, + elapsedMs, + peakHeapMB: Math.round(peakHeapMB), + nodeCount: result.graph.nodeCount, + edgeCount: result.graph.relationshipCount, + }; + } finally { + clearInterval(heapSampler); + clearTimeout(budgetTimer); + fs.rmSync(dir, { recursive: true, force: true }); + } +} + +function printResults(label: string, results: BenchResult[]) { + console.log(`\n${label}`); + console.log('┌──────────┬─────────┬──────────┬───────────┬──────────┬───────┬───────┐'); + console.log('│ Files │ Classes │ NS Count │ Time (ms) │ Heap MB │ Nodes │ Edges │'); + console.log('├──────────┼─────────┼──────────┼───────────┼──────────┼───────┼───────┤'); + for (const r of results) { + console.log( + `│ ${String(r.fileCount).padStart(8)} │ ${String(r.classCount).padStart(7)} │ ${String(r.namespaceCount).padStart(8)} │ ${String(r.elapsedMs).padStart(9)} │ ${String(r.peakHeapMB).padStart(8)} │ ${String(r.nodeCount).padStart(5)} │ ${String(r.edgeCount).padStart(5)} │`, + ); + } + console.log('└──────────┴─────────┴──────────┴───────────┴──────────┴───────┴───────┘'); + + if (results.length >= 2) { + console.log('\nScaling ratios (time_ratio / file_ratio):'); + for (let i = 1; i < results.length; i++) { + const fileRatio = results[i].fileCount / results[i - 1].fileCount; + const timeRatio = results[i].elapsedMs / results[i - 1].elapsedMs; + const scaling = timeRatio / fileRatio; + console.log( + ` ${results[i - 1].fileCount} → ${results[i].fileCount}: ${scaling.toFixed(2)}x (${scaling < 1.5 ? 'linear' : scaling < 3 ? 'superlinear' : 'WARNING: quadratic'})`, + ); + } + } +} + +describe.skipIf(!BENCH_ENABLED)('C# pipeline benchmark', () => { + it('scales with file count — namespaces spread across the solution', async () => { + const scales = [100, 250, 500]; + const results: BenchResult[] = []; + + for (const fileCount of scales) { + const nsLevels = Math.max(2, Math.ceil(Math.sqrt(fileCount / 4))); + const result = await runBenchmark(fileCount, nsLevels, 'spread', 180_000); + results.push(result); + console.log( + ` ${fileCount} files: ${result.elapsedMs}ms, ${result.peakHeapMB}MB heap, ${result.nodeCount} nodes, ${result.edgeCount} edges`, + ); + } + + printResults('C# Pipeline — Namespaces Spread', results); + + for (let i = 1; i < results.length; i++) { + const fileRatio = results[i].fileCount / results[i - 1].fileCount; + const timeRatio = results[i].elapsedMs / results[i - 1].elapsedMs; + expect(timeRatio / fileRatio).toBeLessThan(3); + } + }, 600_000); + + it('scales with file count — all types in one (global) namespace bucket', async () => { + // Regression guard for the Unity-solution OOM: a single namespace + // bucket holds every type def, so naive per-scope binding + // materialisation is O(files²). Time must stay sub-quadratic and the + // run must not OOM. + const scales = [100, 250, 500]; + const results: BenchResult[] = []; + + for (const fileCount of scales) { + const result = await runBenchmark(fileCount, 1, 'concentrated', 180_000); + results.push(result); + console.log( + ` ${fileCount} files: ${result.elapsedMs}ms, ${result.peakHeapMB}MB heap, ${result.nodeCount} nodes, ${result.edgeCount} edges`, + ); + } + + printResults('C# Pipeline — Concentrated Global Namespace', results); + + for (let i = 1; i < results.length; i++) { + const fileRatio = results[i].fileCount / results[i - 1].fileCount; + const timeRatio = results[i].elapsedMs / results[i - 1].elapsedMs; + expect(timeRatio / fileRatio).toBeLessThan(3); + } + }, 600_000); +}); diff --git a/gitnexus/test/integration/resolvers/csharp.test.ts b/gitnexus/test/integration/resolvers/csharp.test.ts index 073b2da16..d837c32d3 100644 --- a/gitnexus/test/integration/resolvers/csharp.test.ts +++ b/gitnexus/test/integration/resolvers/csharp.test.ts @@ -2447,9 +2447,12 @@ describe('C# class-name receiver write ACCESSES (merged Case 2 kind-aware branch // cross-namespace `using` and a colliding local class. Pins both fixes in // the resolver dataset: // -// 1. emitCsharpScopeCaptures + extractFileStructure must use the adaptive -// `getTreeSitterBufferSize` on cache miss, otherwise UserService.cs -// fails to reparse with "Invalid argument" and CreateUser is dropped. +// 1. emitCsharpScopeCaptures must use the adaptive `getTreeSitterBufferSize` +// on cache miss, otherwise UserService.cs fails to reparse with "Invalid +// argument" and CreateUser is dropped. (extractFileStructure no longer +// re-parses on cache miss — it uses the line scanner, +// extractCsharpStructureViaScanner — so this fixture's line-anchored +// namespaces are read identically by either branch.) // 2. populateCsharpNamespaceSiblings must append to bindingAugmentations // instead of mutating frozen finalize-produced BindingRef[] arrays; // otherwise the cross-namespace inject loop throws "Cannot add property diff --git a/gitnexus/test/unit/csharp-namespace-extraction.test.ts b/gitnexus/test/unit/csharp-namespace-extraction.test.ts new file mode 100644 index 000000000..7f39a061f --- /dev/null +++ b/gitnexus/test/unit/csharp-namespace-extraction.test.ts @@ -0,0 +1,99 @@ +import { describe, it, expect } from 'vitest'; +import { extractCsharpStructureViaScanner } from '../../src/core/ingestion/languages/csharp/namespace-siblings.js'; + +// Scanner fallback used on the worker path, where native tree-sitter Trees +// can't cross MessageChannels so `treeCache` is empty. It must reproduce +// the AST walk's `namespaces` / `usingStaticPaths` for the common +// line-anchored declaration forms (see namespace-siblings.ts). +describe('extractCsharpStructureViaScanner', () => { + it('extracts a file-scoped namespace declaration', () => { + const src = `namespace App.Models;\n\npublic class User {}`; + expect(extractCsharpStructureViaScanner(src).namespaces).toEqual(['App.Models']); + }); + + it('extracts a block namespace declaration', () => { + const src = `namespace App.Services\n{\n public class Svc {}\n}`; + expect(extractCsharpStructureViaScanner(src).namespaces).toEqual(['App.Services']); + }); + + it('extracts multiple namespaces in source order', () => { + const src = `namespace A.One\n{\n}\nnamespace A.Two\n{\n}`; + expect(extractCsharpStructureViaScanner(src).namespaces).toEqual(['A.One', 'A.Two']); + }); + + it('returns empty namespaces for a global (no-namespace) file', () => { + const src = `public class Global {}\n`; + expect(extractCsharpStructureViaScanner(src).namespaces).toEqual([]); + }); + + it('captures a plain `using static` path', () => { + const src = `using static System.Math;\nnamespace App;`; + const out = extractCsharpStructureViaScanner(src); + expect(out.usingStaticPaths).toEqual(['System.Math']); + expect(out.namespaces).toEqual(['App']); + }); + + it('captures a `global using static` path', () => { + const src = `global using static App.Utils.Logger;\n`; + expect(extractCsharpStructureViaScanner(src).usingStaticPaths).toEqual(['App.Utils.Logger']); + }); + + it('captures the RHS path of an aliased `using static`', () => { + const src = `using static M = App.Utils.MathUtils;\n`; + expect(extractCsharpStructureViaScanner(src).usingStaticPaths).toEqual(['App.Utils.MathUtils']); + }); + + it('does not treat a plain `using` directive as using-static', () => { + const src = `using System.Collections.Generic;\nusing App.Models;\n`; + expect(extractCsharpStructureViaScanner(src).usingStaticPaths).toEqual([]); + }); + + it('does not treat a `using var`/`using (...)` statement as using-static', () => { + const src = `using var stream = File.Open(p);\nusing (var x = Get()) { }\n`; + expect(extractCsharpStructureViaScanner(src).usingStaticPaths).toEqual([]); + }); + + it('ignores a `// namespace X` line comment', () => { + const src = `// namespace Fake.Comment;\nnamespace App.Real;`; + expect(extractCsharpStructureViaScanner(src).namespaces).toEqual(['App.Real']); + }); + + it('handles indentation before declarations', () => { + const src = `\t\tnamespace App.Indented;\n`; + expect(extractCsharpStructureViaScanner(src).namespaces).toEqual(['App.Indented']); + }); + + it('handles an empty file', () => { + const out = extractCsharpStructureViaScanner(''); + expect(out.namespaces).toEqual([]); + expect(out.usingStaticPaths).toEqual([]); + }); + + // Cross-line comment/string state: a keyword at the start of a line inside + // a block comment or multi-line string must NOT be read as a declaration + // (the worker path would otherwise mis-bucket the file vs the AST). + it('skips a `namespace` line inside a block comment', () => { + const src = `/*\nnamespace Fake.InComment;\n*/\nnamespace App.Real;`; + expect(extractCsharpStructureViaScanner(src).namespaces).toEqual(['App.Real']); + }); + + it('skips a `using static` line inside a block comment', () => { + const src = `/*\nusing static Fake.Helpers;\n*/\nusing static App.Real.Helpers;`; + expect(extractCsharpStructureViaScanner(src).usingStaticPaths).toEqual(['App.Real.Helpers']); + }); + + it('skips a `namespace` line inside a raw string literal', () => { + const src = `var sql = """\nnamespace Fake.InRaw;\n""";\nnamespace App.Real;`; + expect(extractCsharpStructureViaScanner(src).namespaces).toEqual(['App.Real']); + }); + + it('skips a `namespace` line inside a verbatim string literal', () => { + const src = `var s = @"\nnamespace Fake.InVerbatim;\n";\nnamespace App.Real;`; + expect(extractCsharpStructureViaScanner(src).namespaces).toEqual(['App.Real']); + }); + + it('still reads a real declaration after a closed same-line block comment', () => { + const src = `/* header */ class C {}\nnamespace App.Real;`; + expect(extractCsharpStructureViaScanner(src).namespaces).toEqual(['App.Real']); + }); +}); diff --git a/gitnexus/test/unit/scope-resolution/csharp/csharp-hooks.test.ts b/gitnexus/test/unit/scope-resolution/csharp/csharp-hooks.test.ts index 808bce9cf..d0bfb3784 100644 --- a/gitnexus/test/unit/scope-resolution/csharp/csharp-hooks.test.ts +++ b/gitnexus/test/unit/scope-resolution/csharp/csharp-hooks.test.ts @@ -283,7 +283,7 @@ describe('populateCsharpNamespaceSiblings', () => { expect(Object.isFrozen(augmented)).toBe(false); }); - it('parses UTF-8-heavy cache-miss files before namespace sibling injection', () => { + it('scans (no re-parse) UTF-8-heavy cache-miss files before namespace sibling injection', () => { const sibling = classDef('def:b.B', 'b.cs', 'Demo.B'); const moduleA = scope('scope:a:module', 'Module', 'a.cs'); const moduleB = scope('scope:b:module', 'Module', 'b.cs'); @@ -322,6 +322,128 @@ describe('populateCsharpNamespaceSiblings', () => { expect(bindingAugmentations.get(moduleA.id)?.get('B')?.[0]?.def.nodeId).toBe('def:b.B'); }); + + it('routes global-namespace types to workspaceFqnBindings, not per-scope augmentations (#1871 OOM guard)', () => { + // Types declared with NO `namespace` (the global/default namespace) are + // visible from every C# file, so the hook writes ONE workspace-level entry + // per simple name instead of O(scopes x defs) per-scope augmentations — + // the fix for the #1871 Unity-scale OOM. This pins both halves of that + // contract: global types are reachable via `workspaceFqnBindings`, and the + // per-scope augmentation channel stays empty for them. + // + // Note: the mock MUST supply `workspaceFqnBindings` — the global fast path + // reads `indexes.workspaceFqnBindings` directly, so omitting it (as the + // other tests in this suite do) would throw. + const defA = classDef('def:a.A', 'a.cs', 'A'); // simple name => global namespace + const defB = classDef('def:b.B', 'b.cs', 'B'); + const moduleA = scope('scope:a:module', 'Module', 'a.cs'); + const classA = scope('scope:a:class', 'Class', 'a.cs', moduleA.id, [defA]); + const moduleB = scope('scope:b:module', 'Module', 'b.cs'); + const classB = scope('scope:b:class', 'Class', 'b.cs', moduleB.id, [defB]); + const parsedFiles: ParsedFile[] = [ + { + filePath: 'a.cs', + moduleScope: moduleA.id, + scopes: Object.freeze([moduleA, classA]), + parsedImports: Object.freeze([]), + localDefs: Object.freeze([defA]), + referenceSites: Object.freeze([]), + } as ParsedFile, + { + filePath: 'b.cs', + moduleScope: moduleB.id, + scopes: Object.freeze([moduleB, classB]), + parsedImports: Object.freeze([]), + localDefs: Object.freeze([defB]), + referenceSites: Object.freeze([]), + } as ParsedFile, + ]; + const bindingAugmentations = new Map>(); + const workspaceFqnBindings = new Map(); + + populateCsharpNamespaceSiblings( + parsedFiles, + { + bindings: new Map(), + bindingAugmentations, + workspaceFqnBindings, + } as unknown as ScopeResolutionIndexes, + { + fileContents: new Map([ + ['a.cs', 'class A { }\n'], // no `namespace` => global + ['b.cs', 'class B { }\n'], + ]), + }, + ); + + // Global types are reachable workspace-wide via simple-name keys. + expect(workspaceFqnBindings.get('A')?.map((b) => b.def.nodeId)).toEqual(['def:a.A']); + expect(workspaceFqnBindings.get('B')?.map((b) => b.def.nodeId)).toEqual(['def:b.B']); + // O(D) invariant: one entry per unique simple name, never scopes x defs. + expect(workspaceFqnBindings.size).toBe(2); + // The whole point of the fast path: no per-scope augmentation explosion. + expect(bindingAugmentations.size).toBe(0); + // Workspace entries carry the cross-file `namespace` origin (so shadowing + // precedence in lookupBindingsAt orders them after local/finalized). + expect(workspaceFqnBindings.get('A')?.[0]?.origin).toBe('namespace'); + }); + + it('keeps every declaration of a repeated global simple name (partial classes across files)', () => { + // Two global-namespace files each declare `Foo` (a partial class split + // across files => distinct nodeIds, same simple name). Both must survive + // in the workspace channel — the fast path de-dups by nodeId, not by name, + // so partial-class members from both files stay resolvable. + const foo1 = classDef('def:foo1.Foo', 'foo1.cs', 'Foo'); + const foo2 = classDef('def:foo2.Foo', 'foo2.cs', 'Foo'); + const moduleA = scope('scope:foo1:module', 'Module', 'foo1.cs'); + const classA = scope('scope:foo1:class', 'Class', 'foo1.cs', moduleA.id, [foo1]); + const moduleB = scope('scope:foo2:module', 'Module', 'foo2.cs'); + const classB = scope('scope:foo2:class', 'Class', 'foo2.cs', moduleB.id, [foo2]); + const parsedFiles: ParsedFile[] = [ + { + filePath: 'foo1.cs', + moduleScope: moduleA.id, + scopes: Object.freeze([moduleA, classA]), + parsedImports: Object.freeze([]), + localDefs: Object.freeze([foo1]), + referenceSites: Object.freeze([]), + } as ParsedFile, + { + filePath: 'foo2.cs', + moduleScope: moduleB.id, + scopes: Object.freeze([moduleB, classB]), + parsedImports: Object.freeze([]), + localDefs: Object.freeze([foo2]), + referenceSites: Object.freeze([]), + } as ParsedFile, + ]; + const bindingAugmentations = new Map>(); + const workspaceFqnBindings = new Map(); + + populateCsharpNamespaceSiblings( + parsedFiles, + { + bindings: new Map(), + bindingAugmentations, + workspaceFqnBindings, + } as unknown as ScopeResolutionIndexes, + { + fileContents: new Map([ + ['foo1.cs', 'class Foo { }\n'], + ['foo2.cs', 'class Foo { }\n'], + ]), + }, + ); + + // Both partial declarations are kept (de-dup is by nodeId, not name). + expect( + workspaceFqnBindings + .get('Foo') + ?.map((b) => b.def.nodeId) + .sort(), + ).toEqual(['def:foo1.Foo', 'def:foo2.Foo']); + expect(bindingAugmentations.size).toBe(0); + }); }); describe('csharpReceiverBinding', () => { diff --git a/gitnexus/test/unit/scope-resolution/validate-bindings-immutability.test.ts b/gitnexus/test/unit/scope-resolution/validate-bindings-immutability.test.ts index bcf23200c..880fcedf1 100644 --- a/gitnexus/test/unit/scope-resolution/validate-bindings-immutability.test.ts +++ b/gitnexus/test/unit/scope-resolution/validate-bindings-immutability.test.ts @@ -22,10 +22,12 @@ const mkRef = (nodeId: string): BindingRef => const mkIndexes = ( bindings: Map>, augmentations: Map>, + workspace: Map = new Map(), ): ScopeResolutionIndexes => ({ bindings, bindingAugmentations: augmentations, + workspaceFqnBindings: workspace, }) as unknown as ScopeResolutionIndexes; describe('validateBindingsImmutability', () => { @@ -88,6 +90,24 @@ describe('validateBindingsImmutability', () => { expect(onWarn.mock.calls[0][0]).toMatch(/I8/); }); + it('warns when a bucket in indexes.workspaceFqnBindings IS frozen', () => { + vi.stubEnv('NODE_ENV', 'development'); + const workspace = new Map([ + ['User', Object.freeze([mkRef('def:User')]) as BindingRef[]], + ]); + const onWarn = vi.fn(); + + const violations = validateBindingsImmutability( + mkIndexes(new Map(), new Map(), workspace), + onWarn, + ); + + expect(violations).toBe(1); + expect(onWarn).toHaveBeenCalledTimes(1); + expect(onWarn.mock.calls[0][0]).toMatch(/indexes\.workspaceFqnBindings/); + expect(onWarn.mock.calls[0][0]).toMatch(/I8/); + }); + it('does not detect semantically wrong frozen replacements in indexes.bindings', () => { vi.stubEnv('NODE_ENV', 'development'); const bindings = new Map>([ diff --git a/gitnexus/test/unit/scope-resolution/walkers-augmentations.test.ts b/gitnexus/test/unit/scope-resolution/walkers-augmentations.test.ts index d83e1fa36..21d48fc49 100644 --- a/gitnexus/test/unit/scope-resolution/walkers-augmentations.test.ts +++ b/gitnexus/test/unit/scope-resolution/walkers-augmentations.test.ts @@ -32,9 +32,11 @@ const ref = (nodeId: string, origin: BindingRef['origin'] = 'local'): BindingRef function indexesWith({ finalized, augmented, + workspace, }: { finalized?: readonly BindingRef[]; augmented?: readonly BindingRef[]; + workspace?: readonly BindingRef[]; }): ScopeResolutionIndexes { const bindings = new Map>(); if (finalized !== undefined) { @@ -43,7 +45,13 @@ function indexesWith({ } const bindingAugmentations = new Map>(); if (augmented !== undefined) bindingAugmentations.set(SCOPE, new Map([['name', augmented]])); - return { bindings, bindingAugmentations } as unknown as ScopeResolutionIndexes; + const workspaceFqnBindings = new Map(); + if (workspace !== undefined) workspaceFqnBindings.set('name', workspace); + return { + bindings, + bindingAugmentations, + workspaceFqnBindings, + } as unknown as ScopeResolutionIndexes; } function scope(id: ScopeId, bindings = new Map()): Scope { @@ -105,6 +113,33 @@ describe('lookupBindingsAt', () => { expect(out.find((b) => b.def.nodeId === 'A')!.origin).toBe('import'); }); + // Third channel: workspaceFqnBindings (scope-independent — global-namespace + // C# types / PHP FQNs). Consulted LAST, after finalized + augmented. + it('returns the workspace bucket when it is the only channel', () => { + const workspace = [ref('W', 'namespace')]; + const out = lookupBindingsAt(SCOPE, 'name', indexesWith({ workspace })); + expect(out).toEqual(workspace); + expect(out).toBe(workspace); // identity preserved when only one channel populates + }); + + it('appends workspace entries after finalized and augmented', () => { + const finalized = [ref('A', 'import')]; + const augmented = [ref('B', 'namespace')]; + const workspace = [ref('C', 'namespace')]; + const out = lookupBindingsAt(SCOPE, 'name', indexesWith({ finalized, augmented, workspace })); + expect(out.map((b) => b.def.nodeId)).toEqual(['A', 'B', 'C']); + }); + + it('dedupes workspace entries already present in finalized/augmented (workspace loses)', () => { + const finalized = [ref('A', 'import')]; + const augmented = [ref('B', 'namespace')]; + const workspace = [ref('A', 'namespace'), ref('B', 'namespace'), ref('C', 'namespace')]; + const out = lookupBindingsAt(SCOPE, 'name', indexesWith({ finalized, augmented, workspace })); + expect(out.map((b) => b.def.nodeId)).toEqual(['A', 'B', 'C']); + // The surviving A/B keep their finalized/augmented identity, not workspace's. + expect(out.find((b) => b.def.nodeId === 'A')!.origin).toBe('import'); + }); + it('keeps finalized metadata when the same nodeId appears in both channels', () => { const finalizedDef = { nodeId: 'A',