docs: review document for #727

Issue: #727
User-Visible: no

docs/reviews/CODE-REVIEW-727-r1.md

@@ -0,0 +1,196 @@
+# CODE-REVIEW-727-r1
+
+Issue: #727 · этап code · трек `ask` · заход r1 · блокирующих циклов 0/4
+Материал: `git log --oneline origin/dev..HEAD` / `git diff origin/dev...HEAD`,
+ровно `93be52ebe8c3f7860eeb955c5dda2a6d50bdbabe` (рабочая копия уже на нём)
+
+## Скоуп
+
+Один коммит поверх `dev` (`52dc08a0`): ночной режим пакетного ревью `ship` и
+переиспользование его результата гейтом беты по патч-набору (К1–К9, AC1–AC10
+из ТЗ в теле #727). Классы изменений — B (`scripts/**`, `.github/workflows/**`,
+`test/**`) и C (`PROCESS.md`, `docs/process/REVIEWER.md`); файлов класса A нет.
+`User-Visible: no` — изменений в обоих CHANGELOG не требуется и нет
+(проверено: `git diff --stat` по `docs/CHANGELOG*.md` пуст).
+
+Файлы: `scripts/ship-review.mjs` (+428/-…), `scripts/reviews-archive.mjs`,
+`scripts/reviews-index.mjs`, `.github/workflows/_nightly.yml`,
+`.github/workflows/_ship-review.yml`, `PROCESS.md` (§10.4, §11.7),
+`docs/process/REVIEWER.md`, `test/ship-review.test.mjs`,
+`test/nightly-workflow.test.mjs`, `test/process-digests.test.mjs`.
+
+Спец-ревью пройдено за r1→r2 (единственная Medium-находка r1 по К7/АС7 —
+нереализуемость правила архивации для стабильной базы без примера в АС —
+закрыта в r2, вердикт зелёный, `docs/reviews/SPEC-REVIEW-727-r2.md`). Трек
+`ask` подтверждён верно: сложность/риск >3 (четыре поверхности) и публичный
+контракт гейта беты меняется.
+
+## Как проверялось
+
+Прочитано построчно: `scripts/ship-review.mjs` целиком (591 строка, не только
+дифф), `scripts/reviews-archive.mjs`/`reviews-index.mjs` диффы с контекстом
+вызывающих функций (`compareStable`, `stableTagsThrough`, `ordered`),
+`.github/workflows/_nightly.yml` и `_ship-review.yml` целиком, `PROCESS.md`
+§10.4/§11.7 и `docs/process/REVIEWER.md` диффы, весь новый текст
+`test/ship-review.test.mjs` (580 новых строк) и `test/nightly-workflow.test.mjs`
+(137 новых строк).
+
+Каждый AC1–AC9 сверен построчно с кодом (оракул AC → конкретная функция/шаг →
+тест, который его проверяет). AC10 — гейты, ниже.
+
+**Исполнено, не только прочитано:**
+- `node --test test/ship-review.test.mjs test/nightly-workflow.test.mjs
+  test/process-digests.test.mjs test/default-branch-workflows.test.mjs` —
+  29 + … тестов, все зелёные (включая тесты на настоящем bash/git во временных
+  репозиториях — `shipSandbox`, `tempRepo`, `runStep`).
+- `node scripts/mutation-gate.mjs --check` — exit 0, 3 предупреждения (те же
+  три имени, что называет автор как «как на dev»: `corpus-loses-its-short-edge`,
+  `optimize-reports-work-it-did-not-do`, `nightly-reuse-accepts-stale-marker` —
+  все три про геометрический корпус/#650, не про этот дифф). Якорь
+  `ship-review-ignores-merge-marker` найден и зелёный, новые мутанты
+  `ship-review-accepts-partial-coverage`/`ship-review-accepts-high` в реестре
+  есть и зелёные.
+- `node scripts/entry-cost.mjs --check` — зелёный.
+- `node scripts/reviews-index.mjs --check` — зелёный («INDEX.md свеж»).
+- `node scripts/smoke-select.mjs --base origin/dev --head HEAD` — «исполняемого
+  frontend-диффа нет, browser-smoke не выбираются»: `src/**/*.ts` не тронут,
+  смоки объективно нечего выбирать, а не решение их пропустить.
+- **Мутационная проверка вручную** (дисциплина «тест умеет падать») на двух
+  участках с наибольшим риском тихой порчи:
+  - `samePatchSet` → `return true` (покрытие никогда не станет `stale`):
+    упал AC3, AC4, AC5 и интеграционный AC2/AC5-тест — 4 из 19 тестов файла.
+  - `archivePlan`: `ordered.find(...)` → `[...ordered].reverse().find(...)`
+    (брать не ближайшую линию новее базы, а самую дальнюю — тестовый случай
+    АС7 (в), `[v1.78.0, v1.78.1, v1.79.0]` должен дать `v1.78.1`): упал ровно
+    тест АС7. Файл восстановлен `cp` из бэкапа после обеих проверок,
+    `git diff` после — пуст.
+
+**Не прогонял и почему:** `actionlint` — бинарь недоступен в этой песочнице;
+автор заявляет «чистый по изменённым файлам», не перепроверено независимо —
+риск низкий: YAML `run:`-блоки этого диффа уже проверяются `bash -n` в
+исполняемых тестах (`test/ship-review.test.mjs`,
+`test/nightly-workflow.test.mjs`), а структурные инварианты (поля job, порядок
+шагов, `needs`/`if`/`permissions`) — текстовыми assert'ами на реальном файле.
+`npx tsc --noEmit` / `npm test` (полный) / `npm run build` — не перегонял:
+Validate зелёный на этом SHA (ссылка в промпте ревью), дешёвые гейты закрыты
+им. Python/`pytest tests_backend` — не трогается, нет правки
+`custom_components/**/*.py`. `npm run golden:verify` и `invariants` — нет
+`ci:golden`, нет правки геометрии/рендера. Performance — не назван в AC.
+
+## Находки
+
+Нет High. Нет Medium в скоупе задачи.
+
+**Low (наблюдение, не блокирует, не правлю и не снимаю — требует решения
+владельца о сроке).** ТЗ (раздел «Разбиение») инструктирует: «Новый issue E
+(завести в `S1-new`, ссылка на #727): «Красная ночь: комментарий в задачи,
+слитые после последней зелёной»». Поиском по заголовку/метке `S1-new` такой
+issue в репозитории не найден — похоже, не заведён. Это явно вне скоупа
+самого #727 (раздел «Не-скоуп»: «Красная ночь — issue E»), поэтому не влияет
+ни на один AC1–AC10 и не блокирует это ревью; но без заведённой задачи
+инструкция ТЗ технически не выполнена полностью, и решение, которое владелец
+принял при развилке #707, рискует потеряться. Не завожу отдельным issue сам:
+это не найденный попутный дефект кода (критерий §12 «вне скоупа»), а
+административный шаг, порядок исполнения которого (сейчас / при следующей
+правке конвейера) решает не ревьюер.
+
+**Low (наблюдение).** `readRangeDocs` — новая, нетривиальная функция (сортировка
+по глубине коммита через `rev-list --count`, дедупликация документа с одним
+именем между `candidate` и `devRef` в пользу `devRef`, отсечение чужой базы
+для ночных имён по regex). Прямого unit-теста на саму функцию нет: она
+проверяется только через интеграционный `shipSandbox`-тест, где сценарий
+«один и тот же документ с разным содержимым одновременно в дереве кандидата и
+в `origin/dev»` не встречается (там документ либо ещё не существует, либо уже
+запушен в оба места с одним и тем же содержимым). Логика — простая перезапись
+`Map` по порядку обхода `[candidate, devRef]`, поведение прочитано и совпадает
+с докстрингом; риск низкий, повторной сессии не прошу.
+
+## Проверка критериев приёмки
+
+| AC | Что | Доказательство в коде | Тест | Вердикт |
+|---|---|---|---|---|
+| AC1 К1 | Патч-набор: `git patch-id --stable`, без `Release:`, без коммитов только `docs/reviews/**`, порядок не важен, cherry-pick даёт тот же id | `hasReleaseTrailer`, `countsForPatchSet`, `commitPatchIds` (явные опции диффа — `PATCH_DIFF`), `issuePatchSets` | `#727 AC1` — временный git-репозиторий, реальный `git patch-id` | доказано исполнением |
+| AC2 К2 | `tag=nightly` требует `candidate`; имя документа по базе+SHA12; `prepare` отдаёт только `none`/`stale`; блок несёт `mode`/`patches` в хвосте | `shipReviewMode`, `nightlyDocPath`, `shipDocPath`, `anchorBlock`/`parseAnchorBlock` | `#727 AC2` (unit) + `#727 AC2/AC5` и `#727 AC2 _ship-review.yml` (реальный bash, реальный workflow-файл через `stepRun`) | доказано исполнением |
+| AC3 К3 | Четыре статуса покрытия; «последний документ главнее»; чужая база не в счёт; документ без `patches` — по номеру | `shipCoverage` | `#727 AC3` — все переходы статусов, включая «high отменяет ранний clean» и наоборот | доказано исполнением; мутация `samePatchSet→true` ловится тестом |
+| AC4 К4 | Гейт: всё `clean` без документа тега проходит; `none`/`stale`/`high` — отказ с командой (force только когда нужен) | `shipReviewProblems` | `#727 AC4` | доказано исполнением |
+| AC5 К5 | Бета читает дельту; бриф называет прочитанное ночью; `force=true` — всё; пустая дельта — без модели | `planShipReview`, `renderShipBrief` | `#727 AC5` (unit) + `#727 AC2/AC5` (сквозной сценарий на реальном workflow) | доказано исполнением |
+| AC6 К6 | Новая job после Validate, `if: always()`, не красит ночь, ждёт только появления прогона (18×10с) | `.github/workflows/_nightly.yml` (`ship_review` job) | `#727 AC6` (структура YAML) + два теста на реальном bash (`runStep`) — вывод SHA до ожидания, красный Validate красит именно job ожидания, dispatch/appearance таймауты — предупреждение, не красная ночь | доказано исполнением |
+| AC7 К7 | `parseDocName` узнаёт ночное имя; `archivePlan`: база-бета → своя линия, стабильная база → ближайшая новее, без линии новее — `kept` | `parseDocName` (`SHIP_DOC_NAME` с опциональной `-dev-sha12` группой), `archivePlan` (новая ветка `doc.nightly && STABLE_TAG_RE`), `renderIndex` (сортировка ночь/небета, подпись «ночь после») | `#727 AC7` — все четыре примера (а)-(г) из АС дословно | доказано исполнением; мутация `find→reverse().find` ловится тестом (в) |
+| AC8 К8 | Строка о High только ночью, с меткой документа, без повтора на тот же документ | `highCommentBody`, `highCommentTargets`, шаг «High ночью» в `_ship-review.yml` (`if: needs.prepare.outputs.mode == 'nightly'`) | `#727 AC8` (unit) + `#727 AC8 _ship-review.yml` (реальный bash: beta не пишет, nightly без High не пишет Medium/Low, nightly с High — только в задачу без метки) | доказано исполнением |
+| AC9 К9 | Канон — PROCESS.md §11.7/§10.4, REVIEWER.md, ключевое правило в `process-digests` | диффы PROCESS.md/REVIEWER.md прочитаны целиком; `test/process-digests.test.mjs` | `node --test test/process-digests.test.mjs` зелёный; `entry-cost --check` зелёный | доказано исполнением |
+| AC10 | Гейты | — | `gate:small` (заявлен автором, зелёный Validate подтверждает дешёвую часть), `mutation-gate --check`, `reviews-index --check` — перепрогнаны мной, зелёные; якорь найден | доказано исполнением (частично — моим прогоном) |
+
+## Что проверено и корректно
+
+- Патч-набор исключает ровно то, что нужно: `Release:`-коммит кандидата беты
+  (несущий `Issue:` всей линии) и чисто-доковые коммиты — иначе ночной набор
+  никогда бы не совпал с набором на кандидате беты (тест это явно
+  демонстрирует: один и тот же набор до и после `Release:`-коммита).
+- `shipReviewProblems` корректно различает «документа тега нет вовсе» (нет
+  force в команде) от «документ тега есть, но не всё покрывает» (force
+  обязателен) — оба случая сведены к конкретным regex-проверкам в тестах.
+- `archivePlan`: ветвление «база-бета» (точное совпадение, прежняя логика) и
+  «стабильная база» (поиск ближайшей линии строго новее, новая логика через
+  уже существующий `compareStable`) не пересекаются и не регрессируют старый
+  `deepEqual`-тест `#696` (проверено прогоном).
+- `_nightly.yml`: SHA прогона Validate выводится шагом `id: validate`
+  **до** шага ожидания (`gh run watch --exit-status`), поэтому красный
+  Validate не теряет кандидата для ночного ship-ревью — ключевое свойство
+  АС6, прочитано и подтверждено порядком шагов и `outputs:` на уровне job.
+  `ship_review` job зависит только от `dispatch` (не от его успеха — `if:
+  always()`) и несёт `continue-on-error: true`, так что сбой диспетчеризации
+  ship-ревью не красит ночь — ровно то поведение, которое требует АС6
+  («предупреждение, не красная ночь»).
+- `_ship-review.yml`: `mode`/`patches` публикуемого блока берутся из выходов
+  `prepare` (детерминированный скрипт), а не из JSON-результата модели —
+  единственный источник числа patch-id не раздваивается между моделью и
+  шагом публикации (§8, «одно число — один источник»); явно проверено тестом
+  с «подложным» `patches` в результате модели, который публикация
+  игнорирует.
+- Трейлеры коммита корректны: `Issue: #727`, `User-Visible: no`,
+  `Co-Authored-By`/`Claude-Session` — на месте.
+
+## Унаследовано из спец-ревью (без повторной проверки)
+
+Продуктовых вопросов владельцу по ТЗ не было (спец-ревью r2, зелёный,
+`docs/reviews/SPEC-REVIEW-727-r2.md`, материал `a49f7095`). Код-ревью принимает
+формулировки К1–К9 и границы скоупа/не-скоупа как данность и проверяет только
+соответствие кода этим формулировкам — что и сделано выше.
+
+## Чего не проверял
+
+- `actionlint` — бинарь недоступен в песочнице ревью; заявление автора не
+  перепроверено независимо (риск низкий, см. «Как проверялось»).
+- Полные `npx tsc --noEmit` / `npm test` / `npm run build` — не перегонял:
+  зелёный Validate на `93be52eb` (ссылка в промпте) закрывает дешёвые гейты
+  для этого SHA.
+- Живой ночной прогон (`_nightly.yml` → `ship-review.yml -f tag=nightly` на
+  реальном GitHub Actions, включая `git describe` с настоящими тегами в
+  checkout публикации) — автор сам называет это риском и предлагает
+  понаблюдать первую живую ночь; контрактные тесты на реальном bash/git это
+  закрывают настолько, насколько возможно до прод-прогона, но не заменяют его.
+- Browser-smoke, golden, performance, HA-pytest — объективно не применимы к
+  этому диффу (обоснование — «Как проверялось»).
+
+## Вердикт
+
+Все AC1–AC10 доказаны исполнением (не только чтением), два defensive-правила
+(stale-покрытие, выбор ближайшей архивной линии) проверены мутацией вручную и
+тест их ловит. High и Medium в скоупе нет. Одно Low-наблюдение (issue E из
+ТЗ не заведён) — вне скоупа #727, не блокирует.
+
+**Зелёный.**
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `issue/727-nightly-ship-review`, коммит `93be52ebe8c3` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Дерево материала: `990dd76b955a9fc9d630dd09709eaa5d4f052e05`
+  ```
+  git log --all --format='%H %T' | grep 990dd76b955a
+  ```
+- Тело issue: `8ce2942ca915bc938c8c5b4a720bb71d5a06d18c13db0692ee802f6a55662b7a`
+- Вердикт конвейера: `green` · High 0