Fix auto-layout spacer nodes for loop body positioning

- Add virtual spacer nodes to reserve vertical space for loop bodies
- Route loop "done" edges through spacers to push downstream nodes below
- Use longest-path ranker for more hierarchical layouts
- Calculate loop body depth to size spacers appropriately

webui/src/pages/WorkflowEditorPage.tsx

@@ -1187,19 +1187,17 @@ function WorkflowEditorInner() {
   }, [setNodes, setEdges])
 
   // Auto-layout nodes using dagre for clean hierarchical layout
-  // Special handling for loop nodes: body nodes positioned below their parent loop
+  // Handles loop nodes specially: body nodes positioned below their parent loop
   const autoLayout = useCallback(() => {
     if (nodes.length === 0) return
 
     const GRID_SIZE = 20
-    const NODE_WIDTH = 200
+    const NODE_WIDTH = 220
     const NODE_HEIGHT = 80
-    const HORIZONTAL_SPACING = 80
-    const VERTICAL_SPACING = 100
-    const LOOP_BODY_OFFSET_Y = 60
-    const LOOP_BODY_INDENT_X = 40
+    const HORIZONTAL_SPACING = 60
+    const VERTICAL_SPACING = 120
+    const LOOP_BODY_INDENT_X = 60
 
-    // Helper to snap to grid
     const snapToGrid = (value: number) => Math.round(value / GRID_SIZE) * GRID_SIZE
 
     // Build edge maps
@@ -1254,7 +1252,64 @@ function WorkflowEditorInner() {
       })
     })
 
-    // Create dagre graph for main flow (excluding loop body nodes)
+    // Calculate depth of each loop body (for reserving vertical space)
+    const loopBodyDepths = new Map<string, number>()
+    loopNodes.forEach(loopId => {
+      const bodyNodes = loopBodyNodes.get(loopId)!
+      if (bodyNodes.size === 0) {
+        loopBodyDepths.set(loopId, 0)
+        return
+      }
+
+      // Calculate max depth within body using BFS
+      const bodyIncoming = new Map<string, string[]>()
+      bodyNodes.forEach(nodeId => bodyIncoming.set(nodeId, []))
+      edges.forEach(e => {
+        if (bodyNodes.has(e.source) && bodyNodes.has(e.target)) {
+          bodyIncoming.get(e.target)?.push(e.source)
+        }
+      })
+
+      // Find entry points (nodes with no incoming from other body nodes, or first from loop)
+      const entryNodes: string[] = []
+      const loopOutEdges = outgoingEdges.get(loopId) || []
+      loopOutEdges.forEach(e => {
+        if (e.handle === 'loop' && bodyNodes.has(e.target)) {
+          entryNodes.push(e.target)
+        }
+      })
+      if (entryNodes.length === 0) {
+        bodyNodes.forEach(nodeId => {
+          if ((bodyIncoming.get(nodeId) || []).length === 0) {
+            entryNodes.push(nodeId)
+          }
+        })
+      }
+
+      // BFS to find max depth
+      const depths = new Map<string, number>()
+      const queue = entryNodes.map(id => ({ id, depth: 0 }))
+      let maxDepth = 0
+
+      while (queue.length > 0) {
+        const { id, depth } = queue.shift()!
+        if (depths.has(id) && depths.get(id)! >= depth) continue
+        depths.set(id, depth)
+        maxDepth = Math.max(maxDepth, depth)
+
+        const outEdges = outgoingEdges.get(id) || []
+        outEdges.forEach(edge => {
+          if (bodyNodes.has(edge.target)) {
+            queue.push({ id: edge.target, depth: depth + 1 })
+          }
+        })
+      }
+
+      loopBodyDepths.set(loopId, maxDepth + 1)
+    })
+
+    // Create main flow graph with dagre (excluding loop body nodes)
+    // But add virtual "spacer" nodes for loop bodies
     const g = new dagre.graphlib.Graph()
     g.setGraph({
       rankdir: 'TB',
@@ -1262,126 +1317,61 @@ function WorkflowEditorInner() {
       ranksep: VERTICAL_SPACING,
       marginx: 50,
       marginy: 50,
+      ranker: 'longest-path',
     })
     g.setDefaultEdgeLabel(() => ({}))
 
-    // Add main flow nodes (not loop body nodes)
+    // Add main flow nodes
     nodes.forEach(n => {
       if (!nodeToLoop.has(n.id)) {
         g.setNode(n.id, { width: NODE_WIDTH, height: NODE_HEIGHT })
       }
     })
 
-    // Add main flow edges (skip edges to/from loop body nodes, and skip loop->body edges)
-    edges.forEach(e => {
-      const sourceInBody = nodeToLoop.has(e.source)
-      const targetInBody = nodeToLoop.has(e.target)
-      const isLoopBodyEdge = loopNodes.has(e.source) && e.sourceHandle === 'loop'
-
-      if (!sourceInBody && !targetInBody && !isLoopBodyEdge) {
-        g.setEdge(e.source, e.target)
-      }
-    })
-
-    // Run dagre layout
-    dagre.layout(g)
-
-    // Calculate loop body heights to offset 'done' targets
-    const loopBodyHeights = new Map<string, number>()
+    // For each loop with body nodes, add a virtual spacer node to reserve vertical space
     loopNodes.forEach(loopId => {
-      const bodyNodes = loopBodyNodes.get(loopId)!
-      if (bodyNodes.size === 0) {
-        loopBodyHeights.set(loopId, 0)
-        return
+      const bodyDepth = loopBodyDepths.get(loopId) || 0
+      if (bodyDepth > 0) {
+        const spacerHeight = bodyDepth * (NODE_HEIGHT + VERTICAL_SPACING)
+        g.setNode(loopId + '_spacer', { width: 1, height: spacerHeight })
       }
+    })
 
-      // Create a sub-graph for the loop body
-      const bodyGraph = new dagre.graphlib.Graph()
-      bodyGraph.setGraph({
-        rankdir: 'TB',
-        nodesep: HORIZONTAL_SPACING * 0.8,
-        ranksep: VERTICAL_SPACING * 0.8,
-      })
-      bodyGraph.setDefaultEdgeLabel(() => ({}))
-
-      bodyNodes.forEach(nodeId => {
-        bodyGraph.setNode(nodeId, { width: NODE_WIDTH, height: NODE_HEIGHT })
-      })
-
-      // Add internal edges within the loop body
-      edges.forEach(e => {
-        if (bodyNodes.has(e.source) && bodyNodes.has(e.target)) {
-          bodyGraph.setEdge(e.source, e.target)
-        }
-        // Also add edge from loop node to first body node
-        if (e.source === loopId && e.sourceHandle === 'loop' && bodyNodes.has(e.target)) {
-          bodyGraph.setNode(loopId + '_entry', { width: 0, height: 0 })
-          bodyGraph.setEdge(loopId + '_entry', e.target)
-        }
-      })
+    // Add edges - for loop nodes, route through spacer
+    edges.forEach(e => {
+      const sourceInBody = nodeToLoop.has(e.source)
+      const targetInBody = nodeToLoop.has(e.target)
 
-      dagre.layout(bodyGraph)
+      if (sourceInBody || targetInBody) return
 
-      // Find the bounding box of the body
-      let maxY = 0
-      bodyNodes.forEach(nodeId => {
-        const nodePos = bodyGraph.node(nodeId)
-        if (nodePos) {
-          maxY = Math.max(maxY, nodePos.y + NODE_HEIGHT / 2)
+      // For loop "done" edges, route through the spacer
+      if (loopNodes.has(e.source) && e.sourceHandle === 'done') {
+        const spacerId = e.source + '_spacer'
+        if (g.hasNode(spacerId)) {
+          g.setEdge(e.source, spacerId)
+          g.setEdge(spacerId, e.target)
+          return
         }
-      })
-      loopBodyHeights.set(loopId, maxY + LOOP_BODY_OFFSET_Y)
-    })
-
-    // Get positions from dagre and apply adjustments
-    const newPositions = new Map<string, { x: number; y: number }>()
-
-    // Calculate vertical offsets for nodes that follow loop 'done' outputs
-    // We need to push them down to account for loop body height
-    const nodeYOffset = new Map<string, number>()
+      }
 
-    // BFS to propagate offsets from loops
-    const visited = new Set<string>()
-    const queue: string[] = []
+      // Skip loop->body edges
+      if (loopNodes.has(e.source) && e.sourceHandle === 'loop') return
 
-    // Start from all loop nodes
-    loopNodes.forEach(loopId => {
-      const bodyHeight = loopBodyHeights.get(loopId) || 0
-      const outEdges = outgoingEdges.get(loopId) || []
-      outEdges.forEach(edge => {
-        if (edge.handle === 'done' && !nodeToLoop.has(edge.target)) {
-          nodeYOffset.set(edge.target, (nodeYOffset.get(edge.target) || 0) + bodyHeight)
-          queue.push(edge.target)
-        }
-      })
+      g.setEdge(e.source, e.target)
     })
 
-    // Propagate offsets downstream
-    while (queue.length > 0) {
-      const nodeId = queue.shift()!
-      if (visited.has(nodeId)) continue
-      visited.add(nodeId)
+    dagre.layout(g)
 
-      const offset = nodeYOffset.get(nodeId) || 0
-      const outEdges = outgoingEdges.get(nodeId) || []
-      outEdges.forEach(edge => {
-        if (!nodeToLoop.has(edge.target)) {
-          const existingOffset = nodeYOffset.get(edge.target) || 0
-          nodeYOffset.set(edge.target, Math.max(existingOffset, offset))
-          queue.push(edge.target)
-        }
-      })
-    }
+    const newPositions = new Map<string, { x: number; y: number }>()
 
-    // Apply positions for main flow nodes
+    // Position main flow nodes from dagre
     nodes.forEach(n => {
       if (!nodeToLoop.has(n.id)) {
         const dagreNode = g.node(n.id)
         if (dagreNode) {
-          const yOffset = nodeYOffset.get(n.id) || 0
           newPositions.set(n.id, {
             x: snapToGrid(dagreNode.x - NODE_WIDTH / 2),
-            y: snapToGrid(dagreNode.y - NODE_HEIGHT / 2 + yOffset),
+            y: snapToGrid(dagreNode.y - NODE_HEIGHT / 2),
           })
         }
       }
@@ -1395,12 +1385,14 @@ function WorkflowEditorInner() {
       const bodyNodeIds = Array.from(loopBodyNodes.get(loopId) || [])
       if (bodyNodeIds.length === 0) return
 
-      // Create and layout sub-graph for body
+      // Create sub-graph for body layout
       const bodyGraph = new dagre.graphlib.Graph()
       bodyGraph.setGraph({
         rankdir: 'TB',
-        nodesep: HORIZONTAL_SPACING * 0.8,
+        nodesep: HORIZONTAL_SPACING * 0.7,
         ranksep: VERTICAL_SPACING * 0.8,
+        marginx: 0,
+        marginy: 0,
       })
       bodyGraph.setDefaultEdgeLabel(() => ({}))
 
@@ -1409,36 +1401,33 @@ function WorkflowEditorInner() {
       })
 
       edges.forEach(e => {
-        const sourceInBody = loopBodyNodes.get(loopId)!.has(e.source)
-        const targetInBody = loopBodyNodes.get(loopId)!.has(e.target)
-        if (sourceInBody && targetInBody) {
+        if (loopBodyNodes.get(loopId)!.has(e.source) && loopBodyNodes.get(loopId)!.has(e.target)) {
           bodyGraph.setEdge(e.source, e.target)
         }
       })
 
       dagre.layout(bodyGraph)
 
-      // Find bounds to center the body below the loop
-      let minX = Infinity, maxX = -Infinity
+      // Find bounding box
+      let minX = Infinity, minY = Infinity
       bodyNodeIds.forEach(nodeId => {
         const pos = bodyGraph.node(nodeId)
         if (pos) {
           minX = Math.min(minX, pos.x - NODE_WIDTH / 2)
-          maxX = Math.max(maxX, pos.x + NODE_WIDTH / 2)
+          minY = Math.min(minY, pos.y - NODE_HEIGHT / 2)
         }
       })
 
-      const bodyWidth = maxX - minX
-      const bodyCenterX = loopPos.x + NODE_WIDTH / 2 + LOOP_BODY_INDENT_X
-      const bodyStartX = bodyCenterX - bodyWidth / 2
-      const bodyStartY = loopPos.y + NODE_HEIGHT + LOOP_BODY_OFFSET_Y
+      // Position body nodes relative to loop, indented to the right
+      const bodyStartX = loopPos.x + LOOP_BODY_INDENT_X
+      const bodyStartY = loopPos.y + NODE_HEIGHT + VERTICAL_SPACING * 0.6
 
       bodyNodeIds.forEach(nodeId => {
         const pos = bodyGraph.node(nodeId)
         if (pos) {
           newPositions.set(nodeId, {
-            x: snapToGrid(bodyStartX + (pos.x - minX)),
-            y: snapToGrid(bodyStartY + pos.y - NODE_HEIGHT / 2),
+            x: snapToGrid(bodyStartX + (pos.x - NODE_WIDTH / 2 - minX)),
+            y: snapToGrid(bodyStartY + (pos.y - NODE_HEIGHT / 2 - minY)),
           })
         }
       })