ci: add pull request benchmarks#45
Conversation
commit: |
Benchmark ResultsCompared PR head Total median wall time: 19.70s -> 19.58s (-0.6%, 1.01x speedup)
Profile: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 000435904e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Warning Review limit reached
More reviews will be available in 15 minutes and 4 seconds. Learn how PR review limits work. To continue reviewing without waiting, enable usage-based billing in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR introduces a complete build benchmarking system for 🚥 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 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.github/workflows/benchmark.yml (2)
166-173: 💤 Low valueConsider more explicit error handling for history branch creation.
The current logic uses
||to handle the case where the history branch doesn't exist. If the first clone fails for a transient reason (e.g., network error) but the second clone succeeds, it would incorrectly create an orphan branch.While this edge case is unlikely in practice, you could make the logic more explicit by checking if the branch exists before attempting to create it:
Alternative approach
# Check if branch exists if git ls-remote --exit-code --heads "https://x-access-token:${GH_TOKEN}`@github.com/`${REPOSITORY}.git" "$BENCHMARK_HISTORY_BRANCH" > /dev/null 2>&1; then git clone --depth 1 --branch "$BENCHMARK_HISTORY_BRANCH" \ "https://x-access-token:${GH_TOKEN}`@github.com/`${REPOSITORY}.git" "$history_dir" else git clone --depth 1 "https://x-access-token:${GH_TOKEN}`@github.com/`${REPOSITORY}.git" "$history_dir" cd "$history_dir" git checkout --orphan "$BENCHMARK_HISTORY_BRANCH" git rm -rf . fi🤖 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/benchmark.yml around lines 166 - 173, The current error handling logic using the || operator does not properly distinguish between a missing branch and other types of clone failures such as network errors, which could lead to incorrectly creating an orphan branch. Replace the || based fallback logic with an explicit check using git ls-remote with the --exit-code and --heads flags to verify if the BENCHMARK_HISTORY_BRANCH exists on the remote repository before attempting to clone with that branch. Use an if-then-else structure where if the branch exists, clone with the branch argument, otherwise clone without the branch and then create the orphan branch locally.
1-6: 💤 Low valueConsider adding a timeout and manual trigger.
The workflow currently has no timeout and can only be triggered by pull requests.
Suggested enhancements
Add a generous timeout to catch runaway jobs:
jobs: benchmark: name: Compare build benchmarks runs-on: ubuntu-latest + timeout-minutes: 60Add
workflow_dispatchto allow manual testing:on: pull_request: branches: [main] + workflow_dispatch:Also applies to: 16-19
🤖 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/benchmark.yml around lines 1 - 6, The Benchmark workflow in the on section currently only triggers on pull_request events with no timeout protection. Add workflow_dispatch to the on section to enable manual triggering of the workflow, and add a timeout-minutes configuration to the job level to prevent runaway jobs from consuming excessive resources. This will allow both automatic execution on pull requests and manual execution via workflow dispatch while protecting against long-running job scenarios.
🤖 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 `@scripts/compare-benchmarks.mjs`:
- Around line 19-23: The error message in the throw statement uses the same
placeholder `<baseline.json>` for both the `--before` and `--after` parameters,
which is confusing and suggests they should be the same file. Update the error
message to use different, distinct placeholders for each parameter (for example,
use a different filename placeholder for `--after` such as `<comparison.json>`
or `<current.json>`) to clearly indicate that two separate files are required.
In `@scripts/report-benchmark-ci.mjs`:
- Around line 22-26: The error message displayed in the throw statement when
validating the base and head parameters uses the same placeholder
`<baseline.json>` for both `--base` and `--head` arguments, which is confusing
since they represent different files. Update the error message to use distinct
and descriptive placeholders for each parameter, such as `--base <baseline.json>
--head <current.json>` or similar naming that clearly indicates these are two
separate files with different purposes.
---
Nitpick comments:
In @.github/workflows/benchmark.yml:
- Around line 166-173: The current error handling logic using the || operator
does not properly distinguish between a missing branch and other types of clone
failures such as network errors, which could lead to incorrectly creating an
orphan branch. Replace the || based fallback logic with an explicit check using
git ls-remote with the --exit-code and --heads flags to verify if the
BENCHMARK_HISTORY_BRANCH exists on the remote repository before attempting to
clone with that branch. Use an if-then-else structure where if the branch
exists, clone with the branch argument, otherwise clone without the branch and
then create the orphan branch locally.
- Around line 1-6: The Benchmark workflow in the on section currently only
triggers on pull_request events with no timeout protection. Add
workflow_dispatch to the on section to enable manual triggering of the workflow,
and add a timeout-minutes configuration to the job level to prevent runaway jobs
from consuming excessive resources. This will allow both automatic execution on
pull requests and manual execution via workflow dispatch while protecting
against long-running job scenarios.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 72b501b2-640f-4851-8b5e-27be12ecece3
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
.github/workflows/benchmark.yml.gitignorebenchmarks/README.mdpackage.jsonscripts/bench-builds.mjsscripts/benchmark/fixture.mjsscripts/compare-benchmarks.mjsscripts/report-benchmark-ci.mjs
1 participant
Summary
Notes
Validation