fix(autolayout): keep branches in stable rows across layers

apps/sim/lib/workflows/autolayout/core.test.ts

@@ -0,0 +1,108 @@
+/**
+ * @vitest-environment node
+ */
+import { HANDLE_POSITIONS } from '@sim/workflow-renderer'
+import { describe, expect, it, vi } from 'vitest'
+import { layoutBlocksCore } from '@/lib/workflows/autolayout/core'
+import type { Edge } from '@/lib/workflows/autolayout/types'
+import type { BlockState } from '@/stores/workflows/workflow/types'
+
+vi.mock('@/blocks', () => ({
+  getBlock: () => null,
+}))
+
+function createBlock(id: string): BlockState {
+  return {
+    id,
+    type: 'agent',
+    name: id,
+    position: { x: 0, y: 0 },
+    subBlocks: {},
+    outputs: {},
+    enabled: true,
+    height: 120,
+    layout: { measuredWidth: 250, measuredHeight: 120 },
+  } as BlockState
+}
+
+describe('layoutBlocksCore', () => {
+  it('keeps each branch in a stable row regardless of block insertion order', () => {
+    // Two parallel chains from one source. Insertion order is interleaved so the
+    // per-layer order flips: layer1 [a1,b1], layer2 [b2,a2], layer3 [a3,b3].
+    // Row assignment must come from the resolved predecessor position, not from
+    // per-layer insertion-order tie-breaks.
+    const blocks: Record<string, BlockState> = {}
+    for (const id of ['s', 'a1', 'b1', 'b2', 'a2', 'a3', 'b3']) {
+      blocks[id] = createBlock(id)
+    }
+    const edges: Edge[] = [
+      { id: 'e1', source: 's', target: 'a1' },
+      { id: 'e2', source: 's', target: 'b1' },
+      { id: 'e3', source: 'a1', target: 'a2' },
+      { id: 'e4', source: 'b1', target: 'b2' },
+      { id: 'e5', source: 'a2', target: 'a3' },
+      { id: 'e6', source: 'b2', target: 'b3' },
+    ]
+
+    const { nodes } = layoutBlocksCore(blocks, edges, { isContainer: false })
+    const y = (id: string) => nodes.get(id)!.position.y
+
+    const aAboveInLayer1 = y('a1') < y('b1')
+    expect(y('a2') < y('b2')).toBe(aAboveInLayer1)
+    expect(y('a3') < y('b3')).toBe(aAboveInLayer1)
+  })
+
+  it('leaves no vertical overlaps within a layer', () => {
+    const blocks: Record<string, BlockState> = {}
+    for (const id of ['s', 'a1', 'b1', 'c1']) {
+      blocks[id] = createBlock(id)
+    }
+    const edges: Edge[] = [
+      { id: 'e1', source: 's', target: 'a1' },
+      { id: 'e2', source: 's', target: 'b1' },
+      { id: 'e3', source: 's', target: 'c1' },
+    ]
+
+    const { nodes } = layoutBlocksCore(blocks, edges, { isContainer: false })
+    const layer1 = ['a1', 'b1', 'c1']
+      .map((id) => nodes.get(id)!)
+      .sort((a, b) => a.position.y - b.position.y)
+
+    for (let i = 0; i < layer1.length - 1; i++) {
+      expect(layer1[i + 1].position.y).toBeGreaterThanOrEqual(
+        layer1[i].position.y + layer1[i].metrics.height
+      )
+    }
+  })
+
+  it('aligns an error branch to the source block bottom using layout metrics height', () => {
+    // The source's height comes from layout.measuredHeight, not the raw height
+    // field, so an error handle offset read off block.height would fall back to
+    // MIN_HEIGHT and place the error branch near the block's top instead.
+    const source = {
+      ...createBlock('source'),
+      height: undefined,
+      layout: { measuredWidth: 250, measuredHeight: 400 },
+    } as BlockState
+
+    const blocks: Record<string, BlockState> = {
+      source,
+      ok: createBlock('ok'),
+      failed: createBlock('failed'),
+    }
+    const edges: Edge[] = [
+      { id: 'e1', source: 'source', target: 'ok' },
+      { id: 'e2', source: 'source', target: 'failed', sourceHandle: 'error' },
+    ]
+
+    const { nodes } = layoutBlocksCore(blocks, edges, { isContainer: false })
+    const sourceNode = nodes.get('source')!
+
+    // The error branch's target handle lines up with the source's error handle,
+    // which sits ERROR_BOTTOM_OFFSET above the source's real bottom edge.
+    const errorHandleY =
+      sourceNode.position.y + sourceNode.metrics.height - HANDLE_POSITIONS.ERROR_BOTTOM_OFFSET
+    expect(nodes.get('failed')!.position.y + HANDLE_POSITIONS.DEFAULT_Y_OFFSET).toBe(errorHandleY)
+    expect(nodes.get('ok')!.position.y).toBeLessThan(nodes.get('failed')!.position.y)
+  })
+})

apps/sim/lib/workflows/autolayout/core.ts

@@ -1,9 +1,8 @@
 import { createLogger } from '@sim/logger'
-import { BLOCK_DIMENSIONS, HANDLE_POSITIONS } from '@sim/workflow-renderer'
+import { HANDLE_POSITIONS } from '@sim/workflow-renderer'
 import {
   CONTAINER_LAYOUT_OPTIONS,
   DEFAULT_LAYOUT_OPTIONS,
-  MAX_OVERLAP_ITERATIONS,
 } from '@/lib/workflows/autolayout/constants'
 import type { Edge, GraphNode, LayoutOptions } from '@/lib/workflows/autolayout/types'
 import {
@@ -22,11 +21,18 @@ const SUBFLOW_START_HANDLES = new Set(['loop-start-source', 'parallel-start-sour
 
 /**
  * Calculates the Y offset for a source handle based on block type and handle ID.
+ *
+ * The error handle sits relative to the block's bottom, so it must use the same
+ * height the rest of the layout uses (`node.metrics.height`). Reading the raw
+ * `block.height` field instead would mis-place the handle for any block sized
+ * from `layout.measuredHeight` or from an estimate, pulling error branches far
+ * above their real handle.
  */
-function getSourceHandleYOffset(block: BlockState, sourceHandle?: string | null): number {
+function getSourceHandleYOffset(node: GraphNode, sourceHandle?: string | null): number {
+  const block = node.block
+
   if (sourceHandle === 'error') {
-    const blockHeight = block.height || BLOCK_DIMENSIONS.MIN_HEIGHT
-    return blockHeight - HANDLE_POSITIONS.ERROR_BOTTOM_OFFSET
+    return node.metrics.height - HANDLE_POSITIONS.ERROR_BOTTOM_OFFSET
   }
 
   if (sourceHandle && SUBFLOW_START_HANDLES.has(sourceHandle)) {
@@ -200,57 +206,23 @@ export function groupByLayer(nodes: Map<string, GraphNode>): Map<number, GraphNo
 }
 
 /**
- * Resolves vertical overlaps between nodes in the same layer.
+ * Resolves vertical overlaps between nodes within a single layer by pushing
+ * lower nodes down. Must run before the next layer is positioned so successors
+ * derive their Y from resolved predecessor positions — this keeps each branch
+ * in a stable row instead of re-breaking Y ties per layer.
  * X overlaps are prevented by construction via cumulative width-based positioning.
  */
-function resolveVerticalOverlaps(nodes: GraphNode[], verticalSpacing: number): void {
-  let iteration = 0
-  let hasOverlap = true
-
-  while (hasOverlap && iteration < MAX_OVERLAP_ITERATIONS) {
-    hasOverlap = false
-    iteration++
-
-    const nodesByLayer = new Map<number, GraphNode[]>()
-    for (const node of nodes) {
-      if (!nodesByLayer.has(node.layer)) {
-        nodesByLayer.set(node.layer, [])
-      }
-      nodesByLayer.get(node.layer)!.push(node)
-    }
-
-    for (const [layer, layerNodes] of nodesByLayer) {
-      if (layerNodes.length < 2) continue
-
-      layerNodes.sort((a, b) => a.position.y - b.position.y)
-
-      for (let i = 0; i < layerNodes.length - 1; i++) {
-        const node1 = layerNodes[i]
-        const node2 = layerNodes[i + 1]
-
-        const node1Bottom = node1.position.y + node1.metrics.height
-        const requiredY = node1Bottom + verticalSpacing
+function resolveLayerOverlaps(layerNodes: GraphNode[], verticalSpacing: number): void {
+  if (layerNodes.length < 2) return
 
-        if (node2.position.y < requiredY) {
-          hasOverlap = true
-          node2.position.y = requiredY
+  const sorted = [...layerNodes].sort((a, b) => a.position.y - b.position.y)
 
-          logger.debug('Resolved vertical overlap in layer', {
-            layer,
-            block1: node1.id,
-            block2: node2.id,
-            iteration,
-          })
-        }
-      }
+  for (let i = 0; i < sorted.length - 1; i++) {
+    const requiredY = sorted[i].position.y + sorted[i].metrics.height + verticalSpacing
+    if (sorted[i + 1].position.y < requiredY) {
+      sorted[i + 1].position.y = requiredY
     }
   }
-
-  if (hasOverlap) {
-    logger.warn('Could not fully resolve all vertical overlaps after max iterations', {
-      iterations: MAX_OVERLAP_ITERATIONS,
-    })
-  }
 }
 
 /**
@@ -354,7 +326,7 @@ export function calculatePositions(
       for (const edge of incomingEdges) {
         const predecessor = allNodes.get(edge.source)
         if (predecessor) {
-          const sourceHandleOffset = getSourceHandleYOffset(predecessor.block, edge.sourceHandle)
+          const sourceHandleOffset = getSourceHandleYOffset(predecessor, edge.sourceHandle)
           const sourceHandleY = predecessor.position.y + sourceHandleOffset
 
           if (sourceHandleY > bestSourceHandleY) {
@@ -372,9 +344,9 @@ export function calculatePositions(
 
       node.position = { x: xPosition, y: bestSourceHandleY - targetHandleOffset }
     }
-  }
 
-  resolveVerticalOverlaps(Array.from(layers.values()).flat(), verticalSpacing)
+    resolveLayerOverlaps(nodesInLayer, verticalSpacing)
+  }
 }
 
 /**

apps/sim/lib/workflows/autolayout/targeted.test.ts

@@ -345,4 +345,117 @@ describe('applyTargetedLayout', () => {
       result.existing.position.y + existingMetrics.height
     )
   })
+
+  it('places a new sibling container below a frozen container at its rendered height', () => {
+    // The frozen container's children sit further apart than a fresh layout would
+    // place them (a stale layout from when those blocks measured taller). Its
+    // rendered height therefore exceeds the height a hypothetical re-layout
+    // predicts, and the new sibling must clear the rendered one.
+    const spacedChild = (id: string, y: number, parentId: string) =>
+      createBlock(id, {
+        position: { x: 150, y },
+        data: { parentId, extent: 'parent' },
+        layout: { measuredWidth: 250, measuredHeight: 300 },
+        height: 300,
+      })
+
+    const blocks = {
+      start: createBlock('start', {
+        type: 'start_trigger',
+        position: { x: 0, y: 0 },
+        layout: { measuredWidth: 250, measuredHeight: 100 },
+        height: 100,
+      }),
+      frozenLoop: createBlock('frozenLoop', {
+        type: 'loop',
+        position: { x: 400, y: 0 },
+        data: { width: 900, height: 1200 },
+        layout: { measuredWidth: 900, measuredHeight: 1200 },
+      }),
+      frozenTop: spacedChild('frozenTop', 100, 'frozenLoop'),
+      frozenBottom: spacedChild('frozenBottom', 750, 'frozenLoop'),
+      newLoop: createBlock('newLoop', {
+        type: 'loop',
+        position: { x: 0, y: 0 },
+        data: { width: 500, height: 300 },
+      }),
+      newChild: createBlock('newChild', {
+        position: { x: 0, y: 0 },
+        data: { parentId: 'newLoop', extent: 'parent' },
+      }),
+    }
+
+    const edges: Edge[] = [
+      { id: 'e1', source: 'start', target: 'frozenLoop' },
+      { id: 'e2', source: 'start', target: 'newLoop' },
+      { id: 'e3', source: 'frozenTop', target: 'frozenBottom' },
+    ]
+
+    const result = applyTargetedLayout(blocks, edges, {
+      changedBlockIds: ['newLoop', 'newChild'],
+    })
+
+    // Frozen children must not move, so the frozen container keeps its tall size.
+    expect(result.frozenTop.position).toEqual({ x: 150, y: 100 })
+    expect(result.frozenBottom.position).toEqual({ x: 150, y: 750 })
+
+    const containers = [result.frozenLoop, result.newLoop].sort(
+      (a, b) => a.position.y - b.position.y
+    )
+    const upperBottom = containers[0].position.y + getBlockMetrics(containers[0]).height
+    expect(containers[1].position.y).toBeGreaterThanOrEqual(upperBottom)
+  })
+
+  it('shifts downstream blocks right when a container grows from an inserted child', () => {
+    // Inserting inside a container widens it, but that resize happens during
+    // layout and so is absent from the caller's change set. Blocks after the
+    // container must still move clear of its new right edge.
+    const blocks = {
+      loop: createBlock('loop', {
+        type: 'loop',
+        position: { x: 100, y: 100 },
+        data: { width: 900, height: 400 },
+        layout: { measuredWidth: 900, measuredHeight: 400 },
+      }),
+      first: createBlock('first', {
+        position: { x: 150, y: 150 },
+        data: { parentId: 'loop', extent: 'parent' },
+        layout: { measuredWidth: 250, measuredHeight: 120 },
+        height: 120,
+      }),
+      last: createBlock('last', {
+        position: { x: 600, y: 150 },
+        data: { parentId: 'loop', extent: 'parent' },
+        layout: { measuredWidth: 250, measuredHeight: 120 },
+        height: 120,
+      }),
+      inserted: createBlock('inserted', {
+        position: { x: 0, y: 0 },
+        data: { parentId: 'loop', extent: 'parent' },
+      }),
+      after: createBlock('after', {
+        position: { x: 1100, y: 150 },
+        layout: { measuredWidth: 250, measuredHeight: 120 },
+        height: 120,
+      }),
+    }
+
+    const edges: Edge[] = [
+      { id: 'e1', source: 'first', target: 'inserted' },
+      { id: 'e2', source: 'inserted', target: 'last' },
+      { id: 'e3', source: 'loop', target: 'after', sourceHandle: 'loop-end-source' },
+    ]
+
+    const result = applyTargetedLayout(blocks, edges, {
+      changedBlockIds: ['inserted'],
+      shiftSourceBlockIds: ['inserted'],
+    })
+
+    const loopMetrics = getBlockMetrics(result.loop)
+    expect(loopMetrics.width).toBeGreaterThan(900)
+    expect(result.loop.position).toEqual({ x: 100, y: 100 })
+    expect(result.after.position.x).toBeGreaterThanOrEqual(
+      result.loop.position.x + loopMetrics.width
+    )
+  })
 })