From 474fa94b819c5c773b41cc4ca38ea6a5c9c732d3 Mon Sep 17 00:00:00 2001 From: Justin Blumencranz <96924014+j15z@users.noreply.github.com> Date: Thu, 13 Aug 2026 15:25:06 -0700 Subject: [PATCH] fix(autolayout): rescue new notes from blocks they were created on top of --- .../server/workflow/edit-workflow/index.ts | 1 + .../lib/workflows/autolayout/targeted.test.ts | 61 +++++++++++++++++++ apps/sim/lib/workflows/autolayout/targeted.ts | 16 ++++- apps/sim/lib/workflows/diff/diff-engine.ts | 1 + 4 files changed, 77 insertions(+), 2 deletions(-) diff --git a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/index.ts b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/index.ts index 24c1510833b..b7ecfeed71a 100644 --- a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/index.ts +++ b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/index.ts @@ -336,6 +336,7 @@ export const editWorkflowServerTool: BaseServerTool shiftSourceBlockIds, horizontalSpacing: DEFAULT_HORIZONTAL_SPACING, verticalSpacing: DEFAULT_VERTICAL_SPACING, + previousBlocks: workflowState.blocks, }) } catch (error) { logger.warn('Targeted autolayout failed, using default positions', { diff --git a/apps/sim/lib/workflows/autolayout/targeted.test.ts b/apps/sim/lib/workflows/autolayout/targeted.test.ts index ae4b89b1242..87bedd6b4a6 100644 --- a/apps/sim/lib/workflows/autolayout/targeted.test.ts +++ b/apps/sim/lib/workflows/autolayout/targeted.test.ts @@ -458,4 +458,65 @@ describe('applyTargetedLayout', () => { result.loop.position.x + loopMetrics.width ) }) + + it('relocates a newly added note off a block it was created on top of', () => { + // A copilot note is born at the (0,0) placeholder, and a fresh workflow's + // start block lives at (0,0) too. The pre-edit snapshot does not contain + // the note, so the overlap counts as introduced by this edit and the note + // must be moved to the stack below the flow, not preserved as intentional. + const blocks = { + start: createBlock('start', { position: { x: 0, y: 0 } }), + note: createBlock('note', { + type: 'note', + position: { x: 0, y: 0 }, + subBlocks: { + content: { id: 'content', type: 'long-input', value: 'Explains the workflow' }, + }, + }), + } + + const result = applyTargetedLayout(blocks, [], { + changedBlockIds: ['note'], + previousBlocks: { start: blocks.start }, + }) + + const startMetrics = getBlockMetrics(result.start) + expect(result.start.position).toEqual({ x: 0, y: 0 }) + expect(result.note.position.y).toBeGreaterThanOrEqual( + result.start.position.y + startMetrics.height + DEFAULT_VERTICAL_SPACING + ) + }) + + it('preserves a pre-existing note arrangement when an unrelated block is laid out', () => { + const blocks = { + start: createBlock('start', { position: { x: 0, y: 0 } }), + note: createBlock('note', { + type: 'note', + position: { x: 60, y: 10 }, + subBlocks: { + content: { id: 'content', type: 'long-input', value: 'Deliberately parked here' }, + }, + }), + added: createBlock('added', { position: { x: 0, y: 0 } }), + } + + const edges: Edge[] = [{ id: 'e1', source: 'start', target: 'added' }] + + const result = applyTargetedLayout(blocks, edges, { + changedBlockIds: ['added'], + previousBlocks: { + start: createBlock('start', { position: { x: 0, y: 0 } }), + note: createBlock('note', { + type: 'note', + position: { x: 60, y: 10 }, + subBlocks: { + content: { id: 'content', type: 'long-input', value: 'Deliberately parked here' }, + }, + }), + }, + }) + + expect(result.note.position).toEqual({ x: 60, y: 10 }) + expect(result.added.position.x).toBeGreaterThan(result.start.position.x) + }) }) diff --git a/apps/sim/lib/workflows/autolayout/targeted.ts b/apps/sim/lib/workflows/autolayout/targeted.ts index ef76654e179..bf09c74cfed 100644 --- a/apps/sim/lib/workflows/autolayout/targeted.ts +++ b/apps/sim/lib/workflows/autolayout/targeted.ts @@ -35,6 +35,15 @@ export interface TargetedLayoutOptions extends LayoutOptions { shiftSourceBlockIds?: string[] verticalSpacing?: number horizontalSpacing?: number + /** + * Pre-edit block snapshot used to judge whether a note's overlap with a block + * is a pre-existing arrangement (preserved) or was introduced by the edit + * being laid out (relocated). Without it, the post-edit input blocks serve as + * the baseline — which contains newly added notes at their `(0,0)` placeholder, + * so a new note dropped onto a block at the origin (e.g. a fresh workflow's + * start block) reads as intentional and is never rescued. + */ + previousBlocks?: Record } /** @@ -53,6 +62,7 @@ export function applyTargetedLayout( verticalSpacing = DEFAULT_VERTICAL_SPACING, horizontalSpacing = DEFAULT_HORIZONTAL_SPACING, gridSize, + previousBlocks = blocks, } = options if ( @@ -135,8 +145,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 }) + // the baseline positions so pre-existing note arrangements are preserved. A + // note absent from the baseline (newly added by this edit) is always eligible + // for relocation. + resolveNoteOverlaps(blocksCopy, verticalSpacing, { previousBlocks }) return blocksCopy } diff --git a/apps/sim/lib/workflows/diff/diff-engine.ts b/apps/sim/lib/workflows/diff/diff-engine.ts index 84271c60ab4..52b8bb8210f 100644 --- a/apps/sim/lib/workflows/diff/diff-engine.ts +++ b/apps/sim/lib/workflows/diff/diff-engine.ts @@ -490,6 +490,7 @@ export class WorkflowDiffEngine { shiftSourceBlockIds, horizontalSpacing: DEFAULT_HORIZONTAL_SPACING, verticalSpacing: DEFAULT_VERTICAL_SPACING, + previousBlocks: mergedBaseline.blocks, }) Object.entries(layoutedBlocks).forEach(([id, layoutBlock]) => {