From aad75fca2eb87294004a0beb64deeb7844a33fc0 Mon Sep 17 00:00:00 2001 From: hushamsaeed Date: Sat, 2 May 2026 14:14:34 +0500 Subject: [PATCH] v0.1.1: renderer fixes + parser warnings + CLI hardening Found by an autonomous audit. All P0 bugs verified by reproduction. Renderer: - Same-row edges that skip over an intervening node now route up-and-over (was drawing a straight line through the intervening box). Detection is layer-aware: only same-layer co-rowed nodes count as "intervening." - Self-loop edges (a -> a) are no longer rendered as zero-length paths; surfaced as warnings instead. - Marker ids and CSS class names use a per-render namespace (ps-) derived from the DSL so two SVGs embedded on the same HTML page no longer collide on or scoped CSS selectors. - Multi-edges between the same pair stack visually via perpendicular offset; cyclic edges (a->b + b->a) similarly stack via stable direction. - Cite/signature suppressed when there are no nodes drawn (was rendering an empty signed canvas). - IBM Plex Mono dropped from FONT_MONO fallback. Parser: - New warnings[] field surfaces undefined-node edges and self-loops (was only computed in the CLI). - Edges now carry their source line number. Layout: - Removed the hard 1600px canvas cap. 13-node layers were clipping nodes off-canvas. Canvas now grows to honour NODE_MIN_W regardless of count. CLI: - -o / --out now validate that a value follows. - Warnings vs errors distinguished in stderr output. Tests: - 29 tests pass on Node 25.6 (was 23). Examples re-rendered with the new namespace. Co-Authored-By: Claude Opus 4.7 (1M context) --- bin/plinth-sketch.js | 29 +++++--- examples/plinth.svg | 2 +- examples/three-tier.svg | 2 +- package.json | 2 +- src/layout.js | 5 +- src/parser.js | 32 +++++++-- src/renderer.js | 146 ++++++++++++++++++++++++++++++---------- test/parser.test.js | 19 +++++- test/render.test.js | 34 +++++++++- 9 files changed, 213 insertions(+), 58 deletions(-) diff --git a/bin/plinth-sketch.js b/bin/plinth-sketch.js index eea6d13..4832700 100755 --- a/bin/plinth-sketch.js +++ b/bin/plinth-sketch.js @@ -58,7 +58,16 @@ function parseArgs(argv) { if (a === "-w" || a === "--watch") { args.watch = true; continue; } if (a === "--check") { args.check = true; continue; } if (a === "--no-signature") { args.signature = false; continue; } - if (a === "-o" || a === "--out") { args.out = argv[++i]; continue; } + if (a === "-o" || a === "--out") { + const next = argv[i + 1]; + if (next === undefined || (next.startsWith("-") && next !== "-")) { + console.error(`plinth-sketch: ${a} requires a value (file path or '-' for stdout)`); + process.exit(2); + } + args.out = next; + i++; + continue; + } if (a.startsWith("-") && a.length > 1 && a !== "-") { console.error(`plinth-sketch: unknown option: ${a}`); process.exit(2); @@ -94,22 +103,20 @@ function emitSvg(dsl, args) { const lay = layout(parsed); const svg = render(parsed, lay, { signature: args.signature }); - // Surface parse warnings on stderr (don't fail unless --check). + // Surface parse errors (unparsed lines) and warnings (undefined-node edges, + // self-loops) on stderr. --check will fail on either; default render won't. if (parsed.errors.length) { for (const err of parsed.errors) { - console.error(`plinth-sketch: warning: line ${err.line}: cannot parse: ${err.text}`); + console.error(`plinth-sketch: error: line ${err.line}: cannot parse: ${err.text}`); } } - const missing = new Set(); - for (const e of parsed.edges) { - if (!parsed.nodes[e.from]) missing.add(e.from); - if (!parsed.nodes[e.to]) missing.add(e.to); - } - if (missing.size) { - console.error(`plinth-sketch: warning: edge references undefined nodes: ${[...missing].join(", ")}`); + const warnings = parsed.warnings || []; + for (const w of warnings) { + const where = w.line ? `line ${w.line}: ` : ""; + console.error(`plinth-sketch: warning: ${where}${w.message}`); } - const hasErrors = parsed.errors.length > 0 || missing.size > 0; + const hasErrors = parsed.errors.length > 0 || warnings.length > 0; return { svg, hasErrors }; } diff --git a/examples/plinth.svg b/examples/plinth.svg index 9012be0..5961494 100644 --- a/examples/plinth.svg +++ b/examples/plinth.svg @@ -1 +1 @@ -User module · example-access-requestsSDKSubstrate · Helm umbrella chartFoundation · Kubernetesstarter-webNext.js 16 · React 19starter-apiGo 1.25 · chi · pgxsdk-ts · 7 packagesenv · api-client · authz · forms · tables · …sdk-go · 7 packagesauthz · audit · otel · errors · paginate · …CloudNativePGPostgres operator + clusterCerbos PDPpolicy decisions · authzOpenTelemetry Collectortraces · metrics · logsKubernetesAny conformant distributionHTTP · JSONimportsimportsMade with Plinth Sketch \ No newline at end of file +User module · example-access-requestsSDKSubstrate · Helm umbrella chartFoundation · Kubernetesstarter-webNext.js 16 · React 19starter-apiGo 1.25 · chi · pgxsdk-ts · 7 packagesenv · api-client · authz · forms · tables · …sdk-go · 7 packagesauthz · audit · otel · errors · paginate · …CloudNativePGPostgres operator + clusterCerbos PDPpolicy decisions · authzOpenTelemetry Collectortraces · metrics · logsKubernetesAny conformant distributionHTTP · JSONimportsimportsMade with Plinth Sketch \ No newline at end of file diff --git a/examples/three-tier.svg b/examples/three-tier.svg index 1353633..0e050f3 100644 --- a/examples/three-tier.svg +++ b/examples/three-tier.svg @@ -1 +1 @@ -ClientEdgeApplicationDataBrowserSPA + traceparentCDNTLS · cacheAPI GatewayOIDC · rate-limitWeb tierNext.js · React 19API tierGo · chi · pgxPostgresOLTPRediscache · queuesObject storageblobsHTTPSSR · formsMade with Plinth Sketch \ No newline at end of file +ClientEdgeApplicationDataBrowserSPA + traceparentCDNTLS · cacheAPI GatewayOIDC · rate-limitWeb tierNext.js · React 19API tierGo · chi · pgxPostgresOLTPRediscache · queuesObject storageblobsHTTPSSR · formsMade with Plinth Sketch \ No newline at end of file diff --git a/package.json b/package.json index 56f3e2c..c247026 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@plinth-dev/sketch", - "version": "0.1.0", + "version": "0.1.1", "description": "Render Plinth Sketch DSL files to typeset SVG architecture diagrams. Drop the DSL in your repo, render at build time, embed the SVG in your README.", "license": "MIT", "type": "module", diff --git a/src/layout.js b/src/layout.js index fb97b5c..7948dc0 100644 --- a/src/layout.js +++ b/src/layout.js @@ -14,10 +14,9 @@ const NODE_H = 64; const NODE_GAP_X = 32; const NODE_MIN_W = 140; const TOTAL_W_DEFAULT = 960; -const TOTAL_W_MAX = 1600; - export function layout(parsed) { // First pass: figure out the canvas width we need to honour NODE_MIN_W. + // No upper cap — clipping nodes off-canvas is worse than a wide diagram. let canvasW = TOTAL_W_DEFAULT; for (const layer of parsed.layers) { if (!layer.ids.length) continue; @@ -25,7 +24,7 @@ export function layout(parsed) { const totalGap = NODE_GAP_X * (n - 1); const neededInner = LAYER_INTERNAL_PAD_X * 2 + totalGap + NODE_MIN_W * n; const neededTotal = neededInner + PADDING_X * 2; - if (neededTotal > canvasW) canvasW = Math.min(neededTotal, TOTAL_W_MAX); + if (neededTotal > canvasW) canvasW = neededTotal; } const innerW = canvasW - PADDING_X * 2; diff --git a/src/parser.js b/src/parser.js index 8482a33..d801281 100644 --- a/src/parser.js +++ b/src/parser.js @@ -7,8 +7,9 @@ // a -> b : edge label — directed edge (label optional) // @cite text — bottom-right cite text (optional) // -// The parser is intentionally permissive: unknown lines surface as errors[] -// but don't abort the parse. Whitespace tolerant. CRLF tolerant. +// The parser is intentionally permissive: unparsed lines surface in errors[] +// and edges referencing undefined nodes surface in warnings[] — neither +// aborts the parse. Whitespace and CRLF tolerant. const NODE_RE = /^([\w\-./]+)\s*:\s*(.+?)(?:\s+\/\s+(.+))?$/; const EDGE_RE = /^([\w\-./]+)\s*->\s*([\w\-./]+)\s*(?::\s*(.+))?$/; @@ -20,6 +21,7 @@ export function parse(text) { const layers = []; const edges = []; const errors = []; + const warnings = []; let cite = null; let currentLayerName = null; @@ -59,7 +61,7 @@ export function parse(text) { const e = line.match(EDGE_RE); if (e) { - edges.push({ from: e[1], to: e[2], label: e[3] || null }); + edges.push({ from: e[1], to: e[2], label: e[3] || null, line: i + 1 }); continue; } @@ -72,5 +74,27 @@ export function parse(text) { errors.push({ line: i + 1, text: raw }); } - return { nodes, nodeOrder, layers, edges, cite, errors }; + // Surface edges that reference undefined nodes as warnings. Self-loops + // also surface as warnings since they aren't currently rendered. + const missing = new Set(); + for (const e of edges) { + if (!nodes[e.from]) missing.add(e.from); + if (!nodes[e.to]) missing.add(e.to); + if (e.from === e.to) { + warnings.push({ + line: e.line, + kind: "self-loop", + message: `edge ${e.from} -> ${e.to} is a self-loop and is not rendered`, + }); + } + } + if (missing.size) { + warnings.push({ + kind: "undefined-nodes", + ids: [...missing], + message: `undefined node${missing.size === 1 ? "" : "s"}: ${[...missing].join(", ")}`, + }); + } + + return { nodes, nodeOrder, layers, edges, cite, errors, warnings }; } diff --git a/src/renderer.js b/src/renderer.js index 8efcd66..209d13b 100644 --- a/src/renderer.js +++ b/src/renderer.js @@ -17,7 +17,7 @@ const PALETTE = { }; const FONT_SANS = "'IBM Plex Sans', -apple-system, BlinkMacSystemFont, sans-serif"; -const FONT_MONO = "'JetBrains Mono', 'IBM Plex Mono', ui-monospace, 'SF Mono', Menlo, monospace"; +const FONT_MONO = "'JetBrains Mono', ui-monospace, 'SF Mono', Menlo, monospace"; function escapeXml(s) { return String(s) @@ -28,29 +28,51 @@ function escapeXml(s) { .replace(/'/g, "'"); } +// Deterministic short hash of a string. Used for per-render marker / class +// suffixes so the same DSL renders byte-stable across runs while two SVGs +// embedded on one page don't collide on `` or scoped CSS. +function shortHash(s) { + let h = 5381; + for (let i = 0; i < s.length; i++) h = ((h << 5) + h + s.charCodeAt(i)) | 0; + return (h >>> 0).toString(36).slice(0, 8); +} + export function render(parsed, lay, options = {}) { - const { embedFonts = true, signature = true } = options; + const { signature = true } = options; const { width: W, height: H } = lay; + // Per-render namespace for SVG ids/classes — lets multiple Plinth Sketch + // SVGs coexist on the same HTML page without `` collisions + // or CSS class bleed. + const ns = "ps-" + shortHash(JSON.stringify({ + nodes: parsed.nodeOrder, + edges: parsed.edges, + layers: parsed.layers.map((l) => l.name), + cite: parsed.cite, + })); + const arr = `${ns}-arr`; + const arrMute = `${ns}-arr-mute`; + const klass = ns; + const styles = ` -.plinth-topo { font-family: ${FONT_SANS}; } -.plinth-topo .layer-rect { fill: ${PALETTE.bgSoft}; stroke: ${PALETTE.ruleFirm}; stroke-width: 1; stroke-dasharray: 4 3; } -.plinth-topo .comp-rect { fill: ${PALETTE.bg}; stroke: ${PALETTE.accent}; stroke-width: 1; } -.plinth-topo .conn { stroke: ${PALETTE.accent}; stroke-width: 1; fill: none; } -.plinth-topo .conn-mute { stroke: ${PALETTE.accentMute}; stroke-width: 1; fill: none; } -.plinth-topo text { fill: ${PALETTE.ink}; } -.plinth-topo .lbl { font-family: ${FONT_SANS}; font-weight: 500; font-size: 14px; fill: ${PALETTE.ink}; } -.plinth-topo .lbl-sub { font-family: ${FONT_MONO}; font-size: 11px; fill: ${PALETTE.inkMute}; letter-spacing: 0.02em; } -.plinth-topo .layer-label { font-family: ${FONT_SANS}; font-size: 12px; font-weight: 600; fill: ${PALETTE.inkMute}; letter-spacing: 0.04em; } -.plinth-topo .conn-label { font-family: ${FONT_MONO}; font-size: 10.5px; fill: ${PALETTE.inkSoft}; letter-spacing: 0.04em; } -.plinth-topo .made-with { font-family: ${FONT_SANS}; font-size: 11px; fill: ${PALETTE.inkSoft}; } +.${klass} { font-family: ${FONT_SANS}; } +.${klass} .layer-rect { fill: ${PALETTE.bgSoft}; stroke: ${PALETTE.ruleFirm}; stroke-width: 1; stroke-dasharray: 4 3; } +.${klass} .comp-rect { fill: ${PALETTE.bg}; stroke: ${PALETTE.accent}; stroke-width: 1; } +.${klass} .conn { stroke: ${PALETTE.accent}; stroke-width: 1; fill: none; } +.${klass} .conn-mute { stroke: ${PALETTE.accentMute}; stroke-width: 1; fill: none; } +.${klass} text { fill: ${PALETTE.ink}; } +.${klass} .lbl { font-family: ${FONT_SANS}; font-weight: 500; font-size: 14px; fill: ${PALETTE.ink}; } +.${klass} .lbl-sub { font-family: ${FONT_MONO}; font-size: 11px; fill: ${PALETTE.inkMute}; letter-spacing: 0.02em; } +.${klass} .layer-label { font-family: ${FONT_SANS}; font-size: 12px; font-weight: 600; fill: ${PALETTE.inkMute}; letter-spacing: 0.04em; } +.${klass} .conn-label { font-family: ${FONT_MONO}; font-size: 10.5px; fill: ${PALETTE.inkSoft}; letter-spacing: 0.04em; } +.${klass} .made-with { font-family: ${FONT_SANS}; font-size: 11px; fill: ${PALETTE.inkSoft}; } `; - let svg = ``; + let svg = ``; svg += ``; svg += ``; - svg += ``; - svg += ``; + svg += ``; + svg += ``; svg += ``; // Background @@ -75,8 +97,22 @@ export function render(parsed, lay, options = {}) { } } - // Edges — orthogonal L-paths + // For multi-edge / cyclic-edge offset: count occurrences of each + // unordered node-pair encountered so far and bump per-edge offset + // perpendicular to the natural path direction. + const pairSeen = new Map(); + function pairOffset(a, b) { + const key = [a, b].sort().join("\x00"); + const n = (pairSeen.get(key) || 0) + 1; + pairSeen.set(key, n); + return n - 1; // 0 for the first edge in the pair, 1 for the second, etc. + } + + // Edges — orthogonal L-paths. Self-loops are skipped (surfaced as warnings + // in parser). Same-row edges that would skip over an intervening node + // route up-and-over. for (const e of parsed.edges) { + if (e.from === e.to) continue; const from = parsed.nodes[e.from]; const to = parsed.nodes[e.to]; if (!from || !to || from._x === undefined || to._x === undefined) continue; @@ -91,42 +127,78 @@ export function render(parsed, lay, options = {}) { const fromRight = from._x + from._w; const toLeft = to._x; + const sameRow = Math.abs(fromCy - toCy) < 4; + + // Detect intervening same-row node between from and to (sorted by x). + let hasIntervening = false; + if (sameRow && from.layer && from.layer === to.layer) { + const lo = Math.min(fromCx, toCx); + const hi = Math.max(fromCx, toCx); + for (const id of parsed.nodeOrder) { + if (id === e.from || id === e.to) continue; + const m = parsed.nodes[id]; + if (m._x === undefined) continue; + const mCx = m._x + m._w / 2; + const mCy = m._y + m._h / 2; + if (Math.abs(mCy - fromCy) < 4 && mCx > lo && mCx < hi) { + hasIntervening = true; + break; + } + } + } + + // Per-pair offset for multi/cyclic edges. We perpendicular-shift the + // path's mid-section to keep duplicates and reverse-direction edges + // visually distinct. + const offIdx = pairOffset(e.from, e.to); + const offStep = 14; + const offDir = e.from < e.to ? 1 : -1; // stable direction so a→b and b→a stack on opposite sides + const off = offIdx * offStep * offDir; + let path; let labelX, labelY; let cls = "conn"; - // True if the label sits in clear space (above/below boxes) and doesn't - // need a background pill to clear overlapping text. let labelInClearSpace = false; - if (Math.abs(fromCy - toCy) < 4 && fromCx < toCx) { - // Horizontal, same row, left → right. + if (sameRow && hasIntervening) { + // Up-and-over: route above the row, clear of intervening nodes. + const clearY = from._y - 28 - Math.abs(off); + const lx = fromCx < toCx ? fromRight : from._x; + const rx = fromCx < toCx ? toLeft : toLeft + to._w; + path = `M ${lx} ${fromCy} L ${lx} ${clearY} L ${rx} ${clearY} L ${rx} ${toCy}`; + labelX = (lx + rx) / 2; + labelY = clearY - 6; + cls = "conn-mute"; + labelInClearSpace = true; + } else if (sameRow && fromCx < toCx) { + // Horizontal, same row, left → right (no intervening node). // Float the label ABOVE the boxes — the gap between boxes is often // narrower than the label, so a pill in the gap would clip adjacent - // box text. + // box text. `off` lifts duplicates further above. path = `M ${fromRight} ${fromCy} L ${toLeft} ${toCy}`; labelX = (fromRight + toLeft) / 2; - labelY = from._y - 6; + labelY = from._y - 6 - Math.abs(off); cls = "conn-mute"; labelInClearSpace = true; - } else if (Math.abs(fromCy - toCy) < 4 && fromCx > toCx) { + } else if (sameRow && fromCx > toCx) { path = `M ${from._x} ${fromCy} L ${toLeft + to._w} ${toCy}`; labelX = (from._x + toLeft + to._w) / 2; - labelY = from._y - 6; + labelY = from._y - 6 - Math.abs(off); cls = "conn-mute"; labelInClearSpace = true; } else if (fromBottom < toTop) { - const midY = (fromBottom + toTop) / 2; + const midY = (fromBottom + toTop) / 2 + off; if (Math.abs(fromCx - toCx) < 4) { path = `M ${fromCx} ${fromBottom} L ${fromCx} ${toTop}`; labelX = fromCx + 6; - labelY = midY; + labelY = (fromBottom + toTop) / 2; } else { path = `M ${fromCx} ${fromBottom} L ${fromCx} ${midY} L ${toCx} ${midY} L ${toCx} ${toTop}`; labelX = (fromCx + toCx) / 2; labelY = midY - 4; } } else if (toTop < fromBottom && from._y > to._y) { - const midY = (from._y + (to._y + to._h)) / 2; + const midY = (from._y + (to._y + to._h)) / 2 + off; path = `M ${fromCx} ${from._y} L ${fromCx} ${midY} L ${toCx} ${midY} L ${toCx} ${to._y + to._h}`; labelX = (fromCx + toCx) / 2; labelY = midY - 4; @@ -136,8 +208,8 @@ export function render(parsed, lay, options = {}) { labelY = (fromCy + toCy) / 2; } - const markerId = cls === "conn" ? "plinth-arr" : "plinth-arr-mute"; - svg += ``; + const markerRef = cls === "conn" ? arr : arrMute; + svg += ``; if (e.label) { const labelW = e.label.length * 6.4 + 8; @@ -148,11 +220,15 @@ export function render(parsed, lay, options = {}) { } } - // Cite (bottom-right) - if (parsed.cite) { - svg += `${escapeXml(parsed.cite)}`; - } else if (signature) { - svg += `Made with Plinth Sketch`; + // Cite (bottom-right) — suppress when there are no nodes drawn, otherwise + // the SVG looks like an empty signed canvas. + const hasContent = parsed.nodeOrder.some((id) => parsed.nodes[id]._x !== undefined); + if (hasContent) { + if (parsed.cite) { + svg += `${escapeXml(parsed.cite)}`; + } else if (signature) { + svg += `Made with Plinth Sketch`; + } } svg += ""; diff --git a/test/parser.test.js b/test/parser.test.js index 270db9d..dbc1d12 100644 --- a/test/parser.test.js +++ b/test/parser.test.js @@ -17,7 +17,9 @@ test("parses a node without sublabel", () => { test("parses a labelled edge", () => { const r = parse(`@layer Test\na : A\nb : B\na -> b : http`); assert.equal(r.edges.length, 1); - assert.deepEqual(r.edges[0], { from: "a", to: "b", label: "http" }); + assert.equal(r.edges[0].from, "a"); + assert.equal(r.edges[0].to, "b"); + assert.equal(r.edges[0].label, "http"); }); test("parses an unlabelled edge", () => { @@ -25,6 +27,21 @@ test("parses an unlabelled edge", () => { assert.equal(r.edges[0].label, null); }); +test("edges to undefined nodes surface as warnings", () => { + const r = parse(`@layer T\na : A\na -> ghost`); + assert.equal(r.errors.length, 0); + assert.ok(r.warnings.length > 0); + const w = r.warnings.find((w) => w.kind === "undefined-nodes"); + assert.ok(w, "expected an undefined-nodes warning"); + assert.deepEqual(w.ids, ["ghost"]); +}); + +test("self-loop edges surface as warnings", () => { + const r = parse(`@layer T\na : A\na -> a`); + const w = r.warnings.find((w) => w.kind === "self-loop"); + assert.ok(w, "expected a self-loop warning"); +}); + test("ignores comments and blank lines", () => { const r = parse(`# top comment\n\n@layer Test\n# inside\na : A\n`); assert.equal(r.nodeOrder.length, 1); diff --git a/test/render.test.js b/test/render.test.js index 83c5949..fedd163 100644 --- a/test/render.test.js +++ b/test/render.test.js @@ -6,7 +6,39 @@ test("end-to-end produces an SVG document", () => { const svg = sketch(`@layer A\na : A\nb : B\na -> b : edge`); assert.match(svg, /^$/); - assert.match(svg, /class="plinth-topo"/); + // Class is per-render-namespaced (ps-) so multiple SVGs on one page + // don't collide on or scoped CSS. + assert.match(svg, /class="ps-[a-z0-9]+"/); +}); + +test("two renders of the same DSL produce byte-identical output", () => { + const a = sketch(`@layer T\nx : X\ny : Y\nx -> y`); + const b = sketch(`@layer T\nx : X\ny : Y\nx -> y`); + assert.equal(a, b); +}); + +test("two renders of different DSLs use different class namespaces", () => { + const a = sketch(`@layer T\nx : X`); + const b = sketch(`@layer T\ny : Y`); + const nsA = (a.match(/class="(ps-[a-z0-9]+)"/) || [])[1]; + const nsB = (b.match(/class="(ps-[a-z0-9]+)"/) || [])[1]; + assert.ok(nsA && nsB && nsA !== nsB); +}); + +test("self-loop edges are skipped (warned-but-not-rendered)", () => { + const svg = sketch(`@layer T\na : A\na -> a : self`); + // No path element should be emitted for the self-loop. + const pathCount = (svg.match(/ too; just check connection paths via + // the conn class. + const connCount = (svg.match(/class="conn"/g) || []).length + + (svg.match(/class="conn-mute"/g) || []).length; + assert.equal(connCount, 0); +}); + +test("cite is suppressed when no nodes render", () => { + const svg = sketch(`# only comments\n# no nodes`); + assert.ok(!svg.includes("Made with Plinth Sketch")); }); test("escapes HTML-significant characters in labels", () => {