PR Reviewer — Workflow

A structured procedure for reviewing a pull request the way a careful senior engineer would: gather full context first, classify risk before reading line-by-line, work a fixed checklist, and report findings in a consistent, severity-tagged format instead of a stream-of-consciousness comment.

Inputs: pr_ref (required), base_ref (default main), post_comments (default false).

Step 1 — Gather context

Do not start reading the diff yet. First collect:

1. The diff itself.
   • GitHub PR: gh pr diff <pr_ref> (also gh pr view <pr_ref> --json title,body,files,additions,deletions).
   • Local branch/range: git diff <base_ref>...<pr_ref> (three-dot = changes since merge-base).
2. The PR description / linked issue. Understand why this change exists before judging how it was made. A 400-line diff with a one-line "fixes bug" description is itself a finding (see Step 2).
3. The file list with change size, not just the diff text: git diff --stat. This tells you where to spend attention — a 3-line config tweak and a 300-line new module deserve very different scrutiny.
4. CI status. If checks are failing, note it up front; don't duplicate what a linter or test runner already caught.
5. Blast radius. Grep for callers/importers of any changed public function, exported type, API route, or schema. A correct-looking change can still break callers that assumed the old contract.

Step 2 — Classify risk

Before checklist review, assign an overall risk tier. This sets how much scrutiny to apply and belongs at the top of the final report.

Also flag scope mismatch here: if the PR description says "fix typo" but the diff touches auth middleware, that mismatch is itself a High-severity finding — surprise scope is a red flag independent of whether the code is correct.

Step 3 — Work the checklist

Apply rules/review-checklist.md in order: correctness, security, tests, performance. Do not skip categories because the PR "looks like" a docs change — a docs-only diff can still leak an internal hostname or credential. For each checklist item that finds something, record: file:line, what's wrong, why it matters, and a concrete suggested fix (not just "this looks off"). If a checklist item is genuinely not applicable (e.g. no SQL in a frontend-only PR), skip it silently rather than padding the report.

Step 4 — Verify tests actually exercise the change

Don't just check "tests were added" as a checkbox. For each new/changed behavior:

• Find the test that covers it and confirm the assertion would actually fail if the behavior regressed (not just "the function was called").
• Check the diff for tests that were weakened to make CI pass (loosened assertions, increased timeouts, skipped/.only'd tests, removed edge cases).
• For bug fixes, confirm there's a regression test reproducing the original bug.

Step 5 — Compose the report

Use templates/review-comment.md. Order findings by severity (blocker → major → minor → nit), each with file:line and a suggested fix. Lead with the risk tier from Step 2 and a one-paragraph summary of what the PR does and whether it should merge as-is, merge with changes, or needs a design discussion first.

Do not manufacture findings to look thorough — "no issues found in this category" is a valid and useful line. Nits (style, naming, minor readability) go at the bottom and should be clearly optional; never block a PR on nits alone.

Step 6 — Deliver

• If post_comments is false (default): print the report and stop. This is the safe default — do not post anything.
• If post_comments is true: this is a side-effectful, externally-visible action. Confirm with the user which specific comments will be posted before submitting (gh pr comment / gh pr review), and never post secrets, internal URLs, or anything copied verbatim from private context into a public PR thread.

Stop conditions

• Diff is empty or pr_ref doesn't resolve → report the error, do not guess.
• Diff exceeds ~2000 changed lines → say so explicitly and either review in per-file batches or ask the user to scope the review (e.g. to specific files/directories) rather than silently skimming.
• Binary files or generated/vendored files (lockfiles, dist/, minified bundles) in the diff → skip them, note that they were skipped and why.