Code Review

實作後的程式碼審查。四個維度逐段檢視，每個議題給選項和建議，確認後才進下一段。

定位：審查已寫好的程式碼（架構、品質、測試、效能），不是計畫審查。計畫層面的審查請用 /plan-review。

Step 0: 定位審查範圍 + 選擇深度

定位審查範圍

按 ARGUMENTS 判斷：

1. 檔案路徑 → 讀取指定檔案
2. PR 編號（如 #123）→ gh pr diff 123 取得變更
3. diff → git diff + git diff --staged 取得所有未提交變更
4. 空白 → 預設行為同 diff
5. 分支名 → git diff main...<branch> 取得分支差異

讀取變更後，摘要變更範圍（改了哪些檔案、大致做了什麼），確認審查對象正確。

讀取上下文

變更檔案的 完整內容（不只 diff hunk）→ 全部讀取。同時讀取被 import 或依賴的關鍵檔案，理解上下文後才能給出有意義的建議。

選擇審查深度

用 AskUserQuestion 詢問：

• 深度審查：每段最多 4 個議題，適合重要功能或 PR
• 快速審查：每段最多 1 個議題，適合小修改或 hotfix

Step 1-4: 四段審查

依序執行。每段完成後用 AskUserQuestion 確認決策，再進下一段。

第一段：架構

系統設計層面的問題。

• 元件邊界是否合理？有沒有跨層存取或職責混亂？
• 依賴方向是否正確？有沒有循環依賴？
• 資料流模式是否清晰？有沒有隱式狀態傳遞？
• 安全考量：認證、授權、輸入驗證、API 邊界

第二段：程式品質

程式碼本身的問題。

• DRY 違規——積極標記重複邏輯（即使只重複兩次）
• 錯誤處理：是否有未處理的錯誤路徑？catch 區塊是否吞掉錯誤？
• 邊界案例：空值、空陣列、並發、超時、大量資料
• 命名與可讀性：變數名是否表達意圖？邏輯是否過於隱晦？
• 過度工程：不必要的抽象、用不到的彈性、過早最佳化

第三段：測試

測試覆蓋與品質。

• 有沒有新增或修改的邏輯缺少對應測試？
• 測試是否驗證行為而非實作細節？
• 邊界案例覆蓋：錯誤輸入、空值、極端值、並發
• 失敗路徑：網路錯誤、權限不足、資源不存在、超時
• 測試是否可維護？有沒有過度 mock 或脆弱的斷言？

第四段：效能

效能與資源使用。

• N+1 查詢：迴圈中的資料庫/API 呼叫
• 不必要的計算或重複工作
• 記憶體：大物件、未釋放的資源、無界限的集合
• 快取機會：重複的昂貴操作
• 演算法複雜度：有沒有 O(n²) 可以降為 O(n)？

議題呈現格式

每個議題必須包含：

1. 問題描述 — 具體指出 file:line 或 file:SymbolName
2. 選項 — 2-3 個選項（含「不處理」），每個標明：
   • 實作成本（低/中/高）
   • 風險（引入 bug 的可能性）
   • 影響範圍（改幾個檔案？影響其他功能？）
   • 維護負擔（長期影響）
3. 建議 — 推薦的選項及理由

輸出範例

### 1. 品質：驗證邏輯重複

**問題**：`src/handlers/create.ts:23` 和 `src/handlers/update.ts:31`
有幾乎相同的 email 驗證邏輯（正則 + 長度檢查 + domain 白名單）。

**選項**：
- **A) 抽出 validateEmail() 共用函式**（建議）
  成本：低 | 風險：低 | 影響：2 個檔案 | 維護：減少重複
- **B) 不處理**
  成本：零 | 風險：中（改一處忘改另一處）| 影響：無 | 維護：兩份要同步
- **C) 用 zod schema 統一驗證**
  成本：中 | 風險：低 | 影響：需加 zod 依賴 | 維護：宣告式更清晰

**建議 A**：最低成本消除重複，不引入新依賴。

AskUserQuestion 格式

每段結束時用 AskUserQuestion：

• 選項標籤標示 議題編號 + 選項字母
• 建議選項放第一個
• 深度模式範例：1A + 2B + 3A + 4A
• 快速模式直接用選項字母

Step 5: 審查摘要

四段審查完成後，輸出：

## 程式碼審查摘要

| # | 維度 | 議題 | 決策 |
|---|------|------|------|
| 1 | 架構 | UserService 跨層耦合 | A: 透過 OrderService |
| 2 | 品質 | 驗證邏輯重複 | A: 抽出共用函式 |
| 3 | 測試 | 缺少錯誤路徑測試 | A: 補 3 個測試 |
| 4 | 效能 | N+1 查詢 | A: 改用 batch query |

### 待修項目
- [ ] `src/services/user.ts:45` — 改用 OrderService 間接存取
- [ ] `src/handlers/` — 抽出 validateEmail() 到 src/utils/validation.ts
- [ ] `src/handlers/create.test.ts` — 補充 3 個錯誤路徑測試
- [ ] `src/repositories/order.ts:67` — N+1 改為 batch query

如果所有議題都選擇「不處理」，直接輸出 LGTM 並說明原因。

審查原則

• DRY 重要 — 積極標記重複，即使只重複兩次
• 測試優先 — 寧可多測不可少測
• 工程適度 — 不過度抽象，也不太 hacky
• 邊界案例 — 寧可多想不可遺漏
• 明確優於聰明 — explicit > clever
• 只審查變更 — 不要對沒改動的程式碼提意見（除非變更暴露了既有問題）

Now Execute

使用者的審查請求見下方 ARGUMENTS:。從 Step 0 開始。