二人のレビュアー、一つの差分、ゼロの重複

二人のシニアエンジニアが同じプルリクエストをレビューする。片方が欠落している null チェックを指摘する。もう片方がクリーンアップパスでの競合状態を発見する。どちらも両方を見つけられない。

一人のレビュアーだけを割り当てていたら、そのバグの片方が出荷されていた。これはスキルの差ではない。人間の注意力の予測可能な特性であり、Michael Fagan が1976年にIBMで記録したものだ。

Fagan はIBMのソフトウェアパイプラインでの欠陥検出率を測定していた。彼のデータは不快な事実を示した。経験豊富な検査者であっても、グループが最終的に発見した欠陥全体のごく一部しか検出できなかった。真の価値は、個人の専門知識にあったのではない。複数の視点を構造化して組み合わせることにあった。

今日のほとんどのチームは、その組み合わせを構造化していない。シニアエンジニアが会議の合間に差分をざっと読み、スタイルの問題に気づき、承認して先に進む。次のレビュアーも同じことをする。二人とも、来週の火曜日に本番データを破損させるオフバイワンエラーを見逃す。

Fagan はこれを非構造化レビューシンドロームと呼んだ。グループには目があるが、プロセスがない。

Fagan インスペクションとは実際には何か

Fagan インスペクションは、人々が一緒にコードを読んで感情を共有する会議ではない。定義された役割、エントリ条件、測定可能な成果を持つ形式的なプロセスだ。Fagan は、非構造化のレビューが時間を浪費し、レビューなしとほぼ同じ割合で欠陥を漏らすことを知り、これを設計した。

核心の洞察は役割の分離だ。各人は正確に一つの仕事を持つ：

  • モデレーターは会議を進行し、ルールを強制する。彼らは検査しない。
  • リーダーはコードを声に出して言い換える。これにより、グループは作成者が意図したことではなく、コードが実際に何をしているかに直面させられる。
  • テスターは実行パス、境界条件、カバレッジの欠落について考える。
  • 作成者は質問に答えるが、コードを擁護しない。

この分離は、グループレビューで最も一般的な失敗モード、すなわち作成者が全員を自分の懸念から説得してしまうことを防ぐ。

作成者が説明者でもある場合、彼らは曖昧さをなだめすかす。「ああ、その変数は常に呼び出し元によって設定されるんだ。」グループは頷く。誰も確認しない。リーダーの役割はこの習慣を打ち破るために存在する。リーダーが一つの文で関数を言い換えられないなら、その関数は出荷する準備ができていない。

なぜチェックリストが直感に勝るのか

Fagan は検査チェックリストも導入した。これらはスタイルガイドからコピーされた一般的なコーディング標準ではない。レビュー対象となる成果物の特定のタイプに合わせて調整されている。

ステートマシンのチェックリストは問う：すべての遷移を処理したか？リソースアロケーターのチェックリストは問う：すべてのパスで、すべての割り当てに解放が対になっているか？

チェックリストが存在するのは、人間の注意力が偏っているからだ。一万のデータベースクエリを書いた専門家であっても、BEGIN TRANSACTIONブロックを精神的に飛ばしてしまう。彼らの脳はそれを正しいと自動補完する。Fagan は、チェックリスト主導の検査者が、直感主導のレビュアーが見過ごした欠陥を発見したことを突き止めた。専門家が不注意だったからではなく、専門知識が盲点を生み出すからだ。

以下は、単一の Python 関数用の軽量チェックリストだ：

CHECKLIST = [
    "Can a non-author paraphrase what this function does in one sentence?",
    "Does every execution path return or raise predictably?",
    "What happens at the minimum and maximum valid inputs?",
    "What happens at exactly one step past the boundary?",
    "Does the function mutate any argument, closure, or global state?",
    "Is every resource acquired also released on the error path?",
    "If this raises, can the caller distinguish recoverable from fatal?",
]

これは官僚主義ではない。体系的な注意を促す強制機能だ。

Fagan はインスペクションを4つのフェーズに分けた。会議は最も短いものだ

本物の Fagan インスペクションには4つのフェーズがあり、会議自体は最も短いものだ。

**準備。**すべての検査者は、グループが会う前に、チェックリストを持って一人で資料をレビューする。Fagan は、準備をした検査者が、準備なしで入ってきた検査者の約2倍の欠陥を発見したことを突き止めた。会議は、発見を組み合わせるためだけに存在し、それを生成するためではない。

**会議。**リーダーがコードを歩く。テスターがもしもの質問をする。モデレーターが2時間以内に抑える。作成者がメモを取る。会議中に誰もコードを修正しない。欠陥は記録され、グループは先に進む。

**修正。**作成者が記録された欠陥を一人で修正する。

**フォローアップ。**モデレーターがすべての欠陥が対処されたことを確認する。大きな修正では、2回目のインスペクションが必要になる場合がある。

この構造は、現代のプルリクエストには重苦しく感じる。Fagan は、単一の欠陥が数百万ドルの損失をもたらしうるメインフレームソフトウェア向けにこれを設計した。しかし原則は、より小さな文脈にも適応しうる。

やりすぎになる場面

Fagan インスペクションはタダではない。準備時間だけでもかなりのオーバーヘッドを追加する。10行のバグ修正に対して、完全な Fagan インスペクションは馬鹿げている。欠落しているインポートを見つけるのに、4人とチェックリストは必要ない。

ペイオフ曲線は非線形だ。Fagan のデータは、インスペクションが最も効果を発揮するのは、複雑で高リスクなモジュール、すなわちステートマシン、パーサー、リソースマネージャー、非ローカル状態や微妙な順序制約を持つあらゆるものだと示唆した。CRUD ハンドラーやボイラープレートテストには、非公式のレビューで十分だ。

真の間違いは、すべての変更に同じレビュー戦略を適用することだ。ログメッセージのタイポには Fagan インスペクションは必要ない。分散トランザクションコーディネーターには、おそらく必要だ。

今日使える軽量版

IBM の会議文化を持っていなくても、ほとんどのメリットを得られる。以下は、現代のチームに機能する軽量な適応版だ：

  1. **グループディスカッションの前に個別レビューを義務化する。**すべてのレビュアーは、誰かが話す前に書面コメントを提出する。これにより、最初の大きな意見が支配するのを防ぐ。

  2. **リーダーの役割をローテーションする。**誰かが批判する前に、一人のレビュアーに変更を自分の言葉で要約してもらう。できなければ、その変更は大きすぎるか、不明瞭すぎる。

  3. **チームチェックリストを構築する。**上記の7つの質問から始める。ドメイン固有の項目を追加する。四半期ごとに見直す。

  4. **作成者と擁護者を分離する。**作成者は事実に基づく質問に答える。コードが問題ないと主張しない。レビュアーが混乱しているなら、それは議論ではなくデータだ。

  5. **欠陥を記録し、後で修正する。**レビュー中にコードを書き換えない。問題を記録し、終了してから修正する。

以下は、任意の Python モジュールのレビューチェックリストを生成する簡単なスクリプトだ：

import ast
import sys
from pathlib import Path

def generate_checklist(source_path: str) -> list[str]:
    """Generate a Fagan-style checklist from a Python module."""
    source = Path(source_path).read_text()
    tree = ast.parse(source)

    checklist = [
        f"Module {Path(source_path).name}: {len(tree.body)} top-level statements",
        "Can a non-author state the module's responsibility in one sentence?",
    ]

    for node in ast.walk(tree):
        if isinstance(node, ast.FunctionDef):
            checklist.append(
                f"Function '{node.name}': does every path return or raise?"
            )
            if any(isinstance(n, ast.Try) for n in ast.walk(node)):
                checklist.append(
                    f"Function '{node.name}': is every exception handled explicitly?"
                )

    return checklist

if __name__ == "__main__":
    for item in generate_checklist(sys.argv[1]):
        print(f"[ ] {item}")

以下のように実行する：

python inspection_checklist.py src/transaction.py

これはバグを見つけてくれるわけではない。適切な場所を見るよう強制してくれる。

より優秀な人材を採用しても、プロセスの問題は修正されない

Fagan の研究は50年近く前のものだが、その発見は変わっていない。個人のレビュアーは一貫性がない。グループもまた、構造化されない限り一貫性がない。レビュアー間の分散は排除すべき問題ではない。組織化すべきリソースだ。

次に、一人のレビュアーが他の人が見逃したバグを見つけたとき、誰が優れているかと問うのではなく、プロセスが両者が見ているものを組み合わせるのに十分構造化されているか問うべきだ。Fagan はすでに採用でこれを解決しようと試みた。それは機能しない。

FAQ

Fagan インスペクションとは何か？

Fagan インスペクションは、1976年にIBMの Michael Fagan によって開発された、形式的で構造化されたコードレビュープロセスだ。定義された役割（モデレーター、リーダー、テスター、作成者）、準備要件、チェックリストを使用して、ソフトウェア成果物の欠陥検出を最大化する。

なぜ異なるレビュアーが異なるバグを見つけるのか？

人間の注意力は選択的だ。専門家は、コードを素早く読むための精神的ショートカットを発達させるが、その同じショートカットが盲点を生み出す。異なるレビュアーは異なるバックグラウンドと認知パターンを持つため、彼らの盲点は完全には重複しない。Fagan の研究は、インスペクションの価値は、完璧なレビュアーを見つけることではなく、複数の不完全な視点を組み合わせることにあることを示した。

Fagan インスペクションは今日でも使われているか？

完全な形式プロセスは、航空宇宙や医療機器のような安全が重要な産業以外では珍しい。しかし、個別準備、役割分離、チェックリスト主導のレビューという根本的な原則は、高性能ソフトウェアチームによってますます採用されている。核心的なアイデアは、構造化されたウォークスルーや形式的技術レビューのような現代的なプラクティスにも影響を与えた。

完全な Fagan インスペクションのオーバーヘッドは、いつペイするか？

欠陥が重大な影響を及ぼすモジュールにおいて：分散コンセンサスロジック、セキュリティ境界、リソースライフサイクル、ステートマシン。日常的な変更には、軽量な適応版で通常十分だ。レビューの厳格性を実際のリスクに合わせ、すべての差分に同じプロセスを適用するのではない。