--- name: code-review description: | 互動式程式碼審查:逐段檢視架構、程式品質、測試、效能。 可審查 git diff、指定檔案、或整個 PR。 觸發詞:/code-review、review code、審查程式碼、code review user_invocable: true argument_hint: "[file path, PR number, or 'diff'] (optional, defaults to staged + unstaged changes)" --- # 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...` 取得分支差異 讀取變更後,摘要變更範圍(改了哪些檔案、大致做了什麼),確認審查對象正確。 ### 讀取上下文 變更檔案的 **完整內容**(不只 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: 審查摘要 四段審查完成後,輸出: ```markdown ## 程式碼審查摘要 | # | 維度 | 議題 | 決策 | |---|------|------|------| | 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 開始。