From 6f18a71fa080902ae7b3edb3c600dfe302cf9c4c Mon Sep 17 00:00:00 2001 From: Endri Bezati Date: Fri, 6 Nov 2026 09:17:24 +0100 Subject: [PATCH] fix: the selection trace reads the diagram left to right, so a handshake's ready wires do not cut it short Selecting a node in a netlist lit only its two neighbours: every `ready` wire runs against the data it answers, so by the arrows alone every two connected nodes form a cycle, and on a cycle the trace was cut to the direct neighbours. Now the walk follows each wire the way the diagram is read: a wire drawn back to the left (a ready, a loop's feedback) is walked against its arrow, so a laid-out diagram is a DAG and the trace follows the data as far as it goes, upstream to the left and downstream to the right. A port reached that way is walked on from too, since it has wires of its own. A model with no positions, and a cycle that remains, behave as before. On the mul_mul module of a 1597-module netlist, selecting addi73 traces 141 elements upstream where it traced four. Claude-Session: https://claude.ai/code/session_01EToj6RJRMSSHQiqGNMU9wv --- CHANGELOG.md | 8 ++- .../diagram-client/src/selection-trace.ts | 66 ++++++++++++++----- .../test/selection-trace.test.ts | 59 ++++++++++++++++- 3 files changed, 114 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4df83bd..dbe3c0e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,8 +12,12 @@ platform API version. node draws every path through it, what leads to it in green and what it leads to in blue, and fades the rest; after mlir-viewer's trace highlighter. A path crosses group frames through their boundary ports, at any depth, every - piece of a cut edge drawn. On a cycle the trace is cut to the node's direct - neighbours, since reachability would light the whole diagram. A selected + 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 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. diff --git a/packages/diagram-client/src/selection-trace.ts b/packages/diagram-client/src/selection-trace.ts index 3542ae1..ac6d317 100644 --- a/packages/diagram-client/src/selection-trace.ts +++ b/packages/diagram-client/src/selection-trace.ts @@ -16,9 +16,16 @@ * * Cycles. Reachability answers "what does this touch" where the graph is a * DAG. On a cycle it answers "everything": every successor of a state in a - * state machine leads back to it, the whole diagram lights up, and the trace - * reads as broken. So when the walk comes back to the node it started from, the - * trace is cut to the direct neighbours: the edges into the node and out of it. + * state machine leads back to it, and in a handshake netlist every `ready` + * wire runs against the data it answers, so every two neighbours form a + * cycle and nothing past them is a path. So the walk reads the diagram the + * way its reader does, left to right: a wire drawn back to the left (a + * `ready`, a loop's feedback) is walked against its arrow, as if it ran from + * the node it points at to the node it leaves. A laid-out diagram is then a + * DAG, and upstream is what sits to the left on the path, downstream what + * sits to the right. Where a cycle remains all the same (a model with no + * layout yet), the trace is cut to the direct neighbours: the edges into the + * node and out of it. * * A selected edge draws itself, every piece of it, and its two ends. A selected * container draws the nodes that feed it and the nodes it feeds, across its @@ -43,6 +50,9 @@ export interface TraceElement { sourceId?: string; targetId?: string; cssClasses?: string[]; + /** Where the element is drawn, relative to the element that holds it. */ + position?: { x: number; y: number }; + size?: { width: number; height: number }; } /** One step along an edge: the port at its other end, and the edge. */ @@ -54,8 +64,15 @@ export interface TraceGraph { ownerOf: Map; /** A node to its ports, and to itself when an edge ends on it directly. */ portsOf: Map; + /** The edges as the model has them: from source port to target port. */ forward: Map; backward: Map; + /** + * The edges as the diagram reads them, left to right: an edge drawn back + * to the left is turned around. What the walks follow. + */ + flowForward: Map; + flowBackward: Map; edges: Map; /** Ports on a group's frame, walked through without counting the group. */ passThrough: Set; @@ -107,6 +124,8 @@ export function buildTraceGraph(root: TraceElement): TraceGraph { portsOf: new Map(), forward: new Map(), backward: new Map(), + flowForward: new Map(), + flowBackward: new Map(), edges: new Map(), passThrough: new Set(), parentOf: new Map(), @@ -114,9 +133,18 @@ export function buildTraceGraph(root: TraceElement): TraceGraph { }; const groups = new Set(); const edges: TraceElement[] = []; + /** The middle of each node and port across the page, for a model that is laid out. */ + const middle = new Map(); - const visit = (element: TraceElement, node: string | undefined): void => { + const visit = (element: TraceElement, node: string | undefined, originX: number): void => { let owner = node; + let x = originX; + if (element !== root && typeof element.position?.x === 'number') { + x += element.position.x; + if (startsWith(element, 'node') || startsWith(element, 'port')) { + middle.set(element.id, x + (element.size?.width ?? 0) / 2); + } + } if (startsWith(element, 'node')) { graph.parentOf.set(element.id, node); if (node !== undefined) { @@ -141,10 +169,10 @@ export function buildTraceGraph(root: TraceElement): TraceGraph { edges.push(element); } for (const child of element.children ?? []) { - visit(child, owner); + visit(child, owner, x); } }; - visit(root, undefined); + visit(root, undefined, 0); for (const edge of edges) { const source = edge.sourceId as string; @@ -162,6 +190,12 @@ export function buildTraceGraph(root: TraceElement): TraceGraph { graph.edges.set(edge.id, { source, target }); push(graph.forward, source, { port: target, edge: edge.id }); push(graph.backward, target, { port: source, edge: edge.id }); + // Drawn back to the left: read as running the other way. + const [sourceX, targetX] = [middle.get(source), middle.get(target)]; + const turned = sourceX !== undefined && targetX !== undefined && targetX < sourceX; + const [from, to] = turned ? [target, source] : [source, target]; + push(graph.flowForward, from, { port: to, edge: edge.id }); + push(graph.flowBackward, to, { port: from, edge: edge.id }); } return graph; } @@ -177,8 +211,8 @@ interface Walk { } /** - * Walk from the ports of `start` along `adjacency` (forward: downstream, - * backward: upstream). A node reached is recorded and walked on from; a + * Walk from the ports of `start` along `adjacency` (flowForward: downstream, + * flowBackward: upstream). A node reached is recorded and walked on from; a * boundary port is passed through, unrecorded and without costing a hop, so * one hop still reaches the neighbour on the other side of two frames. Nodes in * `blocked` are not entered. @@ -232,8 +266,10 @@ function walk( } else if (owner !== undefined) { nodes.add(owner); if (hops + 1 < maxHops) { + // The port reached is walked on from too: a wire read + // against its arrow lands on a port that has wires of its own. for (const own of graph.portsOf.get(owner) ?? []) { - if (!visited.has(own)) { + if (own === next.port || !visited.has(own)) { visited.add(own); queue.push({ port: own, hops: hops + 1 }); } @@ -261,11 +297,11 @@ function union(...sets: ReadonlyArray>): Set { /** A node's path: every node that leads to it and every node it leads to. */ function traceNode(graph: TraceGraph, node: string): Trace { const start = new Set([node]); - let up = walk(graph, graph.backward, start); - let down = walk(graph, graph.forward, start); + let up = walk(graph, graph.flowBackward, start); + let down = walk(graph, graph.flowForward, start); if (up.cyclic || down.cyclic) { - up = walk(graph, graph.backward, start, 1); - down = walk(graph, graph.forward, start, 1); + up = walk(graph, graph.flowBackward, start, 1); + down = walk(graph, graph.flowForward, start, 1); } return { focus: start, @@ -321,8 +357,8 @@ function traceContainer(graph: TraceGraph, container: string): Trace { } } const start = new Set([container]); - const up = walk(graph, graph.backward, start, 1, held); - const down = walk(graph, graph.forward, start, 1, held); + const up = walk(graph, graph.flowBackward, start, 1, held); + const down = walk(graph, graph.flowForward, start, 1, held); const inside = union(held, start); const scope = new Set(held); for (const [id, { source, target }] of graph.edges) { diff --git a/packages/diagram-client/test/selection-trace.test.ts b/packages/diagram-client/test/selection-trace.test.ts index e73e447..e4631ba 100644 --- a/packages/diagram-client/test/selection-trace.test.ts +++ b/packages/diagram-client/test/selection-trace.test.ts @@ -6,8 +6,11 @@ * * - a node traces every path through it: all it leads from upstream, all it * leads to downstream, the rest faded; - * - on a cycle the walk comes back to the start and would light everything, - * so the trace is cut to the direct neighbours; + * - the walk reads a laid-out diagram left to right: a wire drawn back to + * the left (a handshake's `ready`, a loop's feedback) is walked against + * its arrow, so the trace follows the data past it; + * - on a cycle that remains (no layout) the walk comes back to the start + * and would light everything, so the trace is cut to the direct neighbours; * - nested groups: the models are split by the server's own * `splitEdgesAtGroups`, so the pieces and boundary ports are the ones the * client is sent. A path crosses frames, at any depth, with every piece @@ -69,6 +72,32 @@ function trace(root: El, ...selected: string[]) { const sorted = (set: Iterable) => [...set].sort(); +/** + * A node laid out at `x`, as the server places it: its `in` port on the + * west side, its `out` port on the east, and a handshake pair beside them, + * the output `rdy` on the west (it answers the input) and the input `ack` on + * the east. + */ +function placed(name: string, x: number): El { + return { + id: name, + type: WorkflowDiagramTypes.NODE_ACTOR, + position: { x, y: 0 }, + size: { width: 60, height: 40 }, + children: [ + { id: `${name}.in`, type: WorkflowDiagramTypes.PORT_INPUT, position: { x: -2, y: 10 }, size: { width: 4, height: 4 } }, + { id: `${name}.out`, type: WorkflowDiagramTypes.PORT_OUTPUT, position: { x: 58, y: 10 }, size: { width: 4, height: 4 } }, + { id: `${name}.rdy`, type: WorkflowDiagramTypes.PORT_OUTPUT, position: { x: -2, y: 20 }, size: { width: 4, height: 4 } }, + { id: `${name}.ack`, type: WorkflowDiagramTypes.PORT_INPUT, position: { x: 58, y: 20 }, size: { width: 4, height: 4 } } + ] + }; +} + +/** The `ready` wire answering `edge(to, from)`: drawn from `from`'s west side back to `to`'s east side. */ +function ready(from: string, to: string): El { + return { id: `${from}.rdy->${to}.ack`, type: WorkflowDiagramTypes.EDGE_CONNECTION, sourceId: `${from}.rdy`, targetId: `${to}.ack` }; +} + describe('a selected node', () => { it('traces every path through it, both ways, and fades the rest', () => { const root = model(node('a'), node('b'), node('c'), node('d'), node('x'), @@ -101,6 +130,32 @@ describe('a selected node', () => { expect(sorted(result.downstream)).toEqual(['s1->s2', 's2']); }); + it('walks a wire drawn back to the left against its arrow, so a handshake is no cycle', () => { + // a -> b -> c with the data, each ready answering back to the left: + // by the arrows alone every neighbour pair is a cycle and the trace + // from b would stop at a and c. + const root = model(placed('a', 0), placed('b', 100), placed('c', 200), placed('d', 300), + edge('a', 'b'), edge('b', 'c'), edge('c', 'd'), ready('b', 'a'), ready('c', 'b'), ready('d', 'c')); + const { result } = trace(root, 'b'); + expect(sorted(result.upstream)).toEqual(['a', 'a->b', 'b.rdy->a.ack']); + expect(sorted(result.downstream)).toEqual(['b->c', 'c', 'c->d', 'c.rdy->b.ack', 'd', 'd.rdy->c.ack']); + }); + + 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. + const root = model(placed('s1', 0), placed('s2', 100), placed('s3', 200), placed('s4', 300), + edge('s1', 's2'), edge('s2', 's3'), edge('s3', 's1'), edge('s3', 's4')); + const s1 = trace(root, 's1').result; + expect(sorted(s1.upstream)).toEqual([]); + expect(sorted(s1.downstream)).toEqual(['s1->s2', 's2', 's2->s3', 's3', 's3->s1', 's3->s4', 's4']); + // From inside the loop, the feedback wire joins a node downstream to + // one upstream and is on no path through s2: it stays off the trace. + const s2 = trace(root, 's2').result; + expect(sorted(s2.upstream)).toEqual(['s1', 's1->s2']); + expect(sorted(s2.downstream)).toEqual(['s2->s3', 's3', 's3->s4', 's4']); + }); + it('leaves a virtual edge out of the path', () => { const root = model(node('a'), node('b'), { ...edge('a', 'b'), type: WorkflowDiagramTypes.EDGE_VIRTUAL });