docs: review document for #376

Issue: #376
User-Visible: no

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

@@ -0,0 +1,253 @@
+# SPEC-REVIEW-376-r1
+
+- Issue: https://github.com/Matysh/houseplan-card/issues/376 — «Пачка Low из
+  аудита beta.4: title:null в space-card, roomlabel в Background, персист
+  цвета декора, iso-штрихи мебели, стейл-док, truthy light_pools (а–е)»
+- Этап: ТЗ на ревью (PROCESS.md §2.4), лёгкий трек (метка `small`)
+- ТЗ: тело issue, раздел «# ТЗ (лёгкий трек, редакция 1)», зафиксировано
+  `updatedAt: 2026-08-29T17:34:47Z`
+- Материал ревью: тело issue #376 (аналитика + ТЗ, один автор, без
+  дальнейших комментариев на момент ревью) сверено с `dev` на `4eede5bc`
+- Заход: r1 · блокирующих циклов израсходовано **1 из 2** (лёгкий трек)
+
+## Скоуп ревью
+
+Первый заход — предыдущего вердикта нет, разбор полный. Проверялось:
+соответствие `docs/SCOPE.md`; корректность выбора трека `small` по §5
+PROCESS.md (все критерии обязаны выполняться одновременно); обязательный
+минимальный шаблон лёгкого трека («проблема · контракт · AC1…ACn с
+доказательством · откат»); однозначность и доказуемость каждого AC;
+отсутствие догадок, выданных за факт; сверка каждого фактического
+утверждения ТЗ о текущем коде с исходниками.
+
+## Как проверялось
+
+1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` целиком (жизненный
+   цикл §2, лимит циклов §4, критерии трека §5, артефакты §7, гейты §8).
+2. Прочитано тело issue #376 полностью (аналитика §2.2 + ТЗ §1–§6), других
+   комментариев на issue нет.
+3. Прочитан `docs/CONFIG-COMPATIBILITY.md` целиком — сверить, что такое
+   «новое compatibility-поле» в терминах именно этого проекта, и подобрать
+   аналог для пункта (в).
+4. Найден и прочитан прямой прецедент: issue **#369** («Аудит после
+   v1.69.0-beta.3 (V5) — семь Low-находок») — структурно идентичный кейс:
+   пачка несвязанных Low-находок из точно такого же adversarial-аудита.
+   `SPEC-REVIEW-369-r1.md` и `-r2.md` зафиксировали: «Этап: ТЗ на ревью,
+   **полный трек** (владелец назвал критерий «одна поверхность» §5 как не
+   выполненный — **семь несвязанных поверхностей**)» и результатом стал файл
+   `docs/specs/369-audit-lows.md`, а не ТЗ в теле issue.
+5. Найден и прочитан второй релевантный прецедент: `SPEC-REVIEW-372-r1.md`
+   (родитель пункта (а) этого issue) — там тот же самый компакт-контракт
+   `title` тоже потребовал **полного** трека («аналитика назвала критерий
+   `small`, который задача не проходит — новый UX-контракт для `title`»).
+6. Каждое фактическое утверждение ТЗ о текущем коде сверено с `dev`, не
+   принято на слово:
+   - `src/space-card.ts:811-813` — `title = this._config.title !== undefined
+     ? this._config.title : sp?.title || ''`; `:821`
+     `compactTopFrame: this._config.title === ''`; `:870`
+     `${title ? html\`<div class="hp-static-title">…` — подтверждён факт «`null`
+     уже скрывает заголовок (falsy), но кадр остаётся симметричным» и что
+     `undefined` (отсутствие ключа) не задета правкой.
+   - `src/space-card.ts:290-291` — `if (!this._config.light_pools)
+     disposeGlowRuntime(...)` (truthy-гейт) против рендер-гейта `:842`
+     `lightPools: this._config.light_pools === true` — подтверждён факт:
+     `light_pools: 1` не dispose'ит рантайм (`!1` ложно) и переходит в ветку
+     `_resolveGlowBlend()` (:291), при этом пулы не рендерятся (`1 !== true`).
+     Расхождение реально существует и воспроизводится по описанному пути.
+   - `src/styles/plan.styles.ts:863-865` — `.stage.mode-decor .devlayer,
+     .stage.mode-decor .devlayer *, .stage.mode-decor .dev::before {
+     pointer-events: none; }`; строки 617-619 несут независимое правило
+     `.stage.markup .roomlabel { pointer-events: auto; }` для **другого**
+     режима (`markup`, не `mode-decor`). `.roomlabel` рендерится внутри
+     `.devlayer` (`src/houseplan-card.ts:11064` открывает `.devlayer`,
+     `:11962` — `class="roomlabel …"` рендерится в этом поддереве) — заявление
+     «`.devlayer *` перебивает предыдущий auto для roomlabel в Background»
+     подтверждено: для `mode-decor` специфичность конкурирующих правил равна
+     (0,3,0), и позже объявленное правило (:863) побеждает по порядку
+     источника. Не код-правка — доку добавлять корректно, код не трогается.
+   - `custom_components/houseplan/validation.py:1883` — `vol.Optional(
+     "settings", default=dict): vol.Schema({...}, extra=vol.ALLOW_EXTRA)` —
+     номер строки и факт «опциональный settings-объект с ALLOW_EXTRA»
+     подтверждены; это снижает риск нового ключа (старый бэкенд/фронт не
+     ломается на неизвестном поле), но само по себе не влияет на
+     классификацию трека — см. находку H1.
+   - `src/houseplan-card.ts:984` — `private _decorStyle: DecorStyle = {
+     ...DEFAULT_DECOR_STYLE }` — подтверждено, поле только in-memory,
+     персиста нет.
+   - `src/furniture.ts:392-406` (`furniturePlanScreenScale`) и
+     `src/houseplan-card.ts:8084` (вызов) — подтверждена формула
+     `Math.min(viewportW/viewW, viewportH/viewH)`; `src/houseplan-card.ts:10791`
+     (`class=${isoLayers?.structural ? 'iso-floor-scene' : nothing}`) и `:10830`
+     (`preserveAspectRatio="none"`) подтверждают наличие отдельной
+     iso-трансформации, под которую эта формула не рассчитана.
+   - `docs/TESTING.md:1704` — строка «static room cards show the same
+     data/base projection but no live pools» подтверждена дословно, без
+     оговорки про `light_pools`/#374.
+7. Проверено отсутствие незакрытых меток неопределённости в тексте
+   («уточнить», TBD, голый вопросительный знак вне markdown-разметки) — не
+   найдено.
+
+## Находки
+
+### H1 (High, блокирует) — трек `small` не проходит собственные критерии §5 одновременно; прямой прецедент #369 уже разрешил тот же вопрос иначе
+
+**Файл:** тело issue #376 (раздел «8. Трек» аналитики + весь раздел «ТЗ»).
+
+Критерии §5 PROCESS.md обязаны выполняться **все одновременно**; ни один не
+допускает исключения по «сумме несложных правок». Этот пакет нарушает как
+минимум два:
+
+1. **«Одна поверхность (один диалог, один модуль, один эндпоинт)».** Сама
+   аналитика перечисляет поверхности: `space-card.ts` (а, е), доки (б, д),
+   `houseplan-card.ts` + `houseplan-editor-runtime.ts` + `validation.py` (в),
+   отдельная iso-ветка `houseplan-card.ts` (г). Это минимум четыре разных
+   модуля продуктового кода плюс документация — не «один модуль» ни по
+   какому прочтению.
+2. **«Нет миграции конфига и новых compatibility-полей».** Пункт (в) вводит
+   новый персистентный ключ `settings.decor_default_style` — новую серверную
+   схему (`validation.py`), новый путь чтения при инициализации и новый путь
+   записи с `expected_rev`. Это ровно тот класс изменения, который
+   `docs/CONFIG-COMPATIBILITY.md` каталогизирует как compatibility-поле
+   (пусть и низкого риска благодаря `ALLOW_EXTRA` — см. «Как проверялось»
+   п.6), а не разрешённое исключение для лёгкого трека.
+
+Это не абстрактное толкование: у проекта есть **прямой прецедент с той же
+формой задачи**. Issue #369 — пачка из семи Low-находок ровно того же
+adversarial-аудита — была явно переведена на полный трек с формулировкой
+владельца «семь несвязанных поверхностей» (`SPEC-REVIEW-369-r1.md`,
+`SPEC-REVIEW-369-r2.md`), и результат лёг в `docs/specs/369-audit-lows.md`,
+а не в тело issue. #376 повторяет ту же форму (пачка Low из аудита,
+затрагивающая несвязанные модули) без единого слова о том, почему в этот
+раз критерий «одна поверхность» вдруг выполнен. Второй, более узкий
+прецедент — issue #372 (прямой родитель пункта (а) этого issue): та же
+самая пара «header/compact frame» для `title` уже один раз потребовала
+полного трека («новый UX-контракт для title»), хотя по объёму правки была
+проще, чем нынешний пакет из шести пунктов.
+
+PROCESS.md §5, последний абзац: «Если по ходу выясняется, что критерий
+нарушен (появилась миграция, задело второй модуль) — метка `small`
+снимается, issue возвращается в `S3-spec` и получает нормальный файл ТЗ.
+Это не провал, это ранняя диагностика.» Это именно тот момент.
+
+**Почему High, а не Medium.** Ошибка классификации меняет саму форму
+процесса вокруг задачи: бюджет ревью (2 цикла вместо 4), обязательность
+файла `docs/specs/376-*.md`, необходимость завести отдельный полный ТЗ хотя
+бы для (в). Это нельзя «починить тем же комментарием» без реструктуризации
+задачи — по определению не Medium-в-скоупе.
+
+**Продуктовые решения владельца не оспариваются.** Оба решения 29.08
+(`title: null ≡ ''`; «цвет декора персистится в серверный конфиг») остаются
+в силе — это ответы на продуктовые вопросы, они не про трек. Находка
+касается исключительно процессной классификации, которая по PROCESS.md
+решается аналитиком/ревьюером техническим порядком, не владельцем.
+
+**Рекомендация (одна из двух, автор выбирает):**
+- Выделить (в) в отдельный issue на полном треке (серверная схема +
+  кросс-модульный путь записи — ровно профиль, для которого полный трек
+  существует), оставить (а, б, г, д, е) на `small` — это уже гораздо ближе к
+  «одному модулю» каждый по отдельности и без нового персистентного поля;
+  либо
+- Снять `small` со всего #376 по образцу #369, написать
+  `docs/specs/376-*.md` с полными разделами §7.1.
+
+### M1 (Medium, в скоупе) — раздел «откат» отсутствует полностью
+
+**Файл:** тело issue #376, весь раздел «# ТЗ».
+
+Минимальный обязательный шаблон лёгкого трека (PROCESS.md §5): «проблема ·
+контракт · AC1…ACn с доказательством · **откат**». В тексте нет ни одного
+предложения о том, как откатить любую из шести правок. Особенно заметно для
+(в): текст не говорит, что происходит с уже записанным
+`settings.decor_default_style` при откате коммита/фронтенда — хотя ответ
+уже виден из кода (`extra=vol.ALLOW_EXTRA`, «Как проверялось» п.6: старый
+бэкенд/фронт читает конфиг с незнакомым ключом без ошибки), в ТЗ это нигде
+не зафиксировано как часть контракта.
+
+**Чем чинится в этой же задаче:** один абзац — «откат: правки (а, б, г, д,
+е) — обычный `git revert`, поведения не переживают перезапуск процесса.
+(в): неизвестный ключ `decor_default_style` безопасно игнорируется старым
+бэкендом/фронтом (`ALLOW_EXTRA`); откат кода не требует отдельной обратной
+миграции, задокументированный дефолт `DEFAULT_DECOR_STYLE` продолжает
+действовать.»
+
+### M2 (Medium, в скоупе) — пункты (б) и (д) заявлены в скоупе, но не имеют собственного AC
+
+**Файл:** тело issue #376, раздел «## 2. AC» против раздела «## 1. Объём».
+
+Раздел «Объём» явно называет (б) (предложение в USER-GUIDE(.ru) про
+Background-редактор) и (д) (оговорка в `docs/TESTING.md:1704`) как часть
+поставки. Раздел «AC» перечисляет AC-а, AC-в1…в3, AC-г, AC-е, AC-общ — ни
+одного пункта, который проверял бы именно текст (б) или (д). Без
+собственного AC код-ревьюеру нечем формально подтвердить, что эти два
+пункта скоупа выполнены так, как задумано (а не забыты или не
+сформулированы иначе).
+
+**Чем чинится в этой же задаче:** два дополнительных пункта, например
+«AC-б: `docs/USER-GUIDE.ru.md`, раздел Редактора подложки, содержит
+предложение о неинтерактивности лейблов комнат в этом режиме — доказательство:
+ревью текста» и «AC-д: `docs/TESTING.md:1704` содержит оговорку про
+`light_pools`/#374 — доказательство: ревью текста».
+
+## Что проверено и корректно
+
+- Задача в скоупе `docs/SCOPE.md` — все шесть пунктов чинят существующее
+  поведение уже принятых фич (#372, #362, #360, #361, #374), новых
+  пользовательских job не открывают и не входят в «Out of scope».
+- Продуктовых вопросов владельцу не осталось: оба места, где решение было
+  нужно ((а) `null`≡`''`; (в) персист или нет), уже закрыты записанным
+  решением владельца от 29.08 — текст ТЗ явно на них ссылается, а не
+  переизобретает.
+- Ни одно техническое утверждение ТЗ о текущем поведении кода не
+  оказалось догадкой, выданной за факт — все семь проверенных пунктов (см.
+  «Как проверялось» п.6) подтвердились дословно по номерам строк.
+- Каждый отдельный AC (AC-а, AC-в1…в3, AC-г, AC-е, AC-общ) сформулирован
+  как проверяемое утверждение с названным способом доказательства
+  (`unit`/`смок`) и явной регресс-веткой там, где правка могла задеть
+  соседнее поведение (AC-а: `undefined` и `'   '` остаются нетронутыми;
+  AC-г: существующий юнит #361 для 2D не должен ослабнуть).
+- Раздел «Принятые предположения» корректно выносит технические, не
+  наблюдаемые пользователем решения (формат ключа snake/camelCase, путь
+  записи только из runtime-редактора) в явный блок вместо того, чтобы
+  спрятать их в тексте контракта как решённые факты.
+- i18n корректно отмечен как незадетый — ни один из шести пунктов не
+  добавляет пользовательский текст в UI (только документацию).
+- Риски (в), (а) названы конкретно вместе со снимающим их механизмом
+  (дебаунс + `expected_rev`/#340; регресс-ветка AC-а), а не общими словами.
+
+## Чего не проверял
+
+- Гейты код-ревью (`typecheck`/`test`/`build`/смоки/`check-docs`/
+  инварианты модели/`pytest tests_backend`) — не запускал: на стадии
+  ТЗ продуктовый код не менялся, диффа против `dev` в этом issue нет,
+  гейтам нечего было бы доказать. Ссылка на зелёный Validate `4eede5bc`
+  (https://github.com/Matysh/houseplan-card/actions/runs/33252620807)
+  относится к текущему `dev`, не к материалу этого ТЗ.
+- Точную будущую формулировку абзацев USER-GUIDE(.ru)/TESTING.md для (б,
+  д) — контракт называет смысл и место, не финальный текст; это в пределах
+  «принято предположительно», не находка (при условии, что M2 закрыт
+  добавлением AC на сам факт правки).
+- Реализуемость дебаунса «≥1 с» для записи (в) на практике — оценка риска
+  по описанию, кода ещё нет.
+- Согласованность нового бэкенд-ключа `decor_default_style` с полным
+  реестром `docs/CONFIG-COMPATIBILITY.md`/`scripts/config-field-registry.mjs`
+  за пределами вопроса классификации трека (H1) — это станет предметом
+  код-ревью, если/когда (в) пойдёт в разработку.
+
+## Вердикт
+
+Технически пакет описан аккуратно: каждое утверждение о текущем коде
+проверено построчно и подтвердилось, AC по большинству пунктов проверяемы
+и снабжены регресс-ветками, продуктовые решения владельца зафиксированы, а
+не додуманы. Но сама форма задачи — пачка из шести точечных находок
+adversarial-аудита поперёк как минимум четырёх модулей, одна из которых
+(в) добавляет новый персистентный серверный ключ с кросс-модульным путём
+записи, — не проходит собственные критерии лёгкого трека PROCESS.md §5
+одновременно («одна поверхность», «нет новых compatibility-полей»), причём
+для задачи ровно такой же формы («пачка Low одного аудита») этот же
+проект уже принимал обратное решение на issue #369. Это блокирующая
+находка: она не про содержание AC, а про то, в каком процессе задачу
+вообще можно продолжать. Два Medium (отсутствующий раздел «откат»,
+отсутствующие AC на пункты (б)/(д)) чинятся в этой же задаче без
+отдельного issue, независимо от исхода H1.
+
+`Вердикт: красный · заход r1 · блокирующих циклов 1/2 · High: 1 · Medium: 2 → в задаче`