From 86afd67054c886e7b87a56d4507c41cda0beddd5 Mon Sep 17 00:00:00 2001 From: Blake Gentry Date: Mon, 9 Feb 2026 23:30:47 -0600 Subject: [PATCH 1/3] Refine workflow edge routing Workflow dependency graphs with fan-in/fan-out could still be misleading in several cases: bends could occur under sibling cards, long horizontal segments could run through node bodies, and direct upstream dependencies could appear to terminate at separate nearby merge points. Those patterns made true dependency relationships harder to read at a glance. The routing logic now keeps the original Manhattan style by default, but validates each candidate lane against stricter constraints. Edge selection rejects lanes whose turns fall inside node no-turn zones and also rejects lanes whose orthogonal segments intersect non-endpoint nodes. For mixed fan-in targets, layout now adds a shared `preferredBendX` hint so off-row incoming edges converge on a common merge lane when safe. Coverage was expanded with focused routing tests for node-body crossing avoidance, near-target lane fallback in same-row sibling scenarios, shared preferred merge-lane convergence, and merge-hint assignment at the layout layer. --- CHANGELOG.md | 1 + src/components/WorkflowDiagram.test.ts | 292 +++++++++ src/components/WorkflowDiagram.tsx | 77 ++- src/components/WorkflowDiagramEdge.tsx | 38 ++ src/components/workflowDiagramEdgePath.ts | 554 ++++++++++++++++++ .../workflowDiagramMergeHints.test.ts | 66 +++ src/components/workflowDiagramMergeHints.ts | 76 +++ 7 files changed, 1087 insertions(+), 17 deletions(-) create mode 100644 src/components/WorkflowDiagram.test.ts create mode 100644 src/components/WorkflowDiagramEdge.tsx create mode 100644 src/components/workflowDiagramEdgePath.ts create mode 100644 src/components/workflowDiagramMergeHints.test.ts create mode 100644 src/components/workflowDiagramMergeHints.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 36cf7e0f..a90ab182 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Prevent double slash in URLs for root path prefix. Thanks [Jan Kott](https://github.com/boostvolt)! [PR #487](https://github.com/riverqueue/riverui/pull/487). - Serve UI HTML for wildcard or missing Accept headers and return 406 for explicit non-HTML requests. Fixes #485. [PR #493](https://github.com/riverqueue/riverui/pull/493). - Prevent jobs detail navigation from bouncing back to `/jobs/` on slow networks due to stale filter-sync URL updates. Fixes #495. [PR #504](https://github.com/riverqueue/riverui/pull/504). +- Workflow detail: improve dependency edge routing to avoid turns under task cards, avoid line segments crossing node bodies, and keep fan-in edges visually converged into shared merge lanes when possible. [PR #507](https://github.com/riverqueue/riverui/pull/507). ## [v0.14.0] - 2026-01-02 diff --git a/src/components/WorkflowDiagram.test.ts b/src/components/WorkflowDiagram.test.ts new file mode 100644 index 00000000..b70e5d92 --- /dev/null +++ b/src/components/WorkflowDiagram.test.ts @@ -0,0 +1,292 @@ +import { describe, expect, it } from "vitest"; + +import { buildWorkflowDiagramEdgePath } from "./workflowDiagramEdgePath"; + +const pointsFromPath = (path: string): Array<{ x: number; y: number }> => { + const matches = [...path.matchAll(/-?\d+(?:\.\d+)?,-?\d+(?:\.\d+)?/g)]; + return matches.map((match) => { + const [x, y] = match[0].split(",").map(Number); + return { x, y }; + }); +}; + +const turnCountFromPath = (path: string): number => { + const points = pointsFromPath(path); + if (points.length <= 2) return 0; + + let turns = 0; + for (let index = 1; index < points.length - 1; index += 1) { + const prev = points[index - 1]; + const current = points[index]; + const next = points[index + 1]; + const isTurn = + (prev.x !== current.x || prev.y !== current.y) && + (current.x !== next.x || current.y !== next.y) && + !(prev.x === current.x && current.x === next.x) && + !(prev.y === current.y && current.y === next.y); + + if (isTurn) turns += 1; + } + + return turns; +}; + +const finalHorizontalSegmentLength = (path: string): number => { + const points = pointsFromPath(path); + if (points.length < 2) return 0; + + const prev = points[points.length - 2]; + const last = points[points.length - 1]; + if (prev.y !== last.y) return 0; + + return Math.abs(last.x - prev.x); +}; + +const pathIntersectsRectWithPadding = ({ + nodeRect, + padding, + path, +}: { + nodeRect: { height: number; width: number; x: number; y: number }; + padding: number; + path: string; +}): boolean => { + const points = pointsFromPath(path); + const minX = nodeRect.x - padding; + const maxX = nodeRect.x + nodeRect.width + padding; + const minY = nodeRect.y - padding; + const maxY = nodeRect.y + nodeRect.height + padding; + + for (let index = 0; index < points.length - 1; index += 1) { + const start = points[index]; + const end = points[index + 1]; + + if (start.y === end.y) { + if (start.y < minY || start.y > maxY) continue; + const segMinX = Math.min(start.x, end.x); + const segMaxX = Math.max(start.x, end.x); + if (segMaxX > minX && segMinX < maxX) return true; + continue; + } + + if (start.x === end.x) { + if (start.x < minX || start.x > maxX) continue; + const segMinY = Math.min(start.y, end.y); + const segMaxY = Math.max(start.y, end.y); + if (segMaxY > minY && segMinY < maxY) return true; + } + } + + return false; +}; + +describe("buildWorkflowDiagramEdgePath", () => { + it("keeps same-row edges straight", () => { + const path = buildWorkflowDiagramEdgePath({ + sourceX: 256, + sourceY: 86, + targetX: 712, + targetY: 86, + }); + + expect(path).toBe("M 256,86 L 712,86"); + expect(turnCountFromPath(path)).toBe(0); + }); + + it("uses a simple two-bend path aligned to dagre lane", () => { + const path = buildWorkflowDiagramEdgePath({ + dagrePoints: [ + { x: 221.23, y: 0 }, + { x: 306, y: -20 }, + { x: 484, y: -20 }, + { x: 662, y: -20 }, + { x: 746.76, y: 0 }, + ], + sourceX: 256, + sourceY: 86, + targetX: 712, + targetY: 22, + }); + + expect(path).toBe("M 256,86 L 306,86 L 306,22 L 712,22"); + expect(turnCountFromPath(path)).toBe(2); + }); + + it("keeps a visible final horizontal segment into target", () => { + const path = buildWorkflowDiagramEdgePath({ + dagrePoints: [{ x: 298, y: 20 }], + sourceX: 100, + sourceY: 20, + targetX: 300, + targetY: 140, + }); + + expect(path).toBe("M 100,20 L 280,20 L 280,140 L 300,140"); + expect(finalHorizontalSegmentLength(path)).toBeGreaterThanOrEqual(20); + }); + + it("keeps a visible final horizontal segment for right-to-left edges", () => { + const path = buildWorkflowDiagramEdgePath({ + dagrePoints: [{ x: 210, y: 20 }], + sourceX: 300, + sourceY: 20, + targetX: 200, + targetY: 140, + }); + + const points = pointsFromPath(path); + const bendX = points[1]?.x; + + expect(points.length).toBe(4); + expect(turnCountFromPath(path)).toBe(2); + expect(bendX).toBeGreaterThanOrEqual(220); + expect(finalHorizontalSegmentLength(path)).toBeGreaterThanOrEqual(20); + }); + + it("nudges bend lane when no-turn zones block baseline turns", () => { + const path = buildWorkflowDiagramEdgePath({ + dagrePoints: [{ x: 160, y: 20 }], + nodeRects: [{ height: 40, width: 40, x: 140, y: 0 }], + sourceX: 100, + sourceY: 20, + targetX: 300, + targetY: 140, + }); + + expect(path).toBe("M 100,20 L 120,20 L 120,140 L 300,140"); + expect(turnCountFromPath(path)).toBe(2); + }); + + it("reroutes around intervening nodes while keeping two bends", () => { + const blockingNode = { height: 44, width: 256, x: 220, y: 118 }; + const path = buildWorkflowDiagramEdgePath({ + dagrePoints: [{ x: 300, y: 20 }], + nodeRects: [blockingNode], + sourceX: 100, + sourceY: 20, + targetX: 640, + targetY: 140, + }); + + const points = pointsFromPath(path); + + expect(points.length).toBe(4); + expect(points[0]).toEqual({ x: 100, y: 20 }); + expect(points[3]).toEqual({ x: 640, y: 140 }); + expect(points[1].x).toBeGreaterThan( + blockingNode.x + blockingNode.width + 12, + ); + expect(points[1].x).toEqual(points[2].x); + expect(points[2].y).toBe(140); + expect(turnCountFromPath(path)).toBe(2); + expect(finalHorizontalSegmentLength(path)).toBeGreaterThanOrEqual(20); + expect( + pathIntersectsRectWithPadding({ + nodeRect: blockingNode, + padding: 12, + path, + }), + ).toBe(false); + }); + + it("uses a near-target bend to avoid crossing a same-row sibling node", () => { + const siblingNode = { height: 44, width: 256, x: 1170, y: 211 }; + const path = buildWorkflowDiagramEdgePath({ + dagrePoints: [{ x: 1100, y: 86 }], + nodeRects: [siblingNode], + sourceX: 1032, + sourceY: 86, + targetX: 1650, + targetY: 233, + }); + + const points = pointsFromPath(path); + const bendX = points[1]?.x; + + expect(points.length).toBe(4); + expect(turnCountFromPath(path)).toBe(2); + expect(bendX).toBeGreaterThan(siblingNode.x + siblingNode.width + 12); + expect(finalHorizontalSegmentLength(path)).toBeGreaterThanOrEqual(20); + expect( + pathIntersectsRectWithPadding({ + nodeRect: siblingNode, + padding: 12, + path, + }), + ).toBe(false); + }); + + it("falls back to baseline lane when candidate probing finds no safe lane", () => { + const impossibleLaneBlocker = { + height: 40, + width: 20000, + x: -10000, + y: 60, + }; + const path = buildWorkflowDiagramEdgePath({ + dagrePoints: [{ x: 200, y: 20 }], + nodeRects: [impossibleLaneBlocker], + sourceX: 100, + sourceY: 20, + targetX: 300, + targetY: 140, + }); + + const points = pointsFromPath(path); + + expect(points[1]?.x).toBe(200); + expect(points[2]?.x).toBe(200); + expect( + pathIntersectsRectWithPadding({ + nodeRect: impossibleLaneBlocker, + padding: 12, + path, + }), + ).toBe(true); + }); + + it("adds an approach lane when target y-offset is requested", () => { + const path = buildWorkflowDiagramEdgePath({ + dagrePoints: [{ x: 160, y: 20 }], + sourceX: 100, + sourceY: 20, + targetApproachYOffset: -12, + targetX: 300, + targetY: 140, + }); + + expect(path).toBe( + "M 100,20 L 160,20 L 160,128 L 280,128 L 280,140 L 300,140", + ); + expect(turnCountFromPath(path)).toBe(4); + expect(finalHorizontalSegmentLength(path)).toBeGreaterThanOrEqual(20); + }); + + it("supports a shared preferred merge x for multi-edge convergence", () => { + const topPath = buildWorkflowDiagramEdgePath({ + dagrePoints: [{ x: 420, y: 100 }], + preferredBendX: 760, + sourceX: 200, + sourceY: 100, + targetX: 800, + targetY: 300, + }); + + const bottomPath = buildWorkflowDiagramEdgePath({ + dagrePoints: [{ x: 460, y: 500 }], + preferredBendX: 760, + sourceX: 200, + sourceY: 500, + targetX: 800, + targetY: 300, + }); + + const topPoints = pointsFromPath(topPath); + const bottomPoints = pointsFromPath(bottomPath); + + expect(topPoints[1].x).toBe(760); + expect(bottomPoints[1].x).toBe(760); + expect(turnCountFromPath(topPath)).toBe(2); + expect(turnCountFromPath(bottomPath)).toBe(2); + }); +}); diff --git a/src/components/WorkflowDiagram.tsx b/src/components/WorkflowDiagram.tsx index 26c2bd07..4b10e821 100644 --- a/src/components/WorkflowDiagram.tsx +++ b/src/components/WorkflowDiagram.tsx @@ -1,5 +1,6 @@ import type { Edge, + EdgeTypes, Node, NodeChange, NodeSelectionChange, @@ -17,6 +18,11 @@ import { MiniMap, ReactFlow } from "@xyflow/react"; import { useTheme } from "next-themes"; import { useCallback, useMemo } from "react"; +import type { WorkflowDiagramNodeRect } from "./workflowDiagramEdgePath"; + +import WorkflowDiagramEdge from "./WorkflowDiagramEdge"; +import { withPreferredTargetMergeX } from "./workflowDiagramMergeHints"; + type nameToJobMap = { [key: string]: JobWithKnownMetadata; }; @@ -27,9 +33,6 @@ type WorkflowDiagramProps = { tasks: JobWithKnownMetadata[]; }; -const dagreGraph = new dagre.graphlib.Graph(); -dagreGraph.setDefaultEdgeLabel(() => ({})); - const nodeWidth = 256; const nodeHeight = 44; @@ -38,6 +41,9 @@ const getLayoutedElements = ( edges: Edge[], direction = "TB", ): { edges: Edge[]; nodes: Node[] } => { + const dagreGraph = new dagre.graphlib.Graph({ multigraph: true }); + dagreGraph.setDefaultEdgeLabel(() => ({})); + const isHorizontal = direction === "LR"; dagreGraph.setGraph({ align: "UL", @@ -52,27 +58,56 @@ const getLayoutedElements = ( }); edges.forEach((edge) => { - dagreGraph.setEdge(edge.source, edge.target); + dagreGraph.setEdge(edge.source, edge.target, {}, edge.id); }); dagre.layout(dagreGraph); - nodes.forEach((node) => { + const layoutedNodes = nodes.map((node) => { const nodeWithPosition = dagreGraph.node(node.id); - node.targetPosition = (isHorizontal ? "left" : "top") as Position; - node.sourcePosition = (isHorizontal ? "right" : "bottom") as Position; - - // We are shifting the dagre node position (anchor=center center) to the top left - // so it matches the React Flow node anchor point (top left). - node.position = { - x: nodeWithPosition.x - nodeWidth / 2, - y: nodeWithPosition.y - nodeHeight / 2, + return { + ...node, + // Shift dagre center-based coordinates to React Flow's top-left anchor. + position: { + x: nodeWithPosition.x - nodeWidth / 2, + y: nodeWithPosition.y - nodeHeight / 2, + }, + sourcePosition: (isHorizontal ? "right" : "bottom") as Position, + targetPosition: (isHorizontal ? "left" : "top") as Position, }; + }); - return node; + const nodeRects: WorkflowDiagramNodeRect[] = layoutedNodes.map((node) => ({ + height: node.height ?? node.measured?.height ?? nodeHeight, + width: node.width ?? node.measured?.width ?? nodeWidth, + x: node.position.x, + y: node.position.y, + })); + + const layoutedEdges = edges.map((edge) => { + const edgeWithPoints = dagreGraph.edge({ + name: edge.id, + v: edge.source, + w: edge.target, + }); + const dagrePoints = (edgeWithPoints?.points || []).map( + (point: { x: number; y: number }) => ({ + x: point.x, + y: point.y, + }), + ); + + return { + ...edge, + data: { + ...(edge.data as Record | undefined), + dagrePoints, + nodeRects, + }, + }; }); - return { edges, nodes }; + return { edges: layoutedEdges, nodes: layoutedNodes }; }; const edgeColorsLight = { @@ -101,6 +136,9 @@ const depStatusFromJob = (job: JobWithKnownMetadata) => { const nodeTypes: NodeTypes = { workflowNode: WorkflowNode, }; +const edgeTypes: EdgeTypes = { + workflowEdge: WorkflowDiagramEdge, +}; type NodeTypeKey = Extract; @@ -209,17 +247,21 @@ export default function WorkflowDiagram({ strokeWidth: 2, }, target: job.id.toString(), - type: "smoothstep", + type: "workflowEdge", }; }); return [...acc, ...newEdges]; }, []); - const { edges: layoutedEdges, nodes: layoutedNodes } = getLayoutedElements( + const { edges: layoutedEdgesRaw, nodes: layoutedNodes } = getLayoutedElements( initialNodes, initialEdges, "LR", ); + const layoutedEdges = withPreferredTargetMergeX( + layoutedEdgesRaw, + layoutedNodes, + ); // Use workflow id to scope/reset the ReactFlow instance between navigations const workflowIdForInstance = @@ -252,6 +294,7 @@ export default function WorkflowDiagram({ ; + nodeRects?: WorkflowDiagramNodeRect[]; + preferredBendX?: number; +}; + +export default function WorkflowDiagramEdge({ + data, + markerEnd, + sourceX, + sourceY, + style, + targetX, + targetY, +}: EdgeProps) { + const edgeData = data as undefined | WorkflowDiagramEdgeData; + + const path = buildWorkflowDiagramEdgePath({ + dagrePoints: edgeData?.dagrePoints, + nodeRects: edgeData?.nodeRects, + preferredBendX: edgeData?.preferredBendX, + sourceX, + sourceY, + targetX, + targetY, + }); + + return ; +} diff --git a/src/components/workflowDiagramEdgePath.ts b/src/components/workflowDiagramEdgePath.ts new file mode 100644 index 00000000..9c071ec3 --- /dev/null +++ b/src/components/workflowDiagramEdgePath.ts @@ -0,0 +1,554 @@ +export type WorkflowDiagramNodeRect = { + height: number; + width: number; + x: number; + y: number; +}; + +type Point = { + x: number; + y: number; +}; + +// Extra padding around each node card where edge turns are not allowed. +const turnNodePadding = 12; +// Keep the final horizontal segment into the target handle visibly long. +const minTargetApproach = 20; +// Search granularity when probing for an alternate bend lane. +const bendNudgeStep = 8; +// Upper bound on probing attempts for an alternate bend lane. +const bendNudgeMaxSteps = 24; + +/* +Routing policy (left-to-right workflows): + +1. Build the original-style Manhattan route with a single shared bend lane: + `source -> (bendX, sourceY) -> (bendX, targetY) -> target`. +2. Prefer Dagre's interior lane for `bendX` to stay close to existing layout. + If a shared merge lane is provided, try that lane first. +3. Keep that route unchanged unless it violates either: + - no-turn zones (turn point inside any node + padding), or + - node-body crossing (segment intersects a non-endpoint node), or + - minimum visible target approach distance. +4. If invalid, probe nearby lanes and pick the nearest valid one. + +This preserves the original routing as much as possible while fixing only the +connections that need adjustment. +*/ + +const dedupeConsecutivePoints = (points: Point[]): Point[] => { + return points.filter((point, index) => { + const previous = points[index - 1]; + if (!previous) return true; + + return previous.x !== point.x || previous.y !== point.y; + }); +}; + +const dedupeNearPoints = (points: Point[], epsilon = 0.5): Point[] => { + return points.filter((point, index) => { + const previous = points[index - 1]; + if (!previous) return true; + + return ( + Math.abs(previous.x - point.x) > epsilon || + Math.abs(previous.y - point.y) > epsilon + ); + }); +}; + +const areCollinear = (pointA: Point, pointB: Point, pointC: Point): boolean => { + const crossProduct = + (pointB.x - pointA.x) * (pointC.y - pointA.y) - + (pointB.y - pointA.y) * (pointC.x - pointA.x); + + return Math.abs(crossProduct) < 0.01; +}; + +const simplifyCollinearPoints = (points: Point[]): Point[] => { + if (points.length <= 2) return points; + + const simplified: Point[] = [points[0]]; + + for (let index = 1; index < points.length - 1; index += 1) { + const previous = simplified[simplified.length - 1]; + const current = points[index]; + const next = points[index + 1]; + + if (!areCollinear(previous, current, next)) { + simplified.push(current); + } + } + + simplified.push(points[points.length - 1]); + return simplified; +}; + +const isPointInRectWithPadding = ( + point: Point, + nodeRect: WorkflowDiagramNodeRect, + padding: number, +): boolean => { + return ( + point.x >= nodeRect.x - padding && + point.x <= nodeRect.x + nodeRect.width + padding && + point.y >= nodeRect.y - padding && + point.y <= nodeRect.y + nodeRect.height + padding + ); +}; + +const isPointInAnyRectWithPadding = ( + point: Point, + nodeRects: WorkflowDiagramNodeRect[], + padding: number, +): boolean => { + return nodeRects.some((nodeRect) => + isPointInRectWithPadding(point, nodeRect, padding), + ); +}; + +const toPath = (points: Point[]): string => { + if (points.length === 0) return ""; + if (points.length === 1) return `M ${points[0].x},${points[0].y}`; + + const [start, ...rest] = points; + return `M ${start.x},${start.y} ${rest.map((point) => `L ${point.x},${point.y}`).join(" ")}`; +}; + +const toInteriorDagrePoints = ( + source: Point, + target: Point, + dagrePoints: Point[], +): Point[] => { + const minX = Math.min(source.x, target.x); + const maxX = Math.max(source.x, target.x); + + return dagrePoints.filter( + (point) => point.x > minX + 1 && point.x < maxX - 1, + ); +}; + +const getBaselineBendX = ( + source: Point, + target: Point, + dagrePoints: Point[], +): number => { + const interiorDagrePoints = toInteriorDagrePoints( + source, + target, + dagrePoints, + ); + if (interiorDagrePoints.length > 0) { + // Follow Dagre's suggested lane when available. + return interiorDagrePoints[0].x; + } + + return source.x + (target.x - source.x) / 2; +}; + +const isTargetApproachVisible = ({ + bendX, + source, + target, +}: { + bendX: number; + source: Point; + target: Point; +}): boolean => { + if (source.x <= target.x) { + return target.x - bendX >= minTargetApproach; + } + + return bendX - target.x >= minTargetApproach; +}; + +const targetApproachBoundary = ({ + source, + target, +}: { + source: Point; + target: Point; +}): number => { + if (source.x <= target.x) return target.x - minTargetApproach; + + return target.x + minTargetApproach; +}; + +const isTurnPoint = (pointA: Point, pointB: Point, pointC: Point): boolean => { + return !areCollinear(pointA, pointB, pointC); +}; + +const getTurnPoints = (points: Point[]): Point[] => { + if (points.length <= 2) return []; + + const turns: Point[] = []; + for (let index = 1; index < points.length - 1; index += 1) { + const previous = points[index - 1]; + const current = points[index]; + const next = points[index + 1]; + + if (isTurnPoint(previous, current, next)) turns.push(current); + } + + return turns; +}; + +const isPathTurnSafe = ( + points: Point[], + nodeRects: WorkflowDiagramNodeRect[], +): boolean => { + const turnPoints = getTurnPoints(points); + + return turnPoints.every( + (turnPoint) => + !isPointInAnyRectWithPadding(turnPoint, nodeRects, turnNodePadding), + ); +}; + +const getBlockingRectsForPathTurns = ( + points: Point[], + nodeRects: WorkflowDiagramNodeRect[], +): WorkflowDiagramNodeRect[] => { + const turnPoints = getTurnPoints(points); + if (turnPoints.length === 0) return []; + + return nodeRects.filter((nodeRect) => + turnPoints.some((turnPoint) => + isPointInRectWithPadding(turnPoint, nodeRect, turnNodePadding), + ), + ); +}; + +const isEndpointRect = ( + nodeRect: WorkflowDiagramNodeRect, + source: Point, + target: Point, +): boolean => { + return ( + isPointInRectWithPadding(source, nodeRect, 0) || + isPointInRectWithPadding(target, nodeRect, 0) + ); +}; + +const doesSegmentIntersectRectWithPadding = ( + segmentStart: Point, + segmentEnd: Point, + nodeRect: WorkflowDiagramNodeRect, + padding: number, +): boolean => { + const minX = nodeRect.x - padding; + const maxX = nodeRect.x + nodeRect.width + padding; + const minY = nodeRect.y - padding; + const maxY = nodeRect.y + nodeRect.height + padding; + + if (segmentStart.y === segmentEnd.y) { + const segmentY = segmentStart.y; + if (segmentY < minY || segmentY > maxY) return false; + + const segmentMinX = Math.min(segmentStart.x, segmentEnd.x); + const segmentMaxX = Math.max(segmentStart.x, segmentEnd.x); + + return segmentMaxX > minX && segmentMinX < maxX; + } + + if (segmentStart.x === segmentEnd.x) { + const segmentX = segmentStart.x; + if (segmentX < minX || segmentX > maxX) return false; + + const segmentMinY = Math.min(segmentStart.y, segmentEnd.y); + const segmentMaxY = Math.max(segmentStart.y, segmentEnd.y); + + return segmentMaxY > minY && segmentMinY < maxY; + } + + return false; +}; + +const isPathSegmentSafe = ({ + nodeRects, + points, + source, + target, +}: { + nodeRects: WorkflowDiagramNodeRect[]; + points: Point[]; + source: Point; + target: Point; +}): boolean => { + const obstacleRects = nodeRects.filter( + (nodeRect) => !isEndpointRect(nodeRect, source, target), + ); + + for (let index = 0; index < points.length - 1; index += 1) { + const segmentStart = points[index]; + const segmentEnd = points[index + 1]; + + const hitsObstacle = obstacleRects.some((nodeRect) => + doesSegmentIntersectRectWithPadding( + segmentStart, + segmentEnd, + nodeRect, + turnNodePadding, + ), + ); + if (hitsObstacle) return false; + } + + return true; +}; + +const getBlockingRectsForPathSegments = ({ + nodeRects, + points, + source, + target, +}: { + nodeRects: WorkflowDiagramNodeRect[]; + points: Point[]; + source: Point; + target: Point; +}): WorkflowDiagramNodeRect[] => { + const obstacleRects = nodeRects.filter( + (nodeRect) => !isEndpointRect(nodeRect, source, target), + ); + + return obstacleRects.filter((nodeRect) => { + for (let index = 0; index < points.length - 1; index += 1) { + if ( + doesSegmentIntersectRectWithPadding( + points[index], + points[index + 1], + nodeRect, + turnNodePadding, + ) + ) { + return true; + } + } + + return false; + }); +}; + +const buildPathForBendX = ({ + bendX, + source, + target, + targetApproachYOffset = 0, +}: { + bendX: number; + source: Point; + target: Point; + targetApproachYOffset?: number; +}): Point[] => { + if (source.y === target.y) { + return [source, target]; + } + + if (targetApproachYOffset === 0) { + return [ + source, + { x: bendX, y: source.y }, + { x: bendX, y: target.y }, + target, + ]; + } + + const approachX = targetApproachBoundary({ source, target }); + const laneY = target.y + targetApproachYOffset; + + return [ + source, + { x: bendX, y: source.y }, + { x: bendX, y: laneY }, + { x: approachX, y: laneY }, + { x: approachX, y: target.y }, + target, + ]; +}; + +const buildBendXCandidates = ({ + baselineX, + blockingRects, + source, + target, + targetApproachYOffset = 0, +}: { + baselineX: number; + blockingRects: WorkflowDiagramNodeRect[]; + source: Point; + target: Point; + targetApproachYOffset?: number; +}): number[] => { + const candidates = new Set([baselineX]); + + if (targetApproachYOffset === 0) { + candidates.add(targetApproachBoundary({ source, target })); + } + + // Try lanes just outside blocking node bounds first; these often produce the + // smallest visible adjustment away from the baseline route. + blockingRects.forEach((nodeRect) => { + candidates.add(nodeRect.x - turnNodePadding); + candidates.add(nodeRect.x + nodeRect.width + turnNodePadding); + }); + + for (let step = 1; step <= bendNudgeMaxSteps; step += 1) { + const offset = step * bendNudgeStep; + candidates.add(baselineX - offset); + candidates.add(baselineX + offset); + } + + // Keep search deterministic and local by preferring candidates nearest to the + // baseline lane before trying farther nudges. + return [...candidates].sort( + (candidateA, candidateB) => + Math.abs(candidateA - baselineX) - Math.abs(candidateB - baselineX), + ); +}; + +const chooseValidBendX = ({ + baselineX, + nodeRects, + source, + target, + targetApproachYOffset = 0, +}: { + baselineX: number; + nodeRects: WorkflowDiagramNodeRect[]; + source: Point; + target: Point; + targetApproachYOffset?: number; +}): number => { + const baselinePath = buildPathForBendX({ + bendX: baselineX, + source, + target, + targetApproachYOffset, + }); + const blockingTurnRects = getBlockingRectsForPathTurns( + baselinePath, + nodeRects, + ); + const blockingSegmentRects = getBlockingRectsForPathSegments({ + nodeRects, + points: baselinePath, + source, + target, + }); + const blockingRects = [ + ...new Set([...blockingSegmentRects, ...blockingTurnRects]), + ]; + const targetApproachIsValid = + targetApproachYOffset === 0 + ? isTargetApproachVisible({ bendX: baselineX, source, target }) + : true; + + if ( + isPathTurnSafe(baselinePath, nodeRects) && + isPathSegmentSafe({ nodeRects, points: baselinePath, source, target }) && + targetApproachIsValid + ) { + return baselineX; + } + + const candidates = buildBendXCandidates({ + baselineX, + blockingRects, + source, + target, + targetApproachYOffset, + }); + + for (const bendX of candidates) { + const candidatePath = buildPathForBendX({ + bendX, + source, + target, + targetApproachYOffset, + }); + const candidateTargetApproachIsValid = + targetApproachYOffset === 0 + ? isTargetApproachVisible({ bendX, source, target }) + : true; + + if ( + isPathTurnSafe(candidatePath, nodeRects) && + isPathSegmentSafe({ nodeRects, points: candidatePath, source, target }) && + candidateTargetApproachIsValid + ) { + return bendX; + } + } + + // Fall back to the baseline lane when probing fails so routing stays stable + // and does not jump to unrelated coordinates. + return baselineX; +}; + +const toOrthogonalPoints = ( + source: Point, + target: Point, + dagrePoints: Point[], + nodeRects: WorkflowDiagramNodeRect[], + preferredBendX: number | undefined, + targetApproachYOffset = 0, +): Point[] => { + // Straight line for same-row targets keeps simple cases unchanged. + if (source.y === target.y && targetApproachYOffset === 0) { + return [source, target]; + } + + const baselineX = + preferredBendX ?? getBaselineBendX(source, target, dagrePoints); + const bendX = chooseValidBendX({ + baselineX, + nodeRects, + source, + target, + targetApproachYOffset, + }); + + return buildPathForBendX({ + bendX, + source, + target, + targetApproachYOffset, + }); +}; + +export const buildWorkflowDiagramEdgePath = ({ + dagrePoints, + nodeRects, + preferredBendX, + sourceX, + sourceY, + targetApproachYOffset, + targetX, + targetY, +}: { + dagrePoints?: Point[]; + nodeRects?: WorkflowDiagramNodeRect[]; + preferredBendX?: number; + sourceX: number; + sourceY: number; + targetApproachYOffset?: number; + targetX: number; + targetY: number; +}): string => { + const source = { x: sourceX, y: sourceY }; + const target = { x: targetX, y: targetY }; + + const points = toOrthogonalPoints( + source, + target, + dagrePoints || [], + nodeRects || [], + preferredBendX, + targetApproachYOffset, + ); + + return toPath( + simplifyCollinearPoints(dedupeNearPoints(dedupeConsecutivePoints(points))), + ); +}; diff --git a/src/components/workflowDiagramMergeHints.test.ts b/src/components/workflowDiagramMergeHints.test.ts new file mode 100644 index 00000000..bd80c864 --- /dev/null +++ b/src/components/workflowDiagramMergeHints.test.ts @@ -0,0 +1,66 @@ +import type { Edge, Node } from "@xyflow/react"; + +import { describe, expect, it } from "vitest"; + +import { withPreferredTargetMergeX } from "./workflowDiagramMergeHints"; + +const node = (id: string, x: number, y: number): Node => ({ + data: {}, + height: 44, + id, + position: { x, y }, + width: 256, +}); + +const edge = (id: string, source: string, target: string): Edge => ({ + id, + source, + target, +}); + +const preferredBendX = (edgeData: unknown): number | undefined => { + if (!edgeData || typeof edgeData !== "object") return undefined; + const value = (edgeData as { preferredBendX?: unknown }).preferredBendX; + return typeof value === "number" ? value : undefined; +}; + +describe("withPreferredTargetMergeX", () => { + it("assigns a shared preferred bend x to off-row incoming edges", () => { + const nodes = [ + node("source-above", 100, 20), + node("source-same-row", 100, 200), + node("source-below", 100, 360), + node("target", 1000, 200), + ]; + const edges = [ + edge("e-above", "source-above", "target"), + edge("e-same-row", "source-same-row", "target"), + edge("e-below", "source-below", "target"), + ]; + + const hintedEdges = withPreferredTargetMergeX(edges, nodes); + const byID = new Map(hintedEdges.map((item) => [item.id, item])); + + expect(preferredBendX(byID.get("e-above")?.data)).toBe(980); + expect(preferredBendX(byID.get("e-below")?.data)).toBe(980); + expect(preferredBendX(byID.get("e-same-row")?.data)).toBeUndefined(); + }); + + it("does not assign preferred bend x when there is no same-row incoming edge", () => { + const nodes = [ + node("source-above", 100, 20), + node("source-below", 100, 360), + node("target", 1000, 200), + ]; + const edges = [ + edge("e-above", "source-above", "target"), + edge("e-below", "source-below", "target"), + ]; + + const hintedEdges = withPreferredTargetMergeX(edges, nodes); + const byID = new Map(hintedEdges.map((item) => [item.id, item])); + + expect(preferredBendX(byID.get("e-above")?.data)).toBeUndefined(); + expect(preferredBendX(byID.get("e-below")?.data)).toBeUndefined(); + }); +}); diff --git a/src/components/workflowDiagramMergeHints.ts b/src/components/workflowDiagramMergeHints.ts new file mode 100644 index 00000000..37161237 --- /dev/null +++ b/src/components/workflowDiagramMergeHints.ts @@ -0,0 +1,76 @@ +import type { Edge, Node } from "@xyflow/react"; + +const nodeHeight = 44; +const sameRowTolerance = 1; +const targetMergePadding = 20; + +const nodeCenterY = (node: Node): number => { + const renderedHeight = node.height ?? node.measured?.height ?? nodeHeight; + return node.position.y + renderedHeight / 2; +}; + +export const withPreferredTargetMergeX = ( + edges: Edge[], + nodes: Node[], +): Edge[] => { + const nodeByID = new Map(nodes.map((node) => [node.id, node])); + const incomingByTarget = new Map(); + + edges.forEach((edge) => { + const incomingEdges = incomingByTarget.get(edge.target); + if (incomingEdges) { + incomingEdges.push(edge); + return; + } + + incomingByTarget.set(edge.target, [edge]); + }); + + const preferredBendByEdgeID = new Map(); + + incomingByTarget.forEach((incomingEdges, targetID) => { + if (incomingEdges.length < 2) return; + + const targetNode = nodeByID.get(targetID); + if (!targetNode) return; + + const targetCenterY = nodeCenterY(targetNode); + const offRowEdges: Edge[] = []; + let hasSameRowIncoming = false; + + incomingEdges.forEach((edge) => { + const sourceNode = nodeByID.get(edge.source); + if (!sourceNode) return; + + const sourceCenterY = nodeCenterY(sourceNode); + if (Math.abs(sourceCenterY - targetCenterY) <= sameRowTolerance) { + hasSameRowIncoming = true; + return; + } + + offRowEdges.push(edge); + }); + + if (!hasSameRowIncoming || offRowEdges.length === 0) return; + + const preferredBendX = targetNode.position.x - targetMergePadding; + offRowEdges.forEach((edge) => { + preferredBendByEdgeID.set(edge.id, preferredBendX); + }); + }); + + if (preferredBendByEdgeID.size === 0) return edges; + + return edges.map((edge) => { + const preferredBendX = preferredBendByEdgeID.get(edge.id); + if (preferredBendX === undefined) return edge; + + return { + ...edge, + data: { + ...((edge.data as Record | undefined) || {}), + preferredBendX, + }, + }; + }); +}; From 2952fd8a121c50d9b3c27e57351d800a47c7452a Mon Sep 17 00:00:00 2001 From: Blake Gentry Date: Wed, 11 Feb 2026 10:41:33 -0600 Subject: [PATCH 2/3] Refactor workflow diagram and harden deps Workflow diagram logic had grown monolithic in `src/components/WorkflowDiagram.tsx`, mixing React wiring with graph construction, Dagre layout, and edge styling. It also crashed on some historical payloads where `metadata.deps` was missing, because the graph model read `job.metadata.deps` directly. This refactor moves workflow diagram code into `src/components/workflow-diagram/` and separates responsibilities across `workflowDiagramGraphModel.ts`, `workflowDiagramLayout.ts`, `workflowDiagramGeometry.ts`, and `workflowDiagramConstants.ts`, while keeping edge-path routing and merge hints in dedicated modules. The model builder now normalizes dependency reads with `job.metadata.deps ?? []` so missing deps are treated as empty lists. The change preserves rendering behavior and routing parity while improving maintainability and preventing runtime failures for malformed or historical rows. Coverage adds regressions for missing `metadata.deps` in `WorkflowDiagram.test.tsx` and `workflowDiagramGraphModel.test.ts`, alongside relocated routing and merge-hint tests in the new module folder. --- src/components/WorkflowDetail.tsx | 2 +- src/components/WorkflowDiagram.tsx | 325 ------------------ .../workflow-diagram/WorkflowDiagram.test.tsx | 221 ++++++++++++ .../workflow-diagram/WorkflowDiagram.tsx | 166 +++++++++ .../WorkflowDiagramEdge.tsx | 1 + .../{ => workflow-diagram}/WorkflowNode.tsx | 0 .../{ => workflow-diagram}/reactflow-base.css | 0 .../workflowDiagramConstants.ts | 39 +++ .../workflowDiagramEdgePath.test.ts} | 0 .../workflowDiagramEdgePath.ts | 246 +++++-------- .../workflowDiagramGeometry.ts | 128 +++++++ .../workflowDiagramGraphModel.test.ts | 245 +++++++++++++ .../workflowDiagramGraphModel.ts | 154 +++++++++ .../workflow-diagram/workflowDiagramLayout.ts | 91 +++++ .../workflowDiagramMergeHints.test.ts | 0 .../workflowDiagramMergeHints.ts | 10 +- 16 files changed, 1132 insertions(+), 496 deletions(-) delete mode 100644 src/components/WorkflowDiagram.tsx create mode 100644 src/components/workflow-diagram/WorkflowDiagram.test.tsx create mode 100644 src/components/workflow-diagram/WorkflowDiagram.tsx rename src/components/{ => workflow-diagram}/WorkflowDiagramEdge.tsx (94%) rename src/components/{ => workflow-diagram}/WorkflowNode.tsx (100%) rename src/components/{ => workflow-diagram}/reactflow-base.css (100%) create mode 100644 src/components/workflow-diagram/workflowDiagramConstants.ts rename src/components/{WorkflowDiagram.test.ts => workflow-diagram/workflowDiagramEdgePath.test.ts} (100%) rename src/components/{ => workflow-diagram}/workflowDiagramEdgePath.ts (61%) create mode 100644 src/components/workflow-diagram/workflowDiagramGeometry.ts create mode 100644 src/components/workflow-diagram/workflowDiagramGraphModel.test.ts create mode 100644 src/components/workflow-diagram/workflowDiagramGraphModel.ts create mode 100644 src/components/workflow-diagram/workflowDiagramLayout.ts rename src/components/{ => workflow-diagram}/workflowDiagramMergeHints.test.ts (100%) rename src/components/{ => workflow-diagram}/workflowDiagramMergeHints.ts (89%) diff --git a/src/components/WorkflowDetail.tsx b/src/components/WorkflowDetail.tsx index 4b5b1877..7dea1c19 100644 --- a/src/components/WorkflowDetail.tsx +++ b/src/components/WorkflowDetail.tsx @@ -5,7 +5,7 @@ import RelativeTimeFormatter from "@components/RelativeTimeFormatter"; import RetryWorkflowDialog from "@components/RetryWorkflowDialog"; import { TaskStateIcon } from "@components/TaskStateIcon"; import TopNavTitleOnly from "@components/TopNavTitleOnly"; -import WorkflowDiagram from "@components/WorkflowDiagram"; +import WorkflowDiagram from "@components/workflow-diagram/WorkflowDiagram"; import { useFeatures } from "@contexts/Features.hook"; // (Dialog is now encapsulated in RetryWorkflowDialog) import { ArrowPathIcon, XCircleIcon } from "@heroicons/react/24/outline"; diff --git a/src/components/WorkflowDiagram.tsx b/src/components/WorkflowDiagram.tsx deleted file mode 100644 index 4b10e821..00000000 --- a/src/components/WorkflowDiagram.tsx +++ /dev/null @@ -1,325 +0,0 @@ -import type { - Edge, - EdgeTypes, - Node, - NodeChange, - NodeSelectionChange, - NodeTypes, - Position, -} from "@xyflow/react"; - -import WorkflowNode, { WorkflowNodeData } from "@components/WorkflowNode"; - -import "./reactflow-base.css"; -import dagre from "@dagrejs/dagre"; -import { JobWithKnownMetadata } from "@services/jobs"; -import { JobState } from "@services/types"; -import { MiniMap, ReactFlow } from "@xyflow/react"; -import { useTheme } from "next-themes"; -import { useCallback, useMemo } from "react"; - -import type { WorkflowDiagramNodeRect } from "./workflowDiagramEdgePath"; - -import WorkflowDiagramEdge from "./WorkflowDiagramEdge"; -import { withPreferredTargetMergeX } from "./workflowDiagramMergeHints"; - -type nameToJobMap = { - [key: string]: JobWithKnownMetadata; -}; - -type WorkflowDiagramProps = { - selectedJobId?: bigint; - setSelectedJobId: (id: bigint | undefined) => void; - tasks: JobWithKnownMetadata[]; -}; - -const nodeWidth = 256; -const nodeHeight = 44; - -const getLayoutedElements = ( - nodes: Node[], - edges: Edge[], - direction = "TB", -): { edges: Edge[]; nodes: Node[] } => { - const dagreGraph = new dagre.graphlib.Graph({ multigraph: true }); - dagreGraph.setDefaultEdgeLabel(() => ({})); - - const isHorizontal = direction === "LR"; - dagreGraph.setGraph({ - align: "UL", - edgesep: 100, - nodesep: 20, - rankdir: direction, - ranksep: 100, - }); - - nodes.forEach((node) => { - dagreGraph.setNode(node.id, { height: nodeHeight, width: nodeWidth }); - }); - - edges.forEach((edge) => { - dagreGraph.setEdge(edge.source, edge.target, {}, edge.id); - }); - - dagre.layout(dagreGraph); - - const layoutedNodes = nodes.map((node) => { - const nodeWithPosition = dagreGraph.node(node.id); - return { - ...node, - // Shift dagre center-based coordinates to React Flow's top-left anchor. - position: { - x: nodeWithPosition.x - nodeWidth / 2, - y: nodeWithPosition.y - nodeHeight / 2, - }, - sourcePosition: (isHorizontal ? "right" : "bottom") as Position, - targetPosition: (isHorizontal ? "left" : "top") as Position, - }; - }); - - const nodeRects: WorkflowDiagramNodeRect[] = layoutedNodes.map((node) => ({ - height: node.height ?? node.measured?.height ?? nodeHeight, - width: node.width ?? node.measured?.width ?? nodeWidth, - x: node.position.x, - y: node.position.y, - })); - - const layoutedEdges = edges.map((edge) => { - const edgeWithPoints = dagreGraph.edge({ - name: edge.id, - v: edge.source, - w: edge.target, - }); - const dagrePoints = (edgeWithPoints?.points || []).map( - (point: { x: number; y: number }) => ({ - x: point.x, - y: point.y, - }), - ); - - return { - ...edge, - data: { - ...(edge.data as Record | undefined), - dagrePoints, - nodeRects, - }, - }; - }); - - return { edges: layoutedEdges, nodes: layoutedNodes }; -}; - -const edgeColorsLight = { - blocked: "#cbd5e1", - failed: "#dc2626", - unblocked: "#cbd5e1", -}; -const edgeColorsDark = { - blocked: "#475569", - failed: "#dc2626", - unblocked: "#475569", -}; - -const depStatusFromJob = (job: JobWithKnownMetadata) => { - switch (job.state) { - case JobState.Cancelled: - case JobState.Discarded: - return "failed"; - case JobState.Completed: - return "unblocked"; - default: - return "blocked"; - } -}; - -const nodeTypes: NodeTypes = { - workflowNode: WorkflowNode, -}; -const edgeTypes: EdgeTypes = { - workflowEdge: WorkflowDiagramEdge, -}; - -type NodeTypeKey = Extract; - -export default function WorkflowDiagram({ - selectedJobId, - setSelectedJobId, - tasks, -}: WorkflowDiagramProps) { - const { resolvedTheme } = useTheme(); - - const edgeColors = - resolvedTheme === "dark" ? edgeColorsDark : edgeColorsLight; - - const minimapMaskColor = - resolvedTheme === "dark" ? "rgb(5, 5, 5, 0.5)" : "rgb(250, 250, 250, 0.5)"; - const getMiniMapNodeClassName = ( - node: Node, - ): string => { - const state = node.data?.job?.state; - switch (state) { - case JobState.Available: - case JobState.Pending: - case JobState.Retryable: - case JobState.Scheduled: - return "fill-amber-300/60 stroke-amber-500/60 dark:fill-amber-700/50 dark:stroke-amber-400/50 stroke-1"; - case JobState.Cancelled: - case JobState.Discarded: - return "fill-red-300/60 stroke-red-500/60 dark:fill-red-700/50 dark:stroke-red-400/50 stroke-1"; - case JobState.Completed: - return "fill-green-300/60 stroke-green-500/60 dark:fill-green-500/70 dark:stroke-green-300/70 stroke-1"; - case JobState.Running: - return "fill-blue-300/60 stroke-blue-500/60 dark:fill-blue-700/50 dark:stroke-blue-400/50 stroke-1"; - default: - return "fill-slate-300/60 stroke-slate-600/60 dark:fill-slate-700/50 dark:stroke-slate-400/50 stroke-1"; - } - }; - - // TODO: not ideal to iterate through this list so many times. Should probably - // do that once and save all results at the same time. - - const tasksWithDownstreamDeps = tasks.reduce( - (acc: Record, job) => { - const deps = job.metadata.deps || []; - deps.forEach((depName) => { - acc[depName] = true; - }); - return acc; - }, - {}, - ); - - const initialNodes: Node[] = useMemo( - () => - tasks.map((job) => ({ - connectable: false, - data: { - hasDownstreamDeps: - tasksWithDownstreamDeps[job.metadata.task] || false, - hasUpstreamDeps: job.metadata.deps?.length > 0, - job, - }, - height: nodeHeight, - id: job.id.toString(), - position: { x: 0, y: 0 }, - selected: selectedJobId === job.id, - type: "workflowNode", - width: nodeWidth, - })), - [tasks, selectedJobId, tasksWithDownstreamDeps], - ); - - const jobsByTask = tasks.reduce((acc: nameToJobMap, job) => { - acc[job.metadata.task] = job; - return acc; - }, {}); - - const initialEdges = tasks.reduce((acc: Edge[], job) => { - const metadata = job.metadata; - const newEdges = (metadata.deps || []) - // Filter out any deps that aren't in the list of jobs (maybe due to being - // deleted or cleaned already): - .filter((dep) => jobsByTask[dep]) - .map((depName) => { - const dep = jobsByTask[depName]; - const depStatus = depStatusFromJob(dep); - const edgeColor = edgeColors[depStatus]; - - // Only animate when the downstream job is actively waiting on deps - // i.e., Pending. If the downstream job is cancelled/discarded/etc., - // keep the edge static. - const isActivelyWaiting = job.state === JobState.Pending; - - // Subtle visual distinction: - // - blocked: grey dashed; animated only if actively waiting - // - failed: red dashed; never animated - // - unblocked: grey solid - const strokeDasharray = depStatus === "unblocked" ? "0" : "6 3"; - - return { - animated: depStatus === "blocked" && isActivelyWaiting, - id: `e-${dep.id}-${job.id}`, - source: dep.id.toString(), - style: { - stroke: edgeColor, - strokeDasharray, - strokeWidth: 2, - }, - target: job.id.toString(), - type: "workflowEdge", - }; - }); - return [...acc, ...newEdges]; - }, []); - - const { edges: layoutedEdgesRaw, nodes: layoutedNodes } = getLayoutedElements( - initialNodes, - initialEdges, - "LR", - ); - const layoutedEdges = withPreferredTargetMergeX( - layoutedEdgesRaw, - layoutedNodes, - ); - - // Use workflow id to scope/reset the ReactFlow instance between navigations - const workflowIdForInstance = - tasks[0]?.metadata.workflow_id ?? "unknown-workflow"; - - const isNodeSelectionChange = ( - change: NodeChange, - ): change is NodeSelectionChange => { - return change.type === "select"; - }; - - const onNodesChange = useCallback( - (changes: NodeChange[]) => { - const selectionChanges = changes.filter(isNodeSelectionChange); - if (selectionChanges.length === 0) return; - - const selectedNode = selectionChanges.find((change) => change.selected); - - // If there's a new selected node, set that and we're done: - if (selectedNode && BigInt(selectedNode.id) !== selectedJobId) { - setSelectedJobId(BigInt(selectedNode.id)); - return; - } - }, - [selectedJobId, setSelectedJobId], - ); - - return ( -
- {}} - onNodesChange={onNodesChange} - proOptions={{ hideAttribution: true }} - > - - -
- ); -} diff --git a/src/components/workflow-diagram/WorkflowDiagram.test.tsx b/src/components/workflow-diagram/WorkflowDiagram.test.tsx new file mode 100644 index 00000000..4c7227be --- /dev/null +++ b/src/components/workflow-diagram/WorkflowDiagram.test.tsx @@ -0,0 +1,221 @@ +import type { PropsWithChildren } from "react"; + +import { JobWithKnownMetadata } from "@services/jobs"; +import { JobState } from "@services/types"; +import { act, render, screen } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import WorkflowDiagram from "./WorkflowDiagram"; +import * as workflowDiagramLayout from "./workflowDiagramLayout"; + +const baseDate = new Date("2025-01-01T00:00:00.000Z"); + +const workflowJob = ({ + deps = [], + id, + state = JobState.Available, + task, + workflowID = "wf-1", +}: { + deps?: string[]; + id: number; + state?: JobState; + task: string; + workflowID?: string; +}): JobWithKnownMetadata => ({ + args: {}, + attempt: 0, + attemptedAt: undefined, + attemptedBy: [], + createdAt: baseDate, + errors: [], + finalizedAt: undefined, + id: BigInt(id), + kind: `job-${task}`, + logs: {}, + maxAttempts: 1, + metadata: { + deps, + task, + workflow_id: workflowID, + workflow_staged_at: baseDate.toISOString(), + }, + priority: 1, + queue: "default", + scheduledAt: baseDate, + state, + tags: [], +}); + +type MockReactFlowProps = PropsWithChildren<{ + edges: unknown[]; + nodes: unknown[]; + onNodesChange?: (changes: SelectionChange[]) => void; +}>; + +type SelectionChange = { + id: string; + selected: boolean; + type: "select"; +}; + +let currentTheme: "dark" | "light" = "light"; +let latestReactFlowProps: MockReactFlowProps | undefined; + +vi.mock("next-themes", () => ({ + useTheme: () => ({ resolvedTheme: currentTheme }), +})); + +vi.mock("./WorkflowNode", () => ({ + default: () => null, +})); + +vi.mock("@xyflow/react", () => ({ + BaseEdge: () => null, + MiniMap: () =>
, + ReactFlow: (props: MockReactFlowProps) => { + latestReactFlowProps = props; + + return ( +
+
{props.edges.length}
+
{props.nodes.length}
+ {props.children} +
+ ); + }, +})); + +describe("WorkflowDiagram", () => { + beforeEach(() => { + currentTheme = "light"; + latestReactFlowProps = undefined; + vi.restoreAllMocks(); + }); + + it("renders nodes and edges for a workflow", () => { + const tasks = [ + workflowJob({ id: 1, task: "a" }), + workflowJob({ deps: ["a"], id: 2, task: "b" }), + workflowJob({ deps: ["a", "b"], id: 3, task: "c" }), + ]; + + render( + , + ); + + expect(screen.getByTestId("react-flow")).toBeInTheDocument(); + expect(screen.getByTestId("node-count")).toHaveTextContent("3"); + expect(screen.getByTestId("edge-count")).toHaveTextContent("3"); + }); + + it("calls setSelectedJobId when a node is selected", () => { + const tasks = [ + workflowJob({ id: 1, task: "a" }), + workflowJob({ deps: ["a"], id: 2, task: "b" }), + ]; + + const setSelectedJobID = vi.fn(); + + render( + , + ); + + expect(latestReactFlowProps).toBeDefined(); + + act(() => { + latestReactFlowProps?.onNodesChange?.([ + { id: "2", selected: true, type: "select" }, + ]); + }); + + expect(setSelectedJobID).toHaveBeenCalledWith(2n); + }); + + it("does not rerun layout when only selectedJobId changes", () => { + const tasks = [ + workflowJob({ id: 1, task: "a" }), + workflowJob({ deps: ["a"], id: 2, task: "b" }), + ]; + const layoutSpy = vi.spyOn(workflowDiagramLayout, "getLayoutedElements"); + + const { rerender } = render( + , + ); + + expect(layoutSpy).toHaveBeenCalledTimes(1); + + rerender( + , + ); + + expect(layoutSpy).toHaveBeenCalledTimes(1); + }); + + it("does not rerun layout when only theme changes", () => { + const tasks = [ + workflowJob({ id: 1, task: "a" }), + workflowJob({ deps: ["a"], id: 2, task: "b" }), + ]; + const layoutSpy = vi.spyOn(workflowDiagramLayout, "getLayoutedElements"); + + const { rerender } = render( + , + ); + + expect(layoutSpy).toHaveBeenCalledTimes(1); + + currentTheme = "dark"; + + rerender( + , + ); + + expect(layoutSpy).toHaveBeenCalledTimes(1); + }); + + it("renders when metadata.deps is missing on a task", () => { + const malformedJob = workflowJob({ id: 1, task: "a" }); + ( + malformedJob.metadata as unknown as { + deps?: string[]; + } + ).deps = undefined; + + render( + , + ); + + expect(screen.getByTestId("react-flow")).toBeInTheDocument(); + expect(screen.getByTestId("node-count")).toHaveTextContent("1"); + expect(screen.getByTestId("edge-count")).toHaveTextContent("0"); + }); +}); diff --git a/src/components/workflow-diagram/WorkflowDiagram.tsx b/src/components/workflow-diagram/WorkflowDiagram.tsx new file mode 100644 index 00000000..9af2a0ff --- /dev/null +++ b/src/components/workflow-diagram/WorkflowDiagram.tsx @@ -0,0 +1,166 @@ +import type { + EdgeTypes, + Node, + NodeChange, + NodeSelectionChange, + NodeTypes, +} from "@xyflow/react"; + +import { JobWithKnownMetadata } from "@services/jobs"; +import { JobState } from "@services/types"; +import { MiniMap, ReactFlow } from "@xyflow/react"; +import { useTheme } from "next-themes"; +import { useCallback, useMemo } from "react"; + +import WorkflowDiagramEdge from "./WorkflowDiagramEdge"; +import { + applyEdgeVisuals, + buildWorkflowGraphModel, +} from "./workflowDiagramGraphModel"; +import WorkflowNode, { type WorkflowNodeData } from "./WorkflowNode"; +import "./reactflow-base.css"; + +type WorkflowDiagramProps = { + selectedJobId?: bigint; + setSelectedJobId: (id: bigint | undefined) => void; + tasks: JobWithKnownMetadata[]; +}; + +const edgeColorsLight = { + blocked: "#cbd5e1", + failed: "#dc2626", + unblocked: "#cbd5e1", +}; + +const edgeColorsDark = { + blocked: "#475569", + failed: "#dc2626", + unblocked: "#475569", +}; + +const nodeTypes: NodeTypes = { + workflowNode: WorkflowNode, +}; + +const edgeTypes: EdgeTypes = { + workflowEdge: WorkflowDiagramEdge, +}; + +type NodeTypeKey = Extract; + +const getMiniMapNodeClassName = ( + node: Node, +): string => { + const state = node.data?.job?.state; + + switch (state) { + case JobState.Available: + case JobState.Pending: + case JobState.Retryable: + case JobState.Scheduled: + return "fill-amber-300/60 stroke-amber-500/60 dark:fill-amber-700/50 dark:stroke-amber-400/50 stroke-1"; + case JobState.Cancelled: + case JobState.Discarded: + return "fill-red-300/60 stroke-red-500/60 dark:fill-red-700/50 dark:stroke-red-400/50 stroke-1"; + case JobState.Completed: + return "fill-green-300/60 stroke-green-500/60 dark:fill-green-500/70 dark:stroke-green-300/70 stroke-1"; + case JobState.Running: + return "fill-blue-300/60 stroke-blue-500/60 dark:fill-blue-700/50 dark:stroke-blue-400/50 stroke-1"; + default: + return "fill-slate-300/60 stroke-slate-600/60 dark:fill-slate-700/50 dark:stroke-slate-400/50 stroke-1"; + } +}; + +const isNodeSelectionChange = ( + change: NodeChange, +): change is NodeSelectionChange => { + return change.type === "select"; +}; + +export default function WorkflowDiagram({ + selectedJobId, + setSelectedJobId, + tasks, +}: WorkflowDiagramProps) { + const { resolvedTheme } = useTheme(); + + const edgeColors = + resolvedTheme === "dark" ? edgeColorsDark : edgeColorsLight; + + const minimapMaskColor = + resolvedTheme === "dark" ? "rgb(5, 5, 5, 0.5)" : "rgb(250, 250, 250, 0.5)"; + + // Build structural graph data only from `tasks`. This is the expensive stage + // that includes Dagre layout, so it must not depend on selection or theme. + const model = useMemo(() => buildWorkflowGraphModel(tasks), [tasks]); + + // Theme updates should only restyle existing edges, not recompute topology or + // layout. Keeping this in a separate memo avoids unnecessary layout work. + const layoutedEdges = useMemo( + () => applyEdgeVisuals(model.edges, edgeColors), + [edgeColors, model.edges], + ); + + // Node selection is UI state layered on top of static layout coordinates. + // Apply it separately so clicking nodes does not trigger Dagre. + const layoutedNodes = useMemo( + () => + model.nodes.map((node) => ({ + ...node, + selected: selectedJobId === node.data.job.id, + })), + [model.nodes, selectedJobId], + ); + + // Use workflow id to scope/reset the ReactFlow instance between navigations. + const workflowIdForInstance = + tasks[0]?.metadata.workflow_id ?? "unknown-workflow"; + + const onNodesChange = useCallback( + (changes: NodeChange[]) => { + const selectionChanges = changes.filter(isNodeSelectionChange); + if (selectionChanges.length === 0) return; + + const selectedNode = selectionChanges.find((change) => change.selected); + + if (selectedNode && BigInt(selectedNode.id) !== selectedJobId) { + setSelectedJobId(BigInt(selectedNode.id)); + } + }, + [selectedJobId, setSelectedJobId], + ); + + return ( +
+ {}} + onNodesChange={onNodesChange} + proOptions={{ hideAttribution: true }} + > + + +
+ ); +} diff --git a/src/components/WorkflowDiagramEdge.tsx b/src/components/workflow-diagram/WorkflowDiagramEdge.tsx similarity index 94% rename from src/components/WorkflowDiagramEdge.tsx rename to src/components/workflow-diagram/WorkflowDiagramEdge.tsx index efb86e41..84c36409 100644 --- a/src/components/WorkflowDiagramEdge.tsx +++ b/src/components/workflow-diagram/WorkflowDiagramEdge.tsx @@ -9,6 +9,7 @@ import { type WorkflowDiagramEdgeData = { dagrePoints?: Array<{ x: number; y: number }>; + depStatus?: "blocked" | "failed" | "unblocked"; nodeRects?: WorkflowDiagramNodeRect[]; preferredBendX?: number; }; diff --git a/src/components/WorkflowNode.tsx b/src/components/workflow-diagram/WorkflowNode.tsx similarity index 100% rename from src/components/WorkflowNode.tsx rename to src/components/workflow-diagram/WorkflowNode.tsx diff --git a/src/components/reactflow-base.css b/src/components/workflow-diagram/reactflow-base.css similarity index 100% rename from src/components/reactflow-base.css rename to src/components/workflow-diagram/reactflow-base.css diff --git a/src/components/workflow-diagram/workflowDiagramConstants.ts b/src/components/workflow-diagram/workflowDiagramConstants.ts new file mode 100644 index 00000000..2379a8b1 --- /dev/null +++ b/src/components/workflow-diagram/workflowDiagramConstants.ts @@ -0,0 +1,39 @@ +/** Rendered width of each workflow node card (px). */ +export const nodeWidth = 256; + +/** Rendered height of each workflow node card (px). */ +export const nodeHeight = 44; + +/** + * Extra padding around each node card where edge turns are forbidden (px). + * Prevents bends from visually overlapping node borders. + */ +export const turnNodePadding = 12; + +/** + * Minimum horizontal distance of the final straight segment into the target + * handle (px). Ensures the incoming edge is visually distinct from a vertical + * drop-in. + */ +export const minTargetApproach = 20; + +/** Horizontal distance between successive candidate bend lanes (px). */ +export const bendNudgeStep = 8; + +/** + * Maximum nudge steps to probe in each direction from the baseline bend lane. + * Total search range: +/-192px. + */ +export const bendNudgeMaxSteps = 24; + +/** + * Vertical tolerance for considering two nodes to be on the same row (px). + * Dagre coordinates can land on sub-pixel values, so we allow a tiny delta. + */ +export const sameRowTolerance = 1; + +/** + * Horizontal distance from a target node's left edge where the shared merge + * lane is placed (px). + */ +export const targetMergePadding = 20; diff --git a/src/components/WorkflowDiagram.test.ts b/src/components/workflow-diagram/workflowDiagramEdgePath.test.ts similarity index 100% rename from src/components/WorkflowDiagram.test.ts rename to src/components/workflow-diagram/workflowDiagramEdgePath.test.ts diff --git a/src/components/workflowDiagramEdgePath.ts b/src/components/workflow-diagram/workflowDiagramEdgePath.ts similarity index 61% rename from src/components/workflowDiagramEdgePath.ts rename to src/components/workflow-diagram/workflowDiagramEdgePath.ts index 9c071ec3..f0ec4b2e 100644 --- a/src/components/workflowDiagramEdgePath.ts +++ b/src/components/workflow-diagram/workflowDiagramEdgePath.ts @@ -1,23 +1,23 @@ -export type WorkflowDiagramNodeRect = { - height: number; - width: number; - x: number; - y: number; -}; - -type Point = { - x: number; - y: number; -}; - -// Extra padding around each node card where edge turns are not allowed. -const turnNodePadding = 12; -// Keep the final horizontal segment into the target handle visibly long. -const minTargetApproach = 20; -// Search granularity when probing for an alternate bend lane. -const bendNudgeStep = 8; -// Upper bound on probing attempts for an alternate bend lane. -const bendNudgeMaxSteps = 24; +import { + bendNudgeMaxSteps, + bendNudgeStep, + minTargetApproach, + turnNodePadding, +} from "./workflowDiagramConstants"; +import { + areCollinear, + dedupeConsecutivePoints, + dedupeNearPoints, + doesSegmentIntersectRectWithPadding, + isPointInAnyRectWithPadding, + isPointInRectWithPadding, + type Point, + type Rect, + simplifyCollinearPoints, + toPath, +} from "./workflowDiagramGeometry"; + +export type WorkflowDiagramNodeRect = Rect; /* Routing policy (left-to-right workflows): @@ -32,89 +32,10 @@ Routing policy (left-to-right workflows): - minimum visible target approach distance. 4. If invalid, probe nearby lanes and pick the nearest valid one. -This preserves the original routing as much as possible while fixing only the -connections that need adjustment. +`isCandidatePathValid` centralizes all route validity checks so baseline and +probed candidates use identical acceptance criteria. */ -const dedupeConsecutivePoints = (points: Point[]): Point[] => { - return points.filter((point, index) => { - const previous = points[index - 1]; - if (!previous) return true; - - return previous.x !== point.x || previous.y !== point.y; - }); -}; - -const dedupeNearPoints = (points: Point[], epsilon = 0.5): Point[] => { - return points.filter((point, index) => { - const previous = points[index - 1]; - if (!previous) return true; - - return ( - Math.abs(previous.x - point.x) > epsilon || - Math.abs(previous.y - point.y) > epsilon - ); - }); -}; - -const areCollinear = (pointA: Point, pointB: Point, pointC: Point): boolean => { - const crossProduct = - (pointB.x - pointA.x) * (pointC.y - pointA.y) - - (pointB.y - pointA.y) * (pointC.x - pointA.x); - - return Math.abs(crossProduct) < 0.01; -}; - -const simplifyCollinearPoints = (points: Point[]): Point[] => { - if (points.length <= 2) return points; - - const simplified: Point[] = [points[0]]; - - for (let index = 1; index < points.length - 1; index += 1) { - const previous = simplified[simplified.length - 1]; - const current = points[index]; - const next = points[index + 1]; - - if (!areCollinear(previous, current, next)) { - simplified.push(current); - } - } - - simplified.push(points[points.length - 1]); - return simplified; -}; - -const isPointInRectWithPadding = ( - point: Point, - nodeRect: WorkflowDiagramNodeRect, - padding: number, -): boolean => { - return ( - point.x >= nodeRect.x - padding && - point.x <= nodeRect.x + nodeRect.width + padding && - point.y >= nodeRect.y - padding && - point.y <= nodeRect.y + nodeRect.height + padding - ); -}; - -const isPointInAnyRectWithPadding = ( - point: Point, - nodeRects: WorkflowDiagramNodeRect[], - padding: number, -): boolean => { - return nodeRects.some((nodeRect) => - isPointInRectWithPadding(point, nodeRect, padding), - ); -}; - -const toPath = (points: Point[]): string => { - if (points.length === 0) return ""; - if (points.length === 1) return `M ${points[0].x},${points[0].y}`; - - const [start, ...rest] = points; - return `M ${start.x},${start.y} ${rest.map((point) => `L ${point.x},${point.y}`).join(" ")}`; -}; - const toInteriorDagrePoints = ( source: Point, target: Point, @@ -193,10 +114,7 @@ const getTurnPoints = (points: Point[]): Point[] => { return turns; }; -const isPathTurnSafe = ( - points: Point[], - nodeRects: WorkflowDiagramNodeRect[], -): boolean => { +const isPathTurnSafe = (points: Point[], nodeRects: Rect[]): boolean => { const turnPoints = getTurnPoints(points); return turnPoints.every( @@ -207,8 +125,8 @@ const isPathTurnSafe = ( const getBlockingRectsForPathTurns = ( points: Point[], - nodeRects: WorkflowDiagramNodeRect[], -): WorkflowDiagramNodeRect[] => { + nodeRects: Rect[], +): Rect[] => { const turnPoints = getTurnPoints(points); if (turnPoints.length === 0) return []; @@ -220,7 +138,7 @@ const getBlockingRectsForPathTurns = ( }; const isEndpointRect = ( - nodeRect: WorkflowDiagramNodeRect, + nodeRect: Rect, source: Point, target: Point, ): boolean => { @@ -230,47 +148,13 @@ const isEndpointRect = ( ); }; -const doesSegmentIntersectRectWithPadding = ( - segmentStart: Point, - segmentEnd: Point, - nodeRect: WorkflowDiagramNodeRect, - padding: number, -): boolean => { - const minX = nodeRect.x - padding; - const maxX = nodeRect.x + nodeRect.width + padding; - const minY = nodeRect.y - padding; - const maxY = nodeRect.y + nodeRect.height + padding; - - if (segmentStart.y === segmentEnd.y) { - const segmentY = segmentStart.y; - if (segmentY < minY || segmentY > maxY) return false; - - const segmentMinX = Math.min(segmentStart.x, segmentEnd.x); - const segmentMaxX = Math.max(segmentStart.x, segmentEnd.x); - - return segmentMaxX > minX && segmentMinX < maxX; - } - - if (segmentStart.x === segmentEnd.x) { - const segmentX = segmentStart.x; - if (segmentX < minX || segmentX > maxX) return false; - - const segmentMinY = Math.min(segmentStart.y, segmentEnd.y); - const segmentMaxY = Math.max(segmentStart.y, segmentEnd.y); - - return segmentMaxY > minY && segmentMinY < maxY; - } - - return false; -}; - const isPathSegmentSafe = ({ nodeRects, points, source, target, }: { - nodeRects: WorkflowDiagramNodeRect[]; + nodeRects: Rect[]; points: Point[]; source: Point; target: Point; @@ -303,11 +187,11 @@ const getBlockingRectsForPathSegments = ({ source, target, }: { - nodeRects: WorkflowDiagramNodeRect[]; + nodeRects: Rect[]; points: Point[]; source: Point; target: Point; -}): WorkflowDiagramNodeRect[] => { +}): Rect[] => { const obstacleRects = nodeRects.filter( (nodeRect) => !isEndpointRect(nodeRect, source, target), ); @@ -375,7 +259,7 @@ const buildBendXCandidates = ({ targetApproachYOffset = 0, }: { baselineX: number; - blockingRects: WorkflowDiagramNodeRect[]; + blockingRects: Rect[]; source: Point; target: Point; targetApproachYOffset?: number; @@ -386,8 +270,8 @@ const buildBendXCandidates = ({ candidates.add(targetApproachBoundary({ source, target })); } - // Try lanes just outside blocking node bounds first; these often produce the - // smallest visible adjustment away from the baseline route. + // Try lanes just outside blocking node bounds first; these usually produce + // the smallest visual change away from the baseline route. blockingRects.forEach((nodeRect) => { candidates.add(nodeRect.x - turnNodePadding); candidates.add(nodeRect.x + nodeRect.width + turnNodePadding); @@ -399,14 +283,39 @@ const buildBendXCandidates = ({ candidates.add(baselineX + offset); } - // Keep search deterministic and local by preferring candidates nearest to the - // baseline lane before trying farther nudges. + // Keep search deterministic and local by prioritizing lanes nearest to the + // original baseline before trying farther nudges. return [...candidates].sort( (candidateA, candidateB) => Math.abs(candidateA - baselineX) - Math.abs(candidateB - baselineX), ); }; +const isCandidatePathValid = ({ + bendX, + nodeRects, + points, + source, + target, + targetApproachYOffset, +}: { + bendX: number; + nodeRects: Rect[]; + points: Point[]; + source: Point; + target: Point; + targetApproachYOffset: number; +}): boolean => { + if (!isPathTurnSafe(points, nodeRects)) return false; + if (!isPathSegmentSafe({ nodeRects, points, source, target })) return false; + + if (targetApproachYOffset === 0) { + return isTargetApproachVisible({ bendX, source, target }); + } + + return true; +}; + const chooseValidBendX = ({ baselineX, nodeRects, @@ -415,7 +324,7 @@ const chooseValidBendX = ({ targetApproachYOffset = 0, }: { baselineX: number; - nodeRects: WorkflowDiagramNodeRect[]; + nodeRects: Rect[]; source: Point; target: Point; targetApproachYOffset?: number; @@ -439,15 +348,16 @@ const chooseValidBendX = ({ const blockingRects = [ ...new Set([...blockingSegmentRects, ...blockingTurnRects]), ]; - const targetApproachIsValid = - targetApproachYOffset === 0 - ? isTargetApproachVisible({ bendX: baselineX, source, target }) - : true; if ( - isPathTurnSafe(baselinePath, nodeRects) && - isPathSegmentSafe({ nodeRects, points: baselinePath, source, target }) && - targetApproachIsValid + isCandidatePathValid({ + bendX: baselineX, + nodeRects, + points: baselinePath, + source, + target, + targetApproachYOffset, + }) ) { return baselineX; } @@ -467,15 +377,16 @@ const chooseValidBendX = ({ target, targetApproachYOffset, }); - const candidateTargetApproachIsValid = - targetApproachYOffset === 0 - ? isTargetApproachVisible({ bendX, source, target }) - : true; if ( - isPathTurnSafe(candidatePath, nodeRects) && - isPathSegmentSafe({ nodeRects, points: candidatePath, source, target }) && - candidateTargetApproachIsValid + isCandidatePathValid({ + bendX, + nodeRects, + points: candidatePath, + source, + target, + targetApproachYOffset, + }) ) { return bendX; } @@ -490,11 +401,12 @@ const toOrthogonalPoints = ( source: Point, target: Point, dagrePoints: Point[], - nodeRects: WorkflowDiagramNodeRect[], + nodeRects: Rect[], preferredBendX: number | undefined, targetApproachYOffset = 0, ): Point[] => { - // Straight line for same-row targets keeps simple cases unchanged. + // Straight same-row edges should stay straight unless a target approach + // offset explicitly asks for a merge lane detour. if (source.y === target.y && targetApproachYOffset === 0) { return [source, target]; } diff --git a/src/components/workflow-diagram/workflowDiagramGeometry.ts b/src/components/workflow-diagram/workflowDiagramGeometry.ts new file mode 100644 index 00000000..65f14d7e --- /dev/null +++ b/src/components/workflow-diagram/workflowDiagramGeometry.ts @@ -0,0 +1,128 @@ +export type Point = { + x: number; + y: number; +}; + +export type Rect = { + height: number; + width: number; + x: number; + y: number; +}; + +export const dedupeConsecutivePoints = (points: Point[]): Point[] => { + return points.filter((point, index) => { + const previous = points[index - 1]; + if (!previous) return true; + + return previous.x !== point.x || previous.y !== point.y; + }); +}; + +export const dedupeNearPoints = (points: Point[], epsilon = 0.5): Point[] => { + return points.filter((point, index) => { + const previous = points[index - 1]; + if (!previous) return true; + + return ( + Math.abs(previous.x - point.x) > epsilon || + Math.abs(previous.y - point.y) > epsilon + ); + }); +}; + +export const areCollinear = ( + pointA: Point, + pointB: Point, + pointC: Point, +): boolean => { + const crossProduct = + (pointB.x - pointA.x) * (pointC.y - pointA.y) - + (pointB.y - pointA.y) * (pointC.x - pointA.x); + + return Math.abs(crossProduct) < 0.01; +}; + +export const simplifyCollinearPoints = (points: Point[]): Point[] => { + if (points.length <= 2) return points; + + const simplified: Point[] = [points[0]]; + + for (let index = 1; index < points.length - 1; index += 1) { + const previous = simplified[simplified.length - 1]; + const current = points[index]; + const next = points[index + 1]; + + if (!areCollinear(previous, current, next)) { + simplified.push(current); + } + } + + simplified.push(points[points.length - 1]); + return simplified; +}; + +export const toPath = (points: Point[]): string => { + if (points.length === 0) return ""; + if (points.length === 1) return `M ${points[0].x},${points[0].y}`; + + const [start, ...rest] = points; + return `M ${start.x},${start.y} ${rest.map((point) => `L ${point.x},${point.y}`).join(" ")}`; +}; + +export const isPointInRectWithPadding = ( + point: Point, + rect: Rect, + padding: number, +): boolean => { + return ( + point.x >= rect.x - padding && + point.x <= rect.x + rect.width + padding && + point.y >= rect.y - padding && + point.y <= rect.y + rect.height + padding + ); +}; + +export const isPointInAnyRectWithPadding = ( + point: Point, + rects: Rect[], + padding: number, +): boolean => { + return rects.some((rect) => isPointInRectWithPadding(point, rect, padding)); +}; + +export const doesSegmentIntersectRectWithPadding = ( + segmentStart: Point, + segmentEnd: Point, + rect: Rect, + padding: number, +): boolean => { + const minX = rect.x - padding; + const maxX = rect.x + rect.width + padding; + const minY = rect.y - padding; + const maxY = rect.y + rect.height + padding; + + // The routing layer only emits orthogonal segments. Any diagonal segment is + // treated as non-intersecting here because it indicates an invalid caller. + if (segmentStart.y === segmentEnd.y) { + const segmentY = segmentStart.y; + if (segmentY < minY || segmentY > maxY) return false; + + const segmentMinX = Math.min(segmentStart.x, segmentEnd.x); + const segmentMaxX = Math.max(segmentStart.x, segmentEnd.x); + + return segmentMaxX > minX && segmentMinX < maxX; + } + + if (segmentStart.x === segmentEnd.x) { + const segmentX = segmentStart.x; + if (segmentX < minX || segmentX > maxX) return false; + + const segmentMinY = Math.min(segmentStart.y, segmentEnd.y); + const segmentMaxY = Math.max(segmentStart.y, segmentEnd.y); + + return segmentMaxY > minY && segmentMinY < maxY; + } + + return false; +}; diff --git a/src/components/workflow-diagram/workflowDiagramGraphModel.test.ts b/src/components/workflow-diagram/workflowDiagramGraphModel.test.ts new file mode 100644 index 00000000..d942ddcd --- /dev/null +++ b/src/components/workflow-diagram/workflowDiagramGraphModel.test.ts @@ -0,0 +1,245 @@ +import type { Edge } from "@xyflow/react"; + +import { JobWithKnownMetadata } from "@services/jobs"; +import { JobState } from "@services/types"; +import { describe, expect, it } from "vitest"; + +import { + applyEdgeVisuals, + buildWorkflowGraphModel, + depStatusFromJob, +} from "./workflowDiagramGraphModel"; + +const baseDate = new Date("2025-01-01T00:00:00.000Z"); + +const workflowJob = ({ + deps = [], + id, + state = JobState.Available, + task, + workflowID = "wf-1", +}: { + deps?: string[]; + id: number; + state?: JobState; + task: string; + workflowID?: string; +}): JobWithKnownMetadata => ({ + args: {}, + attempt: 0, + attemptedAt: undefined, + attemptedBy: [], + createdAt: baseDate, + errors: [], + finalizedAt: undefined, + id: BigInt(id), + kind: `job-${task}`, + logs: {}, + maxAttempts: 1, + metadata: { + deps, + task, + workflow_id: workflowID, + workflow_staged_at: baseDate.toISOString(), + }, + priority: 1, + queue: "default", + scheduledAt: baseDate, + state, + tags: [], +}); + +describe("buildWorkflowGraphModel", () => { + it("returns one node and zero edges for a single task", () => { + const model = buildWorkflowGraphModel([workflowJob({ id: 1, task: "a" })]); + + expect(model.nodes).toHaveLength(1); + expect(model.edges).toHaveLength(0); + expect(model.nodes[0].id).toBe("1"); + }); + + it("builds dependency edges in deterministic task/dep order", () => { + const tasks = [ + workflowJob({ id: 1, task: "task-a" }), + workflowJob({ deps: ["task-a"], id: 2, task: "task-b" }), + workflowJob({ deps: ["task-a", "task-b"], id: 3, task: "task-c" }), + ]; + + const model = buildWorkflowGraphModel(tasks); + + expect(model.edges.map((edge) => edge.id)).toEqual([ + "e-1-2", + "e-1-3", + "e-2-3", + ]); + expect(model.edges.map((edge) => `${edge.source}->${edge.target}`)).toEqual( + ["1->2", "1->3", "2->3"], + ); + }); + + it("maps job states to dependency statuses", () => { + expect( + depStatusFromJob( + workflowJob({ id: 1, state: JobState.Completed, task: "a" }), + ), + ).toBe("unblocked"); + expect( + depStatusFromJob( + workflowJob({ id: 2, state: JobState.Cancelled, task: "b" }), + ), + ).toBe("failed"); + expect( + depStatusFromJob( + workflowJob({ id: 3, state: JobState.Discarded, task: "c" }), + ), + ).toBe("failed"); + expect( + depStatusFromJob( + workflowJob({ id: 4, state: JobState.Running, task: "d" }), + ), + ).toBe("blocked"); + }); + + it("drops missing dependency targets", () => { + const tasks = [ + workflowJob({ id: 1, task: "existing" }), + workflowJob({ deps: ["missing", "existing"], id: 2, task: "consumer" }), + ]; + + const model = buildWorkflowGraphModel(tasks); + + expect(model.edges).toHaveLength(1); + expect(model.edges[0].id).toBe("e-1-2"); + }); + + it("sets upstream and downstream flags on node data", () => { + const tasks = [ + workflowJob({ id: 1, task: "a" }), + workflowJob({ deps: ["a"], id: 2, task: "b" }), + workflowJob({ deps: ["b"], id: 3, task: "c" }), + ]; + + const model = buildWorkflowGraphModel(tasks); + const nodeByID = new Map(model.nodes.map((node) => [node.id, node])); + + expect(nodeByID.get("1")?.data.hasUpstreamDeps).toBe(false); + expect(nodeByID.get("1")?.data.hasDownstreamDeps).toBe(true); + + expect(nodeByID.get("2")?.data.hasUpstreamDeps).toBe(true); + expect(nodeByID.get("2")?.data.hasDownstreamDeps).toBe(true); + + expect(nodeByID.get("3")?.data.hasUpstreamDeps).toBe(true); + expect(nodeByID.get("3")?.data.hasDownstreamDeps).toBe(false); + }); + + it("animates only blocked dependencies when downstream job is pending", () => { + const tasks = [ + workflowJob({ id: 1, state: JobState.Available, task: "source-blocked" }), + workflowJob({ + id: 2, + state: JobState.Completed, + task: "source-unblocked", + }), + workflowJob({ + deps: ["source-blocked"], + id: 3, + state: JobState.Pending, + task: "consumer-pending", + }), + workflowJob({ + deps: ["source-unblocked"], + id: 4, + state: JobState.Pending, + task: "consumer-pending-unblocked", + }), + workflowJob({ + deps: ["source-blocked"], + id: 5, + state: JobState.Cancelled, + task: "consumer-not-waiting", + }), + ]; + + const model = buildWorkflowGraphModel(tasks); + const edgeByID = new Map(model.edges.map((edge) => [edge.id, edge])); + + expect(edgeByID.get("e-1-3")?.animated).toBe(true); + expect(edgeByID.get("e-2-4")?.animated).toBe(false); + expect(edgeByID.get("e-1-5")?.animated).toBe(false); + }); + + it("returns empty arrays for empty input", () => { + const model = buildWorkflowGraphModel([]); + + expect(model.nodes).toEqual([]); + expect(model.edges).toEqual([]); + }); + + it("treats missing metadata.deps as an empty dependency list", () => { + const malformedJob = workflowJob({ id: 1, task: "a" }); + ( + malformedJob.metadata as unknown as { + deps?: string[]; + } + ).deps = undefined; + + const model = buildWorkflowGraphModel([malformedJob]); + + expect(model.nodes).toHaveLength(1); + expect(model.nodes[0].data.hasUpstreamDeps).toBe(false); + expect(model.edges).toEqual([]); + }); +}); + +describe("applyEdgeVisuals", () => { + it("applies status-specific styles and preserves edge order", () => { + const edges: Edge[] = [ + { + data: { depStatus: "blocked" }, + id: "blocked-edge", + source: "1", + target: "2", + }, + { + data: { depStatus: "failed" }, + id: "failed-edge", + source: "2", + target: "3", + }, + { + data: { depStatus: "unblocked" }, + id: "unblocked-edge", + source: "3", + target: "4", + }, + ]; + + const styledEdges = applyEdgeVisuals(edges, { + blocked: "#111111", + failed: "#222222", + unblocked: "#333333", + }); + + expect(styledEdges.map((edge) => edge.id)).toEqual([ + "blocked-edge", + "failed-edge", + "unblocked-edge", + ]); + + expect(styledEdges[0].style).toMatchObject({ + stroke: "#111111", + strokeDasharray: "6 3", + strokeWidth: 2, + }); + expect(styledEdges[1].style).toMatchObject({ + stroke: "#222222", + strokeDasharray: "6 3", + strokeWidth: 2, + }); + expect(styledEdges[2].style).toMatchObject({ + stroke: "#333333", + strokeDasharray: "0", + strokeWidth: 2, + }); + }); +}); diff --git a/src/components/workflow-diagram/workflowDiagramGraphModel.ts b/src/components/workflow-diagram/workflowDiagramGraphModel.ts new file mode 100644 index 00000000..24fdc979 --- /dev/null +++ b/src/components/workflow-diagram/workflowDiagramGraphModel.ts @@ -0,0 +1,154 @@ +import type { Edge, Node } from "@xyflow/react"; + +import { JobWithKnownMetadata } from "@services/jobs"; +import { JobState } from "@services/types"; + +import type { WorkflowNodeData } from "./WorkflowNode"; + +import { nodeHeight, nodeWidth } from "./workflowDiagramConstants"; +import { + getLayoutedElements, + type WorkflowDiagramNodeType, +} from "./workflowDiagramLayout"; +import { withPreferredTargetMergeX } from "./workflowDiagramMergeHints"; + +export type WorkflowDependencyStatus = "blocked" | "failed" | "unblocked"; + +export type WorkflowGraphModel = { + edges: Edge[]; + nodes: Node[]; +}; + +const depStatusFromEdgeData = ( + edgeData: unknown, +): undefined | WorkflowDependencyStatus => { + if (!edgeData || typeof edgeData !== "object") return undefined; + + const depStatus = (edgeData as { depStatus?: unknown }).depStatus; + if ( + depStatus === "blocked" || + depStatus === "failed" || + depStatus === "unblocked" + ) { + return depStatus; + } + + return undefined; +}; + +export const depStatusFromJob = ( + job: JobWithKnownMetadata, +): WorkflowDependencyStatus => { + switch (job.state) { + case JobState.Cancelled: + case JobState.Discarded: + return "failed"; + case JobState.Completed: + return "unblocked"; + default: + return "blocked"; + } +}; + +export const buildWorkflowGraphModel = ( + tasks: JobWithKnownMetadata[], +): WorkflowGraphModel => { + const jobsByTask = new Map(); + const tasksWithDownstreamDeps = new Set(); + + // Build dependency lookup structures in one pass so large workflows do not + // pay for repeated scans when building nodes and edges. + tasks.forEach((job) => { + jobsByTask.set(job.metadata.task, job); + + (job.metadata.deps ?? []).forEach((depTaskName) => { + tasksWithDownstreamDeps.add(depTaskName); + }); + }); + + const initialNodes: Node[] = + tasks.map((job) => { + // API payloads can omit `metadata.deps` for some historical rows. Treat + // missing deps as "no upstream dependencies" rather than crashing. + const deps = job.metadata.deps ?? []; + + return { + connectable: false, + data: { + hasDownstreamDeps: tasksWithDownstreamDeps.has(job.metadata.task), + hasUpstreamDeps: deps.length > 0, + job, + }, + height: nodeHeight, + id: job.id.toString(), + position: { x: 0, y: 0 }, + type: "workflowNode", + width: nodeWidth, + }; + }); + + const initialEdges = tasks.reduce((acc, job) => { + const dependencies = job.metadata.deps ?? []; + + dependencies.forEach((depName) => { + const dep = jobsByTask.get(depName); + + // Keep existing behavior: ignore references to tasks that are no longer + // in the current payload. + if (!dep) return; + + const depStatus = depStatusFromJob(dep); + // Animate only when the downstream job is currently waiting for upstream + // dependencies. This keeps cancelled/discarded downstream jobs visually + // static even if their upstream dependency is still blocked. + const isActivelyWaiting = job.state === JobState.Pending; + + acc.push({ + animated: depStatus === "blocked" && isActivelyWaiting, + data: { depStatus }, + id: `e-${dep.id}-${job.id}`, + source: dep.id.toString(), + target: job.id.toString(), + type: "workflowEdge", + }); + }); + + return acc; + }, []); + + const { edges: layoutedEdgesRaw, nodes: layoutedNodes } = getLayoutedElements( + initialNodes, + initialEdges, + "LR", + ); + + return { + edges: withPreferredTargetMergeX(layoutedEdgesRaw, layoutedNodes), + nodes: layoutedNodes, + }; +}; + +export const applyEdgeVisuals = ( + edges: Edge[], + edgeColors: Record, +): Edge[] => { + // Styling is intentionally split from graph building so theme changes only + // trigger this cheap map instead of a full Dagre layout pass. + return edges.map((edge) => { + const depStatus = depStatusFromEdgeData(edge.data) ?? "blocked"; + // Keep the prior visual language: + // - `unblocked`: solid line + // - `blocked` / `failed`: dashed line (color distinguishes failure) + const strokeDasharray = depStatus === "unblocked" ? "0" : "6 3"; + + return { + ...edge, + style: { + ...(edge.style || {}), + stroke: edgeColors[depStatus], + strokeDasharray, + strokeWidth: 2, + }, + }; + }); +}; diff --git a/src/components/workflow-diagram/workflowDiagramLayout.ts b/src/components/workflow-diagram/workflowDiagramLayout.ts new file mode 100644 index 00000000..6c43b495 --- /dev/null +++ b/src/components/workflow-diagram/workflowDiagramLayout.ts @@ -0,0 +1,91 @@ +import type { Edge, Node, Position } from "@xyflow/react"; + +import dagre from "@dagrejs/dagre"; + +import type { Point, Rect } from "./workflowDiagramGeometry"; +import type { WorkflowNodeData } from "./WorkflowNode"; + +import { nodeHeight, nodeWidth } from "./workflowDiagramConstants"; + +export type WorkflowDiagramNodeType = "workflowNode"; + +export const getLayoutedElements = ( + nodes: Node[], + edges: Edge[], + direction = "TB", +): { + edges: Edge[]; + nodes: Node[]; +} => { + const dagreGraph = new dagre.graphlib.Graph({ multigraph: true }); + dagreGraph.setDefaultEdgeLabel(() => ({})); + + const isHorizontal = direction === "LR"; + dagreGraph.setGraph({ + align: "UL", + edgesep: 100, + nodesep: 20, + rankdir: direction, + ranksep: 100, + }); + + nodes.forEach((node) => { + dagreGraph.setNode(node.id, { height: nodeHeight, width: nodeWidth }); + }); + + edges.forEach((edge) => { + dagreGraph.setEdge(edge.source, edge.target, {}, edge.id); + }); + + dagre.layout(dagreGraph); + + const layoutedNodes = nodes.map((node) => { + const nodeWithPosition = dagreGraph.node(node.id); + + return { + ...node, + // Dagre positions nodes by center; React Flow expects top-left coordinates. + position: { + x: nodeWithPosition.x - nodeWidth / 2, + y: nodeWithPosition.y - nodeHeight / 2, + }, + sourcePosition: (isHorizontal ? "right" : "bottom") as Position, + targetPosition: (isHorizontal ? "left" : "top") as Position, + }; + }); + + // Each edge reads from the same snapshot of node rectangles so collision + // checks are consistent regardless of render order. + const nodeRects: Rect[] = layoutedNodes.map((node) => ({ + height: node.height ?? node.measured?.height ?? nodeHeight, + width: node.width ?? node.measured?.width ?? nodeWidth, + x: node.position.x, + y: node.position.y, + })); + + const layoutedEdges = edges.map((edge) => { + const edgeWithPoints = dagreGraph.edge({ + name: edge.id, + v: edge.source, + w: edge.target, + }); + + const dagrePoints: Point[] = (edgeWithPoints?.points || []).map( + (point: { x: number; y: number }) => ({ + x: point.x, + y: point.y, + }), + ); + + return { + ...edge, + data: { + ...(edge.data as Record | undefined), + dagrePoints, + nodeRects, + }, + }; + }); + + return { edges: layoutedEdges, nodes: layoutedNodes }; +}; diff --git a/src/components/workflowDiagramMergeHints.test.ts b/src/components/workflow-diagram/workflowDiagramMergeHints.test.ts similarity index 100% rename from src/components/workflowDiagramMergeHints.test.ts rename to src/components/workflow-diagram/workflowDiagramMergeHints.test.ts diff --git a/src/components/workflowDiagramMergeHints.ts b/src/components/workflow-diagram/workflowDiagramMergeHints.ts similarity index 89% rename from src/components/workflowDiagramMergeHints.ts rename to src/components/workflow-diagram/workflowDiagramMergeHints.ts index 37161237..6f079701 100644 --- a/src/components/workflowDiagramMergeHints.ts +++ b/src/components/workflow-diagram/workflowDiagramMergeHints.ts @@ -1,8 +1,10 @@ import type { Edge, Node } from "@xyflow/react"; -const nodeHeight = 44; -const sameRowTolerance = 1; -const targetMergePadding = 20; +import { + nodeHeight, + sameRowTolerance, + targetMergePadding, +} from "./workflowDiagramConstants"; const nodeCenterY = (node: Node): number => { const renderedHeight = node.height ?? node.measured?.height ?? nodeHeight; @@ -53,6 +55,8 @@ export const withPreferredTargetMergeX = ( if (!hasSameRowIncoming || offRowEdges.length === 0) return; + // Only off-row incoming edges are nudged to a shared lane. Same-row edges + // keep their direct path into the target for readability. const preferredBendX = targetNode.position.x - targetMergePadding; offRowEdges.forEach((edge) => { preferredBendByEdgeID.set(edge.id, preferredBendX); From 4df7817b97c93ea4a8bc21155229d5bb45802c2d Mon Sep 17 00:00:00 2001 From: Blake Gentry Date: Wed, 11 Feb 2026 10:54:25 -0600 Subject: [PATCH 3/3] Share workflow job fixture across tests Workflow diagram tests in `WorkflowDiagram.test.tsx` and `workflowDiagramGraphModel.test.ts` each defined a local `workflowJob` fixture with nearly identical defaults. That duplication made workflow metadata drift likely when fields like `deps` or `workflow_id` changed in one file but not the other. Add a shared `workflowJobFactory` in `src/test/factories/workflowJob.ts`, built on top of `jobFactory`. The factory centralizes workflow-specific metadata defaults (`deps`, `task`, `workflow_id`, `workflow_staged_at`) while still allowing per-test overrides for state and dependency scenarios. Both workflow diagram test files now use the shared factory and no longer carry their own fixture builder logic. This keeps existing behavior coverage while reducing fixture boilerplate and making future test updates easier to apply consistently. --- .../workflow-diagram/WorkflowDiagram.test.tsx | 62 ++-------- .../workflowDiagramGraphModel.test.ts | 109 ++++++++---------- src/test/factories/workflowJob.ts | 48 ++++++++ 3 files changed, 109 insertions(+), 110 deletions(-) create mode 100644 src/test/factories/workflowJob.ts diff --git a/src/components/workflow-diagram/WorkflowDiagram.test.tsx b/src/components/workflow-diagram/WorkflowDiagram.test.tsx index 4c7227be..5b47fb63 100644 --- a/src/components/workflow-diagram/WorkflowDiagram.test.tsx +++ b/src/components/workflow-diagram/WorkflowDiagram.test.tsx @@ -1,52 +1,12 @@ import type { PropsWithChildren } from "react"; -import { JobWithKnownMetadata } from "@services/jobs"; -import { JobState } from "@services/types"; +import { workflowJobFactory } from "@test/factories/workflowJob"; import { act, render, screen } from "@testing-library/react"; import { beforeEach, describe, expect, it, vi } from "vitest"; import WorkflowDiagram from "./WorkflowDiagram"; import * as workflowDiagramLayout from "./workflowDiagramLayout"; -const baseDate = new Date("2025-01-01T00:00:00.000Z"); - -const workflowJob = ({ - deps = [], - id, - state = JobState.Available, - task, - workflowID = "wf-1", -}: { - deps?: string[]; - id: number; - state?: JobState; - task: string; - workflowID?: string; -}): JobWithKnownMetadata => ({ - args: {}, - attempt: 0, - attemptedAt: undefined, - attemptedBy: [], - createdAt: baseDate, - errors: [], - finalizedAt: undefined, - id: BigInt(id), - kind: `job-${task}`, - logs: {}, - maxAttempts: 1, - metadata: { - deps, - task, - workflow_id: workflowID, - workflow_staged_at: baseDate.toISOString(), - }, - priority: 1, - queue: "default", - scheduledAt: baseDate, - state, - tags: [], -}); - type MockReactFlowProps = PropsWithChildren<{ edges: unknown[]; nodes: unknown[]; @@ -95,9 +55,9 @@ describe("WorkflowDiagram", () => { it("renders nodes and edges for a workflow", () => { const tasks = [ - workflowJob({ id: 1, task: "a" }), - workflowJob({ deps: ["a"], id: 2, task: "b" }), - workflowJob({ deps: ["a", "b"], id: 3, task: "c" }), + workflowJobFactory.build({ id: 1, task: "a" }), + workflowJobFactory.build({ deps: ["a"], id: 2, task: "b" }), + workflowJobFactory.build({ deps: ["a", "b"], id: 3, task: "c" }), ]; render( @@ -115,8 +75,8 @@ describe("WorkflowDiagram", () => { it("calls setSelectedJobId when a node is selected", () => { const tasks = [ - workflowJob({ id: 1, task: "a" }), - workflowJob({ deps: ["a"], id: 2, task: "b" }), + workflowJobFactory.build({ id: 1, task: "a" }), + workflowJobFactory.build({ deps: ["a"], id: 2, task: "b" }), ]; const setSelectedJobID = vi.fn(); @@ -142,8 +102,8 @@ describe("WorkflowDiagram", () => { it("does not rerun layout when only selectedJobId changes", () => { const tasks = [ - workflowJob({ id: 1, task: "a" }), - workflowJob({ deps: ["a"], id: 2, task: "b" }), + workflowJobFactory.build({ id: 1, task: "a" }), + workflowJobFactory.build({ deps: ["a"], id: 2, task: "b" }), ]; const layoutSpy = vi.spyOn(workflowDiagramLayout, "getLayoutedElements"); @@ -170,8 +130,8 @@ describe("WorkflowDiagram", () => { it("does not rerun layout when only theme changes", () => { const tasks = [ - workflowJob({ id: 1, task: "a" }), - workflowJob({ deps: ["a"], id: 2, task: "b" }), + workflowJobFactory.build({ id: 1, task: "a" }), + workflowJobFactory.build({ deps: ["a"], id: 2, task: "b" }), ]; const layoutSpy = vi.spyOn(workflowDiagramLayout, "getLayoutedElements"); @@ -199,7 +159,7 @@ describe("WorkflowDiagram", () => { }); it("renders when metadata.deps is missing on a task", () => { - const malformedJob = workflowJob({ id: 1, task: "a" }); + const malformedJob = workflowJobFactory.build({ id: 1, task: "a" }); ( malformedJob.metadata as unknown as { deps?: string[]; diff --git a/src/components/workflow-diagram/workflowDiagramGraphModel.test.ts b/src/components/workflow-diagram/workflowDiagramGraphModel.test.ts index d942ddcd..3fbb3917 100644 --- a/src/components/workflow-diagram/workflowDiagramGraphModel.test.ts +++ b/src/components/workflow-diagram/workflowDiagramGraphModel.test.ts @@ -1,7 +1,7 @@ import type { Edge } from "@xyflow/react"; -import { JobWithKnownMetadata } from "@services/jobs"; import { JobState } from "@services/types"; +import { workflowJobFactory } from "@test/factories/workflowJob"; import { describe, expect, it } from "vitest"; import { @@ -10,48 +10,11 @@ import { depStatusFromJob, } from "./workflowDiagramGraphModel"; -const baseDate = new Date("2025-01-01T00:00:00.000Z"); - -const workflowJob = ({ - deps = [], - id, - state = JobState.Available, - task, - workflowID = "wf-1", -}: { - deps?: string[]; - id: number; - state?: JobState; - task: string; - workflowID?: string; -}): JobWithKnownMetadata => ({ - args: {}, - attempt: 0, - attemptedAt: undefined, - attemptedBy: [], - createdAt: baseDate, - errors: [], - finalizedAt: undefined, - id: BigInt(id), - kind: `job-${task}`, - logs: {}, - maxAttempts: 1, - metadata: { - deps, - task, - workflow_id: workflowID, - workflow_staged_at: baseDate.toISOString(), - }, - priority: 1, - queue: "default", - scheduledAt: baseDate, - state, - tags: [], -}); - describe("buildWorkflowGraphModel", () => { it("returns one node and zero edges for a single task", () => { - const model = buildWorkflowGraphModel([workflowJob({ id: 1, task: "a" })]); + const model = buildWorkflowGraphModel([ + workflowJobFactory.build({ id: 1, task: "a" }), + ]); expect(model.nodes).toHaveLength(1); expect(model.edges).toHaveLength(0); @@ -60,9 +23,13 @@ describe("buildWorkflowGraphModel", () => { it("builds dependency edges in deterministic task/dep order", () => { const tasks = [ - workflowJob({ id: 1, task: "task-a" }), - workflowJob({ deps: ["task-a"], id: 2, task: "task-b" }), - workflowJob({ deps: ["task-a", "task-b"], id: 3, task: "task-c" }), + workflowJobFactory.build({ id: 1, task: "task-a" }), + workflowJobFactory.build({ deps: ["task-a"], id: 2, task: "task-b" }), + workflowJobFactory.build({ + deps: ["task-a", "task-b"], + id: 3, + task: "task-c", + }), ]; const model = buildWorkflowGraphModel(tasks); @@ -80,30 +47,50 @@ describe("buildWorkflowGraphModel", () => { it("maps job states to dependency statuses", () => { expect( depStatusFromJob( - workflowJob({ id: 1, state: JobState.Completed, task: "a" }), + workflowJobFactory.build({ + id: 1, + state: JobState.Completed, + task: "a", + }), ), ).toBe("unblocked"); expect( depStatusFromJob( - workflowJob({ id: 2, state: JobState.Cancelled, task: "b" }), + workflowJobFactory.build({ + id: 2, + state: JobState.Cancelled, + task: "b", + }), ), ).toBe("failed"); expect( depStatusFromJob( - workflowJob({ id: 3, state: JobState.Discarded, task: "c" }), + workflowJobFactory.build({ + id: 3, + state: JobState.Discarded, + task: "c", + }), ), ).toBe("failed"); expect( depStatusFromJob( - workflowJob({ id: 4, state: JobState.Running, task: "d" }), + workflowJobFactory.build({ + id: 4, + state: JobState.Running, + task: "d", + }), ), ).toBe("blocked"); }); it("drops missing dependency targets", () => { const tasks = [ - workflowJob({ id: 1, task: "existing" }), - workflowJob({ deps: ["missing", "existing"], id: 2, task: "consumer" }), + workflowJobFactory.build({ id: 1, task: "existing" }), + workflowJobFactory.build({ + deps: ["missing", "existing"], + id: 2, + task: "consumer", + }), ]; const model = buildWorkflowGraphModel(tasks); @@ -114,9 +101,9 @@ describe("buildWorkflowGraphModel", () => { it("sets upstream and downstream flags on node data", () => { const tasks = [ - workflowJob({ id: 1, task: "a" }), - workflowJob({ deps: ["a"], id: 2, task: "b" }), - workflowJob({ deps: ["b"], id: 3, task: "c" }), + workflowJobFactory.build({ id: 1, task: "a" }), + workflowJobFactory.build({ deps: ["a"], id: 2, task: "b" }), + workflowJobFactory.build({ deps: ["b"], id: 3, task: "c" }), ]; const model = buildWorkflowGraphModel(tasks); @@ -134,25 +121,29 @@ describe("buildWorkflowGraphModel", () => { it("animates only blocked dependencies when downstream job is pending", () => { const tasks = [ - workflowJob({ id: 1, state: JobState.Available, task: "source-blocked" }), - workflowJob({ + workflowJobFactory.build({ + id: 1, + state: JobState.Available, + task: "source-blocked", + }), + workflowJobFactory.build({ id: 2, state: JobState.Completed, task: "source-unblocked", }), - workflowJob({ + workflowJobFactory.build({ deps: ["source-blocked"], id: 3, state: JobState.Pending, task: "consumer-pending", }), - workflowJob({ + workflowJobFactory.build({ deps: ["source-unblocked"], id: 4, state: JobState.Pending, task: "consumer-pending-unblocked", }), - workflowJob({ + workflowJobFactory.build({ deps: ["source-blocked"], id: 5, state: JobState.Cancelled, @@ -176,7 +167,7 @@ describe("buildWorkflowGraphModel", () => { }); it("treats missing metadata.deps as an empty dependency list", () => { - const malformedJob = workflowJob({ id: 1, task: "a" }); + const malformedJob = workflowJobFactory.build({ id: 1, task: "a" }); ( malformedJob.metadata as unknown as { deps?: string[]; diff --git a/src/test/factories/workflowJob.ts b/src/test/factories/workflowJob.ts new file mode 100644 index 00000000..a0aa7935 --- /dev/null +++ b/src/test/factories/workflowJob.ts @@ -0,0 +1,48 @@ +import { JobWithKnownMetadata } from "@services/jobs"; +import { JobState } from "@services/types"; +import { Factory } from "fishery"; + +import { jobFactory } from "./job"; + +const defaultWorkflowStagedAt = new Date("2025-01-01T00:00:00.000Z"); +const defaultWorkflowID = "wf-1"; + +type WorkflowJobFactoryParams = { + deps?: string[]; + id?: bigint | number; + state?: JobState; + task?: string; + workflowID?: string; + workflowStagedAt?: Date; +}; + +export const workflowJobFactory = Factory.define< + JobWithKnownMetadata, + object, + JobWithKnownMetadata, + WorkflowJobFactoryParams +>(({ params, sequence }) => { + const id = + typeof params.id === "bigint" ? params.id : BigInt(params.id ?? sequence); + + const task = params.task ?? `task-${id.toString()}`; + const workflowStagedAt = params.workflowStagedAt ?? defaultWorkflowStagedAt; + + const baseJob = jobFactory.build({ + createdAt: workflowStagedAt, + id, + kind: `job-${task}`, + scheduledAt: workflowStagedAt, + state: params.state ?? JobState.Available, + }); + + return { + ...baseJob, + metadata: { + deps: params.deps ?? [], + task, + workflow_id: params.workflowID ?? defaultWorkflowID, + workflow_staged_at: workflowStagedAt.toISOString(), + }, + }; +});