---
name: code-review-principles
description: PR Code Review 原則與檢查清單。當需要審查 Pull Request、進行程式碼審查、或評估大型重構的品質與風險時使用。涵蓋正確性（資料完整性、Breaking Change、測試覆蓋）、可讀性、架構、安全性、效能、型別安全、第三方依賴等面向。
---

# Code Review 原則

針對 Pull Request 進行結構化、可重複的程式碼審查，確保每次 review 都覆蓋關鍵面向，不遺漏高風險問題。

## 適用時機

- 審查任何 Pull Request
- 評估大型重構或架構遷移的風險
- 合併前的最終品質把關
- 技術債清理的影響評估

---

## Review 流程

### Phase 1：理解全貌

1. **讀 PR 描述**：理解動機（為什麼要改）、範圍（改了什麼）、設計決策（為什麼這樣改）
2. **看改動統計**：files changed、additions、deletions，判斷 PR 規模與風險等級
3. **識別改動分類**：
   - 新增（新模組、新抽象、新型別）
   - 重寫（核心邏輯替換）
   - 遷移（消費端適配新介面）
   - 刪除（移除舊實作）

### Phase 2：由底向上逐層檢查

底層的問題會向上傳播，因此從最底層開始審查：

1. 資料定義層（Schema、型別、資料模型）
2. 基礎設施層（連線管理、第三方整合）
3. 操作層（資料存取、CRUD）
4. 業務邏輯層（服務、排程、同步）
5. 介面層（API、IPC、事件）
6. 消費端（前端、呼叫方）
7. 測試（驗證以上所有層）

### Phase 3：撰寫 Review 報告

依照「風險分級」將發現的問題分類，輸出結構化的 review 結果。

---

## 檢查面向

### 1. 資料完整性（最高優先）

資料遺失是不可逆的，永遠第一個檢查。

- [ ] **原子性**：多步驟寫入操作是否有原子性保證？部分失敗時能否回滾？
- [ ] **關聯清理**：刪除父記錄時，子記錄的清理策略是否明確且正確？
- [ ] **升級相容**：舊版資料升級到新版時，是否有遺失或格式不相容的風險？
- [ ] **衝突處理**：upsert 或 merge 操作的衝突解決邏輯是否正確？是否遺漏必要欄位？
- [ ] **失敗狀態**：操作中途失敗後，系統會停在什麼狀態？是否可恢復？

### 2. 型別安全

- [ ] **定義與實作對齊**：資料定義與程式碼中使用的型別是否一致？nullable 欄位是否正確標注？
- [ ] **列舉值約束**：有限合法值的欄位是否有型別層級的約束？
- [ ] **寫入 vs 讀取型別區分**：寫入操作的型別是否排除了自動生成的欄位？
- [ ] **型別斷言濫用**：是否有不必要的強制轉型繞過了編譯器檢查？
- [ ] **泛型約束**：泛型參數是否有適當的約束，避免傳入不合法的型別？

### 3. 架構

- [ ] **模組邊界清晰**：每個模組/檔案是否只做一件事？對外介面是否最小化，不暴露內部細節？
- [ ] **依賴方向正確**：是否存在循環依賴？依賴是否只往單一方向流動（例如 UI → Store → Service）？
- [ ] **抽象層次一致**：同一層的程式碼是否在相同的抽象層次上操作？是否有層次跳躍（如 UI 直接操作 DB）？
- [ ] **封裝完整**：內部實作細節是否正確隱藏？消費端是否被迫知道不應知道的細節？
- [ ] **過度設計檢查**：新增的抽象、中間層、介面是否有明確存在理由？是否存在 YAGNI 違反？

### 4. 安全性

- [ ] **注入防護**：所有外部輸入是否透過參數化方式傳入？是否存在字串拼接風險？
- [ ] **動態識別符來源**：動態的 key、名稱、路徑是否來自可信來源？
- [ ] **副作用控制**：函式是否修改了傳入的參數物件？是否有非預期的 mutation？
- [ ] **敏感資料洩漏**：日誌或錯誤訊息是否暴露了敏感資訊？
- [ ] **權限邊界**：原有的保護機制被移除時，是否有替代防護？

### 5. 效能

- [ ] **N+1 問題**：是否有迴圈內的逐筆查詢？是否可改為批次操作？
- [ ] **索引覆蓋**：頻繁查詢的條件欄位是否有對應的索引？
- [ ] **全量掃描**：無條件的全量操作是否合理？資料量大時是否會成為瓶頸？
- [ ] **鎖定粒度**：事務範圍是否太大（長時間鎖定）或太小（無法保證一致性）？
- [ ] **記憶體使用**：大量資料是否有分批處理？是否會一次全部載入記憶體？

### 6. Breaking Change 偵測

大型重構最容易遺漏的問題。

- [ ] **刪除的公開介面**：被刪除的型別、函式、常數是否有 diff 外的消費者？需全專案搜尋確認。
- [ ] **命名慣例變更**：欄位或 key 的命名慣例變更是否在所有層一致？
- [ ] **函式簽名變更**：參數的新增或移除是否在所有呼叫端同步更新？
- [ ] **行為語義變更**：同步改非同步、回傳值型別變更、null 語義變更等，是否在所有消費端調整？
- [ ] **跨邊界介面**：跨進程、跨模組、跨服務的資料格式變更是否在收發兩端都已更新？

### 7. 測試覆蓋

- [ ] **正常路徑**：核心操作的正常流程是否有測試？
- [ ] **邊界情況**：空值、null、重複、極端輸入等是否有覆蓋？
- [ ] **錯誤路徑**：操作失敗時的回滾、錯誤訊息、狀態恢復是否有驗證？
- [ ] **升級路徑**：新建與從舊版升級兩條路徑都有驗證？
- [ ] **高風險路徑**：PR 中最複雜或最關鍵的邏輯是否有對應的測試？

### 8. 第三方依賴

- [ ] **版本穩定性**：是否依賴 pre-release 版本（alpha / beta / rc）？風險是否被記錄？
- [ ] **非公開 API 使用**：是否存取了第三方庫的私有屬性或未文件化的方法？升版時可能斷裂。
- [ ] **Lock file 合理性**：依賴鎖定檔的變更是否合理？是否有非預期的間接依賴升級？

### 9. 可讀性

- [ ] **命名表達意圖**：函式、變數、型別的命名是否清楚表達其目的，不需要讀內容才能理解？
- [ ] **函式長度合理**：單一函式是否可以不捲動螢幕閱讀完？複雜邏輯是否有適當的分割？
- [ ] **巢狀層次適當**：是否存在深度超過 3 層的條件巢狀？可否以 early return 或分割函式改善？
- [ ] **相同概念一致命名**：相同的業務概念在不同檔案中是否使用相同名稱？
- [ ] **非直覺決策有說明**：設計限制、特殊處理、workaround 是否有說明「為什麼」而非「做了什麼」？

### 10. 程式碼品質

- [ ] **職責單一**：每個函式或模組是否只做一件事？
- [ ] **命名一致性**：相同概念在不同檔案中是否使用相同名稱？
- [ ] **決策記錄**：非直覺的設計決策是否有註解說明「為什麼」？
- [ ] **錯誤上下文**：捕捉到的錯誤是否附帶足夠的上下文資訊（哪個操作、哪筆資料、什麼原因）？
- [ ] **Dead code**：是否有被註解掉或不再使用的程式碼殘留？

---

## 風險分級

每個發現的問題按以下標準分級：

| 等級 | 標準 | 處理方式 |
|------|------|----------|
| **高** | 可能導致資料遺失、安全漏洞、或 production 崩潰 | 必須在合併前修復 |
| **中** | 可能導致非預期行為、效能問題、或未來維護困難 | 強烈建議修復，可協商 |
| **低** | 程式碼品質、命名風格、缺少測試等改善建議 | 建議修復，不阻擋合併 |

---

## Review 輸出格式

**輸出的內容請保持 markdown 格式**

```markdown
# Code Review: [PR 標題]

**PR:** #number | **作者:** name | **規模:** X files, +Y / -Z

## 概述
[1-3 句話描述 PR 做了什麼、為什麼要做]

## 高風險問題
### 1. [問題標題]
[問題描述 + 建議修復方式]

## 中風險問題
### N. [問題標題]
[問題描述 + 建議]

## 低風險 / 改進建議
- [建議項目列表]

## 整體評價
[1-2 句話的總結評價與分數 (0 ~ 100 分)]
```

---

## 大型重構的額外檢查

當 PR 涉及架構遷移時，額外關注：

1. **升級路徑**：舊版到新版的轉換是否有自動化驗證？手動步驟是否有文件？
2. **Rollback 計畫**：部署後發現問題時，能否快速回退？
3. **雙寫期**：是否需要新舊系統並行運作的過渡期？
4. **開關控制**：是否應該用 feature flag 控制新舊路徑的切換？
5. **相容層生命週期**：為了向下相容而寫的程式碼，何時可以移除？是否有追蹤機制？

---

## 反模式清單

Review 時特別留意以下反模式：

| 反模式 | 為什麼有問題 | 應改為 |
|--------|-------------|--------|
| 強制轉型繞過型別檢查 | 隱藏編譯期錯誤，延遲到 runtime 才爆 | 修正型別定義，從根源解決 |
| 直接修改傳入的參數物件 | 副作用難以追蹤，呼叫端可能被影響 | 建立新物件回傳 |
| 空的錯誤捕捉（吞掉錯誤） | 問題被完全隱藏，無法診斷 | 至少記錄錯誤日誌 |
| 字串拼接組合查詢或命令 | 注入風險 | 使用參數化機制 |
| 存取第三方庫的私有屬性 | 依賴內部實作細節，升版時斷裂 | 使用公開 API |
| 多步驟寫入無原子性保證 | 部分失敗造成資料不一致 | 包在 transaction 或等效機制內 |
| null 與空字串混用 | 語義不明確，下游行為不可預測 | 統一慣例並嚴格遵守 |
| 刪除公開介面但未搜尋消費者 | diff 外的程式碼編譯或執行斷裂 | 合併前全專案搜尋確認無殘留引用 |