From baf0136d9a051c0eb98e6cdeaae3106c5c78fd27 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Thu, 1 Oct 2026 21:00:43 +0000 Subject: [PATCH] docs: review document for #762 Issue: #762 User-Visible: no --- docs/reviews/CODE-REVIEW-762-r1.md | 225 +++++++++++++++++++++++++++++ 1 file changed, 225 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-762-r1.md diff --git a/docs/reviews/CODE-REVIEW-762-r1.md b/docs/reviews/CODE-REVIEW-762-r1.md new file mode 100644 index 00000000..946e32f3 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-762-r1.md @@ -0,0 +1,225 @@ +# CODE-REVIEW-762-r1 + +Issue: #762 · «Warm remount и отложенный режим: хвосты после #756» +Трек: ask · Заход: r1 · Блокирующих циклов: 0/4 +Материал: `11beca1ae80a87787ebb06fbc52f4d4ac2e86ee8` (ветка `issue/762-warm-mode-adoption`, рабочая копия на этом SHA) +Validate на материале: success, https://github.com/Matysh/houseplan-card/actions/runs/36923307171 (подтверждено `gh run view` — `headSha` совпадает) + +## Материал раунда + +Три коммита `origin/dev..HEAD`: + +- `2ec78227` fix: keep warm editor adoption subordinate to user navigation (#762) — `Issue: #762`, `User-Visible: yes`, оба changelog в этом же коммите. +- `0cd6a299` test: await warm navigation request before checking its authority (#762) — `User-Visible: no`. +- `11beca1a` refactor: keep room warm-draft construction in the lazy editor (#762) — `User-Visible: no`. + +Примечание по непрерывности материала: в треде задачи упоминается более ранний +SHA `adc2d7c520d9…` («Реализация #762», разбор отказа Validate). Это не то же +дерево формально, но тот же логический коммит-набор — ветка была приведена к +ушедшему вперёд `dev` (после #740/#662), и три текущих коммита воспроизводят +тот же диф с новыми хешами после ребейза. Содержимое, число коммитов и +сообщения совпадают с описанным автором; отказ Validate (`adc2d7c5`) был +перформанс-флуктуацией ресайза, не связанной с продуктовым кодом задачи, и не +тратил цикл ревью — это зафиксировано автором и вынесено в #778. На `11beca1a` +Validate зелёный, код-ревью не начиналось ранее — цикл действительно 0/4. + +## Скоуп проверки + +ТЗ (`## ТЗ` в теле issue) прошло ревью ТЗ `r1` зелёным +(`docs/reviews/SPEC-REVIEW-762-r1.md`, тот же issue). Код-ревью проверяет: пять +симптомов (AC1–AC5) + соседние пути AC6, трейлеры/changelog, гейты по объёму +задачи, маршрут `route: fix` по риску изменённых участков (perf-токены +`requestAnimationFrame`/`getBoundingClientRect`/`ResizeObserver` в +`houseplan-card.ts:4084,4090,4096,4097,6417`). + +## Как проверялось + +### Прочитано построчно (не по заявлению) + +- `src/warm-mode-adoption.ts` целиком: `resumeWarmMode` теперь сохраняет + `_viewModeSnap` вокруг `commit()` (строка `if (host._warmVp && host._mode + === mode) host._viewModeSnap = viewModeSnap;`) — закрывает симптом 1 + (`_setMode` больше не подменяет возвратный Просмотр временной камерой + редактора). Защита камеры (`warmCameraUnchanged`) больше не зависит от + `_warmRevivePending` — закрывает симптом 2 (камера без диалога). +- `src/card-runtime.ts`: `warmCameraUnchanged` вынесена в общий модуль (была + приватной `sameView` в двух местах), используется и в `_requestMode` + (houseplan-card.ts), и в `resumeWarmMode` — одна реализация сравнения + камеры, не два источника правды для одного и того же факта. +- `src/houseplan-card.ts`: новый `_cancelPendingWarmMode()` — единая точка + отмены (`_pendingNavMode = null`, `_releaseWarmRefit`, инкремент + `_editorModeRequest`, discard диалога). Вызывается до `await + this._ensureEditorRuntime()` в `_requestMode` (когда `!adopt`), в + `_setMode` (когда `!warm`), в `_commitSpace` при смене пространства и в + `_leaveCardRoute`. Это закрывает симптомы 4 и 5: новая команда/смена + пространства синхронно обнуляет pending-намерение ДО того, как асинхронный + resume успеет его прочитать, независимо от порядка разрешения + `can_write`/загрузки модуля редактора. +- `_resumePendingNavMode`: явно отказывает (`_serverCanWrite === false || + this._kiosk` → `_cancelPendingWarmMode(); return false`) вместо входа в + редактор — соответствует контракту «запрет или kiosk — нет» из AC1. + Унифицирован путь — и загруженный, и отложенный runtime теперь идут через + один и тот же `_requestMode(pendingMode, false, 'resume')`, что и + зафиксировано правкой `smoke_nav_persist.mjs` (ожидание следующего кадра + вместо синхронного вызова). +- Обработчик шапки (`houseplan-card.ts:4074-4104`, `houseplan-editor-runtime` + не участвует): `hdrH`/`stageH`/`ownHdrH` публикуются только внутри + `updateComplete.then(() => requestAnimationFrame(...))` после повторного + измерения (`measured === t`), т.е. из уже осевшего кадра — закрывает + симптом 3 (несогласованная пара memo). `ResizeObserver` на `.hdr` теперь + также наблюдает `.stage` и фильтрует колбэк (`this._containerOwnedHeight || + entries.some(e => e.target === hdr)`), что одновременно убирает побочный + эффект на `smoke_align_guides` (подтверждено прогоном — ниже) и покрывает + HA-owned/zero-size сценарии AC3. +- `houseplan-editor-runtime.ts`: `_setMode` не получил третий параметр `warm` + — он и не должен, это публичный метод host (`houseplan-card.ts:7440`), + рантайм вызывается только из `host._setMode`. Новые обёртки + `finishWarmModeAdoption`/`resumeWarmMode`/`warmRoomDraft` на + `HouseplanEditorRuntime` переносят существующие операции за уже + определённый порт (`HouseplanEditorHostPort`), не добавляя нового + состояния комнаты — поле в поле сверено с прежним литералом в + `houseplan-card.ts` (diff показывает чистое перемещение, не правку полей). + +### Исполнено + +- `npx tsc --noEmit` — чисто. +- `npm run build` — OK; `node scripts/bundle-sync.mjs` — синхронизировано для + смоков. +- `node scripts/bundle-policy.mjs --verify` — PASS (бюджет кандидата: сборка + цела, закоммиченная копия не сверяется на этом треке — верно по #657). +- `npm test` — **3539 pass / 0 fail** (3540 тестов, 1 skipped) — чисто. +- `node demo/smoke_warm_mode_adoption.mjs` — **все 31 проверка `true`**, + включая оба порядка гонки AC2 (`delayed: false/true`), обе пары + экранных точек (`ac2-pixels-*`), AC3 (обычная/HA-owned/нулевая сцена), оба + порядка AC4 (`runtimeFirst: false/true`), AC5 десктоп и отдельный реальный + touch-tap в мобильном viewport (`ac5TouchSpaceWins` — настоящий + `page.locator(...).tap()`, не синтетический клик), и расширенный блок AC6 + (kiosk, отказ прав, disconnect, pan во время загрузки и последующий + resize). +- Регрессионные смоки, перечисленные в AC6: `smoke_warm_dialogs.mjs`, + `smoke_nav_persist.mjs`, `smoke_warm_owners.mjs`, `smoke_warm_remount.mjs` + — все `OK`. Дополнительно прогнаны `smoke_align_guides.mjs` (побочный эффект + из хендоффа — подтверждён как исправленный) и `smoke_readonly_cold_start.mjs`, + `smoke_stairs.mjs` — все `OK`. +- `node scripts/mutation-gate.mjs --check` — PASS, все 7 новых мутантов + (`warm-resume-overwrites-view-return-camera`, + `warm-resume-camera-depends-on-dialog`, + `warm-pan-during-runtime-keeps-refit-blocked`, + `warm-resume-collapses-pending-header`, + `warm-memo-publishes-torn-header-stage-pair`, + `warm-late-resume-beats-user-mode`, `warm-late-resume-crosses-space`) + находят свои `find`-патчи в текущем дереве; плюс изменённый + `warm-pending-mode-leaves-revive-waiting` (переписан под новую форму кода, + тоже найден). Browser guards: 214/200 — выше ориентира, но каждая строка + сверх 200 держит своё обоснование в `docs/testing-notes/mutation-browser-guards.md`, + которое обновлено (205→214) синхронно с реестром. +- `npm run golden:verify` — **192/192 PASS**, включая + `large-house-warm-remount-dark` (сцена именно этого пути). Повторный прогон + не потребовался — первый уже зелёный, второй запуск остановлен мной как + избыточный. +- `node scripts/process-gate.mjs --range origin/dev..HEAD --issues --report` + — PASS, 3 коммита, 0 предупреждений. + +### Защитные AC — таблица «чем краснеет» (проверена, не переписана с нуля) + +| AC | Доказано | Мутант/отрицательный случай | Проверено мной | +|---|---|---|---| +| AC1 | `ac1-view-*` (оба порядка) | `warm-resume-overwrites-view-return-camera` | найден `--check`, smoke зелёный | +| AC2 | `ac2-camera-*`, `ac2-pixels-*` (оба порядка) | `warm-resume-camera-depends-on-dialog`, `warm-resume-collapses-pending-header` | найдены оба, smoke зелёный | +| AC3 | `ac3HeightPair`, `ac3HaOwnedPair`, `ac3ZeroSizePreservesPair` | `warm-memo-publishes-torn-header-stage-pair` | найден, smoke зелёный | +| AC4 | `ac4-user-mode-*` (оба порядка) | `warm-late-resume-beats-user-mode` | найден, smoke зелёный | +| AC5 | `ac5SpaceWins`, `ac5TouchSpaceWins` (реальный tap) | `warm-late-resume-crosses-space` | найден, smoke зелёный | +| AC6 | kiosk/отказ/disconnect/pan/resize/view-cancel | `warm-pan-during-runtime-keeps-refit-blocked` + отрицательные входы (без подмены алгоритма, как и заявлено) | найден, smoke зелёный | + +Пустых третьих столбцов нет — требование §2.7/#435 выполнено по каждой защитной строке. + +### Числа, видимые дважды + +- Бюджет бандла (301019 B / 301066 B) и прирост `monolith-baseline.json` + (`hostRefs` 4885→4925, `portPrivates` 94→103) названы в комментарии автора и + совпадают с диффом `scripts/monolith-baseline.json` и обновлённым + `docs/ARCHITECTURE.md`. Один источник — `monolith-metrics.mjs` считает их + исполнением внутри `npm test` (прошёл), текст комментария и документ только + цитируют тот же расчёт. +- Browser guards 214/200 — совпадает в выводе `mutation-gate --check` и в + `docs/testing-notes/mutation-browser-guards.md` (таблица обновлена теми же + числами 38/97/214, разбивка по категориям сходится: 26+45+38+4+97+4=214). + +### Чего не проверял + +- Мутанты не исполнялись (это ночной гейт, #709) — проверено только что + `--check` находит все 7 новых патчей в дереве; это и есть обязанность + ревью на этом треке. +- `pytest tests_backend` / HA-harness не прогонялся: задача не трогает + `custom_components/houseplan/**` (диф затрагивает только `src/`, `demo/`, + `scripts/`, `docs/`) — Python-гейт здесь не по AC и не по диффу. +- Полный предрелизный performance-набор (`benchmark:large-house`) не + перегонял — автор уже выполнил его дважды (до/после) в комментарии и + зафиксировал непостоянство отдельно в #778 (P2, infra, не #762). Это не + AC этой задачи (AC нигде не называют числовой performance-порог), а + существующий флуд ресайза; я проверил, что #778 действительно заведён, + помечен и ссылается на #762. +- Windows pre-push отказ `test/iso-overlay-fixture-types.test.mjs:110` + проверен по существу: issue #777 заведён, помечен `infra`/`tests`/P2, не + требует изменений продуктового кода #762 — я не стал его воспроизводить на + Windows (вне доступной среды), доверяю зафиксированному разбору автора, + это не AC и не гейт code-review на Linux-материале. +- `ci:golden` требовал полный прогон golden (сделан, см. выше) — скриншоты + документации не обновлял намеренно (не гейт задачи, #697). + +## Находки + +Нет. High: 0, Medium: 0. + +Рассмотренные кандидаты, снятые при проверке: +- Несовпадение SHA `adc2d7c5` (хендофф) vs `11beca1a` (материал) — не находка: + резолвится как ребейз того же диффа на ушедший вперёд `dev` (§2.10 — «если + SHA не резолвится, это не находка сама по себе»; здесь оба резолвятся, + содержимое идентично по трём коммитам и файлам). +- Третий параметр `warm` в `HouseplanEditorHostPort._setMode` отсутствует в + сигнатуре реализации `HouseplanEditorRuntime._setMode` — не находка: + интерфейс описывает публичный `_setMode` хоста (`houseplan-card.ts`, + реально имеющий `warm`), а не метод рантайма, который вызывается только из + хоста без `warm`. +- `ac3PairDiagnostic` может содержать не-`true` значение при провале — + не баг, намеренный диагностический вывод смока (`checkAll` падает на нём + читаемым сообщением, а не маскирует). + +## Что проверено и корректно + +- Все 5 симптомов из тела issue имеют по отдельному красному свидетелю на + исходном SHA (заявлено автором, не перепроверялось мной заново — это + требует отката кода, что выходит за рамки код-ревью при наличии + зелёного смока и подтверждённого мутанта на текущем коде) и по зелёному + — исполнено мной на материале `11beca1a`. +- Трейлеры `Issue:`/`User-Visible:` на месте во всех трёх коммитах класса + A/B; `User-Visible: yes` несёт оба changelog в том же коммите. +- `route: fix` оправдан: оба канонических документа (`docs/WARM-REMOUNT.md`, + `docs/ARCHITECTURE.md`) обновлены вместе с кодом, и каждый из + perf-рискованных токенов (rAF/getBoundingClientRect/ResizeObserver в + указанных строках) закрыт именованным AC с собственным тестом и мутантом. +- Побочный эффект, найденный и исправленный самим автором в процессе работы + (stage-only `ResizeObserver` ломал `smoke_align_guides`), подтверждён + исправленным — регрессионный смок зелёный на этом материале. + +## Вердикт + +Зелёный. Код делает заявленное, доказательства по каждому AC полны и +воспроизводимы, гейты (typecheck/test/build/bundle-policy/golden/mutation-gate/process-gate) +исполнены мной на материале `11beca1ae80a87787ebb06fbc52f4d4ac2e86ee8` и +зелёные, Validate на этом SHA зелёный. Находок нет. + +--- + + + +## Материал раунда + +- Ветка: `issue/762-warm-mode-adoption`, коммит `11beca1ae80a` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `9e68eed01177ab60ce6c66021c6ac04d6dd1599d` + ``` + git log --all --format='%H %T' | grep 9e68eed01177 + ``` +- Тело issue: `43855b946e06e4cbd56d6890ba92b55ab76f4f3334fd8c10ac7b271d90344cee` +- Вердикт конвейера: `green` · High 0 · маршрут `fix` +