Code Review

Use this when you have code changes (a PR, a diff, or modified files) and want a structured review covering test coverage, naming, error handling, security, and alignment with the story's acceptance criteria.

Process

Step 1: Gather inputs

Ask the user to provide:

1. Code changes — a diff, PR link (if MCP available), file paths, or pasted code. Include both new and modified files.
2. Story or acceptance criteria — the user story this code implements (optional but strongly recommended). Used to check that the code actually delivers what was specified.
3. Test files — the tests covering these changes (if separate from the diff).
4. Team conventions — any specific coding standards, linting rules, or conventions to check against (optional — the review will use general best practices if none provided).

Step 2: Review the changes

Evaluate the code across these dimensions:

Acceptance criteria alignment

• Does the code implement what the story specifies?
• Are all acceptance criteria covered, or are some missing from the implementation?
• Does the code do things the story didn't ask for (scope creep)?

Test coverage

• Are there tests for the changed code?
• Do tests cover happy path, error paths, and edge cases?
• Are tests testing behavior (not implementation details)?
• Are there missing test cases?
• Gherkin traceability: Is there a test for every Gherkin scenario in the story's AC? Any AC without a corresponding test?

Code quality

• Naming — are variables, functions, and classes named clearly? Can you understand the code without comments?
• Structure — is the code organized logically? Are functions doing one thing?
• Duplication — is there copy-pasted logic that should be extracted?
• Complexity — are there deeply nested conditionals, long functions, or hard-to-follow control flow?

Error handling

• Are errors handled, not swallowed?
• Do error messages help with debugging?
• Are edge cases handled (null inputs, empty collections, missing data)?
• Are external calls (APIs, database, file system) wrapped with appropriate error handling?

Architecture and system impact

• Coupling — does this change introduce tight coupling between modules or services? Could these components change independently?
• Data flow — does data flow in the expected direction? Are there new cross-service calls that should be async instead of sync?
• Observability — are new code paths instrumented? Are errors logged with sufficient context for debugging in production?
• Migration safety — if this changes a database schema or API contract, is it backward compatible? Is there a rollback path?
• Dependency direction — do dependencies point in the right direction? (Domain logic should not depend on infrastructure details.)

Security

• Input validation — is user input validated before use?
• Injection risks — SQL injection, XSS, command injection?
• Authentication/authorization — are access checks in place?
• Data exposure — are sensitive fields (passwords, tokens, PII) protected?
• Dependency concerns — any new dependencies with known vulnerabilities?

Style and consistency

• Does the code follow the project's existing patterns?
• Are imports organized consistently?
• Is formatting consistent with the rest of the codebase?

Step 3: Generate the review

Output a structured review:

Code Review: (PR title or description)

Summary

(One sentence: Approve / Approve with suggestions / Request changes)

Must-fix issues

(Issues that should be resolved before merging)

1. [file:line] (issue) — (why it matters) — (suggested fix)

Suggestions

(Improvements that would make the code better but aren't blockers)

1. [file:line] (suggestion) — (rationale)

Acceptance criteria coverage

Test coverage assessment

• (What's well tested)
• (What's missing)

Test quality

Related skills: Use /test-coverage-audit for a full AC-to-test mapping across the codebase.

Architecture impact

• (Assessment of coupling, data flow, observability additions)
• (Any concerns about system-level implications of this change)

What's good

• (Specific things done well — call out clean patterns, good naming, thorough error handling)

Step 4: Discuss

Ask the user:

• Any issues you'd like me to explain further?
• Want me to write fixes for the must-fix issues?
• Want me to write the missing tests?

Step 5: Team-level review standards (for engineering leads)

If the user is setting up or improving code review culture for a team, generate team review standards:

## Code Review Standards -- {{team name}}

### Review SLOs
| Change type | Target turnaround | Escalation if missed |
|------------|------------------|---------------------|
| Bug fix (< 50 lines) | 4 business hours | Author pings reviewer directly |
| Feature (50-300 lines) | 1 business day | Raise in standup |
| Large change (300+ lines) | 2 business days | Split the PR or schedule synchronous review |
| Security-sensitive | 4 business hours | Ping security champion + lead |

### Review depth by change type
| Change type | Expected depth | Focus areas |
|------------|---------------|-------------|
| Bug fix | Targeted -- verify the fix, check for regression risk | Root cause addressed (not just symptom), test coverage for the bug |
| New feature | Thorough -- full review of all dimensions | AC alignment, test coverage, error handling, security |
| Refactor | Structure-focused -- verify behavior preservation | Tests still pass, no behavior changes, improved readability |
| Dependency update | Risk-focused -- check for breaking changes | Changelog review, breaking API changes, security advisories |
| Config/infra change | Impact-focused -- blast radius assessment | What environments affected, rollback path, monitoring |

### Reviewer assignment
- **Round-robin** for general changes -- spread knowledge, avoid bottlenecks
- **Domain expert** for security, performance, or architecture changes
- **Two reviewers** for changes touching shared libraries, APIs, or data models
- **Pair review** (synchronous) for changes > 500 lines or cross-cutting concerns

### Review health metrics
Track quarterly:
- **Review cycle time:** Median time from PR opened to approved (target: < 1 business day)
- **Review throughness:** % of PRs with at least one substantive comment (not just "LGTM")
- **Knowledge distribution:** Number of unique reviewers per codebase area (avoid single-reviewer silos)
- **PR size distribution:** Track % of PRs > 300 lines (goal: shrink over time)

Ask the user:

• Does the team have existing review norms this should build on?
• Any pain points with current review practices (bottlenecks, rubber-stamping, nitpick wars)?
• Want me to draft a team-specific version based on your current workflow?

Output location

Present the review as formatted text in the conversation.