# Ревью: настройка и журнал Конвейер — скилл `av-dev-pipeline:review-pipeline`, проходы — агенты `av-dev-pipeline:review-*`. Здесь только проектная часть: чем этот проект отличается от умолчаний конвейера и что в нём уже проскакивало. ## Как настроен конвейер ### Типовые узлы Рода узлов проекта и свойства, по которым судится каждый. Рода, а не инвентарь пакетов: род, который проект задумал, но ещё не написал, включён намеренно. **Разбор пакета HAE** (`internal/hae`) - Точка сохраняется дословно; ничего внутри неё не отбрасывается и не переименовывается. - Непонятое содержимое не роняет доставку: она принята, непокрытое названо. - Слой выводится из выравнивания меток, а не из заголовка HAE — тот врёт. - Текст ошибки не содержит значений из входа — род токена и смещение. - Тест гоняется на реальном пакете из `testdata`, а не на выдуманном. **HTTP-обработчик приёма** (`internal/httpapi`) - Код ответа отражает доставку, а не разбор: битый JSON — 400, непонятое содержимое — 200. - Тело попадает в архив раньше, чем в разбор; потеря архива необратима. - Тело и заголовки в лог выше `DEBUG` не уезжают, токены — никогда. - Есть названный предел на размер тела и на заголовки. **Свёртка и репозиторий часовых объектов** (`internal/store`, `internal/fold`) - Результат — функция **префикса** журнала: узел не читает состояние, которое сам же меняет, без границы по `received_at` разбираемой доставки. - Правило выбора между версиями — функция множества версий либо явно функция порядка журнала; третьего состояния нет. - Столкновение разрешается полнотой, а не свежестью; изменение запечатанного часа пишется `WARN`, но данные пишутся. - Транзакция не держит блокировку дольше `busy_timeout`: канонизация и сжатие — вне её. **Файловый архив и ретеншен** (`internal/archive`) - Путь строится из значений, которых отправитель не контролирует. - Удаление тела опирается на колонку, отличающую ноль от «не измерялось». - Место на диске и рост каталога названы числом. **Проигрыватель журнала и CLI** (`internal/replay`, `cmd/`) - Повторный прогон даёт то же состояние и тот же отпечаток. - Новая единица хранения входит в отпечаток и в счётчики отчёта. - Расход памяти не растёт вместе с длиной журнала. - Подмена базы — решение человека при остановленном сервисе, не команды. **Обработчик чтения и адаптер MCP** (Read API, MCP — ещё не написаны) - Агрегат считается только там, где род свёртки измерен; нижний слой HAE не суммируется никогда. - Ответ имеет предел размера, и предел объявлен, а не подразумевается. - Адаптер MCP собственной логики не несёт — те же обработчики. ### Типовые ложноположительные Находки, которые здесь выглядят убедительно и всегда неверны. - **«Ответ 200 на непонятое содержимое проглатывает ошибку.»** Не дефект: инвариант «сохранили — значит приняли». Телефон шлёт молча и не перешлёт — код ответа отражает доставку, а не разбор. - **«`source` не входит в ключ — идентичность неполна.»** Не дефект: поле измерено нестабильным (разведка, находка 36), включение его в ключ задваивает точки. - **«Точка хранится избыточно, поля дублируются.»** Не дефект: точки хранятся дословно, инвариант прямой. Экономия здесь необратима. - **«Часовой объект не считает агрегат при записи.»** Не дефект: своей агрегации в хранении нет, род свёртки выводится сверкой слоёв в ответе. - **«У одной метки три записи сна — дубликат.»** Не дефект: у точки-измерения конец равен началу, под одной меткой лежит до трёх записей. - **«Русские строки в значениях — незакрытая локализация.»** Наполовину: строки приходят на языке телефона, и это факт источника; дефектом является только отсутствие стабильного кода рядом с переводом. ### Вопросы к проходам Форма: `<имя прохода>: <вопрос> (<провенанс>)`. Задаются дополнительно к обязательным. - `ops`: читает ли узел состояние, которое сам же меняет, и остаётся ли результат функцией от **префикса** журнала (запись 2026-08-01, свёртка не воспроизводилась при пересборке). - `ops`: поведение библиотеки, драйвера и `PRAGMA` измерено или вычитано из документации; что возвращается в **вырожденном** случае и отличим ли этот ответ от штатного (запись 2026-08-02 про упразднение `idiom`; прецедент `-1 >= -1` — 1492 тика из 5502). - `ops`: хватит ли сигналов владельцу, когда поток оборвётся ночью (переселено из упразднённого `negative`). - `architecture`: не изобретаем ли то, что уже есть в стандартной библиотеке — своя абстракция, повторяющая форму существующей (переселено из `idiom`). - `architecture`: что опытный человек отсюда удалил бы (переселено из `negative`). - `rubric`: пришпилено ли утверждение теста к числу, производному от размера корпуса — корпус растёт с каждой доставкой (запись 2026-08-02, прогон живого архива был красным). - `adversary`: доводится ли значение точки или тело доставки до лога выше `DEBUG` хотя бы одним путём (запись 2026-08-02, тело 8 МиБ в тексте ошибки). - `triage`: перечислены ли запущенные проходы поимённо и с исходом; непущенный проход идёт в границы покрытия строкой «не запускался» (запись 2026-08-02, чекпоинт кода прошёл без трёх проходов). ### Триггеры профиля Уточняет умолчания конвейера, не отменяет их. - **`deep`** — изменения в правиле разбора, идентичности, слияния или вывода слоя; миграции схемы; всё, что трогает `internal/store`, `internal/fold`, `internal/replay`. - **«Поведение, видимое снаружи»** здесь — код ответа приёма, форма ответа чтения, содержимое архива и **состояние, которое даёт пересборка**: витрина наблюдаема через пересборку, поэтому расхождение с журналом — внешнее поведение, а не внутренняя деталь. - **`reimpl`** запускается по триггеру «новое правило слияния, идентичности или разбора». Единственный раз, когда триаж назвал его отсутствие дырой покрытия, — это была задача с новым правилом слияния сущностей. - **`quick`** — правка документов, конфигурации, сообщений; ничего, что меняет хранимое. ### Недоступно проверке **Не проверит ни один проход.** Реальный профиль нагрузки: телефон шлёт молча и непрерывно, объём и частота меряются только по факту. Поведение приложения HAE за пределами наблюдённого — расписание автоматизаций пожелание, а не гарантия (разведка, находка 28). Полнота словаря переводов после обновления iOS. Секции, которых поток ещё не приносил: `symptoms`, `ecg`, `heartRateNotifications`, `cycleTracking`, `medications` — разбор писался вслепую, и проход может судить только форму кода, не соответствие реальности. **Перестали проверять сознательно.** - Прогон живого архива (`task verify:archive`) и свёртка под удерживаемой блокировкой (`task verify:busy`) в гейт не входят: минута и 25 секунд соответственно, плюс данные, которых нет ни на какой другой машине. Гоняет их человек перед задачей, трогающей разбор или слияние (запись 2026-08-02, прогон живого архива был красным и об этом никто не знал). - Класс «в Go так не пишут» — поимённая сверка с Effective Go, Go Code Review Comments, стайлгайдами Uber и Google — не покрыт вовсе после упразднения `idiom`. Класс обратимый, портит форму кода, а не данные, но признавать это надо в границах покрытия, а не считать проверенным (запись 2026-08-02). - Класс «чего нет в зрелой реализации такого узла» — вне профиля `design`. ## Журнал дефектов Запись на каждый воспроизведённый дефект, сразу, а не ретроспективно: со временем теряется не факт, а причина непоймания. Реализованные задачи, находки ревью и решения сюда не пишутся: у них есть коммит, спека и задача. Здесь только промахи конвейера и решения о его составе. Форма: ## ГГГГ-ММ-ДД — <краткое последствие> [проскочил|пойман] - **Где:** путь:строка либо «конвейер, а не код» - **Симптом:** как обнаружилось, кем и когда - **Причина:** что на самом деле было не так - **Чем воспроизведён:** тест, команда, замер — с числами - **Почему не поймали:** только для проскочивших — какой проход обязан был найти и что ему помешало - **Что меняем:** правило прохода, шаг гейта, конвенция, факт в документе проекта — либо «ничего, цена поимки выше цены дефекта» Последний пункт важнее остальных. Вывод «ничего не меняем» — законный исход: не всякий дефект стоит того, чтобы усложнять ради него ревью каждой задачи. --- ## 2026-08-01 — свёртка не воспроизводилась при пересборке журнала [проскочил] - **Где:** `internal/store/delivery.go`, `LastDerivedLayer` - **Симптом:** прогон живого архива (99 доставок) вторым проходом дал 1742 объекта вместо 1737, а координат сна 182 вместо 174. Нашёл тест сходимости на шаге apply — не ревью. - **Причина:** доставка без плотных метрик наследует слой автоматизации. Запрос брал последний выведенный слой **вообще**, а не последний до этой доставки, поэтому при пересборке доставка наследовала слой «из будущего». Свёртка переставала быть функцией от префикса журнала. - **Почему не поймали:** формулировка «наследует последний надёжно выведенный слой той же автоматизации» звучит однозначно и в спеке, и в дизайне — пропущенное слово «предшествующей» не выглядит пропуском. Проходы `specs` и `architecture` сверяли код со спекой и понятиями, а инвариант «`import + replay` даёт то же состояние» ни один из них не проверял на конкретном правиле: он записан в архитектуре как свойство системы, а не как критерий для каждого узла, читающего состояние. - **Что меняем:** в рубрику `healthlog-review-rubric` и в проход `ops` — вопрос «читает ли узел состояние, которое сам же меняет, и остаётся ли он функцией от префикса журнала». Дешевле правила: любой запрос к `delivery` из свёртки обязан иметь границу по `received_at` разбираемой доставки. Тест сходимости на живом архиве (`internal/fold/replay_test.go`) остаётся постоянным — именно он это поймал. ## 2026-08-02 — прогон живого архива был красным и об этом никто не знал [проскочил] - **Где:** `internal/fold/replay_test.go` (перенесён в `internal/replay/archive_test.go`) - **Симптом:** первый же запуск `task verify:archive` в задаче про пересборку дал `координат sleep_analysis 222, измерено 174`. Проверено прогоном прежней редакции теста на том же архиве: она даёт ровно те же 222, 2049 объектов и тот же отпечаток — значит тест покраснел не от изменений задачи, а сам, когда архив дорос с 94 доставок до 116. - **Причина:** утверждение было пришпилено к **числу, производному от корпуса** (174 координаты сна). Корпус растёт с каждой доставкой, то есть константа протухает по расписанию телефона. Проверяемое свойство при этом другое и от размера корпуса не зависит: ключ по интервалу не схлопывает записи до ключа по метке (222 координаты против 218 меток). - **Почему не поймали:** прогон живого архива намеренно не входит в `task gate` (минута работы, данные есть только на этой машине). У проверки, которую гейт не гоняет, краснота никому не видна — она обнаруживается только следующей задачей, которая до неё дотянется. Ни один проход ревью прогон не запускал: проходы читают код, а не гоняют опциональные команды. - **Что меняем:** утверждение переписано на само свойство (координат строго больше, чем различных меток), измеренные числа остались в `t.Logf`. Правило общее и годится в конвенции: **в проверке на живом корпусе нельзя утверждать число, производное от размера корпуса** — утверждать надо инвариант, а число печатать. Гейт при этом не трогаем: цена ежедневной минуты выше цены такой протухшей константы, а после этой задачи прогон стал ещё и единственным, кто проверяет настоящий проигрыватель журнала. ## 2026-08-02 — чекпоинт кода прошёл без трёх проходов, и ровно они нашли всё [проскочил] - **Где:** конвейер, а не код: коммит `f8200f7` («тренировки и записи с собственным `id`»), шаг 7 скилла `healthlog-task-pipeline`, профиль `deep`. - **Симптом:** изменение было закоммичено и заархивировано как прошедшее ревью. Дозапуск трёх пропущенных проходов на **уже закоммиченном** коде дал девять причин, семь из которых пошли в работу с прогнанными оракулами: скелет из `null` затирает маршрут молча и необратимо; одно поле не той формы уносит тренировку, а доставка при этом числится разобранной; откат бинаря поверх новой схемы стартует без слова; победитель внутри доставки зависит от порядка элементов на проводе; провенанс устаревает на каждой повторной присылке; канонизация идёт внутри транзакции вопреки собственному комментарию (768 МиБ пика, 5.019 с удержания блокировки); тело в 8 МиБ целиком уезжает в текст ошибки и оттуда в `WARN`. - **Причина:** сабагент, проводивший задачу, на чекпоинте кода запустил не все проходы профиля `deep` — не отработали `adversary`, `ops` и архитектурный. Отчёт триажа при этом был выпущен и выглядел полным: он агрегирует то, что ему подали, и о непоступивших проходах не знает. Секция границ покрытия обязана была это назвать, но она заполняется тем же триажем — то есть единственный, кто мог заметить пропуск, узнаёт о нём из того же источника, который его допустил. - **Почему не поймали:** пропуск прохода **не отличим от прохода без находок**. Гейт зелёный, спеки сошлись, applicative-проходы отработали — снаружи это выглядит как чистое ревью. Все семь находок принадлежат ровно тем классам, которые applicative-проходы не достают по построению: враждебно сконструированный вход (`adversary`), поведение под откатом и конкуренцией (`ops`), второй способ делать уже сделанное (архитектура). Recall чек-листа равен длине чек-листа, а этих пунктов в чек-листах нет и быть не может. - **Что меняем:** отчёт ревью обязан перечислять запущенные проходы **поимённо и с исходом**, а оркестратор задачи — сверять этот перечень с составом профиля до того, как коммитить; непущенный проход идёт в границы покрытия строкой «не запускался», а не отсутствует. Правилом линтера это не выражается, автоматической проверки нет — но пропуск, названный в отчёте, стоит одной строки, а пропуск молчащий стоил семи находок и отдельной задачи на их дозакрытие. Состав проходов и профилей при этом не трогаем: они сработали ровно так, как задуманы, — их просто не позвали. ## 2026-08-02 — тест на утечку значений в лог краснел от хода часов [проскочил] - **Где:** `internal/fold/log_test.go`, `TestFoldНесравнимыеНаборыДаютWarn` - **Симптом:** гейт задачи про цену читающего маршрута покраснел на чужом тесте: «в логе оказалось значение точки "5.1"». Значения в логе не было — подстрока нашлась в метке времени записи (`…T20:23:35.193…` содержит `5.1`). Повторный прогон зелёный. - **Причина:** утверждение искало секрет в **сыром буфере** записи, а буфер содержит служебное поле `time` с долями секунды. Вероятность совпадения для двухсимвольного числа с точкой — около процента на прогон, то есть тест флаки по построению, и краснеет он у того, кто мимо проходил. - **Почему не поймали:** шаг `flaky` гейта гоняет набор дважды подряд — вероятность поймать однопроцентную флаки за два прогона мала, а сам тест выглядит образцовым: он проверяет ровно тот инвариант, который проекту дороже всего («данные о здоровье чувствительнее токенов»). Ни один проход ревью не смотрит на тесты чужих задач. - **Что меняем:** правило в [conventions/testing.md](conventions/testing.md) — проверка «в логе нет значения» разбирает запись и выбрасывает `time`, а не ищет в сыром буфере. Гейт не трогаем: два прогона против однопроцентной флаки не помогут, а десять стоили бы дороже самой находки. ## 2026-08-02 — состав конвейера сужен: 11 проходов до 6–9 Не промах, а решение по итогам пяти задач подряд. Записано здесь, потому что именно здесь лежит цена непоймания: если что-то теперь проскочит, первый вопрос будет «не тот ли это класс, который мы перестали проверять». - **Повод:** профиль `deep` стоял на всех пяти задачах сессии и гонял 11 проходов на коде плюс 4 на дизайне — порядка полутора миллионов токенов на задачу. Ревью, а не написание кода, стало основной статьёй расхода. - **На чём основано:** поимённая атрибуция находок надёжна только для дозапуска трёх проходов на `f8200f7` — там оркестратор запускал их сам. В двух циклах, которые вели сабагенты, находки перечислены без указания прохода, и это ограничение вывода названо здесь честно. - **Что убрано и почему:** - `negative` — **удалён**. За сессию ни одной именной находки; блокер про откат релиза он нашёл дублем с `ops`, то есть заплатил триажу дедупликацией. Два его живых вопроса переселены: «хватит ли сигналов владельцу, когда поток оборвётся ночью» — в `ops`, вопрос 7; «что опытный человек отсюда удалил бы» — в `architecture`, вопрос 5. - `rubric` — **только в `design`**. Его же 14 свойств из design-прогона ложатся приёмочными критериями в `tasks.md`; судить код по критерию, под который он писался, — корреляция по построению. - `reimpl` — **по триггеру** «новое правило слияния, идентичности или разбора». Самый дорогой проход конвейера; единственный раз, когда триаж назвал его отсутствие дырой покрытия, — это была задача с новым правилом слияния сущностей, то есть ровно триггерный случай. - **Что переставлено, и это важнее сокращения:** `adversary` и `ops` были в `deep`-только, а `standard` гонял четыре самых слабых generative-прохода. То есть профиль, которым закрывается большинство задач, запускал ровно тех, кто ничего не принёс, и не запускал тех, кто принёс почти всё. Оба переехали в `standard`. Это одновременно дешевле и качественнее. - **Что чуть не убрали по ошибке:** `idiom` был в списке на удаление как «вкусовщина». Отменено фактом: в задаче про цену читающего маршрута он нашёл, что `-1 >= -1` читается как «журнал разобран целиком», и **воспроизвёл** — 1492 тика из 5502. Плюс три эксперимента на дизайне `razbor-metrik-v-obekty`. Вывод, который стоит помнить: этот проход зарабатывает **экспериментами против поведения stdlib и драйвера**, а не цитатами из гайдов, — и потому у него есть внешний оракул. Оценка «не всплыл поимённо ни разу» была верна по имевшимся данным и неверна по существу. - **Что мы сознательно перестали проверять:** класс «чего нет в зрелой реализации такого узла» вне профиля `design`, и «пять вопросов второго инженера» как отдельная постановка. Обратимость этого класса высокая: он портит форму кода и полноту наблюдаемости, а не данные. Если проскочит дефект этого класса — запись сюда и пересмотр решения. - **Побочная выгода, ради которой стоило резать отдельно:** реестр из 6–9 проходов сверяется взглядом. Промах 2026-08-02 (запись выше) был молчащим пропуском трёх проходов из одиннадцати; на коротком списке требование «перечисли запущенные проходы поимённо и с исходом» наконец выполнимо. ## 2026-08-02 — `idiom` тоже упразднён, класс переселён Решение владельца, принятое после того, как оркестратор привёл доводы против удаления (находка на чекпойнте WAL, воспроизведённая: 1492 тика из 5502) и они были выслушаны. Записано отдельной строкой, потому что довод был, и если класс проскочит — искать надо здесь. - **Что переселено, а не выброшено.** Проход зарабатывал экспериментами против поведения stdlib и драйвера, и именно эта способность перенесена поимённо: - «поведение библиотеки, драйвера и `PRAGMA` измеряется, а не вычитывается из документации; что возвращается в **вырожденном** случае и отличим ли этот ответ от штатного» — в `ops`, обязательный вопрос 8, вместе с прецедентом `-1 >= -1` и оговоркой про `data_version` как свойство соединения; - «не изобретаем ли то, что уже есть в библиотеке» — в `architecture`, вопрос 1, с перечнем конструкций stdlib: своя абстракция, повторяющая форму существующей, — находка того же класса, что и второй способ делать одно и то же. - **Что действительно потеряно.** Поимённая сверка с положениями Effective Go, Go Code Review Comments, Go Proverbs и стайлгайдов Uber и Google. Различение «идиоматично» против «распространено» больше не задаётся никем: `architecture` спрашивает про форму решения, `ops` — про поведение под нагрузкой, но ни один не спросит «в Go так не пишут». Класс обратимый — портит форму кода, не данные, — но он теперь не покрыт вовсе, и это надо признавать в границах покрытия, а не считать проверенным. - **Итог по конвейеру:** `quick` 4, `standard` 6, `deep` 7–8, `design` 3. Было 11 на коде и 4 на дизайне.