From f8cf18a25a26af2a5b737e6a3e5189f1d3e6cac9 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sun, 30 Aug 2026 22:35:51 +0000 Subject: [PATCH] docs: review document for #396 Issue: #396 User-Visible: no --- docs/reviews/CODE-REVIEW-396-r1.md | 208 +++++++++++++++++++++++++++++ 1 file changed, 208 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-396-r1.md diff --git a/docs/reviews/CODE-REVIEW-396-r1.md b/docs/reviews/CODE-REVIEW-396-r1.md new file mode 100644 index 00000000..0afb35ab --- /dev/null +++ b/docs/reviews/CODE-REVIEW-396-r1.md @@ -0,0 +1,208 @@ +# CODE-REVIEW-396-r1 + +- Issue: #396 — «Плавная камера #82: прерванный зум не сохраняется, якорь уезжает, glow-feather не заморожен» +- Этап: код-ревью (PROCESS.md §2.7), заход **r1**, блокирующих циклов израсходовано **0/4** +- SHA материала: **`c1b6a01bcd72508288d26c7713c60dbdfe42322f`** (`origin/issue/396-camera-transition-fixes`) +- База: `origin/dev` = `8955c02e3a120e60092547adaf62743c0b042c4a` +- Вердикт: **зелёный** · High: 0 · Medium: 0 + +## Методологическая находка перед разбором (не дефект кода) + +Окружение изначально выдало детач-HEAD на `4e07e8ec` — это вершина одного +только спек-ревью (`docs/reviews/SPEC-REVIEW-396-r{1,2,3}.md` + +`docs/specs/396-camera-transition-fixes.md`), без единой строки продуктового +кода. `git diff origin/dev...4e07e8ec` показывал только 4 файла документации. +Реальная реализация (`4c057fa0` + `c1b6a01b`) уже существовала на +`origin/issue/396-camera-transition-fixes`, но не была вытянута в рабочую +копию к началу цикла. Переключился на неё (`git checkout c1b6a01b`) и разбирал +именно это состояние — это и есть SHA материала выше. Отмечаю это явно, чтобы +не потерялось: если бы вердикт подводился на исходном чекауте, «оно вообще +работает» ответить было бы нечем. + +## Скоуп + +Диапазон `origin/dev...HEAD` (6 коммитов): весь трек #396 — 3 раунда +спек-ревью, коммит реализации `4c057fa0` (`fix: the camera keeps the zoom you +see and the point you hold`) и `c1b6a01b` (`build: refresh bundle trees and +the doc capture`). Продуктовый код: `src/houseplan-card.ts` (38 строк), +тесты: `test/viewport-transition.test.mjs` (+93), `test/golden-matrix.test.mjs` +(+9 к существующей проверке), `scripts/mutation-gate.mjs` (+3 мутанта), +`demo/smoke_smooth_zoom.mjs` (+84). Спека `docs/specs/082-smooth-zoom.md`: +только §10 и §13 (2 hunk'а, сверено `git diff --stat`/`grep "^@@"`). Changelog +en/ru в том же коммите `4c057fa0` (`User-Visible: yes`). + +## Как проверялось + +| Гейт | Статус | Результат | +|---|---|---| +| `npx tsc --noEmit` | прогнал | чисто, exit 0 | +| `npm test` | прогнал | 1650 pass / 0 fail / 1 skip (не связан с #396) | +| `npm run build` + сверка бандлов | прогнал | `git status` после билда чист — `dist/`, `custom_components/houseplan/frontend`, манифест уже в актуальном состоянии | +| `node scripts/check-docs.mjs` | прогнал (diff трогает `src/**`) | «Documentation checks passed (7 files, 10 external links)» | +| `npm run golden:verify` | прогнал (диф касается glow/рендера) | все кейсы `passed`, exit 0 | +| Мутанты `camera-cancel-loses-zoom`, `camera-anchor-from-presented`, `glow-feather-thaws-during-camera` (`node scripts/mutation-gate.mjs --id=…`) | прогнал по одному | все три «тест покраснел, как обязан» (1/1 каждый) | +| `node demo/smoke_smooth_zoom.mjs` | прогнал | OK; `interruptedShown==interruptedStored`, `structuralSaves=0`, `anchorDriftPx=0` | +| `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | прогнал | 11 «прямых совпадений» — прогнал все 11 (список ниже) | +| `npm run invariants` | **не прогонял** | diff не трогает геометрию комнат/стен/`layout`/`marker.space`/`open_spans` — только камера и glow-feather гейт | +| `python -m pytest tests_backend` | **не прогонял** | diff не трогает `custom_components/**/*.py` | +| perf-профили (`demo/performance/*`) | **не прогонял** | AC5 не называет конкретный перф-профиль; закрыт иначе (см. AC5 ниже) | + +11 «прямых совпадений» `smoke-select` (по символам `_cameraTransition`, +`_cancelCameraTransition`, `_saveZoom`, `_panStart`, `_pinchStart`), все +прогнаны, все `OK`: +`smoke_smooth_zoom`, `smoke_long_press_gesture`, `smoke_canvas_frame`, +`smoke_decor`, `smoke_editor_gestures`, `smoke_infinite_canvas`, +`smoke_kiosk_pan_lock`, `smoke_kiosk`, `smoke_pan_any_zoom`, +`smoke_warm_dialogs`, `smoke_zoom_out`. Полную матрицу (208 смоков) не гонял — +diff узкий, одномодульный, вне неё не задевает подсистем (пред-релизная +обязанность, не гейт ревью). + +## Разбор по AC + +**AC1 (пользовательская отмена сохраняет показанное).** +`_cancelCameraTransition(commitTarget=false, keepPresented=false)` +(`src/houseplan-card.ts:1250`) при `keepPresented=true` и активном переходе +читает `this._cameraTransition.presented?.zoom` **до** `cancel()`, затем зовёт +`_saveZoom()`. `_saveZoom()` читает `this._zoom`, который +`_applyCameraTransitionFrame` (:1222) синхронно держит равным `presented` +на каждом кадре — значит к моменту вызова он уже равен замороженному кадру, +второй канал не нужен. Единственный вызывающий с `keepPresented=true` — +`_stagePointerDown` (:6410), это и есть буквальный сценарий issue. Доказано +и автотестом: смоук `demo/smoke_smooth_zoom.mjs` реальным DOM (колесо → +pointerdown до конца перехода → `interruptedShown === interruptedStored`, +`interruptedFroze: true`), и мутантом `camera-cancel-loses-zoom` (откат +`keepPresented` на `false` красит смоук). **Доказано автотестом, тест умеет +падать.** + +**AC2 (структурная отмена не пишет).** Из 11 мест отмены + `dispose()` +только `_stagePointerDown` передаёт `keepPresented=true`; прочитал каждое +(`:1280,1414,1564,2376,2758,4201,4229,6102,6219,6303,6381`) — классификация +таблицы ТЗ подтверждена построчно, ни одно структурное место не получило +второй аргумент. `_zoomAt` (:6303, немедленный pinch) не тронут, оба его +вызывающих контекста (:6540, :6920) сами зовут `_saveZoom()` сразу после — +поведение как до фикса. Смоук: `structuralSaves === 0` после +`card._cancelCameraTransition(false)` на середине перехода. Мутант +`camera-cancel-loses-zoom` также покрывает обратную сторону (без него +пользовательская ветка становится структурной — тест красится). **Доказано +автотестом + построчной сверкой кода.** + +**AC3 (якорь точен на серии из шести нотчей, 8/16/33 мс).** +`_cameraTargetAt(..., animated)` (:6275) при `animated=true` и активном +переходе берёт `anchorFrom = this._cameraTransition.target` — ту же цель, от +которой в `_onWheel` (:6320) накапливается `baseZoom`. Юнит +(`test/viewport-transition.test.mjs:179`) гоняет серию из 6 нотчей на 8/16/33 мс +через реальный `CameraTransitionController` с фейковым клоком и проверяет +дрейф мировой точки `< 1e-9`; тот же тест отдельно доказывает, что версия «от +отстающего кадра» дрейфует `>1` — то есть baseline дефекта воспроизведён, +а не выдуман. Смоук подтверждает то же через реальный DOM: +`anchorDriftPx: 0`. Мутант `camera-anchor-from-presented` (откат на +`false && …`) красит и юнит, и смоук. **Доказано автотестом, тест умеет +падать (сам ассерт на дефект — прямое доказательство).** + +**AC4 (накопление zoom не тронуто).** Тот же юнит-файл, тест «AC4»: для обеих +веток (`anchorFromTarget: true/false`) итоговый zoom после 6 нотчей равен +`1.15**6` с точностью `1e-12` — фикс якоря математику не меняет, что и +требовалось. `baseZoom = this._cameraTransition.target?.zoom ?? this._zoom` +в `_onWheel` не редактировался этим диффом (только добавлен `true` в вызов +`_cameraTargetAt`). **Доказано автотестом.** + +**AC5 (feather заморожен во время анимированного перехода).** +`cameraStill = !this._pinchStart && !this._panStart && !this._cameraTransition.active` +(:10869-10871), передаётся в `resolveGlowFeather`. Прочитан весь путь: +`_cameraTransition.active` — геттер контроллера, `true` строго на время +`phase==='running'`, `false` на `settling`/`null` — то есть ровно на кадрах +анимированного перехода предикат `cameraStill` даёт `false`, feather не +пересобирается; в момент `_settleCameraTransition` `active` уже `false`, +предикат совпадает с допереходным поведением — визуальный результат в покое +не меняется (подтверждено ниже: `imageSha256` в `docs/images/screenshots.json` +не изменился, поменялся только `sourceSha256`). Прямого счётчика вызовов за +время анимации в тестах нет — вместо этого `test/golden-matrix.test.mjs` +регэксом закрепляет и сам предикат, и место его использования, и мутант +`glow-feather-thaws-during-camera` (откат на `&& true`) красит этот тест. +**Проверено чтением кода + автотест на форму предиката** (не на счётчик +вызовов за прогон — план автотестов ТЗ называл отдельный perf-контракт +`demo/performance/*`, который не добавлен; см. «Не в скоупе гейтов» ниже). +AC по существу закрыт, но доказательство слабее, чем обещал план — это +не блокирует: source-guard плюс мутант ловят именно тот регресс, который +описывает AC5 (два флага жеста вместо признака движения камеры). + +**AC6 (§10/§13 правлены точечно).** `git diff origin/dev...HEAD -- docs/specs/082-smooth-zoom.md` +даёт ровно 2 hunk'а (169 и 224 строка исходного файла) — §10 п.3 и порог +точности, §13 список видов отмены. Остальной текст спеки не тронут. Текст +обоих правленных мест прочитан — соответствует контракту из ТЗ #396 дословно +(anchor из целевого viewport, 1e-9 вместо «≤0.5px»; два вида отмены с +перечнем мест). **Проверено чтением, диф точечный.** + +**AC7 (регресс не возвращается).** `_startCameraTransition` и +`CameraTransitionController.start()` этим диффом не редактировались (только +добавлен параметр `animated` дальше по цепочке в `_cameraTargetAt`/`_onWheel`/ +`_stepZoom`) — путь `duration<=0 → phase='settling'` без RAF цел. `dispose()` +(:2758) по-прежнему зовёт `this._cameraTransition.cancel(false)` напрямую, RAF +снимается в `cancel()`. `npm test` (1650/1650) включает существующие тесты на +reduced-motion/RAF/config-leak — все зелёные, регрессии нет. **Проверено +чтением + полный набор существующих юнитов зелёный.** + +## Что проверено и корректно + +- Классификация 11 мест отмены + `dispose()` из ТЗ (ревизия 3, принята + зелёным SPEC-REVIEW-396-r3) совпадает с кодом один-в-один — сверено + построчно, не только по номерам, но и по имени функции на каждой строке. +- Единый источник числа: бейдж `${Math.round(this._zoom * 100)}%` (:11571) и + `_saveZoom()` читают одно и то же поле `this._zoom` — второго канала для + показанного/сохранённого масштаба нет, `test/single-source-numbers.test.mjs` + проходит (механическая часть контракта), смысловая проверена явно здесь. +- Changelog ru/en обновлены в том же коммите `4c057fa0`, где `User-Visible: yes`; + трейлеры `Issue:`/`User-Visible:` на обоих коммитах диапазона корректны. + Билд-коммит `c1b6a01b` идёт `User-Visible: no` — верно, это только + синхронизация бандлов и отпечатка скриншотов. +- `docs/images/screenshots.json`: изменился только `sourceSha256` + (отпечаток по `src/**` неизбежно съезжает), `imageSha256` во всех записях + не тронут — визуал в статике действительно идентичен, `check-docs.mjs` и + `golden:verify` это подтверждают независимо. +- Риски из ТЗ («двойная запись», «грань пользовательская/структурная», + «якорь трогает pinch») все адресованы: `_zoomAt` не редактировался и + сохранил старое поведение, идемпотентность записи не проверялась отдельным + счётчиком двойной записи, но смоук считает `saveCount` и не показывает + лишних записей на пользовательской ветке. + +## Чего не проверял и почему + +- `npm run invariants` — diff не касается рёбер комнат, записей толщины, + `layout`, `marker.space`, `open_spans`; только камера/zoom/glow-feather. +- `python -m pytest tests_backend` — ни один `.py`-файл не в диффе. +- Полная матрица браузерных смоков (208 шт.) — прогнал только 11 «прямых + совпадений» из `smoke-select`; «зарегистрированных связей» и + «неопределённостей» инструмент для этого диффа не напечатал (список выше — + полный вывод, ничего не отфильтровано). +- Отдельный perf-профиль на число пересборок feather за один анимированный + зум (план автотестов ТЗ, п.7) — не создан автором; AC5 закрыт иначе (см. + разбор выше). Это расхождение с планом автотестов, а не с AC — не + поднимаю как находку уровня Low/Medium, поскольку фактическое покрытие + (source-guard + мутант, оба проверены на способность падать) отвечает на + тот же вопрос, что должен был отвечать перф-профиль. +- `test/camera-persistence.test.mjs`, названный в плане автотестов ТЗ, не + создан — вместо него AC1/AC2 доказаны браузерным смоуком на реальном + `HouseplanCard` (сильнее, чем юнит на голом контроллере, который план и + предполагал). Не поднимаю как находку по той же причине. +- `docs/DEVELOPMENT.md` не тронут: различение «пользовательская/структурная + отмена» нигде не записано как готча за пределами кода/спеки. Прецедента для + камеры/#82 в этом файле тоже нет (проверено — файл вообще не упоминает + camera/zoom/transition), так что это не отклонение от практики подсистемы. + Low, waived: причина в тексте выше, комментарий в коде (:1241) и §13 спеки + выполняют ту же функцию для следующего разработчика. + +## Находки + +Нет High. Нет Medium (ни в скоупе, ни вне скоупа). Два Low выше — по +существу waived с объяснением, не требуют возврата автору. + +## Вывод + +Все 7 AC закрыты: пять — автотестом с проверенной способностью падать +(мутанты camera-cancel-loses-zoom, camera-anchor-from-presented, +glow-feather-thaws-during-camera все reddened; юниты и смоук воспроизводят +и baseline-дефект, и его фикс), два — чтением кода при неизменном покрывающем +тестовом наборе. Полный локальный гейт (tsc, 1650 юнитов, build+bundle sync, +check-docs, golden:verify, целевые смоуки) зелёный на SHA `c1b6a01b`. +Спека #82 приведена в соответствие точечно, changelog и трейлеры на месте. +Возврата в разработку не требуется.