mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-05 22:29:05 +00:00
merge: integrate #451 render performance
Owner arbitration accepts the reviewed tip without a fifth review cycle. Issue: #451 User-Visible: yes
This commit is contained in:
Sergey Matyunin
@@ -2,6 +2,12 @@
|
||||
|
||||
## Unreleased
|
||||
|
||||
- Large plans now keep their heavy room, wall, lighting, decor and device scene
|
||||
stable during hover, pan, pinch and editor drags, and unrelated Home Assistant
|
||||
state updates no longer rebuild the full card; interactions remain visually
|
||||
and functionally unchanged while responding much more smoothly
|
||||
([#451](https://github.com/Matysh/houseplan-card/issues/451)).
|
||||
|
||||
## v1.72.0-beta.2 — 2026-09-05
|
||||
|
||||
- Double-clicking or double-tapping free plan background in View or kiosk now
|
||||
|
||||
@@ -8,6 +8,13 @@
|
||||
|
||||
## Не выпущено
|
||||
|
||||
- На больших планах тяжёлая сцена комнат, стен, света, подложки и устройств
|
||||
теперь остаётся неизменной во время hover, pan, pinch и перетаскиваний в
|
||||
редакторах, а посторонние обновления Home Assistant больше не перестраивают
|
||||
карточку целиком; внешний вид и результат действий не меняются, но отклик
|
||||
становится заметно плавнее
|
||||
([#451](https://github.com/Matysh/houseplan-card/issues/451)).
|
||||
|
||||
## v1.72.0-beta.2 — 2026-09-05
|
||||
|
||||
- Двойной клик или двойной тап по свободному фону плана в Просмотре и киоске
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
"fixture": "synthetic-only",
|
||||
"chromium": "151.0.7922.34",
|
||||
"oxipng": "oxipng 10.2.0",
|
||||
"sourceFingerprint": "eb4d09faffea5a1b9a958e2b0ed073fd70924271e02a7fa3a73ed9c3f08805a9",
|
||||
"sourceFingerprint": "eb0b787c06ef06671e953d560312329da5a3774087b9c740e2a4717ea8a78427",
|
||||
"captureScriptSha256": "cadb8e1bcab9f1dcdd7d75b3b90ddcbaaeb2b8c2a098f575a21f39ff70f5c59c",
|
||||
"command": "npm run build && node demo/docs/capture.mjs",
|
||||
"scenarios": {
|
||||
@@ -15,7 +15,7 @@
|
||||
},
|
||||
"theme": "dark",
|
||||
"language": "en",
|
||||
"sourceSha256": "eb4d09faffea5a1b9a958e2b0ed073fd70924271e02a7fa3a73ed9c3f08805a9",
|
||||
"sourceSha256": "eb0b787c06ef06671e953d560312329da5a3774087b9c740e2a4717ea8a78427",
|
||||
"imageSha256": "80a70361dc18dd0461568df332062e6482c633af5d280954f8b675701418a76d"
|
||||
},
|
||||
"view-touch": {
|
||||
@@ -26,7 +26,7 @@
|
||||
},
|
||||
"theme": "dark",
|
||||
"language": "en",
|
||||
"sourceSha256": "eb4d09faffea5a1b9a958e2b0ed073fd70924271e02a7fa3a73ed9c3f08805a9",
|
||||
"sourceSha256": "eb0b787c06ef06671e953d560312329da5a3774087b9c740e2a4717ea8a78427",
|
||||
"imageSha256": "4106cc28847047505f46921ff95765d8abdf5b382d9d17d4c5e4ad129dd8f6be"
|
||||
},
|
||||
"space-create": {
|
||||
@@ -37,7 +37,7 @@
|
||||
},
|
||||
"theme": "dark",
|
||||
"language": "en",
|
||||
"sourceSha256": "eb4d09faffea5a1b9a958e2b0ed073fd70924271e02a7fa3a73ed9c3f08805a9",
|
||||
"sourceSha256": "eb0b787c06ef06671e953d560312329da5a3774087b9c740e2a4717ea8a78427",
|
||||
"imageSha256": "617b51b3648498787b5039980c9f3eceb75ba56ed63a1a20e616bc05bc304362"
|
||||
},
|
||||
"room-contour-close": {
|
||||
@@ -48,7 +48,7 @@
|
||||
},
|
||||
"theme": "dark",
|
||||
"language": "en",
|
||||
"sourceSha256": "eb4d09faffea5a1b9a958e2b0ed073fd70924271e02a7fa3a73ed9c3f08805a9",
|
||||
"sourceSha256": "eb0b787c06ef06671e953d560312329da5a3774087b9c740e2a4717ea8a78427",
|
||||
"imageSha256": "1dab6cc3b9d1bf7d8c40f0e5137f8c688683c9b7eabc5167da99d41ecfdd5b79"
|
||||
},
|
||||
"plan-context-tray": {
|
||||
@@ -59,7 +59,7 @@
|
||||
},
|
||||
"theme": "dark",
|
||||
"language": "en",
|
||||
"sourceSha256": "eb4d09faffea5a1b9a958e2b0ed073fd70924271e02a7fa3a73ed9c3f08805a9",
|
||||
"sourceSha256": "eb0b787c06ef06671e953d560312329da5a3774087b9c740e2a4717ea8a78427",
|
||||
"imageSha256": "94ef50762753c0ac6ddc84d2521c4232a3f9ecd89843810d2df61c517592bed7"
|
||||
},
|
||||
"device-editor": {
|
||||
@@ -70,7 +70,7 @@
|
||||
},
|
||||
"theme": "dark",
|
||||
"language": "en",
|
||||
"sourceSha256": "eb4d09faffea5a1b9a958e2b0ed073fd70924271e02a7fa3a73ed9c3f08805a9",
|
||||
"sourceSha256": "eb0b787c06ef06671e953d560312329da5a3774087b9c740e2a4717ea8a78427",
|
||||
"imageSha256": "7a601769de38aa19c2e280f6ff4cf3695b854b1d1c6d1f8de4e6b5e8f7eb1559"
|
||||
},
|
||||
"device-display-preview": {
|
||||
@@ -81,7 +81,7 @@
|
||||
},
|
||||
"theme": "dark",
|
||||
"language": "en",
|
||||
"sourceSha256": "eb4d09faffea5a1b9a958e2b0ed073fd70924271e02a7fa3a73ed9c3f08805a9",
|
||||
"sourceSha256": "eb0b787c06ef06671e953d560312329da5a3774087b9c740e2a4717ea8a78427",
|
||||
"imageSha256": "0939875f8631694f4de0ef4fd01032010ede8c7d49fb7c0a857612d4b3afff93"
|
||||
},
|
||||
"background-editor": {
|
||||
@@ -92,7 +92,7 @@
|
||||
},
|
||||
"theme": "dark",
|
||||
"language": "en",
|
||||
"sourceSha256": "eb4d09faffea5a1b9a958e2b0ed073fd70924271e02a7fa3a73ed9c3f08805a9",
|
||||
"sourceSha256": "eb0b787c06ef06671e953d560312329da5a3774087b9c740e2a4717ea8a78427",
|
||||
"imageSha256": "054170fd9ef45762b602b4d5c9c3b9ea9724858be61af137970c246f485c13bb"
|
||||
},
|
||||
"room-card": {
|
||||
@@ -103,7 +103,7 @@
|
||||
},
|
||||
"theme": "dark",
|
||||
"language": "en",
|
||||
"sourceSha256": "eb4d09faffea5a1b9a958e2b0ed073fd70924271e02a7fa3a73ed9c3f08805a9",
|
||||
"sourceSha256": "eb0b787c06ef06671e953d560312329da5a3774087b9c740e2a4717ea8a78427",
|
||||
"imageSha256": "2ae4a58853d98e10d12456b2078ec2f6a0b597722c8310d7b016abd2bc42561e"
|
||||
},
|
||||
"device-info": {
|
||||
@@ -114,7 +114,7 @@
|
||||
},
|
||||
"theme": "dark",
|
||||
"language": "en",
|
||||
"sourceSha256": "eb4d09faffea5a1b9a958e2b0ed073fd70924271e02a7fa3a73ed9c3f08805a9",
|
||||
"sourceSha256": "eb0b787c06ef06671e953d560312329da5a3774087b9c740e2a4717ea8a78427",
|
||||
"imageSha256": "cb37f7eefd936f98ef44969b21dfe8e40ee27d1f558abe8d53c0223f6488b8a6"
|
||||
}
|
||||
},
|
||||
|
||||
@@ -0,0 +1,301 @@
|
||||
# CODE-REVIEW-451-r1
|
||||
|
||||
- **Issue:** #451 «План подтормаживает: диагностика на каждый кадр, реактивная камера, отсутствие фильтра HA-тиков»
|
||||
- **Этап:** код-ревью, заход r1 (первый действительно проведённый — предыдущая попытка не читала код: ветка не ребейзилась на `dev` без конфликта, цикл не израсходован)
|
||||
- **Материал:** `git log --oneline origin/dev..HEAD` / `git diff origin/dev...HEAD`, SHA `1850eb18b5e8f0813f9d2efb50694dc4ca1fff26`
|
||||
- **ТЗ:** `docs/specs/451-render-performance.md` (полный трек, `SPEC-REVIEW-451-r2` зелёный)
|
||||
- **Вердикт:** см. итог в конце документа
|
||||
|
||||
## Скоуп диффа
|
||||
|
||||
75 файлов, +3947/‑1166 (без учёта `dist/**`/`custom_components/.../frontend/**`, класс D). Новые модули:
|
||||
`src/render-invalidation.ts`, `src/houseplan-render-lifecycle.ts`, `src/live-viewport.ts`,
|
||||
`src/live-hover.ts`, `src/live-editor.ts`, `src/live-interaction-runtime.ts`,
|
||||
`src/pointer-move-queue.ts`, `src/interaction-types.ts`. Изменены `src/houseplan-card.ts`
|
||||
(overridden `requestUpdate`, diagnostics, terminal reconciliation), `src/houseplan-editor-runtime.ts`
|
||||
(pointer-move очереди для plan/decor/backdrop/opening/resize), `src/render-device-snapshot.ts`
|
||||
(`entityIds`), `src/zigbee-topology-overlay-bridge.ts` (live-layer маркер). Плюс performance harness
|
||||
(`demo/benchmark_large_house.mjs`, `budgets-large-house-interaction.json`, `card-contract.mjs`,
|
||||
`evaluate.mjs`, `performance.yml`) и точечные правки существующих `demo/smoke_*.mjs` под новую разметку
|
||||
`data-hp-live-*`. Геометрия/модель стен, `layout`, `marker.space`, `open_spans` не тронуты — гейт
|
||||
`npm run invariants` не требуется, что подтверждается и явным «Non-scope» ТЗ.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
| Гейт | Команда | Результат |
|
||||
|---|---|---|
|
||||
| typecheck | `npx tsc --noEmit` | зелёный |
|
||||
| unit | `npm test` | 1956 tests, 1955 pass, 1 skip, 0 fail |
|
||||
| build + бандл | `npm run build && cmp dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js` + `diff -rq` на `houseplan-assets` | совпадают байт-в-байт |
|
||||
| docs fingerprint | `node scripts/check-docs.mjs` (обязателен — diff трогает `src/**`) | «Documentation checks passed (7 files, 12 external links)» |
|
||||
| any-гейт | не запускал отдельно — новые файлы не содержат `any` (проверено чтением), `npm test`/`typecheck` зелёные | проверено чтением |
|
||||
| smoke-select | `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | 98 прямых совпадений + 36 слабых из 221 (см. `smoke-select.txt`) |
|
||||
| smoke (почти полный набор) | все 134 файла из прямых+слабых совпадений, `node demo/smoke_<name>.mjs` по одному | 133 OK, 1 нестабильный (см. ниже, не регрессия) |
|
||||
| golden | `npm run golden:verify` | 153/153 сценариев `passed`, 0 diff — доказывает AC9 (pixel-equivalence) |
|
||||
| performance (структурные assert'ы) | `npm run benchmark:large-house-interaction -- --samples=7 --warmups=1` | завершился без throw — структурный контракт (0 full render на hover/pan/editor move, 1 terminal, 0 diagnostics scans) подтверждён на этом SHA |
|
||||
| performance (абсолютные потолки) | `npm run benchmark:compare -- --absolute-only --budgets=demo/performance/budgets-large-house-interaction.json --candidate=<report>` | **6 проверок красные** — см. находку H1 |
|
||||
| backend | не запускал — диф не трогает `custom_components/**/*.py` (проверено чтением: `git diff --name-only` не содержит `.py`) | проверено чтением |
|
||||
| invariants | не запускал — диф не трогает геометрию/`layout`/`marker.space`/`open_spans` (проверено чтением) | проверено чтением |
|
||||
|
||||
Не прогонял полный `demo/smoke_*.mjs` (все ~221) и полный `performance.yml` (`Full Performance`,
|
||||
парный base/candidate на двух чекаутах) — первое избыточно (134 из 221 уже покрывают все «прямые» и
|
||||
«слабые» совпадения smoke-select, второе требует парного окружения, которое не гейт код-ревью
|
||||
(PROCESS.md §8: «полные наборы — предрелizный гейт»). Раздельный прогон `--absolute-only` (см. выше)
|
||||
компенсирует это для абсолютных потолков нового профиля — и именно он нашёл H1.
|
||||
|
||||
## Находки
|
||||
|
||||
### H1 (High, блокирует) — новый performance-профиль `large-house-interaction-v1` не проходит собственный бюджет
|
||||
|
||||
**AC10** требует: «новый interaction profile проходит все structural, absolute и base-relative checks
|
||||
§11». Структурные проверки (0 full render на hover/pan/camera/editor move, ровно 1 terminal, 0
|
||||
diagnostics scans, стабильный heavy DOM, ошибка совмещения SVG/HTML ≤1px) — подтверждены, профиль не
|
||||
бросает исключение. Но абсолютные потолки, которые сам же диф вводит в
|
||||
`demo/performance/budgets-large-house-interaction.json`, не выполняются при фактическом прогоне.
|
||||
|
||||
**Воспроизведение** (детерминировано, не связано со скоростью машины — см. ниже):
|
||||
|
||||
```
|
||||
npm run benchmark:large-house-interaction -- --samples=7 --warmups=1 --output=/tmp/interaction-review7.json
|
||||
npm run benchmark:compare -- --absolute-only \
|
||||
--budgets=demo/performance/budgets-large-house-interaction.json \
|
||||
--candidate=/tmp/interaction-review7.json --output=/tmp/interaction-compare7.json
|
||||
```
|
||||
|
||||
Результат — 6 красных строк:
|
||||
|
||||
```
|
||||
❌ timing.interactionSeriesMs.median | 3501.5 | 3000
|
||||
❌ timing.editorSeriesMs.median | 1841.9 | 750
|
||||
❌ longTask.editorSeries.maxSingleMs | 878 | 150
|
||||
❌ longTask.editorSeries.countP95 | 4 | 3
|
||||
❌ longTask.editorSeries.totalP95Ms | 1633 | 300
|
||||
❌ cache.entries.cleanFloor | 140 | 100
|
||||
```
|
||||
|
||||
`cache.entries.cleanFloor` — счётчик записей `Map`, не таймінг, машинно-независим. Проверено на **двух**
|
||||
независимых прогонах (`--samples=3` и `--samples=7`), значение **140 во всех 10 сэмплах без единого
|
||||
отклонения**. Для контроля прогнан немодифицированный `large-house-v1` (существует и на `dev`, диф его
|
||||
не трогает):
|
||||
|
||||
```
|
||||
npm run benchmark:large-house -- --samples=3 --warmups=1 --output=/tmp/base-profile-review.json
|
||||
# cleanFloor: 100, 100, 100 — ровно документированный потолок (demo/performance/README.md:
|
||||
# «the reviewed fixture warms exactly 100 deterministic room/physical-body entries»)
|
||||
```
|
||||
|
||||
Значит переполнение специфично именно для новой `editor series` части `large-house-interaction-v1`
|
||||
(последовательность `_setMode('plan')` → `_tool='resize'` → cancel → `_setMode('decor')` →
|
||||
`_setMode('view')`), которую этот же диф добавил в `demo/benchmark_large_house.mjs`. Разница ровно
|
||||
+40 записей (два «лишних» полных прохода флор-кэша сверх документированного одного) коррелирует с тем,
|
||||
что тайминговые метрики именно `editorSeriesMs`/`longTask.editorSeries.*` превышены в 2,4× и более —
|
||||
похоже на одну и ту же причину (лишний(е) пересчёт(ы) `_cleanFloor`/`floorMinusBodies` при переключении
|
||||
режима редактора), а не на независимый шум.
|
||||
|
||||
Попытка воспроизвести на упрощённой ручной фикстуре (`page.evaluate` с прямым вызовом `_setMode` без
|
||||
полного цикла реального pointer-move, который использует бенчмарк) рост `_cleanFloorCache.size` не
|
||||
показала — то есть причина не в самом факте смены режима, а где-то в фактической
|
||||
resize/decor-транзакции бенчмарка (drag-preview → cancel) или в стечении с реальными pointer-событиями.
|
||||
Дальнейшая локализация — задача автора; здесь важен сам факт: **AC10 не выполнен по написанному самим
|
||||
автором для этой задачи бюджету**, а не только гипотеза о причине.
|
||||
|
||||
Тайминговые превышения (`interactionSeriesMs`, `editorSeriesMs`, `longTask.editorSeries.*`) в отличие
|
||||
от `cleanFloor` теоретически могут быть частично усилены более медленной/разделяемой машиной ревьюера
|
||||
против выделенной машины владельца (`demo/performance/README.md`: «Local Windows checkout is the
|
||||
day-to-day environment... local report is diagnostic only»), но margin (1842 мс против потолка 750 мс,
|
||||
878 мс одна long task против потолка 150 мс) для чистого шума великоват, и коррелирует с детерминированным
|
||||
`cleanFloor`-превышением. Автору стоит перепроверить оба числа на каноническом Linux CI парном прогоне
|
||||
(`performance.yml`), но независимо от результата **cache-count уже доказан красным и машинно не
|
||||
объясним** — это не «непрогнанный дорогой гейт», это прогнанный и красный.
|
||||
|
||||
**Почему High, а не Medium.** Задача, ценность 10/10 и P1, явно посвящена именно тому, чтобы новый
|
||||
перф-контракт «краснел сам по себе, а не только на фоне предыдущего коммита» (issue, раздел «Почему это
|
||||
не поймал перф-гейт»). Проверка, которую AC10 называет своим доказательством, при фактическом запуске
|
||||
красная. Это не вопрос «недостаточно полно проверили», а обнаруженный красный автотест по собственному,
|
||||
введённому этим же диффом контракту — ровно то, что код-ревью обязано поймать (PROCESS.md §2.7:
|
||||
«ревьюер отвечает за AC… либо доказан автотестом… либо разобран по коду»). Фикс — в скоупе (тот же
|
||||
`demo/benchmark_large_house.mjs`/бюджет), отдельный issue не заводится.
|
||||
|
||||
### M1 (Medium, в скоупе) — обещанный в самом ТЗ «targeted production-bundle smoke» не создан
|
||||
|
||||
ТЗ §13.2 и §14 (план реализации, п. 5) явно называют отдельный артефакт: «Targeted `demo/smoke_*.mjs` —
|
||||
production-bundle render-count и interaction matrix», инструментирующий full-render count, lightweight
|
||||
paints, diagnostics scans, heavy node identity, writes на PRODUCTION-бандле (не на perf-фикстуре) — и
|
||||
именно это заявлено как доказательство AC5/AC6/AC7/AC8 (`smoke` в их графе доказательства).
|
||||
|
||||
`git diff origin/dev...HEAD --diff-filter=A` показывает: новых файлов `demo/smoke_*.mjs` нет вовсе.
|
||||
Изменения в существующих смоках (`smoke_decor.mjs`, `smoke_furniture.mjs`, `smoke_decor_text.mjs`,
|
||||
`smoke_resize_pointer_real_plan.mjs`, `smoke_pan_any_zoom.mjs`, `smoke_room_fit.mjs`,
|
||||
`smoke_infinite_canvas.mjs`, `smoke_opening_measure.mjs`, `smoke_partition_openings.mjs`,
|
||||
`smoke_drag_bounds.mjs`, `smoke_room_tooltip_toggle.mjs`, `smoke_ux_fixes.mjs`) — точечная подгонка под
|
||||
новую разметку `data-hp-live-*`/тайминг RAF, ни один не считает full-render/diagnostics-scan счётчики.
|
||||
|
||||
Единственное место, где эти структурные гарантии вообще проверяются — `demo/benchmark_large_house.mjs
|
||||
--profile=large-house-interaction-v1`, которое подключено только к job'е `performance.yml` → «Full
|
||||
Performance», запускаемой (см. `demo/performance/README.md`, «CI contracts») **на промоушен в `main`,
|
||||
раз в неделю и по ручному диспатчу** — не на каждый push в `dev`. Блокирующий `performance_smoke` в
|
||||
`validate.yml` меряет только `large-house-glow-overlay-v1`, этого профиля не касается. При принятой в
|
||||
проекте модели прямых коммитов в `dev` без PR (AGENTS.md) это означает: следующий обычный коммит,
|
||||
который случайно вернёт diagnostics-scan на каждый рендер или полный render на hover, **не будет
|
||||
пойман никаким гейтом до следующего еженедельного/promotion-прогона** — то есть тем самым классом
|
||||
слепого пятна, ради которого заведён #451 (issue, раздел «Почему это не поймал перф-гейт»: «гейт
|
||||
исправно подтверждал, что "не стало хуже", пока пользователь смотрел на 451-миллисекундные фризы»).
|
||||
|
||||
Фикс в скоупе: добавить обещанный `demo/smoke_*.mjs` (инструментирование `_renderBody`/
|
||||
`_bindingStatus` аналогично тому, что уже сделано внутри `benchmark_large_house.mjs`, но на
|
||||
production-бандле через `demo/serve.mjs`, в постоянно запускаемом наборе).
|
||||
|
||||
### M2 (Medium, в скоупе) — нет мутанта в `scripts/mutation-gate.mjs` для новых защитных механизмов
|
||||
|
||||
PROCESS.md §2.7: «Мутант в `scripts/mutation-gate.mjs` обязателен, когда защита живёт в продуктовом
|
||||
коде и проверяется дорогим гейтом (смок, бэкенд, golden): там ревьюер не воспроизведёт отрицательный
|
||||
прогон второй раз». `git diff origin/dev...HEAD -- scripts/mutation-gate.mjs` — пусто, ни одной новой
|
||||
записи.
|
||||
|
||||
Часть новых защит — чистые юниты, и для них я сам прогнал мутацию и увидел красный тест (см. «Таблица
|
||||
доказательств» ниже: `classifyHassRenderChange`, `RenderLifecycle.diagnostics`) — этого достаточно,
|
||||
отдельный мутант не нужен. Но интеграционная проводка в `houseplan-card.ts` (переопределённый
|
||||
`requestUpdate`, `_flushHa`/terminal reconciliation, RAF-coalesced camera/hover/editor paints) —
|
||||
юнитами не покрыта и проверяется только дорогим Playwright-бенчмарком (тем же, что в M1/H1). Без
|
||||
постоянного мутанта в следующем раунде (после того, как H1 будет исправлен и профиль перестанет
|
||||
краснеть) ревьюер снова не сможет дёшево доказать «тест умеет падать» — придётся заново гонять
|
||||
браузерный бенчмарк вручную, как это делал я.
|
||||
|
||||
### Low — известная нестабильность `smoke_smooth_zoom.mjs` под нагрузкой (не регрессия, к сведению)
|
||||
|
||||
При параллельном прогоне 134 смоков (4 конкурентных пакета) `demo/smoke_smooth_zoom.mjs` дал один
|
||||
красный результат (`wheelRetargetsRunningTween`/`wheelHasIntermediateFrames`/`wheelStreamPersistsOnce`
|
||||
= false). Проверил отдельно: последовательно (без конкуренции за CPU) — 11/11 прогонов зелёные, как на
|
||||
этой ветке, так и на чистом `origin/dev`. Затем воспроизвёл красный результат **на обеих** ветках
|
||||
одинаково при явной конкуренции (4 параллельных `smoke_smooth_zoom.mjs` + 3 тяжёлых смока рядом: 2-3 из
|
||||
4 падали и на `dev`, и на PR). Вывод: это существующая до #451 чувствительность теста к CPU-контеншну
|
||||
(reliance на реальные `requestAnimationFrame` тайминги), не регрессия этой задачи. Не блокирует, в
|
||||
скоуп #451 не входит, отдельный issue не заводится (не находка, а объяснённая флакиность).
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- **AC1** (HA intake ≠ visual invalidation): `requestUpdate('hass', …)` в `houseplan-card.ts`
|
||||
делегирует решение `LiveRuntime.hass()` → `classifyHassRenderChange()`; при `'none'`/deferred-`'state'`
|
||||
intake (`intakeHass`) всё равно выполняется синхронно через `RenderLifecycle.observe`. Доказано
|
||||
юнитом `test/houseplan-render-lifecycle.test.mjs` («HA intake runs once even when an unrelated visual
|
||||
update is skipped») + мутацией (см. ниже).
|
||||
- **AC2** (dependency classifier): `src/render-invalidation.ts`. Структурные top-level поля (`entities`,
|
||||
`devices`, `themes`, `locale.*`, неизвестный новый ключ) — fail-open на `'structural'`; только `states`
|
||||
сравнивается по dependency-scoped `entityIds`. Доказано `test/render-invalidation.test.mjs` (4/4) +
|
||||
мутацией: убрал ранний `return 'none'`→заменил на безусловный `return 'state'` в скомпилированном
|
||||
`test-build/render-invalidation.js` (не в `src/**`, генерируемый файл) — тест
|
||||
«an unrelated HA state row does not invalidate the plan frame» покраснел (`expected 'none', actual
|
||||
'state'`), затем восстановил файл, тест снова зелёный.
|
||||
- **AC3** (intake переживает пропущенный рендер): dependency projection в `_captureRenderDeviceSnapshot`
|
||||
(houseplan-card.ts:4563+) собирает entity/device/area из bindings, room temp/hum source, opening
|
||||
entity refs, live-text, `sun.sun`, vacuum source — доказано чтением и подтверждено проходом смоков
|
||||
`smoke_linked_virtual_light`, `smoke_static_icon`, `smoke_cover_no_plate`,
|
||||
`smoke_cover_plate_precedence`, `smoke_entity_parent_dedup`, `smoke_yellow_principle` (все зелёные).
|
||||
- **AC4** (diagnostics cache): `RenderLifecycle.diagnostics()`/`invalidate()` в
|
||||
`houseplan-render-lifecycle.ts`. Единственный вызов `houseplanDiagnostics()`-подобной логики теперь и
|
||||
в `_renderBody` (через `this._renderLife.diagnostics(...)`), и в публичном `houseplanDiagnostics()` —
|
||||
один источник (см. «Одно число — один источник» ниже). Доказано юнитом («diagnostics scan is cached
|
||||
and invalidated by tracked state presence») + мутацией: заменил guard `if (this.diagnosticsCache)` на
|
||||
`if (false && this.diagnosticsCache)` в `test-build/houseplan-render-lifecycle.js` — тест покраснел
|
||||
(`2 !== 1`, ожидался один скан вместо двух), восстановил, снова зелёный.
|
||||
- **AC5/AC6/AC7** (0 full render на hover/pan/pinch/camera/editor move, 1 terminal, deferred-relevant-tick
|
||||
last-wins): структурные assert'ы внутри `demo/benchmark_large_house.mjs` (throw при нарушении)
|
||||
подтверждены самостоятельным прогоном на этом SHA — все условия выполнены (`irrelevantFullRenders:0`,
|
||||
`hoverFullRenders:0`, `panMoveFullRenders:[0,0,0,0]`/`terminalFullRenders:[1,1,1,1]`,
|
||||
`cameraMoveFullRenders:0`/`cameraTerminalFullRenders:1`, `editorMoveFullRenders:[0,0,0]`,
|
||||
`relevantDuringGestureFullRenders:0` при `relevantDuringGestureIntakes:3`, `diagnosticsScans:0`,
|
||||
`heavyNodeStable:true`, `overlayErrorPx:0.01`). Тайминги того же прогона см. H1.
|
||||
- **AC8** (coalescing сохраняет конечную координату/commit/cancel): `pointer-move-queue.ts`
|
||||
(`queueMicrotask`, гарантированно опустошается до следующего браузерного события) + подключение во
|
||||
всех точках (`_pointerMove`/device, `_physicalMove`, `_rszMove`, `_bdMove`, `_opPointerMove`, `_dtMove`,
|
||||
`_decorMoveUpdate`, `_markupMove`) с явным `flush` на terminal (`up`) и `cancel` на отмене. Доказано
|
||||
юнитом `test/live-editor.test.mjs` («pointer move queue is event-turn coalesced, last-wins and
|
||||
flushable») + прогоном затронутых production-bundle смоков (`smoke_furniture`, `smoke_decor`,
|
||||
`smoke_decor_text`, `smoke_drag_bounds`, `smoke_partition_openings`, `smoke_opening_measure`,
|
||||
`smoke_resize_pointer_real_plan` — все зелёные, включая ровно те сценарии shift-snap/magnet/rotate,
|
||||
которые чувствительны к потере промежуточного состояния).
|
||||
- **AC9** (pixel-equivalence): `npm run golden:verify` — 153/153 `passed`, 0 diff, включая
|
||||
`large-house-zoom-040/250-dark`, `large-house-warm-remount-dark`, junction-серию, hover-серию,
|
||||
dark/light/RU/EN варианты. Прямое доказательство отсутствия непреднамеренных визуальных изменений.
|
||||
- **AC11** (implementation-loop gates): typecheck/test/build зелёные (таблица выше).
|
||||
- **AC12** (schema/backend/i18n/dependencies не изменены): `git diff --name-only` не содержит
|
||||
`.py`/`i18n`/`translations`/`manifest.json`/`hacs.json` — проверено чтением. Новые кэши
|
||||
(`RenderLifecycle.diagnosticsCache`, `LiveRuntime`/`live-viewport`/`live-hover`/`live-editor` state) —
|
||||
все `WeakMap<object,…>`, ключ — сам инстанс карточки; `disconnectedCallback` вызывает
|
||||
`_liveRt?.dispose()` и `_editorRuntime?._disposeLiveEditor()` (cancel RAF, restore hidden/transparent
|
||||
DOM, `cancelHouseplanPointerMove`) — bounded per card, очищаются при disconnect, независимы между
|
||||
несколькими карточками на дашборде (WeakMap не даёт общий каталог).
|
||||
- **AC13** (документация): `demo/performance/README.md` описывает новый профиль, сценарии, структурные
|
||||
assertions, абсолютные потолки, local diagnostics-команду; `docs/CHANGELOG.md`/`docs/CHANGELOG.ru.md`
|
||||
обновлены в том же коммите `c0d61ca3` (`User-Visible: yes`), формулировка «внешний вид и результат
|
||||
действий не меняются» соответствует AC9/non-scope.
|
||||
- **Одно число — один источник**: `houseplanDiagnostics()` (публичный support-report) и внутренний
|
||||
render-path теперь читают один и тот же `RenderLifecycle.diagnostics()` — раньше было два независимых
|
||||
прохода по маркерам с одинаковой логикой (потенциальное расхождение при будущей правке одного из них),
|
||||
теперь физически один вызов на одну инвалидацию (доказано юнитом «scans === 1» выше). Другой видимый
|
||||
дважды параметр — zoom badge (`Math.round(zoom*100)%`) и `_zoom`/`_view`: и DOM-бейдж
|
||||
(`live-viewport.ts:paintLiveViewport`), и canonical-поле читают один и тот же `frameOf(host)` —
|
||||
`host._zoom`, без второго независимого округления.
|
||||
- **Трейлеры**: все 10 коммитов несут `Issue: #451`; ровно один `User-Visible: yes`
|
||||
(`c0d61ca3`, «fix: keep large-plan interactions off the full render path») с обоими changelog в том
|
||||
же коммите; остальные `User-Visible: no` (документация/фикс-ап/ребилд бандла) — корректно.
|
||||
- **Touch/pen/kiosk/fixed-floor контракт** (§9 ТЗ): не даёт отдельной ветки поведения — подтверждено
|
||||
прохождением `smoke_isometric_live_touch`, `smoke_kiosk`, `smoke_kiosk_pan_lock`, `smoke_fixed_floor`,
|
||||
`smoke_touch_tips`, `smoke_grid_scale_invariance` (все зелёные) плюс golden dark/light/RU/EN сценарии.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Полный `demo/smoke_*.mjs` (все ~221) — прогнал 134 (все «прямые» и «слабые» совпадения
|
||||
smoke-select.mjs), сознательно не прогонял оставшиеся ~87 без связи с диффом (полный набор —
|
||||
предрелизный гейт, PROCESS.md §8).
|
||||
- Парный `performance.yml` («Full Performance», base vs candidate на двух чекаутах, реальный Chromium
|
||||
CI-раннер владельца) — недоступен в этом окружении; вместо него прогнал `--absolute-only` (см. H1,
|
||||
нашёл реальную красноту) и структурные assert'ы бенчмарка напрямую. Относительное (base-vs-candidate)
|
||||
сравнение не проверено — вероятно, тоже покраснеет из-за той же причины, что и абсолютное, но это не
|
||||
проверено напрямую.
|
||||
- Backend (`python -m pytest tests_backend`) — не запускал, диф не касается `custom_components/**/*.py`
|
||||
(проверено чтением diff, не исполнением).
|
||||
- `npm run invariants` — не запускал, диф не касается геометрии/`layout`/`marker.space`/`open_spans`
|
||||
(проверено чтением diff и Non-scope раздела ТЗ).
|
||||
- Точная локализация причины лишних 40 записей `cleanFloor` (H1) — воспроизвёл и доказал факт, не нашёл
|
||||
точную строку кода; это оставлено автору при исправлении H1.
|
||||
- Ручная проверка в реальном браузере (вне headless Playwright) и на настоящей Home Assistant — вне
|
||||
доступного окружения; полагался на golden/smoke/unit/performance гейты.
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/451-render-performance`
|
||||
- SHA материала: `1850eb18b5e8f0813f9d2efb50694dc4ca1fff26`
|
||||
- Диапазон: `origin/dev...HEAD` (10 коммитов, `029aba6b`..`1850eb18`)
|
||||
- ТЗ: `docs/specs/451-render-performance.md`, ревью — `docs/reviews/SPEC-REVIEW-451-r2.md` (зелёное)
|
||||
|
||||
## Итог
|
||||
|
||||
**Вердикт: красный.** Одна High-находка (H1 — заявленный самим диффом перф-контракт `AC10` красный при
|
||||
фактическом прогоне `compare.mjs --absolute-only` по собственному новому бюджету, доказано
|
||||
детерминированным счётчиком `cache.entries.cleanFloor: 140 vs 100` плюс коррелирующими превышениями
|
||||
таймингов `editorSeriesMs`/`longTask.editorSeries.*`) блокирует переход. Плюс две Medium-находки в
|
||||
скоупе (M1 — не создан обещанный ТЗ `demo/smoke_*.mjs` для render-count на production-бандле, M2 — нет
|
||||
мутанта в `scripts/mutation-gate.mjs` для интеграционной проводки, проверяемой только дорогим
|
||||
бенчмарком). Сама архитектура решения (разделение intake/visual invalidation, dependency classifier,
|
||||
diagnostics cache, RAF-coalesced live-слои камеры/hover/редакторов, pointer-move очереди) сделана
|
||||
аккуратно и проверяемо: юнит-защиты действительно ловят мутацию, structural assertions бенчмарка
|
||||
действительно бросают исключение при нарушении контракта, golden/smoke/typecheck/build/docs — все
|
||||
зелёные. Возврат автору — на исправление конкретно H1 (и желательно M1/M2 в том же цикле, раз без
|
||||
High они и так были бы Medium-в-скоупе), без расширения скоупа задачи.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/451-render-performance`, коммит `1850eb18b5e8` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `30dcff478a0287b209be4c3066972158d6e6b539`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 30dcff478a02
|
||||
```
|
||||
- ТЗ `docs/specs/451-render-performance.md`, блоб `7c323a29110974aae369077214b9e2a74d9387c1`
|
||||
```
|
||||
git log --all --find-object=7c323a29110974aae369077214b9e2a74d9387c1 -- docs/specs/451-render-performance.md
|
||||
```
|
||||
@@ -0,0 +1,253 @@
|
||||
# CODE-REVIEW-451-r2
|
||||
|
||||
- **Issue:** #451 «План подтормаживает: диагностика на каждый кадр, реактивная камера, отсутствие фильтра HA-тиков»
|
||||
- **Этап:** код-ревью, заход r2
|
||||
- **Материал:** `git log --oneline origin/dev..HEAD` / `git diff origin/dev...HEAD`, точный SHA `cb68492c990b27e91c35130fa3a77806bd157c5e` (ветка `issue/451-render-performance`)
|
||||
- **Предыдущий раунд:** `CODE-REVIEW-451-r1`, SHA `1850eb18b5e8f0813f9d2efb50694dc4ca1fff26`, вердикт красный, High 1 / Medium 2 (SHA назван в самом документе r1, `Материал:`; в комментарии-вердикте issue SHA не упомянут — это несоответствие оформления, не находка по существу)
|
||||
- **ТЗ:** `docs/specs/451-render-performance.md` (полный трек, `SPEC-REVIEW-451-r2` зелёный)
|
||||
- **Вердикт:** см. итог в конце документа
|
||||
|
||||
## Дельта раунда
|
||||
|
||||
`git diff 1850eb18..cb68492c` — 8 источников кода/скриптов вне `dist/**`/`custom_components/.../frontend/**`
|
||||
(класс D, бинарные копии бандла синхронизированы автоматически и не разбираются построчно):
|
||||
|
||||
| Файл | Изменение |
|
||||
|---|---|
|
||||
| `src/junction-limits.ts` | новая экспортируемая `junctionLimitViolations()` — общая реализация full/affected-room проверки, вынесенная из `houseplan-editor-runtime.ts`; добавлен `lightweight`-режим (переиспользует переданный `multiWallNodes`, не пересобирает `wallBodiesGeometry`) |
|
||||
| `src/houseplan-editor-runtime.ts` | `_junctionLimitViolations` делегирует в новую функцию с `roomIds`; `_rszProjectPreview` больше не вызывает `_rszSpaceCandidateGeometry`/`_checkSpacePhysicalGeometry` на каждый move — использует lightweight junction-check, ограниченную `changedRoomIds`; полная проверка (`_rszCandidateRenderable`) осталась только в `finish()`; `_rszAcceptPreview`/`_rszCancelDrag` управляют `_cfgEpoch`/`_physicalBodiesCache.key` точнее; `_renderResizeLayer`/`_renderOpenings`/`_renderDecorLayer` получили фильтры `roomIds`/`onlyIds`/`onlyId` |
|
||||
| `src/live-editor.ts` | resize-preview теперь рисует только затронутые стены (`resizePreviewWalls`, читает `room.poly`/`wall.a|b|cm|key`, строит `WallEdgeBody` сама), а не полный `_renderWallBodies`; decor-preview делает прозрачным только перетаскиваемый шейп, а не весь `.decorlayer` |
|
||||
| `src/houseplan-card.ts` | `_terminalFrame` (0/1/2): `_renderBody()` возвращает `noChange` на кадре отмены resize/decor, если состояние байт-в-байт совпадает с исходным; `requestUpdate` сбрасывает флаг при любом именованном свойстве |
|
||||
| `src/resize-controller.ts` + `test/resize-controller.test.mjs` | `cancel()` возвращает `restoreEpoch`, чтобы `_rszCancelDrag` мог откатить `_cfgEpoch`, а не только `_wallUnionCache` |
|
||||
| `demo/benchmark_large_house.mjs` | измерение `longTask.editorSeries` разбито на три отдельных окна (по одному на resize/resize/decor-часть), вместо одного окна на весь editor-сценарий |
|
||||
| `scripts/bundle-budget.mjs` | `INITIAL_VIEW_GZIP_CEILING` 297 000 → 298 000, с обоснованием |
|
||||
| `scripts/mutation-gate.mjs` + `demo/smoke_render_invalidation.mjs` (новый) | закрывают M1/M2 r1 |
|
||||
|
||||
Геометрия и ссылки на неё прямо затронуты (`junction-limits.ts`, чтение `wall.a/b/cm/key`,
|
||||
`room.poly`, `multiWallNodesForGeometry`, `wallBodiesGeometry`, `innerContourForRoom`) — разбор
|
||||
не сокращается до «только заявленных находок», гейты по геометрии прогнаны (см. ниже).
|
||||
|
||||
## Как проверялось
|
||||
|
||||
| Гейт | Команда | Результат |
|
||||
|---|---|---|
|
||||
| typecheck | `npx tsc --noEmit` | зелёный |
|
||||
| unit | `npm test` | 1956 tests, 1955 pass, 1 skip, 0 fail |
|
||||
| build + бандл | `npm run build && node scripts/bundle-sync.mjs` + `cmp`/`diff -rq` трёх копий | совпадают байт-в-байт |
|
||||
| docs fingerprint | `node scripts/check-docs.mjs --external` (обязателен — diff трогает `src/**`) | **красный**, см. H1 |
|
||||
| any-гейт | `node scripts/no-new-any.mjs --base f8b3445d --head cb68492c` (та же пара, что использует CI-джоба этого пуша) | **красный**, см. H2 |
|
||||
| invariants | `npm run invariants -- --config <экспорт large-house фикстуры>` (diff трогает геометрию/wall-thickness) | «Инварианты выполнены: ссылки разрешимы, записи толщины находятся» |
|
||||
| smoke-select (по дельте) | `node scripts/smoke-select.mjs --base 1850eb18 --head cb68492c` | 58 прямых, 27 слабых, 1 зарегистрированная связь (`smoke_real_plan_masonry.mjs` ← `wallBodiesGeometry`) |
|
||||
| smoke (все прямые + зарегистрированная + 5 смежных по физической геометрии) | 66 файлов, по одному `node demo/smoke_<name>.mjs` (список — в приложении к комментарию) | 64 OK, 1 красный воспроизводимо (`smoke_room_resize.mjs`, см. H3), 1 красный только под конкуренцией за CPU при последовательном прогоне всех 66 (`smoke_edit_walk.mjs`; повторный изолированный прогон — 6/6 OK, тот же класс флакиности, что и `smoke_smooth_zoom` в r1, не регрессия) |
|
||||
| golden | `npm run golden:verify` | 153/153, 0 diff |
|
||||
| performance (структурные assert'ы) | `npm run benchmark:large-house-interaction -- --samples=7 --warmups=1` (×3 независимых прогона) | завершается без throw каждый раз |
|
||||
| performance (абсолютные потолки) | `npm run benchmark:compare -- --absolute-only --budgets=demo/performance/budgets-large-house-interaction.json` на всех 3 прогонах | `cache.entries.cleanFloor` = 100/100 во всех трёх (совпадает с заявленным автором числом, H1 r1 в этой части закрыт); но в каждом из 3 прогонов минимум один тайминговый чек красный (`longTask.editorSeries.maxSingleMs` 231–324 vs 150 во всех трёх; изредка также `timing.interactionSeriesMs.median` и `longTask.editorSeries.totalP95Ms`) — см. «Под вопросом» ниже |
|
||||
| backend | не запускал — диф не трогает `custom_components/**/*.py` (проверено чтением: `git diff 1850eb18..cb68492c --name-only` не содержит `.py`) | проверено чтением |
|
||||
| мутация | `node scripts/mutation-gate.mjs --id=render-invalidation-renders-irrelevant-ha` | «поймано 1 из 1» — M2 r1 подтверждён рабочим |
|
||||
|
||||
CI на этом же SHA (`cb68492c`, run `33918076536`) независимо подтверждает H1 и H2: джоба
|
||||
«Предполётные проверки» красная на `DOCS: failure`, джоба «Фронтенд: типы, юниты, мутанты,
|
||||
синхрон бандла» красная на «Новый код не добавляет any: 11» — обе с теми же файлами/строками,
|
||||
что и локальный прогон ниже. Всё, что стоит за этими двумя джобами по конвейеру (браузерные
|
||||
смоки, перф-смок, golden), было **skipped**, то есть CI не подтверждает и не опровергает
|
||||
performance-часть отдельно от того, что нашёл я локально.
|
||||
|
||||
## Находки
|
||||
|
||||
### H1 (High, блокирует) — отпечаток скриншотов документации устарел, `docs`-джоба красная
|
||||
|
||||
`node scripts/check-docs.mjs --external` (та же команда, что в `validate.yml:59`) завершается:
|
||||
|
||||
```
|
||||
ERROR screenshot source fingerprint is stale; run npm run build && node demo/docs/capture.mjs
|
||||
```
|
||||
|
||||
`visualFingerprint()` считается по всему `src/**` (#245: версия не в счёт, но код — да), а
|
||||
раунд r2 меняет `src/houseplan-card.ts`, `src/houseplan-editor-runtime.ts`,
|
||||
`src/junction-limits.ts`, `src/live-editor.ts`, `src/resize-controller.ts` без сопутствующего
|
||||
`npm run build && node demo/docs/capture.mjs`. Автор прогонял этот гейт в r1 (документ
|
||||
`CODE-REVIEW-451-r1` фиксирует «Documentation checks passed»), но в комментарии о фиксе r1
|
||||
(`Исправления по CODE-REVIEW r1`, 2026-09-04T20:48:54Z) `check-docs` не упомянут вовсе — гейт
|
||||
пропущен именно там, где diff тронул `src/**` заново. Ровно этот класс пропуска уже дважды
|
||||
оставлял `dev` с красной `docs`-джобой до следующей задачи (#230, #234 → #237); здесь он пойман
|
||||
до слияния. Фикс не меняет ни одного пикселя (перерисовка внутренняя), поэтому обновление —
|
||||
чисто отпечаток, `npm run build && node demo/docs/capture.mjs`.
|
||||
|
||||
Подтверждено независимо: CI-прогон `33918076536` на этом же SHA, джоба «Предполётные проверки»,
|
||||
`DOCS: failure`.
|
||||
|
||||
### H2 (High, блокирует) — новый код добавляет 11 непрокомментированных `any`, гейт красный
|
||||
|
||||
`node scripts/no-new-any.mjs --base f8b3445d --head cb68492c` (пара база/head, которую
|
||||
использовала CI-джоба этого пуша) находит 11 новых явных `any` на добавленных строках без
|
||||
обоснования `// any-ok: …`:
|
||||
|
||||
```
|
||||
src/houseplan-card.ts:7792, src/junction-limits.ts:51,54,57,73,
|
||||
src/live-editor.ts:61,62,65,157,163,164
|
||||
```
|
||||
|
||||
Все — в коде, который непосредственно реализует ТЗ этого раунда (сигнатуры
|
||||
`junctionLimitViolations`/`LiveEditorHost`, читающие `config`/`space`/`room` как `any`). Часть
|
||||
можно типизировать через уже существующие интерфейсы (`ServerConfig`, `SpaceModel`), часть —
|
||||
обосновать комментарием на той же строке; гейт не требует нулевого `any` в принципе (в `src/**`
|
||||
их 1034 в 49 файлах), а требует явного решения по каждой НОВОЙ строке.
|
||||
|
||||
Подтверждено независимо: тот же CI-прогон, джоба «Фронтенд: типы, юниты, мутанты, синхрон
|
||||
бандла», шаг «Новый код не добавляет any», идентичный список строк.
|
||||
|
||||
### H3 (High, блокирует) — resize потерял видимую fail-closed реакцию на невозможную геометрию посреди жеста
|
||||
|
||||
`node demo/smoke_room_resize.mjs` падает воспроизводимо (проверено дважды подряд, изолированно
|
||||
и в общем прогоне — одинаково):
|
||||
|
||||
```
|
||||
FAILED (2):
|
||||
- safe_resize.preflight_visible_reason: expected true, got false
|
||||
- safe_resize.preflight_reason_once: expected 1, got 0
|
||||
```
|
||||
|
||||
Смок (существовал до #451, не новый) подставляет `card._checkSpacePhysicalGeometry = () => ({
|
||||
ok: false })` и ожидает, что **во время** протяжки (`pointermove`, до `pointerup`) карточка
|
||||
покажет тост «last safe position/последн…» ровно один раз. До этого раунда `_rszProjectPreview`
|
||||
на каждый move вызывал `_rszSpaceCandidateGeometry()` → `_checkSpacePhysicalGeometry()` и
|
||||
отклонял проекцию с `reason: 'physical-geometry'`, если проверка проваливалась — именно так
|
||||
жест «останавливался на последней безопасной позиции» с видимой причиной. Правка H1-r1 убрала
|
||||
этот вызов из `_rszProjectPreview` целиком (осталась только lightweight junction-limit проверка,
|
||||
ограниченная `changedRoomIds`, — другой класс проверки: буквенные лимиты узлов/клиренса, а не
|
||||
собираемость геометрии стен). Полная проверка (`_checkSpacePhysicalGeometry`) осталась только в
|
||||
`_rszCandidateRenderable`, вызываемой из `_rszUp()` → `finish()` — то есть **только по
|
||||
pointerup**, с общим `resize.commit_failed` тостом вместо специфичного отказа посреди жеста.
|
||||
|
||||
Строка `// #329 AC7a: a step that would ADD a junction-limit violation is never projected, so
|
||||
the drag stops at the last allowed position…` (houseplan-editor-runtime.ts:3657) осталась в коде
|
||||
рядом с местом, где ссылалась на удалённую проверку — комментарий теперь описывает только
|
||||
junction-limit ветку, а не physical-geometry, но текст не обновлён и вводит в заблуждение.
|
||||
|
||||
Ни один юнит-тест не покрывает новую `junctionLimitViolations()`/lightweight-режим отдельно —
|
||||
единственная проверка всей цепочки жила в этом смоке, и правка её не заметила именно потому, что
|
||||
`npm test` эту ветку не касается (только браузерный смок её и держал). Это тот же структурный
|
||||
разрыв, который M2 r1 уже называл для другой части того же диффа: дорогой гейт — единственный
|
||||
свидетель контракта.
|
||||
|
||||
**Почему High.** Регресс в контракте, существовавшем до #451 (не новая функциональность этого
|
||||
issue), ловится существующим (не добавленным этим PR) тестом, воспроизводится детерминированно.
|
||||
`User-Visible: no` в трейлере коммита `cb68492c` в этом свете неточен: сценарий редкий
|
||||
(конкурентное структурное изменение во время жеста), но при его наступлении пользователь раньше
|
||||
видел объяснение и жест останавливался, а теперь видит только общий отказ после отпускания
|
||||
курсора — то есть разное поведение, а не «незаметная» оптимизация.
|
||||
|
||||
## Под вопросом, не поднимаю до находки
|
||||
|
||||
`longTask.editorSeries.maxSingleMs` (потолок 150 мс) красный во всех 3 независимых прогонах
|
||||
`--samples=7 --warmups=1` на этом SHA (231, 324, 231 мс), при этом `cache.entries.cleanFloor`
|
||||
(машинно-независимый счётчик, из-за которого r1 был красным) держит ровно 100/100 во всех трёх —
|
||||
то есть структурная часть H1-r1 закрыта чисто, а тайминговая часть держится у самой границы с
|
||||
переменным исходом (в одном из трёх прогонов краснели ещё `timing.interactionSeriesMs.median` и
|
||||
`longTask.editorSeries.totalP95Ms`, в двух других — нет). Отличие от r1: там margin был ~6×
|
||||
(878 мс против потолка 150) и детерминирован; здесь ~1.5–2× и не детерминирован — характернее
|
||||
для шумной/разделяемой машины ревьюера, чем для структурного регресса (`demo/performance/README.md`
|
||||
прямо называет локальный прогон диагностическим). CI не добрался до перф-джобы на этом SHA
|
||||
(skipped из-за H1/H2), так что канонического парного Linux-сравнения по этому раунду ещё нет ни
|
||||
у кого. Не поднимаю до отдельной находки, но и не закрываю: автору нужен чистый прогон
|
||||
`performance.yml` (или локальный на менее нагруженной машине) после того, как H1/H2/H3 будут
|
||||
исправлены, прежде чем считать AC10 доказанным на этом SHA.
|
||||
|
||||
## Закрытие раунда r1
|
||||
|
||||
| Находка r1 | Чем закрыта | Где видно |
|
||||
|---|---|---|
|
||||
| H1 (перф-бюджет `large-house-interaction-v1` красный, включая машинно-независимый `cache.entries.cleanFloor` 140≠100) | Resize-preview больше не строит канонический union/full-room clearance на каждый move — только lightweight junction-check по `changedRoomIds` | `cache.entries.cleanFloor` = 100/100 в трёх независимых прогонах этого раунда (см. таблицу выше); тайминговая часть бюджета — см. «Под вопросом» |
|
||||
| M1 (нет production-smoke на full-render/diagnostics-scan счётчики) | Добавлен `demo/smoke_render_invalidation.mjs` | файл существует, `node demo/smoke_render_invalidation.mjs` → OK |
|
||||
| M2 (нет мутанта в `scripts/mutation-gate.mjs` для интеграционной проводки) | Добавлен мутант `render-invalidation-renders-irrelevant-ha` | `node scripts/mutation-gate.mjs --id=render-invalidation-renders-irrelevant-ha` → «поймано 1 из 1» |
|
||||
|
||||
## Унаследовано из r1 (не перепроверялось в этом раунде)
|
||||
|
||||
Со ссылкой на `CODE-REVIEW-451-r1` на SHA `1850eb18`, за пределами того, чего касается дельта
|
||||
`1850eb18..cb68492c`:
|
||||
|
||||
- AC1 (HA intake ≠ visual invalidation), AC2 (dependency classifier), AC3 (terminal
|
||||
reconciliation вне resize/decor cancel-ветки), AC4 (RAF-coalesced hover/camera paths вне
|
||||
editor-preview), AC9 (pixel-equivalence) — код этих путей дельтой не тронут; повторно
|
||||
перечитан не был. AC9 переподтверждён косвенно свежим прогоном `golden:verify` (153/153) в
|
||||
этом раунде, остальное — по r1.
|
||||
- Скоуп диффа (75 файлов на SHA r1) вне восьми файлов, изменённых в этом раунде — не
|
||||
перечитывался повторно.
|
||||
- Backend/invariants-обоснование «диф не трогает `custom_components/**/*.py`» — верно и для
|
||||
дельты этого раунда (см. таблицу выше), проверено заново, не только унаследовано.
|
||||
- Трейлеры `Issue:`/`User-Visible:` коммита r1 (`f8b3445d`, `1850eb18` и предыдущие) — не
|
||||
перепроверялись; трейлеры коммита `cb68492c` проверены заново (см. H3 про неточность
|
||||
`User-Visible: no`).
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- `_rszCandidateRenderable`/`_checkSpacePhysicalGeometry` остаются полной, дорогой проверкой на
|
||||
`finish()` (пойнтерап) — итоговый коммит геометрии по-прежнему фейлится закрыто; регресс H3
|
||||
касается только видимой обратной связи ПОСРЕДИ жеста, не итоговой записи (доказано: `roomPoly`
|
||||
до/после в смоке не меняется — `preflight_no_commit` в том же смоке зелёный).
|
||||
- `resize-controller.ts`: `epochBefore`/`restoreEpoch` для cancel корректно откатывают
|
||||
`_cfgEpoch` только при совпадении `snapshotIdentity`, иначе `null` — соответствующие тесты
|
||||
`test/resize-controller.test.mjs` (оба сценария) проходят.
|
||||
- `_terminalFrame` в `houseplan-card.ts`: сбрасывается на любую именованную реактивную запись
|
||||
(`requestUpdate(name, …)` с `name !== undefined`) до того, как `_renderBody()` может вернуть
|
||||
`noChange` — то есть «проглотить» кадр может только сам вызывающий cancel-путь, а не случайное
|
||||
внешнее свойство; `npm test` (1955/1955) и `demo/smoke_render_invalidation.mjs` не показали
|
||||
расхождений DOM.
|
||||
- Единственный источник числа: диф не вводит новых пользовательски видимых величин (внутренний
|
||||
перф-рефакторинг), `npm test` включает `test/single-source-numbers.test.mjs` — зелёный.
|
||||
- `INITIAL_VIEW_GZIP_CEILING` 297 000 → 298 000 обоснован в комментарии тем же коммитом, общий
|
||||
бюджет 300 000 и долг #367 не изменены — проверено чтением `scripts/bundle-budget.mjs`.
|
||||
- Инварианты модели (`npm run invariants`) на конфиге из `demo/fixtures/large-house.mjs`:
|
||||
«ссылки разрешимы, записи толщины находятся» — новая `junctionLimitViolations()` не потеряла
|
||||
соответствие ключей записей толщины решёточным рёбрам на этой фикстуре.
|
||||
|
||||
## Чего не проверял и почему
|
||||
|
||||
- Полный набор `demo/smoke_*.mjs` (222 файла) — избыточно: 58 прямых + 1 зарегистрированная
|
||||
связь смок-селектора плюс 5 смежных по физической геометрии (`smoke_wall_junctions`,
|
||||
`smoke_wall_union_isolation`, `smoke_near_orthogonal_junction`,
|
||||
`smoke_multiwall_strip_containment`, `smoke_glow_geometry_resilience`) дают 64/66 живых
|
||||
прогонов по затронутым модулям — уже нашли реальный регресс (H3). Оставшиеся 27 «слабых»
|
||||
совпадений (общее имя `_mode`/`_baseVb`/`_physicalBodiesCache`/`cellCm`/`NORM_W`) не гонял:
|
||||
все пять физически-геометрических смоков из этого же семейства прошли, дополнительный сигнал
|
||||
от оставшихся маловероятен, а полный набор — предрелизный гейт (PROCESS.md §8), не гейт
|
||||
ревью.
|
||||
- Полный `performance.yml` (`Full Performance`, парный base/candidate на двух чекаутах) — CI это
|
||||
не запускала на этом SHA (skipped из-за H1/H2), у меня нет второго чекаута под парное
|
||||
сравнение; заменено тройным `--absolute-only` прогоном на кандидате (см. «Под вопросом»).
|
||||
- `python -m pytest tests_backend` — диф не трогает `custom_components/**/*.py` (проверено
|
||||
чтением `git diff --name-only`).
|
||||
- Мутация для новой `junctionLimitViolations()`/lightweight-режима отдельно — не потребовалась:
|
||||
H3 уже показывает красный тест по этому пути; добавлять мутант к сломанному коду
|
||||
преждевременно, стоит сделать после фикса H3, иначе он зафиксирует текущее (неверное)
|
||||
поведение.
|
||||
- Повторный запуск `smoke_edit_walk.mjs` больше двух раз для статистики флакиности — не
|
||||
требовалось: тот же класс поведения (падает только под конкуренцией за CPU при плотном
|
||||
последовательном прогоне 66 браузерных смоков подряд, чисто 6/6 в изоляции), что уже описан и
|
||||
принят как некритичный в r1 для `smoke_smooth_zoom`.
|
||||
|
||||
## Итог
|
||||
|
||||
**Вердикт: красный.** Три High-находки, все в скоупе задачи и чинятся в ней же (без отдельных
|
||||
issue): H1 — обновить отпечаток документации (`npm run build && node demo/docs/capture.mjs`),
|
||||
H2 — типизировать или обосновать 11 новых `any`, H3 — вернуть full physical-geometry preflight
|
||||
(или эквивалентную защиту) в путь `_rszProjectPreview`, чтобы посреди resize-жеста
|
||||
невозможная геометрия снова отклонялась видимо, а не только по `pointerup`. После фикса —
|
||||
чистый повторный прогон `npm run benchmark:large-house-interaction`/`compare` для снятия вопроса
|
||||
из раздела «Под вопросом».
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/451-render-performance`, коммит `cb68492c990b` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `aaa8519cee54bf2a81f9e922e583d42d7f594ab4`
|
||||
```
|
||||
git log --all --format='%H %T' | grep aaa8519cee54
|
||||
```
|
||||
- ТЗ `docs/specs/451-render-performance.md`, блоб `7c323a29110974aae369077214b9e2a74d9387c1`
|
||||
```
|
||||
git log --all --find-object=7c323a29110974aae369077214b9e2a74d9387c1 -- docs/specs/451-render-performance.md
|
||||
```
|
||||
@@ -0,0 +1,223 @@
|
||||
# CODE-REVIEW #451 — заход r3
|
||||
|
||||
- **Issue:** #451 — «План тормозит: диагностика считается на каждый кадр, перетаскивание перерисовывает всё, нет фильтра обновлений»
|
||||
- **Этап:** код-ревью (PROCESS.md §2.7)
|
||||
- **Заход:** r3 · блокирующих циклов до этого раунда: 2/4
|
||||
- **Материал раунда:** ветка `issue/451-render-performance`, `HEAD = 444562e47cb746dc1c7d740b66b2f832ca02f064`
|
||||
(сверено `git rev-parse HEAD` непосредственно перед выводом, дерево чистое, PROCESS.md §2.7/#312)
|
||||
- **Дельта раунда:** `git diff cb68492c990b27e91c35130fa3a77806bd157c5e..444562e4` — SHA взят из машинного блока
|
||||
«Материал раунда» документа `docs/reviews/CODE-REVIEW-451-r2.md` (дерево `aaa8519cee54…`, сверено
|
||||
`git cat-file -p cb68492c^{tree}` — совпадает). Файлы дельты (без `dist/**` и бандла стенда):
|
||||
`src/houseplan-card.ts`, `src/houseplan-editor-runtime.ts`, `src/junction-limits.ts`,
|
||||
`src/live-editor.ts`, `src/resize-controller.ts`, `test/resize-controller.test.mjs`,
|
||||
`docs/images/screenshots.json`, `docs/reviews/CODE-REVIEW-451-r2.md`.
|
||||
|
||||
## Скоуп раунда
|
||||
|
||||
r2 (красный, High:3) закончился на коммите `cb68492c` (диагностика — 8 файлов). С тех пор в ветку
|
||||
легли 7 коммитов: `fe03eae2`, `8d787530`, `4b6b4abc`, `a7bee0f5`, `d513f13a` (документ r2),
|
||||
`046efe96`, `444562e4`. Формально они делятся на «закрытие H1–H3 из r2» и одно **не заявленное
|
||||
в r2 замечание** — CI поймал регресс DOM после live-reconciliation ещё на `a7bee0f5` (до публикации
|
||||
документа r2), и он же чинится в этом окне. Разбираю всю дельту по коду, а не только «после
|
||||
документа r2», потому что часть закрывающих коммитов (H2 — `fe03eae2`, H3 — `4b6b4abc`) физически
|
||||
предшествует публикации `d513f13a` — r2 их не проверял (его собственная цитата диапазона
|
||||
`1850eb18..cb68492c` их не включает), и раз так, «наследовать без проверки» для них не могу.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Дельта не геометрию персистентного формата, а раннее прекращение/typing нескольких вызовов и одну
|
||||
внутреннюю функцию (`resizeLivePreflightAllowed`). Прогнал сам, не полагаясь на цитаты из комментариев:
|
||||
|
||||
| Команда | Результат |
|
||||
|---|---|
|
||||
| `npm run build` (после — `git status --short`) | зелёный, дерево не изменилось — бандл воспроизводим байт-в-байт |
|
||||
| `npm run bundle:sync` | зелёный, стенд для браузерных смоков синхронизирован |
|
||||
| `npm test` | 1956 passed / 0 failed / 1 skipped (совпадает с заявленным) |
|
||||
| `node scripts/check-docs.mjs` | «Documentation checks passed (7 files, 12 external links)» — H1 закрыт |
|
||||
| `node scripts/no-new-any.mjs --base cb68492c --head 444562e4` | «Новых any нет» (77 строк в 5 файлах) — H2 закрыт для всей дельты, не только для `fe03eae2` |
|
||||
| `node demo/smoke_room_resize.mjs` | OK — H3 закрыт (`preflight_visible_reason`/`preflight_reason_once` снова true/1) |
|
||||
| `node demo/smoke_resize_pointer_real_plan.mjs` | OK — закрыт не заявленный в r2 регресс `unrelated_pointer_ignored`/`capture_loss_restores_dom` |
|
||||
| `node demo/smoke_resize_outer_reconciliation.mjs` | OK |
|
||||
| `node demo/smoke_resize_audit_1550.mjs` | OK |
|
||||
| `node demo/smoke_resize_inner_dimensions.mjs` | OK |
|
||||
| `node demo/smoke_resize_labels.mjs` | OK |
|
||||
| `node demo/smoke_resize_wall_thickness.mjs` | OK |
|
||||
| `node demo/smoke_junction_limits.mjs` | OK |
|
||||
| `node demo/smoke_wall_key_roundtrip.mjs` | OK |
|
||||
| `node demo/smoke_bg_color.mjs` | OK |
|
||||
| `node demo/smoke_pan_any_zoom.mjs` | OK |
|
||||
|
||||
Плюс независимая архивная проверка через `gh`: CI `33920551036` (SHA `a7bee0f5`, до публикации r2)
|
||||
показал `Смоки в браузере (шард 3 из 3): failure` с точными именами
|
||||
`resize_pointer.unrelated_pointer_ignored` / `resize_pointer.capture_loss_restores_dom` — это
|
||||
доказывает, что регресс был реальным (тест умел падать), а не выдумкой из комментария автора.
|
||||
Финальный CI `33922485666` (SHA `444562e4`) зелёный целиком, включая все 3 шарда смоков и агрегатор.
|
||||
|
||||
`node scripts/smoke-select.mjs --base cb68492c --head 444562e4`: 29 прямых совпадений (`_resize`,
|
||||
`_openingsR`, `_spaceWalls`, `NORM_W`, `_junctionLimitViolations`, `_mode`, `_spaceDisplayForRender`,
|
||||
`SpaceModel`, `WallEntry`). Прогнал все, что касаются resize/geometry напрямую (список выше, 11 из
|
||||
29); остальные 18 прямых совпадений — тот же класс, что уже прогнан (`_openingsR`/`NORM_W`-смоки
|
||||
опенингов/декора, не тронутых логически в этой дельте, тип-рефакторинг без изменения поведения) —
|
||||
не гонял отдельно, см. «Чего не проверял».
|
||||
|
||||
## Закрытие r2
|
||||
|
||||
| Находка r2 | Чем закрыта | Где видно |
|
||||
|---|---|---|
|
||||
| **H1** (`check-docs --external` красный, отпечаток скриншотов устарел) | Каноническая Linux-съёмка на точном SHA, обновлён только source fingerprint | `444562e4`; сам прогнал `node scripts/check-docs.mjs` на HEAD — зелёный |
|
||||
| **H2** (11 новых `any` в `houseplan-card.ts:7792`, `junction-limits.ts:51,54,57,73`, `live-editor.ts:61,62,65,157,163,164`) | Типизация `_junctionLimitViolations`/`junctionLimitViolations` через новый экспортируемый `JunctionSharedGeometry` (`Pick<WallBodiesGeometryResult,...> \| {status:'lightweight',...}`), typed `LiveEditorHost` (`SpaceDisplay`, `SpaceModel`, `WallEntry`, `RenderOpening`) | `fe03eae2` (`src/houseplan-card.ts`, `src/houseplan-editor-runtime.ts`, `src/junction-limits.ts`, `src/live-editor.ts`); сам прогнал `no-new-any.mjs --base cb68492c --head 444562e4` — 0 новых `any` |
|
||||
| **H3** (`_rszProjectPreview` больше не вызывает `_checkSpacePhysicalGeometry()` ни разу во время drag — `smoke_room_resize` падал) | Новая `resizeLivePreflightAllowed(rooms, edgeBudget=64)`: если суммарный периметр (число вершин) комнат кандидата ≤ 64, `_rszProjectPreview` вызывает `_rszSpaceCandidateGeometry` → тот же `_checkSpacePhysicalGeometry`, что и раньше, и отклоняет шаг при `!ok`; иначе (большие планы) остаётся только дешёвая junction-limit проверка, а точная — безусловно на `pointerup` через `_commitPhysicalGeometry` (не изменился, вызывает `_checkSpacePhysicalGeometry` без всяких условий) | `4b6b4abc` (`src/resize-controller.ts:resizeLivePreflightAllowed`, `src/houseplan-editor-runtime.ts:3653,3691,3715-3734`); прочитал `_commitPhysicalGeometry` (строки 2126–2148) — вызов fail-closed проверки безусловный, не зависит от live-preflight; сам прогнал `smoke_room_resize.mjs` — OK |
|
||||
|
||||
Дополнительно (не было заявлено r2 находкой, но CI поймал регресс на `a7bee0f5` — SHA, на котором
|
||||
физически лежал код r2 к моменту анализа, хотя r2 цитировал более ранний `cb68492c`): смоки
|
||||
`resize_pointer.unrelated_pointer_ignored`/`capture_loss_restores_dom` красные. Причина —
|
||||
`_rszMove` ставил в очередь `_rszMoveNow` для ЛЮБОГО pointerId (проверка владения была только внутри
|
||||
`_rszMoveNow`), из-за чего чужой указатель вытеснял из очереди уже поставленное обновление
|
||||
настоящего владельца. Закрыто `046efe96`: guard `ownsPointer` перенесён в начало `_rszMove` (до
|
||||
постановки в очередь), плюс новый флаг `_resizeBaseFrameStable` — если во время resize случился
|
||||
полноценный render (`routeLiveEditorUpdate` вернул «не live» при активном `_resize.preview`),
|
||||
`_rszCancelDrag` больше не притворяется, что можно оставить старый терминальный DOM, и делает один
|
||||
обязательный reconciliation render. Сам прогнал `smoke_resize_pointer_real_plan.mjs` — OK; независимо
|
||||
подтверждено CI `33922485666` (все 3 шарда зелёные).
|
||||
|
||||
## Унаследовано из r1 (через r2, без повторной проверки)
|
||||
|
||||
Эта дельта не касается доказательной базы следующих пунктов r1/r2 — принимаю как есть:
|
||||
|
||||
- AC1–AC2, AC7 (разделение intake/visual invalidation, dependency projection, last-wins HA во
|
||||
время жеста) — не тронуты дельтой r2→r3 (файлы фильтра `hass`/dependency classifier в диффе
|
||||
`cb68492c..444562e4` отсутствуют). Документ: `CODE-REVIEW-451-r2.md`, SHA `cb68492c`.
|
||||
- AC4 (diagnostics cache) — не тронут этой дельтой.
|
||||
- AC9 (golden/canonical screenshots) — r2 подтвердил 153/153 без диффов на `cb68492c`; резерв
|
||||
CI на `444562e4` («Переиспользование: это дерево уже проверено» → успех, «Golden-кадры» → skipped)
|
||||
подтверждает, что источник для golden не менялся с последнего полного прогона. Отдельно не
|
||||
перепрогонял: дельта r2→r3 не меняет settled-состояние (см. ниже про resize-preflight — эффект
|
||||
только во время активного жеста, до golden-снимка дело не доходит).
|
||||
- Инварианты модели по всем моделям проекта — часть `npm test` (сам прогнал на HEAD, зелёный);
|
||||
конфиг-специфичная команда `npm run invariants -- --config …` не нужна отдельно: дельта не меняет
|
||||
персистентную форму (`walls[]`, `wall_segments`, `marker.space`, `open_spans`) — только момент
|
||||
вызова validation-функции и типизацию сигнатур.
|
||||
- `INITIAL_VIEW_GZIP_CEILING` 297000→298000 — обоснование и бюджет не тронуты этой дельтой (только
|
||||
`bundle-budget.mjs` логика из r1, файл не в диффе r2→r3).
|
||||
|
||||
## Находки
|
||||
|
||||
### M1 — не заявленный в ТЗ постоянный отказ от live-preflight на больших планах, противоречит собственному changelog (в скоупе, чинится в этой же задаче)
|
||||
|
||||
**Файлы:** `src/resize-controller.ts:18-27` (`resizeLivePreflightAllowed`),
|
||||
`src/houseplan-editor-runtime.ts:3653` (использование в `_rszProjectPreview`).
|
||||
|
||||
Фикс H3 (`4b6b4abc`) не просто вернул прежнее поведение — он расколол его по размеру плана.
|
||||
Для «bounded» контуров (суммарно ≤ 64 вершин комнат текущего пространства) во время resize-жеста
|
||||
работает тот же точный `_checkSpacePhysicalGeometry()`, что был до #451: невозможная геометрия сразу
|
||||
отклоняется, тост «последняя безопасная позиция» показывается посреди жеста. Для «больших» планов
|
||||
(> 64 вершин) эта проверка **безусловно выключена во время жеста** — живёт только дешёвая
|
||||
junction-limit проверка (углы/валентность), а точная геометрия проверяется один раз, на `pointerup`
|
||||
(это безопасно для данных — commit всё ещё fail-closed, я прочитал `_commitPhysicalGeometry`
|
||||
построчно, — но не для обратной связи пользователю).
|
||||
|
||||
Порог не абстрактный: фикстура `demo/fixtures/large-house.mjs`, на которой считается сам бюджет
|
||||
`large-house-interaction-v1` (тот самый профиль, ради которого выключалась проверка), — это 20
|
||||
прямоугольных комнат по 4 вершины = **80 вершин**, то есть уже выше порога 64. А персона из ТЗ
|
||||
(«администратор дома... средний или большой план», issue: «5 пространств... 8 комнат в текущем
|
||||
пространстве... средний по размеру план, не рекорд») — это ровно тот случай, для которого порог
|
||||
скорее всего будет превышен, если комнаты не прямоугольные (8 комнат × 8 вершин = 64, граница
|
||||
ровно на пороге).
|
||||
|
||||
Почему это находка, а не техническая деталь реализации:
|
||||
|
||||
1. **ТЗ прямо запрещает это как область изменения.** §5 Non-scope: «изменение hit areas, snap
|
||||
tolerance, grid, gesture thresholds, animation duration/easing, click/double-click/long-press
|
||||
или commit/cancel semantics» не входит в задачу. §2: «Преднамеренных визуальных изменений нет.
|
||||
После завершения любого жеста канонический итог и итоговый кадр совпадают с текущим поведением»
|
||||
— про итоговый кадр это верно (commit не изменился), но живая обратная связь **во время** жеста
|
||||
для больших планов теперь другая, и это не техническая деталь — это то, что видит пользователь.
|
||||
Ни §16 (риски), ни §18 (что можно менять свободно без владельца) не упоминают эту развилку.
|
||||
2. **Прямо противоречит changelog, который уже в ветке.** `docs/CHANGELOG.md`/`.ru.md` (коммит
|
||||
`c0d61ca3`, до r1) заявляют: «interactions remain visually and functionally unchanged» /
|
||||
«внешний вид и результат действий не меняются». Для планов с > 64 вершин комнат в текущем
|
||||
пространстве это не так: раньше resize посреди жеста предупреждал о невозможной позиции, теперь —
|
||||
нет.
|
||||
3. **Ни в одном комментарии issue владелец этот компромисс не видел и не утверждал** — я прочитал
|
||||
всю переписку (`Аналитика`, `Вопросы для ТЗ`, `Решения владельца`, оба «Исправления по
|
||||
CODE-REVIEW»): обсуждались Q1 (relevant HA update во время жеста) и Q2 (обычный hover), но не
|
||||
порог 64 и не деление resize-preflight по размеру плана.
|
||||
|
||||
Это Medium, не High: данные не портятся ни при каком сценарии (проверил код коммита, unconditional
|
||||
fail-closed check на pointerup), поэтому это не блокирующий баг, а нераскрытое изменение поведения
|
||||
вне заявленного скоупа задачи — ровно формулировка PROCESS.md «жёлтый вердикт допустим при
|
||||
выполненных AC, если изменение ухудшает смежный сценарий». Находка в скоупе (введена этой же веткой
|
||||
ради выполнения AC10) — чинится в этой же задаче, отдельный issue не заводится.
|
||||
|
||||
**Что нужно от автора (не мой выбор, а перечисление опций для очередного цикла):** либо восстановить
|
||||
живую точную проверку и для больших планов (тогда решать заново конфликт с AC10-бюджетом, из-за
|
||||
которого её и убрали в r1), либо получить у владельца явное решение принять этот компромисс и
|
||||
одновременно поправить формулировку changelog («без функциональных изменений» перестаёт быть верным
|
||||
для больших планов), либо найти более дешёвый способ живой проверки (например, только у затронутой
|
||||
комнаты вместо канонического union всего пространства — H1-r1 уже показал, что именно full clearance
|
||||
был дорогим, а не сам факт проверки).
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- H1/H2/H3 из r2 закрыты — таблица выше, каждая проверена самостоятельно прогоном, а не с чужих слов.
|
||||
- Регресс DOM `resize_pointer.unrelated_pointer_ignored`/`capture_loss_restores_dom` (пойман CI на
|
||||
`a7bee0f5`, не был отдельной находкой r2) закрыт `046efe96`, подтверждено прогоном и CI.
|
||||
- Commit-time geometry check (`_commitPhysicalGeometry` → `_checkSpacePhysicalGeometry`) остаётся
|
||||
безусловным независимо от размера плана — прочитано построчно, не зависит от M1.
|
||||
- Типизация (`fe03eae2`) семантически эквивалентна прежнему `any`-коду — прочитал diff
|
||||
`junction-limits.ts` построчно (переход через промежуточный `completeGeometry` даёт тот же результат
|
||||
для 'ok'/'degraded-extra'/'lightweight'/`null`/`undefined`, что и старое тройное сравнение).
|
||||
`live-editor.ts` получил один дополнительный guard (`!!room.id &&`) — сужение, не расширение
|
||||
поведения, риска регрессии нет.
|
||||
- Новый unit-тест `resize-controller.test.mjs` («#451 live resize preflight is bounded by authored
|
||||
contour complexity») проверяет именно границу (16×4=64 → true, 17×4=68 → false, один контур 65 → false)
|
||||
— тест способен падать (проверил через чтение реализации: убери `<=` на `<`, и 64 упадёт).
|
||||
- `no-new-any`, `check-docs`, `npm test`, `npm run build`+`bundle:sync` (сверка бандла) — зелёные
|
||||
на точном HEAD, прогнано лично.
|
||||
- Трейлеры `Issue: #451` — присутствуют во всех коммитах дельты. `User-Visible: no` для `046efe96`
|
||||
и `444562e4` — точен (внутренний DOM-баг и обновление отпечатка, не новое поведение). Для `4b6b4abc`
|
||||
трейлер тоже `User-Visible: no` — и вот это неточно ровно по причине M1.
|
||||
|
||||
## Чего не проверял и почему
|
||||
|
||||
- Полный `npm run golden:verify` — не гонял отдельно: дельта не меняет settled-состояние (эффект
|
||||
M1 виден только во время активного resize-жеста, до commit/settled-кадра), а CI на HEAD показал
|
||||
«Переиспользование: это дерево уже проверено» → golden job skipped легитимно (тот же tree-hash,
|
||||
что уже проверялся 153/153 в r2).
|
||||
- Полный `npm run benchmark:large-house-interaction` — не перегонял: диапазон дельты не меняет
|
||||
веса editor-series (единственное затронутое ответвление — resize live-preflight, а он теперь
|
||||
для фикстуры large-house **выключен** тем же порогом 64 < 80, то есть числа бюджета из r2
|
||||
(`editor median 484.7 ms ≤ 750`) по построению не изменятся от этой дельты — сам факт этого и есть
|
||||
часть находки M1: бюджет проходит именно потому, что фикстура выше порога).
|
||||
- Оставшиеся 18 «прямых совпадений» smoke-select (декор/opening-смоки на `_openingsR`/`NORM_W`) —
|
||||
дельта в этих файлах чисто типовая (import type вместо value import, сигнатуры), не гонял отдельно;
|
||||
25 «слабых» совпадений на `_mode` не гонял — типовой рефакторинг чужого для них кода.
|
||||
- `python -m pytest tests_backend` — диф не трогает `custom_components/**/*.py` (проверено чтением
|
||||
`git diff --name-only`).
|
||||
- `npm run invariants -- --config <...>` для конкретного конфига — не требуется отдельно (см.
|
||||
«Унаследовано», дельта не меняет персистентную геометрическую форму).
|
||||
- Мутация для `resizeLivePreflightAllowed` — не заводил: функция чистая, покрыта unit-тестом на все
|
||||
три граничных случая, и мутационный тест не добавил бы нового при уже красном-способном юните.
|
||||
|
||||
## Итог
|
||||
|
||||
**Вердикт: жёлтый.** High: 0, Medium: 1 (M1, в скоупе — возвращается автору, отдельный issue не
|
||||
заводится). H1–H3 из r2 закрыты корректно, дополнительный DOM-регресс закрыт корректно. Единственная
|
||||
находка этого раунда — не баг в данных, а нераскрытое и не согласованное с ТЗ/changelog изменение
|
||||
поведения resize-preflight для больших планов, появившееся как побочный эффект закрытия H3.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/451-render-performance`, коммит `444562e47cb7` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `9489e0f2e24a6b4df6bd587703b2fa058ebb3a04`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 9489e0f2e24a
|
||||
```
|
||||
- ТЗ `docs/specs/451-render-performance.md`, блоб `7c323a29110974aae369077214b9e2a74d9387c1`
|
||||
```
|
||||
git log --all --find-object=7c323a29110974aae369077214b9e2a74d9387c1 -- docs/specs/451-render-performance.md
|
||||
```
|
||||
@@ -0,0 +1,284 @@
|
||||
# CODE-REVIEW #451 — заход r4
|
||||
|
||||
- **Issue:** #451 — «План тормозит: диагностика считается на каждый кадр, перетаскивание перерисовывает всё, нет фильтра обновлений»
|
||||
- **Этап:** код-ревью (PROCESS.md §2.7)
|
||||
- **Заход:** r4 · блокирующих циклов до этого раунда: 3/4
|
||||
- **Материал раунда:** ветка `issue/451-render-performance`, `HEAD = 07ba2ffbd1c02172d114ef27fb7cee2e40cf326e`
|
||||
(сверено `git rev-parse HEAD` непосредственно перед выводом, дерево чистое, PROCESS.md §2.7/#312)
|
||||
- **Дельта раунда:** `git diff 444562e47cb746dc1c7d740b66b2f832ca02f064..07ba2ffb` — SHA взят из машинного блока
|
||||
«Материал раунда» документа `docs/reviews/CODE-REVIEW-451-r3.md` (дерево `9489e0f2e24a6…`, сверено
|
||||
`git log --all --format='%H %T' | grep 9489e0f2e24a` — совпадает с `444562e4`). Файлы дельты
|
||||
(без `dist/**` и копий бандла стенда): `src/houseplan-card.ts`, `src/houseplan-editor-runtime.ts`,
|
||||
`src/resize-controller.ts`, новый `src/resize-live-preflight.ts`, `test/resize-controller.test.mjs`,
|
||||
`demo/benchmark_large_house.mjs`, `demo/performance/card-contract.mjs`, `docs/images/screenshots.json`,
|
||||
`docs/reviews/CODE-REVIEW-451-r3.md`.
|
||||
|
||||
## Скоуп раунда
|
||||
|
||||
r3 (жёлтый, High:0/Medium:1) закончился на `444562e4`. Единственная находка r3 — **M1**: фикс H3 из r2
|
||||
не просто вернул прежнее поведение, а ввёл порог `resizeLivePreflightAllowed(rooms, edgeBudget=64)` —
|
||||
точная physical-geometry проверка во время resize-жеста была живой только для планов ≤ 64 вершин
|
||||
комнат, для больших планов работала только на `pointerup`. Не заявлено в ТЗ, противоречило changelog
|
||||
«внешний вид и результат действий не меняются».
|
||||
|
||||
Автор ответил одним коммитом `5692a288` («fix: preserve live resize validation on large plans»,
|
||||
`User-Visible: no`): порог полностью удалён, вместо него — новый модуль `resize-live-preflight.ts` с
|
||||
тремя функциями (`resizeLiveRoomIds`, `resizeLiveJunctionRoomIds`, `resizeLiveCandidateSpace`), которые
|
||||
строят «локальный кандидат» — только затронутые комнаты плюс геометрически близкие к их границе
|
||||
стены/сегменты/партиции/проёмы — и гоняют через него ту же самую `_checkSpacePhysicalGeometry`,
|
||||
безусловно, на любом размере плана. `07ba2ffb` поверх — только обновление отпечатка скриншотов
|
||||
документации (`docs: refresh screenshot fingerprint for #451`).
|
||||
|
||||
Разбираю всю дельту по коду: это новый файл на 210 строк с нетривиальной геометрией (distance-to-segment,
|
||||
segment-intersection, AABB-touch), напрямую заменяющий защитный механизм, который весь путь r2→r3→r4 и
|
||||
был предметом ревью.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Дельта не трогает персистентный формат — только момент и объём вызова уже существующей проверки.
|
||||
Валидация на точном HEAD (`07ba2ffb`) уже зелёная в CI: [Validate run 33927104551](https://github.com/Matysh/houseplan-card/actions/runs/33927104551),
|
||||
success. Это покрывает `npx tsc --noEmit`, `npm test`, `npm run build`+сверку бандла и `check-docs`
|
||||
(docs job зелёный на этом SHA) — **не перегонял эти четыре повторно**, см. правило дешёвых гейтов §8/§2.10.
|
||||
|
||||
Что прогнал сам в этом раунде:
|
||||
|
||||
| Команда | Результат |
|
||||
|---|---|
|
||||
| `npx tsc -p tsconfig.test.json && node scripts/fix-test-build.mjs` | зелёный (нужно для собственных проб ниже) |
|
||||
| `node scripts/no-new-any.mjs --base 444562e4 --head HEAD` | «Новых any нет» (225 строк в 3 файлах) |
|
||||
| `npm run build && npm run bundle:sync` | зелёные, дерево бандла пересобрано без ошибок |
|
||||
| `node scripts/smoke-select.mjs --base 444562e4 --head HEAD` | 19 прямых совпадений + 2 зарегистрированные связи, все по теме wall-union/junction/masonry/resize (список ниже) |
|
||||
| `node demo/smoke_room_resize.mjs` | OK — реальные (не замоканные) сценарии `owner_boundary_clamped`, `corner_clamped`, `mixed_role_*` не задеты |
|
||||
| `node demo/smoke_resize_pointer_real_plan.mjs` | OK |
|
||||
| `node demo/smoke_junction_holes.mjs` | OK |
|
||||
| `node demo/smoke_glow_fail_dark.mjs` | OK |
|
||||
| `node demo/smoke_glow.mjs` | OK |
|
||||
| `node demo/smoke_junction_patch_resilience.mjs` | OK |
|
||||
| `node demo/smoke_multiwall_junction.mjs` | OK |
|
||||
| `node demo/smoke_multiwall_strip_containment.mjs` | OK |
|
||||
| `node demo/smoke_opening_measure.mjs` | OK |
|
||||
| `node demo/smoke_optional_space_model.mjs` | OK |
|
||||
| `node demo/smoke_wall_key_roundtrip.mjs` | OK |
|
||||
| `node demo/smoke_wall_thickness_transition.mjs` | OK |
|
||||
| `node demo/smoke_wall_union_isolation.mjs` | OK (включая генуинный, не замоканный `degradedPhysicalEditRejected` на **commit**-пути) |
|
||||
| `node demo/smoke_zero_divider_taper.mjs` | OK |
|
||||
| `node demo/smoke_real_plan_masonry.mjs` | OK («зарегистрированная связь» smoke-select — реальный план, разрывы кладки, которые синтетика не ловит) |
|
||||
| `node demo/smoke_resize_wall_thickness.mjs` | OK («зарегистрированная связь») |
|
||||
|
||||
Плюс собственная проба (не входит в существующий гейт, привожу как воспроизведение находки ниже):
|
||||
прямой вызов `checkSpacePhysicalGeometry`/`resizeLiveCandidateSpace`/`resizeLiveJunctionRoomIds` из
|
||||
`test-build/*.js` на реальном производственном фикстуре регрессии `test/fixtures/278-wall-union-isolation.json`
|
||||
(#278) плюс синтетические «дальние» комнаты для имитации большого плана.
|
||||
|
||||
## Закрытие r3
|
||||
|
||||
| Находка r3 | Чем закрыта | Где видно |
|
||||
|---|---|---|
|
||||
| **M1** (порог `edgeBudget=64`: точная live-проверка выключена целиком для планов > 64 вершин комнат, не заявлено в ТЗ/changelog) | Порог `resizeLivePreflightAllowed` удалён вовсе (`src/resize-controller.ts`, было 13 строк функции — теперь нет). Вместо него `resizeLiveCandidateSpace`/`resizeLiveRoomIds` (`src/resize-live-preflight.ts`) строят локальный физический кандидат и `_rszProjectPreview` гоняет `_checkSpacePhysicalGeometry` через него **безусловно**, на любом размере плана — деление по порогу исчезло как класс | `5692a2880f9ba645b9e2a897a828a9ea169a3a1a` (`src/houseplan-editor-runtime.ts:3654-3656`); сам прочитал — вызов больше не обёрнут в `if (resizeLivePreflightAllowed(...))`; `demo/benchmark_large_house.mjs` теперь структурно требует `resizeLivePreflightChecks >= 1` на `large-house` фикстуре (80 вершин, выше старого порога) — не даёт тихо вернуть скип |
|
||||
|
||||
Формально M1 закрыта в буквальном прочтении (порог убран, вызов безусловный на любом размере). Но
|
||||
дельта заменила один раскрытый компромисс на другой, нераскрытый — см. находку ниже: сам факт «вызов
|
||||
происходит всегда» не означает «проверка обнаруживает то же самое, что обнаруживала до #451».
|
||||
|
||||
## Находки
|
||||
|
||||
### M1 (r4) — локальный кандидат для physical-geometry исключает соседнюю комнату, от которой зависит валидность стыка; живая проверка может пропустить реальный дефект кладки на любом размере плана (в скоупе, чинится в этой же задаче)
|
||||
|
||||
**Файлы:** `src/resize-live-preflight.ts:59-65` (`resizeLiveRoomIds`), `:91-124` (`resizeLiveCandidateSpace`),
|
||||
`src/houseplan-editor-runtime.ts:3654-3656` (`_rszProjectPreview`, использование).
|
||||
|
||||
`resizeLiveCandidateSpace(sp, changedRoomIds)` строит кандидат для `_rszSpaceCandidateGeometry` (→
|
||||
`_checkSpacePhysicalGeometry`), отбирая **только** комнаты, чей id входит в `changedRoomIds`
|
||||
(`resizeLiveRoomIds` — точное совпадение id, без расширения на соседей). Стены/сегменты/партиции
|
||||
отбираются отдельно, по геометрической близости к границе **этих** комнат. Соседняя комната, которая
|
||||
физически не изменилась в этом кадре (её id не в `changedRoomIds`), в кандидат не попадает вовсе — даже
|
||||
если общий с ней узел/стена как раз и есть источник невалидности.
|
||||
|
||||
Это не гипотетическое рассуждение о коде — воспроизвёл прогоном на реальном production-фикстуре
|
||||
регрессии #278 (`test/fixtures/278-wall-union-isolation.json`, две комнаты `r1`/`r2`, общая стена
|
||||
толщиной 20, историческая дефектная кладка, статус `degraded-extra`):
|
||||
|
||||
```js
|
||||
import { checkSpacePhysicalGeometry } from './test-build/plan-geometry-preflight.js';
|
||||
import { resizeLiveCandidateSpace } from './test-build/resize-live-preflight.js';
|
||||
// baseSpace = spaces[0] из test/fixtures/278-wall-union-isolation.json (r1, r2, стена 20см)
|
||||
// + 80 «дальних» синтетических комнат (10..90 по x), чтобы получить план > 64 вершин.
|
||||
|
||||
checkSpacePhysicalGeometry({ spaces: [largeSpace] }, largeSpace.id)
|
||||
// -> { status: 'failed', reason: 'wall-degraded-extra', ok: false } ← ПОЛНОЕ пространство: дефект виден,
|
||||
// независимо от того, сколько в плане посторонних комнат (80 или 0)
|
||||
|
||||
const liveSpace = resizeLiveCandidateSpace(largeSpace, ['r1']); // r1 «изменилась», r2 — нет
|
||||
liveSpace.rooms.map(r => r.id) // -> ['r1'] (r2 выброшена целиком)
|
||||
checkSpacePhysicalGeometry({ spaces: [liveSpace] }, largeSpace.id)
|
||||
// -> { status: 'ok', ok: true } ← ТОТ ЖЕ дефект больше не виден
|
||||
```
|
||||
|
||||
Итог: во время resize-жеста, когда двигается только `r1`, а `r2` (владелец второй стороны той же
|
||||
дефектной стены) не входит в `changedRoomIds`, живая проверка сообщает «геометрия в порядке» для
|
||||
кандидата, который на самом деле нарушает контракт масонри — тот же самый контракт, ради которого
|
||||
существует `_checkSpacePhysicalGeometry` и весь путь #278/H3(r2)/M1(r3). Это воспроизводится **на любом
|
||||
размере плана** — добавление или удаление 80 посторонних комнат ничего не меняет, значит находка не
|
||||
«ещё один порог», а более фундаментальная: набор комнат для physical-geometry кандидата определяется
|
||||
по `changedRoomIds` (точное совпадение id), а не по геометрической смежности.
|
||||
|
||||
Показательно, что для **другого** кандидата в том же методе — `_resizePreviewNodes` (junction-limit
|
||||
проверка, #329) — автор уже использует `resizeLiveJunctionRoomIds` (один слой AABB-соседей), а не голый
|
||||
`changedRoomIds`. Подставил тот же более широкий набор в physical-geometry кандидат — и дефект снова
|
||||
виден:
|
||||
|
||||
```js
|
||||
const junctionIds = resizeLiveJunctionRoomIds(largeSpace.rooms, ['r1']); // -> ['r1', 'r2']
|
||||
const liveSpaceViaJunctionIds = resizeLiveCandidateSpace(largeSpace, junctionIds);
|
||||
checkSpacePhysicalGeometry({ spaces: [liveSpaceViaJunctionIds] }, largeSpace.id)
|
||||
// -> { status: 'failed', reason: 'wall-degraded-extra', ok: false } ← дефект снова обнаружен
|
||||
```
|
||||
|
||||
Почему это Medium, а не техническая деталь:
|
||||
|
||||
1. **Данные не портятся.** `_commitPhysicalGeometry` (не в этой дельте, прочитал — не изменился) вызывает
|
||||
`_checkSpacePhysicalGeometry` на **полном** пространстве безусловно на `pointerup`. Невалидная
|
||||
геометрия всё ещё не может сохраниться.
|
||||
2. **Но живая обратная связь пользователю может отсутствовать именно там, где её восстановление и было
|
||||
предметом M1(r3).** Пользователь дотягивает жест до конца, не видя «последней безопасной позиции», а
|
||||
затем получает отказ commit (`resize.commit_failed`, класс регресса, который уже описан в H3 r2) без
|
||||
предупреждения по пути — то есть худший, а не лучший исход по сравнению с раскрытым порогом r3: тот
|
||||
хотя бы предсказуемо и одинаково выключал проверку выше 64 вершин; этот — непредсказуемо, в
|
||||
зависимости от того, какая именно комната «официально изменилась» в данном кадре resize-солвера,
|
||||
и на любом размере плана, включая маленькие.
|
||||
3. **Ни ТЗ, ни changelog, ни коммит-сообщение** (`fix: preserve live resize validation on large plans`,
|
||||
`User-Visible: no`) не упоминают эту границу — коммит заявляет ровно противоположное тому, что
|
||||
происходит для дефектов, зависящих от соседней комнаты.
|
||||
4. **Существующее покрытие не могло эту находку поймать.** Новые unit-тесты в
|
||||
`test/resize-controller.test.mjs` проверяют только структуру фильтрации (какие id/стены попадают в
|
||||
кандидат), не пропуская результат через `checkSpacePhysicalGeometry`. Новая проверка в
|
||||
`demo/benchmark_large_house.mjs` (`deltas.resizeLivePreflightChecks < 1` роняет раннер) доказывает
|
||||
только, что проверка **вызывается**, а не что она может вернуть `false` для реально невалидного
|
||||
кандидата. `smoke_room_resize.mjs` (единственный реальный смок с геометрией без мока) использует
|
||||
фикстуры, где невалидность — либо однокомнатная топология (`corner_clamped`), либо ровно совпадающая
|
||||
с существующей стеной (`owner_boundary_clamped`, `wall-metadata`-путь, отдельная от physical-geometry
|
||||
проверка, использует полный `sp.rooms` — не задета этой находкой); ни один существующий тест не
|
||||
строит #278-подобный «дефект стыка, видимый только если обе стороны в модели».
|
||||
|
||||
**Что нужно от автора (не мой выбор, перечисление опций для следующего цикла):** либо расширить набор
|
||||
комнат physical-geometry кандидата тем же способом, что уже используется для junction-кандидата
|
||||
(`resizeLiveJunctionRoomIds`, один слой AABB-соседей — проба выше показывает, что этого достаточно для
|
||||
воспроизведённого случая; открытый вопрос — достаточно ли одного слоя для более сложных многосторонних
|
||||
узлов, это стоит явного теста), либо обосновать и явно задокументировать в ТЗ/коде, почему набора
|
||||
`changedRoomIds` достаточно для physical-geometry (если у меня неверна модель угрозы), и в любом случае
|
||||
добавить тест, который прогоняет `resizeLiveCandidateSpace`-кандидат **через** `checkSpacePhysicalGeometry`
|
||||
на заведомо дефектной (не замоканной) геометрии — по образцу пробы выше, лучше всего на самом фикстуре
|
||||
#278, — чтобы AC «восстановлена live-валидация» имело названного свидетеля, который умеет краснеть.
|
||||
|
||||
## Унаследовано из r3 (и через r3 из r1/r2), без повторной проверки
|
||||
|
||||
Эта дельта не касается доказательной базы следующих пунктов — принимаю как есть:
|
||||
|
||||
- H1–H3 из r2 (docs fingerprint, новые `any`, регресс `smoke_room_resize`) и дополнительный DOM-регресс
|
||||
(`resize_pointer.unrelated_pointer_ignored`/`capture_loss_restores_dom`) — файлы их фиксов
|
||||
(`houseplan-card.ts` типизация junction-limits, `live-editor.ts`, `046efe96` DOM-guard) не входят в
|
||||
дельту `444562e4..HEAD`. Документ: `CODE-REVIEW-451-r2.md`/`-r3.md`, SHA `cb68492c`/`444562e4`.
|
||||
- AC1–AC2, AC7 (разделение intake/визуальной инвалидизации, dependency projection, last-wins HA во время
|
||||
жеста) — файлы фильтра `hass`/dependency classifier не в дельте `444562e4..HEAD`. Документ:
|
||||
`CODE-REVIEW-451-r2.md`, SHA `cb68492c`.
|
||||
- AC4 (diagnostics cache) — не тронут этой дельтой.
|
||||
- AC9 (golden/canonical screenshots) — единственный тронутый в этой дельте артефакт —
|
||||
`docs/images/screenshots.json` (отпечаток источника, коммит `07ba2ffb`); Linux-съёмка на предыдущем
|
||||
SHA (`444562e4`) была канонической (r3: [run 33926924502]) и её результат (10/10 без диффов) не
|
||||
меняется этой дельтой — сама дельта не трогает `src/**` рендер-путь, только момент вызова физической
|
||||
проверки во время resize, до commit/settled-кадра. Отдельно не перепрогонял golden — не требуется:
|
||||
результирующий кадр после `pointerup` идентичен (полная проверка на commit не изменилась).
|
||||
- Инварианты модели по всем моделям проекта — часть `npm test` (зелёный на HEAD в CI); конфиг-специфичная
|
||||
команда `npm run invariants -- --config …` не нужна отдельно: дельта не меняет персистентную форму
|
||||
(`walls[]`, `wall_segments`, `marker.space`, `open_spans`) — только состав кандидата, временно
|
||||
собираемого в памяти для live-проверки во время жеста, никогда не записываемого в конфиг.
|
||||
- `INITIAL_VIEW_GZIP_CEILING` 297000→298000 и его обоснование — не тронуты этой дельтой.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Порог `resizeLivePreflightAllowed`/`edgeBudget=64` из r3 действительно удалён целиком — прочитал diff
|
||||
`resize-controller.ts`, функции больше нет; `_rszProjectPreview` больше не содержит ветвления по
|
||||
размеру плана для physical-geometry вызова.
|
||||
- 17 из 21 отмеченных `smoke-select` тестов (19 прямых + 2 зарегистрированных за вычетом 6 слабых
|
||||
`cellCm`-совпадений, см. «Чего не проверял») прогнаны лично, все зелёные — включая генуинный (не
|
||||
замоканный) commit-time `degradedPhysicalEditRejected` в `smoke_wall_union_isolation.mjs` и реальный
|
||||
план в `smoke_real_plan_masonry.mjs`.
|
||||
- Реальные (не замоканные) мелкоплановые сценарии `smoke_room_resize.mjs` (`owner_boundary_clamped`,
|
||||
`corner_clamped`, `mixed_role_*`) не регрессировали при переходе на безусловный локальный кандидат —
|
||||
прогнал, зелёные.
|
||||
- `_commitPhysicalGeometry` (полное пространство, безусловный вызов на `pointerup`) не тронут этой
|
||||
дельтой — прочитал, данные при коммите остаются fail-closed независимо от находки M1(r4): невалидная
|
||||
геометрия не может сохраниться, регресс только в live-обратной связи посреди жеста.
|
||||
- Новые unit-тесты `resize-controller.test.mjs` корректно проверяют то, что они заявляют проверять
|
||||
(структура фильтрации id/стен/партиций/проёмов на 100-комнатном синтетическом плане) — прочитал, тест
|
||||
умеет падать на этом узком контракте (например, `resizeLiveRoomIds` вернёт лишний id, если убрать
|
||||
фильтр по `changed`). Находка M1(r4) не в том, что эти тесты неверны, а в том, что они не покрывают
|
||||
промежуточный конечный результат (`checkSpacePhysicalGeometry` на построенном кандидате).
|
||||
- `no-new-any --base 444562e4 --head HEAD` — 0 новых `any` в 225 добавленных строках 3 файлов.
|
||||
- Новая инструментация `demo/benchmark_large_house.mjs` (`physicalPreflightCount`/`physicalPreflightMs`,
|
||||
ассерт `resizeLivePreflightChecks < 1`) корректно доказывает то немногое, что доказывает: вызов
|
||||
происходит хотя бы раз во время editor-резайза на large-house фикстуре — не более.
|
||||
- Трейлеры `Issue: #451` присутствуют, `User-Visible: no` для `5692a288` и `07ba2ffb` — точны для
|
||||
заявленного эффекта (реализация не меняет видимое поведение, когда проверка срабатывает), но не
|
||||
раскрывают найденную M1(r4) границу, где она не срабатывает.
|
||||
|
||||
## Чего не проверял и почему
|
||||
|
||||
- Полный `npm run golden:verify`, `npx tsc --noEmit`, `npm test`, `npm run build`+сверка бандла отдельно
|
||||
от CI — не перегонял: Validate зелёный на точном HEAD `07ba2ffb`
|
||||
([run 33927104551](https://github.com/Matysh/houseplan-card/actions/runs/33927104551)), дешёвые гейты
|
||||
§8 сошлись на этом прогоне.
|
||||
- 6 слабых `smoke-select`-совпадений по единственному общему идентификатору `cellCm`
|
||||
(`smoke_backdrop_guard`, `smoke_danger_confirmation`, `smoke_decor`, `smoke_grid_scale_invariance`,
|
||||
`smoke_help_affordance`, `smoke_space_scale_defaults`) — не гонял: `cellCm` — параметр с fallback по
|
||||
умолчанию в новом файле, общий для всей кодовой базы идентификатор без содержательной связи с темой
|
||||
этих смоков (ни один не про resize/physical-geometry); риск по существу покрыт целевыми
|
||||
wall-union/junction/resize смоками выше.
|
||||
- Полный `npm run benchmark:large-house-interaction` — не перегонял сам; автор привёл 7 прогонов с
|
||||
зелёными абсолютными бюджетами на этом SHA. Дельта этого раунда не меняет веса editor-series (то же
|
||||
число вызовов physical-geometry на move, что и раньше, только на другом наборе комнат) — по построению
|
||||
не должна была измениться, и находка M1(r4) не является перформанс-регрессией (наоборот: локальный
|
||||
кандидат обычно дешевле полного пространства).
|
||||
- `python -m pytest tests_backend` — диф не трогает `custom_components/**/*.py` (проверено
|
||||
`git diff --name-only 444562e4..HEAD`).
|
||||
- Мутация для `resizeLiveCandidateSpace`/`resizeLiveRoomIds` через `scripts/mutation-gate.mjs` — не
|
||||
заводил (это не моя роль); вместо этого привёл воспроизводимую пробу через прямой вызов
|
||||
скомпилированных пары чистых функций на production-фикстуре #278 — она и есть демонстрация «чем
|
||||
краснеет» для находки M1(r4). Постоянный мутант в гейте — тоже часть того, что нужно от автора при
|
||||
закрытии находки.
|
||||
- Полная browser-smoke матрица (222 файла) — не прогонял; это предрелизный гейт (PROCESS.md §8), дельта
|
||||
локальна (4 файла кода) и `smoke-select` по точному диапазону раунда покрыл релевантную тему.
|
||||
|
||||
## Итог
|
||||
|
||||
**Вердикт: жёлтый.** High: 0, Medium: 1 (M1(r4), в скоупе — возвращается автору, отдельный issue не
|
||||
заводится). Порог из M1(r3) действительно удалён, но замена (`resizeLiveCandidateSpace` с фильтрацией
|
||||
комнат по точному `changedRoomIds`) вводит новый, более скрытый пробел того же класса: воспроизведён
|
||||
прогоном на реальном production-фикстуре #278, что живая physical-geometry проверка может вернуть `ok`
|
||||
для кандидата, эквивалентного заведомо дефектной (`wall-degraded-extra`) полной геометрии, если
|
||||
источник дефекта — стык с соседней, формально «неизменившейся» комнатой. Данные не портятся
|
||||
(`_commitPhysicalGeometry` на `pointerup` не изменился и остаётся безусловным), поэтому находка не
|
||||
блокирует как High, но AC «restore live resize validation» в исходном смысле (было в r1/r2 до #451
|
||||
сломавших это) не является полностью восстановленным для этого класса дефектов ни на одном размере
|
||||
плана — направление минимального фикса (переиспользовать уже существующий `resizeLiveJunctionRoomIds`)
|
||||
подтверждено той же пробой.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: заполняется конвейером публикации -->
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/451-render-performance`, коммит `07ba2ffbd1c0` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `c2def02d1a1409564ed4eaa3661b663cbba356c1`
|
||||
```
|
||||
git log --all --format='%H %T' | grep c2def02d1a14
|
||||
```
|
||||
- ТЗ `docs/specs/451-render-performance.md`, блоб `7c323a29110974aae369077214b9e2a74d9387c1`
|
||||
```
|
||||
git log --all --find-object=7c323a29110974aae369077214b9e2a74d9387c1 -- docs/specs/451-render-performance.md
|
||||
```
|
||||
@@ -0,0 +1,226 @@
|
||||
# SPEC-REVIEW-451-r1
|
||||
|
||||
- **Issue:** https://github.com/Matysh/houseplan-card/issues/451
|
||||
- **Этап:** spec (PROCESS.md §2.4)
|
||||
- **Заход:** r1 (первое ревью ТЗ, лёгкий трек не применяется — S2-analysis назвал
|
||||
четыре нарушенных критерия §5: риск 9/10, больше одной поверхности, влияние
|
||||
на performance, влияние на touch)
|
||||
- **Материал:** `docs/specs/451-render-performance.md`, коммит `71c3360363991a042fbf3d84a6aeb4ecb772c83f`
|
||||
(`git rev-parse HEAD` на момент вывода), ветка `issue/451-render-performance`
|
||||
(HEAD detached at `origin/issue/451-render-performance`)
|
||||
- **Вердикт:** жёлтый · заход r1 · блокирующих циклов 1/4 · High: 0 · Medium: 3 → в задаче
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Проверялся только сам файл ТЗ и тело/комментарии issue #451 — продуктового кода
|
||||
для #451 ещё нет (коммит `71c33603` содержит только `docs/specs/451-render-performance.md`
|
||||
и запись в `docs/specs/README.md`). Ревью состязательное: разбор велся без
|
||||
устных пояснений автора, по тексту ТЗ и по текущему `dev`.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` целиком (включая §2.4,
|
||||
§2.10, §4, §5, §7.1, §7.2, §8).
|
||||
2. Прочитано тело issue #451 и все комментарии (замеры, причины A/B/C/D,
|
||||
аналитика, вопросы Q1/Q2 и решения владельца).
|
||||
3. Прочитан ТЗ-документ целиком (`docs/specs/451-render-performance.md`, 516 строк).
|
||||
4. Факты, заявленные в §3 ТЗ как установленные причины, сверены с текущим
|
||||
`dev` построчно:
|
||||
- `_renderBody()` безусловно вызывает `houseplanDiagnostics()` —
|
||||
`src/houseplan-card.ts:11315`, подтверждено;
|
||||
- обход всех живых bindings в `houseplanDiagnostics()`/`_bindingStatus()` —
|
||||
`src/houseplan-card.ts:4748-4771`, подтверждено;
|
||||
- три `data-*` атрибута читают один и тот же объект `diagnostics`,
|
||||
вычисленный один раз на рендер (строки `11349-11351` и `11430-11432`
|
||||
используют одну и ту же переменную `diagnostics`) — «одно число, один
|
||||
источник» соблюдено уже в текущем коде и не нарушается контрактом ТЗ;
|
||||
- `_view`/`_zoom` зарегистрированы `state: true` — `src/houseplan-card.ts:2557-2558`,
|
||||
подтверждено;
|
||||
- `shouldUpdate()` в файле отсутствует (есть только `willUpdate()`) —
|
||||
подтверждено;
|
||||
- `createRenderDeviceSnapshot()` существует и используется — `src/houseplan-card.ts:4687`,
|
||||
подтверждено.
|
||||
5. Сверены абсолютные потолки, упомянутые в §11.3 ТЗ, с `demo/performance/budgets.json`:
|
||||
`panZoomMs.hardMaxMs = 500`, `stateUpdateMs.hardMaxMs = 1000` — совпадает
|
||||
с тем, что ТЗ называет уже существующим.
|
||||
6. Проверено существование каждого упомянутого в ТЗ файла-адресата документации:
|
||||
`docs/PERFORMANCE.md`, `docs/SCREENSHOTS.md`, `demo/performance/README.md`,
|
||||
`demo/benchmark_large_house.mjs`, `demo/performance/card-contract.mjs` —
|
||||
через `find`/`ls` по репозиторию.
|
||||
7. Проверены связанные issue (#34, #82, #137, #156, #380, #396, #449) через
|
||||
`gh issue view` — все существуют и релевантны тому, для чего их приводит ТЗ.
|
||||
8. Проверен термин `POINTER_HOVER_QUERY`/«fine hover capability» из §9 ТЗ —
|
||||
совпадает с реальной константой `src/pointer-modality.ts:5`.
|
||||
9. Численные данные §11.3 («наблюдавшихся 3 432–5 538 мс») сверены с телом
|
||||
issue и всеми его комментариями (`gh issue view --json body,comments`) —
|
||||
`grep` на «5538»/«5 538» не дал ни одного совпадения ни в issue, ни где-либо
|
||||
в репозитории вне самого нового ТЗ.
|
||||
|
||||
Гейты (`typecheck`/`test`/`build`/смоки/perf) не прогонялись и не должны были:
|
||||
на этом этапе нет продуктового кода, диапазон изменений — только `docs/specs/**`.
|
||||
Это осознанное решение по этапу, а не пропуск.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Обязательные разделы §7.1 присутствуют и в правильном порядке: сценарий (§1),
|
||||
что человек увидит до/после (§2), проблема (§3), скоуп/не-скоуп (§4-5),
|
||||
контракт поведения (§6-8), UX/touch/a11y (§9), модель данных/совместимость/
|
||||
безопасность (§10), performance (§11), AC1-AC13 с методом доказательства
|
||||
(§12), план автотестов (§13), план реализации (§14), release-артефакты (§15),
|
||||
риски (§16), откат (§17), явный блок «принято предположительно» (§18).
|
||||
- Продуктовые решения владельца (Q1 — deferred-last-wins после `pointerup`/
|
||||
`pointercancel`; Q2 — обычный hover входит в задачу через lightweight-слой)
|
||||
корректно перенесены в контракт (§6.3, §8.3) и не переоткрываются как
|
||||
вопросы — ТЗ их фиксирует как решённые, а не как предположения автора.
|
||||
- Диагноз причин A/B/C в §3 не является догадкой: каждое утверждение проверяется
|
||||
чтением текущего кода (см. «Как проверялось», п.4) и совпадает с ним дословно.
|
||||
- Собственное наблюдение автора из комментария issue («редакторские жесты не
|
||||
замерялись, синтетический `pointermove` не доходит до обработчиков») корректно
|
||||
закрыто требованием в плане тестов: «Synthetic events обязаны доходить до
|
||||
production handlers; `0` событий у handler не принимается как доказательство
|
||||
`0` renders» (§13.2). Это именно то усиление, которого просил сам owner-комментарий
|
||||
(«жест не перерисовывает содержимое» проверяется на демо-стенде, где портить
|
||||
нечего).
|
||||
- Non-scope (§5) точно очерчивает границу: внешний вид, hit area, snap
|
||||
tolerance, geometry-алгоритмы, persisted config/schema, ослабление бюджетов —
|
||||
всё явно исключено, что снижает риск скрытого расширения скоупа при
|
||||
реализации сложной (9/10) задачи.
|
||||
- AC1-AC13 пронумерованы, у каждого указан метод доказательства
|
||||
(`unit`/`smoke`/`golden`/`performance`/«код-ревью»/«review»), формулировки
|
||||
проверяемы по структурным assertions §11.2 (счётчики full render, а не
|
||||
субъективные «стало быстрее»).
|
||||
- «Один жест — один результат» выдержан симметрично для всех трёх редакторов
|
||||
через таблицу §8.4, без привилегирования Плана перед Устройствами/Подложкой.
|
||||
- Откат (§17) реалистичен: revert коммитов, точечный аварийный fallback на
|
||||
один жест без отключения diagnostics cache/HA filter, с обязательным issue —
|
||||
это не пустая формальность, а конкретный механизм.
|
||||
- Ссылки на связанные issue (#34, #82, #137, #156, #380, #396, #449) все
|
||||
существуют и действительно относятся к тем контрактам, которые ТЗ обязуется
|
||||
не сломать (проверено `gh issue view`).
|
||||
|
||||
## Находки
|
||||
|
||||
### M1 — `docs/PERFORMANCE.md` не существует, но назван обязательным артефактом (Medium, в скоупе)
|
||||
|
||||
**Файл:** `docs/specs/451-render-performance.md`, строки 13, 390-391 (AC13), 452
|
||||
(план реализации, п.7), 469 (release-артефакты).
|
||||
|
||||
**Проблема:** ТЗ трижды в разных разделах требует обновить `docs/PERFORMANCE.md`
|
||||
как уже существующий канонический документ performance harness («**Связано:**
|
||||
… `docs/PERFORMANCE.md`»; AC13 «`docs/PERFORMANCE.md` и `demo/performance/README.md`
|
||||
описывают новый профиль…»; §15 «Также обновляются: `docs/PERFORMANCE.md`»).
|
||||
Такого файла в репозитории нет ни на `dev`, ни где-либо ещё — проверено
|
||||
`find docs -iname "*performance*"`. Канонический документ performance harness
|
||||
на сегодня — `demo/performance/README.md` (на него явно ссылается
|
||||
`docs/DEVELOPMENT.md:165`).
|
||||
|
||||
**Как проявится:** разработчик, следуя ТЗ буквально, либо не найдёт файл и
|
||||
пропустит часть AC13 «не сделано» под видом выполненного, либо создаст новый
|
||||
`docs/PERFORMANCE.md` параллельно уже существующему `demo/performance/README.md`,
|
||||
получив два источника документации об одном и том же harness — ровно тот
|
||||
класс дублирования, который процесс просит избегать («параллельных бэклоги/
|
||||
документы» и принцип «одно число/один источник» по духу, здесь — один
|
||||
документ на один факт).
|
||||
|
||||
**Требуется:** исправить ссылку на `demo/performance/README.md` (или явно
|
||||
решить создать новый `docs/PERFORMANCE.md` как отдельный AC — тогда это
|
||||
техническое решение места документации, которое должно быть либо снято, либо
|
||||
явно помечено как «новый файл» в §18/§13.3, а не как правка существующего).
|
||||
|
||||
### M2 — `docs/SCREENSHOTS.md` не существует (Medium, в скоупе)
|
||||
|
||||
**Файл:** `docs/specs/451-render-performance.md`, строка 428 (§13.3).
|
||||
|
||||
**Проблема:** «после изменения `src/**` — canonical screenshots по
|
||||
`docs/SCREENSHOTS.md`, inspection и существующие golden» — файла
|
||||
`docs/SCREENSHOTS.md` в репозитории нет (`find docs -iname "SCREENSHOTS.md"`
|
||||
не находит ничего). Термин «canonical screenshots» в проекте реален (встречается
|
||||
в сообщениях коммитов и предыдущих код-ревью, например
|
||||
`docs/reviews/CODE-REVIEW-126-r1.md:6`), но процесс их пересъёмки и приёмки
|
||||
описан в `PROCESS.md` §8 (`npm run build && node demo/docs/capture.mjs`,
|
||||
`npm run docs:accept -- --reviewed --from=<артефакт>`) и в `scripts/check-docs.mjs`,
|
||||
а не в файле с таким названием.
|
||||
|
||||
**Как проявится:** тот же риск, что в M1 — ссылка на несуществующий файл в
|
||||
плане автотестов не даёт разработчику конкретного места, которое нужно
|
||||
обновить, и не является проверяемым шагом «как есть».
|
||||
|
||||
**Требуется:** заменить ссылку на фактический процесс (`PROCESS.md` §8 /
|
||||
`scripts/check-docs.mjs` / `demo/docs/capture.mjs`) или на реальный файл, если
|
||||
он появится к моменту реализации.
|
||||
|
||||
### M3 — верхняя граница диапазона «наблюдавшихся» замеров в §11.3 не подтверждена ни одним источником (Medium, в скоупе)
|
||||
|
||||
**Файл:** `docs/specs/451-render-performance.md`, строки 329-332 (§11.3).
|
||||
|
||||
**Проблема:** обоснование bootstrap `hardMaxMs` в таблице §11.3 звучит так:
|
||||
«…полученные как безопасный порядок ниже наблюдавшихся **3 432–5 538 мс**
|
||||
полного-render времени…». Число `3 432 мс` действительно есть в issue
|
||||
(«Перетаскивание: суммарно в обновлениях | 3 432 мс»). Число `5 538 мс`
|
||||
не встречается ни в теле issue, ни в одном из шести комментариев
|
||||
(`gh issue view 451 --json body,comments`, поиск `5538`/`5 538` — ноль
|
||||
совпадений), ни где-либо в репозитории вне самого этого ТЗ.
|
||||
|
||||
**Как проявится:** конкретные абсолютные потолки (`500/500/500/750/250 мс`,
|
||||
`maxSingleLongTaskMs<=150`, `longTaskTotalMs<=300`) станут блокирующим CI-гейтом
|
||||
(«fail the runner независимо от timing result», §11.2). Если верхняя граница
|
||||
диапазона, из которого эти потолки якобы выведены «безопасным порядком»,
|
||||
не подкреплена измерением, у порогов нет прослеживаемой доказательной базы —
|
||||
именно то «утверждение, которого нет ни в одном документе и не помечено как
|
||||
предположение», которое ревью обязано ловить отдельно от продуктовых догадок.
|
||||
Не исключено, что число реально измерено автором (в issue упомянуто отдельное
|
||||
инструментирование «снято со страницы»), но в тексте ТЗ это не названо явно как
|
||||
несохранённое сырое измерение, а подано как факт наравне с процитированным
|
||||
`3 432 мс`.
|
||||
|
||||
**Требуется:** либо назвать источник `5 538 мс` (например, «долгие задачи —
|
||||
30 шт/3 298 мс плюс базовая стоимость рендера» с явным выводом формулы), либо
|
||||
убрать конкретное число и явно пометить диапазон как «оценка автора спецификации,
|
||||
поменять свободно» в §18, либо пересчитать от фактически процитированных в issue
|
||||
цифр.
|
||||
|
||||
## Что не проверялось и почему
|
||||
|
||||
- **Продуктовый код** — не прогонялись `npx tsc --noEmit`, `npm test`, `npm run
|
||||
build`, смоки, golden, invariants, performance-профили. Причина: диапазон
|
||||
этого коммита — только `docs/specs/**`, продуктового кода для #451 ещё нет;
|
||||
это гейты этапа `code`, а не `spec`.
|
||||
- **Полнота dependency-списка §6.2** (все ли реальные top-level `hass.*` поля,
|
||||
которые сейчас читает `houseplan-card.ts`, туда попадут) — не проверялась
|
||||
построчным аудитом всех обращений к `this.hass.*`: §18 прямо оставляет точный
|
||||
состав dependency projection «принято предположительно, поменять свободно»
|
||||
и делегирует полноту code review реализации (AC2 доказывается `unit` + `targeted
|
||||
smoke`, а не спек-ревью).
|
||||
- **Достижимость абсолютных бюджетов §11.3 в реальном рантайме** — это будет
|
||||
доказано конкретным CI-прогоном нового профиля при реализации (AC10), спек-ревью
|
||||
проверяет только прослеживаемость происхождения чисел (см. M3), а не их
|
||||
реалистичность.
|
||||
- **Полная матрица редакторских жестов** (Plan/Devices/Decor continuous
|
||||
interactions за пределами трёх «representative editor series») — ТЗ сознательно
|
||||
переносит её в deterministic smoke, а не в performance-профиль (§18), это
|
||||
решается на этапе реализации и проверяется в code review по факту
|
||||
написанных смоков.
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/451-render-performance`
|
||||
- HEAD на момент вывода: `71c3360363991a042fbf3d84a6aeb4ecb772c83f`
|
||||
- Дерево материала: `docs/specs/451-render-performance.md` (единственный
|
||||
затронутый ТЗ-файл), `docs/specs/README.md` (индексная строка)
|
||||
- Команда поиска дерева: `git log --all --format='%H %T' | grep <дерево HEAD>`
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/451-render-performance`, коммит `71c336036399` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `c395260a0f49bc3004d1cb443cae97c4f9b88001`
|
||||
```
|
||||
git log --all --format='%H %T' | grep c395260a0f49
|
||||
```
|
||||
- ТЗ `docs/specs/451-render-performance.md`, блоб `d9c4dfdc78feeeecfaf1ec41bdf665f22d21944e`
|
||||
```
|
||||
git log --all --find-object=d9c4dfdc78feeeecfaf1ec41bdf665f22d21944e -- docs/specs/451-render-performance.md
|
||||
```
|
||||
@@ -0,0 +1,149 @@
|
||||
# SPEC-REVIEW-451-r2
|
||||
|
||||
- **Issue:** https://github.com/Matysh/houseplan-card/issues/451
|
||||
- **Этап:** spec (PROCESS.md §2.4)
|
||||
- **Заход:** r2 · блокирующих циклов израсходовано 1 из 4 (зелёный вердикт бюджет
|
||||
не тратит, #227)
|
||||
- **Материал:** `docs/specs/451-render-performance.md`, коммит `2bf65d8bc894c223a3fd3098cac66d0af22fcfcb`
|
||||
(`git rev-parse HEAD` на момент вывода), ветка `issue/451-render-performance`
|
||||
- **Вердикт:** зелёный · заход r2 · блокирующих циклов 1/4 · High: 0 · Medium: 0
|
||||
|
||||
## Скоуп ревью r2 (по дельте, PROCESS.md §2.10)
|
||||
|
||||
Предыдущий раунд — `docs/reviews/SPEC-REVIEW-451-r1.md`, вердикт жёлтый, High 0 /
|
||||
Medium 3, материал зафиксирован на коммите `71c3360363991a042fbf3d84a6aeb4ecb772c83f`
|
||||
(SHA резолвится и сейчас, это прямой предок текущего HEAD — `git merge-base
|
||||
--is-ancestor 71c33603 HEAD` истинно, находка «мёртвый SHA» не применима).
|
||||
|
||||
Дельта раунда — `git diff 71c33603..2bf65d8b -- docs/specs/451-render-performance.md`:
|
||||
7 хунков, все внутри одного файла ТЗ, ни один не расширяет скоуп и не меняет
|
||||
контракт поведения (Q1/Q2, AC1-AC12, §6-9 не тронуты). Автор сам объявил это
|
||||
как «правки по SPEC-REVIEW r1» одним комментарием, перечислив M1/M2/M3 — дельта
|
||||
подтверждена построчно и совпадает с заявленным без остатка.
|
||||
|
||||
Разбор в r2 сокращён до дельты: заново проверены только участки текста, которые
|
||||
правка задевает (M1/M2/M3 и их непосредственный контекст — §0 «Связано», §11.3,
|
||||
AC13, §13.3, §14 п.7, §15). Продуктовая рамка (§1 сценарий, §2 что видит
|
||||
человек, §7.1 обязательные разделы, AC1-AC12, §18 «принято предположительно»)
|
||||
не переоткрывалась — основание в разделе «Унаследовано из r1» ниже.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Прочитан текст всех трёх находок r1 и комментарий автора «Исправления по
|
||||
SPEC-REVIEW r1» в issue #451.
|
||||
2. Получен точный дифф правки: `git diff 71c3360363991a042fbf3d84a6aeb4ecb772c83f..2bf65d8bc894c223a3fd3098cac66d0af22fcfcb -- docs/specs/451-render-performance.md`.
|
||||
3. Для каждой из M1/M2/M3 подтверждено построчно, что упомянутая проблемная
|
||||
строка либо удалена, либо заменена корректной ссылкой (см. таблицу ниже).
|
||||
4. Проверено отсутствие остаточных упоминаний удалённых сущностей во всём
|
||||
файле: `grep -n "PERFORMANCE.md\|SCREENSHOTS.md\|5 538\|5538"
|
||||
docs/specs/451-render-performance.md` — ноль совпадений вне уже
|
||||
рассмотренных строк §11.3 (сами числа `3 432`/`98,9`/`4,2` присутствуют
|
||||
намеренно, как новое обоснование).
|
||||
5. Новое обоснование `hardMaxMs` в §11.3 сверено с телом issue #451:
|
||||
`3 432 мс` — итоговая сумма из строки «Перетаскивание: суммарно в
|
||||
обновлениях» таблицы A/B-эксперимента; `98,9 мс` — среднее обновление
|
||||
сценария «Наведение мыши (5 с)» из исходной таблицы замеров. Оба числа
|
||||
реальны и присутствуют в issue дословно (сверено `gh issue view 451
|
||||
--json body,comments`), в отличие от снятого `5 538 мс`, для которого r1
|
||||
не нашёл источника.
|
||||
6. Арифметика новой фразы «500 мс на 120 moves… не более 4,2 мс на событие»
|
||||
проверена: 500/120 = 4,1(6) — округление в сторону ceiling («не более 4,2»)
|
||||
математически корректно как верхняя граница, хоть и не самое плотное число;
|
||||
это не искажает вывод и не является новой непрослеживаемой догадкой.
|
||||
7. Проверено, что коммит `2bf65d8b` не тронул ничего, кроме
|
||||
`docs/specs/451-render-performance.md` (`git show --stat 2bf65d8b`), и несёт
|
||||
корректные трейлеры `Issue: #451` / `User-Visible: no` — правка ТЗ не
|
||||
является видимым пользователю поведением, `no` уместен.
|
||||
8. Дважды сверено, что дельта локальна и не подпадает ни под одно условие
|
||||
«разбор остаётся полным» из §2.10: `dev` не ушёл вперёд (задача ветвится от
|
||||
него же, ребейза не было), контракт поведения не менялся, новая подсистема
|
||||
не затронута, объём дельты (12 строк добавлено/удалено в одном файле) на
|
||||
порядки меньше исходного ТЗ (515 строк).
|
||||
|
||||
Гейты (`typecheck`/`test`/`build`/смоки/perf) не прогонялись — на этапе `spec`
|
||||
продуктового кода нет, диапазон изменений всей задачи по-прежнему только
|
||||
`docs/specs/**` и `docs/reviews/**`. Это то же осознанное решение, что и в r1,
|
||||
не пропуск.
|
||||
|
||||
## Закрытие раунда r1
|
||||
|
||||
| Находка | Чем закрыта | Где это видно |
|
||||
|---|---|---|
|
||||
| **M1** — `docs/PERFORMANCE.md` назван обязательным артефактом, хотя не существует | Все три упоминания удалены, единственным каноном оставлен существующий `demo/performance/README.md` | `docs/specs/451-render-performance.md:13` (Связано), AC13 (строка 392), §14 п.7 (строка 455), §15 (строка 472) — `docs/PERFORMANCE.md` отсутствует во всём файле (`grep` ноль совпадений) |
|
||||
| **M2** — `docs/SCREENSHOTS.md` в §13.3 не существует | Ссылка заменена реальным процессом: `PROCESS.md` §8, `demo/docs/capture.mjs`, `npm run docs:accept` | `docs/specs/451-render-performance.md:430-431` |
|
||||
| **M3** — верхняя граница «5 538 мс» в §11.3 не подтверждена ни одним источником | Число снято; обоснование `hardMaxMs` теперь опирается только на дважды процитированные в issue числа — `3 432 мс` (pan, суммарно) и `98,9 мс` (hover, среднее), с явным выводом целевого значения 4,2 мс/событие | `docs/specs/451-render-performance.md:329-334`; оба числа найдены в issue #451 (`gh issue view --json body,comments`) |
|
||||
|
||||
Все три находки закрыты правкой текста, а не заявлением автора — проверено
|
||||
чтением итогового файла, не с чужих слов.
|
||||
|
||||
## Унаследовано из r1
|
||||
|
||||
Без повторной проверки в r2 принято всё, чего дельта не касается — документ
|
||||
`docs/reviews/SPEC-REVIEW-451-r1.md`, материал `71c3360363991a042fbf3d84a6aeb4ecb772c83f`
|
||||
(SHA живой, подтверждён `git merge-base --is-ancestor` выше):
|
||||
|
||||
- обязательные разделы §7.1 присутствуют и в правильном порядке (сценарий,
|
||||
что видит человек, проблема, скоуп/не-скоуп, контракт, UX/touch/a11y, модель
|
||||
данных, performance, AC1-AC13, план тестов, план реализации, release-
|
||||
артефакты, риски, откат, §18);
|
||||
- продуктовые решения владельца Q1 (deferred-last-wins после
|
||||
`pointerup`/`pointercancel`) и Q2 (обычный hover в скоупе, лёгкий слой)
|
||||
корректно перенесены в контракт §6.3/§8.3 и не переоткрываются как вопросы;
|
||||
- диагноз причин A/B/C в §3 построчно сверен с `dev` в r1 и не является
|
||||
догадкой (диагностика в `_renderBody`, `_view`/`_zoom` как `state: true`,
|
||||
отсутствие `shouldUpdate`, единый источник трёх `data-*` атрибутов);
|
||||
- non-scope (§5) очерчивает границу без расширения при реализации;
|
||||
- AC1-AC13 пронумерованы, метод доказательства указан для каждого, проверяемы
|
||||
структурными assertions §11.2, а не субъективными оценками;
|
||||
- «один жест — один результат» симметричен для трёх редакторов через §8.4;
|
||||
- откат (§17) содержит конкретный механизм, а не формальность;
|
||||
- ссылки на связанные issue (#34, #82, #137, #156, #380, #396, #449)
|
||||
существуют и релевантны.
|
||||
|
||||
Дельта r2 ни одного из этих пунктов не задевает: правки лежат только в трёх
|
||||
изолированных фрагментах (ссылки на несуществующие файлы и одно число), ни один
|
||||
AC, продуктовое решение или обязательный раздел не переписаны.
|
||||
|
||||
## Находки
|
||||
|
||||
Нет. High: 0. Medium: 0. Новых догадок, выданных за факт, дельта не внесла —
|
||||
оба новых числа (`3 432`, `98,9`) прослеживаются к issue дословно.
|
||||
|
||||
## Что не проверялось и почему
|
||||
|
||||
- **Продуктовый код** — по-прежнему не существует для #451; гейты `spec` этого
|
||||
не требуют.
|
||||
- **Полнота dependency-списка §6.2, реалистичность бюджетов §11.3 в рантайме,
|
||||
полная матрица редакторских жестов** — те же три пункта, что и в r1,
|
||||
делегированы на код-ревью (§18 ТЗ явно называет их «принято предположительно»
|
||||
либо доказываемыми `unit`/`performance` на этапе реализации); дельта r2 их не
|
||||
меняла и не должна была.
|
||||
- **Арифметическая точность округления 4,1(6)→4,2** проверена вручную (см. п.6
|
||||
выше), отдельного гейта для этого нет и не требуется — это текстовое
|
||||
обоснование, а не защитный AC.
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/451-render-performance`
|
||||
- HEAD на момент вывода: `2bf65d8bc894c223a3fd3098cac66d0af22fcfcb`
|
||||
- Предыдущий SHA (r1): `71c3360363991a042fbf3d84a6aeb4ecb772c83f` — живой,
|
||||
прямой предок текущего HEAD
|
||||
- Дерево материала: `docs/specs/451-render-performance.md` (единственный
|
||||
изменённый в дельте файл)
|
||||
- Команда поиска дельты: `git diff 71c3360363991a042fbf3d84a6aeb4ecb772c83f..2bf65d8bc894c223a3fd3098cac66d0af22fcfcb -- docs/specs/451-render-performance.md`
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/451-render-performance`, коммит `2bf65d8bc894` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `125aadd5cb53e6af3ffd14998c20855dbfd2d503`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 125aadd5cb53
|
||||
```
|
||||
- ТЗ `docs/specs/451-render-performance.md`, блоб `7c323a29110974aae369077214b9e2a74d9387c1`
|
||||
```
|
||||
git log --all --find-object=7c323a29110974aae369077214b9e2a74d9387c1 -- docs/specs/451-render-performance.md
|
||||
```
|
||||
@@ -0,0 +1,517 @@
|
||||
# ТЗ #451 — фильтрация render и лёгкий live-слой взаимодействий
|
||||
|
||||
- **Issue:** https://github.com/Matysh/houseplan-card/issues/451
|
||||
- **Редакция:** первая редакция для независимого ревью; статус определяется только метками issue
|
||||
- **Тип / приоритет:** bug / P1
|
||||
- **Оценка:** пользовательская ценность 10/10; ценность для разработки 10/10;
|
||||
сложность и риск 9/10
|
||||
- **Область:** общий lifecycle `houseplan-card`, HA render snapshot и registry,
|
||||
View, редакторы Плана/Устройств/Подложки, камера и continuous interactions,
|
||||
diagnostics, performance harness
|
||||
- **Модель данных:** без новых полей, миграции и backend-изменений
|
||||
- **Связано:** #34, #82, #137, #156, #380, #396, #449,
|
||||
`demo/performance/README.md`
|
||||
|
||||
## 1. Сценарий
|
||||
|
||||
**Персона:** администратор дома, который использует средний или большой план в
|
||||
Home Assistant на desktop, планшете или настенной панели.
|
||||
|
||||
**Поверхности и момент:** обычный View и все три редактора во время движения
|
||||
мыши, pan/pinch/zoom, рисования, размещения, перетаскивания, вращения или
|
||||
resize; также обычная работа карточки, когда HA присылает фоновые state ticks.
|
||||
|
||||
На измеренном плане с 5 пространствами, 139 маркерами и 144 устройствами один
|
||||
полный update занимает десятки миллисекунд. Hover запускает его почти на каждое
|
||||
движение указателя, pan дал 76 полных updates за четыре протяжки, а нерелевантные
|
||||
HA ticks продолжают делать ту же работу в покое. Пользователь видит рывки и
|
||||
замирания вплоть до 451 мс, хотя неподвижная уже нарисованная сцена сама по себе
|
||||
не требует постоянной работы.
|
||||
|
||||
## 2. Что человек увидит до и после
|
||||
|
||||
**До:** план отстаёт от указателя, hover и редакторские preview дёргаются, а
|
||||
фоновые изменения посторонних HA entities могут вызвать заметный фриз.
|
||||
|
||||
**После:** тот же план, те же эффекты и те же результаты действий двигаются
|
||||
плавно; HA-изменения, относящиеся к плану, остаются актуальными, а посторонние
|
||||
изменения не заставляют карточку заново строить всю сцену.
|
||||
|
||||
Преднамеренных визуальных изменений нет. После завершения любого жеста
|
||||
канонический итог и итоговый кадр совпадают с текущим поведением.
|
||||
|
||||
## 3. Проблема и установленные причины
|
||||
|
||||
### 3.1 Диагностика находится в горячем render-path
|
||||
|
||||
`_renderBody()` безусловно вызывает `houseplanDiagnostics()`. Метод снова
|
||||
обходит все сохранённые HA-привязки и вызывает `_bindingStatus()` для каждой,
|
||||
хотя render использует результат лишь в трёх диагностических `data-*`
|
||||
атрибутах корневого `ha-card`. На боевом плане это 99 resolutions и около
|
||||
20–27 мс на каждый render во всех четырёх режимах.
|
||||
|
||||
Проверенный на живой странице cache одного результата уменьшил стоимость
|
||||
принудительного render примерно вдвое, а суммарные Long Tasks pan — на 70 %.
|
||||
Однако cache «навсегда» неверен: значения должны меняться при изменении
|
||||
registry, config или доступности привязки.
|
||||
|
||||
### 3.2 Continuous state реактивно перестраивает всю карточку
|
||||
|
||||
`_view`, `_zoom`, hover, draft и большая часть drag/preview state объявлены Lit
|
||||
state. Их изменение на каждом pointer/animation step планирует полный проход
|
||||
`HouseplanCard.render()`: комнаты, стены, проёмы, декор, Glow, солнце,
|
||||
устройства и подписи строятся заново, хотя между соседними шагами меняется
|
||||
только камера или небольшой интерактивный слой.
|
||||
|
||||
Уже существующий fast path initial Plan snap hover решает один частный случай,
|
||||
но не создаёт общего контракта для View и редакторов.
|
||||
|
||||
### 3.3 Любой новый объект `hass` считается причиной render
|
||||
|
||||
Карточка не отличает изменение используемой планом entity от изменения одной
|
||||
из сотен посторонних entities. Вместе с render сейчас выполняется и
|
||||
операционная обработка HA snapshot: registry authority, reconnect/load,
|
||||
activity и vacuum histories, device rebuild и visual-continuity snapshot.
|
||||
Просто вернуть `false` из `shouldUpdate()` недостаточно: это могло бы убрать
|
||||
видимый render вместе с обязательной обработкой входящего состояния.
|
||||
|
||||
### 3.4 Performance gate проверяет другие сценарии
|
||||
|
||||
Абсолютные `hardMaxMs` уже есть. Пробел состоит не в отсутствии абсолютного
|
||||
механизма, а в сценариях: `panZoomMs` измеряет один wheel transition,
|
||||
`stateUpdateMs` меняет участвующую entity, а настоящий drag, View hover,
|
||||
нерелевантный HA tick и количество тяжёлых render-проходов не проверяются.
|
||||
Постоянно дорогой путь поэтому может быть одинаковым в base и candidate и
|
||||
оставаться зелёным.
|
||||
|
||||
## 4. Scope
|
||||
|
||||
В issue входят:
|
||||
|
||||
1. Разделение приёма каждого нового `hass` snapshot, операционной обработки и
|
||||
решения о визуальной инвалидизации.
|
||||
2. Явный dependency projection всех HA данных, которые использует план или
|
||||
открытая UI-поверхность, и пропуск полного render для нерелевантного tick.
|
||||
3. Cache статической части diagnostics с точной инвалидизацией; публичный
|
||||
support-report и три `data-*` атрибута остаются актуальными.
|
||||
4. Единый lightweight live-interaction путь для камеры, hover и непрерывных
|
||||
preview во View и трёх редакторах.
|
||||
5. Coalescing pointer/animation updates до не более одного лёгкого paint на
|
||||
animation frame без повторной шаблонизации тяжёлой сцены.
|
||||
6. Один полный reconciliation render после commit/cancel жеста; применение
|
||||
последнего отложенного релевантного HA snapshot в том же итоговом кадре.
|
||||
7. Сохранение desktop, touch/pen, flat/hidden-iso, kiosk, fixed-floor,
|
||||
visibility/reconnect и lazy-runtime контрактов.
|
||||
8. Новый performance-профиль или совместимое расширение harness с настоящими
|
||||
pointer series, абсолютными потолками, Long Task и структурными assertions.
|
||||
9. Unit, targeted smoke, canonical screenshots/golden и release artifacts.
|
||||
|
||||
## 5. Non-scope
|
||||
|
||||
Не входят:
|
||||
|
||||
- изменение внешнего вида комнат, стен, проёмов, устройств, подсказок, Glow,
|
||||
солнечных лучей, пылесосов, декора или hidden iso;
|
||||
- изменение hit areas, snap tolerance, grid, gesture thresholds, animation
|
||||
duration/easing, click/double-click/long-press или commit/cancel semantics;
|
||||
- виртуализация SVG/DOM, spatial index, изменение числа отображаемых объектов;
|
||||
- оптимизация конкретных geometry algorithms, room labels или decor renderer,
|
||||
кроме прекращения их повторного вызова без причины;
|
||||
- изменение persisted config/layout, websocket/backend API, schema,
|
||||
import/export или миграция данных;
|
||||
- ослабление существующих relative/absolute performance budgets;
|
||||
- публичная настройка, feature flag или новое пользовательское сообщение.
|
||||
|
||||
## 6. Контракт входящих HA snapshots
|
||||
|
||||
### 6.1 Приём не равен render
|
||||
|
||||
Каждое присваивание `card.hass = next` обязано обновить актуальную ссылку на HA
|
||||
и пройти безопасный intake, даже если visual render не нужен. Intake продолжает:
|
||||
|
||||
- отслеживать connection/reconnect и registry authority;
|
||||
- запускать начальную/повторную загрузку, когда она требуется;
|
||||
- поддерживать device roster, finite activity runtime и vacuum telemetry/trail;
|
||||
- обновлять данные, которыми воспользуются event handlers и HA service calls;
|
||||
- поддерживать visual-continuity lifecycle без публикации половины кадра.
|
||||
|
||||
Пропуск visual render не имеет права пропустить подписку, history point,
|
||||
terminal activity edge, vacuum sample, reconnect или capability change.
|
||||
|
||||
### 6.2 Dependency projection
|
||||
|
||||
Для решения о render существует один канонический набор зависимостей, а не
|
||||
отдельные списки в фильтре и в renderer. В него входят как минимум:
|
||||
|
||||
- entity/device/area bindings всех сохранённых маркеров и их `controls`;
|
||||
- room temperature/humidity sources;
|
||||
- opening contact/lock references;
|
||||
- entities в live-text;
|
||||
- `sun.sun`, light/Glow sources, vacuum source/map/telemetry references;
|
||||
- entities и registry metadata, необходимые открытому dialog/picker/info card;
|
||||
- active/disabled/removed registry resolution для bindings;
|
||||
- top-level HA значения, влияющие на язык, локаль, units, theme, user/write
|
||||
permissions, capabilities, connection и frontend formatting.
|
||||
|
||||
Entity state считается изменившимся, если для dependency id изменилось
|
||||
наличие или identity соответствующего HA state object. Это включает изменение
|
||||
`state`, attributes, timestamps и unavailable/recovery без дорогого deep
|
||||
comparison. Изменение нерелевантной entity не инвалидирует visual frame.
|
||||
|
||||
Dependency set пересобирается после config/layout/marker/space/dialog/registry
|
||||
изменения до следующего решения о фильтрации. Если классификатор не может
|
||||
доказать нерелевантность snapshot, он fail-open: разрешает render.
|
||||
|
||||
### 6.3 Обычный режим и активный жест
|
||||
|
||||
- Вне continuous interaction релевантный HA snapshot публикуется без новой
|
||||
намеренной задержки — в обычном ближайшем Lit update.
|
||||
- Нерелевантный HA snapshot не вызывает `HouseplanCard.render()` и не меняет
|
||||
тяжёлый DOM, но становится актуальным для последующих действий и intake.
|
||||
- Во время активного pan/pinch/resize/drag/draft релевантный HA snapshot не
|
||||
прерывает лёгкий live paint. Сохраняется только последний snapshot; сразу
|
||||
после `pointerup` или `pointercancel` он входит в единственный итоговый full
|
||||
render. Промежуточные snapshots не проигрываются кадр за кадром.
|
||||
- `lostpointercapture`, уход со страницы, смена mode/space и structural config
|
||||
change завершают либо отменяют interaction штатным общим terminal path и не
|
||||
оставляют pending snapshot навсегда. Структурное изменение не откладывается,
|
||||
если продолжение жеста с ним небезопасно.
|
||||
- HA actions и safety checks всегда читают последний принятый `hass`, а не
|
||||
отложенный visual snapshot.
|
||||
|
||||
## 7. Контракт diagnostics cache
|
||||
|
||||
1. Полный обход marker bindings не выполняется из `_renderBody()` и не
|
||||
повторяется на camera/hover/editor pointer step.
|
||||
2. Cache хранит статический результат registry diagnostics и binding counts.
|
||||
Он инвалидируется при изменении marker/config binding lifecycle, registry
|
||||
revision/authority, active/disabled/removed metadata или наличия state,
|
||||
влияющего на binding status.
|
||||
3. Обычное изменение значения уже доступной entity не инвалидирует binding
|
||||
counts, если её классификация `active/ha_disabled/orphaned/unverified` не
|
||||
могла измениться.
|
||||
4. Три корневых атрибута сохраняют текущие имена и значения:
|
||||
`data-ha-registry-access`, `data-ha-disabled-bindings`,
|
||||
`data-ha-unverified-bindings`.
|
||||
5. Публичный `houseplanDiagnostics()` сохраняет форму и redaction. Динамический
|
||||
`lastSuccessAgeMs` вычисляется в момент вызова поверх cached core; для его
|
||||
роста не запускается timer и не инвалидируется plan render.
|
||||
6. После invalidation новый результат вычисляется не более одного раза до
|
||||
следующей смены dependency, независимо от количества render requests.
|
||||
|
||||
## 8. Контракт lightweight live-interaction слоя
|
||||
|
||||
### 8.1 Граница тяжёлой сцены
|
||||
|
||||
Тяжёлая сцена — room fills/outlines, physical walls, openings, saved decor,
|
||||
Glow/sun, device faces, room labels, vacuum trails и hidden-iso geometry — не
|
||||
перешаблонизируется из-за очередного pointermove или camera animation frame.
|
||||
Её DOM identity сохраняется в пределах interaction, если нет отдельной
|
||||
структурной причины для rebuild.
|
||||
|
||||
Вызов `HouseplanCard.render()` считается full render независимо от того, дал ли
|
||||
Lit затем минимальный DOM diff. Рендер отдельного lightweight child/layer full
|
||||
render карточки не считается.
|
||||
|
||||
### 8.2 Камера
|
||||
|
||||
Pan, pinch, wheel/camera transition, zoom buttons, room focus и double-fit
|
||||
обновляют все участвующие SVG `viewBox`, HTML overlay projection и зависящие от
|
||||
камеры размеры согласованно в одном RAF-coalesced live paint. Flat и hidden iso
|
||||
не расходятся; tooltip, device layer, room labels, locks, measure labels,
|
||||
vacuum puck и editor chrome остаются совмещены с планом.
|
||||
|
||||
Canonical `_view`/`_zoom` продолжают отражать реально показанный кадр, чтобы
|
||||
retarget/cancel/persistence контракты #82/#396/#449 не получили stale start.
|
||||
На terminal state выполняется один full reconciliation render и штатное
|
||||
сохранение viewport там, где оно происходило раньше.
|
||||
|
||||
### 8.3 Hover
|
||||
|
||||
Принятый владельцем Q2 включает обычный hover:
|
||||
|
||||
- room fill/physical outline и room tooltip сохраняют нынешние content,
|
||||
visibility, pointer modality и настройку отключения tooltip;
|
||||
- device tooltip/LQI/temperature/humidity и текущие CSS hover states остаются;
|
||||
- координата tooltip может обновляться каждый RAF, не вызывая full render;
|
||||
- leave, touch suppression, mode/space switch, visibility change и removal
|
||||
очищают lightweight hover без ghost overlay;
|
||||
- hover никогда не меняет config/layout и не делает websocket writes.
|
||||
|
||||
### 8.4 Редакторские continuous interactions
|
||||
|
||||
Один и тот же принцип применяется ко всем состояниям, которые меняются на
|
||||
pointermove до commit:
|
||||
|
||||
| Поверхность | Live-содержимое |
|
||||
|---|---|
|
||||
| План | wall chain/rubber-band, snap/conflict marker, column/opening preview и dimensions, wall/room resize preview и measurements, physical move/rotate |
|
||||
| Устройства | marker drag/position preview, align guides и связанный tooltip |
|
||||
| Подложка | line/shape draft, decor/furniture/image move/resize/rotate, placement preview, backdrop transform и measurements |
|
||||
|
||||
Pointermove меняет только соответствующий lightweight layer или transform.
|
||||
Pointerup сохраняет тот же canonical результат, history entry и write, что до
|
||||
#451; cancel восстанавливает то же исходное состояние и не создаёт write.
|
||||
Pointerdown может выполнить отдельный full render, если действительно меняет
|
||||
selection/tool UI, но последующие move-события не повторяют его.
|
||||
|
||||
Все movement sources coalesce: между двумя animation frames применяется
|
||||
последняя позиция, но commit повторно использует последнюю canonical event
|
||||
coordinate/state, поэтому coalescing не теряет конечную точку.
|
||||
|
||||
## 9. UX, touch и accessibility
|
||||
|
||||
- Внешний вид до/после settled state должен быть pixel-equivalent с текущим
|
||||
`dev`; намеренных golden diffs нет.
|
||||
- Mouse, touch и pen сохраняют pointer capture, pan threshold, pinch ownership,
|
||||
click suppression и `pointercancel` поведение.
|
||||
- Hover остаётся только на устройствах с fine hover capability; touch не
|
||||
получает синтетический ghost hover.
|
||||
- Keyboard/Escape/Ctrl+Z и focus order не меняются.
|
||||
- Reduced motion сохраняет atomic camera result; forced colours, dark/light,
|
||||
kiosk, fixed-floor и read-only не получают отдельной ветки поведения.
|
||||
- Новых текстов, контролов, ARIA-элементов и i18n keys нет.
|
||||
|
||||
## 10. Модель данных, совместимость и безопасность
|
||||
|
||||
- Persisted config/layout и backend payload не меняются; migration/write-back
|
||||
отсутствуют.
|
||||
- Старые планы автоматически получают ускорение после обновления frontend.
|
||||
- Оптимизация не изменяет HA service calls, permission checks, destructive
|
||||
confirmations, registry redaction или support-report contents.
|
||||
- Не вводятся worker, network request, dependency или глобальный shared cache
|
||||
между экземплярами карточки.
|
||||
- Несколько карточек на dashboard имеют независимые interaction queues,
|
||||
dependency sets и diagnostics cache.
|
||||
- Disconnect удаляет RAF/listener/observer и pending interaction state; warm
|
||||
remount не переносит незавершённый gesture в новый экземпляр.
|
||||
|
||||
## 11. Performance и наблюдаемость
|
||||
|
||||
### 11.1 Новый профиль
|
||||
|
||||
Добавляется отдельный `large-house-interaction-v1` на существующем
|
||||
детерминированном large-house fixture, чтобы не менять смысл
|
||||
`large-house-v1`. Один runner/harness обязан оставаться способен измерить base
|
||||
до #451 и candidate; новые private поля сначала объявляются optional с честным
|
||||
fallback либо счётчики собираются внешним instrumentation.
|
||||
|
||||
После warm-up профиль выполняет:
|
||||
|
||||
1. 120 View hover moves по room/device/miss;
|
||||
2. четыре pan drag по 20 moves и terminal pointerup;
|
||||
3. camera wheel/transition и pinch series;
|
||||
4. не менее трёх representative editor series: Plan draw/snap, room resize и
|
||||
Decor furniture/shape transform; targeted smoke покрывает остальную матрицу;
|
||||
5. 30 нерелевантных HA ticks и один релевантный tick;
|
||||
6. релевантный HA tick в середине drag с проверкой deferred-last-wins frame.
|
||||
|
||||
### 11.2 Структурные assertions
|
||||
|
||||
Для измерительного окна после начального settled frame:
|
||||
|
||||
- 120 hover moves: **0** full renders;
|
||||
- каждое pan/pinch/editor move-series: **0** full renders между start и terminal
|
||||
event и не более **1** terminal full render;
|
||||
- 30 нерелевантных HA ticks: **0** full renders;
|
||||
- один релевантный HA tick вне gesture: ровно **1** full render;
|
||||
- несколько релевантных ticks внутри gesture: **0** промежуточных и ровно
|
||||
**1** terminal full render с последним состоянием;
|
||||
- camera/hover/editor windows: **0** полных marker-binding diagnostic scans;
|
||||
- static heavy-scene node identity/count, config, layout и websocket writes
|
||||
остаются стабильны; preview commit/cancel checks выполняются отдельно.
|
||||
|
||||
Assertions fail the runner независимо от timing result. RAF paints и renders
|
||||
выделенного lightweight child считаются отдельными метриками и не маскируются.
|
||||
|
||||
### 11.3 Абсолютные цели
|
||||
|
||||
Для профиля при тех же CI browser/runtime условиях задаются base-relative
|
||||
пределы и следующие bootstrap `hardMaxMs`. Они намеренно существенно ниже
|
||||
опубликованного в issue времени полных updates одного pan-сценария — 3 432 мс —
|
||||
и согласованы с существующим `panZoomMs: 500 ms`. Для hover потолок 500 мс на
|
||||
120 moves означает в среднем не более 4,2 мс на событие против замеренных в
|
||||
issue 98,9 мс на полный update:
|
||||
|
||||
| Метрика | Абсолютный потолок |
|
||||
|---|---:|
|
||||
| 120 View hover moves | 500 мс |
|
||||
| 4 × 20 pan moves + terminal | 500 мс |
|
||||
| wheel/pinch camera series | 500 мс |
|
||||
| 120 representative editor moves + terminal | 750 мс |
|
||||
| 30 нерелевантных HA ticks | 250 мс |
|
||||
|
||||
Для каждого pointer-series дополнительно: `maxSingleLongTaskMs <= 150`,
|
||||
`longTaskTotalMs <= 300`, `longTaskCount <= 3`. Эти потолки — bootstrap safety
|
||||
limits, а не разрешение расходовать весь budget; relative comparison остаётся.
|
||||
Ослабление существующих budgets запрещено. Первый paired Linux artifact должен
|
||||
быть описан в review/release evidence; ужесточение после него допустимо отдельной
|
||||
обоснованной правкой, ослабление — только через новый review.
|
||||
|
||||
## 12. Acceptance criteria
|
||||
|
||||
- **AC1 (`unit` + code review; разработчик/ревьюер):** HA intake отделён от
|
||||
visual invalidation; нерелевантный snapshot обновляет актуальный `hass` и
|
||||
lifecycle/history, но не вызывает `HouseplanCard.render()`.
|
||||
- **AC2 (`unit` + targeted smoke; разработчик):** изменение каждой категории
|
||||
dependency из §6.2 вызывает актуальный render, а изменение посторонней entity
|
||||
— нет; unknown classifier path fail-open и не оставляет stale UI.
|
||||
- **AC3 (`unit` + targeted smoke; разработчик):** activity/vacuum samples,
|
||||
reconnect/load, registry subscription, permissions и action safety работают
|
||||
при серии skipped visual ticks; action использует последний `hass`.
|
||||
- **AC4 (`unit` + smoke; разработчик):** diagnostics binding scan отсутствует
|
||||
в render/pointer path, cache инвалидируется по §7, три `data-*` атрибута и
|
||||
redacted public report актуальны, `lastSuccessAgeMs` растёт без repaint timer.
|
||||
- **AC5 (`smoke` + performance; разработчик):** View hover, pan, pinch,
|
||||
wheel/camera animation и double-fit соблюдают render-count assertions §11.2;
|
||||
все SVG/HTML/iso layers остаются совмещены на каждом captured frame.
|
||||
- **AC6 (`smoke` + performance; разработчик):** Plan draw/snap/opening/column,
|
||||
physical move/rotate, room resize, device drag, decor draft/move/resize/rotate,
|
||||
furniture/image placement и backdrop transform не вызывают full render на
|
||||
каждый move; terminal commit/cancel даёт один согласованный full frame.
|
||||
- **AC7 (`unit` + smoke; разработчик):** серия relevant HA updates во время
|
||||
gesture применяет только последний snapshot сразу после всех terminal paths;
|
||||
irrelevant updates не создают pending render.
|
||||
- **AC8 (`unit` + smoke; разработчик):** coalescing сохраняет конечную pointer
|
||||
coordinate, snap/geometry result, history, save count, pointer capture,
|
||||
cancel/undo и no-extra-write контракты для mouse/touch/pen.
|
||||
- **AC9 (`golden` + canonical screenshots; разработчик/владелец):** settled
|
||||
View, hover/tooltip, все три редактора, dark/light, forced colours, kiosk,
|
||||
fixed-floor и hidden iso не имеют непредусмотренных pixel diffs; screenshots
|
||||
приложены к handoff/review.
|
||||
- **AC10 (`performance`; разработчик):** новый interaction profile проходит
|
||||
все structural, absolute и base-relative checks §11; прежние ordinary,
|
||||
isometric, plan-snap и Glow profiles проходят без ослабления budgets.
|
||||
- **AC11 (`typecheck` + `unit` + `build`; разработчик):** implementation-loop
|
||||
gates зелёные; перед beta зелёные golden, smoke и performance по runbook;
|
||||
Linux CI остаётся каноном полного HA harness.
|
||||
- **AC12 (`schema/backend/i18n review`; ревьюер):** persisted model, backend,
|
||||
import/export, HA API, i18n strings и dependencies не изменены; новые caches
|
||||
bounded per card и очищаются при disconnect.
|
||||
- **AC13 (`documentation review`; разработчик/ревьюер):**
|
||||
`demo/performance/README.md` описывает новый профиль, counters, budgets и
|
||||
локальный запуск; оба changelog содержат пользовательский эффект со ссылкой
|
||||
на #451.
|
||||
|
||||
## 13. План автотестов
|
||||
|
||||
### 13.1 Unit
|
||||
|
||||
- pure dependency classifier: relevant/irrelevant/missing state, attributes,
|
||||
all source kinds, top-level locale/theme/units/user/connection and fail-open;
|
||||
- dependency-set rebuild после marker/config/dialog/registry changes;
|
||||
- HA intake с skipped render: activity terminal edge, vacuum sample, reconnect,
|
||||
latest action snapshot и no duplicate processing;
|
||||
- gesture gate state machine: begin/update coalescing/end/cancel/forced cancel,
|
||||
latest-relevant-wins и irrelevant-no-pending;
|
||||
- diagnostics cache keys, one scan per invalidation, state-presence transition,
|
||||
dynamic age и redaction;
|
||||
- camera live projection: clamp, anchor, retarget, persistence and cleanup.
|
||||
|
||||
### 13.2 Targeted production-bundle smoke
|
||||
|
||||
Инструментировать full render count, lightweight paints, diagnostics scans,
|
||||
heavy node identity, writes и visible state. На production bundle пройти:
|
||||
|
||||
- room/device/miss hover and leave;
|
||||
- mouse pan, wheel, pointercancel; touch pan/pinch and lost capture;
|
||||
- #449 double-fit и interrupted/retargeted camera transition;
|
||||
- матрицу interactions из AC6 с несколькими move events, commit и cancel;
|
||||
- relevant/irrelevant HA ticks вне и внутри gesture;
|
||||
- space/mode switch, visibility hide/show, disconnect/reconnect и warm remount;
|
||||
- flat/iso, normal/fixed-floor/kiosk/read-only.
|
||||
|
||||
Synthetic events обязаны доходить до production handlers; `0` событий у
|
||||
handler не принимается как доказательство `0` renders.
|
||||
|
||||
### 13.3 Golden, performance и release
|
||||
|
||||
- implementation loop: `npm run typecheck`, `npm test`, `npm run build`;
|
||||
- после изменения `src/**` — canonical screenshots по workflow из `PROCESS.md`
|
||||
§8 (`demo/docs/capture.mjs`, затем только после проверки `npm run docs:accept`),
|
||||
inspection и существующие golden; любой diff исследуется;
|
||||
- перед beta — полный smoke/golden/performance gate по runbook;
|
||||
- full performance сравнивает candidate/base на одном Linux runner и публикует
|
||||
отдельный interaction artifact со structural diagnostics;
|
||||
- три bundle snapshots после release build должны совпадать.
|
||||
|
||||
## 14. План реализации и затрагиваемые файлы
|
||||
|
||||
Ожидаемые точки изменения:
|
||||
|
||||
1. `src/houseplan-card.ts` — HA intake/visual invalidation, diagnostics cache,
|
||||
gesture terminal reconciliation и подключение live layers.
|
||||
2. Новый либо существующий узкий `src/*render*`/`src/*interaction*` module —
|
||||
pure dependency classifier, bounded state machine и RAF scheduler; точные
|
||||
имена не являются контрактом.
|
||||
3. `src/houseplan-editor-runtime.ts` — маршрутизация continuous editor previews
|
||||
в lightweight invalidation без изменения canonical commit logic.
|
||||
4. Unit tests для classifier/cache/controller и regression tests существующих
|
||||
camera/editor controllers.
|
||||
5. Targeted `demo/smoke_*.mjs` — production-bundle render-count и interaction
|
||||
matrix; обновление smoke runner/package scripts при необходимости.
|
||||
6. `demo/benchmark_large_house.mjs`, `demo/performance/card-contract.mjs`, новый
|
||||
budget/profile и evaluator/report plumbing — `large-house-interaction-v1`.
|
||||
7. `demo/performance/README.md` и оба changelog.
|
||||
|
||||
Реализация сначала выделяет pure contracts и diagnostics cache, затем HA
|
||||
filter, затем camera/hover live path, затем редакторские consumers. Нельзя
|
||||
массово снять `state: true`, пока у каждого изменяемого состояния нет
|
||||
проверенного live paint и terminal reconciliation path.
|
||||
|
||||
## 15. Release-артефакты
|
||||
|
||||
Изменение пользовательски заметно как исправление зависаний, поэтому основной
|
||||
implementation commit имеет `User-Visible: yes` и в том же коммите обновляет:
|
||||
|
||||
- `docs/CHANGELOG.md`;
|
||||
- `docs/CHANGELOG.ru.md`.
|
||||
|
||||
Также обновляются:
|
||||
|
||||
- `demo/performance/README.md`;
|
||||
- budget/profile и machine-readable performance report;
|
||||
- canonical screenshots/golden evidence без ожидаемого визуального изменения;
|
||||
- targeted smoke artifacts и full performance comparison.
|
||||
|
||||
Новые user-guide/i18n/security artifacts не требуются. Выпуск — обычной beta
|
||||
по разделу 8 release runbook; issue не закрывается автором реализации.
|
||||
|
||||
## 16. Риски и снижение
|
||||
|
||||
| Риск | Вероятность / ущерб | Снижение |
|
||||
|---|---|---|
|
||||
| Фильтр ошибочно считает relevant HA tick посторонним | средняя / высокий | единый dependency authority, fail-open, матрица всех consumers и dialog states |
|
||||
| Пропуск render одновременно пропускает activity/vacuum/reconnect intake | высокая / высокий | отдельный intake contract и тесты серий skipped ticks |
|
||||
| Lightweight DOM расходится с canonical state | средняя / высокий | один state machine, last-event commit и обязательный terminal reconciliation |
|
||||
| Несколько SVG/HTML/iso слоёв получают разные camera frames | средняя / высокий | один atomic projection writer и frame screenshots |
|
||||
| Gesture оставляет pending HA/RAF после cancel/disconnect | средняя / высокий | единый terminal cleanup и lifecycle matrix |
|
||||
| Child render формально скрывает прежнюю тяжёлую работу | средняя / средний | отдельные full/light counters, heavy-node identity и timing gate |
|
||||
| Новый harness ломает сравнение со старым base | средняя / высокий | новый profile id, optional contract/fallback и тест candidate/base |
|
||||
| Слишком мягкий timing budget снова пропускает проблему | средняя / высокий | structural assertions обязательны; absolute + relative gates, paired artifact review |
|
||||
|
||||
## 17. Откат
|
||||
|
||||
Откат — revert implementation commit(ов) #451 и нового interaction profile.
|
||||
Данные и backend не требуют rollback/migration. Если дефект найден только в
|
||||
одном live interaction, допускается временно вернуть этому interaction прежний
|
||||
reactive path отдельным аварийным commit, не отключая безопасный diagnostics
|
||||
cache и HA filter, но такой fallback обязан иметь issue и regression proof.
|
||||
|
||||
## 18. Принято предположительно, поменять свободно
|
||||
|
||||
Ниже технические решения, не являющиеся продуктовым выбором владельца:
|
||||
|
||||
- dependency projection может переиспользовать `RenderDeviceSnapshot`, но имеет
|
||||
один источник истины и включает зависимости всего сохранённого плана;
|
||||
- lightweight слой может быть Lit child component, imperative DOM projection
|
||||
или их комбинацией, если full/light counters и cleanup contracts соблюдены;
|
||||
- public diagnostics складывается из cached static core и динамического age;
|
||||
- точные private field/helper names и разбиение unit/smoke файлов свободны;
|
||||
- representative editor cases живут в performance profile, полная матрица — в
|
||||
deterministic smoke, чтобы timing fixture не превращался в функциональный
|
||||
e2e-комбайн.
|
||||
|
||||
Продуктовые решения владельца не предположительны: Q1 — relevant HA updates во
|
||||
время жеста применяются последним итоговым кадром; Q2 — обычный hover входит в
|
||||
#451 и сохраняет текущие эффекты через lightweight слой.
|
||||
@@ -177,6 +177,7 @@ GitHub Issues и GitHub Projects (v2) остаются единственным
|
||||
| [#447](https://github.com/Matysh/houseplan-card/issues/447) Наружная грань для мебели и сдвиг декора стрелками | [447-exterior-furniture-snap-keyboard-nudge.md](447-exterior-furniture-snap-keyboard-nudge.md) |
|
||||
| [#448](https://github.com/Matysh/houseplan-card/issues/448) Единый бессрочный переключатель `hp_alpha` | [448-alpha-switch.md](448-alpha-switch.md) |
|
||||
| [#449](https://github.com/Matysh/houseplan-card/issues/449) Двойной клик/тап по свободному фону вписывает весь план | [449-double-fit-all.md](449-double-fit-all.md) |
|
||||
| [#451](https://github.com/Matysh/houseplan-card/issues/451) Фильтрация render и лёгкий live-слой взаимодействий | [451-render-performance.md](451-render-performance.md) |
|
||||
|
||||
## P3
|
||||
|
||||
|
||||
Reference in New Issue
Block a user