レビュー担当者をレイヤーごとに分割:コードの読み方に合わせたコードレビュー
One reviewer per layer: splitting code review by the way code is read (djordjepuzic.com)
要約
Claude Codeチームは、AIによるコード生成量の増加に対応するため、コードレビューを「レイヤー」と「レンズ」に分割する新しいシステムを導入しました。このシステムでは、コードの意図、形式、ローカルな意味、連携、実行時、利用方法といった抽象化のレベルごとにレビュー担当者を割り当て、正確性、セキュリティ、履歴、可逆性といった観点からコードを評価します。これにより、レビュー担当者の負担を軽減し、コードの品質と安全性を向上させることを目指しています。
全文翻訳
レビュー担当者をレイヤーごとに分割:コードの読み方に合わせたコードレビュー
[画像: 5つの積み重ねられたコードのレイヤーが、正確性、セキュリティ、履歴、可逆性という4つの色のレンズで貫かれている]
Addy Osmani氏が、Claude Codeチームが現在どのようにコードレビューを行っているかについて共有しました。Claudeがコードの大部分を生成するようになったため、レビュー方法も変更を余儀なくされたのです。彼らのモノレポでは、マージされる変更の数は5月の日量約2,500件から9月には30,000件以上に増加しました。誰もがこれを一行ずつ読んでいるわけではありません。Claude Codeリポジトリでは、6月から9月にかけてマージされたプルリクエストが倍増し、リバートが必要になった割合は40%以上減少しました。
彼らはレビューをやめたわけではありません。レイヤー分けしたのです。変更は6つのチェックレイヤーを通過し、最後のレイヤーの後ろにはロールバックメカニズムが配置されています。各レイヤーは一部のバグを検出し、他のバグを見逃します。6つすべてをすり抜けるバグはほとんどありません。
私はこのシステムを非常に気に入っています。そのうちの一つが、私に執筆させたいと思わせました。
彼らのレイヤーはステージである
6つのレイヤーは時間の経過におけるポイントです。プルリクエストの前に検証が行われ、プルリクエスト上でAIレビュー、次にブロックするセキュリティレビュー、その後CI、そしてリスクに応じてスケーリングされた人間の承認、マージ、段階的なロールアウト、そしてリバートが行われます。変更は、道路のゲートのように、順番にそれらを通過します。
AIレビューゲート内では、一人のレビュアーに変更が問題を解決しているか、より大きなシステムに適合しているか、コードベースの他の部分を壊していないかを確認するように求められます。複数のエージェントが並列で検索し、発見したことについて投票します。しかし、それぞれがコード全体を、行の文字から周囲のシステムまで、すべて読むように求められています。
それは一人の読者には多すぎます。人間のエンジニアは、プロダクトの意図、サービスの境界、nullチェックのエッジケース、構文ルールを同時にすべて把握しているわけではありません。私たちはズームインとズームアウトをします。一つの関数を注意深く読み、次に誰がそれを呼び出しているかを見るために後退し、さらに誰がそれを使用しているかを見るために再び後退します。すべてを一つのコンテキストで持つ単一のエージェントは、ズームすることができません。抽象化のすべてのレベルが同じウィンドウにあり、同じ注意力を競合します。コードの記述と読解は一つの規律ではありません。それは互いに積み重ねられたいくつかの規律であり、それぞれが異なるメンタルモデルを必要とします。
[画像: レビューパイプラインは6つのステージを左から右へ実行します。AIレビューのステージ内では、一人の読者が5つのレイヤーすべてを把握しています:利用、実行時、連携、ローカルな意味、形式。]
コードには意味のレイヤーがある
プログラミングに関する最も古い不満を取り上げましょう。セミコロン、ドット、スペースを一つ変更するだけで、プログラムは壊れます。コードは正確な形式を要求します。それは明白な最初のレイヤーのように見えます。
その通りですが、それは二つに分かれます。変更された文字がコードの解析やコンパイルを妨げる場合、コンパイラがそれをミリ秒単位で、毎回、無料で検出します。エージェントがそこにトークンを費やすべきではありません。Claude Codeチームは、レビュアーが同じ問題を繰り返しフラグを立てる場合は、リンティングルールでソースで修正すると同じことを言っています。
危険な変更は、それでもコンパイルできるものです。 <= の代わりに <。 == の代わりに =。 a?.b の代わりに a.b。 await の欠落。形式は有効です。意味が移動しました。そのバグはまったく形式のレイヤーには存在しません。それは一つ上のレイヤーに存在します。
したがって、レイヤーは文字がどこに座っているかではありません。それらは、それを判断するために理解する必要があることに関するものです。
6つのレイヤー
レイヤー0:意図。 この変更は存在するべきか、そしてそれが主張する問題を解決しているか? Claude Codeチームはこれを人間向けに保持しています:そもそも何かを構築するかどうかを決定します。エージェントは、差分がその説明と一致しているかという、より狭い質問をチェックすることはできます。
レイヤー1:形式。 構文、型、フォーマット、命名、プロジェクトの規約。そのほとんどは決定論的なツールに属します:コンパイラ、型チェッカー、リンター、フォーマッター。エージェントは、ツールでは表現できない規約に対してのみ有用です。
レイヤー2:ローカルな意味。 このコード片が単独で何をするか。関数はその名前と契約が約束することを行っているか? エッジケース、空の入力、null、オフバイワン、エラーパス、そして関数がコストをかけるもの:ブロックする呼び出し、ループ内のクエリ。このレイヤーの読者は、関数とその契約だけを必要とします。ルーティングテーブルやプロダクトロードマップを見る必要はありません。単体テストがその証拠です。
レイヤー3:連携。 この部分がシステムにどのように適合するか。誰がそれを呼び出すか、それが何を呼び出すか、境界を越えるデータの形状、API互換性、データベースマイグレーション、設定。関数はそれ自体で正しい場合でも、すべての呼び出し元の仮定を壊す可能性があります。この読者は、本体ではなく、境界を必要とします。
レイヤー4:実行時。 システムが時間とともに実行されるときに何が起こるか。並行性、共有状態、リトライ、タイムアウト、パフォーマンス、障害モード、フィーチャーフラグ、デプロイ順序。レイヤー3は、部分が接続されているかを尋ねます。レイヤー4は、それらが動き始めたときにどのように振る舞うかを尋ねます。それらは異なるメンタルモデルなので、異なる読者が割り当てられます。
レイヤー5:利用。 人々が実際にシステムをどのように使用するか、そしてこの変更が彼らに何をするか。ワークフロー、デフォルト、ユーザーエクスペリエンス、そして現在間違っているドキュメントと設定。Claude CodeチームのREVIEW.mdガイダンスは、このレイヤーからの2つの失敗を挙げています:ドキュメントは嘘をついてはならず、設定は誤ったルーティングをしてはなりません。
一部の懸念事項は一つのレイヤーに収まりません。セキュリティが最も明確なケースです。インジェクションバグは文字列の形式に、壊れた権限チェックはローカルな意味に、信頼境界でのチェック漏れは連携に存在し得ます。セキュリティはレイヤーではありません。それはすべてを横断します。私はそれを以下で再び取り上げます。それはレイヤーをリストよりも有用なものに変えるからです。
線ではなく木
各レイヤーに独自の担当エージェントを割り当てると、エージェントは木を形成します。
葉では、レイヤー2のエージェントが1つの関数または1つのファイルを読みます。それはシステム全体については何も知りませんが、それがポイントです。その上では、レイヤー3のエージェントが各モジュールの境界を読みます。それは関数本体を決して見ません。それは葉が報告することを見ます:各部分が何をするか、何を受け入れるか、そして何を返すかの短い契約。ルートでは、1つのエージェントがシステムとその利用方法を保持します。
[画像: レビューエージェントの木。葉にある関数エージェント、モジュールごとの連携エージェント、実行時エージェント、ルートにある利用エージェント。質問は下に、契約は上に移動し、競合は葉からルートへエスカレートします。]
情報は契約として、生のコードではなく、上に移動します。これにより、各エージェントのコンテキストは小さく保たれます。Claude Codeチームはすでに、新鮮なコンテキストを持つサブエージェントが、コードを書いたエージェントよりもレビューが優れていることを発見しています。木はこの考えをさらに一歩進めます。各読者は新鮮で狭いコンテキストを得ます。
これは古い考えです。1972年にDavid Parnasは"On the Criteria To Be Used in Decomposing Systems into Modules"を発表しました。彼のルールは情報隠蔽でした。各モジュールは変更されやすい設計上の決定を隠し、システムの残りはそのインターフェースのみを見ます。そのポイントは、システム全体を頭に入れておく必要なしに、一部を変更できることでした。
私たちはコードでそのルールに従い、次にプロンプトでそれを破ります。diff、周囲のファイル、そして抽象化のすべてのレベルを一つのコンテキストウィンドウに入れ、モデルがそれらを整理してくれることを期待します。エージェントの木は、レビュアーのための情報隠蔽です。連携エージェントは、関数がリストをどのようにソートするかには関心がありません。その契約が約束することだけに関心があります。関数エージェントはルーティングテーブルによって邪魔されません。各読者は、それが何であるかによって定義されるのと同じくらい、何が見えないかによって定義されます。
木を下り、そして戻る
木は両方向に機能します。
トップダウンでは、一つの変更が広がります。システムへの変更は複数のモジュールの境界での変更となり、各境界は複数の関数への変更となります。ルートは利用から始まり、質問を下にプッシュします:このエンドポイントは現在一つのケースでnullを返しますが、どの呼び出し元が壊れ、そのうちのどれかがクラッシュしますか?
ボトムアップでは、ローカルな事実がシステム判断に逆流しますが、兄弟ノードにこぼれることはありません。例えば、レイヤー2の葉が、ヘルパーが同期ネットワーク呼び出しを行うようになった、またはループ内でクエリを実行するようになったことに気づいたとします。ローカルでは、関数はまだ正しい値を返します。葉は、そのコストが重要かどうかを知る必要はありません。それは、上に渡す契約にその事実を書き込みます。レイヤー4の実行時読者は、その関数がホットパス上にあることを知っています。レイヤー5の利用読者は、そのパスがチェックアウトページであり、それがハングするようになることを知っています。各レイヤーは、自身のメンタルモデルが見ることができる部分だけを回答します。
[画像: 一つの関数で見つかったブロックする呼び出しが、契約の事実として上に移動します。実行時レイヤーはそれがチェックアウトのホットパス上にあることを認識します。利用レイヤーはチェックアウトページがハングすることに気づきます。兄弟関数には決して伝えられません。]
同じパスが、葉が解決できない問題も運びます。関数が不変条件を壊さずに契約を維持できない場合、葉は即興で回避策を講じるべきではありません。それがローカルパッチが静かにアーキテクチャを壊す方法です。それは問題を、決定を所有するレイヤー、そして最終的には意図を所有する人々に、上に報告します。
その双方向のパスが、各レイヤーで正しいがユーザーにとっては間違っている変更を捉えるものです。
レイヤー×レンズ
セキュリティはレイヤーを横断する唯一の懸念事項ではありません。履歴も同様です:コードがなぜそのように見えるのか、そしてそれがどのような決定を運んでいるのか。可逆性も同様です:問題が発生した場合にどのように変更を元に戻すか。これらをレンズと呼びましょう。レイヤーは、あなたが何を読んでいるかを言います。レンズは、あなたがそれを何のために読んでいるかを言います。
上記の6つのレイヤーはすでに1つのレンズでした。私はすべてのレイヤーを正確性のために読んでいましたが、それを名前付けしていませんでした。レイヤーを行に、レンズを列に配置すると、行列が得られます。意図はそれから外れます。なぜなら、それは人間に任されるからです。
| レイヤー | 正確性 | セキュリティ | 履歴 | 可逆性 |
|---|---|---|---|---|
| 形式 | 解析、型チェック、規約に従っているか? | 機密情報、安全でない文字列構築、禁止API? | コードベースが廃止したパターンを復活させているか? | 差分はクリーンに元に戻せるほど小さく、孤立しているか? |
| ローカルな意味 | 関数はその名前が約束することを行っているか? | 入力を検証し、安全に失敗するか? | 意図的な以前の修正を元に戻しているか? | 保存されたデータを元に戻せない方法で変更しているか? |
| 連携 | 呼び出し元の仮定はまだ有効か? | すべての信頼境界がチェックされているか? | 記録されたアーキテクチャの決定を壊しているか? | 古いバージョンと新しいバージョンを並行して実行できるか? |
| 実行時 | レースコンディション、リトライ、タイムアウト? | リソース枯渇、レート制限、ログ内の機密情報? | この種の変更が以前にインシデントを引き起こしたことがあるか? | デプロイなしでオフにできるか? |
| 利用 | ワークフローはまだ機能しており、ドキュメントはまだ真実か? | ユーザーが本来できないことを見たり、したりできるようになったか? | ユーザーは古い動作に依存しているか? | ロールバック後、ユーザーはデータを回復できるか? |
各セルは独自のメンタルモデルです。エージェントにとって、各セルは独自のクエリと独自のコンテキストでもあります。そして、それらは異なる方向から来ます。
行がロードするものを決定します。ローカル意味読者は1つの関数とその契約をロードします。連携読者はシグネチャとその呼び出し元をロードします。実行時読者は設定、フラグ、デプロイ順序をロードします。利用読者はドキュメント、ワークフロー、テレメトリをロードします。
列が尋ねることを決定します。セキュリティレンズは脅威モデルをもたらします。履歴レンズはgit blame、過去のプルリクエスト、決定ログをもたらします。可逆性レンズはマイグレーションとロールアウト設定をもたらします。
したがって、誰も20個のエージェントを構築する必要はありません。5つのコンテキストリトリーバー(行ごと)と4つのレンズプロンプト(列ごと)を構築します。各セルはそれらの組み合わせです。9つの部品で20個の狭い読者が得られます。新しいレンズ、パフォーマンスやアクセシビリティは、1つのプロンプトの価格で完全な列を追加します。
[画像: 5つのコンテキストリトリーバー(行ごと)と4つのレンズプロンプト(列ごと)が交差して20個の狭いレビュアーを作成します。形式行はツールに属します。ハイライトされたセル、セキュリティレンズ下の連携は、すべての信頼境界がチェックされているかを尋ねます。]
履歴列は弱い部分です。git blameは誰がいつ行を変更したかを示します。それは理由を示しません。変更が記録されたアーキテクチャの決定を壊しているかどうかを尋ねるセルは、その決定がエージェントがクエリできる場所に記録されている場合にのみ機能します。ほとんどのチームは、その理由をチャットスレッドまたは一人の頭の中に保持しています。行列はそのギャップを可視化します:それは弱いモデルではなく、空の入力です。
壊れる可能性のある場所
継ぎ目がリスクです。バグはレイヤー間に住みたがります。そこでは各読者が他の読者がチェックしたと思い込みます。葉が上に渡す契約は、ツリー全体で最も重要な成果物です。それが曖昧であれば、上のレイヤーはコードの代わりに要約をレビューします。
行列は第二の種類の継ぎ目を追加します。変更は連携行では正しく見えても、セキュリティレンズの下でのみ危険である可能性があります。2つのセルが異なる角度から同じ行について報告する場合、その重複はしばしば実際のバグがある場所です。コーディネーターはそれを重複として破棄するのではなく、シグナルとして扱うべきです。
エージェントが増えるとノイズも増えます。Claude Codeチームは、あるプルリクエストが、エージェントの作成者とエージェントのレビュー担当者の間で、一晩で50回以上サイクルしたのを見ました。レビュー担当者がフラグを立て続けたものをチェックしたところ、その約59%は最初のパスから存在していたコードでした。彼らは現在、3回のサイクル後に作成者にアプローチを再考するように求めており、それ以降のレビューは新しい差分に限定しています。レビュアーの木は、同じ規律を、より厳しく適用する必要があります:重複する発見を削除する1人のコーディネーター、何が本物かについての投票、そしてラウンドのキャップ。
コストが3番目の制限です。1行のコピー変更には20人の読者は必要ありません。実際には、行列はまばらです。形式行のほとんどすべては、コンパイラ、リンター、シークレットスキャナーに属し、エージェントには属しません。彼らのリスク分類子は、変更にどれだけの人間のレビューが必要かをすでに決定しています。同じ分類子が、どのセルを実行するかを決定できます。ドキュメントの修正は1つか2つです。請求または自動更新への変更は、完全なグリッドを取得します。
テストする方法
Claude Codeチームは、インシデントを引き起こしたプルリクエストのコレクションを保持しており、毎日そのコレクションに対してレビュアーをスコアリングしています。このコレクションはこのアイデアのテストにも適しています。
各インシデントを、バグが実際に存在したセル:そのレイヤーとレンズでラベル付けします。ほとんどが連携と実行時の行にある場合、すべてを読む単一のレビュアーは、損害が発生している場所で正確に薄く広がっています。列全体が空のままである場合、そのレンズはすでにカバーされているか、誰もそれを見ていないかのどちらかです。ラベルは、最初に構築するセルも示します。
Claude Codeチームはパイプラインをレイヤー化しました。私も読者をレイヤー化します。