Files
houseplan-card/docs/reviews/CODE-REVIEW-411-r1.md
2026-09-01 21:16:15 +00:00

13 KiB
Raw Permalink Blame History

CODE-REVIEW · issue #411 · заход r1

Вердикт: зелёный · заход r1 · блокирующих циклов 0/2 · High: 0 · Medium: 0

Ветка: issue/411-grid-scale-static-fixture, SHA 9d5c0357b05a5a7f46b5fe4dd27d8e96e4b26596. Диапазон: origin/dev..HEAD, 1 коммит.

Скоуп

Issue #411 — smoke_grid_scale_invariance красный на dev (staticPixelsMatch: expected true, got false), устойчиво на трёх коммитах подряд. Аналитика владельца в issue (комментарий от 2026-09-02) установила причину до код-ревью: demo/smoke_grid_scale_invariance.mjs мутирует card._serverCfg/card._layout только у основной карточки; статическая карточка (houseplan-space-card) грузит config через отдельный config-store/hass.callWS и получает немасштабированный demo-config, а layout — уже масштабированный через общую ссылку sharedLayout. Пара cell_cm=1 сравнивала смешанный snapshot сама с собой. Владелец квалифицировал это как класс B (demo/** + тест-контракт), P1, лёгкий трек, без вопросов и без права трогать пороги/src/**.

Diff — ровно то, что заявлено: три файла, demo/** и test/**, src/** не тронут.

AC issue (3 штуки):

  1. Обе статические карточки строятся из согласованных config+layout одного scale-fixture; ни module cache, ни demo backend не подмешивают состояние другого масштаба.
  2. staticPixelsMatch сравнивает кадры одинакового размера и проходит на текущем строгом пороге; диагностика при несовпадении печатает размеры обоих кадров.
  3. Все 20+ утверждений смока проходят без изменений src/** и без ослабления changed/maxDelta/meanDelta; unit-контракт защищает fixture-функцию от возврата смешанного snapshot.

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

Зелёного Validate на этом SHA не было — прогнал гейты сам.

  • npx tsc --noEmit — зелёный, без вывода.
  • npm test — зелёный: 1715 passed, 1 skipped, 0 failed (совпадает с числом из хендофф-комментария автора, перепроверено самостоятельным прогоном, а не переписано с его слов).
  • npm run build — зелёный, git status --short после сборки пуст → три копии бандла (dist, custom_components/houseplan/frontend, demo/srv/assets) не разошлись. Это тривиально ожидаемо: src/** не менялся, но сверка сделана, а не предположена.
  • node scripts/process-gate.mjs --issues — зелёный, одно WARN о том, что диапазон инфраструктурный (файлов класса A нет) — ожидаемо для класса B.
  • node scripts/smoke-select.mjs --base origin/dev --head HEAD — печатает «Browser-smoke этим диффом не выбираются… src/**/*.ts не тронут». Инструмент отвечает на вопрос «что выбрать по изменению исходников карточки»; здесь исходники не менялись, поэтому корректный ответ — «выбирать нечего». Это не основание пропустить прогон смока, названного прямо в AC.
  • demo/smoke_grid_scale_invariance.mjs — прогнан лично дважды (после npm run bundle:sync, без него demo/srv/assets/houseplan-card.js не коммитится и сервер демо-стенда не поднимается). Оба раза: OK, все 22 булевых утверждения true, staticPixelsMatch: true с changed=7, maxDelta=11, meanDelta≈0.0029 — на порядок внутри порога ≤150 / ≤40 / ≤0.05, размеры кадров совпадают. Это исполнение смока «в бою», а не чтение кода: именно оно отвечает на вопрос «оно вообще работает».
  • node --test test/grid-scale-static-fixture.test.mjs — зелёный (3/3). Дисциплина «тест умеет падать» проверена вручную: временно убрал structuredClone из demo/grid-scale-static-fixture.mjs (заменил на присвоение по ссылке) — первый тест немедленно упал на notStrictEqual для config/layout (мутация исходного объекта протекла в патч). Откатил правку, дифф снова чист (git status --short пуст), тест снова зелёный.
  • node scripts/check-docs.mjs — не прогонял: diff не трогает src/**, отпечаток скриншотов документации не может устареть от изменения demo/**/test/**.
  • npm run invariants — не прогонял: diff не касается геометрии, толщины стен, layout, marker.space, open_spans; это правка тестового арматуры вокруг снимков экрана.
  • python -m pytest tests_backend -q — не прогонял: custom_components/**/*.py не тронут.
  • npm run golden:verify, остальные demo/smoke_*.mjs, performance-профили — не прогонял: AC их не называет, src/** не менялся, видимый рендер карточки правка не меняет (меняется только то, как тестовый харнесс собирает fixture для сравнения).

Находки

Нет ни одной находки уровня High или Medium.

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

AC1 (согласованный config+layout, изоляция от module cache и demo backend). Прочитано src/space-card.ts: _snap — единственный источник для render() (строки 824–836), _refreshDevices() (445–465) и _captureRenderDeviceSnapshot() (467–539); все три читают this._snap.config/this._snap.layout без дополнительного кеша, привязанного к предыдущему состоянию. demo/grid-scale-static-fixture.mjs строит патч из structuredClone(config)/structuredClone(layout) — новый объект по идентичности, что важно: несколько WeakMap-кешей в проекте (space-render.ts: staticWallGeometryCache, staticPhysicalBodiesCache, staticLightBarrierCache, staticEnabledClipCache) ключуются по объекту конфига/пространства, и клон гарантирует отсутствие коллизии со старым состоянием того же id. Смок ждёт !compact._loading && compact._snap перед патчем (новая строка 172–174), то есть исходная (несогласованная) загрузка через config-store успевает завершиться и полностью замещается патчем — сам demo backend после этого не участвует. Каждый вызов capture(..., staticCard: true) создаёt свежий houseplan-space-card (__makeStaticGridCard каждый раз удаляет #grid-static-host и создаёт новый), поэтому между reference- и detailed-кадрами нет общего инстанса, который мог бы утащить состояние другого масштаба. Тест grid-scale-static-fixture.test.mjs независимо проверяет то же на уровне функции: structuredClone и мутация источника после вызова не протекают в патч (проверено также адверсарно — см. раздел «Как проверялось»).

AC2 (совпадение размеров, диагностика). pixelDiff теперь возвращает firstSize/ secondSize при несовпадении размеров (строки 287–296) — прочитано и подтверждено регресс-тестом assert.match(source, /firstSize: \[first\.width, first\.height\]/). Пороги changed <= 150 && maxDelta <= 40 && meanDelta <= 0.05 не менялись — проверено и чтением (pixelEquivalent, строка 424–425), и исполнением: реальный прогон дал changed=7, maxDelta=11, meanDelta≈0.0029 — внутри порога с большим запасом, не «впритык подогнано».

AC3 (все утверждения смока зелёные, порог не ослаблен, unit-контракт). Живой прогон смока — все булевы поля true, включая staticPixelsMatch. src/** в диффе нет (git diff --stat подтверждён дважды). Контрактный тест защищает именно то, что называет AC: первый тест — поведение самой fixture-функции (клонирование, отпечатки, отказ на неполном вводе), второй — что smoke_grid_scale_invariance.mjs действительно использует эту функцию и не ослабляет пороги (grep по исходнику смока на точные строки). Это тот же приём, что уже используется в test/bundle-freshness.test.mjs, test/device-marker-polish-contract.test.mjs, test/performance-contract.test.mjs — устоявшийся в проекте способ дать npm test без браузера сигнал о регрессии в Playwright-смоке.

Трейлеры. Коммит 9d5c0357 несёт Issue: #411 и User-Visible: no. Видимое поведение карточки не меняется (правка целиком в тестовой арматуре), User-Visible: no корректен, changelog не требуется и не тронут.

Скоуп/класс. Диапазон — исключительно класс B (demo/**, test/**), src/** не задет, что соответствует классификации владельца в issue. Продуктовая рамка docs/SCOPE.md здесь не применяется впрямую: это не новая фича и не пользовательский сценарий, а починка честности CI-гейта (реального регресса рендера не было, что и показала аналитика владельца и мой независимый прогон смока). Формально это защищает Core user job «View mode — надёжный источник правды о доме» тем, что снимает ложное подозрение на рендер, а не тем, что меняет продукт.

Чего не проверял и почему

  • node scripts/check-docs.mjs — diff не трогает src/**.
  • npm run invariants — geometry/thickness/layout/marker.space/open_spans диффом не задеты.
  • python -m pytest tests_backend -q — custom_components/**/*.py не тронут.
  • Остальные demo/smoke_*.mjs (кроме smoke_grid_scale_invariance) — smoke-select.mjs корректно не выбрал ничего (нет src/**/*.ts в диффе); прогон всего набора не соразмерен задаче, затрагивающей одну demo-фикстуру.
  • npm run golden:verify — рендер карточки в диффе не меняется, меняется только способ подготовки тестового снимка для сравнения.
  • Performance-профили — не названы в AC, пути производительности не задеты.
  • Ручное тестирование в браузере (вне Playwright) — не выполнялось: AC явно верифицируется автотестом/смоком, ручной цикл не предусмотрен на этапе code-review (материал — диапазон коммитов, а не интерактивная сессия).

Раздел «Унаследовано из r»

Не применяется — это первый заход (r1) по этому issue.