houseplan-card/docs/reviews/SPEC-REVIEW-654-r1.md

SPEC-REVIEW-654-r1

Issue: #654 — «2.5D: цвет бумаги читается getComputedStyle в render() — на первом кадре все полы «тёмные»; вероятная вспышка Flat→2.5D при холодной загрузке дашборда» Этап: spec (ревью ТЗ, PROCESS.md §2.4) Заход: r1 · блокирующих циклов израсходовано (после этого вердикта): 1 из 4

Вердикт

Жёлтый. High: 0 · Medium: 1 (в скоупе, возвращается автору) · Low: 0.

Единственная находка — «Release-артефакты» ТЗ не называют обновление docs/ISOMETRIC.md («Activation»), хотя задача вводит новый, ранее нигде не документированный элемент этого контракта: план скрыт загрузочной поверхностью, пока ленивый 2.5D-рантайм не установлен (в т.ч. в kiosk, где общий boot veil сегодня отключается мгновенно). Остальная часть ТЗ — обязательные разделы §7.1, однозначность и доказуемость AC1–AC6, явный блок технических предположений — выполнена корректно, а фактические технические предпосылки (getComputedStyle на .hp-paper, _effectiveProjection(), _isoSceneRuntimeLoader, поведение kiosk в setConfig()) проверены по коду dev и подтвердились буквально.

Скоуп проверки

1. Какую строку docs/SCOPE.md закрывает задача — J1 («живой обзор дома») в рамках уже принятого узкого исключения #89 (детерминированная 2.5D-презентация той же геометрии, без второй модели/свободной камеры); задача не расширяет это исключение, а чинит дефект внутри него.
2. Обязательные разделы ТЗ по PROCESS.md §7.1 (сценарий и «что человек увидит», проблема, скоуп/не-скоуп, поведенческий контракт, UX, данные/миграция, i18n, AC1…ACn с доказательством, план автотестов, риски, откат, release-артефакты).
3. Однозначность и проверяемость каждого AC (AC1–AC6) и то, что заявленный способ доказательства (unit/AST/smoke) реален, а не ссылается на несуществующий код.
4. Соответствие фактических технических утверждений ТЗ действительному состоянию src/houseplan-card.ts, src/iso-materials.ts на dev (это не код-ревью диффа — диффа ещё нет; это проверка, что ТЗ не выдаёт догадку за факт о существующем поведении).
5. Терминология видимого поведения — против docs/USER-GUIDE.ru.md и канонического docs/ISOMETRIC.md (обязательное чтение для видимого поведения по инструкции ревью).
6. Наличие открытых продуктовых вопросов, которые ТЗ решило само вместо вынесения владельцу, и их обоснованность.

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

• Тело issue #654 и комментарии получены через gh issue view 654 --json body,comments,labels,state (MCP mcp__github__get_issue/get_issue_comments были недоступны без разрешения пользователя в этой сессии — gh CLI как эквивалентный путь к тому же публичному API).
• Прочитаны целиком docs/SCOPE.md, docs/process/REVIEWER.md, AGENTS.md (обязательный порядок чтения ревьюера), раздел PROCESS.md §7.1 (обязательные разделы ТЗ, шаблон вердикта).
• Прочитан целиком docs/ISOMETRIC.md (в первую очередь «Activation» и Stage 6 «Raised tiles» — источник формулы isoLightFloorRooms/luma > 0.55).
• Код dev (git rev-parse HEAD = 803c0f0eaab3aea1ea9a87215c35b638a679dafb) проверен построчно на все фактические утверждения ТЗ:
  • src/houseplan-card.ts:10701 — буквально совпадает с цитатой в issue (getComputedStyle(this.renderRoot.querySelector('.hp-paper') ?? this).fill внутри _renderBody(), вызываемой из render()).
  • src/houseplan-card.ts:6002-6008 (_effectiveProjection()) — подтверждён ранний return finish('flat'), пока !this._isoSceneRuntime.
  • src/houseplan-card.ts:2568 (connectedCallback) — подтверждена предзагрузка if (this._isoEnabled) void this._ensureIsoSceneRuntime();.
  • src/houseplan-card.ts:3095 (setConfig()) — подтверждено, что kiosk немедленно гасит _booting/_bootFading («kiosk: 100dvh, nothing to settle»), т.е. общий boot veil в kiosk сегодня действительно не появляется — ровно то, что называет issue.
  • src/houseplan-card.ts:332-337 (BOOT_MIN_MS/BOOT_QUIET_MS/BOOT_MAX_MS, _bootWatch() :6356-6373) — существующий boot-veil таймер имеет безусловный потолок BOOT_MAX_MS = 1200 мс. Это прямое технической напряжение с «переиспользовать существующий bootveil» (если ленивый чанк ставится дольше 1.2 с, безусловное снятие veil обнажит Flat-кадр вопреки AC3) — ТЗ уже закрывает этот риск не как факт, а как открытый технический выбор в разделе «Принято предположительно»: «использование существующего bootveil либо визуально идентичного внутреннего состояния iso-pending». Разбор ниже, в «Что проверено и корректно».
  • src/iso-materials.ts:91-99 (isoLightFloorRooms) — подтверждено, что функция уже является чистой и принимает явный Rgb, DOM не трогает; баг живёт только в вызывающем коде (houseplan-card.ts:10701), что напрямую подтверждает реалистичность AC1 (unit-тест на чистую функцию уже возможен без новой архитектуры).
  • test/iso-stage6.test.mjs:7,43 — isoLightFloorRooms уже юнит-тестируется без DOM, ссылка AC1 не на пустое место.
  • demo/smoke_volumetric_setting.mjs (90 строк, без page.reload()) — подтверждена цитата issue «проверяет только переключение без перезагрузки».
  • demo/smoke_isometric_contract.mjs, demo/smoke_iso_tiles.mjs, demo/smoke_iso_theme_walls.mjs — существуют (план автотестов ссылается на реальные файлы, не выдуманные).
• docs/reviews/INDEX.md проверен на прецедент: обе смежные задачи (#651, #649) в списке затронутых файлов называли docs/ISOMETRIC.md, а ревью #651 прямо содержало Medium-находку за расхождение терминологии ТЗ с этим же каноническим документом — это основа находки ниже, а не единичное мнение.
• Код продукта не менялся, гейты (tsc/npm test/npm run build) не запускались: на этапе spec предмет ревью — текст ТЗ, диффа ещё нет (ветка реализации не создана).

Находки

Medium (в скоупе) — «Release-артефакты» не называют обновление docs/ISOMETRIC.md

Файл: тело issue #654, раздел ## ТЗ, подраздел «Release-артефакты» (и смежно — «Контракт поведения и UX», п. 1–2).

Воспроизведение:

• «Release-артефакты» ТЗ перечисляют только: docs/CHANGELOG.md + docs/CHANGELOG.ru.md (да), «пользовательское руководство/i18n/config migration: нет», golden/perf/security — без единого упоминания docs/ISOMETRIC.md.
• При этом «Контракт поведения и UX» вводит новый пункт жизненного цикла, которого сегодня в docs/ISOMETRIC.md («Activation») нет ни в каком виде: «Пока ленивый 2.5D-рантайм загружается, план не показывается в промежуточном Flat-виде… Это относится и к kiosk, где общий boot veil обычно отключён» — то есть kiosk получает новое видимое поведение (кратковременная загрузочная поверхность), которого у него сегодня нет вовсе (setConfig() гасит _booting/_bootFading безусловно, см. «Как проверялось»).
• Текущий текст docs/ISOMETRIC.md:10-28 («Activation») описывает только: когда включена ленивая загрузка чанка, что переключение без перезагрузки мгновенно, и фингерпринт-фолбэк — ни слова о том, что скрывается сам план на время установки чанка при холодном старте, и что это распространяется на kiosk, где обычный boot veil не работает.

Почему это находка, а не придирка к формулировке. docs/ISOMETRIC.md — канонический документ подсистемы (AGENTS.md, «Reading this first»; PROCESS.md §2.10 требует смотреть его строки перед разбором подсистемы), и обе соседние задачи по той же подсистеме (#649, #651 — см. docs/reviews/INDEX.md) называли именно этот файл среди затронутых при заметно менее существенных изменениях поведения (терминология, geometry). Здесь же вводится реальный новый инвариант жизненного цикла — «первый кадр 2.5D скрыт до готовности рантайма, включая kiosk» — который следующий разработчик, открывший только docs/ISOMETRIC.md, не увидит и рискует нарушить неосознанно (например, при следующей правке _bootWatch()/BOOT_MAX_MS или логики setConfig() для kiosk). Отсутствие этого пункта в «Release-артефактах» — это ровно тот паттерн, что уже стоил Medium-находки в #651.

Чем закрывается. Добавить в «Release-артефакты» строку об обновлении docs/ISOMETRIC.md (раздел «Activation» — или новый короткий подраздел о first-frame gating), фиксирующую: план скрыт до готовности ленивого рантайма при volumetric_view: true, это распространяется на kiosk, terminal failure безопасно возвращает Flat. Формулировка — на усмотрение автора, это техническая правка текста ТЗ, а не новый продуктовый вопрос владельцу.

Класс: Medium, в скоупе задачи — правится прямо в теле ТЗ, отдельный issue не заводится (#202).

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

• Обязательные разделы §7.1 — присутствуют все: «Сценарий и видимое изменение» (отвечает на оба продуктовых вопроса — какая персона/поверхность/ момент и что человек видит одной фразой, без терминов реализации), «Проблема и причины», «Скоуп»/«Не скоуп», «Контракт поведения и UX», «Модель данных и совместимость», «i18n», «Критерии приёмки» (AC1–AC6 — см. ниже), «План автотестов и доказательств», «Производительность и touch» (не обязателен, но уместен для render-path фикса), «Риски», «Принято предположительно, поменять свободно», «Откат», «Release-артефакты».
• Однозначность и доказуемость AC1–AC6. Каждый AC называет конкретный вид доказательства (unit/AST/smoke) и наблюдаемый критерий, а не самоцель реализации:
  • AC1 — чистая функция классификации без DOM; белая бумага + светлая заливка → light-floor, тёмная → нет; cold-reload smoke сверяет первый кадр с установившимся. Подтверждено технически: isoLightFloorRooms уже чистая (см. «Как проверялось»), баг — только в вызове.
  • AC2 — отсутствие getComputedStyle/getBoundingClientRect в render/_renderBody/willUpdate и в iso render-helper поверхностях, AST-проверка + браузерный счётчик обращений на повторных HA-рендерах без смены темы/режима. Способ доказательства (AST, не regex по тексту монолита) соответствует требованию ревьюера не полагаться на текстовые якоря.
  • AC3 — атомарность холодного старта при искусственно задержанном чанке, для обычной карточки и kiosk; критерий (.stage.projection-iso, устойчивый viewBox) — наблюдаемый и не расплывчатый.
  • AC4 — безопасный отказ: конечная загрузочная поверхность, доступный Flat, настройка не меняется, приватные данные не в диагностике — опирается на уже существующий контур EditorRuntimeLoader/safeRuntimeDiagnostic, не изобретает новый механизм отказа.
  • AC5 — точная инвалидация memo по бумаге/fills/rooms; переход в редактор и обратно не переносит цвет одного режима в классификацию другого.
  • AC6 — соседние контракты (volumetric_view как есть, Flat не запрашивает чанк, structural fallback, zoom/устройства) явно перечислены как регрессионный периметр.
• Технические предпосылки ТЗ проверены по коду, а не приняты на слово. Все процитированные строки и функции (_renderBody/render, _effectiveProjection, connectedCallback-предзагрузка, setConfig()-поведение kiosk, EditorRuntimeLoader) существуют буквально там, где заявлено, и ведут себя так, как описано — включая менее очевидное наблюдение issue про kiosk (setConfig() действительно гасит _booting раньше, чем что-либо успевает отрисоваться). Догадок, выданных за факт, в части воспроизведения дефекта не найдено.
• Раздел «Принято предположительно» реалистично закрывает найденное техническое напряжение. BOOT_MAX_MS = 1200 мс — безусловный потолок существующего boot veil — потенциально конфликтует с «дождаться ленивого рантайма» при медленной сети. ТЗ не скрывает эту развилку: явно оставляет автору выбор между переиспользованием bootveil и отдельным iso-pending-состоянием, а ревьюер вправе его оспорить (§7.1). Технической ошибки/недосказанности здесь нет — граница ответственности проведена верно.
• Отсутствие открытых продуктовых вопросов проверено, а не принято на слово. Кроме разобранной выше Medium-находки, ни одного места, где догадка о видимом поведении подана как факт без пометки предположения, не найдено. Формулировка «Продуктовых вопросов для владельца нет» в аналитике — обоснована: единственная пограничная зона (новое поведение kiosk) уже прямо зафиксирована как решение в контракте, а не спрятана.
• Соответствие docs/SCOPE.md. Задача остаётся строго внутри узкого исключения #89 (никакой новой камеры/модели/настройки), корректно исключает #651 (дрейф значков при зуме) и _pointInRoom из скоупа со ссылкой на конкретные issue.
• i18n — новых строк нет, что верно: явно запрещён новый текст/кнопка/ индикатор (Контракт поведения, п. 1).
• Модель данных/совместимость — эфемерный runtime-кэш экземпляра карточки, без схемы/YAML/localStorage/миграции; согласуется с характером фикса (render-path + lifecycle-гейт, не персистентное состояние).
• Откат — простой revert продуктового коммита, без обратной миграции; корректно для чисто presentation-слойного изменения.
• Артефакты плана автотестов существуют. demo/smoke_isometric_contract.mjs, demo/smoke_iso_tiles.mjs, demo/smoke_iso_theme_walls.mjs, demo/smoke_volumetric_setting.mjs — реальные файлы, не выдуманные ссылки; последний подтверждённо не покрывает page.reload(), что и обосновывает необходимость нового/расширенного smoke по AC3.

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

• tsc/npm test/npm run build и прочие гейты §8 не запускал — на этапе spec предмет ревью текст ТЗ, реализации ещё нет (ветка задачи не создана на момент ревью).
• Не проверял браузерное поведение вручную — реализации нет, нечего запускать; это работа код-ревью следующего этапа.
• Не читал построчно весь src/iso-scene-render.ts/src/iso-sun.ts/ src/iso-tiles.ts — только точечно то, что нужно для проверки фактических утверждений ТЗ (существование isoLightFloorRooms, _isoSceneRuntime, boot-veil таймеров). Полный аудит «какие именно файлы входят в 2.5D render-helper поверхности» для AC2 — задача код-ревью, когда появится AST-тест и станет видно его фактический периметр.
• Не оценивал real-world длительность загрузки чанка iso-scene-render (совпадает ли она обычно с BOOT_MAX_MS) — это эмпирический вопрос реализации/код-ревью, а не спецификации; отметил риск как технически реальный, но признал его закрытым явным «принято предположительно», а не находкой.
• Полные наборы golden/perf/smoke не прогонял — кода для прогона ещё нет; это часть «Плана автотестов» и войдёт в работу при реализации (§2.6) и последующем код-ревью (§2.7).

Рекомендация автору

Одна точечная правка текста ТЗ — дополнить «Release-артефакты» пунктом об обновлении docs/ISOMETRIC.md. Правка не требует нового цикла анализа по существу поведенческого контракта: сам контракт (AC1–AC6, риски, допущения) корректен и не меняется.

---

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

• Ветка: dev, коммит 803c0f0eaab3 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: b4eeb2b0840189d8031f291834e57fab092af49b

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

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