From ff4ac3c2657b3cc7f62a083c642af1afd42355d3 Mon Sep 17 00:00:00 2001 From: Lum1104 Date: Sun, 3 May 2026 16:19:50 +0800 Subject: [PATCH] chore(dashboard): address Task 9 + 10 review concerns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cross-cutting fixes for the dagre→ELK migration pattern: - nodesToElkInput accepts an optional layoutOptionsOverride so views can override defaults (e.g. direction) without forking the helper. - DomainGraphView restores its original LR layout by passing { "elk.direction": "RIGHT" } — the previous commit silently shifted it to TB because the helper hardcoded DOWN. - Both overview and DomainGraphView now .catch the ELK promise so strict-mode (DEV) failures don't surface as unhandled rejections. - Both also console.warn returned issues until Task 16 wires the WarningBanner funnel; auto-corrected/dropped issues no longer silently disappear in production. - Minor: DRY the duplicate `as unknown as Node[]` cast in GraphView. Ranksep label-aware spacing in DomainGraphView is intentionally deferred — see Task 14 polish or follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/components/DomainGraphView.tsx | 20 +++++++++---- .../dashboard/src/components/GraphView.tsx | 28 ++++++++++--------- .../packages/dashboard/src/utils/layout.ts | 3 +- 3 files changed, 32 insertions(+), 19 deletions(-) diff --git a/understand-anything-plugin/packages/dashboard/src/components/DomainGraphView.tsx b/understand-anything-plugin/packages/dashboard/src/components/DomainGraphView.tsx index c7155cb..acb4f3e 100644 --- a/understand-anything-plugin/packages/dashboard/src/components/DomainGraphView.tsx +++ b/understand-anything-plugin/packages/dashboard/src/components/DomainGraphView.tsx @@ -191,16 +191,26 @@ function DomainGraphViewInner() { } let cancelled = false; const { nodes: nodesArray, edges: edgesArray, dims } = built; - const elkInput = nodesToElkInput(nodesArray, edgesArray, dims); - applyElkLayout(elkInput, { strict: import.meta.env.DEV }).then( - ({ positioned }) => { + // DomainGraphView used dagre LR; preserve that direction with ELK. + const elkInput = nodesToElkInput(nodesArray, edgesArray, dims, { + "elk.direction": "RIGHT", + }); + applyElkLayout(elkInput, { strict: import.meta.env.DEV }) + .then(({ positioned, issues }) => { if (cancelled) return; + if (issues.length > 0) { + // TODO: Task 16 funnels these into the WarningBanner. + console.warn("[domain ELK] layout issues:", issues); + } setLayout({ nodes: mergeElkPositions(nodesArray, positioned), edges: edgesArray, }); - }, - ); + }) + .catch((err) => { + if (cancelled) return; + console.error("[domain ELK] layout failed:", err); + }); return () => { cancelled = true; }; diff --git a/understand-anything-plugin/packages/dashboard/src/components/GraphView.tsx b/understand-anything-plugin/packages/dashboard/src/components/GraphView.tsx index 55c5635..67c1d6b 100644 --- a/understand-anything-plugin/packages/dashboard/src/components/GraphView.tsx +++ b/understand-anything-plugin/packages/dashboard/src/components/GraphView.tsx @@ -224,21 +224,23 @@ function useOverviewGraph() { } let cancelled = false; const { clusterNodes, flowEdges, dims } = built; - const elkInput = nodesToElkInput( - clusterNodes as unknown as Node[], - flowEdges, - dims, - ); - applyElkLayout(elkInput, { strict: import.meta.env.DEV }).then( - ({ positioned }) => { + const baseNodes = clusterNodes as unknown as Node[]; + const elkInput = nodesToElkInput(baseNodes, flowEdges, dims); + applyElkLayout(elkInput, { strict: import.meta.env.DEV }) + .then(({ positioned, issues }) => { if (cancelled) return; - const positionedNodes = mergeElkPositions( - clusterNodes as unknown as Node[], - positioned, - ); + if (issues.length > 0) { + // TODO: Task 16 wires these into the WarningBanner. Until then, + // surface them in the console so they aren't completely silent. + console.warn("[overview ELK] layout issues:", issues); + } + const positionedNodes = mergeElkPositions(baseNodes, positioned); setOverview({ nodes: positionedNodes, edges: flowEdges }); - }, - ); + }) + .catch((err) => { + if (cancelled) return; + console.error("[overview ELK] layout failed:", err); + }); return () => { cancelled = true; }; diff --git a/understand-anything-plugin/packages/dashboard/src/utils/layout.ts b/understand-anything-plugin/packages/dashboard/src/utils/layout.ts index 0b13ac0..6e408e5 100644 --- a/understand-anything-plugin/packages/dashboard/src/utils/layout.ts +++ b/understand-anything-plugin/packages/dashboard/src/utils/layout.ts @@ -202,10 +202,11 @@ export function nodesToElkInput( nodes: Node[], edges: Edge[], dims: Map, + layoutOptionsOverride?: Record, ): ElkInput { return { id: "root", - layoutOptions: ELK_DEFAULT_LAYOUT_OPTIONS, + layoutOptions: { ...ELK_DEFAULT_LAYOUT_OPTIONS, ...layoutOptionsOverride }, children: nodes.map((n) => { const d = dims.get(n.id); return {