Code Review
Review code with precision. Flag real issues, skip noise.
Severity Levels
🔴 Critical (Block merge)
- •Security vulnerabilities (injection, auth bypass, secrets exposure)
- •Data loss or corruption risks
- •Breaking changes to public APIs
- •Race conditions with data integrity impact
- •Unhandled errors that crash the system
🟡 Important (Should fix)
- •Bugs that affect functionality
- •Performance issues (N+1 queries, unbounded loops, memory leaks)
- •Missing error handling for likely failure cases
- •Violation of established patterns/architecture
- •Missing input validation on external data
- •Hardcoded values that should be configurable
🟢 Minor (Consider fixing)
- •Naming improvements for clarity
- •Code duplication that could be extracted
- •Missing edge case tests
- •Documentation gaps
- •Style inconsistencies not caught by linter
⚪ Skip (Don't mention)
- •Issues a linter/formatter would catch
- •Personal style preferences not in guidelines
- •Pre-existing issues not introduced by this change
- •Nitpicks that don't improve the code meaningfully
Review Checklist
Correctness
- •Does the code do what it claims?
- •Are edge cases handled?
- •Are error conditions caught and handled appropriately?
Security
- •User input sanitized before use?
- •Auth/authz checks in place?
- •Secrets kept out of code?
- •SQL/command injection prevented?
Performance
- •Database queries efficient? (indexes, N+1)
- •Appropriate caching?
- •Unbounded operations paginated?
- •Expensive operations async where appropriate?
Maintainability
- •Clear naming and structure?
- •Appropriate abstractions?
- •Tests cover the important paths?
- •Would a new team member understand this?
Architecture
- •Follows established patterns?
- •Correct layer boundaries?
- •Dependencies flow in the right direction?
- •No circular dependencies introduced?
Giving Feedback
Format
code
🔴 path/to/file.ts:42 - Brief issue description → Suggested fix or approach
Good Feedback
- •Specific: Points to exact location and problem
- •Actionable: Suggests a concrete fix
- •Educational: Explains why it matters
- •Proportional: Severity matches actual impact
Avoid
- •Vague complaints ("this could be better")
- •Rewrites without explanation
- •Blocking on style preferences
- •Piling on - one comment per issue type is enough
Confidence Filter
Before reporting an issue, ask:
- •Is this a real problem or a preference?
- •Would a senior engineer call this out?
- •Is this introduced by this change (not pre-existing)?
- •Does it violate explicit guidelines or just implicit taste?
Only report issues where you're 80%+ confident they matter.
Review Output Structure
code
**Summary:** One-line assessment of the change **Issues:** 🔴 Critical issues (if any) 🟡 Important issues (if any) 🟢 Minor suggestions (limit to 2-3) **Good:** What the code does well (always include something)
Anti-patterns to Watch
| Pattern | Problem | Look For |
|---|---|---|
| God object | Too many responsibilities | Class with 10+ methods, mixed concerns |
| Primitive obsession | Stringly-typed data | IDs as strings, status as raw strings |
| Deep nesting | Hard to follow logic | 4+ levels of indentation |
| Magic values | Unclear intent | Numbers/strings without constants |
| Silent failures | Bugs hide | Empty catch blocks, ignored return values |
| Temporal coupling | Order-dependent calls | Init must be called before use |
| Feature envy | Wrong home for logic | Method uses more of another class |
Positive Signals
Also call out good patterns:
- •Clean separation of concerns
- •Good test coverage on critical paths
- •Defensive error handling
- •Clear naming that documents intent
- •Appropriate use of types
- •Thoughtful API design