diff --git a/gitnexus/src/core/ingestion/mro-processor.ts b/gitnexus/src/core/ingestion/mro-processor.ts index 10c915c32..fa5816eb5 100644 --- a/gitnexus/src/core/ingestion/mro-processor.ts +++ b/gitnexus/src/core/ingestion/mro-processor.ts @@ -441,10 +441,23 @@ export function computeMRO(graph: KnowledgeGraph): MROResult { /** * Check if two parameter type arrays match. - * Lenient: if either side has no type info, match by name only. + * When either side has no type info, fall back to parameterCount comparison + * (arity-compatible matching). If both have parameterCount and they differ, + * return false. If counts match or either is undefined, return true (lenient). */ -function parameterTypesMatch(a: string[], b: string[]): boolean { - if (a.length === 0 || b.length === 0) return true; +function parameterTypesMatch( + a: string[], + b: string[], + aParamCount?: number, + bParamCount?: number, +): boolean { + if (a.length === 0 || b.length === 0) { + // Fall back to arity check when type info is missing + if (aParamCount !== undefined && bParamCount !== undefined) { + return aParamCount === bParamCount; + } + return true; // lenient when either count is unknown + } if (a.length !== b.length) return false; return a.every((t, i) => t === b[i]); } @@ -468,24 +481,24 @@ function emitMethodImplementsEdges( // Get this class's own methods const ownMethodIds = methodMap.get(classId) ?? []; - if (ownMethodIds.length === 0) continue; - // Build a lookup: methodName → Array<{methodId, parameterTypes}> for own methods + // Build a lookup: methodName → Array<{methodId, parameterTypes, parameterCount}> for own methods const ownMethodsByName = new Map< string, - Array<{ methodId: string; parameterTypes: string[] }> + Array<{ methodId: string; parameterTypes: string[]; parameterCount?: number }> >(); for (const methodId of ownMethodIds) { const methodNode = graph.getNode(methodId); if (!methodNode || methodNode.label === 'Property') continue; const name = methodNode.properties.name as string; const parameterTypes = (methodNode.properties.parameterTypes as string[] | undefined) ?? []; + const parameterCount = methodNode.properties.parameterCount as number | undefined; let bucket = ownMethodsByName.get(name); if (!bucket) { bucket = []; ownMethodsByName.set(name, bucket); } - bucket.push({ methodId, parameterTypes }); + bucket.push({ methodId, parameterTypes, parameterCount }); } // Collect ALL transitive ancestors and classify each as EXTENDS or IMPLEMENTS @@ -514,29 +527,72 @@ function emitMethodImplementsEdges( const ancestorName = ancestorMethodNode.properties.name as string; const ancestorParamTypes = (ancestorMethodNode.properties.parameterTypes as string[] | undefined) ?? []; + const ancestorParamCount = ancestorMethodNode.properties.parameterCount as + | number + | undefined; - // Find matching method in own class by name + parameterTypes + // Find matching method in own class by name + parameterTypes/arity const candidates = ownMethodsByName.get(ancestorName); - if (!candidates) continue; - for (const candidate of candidates) { - if (parameterTypesMatch(candidate.parameterTypes, ancestorParamTypes)) { - const edgeKey = `${candidate.methodId}->${ancestorMethodId}`; - if (emitted.has(edgeKey)) break; - emitted.add(edgeKey); - - graph.addRelationship({ - id: generateId('METHOD_IMPLEMENTS', edgeKey), - sourceId: candidate.methodId, - targetId: ancestorMethodId, - type: 'METHOD_IMPLEMENTS', - confidence: 1.0, - reason: '', - }); - edgeCount++; - break; // first match wins for this ancestor method + // Unit 3: If no own method matches, walk the EXTENDS chain to find inherited concrete method + if (!candidates || candidates.length === 0) { + const inherited = findInheritedMethod( + classId, + ancestorName, + ancestorParamTypes, + ancestorParamCount, + graph, + parentMap, + methodMap, + parentEdgeType, + ); + if (inherited) { + const edgeKey = `${inherited.methodId}->${ancestorMethodId}`; + if (!emitted.has(edgeKey)) { + emitted.add(edgeKey); + graph.addRelationship({ + id: generateId('METHOD_IMPLEMENTS', edgeKey), + sourceId: inherited.methodId, + targetId: ancestorMethodId, + type: 'METHOD_IMPLEMENTS', + confidence: 1.0, + reason: '', + }); + edgeCount++; + } } + continue; } + + // Unit 4: Filter candidates by type/arity match, then check for ambiguity + const matching = candidates.filter((c) => + parameterTypesMatch( + c.parameterTypes, + ancestorParamTypes, + c.parameterCount, + ancestorParamCount, + ), + ); + + if (matching.length === 0) continue; + + // If multiple candidates match at name+arity level, emit no edge (ambiguous) + if (matching.length > 1) continue; + + const winner = matching[0]; + const edgeKey = `${winner.methodId}->${ancestorMethodId}`; + if (emitted.has(edgeKey)) continue; + emitted.add(edgeKey); + + graph.addRelationship({ + id: generateId('METHOD_IMPLEMENTS', edgeKey), + sourceId: winner.methodId, + targetId: ancestorMethodId, + type: 'METHOD_IMPLEMENTS', + confidence: 1.0, + reason: '', + }); + edgeCount++; } } } @@ -544,6 +600,75 @@ function emitMethodImplementsEdges( return edgeCount; } +/** + * Walk the class's EXTENDS chain (not IMPLEMENTS) to find the nearest + * concrete method matching the given name and parameter signature. + * Returns the first matching method found in BFS order, or null. + */ +function findInheritedMethod( + classId: string, + methodName: string, + targetParamTypes: string[], + targetParamCount: number | undefined, + graph: KnowledgeGraph, + parentMap: Map, + methodMap: Map, + parentEdgeType: Map>, +): { methodId: string; parameterTypes: string[] } | null { + const visited = new Set(); + const queue: string[] = []; + + // Seed with direct EXTENDS parents only + const directParents = parentMap.get(classId) ?? []; + const directEdges = parentEdgeType.get(classId); + for (const pid of directParents) { + const et = directEdges?.get(pid); + if (et === 'EXTENDS') { + // Also check that the parent is not an Interface/Trait + const parentNode = graph.getNode(pid); + if (parentNode && parentNode.label !== 'Interface' && parentNode.label !== 'Trait') { + queue.push(pid); + } + } + } + + while (queue.length > 0) { + const ancestorId = queue.shift()!; + if (visited.has(ancestorId)) continue; + visited.add(ancestorId); + + // Check this ancestor's methods + const methods = methodMap.get(ancestorId) ?? []; + for (const mid of methods) { + const mNode = graph.getNode(mid); + if (!mNode || mNode.label === 'Property') continue; + if (mNode.properties.name !== methodName) continue; + + 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 }; + } + } + + // Continue walking EXTENDS parents of this ancestor + const grandparents = parentMap.get(ancestorId) ?? []; + const ancestorEdges = parentEdgeType.get(ancestorId); + for (const gp of grandparents) { + if (visited.has(gp)) continue; + const gpEdge = ancestorEdges?.get(gp); + if (gpEdge === 'EXTENDS') { + const gpNode = graph.getNode(gp); + if (gpNode && gpNode.label !== 'Interface' && gpNode.label !== 'Trait') { + queue.push(gp); + } + } + } + } + + return null; +} + /** * Build transitive edge types for a class using BFS from the class to all ancestors. * diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index 1d54cbb21..af129e2f8 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -97,6 +97,7 @@ export const VALID_RELATION_TYPES = new Set([ 'HAS_METHOD', 'HAS_PROPERTY', 'METHOD_OVERRIDES', + 'OVERRIDES', // Legacy alias — dual-read for pre-rename indexes 'METHOD_IMPLEMENTS', 'ACCESSES', 'HANDLES_ROUTE', @@ -177,7 +178,6 @@ export class LocalBackend { private repos: Map = new Map(); private contextCache: Map = new Map(); private initializedRepos: Set = new Set(); - private migratedRepos: Set = new Set(); private reinitPromises: Map> = new Map(); private lastStalenessCheck: Map = new Map(); private groupToolSvc: GroupService | null = null; @@ -410,20 +410,6 @@ export class LocalBackend { try { await initLbug(repoId, handle.lbugPath); this.initializedRepos.add(repoId); - // Migrate legacy OVERRIDES → METHOD_OVERRIDES (once per session per repo) - // TODO: remove this migration after a few releases once most users have migrated indexes - if (!this.migratedRepos.has(repoId)) { - this.migratedRepos.add(repoId); - try { - await executeParameterized( - repoId, - `MATCH ()-[r:CodeRelation {type: 'OVERRIDES'}]->() SET r.type = 'METHOD_OVERRIDES'`, - {}, - ); - } catch { - /* Old index may not have OVERRIDES edges — ignore */ - } - } } catch (err: any) { // If lock error, mark as not initialized so next call retries this.initializedRepos.delete(repoId); @@ -1218,7 +1204,7 @@ export class LocalBackend { repo.id, ` MATCH (caller)-[r:CodeRelation]->(n {id: $symId}) - WHERE r.type IN ['CALLS', 'IMPORTS', 'EXTENDS', 'IMPLEMENTS', 'HAS_METHOD', 'HAS_PROPERTY', 'METHOD_OVERRIDES', 'METHOD_IMPLEMENTS', 'ACCESSES'] + WHERE r.type IN ['CALLS', 'IMPORTS', 'EXTENDS', 'IMPLEMENTS', 'HAS_METHOD', 'HAS_PROPERTY', 'METHOD_OVERRIDES', 'OVERRIDES', 'METHOD_IMPLEMENTS', 'ACCESSES'] RETURN r.type AS relType, caller.id AS uid, caller.name AS name, caller.filePath AS filePath, labels(caller)[0] AS kind LIMIT 30 `, @@ -1308,7 +1294,7 @@ export class LocalBackend { repo.id, ` MATCH (n {id: $symId})-[r:CodeRelation]->(target) - WHERE r.type IN ['CALLS', 'IMPORTS', 'EXTENDS', 'IMPLEMENTS', 'HAS_METHOD', 'HAS_PROPERTY', 'METHOD_OVERRIDES', 'METHOD_IMPLEMENTS', 'ACCESSES'] + WHERE r.type IN ['CALLS', 'IMPORTS', 'EXTENDS', 'IMPLEMENTS', 'HAS_METHOD', 'HAS_PROPERTY', 'METHOD_OVERRIDES', 'OVERRIDES', 'METHOD_IMPLEMENTS', 'ACCESSES'] RETURN r.type AS relType, target.id AS uid, target.name AS name, target.filePath AS filePath, labels(target)[0] AS kind LIMIT 30 `, @@ -1934,9 +1920,11 @@ export class LocalBackend { const rawRelTypes = mappedRelTypes && mappedRelTypes.length > 0 ? mappedRelTypes.filter((t: string) => VALID_RELATION_TYPES.has(t)) - : ['CALLS', 'IMPORTS', 'EXTENDS', 'IMPLEMENTS']; + : ['CALLS', 'IMPORTS', 'EXTENDS', 'IMPLEMENTS', 'METHOD_OVERRIDES', 'METHOD_IMPLEMENTS']; const relationTypes = - rawRelTypes.length > 0 ? rawRelTypes : ['CALLS', 'IMPORTS', 'EXTENDS', 'IMPLEMENTS']; + rawRelTypes.length > 0 + ? rawRelTypes + : ['CALLS', 'IMPORTS', 'EXTENDS', 'IMPLEMENTS', 'METHOD_OVERRIDES', 'METHOD_IMPLEMENTS']; const includeTests = params.includeTests ?? false; const minConfidence = params.minConfidence ?? 0; @@ -2486,9 +2474,11 @@ export class LocalBackend { const rawRelTypes = mappedRelTypes && mappedRelTypes.length > 0 ? mappedRelTypes.filter((t: string) => VALID_RELATION_TYPES.has(t)) - : ['CALLS', 'IMPORTS', 'EXTENDS', 'IMPLEMENTS']; + : ['CALLS', 'IMPORTS', 'EXTENDS', 'IMPLEMENTS', 'METHOD_OVERRIDES', 'METHOD_IMPLEMENTS']; const relationTypes = - rawRelTypes.length > 0 ? rawRelTypes : ['CALLS', 'IMPORTS', 'EXTENDS', 'IMPLEMENTS']; + rawRelTypes.length > 0 + ? rawRelTypes + : ['CALLS', 'IMPORTS', 'EXTENDS', 'IMPLEMENTS', 'METHOD_OVERRIDES', 'METHOD_IMPLEMENTS']; try { return await this._runImpactBFS(repo, sym, symType, dir, { diff --git a/gitnexus/test/unit/mro-processor.test.ts b/gitnexus/test/unit/mro-processor.test.ts index 9be1a9566..dcc8f821e 100644 --- a/gitnexus/test/unit/mro-processor.test.ts +++ b/gitnexus/test/unit/mro-processor.test.ts @@ -867,5 +867,227 @@ describe('computeMRO', () => { }); expect(implementingMethods).toContain(concreteId); }); + + describe('METHOD_IMPLEMENTS inherited + arity matching', () => { + it('inherited implementation: Base.foo satisfies I.foo when C has no own foo', () => { + const graph = createKnowledgeGraph(); + addClass(graph, 'Base', 'java'); + addClass(graph, 'I', 'java', 'Interface'); + addClass(graph, 'C', 'java'); + + addExtends(graph, 'C', 'Base'); + addImplements(graph, 'C', 'I'); + + const baseFoo = addMethod(graph, 'Base', 'foo'); + const iFoo = addMethod(graph, 'I', 'foo', 'Interface'); + + const result = computeMRO(graph); + + const edges: any[] = []; + graph.forEachRelationship((rel) => { + if (rel.type === 'METHOD_IMPLEMENTS') edges.push(rel); + }); + + expect(edges).toHaveLength(1); + expect(edges[0].sourceId).toBe(baseFoo); + expect(edges[0].targetId).toBe(iFoo); + expect(result.methodImplementsEdges).toBe(1); + }); + + it('class has own method — no inherited lookup needed', () => { + const graph = createKnowledgeGraph(); + addClass(graph, 'Base2', 'java'); + addClass(graph, 'I2', 'java', 'Interface'); + addClass(graph, 'C2', 'java'); + + addExtends(graph, 'C2', 'Base2'); + addImplements(graph, 'C2', 'I2'); + + const baseFoo = addMethod(graph, 'Base2', 'foo'); + const iFoo = addMethod(graph, 'I2', 'foo', 'Interface'); + const cFoo = addMethod(graph, 'C2', 'foo'); + + const result = computeMRO(graph); + + const edges: any[] = []; + graph.forEachRelationship((rel) => { + if (rel.type === 'METHOD_IMPLEMENTS') edges.push(rel); + }); + + // Should use C2.foo, not Base2.foo + expect(edges).toHaveLength(1); + expect(edges[0].sourceId).toBe(cFoo); + expect(edges[0].targetId).toBe(iFoo); + }); + + it('deep inheritance chain: GrandBase.foo satisfies I.foo', () => { + const graph = createKnowledgeGraph(); + addClass(graph, 'GrandBase', 'java'); + addClass(graph, 'Base3', 'java'); + addClass(graph, 'I3', 'java', 'Interface'); + addClass(graph, 'C3', 'java'); + + addExtends(graph, 'Base3', 'GrandBase'); + addExtends(graph, 'C3', 'Base3'); + addImplements(graph, 'C3', 'I3'); + + const grandFoo = addMethod(graph, 'GrandBase', 'foo'); + // Base3 has NO foo + const iFoo = addMethod(graph, 'I3', 'foo', 'Interface'); + + const result = computeMRO(graph); + + const edges: any[] = []; + graph.forEachRelationship((rel) => { + if (rel.type === 'METHOD_IMPLEMENTS') edges.push(rel); + }); + + expect(edges).toHaveLength(1); + expect(edges[0].sourceId).toBe(grandFoo); + expect(edges[0].targetId).toBe(iFoo); + expect(result.methodImplementsEdges).toBe(1); + }); + + it('arity mismatch prevents false match', () => { + const graph = createKnowledgeGraph(); + addClass(graph, 'IArity', 'java', 'Interface'); + addClass(graph, 'CArity', 'java'); + addImplements(graph, 'CArity', 'IArity'); + + // Interface method: parameterCount=2, no parameterTypes + const iMethodId = generateId('Method', 'IArity.process'); + graph.addNode({ + id: iMethodId, + label: 'Method', + properties: { name: 'process', filePath: 'src/IArity.ts', parameterCount: 2 }, + }); + graph.addRelationship({ + id: generateId('HAS_METHOD', `${generateId('Interface', 'IArity')}->${iMethodId}`), + sourceId: generateId('Interface', 'IArity'), + targetId: iMethodId, + type: 'HAS_METHOD', + confidence: 1.0, + reason: '', + }); + + // Class method: parameterCount=3, no parameterTypes + const cMethodId = generateId('Method', 'CArity.process'); + graph.addNode({ + id: cMethodId, + label: 'Method', + properties: { name: 'process', filePath: 'src/CArity.ts', parameterCount: 3 }, + }); + graph.addRelationship({ + id: generateId('HAS_METHOD', `${generateId('Class', 'CArity')}->${cMethodId}`), + sourceId: generateId('Class', 'CArity'), + targetId: cMethodId, + type: 'HAS_METHOD', + confidence: 1.0, + reason: '', + }); + + const result = computeMRO(graph); + expect(result.methodImplementsEdges).toBe(0); + }); + + it('arity match when types missing', () => { + const graph = createKnowledgeGraph(); + addClass(graph, 'IArityOk', 'java', 'Interface'); + addClass(graph, 'CArityOk', 'java'); + addImplements(graph, 'CArityOk', 'IArityOk'); + + // Interface method: parameterCount=2, no parameterTypes + const iMethodId = generateId('Method', 'IArityOk.process'); + graph.addNode({ + id: iMethodId, + label: 'Method', + properties: { name: 'process', filePath: 'src/IArityOk.ts', parameterCount: 2 }, + }); + graph.addRelationship({ + id: generateId('HAS_METHOD', `${generateId('Interface', 'IArityOk')}->${iMethodId}`), + sourceId: generateId('Interface', 'IArityOk'), + targetId: iMethodId, + type: 'HAS_METHOD', + confidence: 1.0, + reason: '', + }); + + // Class method: parameterCount=2, no parameterTypes + const cMethodId = generateId('Method', 'CArityOk.process'); + graph.addNode({ + id: cMethodId, + label: 'Method', + properties: { name: 'process', filePath: 'src/CArityOk.ts', parameterCount: 2 }, + }); + graph.addRelationship({ + id: generateId('HAS_METHOD', `${generateId('Class', 'CArityOk')}->${cMethodId}`), + sourceId: generateId('Class', 'CArityOk'), + targetId: cMethodId, + type: 'HAS_METHOD', + confidence: 1.0, + reason: '', + }); + + const result = computeMRO(graph); + expect(result.methodImplementsEdges).toBe(1); + }); + + it('multiple same-arity candidates = ambiguous, no edge emitted', () => { + const graph = createKnowledgeGraph(); + addClass(graph, 'IAmbig', 'java', 'Interface'); + addClass(graph, 'CAmbig', 'java'); + addImplements(graph, 'CAmbig', 'IAmbig'); + + // Interface method: parameterCount=1, no parameterTypes + const iMethodId = generateId('Method', 'IAmbig.handle'); + graph.addNode({ + id: iMethodId, + label: 'Method', + properties: { name: 'handle', filePath: 'src/IAmbig.ts', parameterCount: 1 }, + }); + graph.addRelationship({ + id: generateId('HAS_METHOD', `${generateId('Interface', 'IAmbig')}->${iMethodId}`), + sourceId: generateId('Interface', 'IAmbig'), + targetId: iMethodId, + type: 'HAS_METHOD', + confidence: 1.0, + reason: '', + }); + + // Two class methods named handle, both with parameterCount=1 + const cMethod1 = generateId('Method', 'CAmbig.handle.1'); + graph.addNode({ + id: cMethod1, + label: 'Method', + properties: { name: 'handle', filePath: 'src/CAmbig.ts', parameterCount: 1 }, + }); + graph.addRelationship({ + id: generateId('HAS_METHOD', `${generateId('Class', 'CAmbig')}->${cMethod1}`), + sourceId: generateId('Class', 'CAmbig'), + targetId: cMethod1, + type: 'HAS_METHOD', + confidence: 1.0, + reason: '', + }); + + const cMethod2 = generateId('Method', 'CAmbig.handle.2'); + graph.addNode({ + id: cMethod2, + label: 'Method', + properties: { name: 'handle', filePath: 'src/CAmbig.ts', parameterCount: 1 }, + }); + graph.addRelationship({ + id: generateId('HAS_METHOD', `${generateId('Class', 'CAmbig')}->${cMethod2}`), + sourceId: generateId('Class', 'CAmbig'), + targetId: cMethod2, + type: 'HAS_METHOD', + confidence: 1.0, + reason: '', + }); + + const result = computeMRO(graph); + expect(result.methodImplementsEdges).toBe(0); + }); + }); }); }); diff --git a/gitnexus/test/unit/security.test.ts b/gitnexus/test/unit/security.test.ts index ac6cb8d0e..0adee1915 100644 --- a/gitnexus/test/unit/security.test.ts +++ b/gitnexus/test/unit/security.test.ts @@ -106,7 +106,7 @@ describe('isWriteQuery', () => { describe('VALID_RELATION_TYPES', () => { it('contains all expected relation types', () => { - expect(VALID_RELATION_TYPES.size).toBe(14); + expect(VALID_RELATION_TYPES.size).toBe(15); for (const t of [ 'CALLS', 'IMPORTS', @@ -115,6 +115,7 @@ describe('VALID_RELATION_TYPES', () => { 'HAS_METHOD', 'HAS_PROPERTY', 'METHOD_OVERRIDES', + 'OVERRIDES', 'METHOD_IMPLEMENTS', 'ACCESSES', 'HANDLES_ROUTE',