fix: what the review of the drag routes and the trace turned up

CHANGELOG.md

@@ -15,9 +15,11 @@ platform API version.
   piece of a cut edge drawn. The walk reads the diagram left to right: a wire
   drawn back to the left, a handshake's `ready` or a loop's feedback, is
   walked against its arrow, so a netlist's ready wires do not make a cycle of
-  every two neighbours and the trace follows the data as far as it goes. On a
-  cycle that remains, the trace is cut to the node's direct neighbours, since
-  reachability would light the whole diagram. A selected
+  every two neighbours and the trace follows the data as far as it goes. A
+  wire is read by where its two nodes stand, not their ports, so two nodes
+  that overlap across the page do not turn it. On a cycle that remains, the
+  trace is cut to the node's direct neighbours, since reachability would
+  light the whole diagram. A selected
   edge draws all its pieces and its two ends; a selected container, what feeds
   it and what it feeds, leaving its members as they are. Drawn as GLSP
   feedback, so it survives a relayout or an edit.
@@ -43,8 +45,12 @@ platform API version.
   node held still mid-drag keeps its edges hidden: a drag with no move for 5s
   is taken as one that never finishes. A bounds pass mid-drag, which draws
   the server's model in the hidden layer with the node still at its old
-  place, no longer draws the edges again for the rest of the drag. If no
-  route arrives within 1.5s of the drop the edges are drawn as they were.
+  place, no longer draws the edges again for the rest of the drag. A route
+  that reaches the ports is drawn the moment it arrives, inside the drag's
+  settling window too, instead of a client route that is swapped out
+  later. If no route arrives the edges are drawn as they were: after 1.5s,
+  plus 25ms an edge hidden and 6s at most, since a dense node's routes take
+  the router longer.
 - The server logs one `[dialogram perf] reroute` line per reroute it
   persists: edges kept, edges routed, the router and the time taken.
 
@@ -85,8 +91,8 @@ platform API version.
   move). Now an edge keeps its route and only its ends follow the ports, at
   any distance, not just under 5px as before; the router runs only for an
   edge whose kept route would not do: a straight edge with no bend to
-  carry, a stub that would turn back through its node, a segment that
-  would run through a node. An edge is rerouted for a node that moved onto
+  carry, a stub that would turn back through its node or shrink under 8px,
+  a segment that would run through a node. An edge is rerouted for a node that moved onto
   it only when the node covers it, not when it came within the router's
   padding, and only the node that moved counts, not the one at the far end
   of its edges.

packages/diagram-client/src/elk-live-drag-router.ts

@@ -147,7 +147,7 @@ function anchorOfPort(port: any): { x: number; y: number } | undefined {
 }
 
 /** Whether the edge's stored route starts and ends where its ports are now. */
-function routeReachesPorts(root: Readonly<GModelElement>, edge: Readonly<GModelElement>): boolean {
+export function routeReachesPorts(root: Readonly<GModelElement>, edge: Readonly<GModelElement>): boolean {
     const route = (edge as any).routingPoints as Array<{ x: number; y: number }> | undefined;
     if (!Array.isArray(route) || route.length < 2) {
         return false;
@@ -165,6 +165,9 @@ function routeReachesPorts(root: Readonly<GModelElement>, edge: Readonly<GModelE
 
 /** How long hidden edges may wait for the server's routes before they are drawn as they were. */
 const REVEAL_FALLBACK_MS = 1500;
+/** libavoid's cost grows faster than the edge count: a dense node's routes are given longer. */
+const REVEAL_PER_EDGE_MS = 25;
+const REVEAL_FALLBACK_MAX_MS = 6000;
 /** A finished move with no further move this long after it is the last of its series. */
 const QUIET_MS = 700;
 /** A drag with no move this long is taken as one that never finishes. */
@@ -289,7 +292,8 @@ export class WorkflowElkLiveDragRouter implements IActionHandler {
 
     private hideIncidentEdges(root: Readonly<GModelElement>): void {
         const movedIds = Array.from(this.pendingMovedElementIds).sort().join('\u0000');
-        if (movedIds === this.hiddenForMovedIds && dragHiddenEdgeIds.has(getDragKey(root))) {
+        // Once per drag, a node with no edges included (a reveal resets the key).
+        if (movedIds === this.hiddenForMovedIds) {
             return;
         }
         const { incidentEdgeIds } = this.collectIncidentEdgesFromMovedElements(root, this.pendingMovedElementIds, new Map());
@@ -308,7 +312,12 @@ export class WorkflowElkLiveDragRouter implements IActionHandler {
         if (this.revealTimer) {
             clearTimeout(this.revealTimer);
         }
-        this.revealTimer = setTimeout(() => this.revealHiddenEdges(), REVEAL_FALLBACK_MS);
+        let hidden = 0;
+        for (const ids of dragHiddenEdgeIds.values()) {
+            hidden += ids.size;
+        }
+        const wait = Math.min(REVEAL_FALLBACK_MAX_MS, REVEAL_FALLBACK_MS + REVEAL_PER_EDGE_MS * hidden);
+        this.revealTimer = setTimeout(() => this.revealHiddenEdges(), wait);
     }
 
     private revealHiddenEdges(): void {

packages/diagram-client/src/selection-trace.ts

@@ -145,6 +145,13 @@ export function buildTraceGraph(root: TraceElement): TraceGraph {
                 middle.set(element.id, x + (element.size?.width ?? 0) / 2);
             }
         }
+        // A port on a node stands where its node does: two nodes that overlap
+        // across the page (one under the other, or moved by hand) would
+        // otherwise turn a wire between them by a few pixels of port offset.
+        // A port on a group's frame keeps its own place; the frame is wide.
+        if (startsWith(element, 'port') && node !== undefined && !groups.has(node) && middle.has(node)) {
+            middle.set(element.id, middle.get(node)!);
+        }
         if (startsWith(element, 'node')) {
             graph.parentOf.set(element.id, node);
             if (node !== undefined) {

packages/diagram-client/src/views.ts

@@ -44,7 +44,7 @@ import {
 import { EDGE_OPEN_ANCHOR_X_ARG, EDGE_OPEN_ANCHOR_Y_ARG } from './edge-open-mouse-listener';
 import { clientBehavior } from './profile';
 import { captionIsShared, carriedTextOf } from './edge-caption-sharing';
-import { isEdgeHiddenForDrag, isIncidentEdgeInActiveDrag } from './elk-live-drag-router';
+import { isEdgeHiddenForDrag, isIncidentEdgeInActiveDrag, routeReachesPorts } from './elk-live-drag-router';
 import { BoundaryPortGeometry, WorkflowDiagramCss, WorkflowDiagramMetadata, WorkflowDiagramTypes, normalizePortSide } from '@dialogram/shared';
 import { WorkflowDiagramConstants } from '@dialogram/shared';
 
@@ -1772,7 +1772,11 @@ export class WorkflowEdgeView extends PolylineEdgeView {
         }
         const serverRoute = (edge as any).routingPoints as { x: number; y: number }[] | undefined;
         const hasServerRoute = Array.isArray(serverRoute) && serverRoute.length >= 2;
-        const liveRouted = isIncidentEdgeInActiveDrag(rootOfEdge(edge), String((edge as any).id));
+        // The live router stands in only while the stored route is stale: a
+        // route that reaches the ports where they are now is the server's
+        // answer for the drop, and drawing it at once saves the swap.
+        const liveRouted = isIncidentEdgeInActiveDrag(rootOfEdge(edge), String((edge as any).id))
+            && !routeReachesPorts(rootOfEdge(edge), edge as any);
         const route = hasServerRoute && !liveRouted
             ? serverRoute
             : ((this.edgeRouterRegistry?.route(edge as any, args) as any) ?? serverRoute ?? []);

packages/diagram-client/test/edges-hidden-while-dragged.test.ts

@@ -143,6 +143,19 @@ describe('edges hidden while their node is dragged', () => {
         }
     });
 
+    it('walks the model once per drag for a node with no edges', () => {
+        const m = model();
+        const e: any = { id: 'e', type: 'node:actor', parent: m.root, children: [], position: { x: 800, y: 0 }, size: { width: 140, height: 60 } };
+        m.root.children.push(e);
+        m.elements.set('e', e);
+        const { handler } = router(m.root);
+        const walks = vi.spyOn(handler as any, 'collectIncidentEdgesFromMovedElements');
+        drag(m, handler, 'e', { x: 810, y: 0 });
+        drag(m, handler, 'e', { x: 820, y: 0 });
+        drag(m, handler, 'e', { x: 830, y: 0 });
+        expect(walks).toHaveBeenCalledTimes(1);
+    });
+
     it('hides nothing for a move by arrow key', () => {
         const m = model();
         const { handler } = router(m.root);

packages/diagram-client/test/selection-trace.test.ts

@@ -141,6 +141,15 @@ describe('a selected node', () => {
         expect(sorted(result.downstream)).toEqual(['b->c', 'c', 'c->d', 'c.rdy->b.ack', 'd', 'd.rdy->c.ack']);
     });
 
+    it('reads a wire by where its nodes stand, so two nodes that overlap across the page do not turn it', () => {
+        // b sits under a, a little to the right: b's input port is left of
+        // a's output port, but b is not left of a.
+        const root = model(placed('a', 0), placed('b', 50), edge('a', 'b'));
+        const { result } = trace(root, 'a');
+        expect(sorted(result.upstream)).toEqual([]);
+        expect(sorted(result.downstream)).toEqual(['a->b', 'b']);
+    });
+
     it('reads a loop left to right: its feedback wire is on the downstream side of the node it enters', () => {
         // s1 -> s2 -> s3 -> s1 laid out left to right, the wire back to s1
         // drawn right to left, and s3 -> s4.

packages/diagram-server/src/operations/reroute-edges-avoid-overlaps-handler.ts

@@ -296,6 +296,8 @@ export class WorkflowRerouteEdgesAvoidOverlapsOperationHandler extends Operation
     // ═══════════════════════════════════════════════════════════════════
 
     private static readonly PAD = 15;      // obstacle padding around node boxes
+    /** The shortest stub a kept route may leave a port on (see `slideRoute`). */
+    private static readonly MIN_STUB_PX = 8;
     private static readonly STUB = 25;     // horizontal stub length from ports
     private static readonly EDGE_GAP = 8;  // px between parallel edges in same channel
 
@@ -977,10 +979,12 @@ export class WorkflowRerouteEdgesAvoidOverlapsOperationHandler extends Operation
         if (!isOrthogonalPolyline(slid)) { return undefined; }
 
         // The stub leaves the port outwards: an EAST port to the right, a
-        // WEST port to the left. A node moved past its bend would send the
-        // stub back through itself.
+        // WEST port to the left, and by a real length. A node moved past its
+        // bend would send the stub back through itself; one moved up to its
+        // bend would leave the port on a vertical.
+        const minStub = WorkflowRerouteEdgesAvoidOverlapsOperationHandler.MIN_STUB_PX;
         const outward = (anchor: { x: number; side: 'WEST' | 'EAST' }, bend: { x: number }): boolean =>
-            anchor.side === 'EAST' ? bend.x >= anchor.x - 0.5 : bend.x <= anchor.x + 0.5;
+            anchor.side === 'EAST' ? bend.x >= anchor.x + minStub : bend.x <= anchor.x - minStub;
         if (!outward(srcA, slid[1]) || !outward(tgtA, slid[last - 1])) { return undefined; }
 
         // No segment through a node. The stubs are allowed to touch their
@@ -1090,7 +1094,8 @@ export class WorkflowRerouteEdgesAvoidOverlapsOperationHandler extends Operation
         }
         // A foreign edge is rerouted when the node now covers it, not when
         // the node came within the router's padding of it.
-        const nodeBoxes = this.collectObstacles(root, 0);
+        const pad = WorkflowRerouteEdgesAvoidOverlapsOperationHandler.PAD;
+        const nodeBoxes = obstacles.map(o => ({ id: o.id, x: o.x + pad, y: o.y + pad, r: o.r - pad, b: o.b - pad }));
         const movedNodeBoxes = resetAll ? [] : nodeBoxes.filter(o => movedNodeIds.has(o.id));
         const edgeHitByMovedNode = (edge: GEdge): boolean => {
             if (movedNodeBoxes.length === 0) { return false; }

packages/diagram-server/src/routing/libavoid-router.ts

@@ -212,11 +212,10 @@ export async function isLibavoidAvailable(): Promise<boolean> {
  * row, and the bend after it follows, so the edges leave the port together
  * and part where their ways part.
  */
-function unkink(points: RouterPoint[], anchor: RouterPoint, nudge: number): void {
+function unkink(points: RouterPoint[], anchor: RouterPoint, nudge: number, atStart: boolean): void {
     if (points.length < 2) {
         return;
     }
-    const atStart = Math.abs(points[0].x - anchor.x) <= Math.abs(points[points.length - 1].x - anchor.x);
     const i0 = atStart ? 0 : points.length - 1;
     const i1 = atStart ? 1 : points.length - 2;
     const step = Math.abs(points[i0].y - anchor.y);
@@ -402,8 +401,9 @@ export async function routeOrthogonal(
                 // anchor's row first, then across to meet the route. Orthogonal,
                 // and the nudging survives.
                 if (opts.sharedPortPaths) {
-                    unkink(points, c.source, opts.idealNudgingDistance);
-                    unkink(points, c.target, opts.idealNudgingDistance);
+                    // libavoid lists the points source to target.
+                    unkink(points, c.source, opts.idealNudgingDistance, true);
+                    unkink(points, c.target, opts.idealNudgingDistance, false);
                 }
                 const withStubs = [c.source, ...bridge(c.source, points[0]), ...points,
                     ...bridge(c.target, points[points.length - 1]).reverse(), c.target];

packages/diagram-server/test/reroute-keeps-the-layouts-trunk.test.ts

@@ -134,6 +134,21 @@ describe('reroute after a move keeps the layout\'s routes', () => {
         }
     });
 
+    it('routes anew an edge whose stub would shrink to nothing', async () => {
+        const g = fanIn();
+        // t stands one pixel right of the trunk: the kept route would leave
+        // its port on a 1 px stub, a vertical to the eye.
+        expect(await moveAndReroute(g, 't', { x: 271, y: 100 }, ['a-t', 'b-t'])).toBe('libavoid');
+        for (const id of ['a-t', 'b-t']) {
+            const route = routeOf(g, id);
+            const [bend, end] = route.slice(-2);
+            expect(end).toEqual({ x: 271, y: 129.5 });
+            expect(bend.y).toBe(end.y);
+            expect(end.x - bend.x).toBeGreaterThanOrEqual(8);
+            expect(isOrthogonalPolyline(route)).toBe(true);
+        }
+    });
+
     it('routes anew a straight edge, which has no bend to carry', async () => {
         const g = graph(node('c', 0, 0), node('d', 400, 0), edge('c-d', 'c', 'd', [{ x: 140, y: 29.5 }, { x: 400, y: 29.5 }]));
         expect(await moveAndReroute(g, 'd', { x: 400, y: 60 }, ['c-d'])).toBe('libavoid');