Code Review Rules

Code review's purpose is catching bugs, design problems, and knowledge gaps before they ship — not enforcing a reviewer's personal style preferences. Review for correctness first, and let tooling (not humans) catch everything mechanical.

Correctness first, style last

• Prioritize review attention on correctness (does this do what it claims, are edge cases handled, is the logic right), then design (is this the right approach, does it fit the existing architecture), then only last on style/formatting — a reviewer who opens with formatting nitpicks trains authors to submit smaller, more defensive diffs instead of better ones.
• Never block a PR on a pure style preference that isn't already encoded in the linter/formatter config — if it matters enough to block on, it matters enough to add to the automated config so it's enforced consistently instead of depending on which reviewer is assigned.
• Flag design-level concerns (this will not scale, this duplicates existing logic, this changes a public contract) explicitly and early in the review — a design objection raised after several rounds of nitpicks wastes both people's time on details that may not survive the redesign.

Small diffs get better reviews

• Keep pull requests scoped to one logical change — a reviewer can hold a 200-line focused diff in their head and catch real issues; a reviewer facing a 2000-line diff spanning five concerns skims and rubber-stamps, which defeats the purpose of review entirely.
• Split unrelated changes (a refactor plus a feature, a dependency bump plus a bug fix) into separate PRs even when they touch the same files — bundling them makes it impossible to review, approve, or revert them independently.
• As an author, if a PR is unavoidably large, write a summary that guides the reviewer through it (what changed and why, in what order to read the files) rather than leaving them to reconstruct intent from the diff alone.

Comment severity labels

• Label review comments by severity so the author can triage at a glance: blocking (must be fixed before merge), suggestion (worth considering, author's call), nit (minor, non-blocking polish) — an unlabeled comment forces the author to guess whether it's optional, which either blocks merges unnecessarily or lets real issues slip through as "just a nit."
• Reserve "blocking" for genuine correctness, security, or architectural issues — overusing it on preferences erodes its meaning and slows every review down.
• Batch nits together and make clear collectively they aren't merge-blocking, so the author isn't stuck resolving twenty individually-ambiguous threads to find the two that matter.

Ask, don't command

• Phrase review comments as questions or observations ("what happens if this list is empty?", "have you considered X here?") rather than commands ("change this to X") — a question invites the author's reasoning and sometimes surfaces context the reviewer was missing; a command shuts that down and can be wrong without anyone noticing.
• When a change genuinely must be made a specific way (a security fix, a broken contract), state that plainly and explain why — asking a rhetorical question to soften a hard requirement is worse than being direct, since it obscures what's actually blocking.
• Keep tone collaborative and specific to the code, never to the author — critique the diff, not the person who wrote it.

Author responds to every comment

• As an author, respond to every review comment — even a comment you're not acting on deserves an explicit reply (why you disagree, or acknowledgment with no change) rather than silence, which leaves the reviewer unsure whether it was seen, agreed with, or ignored.
• Resolve a comment thread only after the underlying concern is actually addressed, not just replied to — resolving to clear the queue without a real fix or explanation defeats the review.
• If reviewer and author disagree after discussion, escalate to a synchronous conversation or a third opinion rather than looping indefinitely in comment threads — written back-and-forth is a poor medium for resolving a genuine disagreement.

Approval criteria

• Define what "approved" means for the team explicitly (all blocking comments resolved, CI green, at least N approvals) so approval isn't an ambiguous vibe check that varies by reviewer.
• Never approve a PR you haven't actually read because the author is trusted or the deadline is tight — an approval is a claim that you reviewed the change, and a rubber-stamped approval defeats the entire practice while looking identical to a real one in the PR history.
• Require re-review after a substantive post-approval change (not a typo fix) — an approval given on one version of the diff doesn't automatically cover a materially different version.

Automate what linters can catch

• Enforce formatting, import ordering, naming conventions, and other mechanical rules with a linter/formatter run in CI, not through human review comments — every mechanical nitpick a human types by hand is a rule that should have been a lint config instead.
• Run automated checks (lint, type-check, tests) before a human reviewer looks at the diff — reviewers shouldn't spend their attention on issues a machine would catch in seconds.
• When a recurring human comment pattern emerges across several PRs (the same category of mistake flagged repeatedly), treat it as a signal to add a lint rule or a CI check, not to keep repeating the comment.