From 7149001877f88e0ea1f345d4c298694f04f69c49 Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Fri, 3 Apr 2026 19:58:34 +0100 Subject: [PATCH] fix: detect ambiguity in findInheritedMethod BFS When multiple EXTENDS ancestors provide the same method matching an interface contract, findInheritedMethod now collects all matches across the full BFS and returns null (ambiguous) when 2+ distinct methodIds are found. Diamond paths are deduplicated by methodId so the same method reachable via two paths is not treated as ambiguous. 2 new tests: multi-parent ambiguity (returns null), diamond dedup (returns the single shared method). --- gitnexus/src/core/ingestion/mro-processor.ts | 11 ++++- gitnexus/test/unit/mro-processor.test.ts | 50 ++++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/gitnexus/src/core/ingestion/mro-processor.ts b/gitnexus/src/core/ingestion/mro-processor.ts index fa5816eb5..9b95e2133 100644 --- a/gitnexus/src/core/ingestion/mro-processor.ts +++ b/gitnexus/src/core/ingestion/mro-processor.ts @@ -632,6 +632,11 @@ function findInheritedMethod( } } + // Collect all matches across the full BFS, then check for ambiguity. + // If the same methodId is reachable via multiple paths (diamond), it's not + // ambiguous. Only distinct methodIds count as separate matches. + const matches = new Map(); + while (queue.length > 0) { const ancestorId = queue.shift()!; if (visited.has(ancestorId)) continue; @@ -647,7 +652,7 @@ function findInheritedMethod( const mParamTypes = (mNode.properties.parameterTypes as string[] | undefined) ?? []; const mParamCount = mNode.properties.parameterCount as number | undefined; if (parameterTypesMatch(mParamTypes, targetParamTypes, mParamCount, targetParamCount)) { - return { methodId: mid, parameterTypes: mParamTypes }; + matches.set(mid, { methodId: mid, parameterTypes: mParamTypes }); } } @@ -666,7 +671,9 @@ function findInheritedMethod( } } - return null; + // Ambiguous: 2+ distinct methods from different ancestors + if (matches.size === 1) return matches.values().next().value!; + return null; // 0 matches or ambiguous (2+) } /** diff --git a/gitnexus/test/unit/mro-processor.test.ts b/gitnexus/test/unit/mro-processor.test.ts index dcc8f821e..625249340 100644 --- a/gitnexus/test/unit/mro-processor.test.ts +++ b/gitnexus/test/unit/mro-processor.test.ts @@ -1090,4 +1090,54 @@ describe('computeMRO', () => { }); }); }); + + // ---- findInheritedMethod ambiguity detection ------------------------------ + describe('findInheritedMethod ambiguity', () => { + it('returns null when two EXTENDS parents both provide matching method', () => { + // I { foo() }, B { foo() }, M { foo() }, C extends B + M, C implements I + const graph = createKnowledgeGraph(); + addClass(graph, 'I', 'cpp', 'Interface'); + addClass(graph, 'B', 'cpp'); + addClass(graph, 'M', 'cpp'); + addClass(graph, 'C', 'cpp'); + addImplements(graph, 'C', 'I'); + addExtends(graph, 'C', 'B'); + addExtends(graph, 'C', 'M'); + addMethod(graph, 'I', 'foo', 'Interface'); + addMethod(graph, 'B', 'foo'); + addMethod(graph, 'M', 'foo'); + // C has NO own foo — must walk EXTENDS chain + + const result = computeMRO(graph); + // Ambiguous: B.foo and M.foo both match — no METHOD_IMPLEMENTS edge + const mi = graph.relationships.filter((r) => r.type === 'METHOD_IMPLEMENTS'); + const fooEdges = mi.filter((e) => graph.getNode(e.targetId)?.properties.name === 'foo'); + expect(fooEdges).toHaveLength(0); + }); + + it('diamond dedup: same method via two paths is NOT ambiguous', () => { + // I { foo() }, GrandBase { foo() }, B extends GrandBase, M extends GrandBase + // C extends B + M, C implements I + const graph = createKnowledgeGraph(); + addClass(graph, 'I', 'cpp', 'Interface'); + addClass(graph, 'GrandBase', 'cpp'); + addClass(graph, 'B', 'cpp'); + addClass(graph, 'M', 'cpp'); + addClass(graph, 'C', 'cpp'); + addImplements(graph, 'C', 'I'); + addExtends(graph, 'C', 'B'); + addExtends(graph, 'C', 'M'); + addExtends(graph, 'B', 'GrandBase'); + addExtends(graph, 'M', 'GrandBase'); + addMethod(graph, 'I', 'foo', 'Interface'); + const gbFoo = addMethod(graph, 'GrandBase', 'foo'); + // B and M have NO own foo — both inherit from GrandBase + + const result = computeMRO(graph); + // Not ambiguous: same GrandBase.foo via both paths + const mi = graph.relationships.filter((r) => r.type === 'METHOD_IMPLEMENTS'); + const fooEdge = mi.find((e) => e.sourceId === gbFoo); + expect(fooEdge).toBeDefined(); + }); + }); });