14 KiB
Код-ревью #122 — Isometric Stage 2: hidden visual polish (r2)
- Issue: https://github.com/Matysh/houseplan-card/issues/122
- Спецификация:
docs/specs/122-isometric-stage2.md(SPEC-REVIEW-122-r1: green) - Предыдущий цикл:
docs/reviews/CODE-REVIEW-122-r1.md— красный, High: 2, Medium: 1 → #134 - Диапазон:
git diff origin/dev...HEAD, новый коммит цикла —ef3cc98(fix: preserve isometric fallback rendering), поверх42b3f44(feat: add hidden isometric stage 2),76ce755/4c73e2c/e13215c(ТЗ и документы ревью) - Роль: ревьюер кода (свежая сессия, без контекста реализации и без контекста сессии r1)
- Вердикт: зелёный · цикл r2/4 · High: 0 · Medium: 0
Скоуп
Единственный продуктовый коммит цикла — ef3cc98. Класс файлов: A
(src/houseplan-card.ts, 13 insertions/4 deletions — два места: _baseVb() и
основной <svg>/<g> блок рендера плана) + B (test/isometric-contract.test.mjs,
+5 строк) + C (docs/ISOMETRIC.md, docs/adr/122-isometric-stage2-composition.md)
- D (
dist/houseplan-card.js,custom_components/houseplan/frontend/houseplan-card.js,demo/srv/assets/houseplan-card.js— все три пересобраны и включены в тот же коммит). Диапазон узкий и точечный: правка адресует ровно High-1 и High-2 из r1,demo/smoke_isometric_contract.mjsиdemo/smoke_isometric_live_touch.mjsв этом коммите не менялись (они уже были частью42b3f44).
Трейлеры ef3cc98: Issue: #122, User-Visible: no — корректно, фича
по-прежнему за скрытым Labs-флагом iso, публичный changelog не требуется.
Как проверялось
| Гейт | Команда | Результат |
|---|---|---|
| Typecheck | npx tsc --noEmit |
green, без вывода |
| Unit | npm test |
green, 766/766 |
| Build | npm run build |
green |
| Синхронизация 3 копий бандла | cmp dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js и то же для demo/srv/assets/houseplan-card.js |
обе команды green — байт-в-байт совпадают. High-1 из r1 закрыт |
npm run golden:verify (диф меняет рендер — обязателен) |
после npm run build + cp dist/houseplan-card.js demo/srv/assets/houseplan-card.js |
45/50 сценариев passed; 5 different — все пять из семейства «непринятые Iso-эталоны Stage 2» (isometric-geometry-view-dark/light, isometric-live-layers-dark, isometric-touch-kiosk-dark, isometric-large-warm-remount-dark), что и предсказывал §12.3 ТЗ и r1. isometric-no-borders-dark теперь passed (High-2 закрыт), large-house-zoom-250-dark теперь passed (Medium-1/#134 регрессия исчезла тем же коммитом) |
Повтор golden:verify для этих двух сценариев |
тот же прогон второй раз | оба снова passed — детерминированно, не флейк |
Целевые браузерные смоки (названы в AC1/AC5-AC7/AC9-AC11 и напрямую тронуты правкой _baseVb/рендера) |
node demo/smoke_isometric_contract.mjs, node demo/smoke_isometric_live_touch.mjs (после того же build+copy) |
оба green; в live_touch явно проверяется noBordersUsesFloorSymbols: true и visibleBordersRestoreStage2: true — оба режима show_borders исполнены в реальном DOM, не только прочитаны |
| Трейлеры коммита | git log --format=... ef3cc98 |
Issue: #122, User-Visible: no — оба присутствуют и терминальны |
| Backend | не прогонялся | правок в custom_components/**/*.py нет — не применимо |
Performance (large-house-isometric-v1) |
не прогонялся | как и в r1 — pre-beta гейт, заблокирован #124 по решению владельца, вне скоупа этого код-ревью |
Чего не проверял и почему: полный набор из 127 браузерных смоков — правка
цикла ограничена двумя точками в одном файле плюс правкой рендера, которую уже
покрывают целевые смоки/golden; python -m pytest tests_backend — нет
Python-правок; принятие golden-эталонов Stage 2 Iso — не роль ревьюера и не
требуется до pre-beta (§12.3 ТЗ, решение владельца); дифференциальный прогон на
origin/dev — не повторялся отдельно в этом цикле, т.к. origin/dev не менялся
с r1 и там же было доказано, что оба сценария чисты на dev (причинность уже
локализована в диффе задачи).
Находки
Отсутствуют. Оба High из r1 закрыты, новых High/Medium не найдено.
Что проверено и корректно
- High-1 (три копии бандла) — закрыт.
cmpподтверждает байт-в-байт идентичность всех трёх копий послеnpm run build; коммитef3cc98содержит пересобранныеdist/houseplan-card.js,custom_components/houseplan/frontend/houseplan-card.jsиdemo/srv/assets/houseplan-card.jsвнутри себя же (не отдельным коммитом) — правило «сгенерированное в том же коммите» соблюдено. - High-2 (
show_borders:falseв Iso ломал пиксельный контракт) — закрыт. Причина по коду (src/houseplan-card.ts:4558-4573,_baseVb()): при!showBordersрамка теперь строится какprojectedFrame({ rect: flat, wallHeight: ISO_WALL_HEIGHT })— это в точности старая Stage-1 проекция без вклада Stage 2 floor/opening-геометрии, а не безусловныйunionRect(flat, isoOpeningBounds()), который ломал сцену в42b3f44. Синхронно с этим основной<svg>/внутренний<g>(строки ~14181-14187) теперь используютisoLayers?.structuralвместоisoкак условие для классаiso-floor-scene/transform/viewBox — то есть в ветке «iso активен, но структура скрыта» контент рендерится без Stage-2-трансформации, тем же путём, что и Stage 1. Доказано исполнением:golden:verifyдаётpassedдляisometric-no-borders-darkс нулевым отличием от принятого Stage-1 baseline, повторено дважды детерминированно; смокlive_touchотдельно подтверждаетnoBordersUsesFloorSymbols: trueв реальном DOM. - Атрибут
classвнешнего<svg>(class=${isoLayers?.structural ? 'plan-svg' : nothing}, новая связка по сравнению с прежним безусловнымclass="plan-svg") — прочитано и проверено:.plan-svg { z-index: 1; }вsrc/styles.ts:472имеет смысл только относительно братских.iso-underlay-svg/.iso-shadows-svg/.iso-walls-svg, а те сами рендерятся строго под условиемiso && isoLayers?.structural(src/houseplan-card.ts:14176,:14394) — то есть ровно тогда же, когдаplan-svgполучает класс. Вне Flat и вне structural-iso конкурирующих слоёв с явным z-index нет, поэтому потеря класса не меняет порядок отрисовки. Единственное внешнее использование селектора.plan-svg—demo/smoke_isometric_contract.mjs:49, и там он опрашивается только в сценарии сshow_borders:true(структурный), где класс присутствует. Подтверждено исполнением: ни один из 44 не-iso golden-сценариев (Flat, редакторы, lighting, tray, split-wall и т. д.) не показал отличий. - Medium-1 / #134 (регрессия
large-house-zoom-250-darkпри выключенном Labs) — не было целью этого коммита (правка адресует High-1/High-2), но сценарий проходит чисто тем же коммитом. Открытая гипотеза из r1 (обёртка<g class=${iso ? ... }>как источник 0.1%-регрессии) теперь снята экспериментально: смена условия сisoнаisoLayers?.structuralна этой же строке устранила и её. Issue #134 остаётся открытым (S1-new) — закрытие issue не входит в роль ревьюера кода (§6 PROCESS.md), но по факту дефект, который он описывает, воспроизводимо больше не проявляется в этом дереве; стоит отметить это релиз-менеджеру/автору при обработке #134, а не оставлять как забытый висящий тикет. - AC7 (
unit+smoke+golden) — выполнен:hide_openingsне трогает cuts/state/light (не менялось в этом коммите, покрыто существующими тестами изnpm test);show_borders:falseвосстанавливает Stage-1 no-volume сцену — подтверждено golden. AC15 (Flat иisometric-no-borders-darkнеизменны) — выполнен для не-iso baseline; Stage-2-Iso-эталоны намеренно не приняты на этом этапе (§12.3 ТЗ) и остаютсяdifferent— соответствует контракту, а не находка. AC16 (три копии бандла байт-в-байт + зелёные typecheck/unit/build) — выполнен, доказаноcmp. test/isometric-contract.test.mjs(+5 строк) — источниковые regex-проверки на буквальный текст новых веток_baseVb()и рендера (if (!this._spaceDisplayForRender().showBorders),projectedFrame({ rect: flat, wallHeight: ISO_WALL_HEIGHT }),preserveAspectRatio=...'xMidYMid meet' : 'none', и т. д.). Тест умеет падать при откате этих строк (буквальное совпадение с исходником), но сам по себе не проверяет визуальный результат — это делаетgolden:verifyотдельно. Стиль согласован с уже существующими source-grep проверками того же файла (AC8/AC9/AC14 в r1). Оценка: приемлемо как дополнительная страховка, не как единственное доказательство AC7/AC15 — реальное доказательство даёт golden.- Документация (
docs/ISOMETRIC.md, ADR) — обновлена в том же коммите, описывает новое поведение точно (Stage-1 projected frame возвращается в no-borders ветке вместо union с Stage-2-геометрией); расхождений с кодом не найдено.
Чего не проверял
- Полные 127 браузерных смоков — правка узкая (две точки в одном файле), вне необходимости, определяемой diff'ом; прогнаны два целевых, прямо относящихся к затронутому коду.
python -m pytest tests_backend— нет изменений в Python.large-house-isometric-v1performance — вне гейта код-ревью, заблокирован #124 (решение владельца, повторяет позицию r1).- Принятие golden-эталонов Stage 2 Iso (5
different-сценариев) — не роль ревьюера; это ожидаемое, задокументированное в ТЗ состояние, а не находка. - Отдельный дифференциальный прогон на
origin/devв этом цикле — не повторялся:origin/devне изменился с r1, где причинность уже была локализована в диффе задачи, а не в окружении/Chromium.
Итог
Оба блокирующих High из r1 закрыты одним точечным коммитом (ef3cc98):
пересборка всех трёх копий бандла восстановлена и подтверждена cmp
(High-1); ветвление рамки/трансформации в show_borders:false теперь
воспроизводит принятую Stage-1 no-volume сцену вместо объединения с
Stage-2-геометрией, что подтверждено чистым golden:verify дважды подряд
(High-2). Ранее заведённая Medium-находка (#134) как побочный эффект того же
исправления тоже перестала воспроизводиться в golden — issue стоит закрыть
или, как минимум, обновить при её обработке, но это не входит в роль
ревьюера кода. Новых High/Medium-находок нет. Цикл r2/4 закрывается зелёным
вердиктом.