Code Review

Overview

Conduct focused, structured code reviews that emphasize understanding over criticism. Reviews prioritize questions about ambiguity and code/data flow rather than nitpicking style. Each review produces a consistent format with summary, assumptions, and priority-based comments.

When to Use This Skill

Trigger this skill when:

• Reviewing pull requests or code changes
• User explicitly requests "review this code" or "conduct a code review"
• Analyzing a diff or set of file changes
• Evaluating code before merge or deployment
• JIRA tickets reference code changes that need review

Review Format

All code reviews follow this structure:

1. What These Changes Do

3-5 concise bullet points summarizing the intent and scope of changes.

Example:

• Adds GraphQL endpoint for querying user preferences
• Refactors authentication middleware to support JWT tokens
• Introduces caching layer for frequently accessed data

2. Assumptions

Brief bullet points documenting assumptions made during the review.

Example:

• Assuming PostgreSQL is the target database (not explicitly stated)
• Type definitions suggest this is part of a larger refactor
• Error handling strategy follows existing patterns in the codebase

3. Priority Comments

Comments organized into three priority levels. Each comment is 50-150 words maximum.

High Priority

Issues that must be addressed before merge:

• Security vulnerabilities
• Data loss risks
• Breaking changes without migration path
• Critical logic errors
• Major performance regressions

Medium Priority

Issues that should be addressed soon:

• Unclear code/data flow requiring clarification
• Missing error handling
• Ambiguous function behavior
• Type safety concerns
• Potential edge cases

Low Priority

Suggestions for future improvement:

• Code organization opportunities
• Documentation enhancements
• Minor performance optimizations
• Consistency improvements

Review Process

Step 1: Identify Changes Scope

Read through all modified files to understand:

• What functionality is being added/modified/removed
• Which systems/services are affected
• Whether this is a feature, bug fix, refactor, or chore

Step 2: Check for JIRA Reference

Scan commit messages, PR descriptions, and file paths for JIRA ticket references:

• Pattern: FR-###, BUG-###, TD-###, CHORE-###
• If found, prepare to save review document in the JIRA folder: jira/{category}/{ticket-id}/code-review-{date}.md

JIRA Folder Structure:

jira/
├── feature-requests/
│   └── FR-XXX/
│       ├── code-review-2025-11-09.md
│       └── DESCRIPTION.md
├── bugs/
│   └── BUG-XXX/
├── tech-debt/
│   └── TD-XXX/
└── chores/
    └── CHORE-XXX/

Step 3: Analyze Code Flow

Focus on understanding:

• Data flow: How data moves through functions/components
• Control flow: Branching logic, error paths, edge cases
• Dependencies: What this code relies on and what relies on it
• Assumptions: What must be true for this code to work correctly

Step 4: Formulate Questions

Instead of prescriptive feedback, ask questions about:

• Ambiguous behavior: "What happens when userId is null here?"
• Flow clarity: "How does error propagate from this async call?"
• Missing context: "Is there a reason we're not validating input X?"
• Edge cases: "Have we considered the case where the array is empty?"

Step 5: Prioritize Feedback

Categorize each comment:

• High: Must fix before merge (security, correctness, data integrity)
• Medium: Should address (clarity, robustness, maintainability)
• Low: Nice to have (style, organization, minor optimizations)

Step 6: Write Review

Use the review template from assets/review-template.md and populate all sections:

1. What These Changes Do (3-5 bullets)
2. Assumptions (as many bullets as needed, keep short)
3. High Priority (only critical issues)
4. Medium Priority (questions and concerns)
5. Low Priority (suggestions for future)

Step 7: Save Review Document

If JIRA ticket found:

• Save to jira/{category}/{ticket-id}/code-review-{YYYY-MM-DD}.md
• Use today's date for the filename

If no JIRA ticket:

• Present review inline in the conversation

Review Principles

DO Focus On

✅ Understanding code intent and flow
✅ Asking questions about ambiguity
✅ Identifying security and correctness issues
✅ Flagging unclear data flow
✅ Noting missing error handling
✅ Highlighting assumptions that need validation

DON'T Focus On

❌ Nitpicking style preferences
❌ Subjective code organization debates
❌ Minor naming suggestions
❌ Formatting (linters handle this)
❌ Personal preference for syntax
❌ Extensive refactoring suggestions (unless critical)

Question Framing

Always frame feedback as questions or observations, not commands:

Good:

• "What happens if the API returns null here?"
• "Should we validate userId before passing to the database?"
• "Is there a reason we're not handling the rejected promise?"

Avoid:

• "You need to add validation here."
• "This should use async/await instead."
• "Change this to use map() instead of forEach()."

Word Count Guidelines

Keep reviews concise:

• What These Changes Do: 1-2 sentences per bullet
• Assumptions: 1 sentence per bullet
• Comments: 50-150 words each (strictly enforced)

Short, focused reviews are more actionable than lengthy critiques.

Resources

assets/review-template.md

Pre-formatted template for consistent review structure. Copy and populate all sections.

references/review-guidelines.md

Expanded guidance on identifying different priority levels and framing effective review questions.

Example Review

What These Changes Do:

• Adds real-time WebSocket connection management for live screen updates
• Introduces connection pooling to handle up to 1000 concurrent sessions
• Implements automatic reconnection logic with exponential backoff

Assumptions:

• Redis is available and configured for session storage
• Frontend implements WebSocket protocol correctly
• Network latency is acceptable for real-time requirements

High Priority:

• Memory leak risk: WebSocket connections appear to be held indefinitely without cleanup when clients disconnect unexpectedly. Should we implement a timeout or heartbeat mechanism?

Medium Priority:

• Error handling: What happens when Redis connection fails mid-session? The code doesn't appear to handle this case, which could leave clients in an inconsistent state.
• Type safety: The message parameter in handleIncomingMessage() is typed as any. Can we define a discriminated union for the expected message types?

Low Priority:

• Consider extracting the reconnection logic into a separate utility function for reusability across other real-time features.