docs: code review document for #122 (r1, red)

Issue: #122
User-Visible: no

docs/reviews/CODE-REVIEW-122-r1.md

@@ -0,0 +1,248 @@
+# Код-ревью #122 — Isometric Stage 2: hidden visual polish (r1)
+
+- Issue: https://github.com/Matysh/houseplan-card/issues/122
+- Спецификация: `docs/specs/122-isometric-stage2.md` (SPEC-REVIEW-122-r1: green)
+- Диапазон: `git diff origin/dev...HEAD` на коммите `42b3f44` (`feat: add hidden
+  isometric stage 2`), плюс сопутствующие `76ce755` (ТЗ), `4c73e2c` (ревью ТЗ)
+- Роль: ревьюер кода (свежая сессия, без контекста реализации)
+- Вердикт: **красный · цикл r1/4 · High: 2 · Medium: 1 → #134**
+
+## Скоуп
+
+Класс файлов коммита `42b3f44`: A (`src/houseplan-card.ts`, `src/iso-openings.ts`
+(новый), `src/iso-projection.ts`, `src/iso-walls.ts`, `src/wall-thickness.ts`,
+`src/styles.ts`) + B (`test/**`, `demo/smoke_isometric_contract.mjs`,
+`demo/smoke_isometric_live_touch.mjs`, `demo/golden/matrix.mjs`,
+`tsconfig.test.json`) + C (`docs/ISOMETRIC.md`, `docs/ARCHITECTURE.md`,
+`docs/STATUS.md`, `docs/adr/122-isometric-stage2-composition.md`). Класс D
+(`dist/**`, `custom_components/houseplan/frontend/houseplan-card.js`,
+`demo/srv/assets/houseplan-card.js`, `demo/golden/baselines/**`) **не
+изменён вовсе** — см. High-1.
+
+Трейлеры на всех трёх коммитах диапазона корректны: `Issue: #122`,
+`User-Visible: no` (фича скрытая, публичного changelog не требует — верно).
+
+## Как проверялось
+
+| Гейт | Команда | Результат |
+|---|---|---|
+| Typecheck | `npx tsc --noEmit` | green, без вывода |
+| Unit | `npm test` | green, 766/766 |
+| Build | `npm run build` | green, `dist/houseplan-card.js` собран |
+| Синхронизация 3 копий бандла | `cmp dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js` и то же для `demo/srv/assets/houseplan-card.js` | **FAIL** — расходятся, byte 45, `git diff --stat` 327 insertions / 209 deletions на файл; см. High-1 |
+| Целевые браузерные смоки (названы в AC1/AC5/AC6/AC9/AC10/AC11 и напрямую тронуты диффом) | `node demo/smoke_isometric_contract.mjs`, `node demo/smoke_isometric_live_touch.mjs` — **после** `npm run build && cp dist/houseplan-card.js demo/srv/assets/houseplan-card.js`, т.к. коммит не обновил эту копию | оба green, все проверки в JSON-выводе `true` |
+| `npm run golden:verify` (диф меняет рендер/геометрию/слои — попадает в критерий «по необходимости») | `npm run golden:verify` (после того же build+copy) | 46/53 сценариев `passed`; 7 `different`: 5 ожидаемо-разных iso-сценариев без принятых эталонов (не блокер сам по себе, эталоны Stage 2 умышленно не принимаются на этом этапе — §12.3 ТЗ), но **2 сценария обязаны были остаться пиксель-идентичными и не остались** — см. High-2, Medium-1 |
+| Дифференциальная проверка причинности (A/B) | те же две команды (`npm run build`, `npm run golden:verify`) в `git worktree add /tmp/dev-check origin/dev` | `isometric-no-borders-dark` и `large-house-zoom-250-dark` оба `passed` на `origin/dev` — регрессия строго локализована в этом диффе, не окружение |
+| Backend | не прогонялся | правок в `custom_components/**/*.py` нет — не применимо |
+| Performance (`large-house-isometric-v1`) | не прогонялся | по ТЗ §10/§12.4 и решению владельца это pre-beta гейт, заблокированный отдельно #124; в этом ревью не требуется |
+
+**Чего не проверял и почему:** полный набор из 127 браузерных смоков (диф не
+задевает весь продукт — только iso-поверхность и косвенно `wall-thickness.ts`);
+`python -m pytest tests_backend` (без py-правок); performance-профиль (вне
+гейта код-ревью, зависит от #124); принятие golden-эталонов (не роль
+ревьюера и не требуется до pre-beta).
+
+## Находки
+
+### High-1 — три копии бандла не пересобраны; CI уже красный на этом SHA
+
+Коммит `42b3f44` не трогает `dist/houseplan-card.js`,
+`custom_components/houseplan/frontend/houseplan-card.js` ни
+`demo/srv/assets/houseplan-card.js` — все три остаются побитово равны
+промоушен-коммиту `a282f85` (`build: promote v1.63.0`), то есть Stage-1-коду
+без единой строки Stage 2.
+
+Воспроизведение:
+
+```
+$ npm run build
+$ cmp dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js
+dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js differ: byte 45, line 1
+$ git diff --stat dist/houseplan-card.js
+ dist/houseplan-card.js | 536 ++++++++++++++++++++++++++++++-------------------
+ 1 file changed, 327 insertions(+), 209 deletions(-)
+$ grep -c "invalid isometric opening input" dist/houseplan-card.js   # fresh build
+1
+$ git show HEAD:custom_components/houseplan/frontend/houseplan-card.js | grep -c "invalid isometric opening input"
+0
+$ git show HEAD:demo/srv/assets/houseplan-card.js | grep -c "invalid isometric opening input"
+0
+```
+
+Независимое подтверждение — CI Validate уже прогонялся на этом самом SHA и
+уже красный ровно на этом шаге:
+
+```
+$ gh run view 31746904479 --repo Matysh/houseplan-card
+X frontend in 36s
+  ✓ Build
+  X Card bundle snapshots in sync
+```
+
+Хендофф-комментарий исполнителя утверждает «три отслеживаемые bundle-копии
+byte-identical» — это не так; либо команда не выполнялась, либо результат
+интерпретирован неверно. Смысл дефекта не формальный: реальный
+`custom_components/houseplan/frontend/houseplan-card.js` (то, что грузит HA) и
+`demo/srv/assets/houseplan-card.js` (то, что видит демо-стенд и что реально
+тестируют браузерные смоки/golden в CI до пересборки) сейчас **не содержат
+Stage 2 вовсе**. `smoke`/`golden`/`performance_smoke` в CI зависят от
+`frontend` (`needs: frontend`) и поэтому на этом SHA не запускались вообще —
+ни один из пре-релизных гейтов не имеет зелёного прогона против настоящего
+кода задачи.
+
+Блокирует. AC16 не выполнен.
+
+### High-2 — `show_borders:false` в Iso не сохраняет пиксель-идентичный Stage-1 «no-volume» сценарий
+
+ТЗ §6.6 и ADR «Consequences» требуют: при `show_borders:false` в Iso
+Stage-2-геометрия отсутствует целиком, и AC7/AC15 требуют, чтобы
+`isometric-no-borders-dark` остался неизменным. Golden показывает
+значительную регрессию:
+
+```
+$ npm run build && cp dist/houseplan-card.js demo/srv/assets/houseplan-card.js
+$ npm run golden:verify
+different  isometric-no-borders-dark
+```
+
+Из `artifacts/golden/golden-report.json`:
+`differingPixels: 169919, diffRatio: 0.2194 (≈22%), maxObservedDelta: 206`.
+Повторный прогон — идентичные числа (детерминированно, не флейк). На
+`origin/dev` (та же команда, тот же `git worktree add /tmp/dev-check
+origin/dev`, тот же Chromium 151.0.7922.34) сценарий `passed` чисто.
+
+Причина по коду (`src/houseplan-card.ts:4503-4522`, `_isoScene()`):
+рамка проекции теперь всегда строится с
+`floorDepth: ISO_FLOOR_EDGE_HEIGHT` и объединяется с `isoOpeningBounds()` через
+`unionRect(flat, openingFrame)` **независимо** от `disp.showBorders` /
+`resolveIsoDecoration().structural`. В Stage 1 рамка включала только
+`wallHeight`; Stage 2 добавляет глубину пола и границы проёмов всегда, даже
+когда сама геометрия стен/проёмов/края пола не рисуется (`structural=false`).
+Это меняет коэффициент "сжатия" `floorView`, и весь план визуально сдвигается/
+масштабируется иначе, чем в принятом Stage-1 baseline — то есть нарушение не
+косметическое, это другой кадр.
+
+Похоже на конфликт двух требований ТЗ: AC12 хочет единую рамку, не зависящую
+от переключения `show_borders` (чтобы zoom не менялся), а AC7/AC15 хотят
+байт-в-байт то же изображение, что и Stage 1, где такой рамки не было. Решить
+это — работа автора (например, включать `floorDepth`/`openingFrame` в рамку
+только когда `structural===true`, и объяснить в ADR, почему AC12 всё равно
+держится). Ревьюер такие продуктовые компромиссы не решает.
+
+Блокирует. AC7 и AC15 не выполнены буквально.
+
+### Medium-1 — регрессия пикселей во Flat-сценарии при выключенном Labs → #134
+
+`large-house-zoom-250-dark` (без `labs`/`projection` в `demo/golden/matrix.mjs`
+— чистый Flat, Labs неактивен) на `origin/dev` проходит чисто (`passed`), на
+этой ветке — нет:
+
+```
+different  large-house-zoom-250-dark
+```
+
+`differingPixels: 920, diffRatio: 0.00105, maxObservedDelta: 201` —
+воспроизведено дважды подряд с идентичными числами (не флейк, не дрейф
+Chromium: версия в обоих прогонах `151.0.7922.34`).
+
+Это прямое нарушение AC2 («with Labs inactive... Flat... retain their
+existing state/action/pixel contract») и инварианта из `docs/ISOMETRIC.md`
+(«existing Flat DOM and pixels remain the reference»). В рамках бюджета этого
+ревью не удалось локализовать точный узел DOM/CSS, отвечающий за эту
+конкретную (небольшую по площади, но не нулевую и стабильную) разницу;
+кандидат — новая безусловная обёртка `<g class=${iso ? 'iso-floor-scene' :
+nothing} transform=${iso ? isoFloorMatrixCss() : nothing}>` вокруг содержимого
+плана в `src/houseplan-card.ts` (добавляется в разметку независимо от того,
+активен ли `iso`), но это не доказано построчно — отметка «предположение,
+требует дальнейшего разбора автором», а не факт.
+
+Площадь и видимое воздействие малы (0.1% пикселей одного сценария на большом
+зуме), поэтому находка не расширяет цикл #122 сверх уже блокирующих High-1/
+High-2, но контракт заявлен как безусловный и находка воспроизводима —
+заведён отдельный issue #134 со ссылкой на #122, а не оставлена как TODO
+в этом документе.
+
+## Что проверено и корректно
+
+- **AC1** (`unit`+`smoke`+код): `iso` остаётся единственным Labs id;
+  `demo/smoke_isometric_contract.mjs` подтверждает Flat как дефолт,
+  `isometric-contract.test.mjs` подтверждает отсутствие второго флага/ключа —
+  прочитано и прогнано, зелёно.
+- **AC5/AC6** (открывающиеся элементы): `src/iso-openings.ts` —
+  `buildIsoOpeningBasis`/`projectIsoOpening` алгебраически повторяют
+  существующий floor-symbol (поворот на 0–10° для gate, симметричные leaves
+  для window, один leaf с шарниром для door); `test/iso-openings.test.mjs`
+  проверяет неподвижность базиса при live-изменениях, `flip`-инверсию базиса,
+  совпадение `unavailable`≡no-contact через `openingAmount()`, границы через
+  `isoOpeningBounds`. Панели помечены `aria-hidden="true" pointer-events="none"`
+  (`src/houseplan-card.ts`, рендер `_renderIsoWalls`) — проверено чтением и
+  подтверждено смоком `live_touch` (`touchOpeningIsSafe`, `flatIsoActionParity`).
+  Тесты умеют падать: `assert.deepEqual(gateBasis.leaves.map(l =>
+  Math.abs(l.turnDeg)), [10, 10])` красится при любой правке угла;
+  аналогично для высот leaf.
+- **AC4** (floor edge): `buildIsoFloorGeometry`/`floorFootprintGeometry` —
+  внутренние/общие границы и вложенные дырки не создают ступень (тест
+  «canonical adjacent room union has no edge on its shared boundary», «floor
+  edge follows outer components without internal or nested steps»),
+  независимые partition/column не расширяют footprint (тест «Stage floor
+  footprint excludes detached independent physical bodies»), стабильность к
+  порядку/winding подтверждена отдельным тестом. Прочитано и прогнано зелёно;
+  тесты специфичны настолько, что удаление одной внешней грани красит их.
+- **AC8/AC14** (отсутствие второго свето-слоя, отсутствие новых сетевых/
+  конфигурационных путей): `isometric-contract.test.mjs`
+  («Stage 2 adds no schema, dependency, storage, network or HA action
+  surface») грепает исходники на `localStorage|fetch|XMLHttpRequest|
+  WebSocket|callService|config|schema` и `three|babylon|webgl` — прогнано,
+  зелёно. `src/styles.ts` не содержит нового источника света, только матовые
+  градиенты/тени — прочитано.
+- **AC9** (структурный кэш не растёт от HA/темы/hover): фингерпринт в
+  `_isoSource()` включает `rooms, walls, openCuts, openings (только
+  геометрия/флипы), partitions, roomDrafts, columns, cellCm, gridPitch,
+  wallKeyPitch, camera, wallHeight, floorEdgeHeight, algorithm: 3` и не
+  включает HA-состояние/тему/hover — подтверждено и тестом
+  `isometric-contract.test.mjs` («structural cache includes opening flips and
+  excludes live HA amount», прямая проверка исходного текста `_isoSource`
+  на отсутствие `_openingAmt|openingAmount|.hass|matchMedia|CSS.supports|
+  theme|hover`) и смоком `live_touch` (`sameWallFingerprint`,
+  `haUpdateReusesGeometry`, `contactUpdateMovesOnlyLivePanel`).
+- **AC11** (деградация): `resolveIsoDecoration` — чистая функция, тест
+  «decoration degradation never removes structure or creates floating
+  panels» покрывает все 4 комбинации `filtersSupported`/`forcedColors`/
+  `hideOpenings`/`showBorders`; `src/styles.ts` содержит
+  `@media (forced-colors: active)` и `@supports not (filter: blur(1px))`,
+  оба убирают только тени/нюанс материала, не геометрию. Прочитано, логика
+  соответствует ТЗ.
+- **AC13** — намеренно не проверялось: и по ТЗ (§10/§12.4), и по решению
+  владельца это остаётся pre-beta гейтом, заблокированным #124; хендофф
+  честно пишет «не запускались».
+- Трейлеры коммитов, `User-Visible: no`, отсутствие изменений в
+  `custom_components/**/*.py`, i18n и schema — проверено чтением диффа,
+  корректно.
+
+## Чего не проверял
+
+- Полные 127 браузерных смоков — диф касается только iso-поверхности и
+  `wall-thickness.ts`; прогнаны только два целевых, названных в AC и прямо
+  изменённых в этом диффе.
+- `python -m pytest tests_backend` — нет изменений в Python.
+- `large-house-isometric-v1` performance — вне гейта код-ревью при открытом
+  #124 (см. ТЗ §10, решение владельца).
+- Принятие golden-эталонов — не роль ревьюера; данные из
+  `artifacts/golden/golden-report.json` использованы только как
+  диагностика причинности регрессии, не как повод принять/отклонить
+  baseline.
+- Точная DOM/CSS-причина Medium-1 не изолирована построчно — честно оставлено
+  как открытый вопрос автору в #134.
+
+## Итог
+
+Две High-находки блокируют цикл r1: код не собран в те артефакты, которые
+реально исполняются (High-1, CI уже подтверждает это независимо), и Stage 2
+в текущем виде ломает пиксельный контракт, который ТЗ объявляет безусловным —
+`show_borders:false` в Iso (High-2). Третья находка (регрессия чистого Flat)
+воспроизводима, но мала по воздействию — заведена как Medium #134, а не
+оставлена как TODO. Работа предметно сильная — покрытие юнит-тестами
+геометрии открытий и floor edge выдерживает мутации, изоляция
+структурного/live кэша сделана аккуратно и подтверждена и тестами, и
+смоками — но AC16, AC7 и AC15 не могут считаться выполненными до
+пересборки бандла и разбора причины пиксельной регрессии `show_borders:false`.