An agent that built a graph through MCP left every new node in one row. In the first real test that put nine nodes on a line 2080 px long, with the Owner's links running straight through all the others. Placing nodes by hand cost nine move_node calls and guessed heights. The agent now gives no coordinates, and the editor lays the nodes out. - arrange.ts is a pure function. It puts the flow into columns from left to right and sets an Owner or Get right before the first node it feeds. It orders each column for the fewest crossings and lines nodes up by their linked ports without overlaps. - Node sizes come from the drawn NodeView in the browser and from an estimate in unit tests. Both share the constants in nodeBox.ts. - run_commands arranges the nodes that came without x and y when the call ends, inside the same undo step. Nodes that were there stay. - New command arrange_graph and new tool lint_graph. The lint counts overlaps, crossings, links through foreign nodes, backward links and the longest links, with example ids. - The relay instructions tell the agent not to give coordinates and to fetch far values with a second Get or Owner node. - An Arrange button next to the snap magnet in all eight graph tabs. It arranges the selected nodes, or all of them, as one undo step. Found while testing on a real project: - lint_graph missed a crossing where two links swap rows exactly mirrored. Both curves then share a sample point in the middle, and only a crossing between two sample points was counted. Links that meet on a point count now (segmentsMeet). - The Arrow-key graph with one Owner for four Moves cannot get below 8 crossings. The Owner feeds object in row 2 of Move, between exec and dt of the key, so it always crosses one of them. Tests: - graph-arrange, agent-edit, snap-toggle: 33/33. The swapped-rows test was red before the fix (0 instead of 1). - Four counter-checks that bend arrange or the edit plan, all red. - e2e 71 on melektaus-atx: 18/18. Lint after the build shows 0 overlaps, 0 links through nodes, 0 backward. Arrange takes a tangled graph from 3 backward links and 2 crossings to 0, and one Ctrl+Z restores it. - Gate on melektaus-atx: CHECK EXIT 0, 318 files, 4917 tests, measured before the lint fix. - Live through MCP on the user's project: lint_graph reports 8 crossings after Arrange, matching the screenshot. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
276 lines
11 KiB
TypeScript
276 lines
11 KiB
TypeScript
/**
|
||
* How the editor arranges a graph (#429): columns left to right, no overlaps, few crossings, what
|
||
* stays standing where it stood – and the counts the agent reads to see whether a graph reads well.
|
||
*/
|
||
import { describe, expect, it } from 'vitest';
|
||
import { AddNodeCommand, ConnectCommand } from '../../src/core/commands/graphCommands';
|
||
import {
|
||
arrange,
|
||
COLUMN_GAP,
|
||
type LayoutEdge,
|
||
type LayoutGraph,
|
||
type LayoutNode,
|
||
lintLayout,
|
||
} from '../../src/editor/graph/arrange';
|
||
import { layoutOf } from '../../src/editor/graph/graphLayout';
|
||
import { makeBehavior, makeProject } from '../helpers';
|
||
|
||
/** A node of `rows` port rows, `in`/`out` port keys from the top, 20 units a row under a 30-unit header. */
|
||
function node(id: string, inputs: string[], outputs: string[], x = 0, y = 0): LayoutNode {
|
||
const rows = (keys: string[]) => Object.fromEntries(keys.map((key, i) => [key, 30 + i * 20 + 10]));
|
||
return {
|
||
id,
|
||
x,
|
||
y,
|
||
w: 176,
|
||
h: 36 + Math.max(inputs.length, outputs.length) * 20,
|
||
inputs: rows(inputs),
|
||
outputs: rows(outputs),
|
||
};
|
||
}
|
||
|
||
let edgeCount = 0;
|
||
function edge(
|
||
from: string,
|
||
fromPort: string,
|
||
to: string,
|
||
toPort: string,
|
||
exec = fromPort === 'exec',
|
||
): LayoutEdge {
|
||
edgeCount += 1;
|
||
return { id: `e${edgeCount}`, from: { node: from, port: fromPort }, to: { node: to, port: toPort }, exec };
|
||
}
|
||
|
||
function placed(graph: LayoutGraph, movable?: ReadonlySet<string>): LayoutGraph {
|
||
const spots = arrange(graph, movable);
|
||
return { ...graph, nodes: graph.nodes.map((n) => ({ ...n, ...(spots.get(n.id) ?? {}) })) };
|
||
}
|
||
|
||
/**
|
||
* What the agent built for "walk with the arrow keys": four keys, four moves, in one row as the
|
||
* agent left them – with one owner for all of them, or one owner next to each move.
|
||
*/
|
||
function arrowKeys(owners: 'one' | 'each' = 'one'): LayoutGraph {
|
||
const keys = ['left', 'right', 'up', 'down'];
|
||
const owner = (k: string) => (owners === 'one' ? 'owner' : `owner-${k}`);
|
||
return {
|
||
nodes: [
|
||
...keys.map((k, i) => node(`key-${k}`, [], ['exec', 'dt'], i * 260)),
|
||
...keys.map((k, i) => node(`move-${k}`, ['exec', 'object', 'dx', 'dy', 'dt'], ['exec'], (i + 4) * 260)),
|
||
...[...new Set(keys.map(owner))].map((id, i) => node(id, [], ['object'], (i + 8) * 260)),
|
||
],
|
||
edges: keys.flatMap((k) => [
|
||
edge(`key-${k}`, 'exec', `move-${k}`, 'exec'),
|
||
edge(`key-${k}`, 'dt', `move-${k}`, 'dt'),
|
||
edge(owner(k), 'object', `move-${k}`, 'object'),
|
||
]),
|
||
boxes: [],
|
||
};
|
||
}
|
||
|
||
describe('arrange', () => {
|
||
it('lines a chain up left to right, the flow running straight', () => {
|
||
const graph = placed({
|
||
nodes: [
|
||
node('c', ['exec'], ['exec'], 0, 300),
|
||
node('a', [], ['exec'], 500, 0),
|
||
node('b', ['exec'], ['exec'], 90, 90),
|
||
],
|
||
edges: [edge('a', 'exec', 'b', 'exec'), edge('b', 'exec', 'c', 'exec')],
|
||
boxes: [],
|
||
});
|
||
const at = Object.fromEntries(graph.nodes.map((n) => [n.id, n]));
|
||
expect(at.b!.x).toBe(at.a!.x + 176 + COLUMN_GAP);
|
||
expect(at.c!.x).toBe(at.b!.x + 176 + COLUMN_GAP);
|
||
// The exec ports sit level: a = 0 rows of inputs, b and c one, so the boxes line up as well.
|
||
expect(at.a!.y + at.a!.outputs.exec!).toBe(at.b!.y + at.b!.inputs.exec!);
|
||
expect(at.b!.y + at.b!.outputs.exec!).toBe(at.c!.y + at.c!.inputs.exec!);
|
||
// The whole keeps the top-left corner the nodes had.
|
||
expect(Math.min(...graph.nodes.map((n) => n.x))).toBe(0);
|
||
expect(Math.min(...graph.nodes.map((n) => n.y))).toBe(0);
|
||
});
|
||
|
||
it('turns the agent’s row of nine into a graph without overlaps, links over nodes or backward links', () => {
|
||
const row = arrowKeys();
|
||
const before = lintLayout(row);
|
||
const after = lintLayout(placed(row));
|
||
expect(before.throughNodes).toBeGreaterThan(10);
|
||
expect(after).toMatchObject({ overlaps: 0, throughNodes: 0, backwards: 0 });
|
||
// The owner's link crosses one link of each move's own key, and the links of the key in between
|
||
// on its way to the top and the bottom move: 4 + 2 + 2.
|
||
expect(after.crossings).toBeLessThanOrEqual(8);
|
||
expect(after.longest[0]!.length).toBeLessThan(before.longest[0]!.length / 3);
|
||
});
|
||
|
||
it('crosses less and keeps the links short when each move has an owner of its own', () => {
|
||
const lint = lintLayout(placed(arrowKeys('each')));
|
||
expect(lint).toMatchObject({ overlaps: 0, throughNodes: 0, backwards: 0 });
|
||
// The owner feeds the second row of a move, between the key's flow and its dt: one crossing each.
|
||
expect(lint.crossings).toBe(4);
|
||
expect(lint.longest[0]!.length).toBeLessThan(150);
|
||
});
|
||
|
||
it('runs the flow straight from a key into its move', () => {
|
||
const graph = placed(arrowKeys('each'));
|
||
const at = Object.fromEntries(graph.nodes.map((n) => [n.id, n]));
|
||
for (const k of ['left', 'right', 'up', 'down']) {
|
||
const key = at[`key-${k}`]!;
|
||
const move = at[`move-${k}`]!;
|
||
expect(Math.abs(key.y + key.outputs.exec! - (move.y + move.inputs.exec!))).toBeLessThanOrEqual(12);
|
||
}
|
||
});
|
||
|
||
it('puts the owner just left of the moves it feeds, among the keys, not on top of the column', () => {
|
||
const graph = placed(arrowKeys());
|
||
const owner = graph.nodes.find((n) => n.id === 'owner')!;
|
||
const keys = graph.nodes.filter((n) => n.id.startsWith('key-'));
|
||
const moves = graph.nodes.filter((n) => n.id.startsWith('move-'));
|
||
expect(owner.x).toBe(keys[0]!.x);
|
||
expect(moves.every((m) => m.x > owner.x)).toBe(true);
|
||
const above = keys.filter((k) => k.y < owner.y).length;
|
||
expect(above).toBeGreaterThan(0);
|
||
expect(above).toBeLessThan(4);
|
||
});
|
||
|
||
it('leaves the nodes that are not to move, and keeps clear of them and of groups', () => {
|
||
const graph: LayoutGraph = {
|
||
nodes: [
|
||
node('kept', [], ['exec'], 0, 0),
|
||
node('new-a', ['exec'], ['exec'], 0, 10),
|
||
node('new-b', ['exec'], [], 30, 20),
|
||
],
|
||
edges: [edge('kept', 'exec', 'new-a', 'exec'), edge('new-a', 'exec', 'new-b', 'exec')],
|
||
boxes: [{ id: 'group', x: 200, y: -40, w: 300, h: 200 }],
|
||
};
|
||
const result = placed(graph, new Set(['new-a', 'new-b']));
|
||
const kept = result.nodes.find((n) => n.id === 'kept')!;
|
||
expect([kept.x, kept.y]).toEqual([0, 0]);
|
||
expect(lintLayout(result).overlaps).toBe(0);
|
||
const box = graph.boxes[0]!;
|
||
for (const n of result.nodes.filter((n) => n.id !== 'kept')) {
|
||
const clear = n.x >= box.x + box.w || n.x + n.w <= box.x || n.y >= box.y + box.h || n.y + n.h <= box.y;
|
||
expect(clear).toBe(true);
|
||
}
|
||
// Right of everything, level with the node they hang on.
|
||
expect(Math.min(...result.nodes.filter((n) => n.id !== 'kept').map((n) => n.x))).toBeGreaterThan(
|
||
box.x + box.w,
|
||
);
|
||
expect(Math.min(...result.nodes.filter((n) => n.id !== 'kept').map((n) => n.y))).toBe(0);
|
||
});
|
||
|
||
it('arranges nodes inside the group that holds them where they are, rather than outside it', () => {
|
||
const graph: LayoutGraph = {
|
||
nodes: [node('a', [], ['exec'], 220, 0), node('b', ['exec'], [], 30, 20)],
|
||
edges: [edge('a', 'exec', 'b', 'exec')],
|
||
boxes: [{ id: 'group', x: 0, y: -40, w: 700, h: 300 }],
|
||
};
|
||
const result = placed(graph);
|
||
expect(Math.min(...result.nodes.map((n) => n.x))).toBe(30);
|
||
});
|
||
|
||
it('survives a loop and a link to itself, and stacks parts no link joins', () => {
|
||
const graph = placed({
|
||
nodes: [
|
||
node('a', ['exec'], ['exec']),
|
||
node('b', ['exec'], ['exec']),
|
||
node('c', ['exec'], ['exec']),
|
||
node('lone', [], ['exec']),
|
||
node('other', ['exec'], []),
|
||
],
|
||
edges: [
|
||
edge('a', 'exec', 'b', 'exec'),
|
||
edge('b', 'exec', 'c', 'exec'),
|
||
edge('c', 'exec', 'a', 'exec'),
|
||
edge('b', 'exec', 'b', 'exec'),
|
||
edge('lone', 'exec', 'other', 'exec'),
|
||
],
|
||
boxes: [],
|
||
});
|
||
expect(lintLayout(graph).overlaps).toBe(0);
|
||
const xs = new Set(['a', 'b', 'c'].map((id) => graph.nodes.find((n) => n.id === id)!.x));
|
||
expect(xs.size).toBe(3);
|
||
});
|
||
|
||
it('gives the same answer every time', () => {
|
||
expect([...arrange(arrowKeys())]).toEqual([...arrange(arrowKeys())]);
|
||
});
|
||
});
|
||
|
||
describe('lintLayout', () => {
|
||
it('counts overlaps, crossings, links over nodes and links running backwards', () => {
|
||
const graph: LayoutGraph = {
|
||
nodes: [
|
||
node('a', [], ['exec', 'v'], 0, 0),
|
||
node('b', ['exec', 'v'], [], 400, 0),
|
||
node('mid', [], [], 200, 30),
|
||
node('on-mid', [], [], 210, 40),
|
||
node('back', ['exec'], [], -400, 200),
|
||
],
|
||
edges: [
|
||
edge('a', 'exec', 'b', 'v'),
|
||
edge('a', 'v', 'b', 'exec', false),
|
||
edge('a', 'exec', 'back', 'exec'),
|
||
],
|
||
boxes: [],
|
||
};
|
||
const lint = lintLayout(graph);
|
||
expect(lint.overlaps).toBe(1);
|
||
// The two links from a to b swap rows, and the one to `back` loops across the lower of them;
|
||
// it shares its start port with the upper one, which does not count.
|
||
expect(lint.crossings).toBe(2);
|
||
expect(lint.throughNodes).toBeGreaterThanOrEqual(2);
|
||
expect(lint.backwards).toBe(1);
|
||
expect(lint.examples.find((e) => e.kind === 'overlap')?.ids).toEqual(['mid', 'on-mid']);
|
||
});
|
||
|
||
it('counts two links that swap rows in a perfect mirror, meeting on a point of both curves', () => {
|
||
// A key and an owner side by side feeding one move, as the agent's arrange left them live:
|
||
// dt runs down from the key's second row, the owner's object up into the move's second row.
|
||
const graph: LayoutGraph = {
|
||
nodes: [
|
||
node('key', [], ['exec', 'dt'], 0, 0),
|
||
node('owner', [], ['object'], 0, 80),
|
||
node('move', ['exec', 'object', 'dx', 'dy', 'dt'], [], 248, 0),
|
||
],
|
||
edges: [
|
||
edge('key', 'exec', 'move', 'exec'),
|
||
edge('key', 'dt', 'move', 'dt'),
|
||
edge('owner', 'object', 'move', 'object'),
|
||
],
|
||
boxes: [],
|
||
};
|
||
const at = Object.fromEntries(graph.nodes.map((n) => [n.id, n]));
|
||
// The mirror: each link starts where the other ends, row for row.
|
||
expect(at.key!.outputs.dt).toBe(at.move!.inputs.object);
|
||
expect(at.owner!.y + at.owner!.outputs.object!).toBe(at.move!.inputs.dt);
|
||
expect(lintLayout(graph).crossings).toBe(1);
|
||
});
|
||
});
|
||
|
||
describe('layoutOf', () => {
|
||
it('reads a behavior’s nodes with their port rows, and which links carry the flow', () => {
|
||
const fixture = makeProject();
|
||
const behavior = makeBehavior(fixture);
|
||
const key = new AddNodeCommand(behavior.id, 'OnKeyHeld', 0, 0, {});
|
||
const move = new AddNodeCommand(behavior.id, 'Move', 300, 0, {});
|
||
fixture.stack.execute(key);
|
||
fixture.stack.execute(move);
|
||
fixture.stack.execute(
|
||
new ConnectCommand(
|
||
behavior.id,
|
||
{ node: key.node.id, port: 'exec' },
|
||
{ node: move.node.id, port: 'exec' },
|
||
),
|
||
);
|
||
fixture.stack.execute(
|
||
new ConnectCommand(behavior.id, { node: key.node.id, port: 'dt' }, { node: move.node.id, port: 'dt' }),
|
||
);
|
||
const layout = layoutOf(fixture.project, behavior.id)!;
|
||
expect(layout.edges.map((e) => e.exec)).toEqual([true, false]);
|
||
const [keyNode, moveNode] = layout.nodes;
|
||
expect(keyNode!.outputs.dt! - keyNode!.outputs.exec!).toBe(20);
|
||
expect(moveNode!.h).toBeGreaterThan(keyNode!.h);
|
||
expect(layoutOf(fixture.project, 'nope')).toBeNull();
|
||
});
|
||
});
|