devjarus/code-review

Code Review

A structured checklist for domain leads reviewing specialist-produced code. Work through each section in order — correctness first, then the rest.

---

Checklist

1. Correctness

• [ ] Does the code satisfy the acceptance criteria in the task contract?
• [ ] Are edge cases handled (empty input, zero, null, large values, boundary conditions)?
• [ ] Is error handling present and correct for all failure paths?
• [ ] Does the logic match the intended algorithm? Trace through a representative example.
• [ ] Are there off-by-one errors, incorrect comparisons, or missed negations?

2. Security

• [ ] Is all external input validated (type, length, format, allowed values)?
• [ ] Are SQL queries parameterized (no string concatenation)?
• [ ] Is output encoded for the rendering context (HTML encoding, JSON escaping)?
• [ ] Are CSRF protections in place on state-mutating endpoints?
• [ ] Are there hardcoded secrets, tokens, or credentials anywhere?
• [ ] Are authorization checks present — not just authentication but resource-level ownership checks?

3. Performance

• [ ] Are there N+1 query patterns (a query inside a loop)?
• [ ] Is there unnecessary computation inside hot paths or loops?
• [ ] Are large data structures copied where a reference would suffice?
• [ ] Are expensive operations (network calls, disk I/O) performed synchronously when they could be async or deferred?

4. Conventions

• [ ] Does the code follow patterns established in CLAUDE.md and the codebase?
• [ ] Are names clear, consistent with existing naming conventions, and self-documenting?
• [ ] Is formatting consistent with the project's linter/formatter rules?
• [ ] Are new abstractions necessary, or does existing code already cover the need?

5. Tests

• [ ] Do tests exist for the changed behavior?
• [ ] Do tests verify behavior (not implementation details)?
• [ ] Are failure cases and edge cases tested?
• [ ] Do tests run and pass in CI?

6. Maintainability

• [ ] Can a new team member understand this code in 5 minutes?
• [ ] Are there magic numbers or unexplained constants? (They should be named.)
• [ ] Is the code modifiable without requiring deep knowledge of the surrounding system?
• [ ] Is there duplication that should be extracted into a shared utility?

---

How to Give Feedback

Be specific with location. Reference file:line rather than describing the problem abstractly.

Explain why. "This is wrong" is not actionable. "This creates an N+1 query because getUser is called inside a loop — consider batching with getUsersByIds" is.

Distinguish severity. Use consistent labels so the author knows what must change versus what is optional:

| Label | Meaning | |-------|---------| | blocker | Must be fixed before merge. Correctness, security, or data-loss risk. | | major | Should be fixed before merge. Performance, maintainability, or convention issue. | | minor | Fix if you have time. Small improvements, style preferences. | | nit | Trivial. Take it or leave it. |

Offer alternatives. When blocking a change, suggest a path forward. Don't just reject — help.

Separate opinion from requirement. Prefix personal preferences with "I'd prefer..." to distinguish them from objective issues.