- Add /plan2code-3b-review: 5-step post-implementation review with adaptive scope, 11 dimensions, reference-file architecture, and Plan/Apply/Verify fixes - Unify workflow/skill file naming to single-dash (drop -- and --- conventions) - Add Devin platform support and CLAUDE.md MANDATORY FIRST STEP template - Guide agents to use semantic anchors over line numbers in workflow prompts - Bump version to 1.14.0
10 KiB
Dimension Checklists
Part of plan2code-3b-review — loaded during Step 3 (Analyze).
Non-obvious check items, anti-patterns, and "don't flag" guidance for each review dimension. Focus on what LLMs commonly miss.
1. Correctness
- Trace variable mutations through async paths — can a value change between check and use?
- Check type coercion at boundaries — string-to-number, truthy/falsy assumptions on
0/""/false - Check that map/filter/reduce callbacks handle all element shapes (null members, missing keys)
- Verify copy operations produce deep copies when mutation independence is required
- Check array bounds — does code handle empty arrays and out-of-range indices?
- Verify regex patterns reject edge cases (empty string, special chars, Unicode)
- Check for race conditions — shared mutable state across async reads/writes
- Verify error handling paths return/throw correctly — no silent swallowing
Anti-patterns: catch(e) {} silent suppression · if (value) when 0/""/false are valid · indexOf > 0 instead of !== -1 · Mutating arguments instead of returning new values
Don't flag: Intentional == for null coalescing (if (x == null)) · Missing switch default with TypeScript never exhaustive check · Optional chaining ?. returning undefined
2. Completeness
- Search for TODO, FIXME, HACK, XXX, TEMP — each must be intentional or tracked
- Check cleanup/teardown exists for every setup/initialization
- Verify rollback/undo logic for multi-step operations that can partially fail
- Check event listeners and subscriptions have corresponding unsubscribe/cleanup
- Verify all file/resource handles closed in both success and error paths
- Check all enum/union type values have handling — no missing cases
- Verify pagination — does code fetch all pages or just the first?
- Check retry logic has backoff and maximum retry count
Anti-patterns: Happy-path-only functions that return undefined on error · Event listeners without cleanup · Missing else in critical chains where the "impossible" case can occur
Don't flag: TODOs with linked ticket numbers · Features explicitly "not in scope" · Bare throw in catch blocks (intentional re-throw)
3. Security
- Check for injection — string concatenation in queries instead of parameterized
- Search for hardcoded secrets, API keys, tokens in source files and configs
- Verify sensitive data (passwords, tokens, PII) is not logged, even at debug level
- Check file path operations prevent traversal — no unsanitized
../from user input - Check for mass assignment —
Object.assign/spread from user input without allow-list - Verify error responses don't leak stack traces, internal paths, or system info
- Verify session tokens use cryptographic randomness, not
Math.random() - Check dependency versions against known CVEs
Anti-patterns: Dynamic code evaluation with user-influenced input · Plaintext password storage · JWT none algorithm accepted
Don't flag: Hardcoded non-sensitive values (page sizes, route paths) · Internal tools documented as trusted-environment-only · Test fixture credentials in .test./.fixture. files
4. Performance
- Check for N+1 queries — loops issuing a query per iteration instead of batching
- Check for synchronous blocking in async contexts (fs.readFileSync in server request handlers)
- Check for memory leaks — growing arrays/maps without bounds, unclosed streams
- Check for quadratic complexity — nested iterations over the same collection
- Verify bulk operations used where available (bulk insert vs individual)
- Check regex for catastrophic backtracking potential (nested quantifiers)
- Verify connection pools for database and HTTP clients, not per-request connections
- Check event handlers debounced/throttled for high-frequency events
Anti-patterns: await in for loops instead of Promise.all · Loading entire tables to filter in app code · New regex instances inside loops
Don't flag: readFileSync at startup/module load · Loading small files (<100KB) once · Missing caching in one-shot scripts/CLI tools
5. Standards
- Check function length — functions over 50 lines warrant scrutiny
- Verify DRY applied judiciously — shared logic extracted, not over-abstracted
- Check magic numbers/strings are extracted to named constants
- Verify comments explain "why" not "what"
- Check file organization matches project conventions
- Verify consistent async/await vs callbacks vs promises within a module
- Check naming conventions consistent (camelCase, snake_case, PascalCase)
- Verify linting rules not disabled without justification
Anti-patterns: Mixed naming conventions in one module · God objects/functions · Deep nesting (>3 levels) instead of early returns
Don't flag: Framework-imposed patterns · Single-use helpers for readability · Comments on complex algorithms/regex
6. Tech Debt
- Check for deprecated API usage — verify against current library versions
- Identify commented-out code blocks — should be removed or tracked
- Check for copy-paste blocks that could be shared utilities
- Look for dead feature flags — always true/false with no toggle
- Check for orphaned files — modules with no imports from the codebase
- Verify error messages reference current code, not stale names
- Check for inconsistent abstraction levels — mixing orchestration with low-level ops
- Look for workarounds with "temporary" comments that persisted
Anti-patterns: TODO: remove after migration with no date/ticket · Wrapper functions that just forward args · Multiple implementations of the same utility
Don't flag: Intentional per-platform duplication · Verbose code prioritizing clarity · Deprecated APIs with tracked migration tickets
7. Test Quality
- Check assertions are meaningful — not just
toBeTruthy()on objects - Verify tests are isolated — no shared mutable state between cases
- Check error case tests actually trigger the error path, not just catch any error
- Verify async tests properly await results — no fire-and-forget assertions
- Check for tests that pass for wrong reason — wrong assertion target, always-true conditions
- Verify mock/stub scope is minimal — only mock what's necessary
- Check test descriptions describe behavior, not implementation
- Verify critical paths have coverage — happy path, error path, edge cases
Anti-patterns: expect(fn).not.toThrow() without checking return value · Mocking the module under test · Assertions in callbacks that may never execute
Don't flag: Shared test utility files · Missing tests for generated/scaffolded code · Integration tests using real databases when project prefers it
8. Maintainability
- Check cyclomatic complexity — functions with >10 branch paths are hard to maintain
- Verify dependencies between modules are explicit, not implicit through globals
- Check for tight coupling — can this module be tested independently?
- Verify data transformations are traceable — can you follow a value from input to output?
- Check file length — files over 500 lines warrant scrutiny
- Verify similar operations use consistent approaches throughout
- Check boolean parameters are replaced with enums or option objects for clarity
- Verify function/variable names describe purpose without needing comments
Anti-patterns: Functions requiring implementation knowledge to call · Circular dependencies · God files · Stringly-typed APIs
Don't flag: Long cohesive files (comprehensive test suites) · Inherently complex domain functions · Coupling between genuinely related modules
9. Spec Compliance
- Verify every acceptance criterion has corresponding implementation
- Check file paths, names, directory structures match spec exactly
- Verify implementation doesn't add undocumented behavior beyond spec
- Check all spec-defined edge cases have explicit handling
- Verify task completion state in phase files matches actual implementation
- Check integration points match spec contracts
- Verify function signatures match spec definitions
- Check data formats match spec (JSON schema, file formats)
Anti-patterns: Implementing a "better" approach without raising the divergence · Assuming spec intent on ambiguous points · Marking tasks complete when implementation differs
Don't flag: Minor naming variations preserving intent · Defensive coding beyond spec · Details spec leaves to developer judgment
10. UX/DX
- Verify error messages are actionable — tell user what to do, not just what went wrong
- Check empty states have clear messaging — not blank pages or silent failures
- Verify configuration has sensible defaults — zero-config produces working setup
- Check breaking changes are communicated — deprecation warnings, migration guides
- Verify CLI tools have
--help, consistent flags, meaningful exit codes - Check bulk operations provide progress feedback and handle partial failures
- Verify documentation examples are runnable, not pseudo-code
- Check loading states exist for operations over 1 second
Anti-patterns: Stack traces shown to end users · Required config with no example file · 200 OK with error in body · Silent failures
Don't flag: Verbose debug output · Internal tooling with minimal polish · CLI tools requiring initial setup
11. Improvements
- Check for manual implementations of standard library functionality
- Identify error-prone patterns replaceable with safer abstractions
- Look for synchronous operations that could be parallelized
- Identify complex conditionals clearer as lookup tables or strategy patterns
- Check for hardcoded limits that should be configurable
- Look for opportunities to improve test coverage on critical paths
- Identify documentation gaps — undocumented public APIs
- Check for opportunities to use newer language features improving clarity
Anti-patterns: Rewrites for aesthetic reasons · Patterns from different ecosystems · Optimizations without evidence of problems · Premature DRY abstractions
Don't flag: Correct code written differently than you'd write it · Performance appropriate for actual load · Features tracked in backlog