diff --git a/docs/reviews/SPEC-REVIEW-544-r1.md b/docs/reviews/SPEC-REVIEW-544-r1.md new file mode 100644 index 00000000..7b0d7da7 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-544-r1.md @@ -0,0 +1,160 @@ +# SPEC-REVIEW-544-r1 + +Issue: #544 · Этап: spec (PROCESS.md §2.4) · Заход: r1 · блокирующих циклов 0/4 + +Материал: тело issue #544, раздел `## ТЗ`, на момент ревью. Метка `S4-spec-review`, +предыдущих раундов ревью в комментариях issue нет — это первый заход, полный +разбор без раздела «Унаследовано». + +Продуктовая постановка (`## Пользовательский дефект` … `## Требуемый результат`) +и `## ТЗ` — один документ одной задачи; расхождений между ними нет, ТЗ прямо +ссылается на тот же механизм и тот же браузерный свидетель (45 396 px). + +## Скоуп ревью + +Проверялось: полнота обязательных разделов §7.1, однозначность и доказуемость +каждого AC, соответствие продуктовой рамке `docs/SCOPE.md` (J1/J3), соответствие +`docs/TOUCH-SUPPORT.md` (View pan/pinch — release-blocking гарантия) и +`docs/CANVAS.md` §5 (модель камеры/pan не меняется), фактическая проверка +технических утверждений ТЗ по коду на точном SHA `66a6485418c7663d749642c8c5740c95d3c9f490` +(рабочая копия репозитория стоит на этом коммите — сверено `git rev-parse HEAD`). + +Продуктовых вопросов к владельцу нет (сценарий и видимое поведение зафиксированы +однозначно); открытых технических вопросов нет — все нетривиальные решения +оформлены явным блоком «Принятые технические предположения — можно менять на +ревью» и разобраны ниже. + +## Как проверялось + +Это первый заход, поэтому разбор полный, а не по дельте. + +1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` §2.4/§2.9/§7.1/§7.2/§4. +2. Прочитано тело issue #544 целиком и единственный комментарий (S2-аналитика, + `origin/dev` `66a64854`, трек — полный, `small` не выполнен по performance и + touch-контракту). +3. Прочитаны `docs/TOUCH-SUPPORT.md` (пункт «support convenient pan, pinch zoom + and space switching» в разделе release-blocking гарантий) и `docs/CANVAS.md` + §5 (Zoom and pan — «Pinch and pan remain direct 1:1 gestures», модель pan не + входит в скоуп задачи и в не-скоупе прямо исключена). +4. Прочитан `docs/ARCHITECTURE.md`, раздел «Live viewport: a transform per + frame, a `viewBox` on a budget (#531, 2026-09-11)» — канонический текст + подсистемы, которую трогает задача. +5. Прочитан текущий код на точном SHA, названном в issue, и сверен построчно с + техническими утверждениями ТЗ (детали — в разделе «Находки» и «Проверено»). +6. Прочитаны файлы, которые ТЗ называет доказательной базой: `test/live-viewport.test.mjs`, + наличие `demo/smoke_live_pan_viewbox.mjs` и `scripts/mutation-gate.mjs` с + существующими мутантами на `live-viewport.ts`. + +Гейты (typecheck/test/build/golden) на этом этапе не прогонялись и не нужны: +этап — ревью ТЗ, продуктового кода ещё нет; предмет проверки — текст, а не +бинарник. + +## Находки + +Ни одной High или Medium находки. Ниже — то, что специально проверялось и не +подтвердилось как дефект, для прозрачности разбора. + +- **Проверено и не является находкой: покрытие К3 для декоративных iso-слоёв.** + `.iso-underlay-svg`, `.iso-shadows-svg`, `.iso-walls-svg`, `.iso-overlays-svg` + уже имеют постоянный `overflow: visible` в `src/styles/plan.styles.ts:255-261` + — то есть контракт К3, применённый к ним по общему селектору + `[data-hp-live-viewbox]`, окажется для них no-op (временная запись/снятие + одного и того же эффективного значения). Это не расходится с К2 («осевший DOM + остаётся тем же, что до задачи») и не создаёт двойной работы для ревьюера + кода: единственные SVG, которые реально нуждаются в исправлении — безымянный + flat/floor SVG (`data-hp-live-viewbox="floor"`, без класса, `houseplan-card.ts:11513`) + и iso-камера (`class="plan-svg"`, `data-hp-live-viewbox="camera"`, та же + строка) — оба без явного `overflow`, то есть с вычисленным `hidden` по + умолчанию для corner SVG. Формулировка К3 «и другим целым сценовым SVG» + достаточна именно потому, что механизм в `paintLiveViewport` уже сейчас + обходит оба атрибута общим `querySelectorAll`, без адресного перечисления — + разработчику естественно расширить тот же общий селектор, а не подбирать + список вручную. +- **Проверено и не является находкой: `vactrail`/`radar-ranges`.** Эти два + корневых SVG тоже несут `data-hp-live-viewbox="camera"` + (`houseplan-card.ts:12412`, `radar-render.ts:46`), но уже имеют постоянный + `overflow: visible` (`devices.styles.ts:555-562`, `plan.styles.ts:1460-1467`) + — они не входят в число реально затронутых SVG и не требуют отдельного + упоминания в не-скоупе; попадание под общий селектор для них так же + безопасно, как и для iso-слоёв выше. +- **Проверено и не является находкой: механизм задачи.** Три ключевых + технических утверждения ТЗ сверены построчно с кодом на названном SHA и + подтвердились: (а) `needsViewBoxRefresh` в `src/live-viewport.ts:93-103` + проверяет только `now - anchor.at`, вызывается исключительно из + `scheduleHouseplanViewport`, которую planner вызывает только в ответ на + реальные события камеры (`_pan`/`_zoomAt`/колесо/жест — + `houseplan-card.ts:6462,6662,6951`), и никакой таймер не запускает + перерисовку сам по себе — значит held-дефект действительно может жить дольше + 100 мс, как заявлено в «Механизм»; (б) `setViewBox`/`setLayerProjection` + пишут DOM только на изменение строки, что соответствует заявлению AC1/AC5 о + «нет повторных записей на неизменный кадр»; (в) `docs/ARCHITECTURE.md` + описывает тот же бюджет (100 мс / 15 %) и тот же композиторный transform без + расхождений с ТЗ. +- **Проверено и не является находкой: fake-DOM тесты.** Ссылка ТЗ на + `test/live-viewport.test.mjs#L35` (строки/атрибуты, не пиксели) точна — + прочитан целиком фрагмент `fakeRoot`, он оперирует строковыми `style`/`attrs`, + подтверждая заявленный пробел в текущем оракуле. + +## Проверка обязательных разделов (PROCESS.md §7.1) + +Все обязательные разделы на месте: сценарий · что человек увидит до/после · +проблема · скоуп и не-скоуп · контракт поведения (К1–К6) · UX · модель данных и +миграция (нет) · i18n (нет) · критерии приёмки AC1–AC7 с доказательством +(`unit`/`smoke`/`mutation`/`gates`) · план автотестов · риски · откат · +release-артефакты. Плюс необязательные, но полезные разделы: затронутые файлы, +производительность/бюджеты, «принятые технические предположения». + +- **Сценарий/видимое поведение** — обе фразы персона-центричны (домочадец на + touch, admin на desktop), не используют терминов реализации («промежуточный + кадр» и «камера» — уже канонические термины подсистемы по `CANVAS.md`, не + изобретены задачей). +- **AC однозначны и проверяемы.** Каждый AC называет способ доказательства и, + где применимо, отрицательного свидетеля (AC1, AC2, AC5, AC6), как того + требует §7.1/§2.5. AC2 отдельно требует красного текущего `origin/dev` хотя + бы по одному направлению — это то самое условие «тест умеет падать» будущего + код-ревью, зафиксированное уже в самом ТЗ. +- **Скоуп/не-скоуп** согласован с `SCOPE.md` (J1/J3, touch — гарантированная + поверхность View по `TOUCH-SUPPORT.md`) и не расширяет задачу: явно исключены + новая камера-модель, изменение бюджета #531, новые настройки/лейблы, точный + touch для редакторов. +- **Технические предположения** оформлены отдельным блоком «можно менять на + ревью» с обоснованием (диагностический эксперимент 45 396 → 0 px) — это + ровно то разделение «продуктовое решает владелец, техническое — авторы», + которое требует §7.1. Открытых продуктовых вопросов, замаскированных под + технические, не найдено. +- **Риски** называют именно те места, где предположение может не сработать + (риск 1 — постоянный `overflow:visible` меняет osевший raster; риск 3 — iso + camera/floor не сливать в одну проекцию) — это честные, не декоративные + риски. + +## Что не проверялось и почему + +- Гейты `typecheck`/`test`/`build`/`golden` — не прогонялись: продуктового кода + ещё нет, предмет этапа — текст ТЗ, а не бинарник. +- Реализация мутанта AC6 и содержимое будущего `demo/smoke_live_pan_coverage.mjs` + — не существуют на этом этапе, поэтому не проверялись; их проверит код-ревью. +- Ручного/физического touch-тестирования не проводилось и не требовалось — + ТЗ прямо и корректно ограничивает это релизным гейтом, не доказательством + этой ветки (К6). + +## Вердикт + +Зелёный. Спецификация полна, однозначна, каждый AC проверяем и снабжён +доказательством и (где нужно) отрицательным свидетелем, технические +утверждения о механизме дефекта подтверждены прямым чтением кода на названном +SHA, а не приняты на веру. Продуктовых вопросов владельцу нет. Задача может +переходить в «Готово к разработке». + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `66a6485418c7` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `0d80c47762ca4c2f97c5de2c705b8128788dd307` + ``` + git log --all --format='%H %T' | grep 0d80c47762ca + ``` +- Тело issue: `41fec2c0272d4aa14d3bd714f7664c58f874613196541c14f746ebca2572005d` +- Вердикт конвейера: `green` · High 0