diff --git a/understand-anything-plugin/packages/dashboard/src/utils/__tests__/elk-layout.test.ts b/understand-anything-plugin/packages/dashboard/src/utils/__tests__/elk-layout.test.ts index fc13b5977..ffe0d4f81 100644 --- a/understand-anything-plugin/packages/dashboard/src/utils/__tests__/elk-layout.test.ts +++ b/understand-anything-plugin/packages/dashboard/src/utils/__tests__/elk-layout.test.ts @@ -42,6 +42,107 @@ describe("repairElkInput", () => { expect(issues.some((i) => i.level === "dropped" && /edge/.test(i.message))).toBe(true); }); + it("drops edges pointing at children removed by the orphan-child pass", () => { + const input: ElkInput = { + id: "root", + children: [ + { id: "a", width: 1, height: 1 }, + { id: "orphan", width: 1, height: 1, parentId: "ghost" } as ElkInput["children"][0] & { parentId: string }, + ], + edges: [{ id: "e1", sources: ["a"], targets: ["orphan"] }], + }; + const { input: out, issues } = repairElkInput(input); + expect(out.children!.find((c) => c.id === "orphan")).toBeUndefined(); + expect(out.edges).toHaveLength(0); + expect(issues.some((i) => i.level === "dropped" && i.category === "elk-orphan-edge")).toBe(true); + }); + + it("drops edges pointing at a node removed by the containment-cycle pass", () => { + // x and y mutually contain each other (via shared ids at different levels), + // so fillParents records parentOf(x)=y and parentOf(y)=x and the cycle pass + // removes both. An edge targeting x must then be reconciled away by step 5. + const input: ElkInput = { + id: "root", + children: [ + { id: "x", width: 1, height: 1, children: [{ id: "y", width: 1, height: 1 }] }, + { id: "y", width: 1, height: 1, children: [{ id: "x", width: 1, height: 1 }] }, + { id: "a", width: 1, height: 1 }, + ] as ElkInput["children"], + edges: [{ id: "e1", sources: ["a"], targets: ["x"] }], + }; + const { input: out, issues } = repairElkInput(input); + expect(out.children!.find((c) => c.id === "x")).toBeUndefined(); + expect(out.children!.find((c) => c.id === "y")).toBeUndefined(); + expect(out.edges).toHaveLength(0); + expect( + issues.some((i) => i.level === "dropped" && i.category === "elk-containment-cycle"), + ).toBe(true); + expect( + issues.some( + (i) => + i.level === "dropped" && + i.category === "elk-orphan-edge" && + /\be1\b/.test(i.message), + ), + ).toBe(true); + }); + + it("drops edges pointing at a nested grandchild carried out by an orphan-child parent drop", () => { + // Parent `p` has a missing parentId, so the orphan-child pass (step 3) drops + // it AND its whole subtree, including the nested grandchild `gc`. An edge + // targeting `gc` must be reconciled away by step 5 even though no pass + // explicitly tracked `gc` being removed. + const input: ElkInput = { + id: "root", + children: [ + { id: "a", width: 1, height: 1 }, + { + id: "p", + width: 1, + height: 1, + parentId: "ghost", + children: [{ id: "gc", width: 1, height: 1 }], + } as ElkInput["children"][0] & { parentId: string }, + ], + edges: [{ id: "e1", sources: ["a"], targets: ["gc"] }], + }; + const { input: out, issues } = repairElkInput(input); + expect(out.children!.find((c) => c.id === "p")).toBeUndefined(); + expect(out.edges).toHaveLength(0); + expect( + issues.some( + (i) => + i.level === "dropped" && + i.category === "elk-orphan-edge" && + /\be1\b/.test(i.message), + ), + ).toBe(true); + }); + + it("includes dropped edge ids in the orphan-edge message so distinct losses survive level|message dedupe", () => { + const runOne: ElkInput = { + id: "root", + children: [{ id: "a", width: 1, height: 1 }], + edges: [{ id: "e1", sources: ["a"], targets: ["ghost"] }], + }; + const runTwo: ElkInput = { + id: "root", + children: [{ id: "a", width: 1, height: 1 }], + edges: [{ id: "e2", sources: ["a"], targets: ["ghost"] }], + }; + const msgOne = repairElkInput(runOne).issues.find( + (i) => i.category === "elk-orphan-edge", + )!.message; + const msgTwo = repairElkInput(runTwo).issues.find( + (i) => i.category === "elk-orphan-edge", + )!.message; + expect(msgOne).toContain("e1"); + expect(msgTwo).toContain("e2"); + // Distinct dropped edges must yield distinct messages, otherwise store.ts + // appendLayoutIssues (dedupe by `level|message`) swallows the second loss. + expect(msgOne).not.toEqual(msgTwo); + }); + it("drops children referencing nonexistent parents", () => { const input: ElkInput = { id: "root", diff --git a/understand-anything-plugin/packages/dashboard/src/utils/elk-layout.ts b/understand-anything-plugin/packages/dashboard/src/utils/elk-layout.ts index 8bc9f8628..5e7eff7e8 100644 --- a/understand-anything-plugin/packages/dashboard/src/utils/elk-layout.ts +++ b/understand-anything-plugin/packages/dashboard/src/utils/elk-layout.ts @@ -140,28 +140,7 @@ export function repairElkInput( maybeThrow(strict, issue); } - // 4. dropOrphanEdges - let orphanEdges = 0; - const edges = input.edges.filter((e) => { - const ok = e.sources.every((s) => allIds.has(s)) && - e.targets.every((t) => allIds.has(t)); - if (!ok) { - orphanEdges++; - return false; - } - return true; - }); - if (orphanEdges > 0) { - const issue = makeIssue( - "dropped", - "elk-orphan-edge", - `Dropped ${orphanEdges} edge(s) referencing nonexistent nodes.`, - ); - issues.push(issue); - maybeThrow(strict, issue); - } - - // 5. dropCircularContainment + // 4. dropCircularContainment const parentOf = new Map(); const fillParents = (children: ElkChild[], parent?: string) => { for (const c of children) { @@ -205,6 +184,53 @@ export function repairElkInput( maybeThrow(strict, issue); } + // 5. dropOrphanEdges — this is the single reconciliation point that validates + // edges against the FINAL child tree (childrenD). It therefore drops edges + // pointing at nodes removed by ANY prior pass: original ghost endpoints, the + // orphan-child pass (step 3, which carries dropped parents' whole c.children + // subtrees out), and the containment-cycle pass (step 4). Per-pass drop + // attribution (which pass removed the endpoint) is deliberately NOT tracked + // here; if that traceability is ever needed, the orphan-child/cycle passes + // would each return { children, removedIds } and filter their own edges. + const finalIds = new Set(); + const walkFinal = (children: ElkChild[]) => { + for (const c of children) { + finalIds.add(c.id); + if (c.children) walkFinal(c.children); + } + }; + walkFinal(childrenD); + let orphanEdges = 0; + const droppedEdgeIds: string[] = []; + const edges = input.edges.filter((e) => { + const ok = e.sources.every((s) => finalIds.has(s)) && + e.targets.every((t) => finalIds.has(t)); + if (!ok) { + orphanEdges++; + droppedEdgeIds.push(e.id); + return false; + } + return true; + }); + if (orphanEdges > 0) { + // Include the dropped edge ids so distinct losses across re-layouts produce + // distinct messages — store.ts appendLayoutIssues dedupes by `level|message`, + // so a generic count would swallow a fresh single-edge drop on a later run. + const MAX_LISTED = 20; + const listed = droppedEdgeIds.slice(0, MAX_LISTED).join(", "); + const suffix = + droppedEdgeIds.length > MAX_LISTED + ? `${listed}, …(+${droppedEdgeIds.length - MAX_LISTED} more)` + : listed; + const issue = makeIssue( + "dropped", + "elk-orphan-edge", + `Dropped ${orphanEdges} edge(s) referencing nonexistent nodes: ${suffix}.`, + ); + issues.push(issue); + maybeThrow(strict, issue); + } + return { input: { ...input, children: childrenD, edges }, issues,