Files
healthlog/docs/review-journal.md
T
av 9ad1deeb01 ревью: idiom упразднён, его класс переселён в ops и architecture
- эксперимент против поведения stdlib, драйвера и PRAGMA — обязательный
  вопрос 8 у ops, с прецедентом «-1 >= -1» и оговоркой про data_version
- «не изобретаем ли то, что уже есть в библиотеке» — вопрос 1 у architecture,
  с перечнем конструкций stdlib
- потеряна поимённая сверка с Effective Go и стайлгайдами: класс обратимый,
  но теперь не покрыт вовсе — записано в журнал ревью
- профили: quick 4, standard 6, deep 7–8, design 3
2026-08-02 20:52:45 +03:00

210 lines
21 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Журнал проскочивших дефектов
Сюда попадает дефект, который **прошёл ревью и всплыл позже**. Записывается
сразу, а не ретроспективно: со временем теряется не сам факт, а причина
непоймания — единственное, ради чего журнал существует.
Реализованные задачи, находки ревью и решения сюда не пишутся: у них есть
коммит, спека и беклог. Здесь только промахи конвейера.
Форма записи:
```
## 2026-08-01 — <краткое последствие>
- **Где:** internal/store/bucket.go:120
- **Симптом:** <как обнаружилось, кем и когда>
- **Почему не поймали:** <какой проход обязан был найти и что ему помешало>
- **Что меняем:** <правило прохода, шаг гейта, конвенция — либо «ничего, цена
поимки выше цены дефекта»>
```
Последний пункт важнее остальных. Вывод «ничего не меняем» — законный исход:
не всякий дефект стоит того, чтобы усложнять ради него ревью каждой задачи.
---
## 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.md](conventions.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` 78, `design` 3.
Было 11 на коде и 4 на дизайне.