docs: review document for #33

Issue: #33
User-Visible: no

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

@@ -0,0 +1,266 @@
+# SPEC-REVIEW #33 — r1
+
+- Issue: https://github.com/Matysh/houseplan-card/issues/33
+- Этап: ревью ТЗ (PROCESS.md §2.4)
+- ТЗ: `docs/specs/033-config-schema-lifecycle.md`, ревизия 2
+- SHA материала: `8335191bff03cacf5e8dfa1dbc6ef3102e2ceb69` (docs-only коммит)
+- Заход: r1 · блокирующих циклов израсходовано 0 из 4
+- Вердикт: **жёлтый**
+
+## Скоуп
+
+Задача инженерная (страховка от schema drift): генерируемый манифест схемы
+(Voluptuous → JSON), parity-тест frontend/backend enum с машиночитаемым
+allow-list, тест полноты `config-field-registry.mjs`, три lifecycle-фикстуры.
+Поведение конфига не меняется. Класс A (schema-потребители), полный трек —
+решение аналитики 2026-08-15 не оспаривается. Продуктовая рамка по
+`docs/SCOPE.md`: задача не закрывает ни один Core user job напрямую, это
+защитная инфраструктура под J6 («Keep the plan true as the home evolves»,
+конкретно — «не испортить старый план молча»); ТЗ сам называет это прямо в
+разделе «Что человек увидит» и не пытается выдать инженерную работу за
+пользовательскую ценность. Замечаний по соответствию SCOPE нет.
+
+## Как проверялось
+
+Это первый заход (r1) на текущей ревизии ТЗ — раздел «Унаследовано из
+r<N-1>» не нужен, разбор полный.
+
+1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` §2.4/§2.10/§7.1,
+   `docs/CONFIG-COMPATIBILITY.md` (канонический документ этой подсистемы).
+2. Прочитано тело issue #33 и все три комментария (актуализация 2026-08-14,
+   актуализация 2026-08-30 с ревизией решения, объявление ревизии 2).
+3. Прочитан весь текст ТЗ ревизии 2 целиком.
+4. Каждое фактическое утверждение ТЗ и опорного комментария сверено с текущим
+   деревом (не с памятью и не с рассказом автора):
+   - `scripts/config-field-registry.mjs` — прочитан целиком, посчитаны записи;
+   - `custom_components/houseplan/validation.py` — найдены оба `vol.Remove`
+     (`aspect`, `segments`), найдены `extra=vol.ALLOW_EXTRA`/`PREVENT_EXTRA`
+     по всему файлу;
+   - `custom_components/houseplan/__init__.py` — проверено, что импорт
+     `custom_components.houseplan.validation` без `homeassistant` падает
+     (воспроизведено локально: `ModuleNotFoundError`), что подтверждает
+     собственную оговорку ТЗ о заглушках;
+   - `src/logic.ts`, `src/types.ts`, `src/plan-optimizer.ts`,
+     `src/houseplan-editor-runtime.ts`, `src/zero-walls.ts`, `src/vacuum.ts` —
+     проверено существование (или отсутствие) именованных экспортируемых
+     enum-констант для каждой из 7 пар, названных в Блоке 2;
+   - `scripts/config-audit.mjs` (весь файл, 103 строки) и
+     `test/config-audit.test.mjs` — проверено текущее поведение exit-code;
+   - `src/houseplan-card.ts:3893-3895`, `src/houseplan-editor-runtime.ts:9600-9618`
+     — проверены оба заявленных «реализованных жизнью» решения (show_all,
+     weather_entity);
+   - `package.json` — сверено реальное имя npm-скрипта.
+5. Ни один из артефактов, которые Блок 1–3 объявляют новыми
+   (`scripts/dump-config-schema.py`, `scripts/config-schema-manifest.json`,
+   `scripts/schema-compat-allowlist.mjs`, `test/config-schema-parity.test.mjs`,
+   `test/fixtures/config-lifecycle/`), в дереве не существует — подтверждено
+   `ls`. Это ожидаемо: этап ТЗ, кода ещё нет.
+6. Дешёвый гейт: `npx tsc --noEmit` на HEAD — чисто (0 ошибок). `npm
+   test`/`npm run build`/`node scripts/check-docs.mjs` не гонял: diff этого
+   раунда — один файл, `docs/specs/033-config-schema-lifecycle.md`, ничего в
+   `src/**` не тронуто, отпечаток скриншотов не мог устареть. Прогон
+   test/build на неизменной кодовой базе не сказал бы ничего об этой ревизии
+   ТЗ (кода по ней ещё нет), поэтому не соразмерен задаче ревью ТЗ.
+7. Смоки/golden/инварианты модели/perf — не прогонял: этап ТЗ, продукт не
+   меняется, ни один из этих гейтов не применим до появления кода.
+
+## Находки
+
+Все три ниже — **Medium, в скоупе задачи** (High нет). Они не про
+«разработчику будет неудобно», а про то, что ТЗ утверждает как решённый факт
+то, что не подтверждается текущим деревом или существующим словарём
+подсистемы — то есть ровно тот вид догадки, который проходит ревью, потому
+что выглядит решением.
+
+### M1 — статус `implemented` не существует ни в перечислении, ни в каноне
+
+Блок 2 (Актуализация registry): «4 реализованных решения переводятся в
+статус `implemented`» (show_all, weather_entity, ripple, aspect/segments).
+
+Проверено чтением: `CONFIG_FIELD_STATUSES` в
+`scripts/config-field-registry.mjs:362-369` перечисляет ровно шесть значений
+— `current`, `decision-required`, `deprecated-read`, `migrate-on-write`,
+`migrate-on-settings-save`, `drop-on-validation` — `implemented` среди них
+нет. Канонический `docs/CONFIG-COMPATIBILITY.md`, раздел «Status meanings»,
+описывает семантику тех же (без `current`) значений и тоже не знает
+`implemented`.
+
+Более того, все четыре названных поля УЖЕ носят точный статус механизма:
+`show_all` → `migrate-on-write` (строка 34), `weather_entity` →
+`migrate-on-settings-save` (76), `display=ripple` → `deprecated-read` (90),
+`aspect`/`segments` → `drop-on-validation` (146, 160) — и эти статусы
+подтверждены чтением кода (`houseplan-card.ts:3895`,
+`houseplan-editor-runtime.ts:9618`, `_dropLegacySegments`,
+`validation.py:1590,1670`). Это не ось «pending → done»: `migrate-on-write`
+описывает *постоянно действующий* механизм, а не незавершённую задачу —
+поле остаётся `migrate-on-write`, пока открыто окно read-совместимости,
+независимо от того, когда код появился. Приравнивание «решение реализовано
+жизнью» к новому статусу `implemented` не стыкуется со словарём, который сам
+же документ (`CONFIG-COMPATIBILITY.md`) объявляет каноническим.
+
+**Воспроизведение:** `grep -n "CONFIG_FIELD_STATUSES" -A8
+scripts/config-field-registry.mjs` — шесть значений, `implemented` нет;
+`grep -n "^| \`" docs/CONFIG-COMPATIBILITY.md` — то же самое в таблице.
+
+**Чем чинится:** ТЗ должно явно сказать одно из двух — либо это НЕ смена
+`status` (тогда описать, что именно меняется — `migration`/`compatibility`
+как текст, статус остаётся прежним, точным), либо это добавление седьмого
+значения в перечисление и таблицу CONFIG-COMPATIBILITY.md, с объяснением,
+как оно взаимодействует с AC4 (тест полноты) и AC6 (exit-коды audit).
+Сейчас ни то, ни другое не написано — решение выдано как готовое, но оно не
+существует в модели данных, которой должно принадлежать.
+
+### M2 — 4 из 7 «const-деклараций фронта» для parity-теста не существуют
+
+Блок 2: «parity-тест ... сверяет enum-пары с const-декларациями фронта:
+fill_mode ↔ `SPACE_FILL_MODES`/`ROOM_FILL_MODES`, display ↔ `DISPLAY_MODES`,
+opening.type, tap_action ↔ `TAP_ACTIONS`, vacuum.trail_mode,
+zero_wall_style, bg_mode» — семь пар представлены как один и тот же,
+уже готовый механизм.
+
+Проверено чтением: именованные экспортируемые массивы существуют только для
+3 из 7 — `SPACE_FILL_MODES`/`ROOM_FILL_MODES`, `DISPLAY_MODES`, `TAP_ACTIONS`
+(все в `src/logic.ts:898-919`, экспорт подтверждён). Для остальных четырёх
+такой декларации во фронтенде нет:
+- `opening.type` — TS union-тип `'door' | 'window' | 'gate' | 'passage'`
+  (`src/types.ts:212`), не runtime-значение; enum нельзя прочитать без
+  отдельного массива;
+- `vacuum.trail_mode` — только inline-литерал
+  `['never', 'cleaning', 'always']` внутри `if` в
+  `src/plan-optimizer.ts:285`, не экспортируется;
+- `zero_wall_style` — только строковые литералы `'dashed'`/`'solid'`,
+  разбросанные по `houseplan-editor-runtime.ts`, `houseplan-onboarding-runtime.ts`,
+  `zero-walls.ts:53`, без единого списка;
+- `bg_mode` — на бэкенде есть `_BG_MODE` (`validation.py:1280`), на фронте
+  сопоставимой именованной константы не нашлось.
+
+**Воспроизведение:** `grep -rn "OPENING_TYPES\|trail_mode.*=.*\[\|ZERO_WALL_STYLES\|BG_MODES" src/*.ts`
+— пусто для всех четырёх.
+
+**Чем чинится:** не архитектурная проблема (ввести 4 новых экспортируемых
+константы — тривиально и не меняет поведение), но ТЗ сейчас читается так,
+будто вся инфраструктура для сравнения уже на месте одинаково для всех
+семи пар — это неверно для четырёх. Раз ТЗ прямо перечисляет эти пары как
+цель Блока 2, минимальная правка — явно назвать, что для этих четырёх полей
+работа включает добавление разделяемой константы (или иной механизм
+чтения TS union), а не только «сверить».
+
+### M3 — AC6 описывает несуществующее поведение как «расширение теста»
+
+Блок 3: «Юнит фронта: `config-audit.mjs` на этих фикстурах даёт ожидаемые
+counts (clean / migration available), exit-codes различимы — расширение
+существующего `test/config-audit.test.mjs`.» AC6: «`config-audit.mjs`
+различает exit-codes на фикстурах (юнит).»
+
+Проверено чтением всего `scripts/config-audit.mjs` (103 строки) и
+`test/config-audit.test.mjs`: сегодня `process.exitCode` устанавливается
+в `2` только при ошибке разбора/использования (`--json` без файла, битый
+JSON, неизвестный флаг); при успешном разборе `exitCode` остаётся
+дефолтным (0) **независимо от количества и статуса находок**. Различения
+«clean» и «migration available» через exit-code в инструменте нет вовсе —
+это не существующее поведение, которое расширяется тестом, а новая
+CLI-логика, которую только предстоит спроектировать и написать.
+
+**Воспроизведение:** чтение `scripts/config-audit.mjs:56-103` — единственные
+присвоения `process.exitCode` на строках 74, 79, 102, все на ветках ошибок.
+
+**Чем чинится:** ТЗ должно явно зафиксировать контракт — какие коды что
+означают (например: 0 — чисто, 1 — есть находки со статусом
+`deprecated-read`/`migrate-on-write`/…, 2 — ошибка, как сейчас), иначе
+реализация придумывает эту схему по ходу без утверждённого текста ТЗ, а
+AC6 в текущей формулировке нельзя разобрать «доказано / не доказано»: тест
+против чего сверяться, если контракт не описан.
+
+### Дополнительно (Low, не блокирует, зафиксировано для правки заодно)
+
+- **L1** — раздел «Что человек увидит до и после» называет команду
+  `npm run config:audit`; реальный npm-скрипт — `audit:config`
+  (`package.json:19`, тот же, что описан в `docs/CONFIG-COMPATIBILITY.md`).
+  Опечатка/инверсия слов, но это единственное окно к видимому эффекту
+  задачи и должно цитировать точную команду.
+- **L2** — шапка ТЗ и оба комментария-актуализации утверждают «registry = 24
+  записи»; фактический размер `CONFIG_FIELD_REGISTRY` на HEAD — 27 элементов
+  (24 литеральных объявления + 3, порождённые `.map(['entity','attr','unit'])`
+  на строках 346-359). Не влияет ни на один AC (тест полноты не хардкодит
+  число), чисто фактическая неточность в вводных цифрах ревизии.
+
+## Что проверено и корректно
+
+- Оба «обязательных первых» продуктовых раздела (§7.1) на месте и честны:
+  сценарий и «что человек увидит» прямо говорят «ничего не меняется
+  визуально», без попытки выдать инфраструктуру за пользовательскую
+  ценность.
+- Все обязательные разделы §7.1 присутствуют: сценарий, что видит
+  пользователь, проблема, скоуп/не-скоуп, контракт поведения, UX/i18n,
+  модель данных и миграция, критерии приёмки с доказательствами, план
+  автотестов, риски, откат, release-артефакты, принятые предположения.
+- Ключевое архитектурное решение ревизии 2 (манифест генерируется из
+  Voluptuous, а не пишется руками на 212 путей) — правильный ответ на
+  находку из предыдущей актуализации (человекописанный registry систематически
+  отстаёт от кода) и не пытается зафиксировать «212» как проверяемое число:
+  AC1 проверяет 100%-покрытие и байт-идентичность, а не конкретную цифру —
+  это ограждает тест от дрейфа числа путей в будущем.
+- Утверждения о «уже реализованных жизнью» решениях подтверждены чтением
+  кода: `settings.show_all` действительно удаляется при материализации
+  (`houseplan-card.ts:3895`), `settings.weather_entity` — при сохранении
+  настроек (`houseplan-editor-runtime.ts:9618`), `spaces[].aspect` и
+  `spaces[].segments` действительно `vol.Remove` в схеме
+  (`validation.py:1590,1670`). Это не догадки автора — это точное описание
+  текущего состояния.
+- Собственная оговорка ТЗ о необходимости «заглушек родительских пакетов»
+  для импорта `validation.py` без `homeassistant` — подтверждена
+  независимо: `custom_components/houseplan/__init__.py` действительно
+  импортирует `homeassistant.components.frontend` на верхнем уровне, и
+  прямой импорт `custom_components.houseplan.validation` без HA
+  воспроизводимо падает `ModuleNotFoundError`. Автор не спрятал эту
+  сложность — назвал её и предложил решение.
+- Не-скоуп сформулирован конкретно и без утечки: судьба
+  `group_lights`/`exclude_integrations` явно оставлена #44, здесь они
+  получают только паспорт `allow-extra` — согласуется с текстом issue.
+- Имена реальных identifiers, которые ТЗ действительно проверило (не
+  выдумало): `SPACE_FILL_MODES`, `ROOM_FILL_MODES`, `DISPLAY_MODES`,
+  `TAP_ACTIONS` — все существуют и экспортируются из `src/logic.ts` именно
+  под этими именами.
+- Откат — тривиален и честен (только новые файлы и данные-паспорта, рантайм
+  не тронут).
+
+## Чего не проверял
+
+- Не гонял `npm test`, `npm run build`, `node scripts/check-docs.mjs` — diff
+  этого раунда не касается `src/**` (только spec-файл), эти гейты не
+  сказали бы ничего нового о самой ревизии ТЗ; см. «Как проверялось», п.6.
+- Не гонял смоки, golden, инварианты модели, performance-профили — этап ТЗ,
+  ни один продукт-путь не изменён, эти гейты неприменимы до реализации.
+- Не проверял корректность оценки сложности (6/10) и риска (6/10) из
+  комментария 2026-08-30 по существу — это управленческая оценка, не
+  предмет ревью ТЗ.
+- Не искал независимо каждый из decor/openings/marker путей на предмет ещё
+  не найденных `PREVENT_EXTRA`-узлов сверх `openings[].host` (Партиция и
+  Wall варианты, `validation.py:1500-1516`) — этого узла достаточно, чтобы
+  показать, что формулировка Блока 3 «future-поля переживают валидацию
+  losslessly на всех уровнях» не универсальна; решил не расширять список
+  находок мимо M3 без нового наблюдения, но отмечаю здесь на случай, если
+  автор при доработке ТЗ захочет пройтись по всем `extra=` в
+  `validation.py` целиком, а не точечно.
+
+## Гейты — сводка
+
+| Гейт | Прогнан | Результат |
+|---|---|---|
+| `npx tsc --noEmit` | да | чисто, 0 ошибок |
+| `npm test` | нет | diff docs-only, не относится к предмету ревью ТЗ |
+| `npm run build` | нет | то же |
+| `node scripts/check-docs.mjs` | нет | diff не касается `src/**` |
+| смоки / golden / инварианты / perf | нет | этап ТЗ, продукт не меняется |
+
+## Итог
+
+0 High, 3 Medium (все в скоупе, все — про утверждения ТЗ, не подтверждённые
+текущим деревом или каноническим словарём), 2 Low (правятся заодно).
+Вердикт жёлтый: архитектурное решение ревизии 2 верное и хорошо
+обосновано, но три места выдают непринятое или несуществующее решение за
+факт (M1: статус `implemented`, M2: отсутствующие frontend-константы для
+4 из 7 enum-пар, M3: несуществующее exit-code-поведение `config-audit.mjs`,
+описанное как «расширение теста»). Все три чинятся правкой текста ТЗ в
+пределах той же ревизии, без изменения архитектуры и без обращения к
+владельцу — вопросы технические, не продуктовые.