fix: Codex Round 2 — METHOD_IMPLEMENTS hardening

1. Replace init-time OVERRIDES migration with dual-read: remove the
   unversioned database mutation from ensureInitialized, add OVERRIDES
   to Cypher IN clauses and VALID_RELATION_TYPES so both old and new
   edge type names are recognized without mutating the index.

2. Expand default impact traversal: METHOD_OVERRIDES and
   METHOD_IMPLEMENTS now included in the default fallback so interface
   contract changes report blast radius without explicit relationTypes.

3. Inherited implementation resolution: when class C extends Base and
   implements I, but only Base has foo(), emit METHOD_IMPLEMENTS from
   Base.foo → I.foo by walking the EXTENDS chain for concrete methods.

4. Arity-compatible matching: when parameterTypes are missing, fall back
   to parameterCount comparison. Multiple same-arity candidates produce
   no edge (ambiguous) instead of picking the first match.

6 new tests: inherited implementation (3 depths), arity mismatch,
arity match without types, ambiguous same-arity candidates.
This commit is contained in:
Gergo Magyar 2026-04-03 19:44:18 +01:00
parent 8521189044
commit 4205261c92
4 changed files with 385 additions and 47 deletions

View file

@ -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<string, string[]>,
methodMap: Map<string, string[]>,
parentEdgeType: Map<string, Map<string, 'EXTENDS' | 'IMPLEMENTS'>>,
): { methodId: string; parameterTypes: string[] } | null {
const visited = new Set<string>();
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.
*

View file

@ -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<string, RepoHandle> = new Map();
private contextCache: Map<string, CodebaseContext> = new Map();
private initializedRepos: Set<string> = new Set();
private migratedRepos: Set<string> = new Set();
private reinitPromises: Map<string, Promise<void>> = new Map();
private lastStalenessCheck: Map<string, number> = 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, {

View file

@ -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);
});
});
});
});

View file

@ -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',