docs: review document for #231

Issue: #231
User-Visible: no

docs/reviews/SPEC-REVIEW-231-r1.md

@@ -0,0 +1,192 @@
+# SPEC-REVIEW-231-r1
+
+- **Issue:** #231 — декоративный слой виден поверх заливок комнат
+- **ТЗ:** `docs/specs/231-decor-layer-order.md` @ commit `e023adb` (dev)
+- **Заход:** r1 · блокирующих циклов израсходовано 0 из 4
+- **Вердикт:** жёлтый · High: 0 · Medium: 1 (в скоупе, чинится в этом ТЗ)
+
+## Скоуп ревью
+
+Первый заход, дельты нет — разбор ТЗ целиком: продуктовая рамка (`docs/SCOPE.md`),
+обязательные разделы §7.1 PROCESS.md, однозначность и доказуемость каждого AC,
+отсутствие догадок, выданных за решение, соответствие терминологии
+`docs/USER-GUIDE.ru.md`, согласованность с канонической документацией
+подсистемы (`docs/BACKDROP.md`, `docs/DECOR-EDITOR.md`, `docs/CANVAS.md`,
+`docs/SUN.md`) и с фактическим кодом.
+
+## Как проверялось
+
+1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` §2.4/§7.1/§7.2/§2.9/§2.10.
+2. Прочитано тело issue #231 и все 6 комментариев: два уточнения владельца по
+   целевой позиции слоя, аналитика, вопрос Q1 с default, решение владельца по
+   Q1, финальный хендофф со ссылкой на ТЗ. Открытых продуктовых вопросов на
+   момент ревью нет, `blocked` снят.
+3. Прочитано ТЗ `docs/specs/231-decor-layer-order.md` целиком.
+4. Сверено с фактическим кодом `src/houseplan-card.ts`:
+   - подтверждён текущий порядок вызова `_renderDecorLayer()` (строка 16584) —
+     он действительно предшествует рендеру заливок комнат (цикл с 16596),
+     hover-заливке (16726), обычным тоннелям (16727), Glow-base комнат (16728),
+     Glow-base тоннелей (16729), `_renderGlowLayer` (16730) и `_renderSunRays`
+     (16731); стены (16747), символы проёмов (16763) и `devlayer` (16794) идут
+     ещё позже. Абсолютные номера строк в теле issue (`:16042…:16238`)
+     разошлись с HEAD (там сейчас `:16584…`), но порядок элементов, который
+     единственно имеет значение для ТЗ, подтверждается независимо чтением кода
+     — не является дефектом ТЗ;
+   - целевая позиция ТЗ (между обоими проходами тоннелей и `_renderGlowLayer`)
+     соответствует названным строкам 16729/16730;
+   - проверено, что `.decorlayer .dshape { pointer-events: none; }`
+     (`src/styles.ts:1235`) и включается только в `.stage.mode-decor` —
+     утверждение ТЗ §9/§13 «decor не перехватывает tap/click вне редактора
+     декора» верно независимо от DOM-позиции слоя, не является догадкой;
+   - проверено `.glow-pools-frame/.glow-pools/.glow-spot { isolation: isolate }`
+     (`src/styles.ts:678`) — комментарий у кода прямо говорит, что этот блок
+     изолирует screen-blend от «room data fill, Glow base, paper and
+     backdrop»; ТЗ §8.2 корректно описывает это как риск и явно требует не
+     переносить decor молча, если целевая fixture покажет иной результат —
+     не догадка, а обозначенное предположение (§19.2) с открытым эскейпом;
+   - подтверждено, что `houseplan-space-card` не рисует decor вообще
+     (`grep decor src/space-render.ts` — одно упоминание, к теме не относится);
+   - подтверждено, что изометрический слой (`iso-shadows-svg`/`iso-walls-svg`,
+     16774–16782) — отдельные `<svg>` после закрытия основного плана, поэтому
+     перестановка decor внутри основного SVG не меняет его позицию относительно
+     3D-стен; риск в таблице §16 корректен.
+5. Проверено `docs/BACKDROP.md` — «Layer order» там даёт грубую группировку
+   («sun / walls / openings / rooms / decor» одним пунктом), которая не
+   различает internal order и в этом и есть источник бага; ТЗ верно ставит её
+   актуализацию в release-артефакты (§18).
+6. Проверено `docs/DECOR-EDITOR.md` — контракта видимого порядка в View там нет
+   вообще (только editor-opacity), обновление не требуется, ТЗ не настаивает
+   («при необходимости») — согласовано.
+7. Проверено `docs/CANVAS.md` («who owns the pointer», «Architectural connection
+   overlay») и `docs/SUN.md` (four-phase background) — оба документа говорят о
+   decor на уровне, который порядок внутри плана не меняет; актуализация не
+   нужна, ТЗ их не трогает — корректно.
+8. Проверена регистрация ТЗ в `docs/specs/README.md:71` — присутствует.
+9. Проверены существующие смоки: `demo/smoke_decor.mjs`, `demo/smoke_decor_text.mjs`,
+   `demo/smoke_hide_layers.mjs`, `demo/smoke_backdrop.mjs`, `demo/smoke_glow.mjs`
+   (`hoverLayerOrder`, строки 244–261) — ни один не проверяет порядок decor
+   относительно hover-заливки комнаты (см. находку ниже).
+10. Прогон гейтов для ревью спецификации не требуется (диапазон правок —
+    документация ТЗ и issue, продуктовый код не менялся); `git diff --check`
+    и `node scripts/check-docs.mjs --external`, упомянутые автором в хендоффе,
+    достаточны для этого этапа и не перепроверялись повторно — они не относятся
+    к предмету ревью (корректность и доказуемость контракта), а к гигиене
+    коммита ТЗ.
+
+## Находки
+
+### Medium (в скоупе — чинится в этом же ТЗ)
+
+**M1. Нормативная позиция decor относительно room hover fill не имеет
+привязанного AC/доказательства.**
+
+- **Где:** `docs/specs/231-decor-layer-order.md` §8.1 (нормативный порядок
+  слоёв, пункт «room hover fill» между room fills и opening tunnels) и §14
+  (AC1–AC8).
+- **Суть:** §8.1 явно требует: «любой `[data-hp="decor"]` следует в DOM после
+  room fill, **hover fill**, обоих видов тоннелей и Glow-base» — hover-заливка
+  комнаты прямо включена в обязательный нижний контур. Однако ни один AC этого
+  не проверяет:
+  - AC1 говорит про «room fill» (заливку), не про hover;
+  - AC2 перечисляет ровно три вещи, после которых должен идти decor —
+    «обычный тоннель, Glow-base комнаты и Glow-base тоннель» — hover fill в
+    списке нет;
+  - AC3 описывает верхнюю границу (Glow/солнце/стены/символы/устройства), тоже
+    не про hover.
+  Значит Goal 5 из §5 («Защитить порядок… тестом, который падает при возврате
+  старого расположения слоя») в этой конкретной паре (decor vs hover fill) не
+  реализуется ни одним названным доказательством. Существующий
+  `demo/smoke_glow.mjs` уже умеет сравнивать DOM-позицию (`hoverLayerOrder`,
+  строка 260: `hoverFillLayer.compareDocumentPosition(glowLayer)`), но
+  `decorlayer` в нём не упоминается вовсе — расширить эту же проверку стоит
+  недорого.
+- **Сценарий отказа:** реализация вставляет decor куда-то между room fill и
+  Glow-base (что удовлетворяет AC1/AC2/AC3 буква в букву — «после обычного
+  тоннеля» и т.д. формально не нарушены только если decor вставлен после
+  тоннелей, но ничего не мешает случайно вставить decor **до** hover fill,
+  например прямо перед строкой рендера hover — тоннели и Glow-base всё ещё идут
+  после него по факту кода, так что AC2 останется зелёным, а на реальном плане
+  при наведении на комнату с decor внутри hover-подсветка ляжет поверх
+  декоративной линии/фигуры, а не под неё, что расходится с §8.1 и остаётся
+  недоказанным дефектом.
+- **Почему не High:** это пробел в покрытии доказательства, а не неверное
+  продуктовое решение — сам порядок (decor после hover fill) уже прямо задан
+  диаграммой владельца в теле issue (позиции `:16202` до целевой `:16205→:16206`)
+  и корректно перенесён в §8.1; чинится добавлением одной фразы в AC2 (или
+  отдельного AC) и одной строки в план тестов (§15.1/§15.2), без обращения к
+  владельцу.
+- **Что сделать:** явно включить hover fill в формулировку AC2 (например:
+  «…после hover fill, обычного тоннеля, Glow-base комнаты и Glow-base
+  тоннеля…») и добавить в §15.2/§15.1 конкретное доказательство — расширение
+  `smoke_glow.mjs`/`hoverLayerOrder` или отдельный targeted-assert на
+  `compareDocumentPosition` между `.room-hover-fill-layer` и `.decorlayer`.
+
+## Что проверено и корректно
+
+- Продуктовая рамка: сценарий, персона, «что человек увидит до/после» —
+  присутствуют, соответствуют `docs/SCOPE.md`/аналитике владельца в issue
+  (владелец сам отнёс задачу к J1 в комментарии-аналитике; повторно отдельно
+  в ТЗ можно не обосновывать).
+- Все обязательные разделы §7.1 PROCESS.md на месте: сценарий, что увидит
+  человек, проблема, скоуп/не-скоуп, контракт поведения (§8–§9), UX (§10),
+  данные/миграция/i18n (§11), AC1–AC8 (кроме отмеченного пробела), план
+  автотестов (§15), риски (§16), откат (§17), release-артефакты (§18).
+- Явный блок «Принятые предположения» (§19) присутствует и корректно помечает
+  как assumption именно то, что действительно не было продиктовано владельцем
+  дословно (например, что Glow-base — часть пола, а live Glow/солнце — часть
+  света) — не выдаёт догадку за факт, ревьюер может её оспорить (не оспариваю:
+  подтверждается разделением функций `_renderGlowBaseRooms`/`_renderGlowLayer`
+  в коде).
+- Причина бага (§3) точно подтверждена чтением кода — вызов decor
+  действительно предшествует рендеру комнат.
+- Решение владельца по Q1 (без нового флага «под планом», единый порядок для
+  старого и нового decor) корректно и полностью перенесено в §8.3/§19.1, без
+  расширения скоупа.
+- Открытый технический риск с Glow/CSS isolation (§8.2) не замалчивается: ТЗ
+  прямо требует не переносить decor поверх live Glow «молча» и возвращать
+  вопрос владельцу, если fixture покажет иное — правильная эскалация вместо
+  догадки.
+- «Не входит в задачу» (§7) корректно исключает per-object флаг, изменение
+  геометрии старого декора, любые изменения заливки/Glow/солнца/стен,
+  добавление decor в статическую карточку и в скрытую изометрию отдельным
+  рендерером, новый UI/i18n/миграцию.
+- Touch/UX (§10): корректно ссылается на `docs/TOUCH-SUPPORT.md`, не создаёт
+  новых focus targets/жестов; View/kiosk-контракт (блокирующий по
+  TOUCH-SUPPORT.md) не расширяется и не сужается.
+- Данные/миграция/i18n (§11): корректно — рендер-порядок не персистентное
+  поле, миграция не нужна, что подтверждается отсутствием изменений в
+  `custom_components/houseplan/validation.py`/типах decor.
+- AC5 (documented mutant) явно требует, чтобы smoke падал по существу
+  (AC1/AC2), а не просто по отсутствию DOM-узла — соответствует духу
+  `scripts/mutation-gate.mjs` и извлечённому там же уроку про тесты, которые
+  «ни разу не проверяли на способность падать».
+- Golden-план (§15.3) корректно ограничивает принятие эталона отдельным
+  reviewed Linux-артефактом, Windows — только диагностика; согласуется с
+  AGENTS.md.
+- Регистрация ТЗ в `docs/specs/README.md` на месте.
+
+## Чего не проверял
+
+- Не проверял и не мог проверить визуальный результат Glow-blend на decor в
+  браузере — эта проверка относится к этапу реализации/код-ревью (targeted
+  fixture, §8.2), на этапе спецификации нет кода, который можно запустить.
+- Не прогонял тяжёлые гейты (typecheck/test/build/golden/smoke) — на этом
+  этапе продуктовый код не менялся, они не относятся к предмету ревью ТЗ и
+  будут частью код-ревью.
+- Не проверял `docs/TESTING.md` на предмет уже существующей документации
+  layer-order/mutant contract для decor — в текущей редакции ТЗ такого
+  описания там нет, и ТЗ (§12) верно относит его актуализацию к зонам
+  изменений реализации, а не к самой спецификации.
+- Не оценивал производительность фактически (нет кода) — только сверил, что
+  §13 не описывает новых наблюдателей/таймеров/network calls, что достаточно
+  для стадии ТЗ.
+
+## Резюме
+
+Одна находка Medium в скоупе (M1 — отсутствует доказательство для одной
+конкретной, но явно нормативной, границы порядка: decor относительно room
+hover fill). High-находок нет. Остальной контракт — порядок слоёв, hide/editor
+parity, компат старого decor, touch/UX, данные/i18n, release-артефакты — точен,
+проверяем и обоснован чтением кода, а не заявлением автора. Вердикт: жёлтый,
+возврат автору на правку ТЗ (добавить hover fill в AC2 и тест-план), без
+обращения к владельцу.