mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -0,0 +1,223 @@
|
||||
# CODE-REVIEW-607-r2
|
||||
|
||||
## Скоуп
|
||||
|
||||
Issue [#607](https://github.com/Matysh/houseplan-card/issues/607) — в реальном
|
||||
Home Assistant (ветка `ha-dialog` внутри `hp-dialog`, не нативный `<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`) рендере `<ha-dialog>`;
|
||||
- `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(/<ha-dialog[\s\S]*?>/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` чист).
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/607-ha-dialog-close`, коммит `fc70a3fcf458` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `ebb43bfa79f4ba762532ef83c4e4ea2bd63c70a5`
|
||||
```
|
||||
git log --all --format='%H %T' | grep ebb43bfa79f4
|
||||
```
|
||||
- Тело issue: `c78f51ffc0d5268ea3a82d980f9061b9f2756d0a850072fec514342889af8c9e`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user