QA Code Review
Core Rule: Find Everything in One Pass
Never stop at surface-level issues. Complete the full analysis before writing any fixes. The user is paying per token — finding issues across multiple rounds is a failure mode.
Phase 1: Read Everything First
- Read every source file in the project (not just the ones that look suspicious)
- Read package.json / dependency files for unused or outdated deps
- Read schema files, env templates, and config
Do NOT start writing fixes until Phase 2 is complete.
Phase 2: Systematic Audit Checklist
Mentally simulate the full request lifecycle for every entry point. Check every file against ALL of the following categories:
Runtime & Initialization
- Module-level code that reads env vars or config at import time (eager vs lazy)
- Code that assumes a specific OS (hardcoded paths like
/tmp/, line endings) - Clients/SDKs initialized as
undefinedwhen credentials are missing — will callers get a clear error or a cryptic TypeError?
Error Handling
- Every
await— what happens if it throws? Does the user get stuck with no response? - Error responses — do they leak internal details (stack traces, error messages, env var names)?
- Execution order — if step 3 of 5 fails, has the user already received partial output? Should they?
Concurrency & Idempotency
- Can the same request arrive twice? (webhooks, retries, double-clicks)
- Race conditions — check-then-act patterns without locks (e.g., count check then insert)
- Unique constraint handling — what happens on concurrent inserts?
Data Integrity
- Null/undefined propagation at every function boundary
- Functions that return
null— do all callers handle it? - Template literals with potentially undefined variables (produces
"undefined"string)
Security
- Secrets in error responses or logs
- Missing input validation on external data (webhook payloads, query params)
- API versions — are they current or approaching deprecation?
Dead Code & Hygiene
- Exported functions never imported anywhere
- Constants defined but never read
- Dependencies in package.json not imported in any source file
- Status/enum values defined but never checked by callers
Phase 3: Report Before Fixing
Present the COMPLETE issue list with severity ratings before making any changes:
| Severity | Meaning |
|---|---|
| CRITICAL | Will cause production failures or data loss |
| HIGH | Users will experience broken flows or silent failures |
| MEDIUM | Correctness issues, unnecessary risk, or poor DX |
| LOW | Dead code, style, hygiene |
Format: a numbered table with severity, file, and one-line description.
Phase 4: Fix All Issues
Only after the full list is presented and acknowledged, fix everything in a single batch. Group changes by file for clean diffs.
Anti-Patterns to Avoid
- Stopping after 5-8 findings because it "looks like enough" — always complete the full checklist
- Fixing as you find — this biases you toward shallow, early-discovered issues
- Scanning instead of reasoning — pattern-matching catches formatting issues; mental execution catches runtime bugs
- Skipping non-obvious files — schema.sql, .env.example, and config files contain real bugs too