新人のPRレビュー指摘が、結果的にベテランより筋が良かった話

きっかけは、入社3ヶ月の新人が投げた一言でした。

「これって、何でこうなってるんですか？」

その日のPRレビューで、自分を含めたベテラン格の3人が「LGTM」を出しかけていた変更に対して、彼がぽつりとそう書いたんですね。正直なところ、最初は「まあ、そういうもんだよ」で流しかけました。でも彼のコメントを読み直したら、こっちの顔がだんだん青くなっていったという。

今日はその話を書きます。技術Tipsというより、レビューの場でベテランが何を見落としがちか、という話です。

ベテラン3人がスルーした前提条件

問題になったのは、ある受託案件の一覧取得APIをフロントから叩く箇所でした。ページングの処理で、こんな実装になっていたんです。

// 一覧を全部取ってからクライアント側で絞り込む実装
async function loadActiveItems() {
  const res = await fetch("/api/items");
  const items = await res.json();
  // active なものだけ表示する
  return items.filter((it) => it.active);
}

レビューしていた我々3人の頭の中では、この /api/items は「せいぜい数十件しか返らない」という前提が共有されていました。案件初期の仕様書にそう書いてあったし、実際ステージングでは20件くらいしか入っていなかった。だから誰も気に留めなかったんですよね。

新人の指摘はこうでした。

このAPI、件数の上限ってどこかで決まってますか？ 本番でデータが増えたら全部フロントに落ちてくる書き方に見えるんですが、絞り込みはサーバー側でやらなくて大丈夫ですか？

これ、刺さりました。仕様書の「数十件」はあくまで初期の想定で、運用が進めば増えるのは当たり前。ベテラン3人は「初期仕様のイメージ」を無意識の前提にしたまま、その前提そのものを疑わなかった。経験があるほど「たぶん大丈夫」の解像度が上がってしまって、逆に前提を言語化しなくなるんだな、と痛感しました。

実際、本番のデータを確認したら既に800件を超えていて、このまま出したら初回ロードで全件フェッチする実装が通るところでした。

「経験者が教える場」だと思っていたのがダメだった

ここで正直に白状すると、それまで自分はレビューを無意識に「経験者が新人のコードを直してあげる場」だと捉えていました。だからレビュアーの割り当ても、暗黙的にベテランが新人のPRを見る一方通行になっていたんですよね。

でも今回の一件で、その構造こそが穴だったと気づきました。ベテラン同士のPRは「あの人が書いたなら大丈夫」でお互いに素通りしがちで、前提を疑う視点がむしろ抜け落ちる。新人は前提を共有していないぶん、素朴に「何で？」と聞ける。その「何で？」が一番効くんです。

公式のレビューガイド的なものにはよく「経験者がメンターとして指摘する」と書いてあるんですが、実際やってみると、指摘の質は経験年数とあまり相関しなかったりします。むしろ「前提を知らない人が読んで意味が通るか」を確認する場として使ったほうが、バグも設計の穴も見つかるという感触でした。

変えた運用：指摘の「人」ではなく「中身」で回す

そこで、レビューの運用をいくつか変えました。精神論だと続かないので、仕組みに落としています。

まず、PRテンプレートに「なぜこの実装にしたか（Why）」を必ず書く欄を足しました。これがあると、レビュアーは前提を推測せずに済むし、書く側も「あれ、これ前提を言葉にできないぞ」と自分で気づけるんです。

## 変更概要
<!-- 何を変えたか -->

## なぜこの実装にしたか（Why）
<!-- 前提・制約・却下した代替案。「件数は最大N件想定」など数字も書く -->

## レビュアーに特に見てほしい点
<!-- 不安な箇所を自己申告する -->

次に、レビューコメントに種別のラベルを付ける運用にしました。指摘が「絶対直して」なのか「ただの疑問」なのかが曖昧だと、新人は全部を重く受け取って萎縮するし、ベテランは軽い指摘をしづらい。そこで接頭辞で温度感を明示します。

[must] active判定はサーバー側に寄せる。全件フェッチは本番で破綻する
[ask]  この分岐、後続の仕様変更で増えそうだけど想定内？
[nits] 変数名 loadActiveItems、getActiveItems の方が既存と揃う

[ask] を正式なラベルにしたのが地味に効きました。新人の「これって何でですか？」は全部 [ask] です。疑問を投げること自体が歓迎される、と運用で示せたのが大きかったですね。

種別ごとの扱いはこう整理しています。

そしてもう一つ、レビュアーの割り当てを「ベテラン→新人」の一方通行から、新人がベテランのPRを最初に読む輪番に変えました。ここは好みが分かれるところですが、うちでは合っていました。

# CODEOWNERS 的な割り当てではなく、輪番表で運用
review_rotation:
  # 各PRに「前提を知らない人」を1人必ず入れる
  - first_reviewer: rotate(all_members)   # 新人も等しく初手レビュアーに入る
  - required_approvals: 2
# 改善の余地あり: いまは輪番を手動のスプレッドシートで回している。
# PR数が増えてきたので、そのうちラベルベースで自動割り当てにしたい。

最後のYAMLは正直まだ手運用で、PRが週に15〜20本を超えてきた今はちょっと限界が見えています。ここは自動化したい部分なんですよね。

数字で見た効果

感覚だけだと説得力がないので、運用を変える前後3ヶ月で「検収・結合テスト段階で見つかった設計起因の指摘件数」を数えてみました。

• 変更前の3ヶ月：設計起因の後工程指摘が11件
• 変更後の3ヶ月：同 4件

もちろん案件の中身が違うので厳密な比較ではないんですが、「レビューで前提を疑う」文化が入ったことで、後工程まで漏れる設計の穴が明らかに減りました。今回の全件フェッチも、以前なら本番で気づいて慌てて直すパターンだったはずです。

振り返って

一番の学びは、レビューの価値は指摘した人の経験年数じゃなくて、その場に「前提を共有していない目」があるかどうかで決まる、ということでした。ベテランだけで固めたレビューは、速いけれど前提ごと見落とす。新人の「何で？」は、その前提を外から照らしてくれるライトなんですよね。

今後は、輪番の自動割り当てと、[ask] に対する返答の質（ちゃんと理由を言語化できているか）を、もう少し仕組みとして支えていきたいと思っています。レビュー文化って、放っておくと「強い人の意見が通る場」に戻りがちなので、運用で押し続けないといけないな、と。

もっと良いレビュー運用があったら、ぜひ教えてください。