feat(auto-rebase-health): measure post-restriction fan-out reduction (#739) - #890
feat(auto-rebase-health): measure post-restriction fan-out reduction (#739)#890don-petry wants to merge 11 commits into
Conversation
…739) Phase 2 instrumentation for epic #736. The Phase 1 report (#737) estimated fan-out as runs × ALL open non-Dependabot PRs, which does not reflect the review-ready eligibility gate that landed in petry-projects/.github#468 (issue #465) — so it could not show the reduction the gate buys. Add a "Post-restriction fan-out" section that mirrors the reusable's review-ready predicate against the current open PRs: - count eligible PRs (non-draft AND (current APPROVED review OR the auto-rebase:ready label)), reading actual review states (current-approval wins) exactly as the reusable's lib/eligibility.sh does - report the eligible-PR multiplier, the restricted re-run estimate, and the behind→eligible reduction, with an explicit ≥50% success-metric verdict This is the cleaner before/after signal that feeds the Merge Queue go/no-go decision record (#739). New pure helpers (fmt_reduction, pr_has_current_approval, count_eligible) are unit-tested; main() does the per-PR read I/O (no CI re-runs, no LLM cost, still ≤1 scheduled run/day). render_report stays backward-compatible — the new section renders only when an eligible count is supplied. shellcheck clean at the repo's --severity=warning gate; 28/28 bats pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019LUUSUQHqwLWZ583SAs41K
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
More reviews will be available in 26 minutes and 53 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ 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: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ 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 |
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
|
Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-06-21T14:15:55Z. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances the auto-rebase health report by adding a "Post-restriction fan-out" section to track the reduction in CI re-runs when restricting updates to review-ready PRs. It introduces helper functions to determine PR eligibility based on draft status, approvals, and labels, along with corresponding unit tests. The review feedback suggests valuable improvements: optimizing GitHub API usage by fetching draft status and labels in the initial list call to reduce total requests, handling potential null-user exceptions in jq for deleted users, and utilizing a more idiomatic jq filter for label matching.
| pr_has_current_approval() { | ||
| local json="${1:-}" result | ||
| [ -n "$json" ] || json='[]' | ||
| result=$(printf '%s' "$json" | jq -r ' | ||
| reduce (.[] | select(.state == "APPROVED" or .state == "CHANGES_REQUESTED" or .state == "DISMISSED")) as $r ({}; .[$r.user.login] = $r.state) | ||
| | any(. == "APPROVED")') | ||
| [ "$result" = "true" ] | ||
| } |
There was a problem hiding this comment.
If a review is submitted by a deleted user, the .user object in the GitHub API response can be null. Attempting to access .user.login directly will cause jq to throw a Cannot index null with string "login" error. We should safely navigate .user and ensure .user?.login is not null before indexing.
| pr_has_current_approval() { | |
| local json="${1:-}" result | |
| [ -n "$json" ] || json='[]' | |
| result=$(printf '%s' "$json" | jq -r ' | |
| reduce (.[] | select(.state == "APPROVED" or .state == "CHANGES_REQUESTED" or .state == "DISMISSED")) as $r ({}; .[$r.user.login] = $r.state) | |
| | any(. == "APPROVED")') | |
| [ "$result" = "true" ] | |
| } | |
| pr_has_current_approval() { | |
| local json="${1:-}" result | |
| [ -n "$json" ] || json='[]' | |
| result=$(printf '%s' "$json" | jq -r ' | |
| reduce (.[] | select((.state == "APPROVED" or .state == "CHANGES_REQUESTED" or .state == "DISMISSED") and .user?.login != null)) as $r ({}; .[$r.user.login] = $r.state) | |
| | any(. == "APPROVED")') | |
| [ "$result" = "true" ] | |
| } |
There was a problem hiding this comment.
Done in scripts/auto_rebase_health.sh pr_has_current_approval: added .user?.login != null to the select predicate so reviews from deleted users (where .user is null) are filtered out before the reduce accesses .user.login, preventing the Cannot index null with string "login" jq error. Added two new bats tests covering the null-user case in tests/auto_rebase_health.bats.
| count_eligible() { | ||
| local json="${1:-}" label="${2:-}" | ||
| [ -n "$json" ] || json='[]' | ||
| printf '%s' "$json" | jq --arg L "$label" ' | ||
| [ .[] | ||
| | select(((.draft // false) | not) | ||
| and ((.approved // false) or ([.labels[]?.name] | any(. == $L)))) ] | ||
| | length' | ||
| } |
There was a problem hiding this comment.
We can make the jq filter more idiomatic and efficient by using any/2 directly on .labels[]? instead of constructing an intermediate array [.labels[]?.name] and piping it to any.
| count_eligible() { | |
| local json="${1:-}" label="${2:-}" | |
| [ -n "$json" ] || json='[]' | |
| printf '%s' "$json" | jq --arg L "$label" ' | |
| [ .[] | |
| | select(((.draft // false) | not) | |
| and ((.approved // false) or ([.labels[]?.name] | any(. == $L)))) ] | |
| | length' | |
| } | |
| count_eligible() { | |
| local json="${1:-}" label="${2:-}" | |
| [ -n "$json" ] || json='[]' | |
| printf '%s' "$json" | jq --arg L "$label" ' | |
| [ .[] | |
| | select(((.draft // false) | not) | |
| and ((.approved // false) or any(.labels[]?; .name == $L))) ] | |
| | length' | |
| } |
There was a problem hiding this comment.
Done in scripts/auto_rebase_health.sh count_eligible: replaced [.labels[]?.name] | any(. == $L) with the idiomatic any(.labels[]?; .name == $L), which skips the intermediate array allocation. All existing count_eligible bats tests continue to pass.
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
Dev-Lead — review-changes (applied)Changes committed and pushed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: LOW
Reviewed commit: 98fc98ec100b6993d9bb56ac688541dd712c2505
Review mode: triage-approved (single reviewer)
Summary
Phase 2 instrumentation for the auto-rebase health report (epic #736): adds a "Post-restriction fan-out" section that mirrors the reusable's review-ready eligibility predicate and reports the behind→eligible multiplier reduction with a ≥50% success verdict. Changes are confined to scripts/auto_rebase_health.sh (+117/-8) and tests/auto_rebase_health.bats (+118/-0). New helpers (fmt_reduction, pr_has_current_approval, count_eligible) are pure and unit-tested; main() does best-effort per-PR reads using existing pull-requests:read permission — no new spend, no LLM calls, render_report stays backward-compatible (section renders only when an eligible count is supplied).
Linked issue analysis
No formal closing-issue reference (closingIssuesReferences is empty). The PR is instrumentation feeding the Merge Queue go/no-go decision record (#739) for epic #736 / story #465; the body and commit message document this context clearly. No issue is meant to auto-close, so this is expected rather than a gap.
Findings
No blocking findings.
- Logic is consistent: fmt_reduction and the inline ≥50% verdict both compute (behind - eligible) * 100 / behind, both guard the behind<=0 / divide-by-zero case.
- pr_has_current_approval correctly reduces to last-decision-wins per reviewer and skips null users (.user?.login != null), with explicit unit tests for null-user, COMMENTED-only, and cross-user cases.
- Gemini's earlier (non-blocking, COMMENTED) suggestions are addressed: draft+labels are fetched in the single gh pr list call (one reviews call per PR, not two), and null-user reviews are handled.
- Secret-scan MCP tool (mcp__github__run_secret_scanning) is not exposed in this run; relying on the green gitleaks CI check. The diff contains no secrets — pure shell/jq report logic.
- All variables are quoted; jq calls use --arg/--argjson safely.
CI status
All required checks green at 98fc98e: Lint/ShellCheck, bats unit-tests, CodeQL (actions+python), SonarCloud (Quality Gate passed — 0 issues/hotspots), Secret scan (gitleaks), AgentShield, holdout-guard, test-deletion guard. CodeRabbit APPROVED at head SHA. The CANCELLED dev-lead dispatch/ci-relay entries stem from the [skip ci-relay] commit and are expected; dependency-audit ecosystem subjobs are SKIPPED (no matching ecosystems). No failures. mergeStateStatus is BLOCKED pending the requested org-leads human review.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
|
Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-06-21T17:39:44Z. |
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
|
Dev-Lead — waiting on PR blockers (intent: review-changes)PR: #890 |
|
Note @don-petry I reviewed this PR and no code changes were needed, but it still has blocking checks or reviews (failing or cancelled checks, or changes-requested reviews), so I cannot mark it done yet. I'll re-check automatically. |
|
Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-06-22T00:40:12Z. |
|
Closing — overtaken by events. This PR's purpose was to produce a cleaner before/after fan-out measurement to feed the #739 decision record. #739 shipped via #891 (merged) using the snapshot/eligible-multiplier method, with the ≥50% success metric already verified and recorded. On review, the remaining standalone value is thin: the load-bearing gate signal (the agentic-conflict-resolution rate — sentinels/responses/ |
Pull request was closed
2 participants
What
Phase 2 instrumentation for epic #736. Adds a Post-restriction fan-out section to the auto-rebase health report so the report can show the reduction the review-ready eligibility gate (petry-projects/.github#468, issue #465) actually buys.
The Phase 1 report (#737) estimated fan-out as
runs × ALL open non-Dependabot PRs— it never subtracted ineligible PRs, so it could not surface the reduction. This adds the eligible-PR multiplier and a like-for-like before/after.How
scripts/auto_rebase_health.sh:pr_has_current_approval— reads actual review states (current-approval wins; a laterCHANGES_REQUESTED/DISMISSEDcancels an earlierAPPROVED), mirroring the reusable's.github/scripts/auto-rebase/lib/eligibility.shexactly.count_eligible— non-draft AND (approved ORauto-rebase:readylabel).fmt_reduction— behind→eligible multiplier reduction, with a zero-base guard.render_reportgains an optional eligible-count arg and renders the new section (eligible multiplier, restricted re-run estimate, reduction %, explicit ≥50% success-metric verdict). Backward-compatible: the section renders only when an eligible count is supplied.main()computes eligibility from per-PR reads (onepulls/{n}+pulls/{n}/reviewsper open non-Dependabot PR). Best-effort: any failure leaves the section omitted rather than reported wrongly.Sample output (live, 6-day post-change window)
Cost / safety
pull-requests: read(no permission change).shellcheckclean at the repo's--severity=warninggate; 28/28 bats pass (bats tests/auto_rebase_health.bats).Context
🤖 Generated with Claude Code