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

CODE-REVIEW #732 · заход r1

Материал: 9b2bb26d06eba8c81bde1600117bfeed6eda08fa (рабочая копия на нём же, git log --oneline origin/dev..HEAD — один коммит). Трек: show (до 3 AC, лёгкое код-ревью). Мутанты по диффу не запрашивались.

Скоуп

После #714 и #724 в 2.5D-оверлеях остались пустышки: renderIsoOverlayGrounds и renderIsoRaisedOverlays безусловно возвращали пустой SVG, groundRadius вычислялся и только сравнивался, тестовые фикстуры носили пять полей (view, referenceView, stageSize, layers, selectedDeviceId), которых IsoOverlaySceneInput/IsoOverlayRenderEntry не читают.

Три AC:

• AC1 — пустые рендереры, вызовы и groundRadius удалены; DOM 2.5D не меняется.
• AC2 — фикстуры не носят непрочитанных полей, typecheck тестов это закрепляет.
• AC3 — мутанты не привязаны к удалённому коду, mutation-gate --check без новых предупреждений.

Правка внутренняя (Нечего видит человек: Ничего), трейлер User-Visible: no стоит — изменений CHANGELOG не требуется и нет.

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

Прочитан весь дифф (src/iso-scene-render.ts, src/houseplan-card.ts, 4 тестовых файла). Дополнительно:

1. AC1 доказан чтением, не только заявлением. Сверил версию на origin/dev: renderIsoOverlayGrounds/renderIsoRaisedOverlays уже ДО этого коммита безусловно возвращали emptySvg() независимо от входа (src/iso-scene-render.ts:1117-1128 на dev). Значит удаление этих функций и двух интерполяций ${isoFrame?.grounds ?? nothing} / ${isoFrame?.raised ?? nothing} в houseplan-card.ts не может изменить отрисованный DOM — там и раньше ничего не рендерилось. Это сильнее, чем повторный прогон golden: логически исключает регрессию по построению, а не по выборке эталонов. Прогнал node --test test/isometric-contract.test.mjs — 14/14 зелёных, включая переписанные проверки «ничего не рендерится в overlay-поверхность» и «grounds/raised/groundRadius нигде не остались».
2. AC2. Прогнал node --test test/iso-overlay-fixture-types.test.mjs — 3/3 зелёных. Проверил дисциплину «тест умеет падать»: временно вернул view: {...} в один из литералов overlayScene({...}) в test/iso-scene-render.test.mjs — первый чек теста немедленно покраснел с точной локацией (test/iso-scene-render.test.mjs:346 ... 'view' does not exist in type 'OverlaySceneFixture'), откатил правку, git status — чисто. Также прогнал test/iso-scene-render.test.mjs и test/iso-stage6.test.mjs целиком (после npx tsc -p tsconfig.test.json
   • fix-test-build.mjs) — 46/46 зелёных.
3. AC3. Прогнал node scripts/mutation-gate.mjs --check — browser guards: 200/200, предупреждений mutation registry: 3, ни одного FAIL. Проверил содержание трёх предупреждений — все про несовпадение --test-name-pattern с именами, содержащими ${…} (#650), никак не связаны с groundRadius/renderIsoOverlayGrounds/renderIsoRaisedOverlays. Совпадает с заявлением автора «3, как на dev».
4. Проверил по всему репозиторию (src/, test/, docs/), что удалённые имена (renderIsoOverlayGrounds, renderIsoRaisedOverlays, groundRadius) не остались висящими ссылками нигде, кроме архивного docs/specs/471-*.md (исторический документ, не правится) и самих регрессионных тестов, которые проверяют их отсутствие.
5. Прогнал node scripts/smoke-select.mjs --base origin/dev --head HEAD: «Прямое совпадение» — 10 смоков по символу cellCm (строки диффа рядом со стёртыми полями просто содержат этот параметр по соседству, функционально он не менялся). Символы grounds/raised/groundRadius в выборке не всплыли — ожидаемо, эти смоки ходят по DOM, а не по именам символов. AC1 отдельно называет конкретные iso-смоки (smoke_iso_flat_parity, smoke_isometric_contract, smoke_isometric_live_touch, smoke_iso_tiles) и golden — автор в комментарии заявляет их зелёными; учитывая пункт 1 (DOM гарантированно не меняется по построению), решение не перегонять браузерные смоки и golden самостоятельно.

Что проверено и корректно

• IsoFramePresentation.grounds/.raised и обе привязки в карточке убраны вместе; IsoFramePresentation.overlays — отдельное поле, используется (isoFrame?.overlays → isoOverlaySceneBounds/fit, houseplan-card.ts:10719), дальше живёт не тронутым.
• emptySvg() остаётся нужной — используется в renderIsoUnderlay, renderIsoShadows, renderIsoWalls; не мёртвый код.
• isoRaisedOverlayHalfSize (читается в iso-stage6.test.mjs) не removed — проверено, что функция осталась и не была спутана с удаляемыми.
• Переписанные тесты #724 AC2 и #713 AC3 осмысленно переносят проверку «зум/ресайз не двигают раскладку» на продакшн-путь (через resolveIsoScene → liveFrame), а не просто удаляют покрытие — заменяющий кейс (zoomedIn/zoomed в test/iso-stage6.test.mjs) сам по себе проверяет то же инвариант через структурный кэш.
• Трейлеры Issue: #732, User-Visible: no на месте; CHANGELOG не правится — согласовано с «Нечего видит человек: Ничего».

Чего не проверял и почему

• npx tsc --noEmit, npm test, npm run build целиком — не перегонял: Validate на этом же SHA зелёный (https://github.com/Matysh/houseplan-card/actions/runs/36790189760), бюджет §4 не тратится повторно. Частично (tsc -p tsconfig.test.json + конкретные тестовые файлы) прогнал точечно ради проверки AC, не как замену гейту.
• npm run golden:verify (капча браузером) — не прогонял: метки ci:golden на issue нет, и (см. п.1 выше) отрисовка iso-overlays-svg была пустой уже на dev, так что 19/19 эталонов, заявленные автором, логически не могут измениться этим диффом. Останавливаться на повторном браузерном прогоне ради этого посчитал избыточным для трека show.
• Остальные iso-смоки (smoke_iso_flat_parity, smoke_isometric_contract, smoke_isometric_live_touch, smoke_iso_tiles) браузером сам не гонял — Chromium не поднимал (#696: тело issue явно смоук не называет; AC1 использует их как способ доказательства, но доказательство DOM-инварианта получено чтением кода надёжнее любого выборочного прогона). Решение по строке из smoke-select: все 10 «прямых совпадений» по cellCm — не прогонял отдельно, полагаюсь на п.1 (grounds/raised уже были инертны) и на зелёный Validate.
• npm run invariants — не прогонял: дифф не трогает геометрию модели и ссылки на неё, только убирает неиспользуемое поле и мёртвые функции рендера; расчёт раскладки (buildIsoOverlayRenderScene, isoRaisedOverlayHalfSize) не менялся.
• python -m pytest tests_backend — не прогонял: дифф не касается custom_components/**/*.py.
• Ручного тестирования в браузере не делал (вне бюджета трека show и не требуется отдельным AC сверх перечисленного).

Находки

Блокирующих (High) и находок в скоупе (Medium) нет.

Вне скоупа, не заводил отдельным issue — оценил как Low, не Medium: автор сам явно называет в комментарии два кандидата на будущую уборку — IsoOverlayFitEnvelopeInput.stageSize (объявлено, передаётся из карточки, не читается — проверил чтением: resolveIsoOverlayFitEnvelope, src/iso-scene-render.ts:646-666, поле действительно нигде не используется в теле функции) и пустой iso-overlays-svg (можно убрать ценой смены DOM, требует отдельного решения). Оба — нулевой функциональный риск (один неиспользуемый опциональный параметр, один инертный DOM-узел, на который явно опираются смоки), явно зафиксированы в публичном комментарии к issue, и их самостоятельная правка означала бы расширение скоупа трека show сверх заявленных 3 AC. Открывать отдельный issue ради поля, которое ничего не стоит и не рискует, счёл избыточным для этого захода; если ревьюер следующего раунда сочтёт иначе — материала для S1-new достаточно (см. абзац выше).

Вердикт

Все три AC доказаны: автотестом с подтверждённой способностью падать (AC2), прогоном с нулевым числом FAIL и предупреждениями, не связанными с диффом (AC3), и разбором по коду с чтением прежней и новой версии функций, подкреплённым прогоном обновлённых regression-тестов (AC1). Диффа, который менял бы видимую пользователем величину, нет — правка внутренняя.

Зелёный.

---

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

• Ветка: issue/732-iso-overlay-stubs, коммит 9b2bb26d06eb — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: be4c320efecaf530c61c44d4a75a0ce30623e632

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

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