Make --check graphviz-version-independent; guard both committed diagrams in CI - #19
Merged
Conversation
…ams in CI --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 <title> 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.
… 'index' '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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
--checkbyte-compared the rendered SVG, which is graphviz-version-specific, so it false-failed whenever the committed image and the checking machine used different graphviz builds (this repo's committed diagrams are graphviz 15.0.0;ubuntu-latestapt ships 2.42.2). That made the drift-guard unusable in CI — the exact place it was built for. Its own code even hedged this ("the same-generator caveat every regenerate-and-diff artifact check carries").Fix: for
svgoutput,--checknow compares the graph structure — the sorted set of node ids and directed edges, parsed from the SVG's<title>elements — instead of raw bytes:svgformats keep the byte-compare and its documented same-version caveat.No new flag, no new committed artifact type. ~67 lines (
svgStructure,decodeXmlEntities).Dogfood it — new
diagramsCI jobInstalls
codegraph(pinned@1.4.1), builds a fresh index, and runs--checkon both committed diagrams, so a PR that changes the code but not the diagram fails.It immediately caught real rot
docs/buildDot-callgraph.svgwas already stale — missing thenodeIdentitiesnode/edge that #13 added tobuildDot, because nothing ever re-checked it. Regenerated it, wrapped it incodeshot:buildDotmarkers so it's now guardable, and fixed the now-wrong "five callees" prose inTECHNICAL.md.Tests
83 pass (was 77). New tests cover entity decoding, order-insensitivity, attribute-order tolerance, cosmetic-vs-structural drift, and a real
dot -Tsvgrender whose parsed structure is asserted against the DOT — proving the parser matches actual graphviz output.