Skip to content

Commit 44524d1

Browse files
authored
fix(autolayout): rescue new notes from blocks they were created on top of (#6680)
1 parent 1424809 commit 44524d1

4 files changed

Lines changed: 77 additions & 2 deletions

File tree

apps/sim/lib/copilot/tools/server/workflow/edit-workflow/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,7 @@ export const editWorkflowServerTool: BaseServerTool<EditWorkflowParams, unknown>
336336
shiftSourceBlockIds,
337337
horizontalSpacing: DEFAULT_HORIZONTAL_SPACING,
338338
verticalSpacing: DEFAULT_VERTICAL_SPACING,
339+
previousBlocks: workflowState.blocks,
339340
})
340341
} catch (error) {
341342
logger.warn('Targeted autolayout failed, using default positions', {

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

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,4 +458,65 @@ describe('applyTargetedLayout', () => {
458458
result.loop.position.x + loopMetrics.width
459459
)
460460
})
461+
462+
it('relocates a newly added note off a block it was created on top of', () => {
463+
// A copilot note is born at the (0,0) placeholder, and a fresh workflow's
464+
// start block lives at (0,0) too. The pre-edit snapshot does not contain
465+
// the note, so the overlap counts as introduced by this edit and the note
466+
// must be moved to the stack below the flow, not preserved as intentional.
467+
const blocks = {
468+
start: createBlock('start', { position: { x: 0, y: 0 } }),
469+
note: createBlock('note', {
470+
type: 'note',
471+
position: { x: 0, y: 0 },
472+
subBlocks: {
473+
content: { id: 'content', type: 'long-input', value: 'Explains the workflow' },
474+
},
475+
}),
476+
}
477+
478+
const result = applyTargetedLayout(blocks, [], {
479+
changedBlockIds: ['note'],
480+
previousBlocks: { start: blocks.start },
481+
})
482+
483+
const startMetrics = getBlockMetrics(result.start)
484+
expect(result.start.position).toEqual({ x: 0, y: 0 })
485+
expect(result.note.position.y).toBeGreaterThanOrEqual(
486+
result.start.position.y + startMetrics.height + DEFAULT_VERTICAL_SPACING
487+
)
488+
})
489+
490+
it('preserves a pre-existing note arrangement when an unrelated block is laid out', () => {
491+
const blocks = {
492+
start: createBlock('start', { position: { x: 0, y: 0 } }),
493+
note: createBlock('note', {
494+
type: 'note',
495+
position: { x: 60, y: 10 },
496+
subBlocks: {
497+
content: { id: 'content', type: 'long-input', value: 'Deliberately parked here' },
498+
},
499+
}),
500+
added: createBlock('added', { position: { x: 0, y: 0 } }),
501+
}
502+
503+
const edges: Edge[] = [{ id: 'e1', source: 'start', target: 'added' }]
504+
505+
const result = applyTargetedLayout(blocks, edges, {
506+
changedBlockIds: ['added'],
507+
previousBlocks: {
508+
start: createBlock('start', { position: { x: 0, y: 0 } }),
509+
note: createBlock('note', {
510+
type: 'note',
511+
position: { x: 60, y: 10 },
512+
subBlocks: {
513+
content: { id: 'content', type: 'long-input', value: 'Deliberately parked here' },
514+
},
515+
}),
516+
},
517+
})
518+
519+
expect(result.note.position).toEqual({ x: 60, y: 10 })
520+
expect(result.added.position.x).toBeGreaterThan(result.start.position.x)
521+
})
461522
})

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

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,15 @@ export interface TargetedLayoutOptions extends LayoutOptions {
3535
shiftSourceBlockIds?: string[]
3636
verticalSpacing?: number
3737
horizontalSpacing?: number
38+
/**
39+
* Pre-edit block snapshot used to judge whether a note's overlap with a block
40+
* is a pre-existing arrangement (preserved) or was introduced by the edit
41+
* being laid out (relocated). Without it, the post-edit input blocks serve as
42+
* the baseline — which contains newly added notes at their `(0,0)` placeholder,
43+
* so a new note dropped onto a block at the origin (e.g. a fresh workflow's
44+
* start block) reads as intentional and is never rescued.
45+
*/
46+
previousBlocks?: Record<string, BlockState>
3847
}
3948

4049
/**
@@ -53,6 +62,7 @@ export function applyTargetedLayout(
5362
verticalSpacing = DEFAULT_VERTICAL_SPACING,
5463
horizontalSpacing = DEFAULT_HORIZONTAL_SPACING,
5564
gridSize,
65+
previousBlocks = blocks,
5666
} = options
5767

5868
if (
@@ -135,8 +145,10 @@ export function applyTargetedLayout(
135145
)
136146

137147
// Relocate notes only where this pass introduced an overlap, comparing against
138-
// the original positions so pre-existing note arrangements are preserved.
139-
resolveNoteOverlaps(blocksCopy, verticalSpacing, { previousBlocks: blocks })
148+
// the baseline positions so pre-existing note arrangements are preserved. A
149+
// note absent from the baseline (newly added by this edit) is always eligible
150+
// for relocation.
151+
resolveNoteOverlaps(blocksCopy, verticalSpacing, { previousBlocks })
140152

141153
return blocksCopy
142154
}

apps/sim/lib/workflows/diff/diff-engine.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,7 @@ export class WorkflowDiffEngine {
490490
shiftSourceBlockIds,
491491
horizontalSpacing: DEFAULT_HORIZONTAL_SPACING,
492492
verticalSpacing: DEFAULT_VERTICAL_SPACING,
493+
previousBlocks: mergedBaseline.blocks,
493494
})
494495

495496
Object.entries(layoutedBlocks).forEach(([id, layoutBlock]) => {

0 commit comments

Comments
 (0)