code-reviewer
Reviews pull requests and code changes for quality, security, and best practices. Use when user asks for code review, PR review, or mentions reviewing changes.
Install
npx skills add https://github.com/zhaono1/agent-playbook --skill code-reviewerSKILL.md
Code Reviewer
A comprehensive code review skill that analyzes pull requests and code changes for quality, security, maintainability, and best practices.
When This Skill Activates
This skill activates when you:
- Ask for a code review
- Request a PR review
- Mention reviewing changes
- Say "review this" or "check this code"
Review Process
Phase 1: Context Gathering
Get changed files
git diff main...HEAD --name-only git log main...HEAD --onelineGet the diff
git diff main...HEADUnderstand project context
- Read relevant documentation
- Check existing patterns in similar files
- Identify project-specific conventions
Phase 2: Analysis Categories
1. Correctness
- Logic is sound and matches requirements
- Edge cases are handled
- Error handling is appropriate
- No obvious bugs or typos
2. Security
- No hardcoded secrets or credentials
- Input validation and sanitization
- SQL injection prevention
- XSS prevention (for frontend)
- Authentication/authorization checks
- Safe handling of user data
3. Performance
- No N+1 queries
- Appropriate caching
- Efficient algorithms
- No unnecessary computations
- Memory efficiency
4. Code Quality
- Follows DRY principle
- Follows KISS principle
- Appropriate abstractions
- Clear naming conventions
- Proper typing (if TypeScript)
- No commented-out code
5. Testing
- Tests cover new functionality
- Tests cover edge cases
- Test assertions are meaningful
- No brittle tests
6. Documentation
- Complex logic is explained
- Public APIs have documentation
- JSDoc/TSDoc for functions
- README updated if needed
7. Maintainability
- Code is readable
- Consistent style
- Modular design
- Separation of concerns
Phase 3: Output Format
Use this structured format for review feedback:
# Code Review
## Summary
Brief overview of the changes (2-3 sentences).
## Issues by Severity
### Critical
Must fix before merge.
- [ ] **Issue Title**: Description with file:line reference
### High
Should fix before merge unless there's a good reason.
- [ ] **Issue Title**: Description with file:line reference
### Medium
Consider fixing, can be done in follow-up.
- [ ] **Issue Title**: Description with file:line reference
### Low
Nice to have improvements.
- [ ] **Issue Title**: Description with file:line reference
## Positive Highlights
What was done well in this PR.
## Suggestions
Optional improvements that don't require immediate action.
## Approval Status
- [ ] Approved
- [ ] Approved with suggestions
- [ ] Request changes
Common Issues to Check
Security Issues
| Issue | Pattern | Recommendation |
|---|---|---|
| Hardcoded secrets | const API_KEY = "sk-" |
Use environment variables |
| SQL injection | \"SELECT * FROM...\" + user_input |
Use parameterized queries |
| XSS vulnerability | innerHTML = user_input |
Sanitize or use textContent |
| Missing auth check | New endpoint without @RequireAuth |
Add authentication middleware |
Performance Issues
| Issue | Pattern | Recommendation |
|---|---|---|
| N+1 query | Loop with database call | Use eager loading or batch queries |
| Unnecessary re-render | Missing dependencies in useEffect |
Fix dependency array |
| Memory leak | Event listener not removed | Add cleanup in useEffect return |
| Inefficient loop | Nested loops O(n²) | Consider hash map or different algorithm |
Code Quality Issues
| Issue | Pattern | Recommendation |
|---|---|---|
| Duplicate code | Similar blocks repeated | Extract to function |
| Magic number | if (status === 5) |
Use named constant |
| Long function | Function >50 lines | Split into smaller functions |
| Complex condition | `a && b |
Testing Issues
| Issue | Pattern | Recommendation |
|---|---|---|
| No tests | New feature without test file | Add unit tests |
| Untested edge case | Test only covers happy path | Add edge case tests |
| Brittle test | Test relies on implementation details | Test behavior, not implementation |
| Missing assertion | Test doesn't assert anything | Add proper assertions |
Language-Specific Guidelines
TypeScript
- Use
unknowninstead ofanyfor untyped values - Prefer
interfacefor public APIs,typefor unions - Use strict mode settings
- Avoid
asassertions when possible
React
- Follow Hooks rules
- Use
useCallback/useMemoappropriately (not prematurely) - Prefer function components
- Use proper key props in lists
- Avoid prop drilling with Context
Python
- Follow PEP 8 style guide
- Use type hints
- Use f-strings for formatting
- Prefer list comprehensions over map/filter
- Use context managers for resources
Go
- Handle errors explicitly
- Use named returns for clarity
- Keep goroutines simple
- Use channels for communication
- Avoid package-level state
Before Approving
Confirm the following:
- All critical issues are addressed
- Tests pass locally
- No merge conflicts
- Commit messages are clear
- Documentation is updated
- Breaking changes are documented
Scripts
Run the review checklist script:
python3 scripts/review_checklist.py --base main --output review-checklist.md
References
references/checklist.md- Complete review checklistreferences/security.md- Security review guidelinesreferences/patterns.md- Common patterns and anti-patterns
Related skills
improve-codebase-architecturemattpocock1MScan a codebase for deepening opportunities, present them as a visual HTML report, then grill through whichever one you pick.codebase-designmattpocock691KShared vocabulary for designing deep modules. Use when the user wants to design or improve a module's interface, find deepening opportunities, decide where a seam goes, make code more testable or AI-navigable, or when another skill needs the deep-module vocabulary.web-design-guidelinesvercel-labs676KReview UI code for Web Interface Guidelines compliance. Use when asked to "review my UI", "check accessibility", "audit design", "review UX", or "check my site against best practices".code-reviewmattpocock631KReview the changes since a fixed point (commit, branch, tag, or merge-base) along two axes: Standards (does the code follow this repo's documented coding standards?) and Spec (does the code match what the originating issue/spec asked for?). Runs both reviews in parallel sub-agents and reports them side by side. Use when the user wants to review a branch, a PR, work-in-progress changes, or asks to \"review since X\".