Merge pull request 'feat(dashboard): switch graph layout from Dagre to ELK' (#232) from feat/dashboard-elk-layout into main

2026-05-13 08:28:24 +00:00
parent 497f03c747 484ed520cd
commit 55b3b61498
5 changed files with 227 additions and 104 deletions
@@ -15,6 +15,7 @@
  "dependencies": {
    "@dagrejs/dagre": "^3.0.0",
    "@xyflow/react": "^12.10.2",
+    "elkjs": "^0.11.1",
    "react": "^19.2.6",
    "react-dom": "^19.2.6",
    "react-markdown": "^10.1.0",
@@ -2,7 +2,6 @@ import {
  BaseEdge,
  EdgeLabelRenderer,
  type EdgeProps,
-  getBezierPath,
  getSmoothStepPath,
 } from "@xyflow/react";
 import type { ConditionEdgeData } from "./types.ts";
@@ -21,31 +20,28 @@ export function ConditionEdge(props: EdgeProps) {
    data,
    markerEnd,
  } = props;
-  const edgeData = data as ConditionEdgeData | undefined;
+  const edgeData = data as (ConditionEdgeData & { elkLabelX?: number | null; elkLabelY?: number | null }) | undefined;
  const isFallback = edgeData?.isFallback ?? false;
  const isSelfLoop = source === target;

-  const [path, labelX, labelY] = isSelfLoop
-    ? getSmoothStepPath({
-        sourceX,
-        sourceY,
-        targetX,
-        targetY,
-        sourcePosition,
-        targetPosition,
-        borderRadius: 20,
-      })
-    : getBezierPath({
-        sourceX,
-        sourceY,
-        targetX,
-        targetY,
-        sourcePosition,
-        targetPosition,
-      });
+  const [path, defaultLabelX, defaultLabelY] = getSmoothStepPath({
+    sourceX,
+    sourceY,
+    targetX,
+    targetY,
+    sourcePosition,
+    targetPosition,
+    borderRadius: isSelfLoop ? 20 : 8,
+    offset: isSelfLoop ? 50 : undefined,
+  });

-  const stroke = isFallback ? "var(--color-text-muted)" : "var(--color-text)";
+  const stroke = isFallback ? "var(--color-text-muted)" : "var(--color-accent)";
  const strokeDasharray = isFallback ? "5 4" : undefined;
+  const label = edgeData?.condition ?? "";
+
+  // Use ELK-computed label position if available, otherwise fall back to ReactFlow default
+  const labelX = edgeData?.elkLabelX ?? defaultLabelX;
+  const labelY = edgeData?.elkLabelY ?? defaultLabelY;

  return (
    <>
@@ -55,19 +51,21 @@ export function ConditionEdge(props: EdgeProps) {
        markerEnd={markerEnd}
        style={{ stroke, strokeWidth: 1.5, strokeDasharray }}
      />
-      {edgeData && !isFallback && edgeData.condition !== "" && (
+      {label !== "" && (
        <EdgeLabelRenderer>
          <div
            className="absolute px-1.5 py-0.5 rounded text-[10px] font-mono pointer-events-auto"
            style={{
              transform: `translate(-50%, -50%) translate(${labelX}px, ${labelY}px)`,
-              background: "var(--color-bg)",
+              background: "var(--color-surface)",
              border: "1px solid var(--color-border)",
-              color: "var(--color-text)",
+              color: isFallback ? "var(--color-text-muted)" : "var(--color-text)",
+              whiteSpace: "nowrap",
+              zIndex: 10,
            }}
-            title={edgeData.conditionDescription ?? undefined}
+            title={edgeData?.conditionDescription ?? undefined}
          >
-            {edgeData.condition}
+            {label}
          </div>
        </EdgeLabelRenderer>
      )}
@@ -1,6 +1,6 @@
-import Dagre from "@dagrejs/dagre";
 import type { Edge, Node } from "@xyflow/react";
-import { useMemo } from "react";
+import ELK, { type ElkExtendedEdge, type ElkNode } from "elkjs/lib/elk.bundled.js";
+import { useEffect, useState } from "react";
 import type { WorkflowGraphEdge } from "../../api.ts";
 import type { ConditionEdgeData, NodeState, RoleNodeData, TerminalNodeData } from "./types.ts";

@@ -37,6 +37,10 @@ function nodeSize(id: string): { width: number; height: number } {
  return { width: ROLE_NODE_WIDTH, height: ROLE_NODE_HEIGHT };
 }

+function edgeKey(e: WorkflowGraphEdge): string {
+  return `${e.from}->${e.to}::${e.condition}`;
+}
+
 function buildRoleNode(
  id: string,
  pos: { x: number; y: number },
@@ -68,14 +72,24 @@ function buildTerminalNode(
  };
 }

-function edgeKey(e: WorkflowGraphEdge): string {
-  return `${e.from}->${e.to}::${e.condition}`;
-}
-
-function buildEdge(e: WorkflowGraphEdge): Edge<ConditionEdgeData> {
+function buildEdge(e: WorkflowGraphEdge, elkEdgeMap: Map<string, ElkExtendedEdge>): Edge<ConditionEdgeData> {
  const isFallback = e.condition === "FALLBACK";
+  const key = edgeKey(e);
+  const elkEdge = elkEdgeMap.get(key);
+
+  // Extract ELK's computed label position
+  let labelX: number | null = null;
+  let labelY: number | null = null;
+  if (elkEdge?.labels && elkEdge.labels.length > 0) {
+    const label = elkEdge.labels[0];
+    if (label.x !== undefined && label.y !== undefined) {
+      labelX = label.x + (label.width ?? 0) / 2;
+      labelY = label.y + (label.height ?? 0) / 2;
+    }
+  }
+
  return {
-    id: edgeKey(e),
+    id: key,
    source: e.from,
    target: e.to,
    type: "condition",
@@ -83,45 +97,111 @@ function buildEdge(e: WorkflowGraphEdge): Edge<ConditionEdgeData> {
      condition: e.condition,
      conditionDescription: e.conditionDescription,
      isFallback,
-    },
+      elkLabelX: labelX,
+      elkLabelY: labelY,
+    } as ConditionEdgeData,
  };
 }

-export function useLayout(input: LayoutInput): LayoutResult {
-  return useMemo(() => {
-    const ids = collectNodeIds(input.edges);
+const elk = new ELK();

-    const g = new Dagre.graphlib.Graph({ multigraph: true }).setDefaultEdgeLabel(() => ({}));
-    g.setGraph({ rankdir: "TB", nodesep: 60, ranksep: 80 });
+async function computeLayout(input: LayoutInput): Promise<LayoutResult> {
+  const ids = collectNodeIds(input.edges);

-    for (const id of ids) {
-      const size = nodeSize(id);
-      g.setNode(id, { width: size.width, height: size.height });
-    }
-    for (const e of input.edges) {
-      if (e.from === e.to) {
-        continue;
-      }
-      g.setEdge(e.from, e.to, {}, edgeKey(e));
+  const elkNodes: ElkNode[] = [];
+  for (const id of ids) {
+    const size = nodeSize(id);
+    elkNodes.push({ id, width: size.width, height: size.height });
+  }
+
+  const elkEdges: ElkExtendedEdge[] = input.edges
+    .filter((e) => e.from !== e.to)
+    .map((e) => ({
+      id: edgeKey(e),
+      sources: [e.from],
+      targets: [e.to],
+      labels: e.condition !== ""
+        ? [{ text: e.condition, width: Math.max(e.condition.length * 7 + 16, 60), height: 22 }]
+        : [],
+    }));
+
+  const graph: ElkNode = {
+    id: "root",
+    layoutOptions: {
+      "elk.algorithm": "layered",
+      "elk.direction": "DOWN",
+      // Node spacing
+      "elk.spacing.nodeNode": "30",
+      "elk.layered.spacing.nodeNodeBetweenLayers": "50",
+      // Edge spacing — keep edges apart from each other and from nodes
+      "elk.spacing.edgeNode": "25",
+      "elk.spacing.edgeEdge": "15",
+      "elk.layered.spacing.edgeNodeBetweenLayers": "25",
+      "elk.layered.spacing.edgeEdgeBetweenLayers": "15",
+      // Edge routing
+      "elk.edgeRouting": "ORTHOGONAL",
+      "elk.layered.mergeEdges": "false",
+      // Node placement
+      "elk.layered.nodePlacement.strategy": "NETWORK_SIMPLEX",
+      // Edge label placement
+      "elk.edgeLabels.placement": "CENTER",
+      // Crossing minimization
+      "elk.layered.crossingMinimization.strategy": "LAYER_SWEEP",
+      // Compaction
+      "elk.layered.compaction.postCompaction.strategy": "EDGE_LENGTH",
+      // Cycle breaking — keep main flow top-to-bottom
+      "elk.layered.cycleBreaking.strategy": "DEPTH_FIRST",
+    },
+    children: elkNodes,
+    edges: elkEdges,
+  };
+
+  const laid = await elk.layout(graph);
+
+  // Build map of ELK edge results for label positions
+  const elkEdgeMap = new Map<string, ElkExtendedEdge>();
+  for (const e of laid.edges ?? []) {
+    elkEdgeMap.set(e.id, e);
+  }
+
+  const nodes: Node[] = [];
+  for (const child of laid.children ?? []) {
+    const pos = { x: child.x ?? 0, y: child.y ?? 0 };
+    const state = input.nodeStates.get(child.id) ?? "default";
+    if (child.id === START_ID || child.id === END_ID) {
+      nodes.push(buildTerminalNode(child.id, pos, state));
+    } else {
+      nodes.push(buildRoleNode(child.id, pos, input.roles, state));
    }
+  }

-    Dagre.layout(g);
+  const edges: Edge[] = input.edges.map((e) => buildEdge(e, elkEdgeMap));

-    const nodes: Node[] = [];
-    for (const id of ids) {
-      const dagNode = g.node(id);
-      const size = nodeSize(id);
-      const pos = { x: dagNode.x - size.width / 2, y: dagNode.y - size.height / 2 };
-      const state = input.nodeStates.get(id) ?? "default";
-      if (id === START_ID || id === END_ID) {
-        nodes.push(buildTerminalNode(id, pos, state));
-      } else {
-        nodes.push(buildRoleNode(id, pos, input.roles, state));
-      }
-    }
-
-    const edges: Edge[] = input.edges.map(buildEdge);
-
-    return { nodes, edges };
-  }, [input.edges, input.roles, input.nodeStates]);
+  return { nodes, edges };
+}
+
+const EMPTY_LAYOUT: LayoutResult = { nodes: [], edges: [] };
+
+export function useLayout(input: LayoutInput): LayoutResult {
+  const [layout, setLayout] = useState<LayoutResult>(EMPTY_LAYOUT);
+
+  const edgeJson = JSON.stringify(input.edges);
+  const roleJson = JSON.stringify(input.roles);
+
+  useEffect(() => {
+    let cancelled = false;
+    const parsed = {
+      edges: JSON.parse(edgeJson) as readonly WorkflowGraphEdge[],
+      roles: JSON.parse(roleJson) as Record<string, { description: string }>,
+      nodeStates: input.nodeStates,
+    };
+    computeLayout(parsed).then((result) => {
+      if (!cancelled) setLayout(result);
+    });
+    return () => {
+      cancelled = true;
+    };
+  }, [edgeJson, roleJson, input.nodeStates]);
+
+  return layout;
 }
@@ -6,9 +6,11 @@ import {
  type NodeTypes,
  type OnNodeClick,
  ReactFlow,
+  ReactFlowProvider,
+  useReactFlow,
 } from "@xyflow/react";
 import "@xyflow/react/dist/style.css";
-import { useMemo } from "react";
+import { useEffect, useMemo } from "react";
 import type { WorkflowGraph as WorkflowGraphData } from "../../api.ts";
 import { ConditionEdge } from "./condition-edge.tsx";
 import { RoleNode } from "./role-node.tsx";
@@ -37,12 +39,30 @@ function handleRoleNodeClick(onRoleClick: (roleName: string) => void, node: Node
  onRoleClick(node.id);
 }

-export function WorkflowGraph({ graph, roles, nodeStates, onNodeClick }: Props) {
+function WorkflowGraphInner({ graph, roles, nodeStates, onNodeClick }: Props) {
  const layout = useLayout({ edges: graph.edges, roles, nodeStates });
+  const { fitView } = useReactFlow();

  const onNodeClickHandler: OnNodeClick | undefined =
    onNodeClick !== null ? (_e, node) => handleRoleNodeClick(onNodeClick, node) : undefined;

+  // Re-fit when layout changes (ELK is async)
+  // Use requestAnimationFrame + setTimeout to ensure ReactFlow has processed nodes
+  useEffect(() => {
+    if (layout.nodes.length > 0) {
+      let cancelled = false;
+      requestAnimationFrame(() => {
+        if (cancelled) return;
+        setTimeout(() => {
+          if (!cancelled) fitView({ padding: 0.1, duration: 300 });
+        }, 300);
+      });
+      return () => {
+        cancelled = true;
+      };
+    }
+  }, [layout.nodes, layout.edges, fitView]);
+
  const styledEdges = useMemo(
    () =>
      layout.edges.map((e) => ({
@@ -57,15 +77,25 @@ export function WorkflowGraph({ graph, roles, nodeStates, onNodeClick }: Props)
    [layout.edges],
  );

+  // Generate a stable key that changes when layout changes, to force ReactFlow remount + fitView
+  const layoutKey = useMemo(
+    () => layout.nodes.map((n) => `${n.id}:${n.position.x}:${n.position.y}`).join(","),
+    [layout.nodes],
+  );
+
  return (
    <ReactFlow
+      key={layoutKey}
      nodes={layout.nodes}
      edges={styledEdges}
      nodeTypes={nodeTypes}
      edgeTypes={edgeTypes}
      onNodeClick={onNodeClickHandler}
      fitView
-      fitViewOptions={{ padding: 0.15 }}
+      fitViewOptions={{ padding: 0.1, minZoom: 0.1, maxZoom: 1.5 }}
+      minZoom={0.1}
+      maxZoom={1.5}
+      defaultViewport={{ x: 0, y: 0, zoom: 0.5 }}
      nodesDraggable={false}
      nodesConnectable={false}
      elementsSelectable={false}
@@ -77,3 +107,11 @@ export function WorkflowGraph({ graph, roles, nodeStates, onNodeClick }: Props)
    </ReactFlow>
  );
 }
+
+export function WorkflowGraph(props: Props) {
+  return (
+    <ReactFlowProvider>
+      <WorkflowGraphInner {...props} />
+    </ReactFlowProvider>
+  );
+}
@@ -45,38 +45,45 @@ function ExpandedWorkflowBody({
  const edgeCount = descriptor !== null ? descriptor.graph.edges.length : 0;
  const vc = versionCount(detail);

+  const hasGraph = descriptor !== null && edgeCount > 0;
+
  return (
-    <div className="pt-3 space-y-3 border-t" style={{ borderColor: "var(--color-border)" }}>
-      <div>
-        <p className="text-sm font-medium" style={{ color: "var(--color-text)" }}>
-          {detail.name}
+    <div
+      className="pt-3 border-t flex gap-4"
+      style={{ borderColor: "var(--color-border)" }}
+    >
+      <div className="space-y-3 shrink-0" style={{ minWidth: 200, maxWidth: 280 }}>
+        <div>
+          <p className="text-sm font-medium" style={{ color: "var(--color-text)" }}>
+            {detail.name}
+          </p>
+          <p className="text-xs mt-1 mb-1" style={{ color: "var(--color-text-muted)" }}>
+            Hash
+          </p>
+          <code className="text-xs font-mono block" style={{ color: "var(--color-accent)" }}>
+            {detail.hash}
+          </code>
+        </div>
+        <p className="text-xs" style={{ color: "var(--color-text-muted)" }}>
+          {vc} version{vc !== 1 ? "s" : ""}
        </p>
-        <p className="text-xs mt-1 mb-1" style={{ color: "var(--color-text-muted)" }}>
-          Hash
-        </p>
-        <code className="text-xs font-mono block" style={{ color: "var(--color-accent)" }}>
-          {detail.hash}
-        </code>
+        <div>
+          <p className="text-xs mb-1 font-medium" style={{ color: "var(--color-text-muted)" }}>
+            Description
+          </p>
+          <p className="text-sm whitespace-pre-wrap" style={{ color: "var(--color-text)" }}>
+            {descriptor !== null && descriptor.description !== ""
+              ? descriptor.description
+              : descriptor !== null
+                ? "—"
+                : "No descriptor available for this workflow version."}
+          </p>
+        </div>
      </div>
-      <p className="text-xs" style={{ color: "var(--color-text-muted)" }}>
-        {vc} version{vc !== 1 ? "s" : ""}
-      </p>
-      <div>
-        <p className="text-xs mb-1 font-medium" style={{ color: "var(--color-text-muted)" }}>
-          Description
-        </p>
-        <p className="text-sm whitespace-pre-wrap" style={{ color: "var(--color-text)" }}>
-          {descriptor !== null && descriptor.description !== ""
-            ? descriptor.description
-            : descriptor !== null
-              ? "—"
-              : "No descriptor available for this workflow version."}
-        </p>
-      </div>
-      {descriptor !== null && edgeCount > 0 ? (
+      {hasGraph ? (
        <div
-          className="rounded-lg border overflow-hidden"
-          style={{ borderColor: "var(--color-border)", background: "var(--color-bg)" }}
+          className="rounded-lg border overflow-hidden flex-1"
+          style={{ borderColor: "var(--color-border)", background: "var(--color-bg)", minHeight: 500 }}
        >
          <div
            className="px-3 py-2 text-xs flex justify-between items-center"
@@ -87,7 +94,7 @@ function ExpandedWorkflowBody({
              {edgeCount} edge{edgeCount === 1 ? "" : "s"}
            </span>
          </div>
-          <div style={{ height: 300, width: "100%" }}>
+          <div style={{ height: 600, width: "100%" }}>
            <WorkflowGraph
              graph={descriptor.graph}
              roles={descriptor.roles}
@@ -148,18 +155,17 @@ export function WorkflowList({ agent }: Props) {
  );

  function toggleExpanded(name: string) {
-    let shouldLoad = false;
+    const wasExpanded = expanded.has(name);
    setExpanded((prev) => {
      const next = new Set(prev);
      if (next.has(name)) {
        next.delete(name);
-        return next;
+      } else {
+        next.add(name);
      }
-      next.add(name);
-      shouldLoad = true;
      return next;
    });
-    if (shouldLoad) {
+    if (!wasExpanded) {
      ensureDetailLoaded(name);
    }
  }