From f0381b26160bac615710b781f70cf31b70e7d0e2 Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Fri, 29 May 2026 17:39:55 +0000 Subject: [PATCH] fix(csharp): cover global-namespace workspaceFqnBindings path + doc + using-static perf MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../languages/csharp/namespace-siblings.ts | 5 +- .../model/scope-resolution-indexes.ts | 14 +++-- .../csharp/csharp-hooks.test.ts | 62 +++++++++++++++++++ 3 files changed, 75 insertions(+), 6 deletions(-) diff --git a/gitnexus/src/core/ingestion/languages/csharp/namespace-siblings.ts b/gitnexus/src/core/ingestion/languages/csharp/namespace-siblings.ts index 27d057497..15938fb59 100644 --- a/gitnexus/src/core/ingestion/languages/csharp/namespace-siblings.ts +++ b/gitnexus/src/core/ingestion/languages/csharp/namespace-siblings.ts @@ -318,6 +318,9 @@ 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; @@ -343,7 +346,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; diff --git a/gitnexus/src/core/ingestion/model/scope-resolution-indexes.ts b/gitnexus/src/core/ingestion/model/scope-resolution-indexes.ts index 561595ac5..435a058ab 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. Two languages populate it + * with disjoint key formats that never collide: + * - PHP namespace-siblings (Step 3b): backslash-separated FQNs + * (e.g. `App\Models\User`). + * - C# namespace-siblings: simple names for global-(default-)namespace + * types (e.g. `User`), which are visible from every file — one entry + * per simple name 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/test/unit/scope-resolution/csharp/csharp-hooks.test.ts b/gitnexus/test/unit/scope-resolution/csharp/csharp-hooks.test.ts index 808bce9cf..63f490919 100644 --- a/gitnexus/test/unit/scope-resolution/csharp/csharp-hooks.test.ts +++ b/gitnexus/test/unit/scope-resolution/csharp/csharp-hooks.test.ts @@ -322,6 +322,68 @@ 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); + }); }); describe('csharpReceiverBinding', () => {