Let React Flow own node state so drag frames render inside the pointer event

web_ui/src/app/canvas/SceneCanvas.tsx

@@ -8,6 +8,7 @@ import {
   ViewportPortal,
   experimental_useOnNodesChangeMiddleware,
   useReactFlow,
+  useStoreApi,
   type Connection,
   type Edge,
   type Node,
@@ -143,6 +144,9 @@ const EDGE_TYPES = {
 // fresh literal every time - React Flow reads these by reference in its
 // own internal effects, so a fresh literal every render meant those
 // effects re-ran on every unrelated re-render for no behavioral reason.
+// defaultNodes is read once when React Flow initialises its store; a stable
+// module constant keeps that unambiguous and allocation-free.
+const EMPTY_NODES: SceneFlowNode[] = [];
 const DELETE_KEY_CODES = ["Delete", "Backspace"];
 const PRO_OPTIONS = { hideAttribution: true };
 const DEFAULT_EDGE_OPTIONS = { type: "default" as const };
@@ -2259,7 +2263,12 @@ function CanvasInner({
   // Local node state exists so dragging is fluid; backend snapshots are the
   // truth and reconcile in whenever nothing is being dragged. dragStartRef
   // powers the drag-speed scaling contract (see scaleDragPosition).
-  const [nodes, setNodes] = useState<SceneFlowNode[]>([]);
+  // React Flow owns the rendered node collection (see the <ReactFlow>
+  // element's own defaultNodes comment). This component keeps only a mirror
+  // ref for its own logic - drag corrections, delete routing, scene merge -
+  // so a drag frame never has to travel through React state to reach the
+  // renderer.
+  const storeApi = useStoreApi();
   const dragStartRef = useRef<Map<string, { x: number; y: number }>>(new Map());
   const draggingRef = useRef(false);
   // ADR-011 stage 11.3: the smart-guide size cache - populated ONCE per drag
@@ -2421,7 +2430,10 @@ function CanvasInner({
     );
     // Mirror advances with the state it describes - see nodesRef's comment.
     nodesRef.current = next;
-    setNodes(next);
+    // Straight into React Flow's own store rather than through React state:
+    // this is the same setter React Flow's internal prop-sync would call, so
+    // the renderer is updated in this effect instead of one commit later.
+    storeApi.getState().setNodes(next);
   }, [
     scene, store, onOpenDocumentView, effectiveBranchFocusOriginId, onToggleBranchFocus, focusAcceptedPaths,
     getComposerRoute, filterKinds, filterStatuses,
@@ -2691,15 +2703,14 @@ function CanvasInner({
       // advances in this same event: a drag can deliver several frames
       // before React commits, and each must build on the previous frame's
       // result - see nodesRef's own comment above.
-      const next = applyNodeChanges(changes, currentNodes);
-      // react-hooks/immutability flags this because the same mirror is also
-      // assigned in the scene-sync effect above. Both writers are correct and
-      // both are required: the effect publishes backend snapshots, this
-      // handler publishes drag frames, and a drag can deliver several frames
-      // between commits - which is the entire reason the mirror exists.
+      // React Flow has ALREADY applied this batch to its own store by the
+      // time this handler runs (that is what uncontrolled node state buys:
+      // the renderer is updated synchronously inside the pointer event, the
+      // way working node editors do it, instead of waiting for React state
+      // and a post-paint sync). All that remains here is keeping this
+      // component's mirror in step for its own logic.
       // eslint-disable-next-line react-hooks/immutability
-      nodesRef.current = next;
-      setNodes(next);
+      nodesRef.current = applyNodeChanges(changes, currentNodes);
     },
     [store],
   );
@@ -2723,7 +2734,7 @@ function CanvasInner({
       const chatNodeIds: string[] = [];
       const otherNodeIds: string[] = [];
       for (const deleted of deletedNodes) {
-        const flowNode = nodes.find((n) => n.id === deleted.id);
+        const flowNode = nodesRef.current.find((n) => n.id === deleted.id);
         (flowNode?.type === "chat" ? chatNodeIds : otherNodeIds).push(deleted.id);
       }
       for (const id of chatNodeIds) store.deleteChatNode(id);
@@ -2735,7 +2746,7 @@ function CanvasInner({
         deletedEdges.filter((e) => !dying.has(e.source) && !dying.has(e.target)).map((e) => e.id),
       );
     },
-    [store, nodes],
+    [store],
   );
 
   const onSelectionChange = useCallback(
@@ -2773,7 +2784,19 @@ function CanvasInner({
       data-testid="scene-canvas"
     >
       <ReactFlow
-        nodes={nodes}
+        /* Uncontrolled node state, deliberately. With a controlled `nodes`
+           prop, every drag frame travels app state -> re-render -> React
+           Flow's prop-sync effect (a PASSIVE effect, so it lands after the
+           browser has already had a chance to paint) before the renderer
+           learns the new position. Working node editors do the opposite:
+           Drawflow, for one, writes the node's position and its connection
+           paths synchronously inside the same mousemove handler. Handing
+           node state to React Flow reproduces that shape here - a drag
+           frame is applied to its store inside the pointer event, and the
+           node card and its connections re-render together from that one
+           store write. Scene snapshots from the backend are pushed in
+           explicitly via the store (see the scene-sync effect above). */
+        defaultNodes={EMPTY_NODES}
         edges={edges}
         nodeTypes={NODE_TYPES}
         edgeTypes={EDGE_TYPES}

web_ui/src/app/canvas/drag/dragCorrections.ts

@@ -0,0 +1,193 @@
+/**
+ * The canvas drag pipeline's pure core.
+ *
+ * Dragging a node on this canvas is not "move it to the pointer". Three
+ * corrections shape every frame, and they must all be applied to the SAME
+ * position value before anything renders it:
+ *
+ *   1. the drag-speed factor (scaleDragPosition), the Qt canvas's own
+ *      contract - motion is scaled relative to where the gesture started,
+ *      so at a factor below 1 the node deliberately trails the pointer;
+ *   2. smart-guide snapping, which pulls the position onto an alignment
+ *      with other nodes;
+ *   3. the group cascade, which carries a group's members by whatever delta
+ *      the group itself ended up moving - necessarily AFTER 1 and 2, or
+ *      members ride an uncorrected delta and drift away from their group.
+ *
+ * Why this file exists at all: these corrections used to be inline in
+ * SceneCanvas's change handler, downstream of React Flow's own bookkeeping,
+ * which meant the library and this app held two different positions for the
+ * same node on every frame - and since the library computes CONNECTION
+ * geometry from its own records, a node card and the line attached to it
+ * were placed from different numbers. Correcting inside React Flow's change
+ * pipeline (see useNodeDragPipeline) makes the corrected position the only
+ * position. Keeping the maths here - pure, React-free, framework-free -
+ * is what makes that correctness testable without mounting a canvas, and
+ * keeps the rule "one position, computed once" enforceable in one place
+ * rather than spread through a component.
+ *
+ * Everything in this module is a pure function of its arguments. The only
+ * mutable state a drag needs (where each node started, the size cache) is
+ * owned by the caller and passed in, so a test can drive a hundred
+ * simulated frames deterministically.
+ */
+
+import type { NodeChange } from "@xyflow/react";
+// Type-only import: erased at compile time, so this creates no runtime
+// dependency back onto the component that consumes this module.
+import type { SceneFlowNode } from "../SceneCanvas";
+import { scaleDragPosition } from "../sceneStore";
+import type { GuideLine } from "../smartGuides";
+
+/** Where a node sat when the current gesture began, keyed by node id. */
+export type DragStartPositions = Map<string, { x: number; y: number }>;
+
+/** Node sizes captured once per gesture, for smart-guide alignment. */
+export type DragSizeCache = Map<string, { width: number; height: number }>;
+
+/**
+ * Everything a correction pass needs from the outside world. Passed in
+ * rather than reached for, so this module never depends on React, on the
+ * canvas component, or on when the caller happens to run it.
+ */
+export interface DragCorrectionContext {
+  /** The nodes as they currently stand, including any earlier frame's result. */
+  nodes: SceneFlowNode[];
+  /** Gesture-scoped start positions; this pass records newly-seen nodes into it. */
+  dragStarts: DragStartPositions;
+  /** Gesture-scoped node sizes, built once at gesture start by the caller. */
+  sizeCache: DragSizeCache;
+  /** The scene's drag-speed factor (1 = pointer-exact). */
+  dragFactor: number;
+  /** Whether smart-guide snapping is enabled for this scene. */
+  smartGuidesEnabled: boolean;
+  /** Snap + guide computation for one frame, injected so this stays pure. */
+  computeSnap: (
+    nodes: SceneFlowNode[],
+    nodeId: string,
+    position: { x: number; y: number },
+    sizeCache: DragSizeCache,
+  ) => { position: { x: number; y: number }; guides: GuideLine[] };
+  /** Group-member cascade for one corrected position, injected for the same reason. */
+  computeGroupCascade: (
+    nodes: SceneFlowNode[],
+    draggedId: string,
+    position: { x: number; y: number },
+  ) => NodeChange<SceneFlowNode>[];
+}
+
+export interface DragCorrectionResult {
+  /** The batch to hand onward: corrected changes plus any member changes. */
+  changes: NodeChange<SceneFlowNode>[];
+  /** Alignment guides this frame produced, for the caller to render. */
+  guides: GuideLine[];
+  /** True when this batch contained at least one frame of an active gesture. */
+  touchedGesture: boolean;
+}
+
+/**
+ * True when a position change belongs to a gesture this pipeline is
+ * carrying: either React Flow flagged it as dragging, or it names a node
+ * whose start position we recorded earlier in the same gesture.
+ *
+ * The second half is what catches React Flow's own drag-STOP change. That
+ * one must receive the identical correction, or the value the library
+ * settles on is not the value the user was just looking at - which is
+ * exactly the release-time jump the pre-refactor code worked around by
+ * substituting a position after the fact.
+ */
+function isGestureFrame(
+  change: NodeChange<SceneFlowNode>,
+  dragStarts: DragStartPositions,
+): change is Extract<NodeChange<SceneFlowNode>, { type: "position" }> {
+  if (change.type !== "position" || !change.position) return false;
+  return change.dragging === true || dragStarts.has(change.id);
+}
+
+/**
+ * Apply every drag correction to one batch of node changes.
+ *
+ * Non-position changes, and position changes outside an active gesture,
+ * pass through untouched - this function narrows to gesture frames only and
+ * has no opinion on anything else in the batch.
+ *
+ * Guides accumulate across every co-mover in the batch rather than being
+ * replaced per node. That is a deliberate departure from the Qt original,
+ * which cleared its guides per item and therefore only ever showed the
+ * last-processed item's alignment - an artifact of that toolkit's per-item
+ * callback ordering, not a design decision worth reproducing.
+ */
+export function correctDragChanges(
+  changes: NodeChange<SceneFlowNode>[],
+  context: DragCorrectionContext,
+): DragCorrectionResult {
+  const { nodes, dragStarts, sizeCache, dragFactor, smartGuidesEnabled } = context;
+  const guides: GuideLine[] = [];
+  const memberChanges: NodeChange<SceneFlowNode>[] = [];
+  let touchedGesture = false;
+
+  const corrected = changes.map((change) => {
+    if (!isGestureFrame(change, dragStarts)) return change;
+    touchedGesture = true;
+
+    let start = dragStarts.get(change.id);
+    if (!start) {
+      const node = nodes.find((n) => n.id === change.id);
+      start = node ? { ...node.position } : { ...change.position! };
+      dragStarts.set(change.id, start);
+    }
+
+    // 1. Drag speed, measured from the gesture's own origin.
+    let position = scaleDragPosition(start, change.position!, dragFactor);
+
+    // 2. Smart-guide snap, layered on top of React Flow's native grid snap
+    //    (which, when enabled, already ran before this change was emitted).
+    //    Guides win per axis where both apply, reproducing the Qt canvas's
+    //    own per-axis priority.
+    if (smartGuidesEnabled) {
+      const snapped = context.computeSnap(nodes, change.id, position, sizeCache);
+      position = snapped.position;
+      guides.push(...snapped.guides);
+    }
+
+    // 3. Group members ride the FINAL delta, so they stay in lockstep with
+    //    the group rather than with the raw pointer.
+    memberChanges.push(...context.computeGroupCascade(nodes, change.id, position));
+
+    return { ...change, position };
+  });
+
+  return {
+    changes: memberChanges.length > 0 ? [...corrected, ...memberChanges] : corrected,
+    guides,
+    touchedGesture,
+  };
+}
+
+/**
+ * The positions a settled gesture must persist, for the node that was
+ * released and for every group member it carried.
+ *
+ * Returned as one list because they have to be committed as a single
+ * batch: committing them one at a time makes the server publish a scene
+ * after each, and a group's bounds are derived from its members, so those
+ * intermediate publishes render as a group visibly stretching and
+ * resettling rather than merely being briefly stale.
+ */
+export function collectSettledPositions(
+  nodes: SceneFlowNode[],
+  releasedNodeId: string,
+  isGroup: (node: SceneFlowNode | undefined) => boolean,
+  membersOf: (nodes: SceneFlowNode[], group: SceneFlowNode) => Set<string>,
+): Array<{ id: string; x: number; y: number }> {
+  const settled = nodes.find((n) => n.id === releasedNodeId);
+  if (!settled) return [];
+  const positions = [{ id: releasedNodeId, x: settled.position.x, y: settled.position.y }];
+  if (isGroup(settled)) {
+    for (const memberId of membersOf(nodes, settled)) {
+      const member = nodes.find((n) => n.id === memberId);
+      if (member) positions.push({ id: memberId, x: member.position.x, y: member.position.y });
+    }
+  }
+  return positions;
+}