Skip to content

Commit 05022e3

Browse files
fix(copilot-autolayout): more subflow cases and deal with resizing (#2236)
* fix sidebar hydration issue accurately * improve autolayout subflow cases * fix DOM structure to prevent HMR hydration issues
1 parent 9f884c1 commit 05022e3

File tree

13 files changed

+288
-82
lines changed

13 files changed

+288
-82
lines changed

apps/sim/app/api/workflows/[id]/route.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,23 @@ export async function GET(request: NextRequest, { params }: { params: Promise<{
152152

153153
return NextResponse.json({ data: finalWorkflowData }, { status: 200 })
154154
}
155-
return NextResponse.json({ error: 'Workflow has no normalized data' }, { status: 400 })
155+
156+
const emptyWorkflowData = {
157+
...workflowData,
158+
state: {
159+
deploymentStatuses: {},
160+
blocks: {},
161+
edges: [],
162+
loops: {},
163+
parallels: {},
164+
lastSaved: Date.now(),
165+
isDeployed: workflowData.isDeployed || false,
166+
deployedAt: workflowData.deployedAt,
167+
},
168+
variables: workflowData.variables || {},
169+
}
170+
171+
return NextResponse.json({ data: emptyWorkflowData }, { status: 200 })
156172
} catch (error: any) {
157173
const elapsed = Date.now() - startTime
158174
logger.error(`[${requestId}] Error fetching workflow ${workflowId} after ${elapsed}ms`, error)

apps/sim/app/workspace/[workspaceId]/layout.tsx

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,14 @@ export default function WorkspaceLayout({ children }: { children: React.ReactNod
1414
<ProviderModelsLoader />
1515
<GlobalCommandsProvider>
1616
<Tooltip.Provider delayDuration={600} skipDelayDuration={0}>
17-
<WorkspacePermissionsProvider>
18-
<div className='flex min-h-screen w-full'>
19-
<SidebarNew />
20-
<div className='flex flex-1 flex-col'>{children}</div>
21-
</div>
22-
</WorkspacePermissionsProvider>
17+
<div className='flex min-h-screen w-full'>
18+
<WorkspacePermissionsProvider>
19+
<div className='shrink-0' suppressHydrationWarning>
20+
<SidebarNew />
21+
</div>
22+
{children}
23+
</WorkspacePermissionsProvider>
24+
</div>
2325
</Tooltip.Provider>
2426
</GlobalCommandsProvider>
2527
</>
Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
11
export default function TemplatesLayout({ children }: { children: React.ReactNode }) {
2-
return <div>{children}</div>
2+
return (
3+
<main className='flex flex-1 flex-col h-full overflow-hidden bg-muted/40'>
4+
<div>{children}</div>
5+
</main>
6+
)
37
}

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/layout.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { ErrorBoundary } from '@/app/workspace/[workspaceId]/w/[workflowId]/comp
22

33
export default function WorkflowLayout({ children }: { children: React.ReactNode }) {
44
return (
5-
<main className='h-full overflow-hidden bg-muted/40'>
5+
<main className='flex flex-1 flex-col h-full overflow-hidden bg-muted/40'>
66
<ErrorBoundary>{children}</ErrorBoundary>
77
</main>
88
)

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1295,14 +1295,18 @@ const WorkflowContent = React.memo(() => {
12951295
return
12961296
}
12971297

1298+
// Check if we encountered an error loading this specific workflow to prevent infinite retries
1299+
const hasLoadError = hydration.phase === 'error' && hydration.workflowId === currentId
1300+
12981301
// Check if we need to load the workflow state:
12991302
// 1. Different workflow than currently active
13001303
// 2. Same workflow but hydration phase is not 'ready' (e.g., after a quick refresh)
13011304
const needsWorkflowLoad =
1302-
activeWorkflowId !== currentId ||
1303-
(activeWorkflowId === currentId &&
1304-
hydration.phase !== 'ready' &&
1305-
hydration.phase !== 'state-loading')
1305+
!hasLoadError &&
1306+
(activeWorkflowId !== currentId ||
1307+
(activeWorkflowId === currentId &&
1308+
hydration.phase !== 'ready' &&
1309+
hydration.phase !== 'state-loading'))
13061310

13071311
if (needsWorkflowLoad) {
13081312
const { clearDiff } = useWorkflowDiffStore.getState()

apps/sim/app/workspace/[workspaceId]/w/components/sidebar/sidebar-new.tsx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,15 @@ export function SidebarNew() {
6060
// Session data
6161
const { data: sessionData, isPending: sessionLoading } = useSession()
6262

63-
// Sidebar state
64-
const isCollapsed = useSidebarStore((state) => state.isCollapsed)
63+
// Sidebar state - use store's hydration tracking to prevent SSR mismatch
64+
const hasHydrated = useSidebarStore((state) => state._hasHydrated)
65+
const isCollapsedStore = useSidebarStore((state) => state.isCollapsed)
6566
const setIsCollapsed = useSidebarStore((state) => state.setIsCollapsed)
6667
const setSidebarWidth = useSidebarStore((state) => state.setSidebarWidth)
6768

69+
// Use default (expanded) state until hydrated to prevent hydration mismatch
70+
const isCollapsed = hasHydrated ? isCollapsedStore : false
71+
6872
// Determine if we're on a workflow page (only workflow pages allow collapse and resize)
6973
const isOnWorkflowPage = !!workflowId
7074

apps/sim/app/workspace/[workspaceId]/w/page.tsx

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use client'
22

3-
import { useEffect, useState } from 'react'
3+
import { useEffect } from 'react'
44
import { Loader2 } from 'lucide-react'
55
import { useParams, useRouter } from 'next/navigation'
66
import { createLogger } from '@/lib/logs/console/logger'
@@ -14,21 +14,12 @@ export default function WorkflowsPage() {
1414
const { workflows, setActiveWorkflow } = useWorkflowRegistry()
1515
const params = useParams()
1616
const workspaceId = params.workspaceId as string
17-
const [isMounted, setIsMounted] = useState(false)
1817

1918
// Fetch workflows using React Query
2019
const { isLoading, isError } = useWorkflows(workspaceId)
2120

22-
// Track when component is mounted to avoid hydration issues
21+
// Handle redirection once workflows are loaded
2322
useEffect(() => {
24-
setIsMounted(true)
25-
}, [])
26-
27-
// Handle redirection once workflows are loaded and component is mounted
28-
useEffect(() => {
29-
// Wait for component to be mounted to avoid hydration mismatches
30-
if (!isMounted) return
31-
3223
// Only proceed if workflows are done loading
3324
if (isLoading) return
3425

@@ -50,17 +41,19 @@ export default function WorkflowsPage() {
5041
const firstWorkflowId = workspaceWorkflows[0]
5142
router.replace(`/workspace/${workspaceId}/w/${firstWorkflowId}`)
5243
}
53-
}, [isMounted, isLoading, workflows, workspaceId, router, setActiveWorkflow, isError])
44+
}, [isLoading, workflows, workspaceId, router, setActiveWorkflow, isError])
5445

5546
// Always show loading state until redirect happens
5647
// There should always be a default workflow, so we never show "no workflows found"
5748
return (
58-
<div className='flex h-screen items-center justify-center'>
59-
<div className='text-center'>
60-
<div className='mx-auto mb-4'>
61-
<Loader2 className='h-8 w-8 animate-spin text-muted-foreground' />
49+
<main className='flex flex-1 flex-col h-full overflow-hidden bg-muted/40'>
50+
<div className='flex h-full items-center justify-center'>
51+
<div className='text-center'>
52+
<div className='mx-auto mb-4'>
53+
<Loader2 className='h-8 w-8 animate-spin text-muted-foreground' />
54+
</div>
6255
</div>
6356
</div>
64-
</div>
57+
</main>
6558
)
6659
}

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

Lines changed: 51 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,9 @@ import {
33
CONTAINER_LAYOUT_OPTIONS,
44
DEFAULT_LAYOUT_OPTIONS,
55
MAX_OVERLAP_ITERATIONS,
6-
OVERLAP_MARGIN,
76
} from '@/lib/workflows/autolayout/constants'
87
import type { Edge, GraphNode, LayoutOptions } from '@/lib/workflows/autolayout/types'
98
import {
10-
boxesOverlap,
11-
createBoundingBox,
129
getBlockMetrics,
1310
normalizePositions,
1411
prepareBlockMetrics,
@@ -172,54 +169,48 @@ export function groupByLayer(nodes: Map<string, GraphNode>): Map<number, GraphNo
172169
}
173170

174171
/**
175-
* Resolves overlaps between all nodes, including across layers.
176-
* Nodes in the same layer are shifted vertically to avoid overlap.
177-
* Nodes in different layers that overlap are shifted down.
172+
* Resolves vertical overlaps between nodes in the same layer.
173+
* X overlaps are prevented by construction via cumulative width-based positioning.
178174
*/
179-
function resolveOverlaps(nodes: GraphNode[], verticalSpacing: number): void {
175+
function resolveVerticalOverlaps(nodes: GraphNode[], verticalSpacing: number): void {
180176
let iteration = 0
181177
let hasOverlap = true
182178

183179
while (hasOverlap && iteration < MAX_OVERLAP_ITERATIONS) {
184180
hasOverlap = false
185181
iteration++
186182

187-
// Sort nodes by layer then by Y position for consistent processing
188-
const sortedNodes = [...nodes].sort((a, b) => {
189-
if (a.layer !== b.layer) return a.layer - b.layer
190-
return a.position.y - b.position.y
191-
})
183+
// Group nodes by layer for same-layer overlap resolution
184+
const nodesByLayer = new Map<number, GraphNode[]>()
185+
for (const node of nodes) {
186+
if (!nodesByLayer.has(node.layer)) {
187+
nodesByLayer.set(node.layer, [])
188+
}
189+
nodesByLayer.get(node.layer)!.push(node)
190+
}
192191

193-
for (let i = 0; i < sortedNodes.length; i++) {
194-
for (let j = i + 1; j < sortedNodes.length; j++) {
195-
const node1 = sortedNodes[i]
196-
const node2 = sortedNodes[j]
192+
// Process each layer independently
193+
for (const [layer, layerNodes] of nodesByLayer) {
194+
if (layerNodes.length < 2) continue
197195

198-
const box1 = createBoundingBox(node1.position, node1.metrics)
199-
const box2 = createBoundingBox(node2.position, node2.metrics)
196+
// Sort by Y position for consistent processing
197+
layerNodes.sort((a, b) => a.position.y - b.position.y)
200198

201-
// Check for overlap with margin
202-
if (boxesOverlap(box1, box2, OVERLAP_MARGIN)) {
203-
hasOverlap = true
199+
for (let i = 0; i < layerNodes.length - 1; i++) {
200+
const node1 = layerNodes[i]
201+
const node2 = layerNodes[i + 1]
204202

205-
// If in same layer, shift vertically around midpoint
206-
if (node1.layer === node2.layer) {
207-
const midpoint = (node1.position.y + node2.position.y) / 2
208-
209-
node1.position.y = midpoint - node1.metrics.height / 2 - verticalSpacing / 2
210-
node2.position.y = midpoint + node2.metrics.height / 2 + verticalSpacing / 2
211-
} else {
212-
// Different layers - shift the later one down
213-
const requiredSpace = box1.y + box1.height + verticalSpacing
214-
if (node2.position.y < requiredSpace) {
215-
node2.position.y = requiredSpace
216-
}
217-
}
203+
const node1Bottom = node1.position.y + node1.metrics.height
204+
const requiredY = node1Bottom + verticalSpacing
205+
206+
if (node2.position.y < requiredY) {
207+
hasOverlap = true
208+
node2.position.y = requiredY
218209

219-
logger.debug('Resolved overlap between blocks', {
210+
logger.debug('Resolved vertical overlap in layer', {
211+
layer,
220212
block1: node1.id,
221213
block2: node2.id,
222-
sameLayer: node1.layer === node2.layer,
223214
iteration,
224215
})
225216
}
@@ -228,14 +219,15 @@ function resolveOverlaps(nodes: GraphNode[], verticalSpacing: number): void {
228219
}
229220

230221
if (hasOverlap) {
231-
logger.warn('Could not fully resolve all overlaps after max iterations', {
222+
logger.warn('Could not fully resolve all vertical overlaps after max iterations', {
232223
iterations: MAX_OVERLAP_ITERATIONS,
233224
})
234225
}
235226
}
236227

237228
/**
238-
* Calculates positions for nodes organized by layer
229+
* Calculates positions for nodes organized by layer.
230+
* Uses cumulative width-based X positioning to properly handle containers of varying widths.
239231
*/
240232
export function calculatePositions(
241233
layers: Map<number, GraphNode[]>,
@@ -248,9 +240,27 @@ export function calculatePositions(
248240

249241
const layerNumbers = Array.from(layers.keys()).sort((a, b) => a - b)
250242

243+
// Calculate max width for each layer
244+
const layerWidths = new Map<number, number>()
245+
for (const layerNum of layerNumbers) {
246+
const nodesInLayer = layers.get(layerNum)!
247+
const maxWidth = Math.max(...nodesInLayer.map((n) => n.metrics.width))
248+
layerWidths.set(layerNum, maxWidth)
249+
}
250+
251+
// Calculate cumulative X positions for each layer based on actual widths
252+
const layerXPositions = new Map<number, number>()
253+
let cumulativeX = padding.x
254+
255+
for (const layerNum of layerNumbers) {
256+
layerXPositions.set(layerNum, cumulativeX)
257+
cumulativeX += layerWidths.get(layerNum)! + horizontalSpacing
258+
}
259+
260+
// Position nodes using cumulative X
251261
for (const layerNum of layerNumbers) {
252262
const nodesInLayer = layers.get(layerNum)!
253-
const xPosition = padding.x + layerNum * horizontalSpacing
263+
const xPosition = layerXPositions.get(layerNum)!
254264

255265
// Calculate total height for this layer
256266
const totalHeight = nodesInLayer.reduce(
@@ -285,8 +295,8 @@ export function calculatePositions(
285295
}
286296
}
287297

288-
// Resolve overlaps across all nodes
289-
resolveOverlaps(Array.from(layers.values()).flat(), verticalSpacing)
298+
// Resolve vertical overlaps within layers (X overlaps prevented by cumulative positioning)
299+
resolveVerticalOverlaps(Array.from(layers.values()).flat(), verticalSpacing)
290300
}
291301

292302
/**

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,16 @@
11
import { createLogger } from '@/lib/logs/console/logger'
2+
import {
3+
DEFAULT_HORIZONTAL_SPACING,
4+
DEFAULT_VERTICAL_SPACING,
5+
} from '@/lib/workflows/autolayout/constants'
26
import { layoutContainers } from '@/lib/workflows/autolayout/containers'
37
import { assignLayers, layoutBlocksCore } from '@/lib/workflows/autolayout/core'
48
import type { Edge, LayoutOptions, LayoutResult } from '@/lib/workflows/autolayout/types'
59
import {
610
calculateSubflowDepths,
711
filterLayoutEligibleBlockIds,
812
getBlocksByParent,
13+
prepareContainerDimensions,
914
} from '@/lib/workflows/autolayout/utils'
1015
import type { BlockState } from '@/stores/workflows/workflow/types'
1116

@@ -28,6 +33,19 @@ export function applyAutoLayout(
2833

2934
const blocksCopy: Record<string, BlockState> = JSON.parse(JSON.stringify(blocks))
3035

36+
const horizontalSpacing = options.horizontalSpacing ?? DEFAULT_HORIZONTAL_SPACING
37+
const verticalSpacing = options.verticalSpacing ?? DEFAULT_VERTICAL_SPACING
38+
39+
// Pre-calculate container dimensions by laying out their children (bottom-up)
40+
// This ensures accurate widths/heights before root-level layout
41+
prepareContainerDimensions(
42+
blocksCopy,
43+
edges,
44+
layoutBlocksCore,
45+
horizontalSpacing,
46+
verticalSpacing
47+
)
48+
3149
const { root: rootBlockIds } = getBlocksByParent(blocksCopy)
3250
const layoutRootIds = filterLayoutEligibleBlockIds(rootBlockIds, blocksCopy)
3351

0 commit comments

Comments
 (0)