# Code Review

> 互動式程式碼審查：逐段檢視架構、程式品質、測試、效能。 可審查 git diff、指定檔案、或整個 PR。 觸發詞：/code-review、review code、審查程式碼、code review

- Skill: `yelban/code-review` (Agent Skill)
- Install (CLI): `npx skillmds@latest add yelban/code-review`
- Raw SKILL.md: https://api.skillmd.com/api/skills/yelban/code-review/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Coding & Dev Tools
- Author: yelban (https://skillmd.com/u/yelban)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/yelban/code-review

---


# 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: 審查摘要

四段審查完成後，輸出：

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