code-review

Code Review

The shared rubric for assessing a code change. Used both by a PR review and by the build cycle's branch-diff reviewer — the procedure differs (where the diff comes from, how the verdict is recorded), but the rubric below is the same.

Review the change in full context — read each changed file, not just the hunk. For a large change (>300 lines or >5 files): trace data flow through modified functions, check callers of anything whose signature or behaviour changed for regression risk, and check that tests cover the actual risk areas, not just the happy path.

Precision first — post only what matters

A review is only useful if people trust it. A noisy reviewer gets muted; every low-value comment you post spends the credibility of the ones that matter. So the bar is high-signal only — on what you post. Read "Where the gate applies" below first: if your output is consumed by a later stage rather than by a human, none of the filtering in this section is yours to do.

• Post only Critical and Important findings. Suggestions and Nits (below) are not worth a formal review comment on their own — drop them, or fold at most one genuinely valuable line into the summary. When in doubt about a finding's tier, leave it out of the review.
• If you cannot name the concrete impact — what breaks, and for which input or caller — do not post it. "This could be cleaner" is not a finding; "this crashes when items is empty because line 42 indexes [0]" is.
• One defect per comment, and distinct claims stay distinct. Never fold a second, independent defect into an "Additionally, …" sentence of the first — two defects sharing a comment get read, answered and tracked as one, and one of them is lost. Two comments may share a line. Keep each claim's precision too: "out of date" is not "wrong", and collapsing distinct claims into the harsher one overstates the review's case.
• Confidence gate — refute, don't doubt, and only at the end. Before you finalise, re-read each finding against the actual code and try to refute your own claim. Dropping it requires naming the specific thing that makes it wrong: the guard you missed, the caller that already validates, the type that makes the case unreachable. Unease is not a refutation. "I'm not certain" is an instruction to go and read the other side of the contract, not to delete the finding.