From e73c7ed305588d62ff53a50e5d235abcecb0de59 Mon Sep 17 00:00:00 2001
From: Guilhem Lemouel <guilhemlemouel@gmail.com>
Date: Wed, 11 Mar 2026 18:50:33 +0100
Subject: [PATCH] fix: compute group bounding box using container descendants
 from layout

Thread a shared containerDescendants map through layoutLevel recursion
instead of re-running detectGroups in a separate pass. This ensures
group overlays properly encompass container modules (branchall, branchone,
forloop, whileloop) in width by including their synthetic nodes.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 .../lib/components/graph/FlowGraphV2.svelte   |  5 +-
 .../lib/components/graph/GroupOverlay.svelte  | 87 +++++++++++--------
 .../lib/components/graph/compoundLayout.ts    | 25 +++++-
 frontend/src/lib/components/graph/util.ts     | 38 +++++++-
 4 files changed, 111 insertions(+), 44 deletions(-)

diff --git a/frontend/src/lib/components/graph/FlowGraphV2.svelte b/frontend/src/lib/components/graph/FlowGraphV2.svelte
index 15851b812c..cb63777683 100644
--- a/frontend/src/lib/components/graph/FlowGraphV2.svelte
+++ b/frontend/src/lib/components/graph/FlowGraphV2.svelte
@@ -349,6 +349,7 @@
 	}
 	type NodePos = { position: { x: number; y: number } }
 	let lastNodes: [NodeDep[], (NodeDep & NodePos)[]] | undefined = undefined
+	let currentContainerDescendants: Map<string, string[]> = new Map()
 
 	function layoutNodes(nodes: NodeDep[]): (NodeDep & NodePos)[] {
 		let lastResult = lastNodes?.[1]
@@ -366,7 +367,7 @@
 		}
 
 		// Run recursive compound layout
-		const { positions, bbox } = compoundLayout(nodes, {
+		const { positions, bbox, containerDescendants: layoutContainerDescendants } = compoundLayout(nodes, {
 			nodeWidth: NODE.width,
 			nodeHeight: NODE.height,
 			gapH: NODE.gap.horizontal,
@@ -383,6 +384,7 @@
 			}
 		}))
 
+		currentContainerDescendants = layoutContainerDescendants ?? new Map()
 		lastNodes = [nodes, newNodes]
 		return newNodes
 	}
@@ -1111,6 +1113,7 @@
 					allNodes={nodesWithOffset as (Node & { type: string })[]}
 					{editMode}
 					{showNotes}
+					containerDescendants={currentContainerDescendants}
 				/>
 
 				<!-- SelectionTool for handling selection changes and filtering -->
diff --git a/frontend/src/lib/components/graph/GroupOverlay.svelte b/frontend/src/lib/components/graph/GroupOverlay.svelte
index 4083448614..af9c2c2338 100644
--- a/frontend/src/lib/components/graph/GroupOverlay.svelte
+++ b/frontend/src/lib/components/graph/GroupOverlay.svelte
@@ -1,6 +1,6 @@
 <script lang="ts">
 	import { ViewportPortal, type Node } from '@xyflow/svelte'
-	import { calculateNodesBoundsWithOffset } from './util'
+	import { calculateNodesBoundsWithOffset, expandWithContainerDescendants } from './util'
 	import { getGroupEditorContext, GROUP_HEADER_HEIGHT, GROUP_TOP_MARGIN, type FlowGroup } from './groupEditor.svelte'
 	import { NoteColor, NOTE_COLORS } from './noteColors'
 	import GroupActionBar from './GroupActionBar.svelte'
@@ -13,9 +13,10 @@
 		allNodes: (Node & { type: string })[]
 		editMode: boolean
 		showNotes: boolean
+		containerDescendants: Map<string, string[]>
 	}
 
-	let { hoveredNodeId, allNodes, editMode, showNotes }: Props = $props()
+	let { hoveredNodeId, allNodes, editMode, showNotes, containerDescendants }: Props = $props()
 
 	const groupEditorContext = getGroupEditorContext()
 
@@ -66,40 +67,52 @@
 		return showNotes ? (noteHeights[groupId] ?? 0) : 0
 	}
 
-	// Compute bounds for each group (no header card when expanded)
-	function computeGroupBounds(group: FlowGroup) {
-		if (group.module_ids.length === 0) return null
-		const { minX, minY, maxX, maxY } = calculateNodesBoundsWithOffset(group.module_ids, allNodes)
-		const padding = 16
-		const noteHeight = getGroupNoteHeight(group.id)
-		const topPadding = GROUP_HEADER_HEIGHT + noteHeight + GROUP_TOP_MARGIN
-		const halfHeader = GROUP_HEADER_HEIGHT / 2
-		return {
-			x: minX - padding,
-			y: minY - topPadding + halfHeader,
-			width: maxX - minX + 2 * padding,
-			height: maxY - minY + topPadding - halfHeader + padding,
-			headerY: minY - topPadding
+	// Pre-compute bounds for all groups reactively (tracks allNodes measured changes)
+	let groupBoundsMap = $derived.by(() => {
+		const map: Record<string, { x: number; y: number; width: number; height: number; headerY: number } | null> = {}
+		for (const group of allGroups) {
+			if (group.module_ids.length === 0) {
+				map[group.id] = null
+				continue
+			}
+			if (groupEditorContext?.groupEditor.isRuntimeCollapsed(group.id)) {
+				// Collapsed group bounds
+				const nodeId = `collapsed-group:${group.id}`
+				const node = allNodes.find((n) => n.id === nodeId)
+				if (!node) {
+					map[group.id] = null
+					continue
+				}
+				const width = node.measured?.width ?? 275
+				const nodeHeight = node.measured?.height ?? 34
+				const noteHeight = getGroupNoteHeight(group.id)
+				const headerTotal = GROUP_HEADER_HEIGHT + noteHeight
+				map[group.id] = {
+					x: node.position.x,
+					y: node.position.y - noteHeight,
+					width,
+					height: nodeHeight + noteHeight,
+					headerY: node.position.y - headerTotal
+				}
+			} else {
+				// Expanded group bounds — expand module_ids to include container descendants
+				const expandedIds = expandWithContainerDescendants(group.module_ids, containerDescendants)
+				const { minX, minY, maxX, maxY } = calculateNodesBoundsWithOffset(expandedIds, allNodes)
+				const padding = 16
+				const noteHeight = getGroupNoteHeight(group.id)
+				const topPadding = GROUP_HEADER_HEIGHT + noteHeight + GROUP_TOP_MARGIN
+				const halfHeader = GROUP_HEADER_HEIGHT / 2
+				map[group.id] = {
+					x: minX - padding,
+					y: minY - topPadding + halfHeader,
+					width: maxX - minX + 2 * padding,
+					height: maxY - minY + topPadding - halfHeader + padding,
+					headerY: minY - topPadding
+				}
+			}
 		}
-	}
-
-	// Compute bounds for a collapsed group (tight around the single collapsed node)
-	function computeCollapsedGroupBounds(group: FlowGroup) {
-		const nodeId = `collapsed-group:${group.id}`
-		const node = allNodes.find((n) => n.id === nodeId)
-		if (!node) return null
-		const width = node.measured?.width ?? 275
-		const nodeHeight = node.measured?.height ?? 34
-		const noteHeight = getGroupNoteHeight(group.id)
-		const headerTotal = GROUP_HEADER_HEIGHT + noteHeight
-		return {
-			x: node.position.x,
-			y: node.position.y - noteHeight,
-			width,
-			height: nodeHeight + noteHeight,
-			headerY: node.position.y - headerTotal
-		}
-	}
+		return map
+	})
 
 	function getOutlineColorClass(color?: string, hovered?: boolean): string {
 		const config = NOTE_COLORS[(color as NoteColor) ?? NoteColor.BLUE] ?? NOTE_COLORS[NoteColor.BLUE]
@@ -152,7 +165,7 @@
 
 {#each allGroups as group (group.id)}
 	{#if groupEditorContext?.groupEditor.isRuntimeCollapsed(group.id)}
-		{@const bounds = computeCollapsedGroupBounds(group)}
+		{@const bounds = groupBoundsMap[group.id]}
 		{#if bounds}
 			<!-- Collapsed: bounding box background + border on sides/bottom only (behind nodes) -->
 			<ViewportPortal target="back">
@@ -220,7 +233,7 @@
 			</ViewportPortal>
 		{/if}
 	{:else}
-		{@const bounds = computeGroupBounds(group)}
+		{@const bounds = groupBoundsMap[group.id]}
 		{#if bounds}
 			<!-- Uncollapsed: bounding box background + outline (behind nodes) -->
 			<ViewportPortal target="back">
diff --git a/frontend/src/lib/components/graph/compoundLayout.ts b/frontend/src/lib/components/graph/compoundLayout.ts
index 4bf3ddebda..f793d4ce26 100644
--- a/frontend/src/lib/components/graph/compoundLayout.ts
+++ b/frontend/src/lib/components/graph/compoundLayout.ts
@@ -27,6 +27,7 @@ type LayoutResult = {
 	positions: Map<string, { x: number; y: number }>
 	bbox: { width: number; height: number }
 	contentMinX: number
+	containerDescendants?: Map<string, string[]>
 }
 
 const LOOP_INDENT = 25
@@ -237,6 +238,7 @@ function layoutLevel(
 	allNodes: Map<string, LayoutNode>,
 	constants: LayoutConstants,
 	childrenMap: Map<string, string[]>,
+	containerDescendants: Map<string, string[]>,
 	depth: number = 0
 ): LayoutResult {
 	const positions = new Map<string, { x: number; y: number }>()
@@ -296,6 +298,15 @@ function layoutLevel(
 		}
 	}
 
+	// Populate containerDescendants for each top-level container group
+	for (const group of topLevelGroups) {
+		const owned: string[] = [group.endId]
+		for (const branch of group.branches) {
+			owned.push(branch.labelId, ...branch.innerIds)
+		}
+		containerDescendants.set(group.headId, owned)
+	}
+
 	// Step 2-3: Recursively lay out each group and compute wrapper sizes
 	type GroupLayout = {
 		group: CompoundGroup
@@ -322,7 +333,14 @@ function layoutLevel(
 			const branchNodeIds = [branch.labelId, ...branch.innerIds]
 
 			// Find sub-groups within this branch
-			const result = layoutLevel(branchNodeIds, allNodes, constants, childrenMap, depth + 1)
+			const result = layoutLevel(
+				branchNodeIds,
+				allNodes,
+				constants,
+				childrenMap,
+				containerDescendants,
+				depth + 1
+			)
 
 			branchLayouts.push({
 				labelId: branch.labelId,
@@ -528,7 +546,8 @@ export function compoundLayout(
 	}
 
 	const nodeIds = nodes.map((n) => n.id)
-	const result = layoutLevel(nodeIds, allNodes, c, childrenMap)
+	const containerDescendants = new Map<string, string[]>()
+	const result = layoutLevel(nodeIds, allNodes, c, childrenMap, containerDescendants)
 
 	// Shift positions so minX=0 (left-aligned).
 	// FlowGraphV2 centers with: xCenter = viewport/2 - bbox.width/2
@@ -551,5 +570,5 @@ export function compoundLayout(
 		)
 	}
 
-	return result
+	return { ...result, containerDescendants }
 }
diff --git a/frontend/src/lib/components/graph/util.ts b/frontend/src/lib/components/graph/util.ts
index 08772ce247..eb825ab7e2 100644
--- a/frontend/src/lib/components/graph/util.ts
+++ b/frontend/src/lib/components/graph/util.ts
@@ -227,6 +227,7 @@ export function calculateNodesBoundsWithOffset(
 		id: string
 		position: { x: number; y: number }
 		type: string
+		measured?: { width?: number; height?: number }
 	}>
 ): {
 	minX: number
@@ -239,11 +240,13 @@ export function calculateNodesBoundsWithOffset(
 
 	return nodesToCalculate.reduce(
 		(acc, node) => {
+			const w = node.measured?.width ?? NODE.width
+			const h = node.measured?.height ?? NODE.height
 			return {
 				minX: Math.min(acc.minX, node.position.x),
 				minY: Math.min(acc.minY, node.position.y),
-				maxX: Math.max(acc.maxX, node.position.x + NODE.width),
-				maxY: Math.max(acc.maxY, node.position.y + NODE.height)
+				maxX: Math.max(acc.maxX, node.position.x + w),
+				maxY: Math.max(acc.maxY, node.position.y + h)
 			}
 		},
 		{
@@ -255,6 +258,33 @@ export function calculateNodesBoundsWithOffset(
 	)
 }
 
+/**
+ * Expand a list of node IDs to include all structurally-owned descendants of containers.
+ * Uses the containerDescendants map (container head → owned node IDs) built by compoundLayout,
+ * which correctly bounds traversal at container end nodes (unlike the flow-topology childrenMap
+ * which would leak past container boundaries into downstream nodes).
+ */
+export function expandWithContainerDescendants(
+	nodeIds: string[],
+	containerDescendants: Map<string, string[]>
+): string[] {
+	const result = new Set<string>(nodeIds)
+	const queue = [...nodeIds]
+	let qi = 0
+	while (qi < queue.length) {
+		const current = queue[qi++]
+		const owned = containerDescendants.get(current)
+		if (!owned) continue
+		for (const id of owned) {
+			if (!result.has(id)) {
+				result.add(id)
+				queue.push(id)
+			}
+		}
+	}
+	return Array.from(result)
+}
+
 /**
  * Find all nodes related to the given node IDs, including expanded subflow nodes
  * @param targetNodeIds - Array of node IDs to find related nodes for
@@ -267,17 +297,19 @@ function getAllRelatedSubflowNodes(
 		id: string
 		position: { x: number; y: number }
 		type: string
+		measured?: { width?: number; height?: number }
 	}>
 ): Array<{
 	id: string
 	position: { x: number; y: number }
+	measured?: { width?: number; height?: number }
 }> {
 	const relatedNodeIds = new Set<string>()
 
 	// Add original target nodes
 	targetNodeIds.forEach((id) => relatedNodeIds.add(id))
 
-	// For each target node, check if it's a subflow and find expanded nodes
+	// For each target node, check if it's a subflow or container and find child nodes
 	targetNodeIds.forEach((nodeId) => {
 		// Find nodes like "subflow:{nodeId}:*"
 		const subflowNodes = allNodes.filter(