問題と適用可能なコンテキスト
見慣れないプルリクエストをレビューするために30分与えられました。変更の意図をどのように再構築し、レビューの順序を組み立て、ブロッカーと非ブロッカーのフィードバックを区別し、作成者が行動に移せるコメントを書き、承認・コメント・変更要求をどのように選択するかを説明してください。
これは、バックエンド、フロントエンド、モバイル、インフラ、エンジニアリングマネジメントなどのポジションを対象とした、一般的なソフトウェアエンジニアリングの面接質問です。口頭でのプロセスの質問として尋ねられることもあれば、提示された差分（diff）を実際にレビューするライブレビュー形式で行われることもあります。どちらの形式であっても、単にフォーマットの不備を多く見つけ出すことではなく、制限時間内にユーザーやシステムにとって最も重要な問題を特定できるかが評価されます。
プルリクエストの説明文、リンクされた要件、変更されたファイル、テスト結果は確認できるものの、コードベースには精通しておらず、作成者に逐一質問することはできない状況を前提としてください。面接官から異なる条件が提示された場合は、レビューを始める前にリスクとスコープを再調整してください。
面接官が評価するポイント
第一の評価基準は、その変更が何を意図しているのかを再構築できるかどうかです。要件、APIコントラクト、あるいは障害境界の理解がなければ、コメントは単なる好みの押し付けになってしまいます。優れた回答では、プルリクエストのコンテキストと周辺のコードを読み込み、振る舞いの変化を特定した上で、初めて1行ずつの確認に進みます。
第二の評価基準は優先順位付けです。正確性、データの破損、セキュリティ、認証・認可、並行性、互換性は、通常、命名やコードの配置よりも先に注意を払うべき対象です。面接官は、発生し得る影響を明確に述べ、最もリスクの高いパスに時間を割けるかを見ています。
第三の評価基準は根拠の有無です。「これはバグかもしれません」という指摘は、単なる手掛かりに過ぎません。質の高いフィードバックでは、発生条件、目に見える影響、そしてそれを検証する方法が提示されます。コンテキストが不足している場合は、推測をブロッカー扱いの結論として決めつけるのではなく、的確な質問を投げかけます。
最後に、面接官はレビューの判定とコミュニケーションを評価します。必須の修正、任意の提案、確認のための質問、些細な指摘（Nit）を明確に区別してください。サマリーには、自分が確認した範囲、未検証のまま残っている領域、そして承認または変更要求を下した理由を明記します。作成者の能力ではなく、コードとその影響に焦点を当てて議論を進めます。
回答前に明確にすべき質問
- これは口頭でのプロセスに関する質問ですか、それとも実際の差分を用いたライブレビューですか？ 前者の場合は再現可能な手法を提示します。後者の場合は、手法を簡潔に述べた上で、チェックリストを読み上げるのではなく実際のコード行に対して適用します。
- どのようなコンテキストが提供されていますか？ 要件、APIコントラクト、周辺コードがあれば振る舞いを検証できます。孤立した関数のみの場合は、前提条件を明示し、不明なコントラクトを質問として提示します。
- 高リスクな領域に関わる変更ですか？ 決済、認証・認可、プライバシー、マイグレーション、公開APIなどの領域は、より厳密な根拠が求められ、ドメインエキスパートの参加が必要になる場合があります。リスクの低い社内ツールの場合は、迅速な段階的改善を優先できます。
- 面接官が期待している成果物は何ですか？ インラインコメント、サマリー、承認の判断、テストの推奨事項など、期待される成果物によって時間の配分が異なります。30分を使い始める前にアウトプットを確認してください。
- これは通常業務の変更ですか、それとも緊急修正ですか？ 緊急修正であれば、範囲を絞ったパッチと後続作業への切り出しが正当化されます。ただし、既知のセキュリティやデータ破損のリスクを無視してよい理由にはなりません。
- 影響を受けるすべての領域をレビューする資格がありますか？ 自身の専門外である暗号処理、プライバシー、データベースマイグレーションが含まれている場合は、対応可能な範囲をレビューした上で、過信して承認せず適切な資格を持つレビュアーを要請します。
30秒で答えるフレームワーク
「私はまず、プルリクエストの目的、振る舞いの変更点、障害時の影響範囲を明確にした上で、2つのパスに分けてレビューを実施します。第1パスでは変更の境界、データフロー、高リスクなパスをマッピングし、正確性、セキュリティ、データ保全、並行性、互換性を最優先で確認します。第2パスではエッジケース、エラーハンドリング、テスト、オブザーバビリティ、パフォーマンス、保守性をチェックします。各コメントには重要度、発生条件、影響、期待される結果を明記します。再現可能なブロッカーがあれば変更を要求し、提案や軽微な指摘（Nit）のみであれば承認と合わせて伝えます。最後に、レビューした範囲、未検証のリスク、判断の理由をまとめて完了します。」
ステップごとの詳細な回答
まず基準となるベースラインを確立します。タイトル、説明文、リンクされた要件、APIやデータモデルの変更点、既存のテストを確認します。そして、コントラクトを1文で再定義します。「この変更は、既存の保証を維持しつつ、特定の条件下で対象のユーザーに新しい振る舞いを提供する。」この1文を作成できない場合は、行単位のコメントを始める前にコンテキストを把握してください。正しさの基準がまだ定まっていないためです。
次に、変更の境界をマッピングします。単に変更行だけを見るのではなく、入力、状態変更、外部への副作用、返却パスを追跡します。新しいパラメータを受け取る呼び出し元はどこか？データベースへの書き込みとメッセージ送信は同じ障害境界内にあるか？公開レスポンス、イベント形式、設定のデフォルト値に変更はないか？このパスのアウトプットとして、入力がどこから入り、どの信頼境界を越え、何の状態が変化し、どのように失敗し得るかというモデルを構築します。
30分のタイムボックスの一例として、意図の把握に3分、境界と高リスクパスの確認に7分、詳細な精査に12分、テストと運用のセーフガードに5分、コメント作成と判断に3分を割り当てます。これは差分の規模やリスクに応じて調整するための実践的な配分です。最初の20分を命名の確認だけで消費してしまうのを防ぐことが目的です。
第1パスでは、以下のリスク順序を適用します：
- 振る舞いと正確性： メインパスはコントラクトを満たしているか？空の入力、重複リクエスト、部分的な障害、リトライ時に何が起きるか？
- セキュリティとデータ： 信頼境界を越える前に認可が行われているか？機密データが露出していないか？障害によってデータの損失、重複、元に戻せない状態の変化が発生しないか？
- 並行性と互換性： 同時リクエストによって不変条件が壊れないか？ローリングデプロイ中に古いクライアント、古いデータ、新旧バージョンの混在環境で正常に動作するか？
- アーキテクチャの境界： 責務は適切なコンポーネントに配置されているか？既存の制約をバイパスして状態を重複させていないか？
第2パスでは実装の詳細を精査します。制御フローとエラーの伝播、リソースのクリーンアップ、クエリやループの計算量・規模、ログとメトリクス、コードが誤っている場合にテストが実際に失敗するかどうか、そして命名やコメントが将来の読み手の助けになるかを確認します。フォーマットや自動修正可能なスタイルは最後に回し、ツールで検出できる軽微な問題に人間の判断リソースを奪われないようにします。
見つかった各問題について、今回の変更によって導入されたか、あるいは表面化したものかを確認します。空のitemsが有効なケースで、新規コードがitems[0]を読み取っている場合、それは具体的なリグレッションです。同じファイル内に無関係な既存の複雑さが存在する場合は、今回の変更と組み合わさってセキュリティや正確性のリスクを生じさせない限り、技術的負債として言及するか、後続のタスクを作成するにとどめます。境界を定めなければ、レビューの範囲は際限なく広がってしまいます。
コメントの意図は次の4つに分類します：
- Blocker（ブロッカー）： コントラクト違反、誤った結果、セキュリティ問題、データ破損、許容できない互換性リスクの証拠がある場合。マージ前に必ず解決する必要があります。
- Question（質問）： 結論を変え得るコンテキストが不足している場合。回答によって懸念が解消されるか、またはブロッカーに格上げされます。
- Suggestion（提案）： 設計、保守性、運用の改善として価値があるものの、現在の実装でもマージ基準を満たしている場合。
- Nit（些細な指摘）： 本来はフォーマッターや静的解析で処理されるべき、可読性や一貫性に関する非ブロッカーの指摘。
アクション可能なコメントは、「ラベル + 条件 + 影響 + 期待される結果」を含み、必要に応じて解決の方向性を1つ添えます。例：
Blocker: リクエストでitems=[]が許可されている場合、ここでitems[0].idを読み取ると例外が発生し、バッチエンドポイントが500エラーを返します。ループの前に空配列の処理を追加し、リグレッションテストを作成してください。空の結果を返すか400を返すかはAPIコントラクトの仕様に準拠してください。
空の入力が許容されるか不明な場合は、質問形式にします：「APIコントラクトでは空配列を許容していますか？現在のパスでは500が返されます。許容される仕様であれば、明示的なハンドリングとテストが必要です。」これにより、要件を勝手に決めつけることなく事実と根拠を伝えることができます。
最後にレビューの判定を行います。ブロッカーが未解決の場合は変更要求（Request changes）を選択します。重要なコンテキストが不足している場合は、承認の裏に不安を隠すのではなく、コメント（Comment）を送信します。非ブロッカーの提案のみが残っている場合は承認（Approve）し、それらがマージの必須条件ではないことを明記します。サマリーには、確認したスコープ、主な指摘事項、ランタイムやテストによる根拠、未確認の領域、最終的なステータスを記載します。
CIがグリーン（パス）であっても、レビューが完了した証明にはなりません。テストがクリティカルな分岐を網羅していない可能性があり、静的解析ツールはプロダクトのコントラクトを把握できないためです。逆に、人間のレビューを実行可能なテストの代用にしてはなりません。両者を結びつけ、コメント内で失敗条件を特定し、「修正前には失敗し、修正後にはパスするテスト」の追加を求めてください。
質の高い回答例
「私は最初から1行ずつのアラ探しを始めることはしません。まずプルリクエストの説明文、リンクされた要件、インターフェースの変更点を確認し、達成すべき目的と維持すべき既存の振る舞いを言語化します。コンテキストが不足している場合は、推測をブロッカーとして決めつけるのではなく、前提条件を整理して提示します。
30分の制限時間内であれば、2つのパスに分けてレビューを進めます。第1パスではエントリポイント、状態変更、外部への副作用、返却パスを追跡し、正確性、セキュリティ、データ破損、並行性、互換性を最優先で確認します。第2パスではエッジケース、エラーハンドリング、パフォーマンス、ログ、テスト、保守性を網羅します。スタイルに関する確認は最後に行い、ツールでカバーされておらず、コードの理解に真に影響を与える場合にのみ指摘します。
すべての指摘事項において、『何がトリガーとなるか』『どのような影響があるか』『どう検証するか』の3点に答える必要があります。必須の修正はBlocker、不足しているコンテキストの確認はQuestion、非ブロッカーの改善案はSuggestion、軽微な調整はNitとしてラベル付けします。たとえば、APIが空入力を受け付けるにもかかわらず空配列パスで先頭要素を読み取っている場合、単に『nullの可能性あり』と書くのではなく、500エラーが返される影響を説明し、明示的なハンドリングとリグレッションテストの追加を求めます。
送信する前に、各コメントが今回の差分に関連しているか、個人の好みをルールのように押し通していないか、必要なドメインのレビュアーが見落とされていないかを確認します。再現可能なセキュリティ、データ、正確性の問題があれば変更要求を行い、提案のみであれば承認と合わせて伝えます。作成者が次のアクションを把握でき、後続のレビュアーが私の確認範囲を正しく理解できるよう、サマリーに対象ファイル、テストの根拠、未検証領域、判定理由を記載します。」
よくある落とし穴
- 差分を開いていきなり1行ずつコメントする → 意図やコントラクトの理解がないと、正当なトレードオフを欠陥と誤認してしまう → まず目的、振る舞いの変化、障害境界を明確にする。
- 発見した順にコメントを投稿する → 命名などの細かな指摘によって、データ破損や認証・認可の重大なリスクが埋もれてしまう → 実装の調整に入る前にリスクの精査パスを実施する。
- 「バグかもしれません」とだけ書く → 作成者はトリガー条件が分からず、修正の検証ができない → 条件、影響、根拠、期待される結果を明記する。
- すべてのコメントを必須修正扱いにする → 作成者がマージ基準と単なる好みの区別がつかなくなる → Blocker、Question、Suggestion、Nitのラベルを明示する。
- 網羅性をアピールするために長いチェックリストを暗唱する → データフローに適用されていないリストは、判断力の証明にならない → 1つのクリティカルパスを追跡し、残りのカバレッジについて説明する。
- 既存の古い問題をすべて修正するよう要求する → プルリクエストの範囲が際限なく広がり、リスク管理や検証が困難になる → 安全性や正確性の問題に直接波及しない限り、今回のリグレッションと既存の負債を切り離す。
- CIがグリーンであることを承認の根拠にする → テストが誤った仕様を前提にしていたり、分岐を網羅していなかったりする可能性がある → 重要な反例に対してテストが失敗するかを確認し、不足しているリグレッションテストを要求する。
- コードではなく作成者自身に対してコメントする → 相手の防衛的な態度を生み出し、技術的な根拠が伝わらない → 善意を前提としつつ、コード、条件、影響について客観的に記述する。
- 自身の専門外の領域を承認してしまう → その承認が誤った安心感を与えてしまう → 自分が確認した範囲を明記し、適切なドメインエキスパートによるレビューを要請する。
フォローアップの質問と回答
質問1: 作成者があなたのブロッカー（Blocker）の指摘に異議を唱えた場合はどうしますか？
検証可能なコントラクトと影響の事実に立ち返ります。入力値、リスク、あるいはリリース条件に関する認識に齟齬がないかを確認し、可能であれば最小限の再現手順（repro）を提示します。証拠を示しても合意に至らない場合は、コードオーナーやドメインオーナーに判断を仰ぎ、口頭で決定した内容をプルリクエスト上に記録します。意見の不一致を未解決のまま放置してはいけません。
質問2: プルリクエストが大きすぎて30分で確認しきれない場合はどうしますか？
全体を網羅したかのような誤解を与えないようにします。リスクに基づいてエントリポイント、データ変更、公開コントラクトを優先的に選択し、どのファイルを1行ずつ確認し、どれをスキャンし、どれを確認していないかを明確に伝えます。その上で、プルリクエストの分割や追加のドメインレビュアーを要請します。承認ステータスは、実際に実施したカバレッジと一致させる必要があります。
質問3: 差分の範囲外で既存の重大な問題を発見した場合はどうしますか？
まず、今回の変更がその問題を引き起こしたり増幅させたりしていないかを確認します。その相互作用によって現在のリリースにセキュリティ、データ、正確性のリスクが生じる場合はブロッカーとします。完全に独立した問題である場合は、根拠を記録し、優先度の高い後続タスクを作成してオーナーに通知します。このプルリクエストに際限のないリファクタリングを強制してはいけません。
質問4: テストが不完全な緊急修正を承認できますか？
修正しない場合の直近の損失、パッチが最小限に絞られているか、ロールバックや機能フラグによる無効化パスがあるか、利用可能な最小限の対象検証が行われているかを評価します。明確な緊急時プロセスのもとであれば後続でのテスト追加を容認することもありますが、既知のセキュリティ、データ破損、不可逆なリスクに対しては、依然としてより高い承認基準が求められます。時間的制約があるからといって自動的に承認されるわけではありません。
質問5: 専門知識のないドメインのレビューはどのように行いますか？
一般的な制御フロー、エラーハンドリング、テスト、インターフェース変更の確認は継続しつつ、自分が判断資格を持たない部分を明確に示します。暗号処理、プライバシー、マイグレーション、複雑な並行性などについては、適切なオーナーにレビューを依頼します。部分的なレビューは、それが完全な承認として扱われない場合にのみ価値を持ちます。
質問6: テストカバレッジが非常に高い場合でも、すべての行を読む必要がありますか？
はい。ただし着眼点が変わります。テストはコード化されたケースに対する実行可能な証拠を提供しますが、レビューでは依然として、要件自体が正しいか、見落とされているリスクがないか、設計が不要な複雑さを持ち込んでいないか、ログや互換性が適切かといった点を検証する必要があります。レビューで見つかった重要な反例はテストケースへと落とし込み、将来の正確性がレビュアーの記憶だけに依存しないようにします。