From 84cffb3f92bba469a987584b262ef6d9efee0d44 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Tue, 22 Sep 2026 23:42:32 +0000 Subject: [PATCH] docs: review document for #607 Issue: #607 User-Visible: no --- docs/reviews/CODE-REVIEW-607-r2.md | 223 +++++++++++++++++++++++++++++ 1 file changed, 223 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-607-r2.md diff --git a/docs/reviews/CODE-REVIEW-607-r2.md b/docs/reviews/CODE-REVIEW-607-r2.md new file mode 100644 index 00000000..3592f487 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-607-r2.md @@ -0,0 +1,223 @@ +# CODE-REVIEW-607-r2 + +## Скоуп + +Issue [#607](https://github.com/Matysh/houseplan-card/issues/607) — в реальном +Home Assistant (ветка `ha-dialog` внутри `hp-dialog`, не нативный `` +демо-стенда) крестик HA в шапке диалога настроек, за которым следует выбор +«Продолжить» в подтверждении «Отменить изменения?», оставлял диалог невидимым +с живым состоянием формы до перезагрузки страницы. + +Материал раунда — ровно `fc70a3fcf458f8703acdd78aeeda686fc86e73b7`, рабочая +копия на этом SHA не менялась (`git status` чист до и после моих проверок, +включая два временных прогона мутаций в отдельных worktree — они сами себя +подчищают). + +Это второй заход. Предыдущий вердикт — жёлтый на `9bbf6fb2487bfef24bfab6da256165d0965c278f` +(`docs/reviews/CODE-REVIEW-607-r1.md`, комментарий issue от 2026-09-22T23:21:29Z), +единственная находка — Medium в скоупе, без High. Бюджет: этот жёлтый уже +потратил 1 блокирующий цикл из 4 (см. заголовок задачи), r2 — второй заход +общего счёта. + +**Дельта r1→r2** (`git diff 9bbf6fb2..fc70a3fc`, `git log --oneline 9bbf6fb2..fc70a3fc`): +ровно два файла, только вставки, без единой правки продуктового кода: + +``` +docs/reviews/CODE-REVIEW-607-r1.md | 200 +++++++++++++++++++++++++++++++++++++ +scripts/mutation-registry.mjs | 32 ++++++ +2 files changed, 232 insertions(+) +``` + +`docs/reviews/CODE-REVIEW-607-r1.md` — публикация документа предыдущего +раунда (коммит `4382817e`, не авторская правка). `scripts/mutation-registry.mjs` +— две новые записи мутантов (коммит `fc70a3fc`, `test(dialog): register +rejected-close mutants (#607)`), которыми автор закрывает единственную находку +r1. `origin/dev` за это время не сдвинулся (`61d51dd9`, тот же, что был базой +r1) — ребейза, смены контракта или новой подсистемы нет. Дельта локальна: +разбор сужен до неё и до всего, что она задевает (AC4), остальные AC +наследуются из r1 по §2.9. + +## Что изменилось в этом раунде и как проверялось + +Единственная находка r1 (Medium, в скоупе): три новых защитных поведения AC4 +(`rejectedHaCloseReopensSameShell`, `repeatedRejectIsIdempotent`, +`disconnectedRejectDoesNotReopen` в `demo/smoke_dialog_modal_recovery.mjs`) +проверялись только браузерным смоком без записи в `scripts/mutation-registry.mjs` +— «дорогой гейт без мутанта» по §2.7. + +Автор добавил две записи: + +- `dialog-ha-rejected-close-reopen-disabled` — патчит `.open=${live(!this._closing)}` + → `.open=${true}` во втором (без `describedBy`) рендере ``; +- `dialog-ha-disconnected-reject-reopen-enabled` — убирает `if (this.isConnected)` + перед `requestUpdate()` в `rejectClose()`. + +Я не принял заявление автора на веру и прогнал обе записи лично, а не только +`--check`: + +``` +node scripts/mutation-gate.mjs --check --id=dialog-ha-rejected-close-reopen-disabled + → ok dialog-ha-rejected-close-reopen-disabled + +node scripts/mutation-gate.mjs --check --id=dialog-ha-disconnected-reject-reopen-enabled + → ok dialog-ha-disconnected-reject-reopen-enabled + +node scripts/mutation-gate.mjs --id=dialog-ha-rejected-close-reopen-disabled + → ok чистый прогон: node demo/smoke_dialog_modal_recovery.mjs + → ok dialog-ha-rejected-close-reopen-disabled: заявленный тест покраснел на мутанте + → поймано 1 из 1 + +node scripts/mutation-gate.mjs --id=dialog-ha-disconnected-reject-reopen-enabled + → ok чистый прогон: node demo/smoke_dialog_modal_recovery.mjs + → ok dialog-ha-disconnected-reject-reopen-enabled: заявленный тест покраснел на мутанте + → поймано 1 из 1 +``` + +`--check` подтверждает уникальность анкера (`hits === 1` в `mutation-gate.mjs` +— иначе тест был бы хрупким к любой соседней правке), полный прогон +подтверждает поведение: чистое дерево — смок зелёный, мутированное — тот же +смок красный, рабочая копия обеих команд вернулась к материалу без следа +(`git status` чист после каждой). Оба ID уникальны в реестре, дублей нет +(814 записей всего, проверено программно). + +Соответствие находке r1: r1's собственная таблица «чем краснеет» уже +использовала ровно эти два патча вручную (Мутация 1 = reopen-disabled, +Мутация 2 = disconnected-reopen-enabled) и сама зафиксировала, что Мутация 1 +одним прогоном красит сразу оба поведения — `rejectedHaCloseReopensSameShell` +и `repeatedRejectIsIdempotent` (оба читают `lateHa.open`, которое мутация не +даёт стать `true`). Значит формально «трёх находок → двух ID» — не пробел: +третье поведение (идемпотентность) физически не имеет отдельной ветки кода, +которую можно было бы сломать независимо от первой мутации — `rejectClose()` +не содержит собственного guard от повторного вызова, он безопасен по +построению (второй вызов — no-op над уже консистентным состоянием). Регистрация +двух записей полностью покрывает три названных в r1 поведения. + +### Отдельная адверсарная проверка (не входила в находку r1, проверена из +осторожности) + +`.open=${live(!this._closing)}` встречается в `src/hp-dialog.ts` дважды — во +втором рендере (патчится новым мутантом) и в первом, под `if (this.describedBy)` +(строка 565, используется только `hp-confirm.ts`). Я проверил, не остаётся ли +эта вторая ветка без защиты от регрессии: + +- `hp-confirm.ts` никогда не вызывает `rejectClose()` на себе — на `hp-close` + он всегда диспатчит `hp-confirm-decision` с `accepted: false` и не + восстанавливает modal; сценарий «отклонённое закрытие confirm-диалога» + структурно не существует сегодня, поэтому смок его не проверяет ни для одной + ветки; +- но регрессия в этой ветке всё равно не прошла бы незамеченной: `test/hp-dialog-contract.test.mjs:29-37` + (добавлен в r1, часть каждого `npm test`) — дешёвый, всегда исполняемый + unit-тест, который через `source.match(//g)` находит + ровно оба рендера и `assert.match` на `.open=\$\{live\(!this\._closing\)\}` + для каждого из них. Регистрация мутанта в §2.7 обязательна для защит, + живущих только за дорогим гейтом («там ревьюер не воспроизведёт + отрицательный прогон второй раз») — здесь же гейт дешёвый и его отрицательный + прогон воспроизводим тривиально чтением. Находки нет. + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где это видно | +|---|---|---| +| Medium (в скоупе): три guard-поведения AC4 в `demo/smoke_dialog_modal_recovery.mjs` проверяются только смоком, без записи в `scripts/mutation-registry.mjs` (§2.7) | Добавлены записи `dialog-ha-rejected-close-reopen-disabled` и `dialog-ha-disconnected-reject-reopen-enabled`, патчи — ровно те, что r1 использовал вручную для Мутаций 1–2 | `scripts/mutation-registry.mjs` (коммит `fc70a3fc`), лично перепрогнано мной: `node scripts/mutation-gate.mjs --id=<оба>` — «поймано 1 из 1» для каждого | + +## Унаследовано из r1 + +Ничего в этой дельте не задевает продуктовый код, поведение или доказательства +AC1–AC3 — они наследуются без повторной проверки из +`docs/reviews/CODE-REVIEW-607-r1.md`, SHA `9bbf6fb2487bfef24bfab6da256165d0965c278f`: + +- **AC1/AC2** (реальное восстановление modal и черновика в HA, Discard/чистое + закрытие) — доказаны `demo/verify_ha_dialog_discard_recovery.mjs` на пиновом + `home-assistant-frontend==20260729.7` для всех четырёх форм; r1 прогнал лично. +- **AC3** (нативный fallback без регрессии) — четыре `smoke_*_settings_form` + зелёные, прогнаны r1 лично. +- **Контракт поведения ТЗ, пункты 1–2, 7** — прочитан построчно в r1 + (`src/hp-dialog.ts:449–470, 562–590`), код с тех пор не менялся (дельта его + не касается). +- **Не-скоуп ТЗ, i18n, трейлеры коммита `9bbf6fb2`, changelog** — проверены r1, + диапазон не расширялся. +- **Дешёвые гейты (`typecheck`, `npm test`, `npm run build` со сверкой копий + бандла)** — r1 сослался на зелёный Validate на `9bbf6fb2` + (`.../actions/runs/35795417282`); в этом раунде действует отдельное + подтверждение — зелёный Validate уже на `fc70a3fc` + (`.../actions/runs/35797453549`, дано в постановке задачи) — не перегонялись + повторно мной. +- **`node scripts/check-docs.mjs`, `no-new-any`, `bundle:budget`, golden/скриншоты, + инварианты модели, `pytest tests_backend`** — не применимы или зелёные в r1; + дельта r2 не трогает `src/**`, геометрию, Python-бэкенд, рендер — переносится + без повторного прогона. + +## Что проверено и корректно (в этом раунде) + +- Обе новые записи реестра синтаксически валидны, `id` уникальны в реестре + из 814 мутантов, анкер каждого патча встречается в `src/hp-dialog.ts` ровно + один раз (`--check` зелёный для обеих). +- Обе мутации лично прогнаны целиком (`mutation-gate.mjs` без `--check`): + чистое дерево — `node demo/smoke_dialog_modal_recovery.mjs` зелёный; + мутированное — тот же смок красный; рабочая копия репозитория осталась + чистой после обоих прогонов (мутации выполняются во временных worktree). +- Патчи реестра дословно совпадают с двумя мутациями, которые r1 уже + зафиксировал в своей таблице «чем краснеет» — не новая, самодеятельная + проверка, а формализация уже воспроизведённого результата. +- `node scripts/smoke-select.mjs --base origin/dev --head HEAD` даёт то же + единственное прямое совпадение (`smoke_dialog_modal_recovery.mjs` ← + `rejectClose`), что и в r1 — дельта не расширила смок-поверхность. +- Трейлеры обоих коммитов дельты корректны: `4382817e` (`docs: review + document for #607`) и `fc70a3fc` (`test(dialog): register rejected-close + mutants (#607)`) оба несут `Issue: #607` и `User-Visible: no` — правильно, + видимое поведение не меняется, правка — только тестовая инфраструктура и + публикация документа. Changelog не требуется. +- Отдельно проверил (не по находке r1, а из осторожности к дублирующемуся + коду), что вторая, непатченная ветка `live()` (используется `hp-confirm.ts`, + ветка с `describedBy`) не осталась без защиты от регрессии — она покрыта + дешёвым `test/hp-dialog-contract.test.mjs`, добавленным ещё в r1; отдельного + дорогого гейта для неё не требуется по букве §2.7. + +## Чего не проверял + +- Полный `npm test` / `npx tsc --noEmit` / `npm run build` со сверкой трёх + копий бандла — не перегонял: зелёный Validate на точном SHA `fc70a3fc` + (`.../actions/runs/35797453549`) уже это покрывает, дельта раунда к тому же + не трогает `src/**`. +- `demo/verify_ha_dialog_discard_recovery.mjs` (пиновый HA), четыре + `smoke_*_settings_form`, `node --test test/hp-dialog-contract.test.mjs` — + не перепрогонял отдельно в этом раунде: продуктовый код не менялся с r1, + где все они уже прогнаны лично прежним ревьюером; повторный прогон дельта не + требует. +- `npm run golden:verify`, `python -m pytest tests_backend`, инварианты модели + — не применимо: дельта не меняет рендер, Python-бэкенд, геометрию или + ссылки на неё. +- Полный ночной прогон реестра мутаций (814 записей) — не применим к + ревью; ночной прогон — отдельное расписание (#513). +- Ручное тестирование в живом браузере Home Assistant — вне гейтов цикла + ревью; офлайн-пиновая фикстура — предусмотренная ТЗ замена, прогнана в r1. + +## Вердикт + +Единственная находка r1 (Medium, в скоупе) закрыта и лично перепроверена +поведенчески (не только чтением diff'а или словами автора). Новых находок — +ни High, ни Medium — в дельте r1→r2 нет; отдельная адверсарная проверка +дублирующейся `live()`-ветки тоже не дала находки. AC1–AC4 полностью доказаны +(AC1–AC3 унаследованы из r1 без изменений в их доказательной базе, AC4 теперь +дополнительно защищён мутационным гейтом). Зелёный, без потраченного цикла. + +--- + +**Материал раунда:** SHA `fc70a3fcf458f8703acdd78aeeda686fc86e73b7`, дельта +`9bbf6fb2..fc70a3fc` (`origin/dev` не сдвигался, база `61d51dd9`). Рабочая +копия временно мутировалась дважды во внешних worktree для адверсарной +проверки реестра и была не тронута в основном дереве (`git status` чист). + +--- + + + +## Материал раунда + +- Ветка: `issue/607-ha-dialog-close`, коммит `fc70a3fcf458` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `ebb43bfa79f4ba762532ef83c4e4ea2bd63c70a5` + ``` + git log --all --format='%H %T' | grep ebb43bfa79f4 + ``` +- Тело issue: `c78f51ffc0d5268ea3a82d980f9061b9f2756d0a850072fec514342889af8c9e` +- Вердикт конвейера: `green` · High 0