diff --git a/docs/reviews/CODE-REVIEW-513-r1.md b/docs/reviews/CODE-REVIEW-513-r1.md new file mode 100644 index 00000000..da18e924 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-513-r1.md @@ -0,0 +1,210 @@ +# CODE-REVIEW-513-r1 + +Заход: r1 · блокирующих циклов израсходовано 0 из 2 + +## Материал + +- Диапазон: `git log --oneline origin/dev..HEAD` → один коммит, + `61905bacdc5a9f08cd70562d7a43b50b8ad88c4a` + («ci: full mutation gate runs nightly, outside the development and release + cycle»), рабочая копия уже на нём. +- Диффстат (`git diff origin/dev...HEAD --stat`): + ``` + .github/workflows/mutation-gate.yml | 22 ++++++++++++---------- + docs/TESTING.md | 4 +++- + scripts/mutation-gate.mjs | 5 +++-- + test/mutation-gate.test.mjs | 6 ++++++ + 4 files changed, 24 insertions(+), 13 deletions(-) + ``` +- Трейлеры коммита: `Issue: #513`, `User-Visible: no` — оба присутствуют, + формат верный. `User-Visible: no` соответствует факту: правка не меняет ни + UX, ни модель данных, ни конфиг — только расписание CI-джобы и тексты + комментариев/доков. Правок `docs/CHANGELOG.md`/`docs/CHANGELOG.ru.md` нет, + что и требуется при `User-Visible: no`. +- Метка `small` — ТЗ в теле issue, файла в `docs/specs/` нет и не должно быть. + Соответствует. + +## Скоуп + +Issue #513: полный мутационный прогон (`mutation-gate.yml`) проверяет не +продукт, а способность тестов падать; решение владельца 09.09 — гонять его +ежедневно ночью по расписанию, вне цикла разработки и вне релизного гейта, +отказ заводит issue отчётом (механизм #472). Диффу соответствует ровно эта +формулировка: правка не трогает продуктовый код, `src/**` не затронут. + +Соответствие docs/SCOPE.md: изменение процессное (CI-гигиена), напрямую не +служит ни одной строке Core user jobs, но и не обязано — это класс C +(инфраструктура тестирования), допустимый вне рамки job-листа по той же логике, +что и остальной тестовый/CI-аппарат проекта. + +## Как проверялось + +1. Прочитал тело issue #513 и хендофф-комментарий (Codex, 09.09) через + `gh issue view 513 --json body,comments,labels`. +2. Прочитал полный `git diff origin/dev...HEAD` — четыре файла, все правки по + существу совпадают с ТЗ пункт за пунктом (см. ниже разбор по AC). +3. Проверил трейлеры коммита (`git log -1 --format=%B`). +4. Прогнал `node --test test/mutation-gate.test.mjs` — 42/42 зелёных + (хендофф заявляет «44/44»; расхождение в счётчике не влияет на + содержание, тесты не переименованы и не удалены относительно dev — + не считаю это находкой, только фиксирую расхождение с заявленным числом). +5. Проверил дисциплину «тест умеет падать»: временно откатил cron в + `.github/workflows/mutation-gate.yml` на старое недельное расписание + (`'20 5 * * 1'`) и перезапустил `test/mutation-gate.test.mjs` — новый тест + `#513 AC1` действительно падает (`not ok 14`), остальные тесты не + затронуты. Файл восстановлен `cp` из бэкапа, `git status --short` после + восстановления — пусто, рабочее дерево чистое. +6. Проверил AC2 отдельно: `if: always() && github.event_name == 'schedule' && ...` + в report-job (`.github/workflows/mutation-gate.yml:125`) не менялся этим + диффом и остаётся верным — ручной dispatch по-прежнему не заводит issue. +7. Проверил отсутствие коллизии по времени с ночным Validate: + `nightly.yml` cron `'30 2 * * *'` (02:30 UTC) против нового + `'0 1 * * *'` (01:00 UTC) в `mutation-gate.yml` — разнесены на 1.5 часа, + как заявлено в комментарии коммита. +8. Проверил AC3 («ни один документ процесса не называет полный мутационный + прогон шагом разработки или релиза») не только на изменённых файлах, а + поиском по всему дереву: `grep -rn "предрелизный гейт"`, + `grep -rn "мутацион"` по `*.md`/`*.mjs`/`*.yml`, поиск остатков старого + cron `'20 5 * * 1'` и фразы «по понедельникам». Здесь нашлась находка — + см. ниже. +9. Проверил `docs/specs/README.md` (ТЗ пункт 3 называет этот файл в списке + документации к правке) — файл не содержит вообще ни одного упоминания + мутационного гейта ни до, ни после дельты; менять там было нечего, это не + пропуск, а пункт ТЗ, применимый только если бы там было устаревшее + упоминание. + +## Находки + +### Medium (в скоупе) — `scripts/pre-push-gate.mjs` продолжает называть полный мутационный реестр предрелизным гейтом + +`scripts/pre-push-gate.mjs:22-24` (комментарий) и `scripts/pre-push-gate.mjs:182-183` +(текст, который печатается разработчику при каждом локальном прогоне перед +push) буквально утверждают: + +``` + * HA-харнесс и весь мутационный реестр — предрелизный гейт; здесь только то, +... +console.log(' · golden, полная матрица смоков, HA-харнесс, весь мутационный реестр —' + + ' предрелизный гейт, не этот набор'); +``` + +Это прямо противоречит решению владельца и AC3 этой задачи: полный +мутационный реестр (`mutation-gate.yml`, шардированный прогон) с этим диффом +явно выведен и из цикла разработки, и из релизного гейта — он живёт только по +ночному расписанию, отвязанно от push и релиза (см. новые комментарии в самом +`mutation-gate.yml` и `docs/TESTING.md`). А `pre-push-gate.mjs` — это именно +инструмент цикла разработки (запускается разработчиком перед push) и именно +документ процесса в духе AC3: он объясняет, какие гейты куда относятся. После +этой правки он продолжает говорить каждому разработчику, что «весь +мутационный реестр» нужно гонять «перед стабильным релизом» — а это больше не +так. + +**Воспроизведение:** `node scripts/pre-push-gate.mjs` (или просто чтение файла) +— строка 183 напечатает утверждение, которое прямо противоречит новому тексту +`docs/TESTING.md:33-34` («в цикле разработки и в релизном гейте он не +участвует») и новому комментарию `scripts/mutation-gate.mjs` +(«вне цикла разработки и релиза»). + +**Почему это Medium, а не High:** ничего не ломается технически — CI не +краснеет, тесты не врут о своём результате, само поведение гейтов (cron, +report-job) реализовано верно и проверяется тестом. Это стилистически- +информационный дефект: неверная строка в выводе локального скрипта и в его +шапке-комментарии, который непосредственно относится к предмету задачи и +буквально противоречит её AC3 («ни один документ процесса не называет полный +мутационный прогон шагом разработки или релиза»). Это тот случай, который +задача была обязана закрыть, но не закрыла — правка тривиальна (переформулировать +две строки), поэтому это годится чинить прямо в этой задаче, а не заводить +отдельный issue (Medium в скоупе, PROCESS.md §12). + +**Предложение по правке (не мной вносится, для автора):** заменить +«HA-харнесс и весь мутационный реестр — предрелизный гейт» на формулировку, +согласованную с новым текстом `docs/TESTING.md`/`mutation-gate.mjs` — +например, «HA-харнесс — предрелизный гейт, весь мутационный реестр — ночное +расписание вне цикла разработки и релиза (#513)» — в обоих местах (комментарий +и `console.log`). + +## Разбор по AC + +- **AC1** (cron ежедневный `0 1 * * *`, тест краснеет при смене на недельное). + Выполнен. `mutation-gate.yml:29` → `- cron: '0 1 * * *'`. Тест + `test/mutation-gate.test.mjs:263` пинит этот cron и явно запрещает + day-of-week компонент и фразу «перед стабильным релизом». Тест умеет + падать — проверено экспериментально (см. «Как проверялось», п.5). +- **AC2** (report-job заводит/дописывает issue только для schedule-прогонов). + Выполнен, без изменений в этом диффе — условие `github.event_name == 'schedule'` + на месте (`.github/workflows/mutation-gate.yml:125`), покрыто существующим + тестом `#472 AC1`/сопутствующими (не переписывался этой задачей, дельта его + не касается). +- **AC3** (ни один документ процесса не называет полный прогон шагом + разработки или релиза). **Не выполнен полностью** — см. находку выше: + `scripts/pre-push-gate.mjs` продолжает называть весь мутационный реестр + «предрелизным гейтом» в комментарии и в фактическом выводе скрипта. + `docs/TESTING.md`, `scripts/mutation-gate.mjs` (шапка) и + `.github/workflows/mutation-gate.yml` (комментарии) — исправлены верно. +- **AC4** (`User-Visible: no`; UX/i18n/модель данных не затронуты). + Выполнен — диффу не затрагивает `src/**`, только CI/доки/тест; трейлер + коммита соответствует. + +## Что проверено и корректно + +- Cron не пересекается по времени с ночным Validate (1.5 часа разницы). +- `workflow_dispatch` по-прежнему доступен для отладки самого гейта, но + report-job на него не реагирует — issue не заводится на ручных прогонах, + как и требовалось. +- Текст `docs/TESTING.md` и шапка `scripts/mutation-gate.mjs` согласованы + друг с другом и с новым поведением workflow. +- Мутанты по диффу (#510, локальный `--changed`) — не путаются с полным + ночным прогоном, это разные механизмы, задача их не смешивает. +- Класс изменений (C: workflow/доки/тест) соответствует фактическому диффу, + `git diff --stat` не содержит правок продуктового кода. + +## Чего не проверял и почему + +- `npx tsc --noEmit`, `npm test` (полный), `npm run build` со сверкой трёх + копий бандла — **не прогонял отдельно**. Validate на этом же SHA + (`61905bac`) зелёный (ссылка в задаче на прогон), diff не трогает + `src/**`, поэтому дельта с этим прогоном нулевая по тем поверхностям, + которые эти гейты проверяют. +- `node scripts/check-docs.mjs` — не прогонял: diff не трогает `src/**` + (только `.github/workflows`, `docs/TESTING.md`, `scripts/mutation-gate.mjs`, + `test/mutation-gate.test.mjs`), отпечаток скриншотов документации не мог + устареть от этой правки. +- `npm run invariants` / инварианты модели — не применимо: диффу не + затрагивает геометрию, рёбра, `layout`, `marker.space`, `open_spans`. +- Браузерные смоки `demo/smoke_*.mjs` — не прогонял и не выбирал через + `scripts/smoke-select.mjs`: diff не трогает ни один файл, от которого + зависит рендер или демо-сервер; выборка тривиально пуста по составу diff'а + (workflow/CI-доки/тест одного скрипта). +- `npm run golden:verify` — не применимо, визуал не менялся. +- `python -m pytest tests_backend` — не применимо, `custom_components/**/*.py` + не тронут. +- Performance-профили — не применимо, в AC не названы и чувствительный код не + тронут. +- Полный `node scripts/mutation-gate.mjs` (без `--changed`) — не прогонял: + это дорогой прогон (~37 мин, ~100 job-мин), сама задача выводит его из + цикла разработки и ревью; локально прогнал только дешёвую часть — + `test/mutation-gate.test.mjs` (см. «Как проверялось»). +- «Одно число — один источник»: неприменимо, диффу не добавляет и не меняет + ни одной пользовательски видимой величины (только CI-расписание и текст + документации/комментариев). + +## Вердикт + +Жёлтый. AC1, AC2, AC4 выполнены и подтверждены (тест краснеет на мутации, +report-job условие корректно, User-Visible верен). AC3 не закрыт полностью: +`scripts/pre-push-gate.mjs` остаётся документом процесса, который прямо +противоречит принятому в этой же задаче решению — Medium в скоупе, чинится в +этой же задаче без нового issue. + +--- + + + +## Материал раунда + +- Ветка: `issue/513-mutation-gate-nightly`, коммит `61905bacdc5a` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `0ff88d8c67bc15b1170c5a0164b6d94f15af2bfe` + ``` + git log --all --format='%H %T' | grep 0ff88d8c67bc + ``` +- Вердикт конвейера: `yellow` · High 0