バグを見つけるコードレビューの型
「LGTM」で流れるレビューと、バグを止めるレビューの違い
コードレビューは形式的には同じ作業に見えて、実際には二種類あります。ひとつは「コードが読めて、大きな違和感がなければ承認する」レビュー。もうひとつは「このコードが本番でどう壊れるかを具体的に想像しながら読む」レビューです。前者はスタイルの指摘や命名の提案は出ても、バグはすり抜けます。後者との差は、経験の量ではなく「差分をどう読むか」という手順の差にあります。
差分は上から読まない — データの流れを追う
Pull Request の差分は、ファイル順・変更順に並んでいます。しかし上から順に読むと、変更の意図を追うだけで精一杯になり、影響範囲まで頭が回りません。バグを見つける人は、差分を次の順で読み直しています。
- 入力の入口を探す: フォーム送信・API リクエスト・イベントハンドラなど、この変更に関わるデータがどこから来るかを特定する
- 変換のたびに値を追う: バリデーション → 加工 → 保存/送信、と値が形を変える箇所ごとに「ここで null になり得るか」「型は合っているか」を確認する
- 出口を確認する: レスポンス・DB 書き込み・UI 表示など、最終的にどこへ出ていくかを見て、入口の値と辻褄が合っているか照合する
この「入口→変換→出口」の順で読むと、変更されたコード単体ではなく、データが通り抜ける経路全体としてレビューできます。差分がファイルをまたいでいても、この順番は変わりません。
必ず疑う5箇所
時間が限られたレビューでも、以下の5箇所だけは機械的にチェックすると、見逃しが大きく減ります。
1. 境界値
配列の先頭・末尾、0件・1件・上限件数、空文字と null の違い。「動作確認したのは3件のデータ」で、0件と1000件を試していないケースは多いです。
2. エラー処理
try/catch で握りつぶしていないか、失敗時にユーザーへ何を見せるかが書かれているか。正常系のコードが3行、異常系が0行という差分は要注意です。
3. 非同期処理
複数の非同期処理が同時に走ったときの順序保証。コンポーネントがアンマウントされた後に setState が呼ばれないか、連打で二重送信が起きないか。
4. 権限
API のレスポンスを絞る条件が、フロントの表示条件と一致しているか。「画面上は隠しているが API は返している」は典型的な見落としです。
5. 後方互換
DB カラムの型変更・API レスポンスの形変更が、既存データや旧バージョンのクライアントを壊さないか。マイグレーションと同時にコードをデプロイする前提が崩れていないか。
実例: 「境界値」で止まった1件
あるチームでは、割引クーポンの適用ロジックに discountedPrice = price - coupon.amount という1行が追加された PR がありました。正常系のテストは3件のデータ（1,000円・3,000円・5,000円のクーポン）で通っていましたが、レビュアーが「境界値」の観点で coupon.amount が price を超える場合を確認したところ、価格がマイナスになるケースを再現できました。実装者は「クーポンは常に価格より小さい」という前提で書いていましたが、その前提を検証するコードはどこにもありませんでした。この1件は、境界値を機械的にチェックする習慣がなければ、本番で価格がマイナス表示されるまで気づかれなかった可能性が高いバグです。
レビューコメントの書き方
疑わしい箇所を見つけたら、指摘ではなく質問で書くとレビューが早く回ります。
- ❌「ここバグってます」→ 修正が必要な理由を書く手間が発生し往復が増える
- ✅「配列が空のときここで例外になりませんか？該当箇所のテストはありますか？」→ 相手が自分で確認して直せる
Bugoon での実践
レビューで見つけた不具合をその場で再現できないケースは珍しくありません。Bugoon はブラウザ上でスクリーンショット・アノテーション・操作ステップを記録してバグレポート化し、GitHub Issue として起票できるため、レビュー時に見つけた「境界値で怪しい挙動」をそのまま検証環境で再現してレポート化し、実装者に渡す、という流れを1つのツールで完結できます。カンバンボードでレビュー起因の指摘とユーザー報告を同じ場所で管理できる点も、レビューを「言った・言わない」で終わらせないために活きてきます。