houseplan-card/docs/reviews/CODE-REVIEW-337-r2.md

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).
• Целевые 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)
H2 (kiosk scale зависит от lazy runtime)	_saveKioskScale/_renderKioskDialog возвращены в eager-код хоста, больше не форвардятся в _editorRuntimeOrThrow(); вызов диалога больше не защищён условием this._editorRuntime ?	src/houseplan-card.ts:10629-10662, :11350 (${this._kioskDialog ? this._renderKioskDialog() : nothing})
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)
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»)
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-кнопочного актуального рендера
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)

Новые находки

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). 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 на предсуществующий (не из #337) дефект золотых эталонов Device editor/mobile-ru диалогов (M4) — он не блокирует #337 и не должен чиниться в этой ветке.