diff --git a/docs/reviews/SPEC-REVIEW-462-r1.md b/docs/reviews/SPEC-REVIEW-462-r1.md new file mode 100644 index 00000000..3e732c01 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-462-r1.md @@ -0,0 +1,208 @@ +# SPEC-REVIEW-462-r1 + +- **Issue:** https://github.com/Matysh/houseplan-card/issues/462 +- **Спецификация:** `docs/specs/462-card-resource-registration.md`, коммит `12ddd107` + (ветка `issue/462-card-resource-registration`, `HEAD` на момент ревью — тот же SHA) +- **Трек:** полный (аналитика в issue назвала нарушенные критерии `small`: + несколько поверхностей, новый UX-контракт notification + kiosk auto-reload) +- **Заход:** r1 · блокирующих циклов израсходовано 0 из 4 (лимит на полном треке — 4) + +## Скоуп ревью + +Прочитано перед разбором: `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` (§1–10), +тело issue #462 и все 7 комментариев, `docs/UX-MODES.md` (Kiosk mode), +`docs/TOUCH-SUPPORT.md`, `docs/CONFIG-COMPATIBILITY.md` (выборочно, по ключевым +словам kiosk/reload/version), `docs/USER-GUIDE.ru.md` (установка, киоск, +существующий текст про плашку/перезагрузку из #353), README.md/README.ru.md, +docs/USER-GUIDE.md. + +Сам документ ТЗ прочитан целиком (§1–22). Проверено соответствие §7.1 +`PROCESS.md`: все обязательные разделы присутствуют (сценарий — §1; что человек +увидит — §2; проблема/подтверждённая причина — §3; скоуп/не-скоуп — §6/§7; +контракт поведения — §8–11 + §4; UX — §11.2/§12; модель данных и миграция — +§13; i18n — §14; AC1…AC13 с доказательством — §15; план автотестов — §16; +риски — §19; откат — §20; release-артефакты — §21); плюс обязательный блок +«принято предположительно, поменять свободно» — §22. + +## Как проверялось + +Ревью состязательное: без устных пояснений автора, только issue + ТЗ + код на +`dev`. Каждое нетривиальное техническое утверждение спецификации сверено с +реальным поведением платформы, а не принято на слово: + +1. **Существование ссылок на код.** Прочитаны и подтверждены в `dev` + (`16b4c2d6`/текущий `HEAD`): `_register_lovelace_resource` и + `_lovelace_resources` (`custom_components/houseplan/__init__.py:336–378`), + `manifest.json` (dependencies без `lovelace`), `system_health_info` + (`custom_components/houseplan/system_health.py`), `CARD_VERSION` в + `src/houseplan-card.ts:402` и отдельно в `src/houseplan-editor-runtime.ts:379` + (гейт `scripts/release-contract.mjs` уже держит их синхронными — не новая + проблема, существующий механизм), `_haIntegrationVersion` + (`src/houseplan-card.ts:2039,4094-4095,4882-4883` — **подтверждено**: при + отсутствующем/некорректном `integration_version` текущий код **не + очищает** значение, оставляет старое — именно это правит AC6/§11.1, и это + реальный баг, а не выдумка), `_preflightVersionsDiffer` + (`houseplan-editor-runtime.ts:9610`), `_cyclePausedUntil`/`_zoom`/`_editing` + /`_pendingPhysicalWrites`/`_writesPending`/`_vacFit` + (`src/houseplan-card.ts`, `houseplan-editor-runtime.ts` — все существуют + ровно с тем смыслом, который им приписывает ТЗ). +2. **Проверка ключевой технической гипотезы §3 против реального HA API.** + Спецификация утверждает: hard-зависимость `houseplan → frontend → lovelace` + уже гарантирует порядок настройки на HA 2024.6 (минимально поддерживаемая) и + актуальной ветке. Я не поверил на слово и скачал реальные wheel-пакеты + `homeassistant==2024.6.0` и `homeassistant==2025.1.4` (более новых нет в + доступном индексе) и прочитал `manifest.json` обоих компонентов: + `frontend` **действительно** объявляет `"dependencies": [..., "lovelace", ...]` + в обеих версиях; `lovelace` зависит только от `onboarding`. Цепочка в ТЗ + подтверждена буквально, а не додумана — редкий случай, когда стоило + перепроверить смелое техническое утверждение и оно оказалось верным. +3. **Проверка технической реализуемости §8.2 п.9 (`remove_extra_js_url`).** + В том же скачанном `homeassistant==2024.6.0` функции + `remove_extra_js_url` в `homeassistant/components/frontend/__init__.py` + **нет** (появляется только в более поздних версиях — подтверждено в + `2025.1.4`). Это значит, что состояние `lovelace_resource_with_session_fallback` + из §8.3 — не гипотетический край, а **обязательный путь на всей матрице + минимально поддерживаемой HA**, если retry случился после fallback. ТЗ это + учитывает явно (AC3: «…снимает fallback либо честно маркирует его + остаток»). Найдено правильно. +4. **Проверка `async_at_started`.** Подтверждено по исходнику + `homeassistant/helpers/start.py` (2024.6): возвращает `CALLBACK_TYPE`, + пригодный для `entry.async_on_unload`, ведёт себя точно как описано в §22 п.3. +5. **Сверка документации.** `grep` по всем `.md` файлам репозитория нашёл ровно + 4 места с невалидным плоским `resources:` — README.md:101, README.ru.md:105, + docs/USER-GUIDE.md:94, docs/USER-GUIDE.ru.md:93 — это ровно тот список, + который называет §12/§17. Пропущенных мест нет. +6. **Проверка терминологии.** `kiosk: true`, `_cyclePausedUntil`, 60-секундная + пауза автолистания и условие масштаба 1:1 совпадают с уже описанным + контрактом в `docs/UX-MODES.md` («Kiosk mode»). i18n EN/RU/DE/FR — то же + множество языков, что уже существует в `src/i18n/` и + `custom_components/houseplan/translations/`, не новая номенклатура. + +## Находки + +### Medium (в скоупе задачи) — пересечение нового баннера с существующим тостом #353 + +`docs/USER-GUIDE.ru.md:111-119` документирует уже существующий (issue #353) +механизм: если открытая вкладка держит код одной сборки, а lazy-чанк +редактора (`Plan`/`Devices`/`Подложка`) отдаётся с несовпадающим +`ENTRY_BUILD_FINGERPRINT` (`src/editor-runtime-loader.ts`), при попытке +открыть редактор показывается **тост** «`editor.load_failed` + +`editor.refresh_advice`» — «поможет только перезагрузка страницы». Это +триггерится независимо от `CARD_VERSION`/`integration_version` — по +fingerprint конкретного lazy-чанка, полученного через `import()`. + +Новый runtime version controller (§11.1–§11.2) вводит **второй** независимый +триггер той же самой user-facing просьбы («перезагрузите страницу») — +несовпадение `CARD_VERSION`/`integration_version` — и явно требует показывать +его «в обычном режиме... включая editor и открытый dialog» (§11.2). При этом: + +- ничто в `_setMode`/клике по вкладке редактора (`src/houseplan-card.ts:7429-7444`, + `:11451`) не блокируется и не учитывает состояние version-mismatch — переход + в редактор всегда возможен; +- реалистичный сценарий: HA/интеграция обновились, у пользователя открыта + вкладка → banner уже показан (баннер триггерится раньше, сразу на + следующем `config/get`); пользователь всё равно кликает «План» → lazy-чанк + редактора либо переехал/удалён на новую версию, либо не совпадает + fingerprint'ом → **тост поверх уже показанного баннера**, с другим текстом + того же смысла. + +ТЗ ни разу не обсуждает это пересечение в поведенческих разделах (§8–22): #353 +упомянут только в шапке ("Связано") без анализа. Не решено ни одно из: +подавлять ли тост, пока баннер уже сообщил о том же; должен ли известный +mismatch блокировать/предупреждать вход в редактор заранее; допустимо ли +одновременное появление тоста и баннера как есть. Это ровно то расхождение, +о котором предупреждает §8 `PROCESS.md` («одно число — один источник») в +широком смысле — здесь не число, а **одна и та же пользовательская просьба +через два независимых, не согласованных друг с другом триггера** — тот же +класс дефекта, что стоил продукту #234/#233. + +**Возврат автору.** Достаточно одного явного решения в ТЗ (тост подавляется, +пока показан баннер того же смысла — или наоборот, или оба допустимы с +обоснованием) плюс отражение выбора в AC12/плане тестов. High-находок нет, +поэтому по Medium-в-скоупе — жёлтый вердикт, чинится в этой же задаче +(владелец, 2026-08-19, #202). + +### Low — числовой touch-target 44×44 не имеет источника в каноне + +§11.2 требует «touch-target кнопки не менее 44×44 CSS px». Ни в +`docs/TOUCH-SUPPORT.md`, ни в `docs/UX-MODES.md`, ни в существующем коде +(`grep` по `44px`/`min-width: *44` в `src/*.ts` — пусто) такого порога нет; это +не переиспользование канона, а собственное (хоть и общепринятое, +WCAG/Apple HIG) число автора. Не помечено в §22 как предположение. Влияния на +продукт это не меняет (число консервативное и безопасное), поэтому не +поднимаю до Medium — можно поправить формулировкой «стандартный +accessibility-минимум (не описан в каноне отдельно)» или добавить строкой в +§22. Не блокирует, правится на усмотрение автора либо снимается запиской. + +## Что проверено и корректно + +- Все обязательные разделы §7.1 присутствуют по существу (см. «Скоуп ревью»). +- Каждый AC1–AC13 имеет однозначную формулировку и названный способ + доказательства (`unit`/`backend`/`browser`/`golden`/`build`/`ревью кода`), + без AC, доказательство которых осталось бы неясным. +- Ни одного продуктового вопроса, оставленного нерешённым: единственная + продуктовая развилка (тихая kiosk-перезагрузка) решена владельцем в + комментариях issue (05.09.2026) и внесена в тело/ТЗ дословно. +- Технический выбор `persistent_notification` вместо Repairs (§22 п.1) + первоначально выглядел как неподтверждённое решение владельца («потому что + контракт владельца» без цитируемого комментария) — проверено против + прецедента: `custom_components/houseplan/repairs.py` уже использует Repairs + строго для сохраняющихся дефектов (пропавший файл плана, снимается при + исправлении), что структурно отличается от одноразового onboarding-сообщения + «карточка подключена, перезагрузите». Выбор обоснован существующей + конвенцией репозитория и корректно помечен как «можно менять на ревью» — + не поднимаю как находку. +- Гипотеза о startup race (§3) и связанные с ней §8.2/§22 п.3 проверены против + реального API `homeassistant` 2024.6/2025.1.4 (см. «Как проверялось», п.2–4) + — технически точны. +- Список файлов документации, которые чинит §12, — исчерпывающий (сверено + `grep`). +- Терминология (kiosk, плашка/подсказка, языки i18n) не изобретена, берётся из + `docs/USER-GUIDE.ru.md`/`docs/UX-MODES.md`/существующего `src/i18n`. +- `docs/CONFIG-COMPATIBILITY.md`: заявление §13 «формат плана/layout/store не + меняются, миграция не нужна» согласуется с общей философией документа + (аддитивные optional-поля не требуют миграции схемы). +- `docs/TESTING.md:972` подтверждает необходимость правки release-чеклиста, + названную в §21 (текущий пункт не упоминает обязательность reload). +- SCOPE.md: задача закрывает J4 (zero-to-plan onboarding) и общую надёжность + первого запуска; из «Out of scope» ничего не задето; lock-инвариант не + применим (нет управления замками). + +## Чего не проверял + +- Не проверялась реализация (её нет — это ревью ТЗ, код ещё не написан). +- Не прогонялись `typecheck`/`test`/`build` — на этапе спецификации нет кода + для сборки; гейты применимы к код-ревью, не к этому этапу. +- Не проверялся HA 2026.8 напрямую — недоступен в индексе pip на момент + ревью (последняя видимая версия — 2025.1.4); экстраполяция на «актуальную» + ветку опирается на многолетнюю стабильность связи `frontend → lovelace` в + проверенных версиях, а не на точный релиз. +- Не проверялось поведение `persistent_notification.async_create` с точки + зрения точного API (idempotent `notification_id`) — это стандартный, + многократно проверенный в экосистеме HA паттерн, не специфичный для этой + задачи; углублённая проверка не показалась пропорциональной риску. +- Не оценивался landing page/дистрибуция (вне репозитория, зона Claude по + `AGENTS.md`, не часть ТЗ). + +## Материал раунда + +- Ветка: `issue/462-card-resource-registration` +- SHA материала: `12ddd107a9cdd9b12ea8411360760f684f5293c1` +- Файл ТЗ: `docs/specs/462-card-resource-registration.md` (полная редакция, + зафиксированная этим коммитом) + +--- + + + +## Материал раунда + +- Ветка: `issue/462-card-resource-registration`, коммит `12ddd107a9cd` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `2e4d5a20cc878962b7689e3a8603703577daba01` + ``` + git log --all --format='%H %T' | grep 2e4d5a20cc87 + ``` +- ТЗ `docs/specs/462-card-resource-registration.md`, блоб `fd9bc322a17b303c3e76495ef3233fb9324010d8` + ``` + git log --all --find-object=fd9bc322a17b303c3e76495ef3233fb9324010d8 -- docs/specs/462-card-resource-registration.md + ```