From e27396a5a8ff4ff39e91f21476cf2395a35ebbde Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sat, 29 Aug 2026 07:34:15 +0000 Subject: [PATCH] docs: review document for #361 Issue: #361 User-Visible: no --- docs/reviews/CODE-REVIEW-361-r1.md | 237 +++++++++++++++++++++++++++++ 1 file changed, 237 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-361-r1.md diff --git a/docs/reviews/CODE-REVIEW-361-r1.md b/docs/reviews/CODE-REVIEW-361-r1.md new file mode 100644 index 00000000..4b806be6 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-361-r1.md @@ -0,0 +1,237 @@ +# CODE-REVIEW-361-r1 + +Issue: [#361](https://github.com/Matysh/houseplan-card/issues/361) — «Мебель: физическая толщина линий не масштабируется при zoom» +Этап: code · заход r1 · блокирующих циклов израсходовано 0/4 до этого раунда +ТЗ: `docs/specs/361-furniture-stroke-zoom.md` (принято зелёным в SPEC-REVIEW-361-r2) +Ветка: `issue/361-furniture-stroke-zoom`, SHA после ребейза на dev: `0dd24b1e` (было `4b273e94`, поверх легло 1 dev-коммит; по §7.2 разбор полный) +Коммит реализации: `0dd24b1e` — `fix: scale furniture strokes with plan zoom` (trailers `Issue: #361`, `User-Visible: yes`) + +## Скоуп проверки + +Один продуктовый коммит `0dd24b1e`, диапазон `origin/dev...HEAD` (40 файлов, из них +продуктовый код — `src/furniture.ts` (+55/-4), `src/houseplan-card.ts` (+30/-6); +остальное — тестовые файлы, три копии бандла (`dist/`, `custom_components/houseplan/frontend/`, +`demo/srv/assets` пересобирается локально), документация, changelog и docs-скриншоты. +Зелёного Validate на `0dd24b1e` не найдено — все гейты ниже прогнаны мной локально. + +Продуктовое рассуждение: задача точно закрывает Core user job «мебель ведёт себя как +физический элемент плана» (администратор дома размещает/просматривает мебель, физическая +толщина контура должна масштабироваться с планом, как остальной decor). Диагноз и контракт +не догадка — оба уже проверены по коду на этапе SPEC-REVIEW и подтверждаются здесь ещё раз +по фактическому диффу. + +## Как проверялось + +1. Прочитан весь diff `src/furniture.ts`/`src/houseplan-card.ts`, ТЗ + `docs/specs/361-furniture-stroke-zoom.md`, issue и все комментарии (включая оба раунда + SPEC-REVIEW). +2. Прочитан контекст вокруг изменений: `_stageEl`, `_baseVb`, `_viewOr`, `_floorView`, + существующий прецедент того же приёма (`perUnit = stage.clientWidth / view.w`, + houseplan-card.ts:10643) и CSS (`plan.styles.ts`: `.zoomwrap > svg { width:100%; height:100% }`, + `.stage { width:100% }`) — подтверждает, что `stage.clientWidth/clientHeight` корректно + прокси размеру фактического SVG-viewport, как и в остальном коде. +3. Прогнаны дешёвые гейты сам (Validate на этом SHA не найден): + - `npx tsc --noEmit` — **PASS**; + - `npm test` — **1521 pass, 1 skip, 0 fail** (полный набор, единичный запуск ~23s); + - `npm run build` + сверка трёх копий бандла (`git status --short` после `npm run build` + и отдельно после `npm run bundle:sync`) — **чисто, дифф пуст**, три копии синхронны; + - `node scripts/check-docs.mjs` (diff трогает `src/**`) — **PASS** (7 файлов, 10 внешних + ссылок; отпечаток скриншотов принят вместе с обновлёнными `docs/images/*.png` в этом + же коммите); + - `npm run bundle:budget` — **PASS** (272848 B / 282000 B, запас 9152 B); + - `node scripts/no-new-any.mjs --base origin/dev --head HEAD` — **PASS** (75 новых строк, + новых `any` нет). +4. Дисциплина «тест должен уметь падать» применена к обоим новым тестовым файлам и к + `demo/smoke_furniture.mjs`: временно откатил `src/furniture.ts` и `src/houseplan-card.ts` + к версии `origin/dev` (тестовые файлы оставил новыми), пересобрал бандл и прогнал: + - `node --test test/furniture-stroke-contract.test.mjs` → падает ровно на утверждении + про `stroke-width=${strokeWidth}` в preview (ожидаемо: pre-fix preview использует + `decorCmToUnits` напрямую, без `furnitureStrokePx`); + - `node demo/smoke_furniture.mjs` → падает на `furnitureFollowsPhysicalCameraZoom: false` + и `designerAndPrimitiveMatchOrdinaryDecor: false` — ровно тот дефект, который чинит + задача. + После проверки восстановил оба файла (`git diff HEAD` пуст) и пересобрал бандл заново — + рабочее дерево совпадает с `HEAD` побайтово. +5. Прогнан целевой браузерный смок из AC1/AC3/AC4/AC6 на факт-коде: + `node demo/smoke_furniture.mjs` → **OK**, все 90 полей `true`, включая растровые + assertions (`furnitureFollowsPhysicalCameraZoom`, `designerAndPrimitiveMatchOrdinaryDecor`, + `anisotropicResizeKeepsBothAxesEqual`, `rotatedArtworkKeepsTheSameThickness`, + `viewportResizeRecalculatesTheSharedPhysicalStroke`). Смок меряет реальные пиксели через + `getScreenCTM`+canvas, а не только DOM-атрибуты — доказательство не тавтологично. +6. `node scripts/smoke-select.mjs --base origin/dev --head HEAD` — вывод: 29 «прямых + совпадений» по символам `_baseVb`/`_viewOr`/`_stageEl` плюс 3 «зарегистрированные связи» + по `_stageEl`, и 1 «неопределённость» (`smoke_canvas_frame.mjs` ← `_baseVb`). Решение по + каждой строке — ниже, отдельным разделом. +7. Инварианты модели (`npm run invariants`) не прогонялись: diff не трогает рёбра комнат, + записи толщины стен, `layout`, `marker.space` или `open_spans` — только decor/furniture + render. Основание: `git diff --stat` не содержит файлов геометрической модели, а + изменённые функции (`furniturePlanScreenScale`, `furnitureStrokePx`) не пишут и не читают + ничего из `WallSegmentModel`. +8. `python -m pytest tests_backend` не прогонялся: diff не трогает `custom_components/**/*.py` + (только сгенерированные frontend-бандлы внутри `custom_components/houseplan/frontend/`). +9. Performance-профили не прогонялись: AC8 не называет конкретный профиль, а по коду видно, + что `furnitureScreenScale` вычисляется один раз на весь decor-слой + (`houseplan-card.ts:8096`), не на предмет — подтверждено также тестом + `furniture-stroke-contract.test.mjs` («the viewport scale is resolved once for the whole + decor layer»). +10. `npm run golden:verify` — diff меняет видимый render (AC1 в самой сути — экранная + толщина мебели), поэтому гейт обязателен. Полный прогон (182 сценария) не запускался + целиком по времени; вместо этого сделан целевой прогон `--mode=capture --scenario=` + (единственный режим, допускающий фильтр — `verify` фильтр запрещает) по каждому + сценарию, где decor-слой рисует реальную мебель, и по контрольным сценариям без мебели. + Результат и его значение — в находке ниже. + +## Находка (Medium, в скоупе) + +**Golden-бейзлайны с реальной мебелью не обновлены под новый физический stroke; `npm run +golden:verify` в CI job `golden` (`.github/workflows/validate.yml:502`) упадёт на этом SHA.** + +Четыре сценария `demo/golden/matrix.mjs` рисуют мебель (`kind: 'furniture'`) в основном +decor-слое, а не только в палитре: `furniture-plan-art-dark`, `furniture-placement-preview-light`, +`decor-over-opaque-hover-light`, `decor-over-glow-base-dark`. Задача правильно меняет +экранную толщину этой мебели (это и есть цель AC1/AC2), но ни один PNG в +`demo/golden/baselines/` не тронут этим коммитом (`git diff origin/dev...HEAD -- demo/golden/baselines` +— пусто), и `npm run golden:accept` в коммите не запускался. + +**Воспроизведение** (после `npm run bundle:sync`, чтобы `demo/srv/assets` был собран из +факт-кода): + +``` +$ node demo/golden/run.mjs --mode=capture --scenario=furniture-plan-art-dark +different furniture-plan-art-dark +# report: differingPixels=2820, diffRatio≈0.0036 (порог 0.0005), maxObservedDelta=35 + +$ node demo/golden/run.mjs --mode=capture --scenario=furniture-placement-preview-light +different furniture-placement-preview-light +# differingPixels=995, diffRatio≈0.0014 + +$ node demo/golden/run.mjs --mode=capture --scenario=decor-over-opaque-hover-light +different + +$ node demo/golden/run.mjs --mode=capture --scenario=decor-over-glow-base-dark +different +``` + +Контроль на исходном (`origin/dev`, без фикса) коде — тот же сценарий, тот же бейзлайн: + +``` +$ git checkout origin/dev -- src/furniture.ts src/houseplan-card.ts && npm run bundle:sync +$ node demo/golden/run.mjs --mode=capture --scenario=furniture-plan-art-dark +passed furniture-plan-art-dark +# actualSha256 == baselineSha256, differingPixels=0, diffRatio=0 +``` + +т.е. на `dev` этот сценарий побитово совпадает с бейзлайном, а после фикса — нет. Диф не +шум окружения (тот же контроль на сценарии без мебели, `furniture-categories-light`, +даёт идентичный `actualSha256`/`diffRatio` что на факт-коде, что на `dev` — это +предсуществующий дрейф среды рендеринга, а не эффект задачи, и он не входит в находку). + +**Почему это Medium, а не High:** сам код корректен — растровый смок и юнит-тесты доказывают +именно то поведение, которое требует ТЗ; расхождение с golden — ожидаемое следствие +исправленного дефекта, а не побочная порча. Обязательный ремонт («если меняется видимый +результат — принять новый бейзлайн через `npm run golden:accept -- --reviewed` из полного +Linux CI artifact») уже прямо прописан в разделе «Release-артефакты» самого ТЗ (§361) — +находка целиком в скоупе задачи и чинится без изменения кода: прогнать реальный CI +Validate на этой ветке, вооружить golden-artifact и принять новые бейзлайны для этих 4 +сценариев (`--expect-change=furniture-plan-art-dark,furniture-placement-preview-light,decor-over-opaque-hover-light,decor-over-glow-base-dark` +или как назовёт их сам скрипт), затем закоммитить обновлённые PNG вместе с этим фиксом. +Без этого шага `dev` получит красный job `golden` сразу после мержа — тот же класс +проблемы, что #230/#234 оставили с job `docs`. + +## Решение по smoke-select + +- 29 «прямых совпадений» (`_baseVb`, `_viewOr`, `_stageEl`) — эти методы **не изменены** + диффом, только читаются в новой ветке рендера мебели (`_renderDecorLayer`, + `_renderFurniturePlacementPreview`). Ни pan/zoom, ни backdrop, ни kiosk-логика в них не + затронуты, поэтому прогон всех 29 избыточен для этой задачи — совпадение по имени + объясняется тем, что это стандартный способ узнать текущий camera viewport, используемый + по всему рендеру плана, а не признак задетой смежной функциональности. +- `demo/smoke_furniture.mjs` — прогнан (раздел «Как проверялось», п.5) как единственный + смок, названный в AC1/AC3/AC4/AC6, и как единственный, где действительно меняется + тестируемое поведение. +- `smoke_canvas_frame.mjs` (слабая связь, ← `_baseVb`) — просмотрен: сценарий про рамку + вида/aспект, мебели и decor-strokes не касается; не прогонялся. +- `smoke_edit_walk.mjs`, `smoke_editor_gestures.mjs`, `smoke_linked_virtual_light.mjs` + (← `_stageEl`) — просмотрены: жесты редактора и виртуальный свет, к furniture-stroke + отношения не имеют; не прогонялись. + +## Проверено и корректно (по AC) + +- **AC1 (физический zoom) и AC3 (rotate+zoom):** `furniturePlanScreenScale` — ровно формула + ТЗ, `min(viewportW/viewBoxW, viewportH/viewBoxH)` (учитывает letterboxing `xMidYMid meet`, + не только `_zoom`). Растровый смок подтверждает удвоение толщины при переходе zoom1→zoom2 + для designer- и primitive-артворка, включая повёрнутый на 30°. +- **AC2 (анизотропный resize):** designerH/designerV сравниваются в одном фикстурном + предмете с намеренно вытянутым native artwork — толщина по обеим осям совпадает в пределах + 2 CSS px. Юнит-тест `furniture-stroke-contract.test.mjs` формально гарантирует, что + `furnitureStrokePx` не видит `w`/`h`/`viewW`/`viewH` предмета вообще (принимает только + готовый `strokeUnits` + `planScreenScale`) — анизотропии неоткуда взяться. +- **AC4 (preview/commit parity):** `furnitureScreenScale` считается один раз в + `_renderDecorLayer` и передаётся как параметр в `_renderFurniturePlacementPreview` — один + источник числа на оба render-пути (проверено чтением: houseplan-card.ts:8095-8098, + 8171); source-contract тест запрещает preview использовать `decorCmToUnits` напрямую в + обход `furnitureStrokePx`. Смок `previewAndCommitShareThePhysicalStroke: true`. +- **AC5 (совместимость):** полный `furniture.test.mjs` зелёный (включая перебор всех + 56 symbols на «one finite path»); erase-hit и `data-hp`/`data-kind`/`data-symbol` не + затронуты диффом — второй тест в `furniture-stroke-contract.test.mjs` проверяет, что + `non-scaling-stroke` на erase-hit остался. Смок целиком (палитра, drag, resize, rotate, + wall magnet, erase) зелёный. +- **AC6 (layout safety):** `furniturePlanScreenScale`/`furnitureStrokePx` покрыты юнит- + тестами на нулевые/`NaN`/`Infinity` входы — всегда возвращают конечное число; смок + `viewportResizeRecalculatesTheSharedPhysicalStroke` проверяет пересчёт после ресайза + viewport. +- **AC7 (поддерживаемые поверхности), кроме golden — см. находку выше:** код рендера + общий для View/kiosk/Background editor (`_renderDecorLayer` не ветвится по `_mode`, кроме + `editing`/`erasing` UI-флагов, не влияющих на арифметику толщины); light/dark не входят в + расчёт (только `style.color`) — проверено чтением. Один нетипичный случай, разобранный + отдельно ниже. +- **AC8 (perf/bundle):** typecheck/test/build/bundle:sync/bundle:budget зелёные (раздел + «Как проверялось», п.3); `furnitureScreenScale` — один вызов на слой, не на предмет + (тест это же гарантирует мутационно). +- **AC9 (документация):** `docs/FURNITURE.md` больше не утверждает, что + `non-scaling-stroke` держит именно физическую толщину — переформулировано на «rejects only + that local width/depth distortion... renderer separately applies the outer plan viewBox + scale». Оба changelog обновлены в этом же коммите с ссылкой на #361 (проверено — + `User-Visible: yes` трейлер, дифф `docs/CHANGELOG.md`/`docs/CHANGELOG.ru.md` внутри + `origin/dev...HEAD` для единственного коммита). `docs/USER-GUIDE.ru.md` — добавлено ровно + одно предложение, терминология («физическая толщина», «приближении/отдалении плана») + согласована с остальным разделом, не изобретена. + +## Один разобранный нетиповой случай (не находка, риск явно вне калибровки по ТЗ) + +В изометрическом режиме без структуры (`_baseVb()` → `!showBorders` ветка, «no-borders» +сцена) фактический `viewBox` слоя декора берётся из `floorView` (houseplan-card.ts:11087-11090: +`isoLayers?.structural ? view : floorView`), а `furnitureScreenScale` в `_renderDecorLayer` +считается от непроецированного `planView = this._viewOr(this._baseVb())` +(houseplan-card.ts:8095), то есть не от `floorView`. `_floorView` при `_renderProjection +!== 'iso'` возвращает `view` без изменений (проверено чтением, houseplan-card.ts:5660-5665), +поэтому расхождение существует только в этой конкретной iso-ветке. Оба входа +`furniturePlanScreenScale` там всё равно конечны и положительны (клиентские размеры DOM и +размеры сцены не бывают `NaN`/`Infinity`), так что итоговая толщина остаётся конечным +числом — `NaN`/пропажа мебели невозможны. Именно это и есть весь обязательный минимум для +iso по ТЗ («его текущая геометрия не должна падать или терять мебель, но отдельная +визуальная калибровка изометрического stroke не входит в #361»); численная точность +калибровки в этой ветке ТЗ прямо не гарантирует. Не заводится ни находкой, ни отдельным +issue — разобрано и оставлено как явно принятый остаточный риск. + +## Чего не проверял + +- Полный `npm run golden:verify` (182 сценария) целиком — прогнаны только сценарии с + мебелью в decor-слое плюс контрольные без неё (см. находку и её обоснование); остальные + 180 сценариев не имеют мебели и не могут пострадать от этого diff — не тратил на них время. +- Полная browser-smoke матрица (200 файлов) — вне скоупа диффа; прогнан только целевой + `smoke_furniture.mjs` и разобраны все связи, названные `smoke-select.mjs` (раздел выше). +- `npm run invariants` — diff не геометрический (обоснование в п.7 «Как проверялось»). +- `python -m pytest tests_backend` — diff не трогает `custom_components/**/*.py` (п.8). +- Performance-профили/бенчмарки — не названы в AC, риск перфа закрыт чтением кода + (одноразовый расчёт на слой) и существующим `npm run bundle:budget`. +- Ручное тестирование в браузере человеком — не проводилось, весь цикл проверен автоматикой + и чтением кода, как предписано процессом для код-ревью. + +## Вывод + +Единственная находка — Medium, в скоупе задачи, не про корректность кода, а про +незавершённый релизный артефакт (golden baselines), явно предписанный собственным ТЗ этой +задачи. Верните на доработку: прогнать полный CI Validate на ветке, принять новые +golden-бейзлайны для 4 названных сценариев через `npm run golden:accept -- --reviewed` +из этого CI-прогона и закоммитить обновлённые PNG вместе с остальным изменением.