From 2b16bb4837f5e4dc068b9f8c210f612d66c9891e Mon Sep 17 00:00:00 2001
From: Bobby Powers <bobbypowers@gmail.com>
Date: Wed, 1 Jul 2026 09:17:04 -0700
Subject: [PATCH 1/4] diagram: offset stock-to-stock flows into a Z on
 perpendicular drag

A straight 2-point flow pinned between two stocks had no reroute path
at all: the L-shape conversion required a cloud endpoint, and segment
drags require interior segments. A perpendicular-dominant valve drag
now inserts two corners at the dragged coordinate, keeping both
endpoints pinned to their stock edges; the middle segment is then a
normal interior segment (draggable, and dragging it back onto the
axis collapses the flow to straight via normalizeFlowPoints).

Stock moves preserve a Z's occupied edge: computeFlowRoute and
getFlowAttachmentInfo share one side classifier
(resolveStockAdjacentSide) that keeps an endpoint on its current edge
when the stock-adjacent segment runs parallel to it (the Z riser), so
nudges neither jump the endpoint to another edge nor collide spread
slots with co-located flows. The valve clamps to the Z's middle
segment specifically. LAYOUT.md's stale 20px valve-clearance prose is
corrected to the actual VALVE_CLAMP_MARGIN (10px).
---
 src/diagram/LAYOUT.md                    |   4 +-
 src/diagram/drawing/Flow.tsx             | 200 +++++++++++---
 src/diagram/group-movement.ts            |  44 ++-
 src/diagram/tests/flow-routing.test.ts   | 328 +++++++++++++++++++++++
 src/diagram/tests/group-movement.test.ts |  99 ++++++-
 5 files changed, 618 insertions(+), 57 deletions(-)

diff --git a/src/diagram/LAYOUT.md b/src/diagram/LAYOUT.md
index 785b52348..98ba11016 100644
--- a/src/diagram/LAYOUT.md
+++ b/src/diagram/LAYOUT.md
@@ -444,7 +444,7 @@ Flows connect stocks and clouds with specific constraints:
    - Maintain axis alignment
    - Constrain valve position within valid range
 3. Update cloud positions to follow flow movement
-4. Keep minimum 20px clearance from flow endpoints
+4. Keep minimum 10px clearance from flow endpoints (`VALVE_CLAMP_MARGIN`)
 
 ### Connector Rules
 
@@ -652,7 +652,7 @@ Each element type has specific hit testing:
 1. **Spacing**: Minimum 20px between element edges
 2. **Grid alignment**: Snap to 5px or 10px grid for cleaner layouts
 3. **Label clearance**: Account for label bounds when positioning
-4. **Flow clearance**: Keep valve at least 20px from connected elements
+4. **Flow clearance**: The valve clamps at least 10px (`VALVE_CLAMP_MARGIN`) from flow endpoints
 
 ### Layout Strategies
 1. **Hierarchical**: Arrange in levels (sources → stocks → sinks)
diff --git a/src/diagram/drawing/Flow.tsx b/src/diagram/drawing/Flow.tsx
index 32246baed..1fa68fd33 100644
--- a/src/diagram/drawing/Flow.tsx
+++ b/src/diagram/drawing/Flow.tsx
@@ -84,6 +84,76 @@ function getStockEdgePoint(stockCx: number, stockCy: number, side: Side, offsetF
   }
 }
 
+/**
+ * Which stock edge a flow endpoint currently occupies relative to a given stock
+ * center, or undefined if the point is not on an edge (within tolerance).
+ *
+ * Used to preserve the attachment edge across a stock move when the
+ * stock-adjacent segment runs PARALLEL to that edge -- a Z-shaped flow's riser
+ * hugs a left/right edge with a vertical segment (or a top/bottom edge with a
+ * horizontal one). Classifying by segment orientation alone would misread that
+ * as the perpendicular edge and jump the endpoint on a mere nudge (#819
+ * follow-up). Endpoints are pinned exactly onto edges, so a small tolerance is
+ * safe and avoids float noise.
+ */
+function classifyStockEdge(point: IPoint, center: IPoint): Side | undefined {
+  const EPS = 0.5;
+  const dx = point.x - center.x;
+  const dy = point.y - center.y;
+  const onLeftRightEdge = Math.abs(Math.abs(dx) - StockWidth / 2) <= EPS && Math.abs(dy) <= StockHeight / 2 + EPS;
+  const onTopBottomEdge = Math.abs(Math.abs(dy) - StockHeight / 2) <= EPS && Math.abs(dx) <= StockWidth / 2 + EPS;
+  if (onLeftRightEdge) {
+    return dx >= 0 ? 'right' : 'left';
+  }
+  if (onTopBottomEdge) {
+    return dy >= 0 ? 'bottom' : 'top';
+  }
+  return undefined;
+}
+
+/**
+ * Determine which stock edge a multi-segment (3+ point) flow's stock endpoint
+ * attaches to after a stock move, plus whether the stock-adjacent segment is
+ * horizontal. Shared by computeFlowRoute (which re-pins the endpoint) and
+ * getFlowAttachmentInfo (which groups flows for spread spacing) so the two
+ * ALWAYS agree on a Z-shaped flow's occupied edge -- otherwise a Z and a
+ * straight flow on the same stock edge land in different spread groups and both
+ * center at 0.5, overlapping their endpoints.
+ *
+ * Normally the segment orientation implies the edge (a horizontal segment
+ * leaves a left/right edge). A Z-riser runs PARALLEL to its edge, so when we
+ * know the prior stock center and the endpoint sits on an edge whose axis is
+ * parallel to the segment, preserve that occupied edge instead.
+ */
+function resolveStockAdjacentSide(
+  currentStockPoint: IPoint,
+  adjacentPoint: IPoint,
+  newStockCx: number,
+  newStockCy: number,
+  prevStockCenter: IPoint | undefined,
+): { side: Side; isHorizontalSegment: boolean } {
+  const isHorizontalSegment = isDominantlyHorizontal(currentStockPoint, adjacentPoint);
+
+  if (prevStockCenter) {
+    const occupiedEdge = classifyStockEdge(currentStockPoint, prevStockCenter);
+    if (occupiedEdge) {
+      const edgeIsLeftRight = occupiedEdge === 'left' || occupiedEdge === 'right';
+      if (edgeIsLeftRight !== isHorizontalSegment) {
+        return { side: occupiedEdge, isHorizontalSegment };
+      }
+    }
+  }
+
+  const side: Side = isHorizontalSegment
+    ? adjacentPoint.x > newStockCx
+      ? 'right'
+      : 'left'
+    : adjacentPoint.y > newStockCy
+      ? 'bottom'
+      : 'top';
+  return { side, isHorizontalSegment };
+}
+
 interface FlowAttachmentInfo {
   flow: FlowViewElement;
   side: Side;
@@ -137,6 +207,7 @@ function getFlowAttachmentInfo(
   stockUid: number,
   newStockCx: number,
   newStockCy: number,
+  prevStockCenter?: IPoint,
 ): FlowAttachmentInfo | undefined {
   const points = flow.points;
   if (points.length < 2) {
@@ -161,16 +232,12 @@ function getFlowAttachmentInfo(
   let side: Side;
   let isStraight = false;
 
-  // For 4+ point flows, use the existing segment orientation
+  // For 4+ point flows, use the existing segment orientation (preserving a
+  // Z-riser's occupied edge -- see resolveStockAdjacentSide -- so this agrees
+  // with computeFlowRoute and Z flows spread alongside straight ones).
   if (points.length >= 4) {
     const currentStockPoint = stockIsFirst ? firstPoint : lastPoint;
-    const isHorizontalSegment = isDominantlyHorizontal(currentStockPoint, adjacentPoint);
-
-    if (isHorizontalSegment) {
-      side = adjacentPoint.x > newStockCx ? 'right' : 'left';
-    } else {
-      side = adjacentPoint.y > newStockCy ? 'bottom' : 'top';
-    }
+    ({ side } = resolveStockAdjacentSide(currentStockPoint, adjacentPoint, newStockCx, newStockCy, prevStockCenter));
   } else if (canFlowBeStraight(newStockCx, newStockCy, anchor.x, anchor.y, originalFlowIsHorizontal)) {
     // Straight flow - these naturally separate based on anchor position
     isStraight = true;
@@ -218,11 +285,12 @@ export function computeFlowOffsets(
   stockUid: number,
   newStockCx: number,
   newStockCy: number,
+  prevStockCenter?: IPoint,
 ): Map<number, number> {
   // Get attachment info for all flows
   const attachmentInfos: FlowAttachmentInfo[] = [];
   for (const flow of flows) {
-    const info = getFlowAttachmentInfo(flow, stockUid, newStockCx, newStockCy);
+    const info = getFlowAttachmentInfo(flow, stockUid, newStockCx, newStockCy, prevStockCenter);
     if (info) {
       attachmentInfos.push(info);
     }
@@ -293,6 +361,7 @@ export function computeFlowRoute(
   newStockCx: number,
   newStockCy: number,
   offsetFraction: number = 0.5,
+  prevStockCenter?: IPoint,
 ): FlowViewElement {
   const points = flow.points;
   if (points.length < 2) {
@@ -325,21 +394,19 @@ export function computeFlowRoute(
     const adjacentPointIndex = stockIsFirst ? 1 : points.length - 2;
     const adjacentPoint = at(points, adjacentPointIndex);
 
-    // Determine the ORIGINAL first segment's orientation from the existing geometry.
-    // We must preserve this orientation to avoid creating diagonal segments between
-    // corner1 and corner2 when the stock moves far perpendicular to the segment.
+    // Determine the ORIGINAL first segment's orientation from the existing
+    // geometry (preserved to avoid diagonal segments between corner1 and corner2
+    // when the stock moves far perpendicular to the segment) and the attachment
+    // side, which also preserves a Z-riser's occupied edge to avoid an
+    // endpoint jump on a nudge (see resolveStockAdjacentSide).
     const currentStockPoint = stockIsFirst ? firstPoint : lastPoint;
-    const isHorizontalSegment = isDominantlyHorizontal(currentStockPoint, adjacentPoint);
-
-    // Choose the attachment side based on the preserved orientation
-    let side: Side;
-    if (isHorizontalSegment) {
-      // Horizontal segment: attach left or right based on adjacent point's X
-      side = adjacentPoint.x > newStockCx ? 'right' : 'left';
-    } else {
-      // Vertical segment: attach top or bottom based on adjacent point's Y
-      side = adjacentPoint.y > newStockCy ? 'bottom' : 'top';
-    }
+    const { side, isHorizontalSegment } = resolveStockAdjacentSide(
+      currentStockPoint,
+      adjacentPoint,
+      newStockCx,
+      newStockCy,
+      prevStockCenter,
+    );
 
     // Keep the endpoint on the stock's actual edge
     const stockEdge = getStockEdgePoint(newStockCx, newStockCy, side, offsetFraction);
@@ -723,6 +790,11 @@ export function UpdateStockAndFlows(
   const newStockCx = stockEl.x - moveDelta.x;
   const newStockCy = stockEl.y - moveDelta.y;
 
+  // The stock's center before this move, used to classify which edge each flow
+  // endpoint currently occupies so computeFlowRoute can preserve it (see the
+  // Z-riser edge-preservation there).
+  const prevStockCenter: IPoint = { x: stockEl.x, y: stockEl.y };
+
   stockEl = {
     ...stockEl,
     x: newStockCx,
@@ -730,11 +802,11 @@ export function UpdateStockAndFlows(
   };
 
   // Compute offset fractions to spread multiple flows on the same side
-  const offsets = computeFlowOffsets(flows, stockEl.uid, newStockCx, newStockCy);
+  const offsets = computeFlowOffsets(flows, stockEl.uid, newStockCx, newStockCy, prevStockCenter);
 
   flows = flows.map((flow) => {
     const offset = offsets.get(flow.uid) ?? 0.5;
-    return computeFlowRoute(flow, stockEl, newStockCx, newStockCy, offset);
+    return computeFlowRoute(flow, stockEl, newStockCx, newStockCy, offset, prevStockCenter);
   });
 
   return [stockEl, flows];
@@ -1492,23 +1564,32 @@ export function UpdateFlow(
     return [flowEl, []];
   }
 
-  // For 2-point (straight) flows with at least one cloud, allow perpendicular offset.
-  // This converts the flow to an L-shape, letting users offset flows to avoid overlap
-  // without moving the stocks themselves.
-  if (segments.length === 1 && clouds.length > 0) {
+  // For 2-point (straight) flows, allow a perpendicular valve drag to offset the
+  // route. A flow with a cloud endpoint bends into an L (the cloud moves); a flow
+  // pinned between two stocks -- where neither endpoint can move -- inserts two
+  // corners to form a Z whose middle segment runs parallel to the original axis,
+  // displaced by the perpendicular drag (#819). Once either shape exists, its
+  // middle/bent segment is interior, so the normal segment-drag machinery takes
+  // over and dragging it back onto the axis collapses (via normalizeFlowPoints)
+  // to the straight flow.
+  if (segments.length === 1) {
     const seg = segments[0];
-    const perpDelta = seg.isHorizontal ? moveDelta.y : moveDelta.x;
-    const parDelta = seg.isHorizontal ? moveDelta.x : moveDelta.y;
+    // Classify by the dominant axis rather than exact equality so a
+    // slightly-diagonal legacy flow is offset along the axis it visually runs
+    // (audit finding 5); for exact orthogonal flows this matches seg.isHorizontal.
+    const treatAsHorizontal = isDominantlyHorizontal(seg.p1, seg.p2);
+    const perpDelta = treatAsHorizontal ? moveDelta.y : moveDelta.x;
+    const parDelta = treatAsHorizontal ? moveDelta.x : moveDelta.y;
 
     // Require a significant and dominant perpendicular component to trigger reroute.
-    // This prevents accidental L-shape conversion during normal valve dragging,
-    // since pointer movement often has small perpendicular noise.
+    // This prevents accidental reroute during normal valve dragging, since pointer
+    // movement often has small perpendicular noise.
     const PERP_THRESHOLD = 5;
     const perpAbs = Math.abs(perpDelta);
     const parAbs = Math.abs(parDelta);
     const shouldReroute = perpAbs >= PERP_THRESHOLD && perpAbs > parAbs;
 
-    if (shouldReroute) {
+    if (shouldReroute && clouds.length > 0) {
       const firstPoint = first(points);
       const lastPoint = last(points);
 
@@ -1520,7 +1601,7 @@ export function UpdateFlow(
       let newFirstPoint = firstPoint;
       let newLastPoint = lastPoint;
 
-      if (seg.isHorizontal) {
+      if (treatAsHorizontal) {
         // Horizontal segment: perpendicular movement is vertical (Y changes)
         if (firstIsCloud) {
           newFirstPoint = { ...firstPoint, y: firstPoint.y - moveDelta.y };
@@ -1599,6 +1680,55 @@ export function UpdateFlow(
 
       return [flowEl, updatedClouds];
     }
+
+    if (shouldReroute && clouds.length === 0) {
+      // Both endpoints are attached stocks, so neither can move to absorb the
+      // offset. Keep both endpoints on their current stock edges and insert two
+      // corners at the displaced perpendicular coordinate (the dragged valve's
+      // position), forming a Z: two perpendicular risers bracketing a middle
+      // segment parallel to the original axis. The endpoints are untouched, so
+      // corner clearance is preserved and dragging the middle back onto the axis
+      // collapses the risers to zero length (normalizeFlowPoints) -> straight.
+      const firstPoint = first(points);
+      const lastPoint = last(points);
+
+      let corner1: Point;
+      let corner2: Point;
+      if (treatAsHorizontal) {
+        // Horizontal flow: risers are vertical (on each endpoint's X), the
+        // middle segment is horizontal at the dragged valve's Y.
+        const midY = proposedValve.y;
+        corner1 = { x: firstPoint.x, y: midY, attachedToUid: undefined };
+        corner2 = { x: lastPoint.x, y: midY, attachedToUid: undefined };
+      } else {
+        // Vertical flow: risers are horizontal (on each endpoint's Y), the
+        // middle segment is vertical at the dragged valve's X.
+        const midX = proposedValve.x;
+        corner1 = { x: midX, y: firstPoint.y, attachedToUid: undefined };
+        corner2 = { x: midX, y: lastPoint.y, attachedToUid: undefined };
+      }
+
+      // Normalize like every other mutation path so the orthogonality invariant
+      // doesn't silently rely on the perpendicular threshold keeping the risers
+      // non-degenerate.
+      points = normalizeFlowPoints([firstPoint, corner1, corner2, lastPoint]);
+
+      // Land the valve on the interior (middle) segment the drag created, not
+      // merely the closest segment: a perpendicular-dominant drag can still
+      // carry a large parallel component, which can leave a riser closer to the
+      // dragged position and strand the valve off the offset the user just made.
+      const zSegments = getSegments(points);
+      const middleSeg = zSegments.length === 3 ? zSegments[1] : findClosestSegment(proposedValve, zSegments);
+      const newValve = clampToSegment(proposedValve, middleSeg);
+      flowEl = {
+        ...flowEl,
+        x: newValve.x,
+        y: newValve.y,
+        points,
+      };
+
+      return [flowEl, []];
+    }
   }
 
   // Moving the valve along the flow path.
diff --git a/src/diagram/group-movement.ts b/src/diagram/group-movement.ts
index d2d302802..8302eba06 100644
--- a/src/diagram/group-movement.ts
+++ b/src/diagram/group-movement.ts
@@ -207,10 +207,16 @@ export function computePreRoutedOffsets(
       }
     }
 
-    // Compute offsets at the new stock position
+    // Compute offsets at the new stock position. `element` still holds the
+    // pre-move center, which computeFlowOffsets needs to classify a Z-riser's
+    // occupied edge (so a Z and a straight flow on the same edge spread instead
+    // of overlapping).
     const newStockCx = element.x - delta.x;
     const newStockCy = element.y - delta.y;
-    const offsets = computeFlowOffsets(allFlows, element.uid, newStockCx, newStockCy);
+    const offsets = computeFlowOffsets(allFlows, element.uid, newStockCx, newStockCy, {
+      x: element.x,
+      y: element.y,
+    });
 
     // Store offsets for flows
     for (const [flowUid, offset] of offsets) {
@@ -262,7 +268,10 @@ export function preProcessSelectedFlows(
         const newStockCx = endpoint.x - delta.x;
         const newStockCy = endpoint.y - delta.y;
         const offset = preComputedOffsets.get(element.uid) ?? 0.5;
-        const updatedFlow = computeFlowRoute(element, endpoint, newStockCx, newStockCy, offset);
+        const updatedFlow = computeFlowRoute(element, endpoint, newStockCx, newStockCy, offset, {
+          x: endpoint.x,
+          y: endpoint.y,
+        });
         preProcessedFlows.set(element.uid, updatedFlow);
       }
     } else if (!sourceInSel && sinkInSel && sinkUid !== undefined) {
@@ -271,7 +280,10 @@ export function preProcessSelectedFlows(
         const newStockCx = endpoint.x - delta.x;
         const newStockCy = endpoint.y - delta.y;
         const offset = preComputedOffsets.get(element.uid) ?? 0.5;
-        const updatedFlow = computeFlowRoute(element, endpoint, newStockCx, newStockCy, offset);
+        const updatedFlow = computeFlowRoute(element, endpoint, newStockCx, newStockCy, offset, {
+          x: endpoint.x,
+          y: endpoint.y,
+        });
         preProcessedFlows.set(element.uid, updatedFlow);
       }
     }
@@ -344,7 +356,10 @@ export function processSelectedFlow(
         const newStockCx = sourceEndpoint.x - delta.x;
         const newStockCy = sourceEndpoint.y - delta.y;
         // Use default offset of 0.5 (center) for selected flows
-        let updatedFlow = computeFlowRoute(flow, sourceEndpoint, newStockCx, newStockCy, 0.5);
+        let updatedFlow = computeFlowRoute(flow, sourceEndpoint, newStockCx, newStockCy, 0.5, {
+          x: sourceEndpoint.x,
+          y: sourceEndpoint.y,
+        });
 
         // computeFlowRoute preserves the valve's fractional position, which lags
         // the drag when the flow is selected; re-clamp it to the new path.
@@ -365,7 +380,10 @@ export function processSelectedFlow(
         const newStockCx = sinkEndpoint.x - delta.x;
         const newStockCy = sinkEndpoint.y - delta.y;
         // Use default offset of 0.5 (center) for selected flows
-        let updatedFlow = computeFlowRoute(flow, sinkEndpoint, newStockCx, newStockCy, 0.5);
+        let updatedFlow = computeFlowRoute(flow, sinkEndpoint, newStockCx, newStockCy, 0.5, {
+          x: sinkEndpoint.x,
+          y: sinkEndpoint.y,
+        });
 
         // computeFlowRoute preserves the valve's fractional position, which lags
         // the drag when the flow is selected; re-clamp it to the new path.
@@ -540,7 +558,9 @@ export function routeUnselectedFlows(
         const newStockCy = endpoint.y - delta.y;
         for (const flow of flows) {
           const offset = preComputedOffsets.get(flow.uid) ?? 0.5;
-          updatedFlows.push(computeFlowRoute(flow, endpoint, newStockCx, newStockCy, offset));
+          updatedFlows.push(
+            computeFlowRoute(flow, endpoint, newStockCx, newStockCy, offset, { x: endpoint.x, y: endpoint.y }),
+          );
         }
       } else if (endpoint?.type === 'cloud') {
         for (const flow of flows) {
@@ -811,8 +831,11 @@ export function applyGroupMovement(input: GroupMovementInput): GroupMovementOutp
       continue;
     }
 
-    // Single-flow selection without segmentIndex: delegate to UpdateFlow for flows
-    // with cloud endpoints to preserve perpendicular drag L-shape behavior
+    // Single-flow selection without segmentIndex: delegate to UpdateFlow so a
+    // perpendicular valve drag can reroute. Cloud-ended flows bend into an L;
+    // stock-to-stock flows offset into a Z (#819). UpdateFlow also handles the
+    // plain along-axis valve slide, so this subsumes the old stock-to-stock
+    // clamp path for single-flow selections.
     if (selection.size === 1) {
       const pts = el.points;
       if (pts.length >= 2) {
@@ -820,11 +843,8 @@ export function applyGroupMovement(input: GroupMovementInput): GroupMovementOutp
         const sinkId = last(pts).attachedToUid;
         const source = sourceId !== undefined ? originalElements.get(sourceId) : undefined;
         const sink = sinkId !== undefined ? originalElements.get(sinkId) : undefined;
-        const hasCloud =
-          (source !== undefined && source.type === 'cloud') || (sink !== undefined && sink.type === 'cloud');
 
         if (
-          hasCloud &&
           source &&
           sink &&
           (source.type === 'stock' || source.type === 'cloud') &&
diff --git a/src/diagram/tests/flow-routing.test.ts b/src/diagram/tests/flow-routing.test.ts
index b0470cf76..460d22999 100644
--- a/src/diagram/tests/flow-routing.test.ts
+++ b/src/diagram/tests/flow-routing.test.ts
@@ -6,6 +6,7 @@ import { type Point, type FlowViewElement, type StockViewElement, type CloudView
 
 import {
   computeFlowRoute,
+  computeFlowOffsets,
   UpdateStockAndFlows,
   UpdateCloudAndFlow,
   UpdateFlow,
@@ -1542,6 +1543,333 @@ describe('Flow routing', () => {
     });
   });
 
+  describe('UpdateFlow - Z-shape offset for stock-to-stock flows', () => {
+    // Both endpoints are stocks, so neither can move to absorb an offset. A
+    // perpendicular valve drag must insert two corners to form a Z whose middle
+    // segment runs parallel to the original axis, displaced by the drag.
+    const stockAUid = 1;
+    const stockBUid = 4;
+
+    // A horizontal straight flow between two stocks: A(100,100) -> B(250,100).
+    function makeHorizontalStockFlow(): {
+      flow: FlowViewElement;
+      stockA: StockViewElement;
+      stockB: StockViewElement;
+    } {
+      const stockA = makeStock(stockAUid, 100, 100, [], [flowUid]);
+      const stockB = makeStock(stockBUid, 250, 100, [flowUid], []);
+      const flow = makeFlow(flowUid, 175, 100, [
+        { x: 100 + StockWidth / 2, y: 100, attachedToUid: stockAUid }, // A right edge = 122.5
+        { x: 250 - StockWidth / 2, y: 100, attachedToUid: stockBUid }, // B left edge = 227.5
+      ]);
+      return { flow, stockA, stockB };
+    }
+
+    // A vertical straight flow between two stocks: A(100,100) -> B(100,250).
+    function makeVerticalStockFlow(): {
+      flow: FlowViewElement;
+      stockA: StockViewElement;
+      stockB: StockViewElement;
+    } {
+      const stockA = makeStock(stockAUid, 100, 100, [], [flowUid]);
+      const stockB = makeStock(stockBUid, 100, 250, [flowUid], []);
+      const flow = makeFlow(flowUid, 100, 175, [
+        { x: 100, y: 100 + StockHeight / 2, attachedToUid: stockAUid }, // A bottom edge = 117.5
+        { x: 100, y: 250 - StockHeight / 2, attachedToUid: stockBUid }, // B top edge = 232.5
+      ]);
+      return { flow, stockA, stockB };
+    }
+
+    it('should form a Z when dragging a horizontal flow perpendicular (up)', () => {
+      const { flow, stockA, stockB } = makeHorizontalStockFlow();
+
+      // Drag valve up by 30 (moveDelta.y = 30 => middle segment at y = 100 - 30 = 70)
+      const [newFlow, updatedClouds] = UpdateFlow(flow, [stockA, stockB], { x: 0, y: 30 });
+
+      // Two corners inserted -> four points
+      expect(newFlow.points.length).toBe(4);
+      // No clouds involved
+      expect(updatedClouds.length).toBe(0);
+
+      // Endpoints stay pinned on their stock edges, still attached
+      expect(newFlow.points[0].x).toBe(122.5);
+      expect(newFlow.points[0].y).toBe(100);
+      expect(newFlow.points[0].attachedToUid).toBe(stockAUid);
+      expect(newFlow.points[3].x).toBe(227.5);
+      expect(newFlow.points[3].y).toBe(100);
+      expect(newFlow.points[3].attachedToUid).toBe(stockBUid);
+
+      // Corners are unattached and sit at the displaced Y, forming a horizontal
+      // middle segment parallel to the original axis.
+      expect(newFlow.points[1].attachedToUid).toBeUndefined();
+      expect(newFlow.points[2].attachedToUid).toBeUndefined();
+      expect(newFlow.points[1].x).toBe(122.5); // riser stays on A's edge X
+      expect(newFlow.points[1].y).toBe(70);
+      expect(newFlow.points[2].x).toBe(227.5); // riser stays on B's edge X
+      expect(newFlow.points[2].y).toBe(70);
+
+      // Valve lands on the displaced middle segment
+      expect(newFlow.y).toBe(70);
+    });
+
+    it('should form a Z when dragging a horizontal flow perpendicular (down)', () => {
+      const { flow, stockA, stockB } = makeHorizontalStockFlow();
+
+      // Drag valve down by 40 (moveDelta.y = -40 => middle segment at y = 140)
+      const [newFlow] = UpdateFlow(flow, [stockA, stockB], { x: 0, y: -40 });
+
+      expect(newFlow.points.length).toBe(4);
+      expect(newFlow.points[1].y).toBe(140);
+      expect(newFlow.points[2].y).toBe(140);
+      // Middle segment is horizontal (parallel to the original horizontal axis)
+      expect(newFlow.points[1].y).toBe(newFlow.points[2].y);
+      expect(newFlow.y).toBe(140);
+    });
+
+    it('should form a Z when dragging a vertical flow perpendicular (right)', () => {
+      const { flow, stockA, stockB } = makeVerticalStockFlow();
+
+      // Drag valve right by 30 (moveDelta.x = -30 => middle segment at x = 130)
+      const [newFlow, updatedClouds] = UpdateFlow(flow, [stockA, stockB], { x: -30, y: 0 });
+
+      expect(newFlow.points.length).toBe(4);
+      expect(updatedClouds.length).toBe(0);
+
+      // Endpoints pinned to their stock edges (top/bottom), still attached
+      expect(newFlow.points[0].x).toBe(100);
+      expect(newFlow.points[0].y).toBe(117.5);
+      expect(newFlow.points[0].attachedToUid).toBe(stockAUid);
+      expect(newFlow.points[3].x).toBe(100);
+      expect(newFlow.points[3].y).toBe(232.5);
+      expect(newFlow.points[3].attachedToUid).toBe(stockBUid);
+
+      // Corners at displaced X, forming a vertical middle segment
+      expect(newFlow.points[1].x).toBe(130);
+      expect(newFlow.points[1].y).toBe(117.5);
+      expect(newFlow.points[2].x).toBe(130);
+      expect(newFlow.points[2].y).toBe(232.5);
+      expect(newFlow.points[1].x).toBe(newFlow.points[2].x); // middle is vertical
+
+      // Valve lands on the displaced middle segment
+      expect(newFlow.x).toBe(130);
+    });
+
+    it('should not form a Z on a parallel (along-axis) drag', () => {
+      const { flow, stockA, stockB } = makeHorizontalStockFlow();
+
+      // Purely parallel drag: valve slides along the flow, no reroute
+      const [newFlow, updatedClouds] = UpdateFlow(flow, [stockA, stockB], { x: -20, y: 0 });
+
+      expect(newFlow.points.length).toBe(2);
+      expect(updatedClouds.length).toBe(0);
+      expect(newFlow.y).toBe(100);
+      // Valve slid along the segment (proposed 195, within [132.5, 217.5])
+      expect(newFlow.x).toBe(195);
+    });
+
+    it('should not form a Z when perpendicular movement is below threshold', () => {
+      const { flow, stockA, stockB } = makeHorizontalStockFlow();
+
+      // Perpendicular component (3) is below the 5px threshold
+      const [newFlow] = UpdateFlow(flow, [stockA, stockB], { x: -20, y: 3 });
+
+      expect(newFlow.points.length).toBe(2);
+      expect(newFlow.y).toBe(100);
+    });
+
+    it('should not form a Z when parallel movement dominates', () => {
+      const { flow, stockA, stockB } = makeHorizontalStockFlow();
+
+      // Perpendicular (20) present but parallel (30) is larger
+      const [newFlow] = UpdateFlow(flow, [stockA, stockB], { x: -30, y: 20 });
+
+      expect(newFlow.points.length).toBe(2);
+    });
+
+    it('should form a horizontal-middle Z for a slightly-diagonal (near-horizontal) flow', () => {
+      // Near-horizontal legacy flow: endpoints drift 5px in Y but the run is
+      // dominantly horizontal, so the offset must be along Y with a horizontal
+      // middle segment (audit finding 5 dominant-axis classification).
+      const stockA = makeStock(stockAUid, 100, 100, [], [flowUid]);
+      const stockB = makeStock(stockBUid, 250, 105, [flowUid], []);
+      const flow = makeFlow(flowUid, 175, 102, [
+        { x: 122.5, y: 100, attachedToUid: stockAUid },
+        { x: 227.5, y: 105, attachedToUid: stockBUid },
+      ]);
+
+      // Drag up by 30 (proposedValve.y = 102 - 30 = 72)
+      const [newFlow] = UpdateFlow(flow, [stockA, stockB], { x: 0, y: 30 });
+
+      expect(newFlow.points.length).toBe(4);
+      // Middle segment is horizontal at the displaced Y
+      expect(newFlow.points[1].y).toBe(72);
+      expect(newFlow.points[2].y).toBe(72);
+      // Risers stay on the endpoints' X, so each is vertical despite the drift
+      expect(newFlow.points[1].x).toBe(122.5);
+      expect(newFlow.points[2].x).toBe(227.5);
+      // Endpoints keep their (drifted) positions
+      expect(newFlow.points[0].y).toBe(100);
+      expect(newFlow.points[3].y).toBe(105);
+    });
+
+    it('should collapse a Z back to a straight flow when the middle drags onto the axis', () => {
+      // Freshly-formed Z: middle segment offset to y=70.
+      const zFlow = makeFlow(flowUid, 175, 70, [
+        { x: 122.5, y: 100, attachedToUid: stockAUid },
+        { x: 122.5, y: 70 },
+        { x: 227.5, y: 70 },
+        { x: 227.5, y: 100, attachedToUid: stockBUid },
+      ]);
+      const stockA = makeStock(stockAUid, 100, 100, [], [flowUid]);
+      const stockB = makeStock(stockBUid, 250, 100, [flowUid], []);
+
+      // Drag the middle segment (index 1) back down onto the original axis (y=100).
+      // moveDelta.y = -30 moves the corners down by 30 -> y=100, collapsing.
+      const [newFlow] = UpdateFlow(zFlow, [stockA, stockB], { x: 0, y: -30 }, 1);
+
+      // The colinear/zero-length cleanup in normalizeFlowPoints restores a straight flow
+      expect(newFlow.points.length).toBe(2);
+      expect(newFlow.points[0].x).toBe(122.5);
+      expect(newFlow.points[0].y).toBe(100);
+      expect(newFlow.points[1].x).toBe(227.5);
+      expect(newFlow.points[1].y).toBe(100);
+    });
+
+    it('should keep a Z valid when an attached stock is moved (UpdateStockAndFlows)', () => {
+      // Z flow with middle segment at y=70, between A(100,100) and B(250,100).
+      const zFlow = makeFlow(flowUid, 175, 70, [
+        { x: 122.5, y: 100, attachedToUid: stockAUid },
+        { x: 122.5, y: 70 },
+        { x: 227.5, y: 70 },
+        { x: 227.5, y: 100, attachedToUid: stockBUid },
+      ]);
+      const stockA = makeStock(stockAUid, 100, 100, [], [flowUid]);
+
+      // Move stock A left by 20 (moveDelta inverted: +20 => new center x = 80)
+      const [newStock, newFlows] = UpdateStockAndFlows(stockA, [zFlow], { x: 20, y: 0 });
+
+      expect(newStock.x).toBe(80);
+      const routed = newFlows[0];
+      // Still a valid Z (four points), not corrupted into a diagonal or a stub
+      expect(routed.points.length).toBe(4);
+      // The middle segment is still horizontal at its displaced Y
+      expect(routed.points[1].y).toBe(70);
+      expect(routed.points[2].y).toBe(70);
+      // Endpoints remain attached to their stocks
+      expect(routed.points[0].attachedToUid).toBe(stockAUid);
+      expect(routed.points[3].attachedToUid).toBe(stockBUid);
+      // The moved endpoint stays on stock A's RIGHT edge (it must not jump to
+      // the top/bottom edge just because the riser is vertical): new center
+      // x=80 => right edge x=102.5, y unchanged.
+      expect(routed.points[0].x).toBe(80 + StockWidth / 2);
+      expect(routed.points[0].y).toBe(100);
+      // The riser stays aligned with the moved endpoint (vertical at x=102.5)
+      expect(routed.points[1].x).toBe(80 + StockWidth / 2);
+      // Segments strictly alternate orientation (orthogonal path preserved)
+      const segs = getSegments(routed.points);
+      expect(segs.length).toBe(3);
+      for (let i = 0; i < segs.length - 1; i++) {
+        expect(segs[i].isHorizontal).not.toBe(segs[i + 1].isHorizontal);
+      }
+    });
+
+    it('should not jump the endpoint edge across a repeated stock-nudge sequence', () => {
+      // Repeatedly nudging a stock on a Z must keep the endpoint on the SAME
+      // edge each time (no oscillation/jump between right and top).
+      let flow: FlowViewElement = makeFlow(flowUid, 175, 70, [
+        { x: 122.5, y: 100, attachedToUid: stockAUid },
+        { x: 122.5, y: 70 },
+        { x: 227.5, y: 70 },
+        { x: 227.5, y: 100, attachedToUid: stockBUid },
+      ]);
+      let cx = 100;
+      for (let i = 0; i < 4; i++) {
+        const stockA = makeStock(stockAUid, cx, 100, [], [flowUid]);
+        // Nudge stock A left by 10 each iteration (moveDelta +10 => cx - 10)
+        const [newStock, newFlows] = UpdateStockAndFlows(stockA, [flow], { x: 10, y: 0 });
+        cx = newStock.x;
+        flow = newFlows[0];
+        // Endpoint stays pinned to the RIGHT edge of the (moved) stock, never
+        // jumping to the top edge; middle segment preserved at y=70.
+        expect(flow.points.length).toBe(4);
+        expect(flow.points[0].attachedToUid).toBe(stockAUid);
+        expect(flow.points[0].x).toBe(cx + StockWidth / 2);
+        expect(flow.points[0].y).toBe(100);
+        expect(flow.points[1].y).toBe(70);
+        expect(flow.points[2].y).toBe(70);
+      }
+      expect(cx).toBe(60); // 100 - 4*10
+    });
+
+    it('should spread a Z and a straight flow that share a stock edge (no endpoint overlap)', () => {
+      // Probe: stock A has two right-edge outflows -- one straight (A->B) and one
+      // Z (A->C, vertical riser off the right edge). getFlowAttachmentInfo must
+      // classify the Z as 'right' (its occupied edge), matching computeFlowRoute,
+      // so the two land in the same spread group and their endpoints don't
+      // collide at the edge center.
+      const stockCUid = 6;
+      const straight = makeFlow(2, 175, 100, [
+        { x: 122.5, y: 100, attachedToUid: stockAUid },
+        { x: 227.5, y: 100, attachedToUid: stockBUid },
+      ]);
+      const zFlow = makeFlow(5, 200, 60, [
+        { x: 122.5, y: 100, attachedToUid: stockAUid },
+        { x: 122.5, y: 60 },
+        { x: 300, y: 60 },
+        { x: 300, y: 100, attachedToUid: stockCUid },
+      ]);
+      const stockA = makeStock(stockAUid, 100, 100, [], [2, 5]);
+
+      // Offsets: both flows are on the 'right' side, so they spread. The straight
+      // flow is pinned to 0.5; the Z takes a non-center slot.
+      const offsets = computeFlowOffsets([straight, zFlow], stockAUid, 95, 100, { x: 100, y: 100 });
+      expect(offsets.get(2)).toBe(0.5);
+      expect(offsets.get(5)).not.toBe(0.5);
+      expect(offsets.get(5)).toBeCloseTo(2 / 3, 5);
+
+      // End to end: nudge A right by 5 (delta +5 => new center x=95). Both
+      // endpoints stay on the right edge (x=117.5) but at DIFFERENT y -- before
+      // the fix both landed at (117.5, 100).
+      const [newStock, newFlows] = UpdateStockAndFlows(stockA, [straight, zFlow], { x: 5, y: 0 });
+      expect(newStock.x).toBe(95);
+      const straightEnd = newFlows[0].points[0];
+      const zEnd = newFlows[1].points[0];
+      expect(straightEnd.x).toBe(95 + StockWidth / 2);
+      expect(zEnd.x).toBe(95 + StockWidth / 2);
+      expect(straightEnd.y).toBe(100);
+      // The Z endpoint must not coincide with the straight endpoint.
+      expect(zEnd.y).not.toBe(straightEnd.y);
+      // The Z is still a valid 4-point flow with its middle preserved at y=60.
+      expect(newFlows[1].points.length).toBe(4);
+      expect(newFlows[1].points[1].y).toBe(60);
+      expect(newFlows[1].points[2].y).toBe(60);
+    });
+
+    it('should clamp the valve to the middle segment on a perpendicular-dominant drag with large parallel component', () => {
+      // Probe from review: perpendicular dominates (82 > 80) so a Z forms, but
+      // the large parallel component would put the CLOSEST segment on a riser.
+      // The valve must land on the offset middle segment, not a riser.
+      const stockA = makeStock(stockAUid, 100, 100, [], [flowUid]);
+      const stockB = makeStock(stockBUid, 250, 100, [flowUid], []);
+      const flow = makeFlow(flowUid, 175, 100, [
+        { x: 122.5, y: 100, attachedToUid: stockAUid },
+        { x: 227.5, y: 100, attachedToUid: stockBUid },
+      ]);
+
+      const [newFlow] = UpdateFlow(flow, [stockA, stockB], { x: 80, y: 82 });
+
+      // Z formed at the displaced Y (100 - 82 = 18)
+      expect(newFlow.points.length).toBe(4);
+      expect(newFlow.points[1].y).toBe(18);
+      expect(newFlow.points[2].y).toBe(18);
+      // Valve is on the horizontal MIDDLE segment (y=18), not stranded on a
+      // vertical riser at x=122.5 / x=227.5.
+      expect(newFlow.y).toBe(18);
+      expect(newFlow.x).toBeGreaterThan(122.5);
+      expect(newFlow.x).toBeLessThan(227.5);
+    });
+  });
+
   describe('moveSegment', () => {
     it('should move horizontal segment up/down', () => {
       // L-shaped flow with horizontal middle concept:
diff --git a/src/diagram/tests/group-movement.test.ts b/src/diagram/tests/group-movement.test.ts
index f18398e67..cc71e4c62 100644
--- a/src/diagram/tests/group-movement.test.ts
+++ b/src/diagram/tests/group-movement.test.ts
@@ -315,6 +315,86 @@ describe('Group Movement', () => {
       expect(newFlow.points[newFlow.points.length - 1].attachedToUid).toBe(3);
       expect(newFlow.points[newFlow.points.length - 1].x).toBe(200);
     });
+
+    it('should not jump a Z flow endpoint to a different edge when its stock is nudged', () => {
+      // Z-shaped stock-to-stock flow: endpoint on A's RIGHT edge with a vertical
+      // riser. Nudging A must keep the endpoint on the right edge (translate),
+      // not jump it to the top edge (#819 follow-up). This exercises the real
+      // interactive path (applyGroupMovement -> computeFlowRoute), not
+      // UpdateStockAndFlows.
+      const stockA = makeStock(1, 100, 100, [], [2]);
+      const stockB = makeStock(3, 250, 100, [2], []);
+      const zFlow = makeFlow(2, 175, 70, [
+        { x: 122.5, y: 100, attachedToUid: 1 },
+        { x: 122.5, y: 70 },
+        { x: 227.5, y: 70 },
+        { x: 227.5, y: 100, attachedToUid: 3 },
+      ]);
+
+      const elements = new Map<UID, ViewElement>([
+        [1, stockA],
+        [2, zFlow],
+        [3, stockB],
+      ]);
+
+      // Select only stock A and nudge it left by 20 (delta +20 => new center 80)
+      const result = testApplyGroupMovement(elements, new Set<UID>([1]), { x: 20, y: 0 });
+
+      const newStock = result.get(1) as StockViewElement;
+      expect(newStock.x).toBe(80);
+
+      const newFlow = result.get(2) as FlowViewElement;
+      // Endpoint stays on A's RIGHT edge (80 + 22.5), y unchanged -- no jump to
+      // the top edge (which would be y=82.5).
+      expect(newFlow.points[0].attachedToUid).toBe(1);
+      expect(newFlow.points[0].x).toBe(80 + StockWidth / 2);
+      expect(newFlow.points[0].y).toBe(100);
+      // Riser stays aligned; middle segment preserved at y=70; still a Z.
+      expect(newFlow.points.length).toBe(4);
+      expect(newFlow.points[1].x).toBe(80 + StockWidth / 2);
+      expect(newFlow.points[1].y).toBe(70);
+      expect(newFlow.points[2].y).toBe(70);
+    });
+
+    it('should spread a Z and a straight flow sharing a stock edge through the interactive path', () => {
+      // Same probe as the flow-routing spread test, but via applyGroupMovement's
+      // computePreRoutedOffsets path (the interactive stock drag). A straight and
+      // a Z outflow both occupy A's right edge; nudging A must spread them, not
+      // stack both endpoints at the edge center.
+      const stockA = makeStock(1, 100, 100, [], [2, 5]);
+      const stockB = makeStock(3, 250, 100, [2], []);
+      const stockC = makeStock(6, 300, 250, [5], []);
+      const straight = makeFlow(2, 175, 100, [
+        { x: 122.5, y: 100, attachedToUid: 1 },
+        { x: 227.5, y: 100, attachedToUid: 3 },
+      ]);
+      const zFlow = makeFlow(5, 200, 60, [
+        { x: 122.5, y: 100, attachedToUid: 1 },
+        { x: 122.5, y: 60 },
+        { x: 300, y: 60 },
+        { x: 300, y: 100, attachedToUid: 6 },
+      ]);
+
+      const elements = new Map<UID, ViewElement>([
+        [1, stockA],
+        [2, straight],
+        [3, stockB],
+        [5, zFlow],
+        [6, stockC],
+      ]);
+
+      const result = testApplyGroupMovement(elements, new Set<UID>([1]), { x: 5, y: 0 });
+
+      const straightEnd = (result.get(2) as FlowViewElement).points[0];
+      const zEnd = (result.get(5) as FlowViewElement).points[0];
+      // Both on A's right edge (x = 95 + 22.5), but NOT at the same point.
+      expect(straightEnd.x).toBe(95 + StockWidth / 2);
+      expect(zEnd.x).toBe(95 + StockWidth / 2);
+      expect(zEnd.y).not.toBe(straightEnd.y);
+      // Z stays a valid 4-point flow with its middle preserved.
+      expect((result.get(5) as FlowViewElement).points.length).toBe(4);
+      expect((result.get(5) as FlowViewElement).points[1].y).toBe(60);
+    });
   });
 
   describe('Offset preservation with translated and routed flows', () => {
@@ -774,10 +854,10 @@ describe('Group Movement', () => {
       expect(newFlow.points[newFlow.points.length - 1].x).toBe(200 - StockWidth / 2);
     });
 
-    it('should clamp valve to flow path when moving perpendicular to flow direction', () => {
+    it('should offset a stock-to-stock flow into a Z when dragged perpendicular', () => {
       // Setup: Stock A -> horizontal Flow -> Stock B
-      // When we select only the flow (not the stocks) and move it perpendicular,
-      // the valve should stay on the flow path, not move off into empty space
+      // Selecting only the flow and dragging it perpendicular offsets the route
+      // into a Z (two corners), since neither stock endpoint can move (#819).
       const stockA = makeStock(1, 100, 100, [], [2]);
       const stockB = makeStock(3, 200, 100, [2], []);
       const flow = makeFlow(2, 150, 100, [
@@ -793,7 +873,7 @@ describe('Group Movement', () => {
 
       // Only select the flow, move UP (perpendicular to the horizontal flow)
       const selection = new Set<UID>([2]);
-      const delta = { x: 0, y: 50 }; // Move up 50
+      const delta = { x: 0, y: 50 }; // Move up 50 => middle segment at y=50
 
       const result = testApplyGroupMovement(elements, selection, delta);
 
@@ -801,13 +881,16 @@ describe('Group Movement', () => {
       expect((result.get(1) as StockViewElement).y).toBe(100);
       expect((result.get(3) as StockViewElement).y).toBe(100);
 
-      // Flow valve should be clamped to stay on the horizontal flow path (y=100)
+      // Flow becomes a Z (four points); endpoints stay pinned on the stock edges
       const newFlow = result.get(2) as FlowViewElement;
-      expect(newFlow.y).toBe(100); // Should stay on the flow path
-
-      // Flow endpoints should stay fixed
+      expect(newFlow.points.length).toBe(4);
       expect(newFlow.points[0].y).toBe(100);
       expect(newFlow.points[newFlow.points.length - 1].y).toBe(100);
+      // Middle segment (the two corners) sits at the displaced Y
+      expect(newFlow.points[1].y).toBe(50);
+      expect(newFlow.points[2].y).toBe(50);
+      // Valve rides the offset middle segment
+      expect(newFlow.y).toBe(50);
     });
   });
 

From 0bca1897c858a796c146016ea75d62fb8bb0bf68 Mon Sep 17 00:00:00 2001
From: Bobby Powers <bobbypowers@gmail.com>
Date: Wed, 1 Jul 2026 09:56:49 -0700
Subject: [PATCH 2/4] diagram: preserve the drawn flow when a flow-attach patch
 fails

handleFlowAttach early-returned on patch failure without committing
the view, silently discarding the flow the user just drew (Canvas had
already cleared its in-creation staging on pointer-up). Worse, the
selection was still set to the never-committed flow's uid, and
handleEditingNameDone resolved that selection with the throwing
getElementByUid -- the deferred tool-change editing-done then wedged
the editor in a repeated-exception loop. The failure path now commits
the optimistic view like handleCreateVariable and handleSelectionDelete
already do, so the drawn flow stays selectable/renamable/deletable and
the engine error surfaces via the toast.

On a genuinely failing upsertFlow this persists a view flow with no
backing variable; that divergence class is already produced by the
sibling handlers, renders via the dangling-ref guards, is removable by
keyboard delete, and self-heals on name-edit Escape. Defense in depth:
handleEditingNameDone resolves the selection with tryGetElementByUid
and tears down cleanly on a phantom or non-singleton selection,
mirroring the render-time guard.
---
 src/diagram/Editor.tsx                        |  14 +-
 src/diagram/drawing/Canvas.tsx                |  16 +-
 ...nvas-gestures-flow-attach-failure.test.tsx | 149 ++++++++++++++++++
 .../tests/editor-flow-attach-failure.test.ts  | 147 +++++++++++++++++
 4 files changed, 318 insertions(+), 8 deletions(-)
 create mode 100644 src/diagram/tests/canvas-gestures-flow-attach-failure.test.tsx
 create mode 100644 src/diagram/tests/editor-flow-attach-failure.test.ts

diff --git a/src/diagram/Editor.tsx b/src/diagram/Editor.tsx
index a8a10e4ed..bda8aca7e 100644
--- a/src/diagram/Editor.tsx
+++ b/src/diagram/Editor.tsx
@@ -937,12 +937,14 @@ export const Editor = React.memo(function Editor(props: EditorProps): React.Reac
         const patch: JsonProjectPatch = {
           models: [{ name: modelName(), ops: [...result.ops] }],
         };
-        // On patch failure, commit the selection/creation flag but DO NOT
-        // update the view -- preserving the original behavior exactly.
-        if (!(await applyPatchOrReportError(patch, 'flow attach'))) {
-          setState({ selection, flowStillBeingCreated: inCreation });
-          return;
-        }
+        // Apply the model-level ops but do NOT bail on failure: the drawn flow
+        // must still be committed to the view (issue #820). The controller
+        // reports any failure via onError (the toast); the view update below
+        // runs regardless, matching handleCreateVariable/handleSelectionDelete.
+        // Discarding it here previously both lost the user's work AND left the
+        // just-created-flow name edit selecting a flow that was never committed
+        // to the view -- the handleEditingNameDone getElementByUid crash.
+        await applyPatchOrReportError(patch, 'flow attach');
       }
 
       await updateView({ ...view, nextUid: result.nextUid, elements: [...result.elements] }, { recordHistory: true });
diff --git a/src/diagram/drawing/Canvas.tsx b/src/diagram/drawing/Canvas.tsx
index dda349ea2..0a49c02bd 100644
--- a/src/diagram/drawing/Canvas.tsx
+++ b/src/diagram/drawing/Canvas.tsx
@@ -2059,8 +2059,20 @@ export const Canvas = React.memo(function Canvas(props: CanvasProps): React.Reac
       return;
     }
 
-    const uid = only(latest.current.props.selection);
-    const element = getElementByUid(uid);
+    // Resolve the element being named through the NON-throwing lookup, mirroring
+    // the render-time guard (see the editingElement resolution). A failed
+    // flow-attach can leave the selection referencing a flow that was never
+    // committed to the view (issue #820); dereferencing it with the throwing
+    // getElementByUid wedged the editor in a repeated-exception loop. When the
+    // selection is not a single resolvable element there is nothing to commit --
+    // tear the editor down cleanly instead of crashing.
+    const selection = latest.current.props.selection;
+    const uid = selection.size === 1 ? only(selection) : undefined;
+    const element = uid !== undefined ? tryGetElementByUid(uid) : undefined;
+    if (uid === undefined || element === undefined) {
+      clearPointerState();
+      return;
+    }
     const oldName = displayName(defined((element as NamedViewElement).name));
 
     if (uid === inCreationUid) {
diff --git a/src/diagram/tests/canvas-gestures-flow-attach-failure.test.tsx b/src/diagram/tests/canvas-gestures-flow-attach-failure.test.tsx
new file mode 100644
index 000000000..8c34e807a
--- /dev/null
+++ b/src/diagram/tests/canvas-gestures-flow-attach-failure.test.tsx
@@ -0,0 +1,149 @@
+/**
+ * @jest-environment jsdom
+ *
+ * Copyright 2026 The Simlin Authors. All rights reserved.
+ * Use of this source code is governed by the Apache License,
+ * Version 2.0, that can be found in the LICENSE file.
+ */
+
+// Regression tests for the create-flow name-edit crash (issue #820 / the
+// getElementByUid crash surfaced during the #819 review).
+//
+// When a just-drawn flow's attach patch fails, the host (Editor) does not
+// commit the flow into the view, yet the Canvas has already handed off to the
+// just-created-flow name edit. Its `props.selection` then references a flow
+// that is not in the view. The render already tolerates that (it resolves the
+// editing element through the NON-throwing tryGetElementByUid and skips the
+// editor), but `handleEditingNameDone` used the THROWING getElementByUid, so
+// any path that fired it against the phantom selection -- notably the deferred
+// editing-done scheduled when the active tool changes -- threw
+// `expected non-undefined object` and wedged the editor in a repeated-exception
+// loop, making the whole editor appear broken.
+//
+// Two paths reach handleEditingNameDone:
+//   - the COMMIT path (a non-empty name, e.g. the deferred tool-change), which
+//     resolved the selected element -- this is where the crash lived; and
+//   - the CANCEL path (Escape or an empty-name commit -> creatingFlow ->
+//     onDeleteSelection), which never dereferences the element.
+// The first test pins the commit-path crash fix against a phantom; the second
+// pins that the cancel/delete-on-cancel path stays benign (single-fire, no
+// throw). A cancel can only be issued against a RENDERED editor, which only
+// renders when the element resolves -- so a cancel is never issued against an
+// unresolved phantom, and the cancel branch is dereference-free regardless.
+
+import { act, fireEvent } from '@testing-library/react';
+
+import type { StockFlowView } from '@simlin/core/datamodel';
+
+import { makeCloud, makeFlow, pointerDown, pointerMove, pointerUp, renderCanvas } from './canvas-gesture-harness';
+
+// Drain one macrotask so the render-scheduled deferred editing-done runs.
+async function flushDeferred(): Promise<void> {
+  await act(async () => {
+    await new Promise((resolve) => setTimeout(resolve, 0));
+  });
+}
+
+// Capture any uncaught error dispatched to window (a throw inside the deferred
+// editing-done timer surfaces here rather than at the call site).
+function captureWindowErrors(): { errors: unknown[]; stop: () => void } {
+  const errors: unknown[] = [];
+  const onError = (e: ErrorEvent): void => {
+    errors.push(e.error ?? e.message);
+  };
+  window.addEventListener('error', onError);
+  return { errors, stop: () => window.removeEventListener('error', onError) };
+}
+
+describe('Canvas name-edit teardown for a just-created flow', () => {
+  it('the deferred editing-done does not crash on a phantom selection (commit path)', async () => {
+    const h = renderCanvas({ elements: [], selectedTool: 'flow' });
+    // Model the failed-attach path: the host neither commits the drawn flow
+    // into the view nor selects a committed element, so the Canvas's selection
+    // is left pointing at the (now-cleared) in-creation flow.
+    h.callbacks.onMoveFlow.mockImplementation(() => {});
+    h.clearMountCalls();
+
+    // Draw the flow. Pointer-up hands off into the just-created-flow name edit
+    // (interaction = editingName/creatingFlow). The editor itself does not
+    // render because the selected element is unresolvable -- exactly the phantom
+    // state a failed attach leaves behind.
+    pointerDown(h.svg, 200, 200);
+    pointerMove(h.svg, 300, 200, { buttons: 1 });
+    pointerUp(h.svg, 300, 200);
+
+    // Changing the active tool schedules the deferred editing-done, which
+    // resolves the selection with a non-empty name (the commit path). Before the
+    // fix this threw from getElementByUid.
+    const capture = captureWindowErrors();
+    try {
+      h.setProps({ selectedTool: undefined });
+      await flushDeferred();
+    } finally {
+      capture.stop();
+    }
+
+    expect(capture.errors).toEqual([]);
+    // Nothing to commit against a phantom: no rename, no create -- and, because
+    // this is the commit path (non-empty name), no delete either.
+    expect(h.callbacks.onRenameVariable).not.toHaveBeenCalled();
+    expect(h.callbacks.onCreateVariable).not.toHaveBeenCalled();
+    expect(h.callbacks.onDeleteSelection).not.toHaveBeenCalled();
+    // The editor is torn down (no lingering contenteditable).
+    expect(h.query('[contenteditable]')).toBeNull();
+  });
+
+  it('cancelling a just-created flow name edit fires exactly one delete and never throws (cancel path)', () => {
+    const h = renderCanvas({ elements: [], selectedTool: 'flow' });
+    // Success materializer: commit the drawn flow so the name editor renders for
+    // a real element. A cancel can only be issued against a rendered editor, so
+    // this is the only way to exercise the genuine cancel/delete-on-cancel path.
+    h.callbacks.onMoveFlow.mockImplementation(() => {
+      const source = makeCloud(51, 50, 200, 200);
+      const sink = makeCloud(52, 50, 300, 200);
+      const flow = makeFlow(
+        50,
+        'New Flow',
+        [
+          { x: 200, y: 200, attachedToUid: 51 },
+          { x: 300, y: 200, attachedToUid: 52 },
+        ],
+        { x: 250, y: 200 },
+      );
+      const view: StockFlowView = {
+        nextUid: 60,
+        elements: [source, sink, flow],
+        viewBox: { x: 0, y: 0, width: 1000, height: 1000 },
+        zoom: 1,
+        useLetteredPolarity: false,
+      };
+      h.setProps({ view, selection: new Set([50]) });
+    });
+    h.clearMountCalls();
+
+    pointerDown(h.svg, 200, 200);
+    pointerMove(h.svg, 300, 200, { buttons: 1 });
+    pointerUp(h.svg, 300, 200);
+    const editable = h.query('[contenteditable]');
+    expect(editable).not.toBeNull();
+
+    const capture = captureWindowErrors();
+    try {
+      act(() => {
+        fireEvent.keyUp(editable!, { code: 'Escape' });
+      });
+    } finally {
+      capture.stop();
+    }
+
+    expect(capture.errors).toEqual([]);
+    // Cancelling the initial name edit of a just-created flow deletes it EXACTLY
+    // once. The cancel branch calls onDeleteSelection without dereferencing the
+    // element, so it stays benign for any target and cannot double-fire.
+    expect(h.callbacks.onDeleteSelection).toHaveBeenCalledTimes(1);
+    // No rename/create on a cancel, and the editor is torn down.
+    expect(h.callbacks.onRenameVariable).not.toHaveBeenCalled();
+    expect(h.callbacks.onCreateVariable).not.toHaveBeenCalled();
+    expect(h.query('[contenteditable]')).toBeNull();
+  });
+});
diff --git a/src/diagram/tests/editor-flow-attach-failure.test.ts b/src/diagram/tests/editor-flow-attach-failure.test.ts
new file mode 100644
index 000000000..301a23d59
--- /dev/null
+++ b/src/diagram/tests/editor-flow-attach-failure.test.ts
@@ -0,0 +1,147 @@
+/**
+ * @jest-environment jsdom
+ *
+ * Copyright 2026 The Simlin Authors. All rights reserved.
+ * Use of this source code is governed by the Apache License,
+ * Version 2.0, that can be found in the LICENSE file.
+ */
+
+// Issue #820: a failed flow-attach patch must not silently discard the drawn
+// flow. handleFlowAttach used to early-return when the model-level patch failed,
+// leaving the drawn flow uncommitted -- so the flow the user just drew vanished
+// (the only feedback a transient toast) AND the just-created-flow name edit was
+// left selecting a flow that was not in the view (the getElementByUid crash).
+//
+// The fix commits the optimistic view regardless of patch success, matching the
+// sibling handlers (handleCreateVariable / handleSelectionDelete): the drawn
+// flow stays on the canvas (and stays a real, selectable element), while the
+// engine error still surfaces via the toast.
+//
+// This drives the real Editor + a real ProjectController wired to a fake engine
+// scripted to reject every applyPatch (the corrupt-project failure mode). Canvas
+// is mocked to a null renderer that captures the props (notably onMoveFlow =
+// handleFlowAttach) so the flow-attach can be invoked directly.
+
+import { TextEncoder, TextDecoder } from 'util';
+Object.assign(globalThis, { TextEncoder, TextDecoder });
+
+import * as React from 'react';
+import { act, render, screen } from '@testing-library/react';
+
+import type { FlowViewElement, StockFlowView, ViewElement } from '@simlin/core/datamodel';
+import { Project as EngineProject } from '@simlin/engine';
+import { inCreationCloudUid, fauxCloudTargetUid } from '../drawing/creation-sentinels';
+
+import { makeFakeEngine, validProjectJson } from './fake-engine';
+
+// Capture the props the Editor hands to Canvas so a test can invoke onMoveFlow
+// (handleFlowAttach) directly and read back the committed view/selection.
+let capturedCanvasProps: Record<string, unknown> | undefined;
+jest.mock('../drawing/Canvas', () => ({
+  __esModule: true,
+  Canvas: (props: Record<string, unknown>) => {
+    capturedCanvasProps = props;
+    return null;
+  },
+  inCreationUid: -2,
+}));
+
+import { Editor, type EditorProps } from '../Editor';
+
+function makeProps(overrides: Partial<EditorProps> = {}): EditorProps {
+  return {
+    inputFormat: 'json',
+    initialProjectJson: validProjectJson(),
+    initialProjectVersion: 1,
+    name: 'test-project',
+    onSave: async () => 1,
+    ...overrides,
+  } as EditorProps;
+}
+
+async function flushTimers(): Promise<void> {
+  for (let i = 0; i < 5; i++) {
+    await act(async () => {
+      await new Promise((resolve) => setTimeout(resolve, 0));
+    });
+  }
+}
+
+// An in-creation flow drawn out of empty space toward empty space: source
+// staged on the in-creation source cloud, sink on the faux sink target. This is
+// the element Canvas passes to onMoveFlow at pointer-up, with targetUid 0 (no
+// snap target) and a faux target center for the released sink position.
+function inCreationFlow(): FlowViewElement {
+  return {
+    type: 'flow',
+    uid: -2, // inCreationUid
+    var: undefined,
+    name: 'New Flow',
+    ident: 'new_flow',
+    x: 200,
+    y: 200,
+    labelSide: 'bottom',
+    isZeroRadius: false,
+    points: [
+      { x: 200, y: 200, attachedToUid: inCreationCloudUid },
+      { x: 200, y: 200, attachedToUid: fauxCloudTargetUid },
+    ],
+  };
+}
+
+describe('Editor flow-attach patch failure (issue #820)', () => {
+  afterEach(() => {
+    jest.restoreAllMocks();
+    capturedCanvasProps = undefined;
+  });
+
+  it('preserves the drawn flow (and selects a real element) when the attach patch fails', async () => {
+    // A fake engine that rejects every applyPatch models the corrupt-project
+    // failure mode the issue was observed against.
+    const engine = makeFakeEngine({ applyPatchThrows: true, json: validProjectJson() });
+    jest.spyOn(EngineProject, 'openJson').mockResolvedValue(engine as unknown as EngineProject);
+
+    act(() => {
+      render(React.createElement(Editor, makeProps()));
+    });
+    await flushTimers();
+
+    const props = capturedCanvasProps;
+    if (!props) {
+      throw new Error('Editor never rendered Canvas');
+    }
+    const onMoveFlow = props.onMoveFlow as (
+      flow: FlowViewElement,
+      targetUid: number,
+      delta: { x: number; y: number },
+      fauxTargetCenter: { x: number; y: number } | undefined,
+      inCreation: boolean,
+      isSourceAttach?: boolean,
+    ) => Promise<void>;
+
+    await act(async () => {
+      await onMoveFlow(inCreationFlow(), 0, { x: -100, y: 0 }, { x: 300, y: 200 }, true, false);
+    });
+    await flushTimers();
+
+    // The committed view must contain the drawn flow -- it was NOT discarded.
+    const committedView = capturedCanvasProps!.view as StockFlowView;
+    const flows = committedView.elements.filter((e: ViewElement): e is FlowViewElement => e.type === 'flow');
+    expect(flows).toHaveLength(1);
+    const flow = flows[0];
+
+    // The flow carries a real (committed, non-sentinel) uid.
+    expect(flow.uid).toBeGreaterThan(0);
+
+    // The selection references that real, in-view element -- no phantom that a
+    // later name-edit commit would dereference and crash on.
+    const selection = capturedCanvasProps!.selection as ReadonlySet<number>;
+    expect(selection.has(flow.uid)).toBe(true);
+    expect(committedView.elements.some((e: ViewElement) => e.uid === flow.uid)).toBe(true);
+
+    // The failure is NOT swallowed: the controller's onError surfaces the engine
+    // error as a toast (the non-silent feedback the preserve-the-flow UX relies
+    // on). The fake engine rejects with message 'patch rejected'.
+    expect(screen.getAllByText('patch rejected').length).toBeGreaterThan(0);
+  });
+});

From 52fcb31fab376c9fd5886170ad6968f6b7b976ba Mon Sep 17 00:00:00 2001
From: Bobby Powers <bobbypowers@gmail.com>
Date: Wed, 1 Jul 2026 10:15:48 -0700
Subject: [PATCH 3/4] core: unify stockFlowViewToJson, delete the diagram-local
 copy

The StockFlowView -> JsonView serializer existed twice: the
@simlin/core/datamodel implementation (used by model serialization and
merge-live-view) and an independent copy in diagram/view-conversion.ts
(used by Editor and project-controller). The same view could therefore
serialize differently depending on code path, in exactly the save and
live-view-merge paths recent flow-editing bugs went through. All
callers now use the core implementation and the diagram copy is
deleted.

The two implementations were byte-identical for every already-correct
case; the diagram copy's genuine divergences were all bugs, fixed by
the unification: link polarity and the view's useLetteredPolarity flag
were silently dropped from saves, a link carrying both arc and
multiPoint emitted both (the engine treats them as alternatives, arc
first), and a viewBox with one zero dimension was discarded. Each
resolution matches the engine's json.rs semantics, pinned by new
round-trip tests through the real deserializers, including the
Z-shaped-flow unattached-corner case. The remaining view-fidelity gaps
(ViewElementCompat, View.font, group is_mdl_view_marker, View.name)
are dropped identically by both implementations in a different layer
and stay tracked as issue 811.
---
 src/core/tests/datamodel.test.ts             | 120 ++++++++++++++
 src/diagram/CLAUDE.md                        |   1 -
 src/diagram/Editor.tsx                       |   2 +-
 src/diagram/project-controller.ts            |   2 +-
 src/diagram/tests/project-controller.test.ts |   8 +-
 src/diagram/view-conversion.ts               | 158 -------------------
 6 files changed, 128 insertions(+), 163 deletions(-)
 delete mode 100644 src/diagram/view-conversion.ts

diff --git a/src/core/tests/datamodel.test.ts b/src/core/tests/datamodel.test.ts
index ef6b684e0..642bea242 100644
--- a/src/core/tests/datamodel.test.ts
+++ b/src/core/tests/datamodel.test.ts
@@ -932,6 +932,126 @@ describe('StockFlowView', () => {
   });
 });
 
+// These pin the fields the (now-deleted) diagram-local stockFlowViewToJson copy
+// silently dropped, so unifying every diagram caller onto this implementation
+// cannot regress them (issue #821, an #811-class field-drop cluster).
+describe('StockFlowView serialization completeness (#821)', () => {
+  it('preserves link polarity through a round-trip', () => {
+    for (const polarity of ['+', '-'] as const) {
+      const link: LinkViewElement = {
+        type: 'link',
+        uid: 4,
+        fromUid: 1,
+        toUid: 2,
+        arc: 30,
+        isStraight: false,
+        multiPoint: undefined,
+        polarity,
+        x: NaN,
+        y: NaN,
+        isZeroRadius: false,
+        ident: undefined,
+      };
+      const json = stockFlowViewToJson({
+        nextUid: 5,
+        elements: [link],
+        viewBox: { x: 0, y: 0, width: 0, height: 0 },
+        zoom: -1,
+        useLetteredPolarity: false,
+      });
+      const linkJson = json.elements[0] as JsonLinkViewElement;
+      expect(linkJson.polarity).toBe(polarity);
+      expect(linkViewElementFromJson(linkJson).polarity).toBe(polarity);
+    }
+  });
+
+  it('treats arc and multiPoint as mutually exclusive (arc wins, matching fromJson)', () => {
+    const link: LinkViewElement = {
+      type: 'link',
+      uid: 4,
+      fromUid: 1,
+      toUid: 2,
+      arc: 45,
+      isStraight: false,
+      multiPoint: [{ x: 1, y: 2, attachedToUid: undefined }],
+      polarity: undefined,
+      x: NaN,
+      y: NaN,
+      isZeroRadius: false,
+      ident: undefined,
+    };
+    const json = stockFlowViewToJson({
+      nextUid: 5,
+      elements: [link],
+      viewBox: { x: 0, y: 0, width: 0, height: 0 },
+      zoom: -1,
+      useLetteredPolarity: false,
+    });
+    const linkJson = json.elements[0] as JsonLinkViewElement;
+    expect(linkJson.arc).toBe(45);
+    expect(linkJson.multiPoints).toBeUndefined();
+  });
+
+  it('preserves useLetteredPolarity when set', () => {
+    const json = stockFlowViewToJson({
+      nextUid: 1,
+      elements: [],
+      viewBox: { x: 0, y: 0, width: 0, height: 0 },
+      zoom: -1,
+      useLetteredPolarity: true,
+    });
+    expect(json.useLetteredPolarity).toBe(true);
+    const restored = stockFlowViewFromJson(json, new Map());
+    expect(restored.useLetteredPolarity).toBe(true);
+  });
+
+  it('keeps a viewBox with a single non-zero dimension (OR, not AND)', () => {
+    const json = stockFlowViewToJson({
+      nextUid: 1,
+      elements: [],
+      viewBox: { x: 0, y: 0, width: 800, height: 0 },
+      zoom: -1,
+      useLetteredPolarity: false,
+    });
+    expect(json.viewBox).toEqual({ x: 0, y: 0, width: 800, height: 0 });
+  });
+
+  it('round-trips a Z-shape flow whose corner points are unattached (#819)', () => {
+    const flow: FlowViewElement = {
+      type: 'flow',
+      uid: 10,
+      name: 'Births',
+      ident: 'births',
+      var: undefined,
+      x: 50,
+      y: 200,
+      labelSide: 'center',
+      points: [
+        { x: 0, y: 200, attachedToUid: undefined },
+        { x: 50, y: 200, attachedToUid: undefined },
+        { x: 50, y: 100, attachedToUid: undefined },
+        { x: 100, y: 100, attachedToUid: 1 },
+      ],
+      isZeroRadius: false,
+    };
+    const json = stockFlowViewToJson({
+      nextUid: 11,
+      elements: [flow],
+      viewBox: { x: 0, y: 0, width: 0, height: 0 },
+      zoom: -1,
+      useLetteredPolarity: false,
+    });
+    const flowJson = json.elements[0] as JsonFlowViewElement;
+    // Unattached corners omit attachedToUid entirely; the attached corner keeps it.
+    expect(flowJson.points.map((p) => p.attachedToUid)).toEqual([undefined, undefined, undefined, 1]);
+    for (const p of flowJson.points.slice(0, 3)) {
+      expect('attachedToUid' in p).toBe(false);
+    }
+    const restored = flowViewElementFromJson(flowJson);
+    expect(restored.points.map((p) => p.attachedToUid)).toEqual([undefined, undefined, undefined, 1]);
+  });
+});
+
 describe('canBeModuleInput and isPublic', () => {
   it('should default canBeModuleInput and isPublic to false for Stock', () => {
     const json = { name: 'pop', inflows: [], outflows: [] };
diff --git a/src/diagram/CLAUDE.md b/src/diagram/CLAUDE.md
index ecba3b89e..6cbe8c3d1 100644
--- a/src/diagram/CLAUDE.md
+++ b/src/diagram/CLAUDE.md
@@ -20,7 +20,6 @@ For build/test/lint commands, see [docs/dev/commands.md](/docs/dev/commands.md).
 - `group-movement.ts` -- Group manipulation and movement logic
 - `flow-attach.ts` -- Pure functional core for flow attach/reattach/create (`computeFlowAttachment`): the source/sink endpoint and `updateStockFlows` op builders extracted from `Editor.handleFlowAttach`. Returns the new elements, model ops, and selection; the Editor shell applies them.
 - `selection-logic.ts` -- Selection-set arithmetic (mouse-down/up selection decisions, deferred-single-select, reattachment override, pointer-state reset). The pure slice the Canvas interaction model composes.
-- `view-conversion.ts` -- View coordinate conversions
 - `arc-utils.ts` -- Arc geometry helpers (`radToDeg`, `degToRad`, arc math)
 - `keyboard-shortcuts.ts` -- Keyboard shortcut handling
 - `StaticDiagram.tsx` -- Static (non-interactive) diagram renderer
diff --git a/src/diagram/Editor.tsx b/src/diagram/Editor.tsx
index bda8aca7e..6e2f8bdf7 100644
--- a/src/diagram/Editor.tsx
+++ b/src/diagram/Editor.tsx
@@ -15,7 +15,6 @@ import { canonicalize } from '@simlin/core/canonicalize';
 
 import { Project as EngineProject } from '@simlin/engine';
 import type { JsonProjectPatch, JsonModelOperation, JsonSimSpecs, JsonArrayedEquation } from '@simlin/engine';
-import { stockFlowViewToJson } from './view-conversion';
 import {
   Project,
   Model,
@@ -37,6 +36,7 @@ import {
   auxToJson,
   moduleToJson,
   arrayedEquationToJson,
+  stockFlowViewToJson,
   type ModuleReference,
 } from '@simlin/core/datamodel';
 import { defined, exists, setsEqual } from '@simlin/core/common';
diff --git a/src/diagram/project-controller.ts b/src/diagram/project-controller.ts
index d84a1da63..ad4d21a76 100644
--- a/src/diagram/project-controller.ts
+++ b/src/diagram/project-controller.ts
@@ -36,13 +36,13 @@ import {
   projectAttachData,
   findNonFiniteViewCoord,
   stockFlowViewFromJson,
+  stockFlowViewToJson,
 } from '@simlin/core/datamodel';
 import { defined, mapSet, setsEqual, toInt, uint8ArraysEqual, type Series } from '@simlin/core/common';
 import { first, getOrThrow } from '@simlin/core/collections';
 import type { JsonProjectPatch, JsonModelOperation, ErrorDetail, JsonProject } from '@simlin/engine';
 import { SimlinErrorKind, SimlinUnitErrorKind } from '@simlin/engine';
 
-import { stockFlowViewToJson } from './view-conversion';
 import { preserveLiveView } from './merge-live-view';
 import { advanceProjectHistory } from './project-history';
 import { type ModuleStackEntry, currentModelName, pushModule, popModule, navigateToLevel } from './module-navigation';
diff --git a/src/diagram/tests/project-controller.test.ts b/src/diagram/tests/project-controller.test.ts
index cb74f0eef..7af6a3321 100644
--- a/src/diagram/tests/project-controller.test.ts
+++ b/src/diagram/tests/project-controller.test.ts
@@ -12,12 +12,16 @@
 // error cache, navigation, and snapshot immutability/coalescing -- all against
 // the FakeEngineProject helper, with no jsdom or real WASM.
 
-import { projectFromJson, type StockFlowView, type StockViewElement } from '@simlin/core/datamodel';
+import {
+  projectFromJson,
+  stockFlowViewToJson,
+  type StockFlowView,
+  type StockViewElement,
+} from '@simlin/core/datamodel';
 import type { JsonProject, ErrorDetail } from '@simlin/engine';
 import { SimlinErrorKind } from '@simlin/engine';
 
 import { ProjectController, MaxUndoSize, type ProjectSnapshot } from '../project-controller';
-import { stockFlowViewToJson } from '../view-conversion';
 import {
   makeFakeEngine,
   makeControllerConfig,
diff --git a/src/diagram/view-conversion.ts b/src/diagram/view-conversion.ts
deleted file mode 100644
index 6165e9b2f..000000000
--- a/src/diagram/view-conversion.ts
+++ /dev/null
@@ -1,158 +0,0 @@
-// Copyright 2026 The Simlin Authors. All rights reserved.
-// Use of this source code is governed by the Apache License,
-// Version 2.0, that can be found in the LICENSE file.
-
-import type {
-  JsonView,
-  JsonViewElement,
-  JsonRect,
-  JsonFlowPoint,
-  JsonLinkPoint,
-  JsonLinkViewElement,
-} from '@simlin/engine';
-
-import { ViewElement, StockFlowView, Rect, Point } from '@simlin/core/datamodel';
-
-function rectToJson(rect: Rect): JsonRect {
-  return {
-    x: rect.x,
-    y: rect.y,
-    width: rect.width,
-    height: rect.height,
-  };
-}
-
-function pointToFlowPoint(point: Point): JsonFlowPoint {
-  return {
-    x: point.x,
-    y: point.y,
-    attachedToUid: point.attachedToUid,
-  };
-}
-
-function pointToLinkPoint(point: Point): JsonLinkPoint {
-  return {
-    x: point.x,
-    y: point.y,
-  };
-}
-
-function elementToJson(element: ViewElement): JsonViewElement {
-  switch (element.type) {
-    case 'stock':
-      return {
-        type: 'stock',
-        uid: element.uid,
-        name: element.name,
-        x: element.x,
-        y: element.y,
-        labelSide: element.labelSide,
-      };
-
-    case 'flow':
-      return {
-        type: 'flow',
-        uid: element.uid,
-        name: element.name,
-        x: element.x,
-        y: element.y,
-        points: element.points.map(pointToFlowPoint),
-        labelSide: element.labelSide,
-      };
-
-    case 'aux':
-      return {
-        type: 'aux',
-        uid: element.uid,
-        name: element.name,
-        x: element.x,
-        y: element.y,
-        labelSide: element.labelSide,
-      };
-
-    case 'cloud':
-      return {
-        type: 'cloud',
-        uid: element.uid,
-        flowUid: element.flowUid,
-        x: element.x,
-        y: element.y,
-      };
-
-    case 'link': {
-      const result: JsonLinkViewElement = {
-        type: 'link',
-        uid: element.uid,
-        fromUid: element.fromUid,
-        toUid: element.toUid,
-      };
-
-      if (element.arc !== undefined) {
-        result.arc = element.arc;
-      }
-
-      if (element.multiPoint) {
-        result.multiPoints = element.multiPoint.map(pointToLinkPoint);
-      }
-
-      return result;
-    }
-
-    case 'module':
-      return {
-        type: 'module',
-        uid: element.uid,
-        name: element.name,
-        x: element.x,
-        y: element.y,
-        labelSide: element.labelSide,
-      };
-
-    case 'alias':
-      return {
-        type: 'alias',
-        uid: element.uid,
-        aliasOfUid: element.aliasOfUid,
-        x: element.x,
-        y: element.y,
-        labelSide: element.labelSide,
-      };
-
-    case 'group':
-      // GroupViewElement stores center-based x/y internally, convert to top-left for JSON
-      return {
-        type: 'group',
-        uid: element.uid,
-        name: element.name,
-        x: element.x - element.width / 2,
-        y: element.y - element.height / 2,
-        width: element.width,
-        height: element.height,
-      };
-  }
-}
-
-/**
- * Convert a StockFlowView (datamodel) to a JsonView (engine).
- */
-export function stockFlowViewToJson(view: StockFlowView): JsonView {
-  const elements: JsonViewElement[] = [];
-
-  for (const element of view.elements) {
-    elements.push(elementToJson(element));
-  }
-
-  const result: JsonView = {
-    elements,
-  };
-
-  if (view.viewBox && view.viewBox.width > 0 && view.viewBox.height > 0) {
-    result.viewBox = rectToJson(view.viewBox);
-  }
-
-  if (view.zoom > 0) {
-    result.zoom = view.zoom;
-  }
-
-  return result;
-}

From 0ecdc68843509254e887a02e9264215f5e3c5ff6 Mon Sep 17 00:00:00 2001
From: Bobby Powers <bobbypowers@gmail.com>
Date: Wed, 1 Jul 2026 10:21:00 -0700
Subject: [PATCH 4/4] diagram: remove the dead non-cloud branch from
 adjustFlows

adjustFlows carried two deliberately-different valve-fraction
formulas selected by an optional isCloud parameter, but its single
caller (UpdateCloudAndFlow's straight-flow parallel-drag path) always
passed true, so the signed non-cloud formula could never execute. The
cloud formula is now inlined and the parameter removed; the surviving
comment keeps the load-bearing rationale (mirror the valve around the
segment with an absolute scaled offset so an off-center valve stays
between the ends when a drag flips segment orientation, plus the
zero-denominator guard).

Behavior-preserving: the inlined body is character-identical to the
old isCloud branch, and the pre-existing characterization tests
(off-center, centered, zero-delta, non-finite guard, degenerate
self-loop) pass unchanged.
---
 src/diagram/drawing/Flow.tsx           | 78 +++++++++-----------------
 src/diagram/tests/flow-routing.test.ts | 18 +++---
 2 files changed, 34 insertions(+), 62 deletions(-)

diff --git a/src/diagram/drawing/Flow.tsx b/src/diagram/drawing/Flow.tsx
index 1fa68fd33..6d08a9b18 100644
--- a/src/diagram/drawing/Flow.tsx
+++ b/src/diagram/drawing/Flow.tsx
@@ -575,7 +575,6 @@ function adjustFlows(
   origStock: StockViewElement | CloudViewElement,
   stock: StockViewElement | CloudViewElement,
   flows: readonly FlowViewElement[],
-  isCloud?: boolean,
 ): readonly FlowViewElement[] {
   return flows.map((flow: FlowViewElement) => {
     let horizontal = isHorizontal(flow);
@@ -695,57 +694,32 @@ function adjustFlows(
       };
     }
 
-    // The isCloud and non-cloud branches deliberately compute the valve
-    // differently and must NOT be unified. isCloud mirrors the valve around the
-    // segment: base = min(otherEnd, moved end) plus the ABSOLUTE scaled offset,
-    // so an off-center valve stays between the two ends even when the drag flips
-    // the segment orientation (the moved end crossing past otherEnd). The
-    // non-cloud branch offsets by the SIGNED scaled distance from otherEnd --
-    // correct for a stock endpoint that keeps its side, but divergent from the
-    // cloud formula for off-center / sign-flip cases. The shared arithmetic (the
-    // raw fraction, `d`) is too small and entangled with these differing
-    // combinators to factor out without obscuring both formulas.
+    // Re-place the valve by mirroring it around the segment: base =
+    // min(otherEnd, moved end) plus the ABSOLUTE scaled offset, so an off-center
+    // valve stays between the two ends even when the drag flips the segment
+    // orientation (the moved end crossing past otherEnd).
     //
-    // NOTE: the non-cloud branch is currently unreachable -- adjustFlows' sole
-    // caller (UpdateCloudAndFlow, straight-flow parallel drag) always passes
-    // isCloud=true. It is retained as the general signed formula.
-    if (isCloud) {
-      // Guard the denominators against zero: for a vertical flow origStock.x ===
-      // otherEnd.x (and likewise y for a horizontal flow), which without the
-      // `|| 1` divides by zero and yields a NaN/Infinity valve -- serialized to
-      // JSON null, that bricks the model (#818).
-      const fraction = {
-        x: flow.x === otherEnd.x ? 0.5 : (stock.x - otherEnd.x) / (origStock.x - otherEnd.x || 1),
-        y: flow.y === otherEnd.y ? 0.5 : (stock.y - otherEnd.y) / (origStock.y - otherEnd.y || 1),
-      };
-      const d = {
-        x: flow.x === otherEnd.x ? stock.x - otherEnd.x : flow.x - otherEnd.x,
-        y: flow.y === otherEnd.y ? stock.y - otherEnd.y : flow.y - otherEnd.y,
-      };
-      const base = {
-        x: Math.min(otherEnd.x, stock.x),
-        y: Math.min(otherEnd.y, stock.y),
-      };
-      flow = {
-        ...flow,
-        x: base.x + Math.abs(fraction.x * d.x),
-        y: base.y + Math.abs(fraction.y * d.y),
-      };
-    } else {
-      const fraction = {
-        x: (stock.x - otherEnd.x) / (origStock.x - otherEnd.x || 1),
-        y: (stock.y - otherEnd.y) / (origStock.y - otherEnd.y || 1),
-      };
-      const d = {
-        x: flow.x - otherEnd.x,
-        y: flow.y - otherEnd.y,
-      };
-      flow = {
-        ...flow,
-        x: otherEnd.x + fraction.x * d.x,
-        y: otherEnd.y + fraction.y * d.y,
-      };
-    }
+    // Guard the denominators against zero: for a vertical flow origStock.x ===
+    // otherEnd.x (and likewise y for a horizontal flow), which without the `|| 1`
+    // divides by zero and yields a NaN/Infinity valve -- serialized to JSON null,
+    // that bricks the model (#818).
+    const fraction = {
+      x: flow.x === otherEnd.x ? 0.5 : (stock.x - otherEnd.x) / (origStock.x - otherEnd.x || 1),
+      y: flow.y === otherEnd.y ? 0.5 : (stock.y - otherEnd.y) / (origStock.y - otherEnd.y || 1),
+    };
+    const d = {
+      x: flow.x === otherEnd.x ? stock.x - otherEnd.x : flow.x - otherEnd.x,
+      y: flow.y === otherEnd.y ? stock.y - otherEnd.y : flow.y - otherEnd.y,
+    };
+    const base = {
+      x: Math.min(otherEnd.x, stock.x),
+      y: Math.min(otherEnd.y, stock.y),
+    };
+    flow = {
+      ...flow,
+      x: base.x + Math.abs(fraction.x * d.x),
+      y: base.y + Math.abs(fraction.y * d.y),
+    };
 
     return { ...flow, points };
   });
@@ -951,7 +925,7 @@ export function UpdateCloudAndFlow(
       y: proposed.y,
     };
 
-    flow = first(adjustFlows(origCloud, cloud, [flow], true));
+    flow = first(adjustFlows(origCloud, cloud, [flow]));
     return [cloud, flow];
   }
 
diff --git a/src/diagram/tests/flow-routing.test.ts b/src/diagram/tests/flow-routing.test.ts
index 460d22999..a4cd5f1dd 100644
--- a/src/diagram/tests/flow-routing.test.ts
+++ b/src/diagram/tests/flow-routing.test.ts
@@ -2661,13 +2661,12 @@ describe('Flow routing', () => {
     });
   });
 
-  describe('UpdateCloudAndFlow - valve fraction on parallel cloud drag (adjustFlows isCloud branch)', () => {
+  describe('UpdateCloudAndFlow - valve fraction on parallel cloud drag (adjustFlows valve formula)', () => {
     // A parallel drag of a straight cloud flow constrains the cloud to the flow
-    // axis and re-places the valve via adjustFlows' isCloud branch, which
-    // preserves the valve's fractional position between the fixed other end and
-    // the moved cloud using base=min(otherEnd, cloud) + abs(fraction*d). These
-    // pin that formula's output (the ONLY reachable adjustFlows branch: its sole
-    // caller passes isCloud=true).
+    // axis and re-places the valve via adjustFlows, which preserves the valve's
+    // fractional position between the fixed other end and the moved cloud using
+    // base=min(otherEnd, cloud) + abs(fraction*d). These pin that formula's
+    // output.
     // Fixture: source point at x=100 (uid 1), cloud sink at x=200 (uid 3), flow
     // horizontal at y=100; drag the cloud right by 20 (cloud -> x=220).
     const makeParallelCase = (valveX: number) => {
@@ -3076,10 +3075,9 @@ describe('Flow routing', () => {
     // adjustFlows computes the valve's fractional position by dividing by
     // (origStock.x - otherEnd.x) / (origStock.y - otherEnd.y). For a flow whose
     // attached endpoint and other end share an axis (a vertical or horizontal
-    // flow) that denominator is zero. The non-cloud branch guarded it with
-    // `|| 1`, but the cloud branch did not -- so dragging a cloud on such a flow
-    // produced a NaN/Infinity valve coordinate, which serialized to JSON null and
-    // bricked the model. Moving must always yield finite coordinates.
+    // flow) that denominator is zero. Before the `|| 1` guard, dragging a cloud
+    // on such a flow produced a NaN/Infinity valve coordinate, which serialized to
+    // JSON null and bricked the model. Moving must always yield finite coordinates.
     it('keeps the valve finite for a vertical cloud flow with an off-axis valve', () => {
       // Vertical flow: cloud (uid 1) and the other end (uid 2) share x = 100.
       // The valve sits off that axis (x = 150), as can happen with the degenerate