From 319b25c281c5eb975ca02c9c8913b7d7a8674ec5 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Tue, 1 Sep 2026 21:16:15 +0000 Subject: [PATCH] docs: review document for #411 Issue: #411 User-Visible: no --- docs/reviews/CODE-REVIEW-411-r1.md | 144 +++++++++++++++++++++++++++++ 1 file changed, 144 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-411-r1.md diff --git a/docs/reviews/CODE-REVIEW-411-r1.md b/docs/reviews/CODE-REVIEW-411-r1.md new file mode 100644 index 00000000..f64d9c94 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-411-r1.md @@ -0,0 +1,144 @@ +# 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.