TomazWang Reviewing Code Skill
Use this skill when manually reviewing code or providing code feedback to ensure systematic, thorough, and constructive reviews.
When to Use
- User asks you to review code
- Providing feedback on implementations
- Evaluating pull requests
- Code quality assessments
Review Process
1. Understand Context
Before reviewing, understand:
- What is the goal? What problem does this code solve?
- What changed? What files/functions were modified?
- Why this approach? Are there design decisions to understand?
- What's the scope? Is this a fix, feature, refactor?
2. Review Layers
Review in this order (inspired by superpowers:systematic-debugging approach):
Layer 1: Correctness
- Does the code do what it's supposed to?
- Are edge cases handled?
- Is error handling appropriate?
- Are there obvious bugs?
Layer 2: Security
- Are there security vulnerabilities?
- Is input validated?
- Is authentication/authorization correct?
- Is sensitive data protected?
Layer 3: Performance
- Are there inefficiencies?
- Is caching used where appropriate?
- Are database queries optimized?
- Is memory usage reasonable?
Layer 4: Testing
- Are there tests?
- Do tests cover edge cases?
- Are tests clear and maintainable?
- Is test coverage adequate?
Layer 5: Maintainability
- Is the code readable?
- Are names clear?
- Is the design extensible?
- Is there documentation?
Layer 6: Style
- Does it follow conventions?
- Is formatting consistent?
- Are comments helpful (not redundant)?
3. Provide Feedback
Be constructive:
- Start with positives
- Explain the "why" behind suggestions
- Provide examples of better approaches
- Distinguish must-fix from nice-to-have
Be specific:
❌ "This function is too complex"
✓ "This function has cyclomatic complexity of 15. Consider extracting
lines 45-67 into a separate validateInput() function."
❌ "Fix the security issue"
✓ "The SQL query on line 45 uses string interpolation which allows
SQL injection. Use parameterized queries:
db.query('SELECT * FROM users WHERE email = ?', [email])"
Be empathetic:
- Assume good intent
- Ask questions instead of making demands
- Acknowledge tradeoffs
- Suggest, don't command
4. Categorize Findings
Critical (Must Fix):
- Security vulnerabilities
- Data corruption risks
- Production-breaking bugs
- Legal/compliance violations
High Priority (Should Fix):
- Logic errors
- Poor error handling
- Missing tests for core functionality
- Significant performance issues
Medium Priority (Consider Fixing):
- Code duplication
- Unclear naming
- Missing edge case handling
- Minor performance optimizations
Low Priority (Nice to Have):
- Style inconsistencies
- Better variable names
- Additional comments
- Refactoring opportunities
5. Summarize
Provide executive summary:
## Review Summary
**Overall:** Looks good with some important fixes needed
**Critical Issues:** 1
- SQL injection vulnerability in login endpoint
**High Priority:** 3
- Missing error handling in payment processing
- No tests for authentication flow
- N+1 query in user dashboard
**Medium Priority:** 5
[List briefly...]
**Low Priority:** 8
[List briefly...]
**Strengths:**
- Clear code organization
- Good use of TypeScript types
- Comprehensive error messages
**Recommendations:**
1. Fix SQL injection (critical)
2. Add tests for authentication
3. Optimize dashboard queries
Feedback Templates
Bug Report
**Issue:** [Brief description]
**Location:** file.ts:line
**Severity:** [Critical/High/Medium/Low]
**Problem:**
[Explain what's wrong and why it's a problem]
**Evidence:**
```code
[Show the problematic code]
Impact: [What could go wrong]
Suggested Fix:
[Show corrected code]
### Improvement Suggestion
Suggestion: [Brief description] Location: file.ts:line Priority: [High/Medium/Low]
Current Approach: [Explain current implementation]
Suggested Improvement: [Explain better approach and why]
Example:
[Show improved code]
Benefits:
- [List advantages]
### Positive Feedback
Nice work on: [Specific aspect]
Why it's good: [Explain what makes it good]
Example:
[Show the good code]
## Review Principles
Based on industry best practices and the systematic approaches from Anthropic's code-review plugin and Superpowers:
1. **Systematic, not ad-hoc** - Follow the layer-by-layer approach
2. **Evidence-based** - Point to specific code, not vague feelings
3. **Constructive** - Help improve, don't just criticize
4. **Prioritized** - Distinguish critical from nice-to-have
5. **Actionable** - Provide clear next steps
6. **Balanced** - Acknowledge good along with issues
7. **Educational** - Explain the "why" to help learning
## Red Flags to Watch For
**Security:**
- String concatenation in SQL/commands
- Unvalidated user input
- Hardcoded secrets
- Missing authentication/authorization
**Correctness:**
- Off-by-one errors
- Null pointer dereferences
- Race conditions
- Missing error handling
**Performance:**
- N+1 queries
- Unbounded loops
- Memory leaks
- Unnecessary computations
**Maintainability:**
- God objects (>500 lines)
- Deep nesting (>4 levels)
- Magic numbers
- Unclear naming
## Example Review
Code to review:
async function getUsers(req, res) {
const users = await db.query("SELECT * FROM users WHERE role = '" + req.query.role + "'");
res.send(users);
}
Review using this skill:
Layer 1: Correctness ✓ Basic functionality works
Layer 2: Security ❌ SQL injection vulnerability (CRITICAL) ❌ No input validation (HIGH) ❌ Missing authentication check (HIGH)
Layer 3: Performance ⚠️ SELECT * may fetch unnecessary columns (MEDIUM)
Layer 4: Testing ❌ No tests visible (HIGH)
Layer 5: Maintainability ⚠️ Missing error handling (MEDIUM) ⚠️ No TypeScript types (MEDIUM)
Layer 6: Style ⚠️ Function name could be more specific (LOW)
Feedback:
Critical Issues
1. SQL Injection Vulnerability Location: Line 2 Severity: Critical
The SQL query concatenates user input directly into the query string, allowing SQL injection attacks.
Current (Vulnerable):
const users = await db.query("SELECT * FROM users WHERE role = '" + req.query.role + "'");
Suggested Fix:
const users = await db.query(
"SELECT * FROM users WHERE role = ?",
[req.query.role]
);
Impact: Attacker could access/modify all database data.
High Priority Issues
2. Missing Authentication This endpoint should verify the user is authenticated before returning user data.
3. No Input Validation
Validate that role is one of the expected values.
4. Missing Tests Add tests covering normal case, SQL injection attempts, unauthorized access.
[Continue with medium/low priority...]
Summary: Fix the SQL injection immediately. Add auth check and input validation before deploying. Consider adding tests to prevent regression.
This systematic approach ensures thorough, helpful code reviews.