PR Review Sweep

Automated reviewers (cubic-dev-ai, chatgpt-codex-connector / Codex) and humans leave findings on PRs asynchronously — often seconds to minutes after the PR opens, and sometimes only after it merges. A PR that built green is not a reviewed PR. This skill makes the sweep a deliberate step so real P1s (races, state-clobbers, unguarded destructive paths) don't slip through on the exact kind of change where they hurt most.

The rule that motivated this skill: a self-identified P1 on a destructive path was shipped as a "follow-up" and the reviewers confirmed it (plus three more) after merge. Cheap to catch at sweep time; a roundtrip to catch later.

When to sweep (and when not to)

Always sweep — material PRs:

• Touches shipped source / logic: core/**, examples/**, ship/**, tools/cli/**, src/** (Shipyard), CMake/build wiring.
• Concurrency, lock ordering, state machines, serialization, RT-audio, or any destructive / mutating path (delete, abandon, overwrite, force). Treat these as high-risk regardless of diff size.
• Large diffs, or a stack of commits — more surface, more asynchronous review.

Skip is fine — non-material PRs:

• Docs-only / comment-only / pure rename / formatting.
• Trivial config or version-bump-only changes.

If unsure, sweep. It costs one API read and a few minutes of triage.

The sweep

Use ghapp for GitHub API reads (its own rate-limit bucket; plain gh burns the shared personal token). Use gh for creating/merging PRs.

Pull all three comment surfaces — findings land in different places:

REPO=danielraffel/<repo>; N=<pr>
# Inline review comments (code-anchored — where the bots put P1/P2 findings)
ghapp api repos/$REPO/pulls/$N/comments \
  --jq '.[] | "--- \(.path):\(.line // .original_line) by \(.user.login)\n\(.body)\n"'
# PR-level reviews (approve/request-changes summaries)
ghapp api repos/$REPO/pulls/$N/reviews \
  --jq '.[] | "\(.user.login) [\(.state)]: \(.body)"'
# Issue comments (human notes, "potential issues flagged here")
ghapp api repos/$REPO/issues/$N/comments \
  --jq '.[] | "--- by \(.user.login)\n\(.body)\n"'

Bot findings are usually tagged P1/P2 with a confidence and an "AI agents" prompt block. They are signal, not gospel.

Triage each finding

1. Verify against the actual code, not the bot's summary. Read the file at the anchored line. Confirm the failure scenario is reachable (concrete inputs → wrong output). Don't dismiss a P1 because it's "narrow" — a narrow race on a destructive path is still a must-fix.
2. Bucket: must-fix (correctness / data-loss / a live thing broken), should-fix (real but bounded), nit (style / naming). Default-off opt-ins still get their P1s fixed before anyone can enable them.
3. Don't blindly apply the bot's suggested patch — reproduce the reasoning, then write the fix that fits the code. Bots are right about what, often wrong about how.

Act

• PR still open: push the fixes to the branch; re-run the local gates; let the same reviewers re-review before merge.
• Already merged (common on Shipyard — see below): ship a follow-up PR that fixes every confirmed finding, with a test per fix (tests ship with fixes), then post a closeout comment on the original PR mapping each finding → fix.
• Ship the test. Every correctness fix lands with the Catch2 / cargo test case that would have caught it. "It compiles" is not the acceptance criterion.

Reporting the PR's state after a sweep

• Quote the required-check state from shipyard landing --pr <n> (its verdict line where it prints one) or the head's statusCheckRollup, read at the time of the claim, with the head SHA. Do not summarize from memory.
• Never call a failure a flake, infra, "not a code failure", or "still in progress" while a required check is red on the current head, or when the same test failed on an earlier head of the PR. A repeat is a code signal.
• If the PR is armed or queued, push only real fixes; do not rebase it or merge main into it (the queue does that, and moving the head cancels the running gate). The ci skill has the full rules.

Timing traps

• Reviews are asynchronous. Opening the PR does not mean the review has run. For a material PR, either wait for the review run before merging, or commit to a post-merge sweep and be ready to follow up.
• Shipyard has no required status checks → gh pr merge --auto fires the moment the PR is mergeable, which is typically before the reviewers finish. So on Shipyard, the post-merge sweep is not optional — always re-read pulls/$N/comments after the merge lands and follow up if anything is P1/P2.
• Pulp PRs route through shipyard pr and merge on green; Codex review comments can also arrive post-merge. Same discipline: sweep after, follow up.

Cross-repo

This applies to any repo the workspace ships to — Pulp and Shipyard both get automated bot review. This skill lives in Pulp's .agents/skills/ (the shared source of truth); the practice is identical when working in the Shipyard tree.