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