Code Review Mode

Perform a comprehensive code review with structured analysis across multiple dimensions.

When to Use This Skill

• When reviewing pull requests or merge requests
• Before merging feature branches into main/trunk
• When asked to review specific files or changes
• During pair programming review sessions
• For post-implementation quality checks

Workflow

Phase 1: Context Gathering

1. Identify the scope:

   • Determine which files/commits to review (diff, specific files, or branch comparison)
   • Get the diff: git diff for the relevant range

2. Understand the change:

   • Review commit messages and PR description (if available)
   • Check git history for related recent changes
   • Identify the purpose and expected behavior of the change

3. Map dependencies:

   • Identify what other code depends on the changed files
   • Check for related test files
   • Note any configuration or schema changes

Phase 2: Multi-Dimensional Review

Evaluate the code against each dimension in the Review Dimensions section below. For each dimension:

1. Walk through the relevant checklist
2. Note any issues with severity (Critical/High/Medium/Low)
3. Record the specific location (file:line)
4. Document suggested fixes

Phase 3: Synthesis

Produce a structured review following the template in references/code-review-template.md.

1. Determine verdict:

   • APPROVE - No critical/high issues, code is ready to merge
   • NEEDS WORK - Has issues that must be addressed before merge
   • BLOCK - Has critical issues, security vulnerabilities, or fundamental design problems

2. Organize findings by severity

3. Acknowledge strengths - what's done well

4. Provide actionable fixes - not just problems, but solutions

Review Dimensions

Correctness

• Logic is correct and handles all expected cases
• Edge cases are handled (null, empty, boundary values)
• Assumptions are documented or validated
• Error states are handled appropriately
• Types are correct and conversions are safe

Security

• No injection vulnerabilities (SQL, command, XSS)
• Input is validated and sanitized
• No secrets, credentials, or keys in code
• Authentication/authorization checks are correct
• Dependencies are secure and up-to-date

Performance

• No unnecessary loops or redundant operations
• Algorithms are appropriate for the data size
• Memory usage is reasonable
• No N+1 queries or unbounded fetches
• Caching is used appropriately

Reliability

• Errors are caught and handled gracefully
• Failures don't leave system in bad state
• Retry logic has backoff and limits
• Resources are properly cleaned up (connections, files, etc.)
• Timeouts are set for external calls

Maintainability

• Code is readable and self-documenting
• Functions/methods have single responsibility
• Naming is clear and consistent
• No unnecessary duplication
• Follows project conventions and patterns

Testing

• Tests exist for new/changed behavior
• Edge cases and error paths are tested
• Tests are deterministic (not flaky)
• Test names clearly describe what they verify
• Mocks/stubs are appropriate and not excessive

Escalation Criteria

Flag for additional human review when:

Concern	Escalation Trigger
Security	Any auth changes, crypto usage, user data handling, or potential vulnerabilities
Reliability	Changes to error handling, retry logic, or failure recovery paths
Performance	Changes to hot paths, database queries, or algorithms with scale concerns
Architecture	New patterns, significant structural changes, or cross-cutting concerns
Testing	Reduced coverage, disabled tests, or changes to test infrastructure

Constraints

• Review only what's in scope - don't expand to unrelated code unless it's directly affected
• Be specific - reference exact file:line locations for all findings
• Provide fixes, not just criticisms - every issue should have a suggested resolution
• Calibrate severity appropriately - not everything is critical
• Acknowledge good work - positive feedback matters too

Examples

Example: Reviewing a New API Endpoint

Scope: PR adds POST /api/users endpoint in internal/api/users.go

Phase 1 output:

Files changed:
- internal/api/users.go (new handler)
- internal/api/routes.go (route registration)
- internal/db/users.go (new repository method)

Related tests: internal/api/users_test.go (new)
Purpose: Add user creation endpoint for onboarding flow

Phase 2 findings:

Severity	Issue	Location	Fix
High	SQL injection via unsanitized email	db/users.go:45	Use parameterized query
Medium	Missing rate limiting	api/users.go:23	Add rate limiter middleware
Low	Error message exposes internal details	api/users.go:67	Return generic error to client

Phase 3 verdict:

Verdict: NEEDS WORK

Critical/High Issues:
- SQL injection vulnerability must be fixed before merge

What's Done Well:
- Clean separation between handler and repository
- Comprehensive input validation struct
- Good test coverage for happy path

Action Items:
1. [High] Fix SQL injection in db/users.go:45
2. [Medium] Add rate limiting to endpoint
3. [Low] Sanitize error messages returned to client

---

Begin by gathering context on the target files/commits before conducting the multi-dimensional review.