fix: auto-layout zero-position nodes on hydrate, fix new-node x===y bug
- computeAutoLayout() BFS tree layout seeds from anchored nodes; assigns
distinct x/y to workspaces returned at 0,0 by the API and persists via PATCH
- buildNodesAndEdges() accepts layoutOverrides map so hydration uses computed
positions instead of raw 0,0 coordinates
- canvas-events WORKSPACE_PROVISIONING grid layout replaces offset===offset
assignment that caused position:{x:t,y:t} in the minified bundle
- 8 new vitest tests cover computeAutoLayout and override behaviour (365 pass)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
e920aaab8e
commit
1a56c03192
@ -1,5 +1,5 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { buildNodesAndEdges, extractSkillNames } from "../canvas-topology";
|
||||
import { buildNodesAndEdges, extractSkillNames, computeAutoLayout } from "../canvas-topology";
|
||||
import type { WorkspaceData } from "../socket";
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
@ -263,3 +263,97 @@ describe("extractSkillNames – mixed valid/invalid skills", () => {
|
||||
expect(extractSkillNames(card)).toEqual(["Coding", "Testing"]);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// computeAutoLayout
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe("computeAutoLayout – all nodes already positioned", () => {
|
||||
it("returns empty map when all nodes have non-zero positions", () => {
|
||||
const workspaces = [
|
||||
makeWS({ id: "a", x: 100, y: 200 }),
|
||||
makeWS({ id: "b", x: 400, y: 200 }),
|
||||
];
|
||||
const overrides = computeAutoLayout(workspaces);
|
||||
expect(overrides.size).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe("computeAutoLayout – empty workspace list", () => {
|
||||
it("returns empty map", () => {
|
||||
const overrides = computeAutoLayout([]);
|
||||
expect(overrides.size).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe("computeAutoLayout – single zero-position root node", () => {
|
||||
it("assigns a position to the zero node", () => {
|
||||
const workspaces = [makeWS({ id: "ws-1", x: 0, y: 0 })];
|
||||
const overrides = computeAutoLayout(workspaces);
|
||||
expect(overrides.has("ws-1")).toBe(true);
|
||||
const pos = overrides.get("ws-1")!;
|
||||
expect(typeof pos.x).toBe("number");
|
||||
expect(typeof pos.y).toBe("number");
|
||||
});
|
||||
});
|
||||
|
||||
describe("computeAutoLayout – multiple zero-position root nodes", () => {
|
||||
it("spreads siblings horizontally (distinct x values)", () => {
|
||||
const workspaces = [
|
||||
makeWS({ id: "a", x: 0, y: 0 }),
|
||||
makeWS({ id: "b", x: 0, y: 0 }),
|
||||
makeWS({ id: "c", x: 0, y: 0 }),
|
||||
];
|
||||
const overrides = computeAutoLayout(workspaces);
|
||||
const positions = ["a", "b", "c"].map((id) => overrides.get(id)!);
|
||||
const xs = positions.map((p) => p.x);
|
||||
// All x values should be unique (nodes spread horizontally)
|
||||
const uniqueXs = new Set(xs);
|
||||
expect(uniqueXs.size).toBe(3);
|
||||
// All at depth 0 → y should be 0
|
||||
for (const p of positions) {
|
||||
expect(p.y).toBe(0);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("computeAutoLayout – parent with zero-position children", () => {
|
||||
it("places child at greater y than parent", () => {
|
||||
const workspaces = [
|
||||
makeWS({ id: "parent", x: 0, y: 0 }),
|
||||
makeWS({ id: "child", parent_id: "parent", x: 0, y: 0 }),
|
||||
];
|
||||
const overrides = computeAutoLayout(workspaces);
|
||||
const parentPos = overrides.get("parent")!;
|
||||
const childPos = overrides.get("child")!;
|
||||
expect(childPos.y).toBeGreaterThan(parentPos.y);
|
||||
});
|
||||
});
|
||||
|
||||
describe("computeAutoLayout – anchored node not overridden", () => {
|
||||
it("does not include already-positioned node in overrides", () => {
|
||||
const workspaces = [
|
||||
makeWS({ id: "anchored", x: 500, y: 300 }),
|
||||
makeWS({ id: "zero", x: 0, y: 0 }),
|
||||
];
|
||||
const overrides = computeAutoLayout(workspaces);
|
||||
expect(overrides.has("anchored")).toBe(false);
|
||||
expect(overrides.has("zero")).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("buildNodesAndEdges – layoutOverrides applied", () => {
|
||||
it("uses override position instead of ws.x/ws.y for zero-position nodes", () => {
|
||||
const workspaces = [makeWS({ id: "ws-1", x: 0, y: 0 })];
|
||||
const overrides = new Map([["ws-1", { x: 150, y: 250 }]]);
|
||||
const { nodes } = buildNodesAndEdges(workspaces, overrides);
|
||||
expect(nodes[0].position).toEqual({ x: 150, y: 250 });
|
||||
});
|
||||
|
||||
it("leaves non-overridden node at its own position", () => {
|
||||
const workspaces = [makeWS({ id: "ws-2", x: 100, y: 200 })];
|
||||
const overrides = new Map<string, { x: number; y: number }>();
|
||||
const { nodes } = buildNodesAndEdges(workspaces, overrides);
|
||||
expect(nodes[0].position).toEqual({ x: 100, y: 200 });
|
||||
});
|
||||
});
|
||||
|
||||
@ -88,10 +88,13 @@ export function handleCanvasEvent(
|
||||
),
|
||||
});
|
||||
} else {
|
||||
// Spread new nodes so they don't stack at (0,0)
|
||||
const offset = nodes.length * 40;
|
||||
const x = offset;
|
||||
const y = offset;
|
||||
// Spread new nodes in a grid so they don't stack
|
||||
const GRID_COL_WIDTH = 320;
|
||||
const GRID_ROW_HEIGHT = 160;
|
||||
const GRID_COLS = 4;
|
||||
const rootIndex = nodes.filter((n) => !n.data.parentId).length;
|
||||
const x = (rootIndex % GRID_COLS) * GRID_COL_WIDTH;
|
||||
const y = Math.floor(rootIndex / GRID_COLS) * GRID_ROW_HEIGHT;
|
||||
|
||||
set({
|
||||
nodes: [
|
||||
|
||||
@ -2,38 +2,151 @@ import type { Node, Edge } from "@xyflow/react";
|
||||
import type { WorkspaceData } from "./socket";
|
||||
import type { WorkspaceNodeData } from "./canvas";
|
||||
|
||||
const H_SPACING = 320;
|
||||
const V_SPACING = 200;
|
||||
|
||||
/**
|
||||
* Computes auto-layout positions for workspaces that have no persisted position
|
||||
* (x === 0 AND y === 0). Workspaces with an existing non-zero position are used
|
||||
* as anchors and are never moved.
|
||||
*
|
||||
* Returns a Map of workspace IDs → {x, y} for every workspace that was assigned
|
||||
* a computed position (i.e. only the ones that were at 0,0). Callers should
|
||||
* persist these back to the API so the positions survive reload.
|
||||
*/
|
||||
export function computeAutoLayout(
|
||||
workspaces: WorkspaceData[]
|
||||
): Map<string, { x: number; y: number }> {
|
||||
const overrides = new Map<string, { x: number; y: number }>();
|
||||
|
||||
// Separate anchored (already positioned) from zero-position workspaces
|
||||
const anchored = new Set<string>();
|
||||
for (const ws of workspaces) {
|
||||
if (ws.x !== 0 || ws.y !== 0) {
|
||||
anchored.add(ws.id);
|
||||
}
|
||||
}
|
||||
|
||||
// If every workspace is already positioned, nothing to do
|
||||
const needsLayout = workspaces.filter((ws) => !anchored.has(ws.id));
|
||||
if (needsLayout.length === 0) return overrides;
|
||||
|
||||
// Build parent→children map
|
||||
const children = new Map<string | null, WorkspaceData[]>();
|
||||
for (const ws of workspaces) {
|
||||
const pid = ws.parent_id ?? null;
|
||||
if (!children.has(pid)) children.set(pid, []);
|
||||
children.get(pid)!.push(ws);
|
||||
}
|
||||
|
||||
// Sort children by name for deterministic layout
|
||||
for (const list of children.values()) {
|
||||
list.sort((a, b) => a.name.localeCompare(b.name));
|
||||
}
|
||||
|
||||
// Assigned positions (includes anchors from the original data + computed overrides)
|
||||
const assigned = new Map<string, { x: number; y: number }>();
|
||||
for (const ws of workspaces) {
|
||||
if (anchored.has(ws.id)) {
|
||||
assigned.set(ws.id, { x: ws.x, y: ws.y });
|
||||
}
|
||||
}
|
||||
|
||||
// BFS from root nodes that need layout
|
||||
// Track the next X offset per depth row to spread siblings horizontally
|
||||
const rowNextX = new Map<number, number>();
|
||||
|
||||
// Enqueue root-level nodes that need layout
|
||||
const queue: Array<{ ws: WorkspaceData; depth: number }> = [];
|
||||
const rootsNeedingLayout = (children.get(null) ?? []).filter(
|
||||
(ws) => !anchored.has(ws.id)
|
||||
);
|
||||
for (const ws of rootsNeedingLayout) {
|
||||
queue.push({ ws, depth: 0 });
|
||||
}
|
||||
|
||||
while (queue.length > 0) {
|
||||
const { ws, depth } = queue.shift()!;
|
||||
|
||||
// Skip if already assigned (e.g. anchored)
|
||||
if (assigned.has(ws.id)) {
|
||||
// Still enqueue its unpositioned children
|
||||
const kids = (children.get(ws.id) ?? []).filter(
|
||||
(c) => !anchored.has(c.id) && !assigned.has(c.id)
|
||||
);
|
||||
for (const kid of kids) {
|
||||
queue.push({ ws: kid, depth: depth + 1 });
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
// Find parent's x as the center hint for this node
|
||||
const parentPos = ws.parent_id ? assigned.get(ws.parent_id) : undefined;
|
||||
const parentX = parentPos?.x ?? 0;
|
||||
|
||||
// Place node at the next available slot in this row
|
||||
const currentRowX = rowNextX.get(depth) ?? (parentX - H_SPACING / 2);
|
||||
const x = Math.max(currentRowX, parentX);
|
||||
const y = depth * V_SPACING;
|
||||
|
||||
assigned.set(ws.id, { x, y });
|
||||
overrides.set(ws.id, { x, y });
|
||||
rowNextX.set(depth, x + H_SPACING);
|
||||
|
||||
// Enqueue children that need layout
|
||||
const kids = (children.get(ws.id) ?? []).filter(
|
||||
(c) => !anchored.has(c.id) && !assigned.has(c.id)
|
||||
);
|
||||
for (const kid of kids) {
|
||||
queue.push({ ws: kid, depth: depth + 1 });
|
||||
}
|
||||
}
|
||||
|
||||
return overrides;
|
||||
}
|
||||
|
||||
/**
|
||||
* Converts raw workspace data from the API into React Flow nodes and edges.
|
||||
* Accepts an optional layoutOverrides map (from computeAutoLayout) to override
|
||||
* positions for workspaces that were at 0,0.
|
||||
*/
|
||||
export function buildNodesAndEdges(workspaces: WorkspaceData[]): {
|
||||
export function buildNodesAndEdges(
|
||||
workspaces: WorkspaceData[],
|
||||
layoutOverrides: Map<string, { x: number; y: number }> = new Map()
|
||||
): {
|
||||
nodes: Node<WorkspaceNodeData>[];
|
||||
edges: Edge[];
|
||||
} {
|
||||
// All workspaces become nodes (children are rendered inside parent via WorkspaceNode)
|
||||
const nodes: Node<WorkspaceNodeData>[] = workspaces.map((ws) => ({
|
||||
id: ws.id,
|
||||
type: "workspaceNode",
|
||||
position: { x: ws.x, y: ws.y },
|
||||
// Don't set React Flow parentId — children render embedded inside the WorkspaceNode component
|
||||
data: {
|
||||
name: ws.name,
|
||||
status: ws.status,
|
||||
tier: ws.tier,
|
||||
agentCard: ws.agent_card,
|
||||
activeTasks: ws.active_tasks,
|
||||
collapsed: ws.collapsed,
|
||||
role: ws.role,
|
||||
lastErrorRate: ws.last_error_rate,
|
||||
lastSampleError: ws.last_sample_error,
|
||||
url: ws.url,
|
||||
parentId: ws.parent_id,
|
||||
currentTask: ws.current_task || "",
|
||||
runtime: ws.runtime || "",
|
||||
needsRestart: false,
|
||||
},
|
||||
// Hide child nodes from canvas — they render inside the parent WorkspaceNode
|
||||
hidden: !!ws.parent_id,
|
||||
}));
|
||||
const nodes: Node<WorkspaceNodeData>[] = workspaces.map((ws) => {
|
||||
const override = layoutOverrides.get(ws.id);
|
||||
const x = override?.x ?? ws.x;
|
||||
const y = override?.y ?? ws.y;
|
||||
return {
|
||||
id: ws.id,
|
||||
type: "workspaceNode",
|
||||
position: { x, y },
|
||||
// Don't set React Flow parentId — children render embedded inside the WorkspaceNode component
|
||||
data: {
|
||||
name: ws.name,
|
||||
status: ws.status,
|
||||
tier: ws.tier,
|
||||
agentCard: ws.agent_card,
|
||||
activeTasks: ws.active_tasks,
|
||||
collapsed: ws.collapsed,
|
||||
role: ws.role,
|
||||
lastErrorRate: ws.last_error_rate,
|
||||
lastSampleError: ws.last_sample_error,
|
||||
url: ws.url,
|
||||
parentId: ws.parent_id,
|
||||
currentTask: ws.current_task || "",
|
||||
runtime: ws.runtime || "",
|
||||
needsRestart: false,
|
||||
},
|
||||
// Hide child nodes from canvas — they render inside the parent WorkspaceNode
|
||||
hidden: !!ws.parent_id,
|
||||
};
|
||||
});
|
||||
|
||||
// No parent→child edges — children are embedded inside the parent node.
|
||||
// Only create edges between siblings or cross-team connections if needed in future.
|
||||
|
||||
@ -8,7 +8,7 @@ import {
|
||||
import { api } from "@/lib/api";
|
||||
import type { WorkspaceData, WSMessage } from "./socket";
|
||||
import { handleCanvasEvent } from "./canvas-events";
|
||||
import { buildNodesAndEdges } from "./canvas-topology";
|
||||
import { buildNodesAndEdges, computeAutoLayout } from "./canvas-topology";
|
||||
|
||||
// Re-export extracted types and functions so existing imports from "@/store/canvas" keep working
|
||||
export { summarizeWorkspaceCapabilities } from "./canvas-capabilities";
|
||||
@ -209,8 +209,12 @@ export const useCanvasStore = create<CanvasState>((set, get) => ({
|
||||
},
|
||||
|
||||
hydrate: (workspaces: WorkspaceData[]) => {
|
||||
const { nodes, edges } = buildNodesAndEdges(workspaces);
|
||||
const layoutOverrides = computeAutoLayout(workspaces);
|
||||
const { nodes, edges } = buildNodesAndEdges(workspaces, layoutOverrides);
|
||||
set({ nodes, edges });
|
||||
for (const [nodeId, { x, y }] of layoutOverrides) {
|
||||
api.patch(`/workspaces/${nodeId}`, { x, y }).catch(() => {});
|
||||
}
|
||||
},
|
||||
|
||||
applyEvent: (msg: WSMessage) => {
|
||||
|
||||
Loading…
Reference in New Issue
Block a user