code-review

Code review is the primary quality gate in modern software development. Poor review practices lead to slow feature velocity, missed defects, team friction, and knowledge silos. This skill codifies proven practices that prevent these failure modes. </context>

Key insights from research:

• Review effectiveness drops dramatically beyond 200-400 LOC or 60-90 minutes
• Fast review turnaround correlates with better delivery performance
• Clear comment classification reduces author-reviewer friction
• Two-pass review (design first, details second) catches more issues
• Modern code review spreads knowledge and raises team awareness—not just defect hunting

The skill balances prescriptive guidance with flexibility—providing clear workflows without being overly rigid. </contemplation>

Core Principles

What Authors Should Do

Use this format (skim-friendly):

## Context
<!-- Why is this needed? What problem does it solve? -->
<!-- Link to issue/ticket if applicable -->

## Changes
<!-- High-level summary of what changed (not every line) -->
- Key change 1
- Key change 2
- Key change 3

## Test Plan
<!-- How can reviewers verify this works? -->
- [ ] Unit tests: `npm test path/to/tests`
- [ ] Manual testing: Steps to reproduce
- [ ] Edge cases verified: List specific scenarios tested

## Deployment Notes
- [ ] Database migration: Yes/No (details if yes)
- [ ] Feature flag: Yes/No (name if yes)
- [ ] Configuration changes: Yes/No (what changed)
- [ ] Breaking changes: Yes/No (migration path if yes)
- [ ] Rollback plan: Describe how to safely revert

## Security Considerations
<!-- If touching auth/permissions/data/secrets -->
- Input validation strategy
- Authorization checks
- Data exposure risks addressed

What Reviewers Should Do

Focus on big picture before diving into details:

1. Read PR Description

   • Understand context: Why is this needed?
   • Verify approach makes sense
   • Check scope is appropriate

2. Assess Design & Architecture

   • Does this solve the right problem?
   • Is the approach sound?
   • Any architectural concerns?
   • Fits existing patterns or justified deviation?

3. Security & Risk Pass

   • Any obvious security vulnerabilities?
   • Authentication/authorization correct?
   • Data validation present where needed?
   • Use /review-security for auth/permissions/data changes

4. Scope Check

   • PR focused on single concern?
   • Size reasonable (< 400 lines ideal)?
   • No unrelated changes?

Decision Point: If major issues found (wrong approach, architectural problems, security flaws), provide feedback and STOP. No need for detailed review if design is flawed. </pass>

Only proceed if Pass 1 looks good:

1. Logic & Correctness

   • Logic is correct and handles edge cases
   • Error handling is appropriate
   • State changes are safe and intentional
   • Use /review-quality for TypeScript/logic review

2. Testing Quality

   • Tests cover new functionality
   • Tests are meaningful (not just for coverage)
   • Edge cases and error scenarios tested
   • Use /review-testing for test quality analysis

3. Code Quality & Readability

   • Code is readable and maintainable
   • Naming is clear and consistent
   • No obvious performance issues
   • Use /review-readability for style/naming analysis

4. Documentation

   • Public APIs have JSDoc comments
   • Complex logic has explanatory comments
   • README/docs updated if needed

Time Management: Set 30-minute timer. If you can't complete review in time:

• Stop and comment: "This is more complex than I can review in one session. Tagging @teammate."
• Or: Request PR be split if too large
• Or: Schedule dedicated review time and let author know </pass>

Comment Classification System

<comment_types> Use these prefixes to clarify feedback severity and reduce friction:

[critical] - Must fix before merge

• Security vulnerabilities
• Data loss or corruption risks
• Breaking changes without migration path
• Incorrect core logic

[issue] - Should fix, discuss if cannot

• Bugs or logic errors
• Missing error handling
• Significant performance problems
• Missing tests for core functionality

[suggestion] - Nice to have, author decides

• Better patterns or approaches
• Performance optimizations
• Refactoring opportunities
• Code organization improvements

[nit] - Purely cosmetic, optional

• Variable naming preferences
• Code style (should be automated)
• Whitespace, formatting
• Comment typos

[question] - Seeking clarification

• Understanding intent
• Asking about edge cases
• Requesting explanation of approach
• Verifying assumptions

[praise] - Calling out good work

• Excellent patterns worth highlighting
• Clever solutions
• Good test coverage
• Clear documentation </comment_types>

Final Decision

After both passes (< 30 minutes total):

• Approve: No blocking issues, ready to merge (or after minor nits addressed)
• Request Changes: Critical or issue-level items must be fixed before merge
• Comment: Leaving feedback but deferring final decision to another reviewer

Batch Nits: Leave all [nit] comments in single commen