品質審查
目標:$ARGUMENTS
審查目標的設計與測試品質。報告發現;不編輯程式碼。
1. 載入評分標準
在查看程式碼前,先閱讀以下兩項:
~/.claude/CODING_STANDARDS.md是評分標準。每個發現都應引用其中一個章節標題。- design-vocabulary.md 包含用於表述發現的術語,以及下面用於驗證它們的測試。
如果存在,也閱讀儲存庫的 AGENTS.md / CLAUDE.md。如果這些檔案明確說明儲存庫故意採取某些標準不鼓勵的做法,請在報告中說明一次,而不是標記每一個實例。你只在程式碼中觀察到的一種習慣(例如每個模組都讀取環境變數、每個測試都進行 monkeypatch)並不算是記錄在案的選擇:遵循此習慣的新程式碼仍然是一個發現。
2. 解析目標
- 無參數:目前分支相對於其基礎分支。
git merge-base HEAD origin/main(或master),然後git diff <base>...HEAD加上來自git diff HEAD的未提交變更。 - PR 編號或 URL:
gh pr diff <n>和gh pr view <n> --json baseRefName,headRefName,files。 - 分支:
git diff <base>...<branch>。 - 路徑,或
.代表整個儲存庫:按現狀審查該路徑下的現有程式碼,不進行差異比對。這是程式碼庫模式,見下文。
3. 閱讀足夠內容以判斷設計
變更模式(差異、分支、PR)
僅靠差異無法判斷模組是否深層。對於每個變更的模組,閱讀整個檔案,並使用 grep 搜尋其呼叫者和測試。深度、所有權和重複是關於呼叫點的判斷,因此需要查看它們。
將發現範圍限定在變更引入或惡化的部分。如果差異觸及的程式碼中存在一個既有的問題,而變更是修復它的自然時機,則該問題屬於範圍內;如果問題存在於未觸及的程式碼中,則不屬於範圍。
新模組會獲得如同程式碼庫模式中的模組級審查:像其他章節一樣完整地走查型別和可讀性。對於每個新建立或大幅改寫的模組,在判斷之前列出每個函式及其參數和回傳型別,並注意它接受、回傳或讀取鍵的每個未指定型別的 map、dict 或 tuple,以及每個單字母名稱。
在判斷可讀性之前,使用該語言的常用工具測量每個變更過的原始檔案的循環複雜度:例如 Python 使用 radon cc -s、Go 使用 gocyclo、JavaScript/TypeScript 使用 eslint 的 complexity 規則、Rust 使用 clippy 的 cognitive_complexity、Java 使用 PMD。如果儲存庫有自己的工具則透過其執行,否則使用一次性執行器(uvx、npx、go run)。優先選擇能計算短路布林運算元的工具;有些工具(例如 ruff 的 C901)會跳過它們,從而低估了最糟的函式。如果沒有可用的工具,則按照評分標準的定義手動計算決策點。差異新增且分數高於 10 的函式,或是既有分數被推過 10 或進一步提高的函式,都是候選對象。對於既有函式,也需計算基礎版本的分數(將 git show <base>:<file> 寫入暫存檔案並測量),以便發現可以說明變更新增了什麼。
當差異新增超過約 150 行原始碼時,在你審查設計章節的同時,平行啟動兩個 general-purpose 子代理,每個子代理都獲得差異、變更檔案列表以及步驟 4 中各個章節的驗證規則:一個負責型別、可讀性和「傾向於帶來回報的不可變資料」,另一個負責測試,並將斷言檢查應用於每個新的測試。在保留其結果前,先驗證每個子代理回傳的內容。
程式碼庫模式(路徑或 .)
路徑下的所有內容都在範圍內,但發現的門檻會隨著範圍的大小而提高。單一模組或小型目錄(大致低於 1,500 行原始碼)會獲得模組級、函式級和命名級的發現:像其他章節一樣完整地走查型別和可讀性。一個套件或整個儲存庫只會得到重大問題:阻礙程式碼庫進步的設計決策和架構模式。當使用者命名一個功能或工具(例如「整個 CLI」)而其結果是一個模組時,同樣適用:以模組規模進行判斷。對於套件級或儲存庫級的每個候選對象,提出以下問題:
- 有多少模組、測試或未來變更需要為此付出代價?如果成本僅限於一個函式,在這個規模下這只是個小問題。
- 修復它會改變這個儲存庫中新程式碼的編寫方式嗎?如果修復僅是幾行本地程式碼且對呼叫者沒有影響,則放棄它。
- 這是一個模式還是一次性的?個別實例僅作為模式的證據。
重大問題的例子:一個沒有所屬模組的外部系統,因此每個呼叫者都重複進行認證和分頁;策略和編排融合在進入點中,因此決策只能透過端對端測試來驗證;狀態所有權分散在多個模組中,每個模組都知道儲存佈局;用戶端和配置在建置時構建,因此沒有真實環境就無法測試;一個測試套件因為沒有接縫而到處修補內部實作。一個長函式、一個命名不當的變數、一個單獨被吞沒的異常或一個弱斷言,本身都不是重大問題。
這些問題跨越模組,因此在深入閱讀檔案之前,先建立一個對應圖:
- 模組(檔案或套件)及其大致功能,透過
git ls-files <path>和快速瀏覽每個模組的頂部來取得 - 主要元件如何連接:什麼呼叫什麼,以及狀態和資料如何在進入點、模組和外部系統之間流動
- 每個外部系統(API、資料庫、佇列、檔案格式、模型)以及每個直接與之通訊的模組
- 每個讀取環境或配置的地方(
os.environ、getenv、process.env、旗標解析) - 模組級可變狀態和匯入時副作用(在模組範圍內建置的用戶端、連線、快取、鎖)
- 進入點以及測試目錄,每個測試檔案練習了哪些模組
大部分工作可以透過 grep 完成。保持對應圖簡短;它是一個工作索引,而不是報告的一部分。
如果範圍很小(大致低於 3,000 行原始碼),直接根據對應圖進行審查,並平行啟動兩個 general-purpose 子代理,每個子代理都獲得檔案列表和步驟 4 中各個章節的驗證規則。一個只獲得評分標準的型別、可讀性和「傾向於帶來回報的不可變資料」章節。另一個只獲得測試章節,並且必須將斷言檢查應用於每個測試。這樣這些章節就能得到充分的關注,而不是在前面的章節之後才處理剩下的部分。在保留其結果前,先驗證每個子代理回傳的內容。否則,將其分割成相關模組的區域,每個區域都包含其測試,並將每個區域交給一個平行啟動的 general-purpose 子代理。給予每個子代理評分標準路徑、詞彙路徑、對應圖、其檔案列表、步驟 4 驗證規則以及上面的重大問題門檻,並告訴它不要編輯任何東西。一個只看見自己區域的子代理會傾向於局部的小問題,因此要求它提供其區域中看到的設計模式以及每個模式的證據(檔案:行號),而不是一行一行的發現列表。跨模組的問題來自對應圖,因此你需要自己處理這些,然後將它們與子代理的模式結合,合併那些來自不同區域但屬於相同底層決策的模式,並在報告前重新檢查每個證據。
4. 根據每個章節進行審查
按順序走查標準:模組、狀態、型別、依賴與配置、錯誤、可讀性、測試。對於每個候選發現,在保留前進行驗證:
- 淺層模組或傳遞型模組:應用刪除測試。說明呼叫者以及哪些複雜度會重新出現在它們身上,或證明不會有任何影響。
- 尚未證明其價值的抽象層:計算接縫處的配接器數量。一個測試偽物件算一個。只有一個配接器且背後沒有隱藏任何東西,這是一個發現;兩個則是一個真正的接縫。
- 缺少抽象層:同樣嚴格地檢查相反方向。尋找一個在三個或更多位置中內聯處理的關注點,無論是在一個模組內還是跨模組。
- 缺少所有者 / 重複的關注點:列出每個撰寫該關注點的位置(檔案:行號),而不是「這在多處出現」。
- 沒有所有者的概念:列出每個表示、解析或建構它的位置(檔案:行號),以及任何簽名中具有不同意義的同型別參數並列的情況。識別符號也算作概念,而不僅僅是結構化資料。不同種類的名稱和 ID 作為相同的基本型別傳遞(來源和目標儲存庫名稱、分支名稱和 ref、兩種 ID)是主要需要捕捉的情況。然後說明哪種所有者合適。如果呼叫者持有並傳遞該概念,建議為其建立一個型別。如果他們僅需要用它來做某事,則建議將其隱藏在執行該工作的模組內部。不要預設使用領域型別。
- 排序約束或手動建置的鍵/路徑/ID:顯示呼叫者進行排序或建構的過程。
- 共享的可變狀態、載入時副作用、深層配置讀取:命名全域變數或匯入時呼叫,以及被其阻擋的測試或第二個配置。
- 快照寫入:顯示讀取、長時間執行的步驟以及回寫。
- 可變資料但最好改為不可變:僅在可變性有具體成本時才報告。顯示第二個持有者看到意外的變更、使其不安全的快取或鍵,或在遠離值建立位置進行的變更。一個由單一所有者透過其自身程式碼進行變更的可變結構,不是一個發現。如果不可變版本會影響效能,請說明要記憶化或共享的內容,以及該快取位於何處。
- 可以表示的無效狀態:命名永遠不應發生的狀態,並顯示允許它的型別(必須一起設定的可選欄位、布林值的組合、具有少數允許值的字串)。列出僅因為它而存在的防衛式檢查或
None檢查(檔案:行號)。提出使其不可能發生的形狀,並說明該變更會刪除哪些防衛式檢查。 - 作為概念的 Dict 或 Tuple:列出讀取的鍵以及每個讀取它們的函式(檔案:行號),以及每個建構該 dict 的位置。使用儲存庫現有的風格(pydantic 模型或 dataclass、TypeScript 介面、Go struct,無論儲存庫已使用什麼)提出記錄型別,僅包含程式碼使用的欄位,並說明載荷在何處被解析成該型別。
- 擁有外部系統的模組:檢查其函式是否透過模組全域變數或每次呼叫的環境讀取來共享憑證、身分、基礎 URL 或快取,以及測試是否修補其多個函式。如果是,建議建立一個具有領域命名方法的用戶端類別,在建置時建立並傳入,並說明偽物件將取代哪些修補。
- 缺少文件註解:列出每個沒有文件註解的非平凡函式(檔案:行號),以及其文件註解需要說明而簽名未說明的內容(回退、錯誤模式、觸及的狀態、輸入形狀)。每個模組將它們分組為一個可讀性發現。
- 單字母名稱:將它們報告為一個分組的可讀性發現,列出每個名稱(檔案:行號)並提出建議名稱。它們計入可讀性上限,作為一個發現,絕不能作為放棄它們的理由。
- 被吞沒的錯誤:顯示從失敗到結果變化的路徑,而僅有一行日誌記錄它。
- 具有多種含義的名稱:對其進行 grep,並列出它在每個位置上持有的內容。
- 隱藏其層級或物件的名稱:顯示呼叫鏈或呼叫點,其中它讀起來不明確,並提出替換名稱。
- 執行多個操作的函式:嘗試撰寫其一句話摘要。如果無法做到,引用阻止你的分支、try 區塊或模式參數。
- 過於複雜的函式:對於來自複雜度測量的每個候選函式,給出其分數,然後按順序列出它執行的階段(過濾輸入、擷取、索取、驗證、建置、建立、記錄、通知……),包括它們的行號範圍以及每個階段的失敗規則。提出將拆分為具名函式,每個階段或決策一個,附上簽名,並說明每個擷取出來的部分如何在不修補的情況下可測試;通常決策會變成純函式,而原始程式碼則讀起來像是簡短的具名步驟序列。每個函式一個發現。高於 20 分時,它與設計發現同級;11 到 20 分是一個不計入可讀性上限的可讀性發現。不要因為該函式是進入點或儲存庫中有更大的函式而放棄拆分。
- 測試斷言:命名會使斷言失敗的產品錯誤。如果無法做到,那就是發現。對於參數化或表格驅動的測試,追蹤每個輸入通過程式碼的過程,並說明輸入的哪一部分觸發了測試案例所命名的行為。一個其區分性輸入被較早分支消耗的案例,即使存在針對它的測試,也不涵蓋該行為。同樣,檢查測試夾具或偽物件是否確實能產生會觸發每個防衛式檢查或拒絕路徑的輸入。如果永遠不能,則該防衛式檢查未被測試。
- 測試替身:說明測試的內容,然後說明替身是否位於該邊界之外或重新實作了產品邏輯。
- 修補內部實作的測試:指出應該被提取出來的決策或應該被傳入的依賴項,因為這才是真正的修復。
- 在任何發現的修復中提出的接縫或埠:賦予其在領域中命名的操作(
close_ticket(id)),而不是傳輸動詞(get/post/put)。一個僅轉發請求的埠無法通過刪除測試,並且其偽物件將不得不重新實作路由。在報告前,根據刪除測試檢查每個提出的接縫。
給予測試與前面章節相同的深度,即使前面的章節產生了許多發現。針對每個測試檔案執行上述每個測試檢查:對於每個斷言,命名會使其失敗的產品錯誤。一個安全相關的行為沒有失敗斷言(脫敏、認證檢查、核准關卡)與設計發現同級。
刪除任何你無法具體化的內容。刪除標準未涵蓋的風格問題。除非它們直接源自品質問題(例如,遺失資料的快照寫入),否則不要在此處報告正確性錯誤;用一行提及它們,並指向 /code-review。
如果這是針對 pr/issue 審查的工作流程,請進行後續操作。