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

CODE-REVIEW-649-r1

Issue: #649 · 2.5D, этап 6: объёмные иконки-плитки с тенью, мягкий свет по солнцу, слой декора без изменений, режим из experimental в General settings.

Материал: git log --oneline origin/dev..HEAD / git diff origin/dev...HEAD, ровно на SHA de4521823ba69c390fda4c0534e52e24a63cc66a (рабочая копия уже на нём; ветку не переключал, git fetch/pull/checkout не выполнял).

de452182 test(mutants): stage3-w12 guard follows the renamed #649 contract test
e59c89df feat: 2.5D stage 6 — public setting, raised tiles, soft sun, theme-free walls (#649)

164 файла, +5570/−3767. Трейлеры на обоих коммитах корректны: Issue: #649, User-Visible: yes на e59c89df (оба changelog в том же коммите), User-Visible: no на de452182 (гейт мутанта, без поведенческих изменений). Заход r1 код-ревью, ТЗ (r2 в теле issue) уже прошло зелёное SPEC-REVIEW.

Скоуп

Реализация всех 4 пунктов ТЗ: (1) объёмные плитки-маркеры с торцом, тенью и рамками только в 2.5D; (2) мягкий свет по солнцу в 2.5D вместо проекции Flat-клиньев; (3) фикс толщины линий мебели + стены/проёмы/подписи не зависят от темы; (4) settings.volumetric_view — публичная установка в General settings, удаление alpha-входа, кнопки шапки и пункта меню телефона. AC1–AC13 таблицы приёмки соответствуют этим пунктам; AC13 (перформанс) и часть AC1/AC12 (Linux golden) по тексту ТЗ — предрелизные гейты.

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

Ревью велось параллельно пятью под-агентами (каждый — отдельная нить чтения кода/тестов по разделу ТЗ) плюс моя собственная проверка: process-документы, трейлеры, docs-дифф, golden/mutation-реестр, числовые пересчёты, и реальный прогон гейтов в песочнице (см. таблицу ниже). Каждое утверждение AC сверялось построчно с кодом; для тестов/смоков проверялось, может ли конкретная проверка реально упасть (не просто «элемент существует»), и что парные мутанты реестра патчат существующие, не протухшие строки.

Гейты — что прогнано и с каким результатом

Гейт	Прогнан	Результат
Validate (мутанты) на de452182	нет, зачтён по ссылке (условие задачи)	success, https://github.com/Matysh/houseplan-card/actions/runs/36153319135
npx tsc --noEmit (через npm run build)	да	0 ошибок
npm run build	да	чисто, дифф dist/** после bundle:sync — нулевой (git status пуст), т.е. закоммиченные dist/, custom_components/houseplan/frontend/, demo/srv/assets побайтово совпадают со свежей сборкой
npm test	нет (принято по Validate)	—
python -m pytest tests_backend	нет — в песочнице нет модуля pytest (pip show pytest → not found)	не прогнан; см. «Чего не проверял»
node demo/smoke_iso_tiles.mjs (AC2–AC6)	да	OK, все 33 проверки true
node demo/smoke_iso_sun.mjs (AC7)	да	OK, все 18 проверок true
node demo/smoke_iso_theme_walls.mjs (AC8, AC9)	да	OK, все проверки true, включая prismOpaque, furnitureSameWidth
node demo/smoke_volumetric_setting.mjs (AC10, AC11)	да	OK, все 18 проверок true
node demo/smoke_isometric_contract.mjs	да (прямое совпадение smoke-select)	OK
node demo/smoke_isometric_live_touch.mjs	да (прямое совпадение)	OK, включая noBordersHasNoRaisedOrVolumeLayers, kioskReadsSetting, kioskSettingOffIsFlat
node demo/smoke_room_fit.mjs	да (прямое совпадение, _effectiveProjection)	OK
node demo/smoke_grid_scale_invariance.mjs	да (прямое совпадение)	OK, включая isoHasNoCardToggle
node demo/smoke_live_pan_coverage.mjs	да (прямое совпадение)	OK
node scripts/smoke-select.mjs --base origin/dev --head HEAD	да	78 прямых совпадений, множество «слабых связей» — решение по строкам ниже
node scripts/model-invariants.mjs	нет, признано неприменимым	задача не меняет каноническую геометрию (стены/комнаты/layout); только презентационные множители 2.5D и новые рендер-модули — см. обоснование ниже
npm run golden:verify	нет	Flat golden — предрелизный гейт по тексту самого AC1; iso-сцены Stage 6 сознательно не в матрице (см. «Проверено и корректно»)

Решение по smoke-select (78 прямых совпадений). Ядро (_effectiveProjection, _ensureIsoSceneRuntime, _isoSceneRuntime, _kiosk в связке с проекцией) — прогнано выше (5 смоков). Остальные ~70 совпадений держатся на широко встречающихся символах (_spaceModel, _mode, _openSettingsDialog, _wallUnionGeometry, _physicalBodiesR, _spaceWalls), которые задача не меняет по существу (её дифф не трогает работу стен/junction/диалогов — только использует их выход для 2.5D-презентации и упомянутые символы совпадают, потому что строки диффа их вызывают, а не потому что их поведение изменилось). Не прогонял: риск регресса в этих смоках от данной задачи оцениваю как пренебрежимо малый (сами эти системы не тронуты); при необходимости — решение ревьюера, зафиксированное здесь.

Находки

Medium (в скоупе — правится в этой же задаче)

M1. AC7: обрезка барьерами (физические тела и Solid-перегородки) луча света в 2.5D нигде не доказана тестом.

src/iso-sun.ts:111-114 (computeIsoSunBeams) корректно переиспользует directionalOccluders/floorMinusBodies из physical-geometry.ts — тот же приём, что и Flat в houseplan-card.ts:9961-9971 (_sunInputs() строит общий occluders для обоих рендеров, houseplan-card.ts:9889-9894). Формула, градиент, стопы, лучики, длина от elevation, подоконник — всё это доказано test/iso-stage6.test.mjs и demo/smoke_iso_sun.mjs (я прогнал последний — все 18 проверок true). Но ни один юнит, ни смок, ни одна из STAGE6_ACCEPTANCE_SCENARIOS/GOLDEN_SCENARIOS не ставит физическое тело или Solid-перегородку на пути 2.5D-луча и не проверяет, что часть параллелограмма действительно вырезается.

Конкретный сценарий отказа: если строка directionalOccluders(input.occluders, away, travel) (src/iso-sun.ts:112) получит перепутанный знак направления (away вместо вектора к солнцу) или неверную длину экструзии (travel вместо depth), луч в 2.5D будет либо не обрезаться преградами вовсе, либо обрезаться неправильной стороной — а обрезка контуром комнаты по-прежнему даст непустой результат, так что ни один текущий тест не покраснеет. Это ровно тот защитный элемент AC, который ТЗ называет прямо («Обрезка... теми же барьерами, что у Flat: физические тела и Solid-перегородки») и который не входит ни в один из трёх названных в таблице приёмки мутантов («длина без elevation», «лучики на тёмном полу», «Flat-клинья остались»).

Требуется: один юнит (или расширение demo/smoke_iso_sun.mjs) со сценой, где физическое тело/Solid-перегородка стоит на пути луча, и явная проверка, что часть полигона вырезана (например, площадь polys меньше площади без преграды, либо конкретная точка за преградой не в polys).

Low (правлю запись/снимаю с примечанием, отдельного цикла не требуют)

L1. Устаревшие «alpha/hidden»-формулировки, оставшиеся рядом с уже исправленным текстом в тех же файлах.

• docs/DEVELOPMENT.md:231-234 — «Hidden Stage 3 rendering is a separate iso-scene-render-* chunk. An alpha-off View must not request it» — противоречит исправленному абзацу пятью строками ниже (249-251) того же файла: «Since #649 the registry is empty: 2.5D left alpha for the General settings switch settings.volumetric_view».
• docs/ARCHITECTURE.md:31 — таблица модулей всё ещё называет iso-scene-render.ts «hidden alpha-only Stage 4 scene/runtime boundary», хотя абзац на строках ~86-88 того же файла уже описывает гейт как settings.volumetric_view (#649).
• src/houseplan-card.ts:627-628 — комментарий над _isoSceneRuntimeLoader: «Stage 4 is an opt-in alpha surface» — противоречит корректному соседнему комментарию (~2305) «one installation-wide switch… no alpha».
• demo/benchmark_large_house.mjs:145 — «experimental Iso runtime», косметика.

Ни на поведение, ни на один тест не влияет — только вводит в заблуждение читателя кода/доков. Не блокирует; правится по усмотрению автора либо снимается.

L2. Число в хендоффе не совпадает с фактическим значением в диффе. Комментарий автора в issue: «monolith-baseline.json: bundleBytes 2 528 415 → 2 540 748»; фактический дифф — 2528415 → 2540346 (разница 402 Б). Артефакт сам по себе непротиворечив (утверждение «монолит 12 880 строк (потолок 12 889)» подтверждено — wc -l src/houseplan-card.ts = 12880); это опечатка в тексте хендоффа, не в коде. Снимаю как не влияющую на решение.

L3. isoTileStateCss() жёстко считает торец состояний в theme: 'light' без ветки .theme-dark.<state>. src/iso-tiles.ts:109-115 — для всех шести состояний (on/open/lock-locked/ lock-unlocked/unavail/alarm) правило --iso-edge не различает тему. Сейчас безопасно: ни один из цветов ISO_STATE_BODIES не проходит порог luma<70 и не близок к белому (минимум — #F0410C, luma ≈ 98.4), так что смена темы результата не меняет. Если в будущем добавят тёмное состояние, тёмная тема получит светлотемный торец #5b5e5a вместо #4a4a4a — ни юнит, ни смок этого не поймают. Не требует правки сейчас; фиксирую для следующей задачи, которая тронет ISO_STATE_BODIES.

L4. AC5: явно не проверены «Alert > чистый Hover» и «Alert > чистый Selected» по отдельности. demo/smoke_iso_tiles.mjs проверяет alarm поверх уже активной комбинации sel + focus-visible, а не поверх изолированного hover или изолированного selected. По специфичности CSS-селекторов (iso-tiles.styles.ts:61-68) приоритет должен сохраниться и в этих парах — проверено чтением, не исполнением.

L5. Нет отдельного теста на виртуальные устройства (Q1: «без пунктира, как обычные»). noRing в smoke_iso_tiles.mjs проверяется на обычных маркерах; для .dev.virtual полагаюсь на специфичность правил (5 классов .stage.projection-iso.mode-view .dev... против 2 у .virtual .device-shell- frame{border-style:dashed}) — проверено чтением.

L6. Тени и плитки продолжают рисоваться при show_borders: false, хотя SVG-слой raised-overlay корректно не строится (это отдельный, уже существующий и по-прежнему проходящий тест noBordersHasNoRaisedOrVolumeLayers в demo/smoke_isometric_live_touch.mjs). Автор сам называет это решением, которое стоит проверить ревьюеру. Прочитал код: isoOverlays?.devices.get(d.id) (houseplan-card.ts:11167) через опциональную цепочку не роняет рендер плиток и их теней, когда оверлей-сцена не построена — поведение соответствует намерению (плитки/тени — независимый HTML/CSS слой, не завязанный на SVG raised-overlay). Автоматической проверки именно этого поведения (ни в одну, ни в другую сторону) нет ни в одном тесте. Не входит ни в один AC таблицы приёмки; фиксирую как задокументированный пробел покрытия, не как дефект.

Проверено и корректно (доказано; для геометрии/формул — построчным

пересчётом, не «на глаз»)

• AC1 (Flat неизменен). Структурно исключено смешение: все правила iso-tiles.styles.ts и добавленные материалы стен скопированы под .stage.projection-iso.mode-view (проверено юнит-тестом test/styles-split.test.mjs, который парсит CSS AST и требует префикс на КАЖДОМ селекторе, кроме @media-обёрток) — Flat-каскад физически не может задеть ни одно правило. Мебель теперь использует ОДИН вызов furniturePlanScreenScale(...) для обеих проекций (ветка iso ? 1 : … убрана целиком, второго места с иной логикой нет — grep подтвердил единственность). Смоки flatWedgesUnchanged, flatBack, offRestoresFlat, editorFlat — прогнаны, все true. Байт-в-байт Flat golden — предрелизный гейт по тексту самого AC1, вне обязанностей код-ревью.
• AC2 (плитка/торец/×1.12/бейдж). Все числа (radius=22/80, depth=8/80, lift=6/80, badgeGap=6/80) и формула торца с тремя особыми случаями (luma<70 → #5b5e5a/#4a4a4a, near-white в тёмной теме → #4a4a4a, светлый пол → brightness(.82) saturate(.8)) сверены построчно с src/iso-tiles.ts и ТЗ; юнит test/iso-stage6.test.mjs и smoke_iso_tiles.mjs (прогнан, true) снимают реальный getComputedStyle/getBoundingClientRect, а не факт существования DOM-узла.
• AC3 (коллизии ×1.12, touch ≥44×44). iso-scene-render.ts:896,947 прокидывают ISO_ICON_SCALE в isoRaisedOverlayHalfSize; юнит строит реальную сцену через buildIsoOverlayRenderScene и алгебраически сверяет половину footprint. Мутант iso-collision-without-icon-scale бьёт именно эту строку.
• AC4 (один слой теней, таблица параметров, не поверх соседа). Таблица 4×(сдвиг, размытие, opacity) в isoTileShadow() пересчитана и совпадает с ТЗ построчно (например, светлая·светлый пол: dx=8/80=0.1D, dy=34/80= 0.425D, sigma=11/80=0.1375D). shadowNeverOnNeighbourTile — не поверхностная проверка: маркер намеренно ставится над соседним по вертикали, сравнивается внутренность нижней плитки побайтово со включённым/выключенным слоем теней. z-index:-1 внутри stacking context .devlayer даёт гарантию сильнее буквы ТЗ (тень всегда под ВСЕМИ маркерами, а не только под соседним).
• AC5 (рамки, приоритет, тело не перекрашено). Приоритет Alert > Focus > Selected > Hover реализован специфичностью и порядком правил; смок реально включает классы/фокус и сравнивает border-top-color и геометрию рамки. (См. L4 про неполный перебор пар.)
• AC6 (forced-colors/no-filter). Оба медиа-блока синхронно гасят тень и торец, не трогая размер/рамки; смок эмулирует forcedColors:'active' и реально это проверяет.
• AC7 (свет). Формула длины, параллелограмм (s = n·L + t·L·(dir·t)/(dir·n)), градиенты, лучики (только светлый пол), подоконник — все числа совпадают с ТЗ и docs/SUN.md дословно; ворота видимости (elevation, fade, редактор, sun_rays) реализованы ОДИН раз и лишь переключают геометрию по проекции, так что Flat-код не тронут. Три названных в таблице приёмки мутанта — реальны и указывают на существующие строки. (См. M1 про пробел в барьерах.)
• AC8 (мебель). stroke-width идентичен в Flat и 2.5D (смок: 1.1818 px = 1.1818 px, прогнан лично — true); юнит furniture-stroke- contract матчит новую строку кода регулярным выражением, которое ловит и старую (мутант «iso ? 1 : возвращён»).
• AC9 (тема не красит стены). Пересчитал вручную: #ffffff × 0.77 → #c4c4c4, × 0.68 → #adadad, × 0.60 → #999999 — совпадает с ТЗ и лабораторией. .theme-dark .iso-*/prefers-color-scheme правила физически удалены (не ослаблены — grep не находит), кроме разрешённого исключения (iso-ambient-shadow, внешняя тень здания). Смок themeKeepsWalls/colourSchemeKeepsWalls (прогнан, true) снимает getComputedStyle в трёх режимах (light/dark/system) и требует побайтового совпадения.
• AC10/AC11 (настройка, alpha-вход убран). Поле сериализуется только при true (delete settings.volumetric_view иначе — houseplan-editor- runtime.ts:9512-9513); единое правило вида для карточки/страницы/киоска через _isoEnabled = volumetricViewOf(this._settings); редакторы и houseplan-space-card структурно без iso-кода (не флаг — код физически отсутствует); ленивая загрузка iso-scene-render условна на _isoEnabled; LABS_FLAGS пуст, кнопка и пункт меню физически вырезаны из шаблона (не CSS-скрытие); houseplan_card_view_v1 не читается. validation.py — строгий bool (тест бьёт None, 0, 1, "true", "iso", [], {}). config-schema.json, support_package.py синхронны. i18n en/ru/de/fr — текст совпадает с ТЗ дословно; view.volumetric/view.flat освобождены и нигде не используются (grep пуст). Смок smoke_volumetric_setting.mjs (прогнан лично, все 18 true) включает нетривиальные детали: превью не переключает вид до сохранения (previewDoesNotSwitch), false не сериализуется (falseNotStored).
• AC12 (макеты). Прочитал docs/design/649-25d-stage6/ACCEPTANCE.md целиком и все 7 PNG кадров. Единственное осознанное расхождение (цвет рамки hover — #0C82F0 продукта против янтарного в лабе для светильников) объяснено ссылкой на ТЗ и подтверждено кадром. Мебель, стены, тени, свет, плитки — визуально соответствуют описанию таблицы «элемент · эталон · продукт»; необъяснённых расхождений не нашёл.
• Golden/матрица. STAGE6_ACCEPTANCE_SCENARIOS намеренно отделены от GOLDEN_SCENARIOS (не имеют эталона — по правилу #641 каждая сцена матрицы обязана иметь принятый baseline, а baseline не может существовать раньше сцены); test/golden-matrix.test.mjs обновлён на модель активации через settings.volumetric_view вместо alpha. Корректное, задокументированное решение, не находка.
• Мутационный реестр. 16 новых мутантов (iso-tile-ring-visible … labs-iso-returns) — патчи адресуют существующие строки (проверено сверкой find-строк с текущим кодом), guard-команды осмысленны и соответствуют названным в таблице приёмки красным сценариям. de452182 чинит один протухший guard (stage3-w12-separate-alpha-url-key-restored) после переименования теста в e59c89df — сам факт находки и починки говорит о добросовестной проверке автора (обнаружено собственным Validate, не мной).
• Бюджет бандла. INITIAL_VIEW_GZIP_CEILING 292 600 → 294 900, обоснование (CSS плиток/материалов не может ждать ленивый граф) правдоподобно и арифметически сходится (294 004 замер + 896 Б запаса = 294 900).

Чего не проверял

• python -m pytest tests_backend — в песочнице ревью нет модуля pytest (pip show pytest → not found); полагаюсь на чтение custom_components/houseplan/validation.py/support_package.py и tests_backend/test_validation.py/test_support_package.py (проверено построчно одним из под-агентов) плюс на то, что Validate на этом SHA включает backend-джобу по стандартному пайплайну push (AGENTS.md, «Gates»). Если backend-джоба Validate не входила в засчитанный success — это стоит явно подтвердить отдельно.
• npm run golden:verify / npm run golden:capture — по тексту AC1/AC12 это предрелизный гейт (iso Stage 6 сцены сознательно не в матрице, baseline ещё не снят на Linux CI); не гейт код-ревью.
• Полный npm test — принят по зелёному Validate на этом SHA, не перегонял.
• Full Performance (AC13) — по тексту AC13 сама предрелизный гейт.
• ~70 «слабых» смоков из smoke-select (см. таблицу гейтов) — решение по каждому: не запускал, риск для не тронутых по существу систем (стены/ диалоги/junction) от этой задачи оцениваю как минимальный.
• Настоящий планшет/телефон, Safari, @supports not (filter) — эмулировать нечем в песочнице; отмечено и автором.
• Наведение мышью в живой лаборатории дизайнера (эталон вложения 11) — недоступно из песочницы; полагаюсь на приложенный кадр pair-hover.png и форму рамки, подтверждённую тестами.

Вывод

High: 0. Medium: 1, в скоупе задачи (M1 — обрезка барьерами 2.5D-луча непроверена автотестом, конкретный сценарий регрессии описан выше). Low: 6, правятся по усмотрению автора или сняты с примечанием (L2 снята — опечатка хендоффа, не влияет на решение). Реализация в остальном очень плотная: все числовые формулы (радиус, торец, тень, стены, свет) пересчитаны вручную и совпадают с ТЗ и лабораторией; 9 браузерных смоков прогнаны лично в песочнице и все зелёные; tsc/build/three-way bundle sync — чисто; docs согласованы с кодом почти везде (кроме отмеченного в L1). Единственная причина жёлтого вердикта — M1: узкий, дёшево устранимый пробел защитного покрытия одного конкретно названного в ТЗ поведения (обрезка барьерами), не общая проблема реализации.

---

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

• Ветка: issue/649-25d-stage6, коммит de4521823ba6 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: 571541905136a48c1729755f6d7877e5ac6aece4

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

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