| name | plan-mode-review |
| description | 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. |
| version | 1.0.0 |
| author | Kevin Liu |
| license | MIT |
| metadata | {"agent-machines":{"tags":["review","planning","code-quality","architecture"],"related_skills":["agent-ethos","empirical-verification","security-audit"]}} |
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".
