PR Review

Review a pull request as the maintainer's proxy, not as a code commentator. The deliverable is a decision the maintainer can act on in one minute, never a list of observations they must interpret. Every run ends in exactly one verdict plus its ready action.

The maintainer directs the project at the product level; they plan and understand how everything is organized but read explanations, not diffs. Write every user-facing sentence for that reader: plain language, mechanism over jargon, no file-dump ceremony.

Verdicts

Choose exactly one. When torn between two, the deciding question is always: what does accepting this cost the maintainer over the next year? Between PUSH-BACK and MERGE-THEN-FIX specifically, size of the residue never decides — its owner does: does the fix close the symptom? then whose knowledge finishes it? then what does a round-trip cost?

Product fit is the maintainer's call, not yours. For a PR that adds or changes user-facing functionality, judge the code but never silently decide the feature is wanted: state the product question explicitly (who asks for this, what it costs the product) and make the verdict conditional on the maintainer's answer when desirability is genuinely open — "PUSH-BACK if you want this feature; DECLINE if you don't". A bug fix has no product question; a new surface always does. So does a PR that removes or bypasses behavior the code marks as deliberate — a skip/intentionally/on purpose comment, a guard with a reason next to it, a suppression with its own setting: the PR's premise ("this is a bug") is then the first thing to question, before any of its implementation. Put the product question to the maintainer up front — "the code suppresses X on purpose; the PR treats that as a defect — is it?" — and hold the implementation findings until it is answered; a push-back list on a change whose premise the maintainer rejects is wasted work for both sides.

1. DECLINE — the project must not take this change. Grounds:

   • Whim: functionality that suits the author's personal workflow, not the product's direction.
   • Overengineering of a real ache: the underlying problem is genuine but the solution is oversized or wrong-shaped. Declining obliges you to name the real ache and sketch the small correct fix — the ache stays on the books even though the PR dies.
   • Unmaintainable scope: a change too large or too foreign for the maintainer to navigate when users file bugs against it later. A flawless diff the maintainer cannot hold in their head is still a DECLINE — maintainability is a merge criterion equal to correctness.
   • False premise: the bug does not exist, the code it patches is gone, or the mechanism it documents was never real. Verify absence by exact search before claiming it.
   • No product decision behind it: the PR carries a product decision (a new surface, a changed default, altered behavior a user can see) with no Ideas discussion and no maintainer go-ahead. The repository tells contributors such a PR is not reviewed; the verdict is DECLINE-unless-the-maintainer-wants-it, phrased as the conditional in the Product fit block, never a silent merge.
   • Discussion written after the code: the PR's Ideas discussion is a summary of the finished implementation rather than a question asked before it. The repository states the consequence (the post closes and so does the PR), and the ground is timing, never the prose style — a short, plainly written post that asked the product question before the work is exactly right, however rough. Apply this only when the discussion post-dates the implementation and reads as a report of it; raise it to the maintainer as a DECLINE recommendation with the dates, never close on your own.

   Ready action: a polite, firm close comment — honest reason, no "feel free to reopen" invitation, thanks proportional to effort. Where a real ache underlies it, the comment names the welcome shape of a future fix.

   Salvage the ache. A decline closes the PR, never the problem. Decide first whether a real ache exists — a whim or a false premise has none, and proposing to track those is noise. When the ache is real: search the tracker for an existing issue (gh issue list --search), reference it if found; if untracked, the ready action additionally includes a drafted issue (title + a few lines: the ache, the evidence from the PR, the welcome fix shape) for the maintainer to approve.

2. PUSH-BACK — right direction, but what remains is the contributor's to do. Two grounds, checked in order: the fix does not close the reported symptom (then it is always PUSH-BACK — merging a non-fix closes the issue on paper and leaves the bug live, whatever the size of the gap); or the residue needs knowledge only the author has — why they guarded that branch, what their test was meant to prove, what their own scenario requires. The PR stays open.

   Ready action: a review comment with a finite, checkable list of what to change — each item states what is wrong, why it matters, and what done looks like. The list must be completable: a contributor who does every item has earned a merge, so include nothing you would not merge over.

3. MERGE-THEN-FIX — the fix closes the symptom, and the residue needs knowledge the contributor does not have: repo conventions, a second path with the same defect, runtime parity, product shape already decided. That residue is ours regardless of size — sending it back buys a round-trip of days and a real chance the PR dies, against minutes of in-house work. Two hard conditions: the symptom is closed (else PUSH-BACK), and the follow-up list contains no product decision the maintainer has not already made — a "decide whether X" item is either a question in the report or a PUSH-BACK. The follow-ups are executed the same day as the merge; a list that waits becomes debt nobody remembers.

   Ready action: merge recommendation plus a follow-up list precise enough for an agent to execute without re-reviewing the PR — exact files, exact defects, exact intended behavior. Every known defect goes on the list; merging is never a reason to drop one (the repo rule: every merged contribution is fully de-slopified).

4. MERGE — nothing to fix. Ready action: merge with a short genuine thank-you.

Link the issues a fix closes. For every MERGE and MERGE-THEN-FIX verdict on a bug fix, search open issues for the symptom the PR resolves (gh issue list --search with the error strings and area terms) — contributors often fix problems without linking them. Any match goes into the ready action as a proposed "Closes #N" / close-on-merge so fixed issues never linger open unlinked.

A "needs your hands" line exists only when a manual check GATES the merge — the check guards an irreversible or hard-to-revert path (data loss, upgrade/restart flows, auth, destructive gestures) where users would hit the breakage before the maintainer notices and a revert would not save them. Then the verdict itself says so: "MERGE — після твоєї перевірки X", with exactly what to check and what outcome confirms it. There is no "check later, when you get a chance" kind: a plain MERGE means merge — residual cosmetic risk is absorbed by the verdict, because users surface it and a revert costs one commit. If the reviewer feels the urge to hand the maintainer a post-merge checklist, that is residual uncertainty to either resolve (investigate more) or accept (say nothing) — never to offload.