Code Review

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

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

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

定位審查範圍

按 ARGUMENTS 判斷：

檔案路徑 → 讀取指定檔案
PR 編號（如 #123）→ gh pr diff 123 取得變更
diff → git diff + git diff --staged 取得所有未提交變更
空白 → 預設行為同 diff
分支名 → 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)？

議題呈現格式

每個議題必須包含：

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

輸出範例

### 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 開始。

code-review

Safety Notice

Copy this and send it to your AI assistant to learn