Code Review

/plan-review

Step 0: Define Review Scope + Select Review Depth

Define Review Scope

1. File path → Read the specified file
2. PR number (e.g.,

   #123

   ) → Retrieve changes via

   gh pr diff 123

3. diff

   → Retrieve all unstaged changes via

   git diff

   +

   git diff --staged

4. Empty → Default behavior is the same as

   diff

5. Branch name → Retrieve branch differences via

   git diff main...<branch>

Read Context

Select Review Depth

• In-depth Review: Max 4 issues per section, suitable for important features or PRs
• Quick Review: Max 1 issue per section, suitable for small changes or hotfixes

Step 1-4: Four-Section Review

Section 1: Architecture

• Are component boundaries reasonable? Is there cross-layer access or confused responsibilities?
• Is the dependency direction correct? Are there circular dependencies?
• Is the data flow pattern clear? Is there implicit state transfer?
• Security considerations: Authentication, authorization, input validation, API boundaries

Section 2: Code Quality

• DRY violations—proactively flag duplicate logic (even if only repeated twice)
• Error handling: Are there unhandled error paths? Do catch blocks swallow errors?
• Edge cases: Null values, empty arrays, concurrency, timeouts, large datasets
• Naming and readability: Do variable names express intent? Is the logic too obscure?
• Over-engineering: Unnecessary abstractions, unused flexibility, premature optimization

Section 3: Testing

• Is there any new or modified logic without corresponding tests?
• Do tests validate behavior rather than implementation details?
• Edge case coverage: Invalid inputs, null values, extreme values, concurrency
• Failure paths: Network errors, insufficient permissions, non-existent resources, timeouts
• Are tests maintainable? Is there over-mocking or fragile assertions?

Section 4: Performance

• N+1 queries: Database/API calls within loops
• Unnecessary computations or repeated work
• Memory: Large objects, unreleased resources, unbounded collections
• Caching opportunities: Repeated expensive operations
• Algorithm complexity: Can any O(n²) operations be reduced to O(n)?

Issue Presentation Format

1. Issue Description — Specifically indicate

   file:line

   or

   file:SymbolName

2. Options — 2-3 options (including "No action"), each marked with:
   • Implementation cost (Low/Medium/High)
   • Risk (Likelihood of introducing bugs)
   • Scope of impact (How many files to modify? Will other functions be affected?)
   • Maintenance burden (Long-term impact)
3. Recommendation — Recommended option and rationale

Output Example

### 1. Quality: Duplicate Validation Logic

**Issue**: `src/handlers/create.ts:23` and `src/handlers/update.ts:31`
have nearly identical email validation logic (regex + length check + domain whitelist).

**Options**:
- **A) Extract validateEmail() as a shared function** (Recommended)
  Cost: Low | Risk: Low | Impact: 2 files | Maintenance: Reduces duplication
- **B) No action**
  Cost: Zero | Risk: Medium (Forgot to update one when changing the other) | Impact: None | Maintenance: Two copies need to be synchronized
- **C) Unify validation with zod schema**
  Cost: Medium | Risk: Low | Impact: Requires adding zod dependency | Maintenance: Declarative and clearer

**Recommendation A**: Eliminates duplication at the lowest cost without introducing new dependencies.

AskUserQuestion Format

• Option labels marked with Issue number + Option letter
• Recommended option placed first
• In-depth mode example:

  1A + 2B + 3A + 4A

• Quick mode uses option letters directly

Step 5: Review Summary

## Code Review Summary

| # | Dimension | Issue | Decision |
|---|-----------|-------|----------|
| 1 | Architecture | UserService cross-layer coupling | A: Via OrderService |
| 2 | Quality | Duplicate validation logic | A: Extract shared function |
| 3 | Testing | Missing failure path tests | A: Add 3 tests |
| 4 | Performance | N+1 query | A: Switch to batch query |

### To-Do Items
- [ ] `src/services/user.ts:45` — Use OrderService for indirect access
- [ ] `src/handlers/` — Extract validateEmail() to src/utils/validation.ts
- [ ] `src/handlers/create.test.ts` — Add 3 failure path tests
- [ ] `src/repositories/order.ts:67` — Change N+1 query to batch query

Review Principles

• DRY is important — Proactively flag duplicates, even if only repeated twice
• Testing first — Better to test more than less
• Moderate engineering — Neither over-abstract nor too hacky
• Edge cases — Better to think more than miss
• Explicit over clever — explicit > clever
• Only review changes — Do not comment on unchanged code (unless changes expose existing issues)

Now Execute

ARGUMENTS: