Skip to content

ci(perf): Add pending comment before benchmark runs#1267

Merged
yamadashy merged 8 commits intomainfrom
ci/perf-benchmark-pending-comment
Mar 21, 2026
Merged

ci(perf): Add pending comment before benchmark runs#1267
yamadashy merged 8 commits intomainfrom
ci/perf-benchmark-pending-comment

Conversation

@yamadashy
Copy link
Copy Markdown
Owner

@yamadashy yamadashy commented Mar 21, 2026

Add a post-pending job that immediately posts a "Benchmark in progress..." comment (Cloudflare Pages style) when the workflow starts. If a comment already exists, it gets updated to the pending state on each CI run. The final results overwrite this comment once benchmarks complete.

Changes

  • Add post-pending job: posts/updates a pending comment in parallel with benchmark execution
  • Redesign comment layout to key-value table style (inspired by Cloudflare Pages deployment comments)
  • Move pull-requests: write to workflow-level permissions (needed by both post-pending and comment jobs)

Checklist

  • Run npm run test
  • Run npm run lint

Post a "ベンチマーク取得中..." comment on the PR immediately when the
workflow starts, so users know the benchmark is in progress. If a
comment already exists, it gets updated to the pending state on each
CI run. The final results overwrite this comment once benchmarks complete.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@gemini-code-assist

This comment has been minimized.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 21, 2026

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a27eb3a6-176d-42d6-a576-fe7dbf1f8ac4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Updated GitHub Actions workflow permissions structure by adding top-level pull-requests: write. Added new post-pending job for posting or updating PR comments using HTML markers and gh api. Reorganized benchmark table format from PR | main to main → PR with adjusted placeholder rows for missing results.

Changes

Cohort / File(s) Summary
GitHub Actions Workflow Configuration
.github/workflows/perf-benchmark.yml
Reorganized workflow-level permissions, added post-pending job to manage PR comment posting/updating with fixed HTML markers using gh API, removed job-level permission from comment job, and reformatted benchmark table layout and placeholder row output.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: adding a pending comment to PRs before benchmark runs start.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The PR description includes a clear summary of changes and a completed checklist matching the repository template requirements.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ci/perf-benchmark-pending-comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages bot commented Mar 21, 2026

Deploying repomix with  Cloudflare Pages  Cloudflare Pages

Latest commit: 18491be
Status: ✅  Deploy successful!
Preview URL: https://79d7cf3f.repomix.pages.dev
Branch Preview URL: https://ci-perf-benchmark-pending-co.repomix.pages.dev

View logs

Merge the separate PR and main columns into a single "main → PR" column
to make it easier to see how performance changed at a glance.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@claude

This comment has been minimized.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 21, 2026

⚡ Performance Benchmark

Latest commit:18491be
Status:✅ Benchmark complete!
Ubuntu:2.52s (±0.01s) → 2.52s (±0.02s) · -0.00s (-0.0%)
macOS:1.29s (±0.03s) → 1.85s (±0.24s) · +0.56s (+43.6%)
Windows:3.18s (±0.05s) → 3.68s (±0.17s) · +0.51s (+15.9%)
Details
  • Packing the repomix repository with node bin/repomix.cjs
  • Warmup: 2 runs (discarded)
  • Measurement: 10 runs / 20 on macOS (median ± IQR)
  • Workflow run

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.18%. Comparing base (5ff6e8a) to head (18491be).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1267   +/-   ##
=======================================
  Coverage   87.18%   87.18%           
=======================================
  Files         115      115           
  Lines        4324     4324           
  Branches     1002     1002           
=======================================
  Hits         3770     3770           
  Misses        554      554           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Adopt a key-value table layout inspired by Cloudflare Pages deployment
comments. Shows latest commit and status in a unified table, with
benchmark results as rows in the same table on completion. Details
moved to a collapsible section.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/workflows/perf-benchmark.yml (1)

141-145: ⚠️ Potential issue | 🟠 Major

Update the comment job to run even when a benchmark matrix leg fails.

With needs: benchmark, GitHub Actions applies an implicit success() check that skips this job if the benchmark job fails. The !cancelled() condition does not override this implicit check. Since fail-fast: false allows individual matrix legs to fail, the PR comment remains stuck on "Benchmark in progress..." even though the report code already handles missing artifacts gracefully.

Suggested fix
  comment:
    name: Comment Results
    needs: benchmark
    runs-on: ubuntu-latest
-   if: ${{ !cancelled() }}
+   if: ${{ always() && !cancelled() }}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/perf-benchmark.yml around lines 141 - 145, The comment job
is being skipped when the benchmark job fails due to the implicit success()
check from needs: benchmark; update the job "comment" to override that by
changing its if condition to use always() and preserve the cancelled guard
(e.g., set the if expression to always() && !cancelled()) so the job runs even
if individual benchmark matrix legs fail while still skipping on cancellation;
keep needs: benchmark to maintain ordering.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/perf-benchmark.yml:
- Around line 16-19: The workflow runs post-pending and benchmark in parallel;
add a job dependency so benchmark waits for post-pending: in the benchmark job
block (job name "benchmark") add needs: post-pending and replace/augment its if
with if: ${{ always() && !cancelled() }} so the benchmark job will only start
after post-pending completes while still running when post-pending is skipped
(due to forks) and honoring cancellations.

---

Outside diff comments:
In @.github/workflows/perf-benchmark.yml:
- Around line 141-145: The comment job is being skipped when the benchmark job
fails due to the implicit success() check from needs: benchmark; update the job
"comment" to override that by changing its if condition to use always() and
preserve the cancelled guard (e.g., set the if expression to always() &&
!cancelled()) so the job runs even if individual benchmark matrix legs fail
while still skipping on cancellation; keep needs: benchmark to maintain
ordering.
🪄 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: 13c3fdc5-ac58-4811-b626-c2609b130c28

📥 Commits

Reviewing files that changed from the base of the PR and between f25236d and 529d7e8.

📒 Files selected for processing (1)
  • .github/workflows/perf-benchmark.yml

@claude

This comment has been minimized.

HTML <table> doesn't require a header row, matching the clean look of
Cloudflare Pages deployment comments.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@claude

This comment has been minimized.

macOS GitHub Actions runners (M1 Mac mini) show higher variance due to
Spotlight indexing, thermal throttling, and shared hardware. Increase
measurement runs from 10 to 20 for macOS to stabilize the median.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
devin-ai-integration[bot]

This comment was marked as resolved.

@claude

This comment has been minimized.

- Move pull-requests: write to job-level permissions on post-pending
  and comment jobs only, so the benchmark job (which executes untrusted
  PR code) does not get unnecessary write access
- Use always() && !cancelled() on comment job to ensure pending comment
  is overwritten even when benchmark fails
- Fix details text to reflect macOS using 20 measurement runs

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@claude

This comment has been minimized.

…crets

These workflows use repository-level secrets (CODECOV_TOKEN,
CLAUDE_CODE_OAUTH_TOKEN, COMMITTER_TOKEN) without dedicated
environments, which is acceptable for this project's threat model.
Adding dedicated environments would add overhead without meaningful
security benefit for these use cases.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@claude

This comment has been minimized.

The rule was added in zizmor v1.23.x. The previous config used
secrets-inherit which is a different rule.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Mar 21, 2026

Code Review (Consolidated)

This PR has been reviewed across multiple iterations. All previously raised issues have been addressed. Here's the final status:

All feedback resolved

Item Status
comment job condition: always() && !cancelled() ✅ Fixed (line 158)
Details section: "10 runs / 20 on macOS" ✅ Fixed (line 224)
secrets-outside-env rule name in zizmor.yml ✅ Fixed in latest commit

Summary

  • Pending comment UX: Clean implementation — posts a "Benchmark in progress..." comment immediately, overwritten by final results using the same <!-- repomix-perf-benchmark --> marker
  • Table redesign: Cloudflare Pages-inspired key-value layout is more scannable than the previous markdown table
  • Permissions: Correctly elevated pull-requests: write to workflow level for both post-pending and comment jobs
  • Fork safety: Both comment-writing jobs guarded with github.event.pull_request.head.repo.fork == false
  • Zizmor config: Appropriate suppression of secrets-outside-env warnings for workflows using repository-level secrets

Verdict: LGTM — no remaining issues. Ready to merge.

🤖 Generated with Claude Code

@yamadashy yamadashy merged commit 46a97da into main Mar 21, 2026
62 checks passed
@yamadashy yamadashy deleted the ci/perf-benchmark-pending-comment branch March 21, 2026 05:58
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.

1 participant