Systematically review pull requests, feature implementations, and code changes to ensure quality, maintainability, security, and adherence to best practices. Use when reviewing code before merging, conducting peer reviews, performing self-reviews, auditing code quality, checking for security vulnerabilities, ensuring consistent coding standards, verifying test coverage, assessing performance implications, evaluating architectural decisions, or providing constructive feedback to improve team code quality.
Systematically review pull requests, feature implementations, and code changes to ensure quality, maintainability, security, and adherence to best practices. Use when reviewing code before merging, conducting peer reviews, performing self-reviews, auditing code quality, checking for security vulnerabilities, ensuring consistent coding standards, verifying test coverage, assessing performance implications, evaluating architectural decisions, or providing constructive feedback to improve team code quality.
Code Review - Systematic Code Quality Analysis
When to use this skill
Reviewing pull requests before merging to main branch
Conducting peer code reviews for team members
Performing self-reviews before submitting code for review
Auditing code quality and standards compliance
Checking for security vulnerabilities and bad practices
Verifying adequate test coverage exists
Assessing performance implications of changes
Evaluating architectural and design decisions
Ensuring consistency with project coding standards
Providing constructive feedback to improve code quality
Reviewing critical business logic or sensitive operations
Checking for common anti-patterns and code smells
When to use this skill
Reviewing pull requests, feature implementations, or code changes before merging to ensure quality, maintainability, and adherence to best practices.
When working on related tasks or features
During development that requires this expertise
Use when: Reviewing pull requests, feature implementations, or code changes before merging to ensure quality, maintainability, and adherence to best practices.
Core Principles
Review for Understanding First - Before suggesting changes, ensure you understand the intent and context
- Point to exact lines with concrete suggestions
Be Specific and Actionable
Balance Positives with Improvements - Acknowledge good patterns while suggesting enhancements
Focus on Impact - Prioritize critical issues (security, correctness) over style preferences
Educate, Don't Just Correct - Explain why a change matters
Review Checklist
1. Correctness & Logic
✓ Does the code do what it claims to do?
✓ Are edge cases handled (null, empty, boundary values)?
✓ Are there potential race conditions or timing issues?
✓ Is error handling appropriate and complete?
✓ Are assumptions validated?
2. Security
✓ Input validation on all user-provided data?
✓ SQL injection, XSS, CSRF protections?
✓ Secrets/credentials properly secured (env vars, not hardcoded)?
✓ Authentication and authorization checks?
✓ Rate limiting on public endpoints?
3. Performance
✓ N+1 query problems?
✓ Unnecessary database calls or API requests?
✓ Memory leaks (event listeners, subscriptions)?
✓ Proper pagination for large datasets?
✓ Efficient algorithms (avoid O(n²) when O(n log n) possible)?
4. Maintainability
✓ Clear, descriptive names for variables/functions?
✓ Functions do one thing well (Single Responsibility)?
✓ DRY - no copy-paste duplication?
✓ Magic numbers replaced with named constants?
✓ Complex logic explained with comments?
5. Testing
✓ Tests cover happy path and error cases?
✓ Tests are deterministic (no flaky tests)?
✓ Edge cases tested?
✓ Integration points mocked/stubbed appropriately?
✓ Test names describe what they verify?
6. Code Style & Standards
✓ Consistent with project conventions?
✓ Follows language idioms?
✓ No unused imports or dead code?
✓ Proper error types thrown/returned?
✓ TypeScript types specific (not 'any')?
Review Process
Step 1: High-Level Review
1. Read PR description and linked issues
2. Understand the "why" behind changes
3. Scan file list - does scope match description?
4. Check for missing files (tests, migrations, docs)
Step 2: Deep Code Review
1. Review critical paths first (security, data integrity)
2. Check test coverage and quality
3. Look for architectural issues
4. Review error handling
5. Check for performance concerns
Step 3: Provide Feedback
Format: [SEVERITY] Issue - Specific suggestion
Example:
[CRITICAL] SQL Injection vulnerability on line 45
- Use parameterized queries instead of string concatenation
- Change: `query = f"SELECT * FROM users WHERE id = {user_id}"`
- To: `query = "SELECT * FROM users WHERE id = ?"` with params
[SUGGESTION] Consider extracting this 50-line function into smaller pieces
- Lines 100-150 could be broken into:
- `validateInput()` (lines 100-120)
- `processData()` (lines 121-140)
- `formatOutput()` (lines 141-150)
Feedback Severity Levels
[CRITICAL] - Security issue, data loss risk, broken functionality
[PRAISE] Excellent input validation (lines 12-20)
The zod schema here is comprehensive and includes all necessary checks. This prevents malformed data from reaching the database.
Common Anti-Patterns to Flag
1. Silent Failures
// Bad - errors disappeartry {
awaitcriticalOperation();
} catch (e) {
console.log('oops'); // ❌
}
// Good - proper error handlingtry {
awaitcriticalOperation();
} catch (e) {
logger.error('Critical operation failed', { error: e, context: {...} });
thrownewCriticalOperationError('Failed to process', { cause: e });
}
2. Callback Hell / Pyramid of Doom
// BadgetData((data) => {
processData(data, (result) => {
saveResult(result, (saved) => {
// 3+ levels deep ❌
});
});
});
// Good - use async/awaitconst data = awaitgetData();
const result = awaitprocessData(data);
const saved = awaitsaveResult(result);
3. God Functions
// Bad - function doing too muchfunctionhandleUserRequest(req) {
// 200 lines of validation, processing, formatting, saving ❌
}
// Good - split responsibilitiesfunctionhandleUserRequest(req) {
const validated = validateRequest(req);
const processed = processUserData(validated);
const formatted = formatResponse(processed);
returnsaveAndRespond(formatted);
}
When to Block vs Approve with Comments
Block merge (Request Changes):
Security vulnerabilities
Data loss risks
Broken functionality
Missing critical tests
Major performance issues
Approve with comments:
Style improvements
Refactoring suggestions
Minor performance optimizations
Documentation enhancements
Nice-to-have tests
Automated Checks
Before manual review, ensure automated checks pass:
Remember: The goal is to improve code quality while maintaining team morale. Be thorough but respectful, specific but not pedantic, and always explain the "why" behind suggestions.
