Как я ловлю галлюцинации LLM в коде: пайплайн проверок в CI

Месяца четыре назад мне на ревью пришел пулл‑реквест, в котором из цикла обновления частиц вызывалась функция FrameAllocator::TryReclaimTail(), с правильным префиксом, в нашем стиле именования и даже с комментарием над вызовом, где объяснялось, зачем хвост кадрового аллокатора надо подбирать именно здесь, и выглядело все это настолько по‑домашнему, что я пролистал дифф, поставил апрув и ушел обедать. На PC сборка прошла и тесты позеленели, потому что вызов сидел внутри старого профайлерного макроса, который на десктопе разворачивался в пустоту вместе со всем содержимым скобок, а через два дня ночной билд под консоль упал на линковке, и выяснилось, что такой функции в движке нет и никогда не было.

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

Запрещать модели у нас никто не стал, потому что запрет сейчас работает примерно как запрет стековерфлоу году в две тысячи десятом, когда пользоваться им продолжают все, просто перестают в этом признаваться на ревью. Поэтому за полгода вокруг сборки наросла цепочка проверок, которая ловит большую часть того, что модель придумывает, и про которую меня с завидной регулярностью спрашивают в курилке после докладов, так что попробую рассказать, из чего она собрана и во что она нам обходится.

Компилятор как первое сито

Первым ситом у нас работает обычная сборка, и это самая скучная часть всей конструкции, но на нее приходится больше половины пойманных выдумок, а настроить ее как следует стоит дешевле всего остального. Правило простое: каждый PR собирается сразу всеми тулчейнами, которые есть в проекте, MSVC для десктопа и редактора, clang под две консоли и gcc для серверной части, и собирается с предупреждениями как ошибками. Раньше консольные билды ездили ночью, чтобы не греть ферму днем, и как раз в эту щель тогда провалился TryReclaimTail, так что теперь PR ждет все платформы, ферма обходится примерно вдвое дороже, а бухгалтерия до сих пор уверена, что программисты просто полюбили смотреть на зеленые галочки.

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

// было: на десктопе аргумент исчезал вместе с ошибкой
#define PROF_ALLOC_SCOPE(expr)

// стало: выражение не вычисляется, но имена в нем обязаны существовать
#define PROF_ALLOC_SCOPE(expr) ((void)sizeof((expr), 0))

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

Отдельная история с неявными преобразованиями, когда модель зовет существующую функцию с перепутанным порядком аргументов и int спокойно превращается во float, а bool в индекс массива. Против этого помогают ‑Wconversion и его аналоги на других компиляторах, которые у нас много лет были выключены, потому что на старом коде они выдавали несколько тысяч предупреждений, поэтому включили их только для файлов, которые трогает PR, и выглядит это странно, зато каждый новый кусок кода вынужден быть аккуратным, а старый постепенно чистится по мере того, как его кто‑нибудь задевает.

Имена, которых вчера не было

Следующая проверка выглядит почти наивной и при этом приносит больше всего неожиданного из того, что пролезает мимо сборки. Из добавленных строк диффа мы вытаскиваем все идентификаторы и смотрим, встречается ли каждый из них где‑нибудь в репозитории или в заголовках сторонних либ, и если имя впервые появилось в том же PR, где его вызывают, и при этом в этом же PR его никто не объявил, бот вешает на PR метку и пишет комментарий с найденными именами.

added    = identifiers_from_diff(diff, only_added=True)
declared = declarations_in_diff(diff)
known    = load_index("symbols.idx")   # собирается ночью по всему репо и third_party

for name in sorted(added - declared - known):
    comment(pr, f"{name}: не нашел ни объявления, ни использования")

Казалось бы, если имени нет, то компилятор об этом и так скажет, но в этом месте есть несколько дыр, в которые модель попадает с завидным постоянством. Первая дыра в коде под ifdef для конфигураций, которые PR‑сборка не покрывает, вроде отладочных инструментов редактора, собираемых отдельным таргетом, вторая в шаблонах, которые пока никто не инстанцировал, потому что тело шаблона с выдуманным методом прекрасно компилируется ровно до первого вызова, а третья в именах, которые живут в строках и которых компилятор вообще не видит, про них будет отдельный разговор ниже.

Платить за эту проверку приходится ложными срабатываниями, и поначалу их было столько, что я всерьез собирался ее выключить. Индекс не видит имен, которые склеиваются макросами через ##, а у нас половина компонентов объявляется чем‑то вроде DECLARE_COMPONENT(Health), из которого потом вырастают HealthComponent, HealthComponentPool и еще пара служебных структур, и первые две недели бот ругался на каждый второй PR. Пришлось строить индекс по препроцессированным исходникам, что съело еще минуты три на ночной сборке и около двух гигабайт на диске, и завести файл исключений, который, как и любой файл исключений, имеет склонность разрастаться быстрее, чем его кто‑нибудь успевает прочитать.

Ключи конфигов и прочие фантомы

Самый неприятный класс выдумок живет в строковых литералах, потому что там у компилятора нет вообще никакого мнения. Модель видит, что в соседнем файле читается render.shadows.cascade_count, и уверенно дописывает чтение соседнего ключа, который звучит ровно так же правдоподобно:

// ключ выглядит родным, но в конфигах его никогда не было
float bias = Cfg::GetFloat("render.shadows.cascade_bias", 0.0f);

Ключа нет, геттер молча возвращает дефолтный ноль, тени на одном из уровней слегка отлипают от стен, и замечает это художник по свету недели через три, когда уже никто не помнит, какой PR это принес. То же самое происходит с именами консольных переменных, игровых событий, функций, прокинутых в Lua, параметров шейдеров и ассетов, на которые ссылаются по пути, и везде модель пишет то, что вполне могло бы там быть, если бы кто‑то когда‑то это завел.

Лечится это схемой, которую мы генерируем из всех конфигов, Lua‑биндингов и манифестов ассетов при каждой ночной сборке, и проверкой, которая находит в диффе вызовы Cfg::Get*, CVar::Find, Events::Subscribe и их родственников со строковым литералом и сверяет литерал со схемой. Начинали мы с регулярного выражения на пятнадцать строк и до сих пор с ним живем, потому что на нашем коде оно находит процентов девяносто таких вызовов, а нормальный clang‑tidy чек для оставшихся десяти мы начинали писать дважды и оба раза уходили на что‑то более горящее. Заодно новые геттеры конфигов мы сделали без значения по умолчанию, и если ключа нет, игра в дев‑сборке громко падает прямо на старте, что дизайнеры поначалу встретили без особого восторга и с вопросами, кто это придумал.

Пакеты из воздуха

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

Спасло то, что сборочные машины давно ходят только во внутреннее зеркало, а пакеты в зеркало добавляются руками, так что CI просто не смог его скачать и упал, и автор PR пошел к билд‑инженеру с вопросом «а почему у вас до сих пор нет такой популярной либы». Теперь любое изменение в vcpkg.json, conanfile, requirements и package.json требует апрува от кого‑то из билд‑команды, а до этого апрува CI отдельно проверяет, что каждый новый пакет есть в зеркале, что у него больше одного релиза и что ему больше пары месяцев, и ловит эта проверка в основном честные опечатки, но ради одного нечестного случая ее стоит держать включенной.

Подозрительные заплатки

Отдельно мы смотрим на то, как модель чинит то, что сломалось, потому что когда ее просят «почини тест» или «убери ворнинг», она чаще всего находит самую короткую дорогу к зеленой галочке. В добавленных строках диффа бот ищет pragma, отключающие предупреждения, NOLINT, const_cast, memory_order_relaxed, префикс DISABLED_ в именах тестов, изменения ожидаемых значений в существующих тестах и ранние возвраты по nullptr в начале функций, где раньше такой проверки не было. Последнее в живом коде выглядит обычно как‑то так:

void Ragdoll::Update(float dt)
{
    if (!m_skeleton)   // добавлено, чтобы тест перестал падать
        return;        // почему скелета нет, так никто и не узнал
    ...
}

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

Тесты, которые ничего не проверяют

Модель охотно пишет тесты, и выглядят они солидно, с хорошими именами, с подготовкой данных и десятком ассертов, только проверяют они чаще всего то, что код делает сейчас, потому что ожидаемые значения она берет из той же реализации, которую только что сама написала. Бывают и совсем грустные случаи, когда в тесте замокано все вокруг, включая сам тестируемый класс, и тест добросовестно убеждается, что мок возвращает ровно то, что ему велели вернуть.

Для этого мы прикрутили мутационное тестирование на базе Mull, но запускаем его только по функциям, которые трогает PR, потому что прогон по всему движку занимает больше суток и на ферме ему просто нет места. Mull портит код в этих функциях, переворачивает сравнения, меняет константы, выкидывает вызовы, и смотрит, сколько испорченных версий тесты из PR замечают, а если выжила большая часть мутантов, бот пишет об этом в PR с примерами строк, которые можно сломать безнаказанно. Первый же запуск на свежем PR с сорока новыми тестами на систему инвентаря дал довольно отрезвляющую картину, потому что из примерно семидесяти мутантов тесты убили шесть, а остальные прекрасно жили с перевернутыми условиями и нулями вместо констант.

Обходится это в пятнадцать‑двадцать пять минут на каждый PR, где есть тесты, и в некоторое количество обиды у авторов, потому что сообщение «ваши тесты не заметили перевернутое сравнение» читать неприятно, особенно когда тесты вроде бы писал ты, а по факту их за тебя написала модель, пока ты ходил за кофе.

Санитайзеры и время жизни

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

std::string_view name = entity.GetDebugName();   // вернули std::string по значению
jobs.Schedule([&] { Log("%s", name.data()); });   // строка уже умерла, джоб еще в очереди

На ревью это пролетает мимо глаз почти всегда, особенно если GetDebugName когда‑то возвращал const std::string&, а потом его поменяли, и модель видела старую версию где‑нибудь в соседнем файле или в истории. Поэтому каждый PR дополнительно собирается с ASan и UBSan для модулей, которые он затрагивает, и гоняет их юнит‑тесты, а TSan мы запускаем ночью на стресс‑тестах джоб‑системы, потому что замедление в пять‑десять раз на каждом PR никто терпеть бы не согласился, да и ложных срабатываний на нашем самописном lock‑free коде у него столько, что разбирать их днем не хватило бы ни времени, ни нервов.

Реплеи и плавающая запятая

Симуляция у нас детерминированная, на этом построены и реплеи, и сетевой режим, и вот тут модель ломает то, чего не замечает ни один юнит‑тест. Она охотно «оптимизирует» суммирование, переставляя слагаемые, заменяет наши детерминированные sin и sqrt на стандартные, вставляет std::fma, итерируется по unordered_map в геймплейном коде, и каждое такое изменение математически почти безобидно, только через пару тысяч тиков состояние на двух машинах расходится, и игрок видит, как юнит соперника у него на экране стоит в другом углу карты.

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

Вторая модель в роли ревьювера

Была у нас и очевидная идея поставить вторую модель ревьюить код первой, и мы ее честно гоняли месяца два, но ревьювер находил реальные проблемы примерно с той же уверенностью, что и выдуманные, люди быстро перестали читать его комментарии целиком, а когда перестают читать целиком, со временем перестают читать вообще. От этой затеи прижилось одно узкое применение, при котором модель получает описание PR и дифф и пишет только то, что дифф делает сверх описанного.

Агенты любят по дороге заодно поправить соседний файл, переименовать параметр или поменять дефолт, и автор PR, который смотрел на свою задачу, этого часто сам не замечает, а короткий комментарий «кроме заявленного, изменен порог видимости в AISensing.cpp» читается за пять секунд и пару раз уже сэкономил нам по неделе отлова странного поведения ботов на плейтесте.

Сколько это стоит

Если сложить минуты по шагам, картина по времени на один PR выглядит примерно так:

было:   сборка PC + юнит-тесты                     ~14 мин
стало:  все тулчейны параллельно                   ~22 мин
        индекс имен и схема конфигов               ~2 мин
        ASan/UBSan по затронутым модулям           ~12 мин
        реплеи (если трогали симуляцию)            ~18 мин
        мутации (если в PR есть тесты)             ~15-25 мин
        --------------------------------------------------
        машинного времени на типичный PR           ~60-80 мин
        ожидания у человека                        ~40 мин

Часть шагов идет параллельно, поэтому живой человек ждет минут сорок, но ферма считает все честно, и при наших шестидесяти PR в день это дает лишних часов пятьдесят машинного времени ежедневно. Плюс примерно половину ставки одного билд‑инженера, который держит этот пайплайн в живом состоянии, разбирает файл исключений и объясняет коллегам, почему бот опять ругается. Если положить это рядом с тем, сколько времени модели экономят на написании кода, мы, по моим грубым прикидкам, остаемся в плюсе. Только плюс этот заметно скромнее того, о котором рассказывают на презентациях, где ускорение меряют от первой строки до первого коммита и благополучно забывают про все, что происходит с кодом после пуша.

Где сито пропускает

Есть выдумки, против которых вся эта конструкция бессильна, потому что они компилируются, проходят санитайзеры, не трогают строки и не ломают детерминизм. Модель передает градусы в функцию, которая ждет радианы, путает левую и правую систему координат при конвертации из формата DCC‑пакета, пишет формулу затухания урона, которая выглядит как настоящая и даже похожа на ту, что лежит в дизайн‑документе, и все это ловится только плейтестом, QA или игроками, причем последними чаще, чем хотелось бы.

Вторая дыра сделана нашими же руками, потому что проверки, которые срабатывают часто, люди учатся обходить быстрее, чем читать. Мы уже видели PR, где имя новой функции добавили в файл исключений вместе с самой функцией, видели перезаписанные реплеи без единого слова о том, что поменялось в балансе, и пару раз видели, как в ответ на комментарий бота автор просил модель «сделать так, чтобы бот не ругался», и модель с этой задачей, разумеется, справлялась блестяще.

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

Галочка в пулл‑реквесте

Последнее, что у нас появилось, и, наверное, самое дешевое из всего списка, это галочка в описании PR, которую автор ставит, если код писался с помощью модели, и такие PR обязательно смотрит кто‑то из владельцев модуля, даже когда все проверки зеленые. Ставят ее далеко не все и не всегда, проверить это никак нельзя, но за полгода она стала таким же привычным делом, как ссылка на задачу в трекере, и ревьювер, видя ее, читает дифф медленнее и чаще идет проверять, существует ли вообще‑то, что вызывается.

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

З.Ы. Скрипт для проверки новых имен я тоже набрасывал вместе с моделью и первым, что он поймал после запуска, был несуществующий параметр libclang в нем самом.