docs: review document for #486

Issue: #486
User-Visible: no

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

@@ -0,0 +1,228 @@
+# SPEC-REVIEW-486-r1
+
+- **Issue:** #486 — «Панель House Plan в боковом меню Home Assistant: вход как у
+  приложения и редактор на всю страницу»
+- **Этап:** ТЗ на ревью (PROCESS.md §2.4)
+- **Заход:** r1 · блокирующих циклов израсходовано 0 из 4 (первый заход, цикл
+  ещё не открыт)
+- **Материал:** `docs/specs/486-house-plan-panel.md` на ветке
+  `issue/486-house-plan-panel`, коммит `2821c359` (совпадает с вершиной ветки
+  на момент ревью и с SHA зелёного Validate,
+  https://github.com/Matysh/houseplan-card/actions/runs/34159261423)
+- **Трек:** полный (`S4-spec-review`, без меток `small`/`trivial`)
+- **Ревьюер:** Claude (отдельная от автора модель, роль по PROCESS.md §6)
+
+## Вердикт
+
+**Зелёный.** High: 0, Medium: 0. Три Low-замечания — сняты решением ревьюера с
+записью ниже (правки не требуются для перехода в «Готово к разработке»).
+
+## Скоуп ревью
+
+Проверялось только ТЗ (`docs/specs/486-house-plan-panel.md`, 709 строк) и его
+согласованность с:
+
+1. телом issue #486 и обоими комментариями (`S2 — аналитика и оценка`,
+   `S3 — ТЗ готово`);
+2. `docs/SCOPE.md` (границы продукта, персоны, lock invariant);
+3. `PROCESS.md` §7.1 (обязательные разделы ТЗ) и §2.5 (чек-лист DoR);
+4. канон подсистем, которые задача трогает: `docs/UX-MODES.md` (View как
+   default, last-space, route-departure, admin_only), `docs/TOUCH-SUPPORT.md`
+   (hit target, desktop-first редакторы), `docs/CONFIG-COMPATIBILITY.md`
+   (миграции — задача заявляет «нет», проверено, что в файле нет
+   противоречащих обязательств);
+5. фактическим состоянием кода на исходной вершине `origin/dev` — ТЗ раздел
+   «3. Подтверждённое текущее состояние» описывает семь конкретных утверждений
+   о текущем коде, и именно они несут наибольший риск «догадка, выданная за
+   факт» (см. ниже).
+
+Код реализации не существует (`S4-spec-review`, до `S5-ready`), поэтому
+код-ревью, автотесты, mutation-таблицы и гейты §8 к этому этапу не относятся и
+не прогонялись — они станут предметом код-ревью после `S6-in-progress`.
+
+## Как проверялось
+
+### 1. Сверка утверждений о текущем коде с фактическим `origin/dev`
+
+Раздел 3 ТЗ и обосновывающие его решения раздела 4 опираются на семь фактических
+утверждений о поведении кода, которого ТЗ не меняет. Каждое проверено чтением
+`origin/dev` (агентом Explore и напрямую):
+
+| # | Утверждение ТЗ | Результат проверки |
+|---|---|---|
+| 1 | `__init__.py` не регистрирует HA panel | Подтверждено: `custom_components/houseplan/__init__.py` — ни `panel_custom`, ни `async_register_panel`, ни `async_register_built_in_panel` не встречаются нигде в `custom_components/houseplan/**/*.py` |
+| 2 | `frontend_registration.py` — Lovelace resource, fallback, versioned URL, одноразовое уведомление, System Health | Подтверждено построчно: `async_register_lovelace_resource()`, `_add_fallback()`, `module_url = f"{FRONTEND_URL}?v={VERSION}"`, `_async_create_reload_notice()` с one-shot флагом в `entry.data`, отдельный `system_health.py` |
+| 3 | Empty-state ветка не проверяет `_canEdit`: read-only пользователь тоже может получить авто-открытие onboarding | Подтверждено: `src/houseplan-card.ts:4162-4178` — условие авто-открытия `_openSpaceDialog('create')`/import-диалога проверяет `_serverStorage`, `_loadOk`, `_model.length === 0`, `!_onboardingShown`, но не `_canEdit` (геттер `_canEdit` определён на `houseplan-card.ts:1086` и используется в 10+ других местах — то есть пропуск в этой ветке точечный, не системный) |
+| 4 | Есть `getCardSize()`, нет `getGridOptions()` | Подтверждено: `getCardSize()` на `houseplan-card.ts:3833`; `getGridOptions` — 0 совпадений в `src/**` (проверено Grep по всему дереву) |
+| 5 | `rollup.config.mjs` — один entry; `bundle-manifest.mjs` берёт первый `isEntry` | Подтверждено дословно: `rollup.config.mjs:18` `input: 'src/houseplan-card.ts'` (одна строка); `scripts/bundle-manifest.mjs:63` `files.find((file) => file.isEntry)?.path` |
+| 6 | HA 2024.6.0 — минимальная поддерживаемая версия | Подтверждено: `docs/USER-GUIDE.md:77`/`docs/USER-GUIDE.ru.md:78` `Home Assistant 2024.6.0 или новее` |
+| 7 | Существующий route-departure/last-space контракт (сброс editor при уходе с route, память только о пространстве) | Подтверждено в `docs/UX-MODES.md:20-24` и в коде: `LS_NAV` (`houseplan-card.ts:500`), запись/чтение (`:7447,7458`), `_leaveCardRoute()` со сбросом `_mode` в `'view'` (`:7470-7477`) |
+
+Ни одного случая «догадка, выданная за факт» не найдено: каждое фактическое
+утверждение о текущем состоянии либо подтверждено кодом, либо (для API HA
+2024.6, недоступного в этом репозитории — `panel_custom.async_register_panel`,
+приватный `hass.data[frontend.DATA_PANELS]`) явно помечено как риск с named
+защитой — compatibility-тестами на minimum/current HA (§9.3, риск-таблица §22)
+— а не подано как решённый факт.
+
+Отдельно проверено, что раздел «5. Принятые предположения» содержит только
+технические решения (имя route/element/icon, используемый публичный event,
+точное число px hit target, обработка native user overrides) — ни одно не
+подменяет продуктовый вопрос. Проверены все пять «Открытых вопросов к S2» из
+тела issue: все пять закрыты явными решениями либо в S2-комментарии (доступ
+`OWNER`, см. ниже), либо в самом ТЗ (§4 пп. 1, 4, 7, 8; §8 «Не входит»).
+
+### 2. Проверка полномочий продуктовых решений
+
+`gh issue view 486 --json comments` подтверждает, что оба решающих комментария
+(S2-аналитика с принятыми решениями для ТЗ и S3 «ТЗ готово») имеют
+`authorAssociation: OWNER` — то есть продуктовые вопросы (что видит
+read-only/writer пользователь без плана, видимость панели всем аутентифицированным
+пользователям, объём `getGridOptions` в этом же issue) закрыты стороной, которая
+имеет на это право по PROCESS.md §7.1, а не додуманы автором ТЗ.
+
+### 3. Проверка обязательных разделов ТЗ (PROCESS.md §7.1)
+
+| Раздел §7.1 | Есть в ТЗ | Где |
+|---|---|---|
+| Сценарий | ✅ | §1 |
+| Что человек увидит до/после | ✅ | §2 |
+| Проблема | ✅ (в §1 и явной ссылке на тело issue с цитатой Reddit-отзыва) | §1 |
+| Скоуп и не-скоуп | ✅ | §7, §8 |
+| Контракт поведения | ✅ | §9–§16 |
+| UX | ✅ | §12, §17 |
+| Модель данных и миграция | ✅ («не меняется», явно) | §16 |
+| i18n | ✅ (точные ключи EN/RU/DE/FR) | §15.2 |
+| AC1…ACn с доказательством | ✅ (15 AC, у каждого назван метод: backend/unit/smoke/golden/mutation/code review) | §19 |
+| План автотестов | ✅ | §20 |
+| Риски | ✅ (таблица вероятность/ущерб/защита) | §22 |
+| Откат | ✅ (независимый по частям: panel registration, panel entry, `getGridOptions`, notification text) | §23 |
+| Release-артефакты | ✅ | §24 |
+
+Все обязательные разделы присутствуют и не формальны — каждый содержит
+конкретные, проверяемые утверждения, а не пересказ задачи.
+
+### 4. Проверка непротиворечивости с продуктовым скоупом
+
+- Lock invariant (`docs/SCOPE.md` «Standing rule» и раздел про lock) не
+  затронут: панель не вводит новую поверхность actuation, `can_write`/`admin_only`
+  остаются единственным источником прав (§10), sidebar visibility явно назван
+  не-авторизацией.
+- `docs/TOUCH-SUPPORT.md`: View остаётся полностью touch-поверхностью,
+  редакторы остаются desktop-first best-effort — ТЗ прямо говорит, что #486 не
+  подменяет отдельный touch-аудит #31 (§17). Соответствует политике «улучшение
+  трогаем, только если оно дешёвое и не усложняет desktop-модель».
+  «Household members»/«Guests» из `docs/SCOPE.md` не переопределяются: панель
+  не меняет, кто есть кто, только точку входа.
+- `docs/CONFIG-COMPATIBILITY.md`: миграций нет, `CardConfig`/schema не меняются
+  — заявлено в ТЗ (§16) и не противоречит канону совместимости.
+
+## Находки
+
+Ничего блокирующего. Ниже — три Low, каждое снято решением ревьюера с записью
+(правка не требуется для перехода в `S5-ready`):
+
+1. **Low — неоднородная терминология метода доказательства.** AC4/AC5
+   называют способ доказательства «bundle gate», AC13 — «i18n gate»; в
+   каноническом словаре PROCESS.md §2.5 таких категорий нет (только
+   `unit`/`backend`/`smoke`/`golden`/«ревью кода»). По факту оба относятся к
+   `unit`: `test/i18n.test.mjs`/`test/i18n-dead-keys.test.mjs` и
+   `test/bundle-assets.test.mjs` уже существуют и являются частью обычного
+   `npm test`. Снимаю: способ доказательства однозначен и проверяем, несмотря
+   на неканоническую метку; переименование ничего не меняет по существу и не
+   стоит повторного цикла.
+2. **Low — двусмысленная формулировка одного пункта AC7.** «narrow
+   read-only/empty `320×720`» можно прочитать как один golden (комбинированное
+   состояние «пустой план, read-only» из таблицы §13) либо как два отдельных
+   golden (narrow read-only И отдельно narrow empty). Более естественное чтение
+   — первое (в §13 «Пустой план, read-only» — это ровно одна строка состояния),
+   и тестовый план §20 п. 2 отдельно называет «Empty writer onboarding and
+   empty read-only state» как отдельную пару сценариев от AC7. Снимаю: контекст
+   §13/§20 разрешает неоднозначность без необходимости переписывать AC.
+3. **Low — бюджет `initialPanelOnlyGzipBytes ≤ 8 KiB` (§11.4, AC5) не имеет
+   измеренного основания.** В отличие от `INITIAL_VIEW_GZIP_BUDGET` (текущее
+   значение 301 066 Б, откалиброванное по факту с историей в
+   `scripts/bundle-budget.mjs`), число «8 KiB» для нового panel-only довеска
+   нигде в репозитории не встречалось раньше и не подтверждено измерением
+   реального шелла (панель ещё не реализована). Снимаю: это чисто техническая,
+   легко пересматриваемая величина (откат §23 п.3 прямо разрешает независимую
+   правку), а не продуктовый контракт; если фактический размер шелла на
+   код-ревью выйдет за 8 KiB — это стандартная процедура повышения потолка «в
+   том же коммите с объяснением» (правило уже описано в
+   `scripts/bundle-budget.mjs`), а не дефект ТЗ.
+
+## Что проверено и корректно
+
+- Все продуктовые вопросы, поднятые в теле issue («Открытые вопросы к S2»),
+  закрыты явными решениями, авторизованными стороной с правом (`OWNER`).
+- Ни одно утверждение о текущем состоянии кода (раздел 3 ТЗ) не оказалось
+  домыслом — семь фактических тезисов проверены построчно на `origin/dev` и
+  подтвердились, включая не самый очевидный (пропуск `_canEdit` в конкретной
+  empty-state ветке `houseplan-card.ts:4162-4178`, а не во всём empty-state
+  рендере — кнопка «Add space» в табах уже корректно гейтуется `_canEdit` на
+  строке 11439, и ТЗ не путает эти два места).
+- API-риски, которые нельзя проверить чтением этого репозитория (совместимость
+  `panel_custom.async_register_panel` и приватного
+  `hass.data[frontend.DATA_PANELS]` с HA 2024.6), явно не выданы за факт: они
+  вынесены в риск-таблицу §22 и закрыты требованием compatibility-тестов на
+  minimum/current HA в AC1/AC2/AC14 — это корректный способ обращения с
+  неопределённостью на этапе ТЗ (не гадать, а требовать доказательство от
+  реализации).
+- Разделение product/technical в §5 «Принятые предположения» выполнено
+  корректно: каждый пункт — техническое решение или несущественная деталь
+  presentation, ни один не подменяет вопрос «что видит пользователь».
+- Скоуп и не-скоуп (§7–§8) точно соответствуют тому, что зафиксировано в S2 и
+  предложении issue: явно исключены server config панели, перенос CardConfig,
+  новый onboarding, kiosk панели, редизайн редакторов, #437 и отдельный
+  standalone asset.
+- «Одно число — один источник» (гейт §8/PROCESS.md): в этом диффе нет
+  пользовательски видимой величины, показанной дважды (панель не вводит новых
+  измерений/площадей/подписей) — типовой риск #234/#233 к этой задаче не
+  относится, признаю применимость проверил и отрицаю её.
+- i18n-скоуп (EN/RU/DE/FR) совпадает с реально существующими локалями и в
+  `custom_components/houseplan/translations/` и в `src/i18n/` — ТЗ не
+  придумывает несуществующий язык и не забывает существующий.
+- Rollback (§23) не требует миграции данных и позволяет откатывать panel
+  registration, panel entry и `getGridOptions` независимо друг от друга —
+  соответствует стандарту отката DoR (§2.5).
+
+## Чего не проверял и почему
+
+- **Код реализации** — не существует на этом этапе; будет предметом код-ревью
+  после `S6-in-progress`. Мутационные таблицы, конкретные unit/smoke/golden
+  тесты, бэкенд compatibility-стабы и фактический бюджет собранного
+  `houseplan-panel.js` не могут быть проверены до реализации.
+- **Точное поведение `panel_custom.async_register_panel` и приватного
+  `hass.data[frontend.DATA_PANELS]` в реальном Home Assistant 2024.6** — вне
+  репозитория (HA — внешняя зависимость, не vendored). ТЗ признаёт это риском и
+  требует compatibility-тестов; я не запускал реальный HA core, чтобы
+  independently подтвердить сигнатуры — это и не задача этапа ТЗ.
+- **Дешёвые гейты (`typecheck`/`test`/`build`) и `check-docs`** не
+  перепрогонялись: код не менялся, Validate зелёный на этом же SHA `2821c359`
+  (https://github.com/Matysh/houseplan-card/actions/runs/34159261423), а этап
+  ТЗ не предполагает изменений в `src/**`/`custom_components/**` — сам ТЗ-файл
+  лежит в `docs/**` (класс C), для которого дешёвые гейты кода не применимы.
+- **Golden/smoke/performance-профили, инварианты модели** — неприменимо: ТЗ не
+  меняет рендер, геометрию или ссылки на неё; изменение полностью в
+  `docs/specs/**`.
+
+## Материал раунда
+
+Заполняется конвейером публикации автоматически (PROCESS.md §2.10, п.1) —
+руками не редактировался.
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `issue/486-house-plan-panel`, коммит `2821c359634a` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Дерево материала: `736e05616d432cc61befaed33fbb1d114e6c79b9`
+  ```
+  git log --all --format='%H %T' | grep 736e05616d43
+  ```
+- ТЗ `docs/specs/486-house-plan-panel.md`, блоб `dd37e779f5cd5e4dae09b6b70453634448ece9cd`
+  ```
+  git log --all --find-object=dd37e779f5cd5e4dae09b6b70453634448ece9cd -- docs/specs/486-house-plan-panel.md
+  ```