Два ревьюера, один diff, нулевое пересечение

Два senior-инженера ревьюят один и тот же pull request. Один отмечает пропущенный null check. Другой находит race condition в cleanup path. Ни один из них не находит оба.

Если бы вы назначили только одного ревьюера, один из этих багов попал бы в продакшн. Это не пробел в навыках. Это предсказуемое свойство человеческого внимания, и Michael Fagan задокументировал его в IBM в 1976 году.

Fagan измерял показатели обнаружения дефектов в software pipeline IBM. Его данные показали нечто неудобное: даже опытные инспекторы находили лишь долю от общего числа дефектов, которые группа в итоге обнаруживала. Реальная ценность заключалась не в экспертизе какого-либо одного человека. Она заключалась в структурированном сочетании нескольких перспектив.

Большинство команд сегодня не структурируют это сочетание. Senior-инженер пробегает глазами diff между встречами, замечает issue со style, апрувит и идёт дальше. Следующий ревьюер делает то же самое. Оба пропускают off-by-one ошибку, которая в следующий вторник испортит production-данные.

Fagan назвал это синдромом неструктурированного review. У группы есть глаза, но нет процесса.

Что такое Fagan inspection на самом деле

Fagan inspection — это не встреча, на которой люди вместе читают код и делятся чувствами. Это формальный процесс с определёнными ролями, entry criteria и измеримым output. Fagan разработал его, потому что неструктурированные reviews тратили время и пропускали дефекты примерно с той же скоростью, что и отсутствие review вообще.

Ключевой инсайт — разделение ролей. У каждого ровно одна задача:

  • Moderator ведёт встречу и обеспечивает соблюдение правил. Он не инспектирует.
  • Reader пересказывает код вслух. Это заставляет группу столкнуться с тем, что код реально делает, а не с тем, что author задумал.
  • Tester думает о execution paths, boundary conditions и coverage gaps.
  • Author отвечает на вопросы, но не защищает код.

Это разделение предотвращает самый распространённый failure mode группового review: когда author убеждает всех отказаться от своих опасений.

Когда author одновременно является объясняющим, он сглаживает двусмысленность. «О, эта переменная всегда устанавливается caller.» Группа кивает. Никто не проверяет. Роль reader существует, чтобы сломать эту привычку. Если reader не может пересказать функцию одним предложением, функция не готова к ship.

Почему checklists побеждают интуицию

Fagan также ввёл inspection checklists. Это не generic coding standards, скопированные из style guide. Они адаптированы под конкретный тип артефакта, который reviewится.

Checklist для state machine спрашивает: вы обработали каждый transition? Checklist для resource allocator спрашивает: каждая allocation спарена с deallocation на каждом path?

Checklist существует, потому что человеческое внимание неравномерно. Эксперт, написавший десять тысяч database queries, мысленно перепрыгнет через блок BEGIN TRANSACTION. Его мозг автодополнит его как правильный. Fagan обнаружил, что инспекторы, ориентированные на checklist, находили дефекты, мимо которых проходили интуитивно ориентированные ревьюеры — не потому, что эксперты были небрежны, а потому, что экспертиза создаёт blind spots.

Вот lightweight checklist для одной 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?",
]

Это не бюрократия. Это forcing function для систематического внимания.

Fagan разделил inspection на четыре фазы, и встреча — самая короткая

Настоящий Fagan inspection имеет четыре фазы, и сама встреча — самая короткая.

Preparation. Каждый инспектор reviewит материал в одиночку, с checklist, до того как группа собирается. Fagan обнаружил, что подготовленные инспекторы находили примерно вдвое больше дефектов, чем те, кто приходил с ходу. Встреча существует только для комбинирования findings, а не для их генерации.

Встреча. Reader проходит по коду. Tester задаёт what-if вопросы. Moderator укладывается в два часа. Author делает заметки. Никто не фиксит код во время встречи. Дефекты логируются, и группа движется дальше.

Rework. Author исправляет залогированные дефекты в одиночку.

Follow-up. Moderator верифицирует, что каждый дефект был адресован. Большие reworks могут спровоцировать второй inspection.

Эта структура кажется тяжеловесной для современного pull request. Fagan проектировал её для mainframe software, где один дефект мог стоить миллионов. Принципы всё ещё адаптируются и к более мелким контекстам.

Где это становится оверкиллом

Fagan inspections не бесплатны. Время preparation само по себе добавляет значительный overhead. Для десятистрочного bugfix полный Fagan inspection — абсурд. Вам не нужны четыре человека и checklist, чтобы найти пропущенный import.

Кривая payoff нелинейна. Данные Fagan’а говорили, что inspections больше всего окупаются для сложных, high-risk модулей: state machines, parsers, resource managers, всё что угодно с non-local state или тонкими ordering constraints. Для CRUD handlers и boilerplate tests неформальный review вполне подходит.

Настоящая ошибка — применять одну и ту же review-стратегию к каждому изменению. Опечатка в лог-сообщении не нуждается в Fagan inspection. Distributed transaction coordinator, скорее всего, нуждается.

Легковесная версия, которую вы можете использовать сегодня

Вам не нужна meeting-культура IBM, чтобы получить большую часть выгоды. Вот lightweight адаптация, которая работает для современных команд:

  1. Требовать индивидуальный review до группового обсуждения. Каждый ревьюер сабмитит written comments до того, как кто-либо заговорит. Это предотвращает доминирование первого громкого мнения.

  2. Ротировать роль reader. Попросите одного ревьюера резюмировать изменение своими словами до того, как кто-либо его критикует. Если он не может, изменение слишком большое или непонятное.

  3. Построить team checklist. Начните с семи вопросов выше. Добавьте домен-специфичные items. Пересматривайте ежеквартально.

  4. Разделить author и defender. Author отвечает на фактические вопросы. Он не спорит, что код в порядке. Если ревьюер запутался, это данные, а не дебаты.

  5. Логировать дефекты, фиксить позже. Не переписывайте код во время review. Залогируйте issue, завершите, потом исправьте.

Вот простой скрипт для генерации review checklist для любого 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’у почти пятьдесят лет, но вывод не изменился. Индивидуальные ревьюеры непоследовательны. Группы тоже непоследовательны, если вы их не структурируете. Вариативность между ревьюерами — не проблема, которую нужно устранить. Это ресурс, который нужно организовать.

В следующий раз, когда один ревьюер найдёт баг, который пропустил другой, не спрашивайте, кто лучше. Спросите, достаточно ли ваш процесс структурирован, чтобы комбинировать то, что видят оба. Fagan уже пытался нанять себя из этой ситуации. Это не работает.

FAQ

Что такое Fagan inspection?

Fagan inspection — это формальный, структурированный процесс code review, разработанный Michael Fagan в IBM в 1976 году. Он использует определённые роли (moderator, reader, tester, author), требования к preparation и checklists для максимизации обнаружения дефектов в software artifacts.

Почему разные ревьюеры находят разные баги?

Человеческое внимание избирательно. Эксперты развивают ментальные shortcuts, позволяющие им быстро читать код, но те же shortcuts создают blind spots. У разных ревьюеров разные backgrounds и когнитивные patterns, поэтому их blind spots не перекрываются идеально. Исследование Fagan’а показало, что ценность inspection заключается в комбинировании нескольких неполных перспектив, а не в поиске одного идеального ревьюера.

Используются ли Fagan inspections сегодня?

Полный формальный процесс редок вне safety-critical индустрий, таких как аэрокосмическая отрасль и медицинские устройства. Однако базовые принципы — индивидуальная preparation, разделение ролей и review, управляемый checklists — всё чаще адаптируются high-performing software-командами. Ключевые идеи также повлияли на современные практики, такие как structured walkthroughs и formal technical reviews.

Когда overhead полного Fagan inspection оправдан?

Для модулей, где дефект имеет тяжёлые последствия: distributed consensus logic, security boundaries, resource lifecycles и state machines. Для рутинных изменений обычно достаточно lightweight адаптаций. Соотносите rigor review с реальным риском, а не один и тот же процесс к каждому diff.