diff --git a/app/api/chat/route.ts b/app/api/chat/route.ts index d906a2d..ccea08d 100644 --- a/app/api/chat/route.ts +++ b/app/api/chat/route.ts @@ -786,7 +786,9 @@ Loops are fine — an arrow back to an earlier step is drawn as a loop. So are a Replaces the whole diagram, because one new arrow can change which row several nodes belong in. To edit afterwards, use restructure_diagram with the ids from the outline this returns. -Shapes: "decision" for a branch (diamond), "terminator" for a start or end point, "data" for input or output, "document" for a report, "round" for a soft-edged step, "box" (default) for a plain step. Set icon instead of shape to draw a node as a catalog icon — look the name up with search_stencils first.`, +Shapes: "decision" for a branch (diamond), "terminator" for a start or end point, "data" for input or output, "document" for a report, "round" for a soft-edged step, "box" (default) for a plain step. Set icon instead of shape to draw a node as a catalog icon — look the name up with search_stencils first. + +Grouping: when the nodes fall into natural zones (remote vs local, frontend vs backend, roles, phases), set the same group name on each zone's nodes. The engine colours each group consistently from its own palette. Name groups by meaning; never pick hex colours.`, inputSchema: z.object({ nodes: z .array( @@ -809,6 +811,12 @@ Shapes: "decision" for a branch (diamond), "terminator" for a start or end point .describe( "Catalog stencil name; draws this node as an icon", ), + group: z + .string() + .optional() + .describe( + "Semantic group name, e.g. 'remote' or 'local'. Nodes sharing a group get the same colour from the engine's palette — never pick colours yourself", + ), }), ) .describe("Every box in the diagram"), diff --git a/lib/diagram-engine/graph.ts b/lib/diagram-engine/graph.ts index 250e0b1..bc4beaa 100644 --- a/lib/diagram-engine/graph.ts +++ b/lib/diagram-engine/graph.ts @@ -23,6 +23,7 @@ */ import type { Operation } from "./operations" +import { groupColour } from "./render" import type { BoxShape } from "./types" /** A node in the graph the caller wants drawn. */ @@ -33,6 +34,12 @@ export interface GraphNode { shape?: BoxShape /** Catalog stencil name. When set the node renders as an icon rather than a box. */ icon?: string + /** + * Semantic group name, e.g. "remote" or "local". Nodes sharing a group get the same + * fill colour from the engine's palette, assigned in order of first appearance — the + * caller names the grouping and never touches a colour. + */ + group?: string } /** An arrow. Direction matters: it is what determines the layering. */ @@ -302,8 +309,19 @@ export function graphToOperations( }, ] const byId = new Map(nodes.map((n) => [n.id, n])) + // Groups become colours here, in order of first appearance, so "the second group named + // is green" holds for every diagram the engine draws. The caller only names groups. + const groupIndex = new Map() + for (const n of nodes) + if (n.group && !groupIndex.has(n.group)) + groupIndex.set(n.group, groupIndex.size) + const add = (id: string, parent: string): Operation => { const n = byId.get(id) as GraphNode + const colour = + n.group !== undefined + ? groupColour(groupIndex.get(n.group) ?? 0) + : null return n.icon ? { op: "add_icon", @@ -318,6 +336,9 @@ export function graphToOperations( parent, label: n.label, ...(n.shape && n.shape !== "box" ? { shape: n.shape } : {}), + ...(colour + ? { fill: colour.fill, stroke: colour.stroke } + : {}), } } diff --git a/lib/diagram-engine/operations.ts b/lib/diagram-engine/operations.ts index eed8c41..2533eab 100644 --- a/lib/diagram-engine/operations.ts +++ b/lib/diagram-engine/operations.ts @@ -54,6 +54,16 @@ export const OperationSchema = z.discriminatedUnion("op", [ id: z.string(), parent: z.string().optional(), label: z.string(), + fill: z + .string() + .optional() + .describe( + "Fill colour, e.g. #DAE8FC. Prefer draw_graph's group field over picking colours", + ), + stroke: z + .string() + .optional() + .describe("Border colour; pair it with fill"), shape: z .enum([ "box", @@ -358,6 +368,8 @@ export function applyOperations( ...(op.shape && op.shape !== "box" ? { shape: op.shape } : {}), + ...(op.fill ? { fill: op.fill } : {}), + ...(op.stroke ? { stroke: op.stroke } : {}), ...cellOf(op), } else if (op.op === "add_container") diff --git a/lib/diagram-engine/render.ts b/lib/diagram-engine/render.ts index fca386d..7a618df 100644 --- a/lib/diagram-engine/render.ts +++ b/lib/diagram-engine/render.ts @@ -43,6 +43,7 @@ import { type Rect, type SequenceNode, } from "./types" +import type { Point } from "./visgraph" /** Escape the five characters that would break an XML attribute. */ export function esc(s: string): string { @@ -62,6 +63,33 @@ export type StyleResolver = ( const FALLBACK_BOX = "rounded=0;whiteSpace=wrap;html=1;fillColor=#FFFFFF;strokeColor=#5A6B7B;fontColor=#1A1A1A;fontSize=11;verticalAlign=middle;" + +/** + * The palette for semantic groups: paired fill/stroke, assigned to group names in order of + * first appearance. + * + * The engine owns these hex values and the model never sees them — it only names groups + * ("remote", "local", "temp"), which is the judgement it is actually good at. Letting the + * model pick colours per diagram produced mismatched saturations and a different look every + * time; a fixed palette is what makes two diagrams from the same engine look related. + * + * All six fills are low-saturation tints that keep #1A1A1A text readable, each with a + * darker stroke of the same hue, deliberately quieter than the near-black edge lines so + * colour reads as grouping rather than emphasis. + */ +const GROUP_PALETTE: { fill: string; stroke: string }[] = [ + { fill: "#DAE8FC", stroke: "#6C8EBF" }, // blue + { fill: "#D5E8D4", stroke: "#82B366" }, // green + { fill: "#FFE6CC", stroke: "#D79B00" }, // orange + { fill: "#E1D5E7", stroke: "#9673A6" }, // purple + { fill: "#F8CECC", stroke: "#B85450" }, // red + { fill: "#FFF2CC", stroke: "#D6B656" }, // yellow +] + +/** fill/stroke for the n-th distinct group. Wraps: a 7th group reuses the 1st colour. */ +export function groupColour(index: number): { fill: string; stroke: string } { + return GROUP_PALETTE[index % GROUP_PALETTE.length] +} const FALLBACK_FRAME = "rounded=0;whiteSpace=wrap;html=1;fillColor=#FFFFFF;strokeColor=#999999;fontColor=#1A1A1A;fontSize=12;fontStyle=1;verticalAlign=top;align=left;spacingLeft=8;spacingTop=4;" const TITLE_STYLE = @@ -432,6 +460,103 @@ function edgeLabel(l: LinkSpec): string { return l.label ? `${l.step}. ${l.label}` : `${l.step}.` } +/** + * Slide each edge label along its own edge to a spot where it covers nothing. + * + * A label renders centred on the path midpoint, and on a long edge that midpoint is + * frequently on top of something — the edge was routed AROUND the boxes, so its middle + * passes exactly the things it avoided, and the router has never known labels exist. + * Measured on a git-workflow diagram: four labels sat on unrelated boxes or on each other. + * + * For each labelled edge, in order of path length (longest first, since they have the + * fewest clear spots), positions along the path are tried from the middle outwards; the + * first where the label's rectangle overlaps no box and no already-placed label wins. + * draw.io expresses the position as the geometry's relative x: −1 at the source, 0 at the + * midpoint, +1 at the target. + * + * The label's size is an estimate (7px per character, one line). That is fine here: the + * goal is to stop labels sitting ON things, and a near miss by a few pixels still reads + * clearly, where the current midpoint placement puts them dead centre on a box. + */ +function placeLabels( + edges: { id: string; label: string; path: Point[] }[], + boxes: Rect[], +): Map { + const placed: Rect[] = [] + const out = new Map() + + const measure = (label: string): { w: number; h: number } => ({ + w: Math.min(160, label.length * 7 + 8), + h: 16, + }) + const pointAt = (path: Point[], t: number): Point => { + let total = 0 + const segs = path.slice(0, -1).map((p, i) => { + const len = + Math.abs(path[i + 1].x - p.x) + Math.abs(path[i + 1].y - p.y) + total += len + return { a: p, b: path[i + 1], len } + }) + let at = total * t + for (const s of segs) { + if (at <= s.len || s === segs[segs.length - 1]) { + const f = s.len ? Math.min(1, at / s.len) : 0 + return { + x: s.a.x + (s.b.x - s.a.x) * f, + y: s.a.y + (s.b.y - s.a.y) * f, + } + } + at -= s.len + } + return path[0] + } + const overlaps = (r: Rect, list: Rect[]) => + list.some( + (o) => + r.x < o.x + o.w && + o.x < r.x + r.w && + r.y < o.y + o.h && + o.y < r.y + r.h, + ) + + const byLength = [...edges].sort((p, q) => { + const len = (e: { path: Point[] }) => + e.path.reduce( + (s, pt, i) => + i === 0 + ? 0 + : s + + Math.abs(pt.x - e.path[i - 1].x) + + Math.abs(pt.y - e.path[i - 1].y), + 0, + ) + return len(q) - len(p) + }) + + // The midpoint first — it is where a reader expects the label — then nearby spots, + // preferring the source half slightly: a label near the arrow's origin still reads as + // naming the action. + const TRIES = [0.5, 0.42, 0.58, 0.34, 0.66, 0.26, 0.74, 0.18, 0.82] + for (const e of byLength) { + const { w, h } = measure(e.label) + let chosen = 0.5 + for (const t of TRIES) { + const c = pointAt(e.path, t) + const rect = { x: c.x - w / 2, y: c.y - h / 2, w, h } + if (!overlaps(rect, boxes) && !overlaps(rect, placed)) { + chosen = t + break + } + } + const c = pointAt(e.path, chosen) + placed.push({ x: c.x - w / 2, y: c.y - h / 2, w, h }) + // Even a spot that still overlaps is recorded, so the NEXT label avoids stacking + // on top of it — two labels on one point is strictly worse than one on a box. + if (chosen !== 0.5) out.set(e.id, chosen * 2 - 1) + } + return out +} + /** * One `` for an edge, carrying the route the router computed. * @@ -446,7 +571,12 @@ function edgeLabel(l: LinkSpec): string { * sits at the path midpoint and needs a straight segment under it) or a deliberate detour * around something a straight line would have hit. */ -function edgeXml(l: LinkSpec, index: number, route?: RoutedEdge): string { +function edgeXml( + l: LinkSpec, + index: number, + route?: RoutedEdge, + labelAt?: number, +): string { const label = edgeLabel(l) let style = l.style ?? EDGE_STYLE if (!l.style) { @@ -467,10 +597,16 @@ function edgeXml(l: LinkSpec, index: number, route?: RoutedEdge): string { ) .join("")}` : "" + // The geometry's x is the label's position along the path: −1 source, 0 middle, +1 + // target. Written only when the label had to move off the midpoint to cover nothing. + const geo = + labelAt !== undefined + ? `${points}` + : `${points}` return ( `` + - `${points}` + + geo + `` ) } @@ -731,8 +867,42 @@ export function renderDiagram( obstacles, frames, ) + // Where each label goes along its edge. The router only kept LINES off the boxes; a + // label sits at the path midpoint, which on a long edge is exactly beside the things + // the line was routed around. + const labelled = routable + .map(({ link: l, index }, i) => { + const label = edgeLabel(l) + if (!label) return null + const a = cellById.get(l.source) + const b = cellById.get(l.target) + const r = routes[i] + if (!a || !b || !r) return null + const sp = { + x: a.x + r.exit.x * a.w, + y: a.y + r.exit.y * a.h, + } + const ep = { + x: b.x + r.entry.x * b.w, + y: b.y + r.entry.y * b.h, + } + return { + id: l.id ?? `ed${index + 1}`, + label, + path: [sp, ...r.waypoints, ep], + } + }) + .filter((e): e is { id: string; label: string; path: Point[] } => + Boolean(e), + ) + const labelBoxes = [...obstacles] + .map((id) => cellById.get(id)) + .filter((r): r is Rect => Boolean(r)) + const labelAt = placeLabels(labelled, labelBoxes) + routable.forEach(({ link, index }, i) => { - cells.push(edgeXml(link, index, routes[i])) + const id = link.id ?? `ed${index + 1}` + cells.push(edgeXml(link, index, routes[i], labelAt.get(id))) }) for (const m of messages) cells.push(messageXml(m.link, m.index, m.y, cellById)) diff --git a/lib/diagram-engine/route.ts b/lib/diagram-engine/route.ts index 9bcc70e..d87c052 100644 --- a/lib/diagram-engine/route.ts +++ b/lib/diagram-engine/route.ts @@ -436,6 +436,60 @@ export function routeEdges( } } + // --- stage 1c: pair opposite edges + // + // A→B and B→A are one relationship drawn as two arrows — "git add" out, "git reset" + // back. Left to the general de-collide they land on port positions chosen for entirely + // separate reasons at each end, so one line runs straight while its partner wanders off + // through a different corridor. A reader expects a matched pair: two parallel lines a + // constant gap apart, one clearly out and one clearly back. + // + // The two tracks are ABSOLUTE positions in the strip where the boxes overlap — the + // corridor's centre ± half a track gap — converted back to a fraction of each box. + // Assigning the same fraction to both boxes instead only works when they happen to be + // the same size and aligned; on real diagrams it put the two lines 79px apart with a + // kink in one of them. + const seen = new Map() + edges.forEach((e, i) => { + seen.set(`${e.source}|${e.target}`, i) + }) + const PAIR_GAP = 24 + const pairedDone = new Set() + edges.forEach((e, i) => { + if (pairedDone.has(i)) return + const j = seen.get(`${e.target}|${e.source}`) + if (j === undefined || j === i || pairedDone.has(j)) return + const fi = faces[i] + const fj = faces[j] + const a = rects.get(e.source) + const b = rects.get(e.target) + if (!fi || !fj || !a || !b) return + // Only pair edges that agree on the axis; when they disagree the geometry wants + // them apart, and forcing them together would fight the search. + if (fi.horiz !== fj.horiz) return + + // The strip both boxes span, on the axis ACROSS the arrows. Two straight parallel + // tracks need the corridor to hold them both. + const lo = fi.horiz ? Math.max(a.y, b.y) : Math.max(a.x, b.x) + const hi = fi.horiz + ? Math.min(a.y + a.h, b.y + b.h) + : Math.min(a.x + a.w, b.x + b.w) + if (hi - lo < PAIR_GAP + 12) return + + const mid = (lo + hi) / 2 + const t1 = mid - PAIR_GAP / 2 + const t2 = mid + PAIR_GAP / 2 + const fracOf = (r: Rect, track: number) => + fi.horiz ? (track - r.y) / r.h : (track - r.x) / r.w + + pairedDone.add(i) + pairedDone.add(j) + frac[i].s = fracOf(a, t1) + frac[i].t = fracOf(b, t1) + frac[j].s = fracOf(b, t2) + frac[j].t = fracOf(a, t2) + }) + // --- stage 2: try shapes in order of directness, keep the first that is clear // // Segments already claimed by a routed edge, so later edges can avoid sharing a lane diff --git a/lib/system-prompts.ts b/lib/system-prompts.ts index 18b77ac..44823bc 100644 --- a/lib/system-prompts.ts +++ b/lib/system-prompts.ts @@ -73,11 +73,12 @@ parameters: { tool name: draw_graph description: Draw a flowchart, decision tree, dependency graph, ER diagram or site map from nodes and arrows alone. You give NO positions and NO nesting; the engine works out how many rows there are, who shares a row, and who goes left of whom, so arrows do not cross or run through unrelated boxes. Replaces the whole diagram — use restructure_diagram to edit afterwards. parameters: { - nodes: Array<{id: string, label: string, shape?: "box"|"decision"|"terminator"|"round"|"data"|"document", icon?: string}> + nodes: Array<{id: string, label: string, shape?: "box"|"decision"|"terminator"|"round"|"data"|"document", icon?: string, group?: string}> edges: Array<{source: string, target: string, label?: string, dashed?: boolean}> title?: string flow?: "col" | "row" // col (default): top to bottom. row: left to right } +Set the same group name on nodes that belong to one zone (remote vs local, frontend vs backend, roles); the engine colours each group consistently. Never pick colours yourself. ---End of tools--- ## Choosing the right tool diff --git a/tests/unit/diagram-engine-graph.test.ts b/tests/unit/diagram-engine-graph.test.ts index ee50923..e1fe85b 100644 --- a/tests/unit/diagram-engine-graph.test.ts +++ b/tests/unit/diagram-engine-graph.test.ts @@ -4,6 +4,7 @@ import { type GraphEdge, type GraphNode, graphToOperations, + restructureDiagram, } from "@/lib/diagram-engine" import { absoluteRects, @@ -442,3 +443,234 @@ describe("graphToOperations: arrows leave their own shape cleanly", () => { expect(waypointsInsideBoxes(r.xml as string)).toEqual([]) }) }) + +/** + * Edge labels must not sit on boxes or on each other. + * + * The router keeps LINES off the boxes, but a label renders at its edge's midpoint — and on + * a long edge that midpoint is beside exactly the things the line was routed around. On the + * reported git-workflow diagram, four labels sat on unrelated boxes or on other labels. + * `placeLabels` slides each label along its own edge to a clear spot, written as the + * geometry's relative x. + */ +describe("edge labels avoid boxes and each other", () => { + it("places every git-workflow label on empty space", () => { + const r = drawGraph( + [ + { + id: "remote", + label: "Remote Repository (origin / GitHub)", + shape: "data", + }, + n("work", "Working Directory (edited files)"), + n("stage", "Staging Area (index)"), + { + id: "stash", + label: "Stash (temporary shelf)", + shape: "round", + }, + n("local", "Local Repository (.git commits)"), + { + id: "branch", + label: "Branch / Merge (feature -> main)", + shape: "decision", + }, + ], + [ + e("remote", "work", "git clone"), + e("work", "stage", "git add"), + { + source: "stage", + target: "work", + label: "git reset", + dashed: true, + }, + e("work", "stash", "git stash"), + e("stash", "work", "git stash pop"), + e("stage", "local", "git commit"), + { + source: "local", + target: "stage", + label: "git checkout", + dashed: true, + }, + e("local", "work", "git pull / merge"), + { + source: "remote", + target: "local", + label: "git fetch", + dashed: true, + }, + e("local", "branch", "git branch / checkout -b"), + e("branch", "remote", "git push"), + ], + { title: "Git Operations Overview" }, + ) + expect(r.errors).toEqual([]) + const xml = r.xml as string + const rects = absoluteRects(xml) + const ids = ["remote", "work", "stage", "stash", "local", "branch"] + + // Recompute each label's rectangle the way draw.io places it: at the geometry's + // relative x along the path (−1 source, 0 middle, +1 target). + const labels: { + id: string + x: number + y: number + w: number + h: number + }[] = [] + for (const p of edgePaths(xml, rects)) { + if (!p.label) continue + const m = xml.match( + new RegExp( + `]*>\\s* { + const len = + Math.abs(p.points[i + 1].x - pt.x) + + Math.abs(p.points[i + 1].y - pt.y) + total += len + return { a: pt, b: p.points[i + 1], len } + }) + let at = total * t + let pos = p.points[0] + for (const s of segs) { + if (at <= s.len || s === segs[segs.length - 1]) { + const f = s.len ? Math.min(1, at / s.len) : 0 + pos = { + x: s.a.x + (s.b.x - s.a.x) * f, + y: s.a.y + (s.b.y - s.a.y) * f, + } + break + } + at -= s.len + } + const w = Math.min(160, p.label.length * 7 + 8) + labels.push({ id: p.id, x: pos.x - w / 2, y: pos.y - 8, w, h: 16 }) + } + + const bad: string[] = [] + for (const l of labels) { + for (const id of ids) { + const b = rectOf(rects, id) + if ( + l.x < b.x + b.w && + l.x + l.w > b.x && + l.y < b.y + b.h && + l.y + l.h > b.y + ) + bad.push(`${l.id} label sits on ${id}`) + } + for (const m of labels) + if ( + m !== l && + l.id < m.id && + l.x < m.x + m.w && + l.x + l.w > m.x && + l.y < m.y + m.h && + l.y + l.h > m.y + ) + bad.push(`${l.id} label sits on ${m.id} label`) + } + expect(bad).toEqual([]) + }) +}) + +/** + * A→B and B→A are one relationship drawn as two arrows, and a reader expects a matched + * pair: two parallel lines a constant gap apart. Routed independently they land on port + * positions chosen for unrelated reasons, so one line runs straight while its partner + * wanders through a different corridor with a kink in it. + */ +describe("opposite edges are drawn as a parallel pair", () => { + it("gives git add / git reset two straight tracks a constant gap apart", () => { + const r = drawGraph( + [n("work", "Working Directory"), n("stage", "Staging Area")], + [ + e("work", "stage", "git add"), + { + source: "stage", + target: "work", + label: "git reset", + dashed: true, + }, + ], + ) + expect(r.errors).toEqual([]) + const xml = r.xml as string + const rects = absoluteRects(xml) + const paths = edgePaths(xml, rects) + const fwd = paths.find((p) => p.source === "work") + const rev = paths.find((p) => p.source === "stage") + if (!fwd || !rev) throw new Error("both edges must render") + + // Both straight: two points, no waypoints. + expect(fwd.points).toHaveLength(2) + expect(rev.points).toHaveLength(2) + // Parallel vertical tracks a constant gap apart. + expect(Math.abs(fwd.points[0].x - fwd.points[1].x)).toBeLessThan(1) + expect(Math.abs(rev.points[0].x - rev.points[1].x)).toBeLessThan(1) + const gap = Math.abs(fwd.points[0].x - rev.points[0].x) + expect(gap).toBeGreaterThan(12) + expect(gap).toBeLessThan(40) + }) +}) + +/** + * Semantic groups: the caller names zones, the engine colours them. + * + * The model is good at judging which nodes belong together and bad at picking hex colours + * that match; letting it choose produced mismatched saturations and a different look per + * diagram. So `group` maps to a fixed engine palette in order of first appearance, and the + * same grouping always produces the same colours. + */ +describe("draw_graph: semantic groups", () => { + it("colours nodes by group, consistently, without the caller naming a colour", () => { + const r = drawGraph( + [ + { id: "a", label: "A", group: "remote" }, + { id: "b", label: "B", group: "local" }, + { id: "c", label: "C", group: "local" }, + n("d", "D"), + ], + [e("a", "b"), e("b", "c"), e("c", "d")], + ) + expect(r.errors).toEqual([]) + const xml = r.xml as string + const fillOf = (id: string) => { + const m = xml.match(new RegExp(`id="${id}"[^>]*style="([^"]*)"`)) + return [...(m?.[1] ?? "").matchAll(/fillColor=([^;]*)/g)].pop()?.[1] + } + // First group named gets the first palette slot; same group, same colour. + expect(fillOf("a")).toBe("#DAE8FC") + expect(fillOf("b")).toBe("#D5E8D4") + expect(fillOf("c")).toBe("#D5E8D4") + // No group: the plain white fallback. + expect(fillOf("d")).toBe("#FFFFFF") + }) + + it("group colours survive a round trip through the canvas", () => { + const r = drawGraph( + [ + { id: "a", label: "A", group: "g1" }, + { id: "b", label: "B", group: "g2" }, + ], + [e("a", "b")], + ) + const again = restructureDiagram(r.xml as string, []) + const fillOf = (xml: string, id: string) => { + const m = xml.match(new RegExp(`id="${id}"[^>]*style="([^"]*)"`)) + return [...(m?.[1] ?? "").matchAll(/fillColor=([^;]*)/g)].pop()?.[1] + } + expect(fillOf(again.xml as string, "a")).toBe( + fillOf(r.xml as string, "a"), + ) + expect(fillOf(again.xml as string, "b")).toBe( + fillOf(r.xml as string, "b"), + ) + }) +})