Skip to content

Fail a pull request whose benchmark charts drift from their history - #495

Merged
matt-edmondson merged 2 commits into
mainfrom
fix/303-verify-charts
Oct 30, 2026
Merged

matt-edmondson merged 2 commits into
mainfrom
fix/303-verify-charts

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #303

What changed

  • New .github/workflows/verify-charts.yml. It runs on pull requests that touch docs/benchmarks/**, scripts/benchmark-history.cs, the new render script, or either workflow. It redraws all three SVG pairs from the committed history JSON and fails on any byte difference. Untracked output counts as a difference too. Each drifting file gets an ::error file=…:: annotation, followed by a git diff --stat.
  • New scripts/render-benchmark-charts.sh. The render loop moves out of benchmark-history.yml into this script, and both workflows run it. That is the triage's "reuse the render step". The invocation is unchanged: dotnet run scripts/benchmark-history.cs -- render --subject … --history … --out …, with stdin from /dev/null.
  • One subject table. The guard reads SUBJECTS from benchmark-history.yml's env with yq, which ships on the hosted Ubuntu images. That env stays the only place the table is written.
  • CLAUDE.md now mentions the guard in the "committed output" bullet of the benchmarks section.

Verification

Run locally. yq was swapped for an equivalent Python read of the same key, and the rest is the workflow's own step scripts:

  • On current main, all three charts redraw byte-identical and the check passes (exit 0). This is the "passes on main" criterion.
  • With a committed edit to one meanNs in strings-history.json, the check fails (exit 1). It names strings-performance.svg and strings-performance-dark.svg, and only those.
  • Both workflow files parse as YAML. Not run locally: actionlint, and a real Actions run. This PR touches benchmark-history.yml, so the new workflow runs on this PR itself.

Note

#494 (for #320) also edits benchmark-history.yml, in its trigger and job header. That is a different region from the render step this PR replaces, so the two should merge cleanly in either order.


Generated by Claude Code

…303)

Nothing checked that the committed SVGs under docs/benchmarks/ are what
scripts/benchmark-history.cs draws from the committed history JSON, so a
hand-edited history or a renderer change could leave the README charts
wrong without any check failing.

verify-charts.yml runs on pull requests that touch the histories, the
charts, the renderer or the recorder workflow. It redraws every chart and
fails on any difference, naming each drifting file.

The render loop moves out of benchmark-history.yml into
scripts/render-benchmark-charts.sh, and both workflows run it. The guard
reads the subject table from benchmark-history.yml's env, so the guard and
the recorder can't drift apart.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017vfJW2sHrLz9iC1cJzLLZy
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017vfJW2sHrLz9iC1cJzLLZy
@sonarqubecloud

Copy link
Copy Markdown

@matt-edmondson
matt-edmondson merged commit a43b423 into main Oct 30, 2026
17 checks passed
@matt-edmondson
matt-edmondson deleted the fix/303-verify-charts branch October 30, 2026 02:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No verify-charts guard: committed benchmark SVGs can drift from docs/benchmarks/*history.json without any PR failing

2 participants