From 65a1538d537de2e44fba0a237fac5b01ff364503 Mon Sep 17 00:00:00 2001 From: Eric Minish Date: Sun, 19 Jul 2026 00:09:06 -0400 Subject: [PATCH 1/2] Make --check graphviz-version-independent; guard both committed diagrams in CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --check byte-compared the rendered SVG, so it false-failed whenever the committed image and the checking machine used different graphviz versions (committed diagrams are graphviz 15.0.0; ubuntu-latest apt is 2.42.2) — which made the drift-guard unusable in CI, the very place it was built for. For svg output, --check now compares the graph STRUCTURE (the set of node ids and directed edges, parsed from the SVG's elements) instead of raw bytes: version-independent, order-insensitive (codegraph's enumeration order isn't guaranteed stable run-to-run), and it fails only on structural drift — a caller/callee/edge appearing or disappearing — not cosmetic recolors. Non-svg formats keep the byte-compare and its same-graphviz-version caveat. A new CI `diagrams` job dogfoods this: installs codegraph, builds a fresh index, and runs --check on both committed diagrams, so a PR that changes the code but not the diagram fails. Building it immediately caught real rot: docs/buildDot-callgraph.svg was stale, missing the nodeIdentities node/edge that #13 added to buildDot. Regenerated it, wrapped it in codeshot:buildDot markers so it's guardable, and fixed the now-wrong "five callees" prose in TECHNICAL.md. --- .github/workflows/ci.yml | 34 ++++++++++ TECHNICAL.md | 8 ++- USAGE.md | 14 +++-- docs/buildDot-callgraph.svg | 120 +++++++++++++++++++++++------------- render/callgraph.js | 67 +++++++++++++++++++- test/run.js | 62 +++++++++++++++++++ 6 files changed, 255 insertions(+), 50 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5f998d1..5850af0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -30,3 +30,37 @@ jobs: - name: Install graphviz (codeshot renders via `dot`) run: sudo apt-get update && sudo apt-get install -y graphviz - run: npm test + + diagrams: + name: Diagrams are current (codeshot --check) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 + with: + node-version: "20" + + # codeshot renders through graphviz. --check compares diagram STRUCTURE + # (node/edge set) for svg, not rendered bytes, so the CI graphviz version + # need not match the one that generated the committed image. + - name: Install graphviz + run: sudo apt-get update && sudo apt-get install -y graphviz + + # The index codeshot reads. Pinned (repo convention: pin dependency versions). + - name: Install codegraph + run: npm install -g @colbymchenry/codegraph@1.4.1 + + # Full rebuild from scratch → a clean, complete index (0 unresolved refs). + - name: Build the codegraph index + run: | + codegraph index . + codegraph status . # visibility: a partial index prints an unresolved-refs warning + + # Blocking gate: fail the PR if either committed diagram no longer matches + # what the current code produces. Structural compare = graphviz-version proof. + - name: Check committed diagrams are current + run: | + node render/callgraph.js --architecture --path . --format svg \ + --out docs/architecture.svg --embed TECHNICAL.md --check + node render/callgraph.js buildDot --path . --format svg \ + --out docs/buildDot-callgraph.svg --embed TECHNICAL.md --check diff --git a/TECHNICAL.md b/TECHNICAL.md index e72d006..6c41616 100644 --- a/TECHNICAL.md +++ b/TECHNICAL.md @@ -105,11 +105,13 @@ See [README.md](README.md#design-decisions)'s "Design decisions" section for why ### Example: Codeshot, drawn by Codeshot -The diagram below is not hand-drawn — it was produced by running Codeshot on its own CodeGraph index (`codeshot buildDot --path . --format svg --out docs/buildDot-callgraph.svg`) and committed verbatim. It draws the call trail of `buildDot`, the pure heart of the tool: +The diagram below is not hand-drawn — it was produced by running Codeshot on its own CodeGraph index (`codeshot buildDot --path . --format svg --out docs/buildDot-callgraph.svg --embed TECHNICAL.md`) and committed verbatim. It draws the call trail of `buildDot`, the pure heart of the tool: -![Call graph of buildDot, generated by Codeshot](docs/buildDot-callgraph.svg) +<!-- codeshot:buildDot:start --> +![buildDot call graph — generated by codeshot](docs/buildDot-callgraph.svg) +<!-- codeshot:buildDot:end --> -It doubles as a live legend for the [Visual Encoding](#visual-encoding) rules below. `main` calls `buildDot` at a real call site, so that edge is a solid indigo arrow. `test/run.js` only does `require('./render/callgraph.js')` — a module-level import, not a function-level call — so CodeGraph reports it as `"kind":"file"` and Codeshot draws it dotted/gray, labeled `file`, rather than pretending it's a confirmed call. (That file-kind styling wins even though `run.js` is a test file, which would otherwise be dashed — the precedence rule described in [Visual Encoding](#visual-encoding).) On the right, the five callees are the pure helpers `buildDot` composes to turn caller/callee arrays into a DOT string. Regenerate it any time with the command above. +It doubles as a live legend for the [Visual Encoding](#visual-encoding) rules below. `main` calls `buildDot` at a real call site, so that edge is a solid indigo arrow. `test/run.js` only does `require('./render/callgraph.js')` — a module-level import, not a function-level call — so CodeGraph reports it as `"kind":"file"` and Codeshot draws it dotted/gray, labeled `file`, rather than pretending it's a confirmed call. (That file-kind styling wins even though `run.js` is a test file, which would otherwise be dashed — the precedence rule described in [Visual Encoding](#visual-encoding).) On the right, the callees are the pure helpers `buildDot` composes to turn caller/callee arrays into a DOT string. Regenerate it any time with the command above; `--check` (see [USAGE.md](USAGE.md)) fails CI if the code drifts from this committed picture. **Why is `buildDot` exported from a file that also runs as a CLI?** `render/callgraph.js` guards its `main()` call with `if (require.main === module)`, so `node render/callgraph.js <symbol>` still runs the CLI, but `require('./render/callgraph.js')` (used by `test/run.js`) gets `{ buildDot, isTestRef }` without executing anything. This keeps the test suite dependency-free — no test framework, no mocking of `execFileSync`. diff --git a/USAGE.md b/USAGE.md index 57aae31..1c68a95 100644 --- a/USAGE.md +++ b/USAGE.md @@ -84,10 +84,16 @@ codeshot --architecture --path . --embed TECHNICAL.md --format svg --check ``` Drop that into CI or a pre-commit hook to fail the build when someone changes -the code but not the diagram. One caveat, inherent to any "regenerate and diff -a binary artifact" check: it compares rendered bytes, so CI must use the same -`graphviz` version that generated the committed image, or it will report a -spurious mismatch. +the code but not the diagram. For `svg` output (the recommended `--embed` +format) `--check` compares the diagram's **structure** — the set of nodes and +call edges — not the raw rendered bytes, so it is **graphviz-version +independent**: the committed image and the CI machine can run different +`graphviz` builds without a spurious mismatch. By design it only fails on +*structural* drift (a caller/callee/edge appearing or disappearing); a +cosmetic-only change with the identical graph — e.g. a re-color — is not +flagged. Non-`svg` formats (png, svgz, …) have no recoverable structure and +fall back to a raw byte-compare, which does require CI to use the same +`graphviz` version that generated the committed image. ## What to Do When Something Breaks diff --git a/docs/buildDot-callgraph.svg b/docs/buildDot-callgraph.svg index 3be60a4..55b2bce 100644 --- a/docs/buildDot-callgraph.svg +++ b/docs/buildDot-callgraph.svg @@ -4,101 +4,137 @@ <!-- Generated by graphviz version 15.0.0 (20260523.1842) --> <!-- Title: callgraph Pages: 1 --> -<svg width="450pt" height="309pt" - viewBox="0.00 0.00 450.00 309.00" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"> -<g id="graph0" class="graph" transform="scale(1 1) rotate(0) translate(14.4 294.4)"> +<svg width="450pt" height="370pt" + viewBox="0.00 0.00 450.00 370.00" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"> +<g id="graph0" class="graph" transform="scale(1 1) rotate(0) translate(14.4 355.4)"> <title>callgraph - + buildDot - -buildDot + +buildDot dedupeNodes - -dedupeNodes + + +dedupeNodes + + buildDot->dedupeNodes - - + + dedupeEdges - -dedupeEdges + + +dedupeEdges + + buildDot->dedupeEdges - - + + allocateRenderBudget - -allocateRenderBudget + + +allocateRenderBudget + + buildDot->allocateRenderBudget - - + + - + +nodeIdentities + + +nodeIdentities + + + + + +buildDot->nodeIdentities + + + + + edgeStyleAttrs + edgeStyleAttrs + + - + buildDot->edgeStyleAttrs - - + + - + depthColor + depthColor + + - + buildDot->depthColor - + - + -main - -main +run.js + + +run.js + - + + -main->buildDot - - +run.js->buildDot + + +file - + -run.js - -run.js +main + + +main + - + + -run.js->buildDot - - -file +main->buildDot + + diff --git a/render/callgraph.js b/render/callgraph.js index f3ac2b4..0e08d8c 100755 --- a/render/callgraph.js +++ b/render/callgraph.js @@ -28,6 +28,13 @@ const DEFAULT_LIMIT = 50; // format ignores them, so buildDot is told to emit them only for these. const SVG_TOOLTIP_FORMATS = new Set(['svg', 'svgz']); +// --check compares the graph STRUCTURE (node/edge set) rather than raw image +// bytes for these formats — see svgStructure. Only plain svg qualifies: it's +// text we can parse, and its s carry the semantic ids version-independently. +// svgz (gzipped) and raster formats have no recoverable structure, so they fall +// back to the byte-compare and its same-graphviz-version caveat. +const STRUCTURAL_CHECK_FORMATS = new Set(['svg']); + // Multi-hop traversal (--depth > 1) makes one sequential codegraph call per // newly discovered node, per hop — on a well-connected symbol that fans out // fast. This caps total discovered nodes across both directions combined so @@ -562,6 +569,50 @@ function renderDotToBuffer(dot, format) { } } +// graphviz XML-encodes a few characters inside node/edge <title>s — notably the +// '->' of an edge id becomes '->' — so a parsed title has to be decoded +// back to the raw id codeshot wrote into the DOT. Handles the named entities +// graphviz emits plus numeric ones; '&' is undone last so an already-encoded +// '&lt;' doesn't get double-decoded. +function decodeXmlEntities(s) { + return String(s) + .replace(/&#[xX]([0-9a-fA-F]+);/g, (_, h) => String.fromCharCode(parseInt(h, 16))) + .replace(/&#(\d+);/g, (_, d) => String.fromCharCode(Number(d))) + .replace(/</g, '<') + .replace(/>/g, '>') + .replace(/"/g, '"') + .replace(/'/g, "'") + .replace(/&/g, '&'); +} + +// The version-independent heart of --check for svg output: reduce a graphviz SVG +// to just its call STRUCTURE — the set of node ids and the set of directed edges +// — discarding layout coordinates, colors, fonts, and the graphviz version stamp, +// none of which are semantic drift. graphviz renders every node/edge as a +// `<g class="node|edge">` whose `<title>` is verbatim the id codeshot put in the +// DOT (a node's id, or an edge's "from->to"); that title text is identical across +// graphviz versions even though the surrounding geometry is not — which is exactly +// why a byte-compare of the rendered svg false-failed in CI whenever the committed +// image and the checking machine used different graphviz builds. Returns a +// canonical signature (nodes sorted, then edges sorted) so two svgs with the same +// graph but a different node emission ORDER — codegraph's enumeration order isn't +// guaranteed stable run-to-run — still compare equal. Cosmetic drift (a test edge +// losing its dash, a recolor) is deliberately NOT captured: only a node or a call +// appearing or disappearing changes the structure and fails the check. The regex +// tolerates either attribute order (`id` before or after `class`), which also +// varies by graphviz version, and skips the graph's own top-level <title>. +function svgStructure(svg) { + const nodes = new Set(); + const edges = new Set(); + const re = /<g\b[^>]*\bclass="(node|edge)"[^>]*>\s*<title>([\s\S]*?)<\/title>/g; + let m; + while ((m = re.exec(String(svg))) !== null) { + const title = decodeXmlEntities(m[2].trim()); + (m[1] === 'node' ? nodes : edges).add(title); + } + return `nodes:\n${[...nodes].sort().join('\n')}\nedges:\n${[...edges].sort().join('\n')}`; +} + // --embed keeps a generated diagram inside a committed markdown doc, refreshed // in place — the same idempotent HTML-comment-marker pattern doctoc and // terraform-docs use. Each embed is keyed by an id (`arch`, or the symbol @@ -635,7 +686,20 @@ function finishOutput(dot, { format, outFile, embedFile, check, markerId, alt }) const fresh = renderDotToBuffer(dot, format); let committed = null; try { committed = fs.readFileSync(outFile); } catch { /* missing → stale */ } - const imageStale = !committed || !committed.equals(fresh); + // svg: compare the graph structure (node/edge set), which is graphviz-version + // independent, so a committed image rendered by one graphviz build and a fresh + // render on another (e.g. a laptop vs CI) don't false-drift on layout/version + // bytes — the reason a raw byte-compare made --check unusable in CI. Other + // formats have no recoverable structure and keep the byte-compare (and its + // documented same-graphviz-version caveat). + let imageStale; + if (!committed) { + imageStale = true; + } else if (STRUCTURAL_CHECK_FORMATS.has(format.toLowerCase())) { + imageStale = svgStructure(committed.toString('utf8')) !== svgStructure(fresh.toString('utf8')); + } else { + imageStale = !committed.equals(fresh); + } const docStale = docContent !== expected; if (imageStale || docStale) { if (imageStale) console.error(`codeshot: --check: diagram '${outFile}' is out of date (${committed ? 'differs from a fresh render' : 'missing'}) — rerun 'codeshot ... --embed ${embedFile}' and commit the result.`); @@ -880,4 +944,5 @@ module.exports = { filterCallableSymbols, symbolBudgetWarning, duplicateNameWarning, aggregateFileEdges, topFilesByWeight, buildArchitectureDot, architectureOutputBaseName, applyEmbed, embedMarkers, embedRelLink, parseUnresolvedRefs, + svgStructure, decodeXmlEntities, }; diff --git a/test/run.js b/test/run.js index ef31010..23b3b7d 100644 --- a/test/run.js +++ b/test/run.js @@ -8,6 +8,7 @@ const { filterCallableSymbols, symbolBudgetWarning, duplicateNameWarning, aggregateFileEdges, topFilesByWeight, buildArchitectureDot, architectureOutputBaseName, applyEmbed, embedMarkers, embedRelLink, parseUnresolvedRefs, + svgStructure, decodeXmlEntities, } = require('../render/callgraph.js'); let passed = 0; @@ -774,6 +775,67 @@ test('CLI --embed into a nonexistent doc is rejected (refresh, not create)', () assert.strictEqual(threw, true, 'expected --embed into a missing doc to be rejected'); }); +// --- svgStructure: version-independent --check comparison ------------- + +test('decodeXmlEntities undoes the entities graphviz emits in a <title>', () => { + assert.strictEqual(decodeXmlEntities('a->b'), 'a->b'); + assert.strictEqual(decodeXmlEntities('x && y'), 'x && y'); + assert.strictEqual(decodeXmlEntities('<tag> "q" 'a''), '<tag> "q" \'a\''); + assert.strictEqual(decodeXmlEntities('AB'), 'AB'); + // '&' undone last so an already-encoded entity isn't double-decoded + assert.strictEqual(decodeXmlEntities('&lt;'), '<'); +}); + +// A minimal graphviz-shaped svg: the graph carries its own <title>, then each +// node/edge is a <g class="node|edge"> whose <title> is the id codeshot wrote. +const SVG_FIXTURE = `<?xml version="1.0"?> +<svg><g id="graph0" class="graph"><title>callgraph +main +buildDot +main->buildDot +`; + +test('svgStructure extracts node/edge titles and skips the graph title', () => { + assert.strictEqual( + svgStructure(SVG_FIXTURE), + 'nodes:\nbuildDot\nmain\nedges:\nmain->buildDot' + ); +}); + +test('svgStructure is order-insensitive (reordered nodes → same signature)', () => { + const reordered = SVG_FIXTURE + .replace('main\n', '') + .replace('buildDot', + 'buildDot\nmain'); + assert.strictEqual(svgStructure(reordered), svgStructure(SVG_FIXTURE)); +}); + +test('svgStructure tolerates class-before-id attribute order (graphviz version drift)', () => { + const flipped = SVG_FIXTURE.replace(//g, ''); + assert.strictEqual(svgStructure(flipped), svgStructure(SVG_FIXTURE)); +}); + +test('svgStructure ignores cosmetic/version bytes but catches a dropped node or edge', () => { + // version stamp + coordinate jitter must NOT change the signature + const cosmetic = `\n` + + SVG_FIXTURE.replace('', ''); + assert.strictEqual(svgStructure(cosmetic), svgStructure(SVG_FIXTURE)); + // but removing the edge is real drift + const noEdge = SVG_FIXTURE.replace(/\n/, ''); + assert.notStrictEqual(svgStructure(noEdge), svgStructure(SVG_FIXTURE)); +}); + +test('svgStructure matches real graphviz output (buildDot rendered via dot -Tsvg)', () => { + const { execFileSync } = require('child_process'); + const dot = buildDot('Target', [{ name: 'caller', filePath: 'a.js' }], [{ name: 'callee', filePath: 'b.js' }]); + const svg = execFileSync('dot', ['-Tsvg'], { input: dot, encoding: 'utf8' }); + const sig = svgStructure(svg); + // every node the DOT declared appears, and both directed edges are recovered + for (const n of ['Target', 'caller', 'callee']) assert.match(sig, new RegExp(`(^|\\n)${n}(\\n|$)`)); + assert.match(sig, /caller->Target/); + assert.match(sig, /Target->callee/); +}); + // --- index health check (parseUnresolvedRefs) ------------------------- test('parseUnresolvedRefs extracts the count from codegraph status, comma- and ANSI-tolerant', () => { From 939c09ef30b6a8f4af30baa6cc7a440a84f3806e Mon Sep 17 00:00:00 2001 From: Eric Minish Date: Sun, 19 Jul 2026 00:12:26 -0400 Subject: [PATCH 2/2] ci: use 'codegraph init' (builds initial index in fresh checkout) not 'index' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 'codegraph index' rebuilds an existing index and errors when the repo was never initialized — which a CI checkout, with no committed .codegraph, never was. The diagrams job hit '✗ CodeGraph not initialized'. 'init' builds the initial index. --- .github/workflows/ci.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5850af0..37d1c14 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -50,10 +50,12 @@ jobs: - name: Install codegraph run: npm install -g @colbymchenry/codegraph@1.4.1 - # Full rebuild from scratch → a clean, complete index (0 unresolved refs). + # `init` builds the initial index in a fresh checkout (`index` rebuilds an + # existing one and errors if the repo was never initialized — which a CI + # checkout, with no committed .codegraph, never was). - name: Build the codegraph index run: | - codegraph index . + codegraph init . codegraph status . # visibility: a partial index prints an unresolved-refs warning # Blocking gate: fail the PR if either committed diagram no longer matches