Preserve both endpoints when repairing devtools chart routes

.changeset/safe-chart-endpoints.md

@@ -0,0 +1,7 @@
+---
+"@typeonce/effect-machine-devtools": patch
+---
+
+Fix devtools charts failing to render retry transitions into compound states. Route repairs preserve the direction and clearance at both ends of each transition, including straight routes that need a detour to reach a state header.
+
+Keep the devtools platform dependencies on the supported Effect prerelease so fresh installations can start the CLI without missing-module errors.

packages/devtools/package.json

@@ -34,6 +34,7 @@
   "dependencies": {
     "@effect/platform-browser": "4.0.0-rc.112",
     "@effect/platform-node": "4.0.0-rc.112",
+    "@effect/platform-node-shared": "4.0.0-rc.112",
     "@typeonce/effect-machine": "workspace:^",
     "chokidar": "4.0.3",
     "elkjs": "0.12.0",

packages/devtools/src/internal/browser/chart-layout.ts

@@ -896,57 +896,67 @@ const shortenTransitionRoute = (
   return candidate
 }
 
-const normalizeTerminalDirection = (
-  edge: ChartEdge,
+// Both repairs use a route oriented away from the endpoint being repaired.
+// An attached opposite endpoint is a constraint, including on two-point routes
+// where replacing the connection would otherwise replace both endpoint steps.
+const normalizeEndpointDirection = (
   points: ReadonlyArray<ChartPoint>,
-  nodes: ReadonlyMap<string, LaidOutChartNode>,
-  allNodes: ReadonlyArray<LaidOutChartNode>
+  node: LaidOutChartNode,
+  opposite: LaidOutChartNode | undefined,
+  obstacles: ReadonlyArray<ChartRect>
 ): ReadonlyArray<ChartPoint> => {
-  if (edge.target === null || isSelfTransition(edge) || points.length < 2) return points
-  const target = nodes.get(edge.target)
-  if (target === undefined) return points
-  const rawEnd = points.at(-1)!
-  const side = endpointSide(rawEnd, target)
-  const end = pointOnNodeBoundary(rawEnd, target, side)
-  const prefix = points.slice(0, -1)
-  const previous = prefix.at(-1)!
-  const attached = compactPoints([...prefix, end])
-  if (isOutwardStep(end, previous, side)) return attached
-
-  const targetStub = outwardPoint(end, side, 18)
-  const obstacles = [...routeObstacles(edge, allNodes), routeNodeRect(target)]
-  return endpointConnections(previous, targetStub, routeNodeRect(target), side)
-    .filter((connection) =>
-      !connection.slice(0, -1).some((point) => point.x === end.x && point.y === end.y) &&
-      routeIsClear([...connection, end], obstacles)
+  const rawStart = points[0]!
+  const side = endpointStepSide(rawStart, points[1]!, node)
+  const start = pointOnNodeBoundary(rawStart, node, side)
+  const end = points.at(-1)!
+  const previous = points.at(-2)!
+  const oppositeSide = opposite === undefined ? undefined : endpointStepSide(end, previous, opposite)
+  const preserveOpposite = opposite !== undefined && oppositeSide !== undefined &&
+    nodeBoundaryDistance(end, opposite) <= 0.5 && isOutwardStep(end, previous, oppositeSide)
+  const validEndpoints = (route: ReadonlyArray<ChartPoint>): boolean =>
+    isOutwardStep(route[0]!, route[1]!, side) && chartRouteLength(route.slice(0, 2)) >= 9 &&
+    (!preserveOpposite ||
+      isOutwardStep(route.at(-1)!, route.at(-2)!, oppositeSide!) && chartRouteLength(route.slice(-2)) >= 9)
+
+  const tail = points.slice(1)
+  const attached = compactPoints([start, ...tail])
+  if (attached.length >= 2 && validEndpoints(attached)) return attached
+
+  if (tail.length === 1 && preserveOpposite) {
+    tail.unshift(outwardPoint(end, oppositeSide!, 18))
+  }
+  const stub = outwardPoint(start, side, 18)
+  const endpointObstacles = [
+    ...obstacles,
+    routeNodeRect(node),
+    ...(preserveOpposite ? [routeNodeRect(opposite!)] : [])
+  ]
+  return endpointConnections(stub, tail[0]!, routeNodeRect(node), side)
+    .map((connection) => compactPoints([start, ...connection, ...tail.slice(1)]))
+    .filter((route) =>
+      route.length >= 2 && validEndpoints(route) &&
+      !route.slice(1).some((point) => point.x === start.x && point.y === start.y) &&
+      routeIsClear(route, endpointObstacles)
     )
-    .map((connection) => compactPoints([...prefix, ...connection.slice(1), end]))
     .sort((left, right) => chartRouteLength(left) - chartRouteLength(right))[0] ?? attached
 }
 
-const normalizeSourceDirection = (
+const normalizeTransitionEndpoints = (
   edge: ChartEdge,
   points: ReadonlyArray<ChartPoint>,
   nodes: ReadonlyMap<string, LaidOutChartNode>,
   allNodes: ReadonlyArray<LaidOutChartNode>
 ): ReadonlyArray<ChartPoint> => {
   if (isSelfTransition(edge) || points.length < 2) return points
   const source = nodes.get(edge.source)
-  if (source === undefined || source.node.children.length > 0) return points
-  const rawStart = points[0]!
-  const side = endpointSide(rawStart, source)
-  const start = pointOnNodeBoundary(rawStart, source, side)
-  const tail = points.slice(1)
-  const next = tail[0]!
-  const attached = compactPoints([start, ...tail])
-  if (isOutwardStep(start, next, side)) return attached
-
-  const sourceStub = outwardPoint(start, side, 18)
-  const obstacles = [...routeObstacles(edge, allNodes), routeNodeRect(source)]
-  return endpointConnections(sourceStub, next, routeNodeRect(source), side)
-    .filter((connection) => routeIsClear([start, ...connection, ...tail.slice(1)], obstacles))
-    .map((connection) => compactPoints([start, ...connection, ...tail.slice(1)]))
-    .sort((left, right) => chartRouteLength(left) - chartRouteLength(right))[0] ?? attached
+  const target = edge.target === null ? undefined : nodes.get(edge.target)
+  const obstacles = routeObstacles(edge, allNodes)
+  const fromSource = source === undefined || source.node.children.length > 0
+    ? points
+    : normalizeEndpointDirection(points, source, target, obstacles)
+  return target === undefined
+    ? fromSource
+    : [...normalizeEndpointDirection([...fromSource].reverse(), target, source, obstacles)].reverse()
 }
 
 const headerDetour = (
@@ -1207,24 +1217,19 @@ const collectLayout = (
       if (elkPoints === undefined) return []
       const chartEdge = chartEdges.get(edge.id)
       if (chartEdge === undefined) return []
-      const points = normalizeTerminalDirection(
+      const points = normalizeTransitionEndpoints(
         chartEdge,
-        normalizeSourceDirection(
+        avoidCompoundHeaders(
           chartEdge,
-          avoidCompoundHeaders(
+          (shortenRoutes ? shortenTransitionRoute : (_edge: ChartEdge, route: ReadonlyArray<ChartPoint>) => route)(
             chartEdge,
-            (shortenRoutes ? shortenTransitionRoute : (_edge: ChartEdge, route: ReadonlyArray<ChartPoint>) => route)(
-              chartEdge,
-              normalizeHierarchyRoute(chartEdge, elkPoints, nodesByPath, nodes, hierarchyLanes),
-              nodesByPath,
-              nodes,
-              directLanes
-            ),
+            normalizeHierarchyRoute(chartEdge, elkPoints, nodesByPath, nodes, hierarchyLanes),
+            nodesByPath,
             nodes,
-            hierarchyLanes
+            directLanes
           ),
-          nodesByPath,
-          nodes
+          nodes,
+          hierarchyLanes
         ),
         nodesByPath,
         nodes
@@ -1700,7 +1705,7 @@ export const layoutChartWith = (
           new ChartLayoutError({
             cause: { failures, invalid },
             message:
-              `ELK did not produce a safe layout for ${model.machineId} after ${layoutProfiles.length} deterministic attempts: ${detail}`
+              `Chart routing did not produce a safe layout for ${model.machineId} after ${layoutProfiles.length} deterministic attempts: ${detail}`
           })
         )
       }

packages/devtools/test/internal/browser/ChartEndpoints.test.ts

@@ -0,0 +1,182 @@
+import { assert, describe, it } from "@effect/vitest"
+import { Machine } from "@typeonce/effect-machine"
+import { Effect, Schema } from "effect"
+import type { ElkNode } from "elkjs/lib/elk-api.js"
+import {
+  type ChartPoint,
+  chartRouteLength,
+  layoutChart,
+  layoutChartWith,
+  validateChartLayout
+} from "../../../src/internal/browser/chart-layout.js"
+import { type ChartModel, makeChartModel } from "../../../src/internal/browser/chart-model.js"
+import * as MachineDocument from "../../../src/MachineDocument.js"
+
+const copyButtonModel = (): ChartModel => {
+  const root = Machine.state({
+    states: {
+      Idle: {},
+      Working: { fields: { text: Schema.String }, states: { Clicked: {}, Copying: {} } },
+      Copied: {},
+      Failed: { fields: { message: Schema.String } }
+    }
+  })
+  const targets = Machine.targets(root)
+  const machine = Machine.make({
+    id: "CopyButton",
+    root,
+    events: Machine.events({ Copy: { text: Schema.String } }),
+    effects: { copy: (text: string) => text.length > 0 ? Effect.void : Effect.fail({ message: "failure" }) },
+    timers: { clicked: "120 millis", confirmation: "2 seconds", errorFeedback: "1 second" }
+  }).handle({
+    initial: { target: targets.root.Idle },
+    states: {
+      Idle: {
+        on: {
+          Copy: {
+            target: targets.root.Working,
+            guard: ({ event }) => event.text.length > 0,
+            data: ({ event }) => ({ text: event.text })
+          }
+        }
+      },
+      Working: {
+        initial: { target: targets.root.Working.Clicked },
+        invoke: {
+          src: "copy",
+          input: ({ state }) => state.text,
+          onDone: { target: targets.root.Copied },
+          onFailure: { target: targets.root.Failed, data: ({ error }) => ({ message: error.message }) }
+        },
+        states: {
+          Clicked: { invoke: { src: "clicked", onDone: { target: targets.root.Working.Copying } } },
+          Copying: {}
+        }
+      },
+      Copied: { invoke: { src: "confirmation", onDone: { target: targets.root.Idle } } },
+      Failed: {
+        invoke: { src: "errorFeedback", onDone: { target: targets.root.Idle } },
+        on: {
+          Copy: {
+            target: targets.root.Working,
+            guard: ({ event }) => event.text.length > 0,
+            data: ({ event }) => ({ text: event.text })
+          }
+        }
+      }
+    }
+  })
+  return makeChartModel(MachineDocument.make(machine))
+}
+
+const model = copyButtonModel()
+const variants: ReadonlyArray<readonly [string, ChartModel]> = [
+  ["original", model],
+  ["reversed declarations", { ...model, nodes: [...model.nodes].reverse() }],
+  ["reversed edges", { ...model, edges: [...model.edges].reverse() }],
+  ["long labels", {
+    ...model,
+    edges: model.edges.map((edge) => ({ ...edge, label: edge.label + " with additional feedback details" }))
+  }],
+  ["without failure feedback", {
+    ...model,
+    edges: model.edges.filter((edge) => !(edge.source === "Failed" && edge.target === "Idle"))
+  }],
+  ["without success feedback", {
+    ...model,
+    edges: model.edges.filter((edge) => !(edge.source === "Copied" && edge.target === "Idle"))
+  }]
+]
+
+describe("chart route endpoints", () => {
+  it.each(variants)("routes CopyButton with %s", async (_name, model) => {
+    const layout = await Effect.runPromise(layoutChart(model))
+    const repeated = await Effect.runPromise(layoutChart(model))
+    assert.deepStrictEqual(validateChartLayout(model, layout).issues, [])
+    assert.deepStrictEqual(repeated, layout)
+    const retry = layout.edges.find((edge) =>
+      edge.kind === "transition" && edge.edge.source === "Failed" && edge.edge.target === "Working"
+    )
+    assert.isDefined(retry)
+    assert.isAtLeast(chartRouteLength(retry!.points.slice(0, 2)), 9)
+    assert.isAtLeast(chartRouteLength(retry!.points.slice(-2)), 9)
+  })
+
+  // Rotate the same geometry to exercise every side, then reverse the edge to
+  // exercise source repair and terminal repair through the full layout pipeline.
+  for (const turns of [0, 1, 2, 3]) {
+    for (const reverse of [false, true]) {
+      it(`preserves the opposite endpoint with ${turns} quarter turns and reverse=${reverse}`, async () => {
+        const rotate = (point: ChartPoint): ChartPoint => {
+          let result = point
+          for (let turn = 0; turn < turns; turn++) result = { x: 600 - result.y, y: result.x }
+          return result
+        }
+        const rect = (id: string, x: number, y: number, width: number, height: number): ElkNode => {
+          const start = rotate({ x, y })
+          const end = rotate({ x: x + width, y: y + height })
+          return {
+            id,
+            x: Math.min(start.x, end.x),
+            y: Math.min(start.y, end.y),
+            width: Math.abs(start.x - end.x),
+            height: Math.abs(start.y - end.y)
+          }
+        }
+        const points = [{ x: 250, y: 400 }, { x: 250, y: 300 }].map(rotate)
+        if (reverse) points.reverse()
+        const model: ChartModel = {
+          machineId: "endpoint-repair",
+          roots: ["A", "B"],
+          nodes: ["A", "B"].map((path) => ({
+            path,
+            label: path,
+            type: "atomic",
+            parent: null,
+            children: [],
+            active: false,
+            initial: false,
+            activities: []
+          })),
+          edges: [{
+            id: "edge",
+            transitionId: "edge",
+            branchIds: ["edge"],
+            source: reverse ? "B" : "A",
+            target: reverse ? "A" : "B",
+            kind: "target",
+            label: "go",
+            accessibleLabel: "go",
+            badges: [],
+            trigger: { type: "event", event: "go" },
+            activityKind: null,
+            reenter: false,
+            acceptance: "required"
+          }],
+          runtimeTargets: [],
+          initials: []
+        }
+        const graph: ElkNode = {
+          id: "graph",
+          width: 600,
+          height: 600,
+          children: [rect("A", 200, 400, 180, 88), rect("B", 100, 100, 120, 200)],
+          edges: [{
+            id: "edge",
+            sources: [model.edges[0]!.source],
+            targets: [model.edges[0]!.target!],
+            sections: [{ id: "section", startPoint: points[0]!, endPoint: points[1]! }]
+          }]
+        }
+        const layout = await Effect.runPromise(layoutChartWith(model, async () => graph))
+        assert.deepStrictEqual(validateChartLayout(model, layout).issues, [])
+        const route = layout.edges[0]!.points
+        assert.isAtLeast(chartRouteLength(route.slice(0, 2)), 9)
+        assert.isAtLeast(chartRouteLength(route.slice(-2)), 9)
+        for (let index = 1; index < route.length; index++) {
+          assert.isTrue(route[index - 1]!.x === route[index]!.x || route[index - 1]!.y === route[index]!.y)
+        }
+      })
+    }
+  }
+})

scripts/release-contract.test.mjs

@@ -32,3 +32,14 @@ test("all Effect Machine packages release with the same version", async () => {
     "all Effect Machine packages must remain in the same Changesets fixed group"
   )
 })
+
+test("devtools pins its platform packages to the supported Effect prerelease", async () => {
+  const devtools = await readJson("packages/devtools/package.json")
+  for (const name of ["@effect/platform-browser", "@effect/platform-node", "@effect/platform-node-shared"]) {
+    assert.equal(
+      devtools.dependencies[name],
+      devtools.peerDependencies.effect,
+      `${name} must match the supported Effect version`
+    )
+  }
+})