docs: review document for #396

Issue: #396
User-Visible: no

docs/reviews/SPEC-REVIEW-396-r1.md

@@ -0,0 +1,252 @@
+# SPEC-REVIEW-396-r1
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/396
+- **ТЗ под ревью:** `docs/specs/396-camera-transition-fixes.md`, коммит `4e0a30a78161a4ff59679454d1f8b5c471e10008` (HEAD ветки `issue/396-camera-transition-fixes`)
+- **Этап:** spec (PROCESS.md §2.4)
+- **Трек:** обычный (без метки `small`/`trivial`)
+- **Заход:** r1 · блокирующих циклов ревью ТЗ израсходовано 0 из 4 (до этого вердикта)
+
+## Скоуп ревью
+
+Issue #396 переносит три находки внешнего аудита (`AUDIT-2026-08-31-v1700beta1.md`,
+в репозитории не лежит — внешний документ, ссылка не проверяется) в одно ТЗ,
+поскольку все три касаются одной подсистемы (камера-переход из #82):
+
+- **B1 (High)** — прерванный пользователем зум не сохраняется: регресс против
+  v1.69.0;
+- **B2 (High)** — якорь под курсором вычисляется от отстающего кадра при
+  быстрой серии wheel-событий;
+- **M2 (Medium)** — glow-feather не заморожен во время нового camera tween.
+
+Ревьюер не видел устных пояснений автора; материал — только issue #396, ТЗ
+`docs/specs/396-camera-transition-fixes.md` и код на `origin/dev`/ветке задачи.
+Продуктового кода к этому изменению ещё нет (ветка содержит один doc-коммит),
+поэтому единственный вопрос ревью — проверить каждое фактическое утверждение
+ТЗ о поведении `src/viewport-transition.ts` и `src/houseplan-card.ts` построчно
+против реального кода, ровно так же, как это требует PROCESS.md §2.4 и
+прецедент SPEC-REVIEW-197-r1.
+
+## Как проверялось
+
+1. Прочитаны целиком `docs/SCOPE.md`, `PROCESS.md` (действующая редакция,
+   §1, §2.4, §2.5, §2.10, §7.1, §7.2), `AGENTS.md`.
+2. Прочитано тело issue #396 (единственный комментарий — сам текст issue,
+   других комментариев нет).
+3. Прочитан `docs/specs/082-smooth-zoom.md` целиком — канонический документ
+   подсистемы, поскольку #396 правит его §10 и §13 (AC6).
+4. Прочитан весь текст ТЗ `docs/specs/396-camera-transition-fixes.md` (206
+   строк), сверены обязательные разделы §7.1 PROCESS.md — присутствуют все:
+   сценарий, что человек увидит до/после, проблема и контракты по пунктам,
+   скоуп/не-скоуп, UX, модель данных и миграция, i18n, критерии приёмки
+   AC1–AC7 (у каждого назван способ доказательства), план автотестов, риски,
+   откат, release-артефакты, блок принятых технических предположений.
+5. Построчно сверены все технические утверждения ТЗ с фактическим кодом:
+   - `src/viewport-transition.ts` (212 строк) прочитан целиком —
+     `CameraTransitionController.cancel(commitTarget)` (`:189-200`),
+     `dispose()` (`:202`), `_settle()` (`:204-211`);
+   - все вызовы `_cancelCameraTransition(...)` и `_cameraTransition.cancel/dispose`
+     в `src/houseplan-card.ts` пересчитаны поимённо:
+     `grep -n "_cancelCameraTransition(false)\|_cancelCameraTransition(true)\|_cameraTransition\.\(cancel\|dispose\)" src/houseplan-card.ts`;
+   - каждый найденный вызов прочитан в контексте функции, которая его содержит
+     (`_onMotionChange`, `_startCameraTransition`, `_cancelModeTransition`,
+     `_adoptMode`, `_commitSpace`, `_adoptStructuralResponses` ×2, `_applyView`,
+     `_refitView`, `_zoomAt`, `_restoreZoom`, `_stagePointerDown`,
+     `disconnectedCallback`);
+   - оба вызова `_zoomAt(...)` (`:6517`, `:6897`, внутри pinch-путей
+     `_stagePointerMove`/`_touchGesture...`) прочитаны с окружением, чтобы
+     проверить, действительно ли за ними сразу следует `_saveZoom()`;
+   - `_cameraTargetAt` (`:6262-6279`) и `cameraTargetAtAnchor` (`viewport-transition.ts:94-116`)
+     прочитаны вместе, чтобы проверить claim B2 (мировая точка считается от
+     `this._cameraState()`, то есть от представленного кадра, независимо от
+     того, какой `targetZoom` передал вызывающий код);
+   - `resolveGlowFeather(...)` вызов на `:10845-10847` сверен с цитатой ТЗ
+     (`:10842-10847`) — совпадает дословно.
+6. Гейты (`typecheck`/`test`/`build`) не прогонялись — на этапе ревью ТЗ
+   продуктового кода не существует (только doc-коммит), что вне скоупа этапа
+   (PROCESS.md §2.4/§8).
+
+## Находки
+
+### High-1 — центральное фактическое утверждение B1 («семь мест») не совпадает с реальными вызовами и не покрывает главный сценарий issue
+
+**Файл:** `docs/specs/396-camera-transition-fixes.md`, раздел «(1) B1 — прерванный
+зум не сохраняется» и раздел «Скоуп / не-скоуп».
+
+ТЗ утверждает: «`_cancelCameraTransition(false)` зовут семь мест: `:1267`,
+`:1401`, `:1551`, `:4188`, `:4216`, `:6089`, `:6206` — pointerdown по сцене,
+смена пространства, `_refitView`, adoption, reload, `_zoomAt`, disconnect.»
+(позиционное соответствие семи чисел семи описаниям в том же порядке).
+
+Фактическая проверка (`grep -n "_cancelCameraTransition(false)\|_cancelCameraTransition(true)\|_cameraTransition\.\(cancel\|dispose\)" src/houseplan-card.ts`
+на HEAD ветки задачи) даёт **десять** мест с `commitTarget=false` плюс одно
+место, где котроллер отменяется напрямую, минуя обёртку:
+
+| Строка | Функция | Что это на самом деле |
+|---|---|---|
+| `:1267` | `_startCameraTransition`, no-op ветка (`sameCameraState(current, target)`) | не «pointerdown», а ранний выход при запросе уже показанного состояния |
+| `:1401` | `_cancelModeTransition` | общий helper для смены mode/space, не «смена пространства» отдельно |
+| `:1551` | `_commitSpace` | это и есть настоящая «смена пространства» — но в списке ей приписана строка `:1401` |
+| `:4188` | `_adoptStructuralResponses` (config adopt) | «adoption» — совпадает |
+| `:4216` | `_adoptStructuralResponses` (layout adopt) | второе «adoption», в списке названо «reload» |
+| `:6089` | `_applyView` | не «`_zoomAt`» — другая функция |
+| `:6206` | `_refitView` | resize, не «disconnect» |
+| `:6283` | `_zoomAt` (pinch, immediate) | **отсутствует в списке семи** |
+| `:6360` | `_restoreZoom` | **отсутствует в списке семи**, не упомянута нигде в ТЗ |
+| `:6387` | `_stagePointerDown` | **отсутствует в списке семи** — это и есть буквальный «pointerdown по сцене» из пользовательского сценария issue |
+| `:2745` | `disconnectedCallback` | вызывает `this._cameraTransition.dispose()` напрямую, минуя `_cancelCameraTransition` — не через строку `:6206`, как подразумевает список |
+
+Т.е. ни одна из строк реального `_stagePointerDown` (`:6387`) и реального
+`_zoomAt` (`:6283`) не входит в перечисленные семь чисел, хотя проза ТЗ прямо
+называет «pointerdown по сцене» и «`_zoomAt`» в качестве двух из этих семи.
+Disconnect вообще не проходит через `_cancelCameraTransition` — там отдельный
+вызов `.dispose()` на `:2745`.
+
+**Проверено исполнением/чтением, что `:6387` — это настоящий воспроизводящий
+сценарий из issue.** `_stagePointerDown` (`houseplan-card.ts:6386-6446`) вызывает
+`this._cancelCameraTransition(false)` первой строкой и не содержит ни одного
+вызова `_saveZoom()` внутри себя или в путях, которые гарантированно
+выполняются после простого клика (не переросшего в pan/pinch). Ровно это —
+«колесо/кнопка «+» → сразу клик по плану» из раздела «Пользовательский
+сценарий» B1. Сопутствующая проверка: `_zoomAt` (`:6283`) технически тоже
+вызывает `_cancelCameraTransition(false)`, но оба его реальных вызывающих
+контекста (`:6517` — pinch move, `:6897` — touch multitouch move) сразу же
+следом вызывают `_saveZoom()` — то есть через `_zoomAt` состояние фактически
+не теряется, вопреки тому, что список подписывает его как один из семи
+«теряющих» вызовов.
+
+**Почему это блокирует, а не опечатка в номерах строк.** Раздел «Скоуп /
+не-скоуп» ТЗ перечисляет затронутые функции: «`_startCameraTransition`,
+`_cameraTargetAt`, `_onWheel`, `_stepZoom`, `_cancelCameraTransition`,
+glow-гейт» — и **не называет `_stagePointerDown` ни разу**. DoR (PROCESS.md
+§2.5) требует «перечислены затронутые файлы и модули» именно для того, чтобы
+разработчик не должен был заново открывать код в поисках места правки. Если
+разработчик читает этот список как объявление границы работы (а не как
+приглашение заново обыскать весь файл), правка `_stagePointerDown` не
+попадёт в реализацию — а именно там лежит буквальный баг из пользовательского
+сценария issue. Раздел «Риски» ТЗ («Тонкая грань «пользовательская против
+структурной»») сам ссылается на «список мест перечислен в спеке #82 §13» как
+на средство защиты от ошибки классификации — но источник (список из этого же
+ТЗ) неточен, то есть заявленная мера риска опирается на тот же дефектный
+список.
+
+Смягчающее обстоятельство: план автотестов (шаг 5 browser smoke, «Колесо →
+pointerdown по плану до окончания перехода → перезагрузка карты →
+восстановленный зум равен показанному до клика (AC1 на реальном DOM)») сам по
+себе устроен так, что реализация, не тронувшая `_stagePointerDown`, эту
+проверку не пройдёт — то есть дефект будет пойман на код-ревью. Но это
+означает лишний цикл там, где спек-ревью для того и существует, чтобы поймать
+это раньше и дешевле (тот же аргумент, что в SPEC-REVIEW-197-r1 High-1:
+разработчик рискует чинить не тот код, опираясь на неверную фактическую базу
+раздела).
+
+**Что нужно поправить.** В разделе «(1) B1» — заменить список из семи чисел
+на фактическую сверку (реальных мест десять плюс `disconnectedCallback`,
+использующий `.dispose()` напрямую), явно отметить, какие из них
+пользовательские (как минимум `:6387`), а какие структурные, и без
+двусмысленности показать, что `_zoomAt` (`:6283`/`:6517`/`:6897`) уже не
+теряет состояние благодаря существующему `_saveZoom()` сразу после — то есть
+не требует правки по контракту B1. В разделе «Скоуп / не-скоуп» — добавить
+`_stagePointerDown` в список затронутых функций (и, при необходимости,
+`_restoreZoom`, если ревью реализации решит, что он тоже нуждается в явной
+классификации).
+
+## Что проверено и корректно
+
+- **B1 — контракт и AC1/AC2 сформулированы верно**, несмотря на дефектный
+  список строк: `CameraTransitionController.cancel(false)`
+  (`viewport-transition.ts:189-200`) действительно не вызывает `_hooks.settled`
+  и, следовательно, никогда не проходит через `_settleCameraTransition` →
+  `_saveZoom()` (`houseplan-card.ts:1232-1244`) — регресс реален и подтверждён
+  чтением обоих файлов независимо от того, какие конкретно строки его
+  проявляют. Различение «пользовательская отмена / структурная отмена»
+  (таблица в разделе «(1)») продуктово согласовано с уже принятым §11
+  `docs/specs/082-smooth-zoom.md` («Pointerdown, который начинает pan, pinch,
+  draw, drag или selection, сначала фиксирует представленный кадр и отменяет
+  tween без скачка» — то есть #82 уже требовал этого поведения, #396 первым
+  делает его явным контрактом с AC).
+- **B2 подтверждён чтением и математически обоснован.** `_cameraTargetAt`
+  (`houseplan-card.ts:6262-6279`) всегда строит якорь от
+  `this._cameraState()` (`:6273`) — то есть от представленного, а не целевого
+  кадра — независимо от того, что `_onWheel` (`:6294-6308`) уже правильно
+  берёт `baseZoom` из `this._cameraTransition.target?.zoom ?? this._zoom`
+  (`:6301`). Это ровно расхождение «масштаб — из target, точка — из
+  presented», которое ТЗ описывает как корень утечки якоря. Контрактная
+  формулировка («масштаб и мировая точка берутся из одного и того же
+  состояния») корректно закрывает разрыв, а замечание про `_zoomAt`
+  («там tween заведомо отменён, представленное и есть текущее») подтверждено:
+  `_zoomAt` (`:6282-6292`) вызывает `_cancelCameraTransition(false)` первой
+  строкой, поэтому к моменту вычисления якоря активного перехода уже нет и
+  presented/target совпадают по определению.
+- **M2 подтверждён дословным совпадением цитаты.** `resolveGlowFeather(...)`
+  вызов на `houseplan-card.ts:10845-10847` с третьим аргументом
+  `!this._pinchStart && !this._panStart` совпадает с цитатой ТЗ буквально —
+  признак «камера движется» не учитывает новый camera tween, только pinch/pan
+  gesture state.
+- **Правка `docs/specs/082-smooth-zoom.md` §10/§13 обоснована.** Прочитан
+  документ целиком (386 строк): §10 п.3 действительно говорит «в
+  представленном viewport», а порог ниже — «не более 0.5 CSS px», то есть
+  спека #82 сама себе противоречит после реализации B2-бага (п.3 выполнен,
+  порог нарушен) — формулировка AC6 «привести в соответствие» корректна и не
+  требует продуктового решения владельца (чисто согласование текста с уже
+  принятым поведенческим контрактом п.2 §10).
+- **Структура ТЗ и обязательные разделы (PROCESS.md §7.1)** — все на месте,
+  включая i18n («Новых строк нет» — верно, ни один из трёх контрактов не
+  добавляет UI-текст), модель данных/миграция («формат `LS_ZOOM` не меняется»
+  — верно, персист остаётся тем же ключом/структурой), touch (пункт «Немедленный
+  `_zoomAt` (pinch) продолжает работать от представленного состояния» —
+  корректно определяет границу с touch-контрактом), откат (точечный, три
+  независимых правки, без миграции данных — согласуется с характером задачи).
+- **Трек.** Обычный трек (не `small`) обоснован по факту: три независимых
+  находки на одной подсистеме, правка канонического документа #82 — критерий
+  §5 PROCESS.md («нет нового UX-контракта» технически спорен, но задача явно
+  не «одна поверхность» и трогает канон #82, что само по себе исключение из
+  лёгкого трека).
+- **Открытых продуктовых вопросов к владельцу не осталось и не должно
+  остаться** — единственный найденный пробел (High-1) технический (какие
+  именно строки/функции меняются), а не продуктовый: ни «что видит
+  пользователь», ни «объём видимых изменений в issue» здесь не решаются.
+  Технический спор автора и ревьюера решается вердиктом, не владельцем
+  (PROCESS.md §7.1).
+- **Мутанты названы конкретно** (`camera-anchor-from-presented`,
+  `camera-cancel-loses-zoom`, `glow-feather-thaws-during-camera`) и каждый
+  указывает, какой юнит должен покраснеть — соответствует правилу «тест умеет
+  падать» (PROCESS.md §2.7, применимо и к дизайну теста на этапе ТЗ).
+
+## Чего не проверял
+
+- Код продукта не существует на этой стадии (только ТЗ) — гейты
+  `typecheck`/`test`/`build`/`check-docs`/`invariants`/смоки/`golden:verify`
+  неприменимы и не запускались; это нормально для этапа ТЗ, а не пропуск.
+- Не проверял содержимое внешнего аудита `AUDIT-2026-08-31-v1700beta1.md` —
+  файл не лежит в репозитории (только ссылка на него в теле issue), поэтому
+  его точные измерения (14.5/16.1/10.3 px и т.п.) не перепроверялись
+  инструментально; для ревью достаточно того, что оба бага (B1, B2)
+  независимо подтверждены чтением реального кода на `origin/dev`.
+- Не прогонял `test-build/viewport-transition.js` с фейковым клоком, на
+  который ссылается ТЗ («переход 1.0 → 1.15 за 220 мс, обрыв на 120 мс →
+  `settled` вызван 0 раз, `presented.zoom = 1.1413`») — подтвердил тот же
+  вывод логическим чтением `CameraTransitionController.cancel()`
+  (`viewport-transition.ts:189-200`: `commitTarget=false` выходит до вызова
+  `this._hooks.settled`), числовое значение `1.1413` не пересчитывал.
+- Не искал дополнительные вызовы `_cancelCameraTransition`/`_cameraTransition.cancel`
+  за пределами `src/houseplan-card.ts` (например, в `src/houseplan-editor-runtime.ts`)
+  — раздел «Скоуп» ТЗ ограничивает камеру ядром `houseplan-card.ts`, и
+  `grep -n "_cameraTransition\b" src/houseplan-editor-runtime.ts` не выполнялся;
+  при исправлении High-1 автору стоит перепроверить и этот файл на всякий
+  случай, хотя #82 §19 явно оставляет бизнес-логику zoom в core.
+
+## Вердикт
+
+Красный. High: 1 (B1 — список мест «`_cancelCameraTransition(false)`»
+фактически неверен и не включает `_stagePointerDown`/`_zoomAt`, а раздел
+«Скоуп» не называет `_stagePointerDown` как затрагиваемую функцию — риск, что
+буквальный пользовательский сценарий issue останется незачинённым). Medium: 0.
+Контракты B1/B2/M2 и AC1–AC7 по существу верны и не требуют продуктового
+решения владельца; правка текстовая — исправить раздел «(1) B1» и «Скоуп /
+не-скоуп» по фактической сверке выше, без изменения AC. После правки —
+повторный заход по дельте (PROCESS.md §2.10): проверить перечень мест и то,
+что `_stagePointerDown` явно назван местом правки, остальные разделы
+(B2/M2, AC3-AC7, риски §10/§13) наследуются без повторной проверки.
+
+**Вердикт: красный · заход r1 · блокирующих циклов 1/4 · High: 1 · Medium: 0
+→ в задаче · Документ: docs/reviews/SPEC-REVIEW-396-r1.md**