| name | code-reviewer |
| description | 8-layer structured code review. Use before any PR, merge, or deploy. Triggers on "review this code", "review my PR", "check this before I merge", "ai review", or when the user shares a diff, file, or code block requesting evaluation. Covers logic, edge cases, security, payroll compliance, error handling, performance, test coverage, and readability — in that order by severity. |
Code Reviewer Skill
Review Protocol
Execute all 8 layers in order. Report findings at each layer before moving to next.
Do not skip a layer because it seems unlikely to have issues.
The ordering is by severity: logic errors are caught first, style last.
A finding in Layer 3 (Security) or Layer 4 (Compliance) = BLOCK.
The PR does not merge until Layer 3 and 4 findings are resolved.
Layer 1 — Logic Correctness
- Does the code do what the spec or ticket requires?
- Are there off-by-one errors, wrong comparators, or inverted conditions?
- Are all return paths handled? (Missing else, unhandled null, early return that skips cleanup)
- Are boolean conditions correctly composed? (AND/OR precedence, short-circuit behavior)
- Are async operations correctly awaited? (Missing await, unhandled promise rejection)
- Do loops terminate? Are loop counters correct?
Layer 2 — Edge Cases & Boundary Conditions
- Empty collections, null inputs, undefined, zero values, negative numbers
- String: empty string, whitespace-only, max length, special characters, unicode
- Date/time: timezone handling, DST transitions, leap year, end of month, end of year
- Concurrency: race conditions, double-submit, state mutations across async boundaries
- Large inputs: will this scale to 10,000 employees? 100,000 records?
Layer 3 — Security (BLOCK if any finding)
- SQL/NoSQL injection: any user input flowing into a query without parameterization?
- XSS: any user input rendered as HTML without escaping?
- Command injection: any user input in a shell command or exec call?
- Authentication: is this endpoint protected? JWT verified? Session validated?
- Authorization: is the right role checked? Not just "logged in" but "allowed to do this"?
- Data exposure: does any response include fields the client should not see?
(SSN, full bank account, other employees' data)
- Secret management: no hardcoded credentials, API keys, connection strings, or tokens
- New dependencies: are they pinned? Audited? No known CVEs?
Layer 4 — Compliance & Regulatory (BLOCK if any finding)
FLSA:
- Overtime rules correct (40h threshold, rate calculation)?
- Exempt/non-exempt classification applied correctly?
- Recordkeeping fields present for new employee records?
IRS Publication 15:
- Withholding tables current for the tax year?
- Supplemental wage rate (22% federal) applied correctly?
- Fringe benefit treatment correct?
Privacy (CCPA/HIPAA where applicable):
- PII collected only where necessary?
- PII stored encrypted at rest?
- No PII logged in plain text (console.log, debug output)?
- Data retention rules respected?
Audit trail:
- All changes to pay-affecting records logged with who/when/what?
- Audit log entries are immutable (append-only)?
Multi-state:
- New state introduced? All state-specific rules handled?
- Apportionment logic correct for multi-state employees?
Layer 5 — Error Handling
- Every external call (DB, API, file system, third party) has explicit failure handling
- Errors are caught at the right level (not swallowed silently)
- Error logs include enough context to debug in production (request ID, employee ID, pay period)
- User-facing error messages reveal no internal state, stack traces, or system details
- Payroll fail-safe: partial payment states are worse than no payment — ensure atomicity
Layer 6 — Performance & Scalability
- N+1 query patterns: is there a DB call inside a loop? Batch it.
- Missing indexes on newly queried columns? Check EXPLAIN plan.
- Memory leaks: resources opened (connections, streams, file handles) but not closed?
- Payload size: are large collections paginated or streamed?
- Synchronous operations blocking the event loop on CPU-intensive payroll calculations?
Layer 7 — Test Coverage
- Are new behaviors covered by tests?
- Do the tests verify behavior (what the code does) vs implementation (how it does it)?
- Are edge cases from Layer 2 covered by tests?
- Are the tests brittle? (Testing internal state, mocking too much, snapshots of huge objects)
- Can edge cases be tested without standing up the entire application?
Layer 8 — Readability & Maintainability
- Naming: do names describe what the thing IS, not how it's implemented?
✅
calculateBiweeklyGross ❌ processStep2
- Function length: over 40 lines is a signal. Review for extraction opportunity.
- Comments: present only where the "why" is non-obvious
(not describing what the code does — that's what code is for)
- Duplication: any logic repeated more than twice? Extract and name it.
- Complexity: cyclomatic complexity above 10 in one function? Split it.
Output Format
## Code Review Report — [Date]
**Layer 1 (Logic):** [PASS | N findings — list below]
**Layer 2 (Edge Cases):** [PASS | N findings — list below]
**Layer 3 (Security):** [PASS | N findings — BLOCK if any]
**Layer 4 (Compliance):** [PASS | N findings — BLOCK if any]
**Layer 5 (Errors):** [PASS | N findings]
**Layer 6 (Performance):**[PASS | N findings]
**Layer 7 (Tests):** [PASS | N findings]
**Layer 8 (Readability):**[PASS | N findings]
**Verdict:** APPROVE / REQUEST CHANGES / BLOCK
**Critical findings (must fix before merge):**
[Layer 3–4 findings in detail]
**Should-fix findings (strong recommendation):**
[Layer 1–2 findings in detail]
**Consider fixing (low priority):**
[Layer 5–8 findings]
