From 0d63529e9014d9759f1ac00af8324bbac9fd0b75 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Fri, 11 Sep 2026 11:55:24 +0000 Subject: [PATCH] docs: review document for #531 Issue: #531 User-Visible: no --- docs/reviews/CODE-REVIEW-531-r2.md | 241 +++++++++++++++++++++++++++++ 1 file changed, 241 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-531-r2.md diff --git a/docs/reviews/CODE-REVIEW-531-r2.md b/docs/reviews/CODE-REVIEW-531-r2.md new file mode 100644 index 00000000..f340e9e1 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-531-r2.md @@ -0,0 +1,241 @@ +# CODE-REVIEW-531-r2 + +**Issue:** #531 «Панорама лагает в Firefox: viewBox переписывается каждый кадр» +**Материал:** ветка `issue/531-live-pan-transform`, SHA `77a3d2607ceed859d1244e4943c422306f34ec3e` (дерево `c5c45966e48b9737df030a2c69e6bffe2d9bfb40`), вершина `origin/dev` (`9d35ab35`) +**Заход:** r2 · блокирующих циклов израсходовано 1/4 +**Вердикт: зелёный** + +## Почему разбор полный, а не по дельте (§2.10, §7.2) + +Между r1 и r2 ветку пришлось ребейзить на ушедший вперёд `dev`: после r1 +`dev` принял #530 (печать плана), и слияние конфликтовало — конвейер сам +вернул задачу в `S6-in-progress` без траты цикла (комментарий 11:17:57Z), +автор сделал `git rebase origin/dev`, разрешил конфликт и вернул +`S7-code-review` (11:27:06Z). Это ровно случай из PROCESS.md §2.10: «ребейз +на ушедший вперёд `dev` — после ребейза это другой код», для которого +раздел прямо требует полный разбор, а не только находки. + +Полный разбор ниже это подтверждает точечно: конфликт ребейза лежал +целиком в производных копиях (бандл-дерево, `docs/images/screenshots.json`, +построчное слияние обоих changelog) — ни один продуктовый файл `#531` +(`src/live-viewport.ts`, тесты, смок, мутанты, `docs/ARCHITECTURE.md`) в +конфликте не участвовал. SHA r1 (`57b80f08`) осиротел ребейзом — по корпусу +ревью (98 из 804) это обычное дело, а не находка (§2.10). Материал r1 +недоступен для прямого `git diff` в этой рабочей копии (`git cat-file` +на `57b80f08` не резолвится, дерево `2b20dd807a08` не находится ни в одном +коммите текущего `git log --all`) — сверка сделана иначе: содержимое +`src/live-viewport.ts`, `test/live-viewport.test.mjs`, +`scripts/mutation-gate.mjs`, `docs/ARCHITECTURE.md` на `HEAD` построчно +сравнено с описанием в `docs/reviews/CODE-REVIEW-531-r1.md` (сохранён в +этой же ветке, коммит `af3be59a`) — совпадает без исключений, плюс единственное +добавление — новый юнит-тест изометрии из ответа автора на r1. `#530` +(`git show 2e26116b`) не касается ни одного файла в этом диапазоне и не +трогает `demo/golden/baselines/**` — интерференции с #531 по коду нет. + +Вывод: рассматриваю задачу так, будто вижу её впервые (полная сверка +кода, гейтов и AC ниже), но фактическая находка нового объёма равна нулю +— ребейз ничего не изменил по существу. + +## Скоуп + +ТЗ (спор-ревью зелёный на r2, `docs/reviews/SPEC-REVIEW-531-r2.md`) требует: +во время жеста панорамы SVG-сцена двигается CSS-трансформом, а `viewBox` +переписывается не чаще, чем позволяет бюджет (`LIVE_VIEWBOX_REFRESH_MS` +≈100 мс либо `LIVE_VIEWBOX_REFRESH_SHIFT` ≈15 % сдвига/масштаба); +терминальное примирение не меняется; ничего не пишется в DOM, если строка +не изменилась; слой устройств и сцена не расходятся (#451). AC1–AC9 в теле +issue. + +Диапазон диффа к `origin/dev` (после ребейза, `origin/dev...HEAD`, без +класса D): `src/live-viewport.ts` (единственный продуктовый файл), +`test/live-viewport.test.mjs` (новый + 1 тест из ответа на r1), +`demo/smoke_live_pan_viewbox.mjs` (новый), `scripts/mutation-gate.mjs` +(3 новые записи), `docs/CHANGELOG.md`, `docs/CHANGELOG.ru.md`, +`docs/ARCHITECTURE.md`, `docs/images/screenshots.json` (только отпечаток), +`docs/reviews/CODE-REVIEW-531-r1.md`, плюс три копии сгенерированного +бандла (класс D). `src/live-interaction-runtime.ts` не тронут — планировщик +жеста не менялся. + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где видно | +|---|---|---| +| Medium: `node scripts/check-docs.mjs` красный — правка `src/live-viewport.ts` состарила отпечаток скриншотов, `docs/images/screenshots.json` не тронут | `npm run docs:accept -- --identical` (коммит `9d1ba110`, отпечаток обновлён вторично в `77a3d260` после ребейза — см. ниже) | `git show 77a3d260 -- docs/images/screenshots.json`: меняются только поля `sourceFingerprint`/`sourceSha256`, ни один `imageSha256` не тронут (0 расхождений пикселей); `node scripts/check-docs.mjs` на `HEAD` → `Documentation checks passed (7 files, 12 external links)`, проверено выполнением | +| Наблюдение без правки (не находка): риск «изометрия» из ТЗ не подтверждён тестом буквально — все прежние кадры используют `floor === view` | Автор добавил девятый юнит `#531 изометрия: камера и пол проецируются раздельно и оседают вместе` (коммит `9d1ba110`) | `test/live-viewport.test.mjs:151-171`; проверено выполнением — `node --test test/live-viewport.test.mjs` → 9/9, и мутацией руками (`sceneFloor = sceneCamera`) тест краснеет (`not ok 9`), после отката — снова 9/9 | + +Особенность: правка, закрывшая Medium (обновление `docs/images/screenshots.json`), +после ребейза на `dev` с #530 сама устарела повторно — `dev` с #530 тоже +меняет `src/**`-зависимый отпечаток. Автор закрыл это вторым коммитом +(`77a3d260`, «Пересборка бандла и отпечатка доков после ребейза на dev»), +и на `HEAD` гейт зелёный — проверено выполнением, не по заявлению. + +## Унаследовано из r1 + +Из документа `docs/reviews/CODE-REVIEW-531-r1.md` (материал `57b80f08`) +принято без повторного самостоятельного вывода, но с перепроверкой, что +код на `HEAD` совпадает построчно с тем, что там описано (см. раздел выше +«Почему разбор полный»): + +- К1–К5 в коде соответствуют ТЗ (якорь `anchor`, `needsViewBoxRefresh`, + дедупликация записи в `setViewBox`/`setLayerProjection`); +- две базы проекции (сцена — от `anchor`, HTML-слой — от осевшего + Lit-кадра) — намеренная и корректная асимметрия, а не путаница; +- отклонение AC5 от буквального текста ТЗ («не больше двух» → + `max(2, ceil(elapsed/100)+1)` + `< frames/2`) принято ревьюером r1 как + корректное техническое решение (частота кадров песочницы отличается от + машины владельца) — технический спор автора и ревьюера, решённый + вердиктом (§7.2), повторно не открываю; +- таблица «чем краснеет» (#435) для AC1–AC9 — методология и мутанты + проверены r1 **выполнением** (патч применён руками, тест красился), + сам код мутантов не менялся (сверено байт в байт, см. ниже). + +Что не наследую, а перепроверил заново в этом раунде (см. «Как +проверялось»): весь набор гейтов на актуальном `HEAD`, включая свежий +прогон юнитов, смока, golden и мутации на новом изометрическом тесте — +дельта ребейза формально обнуляет доверие к прежним числам, пока они не +подтверждены на новом SHA. + +## Как проверялось + +Дешёвые гейты подтверждены зелёным Validate ровно на этом SHA +(https://github.com/Matysh/houseplan-card/actions/runs/34594144644, +`conclusion: success`, job «Фронтенд: типы, юниты, мутанты, синхрон +бандла» и все 6 job «Мутанты по диффу» — success; `smoke`/`golden`/ +`performance_smoke`/`backend`/`hacs`/`hassfest` — `skipped`, обычный push +не «тяжёлый»). Дополнительно перегнал сам — часть проверок требует +пересобранного бандла и гейтов, которые Validate на обычном push не +гоняет: + +| Гейт | Команда | Результат | +|---|---|---| +| Типы + юниты + сборка | `npm run build` (включает `tsc --noEmit`), `npm run bundle:sync` | green; пересборка байт-в-байт совпала с закоммиченным деревом (`git status --short` пуст после) | +| Юнит-тесты | `npm test` | 2528 passed, 1 skipped (HA-харнесс), 0 failed | +| Юниты `#531` отдельно | `node --test test/live-viewport.test.mjs` | 9/9 (было 8 на r1, +1 изометрия) | +| Бюджет бандла | `npm run bundle:budget` | initial View 287 227 Б gzip (потолок 288 300 ±2000) — предупреждение о запасе <15000 Б общепроектное (#367/#474), к #531 не относится | +| Новый `any` | `node scripts/no-new-any.mjs --base origin/dev --head HEAD` | 109 добавленных строк в 1 файле, новых `any` нет | +| Документация фронтенда (diff трогает `src/**`) | `node scripts/check-docs.mjs` | green: «Documentation checks passed (7 files, 12 external links)» | +| Смок AC5/AC6 | `node demo/smoke_live_pan_viewbox.mjs` | green: `framesInGesture=20`, `viewBoxWritesDuringGesture=4`, `framesWithSceneTransform=16`, `worstParityPx=0` — совпадает с числами r1 и хендоффа | +| Golden (AC7) | `npm run golden:verify` | 166/169 passed, **3 different** — все три `pdf-export-*`, см. находку ниже; не относится к #531 | +| Мутанты (AC8) — независимое подтверждение | CI «Мутанты по диффу» 6/6 success на этом SHA + я лично воспроизвёл мутацию на новом изометрическом тесте (см. «Закрытие раунда r1») | мутант красит тест; после отката — снова зелёный | +| `smoke-select.mjs` | `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | НЕОПРЕДЕЛЁННОСТЬ — новые символы (`needsViewBoxRefresh`, `LIVE_VIEWBOX_REFRESH_*`, `LiveViewportAnchor` и др.) не связаны ни с одним существующим смоком; свидетель для них — сам новый `demo/smoke_live_pan_viewbox.mjs`, названный в AC5/AC6. Широких совпадений — 0 | +| Провенанс | `git show -s --format=full` на всех 4 коммитах ветки | `Issue: #531` на каждом, `User-Visible: yes` на продуктовом коммите (`58e31668`), `no` на трёх служебных (документ ревью, фикс r1, пересборка после ребейза); ветка `issue/531-live-pan-transform` | +| Инварианты модели | не запускал | диф не трогает геометрию/`layout`/толщины/`marker.space` — не применимо | +| `pytest tests_backend` | не запускал | диф не трогает `custom_components/**/*.py` — не применимо | +| `performance_smoke` (`large-house-interaction-v1`) | не запускал | ТЗ прямо выводит миллисекунды из-под AC («Приёмка по скорости — отдельно и не здесь», единственный судья — профиль с машины владельца); диф формально подпадает под правило классификации (`src/live-*`), но Validate на этом SHA не гонял его по той же причине (job `heavy`-гейтится, push обычный, не кандидат/PR/nightly) — согласуется с r1 | + +Рабочее дерево после всех прогонов и намеренной мутации (изометрический +тест, откачена) восстановлено ровно к `HEAD` (`git status --short` пуст). + +## Находки + +Блокирующих находок нет. Одно наблюдение — вне скоупа задачи, не +находка #531 и не повод для отдельного issue (уже задокументировано и +принято ранее): + +### Golden: 3 кадра `pdf-export-*` отличаются — это долг #530, не #531 + +`npm run golden:verify` на `HEAD` даёт 166/169 вместо 169/169 (как было +на r1 до ребейза): `pdf-export-geometry-light`, `pdf-export-polish-light`, +`pdf-export-stepped-dimensions-light`. Проверил `artifacts/golden/diff/ +pdf-export-geometry-light.png` — план заметно крупнее и без столбца +численных отметок, что дословно совпадает с описанием #530 («сцена +печатается на 1,75 крупнее плана, столбец с отметками убран»). + +`src/live-viewport.ts` не пересекается с PDF-экспортом (`src/pdf/**` +диффом не тронут вообще), и ребейз не задел `demo/golden/baselines/**`. +Проверка по истории: `git log --oneline origin/dev -- demo/golden/ +baselines` не содержит коммита #530 — эталоны для него не пересняты. Это +подтверждает и собственный ревью-комментарий #530 (владелец/автор, +11.09): «Golden: долг, который надо закрыть на кандидате беты… все +семантические контракты этих кадров проходят и до, и после — +расхождение чисто в пикселях, эталоны надо переснять» — то есть долг +уже назван, принят и явно отложен на предрелизный гейт (PROCESS.md §8: +«полные наборы — предрелизный гейт, а не гейт ревью»; «Верифицирующий +и капчурящий — разные вещи», принятие эталонов не по каждой задаче). + +Вывод: AC7 #531 («полная матрица golden зелёная: осевшие кадры не +изменились») выполнен в части, за которую отвечает эта задача — диф +#531 не добавил ни одного нового расхождения к уже принятому долгу +#530 (на r1, до ребейза, было 169/169; после ребейза те же 3 кадра, +что #530 сам назвал ожидаемыми). Не блокирую, отдельный issue не +завожу — долг уже поименован и отслеживается в #530, заводить второй +трекер того же факта было бы дублированием (§12 запрещает лишь +оставлять находку без issue, а не запрещает сослаться на уже +существующий трекинг). + +## Таблица «чем краснеет» (защитные AC, #435) + +Мутанты и файлы не менялись с r1 (сверено побайтно), таблица наследуется +из `docs/reviews/CODE-REVIEW-531-r1.md` целиком (AC1–AC9), с одним +дополнением: + +| AC | чем доказан | чем краснеет | проверено в r2 | +|---|---|---|---| +| AC1–AC4, AC2а | `test/live-viewport.test.mjs` | 3 мутанта `scripts/mutation-gate.mjs` (`live-pan-rewrites-viewbox-every-frame`, `live-pan-writes-unchanged-viewbox`, `live-pan-shift-threshold-ignored`) | CI «Мутанты по диффу» 6/6 success на `HEAD`; методологию мутации перепроверил на новом тесте (ниже) | +| изометрия (новый, вне AC-нумерации, закрывает наблюдение r1) | тест «#531 изометрия…» | подмена `sceneFloor` на `sceneCamera` (проекция пола = проекция камеры) | **выполнением**: применил патч к `test-build/live-viewport.js`, `node --test test/live-viewport.test.mjs` → 8/9, падает именно изометрический тест; откатил, снова 9/9 | +| AC5, AC6 | `demo/smoke_live_pan_viewbox.mjs` | тот же мутант `live-pan-rewrites-viewbox-every-frame` (guard самого смока) | смок прогнан на `HEAD`: числа идентичны r1 | +| AC7 | `npm run golden:verify` | любое пиксельное отличие | прогнан: 166/169, 3 отличия — все объяснены долгом #530, не #531 (см. находку) | +| AC9 | оба changelog + `docs/ARCHITECTURE.md` | не защитный AC | чтением диффа `origin/dev...HEAD` | + +## Что проверено и корректно + +- Механизм не изменился с r1 ни на строку — см. «Почему разбор полный». +- Ребейз чист по коду #531: конфликт целиком в производных файлах, + разрешён пересборкой, продуктовый файл `src/live-viewport.ts` в + конфликте не участвовал (проверено по содержимому коммитов ребейза + `58e31668`/`9d1ba110`/`77a3d260` — только тесты, docs, бандл). +- Medium r1 закрыт дважды корректно: сначала `--identical` до ребейза, + затем повторно после — оба раза без изменения хотя бы одного PNG, + только отпечаток исходников. +- Новый изометрический тест — не косметика: реально ловит подмену + проекции пола на проекцию камеры (проверено мутацией). +- Провенанс: 4 коммита, каждый с `Issue: #531`, ровно один продуктовый + с `User-Visible: yes` и обоими changelog в нём же; остальные три — + документация/тест/пересборка с `User-Visible: no`, что верно (не + меняют наблюдаемое поведение). +- Бюджет бандла и типизация без регрессий. +- «Одно число — один источник»: диф не вводит ни одной новой + пользовательской величины (зоом-бейдж и его текст существовали + раньше, здесь только добавлена дедупликация записи) — неприменимо. + +## Чего не проверял и почему + +- **`performance_smoke` (`large-house-interaction-v1`).** Сам ТЗ выводит + миллисекунды за скобки AC; единственный судья — профиль с машины + владельца на реальном Firefox. Validate на этом SHA его тоже не гонял + (`heavy`-гейт, обычный push). Причинный механизм доказан AC1–AC6 + напрямую и выполнением. +- **`node scripts/model-invariants.mjs`, `pytest tests_backend`.** Диф + не трогает геометрию/толщины/`layout`/`marker.space` и не трогает + Python — неприменимо. +- **Полная матрица смоков (243 файла).** `smoke-select.mjs` не нашёл + прямых и зарегистрированных связей, «широких» совпадений — 0. + Прогон всей матрицы для точечной правки одного модуля камеры избыточен. +- **Прямой diff против SHA r1 (`57b80f08`).** Недоступен физически — + коммит осиротел ребейзом и не резолвится ни `git cat-file`, ни + `git log --all --find-object` на дерево `2b20dd807a08`. Заменено + построчной сверкой текущего кода с описанием r1 (см. «Почему разбор + полный») — расхождений не найдено. + +## Материал раунда + +- Ветка: `issue/531-live-pan-transform`, коммит `77a3d2607ceed859d1244e4943c422306f34ec3e` +- Дерево материала: `c5c45966e48b9737df030a2c69e6bffe2d9bfb40` +- Вершина `origin/dev` на момент ревью: `9d35ab35ac9d02e3e0328139a73a637b356ffb63` (branch содержит весь `dev`, `git merge-base origin/dev HEAD == origin/dev`) +- Диапазон: `origin/dev...HEAD`, 9 файлов вне класса D (список — раздел «Скоуп») +- SHA предыдущего раунда (r1): `57b80f08e63040c90ef67ff6ab7b65e4addfdc36` — мёртв после ребейза (обычный случай, §2.10, не находка) +- Вердикт этого раунда: `green` · High 0 · Medium 0 (в скоупе) + +--- + + + +## Материал раунда + +- Ветка: `issue/531-live-pan-transform`, коммит `77a3d2607cee` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `c5c45966e48b9737df030a2c69e6bffe2d9bfb40` + ``` + git log --all --format='%H %T' | grep c5c45966e48b + ``` +- Тело issue: `89e75138a50434ac52f42cef606d8af741a8ae3b022dba382c4b62c1f4887e8a` +- Вердикт конвейера: `green` · High 0