Skip to content
Merged
83 changes: 71 additions & 12 deletions docs/reviewer-report.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ matching the delivery pattern of the [Token Cost Observatory](./token-report.md)
## Reviewers tracked

Identity comes from each bot's GraphQL App **login** — the single source of truth is the
advisory-review gate (`scripts/lib/advisory-review-gate.sh`), so this report can never
reviewer-source registry (`scripts/lib/reviewer-sources.tsv`, [#1425](https://github.com/petry-projects/.github-private/issues/1425)),
which the advisory-review gate and this report both project from, so the report can never
drift from the approval gate's notion of who these bots are.

| Reviewer | GraphQL login |
Expand All @@ -44,18 +45,61 @@ counts), and issue `comments`. Pagination stops as soon as a page's PRs predate
lookback window (PRs are ordered by `UPDATED_AT` descending), so we never walk a repo's
entire history. This is far fewer API calls than per-PR REST fetches.

Pages are deliberately **light** — `PR_PAGE_SIZE` PRs per page (default 10) with bounded
nested connections — because a heavy page trips GitHub's GraphQL resource limit, which
returns an HTTP-200 body carrying an `errors` array and **null `data`**. The old code read
that as "no PRs" and silently dropped the whole repo (the ~8× Graphite undercount, [#1908](https://github.com/petry-projects/.github-private/issues/1908)).
A failed page is now **retried with a halved page size and backoff** (`PR_FETCH_MAX_ATTEMPTS`,
default 4); a repo that still fails is recorded as a `{kind:"collect_error"}` record and
surfaced prominently in the report — never a silent drop.

### Check-run reviews (Graphite)

Some reviewers report a review **only as a check run**, not as a PR review or comment.
Graphite's "Graphite / AI Reviews" check run is the canonical case: a clean pass posts the
check run with **zero** PR comments, so the old scorecard read every clean pass as *No
response* and undercounted its engagement. Which bots report this way — and the exact
check-run name — is **data-driven** in the reviewer-source registry
(`scripts/lib/reviewer-sources.tsv`, `check_run_name` column); nothing is hard-coded to
`graphite-app`. For each in-window PR we make a **separate, lightweight REST call** for the
head commit's check runs (`GET /repos/{o}/{r}/commits/{sha}/check-runs`) so the heavy PR
page query stays under the resource limit. Each matched check run is classified:

- **Real response.** The run is completed, did **not** fail, and either has a `success`
conclusion or a summary that says the review ran. Latency is measured to its
`completed_at`. A run that failed does not count, even if its summary mentions the
review. Failed conclusions are `failure`, `cancelled`, `timed_out`, `action_required`,
`stale` and `startup_failure`.
- **Refusal.** The run is `completed/skipped` and its summary gives a decline reason, such
as "too large", "did not run" or a quota/rate-limit notice.
- **Contributes nothing.** Everything else: queued or in-progress runs, failed runs, and
runs skipped with an empty or unrelated summary. Without a PR review or comment, the PR
stays in **No response**.

Each PR node is normalized (pure `jq`) into two record kinds:

- `{kind:"pr", …}` — one per PR (the eligible-PR **denominator**).
- `{kind:"bot_pr", …}` — one per tracked bot that touched the PR, carrying its
`real_responses` / `refusals` split (a rate-limit-only touch is a refusal), latency to
first real response, verdict counts, inline-comment count, thread resolution, and reactions.
`real_responses` / `refusals` split (a rate-limit-only touch is a refusal, a check-run
"too large" skip is a refusal), latency to first real response, verdict counts,
inline-comment count, thread resolution, and reactions. A check-run clean pass adds **0**
to the review-event count but fills the `real_responses` / reviewed bucket.

Three diagnostic record kinds keep collection gaps visible (never silent, [#1908](https://github.com/petry-projects/.github-private/issues/1908)):

- `{kind:"collect_error", repo, reason}` — a repo that could not be collected after retries.
- `{kind:"truncation", repo, pr, connection}` — a nested connection (≥50), a review thread's
comments (≥20), or the per-repo PR pagination (`MAX_PR_PAGES`) that hit its fetch cap, so a
bot's reviews/comments on that PR — or a repo's later in-window PRs — may be undercounted.
- `{kind:"check_run_error", repo, pr}` — a per-PR check-run REST fetch that failed, so a
check-run reporter's clean pass on that PR may be missed and miscounted as *No response*.

Human authors and untracked bots are dropped at normalization time.
All are rendered in a **Collection health** section at the top of the report. Human authors
and untracked bots are dropped at normalization time.

## Metrics

Everything is deterministic. **Reviews** and **Rate-limited** count *every event*, not
Everything is deterministic. **Reviews** and **Refused** count *every event*, not
distinct PRs — GitHub creates a new review submission each time a bot re-reviews after a new
commit, so a PR reviewed across 5 commits contributes 5 reviews. This matters: on live data
CodeRabbit produced ~2× more review events than the distinct-PR count would suggest (up to 11
Expand All @@ -66,11 +110,11 @@ reviews on a single PR).
| **Total PRs** | Review-eligible (non-draft) PRs active in the window; the denominator each row is measured against. |
| **Reviews** | Count of reviews the bot submitted, **each occurrence** (multiple per PR from multiple commits all count). A bot that posts no formal review but delivers its verdict as a top-level comment (e.g. SonarCloud's quality-gate comment) has that comment counted as its review; bots that do submit formal reviews are unaffected, so their extra summary comments are never double-counted. Rate-limit notices are never counted. |
| **✅ / 🔄** | Of those reviews, how many carried state APPROVED / CHANGES_REQUESTED. |
| **Rate-limited** | Count of out-of-quota / rate-limit refusal events (detected by body text via the shared gate pattern), each occurrence. |
| **No response** | Eligible PRs the bot never engaged with at all — no review, comment, or refusal. |
| **Refused** | Count of refusal events — out-of-quota / rate-limit notices (detected by body text via the shared gate pattern) **and** a check-run reporter's own decline (a `too large` / `did not run` skip) — each occurrence. |
| **No response** | Eligible PRs the bot never engaged with at all: no review, no comment, no refusal, and **no check run classified as a real response or a refusal**. For a check-run reporter (Graphite), a clean-pass check run counts as engagement, so it no longer lands here. A queued, failed or unexplained-skip run still leaves the PR in **No response**. |
| **Latency p50 / p95** | Seconds from PR creation to the bot's first **real** review (refusals excluded, so quota notices don't pollute the percentiles). Targets the "review arrived after auto-approval" failure mode (PR #453). |
| **Coverage overlap** | PRs reviewed by ≥2 bots — a redundancy vs specialization signal. |
| **Trend** | Week-over-week Δ (▲/▼) on Reviews and Rate-limited (event counts) vs last week's snapshot. Arrows are directional only. |
| **Trend** | Week-over-week Δ (▲/▼) on Reviews and Refused (event counts) vs last week's snapshot. Arrows are directional only. |

Per-PR bucket counts (`reviewed_prs` / `refused_prs` / `no_response_prs`, which partition the
eligible PRs), plus thread-resolution and reaction counts, are still computed and stored in the
Expand Down Expand Up @@ -157,7 +201,8 @@ ORG=petry-projects LOOKBACK_DAYS=7 GH_TOKEN="$(gh auth token)" \

| File | Role |
|---|---|
| `scripts/lib/advisory-review-gate.sh` | Single source of truth for bot logins + rate-limit body patterns (reused, not forked). |
| `scripts/lib/reviewer-sources.tsv` + `scripts/lib/reviewer-sources.sh` | The reviewer-source registry ([#1425](https://github.com/petry-projects/.github-private/issues/1425)) + its sourced lookup helpers — the single source of truth for bot logins and the `check_run_name` map, projected by the gate and this report. |
| `scripts/lib/advisory-review-gate.sh` | Advisory approval gate; rate-limit body patterns (reused, not forked). Its bot list is a registry projection. |
| `scripts/lib/comment-noise.sh` | Pure no-action agent-comment classifier + `cn_render_noise_section` (the noise metric). Unit-tested in `tests/comment_noise.bats`. |
| `scripts/reviewer_report.sh` | Org-wide GraphQL collection (`main` / `collect_org_reviews`) + pure normalization, aggregation, and rendering (`aggregate_snapshot`, `render_reviewer_report`). |
| `tests/reviewer_report.bats` | Unit tests for the pure normalize / aggregate / render path (no network). |
Expand All @@ -173,13 +218,27 @@ ORG=petry-projects LOOKBACK_DAYS=7 GH_TOKEN="$(gh auth token)" \
| `REVIEWER_REPORT_OUT` | Optional path to also write the report to (used by the workflow to post the issue comment). |
| `REVIEWER_SNAPSHOT_OUT` | Optional path to write this week's per-bot snapshot (uploaded as the WoW artifact). |
| `REVIEWER_PREV_SNAPSHOT` | Optional path to last week's snapshot for deltas; `main()` fetches it automatically in CI. |
| `GH_OP_TIMEOUT` / `COLLECT_CONCURRENCY` / `MAX_PR_PAGES` | Per-call timeout (s), concurrent per-repo sweeps, and PR-page cap per repo. |
| `GH_OP_TIMEOUT` / `COLLECT_CONCURRENCY` / `MAX_PR_PAGES` | Per-call timeout (s), concurrent per-repo sweeps, and PR-page cap per repo (default 50). |
| `PR_PAGE_SIZE` / `PR_FETCH_MAX_ATTEMPTS` | PRs per GraphQL page (default 10 — kept light to stay under the resource limit) and maximum fetch attempts, including the initial request, before the repo is recorded as a `collect_error` (default 4). |

## Known limitations

- **GraphQL login form**: bot logins are matched without the `[bot]` suffix (the GraphQL
form). If a bot ever posts under a different identity, add it to the shared registry.
- **Latency denominator**: PRs opened as drafts and later marked ready use creation time
as the latency baseline in v1, which can overstate latency for long-draft PRs.
- **Per-repo pagination**: bounded by `MAX_PR_PAGES` (default 20 × 25 PRs). A repo with
more in-window PRs than that cap would be truncated; the cap is generous for this org.
- **Per-repo pagination**: bounded by `MAX_PR_PAGES` (default 50) × `PR_PAGE_SIZE` (default
Comment thread
don-petry marked this conversation as resolved.
10 PRs). A repo with more in-window PRs than that cap is truncated; when the cap stops a
repo it emits a `{kind:"truncation", connection:"pullRequests"}` record surfaced in the
**Collection health** section, so the scorecard is marked as a floor rather than silently
dropping the overflow. The cap is generous for this org.
- **Nested-connection cap**: a PR's `reviews` / `comments` / `reviewThreads` are fetched up
Comment thread
don-petry marked this conversation as resolved.
to 50 each, and each review thread's `comments` up to 20. A PR that exceeds 50 on any of
those connections — or 20 comments on any single thread — emits a `{kind:"truncation"}`
record (the per-thread case as `connection:"reviewThreads.comments"`) rendered in the
**Collection health** section, so an undercount there is stated explicitly rather than
hidden. The scorecard counts every node it did fetch, including bots beyond the 50th review.
- **Check-run summary text**: a completed, non-failed check run is read as *review ran* when
its conclusion is `success` or its summary matches `review ran`. A reporter that changes its
summary wording would need the matcher updated. The reporter set and check-run name live
in `scripts/lib/reviewer-sources.tsv`.
13 changes: 13 additions & 0 deletions scripts/lib/reviewer-sources.sh
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,19 @@ reviewer_sources_advisory_gate_logins() {
awk -F'\t' '!/^[[:space:]]*#/ && $1 != "" && $4 == "yes" { print $1 }' "$REVIEWER_SOURCES_MANIFEST"
}

# reviewer_sources_check_run_reporters
# Sources that report a review through a check run (column 5 != "-"), emitted as
# "<login>\t<check_run_name>" one per line. A bot listed here posts each review as
# a check run whose completed run counts as a real response even with zero PR
# comments/reviews (the Graphite clean-pass case, #1908). Empty output means no
# registered source reports via check runs. Data-driven: the scorecard reads this
# instead of hard-coding graphite-app.
reviewer_sources_check_run_reporters() {
_reviewer_sources_manifest_or_die reviewer_sources_check_run_reporters || return 1
awk -F'\t' '!/^[[:space:]]*#/ && $1 != "" && $5 != "" && $5 != "-" { print $1 "\t" $5 }' \
"$REVIEWER_SOURCES_MANIFEST"
}

# reviewer_sources_trusted_bots_csv
# dev-lead's webhook-facing trusted set: each trusted login with a "[bot]"
# suffix, joined by commas. This is exactly the TRUSTED_BOTS default.
Expand Down
27 changes: 17 additions & 10 deletions scripts/lib/reviewer-sources.tsv
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,29 @@
# (creates_threads=no) MAY be advisory-only-and-non-blocking (dev_lead_trusted=no)
# with the reason recorded in the rationale column.
#
# schema_version: 1
# schema_version: 2
#
# Columns (tab-separated):
# login — the bot's GraphQL App login (no "[bot]" suffix). dev-lead's
# webhook-facing trusted set appends "[bot]" to each login.
# creates_threads — yes|no: posts inline review comments (→ blocking review thread).
# dev_lead_trusted — yes|no: in dev-lead's trusted set (dev-lead may act on it).
# advisory_gate — yes|no: one of the bots the advisory approval gate waits on.
# check_run_name — the exact check-run name this bot reports its review through,
# or "-" when it reports only via PR reviews/comments. A bot with
# a check_run_name posts each review as a check run (e.g. Graphite's
# "Graphite / AI Reviews"); a completed run whose summary says the
# review ran is a real response EVEN WITH ZERO comments and no PR
# review, so the scorecard fetches it separately (#1908). Data, not
# code: the scorecard reads this column instead of hard-coding logins.
# rationale — why this bot is classified as it is.
#
# login creates_threads dev_lead_trusted advisory_gate rationale
copilot-pull-request-reviewer yes yes yes Inline review comments; dev-lead addresses the findings.
gemini-code-assist yes yes yes Inline review comments; dev-lead addresses the findings.
chatgpt-codex-connector yes yes yes Inline review comments (pull_request_review_comment); dev-lead addresses the findings.
coderabbitai yes yes no Inline review comments; tracked by the scorecard and rate-limit detector but not part of the advisory approval-wait set.
sonarqubecloud no yes yes Top-level quality-gate comment only — no inline review threads; trusted so dev-lead acts on the comment via the issue-comment path.
qodo-code-review yes yes yes Inline review comments (#1355 recognized it for gating/scorecard; trusted for acting per #1425).
codeant-ai yes yes yes Inline review comments (#1355 recognized it for gating/scorecard; trusted for acting per #1425).
graphite-app yes yes yes Inline review comments; root cause of the 2026-08-02 unclearable-thread deadlock — now trusted so dev-lead can act on its findings (#1425).
# login creates_threads dev_lead_trusted advisory_gate check_run_name rationale
copilot-pull-request-reviewer yes yes yes - Inline review comments; dev-lead addresses the findings.
gemini-code-assist yes yes yes - Inline review comments; dev-lead addresses the findings.
chatgpt-codex-connector yes yes yes - Inline review comments (pull_request_review_comment); dev-lead addresses the findings.
coderabbitai yes yes no - Inline review comments; tracked by the scorecard and rate-limit detector but not part of the advisory approval-wait set.
sonarqubecloud no yes yes - Top-level quality-gate comment only — no inline review threads; trusted so dev-lead acts on the comment via the issue-comment path.
qodo-code-review yes yes yes - Inline review comments (#1355 recognized it for gating/scorecard; trusted for acting per #1425).
codeant-ai yes yes yes - Inline review comments (#1355 recognized it for gating/scorecard; trusted for acting per #1425).
graphite-app yes yes yes Graphite / AI Reviews Inline review comments AND clean passes reported ONLY as the "Graphite / AI Reviews" check run (no PR review) — the scorecard must read that check run or every clean pass is miscounted as No response (#1908). Root cause of the 2026-08-02 unclearable-thread deadlock — trusted so dev-lead can act on its findings (#1425).
Loading
Loading