Code-Review

Code Review

Find the real defects a change causes or exposes, rank them by what they cost users and the business, and give a clear verdict. Review read-only: change nothing while reviewing.

Gather the context

• The diff against its base, the commits, and the task or spec it implements, including the expected change in behavior.
• The project's written standards (AGENTS.md, CONTRIBUTING, style guides). What linters and formatters enforce is not your job.
• Around every hunk: the whole function and file, the callers of changed functions, the readers of changed data, configuration, migrations and tests.

What to check, in this order

1. The spec: requirements missing or partial, behavior nobody asked for, requirements implemented wrongly. Where the spec is silent, a reasonable user's expectation still counts.
2. Effects beyond the diff: callers broken by a changed signature, return value or error; data in a new shape that old readers or old rows cannot handle; changed defaults; removed behavior someone relies on.
3. Edge and failure paths: empty, null, boundaries, invalid input; errors swallowed or mapped wrongly; partial failure; retries and duplicates; races and lost updates, which you find by reasoning, not by running.
4. Security: server-side authorization for the specific record, input validation, injection, secrets and personal data in code, logs and responses.
5. Data: migrations safe on existing rows and while old code still runs; transactions around writes that belong together.
6. Tests: present for the change, checking behavior rather than implementation, and able to fail. Probe one: invert a condition the change adds and see whether a test goes red; if none does, name the missing case.
7. Design: fits the existing patterns, no more complex or general than the task needs, leaves the code base healthier.
8. Performance where it matters: N+1 queries, unbounded loads, heavy work in hot paths.

Severity

Use exactly the four values a review's findings carry: critical, major, minor, suggestion. Critical and major need concrete evidence (file and line, quoted code, a failing case).

• critical: breaks a main flow, loses or corrupts data, moves money wrongly, opens a security or tenant hole. Blocks the merge.
• major: a real defect or regression with limited reach, code that fails an acceptance criterion or breaks existing behavior, a missing test for risky logic, poor error handling. Fix before merging.
• minor: a real but small problem that can be fixed in passing. Never blocks.
• suggestion: an optional improvement, or a style or naming preference; never reported as a defect. Lead with what matters: one structural problem outweighs ten suggestions.

Each finding

• Location (file and line or function), what is wrong, a concrete failing scenario (this input or state gives this wrong result), the impact on users or the business, and a suggested fix.
• Verify before reporting: read the code you comment on, follow the caller, run the test when you can. Mark it "possible" if you could not confirm it.
• List what you considered and set aside as out of scope, one line each with the reason, so nothing is dropped silently.
• Name what was done well, briefly and specifically.

Verdict

End with one of: ready to merge, ready after the listed fixes, or not ready. Give a one- or two-sentence reason, the checks you ran, and the parts you did not review.

Avoid

• "Looks good" without reading every changed line, or comments on code you did not read.
• Inflating suggestions into defects, or softening a real bug.
• Blocking on personal preference, or asking for rewrites the task does not need.