diff --git a/understand-anything-plugin/packages/core/src/__tests__/schema.test.ts b/understand-anything-plugin/packages/core/src/__tests__/schema.test.ts index b57a6e5d3..b54674f3a 100644 --- a/understand-anything-plugin/packages/core/src/__tests__/schema.test.ts +++ b/understand-anything-plugin/packages/core/src/__tests__/schema.test.ts @@ -570,6 +570,48 @@ describe("permissive validation", () => { expect(result.issues.some((i) => i.level === "dropped")).toBe(true); }); + it("drops duplicate node IDs and keeps the first valid node", () => { + const graph = structuredClone(validGraph); + graph.nodes.push({ + ...structuredClone(graph.nodes[0]), + name: "duplicate.ts", + summary: "Duplicate node that must not reach the dashboard", + }); + + const result = validateGraph(graph); + + expect(result.success).toBe(true); + expect(result.data!.nodes).toHaveLength(1); + expect(result.data!.nodes[0].name).toBe("index.ts"); + expect(result.issues).toContainEqual({ + level: "dropped", + category: "duplicate-node-id", + message: 'nodes[1]: duplicate id "node-1" — removed', + path: "nodes[1].id", + }); + }); + + it("keeps a later valid node when an earlier matching ID is invalid", () => { + const graph = structuredClone(validGraph); + (graph.nodes[0] as any).type = "invalid_type"; + graph.nodes.push({ + ...structuredClone(validGraph.nodes[0]), + name: "valid.ts", + }); + + const result = validateGraph(graph); + + expect(result.success).toBe(true); + expect(result.data!.nodes).toHaveLength(1); + expect(result.data!.nodes[0].name).toBe("valid.ts"); + expect(result.issues).toContainEqual( + expect.objectContaining({ level: "dropped", category: "invalid-node" }) + ); + expect(result.issues).not.toContainEqual( + expect.objectContaining({ category: "duplicate-node-id" }) + ); + }); + it("filters dangling nodeIds from layers", () => { const graph = structuredClone(validGraph); graph.layers[0].nodeIds.push("non-existent-node"); diff --git a/understand-anything-plugin/packages/core/src/analyzer/graph-builder.test.ts b/understand-anything-plugin/packages/core/src/analyzer/graph-builder.test.ts index a9dc9c905..c479f07a1 100644 --- a/understand-anything-plugin/packages/core/src/analyzer/graph-builder.test.ts +++ b/understand-anything-plugin/packages/core/src/analyzer/graph-builder.test.ts @@ -134,6 +134,54 @@ describe("GraphBuilder", () => { }); }); + it("skips duplicate structural node IDs and their contains edges", () => { + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + const builder = new GraphBuilder("test-project", "abc123"); + const analysis: StructuralAnalysis = { + functions: [ + { name: "load", lineRange: [10, 12], params: ["value"], returnType: "void" }, + { name: "load", lineRange: [14, 16], params: ["value"], returnType: "void" }, + ], + classes: [ + { name: "Loader", lineRange: [1, 20], methods: ["load"], properties: [] }, + { name: "Loader", lineRange: [22, 40], methods: ["load"], properties: [] }, + ], + imports: [], + exports: [], + }; + + builder.addFileWithAnalysis("src/Loader.java", analysis, { + summary: "Loader source", + tags: [], + complexity: "moderate", + fileSummary: "Contains overloaded load methods", + summaries: { + load: "Loads a value", + Loader: "Loads values", + }, + }); + + const graph = builder.build(); + const nodeIds = graph.nodes.map((node) => node.id); + const containsEdges = graph.edges.filter((edge) => edge.type === "contains"); + + expect(new Set(nodeIds).size).toBe(nodeIds.length); + expect(graph.nodes.filter((node) => node.type === "function")).toHaveLength(1); + expect(graph.nodes.filter((node) => node.type === "class")).toHaveLength(1); + expect(graph.nodes.find((node) => node.type === "function")?.lineRange).toEqual([10, 12]); + expect(graph.nodes.find((node) => node.type === "class")?.lineRange).toEqual([1, 20]); + expect(containsEdges).toHaveLength(2); + expect(warnSpy).toHaveBeenCalledTimes(2); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('Duplicate node ID "function:src/Loader.java:load"'), + ); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('Duplicate node ID "class:src/Loader.java:Loader"'), + ); + + warnSpy.mockRestore(); + }); + it("should create import edges between files", () => { const builder = new GraphBuilder("test-project", "abc123"); diff --git a/understand-anything-plugin/packages/core/src/analyzer/graph-builder.ts b/understand-anything-plugin/packages/core/src/analyzer/graph-builder.ts index 16da567d5..2823b8106 100644 --- a/understand-anything-plugin/packages/core/src/analyzer/graph-builder.ts +++ b/understand-anything-plugin/packages/core/src/analyzer/graph-builder.ts @@ -130,8 +130,7 @@ export class GraphBuilder { // Create function nodes with "contains" edges for (const fn of analysis.functions) { const funcId = `function:${filePath}:${fn.name}`; - this.nodeIds.add(funcId); - this.nodes.push({ + this.addChildNode({ id: funcId, type: "function", name: fn.name, @@ -140,22 +139,13 @@ export class GraphBuilder { summary: meta.summaries[fn.name] ?? "", tags: [], complexity: meta.complexity, - }); - - this.edges.push({ - source: fileId, - target: funcId, - type: "contains", - direction: "forward", - weight: 1, - }); + }, fileId); } // Create class nodes with "contains" edges for (const cls of analysis.classes) { const classId = `class:${filePath}:${cls.name}`; - this.nodeIds.add(classId); - this.nodes.push({ + this.addChildNode({ id: classId, type: "class", name: cls.name, @@ -164,15 +154,7 @@ export class GraphBuilder { summary: meta.summaries[cls.name] ?? "", tags: [], complexity: meta.complexity, - }); - - this.edges.push({ - source: fileId, - target: classId, - type: "contains", - direction: "forward", - weight: 1, - }); + }, fileId); } } diff --git a/understand-anything-plugin/packages/core/src/plugins/extractors/__tests__/java-extractor.test.ts b/understand-anything-plugin/packages/core/src/plugins/extractors/__tests__/java-extractor.test.ts index 700e99851..3e0cee2d3 100644 --- a/understand-anything-plugin/packages/core/src/plugins/extractors/__tests__/java-extractor.test.ts +++ b/understand-anything-plugin/packages/core/src/plugins/extractors/__tests__/java-extractor.test.ts @@ -97,6 +97,23 @@ describe("JavaExtractor", () => { parser.delete(); }); + it("preserves overloaded methods as distinct structural entries", () => { + const { tree, parser, root } = parse(`public class Loader { + public void load(String value) {} + public void load(int value) {} +} +`); + const result = extractor.extractStructure(root); + const overloads = result.functions.filter((fn) => fn.name === "load"); + + expect(overloads).toHaveLength(2); + expect(overloads.map((fn) => fn.params)).toEqual([["value"], ["value"]]); + expect(overloads[0].lineRange).not.toEqual(overloads[1].lineRange); + + tree.delete(); + parser.delete(); + }); + it("extracts methods with generic return types", () => { const { tree, parser, root } = parse(`public class Foo { public List getItems() { diff --git a/understand-anything-plugin/packages/core/src/schema.ts b/understand-anything-plugin/packages/core/src/schema.ts index a4c308aef..0af23d99e 100644 --- a/understand-anything-plugin/packages/core/src/schema.ts +++ b/understand-anything-plugin/packages/core/src/schema.ts @@ -606,11 +606,22 @@ export function validateGraph(data: unknown): ValidationResult { // Tier 3: Validate nodes individually, drop broken const validNodes: z.infer[] = []; + const nodeIds = new Set(); if (Array.isArray(fixed.nodes)) { for (let i = 0; i < fixed.nodes.length; i++) { const node = fixed.nodes[i] as Record; const result = GraphNodeSchema.safeParse(node); if (result.success) { + if (nodeIds.has(result.data.id)) { + issues.push({ + level: "dropped", + category: "duplicate-node-id", + message: `nodes[${i}]: duplicate id "${result.data.id}" — removed`, + path: `nodes[${i}].id`, + }); + continue; + } + nodeIds.add(result.data.id); validNodes.push(result.data); } else { const name = node?.name || node?.id || `index ${i}`; @@ -635,7 +646,6 @@ export function validateGraph(data: unknown): ValidationResult { } // Tier 3: Validate edges + referential integrity - const nodeIds = new Set(validNodes.map((n) => n.id)); const validEdges: z.infer[] = []; if (Array.isArray(fixed.edges)) { for (let i = 0; i < fixed.edges.length; i++) {