From 50826c0ac81c448c6e867924b3579c83dc951b95 Mon Sep 17 00:00:00 2001 From: "dayuan.jiang" Date: Sun, 9 Aug 2026 21:56:09 +0900 Subject: [PATCH] =?UTF-8?q?feat(diagram-engine):=20open=20shape=20vocabula?= =?UTF-8?q?ry=20=E2=80=94=20catalog=20+=20pass-through,=20structured=20sty?= =?UTF-8?q?le=20merge?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The expressiveness gap traced to vocabulary: draw.io has hundreds of shapes, the declarative layer allowed six. Opening it, with the failure modes from design review handled: - shapes.ts: a ~20-entry curated catalog (full style fragment incl. the matching perimeter — required or edges connect to the bounding box; a text-scale factor verified in the real editor — the same sentence overflows a 1.0x rhombus and fits a 1.5x one; labelOutside+glyph for umlActor-style figures). Any other token passes through verbatim: draw.io degrades unknown shapes to rectangles safely (verified). Injection-capable tokens (;/=) are rejected outright. - Pass-through emits a WARNING with a nearest-catalog hint (bounded edit distance), so a typo'd 'cyclinder' is a one-turn fix instead of a silently rectangular node forever. set_shape/set_role/set_group operations make the fix possible without remove+re-add (which would drop links). - mergeStyle(): style fragments merge per-key (later wins, bare shape classes displace each other) instead of string concatenation. This is what makes shape and theme composable by rule — shape owns geometry keys, theme owns colour/type keys, and an overlap resolves by order instead of emitting contradictory duplicates. - dai_shape marker carries the declared token through the round trip: appearance-based reverse mapping cannot distinguish aliases (diamond vs decision) or a rotated queue from a cylinder. - dai_auto marker separates engine-measured size from user-fixed size: parsed boxes no longer freeze the first layout's numbers, so changing a label re-measures. Pinned nodes keep everything, as before. - draw_graph's closed shape enum opened to match (first instance of the schema-drift problem the review predicted). 14 new tests: merge ownership, injection rejection, near-match hints, alias-preserving round trip, re-measure on label change, mxgraph.* tokens staying boxes with role/group intact. 556 unit tests green; acceptance diagram (person/hexagon/cylinder/queue/cloud/decision/callout) verified in the real editor. --- app/api/chat/route.ts | 16 +- lib/diagram-engine/index.ts | 21 +- lib/diagram-engine/layout.ts | 22 ++- lib/diagram-engine/markers.ts | 27 +++ lib/diagram-engine/operations.ts | 85 +++++++- lib/diagram-engine/parse.ts | 27 ++- lib/diagram-engine/render.ts | 58 +++--- lib/diagram-engine/shapes.ts | 235 +++++++++++++++++++++++ lib/diagram-engine/types.ts | 17 +- lib/system-prompts.ts | 15 +- tests/unit/diagram-engine-shapes.test.ts | 196 +++++++++++++++++++ 11 files changed, 644 insertions(+), 75 deletions(-) create mode 100644 lib/diagram-engine/shapes.ts create mode 100644 tests/unit/diagram-engine-shapes.test.ts diff --git a/app/api/chat/route.ts b/app/api/chat/route.ts index add668a..73235ee 100644 --- a/app/api/chat/route.ts +++ b/app/api/chat/route.ts @@ -790,7 +790,7 @@ 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 say what a node IS: "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, "cylinder" for a database, "queue" for a message queue, "person" for an actor or user, "cloud" for an external system, "hexagon" for a service, "ellipse" for a concept, "box" (default) for a plain step. Any other draw.io shape token also works verbatim. 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({ @@ -800,15 +800,11 @@ Grouping: when the nodes fall into natural zones (remote vs local, frontend vs b id: z.string(), label: z.string(), shape: z - .enum([ - "box", - "decision", - "terminator", - "round", - "data", - "document", - ]) - .optional(), + .string() + .optional() + .describe( + "What the node IS: decision, terminator, round, data, document, cylinder (database), queue, person (actor), cloud (external), hexagon (service), ellipse. Any draw.io shape token also works", + ), icon: z .string() .optional() diff --git a/lib/diagram-engine/index.ts b/lib/diagram-engine/index.ts index 8deac7f..fdcc57b 100644 --- a/lib/diagram-engine/index.ts +++ b/lib/diagram-engine/index.ts @@ -25,7 +25,8 @@ import { } from "./operations" import { parseDiagram } from "./parse" import { renderDiagram } from "./render" -import type { DiagramTree } from "./types" +import { nearestShape, resolveShape } from "./shapes" +import { type DiagramTree, walkTree } from "./types" export interface RestructureResult { /** New canvas XML, or null when the request could not be carried out. */ @@ -85,6 +86,24 @@ export function restructureDiagram( ) } + // Shape tokens: an injection-capable token is an error; an unknown-but-safe one + // passes through (draw.io degrades it to a rectangle) but gets a warning, so a typo + // is a one-turn fix instead of a silently rectangular "cyclinder" forever. + for (const n of walkTree(applied.tree)) { + if (n.kind !== "box" || !n.shape || n.shape === "box") continue + const resolved = resolveShape(n.shape) + if (!resolved) { + errors.push( + `shape "${n.shape}" (node ${n.id}) contains characters that are not allowed in a shape token.`, + ) + } else if (resolved.passthrough) { + const near = nearestShape(n.shape) + warnings.push( + `shape "${n.shape}" (node ${n.id}) is not in the engine's catalog — passed through to draw.io, which renders unknown shapes as rectangles.${near ? ` Did you mean "${near}"?` : ""}`, + ) + } + } + if (errors.length > 0) return { xml: null, outline: outline(applied.tree), errors, warnings } diff --git a/lib/diagram-engine/layout.ts b/lib/diagram-engine/layout.ts index ca71a1b..3e584d3 100644 --- a/lib/diagram-engine/layout.ts +++ b/lib/diagram-engine/layout.ts @@ -28,6 +28,7 @@ * `pool()` primitive; sequence and radial are original to this repository. */ +import { resolveShape } from "./shapes" import { type Role, roleMetrics } from "./theme" import type { ContainerNode, @@ -212,7 +213,18 @@ function visibleText(label: string): string { export function autoBoxSize( label: string, role?: Role, + shape?: string, ): { w: number; h: number } { + const spec = shape ? resolveShape(shape)?.spec : undefined + // A glyph shape (umlActor…) has a fixed figure with the label below it: the slot is + // the figure plus a line of text, and the text length does not scale the figure. + if (spec?.labelOutside && spec.glyph) { + const text = visibleText(label) + return { + w: Math.max(spec.glyph.w + 20, Math.min(160, text.length * 7 + 16)), + h: spec.glyph.h + 22, + } + } const r = roleMetrics(role) const maxW = Math.round(260 * Math.max(1, r.charScale)) const explicit = visibleText(label).split("\n") @@ -233,7 +245,13 @@ export function autoBoxSize( 0, ) const lineH = Math.round(r.fontSize * 1.6) - return { w, h: Math.max(r.minH, lines * lineH + 26) } + const h = Math.max(r.minH, lines * lineH + 26) + // A non-rectangular outline inscribes a smaller text area than its bounding box — + // a rhombus exactly half — so the box grows by the shape's measured factor. + // Verified in the real editor: the same sentence overflows a 1.0× rhombus and fits + // a 1.5× one. + const s = spec?.textScale ?? 1 + return { w: Math.round(w * s), h: Math.round(h * s) } } /** @@ -432,7 +450,7 @@ function measure( return { node: n, rect: { x: 0, y: 0, ...s }, children: [] } } if (n.kind === "box") { - const auto = autoBoxSize(n.label, n.role) + const auto = autoBoxSize(n.label, n.role, n.shape) return { node: n, rect: { x: 0, y: 0, w: n.w ?? auto.w, h: n.h ?? auto.h }, diff --git a/lib/diagram-engine/markers.ts b/lib/diagram-engine/markers.ts index 4aa282b..aa585bb 100644 --- a/lib/diagram-engine/markers.ts +++ b/lib/diagram-engine/markers.ts @@ -64,6 +64,18 @@ export const MARKER = { role: "dai_role", /** The node's semantic zone, whose hue ramp colours it. */ group: "dai_group", + /** + * The declared shape token, verbatim. Appearance-based reverse mapping is ambiguous + * (aliases, rotated variants, styles with no unique shape= token), so the round trip + * carries the declaration itself. + */ + shape: "dai_shape", + /** + * Marks a node's size as engine-computed rather than user-fixed. Without it, the + * w/h read back from the canvas would freeze the first layout's measurement: change + * the label and the box would keep the old size instead of re-measuring. + */ + auto: "dai_auto", /** Share of the parent's leftover flow-axis space — flex-grow. */ grow: "dai_grow", /** Cross-axis position within the parent: "start" | "center" | "end". */ @@ -388,6 +400,21 @@ export function stampGroup(style: string, group: string): string { return append(style, MARKER.group, encodeURIComponent(group)) } +/** Stamp the declared shape token, so the round trip carries the declaration itself. */ +export function stampShape(style: string, shape: string): string { + return append(style, MARKER.shape, encodeURIComponent(shape)) +} + +/** Mark a node's size as engine-computed, so a re-layout re-measures it. */ +export function stampAuto(style: string): string { + return append(style, MARKER.auto, 1) +} + +/** Was this node's size computed by the engine (vs fixed by the user or the model)? */ +export function isAutoSized(style: string): boolean { + return readMarker(style, MARKER.auto) === "1" +} + type FlexAlign = "start" | "center" | "end" | "stretch" /** Stamp the flex fields a node carries, so a round-trip preserves them. */ diff --git a/lib/diagram-engine/operations.ts b/lib/diagram-engine/operations.ts index e0fbde8..f6cdf04 100644 --- a/lib/diagram-engine/operations.ts +++ b/lib/diagram-engine/operations.ts @@ -86,17 +86,10 @@ export const OperationSchema = z.discriminatedUnion("op", [ .optional() .describe("Border colour; pair it with fill"), shape: z - .enum([ - "box", - "decision", - "terminator", - "round", - "data", - "document", - ]) + .string() .optional() .describe( - "Flowchart outline: decision=diamond, terminator=start/end, data=input/output, document=report. Omit for a plain rectangle", + "What the node IS, drawn as its conventional outline. Catalog: decision/diamond, terminator (start/end), round, data (input/output), document, cylinder (database), queue, person (actor/user), cloud (external system), hexagon (service), ellipse (concept), callout (note), step (pipeline stage), note, card, process, tape, cube. Any other draw.io shape token also works verbatim. Omit for a plain rectangle", ), grow: z .number() @@ -261,6 +254,36 @@ export const OperationSchema = z.discriminatedUnion("op", [ id: z.string(), label: z.string(), }), + z.object({ + op: z.literal("set_shape"), + id: z.string(), + shape: z + .string() + .describe("New shape token; 'box' resets to a plain rectangle"), + }), + z.object({ + op: z.literal("set_role"), + id: z.string(), + role: z + .enum([ + "banner", + "heading", + "body", + "callout", + "good", + "bad", + "metric", + "muted", + ]) + .describe("New information role; 'body' resets to the default"), + }), + z.object({ + op: z.literal("set_group"), + id: z.string(), + group: z + .string() + .describe("New semantic zone; empty string removes the zone"), + }), z.object({ op: z.literal("set_dir"), id: z.string().describe("Container to re-orient"), @@ -579,6 +602,50 @@ export function applyOperations( break } + case "set_shape": { + const node = findNode(tree, op.id) + if (!node || node.kind !== "box") { + errors.push(`set_shape: "${op.id}" is not a box`) + break + } + // The verbatim style is last render's composition with the OLD shape + // baked in; keeping it would override the new declaration entirely. + if (op.shape === "box") delete node.shape + else node.shape = op.shape + delete node.style + delete node.w + delete node.h + break + } + + case "set_role": { + const node = findNode(tree, op.id) + if (!node || (node.kind !== "box" && node.kind !== "group")) { + errors.push( + `set_role: "${op.id}" is not a box or container`, + ) + break + } + if (op.role === "body") delete node.role + else node.role = op.role + delete node.style + break + } + + case "set_group": { + const node = findNode(tree, op.id) + if (!node || (node.kind !== "box" && node.kind !== "group")) { + errors.push( + `set_group: "${op.id}" is not a box or container`, + ) + break + } + if (op.group === "") delete node.group + else node.group = op.group + delete node.style + break + } + case "set_dir": { const node = findNode(tree, op.id) if (!node || !isContainer(node)) { diff --git a/lib/diagram-engine/parse.ts b/lib/diagram-engine/parse.ts index a8413b5..d2946c5 100644 --- a/lib/diagram-engine/parse.ts +++ b/lib/diagram-engine/parse.ts @@ -25,6 +25,7 @@ import { extractDiagramXML } from "@/lib/utils" import { type Direction, hasMarkers, + isAutoSized, isLaneChrome, isPinned, MARKER, @@ -361,6 +362,12 @@ function zoneOf(style: string): string | undefined { return v ? decodeURIComponent(v) : undefined } +/** The declared shape token stamped on a cell, if any. */ +function shapeOf(style: string): string | undefined { + const v = readMarker(style, MARKER.shape) + return v ? decodeURIComponent(v) : undefined +} + /** The flex fields stamped on a cell, as sparse properties ready to spread. */ function flexOf(style: string): { grow?: number @@ -1069,19 +1076,29 @@ export function parseDiagram(xml: string, pageIndex = 0): ParseResult { const headH = lifeline ? Number(styleValue(c.style, "size") ?? "44") : undefined + // An engine-measured box must NOT read its size back as a fixed size: + // that would freeze the first layout's measurement, so a later label or + // shape change would keep the stale box instead of re-measuring. A pinned + // node is the exception — the user froze it, geometry and all. + const auto = isAutoSized(c.style) && !pinned const node: BoxNode = { kind: "box", id: c.id, label: c.value, - w: c.geo ? Math.round(c.geo.w) : undefined, + w: auto ? undefined : c.geo ? Math.round(c.geo.w) : undefined, h: lifeline ? Math.round(headH && headH > 0 ? headH : 44) - : c.geo - ? Math.round(c.geo.h) - : undefined, + : auto + ? undefined + : c.geo + ? Math.round(c.geo.h) + : undefined, fill: styleValue(c.style, "fillColor"), stroke: styleValue(c.style, "strokeColor"), - shape: boxShape(c.style), + // The declared token wins: appearance-based reverse mapping cannot + // distinguish aliases (decision vs diamond) or identify a passed-through + // token whose style is just `shape=`. + shape: shapeOf(c.style) ?? boxShape(c.style), role: roleOf(c.style), group: zoneOf(c.style), ...flexOf(c.style), diff --git a/lib/diagram-engine/render.ts b/lib/diagram-engine/render.ts index 895ea82..b8ba6f4 100644 --- a/lib/diagram-engine/render.ts +++ b/lib/diagram-engine/render.ts @@ -23,6 +23,7 @@ import { } from "./layout" import { isInvisible, + stampAuto, stampCell, stampContainer, stampFlex, @@ -34,11 +35,12 @@ import { stampRadial, stampRole, stampSequence, + stampShape, } from "./markers" import { type RoutedEdge, routeEdges } from "./route" +import { mergeStyle, resolveShape } from "./shapes" import { hueOf, NEUTRAL, themedStyle } from "./theme" import { - type BoxShape, type DiagramNode, type DiagramTree, isContainer, @@ -88,27 +90,6 @@ const TITLE_STYLE = const EDGE_STYLE = "edgeStyle=orthogonalEdgeStyle;html=1;rounded=0;jettySize=auto;orthogonalLoop=1;fontSize=10;fontColor=light-dark(#1B2733,#CFE0F0);strokeColor=light-dark(#1A1A1A,#E0E0E0);strokeWidth=1;" -/** - * Flowchart outlines, as mxGraph draws them. - * - * All six are core mxGraph shapes, not stencils from a shape library, so they render - * without the catalog and without any extra dependency. The notation is conventional: a - * reader takes a diamond to mean a branch and a stadium to mean a start or end point, so - * drawing every step as the same rectangle loses information the shape was carrying. - */ -const BOX_SHAPES: Record = { - box: "rounded=0;", - round: "rounded=1;arcSize=12;", - /** Decision — a diamond. */ - decision: "rhombus;", - /** Start or end — a stadium. draw.io draws `rounded=1` at arcSize 50 as a full stadium. */ - terminator: "rounded=1;arcSize=50;", - /** Input or output — a parallelogram. */ - data: "shape=parallelogram;perimeter=parallelogramPerimeter;fixedSize=1;size=14;", - /** A document or report — a rectangle with a wavy bottom edge. */ - document: "shape=document;boundedLbl=1;", -} - // ---- swimlane pool chrome ---- /** Hairline between lane bands: present, but quieter than the shapes sitting on it. */ @@ -172,21 +153,34 @@ function styleFor( } if (n.kind === "box") { - let base = n.style ?? FALLBACK_BOX - if (!n.style) { - // Order matters, later keys win in draw.io: outline, then the theme's - // composition of role x hue, then explicit colours on top of everything. - if (n.shape) base += BOX_SHAPES[n.shape] - if (n.role || n.group) - base += themedStyle(n.role ?? "body", hue(n.group), "leaf") - if (n.fill) base += `fillColor=${n.fill};` - if (n.stroke) base += `strokeColor=${n.stroke};` - if (n.bold) base += "fontStyle=1;" + let base: string + if (n.style) { + base = n.style + } else { + // Structured merge, ownership by fragment order: the fallback's neutral + // look, the shape's geometry keys, the theme's colour/type keys, explicit + // colours last. Each key ends up in the style exactly once — a theme that + // says rounded=1 cannot leave a contradictory duplicate on a rhombus. + const shape = n.shape ? resolveShape(n.shape) : null + base = mergeStyle( + FALLBACK_BOX, + shape?.spec.style, + n.role || n.group + ? themedStyle(n.role ?? "body", hue(n.group), "leaf") + : undefined, + n.fill ? `fillColor=${n.fill};` : undefined, + n.stroke ? `strokeColor=${n.stroke};` : undefined, + n.bold ? "fontStyle=1;" : undefined, + ) } let stamped = stampLeaf(base, "box") + if (n.shape && n.shape !== "box") stamped = stampShape(stamped, n.shape) if (n.role && n.role !== "body") stamped = stampRole(stamped, n.role) if (n.group) stamped = stampGroup(stamped, n.group) stamped = stampFlex(stamped, { grow: n.grow, align: n.align }) + // Engine-measured (no explicit w/h): mark it, so the parser re-measures next + // time instead of freezing this layout's numbers as a fixed size. + if (n.w == null && n.h == null) stamped = stampAuto(stamped) return n.cell ? stampCell(stamped, n.cell) : stamped } diff --git a/lib/diagram-engine/shapes.ts b/lib/diagram-engine/shapes.ts new file mode 100644 index 0000000..067253c --- /dev/null +++ b/lib/diagram-engine/shapes.ts @@ -0,0 +1,235 @@ +/** + * The shape vocabulary: what a box can BE, beyond a labelled rectangle. + * + * draw.io ships hundreds of shapes; the engine's declarative layer used to allow six. + * That gap — not colours, not spacing — was why engine output looked flat next to + * hand-written XML: a database drawn as a grey rectangle labelled "database" instead of + * a cylinder. This module opens the vocabulary in two tiers: + * + * CATALOG — ~20 curated shapes the engine fully understands. Each entry carries the + * complete style fragment (including the matching `perimeter=`, which draw.io's own + * style reference warns is required or edges connect to the bounding box), how much + * larger the box must be for its text to fit inside the non-rectangular outline + * (verified empirically in the real editor: a rhombus needs ~1.5× the rectangle's + * size for the same text), and whether the label renders below the glyph instead of + * inside it. + * + * PASS-THROUGH — any other token that looks like a draw.io shape name is emitted + * verbatim as `shape=;`. Verified in the real editor: an unknown token + * degrades to a rectangle, it does not break the page. A conservative text scale + * covers the common case that the real shape is roughly convex. The tool response + * carries a near-match hint ("cyclinder → cylinder?") so a typo is a one-turn fix, + * not a silent permanent degradation. + * + * Style strings are merged structurally, not concatenated: each fragment is parsed to + * key=value tokens and later fragments override earlier ones per key. This is what + * makes shape and theme composable by rule — the shape fragment owns geometry keys + * (shape, perimeter, rounded…), the theme owns colour and type keys, and an overlap + * (a theme that says rounded=1 on a rhombus) resolves by order instead of emitting + * two conflicting tokens. + */ + +/** How a known shape renders and measures. */ +export interface ShapeSpec { + /** Geometry style tokens ONLY — no colours, no fonts; those belong to the theme. */ + style: string + /** + * How much larger than a rectangle the box must be for the same text to fit + * inside the outline. 1.0 for the rectangle family; ~1.5 for a rhombus, whose + * inscribed rectangle is half its bounding box. + */ + textScale: number + /** The label renders below the glyph, not inside it (umlActor and friends). */ + labelOutside?: boolean + /** Fixed glyph size for labelOutside shapes, which do not scale with text. */ + glyph?: { w: number; h: number } +} + +/** + * The curated catalog. Keys are the vocabulary the model is taught; several are + * semantic aliases for the same geometry (decision/diamond), because the model will + * reach for both names. + */ +export const SHAPE_CATALOG: Record = { + // ---- the rectangle family (the original six) ---- + box: { style: "rounded=0;", textScale: 1 }, + round: { style: "rounded=1;arcSize=12;", textScale: 1 }, + terminator: { style: "rounded=1;arcSize=50;", textScale: 1.15 }, + decision: { + style: "rhombus;perimeter=rhombusPerimeter;", + textScale: 1.5, + }, + diamond: { + style: "rhombus;perimeter=rhombusPerimeter;", + textScale: 1.5, + }, + data: { + style: "shape=parallelogram;perimeter=parallelogramPerimeter;fixedSize=1;size=14;", + textScale: 1.2, + }, + document: { style: "shape=document;boundedLbl=1;", textScale: 1.15 }, + + // ---- the semantic vocabulary (D2's tier: a node that IS a thing) ---- + /** A database or datastore. */ + cylinder: { + style: "shape=cylinder3;boundedLbl=1;backgroundOutline=1;size=12;", + textScale: 1.3, + }, + /** A message queue: a cylinder on its side. */ + queue: { + style: "shape=cylinder3;direction=south;boundedLbl=1;backgroundOutline=1;size=12;", + textScale: 1.3, + }, + /** An actor or user. Label below the figure. */ + person: { + style: "shape=umlActor;verticalLabelPosition=bottom;verticalAlign=top;outlineConnect=0;", + textScale: 1, + labelOutside: true, + glyph: { w: 40, h: 60 }, + }, + /** An external system, the internet. */ + cloud: { style: "ellipse;shape=cloud;", textScale: 1.6 }, + /** A service or process step. */ + hexagon: { + style: "shape=hexagon;perimeter=hexagonPerimeter2;fixedSize=1;size=16;", + textScale: 1.25, + }, + /** A concept, state or category. */ + ellipse: { style: "ellipse;", textScale: 1.3 }, + /** A speech-bubble annotation. */ + callout: { + style: "shape=callout;perimeter=calloutPerimeter;rounded=1;size=16;position=0.5;base=24;", + textScale: 1.35, + }, + /** A chevron stage in a pipeline. */ + step: { + style: "shape=step;perimeter=stepPerimeter;fixedSize=1;size=16;", + textScale: 1.2, + }, + /** A sticky note. */ + note: { style: "shape=note;size=14;", textScale: 1.1 }, + /** A card with a cut corner. */ + card: { style: "shape=card;size=14;", textScale: 1.1 }, + /** A process box with side bars (predefined subroutine). */ + process: { style: "shape=process;size=0.1;", textScale: 1.2 }, + /** Punched tape — legacy data, files. */ + tape: { style: "shape=tape;size=0.2;", textScale: 1.3 }, + /** A double-walled cube. */ + cube: { style: "shape=cube;size=12;", textScale: 1.25 }, +} + +/** A shape token that may pass through unrecognised: draw.io style-key charset only. */ +const SAFE_TOKEN = /^[a-zA-Z0-9._]+$/ + +export interface ResolvedShape { + spec: ShapeSpec + /** Set when the token was not in the catalog and passed through verbatim. */ + passthrough?: boolean +} + +/** + * Resolve a shape token: catalog entry, safe pass-through, or null for a token that + * could inject style keys (`;`/`=`/quotes) and must be rejected outright. + */ +export function resolveShape(token: string): ResolvedShape | null { + const known = SHAPE_CATALOG[token] + if (known) return { spec: known } + if (!SAFE_TOKEN.test(token)) return null + // Unknown but safe: emit verbatim. draw.io degrades an unregistered shape to a + // rectangle, so the worst case is a plain box — same as before the vocabulary + // existed. The conservative scale covers roughly-convex real shapes. + return { + spec: { style: `shape=${token};`, textScale: 1.25 }, + passthrough: true, + } +} + +/** The catalog key most similar to a token, for "did you mean" hints. */ +export function nearestShape(token: string): string | null { + const t = token.toLowerCase() + let best: string | null = null + let bestD = 3 // more than 2 edits away is not a typo + for (const key of Object.keys(SHAPE_CATALOG)) { + const d = editDistance(t, key.toLowerCase(), bestD) + if (d < bestD) { + bestD = d + best = key + } + } + return best +} + +/** Bounded Levenshtein distance; returns limit when the strings are further apart. */ +function editDistance(a: string, b: string, limit: number): number { + if (Math.abs(a.length - b.length) >= limit) return limit + const prev = new Array(b.length + 1) + for (let j = 0; j <= b.length; j++) prev[j] = j + for (let i = 1; i <= a.length; i++) { + let diag = prev[0] + prev[0] = i + let rowMin = prev[0] + for (let j = 1; j <= b.length; j++) { + const cur = Math.min( + prev[j] + 1, + prev[j - 1] + 1, + diag + (a[i - 1] === b[j - 1] ? 0 : 1), + ) + diag = prev[j] + prev[j] = cur + if (cur < rowMin) rowMin = cur + } + if (rowMin >= limit) return limit + } + return Math.min(prev[b.length], limit) +} + +// ---- structured style merge ---- + +/** + * Merge style fragments by key, later fragments winning. + * + * A draw.io style is `tok;key=value;key=value;` — bare class tokens (rhombus, ellipse, + * text) come first and key=value pairs follow. String concatenation made every + * conflict a duplicate key resolved by draw.io's last-wins rule, which worked until + * shape fragments and theme fragments both owned geometry keys (a theme's rounded=1 + * against a shape's rhombus). Merging structurally keeps exactly one token per key and + * one bare-token set, so the output is canonical and the ownership rule — theme owns + * colour and type, shape owns geometry — is enforced by fragment ORDER, not by hoping + * the keys never meet. + * + * Bare tokens are kept in first-appearance order, except that a later fragment's bare + * SHAPE CLASS (rhombus/ellipse/triangle) replaces an earlier one — two shape classes + * on one cell is a contradiction, not a union. + */ +export function mergeStyle(...fragments: (string | undefined)[]): string { + const bare: string[] = [] + const kv = new Map() + const SHAPE_CLASSES = new Set(["rhombus", "ellipse", "triangle"]) + for (const f of fragments) { + if (!f) continue + for (const tok of f.split(";")) { + if (tok === "") continue + const eq = tok.indexOf("=") + if (eq < 0) { + if (SHAPE_CLASSES.has(tok)) { + const i = bare.findIndex((b) => SHAPE_CLASSES.has(b)) + if (i >= 0) bare.splice(i, 1) + } + if (!bare.includes(tok)) bare.push(tok) + continue + } + const key = tok.slice(0, eq) + kv.set(key, tok.slice(eq + 1)) + // An explicit shape= also displaces a bare shape class from an earlier + // fragment — same contradiction as two bare classes. + if (key === "shape") { + const i = bare.findIndex((b) => SHAPE_CLASSES.has(b)) + if (i >= 0) bare.splice(i, 1) + } + } + } + let out = bare.join(";") + if (out) out += ";" + for (const [k, v] of kv) out += `${k}=${v};` + return out +} diff --git a/lib/diagram-engine/types.ts b/lib/diagram-engine/types.ts index 4504c4e..4933306 100644 --- a/lib/diagram-engine/types.ts +++ b/lib/diagram-engine/types.ts @@ -32,19 +32,14 @@ export interface PoolCell { export type Align = "start" | "center" | "end" | "stretch" /** - * The outline a flowchart box is drawn with. + * What a box IS, drawn as its conventional outline. * - * Flowchart notation is conventional, not decorative: a reader takes a diamond to mean a - * branch and a stadium to mean an entry or exit point. Rendering every step as the same - * rectangle throws that away. + * Open vocabulary: catalog names ("cylinder", "decision", "person"…) get full engine + * support — correct perimeter, text sized to fit the outline. Any other draw.io shape + * token passes through verbatim and degrades to a rectangle if the editor does not + * know it. See shapes.ts. */ -export type BoxShape = - | "box" - | "decision" - | "terminator" - | "round" - | "data" - | "document" +export type BoxShape = string /** A catalog icon: a real stencil, drawn at a fixed glyph size with a label below. */ export interface IconNode { diff --git a/lib/system-prompts.ts b/lib/system-prompts.ts index c84aec7..d6443c6 100644 --- a/lib/system-prompts.ts +++ b/lib/system-prompts.ts @@ -73,7 +73,7 @@ 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, group?: string}> + nodes: Array<{id: string, label: string, shape?: string, 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 @@ -166,10 +166,15 @@ Mind maps and org charts (add_radial): - spread: "radial" for a mind map (branches on both sides, compact). "down" for an org chart (everything below its manager, which is the only way a reporting line reads correctly). -Flowchart box shapes, for both draw_graph and add_box: -- "decision" for a branch (a diamond), "terminator" for a start or end point, "data" for input or - output, "document" for a report, "round" for a soft-edged step. Use them: a reader takes a - diamond to mean a choice, so drawing every step as the same rectangle loses that. +Box shapes, for both draw_graph and add_box — a shape says what a node IS: +- Flowchart: "decision" (a diamond) for a branch, "terminator" for a start or end point, "data" + for input or output, "document" for a report, "round" for a soft-edged step. +- Semantic: "cylinder" for a database, "queue" for a message queue, "person" for an actor or + user, "cloud" for an external system, "hexagon" for a service, "ellipse" for a concept, + "callout" for a note, "step" for a pipeline stage, "note", "card", "process", "tape", "cube". +- Any other draw.io shape token also works verbatim (unknown ones render as rectangles). + Use shapes: a database drawn as a cylinder needs no "database" caption; a reader takes a + diamond to mean a choice. Drawing everything as the same rectangle throws that away. Core capabilities: - Create professional flowcharts, mind maps, entity diagrams, and technical illustrations diff --git a/tests/unit/diagram-engine-shapes.test.ts b/tests/unit/diagram-engine-shapes.test.ts new file mode 100644 index 0000000..be03552 --- /dev/null +++ b/tests/unit/diagram-engine-shapes.test.ts @@ -0,0 +1,196 @@ +import { describe, expect, it } from "vitest" +import { restructureDiagram } from "@/lib/diagram-engine" +import { autoBoxSize } from "@/lib/diagram-engine/layout" +import { parseDiagram } from "@/lib/diagram-engine/parse" +import { + mergeStyle, + nearestShape, + resolveShape, +} from "@/lib/diagram-engine/shapes" + +/** + * The open shape vocabulary: catalog shapes fully understood, anything else passed + * through. What these tests pin down is not the happy path but the failure modes the + * design review flagged: silent degradation without feedback, round-trips freezing + * measurements, appearance-based reverse mapping dropping declarations, and string + * concatenation emitting contradictory style keys. + */ + +const styleOf = (xml: string, id: string): string => + xml.match(new RegExp(`id="${id}"[^>]*style="([^"]*)"`))?.[1] ?? "" + +describe("style merge ownership", () => { + it("keeps one token per key, later fragments winning", () => { + const out = mergeStyle( + "rounded=0;fillColor=#FFF;", + "rounded=1;arcSize=50;", + ) + expect(out.match(/rounded=/g)).toHaveLength(1) + expect(out).toContain("rounded=1") + expect(out).toContain("fillColor=#FFF") + }) + it("a later shape class displaces an earlier bare class", () => { + expect(mergeStyle("rhombus;", "ellipse;")).not.toContain("rhombus") + // and an explicit shape= displaces a bare class too + const out = mergeStyle("rhombus;", "shape=cloud;") + expect(out).not.toContain("rhombus") + expect(out).toContain("shape=cloud") + }) +}) + +describe("shape resolution", () => { + it("catalog shapes carry their perimeter", () => { + expect(resolveShape("decision")?.spec.style).toContain( + "perimeter=rhombusPerimeter", + ) + expect(resolveShape("hexagon")?.spec.style).toContain( + "perimeter=hexagonPerimeter2", + ) + }) + it("unknown-but-safe tokens pass through; injection-capable ones are rejected", () => { + const pass = resolveShape("mxgraph.flowchart.or") + expect(pass?.passthrough).toBe(true) + expect(pass?.spec.style).toBe("shape=mxgraph.flowchart.or;") + expect(resolveShape("x;fillColor=red")).toBeNull() + expect(resolveShape("a=b")).toBeNull() + }) + it("suggests the nearest catalog name for a typo", () => { + expect(nearestShape("cyclinder")).toBe("cylinder") + expect(nearestShape("hexgon")).toBe("hexagon") + expect(nearestShape("zzzzzz")).toBeNull() + }) +}) + +describe("engine integration", () => { + it("renders a catalog shape with theme colours composed on top", () => { + const r = restructureDiagram("", [ + { + op: "add_box", + id: "db", + label: "users", + shape: "cylinder", + group: "storage", + }, + ]) + expect(r.errors).toEqual([]) + const s = styleOf(r.xml as string, "db") + expect(s).toContain("shape=cylinder3") + // theme owns colour: the group hue's tint, not the fallback white + expect(s).toContain("fillColor=#DAE8FC") + // exactly one fillColor — structured merge, not concatenation + expect(s.match(/fillColor=/g)).toHaveLength(1) + }) + + it("warns (not errors) on a pass-through token, with a near-match hint", () => { + const r = restructureDiagram("", [ + { op: "add_box", id: "a", label: "x", shape: "cyclinder" }, + ]) + expect(r.errors).toEqual([]) + expect(r.xml).toBeTruthy() + expect(r.warnings.join(" ")).toContain('Did you mean "cylinder"?') + }) + + it("rejects an injection-capable token as an error", () => { + const r = restructureDiagram("", [ + { + op: "add_box", + id: "a", + label: "x", + shape: "box;container=1", + }, + ]) + expect(r.errors.join(" ")).toContain("not allowed") + expect(r.xml).toBeNull() + }) + + it("sizes a decision box larger than a plain box for the same text", () => { + const text = "Is the request authorized to proceed?" + const plain = autoBoxSize(text) + const rhombus = autoBoxSize(text, undefined, "decision") + expect(rhombus.w).toBeGreaterThan(plain.w * 1.3) + expect(rhombus.h).toBeGreaterThan(plain.h * 1.3) + }) +}) + +describe("round trip", () => { + it("carries the declared token back via dai_shape, aliases intact", () => { + const r = restructureDiagram("", [ + { op: "add_box", id: "d", label: "choice?", shape: "diamond" }, + { op: "add_box", id: "q", label: "jobs", shape: "queue" }, + ]) + const back = parseDiagram(r.xml as string) + const d = back.tree.roots.find((n) => n.id === "d") + const q = back.tree.roots.find((n) => n.id === "q") + // "diamond" must not come back as "decision" — the declaration survives. + expect(d?.kind === "box" && d.shape).toBe("diamond") + // "queue" is a rotated cylinder; appearance alone cannot tell them apart. + expect(q?.kind === "box" && q.shape).toBe("queue") + }) + + it("re-measures on label change instead of freezing the first layout's size", () => { + const r1 = restructureDiagram("", [ + { op: "add_box", id: "a", label: "hi" }, + ]) + const r2 = restructureDiagram(r1.xml as string, [ + { + op: "set_label", + id: "a", + label: "a much longer label that plainly needs a wider box to fit", + }, + ]) + const w = (xml: string) => + Number( + xml.match( + /id="a"[^>]*>\s*]*width="([\d.]+)"/, + )?.[1], + ) + expect(w(r2.xml as string)).toBeGreaterThan(w(r1.xml as string)) + }) + + it("a box with an mxgraph.* token stays a box, keeping role and group", () => { + const r = restructureDiagram("", [ + { + op: "add_box", + id: "b", + label: "x", + shape: "mxgraph.flowchart.or", + role: "callout", + group: "z1", + }, + ]) + const back = parseDiagram(r.xml as string) + const b = back.tree.roots.find((n) => n.id === "b") + expect(b?.kind).toBe("box") + if (b?.kind !== "box") return + expect(b.shape).toBe("mxgraph.flowchart.or") + expect(b.role).toBe("callout") + expect(b.group).toBe("z1") + }) +}) + +describe("set_shape / set_role / set_group", () => { + it("changes shape in place, dropping the stale style and size", () => { + const r1 = restructureDiagram("", [ + { op: "add_box", id: "a", label: "db" }, + ]) + const r2 = restructureDiagram(r1.xml as string, [ + { op: "set_shape", id: "a", shape: "cylinder" }, + ]) + expect(r2.errors).toEqual([]) + expect(styleOf(r2.xml as string, "a")).toContain("shape=cylinder3") + }) + it("set_role and set_group restyle without losing the label", () => { + const r1 = restructureDiagram("", [ + { op: "add_box", id: "a", label: "warn" }, + ]) + const r2 = restructureDiagram(r1.xml as string, [ + { op: "set_role", id: "a", role: "bad" }, + { op: "set_group", id: "a", group: "zone1" }, + ]) + expect(r2.errors).toEqual([]) + const s = styleOf(r2.xml as string, "a") + expect(s).toContain("dai_role=bad") + expect(s).toContain("dai_group=zone1") + expect(r2.xml).toContain('value="warn"') + }) +})