大半のレビューチェックリストはプラシーボだ

チームにコードレビューのチェックリストがあるなら、誰も開かないWikiページに埋もれている可能性が高い。おそらく「check for off-by-one errors」や「verify error handling」といった内容が書かれている。これらは真実だ。しかし、行動を変えるにはあまりにも曖昧すぎる。

「check for bugs」を確認しろと言うチェックリストは、チェックリストではない。バグが存在することを思い出させるだけのものだ。

Fagan inspectionにおけるチェックリストは、異なる目的を果たす。心配すべきことのリストではなく、レビュー対象の実際のコードベースで観測された特定の欠陥カテゴリーにレビュアーの注意を向ける構造化されたツールだ。データから構築され、アーティファクトの種類に合わせて調整され、義務付けられた個人準備の際に使用される。正しく使用されれば、構造化されたインスペクションが非公式レビューの3〜4倍の欠陥を発見できる主な理由の一つとなる。

チェックリストを「構造化」するものは何か

ここで「構造化」という言葉が重要だ。構造化されたチェックリストは、より長いチェックリストではない。特定の設計特性を持つチェックリストだ。

第一に、それは逸脱した欠陥データから導出される。項目は、一般的なベストプラクティス文書ではなく、実際にプロダクションに流入したバグから来る。最近の3つのインシデントがすべて非同期クリーンアップにおけるrace conditionsを伴っていた場合、そのカテゴリーは独自のチェックリスト項目を得る。null dereferencesが2年間問題になっていないなら、その項目は削除される。

第二に、それはレビュー対象のアーティファクト種別に限定される。Fagan inspectionは当初、要件文書、設計文書、ソースコード、テスト計画に対して異なるチェックリストを使用した。各アーティファクトには異なる欠陥カテゴリーがある。設計文書のチェックリストは、インターフェースの一貫性や結合度を問う。ソースコードのチェックリストは、境界条件やリソースのクリーンアップを問う。両方を混ぜ合わせると、両方が薄まる。

第三に、それは会議中ではなく、個人準備の際に使用される。各インスペクターは、グループが集まる前に、チェックリストを持って一人で資料を読む。チェックリストは、各人がコードを孤立して読む際に何を見るかを形作る。共有の参照文書ではない。個人のレンズだ。

第四に、それは使いやすいほど短い。40項目のチェックリストは、ツールではなくカタログだ。Faganは、アーティファクト種別あたり概ね10〜15項目を推奨した。この制約が優先順位付けを強制する。重要なカテゴリーを残し、ノイズを捨てるのだ。

自社のデータから構築する方法

構造化チェックリストを構築する最良の方法は、すでに何が間違ったかを見ることだ。以下は実践的なプロセスだ。

インシデントトラッカー、バグデータベース、またはポストモーテムノートから始めよう。レビューを逃れてプロダクションやテストに到達した最新20件の欠陥を抽出する。各件を症状ではなく根本原因で分類する。「Page crashed」は症状だ。「Missing null check after external API response」は根本原因だ。

根本原因をカテゴリーにグループ分けしよう。おそらく、逸脱した欠陥の80%が3〜5のカテゴリーに収まることがわかるだろう。それがチェックリスト項目だ。

各カテゴリーに対して、曖昧な思い出しではなく、具体的なトリガー質問を書く。「Check for nulls」は曖昧だ。「For every function that calls an external API, verify the response is validated before use」はトリガー質問だ。レビュアーに、何を探し、どこで探すべきかを正確に伝える。

具体的な例を挙げよう。チームがPythonサービスを提供し、直近20件のプロダクション障害を分析したとする。以下のような分布が見つかったとする:

  • 6件:外部呼び出しでのエラーハンドリング欠如
  • 5件:データベーストランザクション境界の誤り
  • 4件:ページネーションロジックにおけるoff-by-one
  • 3件:キャッシュデータにおけるrace conditions
  • 2件:機密データのログ記録

チェックリストは5項目あり、カテゴリーごとに1項目ずつあるべきだ。各項目は、自社の特定のパターンに結びついたトリガー質問であるべきだ。

#!/usr/bin/env python3
"""Build a structured review checklist from escaped defect data."""

from collections import Counter
from dataclasses import dataclass
from typing import List


@dataclass
class EscapedDefect:
    id: str
    root_cause: str
    trigger_question: str


def build_checklist(defects: List[EscapedDefect], max_items: int = 10) -> List[str]:
    """Build a Fagan-style checklist from escaped defect data.

    Groups by root cause, sorts by frequency, and returns trigger
    questions for the top categories.
    """
    counts = Counter(d.root_cause for d in defects)
    top_causes = counts.most_common(max_items)

    # Map root causes back to their most representative trigger question
    cause_to_question = {}
    for d in defects:
        if d.root_cause not in cause_to_question:
            cause_to_question[d.root_cause] = d.trigger_question

    checklist = []
    for cause, count in top_causes:
        question = cause_to_question[cause]
        checklist.append(f"[{count}x] {question}")

    return checklist


# Example: escaped defects from a Python web service
DEFECTS = [
    EscapedDefect("BUG-101", "missing-external-error-handling",
                  "For every external API call, is the response validated before use?"),
    EscapedDefect("BUG-102", "missing-external-error-handling",
                  "For every external API call, is the response validated before use?"),
    EscapedDefect("BUG-103", "missing-external-error-handling",
                  "For every external API call, is the response validated before use?"),
    EscapedDefect("BUG-104", "transaction-boundary-error",
                  "Does every database write have the correct transaction scope?"),
    EscapedDefect("BUG-105", "transaction-boundary-error",
                  "Does every database write have the correct transaction scope?"),
    EscapedDefect("BUG-106", "pagination-off-by-one",
                  "For every pagination query, are the limit and offset tested at boundaries?"),
    EscapedDefect("BUG-107", "cache-race-condition",
                  "For every cached value, is there a clear invalidation or TTL strategy?"),
    EscapedDefect("BUG-108", "sensitive-data-in-logs",
                  "Does any log statement include user data, tokens, or PII?"),
]

if __name__ == "__main__":
    checklist = build_checklist(DEFECTS, max_items=5)
    print("Structured Review Checklist")
    print("=" * 40)
    for item in checklist:
        print(f"  [ ] {item}")

このスクリプトを自社のバグデータに対して実行すれば、他社のものではなく、自社の実際の障害モードに結びついたチェックリストが手に入る。

レビューセッションは実際にどう見えるか

チェックリストを手に、レビューセッションには特定のリズムがある。

個人準備の間、各インスペクターは1時間あたり概ね100〜150行の速度で、一人で資料を読む。彼らはチェックリストをレンズとして使用し、スクリプトとしてではない。チェックリストを順番に1項目ずつこなすわけではない。コードを自然に読み、チェックリストのカテゴリーに一致するパターンに出会ったら立ち止まり、慎重に調べる。

チェックリストは読解の代替ではない。脳が見落としそうなもののパターンマッチャーだ。

インスペクションミーティングでは、リーダーがコードを声に出して言い換える。インスペクターが潜在的な欠陥を見つけたら、即座に指摘する。チェックリストのカテゴリーが共通の語彙を提供する。「これはおかしい」と言う代わりに、「これはトランザクション境界エラーのように見える、カテゴリー2」と言える。モデレーターが記録する。著者は聞く。誰も修正案を出さない。

チェックリストは会議中に参照されない。その時点までに、インスペクターはすでにそれを内面化している。会議は、各人が孤立して見つけたものを相互確認するためのものだ。

汎用チェックリストが失敗する理由

チェックリストを試すほとんどのチームは、間違った種類を使うので諦める。

ブログ記事からコピーされた汎用チェックリストには感情的な重みがない。レビュアーに、自分のコードベースでは起こったこともないようなことを探せと頼む。レビュアーは目を通し、見慣れたものがないと、いつものようにdiffを読みに戻る。

逸脱した欠陥から構築された構造化チェックリストは異なる。各項目は、実際の時間を費やした実際のインシデントを表す。レビュアーはポストモーテムを見ているので、これらのバグを知っている。チェックリストは、レビュー行動と具体的で記憶に残る障害を結びつける。

もう一つの一般的な失敗は、チェックリストを準備ツールではなく会議ツールとして扱うことだ。レビュー会議中にチェックリストを取り出すと、グループでのざっと見のスクリプトになってしまう。全員が同じ項目を読み、同じコードを見て、同じ明白な観察に収束する。チェックリストの力は個人準備にあり、6人が同じカテゴリーを独立して適用し、異なるものを見つけるのだ。

大半のチームが無視する保守負担

チェックリストは記念碑ではない。コードより速く腐る生きた文書だ。

何かがうまくいかないたびにチェックリスト項目を追加し、一度も削除しないと、6か月後には40項目になる。その時点で、レビュアーは壁紙のように扱い始める。

Faganは、数回のインスペクションごとにチェックリスト自体を見直すことを推奨した。直近5回のレビューで発見を引き起こさなかった項目を削除する。プロダクションに逸脱した新しい欠陥カテゴリーに対してのみ項目を追加する。総数を15未満に保つ。短く保てなければ、優先順位付けができていない。

以下は、過去の発見データに基づいてチェックリストを剪定する軽量スクリプトだ:

#!/usr/bin/env python3
"""Prune a review checklist: remove items that no longer trigger findings."""

from dataclasses import dataclass
from typing import List, Dict


@dataclass
class ChecklistItem:
    category: str
    trigger_question: str
    findings_last_5_reviews: int


def prune_checklist(items: List[ChecklistItem], min_findings: int = 1) -> List[ChecklistItem]:
    """Remove checklist items that have not produced findings recently.

    A Fagan-style checklist should be short enough to be usable.
    Items that sit idle waste attention.
    """
    kept = [item for item in items if item.findings_last_5_reviews >= min_findings]
    removed = [item for item in items if item.findings_last_5_reviews < min_findings]

    print(f"Kept {len(kept)} items, removed {len(removed)} items")
    for item in removed:
        print(f"  REMOVED: [{item.category}] {item.trigger_question}")

    return kept


# Example: current checklist with finding counts from the last 5 reviews
CURRENT_CHECKLIST = [
    ChecklistItem("missing-external-error-handling",
                  "For every external API call, is the response validated before use?", 4),
    ChecklistItem("transaction-boundary-error",
                  "Does every database write have the correct transaction scope?", 3),
    ChecklistItem("pagination-off-by-one",
                  "For every pagination query, are the limit and offset tested at boundaries?", 0),
    ChecklistItem("cache-race-condition",
                  "For every cached value, is there a clear invalidation or TTL strategy?", 1),
    ChecklistItem("sensitive-data-in-logs",
                  "Does any log statement include user data, tokens, or PII?", 0),
]

if __name__ == "__main__":
    pruned = prune_checklist(CURRENT_CHECKLIST, min_findings=1)
    print("\nActive checklist:")
    for item in pruned:
        print(f"  [ ] [{item.findings_last_5_reviews}x] {item.trigger_question}")

この見直しを四半期ごとに計画する。古くなったチェックリストは、ツールを無視するようレビュアーを訓練するため、ないよりも悪い。

トレードオフ:データ対速度

欠陥データから構造化チェックリストを構築するには時間がかかる。正確なインシデント分類が必要だ。リストを短く保つための規律が必要だ。最新状態を保つためのプロセスが必要だ。

汎用チェックリストは書くのに5分、無関係になるのに5週間かかる。

構造化版は作成は遅いが、使用は速い。15の対象を絞ったトリガー質問を持つレビュアーは、40の一般的な思い出しを持つレビュアーよりも効率的にコードをスキャンする。具体性が時間を節約する。

実際のコストは組織的だ。根本原因を抽出するのに十分な詳細な逸脱欠陥の記録が必要だ。多くのチームにはそれがない。彼らのバグトラッカーには「user report: page broken」のようなタイトルがあり、フォローアップ分析はない。根本原因データがなければ、証拠に基づくチェックリストを構築できない。他社のものをコピーするだけだ。

よくある質問

What is a structured checklist-based review?

各インスペクターが、実際の逸脱欠陥データから構築されたチェックリストを、グループのインスペクションミーティング前の義務付けられた個人準備中に使用するレビュープロセス。チェックリストには、汎用的な思い出しではなく、対象を絞ったトリガー質問が含まれる。

How is this different from a normal code review checklist?

通常のチェックリストは、しばしば汎用的で、テンプレートからコピーされ、レビュー中に気軽に参照される。構造化チェックリストは、チーム固有の欠陥履歴から導出され、アーティファクト種別に限定され、定められた読解速度で集中的な個人準備中に使用される。

How many items should a structured checklist have?

アーティファクト種別あたり10〜15項目。それ以上あると、レビュアーはざっと見始める。5未満だと、カテゴリーを見落としている可能性がある。

How often should the checklist be updated?

3〜5回のインスペクションごと、または四半期ごと。発見を生み出さなかった項目を削除する。新しく逸脱した欠陥カテゴリーに対してのみ項目を追加する。

Can this work without a full Fagan inspection process?

はい。チェックリスト自体が、Fagan法の最も移植可能な部分だ。個人準備を義務付け、レビュアーに自社データから構築された構造化チェックリストを渡し、リーダーとともにtime-boxedミーティングを実施する。完全な6フェーズのセレモニーがなくても、非公式レビューより多くの欠陥を発見できる。

1つのモジュールと1四半期から始めよう

レビュープロセス全体を刷新する必要はない。過去6か月間にプロダクション欠陥があった1つのモジュールを選ぶ。根本原因を集める。5項目のチェックリストを構築する。グループディスカッションの前に、2人のレビュアーがコードとチェックリストに30分費やすことを義務付ける。

彼らが見つけたものを記録する。同じコードに対して通常のプルリクエストレビューが捉えたものと比較する。その1回の比較が、構造化チェックリストがチームにとって労力に見合う価値があるかどうかを教えてくれる。

データがイエスと言えば、もう1つのモジュールに拡大する。データがノーと言えば、非公式レビューはすでに十分に優れているか、あるいは欠陥データが有用なチェックリストを構築するには詳細不足だ。どちらにせよ、借りた意見ではなく、実際の測定値を手に入れることができる。