houseplan-card/docs/reviews/CODE-REVIEW-724-r1.md

CODE-REVIEW — issue #724, заход r1

Материал раунда: 877dccd8430aad63e4ebe8c93dd10febfbdae25e (git log --oneline origin/dev..HEAD — один коммит; git diff origin/dev...HEAD --stat — 9 файлов, +193/-182). Трек show. Validate на этом SHA: success (https://github.com/Matysh/houseplan-card/actions/runs/36784536500).

Скоуп

Продолжение #714: убрать данные оверлейной 2.5D-сцены, которые ничего не решают — IsoWallSilhouette/isoWallSilhouettesOf (служили только ключом кэша), поля tether/grounding/raisedScene/owner.area и входы hovered/focused/selected/filtersSupported в IsoOverlayPlacement* (игнорировались резолвером), флаг resolveCollisions и парный слот кэша fit/live. Кэши размещений и рендер-сцены переключены на ключ structure: IsoWallGeometry (scene.geometry из resolveIsoScene) вместо искусственно строившегося массива силуэтов. Видимое поведение не меняется (User-Visible: no, тело issue: «Что видит человек: ничего»).

Соответствие docs/SCOPE.md: техдолг внутри уже принятой подсистемы 2.5D (Core user jobs это не расширяет и не сужает — правка корректно вне скоупа трека spec, только show/code).

Как проверялось

Дельта нулевая (r1, предыдущего раунда нет) — разбор полный.

1. Чтение диффа построчно по всем 9 файлам (git diff origin/dev...HEAD для каждого файла отдельно): iso-overlays.ts, iso-scene-render.ts, houseplan-card.ts, scripts/mutation-registry.mjs, docs/ISOMETRIC.md, 4 тестовых файла.
2. Проверены все использования удалённых символов по всему дереву (wallSilhouettes|raisedScene|isoWallSilhouettesOf|resolveCollisions| IsoWallSilhouette|.tether|.grounding|owner.area|filtersSupported) — единственные оставшиеся совпадения (filtersSupported в iso-openings.ts) относятся к другому, не удалённому контракту (capability-проба фильтров материалов), не к IsoOverlayPlacementInput.
3. Сверены find-строки всех трёх патчей mutation-registry.mjs (W2, W6, и косвенно W5) с реальным текстом src/iso-scene-render.ts / src/houseplan-card.ts — совпадают побайтово (grep -n на точные строки).
4. Прогнаны сами мутанты (не просто --check, а исполнение патча и гварда):
   • node scripts/mutation-gate.mjs --id=iso-placement-cache-survives-silhouette-change → поймано 1 из 1 (гвард — новый #473 W2|#724 AC2 — реально краснеет на мутанте, дисциплина «тест умеет падать» подтверждена не чтением, а исполнением);
   • --id=stage3-w6-no-borders-keeps-raised-plates → поймано 1 из 1;
   • --id=stage3-w5-runtime-nudge-writes-storage → поймано 1 из 1 (описание больше не говорит про «nudge», но гвард по-прежнему реагирует на посторонний localStorage.setItem).
   • node scripts/mutation-gate.mjs --check → 3 предупреждения реестра — то же число, что автор указал как совпадающее с dev.
5. Типы: tsc --noEmit прогнан как часть npm run build (см. «Гейты»).
6. Юнит-тесты новых AC2-кейсов прочитаны построчно: они идут по продуктовому пути createIsoStructuralSource → resolveIsoScene → buildIsoOverlayRenderScene, а не через синтетический structureOf() — ключ кэша берётся из настоящей структурной LRU, а не собирается вручную, так что тест доказывает именно продовое поведение, а не артефакт фикстуры.
7. Golden 2.5D и полный набор (npm run golden:verify) — см. «Гейты».

Находки

Находок нет.

Разобрано отдельно и не являются находками:

• Тождественность семантики raisedScene→visualScene: до правки raisedScene всегда равнялся visualScene в обеих ветках (floor/raised) — замена мест чтения на visualScene (в layoutSignature, в isometric-contract.test.mjs, в iso-overlays.test.mjs) не меняет значение, только имя поля. Подтверждено чтением обоих диффов iso-overlays.ts/iso-scene-render.ts.
• Идентичность объекта-ключа: раньше wallSilhouettes был новым массивом при каждом промахе кэша resolveIsoScene (та же исходная семантика, что описана в docs/ISOMETRIC.md до правки — «структурная LRU выдаёт тот же объект» только при попадании в кэш). Теперь роль идентичности играет geometry (тоже новый объект на каждый промах структурного кэша, buildIsoWallGeometry(...) вызывается заново). Поведение инвалидации кэша оверлеев (переживает zoom/resize/HA-состояние, пересобирается при правке геометрии) не изменилось — подтверждено и чтением, и двумя новыми тестами AC2, прогнанными зелёными, и третьим прогоном через явную мутацию ключа (см. п. 4 выше).
• ISO_PLACEMENT_CACHE_ANY = {} as IsoWallGeometry в мутанте W2 — фиктивный объект только для константного ключа WeakMap, не участвует в рантайме продукта, используется исключительно как патч-мутант.

Проверено и корректно

• AC1. Все перечисленные в ТЗ мёртвые поля/входы/типы удалены, ни одного оставшегося упоминания в src/**, demo/**, test/**, scripts/**. tsc --noEmit (через npm run build) проходит. 21 изометрическая golden-сцена и полный набор golden — passed, без новых кадров (эталоны не тронуты в диффе: git diff --stat не содержит demo/golden/**). Юнит-тесты — 3314 passed / 0 failed / 1 skipped (тот же skip, что и на dev — не новый). Четыре явно названных в ТЗ смока (smoke_iso_flat_parity, smoke_isometric_contract, smoke_isometric_live_touch, smoke_iso_tiles) прогнаны лично (после npm run bundle:sync, иначе харнесс бьётся о устаревший demo/srv бандл) — все четыре зелёные.
• AC2 (кэш). Два новых теста в test/iso-scene-render.test.mjs (#724 AC2: the overlay scene survives zoom, resize and HA state... и #724 AC2: a room edit that changes the owner is a new structure...) идут по продуктовому пути и явно проверяют: (а) одинаковые стены → тот же geometry-объект → zoom/resize/HA-апдейт не пересчитывают размещение (assert.strictEqual); (б) более толстая стена той же комнаты → новый ключ структурной LRU → новый geometry → новая сцена оверлеев, но позиция тайла не меняется (стены не двигают значок); (в) правка комнаты, меняющая владельца точки, даёт новую структуру и нового владельца (не «зависшего» от снесённого плана). Мутационная проверка (п.4) подтверждает, что при подмене реального ключа на константу тест красный.
• AC3 (реестр мутантов). W2 переключён на реальный ключ input.structure (id сохранён для истории, это осознанно и не создаёт путаницы — гвард проверяет актуальный код), гвард дополнительно гоняет оба новых AC2-теста. W6 патч переведён на новую строку structure: structural.geometry. W5 описание больше не упоминает «nudge» (которого с #713 нет), гвард не менялся (smoke), проверен мутацией — реагирует. Регулярка isometric-contract.test.mjs больше не ищет силуэтную конструкцию, а проверяет актуальный ключ и явно (assert.doesNotMatch) убеждается, что wallSilhouettes|isoWallSilhouettesOf|resolveCollisions не вернулись. mutation-gate --check — 3 предупреждения, совпадает с заявленным (и это число не новое: структурные предупреждения реестра относятся к посторонним мутантам, не к правкам этого диффа).
• docs/ISOMETRIC.md. Добавленный абзац («placement и render-scene caches are keyed by the wall geometry… #724») точно описывает код: ключ — scene.geometry, инвалидация — на любую структурную правку (стены, комнаты, проёмы — все входят в фингерпринт структурной LRU, см. неизменную часть документа выше по файлу). Никакая другая пользовательская величина в документе не задваивается (правка внутренняя, §8 не применим — нет числа, видимого пользователю).
• Трейлеры. Issue: #724, User-Visible: no — присутствуют, соответствуют телу issue («видит человек: ничего»). Правок changelog нет ни в одном файле — корректно для User-Visible: no.
• Ветка/dev. Материал — ветка как есть (трек show, #696), ребейз не требовался в этом раунде.

Гейты — что прогнано и почему

Дешёвые гейты на 877dccd8 уже подтверждены Validate (tsc --noEmit, npm test, npm run build со сверкой бандла) — по правилам раунда их не обязательно перегонять, но для проверки AC1/AC2 (кэш, тип IsoWallGeometry как ключ WeakMap) и мутационного реестра (AC3) потребовалось фактическое исполнение, поэтому прогнано:

• npm run build (включает tsc --noEmit) — зелёный.
• npm test — зелёный, 3314 passed / 0 failed / 1 skipped (тот же skip, что и на dev).
• npm run bundle:sync — нужен был для смоков (демо-харнесс подхватывает demo/srv/assets, не dist/ напрямую; без синка смоки падали с Failed to fetch dynamically imported module — это не находка, а артефакт локального состояния, не тронутого диффом).
• 4 браузерных смока, явно названных в ТЗ (smoke_iso_flat_parity, smoke_isometric_contract, smoke_isometric_live_touch, smoke_iso_tiles) — все зелёные.
• node scripts/smoke-select.mjs --base origin/dev --head HEAD — ответ НЕОПРЕДЕЛЁННОСТЬ (ни один смок не связан доказуемо с изменёнными символами IsoWallGeometry, isoOverlayPlacementCache и т. д.) плюс список слабых связей (35 смоков через общий _config). Так как ТЗ явно называет 4 смока (закрыты выше), а остальные связи инструмент считает слабыми (решение ревьюера, не обязанность) — дополнительные смоки из списка слабых связей не прогонялись: изменение не трогает ни один из путей, которые они бы покрывали (диалоги, локали, kiosk и т. п.), а прямая логика 2.5D покрыта названными 4 смоками + golden + юнитами.
• npm run golden:verify — зелёный, полный набор, включая все 21 изометрические сцены (isometric-*), без единого fail. Формально ci:golden метки на этом PR не было, но правка трогает рендер-путь (iso-scene-render.ts), поэтому прогнано по инструкции «golden, если diff трогает рендер», а не по метке.
• node scripts/mutation-gate.mjs --check — зелёный (3 предупреждения, как на dev).
• Точечное исполнение трёх изменённых мутантов (--id=iso-placement-cache-survives-silhouette-change, --id=stage3-w6-no-borders-keeps-raised-plates, --id=stage3-w5-runtime-nudge-writes-storage) — все поймали мутацию (поймано 1 из 1 в каждом случае).

Что не проверял:

• python -m pytest tests_backend — не запускал: диффа в custom_components/**/*.py нет.
• npm run invariants — не запускал: диффа в геометрии модели (координаты, wall_segments, room poly) нет, правка — кэш-ключи и мёртвые поля presentation-слоя, не модель.
• Performance-профили (benchmark:*) — не запускал: не названы в AC, профиль кэша не меняет алгоритмическую сложность (тот же паттерн WeakMap-по-идентичности-объекта, просто другой объект-идентификатор).
• Полный npm run gate:small (автор его уже прогнал и привёл результат «зелёный», а дешёвые компоненты — tsc/test/build — перепроверены выше напрямую) — не гонял отдельно, поскольку это предрелизный/полный набор, избыточный для объёма этой задачи (PROCESS.md §8); прогонял именно то, что этот диф требует доказать.
• Слабые/широкие смоки из списка smoke-select (35 шт. по общему _config, и «широкий» порог 56) — не прогонял: связь слабая и признана ревьюером недостаточной, диффа за пределами iso-* файлов нет.
• Визуальный минимум (8 смоков, упомянутый выводом smoke-select) — не прогонял: он актуален для диффов, трогающих общий визуальный рендер-конвейер плоского вида; здесь диф ограничен 2.5D-веткой, покрытой явно названными смоками и golden напрямую.

Побочные наблюдения (не находки)

Во время прогона npm run build/bundle:sync в рабочей копии возникли недетерминированные хеши файлов dist/houseplan-assets/* (новые имена чанков при том же содержимом) — это артефакт локальной пересборки ревьюера, не диффа PR; откачено (git checkout -- dist/ + удаление новых untracked файлов) перед завершением ревью, рабочая копия чистая.

Вердикт

Все AC доказаны: автотестом (AC2 — двумя новыми тестами, проверенными исполнением мутации на красноту), реестром мутантов, исполненным напрямую (AC3), и прямым исполнением golden/смоков/юнитов/build (AC1). High/Medium находок нет. Зелёный.

---

Материал раунда

• Ветка: issue/724-iso-overlay-dead-data, коммит 877dccd8430a — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: d3fee2ae704d2a89b3317e722e04272ef5d71453

  git log --all --format='%H %T' | grep d3fee2ae704d
  

• Тело issue: b6202e7a98a172c6dc44df0980c516b466317988c8375cb5fad9e728bb4fa820
• Вердикт конвейера: green · High 0