Code Review

Code Review

This skill provides expert code review capabilities focusing on code quality, security vulnerabilities, and maintainability. It analyzes code changes and provides prioritized, actionable feedback.

When to Use This Skill

• After writing or modifying code to ensure quality standards
• Before merging pull requests or deploying changes
• When conducting security audits or vulnerability assessments
• When establishing code quality standards for a project
• When reviewing code for performance optimizations
• When ensuring code follows project conventions and best practices

What This Skill Does

1. Analyzes Code Changes: Reviews git diffs and modified files to understand what changed 2. Security Auditing: Identifies exposed secrets, API keys, and security vulnerabilities 3. Quality Assessment: Evaluates code readability, maintainability, and best practices 4. Performance Review: Identifies potential performance issues and optimization opportunities 5. Standards Compliance: Ensures code follows project conventions and style guidelines 6. Prioritized Feedback: Organizes findings by severity (Critical, Warnings, Suggestions)

How to Use

Basic Code Review

Review the recent code changes in this branch

Check this file for security issues and code quality

Review Specific Files

Review src/auth.js for security vulnerabilities

Review the changes in this pull request

Review Process

1. Analyze Changes

When invoked:

1. Get Recent Changes: Run git diff to see what changed

   git diff $(git merge-base HEAD main)..HEAD

2. Focus on Modified Files: Prioritize files with actual changes 3. Review Context: Understand the purpose of changes from commit messages or conversation

2. Review Checklist

For each file, check:

Code Quality:

• Code is simple and readable
• Functions and variables are well-named
• No duplicated code
• Proper error handling
• Good test coverage
• Performance considerations addressed

Security:

• No exposed secrets or API keys
• Input validation implemented
• Proper authentication/authorization
• Safe handling of user input
• No SQL injection or XSS vulnerabilities

Maintainability:

• Consistent code style
• Appropriate comments (not excessive)
• Clear function/method structure
• Proper separation of concerns
• No unnecessary complexity

Best Practices:

• Follows framework/library conventions
• Proper use of design patterns
• Efficient algorithms and data structures
• Appropriate use of async/await or promises
• Proper resource cleanup

3. Organize Feedback

Provide feedback in three priority levels:

Critical Issues (Must Fix):

• Security vulnerabilities
• Bugs that will cause failures
• Code that breaks existing functionality
• Performance issues that will impact users

Warnings (Should Fix):

• Code quality issues that affect maintainability
• Potential bugs or edge cases
• Performance optimizations
• Missing error handling
• Incomplete test coverage

Suggestions (Consider Improving):

• Code style improvements
• Refactoring opportunities
• Documentation enhancements
• Best practice recommendations
• Minor optimizations

4. Provide Specific Examples

For each issue found:

• Location: File path and line number
• Issue: Clear description of the problem
• Impact: Why this matters
• Fix: Specific code example showing how to fix it
• Rationale: Explanation of why the fix is better

Examples

Example 1: Security Issue Found

Input: Review src/api/auth.js

Output:

## Code Review: src/api/auth.js

### Critical Issues

**1. Exposed API Key**
- **Location**: `src/api/auth.js:23`
- **Issue**: Hardcoded API key in source code
- **Code**:

const API_KEY = "sk_live_1234567890abcdef"; // ❌ Exposed secret

- **Fix**: Use environment variables

const API_KEY = process.env.API_KEY; // ✅ Secure

- **Impact**: This exposes your API key in version control, allowing unauthorized access

### Warnings

**2. Missing Input Validation**

- **Location**: `src/api/auth.js:45`
- **Issue**: No validation on user input before processing
- **Recommendation**: Add input validation before authentication

### Example 2: Code Quality Review

**Input**: Review recent changes

**Output**:

Code Review Summary

Files Reviewed: 5 Critical Issues: 0 Warnings: 3 Suggestions: 5

Warnings

1. Duplicated Code

• Files: src/utils/format.js:12-25 and src/utils/display.js:8-21
• Issue: Same formatting logic duplicated
• Fix: Extract to shared utility function

2. Missing Error Handling

• Location: src/api/users.js:34
• Issue: No try/catch for async operation
• Fix: Add proper error handling

Suggestions

1. Improve Variable Naming

• Location: src/components/List.jsx:15
• Current: const d = data.map(...)
• Suggested: const formattedItems = data.map(...)

## Reference Files

For comprehensive review checklists, load reference files as needed:

- **`references/review_checklist.md`** - Detailed checklists for security, code quality, performance, testing, documentation, and best practices
- **`references/CODE_ANALYSIS.template.md`** - Code analysis report template with security, performance, and maintainability sections

When conducting thorough reviews, load `references/review_checklist.md` and use the appropriate checklist sections.

## Best Practices

### Review Focus Areas

1. **Security First**: Always check for security vulnerabilities first
2. **Context Matters**: Understand the purpose of changes before reviewing
3. **Be Constructive**: Provide actionable feedback, not just criticism
4. **Prioritize**: Focus on critical issues that must be fixed
5. **Explain Why**: Help developers understand the reasoning behind suggestions

### Review Guidelines

- **Be Specific**: Point to exact lines and provide code examples
- **Be Balanced**: Acknowledge good code as well as issues
- **Be Practical**: Consider the context and urgency of changes
- **Be Educational**: Help developers learn and improve
- **Be Consistent**: Apply the same standards across all reviews

### Common Patterns to Check

**Security:**

- Hardcoded secrets or credentials
- SQL injection vulnerabilities
- XSS vulnerabilities
- Missing authentication/authorization
- Insecure random number generation

**Code Quality:**

- Code duplication
- Magic numbers without constants
- Deeply nested conditionals
- Functions that do too much
- Poor error messages

**Performance:**

- N+1 query problems
- Missing indexes
- Inefficient algorithms
- Unnecessary re-renders (React)
- Memory leaks

## Related Use Cases

- Pre-commit code reviews
- Pull request reviews
- Security audits
- Code quality assessments
- Onboarding new team members
- Establishing coding standards

Code Analysis Report Template

Generated: {{TIMESTAMP}} Target: {{FILE_OR_DIRECTORY}} Analyzer: {{TOOL_OR_METHOD}}

---

Executive Summary

---

Findings Summary

---

Critical Issues

{{ISSUE_ID}}: {{ISSUE_TITLE}}

File: {{FILE_PATH}}:{{LINE_NUMBER}} Category: {{SECURITY/PERFORMANCE/BUG/STYLE}}

```{{LANGUAGE}} {{CODE_SNIPPET}}

**Problem:** {{DESCRIPTION}}

**Impact:** {{IMPACT_DESCRIPTION}}

**Recommendation:**

{{FIXED_CODE}}

---

## Architecture Analysis

### Dependencies

{{DEPENDENCY_TREE_OR_DIAGRAM}}

### Complexity Metrics

| File | Cyclomatic | Cognitive | Lines | Status |
|------|------------|-----------|-------|--------|
| {{FILE}} | {{N}} | {{N}} | {{N}} | {{STATUS}} |

### Code Smells

| Smell | Occurrences | Files Affected |
|-------|-------------|----------------|
| Long Method | {{N}} | {{FILES}} |
| Large Class | {{N}} | {{FILES}} |
| Duplicate Code | {{N}} | {{FILES}} |
| Dead Code | {{N}} | {{FILES}} |

---

## Security Analysis

### Vulnerabilities

| ID | Severity | Type | Location |
|----|----------|------|----------|
| {{CVE/CWE}} | {{SEV}} | {{TYPE}} | {{FILE:LINE}} |

### Dependency Audit

| Package | Current | Latest | Vulnerabilities |
|---------|---------|--------|-----------------|
| {{PKG}} | {{VER}} | {{VER}} | {{COUNT}} |

### Secrets Detection

| Type | File | Status |
|------|------|--------|
| API Key | {{FILE}} | {{EXPOSED/SAFE}} |

---

## Performance Analysis

### Hot Spots

| File | Function | Time % | Calls |
|------|----------|--------|-------|
| {{FILE}} | {{FUNC}} | {{N}}% | {{N}} |

### Memory Usage

| Component | Allocated | Retained | Status |
|-----------|-----------|----------|--------|
| {{COMP}} | {{SIZE}} | {{SIZE}} | {{STATUS}} |

### Database Queries

| Query | Avg Time | N+1 Risk | Index Used |
|-------|----------|----------|------------|
| {{QUERY}} | {{MS}}ms | {{Y/N}} | {{Y/N}} |

---

## Test Coverage

### Coverage by Module

| Module | Lines | Branches | Functions |
|--------|-------|----------|-----------|
| {{MOD}} | {{N}}% | {{N}}% | {{N}}% |

### Untested Code

| File | Lines Missing | Critical |
|------|---------------|----------|
| {{FILE}} | {{LINES}} | {{Y/N}} |

---

## Recommendations

### Immediate Actions (Critical/High)

1. **{{ACTION_1}}**
   - File: `{{FILE}}`
   - Effort: {{LOW/MED/HIGH}}
   - Impact: {{DESCRIPTION}}

### Short-term Improvements

1. {{IMPROVEMENT_1}}
2. {{IMPROVEMENT_2}}

### Technical Debt

| Item | Priority | Effort | Value |
|------|----------|--------|-------|
| {{ITEM}} | {{P1-4}} | {{EST}} | {{HIGH/MED/LOW}} |

---

## Appendix

### A. Full Issue List

<details>
<summary>Click to expand ({{N}} issues)</summary>

{{FULL_ISSUE_LIST}}

</details>

### B. Tool Configuration

{{TOOL_CONFIG}}

### C. Analysis Commands

{{COMMANDS_USED}}

---

## Quality Checklist

- [ ] All critical issues have recommendations
- [ ] Security vulnerabilities documented
- [ ] Performance bottlenecks identified
- [ ] Test coverage gaps highlighted
- [ ] Technical debt quantified
- [ ] Recommendations prioritized by impact

Code Review Checklist

Comprehensive checklist for conducting thorough code reviews across different aspects of code quality.

Security Checklist

Authentication & Authorization

• [ ] Proper authentication implemented
• [ ] Authorization checks on all protected routes
• [ ] Role-based access control (RBAC) implemented correctly
• [ ] Session management secure
• [ ] Token expiration handled
• [ ] Password hashing used (bcrypt, argon2, etc.)
• [ ] No hardcoded credentials

Input Validation

• [ ] All user inputs validated
• [ ] SQL injection prevention (parameterized queries)
• [ ] XSS prevention (output encoding)
• [ ] CSRF protection implemented
• [ ] File upload validation (type, size, content)
• [ ] Path traversal prevention
• [ ] Input sanitization applied

Data Protection

• [ ] Sensitive data encrypted at rest
• [ ] Sensitive data encrypted in transit (HTTPS)
• [ ] No secrets in code or config files
• [ ] Environment variables used for secrets
• [ ] API keys properly secured
• [ ] Database credentials secure
• [ ] Logs don't contain sensitive information

Security Headers

• [ ] Security headers configured (CSP, HSTS, etc.)
• [ ] CORS properly configured
• [ ] Content Security Policy set
• [ ] X-Frame-Options set
• [ ] X-Content-Type-Options set

Code Quality Checklist

Readability

• [ ] Code is clear and self-documenting
• [ ] Variable names are descriptive
• [ ] Function names describe what they do
• [ ] Comments explain why, not what
• [ ] Code follows project style guide
• [ ] Consistent formatting

Structure

• [ ] Functions do one thing
• [ ] Functions are appropriately sized (< 50 lines ideal)
• [ ] No code duplication (DRY principle)
• [ ] Proper separation of concerns
• [ ] Logical code organization
• [ ] Appropriate use of design patterns

Error Handling

• [ ] Errors handled appropriately
• [ ] Error messages are helpful
• [ ] No silent failures
• [ ] Proper exception types used
• [ ] Error logging implemented
• [ ] Graceful degradation

Performance Checklist

Algorithm Efficiency

• [ ] Appropriate data structures used
• [ ] Algorithms are efficient (O(n) vs O(n²))
• [ ] No unnecessary loops
• [ ] Early returns used when possible
• [ ] Lazy loading implemented where appropriate

Database

• [ ] Queries are optimized
• [ ] Appropriate indexes exist
• [ ] No N+1 query problems
• [ ] Connection pooling used
• [ ] Query result caching implemented
• [ ] Batch operations used when possible

Resource Management

• [ ] Memory leaks prevented
• [ ] Resources properly closed/released
• [ ] Connection pooling implemented
• [ ] File handles closed
• [ ] Event listeners cleaned up
• [ ] Timers/intervals cleared

Testing Checklist

Test Coverage

• [ ] Unit tests for new code
• [ ] Integration tests for critical paths
• [ ] Edge cases tested
• [ ] Error cases tested
• [ ] Test coverage meets threshold

Test Quality

• [ ] Tests are independent
• [ ] Tests are deterministic
• [ ] Tests have clear names
• [ ] Tests follow AAA pattern (Arrange, Act, Assert)
• [ ] Mocks used appropriately
• [ ] Test data factories used

Documentation Checklist

Code Documentation

• [ ] Public APIs documented
• [ ] Complex logic explained
• [ ] Function parameters documented
• [ ] Return values documented
• [ ] Examples provided for complex functions

Project Documentation

• [ ] README updated if needed
• [ ] API documentation updated
• [ ] Architecture docs updated
• [ ] Changelog updated
• [ ] Migration guides if breaking changes

Best Practices Checklist

Language-Specific

• [ ] Follows language best practices
• [ ] Uses modern language features appropriately
• [ ] Avoids deprecated patterns
• [ ] Type safety (TypeScript, etc.)

Framework-Specific

• [ ] Follows framework conventions
• [ ] Uses framework features correctly
• [ ] No anti-patterns
• [ ] Performance best practices followed

General

• [ ] No magic numbers (use constants)
• [ ] No hardcoded values
• [ ] Configuration externalized
• [ ] Logging implemented appropriately
• [ ] Monitoring/metrics added