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

CODE-REVIEW-337-r3

• Issue: https://github.com/Matysh/houseplan-card/issues/337
• Ветка: issue/337-lazy-editor-chunk, ревьюемый SHA: 16c616be165604aca8eb9c7cdeab59024a828006
• Заход: r3 · блокирующих циклов израсходовано 2/4 до этого разбора
• Вердикт: красный.

Скоуп разбора

Предыдущий вердикт — CODE-REVIEW-337-r2, SHA ревью 2beafc06398c16d1cad80fddbe77e6a40f111b9a (красный, High:1 — H6, Medium:0). Этот SHA и SHA r1 (6d338b78) в текущей истории отсутствуют: ветка была перебазирована. Проверил, на что именно:

git log --oneline 59ae6b1 -3
59ae6b10 ci: полная история без блобов там, где содержимое старых файлов не нужно
80a664bc docs: рекомендовать blobless-клон вместо полного
508945c0 fix: a 0° pair is the shared-wall model, not a duplicate — field revert of #331 §2.2

508945c0 — это тот же dev, на котором уже стояли и r1, и r2 (см. r2-документ: «branch и так уже стоит на 508945c0 = origin/dev»). Между r2 и этим заходом dev продвинулся всего на два коммита класса C/B (docs, ci, ни один не в src/** и не в custom_components/**/*.py) — не «ребейз на ушедший вперёд dev» в смысле §2.10/§7.2 (другая подсистема, смена контракта), а техническая синхронизация без содержательного чужого кода. Полный разбор с нуля не требуется; веду по дельте.

Материал дельты r2→r3 — единственный содержательный коммит:

16c616be fix: preserve warm editor camera during lazy adoption   (src/houseplan-card.ts +28/-4, плюс зеркальные dist/*)

(Коммиты 8afc8636 — документ r2, 6b482f4c/946f67ac/6f9df14d предшествуют r2 и уже разобраны в CODE-REVIEW-337-r2 — не входят в дельту этого раунда.)

Единственная находка предыдущего раунда — H6 (потеря бит-точности камеры warm-remount при lazy-adopt). Разбор ограничен: (а) верификацией фикса H6, (б) всеми проверками, до которых дотягивается 15-файловый diff (src/houseplan-card.ts + зеркальные dist//custom_components/houseplan/frontend/), (в) обязательным по AC13/§8 check-docs (диф трогает src/**).

Как проверялось

Дешёвые гейты — прогнаны лично на 16c616be, не приняты со слов автора:

Гейт	Команда	Результат
typecheck	npx tsc --noEmit	pass, exit 0
unit	npm test	1440 total, 1439 pass, 1 skip, 0 fail (у автора 1438/2 — расхождение на один conditional skip, см. ниже, не регрессия)
build+sync+budget	npm run build && npm run bundle:sync && npm run bundle:budget	initial View 255 910 B ≤ 256 000 B (запас 90 B, у автора — то же число); lazy editor 131 779 B; три дерева (dist, custom_components/houseplan/frontend, demo/srv/assets) синхронны
docs fingerprint (диф трогает src/**, AC13 прямо называет check-docs доказательством)	node scripts/check-docs.mjs	FAIL, exit 1: ERROR screenshot source fingerprint is stale; run npm run build && node demo/docs/capture.mjs — см. находку H7
mutation, changed-range	node scripts/mutation-gate.mjs --check --changed=16c616be^..16c616be	30/30 применимых guard'ов ловят подмену

Разница в skip-счётчике: у меня единственный skip — issue 281 private exact fixture has no enabled zero-range handle # SKIP private #281 fixture is not present, условный пропуск из-за отсутствующей приватной фикстуры в песочнице, к #337 не относится. Не исследовал, какой второй тест пропущен у автора на Windows — сумма (1440) совпадает, расхождение не в счёте тестов #337.

Целевые browser smokes (по дельте — файл трогает ровно зону _requestMode / _refitView / _setMode / disconnect-guard, разбор не «по названию», а по затронутым символам):

Smoke	Зачем выбран	Результат
smoke_warm_dialogs.mjs ×7 подряд, свежий bundle:sync	тот самый смок, поймавший H6 (5/7 fail в r2)	7/7 OK, bViewBitExact не подводит ни разу
smoke_warm_owners.mjs	та же warm-adopt зона (_warmAdoptViewport/_requestMode(adopt))	OK
smoke_preloader_lifecycle.mjs	та же зона, ранее ловил H1	OK
smoke_kiosk.mjs	ранее ловил H2, эта зона трогает общий _requestMode	OK
smoke_mode_transition.mjs	напрямую использует _modeTransitionBusy, которым теперь делит guard _refitView	OK
smoke_resize_wall_thickness.mjs	использует resize/refit во время editor-режима	OK
smoke_warm_remount.mjs	смежный warm-remount контракт, слабая связь по smoke-select	OK
smoke_room_resize.mjs, smoke_optimize_geometry_preflight.mjs	H3-зона (this.host. preflight), проверка на отсутствие регресса от нового isConnected/_warmModeRequest раннего return	OK

node scripts/smoke-select.mjs --base origin/dev --head HEAD даёт 175/195 «прямое совпадение» — бесполезно как фильтр для этого раунда: origin/dev не содержит всей задачи #337 целиком, поэтому инструмент показывает список для ВСЕЙ ветки, а не для дельты r2→r3. Использовал его для контекста (список выше уже включает smoke_warm_dialogs, smoke_mode_transition и smoke_warm_remount из числа тех, что он называет), но подбирал набор вручную по символам самого диффа, а не по выдаче инструмента.

Полный census 195 smoke-файлов и golden:verify не перезапускал: он уже пройден дважды (195/195 в r1, 195/195 в r2) на неизменной по этому раунду части дерева, а этот раунд — один файл в одной узкой зоне таймингов, для которой census уже доказанно нечувствителен (сам census не поймал H6 ни разу, только точечный 7×-прогон). Полный набор — предрелизный гейт (§8), не гейт ревью при такой дельте.

model-invariants — не запускал: diff не меняет ни один формат геометрической записи (layout, marker.space, open_spans, ключи толщины) — правка только про _view/_zoom кэш камеры и таймер refit. Backend/pytest — не тронут этой дельтой, не перепрогонял.

Находки

High (блокирует)

H7. check-docs красный на ревьюемом SHA — стало устаревшим ровно из-за этого коммита, AC13 не доказан.

node scripts/check-docs.mjs → ERROR screenshot source fingerprint is stale (exit 1). Причина механическая и не спорная: scripts/source-fingerprint.mjs хэширует весь src/**; 16c616be меняет src/houseplan-card.ts (единственный src-файл в этом коммите) и не обновляет sourceFingerprint в docs/images/screenshots.json, поэтому хэши расходятся. На SHA r2 (2beafc06) check-docs был зелёным (сам документ r2: «pass (7 файлов, 10 внешних ссылок)») — регрессия введена именно этим, последним коммитом раунда, а не унаследована.

Это прямо предусмотренный в задаче случай, а не мелочь: AC13 спецификации (docs/specs/337-lazy-editor-chunk.md:346-348) называет check-docs прямым доказательством этого критерия приёмки, а §8 PROCESS.md называет его «реальным блокером» именно за то, что отпечаток покрывает весь src/** — «любая правка фронтенда делает его устаревшим… выбирать тут нечего». Цена пропуска уже была измерена дважды на этой же кодовой базе: «скриншоты не пересняли в #230 и #234, и dev стоял с красным job docs, пока это не нашли при следующей задаче (#237)». Слияние 16c616be в dev в текущем виде воспроизводит ровно этот сценарий.

Автор в хендоффе смешал эту находку с H5/#346: «известный stale screenshot fingerprint; H5 уже независимо подтверждён ревьюером и вынесен в #346» — но H5/#346 про golden-эталоны (demo/golden/baselines/**, устаревший baseline тулбара Device editor и mobile-ru диалогов, не связан с #337 и воспроизводится на чистом dev), а это — про fingerprint скриншотов документации (docs/images/screenshots.json, docs-section в README/USER-GUIDE), совершенно другой гейт с другим владением. Ссылка на #346 не закрывает эту находку: #346 не затрагивает sourceFingerprint, а причина здесь — собственный код-коммит этой ветки, а не предсуществующее состояние dev.

Сам автор уже дважды в этой же задаче правильно закрывал этот же гейт без замены единого PNG («fingerprint обновлён без замены PNG, потому что наблюдаемого UI-изменения нет» — хендофф после H1–H4). Здесь этот шаг просто пропущен для последнего коммита.

Воспроизведение: git checkout 16c616be -- . (или просто на HEAD), затем npm run build && node scripts/check-docs.mjs → exit 1 с текстом выше. Исправление в скоупе задачи: прогнать Docs screenshots (workflow_dispatch) и принять новый fingerprint через npm run docs:accept -- --reviewed --from=<артефакт> (замены PNG не требуется, если рендер не изменился — ровно то, что уже делалось между r1 и r2 этой же задачи).

Medium

Нет находок этого уровня в дельте r2→r3.

Low

Нет.

Что проверено и корректно

• Сама механика фикса H6 логически закрывает найденную гонку. Разобрал правку построчно (src/houseplan-card.ts:803-925, 2549, 5880, 6662):
  • новое поле _warmModeRequest выставляется только на adopt-пути _requestMode (:894-901) и гейтит _refitView (:5880: if (this._modeTransitionBusy || this._warmModeRequest) return;) — то есть ResizeObserver-триггерный пересчёт x/w камеры (источник H6) подавлен на всё время окна между синхронным восстановлением _view и асинхронным переключением _mode через _requestMode(restoredMode, false, true);
  • после updateComplete и двух requestAnimationFrame (реальная расстановка раскладки после переключения режима) код берёт фактический stage.clientWidth/Height как новый _lastValidStageSize и снимает guard (:918-924) — следующий настоящий resize сравнивается с верной базой, а не с устаревшей;
  • guard снимается на всех путях выхода: неудача _ensureEditorRuntime() (:903), опережающий запрос или дисконнект во время await (:906-907, добавленная проверка !this.isConnected), обычная смена режима пользователем через _setMode (:6662), и disconnectedCallback (:2549) — то есть защита не может «залипнуть».
  • _warmAdoptViewport/adopt=true вызывается только на свежесозданном экземпляре карточки при warm-remount (:3110, 3243), где _refitRaf/ _pendingRefitSize заведомо в начальном состоянии — отмена «на всякий случай» на :899-900 не теряет ничьих легитимных запросов.
• Эмпирически гонка не воспроизводится. smoke_warm_dialogs.mjs — тот же файл и тот же сценарий, что дал 5 отказов из 7 в r2 — 7/7 OK на этом SHA после bundle:sync. Это выполняет дисциплину «тест умеет падать»: он падал большинством прогонов на предыдущей ревизии кода и на новой этого не делает ни разу — не совпадение единичного зелёного прогона.
• Соседние сценарии той же зоны не пострадали: warm_owners, preloader_lifecycle (H1-зона), kiosk (H2-зона), room_resize, optimize_geometry_preflight (H3-зона), mode_transition, resize_wall_thickness, warm_remount — все зелёные лично.
• Бюджет (AC1) — 255 910 B ≤ 256 000 B, посчитано лично. Запас 90 B, сократился с 222 B (r2) — тенденция, названная риском уже в r1/r2, продолжается и здесь и стоит держать в уме на любой следующей правке; формально AC1 выполнен, не блокирую этим отдельно.
• Mutation gate по коммиту — 30/30 применимых guard'ов на 16c616be^..16c616be ловят подмену; изменённая зона осталась под контролем существующих guard'ов, новых guard'ов эта точечная правка не требовала.
• Трейлеры. Issue: #337, User-Visible: no — корректно: пользовательский changelog для #337 уже зафиксирован отдельным коммитом ранее (r1), сама правка не добавляет нового видимого поведения, а завершает контракт до релиза (задача ещё не в dev, User-Visible относится к тому, что увидит пользователь после выпуска).
• Одно число — один источник. Диф не вводит и не дублирует ни одной видимой пользователю величины: _view/_zoom/_lastValidStageSize — это внутреннее состояние камеры рендера, не выводится текстом/подписью нигде в UI. Правило test/single-source-numbers.test.mjs не затронуто и проходит в общем npm test.

Закрытие раунда r2

Находка r2	Чем закрыта	Где видно
H6 (потеря бит-точности камеры warm-remount, 5/7 fail на smoke_warm_dialogs)	_refitView подавлен на время адаптации новым _warmModeRequest-guard'ом; после двух rAF после updateComplete берётся фактический размер сцены как новая база для рефита — устраняет саму гонку резайза во время переключения режима	src/houseplan-card.ts:806, 894-925, 2549, 5880, 6662; эмпирически — smoke_warm_dialogs.mjs 7/7 OK (прогнал лично) против 5/7 FAIL на предыдущей ревизии

Унаследовано из r2

Принято без повторной проверки в этом раунде — дельта (16c616be, один src-файл + зеркальные dist/) их не касается:

• AC1 (initial budget) — пересчитан лично в этом же раунде (см. таблицу гейтов), не просто унаследован: 255 910 B ≤ 256 000 B.
• AC2 (lazy boundary), AC5 (loader atomicity), AC6 (failure сохраняет View) — вне дельты; вывод CODE-REVIEW-337-r2.md (SHA 2beafc06, раздел «Закрытие раунда r1» + «Что проверено и корректно») остаётся в силе.
• AC7 (asset security), AC8 (полнота distribution), AC9 (CSS-минификатор) — не тронуты этой дельтой; вывод r2 (наследующий вывод r1, SHA 6d338b78) остаётся в силе.
• AC10 (fingerprint mismatch runtime — версии не смешиваются), AC11 (onboarding/async config editor), AC12 (no model drift) — вне дельты, наследуются из r2/r1.
• H2, H3, H4, M1–M3 (r1) и H5/M4 → #346 (golden baselines, вне скоупа #337) — закрыты и подтверждены независимым прогоном ещё в r2 (CODE-REVIEW-337-r2.md, SHA 2beafc06); эта дельта их зону не трогает.
• Полный browser-smoke census (195/195) и golden:verify (127/131, 4 «different» = #346) — пройдены дважды (r1 SHA 6d338b78, r2 SHA 2beafc06) на неизменной этим раундом части дерева; не перезапускал целиком, см. «Как проверялось».

Отдельно от AC-цепочки: AC13 (docs) не наследуется — именно на нём найдена новая находка H7 этого раунда (было зелёным на SHA r2, стало красным на SHA r3).

Чего не проверял

• Полный HA-harness backend (python -m pytest tests_backend -q с установленным homeassistant) — недоступен в этой песочнице, дельта не трогает custom_components/**/*.py.
• node scripts/model-invariants.mjs — не запускал, дельта не меняет формат геометрической записи (см. «Как проверялось»).
• Полный browser-smoke census (195 файлов) и golden:verify — не перезапускал в этом раунде; обоснование выше и в разделе «Унаследовано».
• Perf-профили — не запускал, не названы в AC и не затронуты этой дельтой.
• Частоту гонки H6 в headless Linux CI отдельно от своей песочницы — не измерял; 7/7 подряд в своём окружении расцениваю как достаточное доказательство закрытия конкретно найденной гонки (тот же порог, что задавал сам r2-документ), но абсолютную частоту в CI не оцениваю числом.
• Ручное тестирование в реальном Home Assistant — не выполнялось (в процессе нет фазы ручного тестирования, код-ревью отвечает за AC вместо него).

Итог

Фикс H6 логически и эмпирически закрывает единственную блокирующую находку r2: разобран по коду, зона правки атомарна и не расширяет скоуп, гонка не воспроизводится ни разу за 7 прогонов там, где раньше падала в 5 из 7. Однако тот же самый коммит красит обязательный по AC13 гейт check-docs — не унаследованная, а новая, введённая этим раундом находка (H7), с уже дважды на этой задаче отработанным способом закрытия. Вердикт остаётся красным по формальному правилу «High блокирует», при том что содержательная причина возврата — узкая: один пропущенный шаг (пересъёмка/приёмка fingerprint), а не дефект самого фикса камеры.