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

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

* fix(autolayout): container overlap on insert and error-branch alignment

Lay out contained groups before their containers so a container carries the
height it renders at before siblings are placed beside it. Targeted layout
sized containers from a hypothetical child layout it never applied, so a
container whose children stay frozen rendered taller than the space reserved
for it and overlapped its neighbour.

Also mark containers resized by a child edit so following blocks shift clear,
and derive the error handle offset from the layout's own block height instead
of the raw height field.

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,56 +206,22 @@ 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
+function resolveLayerOverlaps(layerNodes: GraphNode[], verticalSpacing: number): void {
+  if (layerNodes.length < 2) return
 
-  while (hasOverlap && iteration < MAX_OVERLAP_ITERATIONS) {
-    hasOverlap = false
-    iteration++
+  const sorted = [...layerNodes].sort((a, b) => a.position.y - b.position.y)
 
-    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 (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
     }
-
-    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
-
-        if (node2.position.y < requiredY) {
-          hasOverlap = true
-          node2.position.y = requiredY
-
-          logger.debug('Resolved vertical overlap in layer', {
-            layer,
-            block1: node1.id,
-            block2: node2.id,
-            iteration,
-          })
-        }
-      }
-    }
-  }
-
-  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
+    )
+  })
 })

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

@@ -13,10 +13,12 @@ import {
   getBlockMetrics,
   getBlocksByParent,
   hasFinitePosition,
+  isContainerType,
   prepareContainerDimensions,
   resolveNoteOverlaps,
   shouldSkipAutoLayout,
   snapPositionToGrid,
+  sortContainersDeepestFirst,
 } from '@/lib/workflows/autolayout/utils'
 import type { BlockState } from '@/stores/workflows/workflow/types'
 
@@ -66,6 +68,8 @@ export function applyTargetedLayout(
   const shiftSourceSet = new Set(shiftSourceBlockIds)
   const blocksCopy: Record<string, BlockState> = structuredClone(blocks)
 
+  const containerSizesBeforeLayout = measureContainers(blocksCopy)
+
   prepareContainerDimensions(
     blocksCopy,
     edges,
@@ -79,6 +83,43 @@ export function applyTargetedLayout(
 
   const subflowDepths = calculateSubflowDepths(blocksCopy, edges, assignLayers)
 
+  // Lay out contained groups before the groups that contain them. `prepareContainerDimensions`
+  // sizes a container from a hypothetical layout of its children, but targeted layout keeps
+  // frozen children where they are — so that size only becomes real once the container's own
+  // group has run. Finishing inner groups first means every container carries the size it will
+  // actually render at before anything is positioned beside it.
+  for (const containerId of sortContainersDeepestFirst(
+    Array.from(groups.children.keys()),
+    blocksCopy
+  )) {
+    layoutGroup(
+      containerId,
+      groups.children.get(containerId) ?? [],
+      blocksCopy,
+      edges,
+      changedSet,
+      resizedSet,
+      shiftSourceSet,
+      verticalSpacing,
+      horizontalSpacing,
+      subflowDepths,
+      gridSize
+    )
+
+    // A container resized by an edit to its *children* is invisible to the caller's change
+    // set. Mark it resized now that its final size is known, so the enclosing group shifts
+    // neighbours out of its way. Deepest-first ordering means a nested container is already
+    // marked before the group that contains it runs.
+    const sizeBefore = containerSizesBeforeLayout.get(containerId)
+    const container = blocksCopy[containerId]
+    if (!sizeBefore || !container) continue
+
+    const sizeAfter = getBlockMetrics(container)
+    if (sizeAfter.width !== sizeBefore.width || sizeAfter.height !== sizeBefore.height) {
+      resizedSet.add(containerId)
+    }
+  }
+
   layoutGroup(
     null,
     groups.root,
@@ -93,22 +134,6 @@ export function applyTargetedLayout(
     gridSize
   )
 
-  for (const [parentId, childIds] of groups.children.entries()) {
-    layoutGroup(
-      parentId,
-      childIds,
-      blocksCopy,
-      edges,
-      changedSet,
-      resizedSet,
-      shiftSourceSet,
-      verticalSpacing,
-      horizontalSpacing,
-      subflowDepths,
-      gridSize
-    )
-  }
-
   // Relocate notes only where this pass introduced an overlap, comparing against
   // the original positions so pre-existing note arrangements are preserved.
   resolveNoteOverlaps(blocksCopy, verticalSpacing, { previousBlocks: blocks })
@@ -116,6 +141,23 @@ export function applyTargetedLayout(
   return blocksCopy
 }
 
+/**
+ * Captures the current on-canvas size of every container block.
+ */
+function measureContainers(
+  blocks: Record<string, BlockState>
+): Map<string, { width: number; height: number }> {
+  const sizes = new Map<string, { width: number; height: number }>()
+
+  for (const [id, block] of Object.entries(blocks)) {
+    if (!isContainerType(block.type)) continue
+    const { width, height } = getBlockMetrics(block)
+    sizes.set(id, { width, height })
+  }
+
+  return sizes
+}
+
 /**
  * Selects the best anchor block for offset computation.
  * Prefers an upstream (predecessor) anchor over a downstream one because

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

@@ -622,6 +622,35 @@ export function calculateSubflowDepths(
   return depths
 }
 
+/**
+ * Orders container IDs by nesting depth, deepest first.
+ *
+ * A container's size depends on its children, and a nested container counts as
+ * one of those children, so inner containers must be resolved before the outer
+ * ones that measure them.
+ */
+export function sortContainersDeepestFirst(
+  containerIds: string[],
+  blocks: Record<string, BlockState>
+): string[] {
+  const containerDepth = new Map<string, number>()
+
+  for (const containerId of containerIds) {
+    let depth = 0
+    let currentId: string | undefined = containerId
+    while (currentId) {
+      const parentId: string | undefined = blocks[currentId]?.data?.parentId
+      currentId = parentId
+      if (currentId) depth++
+    }
+    containerDepth.set(containerId, depth)
+  }
+
+  return [...containerIds].sort(
+    (a, b) => (containerDepth.get(b) ?? 0) - (containerDepth.get(a) ?? 0)
+  )
+}
+
 /**
  * Layout function type for preparing container dimensions.
  * Returns laid out nodes and bounding dimensions.
@@ -663,29 +692,7 @@ export function prepareContainerDimensions(
 ): void {
   const { children } = getBlocksByParent(blocks)
 
-  // Build dependency graph to process nested containers bottom-up
-  const containerIds = Array.from(children.keys())
-  const containerDepth = new Map<string, number>()
-
-  // Calculate nesting depth for each container
-  for (const containerId of containerIds) {
-    let depth = 0
-    let currentId: string | undefined = containerId
-    while (currentId) {
-      const block: BlockState | undefined = blocks[currentId]
-      const parentId: string | undefined = block?.data?.parentId
-      currentId = parentId
-      if (currentId) depth++
-    }
-    containerDepth.set(containerId, depth)
-  }
-
-  // Sort containers by depth (deepest first) for bottom-up processing
-  const sortedContainerIds = containerIds.sort((a, b) => {
-    const depthA = containerDepth.get(a) ?? 0
-    const depthB = containerDepth.get(b) ?? 0
-    return depthB - depthA
-  })
+  const sortedContainerIds = sortContainersDeepestFirst(Array.from(children.keys()), blocks)
 
   // Process each container, laying out its children to determine dimensions
   for (const containerId of sortedContainerIds) {