ci(bench-canonical): in-place README badge rewrite from cron (#477) - #481
Merged
Conversation
Adds two steps before the existing commit+push: 1. Compute badge text via `python -m benchmarks.badge` against the merged JSON (gated on bench succeeding so an errored run doesn't rewrite the badge to a stale 0/N line). 2. Sync the current main README into the bench-canonical-results worktree and awk-rewrite the marker block. Same commit as the JSON entry; one cron commit per run. main is not touched — operator cherry-picks if they want main current.
Reviewer's GuideWires the nightly bench-canonical cron to compute a reproducibility summary line from the merged benchmark JSON via a new benchmarks.badge module, and rewrites the README badge block in the bench-canonical-results worktree in-place, with unit tests for the badge computation logic. Sequence diagram for bench-canonical cron computing and rewriting README badgesequenceDiagram
actor Maintainer
participant GitHubActions as GitHub_Actions_Cron
participant BenchJob as Bench_Canonical_Job
participant BadgeModule as Benchmarks_Badge_Module
participant GitWorktree as Bench_Canonical_Results_Worktree
participant RemoteRepo as Remote_Repository
Maintainer->>GitHubActions: Schedule nightly bench_canonical workflow
GitHubActions->>BenchJob: Run benchmarks and merge JSON
BenchJob-->>GitHubActions: Set output out = merged_report_path or ''
GitHubActions->>GitHubActions: Check steps.bench.outputs.out != ''
alt Bench_run_succeeded
GitHubActions->>BadgeModule: python -m benchmarks.badge merged_report_path
BadgeModule->>BadgeModule: _count_invocations(headline_cut)
BadgeModule->>BadgeModule: _count_ok(results)
BadgeModule-->>GitHubActions: badge_text
GitHubActions->>GitWorktree: Copy README.md into .bench-results-branch/README.md
GitGitHubActions->>GitWorktree: awk replace text between bench_canonical_badge markers
GitWorktree-->>GitHubActions: Updated README with new badge line
else Bench_run_errored
GitHubActions->>GitHubActions: Skip badge and README rewrite
end
GitHubActions->>GitWorktree: Commit JSON and updated README
GitWorktree->>RemoteRepo: Push bench_canonical_results branch
RemoteRepo-->>Maintainer: Updated README badge visible on bench_canonical_results
Class diagram for benchmarks.badge module structureclassDiagram
class Benchmarks_Badge_Module {
+int _count_invocations(headline_cut: Mapping~str, list~)
+int _count_ok(results: Mapping~str, Mapping~str, Mapping~)
+str compute_badge_text(report_path: Path, today: str)
+int main(argv: list~str~)
}
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
yoshi280
added
author-Gylf
📝 WalkthroughWalkthroughThis PR implements automated reproducibility badge updates in the README from nightly benchmark canonical results. It introduces a badge computation module, workflow steps to sync and rewrite the badge block in README, and comprehensive tests for the badge logic. ChangesBadge Cron Update
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if: steps.bench.outputs.out != '' | ||
| run: | | ||
| set -euo pipefail | ||
| text=$(uv run python -m benchmarks.badge "${{ steps.bench.outputs.out }}") |
| run: | | ||
| set -euo pipefail | ||
| cp README.md .bench-results-branch/README.md | ||
| new="${{ steps.badge.outputs.text }}" awk ' |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The badge calculation currently counts all
results[adapter][sub_key]entries via_count_okwithout consultingheadline_cut, which can diverge from the intended "headline" subset if extra results are present; consider iterating only the (adapter, sub_key) pairs defined inheadline_cutto keep the numerator/denominator semantics aligned with the issue description.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The badge calculation currently counts all `results[adapter][sub_key]` entries via `_count_ok` without consulting `headline_cut`, which can diverge from the intended "headline" subset if extra results are present; consider iterating only the (adapter, sub_key) pairs defined in `headline_cut` to keep the numerator/denominator semantics aligned with the issue description.
## Individual Comments
### Comment 1
<location path=".github/workflows/bench-canonical.yml" line_range="138-148" />
<code_context>
+ run: |
+ set -euo pipefail
+ cp README.md .bench-results-branch/README.md
+ new="${{ steps.badge.outputs.text }}" awk '
+ BEGIN { in_block=0 }
+ /<!-- bench-canonical-badge:start -->/ { print; print ENVIRON["new"]; in_block=1; next }
+ /<!-- bench-canonical-badge:end -->/ { print; in_block=0; next }
+ in_block { next }
+ { print }
+ ' .bench-results-branch/README.md > .bench-results-branch/README.md.new
+ mv .bench-results-branch/README.md.new .bench-results-branch/README.md
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Guard against shell interpretation of special characters in the badge text when passing it through the `new` env var.
Using `new="${{ steps.badge.outputs.text }}" awk ...` assumes the badge text never includes shell‑significant characters (`$`, backticks, backslashes, quotes, etc.). A future change to the badge text could cause the shell to reinterpret it and break this step. To harden this, consider passing the value to `awk` without letting the shell re‑parse it—for example, write the text to a temp file that `awk` reads, or use a safely quoted `printf` and `env NEW_TEXT="..." awk ...` pattern.
```suggestion
run: |
set -euo pipefail
cp README.md .bench-results-branch/README.md
printf '%s\n' "${{ steps.badge.outputs.text }}" > .bench-results-branch/new_badge.txt
awk -v badge_file=".bench-results-branch/new_badge.txt" '
BEGIN { in_block=0 }
/<!-- bench-canonical-badge:start -->/ {
print
while ((getline line < badge_file) > 0) {
print line
}
close(badge_file)
in_block=1
next
}
/<!-- bench-canonical-badge:end -->/ { print; in_block=0; next }
in_block { next }
{ print }
' .bench-results-branch/README.md > .bench-results-branch/README.md.new
mv .bench-results-branch/README.md.new .bench-results-branch/README.md
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+138
to
+148
| run: | | ||
| set -euo pipefail | ||
| cp README.md .bench-results-branch/README.md | ||
| new="${{ steps.badge.outputs.text }}" awk ' | ||
| BEGIN { in_block=0 } | ||
| /<!-- bench-canonical-badge:start -->/ { print; print ENVIRON["new"]; in_block=1; next } | ||
| /<!-- bench-canonical-badge:end -->/ { print; in_block=0; next } | ||
| in_block { next } | ||
| { print } | ||
| ' .bench-results-branch/README.md > .bench-results-branch/README.md.new | ||
| mv .bench-results-branch/README.md.new .bench-results-branch/README.md |
There was a problem hiding this comment.
🚨 suggestion (security): Guard against shell interpretation of special characters in the badge text when passing it through the new env var.
Using new="${{ steps.badge.outputs.text }}" awk ... assumes the badge text never includes shell‑significant characters ($, backticks, backslashes, quotes, etc.). A future change to the badge text could cause the shell to reinterpret it and break this step. To harden this, consider passing the value to awk without letting the shell re‑parse it—for example, write the text to a temp file that awk reads, or use a safely quoted printf and env NEW_TEXT="..." awk ... pattern.
Suggested change
| run: | | |
| set -euo pipefail | |
| cp README.md .bench-results-branch/README.md | |
| new="${{ steps.badge.outputs.text }}" awk ' | |
| BEGIN { in_block=0 } | |
| /<!-- bench-canonical-badge:start -->/ { print; print ENVIRON["new"]; in_block=1; next } | |
| /<!-- bench-canonical-badge:end -->/ { print; in_block=0; next } | |
| in_block { next } | |
| { print } | |
| ' .bench-results-branch/README.md > .bench-results-branch/README.md.new | |
| mv .bench-results-branch/README.md.new .bench-results-branch/README.md | |
| run: | | |
| set -euo pipefail | |
| cp README.md .bench-results-branch/README.md | |
| printf '%s\n' "${{ steps.badge.outputs.text }}" > .bench-results-branch/new_badge.txt | |
| awk -v badge_file=".bench-results-branch/new_badge.txt" ' | |
| BEGIN { in_block=0 } | |
| /<!-- bench-canonical-badge:start -->/ { | |
| while ((getline line < badge_file) > 0) { | |
| print line | |
| } | |
| close(badge_file) | |
| in_block=1 | |
| next | |
| } | |
| /<!-- bench-canonical-badge:end -->/ { print; in_block=0; next } | |
| in_block { next } | |
| { print } | |
| ' .bench-results-branch/README.md > .bench-results-branch/README.md.new | |
| mv .bench-results-branch/README.md.new .bench-results-branch/README.md |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/bench-canonical.yml:
- Around line 138-148: The AWK rewrite silently drops content if the end marker
is missing; before replacing .bench-results-branch/README.md, add a guard that
counts occurrences of the start and end markers (e.g., using grep -c on "<!--
bench-canonical-badge:start -->" and "<!-- bench-canonical-badge:end -->") and
exit non‑zero if the counts are not exactly as expected (e.g., one start and one
end or matching counts), aborting the job with a clear error; implement this
check in the same run block immediately before the AWK transform so the script
fails fast instead of producing a truncated README.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ee07a74d-1de3-47d9-bff3-63b73bd3c7a2
📒 Files selected for processing (3)
.github/workflows/bench-canonical.ymlbenchmarks/badge.pytests/test_benchmarks_badge.py
Comment on lines
+138
to
+148
| run: | | ||
| set -euo pipefail | ||
| cp README.md .bench-results-branch/README.md | ||
| new="${{ steps.badge.outputs.text }}" awk ' | ||
| BEGIN { in_block=0 } | ||
| /<!-- bench-canonical-badge:start -->/ { print; print ENVIRON["new"]; in_block=1; next } | ||
| /<!-- bench-canonical-badge:end -->/ { print; in_block=0; next } | ||
| in_block { next } | ||
| { print } | ||
| ' .bench-results-branch/README.md > .bench-results-branch/README.md.new | ||
| mv .bench-results-branch/README.md.new .bench-results-branch/README.md |
There was a problem hiding this comment.
Fail fast when badge markers are missing/unbalanced.
Line 143–Line 146 can silently truncate the README tail if the end marker is absent. Add an explicit marker-count guard before rewrite.
Suggested hardening
- name: Sync README into bench-canonical-results worktree + rewrite badge
if: steps.badge.outputs.text != ''
run: |
set -euo pipefail
cp README.md .bench-results-branch/README.md
+ start_count=$(grep -c '<!-- bench-canonical-badge:start -->' .bench-results-branch/README.md || true)
+ end_count=$(grep -c '<!-- bench-canonical-badge:end -->' .bench-results-branch/README.md || true)
+ if [ "$start_count" -ne 1 ] || [ "$end_count" -ne 1 ]; then
+ echo "::error::README badge markers must exist exactly once (start=$start_count end=$end_count)"
+ exit 1
+ fi
new="${{ steps.badge.outputs.text }}" awk '
BEGIN { in_block=0 }
/<!-- bench-canonical-badge:start -->/ { print; print ENVIRON["new"]; in_block=1; next }
/<!-- bench-canonical-badge:end -->/ { print; in_block=0; next }
in_block { next }
{ print }
' .bench-results-branch/README.md > .bench-results-branch/README.md.new
mv .bench-results-branch/README.md.new .bench-results-branch/README.md🧰 Tools
🪛 GitHub Check: zizmor
[notice] 141-141:
code injection via template expansion
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/bench-canonical.yml around lines 138 - 148, The AWK
rewrite silently drops content if the end marker is missing; before replacing
.bench-results-branch/README.md, add a guard that counts occurrences of the
start and end markers (e.g., using grep -c on "<!-- bench-canonical-badge:start
-->" and "<!-- bench-canonical-badge:end -->") and exit non‑zero if the counts
are not exactly as expected (e.g., one start and one end or matching counts),
aborting the job with a clear error; implement this check in the same run block
immediately before the AWK transform so the script fails fast instead of
producing a truncated README.
Collaborator
Author
|
[claim:review:Toug:2026-05-08T07:49:51Z] |
Collaborator
Author
|
[release:review:Toug:2026-05-08T07:52:30Z] |
Collaborator
Author
|
[claim:review:Kulili:2026-05-08T07:52:35Z] |
Collaborator
Author
|
[release:review:Kulili:2026-05-08T07:53:54Z] |
Closed
16 tasks
5 tasks
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
3 participants
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.
Closes #477.
Wires the README reproducibility badge to update in-place from the nightly
bench-canonicalcron, replacing the manual406ef03placeholder with auto-rewrite from the cron's merged JSON.What lands
benchmarks/badge.py— pure functioncompute_badge_text(report_path, today=None) -> str. Iteratesresults[adapter][sub_key]._status == "ok"againstheadline_cut-derived total, returns a string of the form:Skipped invocations (per [v2.1] Bench dispatcher exit-code 3-state contract (ok / skipped / error) #479's planned 3-state contract) do not count as ok. Unit-tested at
tests/test_benchmarks_badge.py..github/workflows/bench-canonical.yml— two new steps before the existingCommit + push to bench-canonical-results:python -m benchmarks.badgeon the merged JSON. Gated onsteps.bench.outputs.out != ''so an errored run doesn't rewrite the badge to a stale0/Nline; previous badge stays.Same commit as the JSON — one cron commit per run.
Acceptance crosscheck (vs issue body)
<!-- bench-canonical-badge:start -->/<!-- bench-canonical-badge:end -->markers (already present from PR feat(bench): aelf bench all reproducibility harness (#437) #465).bench-canonical-resultsonly; main does not auto-update.✅whenpass == total > 0;⚠️otherwise.What's not in this PR
_statusdifferentiation (skipped_data_missingvserror) is owned by [v2.1] Bench dispatcher exit-code 3-state contract (ok / skipped / error) #479; the badge module already treats anything other thanokas not-ok, so the badge math will be correct as soon as [v2.1] Bench dispatcher exit-code 3-state contract (ok / skipped / error) #479 lands.Test plan
uv run pytest tests/test_benchmarks_badge.py tests/test_bench_dispatcher.py tests/test_bench_tolerance.py tests/test_benchmarks_dir.py(54 passed locally).python -m benchmarks.badge benchmarks/results/v2.0.0.json --today 2026-05-08→reproducibility: ⚠️ 6/11 ok · last run 2026-05-08.Summary by Sourcery
Integrate automated generation and in-place update of the README reproducibility badge from nightly bench-canonical cron runs.
New Features:
Enhancements:
Tests:
Summary by CodeRabbit
Release Notes
New Features
Tests