mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -0,0 +1,184 @@
|
||||
# CODE-REVIEW-531-r1
|
||||
|
||||
**Issue:** #531 «Панорама лагает в Firefox: viewBox переписывается каждый кадр»
|
||||
**Материал:** ветка `issue/531-live-pan-transform`, SHA `57b80f08e63040c90ef67ff6ab7b65e4addfdc36` (дерево `2b20dd807a08f98ef09ed50cc0cd04db39138ea3`)
|
||||
**Заход:** r1 · блокирующих циклов израсходовано 0/4
|
||||
**Вердикт: жёлтый**
|
||||
|
||||
## Скоуп
|
||||
|
||||
ТЗ (спор-ревью зелёный на r2, `docs/reviews/SPEC-REVIEW-531-r2.md`) требует: во время
|
||||
жеста панорамы SVG-сцена двигается CSS-трансформом, а `viewBox` переписывается не
|
||||
чаще, чем позволяет бюджет (`LIVE_VIEWBOX_REFRESH_MS` ≈100 мс либо
|
||||
`LIVE_VIEWBOX_REFRESH_SHIFT` ≈15 % сдвига/масштаба); терминальное примирение не
|
||||
меняется; ничего не пишется в DOM, если строка не изменилась; слой устройств и
|
||||
сцена не расходятся (#451). AC1–AC9 в теле issue.
|
||||
|
||||
Диапазон диффа к `origin/dev`: `src/live-viewport.ts` (единственный продуктовый
|
||||
файл), `test/live-viewport.test.mjs` (новый), `demo/smoke_live_pan_viewbox.mjs`
|
||||
(новый), `scripts/mutation-gate.mjs` (3 новые записи), `docs/CHANGELOG.md`,
|
||||
`docs/CHANGELOG.ru.md`, `docs/ARCHITECTURE.md`, плюс три копии сгенерированного
|
||||
бандла (класс D). `src/live-interaction-runtime.ts`, вопреки предположению аналитики
|
||||
о затронутых поверхностях, не тронут — планировщик жеста не менялся, значит и
|
||||
touch-путь (тот же `PointerEvent`) не отличается от мышиного ни для одной из
|
||||
проверок ниже.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Дешёвые гейты уже зелёные в Validate на этом SHA (ссылка на прогон в задаче
|
||||
ревью), но я перегнал их и сам, поскольку часть проверок ниже требует
|
||||
пересобранного бандла и рабочего дерева на этом же SHA:
|
||||
|
||||
| Гейт | Команда | Результат |
|
||||
|---|---|---|
|
||||
| Типы + юниты + сборка | `npm run build` (включает `tsc --noEmit`) | green |
|
||||
| Юнит-тесты | `npm test` | 2525 passed, 1 skipped, 0 failed |
|
||||
| Три копии бандла | `npm run bundle:sync` + `cmp` попарно `dist`/`custom_components`/`demo/srv/assets` | идентичны |
|
||||
| Бюджет бандла | `npm run bundle:budget` | initial View 287 284 Б (потолок 288 300 ±2000) — совпадает с заявленным в хендоффе |
|
||||
| Новый `any` | `node scripts/no-new-any.mjs --base origin/dev --head HEAD` | 109 добавленных строк, новых `any` нет |
|
||||
| Провенанс коммита | `git show -s --format=full HEAD` | `Issue: #531`, `User-Visible: yes`, ветка/трейлеры на месте |
|
||||
| Документация фронтенда (diff трогает `src/**`) | `node scripts/check-docs.mjs` | **ERROR** — см. находку Medium ниже |
|
||||
| Смок AC5/AC6 | `node demo/smoke_live_pan_viewbox.mjs` | green: `viewBoxWritesDuringGesture=4`, `worstParityPx=0`, совпадает с хендоффом |
|
||||
| Golden (AC7) | `npm run golden:verify` | **169/169 passed, 0 расхождений** — полнее, чем в хендоффе (167 + 2 таймаута песочницы автора); подтверждает, что оба «зависших» сценария у автора не относились к реальному регрессу |
|
||||
| Мутанты (AC8) | см. таблицу «чем краснеет» ниже | 3/3 новых + 1 соседний старый — все проверены **выполнением**, не чтением |
|
||||
| `smoke-select.mjs` | `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | НЕОПРЕДЕЛЁННОСТЬ — новые символы (`needsViewBoxRefresh`, `LIVE_VIEWBOX_REFRESH_*` и т.д.) не связаны ни с одним существующим смоком; это ожидаемо для новой сущности, свидетель для неё — именно новый `demo/smoke_live_pan_viewbox.mjs`, названный в AC5/AC6 напрямую. Дополнительных «широких» совпадений выборка не дала (0 смоков) |
|
||||
| Инварианты модели | не запускал | диф не трогает геометрию/`layout`/толщины — не применимо |
|
||||
| `pytest tests_backend` | не запускал | диф не трогает `custom_components/**/*.py` — не применимо |
|
||||
| `performance_smoke` (`large-house-interaction-v1`) | не запускал | явно выведено из-под AC самим ТЗ («Приёмка по скорости — отдельно и не здесь»); Validate на этом SHA его тоже не гонял (job гейтится `heavy`, а этот push — обычный, не кандидат/PR/`workflow_dispatch full=true`) |
|
||||
|
||||
Рабочее дерево после всех прогонов и намеренных мутаций восстановлено ровно к
|
||||
`57b80f08` (`git status --short` пуст, `git diff HEAD -- src/live-viewport.ts`
|
||||
пуст) — материал ревью не изменён.
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium (в скоупе задачи) — устаревший отпечаток скриншотов документации
|
||||
|
||||
`node scripts/check-docs.mjs` на `57b80f08` красный:
|
||||
|
||||
```
|
||||
ERROR screenshot source fingerprint is stale; run npm run docs:capture and accept before the beta candidate (#479)
|
||||
```
|
||||
|
||||
На `origin/dev` (до этого диффа) тот же скрипт зелёный: `Documentation checks
|
||||
passed (7 files, 12 external links)`. Значит именно правка `src/live-viewport.ts`
|
||||
делает отпечаток `docs/images/screenshots.json` устаревшим (отпечаток считается по
|
||||
всему `src/**`, PROCESS.md §8) — а `docs/images/screenshots.json` в этом диффе не
|
||||
трогался (`git diff origin/dev...HEAD -- docs/images/screenshots.json` пуст).
|
||||
|
||||
Это не всплыло в Validate на этом SHA, потому что `docs`-job на обычном push
|
||||
(не кандидат, не PR, не `workflow_dispatch full=true`) идёт в режиме `warn`, а
|
||||
не `strict` — job зелёный, хотя отпечаток фактически устарел. Ровно тот же
|
||||
сценарий, что увёл `dev` в красный `docs`-job на #230/#234 до следующей задачи.
|
||||
|
||||
Правка не меняет ни один снятый кадр документации (сама механика жеста — не то,
|
||||
что попадает на статичные скриншоты), и `golden:verify` подтверждает нулевое
|
||||
визуальное расхождение по всей матрице — то есть цена закрытия минимальна:
|
||||
`npm run docs:accept -- --identical` должен пройти чисто (перезаписать только
|
||||
отпечаток исходников, не сами PNG) и закоммититься в этом же issue. Я не
|
||||
выполнял это сам — правка `docs/**` вне git-дерева ревьюера не моя роль, а
|
||||
предмет авторского коммита.
|
||||
|
||||
**Воспроизведение:** `node scripts/check-docs.mjs` на `57b80f08` (без каких-либо
|
||||
изменений с моей стороны).
|
||||
|
||||
## Таблица «чем краснеет» (защитные AC, #435)
|
||||
|
||||
| AC | чем доказан | чем краснеет | проверено |
|
||||
|---|---|---|---|
|
||||
| AC1 (кадр жеста — трансформ, не `viewBox`) | `test/live-viewport.test.mjs`, тест «#531 AC1» | мутант `live-pan-rewrites-viewbox-every-frame` (`needsViewBoxRefresh(...)` → `true`) | **выполнением**: применил патч, `tsc -p tsconfig.test.json` + `fix-test-build` + `node --test` → 6/8 (AC1 и AC2 падают) |
|
||||
| AC2 (перезапись по бюджету времени, трансформ снимается) | тест «#531 AC2» | тот же мутант | тем же прогоном (см. выше) |
|
||||
| AC2а (сдвиговый триггер срабатывает раньше времени) | тест «#531 AC2а» | мутант `live-pan-shift-threshold-ignored` (удаление сдвиговых веток) | **выполнением**: 7/8, падает именно AC2а |
|
||||
| AC3 (тихий кадр не пишет в DOM) | тест «#531 AC3» | мутант `live-pan-writes-unchanged-viewbox` (снятие проверки тождества в `setViewBox`) **не красит AC3** — в этом кадре `refresh=false`, и `setViewBox` вовсе не вызывается, проверка внутри неё недостижима. Красит **AC4** (см. ниже) | выполнением; уточнение важно само по себе — без него третий столбец для AC3 был бы пуст |
|
||||
| AC4 (терминальное примирение идемпотентно) | тест «#531 AC4» | мутант `live-pan-writes-unchanged-viewbox` | **выполнением**: 7/8, падает AC4 (повторный `force:true` пишет лишний раз) |
|
||||
| AC5 (≤ бюджета перезаписей за 20 кадров жеста) | `demo/smoke_live_pan_viewbox.mjs` | мутант `live-pan-rewrites-viewbox-every-frame`, guard = этот смок (дорогой гейт, повторно не гонял чужую отрицательную пробу — но тот же мутант уже поймал юнит-прогон выше, независимое подтверждение) | смок прогнан чистым: `viewBoxWritesDuringGesture=4` при `framesInGesture=20` |
|
||||
| AC6 (маркер и сцена в пределах 1 px, #451) | тот же смок, `worstParityPx`/`markerFollowsTheScene` | любое расхождение экранных смещений маркера и узла сцены | прогнан: `worstParityPx=0` |
|
||||
| AC7 (осевшие кадры не изменились) | `npm run golden:verify` | любое отличие хотя бы одного пикселя от эталона — это сравнение, не мутация | прогнан: 169/169, 0 расхождений |
|
||||
| AC8 (мутанты реестра) | `scripts/mutation-gate.mjs`, 3 новые записи | сами мутанты | все три плюс соседний `live-viewport-identity-projection-not-recognized` — выполнением, см. выше |
|
||||
| AC9 (документация, трейлеры) | `docs/CHANGELOG.md`, `docs/CHANGELOG.ru.md`, `docs/ARCHITECTURE.md`, коммит | не защитный AC — обычное сравнение | чтением диффа |
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- **К1/К2/К3/К4 в коде соответствуют ТЗ.** `paintLiveViewport` держит `anchor`
|
||||
(кадр, чей `viewBox` реально в DOM, и момент записи); сцена всегда проецируется
|
||||
от `anchor` к текущему кадру тем же `liveLayerProjection`, что и HTML-слои;
|
||||
`viewBox` переписывается только когда `needsViewBoxRefresh` (время **или**
|
||||
сдвиг **или** масштаб) либо `force`; и `setViewBox`/`setLayerProjection`
|
||||
дедуплицируют запись по строке в обоих случаях.
|
||||
- **К5/#451 не нарушен.** Сцена и HTML-слой двигаются одним и тем же
|
||||
преобразованием на каждом кадре (для сцены — от `anchor`, для слоя — от
|
||||
последнего осевшего кадра Lit, но оба landing point — текущий вид); смок
|
||||
AC6 подтверждает 0 px расхождения после 20 кадров жеста.
|
||||
- **Две базы проекции (сцена от `anchor`, слой от `painted`) — намеренная и
|
||||
корректная асимметрия**, а не путаница: HTML-слой позиционирован в процентах
|
||||
осевшего Lit-кадра и не может проецироваться от `anchor`, у него нет своего
|
||||
атрибута для дедупликации записи.
|
||||
- **Дополнительный триггер по масштабу** (`needsViewBoxRefresh` проверяет не
|
||||
только сдвиг по x/y, но и изменение `w`) — расширение сверх буквального текста
|
||||
К2 (там речь только о сдвиге), но действует строго в сторону усиления защиты
|
||||
от пустой полосы на краю при зуме, не ослабляет ни один AC. Не считаю
|
||||
находкой.
|
||||
- **AC5 в реализации отличается от буквального текста ТЗ** («не больше двух» →
|
||||
формула `max(2, ceil(elapsed/100)+1)` + `< frames/2`). Автор явно вынес это
|
||||
отклонение на решение ревью. Обоснование верное: 20 кадров смока на 60 Гц
|
||||
песочницы — это ≈330 мс, то есть 3–4 бюджетных окна по 100 мс, а не 139 мс
|
||||
(2 окна) как на владельческих 144 Гц; жёсткая «2» сделала бы свидетеля
|
||||
зависимым от частоты кадров машины прогона. Формула сохраняет содержательную
|
||||
проверку (перезаписей заметно меньше, чем кадров) и ловится тем же мутантом.
|
||||
Принимаю без правки: технический спор автора и ревьюера решается вердиктом
|
||||
(§7.2), не отправляю владельцу.
|
||||
- **Провенанс.** Один коммит, `Issue: #531`, `User-Visible: yes`, оба
|
||||
changelog правлены в нём же, `docs/ARCHITECTURE.md` описывает механику.
|
||||
- **Бюджет и типизация** без регрессий (см. таблицу гейтов).
|
||||
|
||||
## Чего не проверял и почему
|
||||
|
||||
- **`performance_smoke` (`large-house-interaction-v1`).** Сам ТЗ выводит
|
||||
миллисекунды за скобки AC («Приёмка по скорости — отдельно и не здесь»,
|
||||
единственный судья — профиль с машины владельца); Validate на этом SHA его
|
||||
тоже не гонял, поскольку job `heavy`-гейтится, а push не кандидат/PR/ручной
|
||||
полный прогон. Причинный механизм (сцена не растеризуется каждый кадр)
|
||||
доказан AC1–AC6 напрямую.
|
||||
- **`node scripts/model-invariants.mjs`, `pytest tests_backend`.** Диф не
|
||||
трогает геометрию/толщины/`layout` и не трогает Python — не применимо.
|
||||
- **Полная матрица смоков (243 файла).** `smoke-select.mjs` не нашёл ни прямых,
|
||||
ни зарегистрированных связей (только НЕОПРЕДЕЛЁННОСТЬ на новых символах,
|
||||
для которых свидетель — сам новый смок), «широких» совпадений — 0. Прогон
|
||||
всей матрицы для точечной правки одного модуля камеры избыточен.
|
||||
- **Мутант `live-pan-rewrites-viewbox-every-frame` против самого браузерного
|
||||
смока** (а не только против юнитов, которые тот же мутант тоже красит) —
|
||||
дорогой гейт, свидетеля второй раз не воспроизводил; принял отчёт автора
|
||||
(3/3 пойманы) вместе с независимым фактом, что тот же мутант уже красит
|
||||
юнит-прогон.
|
||||
- **Изометрический путь (риск 3 ТЗ).** Ни один тест (юнит или смок) не
|
||||
использует кадр, где `floor !== view` — то есть заявленное в разделе «Риски»
|
||||
ТЗ «проверяется тем же юнитом на изометрических данных» текстом теста не
|
||||
подтверждается: `frame()`-хелпер в `test/live-viewport.test.mjs` всегда
|
||||
строит `floor` равным `view`, а смок работает в `_setMode('view')`
|
||||
(не изо). Код при этом полностью симметричен для `camera`/`floor` (одна и
|
||||
та же пара функций, различается только поле кадра), так что риск разъехаться
|
||||
именно на изометрии низкий, но эмпирически не закрыт. Не блокирую — это не
|
||||
требование ни одного из AC1–AC9, только пункт из повествовательного раздела
|
||||
«Риски»; фиксирую как наблюдение, а не как находку Medium/Low с обязательной
|
||||
правкой.
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- SHA: `57b80f08e63040c90ef67ff6ab7b65e4addfdc36`
|
||||
- Дерево: `2b20dd807a08f98ef09ed50cc0cd04db39138ea3`
|
||||
- Диапазон: `origin/dev...HEAD` (53 файла, из них продуктовый — только
|
||||
`src/live-viewport.ts`; остальное — тесты/смок/мутанты/документация/бандл)
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/531-live-pan-transform`, коммит `57b80f08e630` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `2b20dd807a08f98ef09ed50cc0cd04db39138ea3`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 2b20dd807a08
|
||||
```
|
||||
- Тело issue: `89e75138a50434ac52f42cef606d8af741a8ae3b022dba382c4b62c1f4887e8a`
|
||||
- Вердикт конвейера: `yellow` · High 0
|
||||
Reference in New Issue
Block a user