ci(bench): compare fresh PR benchmark baselines#999
Conversation
📝 WalkthroughWalkthroughThe bench-pr.yml GitHub Actions workflow was modified to increase the job timeout, rework baseline benchmarking to freshly measure the PR base commit (via cache restore, checkout, build, and hyperfine) when BASE_SHA is available, adjust head benchmark phase configuration, and update the resulting PR comment text. ChangesBench Workflow Update
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Workflow as Bench Workflow
participant Cache as Registry Cache
participant Base as PR Base Commit
participant Head as PR Head Commit
participant Comment as PR Comment
Workflow->>Cache: Restore hermetic registry cache
alt BASE_SHA available
Workflow->>Base: Checkout, build, run hyperfine (BENCH_PHASES=0)
else BASE_SHA absent
Workflow->>Workflow: Copy committed results.json
end
Workflow->>Head: Run hyperfine (BENCH_PHASES=1)
Workflow->>Comment: Post footer noting fresh benchmarks and 2 scenarios
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
Greptile SummaryThis PR fixes the PR bench workflow so that the base SHA is benchmarked fresh on the same runner rather than diffed against the committed
Confidence Score: 5/5CI-only workflow change; no application runtime impact. The base/head checkout sequencing and fallback paths are correct. The fix is narrowly scoped: the old No files require special attention. Important Files Changed
Reviews (2): Last reviewed commit: "ci(bench): always measure PR base benchm..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/bench-pr.yml (1)
52-68: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant existence check writes and discards content.
git show "$BASE_SHA:benchmarks/results.json" > /tmp/aube-bench-base.jsonis used solely to validate the file exists atBASE_SHA; the captured content is immediately overwritten by the hyperfineRESULTS_JSONoutput on success. Usinggit cat-file -e "$BASE_SHA:benchmarks/results.json"(no output redirection needed) would make the intent clearer without the throwaway write.♻️ Proposed simplification
- if [ -n "$BASE_SHA" ] && git show "$BASE_SHA:benchmarks/results.json" > /tmp/aube-bench-base.json 2>/dev/null; then + if [ -n "$BASE_SHA" ] && git cat-file -e "$BASE_SHA:benchmarks/results.json" 2>/dev/null; then🤖 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-pr.yml around lines 52 - 68, The BASE_SHA existence check in the bench workflow is doing a throwaway write by using git show solely to confirm benchmarks/results.json exists, then overwriting that file later in the same job. Update the conditional in the bench-pr workflow to use a non-output existence check such as git cat-file -e for benchmarks/results.json, keeping the surrounding BASE_SHA, RESULTS_JSON, and benchmark execution flow unchanged.
🤖 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.
Nitpick comments:
In @.github/workflows/bench-pr.yml:
- Around line 52-68: The BASE_SHA existence check in the bench workflow is doing
a throwaway write by using git show solely to confirm benchmarks/results.json
exists, then overwriting that file later in the same job. Update the conditional
in the bench-pr workflow to use a non-output existence check such as git
cat-file -e for benchmarks/results.json, keeping the surrounding BASE_SHA,
RESULTS_JSON, and benchmark execution flow unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: eab29539-eaa2-4b8c-a728-faf50fd286be
📒 Files selected for processing (1)
.github/workflows/bench-pr.yml
1 participant
Summary
pr-benchWhy
The previous PR benchmark compared a fresh 3-run head measurement against the committed
benchmarks/results.jsonbaseline. Warm install timings are filesystem-heavy and noisy on the CI runner, so stale cross-run baselines could look like large regressions even when a same-run A/B does not reproduce them.Validation
actionlint .github/workflows/bench-pr.ymlNote
Low Risk
CI-only workflow changes; longer runs and more reliable A/B comparison with no production code impact.
Overview
PR bench workflow now measures base and head on the same runner instead of comparing head against committed
benchmarks/results.json, cutting down false regressions from noisy warm-install timings.When a base SHA exists, the workflow checks out that commit, builds release, and runs the full hermetic bench (
gvs-warm,gvs-cold) into/tmp/aube-bench-base.jsonwith phase timings off (BENCH_PHASES=0). It restores~/.cache/aube-bench/registryviaactions/cache, bumps the job timeout to 45 minutes, and turns phase timings on for the head run only (BENCH_PHASES=1). The PR comment footer now states that base/head were freshly benchmarked and that 2 scenarios are used.Reviewed by Cursor Bugbot for commit 5d27e19. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit