code-review-and-quality

程式碼審查與品質

概述

多維度程式碼審查，附帶品質門檻。每次變更在合併前都必須經過審查——沒有例外。審查涵蓋五個軸向：正確性、可讀性、架構、安全性與效能。

核准標準： 當變更明顯改善整體程式碼健康度時就核准，即使它不完美。完美的程式碼不存在——目標是持續改善。不要因為變更不是你寫的方式就阻擋它。如果它改善了程式碼庫並遵循專案慣例，就核准。

使用時機

• 合併任何 PR 或變更之前
• 完成功能實作之後
• 當另一個代理或模型產生的程式碼需要評估時
• 重構現有程式碼時
• 任何錯誤修正之後（同時審查修正與回歸測試）

五軸向審查

每次審查都從這些維度評估程式碼：

1. 正確性

程式碼是否做到它聲稱的事？

• 是否符合規格或任務需求？
• 是否處理邊界情況（null、空值、邊界值）？
• 是否處理錯誤路徑（不只是快樂路徑）？
• 是否通過所有測試？測試是否真的測試了正確的事？
• 是否有差一錯誤、競爭條件或狀態不一致？

2. 可讀性與簡潔性

另一位工程師（或代理）能否在沒有作者解釋的情況下理解這段程式碼？

• 命名是否具描述性且符合專案慣例？（不要無上下文的 temp、data、result）
• 控制流程是否直接明瞭（避免巢狀三元運算子、深層回呼）？
• 程式碼組織是否合乎邏輯（相關程式碼分組、清晰的模組邊界）？
• 是否有任何「聰明」的技巧應該簡化？
• 能否用更少的行數完成？（1000 行但 100 行就夠是失敗）
• 抽象化是否值得其複雜度？（在第三個使用案例之前不要泛化）
• 註解是否有助於釐清不明顯的意圖？（但不要註解明顯的程式碼。）
• 是否有死程式碼：無作用的變數（_unused）、向後相容的墊片、或 // removed 註解？
• 是否將新的條件式硬塞到無關的流程中？ 這是設計氣味，不是小問題——將邏輯推入自己的輔助函式、狀態或策略，而不是纏繞現有路徑。
• 是否出現對相同形狀的重複條件判斷？ 這表示缺少模型或調度器。「暫時」的分支通常是永久的技術債。

3. 架構

變更是否符合系統設計？

• 是否遵循現有模式或引入新模式？如果是新的，是否有正當理由？
• 是否保持乾淨的模組邊界？
• 是否有應該共用的重複程式碼？
• 相依性是否流向正確方向（無循環相依）？
• 抽象層級是否適當（不過度設計，也不過度耦合）？
• 這個重構是降低複雜度還是只是搬移複雜度？ 計算讀者為了理解變更必須掌握的概念數量。如果「更乾淨」的版本沒有改變這個數量，那它並不更乾淨——優先選擇讓整個分支、模式或層級消失的重構，而不是重新集中相同邏輯的重構。優先選擇刪除抽象化而不是打磨它。
• 功能特定邏輯是否洩漏到共用或通用模組？ 將邏輯保留在其所屬層級，重複使用現有的標準輔助函式而不是近似副本，不要讓架構偏移常態化。
• 型別邊界是否明確？ 質疑隨意的 any/unknown/可選/型別轉換，以及掩蓋不明確不變量的靜默降級——讓邊界明確通常會讓周圍的控制流程更簡單。

4. 安全性

詳細安全指引請參閱 security-and-hardening。變更是否引入漏洞？

• 使用者輸入是否經過驗證與清理？
• 機密是否排除在程式碼、日誌與版本控制之外？
• 需要的地方是否檢查了身分驗證/授權？
• SQL 查詢是否使用參數化（無字串串接）？
• 輸出是否編碼以防止 XSS？
• 相依性是否來自可信來源且無已知漏洞？
• 來自外部來源（API、日誌、使用者內容、設定檔）的資料是否視為不受信任？
• 外部資料流是否在系統邊界進行驗證後才用於邏輯或渲染？

5. 效能

詳細效能分析與最佳化請參閱 performance-optimization。變更是否引入效能問題？

• 是否有 N+1 查詢模式？
• 是否有無界迴圈或無限制的資料擷取？
• 是否有應該非同步的同步操作？
• UI 元件中是否有不必要的重新渲染？
• 列表端點是否缺少分頁？
• 熱路徑中是否建立了大型物件？

結構性補救措施

當你標記結構性問題時，提出具體的移動方向——而不只是問題。只說「這很複雜」的審查讓作者無所適從。考慮命名的重構方式：

• 用型別模型或明確的調度器取代條件鏈
• 將重複分支合併為單一更清晰的流程
• 將編排與商業邏輯分離，讓各自獨立可讀
• 將功能特定邏輯移出共用模組，放入擁有該概念的套件
• 重複使用標準輔助函式，而不是自製的近似副本
• 讓型別邊界明確，使下游分支消失
• 刪除傳遞包裝器，它增加了間接性卻沒有釐清 API
• 提取輔助函式，或將大型檔案拆分為聚焦的模組

優先選擇移除活動部件的補救措施，而不是將相同複雜度分散各處。

變更規模

小而聚焦的變更容易審查、更快合併、更安全部署。目標規模如下：

~100 行變更   → 良好。一次審查即可完成。
~300 行變更   → 可接受，如果是單一邏輯變更。
~1000 行變更  → 太大。請拆分。

注意檔案大小，而不只是 diff 大小。 小的 diff 仍可能將檔案推過健康邊界——單一檔案總行數約 1000 行（不同於上述 ~1000 行變更的門檻）是常見的檢查信號，不是硬性上限。當變更明顯增大已很大的檔案時，詢問是否應先提取輔助函式、子元件或模組，然後再添加更多內容。先分解，再添加。

什麼算「一個變更」： 一個自包含的修改，處理一件事，包含相關測試，並在提交後保持系統功能正常。功能的一部分——不是整個功能。

變更太大時的拆分策略：

策略	方式	時機
堆疊	提交一個小變更，基於它開始下一個	順序相依
按檔案群組	為需要不同審查者的群組分開變更	跨領域關注點
水平	先建立共用程式碼/樁，然後消費者	分層架構
垂直	將功能分解為較小的全端切片	功能開發

何時大型變更可接受： 完整的檔案刪除與自動化重構，審查者只需驗證意圖，而非每一行。

將重構與功能開發分開。 同時重構現有程式碼與新增行為的變更是兩個變更——分開提交。小型清理（變數重新命名）可由審查者酌情包含。

變更描述

每個變更需要一個在版本控制歷史中獨立的描述。

第一行： 簡短、祈使句、獨立。「Delete the FizzBuzz RPC」而不是「Deleting the FizzBuzz RPC.」必須足夠資訊性，讓搜尋歷史的人無需閱讀 diff 就能理解變更。

內文： 變更什麼以及為什麼。包含程式碼中不明顯的背景、決策與理由。連結到錯誤編號、基準測試結果或設計文件。當存在方法缺點時承認它們。

反模式：「Fix bug」、「Fix build」、「Add patch」、「Moving code from A to B」、「Phase 1」、「Add convenience functions.」

審查流程

步驟 1：了解背景

在查看程式碼之前，先了解意圖：

- 這個變更試圖達成什麼？
- 它實作了哪個規格或任務？
- 預期的行為變更是什麼？

步驟 2：先審查測試

測試揭示意圖與涵蓋範圍：

- 變更是否有對應的測試？
- 測試是否測試行為（而非實作細節）？
- 邊界情況是否涵蓋？
- 測試是否有描述性的名稱？
- 如果程式碼改變，測試是否能捕捉回歸？

步驟 3：審查實作

帶著五個軸向瀏覽程式碼：

對於每個變更的檔案：
1. 正確性：這段程式碼是否做了測試說它該做的事？
2. 可讀性：我能否在沒有幫助的情況下理解？
3. 架構：這是否符合系統？
4. 安全性：是否有任何漏洞？
5. 效能：是否有任何瓶頸？

步驟 4：分類發現

為每個評論標記嚴重性，讓作者知道哪些是必須處理的，哪些是可選的：

前綴	意義	作者行動
(無前綴)	必須修改	合併前必須處理
Critical:	阻擋合併	安全漏洞、資料遺失、功能損壞
Nit:	次要、可選	作者可忽略——格式、風格偏好
Optional: / Consider:	建議	值得考慮但不強制
FYI	僅供參考	無需行動——未來參考的背景

這可防止作者將所有回饋視為強制，浪費時間在可選建議上。

從重要的事開始。 按影響力排序發現：正確性與安全性優先，然後是結構性回歸與遺漏的簡化，最後才是其他事項。不要把真正的問題埋在格式小問題下——少數高信心的評論勝過長串列表。如果你有一個結構性問題和十個小問題，結構性問題就是審查的重點。

步驟 5：驗證驗證方式

檢查作者的驗證故事：

- 執行了哪些測試？
- 建置是否通過？
- 變更是否經過手動測試？
- UI 變更是否有截圖？
- 是否有前後比較？

多模型審查模式

使用不同模型進行不同審查視角：

模型 A 撰寫程式碼
    │
    ▼
模型 B 審查正確性與架構
    │
    ▼
模型 A 處理回饋
    │
    ▼
人類做出最終決定

這能捕捉單一模型可能遺漏的問題——不同模型有不同的盲點。

審查代理的範例提示：

Review this code change for correctness, security, and adherence to
our project conventions. The spec says [X]. The change should [Y].
Flag any issues as Critical, Required, Optional, or Nit.

死程式碼衛生

在任何重構或實作變更後，檢查孤兒程式碼：

1. 識別現在無法到達或未使用的程式碼
2. 明確列出它們
3. 刪除前先詢問：「我應該移除這些現在未使用的元素嗎：[列表]？」

不要讓死程式碼留著——它會混淆未來的讀者與代理。但也不要默默刪除你不確定的東西。有疑問時，先問。

DEAD CODE IDENTIFIED:
- formatLegacyDate() in src/utils/date.ts — replaced by formatDate()
- OldTaskCard component in src/components/ — replaced by TaskCard
- LEGACY_API_URL constant in src/config.ts — no remaining references
→ Safe to remove these?

審查速度

緩慢的審查會阻礙整個團隊。切換上下文進行審查的成本低於強加給他人的等待成本。

• 一個工作日內回應——這是上限，不是目標
• 理想節奏： 收到審查請求後不久就回應，除非正在深度專注編碼。一個典型的變更應在一天內完成多輪審查
• 優先快速個別回應，而非快速最終核准。快速回饋能減少挫折感，即使需要多輪審查
• 大型變更： 要求作者拆分，而不是審查一個巨大的變更集

處理分歧

解決審查爭議時，應用此層級：

1. 技術事實與數據優先於意見與偏好
2. 風格指南是風格事項的絕對權威
3. 軟體設計必須基於工程原則評估，而非個人偏好
4. 程式碼庫一致性在不會降低整體健康度的情況下可接受

不要接受「我之後再清理。」 經驗顯示延遲清理很少發生。要求在提交前清理，除非是真正的緊急情況。如果周圍的問題無法在此變更中處理，要求提交一個自我指派的錯誤。

審查中的誠實

審查程式碼時——無論是由你、另一個代理或人類撰寫：

• 不要橡皮圖章。 沒有審查證據的「LGTM」對任何人都沒有幫助。
• 不要軟化真正的問題。 當這是一個會影響生產環境的錯誤時說「這可能是小問題」是不誠實的。
• 盡可能量化問題。「這個 N+1 查詢會為列表中的每個項目增加約 50ms」比「這可能會慢」更好。
• 對有明顯問題的方法提出異議。 奉承是審查中的失敗模式。如果實作有問題，直接說出來並提出替代方案。
• 優雅地接受覆寫。 如果作者有完整背景並不同意，尊重他們的判斷。評論程式碼，而不是人——將個人批評重新框架為關注程式碼本身。

相依性紀律

程式碼審查的一部分是相依性審查：

添加任何相依性之前：

1. 現有技術棧是否已解決這個問題？（通常是的。）
2. 相依性有多大？（檢查套件影響。）
3. 是否積極維護？（檢查最後提交、開放問題。）
4. 是否有已知漏洞？（npm audit）
5. 授權條款是什麼？（必須與專案相容。）

規則： 優先使用標準函式庫與現有工具，而非新相依性。每個相依性都是一個負債。

審查檢查清單

## Review: [PR/Change title]

### Context
- [ ] I understand what this change does and why

### Correctness
- [ ] Change matches spec/task requirements
- [ ] Edge cases handled
- [ ] Error paths handled
- [ ] Tests cover the change adequately

### Readability
- [ ] Names are clear and consistent
- [ ] Logic is straightforward
- [ ] No unnecessary complexity

### Architecture
- [ ] Follows existing patterns
- [ ] No unnecessary coupling or dependencies
- [ ] Appropriate abstraction level
- [ ] Refactors reduce complexity rather than relocate it
- [ ] No feature logic in shared modules; file stays within a healthy size

### Security
- [ ] No secrets in code
- [ ] Input validated at boundaries
- [ ] No injection vulnerabilities
- [ ] Auth checks in place
- [ ] External data sources treated as untrusted

### Performance
- [ ] No N+1 patterns
- [ ] No unbounded operations
- [ ] Pagination on list endpoints

### Verification
- [ ] Tests pass
- [ ] Build succeeds
- [ ] Manual verification done (if applicable)

### Verdict
- [ ] **Approve** — Ready to merge
- [ ] **Request changes** — Issues must be addressed

另請參閱

• 詳細安全審查指引，請參閱 references/security-checklist.md
• 效能審查檢查，請參閱 references/performance-checklist.md

常見合理化藉口

合理化	現實
「它能運作，這樣就夠好了」	能運作但不可讀、不安全或架構錯誤的程式碼會產生複利的技術債。
「我寫的，所以我知道它是對的」	作者對自己的假設有盲點。每個變更都需要另一雙眼睛。
「我們之後再清理」	之後永遠不會來。審查就是品質門檻——好好利用它。要求在合併前清理，而不是之後。
「AI 生成的程式碼大概沒問題」	AI 程式碼需要更多審查，而不是更少。它自信且看似合理，即使錯了也是如此。
「測試通過了，所以沒問題」	測試是必要但不充分的。它們無法捕捉架構問題、安全問題或可讀性問題。
「這個重構讓它更乾淨」	搬移複雜度不是降低複雜度。如果讀者仍然需要掌握相同數量的概念，結構就沒有改善——尋找讓分支消失的版本。
「只是對這個檔案的小新增」	小的 diff 仍然會將檔案推過健康大小，並將分支硬塞到無關的流程中。判斷最終的結構，而不是 diff 大小。

紅旗

• PR 未經任何審查就合併
• 只檢查測試是否通過的審查（忽略其他軸向）
• 沒有實際審查證據的「LGTM」
• 安全敏感變更沒有安全重點審查
• 大 PR 被認為「太大無法妥善審查」（請拆分）
• 錯誤修正 PR 沒有回歸測試
• 審查評論沒有嚴重性標籤——不清楚哪些是必須的，哪些是可選的
• 接受「我之後再修」——它永遠不會發生
• 重構只是搬移程式碼，沒有減少讀者必須掌握的概念數量
• 變更讓已很大的檔案變得更大，而不是分解它
• 新的條件式散落在無關的程式碼路徑中（缺少抽象化）
• 自製的輔助函式重複了現有的標準輔助函式，或功能邏輯被放在共用模組中

驗證

審查完成後：

• [ ] 所有 Critical 問題已解決
• [ ] 所有 Required（無前綴）變更已解決或明確延後並附上理由
• [ ] 測試通過
• [ ] 建置成功
• [ ] 驗證故事已記錄（變更了什麼、如何驗證）

推定阻擋者： 針對以下每一項提出更簡單的設計；僅當變更積極惡化結構時才升級為 Required：重構搬移複雜度而非降低它；變更將檔案推過大小邊界且未分解；功能邏輯添加到共用模組；現有標準輔助函式的近似副本；隱藏不明確不變量的靜默降級。