From ba599a4ea167a16679c242d746263837ce9406ae Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sat, 27 Jun 2026 15:39:01 -0700 Subject: [PATCH] FN-7136: preserve workflow edges between adjacent group nodes Keep workflow graph edges connected when optional-group containers appear consecutively. - Reject stale saved top-level layout coordinates only when connected container spans overlap in the same visual row. - Advance auto-layout layers by each consecutive container layer's rendered width. - Add coverage for desktop flow mapping, auto-layout, mobile graph rendering, and release-note changeset. Files changed: .../fn-7136-consecutive-optional-group-edges.md | 7 ++ .../__tests__/workflow-auto-layout.test.ts | 116 +++++++++++++++++++- .../__tests__/workflow-flow-mapping.test.ts | 122 ++++++++++++++++++++- .../__tests__/workflow-mobile-graph.test.ts | 10 +- .../app/components/workflow-auto-layout.ts | 3 + .../app/components/workflow-flow-mapping.ts | 50 ++++++++- 6 files changed, 299 insertions(+), 9 deletions(-) Fusion-Task-Id: FN-7136 Fusion-Task-Lineage: 1fa1e528-a68e-4eee-9018-6752fe62a1f5 Co-authored-by: Fusion (runfusion.ai) --- ...n-7136-consecutive-optional-group-edges.md | 7 + .../__tests__/workflow-auto-layout.test.ts | 116 ++++++++++++++++- .../__tests__/workflow-flow-mapping.test.ts | 122 +++++++++++++++++- .../__tests__/workflow-mobile-graph.test.ts | 10 +- .../app/components/workflow-auto-layout.ts | 3 + .../app/components/workflow-flow-mapping.ts | 50 ++++++- 6 files changed, 299 insertions(+), 9 deletions(-) create mode 100644 .changeset/fn-7136-consecutive-optional-group-edges.md diff --git a/.changeset/fn-7136-consecutive-optional-group-edges.md b/.changeset/fn-7136-consecutive-optional-group-edges.md new file mode 100644 index 0000000000..9075faab2f --- /dev/null +++ b/.changeset/fn-7136-consecutive-optional-group-edges.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Fix workflow view so the Code Review and Browser Verification blocks show connected edges. +category: fix +dev: Auto-layout/fallback spacing now advances by every consecutive container node's rendered width so back-to-back optional-group/foreach/loop nodes no longer overlap adjacent handles; covered by a consecutive-container connectivity regression test. diff --git a/packages/dashboard/app/components/__tests__/workflow-auto-layout.test.ts b/packages/dashboard/app/components/__tests__/workflow-auto-layout.test.ts index 2c1ad86765..32a6bacbbc 100644 --- a/packages/dashboard/app/components/__tests__/workflow-auto-layout.test.ts +++ b/packages/dashboard/app/components/__tests__/workflow-auto-layout.test.ts @@ -1,8 +1,9 @@ import { describe, it, expect } from "vitest"; import type { Node as FlowNode, Edge as FlowEdge } from "@xyflow/react"; -import type { WorkflowIrColumn } from "@fusion/core"; +import { BUILTIN_CODING_WORKFLOW_IR, BUILTIN_STEPWISE_CODING_WORKFLOW_IR } from "@fusion/core"; +import type { WorkflowDefinition, WorkflowIrColumn } from "@fusion/core"; import type { WorkflowFlowNodeData } from "../nodes/WorkflowNodeTypes"; -import { autoLayout, applyAutoLayout } from "../workflow-auto-layout"; +import { autoLayout, applyAutoLayout, WF_AUTO_LAYOUT_GAP_X } from "../workflow-auto-layout"; import { strictColumnForY, bandTop, @@ -10,6 +11,7 @@ import { COLUMN_BAND_HEIGHT, FOREACH_GROUP_WIDTH, WF_CARD_MAX_WIDTH, + irToFlow, } from "../workflow-flow-mapping"; type N = FlowNode; @@ -51,6 +53,59 @@ function midBand(index: number): number { return bandTop(index) + COLUMN_BAND_HEIGHT / 2; } +function workflowDef(ir: WorkflowDefinition["ir"]): WorkflowDefinition { + return { + id: ir.name, + kind: "workflow", + name: ir.name, + description: "", + ir, + layout: {}, + createdAt: "2024-01-01T00:00:00.000Z", + updatedAt: "2024-01-01T00:00:00.000Z", + }; +} + +function nodeWidthFromFlow(node: FlowNode): number { + return typeof node.style?.width === "number" ? node.style.width : WF_CARD_MAX_WIDTH; +} + +function assertAutoLayoutRunConnected( + name: string, + def: WorkflowDefinition, + run: readonly string[], +): void { + const columns = def.ir.version === "v2" ? def.ir.columns : []; + const flow = irToFlow(def); + const byId = new Map(flow.nodes.map((candidate) => [candidate.id, candidate] as const)); + const positions = autoLayout(flow.nodes, flow.edges, columns); + + for (const id of run) { + const node = byId.get(id); + const position = positions.get(id); + expect(node, `${name} ${id} node`).toBeTruthy(); + expect(position, `${name} ${id} position`).toBeTruthy(); + if (node && position && node.data.column) { + expect(strictColumnForY(position.y, columns), `${name} ${id} column`).toBe(node.data.column); + } + } + + for (let index = 0; index < run.length - 1; index++) { + const currentId = run[index]; + const nextId = run[index + 1]; + const current = byId.get(currentId)!; + const next = byId.get(nextId)!; + const currentPosition = positions.get(currentId)!; + const nextPosition = positions.get(nextId)!; + expect( + nextPosition.x, + `${name} ${currentId}->${nextId} should leave rendered-width gap`, + ).toBeGreaterThanOrEqual(currentPosition.x + nodeWidthFromFlow(current) + WF_AUTO_LAYOUT_GAP_X); + expect(current.type, `${name} ${currentId} type`).toBe(current.data.kind); + expect(next.type, `${name} ${nextId} type`).toBe(next.data.kind); + } +} + describe("autoLayout — v2 (column-preserving)", () => { it("linear chain: strictly increasing x and every node keeps its column", () => { const nodes: N[] = [ @@ -170,8 +225,61 @@ describe("autoLayout — foreach / unreachable / cycles", () => { ]; const pos = autoLayout(nodes, [edge("start", "verify"), edge("verify", "review"), edge("review", "end")], COLUMNS_3); - expect(pos.get("verify")!.x).toBeGreaterThanOrEqual(pos.get("start")!.x + WF_CARD_MAX_WIDTH); - expect(pos.get("review")!.x).toBeGreaterThanOrEqual(pos.get("verify")!.x + FOREACH_GROUP_WIDTH); + expect(pos.get("verify")!.x).toBeGreaterThanOrEqual(pos.get("start")!.x + WF_CARD_MAX_WIDTH + WF_AUTO_LAYOUT_GAP_X); + expect(pos.get("review")!.x).toBeGreaterThanOrEqual(pos.get("verify")!.x + FOREACH_GROUP_WIDTH + WF_AUTO_LAYOUT_GAP_X); + }); + + it("keeps built-in consecutive container runs connected in the editor layout", () => { + assertAutoLayoutRunConnected("coding", workflowDef(BUILTIN_CODING_WORKFLOW_IR), [ + "execute", + "browser-verification", + "code-review", + "review", + ]); + assertAutoLayoutRunConnected("stepwise", workflowDef(BUILTIN_STEPWISE_CODING_WORKFLOW_IR), [ + "steps", + "browser-verification", + "code-review", + "review", + ]); + }); + + it("keeps consecutive optional-group foreach and loop containers connected", () => { + const synthetic: WorkflowDefinition["ir"] = { + version: "v2", + name: "container-run", + columns: COLUMNS_3, + nodes: [ + { id: "start", kind: "start", column: "triage" }, + { + id: "optional", + kind: "optional-group", + column: "in-progress", + config: { defaultOn: false, template: { nodes: [], edges: [] } }, + }, + { + id: "foreach", + kind: "foreach", + column: "in-progress", + config: { source: "task-steps", template: { nodes: [], edges: [] } }, + }, + { + id: "loop", + kind: "loop", + column: "in-progress", + config: { maxIterations: 2, template: { nodes: [], edges: [] } }, + }, + { id: "end", kind: "end", column: "done" }, + ], + edges: [ + { from: "start", to: "optional", condition: "success" }, + { from: "optional", to: "foreach", condition: "success" }, + { from: "foreach", to: "loop", condition: "success" }, + { from: "loop", to: "end", condition: "success" }, + ], + }; + + assertAutoLayoutRunConnected("synthetic", workflowDef(synthetic), ["optional", "foreach", "loop", "end"]); }); it("repositions a foreach group but leaves its parentId children untouched", () => { diff --git a/packages/dashboard/app/components/__tests__/workflow-flow-mapping.test.ts b/packages/dashboard/app/components/__tests__/workflow-flow-mapping.test.ts index e5d127b3f3..a4ca8d96a9 100644 --- a/packages/dashboard/app/components/__tests__/workflow-flow-mapping.test.ts +++ b/packages/dashboard/app/components/__tests__/workflow-flow-mapping.test.ts @@ -35,6 +35,7 @@ import { cascadeDelete, COLUMN_BAND_HEIGHT, WF_CARD_WIDTH, + WF_FALLBACK_NODE_GAP, WF_CARD_MAX_WIDTH, WF_CARD_HEIGHT, FOREACH_GROUP_WIDTH, @@ -79,6 +80,27 @@ function assertContainerHandles(kind: "optional-group" | "foreach" | "loop", dat } } +function assertRunDoesNotOverlap( + name: string, + nodes: FlowNode[], + run: readonly string[], + gap = 0, +): void { + const byId = new Map(nodes.map((node) => [node.id, node] as const)); + for (let index = 0; index < run.length - 1; index++) { + const currentId = run[index]; + const nextId = run[index + 1]; + const current = byId.get(currentId); + const next = byId.get(nextId); + expect(current, `${name} ${currentId}`).toBeTruthy(); + expect(next, `${name} ${nextId}`).toBeTruthy(); + expect( + next!.position.x, + `${name} ${currentId}->${nextId} should leave rendered-width gap`, + ).toBeGreaterThanOrEqual(current!.position.x + nodeWidth(current!) + gap); + } +} + describe("workflow-flow-mapping name preservation", () => { it("does not inject synthetic names for unnamed start/end/merge nodes on round-trip", () => { const ir: WorkflowDefinition["ir"] = { @@ -374,11 +396,12 @@ describe("workflow-flow-mapping v2 round-trip", () => { { id: "start", type: "start", position: { x: 0, y: 0 }, data: { kind: "start", label: "start", column: "todo" } }, { id: "end", type: "end", position: { x: 100, y: 0 }, data: { kind: "end", label: "end", column: "todo" } }, ]; - const staleIr = flowToIr("wf", nodes, [], nextColumns).ir; + const startToEnd = [{ id: "e-start-end", source: "start", target: "end", data: { condition: "success" } }]; + const staleIr = flowToIr("wf", nodes, startToEnd, nextColumns).ir; expect(() => parseWorkflowIr(staleIr)).toThrow(/references undefined column 'todo'/); const reconciled = reconcileNodeColumns(nodes.filter((node) => !isColumnBandNode(node.id)), nextColumns); - const { ir: out } = flowToIr("wf", [...columnsToBandNodes(nextColumns), ...reconciled], [], nextColumns); + const { ir: out } = flowToIr("wf", [...columnsToBandNodes(nextColumns), ...reconciled], startToEnd, nextColumns); expect(() => parseWorkflowIr(out)).not.toThrow(); if (out.version !== "v2") throw new Error("expected v2"); @@ -912,7 +935,9 @@ describe("workflow-flow-mapping foreach + rework round-trip", () => { kind: "optional-group" | "foreach" | "loop"; }> = [ { name: "coding browser verification", ir: BUILTIN_CODING_WORKFLOW_IR, id: "browser-verification", kind: "optional-group" }, + { name: "coding code review", ir: BUILTIN_CODING_WORKFLOW_IR, id: "code-review", kind: "optional-group" }, { name: "stepwise browser verification", ir: BUILTIN_STEPWISE_CODING_WORKFLOW_IR, id: "browser-verification", kind: "optional-group" }, + { name: "stepwise code review", ir: BUILTIN_STEPWISE_CODING_WORKFLOW_IR, id: "code-review", kind: "optional-group" }, { name: "stepwise foreach", ir: BUILTIN_STEPWISE_CODING_WORKFLOW_IR, id: "steps", kind: "foreach" }, { name: "coding inserted loop", ir: codingWithLoop, id: "diagnostic-loop", kind: "loop" }, { name: "stepwise inserted loop", ir: stepwiseWithLoop, id: "post-steps-loop", kind: "loop" }, @@ -954,6 +979,55 @@ describe("workflow-flow-mapping foreach + rework round-trip", () => { expect(nodes.filter((node) => node.parentId === testCase.id).every((node) => node.zIndex! > group!.zIndex!), testCase.name).toBe(true); } + assertRunDoesNotOverlap("coding consecutive fallback", irToFlow(makeDef(BUILTIN_CODING_WORKFLOW_IR)).nodes, [ + "execute", + "browser-verification", + "code-review", + "review", + ]); + assertRunDoesNotOverlap("stepwise consecutive fallback", irToFlow(makeDef(BUILTIN_STEPWISE_CODING_WORKFLOW_IR)).nodes, [ + "steps", + "browser-verification", + "code-review", + "review", + ]); + + const consecutiveMixedContainers: WorkflowDefinition["ir"] = { + version: "v2", + name: "mixed-containers", + columns: BUILTIN_CODING_WORKFLOW_IR.columns, + nodes: [ + { id: "start", kind: "start", column: "todo" }, + { + id: "optional", + kind: "optional-group", + column: "in-progress", + config: { defaultOn: false, template: { nodes: [], edges: [] } }, + }, + { + id: "foreach", + kind: "foreach", + column: "in-progress", + config: { source: "task-steps", template: { nodes: [], edges: [] } }, + }, + { + id: "loop", + kind: "loop", + column: "in-progress", + config: { maxIterations: 2, template: { nodes: [], edges: [] } }, + }, + { id: "end", kind: "end", column: "done" }, + ], + edges: [ + { from: "start", to: "optional", condition: "success" }, + { from: "optional", to: "foreach", condition: "success" }, + { from: "foreach", to: "loop", condition: "success" }, + { from: "loop", to: "end", condition: "success" }, + ], + }; + const mixedFlow = irToFlow(makeDef(consecutiveMixedContainers)); + assertRunDoesNotOverlap("mixed consecutive containers", mixedFlow.nodes, ["optional", "foreach", "loop", "end"]); + const staleBuiltInLayout: WorkflowDefinition["layout"] = { start: { x: 60, y: 160 }, execute: { x: 230, y: 160 }, @@ -973,6 +1047,50 @@ describe("workflow-flow-mapping foreach + rework round-trip", () => { expect(targetRight, `stale layout ${edge.source}->${edge.target} should not overlap`).toBeLessThanOrEqual(source.position.x); } } + + const completeButStaleLayout = Object.fromEntries( + BUILTIN_CODING_WORKFLOW_IR.nodes.map((node, index) => [node.id, { x: 60 + index * 170, y: 160 }]), + ); + const { nodes: completeStaleNodes } = irToFlow(makeDef(BUILTIN_CODING_WORKFLOW_IR, completeButStaleLayout)); + const completeStaleById = new Map(completeStaleNodes.map((node) => [node.id, node] as const)); + expect(completeStaleById.get("browser-verification")?.position.x).not.toBe(completeButStaleLayout["browser-verification"].x); + assertRunDoesNotOverlap("complete stale coding layout", completeStaleNodes, [ + "execute", + "browser-verification", + "code-review", + "review", + ], WF_FALLBACK_NODE_GAP); + + const compactManualIr: WorkflowDefinition["ir"] = { + version: "v2", + name: "compact-manual-containers", + columns: BUILTIN_CODING_WORKFLOW_IR.columns, + nodes: [ + { id: "start", kind: "start", column: "todo" }, + { id: "optional", kind: "optional-group", column: "in-progress", config: { template: { nodes: [], edges: [] } } }, + { id: "review", kind: "prompt", column: "in-progress", config: { prompt: "review" } }, + { id: "vertical", kind: "optional-group", column: "in-progress", config: { template: { nodes: [], edges: [] } } }, + { id: "end", kind: "end", column: "done" }, + ], + edges: [ + { from: "start", to: "optional", condition: "success" }, + { from: "optional", to: "review", condition: "success" }, + { from: "review", to: "vertical", condition: "success" }, + { from: "vertical", to: "end", condition: "success" }, + ], + }; + const compactManualLayout: WorkflowDefinition["layout"] = { + start: { x: 40, y: 160 }, + optional: { x: 280, y: 160 }, + review: { x: 850, y: 160 }, + vertical: { x: 850, y: 460 }, + end: { x: 1420, y: 460 }, + }; + const { nodes: compactManualNodes } = irToFlow(makeDef(compactManualIr, compactManualLayout)); + const compactManualById = new Map(compactManualNodes.map((node) => [node.id, node] as const)); + expect(compactManualById.get("optional")?.position).toEqual(compactManualLayout.optional); + expect(compactManualById.get("review")?.position).toEqual(compactManualLayout.review); + expect(compactManualById.get("vertical")?.position).toEqual(compactManualLayout.vertical); }); // FNXC:WorkflowOptionalGroup 2026-06-21-11:30: Deleting an optional-group must diff --git a/packages/dashboard/app/components/__tests__/workflow-mobile-graph.test.ts b/packages/dashboard/app/components/__tests__/workflow-mobile-graph.test.ts index 6a3a8c6634..623447400a 100644 --- a/packages/dashboard/app/components/__tests__/workflow-mobile-graph.test.ts +++ b/packages/dashboard/app/components/__tests__/workflow-mobile-graph.test.ts @@ -133,7 +133,7 @@ describe("buildMobileWorkflowGraph", () => { expect(rows[1].summary).toBe("Gate (blocks)"); }); - it("summarizes Browser Verification container connections for built-in mobile outlines", () => { + it("summarizes consecutive optional-group container connections for built-in mobile outlines", () => { for (const [name, ir, incoming] of [ ["coding", BUILTIN_CODING_WORKFLOW_IR, "execute"], ["stepwise", BUILTIN_STEPWISE_CODING_WORKFLOW_IR, "steps"], @@ -141,9 +141,11 @@ describe("buildMobileWorkflowGraph", () => { const { nodes, edges } = irToFlow(workflowDef(ir)); const rows = buildMobileWorkflowGraph(nodes, edges, ir.version === "v2" ? ir.columns : []); const browserVerification = rows.find((row) => row.id === "browser-verification"); + const codeReview = rows.find((row) => row.id === "code-review"); const incomingRow = rows.find((row) => row.id === incoming); expect(browserVerification?.kind, name).toBe("optional-group"); + expect(codeReview?.kind, name).toBe("optional-group"); expect(incomingRow?.outgoing.some((out) => out.target === "browser-verification"), name).toBe(true); expect(browserVerification?.outgoing.map((out) => [out.target, out.label]), name).toEqual( expect.arrayContaining([ @@ -151,6 +153,12 @@ describe("buildMobileWorkflowGraph", () => { ["end", "failure"], ]), ); + expect(codeReview?.outgoing.map((out) => [out.target, out.label]), name).toEqual( + expect.arrayContaining([ + ["review", "success"], + ["end", "failure"], + ]), + ); } }); diff --git a/packages/dashboard/app/components/workflow-auto-layout.ts b/packages/dashboard/app/components/workflow-auto-layout.ts index fd4c51a830..1e2c38270e 100644 --- a/packages/dashboard/app/components/workflow-auto-layout.ts +++ b/packages/dashboard/app/components/workflow-auto-layout.ts @@ -158,6 +158,9 @@ export function autoLayout( /* * FNXC:WorkflowContainerEdges 2026-06-26-08:05: * Auto-layout must space graph layers by the widest rendered node in each prior layer. Container nodes render wider than cards, so fixed card-width layer spacing can place the next optional-group/foreach/loop on top of the previous container's source handle. + * + * FNXC:WorkflowContainerEdges 2026-06-27-22:16: + * The invariant is run-length based, not single-container based: every layer in a consecutive optional-group/foreach/loop sequence must advance by that layer's rendered width so N adjacent containers keep visible target/source edge handles. */ const layerMaxWidths = new Map(); for (const [layerIndex, layerIds] of layers) { diff --git a/packages/dashboard/app/components/workflow-flow-mapping.ts b/packages/dashboard/app/components/workflow-flow-mapping.ts index 66f5450c26..cf88cbaee1 100644 --- a/packages/dashboard/app/components/workflow-flow-mapping.ts +++ b/packages/dashboard/app/components/workflow-flow-mapping.ts @@ -292,6 +292,23 @@ function fallbackWidthForNode(node: WorkflowIrNode): number { return groupTemplateConfigOf(node) ? FOREACH_GROUP_WIDTH : WF_CARD_WIDTH; } +function fallbackHeightForNode(node: WorkflowIrNode): number { + return groupTemplateConfigOf(node) ? FOREACH_GROUP_HEIGHT : WF_CARD_HEIGHT; +} + +function isContainerNode(node: WorkflowIrNode): boolean { + return Boolean(groupTemplateConfigOf(node)); +} + +function verticalSpansOverlap( + a: { y: number }, + aHeight: number, + b: { y: number }, + bHeight: number, +): boolean { + return a.y < b.y + bHeight && b.y < a.y + aHeight; +} + /** * FNXC:WorkflowContainerEdges 2026-06-26-07:30: * Container nodes are much wider than step cards, so fallback graph layout must advance by each rendered node width. Fixed index spacing lets optional-group/foreach/loop backgrounds overlap adjacent handles, making correct top-level edges look visually disconnected in the workflow editor. @@ -306,6 +323,32 @@ function fallbackXPositionsForNodes(nodes: readonly WorkflowIrNode[], originX = return positions; } +function hasUsableTopLevelLayout(def: WorkflowDefinition): boolean { + if (!def.ir.nodes.every((node) => Boolean(def.layout?.[node.id]))) return false; + + const byId = new Map(def.ir.nodes.map((node) => [node.id, node] as const)); + for (const edge of def.ir.edges) { + if (edge.kind === "rework") continue; + const source = byId.get(edge.from); + const target = byId.get(edge.to); + const sourcePos = def.layout?.[edge.from]; + const targetPos = def.layout?.[edge.to]; + if (!source || !target || !sourcePos || !targetPos) continue; + if (!isContainerNode(source) && !isContainerNode(target)) continue; + if (!verticalSpansOverlap(sourcePos, fallbackHeightForNode(source), targetPos, fallbackHeightForNode(target))) continue; + + const sourceRight = sourcePos.x + fallbackWidthForNode(source); + const targetRight = targetPos.x + fallbackWidthForNode(target); + if (sourcePos.x <= targetPos.x) { + if (sourceRight > targetPos.x) return false; + } else if (targetRight > sourcePos.x) { + return false; + } + } + + return true; +} + /** CSS class for an edge given its condition + rework kind. Rework takes * precedence; failure edges get the distinct failure styling; success and other * conditions get no class (default styling). R2's two-channel rule (label always @@ -365,10 +408,13 @@ export function irToFlow(def: WorkflowDefinition): { /* * FNXC:WorkflowContainerEdges 2026-06-26-07:58: * Built-in workflow layouts can lag behind newly inserted container nodes. Mixing stale saved positions with fallback-only optional-group/foreach/loop positions reintroduces overlapping handles, so an incomplete top-level layout falls back as one coherent graph instead of preserving disconnected partial coordinates. + * + * FNXC:WorkflowContainerEdges 2026-06-27-22:15: + * A layout can also be complete-but-stale after consecutive 560px containers are inserted into a formerly card-spaced path. The read-only graph does not run auto-layout, so reject saved top-level coordinates only when a connected optional-group/foreach/loop span actually overlaps another node in the same visual row. This preserves intentional compact or vertical custom layouts while repairing stale container-handle occlusion. */ - const hasCompleteTopLevelLayout = def.ir.nodes.every((node) => Boolean(def.layout?.[node.id])); + const useSavedTopLevelLayout = hasUsableTopLevelLayout(def); const stepNodes = def.ir.nodes.map((node): FlowNode => { - const pos = hasCompleteTopLevelLayout ? def.layout?.[node.id] : undefined; + const pos = useSavedTopLevelLayout ? def.layout?.[node.id] : undefined; const kind = editorKind(node); const column = isV2(def.ir) ? node.column : undefined; const colIndex = column ? columns.findIndex((c) => c.id === column) : -1;