Improve workflow auto-layout with dagre spacer nodes

Rewrite auto-layout function to properly handle loops:
- Use dagre library for directed graph layout
- Add virtual spacer nodes to reserve vertical space for loop bodies
- Position loop body nodes below their parent loop with indentation
- Create separate mini-graphs for loop body internal layout
- Handle nested loops and exit edges properly

webui/src/pages/WorkflowEditorPage.tsx

@@ -1186,36 +1186,28 @@ function WorkflowEditorInner() {
     setShowDeleteConfirm(null)
   }, [setNodes, setEdges])
 
-  // Auto-layout nodes using dagre for clean hierarchical layout
-  // Handles loop nodes specially: body nodes positioned below their parent loop
+  // Auto-layout nodes using dagre for clean top-to-bottom flow
   const autoLayout = useCallback(() => {
     if (nodes.length === 0) return
 
     const GRID_SIZE = 20
     const NODE_WIDTH = 220
     const NODE_HEIGHT = 80
-    const HORIZONTAL_SPACING = 60
-    const VERTICAL_SPACING = 120
-    const LOOP_BODY_INDENT_X = 60
+    const HORIZONTAL_SPACING = 80
+    const VERTICAL_SPACING = 100
 
     const snapToGrid = (value: number) => Math.round(value / GRID_SIZE) * GRID_SIZE
 
-    // Build edge maps
+    // Build edge maps for loop detection
     const outgoingEdges = new Map<string, Array<{ target: string; handle: string }>>()
-    const incomingEdges = new Map<string, Array<{ source: string; handle: string }>>()
-    nodes.forEach(n => {
-      outgoingEdges.set(n.id, [])
-      incomingEdges.set(n.id, [])
-    })
+    nodes.forEach(n => outgoingEdges.set(n.id, []))
     edges.forEach(e => {
       outgoingEdges.get(e.source)?.push({ target: e.target, handle: e.sourceHandle || 'main' })
-      incomingEdges.get(e.target)?.push({ source: e.source, handle: e.sourceHandle || 'main' })
     })
 
     // Identify loop nodes and their body nodes
     const loopNodes = new Set<string>()
     const loopBodyNodes = new Map<string, Set<string>>()
-    const nodeToLoop = new Map<string, string>()
 
     nodes.forEach(n => {
       if (n.data.type === 'loop') {
@@ -1224,7 +1216,7 @@ function WorkflowEditorInner() {
       }
     })
 
-    // Find body nodes for each loop (connected via 'loop' output handle)
+    // Find body nodes for each loop (nodes connected via "loop" output, not "done")
     const findLoopBodyNodes = (loopId: string, nodeId: string, visited: Set<string>) => {
       if (visited.has(nodeId)) return
       visited.add(nodeId)
@@ -1232,12 +1224,13 @@ function WorkflowEditorInner() {
       const bodySet = loopBodyNodes.get(loopId)!
       if (nodeId !== loopId) {
         bodySet.add(nodeId)
-        nodeToLoop.set(nodeId, loopId)
       }
 
       const outEdges = outgoingEdges.get(nodeId) || []
       for (const edge of outEdges) {
+        // Don't follow "done" edges from the loop itself
         if (nodeId === loopId && edge.handle === 'done') continue
+        // Don't include other loops as body nodes
         if (loopNodes.has(edge.target) && edge.target !== loopId) continue
         findLoopBodyNodes(loopId, edge.target, visited)
       }
@@ -1247,231 +1240,215 @@ function WorkflowEditorInner() {
       const outEdges = outgoingEdges.get(loopId) || []
       const loopEdges = outEdges.filter(e => e.handle === 'loop')
       const visited = new Set<string>()
-      loopEdges.forEach(edge => {
-        findLoopBodyNodes(loopId, edge.target, visited)
-      })
+      loopEdges.forEach(edge => findLoopBodyNodes(loopId, edge.target, visited))
     })
 
-    // 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)
-        }
-      })
+    // Calculate depth of loop body (how many nodes deep)
+    const getLoopBodyDepth = (loopId: string): number => {
+      const bodyNodes = loopBodyNodes.get(loopId)
+      if (!bodyNodes || bodyNodes.size === 0) return 1
 
-      // 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
+      const visited = new Set<string>()
 
-      while (queue.length > 0) {
-        const { id, depth } = queue.shift()!
-        if (depths.has(id) && depths.get(id)! >= depth) continue
-        depths.set(id, depth)
+      const dfs = (nodeId: string, depth: number) => {
+        if (visited.has(nodeId)) return
+        visited.add(nodeId)
         maxDepth = Math.max(maxDepth, depth)
 
-        const outEdges = outgoingEdges.get(id) || []
-        outEdges.forEach(edge => {
+        const outEdges = outgoingEdges.get(nodeId) || []
+        for (const edge of outEdges) {
           if (bodyNodes.has(edge.target)) {
-            queue.push({ id: edge.target, depth: depth + 1 })
+            dfs(edge.target, depth + 1)
           }
-        })
+        }
       }
 
-      loopBodyDepths.set(loopId, maxDepth + 1)
-    })
+      // Start from loop's direct children
+      const loopOutEdges = outgoingEdges.get(loopId) || []
+      loopOutEdges.filter(e => e.handle === 'loop').forEach(e => {
+        dfs(e.target, 1)
+      })
+
+      return Math.max(maxDepth, 1)
+    }
 
-    // Create main flow graph with dagre (excluding loop body nodes)
-    // But add virtual "spacer" nodes for loop bodies
+    // Create dagre graph
     const g = new dagre.graphlib.Graph()
     g.setGraph({
       rankdir: 'TB',
       nodesep: HORIZONTAL_SPACING,
       ranksep: VERTICAL_SPACING,
-      marginx: 50,
-      marginy: 50,
       ranker: 'longest-path',
     })
     g.setDefaultEdgeLabel(() => ({}))
 
-    // Add main flow nodes
+    // Add all real nodes
     nodes.forEach(n => {
-      if (!nodeToLoop.has(n.id)) {
-        g.setNode(n.id, { width: NODE_WIDTH, height: NODE_HEIGHT })
-      }
+      g.setNode(n.id, { width: NODE_WIDTH, height: NODE_HEIGHT })
     })
 
-    // For each loop with body nodes, add a virtual spacer node to reserve vertical space
+    // Add virtual spacer nodes for loop bodies to reserve vertical space
+    const spacerNodes = new Map<string, string>()
     loopNodes.forEach(loopId => {
-      const bodyDepth = loopBodyDepths.get(loopId) || 0
-      if (bodyDepth > 0) {
-        const spacerHeight = bodyDepth * (NODE_HEIGHT + VERTICAL_SPACING)
-        g.setNode(loopId + '_spacer', { width: 1, height: spacerHeight })
-      }
+      const spacerId = `spacer_${loopId}`
+      spacerNodes.set(loopId, spacerId)
+      const depth = getLoopBodyDepth(loopId)
+      // Create spacer with height based on loop body depth
+      g.setNode(spacerId, {
+        width: NODE_WIDTH,
+        height: NODE_HEIGHT * depth + VERTICAL_SPACING * (depth - 1)
+      })
     })
 
-    // Track edges that exit from loop bodies to main flow nodes
-    // These need to be represented as loop -> target edges to maintain ordering
-    const loopExitEdges = new Map<string, Set<string>>()
-    loopNodes.forEach(loopId => loopExitEdges.set(loopId, new Set()))
-
-    edges.forEach(e => {
-      const sourceLoop = nodeToLoop.get(e.source)
-      const targetInBody = nodeToLoop.has(e.target)
+    // Find nodes that exit from loop body to main flow (connected to nodes outside the loop body)
+    const loopExitTargets = new Map<string, Set<string>>()
+    loopNodes.forEach(loopId => {
+      loopExitTargets.set(loopId, new Set())
+      const bodyNodes = loopBodyNodes.get(loopId)
+      if (!bodyNodes) return
 
-      // If source is in a loop body and target is NOT in any body and not a loop, track as exit edge
-      if (sourceLoop && !targetInBody && !loopNodes.has(e.target)) {
-        loopExitEdges.get(sourceLoop)!.add(e.target)
-      }
-      // If source is in a loop body and target is another loop (not the same loop), track as exit edge
-      if (sourceLoop && loopNodes.has(e.target) && e.target !== sourceLoop) {
-        loopExitEdges.get(sourceLoop)!.add(e.target)
-      }
+      bodyNodes.forEach(bodyNodeId => {
+        const outEdges = outgoingEdges.get(bodyNodeId) || []
+        outEdges.forEach(edge => {
+          // If edge target is not a body node and not the loop itself, it's an exit edge
+          if (!bodyNodes.has(edge.target) && edge.target !== loopId) {
+            loopExitTargets.get(loopId)?.add(edge.target)
+          }
+        })
+      })
     })
 
-    // Add edges - for loop nodes, route through spacer
+    // Add edges to dagre, routing loop flows through spacers
     edges.forEach(e => {
-      const sourceInBody = nodeToLoop.has(e.source)
-      const targetInBody = nodeToLoop.has(e.target)
-
-      // Skip edges where source or target is a body node (handled separately)
-      if (sourceInBody || targetInBody) return
+      const sourceNode = nodes.find(n => n.id === e.source)
+      const sourceHandle = e.sourceHandle || 'main'
 
-      // 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)
+      if (sourceNode?.data.type === 'loop') {
+        const spacerId = spacerNodes.get(e.source)!
+        if (sourceHandle === 'loop') {
+          // Loop body connection: loop -> spacer (don't add to dagre, handled separately)
+          return
+        } else if (sourceHandle === 'done') {
+          // Done connection: goes from spacer to target
           g.setEdge(spacerId, e.target)
           return
         }
       }
 
-      // Skip loop->body edges
-      if (loopNodes.has(e.source) && e.sourceHandle === 'loop') return
+      // Check if this edge is from a loop body node to something outside
+      let isExitEdge = false
+      loopNodes.forEach(loopId => {
+        const bodyNodes = loopBodyNodes.get(loopId)
+        if (bodyNodes?.has(e.source) && !bodyNodes.has(e.target) && e.target !== loopId) {
+          isExitEdge = true
+          // Route through spacer to ensure proper ordering
+          const spacerId = spacerNodes.get(loopId)!
+          g.setEdge(spacerId, e.target)
+        }
+      })
 
-      g.setEdge(e.source, e.target)
-    })
+      if (!isExitEdge) {
+        // Regular edge - only add if not loop body internal
+        let isLoopBodyInternal = false
+        loopNodes.forEach(loopId => {
+          const bodyNodes = loopBodyNodes.get(loopId)
+          if (bodyNodes?.has(e.source) && (bodyNodes.has(e.target) || e.target === loopId)) {
+            isLoopBodyInternal = true
+          }
+        })
 
-    // Connect each loop to its spacer (if it has one)
-    // This ensures the spacer is positioned directly below the loop
-    loopNodes.forEach(loopId => {
-      const spacerId = loopId + '_spacer'
-      if (g.hasNode(spacerId)) {
-        g.setEdge(loopId, spacerId)
+        if (!isLoopBodyInternal) {
+          g.setEdge(e.source, e.target)
+        }
       }
     })
 
-    // Add edges from loops (via spacers) to nodes that their body exits to
-    // This ensures proper ordering when body nodes connect to main flow nodes
+    // Connect loops to their spacers
     loopNodes.forEach(loopId => {
-      const exitTargets = loopExitEdges.get(loopId)!
-      const spacerId = loopId + '_spacer'
-      exitTargets.forEach(targetId => {
-        // Route through spacer if it exists, to place target below the loop body
-        if (g.hasNode(spacerId)) {
-          g.setEdge(spacerId, targetId)
-        } else {
-          g.setEdge(loopId, targetId)
-        }
-      })
+      const spacerId = spacerNodes.get(loopId)!
+      g.setEdge(loopId, spacerId)
     })
 
+    // Run dagre layout
     dagre.layout(g)
 
+    // Extract positions
     const newPositions = new Map<string, { x: number; y: number }>()
 
-    // Position main flow nodes from dagre
+    // Position real nodes
     nodes.forEach(n => {
-      if (!nodeToLoop.has(n.id)) {
-        const dagreNode = g.node(n.id)
-        if (dagreNode) {
-          newPositions.set(n.id, {
-            x: snapToGrid(dagreNode.x - NODE_WIDTH / 2),
-            y: snapToGrid(dagreNode.y - NODE_HEIGHT / 2),
-          })
-        }
+      const nodeData = g.node(n.id)
+      if (nodeData) {
+        newPositions.set(n.id, {
+          x: snapToGrid(nodeData.x - NODE_WIDTH / 2),
+          y: snapToGrid(nodeData.y - NODE_HEIGHT / 2),
+        })
       }
     })
 
-    // Position loop body nodes below their parent loop
+    // Position loop body nodes based on their loop's spacer position
     loopNodes.forEach(loopId => {
+      const spacerId = spacerNodes.get(loopId)!
+      const spacerData = g.node(spacerId)
       const loopPos = newPositions.get(loopId)
-      if (!loopPos) return
+      const bodyNodes = loopBodyNodes.get(loopId)
 
-      const bodyNodeIds = Array.from(loopBodyNodes.get(loopId) || [])
-      if (bodyNodeIds.length === 0) return
+      if (!spacerData || !loopPos || !bodyNodes || bodyNodes.size === 0) return
 
-      // Create sub-graph for body layout
+      // Create a mini-graph for loop body nodes
       const bodyGraph = new dagre.graphlib.Graph()
       bodyGraph.setGraph({
         rankdir: 'TB',
-        nodesep: HORIZONTAL_SPACING * 0.7,
-        ranksep: VERTICAL_SPACING * 0.8,
-        marginx: 0,
-        marginy: 0,
+        nodesep: HORIZONTAL_SPACING / 2,
+        ranksep: VERTICAL_SPACING / 2,
       })
       bodyGraph.setDefaultEdgeLabel(() => ({}))
 
-      bodyNodeIds.forEach(nodeId => {
-        bodyGraph.setNode(nodeId, { width: NODE_WIDTH, height: NODE_HEIGHT })
+      bodyNodes.forEach(bodyId => {
+        bodyGraph.setNode(bodyId, { width: NODE_WIDTH, height: NODE_HEIGHT })
       })
 
+      // Add edges between body nodes
       edges.forEach(e => {
-        if (loopBodyNodes.get(loopId)!.has(e.source) && loopBodyNodes.get(loopId)!.has(e.target)) {
+        if (bodyNodes.has(e.source) && bodyNodes.has(e.target)) {
           bodyGraph.setEdge(e.source, e.target)
         }
       })
 
+      // Add edges from loop to first body nodes
+      const loopOutEdges = outgoingEdges.get(loopId) || []
+      loopOutEdges.filter(e => e.handle === 'loop').forEach(e => {
+        if (bodyNodes.has(e.target)) {
+          bodyGraph.setNode('_loop_start', { width: 0, height: 0 })
+          bodyGraph.setEdge('_loop_start', e.target)
+        }
+      })
+
       dagre.layout(bodyGraph)
 
-      // 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)
-          minY = Math.min(minY, pos.y - NODE_HEIGHT / 2)
+      // Position body nodes below and indented from the loop
+      const loopCenterX = loopPos.x + NODE_WIDTH / 2
+      const bodyStartY = loopPos.y + NODE_HEIGHT + VERTICAL_SPACING / 2
+
+      let minX = Infinity, maxX = -Infinity
+      bodyNodes.forEach(bodyId => {
+        const bodyData = bodyGraph.node(bodyId)
+        if (bodyData) {
+          minX = Math.min(minX, bodyData.x)
+          maxX = Math.max(maxX, bodyData.x)
         }
       })
 
-      // 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
+      const bodyWidth = maxX - minX
+      const offsetX = loopCenterX - (minX + bodyWidth / 2) + 40 // Indent to the right
 
-      bodyNodeIds.forEach(nodeId => {
-        const pos = bodyGraph.node(nodeId)
-        if (pos) {
-          newPositions.set(nodeId, {
-            x: snapToGrid(bodyStartX + (pos.x - NODE_WIDTH / 2 - minX)),
-            y: snapToGrid(bodyStartY + (pos.y - NODE_HEIGHT / 2 - minY)),
+      bodyNodes.forEach(bodyId => {
+        const bodyData = bodyGraph.node(bodyId)
+        if (bodyData) {
+          newPositions.set(bodyId, {
+            x: snapToGrid(bodyData.x - NODE_WIDTH / 2 + offsetX),
+            y: snapToGrid(bodyData.y - NODE_HEIGHT / 2 + bodyStartY),
           })
         }
       })