mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-30 19:58:50 +00:00
@@ -0,0 +1,232 @@
|
||||
# SPEC-REVIEW-531-r1
|
||||
|
||||
**Issue:** #531 — «Панорама плана в Firefox: `viewBox` переписывается на каждый
|
||||
кадр жеста, композитор ждёт 13 vsync-тиков»
|
||||
**Этап:** spec (ревью ТЗ, PROCESS.md §2.4)
|
||||
**Трек:** полный (автор называет нарушенный критерий §5: «нет влияния на
|
||||
производительность» — обоснованно, см. «Что проверено и корректно»)
|
||||
**Заход:** r1 · блокирующих циклов израсходовано 0 из 4
|
||||
**Материал:** тело issue #531 на dev `af2081cb` (рабочая копия на этом же SHA)
|
||||
|
||||
## Скоуп
|
||||
|
||||
ТЗ живёт в теле issue #531, раздел `## ТЗ`, написан после аналитики S2
|
||||
(комментарий от 11.09). Предмет: во время живого жеста панорамы/зума
|
||||
(`paintLiveViewport`, `src/live-viewport.ts`) двигать SVG-узлы сцены
|
||||
(`[data-hp-live-viewbox]`) тем же CSS-трансформом, что уже применяется к
|
||||
HTML-слою устройств, вместо безусловной перезаписи атрибута `viewBox` на
|
||||
каждый кадр; переписывать `viewBox` редко, по бюджету времени и/или
|
||||
накопленному сдвигу; не писать в DOM значения, совпадающие с уже записанными.
|
||||
Терминальное примирение (`commitHouseplanViewport`) не меняется по результату.
|
||||
Контракт К1–К6, AC1–AC9.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Чтением кода без исполнения — на этапе ТЗ продуктового кода ещё нет, гейты не
|
||||
прогонялись (правка кода не входит в этот этап; изменений класса A в этом
|
||||
материале нет). Каждое техническое утверждение ТЗ сверено построчно с
|
||||
`src/live-viewport.ts`, `src/live-interaction-runtime.ts`,
|
||||
`src/houseplan-card.ts` на текущем `dev` (`af2081cb`).
|
||||
|
||||
- `docs/SCOPE.md` — J1 («живой пространственный обзор»), View mode как
|
||||
продукт для двух персон из трёх; правка не добавляет UI и не открывает
|
||||
новую поверхность действий — конфликта со скоупом нет.
|
||||
- `AGENTS.md`, `PROCESS.md` §2.3–§2.5, §5, §7.1 — формальные требования к ТЗ
|
||||
и выбору трека.
|
||||
- `src/live-viewport.ts` целиком (166 строк): `paintLiveViewport`,
|
||||
`liveLayerProjection`, `isIdentityLiveLayerProjection`, `setLayerProjection`,
|
||||
`scheduleHouseplanViewport`, `commitHouseplanViewport`, `frameOf`.
|
||||
- `src/live-interaction-runtime.ts` целиком: `LiveRuntime.viewport/commit`
|
||||
вызывают ровно эти функции, других путей записи `viewBox` нет.
|
||||
- `src/houseplan-card.ts:6149-6153` — `_floorView`: в неизометрическом режиме
|
||||
возвращает **тот же объект** `view` без копирования, что делает утверждение
|
||||
спеки «`_floorView(view) === view`» (принятое предположение №3) буквально
|
||||
верным, а не приближением; `data-hp-live-viewbox="camera"/"floor"` и
|
||||
`data-hp-live-layer="camera"` (строки 11507–12801) — теги существуют и
|
||||
используются как описано, включая изометрическую и обычную ветки.
|
||||
- `demo/benchmark_large_house.mjs:643-919` — единственная существующая
|
||||
проверка, трогающая `viewBox` во время жеста; проверяет только сам факт
|
||||
сдвига после завершения жеста, не частоту перезаписи.
|
||||
- `test/live-viewport.test.mjs` (31 строка) — файл уже существует (ТЗ верно
|
||||
пишет «создаётся, если нет», не заявляя, что он новый).
|
||||
- `scripts/mutation-gate.mjs:7551-7558` — существующий мутант на
|
||||
`isIdentityLiveLayerProjection`, прецедент для новых AC8-мутантов.
|
||||
- `docs/ARCHITECTURE.md:1908` — раздел про `live-viewport.ts` уже существует,
|
||||
есть что расширять по AC9.
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium — второй триггер бюджета (К2) не имеет ни одного AC и ни одной названной константы
|
||||
|
||||
**Место:** тело issue #531, раздел `### Контракт`, пункт К2.
|
||||
|
||||
К2 определяет **два независимых** триггера перезаписи `viewBox`: истечение
|
||||
времени (`LIVE_VIEWBOX_REFRESH_MS`, с ориентиром «≈100 мс») **или**
|
||||
накопленный сдвиг, превысивший «долю видимой области». Первому триггеру дан
|
||||
и ориентир значения, и прямой тест: AC2 явно описывает сценарий «по истечении
|
||||
бюджета `viewBox` переписывается ровно один раз». Второму триггеру не дано ни
|
||||
ориентировочного значения константы (в отличие от 100 мс для первого), ни
|
||||
отдельного AC — ни одного из AC1–AC9 нельзя провалить, если реализация вообще
|
||||
не построит путь «накопленный сдвиг → досрочная перезапись», при условии, что
|
||||
временной бюджет продолжает работать:
|
||||
|
||||
- AC1–AC4 — модульные сценарии с явными кадрами/вызовами, ни один не создаёт
|
||||
ситуацию «время в пределах бюджета, но сдвиг уже большой»;
|
||||
- AC5 (смок, реальная панорама) считает *суммарное* число перезаписей
|
||||
`viewBox` за 20 кадров жеста («не больше двух») — этот счётчик не
|
||||
различает, каким триггером вызвана каждая перезапись, и пройдёт одинаково
|
||||
и с рабочим, и с полностью бездействующим сдвиговым триггером (например,
|
||||
если реализация просто не добавит вторую ветку условия);
|
||||
- `demo/benchmark_large_house.mjs` (единственная существующая проверка живого
|
||||
`viewBox`) тоже смотрит только на факт сдвига после жеста, не на триггер.
|
||||
|
||||
Формально К2 — часть контракта, обязательного к реализации, но ни один AC не
|
||||
способен покраснеть на его отсутствии. Это ровно тот класс дефекта, который
|
||||
должен ловить ревью ТЗ (PROCESS.md §2.4: «найти, где ТЗ не выполнимо или не
|
||||
проверяемо») и который DoR прямо требует закрыть (PROCESS.md §2.5:
|
||||
«AC1…ACn — пронумерованные проверяемые критерии приёмки» для контракта
|
||||
целиком).
|
||||
|
||||
**Требуется от автора** (правка ТЗ в этом же issue, без нового): либо назвать
|
||||
константу-ориентир для доли видимой области и добавить AC/строку в
|
||||
существующий AC2, отдельно проверяющую срабатывание именно по сдвигу (кадры
|
||||
без истечения временного бюджета, но с большим накопленным перемещением) —
|
||||
либо, если авторское намерение — необязательная оптимизация без отдельной
|
||||
гарантии, явно перенести пункт К2-«сдвиг» в блок «Принятые технические
|
||||
предположения» и снять его из нормативного контракта К-разряда. Оба выхода
|
||||
дёшевы; несделанное — жёлтый вердикт, не блокирует до второго захода.
|
||||
|
||||
Серьёзность — Medium, строго в скоупе задачи: без High-находок это жёлтый
|
||||
вердикт, автор правит ТЗ, фикс проходит повторный (второй) заход.
|
||||
|
||||
### Low — обязательные разделы §7.1 присутствуют по содержанию, но не как отдельные заголовки; снято без правки
|
||||
|
||||
**Место:** тело issue #531, раздел `## ТЗ`.
|
||||
|
||||
PROCESS.md §7.1 перечисляет как отдельные обязательные разделы «проблема»,
|
||||
«скоуп и не-скоуп», «UX», «модель данных и миграция», «release-артефакты».
|
||||
В тексте ТЗ они не оформлены отдельными заголовками: проблема раскрыта внутри
|
||||
`### Сценарий и что человек увидит` (абзац «До.» — числа задержки, тик
|
||||
рефреш-драйвера, причина), не-скоуп — пунктом К6 внутри `### Контракт`
|
||||
(изометрия работает по тому же механизму без отдельной оптимизации;
|
||||
тайлинг/предрендер/изменения сцены вне объёма), UX — фразой «Ни одной новой
|
||||
настройки, вид плана не меняется» там же, модель данных/миграция и
|
||||
release-артефакты — разделом `### i18n, миграция, бэкенд` («Ничего») и AC9
|
||||
соответственно.
|
||||
|
||||
Содержательно каждый пункт присутствует и проверяем по коду (см. «Что
|
||||
проверено и корректно»); расхождение чисто структурное — читатель находит
|
||||
информацию не по заголовку, а по контексту. Не блокирует и не меняет ни
|
||||
одного вывода этого ревью. Решение ревьюера: **снять без правки**, фиксирую
|
||||
для будущего раунда, если дельта когда-нибудь коснётся структуры документа.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- **Продуктовая рамка (§7.1, два первых раздела).** Персона не названа явно
|
||||
словом, но сценарий однозначен и совпадает с `docs/SCOPE.md`: любой человек,
|
||||
тянущий план мышью или пальцем, на любой поверхности, в любом режиме —
|
||||
панорама используется и в View (планшет/киоск), и в редакторах. «До/После»
|
||||
— одной фразой каждое, без терминов реализации («план едет за курсором
|
||||
плавно», «вид плана не меняется»).
|
||||
- **Трек выбран верно.** Единственный названный критерий §5 («нет влияния на
|
||||
производительность») нарушен по существу: правка целиком посвящена частоте
|
||||
дорогой операции (растеризация SVG) в кадре жеста, а раздел «Производительность
|
||||
и бюджеты» прямо называет `large-house-interaction-v1` как гейт.
|
||||
- **Все технические утверждения о текущем коде подтверждены чтением.**
|
||||
`paintLiveViewport` действительно пишет `viewBox` безусловно (строки 95,
|
||||
98) и не имеет проверки на тождество для него, при этом у слоя-трансформа
|
||||
такая проверка уже есть (`isIdentityLiveLayerProjection`, строка 105) —
|
||||
ровно то расхождение, которое К4 просит устранить. `_floorView` в
|
||||
неизометрическом режиме возвращает тот же объект `view` — принятое
|
||||
предположение №3 не приближение, а факт.
|
||||
- **К1–К5 внутренне непротиворечивы.** «Каждый кадр — только трансформ» (К1)
|
||||
и «раз в бюджет — новый `viewBox`» (К2, по времени) не конфликтуют: кадр,
|
||||
на котором истёк бюджет, — не нарушение К1, а исключение, явно описанное
|
||||
тем же пунктом К2 («после чего трансформ возвращается в единицу в том же
|
||||
кадре»).
|
||||
- **AC1–AC4, AC6, AC9 проверяемы и имеют названный способ доказательства**,
|
||||
соответствующий существующей инфраструктуре: `test/live-viewport.test.mjs`
|
||||
уже существует и расширяем, `scripts/mutation-gate.mjs` уже содержит
|
||||
прецедент мутанта на этот же модуль, `docs/ARCHITECTURE.md` уже содержит
|
||||
раздел про `live-viewport.ts` для AC9.
|
||||
- **AC7 (golden) и AC5 (смок) не выдают синтетическую слабость сборки за
|
||||
доказательство.** Раздел «Приёмка по скорости — отдельно и не здесь» прямо
|
||||
и честно признаёт: headless-песочница держит 60 к/с на обоих путях, поэтому
|
||||
итоговый приговор по миллисекундам даёт только профиль с машины владельца
|
||||
— AC внутри репозитория доказывают механизм (частота записи в DOM), а не
|
||||
цифру задержки. Это ровно то разделение, которого требует §2.4
|
||||
(доказательство, а не заявление).
|
||||
- **Риски по существу и с путём отступления.** Пустая полоса на набегающем
|
||||
крае (риск 1) ограничена именованной константой бюджета — при
|
||||
«недостаточно» её можно уменьшить без нового цикла ревью; риск изометрии
|
||||
(риск 3) закрыт тем же юнитом на изометрических данных; риск округления на
|
||||
границе (риск 4) держится существующим пиксельным порогом контракта #451
|
||||
(К5, AC6), а не новым числом.
|
||||
- **Touch.** Заявление «панорама пальцем — тот же путь» подтверждено кодом:
|
||||
единственная точка входа `_liveVp()` (`houseplan-card.ts:4543`) вызывается
|
||||
из одних и тех же мест независимо от типа указателя (Pointer Events
|
||||
унифицируют мышь/тач) — отдельного тач-пути для панорамы нет, значит и
|
||||
отдельного тач-контракта не требуется. `docs/TOUCH-SUPPORT.md` не нарушается:
|
||||
раздел «Touch» ТЗ корректно называет вывод «без смены контракта».
|
||||
- **Принятые технические предположения — действительно технические.** Ни
|
||||
одно не подменяет продуктовое решение: все три — про то, как
|
||||
реализовывать (применимость проективного преобразования к SVG,
|
||||
неизменность толщин линий во время жеста, равенство `_floorView(view)` и
|
||||
`view` в обычном режиме), ни одно не про то, что видит пользователь.
|
||||
- **Откат.** «Ревёрт коммита» достаточен и корректен: правка не трогает
|
||||
конфиг, миграций нет, флага Labs не заявлено и не требуется по масштабу
|
||||
изменения.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Гейты `npx tsc --noEmit`/`npm test`/`npm run build` не запускались — в этом
|
||||
материале нет изменений класса A/B/D относительно `dev` (диапазон
|
||||
`origin/dev..HEAD` пуст), прогонять их значило бы проверять пустое
|
||||
множество.
|
||||
- `node scripts/check-docs.mjs` не запускался по той же причине — `src/**` не
|
||||
менялся.
|
||||
- Реализуемость числового бюджета `LIVE_VIEWBOX_REFRESH_MS≈100 мс` и порога
|
||||
«доля видимой области» на реальном железе владельца не оценивалась — это по
|
||||
тексту самого ТЗ («Приёмка по скорости — отдельно и не здесь») предмет
|
||||
профиля с машины владельца на этапе код-ревью/пре-релиза, не текста спеки.
|
||||
- Поведение в изометрическом режиме не проверялось построчно за пределами
|
||||
`_floorView` и наличия тех же data-тегов на изометрических SVG (строки
|
||||
11507, 11753–11762) — К6 явно исключает отдельную изометрическую
|
||||
оптимизацию из объёма этой задачи, детальная проверка изо-пути остаётся
|
||||
код-ревью по факту диффа.
|
||||
- Точное поведение при одновременном зуме и панораме (`_zoom` в кадре) не
|
||||
прослеживалось до реализации — контракт не даёт для этого случая
|
||||
отдельного AC, а зум уже часть существующего `LiveViewportFrame` без
|
||||
изменений в этом ТЗ.
|
||||
|
||||
## Вердикт
|
||||
|
||||
**Жёлтый · заход r1/4 · High: 0 · Medium: 1 → в задаче.**
|
||||
|
||||
ТЗ технически точное — все конкретные утверждения о коде подтвердились,
|
||||
трек выбран обоснованно, продуктовая рамка и границы (К6, Riски, откат)
|
||||
сформулированы честно и без догадок, выданных за факт. Единственная
|
||||
находка — контрактный пункт К2 (сдвиговый триггер перезаписи) не имеет
|
||||
доказательства ни одним из AC1–AC9 и не назван даже ориентировочной
|
||||
константой, поэтому реализация может законно пройти весь набор AC, вообще не
|
||||
построив этот путь. Это в точности класс дефекта, который ревью ТЗ обязано
|
||||
ловить (§2.4: «не выполнимо или не проверяемо»). Правка дешёвая — либо один
|
||||
AC плюс ориентир константы, либо честный перенос пункта в «Принятые
|
||||
технические предположения» как необязательной оптимизации. Low-находка о
|
||||
неформальной структуре разделов снята без правки, записью в этом документе.
|
||||
|
||||
Следующий статус при исправлении — «Готово к разработке» (`S5-ready`).
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `dev`, коммит `af2081cbc40e` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `08815814f33b6c836ecbd32deabf793353fc1bb1`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 08815814f33b
|
||||
```
|
||||
- Тело issue: `f2fb7dcc85656bdbedb38eff70aa8b3ef28804c4aab4bccc41c36879e7533e95`
|
||||
- Вердикт конвейера: `yellow` · High 0
|
||||
Reference in New Issue
Block a user