From cf8b8f03070e99754945c230a3ba08f272ee9639 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Thu, 1 Oct 2026 03:25:20 +0000 Subject: [PATCH] docs: review document for #727 Issue: #727 User-Visible: no --- docs/reviews/CODE-REVIEW-727-r2.md | 262 +++++++++++++++++++++++++++++ 1 file changed, 262 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-727-r2.md diff --git a/docs/reviews/CODE-REVIEW-727-r2.md b/docs/reviews/CODE-REVIEW-727-r2.md new file mode 100644 index 00000000..73a84e98 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-727-r2.md @@ -0,0 +1,262 @@ +# CODE-REVIEW-727-r2 + +Issue: #727 · этап code · трек `ask` · заход r2 · блокирующих циклов 0/4 +Материал: `git log --oneline origin/dev..HEAD` / `git diff origin/dev...HEAD`, +ровно `00d1a7b6c3e09072c3d3b3381ada3ab37e16aab3` (рабочая копия уже на нём) + +## Скоуп + +Тот же контракт, что в r1: ночной режим пакетного ship-ревью на `dev` и +переиспользование его результата гейтом беты по патч-набору (К1–К9, AC1–AC10 +из ТЗ в теле #727, спец-ревью зелёное с r2, `docs/reviews/SPEC-REVIEW-727-r2.md`). +Классы изменений — B (`scripts/**`, `.github/workflows/**`, `test/**`) и C +(`PROCESS.md`, `docs/process/REVIEWER.md`); файлов класса A нет. `User-Visible: no` +подтверждено — `git diff origin/dev...HEAD --stat -- docs/CHANGELOG*.md` пуст. + +**Почему есть r2, хотя r1 был зелёным.** Пока шёл код-ревью r1 (материал +`93be52eb` поверх `dev@52dc08a0`), `dev` продвинулся на один коммит (слился +`3bb1b534` — «the moon with any background», #718, несвязан с #727). Ветка +`issue/727-nightly-ship-review` ребейзнулась на новую вершину `dev@8fac66e1`, +патч-набор диффа изменил содержимое (другой `patch-id`), и комментарий автора +в issue (2026-10-01T03:12:22Z) прямо зафиксировал: «Дифф изменился при ребейзе +— вердикт к нему не применим (§2.10, #492) … новый заход ревью читает +актуальный код». Это ровно случай из промпта ревьюера «ребейз на ушедший +вперёд dev» — делаю **полный** разбор, а не только дельту: материал r1 +(`93be52eb`) физически отсутствует в репозитории (`git cat-file -t` → объект не +найден), сравнить дельту напрямую нечем. + +Файлы диффа не изменились по составу и почти не изменились по объёму +относительно r1: `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` (+580), +`test/nightly-workflow.test.mjs` (+137), `test/process-digests.test.mjs` (+2); +плюс три новых коммита-артефакта публикации r1 (`docs/reviews/CODE-REVIEW-727-r1.md`, +два обновления `docs/reviews/INDEX.md`) — чисто документные, не продуктовый код. +`.github/workflows/ship-review.yml` (тонкий файл) не тронут — подтверждено +(`git log origin/dev..HEAD -- .github/workflows/ship-review.yml` пуст), как и +заявляет ТЗ («входы и права тонких файлов уже в main»). + +## Как проверялось + +Поскольку материал r1 недоступен для прямого `git diff ..HEAD`, разбор +сделан заново и независимо от выводов r1 — не как повторная вера документу, +а как самостоятельное построчное чтение: + +1. **Эквивалентность содержимого r1→r2.** `git diff origin/dev...HEAD --stat` + даёт тот же список файлов и почти те же цифры строк, что называет r1 + (`scripts/ship-review.mjs` 428, `test/ship-review.test.mjs` 580, + `test/nightly-workflow.test.mjs` 137) — рабочий код не переписывался + заново, только сместился контекст диффа при ребейзе. Родитель коммита + `e852ee2e` — ровно текущий `origin/dev` (`8fac66e1`, `git log -1 --format='%H %P'`), + то есть ребейз уже выполнен и ветка не отстаёт. +2. **Построчное независимое чтение всего диффа** (не выжимка из r1, а + собственное прочтение): `git diff origin/dev...HEAD -- scripts/ship-review.mjs` + целиком (все новые функции — `shipReviewMode`, `nightlyDocPath`, + `shipDocPath`, `hasReleaseTrailer`, `countsForPatchSet`, `commitPatchIds`, + `issuePatchSets`, `formatPatches`/`parsePatches`, `samePatchSet`, + `shipCoverage`, `planShipReview`, `reviewSubject`, `renderShipBrief`, + `anchorBlock`/`parseAnchorBlock`, `shipReviewProblems`, `highCommentBody`, + `highCommentTargets`, `readRangeDocs`); `scripts/reviews-archive.mjs` (ветка + `doc.nightly && STABLE_TAG_RE.test(doc.tag)` в `archivePlan`, с проверкой, + что `ordered` в этом файле отсортирован **по возрастанию**, — то есть + `.find()` действительно берёт ближайшую, не дальнюю линию, АС7(в)); + `scripts/reviews-index.mjs` (`SHIP_DOC_NAME` с опциональной + `-dev-[0-9a-f]{12}` группой, `parseDocName`, сортировка `renderIndex`); + `.github/workflows/_nightly.yml` и `_ship-review.yml` целиком (порядок + шагов, `outputs:`, `if: always()`, `continue-on-error: true`, источник + `mode`/`patches` публикации); `PROCESS.md` §10.4/§11.7 и + `docs/process/REVIEWER.md` диффы; весь новый текст `test/ship-review.test.mjs` + и `test/nightly-workflow.test.mjs`. +3. Каждый AC1–AC9 тела issue сверен с этим независимым чтением (оракул AC → + функция/шаг → тест). AC10 — гейты, ниже. +4. Тело issue #727 и комментарии прочитаны через `gh issue view 727 --json + comments` (MCP-инструменты `mcp__github__get_issue*` отказали разрешением в + этой песочнице — `gh` CLI сработал напрямую, уже аутентифицирован как + `github-actions[bot]`). Контракт К1–К9 не менялся с r1/спец-ревью r2; + последний комментарий автора — объяснение самого ребейза (выше). + +**Исполнено, не только прочитано:** +- `node --test test/ship-review.test.mjs test/nightly-workflow.test.mjs + test/process-digests.test.mjs` — 29/29 зелёных; отдельно + `test/default-branch-workflows.test.mjs` — 45/45 зелёных. +- `node scripts/mutation-gate.mjs --check` — exit 0, якорь + `ship-review-ignores-merge-marker` и мутанты `ship-review-accepts-partial-coverage`, + `ship-review-accepts-high` зелёные. Уточнение к r1: эти три мутанта заведены + ещё в #696 (`git log -L` по `scripts/mutation-registry.mjs` — коммит + `e1ae8f4a`), не новые в #727; `scripts/mutation-registry.mjs` в дельте + `origin/dev...HEAD` не меняется. Проверено, что их `find`-строки + (`if (missing.length) {`, `else if (block.high > 0) problems.push(`, + `return names.includes('track:ship') || comments.some(...)`) совпадают + дословно с переписанным кодом `shipReviewProblems` — переписывание функции + в #727 не осиротило мутанты молча. +- `node scripts/entry-cost.mjs --check` — зелёный (author/reviewer/canon в + бюджете слов). +- `node scripts/reviews-index.mjs --check` — «INDEX.md свеж»; независимо + пересчитан файл: `ls docs/reviews/*.md | grep -v INDEX | wc -l` → 224, + совпадает со строкой заголовка индекса. +- `node scripts/smoke-select.mjs --base origin/dev --head HEAD` — «исполняемого + frontend-диффа нет, browser-smoke не выбираются»: `src/**/*.ts` не тронут. +- **Мутационная проверка вручную** (дисциплина «тест умеет падать», + независимо от r1 — тот же метод, своя команда): временно `cp` бэкап, правка, + прогон, восстановление, `git diff --stat` после — пусто. + - `samePatchSet` → `return true` всегда: упало 5 из 19 тестов + `ship-review.test.mjs` (AC3, AC4, AC5, АС7-тест, сквозной + `_ship-review.yml`-тест) — покрытие никогда не становится `stale`. + - `archivePlan`: `ordered.find(...)` → `[...ordered].reverse().find(...)`: + упал ровно тест АС7 (сценарий `в`, `[v1.78.0, v1.78.1, v1.79.0]` обязан + дать `v1.78.1`, мутант даёт `v1.79.0`). + Оба файла восстановлены из бэкапа, `git diff --stat` — пусто, полный прогон + после восстановления снова 29/29. +- Трейлеры всех четырёх коммитов диапазона (`git log -1 --format=%B ` на + `e852ee2e`, `60581038`, `ac4dd1c5`, `00d1a7b6`): `Issue: #727`, + `User-Visible: no` на каждом; `Co-Authored-By`/`Claude-Session` — на + коммите продуктового кода. +- Проверка закрытия Low-наблюдения r1 (issue E «Красная ночь» из ТЗ) — + `gh search issues`/`gh issue list --label S1-new`: находка снята, см. + «Закрытие раунда r1». + +**Не прогонял и почему:** `actionlint` — бинарь недоступен в этой песочнице +(`which actionlint` → код 1), как и в r1; риск тот же и так же низкий — YAML +`run:`-блоки проверяются `bash -n` в исполняемых тестах, структурные +инварианты — текстовыми assert’ами на реальном файле (см. `stepRun`/`job` в +`test/nightly-workflow.test.mjs`, выдержки выше). `npx tsc --noEmit` / `npm +test` (полный) / `npm run build` — не перегонял: Validate зелёный на этом +точном SHA `00d1a7b6` (ссылка в промпте ревью, прогон 36809481316), дешёвые +гейты этим закрыты. Python/`pytest tests_backend` — нет правки +`custom_components/**/*.py`. `npm run golden:verify`/`invariants` — нет +`ci:golden`, нет правки геометрии/рендера. Performance — не назван в AC. +Живой ночной прогон на реальном GitHub Actions — не воспроизводим в песочнице +ревью, остаётся риском первой ночи, как и в r1. + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где это видно | +|---|---|---| +| Low (наблюдение): ТЗ требует завести issue E («Красная ночь: комментарий в задачи, слитые после последней зелёной», `S1-new`, ссылка на #727) — на момент r1 такой issue не найден | Issue заведён | `gh issue view 736` — заголовок дословно совпадает с текстом ТЗ, тело «Выделено из #727 (issue E его ТЗ)», создан 2026-10-01T03:01:10Z, метки `infra`, `S1-new`, `process` | +| Low (наблюдение): `readRangeDocs` не имеет прямого unit-теста на дедупликацию «кандидат vs `devRef`» | Не предмет этого раунда — не находка, требующая действия (r1 явно отметил «риск низкий, повторной сессии не прошу»); код `readRangeDocs` не менялся между r1 и r2 (дифф идентичен), логика перечитана заново в этом раунде и подтверждена (см. «Как проверялось» п.2) | `scripts/ship-review.mjs`, функция `readRangeDocs` | + +Других находок r1 не было (High 0, Medium 0 в r1). + +## Унаследовано из r1 (без повторной проверки, с явной переподтверждённой частью) + +Этот раунд — не формальная дельта (материал r1 недоступен для diff), поэтому +весь AC-разбор выполнен заново и независимо (раздел «Как проверялось»), а не +унаследован слепо. Без повторного исполнения приняты только факты среды, +которые не зависят от содержимого диффа #727 и совпадают с r1 +(`docs/reviews/CODE-REVIEW-727-r1.md`, материал `93be52eb`, ныне недостижим +как объект — ребейз, но содержательно эквивалентен текущему `e852ee2e`, +доказательство — «Как проверялось» п.1): + +- `track:ship` и маркер `hp:ship-merge` как признак ship-задачи (`isShipIssue`, + не изменился с #696/#716, вне диффа #727); +- поведение `git patch-id --stable` и `git diff-tree` как документированное + поведение инструментов, не специфика кода задачи; +- трек `ask` обоснован (сложность/риск >3, публичный контракт §11.7) — + подтверждено спец-ревью r1/r2, код не меняет это решение. + +Точечно переподтверждено в этом раунде (не просто унаследовано): все функции, +которые r1 называет по имени, перечитаны в текущем диффе заново (список в +«Как проверялось» п.2) — совпадение не предполагается, а показано. + +## Что проверено и корректно + +- Патч-набор по-прежнему исключает ровно `Release:`-коммит кандидата беты и + коммиты только `docs/reviews/**` — иначе ночной набор никогда не совпал бы с + набором на кандидате беты; логика (`hasReleaseTrailer`, `countsForPatchSet`) + не изменилась между r1 и r2 (идентичный текст в обоих диффах). +- `archivePlan`: новая ветка `doc.nightly && STABLE_TAG_RE.test(doc.tag)` + корректно использует уже отсортированный по возрастанию `ordered` + (`[...lines].sort((a, b) => compareStable(a.tag, b.tag))`, строка 79) — + `.find(line => compareStable(line.tag, doc.tag) > 0)` берёт ближайшую, а не + последнюю линию новее базы; без линии новее — `kept` с объяснением. Это + прочитано мной заново (не со слов r1) и подтверждено мутацией «переворот на + `.reverse().find`» (упал АС7-тест). +- `_nightly.yml`: `head_sha` — выход job `dispatch`, записанный шагом + `id: validate` **до** шага ожидания (`gh run watch --exit-status`) — + красный Validate не теряет SHA кандидата. `ship_review` зависит только от + `dispatch` (`if: always()`, `continue-on-error: true`) — отказ + диспетчеризации ship-ревью не красит ночь. Оба факта перечитаны в текущем + YAML, не взяты на веру. +- `_ship-review.yml`: `mode`/`patches` публикуемого блока — из выходов + `prepare` (`needs.prepare.outputs.mode`/`.patches`), не из `result.json` + модели — единственный источник числа patch-id не раздваивается (§8). + Строка о High ночью (`needs.prepare.outputs.mode == 'nightly'`) пишется + через `comment-high`, который сам же отказывается молча при `mode != nightly` + или `high == 0` — двойная защита от шума в бета-режиме. +- `scripts/mutation-registry.mjs` не входит в дельту #727: три мутанта, + которые использует этот код, принадлежат #696 и остаются валидными после + переписывания `shipReviewProblems` (их `find`-строки совпадают дословно). +- Индекс (`docs/reviews/INDEX.md`) после публикации r1 корректен: счётчик + документов (224) совпадает с фактическим числом файлов, новая строка + `#727 code · r1 · 🟢` на месте, порядок (ночь/бета/релиз, затем issue по + убыванию) не нарушен. +- Трейлеры всех коммитов диапазона корректны; изменений в обоих CHANGELOG нет, + что соответствует `User-Visible: no`. +- Единственное Low-наблюдение r1 закрыто заведением issue #736. + +## Проверка критериев приёмки + +Разбор не изменился по существу относительно r1 (код идентичен содержательно +— см. «Как проверялось» п.1), но выполнен заново по текущему коду, а не +скопирован: + +| AC | Что | Доказательство в коде | Тест | Вердикт | +|---|---|---|---|---| +| AC1 К1 | Патч-набор: `git patch-id --stable`, без `Release:`, без коммитов только `docs/reviews/**`, порядок не важен, cherry-pick даёт тот же id | `hasReleaseTrailer`, `countsForPatchSet`, `commitPatchIds` (явные опции `PATCH_DIFF`), `issuePatchSets` | `#727 AC1` — реальный git в temp-репозитории | доказано исполнением | +| AC2 К2 | `tag=nightly` требует `candidate`; имя документа по базе+SHA12; `prepare` отдаёт только `none`/`stale`; блок несёт `mode`/`patches` в хвосте | `shipReviewMode`, `nightlyDocPath`, `shipDocPath`, `anchorBlock`/`parseAnchorBlock` | `#727 AC2` (unit) + `#727 AC2/AC5 _ship-review.yml` (реальный bash) | доказано исполнением | +| AC3 К3 | Четыре статуса покрытия; «последний документ главнее»; чужая база не в счёт; документ без `patches` — по номеру | `shipCoverage` | `#727 AC3` | доказано исполнением; мутация `samePatchSet→true` ловится (перепроверено лично) | +| AC4 К4 | Гейт: всё `clean` без документа тега проходит; `none`/`stale`/`high` — отказ с командой | `shipReviewProblems` | `#727 AC4` | доказано исполнением | +| AC5 К5 | Бета читает дельту; бриф называет прочитанное ночью; `force=true` — всё; пустая дельта — без модели | `planShipReview`, `renderShipBrief` | `#727 AC5` + `#727 AC2/AC5` (сквозной, реальный bash) | доказано исполнением | +| AC6 К6 | Новая job после Validate, `if: always()`, не красит ночь, ждёт только появления прогона (18×10с) | `.github/workflows/_nightly.yml` (`ship_review`) | `#727 AC6` + два теста на реальном bash (`runStep`) | доказано исполнением; порядок шагов/`outputs:` перечитан лично | +| AC7 К7 | `parseDocName` узнаёт ночное имя; `archivePlan`: база-бета → своя линия, стабильная база → ближайшая новее, без линии новее — `kept` | `parseDocName` (`SHIP_DOC_NAME` с опциональной `-dev-sha12`), `archivePlan`, `renderIndex` | `#727 AC7` — все четыре примера (а)-(г) | доказано исполнением; мутация `find→reverse().find` перепроверена лично, ловится | +| AC8 К8 | Строка о High только ночью, с меткой документа, без повтора | `highCommentBody`, `highCommentTargets`, шаг «High ночью» (`if: needs.prepare.outputs.mode == 'nightly'`) | `#727 AC8` + `#727 AC8 _ship-review.yml` (реальный bash) | доказано исполнением | +| AC9 К9 | Канон — PROCESS.md §11.7/§10.4, REVIEWER.md, ключевое правило в `process-digests` | диффы прочитаны целиком лично | `node --test test/process-digests.test.mjs` зелёный; `entry-cost --check` зелёный | доказано исполнением | +| AC10 | Гейты | — | `mutation-gate --check`, `reviews-index --check`, `entry-cost --check` — перепрогнаны мной в этом раунде, зелёные | доказано исполнением (моим прогоном в r2) | + +## Чего не проверял + +- `actionlint` — бинарь недоступен в песочнице (как в r1); заявление автора + не перепроверено независимо, риск низкий (см. «Как проверялось»). +- Полные `npx tsc --noEmit` / `npm test` (весь набор) / `npm run build` — не + перегонял: зелёный Validate на точном SHA `00d1a7b6` (ссылка в промпте) + закрывает дешёвые гейты для этого материала. +- Живой ночной прогон (`_nightly.yml` → `ship-review.yml -f tag=nightly` на + настоящем GitHub Actions, включая `git describe` с реальными тегами при + публикации) — не воспроизводим до продакшена; контрактные тесты на реальном + bash/git закрывают то, что можно закрыть без него. Первая реальная ночь + остаётся риском, как и в r1. +- Browser-smoke, golden, performance, HA-pytest — объективно не применимы + (`src/**/*.ts`, геометрия/рендер, `custom_components/**/*.py` не тронуты). +- Прямой доступ к issue через MCP (`mcp__github__get_issue*`) — инструмент + отказал разрешением в этой песочнице; использован `gh` CLI напрямую + (уже аутентифицирован), результат тот же набор данных. + +## Вердикт + +Все AC1–AC9 проверены заново и независимо (не унаследованы слепо из r1, так +как материал r1 стал недостижим после ребейза на ушедший вперёд `dev`) — +построчным чтением всего диффа плюс исполнением тестов и двух ручных мутаций +на самых защитных участках (`samePatchSet`, выбор архивной линии в +`archivePlan`), обе ловятся тестами. AC10 — гейты пересняты в этом раунде. +Содержимое диффа эквивалентно тому, что видел r1 (тот же состав файлов, те же +объёмы правок, родитель коммита — ровно текущий `origin/dev`): разница — это +ребейз на влившийся параллельно и несвязанный #718, что явно подтвердил и +автор в issue. High и Medium нет. Единственное Low-наблюдение r1 закрыто +(issue #736 заведён). Новых находок в этом раунде нет. + +**Зелёный.** + +--- + + + +## Материал раунда + +- Ветка: `issue/727-nightly-ship-review`, коммит `00d1a7b6c3e0` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `c1b179ff64fac8b2b842b513e73a7524cae3b957` + ``` + git log --all --format='%H %T' | grep c1b179ff64fa + ``` +- Тело issue: `8ce2942ca915bc938c8c5b4a720bb71d5a06d18c13db0692ee802f6a55662b7a` +- Вердикт конвейера: `green` · High 0 · маршрут `fix`