scriptoriumadmin/GitNexus
1
0
Fork 0
mirror of https://github.com/abhigyanpatwari/GitNexus.git synced 2026-09-20 00:11:37 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 7

fix: adversarial review findings for METHOD_IMPLEMENTS

Browse source 
Address 3 findings from Codex adversarial review:

1. Legacy OVERRIDES backward compat: map OVERRIDES → METHOD_OVERRIDES
   at the _impactImpl API boundary before VALID_RELATION_TYPES filtering.
   Prevents silent traversal widening when clients send legacy type name.

2. Thread METHOD_IMPLEMENTS through MCP tools:
   - Added to VALID_RELATION_TYPES (impact traversal)
   - Added to IMPACT_RELATION_CONFIDENCE (0.85 floor)
   - Added to context() incoming/outgoing Cypher queries
   - Updated security + confidence tests

3. Transitive interface ancestor support: emitMethodImplementsEdges()
   now uses gatherAncestors() + buildTransitiveEdgeTypes() to walk the
   full inheritance chain. C implements B extends A (interface) now
   links C.foo to both B.foo and A.foo. Diamond paths deduplicated.
   4 new unit tests covering transitive chains, inherited contracts,
   diamond dedup, and class-only exclusion.
This commit is contained in:
Gergo Magyar 2026-04-03 17:57:27 +01:00
parent deabfcb046
commit 8521189044
5 changed files with 191 additions and 29 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

51
gitnexus/src/core/ingestion/mro-processor.ts
View file

@ -488,42 +488,53 @@ function emitMethodImplementsEdges(
bucket.push({ methodId, parameterTypes });
}
// For each parent, check if it's an interface/trait or connected via IMPLEMENTS
for (const parentId of parentIds) {
const parentNode = graph.getNode(parentId);
if (!parentNode) continue;
// Collect ALL transitive ancestors and classify each as EXTENDS or IMPLEMENTS
const allAncestors = gatherAncestors(classId, parentMap);
const ancestorEdgeTypes = buildTransitiveEdgeTypes(classId, parentMap, parentEdgeType);
const isInterfaceLike = parentNode.label === 'Interface' || parentNode.label === 'Trait';
const edgeType = parentEdgeType.get(classId)?.get(parentId);
if (!isInterfaceLike && edgeType !== 'IMPLEMENTS') continue;
// Dedup set: avoid duplicate edges from diamond paths
const emitted = new Set<string>();
// Get parent's methods
const parentMethodIds = methodMap.get(parentId) ?? [];
// For each ancestor, check if it's an interface/trait or classified as IMPLEMENTS
for (const ancestorId of allAncestors) {
const ancestorNode = graph.getNode(ancestorId);
if (!ancestorNode) continue;
for (const parentMethodId of parentMethodIds) {
const parentMethodNode = graph.getNode(parentMethodId);
if (!parentMethodNode || parentMethodNode.label === 'Property') continue;
const isInterfaceLike = ancestorNode.label === 'Interface' || ancestorNode.label === 'Trait';
const classifiedEdgeType = ancestorEdgeTypes.get(ancestorId);
if (!isInterfaceLike && classifiedEdgeType !== 'IMPLEMENTS') continue;
const parentName = parentMethodNode.properties.name as string;
const parentParamTypes =
(parentMethodNode.properties.parameterTypes as string[] | undefined) ?? [];
// Get ancestor's methods
const ancestorMethodIds = methodMap.get(ancestorId) ?? [];
for (const ancestorMethodId of ancestorMethodIds) {
const ancestorMethodNode = graph.getNode(ancestorMethodId);
if (!ancestorMethodNode || ancestorMethodNode.label === 'Property') continue;
const ancestorName = ancestorMethodNode.properties.name as string;
const ancestorParamTypes =
(ancestorMethodNode.properties.parameterTypes as string[] | undefined) ?? [];
// Find matching method in own class by name + parameterTypes
const candidates = ownMethodsByName.get(parentName);
const candidates = ownMethodsByName.get(ancestorName);
if (!candidates) continue;
for (const candidate of candidates) {
if (parameterTypesMatch(candidate.parameterTypes, parentParamTypes)) {
if (parameterTypesMatch(candidate.parameterTypes, ancestorParamTypes)) {
const edgeKey = `${candidate.methodId}->${ancestorMethodId}`;
if (emitted.has(edgeKey)) break;
emitted.add(edgeKey);
graph.addRelationship({
id: generateId('METHOD_IMPLEMENTS', `${candidate.methodId}->${parentMethodId}`),
id: generateId('METHOD_IMPLEMENTS', edgeKey),
sourceId: candidate.methodId,
targetId: parentMethodId,
targetId: ancestorMethodId,
type: 'METHOD_IMPLEMENTS',
confidence: 1.0,
reason: '',
});
edgeCount++;
break; // first match wins for this parent method
break; // first match wins for this ancestor method
}
}
}

25
gitnexus/src/mcp/local/local-backend.ts
View file

@ -97,6 +97,7 @@ export const VALID_RELATION_TYPES = new Set([
'HAS_METHOD',
'HAS_PROPERTY',
'METHOD_OVERRIDES',
'METHOD_IMPLEMENTS',
'ACCESSES',
'HANDLES_ROUTE',
'FETCHES',
@ -117,7 +118,8 @@ export const VALID_RELATION_TYPES = new Set([
* CALLS / IMPORTS – direct, strongly-typed references → 0.9
* EXTENDS – class hierarchy, statically verifiable → 0.85
* IMPLEMENTS – interface contract, statically verifiable → 0.85
* METHOD_OVERRIDES – method override, statically verifiable → 0.85
* METHOD_OVERRIDES – method override, statically verifiable → 0.85
* METHOD_IMPLEMENTS – interface method implementation, statically verifiable → 0.85
* HAS_METHOD – structural containment → 0.95
* HAS_PROPERTY – structural containment → 0.95
* ACCESSES – field read/write, may be indirect → 0.8
@ -130,6 +132,7 @@ export const IMPACT_RELATION_CONFIDENCE: Readonly<Record<string, number>> = {
EXTENDS: 0.85,
IMPLEMENTS: 0.85,
METHOD_OVERRIDES: 0.85,
METHOD_IMPLEMENTS: 0.85,
HAS_METHOD: 0.95,
HAS_PROPERTY: 0.95,
ACCESSES: 0.8,
@ -1215,7 +1218,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', 'ACCESSES']
WHERE r.type IN ['CALLS', 'IMPORTS', 'EXTENDS', 'IMPLEMENTS', 'HAS_METHOD', 'HAS_PROPERTY', 'METHOD_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
`,
@ -1305,7 +1308,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', 'ACCESSES']
WHERE r.type IN ['CALLS', 'IMPORTS', 'EXTENDS', 'IMPLEMENTS', 'HAS_METHOD', 'HAS_PROPERTY', 'METHOD_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
`,
@ -1924,9 +1927,13 @@ export class LocalBackend {
const { target, direction } = params;
const maxDepth = params.maxDepth || 3;
// Map legacy relation type names before filtering (backward compat for OVERRIDES → METHOD_OVERRIDES)
const mappedRelTypes = params.relationTypes?.map((t: string) =>
t === 'OVERRIDES' ? 'METHOD_OVERRIDES' : t,
);
const rawRelTypes =
params.relationTypes && params.relationTypes.length > 0
? params.relationTypes.filter((t) => VALID_RELATION_TYPES.has(t))
mappedRelTypes && mappedRelTypes.length > 0
? mappedRelTypes.filter((t: string) => VALID_RELATION_TYPES.has(t))
: ['CALLS', 'IMPORTS', 'EXTENDS', 'IMPLEMENTS'];
const relationTypes =
rawRelTypes.length > 0 ? rawRelTypes : ['CALLS', 'IMPORTS', 'EXTENDS', 'IMPLEMENTS'];
@ -2472,9 +2479,13 @@ export class LocalBackend {
const symType =
typeof labelRaw === 'string' && labelRaw.trim().length > 0 ? labelRaw.trim() : '';
// Map legacy relation type names (backward compat for OVERRIDES → METHOD_OVERRIDES)
const mappedRelTypes = opts.relationTypes?.map((t: string) =>
t === 'OVERRIDES' ? 'METHOD_OVERRIDES' : t,
);
const rawRelTypes =
opts.relationTypes && opts.relationTypes.length > 0
? opts.relationTypes.filter((t) => VALID_RELATION_TYPES.has(t))
mappedRelTypes && mappedRelTypes.length > 0
? mappedRelTypes.filter((t: string) => VALID_RELATION_TYPES.has(t))
: ['CALLS', 'IMPORTS', 'EXTENDS', 'IMPLEMENTS'];
const relationTypes =
rawRelTypes.length > 0 ? rawRelTypes : ['CALLS', 'IMPORTS', 'EXTENDS', 'IMPLEMENTS'];

7
gitnexus/test/unit/impact-confidence.test.ts
View file

@ -34,10 +34,14 @@ describe('IMPACT_RELATION_CONFIDENCE', () => {
expect(IMPACT_RELATION_CONFIDENCE['IMPLEMENTS']).toBe(0.85);
});
it('OVERRIDES has confidence 0.85 (statically verifiable override)', () => {
it('METHOD_OVERRIDES has confidence 0.85 (statically verifiable override)', () => {
expect(IMPACT_RELATION_CONFIDENCE['METHOD_OVERRIDES']).toBe(0.85);
});
it('METHOD_IMPLEMENTS has confidence 0.85 (statically verifiable implementation)', () => {
expect(IMPACT_RELATION_CONFIDENCE['METHOD_IMPLEMENTS']).toBe(0.85);
});
it('HAS_METHOD has confidence 0.95 (structural containment)', () => {
expect(IMPACT_RELATION_CONFIDENCE['HAS_METHOD']).toBe(0.95);
});
@ -77,6 +81,7 @@ describe('confidenceForRelType', () => {
expect(confidenceForRelType('EXTENDS')).toBe(0.85);
expect(confidenceForRelType('IMPLEMENTS')).toBe(0.85);
expect(confidenceForRelType('METHOD_OVERRIDES')).toBe(0.85);
expect(confidenceForRelType('METHOD_IMPLEMENTS')).toBe(0.85);
expect(confidenceForRelType('HAS_METHOD')).toBe(0.95);
expect(confidenceForRelType('HAS_PROPERTY')).toBe(0.95);
expect(confidenceForRelType('ACCESSES')).toBe(0.8);

134
gitnexus/test/unit/mro-processor.test.ts
View file

@ -71,6 +71,25 @@ function addExtends(
});
}
function addInterfaceExtends(
graph: KnowledgeGraph,
childName: string,
parentName: string,
childLabel: 'Interface' | 'Trait' = 'Interface',
parentLabel: 'Interface' | 'Trait' = 'Interface',
) {
const childId = generateId(childLabel, childName);
const parentId = generateId(parentLabel, parentName);
graph.addRelationship({
id: generateId('EXTENDS', `${childId}->${parentId}`),
sourceId: childId,
targetId: parentId,
type: 'EXTENDS',
confidence: 1.0,
reason: '',
});
}
function addImplements(
graph: KnowledgeGraph,
childName: string,
@ -714,6 +733,121 @@ describe('computeMRO', () => {
expect(result.methodImplementsEdges).toBe(0);
});
describe('METHOD_IMPLEMENTS transitive ancestors', () => {
it('transitive interface chain: C.foo links to both B.foo and A.foo', () => {
// A (Interface) has foo, B (Interface) has foo extends A, C (Class) implements B
const graph = createKnowledgeGraph();
addClass(graph, 'A', 'java', 'Interface');
addClass(graph, 'B', 'java', 'Interface');
addClass(graph, 'C', 'java');
addInterfaceExtends(graph, 'B', 'A');
addImplements(graph, 'C', 'B');
const aFoo = addMethod(graph, 'A', 'foo', 'Interface');
const bFoo = addMethod(graph, 'B', 'foo', 'Interface');
addMethod(graph, 'C', 'foo');
const result = computeMRO(graph);
const edges: any[] = [];
graph.forEachRelationship((rel) => {
if (rel.type === 'METHOD_IMPLEMENTS') edges.push(rel);
});
// C.foo should link to both B.foo and A.foo
expect(edges.some((e) => e.targetId === bFoo)).toBe(true);
expect(edges.some((e) => e.targetId === aFoo)).toBe(true);
expect(result.methodImplementsEdges).toBeGreaterThanOrEqual(2);
});
it('inherited contract method only on grandparent: C.bar links to A.bar', () => {
// A (Interface) has bar, B (Interface) extends A but has NO bar, C implements B
const graph = createKnowledgeGraph();
addClass(graph, 'A', 'java', 'Interface');
addClass(graph, 'B', 'java', 'Interface');
addClass(graph, 'C', 'java');
addInterfaceExtends(graph, 'B', 'A');
addImplements(graph, 'C', 'B');
const aBar = addMethod(graph, 'A', 'bar', 'Interface');
// B has no bar method
addMethod(graph, 'C', 'bar');
const result = computeMRO(graph);
const edges: any[] = [];
graph.forEachRelationship((rel) => {
if (rel.type === 'METHOD_IMPLEMENTS') edges.push(rel);
});
// C.bar should link to A.bar even though A is not a direct parent
expect(edges.some((e) => e.targetId === aBar)).toBe(true);
expect(result.methodImplementsEdges).toBeGreaterThanOrEqual(1);
});
it('diamond deduplication: E.foo gets exactly one edge to A.foo', () => {
// A (Interface) has foo
// B (Interface) has foo, extends A
// D (Interface) has foo, extends A
// E (Class) implements B and D
const graph = createKnowledgeGraph();
addClass(graph, 'A', 'java', 'Interface');
addClass(graph, 'B', 'java', 'Interface');
addClass(graph, 'D', 'java', 'Interface');
addClass(graph, 'E', 'java');
addInterfaceExtends(graph, 'B', 'A');
addInterfaceExtends(graph, 'D', 'A');
addImplements(graph, 'E', 'B');
addImplements(graph, 'E', 'D');
const aFoo = addMethod(graph, 'A', 'foo', 'Interface');
const bFoo = addMethod(graph, 'B', 'foo', 'Interface');
const dFoo = addMethod(graph, 'D', 'foo', 'Interface');
addMethod(graph, 'E', 'foo');
const result = computeMRO(graph);
const eFoo = generateId('Method', 'E.foo');
const edges: any[] = [];
graph.forEachRelationship((rel) => {
if (rel.type === 'METHOD_IMPLEMENTS') edges.push(rel);
});
// Filter to only edges FROM E.foo
const eFooEdges = edges.filter((e) => e.sourceId === eFoo);
// E.foo should link to B.foo, D.foo, and exactly ONE A.foo (deduplicated)
expect(eFooEdges.filter((e) => e.targetId === bFoo)).toHaveLength(1);
expect(eFooEdges.filter((e) => e.targetId === dFoo)).toHaveLength(1);
expect(eFooEdges.filter((e) => e.targetId === aFoo)).toHaveLength(1);
// Total from E.foo: 3 edges (B.foo + D.foo + A.foo), not 4
expect(eFooEdges).toHaveLength(3);
});
it('no transitive through class-only chain', () => {
// A (Class) has foo, B (Class) extends A has foo, C (Class) extends B has foo
const graph = createKnowledgeGraph();
addClass(graph, 'A', 'java');
addClass(graph, 'B', 'java');
addClass(graph, 'C', 'java');
addExtends(graph, 'B', 'A');
addExtends(graph, 'C', 'B');
addMethod(graph, 'A', 'foo');
addMethod(graph, 'B', 'foo');
addMethod(graph, 'C', 'foo');
const result = computeMRO(graph);
// All class-extends, no interface involved → 0 METHOD_IMPLEMENTS edges
expect(result.methodImplementsEdges).toBe(0);
});
});
it('is queryable via MATCH pattern', () => {
const graph = createKnowledgeGraph();
addClass(graph, 'IRepo', 'typescript', 'Interface');

3
gitnexus/test/unit/security.test.ts
View file

@ -106,7 +106,7 @@ describe('isWriteQuery', () => {
describe('VALID_RELATION_TYPES', () => {
it('contains all expected relation types', () => {
expect(VALID_RELATION_TYPES.size).toBe(13);
expect(VALID_RELATION_TYPES.size).toBe(14);
for (const t of [
'CALLS',
'IMPORTS',
@ -115,6 +115,7 @@ describe('VALID_RELATION_TYPES', () => {
'HAS_METHOD',
'HAS_PROPERTY',
'METHOD_OVERRIDES',
'METHOD_IMPLEMENTS',
'ACCESSES',
'HANDLES_ROUTE',
'FETCHES',