From bf4c1bb674d9d69b781d377c0624f0fb249664d8 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 20:32:51 +0000 Subject: [PATCH] docs: review document for #486 Issue: #486 User-Visible: no --- docs/reviews/SPEC-REVIEW-486-r1.md | 228 +++++++++++++++++++++++++++++ 1 file changed, 228 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-486-r1.md diff --git a/docs/reviews/SPEC-REVIEW-486-r1.md b/docs/reviews/SPEC-REVIEW-486-r1.md new file mode 100644 index 00000000..7eb6377e --- /dev/null +++ b/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) — +руками не редактировался. + +--- + + + +## Материал раунда + +- Ветка: `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 + ```