我不明白自己為什麼寫不出有厚度的 review

明明看的是同一個 PR，自己只會在最後給個「LGTM」，旁邊的人卻能提出 10 件精準的指正。想問他到底看了什麼，得到的回答卻只是「我只是看自己在意的地方而已」。你也有過同樣的經驗嗎。

我想，那就只能實際數數看了，於是我用 GitHub API 把那個人的 inline comment 全部抓出來。涵蓋 3 個 repository、44 個 PR 的187 則。我把全部讀過一遍，並依照「在談什麼主題」來分類。

先說結論，最大的勢力不是 bug 指摘。反而是對「目前沒有壞掉的地方」的指摘，占了整體的三分之一，而我完全沒有碰到的，也正是那一塊。老實說，真的有點受打擊。

蒐集方式

GitHub 的 REST API 有一個可以一次取得整個 repository review comment 的端點。因為比一個 PR 一個 PR 跑更快，所以我用了這個。

gh api "repos/{owner}/{repo}/pulls/comments?per_page=100" --paginate \
  | jq -c '.[] | select(.user.login=="目標使用者名稱")'

這樣拿到的 212 則裡，扣掉 thread 內的 21 則回覆，以及同一個 review 被多次送出、本文完全一致的 4 則後，剩下的187 則成為分析對象。

我建立了 3 個分類軸：

1. 型別 — 指的是什麼（條件錯誤／dead code／不對稱……）
2. 動作 — 為了找出來，打開了什麼（同一個 repository 的其他檔案／呼叫端／官方文件……）
3. 領域 — 到底把注意力放在哪裡

這篇文章要談的是第 3 個「領域」。第 1、2 個軸是「怎麼找」的話題，但 review 卡住的原因，通常不是「不知道怎麼找」，而是「注意力根本沒朝那裡去」，所以我認為領域在實務上更有幫助。

11 個領域的分布

187 則分成 11 個大領域（再細分成 37 個中領域）。依照件數多寡排序如下。

排名領域這個領域在問什麼件數1位和旁邊的程式碼有一致嗎跟做同一件事的地方有沒有不一致312位契約與意圖有被保留下來嗎半年後的人能不能得出同樣的判斷303位值本身正確嗎這個數字、這個判定對不對254位有注意到失敗嗎壞掉的時候人能不能知道175位從使用者角度看如何營運人員和使用者手上會發生什麼166位與外部的邊界自己沒寫的東西真的會那樣運作嗎157位資料會不會壞掉當它當掉、排序、消失時還剩下什麼137位設計是否清晰下一個要接手的人讀得懂嗎139位速度與成本要花幾秒、幾塊錢1210位變更會影響到哪裡自己寫的人看到的畫面之外也會受影響嗎1111位能不能驗證之後能不能確認它真的生效了4前兩名加起來就有 61 則，占整體 33%。另一方面，通常所謂的 bug 相當項目（第 3 名「值本身」＋第 7 名「資料會不會壞掉」）共有 38 則，占 20%。每 5 則只有 1 則。

Layer 變了，領域也會跟著換

同一個人的 review，對象是後端、行動端還是 Web 前端時，厚重的領域明顯不同。

後端在「值是否正確」上特別突出，占 20%（金額、時間、DB constraint）。行動端偏向「資料會不會壞掉」「變更波及範圍」「外部 SDK」。Web 前端則是「一致性」與「意圖的保存」就佔了一半，幾乎沒有「與外部的邊界」和「能不能驗證」。

不需要每次都把全部領域掃過一遍，只要能依照差分所在的 layer 來縮小範圍就夠了。光是知道這件事，我就輕鬆很多了。

各領域實際上在指什麼

接下來是重點。每個領域我都列出 2 個實例。案例都已對外一般化，但指摘的結構完全相同。

第 1 位 和旁邊的程式碼有一致嗎（31 則）

最大勢力。而且這 31 則裡有 27 則（87%）是因為「打開了 repository 內的其他地方」才出現的。

總計與明細的邏輯不一樣。 總分是用 Math.abs(quantity) 加上異常值 fallback 0 去彙總，但每一列的顯示卻沒有 Math.abs，而且 fallback 是 1。一般資料下會對得上，但如果是退貨等導致數量變成負數，就會出現「每列加起來對不起來總計」的畫面。因為「總計」和「明細」一定是一對，所以只要兩邊都打開看，就會發現。

別名在正式環境根本沒被參照。 把共通處理抽到另一個 module 的 refactor 中，原本的 class 留了 5 個委派用別名。但搬過去的實作裡部份直接參照模組全域變數，所以那些別名在正式程式碼中完全沒有被呼叫。「就算在測試裡 @patch 也不會被替換，最後呼叫到的是實物」「即使改掉 SQL 常數，實際執行的 SQL 也不會變」就是這種狀態。明明是行為不變的 refactor，結果只讓測試的接縫悄悄變成假的。

第 2 位 契約與意圖有被保留下來嗎（30 則）

我原本完全沒想到這會排第二。這不是 bug，也不是設計缺陷，而是防止「為什麼這樣寫」消失的指摘。

魔術數字的根據。 CSS 裡有一個 calc(50% + 3.5rem)，review 端先自己推算出「親層寬度 6rem + gap 1rem = 7rem 的一半往右偏移」，再接著說：「意圖本身很好，但如果之後親層寬度或 gap 改了，這裡也必須一起改，建議加個說明註解。」先確認結果正確，再指出依賴關係不明顯的問題。

用 git 歷史驗證『已移轉』的說法。 在把相機設定移到新 class 的 PR 裡，甚至回溯到舊實作的 commit 去證明設定值的指定沒有被延續。另一個 PR 則是針對 PR 說明中寫的「問題原因就是這個」這種因果關係，直接追溯當時的 commit 來反駁。PR 說明是主張，不是事實。

第 3 位 值本身正確嗎（25 則）

數字只要差 1，餘額或彙總值就會悄悄錯掉。而且測試通常還會通過，因為測試也常常帶著同樣的先入為主。

把單價和總價搞混。 清單中的金額從 price * quantity 被改成只顯示 price，數量乘算消失了。確認 API 實作後可知 price 是含稅的「單價」，總價應該乘上數量算出來。甚至還實際確認了同一筆交易在畫面上變成清單 550 日圓、詳細 230 日圓不一致，才提出指摘。

到期判定會因為 DB 設定而差 9 小時。 有一段判定有效期限的 SQL 如下：

-- expires_at 是 timestamp without time zone（放的是 UTC 的值）
SELECT * FROM items WHERE expires_at < NOW();

expires_at 是沒有時區的型別，裡面的值是「以 UTC 為準的時間」。另一方面，NOW() 會回傳帶時區的值。因為型別不同，PostgreSQL 會先把其中一邊轉型再比較，而這時沒有時區的值會被解讀成「session 的 TimeZone 設定所代表的本地時間」（PostgreSQL 日期/時間資料型別 §8.5.1.3）。

也就是說，如果 TimeZone 設成 Asia/Tokyo，原本以 UTC 存進去的值會被當成 JST 讀取，結果會比實際時間早 9 小時被判定為「已過期」。如果這發生在金錢或點數的有效期限上，原本還活著的東西就會被消失。

有趣的是，這個指摘會先承認「現在確實是正常運作」，然後再說明為什麼現在會正常運作。原因只是 DB container 的 TimeZone 剛好沒指定，所以是 UTC。真正影響的是 DB session 的設定，不是 app 端的時區，因此只要 DB 啟動參數一變就會壞。接著再提議改成不依賴設定的寫法。

-- 兩邊都變成不帶時區的 UTC，因此不受 session 設定影響
SELECT * FROM items WHERE expires_at < NOW() AT TIME ZONE 'utc';

第 4 位 有注意到失敗嗎（17 則）

功能上明明正常，卻沒有人知道它其實已經壞了。這個獨立領域竟然有 17 則。

HTTP client 的設定讓 catch 根本不會觸發。 aspida 的 client 初始化時用了 throwHttpErrors: false（預設值），所以 await 的 post 就算收到 400/500 也不會丟出例外。結果即使包在 try/catch 裡，catch 區塊也不會進去，API 拒絕時還是會顯示「已變更」的成功 banner。設定檔的一行，直接推翻了「有 try/catch 就安全」的直覺。

明明有很親切的錯誤訊息，卻在中途被吃掉。 新增了一個像「可能已經註冊過同一個對象」這樣友善的 exception message，但負責接住它的 decorator 沒有把它列入捕捉清單，所以最後掉進上層的 except Exception，被換成「發生錯誤，請聯絡管理員。」。看到這種友善訊息，就要一路追查它是否真的能到畫面上，這也是很固定的檢查方式。

第 5 位 從使用者角度看如何（16 則）

從程式碼角度正確，但坐在畫面前的人會很困擾的領域。

重導向時只有篩選條件掉了。 在管理介面先篩選某個使用者再操作時，回到上一頁的 URL 組裝時沒有把使用者 ID 帶回去，結果回到了「已經取消篩選」的清單，卻保留了相同的頁碼。只保留頁碼，前一刻看的那一列和現在看到的列可能完全不同。講到這個程度，UX 問題就變成誤操作風險了。

遮罩層沒有完全蓋住。 處理中的 loading overlay 放在容器內部，沒有遮住 footer 的按鈕。雖然重複執行已經靠另一個 flag 擋住，但處理中的資料仍然留下了可刪除的路徑。

第 6 位 與外部的邊界（15 則）

自己沒寫的東西（SDK、OS API、CLI、framework），真的會那樣運作嗎。

把 setter 當成 adder 在呼叫。 ML Kit 的條碼掃描器會透過 Builder 指定格式，而 setBarcodeFormats(int format, int... formats) 官方文件寫著：如果多次呼叫這個方法，只有最後一次會生效。結果用 forEach 一個個傳進去時，最後只剩 1 個格式。而且還一路追到呼叫端的常數定義，連哪些卡片會讀不到都找出來了。

Content-Type 不對，body 就讀不到。 Flask 的 request.get_json(silent=True)，如果 request 的 mimetype 沒有表示是 JSON，不管 body 裡面是什麼，都會回傳 None（Flask API 參考）。對方即使送來的是正確 JSON，只要 header 沒帶對，也會被擋下來。這種很容易在外部串接當天踩到。

另外，沒加 silent 時的行為會隨版本而變：Flask 2.1 會回 400，2.3 則會回 415。silent=True 這邊則一直都是 None。這類「版本有變／沒變」的地方，這位 review 還會先看 lock file 確認實際版本再去查，這正是指摘有力的地方。

第 7 位 資料會不會壞掉（13 則）

正常情況下一定看不出來，只有在故障、同時執行、畫面切換時才會冒出來的領域。

一旦離開中介表就會孤兒化。 剛建立使用者時還能查到，但之後如果所屬表的 row 被刪掉，取得函式的任何分支都查不到。record 還在，卻變成沒有人能參照的狀態。

DB constraint 並沒有保證 app 的前提。 對 GROUP BY user_id 後取 MIN(company_id) 的實作，有人指出：「DB constraint 並沒有強制 1 個使用者只能對應 1 間公司（其中一邊是公司層級 unique，另一邊是設施層級 unique），所以一旦建立出多重隸屬的 row，畫面就只會看到其中一邊。」也就是說，連 schema 定義都打開來確認 app 暗中假設的 cardinality 是否成立。

第 7 位 設計是否清晰（13 則）

所謂「很像 code review 的 code review」。雖然同率第 7，但只占整體 7%。

對可能回傳 null 的值做型別斷言。 會回傳 null 的函式，其回傳值卻連續 3 次寫了 as string。因為外層已經檢查過存在與否，所以把變數先存起來即可，不需要型別斷言，這就是 3 行的改寫建議。

用事故說明型別沒有發揮作用。 在 TanStack Query（以 v5 系列確認）中，將自訂 key 傳給 query 的 meta，但因為沒有在 Register interface 中宣告 queryMeta，meta 的型別仍然是 Record<string, unknown>。也就是說，什麼 key 名都能通過，key 打錯了也不會被 compile 抓到。把它說成「型別太寬鬆」不如說成「typo 會通過」，問題就更清楚了。

第 9 位 速度與成本（12 則）

這個領域的指摘幾乎都有實測數字。不是「看起來很重」，而是直接講數字。

索引與呼叫頻率。 對新加入的搜尋 query，打開 model 定義裡的 index 宣告，確認「對應的 index 不存在」。接著再指出：「同樣的 query 在別處也有，但那裡只會執行一次；這裡是再進入路徑，而 session 有效期限是 365 天，所以每次存取都會經過這裡。」真正有力的不是有沒有 index，而是呼叫頻率的差異被講清楚了。

timeout 要和基礎設施加總。 在把 app 端 timeout 設成 40 秒的差分裡，還拿出 production load balancer 的實測值（近 7 天平均 0.4 秒多、p99 3 秒多、最大 18 秒），再說「如果這裡用掉 40 秒，總和會逼近 1 分鐘，接近 ALB 的 idle_timeout 60 秒」。這種會去查外層 timeout 再加總的思維，很關鍵。

第 10 位 變更會影響到哪裡（11 則）

這不是 bug，而是「超出預期範圍」的指摘。就算 PR 說明是對的，也完全成立。

prop 的預設值。 共用表單元件新增了一個顯示選項，而預設值是「啟用」。就算既有畫面（登入、設定等）沒有傳這個 prop，行為也會改變，所以才會問要不要改成 opt-in。

直接破壞性地修改傳入的物件。 merge 函式直接改寫引數 dict 的 score，所以呼叫端持有的陣列內容也一起變了。雖然「現在同一個函式裡用完就結束，所以暫時沒影響」先被說出來了，但之後一旦拿去做 log 或存檔就會混在一起，這是在提醒未來風險。

第 11 位 能不能驗證（4 則）

數量最少，但屬於會打破「有測試就安心」這種想法的類型，所以我把它獨立出來。

忘記 mock，結果真的送出去了。 有 3 個測試會走到送通知的流程，但通知 client 沒有被 mock。測試用的設定 key 清單裡也沒有包含那個環境變數，所以只要在有設定該變數的環境執行，通知就會真的送出。同一個檔案裡其他測試都有好好 mock，正是從這種不對稱看出來的。

根本無法在合併前測試。 CI workflow 的 job 加了 if 條件，非目標 branch 即使手動執行也會整個 job 被 skip。也就是說，無法在 merge 前確認「這個修正到底有沒有用」。這個指摘不只是指出問題，還附上了一個暫時 branch 直接空跑的具體指令。

數出來後發現的 3 件事

1. 指摘的三分之一不是 bug，而是「阻止劣化」的一側

第 1 名（和旁邊的程式碼有一致嗎、31 則）與第 2 名（契約與意圖、30 則）合計 61 則，占 33%。這兩個領域共同點在於，提出指摘的當下其實什麼都還沒壞。

真正會壞的是「新增一個選項時」「改共用樣式值時」「只改了一邊時」。也就是未來的修改。這位 reviewer 的角色不是幫 PR 過關，而是替 codebase 降低劣化速度。

我原本以為「review = 找 bug」。所以對還沒壞的程式碼，我什麼也說不出來，也想不到該說什麼。這就是我最大的發現。

2. 「能不能注意到失敗」本身就是獨立品質項目

第 4 名的 17 則，功能上全部都正常運作。silent failure、到不了的 log、可以從外部打到的通知、被吃掉的錯誤訊息。功能測試一條也不會掛。

如果只看「有沒有動」，這 17 則會完全消失。反過來說，會不會把「壞掉時人類能不能知道」列成品質項目，就是深度 review 的分水嶺。

3. 69% 是因為「又打開了一個檔案」

第二個軸（打開了什麼）統計起來，187 則裡有129 則（69%）是因為看了差分以外的東西。

只靠從上到下讀 diff 就能提出的，只有 58 則（31%）。也就是說，實力差的本質，不是觀察角度的天份，而是有沒有再多打開一個檔案。

而且第 1 名領域中，有 87% 是從「打開其他地方」這個動作產生的，而且大多只是 grep、再把成對的檔案並排看而已，幾乎不需要什麼專業知識。

找出自己的漏洞

分類結束後，我也把自己在同一個 PR 上提出的 review，按同樣的 11 個領域重新分類。結果第 3 名（值）和第 5 名（使用者）有，但第 1 名、第 2 名、第 10 名完全沒有。也就是說，我只看了 3 個領域。

於是我決定從報酬率最高的中領域開始養成習慣。

中領域（所屬大領域）要做的事件數根據保存（契約與意圖）對數值常數／固定值去問「這個根據寫在哪裡」20對稱性（和旁邊的程式碼一致嗎）找出成對名稱（列表/詳細、總計/明細、iOS/Android）並排查看10重複定義（和旁邊的程式碼一致嗎）grep 新定義常數的值9dead code（和旁邊的程式碼一致嗎）用 grep 計算參照數／在上游確認 guard 會變成 false 的條件8silent failure（有注意到失敗嗎）打開 HTTP client 設定／把 catch 的可達條件全部說出來8變更波及範圍（變更會影響到哪裡）grep 變更元件的呼叫端數量7621、這 6 項就有 62 則，占整體 33%。而且第 1 名的「根據保存」中，20 則裡有 14 則只要看 diff 就能提出。也就是說，甚至連一個檔案都不用打開就能開始的事情，件數反而最多。順序上，從這裡下手肯定最有效率。

數過才知道有幫助

觀察「很會 review 的人」並模仿時，通常會先注意到措辭和指摘的細膩程度。我一開始也想先抄那一套。但真的數完 187 則後才發現，拉開差距的不是文筆，而是注意力到底有沒有分散到 11 個地方，還是只有 3 個地方。

如果你手上也有一個「這個人的 review 超強」的人，真的很推薦用 GitHub API 全部抓出來數一次。不用半天。你自己從來沒踏進去過哪些領域，會赤裸裸地現形。

你們團隊裡，這 11 個領域又補了幾個呢。

（本文中的技術描述，皆以 2026 年 8 月時點的官方文件確認。件數與分類則是作者手動分類所得的個人資料集）

原文出處：https://qiita.com/ktdatascience/items/02b6b45e2ca7d34ad146