From 67019b0df7eb0f4527e000544bfef4e7e149d8c5 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 4 Jun 2026 14:21:11 -0700 Subject: [PATCH 1/2] fix(autolayout): relocate notes that overlap blocks after layout --- .../sim/lib/workflows/autolayout/constants.ts | 7 +- apps/sim/lib/workflows/autolayout/index.ts | 3 + apps/sim/lib/workflows/autolayout/targeted.ts | 18 +- .../lib/workflows/autolayout/utils.test.ts | 213 ++++++++++++++++++ apps/sim/lib/workflows/autolayout/utils.ts | 150 ++++++++++++ 5 files changed, 379 insertions(+), 12 deletions(-) create mode 100644 apps/sim/lib/workflows/autolayout/utils.test.ts diff --git a/apps/sim/lib/workflows/autolayout/constants.ts b/apps/sim/lib/workflows/autolayout/constants.ts index d32e5d68b40..8e64b3cfca2 100644 --- a/apps/sim/lib/workflows/autolayout/constants.ts +++ b/apps/sim/lib/workflows/autolayout/constants.ts @@ -63,10 +63,15 @@ export const OVERLAP_MARGIN = 30 */ export const MAX_OVERLAP_ITERATIONS = 20 +/** + * Note block type identifier + */ +export const NOTE_BLOCK_TYPE = 'note' + /** * Block types excluded from autolayout */ -export const AUTO_LAYOUT_EXCLUDED_TYPES = new Set(['note']) +export const AUTO_LAYOUT_EXCLUDED_TYPES = new Set([NOTE_BLOCK_TYPE]) /** * Container block types that can have children diff --git a/apps/sim/lib/workflows/autolayout/index.ts b/apps/sim/lib/workflows/autolayout/index.ts index 2bbfca7f992..c9a23767c06 100644 --- a/apps/sim/lib/workflows/autolayout/index.ts +++ b/apps/sim/lib/workflows/autolayout/index.ts @@ -12,6 +12,7 @@ import { filterLayoutEligibleBlockIds, getBlocksByParent, prepareContainerDimensions, + resolveNoteOverlaps, } from '@/lib/workflows/autolayout/utils' import type { BlockState } from '@/stores/workflows/workflow/types' @@ -74,6 +75,8 @@ export function applyAutoLayout( layoutContainers(blocksCopy, edges, options) + resolveNoteOverlaps(blocksCopy, verticalSpacing) + logger.info('Auto layout completed successfully', { blockCount: Object.keys(blocksCopy).length, }) diff --git a/apps/sim/lib/workflows/autolayout/targeted.ts b/apps/sim/lib/workflows/autolayout/targeted.ts index e6021885fef..cd8539e84be 100644 --- a/apps/sim/lib/workflows/autolayout/targeted.ts +++ b/apps/sim/lib/workflows/autolayout/targeted.ts @@ -11,7 +11,9 @@ import { filterLayoutEligibleBlockIds, getBlockMetrics, getBlocksByParent, + hasFinitePosition, prepareContainerDimensions, + resolveNoteOverlaps, shouldSkipAutoLayout, snapPositionToGrid, } from '@/lib/workflows/autolayout/utils' @@ -107,6 +109,10 @@ export function applyTargetedLayout( ) } + // 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 }) + return blocksCopy } @@ -184,7 +190,7 @@ function layoutGroup( const invalidPositions = layoutEligibleChildIds.filter((id) => { const block = blocks[id] if (!block) return false - return !hasPosition(block) + return !hasFinitePosition(block) }) const needsLayoutSet = new Set([...requestedLayout, ...invalidPositions]) const needsLayout = Array.from(needsLayoutSet) @@ -536,13 +542,3 @@ function updateContainerDimensions( measuredHeight: parentBlock.data.height, } } - -/** - * Checks if a block has a valid, finite position. - * Returns false for missing, undefined, NaN, or Infinity coordinates. - */ -function hasPosition(block: BlockState): boolean { - if (!block.position) return false - const { x, y } = block.position - return Number.isFinite(x) && Number.isFinite(y) -} diff --git a/apps/sim/lib/workflows/autolayout/utils.test.ts b/apps/sim/lib/workflows/autolayout/utils.test.ts new file mode 100644 index 00000000000..91ac51d7440 --- /dev/null +++ b/apps/sim/lib/workflows/autolayout/utils.test.ts @@ -0,0 +1,213 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it, vi } from 'vitest' +import { DEFAULT_VERTICAL_SPACING } from '@/lib/workflows/autolayout/constants' +import { resolveNoteOverlaps } from '@/lib/workflows/autolayout/utils' +import type { BlockState } from '@/stores/workflows/workflow/types' + +vi.mock('@/blocks', () => ({ + getBlock: () => null, +})) + +function createBlock( + id: string, + type: string, + position: { x: number; y: number }, + overrides: Partial = {} +): BlockState { + return { + id, + type, + name: id, + position, + subBlocks: {}, + outputs: {}, + enabled: true, + layout: { measuredWidth: 250, measuredHeight: 120 }, + ...overrides, + } as BlockState +} + +describe('resolveNoteOverlaps', () => { + it('relocates a note that overlaps a laid-out block', () => { + const blocks: Record = { + a: createBlock('a', 'agent', { x: 150, y: 150 }), + note: createBlock( + 'note', + 'note', + { x: 160, y: 160 }, + { + height: 120, + layout: { measuredHeight: 120 }, + } + ), + } + + resolveNoteOverlaps(blocks, DEFAULT_VERTICAL_SPACING) + + // Block is untouched; note is pushed below the block's bottom edge. + expect(blocks.a.position).toEqual({ x: 150, y: 150 }) + expect(blocks.note.position.x).toBe(150) + expect(blocks.note.position.y).toBeGreaterThanOrEqual(150 + 120 + DEFAULT_VERTICAL_SPACING - 1) + }) + + it('leaves a note that does not overlap any block in place', () => { + const blocks: Record = { + a: createBlock('a', 'agent', { x: 150, y: 150 }), + note: createBlock( + 'note', + 'note', + { x: 2000, y: 2000 }, + { + height: 120, + layout: { measuredHeight: 120 }, + } + ), + } + + resolveNoteOverlaps(blocks, DEFAULT_VERTICAL_SPACING) + + expect(blocks.note.position).toEqual({ x: 2000, y: 2000 }) + }) + + it('stacks multiple overlapping notes without overlapping each other', () => { + const blocks: Record = { + a: createBlock('a', 'agent', { x: 150, y: 150 }), + note1: createBlock( + 'note1', + 'note', + { x: 150, y: 150 }, + { + height: 100, + layout: { measuredHeight: 100 }, + } + ), + note2: createBlock( + 'note2', + 'note', + { x: 200, y: 200 }, + { + height: 100, + layout: { measuredHeight: 100 }, + } + ), + } + + resolveNoteOverlaps(blocks, DEFAULT_VERTICAL_SPACING) + + const n1 = blocks.note1.position + const n2 = blocks.note2.position + // Both relocated, stacked in reading order with no vertical overlap. + expect(n2.y).toBeGreaterThanOrEqual(n1.y + 100) + }) + + it('does nothing when there are no notes', () => { + const blocks: Record = { + a: createBlock('a', 'agent', { x: 150, y: 150 }), + b: createBlock('b', 'agent', { x: 500, y: 150 }), + } + + resolveNoteOverlaps(blocks, DEFAULT_VERTICAL_SPACING) + + expect(blocks.a.position).toEqual({ x: 150, y: 150 }) + expect(blocks.b.position).toEqual({ x: 500, y: 150 }) + }) + + describe('targeted mode (previousBlocks)', () => { + it('relocates a note when a block was moved onto it', () => { + const previousBlocks: Record = { + a: createBlock('a', 'agent', { x: 2000, y: 2000 }), + note: createBlock( + 'note', + 'note', + { x: 150, y: 150 }, + { + height: 120, + layout: { measuredHeight: 120 }, + } + ), + } + // Block "a" has been shifted onto the note by the layout pass. + const blocks: Record = { + a: createBlock('a', 'agent', { x: 150, y: 150 }), + note: createBlock( + 'note', + 'note', + { x: 150, y: 150 }, + { + height: 120, + layout: { measuredHeight: 120 }, + } + ), + } + + resolveNoteOverlaps(blocks, DEFAULT_VERTICAL_SPACING, { previousBlocks }) + + expect(blocks.note.position.x).toBe(150) + expect(blocks.note.position.y).toBeGreaterThan(150) + }) + + it('preserves a pre-existing overlap not introduced by this pass', () => { + // The note already overlapped block "a" before the pass; "a" did not move. + const previousBlocks: Record = { + a: createBlock('a', 'agent', { x: 150, y: 150 }), + note: createBlock( + 'note', + 'note', + { x: 160, y: 160 }, + { + height: 120, + layout: { measuredHeight: 120 }, + } + ), + } + const blocks: Record = { + a: createBlock('a', 'agent', { x: 150, y: 150 }), + note: createBlock( + 'note', + 'note', + { x: 160, y: 160 }, + { + height: 120, + layout: { measuredHeight: 120 }, + } + ), + } + + resolveNoteOverlaps(blocks, DEFAULT_VERTICAL_SPACING, { previousBlocks }) + + expect(blocks.note.position).toEqual({ x: 160, y: 160 }) + }) + + it('relocates when a newly added block (no prior position) lands on a note', () => { + const previousBlocks: Record = { + note: createBlock( + 'note', + 'note', + { x: 150, y: 150 }, + { + height: 120, + layout: { measuredHeight: 120 }, + } + ), + } + const blocks: Record = { + a: createBlock('a', 'agent', { x: 150, y: 150 }), + note: createBlock( + 'note', + 'note', + { x: 150, y: 150 }, + { + height: 120, + layout: { measuredHeight: 120 }, + } + ), + } + + resolveNoteOverlaps(blocks, DEFAULT_VERTICAL_SPACING, { previousBlocks }) + + expect(blocks.note.position.y).toBeGreaterThan(150) + }) + }) +}) diff --git a/apps/sim/lib/workflows/autolayout/utils.ts b/apps/sim/lib/workflows/autolayout/utils.ts index ab42b48c9e3..36a025f661b 100644 --- a/apps/sim/lib/workflows/autolayout/utils.ts +++ b/apps/sim/lib/workflows/autolayout/utils.ts @@ -4,6 +4,7 @@ import { CONTAINER_PADDING, CONTAINER_PADDING_X, CONTAINER_PADDING_Y, + NOTE_BLOCK_TYPE, ROOT_PADDING_X, ROOT_PADDING_Y, } from '@/lib/workflows/autolayout/constants' @@ -313,6 +314,155 @@ export function boxesOverlap(box1: BoundingBox, box2: BoundingBox, margin = 0): ) } +/** + * Resolves the on-canvas dimensions of a note block. + * Notes are fixed-width and use a deterministic height, but fall back to any + * stored measurement or data override when present. + */ +function getNoteDimensions(block: BlockState): { width: number; height: number } { + const width = Math.max( + resolveNumeric(block.data?.width, 0), + block.layout?.measuredWidth ?? 0, + BLOCK_DIMENSIONS.FIXED_WIDTH + ) + + const defaultHeight = + BLOCK_DIMENSIONS.HEADER_HEIGHT + + BLOCK_DIMENSIONS.NOTE_CONTENT_PADDING + + BLOCK_DIMENSIONS.NOTE_BASE_CONTENT_HEIGHT + + const height = Math.max( + resolveNumeric(block.data?.height, 0), + block.layout?.measuredHeight ?? 0, + block.height ?? 0, + defaultHeight + ) + + return { width, height } +} + +/** + * Checks if a block has a valid, finite position. + * Returns false for missing, undefined, NaN, or Infinity coordinates. + */ +export function hasFinitePosition(block: BlockState): boolean { + return Number.isFinite(block.position?.x) && Number.isFinite(block.position?.y) +} + +/** + * Determines whether a note and a block overlapped at their pre-layout + * positions. Used by targeted layout to distinguish overlaps introduced by the + * current pass from arrangements that already existed. + */ +function noteOverlappedBlockBefore( + previousBlocks: Record, + noteId: string, + noteDimensions: { width: number; height: number }, + blockId: string +): boolean { + const previousNote = previousBlocks[noteId] + const previousBlock = previousBlocks[blockId] + if (!previousNote || !previousBlock) return false + + // A block without a finite prior position was not yet placed on the canvas, + // so it could not have overlapped anything before this pass. + if (!hasFinitePosition(previousNote) || !hasFinitePosition(previousBlock)) return false + + const noteBox = createBoundingBox(previousNote.position, noteDimensions) + const blockBox = createBoundingBox(previousBlock.position, getBlockMetrics(previousBlock)) + return boxesOverlap(blockBox, noteBox) +} + +export interface ResolveNoteOverlapsOptions { + /** + * Pre-layout block snapshot. When provided, only notes whose overlap with a + * block was *introduced* by the current pass are relocated (i.e. they overlap + * now but did not at these positions). Targeted layout passes this so that + * pre-existing note arrangements are preserved. When omitted (full + * auto-layout), any note overlapping a block is relocated. + */ + previousBlocks?: Record +} + +/** + * Relocates note blocks that overlap the laid-out workflow blocks. + * + * Notes are excluded from the topological layout because they are free-form + * annotations, but repositioned workflow blocks can land on top of them. This + * pass keeps notes that already sit in clear space and stacks any relocated + * notes in a column beneath their parent group's blocks, preserving the notes' + * relative reading order. Notes are grouped by parent so notes inside a + * container are resolved against that container's children only. + * + * Placement is always computed against the full set of blocks and notes in the + * group, so a relocated note never collides with anything regardless of which + * overlaps triggered the relocation. + */ +export function resolveNoteOverlaps( + blocks: Record, + verticalSpacing: number, + options: ResolveNoteOverlapsOptions = {} +): void { + const { previousBlocks } = options + const { root, children } = getBlocksByParent(blocks) + const groups: string[][] = [root, ...Array.from(children.values())] + + for (const groupIds of groups) { + const obstacles: Array<{ id: string; box: BoundingBox }> = [] + const noteIds: string[] = [] + let maxBottom = Number.NEGATIVE_INFINITY + let minX = Number.POSITIVE_INFINITY + + for (const id of groupIds) { + const block = blocks[id] + if (!block) continue + + if (block.type === NOTE_BLOCK_TYPE) { + noteIds.push(id) + const { height } = getNoteDimensions(block) + maxBottom = Math.max(maxBottom, block.position.y + height) + continue + } + + if (shouldSkipAutoLayout(block)) continue + + const box = createBoundingBox(block.position, getBlockMetrics(block)) + obstacles.push({ id, box }) + maxBottom = Math.max(maxBottom, box.y + box.height) + minX = Math.min(minX, box.x) + } + + if (noteIds.length === 0 || obstacles.length === 0) continue + + noteIds.sort((a, b) => { + const posA = blocks[a].position + const posB = blocks[b].position + return posA.y - posB.y || posA.x - posB.x + }) + + let stackY = maxBottom + verticalSpacing + + for (const id of noteIds) { + const note = blocks[id] + const dimensions = getNoteDimensions(note) + const noteBox = createBoundingBox(note.position, dimensions) + + const needsRelocation = obstacles.some(({ id: blockId, box }) => { + if (!boxesOverlap(box, noteBox)) return false + if (previousBlocks) { + return !noteOverlappedBlockBefore(previousBlocks, id, dimensions, blockId) + } + return true + }) + + if (!needsRelocation) continue + + note.position = { x: minX, y: stackY } + stackY += dimensions.height + verticalSpacing + } + } +} + /** * Groups blocks by their parent container */ From 8c1c3e01a939676be31d282063a40317d0a81ca4 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 4 Jun 2026 14:36:09 -0700 Subject: [PATCH 2/2] fix(autolayout): harden note overlap resolution against resize and non-finite positions --- .../lib/workflows/autolayout/utils.test.ts | 25 +++++++++++++++++++ apps/sim/lib/workflows/autolayout/utils.ts | 11 +++++--- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/apps/sim/lib/workflows/autolayout/utils.test.ts b/apps/sim/lib/workflows/autolayout/utils.test.ts index 91ac51d7440..b8cb51abe74 100644 --- a/apps/sim/lib/workflows/autolayout/utils.test.ts +++ b/apps/sim/lib/workflows/autolayout/utils.test.ts @@ -114,6 +114,31 @@ describe('resolveNoteOverlaps', () => { expect(blocks.b.position).toEqual({ x: 500, y: 150 }) }) + it('never produces non-finite coordinates when a block has a NaN position', () => { + const blocks: Record = { + bad: createBlock('bad', 'agent', { x: Number.NaN, y: Number.NaN }), + a: createBlock('a', 'agent', { x: 150, y: 150 }), + note: createBlock( + 'note', + 'note', + { x: 150, y: 150 }, + { + height: 120, + layout: { measuredHeight: 120 }, + } + ), + } + + resolveNoteOverlaps(blocks, DEFAULT_VERTICAL_SPACING) + + // The corrupted block is ignored; the note still relocates off block "a" + // using only finite coordinates. + expect(Number.isFinite(blocks.note.position.x)).toBe(true) + expect(Number.isFinite(blocks.note.position.y)).toBe(true) + expect(blocks.note.position.x).toBe(150) + expect(blocks.note.position.y).toBeGreaterThan(150) + }) + describe('targeted mode (previousBlocks)', () => { it('relocates a note when a block was moved onto it', () => { const previousBlocks: Record = { diff --git a/apps/sim/lib/workflows/autolayout/utils.ts b/apps/sim/lib/workflows/autolayout/utils.ts index 36a025f661b..f25c1086745 100644 --- a/apps/sim/lib/workflows/autolayout/utils.ts +++ b/apps/sim/lib/workflows/autolayout/utils.ts @@ -357,7 +357,6 @@ export function hasFinitePosition(block: BlockState): boolean { function noteOverlappedBlockBefore( previousBlocks: Record, noteId: string, - noteDimensions: { width: number; height: number }, blockId: string ): boolean { const previousNote = previousBlocks[noteId] @@ -368,7 +367,9 @@ function noteOverlappedBlockBefore( // so it could not have overlapped anything before this pass. if (!hasFinitePosition(previousNote) || !hasFinitePosition(previousBlock)) return false - const noteBox = createBoundingBox(previousNote.position, noteDimensions) + // Derive dimensions from the prior blocks so a resize between passes does not + // pair new dimensions with the old position. + const noteBox = createBoundingBox(previousNote.position, getNoteDimensions(previousNote)) const blockBox = createBoundingBox(previousBlock.position, getBlockMetrics(previousBlock)) return boxesOverlap(blockBox, noteBox) } @@ -415,7 +416,9 @@ export function resolveNoteOverlaps( for (const id of groupIds) { const block = blocks[id] - if (!block) continue + // Skip non-finite positions so corrupted coordinates never propagate into + // minX / maxBottom (and therefore into relocated note positions). + if (!block || !hasFinitePosition(block)) continue if (block.type === NOTE_BLOCK_TYPE) { noteIds.push(id) @@ -450,7 +453,7 @@ export function resolveNoteOverlaps( const needsRelocation = obstacles.some(({ id: blockId, box }) => { if (!boxesOverlap(box, noteBox)) return false if (previousBlocks) { - return !noteOverlappedBlockBefore(previousBlocks, id, dimensions, blockId) + return !noteOverlappedBlockBefore(previousBlocks, id, blockId) } return true })