Code Reviewer Skill

This skill provides thorough code review capabilities across all programming languages and frameworks. Use this whenever you need to analyze code for quality, security, performance, or maintainability concerns.

Review Scope

When reviewing code, analyze across these dimensions:

1. Security Analysis

• Identify potential security vulnerabilities (SQL injection, XSS, CSRF, etc.)
• Check for hardcoded credentials, API keys, or sensitive data
• Verify proper input validation and sanitization
• Review authentication and authorization implementations
• Check for insecure cryptographic practices
• Identify potential data leakage points

2. Bug Detection

• Identify logical errors and edge cases
• Check for null/undefined reference errors
• Detect off-by-one errors and boundary conditions
• Identify race conditions and concurrency issues
• Check for memory leaks and resource management
• Verify error handling and exception management

3. Performance Issues

• Identify inefficient algorithms and data structures
• Detect N+1 query problems in database operations
• Check for unnecessary loops and redundant operations
• Identify memory-intensive operations
• Review caching strategies and opportunities
• Analyze computational complexity (Big O)

4. Code Quality & Best Practices

• Verify adherence to language-specific conventions
• Check code readability and maintainability
• Identify code duplication and suggest DRY improvements
• Review naming conventions (variables, functions, classes)
• Check function/method length and complexity
• Verify proper separation of concerns
• Assess test coverage and testability

5. Architecture & Design

• Review design patterns usage and appropriateness
• Check for SOLID principles violations
• Identify tight coupling and low cohesion
• Review dependency management
• Assess scalability considerations
• Check for proper abstraction layers

Review Process

Follow this structured approach when reviewing code:

Step 1: Initial Assessment

1. Understand the context and purpose of the code changes
2. Identify the programming language and framework
3. Note any stated requirements or constraints
4. Review related documentation if provided

Step 2: Systematic Analysis

Analyze the code in this order:

1. Security first - Check for vulnerabilities
2. Correctness - Verify logical correctness and bug-free implementation
3. Performance - Identify optimization opportunities
4. Quality - Review code structure and best practices
5. Architecture - Assess design decisions

Step 3: Categorized Findings

Organize findings by severity:

🔴 Critical Issues (Must fix immediately)

• Security vulnerabilities
• Data loss risks
• System crashes or breaking bugs
• Complete functional failures

🟠 Major Issues (Should fix before merge)

• Performance bottlenecks
• Logic errors in non-critical paths
• Poor error handling
• Significant maintainability concerns

🟡 Minor Issues (Nice to fix)

• Code style inconsistencies
• Minor optimizations
• Naming improvements
• Documentation gaps

💡 Suggestions (Optional improvements)

• Refactoring opportunities
• Alternative approaches
• Design pattern recommendations
• Future enhancement ideas

Step 4: Provide Solutions

For each issue:

1. Explain the problem clearly with specific line references
2. Describe the impact (security, performance, maintainability)
3. Provide concrete fix with code examples when possible
4. Explain why the fix is better

Output Format

Structure your review as follows:

# Code Review Summary

## Overview
[Brief summary of what was reviewed and general assessment]

## Findings Summary
- Critical: X issues
- Major: Y issues
- Minor: Z issues
- Suggestions: W items

---

## 🔴 Critical Issues

### Issue 1: [Title]
**Location:** `file.js:42-45`
**Problem:** [Description]
**Impact:** [Security/Performance/Functional impact]
**Fix:** 
```language
[Code example showing the fix]

Explanation: [Why this fix works]

🟠 Major Issues

[Same format as Critical]

🟡 Minor Issues

[Same format, can be more concise]

💡 Suggestions

[Same format, focus on improvements]

Positive Aspects

[Highlight what was done well]

Overall Assessment

[Summary and recommendation: Approve/Request Changes/Reject]

## Language-Specific Guidelines

### JavaScript/TypeScript
- Check for proper async/await usage vs promises
- Verify type safety (TypeScript)
- Look for common pitfalls: `==` vs `===`, variable hoisting, closure issues
- Check for proper React hooks usage and dependencies
- Verify proper event listener cleanup

### Python
- Check PEP 8 compliance
- Verify proper exception handling (avoid bare except)
- Look for mutable default arguments
- Check for proper context managers (`with` statements)
- Verify proper async/await usage

### Java
- Check for proper exception handling
- Verify resource cleanup (try-with-resources)
- Look for potential NullPointerExceptions
- Check thread safety in concurrent code
- Verify proper use of generics

### Go
- Check error handling (never ignore errors)
- Verify proper goroutine and channel usage
- Look for potential race conditions
- Check for proper defer usage
- Verify context propagation

### SQL
- Check for SQL injection vulnerabilities
- Verify proper indexing strategies
- Look for N+1 queries
- Check for inefficient JOINs
- Verify transaction handling

## Framework-Specific Checks

### React
- Check for unnecessary re-renders
- Verify proper key usage in lists
- Check useEffect dependencies
- Verify proper state management
- Look for memory leaks in effects

### Node.js/Express
- Check middleware ordering
- Verify error handling middleware
- Look for callback hell
- Check for proper async error handling
- Verify CORS configuration

### Django
- Check for ORM N+1 queries
- Verify CSRF protection
- Check for SQL injection in raw queries
- Verify proper form validation
- Check middleware configuration

### Spring Boot
- Check for proper dependency injection
- Verify transaction boundaries
- Look for N+1 queries with JPA
- Check exception handling
- Verify security configuration

## Common Anti-Patterns to Flag

1. **God Objects/Classes** - Single class doing too much
2. **Magic Numbers** - Unexplained numeric constants
3. **Deep Nesting** - Excessive indentation levels (>3-4)
4. **Long Methods** - Functions over 50-100 lines
5. **Premature Optimization** - Complex optimizations without profiling
6. **Copy-Paste Programming** - Duplicated code blocks
7. **Cargo Cult Programming** - Code without understanding
8. **Golden Hammer** - Using same solution for everything
9. **Spaghetti Code** - Unclear control flow
10. **Shotgun Surgery** - Changes requiring edits in many places

## Review Checklist

Before completing the review, verify:

- [ ] All security concerns addressed
- [ ] Critical bugs identified
- [ ] Performance issues noted
- [ ] Code follows project conventions
- [ ] Error handling is appropriate
- [ ] Tests are adequate (if applicable)
- [ ] Documentation is clear
- [ ] Dependencies are justified
- [ ] No hardcoded secrets
- [ ] Resource cleanup is proper

## Tone and Communication

- Be constructive and respectful
- Focus on the code, not the person
- Explain the "why" behind suggestions
- Acknowledge good practices
- Offer learning resources when helpful
- Distinguish between opinions and standards
- Be specific with examples
- Prioritize actionable feedback

## When to Escalate

Flag for human review if you find:
- Critical security vulnerabilities requiring immediate attention
- Fundamental architectural problems
- Regulatory compliance issues
- Licensing violations
- Extremely complex logic requiring domain expertise

## Examples

### Example 1: Security Issue

Issue: SQL Injection Vulnerability

Location: user_service.py:23 Problem: User input directly interpolated into SQL query Impact: 🔴 Critical - Allows arbitrary SQL execution, potential data breach

Current Code:

query = f"SELECT * FROM users WHERE username = '{username}'"
cursor.execute(query)

Fixed Code:

query = "SELECT * FROM users WHERE username = %s"
cursor.execute(query, (username,))

Explanation: Using parameterized queries prevents SQL injection by properly escaping user input. The database driver handles sanitization, eliminating the attack vector.

### Example 2: Performance Issue

Issue: N+1 Query Problem

Location: order_controller.js:15-20 Problem: Fetching related data in a loop instead of using eager loading Impact: 🟠 Major - Performance degrades linearly with number of orders

Current Code:

const orders = await Order.findAll();
for (const order of orders) {
  order.customer = await Customer.findByPk(order.customerId); // N+1!
}

Fixed Code:

const orders = await Order.findAll({
  include: [{ model: Customer }]
});

Explanation: Eager loading fetches all related customers in a single query using JOIN, reducing database round-trips from N+1 to 2 queries total.

## Additional Resources

When suggesting improvements, reference:
- Official language/framework documentation
- Security best practices (OWASP, CWE)
- Performance optimization guides
- Design pattern catalogs
- Community style guides

## Notes

- Adapt review depth to the scope of changes
- For small PRs, focus on critical issues
- For major refactors, provide comprehensive analysis
- Consider project-specific conventions if provided
- Balance thoroughness with pragmatism
- Remember that perfect code doesn't exist - focus on meaningful improvements