diagram: flow-editing audit hardening (rename corruption, name-edit UX, routing robustness)

docs/README.md

@@ -19,6 +19,7 @@
 - [dev/python.md](dev/python.md) -- Python (pysimlin) development standards
 - [dev/workflow.md](dev/workflow.md) -- Problem-solving philosophy and TDD workflow
 - [dev/frontend-audit-2026-06.md](dev/frontend-audit-2026-06.md) -- June 2026 frontend bug audit of src/app and src/diagram: findings, fix status, tracked follow-ups
+- [dev/flow-editing-audit-2026-06.md](dev/flow-editing-audit-2026-06.md) -- June 2026 deep audit of interactive flow editing: rename/stale-view corruption chain, name-editor UX, routing robustness, group-movement dedup
 
 ## Project Management
 

docs/dev/flow-editing-audit-2026-06.md

@@ -0,0 +1,147 @@
+# Flow-Editing Audit (June 2026)
+
+Deep audit of the interactive flow-editing path in `src/diagram` (the TypeScript
+editor -- not the engine's auto-layout). Sources: full read of `drawing/Flow.tsx`,
+`flow-attach.ts`, `group-movement.ts`, `drawing/Canvas.tsx`,
+`drawing/canvas-interaction.ts`, the Editor/controller flow handlers, and the
+flow test suites; plus live exploration against the dev server (the
+`bpowers/fooz` scratch project). This document records what was found and which
+fixes were chosen, so the PR built from it has a durable rationale.
+
+## P1: Correctness bugs found live
+
+### 1. Rename leaves the live view stale; the next edit persists the divergence
+
+`Editor.handleRename` is the only handler that mutates the view through a patch
+op (`upsertView` bundled with `renameVariable`) instead of going through
+`ProjectController.updateView`. The patch applies correctly in the engine and
+the save is correct, but `updateProject`'s unconditional `preserveLiveView`
+keeps the active model's (pre-rename) live view, so the on-screen label never
+changes -- the rename looks like it silently failed. Any subsequent
+geometry-editing gesture then round-trips that stale view back through
+`updateView`, overwriting the engine's corrected view: the model says the new
+ident, the view says the old one, and the divergence is saved.
+
+Observed end-to-end in `bpowers/fooz`: a flow rename left the model variable
+`drain_` with a view element still named "New Flow 1"; clicking that flow's
+valve crashes the editor on every page load (see finding 3).
+
+Fix: `handleRename` applies the content op (`renameVariable`) via
+`applyPatchOrReportError` and commits the renamed view through
+`updateView(updatedView, { recordHistory: true })` like every other discrete
+edit, so the optimistic live view is the renamed one. Defense in depth: the
+controller's `applyPatch` path now adopts the engine view (skips
+`preserveLiveView`) when the patch it just applied contains an `upsertView` op
+for the active model -- an explicit view op is newer user intent than the
+optimistic live view by construction.
+
+### 2. Plain Enter does not commit a name edit
+
+`EditableLabel.handleKeyPress` committed only on ctrl/shift/alt+Enter; a plain
+Enter inserted a newline into the (multi-line-capable) Slate label. Every
+"create element, type name, press Enter" interaction therefore left the editor
+open with a trailing newline staged. On click-away the name committed with the
+embedded newline; `handleRename` escaped it (`replace('\n', '\\n')`, first
+occurrence only) and `canonicalize` mapped the result to idents like `drain_`.
+
+Fix: plain Enter (and NumpadEnter) commits; shift+Enter inserts the newline
+(standard convention, still supporting multi-line labels); Escape still
+cancels. Name commits are sanitized: trailing/leading whitespace and blank
+lines are trimmed, interior newlines are preserved as intentional line breaks,
+and a commit that produces an empty name is treated as cancel (which for a
+just-created flow keeps the existing delete-on-cancel semantics).
+
+### 3. A model/view divergence crashes the whole editor and cannot be repaired
+
+`Editor.getDetails` resolves the selected element's variable with `getOrThrow`
+during render, so a view element whose variable is missing takes down the
+entire editor via the ErrorBoundary. Because the details panel is ALSO the only
+delete affordance, and there is no keyboard delete, a corrupt element cannot be
+removed: clicking it to delete crashes first. (Same hardening class as the
+dangling-ref rendering fixes #812/#817.)
+
+Fix: `getDetails` degrades gracefully (no panel + console.warn) when the
+variable is missing, and keyboard deletion (finding 4) provides a
+details-independent repair path.
+
+### 4. No keyboard delete; clouds and corrupt elements have no delete path at all
+
+The Editor's global keydown only handles undo/redo. Deletion requires opening
+the variable-details panel -- which unnamed elements (clouds) do not have, and
+which crashes for divergent elements (finding 3).
+
+Fix: Delete/Backspace deletes the current selection (guarded by
+`isEditableElement` so text fields are unaffected, and inert while a name edit
+is showing). Escape clears an armed creation tool, else the selection.
+
+## P2: Geometry robustness
+
+### 5. Slightly-diagonal legacy flows are misrouted
+
+All routing classification uses exact coordinate equality (`p1.y === p2.y`).
+The pre-fix creation bugs left persisted flows that are a few pixels off-axis
+(the fooz "New Flow" drifts 5.5px); imported models can carry the same. Such a
+visually-horizontal flow classifies as *vertical* (its endpoint y's differ), so
+dragging its stock routes an L the wrong way -- valve and label end up stacked
+on the source cloud.
+
+Fix: orientation decisions in the routing paths (`computeFlowRoute` /
+`getFlowAttachmentInfo` / `adjustFlows`) classify by the dominant axis of the
+anchor-side segment instead of exact equality. Exact equality remains the
+definition of *orthogonal* for rendering/normalization; dominance only breaks
+ties for classification so near-axis data routes the way it looks.
+
+### 6. Flow creation leaves the source endpoint at the stock's center
+
+Dragging a new flow out of a stock stages the source point at the stock CENTER
+and `computeFlowAttachment` routes only the sink, so the persisted source
+endpoint sits at the center (hidden by z-order) instead of on the stock edge,
+violating the LAYOUT.md attachment rule until the next stock drag re-pins it.
+
+Fix: at commit, creation routes the source point onto the stock edge facing the
+sink (same `UpdateCloudAndFlow` routing the sink already gets).
+
+## P3: Duplication and dead weight (behavior-preserving cleanup)
+
+- `group-movement.ts` contains the cloud L-shape conversion three times
+  (`routeCloudEndpointFlow` plus two ~50-line copies inside
+  `routeUnselectedFlows`' source/sink loops, which are themselves
+  near-duplicates); the copies' `cloudIsSelected` guard is always true by
+  construction (the maps are keyed by selected endpoints). Extracted into one
+  helper; the source/sink loops parameterized.
+- `preProcessSelectedFlows` returns a `sideEffects` array that is always
+  empty; `hasSelectedEndpoints` reduces to `selection.size > 0`;
+  `routeUnselectedFlows` takes two element iterables that every caller passes
+  identically; the deprecated `Point2D` alias. All removed/simplified.
+- `processSelectedFlow` repeats the "proposed valve, clamp to new path" block
+  three times; extracted.
+- `adjustFlows` (Flow.tsx) carries a `FIXME: reduce this duplication` --
+  near-identical cloud/non-cloud valve-fraction math whose duplication caused
+  the #818 NaN guard to be needed twice. Unified.
+- The creation sentinel UIDs are duplicated between `Canvas.tsx` and
+  `flow-attach.ts` with a "must stay in sync" comment and a pinning test;
+  moved to one shared module (`drawing/creation-sentinels.ts`) imported by
+  both.
+- The straight-flow -> L-shape conversion exists independently in
+  `UpdateCloudAndFlow`, `UpdateFlow`, and `routeCloudEndpointFlow`; the
+  perpendicular-dominance threshold logic is shared now.
+
+## Deferred (tracked as issues, not in this PR)
+
+- Straight stock-to-stock flows cannot be offset at all: the perpendicular
+  valve-drag reroute requires a cloud endpoint, and segment drags require
+  interior segments. Offsetting needs a Z-shape (two corners), a new
+  capability.
+- The failed-patch path in `handleFlowAttach` silently discards the drawn
+  flow (toast only); consider preserving the gesture or a clearer error
+  surface.
+- `bpowers/fooz` still contains the pre-fix corrupt pair (model `drain_` /
+  view "New Flow 1") plus scratch elements; after this PR the corrupt flow can
+  be deleted in the UI (keyboard delete + non-crashing details).
+
+## Testing approach
+
+Controller-level reproduction tests (fake engine) pin finding 1; jsdom
+component tests pin findings 2-4; pure-function tests pin findings 5-6 and all
+P3 refactors (characterization tests written against current behavior before
+each extraction).

src/diagram/Editor.tsx

@@ -40,7 +40,7 @@ import {
   type ModuleReference,
 } from '@simlin/core/datamodel';
 import { defined, exists, setsEqual } from '@simlin/core/common';
-import { getOrThrow, only } from '@simlin/core/collections';
+import { only } from '@simlin/core/collections';
 
 import { AuxIcon } from './AuxIcon';
 import { Toast } from './ErrorToast';
@@ -57,7 +57,7 @@ import { ModuleDetails } from './ModuleDetails';
 import { ErrorDetails } from './ErrorDetails';
 import { ZoomBar } from './ZoomBar';
 import { Canvas, inCreationUid } from './drawing/Canvas';
-import { Point, searchableName } from './drawing/common';
+import { encodeNameNewlines, Point, searchableName } from './drawing/common';
 import { computeFlowAttachment } from './flow-attach';
 import { applyGroupMovement } from './group-movement';
 import { detectUndoRedo, isEditableElement } from './keyboard-shortcuts';
@@ -557,21 +557,45 @@ export const Editor = React.memo(function Editor(props: EditorProps): React.Reac
   // `latest`/`r`, never a stale render closure, so empty deps is correct.
 
   const handleKeyDown = React.useCallback((e: KeyboardEvent): void => {
-    const { props: p } = latest.current;
-    // Don't handle shortcuts in embedded mode or editable fields
+    const { props: p, state } = latest.current;
+    // Don't handle shortcuts in embedded mode or editable fields (text
+    // inputs, the equation editor, and the canvas's inline name editor are
+    // all contenteditable/inputs, so typing there never triggers these).
     if (p.embedded || isEditableElement(e.target)) {
       return;
     }
 
     const action = detectUndoRedo(e);
-    if (!action) {
+    if (action) {
+      const isEnabled = action === 'undo' ? isUndoEnabled() : isRedoEnabled();
+      if (isEnabled) {
+        e.preventDefault();
+        handleUndoRedo(action);
+      }
       return;
     }
 
-    const isEnabled = action === 'undo' ? isUndoEnabled() : isRedoEnabled();
-    if (isEnabled) {
-      e.preventDefault();
-      handleUndoRedo(action);
+    // Escape: disarm the active creation tool first; with no tool armed,
+    // clear the selection (which also closes the details panel).
+    if (e.key === 'Escape') {
+      if (state.selectedTool !== undefined) {
+        setState({ selectedTool: undefined });
+      } else if (state.selection.size > 0) {
+        handleSelection(new Set<UID>());
+      }
+      return;
+    }
+
+    // Delete/Backspace: delete the current selection. This is the only
+    // delete affordance for unnamed elements (clouds) and for elements whose
+    // details panel cannot open, so it must not depend on the panel.
+    if (e.key === 'Delete' || e.key === 'Backspace') {
+      const readOnly = p.readOnlyMode || isStdlibModel(modelName());
+      if (!readOnly && state.selection.size > 0) {
+        e.preventDefault();
+        void handleSelectionDelete();
+      }
+      return;
     }
   }, []);
 
@@ -658,7 +682,14 @@ export const Editor = React.memo(function Editor(props: EditorProps): React.Reac
   };
 
   const handleDialClick = React.useCallback((_event: React.MouseEvent<HTMLButtonElement>): void => {
-    setState((prev) => ({ dialOpen: !prev.dialOpen }));
+    // Toggle the palette open/closed. Closing it deselects the active tool so the
+    // canvas returns to plain selection mode -- matching the load-time state where
+    // no tool is selected (the clearing was dropped in 5191a9b6 and is restored
+    // here). Opening leaves any selected tool untouched.
+    setState((prev) => ({
+      dialOpen: !prev.dialOpen,
+      selectedTool: prev.dialOpen ? undefined : prev.selectedTool,
+    }));
   }, []);
 
   const handleDialClose = React.useCallback((_e: React.SyntheticEvent, reason: CloseReason): void => {
@@ -684,7 +715,10 @@ export const Editor = React.memo(function Editor(props: EditorProps): React.Reac
 
     const view = defined(getView());
     const oldIdent = canonicalize(oldName);
-    newName = newName.replace('\n', '\\n');
+    // Encode ALL line breaks to the stored backslash-n form -- the previous
+    // single-occurrence replace left raw newlines in multi-line names, which
+    // canonicalized into malformed idents.
+    newName = encodeNameNewlines(newName);
 
     const elements = view.elements.map((element: ViewElement) => {
       if (!isNamedViewElement(element)) {
@@ -2016,7 +2050,17 @@ export const Editor = React.memo(function Editor(props: EditorProps): React.Reac
     const model = defined(getModel());
 
     const ident = defined(namedElement.ident);
-    const variable = getOrThrow(model.variables, ident);
+    // A view element whose variable is missing means the model and view have
+    // diverged (corrupted data, or a transient during multi-step updates).
+    // Throwing here took the WHOLE editor down via the ErrorBoundary during
+    // render -- and since this panel is also the delete affordance, the
+    // corrupt element became unremovable. Degrade to no panel instead; the
+    // element stays selectable and keyboard-deletable.
+    const variable = model.variables.get(ident);
+    if (variable === undefined) {
+      console.warn(`variable details unavailable: no variable '${ident}' in model '${modelName()}'`);
+      return;
+    }
 
     if (variable.type === 'module') {
       return (
@@ -2079,44 +2123,36 @@ export const Editor = React.memo(function Editor(props: EditorProps): React.Reac
     setState({ selectedTool: undefined });
   }, []);
 
+  // Clicking a tool selects it; clicking the already-active tool deselects it
+  // (toggle), returning to plain selection mode -- the load-time state.
   const handleSelectStock = React.useCallback((e: React.MouseEvent<HTMLButtonElement>): void => {
     e.preventDefault();
     e.stopPropagation();
-    setState({
-      selectedTool: 'stock',
-    });
+    setState((prev) => ({ selectedTool: prev.selectedTool === 'stock' ? undefined : 'stock' }));
   }, []);
 
   const handleSelectFlow = React.useCallback((e: React.MouseEvent<HTMLButtonElement>): void => {
     e.preventDefault();
     e.stopPropagation();
-    setState({
-      selectedTool: 'flow',
-    });
+    setState((prev) => ({ selectedTool: prev.selectedTool === 'flow' ? undefined : 'flow' }));
   }, []);
 
   const handleSelectAux = React.useCallback((e: React.MouseEvent<HTMLButtonElement>): void => {
     e.preventDefault();
     e.stopPropagation();
-    setState({
-      selectedTool: 'aux',
-    });
+    setState((prev) => ({ selectedTool: prev.selectedTool === 'aux' ? undefined : 'aux' }));
   }, []);
 
   const handleSelectLink = React.useCallback((e: React.MouseEvent<HTMLButtonElement>): void => {
     e.preventDefault();
     e.stopPropagation();
-    setState({
-      selectedTool: 'link',
-    });
+    setState((prev) => ({ selectedTool: prev.selectedTool === 'link' ? undefined : 'link' }));
   }, []);
 
   const handleSelectModule = React.useCallback((e: React.MouseEvent<HTMLButtonElement>): void => {
     e.preventDefault();
     e.stopPropagation();
-    setState({
-      selectedTool: 'module',
-    });
+    setState((prev) => ({ selectedTool: prev.selectedTool === 'module' ? undefined : 'module' }));
   }, []);
 
   // Undo/redo is fully owned by the controller: it moves the undo cursor,

src/diagram/LAYOUT.md

@@ -47,7 +47,7 @@ The main container for a system dynamics diagram with these properties:
 ```typescript
 {
   nextUid: number,              // Next available UID for new elements
-  elements: List<ViewElement>,  // All visual elements in the view
+  elements: readonly ViewElement[],  // All visual elements in the view
   viewBox: Rect,                // Bounding box of the view
   zoom: number                   // Zoom factor (negative means auto-fit)
 }
@@ -91,8 +91,8 @@ StockViewElement {
   y: number,                // Center Y coordinate
   labelSide: LabelSide,     // 'top'|'bottom'|'left'|'right'|'center'
   isZeroRadius: boolean,    // Always false for stocks
-  inflows: List<UID>,       // UIDs of inflowing flows (computed)
-  outflows: List<UID>       // UIDs of outflowing flows (computed)
+  inflows: readonly UID[],       // UIDs of inflowing flows (computed)
+  outflows: readonly UID[]       // UIDs of outflowing flows (computed)
 }
 ```
 
@@ -128,7 +128,7 @@ FlowViewElement {
   x: number,                // Valve center X
   y: number,                // Valve center Y
   labelSide: LabelSide,
-  points: List<Point>,      // Path points defining the flow
+  points: readonly Point[],      // Path points defining the flow
   isZeroRadius: boolean     // Always false for flows
 }
 
@@ -143,13 +143,23 @@ Point {
 1. **Must have at least 2 points** (source and destination)
 2. **First point** connects to source (stock or cloud)
 3. **Last point** connects to sink (stock or cloud)
-4. **Intermediate points** define the path shape (currently only 2-point flows supported)
+4. **Intermediate points** define the path shape. Multi-segment (L- and
+   Z-shaped) flows are fully supported: stocks moving off-axis produce a
+   corner point, endpoint drags with a dominant perpendicular component
+   convert a straight flow to an L, and interior segments of 3+ point flows
+   can be dragged perpendicular to their direction. `normalizeFlowPoints`
+   removes zero-length and colinear interior segments after every mutation so
+   segments strictly alternate horizontal/vertical.
 5. **Flows are constrained** to horizontal or vertical segments when connected to stocks
 6. **Stock connections**:
    - Endpoints snap to stock edges (±width/2 or ±height/2)
    - Must be at least 3px from corners
+   - Multiple flows attaching to the same stock side spread evenly along the
+     edge (n flows sit at i/(n+1) fractions; straight flows keep their
+     anchor-determined position but still reserve a slot)
 7. **Cloud connections**:
-   - Attach at CloudRadius (13.5px) distance from cloud center
+   - The endpoint sits at the cloud center; the rendered path is retracted by
+     CloudRadius (13.5px) so the arrowhead lands on the cloud's edge
    - Clouds adjust position when dragged
 
 **Path Rendering**:
@@ -272,7 +282,7 @@ LinkViewElement {
   toUid: number,            // Destination element UID
   arc?: number,             // Takeoff angle in radians (undefined = straight)
   isStraight: boolean,      // Force straight line
-  multiPoint?: List<Point>  // Multi-segment connectors (future/not implemented)
+  multiPoint?: readonly Point[]  // Multi-segment connectors (future/not implemented)
 }
 ```