eng-docs

レビュー

13 min read

レビュー

最初に壊れるのは変更の粒度。 既存のレビュー規範はどれも「人間が書く速さ」を前提に釣り合っていて、 その前提が外れると真っ先に崩れるのがここ。

元の規範

コードレビューの進め方と コードレビュー(書籍第9章)。

とくに効いてくるのが、 小さな変更にするの基準。

大雑把には100行の変更は適度なサイズで、1000行になると大きすぎると言えます
50ファイルにもわたる変更であれば普通は「大きすぎる」と判断される
機能変更やバグ修正と、リファクタリングは別の変更にするのが普通はベスト

何が崩れるか

100行が数分で出る

この基準は「書くのに何時間もかかる」ことで自然に守られていた。 1000行書くのが大変だから、1000行の変更が来なかった。

AI は数分で出す。しかも品質は落ちないので、 「大きいから雑だろう」という間接的なシグナルも効かない。

元の規範が挙げている、小さい変更の恩恵を並べ直すと、 AI 駆動では効く理由が変わっているものがある。

恩恵AI 駆動での効き方
速くレビューできる**変わらず効く。**レビューする側は人間のまま
隅々までレビューできる**より効く。**レビューが唯一の関門になるため
バグが混入する可能性が減る**効き方が変わる。**タイプミス由来は減り、前提の取り違え由来が残る
変更が却下されても無駄になる作業が少ない**効かなくなる。**作り直す費用が下がったので、捨てるのが安い
ロールバックしやすい最も効く。ここが唯一のブレーキになる

「作業が無駄になる」を理由に大きい変更を許容しない、という論法は使えなくなった。 残る理由はレビューできるかどうかと戻せるかどうかの2つに絞られる。

説明がいくらでも出てくる

元の規範には「顧客は常に正しい」がある。

コードの意味が分からないという質問は、一つ残らず妥当なものとして扱う

これは「批評に応じてアプローチやロジックを変えろ」という意味ではない。 自分のコードをもっと明確に説明する必要があるかもしれない、という意味。

(出典: コードレビュー(書籍第9章))

AI 相手だと、この規範の後半だけが空回りする。 質問すれば必ず説明が返ってくるし、その説明はたいてい筋が通っている。 説明が出てきたことは、コードが正しいことの証拠にならない。

区別する基準は1つ。

コードを見て言えることか、コードの外から持ってきた話か

「この分岐は N が負のときのため」は前者（コードで確かめられる）。 「一般にこのパターンが推奨されている」は後者で、レビューの根拠にはならない。

「まったく新しいコード」の比率が上がる

変更の種類ごとの見どころは 4種類に分けて、種類ごとに見る場所を変えろと言っている。

AI 駆動では、この4種類の分布が新規側に寄る。 既存コードを読んで最小限直すより、書き直す方が安くなる場面が増えるため。

元の規範では、新規コードは設計レビューの比重が大きくなる種類だった。 つまりレビューの重心が、行単位の指摘から設計の妥当性へ移る。

アンチパターンの一部が、そのままでは当てはまらない

元の規範はアンチパターンをこう挙げている。

どんな変更に対してもレビュアーがいちいち難色を示する
※コードの隅々まで磨きをかけることを要求すべきではない
※開発者は改善を行う意欲を失う

**理由が「開発者が意欲を失うから」になっている。**AI は意欲を失わない。

だからAI が書いた分については、この遠慮は要らない。 納得いくまで指摘して直させてよい。

ただし他の3つはそのまま生きている。

細かい点(リファクタ・変更とは関係ない既存ソースの改善)から指摘する
指摘がシステムのコードの健康状態を悪化させる
指摘によりコードがより複雑になる

これらは書き手の感情ではなくコードベースの状態についての話なので、 書き手が誰であっても同じように成り立つ。

どう仕掛けるか

粒度は依頼の側で決める

出てきた差分を後から分割するのは高くつく。 着手前に「この変更は何行くらいになるか」を言わせて、大きすぎるなら分けて頼む。

機械が見られるものを人間から外す

静的解析の主張が、 ここで効き方を変える。

ありふれた問題を自動で目立たせることで、レビュアーの時間を節約できる

自動的に修正できるものは、すべて自動的に修正されるべき

元は「レビュアーの時間の節約」という費用の話だった。 AI 駆動では、レビュー帯域が全体の律速になるので、これは費用ではなく制約になる。

同じ節にある実質的誤検出の考え方も、そのまま使える。 AI がその指摘を受けて何も直さないなら、そのルールは実質的誤検出として外す。

差分の説明を、形式を決めて書かせる

元の規範の ディスクリプションの形式を そのまま使う。1行目に要約、3行目以降に「どうしてこれが最良の方法なのか」。

AI 駆動では、ここに1項目足す価値がある。「確かめていないこと」の欄。 書いてあれば、レビュアーはそこを重点的に見る。

自動生成された変更として扱ってよい場面がある

元の規範は リファクタリングと大規模変更(LSC)で、 自動生成された変更のレビュー範囲を限定してよいと言っている。

  • 見るのは自分たちのコードに特有の懸念点だけ
  • 変更を生成するプロセス全体は既にレビュー済み

同じ扱いができるのは、生成の指示そのものをレビューできる場合だけ。 「この置換を全ファイルに適用して」と頼んだ結果なら、指示がレビュー対象になる。 「いい感じに直して」の結果は当てはまらない。指示が具体的でないと、この省力化は使えない。

そのまま貼る

CLAUDE.md に足す。

## 変更の出し方
 
- **1つの変更は1つのことだけをする。** 機能変更・バグ修正・リファクタリングを混ぜない。
  混ざりそうになったら、混ぜずに順番にやる
- **着手前に規模を見積もって伝える。** 200行を超えそうなら、
  分けられないか提案してから始める
- **変更の説明は次の形式で書く。**
  1行目に何をしたかの要約、空行、それ以降に「なぜこの方法が最良か」「確かめていないこと」
- **「確かめていないこと」を必ず書く。** 実行して確認していない箇所、
  前提を置いた箇所、影響範囲を追い切れていない箇所を挙げる。空なら「なし」と書く
- **リンター・フォーマッター・型チェックが通る状態にしてから報告する。**
  機械が指摘できるものを人間のレビューに回さない
- **既存のコードと同じ書き方に合わせる。** 自分の流儀ではなく、
  周囲のコードの命名・構造・コメントの密度に寄せる
- **指摘されたら、同意できないときは理由と代案を出す。** 黙って書き換えない

レビューする側（人間）が見る場所は、既存の レビューチェックリストに足す形で、 このツリー特有のものだけを挙げる。

## AI が書いた変更を見るとき
 
- 説明ではなくコードを見る。もっともらしい説明は根拠にならない
- 呼び出し元・参照元が全部追われているか（変更の外にある使用箇所）
- 頼んでいないものが増えていないか（設定項目、抽象化の層、使われない分岐）
- テストが実装をなぞっているだけになっていないか
- 実在しない API・パッケージ・オプションを使っていないか
- 「確かめていないこと」に書かれた箇所を重点的に見る

関連

バックリンク