Как ревьюить код, который написал агент

← назад к разделу

Агент за час прислал изменение на восемьсот строк. Тесты зелёные, описание убедительное, код выглядит аккуратно. Ревьюер открывает его в конце дня, пролистывает, не находит, к чему придраться, и вливает. Через две недели выясняется, что поле в ответе API переименовано «для удобства», при пустой корзине заказ создаётся, а платёжный шлюз, вернувший ошибку, тихо превращается в успешный ответ. Ничего из этого не было видно на успешном пути, а успешный путь — единственное, что успели проверить.

Ревью кода от агента отличается от привычного не тем, что «агент пишет хуже», а тем, что он пишет больше и увереннее, и ошибки его сидят не там, куда смотрит привычный ревьюер. После статьи вы будете знать пять точек, на которые смотрят в фиксированном порядке, что отдать машине, что делать с находкой, как читать изменение, где агент «заодно» переписал соседей, и как ревьюить собственный код, когда ревьюер и автор — вы один.

Почему привычное ревью здесь не работает

Старая модель: опытный разработчик читает изменение на двести-четыреста строк, оставляет пять-десять замечаний, автор правит, изменение вливают. Два-четыре таких изменения в день на ревьюера.

С агентом та же команда даёт в пять-десять раз больше кода за тот же день, и старая модель рвётся в четырёх местах:

• объём. Ревьюер физически не успевает читать. Если тратить прежнее время на строку, ревью становится узким местом, и команда начинает вливать без него быстрее, чем кто-то это замечает;
• уверенный тон. Агент пишет спокойным голосом «вот так и надо», и чем мощнее модель, тем убедительнее звучит выдумка. Глаза скользят по правильно выглядящему коду, который не работает;
• контекст не сохраняется. Каждое изменение пишется с чистого листа: агент не помнит, что в прошлый раз команда договорилась о подходе к пустым значениям. Без явных правил получается локально корректный код, несовместимый с остальной базой;
• покрытие смещается на успешный путь. Агент отлично пишет основной сценарий, потому что в обучающей выборке его большинство. Пустая коллекция, отсутствие данных, конкурентный доступ, ошибка вниз по стеку представлены хуже, и пишутся хуже.

Из этого следует не «агента отменить», а поменять модель ревью под новый поток: смотреть в конкретные точки в конкретном порядке и всё остальное отдать машине.

Пять точек в фиксированном порядке

Порядок важен: если первое сломано, дальше можно не идти, изменение возвращается целиком. Смотреть на именование переменных в коде, который реализует не ту операцию, — потерянное время.

Контракт на стыках

Сигнатура API, публичные методы доменных сервисов, события на шине — всё, что видно соседним сервисам и команде. Агент любит чуть-чуть менять контракт ради удобства: переименовать поле в ответе, заменить опциональную обёртку на пустое значение, вернуть логическое значение вместо ничего «для удобной проверки». Такие правки молча ломают всё снаружи, и в тестах самого изменения это не видно, потому что тесты агент написал под новый контракт.

Что сверяют: сигнатура совпадает с описанием в OpenAPI, AsyncAPI или спецификации; имена полей те же, что в спеке; ошибки объявлены и возвращаются ровно те, что обещаны; если задача описана спецификацией как кодом, поведение совпадает с её разделами про команды и запросы.

Соответствие устройству проекта

Любая команда работает по какой-то методологии: свои слои и границы между ними. Агент без явных правил соскальзывает на «как принято в обучающей выборке», а это в среднем толстый сервисный слой с бизнес-логикой в одном классе.

Что сверяют: контроллер не содержит бизнес-логики, только маппинг и вызов; бизнес-правила в обработчике операции, а не размазаны по слоям; инварианты в агрегате, а не в обработчике (см. тактические паттерны); события публикуются в той же транзакции, что и запись (через outbox, а не «после сохранения вызвали publish»); доступ к данным через типизированные запросы, а не через случайный объект из доменного слоя прямо в контроллере.

Крайние случаи

Самое типичное место, где агент спотыкается, и самое неудобное для ревьюера: ветки, которых в успешном сценарии нет, надо искать. Четыре сценария, которые проверяют всегда, и что происходит, если их нет:

Как проверить это быстро, не читая восемьсот строк: попросить агента, который писал код, перечислить ветки отказа. «Для операции оплаты заказа перечисли, что произойдёт при пустой корзине, повторном запросе с тем же ключом, отказе шлюза и одновременной оплате двумя запросами; для каждой укажи строку кода и тест». Если в ответе на какой-то сценарий нет ни строки, ни теста, находка готова без чтения. Ту же таблицу полезно попросить у агента до написания кода: тогда ветки появятся сразу.

Если в коде нет явных проверок и тестов на эти четыре точки, изменение требует доработки, даже если успешный сценарий работает.

Тесты на поведение

Самая коварная точка. Агент генерирует тесты, которые выглядят как тесты, но проверяют только то, что код не упал. Слева типичный такой тест, справа тот же тест, переписанный на проверку поведения:

@Test
void shouldCreateOrder() {
    var order = service.create(command);
    assertNotNull(order);
    verify(repository).save(any());
}

@Test
void createsPendingOrderWithTotalAndEvent() {
    var order = service.create(commandFor(2, Money.of(300)));
    assertEquals(OrderStatus.PENDING, order.status());
    assertEquals(Money.of(600), order.total());
    verify(repository).save(argThat(o -> o.customerId().equals(CUSTOMER)));
    assertEquals(List.of(new OrderCreated(order.id())), events.published());
}

func TestCreateOrder(t *testing.T) {
    svc, repo := newTestService(t)
    order, err := svc.CreateOrder(ctx, input)
    if err != nil || order == nil || !repo.saveCalled {
        t.Fatal("expected order")
    }
}

func TestCreateOrder_PendingWithTotalAndEvent(t *testing.T) {
    svc, repo := newTestService(t)
    order, err := svc.CreateOrder(ctx, inputFor(2, 300))
    if err != nil {
        t.Fatal(err)
    }
    if order.Status != StatusPending || order.Total != 600 {
        t.Fatalf("got %v %d", order.Status, order.Total)
    }
    if repo.saved.CustomerID != customerID {
        t.Fatalf("saved for %s", repo.saved.CustomerID)
    }
    if len(svc.events) != 1 || svc.events[0].OrderID != order.ID {
        t.Fatalf("events: %v", svc.events)
    }
}

it('should create order', async () => {
    const order = await service.createOrder(input);
    expect(order).not.toBeNull();
    expect(repository.save).toHaveBeenCalled();
});

it('creates pending order with total and event', async () => {
    const order = await service.createOrder(inputFor(2, 300));
    expect(order.status).toBe('PENDING');
    expect(order.total).toBe(600);
    expect(repository.save).toHaveBeenCalledWith(expect.objectContaining({ customerId }));
    expect(events.published()).toEqual([{ type: 'OrderCreated', orderId: order.id }]);
});

def test_create_order():
    order = service.create_order(command)
    assert order is not None
    repository.save.assert_called()


def test_creates_pending_order_with_total_and_event():
    order = service.create_order(command_for(items=2, price=300))
    assert order.status == OrderStatus.PENDING
    assert order.total == 600
    saved = repository.save.call_args.args[0]
    assert saved.customer_id == CUSTOMER
    assert events.published() == [OrderCreated(order.id)]

Первый тест пройдёт при любом статусе, любой сумме и без события. Второй проверяет то, что обещано требованием: статус, сумму, кому сохранили и какое событие ушло. Признаки первого вида, по которым его видно за секунду: проверка «не пусто» вместо значения, «метод вызвался» с любыми аргументами вместо конкретных, отсутствие проверки события и проверки на ожидаемое исключение при невалидном входе. Как быстро убедиться, что тест вообще что-то держит, — сломать код и посмотреть, покраснеет ли он; этот приём разбирает статья «Ревью и тестирование кода с агентом».

Импорты и зависимости

Самая частая выдумка агента — методы и пакеты, которых нет. У популярных библиотек правдоподобные имена, и модель дописывает недостающие по памяти. Что сверяют: все зависимости разрешаются (сборка это покажет, но агент нередко пишет код, не собирая его); методы запросов к хранилищу существуют в подключённой версии библиотеки; версии в манифесте зависимостей совпадают с теми, под которые написан код. На языке со статическими типами большую часть этой точки закрывает компилятор; на динамическом — только запуск, о чём статья про выбор языка.

Что делать с находкой

Ревью доводят до находки и часто на этом обрывают: «требует доработки». Но у находки в коде от агента три разных исхода, и выбирают между ними осознанно.

Вернуть агенту в том же контексте. Подходит для локальных находок: не хватает проверки на пустую коллекцию, тест проверяет не то. Агент помнит задачу, правка занимает минуту. Замечание формулируют как критерий, а не как жалобу: не «тут плохо с ошибками», а «при отказе шлюза операция должна вернуть ошибку PaymentFailed и не менять статус заказа; добавь ветку и тест».

Начать заново в новой сессии. Подходит, когда сломана первая или вторая точка: не тот контракт, не та операция, логика не в том слое. Чинить такое правками поверх дорого: агент будет латать симптомы, сохраняя неверную основу. Дешевле выбросить изменение, уточнить задание (контракт, границы, крайние случаи) и запустить с чистого листа. Это не поражение, а нормальный ход: набор кода стоит минуты, а уточнение задания — то, что вы должны были сделать сразу.

Поправить руками. Подходит для мелочей, которые быстрее исправить, чем описать: имя, порядок аргументов, лишний импорт.

Отдельный случай — изменение, где агент «заодно» переписал соседей: задача «добавь поле», а в изменении шестьсот строк, из которых к задаче относятся двадцать. Не читайте всё. Первым делом просят разделить: «вынеси всё, что не относится к добавлению поля, в отдельное изменение или откати». Постороннее либо уходит отдельным изменением со своим ревью, либо исчезает. Читать шестьсот строк ради двадцати — ровно то, на чём ломается ревью под потоком агента. Лечится это ещё раньше, на этапе задания: узкая задача с явной границей «ничего кроме» даёт узкий диф.

Что не ревьюить руками

Главное правило: если это можно проверить машиной, проверяет машина. Человеческое внимание дорого, тратьте его на то, что машина не может.

Машине отдают стиль и форматирование (Checkstyle, golangci-lint, Prettier, ruff), порядок импортов, шаблонный код (кодогенерация или языковые средства для value object, equals, toString), локальные имена переменных в коротких методах, пробелы и переводы строк (хуки git), архитектурные инварианты (ArchUnit, dependency-cruiser или аналог: контроллер не вызывает репозиторий, ядро не импортирует инфраструктуру).

Если машина это уже ловит, в ревью таких замечаний быть не должно. Если приходят, настройте pre-commit или CI, а не нагружайте человека.

Слои процесса: человек последним

Объём не победить более внимательным ревьюером. Нужен слоёный процесс, где каждый слой отсекает свою долю замечаний, а человек подключается последним, к тому, что осталось.

Слой 1, pre-commit локально. Разработчик коммитит, и на изменённых файлах прогоняются исполняемые правила. Нарушение блокирует коммит до исправления. Замечание видно до открытия изменения, и раунд «ревьюер написал, автор поправил, снова ревью» не нужен. Условие: быстро, до пяти секунд, только по дифу.

Слой 2, исполняемые правила в CI. На каждое изменение запускается исполняемый стандарт: правила проекта, которые агент применяет к дифу и комментирует. Каждое замечание цитирует код правила и ссылку на его описание. Слияние это не блокирует, команда сама решает, что принять, но большая часть банальных замечаний до человека уже не доходит: они либо приняты, либо обоснованно отвергнуты автором.

Слой 3, сравнение со спецификацией. Если контракт задачи лежит в репозитории рядом с кодом (как его писать, разбирает статья «От модели понятий к контракту»), агент в CI сверяет код с ним: команды из контракта соответствуют операциям, у каждого критерия приёмки есть тест, объявленные события публикуются. Расхождение — это или ошибка кода, или устаревший контракт, и в обоих случаях его чинят в этом же изменении, а не «доработаем потом». Так контракт остаётся живым, а ревьюер получает не диф, а список расхождений.

Слой 4, человек. К этому моменту до него дошёл код без замечаний по стилю, с проверенными правилами, с тестами, прошедшими первичную проверку, и с отчётом о соответствии контракту. Человек тратит внимание на то, что машина не может: вписывается ли решение в архитектуру целиком, то ли это, что нужно бизнесу, и выбран ли лучший из компромиссов. Десять минут на серьёзное изменение в этом режиме — не «прочитал восемьсот строк», а разговор о трёх-четырёх архитектурных моментах.

Когда ревьюер — вы сами

Продукт-инженер часто один: агент написал, вы же и ревьюер, и приёмщик. «Человек последним» тогда означает «я и первый, и последний», и главная опасность в том, что вы читаете код с теми же предположениями, с которыми ставили задачу. Три приёма, которые это компенсируют.

Первое — разнести автора и ревьюера по сессиям. Ревью делает агент в новой сессии, без истории написания, с дифом и исходным требованием на входе, лучше другой моделью. Он не знает, что вы имели в виду, и читает требование заново; расхождение двух трактовок и есть находка.

Второе — ходить по пяти точкам списком, а не глазами. В одиночку соблазн пролистать особенно велик. Список из пяти вопросов с ответом «где строка, где тест» занимает десять минут и не зависит от настроения.

Третье — не вливать в тот же час. Изменение, написанное и принятое в одном порыве, читается теми же глазами. Утро следующего дня или хотя бы обед между написанием и приёмкой дают тот эффект, ради которого в команде существует второй человек.

Анти-паттерны

Они появляются сами: никто не учит обратному, а под нагрузкой команда соскальзывает на удобные сокращения.

• «Всё работает, вливаем». Тесты зелёные, локально запускается. Проверена только успешная ветка; крайние случаи, инварианты и конкурентные сценарии никто не смотрел.
• «Не разбираюсь в этом куске, доверяю агенту». Агент написал интеграцию с новой библиотекой, ревьюер её не знает и вливает на доверии. Через месяц выясняется, что половина API выдумана, а работает оно только в тесте успешного пути, который тоже написал агент. Самый опасный паттерн: он растёт вместе с мощностью модели.
• «Тест потом». Код есть, релиз горит, тест откладывают. Когда возвращаются, контекст забыт, и тест пишут по тому, что в коде, а не по тому, что должно быть. Тест закрепляет ошибки, а не ловит их.
• «Агент заодно отрефакторил». Задача на двадцать строк, изменение на шестьсот. Ревьюер либо тратит два часа, либо вливает как есть. Лечится узкой задачей и просьбой разделить, см. выше.
• «Одно большое изменение на спринт». Агент пишет быстро, и хочется вкатить фичу целиком. Это рвёт цикл ревью по объёму. Дробите, как дробили бы при ручном написании: одна логическая единица — одно изменение.

Чек-лист

Его можно положить в репозиторий команды, например в docs/code-review.md, и ссылаться из шаблона изменения.

Перед открытием изменения, автор

• pre-commit с исполняемыми правилами прошёл без блокирующих замечаний;
• на все четыре крайних случая (пусто, пустая коллекция, конкурентный доступ, отказ вниз по стеку) есть тесты;
• тесты проверяют поведение, а не «не упало»;
• изменение содержит одну логическую единицу, ничего «заодно».

При ревью, ревьюер

• контракт на стыках совпадает со спекой;
• контроллер без бизнес-логики, инварианты в агрегате, а не в обработчике;
• транзакции там, где надо, и не там, где не надо; события в той же транзакции, что и запись;
• доступ к данным через типизированные запросы, не из контроллера напрямую;
• зависимости разрешаются, версии совпадают, выдуманного API нет;
• тесты проверяют конкретные значения, а не «не пусто» и «вызвался»;
• исполняемые правила прошли в CI, сравнение со спецификацией совпало.

Перед слиянием

• блокирующие замечания закрыты;
• неблокирующие приняты или явно обоснованы;
• архитектурные тесты зелёные.

Коротко

• Агент пишет больше и увереннее, его ошибки не видны на успешном пути, и привычное ревью «прочитать всё» под таким потоком рвётся.
• Пять точек в фиксированном порядке: контракт на стыках, устройство проекта, крайние случаи, тесты на поведение, зависимости. Сломано первое — дальше не идти.
• Крайние случаи проверяют быстро, попросив агента перечислить ветки отказа со строкой и тестом на каждую; отсутствующая строка — готовая находка.
• Тест, который проверяет «не пусто» и «метод вызвался», ничего не проверяет; настоящий называет ожидаемые значения и событие.
• С находкой три исхода: вернуть агенту в том же контексте, начать заново в новой сессии (если сломан контракт или операция), поправить руками. Изменение «заодно» сначала просят разделить.
• Всё, что проверяет машина, проверяет машина: стиль, импорты, шаблонный код, архитектурные инварианты.
• Слои процесса: pre-commit, исполняемые правила в CI, сравнение со спецификацией, человек последним, на смысл и компромиссы.
• В одиночку: ревью агентом в новой сессии, пять точек списком, пауза между написанием и приёмкой.

Что почитать дальше

• Ревью и тестирование кода с агентом — как поручить агенту первый проход и как за минуту проверить его тест.
• Приёмка результата AI — приёмка это не ревью: сверка с контрактом, а не качество кода.
• От модели понятий к контракту — контракт, с которым сверяет код третий слой процесса.
• Исполняемые правила против SonarQube и ESLint — чем свод правил для агента отличается от статического анализатора.