Анализ крупных рефакторингов в ИИ: почему прохождение тестов не раскрывает устранённую цель
Узнайте, почему рефакторинг с использованием ИИ тайно удаляет критически важный код, почему тесты не выявляют этого, и о пяти практиках ревью, которые делают крупные рефакторинги безопасными для выпуска.
Источники: I Let Claude Refactor 4,000 Lines. Here’s the Part I Had to Undo.
Ассистенты по программированию на основе ИИ теперь настолько сильны, что могут отремонтировать заброшенный модуль за один день, причем результат обычно оказывается лучше, чем у исходного кода. Опасность заключается не в небрежном коде, а в коде, выглядящем корректно, но тихо устраняющем функционал, который никто не описал. В этом руководстве рассматривается реальный пример подобной неудачи, объясняется, почему и тестовый набор, и проверяющий его не заметили проблему, а также предлагается конкретный процесс проверки при крупном масштабе рефакторинга с использованием ИИ.
Рефакторинг, улучшивший код и сломавший систему
Рассмотрим модуль, в котором накапливались изменения в течение двух лет: около 4 000 строк кода в четырех файлах, участок кодовой базы, к которому инженеры стараются не прикасаться, если это возможно. В нем нет единообразной обработки ошибок, существуют три разных способа парсинга дат, и есть несколько функций, вызовы которых никто не может с уверенностью подтвердить.
Разработчик передаёт код ИИ-ассистенту в течение двух сессий и получает результат, явно лучший: единые шаблоны, корректные указатели типов, удалённые ненужные ветки, примерно на треть меньше строк. Тесты проходят. Разработчик проверяет изменения, или думает, что проверил их, и выпускает код.
Шесть дней спустя при интеграции с партнёром начинают пропадать записи. Никаких явных ошибок нет; просто около четырёх процентов записей перестают поступать. Причиной стало рефакторинг, а точнее — изменение, которое разработчик прочитал и одобрил.
В чём ассистент действительно отличился
Стоит быть точным относительно того, что пошло хорошо, потому что вывод не в том, что «ИИ плохо справляется с рефакторингом». Ассистент отлично справился со всеми преобразованиями, которые зависели только от кода перед ним:
- объединение трёх функций для парсинга дат в одну функцию
Для механических, локальных изменений, которые можно проверить путем их просмотра, этот инструмент работал быстрее и надежнее человека, к тому же он не терял фокус на глубине в несколько сотен строк, где ручные рефакторинги обычно теряют эффективность. Когда задача заключается в том, чтобы «сделать код более понятным для передачи того же поведения», эти инструменты показывают отличные результаты. Проблемы начинаются там, где возникает неопределенность относительно того, что именно входит в понятие «то же поведение».
Почему удаляется код, выполняющий ключевые функции
Код, не имеющий очевидной причины для существования, выглядит точно как мусор. Вот одобренное изменение:
# before
def normalise_timestamp(raw):
ts = raw.strip()
if ts.endswith("Z") and "." in ts:
ts = ts.split(".")[0] + "Z" # partner sends 7-digit microseconds
return datetime.fromisoformat(ts.replace("Z", "+00:00"))
# after
def normalise_timestamp(raw):
return datetime.fromisoformat(raw.strip().replace("Z", "+00:00"))
Удалённая ветвь выглядит как типичный объект для рефакторинга: специальный случай, необычно точная проверка суффикса, разделение строки. Она кажется избыточной, поскольку функция fromisoformat уже парсит ISO-временные метки.
Проблема заключается в входных данных. Один из партнеров отправляет временные метки с семью знаками после запятой. До Python 3.11 функция fromisoformat обрабатывала только те форматы, которые генерировала сама функция isoformat(), то есть не более шести знаков после запятой, поэтому значение с семью знаками вызывало ошибку ValueError. В Python 3.11 и новее версиях поддерживается гораздо более широкий набор форматов ISO 8601, что делает систему более терпимой к таким данным; следовательно, будет ли эта изменение приводить к сбоям, зависит от конкретной версии языка, что ещё раз подчеркивает необходимость проверки вместо предположений. Эта ветка появилась из-за заявки на поддержку, поступившей восемнадцать месяцев ранее, и её краткий комментарий внутри кода оказался недостаточным, чтобы кто-либо, будь то человек или искусственный интеллект, правильно оценил её значимость по сравнению с тем, насколько легко её можно было устранить. Стоит также отметить, что старая ветка выполняла больше функций, чем казалось: она полностью игнорировала дробную часть временной метки, поэтому даже результаты работы «новой» версии отличаются для данных, которые всё же удается распарсить.
Основная проблема проста: модель могла видеть код, но не инцидент, приведший к его появлению. Заявка на решение проблемы, анализ после инцидента, чат-сообщения и время, потраченное на отладку, все это находилось вне файла. Выполнение команды git blame показало бы сообщение о коммите вроде «исправление парсинга временных меток для пит-стримов партнеров», что и было единственным обоснованием, но никто не попросил ассистента на это посмотреть. Рецензент одобрил удаление по той же причине, по которой это предложил ассистент: оба изучали различия в коде, а не историю изменений.
Почему набор тестов оставался в рабочем состоянии
Тесты проверяли, что возвращает функция для входных данных, придуманных кем-то, а проблематичные входные данные по определению были такими, которые никто не придумал.
В данных о тестах содержались временные метки с трёмя десятичными знаками, потому что именно так люди вводят данные для тестов вручную. Значения из семи цифр появлялись только в прямом потоке одного из партнёров.
Именно поэтому крупные рефакторинги являются рискованными, независимо от наличия искусственного интеллекта: прохождение тестов подтверждает лишь то, что поведение, описанное в тестах, сохраняется. Это ничего не говорит о поведении, которое не описано. Когда вы вручную рефакторите тридцать строк кода, вы уделяете внимание каждой из них. Когда сразу меняется четыре тысячи строк, тестовый набор в итоге выполняет почти всю проверку, хотя изначально он не был создан для такой нагрузки.
Показатели покрытия тоже не помогли. Функция имела полное покрытие строк: каждая строка выполнялась во время тестов. Однако один из путей просто никогда не выполнялся с теми данными, которые имели значение. Показатель покрытия строк отражает лишь то, какой код был выполнен, а не какие варианты входных данных использовались.
Сортировка изменений ИИ по уровню риска
Практическим решением является прекращение единообразного анализа различий, сгенерированных ИИ, и их сортировка по категориям. У этих категорий совершенно разные профили риска:
- Низкий риск: аннотации типов, переименование элементов, форматирование, досрочное возвращение результатов и подобные механические изменения. Они локальны и легко проверяются путем прочтения.
- Средний риск: объединение дублирующихся функций. Обычно это безопасно, но необходимо убедиться, что объединенная версия корректно обрабатывает входные данные всех пользователей.
- Высокий риск: удаление кода, который, по-видимому, не используется, устранение специальных случаев, кажущихся избыточными, и унификация поведения, которое выглядит несогласованным.
Все элементы с высоким риском имеют одну общую особенность: в каждом случае модель правильно замечает, что что-то не так, и именно это нарушение является причиной ее существования.
Последний пункт также влияет более тонко. Представьте модуль, который форматирует ошибки иначе, чем остальные, и операцию рефакторинга, призванную привести его к единообразию. Это различие было намеренным, поскольку последующий потребитель обрабатывает именно такой формат ошибок. Стремление к единообразию было разумным инстинктом; однако применение его здесь стало ошибкой.
Пять рекомендаций для следующего крупного рефакторинга
Они расположены примерно в порядке того, насколько каждая из них могла бы предотвратить вышеописанный инцидент.
Сохраняйте размер каждых изменений на уровне, позволяющем их действительно рассмотреть
Четыре тысячи строк — это слишком большой объем для тщательного анализа. При превышении нескольких сотен строк большинство людей лишь бегло просматривают код в поисках очевидных проблем, вместо того чтобы серьезно разбираться с каждым фрагментом, как бы они себе ни говорили. Ограничьте каждую сессию одним модулем, а каждый запрос к репозиторию — изменениями только в одном модуле. Узким местом является не пропускная способность модели, а внимание проверяющего.
Требуйте обоснование для каждого удаления
Прежде чем принять результат, попросите ассистента предоставить список всего, что он удалил, и для каждого элемента — причину, по которой он считает удаление безопасным. Формулирование обоснований в текстовом виде — один из самых ценных шагов, которые вы можете предпринять. Любое обоснование вроде «это казалось избыточным» следует рассматривать как сигнал к ручному расследованию.
Предоставьте модели историю изменений
Попросите ассистента использовать команду git blame для каждой строки, удаление которой он предлагает, и сообщить сообщение о коммите. Причина появления необычной ветки часто записывается там, и модель хорошо использует этот контекст, когда он у нее есть; однако она сама не будет искать его без указания.
Пишите комментарии с объяснением «почему» перед рефакторингом
Сначала добавляйте пояснительные комментарии, а не позже. Точный комментарий с указанием имени партнера, семизначных дробей и шестизначного лимита парсера, вероятно, предотвратил бы это удаление. Размытое упоминание временных меток партнера этого не сделало. Если тот, кто будет читать код дальше — будь то человек или инструмент, — не сможет понять, зачем существует та или иная строка, она готова к исчезновению после следующей процедуры очистки.
Сравнение старой и новой версий с реальным трафиком
Подайте данные за день из реальных производственных условий через старую и новую реализации и сравните результаты. Для простых операций вроде парсинга и нормализации это не требует много ресурсов, и такой подход позволяет выявить именно те проблемы, которые ускользают от стандартных тестов, поскольку реальный трафик включает данные, о которых никто не подумал записать. Если воспроизведение трафика невозможно, лучшим вариантом будет выборка реальных данных для использования в тестах на обратную совместимость.
Основные выводы
Рефакторинг всё равно оказался полезным: модуль стал лучше, работа заняла лишь один день, а единственный сбой среди четырёх тысяч изменённых строк — это незначительно по сравнению с многочисленными крупными ручными рефакторингами. Называть результат «ошибкой ИИ» — значит упускать суть. ИИ сделал разумный вывод на основе того, что мог увидеть, а именно — специальный случай без указанных причин; человек-ревизор пришёл к тому же выводу, рассматривая тот же неполный результат.
- Репозиторий — это лишь часть системы; причины необычного кода содержатся в задачах, сообщениях об изменениях, анализах после сбоев и в памяти людей.
- Инструмент, видящий только код, с уверенностью удалит всё то, для чего никогда не было описано обоснование.
- Положительные тесты и полное покрытие строк не доказывают, что поведение сохраняется для входных данных, которые никто не моделировал.