From d9ba9aa998f6db8ac7763427b00a962fb13893e4 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Wed, 8 Apr 2026 23:24:07 +0100 Subject: [PATCH 01/12] SM-10: Add MRO fast path before D2 fuzzy widening in resolveCallTarget (#741) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Initial plan * Add MRO fast path before D2 fuzzy widening in resolveCallTarget When receiverTypeName is known, try resolveMethodByOwner (owner-scoped + MRO lookup) before falling back to the expensive lookupFuzzy in D2. This short-circuits cross-file member call resolution for the common non-overloaded case. The fast path is skipped when overload disambiguation hints are available (overloadHints or preComputedArgTypes) to avoid picking the wrong overload for same-return-type overloaded methods. Passes heritageMap to resolveCallTarget from all 4 call sites: - Language seed path (processCalls) - Sequential path (processCalls) - walkMixedChain fallback - Worker path (processCallsFromExtracted) Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/9e49521f-2472-47bc-96e9-be4a46b073f0 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * fix(SM-10): address PR #741 review Correctness: - Module-alias guard for D0. When call.receiverName matches an active entry in ctx.moduleAliasMap for the current file, D0 is now skipped and resolution falls through to D1-D4 which respects the alias-narrowed candidate pool. Prevents a homonymous class in a different file from being picked by ctx.resolve(receiverTypeName) inside resolveMethodByOwner. New unit test pins the contract. Unit tests (call-processor.test.ts — 3 new): - D0 hit: child.parentMethod() resolves via MRO walk when heritageMap is provided. - D0 skipped: same scenario still resolves via D1-D4 when heritageMap is undefined (backward-compat guard). - Module-alias guard: two files both define class User with a save() method; 'import auth_mod as auth' in app.py must resolve auth.user.save() to auth_mod.py, not user_mod.py. Integration language coverage (+3 fixtures/tests): - swift-child-extends-parent — first-wins, gated on swiftAvailable. - ruby-child-extends-parent — first-wins. - php-child-extends-parent — first-wins (uses ParentClass since 'Parent' is a PHP reserved word). * test(SM-10): address second PR #741 review round Unit tests (call-processor.test.ts, +2 new): - overloadHints guard: Java source with two same-return-type overloads method(int) and method(String), int added first so lookupMethodByOwner would return it. processCalls auto-generates overloadHints for Java, forcing D0 to be skipped. o.method("hello") must resolve to method(String) via literal-inferred disambiguation. - preComputedArgTypes guard: worker-path equivalent via processCallsFromExtracted with ExtractedCall.argTypes=['String']. Same two overloads, same correctness guarantee. Integration tests (+2 fixtures + test blocks): - go-child-extends-parent — struct embedding, first-wins (Go structs are labeled 'Struct' not 'Class' in GitNexus). - dart-child-extends-parent — extends, first-wins, gated on dartAvailable like other Dart tests. Documentation: - Expanded the fallthrough comment in resolveMethodByOwner to clarify that unknown-extension paths land on plain lookupMethodByOwner without an ancestor walk, and that D1-D4 still runs on D0 miss. * test(SM-10): D0 miss with heritageMap present falls through to D1-D4 Closes the last remaining gap from PR #741 review round 3. The existing 'D0 skipped' test only covered the heritageMap=undefined case, leaving the miss-with-heritageMap path implicitly covered by integration tests only. This adds a focused unit test where: - Class Obj has a method doWork findable via tiered resolution (import-scoped) but intentionally NOT registered in methodByOwner (no ownerId), so lookupMethodByOwner misses. - heritageMap is provided but built from an empty heritage array, so getAncestors(class:Obj) returns []. The MRO walk yields no parents. - lookupMethodByOwnerWithMRO therefore returns undefined → D0 miss. - D1 resolves the receiver type; D2 widens via lookupFuzzy; D3 file-filter picks the single matching candidate. - A CALLS edge must still be emitted — D0 miss must not swallow the call. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> Co-authored-by: Gergo Magyar --- gitnexus/src/core/ingestion/call-processor.ts | 47 ++- .../dart-child-extends-parent/app.dart | 8 + .../dart-child-extends-parent/child.dart | 3 + .../dart-child-extends-parent/parent.dart | 5 + .../go-child-extends-parent/go.mod | 3 + .../go-child-extends-parent/models/child.go | 5 + .../go-child-extends-parent/models/parent.go | 7 + .../go-child-extends-parent/services/app.go | 8 + .../php-child-extends-parent/src/App.php | 14 + .../php-child-extends-parent/src/Child.php | 7 + .../php-child-extends-parent/src/Parent.php | 11 + .../ruby-child-extends-parent/lib/app.rb | 8 + .../ruby-child-extends-parent/lib/child.rb | 4 + .../ruby-child-extends-parent/lib/parent.rb | 5 + .../Sources/App.swift | 6 + .../Sources/Child.swift | 2 + .../Sources/Parent.swift | 5 + .../test/integration/resolvers/dart.test.ts | 38 ++ .../test/integration/resolvers/go.test.ts | 32 ++ .../test/integration/resolvers/php.test.ts | 27 ++ .../test/integration/resolvers/ruby.test.ts | 27 ++ .../test/integration/resolvers/swift.test.ts | 33 ++ gitnexus/test/unit/call-processor.test.ts | 345 ++++++++++++++++++ 23 files changed, 648 insertions(+), 2 deletions(-) create mode 100644 gitnexus/test/fixtures/lang-resolution/dart-child-extends-parent/app.dart create mode 100644 gitnexus/test/fixtures/lang-resolution/dart-child-extends-parent/child.dart create mode 100644 gitnexus/test/fixtures/lang-resolution/dart-child-extends-parent/parent.dart create mode 100644 gitnexus/test/fixtures/lang-resolution/go-child-extends-parent/go.mod create mode 100644 gitnexus/test/fixtures/lang-resolution/go-child-extends-parent/models/child.go create mode 100644 gitnexus/test/fixtures/lang-resolution/go-child-extends-parent/models/parent.go create mode 100644 gitnexus/test/fixtures/lang-resolution/go-child-extends-parent/services/app.go create mode 100644 gitnexus/test/fixtures/lang-resolution/php-child-extends-parent/src/App.php create mode 100644 gitnexus/test/fixtures/lang-resolution/php-child-extends-parent/src/Child.php create mode 100644 gitnexus/test/fixtures/lang-resolution/php-child-extends-parent/src/Parent.php create mode 100644 gitnexus/test/fixtures/lang-resolution/ruby-child-extends-parent/lib/app.rb create mode 100644 gitnexus/test/fixtures/lang-resolution/ruby-child-extends-parent/lib/child.rb create mode 100644 gitnexus/test/fixtures/lang-resolution/ruby-child-extends-parent/lib/parent.rb create mode 100644 gitnexus/test/fixtures/lang-resolution/swift-child-extends-parent/Sources/App.swift create mode 100644 gitnexus/test/fixtures/lang-resolution/swift-child-extends-parent/Sources/Child.swift create mode 100644 gitnexus/test/fixtures/lang-resolution/swift-child-extends-parent/Sources/Parent.swift diff --git a/gitnexus/src/core/ingestion/call-processor.ts b/gitnexus/src/core/ingestion/call-processor.ts index ecb778a46..deb58e9e9 100644 --- a/gitnexus/src/core/ingestion/call-processor.ts +++ b/gitnexus/src/core/ingestion/call-processor.ts @@ -787,6 +787,8 @@ export const processCalls = async ( ctx, undefined, widenCache, + undefined, + heritageMap, ); if (!resolved) return; @@ -1033,6 +1035,8 @@ export const processCalls = async ( ctx, hints, widenCache, + undefined, + heritageMap, ); if (!resolved) return; @@ -1285,6 +1289,7 @@ const resolveCallTarget = ( overloadHints?: OverloadHints, widenCache?: WidenCache, preComputedArgTypes?: (string | undefined)[], + heritageMap?: HeritageMap, ): ResolveResult | null => { const tiered = ctx.resolve(call.calledName, currentFile); if (!tiered) return null; @@ -1360,6 +1365,35 @@ const resolveCallTarget = ( // belong to the wrong class (e.g. super.save() should hit the parent's save, // not the child's own save method in the same file). if (call.callForm === 'member' && call.receiverTypeName) { + // D0. MRO fast path: when heritageMap is available, try owner-scoped + MRO + // lookup before falling back to the expensive D2 fuzzy widening. + // This short-circuits the lookupFuzzy call for every cross-file member call. + // Skip conditions: + // (a) overloadHints or preComputedArgTypes present — the MRO lookup may + // pick the wrong overload for same-return-type overloads since it + // does not consider argument types. D2-D4+E handles those correctly. + // (b) A module alias on call.receiverName is active for this file — the + // alias block above already narrowed `filteredCandidates` to a + // specific file (e.g. Python `import auth; auth.user.save()`). + // resolveMethodByOwner re-resolves `receiverTypeName` from scratch + // via `ctx.resolve`, which ignores that narrowing and could pick a + // homonymous class from the wrong file. Fall through to D1-D4 which + // respects the alias-filtered candidate pool. + const hasActiveModuleAlias = + !!call.receiverName && ctx.moduleAliasMap?.get(currentFile)?.has(call.receiverName) === true; + if (!overloadHints && !preComputedArgTypes && !hasActiveModuleAlias) { + const mroResult = resolveMethodByOwner( + call.receiverTypeName, + call.calledName, + currentFile, + ctx, + heritageMap, + ); + if (mroResult) { + return toResolveResult(mroResult, tiered.tier); + } + } + // D1. Resolve the receiver type const typeResolved = ctx.resolve(call.receiverTypeName, currentFile); if (typeResolved && typeResolved.candidates.length > 0) { @@ -1628,8 +1662,12 @@ const resolveMethodByOwner = ( } } - // Fallback when no HeritageMap (or the file extension is unrecognized): - // plain direct lookup with no ancestor walk. + // Fallback when no HeritageMap (or the file extension is unrecognized by + // `getLanguageFromFilename`, e.g. a synthetic path or an extension that is + // not registered in supported-languages.ts): plain direct lookup with no + // ancestor walk. All primary languages register their extensions, so this + // branch is only reached for edge cases where the MRO walk would not be + // applicable anyway. D1-D4 in resolveCallTarget still runs on D0 miss. return ctx.symbols.lookupMethodByOwner(classDef.nodeId, methodName); }; @@ -1839,6 +1877,10 @@ const walkMixedChain = ( { calledName: step.name, callForm: 'member', receiverTypeName: currentType }, filePath, ctx, + undefined, + undefined, + undefined, + heritageMap, ); if (!resolved) { // Stdlib passthrough: unwrap(), clone(), etc. preserve the receiver type @@ -1988,6 +2030,7 @@ export const processCallsFromExtracted = async ( undefined, widenCache, effectiveCall.argTypes, + heritageMap, ); if (!resolved) { // Vue template component fallback: match calledName against imported .vue basenames diff --git a/gitnexus/test/fixtures/lang-resolution/dart-child-extends-parent/app.dart b/gitnexus/test/fixtures/lang-resolution/dart-child-extends-parent/app.dart new file mode 100644 index 000000000..0c431a071 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/dart-child-extends-parent/app.dart @@ -0,0 +1,8 @@ +import 'child.dart'; + +class App { + void run() { + final c = Child(); + c.parentMethod(); + } +} diff --git a/gitnexus/test/fixtures/lang-resolution/dart-child-extends-parent/child.dart b/gitnexus/test/fixtures/lang-resolution/dart-child-extends-parent/child.dart new file mode 100644 index 000000000..fafe63719 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/dart-child-extends-parent/child.dart @@ -0,0 +1,3 @@ +import 'parent.dart'; + +class Child extends Parent {} diff --git a/gitnexus/test/fixtures/lang-resolution/dart-child-extends-parent/parent.dart b/gitnexus/test/fixtures/lang-resolution/dart-child-extends-parent/parent.dart new file mode 100644 index 000000000..28c09da37 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/dart-child-extends-parent/parent.dart @@ -0,0 +1,5 @@ +class Parent { + String parentMethod() { + return 'parent'; + } +} diff --git a/gitnexus/test/fixtures/lang-resolution/go-child-extends-parent/go.mod b/gitnexus/test/fixtures/lang-resolution/go-child-extends-parent/go.mod new file mode 100644 index 000000000..192e075e8 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/go-child-extends-parent/go.mod @@ -0,0 +1,3 @@ +module example.com/app + +go 1.21 diff --git a/gitnexus/test/fixtures/lang-resolution/go-child-extends-parent/models/child.go b/gitnexus/test/fixtures/lang-resolution/go-child-extends-parent/models/child.go new file mode 100644 index 000000000..aac88e503 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/go-child-extends-parent/models/child.go @@ -0,0 +1,5 @@ +package models + +type Child struct { + Parent +} diff --git a/gitnexus/test/fixtures/lang-resolution/go-child-extends-parent/models/parent.go b/gitnexus/test/fixtures/lang-resolution/go-child-extends-parent/models/parent.go new file mode 100644 index 000000000..4aa63ee03 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/go-child-extends-parent/models/parent.go @@ -0,0 +1,7 @@ +package models + +type Parent struct{} + +func (p *Parent) ParentMethod() string { + return "parent" +} diff --git a/gitnexus/test/fixtures/lang-resolution/go-child-extends-parent/services/app.go b/gitnexus/test/fixtures/lang-resolution/go-child-extends-parent/services/app.go new file mode 100644 index 000000000..cde995386 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/go-child-extends-parent/services/app.go @@ -0,0 +1,8 @@ +package services + +import "example.com/app/models" + +func Run() { + c := &models.Child{} + c.ParentMethod() +} diff --git a/gitnexus/test/fixtures/lang-resolution/php-child-extends-parent/src/App.php b/gitnexus/test/fixtures/lang-resolution/php-child-extends-parent/src/App.php new file mode 100644 index 000000000..71ddc605a --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/php-child-extends-parent/src/App.php @@ -0,0 +1,14 @@ +parentMethod(); + } +} diff --git a/gitnexus/test/fixtures/lang-resolution/php-child-extends-parent/src/Child.php b/gitnexus/test/fixtures/lang-resolution/php-child-extends-parent/src/Child.php new file mode 100644 index 000000000..a031de193 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/php-child-extends-parent/src/Child.php @@ -0,0 +1,7 @@ + String { + return "parent" + } +} diff --git a/gitnexus/test/integration/resolvers/dart.test.ts b/gitnexus/test/integration/resolvers/dart.test.ts index 7f995e09d..82fa77f22 100644 --- a/gitnexus/test/integration/resolvers/dart.test.ts +++ b/gitnexus/test/integration/resolvers/dart.test.ts @@ -474,3 +474,41 @@ describe.skipIf(!dartAvailable)('Dart interface dispatch (METHOD_IMPLEMENTS)', ( expect(saveEdge).toBeDefined(); }); }); + +// --------------------------------------------------------------------------- +// SM-9/SM-10: lookupMethodByOwnerWithMRO + D0 fast path — Dart first-wins +// --------------------------------------------------------------------------- + +describe.skipIf(!dartAvailable)( + 'Dart Child extends Parent — inherited method resolution (SM-9)', + () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo( + path.join(FIXTURES, 'dart-child-extends-parent'), + () => {}, + ); + }, 60000); + + it('detects Parent and Child classes', () => { + const classes = getNodesByLabel(result, 'Class'); + expect(classes).toContain('Parent'); + expect(classes).toContain('Child'); + }); + + it('emits EXTENDS edge: Child → Parent', () => { + const extends_ = getRelationships(result, 'EXTENDS'); + expect(edgeSet(extends_)).toContain('Child → Parent'); + }); + + it('resolves c.parentMethod() to Parent.parentMethod via first-wins MRO walk', () => { + const calls = getRelationships(result, 'CALLS'); + const parentMethodCall = calls.find( + (c) => c.target === 'parentMethod' && c.targetFilePath.includes('parent.dart'), + ); + expect(parentMethodCall).toBeDefined(); + expect(parentMethodCall!.source).toBe('run'); + }); + }, +); diff --git a/gitnexus/test/integration/resolvers/go.test.ts b/gitnexus/test/integration/resolvers/go.test.ts index 0ec9b2f6c..69761f889 100644 --- a/gitnexus/test/integration/resolvers/go.test.ts +++ b/gitnexus/test/integration/resolvers/go.test.ts @@ -1345,3 +1345,35 @@ describe('Go method enrichment', () => { expect(classifyCall).toBeDefined(); }); }); + +// --------------------------------------------------------------------------- +// SM-9/SM-10: lookupMethodByOwnerWithMRO + D0 fast path — Go struct embedding +// --------------------------------------------------------------------------- + +describe('Go Child embeds Parent — inherited method resolution (SM-9)', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo(path.join(FIXTURES, 'go-child-extends-parent'), () => {}); + }, 60000); + + it('detects Parent and Child structs', () => { + const structs = getNodesByLabel(result, 'Struct'); + expect(structs).toContain('Parent'); + expect(structs).toContain('Child'); + }); + + it('emits EXTENDS edge: Child → Parent (struct embedding)', () => { + const extends_ = getRelationships(result, 'EXTENDS'); + expect(edgeSet(extends_)).toContain('Child → Parent'); + }); + + it('resolves c.ParentMethod() to Parent.ParentMethod via first-wins MRO walk', () => { + const calls = getRelationships(result, 'CALLS'); + const parentMethodCall = calls.find( + (c) => c.target === 'ParentMethod' && c.targetFilePath.includes('parent.go'), + ); + expect(parentMethodCall).toBeDefined(); + expect(parentMethodCall!.source).toBe('Run'); + }); +}); diff --git a/gitnexus/test/integration/resolvers/php.test.ts b/gitnexus/test/integration/resolvers/php.test.ts index a72da9439..e120d92f4 100644 --- a/gitnexus/test/integration/resolvers/php.test.ts +++ b/gitnexus/test/integration/resolvers/php.test.ts @@ -1780,3 +1780,30 @@ describe('PHP abstract dispatch', () => { expect(names).toEqual(['find', 'save']); }); }); + +// --------------------------------------------------------------------------- +// SM-9/SM-10: lookupMethodByOwnerWithMRO + D0 fast path — PHP first-wins +// --------------------------------------------------------------------------- + +describe('PHP Child extends ParentClass — inherited method resolution (SM-9)', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo(path.join(FIXTURES, 'php-child-extends-parent'), () => {}); + }, 60000); + + it('detects ParentClass and Child classes', () => { + const classes = getNodesByLabel(result, 'Class'); + expect(classes).toContain('ParentClass'); + expect(classes).toContain('Child'); + }); + + it('resolves $c->parentMethod() to ParentClass::parentMethod via first-wins MRO walk', () => { + const calls = getRelationships(result, 'CALLS'); + const parentMethodCall = calls.find( + (c) => c.target === 'parentMethod' && c.targetFilePath.includes('Parent.php'), + ); + expect(parentMethodCall).toBeDefined(); + expect(parentMethodCall!.source).toBe('run'); + }); +}); diff --git a/gitnexus/test/integration/resolvers/ruby.test.ts b/gitnexus/test/integration/resolvers/ruby.test.ts index 865c41c4a..7e508e756 100644 --- a/gitnexus/test/integration/resolvers/ruby.test.ts +++ b/gitnexus/test/integration/resolvers/ruby.test.ts @@ -1330,3 +1330,30 @@ describe('Ruby overload dispatch (format vs format_with_prefix)', () => { expect(methods).toContain('run'); }); }); + +// --------------------------------------------------------------------------- +// SM-9/SM-10: lookupMethodByOwnerWithMRO + D0 fast path — Ruby first-wins +// --------------------------------------------------------------------------- + +describe('Ruby Child extends Parent — inherited method resolution (SM-9)', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo(path.join(FIXTURES, 'ruby-child-extends-parent'), () => {}); + }, 60000); + + it('detects Parent and Child classes', () => { + const classes = getNodesByLabel(result, 'Class'); + expect(classes).toContain('Parent'); + expect(classes).toContain('Child'); + }); + + it('resolves c.parent_method to Parent#parent_method via first-wins MRO walk', () => { + const calls = getRelationships(result, 'CALLS'); + const parentMethodCall = calls.find( + (c) => c.target === 'parent_method' && c.targetFilePath.includes('parent.rb'), + ); + expect(parentMethodCall).toBeDefined(); + expect(parentMethodCall!.source).toBe('run'); + }); +}); diff --git a/gitnexus/test/integration/resolvers/swift.test.ts b/gitnexus/test/integration/resolvers/swift.test.ts index 272770527..f00435165 100644 --- a/gitnexus/test/integration/resolvers/swift.test.ts +++ b/gitnexus/test/integration/resolvers/swift.test.ts @@ -865,3 +865,36 @@ describe.skipIf(!swiftAvailable)('Swift overloaded method disambiguation', () => expect(mi.length).toBe(3); }); }); + +// --------------------------------------------------------------------------- +// SM-9/SM-10: lookupMethodByOwnerWithMRO + D0 fast path — Swift first-wins +// --------------------------------------------------------------------------- + +describe.skipIf(!swiftAvailable)( + 'Swift Child extends Parent — inherited method resolution (SM-9)', + () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo( + path.join(FIXTURES, 'swift-child-extends-parent'), + () => {}, + ); + }, 60000); + + it('detects Parent and Child classes', () => { + const classes = getNodesByLabel(result, 'Class'); + expect(classes).toContain('Parent'); + expect(classes).toContain('Child'); + }); + + it('resolves c.parentMethod() to Parent.parentMethod via first-wins MRO walk', () => { + const calls = getRelationships(result, 'CALLS'); + const parentMethodCall = calls.find( + (c) => c.target === 'parentMethod' && c.targetFilePath.includes('Parent.swift'), + ); + expect(parentMethodCall).toBeDefined(); + expect(parentMethodCall!.source).toBe('run'); + }); + }, +); diff --git a/gitnexus/test/unit/call-processor.test.ts b/gitnexus/test/unit/call-processor.test.ts index bb4590d5a..81dc9f042 100644 --- a/gitnexus/test/unit/call-processor.test.ts +++ b/gitnexus/test/unit/call-processor.test.ts @@ -1591,3 +1591,348 @@ describe('processCallsFromExtracted — interface dispatch', () => { expect(toB?.reason).toBe('interface-dispatch'); }); }); + +// --------------------------------------------------------------------------- +// SM-10: D0 MRO fast path in resolveCallTarget +// --------------------------------------------------------------------------- + +describe('processCalls — D0 MRO fast path (SM-10)', () => { + let graph: ReturnType; + let ctx: ResolutionContext; + + beforeEach(() => { + graph = createKnowledgeGraph(); + ctx = createResolutionContext(); + }); + + const setupChildParent = () => { + const parentFile = 'src/models/Parent.java'; + const childFile = 'src/models/Child.java'; + const appFile = 'src/services/App.java'; + const parentId = 'class:models/Parent.java:Parent'; + const childId = 'class:models/Child.java:Child'; + const parentMethodId = 'method:models/Parent.java:parentMethod'; + + ctx.symbols.add(parentFile, 'Parent', parentId, 'Class'); + ctx.symbols.add(childFile, 'Child', childId, 'Class'); + ctx.symbols.add(parentFile, 'parentMethod', parentMethodId, 'Method', { + ownerId: parentId, + returnType: 'String', + }); + ctx.importMap.set(appFile, new Set([childFile, parentFile])); + return { parentFile, childFile, appFile, parentId, childId, parentMethodId }; + }; + + it('D0 hit: child.parentMethod() resolves via MRO walk when heritageMap is provided', async () => { + const { parentMethodId, appFile, parentFile, childFile } = setupChildParent(); + + const heritage: ExtractedHeritage[] = [ + { + filePath: childFile, + className: 'Child', + parentName: 'Parent', + kind: 'extends', + }, + ]; + const heritageMap = buildHeritageMap(heritage, ctx); + + await processCalls( + graph, + [ + { + path: parentFile, + content: + 'package models;\npublic class Parent {\n public String parentMethod() { return ""; }\n}\n', + }, + { + path: childFile, + content: 'package models;\npublic class Child extends Parent {}\n', + }, + { + path: appFile, + content: + 'package services;\nimport models.Child;\npublic class App {\n public void run() {\n Child c = new Child();\n c.parentMethod();\n }\n}\n', + }, + ], + createASTCache(), + ctx, + undefined, + undefined, + undefined, + undefined, + undefined, + heritageMap, + ); + + const parentMethodCalls = graph.relationships.filter( + (r) => r.type === 'CALLS' && r.targetId === parentMethodId, + ); + expect(parentMethodCalls).toHaveLength(1); + }); + + it('D0 miss: heritageMap provided but method not in MRO chain falls through to D1-D4', async () => { + // Setup: Class Obj has a method `doWork` that is findable via tiered + // resolution (import-scoped lookup), but intentionally NOT registered in + // methodByOwner (no `ownerId` property). heritageMap is provided but has + // no ancestry entry for class:Obj. Expected flow: + // D0: lookupMethodByOwner(classId, 'doWork') → undefined + // heritageMap.getAncestors(classId) → [] + // lookupMethodByOwnerWithMRO returns undefined → D0 miss + // D1-D4: receiver type resolves to Obj; D2 widens via lookupFuzzy; + // D3 file-filter picks the only candidate in Obj's file. + // Guarantees D0 miss does not swallow the call — D1-D4 still runs. + const classFile = 'src/models/Obj.java'; + const appFile = 'src/services/App.java'; + const classId = 'class:models/Obj.java:Obj'; + const doWorkId = 'method:models/Obj.java:doWork'; + + ctx.symbols.add(classFile, 'Obj', classId, 'Class'); + // Intentionally omit ownerId so methodByOwner has no entry — forces D0 miss. + ctx.symbols.add(classFile, 'doWork', doWorkId, 'Method', { + returnType: 'void', + parameterCount: 0, + }); + ctx.importMap.set(appFile, new Set([classFile])); + + // Empty heritage — no ancestry for Obj, so the MRO walk yields no parents. + const heritageMap = buildHeritageMap([], ctx); + + const calls: ExtractedCall[] = [ + { + filePath: appFile, + calledName: 'doWork', + sourceId: 'method:services/App.java:run', + argCount: 0, + callForm: 'member', + receiverTypeName: 'Obj', + }, + ]; + + await processCallsFromExtracted(graph, calls, ctx, undefined, undefined, heritageMap); + + const doWorkCalls = graph.relationships.filter( + (r) => r.type === 'CALLS' && r.targetId === doWorkId, + ); + expect(doWorkCalls).toHaveLength(1); + }); + + it('D0 skipped: same scenario still resolves via D1-D4 when heritageMap is undefined', async () => { + const { parentMethodId, appFile, parentFile, childFile } = setupChildParent(); + + await processCalls( + graph, + [ + { + path: parentFile, + content: + 'package models;\npublic class Parent {\n public String parentMethod() { return ""; }\n}\n', + }, + { + path: childFile, + content: 'package models;\npublic class Child extends Parent {}\n', + }, + { + path: appFile, + content: + 'package services;\nimport models.Child;\npublic class App {\n public void run() {\n Child c = new Child();\n c.parentMethod();\n }\n}\n', + }, + ], + createASTCache(), + ctx, + // no heritageMap — D0 fast path must be skipped, D1-D4 must still resolve + ); + + const parentMethodCalls = graph.relationships.filter( + (r) => r.type === 'CALLS' && r.targetId === parentMethodId, + ); + expect(parentMethodCalls).toHaveLength(1); + }); + + it('overloadHints guard: D0 skipped so literal-inferred overload disambiguation picks the right overload', async () => { + // Java sequential path: processCalls auto-generates `overloadHints` for + // languages whose provider exposes `inferLiteralType` (Java/Kotlin/C#/C++). + // When two overloads share the same return type, lookupMethodByOwner + // returns defs[0] (the first-added overload) regardless of argument + // types. Without the D0 guard this would mis-resolve `o.method("hello")` + // to method(int). With the guard, D0 is skipped because overloadHints + // is present, and the literal-inferred overload path in D2-D4+E picks + // method(String) correctly. + const classFile = 'src/models/Obj.java'; + const appFile = 'src/services/App.java'; + const classId = 'class:models/Obj.java:Obj'; + const methodIntId = 'method:models/Obj.java:method(int)'; + const methodStringId = 'method:models/Obj.java:method(String)'; + + ctx.symbols.add(classFile, 'Obj', classId, 'Class'); + // int overload added FIRST so lookupMethodByOwner would return it. + ctx.symbols.add(classFile, 'method', methodIntId, 'Method', { + ownerId: classId, + returnType: 'String', + parameterCount: 1, + parameterTypes: ['int'], + }); + ctx.symbols.add(classFile, 'method', methodStringId, 'Method', { + ownerId: classId, + returnType: 'String', + parameterCount: 1, + parameterTypes: ['String'], + }); + ctx.importMap.set(appFile, new Set([classFile])); + + const heritageMap = buildHeritageMap([], ctx); + + await processCalls( + graph, + [ + { + path: classFile, + content: + 'package models;\npublic class Obj {\n public String method(int x) { return ""; }\n public String method(String s) { return ""; }\n}\n', + }, + { + path: appFile, + content: + 'package services;\nimport models.Obj;\npublic class App {\n public void run() {\n Obj o = new Obj();\n o.method("hello");\n }\n}\n', + }, + ], + createASTCache(), + ctx, + undefined, + undefined, + undefined, + undefined, + undefined, + heritageMap, + ); + + // Exactly one resolved call, and it must target the String overload. + const methodCalls = graph.relationships.filter( + (r) => r.type === 'CALLS' && (r.targetId === methodIntId || r.targetId === methodStringId), + ); + expect(methodCalls).toHaveLength(1); + expect(methodCalls[0].targetId).toBe(methodStringId); + }); + + it('preComputedArgTypes guard: D0 skipped so arg-type disambiguation picks the right overload', async () => { + // Two overloads of the same method with identical return types live on + // the same owner class. Without the D0 guard, lookupMethodByOwner would + // return defs[0] (the first overload added) regardless of argument types, + // silently mis-resolving an `obj.method("hello")` call to method(int). + // With the guard, preComputedArgTypes forces D0 to be skipped and D2-D4+E + // disambiguates by parameter type. + const classFile = 'src/models/Obj.java'; + const appFile = 'src/services/App.java'; + const classId = 'class:models/Obj.java:Obj'; + const methodIntId = 'method:models/Obj.java:method(int)'; + const methodStringId = 'method:models/Obj.java:method(String)'; + + ctx.symbols.add(classFile, 'Obj', classId, 'Class'); + // int overload added FIRST — without the guard this would be returned by + // lookupMethodByOwner's same-return-type fast path. + ctx.symbols.add(classFile, 'method', methodIntId, 'Method', { + ownerId: classId, + returnType: 'String', + parameterCount: 1, + parameterTypes: ['int'], + }); + ctx.symbols.add(classFile, 'method', methodStringId, 'Method', { + ownerId: classId, + returnType: 'String', + parameterCount: 1, + parameterTypes: ['String'], + }); + ctx.importMap.set(appFile, new Set([classFile])); + + const heritageMap = buildHeritageMap([], ctx); + + const calls: ExtractedCall[] = [ + { + filePath: appFile, + calledName: 'method', + sourceId: 'method:services/App.java:run', + argCount: 1, + callForm: 'member', + receiverTypeName: 'Obj', + argTypes: ['String'], + }, + ]; + + await processCallsFromExtracted(graph, calls, ctx, undefined, undefined, heritageMap); + + const methodCalls = graph.relationships.filter((r) => r.type === 'CALLS'); + // Exactly one resolved call, and it must target the String overload — + // NOT the int overload that lookupMethodByOwner would have returned. + expect(methodCalls).toHaveLength(1); + expect(methodCalls[0].targetId).toBe(methodStringId); + }); + + it('module-alias guard: D0 skipped when receiverName matches an active module alias', async () => { + // Setup: two files each define a class named User with a method save(). + // The caller has a Python-style module alias `import auth_mod as auth`, + // so auth.User().save() must resolve to auth_mod.py, NOT user_mod.py. + // D0 would call ctx.resolve('User') and could pick the wrong file; the + // alias guard must short-circuit D0 so the alias-filtered D1-D4 path + // runs and picks the correct file. + const authModFile = 'auth_mod.py'; + const userModFile = 'user_mod.py'; + const appFile = 'app.py'; + const authUserId = 'class:auth_mod.py:User'; + const userUserId = 'class:user_mod.py:User'; + const authSaveId = 'method:auth_mod.py:save'; + const userSaveId = 'method:user_mod.py:save'; + + ctx.symbols.add(authModFile, 'User', authUserId, 'Class'); + ctx.symbols.add(userModFile, 'User', userUserId, 'Class'); + ctx.symbols.add(authModFile, 'save', authSaveId, 'Method', { + ownerId: authUserId, + returnType: 'bool', + }); + ctx.symbols.add(userModFile, 'save', userSaveId, 'Method', { + ownerId: userUserId, + returnType: 'bool', + }); + // Register the module alias: in app.py, `auth` points to auth_mod.py. + const aliasMap = new Map([['auth', authModFile]]); + ctx.moduleAliasMap.set(appFile, aliasMap); + ctx.importMap.set(appFile, new Set([authModFile])); + + const heritageMap = buildHeritageMap([], ctx); + + await processCalls( + graph, + [ + { + path: authModFile, + content: 'class User:\n def save(self):\n return True\n', + }, + { + path: userModFile, + content: 'class User:\n def save(self):\n return True\n', + }, + { + path: appFile, + content: + 'import auth_mod as auth\n\ndef run():\n user = auth.User()\n user.save()\n', + }, + ], + createASTCache(), + ctx, + undefined, + undefined, + undefined, + undefined, + undefined, + heritageMap, + ); + + // save() must resolve to auth_mod.py, NOT user_mod.py. + const authSave = graph.relationships.find( + (r) => r.type === 'CALLS' && r.targetId === authSaveId, + ); + const userSave = graph.relationships.find( + (r) => r.type === 'CALLS' && r.targetId === userSaveId, + ); + expect(authSave).toBeDefined(); + expect(userSave).toBeUndefined(); + }); +}); From 3f28f7ead5e67cbb9ae546dda0b4a28502c2a0de Mon Sep 17 00:00:00 2001 From: Pratyush Sharma <56130065+pratyush618@users.noreply.github.com> Date: Thu, 9 Apr 2026 10:45:14 +0530 Subject: [PATCH 02/12] fix(web): correct dev-mode serve command in OnboardingGuide (#725) --- gitnexus-web/src/components/OnboardingGuide.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/gitnexus-web/src/components/OnboardingGuide.tsx b/gitnexus-web/src/components/OnboardingGuide.tsx index cbfd16236..796e0f148 100644 --- a/gitnexus-web/src/components/OnboardingGuide.tsx +++ b/gitnexus-web/src/components/OnboardingGuide.tsx @@ -201,7 +201,7 @@ interface OnboardingGuideProps { } export const OnboardingGuide = ({ isPolling }: OnboardingGuideProps) => { - const primary = isDev ? 'cd gitnexus && npm run serve' : 'npx gitnexus@latest serve'; + const primary = isDev ? 'npm run --prefix gitnexus serve' : 'npx gitnexus@latest serve'; const termLabel = isDev ? 'Start backend' : 'Terminal'; // Step states: step 1 = copy command, step 2 = run/wait, step 3 = auto-connect @@ -277,7 +277,9 @@ export const OnboardingGuide = ({ isPolling }: OnboardingGuideProps) => { state={step2State} number={2} title={isPolling ? 'Waiting for server to start' : 'Paste and run in your terminal'} - description={isPolling ? undefined : 'Open a new terminal window, paste, and hit Enter.'} + description={ + isPolling ? undefined : 'Open a terminal at the project root, paste, and hit Enter.' + } > {isPolling && } From fd67cfd5a75a14025675c85ae70813be2e042026 Mon Sep 17 00:00:00 2001 From: Cocoon-Break <54054995+kuishou68@users.noreply.github.com> Date: Thu, 9 Apr 2026 13:16:14 +0800 Subject: [PATCH 03/12] docs: fix web UI install link spacing in README (#731) --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index f0f1d7571..65ac7d332 100644 --- a/README.md +++ b/README.md @@ -52,7 +52,7 @@ https://github.com/user-attachments/assets/172685ba-8e54-4ea7-9ad1-e31a3398da72 | **What** | Index repos locally, connect AI agents via MCP | Visual graph explorer + AI chat in browser | | **For** | Daily development with Cursor, Claude Code, Codex, Windsurf, OpenCode | Quick exploration, demos, one-off analysis | | **Scale** | Full repos, any size | Limited by browser memory (~5k files), or unlimited via backend mode | -| **Install** | `npm install -g gitnexus` | No install —[gitnexus.vercel.app](https://gitnexus.vercel.app) | +| **Install** | `npm install -g gitnexus` | No install — [gitnexus.vercel.app](https://gitnexus.vercel.app) | | **Storage** | LadybugDB native (fast, persistent) | LadybugDB WASM (in-memory, per session) | | **Parsing** | Tree-sitter native bindings | Tree-sitter WASM | | **Privacy** | Everything local, no network | Everything in-browser, no server | From 9f9bbcd74495e0a57bc33865229a66331cf82cc5 Mon Sep 17 00:00:00 2001 From: evolution Date: Thu, 9 Apr 2026 13:18:17 +0800 Subject: [PATCH 04/12] feat: support GITNEXUS_HOME env var to customize global directory (#746) --- gitnexus/src/storage/repo-manager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gitnexus/src/storage/repo-manager.ts b/gitnexus/src/storage/repo-manager.ts index cc447aa4c..b233d44fb 100644 --- a/gitnexus/src/storage/repo-manager.ts +++ b/gitnexus/src/storage/repo-manager.ts @@ -212,7 +212,7 @@ export const addToGitignore = async (repoPath: string): Promise => { * Get the path to the global GitNexus directory */ export const getGlobalDir = (): string => { - return path.join(os.homedir(), '.gitnexus'); + return process.env.GITNEXUS_HOME || path.join(os.homedir(), '.gitnexus'); }; /** From 4fde5f241b3fa314cffa4a5363d4ce9c7e54b974 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Murat=20=C3=87elik?= Date: Thu, 9 Apr 2026 08:18:32 +0300 Subject: [PATCH 05/12] feat: print skipped large file paths in verbose analyze output (#745) --- gitnexus/src/core/ingestion/filesystem-walker.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/gitnexus/src/core/ingestion/filesystem-walker.ts b/gitnexus/src/core/ingestion/filesystem-walker.ts index 11721d6a7..575efc0f3 100644 --- a/gitnexus/src/core/ingestion/filesystem-walker.ts +++ b/gitnexus/src/core/ingestion/filesystem-walker.ts @@ -1,3 +1,4 @@ +import { isVerboseIngestionEnabled } from './utils/verbose.js'; import fs from 'fs/promises'; import path from 'path'; import { glob } from 'glob'; @@ -43,6 +44,7 @@ export const walkRepositoryPaths = async ( const entries: ScannedFile[] = []; let processed = 0; let skippedLarge = 0; + const skippedLargePaths: string[] = []; for (let start = 0; start < filtered.length; start += READ_CONCURRENCY) { const batch = filtered.slice(start, start + READ_CONCURRENCY); @@ -52,6 +54,7 @@ export const walkRepositoryPaths = async ( const stat = await fs.stat(fullPath); if (stat.size > MAX_FILE_SIZE) { skippedLarge++; + skippedLargePaths.push(relativePath.replace(/\\/g, '/')); return null; } return { path: relativePath.replace(/\\/g, '/'), size: stat.size }; @@ -73,6 +76,11 @@ export const walkRepositoryPaths = async ( console.warn( ` Skipped ${skippedLarge} large files (>${MAX_FILE_SIZE / 1024}KB, likely generated/vendored)`, ); + if (isVerboseIngestionEnabled()) { + for (const p of skippedLargePaths) { + console.warn(` - ${p}`); + } + } } return entries; From 9ab92a97d09ebfa88e6e4400f855426b2ec009c2 Mon Sep 17 00:00:00 2001 From: Pratyush Sharma <56130065+pratyush618@users.noreply.github.com> Date: Thu, 9 Apr 2026 11:10:17 +0530 Subject: [PATCH 06/12] fix(deps): pin tree-sitter-c override to resolve peer dep conflict (#720) (#723) --- gitnexus/package.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gitnexus/package.json b/gitnexus/package.json index 7699208e2..07723f3e3 100644 --- a/gitnexus/package.json +++ b/gitnexus/package.json @@ -104,7 +104,8 @@ "overrides": { "@huggingface/transformers": { "onnxruntime-node": "$onnxruntime-node" - } + }, + "tree-sitter-c": "0.23.2" }, "engines": { "node": ">=20.0.0" From d6debf3324822b40a40cebf4de084b26c7f3c4e5 Mon Sep 17 00:00:00 2001 From: Roshan Warrier Date: Thu, 9 Apr 2026 12:56:15 +0530 Subject: [PATCH 07/12] fix(symbol-table): index constructors in methodByOwner (#753) Co-authored-by: txhno <198242577+txhno@users.noreply.github.com> --- .../src/app.ts | 4 ++-- .../src/repo.ts | 4 +++- .../src/user.ts | 4 +++- .../integration/resolvers/typescript.test.ts | 18 ++++++++++++++++++ 4 files changed, 26 insertions(+), 4 deletions(-) diff --git a/gitnexus/test/fixtures/lang-resolution/typescript-constructor-type-inference/src/app.ts b/gitnexus/test/fixtures/lang-resolution/typescript-constructor-type-inference/src/app.ts index 33433b777..14add7d54 100644 --- a/gitnexus/test/fixtures/lang-resolution/typescript-constructor-type-inference/src/app.ts +++ b/gitnexus/test/fixtures/lang-resolution/typescript-constructor-type-inference/src/app.ts @@ -2,8 +2,8 @@ import { User } from './user'; import { Repo } from './repo'; export function processEntities(): void { - const user = new User(); - const repo = new Repo(); + const user = new User('alice'); + const repo = new Repo('/tmp/repo'); user.save(); repo.save(); } diff --git a/gitnexus/test/fixtures/lang-resolution/typescript-constructor-type-inference/src/repo.ts b/gitnexus/test/fixtures/lang-resolution/typescript-constructor-type-inference/src/repo.ts index 19631246b..671de6458 100644 --- a/gitnexus/test/fixtures/lang-resolution/typescript-constructor-type-inference/src/repo.ts +++ b/gitnexus/test/fixtures/lang-resolution/typescript-constructor-type-inference/src/repo.ts @@ -1,5 +1,7 @@ export class Repo { + constructor(private readonly path: string) {} + save(): boolean { - return false; + return this.path.length > 0; } } diff --git a/gitnexus/test/fixtures/lang-resolution/typescript-constructor-type-inference/src/user.ts b/gitnexus/test/fixtures/lang-resolution/typescript-constructor-type-inference/src/user.ts index e2af97ca7..f20c2200f 100644 --- a/gitnexus/test/fixtures/lang-resolution/typescript-constructor-type-inference/src/user.ts +++ b/gitnexus/test/fixtures/lang-resolution/typescript-constructor-type-inference/src/user.ts @@ -1,5 +1,7 @@ export class User { + constructor(private readonly name: string) {} + save(): boolean { - return true; + return this.name.length > 0; } } diff --git a/gitnexus/test/integration/resolvers/typescript.test.ts b/gitnexus/test/integration/resolvers/typescript.test.ts index 53c2287c5..c883678ac 100644 --- a/gitnexus/test/integration/resolvers/typescript.test.ts +++ b/gitnexus/test/integration/resolvers/typescript.test.ts @@ -519,6 +519,16 @@ describe('TypeScript constructor-inferred type resolution', () => { expect(saveMethods.length).toBe(2); }); + it('resolves explicit constructor calls for User and Repo', () => { + const calls = getRelationships(result, 'CALLS'); + const userCtor = calls.find((c) => c.target === 'User' && c.targetFilePath === 'src/user.ts'); + const repoCtor = calls.find((c) => c.target === 'Repo' && c.targetFilePath === 'src/repo.ts'); + expect(userCtor).toBeDefined(); + expect(repoCtor).toBeDefined(); + expect(userCtor!.targetLabel).toBe('Class'); + expect(repoCtor!.targetLabel).toBe('Class'); + }); + it('resolves user.save() to src/user.ts via constructor-inferred type', () => { const calls = getRelationships(result, 'CALLS'); const userSave = calls.find((c) => c.target === 'save' && c.targetFilePath === 'src/user.ts'); @@ -538,6 +548,14 @@ describe('TypeScript constructor-inferred type resolution', () => { const saveCalls = calls.filter((c) => c.target === 'save'); expect(saveCalls.length).toBe(2); }); + + it('resolves constructor calls for both User and Repo', () => { + const calls = getRelationships(result, 'CALLS'); + const userCtor = calls.find((c) => c.target === 'User'); + const repoCtor = calls.find((c) => c.target === 'Repo'); + expect(userCtor).toBeDefined(); + expect(repoCtor).toBeDefined(); + }); }); // --------------------------------------------------------------------------- From bb68cc1eb0f8d5a718f6b40feaca014de7879a15 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Thu, 9 Apr 2026 09:52:12 +0100 Subject: [PATCH 08/12] Extract `resolveMemberCall` from `resolveCallTarget` (SM-11) (#744) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Initial plan * feat(SM-11): extract resolveMemberCall from resolveCallTarget - Create resolveMemberCall(ownerType, methodName, currentFile, ctx, heritageMap?) that uses owner-scoped + MRO resolution only (no fuzzy lookup) - resolveCallTarget delegates member calls (D0 path) to resolveMemberCall - walkMixedChain uses resolveMemberCall for owner-scoped member-call resolution - Add 7 unit tests for resolveMemberCall covering direct, inherited, MRO, null cases, and confidence tier assertions - Export resolveMemberCall for external use Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/3b7889a9-5f2f-4572-8904-45084210f10d * fix(SM-11): address PR #744 review Blocking fixes: - B1: Revert unrelated package-lock.json gitnexus-shared addition - B2: Document confidence-tier semantic change on resolveMemberCall Performance / coupling fixes: - S1: walkMixedChain now calls resolveMethodByOwner directly (hot path) to avoid throwaway ResolveResult allocation per chain step - S2: Thread tier from resolveMethodByOwner via { def, tier } tuple; eliminates double ctx.resolve Alignment with semantic-model plan (Phase 3 target): - resolveMethodByOwner now iterates ALL class-like candidates from ctx.resolve, deduplicating matches by nodeId. Absorbs D4's ownerId-filtering into the owner-scoped path. - Handles homonym classes (two Users in different files) without falling through to D1-D4 fuzzy widening - Shared-ancestor MRO walks automatically dedup (both homonyms walk to same base method) - Unified direct-vs-MRO lookup under a single canWalkMRO check Tests added: - T1: Three D0 skip-condition tests via new _resolveCallTargetForTesting internal export (overloadHints, preComputedArgTypes, hasActiveModuleAlias) - T2: Rust qualified-syntax null test (trait-inherited method) + direct impl control - T3: C++ leftmost-base diamond inheritance test - B2 lock-in: cross-file class tier assertion - Homonym disambiguation: only-one-owns-method, both-own-method ambiguity, shared-ancestor MRO convergence Verification: - tsc --noEmit: clean - vitest run test/unit/: 3014 passed - vitest run test/integration/resolvers/: 1746 passed * test(SM-11): address second PR #744 review round + per-language integration tests Review fixes (https://github.com/abhigyanpatwari/GitNexus/pull/744#issuecomment-4211877593): P1 (Performance): Replace Map allocation in resolveMethodByOwner with a firstDef+ambiguous flag pattern. Zero allocation for the common single-candidate case on the hot path — the previous Map approach allocated on every member call regardless of whether deduplication was needed. P2 (Test gap): Strengthen the module-alias D0 skip test with a homonym fixture (two Users in different files). Previously the test passed whether or not D0 was actually bypassed; the new version proves D0 must be skipped by showing that resolveMemberCall directly returns null (ambiguous) but D1-D4 with alias narrowing picks the right one. Also fixes the underlying D2-vs-alias widening interaction: when filteredCandidates was narrowed by module-alias disambiguation, D2 no longer widens back to the full fuzzy pool (introduces aliasNarrowed boolean flag). L1 (Language coverage): Add C# and Kotlin implements-split tests at the resolveMemberCall layer. L2 (Maintainability): Export OverloadHints as @internal so the test can use a direct cast instead of fragile Parameters<...> type inference. Per-language integration tests: - rust-child-extends-parent: Direct impl method resolution via D0 (with honest documentation of the trait-method-as-Function gap that is Phase 5 / SM-16 scope) - java-interface-default-method: User implements Validator with default method resolved via implements-split MRO - csharp-interface-default-method: Same pattern for C# 8.0+ default interface methods - kotlin-interface-default-method: Same pattern for Kotlin interfaces with default implementations - python-multi-level-mro: 3-level C3 linearization (Grandparent ← Parent ← Child) - cpp-diamond-inheritance: Classic diamond (Base ← A, B ← Derived) via leftmost-base MRO Verification: - tsc --noEmit: clean - vitest run test/unit/: 3015 passed - vitest run test/integration/resolvers/: 1763 passed (+17 new per-language tests) * fix(SM-11): Codex adversarial review corrections + deeper D0 fixes Addresses the three high-severity findings from the Codex adversarial review of PR #744 (https://github.com/abhigyanpatwari/GitNexus/pull/744#issuecomment-4212075120), plus four deeper fixes discovered during regression triage. All discovered issues are now addressed end-to-end rather than papered over with tail-return fallbacks. Codex review findings: R1 (C++ diamond): The cpp-diamond-inheritance fixture used non-virtual inheritance, which is genuinely ambiguous in real C++ (two Base subobjects). Changed A and B to use 'virtual public Base' so there's a single shared Base subobject and d.method() is an unambiguous call that the leftmost-base MRO walk correctly resolves. R2 (C# default-interface): The csharp-interface-default-method fixture called user.Validate() via a User-typed variable, but C# does not inherit default interface methods as callable class members — the call is only valid through an interface-typed variable. Changed App.cs to 'IValidator user = new User(...)' which is the idiomatic dispatch pattern. R3 (resolveCallTarget tail-return): When D1-D4 receiver filtering produced zero file-matched and zero owner-matched candidates for a member call, the function fell through to the permissive single-candidate tail return — silently emitting CALLS edges for methods that don't belong to the receiver. Added an explicit null-route inside the D1-D4 block that fires only when both filters yielded 0. R4 (Rust negative assertion): Added the c.trait_only() negative integration test in rust.test.ts demonstrating that direct member calls on Rust structs do not walk trait ancestry. The test now passes because of R3 (previously fell through to the tail return). Regression triage discoveries: 1. D0 was dead code on the sequential pipeline. The sequential path sets overloadHints for every call regardless of whether the method is overloaded, and the original D0 skip condition '!overloadHints && !preComputedArgTypes' was therefore always false. The Java/C#/C++ SM-9/SM-10 inheritance tests were passing ONLY via the tail-return fallback. Fix: narrow the skip to 'overloadHints && filteredCandidates.length > 1' — skip D0 only when there are actually multiple candidates that need overload disambiguation. 2. lookupMethodByOwner couldn't disambiguate arity-differing overloads (e.g. C++ greet() vs greet(string)). With D0 now firing on the sequential path, same-name/different-arity overloads would collapse to an arbitrary first pick. Fix: added an optional argCount parameter to lookupMethodByOwner + lookupMethodByOwnerWithMRO that filters the overload set by parameterCount/requiredParameterCount before the returnType dedup. 3. Python and Rust class methods are captured as Function nodes (not Method) with ownerId set to the class. The methodByOwner index only accepted 'Method' and 'Constructor' types, so Python class methods and Rust trait methods were invisible to D0. Fix: extended the methodByOwner indexing condition to include 'Function' when ownerId is set. This also unlocks the Rust trait-method negative assertion by ensuring the qualified-syntax MRO strategy has something to return null for. 4. D0 was being skipped when a local variable shadowed an imported module name (Python 'from models.c import C; c = C()' creates both a module alias 'c → models/c.py' AND a typed local 'c'). Fix: the D0 skip now gates on 'aliasNarrowed' (a new boolean tracking whether the alias block actually narrowed filteredCandidates) instead of 'hasActiveModuleAlias'. If the method isn't in the aliased module, the receiver is a typed local variable and D0 should run. 5. PHP trait walk missed the HasTimestamps trait because lookupClassByName did not include 'Trait' type. buildHeritageMap uses lookupClassByName to resolve parent names, so 'BaseModel use HasTimestamps' was failing to register an ancestor edge for BaseModel → HasTimestamps. Fix: added 'Trait' to CLASS_TYPES. The trait is now a valid class-like type for heritage resolution (PHP use, Rust impl Trait for Struct, Scala traits). Test updates: - Updated the 'no heritageMap' unit test in call-processor.test.ts to assert the correct null-route behavior instead of the old tail-return fallback. - Added a new unit test asserting Trait inclusion in the class set. - Updated the 'does NOT include other type-like labels' test to remove Trait from its rejection set. Verification: - tsc --noEmit: clean - vitest run test/unit/: 3016 passed (+1 new Trait inclusion test) - vitest run test/integration/resolvers/: 1764 passed (+1 new Rust negative assertion) - Zero regressions --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Gergo Magyar Co-authored-by: Gergo Magyar --- gitnexus/src/core/ingestion/call-processor.ts | 290 ++++++++-- gitnexus/src/core/ingestion/symbol-table.ts | 86 ++- .../cpp-diamond-inheritance/src/A.h | 10 + .../cpp-diamond-inheritance/src/B.h | 7 + .../cpp-diamond-inheritance/src/Base.h | 6 + .../cpp-diamond-inheritance/src/Derived.h | 6 + .../cpp-diamond-inheritance/src/app.cpp | 6 + .../csharp-interface-default-method/App.cs | 15 + .../csharp-interface-default-method/User.cs | 11 + .../Validator.cs | 6 + .../java-interface-default-method/App.java | 6 + .../java-interface-default-method/User.java | 7 + .../Validator.java | 5 + .../src/App.kt | 6 + .../src/User.kt | 3 + .../src/Validator.kt | 5 + .../python-multi-level-mro/app.py | 6 + .../python-multi-level-mro/child.py | 5 + .../python-multi-level-mro/grandparent.py | 3 + .../python-multi-level-mro/parent.py | 5 + .../rust-child-extends-parent/src/child.rs | 16 + .../rust-child-extends-parent/src/main.rs | 17 + .../rust-child-extends-parent/src/parent.rs | 11 + .../test/integration/resolvers/cpp.test.ts | 34 ++ .../test/integration/resolvers/csharp.test.ts | 34 ++ .../test/integration/resolvers/java.test.ts | 34 ++ .../test/integration/resolvers/kotlin.test.ts | 34 ++ .../test/integration/resolvers/python.test.ts | 30 ++ .../test/integration/resolvers/rust.test.ts | 63 +++ gitnexus/test/unit/call-processor.test.ts | 24 +- gitnexus/test/unit/symbol-table.test.ts | 509 +++++++++++++++++- 31 files changed, 1228 insertions(+), 72 deletions(-) create mode 100644 gitnexus/test/fixtures/lang-resolution/cpp-diamond-inheritance/src/A.h create mode 100644 gitnexus/test/fixtures/lang-resolution/cpp-diamond-inheritance/src/B.h create mode 100644 gitnexus/test/fixtures/lang-resolution/cpp-diamond-inheritance/src/Base.h create mode 100644 gitnexus/test/fixtures/lang-resolution/cpp-diamond-inheritance/src/Derived.h create mode 100644 gitnexus/test/fixtures/lang-resolution/cpp-diamond-inheritance/src/app.cpp create mode 100644 gitnexus/test/fixtures/lang-resolution/csharp-interface-default-method/App.cs create mode 100644 gitnexus/test/fixtures/lang-resolution/csharp-interface-default-method/User.cs create mode 100644 gitnexus/test/fixtures/lang-resolution/csharp-interface-default-method/Validator.cs create mode 100644 gitnexus/test/fixtures/lang-resolution/java-interface-default-method/App.java create mode 100644 gitnexus/test/fixtures/lang-resolution/java-interface-default-method/User.java create mode 100644 gitnexus/test/fixtures/lang-resolution/java-interface-default-method/Validator.java create mode 100644 gitnexus/test/fixtures/lang-resolution/kotlin-interface-default-method/src/App.kt create mode 100644 gitnexus/test/fixtures/lang-resolution/kotlin-interface-default-method/src/User.kt create mode 100644 gitnexus/test/fixtures/lang-resolution/kotlin-interface-default-method/src/Validator.kt create mode 100644 gitnexus/test/fixtures/lang-resolution/python-multi-level-mro/app.py create mode 100644 gitnexus/test/fixtures/lang-resolution/python-multi-level-mro/child.py create mode 100644 gitnexus/test/fixtures/lang-resolution/python-multi-level-mro/grandparent.py create mode 100644 gitnexus/test/fixtures/lang-resolution/python-multi-level-mro/parent.py create mode 100644 gitnexus/test/fixtures/lang-resolution/rust-child-extends-parent/src/child.rs create mode 100644 gitnexus/test/fixtures/lang-resolution/rust-child-extends-parent/src/main.rs create mode 100644 gitnexus/test/fixtures/lang-resolution/rust-child-extends-parent/src/parent.rs diff --git a/gitnexus/src/core/ingestion/call-processor.ts b/gitnexus/src/core/ingestion/call-processor.ts index deb58e9e9..a739c8441 100644 --- a/gitnexus/src/core/ingestion/call-processor.ts +++ b/gitnexus/src/core/ingestion/call-processor.ts @@ -1191,9 +1191,15 @@ const toResolveResult = (definition: SymbolDefinition, tier: ResolutionTier): Re returnType: definition.returnType, }); -/** Optional hints for overload disambiguation via argument literal types. - * Only available on the sequential path (has AST); worker path passes undefined. */ -interface OverloadHints { +/** + * Optional hints for overload disambiguation via argument literal types. + * Only available on the sequential path (has AST); worker path passes undefined. + * + * @internal Exported so tests can exercise the D0 skip-condition path without + * constructing a real SyntaxNode. Do not use outside `call-processor.ts` + * and its unit tests. + */ +export interface OverloadHints { callNode: SyntaxNode; inferLiteralType: LiteralTypeInferrer; typeEnv?: TypeEnvironment; @@ -1279,6 +1285,31 @@ const tryOverloadDisambiguation = ( /** Per-file cache for the widen path's lookupFuzzy calls. Cleared between files. */ type WidenCache = Map; +/** @internal Exported for unit tests of D0 skip conditions (SM-11). Do not use outside tests. */ +export const _resolveCallTargetForTesting = ( + call: Pick< + ExtractedCall, + 'calledName' | 'argCount' | 'callForm' | 'receiverTypeName' | 'receiverName' + >, + currentFile: string, + ctx: ResolutionContext, + opts?: { + overloadHints?: OverloadHints; + widenCache?: WidenCache; + preComputedArgTypes?: (string | undefined)[]; + heritageMap?: HeritageMap; + }, +): ResolveResult | null => + resolveCallTarget( + call, + currentFile, + ctx, + opts?.overloadHints, + opts?.widenCache, + opts?.preComputedArgTypes, + opts?.heritageMap, + ); + const resolveCallTarget = ( call: Pick< ExtractedCall, @@ -1328,6 +1359,10 @@ const resolveCallTarget = ( // selects auth.py via moduleAliasMap. Runs for ALL member calls with a known module alias, // not just ambiguous ones — same-file tier may shadow the correct cross-module target when // the caller defines a function with the same name as the callee (Issue #417). + // + // Tracks `aliasNarrowed` so the D2 widening step below does NOT undo the alias filtering + // by calling lookupFuzzy again (which would re-introduce homonym candidates from other files). + let aliasNarrowed = false; if (call.callForm === 'member' && call.receiverName) { const aliasMap = ctx.moduleAliasMap?.get(currentFile); if (aliasMap) { @@ -1336,6 +1371,7 @@ const resolveCallTarget = ( const aliasFiltered = filteredCandidates.filter((c) => c.filePath === moduleFile); if (aliasFiltered.length > 0) { filteredCandidates = aliasFiltered; + aliasNarrowed = true; } else { // Same-file tier returned a local match, but the alias points elsewhere. // Widen to global candidates and filter to the aliased module's file. @@ -1350,7 +1386,10 @@ const resolveCallTarget = ( const widened = filterCallableCandidates(fuzzyDefs, call.argCount, call.callForm).filter( (c) => c.filePath === moduleFile, ); - if (widened.length > 0) filteredCandidates = widened; + if (widened.length > 0) { + filteredCandidates = widened; + aliasNarrowed = true; + } } } } @@ -1365,33 +1404,45 @@ const resolveCallTarget = ( // belong to the wrong class (e.g. super.save() should hit the parent's save, // not the child's own save method in the same file). if (call.callForm === 'member' && call.receiverTypeName) { - // D0. MRO fast path: when heritageMap is available, try owner-scoped + MRO - // lookup before falling back to the expensive D2 fuzzy widening. - // This short-circuits the lookupFuzzy call for every cross-file member call. + // D0. Delegate to resolveMemberCall (SM-11): owner-scoped + MRO lookup + // before falling back to the expensive D1-D4 fuzzy widening. // Skip conditions: // (a) overloadHints or preComputedArgTypes present — the MRO lookup may // pick the wrong overload for same-return-type overloads since it - // does not consider argument types. D2-D4+E handles those correctly. + // does not consider argument types. D1-D4+E handles those correctly. // (b) A module alias on call.receiverName is active for this file — the // alias block above already narrowed `filteredCandidates` to a - // specific file (e.g. Python `import auth; auth.user.save()`). - // resolveMethodByOwner re-resolves `receiverTypeName` from scratch - // via `ctx.resolve`, which ignores that narrowing and could pick a - // homonymous class from the wrong file. Fall through to D1-D4 which - // respects the alias-filtered candidate pool. - const hasActiveModuleAlias = - !!call.receiverName && ctx.moduleAliasMap?.get(currentFile)?.has(call.receiverName) === true; - if (!overloadHints && !preComputedArgTypes && !hasActiveModuleAlias) { - const mroResult = resolveMethodByOwner( + // specific file. resolveMemberCall re-resolves `receiverTypeName` + // from scratch via `ctx.resolve`, which ignores that narrowing and + // could pick a homonymous class from the wrong file. Fall through to + // D1-D4 which respects the alias-filtered candidate pool. + // D0 skip for overload disambiguation: only fires when the name actually + // has multiple candidates in the tiered pool. The sequential path sets + // `overloadHints` for every call regardless of whether the method is + // overloaded — skipping D0 unconditionally would make this fast path + // dead code for the sequential pipeline. By gating on + // `filteredCandidates.length > 1`, we preserve the original intent + // (let D1-D4+E pick the right overload when there are multiple) while + // allowing D0 to fire for the common single-candidate case. + const hasOverloadConcern = + (!!overloadHints || !!preComputedArgTypes) && filteredCandidates.length > 1; + // D0 skip for active module alias: only fires when the alias block above + // actually narrowed filteredCandidates. In Python, a local variable can + // shadow an imported module name (e.g. `from models.c import C; c = C()` + // creates both a module alias `c → models/c.py` AND a typed local `c`). + // Checking `aliasNarrowed` rather than `ctx.moduleAliasMap.has(receiverName)` + // ensures D0 still runs when the method isn't in the aliased module — + // which means the receiver is a typed local variable, not a module reference. + if (!hasOverloadConcern && !aliasNarrowed) { + const memberResult = resolveMemberCall( call.receiverTypeName, call.calledName, currentFile, ctx, heritageMap, + call.argCount, ); - if (mroResult) { - return toResolveResult(mroResult, tiered.tier); - } + if (memberResult) return memberResult; } // D1. Resolve the receiver type @@ -1403,8 +1454,13 @@ const resolveCallTarget = ( // D2. Widen candidates: same-file tier may miss the parent's method when // it lives in another file. Query the symbol table directly for all // global methods with this name, then apply arity/kind filtering. + // + // When the candidate set was already narrowed by module-alias + // disambiguation, do NOT widen back to the full fuzzy pool — that + // would undo the alias narrowing and reintroduce homonym candidates + // from other files. const methodPool = - filteredCandidates.length <= 1 + filteredCandidates.length <= 1 && !aliasNarrowed ? filterCallableCandidates( ctx.symbols.lookupFuzzy(call.calledName), call.argCount, @@ -1435,6 +1491,23 @@ const resolveCallTarget = ( if (disambiguated) return toResolveResult(disambiguated, tiered.tier); return null; } + + // Zero-match null-route: we committed to receiver narrowing (D1 succeeded) + // but both file-based (D3) and owner-based (D4) filters produced zero + // matches. The lone candidate in `filteredCandidates` does not belong to + // this receiver type — refuse to emit a CALLS edge rather than fall + // through to the permissive single-candidate tail return. + // + // Addresses Codex review finding R3 (PR #744): member calls where + // fuzzy fallback picked a globally-matching symbol that has no + // relationship to the receiver's class hierarchy were silently + // producing false-positive edges. Example: Rust `c.trait_only()` where + // `trait_only` is captured as a Function node with no ownerId — it + // matches the name but fails both file and owner narrowing, so the + // old tail return would pick it incorrectly. + if (fileFiltered.length === 0 && ownerFiltered.length === 0) { + return null; + } } } @@ -1630,9 +1703,33 @@ const resolveFieldOwnership = ( /** * Resolve a method by owner type name using the eagerly-populated methodByOwner index. - * Returns the SymbolDefinition if an unambiguous method is found, undefined otherwise. - * Falls through to undefined for: unknown type, no class-like candidates, ambiguous overloads. - * When heritageMap is provided, falls back to MRO-aware parent chain walking. + * Returns `{ def, tier }` when an unambiguous method is found, `undefined` otherwise. + * + * **Multi-candidate iteration (homonym disambiguation):** when `ctx.resolve(ownerType)` + * returns multiple class-like candidates (e.g. two classes named `User` in different + * files reachable from the call site), each is probed with `lookupMethodByOwnerWithMRO`. + * Results are deduplicated by `nodeId` so that: + * + * - homonym classes that both walk up to the SAME ancestor's method collapse to 1 hit + * - aliased re-exports that produce two candidates pointing at the same def collapse too + * + * After deduplication: + * + * - 0 unique matches → `undefined` (owner-scoped path has no answer; D1-D4 fuzzy + * fallback in `resolveCallTarget` may still find something via lookupFuzzy) + * - 1 unique match → return it + * - ≥2 unique matches → `undefined` (genuine homonym ambiguity; don't silently pick one) + * + * This absorbs what was previously D4's job inside `resolveCallTarget` — "filter fuzzy + * candidates to those whose ownerId is in the receiver type's nodeId set" — into the + * owner-scoped path, aligning with the plan's target: + * + * `resolveCallTarget` D2 widening → `model.lookupMethodWithMRO(ownerNodeId, name)` + * + * The returned `tier` reflects how the owner TYPE was resolved (not the method name). + * Threaded out here so callers don't need a second `ctx.resolve(ownerType, ...)` call — + * this decouples callers from `ctx.resolve`'s per-file caching contract, which SM-16 + * will restructure when it replaces the `lookupFuzzy` data source. */ const resolveMethodByOwner = ( receiverTypeName: string, @@ -1640,35 +1737,110 @@ const resolveMethodByOwner = ( filePath: string, ctx: ResolutionContext, heritageMap?: HeritageMap, -): SymbolDefinition | undefined => { + argCount?: number, +): { def: SymbolDefinition; tier: ResolutionTier } | undefined => { const typeResolved = ctx.resolve(receiverTypeName, filePath); if (!typeResolved) return undefined; - const classDef = typeResolved.candidates.find((d) => CLASS_LIKE_TYPES.has(d.type)); - if (!classDef) return undefined; - // When HeritageMap is available, delegate to MRO-aware lookup which performs - // the direct owner lookup itself before walking ancestors — avoids a double - // direct lookup on the hot path. - if (heritageMap) { - const language = getLanguageFromFilename(filePath); - if (language) { - return lookupMethodByOwnerWithMRO( - classDef.nodeId, - methodName, - heritageMap, - ctx.symbols, - language, - ); + // MRO walking needs a language hint; compute once and reuse for every candidate. + // Unknown extension → fall back to plain direct lookup (D1-D4 still runs on miss). + const language = heritageMap ? getLanguageFromFilename(filePath) : null; + const canWalkMRO = heritageMap != null && language != null; + + // Iterate all class-like candidates tracking the first unambiguous hit. + // Zero-allocation fast path: the common case is exactly one class candidate, + // so we avoid building a Map. A second hit with a different `nodeId` flips + // `ambiguous` and short-circuits the loop. Diamond MRO convergence on the + // same inherited method collapses to one hit because `nodeId` matches. + // + // firstDef === undefined → owner-scoped resolution found nothing + // firstDef && !ambiguous → unambiguous answer + // ambiguous → genuine homonym ambiguity — refuse to pick + // + // argCount is threaded through so arity-differing overloads + // (e.g. C++ `greet()` vs `greet(string)`) are disambiguated inside the + // owner-scoped lookup rather than collapsing to an arbitrary first pick. + let firstDef: SymbolDefinition | undefined; + let ambiguous = false; + for (const candidate of typeResolved.candidates) { + if (!CLASS_LIKE_TYPES.has(candidate.type)) continue; + const def = canWalkMRO + ? lookupMethodByOwnerWithMRO( + candidate.nodeId, + methodName, + heritageMap, + ctx.symbols, + language, + argCount, + ) + : ctx.symbols.lookupMethodByOwner(candidate.nodeId, methodName, argCount); + if (!def) continue; + if (!firstDef) { + firstDef = def; + } else if (def.nodeId !== firstDef.nodeId) { + ambiguous = true; + break; } } - // Fallback when no HeritageMap (or the file extension is unrecognized by - // `getLanguageFromFilename`, e.g. a synthetic path or an extension that is - // not registered in supported-languages.ts): plain direct lookup with no - // ancestor walk. All primary languages register their extensions, so this - // branch is only reached for edge cases where the MRO walk would not be - // applicable anyway. D1-D4 in resolveCallTarget still runs on D0 miss. - return ctx.symbols.lookupMethodByOwner(classDef.nodeId, methodName); + if (!firstDef || ambiguous) return undefined; + return { def: firstDef, tier: typeResolved.tier }; +}; + +// --------------------------------------------------------------------------- +// SM-11: Owner-scoped + MRO member-call resolution (no fuzzy lookup) +// --------------------------------------------------------------------------- + +/** + * Resolve a member call using owner-scoped + MRO resolution only (no fuzzy lookup). + * Used for `obj.method()` calls where the receiver type is known. + * + * Delegates to {@link resolveMethodByOwner} which performs an O(1) owner-scoped + * method lookup and, when a {@link HeritageMap} is provided, walks the MRO chain + * via {@link lookupMethodByOwnerWithMRO}. + * + * {@link resolveCallTarget} delegates here for member calls before falling back + * to the more expensive fuzzy-widening path (D1-D4). + * + * **SEMANTIC CHANGE (2026-04-09):** The confidence tier now reflects how the + * owner TYPE was resolved, not how the method NAME was resolved globally. The + * previous D0 fast path in `resolveCallTarget` used `tiered.tier` from + * `ctx.resolve(calledName, ...)` — a name-based tier that matched what D1-D4 + * fuzzy widening would produce. The new tier is owner-type-based, which is + * more accurate for owner-scoped resolution (the discriminant IS the class, + * not the method name). Downstream consumers that filter CALLS edges by + * confidence threshold may see shifted values on otherwise-unchanged code. + * See the "returns result with correct confidence tier" tests below for the + * locked-in behavior. + * + * **Performance:** Callers that only need the return type (e.g. `walkMixedChain`) + * should call {@link resolveMethodByOwner} directly and use the `.def.returnType` + * field instead, to avoid building a throwaway `ResolveResult`. + * + * @param ownerType - The receiver's type name (e.g. 'User') + * @param methodName - The method being called (e.g. 'save') + * @param currentFile - File path of the call site + * @param ctx - Resolution context + * @param heritageMap - Optional heritage map for MRO-aware ancestor walking + */ +export const resolveMemberCall = ( + ownerType: string, + methodName: string, + currentFile: string, + ctx: ResolutionContext, + heritageMap?: HeritageMap, + argCount?: number, +): ResolveResult | null => { + const resolved = resolveMethodByOwner( + ownerType, + methodName, + currentFile, + ctx, + heritageMap, + argCount, + ); + if (!resolved) return null; + return toResolveResult(resolved.def, resolved.tier); }; // --------------------------------------------------------------------------- @@ -1761,9 +1933,12 @@ export const lookupMethodByOwnerWithMRO = ( heritageMap: HeritageMap, symbols: SymbolTable, language: SupportedLanguages, + argCount?: number, ): SymbolDefinition | undefined => { - // Direct lookup first (child override — no walk needed) - const direct = symbols.lookupMethodByOwner(ownerNodeId, methodName); + // Direct lookup first (child override — no walk needed). + // argCount is threaded through so arity-differing overloads on the direct + // owner can be disambiguated before the MRO walk starts. + const direct = symbols.lookupMethodByOwner(ownerNodeId, methodName, argCount); if (direct) return direct; const strategy = getProvider(language).mroStrategy; @@ -1790,9 +1965,10 @@ export const lookupMethodByOwnerWithMRO = ( ancestors = heritageMap.getAncestors(ownerNodeId); } - // Walk ancestors in MRO order — first match wins + // Walk ancestors in MRO order — first match wins. + // argCount narrows overloaded ancestors the same way as the direct lookup. for (const ancestorId of ancestors) { - const method = symbols.lookupMethodByOwner(ancestorId, methodName); + const method = symbols.lookupMethodByOwner(ancestorId, methodName, argCount); if (method) return method; } @@ -1861,12 +2037,16 @@ const walkMixedChain = ( continue; } // Fast path: O(1) owner-scoped method lookup via methodByOwner index. - // Avoids fuzzy lookup when the owner type is known and the method is unambiguous. // Note: CALLS edges for intermediate chain steps are NOT emitted here — walkMixedChain // only threads types. CALLS edges come from the outer per-call-expression loop in processCalls. - const methodDef = resolveMethodByOwner(currentType, step.name, filePath, ctx, heritageMap); - if (methodDef?.returnType) { - const fastRetType = extractReturnTypeName(methodDef.returnType); + // + // We call `resolveMethodByOwner` directly (NOT `resolveMemberCall`) because this is + // a hot path — called per chain step per call expression — and we only need the + // return type string. Going through `resolveMemberCall` would allocate a throwaway + // `ResolveResult` with confidence/reason that we immediately discard. + const owned = resolveMethodByOwner(currentType, step.name, filePath, ctx, heritageMap); + if (owned?.def.returnType) { + const fastRetType = extractReturnTypeName(owned.def.returnType); if (fastRetType) { currentType = fastRetType; continue; diff --git a/gitnexus/src/core/ingestion/symbol-table.ts b/gitnexus/src/core/ingestion/symbol-table.ts index 086148f2f..b8d39170c 100644 --- a/gitnexus/src/core/ingestion/symbol-table.ts +++ b/gitnexus/src/core/ingestion/symbol-table.ts @@ -1,6 +1,18 @@ import type { NodeLabel } from 'gitnexus-shared'; -export const CLASS_TYPES = new Set(['Class', 'Struct', 'Interface', 'Enum', 'Record']); +export const CLASS_TYPES = new Set([ + 'Class', + 'Struct', + 'Interface', + 'Enum', + 'Record', + // Traits are class-like for heritage resolution: PHP `use Trait;`, Rust + // `impl Trait for Struct`, and Scala traits all contribute methods to the + // hierarchy of their using/implementing type. Including Trait here lets + // buildHeritageMap resolve `h.parentName` to a Trait nodeId so the MRO + // walker can visit the trait and find its methods. + 'Trait', +]); export interface SymbolDefinition { nodeId: string; @@ -93,7 +105,24 @@ export interface SymbolTable { * overloads share the same returnType, undefined when return types differ (ambiguous). * Used by walkMixedChain for deterministic cross-class chain resolution. */ - lookupMethodByOwner: (ownerNodeId: string, methodName: string) => SymbolDefinition | undefined; + /** + * Lookup a method by owner class + name, optionally filtered by arity. + * + * When `argCount` is provided, overloads whose parameter count doesn't + * accommodate the call's argument count are filtered out before the + * returnType dedup runs. This lets D0 (`resolveMemberCall`) disambiguate + * arity-differing overloads (e.g. C++ `greet()` vs `greet(string)`) that + * would otherwise collide on the shared `ownerId + methodName` key. + * + * Same-arity, same-returnType overloads (e.g. `save(int)` vs `save(String)`, + * both returning `void`) still collapse to the first match — callers must + * gate D0 on overload concern before invoking this function for that case. + */ + lookupMethodByOwner: ( + ownerNodeId: string, + methodName: string, + argCount?: number, + ) => SymbolDefinition | undefined; /** * Look up class-like definitions (Class, Struct, Interface, Enum, Record) by name. @@ -225,9 +254,16 @@ export const createSymbolTable = (): SymbolTable => { } globalIndex.get(name)!.push(def); - // C2. Methods and constructors with ownerId go to methodByOwner index - // (in addition to globalIndex). - if ((type === 'Method' || type === 'Constructor') && metadata?.ownerId) { + // C2. Methods, constructors, and ownerId-bound Functions go to + // methodByOwner index (in addition to globalIndex). + // + // Some language extractors emit class methods as `Function` with an + // `ownerId` — notably Python (`def method(self):` inside a class body), + // Rust trait methods, and Kotlin object/companion methods. Treating + // `Function` with ownerId the same as `Method` here makes D0 + // (`resolveMemberCall`) work uniformly across all supported languages + // instead of silently falling through to D1-D4 fuzzy widening. + if ((type === 'Method' || type === 'Constructor' || type === 'Function') && metadata?.ownerId) { const key = `${metadata.ownerId}\0${name}`; const existing = methodByOwner.get(key); if (existing) { @@ -303,18 +339,42 @@ export const createSymbolTable = (): SymbolTable => { const lookupMethodByOwner = ( ownerNodeId: string, methodName: string, + argCount?: number, ): SymbolDefinition | undefined => { const defs = methodByOwner.get(`${ownerNodeId}\0${methodName}`); if (!defs || defs.length === 0) return undefined; - if (defs.length === 1) return defs[0]; - // Multiple overloads: return first if all share the same defined returnType (safe for chain resolution). - // Return undefined if return types differ or are absent (truly ambiguous — can't determine which overload). - const firstReturnType = defs[0].returnType; - if (firstReturnType === undefined) return undefined; - for (let i = 1; i < defs.length; i++) { - if (defs[i].returnType !== firstReturnType) return undefined; + + // Arity narrowing: when an argCount is provided and there are multiple + // overloads, keep only those whose parameterCount can accommodate the + // call. This resolves arity-differing overloads (e.g. C++ `greet()` vs + // `greet(string)`) that share the same `ownerId + methodName` key. + // + // Candidates with `parameterCount === undefined` (extractor didn't + // populate the count — typically variadic or unknown) are retained + // conservatively so that legitimate variadic matches still resolve. + let pool = defs; + if (argCount !== undefined && defs.length > 1) { + const arityMatched = defs.filter((d) => { + if (d.parameterCount === undefined) return true; + const min = d.requiredParameterCount ?? d.parameterCount; + return argCount >= min && argCount <= d.parameterCount; + }); + // Only adopt the arity-narrowed pool when it found matches; if arity + // rules out every candidate, fall back to the unfiltered set so the + // caller's fuzzy path still has something to work with. + if (arityMatched.length > 0) pool = arityMatched; } - return defs[0]; + + if (pool.length === 1) return pool[0]; + // Multiple overloads after arity narrowing: return first if all share + // the same defined returnType (safe for chain resolution), undefined if + // return types differ (truly ambiguous — can't determine which overload). + const firstReturnType = pool[0].returnType; + if (firstReturnType === undefined) return undefined; + for (let i = 1; i < pool.length; i++) { + if (pool[i].returnType !== firstReturnType) return undefined; + } + return pool[0]; }; const lookupClassByName = (name: string): SymbolDefinition[] => { diff --git a/gitnexus/test/fixtures/lang-resolution/cpp-diamond-inheritance/src/A.h b/gitnexus/test/fixtures/lang-resolution/cpp-diamond-inheritance/src/A.h new file mode 100644 index 000000000..314c3556f --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/cpp-diamond-inheritance/src/A.h @@ -0,0 +1,10 @@ +#pragma once +#include "Base.h" + +// Virtual inheritance: together with `B : virtual public Base`, this creates +// a single shared `Base` subobject under `Derived`, so `d.method()` is an +// unambiguous call in real C++. Without the `virtual` keyword, a non-virtual +// diamond would produce two separate `Base` subobjects and the call would +// be ambiguous, requiring `d.A::method()` or `d.B::method()` to disambiguate. +class A : virtual public Base { +}; diff --git a/gitnexus/test/fixtures/lang-resolution/cpp-diamond-inheritance/src/B.h b/gitnexus/test/fixtures/lang-resolution/cpp-diamond-inheritance/src/B.h new file mode 100644 index 000000000..7a9b90fff --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/cpp-diamond-inheritance/src/B.h @@ -0,0 +1,7 @@ +#pragma once +#include "Base.h" + +// See the comment in A.h — both sides of the diamond use virtual inheritance +// so there is exactly one `Base` subobject under `Derived`. +class B : virtual public Base { +}; diff --git a/gitnexus/test/fixtures/lang-resolution/cpp-diamond-inheritance/src/Base.h b/gitnexus/test/fixtures/lang-resolution/cpp-diamond-inheritance/src/Base.h new file mode 100644 index 000000000..2b8382711 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/cpp-diamond-inheritance/src/Base.h @@ -0,0 +1,6 @@ +#pragma once + +class Base { +public: + int method() { return 42; } +}; diff --git a/gitnexus/test/fixtures/lang-resolution/cpp-diamond-inheritance/src/Derived.h b/gitnexus/test/fixtures/lang-resolution/cpp-diamond-inheritance/src/Derived.h new file mode 100644 index 000000000..2b6e89b46 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/cpp-diamond-inheritance/src/Derived.h @@ -0,0 +1,6 @@ +#pragma once +#include "A.h" +#include "B.h" + +class Derived : public A, public B { +}; diff --git a/gitnexus/test/fixtures/lang-resolution/cpp-diamond-inheritance/src/app.cpp b/gitnexus/test/fixtures/lang-resolution/cpp-diamond-inheritance/src/app.cpp new file mode 100644 index 000000000..8bc9b4969 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/cpp-diamond-inheritance/src/app.cpp @@ -0,0 +1,6 @@ +#include "Derived.h" + +void run() { + Derived d; + d.method(); +} diff --git a/gitnexus/test/fixtures/lang-resolution/csharp-interface-default-method/App.cs b/gitnexus/test/fixtures/lang-resolution/csharp-interface-default-method/App.cs new file mode 100644 index 000000000..2e516fd93 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/csharp-interface-default-method/App.cs @@ -0,0 +1,15 @@ +namespace InterfaceDefault; + +public class App +{ + public static void Run() + { + // Default interface methods in C# 8.0+ are reachable ONLY through + // the interface type, not as inherited class members. Declaring the + // variable as IValidator is the idiomatic way to invoke Validate(). + // `User user = new User(...); user.Validate();` would be a compile + // error because User does not expose Validate as a class member. + IValidator user = new User("alice"); + user.Validate(); + } +} diff --git a/gitnexus/test/fixtures/lang-resolution/csharp-interface-default-method/User.cs b/gitnexus/test/fixtures/lang-resolution/csharp-interface-default-method/User.cs new file mode 100644 index 000000000..3d380a83f --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/csharp-interface-default-method/User.cs @@ -0,0 +1,11 @@ +namespace InterfaceDefault; + +public class User : IValidator +{ + public string Name { get; } + + public User(string name) + { + Name = name; + } +} diff --git a/gitnexus/test/fixtures/lang-resolution/csharp-interface-default-method/Validator.cs b/gitnexus/test/fixtures/lang-resolution/csharp-interface-default-method/Validator.cs new file mode 100644 index 000000000..620e0a207 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/csharp-interface-default-method/Validator.cs @@ -0,0 +1,6 @@ +namespace InterfaceDefault; + +public interface IValidator +{ + bool Validate() => true; +} diff --git a/gitnexus/test/fixtures/lang-resolution/java-interface-default-method/App.java b/gitnexus/test/fixtures/lang-resolution/java-interface-default-method/App.java new file mode 100644 index 000000000..57564f7d2 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/java-interface-default-method/App.java @@ -0,0 +1,6 @@ +public class App { + public static void run() { + User user = new User("alice"); + user.validate(); + } +} diff --git a/gitnexus/test/fixtures/lang-resolution/java-interface-default-method/User.java b/gitnexus/test/fixtures/lang-resolution/java-interface-default-method/User.java new file mode 100644 index 000000000..ceab78b1c --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/java-interface-default-method/User.java @@ -0,0 +1,7 @@ +public class User implements Validator { + private String name; + + public User(String name) { + this.name = name; + } +} diff --git a/gitnexus/test/fixtures/lang-resolution/java-interface-default-method/Validator.java b/gitnexus/test/fixtures/lang-resolution/java-interface-default-method/Validator.java new file mode 100644 index 000000000..40796bc53 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/java-interface-default-method/Validator.java @@ -0,0 +1,5 @@ +public interface Validator { + default boolean validate() { + return true; + } +} diff --git a/gitnexus/test/fixtures/lang-resolution/kotlin-interface-default-method/src/App.kt b/gitnexus/test/fixtures/lang-resolution/kotlin-interface-default-method/src/App.kt new file mode 100644 index 000000000..42e0897c7 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/kotlin-interface-default-method/src/App.kt @@ -0,0 +1,6 @@ +package example + +fun run() { + val user = User("alice") + user.validate() +} diff --git a/gitnexus/test/fixtures/lang-resolution/kotlin-interface-default-method/src/User.kt b/gitnexus/test/fixtures/lang-resolution/kotlin-interface-default-method/src/User.kt new file mode 100644 index 000000000..279e5a6fa --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/kotlin-interface-default-method/src/User.kt @@ -0,0 +1,3 @@ +package example + +class User(val name: String) : Validator diff --git a/gitnexus/test/fixtures/lang-resolution/kotlin-interface-default-method/src/Validator.kt b/gitnexus/test/fixtures/lang-resolution/kotlin-interface-default-method/src/Validator.kt new file mode 100644 index 000000000..7ddd1631f --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/kotlin-interface-default-method/src/Validator.kt @@ -0,0 +1,5 @@ +package example + +interface Validator { + fun validate(): Boolean = true +} diff --git a/gitnexus/test/fixtures/lang-resolution/python-multi-level-mro/app.py b/gitnexus/test/fixtures/lang-resolution/python-multi-level-mro/app.py new file mode 100644 index 000000000..30e1be47a --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/python-multi-level-mro/app.py @@ -0,0 +1,6 @@ +from child import Child + + +def run() -> None: + c = Child() + c.gp_method() diff --git a/gitnexus/test/fixtures/lang-resolution/python-multi-level-mro/child.py b/gitnexus/test/fixtures/lang-resolution/python-multi-level-mro/child.py new file mode 100644 index 000000000..be1852fa6 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/python-multi-level-mro/child.py @@ -0,0 +1,5 @@ +from parent import Parent + + +class Child(Parent): + pass diff --git a/gitnexus/test/fixtures/lang-resolution/python-multi-level-mro/grandparent.py b/gitnexus/test/fixtures/lang-resolution/python-multi-level-mro/grandparent.py new file mode 100644 index 000000000..675d7a412 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/python-multi-level-mro/grandparent.py @@ -0,0 +1,3 @@ +class Grandparent: + def gp_method(self) -> str: + return "grandparent" diff --git a/gitnexus/test/fixtures/lang-resolution/python-multi-level-mro/parent.py b/gitnexus/test/fixtures/lang-resolution/python-multi-level-mro/parent.py new file mode 100644 index 000000000..6a24add39 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/python-multi-level-mro/parent.py @@ -0,0 +1,5 @@ +from grandparent import Grandparent + + +class Parent(Grandparent): + pass diff --git a/gitnexus/test/fixtures/lang-resolution/rust-child-extends-parent/src/child.rs b/gitnexus/test/fixtures/lang-resolution/rust-child-extends-parent/src/child.rs new file mode 100644 index 000000000..1e9755c90 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/rust-child-extends-parent/src/child.rs @@ -0,0 +1,16 @@ +use crate::parent::Parent; + +pub struct Child; + +impl Child { + // Direct impl method — MUST resolve via resolveMemberCall owner-scoped path. + pub fn own_method(&self) -> &str { + "child-own" + } +} + +// Trait implementation — `trait_only` is provided by the trait's default impl +// but is NOT reachable via direct `obj.trait_only()` in Rust without the trait +// being in scope. The resolver correctly treats qualified-syntax MRO as opaque +// to direct member calls. +impl Parent for Child {} diff --git a/gitnexus/test/fixtures/lang-resolution/rust-child-extends-parent/src/main.rs b/gitnexus/test/fixtures/lang-resolution/rust-child-extends-parent/src/main.rs new file mode 100644 index 000000000..bac6fd160 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/rust-child-extends-parent/src/main.rs @@ -0,0 +1,17 @@ +mod child; +mod parent; + +use crate::child::Child; + +fn run() { + let c = Child; + // Direct impl method — SHOULD resolve to Child::own_method. + c.own_method(); + // Trait-inherited default — direct member-call SHOULD NOT resolve to + // Parent::trait_only under Rust's qualified-syntax MRO strategy. + c.trait_only(); +} + +fn main() { + run(); +} diff --git a/gitnexus/test/fixtures/lang-resolution/rust-child-extends-parent/src/parent.rs b/gitnexus/test/fixtures/lang-resolution/rust-child-extends-parent/src/parent.rs new file mode 100644 index 000000000..e14a6f86c --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/rust-child-extends-parent/src/parent.rs @@ -0,0 +1,11 @@ +// Trait "parent" — methods on a Rust trait are NOT reachable via direct +// `obj.method()` syntax on structs that implement the trait unless the trait +// itself is in scope. Our qualified-syntax MRO strategy reflects this: direct +// member calls do NOT walk trait ancestry, so `c.trait_only()` below should +// produce NO CALLS edge to `Parent::trait_only`. + +pub trait Parent { + fn trait_only(&self) -> &str { + "parent-default" + } +} diff --git a/gitnexus/test/integration/resolvers/cpp.test.ts b/gitnexus/test/integration/resolvers/cpp.test.ts index 630c86696..0839d59f0 100644 --- a/gitnexus/test/integration/resolvers/cpp.test.ts +++ b/gitnexus/test/integration/resolvers/cpp.test.ts @@ -1548,3 +1548,37 @@ describe('C++ Child extends Parent — inherited method resolution (SM-9)', () = expect(parentMethodCall!.source).toBe('run'); }); }); + +describe('C++ Derived : A, B — diamond inheritance via leftmost-base MRO (SM-11)', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo(path.join(FIXTURES, 'cpp-diamond-inheritance'), () => {}); + }, 60000); + + it('detects Base, A, B, and Derived classes', () => { + const classes = getNodesByLabel(result, 'Class'); + expect(classes).toContain('Base'); + expect(classes).toContain('A'); + expect(classes).toContain('B'); + expect(classes).toContain('Derived'); + }); + + it('emits EXTENDS edges for both branches: A → Base, B → Base, Derived → A, Derived → B', () => { + const extends_ = getRelationships(result, 'EXTENDS'); + const edges = edgeSet(extends_); + expect(edges).toContain('A → Base'); + expect(edges).toContain('B → Base'); + expect(edges).toContain('Derived → A'); + expect(edges).toContain('Derived → B'); + }); + + it('resolves d.method() to Base::method via leftmost-base MRO walk', () => { + const calls = getRelationships(result, 'CALLS'); + const methodCall = calls.find( + (c) => c.target === 'method' && c.targetFilePath.includes('Base.h'), + ); + expect(methodCall).toBeDefined(); + expect(methodCall!.source).toBe('run'); + }); +}); diff --git a/gitnexus/test/integration/resolvers/csharp.test.ts b/gitnexus/test/integration/resolvers/csharp.test.ts index 5db630bf2..af2dbb786 100644 --- a/gitnexus/test/integration/resolvers/csharp.test.ts +++ b/gitnexus/test/integration/resolvers/csharp.test.ts @@ -1963,3 +1963,37 @@ describe('C# Child extends Parent — inherited method resolution (SM-9)', () => expect(parentMethodCall!.source).toBe('Run'); }); }); + +// --------------------------------------------------------------------------- +// SM-11: C# User : IValidator — interface default method via implements-split +// --------------------------------------------------------------------------- + +describe('C# User implements IValidator — interface default method (SM-11)', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo( + path.join(FIXTURES, 'csharp-interface-default-method'), + () => {}, + ); + }, 60000); + + it('detects IValidator interface and User class', () => { + expect(getNodesByLabel(result, 'Interface')).toContain('IValidator'); + expect(getNodesByLabel(result, 'Class')).toContain('User'); + }); + + it('emits IMPLEMENTS edge: User → IValidator', () => { + const impls = getRelationships(result, 'IMPLEMENTS'); + expect(edgeSet(impls)).toContain('User → IValidator'); + }); + + it('resolves user.Validate() to IValidator.Validate via implements-split MRO', () => { + const calls = getRelationships(result, 'CALLS'); + const validateCall = calls.find( + (c) => c.target === 'Validate' && c.targetFilePath.includes('Validator.cs'), + ); + expect(validateCall).toBeDefined(); + expect(validateCall!.source).toBe('Run'); + }); +}); diff --git a/gitnexus/test/integration/resolvers/java.test.ts b/gitnexus/test/integration/resolvers/java.test.ts index 8a09030ed..d9bae90bf 100644 --- a/gitnexus/test/integration/resolvers/java.test.ts +++ b/gitnexus/test/integration/resolvers/java.test.ts @@ -2133,3 +2133,37 @@ describe('Java Child extends Parent — inherited method resolution (SM-9)', () expect(parentMethodCall!.source).toBe('run'); }); }); + +// --------------------------------------------------------------------------- +// SM-11: Java User implements Validator — interface default method (Java 8+) +// --------------------------------------------------------------------------- + +describe('Java User implements Validator — interface default method (SM-11)', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo( + path.join(FIXTURES, 'java-interface-default-method'), + () => {}, + ); + }, 60000); + + it('detects Validator interface and User class', () => { + expect(getNodesByLabel(result, 'Interface')).toContain('Validator'); + expect(getNodesByLabel(result, 'Class')).toContain('User'); + }); + + it('emits IMPLEMENTS edge: User → Validator', () => { + const impls = getRelationships(result, 'IMPLEMENTS'); + expect(edgeSet(impls)).toContain('User → Validator'); + }); + + it('resolves user.validate() to Validator.validate via implements-split MRO', () => { + const calls = getRelationships(result, 'CALLS'); + const validateCall = calls.find( + (c) => c.target === 'validate' && c.targetFilePath.includes('Validator.java'), + ); + expect(validateCall).toBeDefined(); + expect(validateCall!.source).toBe('run'); + }); +}); diff --git a/gitnexus/test/integration/resolvers/kotlin.test.ts b/gitnexus/test/integration/resolvers/kotlin.test.ts index 1163538e4..056a2514a 100644 --- a/gitnexus/test/integration/resolvers/kotlin.test.ts +++ b/gitnexus/test/integration/resolvers/kotlin.test.ts @@ -2026,3 +2026,37 @@ describe('Kotlin Child extends Parent — inherited method resolution (SM-9)', ( expect(parentMethodCall!.source).toBe('run'); }); }); + +// --------------------------------------------------------------------------- +// SM-11: Kotlin User : Validator — interface default method via implements-split +// --------------------------------------------------------------------------- + +describe('Kotlin User implements Validator — interface default method (SM-11)', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo( + path.join(FIXTURES, 'kotlin-interface-default-method'), + () => {}, + ); + }, 60000); + + it('detects Validator interface and User class', () => { + expect(getNodesByLabel(result, 'Interface')).toContain('Validator'); + expect(getNodesByLabel(result, 'Class')).toContain('User'); + }); + + it('emits IMPLEMENTS edge: User → Validator', () => { + const impls = getRelationships(result, 'IMPLEMENTS'); + expect(edgeSet(impls)).toContain('User → Validator'); + }); + + it('resolves user.validate() to Validator.validate via implements-split MRO', () => { + const calls = getRelationships(result, 'CALLS'); + const validateCall = calls.find( + (c) => c.target === 'validate' && c.targetFilePath.includes('Validator.kt'), + ); + expect(validateCall).toBeDefined(); + expect(validateCall!.source).toBe('run'); + }); +}); diff --git a/gitnexus/test/integration/resolvers/python.test.ts b/gitnexus/test/integration/resolvers/python.test.ts index f2219463e..b7d60b20e 100644 --- a/gitnexus/test/integration/resolvers/python.test.ts +++ b/gitnexus/test/integration/resolvers/python.test.ts @@ -2146,3 +2146,33 @@ describe('Python Child extends Parent — inherited method resolution (SM-9)', ( expect(parentMethodCall!.source).toBe('run'); }); }); + +describe('Python Grandchild→Child→Parent — 3-level C3 MRO walk (SM-11)', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo(path.join(FIXTURES, 'python-multi-level-mro'), () => {}); + }, 60000); + + it('detects Grandparent, Parent, and Child classes', () => { + const classes = getNodesByLabel(result, 'Class'); + expect(classes).toContain('Grandparent'); + expect(classes).toContain('Parent'); + expect(classes).toContain('Child'); + }); + + it('emits EXTENDS chain: Child → Parent, Parent → Grandparent', () => { + const extends_ = getRelationships(result, 'EXTENDS'); + expect(edgeSet(extends_)).toContain('Child → Parent'); + expect(edgeSet(extends_)).toContain('Parent → Grandparent'); + }); + + it('resolves c.gp_method() to Grandparent.gp_method via 3-level C3 walk', () => { + const calls = getRelationships(result, 'CALLS'); + const gpCall = calls.find( + (c) => c.target === 'gp_method' && c.targetFilePath.includes('grandparent.py'), + ); + expect(gpCall).toBeDefined(); + expect(gpCall!.source).toBe('run'); + }); +}); diff --git a/gitnexus/test/integration/resolvers/rust.test.ts b/gitnexus/test/integration/resolvers/rust.test.ts index 496f15e9b..5e03da39b 100644 --- a/gitnexus/test/integration/resolvers/rust.test.ts +++ b/gitnexus/test/integration/resolvers/rust.test.ts @@ -1857,3 +1857,66 @@ describe('Rust abstract dispatch (Repository trait)', () => { expect(names).toEqual(['find', 'save']); }); }); + +// --------------------------------------------------------------------------- +// SM-11: Rust Child extends Parent — qualified-syntax MRO +// +// Companion integration test for the unit-level Rust qualified-syntax tests +// in symbol-table.test.ts. Validates end-to-end that: +// +// 1. Direct `impl` methods on a struct resolve through the D0 owner-scoped +// path (`resolveMemberCall`) — the positive control. +// +// 2. Trait-inherited default methods are NOT reachable via direct +// `obj.trait_method()` syntax. Rust requires the trait to be in scope +// and uses qualified syntax for trait dispatch; the resolver correctly +// treats direct member calls as opaque to trait ancestry. +// +// Previously this case emitted a false-positive CALLS edge via the +// permissive tail-return in resolveCallTarget — Codex review finding +// R3 (PR #744). The tail-return is now null-routed when D1-D4 receiver +// filtering produces zero matches on both file and owner dimensions. +// --------------------------------------------------------------------------- + +describe('Rust Child extends Parent — qualified-syntax MRO (SM-11)', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo(path.join(FIXTURES, 'rust-child-extends-parent'), () => {}); + }, 60000); + + it('detects Child struct and Parent trait', () => { + const structs = getNodesByLabel(result, 'Struct'); + expect(structs).toContain('Child'); + const traits = getNodesByLabel(result, 'Trait'); + expect(traits).toContain('Parent'); + }); + + it('resolves c.own_method() to Child::own_method via D0 owner-scoped path', () => { + // Direct impl method — D0 short-circuits to lookupMethodByOwner which + // returns Child::own_method without falling through to D1-D4 fuzzy. + const calls = getRelationships(result, 'CALLS'); + const ownCall = calls.find( + (c) => + c.target === 'own_method' && c.source === 'run' && c.targetFilePath.includes('child.rs'), + ); + expect(ownCall).toBeDefined(); + }); + + it('does NOT resolve c.trait_only() to Parent::trait_only via direct member call', () => { + // Qualified-syntax MRO: direct member calls on structs do not walk trait + // ancestry. `c.trait_only()` must null-route because `trait_only` is + // defined on the trait, not on the Child struct. + // + // The resolveCallTarget tail-return tightening (R3) is what makes this + // assertion testable: before the fix, resolveCallTarget would fall + // through D1-D4 (zero file matches, zero owner matches) and silently + // pick the single fuzzy candidate as a false-positive edge. + const calls = getRelationships(result, 'CALLS'); + const traitCall = calls.find( + (c) => + c.target === 'trait_only' && c.source === 'run' && c.targetFilePath.includes('parent.rs'), + ); + expect(traitCall).toBeUndefined(); + }); +}); diff --git a/gitnexus/test/unit/call-processor.test.ts b/gitnexus/test/unit/call-processor.test.ts index 81dc9f042..b0c2b04eb 100644 --- a/gitnexus/test/unit/call-processor.test.ts +++ b/gitnexus/test/unit/call-processor.test.ts @@ -1716,7 +1716,24 @@ describe('processCalls — D0 MRO fast path (SM-10)', () => { expect(doWorkCalls).toHaveLength(1); }); - it('D0 skipped: same scenario still resolves via D1-D4 when heritageMap is undefined', async () => { + it('no heritageMap: inherited methods are unresolvable (null-routed, not false-positive)', async () => { + // Without a HeritageMap, the resolver cannot know that Parent.parentMethod + // belongs to Child's ancestry. The old D1-D4 tail-return would silently + // pick the lone fuzzy candidate and emit a CALLS edge — but that was an + // accidental match that happened to line up because `parentMethod` + // was unique in the global index. + // + // After the R3 tail-return tightening (PR #744 Codex review), member + // calls whose D1-D4 narrowing produces zero file-matched and zero + // owner-matched candidates null-route instead of falling through. + // The test now asserts the honest answer: without heritage information, + // we cannot attribute `c.parentMethod()` to `Parent` and therefore + // emit no edge. + // + // In the real ingestion pipeline, heritageMap is always threaded + // through, so this scenario is only reachable in tests that explicitly + // omit it. Keeping the test confirms the null-route behavior and + // documents the invariant "no heritage → no inherited-method edges". const { parentMethodId, appFile, parentFile, childFile } = setupChildParent(); await processCalls( @@ -1739,13 +1756,14 @@ describe('processCalls — D0 MRO fast path (SM-10)', () => { ], createASTCache(), ctx, - // no heritageMap — D0 fast path must be skipped, D1-D4 must still resolve + // no heritageMap — D0 MRO walk is unavailable, D1-D4 receiver filtering + // also cannot link c.parentMethod() to Parent, so no edge is emitted. ); const parentMethodCalls = graph.relationships.filter( (r) => r.type === 'CALLS' && r.targetId === parentMethodId, ); - expect(parentMethodCalls).toHaveLength(1); + expect(parentMethodCalls).toHaveLength(0); }); it('overloadHints guard: D0 skipped so literal-inferred overload disambiguation picks the right overload', async () => { diff --git a/gitnexus/test/unit/symbol-table.test.ts b/gitnexus/test/unit/symbol-table.test.ts index b2ba7e288..1ad33417a 100644 --- a/gitnexus/test/unit/symbol-table.test.ts +++ b/gitnexus/test/unit/symbol-table.test.ts @@ -798,8 +798,19 @@ describe('SymbolTable', () => { expect(table.lookupClassByName('Qux')).toEqual([]); }); + it('includes Trait in the class set (PHP use, Rust impl, Scala traits)', () => { + // Traits are class-like for heritage resolution — they contribute + // methods to the using/implementing type's hierarchy. buildHeritageMap + // relies on this to resolve `use Trait;` edges in PHP, `impl Trait for + // Struct` in Rust, etc. Added as part of PR #744 (SM-11 Codex review + // fixes) after the PHP HasTimestamps trait walk gap was discovered. + table.add('src/a.rs', 'Writer', 'trait:Writer', 'Trait'); + const results = table.lookupClassByName('Writer'); + expect(results).toHaveLength(1); + expect(results[0].nodeId).toBe('trait:Writer'); + }); + it('does NOT include other type-like labels outside the allowed class set', () => { - table.add('src/a.rs', 'User', 'trait:User', 'Trait'); table.add('src/a.ts', 'User', 'type:User', 'Type'); expect(table.lookupClassByName('User')).toEqual([]); }); @@ -1398,3 +1409,499 @@ describe('lookupMethodByOwnerWithMRO', () => { expect(result!.nodeId).toBe('method:User:getName'); }); }); + +// --------------------------------------------------------------------------- +// resolveMemberCall — SM-11: owner-scoped + MRO member-call resolution +// --------------------------------------------------------------------------- + +import { + _resolveCallTargetForTesting, + resolveMemberCall, + type OverloadHints, +} from '../../src/core/ingestion/call-processor.js'; + +describe('resolveMemberCall', () => { + let ctx: ResolutionContext; + + beforeEach(() => { + ctx = createResolutionContext(); + }); + + it('resolves direct method on owner type', () => { + ctx.symbols.add('src/user.ts', 'User', 'class:User', 'Class'); + ctx.symbols.add('src/user.ts', 'save', 'method:User:save', 'Method', { + returnType: 'void', + ownerId: 'class:User', + }); + ctx.importMap.set('src/app.ts', new Set(['src/user.ts'])); + + const result = resolveMemberCall('User', 'save', 'src/app.ts', ctx); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('method:User:save'); + expect(result!.returnType).toBe('void'); + expect(result!.confidence).toBeGreaterThan(0); + }); + + it('resolves inherited method via MRO walk', () => { + ctx.symbols.add('src/parent.java', 'Parent', 'class:Parent', 'Class'); + ctx.symbols.add('src/child.java', 'Child', 'class:Child', 'Class'); + ctx.symbols.add('src/parent.java', 'validate', 'method:Parent:validate', 'Method', { + returnType: 'boolean', + ownerId: 'class:Parent', + }); + ctx.importMap.set('src/app.java', new Set(['src/child.java', 'src/parent.java'])); + + const heritage: ExtractedHeritage[] = [ + { filePath: 'src/child.java', className: 'Child', parentName: 'Parent', kind: 'extends' }, + ]; + const map = buildHeritageMap(heritage, ctx); + + const result = resolveMemberCall('Child', 'validate', 'src/app.java', ctx, map); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('method:Parent:validate'); + expect(result!.returnType).toBe('boolean'); + }); + + it('returns null for unknown owner type', () => { + const result = resolveMemberCall('NonExistent', 'save', 'src/app.ts', ctx); + expect(result).toBeNull(); + }); + + it('returns null for unknown method on known owner', () => { + ctx.symbols.add('src/user.ts', 'User', 'class:User', 'Class'); + ctx.importMap.set('src/app.ts', new Set(['src/user.ts'])); + + const result = resolveMemberCall('User', 'nonExistentMethod', 'src/app.ts', ctx); + expect(result).toBeNull(); + }); + + it('returns result with correct confidence tier for same-file resolution', () => { + ctx.symbols.add('src/app.ts', 'User', 'class:User', 'Class'); + ctx.symbols.add('src/app.ts', 'save', 'method:User:save', 'Method', { + returnType: 'void', + ownerId: 'class:User', + }); + + const result = resolveMemberCall('User', 'save', 'src/app.ts', ctx); + + expect(result).not.toBeNull(); + expect(result!.confidence).toBe(0.95); // same-file tier + expect(result!.reason).toBe('same-file'); + }); + + it('returns result with import-scoped tier for cross-file resolution', () => { + ctx.symbols.add('src/user.ts', 'User', 'class:User', 'Class'); + ctx.symbols.add('src/user.ts', 'save', 'method:User:save', 'Method', { + returnType: 'void', + ownerId: 'class:User', + }); + ctx.importMap.set('src/app.ts', new Set(['src/user.ts'])); + + const result = resolveMemberCall('User', 'save', 'src/app.ts', ctx); + + expect(result).not.toBeNull(); + expect(result!.confidence).toBe(0.9); // import-scoped tier + expect(result!.reason).toBe('import-resolved'); + }); + + it('resolves with heritage map across C3 MRO chain (Python)', () => { + ctx.symbols.add('src/a.py', 'A', 'class:A', 'Class'); + ctx.symbols.add('src/b.py', 'B', 'class:B', 'Class'); + ctx.symbols.add('src/c.py', 'C', 'class:C', 'Class'); + ctx.symbols.add('src/a.py', 'foo', 'method:A:foo', 'Method', { + returnType: 'str', + ownerId: 'class:A', + }); + ctx.importMap.set('src/main.py', new Set(['src/a.py', 'src/b.py', 'src/c.py'])); + + const heritage: ExtractedHeritage[] = [ + { filePath: 'src/c.py', className: 'C', parentName: 'B', kind: 'extends' }, + { filePath: 'src/b.py', className: 'B', parentName: 'A', kind: 'extends' }, + ]; + const map = buildHeritageMap(heritage, ctx); + + const result = resolveMemberCall('C', 'foo', 'src/main.py', ctx, map); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('method:A:foo'); + expect(result!.returnType).toBe('str'); + }); + + // ------------------------------------------------------------------------- + // Locks in the B2 semantic change: tier reflects how the OWNER TYPE was + // resolved, not how the method name was resolved globally. + // ------------------------------------------------------------------------- + it('uses owner-type tier: cross-file class resolution → import-scoped confidence', () => { + // Scenario: owner class 'User' is defined in user.ts (imported from app.ts). + // The method 'save' exists ONLY on User (no homonyms). Old behaviour would + // have used the tier of resolving "save" globally; new behaviour uses the + // tier of resolving "User". Both happen to yield import-scoped here — + // the test locks that the reported tier tracks the class lookup. + ctx.symbols.add('src/user.ts', 'User', 'class:User', 'Class'); + ctx.symbols.add('src/user.ts', 'save', 'method:User:save', 'Method', { + returnType: 'void', + ownerId: 'class:User', + }); + ctx.importMap.set('src/app.ts', new Set(['src/user.ts'])); + + const result = resolveMemberCall('User', 'save', 'src/app.ts', ctx); + + expect(result).not.toBeNull(); + expect(result!.confidence).toBe(0.9); // import-scoped + expect(result!.reason).toBe('import-resolved'); + }); + + // ------------------------------------------------------------------------- + // T2: Rust qualified-syntax — trait-inherited methods must return null + // because they require `TraitName::method(obj)` call syntax, not `obj.method()`. + // Only struct's OWN impl methods are reachable via direct member calls. + // ------------------------------------------------------------------------- + it('Rust: returns null for trait-inherited method (qualified-syntax MRO)', () => { + // Trait Writer defines `save`. Struct User has an impl_item but NO save + // method of its own — save is only available via trait. + ctx.symbols.add('src/writer.rs', 'Writer', 'trait:Writer', 'Trait'); + ctx.symbols.add('src/user.rs', 'User', 'struct:User', 'Struct'); + ctx.symbols.add('src/writer.rs', 'save', 'method:Writer:save', 'Method', { + returnType: 'bool', + ownerId: 'trait:Writer', + }); + ctx.importMap.set('src/app.rs', new Set(['src/writer.rs', 'src/user.rs'])); + + const heritage: ExtractedHeritage[] = [ + // User implements Writer — in Rust this is `impl Writer for User`. + { filePath: 'src/user.rs', className: 'User', parentName: 'Writer', kind: 'implements' }, + ]; + const map = buildHeritageMap(heritage, ctx); + + // Rust's qualified-syntax strategy short-circuits trait inheritance walks, + // so `user.save()` (direct call) does not resolve. + const result = resolveMemberCall('User', 'save', 'src/app.rs', ctx, map); + expect(result).toBeNull(); + }); + + it('Rust: direct impl methods still resolve (distinction check for T2)', () => { + // Positive control: a method defined directly on User (not via trait) + // resolves normally — demonstrates the null in the previous test is + // specifically due to the trait-inheritance path, not a broken fixture. + ctx.symbols.add('src/user.rs', 'User', 'struct:User', 'Struct'); + ctx.symbols.add('src/user.rs', 'name', 'method:User:name', 'Method', { + returnType: 'String', + ownerId: 'struct:User', + }); + ctx.importMap.set('src/app.rs', new Set(['src/user.rs'])); + + const result = resolveMemberCall('User', 'name', 'src/app.rs', ctx); + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('method:User:name'); + expect(result!.returnType).toBe('String'); + }); + + // ------------------------------------------------------------------------- + // T3: C/C++ leftmost-base diamond inheritance at the resolveMemberCall layer. + // ------------------------------------------------------------------------- + // ------------------------------------------------------------------------- + // Homonym disambiguation: when two class candidates share a name but only + // ONE of them owns the method, resolveMemberCall should return that one + // without falling through to the fuzzy D2 widening path. Absorbs what was + // previously D4's ownerId-filtering job into the owner-scoped path. + // ------------------------------------------------------------------------- + it('disambiguates homonym classes: only one owns the method', () => { + // Two classes both named `User` — one in auth.py (has `save`), one in + // legacy.py (has `archive` but no `save`). Both are imported from app.py. + ctx.symbols.add('src/auth.py', 'User', 'class:auth:User', 'Class'); + ctx.symbols.add('src/auth.py', 'save', 'method:auth:User:save', 'Method', { + returnType: 'None', + ownerId: 'class:auth:User', + }); + ctx.symbols.add('src/legacy.py', 'User', 'class:legacy:User', 'Class'); + ctx.symbols.add('src/legacy.py', 'archive', 'method:legacy:User:archive', 'Method', { + returnType: 'None', + ownerId: 'class:legacy:User', + }); + ctx.importMap.set('src/app.py', new Set(['src/auth.py', 'src/legacy.py'])); + + // `user.save()` is unambiguous — only auth.User has `save`. + const saveResult = resolveMemberCall('User', 'save', 'src/app.py', ctx); + expect(saveResult).not.toBeNull(); + expect(saveResult!.nodeId).toBe('method:auth:User:save'); + + // `user.archive()` is also unambiguous — only legacy.User has `archive`. + const archiveResult = resolveMemberCall('User', 'archive', 'src/app.py', ctx); + expect(archiveResult).not.toBeNull(); + expect(archiveResult!.nodeId).toBe('method:legacy:User:archive'); + }); + + it('returns null when homonym classes BOTH own the method (genuine ambiguity)', () => { + // Both homonym Users define a `save` method — resolveMemberCall refuses + // to pick one. The caller (resolveCallTarget) falls through to D1-D4 which + // may or may not be able to narrow further. + ctx.symbols.add('src/auth.py', 'User', 'class:auth:User', 'Class'); + ctx.symbols.add('src/auth.py', 'save', 'method:auth:User:save', 'Method', { + returnType: 'None', + ownerId: 'class:auth:User', + }); + ctx.symbols.add('src/legacy.py', 'User', 'class:legacy:User', 'Class'); + ctx.symbols.add('src/legacy.py', 'save', 'method:legacy:User:save', 'Method', { + returnType: 'None', + ownerId: 'class:legacy:User', + }); + ctx.importMap.set('src/app.py', new Set(['src/auth.py', 'src/legacy.py'])); + + const result = resolveMemberCall('User', 'save', 'src/app.py', ctx); + expect(result).toBeNull(); + }); + + it('homonym + shared ancestor: both walk MRO to the same method (dedups to 1)', () => { + // Two homonym `User` classes in different files, both extending a common + // `BaseUser` that owns `save`. Direct lookup on either User misses; MRO + // walks both find BaseUser.save. Dedup by nodeId yields a single result. + ctx.symbols.add('src/base.ts', 'BaseUser', 'class:BaseUser', 'Class'); + ctx.symbols.add('src/base.ts', 'save', 'method:BaseUser:save', 'Method', { + returnType: 'void', + ownerId: 'class:BaseUser', + }); + ctx.symbols.add('src/a.ts', 'User', 'class:a:User', 'Class'); + ctx.symbols.add('src/b.ts', 'User', 'class:b:User', 'Class'); + ctx.importMap.set('src/app.ts', new Set(['src/base.ts', 'src/a.ts', 'src/b.ts'])); + + const heritage: ExtractedHeritage[] = [ + { filePath: 'src/a.ts', className: 'User', parentName: 'BaseUser', kind: 'extends' }, + { filePath: 'src/b.ts', className: 'User', parentName: 'BaseUser', kind: 'extends' }, + ]; + const map = buildHeritageMap(heritage, ctx); + + const result = resolveMemberCall('User', 'save', 'src/app.ts', ctx, map); + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('method:BaseUser:save'); + }); + + it('C++: resolves diamond inheritance via leftmost-base MRO', () => { + // Diamond: + // Base + // / \ + // A B + // \ / + // Derived + // + // Both A and B inherit `method` from Base. Derived extends (A, B). + // Leftmost-base strategy walks A's chain first → finds Base::method. + ctx.symbols.add('src/base.h', 'Base', 'class:Base', 'Class'); + ctx.symbols.add('src/a.h', 'A', 'class:A', 'Class'); + ctx.symbols.add('src/b.h', 'B', 'class:B', 'Class'); + ctx.symbols.add('src/derived.h', 'Derived', 'class:Derived', 'Class'); + ctx.symbols.add('src/base.h', 'method', 'method:Base:method', 'Method', { + returnType: 'int', + ownerId: 'class:Base', + }); + ctx.importMap.set( + 'src/app.cpp', + new Set(['src/base.h', 'src/a.h', 'src/b.h', 'src/derived.h']), + ); + + const heritage: ExtractedHeritage[] = [ + { filePath: 'src/a.h', className: 'A', parentName: 'Base', kind: 'extends' }, + { filePath: 'src/b.h', className: 'B', parentName: 'Base', kind: 'extends' }, + { filePath: 'src/derived.h', className: 'Derived', parentName: 'A', kind: 'extends' }, + { filePath: 'src/derived.h', className: 'Derived', parentName: 'B', kind: 'extends' }, + ]; + const map = buildHeritageMap(heritage, ctx); + + const result = resolveMemberCall('Derived', 'method', 'src/app.cpp', ctx, map); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('method:Base:method'); + expect(result!.returnType).toBe('int'); + }); + + // ------------------------------------------------------------------------- + // L1: C# / Kotlin implements-split strategy through resolveMemberCall. + // lookupMethodByOwnerWithMRO already has strategy-level coverage for these + // languages; these tests add the resolveMemberCall layer (tier resolution + // + class candidate iteration + MRO walk) on top. + // ------------------------------------------------------------------------- + it('C#: walks implements-split to find inherited method via interface', () => { + // C# uses implements-split MRO: class base chain walked first, then + // interfaces. Here IService declares Save which is implemented by the + // base class BaseService — MyService inherits Save through the class. + ctx.symbols.add('src/iservice.cs', 'IService', 'interface:IService', 'Interface'); + ctx.symbols.add('src/base.cs', 'BaseService', 'class:BaseService', 'Class'); + ctx.symbols.add('src/my.cs', 'MyService', 'class:MyService', 'Class'); + ctx.symbols.add('src/base.cs', 'Save', 'method:BaseService:Save', 'Method', { + returnType: 'void', + ownerId: 'class:BaseService', + }); + ctx.importMap.set('src/app.cs', new Set(['src/iservice.cs', 'src/base.cs', 'src/my.cs'])); + + const heritage: ExtractedHeritage[] = [ + { + filePath: 'src/base.cs', + className: 'BaseService', + parentName: 'IService', + kind: 'implements', + }, + { filePath: 'src/my.cs', className: 'MyService', parentName: 'BaseService', kind: 'extends' }, + ]; + const map = buildHeritageMap(heritage, ctx); + + const result = resolveMemberCall('MyService', 'Save', 'src/app.cs', ctx, map); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('method:BaseService:Save'); + expect(result!.returnType).toBe('void'); + }); + + it('Kotlin: walks implements-split to find inherited method via interface', () => { + // Kotlin shares the implements-split MRO strategy with Java/C#. A class + // inheriting from an interface that provides a default method should + // resolve `obj.method()` to the interface's implementation. + ctx.symbols.add('src/validator.kt', 'Validator', 'interface:Validator', 'Interface'); + ctx.symbols.add('src/user.kt', 'User', 'class:User', 'Class'); + ctx.symbols.add('src/validator.kt', 'validate', 'method:Validator:validate', 'Method', { + returnType: 'Boolean', + ownerId: 'interface:Validator', + }); + ctx.importMap.set('src/app.kt', new Set(['src/validator.kt', 'src/user.kt'])); + + const heritage: ExtractedHeritage[] = [ + { + filePath: 'src/user.kt', + className: 'User', + parentName: 'Validator', + kind: 'implements', + }, + ]; + const map = buildHeritageMap(heritage, ctx); + + const result = resolveMemberCall('User', 'validate', 'src/app.kt', ctx, map); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('method:Validator:validate'); + expect(result!.returnType).toBe('Boolean'); + }); +}); + +// --------------------------------------------------------------------------- +// T1: D0 skip-condition tests — verify resolveCallTarget bypasses the +// resolveMemberCall fast path when overloadHints, preComputedArgTypes, or a +// module alias is active. +// --------------------------------------------------------------------------- + +describe('resolveCallTarget D0 skip conditions (SM-11)', () => { + let ctx: ResolutionContext; + + beforeEach(() => { + ctx = createResolutionContext(); + }); + + it('module alias: picks alias-scoped class over homonym (D0 actually bypassed)', () => { + // Python-style: `import auth; auth.User.save()` where BOTH auth.py and + // other.py define a `User` class with a `save` method. The test proves: + // + // 1. Without the alias: resolveMemberCall sees two homonym Users, + // both own `save`, and correctly returns null (refuses to guess). + // 2. With the alias: D0 is skipped via `hasActiveModuleAlias`, and + // D1-D4 — respecting the alias-narrowed filteredCandidates — picks + // the auth.py User.save method. + // + // A regression where D0 silently ran would produce null (ambiguous) + // instead of the correct answer, so this test actually exercises the + // skip path rather than just verifying a single-candidate happy path. + ctx.symbols.add('src/auth.py', 'User', 'class:auth:User', 'Class'); + ctx.symbols.add('src/auth.py', 'save', 'method:auth:User:save', 'Method', { + returnType: 'None', + ownerId: 'class:auth:User', + }); + ctx.symbols.add('src/other.py', 'User', 'class:other:User', 'Class'); + ctx.symbols.add('src/other.py', 'save', 'method:other:User:save', 'Method', { + returnType: 'None', + ownerId: 'class:other:User', + }); + ctx.importMap.set('src/app.py', new Set(['src/auth.py', 'src/other.py'])); + ctx.moduleAliasMap.set('src/app.py', new Map([['auth', 'src/auth.py']])); + + // Control: without alias narrowing, resolveMemberCall sees both Users + // own `save` and correctly refuses to pick one. + const ambiguous = resolveMemberCall('User', 'save', 'src/app.py', ctx); + expect(ambiguous).toBeNull(); + + // With alias narrowing active, D0 is skipped and D1-D4 picks auth.py's + // User.save because the alias block already narrowed filteredCandidates + // to auth.py (and the D2 widening step is gated on `!aliasNarrowed`). + const aliased = _resolveCallTargetForTesting( + { + calledName: 'save', + callForm: 'member', + receiverTypeName: 'User', + receiverName: 'auth', // triggers hasActiveModuleAlias → D0 skipped + }, + 'src/app.py', + ctx, + ); + + expect(aliased).not.toBeNull(); + expect(aliased!.nodeId).toBe('method:auth:User:save'); + }); + + it('overloadHints present: D0 bypassed, D1-D4 handles resolution', () => { + // When overloadHints is supplied, the D0 fast path must be skipped + // because lookupMethodByOwner does not consider argument types and + // would pick an arbitrary overload for same-return-type overloads. + // + // This test verifies that the skip does not break resolution: passing + // a dummy overloadHints object should still yield the correct method + // via the D1-D4 path. + ctx.symbols.add('src/user.ts', 'User', 'class:User', 'Class'); + ctx.symbols.add('src/user.ts', 'save', 'method:User:save', 'Method', { + returnType: 'void', + ownerId: 'class:User', + }); + ctx.importMap.set('src/app.ts', new Set(['src/user.ts'])); + + // Minimal stub; D1-D4 only calls tryOverloadDisambiguation when there are + // multiple candidates, so an empty object is fine for single-candidate cases. + const dummyHints = {} as OverloadHints; + + const result = _resolveCallTargetForTesting( + { + calledName: 'save', + callForm: 'member', + receiverTypeName: 'User', + }, + 'src/app.ts', + ctx, + { overloadHints: dummyHints }, + ); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('method:User:save'); + }); + + it('preComputedArgTypes present: D0 bypassed, D1-D4 handles resolution', () => { + // Analogous to the overloadHints case: when preComputedArgTypes is supplied + // (worker path), D0 must be skipped so that type-based overload + // disambiguation in D1-D4 is authoritative. + ctx.symbols.add('src/user.ts', 'User', 'class:User', 'Class'); + ctx.symbols.add('src/user.ts', 'save', 'method:User:save', 'Method', { + returnType: 'void', + ownerId: 'class:User', + }); + ctx.importMap.set('src/app.ts', new Set(['src/user.ts'])); + + const result = _resolveCallTargetForTesting( + { + calledName: 'save', + callForm: 'member', + receiverTypeName: 'User', + argCount: 0, + }, + 'src/app.ts', + ctx, + { preComputedArgTypes: [] }, + ); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('method:User:save'); + }); +}); From 3f0b8c1a5b67b352005c4401b1c0102649ffd8fb Mon Sep 17 00:00:00 2001 From: Kunal Hemnani <143599188+kunalhemnani1@users.noreply.github.com> Date: Thu, 9 Apr 2026 16:27:36 +0530 Subject: [PATCH 09/12] fix(ingestion): replace lookupExact with lookupExactAll in named-binding-processor (#755) --- gitnexus/src/core/ingestion/named-binding-processor.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gitnexus/src/core/ingestion/named-binding-processor.ts b/gitnexus/src/core/ingestion/named-binding-processor.ts index cf54cee59..1802f86e8 100644 --- a/gitnexus/src/core/ingestion/named-binding-processor.ts +++ b/gitnexus/src/core/ingestion/named-binding-processor.ts @@ -41,7 +41,7 @@ export function walkBindingChain( const targetName = binding.exportedName; const resolvedDefs = targetName !== lookupName || depth > 0 - ? symbolTable.lookupFuzzy(targetName).filter((def) => def.filePath === binding.sourcePath) + ? symbolTable.lookupExactAll(binding.sourcePath, targetName) : allDefs.filter((def) => def.filePath === binding.sourcePath); if (resolvedDefs.length > 0) return resolvedDefs; From 4450a14b98d782e1976b44dd60c512a9b46bf415 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Thu, 9 Apr 2026 12:09:07 +0100 Subject: [PATCH 10/12] feat(SM-12): Extract `resolveStaticCall` from `resolveCallTarget` (#754) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Initial plan * feat(SM-12): extract resolveStaticCall from resolveCallTarget - Add resolveStaticCall(className, methodName, currentFile, ctx, argCount?) using lookupClassByName + lookupMethodByOwner for O(1) constructor/static resolution - Add S0 fast path in resolveCallTarget for constructor/free-form class calls - Export resolveStaticCall from call-processor.ts - Add 11 unit tests covering constructor resolution, confidence tiers, arity disambiguation, and resolveCallTarget delegation Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/c9471ca9-57ff-4dae-956e-e7ffdc326bc4 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * chore: revert unrelated package-lock.json change Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/c9471ca9-57ff-4dae-956e-e7ffdc326bc4 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * refactor: shorten verbose test name per code review feedback Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/c9471ca9-57ff-4dae-956e-e7ffdc326bc4 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * fix(SM-12): address PR #754 review findings Addresses Claude's review comments on PR #754: Performance - Pass pre-computed `tiered` result into `resolveStaticCall` as optional `tieredOverride` parameter, eliminating the duplicate `ctx.resolve(className, currentFile)` on every constructor call path. - Cache `freeFormHasClassTarget` in `resolveCallTarget` so the S0 fast path and the free-form constructor retry share a single `.some()` scan. Architecture - Reconcile `CLASS_LIKE_TYPES` (call-processor) with `CLASS_TYPES` (symbol-table): `CLASS_LIKE_TYPES = [...CLASS_TYPES, 'Impl']`. This makes the relationship explicit — the call resolver's set is a strict superset of the heritage-index set, guaranteeing anything reachable via `lookupClassByName` also passes the resolver filter. Trait is now included (harmless: traits have no Constructor nodes, so step-3 returns undefined and step-5 still returns the class-like node when unique). Documented the Interface inclusion rationale (static methods + MRO walker). - Collapse `resolveStaticCall`'s `methodName` parameter into `className` — all call sites passed identical values. Named constructors (Dart `User.fromJson()`) arrive as member calls and go through `resolveMemberCall`. Documented the reserved path for when a language surfaces a static-method-shaped call with a distinct member name. - Document the known gap: `callForm === 'member'` constructor patterns (e.g. Python `models.User()`) are handled by the tail fallback, not S0. Tests - Add tiered-override test asserting `ctx.resolve` is not re-invoked when a pre-computed result is passed in. - Add language-specific `_resolveCallTargetForTesting` integration tests for Java (`new User()`), Python (`User()`), and Kotlin (`User()`). Verification: 3031 unit + 1766 integration tests pass, zero regressions. * fix(SM-12): restrict resolveStaticCall fallback to instantiable kinds Addresses the high-severity finding from the Codex adversarial review of PR #754: `resolveStaticCall`'s step-5 "return the class itself when no Constructor node is found" fallback reused `CLASS_LIKE_TYPES`, which — after SM-11 and PR #754's reconciliation — now includes `Interface`, `Trait`, and `Impl`. That is the method-dispatch set, not the instantiable set, so constructor-shaped calls could resolve to non-instantiable nodes and emit false `CALLS` edges. Concrete failure: Rust same-file `impl User { ... }` alongside `struct User { ... }` — both land at same-file tier, the Impl is not filtered out, and the step-5 fallback produces a `CALLS` edge to the `Impl` block instead of the `Struct`. The same widening exposed Interface / Trait targets in Java / C# / PHP / Scala. Fix - Introduce `INSTANTIABLE_CLASS_TYPES = {'Class', 'Struct', 'Record'}` as a sibling to `CLASS_LIKE_TYPES`, documenting the contract explicitly and cross-referencing `CONSTRUCTOR_TARGET_TYPES`. - Update `CLASS_LIKE_TYPES` JSDoc to clarify it is the method-dispatch set and add an anti-pattern warning against reusing it for constructor-fallback filtering. - Tighten `resolveStaticCall` step 5: filter `classCandidates` through `INSTANTIABLE_CLASS_TYPES` before the `length === 1` check. This strips `Impl` from the Rust shadowing scenario (leaving `Struct` as the sole instantiable target) and null-routes Interface / Trait / `Impl`-alone scenarios, matching the SM-10 R3 null-route precedent. - Step 3 (explicit Constructor lookup via `lookupMethodByOwner`) is intentionally unchanged — its `def.type === 'Constructor'` check is the correct contract, and legitimate Constructor nodes attached to `Impl` owners still resolve correctly. Tests (+10 regression scenarios) - Positive guards: Struct, Record fallback paths. - Null-route: Interface (Java/C#/TS), PHP Trait, Rust Trait. - Rust same-file shadowing: Struct wins over Impl. - Rust Impl-alone: null-routes (no Struct present). - Step-3 preservation: Constructor owned by Impl still resolves to the Constructor node, proving step-5 tightening doesn't leak into step 3. - Full cascade via `_resolveCallTargetForTesting` for Interface and Trait — confirms no downstream path silently re-introduces the edge. Verification - `tsc --noEmit` clean - 3041 unit tests pass (+10) - 1766 integration tests pass - Zero regressions Plan: docs/plans/2026-04-09-002-fix-sm12-constructor-fallback-instantiable-only-plan.md Codex review job: review-mnrao7fr-nv9y0e * fix(SM-12): address PR #754 second review round Addresses the 9 findings from the follow-up review on PR #754. Performance - Align `freeFormHasClassTarget` with `INSTANTIABLE_CLASS_TYPES`: drop `Enum` (S0 would always return null for it — wasted lookup work) and add `Record` (C# records and Kotlin data classes were bypassing S0 entirely). The trigger set and the fallback filter set now agree by construction, documented inline. Documentation - Remove stale single-line JSDoc on `CLASS_LIKE_TYPES` (line 57) that duplicated the full multi-line block immediately below it — tooling picks up the first block so the old one-liner was shadowing the current explanation. - Rewrite the `resolveStaticCall` JSDoc step list to match the actual step boundaries in the implementation (steps 3, 4, 5 were blurred in the old description). - Add inline comment on step 3 documenting the same-name lookup assumption (`${candidate.nodeId}\0${className}`) and the symmetric miss case for Python `__init__`-style constructors. - Add inline comment on step 4 documenting that it also catches the ambiguous-step-3 case, and warning against removing the check without handling that path explicitly. - Add inline comment on step 5 enumerating the three length outcomes (0 / 1 / >1) so future readers see the dominant null-route case. - Document Ruby `User.new` as a known gap alongside Python `models.User()` in the S0 header comment. Tests (+2 scenarios) - Record free-form constructor call via `_resolveCallTargetForTesting` exercises the aligned `freeFormHasClassTarget` trigger end-to-end, closing the gap where the direct `resolveStaticCall` test passed but the integration path was silently bypassing S0. - Arity threading via `_resolveCallTargetForTesting` asserts that `call.argCount` flows through resolveCallTarget → S0 → resolveStaticCall → lookupMethodByOwner, catching any future regression where the argCount is dropped at the S0 call site. Verification - `tsc --noEmit` clean - 3043 unit tests pass (+2) - 1766 integration tests pass - Zero regressions Plan: docs/plans/2026-04-09-002-fix-sm12-constructor-fallback-instantiable-only-plan.md Review: https://github.com/abhigyanpatwari/GitNexus/pull/754#issuecomment-4213536094 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> Co-authored-by: Gergo Magyar --- gitnexus/src/core/ingestion/call-processor.ts | 245 +++++++++- gitnexus/test/unit/symbol-table.test.ts | 424 ++++++++++++++++++ 2 files changed, 663 insertions(+), 6 deletions(-) diff --git a/gitnexus/src/core/ingestion/call-processor.ts b/gitnexus/src/core/ingestion/call-processor.ts index a739c8441..2a47d5f39 100644 --- a/gitnexus/src/core/ingestion/call-processor.ts +++ b/gitnexus/src/core/ingestion/call-processor.ts @@ -1,9 +1,11 @@ import { KnowledgeGraph } from '../graph/types.js'; import { ASTCache } from './ast-cache.js'; import type { SymbolDefinition, SymbolTable } from './symbol-table.js'; +import { CLASS_TYPES } from './symbol-table.js'; import Parser from 'tree-sitter'; import type { ResolutionContext } from './resolution-context.js'; import { TIER_CONFIDENCE, type ResolutionTier } from './resolution-context.js'; +import type { TieredCandidates } from './resolution-context.js'; import { isLanguageAvailable, loadParser, loadLanguage } from '../tree-sitter/parser-loader.js'; import { getProvider } from './languages/index.js'; import { generateId } from '../../lib/utils.js'; @@ -52,8 +54,51 @@ import { extractParsedCallSite } from './call-sites/extract-language-call-site.j * Populated during call processing, consumed by Phase 14 re-resolution pass. */ export type ExportedTypeMap = Map>; -/** Types that represent class-like declarations (used for receiver/owner resolution). */ -const CLASS_LIKE_TYPES = new Set(['Class', 'Struct', 'Interface', 'Enum', 'Record', 'Impl']); +/** + * Type labels treated as class-like **method-dispatch receivers** by the call + * resolver — the set walked by the MRO / heritage path for member and static + * method calls. + * + * Derived from `CLASS_TYPES` (the heritage-index set in symbol-table) plus + * `Impl` — Rust `impl` blocks are the definition site of methods for a struct + * and must be walkable as receiver-type candidates even though they are not + * indexed by `lookupClassByName` (which keys off struct/trait names). Keeping + * this set a strict superset of `CLASS_TYPES` guarantees that anything + * reachable via `lookupClassByName` also passes this filter, so the two call + * paths cannot diverge silently. + * + * `Interface` is included even though interfaces cannot be directly + * instantiated in Java/C#/TypeScript: the resolver still needs to reach + * interface nodes for static-method dispatch (`Interface.staticMethod()`) and + * default-method resolution via the MRO walker. + * + * **Do not reuse this set for constructor-fallback filtering.** Constructors + * can only instantiate a narrower subset — see `INSTANTIABLE_CLASS_TYPES` + * below. `resolveStaticCall`'s step-5 class-node fallback uses the narrower + * set to prevent false `CALLS` edges from constructor-shaped calls to + * `Interface`, `Trait`, or `Impl` nodes. + */ +const CLASS_LIKE_TYPES = new Set([...CLASS_TYPES, 'Impl']); + +/** + * Type labels that can be the target of a constructor-shaped call when no + * explicit `Constructor` symbol is indexed — the "return the type itself as + * the call target" fallback set. + * + * Strict subset of both `CLASS_LIKE_TYPES` and `CONSTRUCTOR_TARGET_TYPES`. + * Excludes: + * - `Interface` / `Trait` — not instantiable by definition in any + * supported language. + * - `Impl` — Rust `impl` blocks are method-definition containers, not + * the type itself; the owning `Struct` is the correct target. + * - `Enum` — excluded pending language-specific support with motivating + * test fixtures (matches `CONSTRUCTOR_TARGET_TYPES`). + * + * Used exclusively by `resolveStaticCall`'s step-5 class-node fallback. + * Keep in sync with `CONSTRUCTOR_TARGET_TYPES` (which additionally contains + * `'Constructor'` for explicit-constructor-node filtering) when extending. + */ +const INSTANTIABLE_CLASS_TYPES = new Set(['Class', 'Struct', 'Record']); const MAX_EXPORTS_PER_FILE = 500; const MAX_TYPE_NAME_LENGTH = 256; @@ -1331,14 +1376,55 @@ const resolveCallTarget = ( call.callForm, ); + // S0. Constructor/static fast path (SM-12): O(1) class + constructor lookup + // via lookupClassByName + lookupMethodByOwner before falling back to the + // existing filtering + fuzzy-widening path. Falls back to the class node + // itself when no Constructor symbol is indexed for the type. + // + // Handles: + // (a) callForm === 'constructor' — explicit `new User()` in Java/TS/C#/etc. + // (b) callForm === 'free' with class target — implicit `User()` in Swift/Kotlin + // + // Known gaps (handled by the existing tail fallback at the bottom of + // this function, not S0): + // - `callForm === 'member'` constructor patterns (e.g. Python + // `models.User()` after `import models`, Ruby `User.new`). Extending + // S0 to cover them would require threading receiver-type resolution + // through the module-alias logic; revisit if it shows up as a hot + // spot. + // + // The `.some()` trigger below must stay aligned with + // `INSTANTIABLE_CLASS_TYPES` — any type admitted here that is not in + // that set will cause S0 → `resolveStaticCall` to run and return null, + // wasting two lookup passes per call. `Enum` is deliberately excluded + // (same rationale as `INSTANTIABLE_CLASS_TYPES`); `Record` is included + // so C# records and Kotlin data classes reach the fast path. + const freeFormHasClassTarget = + call.callForm === 'free' && + filteredCandidates.length === 0 && + tiered.candidates.some((c) => c.type === 'Class' || c.type === 'Struct' || c.type === 'Record'); + if (call.callForm === 'constructor' || freeFormHasClassTarget) { + // Reuse the pre-computed `tiered` result — resolveStaticCall's class name + // is identical to `call.calledName` here, so re-running ctx.resolve would + // duplicate the tiered-lookup work performed at the top of this function. + const staticResult = resolveStaticCall( + call.calledName, + currentFile, + ctx, + call.argCount, + tiered, + ); + if (staticResult) return staticResult; + } + // Swift/Kotlin: constructor calls look like free function calls (no `new` keyword). // If free-form filtering found no callable candidates but the symbol resolves to a // Class/Struct, retry with constructor form so CONSTRUCTOR_TARGET_TYPES applies. if (filteredCandidates.length === 0 && call.callForm === 'free') { - const hasTypeTarget = tiered.candidates.some( - (c) => c.type === 'Class' || c.type === 'Struct' || c.type === 'Enum', - ); - if (hasTypeTarget) { + // `freeFormHasClassTarget` was already computed for the S0 fast path + // above under the same `callForm === 'free' && filteredCandidates.length === 0` + // precondition. Reuse it to avoid a second `.some()` scan on the same pool. + if (freeFormHasClassTarget) { filteredCandidates = filterCallableCandidates( tiered.candidates, call.argCount, @@ -1843,6 +1929,153 @@ export const resolveMemberCall = ( return toResolveResult(resolved.def, resolved.tier); }; +// --------------------------------------------------------------------------- +// SM-12: Constructor/static call resolution (no fuzzy lookup) +// --------------------------------------------------------------------------- + +/** + * Resolve a constructor or static call using class-scoped lookup (no fuzzy lookup). + * Used for `new User()` / `User()` calls where the calledName targets a class. + * + * Uses {@link SymbolTable.lookupClassByName} for O(1) class lookup and + * {@link SymbolTable.lookupMethodByOwner} for constructor resolution. + * {@link resolveCallTarget} delegates here for constructor and free-form calls + * that target a class, before falling back to the more expensive fuzzy-widening + * path (D1-D4). + * + * Resolution strategy: + * 1. `lookupClassByName(className)` — O(1) pre-check; bail early if no class exists. + * 2. `ctx.resolve(className, currentFile)` — import-scoped tier for confidence. + * 3. Filter to class-like candidates via `CLASS_LIKE_TYPES` and walk each + * with `lookupMethodByOwner(classNodeId, className, argCount)` — O(1) + * constructor lookup. Only accept results with `type === 'Constructor'`. + * 4. If step 3 found nothing and the tiered pool contains ownerless + * `Constructor` nodes (common in some extractors), bail out so + * `filterCallableCandidates` downstream handles Constructor-vs-Class + * preference correctly. + * 5. Class-node fallback: filter `classCandidates` through + * `INSTANTIABLE_CLASS_TYPES` and return the sole survivor when there is + * exactly one. Null-route on zero survivors (Interface / Trait / Impl + * stripped) or multiple (homonym ambiguity). + * + * @param className - The class name (e.g. 'User'). Also used as the method + * name for the `lookupMethodByOwner` scan, because the + * only constructor-shaped call we handle today is + * `ClassName(...)` / `new ClassName(...)`. Named + * constructors like Dart `User.fromJson()` arrive as + * member calls and route through `resolveMemberCall`, + * so this function does not yet need a separate + * `methodName` parameter. Revisit if a language surfaces + * a static-method-shaped call with a distinct member + * name. + * @param currentFile - File path of the call site + * @param ctx - Resolution context + * @param argCount - Optional argument count for arity filtering + * @param tieredOverride - Pre-computed tiered candidates for `className` from + * an upstream `ctx.resolve` call. When provided, skips + * the redundant lookup inside this function. Leave + * unset for direct callers without a prior resolution. + */ +export const resolveStaticCall = ( + className: string, + currentFile: string, + ctx: ResolutionContext, + argCount?: number, + tieredOverride?: TieredCandidates, +): ResolveResult | null => { + // 1. Pre-check: does a class with this name exist at all? (O(1)) + // This guards against the expensive `ctx.resolve` walk when the name + // is clearly not class-like (e.g. plain functions). When `tieredOverride` + // is supplied, the caller has already paid for the tiered lookup, so this + // pre-check still prevents the class-candidate filter + lookupMethodByOwner + // loop from running on obviously non-class targets. + const allClasses = ctx.symbols.lookupClassByName(className); + if (allClasses.length === 0) return null; + + // 2. Scope via ctx.resolve for import-tier information. Reuse the caller's + // tiered result when provided — it is computed from the same name and + // file context, so re-running the walk would be a pure waste. + const typeResolved = tieredOverride ?? ctx.resolve(className, currentFile); + if (!typeResolved) return null; + + const classCandidates = typeResolved.candidates.filter((c) => CLASS_LIKE_TYPES.has(c.type)); + if (classCandidates.length === 0) return null; + + // 3. Try lookupMethodByOwner for explicit Constructor nodes. + // Only accept results with type === 'Constructor' — a Method or Function + // that happens to share the class name (e.g. C++ methods named after + // their class) is not a constructor for resolution purposes. + // Same dedup logic as resolveMethodByOwner: diamond inheritance converging + // on the same constructor collapses to one hit. + // + // Same-name assumption: the lookup key is `${candidate.nodeId}\0${className}`, + // so this finds Constructor nodes whose symbol name equals the class name + // (`class User` with a `Constructor` named `User`). Constructors indexed + // under a different name (e.g. Python `__init__`) will not be found here — + // but they also won't appear in the tiered pool for `ctx.resolve(className)` + // for the same reason, so step 4's Constructor-presence check will not + // see them either. The two miss cases are symmetric. If a future extractor + // indexes Constructor nodes under an alternative name while still setting + // `ownerId`, this assumption will need revisiting. + let firstDef: SymbolDefinition | undefined; + let ambiguous = false; + for (const candidate of classCandidates) { + const def = ctx.symbols.lookupMethodByOwner(candidate.nodeId, className, argCount); + if (!def || def.type !== 'Constructor') continue; + if (!firstDef) { + firstDef = def; + } else if (def.nodeId !== firstDef.nodeId) { + ambiguous = true; + break; + } + } + + if (firstDef && !ambiguous) { + return toResolveResult(firstDef, typeResolved.tier); + } + + // 4. lookupMethodByOwner found nothing — check whether the tiered pool + // contains Constructor nodes that lack ownerId (common in some extractors). + // If so, bail out so the existing filterCallableCandidates path handles + // Constructor-vs-Class preference correctly. + // + // This branch also catches the step-3 ambiguous case (`ambiguous = true` + // with two distinct Constructor nodes across multiple class candidates): + // the same Constructor nodes are indexed under the class name in the + // tiered pool, so `.some(Constructor)` is true here and we defer to + // `filterCallableCandidates` downstream rather than guess which overload + // to pick. Do not remove this check without also handling the ambiguous + // step-3 path explicitly. + if (typeResolved.candidates.some((c) => c.type === 'Constructor')) { + return null; + } + + // 5. No constructor nodes at all — fall back to the class node itself, but + // ONLY when it is actually instantiable. Interface / Trait / Impl / Enum + // are deliberately excluded via `INSTANTIABLE_CLASS_TYPES` to prevent + // false `CALLS` edges from constructor-shaped calls to non-instantiable + // nodes. This also disambiguates the Rust same-file shadowing case + // (`struct User` + `impl User` both present at same-file tier): the + // Impl is stripped, leaving the Struct as the sole instantiable target. + // Addresses Codex review finding on PR #754. + const instantiableCandidates = classCandidates.filter((c) => + INSTANTIABLE_CLASS_TYPES.has(c.type), + ); + // Three outcomes below, in order of likelihood after the fix: + // length === 0 → all candidates were stripped as non-instantiable (e.g. + // Interface / Trait / Impl). Null-route via the fall-through `return + // null` — this is the dominant Codex-fix case. + // length === 1 → a single instantiable candidate remains, return it. + // length > 1 → two or more instantiable classes share the name (e.g. + // homonym classes across files with no import narrowing). Fall through + // to `return null` so the caller null-routes rather than guess. + if (instantiableCandidates.length === 1) { + return toResolveResult(instantiableCandidates[0], typeResolved.tier); + } + + return null; +}; + // --------------------------------------------------------------------------- // MRO-aware method resolution via HeritageMap (SM-9) // --------------------------------------------------------------------------- diff --git a/gitnexus/test/unit/symbol-table.test.ts b/gitnexus/test/unit/symbol-table.test.ts index 1ad33417a..b861c31e7 100644 --- a/gitnexus/test/unit/symbol-table.test.ts +++ b/gitnexus/test/unit/symbol-table.test.ts @@ -1905,3 +1905,427 @@ describe('resolveCallTarget D0 skip conditions (SM-11)', () => { expect(result!.nodeId).toBe('method:User:save'); }); }); + +// --------------------------------------------------------------------------- +// resolveStaticCall — SM-12: constructor/static call resolution +// --------------------------------------------------------------------------- + +import { resolveStaticCall } from '../../src/core/ingestion/call-processor.js'; + +describe('resolveStaticCall', () => { + let ctx: ResolutionContext; + + beforeEach(() => { + ctx = createResolutionContext(); + }); + + it('resolves constructor with ownerId via lookupMethodByOwner', () => { + ctx.symbols.add('src/user.ts', 'User', 'class:User', 'Class'); + ctx.symbols.add('src/user.ts', 'User', 'ctor:User', 'Constructor', { + returnType: 'User', + ownerId: 'class:User', + }); + ctx.importMap.set('src/app.ts', new Set(['src/user.ts'])); + + const result = resolveStaticCall('User', 'src/app.ts', ctx); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('ctor:User'); + }); + + it('returns class node when no constructor exists', () => { + ctx.symbols.add('src/user.ts', 'User', 'class:User', 'Class'); + ctx.importMap.set('src/app.ts', new Set(['src/user.ts'])); + + const result = resolveStaticCall('User', 'src/app.ts', ctx); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('class:User'); + }); + + it('returns null for non-class symbol', () => { + ctx.symbols.add('src/utils.ts', 'helper', 'func:helper', 'Function'); + ctx.importMap.set('src/app.ts', new Set(['src/utils.ts'])); + + const result = resolveStaticCall('helper', 'src/app.ts', ctx); + + expect(result).toBeNull(); + }); + + it('returns null when className does not exist', () => { + const result = resolveStaticCall('NonExistent', 'src/app.ts', ctx); + + expect(result).toBeNull(); + }); + + it('returns null when Constructor nodes lack ownerId', () => { + ctx.symbols.add('src/user.ts', 'User', 'class:User', 'Class'); + ctx.symbols.add('src/user.ts', 'User', 'ctor:User', 'Constructor', { + parameterCount: 1, + }); + ctx.importMap.set('src/app.ts', new Set(['src/user.ts'])); + + // Constructor lacks ownerId, so lookupMethodByOwner won't find it. + // resolveStaticCall detects Constructor nodes and returns null to + // let filterCallableCandidates handle the Constructor-vs-Class preference. + const result = resolveStaticCall('User', 'src/app.ts', ctx); + + expect(result).toBeNull(); + }); + + it('disambiguates constructor by arity', () => { + ctx.symbols.add('src/user.ts', 'User', 'class:User', 'Class'); + ctx.symbols.add('src/user.ts', 'User', 'ctor:User:0', 'Constructor', { + parameterCount: 0, + returnType: 'User', + ownerId: 'class:User', + }); + ctx.symbols.add('src/user.ts', 'User', 'ctor:User:2', 'Constructor', { + parameterCount: 2, + returnType: 'User', + ownerId: 'class:User', + }); + ctx.importMap.set('src/app.ts', new Set(['src/user.ts'])); + + const result = resolveStaticCall('User', 'src/app.ts', ctx, 2); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('ctor:User:2'); + }); + + it('returns correct confidence tier for import-scoped class', () => { + ctx.symbols.add('src/user.ts', 'User', 'class:User', 'Class'); + ctx.importMap.set('src/app.ts', new Set(['src/user.ts'])); + + const result = resolveStaticCall('User', 'src/app.ts', ctx); + + expect(result).not.toBeNull(); + expect(result!.confidence).toBe(0.9); // import-scoped tier + expect(result!.reason).toBe('import-resolved'); + }); + + it('returns correct confidence tier for same-file class', () => { + ctx.symbols.add('src/app.ts', 'User', 'class:User', 'Class'); + + const result = resolveStaticCall('User', 'src/app.ts', ctx); + + expect(result).not.toBeNull(); + expect(result!.confidence).toBe(0.95); // same-file tier + expect(result!.reason).toBe('same-file'); + }); + + it('returns null for ambiguous homonym classes without constructor', () => { + ctx.symbols.add('src/a.ts', 'User', 'class:a:User', 'Class'); + ctx.symbols.add('src/b.ts', 'User', 'class:b:User', 'Class'); + ctx.importMap.set('src/app.ts', new Set(['src/a.ts', 'src/b.ts'])); + + const result = resolveStaticCall('User', 'src/app.ts', ctx); + + // Two classes with same name — ambiguous, should return null + expect(result).toBeNull(); + }); + + it('routes through resolveCallTarget for constructor callForm', () => { + ctx.symbols.add('src/user.ts', 'User', 'class:User', 'Class'); + ctx.importMap.set('src/app.ts', new Set(['src/user.ts'])); + + const result = _resolveCallTargetForTesting( + { + calledName: 'User', + callForm: 'constructor', + }, + 'src/app.ts', + ctx, + ); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('class:User'); + }); + + it('routes through resolveCallTarget for free-form call targeting a class (Swift/Kotlin)', () => { + ctx.symbols.add('src/user.swift', 'User', 'class:User', 'Class'); + ctx.importMap.set('src/app.swift', new Set(['src/user.swift'])); + + const result = _resolveCallTargetForTesting( + { + calledName: 'User', + callForm: 'free', + }, + 'src/app.swift', + ctx, + ); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('class:User'); + }); + + it('reuses the pre-computed tiered result instead of calling ctx.resolve twice', () => { + ctx.symbols.add('src/user.ts', 'User', 'class:User', 'Class'); + ctx.symbols.add('src/user.ts', 'User', 'ctor:User', 'Constructor', { + returnType: 'User', + ownerId: 'class:User', + }); + ctx.importMap.set('src/app.ts', new Set(['src/user.ts'])); + + // Spy on ctx.resolve to prove the override short-circuits the second lookup. + const originalResolve = ctx.resolve.bind(ctx); + let resolveCallCount = 0; + ctx.resolve = ((name: string, fromFile: string) => { + resolveCallCount++; + return originalResolve(name, fromFile); + }) as typeof ctx.resolve; + + const tieredOverride = originalResolve('User', 'src/app.ts'); + expect(tieredOverride).not.toBeNull(); + resolveCallCount = 0; // reset after the setup call + + const result = resolveStaticCall('User', 'src/app.ts', ctx, undefined, tieredOverride!); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('ctor:User'); + expect(resolveCallCount).toBe(0); // ctx.resolve must not have been called again + }); + + it('routes through resolveCallTarget for Java constructor call (new User())', () => { + ctx.symbols.add('src/User.java', 'User', 'class:java:User', 'Class'); + ctx.symbols.add('src/User.java', 'User', 'ctor:java:User', 'Constructor', { + returnType: 'User', + ownerId: 'class:java:User', + }); + ctx.importMap.set('src/App.java', new Set(['src/User.java'])); + + const result = _resolveCallTargetForTesting( + { + calledName: 'User', + callForm: 'constructor', + }, + 'src/App.java', + ctx, + ); + + expect(result).not.toBeNull(); + // Prefers Constructor node over Class node when ownerId is present. + expect(result!.nodeId).toBe('ctor:java:User'); + }); + + it('routes through resolveCallTarget for Python free-form constructor (User())', () => { + ctx.symbols.add('models/user.py', 'User', 'class:py:User', 'Class'); + ctx.importMap.set('app.py', new Set(['models/user.py'])); + + const result = _resolveCallTargetForTesting( + { + calledName: 'User', + callForm: 'free', + }, + 'app.py', + ctx, + ); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('class:py:User'); + }); + + it('routes through resolveCallTarget for Kotlin free-form constructor (User())', () => { + ctx.symbols.add('src/User.kt', 'User', 'class:kt:User', 'Class'); + ctx.importMap.set('src/App.kt', new Set(['src/User.kt'])); + + const result = _resolveCallTargetForTesting( + { + calledName: 'User', + callForm: 'free', + }, + 'src/App.kt', + ctx, + ); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('class:kt:User'); + }); + + // ------------------------------------------------------------------------- + // Instantiability guard (Codex review follow-up, plan 2026-04-09-002): + // The step-5 class-node fallback must only return instantiable kinds + // (Class / Struct / Record). Interface / Trait / Impl / Enum targets are + // null-routed to prevent false CALLS edges to non-instantiable nodes. + // ------------------------------------------------------------------------- + + it('returns a Struct node when no constructor exists (positive regression guard)', () => { + ctx.symbols.add('src/user.rs', 'User', 'struct:User', 'Struct'); + ctx.importMap.set('src/app.rs', new Set(['src/user.rs'])); + + const result = resolveStaticCall('User', 'src/app.rs', ctx); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('struct:User'); + }); + + it('returns a Record node when no constructor exists (positive regression guard)', () => { + ctx.symbols.add('src/User.cs', 'User', 'record:User', 'Record'); + ctx.importMap.set('src/App.cs', new Set(['src/User.cs'])); + + const result = resolveStaticCall('User', 'src/App.cs', ctx); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('record:User'); + }); + + it('null-routes when the sole candidate is an Interface (Java/C#/TS)', () => { + // Constructor-shaped call on an interface name — not legal source, but + // the resolver must refuse to emit a CALLS edge to a non-instantiable node. + ctx.symbols.add('src/validator.java', 'IValidator', 'iface:IValidator', 'Interface'); + ctx.importMap.set('src/app.java', new Set(['src/validator.java'])); + + const result = resolveStaticCall('IValidator', 'src/app.java', ctx); + + expect(result).toBeNull(); + }); + + it('null-routes when the sole candidate is a Trait (PHP/Rust/Scala)', () => { + // PHP `HasTimestamps` trait — not instantiable via constructor syntax. + ctx.symbols.add('src/timestamps.php', 'HasTimestamps', 'trait:HasTimestamps', 'Trait'); + ctx.importMap.set('src/model.php', new Set(['src/timestamps.php'])); + + const result = resolveStaticCall('HasTimestamps', 'src/model.php', ctx); + + expect(result).toBeNull(); + }); + + it('null-routes when the sole candidate is a Rust Trait (Display)', () => { + ctx.symbols.add('src/fmt.rs', 'Display', 'trait:rs:Display', 'Trait'); + ctx.importMap.set('src/app.rs', new Set(['src/fmt.rs'])); + + const result = resolveStaticCall('Display', 'src/app.rs', ctx); + + expect(result).toBeNull(); + }); + + it('prefers the Struct over the Impl when both share the same name and file (Rust shadowing)', () => { + // Rust `impl User { ... }` alongside `struct User { ... }` in the same file. + // Same-file tier returns both via lookupExactAll, both pass CLASS_LIKE_TYPES, + // but the instantiability filter must strip the Impl so the Struct wins. + ctx.symbols.add('src/user.rs', 'User', 'struct:rs:User', 'Struct'); + ctx.symbols.add('src/user.rs', 'User', 'impl:rs:User', 'Impl'); + + const result = resolveStaticCall('User', 'src/user.rs', ctx); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('struct:rs:User'); + }); + + it('null-routes when the sole candidate is a Rust Impl block (no Struct present)', () => { + // Pathological extractor output: only the Impl survives tier resolution. + // The instantiability filter must reject it rather than emit a wrong edge. + ctx.symbols.add('src/user.rs', 'User', 'impl:rs:User', 'Impl'); + + const result = resolveStaticCall('User', 'src/user.rs', ctx); + + expect(result).toBeNull(); + }); + + it('still returns an explicit Constructor even when the owner is an Impl (step-3 preservation)', () => { + // Step 3 (lookupMethodByOwner walk) must not be affected by the step-5 + // tightening — a legitimate Constructor node owned by an Impl in a Rust + // extractor still resolves correctly. The Struct is also present so that + // step-1's lookupClassByName pre-check succeeds (Impl alone isn't in the + // classByName index). + ctx.symbols.add('src/user.rs', 'User', 'struct:rs:User', 'Struct'); + ctx.symbols.add('src/user.rs', 'User', 'impl:rs:User', 'Impl'); + ctx.symbols.add('src/user.rs', 'User', 'ctor:rs:User', 'Constructor', { + returnType: 'User', + ownerId: 'impl:rs:User', + }); + ctx.importMap.set('src/app.rs', new Set(['src/user.rs'])); + + const result = resolveStaticCall('User', 'src/app.rs', ctx); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('ctor:rs:User'); + }); + + it('routes through resolveCallTarget and null-routes Interface constructor-shaped calls', () => { + ctx.symbols.add('src/validator.java', 'IValidator', 'iface:IValidator', 'Interface'); + ctx.importMap.set('src/app.java', new Set(['src/validator.java'])); + + const result = _resolveCallTargetForTesting( + { + calledName: 'IValidator', + callForm: 'constructor', + }, + 'src/app.java', + ctx, + ); + + // Full cascade: S0 → resolveStaticCall → step-5 instantiability filter → null. + // If any downstream path silently re-introduces the wrong edge, this fails. + expect(result).toBeNull(); + }); + + it('routes through resolveCallTarget and null-routes Trait free-form calls', () => { + ctx.symbols.add('src/timestamps.php', 'HasTimestamps', 'trait:HasTimestamps', 'Trait'); + ctx.importMap.set('src/model.php', new Set(['src/timestamps.php'])); + + const result = _resolveCallTargetForTesting( + { + calledName: 'HasTimestamps', + callForm: 'free', + }, + 'src/model.php', + ctx, + ); + + expect(result).toBeNull(); + }); + + it('routes Record free-form constructor call through S0 (C# record / Kotlin data class)', () => { + // Verifies that `freeFormHasClassTarget` triggers S0 for Record candidates. + // Before the alignment fix, `Record` was absent from the trigger `.some()`, + // so S0 was bypassed and Record free-form calls fell through to the + // constructor-form retry path. This test would have silently passed with + // the old (wasteful) code path — with the fix, S0 resolves it directly. + ctx.symbols.add('src/User.cs', 'User', 'record:cs:User', 'Record'); + ctx.importMap.set('src/App.cs', new Set(['src/User.cs'])); + + const result = _resolveCallTargetForTesting( + { + calledName: 'User', + callForm: 'free', + }, + 'src/App.cs', + ctx, + ); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('record:cs:User'); + }); + + it('threads argCount through resolveCallTarget → S0 → resolveStaticCall for arity disambiguation', () => { + // Regression guard: if call.argCount were ever dropped at the S0 call + // site, the 2-arg constructor would resolve to the 0-arg overload (or + // return null via ambiguity). This test fails in either case. + ctx.symbols.add('src/user.ts', 'User', 'class:User', 'Class'); + ctx.symbols.add('src/user.ts', 'User', 'ctor:User:0', 'Constructor', { + parameterCount: 0, + returnType: 'User', + ownerId: 'class:User', + }); + ctx.symbols.add('src/user.ts', 'User', 'ctor:User:2', 'Constructor', { + parameterCount: 2, + returnType: 'User', + ownerId: 'class:User', + }); + ctx.importMap.set('src/app.ts', new Set(['src/user.ts'])); + + const result = _resolveCallTargetForTesting( + { + calledName: 'User', + callForm: 'constructor', + argCount: 2, + }, + 'src/app.ts', + ctx, + ); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('ctor:User:2'); + }); +}); From 338cb01ee06ae87aad2f959053fa1421e07f22e0 Mon Sep 17 00:00:00 2001 From: JaysonAlbert Date: Fri, 10 Apr 2026 00:40:24 +0800 Subject: [PATCH 11/12] [codex] fix large repository graph loading (#732) * fix(web): stream large graph responses * fix(server): harden graph streaming * fix(ci): stabilize graph loading coverage --------- Co-authored-by: gfwangjie --- gitnexus-web/e2e/server-connect.spec.ts | 50 ++- gitnexus-web/src/components/StatusBar.tsx | 2 +- gitnexus-web/src/services/backend-client.ts | 67 +++- .../test/unit/server-connection.test.ts | 138 +++++++- gitnexus/src/core/lbug/lbug-adapter.ts | 28 ++ gitnexus/src/server/api.ts | 308 +++++++++++++++--- .../test/unit/api-graph-streaming.test.ts | 235 +++++++++++++ 7 files changed, 746 insertions(+), 82 deletions(-) create mode 100644 gitnexus/test/unit/api-graph-streaming.test.ts diff --git a/gitnexus-web/e2e/server-connect.spec.ts b/gitnexus-web/e2e/server-connect.spec.ts index eb241da10..0705b3a27 100644 --- a/gitnexus-web/e2e/server-connect.spec.ts +++ b/gitnexus-web/e2e/server-connect.spec.ts @@ -1,4 +1,4 @@ -import { test, expect, type TestInfo } from '@playwright/test'; +import { test, expect } from '@playwright/test'; /** * E2E tests for the GitNexus web UI — exploring view features. @@ -58,36 +58,41 @@ test.beforeAll(async () => { * For these tests we require at least one indexed repo, so pick the first * landing card when present and then wait for the exploring view. */ -async function waitForGraphLoaded(page: import('@playwright/test').Page, testInfo: TestInfo) { +async function waitForGraphLoaded(page: import('@playwright/test').Page) { await page.goto('/'); - const landingCard = page.locator('[data-testid="landing-repo-card"]').first(); + const landingCards = page.locator('[data-testid="landing-repo-card"]'); + const preferredLandingCard = landingCards + .filter({ hasText: /GitNexus|local-integration/ }) + .first(); try { - await landingCard.waitFor({ state: 'visible', timeout: 15_000 }); + await landingCards.first().waitFor({ state: 'visible', timeout: 15_000 }); + const landingCard = + (await preferredLandingCard.count()) > 0 ? preferredLandingCard : landingCards.first(); await landingCard.click(); } catch { // Landing screen may not appear (e.g. ?server auto-connect) } - await expect(page.locator('[data-testid="status-ready"]')).toBeVisible({ timeout: 30_000 }); - await expect(page.getByText(/\d+ nodes/).first()).toBeVisible(); - await page.screenshot({ path: testInfo.outputPath('graph-loaded.png') }); + const statusBar = page.getByRole('contentinfo'); + await expect(statusBar.getByText('Ready', { exact: true })).toBeVisible({ timeout: 45_000 }); + await expect(statusBar).toContainText(/nodes/, { + timeout: 20_000, + }); } test.describe('Server Connection & Graph Loading', () => { - test('selects a repo from landing and loads graph', async ({ page }, testInfo) => { - await waitForGraphLoaded(page, testInfo); - await page.screenshot({ path: testInfo.outputPath('graph-loaded-full.png'), fullPage: true }); + test('selects a repo from landing and loads graph', async ({ page }) => { + await waitForGraphLoaded(page); }); }); test.describe('Nexus AI', () => { - test('panel opens and agent initializes without error', async ({ page }, testInfo) => { - await waitForGraphLoaded(page, testInfo); + test('panel opens and agent initializes without error', async ({ page }) => { + await waitForGraphLoaded(page); await page.getByRole('button', { name: 'Nexus AI' }).click(); await expect(page.getByText('Ask me anything')).toBeVisible({ timeout: 15_000 }); - await page.screenshot({ path: testInfo.outputPath('nexus-ai-panel.png'), fullPage: true }); const errorBanner = page.getByText('Database not ready'); expect(await errorBanner.isVisible().catch(() => false)).toBe(false); @@ -95,8 +100,8 @@ test.describe('Nexus AI', () => { }); test.describe('Processes Panel', () => { - test('shows process list and View button works', async ({ page }, testInfo) => { - await waitForGraphLoaded(page, testInfo); + test('shows process list and View button works', async ({ page }) => { + await waitForGraphLoaded(page); await page.getByRole('button', { name: 'Nexus AI' }).click(); await page.getByText('Processes').click(); @@ -104,7 +109,6 @@ test.describe('Processes Panel', () => { await expect(page.locator('[data-testid="process-list-loaded"]')).toBeVisible({ timeout: 15_000, }); - await page.screenshot({ path: testInfo.outputPath('processes-panel.png'), fullPage: true }); const processRow = page.locator('[data-testid="process-row"]').first(); await expect(processRow).toBeVisible({ timeout: 10_000 }); @@ -114,14 +118,10 @@ test.describe('Processes Panel', () => { await viewBtn.waitFor({ state: 'visible', timeout: 5_000 }); await viewBtn.click(); await expect(page.locator('[data-testid="process-modal"]')).toBeVisible({ timeout: 5_000 }); - await page.screenshot({ - path: testInfo.outputPath('process-view-clicked.png'), - fullPage: true, - }); }); - test('lightbulb highlights nodes in graph', async ({ page }, testInfo) => { - await waitForGraphLoaded(page, testInfo); + test('lightbulb highlights nodes in graph', async ({ page }) => { + await waitForGraphLoaded(page); await page.getByRole('button', { name: 'Nexus AI' }).click(); await page.getByText('Processes').click(); @@ -137,13 +137,12 @@ test.describe('Processes Panel', () => { await lightbulb.waitFor({ state: 'visible', timeout: 5_000 }); await lightbulb.click(); await expect(processRow).toHaveClass(/bg-amber-950/, { timeout: 5_000 }); - await page.screenshot({ path: testInfo.outputPath('after-highlight.png'), fullPage: true }); }); }); test.describe('Turn Off All Highlights', () => { - test('selecting a node dims others, button clears it', async ({ page }, testInfo) => { - await waitForGraphLoaded(page, testInfo); + test('selecting a node dims others, button clears it', async ({ page }) => { + await waitForGraphLoaded(page); await expect(page.locator('canvas').first()).toBeVisible({ timeout: 10_000 }); @@ -160,6 +159,5 @@ test.describe('Turn Off All Highlights', () => { await expect(highlightToggle).toHaveAttribute('title', 'Turn on AI highlights', { timeout: 5_000, }); - await page.screenshot({ path: testInfo.outputPath('highlights-cleared.png'), fullPage: true }); }); }); diff --git a/gitnexus-web/src/components/StatusBar.tsx b/gitnexus-web/src/components/StatusBar.tsx index a618c3c1c..7468072fa 100644 --- a/gitnexus-web/src/components/StatusBar.tsx +++ b/gitnexus-web/src/components/StatusBar.tsx @@ -64,7 +64,7 @@ export const StatusBar = () => { {/* Right - Stats */} -
+
{graph && ( <> {nodeCount} nodes diff --git a/gitnexus-web/src/services/backend-client.ts b/gitnexus-web/src/services/backend-client.ts index 256219eb6..2c04b66dd 100644 --- a/gitnexus-web/src/services/backend-client.ts +++ b/gitnexus-web/src/services/backend-client.ts @@ -404,13 +404,18 @@ export const fetchGraph = async ( onProgress?: (downloaded: number, total: number | null) => void; }, ): Promise<{ nodes: GraphNode[]; relationships: GraphRelationship[] }> => { - const params = [repoParam(repo), opts?.includeContent ? 'includeContent=true' : ''] + const params = [repoParam(repo), opts?.includeContent ? 'includeContent=true' : '', 'stream=true'] .filter(Boolean) .join('&'); const url = `${_backendUrl}/api/graph${params ? `?${params}` : ''}`; const response = await fetchWithTimeout(url, { signal: opts?.signal }, 60_000); await assertOk(response); + const contentType = response.headers.get('Content-Type') || ''; + if (contentType.includes('application/x-ndjson')) { + return parseNdjsonGraphResponse(response, opts?.onProgress); + } + if (!opts?.onProgress || !response.body) { return response.json() as Promise<{ nodes: GraphNode[]; relationships: GraphRelationship[] }>; } @@ -439,6 +444,66 @@ export const fetchGraph = async ( return JSON.parse(new TextDecoder().decode(combined)); }; +const parseNdjsonGraphResponse = async ( + response: Response, + onProgress?: (downloaded: number, total: number | null) => void, +): Promise<{ nodes: GraphNode[]; relationships: GraphRelationship[] }> => { + if (!response.body) { + throw new BackendError('No response body', response.status, 'server'); + } + + const contentLength = response.headers.get('Content-Length'); + const total = contentLength ? parseInt(contentLength, 10) : null; + const reader = response.body.getReader(); + const decoder = new TextDecoder(); + const nodes: GraphNode[] = []; + const relationships: GraphRelationship[] = []; + let buffer = ''; + let downloaded = 0; + + const parseLine = (line: string) => { + const trimmed = line.trim(); + if (!trimmed) return; + + const record = JSON.parse(trimmed) as + | { type: 'node'; data: GraphNode } + | { type: 'relationship'; data: GraphRelationship } + | { type: 'error'; error: string }; + + if (record.type === 'node') { + nodes.push(record.data); + return; + } + if (record.type === 'relationship') { + relationships.push(record.data); + return; + } + if (record.type === 'error') { + throw new BackendError(record.error, response.status || 500, 'server'); + } + }; + + while (true) { + const { done, value } = await reader.read(); + if (done) break; + + downloaded += value.length; + onProgress?.(downloaded, total); + buffer += decoder.decode(value, { stream: true }); + + const lines = buffer.split('\n'); + buffer = lines.pop() || ''; + for (const line of lines) { + parseLine(line); + } + } + + buffer += decoder.decode(); + parseLine(buffer); + + return { nodes, relationships }; +}; + /** Execute a Cypher query. Returns rows. */ export const runQuery = async ( cypher: string, diff --git a/gitnexus-web/test/unit/server-connection.test.ts b/gitnexus-web/test/unit/server-connection.test.ts index a818adb13..f5ee43c53 100644 --- a/gitnexus-web/test/unit/server-connection.test.ts +++ b/gitnexus-web/test/unit/server-connection.test.ts @@ -1,5 +1,5 @@ -import { describe, expect, it } from 'vitest'; -import { normalizeServerUrl } from '../../src/services/backend-client'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { fetchGraph, normalizeServerUrl, setBackendUrl } from '../../src/services/backend-client'; describe('normalizeServerUrl', () => { it('adds http:// to localhost', () => { @@ -31,3 +31,137 @@ describe('normalizeServerUrl', () => { expect(normalizeServerUrl('https://gitnexus.example.com')).toBe('https://gitnexus.example.com'); }); }); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('fetchGraph', () => { + it('requests streamed graph responses from the backend', async () => { + setBackendUrl('http://localhost:4747'); + + const fetchMock = vi.fn().mockResolvedValue( + new Response('{"nodes":[],"relationships":[]}', { + status: 200, + headers: { + 'Content-Type': 'application/json', + }, + }), + ); + vi.stubGlobal('fetch', fetchMock); + + await fetchGraph('big-repo'); + + expect(fetchMock).toHaveBeenCalledWith( + expect.stringContaining('/api/graph?repo=big-repo&stream=true'), + expect.any(Object), + ); + }); + + it('parses NDJSON graph streams incrementally', async () => { + setBackendUrl('http://localhost:4747'); + + const encoder = new TextEncoder(); + const stream = new ReadableStream({ + start(controller) { + controller.enqueue( + encoder.encode( + [ + '{"type":"node","data":{"id":"File:src/app.ts","label":"File","properties":{"name":"app.ts","filePath":"src/app.ts"}}}\n', + '{"type":"relationship","data":{"id":"File:src/app.ts_CONTAINS_Function:src/app.ts:main","type":"CONTAINS","sourceId":"File:src/app.ts","targetId":"Function:src/app.ts:main"}}\n', + ].join(''), + ), + ); + controller.close(); + }, + }); + + vi.stubGlobal( + 'fetch', + vi.fn().mockResolvedValue( + new Response(stream, { + status: 200, + headers: { + 'Content-Type': 'application/x-ndjson', + }, + }), + ), + ); + + const progress = vi.fn(); + const result = await fetchGraph('big-repo', { onProgress: progress }); + + expect(result.nodes).toHaveLength(1); + expect(result.relationships).toHaveLength(1); + expect(result.nodes[0].id).toBe('File:src/app.ts'); + expect(result.relationships[0].type).toBe('CONTAINS'); + expect(progress).toHaveBeenCalled(); + }); + + it('parses NDJSON graph lines split across chunks', async () => { + setBackendUrl('http://localhost:4747'); + + const encoder = new TextEncoder(); + const stream = new ReadableStream({ + start(controller) { + controller.enqueue( + encoder.encode( + '{"type":"node","data":{"id":"File:src/app.ts","label":"File","properties":{"name":"app.ts"', + ), + ); + controller.enqueue( + encoder.encode( + ',"filePath":"src/app.ts"}}}\n{"type":"relationship","data":{"id":"File:src/app.ts_CONTAINS_Function:src/app.ts:main","type":"CONTAINS","sourceId":"File:src/app.ts","targetId":"Function:src/app.ts:main"}}\n', + ), + ); + controller.close(); + }, + }); + + vi.stubGlobal( + 'fetch', + vi.fn().mockResolvedValue( + new Response(stream, { + status: 200, + headers: { + 'Content-Type': 'application/x-ndjson', + }, + }), + ), + ); + + const result = await fetchGraph('big-repo'); + + expect(result.nodes).toHaveLength(1); + expect(result.relationships).toHaveLength(1); + expect(result.nodes[0].properties.filePath).toBe('src/app.ts'); + }); + + it('throws backend errors emitted in the NDJSON stream', async () => { + setBackendUrl('http://localhost:4747'); + + const encoder = new TextEncoder(); + const stream = new ReadableStream({ + start(controller) { + controller.enqueue(encoder.encode('{"type":"error","error":"stream failed"}\n')); + controller.close(); + }, + }); + + vi.stubGlobal( + 'fetch', + vi.fn().mockResolvedValue( + new Response(stream, { + status: 200, + headers: { + 'Content-Type': 'application/x-ndjson', + }, + }), + ), + ); + + await expect(fetchGraph('big-repo')).rejects.toMatchObject({ + message: 'stream failed', + }); + }); +}); diff --git a/gitnexus/src/core/lbug/lbug-adapter.ts b/gitnexus/src/core/lbug/lbug-adapter.ts index 1363257d5..88a6e9bba 100644 --- a/gitnexus/src/core/lbug/lbug-adapter.ts +++ b/gitnexus/src/core/lbug/lbug-adapter.ts @@ -637,6 +637,34 @@ export const executeQuery = async (cypher: string): Promise => { return rows; }; +export const streamQuery = async ( + cypher: string, + onRow: (row: any) => void | Promise, +): Promise => { + if (!conn) { + throw new Error('LadybugDB not initialized. Call initLbug first.'); + } + + const queryResult = await conn.query(cypher); + const result = Array.isArray(queryResult) ? queryResult[0] : queryResult; + let rowCount = 0; + + try { + while (await result.hasNext()) { + const row = await result.getNext(); + await onRow(row); + rowCount++; + } + return rowCount; + } finally { + try { + await result.close(); + } catch { + // Best-effort cleanup only. + } + } +}; + /** * Execute a single parameterized query (prepare/execute pattern). * Prevents Cypher injection by binding values as parameters. diff --git a/gitnexus/src/server/api.ts b/gitnexus/src/server/api.ts index 8422af6ba..8111c287b 100644 --- a/gitnexus/src/server/api.ts +++ b/gitnexus/src/server/api.ts @@ -18,6 +18,7 @@ import { executeQuery, executePrepared, executeWithReusedStatement, + streamQuery, closeLbug, withLbugDb, } from '../core/lbug/lbug-adapter.js'; @@ -105,75 +106,228 @@ export const isAllowedOrigin = (origin: string | undefined): boolean => { return false; }; +type GraphStreamRecord = + | { type: 'node'; data: GraphNode } + | { type: 'relationship'; data: GraphRelationship } + | { type: 'error'; error: string }; + +export class ClientDisconnectedError extends Error { + constructor() { + super('Client disconnected during graph stream'); + this.name = 'ClientDisconnectedError'; + } +} + +export const isIgnorableGraphQueryError = (err: unknown): boolean => { + const message = err instanceof Error ? err.message : String(err); + return ( + message.includes('does not exist') || + message.includes('not found') || + message.includes('No table named') + ); +}; + +const ensureStreamIsWritable = (res: express.Response, signal?: AbortSignal): void => { + if (signal?.aborted || res.destroyed || res.writableEnded) { + throw new ClientDisconnectedError(); + } +}; + +const waitForDrain = async (res: express.Response, signal?: AbortSignal): Promise => { + ensureStreamIsWritable(res, signal); + + await new Promise((resolve, reject) => { + const cleanup = () => { + res.off('drain', onDrain); + res.off('close', onClose); + signal?.removeEventListener('abort', onAbort); + }; + + const onDrain = () => { + cleanup(); + resolve(); + }; + const onClose = () => { + cleanup(); + reject(new ClientDisconnectedError()); + }; + const onAbort = () => { + cleanup(); + reject(new ClientDisconnectedError()); + }; + + res.once('drain', onDrain); + res.once('close', onClose); + signal?.addEventListener('abort', onAbort, { once: true }); + + if (signal?.aborted || res.destroyed || res.writableEnded) { + onAbort(); + } + }); + + ensureStreamIsWritable(res, signal); +}; + +const isClientDisconnectWriteError = (err: unknown): boolean => { + if (!(err instanceof Error)) return false; + return ( + (err as NodeJS.ErrnoException).code === 'ERR_STREAM_DESTROYED' || + (err as NodeJS.ErrnoException).code === 'EPIPE' || + (err as NodeJS.ErrnoException).code === 'ECONNRESET' || + err.message.includes('write after end') + ); +}; + +export const writeNdjsonRecord = async ( + res: express.Response, + record: GraphStreamRecord, + signal?: AbortSignal, +): Promise => { + ensureStreamIsWritable(res, signal); + + try { + const canContinue = res.write(JSON.stringify(record) + '\n'); + if (!canContinue) { + await waitForDrain(res, signal); + } + } catch (err) { + if (isClientDisconnectWriteError(err)) { + throw new ClientDisconnectedError(); + } + throw err; + } +}; + const buildGraph = async ( includeContent = false, ): Promise<{ nodes: GraphNode[]; relationships: GraphRelationship[] }> => { const nodes: GraphNode[] = []; for (const table of NODE_TABLES) { try { - let query = ''; - if (table === 'File') { - query = includeContent - ? `MATCH (n:File) RETURN n.id AS id, n.name AS name, n.filePath AS filePath, n.content AS content` - : `MATCH (n:File) RETURN n.id AS id, n.name AS name, n.filePath AS filePath`; - } else if (table === 'Folder') { - query = `MATCH (n:Folder) RETURN n.id AS id, n.name AS name, n.filePath AS filePath`; - } else if (table === 'Community') { - query = `MATCH (n:Community) RETURN n.id AS id, n.label AS label, n.heuristicLabel AS heuristicLabel, n.cohesion AS cohesion, n.symbolCount AS symbolCount`; - } else if (table === 'Process') { - query = `MATCH (n:Process) RETURN n.id AS id, n.label AS label, n.heuristicLabel AS heuristicLabel, n.processType AS processType, n.stepCount AS stepCount, n.communities AS communities, n.entryPointId AS entryPointId, n.terminalId AS terminalId`; - } else { - query = includeContent - ? `MATCH (n:${table}) RETURN n.id AS id, n.name AS name, n.filePath AS filePath, n.startLine AS startLine, n.endLine AS endLine, n.content AS content` - : `MATCH (n:${table}) RETURN n.id AS id, n.name AS name, n.filePath AS filePath, n.startLine AS startLine, n.endLine AS endLine`; - } - - const rows = await executeQuery(query); + const rows = await executeQuery(getNodeQuery(table, includeContent)); for (const row of rows) { - nodes.push({ - id: row.id ?? row[0], - label: table as GraphNode['label'], - properties: { - name: row.name ?? row.label ?? row[1], - filePath: row.filePath ?? row[2], - startLine: row.startLine, - endLine: row.endLine, - content: includeContent ? row.content : undefined, - heuristicLabel: row.heuristicLabel, - cohesion: row.cohesion, - symbolCount: row.symbolCount, - processType: row.processType, - stepCount: row.stepCount, - communities: row.communities, - entryPointId: row.entryPointId, - terminalId: row.terminalId, - } as GraphNode['properties'], - }); + nodes.push(mapGraphNodeRow(table, row, includeContent)); + } + } catch (err) { + if (!isIgnorableGraphQueryError(err)) { + throw err; } - } catch { - // ignore empty tables } } const relationships: GraphRelationship[] = []; - const relRows = await executeQuery( - `MATCH (a)-[r:CodeRelation]->(b) RETURN a.id AS sourceId, b.id AS targetId, r.type AS type, r.confidence AS confidence, r.reason AS reason, r.step AS step`, - ); + const relRows = await executeQuery(GRAPH_RELATIONSHIP_QUERY); for (const row of relRows) { - relationships.push({ - id: `${row.sourceId}_${row.type}_${row.targetId}`, - type: row.type, - sourceId: row.sourceId, - targetId: row.targetId, - confidence: row.confidence, - reason: row.reason, - step: row.step, - }); + relationships.push(mapGraphRelationshipRow(row)); } return { nodes, relationships }; }; +const GRAPH_RELATIONSHIP_QUERY = + `MATCH (a)-[r:CodeRelation]->(b) RETURN a.id AS sourceId, b.id AS targetId, ` + + `r.type AS type, r.confidence AS confidence, r.reason AS reason, r.step AS step`; + +const quoteNodeTable = (table: string): string => `\`${table.replace(/`/g, '``')}\``; + +const getNodeQuery = (table: string, includeContent: boolean): string => { + const tableLabel = quoteNodeTable(table); + + if (table === 'File') { + return includeContent + ? `MATCH (n:${tableLabel}) RETURN n.id AS id, n.name AS name, n.filePath AS filePath, n.content AS content` + : `MATCH (n:${tableLabel}) RETURN n.id AS id, n.name AS name, n.filePath AS filePath`; + } + if (table === 'Folder') { + return `MATCH (n:${tableLabel}) RETURN n.id AS id, n.name AS name, n.filePath AS filePath`; + } + if (table === 'Community') { + return `MATCH (n:${tableLabel}) RETURN n.id AS id, n.label AS label, n.heuristicLabel AS heuristicLabel, n.cohesion AS cohesion, n.symbolCount AS symbolCount`; + } + if (table === 'Process') { + return `MATCH (n:${tableLabel}) RETURN n.id AS id, n.label AS label, n.heuristicLabel AS heuristicLabel, n.processType AS processType, n.stepCount AS stepCount, n.communities AS communities, n.entryPointId AS entryPointId, n.terminalId AS terminalId`; + } + if (table === 'Route') { + return `MATCH (n:${tableLabel}) RETURN n.id AS id, n.name AS name, n.filePath AS filePath, n.responseKeys AS responseKeys, n.errorKeys AS errorKeys, n.middleware AS middleware`; + } + if (table === 'Tool') { + return `MATCH (n:${tableLabel}) RETURN n.id AS id, n.name AS name, n.filePath AS filePath, n.description AS description`; + } + return includeContent + ? `MATCH (n:${tableLabel}) RETURN n.id AS id, n.name AS name, n.filePath AS filePath, n.startLine AS startLine, n.endLine AS endLine, n.content AS content` + : `MATCH (n:${tableLabel}) RETURN n.id AS id, n.name AS name, n.filePath AS filePath, n.startLine AS startLine, n.endLine AS endLine`; +}; + +const mapGraphNodeRow = (table: string, row: any, includeContent: boolean): GraphNode => ({ + id: row.id ?? row[0], + label: table as GraphNode['label'], + properties: { + name: row.name ?? row.label ?? row[1], + filePath: row.filePath ?? row[2], + startLine: row.startLine, + endLine: row.endLine, + content: includeContent ? row.content : undefined, + responseKeys: row.responseKeys, + errorKeys: row.errorKeys, + middleware: row.middleware, + heuristicLabel: row.heuristicLabel, + cohesion: row.cohesion, + symbolCount: row.symbolCount, + description: row.description, + processType: row.processType, + stepCount: row.stepCount, + communities: row.communities, + entryPointId: row.entryPointId, + terminalId: row.terminalId, + } as GraphNode['properties'], +}); + +const mapGraphRelationshipRow = (row: any): GraphRelationship => ({ + id: `${row.sourceId}_${row.type}_${row.targetId}`, + type: row.type, + sourceId: row.sourceId, + targetId: row.targetId, + confidence: row.confidence, + reason: row.reason, + step: row.step, +}); + +export const streamGraphNdjson = async ( + res: express.Response, + includeContent = false, + signal?: AbortSignal, +): Promise => { + for (const table of NODE_TABLES) { + try { + await streamQuery(getNodeQuery(table, includeContent), async (row) => { + await writeNdjsonRecord( + res, + { + type: 'node', + data: mapGraphNodeRow(table, row, includeContent), + }, + signal, + ); + }); + } catch (err) { + if (!isIgnorableGraphQueryError(err)) { + throw err; + } + } + } + + await streamQuery(GRAPH_RELATIONSHIP_QUERY, async (row) => { + await writeNdjsonRecord( + res, + { + type: 'relationship', + data: mapGraphRelationshipRow(row), + }, + signal, + ); + }); +}; + /** * Mount an SSE progress endpoint for a JobManager. * Handles: initial state, terminal events, heartbeat, event IDs, client disconnect. @@ -464,10 +618,60 @@ export const createServer = async (port: number, host: string = '127.0.0.1') => } const lbugPath = path.join(entry.storagePath, 'lbug'); const includeContent = req.query.includeContent === 'true'; + const stream = req.query.stream === 'true'; + + if (stream) { + const abortController = new AbortController(); + let responseFinished = false; + const markFinished = () => { + responseFinished = true; + }; + const abortStreaming = () => { + if (!responseFinished) { + abortController.abort(); + } + }; + + res.setHeader('Content-Type', 'application/x-ndjson; charset=utf-8'); + res.setHeader('Cache-Control', 'no-cache'); + res.flushHeaders(); + + req.once('aborted', abortStreaming); + res.once('finish', markFinished); + res.once('close', abortStreaming); + + try { + await withLbugDb(lbugPath, async () => + streamGraphNdjson(res, includeContent, abortController.signal), + ); + if (!abortController.signal.aborted && !res.writableEnded) { + res.end(); + } + } finally { + req.off('aborted', abortStreaming); + res.off('finish', markFinished); + res.off('close', abortStreaming); + } + return; + } + const graph = await withLbugDb(lbugPath, async () => buildGraph(includeContent)); res.json(graph); } catch (err: any) { - res.status(500).json({ error: err.message || 'Failed to build graph' }); + if (err instanceof ClientDisconnectedError) { + return; + } + const message = err.message || 'Failed to build graph'; + if (res.headersSent) { + try { + res.write(JSON.stringify({ type: 'error', error: message }) + '\n'); + } catch { + // Best-effort only after streaming has started. + } + res.end(); + return; + } + res.status(500).json({ error: message }); } }); diff --git a/gitnexus/test/unit/api-graph-streaming.test.ts b/gitnexus/test/unit/api-graph-streaming.test.ts new file mode 100644 index 000000000..0cc69833c --- /dev/null +++ b/gitnexus/test/unit/api-graph-streaming.test.ts @@ -0,0 +1,235 @@ +import { EventEmitter } from 'node:events'; +import { describe, expect, it, vi, beforeEach } from 'vitest'; + +const { lbugMocks } = vi.hoisted(() => ({ + lbugMocks: { + streamQuery: vi.fn(), + }, +})); + +vi.mock('../../src/core/lbug/lbug-adapter.js', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, ...lbugMocks }; +}); + +import { ClientDisconnectedError, streamGraphNdjson } from '../../src/server/api.js'; + +const createMockResponse = (writeImpl?: (chunk: string) => boolean) => { + const response = new EventEmitter() as any; + response.writableEnded = false; + response.destroyed = false; + response.write = vi.fn((chunk: string) => (writeImpl ? writeImpl(chunk) : true)); + return response; +}; + +describe('streamGraphNdjson', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('waits for drain when writes hit backpressure', async () => { + lbugMocks.streamQuery.mockImplementation( + async (query: string, onRow: (row: any) => Promise) => { + if (query.includes('MATCH (n:`File`)')) { + await onRow({ id: 'File:src/app.ts', name: 'app.ts', filePath: 'src/app.ts' }); + return 1; + } + if (query.includes('CodeRelation')) { + await onRow({ + sourceId: 'File:src/app.ts', + targetId: 'Function:src/app.ts:main', + type: 'CONTAINS', + }); + return 1; + } + return 0; + }, + ); + + const writes: string[] = []; + let firstWrite = true; + const response = createMockResponse((chunk) => { + writes.push(chunk); + if (firstWrite) { + firstWrite = false; + return false; + } + return true; + }); + + let settled = false; + const pending = streamGraphNdjson(response, false).then(() => { + settled = true; + }); + + await Promise.resolve(); + expect(writes).toHaveLength(1); + expect(settled).toBe(false); + + response.emit('drain'); + await pending; + + expect(writes).toHaveLength(2); + }); + + it('stops streaming when the client disconnects', async () => { + const controller = new AbortController(); + lbugMocks.streamQuery.mockImplementation( + async (query: string, onRow: (row: any) => Promise) => { + if (!query.includes('MATCH (n:`File`)')) { + return 0; + } + await onRow({ id: 'File:src/app.ts', name: 'app.ts', filePath: 'src/app.ts' }); + controller.abort(); + await onRow({ id: 'File:src/other.ts', name: 'other.ts', filePath: 'src/other.ts' }); + return 2; + }, + ); + + const response = createMockResponse(); + + await expect(streamGraphNdjson(response, false, controller.signal)).rejects.toBeInstanceOf( + ClientDisconnectedError, + ); + expect(response.write).toHaveBeenCalledTimes(1); + }); + + it('rethrows non-missing table errors', async () => { + lbugMocks.streamQuery.mockImplementation(async (query: string) => { + if (query.includes('MATCH (n:`File`)')) { + throw new Error('database unavailable'); + } + return 0; + }); + + const response = createMockResponse(); + await expect(streamGraphNdjson(response, false)).rejects.toThrow('database unavailable'); + }); + + it('ignores missing-table errors while continuing the stream', async () => { + lbugMocks.streamQuery.mockImplementation( + async (query: string, onRow: (row: any) => Promise) => { + if (query.includes('MATCH (n:`File`)')) { + throw new Error('Table File does not exist'); + } + if (query.includes('CodeRelation')) { + await onRow({ + sourceId: 'File:src/app.ts', + targetId: 'Function:src/app.ts:main', + type: 'CONTAINS', + }); + return 1; + } + return 0; + }, + ); + + const response = createMockResponse(); + await expect(streamGraphNdjson(response, false)).resolves.toBeUndefined(); + expect(response.write).toHaveBeenCalledTimes(1); + }); + + it('quotes node table names in generated Cypher queries', async () => { + lbugMocks.streamQuery.mockImplementation(async () => 0); + + const response = createMockResponse(); + await expect(streamGraphNdjson(response, false)).resolves.toBeUndefined(); + + expect(lbugMocks.streamQuery).toHaveBeenCalledWith( + expect.stringContaining('MATCH (n:`Macro`)'), + expect.any(Function), + ); + }); + + it('streams Route and Tool nodes without requiring startLine fields', async () => { + lbugMocks.streamQuery.mockImplementation( + async (query: string, onRow: (row: any) => Promise) => { + if (query.includes('MATCH (n:`Route`)')) { + expect(query).not.toContain('startLine'); + await onRow({ + id: 'Route:/api/graph:GET', + name: 'GET /api/graph', + filePath: 'src/server/api.ts', + responseKeys: ['nodes', 'relationships'], + errorKeys: ['error'], + middleware: ['withAuth'], + }); + return 1; + } + if (query.includes('MATCH (n:`Tool`)')) { + expect(query).not.toContain('startLine'); + await onRow({ + id: 'Tool:gitnexus_query', + name: 'gitnexus_query', + filePath: 'src/mcp/resources.ts', + description: 'Query the code graph', + }); + return 1; + } + return 0; + }, + ); + + const writes: string[] = []; + const response = createMockResponse((chunk) => { + writes.push(chunk); + return true; + }); + + await expect(streamGraphNdjson(response, false)).resolves.toBeUndefined(); + + const records = writes.map((chunk) => JSON.parse(chunk)); + expect(records).toContainEqual({ + type: 'node', + data: { + id: 'Route:/api/graph:GET', + label: 'Route', + properties: { + name: 'GET /api/graph', + filePath: 'src/server/api.ts', + startLine: undefined, + endLine: undefined, + content: undefined, + responseKeys: ['nodes', 'relationships'], + errorKeys: ['error'], + middleware: ['withAuth'], + heuristicLabel: undefined, + cohesion: undefined, + symbolCount: undefined, + description: undefined, + processType: undefined, + stepCount: undefined, + communities: undefined, + entryPointId: undefined, + terminalId: undefined, + }, + }, + }); + expect(records).toContainEqual({ + type: 'node', + data: { + id: 'Tool:gitnexus_query', + label: 'Tool', + properties: { + name: 'gitnexus_query', + filePath: 'src/mcp/resources.ts', + startLine: undefined, + endLine: undefined, + content: undefined, + responseKeys: undefined, + errorKeys: undefined, + middleware: undefined, + heuristicLabel: undefined, + cohesion: undefined, + symbolCount: undefined, + description: 'Query the code graph', + processType: undefined, + stepCount: undefined, + communities: undefined, + entryPointId: undefined, + terminalId: undefined, + }, + }, + }); + }); +}); From d09078925ee8a243da8f133710a45d5ba4f88837 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Thu, 9 Apr 2026 17:41:28 +0100 Subject: [PATCH 12/12] Extract `resolveFreeCall` from `resolveCallTarget` (SM-13) (#756) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Initial plan * feat(SM-13): extract resolveFreeCall from resolveCallTarget Extract the free-function call resolution path into a dedicated `resolveFreeCall(calledName, filePath, ctx)` function that uses `lookupExact` + import-scoped resolution via `ctx.resolve()`. - Free function calls (foo()) now route through `resolveFreeCall` - Swift/Kotlin implicit constructors (User()) delegate to `resolveStaticCall` within `resolveFreeCall` - `resolveCallTarget` dispatches `callForm === 'free'` early, removing the inline freeFormHasClassTarget logic - S0 block simplified to only handle `callForm === 'constructor'` - Global (Tier 3) fallthrough preserved via ctx.resolve() until Phase 5 - 9 new unit tests for resolveFreeCall - All 163 unit tests pass, all 1199 integration resolver tests pass Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/c5f2e73a-259a-438c-b5c8-286b82e3c215 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * chore: revert unrelated package-lock.json change Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/c5f2e73a-259a-438c-b5c8-286b82e3c215 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * fix(SM-13): address PR #756 review findings on resolveFreeCall Addresses all 7 findings from the PR #756 review comment. Code (R1, finding #1) - Replace the literal `'Class' | 'Struct' | 'Record'` check in `hasClassTarget` with `INSTANTIABLE_CLASS_TYPES.has(c.type)`. Converts an invariant that was previously comment-enforced ("keep this list aligned with INSTANTIABLE_CLASS_TYPES") into one enforced structurally. Any future extension of the set propagates here automatically. The narrower Swift extension dedup block below still uses literal `'Class' | 'Struct'` by design — Swift extensions only produce Class duplicates in practice, Record is deliberately excluded there, and the inline comment now documents that asymmetry. Tests (+12 regression scenarios) Finding #2 — language coverage - Go free function (doStuff()) - Python free function (def helper(): ... helper()) - Rust free function outside any impl block - Java statically-imported function - JavaScript module-level function Each exercises `_resolveCallTargetForTesting` with `callForm='free'` and the language-specific file extension. `resolveFreeCall` has no file-extension branching, so these guard the dispatch chain per language without assuming extractor-specific symbol shapes. Finding #3 — argCount threading - 2-arg overload selected when argCount=2 - 0-arg overload selected when argCount=0 Finding #5 — Tier 3 (global) resolution - Function globally visible but not imported. Asserts exact `TIER_CONFIDENCE.global === 0.5` and `reason === 'global'` to catch silent drift if the tier table is ever refactored. Finding #6 — preComputedArgTypes worker path - String overload matched via preComputedArgTypes=['String'] - Int overload matched via preComputedArgTypes=['int'] (lowercase, mirroring the parse-worker's inferred-literal shape; stored 'Int' is normalized via normalizeJvmTypeName at comparison time) Finding #7 — Enum null-route documentation - Enum-only free call asserts `toBeNull()` with an explanatory comment linking to the INSTANTIABLE_CLASS_TYPES rationale. NOT marked skipped — current behavior is intentional, not broken. Finding #4 — Swift extension dedup guard - Two same-name Class entries at different path lengths; exercises the full dispatch chain: 1. filterCallableCandidates with 'free' strips Class → length 0 2. hasClassTarget triggers resolveStaticCall 3. Homonym ambiguity null-routes per SM-12 round-1 contract 4. Constructor-form retry repopulates with both Classes 5. Dedup block sorts by filePath.length → shortest path wins Verification - `tsc --noEmit` clean - 3064 unit tests pass (+12) - 1766 integration tests pass - Zero regressions Plan: docs/plans/2026-04-09-003-fix-sm13-resolve-free-call-review-findings-plan.md Review: https://github.com/abhigyanpatwari/GitNexus/pull/756#issuecomment-4213879002 * refactor(SM-13): extract dedupSwiftExtensionCandidates shared helper Follow-up to the PR #756 review fix. SM-13 duplicated the Swift extension same-name collision dedup block between `resolveCallTarget` and `resolveFreeCall` — two copies of identical 15-line logic with the same heuristic (`filePath.length` sort, Class/Struct-only, `length > 1` guard). Extract a single shared helper so the two sites cannot drift. Changes - New `dedupSwiftExtensionCandidates(candidates, tier)` helper defined alongside `tryOverloadDisambiguation`, with JSDoc documenting: - The Swift extension scenario it addresses - Why it is intentionally narrower than INSTANTIABLE_CLASS_TYPES (Class/Struct only, not Record — C#/Kotlin records don't exhibit the multi-file definition pattern, widening risks accidental dedup of legitimately distinct record types) - The return-null-on-no-match contract so callers can fall through - `resolveCallTarget` tail dedup (was lines 1593-1610): replaced with a single `dedupSwiftExtensionCandidates` call - `resolveFreeCall` tail dedup (was lines 1994-2012): same replacement - Net line count: -32 insertions, -9 deletions in the consumer sites, +36 for the shared helper + JSDoc Verification - `tsc --noEmit` clean - 3064 unit tests pass (including the R7 Swift dedup guard test added in the previous commit that exercises the full free-form retry chain through this helper) - 1766 integration tests pass - Zero regressions Follows-up on: https://github.com/abhigyanpatwari/GitNexus/pull/756 * docs(SM-13): address PR #756 final review — comment cleanup only Three documentation-only findings from the approval review. No behavior change, no new tests, no code path modifications. Finding #1 — stale line-number comment - The comment inside `resolveFreeCall` at the `hasClassTarget` site referenced "lines ~1994-2008" for the Swift extension dedup block. Those lines were the inlined pre-SM-13 version; the block has since been extracted to `dedupSwiftExtensionCandidates`. Replaced the line reference with the helper name so future readers don't chase dead line numbers. Finding #2 — fuzzy-widening asymmetry undocumented - `resolveFreeCall` intentionally has no `widenCache` parameter and no D2 fuzzy-widening pass (unlike `resolveCallTarget`'s member-call path). Added an explicit "Asymmetry vs `resolveCallTarget`" paragraph to the JSDoc so a caller comparing the two signatures knows the skipped pass is deliberate and tied to Phase 5. Finding #3 — constructor-form retry reasons undocumented - `resolveStaticCall` can return null for three distinct reasons (empty instantiable pool, homonym ambiguity, ownerless Constructor nodes). The retry below it unconditionally re-filters with `'constructor'` form, which is correct for all three but not obvious. Added a structured three-case comment enumerating each reason and linking (a) to the SM-12 null-route contract, (b) to the R7 dedup test, and (c) to the currently-uncovered ownerless- Constructor path (noted as a future test candidate). Verification - `tsc --noEmit` clean - 175 `resolveFreeCall` + `resolveStaticCall` + sibling tests pass (sanity check — no behavior change expected) - No regressions Follows-up on: https://github.com/abhigyanpatwari/GitNexus/pull/756#issuecomment-4215739052 * test(SM-13): cover ownerless-Constructor retry + PHP free function Two low-severity test gaps from PR #756 review comment 4215739052 — previously addressed doc-only, now have concrete test coverage. Finding #3 low — ownerless-Constructor retry path (previously comment-only) - The retry after resolveStaticCall returns null handles three distinct null-return reasons. Cases (a) and (b) were already tested (Interface/ Trait null-route from SM-12, Swift shadowing dedup from R7). Case (c) — resolveStaticCall step-4 bailout when the tiered pool contains ownerless Constructor nodes — was only covered by a comment. - New test: Class + ownerless Constructor in tiered pool, callForm='free'. Exercises the full chain: 1. resolveStaticCall step 3 walks classCandidates via lookupMethodByOwner — ownerless Constructor not in methodByOwner, nothing found. 2. Step 4 detects Constructor in tiered pool, bails with null. 3. resolveFreeCall retry re-runs filterCallableCandidates with 'constructor' form, which prefers Constructor over Class per CONSTRUCTOR_TARGET_TYPES ordering. 4. Single survivor returned. - Asserts the Constructor node (not the Class) is the resolved target. Low — PHP free function coverage gap - The language coverage table in the same review flagged PHP free functions (top-level `function helper()` outside any class) as uncovered. Added a test mirroring the existing Go/Python/Rust/Java/ JS language tests — exercises the `.php` dispatch path for free calls. Ruby and C/C++ remain uncovered; deferred to a future round since those languages also have other gaps in the broader test file. Verification - `tsc --noEmit` clean - 3066 unit tests pass (+2 new regression tests) - 1766 integration tests pass - Zero regressions Follows-up on: https://github.com/abhigyanpatwari/GitNexus/pull/756#issuecomment-4215739052 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> Co-authored-by: Gergo Magyar --- gitnexus/src/core/ingestion/call-processor.ts | 245 +++++++--- gitnexus/test/unit/symbol-table.test.ts | 441 ++++++++++++++++++ 2 files changed, 632 insertions(+), 54 deletions(-) diff --git a/gitnexus/src/core/ingestion/call-processor.ts b/gitnexus/src/core/ingestion/call-processor.ts index 2a47d5f39..1dadcaf54 100644 --- a/gitnexus/src/core/ingestion/call-processor.ts +++ b/gitnexus/src/core/ingestion/call-processor.ts @@ -1317,6 +1317,44 @@ const tryOverloadDisambiguation = ( return matchCandidatesByArgTypes(candidates, argTypes); }; +/** + * Collapse Swift-extension duplicate Class/Struct candidates to the primary + * definition, preferring the shortest file path. + * + * Swift extensions (`extension User { ... }` in a separate file) create + * multiple `Class` nodes sharing the same symbol name — one for the primary + * declaration and one per extension file. When overload disambiguation and + * receiver narrowing both fail to converge on a single candidate, this + * heuristic picks the primary definition based on the assumption that it + * lives at the shortest file path (e.g. `User.swift` over `UserExtensions.swift`). + * + * Intentionally narrower than {@link INSTANTIABLE_CLASS_TYPES}: only `Class` + * and `Struct` are considered, not `Record`. Swift extensions only produce + * `Class` duplicates in practice, and C#/Kotlin records do not exhibit the + * same multi-file-definition pattern, so widening this set risks accidental + * dedup of legitimately distinct record types. + * + * Returns a `ResolveResult` when the heuristic fires, `null` when the + * candidate pool does not match the shape (mixed types, non-Class/Struct + * kinds, or `length <= 1`). Callers should fall through to their own null + * return when this helper returns `null`. + * + * Shared between `resolveCallTarget` and `resolveFreeCall` — SM-13 originally + * duplicated this block into both functions. Having a single source of truth + * prevents the two copies from drifting if the heuristic is ever tuned. + */ +const dedupSwiftExtensionCandidates = ( + candidates: readonly SymbolDefinition[], + tier: ResolutionTier, +): ResolveResult | null => { + if (candidates.length <= 1) return null; + const allSameType = candidates.every((c) => c.type === candidates[0].type); + if (!allSameType) return null; + if (candidates[0].type !== 'Class' && candidates[0].type !== 'Struct') return null; + const sorted = [...candidates].sort((a, b) => a.filePath.length - b.filePath.length); + return toResolveResult(sorted[0], tier); +}; + /** * Resolve a function call to its target node ID using priority strategy: * A. Narrow candidates by scope tier via ctx.resolve() @@ -1370,6 +1408,20 @@ const resolveCallTarget = ( const tiered = ctx.resolve(call.calledName, currentFile); if (!tiered) return null; + // SM-13: Free function calls route through resolveFreeCall. + // Handles pure free calls (foo()) and Swift/Kotlin implicit constructors (User()). + if (call.callForm === 'free') { + return resolveFreeCall( + call.calledName, + currentFile, + ctx, + call.argCount, + tiered, + overloadHints, + preComputedArgTypes, + ); + } + let filteredCandidates = filterCallableCandidates( tiered.candidates, call.argCount, @@ -1377,13 +1429,10 @@ const resolveCallTarget = ( ); // S0. Constructor/static fast path (SM-12): O(1) class + constructor lookup - // via lookupClassByName + lookupMethodByOwner before falling back to the - // existing filtering + fuzzy-widening path. Falls back to the class node - // itself when no Constructor symbol is indexed for the type. - // - // Handles: - // (a) callForm === 'constructor' — explicit `new User()` in Java/TS/C#/etc. - // (b) callForm === 'free' with class target — implicit `User()` in Swift/Kotlin + // via lookupClassByName + lookupMethodByOwner. + // Handles callForm === 'constructor' — explicit `new User()` in Java/TS/C#/etc. + // Free-form class targets (Swift/Kotlin `User()`) are handled by + // resolveFreeCall above (SM-13). // // Known gaps (handled by the existing tail fallback at the bottom of // this function, not S0): @@ -1392,21 +1441,7 @@ const resolveCallTarget = ( // S0 to cover them would require threading receiver-type resolution // through the module-alias logic; revisit if it shows up as a hot // spot. - // - // The `.some()` trigger below must stay aligned with - // `INSTANTIABLE_CLASS_TYPES` — any type admitted here that is not in - // that set will cause S0 → `resolveStaticCall` to run and return null, - // wasting two lookup passes per call. `Enum` is deliberately excluded - // (same rationale as `INSTANTIABLE_CLASS_TYPES`); `Record` is included - // so C# records and Kotlin data classes reach the fast path. - const freeFormHasClassTarget = - call.callForm === 'free' && - filteredCandidates.length === 0 && - tiered.candidates.some((c) => c.type === 'Class' || c.type === 'Struct' || c.type === 'Record'); - if (call.callForm === 'constructor' || freeFormHasClassTarget) { - // Reuse the pre-computed `tiered` result — resolveStaticCall's class name - // is identical to `call.calledName` here, so re-running ctx.resolve would - // duplicate the tiered-lookup work performed at the top of this function. + if (call.callForm === 'constructor') { const staticResult = resolveStaticCall( call.calledName, currentFile, @@ -1417,22 +1452,6 @@ const resolveCallTarget = ( if (staticResult) return staticResult; } - // Swift/Kotlin: constructor calls look like free function calls (no `new` keyword). - // If free-form filtering found no callable candidates but the symbol resolves to a - // Class/Struct, retry with constructor form so CONSTRUCTOR_TARGET_TYPES applies. - if (filteredCandidates.length === 0 && call.callForm === 'free') { - // `freeFormHasClassTarget` was already computed for the S0 fast path - // above under the same `callForm === 'free' && filteredCandidates.length === 0` - // precondition. Reuse it to avoid a second `.some()` scan on the same pool. - if (freeFormHasClassTarget) { - filteredCandidates = filterCallableCandidates( - tiered.candidates, - call.argCount, - 'constructor', - ); - } - } - // Module-qualified constructor pattern: e.g. Python `import models; models.User()`. // The attribute access gives callForm='member', but the callee may be a Class — a valid // constructor target. Re-try with constructor-form filtering so that `module.ClassName()` @@ -1610,22 +1629,11 @@ const resolveCallTarget = ( } if (filteredCandidates.length !== 1) { - // Deduplicate: Swift extensions create multiple Class nodes with the same name. - // When all candidates share the same type and differ only by file (extension vs - // primary definition), they represent the same symbol. Prefer the primary - // definition (shortest file path: Product.swift over ProductExtension.swift). - if (filteredCandidates.length > 1) { - const allSameType = filteredCandidates.every((c) => c.type === filteredCandidates[0].type); - if ( - allSameType && - (filteredCandidates[0].type === 'Class' || filteredCandidates[0].type === 'Struct') - ) { - const sorted = [...filteredCandidates].sort( - (a, b) => a.filePath.length - b.filePath.length, - ); - return toResolveResult(sorted[0], tiered.tier); - } - } + // See `dedupSwiftExtensionCandidates` — returns non-null only when the + // Swift-extension same-name collision heuristic applies. Otherwise null- + // route (ambiguous candidates should not produce a wrong edge). + const deduped = dedupSwiftExtensionCandidates(filteredCandidates, tiered.tier); + if (deduped) return deduped; return null; } @@ -1929,6 +1937,135 @@ export const resolveMemberCall = ( return toResolveResult(resolved.def, resolved.tier); }; +// --------------------------------------------------------------------------- +// SM-13: Free-function call resolution +// --------------------------------------------------------------------------- + +/** + * Resolve a free-function call using `lookupExact` (same-file) + import-scoped + * resolution via `ctx.resolve()`. + * + * Used for `foo()`, `doStuff()` — unqualified calls with no receiver. + * Also handles Swift/Kotlin implicit constructors (`User()` without `new`) + * by delegating to {@link resolveStaticCall} when the tiered pool contains + * class-like targets. + * + * {@link resolveCallTarget} delegates here for `callForm === 'free'` before + * processing constructor and member calls. + * + * **Design note (SM-13):** This path still falls through to Tier 3 (global) + * via `ctx.resolve()`. Fuzzy global resolution remains until Phase 5 replaces + * `lookupFuzzy` with a scoped data source. + * + * **Asymmetry vs `resolveCallTarget`:** `resolveFreeCall` intentionally does + * NOT take a `widenCache` parameter and does NOT run a D2 fuzzy-widening + * pass. Member calls (`resolveCallTarget`'s main body) widen via + * `lookupFuzzy` to reach parent-class methods defined in different files; + * free calls have no receiver type and rely exclusively on the tiered pool + * from `ctx.resolve()`. Phase 5 will revisit whether free calls need a + * scoped widening pass once `lookupFuzzy` is retired. + * + * @param calledName - The called function name (e.g. 'doStuff') + * @param filePath - File path of the call site + * @param ctx - Resolution context + * @param argCount - Optional argument count for arity filtering + * @param tieredOverride - Pre-computed tiered candidates from an upstream + * `ctx.resolve` call. When provided, skips the redundant + * lookup inside this function. + * @param overloadHints - Optional AST-based overload disambiguation hints + * @param preComputedArgTypes - Optional pre-computed argument types (worker path) + */ +export const resolveFreeCall = ( + calledName: string, + filePath: string, + ctx: ResolutionContext, + argCount?: number, + tieredOverride?: TieredCandidates, + overloadHints?: OverloadHints, + preComputedArgTypes?: (string | undefined)[], +): ResolveResult | null => { + const tiered = tieredOverride ?? ctx.resolve(calledName, filePath); + if (!tiered) return null; + + let filteredCandidates = filterCallableCandidates(tiered.candidates, argCount, 'free'); + + // Class-target fast path: Swift/Kotlin `User()` — free-form call targeting a + // class. Delegates to resolveStaticCall for O(1) class + constructor lookup. + // The `.some()` trigger must stay aligned with `INSTANTIABLE_CLASS_TYPES` — + // any type admitted here that is not in that set will cause resolveStaticCall + // to return null, wasting two lookup passes per call. `Enum` is deliberately + // excluded; `Record` is included so C# records and Kotlin data classes reach + // the fast path. + // Align with INSTANTIABLE_CLASS_TYPES by reusing the set directly rather + // than enumerating literal strings. This converts an invariant that was + // previously enforced by a comment ("keep this list aligned with + // INSTANTIABLE_CLASS_TYPES") into one enforced structurally — any future + // extension of the set (e.g. Kotlin `object`) propagates here automatically. + // The `dedupSwiftExtensionCandidates` helper used in the tail of this + // function deliberately uses a narrower literal `'Class' | 'Struct'` check + // — Swift extensions only produce Class duplicates in practice, so Record + // is excluded there by design. Do not collapse that helper into + // INSTANTIABLE_CLASS_TYPES. + const hasClassTarget = + filteredCandidates.length === 0 && + tiered.candidates.some((c) => INSTANTIABLE_CLASS_TYPES.has(c.type)); + if (hasClassTarget) { + const staticResult = resolveStaticCall(calledName, filePath, ctx, argCount, tiered); + if (staticResult) return staticResult; + // Retry with constructor form: Swift/Kotlin constructor calls look like + // free function calls (no `new` keyword). If resolveStaticCall didn't + // match, re-filter with constructor form so CONSTRUCTOR_TARGET_TYPES + // applies. + // + // The retry fires for every null return from `resolveStaticCall`, which + // can happen for three distinct reasons — all three are handled below: + // + // (a) No explicit `Constructor` node found and zero instantiable + // class candidates (e.g. Interface/Trait/Impl only — the SM-12 + // null-route contract). `filterCallableCandidates` with + // `'constructor'` form will also return nothing → we fall + // through to the final null return. Correct. + // + // (b) Homonym ambiguity — two or more instantiable class candidates + // share the name (e.g. `User` in two files, same tier). The + // retry repopulates `filteredCandidates` with both Classes and + // they flow into `dedupSwiftExtensionCandidates` below, which + // either picks the shortest-path primary or null-routes. + // Covered by the R7 Swift-extension dedup test. + // + // (c) `resolveStaticCall` step 4 bailed because the tiered pool + // contains ownerless `Constructor` nodes (some extractors emit + // constructors without `ownerId`). Those `Constructor` nodes + // survive the constructor-form filter below and reach overload + // disambiguation, giving the existing filter path a chance to + // pick the right one. Correct but currently uncovered by a + // dedicated test — the R5 `preComputedArgTypes` path exercises + // overload disambiguation for Functions, which is structurally + // the same code. + filteredCandidates = filterCallableCandidates(tiered.candidates, argCount, 'constructor'); + } + + // E. Overload disambiguation + if (filteredCandidates.length > 1) { + const disambiguated = overloadHints + ? tryOverloadDisambiguation(filteredCandidates, overloadHints) + : preComputedArgTypes + ? matchCandidatesByArgTypes(filteredCandidates, preComputedArgTypes) + : null; + if (disambiguated) return toResolveResult(disambiguated, tiered.tier); + } + + if (filteredCandidates.length !== 1) { + // See `dedupSwiftExtensionCandidates` — shared helper, single source of + // truth for the Swift-extension same-name collision heuristic. + const deduped = dedupSwiftExtensionCandidates(filteredCandidates, tiered.tier); + if (deduped) return deduped; + return null; + } + + return toResolveResult(filteredCandidates[0], tiered.tier); +}; + // --------------------------------------------------------------------------- // SM-12: Constructor/static call resolution (no fuzzy lookup) // --------------------------------------------------------------------------- diff --git a/gitnexus/test/unit/symbol-table.test.ts b/gitnexus/test/unit/symbol-table.test.ts index b861c31e7..9378a6008 100644 --- a/gitnexus/test/unit/symbol-table.test.ts +++ b/gitnexus/test/unit/symbol-table.test.ts @@ -1417,6 +1417,7 @@ describe('lookupMethodByOwnerWithMRO', () => { import { _resolveCallTargetForTesting, resolveMemberCall, + resolveFreeCall, type OverloadHints, } from '../../src/core/ingestion/call-processor.js'; @@ -2329,3 +2330,443 @@ describe('resolveStaticCall', () => { expect(result!.nodeId).toBe('ctor:User:2'); }); }); + +// --------------------------------------------------------------------------- +// resolveFreeCall — SM-13: free-function call resolution +// --------------------------------------------------------------------------- + +describe('resolveFreeCall', () => { + let ctx: ResolutionContext; + + beforeEach(() => { + ctx = createResolutionContext(); + }); + + it('resolves a free function call via import-scoped resolution', () => { + ctx.symbols.add('src/utils.ts', 'doStuff', 'func:doStuff', 'Function'); + ctx.importMap.set('src/app.ts', new Set(['src/utils.ts'])); + + const result = resolveFreeCall('doStuff', 'src/app.ts', ctx); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('func:doStuff'); + expect(result!.confidence).toBe(0.9); // import-scoped tier + expect(result!.reason).toBe('import-resolved'); + }); + + it('resolves a free function call via same-file resolution', () => { + ctx.symbols.add('src/app.ts', 'helper', 'func:helper', 'Function'); + + const result = resolveFreeCall('helper', 'src/app.ts', ctx); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('func:helper'); + expect(result!.confidence).toBe(0.95); // same-file tier + expect(result!.reason).toBe('same-file'); + }); + + it('returns null when no candidates exist', () => { + const result = resolveFreeCall('nonexistent', 'src/app.ts', ctx); + expect(result).toBeNull(); + }); + + it('returns null for ambiguous free function calls (multiple candidates)', () => { + ctx.symbols.add('src/a.ts', 'doStuff', 'func:a:doStuff', 'Function'); + ctx.symbols.add('src/b.ts', 'doStuff', 'func:b:doStuff', 'Function'); + ctx.importMap.set('src/app.ts', new Set(['src/a.ts', 'src/b.ts'])); + + const result = resolveFreeCall('doStuff', 'src/app.ts', ctx); + + expect(result).toBeNull(); + }); + + it('delegates to resolveStaticCall for free-form class targets (Swift/Kotlin)', () => { + ctx.symbols.add('src/user.swift', 'User', 'class:User', 'Class'); + ctx.importMap.set('src/app.swift', new Set(['src/user.swift'])); + + const result = resolveFreeCall('User', 'src/app.swift', ctx); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('class:User'); + }); + + it('delegates to resolveStaticCall for Record free-form targets (C#/Kotlin)', () => { + ctx.symbols.add('src/User.cs', 'User', 'record:cs:User', 'Record'); + ctx.importMap.set('src/App.cs', new Set(['src/User.cs'])); + + const result = resolveFreeCall('User', 'src/App.cs', ctx); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('record:cs:User'); + }); + + it('null-routes Trait free-form calls via resolveStaticCall', () => { + ctx.symbols.add('src/timestamps.php', 'HasTimestamps', 'trait:HasTimestamps', 'Trait'); + ctx.importMap.set('src/model.php', new Set(['src/timestamps.php'])); + + const result = resolveFreeCall('HasTimestamps', 'src/model.php', ctx); + + expect(result).toBeNull(); + }); + + it('uses tieredOverride when provided', () => { + ctx.symbols.add('src/utils.ts', 'doStuff', 'func:doStuff', 'Function'); + ctx.importMap.set('src/app.ts', new Set(['src/utils.ts'])); + + const tiered = ctx.resolve('doStuff', 'src/app.ts'); + expect(tiered).not.toBeNull(); + + // Spy on ctx.resolve to verify it is NOT called again + const originalResolve = ctx.resolve.bind(ctx); + let resolveCallCount = 0; + ctx.resolve = ((name: string, fromFile: string) => { + resolveCallCount++; + return originalResolve(name, fromFile); + }) as typeof ctx.resolve; + + const result = resolveFreeCall('doStuff', 'src/app.ts', ctx, undefined, tiered!); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('func:doStuff'); + expect(resolveCallCount).toBe(0); + }); + + it('routes through resolveCallTarget for free-form calls', () => { + ctx.symbols.add('src/utils.ts', 'doStuff', 'func:doStuff', 'Function'); + ctx.importMap.set('src/app.ts', new Set(['src/utils.ts'])); + + const result = _resolveCallTargetForTesting( + { + calledName: 'doStuff', + callForm: 'free', + }, + 'src/app.ts', + ctx, + ); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('func:doStuff'); + }); + + // ------------------------------------------------------------------------- + // PR #756 review follow-up (plan 2026-04-09-003): language coverage, + // arity threading, Tier 3 resolution, preComputedArgTypes worker path, + // Enum null-route, and Swift extension dedup guard. + // ------------------------------------------------------------------------- + + // R2 — Language coverage: Go, Python, Rust, Java, JavaScript free-function + // dispatch through _resolveCallTargetForTesting. resolveFreeCall has no + // file-extension branching; these guard the dispatch chain per language. + + it('resolves a Go free function (doStuff())', () => { + ctx.symbols.add('src/helper.go', 'doStuff', 'func:go:doStuff', 'Function'); + ctx.importMap.set('src/main.go', new Set(['src/helper.go'])); + + const result = _resolveCallTargetForTesting( + { calledName: 'doStuff', callForm: 'free' }, + 'src/main.go', + ctx, + ); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('func:go:doStuff'); + }); + + it('resolves a Python free function (def helper(): ... helper())', () => { + ctx.symbols.add('helpers.py', 'helper', 'func:py:helper', 'Function'); + ctx.importMap.set('app.py', new Set(['helpers.py'])); + + const result = _resolveCallTargetForTesting( + { calledName: 'helper', callForm: 'free' }, + 'app.py', + ctx, + ); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('func:py:helper'); + }); + + it('resolves a Rust free function outside any impl block (free_fn())', () => { + ctx.symbols.add('src/helpers.rs', 'free_fn', 'func:rs:free_fn', 'Function'); + ctx.importMap.set('src/main.rs', new Set(['src/helpers.rs'])); + + const result = _resolveCallTargetForTesting( + { calledName: 'free_fn', callForm: 'free' }, + 'src/main.rs', + ctx, + ); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('func:rs:free_fn'); + }); + + it('resolves a Java statically-imported function (doStuff() after import static Utils.doStuff)', () => { + // Note: this simulates the extractor output post static import by + // indexing the function directly in its declaring file. The test guards + // the dispatch chain for .java files, not the extractor's handling of + // static imports specifically. + ctx.symbols.add('src/Utils.java', 'doStuff', 'func:java:doStuff', 'Function'); + ctx.importMap.set('src/App.java', new Set(['src/Utils.java'])); + + const result = _resolveCallTargetForTesting( + { calledName: 'doStuff', callForm: 'free' }, + 'src/App.java', + ctx, + ); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('func:java:doStuff'); + }); + + it('resolves a JavaScript module-level function (moduleFn())', () => { + ctx.symbols.add('src/helpers.js', 'moduleFn', 'func:js:moduleFn', 'Function'); + ctx.importMap.set('src/app.js', new Set(['src/helpers.js'])); + + const result = _resolveCallTargetForTesting( + { calledName: 'moduleFn', callForm: 'free' }, + 'src/app.js', + ctx, + ); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('func:js:moduleFn'); + }); + + // R3 — Arity filtering: call.argCount must narrow overloaded free functions + // differing only in parameter count. + + it('narrows overloaded free functions by argCount (2-arg overload selected)', () => { + ctx.symbols.add('src/utils.ts', 'helper', 'func:helper:0', 'Function', { + parameterCount: 0, + }); + ctx.symbols.add('src/utils.ts', 'helper', 'func:helper:2', 'Function', { + parameterCount: 2, + }); + ctx.importMap.set('src/app.ts', new Set(['src/utils.ts'])); + + const result = _resolveCallTargetForTesting( + { calledName: 'helper', callForm: 'free', argCount: 2 }, + 'src/app.ts', + ctx, + ); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('func:helper:2'); + }); + + it('narrows overloaded free functions by argCount (0-arg overload selected)', () => { + ctx.symbols.add('src/utils.ts', 'helper', 'func:helper:0', 'Function', { + parameterCount: 0, + }); + ctx.symbols.add('src/utils.ts', 'helper', 'func:helper:2', 'Function', { + parameterCount: 2, + }); + ctx.importMap.set('src/app.ts', new Set(['src/utils.ts'])); + + const result = _resolveCallTargetForTesting( + { calledName: 'helper', callForm: 'free', argCount: 0 }, + 'src/app.ts', + ctx, + ); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('func:helper:0'); + }); + + // R4 — Tier 3 (global) resolution: function globally visible but not + // imported. Locks in TIER_CONFIDENCE.global === 0.5 and reason === 'global' + // so a silent tier-table refactor surfaces here. + + it('resolves a globally-visible free function via Tier 3 with global confidence', () => { + ctx.symbols.add('lib/global.ts', 'helper', 'func:global:helper', 'Function'); + // No importMap entry — must fall through to Tier 3 (global). + + const result = _resolveCallTargetForTesting( + { calledName: 'helper', callForm: 'free' }, + 'src/app.ts', + ctx, + ); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('func:global:helper'); + expect(result!.confidence).toBe(0.5); // TIER_CONFIDENCE.global + expect(result!.reason).toBe('global'); + }); + + // R5 — preComputedArgTypes worker path: when parse-worker pre-computes + // argument types, the disambiguation routes through matchCandidatesByArgTypes. + // Preconditions (verified at feasibility review): + // 1. filteredCandidates.length > 1 — both overloads must survive arity + // filtering, so argCount left unset here. + // 2. overloadHints must be undefined — it takes precedence over + // preComputedArgTypes at the disambiguation site. + + it('disambiguates overloads via preComputedArgTypes (String overload matched)', () => { + ctx.symbols.add('src/utils.ts', 'helper', 'func:helper:str', 'Function', { + parameterCount: 1, + parameterTypes: ['String'], + }); + ctx.symbols.add('src/utils.ts', 'helper', 'func:helper:int', 'Function', { + parameterCount: 1, + parameterTypes: ['Int'], + }); + ctx.importMap.set('src/app.ts', new Set(['src/utils.ts'])); + + const result = _resolveCallTargetForTesting( + { calledName: 'helper', callForm: 'free', argCount: 1 }, + 'src/app.ts', + ctx, + { preComputedArgTypes: ['String'] }, + ); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('func:helper:str'); + }); + + it('disambiguates overloads via preComputedArgTypes (Int overload matched)', () => { + ctx.symbols.add('src/utils.ts', 'helper', 'func:helper:str', 'Function', { + parameterCount: 1, + parameterTypes: ['String'], + }); + ctx.symbols.add('src/utils.ts', 'helper', 'func:helper:int', 'Function', { + parameterCount: 1, + parameterTypes: ['Int'], + }); + ctx.importMap.set('src/app.ts', new Set(['src/utils.ts'])); + + const result = _resolveCallTargetForTesting( + { calledName: 'helper', callForm: 'free', argCount: 1 }, + 'src/app.ts', + ctx, + // `Int` is normalized to `int` on the stored side via normalizeJvmTypeName + // (matchCandidatesByArgTypes:1287). Real parse-worker-emitted argTypes are + // already lowercase primitive names inferred from literals, so this + // mirrors production call-site shape. + { preComputedArgTypes: ['int'] }, + ); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('func:helper:int'); + }); + + // R6 — Enum free-form null-route: locks in the current behavior that + // `Color()`-style calls on Enum types return null because Enum is + // deliberately excluded from INSTANTIABLE_CLASS_TYPES. This is intentional + // per PR #754 round 1 (see `call-processor.ts` INSTANTIABLE_CLASS_TYPES + // JSDoc — "Enum excluded pending language-specific support with motivating + // test fixtures"). If a future extension adds Enum to the set, this test + // will need to be updated alongside that work — that is the correct signal. + + it('null-routes Enum free-form calls (Color() — no instantiable fallback)', () => { + ctx.symbols.add('src/color.ts', 'Color', 'enum:Color', 'Enum'); + ctx.importMap.set('src/app.ts', new Set(['src/color.ts'])); + + const result = _resolveCallTargetForTesting( + { calledName: 'Color', callForm: 'free' }, + 'src/app.ts', + ctx, + ); + + // Enum not in INSTANTIABLE_CLASS_TYPES → hasClassTarget is false → + // resolveStaticCall is not called → tail dedup also doesn't fire → + // falls through to the final null return. + expect(result).toBeNull(); + }); + + // R7 — Swift extension dedup `filePath.length` heuristic guard: + // Two same-name Class entries at different path lengths. The free-form + // dispatch chain goes: + // 1. filterCallableCandidates(tiered, argCount, 'free') strips Class → + // filteredCandidates.length === 0 + // 2. hasClassTarget is true (both are Class) + // 3. resolveStaticCall runs, has 2 homonym Class candidates → + // instantiableCandidates.length > 1 → returns null (SM-12 round-1 + // null-route contract) + // 4. Constructor-form retry: filterCallableCandidates(tiered, argCount, + // 'constructor') keeps Class entries → filteredCandidates.length === 2 + // 5. Falls through to the Swift extension dedup block → sorts by + // filePath.length → returns the shortest path. + + it('dedupes Swift extension candidates by shortest file path (free-form retry path)', () => { + // Two same-name Class entries, different path lengths. + ctx.symbols.add('src/User.swift', 'User', 'class:User:primary', 'Class'); + ctx.symbols.add('src/Extensions/UserExtensions.swift', 'User', 'class:User:extension', 'Class'); + ctx.importMap.set( + 'src/App.swift', + new Set(['src/User.swift', 'src/Extensions/UserExtensions.swift']), + ); + + const result = _resolveCallTargetForTesting( + { calledName: 'User', callForm: 'free' }, + 'src/App.swift', + ctx, + ); + + // The shortest file path wins per the existing heuristic. This is a + // behavior guard for finding #4 in the PR #756 review — if the dedup + // heuristic changes, this test surfaces that intent. + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('class:User:primary'); + }); + + // ------------------------------------------------------------------------- + // PR #756 final review follow-up (comment 4215739052): + // - Finding #3 low: ownerless-Constructor retry path (previously covered + // by comment only) — adds the concrete test the reviewer asked for. + // - Low-severity coverage gap: PHP free function (from the language + // coverage table in the same review). + // ------------------------------------------------------------------------- + + it('routes through resolveStaticCall retry when tiered pool contains an ownerless Constructor (free-form)', () => { + // This exercises the third null-return reason documented in the retry + // comment inside resolveFreeCall: resolveStaticCall's step-4 bailout when + // the tiered pool contains Constructor nodes that lack ownerId (common in + // some extractors). In that case: + // 1. resolveStaticCall step 3 walks classCandidates via lookupMethodByOwner + // — the ownerless Constructor is NOT in methodByOwner, so nothing found. + // 2. Step 4 detects the Constructor in the tiered pool and bails out + // with null so filterCallableCandidates can handle Constructor-vs- + // Class preference correctly. + // 3. resolveFreeCall's retry re-runs filterCallableCandidates with + // 'constructor' form, which — per CONSTRUCTOR_TARGET_TYPES — prefers + // the Constructor node over the Class node. + // 4. Single survivor → returned as the call target. + ctx.symbols.add('src/user.ts', 'User', 'class:User', 'Class'); + ctx.symbols.add('src/user.ts', 'User', 'ctor:User:ownerless', 'Constructor', { + parameterCount: 0, + // No ownerId — this is the pathological extractor output the retry path + // exists to handle. + }); + ctx.importMap.set('src/app.ts', new Set(['src/user.ts'])); + + const result = _resolveCallTargetForTesting( + { calledName: 'User', callForm: 'free' }, + 'src/app.ts', + ctx, + ); + + // The Constructor survives filterCallableCandidates's 'constructor' form + // filter and is preferred over the Class (CONSTRUCTOR_TARGET_TYPES puts + // Constructor first). Guards the (c) case in the retry-reasons comment. + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('ctor:User:ownerless'); + }); + + it('resolves a PHP free function (top-level helper())', () => { + // PHP allows top-level function definitions outside any class. The + // language coverage table in PR #756 review flagged this as uncovered; + // this test exercises the `.php` dispatch path for free calls. Matches + // the shape of the existing Go/Python/Rust/Java/JS language tests above. + ctx.symbols.add('src/helpers.php', 'helper', 'func:php:helper', 'Function'); + ctx.importMap.set('src/app.php', new Set(['src/helpers.php'])); + + const result = _resolveCallTargetForTesting( + { calledName: 'helper', callForm: 'free' }, + 'src/app.php', + ctx, + ); + + expect(result).not.toBeNull(); + expect(result!.nodeId).toBe('func:php:helper'); + }); +});