Repository navigation
feat: implement issue #1030 — [dev-lead timeout follow-up 3/3] Report timeout wall-rate as prevalence (over total dev-lead runs) - #1057
Conversation
… timeout wall-rate as prevalence (over total dev-lead runs)
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 45 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?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 reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. 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, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ 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. |
![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 updates the dev-lead timeout rate calculation to report prevalence (timeouts divided by total dev-lead runs) instead of a rate based solely on failure markers. It modifies fleet_monitor.sh to extract and append total runs from the metrics file, updates fleet_report.sh to parse this new column and calculate the prevalence rate, and adds corresponding test coverage in fleet_report.bats to handle edge cases like zero or non-numeric runs. I have no feedback to provide as the implementation is robust and well-tested.
Dev-Lead — fix-reviews (no-changes)Agent reasoning |
There was a problem hiding this comment.
Pull request overview
Implements issue #1030’s refinement to the Dev-Lead timeout (“wall”) report so the reported “Timeout rate” is intended to be a prevalence metric (timeouts over total Dev-Lead runs), rather than a composition metric (timeouts over failure markers).
Changes:
- Extends the Dev-Lead timeout TSV row format to include
total_runsalongside marker breakdown counts. - Updates
generate_dev_lead_timeout_reportto render aTotal runscolumn and compute the timeout rate usingtimeouts / total_runs. - Adds/updates Bats tests to validate prevalence formatting, including decimal percentages and
n/aon zero/unavailable totals.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
scripts/fleet_monitor.sh |
Appends total_runs (from existing per-workflow metrics) into each Dev-Lead timeout row. |
scripts/fleet_report.sh |
Renders the new column set and switches timeout-rate computation to use total_runs as the denominator. |
tests/fleet_report.bats |
Updates and adds tests for the new prevalence-rate behavior and n/a handling. |
…1030) Address Copilot on #1057: the timeout prevalence rate summed total-runs only from repos that had >=1 dev-lead marker (rows in the reason file), so every all-success repo (runs but no markers) was dropped from the denominator — the exact inflation #1030 set out to fix (label claimed 'over all runs' while the denominator was partial). - fleet_monitor.sh: compute fleet_dl_runs = sum of every repo's dev-lead.yml 'total' across ALL repos in the per-workflow metrics (sentinel '?' skipped), and pass it into the report. - fleet_report.sh: generate_dev_lead_timeout_report now takes the fleet run total as arg 2 and uses it as the rate denominator + the fleet-total 'Total runs' cell; added a note that per-repo rows list only marker repos while the fleet total/denominator include all-success repos. - tests: pass the fleet denominator; new regression test asserts 2 timeouts / 100 fleet runs = 2% (not 2/10 = 20%) when only one repo has markers. shellcheck clean; fleet_report bats green (incl. the new #1030 case). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUhVsBbihbTjVvbcVf1WuU
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
Dev-Lead — waiting on PR blockers (intent: review-changes)PR: #1057 |
|
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. |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: 93c7d6c293f5cf37841df3e0e58076d55b1ea6c8
Review mode: triage-approved (single reviewer)
Summary
Converts the dev-lead timeout 'wall rate' from a composition metric (timeouts / failure markers) to a prevalence metric (timeouts / total fleet dev-lead runs), per issue #1030. fleet_monitor.sh sources per-repo and fleet-wide dev-lead.yml run totals from the per-workflow metrics already collected (no new API scan); fleet_report.sh adds a 'Total runs' column, uses the fleet-wide total as the rate denominator, and handles zero/non-numeric totals as n/a with no divide-by-zero. Tests cover the K/N rate, decimal rendering, n/a cases, and a dedicated regression test for the all-fleet-denominator fix.
Linked issue analysis
Closes #1030. All six acceptance criteria are substantively met: (1) rate is timeout_count / total dev-lead runs, labeled as prevalence over all runs; (2) run totals come from the existing per-workflow metrics (dev-lead.yml row, matched by basename) and are wired into generate_dev_lead_timeout_report as a parameter; (3) raw timeout and engine-error counts are retained in the table; (4) missing/zero/non-numeric run counts render n/a with no divide-by-zero or set -e failure; (5) bats fixtures assert rate = K/N (3/100 = 3%, 37.5% decimal case) and zero/non-numeric → n/a; (6) shellcheck and bats CI checks are green. A regression test additionally locks in the fleet-wide denominator (2/100 = 2%, not 2/10 = 20% from marker-repos-only).
Findings
- All 3 prior Copilot review findings (prevalence denominator computed only from marker repos, inflating the rate) were fixed by commit 3e4cf90: fleet_dl_runs now sums dev-lead.yml totals across ALL repos in the metrics file, and a regression test asserts the behavior. All review threads are resolved.
- Correctness: awk matching is by repo field + workflow-path basename with an exit-on-first-match; ERROR sentinel '?' and missing rows sanitize to 0 → n/a. The report note clarifies that per-repo rows list only marker repos while the fleet total/denominator include all-success repos — good, this prevents misreading the table.
- Security: no secrets, no injection surface (awk -v variable passing, internally generated TSV data), no workflow/permission changes. Gitleaks, CodeQL, AgentShield, and the agent security scan are all green.
- The run_secret_scanning MCP tool was not available in this environment; the gitleaks CI check covers secret detection and passed.
- Minor (non-blocking): when fleet_runs is 0 the rate line renders 'n/a (N / n/a)', which is slightly awkward but matches the tested, documented behavior.
CI status
All code-verification checks green: Lint, ShellCheck (x2), bats, unit-tests, CodeQL (actions + python), SonarCloud quality gate, gitleaks secret scan, Agent Security Scan, AgentShield, holdout-guard, test-deletion guard, template-drift, validate-agent-profiles, gh-aw-compile, dependency-audit. Two CANCELLED jobs (dev-lead / dispatch, dev-lead / ci-relay) belong to the authoring Dev-Lead Agent orchestration workflow, superseded by the merge-commit push — they are not code checks and do not gate code quality. CodeRabbit approved at the head SHA.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
Closes #1030
Implemented by dev-lead agent. Please review.