diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 755a8c41..122bf63c 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -560,6 +560,12 @@ jobs: echo "ok $name" else echo "FAIL $name" + # Диагностические строки смока печатаются раньше вердикта, и + # `tail -20` их срезал: на #411 сам смок сообщал, НАСКОЛЬКО + # разошлись кадры (введено в #302), а в логе прогона осталось + # только «expected true, got false». Числа теперь достаются + # адресно — они и есть разница между «чинить» и «гадать». + grep -aE 'pixel-diffs|^EXC |diagnostic' "/tmp/smoke-logs/$name.log" | tail -5 || true tail -20 "/tmp/smoke-logs/$name.log" fail=1 fi diff --git a/demo/grid-scale-static-fixture.mjs b/demo/grid-scale-static-fixture.mjs new file mode 100644 index 00000000..358c178c --- /dev/null +++ b/demo/grid-scale-static-fixture.mjs @@ -0,0 +1,22 @@ +/** + * Build the exact snapshot consumed by the static-card half of the grid-scale + * smoke. The main and static cards normally load through different stores; a + * comparative fixture must therefore hand the static card one atomic + * config+layout pair instead of mixing the demo backend config with the main + * card's temporary layout. + */ +export function coherentGridScaleStaticPatch({ config, layout, revision }) { + if (!config || typeof config !== 'object' || !Array.isArray(config.spaces)) { + throw new Error('grid-scale static fixture requires a complete config'); + } + if (!layout || typeof layout !== 'object' || Array.isArray(layout)) { + throw new Error('grid-scale static fixture requires a complete layout'); + } + const token = Number.isFinite(revision) ? revision : 0; + return { + config: structuredClone(config), + configFingerprint: `grid-scale-fixture:${token}:config`, + layout: structuredClone(layout), + layoutFingerprint: `grid-scale-fixture:${token}:layout`, + }; +} diff --git a/demo/smoke_grid_scale_invariance.mjs b/demo/smoke_grid_scale_invariance.mjs index 8f318a87..9bef02e5 100644 --- a/demo/smoke_grid_scale_invariance.mjs +++ b/demo/smoke_grid_scale_invariance.mjs @@ -1,5 +1,6 @@ // Issue #239: a finer coordinate grid changes precision, never the visible plan. import { launch, checkAll, finish } from './serve.mjs'; +import { coherentGridScaleStaticPatch } from './grid-scale-static-fixture.mjs'; const { page, browser } = await launch({ width: 1000, height: 860 }, 1, [], { reducedMotion: 'reduce', @@ -168,8 +169,16 @@ await page.evaluate(async () => { while (!compact.renderRoot?.querySelector('.hp-static-stage') && Date.now() < deadline) { await new Promise((resolve) => setTimeout(resolve, 30)); } + while ((compact._loading || !compact._snap) && Date.now() < deadline) { + await new Promise((resolve) => setTimeout(resolve, 30)); + } await compact.updateComplete; await new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve))); + return { + config: card._serverCfg, + layout: sharedLayout, + revision: card._cfgEpoch, + }; }; }); @@ -240,7 +249,21 @@ const capture = async (cellCm, mode, { const pixels = await stableScreenshot(stage); let staticPixels = null; if (staticCard) { - await page.evaluate(() => window.__makeStaticGridCard()); + const source = await page.evaluate(() => window.__makeStaticGridCard()); + const patch = coherentGridScaleStaticPatch(source); + await page.evaluate(async (nextPatch) => { + const compact = document.querySelector('#grid-static-host houseplan-space-card'); + if (!compact) throw new Error('grid-scale static fixture card disappeared'); + if (!compact._snap) throw new Error('grid-scale static fixture did not load its base snapshot'); + // Preserve runtime-only Set/Map values from the in-page snapshot. Only + // config+layout cross the Playwright boundary; both come from one scale. + compact._snap = { ...compact._snap, ...nextPatch }; + compact._loadedOnce = true; + compact._refreshDevices(); + compact.requestUpdate(); + await compact.updateComplete; + await new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve))); + }, patch); staticPixels = await stableScreenshot(staticStage()); } return { metrics, pixels, staticPixels }; @@ -262,7 +285,14 @@ const pixelDiff = async (left, right) => page.evaluate(async ([a, b]) => { const decode = async (base64) => createImageBitmap(await (await fetch(`data:image/png;base64,${base64}`)).blob()); const [first, second] = await Promise.all([decode(a), decode(b)]); if (first.width !== second.width || first.height !== second.height) { - return { sameSize: false, changed: Infinity, maxDelta: Infinity, meanDelta: Infinity }; + return { + sameSize: false, + firstSize: [first.width, first.height], + secondSize: [second.width, second.height], + changed: Infinity, + maxDelta: Infinity, + meanDelta: Infinity, + }; } const canvas = document.createElement('canvas'); canvas.width = first.width; canvas.height = first.height; 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. diff --git a/test/grid-scale-static-fixture.test.mjs b/test/grid-scale-static-fixture.test.mjs new file mode 100644 index 00000000..2a9cd4e7 --- /dev/null +++ b/test/grid-scale-static-fixture.test.mjs @@ -0,0 +1,39 @@ +import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; +import test from 'node:test'; +import { coherentGridScaleStaticPatch } from '../demo/grid-scale-static-fixture.mjs'; + +test('grid-scale static fixture keeps config and layout in one isolated snapshot', () => { + const config = { spaces: [{ id: 'fixture', cell_cm: 1, view_box: [0, 0, 5, 5] }] }; + const layout = { device: { s: 'fixture', x: 1.2, y: 2.4 } }; + const patch = coherentGridScaleStaticPatch({ config, layout, revision: 42 }); + + assert.deepEqual(patch.config, config); + assert.deepEqual(patch.layout, layout); + assert.notEqual(patch.config, config); + assert.notEqual(patch.layout, layout); + assert.equal(patch.configFingerprint, 'grid-scale-fixture:42:config'); + assert.equal(patch.layoutFingerprint, 'grid-scale-fixture:42:layout'); + + config.spaces[0].cell_cm = 5; + layout.device.x = 99; + assert.equal(patch.config.spaces[0].cell_cm, 1); + assert.equal(patch.layout.device.x, 1.2); +}); + +test('grid-scale smoke uses the coherent static snapshot without weakening raster limits', () => { + const source = readFileSync(new URL('../demo/smoke_grid_scale_invariance.mjs', import.meta.url), 'utf8'); + assert.match(source, /coherentGridScaleStaticPatch\(source\)/); + assert.match(source, /compact\._snap = \{ \.\.\.compact\._snap, \.\.\.nextPatch \}/); + assert.match(source, /firstSize: \[first\.width, first\.height\]/); + assert.match(source, /secondSize: \[second\.width, second\.height\]/); + assert.match(source, /diff\.changed <= 150 && diff\.maxDelta <= 40 && diff\.meanDelta <= 0\.05/); +}); + +test('grid-scale static fixture rejects partial inputs', () => { + assert.throws(() => coherentGridScaleStaticPatch({}), /complete config/); + assert.throws(() => coherentGridScaleStaticPatch({ config: {}, layout: {} }), /complete config/); + assert.throws(() => coherentGridScaleStaticPatch({ + config: { spaces: [] }, layout: [], + }), /complete layout/); +});