Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> getItems() {
Expand Down
12 changes: 11 additions & 1 deletion understand-anything-plugin/packages/core/src/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -606,11 +606,22 @@ export function validateGraph(data: unknown): ValidationResult {

// Tier 3: Validate nodes individually, drop broken
const validNodes: z.infer<typeof GraphNodeSchema>[] = [];
const nodeIds = new Set<string>();
if (Array.isArray(fixed.nodes)) {
for (let i = 0; i < fixed.nodes.length; i++) {
const node = fixed.nodes[i] as Record<string, unknown>;
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}`;
Expand All @@ -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<typeof GraphEdgeSchema>[] = [];
if (Array.isArray(fixed.edges)) {
for (let i = 0; i < fixed.edges.length; i++) {
Expand Down