name: code-review
description: Reviews code changes for bugs, style issues, security vulnerabilities, and best practices. Use when reviewing PRs, checking code quality, or validating implementations.

Code Review Skill

Systematic approach to reviewing code changes for quality, correctness, and maintainability.

When to Use This Skill

• Reviewing pull requests or merge requests
• Checking code quality before committing
• Validating implementation against requirements
• Finding potential bugs or security issues
• Ensuring code follows project conventions

Review Checklist

1. Correctness

• [ ] Does the code do what it's supposed to do?
• [ ] Are edge cases handled properly?
• [ ] Are error conditions handled gracefully?
• [ ] Is the logic correct and complete?

2. Security

• [ ] No hardcoded credentials or secrets
• [ ] Input validation is present
• [ ] SQL injection prevention (parameterized queries)
• [ ] XSS prevention (proper escaping)
• [ ] Authentication/authorization checks

3. Code Quality

• [ ] Follows project coding standards
• [ ] Meaningful variable and function names
• [ ] No code duplication (DRY principle)
• [ ] Functions are focused and single-purpose
• [ ] Proper use of comments (why, not what)

4. Performance

• [ ] No obvious inefficiencies (N+1 queries, etc.)
• [ ] Appropriate data structures used
• [ ] Caching considered where appropriate
• [ ] No memory leaks or resource leaks

5. Maintainability

• [ ] Code is readable and self-documenting
• [ ] Complex logic is well-commented
• [ ] Tests are included for new functionality
• [ ] Documentation updated if needed

How to Provide Feedback

Be Specific

- ❌ "This code is wrong"
+ ✅ "The null check on line 42 should happen before accessing user.name"

Explain Why

- ❌ "Use const instead of let"
+ ✅ "Use const instead of let since this value is never reassigned"

Suggest Alternatives

- ❌ "This is inefficient"
+ ✅ "Consider using a Map instead of array.find() for O(1) lookup"

Use Appropriate Severity

Decision Tree

Is this a critical/security issue?
├── YES → 🔴 BLOCKER - Must fix
└── NO
    ├── Does it affect functionality?
    │   ├── YES → 🟠 SHOULD - Fix recommended
    │   └── NO
    │       ├── Improves code quality?
    │       │   ├── YES → 🟡 COULD - Nice improvement
    │       │   └── NO → 💡 NIT - Optional

Example Review Comments

Good Example

🔴 BLOCKER: SQL Injection vulnerability

The user input is directly interpolated into the SQL query on line 87:
`db.query(`SELECT * FROM users WHERE id = ${userId}`)`

Please use parameterized queries instead:
`db.query('SELECT * FROM users WHERE id = ?', [userId])`

Another Example

🟡 COULD: Consider extracting to helper function

Lines 45-60 duplicate the validation logic from AuthService.
Consider extracting to a shared helper:

`const isValidEmail = (email) => /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email);`

Best Practices

1. Review in context - Understand the PR's purpose before diving into code
2. Be constructive - Focus on the code, not the person
3. Acknowledge good work - Praise well-written code
4. Ask questions - If unsure, ask rather than assume
5. Keep it focused - Don't bring up unrelated issues
6. Be timely - Review promptly to unblock teammates