mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -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<N-1>»
|
||||
|
||||
Не применяется — это первый заход (r1) по этому issue.
|
||||
Reference in New Issue
Block a user