docs: review document for #337

Issue: #337
User-Visible: no
This commit is contained in:
claude[bot]
2026-08-28 05:40:28 +00:00
parent 165dabc5d7
commit 3cb9e2ba62
+248
View File
@@ -0,0 +1,248 @@
# CODE-REVIEW-337-r2
- Issue: https://github.com/Matysh/houseplan-card/issues/337
- Материал: `git diff 6d338b78..HEAD` (r1 SHA `6d338b78a0794491639edee9171662cdd260c42b` →
HEAD `2beafc06398c16d1cad80fddbe77e6a40f111b9a`), ветка `issue/337-lazy-editor-chunk`.
4 коммита, 32 файла (без учёта сгенерированных `dist/*`/`custom_components/houseplan/frontend/*`,
которые зеркалят исходники): `src/houseplan-card.ts` (+79/-13), `src/houseplan-editor-runtime.ts`
(+55/-55, преимущественно механическая правка), `scripts/mutation-gate.mjs`, два unit-теста,
11 browser-smoke фикстур, документ CODE-REVIEW-337-r1, обновлённый `docs/images/screenshots.json`.
- Этап: код-ревью, заход r2, блокирующих циклов израсходовано 1/4 до этого разбора.
- **Вердикт: красный.**
## Скоуп разбора
Предыдущий вердикт (CODE-REVIEW-337-r1, SHA `6d338b78`) — красный, 5 High + 3 Medium.
Дельта этого раунда — точечные правки по каждой находке, а не переработка задачи:
два изменённых src-файла, обновлённые smoke-фикстуры под новый async-контракт,
одна правка mutation-gate (стал ссылаться на актуальный файл стилей после переноса
CSS), плюс сам документ r1 и пересборка bandle-артефактов. Это не ребейз на ушедший
`dev` (branch и так уже стоит на `508945c0` = `origin/dev`), не смена контракта
поведения и не новая подсистема — разбор веду по дельте, а не с нуля, но
дополнительно перепрогоняю **весь** browser-smoke census и `golden:verify`, а не
только смоки, названные автором: H4-фикс (`_deviceInboxTabKey`/`@click=${this._x}`
→ `@click=${() => this._x()}`) механически переписал практически КАЖДЫЙ
обработчик события в `houseplan-editor-runtime.ts` (не только вкладки инвентаря
устройств) — это затрагивает общий рендер-путь всех редакторских диалогов, а не
только те четыре сценария, что назвал автор.
## Как проверялось
Дешёвые гейты (перепрогнаны самостоятельно, а не приняты со слов автора):
| Гейт | Команда | Результат |
|---|---|---|
| typecheck | `npx tsc --noEmit` | pass (5.6s) |
| unit | `npm test` | 1439 total, 1438 pass, 1 skip, 0 fail — совпадает с заявлением автора |
| build+sync+budget | `npm run build && npm run bundle:sync && npm run bundle:budget` | initial View 255 778 B ≤ 256 000 B (запас 222 B — **уже, чем 615 B на r1**); lazy editor 131 785 B; `bundle:sync` не оставил расхождений в дереве (только биты прав доступа 755→644 на двух артефактах из-за локального `tsc`, отменено `git checkout --`, содержимого не касалось) |
| docs fingerprint | `node scripts/check-docs.mjs` (diff трогает `src/**`) | pass (7 файлов, 10 внешних ссылок) |
| mutation, changed-range | `node scripts/mutation-gate.mjs --check --changed=6d338b78..HEAD` | 58/58 применимых guard'ов поймали подмену |
Тяжёлые гейты — по дельте и по решению ревьюера, не выборочно на веру:
- **Полный browser-smoke census** (195 файлов, `for f in demo/smoke_*.mjs; do
node "$f"; done` после `bundle:sync`) — **195/195 pass**, включая
`smoke_warm_dialogs` (единственный прогон в составе census). Это ожидаемо и
НЕ противоречит находке H6 ниже: точечный прогон того же файла 7 раз подряд
дал 5 отказов из 7 (≈71% красных) — при однократном прогоне в общем census
благополучный исход (≈29% по моей выборке) не редкость. Единичный зелёный
census — статистический шум для гонки такой частоты, а не доказательство
исправности; полагаться на однократный прогон здесь означало бы повторить
саму ошибку хендоффа автора.
- **`npm run golden:verify`** — 127 passed / **4 different** — те же четыре
кадра, что в r1: `geometry-devices-editor-dark`, `device-dialog-mobile-ru`,
`toggle-entity-dialog-mobile-ru`, `device-ripple-color-popover-mobile-ru`.
- **Независимая проверка авторского заявления по H5** (не принято на веру: автор
утверждает «pre-existing stale baseline, не регресс #337» — это фактическое
утверждение о состоянии `dev`, которое легко проверить и легко сфальсифицировать).
Поднял `git worktree add /tmp/dev-check origin/dev --detach` (чистый `508945c0`,
ancestor этой ветки), `npm ci && npm run build && npm run bundle:sync`, затем
`node demo/golden/run.mjs --mode=capture --scenario=<id>` (verify требует полную
матрицу, capture позволяет точечный diff) для всех четырёх кадров:
**все четыре «different» воспроизводятся на чистом `dev` без единого изменения
из #337.** Открыл сами diff-PNG: `geometry-devices-editor-dark` — актуальный
рендер показывает два раздельных, не наложенных друг на друга элемента
тулбара («Devices», «Icon rules»); диффовое наложение возникает потому что
**эталон** старше и содержит три кнопки («Add», «Hidden and disabled»,
«Icon rules») — устаревший baseline `dev`, а не порча layout веткой. Три
mobile-ru кадра показывают одинаковую узкую полосу по правому/нижнему краю
диалога и на чистом `dev` — тоже baseline drift, не регресс. **H5 закрыт**:
находка воспроизводится на `dev` и не относится к #337; это отдельный,
предсуществующий дефект золотых эталонов — завёл отдельный issue
([#346](https://github.com/Matysh/houseplan-card/issues/346)).
- **Целевые smoke, привязанные к находкам r1**, каждый прогнан минимум по
разу, некоторые — многократно при подозрении на нестабильность (ниже):
`smoke_preloader_lifecycle`, `smoke_warm_owners`, `smoke_kiosk`,
`smoke_room_resize`, `smoke_optimize_geometry_preflight`,
`smoke_device_inbox`, `smoke_fixed_floor`, `smoke_orphan_space_references`,
`smoke_general_settings`, `smoke_ha_controls`, `smoke_nav_persist`,
`smoke_partition_openings`, `smoke_grid_scale_invariance`,
`smoke_edit_walk`, `smoke_help_affordance`, `smoke_opening_entity_search`.
- **`smoke_warm_dialogs` — прогнан 7 раз подряд после `bundle:sync`, а не один
раз.** Причина повторов: первый запуск дал `bViewBitExact` FAIL на кадре,
близком, но не идентичном сохранённой камере — при плавающей точке одного
разового отказа недостаточно, чтобы отличить гонку от единичной аномалии
окружения. Результат: **5 из 7 запусков красные**, см. находку H6 ниже. Это
прямо противоречит заявлению автора «warm_dialogs green» в хендоффе.
- **model-invariants** — не запускал: дельта не меняет ни один формат хранения
геометрии (`layout`, `marker.space`, `open_spans`, ключи толщины); H3-фикс
меняет только то, ЧЕРЕЗ КАКОЙ объект (`this` vs `this.host`) вызывается уже
существующая preflight-проверка, не сами данные. Не тот инструмент для этой
дельты — то же обоснование, что в r1.
- **Backend/pytest** — не тронуты этой дельтой (`custom_components/**/*.py` не
входит в diff `6d338b78..HEAD`), не перепрогонял.
## Закрытие раунда r1
| Находка r1 | Чем закрыта | Где видно |
|---|---|---|
| **H1** (warm-remount коммитит mode в обход `_ensureEditorRuntime()`) | `_warmAdoptViewport` больше не коммитит редакторский `mode` напрямую: адаптирует `'view'` синхронно, а восстановление `mode` уходит через `_requestMode(restoredMode, false, true)`, который проходит `_ensureEditorRuntime()`; новый параметр `adopt` в `_requestMode` атомарно вызывает `_adoptMode` + возобновляет отложенный `_warmReviveDialog` | `src/houseplan-card.ts:890-901` (сигнатура/adopt-ветка `_requestMode`), `:3190-3222` (`_warmAdoptViewport`), `:3358-3369` (`_warmReviveDialog` откладывает потребление диалога, если ждёт runtime) | **Частично** — крах устранён (`smoke_preloader_lifecycle`, `smoke_warm_owners` зелёные детерминированно), но см. новую находку **H6**: та же зона правки сломала бит-точность камеры |
| **H2** (kiosk scale зависит от lazy runtime) | `_saveKioskScale`/`_renderKioskDialog` возвращены в eager-код хоста, больше не форвардятся в `_editorRuntimeOrThrow()`; вызов диалога больше не защищён условием `this._editorRuntime ?` | `src/houseplan-card.ts:10629-10662`, `:11350` (`${this._kioskDialog ? this._renderKioskDialog() : nothing}`) | `smoke_kiosk` зелёный (сам прогнал) |
| **H3** (fail-closed resize/optimize preflight не перехватывается подменой) | Внутренние вызовы `_checkSpacePhysicalGeometry`/`_checkOptimizeGeometry` внутри runtime переведены на `this.host._checkSpacePhysicalGeometry(...)`/`this.host._checkOptimizeGeometry(...)` — то есть идут через тот же публичный метод хоста, который переопределяет смок; сами реализации переименованы в `..._Impl` и не вызываются напрямую нигде, кроме хоста | `src/houseplan-editor-runtime.ts:1117-1120` (интерфейс `HouseplanEditorHostPort` пополнен), `:1761, 2085, 2156, 2265, 2288, 3609, 3626, 8591, 8599, 8688, 8721` (все внутренние вызовы через `this.host.`), `src/houseplan-card.ts:9973/9984` (`_checkOptimizeGeometry`/`_checkSpacePhysicalGeometry` хоста форвардят в `..._Impl`) | `smoke_room_resize` и `smoke_optimize_geometry_preflight` зелёные (сам прогнал) |
| **H4** (`_deviceInboxTabKey` падает — `this` не тот) | Метод стал полем-стрелкой (`public _deviceInboxTabKey = (event) => {...}`), а КАЖДЫЙ голый `@event=${this._method}` в файле переписан в `@event=${() => this._method(...)}` — то есть устранён весь класс бага, а не только один обработчик; закреплено регрессионным unit-тестом, который grep'ает файл на голые `@x=${this._y}` и требует пустой список | `src/houseplan-editor-runtime.ts:7401-7410` (метод), десятки мест по файлу (см. `git diff 6d338b78..HEAD -- src/houseplan-editor-runtime.ts`), `test/editor-runtime-loader.test.mjs:124-133` («lazy runtime event handlers keep the runtime receiver») | `smoke_device_inbox` зелёный (сам прогнал); тест зелёный в `npm test` |
| **H5** (4 golden-дельты, включая порчу тулбара Device editor) | Не изменено ни строки, относящейся к golden — **не регресс #337**: independently подтверждено, что все 4 кадра дают «different» и на чистом `origin/dev` (`508945c0`) без единого коммита этой ветки, см. раздел «Как проверялось» | `git diff origin/dev...HEAD -- demo/golden/baselines/` пуст (нулевой diff эталонов); скриншот-diff `geometry-devices-editor-dark` на чистом `dev` показывает то же наложение старого 3-кнопочного baseline поверх нового 2-кнопочного актуального рендера | **Закрыт как не относящийся к #337**; отдельный issue [#346](https://github.com/Matysh/houseplan-card/issues/346), не блокирует эту задачу |
| **M1** (`fixed_floor`/`orphan_space_references` создают `houseplan-card-editor` напрямую) | Обе фикстуры переведены на `await customElements.get('houseplan-card').getConfigElement()` | `demo/smoke_fixed_floor.mjs:75`, `demo/smoke_orphan_space_references.mjs:191` | Оба смока зелёные (сам прогнал) |
| **M2** (`general_settings` ищет версию только в entry-файле) | Смок теперь читает `houseplan-assets.json`, конкатенирует все `.js`-файлы манифеста и ищет баннер в объединённой строке | `demo/smoke_general_settings.mjs:61-67` | Зелёный (сам прогнал) |
| **M3** (9 нерасследованных красных смоков) | `ha_controls`, `nav_persist`, `partition_openings`, `grid_scale_invariance` явно адаптированы под async editor runtime (`_editorRuntime`, `await c._requestMode(...)`, `await c._ensureEditorRuntime()`) | `demo/smoke_ha_controls.mjs`, `demo/smoke_nav_persist.mjs`, `demo/smoke_partition_openings.mjs`, `demo/smoke_grid_scale_invariance.mjs` (везде в диффе `6d338b78..HEAD`) | Все четыре зелёные при точечном прогоне; `edit_walk`, `help_affordance`, `opening_entity_search` не изменялись в дельте — перепрогнаны отдельно, зелёные, входят в «унаследовано» ниже |
## Новые находки
### High (блокирует)
**H6. Warm-remount editor-камера теряет бит-точность — визуальный «прыжок»
кадра, воспроизводится в большинстве прогонов.**
Контракт (комментарий самого смока, `demo/smoke_warm_dialogs.mjs:1-9`,
`docs/WARM-REMOUNT.md`): при пересоздании элемента картой (Lovelace warm
remount) камера редактора должна восстановиться **бит-в-бит — ни один кадр не
отличается от прежнего**. Прогнал `node demo/smoke_warm_dialogs.mjs` **7 раз
подряд** после `npm run bundle:sync`: **5 из 7 — FAIL** на `bViewBitExact`:
```
FAILED (1):
- bViewBitExact: expected true, got "кадр 20мс: zoom=3.4
view=[256.7064599980745,247.64705882352942,346.587080003851,264.70588235294116]
(ждали 3.4 / [254.1156927887434,247.64705882352942,351.7686144225132,264.70588235294116])"
```
Во всех пяти отказов кадр приходит на 9–20мс после пересоздания, `zoom` и `y/h`
точно совпадают с ожиданием, а `x`/`w` — нет (расхождение 2–6 логических
единиц) — то есть не полный сброс камеры, а короткий одиночный кадр с
пересчитанной шириной вида, прежде чем всё встаёт на место. Это ровно та зона,
что переписал H1-фикс: `_warmAdoptViewport` (`src/houseplan-card.ts:3196-3222`)
синхронно восстанавливает `_zoom`/`_view` в точные сохранённые числа ДО того,
как `_mode` меняется на `'devices'`, но сам `_mode` первый кадр или два остаётся
`'view'` (полноширинная сцена без редакторского тулбара), а асинхронный
`_requestMode(restoredMode, false, true)` переключает `_mode` только после
`_ensureEditorRuntime()`. Между этими двумя моментами существует окно, где
камера уже стоит в «devices»-числах, но сцена ещё рендерится в View-раскладке
(другая доступная ширина stage) либо наоборот — и какой-то путь размера/layout
(вероятно, привязанный к `ResizeObserver`/первому измерению `_stageEl` для
нового инстанса, `_lastValidStageSize` пуст на свежесозданном элементе)
пересчитывает `x`/`w` под несовпадающую раскладку на один кадр.
**Смок умеет падать** — это не переписанный этой задачей файл, докстрока прямо
говорит «ПАДАЕТ на сборке до DEV-B703-03» (задача, которая изначально ввела
это требование), и я лично наблюдал детерминированный, воспроизводимый по
большинству прогонов красный результат, а не разовую аномалию: 5/7. Полный
census (195/195, единичный прогон каждого файла) этот файл не поймал —
статистически ожидаемо при частоте отказа 71% и единственной попытке (см. раздел
«Как проверялось»), а не признак исправности.
Это отдельная находка от H1 (не то же самое, что было в r1): в r1 весь смок падал
с необработанным исключением ДО того, как код доходил до этой проверки —
H1-фикс устранил исключение, но обнажил под ним ещё один, ранее не наблюдаемый
дефект в том же коде. Прямо противоречит хендоффу автора («warm_dialogs...
green», перечислено как pass в списке «targeted review smokes»). Учитывая
majority-red частоту (71%) и то, что визуальный прыжок камеры при
warm-remount — это ровно тот UX-дефект, ради предотвращения которого
существует весь контракт DEV-B703-03/WARM-REMOUNT.md, серьёзность максимальная.
### Medium (вне скоупа #337 — отдельный issue)
**M4 (→ [#346](https://github.com/Matysh/houseplan-card/issues/346)). Golden-эталоны
`geometry-devices-editor-dark`, `device-dialog-mobile-ru`,
`toggle-entity-dialog-mobile-ru`, `device-ripple-color-popover-mobile-ru`
устарели на `origin/dev` независимо от #337.** Воспроизведено на чистом
`508945c0`: `node demo/golden/run.mjs --mode=capture --scenario=<id>` для
каждого из четырёх даёт `different`; diff-PNG `geometry-devices-editor-dark`
показывает наложение устаревшего 3-кнопочного тулбара эталона на актуальный
2-кнопочный рендер (тулбар Device editor был упрощён каким-то более ранним,
уже смёрженным в `dev` изменением, после которого эталон не переснимали); три
mobile-ru кадра — устаревший размер диалога по правому/нижнему краю. Нужно
принять новые эталоны через `npm run golden:accept -- --reviewed` по чистому
`dev`, не через эту ветку. Issue заведён.
## Что проверено и корректно
- **Бюджет (AC1).** 255 778 B ≤ 256 000 B, посчитано лично. Запас упал с 615 B
(r1) до 222 B — риск, названный ещё в r1, материализуется: правки H1/H4
добавили код (новый параметр `_requestMode`, обёртки обработчиков), и запас
почти исчерпан. Не блокирую этим, так как AC1 формально выполнен и риск уже
зафиксирован, но следующая правка (в том числе исправление H6) обязана
держать бюджет в уме.
- **H2/H3/H4/M1/M2 фактически исправлены** — не только по диффу, но по
собственному прогону соответствующих smoke, см. таблицу выше.
- **H5 закрыт корректно и добросовестно** — авторское расследование не просто
заявлено, а воспроизведено мной независимо на чистом `dev`.
- **Мутационное покрытие затронутых guard'ов (changed-range)** — 58/58 ловят
подмену; changed-range корректно расширился под новый файл
`src/styles/plan.styles.ts`, где теперь реально лежит проверяемый CSS.
- **Полный browser-smoke census** — 195/195 pass на единичном прогоне (не
противоречит H6, см. выше).
- **Trailers/changelog.** Все 4 коммита несут `Issue: #337`; все — `User-Visible:
no` — корректно, т.к. пользовательское поведение (после исправления
H1–H4) возвращено к состоянию ДО дефектов r1, changelog для #337 уже писался
отдельным коммитом на предыдущем этапе (`c1aaddc7`, зафиксирован в r1).
## Унаследовано из r1
Принято без повторной проверки в этом раунде, дельта их не касается:
- **AC7 (backend asset route)** — `tests_backend/test_frontend_assets.py`,
не входит в diff `6d338b78..HEAD`; вывод r1 (CODE-REVIEW-337-r1.md,
SHA `6d338b78`, раздел «Что проверено и корректно») остаётся в силе.
- **AC8 (bundle sync/freshness/HACS zip/release scripts)** — не входит в
дельту; вывод r1 наследуется.
- **AC9 (CSS-минификатор, adversarial fixtures)** — не тронут этой дельтой,
кроме одной ссылки в `mutation-gate.mjs` на новый путь файла стилей, что не
меняет сам минификатор; вывод r1 наследуется.
- **AC10 (fingerprint mismatch)**, **AC12 (no model drift)**, **AC13 (докс не
обещает single-file install)** — вне дельты, наследуются из r1 документа.
- **Прочие смоки, не входящие в таблицу закрытия r1**, включая полный набор
195 файлов — пройдены заново единичным census + точечными повторными
прогонами там, где я подозревал гонку, а не приняты по r1 без проверки:
дельта тронула общий рендер-путь всех редакторских диалогов (H4-правка),
поэтому я не ограничился списком автора.
## Чего не проверял
- **Полный HA-harness backend** (`python -m pytest tests_backend -q`) —
недоступен в этой песочнице (нет `homeassistant`), дельта не трогает
`custom_components/**/*.py`, поэтому и не требовался в этом раунде.
- **`node scripts/model-invariants.mjs` / `npm run invariants`** — не
запускал, дельта не меняет формат геометрической записи (обоснование выше).
- **Perf-профили** — не запускал, не названы в AC и не затронуты дельтой.
- **Ручное тестирование в реальном Home Assistant** — не выполнялось.
- **Частоту гонки H6 в CI-окружении** (headless Linux CI vs моя песочница) —
не измерял отдельно; предполагаю сравнимую или более высокую частоту (CI
раннеры обычно медленнее/более вариативны по таймингу кадров, что скорее
усилит гонку, чем скроет её), но это предположение, а не измерение.
## Итог
Три из пяти High r1 закрыты полностью (H2, H3, H4), H5 закрыт как посторонний
(и это подтверждено, а не принято на слово), но **H1 закрыт лишь частично**:
устранённый крах открыл дорогу новому, статистически частому (5 из 7 прогонов)
дефекту в той же самой warm-remount/lazy-adopt зоне — H6, видимый прыжок
камеры редактора при пересоздании карточки. Это блокирующая находка, вердикт
остаётся красным. Заявление автора «warm_dialogs... green» не подтвердилось
при независимом многократном прогоне — рекомендую на следующем заходе
прогонять чувствительные к таймингу smoke (`smoke_warm_dialogs`,
`smoke_warm_owners`, `smoke_preloader_lifecycle`) не менее 3–5 раз подряд
перед хендоффом, а не один раз.
Отдельно: заведён новый issue [#346](https://github.com/Matysh/houseplan-card/issues/346)
на предсуществующий (не из #337) дефект золотых эталонов Device
editor/mobile-ru диалогов (M4) — он не блокирует #337 и не должен чиниться в
этой ветке.