code-review-principles

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. 安全性

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

4. 效能

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

5. Breaking Change 偵測

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

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

6. 測試覆蓋

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

7. 第三方依賴

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

8. 程式碼品質

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

---

風險分級

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

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

---

Review 輸出格式

輸出的內容請保持 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 外的程式碼編譯或執行斷裂	合併前全專案搜尋確認無殘留引用