Skip to content

Commit c7604c7

Browse files
icecrasher321icecrasher321
committed
address comments
1 parent 3dff617 commit c7604c7

File tree

3 files changed

+249
-3
lines changed

3 files changed

+249
-3
lines changed

‎apps/sim/lib/workflows/autolayout/change-set.test.ts‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,31 @@ describe('getTargetedLayoutChangeSet', () => {
8181
expect(getTargetedLayoutChangeSet({ before, after })).toEqual([])
8282
})
8383

84+
it('does not relayout a pre-existing block legitimately placed at the origin', () => {
85+
const before = createWorkflowState({
86+
blocks: {
87+
start: createBlock('start', { position: { x: 0, y: 0 } }),
88+
},
89+
})
90+
91+
const after = createWorkflowState({
92+
blocks: {
93+
start: createBlock('start', {
94+
position: { x: 0, y: 0 },
95+
subBlocks: {
96+
prompt: {
97+
id: 'prompt',
98+
type: 'long-input',
99+
value: 'updated',
100+
},
101+
},
102+
}),
103+
},
104+
})
105+
106+
expect(getTargetedLayoutChangeSet({ before, after })).toEqual([])
107+
})
108+
84109
it('reopens only the downstream path when an edge is added later', () => {
85110
const before = createWorkflowState({
86111
blocks: {
@@ -129,6 +154,70 @@ describe('getTargetedLayoutChangeSet', () => {
129154
})
130155
})
131156

157+
it('returns a pure shift source when a stable block gains an edge to an already-connected target', () => {
158+
const before = createWorkflowState({
159+
blocks: {
160+
source: createBlock('source', { position: { x: 100, y: 100 } }),
161+
upstream: createBlock('upstream', { position: { x: 100, y: 300 } }),
162+
target: createBlock('target', { position: { x: 400, y: 100 } }),
163+
end: createBlock('end', { position: { x: 700, y: 100 } }),
164+
},
165+
edges: [
166+
{
167+
id: 'edge-1',
168+
source: 'upstream',
169+
target: 'target',
170+
sourceHandle: 'source',
171+
targetHandle: 'target',
172+
},
173+
{
174+
id: 'edge-2',
175+
source: 'target',
176+
target: 'end',
177+
sourceHandle: 'source',
178+
targetHandle: 'target',
179+
},
180+
],
181+
})
182+
183+
const after = createWorkflowState({
184+
blocks: {
185+
source: createBlock('source', { position: { x: 100, y: 100 } }),
186+
upstream: createBlock('upstream', { position: { x: 100, y: 300 } }),
187+
target: createBlock('target', { position: { x: 400, y: 100 } }),
188+
end: createBlock('end', { position: { x: 700, y: 100 } }),
189+
},
190+
edges: [
191+
{
192+
id: 'edge-1',
193+
source: 'upstream',
194+
target: 'target',
195+
sourceHandle: 'source',
196+
targetHandle: 'target',
197+
},
198+
{
199+
id: 'edge-2',
200+
source: 'target',
201+
target: 'end',
202+
sourceHandle: 'source',
203+
targetHandle: 'target',
204+
},
205+
{
206+
id: 'edge-3',
207+
source: 'source',
208+
target: 'target',
209+
sourceHandle: 'source',
210+
targetHandle: 'target',
211+
},
212+
],
213+
})
214+
215+
expect(getTargetedLayoutImpact({ before, after })).toEqual({
216+
layoutBlockIds: [],
217+
shiftSourceBlockIds: ['source'],
218+
})
219+
})
220+
132221
it('keeps the upstream source anchored when inserting between existing blocks', () => {
133222
const before = createWorkflowState({
134223
blocks: {

‎apps/sim/lib/workflows/autolayout/change-set.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ export function getTargetedLayoutImpact({
4141
}
4242
}
4343

44-
for (const blockId of getBlocksWithInvalidPositions(after)) {
44+
for (const blockId of getBlocksWithInvalidPositions(after, beforeBlockIds)) {
4545
layoutBlockIds.add(blockId)
4646
}
4747

@@ -96,15 +96,21 @@ export function getTargetedLayoutChangeSet(options: TargetedLayoutChangeSetOptio
9696

9797
/**
9898
* Returns block IDs that cannot be treated as stable layout anchors.
99+
* Existing blocks are only considered invalid for missing or non-finite
100+
* coordinates; `(0,0)` is reserved as a layout sentinel only for newly added
101+
* blocks and parent-change handling above.
99102
*/
100-
function getBlocksWithInvalidPositions(after: Pick<WorkflowState, 'blocks'>): string[] {
103+
function getBlocksWithInvalidPositions(
104+
after: Pick<WorkflowState, 'blocks'>,
105+
beforeBlockIds: Set<string>
106+
): string[] {
101107
return Object.keys(after.blocks || {}).filter((blockId) => {
102108
const position = after.blocks[blockId]?.position
103109
return (
104110
!position ||
105111
!Number.isFinite(position.x) ||
106112
!Number.isFinite(position.y) ||
107-
(position.x === 0 && position.y === 0)
113+
(!beforeBlockIds.has(blockId) && position.x === 0 && position.y === 0)
108114
)
109115
})
110116
}

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

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { describe, expect, it } from 'vitest'
5+
import { applyTargetedLayout } from '@/lib/workflows/autolayout/targeted'
6+
import type { Edge } from '@/lib/workflows/autolayout/types'
7+
import { getBlockMetrics } from '@/lib/workflows/autolayout/utils'
8+
import type { BlockState } from '@/stores/workflows/workflow/types'
9+
10+
function createBlock(id: string, overrides: Partial<BlockState> = {}): BlockState {
11+
return {
12+
id,
13+
type: 'function',
14+
name: id,
15+
position: { x: 0, y: 0 },
16+
subBlocks: {},
17+
outputs: {},
18+
enabled: true,
19+
...overrides,
20+
}
21+
}
22+
23+
describe('applyTargetedLayout', () => {
24+
it('shifts downstream frozen blocks when only shift sources are provided', () => {
25+
const blocks = {
26+
source: createBlock('source', {
27+
position: { x: 100, y: 100 },
28+
}),
29+
target: createBlock('target', {
30+
position: { x: 400, y: 100 },
31+
}),
32+
end: createBlock('end', {
33+
position: { x: 760, y: 100 },
34+
}),
35+
}
36+
const edges: Edge[] = [
37+
{
38+
id: 'edge-1',
39+
source: 'source',
40+
target: 'target',
41+
sourceHandle: 'source',
42+
targetHandle: 'target',
43+
},
44+
{
45+
id: 'edge-2',
46+
source: 'target',
47+
target: 'end',
48+
sourceHandle: 'source',
49+
targetHandle: 'target',
50+
},
51+
]
52+
53+
const result = applyTargetedLayout(blocks, edges, {
54+
changedBlockIds: [],
55+
shiftSourceBlockIds: ['source'],
56+
})
57+
58+
expect(result.source.position).toEqual({ x: 100, y: 100 })
59+
expect(result.target.position).toEqual({ x: 530, y: 100 })
60+
expect(result.end.position).toEqual({ x: 960, y: 100 })
61+
})
62+
63+
it('places new linear blocks without moving anchors', () => {
64+
const blocks = {
65+
anchor: createBlock('anchor', {
66+
position: { x: 150, y: 150 },
67+
}),
68+
changed: createBlock('changed', {
69+
position: { x: 0, y: 0 },
70+
}),
71+
}
72+
const edges: Edge[] = [
73+
{
74+
id: 'edge-1',
75+
source: 'anchor',
76+
target: 'changed',
77+
},
78+
]
79+
80+
const result = applyTargetedLayout(blocks, edges, {
81+
changedBlockIds: ['changed'],
82+
})
83+
84+
expect(result.anchor.position).toEqual({ x: 150, y: 150 })
85+
expect(result.changed.position.x).toBeGreaterThan(result.anchor.position.x)
86+
expect(result.changed.position.y).toBe(result.anchor.position.y)
87+
})
88+
89+
it('keeps root-level insertions closer to anchored blocks near the top of the canvas', () => {
90+
const blocks = {
91+
start: createBlock('start', {
92+
position: { x: 0, y: 0 },
93+
}),
94+
changed: createBlock('changed', {
95+
position: { x: 0, y: 0 },
96+
}),
97+
agent: createBlock('agent', {
98+
position: { x: 410.94, y: 2.33 },
99+
}),
100+
}
101+
const edges: Edge[] = [
102+
{
103+
id: 'edge-1',
104+
source: 'start',
105+
target: 'changed',
106+
},
107+
{
108+
id: 'edge-2',
109+
source: 'changed',
110+
target: 'agent',
111+
},
112+
]
113+
114+
const result = applyTargetedLayout(blocks, edges, {
115+
changedBlockIds: ['changed'],
116+
})
117+
118+
expect(result.changed.position.y).toBeLessThan(150)
119+
})
120+
121+
it('places new parallel children below tall anchored siblings', () => {
122+
const blocks = {
123+
parallel: createBlock('parallel', {
124+
type: 'parallel',
125+
position: { x: 200, y: 150 },
126+
data: { width: 600, height: 500 },
127+
layout: { measuredWidth: 600, measuredHeight: 500 },
128+
}),
129+
existing: createBlock('existing', {
130+
position: { x: 180, y: 100 },
131+
data: { parentId: 'parallel', extent: 'parent' },
132+
layout: { measuredWidth: 250, measuredHeight: 220 },
133+
height: 220,
134+
}),
135+
changed: createBlock('changed', {
136+
position: { x: 0, y: 0 },
137+
data: { parentId: 'parallel', extent: 'parent' },
138+
}),
139+
}
140+
141+
const result = applyTargetedLayout(blocks, [], {
142+
changedBlockIds: ['changed'],
143+
})
144+
145+
const existingMetrics = getBlockMetrics(result.existing)
146+
expect(result.parallel.position).toEqual({ x: 200, y: 150 })
147+
expect(result.changed.position.y).toBeGreaterThanOrEqual(
148+
result.existing.position.y + existingMetrics.height
149+
)
150+
})
151+
})

0 commit comments

Comments
 (0)