From 7a2c0a452b00fcb90fbc5c01a425f0e94bc5aef0 Mon Sep 17 00:00:00 2001 From: Guilhem Date: Wed, 4 Mar 2026 14:34:05 +0000 Subject: [PATCH] fix(frontend): address PR review feedback on flow layout - Add max recursion depth guard (50) to layoutLevel to prevent stack overflow with malformed flow data - Log swallowed decrossOpt error as console.debug for debuggability - Initialize maxY to -Infinity for correctness with negative positions - Fix indentation artifacts in graphBuilder data objects Co-Authored-By: Claude Opus 4.6 --- .../lib/components/graph/compoundLayout.ts | 25 ++++++++++++++++--- .../components/graph/graphBuilder.svelte.ts | 10 ++++---- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/frontend/src/lib/components/graph/compoundLayout.ts b/frontend/src/lib/components/graph/compoundLayout.ts index 25928649d6..1c294b06de 100644 --- a/frontend/src/lib/components/graph/compoundLayout.ts +++ b/frontend/src/lib/components/graph/compoundLayout.ts @@ -166,7 +166,8 @@ function runSugiyama( return [w + constants.gapH, h + constants.gapV] as readonly [number, number] }) boxSize = layout(dag as any) as any - } catch { + } catch (e) { + console.debug('[compoundLayout] decrossOpt failed, falling back to decrossTwoLayer:', e) const layout = sugiyama() .decross(decrossTwoLayer()) .coord(coordCenter()) @@ -229,11 +230,14 @@ function runSugiyama( * 5. Run sugiyama on the simplified graph * 6. Expand wrapper positions back to absolute positions */ +const MAX_RECURSION_DEPTH = 50 + function layoutLevel( nodeIds: string[], allNodes: Map, constants: LayoutConstants, - childrenMap: Map + childrenMap: Map, + depth: number = 0 ): LayoutResult { const positions = new Map() const nodeIdSet = new Set(nodeIds) @@ -246,6 +250,19 @@ function layoutLevel( } } + if (depth >= MAX_RECURSION_DEPTH) { + console.warn('[compoundLayout] Max recursion depth reached, falling back to flat layout') + const flatNodes = nodeIds.map((id) => { + const n = allNodes.get(id)! + return { id, parentIds: (n.parentIds ?? []).filter((pid) => nodeIdSet.has(pid)) } + }) + const result = runSugiyama(flatNodes, constants) + for (const [id, pos] of result.positions) { + positions.set(id, pos) + } + return { positions, bbox: { width: result.width, height: result.height }, contentMinX: 0 } + } + // Step 1: detect compound groups at this level const groups = detectGroups(nodeIdSet, allNodes, childrenMap) @@ -305,7 +322,7 @@ function layoutLevel( const branchNodeIds = [branch.labelId, ...branch.innerIds] // Find sub-groups within this branch - const result = layoutLevel(branchNodeIds, allNodes, constants, childrenMap) + const result = layoutLevel(branchNodeIds, allNodes, constants, childrenMap, depth + 1) branchLayouts.push({ labelId: branch.labelId, @@ -453,7 +470,7 @@ function layoutLevel( let minX = Infinity let maxX = -Infinity let minY = Infinity - let maxY = 0 + let maxY = -Infinity for (const pos of positions.values()) { minX = Math.min(minX, pos.x - constants.nodeWidth / 2) maxX = Math.max(maxX, pos.x + constants.nodeWidth / 2) diff --git a/frontend/src/lib/components/graph/graphBuilder.svelte.ts b/frontend/src/lib/components/graph/graphBuilder.svelte.ts index ee45c4dc95..a0fa84bb48 100644 --- a/frontend/src/lib/components/graph/graphBuilder.svelte.ts +++ b/frontend/src/lib/components/graph/graphBuilder.svelte.ts @@ -650,7 +650,7 @@ export function graphBuilder( const startNode: NodeLayout = { id: `${module.id}-branch-0`, data: { - id: module.id, + id: module.id, branchIndex: -1, eventHandlers: eventHandlers, flowModuleState: extra.flowModuleStates?.[module.id], @@ -675,7 +675,7 @@ export function graphBuilder( const startNode: NodeLayout = { id: `${module.id}-branch-${branchIndex}`, data: { - label: defaultIfEmptyString(branch.summary, `Branch ${branchIndex + 1}`), + label: defaultIfEmptyString(branch.summary, `Branch ${branchIndex + 1}`), id: module.id, branchIndex: branchIndex, eventHandlers: eventHandlers, @@ -878,7 +878,7 @@ export function graphBuilder( const startNode: NodeLayout = { id: `${module.id}-branch-${branchIndex}`, data: { - label: defaultIfEmptyString(branch.summary, 'Branch ' + (branchIndex + 1)), + label: defaultIfEmptyString(branch.summary, 'Branch ' + (branchIndex + 1)), preLabel: branch.summary ? '' : branch.expr, id: module.id, branchIndex: branchIndex, @@ -921,7 +921,7 @@ export function graphBuilder( const startNode: NodeLayout = { id: startId, data: { - label: `Start of subflow ${idWithoutPrefix}`, + label: `Start of subflow ${idWithoutPrefix}`, id: startId, subflowId: module.id, eventHandlers: eventHandlers, @@ -949,7 +949,7 @@ export function graphBuilder( const endNode: NodeLayout = { id: endId, data: { - label: `End of subflow ${idWithoutPrefix}`, + label: `End of subflow ${idWithoutPrefix}`, id: endId, subflowId: module.id, eventHandlers: eventHandlers,