From 5165f43803590503aa7a0746a10369d6f3909d8f Mon Sep 17 00:00:00 2001 From: anandpant <109482096+anandpant@users.noreply.github.com> Date: Fri, 23 Oct 2026 21:30:51 +0000 Subject: [PATCH] fix(mcp): draw the logo a group's label names, before its name, when the diagram already draws logos (#537) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What MCP flowchart builds now draw the logo a group's label names, before the group's name, when the diagram already draws logos. In Frontierclimb r23 (Sonnet, ai-app-stack-logos s4), the model drew Next.js, Redis, OpenAI and Sentry logos but left its "Vercel" group a bare name. That group now draws the Vercel mark. ## Rule Same terms as #519's node fill, MCP `execute` only (`fillNamedLogos`): - **Fills** a group whose label names exactly one catalog logo, distinctively (`logoNamedByLabel`). - **Never fills:** - a diagram whose nodes draw no logo; - labels naming several products or an everyday word ("GKE cluster (Kubernetes)", "Frontend", "Edge"); - a group whose logo a node inside it already draws, nested groups included; - a group inside another group that already draws that logo (the outer one keeps it); - groups that hold no nodes and so aren't drawn. - Reports an `icon_filled` info note with `ref: { kind: "group", id }`. **Why the no-duplicate rule covers nodes inside the group only:** a header logo that repeats a member's logo says nothing new inside the same box. A node outside the group with the same logo names its own product, so it doesn't block the fill. Across the corpus, no fill shares its slug with any drawn node anyway. ## Rendering Groups couldn't carry a logo anywhere before this PR (IR, renderer, scene, Excalidraw or PNG). This PR adds the smallest header logo consistent with node logos, 18 px: - **Frame (plain group):** inside the frame's top-left corner, 10 px in, under the frame's name. It is a frame child (`frameId` = the frame), so it moves with the frame in Excalidraw and Studio and isn't clipped. A frame's padding is at least 40 px (`GROUP_PADDING`), so members always clear the logo. The padded logo is a keep-out alongside the frame's name for routes, branch labels and marker room. - **Zone:** at the start of the header strip, with the label shifted after it. The zone widens to fit both. The logo shares the zone outline's `groupIds`, so the two move together. ## API and model-facing surface - `DiagramGroup.icon` (core IR) and the canvas frame `icon` are new. A zone (`rendererRole: "zone"`) may now carry an `icon`. - The `FlowchartSpec` group contract, guidance and prompts are unchanged; groups take no authored logo. - **One generated-types line changes:** the `CanvasElement` frame member gains `icon?: { slug; size }`. The fill has to survive the scene schema: the artifact viewer, patches, CLI and Studio all decode stored scenes through `FrameSceneElement`, which drops unknown keys. That text appears only in docs topics. - Generation drops any `icon` a model puts on a group, so `/api/v1/generate` stays unchanged. ## Measured Oct 17–23 Frontierclimb corpus: 5,434 specs replayed through Code Mode with MCP fill on, main against this branch. - **Byte-identical:** 5,073 of 5,434 in scene and Excalidraw. All 361 changed specs gained a group logo; nothing else changed. - **Fills:** 361, all checked by hand, every one on a diagram that already draws node logos (0 on logo-less diagrams): - "AWS cloud" → `aws` (208, hybrid-cloud zones) - "GKE cluster" → `gke` (147, kubernetes-cluster zones) - "Vercel" → `vercel` (5, ai-app-stack frames, including r23 s4) - "AWS · us-east-1" → `aws` (1, top-level region) - **Collisions:** none between a logo and a node, frame, route or text. No drawn node repeats a group's logo. - **Placement:** all 5 frame logos have their frame's `frameId` and sit inside it; all 356 zone logos sit inside their zone and share its group. - **PNGs inspected** for all four pairs; main's PNGs are identical apart from the logo. ## Proof - Failing-first tests: - agent: frame and zone fills, the r23 s4 spec, and negatives for logo-less diagrams, multi-vendor and everyday-word labels, duplicates and host opt-in; - core: embedding, validation and header geometry; - generation: drops group icons. - `pnpm nx affected -t typecheck,test,build`, `pnpm run check` and `build-storybook diagram-ui` pass. New `GroupLogo` and `ZoneLogo` stories. --- .../mcp-docs/code-mode-types.generated.txt | 2 +- .../agent/src/lib/code-mode/contract.ts | 1 + .../agent/src/lib/code-mode/icons.test.ts | 257 ++++++++++++++++++ .../diagram/agent/src/lib/code-mode/icons.ts | 81 +++++- .../agent/src/lib/code-mode/runtime.ts | 32 ++- .../diagram/agent/src/lib/flowchart/spec.ts | 14 +- .../diagram/core/src/canvas-icons.test.ts | 38 ++- packages/diagram/core/src/canvas.ts | 44 +-- .../diagram/core/src/group-regions.test.ts | 26 +- packages/diagram/core/src/group-regions.ts | 54 +++- packages/diagram/core/src/intermediate.ts | 2 + packages/diagram/core/src/types/flowchart.ts | 46 ++++ packages/diagram/excalidraw/README.md | 5 + .../diagram/excalidraw/src/lib/convert.ts | 36 ++- .../diagram/generation/src/lib/candidates.ts | 11 + .../diagram/generation/src/lib/zones.test.ts | 21 ++ packages/diagram/renderer/src/scene.ts | 68 +++-- .../excalidraw-scene-canvas.stories.tsx | 24 +- .../node-logo-scene.fixtures.ts | 25 ++ 19 files changed, 726 insertions(+), 61 deletions(-) diff --git a/apps/playground/src/server/codemode/mcp-docs/code-mode-types.generated.txt b/apps/playground/src/server/codemode/mcp-docs/code-mode-types.generated.txt index be2e8922..e69a2ce5 100644 --- a/apps/playground/src/server/codemode/mcp-docs/code-mode-types.generated.txt +++ b/apps/playground/src/server/codemode/mcp-docs/code-mode-types.generated.txt @@ -34,7 +34,7 @@ type PartialArtifact = { artifactId?: string; diagramId?: string; formats?: Arra type DiagramSelector = { ids?: Array; nodeIds?: Array; edgeIds?: Array; kinds?: Array<"start" | "process" | "decision" | "end">; labels?: Array; scope?: "all" | "nodes" | "edges"; }; -type CanvasElement = { frameId?: string; groupIds?: Array; layerId?: string; locked?: boolean; opacity?: number; zIndex?: number; fillColor?: string; fillStyle?: "cross-hatch" | "hachure" | "solid"; roughness?: 0 | 1 | 2; strokeColor?: string; strokeStyle?: "dashed" | "dotted" | "solid"; strokeWidth?: 1 | 2 | 4; type: "node"; id: string; nodeId: string; kind?: string; icon?: { slug: string; size: number; }; rendererRole?: "zone" | "sequence-lifeline" | "sequence-activation" | "timeline-marker" | "chart-mark" | "state-marker" | "gantt-mark" | "er-table" | "er-optional" | "class-box" | "swimlane" | "swimlane-header"; shape: "rectangle" | "ellipse" | "diamond" | "circle" | "polygon" | "cylinder" | "pipe" | "document" | "bucket"; points?: Array<{ x: number; y: number; }>; textColor?: string; x: number; y: number; width: number; height: number; label: string; } | { frameId?: string; groupIds?: Array; layerId?: string; locked?: boolean; opacity?: number; zIndex?: number; type: "text"; id: string; containerId?: string; textColor?: string; x: number; y: number; text: string; fontSize: number; fontFamily?: "hand" | "mono" | "sans"; lineHeight?: number; maxWidth?: number; textAlign?: "center" | "left" | "right"; verticalAlign?: "bottom" | "middle" | "top"; } | { frameId?: string; groupIds?: Array; layerId?: string; locked?: boolean; opacity?: number; zIndex?: number; fillColor?: string; fillStyle?: "cross-hatch" | "hachure" | "solid"; roughness?: 0 | 1 | 2; strokeColor?: string; strokeStyle?: "dashed" | "dotted" | "solid"; strokeWidth?: 1 | 2 | 4; type: "arrow"; id: string; edgeId: string; sourceNodeId: string; targetNodeId: string; startArrowhead?: "arrow" | "bar" | "circle" | "diamond" | "triangle" | "crowfoot_one" | "crowfoot_many" | "crowfoot_one_or_many" | "triangle_outline" | "diamond_outline" | null; endArrowhead?: "arrow" | "bar" | "circle" | "diamond" | "triangle" | "crowfoot_one" | "crowfoot_many" | "crowfoot_one_or_many" | "triangle_outline" | "diamond_outline" | null; textColor?: string; points: Array<{ x: number; y: number; }>; curved?: boolean; label?: string; } | { frameId?: string; groupIds?: Array; layerId?: string; locked?: boolean; opacity?: number; zIndex?: number; fillColor?: string; fillStyle?: "cross-hatch" | "hachure" | "solid"; roughness?: 0 | 1 | 2; strokeColor?: string; strokeStyle?: "dashed" | "dotted" | "solid"; strokeWidth?: 1 | 2 | 4; type: "line"; id: string; points: Array<{ x: number; y: number; }>; startBinding?: { elementId: string; focus?: number; gap?: number; }; endBinding?: { elementId: string; focus?: number; gap?: number; }; startArrowhead?: "arrow" | "bar" | "circle" | "diamond" | "triangle" | "crowfoot_one" | "crowfoot_many" | "crowfoot_one_or_many" | "triangle_outline" | "diamond_outline" | null; endArrowhead?: "arrow" | "bar" | "circle" | "diamond" | "triangle" | "crowfoot_one" | "crowfoot_many" | "crowfoot_one_or_many" | "triangle_outline" | "diamond_outline" | null; label?: string; textColor?: string; } | { frameId?: string; groupIds?: Array; layerId?: string; locked?: boolean; opacity?: number; zIndex?: number; fillColor?: string; fillStyle?: "cross-hatch" | "hachure" | "solid"; roughness?: 0 | 1 | 2; strokeColor?: string; strokeStyle?: "dashed" | "dotted" | "solid"; strokeWidth?: 1 | 2 | 4; type: "frame"; id: string; nodeId?: string; name?: string; x: number; y: number; width: number; height: number; }; +type CanvasElement = { frameId?: string; groupIds?: Array; layerId?: string; locked?: boolean; opacity?: number; zIndex?: number; fillColor?: string; fillStyle?: "cross-hatch" | "hachure" | "solid"; roughness?: 0 | 1 | 2; strokeColor?: string; strokeStyle?: "dashed" | "dotted" | "solid"; strokeWidth?: 1 | 2 | 4; type: "node"; id: string; nodeId: string; kind?: string; icon?: { slug: string; size: number; }; rendererRole?: "zone" | "sequence-lifeline" | "sequence-activation" | "timeline-marker" | "chart-mark" | "state-marker" | "gantt-mark" | "er-table" | "er-optional" | "class-box" | "swimlane" | "swimlane-header"; shape: "rectangle" | "ellipse" | "diamond" | "circle" | "polygon" | "cylinder" | "pipe" | "document" | "bucket"; points?: Array<{ x: number; y: number; }>; textColor?: string; x: number; y: number; width: number; height: number; label: string; } | { frameId?: string; groupIds?: Array; layerId?: string; locked?: boolean; opacity?: number; zIndex?: number; type: "text"; id: string; containerId?: string; textColor?: string; x: number; y: number; text: string; fontSize: number; fontFamily?: "hand" | "mono" | "sans"; lineHeight?: number; maxWidth?: number; textAlign?: "center" | "left" | "right"; verticalAlign?: "bottom" | "middle" | "top"; } | { frameId?: string; groupIds?: Array; layerId?: string; locked?: boolean; opacity?: number; zIndex?: number; fillColor?: string; fillStyle?: "cross-hatch" | "hachure" | "solid"; roughness?: 0 | 1 | 2; strokeColor?: string; strokeStyle?: "dashed" | "dotted" | "solid"; strokeWidth?: 1 | 2 | 4; type: "arrow"; id: string; edgeId: string; sourceNodeId: string; targetNodeId: string; startArrowhead?: "arrow" | "bar" | "circle" | "diamond" | "triangle" | "crowfoot_one" | "crowfoot_many" | "crowfoot_one_or_many" | "triangle_outline" | "diamond_outline" | null; endArrowhead?: "arrow" | "bar" | "circle" | "diamond" | "triangle" | "crowfoot_one" | "crowfoot_many" | "crowfoot_one_or_many" | "triangle_outline" | "diamond_outline" | null; textColor?: string; points: Array<{ x: number; y: number; }>; curved?: boolean; label?: string; } | { frameId?: string; groupIds?: Array; layerId?: string; locked?: boolean; opacity?: number; zIndex?: number; fillColor?: string; fillStyle?: "cross-hatch" | "hachure" | "solid"; roughness?: 0 | 1 | 2; strokeColor?: string; strokeStyle?: "dashed" | "dotted" | "solid"; strokeWidth?: 1 | 2 | 4; type: "line"; id: string; points: Array<{ x: number; y: number; }>; startBinding?: { elementId: string; focus?: number; gap?: number; }; endBinding?: { elementId: string; focus?: number; gap?: number; }; startArrowhead?: "arrow" | "bar" | "circle" | "diamond" | "triangle" | "crowfoot_one" | "crowfoot_many" | "crowfoot_one_or_many" | "triangle_outline" | "diamond_outline" | null; endArrowhead?: "arrow" | "bar" | "circle" | "diamond" | "triangle" | "crowfoot_one" | "crowfoot_many" | "crowfoot_one_or_many" | "triangle_outline" | "diamond_outline" | null; label?: string; textColor?: string; } | { frameId?: string; groupIds?: Array; layerId?: string; locked?: boolean; opacity?: number; zIndex?: number; fillColor?: string; fillStyle?: "cross-hatch" | "hachure" | "solid"; roughness?: 0 | 1 | 2; strokeColor?: string; strokeStyle?: "dashed" | "dotted" | "solid"; strokeWidth?: 1 | 2 | 4; type: "frame"; id: string; nodeId?: string; name?: string; icon?: { slug: string; size: number; }; x: number; y: number; width: number; height: number; }; type CanvasSpec = { kind: "canvas"; version: number; diagramId: string; title: string; width: number; height: number; accentColor: string; backgroundColor: string; elements: Array; icons?: { [key: string]: { name: string; svg: string; }; }; layers: Array<{ id: string; name?: string; locked?: boolean; visible?: boolean; }>; layouts: Array<{ type: "row" | "column" | "stack"; ids: Array; x?: number; y?: number; gap?: number; } | { type: "grid"; ids: Array; columns: number; x?: number; y?: number; columnGap?: number; rowGap?: number; } | { type: "align"; ids: Array; axis: "x" | "y"; alignment: "center" | "end" | "start"; } | { type: "distribute"; ids: Array; axis: "x" | "y"; gap?: number; }>; zOrder: Array; }; diff --git a/packages/diagram/agent/src/lib/code-mode/contract.ts b/packages/diagram/agent/src/lib/code-mode/contract.ts index f7d2f150..d49a3a62 100644 --- a/packages/diagram/agent/src/lib/code-mode/contract.ts +++ b/packages/diagram/agent/src/lib/code-mode/contract.ts @@ -1595,6 +1595,7 @@ export class FrameSceneElement extends Schema.Class("FrameSce id: RequiredNonEmptyString, nodeId: optionalContract(NonEmptyString), name: optionalContract(NonEmptyString).pipe(Schema.mutableKey), + icon: optionalContract(CanvasNodeIcon).pipe(Schema.mutableKey), x: FiniteNumber.pipe(Schema.mutableKey), y: FiniteNumber.pipe(Schema.mutableKey), width: PositiveNumber.pipe(Schema.mutableKey), diff --git a/packages/diagram/agent/src/lib/code-mode/icons.test.ts b/packages/diagram/agent/src/lib/code-mode/icons.test.ts index bb5b2ea6..300b9db2 100644 --- a/packages/diagram/agent/src/lib/code-mode/icons.test.ts +++ b/packages/diagram/agent/src/lib/code-mode/icons.test.ts @@ -586,6 +586,263 @@ describe("named logo fill", () => { }), ]); }); + + describe("flowchart groups", () => { + // Frontierclimb r23 ai-app-stack-logos s4 (Sonnet): the model drew Next.js, + // Redis, OpenAI and Sentry logos but left its "Vercel" group a bare name. + const r23AppSpec = { + title: "Request flow", + nodes: [ + { + id: "page", + label: "Next.js page receives the question", + kind: "start", + icon: { slug: "nextjs" }, + group: "vercel", + }, + { + id: "check", + label: "Check Redis cache", + kind: "process", + icon: { slug: "redis" }, + group: "vercel", + }, + { id: "hit", label: "Cache hit?", kind: "decision", group: "vercel" }, + { id: "call", label: "Call OpenAI model", kind: "process", icon: { slug: "openai" } }, + { id: "store", label: "Store answer in Redis", kind: "process", icon: { slug: "redis" } }, + { id: "log", label: "Log any errors to Sentry", kind: "process", icon: { slug: "sentry" } }, + { id: "ret", label: "Return the answer", kind: "end" }, + { id: "cached", label: "Return cached answer", kind: "end" }, + ], + edges: [ + { source: "page", target: "check" }, + { source: "check", target: "hit" }, + { source: "hit", target: "cached", label: "yes" }, + { source: "hit", target: "call", label: "no" }, + { source: "call", target: "store" }, + { source: "store", target: "log" }, + { source: "log", target: "ret" }, + ], + groups: [{ id: "vercel", label: "Vercel" }], + layout: { direction: "TB" }, + }; + const options = { + artifactFormats: ["scene", "excalidraw"], + inlineArtifacts: ["scene", "excalidraw"], + }; + + /** Each drawn group's logo slug by group id, from the scene's frames and zones. */ + function groupLogos(result: Awaited["buildFlowchart"]>>) { + if (!result.ok) throw new Error(JSON.stringify(result.issues)); + const scene = inlineScene(result); + return Object.fromEntries( + scene.elements.flatMap((element) => + (element.type === "frame" || + (element.type === "node" && element.rendererRole === "zone")) && + element.icon + ? [[element.id.slice("group:".length), element.icon.slug]] + : [], + ), + ); + } + + /** A logo-drawing chain of nodes inside the given groups. */ + function grouped( + groups: ReadonlyArray<{ id: string; label: string; parent?: string; kind?: string }>, + nodes: ReadonlyArray, + ) { + const spec = chain( + nodes.map(([id, label, slug]) => (slug ? ([id, label, slug] as const) : [id, label])), + ); + return { + ...spec, + nodes: spec.nodes.map((node, index) => { + const group = nodes[index]?.[3]; + return group ? { ...node, group } : node; + }), + groups, + }; + } + + it("draws the logo a group's label names before its name, with an info note", async () => { + const result = await filling().buildFlowchart({ spec: r23AppSpec, options }); + + expect(groupLogos(result)).toEqual({ vercel: "vercel" }); + expect(result.issues).toEqual([ + { + code: "icon_filled", + severity: "info", + stage: "input", + ref: { kind: "group", id: "vercel" }, + message: + 'Group "vercel" draws the "vercel" logo because its label "Vercel" names Vercel.', + hint: "No action needed.", + }, + ]); + if (!result.ok) return; + // The spec the model wrote stays as written: groups take no authored logo. + expect(result.normalizedSpec.groups).toEqual([{ id: "vercel", label: "Vercel" }]); + const scene = inlineScene(result); + expect(Object.keys(scene.icons ?? {})).toContain("vercel"); + const frame = scene.elements.find((element) => element.id === "group:vercel"); + const excalidraw = result.artifact.formats.find((format) => format.format === "excalidraw") + ?.inline as { + elements: Array<{ + customData?: Record; + frameId: string | null; + groupIds: string[]; + type: string; + x: number; + y: number; + width: number; + }>; + }; + const logo = excalidraw.elements.find( + (element) => element.customData?.["sketchiGroupIcon"] === "vercel", + ); + // Inside the frame's top-left corner, one of its children, so it moves with the frame. + expect(frame?.type).toBe("frame"); + if (frame?.type !== "frame") return; + expect(logo).toMatchObject({ + type: "image", + frameId: "group:vercel", + groupIds: [], + x: frame.x + 10, + y: frame.y + 10, + width: 18, + }); + // Members and routes keep clear of it. + const keepOut = { x: frame.x + 6, y: frame.y + 6, width: 26, height: 26 }; + for (const element of scene.elements) { + if (element.type === "node") + expect( + element.x >= keepOut.x + keepOut.width || element.y >= keepOut.y + keepOut.height, + ).toBe(true); + if (element.type === "arrow") + for (const point of element.points) + expect( + point.x >= keepOut.x + keepOut.width || + point.y >= keepOut.y + keepOut.height || + point.x <= keepOut.x || + point.y <= keepOut.y, + ).toBe(true); + } + }); + + it("draws a zone's logo in its header, its label after it", async () => { + const spec = { + ...r23AppSpec, + groups: [{ id: "vercel", label: "Vercel", kind: "cloud" }], + }; + const result = await filling().buildFlowchart({ spec, options }); + + expect(groupLogos(result)).toEqual({ vercel: "vercel" }); + if (!result.ok) return; + const scene = inlineScene(result); + const zone = scene.elements.find((element) => element.id === "group:vercel"); + const label = scene.elements.find((element) => element.id === "zone-label:vercel"); + if (zone?.type !== "node" || label?.type !== "text") throw new Error("Expected a zone."); + const excalidraw = result.artifact.formats.find((format) => format.format === "excalidraw") + ?.inline as { + elements: Array<{ + customData?: Record; + groupIds: string[]; + id: string; + x: number; + y: number; + }>; + }; + const logo = excalidraw.elements.find( + (element) => element.customData?.["sketchiGroupIcon"] === "vercel", + ); + expect(logo).toMatchObject({ x: zone.x + 12, y: zone.y + 9 }); + // It shares the zone's group, so the two move together. + const outline = excalidraw.elements.find((element) => element.id === "group:vercel"); + expect(outline?.groupIds[0]).toBeDefined(); + expect(logo?.groupIds).toEqual(outline?.groupIds); + // The label starts after the logo and its gap. + const plain = await filling().buildFlowchart({ + spec: { ...spec, nodes: spec.nodes.map(({ icon: _icon, ...node }) => node) }, + options, + }); + if (!plain.ok) throw new Error(JSON.stringify(plain.issues)); + const plainLabel = inlineScene(plain).elements.find( + (element) => element.id === "zone-label:vercel", + ); + const plainZone = inlineScene(plain).elements.find( + (element) => element.id === "group:vercel", + ); + if (plainLabel?.type !== "text" || plainZone?.type !== "node") throw new Error("No zone."); + expect(label.x - zone.x - (plainLabel.x - plainZone.x)).toBe(24); + }); + + it("never fills a group in a diagram that draws no logo", async () => { + const spec = { + ...r23AppSpec, + nodes: r23AppSpec.nodes.map(({ icon: _icon, ...node }) => node), + }; + const result = await filling().buildFlowchart({ spec, options }); + + expect(groupLogos(result)).toEqual({}); + expect(result.issues).toEqual([]); + }); + + it("never fills a group whose label names several products or an everyday word", async () => { + const result = await filling().buildFlowchart({ + spec: grouped( + [ + { id: "both", label: "Vercel and Cloudflare" }, + { id: "front", label: "Frontend" }, + { id: "edge", label: "Edge" }, + { id: "stream", label: "Stream processing" }, + ], + [ + ["push", "Push to GitHub", "github", "both"], + ["page", "Render the page", undefined, "front"], + ["route", "Route the request", undefined, "edge"], + ["events", "Process events", undefined, "stream"], + ], + ), + options, + }); + + expect(groupLogos(result)).toEqual({}); + expect(result.issues).toEqual([]); + }); + + it("never repeats a logo a node inside the group, or a group around it, already draws", async () => { + const result = await filling().buildFlowchart({ + spec: grouped( + [ + { id: "cf", label: "Cloudflare", kind: "cloud" }, + { id: "aws", label: "AWS", kind: "cloud" }, + { id: "region", label: "AWS us-east-1", parent: "aws", kind: "region" }, + ], + [ + ["push", "Push to GitHub", "github"], + ["worker", "Cloudflare Workers", "cloudflare", "cf"], + ["queue", "Enqueue the job", undefined, "region"], + ], + ), + options, + }); + + expect(groupLogos(result)).toEqual({ aws: "aws" }); + expect(result.issues.map((issue) => [issue.code, issue.ref?.id])).toEqual([ + ["icon_filled", "aws"], + ]); + }); + + it("only fills when the host asks, as MCP builds do", async () => { + const result = await runtime(productionCatalog).buildFlowchart({ + spec: r23AppSpec, + options, + }); + + expect(groupLogos(result)).toEqual({}); + expect(result.issues).toEqual([]); + }); + }); }); describe("createCanvas icons", () => { diff --git a/packages/diagram/agent/src/lib/code-mode/icons.ts b/packages/diagram/agent/src/lib/code-mode/icons.ts index 7892f3ea..177bfcd0 100644 --- a/packages/diagram/agent/src/lib/code-mode/icons.ts +++ b/packages/diagram/agent/src/lib/code-mode/icons.ts @@ -234,6 +234,68 @@ export function resolveNodeIcons< return { issues, nodes: filled }; } +/** + * With fillNamedLogos, once a flowchart's nodes draw a logo, each group whose + * label unmistakably names one ("Vercel") draws it before that label, reported + * as an icon_filled note, on the same terms as resolveNodeIcons fills a node. + * Groups take no authored logo, so this is the only way one gets a logo. A + * group skips a logo a node inside it already draws, or a group around it: + * the group's header would only repeat it. Only groups that hold nodes draw. + */ +export function fillGroupLogos( + groups: + | readonly { readonly id: string; readonly label: string; readonly parent?: string }[] + | undefined, + /** Nodes as resolveNodeIcons left them. */ + nodes: readonly { readonly group?: string; readonly icon?: { readonly slug: string } }[], + catalog: CodeModeIconCatalog | undefined, + policy: { readonly fillNamedLogos?: boolean } = {}, +): { readonly issues: CodeModeIssue[]; readonly logos: ReadonlyMap } { + const logos = new Map(); + const issues: CodeModeIssue[] = []; + if (!groups?.length || !policy.fillNamedLogos || !catalog || !nodes.some((node) => node.icon)) { + return { issues, logos }; + } + const parentOf = new Map( + groups.flatMap((group) => (group.parent === undefined ? [] : [[group.id, group.parent]])), + ); + // A group and every group around it, innermost first. + const chain = (id: string | undefined) => { + const ids: string[] = []; + for (let group = id; group !== undefined && !ids.includes(group); group = parentOf.get(group)) + ids.push(group); + return ids; + }; + // The logos the nodes in each group draw, nested groups' nodes included. + const inside = new Map>(); + for (const node of nodes) { + for (const id of chain(node.group)) { + const slugs = inside.get(id) ?? new Set(); + if (node.icon) slugs.add(node.icon.slug); + inside.set(id, slugs); + } + } + // Outermost first, so a group around another keeps the logo they share. + const depth = (id: string) => chain(id).length; + for (const group of [...groups].sort((left, right) => depth(left.id) - depth(right.id))) { + const held = inside.get(group.id); + if (!held) continue; + const icon = catalog.logoNamedByLabel(group.label); + if (!icon || held.has(icon.slug)) continue; + if (chain(group.parent).some((id) => logos.get(id) === icon.slug)) continue; + logos.set(group.id, icon.slug); + issues.push({ + code: "icon_filled", + severity: "info", + stage: "input", + ref: { kind: "group", id: group.id }, + message: `Group "${group.id}" draws the "${icon.slug}" logo because its label "${group.label}" names ${icon.name}.`, + hint: "No action needed.", + }); + } + return { issues, logos }; +} + function droppedIconIssue(input: { readonly catalog: CodeModeIconCatalog | undefined; readonly dropped: DroppedCanvasIcon; @@ -246,11 +308,16 @@ function droppedIconIssue(input: { id: dropped.elementId, path: `elements[${dropped.index}].icon${dropped.reason === "unavailable" ? ".slug" : ""}`, }; - const subject = `Icon "${dropped.slug}" on node "${dropped.elementId}"`; + const { element } = input; + const owner = + element?.type === "frame" || (element?.type === "node" && element.rendererRole === "zone") + ? "group" + : "node"; + const subject = `Icon "${dropped.slug}" on ${owner} "${dropped.elementId}"`; if (dropped.reason === "unavailable" && !input.known) { return unknownIconIssue({ catalog: input.catalog, - message: `${subject} is not a Sketchi node logo; the node renders without a logo.`, + message: `${subject} is not a Sketchi node logo; the ${owner} renders without a logo.`, ref, slug: dropped.slug, }); @@ -258,7 +325,7 @@ function droppedIconIssue(input: { const band = input.element?.type === "node" ? canvasNodeIconBand(input.element.icon) : 0; const details = { budget: { - message: `${subject} exceeds the per-diagram logo budget; the node renders without a logo.`, + message: `${subject} exceeds the per-diagram logo budget; the ${owner} renders without a logo.`, hint: "Use fewer distinct logos in one diagram.", }, lifeline: { @@ -270,7 +337,7 @@ function droppedIconIssue(input: { hint: `Make node "${dropped.elementId}" about ${band}px taller or shorten its label to keep the logo.`, }, unavailable: { - message: `${subject} could not be loaded; the node renders without a logo.`, + message: `${subject} could not be loaded; the ${owner} renders without a logo.`, hint: "Retry the request; if the logo stays unavailable, omit icon.", }, }[dropped.reason]; @@ -296,7 +363,7 @@ export const embedSceneIcons = Effect.fn("codeMode.icons.embedScene")(function* catalog: CodeModeIconCatalog | undefined, ) { const elements = scene.elements.map((element) => - element.type === "node" && element.icon + (element.type === "node" || element.type === "frame") && element.icon ? { ...element, icon: { @@ -309,7 +376,9 @@ export const embedSceneIcons = Effect.fn("codeMode.icons.embedScene")(function* const slugs = [ ...new Set( elements.flatMap((element) => - element.type === "node" && element.icon ? [element.icon.slug] : [], + (element.type === "node" || element.type === "frame") && element.icon + ? [element.icon.slug] + : [], ), ), ]; diff --git a/packages/diagram/agent/src/lib/code-mode/runtime.ts b/packages/diagram/agent/src/lib/code-mode/runtime.ts index 8b454274..9bfffbaa 100644 --- a/packages/diagram/agent/src/lib/code-mode/runtime.ts +++ b/packages/diagram/agent/src/lib/code-mode/runtime.ts @@ -180,7 +180,12 @@ import { type PatchableScene, type QualityReport, } from "./contract.js"; -import { embedSceneIcons, resolveNodeIcons, type CodeModeIconCatalog } from "./icons.js"; +import { + embedSceneIcons, + fillGroupLogos, + resolveNodeIcons, + type CodeModeIconCatalog, +} from "./icons.js"; import { cleanToolString } from "../clean-tool-string.js"; import { assessFlowchartQuality } from "../flowchart/quality.js"; import { flowchartDiagramInput, normalizeFlowchartSpec } from "../flowchart/spec.js"; @@ -225,8 +230,9 @@ export interface CodeModeRuntimeOptions { }; /** * Internal per-call policy for MCP builds: in a diagram that draws logos, a - * node without one gets the logo its label unmistakably names (see - * resolveNodeIcons). Generation grounds logos in the prompt instead. + * node without one, or a flowchart group, gets the logo its label + * unmistakably names (see resolveNodeIcons and fillGroupLogos). Generation + * grounds logos in the prompt instead. */ fillNamedLogos?: boolean; artifactUrl?: (input: { artifactId: string; format: ArtifactFormat }) => string; @@ -1878,11 +1884,11 @@ function normalizePatchableScene(scene: PatchableScene): RenderedDiagramScene | (element.rendererRole === "er-table" && isStructurallyValidErTable(element)) || (element.rendererRole === "er-optional" && isStructurallyValidErOptional(element)) || (element.rendererRole === "class-box" && isStructurallyValidClassBox(element)) || - ((element.rendererRole === "swimlane" || - element.rendererRole === "swimlane-header" || - element.rendererRole === "zone") && + ((element.rendererRole === "swimlane" || element.rendererRole === "swimlane-header") && element.shape === "rectangle" && - !element.icon) + !element.icon) || + // A zone's logo sits in its header. + (element.rendererRole === "zone" && element.shape === "rectangle") ? { rendererRole: element.rendererRole } : {}), ...(element.textColor ? { textColor: element.textColor } : {}), @@ -4075,6 +4081,9 @@ const buildFlowchartWorkflow = Effect.fn("codeMode.buildFlowchart.workflow")(fun const resolvedIcons = resolveNodeIcons(authoredSpec.nodes, environment.icons, undefined, { fillNamedLogos: environment.fillNamedLogos === true, }); + const groupLogos = fillGroupLogos(authoredSpec.groups, resolvedIcons.nodes, environment.icons, { + fillNamedLogos: environment.fillNamedLogos === true, + }); const normalizedSpec = { ...authoredSpec, nodes: resolvedIcons.nodes }; const baseContext = { buildId, @@ -4084,7 +4093,7 @@ const buildFlowchartWorkflow = Effect.fn("codeMode.buildFlowchart.workflow")(fun const parsedDiagram = Schema.decodeUnknownResult(FlowchartDiagramSchema, { errors: "all", reportInput: true, - })(flowchartDiagramInput(normalizedSpec)); + })(flowchartDiagramInput(normalizedSpec, groupLogos.logos)); if (!Result.isSuccess(parsedDiagram)) { return yield* new BuildFlowchartFailure({ status: "invalid_flowchart", @@ -4168,7 +4177,12 @@ const buildFlowchartWorkflow = Effect.fn("codeMode.buildFlowchart.workflow")(fun normalizedSpec, quality, artifact, - issues: [...resolvedIcons.issues, ...embedded.issues, ...longDescriptionIssues(scene)], + issues: [ + ...resolvedIcons.issues, + ...groupLogos.issues, + ...embedded.issues, + ...longDescriptionIssues(scene), + ], } satisfies Extract; }); diff --git a/packages/diagram/agent/src/lib/flowchart/spec.ts b/packages/diagram/agent/src/lib/flowchart/spec.ts index 205fbb7c..bc85014f 100644 --- a/packages/diagram/agent/src/lib/flowchart/spec.ts +++ b/packages/diagram/agent/src/lib/flowchart/spec.ts @@ -76,14 +76,24 @@ export function normalizeFlowchartSpec(spec: FlowchartSpec): NormalizedFlowchart }; } -export function flowchartDiagramInput(spec: NormalizedFlowchartSpec) { +/** The core diagram input for a spec, each group drawing the logo `groupLogos` gives it by id. */ +export function flowchartDiagramInput( + spec: NormalizedFlowchartSpec, + groupLogos: ReadonlyMap = new Map(), +) { + const groupIcon = (id: string) => { + const slug = groupLogos.get(id); + return slug ? { icon: { slug } } : {}; + }; return { id: spec.id, title: spec.title, type: FLOWCHART_TYPE, nodes: spec.nodes.map((node) => ({ ...node, metadata: {} })), edges: spec.edges.map((edge) => ({ ...edge, metadata: {} })), - ...(spec.groups ? { groups: spec.groups.map((group) => ({ ...group })) } : {}), + ...(spec.groups + ? { groups: spec.groups.map((group) => ({ ...group, ...groupIcon(group.id) })) } + : {}), ...(spec.lanes ? { lanes: spec.lanes.map((lane) => ({ ...lane })) } : {}), layout: { direction: spec.layout.direction, diff --git a/packages/diagram/core/src/canvas-icons.test.ts b/packages/diagram/core/src/canvas-icons.test.ts index 81d2bedf..1750963a 100644 --- a/packages/diagram/core/src/canvas-icons.test.ts +++ b/packages/diagram/core/src/canvas-icons.test.ts @@ -9,6 +9,7 @@ import { canvasNodeIconBox, embedCanvasIcons, getCanvasValidationIssues, + type CanvasFrameElement, type CanvasIconAsset, type CanvasShapeElement, type CanvasSpec, @@ -34,7 +35,10 @@ function node(id: string, overrides: Partial = {}): CanvasSh }; } -function canvas(elements: CanvasShapeElement[], overrides: Partial = {}): CanvasSpec { +function canvas( + elements: (CanvasShapeElement | CanvasFrameElement)[], + overrides: Partial = {}, +): CanvasSpec { return { kind: "canvas", version: CANVAS_SPEC_VERSION, @@ -251,6 +255,38 @@ describe("embedCanvasIcons safety", () => { ]); expect(result.scene.icons).toBeUndefined(); }); + + it("keeps a group's logo on its frame or zone, whatever its name's length", () => { + const icon = { slug: "docker", size: 18 }; + const zone = node("group:cloud", { + icon, + label: "A zone named at a length no header strip could hold beside a logo", + rendererRole: "zone", + width: 40, + height: 40, + }); + const frame: CanvasFrameElement = { + type: "frame", + id: "group:vercel", + name: "Vercel", + icon, + x: 0, + y: 0, + width: 200, + height: 100, + }; + const result = embedCanvasIcons(canvas([zone, frame]), () => DOCKER); + + expect(result.dropped).toEqual([]); + expect(result.scene.elements).toEqual([zone, frame]); + expect(Object.keys(result.scene.icons ?? {})).toEqual(["docker"]); + expect(getCanvasValidationIssues(result.scene)).toEqual([]); + expect( + embedCanvasIcons(canvas([zone, frame]), () => undefined).dropped.map( + (dropped) => dropped.reason, + ), + ).toEqual(["unavailable", "unavailable"]); + }); }); describe("node icon validation", () => { diff --git a/packages/diagram/core/src/canvas.ts b/packages/diagram/core/src/canvas.ts index 0de16e61..d9555e96 100644 --- a/packages/diagram/core/src/canvas.ts +++ b/packages/diagram/core/src/canvas.ts @@ -364,8 +364,8 @@ export interface CanvasIconAsset { * band) and optional-end rings, class boxes (a class's outline, name band * and methods compartment), swimlanes (a lane's band and its header), and * architecture zones (a region, VPC, subnet or cluster outline). - * They carry no icon, and only a table's or a class's name band and a lane's - * header carry a bound label. + * Only a zone carries an icon, drawn in its header, and only a table's or a + * class's name band and a lane's header carry a bound label. */ export const CANVAS_RENDERER_ROLES = [ "zone", @@ -537,6 +537,8 @@ export interface CanvasFrameElement extends CanvasElementComposition, CanvasStro */ readonly nodeId?: string; readonly name?: string; + /** A logo drawn before its name (see canvasGroupIconBox). */ + readonly icon?: CanvasNodeIcon; readonly x: number; readonly y: number; readonly width: number; @@ -823,9 +825,11 @@ export interface CanvasIconEmbedding { readonly scene: CanvasSpec; } -function withoutIcon(element: CanvasShapeElement): CanvasShapeElement { +function withoutIcon( + element: Element, +): Element { const { icon: _icon, ...rest } = element; - return rest; + return rest as Element; } function canvasLayerVisibility(canvas: CanvasSpec): (element: CanvasElement) => boolean { @@ -897,9 +901,9 @@ export function embedCanvasIcons( const boundLabels = visibleBoundLabels(canvas, isVisible); let totalBytes = 0; const elements = canvas.elements.map((element, index): CanvasElement => { - if (element.type !== "node" || !element.icon) return element; + if ((element.type !== "node" && element.type !== "frame") || !element.icon) return element; const icon = element.icon; - const drop = (reason: DroppedCanvasIcon["reason"]): CanvasShapeElement => { + const drop = (reason: DroppedCanvasIcon["reason"]): CanvasElement => { dropped.push({ elementId: element.id, index, reason, slug: icon.slug }); return withoutIcon(element); }; @@ -911,8 +915,15 @@ export function embedCanvasIcons( ) { return drop("unavailable"); } - if (element.rendererRole) return drop("lifeline"); - if (element.shape !== "polygon" && isVisible(element)) { + if (element.type === "node" && element.rendererRole && element.rendererRole !== "zone") + return drop("lifeline"); + // A group's logo sits beside its name, which its renderer sized to hold both. + if ( + element.type === "node" && + !element.rendererRole && + element.shape !== "polygon" && + isVisible(element) + ) { const label = boundLabels.get(element.id)?.element; if ( nodeLabelOverflows(element, label, icon) && @@ -941,16 +952,17 @@ export function embedCanvasIcons( }; } -function validateNodeIcon( +function validateIcon( canvas: CanvasSpec, - element: CanvasShapeElement, + element: CanvasShapeElement | CanvasFrameElement, index: number, ): CanvasValidationIssue[] { const icon = element.icon; if (!icon) return []; const path = `elements[${index}].icon`; + const noun = element.type === "frame" ? "Frame" : "Node"; const issues: CanvasValidationIssue[] = []; - if (element.rendererRole) { + if (element.type === "node" && element.rendererRole && element.rendererRole !== "zone") { issues.push({ code: "invalid_icon", elementId: element.id, @@ -962,14 +974,14 @@ function validateNodeIcon( issues.push({ code: "invalid_icon", elementId: element.id, - message: `Node "${element.id}" has an invalid icon slug "${icon.slug}".`, + message: `${noun} "${element.id}" has an invalid icon slug "${icon.slug}".`, path: `${path}.slug`, }); } else if (!canvas.icons || !Object.hasOwn(canvas.icons, icon.slug)) { issues.push({ code: "invalid_icon", elementId: element.id, - message: `Node "${element.id}" references icon "${icon.slug}" without an embedded asset.`, + message: `${noun} "${element.id}" references icon "${icon.slug}" without an embedded asset.`, path: `${path}.slug`, }); } @@ -981,7 +993,7 @@ function validateNodeIcon( issues.push({ code: "invalid_icon", elementId: element.id, - message: `Node "${element.id}" icon size must be ${CANVAS_NODE_ICON.minSize}-${CANVAS_NODE_ICON.maxSize}.`, + message: `${noun} "${element.id}" icon size must be ${CANVAS_NODE_ICON.minSize}-${CANVAS_NODE_ICON.maxSize}.`, path: `${path}.size`, }); } @@ -1132,8 +1144,8 @@ export function getCanvasValidationIssues(canvas: CanvasSpec): CanvasValidationI issues.push(...validateIconAssets(canvas)); canvas.elements.forEach((element, index) => { issues.push(...validateElementLimits(element, index)); - if (element.type === "node") { - issues.push(...validateNodeIcon(canvas, element, index)); + if (element.type === "node" || element.type === "frame") { + issues.push(...validateIcon(canvas, element, index)); } if (element.type === "node" && element.shape !== "polygon" && isLayerVisible(element)) { const boundLabel = nodeLabelsByContainerId.get(element.id); diff --git a/packages/diagram/core/src/group-regions.test.ts b/packages/diagram/core/src/group-regions.test.ts index 4eb1b77e..a3c76461 100644 --- a/packages/diagram/core/src/group-regions.test.ts +++ b/packages/diagram/core/src/group-regions.test.ts @@ -1,6 +1,11 @@ import { describe, expect, it } from "vitest"; -import { CANVAS_FRAME_NAME, canvasFrameNameBox, canvasFrameNameWidth } from "./group-regions"; +import { + CANVAS_FRAME_NAME, + canvasFrameNameBox, + canvasFrameNameWidth, + canvasGroupIconBox, +} from "./group-regions"; /** Helvetica advance widths, in thousandths of an em (Adobe AFM). */ const HELVETICA: Readonly> = { @@ -50,6 +55,25 @@ describe("frame names", () => { ); }); + it("puts a frame's logo inside its top-left corner", () => { + const icon = { slug: "vercel", size: 18 }; + const bare = { type: "frame" as const, x: 100, y: 80, width: 300, height: 200 }; + + expect(canvasGroupIconBox({ ...bare, icon })).toEqual({ x: 110, y: 90, width: 18, height: 18 }); + expect(canvasGroupIconBox(bare)).toBeUndefined(); + }); + + it("puts a zone's logo at the start of its header strip", () => { + const icon = { slug: "aws", size: 18 }; + + expect(canvasGroupIconBox({ type: "node", x: 100, y: 80, icon })).toEqual({ + x: 112, + y: 89, + width: 18, + height: 18, + }); + }); + it("keeps a lowercase name close to its painted width", () => { expect(canvasFrameNameWidth("desk support routes")).toBeLessThan( 1.4 * helveticaWidth("desk support routes"), diff --git a/packages/diagram/core/src/group-regions.ts b/packages/diagram/core/src/group-regions.ts index 1473adc5..a9528abf 100644 --- a/packages/diagram/core/src/group-regions.ts +++ b/packages/diagram/core/src/group-regions.ts @@ -1,4 +1,10 @@ -import type { CanvasFrameElement, CanvasSpec } from "./canvas.js"; +import type { + CanvasElementBounds, + CanvasFrameElement, + CanvasNodeIcon, + CanvasShapeElement, + CanvasSpec, +} from "./canvas.js"; import { AXIS_ALIGNED_EPSILON, segmentCrossesBoundsInterior, @@ -18,6 +24,52 @@ export const CANVAS_FRAME_NAME = Object.freeze({ height: 21, }); +/** + * A group's logo, as tall as its name's line: a zone's sits this gap before + * its label, and a frame's this far inside its top-left corner, within the + * padding that keeps members and their routes off the frame's sides. + */ +export const CANVAS_GROUP_ICON = Object.freeze({ gap: 6, inset: 10, size: 18 }); + +/** + * A zone's header strip across its top, which holds its label this far in from + * the zone's left side, after its logo when it has one. + */ +export const CANVAS_ZONE_HEADER = Object.freeze({ height: 36, labelInset: 12 }); + +/** How far a group's logo pushes its name along: the logo and its gap. */ +export function canvasGroupIconRun(icon: CanvasNodeIcon | undefined): number { + return icon ? icon.size + CANVAS_GROUP_ICON.gap : 0; +} + +/** + * Absolute box of a group's logo. A zone draws it at the start of its header, + * its label after it. A frame draws it inside its top-left corner, under its + * name, so the logo is one of the frame's children and moves with it; outside + * the frame, Excalidraw would clip it. + */ +export function canvasGroupIconBox( + element: + | Pick + | Pick, +): CanvasElementBounds | undefined { + const { icon } = element; + if (!icon) return undefined; + return element.type === "frame" + ? { + x: element.x + CANVAS_GROUP_ICON.inset, + y: element.y + CANVAS_GROUP_ICON.inset, + width: icon.size, + height: icon.size, + } + : { + x: element.x + CANVAS_ZONE_HEADER.labelInset, + y: element.y + (CANVAS_ZONE_HEADER.height - icon.size) / 2, + width: icon.size, + height: icon.size, + }; +} + /** Glyphs wider than an em in Helvetica (W, M, @, %) and in Excalifont (%). */ const WIDEST_NAME_GLYPHS = /[@%MW]/u; /** Capitals, digits, m, w and wide symbols: up to 0.86 em in either face. */ diff --git a/packages/diagram/core/src/intermediate.ts b/packages/diagram/core/src/intermediate.ts index 8ef0c86f..41793c5a 100644 --- a/packages/diagram/core/src/intermediate.ts +++ b/packages/diagram/core/src/intermediate.ts @@ -81,6 +81,8 @@ export class DiagramGroup extends Schema.Class("DiagramGroup")({ /** The id of the declared group this group sits inside. */ parent: Schema.optional(NonEmptyString), kind: Schema.optional(DiagramZoneKindSchema), + /** A catalog mark drawn before the group's label, such as its platform's logo. */ + icon: Schema.optional(DiagramIconRef), }) {} export const DiagramGroupSchema = DiagramGroup; diff --git a/packages/diagram/core/src/types/flowchart.ts b/packages/diagram/core/src/types/flowchart.ts index 74f54a50..c088945c 100644 --- a/packages/diagram/core/src/types/flowchart.ts +++ b/packages/diagram/core/src/types/flowchart.ts @@ -671,4 +671,50 @@ export const deployPipelineLogoFlowchart = parseFlowchartDiagram({ }, }); +/** + * A request flow whose Vercel group draws its platform's logo before its name, + * among node logos (Frontierclimb r23 ai-app-stack-logos s4). + */ +export const requestFlowGroupLogoFlowchart = parseFlowchartDiagram({ + id: "request-flow-group-logo", + title: "Request flow", + type: flowchartDiagramType, + groups: [{ id: "vercel", label: "Vercel", icon: { slug: "vercel" } }], + nodes: [ + { + id: "page", + label: "Next.js page receives the question", + kind: "start", + icon: { slug: "nextjs" }, + group: "vercel", + }, + { + id: "check", + label: "Check Redis cache", + kind: "process", + icon: { slug: "redis" }, + group: "vercel", + }, + { id: "hit", label: "Cache hit?", kind: "decision", group: "vercel" }, + { id: "call", label: "Call OpenAI model", kind: "process", icon: { slug: "openai" } }, + { id: "store", label: "Store answer in Redis", kind: "process", icon: { slug: "redis" } }, + { id: "log", label: "Log any errors to Sentry", kind: "process", icon: { slug: "sentry" } }, + { id: "ret", label: "Return the answer", kind: "end" }, + { id: "cached", label: "Return cached answer", kind: "end" }, + ], + edges: [ + { id: "page-check", source: "page", target: "check" }, + { id: "check-hit", source: "check", target: "hit" }, + { id: "hit-cached", source: "hit", target: "cached", label: "yes" }, + { id: "hit-call", source: "hit", target: "call", label: "no" }, + { id: "call-store", source: "call", target: "store" }, + { id: "store-log", source: "store", target: "log" }, + { id: "log-ret", source: "log", target: "ret" }, + ], + layout: { + direction: "TB", + edgeRouting: "orthogonal", + }, +}); + export const flowchartEvaluationFixtures = [flowchartFixture, pharmaBatchDispositionFlowchart]; diff --git a/packages/diagram/excalidraw/README.md b/packages/diagram/excalidraw/README.md index 36878f1f..0d87c870 100644 --- a/packages/diagram/excalidraw/README.md +++ b/packages/diagram/excalidraw/README.md @@ -42,3 +42,8 @@ The node's shape, bound label, and logo share one group so they move together. Bottom- and top-aligned bound labels use Excalidraw's own position rules, so a logo node's label stays put when it is edited. Pass `files` through to Excalidraw's `initialData` so logos paint on the first frame. + +A group's logo (an `icon` on a frame or a zone) is placed with +`canvasGroupIconBox`: at the start of a zone's header, before its label, and +shares the zone's group; or inside a frame's top-left corner, under its name, +as one of the frame's children (`frameId`), so it moves with the frame. diff --git a/packages/diagram/excalidraw/src/lib/convert.ts b/packages/diagram/excalidraw/src/lib/convert.ts index c91778ee..5c2548f6 100644 --- a/packages/diagram/excalidraw/src/lib/convert.ts +++ b/packages/diagram/excalidraw/src/lib/convert.ts @@ -13,6 +13,7 @@ import { CANVAS_CROSSABLE_RENDERER_ROLES, canvasBoundTextBox, canvasBoundTextInset, + canvasGroupIconBox, canvasNodeIconBox, canvasComposedOnOutline, canvasShapeOutline, @@ -349,13 +350,19 @@ function shapeStrokeElements(input: { function iconImageElement(input: { fileId: string; id: string; - node: NodeSceneElement; + node: NodeSceneElement | FrameSceneElement; }): ExcalidrawElement | null { - const box = canvasNodeIconBox(input.node); - if (!box || !input.node.icon) return null; + const { node } = input; + const group = node.type === "frame" || node.rendererRole === "zone"; + const box = group ? canvasGroupIconBox(node) : canvasNodeIconBox(node); + if (!box || !node.icon) return null; return { - ...elementBase(input.id, input.node), - groupIds: [logoGroupId(input.node), ...(input.node.groupIds ?? [])], + ...elementBase(input.id, node), + // A frame's logo is one of its children, so it moves with the frame; + // Excalidraw cannot group a frame. Other logos share their shape's group. + ...(node.type === "frame" + ? { frameId: node.id, groupIds: [] } + : { groupIds: [logoGroupId(node), ...(node.groupIds ?? [])] }), type: "image", x: box.x, y: box.y, @@ -364,7 +371,7 @@ function iconImageElement(input: { backgroundColor: "transparent", boundElements: null, crop: null, - customData: { sketchiNodeIcon: input.node.icon.slug }, + customData: group ? { sketchiGroupIcon: node.icon.slug } : { sketchiNodeIcon: node.icon.slug }, fileId: input.fileId, roundness: null, scale: [1, 1], @@ -753,7 +760,10 @@ function synthesizeNodeLabels(sourceElements: readonly SceneElement[]): { return { textElements, generatedLabelSourceIds }; } -function iconElementId(node: NodeSceneElement, usedElementIds: Set): string { +function iconElementId( + node: NodeSceneElement | FrameSceneElement, + usedElementIds: Set, +): string { // Excalidraw's SVG export selects per-element image symbols by id, so the // id must be a valid CSS identifier fragment (node ids contain colons). const baseId = `${node.id.replace(/[^\w-]/gu, "-")}-icon`; @@ -861,6 +871,18 @@ function buildElements( // A composite state's transitions bind to its frame. if (frame.nodeId !== undefined) shapeElementsByNodeId.set(frame.nodeId, renderedFrame); elements.push(renderedFrame); + const asset = + frame.icon && scene.icons && Object.hasOwn(scene.icons, frame.icon.slug) + ? scene.icons[frame.icon.slug] + : undefined; + if (!frame.icon || typeof asset?.svg !== "string") continue; + const fileId = iconFileId(frame.icon.slug, asset.svg); + const id = iconElementId(frame, usedElementIds); + const image = iconImageElement({ fileId, id, node: frame }); + if (!image) continue; + files[fileId] ??= iconFile(fileId, asset.svg); + generatedSourceIds.set(id, frame.id); + elements.push(image); } for (const text of textElements) { diff --git a/packages/diagram/generation/src/lib/candidates.ts b/packages/diagram/generation/src/lib/candidates.ts index 41f91448..36fb9df7 100644 --- a/packages/diagram/generation/src/lib/candidates.ts +++ b/packages/diagram/generation/src/lib/candidates.ts @@ -192,9 +192,20 @@ function sanitizeGeneratedNodeIcons( function normalizeGeneratedFlowchartInput(input: unknown, diagnostics: string[]): unknown { const styled = withSketchiDiagramStyle(input); if (!isUnknownRecord(styled)) return styled; + const groups = styled["groups"]; const withIcons: Record = { ...styled, nodes: sanitizeGeneratedNodeIcons(styled["nodes"], diagnostics), + // Group logos are filled by Code Mode builds only; generation draws none. + ...(Array.isArray(groups) + ? { + groups: groups.map((group) => { + if (!isUnknownRecord(group)) return group; + const { icon: _icon, ...withoutIcon } = group; + return withoutIcon; + }), + } + : {}), }; const edges = withIcons["edges"]; if (!Array.isArray(edges)) { diff --git a/packages/diagram/generation/src/lib/zones.test.ts b/packages/diagram/generation/src/lib/zones.test.ts index 7613f7b1..01992c75 100644 --- a/packages/diagram/generation/src/lib/zones.test.ts +++ b/packages/diagram/generation/src/lib/zones.test.ts @@ -41,4 +41,25 @@ describe("zone generation", () => { ); expect(diagram.nodes.map((node) => node.shape)).toEqual(nodes.map((node) => node.shape)); }); + + it("draws no logo a model puts on a group, well-formed or not", () => { + const parsed = candidate({ + id: "edge", + title: "Edge", + type: "flowchart", + groups: [ + { id: "vercel", label: "Vercel", icon: { slug: "vercel" } }, + { id: "cloud", label: "Cloud", icon: "not an icon" }, + ], + nodes: [ + { id: "page", label: "Page", kind: "start", group: "vercel" }, + { id: "api", label: "API", kind: "end", group: "cloud" }, + ], + edges: [{ id: "a", source: "page", target: "api" }], + }); + expect(parsed.error).toBeUndefined(); + const diagram = parsed.diagram; + if (diagram?.type !== "flowchart") throw new Error("Expected a flowchart."); + expect(diagram.groups?.map((group) => group.icon)).toEqual([undefined, undefined]); + }); }); diff --git a/packages/diagram/renderer/src/scene.ts b/packages/diagram/renderer/src/scene.ts index 30433448..d2a3d518 100644 --- a/packages/diagram/renderer/src/scene.ts +++ b/packages/diagram/renderer/src/scene.ts @@ -7,7 +7,11 @@ import { canvasNodeIconBand, type AxisAlignedSegment, CANVAS_FRAME_NAME, + CANVAS_GROUP_ICON, CANVAS_SHAPE_CURVE, + CANVAS_ZONE_HEADER, + canvasGroupIconBox, + canvasGroupIconRun, canvasComposedOutlinePoint, canvasFrameNameBox, type CanvasComposedShapeKind, @@ -557,8 +561,8 @@ const GROUP_UNIT_PREFIX = "\u0000group:"; * far in from the zone's left side. */ const ZONE_PADDING = 24; -const ZONE_HEADER = 36; -const ZONE_LABEL_INSET = 12; +const ZONE_HEADER = CANVAS_ZONE_HEADER.height; +const ZONE_LABEL_INSET = CANVAS_ZONE_HEADER.labelInset; const ZONE_LABEL_FONT_SIZE = CANVAS_FRAME_NAME.fontSize; interface Box { @@ -572,11 +576,25 @@ interface Box { interface GroupFrame { readonly box: Box; readonly elementId: string; + /** The logo drawn before its label. */ + readonly icon?: CanvasNodeIcon; readonly id: string; /** A zone's kind, which sets its outline and fill. */ readonly kind?: DiagramZoneKind; readonly label: string; readonly labelBox: Box; + /** A frame's logo inside its top-left corner, padded by the name's clearance. */ + readonly logoBox?: Box; +} + +/** What routes and labels keep off for a group: its name, and a frame's logo. */ +function groupKeepOuts(frame: GroupFrame): Box[] { + return frame.logoBox ? [frame.labelBox, frame.logoBox] : [frame.labelBox]; +} + +/** The logo a group draws before its label. */ +function groupIcon(group: DiagramGroup): CanvasNodeIcon | undefined { + return group.icon ? { slug: group.icon.slug, size: CANVAS_GROUP_ICON.size } : undefined; } interface GroupLayout { @@ -2044,9 +2062,13 @@ function positionGroupedNodes( ), ) + (slack.get(group.id) ?? 0); const labelWidth = canvasFrameNameWidth(group.label); - // Where the name ends, measured from the frame's left side. + // Where the name ends, measured from the frame's left side; a frame's logo + // sits left of that side, a zone's before its label. const nameEnd = zoned - ? ZONE_LABEL_INSET + labelWidth + CANVAS_FRAME_NAME.clearance + ? ZONE_LABEL_INSET + + canvasGroupIconRun(groupIcon(group)) + + labelWidth + + CANVAS_FRAME_NAME.clearance : labelWidth + 2 * CANVAS_FRAME_NAME.clearance; const ports = nodes .filter((node) => crossing.has(node.nodeId) && inset(node, "top") <= AXIS_ALIGNED_EPSILON) @@ -2199,15 +2221,19 @@ function positionGroupedNodes( : structure.groups ).map(({ group }): GroupFrame => { const box = boxes.get(group.id)!; + const icon = groupIcon(group); + const logo = !zoned && icon ? canvasGroupIconBox({ type: "frame", ...box, icon }) : undefined; return { box, elementId: `group:${group.id}`, + ...(icon ? { icon } : {}), id: group.id, ...(zoned && group.kind !== undefined ? { kind: group.kind } : {}), label: group.label, labelBox: zoned - ? zoneLabelBox(box, group.label) + ? zoneLabelBox(box, group.label, icon) : canvasFrameNameBox({ ...box, name: group.label }), + ...(logo ? { logoBox: padBox(logo, CANVAS_FRAME_NAME.clearance) } : {}), }; }); return { @@ -2258,13 +2284,13 @@ function groupRoot(structure: GroupStructure, groupId: string): string { return groupRoots(structure, groupId).at(-1)!; } -/** Where a zone's label is painted in its header strip, padded by its clearance. */ -function zoneLabelBox(box: Box, label: string): Box { +/** Where a zone's label, and the logo before it, are painted in its header strip, padded by its clearance. */ +function zoneLabelBox(box: Box, label: string, icon: CanvasNodeIcon | undefined): Box { const { clearance, height } = CANVAS_FRAME_NAME; return { x: box.x + ZONE_LABEL_INSET - clearance, y: box.y + (ZONE_HEADER - height) / 2 - clearance, - width: canvasFrameNameWidth(label) + 2 * clearance, + width: canvasGroupIconRun(icon) + canvasFrameNameWidth(label) + 2 * clearance, height: height + 2 * clearance, }; } @@ -3076,11 +3102,13 @@ function groupObstacles(groups: GroupLayout | undefined, route: RoutedEdge): Nod const shared = sharedGroup(groups, route.source.nodeId, route.target.nodeId); return groups.frames.flatMap((frame) => { const label = obstacleBox(`group-label:${frame.id}`, frame.labelBox); + const logo = frame.logoBox ? [obstacleBox(`group-logo:${frame.id}`, frame.logoBox)] : []; if (frame.id === shared) { const { x, y, width, height } = frame.box; const wall = GROUP_WALL; return [ label, + ...logo, obstacleBox(`group-wall:${frame.id}:top`, { x: x - wall, y: y - wall, @@ -3107,7 +3135,7 @@ function groupObstacles(groups: GroupLayout | undefined, route: RoutedEdge): Nod }), ]; } - return ends.has(frame.id) ? [label] : [label, obstacleBox(frame.elementId, frame.box)]; + return ends.has(frame.id) ? [label, ...logo] : [label, obstacleBox(frame.elementId, frame.box)]; }); } @@ -3195,7 +3223,7 @@ function groupRouteDefects( if ( segments.some( (segment) => - segmentCrossesBoundsInterior(segment, frame.labelBox) || + groupKeepOuts(frame).some((box) => segmentCrossesBoundsInterior(segment, box)) || (!own && segmentCrossesBoundsInterior(segment, frame.box)) || outlines[index]!.some( (edge) => @@ -5988,9 +6016,10 @@ function placedBranchLabels( // boundaries are lines a label keeps clear of. const nodes = [ ...shapes, - ...(groups?.frames ?? []).map((frame) => + ...(groups?.frames ?? []).flatMap((frame) => [ obstacleBox(`group-label:${frame.id}`, frame.labelBox), - ), + ...(frame.logoBox ? [obstacleBox(`group-logo:${frame.id}`, frame.logoBox)] : []), + ]), ...(lanes?.frames ?? []).map((frame) => obstacleBox(`lane-header:${laneElementId(frame)}`, frame.header), ), @@ -7190,7 +7219,8 @@ function alignMarkers( (groups?.frames ?? []).every( (candidate) => around.has(candidate.id) || - (!boxesOverlap(room, candidate.box) && !boxesOverlap(room, candidate.labelBox)), + (!boxesOverlap(room, candidate.box) && + groupKeepOuts(candidate).every((box) => !boxesOverlap(room, box))), ) && (!frame || (room.x >= frame.box.x && @@ -7383,13 +7413,15 @@ function crowdedGroups(layout: DiagramLayout): Set { for (const frame of groups.frames) if ( frameOutline(frame.box).some((edge) => segmentCrossesBoundsInterior(edge, box)) || - boxesOverlap(box, frame.labelBox) + groupKeepOuts(frame).some((keepOut) => boxesOverlap(box, keepOut)) ) crowded.add(frame.id); if (group === undefined) continue; if ( outlines.some((edge) => segmentCrossesBoundsInterior(edge, box)) || - groups.frames.some((frame) => boxesOverlap(box, frame.labelBox)) + groups.frames.some((frame) => + groupKeepOuts(frame).some((keepOut) => boxesOverlap(box, keepOut)), + ) ) crowded.add(group); } @@ -9401,13 +9433,15 @@ function zoneElements( accent: string, ): (NodeSceneElement | TextSceneElement)[] { return groups.frames.flatMap((frame): (NodeSceneElement | TextSceneElement)[] => { - const maxWidth = Math.max(1, frame.box.width - 2 * ZONE_LABEL_INSET); + const logo = canvasGroupIconRun(frame.icon); + const maxWidth = Math.max(1, frame.box.width - 2 * ZONE_LABEL_INSET - logo); return [ { type: "node", id: frame.elementId, nodeId: frame.elementId, kind: frame.kind ?? "zone", + ...(frame.icon ? { icon: frame.icon } : {}), rendererRole: "zone", shape: "rectangle", ...frame.box, @@ -9422,6 +9456,7 @@ function zoneElements( x: frame.box.x + ZONE_LABEL_INSET + + logo + boundLabelWidth(frame.label, ZONE_LABEL_FONT_SIZE, maxWidth) / 2, y: frame.box.y + ZONE_HEADER / 2, text: frame.label, @@ -9491,6 +9526,7 @@ function withGroupFrames( type: "frame", id: frame.elementId, name: frame.label, + ...(frame.icon ? { icon: frame.icon } : {}), // Edges into and out of a composite bind to its frame. ...(groups.composites.has(frame.id) ? { nodeId: frame.id } : {}), ...frame.box, diff --git a/packages/diagram/ui/src/components/excalidraw-scene-canvas/excalidraw-scene-canvas.stories.tsx b/packages/diagram/ui/src/components/excalidraw-scene-canvas/excalidraw-scene-canvas.stories.tsx index ec4919dd..6571c69b 100644 --- a/packages/diagram/ui/src/components/excalidraw-scene-canvas/excalidraw-scene-canvas.stories.tsx +++ b/packages/diagram/ui/src/components/excalidraw-scene-canvas/excalidraw-scene-canvas.stories.tsx @@ -10,7 +10,7 @@ import { convertSceneToExcalidraw } from "@sketchi/diagram-excalidraw"; import { renderIntermediateDiagram } from "@sketchi/diagram-renderer"; import { ExcalidrawSceneCanvas } from "./excalidraw-scene-canvas"; -import { nodeLogoScene, participantLogoScene } from "./node-logo-scene.fixtures"; +import { groupLogoScene, nodeLogoScene, participantLogoScene } from "./node-logo-scene.fixtures"; const scene = convertSceneToExcalidraw(renderIntermediateDiagram(flowchartFixture)); @@ -92,6 +92,28 @@ export const EditableNodeLogosLeftToRight: Story = { tags: ["test"], }; +/** The Vercel mark before its group's name, among node logos. */ +export const GroupLogo: Story = { + args: { + scene: convertSceneToExcalidraw(groupLogoScene()), + title: "Request flow with a group logo", + viewModeEnabled: true, + zenModeEnabled: true, + }, + tags: ["test"], +}; + +/** The Vercel mark at the start of a cloud zone's header. */ +export const ZoneLogo: Story = { + args: { + scene: convertSceneToExcalidraw(groupLogoScene(true)), + title: "Request flow with a zone logo", + viewModeEnabled: true, + zenModeEnabled: true, + }, + tags: ["test"], +}; + /** GitHub, Cloudflare, and Docker marks in sequence participant headers. */ export const SequenceParticipantLogos: Story = { args: { diff --git a/packages/diagram/ui/src/components/excalidraw-scene-canvas/node-logo-scene.fixtures.ts b/packages/diagram/ui/src/components/excalidraw-scene-canvas/node-logo-scene.fixtures.ts index db4274a2..ef09a41f 100644 --- a/packages/diagram/ui/src/components/excalidraw-scene-canvas/node-logo-scene.fixtures.ts +++ b/packages/diagram/ui/src/components/excalidraw-scene-canvas/node-logo-scene.fixtures.ts @@ -1,6 +1,7 @@ import { deployPipelineLogoFlowchart, deployWebhookLogoSequence, + requestFlowGroupLogoFlowchart, embedCanvasIcons, type CanvasIconAsset, type LayoutDirection, @@ -15,11 +16,21 @@ import cloudflare from "@sketchi/icon-catalog/svg/cloud-vendors/cloudflare.svg?r import docker from "@sketchi/icon-catalog/svg/devtools-ci/docker.svg?raw"; import github from "@sketchi/icon-catalog/svg/devtools-ci/github.svg?raw"; import vitest from "@sketchi/icon-catalog/svg/devtools-ci/vitest.svg?raw"; +import openai from "@sketchi/icon-catalog/svg/ai-model-labs/openai.svg?raw"; +import vercel from "@sketchi/icon-catalog/svg/cloud-hosting-paas/vercel.svg?raw"; +import redis from "@sketchi/icon-catalog/svg/data-storage/redis.svg?raw"; +import nextjs from "@sketchi/icon-catalog/svg/frontend-frameworks/nextjs.svg?raw"; +import sentry from "@sketchi/icon-catalog/svg/observability-security/sentry.svg?raw"; const NODE_LOGOS: Readonly> = { cloudflare: { name: "Cloudflare", svg: normalizeNodeLogoSvg(cloudflare) }, docker: { name: "Docker", svg: normalizeNodeLogoSvg(docker) }, github: { name: "GitHub", svg: normalizeNodeLogoSvg(github) }, + nextjs: { name: "Next.js", svg: normalizeNodeLogoSvg(nextjs) }, + openai: { name: "OpenAI", svg: normalizeNodeLogoSvg(openai) }, + redis: { name: "Redis", svg: normalizeNodeLogoSvg(redis) }, + sentry: { name: "Sentry", svg: normalizeNodeLogoSvg(sentry) }, + vercel: { name: "Vercel", svg: normalizeNodeLogoSvg(vercel) }, vitest: { name: "Vitest", svg: normalizeNodeLogoSvg(vitest) }, }; @@ -42,3 +53,17 @@ export function nodeLogoScene(direction: LayoutDirection = "TB"): RenderedDiagra export function participantLogoScene(): RenderedDiagramScene { return embedCanvasIcons(renderSequenceDiagram(deployWebhookLogoSequence), nodeLogo).scene; } + +/** + * The request-flow fixture: its Vercel group draws the Vercel logo before its + * name, as a frame or, with a kind, as a zone. + */ +export function groupLogoScene(zoned = false): RenderedDiagramScene { + const groups = requestFlowGroupLogoFlowchart.groups?.map((group) => + zoned ? { ...group, kind: "cloud" as const } : group, + ); + return embedCanvasIcons( + renderIntermediateDiagram({ ...requestFlowGroupLogoFlowchart, ...(groups ? { groups } : {}) }), + nodeLogo, + ).scene; +}