Keep other nodes out of a group the agent makes round new nodes
A group made in an agent call is fitted round its nodes only after the call has arranged them, and the arrangement knew nothing of it. With On key held, Move and Owner placed fresh, a group round key and move and one round the owner, the Owner landed beside the key. Move is taller than the key, so the Owner stood in the first group and the two groups lay on each other. lint_graph counted none of it. Found while testing copy_nodes (#434). - arrange takes the new option blocks: sets of arranged nodes a group is fitted round afterwards. Once the columns are laid out, whatever would stand in such a group goes below it, a node or the group of another set, with everything arranged under it. - The flow keeps its columns. A first try laid each set out as one block instead, but then the link from the Owner ran over the key inside the group (throughNodes 1). - AgentEdit passes every group made in the call whose nodes are all new. Groups round nodes already there and the Arrange button stay as they were. Tests: - agent-edit: new test with the setup that showed the fault. No node stands in a group not its own, the groups lie apart, lint_graph has 0 overlaps, throughNodes, backwards and coveredNotes. - Counter-check: without the new step exactly that test is red. - Agent, graph-arrange and graph-follow tests 120/120. - e2e 71 on atx: ALL PASSED, 26 checks. - Gate on atx: CHECK EXIT 0, 327 files, 5025 tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
fb981b5b0f
commit
e4ccf5f72f
5 files changed
+118
-7
No files matched your search
@@ -96,8 +96,13 @@ export class AgentEdit implements Command {
|
||||
}
|
||||
}
|
||||
// A group made here holds its nodes, which arranging sees through; a comment sits above them.
|
||||
// Round new nodes alone, it gets room: what else would stand in it goes below it.
|
||||
for (const [assetId, nodes] of unplaced) {
|
||||
const arranged = arrangeCommand(project, assetId, nodes, this.measure(assetId), true);
|
||||
const blocks = frames
|
||||
.filter((frame) => frame.asset === assetId && frame.kind === 'group')
|
||||
.map((frame) => frame.nodes)
|
||||
.filter((held) => held.length > 0 && held.every((node) => nodes.includes(node)));
|
||||
const arranged = arrangeCommand(project, assetId, nodes, this.measure(assetId), true, blocks);
|
||||
if (arranged) run(arranged);
|
||||
}
|
||||
for (const frame of frames) {
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { Box } from '../framing';
|
||||
import { ANNOTATION_TITLE_HEIGHT, GROUP_MARGIN } from './annotationDrag';
|
||||
import { curvePoint } from './beads';
|
||||
|
||||
/**
|
||||
@@ -44,6 +45,11 @@ export interface ArrangeOptions {
|
||||
open?: ReadonlySet<string>;
|
||||
/** The nodes are new and stand nowhere yet: next to what they link to is tried before where they are. */
|
||||
fresh?: boolean;
|
||||
/**
|
||||
* Arranged nodes a group is to be fitted round afterwards: what else would stand in that group goes
|
||||
* below it, so that it takes in no node not its own and lies on no other such group.
|
||||
*/
|
||||
blocks?: readonly (readonly string[])[];
|
||||
}
|
||||
|
||||
export const COLUMN_GAP = 72;
|
||||
@@ -86,10 +92,10 @@ export function arrange(
|
||||
options: ArrangeOptions = {},
|
||||
): Map<string, { x: number; y: number }> {
|
||||
const moving = graph.nodes.filter((node) => !movable || movable.has(node.id));
|
||||
const placed = new Map<string, { x: number; y: number }>();
|
||||
if (moving.length === 0) return placed;
|
||||
if (moving.length === 0) return new Map();
|
||||
const ids = new Set(moving.map((node) => node.id));
|
||||
const edges = graph.edges.filter((edge) => ids.has(edge.from.node) && ids.has(edge.to.node));
|
||||
const placed = new Map<string, { x: number; y: number }>();
|
||||
let top = 0;
|
||||
for (const part of parts(moving, edges)) {
|
||||
const partIds = new Set(part.map((node) => node.id));
|
||||
@@ -99,9 +105,71 @@ export function arrange(
|
||||
for (const [id, spot] of spots) placed.set(id, { x: spot.x, y: spot.y + top });
|
||||
top += height + PART_GAP;
|
||||
}
|
||||
clearBlocks(moving, placed, options.blocks ?? []);
|
||||
return settle(graph, moving, placed, options);
|
||||
}
|
||||
|
||||
/** How far a group reaches past its nodes: its margin, and its title bar on top. */
|
||||
const BLOCK_SIDE = GROUP_MARGIN;
|
||||
const BLOCK_TOP = GROUP_MARGIN + ANNOTATION_TITLE_HEIGHT;
|
||||
|
||||
/**
|
||||
* Makes room for the groups of `blocks`: what would stand in one – a node, or the group of another
|
||||
* block – goes below it, together with everything arranged under it, until nothing does. The flow
|
||||
* keeps its columns; sets that share a node with an earlier one are left out.
|
||||
*/
|
||||
function clearBlocks(
|
||||
nodes: readonly LayoutNode[],
|
||||
placed: Map<string, { x: number; y: number }>,
|
||||
blocks: readonly (readonly string[])[],
|
||||
): void {
|
||||
const sets: string[][] = [];
|
||||
for (const set of blocks) {
|
||||
const present = set.filter((id) => placed.has(id));
|
||||
if (present.length > 0 && !present.some((id) => sets.some((other) => other.includes(id))))
|
||||
sets.push(present);
|
||||
}
|
||||
if (sets.length === 0) return;
|
||||
const units = [
|
||||
...sets,
|
||||
...nodes.filter((node) => !sets.some((set) => set.includes(node.id))).map((node) => [node.id]),
|
||||
];
|
||||
const byId = new Map(nodes.map((node) => [node.id, node]));
|
||||
const frameOf = (unit: readonly string[]) => {
|
||||
const boxes = unit.map((id) => ({ ...byId.get(id)!, ...placed.get(id)! }));
|
||||
const minX = Math.min(...boxes.map((box) => box.x));
|
||||
const minY = Math.min(...boxes.map((box) => box.y));
|
||||
const maxX = Math.max(...boxes.map((box) => box.x + box.w));
|
||||
const maxY = Math.max(...boxes.map((box) => box.y + box.h));
|
||||
const framed = sets.includes(unit as string[]);
|
||||
const side = framed ? BLOCK_SIDE : 0;
|
||||
return {
|
||||
x: minX - side,
|
||||
y: minY - (framed ? BLOCK_TOP : 0),
|
||||
w: maxX - minX + 2 * side,
|
||||
h: maxY - minY + (framed ? BLOCK_TOP + GROUP_MARGIN : 0),
|
||||
};
|
||||
};
|
||||
for (let round = 0; round < units.length * sets.length; round++) {
|
||||
const frames = new Map(units.map((unit) => [unit, frameOf(unit)]));
|
||||
const crowded = sets
|
||||
.map((set) => ({ set, frame: frames.get(set)! }))
|
||||
.sort((a, b) => a.frame.y - b.frame.y)
|
||||
.find(({ set, frame }) =>
|
||||
units.some((unit) => unit !== set && overlap(grown(frames.get(unit)!, ROW_GAP), frame)),
|
||||
);
|
||||
if (!crowded) return;
|
||||
const { set, frame } = crowded;
|
||||
const tops = units
|
||||
.filter((unit) => unit !== set && overlap(grown(frames.get(unit)!, ROW_GAP), frame))
|
||||
.map((unit) => frames.get(unit)!.y);
|
||||
const from = Math.min(...tops);
|
||||
const by = frame.y + frame.h + ROW_GAP - from;
|
||||
for (const unit of units)
|
||||
if (unit !== set && frames.get(unit)!.y >= from) for (const id of unit) placed.get(id)!.y += by;
|
||||
}
|
||||
}
|
||||
|
||||
/** Groups of nodes that links join, in the order their first node has in the graph. */
|
||||
function parts(nodes: readonly LayoutNode[], edges: readonly LayoutEdge[]): LayoutNode[][] {
|
||||
const root = new Map(nodes.map((node) => [node.id, node.id]));
|
||||
|
||||
@@ -55,7 +55,8 @@ export function layoutOf(
|
||||
/**
|
||||
* The moves that arrange these nodes of a graph (all of them without a list), with the groups and
|
||||
* comments going along, as one undo step; null when nothing has to move. `fresh` nodes are new ones
|
||||
* the agent gave no place: put next to what they link to, they join its group.
|
||||
* the agent gave no place: put next to what they link to, they join its group. `blocks` are sets of
|
||||
* them a new group is fitted round afterwards: what else would stand in it goes below it.
|
||||
*/
|
||||
export function arrangeCommand(
|
||||
project: Project,
|
||||
@@ -63,12 +64,13 @@ export function arrangeCommand(
|
||||
nodeIds: readonly string[] | null,
|
||||
measure?: MeasureNode,
|
||||
fresh = false,
|
||||
blocks: readonly (readonly string[])[] = [],
|
||||
): Command | null {
|
||||
const layout = layoutOf(project, assetId, measure);
|
||||
if (!layout) return null;
|
||||
const movable = nodeIds ? new Set(nodeIds) : undefined;
|
||||
const joining = fresh && movable ? adoptingGroups(layout, movable) : [];
|
||||
const spots = arrange(layout, movable, { open: new Set(joining), fresh });
|
||||
const spots = arrange(layout, movable, { open: new Set(joining), fresh, blocks });
|
||||
const boxes = followedBoxes(layout, spots, { groups: joining, nodes: nodeIds ?? [] });
|
||||
const moves = layout.nodes.flatMap((node) => {
|
||||
const spot = spots.get(node.id);
|
||||
|
||||
@@ -8,8 +8,9 @@ import { hostFor, text } from './agentTestHost';
|
||||
/** Copying nodes through the agent's tools (#434). */
|
||||
|
||||
/**
|
||||
* Behavior Walk, placed by hand so no group takes in a node not its own: a key held moving its owner, a Variables node reading `speed` (kept), a group round
|
||||
* key and move, a comment above the Variables node, and a group round the owner alone.
|
||||
* Behavior Walk, placed by hand: a key held moving its owner, a Variables node reading `speed`
|
||||
* (kept), a group round key and move, a comment above the Variables node, and a group round the
|
||||
* owner alone.
|
||||
*/
|
||||
async function walkAndRun(fixture: SceneFixture, host: ToolHost) {
|
||||
const steps = [
|
||||
|
||||
@@ -241,6 +241,41 @@ describe('run_commands', () => {
|
||||
expect(behavior.graph.groups![0]).toEqual(before);
|
||||
});
|
||||
|
||||
it('gives a group made round new nodes room, what else would stand in it going below', async () => {
|
||||
const fixture = makeSpriteScene('Hero');
|
||||
const { host } = hostFor(fixture);
|
||||
const steps = [
|
||||
...walkLeft(fixture),
|
||||
{ command: 'add_group', args: { asset: '$walk', nodes: ['$key', '$move'], title: 'Walk' } },
|
||||
{ command: 'add_group', args: { asset: '$walk', nodes: ['$owner'], title: 'Who' } },
|
||||
];
|
||||
const result = await runTool('run_commands', { steps, label: 'walk' }, host);
|
||||
expect(result.isError, text(result)).toBeFalsy();
|
||||
|
||||
const behavior = fixture.project.assets.find((asset) => asset.name === 'Walk')!;
|
||||
const { graph } = findGraphAsset(fixture.project, behavior.id)!;
|
||||
const [walk, who] = graph.groups!;
|
||||
const centred = (box: { x: number; y: number; w: number; h: number }) =>
|
||||
graph.nodes
|
||||
.filter((node) => {
|
||||
const cx = node.x + NODE_WIDTH / 2;
|
||||
const cy = node.y + 20;
|
||||
return cx >= box.x && cx <= box.x + box.w && cy >= box.y && cy <= box.y + box.h;
|
||||
})
|
||||
.map((node) => node.type);
|
||||
// Move is taller than the key: before, the Owner beside the key came to stand in the Walk group.
|
||||
expect(centred(walk!)).toEqual(['OnKeyHeld', 'Move']);
|
||||
expect(centred(who!)).toEqual(['Owner']);
|
||||
const apart =
|
||||
who!.x + who!.w <= walk!.x ||
|
||||
walk!.x + walk!.w <= who!.x ||
|
||||
who!.y + who!.h <= walk!.y ||
|
||||
walk!.y + walk!.h <= who!.y;
|
||||
expect(apart).toBe(true);
|
||||
const lint = JSON.parse(text(await runTool('lint_graph', { asset: behavior.id }, host))) as GraphLint;
|
||||
expect(lint).toMatchObject({ overlaps: 0, throughNodes: 0, backwards: 0, coveredNotes: 0 });
|
||||
});
|
||||
|
||||
it('changes nothing when a step fails, and names the step and why', async () => {
|
||||
const fixture = makeSpriteScene();
|
||||
const { host } = hostFor(fixture);
|
||||
|
||||
Reference in new issue
Block a user