自分のレビューが薄い理由が分からなかった
同じPRを見ているのに、自分は「LGTM」で終わり、隣の人は10件の的確な指摘を出してくる。何を見ればそうなるのか、聞いてみても「気になった箇所を見てるだけ」という答えしか返ってこない。同じ経験、ありませんか。
それなら実物を数えるしかない、と思ってGitHub APIでその人のインラインコメントを全部引っ張ってきました。3リポジトリ・44本のPRに付いた187件。全部読んで、「どんな主題について語っているか」で分類しています。
結論から書くと、最大勢力はバグ指摘ではありませんでした。むしろ「今は何も壊れていない箇所」への指摘が全体の3分の1を占めていて、自分がまったく手を出せていなかったのもそこ。正直、けっこうへこみました。
集め方
GitHubのREST APIには、リポジトリ全体のレビューコメントを一括で取れるエンドポイントがあります。PRを1本ずつ回すより速いので、こちらを使いました。
gh api "repos/{owner}/{repo}/pulls/comments?per_page=100" --paginate \
| jq -c '.[] | select(.user.login=="対象のユーザー名")'
これで取れた212件のうち、スレッド内の返信21件と、同一レビューが複数回サブミットされて本文が完全一致していた4件を除いた187件が分析対象です。
分類の軸は3つ作りました。
- 型 — 何を指摘したか（条件の誤り／デッドコード／非対称…）
- 動作 — 見つけるために何を開いたか（既存の別ファイル／呼び先／公式ドキュメント…）
- 領域 — そもそも何に注意を向けているか
この記事は3番目の「領域」の話です。1と2は「どう見つけるか」の話ですが、レビューで手が止まる原因はたいてい「探し方を知らない」ではなく「そこに注意が向いていない」なので、領域のほうが実務に効くと考えました。
11領域の分布
187件は11の大領域（さらに細かく37の中領域）に分かれました。件数の多い順に並べます。
| 順位 | 領域 | この領域が立てている問い | 件数 |
|---|---|---|---|
| 1位 | 隣のコードと揃っているか | 同じことをしている場所と食い違ってないか | 31 |
| 2位 | 契約と意図は保存されているか | 半年後の人が同じ判断に辿り着けるか | 30 |
| 3位 | 値そのものは正しいか | この数字・この判定は合っているか | 25 |
| 4位 | 失敗に気づけるか | 壊れたとき人間は知れるか | 17 |
| 5位 | 使う人から見てどうか | 運用担当とユーザーの手元で何が起きるか | 16 |
| 6位 | 外部との境界 | 自分が書いていないものは本当にそう動くか | 15 |
| 7位 | データが壊れないか | 落ちた・並んだ・消えたとき何が残るか | 13 |
| 7位 | 設計の見通し | 次に触る人が読めるか | 13 |
| 9位 | 速度とコスト | 何秒・何円かかるか | 12 |
| 10位 | 変更はどこまで効くか | 書いた人が見ている画面の外にも届くか | 11 |
| 11位 | 検証できるか | 効いたことをあとから確かめられるか | 4 |
上位2つで61件、全体の33%。一方、いわゆるバグ相当（3位の「値そのもの」＋7位の「データが壊れないか」）は38件で20%です。5件に1件しかない。
レイヤーが変わると領域も入れ替わる
同じ人のレビューでも、対象がバックエンドかモバイルかWebフロントかで、厚い領域がはっきり違いました。
バックエンドは「値の正しさ」が20%で突出（金額・時刻・DB制約）。モバイルは「データが壊れないか」「変更の波及」「外部SDK」に寄る。Webフロントは「一貫性」と「意図の保存」だけで半分を占め、「外部との境界」と「検証できるか」はゼロです。
全領域を毎回見る必要はなくて、差分のレイヤーで絞れる。これが分かっただけでもだいぶ気が楽になりました。
各領域で実際に何を指摘しているのか
ここからが本題です。11領域それぞれ、実物を2件ずつ並べます。事例は社外向けに一般化していますが、指摘の構造はそのままです。
1位 隣のコードと揃っているか（31件）
最大勢力です。しかもこの31件のうち27件（87%）が「リポジトリ内の別の場所を開いた」ことから生まれていました。
合計と明細でロジックが違う。 合計点数は Math.abs(quantity) ＋異常値フォールバック 0 で集計しているのに、各行の表示は Math.abs なし・フォールバック 1。通常データなら合いますが、返品などで数量がマイナスになると「各行を足しても合計にならない」表示になる。「合計」と「明細」は必ずペアなので両方開く、というだけの動作です。
別名が本番から参照されていない。 共通処理を別モジュールへ切り出すリファクタで、元クラスに委譲用の別名を5つ残していた。ところが移動先が内部でモジュールグローバルを直接参照していたため、その別名は本番コードからの参照が0。「テストで @patch しても差し替わらず実物が呼ばれる」「SQL定数を書き換えても実行されるSQLは変わらない」状態でした。挙動不変のリファクタなのに、テストの縫い目だけが静かに偽物になっていたわけです。
2位 契約と意図は保存されているか（30件）
これが2位に来るとは思っていませんでした。バグでも設計ミスでもなく、「なぜそう書いたか」が失われることを防ぐ指摘です。
マジックナンバーの根拠。 CSSに calc(50% + 3.5rem) という値があり、レビュー側で「親の幅6rem + gap 1rem = 7rem の半分だけ右にずらす計算ですね」と自分で解いたうえで、「意図としてはバッチリですが、親の幅やgapを変えたときにここも追従修正が必要になるので一言コメントを」と続けている。合っていることを確認してから、依存が見えないことだけを指摘する。
「移行しました」の裏をgit履歴で取る。 カメラ設定を新クラスへ移行したPRで、旧実装のコミットまで遡って設定値の指定が引き継がれていないことを立証していた。別のPRでは、PR説明に書かれた「このバグが原因だった」という因果そのものを、当時のコミットを特定して反証しています。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は片方を変換してから比べるのですが、このときタイムゾーンなしの値は「セッションの TimeZone 設定のローカル時刻」として解釈されます（PostgreSQL 日付/時刻データ型 §8.5.1.3）。
つまり TimeZone が Asia/Tokyo になっていると、UTCのつもりで入れた値がJSTとして読まれ、実際より9時間早く「期限切れ」と判定される。金銭やポイントの有効期限でこれが起きると、まだ生きているはずのものが消えます。
指摘の書き方が面白いのはここからで、「今は正しく動く」ことをまず認めたうえで、なぜ今は動くのかまで書いている。DBコンテナの TimeZone が未指定でたまたまUTCだから、というだけの理由です。効いているのはアプリ側のTZではなくDBセッション側の設定なので、DBの起動オプションが1つ変わるだけで壊れる。そのうえで、設定に依存しない書き方への変更を提案しています。
-- 両辺ともタイムゾーンなしのUTCになるので、セッション設定に左右されない
SELECT * FROM items WHERE expires_at < NOW() AT TIME ZONE 'utc';
4位 失敗に気づけるか（17件）
機能としては正しく動くのに、壊れたことが誰にも伝わらない。これが独立した領域として17件もありました。
HTTPクライアントの設定でcatchが発火しない。 aspida のクライアントが throwHttpErrors: false（既定値）で初期化されていたため、await した post が400/500を受けても例外を投げない。結果、try/catch で囲んでいるのに catch 節に入らず、APIが拒否しても「変更しました」の成功バナーが出ていました。設定ファイル1行が、「try/catch があるから安全」という直感をひっくり返している。
丁寧なエラー文言が途中で潰される。 「同じ対象へ既に登録済みの可能性があります」という親切な例外メッセージを新設したのに、それを拾うデコレータの捕捉リストに入っていないので、上位の except Exception に落ちて「エラーが発生しました。管理者にお問い合わせください。」に置き換わる。丁寧な文言を見つけたら、それが画面に到達する経路を追う。ここも定型の動きです。
5位 使う人から見てどうか（16件）
コードとしては正しいが、画面の前にいる人が困る領域。
リダイレクトで絞り込みだけ落ちる。 管理画面で特定ユーザーに絞ってから操作すると、戻り先URLの組み立てにユーザーIDが渡されておらず、絞り込みが外れた一覧の同じページ番号に戻る。ページ番号だけ保持されるぶん、直前まで見ていた行とはまったく別の行が並ぶ。ここまで具体化すると、単なるUXの話が誤操作のリスクに変わる。
オーバーレイが覆いきれていない。 処理中のローディングオーバーレイがコンテナの内側に置かれていて、フッターのボタンを覆っていない。二重実行自体は別のフラグで防げているものの、処理中に対象データを削除できる導線が残っていました。
6位 外部との境界（15件）
自分が書いていないもの（SDK・OS API・CLI・フレームワーク）は、本当にそう動くのか。
setter を adder だと思って呼んでいる。 ML KitのバーコードスキャナはフォーマットをBuilderで指定しますが、setBarcodeFormats(int format, int... formats) の公式ドキュメントには "Only the last call will be respected if calling this method multiple times" と書かれています。forEach で1つずつ渡すと最後の1件しか残らない。しかもそこから呼び出し元の定数定義まで追って、どのカードが読めなくなるかまで特定していました。
Content-Type次第でボディが読めない。 Flaskの request.get_json(silent=True) は、リクエストのmimetypeがJSONを示していなければ、ボディの中身に関わらず None を返します（Flask API リファレンス）。相手が正しいJSONを送ってきてもヘッダを付けていなければ弾かれる。外部連携の疎通当日にハマるやつです。
ちなみに silent を付けない場合の挙動はバージョンで変わっていて、Flask 2.1 で400、2.3 で415を返すようになりました。silent=True 側は昔から None のまま変わっていません。この手の「バージョンで変わった／変わっていない」を lockファイルで実バージョンを確認してから調べているのが、この指摘の効き所でした。
7位 データが壊れないか（13件）
正常系では絶対に露見せず、障害時・同時実行時・画面遷移時にだけ顔を出す領域です。
中間テーブルから外れると孤児化する。 ユーザーを作った直後は引けるが、後から所属テーブルの行が消えると、取得関数のどの分岐でも引けなくなる。レコードは残っているのに誰からも参照できない状態になります。
DB制約がアプリの前提を保証していない。 GROUP BY user_id して MIN(company_id) を採る実装に対して、「DB制約は1ユーザー×1会社を強制していない（片方は会社単位ユニーク、もう片方は施設単位ユニーク）ので、複数所属の行が作れた瞬間に画面では片方しか見えなくなる」。アプリが暗黙に前提にしているカーディナリティを、スキーマ定義まで開いて確かめているわけです。
7位 設計の見通し（13件）
いわゆる「コードレビューらしいコードレビュー」。同率7位ですが、全体の7%しかありません。
nullを返しうる値へのキャスト。 null を返す可能性がある関数の戻り値に as string を3回。外側で存在チェック済みなので、変数に一度入れればキャストは不要になる、という3行の書き換え提案。
型が効いていないことを事故で語る。 TanStack Query（v5系で確認）でクエリの meta に独自キーを渡していたが、Register インターフェースに queryMeta を宣言していないため meta の型が Record<string, unknown> のまま。どんなキー名でも通るので、キー名をタイポしてもコンパイルで拾えません。「型が緩い」ではなく「タイポが通る」と言うと、直す理由がはっきりします。
9位 速度とコスト（12件）
この領域の指摘は、ほぼ全件に実測値が入っています。「重そう」ではなく数字で言う。
インデックスと呼ばれる頻度。 新しく追加された検索クエリに対して、モデル定義のインデックス宣言を開いて「対応するインデックスがない」と確認。さらに「同じクエリは別の場所にもあるが、あちらは初回1回きり、こちらは再入路なのでセッション有効期限が365日である以上、毎回のアクセスがここを通る」と続く。インデックスの有無より、呼ばれる頻度の差を言語化しているのが効き所です。
タイムアウトはインフラと足し算する。 アプリ側のタイムアウトを40秒に設定した差分に対し、本番ロードバランサーの実測値（直近7日で平均0.4秒台、p99が3秒台、最大18秒）を出したうえで「ここで40秒使うと合計1分近くになり、ALBのidle_timeout 60秒に迫る」と指摘していた。自分より外側のタイムアウトを調べて足し算する、という発想です。
10位 変更はどこまで効くか（11件）
「バグ」ではなく「意図した範囲を超えている」という指摘。PRの説明文が正しくても成立します。
propのデフォルト値。 共通フォーム部品に新しい表示オプションが追加され、そのデフォルトが「有効」になっていた。この prop を渡していない既存画面（ログイン・設定など）でも挙動が変わってしまう、opt-in にしませんか、という話。
引数のオブジェクトを破壊的に書き換えている。 マージ関数が引数で受け取ったdictのスコアを直接書き換えるので、呼び出し側が持っている配列の中身も一緒に変わる。「いまは同じ関数の中で使い終わるので影響はないのですが」と前置きしたうえで、あとからログや保存に回したときに混ざる、という将来の話をしている。
11位 検証できるか（4件）
件数は最少ですが、「テストがあるから安心」を崩すタイプなので独立させました。
モックし忘れで本物に飛ぶ。 通知を送る経路を通るテストが3本、通知クライアントをモックしていなかった。テスト用の設定キー一覧にもその環境変数が入っていないため、その変数が設定された環境でテストを流すと実際に通知が飛ぶ。同じファイルの他のテストはきちんとモックしていて、その非対称から見つけています。
そもそもマージ前に試せない。 CIワークフローのジョブに if 条件が付いていて、対象ブランチ以外では手動実行してもジョブごとスキップされる。「この修正が効くか」をマージ前に確認できない構造でした。指摘だけで終わらせず、一時ブランチで空打ちする具体的なコマンドまで添えてある。
数えて分かった3つのこと
1. 指摘の3分の1は「バグ」ではなく「劣化を止める側」だった
1位（隣のコードと揃っているか・31件）と2位（契約と意図・30件）で61件、33%。この2領域に共通するのは、指摘した時点では何も壊れていないことです。
壊れるのは「新しい選択肢を1つ足したとき」「共通スタイルの値を変えたとき」「片方だけ直したとき」。要するに未来の改修です。この人のレビューは、PRを通すための検査ではなく、コードベースの劣化速度を落とす作業として設計されていました。
自分は「レビュー＝バグを見つけること」だと思っていました。だから壊れていないコードには何も言えなかったし、言うことが思いつかなかった。ここが一番の発見です。
2. 「気づけるか」が独立した品質項目になっている
4位の17件は、機能としては全部正しく動きます。silent failure、届かないログ、外部から撃てる通知、潰されるエラー文言。機能テストでは1件も落ちません。
「動くか」だけを見ていると、この17件はゼロになる。逆に言えば、「壊れたときに人間が知れるか」を品質項目としてリストに持っているかどうかが、深掘りレビューの分かれ目でした。
3. 69%は「もう1枚ファイルを開いた」から出ている
2番目の軸（何を開いたか）で数えると、187件のうち129件（69%）が差分の外を見た結果でした。
差分だけを上から下に読んで出せるのは58件（31%）しかありません。実力差の正体は着眼点のセンスではなく、もう1枚ファイルを開いたかどうかという作業量だった、ということになります。
しかも1位の領域は、その87%が「別の場所を開く」動作から生まれていて、その大半はgrepと「対になるファイルを並べる」だけ。専門知識をほとんど必要としません。
自分の穴を見つける
分類が終わったあと、同じPRに自分が出したレビューを同じ11領域に振ってみました。3位（値）と5位（使う人）には入っていたのに、1位・2位・10位には1件も入っていない。見ていた領域が3つしかなかったわけです。
そこで、費用対効果の高い中領域から順に習慣にすることにしました。
| 中領域（属する大領域） | やること | 件数 |
|---|---|---|
| 根拠の保存（契約と意図） | 数値リテラル・固定値に「この根拠はどこに書いてあるか」と聞く | 20 |
| 対の非対称（隣のコードと揃っているか） | 対になる名前（一覧/詳細、合計/明細、iOS/Android）を探して並べる | 10 |
| 二重定義（隣のコードと揃っているか） | 新しく定義した定数の値をgrepする | 9 |
| デッドコード（隣のコードと揃っているか） | 参照をgrepで数える／ガードがfalseになる条件を上流で確認 | 8 |
| silent failure（失敗に気づけるか） | HTTPクライアントの設定を開く／catch の到達条件を全部言う |
8 |
| 変更の波及範囲（変更はどこまで効くか） | 変更した部品の呼び出し元をgrepで数える | 7 |
この6つで62件、全体の33%です。しかも1位の「根拠の保存」は20件中14件が差分を読むだけで出せるものでした。ファイルを1枚も開かずに始められるものが、いちばん件数が多い。順番としては、ここから始めるのが確実に速そうです。
数えてよかった
「レビューがうまい人」を観察して真似ようとすると、たいてい言い回しや指摘の丁寧さのほうに目が行きます。私も最初はそこを写経しようとしていました。でも実際に187件を数えてみたら、差がついていたのは文章力ではなく、注意の向け先が11箇所あるか3箇所しかないか、でした。
もし手元に「この人のレビューはすごい」と思える相手がいるなら、GitHub APIで全部引っ張って数えてみるのをおすすめします。半日もかかりません。自分がどの領域に一度も足を踏み入れていないかが、身も蓋もなく出てきます。
みなさんのチームでは、この11領域のうちいくつが埋まっていますか。
（本記事の技術的な記述は2026年8月時点の公式ドキュメントで確認したものです。件数・分類は筆者が手作業で分類した私的データセットに基づきます）