ci(benchmarks): surface the gate table as a sticky PR comment#148
Merged
Conversation
Design for posting the existing bench-check sweep table as a sticky PR comment (and run Summary) via a new `benchmarks check --markdown` mode and a permission-scoped benchmarks.yml. Gate and baseline unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`benchmarks check --markdown` renders the sweep table plus a pass/fail verdict as a GitHub Markdown document on stdout, so CI can post it as a PR comment. Gate logic and exit codes unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Move the bench job into its own permission-scoped benchmarks.yml, run it via `just bench-report`, tee the markdown to the run Summary, and post it with `gh pr comment --edit-last --create-if-none`. Scopes `pull-requests: write` to the one job instead of widening the reusable _checks.yml token. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
GITHUB_STEP_SUMMARY is unique per step, so the comment step read its own empty summary and gh pr comment silently failed on a blank body. Tee the report to a RUNNER_TEMP scratch file that survives across steps and post that. Add a concurrency group so rapid PR pushes don't race the sticky comment. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
GITHUB_STEP_SUMMARY is per-step, so the comment must read a RUNNER_TEMP scratch file the capture step also tees to, not the summary path. Record accepting the lost weekly scheduled counter gate as the cost of the scoped write token. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Benchmark gate✅ gate passed
Gated (fails the build): |
`docker compose run` leaks buildkit image-build output onto stdout in CI, so the first sticky comment was ~120 lines of build noise before the table. The report always starts with the `## Benchmark gate` heading and the container prints it last, so capture from that sentinel to EOF. Manage the exit code by hand (set +e, re-exit with the gate status) so a failing gate still posts its report yet still fails the build. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The first live PR comment was buried under buildkit build output because `docker compose run` leaks it to stdout in CI. Document slicing the report from its `## Benchmark gate` sentinel and owning the exit code by hand, and correct the earlier claim that -T alone yields clean stdout. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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
Surface the benchmark gate's sweep table as a sticky PR comment (and on the run Summary page), without changing the gate itself. Today
just bench-checkcomputes the table on every PR but it only lives in the CI log.Design + rationale:
planning/changes/2026-07-17.03-benchmark-pr-comment.md.How
benchmarks check --markdown— new output mode; renders the existing sweep + a✅/❌verdict as GitHub Markdown on stdout. Same exit code as before (0 pass / 1 fail), so the gate is untouched. Python owns the whole comment body (unit-tested)..github/workflows/benchmarks.yml— new, permission-scoped (pull-requests: writeonly here). Runsjust bench-report, tees the Markdown to$GITHUB_STEP_SUMMARYand a$RUNNER_TEMPscratch file, and posts it withgh pr comment --edit-last --create-if-none. Thebenchjob moves out of the reusable_checks.ymlso the write token isn't widened across lint/docs/pytest.just bench-report—docker compose run -T ... check --markdown(-Tkeeps stdout clean for capture).modern-di's
benchmark-actionmechanism was deliberately not copied: it gates on wall-clock timing, which this repo's design rejects as flaky. Only the PR comment is ported; the deterministic counter gate stays as-is.Notes
baseline.json,EXACT_KEYS/TOLERANT_KEYS, and the--cov-fail-under=100gate are all unchanged.continue-on-errors and the Summary page still shows the table. With 0 forks today this is theoretical.benchno longer runs on the Monday scheduled dependency-drift check (cost of the scoped token) — see the spec's Non-goals.Testing
tests/test_benchmarks.pypass; ruff + ty clean.just bench-reportexercised in Docker: clean Markdown, exit 0 against the committed baseline.🤖 Generated with Claude Code