shandar Code Review Skill
This skill defines how to perform a structured code review. Use it when reviewing your own output before presenting to the user, or when asked to review existing code.
Review Mindset
- You are a reviewer, not an author. Your job is to find problems, not defend choices.
- Every comment must be actionable. "This could be better" is not a review comment. "Extract this into a utility function to reduce duplication with line 45" is.
- Prioritize correctness over style. A bug matters more than a naming preference.
Review Checklist
For every code change, check the following in order:
1. Correctness
- Does the code do what it claims to do?
- Are all edge cases handled? (null, empty, boundary values, error states)
- Are async operations properly awaited? Error-handled?
- Are types correct and strict? (no
anyescapes, no type assertions without justification)
2. Security
- Is user input sanitized before rendering or database writes?
- Are secrets kept out of client code?
- Are permissions checked server-side, not just hidden in UI?
- Are dependencies from trusted sources?
3. Readability
- Can you understand the code without the author explaining it?
- Are variable and function names descriptive and consistent?
- Is the code structured logically? (related things together, clear flow)
- Are comments present where the why isn't obvious? Absent where it is?
4. Performance
- Are there unnecessary re-renders, re-fetches, or re-computations?
- Are large operations debounced or throttled?
- Are heavy modules lazy-loaded?
- Is data fetching efficient? (no N+1 queries, no over-fetching)
5. Maintainability
- Is the code DRY without being over-abstracted?
- Are there magic numbers or hardcoded values that should be constants?
- Is the change scoped correctly? (no mixed concerns in one commit)
- Will the next developer understand this in 6 months?
6. Testing
- Is the change tested? (new behavior has tests, bug fixes have regression tests)
- Are tests testing behavior, not implementation?
- Are test names descriptive?
Severity Levels
| Level | Meaning | Action Required |
|---|---|---|
| Blocker | Bug, security flaw, or data loss risk | Must fix before merge |
| Warning | Logic concern, missing edge case, performance issue | Should fix; merge with caution if deferred |
| Suggestion | Readability improvement, naming preference, minor optimization | Optional; author decides |
| Nit | Formatting, whitespace, trivial style | Fix if easy; ignore if not |
Comment Format
[SEVERITY] file:line — description
Example:
[Blocker] src/auth/login.ts:42 — Password comparison uses == instead of timing-safe comparison.
[Warning] src/hooks/useCart.ts:18 — Missing cleanup in useEffect return. Will cause memory leak.
[Suggestion] src/utils/format.ts:7 — Consider renaming `fmt` to `formatCurrency` for clarity.
Self-Review Protocol
Before presenting any code to the user:
- Re-read every line you changed
- Run through the checklist above mentally
- Flag anything you're less than 90% confident about
- Verify the change compiles and builds
- Confirm you haven't modified files outside the declared scope
Anti-Patterns
- Rubber-stamp reviews: Approving without actually reading. Never.
- Style-only reviews: Catching semicolons but missing null pointer exceptions.
- Ego reviews: Rewriting working code to match personal preference.
- Drive-by comments: Pointing out issues without suggesting fixes.