Repository navigation
feat: implement issue #1997 — Lower the advisory quorum to 5: drop the three reviewers that produce no countable signal (re-file of #1894 AC #3) - #2003
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Dev-Lead — waiting on PR blockers (intent: review-changes)PR: #2003 |
|
Note @don-petry I reviewed this PR and no code changes were needed, but I can't mark it done yet: required check |
|
No description provided. |
![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 advisory review gate configuration by removing three inactive bots (Copilot PR Reviewer, Codex, and Qodo Merge) from the active advisory-wait set, reducing the count from 8 to 5. The fallback bot list in review-one-pr.sh has been updated accordingly, and a new consistency test has been added to prevent future configuration drift. Corresponding unit tests have also been updated to reflect these changes. There are no review comments, and I have no additional feedback to provide.
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
CodeAnt Nitpicks1 code suggestion1. The first grep match is the explanatory comment's
|
|
Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-10-01T20:43:02Z. |
|
Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-10-01T20:43:04Z. |
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
Superseded by automated re-review at
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 5 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Superseded by automated re-review at
|
Superseded by automated re-review at
|
|
Addressed by the drift-guard hardening in |
|
Acknowledged — this is a transient Codex usage-limit notice, not a code finding. No action needed in this PR; the advisory gate already treats a rate-limited bot as non-participating and the pr-review sweep re-reviews once the limit resets. |
|
Acknowledged — this is a Qodo trial-ended/billing-paused notice, not a code finding. No action needed here; Qodo was dropped from the advisory approval-wait quorum in #1997 (it stays dev-lead-trusted), so its paused state does not block this PR. |
|
Acknowledged — this is a CodeRabbit review-limit (rate-limit) notice, not a code finding. No action needed; CodeRabbit is tracked by the rate-limit detector/scorecard but is not part of the advisory approval-wait set, so it does not block this PR. |
|
Acknowledged — SonarCloud reports its Quality Gate as passed (0 new issues, 0 security hotspots). This is a clean informational status with no finding to address. |
Dev-Lead — fix-reviews (partial)A commit was pushed, but not every requested change was applied. Per requested item:
The unaddressed items above still need work. |
|
Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-10-01T21:01:18Z. |
|
|
Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-10-01T21:04:08Z. |
#2003 set advisory_gate=no on chatgpt-codex-connector and qodo-code-review and extended their rationale. This branch added their info_status_pattern. The merged rows keep main's columns 1-5 (advisory_gate=no), this branch's pattern, and both rationales. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015fdDZDb4mhT65MU1izeA4R
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015fdDZDb4mhT65MU1izeA4R
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: 2268844dc3ba6bc6b3ae4a87ae827750db32e3cc
Cascade: triage → deep+duck (triage: haiku 4.5 [sonnet 5.5, sonnet 5] → deep: opus 5.5 [opus 4.8, sonnet 5.5] + duck: gemini-3.8-flash [sonnet 5.5] → audit: opus 5.5 [opus 4.8, opus 4.7])
Summary
Both reviewers rated the risk MEDIUM and approved, so they fully agree. The PR lowers the advisory quorum from 8 to 5 consistently across the registry, the gate fallback literal and review-one-pr.sh, with new consistency tests. Both flagged the same behavior change (triage feedback now covers all trusted bots under the shared 8000-char cap) and the graphite-app check-run limitation, though at different lines, so they were kept as separate entries. The rubber duck added one unique minor point, that the grep regex in the _triage_bots test is fragile, and noted that CI status could not be fully confirmed because of API rate limits.
Cross-engine agreement
full
Downstream impact
This change is consumed by 8 downstream repo(s) that pin the affected reusable workflow / lib / prompt. Impacted consumers:
Impacted shared surfaces:
- .github/workflows/dev-lead-reusable.yml
- .github/workflows/pr-review.yml
Impacted consumers (8, fetching up to 10):
- petry-projects/.github (pins .github/workflows/dev-lead-reusable.yml)
.github/workflows/dev-lead.yml
- petry-projects/.github-private (pins .github/workflows/dev-lead-reusable.yml,.github/workflows/pr-review.yml)
.github/workflows/dev-lead.yml
.github/workflows/pr-review-canary.yml
.github/workflows/pr-review-trigger.yml
.github/workflows/test-dev-lead.yml
- petry-projects/ContentTwin (pins .github/workflows/dev-lead-reusable.yml,.github/workflows/pr-review.yml)
.github/workflows/dev-lead.yml
.github/workflows/pr-review.yml
- petry-projects/TalkTerm (pins .github/workflows/dev-lead-reusable.yml)
.github/workflows/dev-lead.yml
- petry-projects/bmad-bgreat-suite (pins .github/workflows/dev-lead-reusable.yml,.github/workflows/pr-review.yml)
.github/workflows/dev-lead.yml
.github/workflows/pr-review.yml
- petry-projects/broodly (pins .github/workflows/dev-lead-reusable.yml)
.github/workflows/dev-lead.yml
- petry-projects/google-app-scripts (pins .github/workflows/dev-lead-reusable.yml,.github/workflows/pr-review.yml)
.github/workflows/dev-lead.yml
.github/workflows/pr-review.yml
- petry-projects/markets (pins .github/workflows/dev-lead-reusable.yml,.github/workflows/pr-review.yml)
.github/workflows/dev-lead.yml
.github/workflows/pr-review.yml
Findings
- info: CodeAnt claim that the test's grep matches a comment rather than the fallback literal is refuted at head 2268844: the grep returns exactly one match, the real literal on line 1059. (
tests/test_reviewer_sources.bats:231) - info: Triage feedback bot set moved from the advisory-gate set to the dev_lead_trusted set, adding bots such as coderabbitai. More bots share the 8000-char ADVISORY_BOT_FEEDBACK cap, though each review body is capped at 800 chars. Acceptable. (
scripts/review-one-pr.sh:1048) - info: Triage feedback set widens from the old advisory subset to all 9 trusted bots, and the 8000-char cap now covers more bots. This is intentional, but the prompt content is no longer byte-identical to before. (
scripts/review-one-pr.sh:1039) - info: graphite-app stays in the quorum but its clean passes appear only as a check run, which the gate cannot count. Issue #1997 scopes this out. Needs a follow-up issue for gate-side check-run counting. (
scripts/lib/reviewer-sources.tsv:58) - info: graphite-app stays in the quorum but its clean passes are only a check run, so it can still hit the timeout fallback. Documented in the PR as a known limitation. (
scripts/lib/reviewer-sources.tsv:59) - info: Cubic P3 is addressed by the new test pinning the ADVISORY_BOTS fallback literal to the registry advisory-gate projection. Verified by hand with REVIEWER_SOURCES_MANIFEST=/nonexistent. (
scripts/lib/advisory-review-gate.sh:41) - info: The _triage_bots literal test uses a grep -oE '[^]]*' regex, which is fragile if the literal contains a ']' or is reformatted across lines. It fails closed (count != 1). (
tests/test_reviewer_sources.bats:262) - info: The GitHub API rate limit prevented re-confirming review-thread resolution and CI status. Cubic inline findings are fixed or out of scope, and shellcheck is clean on both changed scripts.
Reviewed by the PR-review cascade (triage: haiku 4.5 [sonnet 5.5, sonnet 5] → deep: opus 5.5 [opus 4.8, sonnet 5.5] + duck: gemini-3.8-flash [sonnet 5.5] → audit: opus 5.5 [opus 4.8, opus 4.7]). Reply if you need a human review.
|
pr-review approved on PARTIAL advisory evidence: 5/7 required advisory bots reported before the gate's head-age-timeout fallback proceeded. Recorded for the miss-rate metric (#1596). |
2 participants
Problem
Lower the advisory quorum to 5: drop the three reviewers that produce no countable signal (re-file of #1894 AC #3)
From the issue: As the fleet operator, I want the advisory quorum to count only reviewers that actually produce a countable signal, so that pr-review stops reaching approval through a timeout fallback on every single PR.
Risk
Low — changes automation shell logic under scripts/, covered by shellcheck (--severity=warning) and the bats suite.
Test plan
Tests added/updated:
tests/dev-lead/unit/test_advisory_review_gate.bats,tests/test_reviewer_sources.bats. Verification:bash scripts/dev-lead-lint.sh(shellcheck --severity=warning) ran pre-commit; the bats suite runs in CI.Rollback
Revert this PR. No non-revertible side effects (no tags, migrations, or external state).
Monitoring
This PR's Lint (shellcheck) and bats checks show pass/fail; watch subsequent dev-lead / pr-review runs for behavioral regressions.
Closes #1997