Plan-Mode Review Checklist
A structured review pass that catches issues before they ship. Walk through each section in order. A finding in section 1 invalidates everything in sections 2-4 -- don't review code that isn't the right code.
1. Architecture
- Is the right system absorbing this change? If a frontend bug is being fixed in the backend, stop.
- Are existing patterns being matched, or is a new pattern being introduced? New patterns require justification.
- Is this a one-way door (hard to reverse) or a two-way door? One-way decisions get more scrutiny.
- Does the public API change? If yes, what's the migration story?
- Does the change cross trust boundaries? (See
security-audit.)
2. Code quality
- Functions ≤ 70 lines, files ≤ 500 lines, args ≤ 5, nesting ≤ 3 levels.
- Function name doesn't contain "and"; if it does, split.
- No
any (TypeScript) or Any (Python) without explicit, documented justification.
- No fallback chains (
a or b or c). Explicit branches.
- No ternaries used for control flow. Early returns instead.
- Comments explain intent, not what. Remove narrative comments.
- Match the surrounding style. If the file uses factories, use factories.
3. Tests
- New behavior has a new test. Bug fixes have a regression test that fails before the fix.
- Tests describe invariants ("a paid user cannot see the trial banner"), not specific bugs ("regression test for issue #1234").
- No tests that mock the system under test (mock-echo). Tests must exercise real behavior.
- Tests pass under the real CI command, not a local subset.
4. Performance
- Hot paths: any new allocation in a per-request path? Any new DB round-trip? Any N+1?
- New dependencies pulled in transitively? Bundle size delta if frontend.
- Any new feature flag, cache, or polling? Justify TTLs.
5. Errors and observability
- Failure modes have specific error types, not generic
Exception / Error.
- User-facing errors don't leak internal state (host names, IPs, stack traces, schema).
- Operator-facing logs are structured (logger.error with kv pairs), not f-string blobs.
- Correct HTTP status code: 422 for malformed fields, 409 for state conflicts, 400 for syntax, 500 for our bugs.
6. Docs
- Module docstring updated if public surface changed.
- README example updated if usage changed.
- Migration note if this is a breaking change.
7. Pre-ship
pnpm typecheck clean (or npx tsc --noEmit for the affected project).pnpm lint clean.pnpm test <project> passing.pnpm build <project> passing.- Pre-existing failures are reported separately from new failures.
When I run this against my own work
I run this checklist on every change before declaring it done, in this order. If any item is "no" or "unclear", I block the change and report. The user gets the unedited list of failures, not a clean "looks good".
1---2name: plan-mode-review3description: Structured review checklist before implementing changes. Use when planning a feature, before opening a PR, or when the user shares code that needs scrutiny across architecture, code quality, tests, and performance.4license: MIT5---67# Plan-Mode Review Checklist89A structured review pass that catches issues before they ship. Walk through each section in order. A finding in section 1 invalidates everything in sections 2-4 -- don't review code that isn't the right code.1011## 1. Architecture1213- Is the right system absorbing this change? If a frontend bug is being fixed in the backend, stop.14- Are existing patterns being matched, or is a new pattern being introduced? New patterns require justification.15- Is this a one-way door (hard to reverse) or a two-way door? One-way decisions get more scrutiny.16- Does the public API change? If yes, what's the migration story?17- Does the change cross trust boundaries? (See `security-audit`.)1819## 2. Code quality2021- Functions ≤ 70 lines, files ≤ 500 lines, args ≤ 5, nesting ≤ 3 levels.22- Function name doesn't contain "and"; if it does, split.23- No `any` (TypeScript) or `Any` (Python) without explicit, documented justification.24- No fallback chains (`a or b or c`). Explicit branches.25- No ternaries used for control flow. Early returns instead.26- Comments explain *intent*, not *what*. Remove narrative comments.27- Match the surrounding style. If the file uses factories, use factories.2829## 3. Tests3031- New behavior has a new test. Bug fixes have a regression test that fails before the fix.32- Tests describe invariants ("a paid user cannot see the trial banner"), not specific bugs ("regression test for issue #1234").33- No tests that mock the system under test (mock-echo). Tests must exercise real behavior.34- Tests pass under the real CI command, not a local subset.3536## 4. Performance3738- Hot paths: any new allocation in a per-request path? Any new DB round-trip? Any N+1?39- New dependencies pulled in transitively? Bundle size delta if frontend.40- Any new feature flag, cache, or polling? Justify TTLs.4142## 5. Errors and observability4344- Failure modes have specific error types, not generic `Exception` / `Error`.45- User-facing errors don't leak internal state (host names, IPs, stack traces, schema).46- Operator-facing logs are structured (logger.error with kv pairs), not f-string blobs.47- Correct HTTP status code: 422 for malformed fields, 409 for state conflicts, 400 for syntax, 500 for our bugs.4849## 6. Docs5051- Module docstring updated if public surface changed.52- README example updated if usage changed.53- Migration note if this is a breaking change.5455## 7. Pre-ship5657- `pnpm typecheck` clean (or `npx tsc --noEmit` for the affected project).58- `pnpm lint` clean.59- `pnpm test <project>` passing.60- `pnpm build <project>` passing.61- Pre-existing failures are reported separately from new failures.6263## When I run this against my own work6465I run this checklist on every change before declaring it done, in this order. If any item is "no" or "unclear", I block the change and report. The user gets the unedited list of failures, not a clean "looks good".
kevin-liu-01