Code Reviewer Skill
Core Philosophy
"Review for the author and the future maintainer."
Feedback should be clear, actionable, and prioritized. Explain the "why" for non-obvious suggestions.
Related Skills
Code review should work alongside these skills. Invoke them when their domain is relevant:
| Skill |
When to Invoke |
How to Invoke |
| security-reviewer |
Deep security audit for auth, crypto, injection, sensitive data. Always invoke for security-critical changes. |
Read skills/security-reviewer/SKILL.md and follow its protocol |
| testing |
Verify tests exist and pass. If tests are missing, STOP and require tests before approving. |
Read skills/testing/SKILL.md and follow its protocol |
| performance |
Profile hotspots, query patterns, algorithmic complexity. Invoke when performance is a concern. |
Read skills/performance/SKILL.md and follow its protocol |
IMPORTANT: Code review should NOT approve code that lacks tests. If the change has no tests, flag it as Critical and require tests before merge.
Protocol
1. Scope
- Read the full diff or changed files.
- Consider context: ticket/PR description, related tests, and project structure.
- Verify tests exist for new/changed code. If missing, mark as Critical.
2. Review Dimensions
| Dimension |
Focus |
| Correctness |
Logic, edge cases, off-by-one, null/empty handling, error paths. |
| Testing |
⛔ Tests MUST exist. New code must have tests. No tests = Critical blocker. Invoke testing skill if needed. |
| Security |
Input validation, injection risks, sensitive data. Invoke security-reviewer for auth, crypto, or sensitive changes. |
| Performance |
N+1 queries, algorithmic complexity, memory leaks. Invoke performance skill for hotspots or optimization. |
| Readability |
Names, structure, comments where needed, no dead or misleading code. |
| Maintainability |
Duplication, coupling, testability, size/complexity of functions and modules. |
| Conventions |
Style, patterns, and practices used in the rest of the project. |
| Accessibility |
Alt text, ARIA labels, keyboard navigation, color contrast, semantic HTML (for UI changes). |
| i18n |
No hardcoded user-facing strings, locale-aware formatting, RTL support (for UI changes). |
| Migrations |
Reversible, zero-downtime safe, data integrity preserved, tested (for database changes). |
3. Severity Levels
Format feedback with severity so the author can triage. The code-reviewer uses a simplified 3-tier model; the security-reviewer uses a more granular 4-tier model (Critical/High/Medium/Low):
| Severity |
Code Review Meaning |
Security Review Equivalent |
| Critical |
Bug or correctness/security issue; must fix before merge |
Critical / High |
| Suggestion |
Improvement for readability, performance, or maintainability |
Medium |
| Nit |
Minor style or preference; optional |
Low / Info |
4. Output Format
Structure the review as:
## Summary
[1–2 sentences on what the change does and overall assessment]
## Gate Status
- [ ] Tests: [Pass/Fail/Missing] – If missing, mark Critical
- [ ] Build: [Pass/Fail/N/A]
- [ ] Security: [OK/Needs deep audit via security-reviewer]
## Critical
- [Item] – [Brief explanation and, if possible, fix or reference]
- ⛔ **No tests for [component]** – Tests are mandatory. Add tests before merge.
## Suggestions
- [Item] – [Explanation]
## Nits
- [Item]
## Positive notes (optional)
- [What was done well]
## Related Skill Invocations (if needed)
- [ ] Invoke **security-reviewer** for: [reason]
- [ ] Invoke **performance** for: [reason]
- [ ] Invoke **testing** to add tests for: [components]
5. Commands
- No special commands. Review the code the user points at (file, diff, or PR).
- If the repo has a review checklist or CONTRIBUTING.md, align with it.
6. When to Invoke Related Skills
| Situation |
Action |
| No tests for new code |
⛔ Mark Critical. Invoke testing skill to add tests. |
| Auth, crypto, or sensitive data |
Invoke security-reviewer for deep audit. |
| Performance concerns (N+1, loops, memory) |
Invoke performance skill to profile. |
| UI changes without a11y/i18n |
Flag in review; may need testing for a11y tests. |
| Database migrations |
Ensure migration tests exist; invoke testing if missing. |
Scope Boundary: Code-reviewer owns correctness, readability, maintainability, testing coverage, conventions, accessibility, and i18n. For vulnerabilities, authentication, cryptography, and sensitive data exposure, defer to the security-reviewer skill.
In workflow context (Phase 5): Code-reviewer and security-reviewer run in parallel. If issues are found, the workflow MUST stop and fix before proceeding.
Checklist
⛔ BLOCKING (must pass before approval):
Review quality:
⚠️ Do NOT approve code without tests. Mark as Critical and require tests first.
1---2name: code-reviewer3description: Review code for correctness, readability, maintainability, accessibility, i18n, and alignment with project conventions. Use when the user asks for a code review, review this PR, review my code, or when examining diffs, pull requests, or patches.4---56# Code Reviewer Skill78## Core Philosophy910**"Review for the author and the future maintainer."**1112Feedback should be clear, actionable, and prioritized. Explain the "why" for non-obvious suggestions.1314---1516## Related Skills1718Code review should work alongside these skills. **Invoke them when their domain is relevant:**1920| Skill | When to Invoke | How to Invoke |21| --------------------- | ------------------------------------------------------------------------------------------------------------- | ------------- |22| **security-reviewer** | Deep security audit for auth, crypto, injection, sensitive data. Always invoke for security-critical changes. | Read `skills/security-reviewer/SKILL.md` and follow its protocol |23| **testing** | Verify tests exist and pass. If tests are missing, **STOP** and require tests before approving. | Read `skills/testing/SKILL.md` and follow its protocol |24| **performance** | Profile hotspots, query patterns, algorithmic complexity. Invoke when performance is a concern. | Read `skills/performance/SKILL.md` and follow its protocol |2526**IMPORTANT:** Code review should **NOT** approve code that lacks tests. If the change has no tests, flag it as **Critical** and require tests before merge.2728---2930## Protocol3132### 1. Scope3334- Read the full diff or changed files.35- Consider context: ticket/PR description, related tests, and project structure.36- **Verify tests exist** for new/changed code. If missing, mark as Critical.3738### 2. Review Dimensions3940| Dimension | Focus |41| ------------------- | ----------------------------------------------------------------------------------------------------------------------- |42| **Correctness** | Logic, edge cases, off-by-one, null/empty handling, error paths. |43| **Testing** | ⛔ **Tests MUST exist.** New code must have tests. No tests = Critical blocker. Invoke **testing** skill if needed. |44| **Security** | Input validation, injection risks, sensitive data. **Invoke security-reviewer** for auth, crypto, or sensitive changes. |45| **Performance** | N+1 queries, algorithmic complexity, memory leaks. **Invoke performance** skill for hotspots or optimization. |46| **Readability** | Names, structure, comments where needed, no dead or misleading code. |47| **Maintainability** | Duplication, coupling, testability, size/complexity of functions and modules. |48| **Conventions** | Style, patterns, and practices used in the rest of the project. |49| **Accessibility** | Alt text, ARIA labels, keyboard navigation, color contrast, semantic HTML (for UI changes). |50| **i18n** | No hardcoded user-facing strings, locale-aware formatting, RTL support (for UI changes). |51| **Migrations** | Reversible, zero-downtime safe, data integrity preserved, tested (for database changes). |5253### 3. Severity Levels5455Format feedback with severity so the author can triage. The code-reviewer uses a simplified 3-tier model; the security-reviewer uses a more granular 4-tier model (Critical/High/Medium/Low):5657| Severity | Code Review Meaning | Security Review Equivalent |58|----------|-------------------|---------------------------|59| **Critical** | Bug or correctness/security issue; must fix before merge | Critical / High |60| **Suggestion** | Improvement for readability, performance, or maintainability | Medium |61| **Nit** | Minor style or preference; optional | Low / Info |6263### 4. Output Format6465Structure the review as:6667```markdown68## Summary6970[1–2 sentences on what the change does and overall assessment]7172## Gate Status7374- [ ] Tests: [Pass/Fail/Missing] – If missing, mark Critical75- [ ] Build: [Pass/Fail/N/A]76- [ ] Security: [OK/Needs deep audit via security-reviewer]7778## Critical7980- [Item] – [Brief explanation and, if possible, fix or reference]81- ⛔ **No tests for [component]** – Tests are mandatory. Add tests before merge.8283## Suggestions8485- [Item] – [Explanation]8687## Nits8889- [Item]9091## Positive notes (optional)9293- [What was done well]9495## Related Skill Invocations (if needed)9697- [ ] Invoke **security-reviewer** for: [reason]98- [ ] Invoke **performance** for: [reason]99- [ ] Invoke **testing** to add tests for: [components]100```101102### 5. Commands103104- No special commands. Review the code the user points at (file, diff, or PR).105- If the repo has a review checklist or CONTRIBUTING.md, align with it.106107### 6. When to Invoke Related Skills108109| Situation | Action |110| --------------------------------------------- | ------------------------------------------------------------ |111| **No tests for new code** | ⛔ Mark Critical. Invoke **testing** skill to add tests. |112| **Auth, crypto, or sensitive data** | Invoke **security-reviewer** for deep audit. |113| **Performance concerns (N+1, loops, memory)** | Invoke **performance** skill to profile. |114| **UI changes without a11y/i18n** | Flag in review; may need **testing** for a11y tests. |115| **Database migrations** | Ensure migration tests exist; invoke **testing** if missing. |116117**Scope Boundary:** Code-reviewer owns **correctness, readability, maintainability, testing coverage, conventions, accessibility, and i18n**. For vulnerabilities, authentication, cryptography, and sensitive data exposure, defer to the **security-reviewer** skill.118119**In workflow context (Phase 5):** Code-reviewer and security-reviewer run in parallel. If issues are found, the workflow MUST stop and fix before proceeding.120121---122123## Checklist124125**⛔ BLOCKING (must pass before approval):**126127- [ ] **Tests exist** for new/changed code (invoke **testing** skill if missing)128- [ ] **No security vulnerabilities** (invoke **security-reviewer** for deep audit)129- [ ] **Build passes** (for frontend/compiled languages)130131**Review quality:**132133- [ ] All changed files and main code paths considered.134- [ ] Feedback is specific (file/line or snippet when helpful).135- [ ] Severity is consistent (critical vs suggestion vs nit).136- [ ] No pile-on: limit to the most important points unless asked for depth.137- [ ] Performance concerns flagged (invoke **performance** skill if needed).138- [ ] Accessibility checked for UI changes (alt text, keyboard, ARIA).139- [ ] i18n checked for UI changes (no hardcoded strings, locale formatting).140- [ ] Migration safety checked for DB changes (reversible, zero-downtime).141142**⚠️ Do NOT approve code without tests. Mark as Critical and require tests first.**
micaelmalta