19 KiB
CODE-REVIEW-82-r2
- Issue: #82 «Плавное масштабирование плана: анимация zoom/fit/reset»
- Этап: code (PROCESS.md §2.7)
- Заход: r2 · блокирующих циклов израсходовано 0 из 4 (r1 был зелёным, бюджет не тратил, #227)
- Материал:
origin/dev...HEAD, HEAD =cc73252aa73007db3bd636b02eec3dd8ece871b2 - origin/dev на момент ревью:
e39cd3dfa1453be140ab809cd4344f9474bd0b2c - Диапазон: 13 коммитов,
8a18e812…cc73252a - Вердикт: зелёный · High: 0 · Medium: 0
Почему разбор полный, а не по дельте
После зелёного r1 (вердикт на SHA 779382af, комментарий 2026-08-30T15:51:14Z)
ветка была ребейзнута на ушедший вперёд dev (e39cd3df), а после этого
довешено ещё 6 коммитов, которых r1 не видел (a945ca47…b5324ce4, см. ниже).
Ребейз на ушедший вперёд dev — один из явных случаев, для которых инструкция
требует полный разбор, а не разбор по дельте (§7.2: после ребейза это другой
код). Поэтому весь диапазон origin/dev...HEAD перечитан и перепроверен
заново, включая часть, которую r1 уже одобрил, а не только новые 6 коммитов.
Отдельное наблюдение по процессу (не находка к коду, Low): комментарий
владельца «Финальный rebase… Единственный конфликт был в
docs/images/screenshots.json… исходный код и поведение камеры не
конфликтовали» описывает состояние ДО того, как в ветку попали ещё 6 коммитов
(a945ca47…b5324ce4), один из которых (64f54fd4) меняет реальное поведение
src/houseplan-card.ts (см. AC1/AC5 ниже), а не только тесты и скриншоты.
Ни один из этих 6 коммитов не описан в отдельном комментарии issue. Это не
блокирует ревью — полный разбор ниже покрывает эти коммиты по существу — но
статус-комментарии в этом раунде не отражали фактический diff на момент
возврата задачи в S7.
Что проверено и как
Дешёвые гейты
Validate на точном HEAD cc73252a зелёный
(https://github.com/Matysh/houseplan-card/actions/runs/33320914612), поэтому
tsc --noEmit, npm test (1642, см. предыдущий зелёный прогон) и
npm run build со сверкой копий бандла не перегонялись как отдельная
проверка. Тем не менее npm run bundle:sync (= tsc --noEmit && rollup -c +
раскладка) выполнен локально по ходу подготовки к golden — прошёл чисто,
dist/custom_components/.../frontend/demo/srv/assets идентичны после
сборки, git status остался чистым (значит закоммиченные бандлы уже
соответствуют исходникам этого HEAD).
node scripts/check-docs.mjs — выполнен (diff трогает src/**):
Documentation checks passed (7 files, 10 external links).
Golden (не покрывается перечнем «дешёвых» гейтов из шапки этого раунда)
Diff меняет рендер (camera-контроллер, 8 «унаследованных из dev» golden-baseline
и 4 doc-скриншота, включая правку screenshots.json). Запущен
npm run golden:verify — 147/147 passed, включая все 8 файлов из коммита
«test: accept inherited dev golden updates» (device-dialog-desktop-de,
device-dialog-mobile-ru, toggle-entity-dialog-mobile-ru,
device-ripple-color-popover-mobile-ru, tray-medium-group-en и т.д. — все
passed в выводе). Это подтверждает утверждение коммита не как заявление
автора, а как воспроизведённый прогон: текущий код рендерит ровно то, что
принято в качестве эталона, независимо от того, что сами PNG побайтово
отличаются от версии в origin/dev (шумовая перекодировка/иной прогон
захвата — не дефект).
Browser smoke — выбор по диффу
node scripts/smoke-select.mjs --base origin/dev --head HEAD: 49 прямых
совпадений, 33 слабые связи (общий символ вроде _modeTransitionBusy,
stopPropagation). Прогнаны:
- все смок-файлы, которые сам diff редактирует (значит, r1 их в этом виде
не видел):
smoke_canvas_frame,smoke_infinite_canvas,smoke_kiosk,smoke_kiosk_pan_lock— все правки только добавляютawaitна устаканивание камеры перед проверкой существующих инвариантов, поведение не меняют; - главный доказатель AC —
smoke_smooth_zoom(250 строк, покрывает AC1–AC12, AC14 согласно ТЗ §18); - обязательный минимум регрессий из ТЗ §18 и уже проверенный r1 набор:
smoke_zoom_out,smoke_editor_gestures,smoke_furniture,smoke_opening_preview,smoke_visual_continuity(#73),smoke_warm_remount,smoke_mode_transition(#101),smoke_isometric_contract,smoke_glow,smoke_glow_geometry_resilience.
Все 15 — зелёные (полный вывод каждого — все ключи true/OK).
Остальные 34 «прямых» совпадения из смок-select не прогнаны: они привязаны к
_baseVb/_viewOr/_cursorPt/_modeTransitionBusy — символам, которые
внутри строки почти не меняли поведение (только сигнатуры точек входа и
уборка opening-курсора при старте команды камеры, без изменения самой логики
черчения/декора/wall-thickness). Слабые связи (33) не прогонялись — общее имя
без прямого попадания в изменённый символ.
Что не прогонялось и почему
python -m pytest tests_backend— diff не трогаетcustom_components/**/*.py(только сгенерированный JS-бандл внутриcustom_components/.../frontend).npm run invariants— diff не трогает геометрию (нет правок рёбер комнат, записей толщины,layout,marker.space,open_spans); камера работает только сviewBox/zoom.- Performance-профиль/screencast для AC13 — не прогонялся. ТЗ §21 и матрица
AC13 сами относят это к предрелизному Full Performance гейту, не к
автотесту в цикле ревью; то же решение уже дважды принято (r1 spec-review и
r1 code-review). Косвенно AC13-риск (Glow/geometry rebuild на кадре)
проверен
glowAndStructuralFrameStayStableвsmoke_smooth_zoomиsmoke_glow_geometry_resilience— оба зелёные. test/single-source-numbers.test.mjs— механически входит вnpm test(уже зелёный по Validate). Смысловая проверка сделана вручную: zoom badge (src/houseplan-card.ts:11517,Math.round(this._zoom * 100)}%) — единственное место, где процент зума выводится пользователю; он читаетthis._zoomнапрямую, то же поле, которое_applyCameraTransitionFrameобновляет на каждом кадре. Второго источника числа не найдено.
AC1–AC14: доказательства
| AC | Статус | Доказательство |
|---|---|---|
| AC1 | Выполнен | smoke_smooth_zoom: buttonHasIntermediateFrame, fitHomeAndDoubleTapAnimate, farFitUsesTheSameTransition — все true; код _stepZoom/_resetZoom/_fitAll/_fitFar в src/houseplan-card.ts:6270-6345 строят target существующей fitView/_clampView и передают его в CameraTransitionController |
| AC2 | Выполнен | smoke_smooth_zoom.pinchIsDirectWithoutPostAnimation, smoke_editor_gestures (pinch/pan в редакторе) — зелёные; _zoomAt() (immediate-путь) не создаёт transition |
| AC3 | Выполнен | Прочитано кодом: единственные reactive _zoom/_view пишутся только в _applyCameraTransitionFrame/_settleCameraTransition/_zoomAt, все SVG/HTML/hit-слои читают их через общий рендер-путь; smoke_smooth_zoom.pointerdownFreezesPresentedFrame подтверждает, что pointerdown видит реально представленный кадр, а не устаревший target |
| AC4 | Выполнен | smoke_smooth_zoom.wheelRetargetsRunningTween (разные token, reverseFrom≈presentedBeforeReverse), wheelReversalKeepsAnchor (≤0.5px) |
| AC5 | Выполнен | smoke_smooth_zoom.exactLimitsAndFitAreNoops (active=false, saves=0 на fit/max/min); код _startCameraTransition (houseplan-card.ts:1207-1223) возвращает false без RAF/_saveZoom, если target совпадает с текущим или с уже бегущим target |
| AC6 | Выполнен | pointerdownFreezesPresentedFrame, projectionCancelsAndIsoSettlesExactly, hiddenCommitsTargetOnce — все true; код: _cancelCameraTransition вызван в _stagePointerDown, _commitSpace, _setProjection-пути (через _cancelModeTransition/adoption), _refitView, _pageVisibility('hidden'), config/layout adoption (b3abfc67) |
| AC7 | Выполнен | smoke_warm_remount, smoke_visual_continuity (#73) — зелёные без изменений поведения |
| AC8 | Выполнен | reducedMotionIsImmediate (true); _onMotionChange вызывает _cancelCameraTransition(true) при включении reduced motion, _startCameraTransition передаёт duration=0 при _reducedMotion |
| AC9 | Выполнен | buttonPersistsOnce/wheelStreamPersistsOnce (saves===1); _saveZoom() вызывается только из _settleCameraTransition, один раз на settle |
| AC10 | Выполнен | projectionCancelsAndIsoSettlesExactly — итоговый viewBox/zoom точно совпадает с независимо вычисленным _cameraTargetAt |
| AC11 | Выполнен | glowAndStructuralFrameStayStable (JSON.stringify до/после равны); smoke_glow, smoke_glow_geometry_resilience зелёные |
| AC12 | Выполнен | disconnectedCallback вызывает this._cameraTransition.dispose() (houseplan-card.ts:2745); unit viewport-transition.test.mjs — dispose()→cancel(false) не оставляет RAF (fake.callbacks.size===0 в тесте «cancel policies») |
| AC13 | Deferred (не автотест здесь) | То же решение, что в r1 spec- и code-review: закрывается предрелизным Full Performance/screencast гейтом, не циклом ревью. Косвенно риск закрыт glowAndStructuralFrameStayStable |
| AC14 | Выполнен | coldViewDoesNotLoadEditorRuntime (true, проверка через performance.getEntriesByType('resource')); src/houseplan-editor-runtime.ts не тронут диапазоном |
Дельта сверх того, что видел r1
Сверх c465eae8/54403704/b3abfc67 (уже одобрены r1 под именами
895b120e…779382af) в диапазон добавлено:
a945ca47—demo/docs/capture.mjs: ждётcard._cameraTransition?.activeперед снимком, иначе бросает. Тестовая инфраструктура, поведение продукта не меняет.64f54fd4— единственный коммит с правкой продуктового кода сверх того, что видел r1:_fitFar()/_fitAll()получили безусловныйthis.requestUpdate()перед_resetZoom(...)(houseplan-card.ts:5975, 5985). Причина:_showFar— обычное приватное поле, не Lit@state()(houseplan-card.ts:5755), поэтому раньше рендер после клика по far-hint зависел от того, что_resetZoom()синхронно меняла_zoom/_viewи тем самым просила update. С #82_startCameraTransitionможет вернутьfalseбез единого вызова хуков (когда camera-target уже совпадает с текущим) — и тогда без явногоrequestUpdate()смена_showFarне попала бы в рендер. Остальные правки того же коммита —awaitна устаканивание камеры вdemo/golden/harness.mjs,smoke_canvas_frame,smoke_infinite_canvas,smoke_kiosk— тестовая инфраструктура. Low, не блокирует: конкретно этот no-op-путь (far-hint нажат, когда camera-target уже равен текущему — например повторный клик до истечения предыдущего fit-far) не покрыт отдельной smoke-проверкой; существующие тесты far-hint (smoke_infinite_canvas.hintGoneAfterShow) всегда бьют по сценарию с реальным движением камеры. Правка защитная и дешёвая (лишнийrequestUpdate()не создаёт видимых артефактов), риск регрессии низкий, поэтому не блокирует зелёный вердикт.4c4399a7,4e24e266,9171644f— три последовательных пересъёма doc-скриншотов (08-room-card.png,09-device-info.png,06-device-editor.png) с обновлениемscreenshots.json;check-docs.mjsзелёный на финальном состоянии.b5324ce4— 8 golden-baseline изorigin/dev, подтверждены прогоном golden:verify выше (не только заявлением коммита).
Закрытие раунда r1
| Находка r1 | Чем закрыта | Где это видно |
|---|---|---|
55cf36d6/b3abfc67 fix: preserve structural adoption guard — по факту перестановка двух независимых операторов без наблюдаемой разницы поведения, замечание только к формулировке сообщения коммита |
Не блокировала r1, не блокирует и сейчас; независимо перечитан diff коммита b3abfc67 в этом раунде — подтверждаю: переставлен только порядок this._cancelCameraTransition(false) относительно очистки _geometryHistory/_devicePositionHistory/_pendingPhysicalWrites, эти операции не пересекаются по состоянию |
git show b3abfc67 -- src/houseplan-card.ts |
| AC13 закрыт только предрелizным Full Performance гейтом, не автотестом в цикле | Тот же прецедент подтверждён и на этом SHA — решение не изменилось | Раздел AC13 выше |
Унаследовано из r1
Поскольку ребейз на ушедший вперёд dev требует полного разбора (§7.2), из
r1 ничего не принято «на слово» без собственной перепроверки в этом
раунде — весь диапазон, включая уже одобренные r1 коммиты, перечитан заново
и его гейты (golden, smoke, check-docs, build) перезапущены в этом раунде.
Без повторной проверки принято только решение уровня продукта/процесса,
которое не является предметом код-ревью:
- продуктовая корректность ТЗ (структура §7.1, формулировки AC, отсутствие
недекларированных догадок) — из
SPEC-REVIEW-82-r1.md(docs/reviews/SPEC-REVIEW-82-r1.md, ТЗ на SHA141d79a9); код-ревью не переоценивает качество ТЗ, только соответствие ему; - прецедент «AC13 закрывается предрелизным Full Performance гейтом» — принят
ещё в
SPEC-REVIEW-82-r1.mdи переподтверждёнCODE-REVIEW-82-r1.md; в этом раунде не пересматривается заново, только сверено, что новых Glow/geometry-путей в дельте нет (см. выше).
Итог
High: 0. Medium: 0. Low: 3 (2 унаследованы и переподтверждены, 1 новая —
непокрытый smoke-тестом no-op-путь _fitFar()/_fitAll(), не блокирует).
Все AC1–AC12, AC14 подтверждены исполненным автотестом или разобраны по коду
с указанием строк; AC13 корректно отложен на предрелизный гейт по уже
принятому прецеденту. Вердикт: зелёный.