docs: review document for #655

Issue: #655
User-Visible: no

docs/reviews/INDEX.md

@@ -1,6 +1,6 @@
 # Индекс ревью
 
-Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1108, issue: 396. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
+Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1109, issue: 397. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
 
 | Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы |
 |---|---|---|---|---:|---:|---|---|
@@ -41,6 +41,7 @@
 | #657 | [CODE-REVIEW-657-r1.md](CODE-REVIEW-657-r1.md) | code · r1 | 🔴 красный | 1 | 0 | после fast-forward слияния (dev не двигался за время ревью) docs/reviews/INDEX.md остаё… | `docs/reviews/INDEX.md` `test/reviews-index.test.mjs` `_process.yml` `merge-candidate.mjs` `test/merge-candidate.test.mjs` `scripts/reviews-index.mjs` |
 | #657 | [CODE-REVIEW-657-r2.md](CODE-REVIEW-657-r2.md) | code · r2 | 🟢 зелёный | 0 | 0 | из r1 — проверка закрытия | `scripts/merge-candidate.mjs` `PROCESS.md` `mutation-registry.mjs` `INDEX.md` |
 | #656 | [CODE-REVIEW-656-r1.md](CODE-REVIEW-656-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
+| #655 | [SPEC-REVIEW-655-r1.md](SPEC-REVIEW-655-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | — | — |
 | #654 | [SPEC-REVIEW-654-r1.md](SPEC-REVIEW-654-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 1 | «Release-артефакты» не называют обновление docs/ISOMETRIC.md | `docs/ISOMETRIC.md` `docs/CHANGELOG.md` `docs/CHANGELOG.ru.md` `docs/reviews/INDEX.md` |
 | #654 | [SPEC-REVIEW-654-r2.md](SPEC-REVIEW-654-r2.md) | spec · r2 | 🟢 зелёный | 0 | 0 | — | — |
 | #654 | [CODE-REVIEW-654-r1.md](CODE-REVIEW-654-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |

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

@@ -0,0 +1,216 @@
+# SPEC-REVIEW-655-r1
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/655
+- **Этап:** `S4-spec-review` (ревью ТЗ, PROCESS.md §2.4)
+- **Трек:** лёгкий (`small`), автор явно называет критерий: одна backend-поверхность
+  жизненного цикла, сложность 3/10, без миграции, нового UX-контракта, touch/perf
+  влияния.
+- **Материал:** тело issue #655 целиком (разделы «Проблема», «Ожидается», «Не
+  скоуп», «Аналитика», `## ТЗ`) + два комментария владельца (аналитика/оценка,
+  передача на ревью) + два служебных комментария о падении автоматического
+  прогона `model review` (полезны только как контекст: метка не менялась, до
+  этого раунда задача реального ревью не проходила).
+- **Заход:** r1 · блокирующих циклов израсходовано 0 из 2
+- **Роль:** ревьюер ТЗ (не автор)
+
+## Скоуп ревью
+
+Backend-регрессия долговечности: `VirtualLightController` (0,5 с коалесценция)
+и `TrailRecorder` (10 с debounce) гарантированно сбрасывают отложенную запись
+при unload/reload интеграции, но не при штатной остановке/перезапуске HA
+(`EVENT_HOMEASSISTANT_STOP`/`entry.async_shutdown`, не `async_unload_entry`).
+Задача — добавить единый идемпотентный shutdown-путь для обоих хранилищ и
+перевести фоновую задачу виртуальных ламп с голого `asyncio.create_task` на
+HA-tracked API. Не в скоупе: троттлинг toggle, ACL `system-read-only`,
+интервалы/формат store/трейлов, WebSocket API.
+
+**SCOPE-проверка (`docs/SCOPE.md`):** владелец относит задачу к J1 (живой
+статус на плане должен отражать последнее принятое состояние) и J6
+(«keep the plan true as the home evolves» — состояние обязано пережить
+штатное обслуживание HA). Обе фичи (virtual light toggle, trail book) уже
+приняты в продукт раньше этой задачи; здесь чинится не расширение
+функциональности, а её надёжность — новой scope-дыры это не открывает и
+существующий периметр не расширяет.
+
+## Как проверялось
+
+1. Прочитаны `docs/SCOPE.md`, `docs/process/REVIEWER.md` (по ссылкам —
+   PROCESS.md §2.4, §7.1, §7.2, §4; §2.10 не применялся — это r1), `AGENTS.md`.
+2. Прочитано тело issue #655 целиком (`gh issue view 655 --json body,comments,
+   labels`) — все четыре комментария владельца/пайплайна, вопросов владельцу в
+   тексте нет, подтверждаю.
+3. Сверены обязательные разделы §7.1:
+   - **сценарий + «что человек увидит»** — присутствуют одной фразой в разделе
+     «Ожидается» (это часть тела issue, не только `## ТЗ`; материал — тело
+     issue целиком): «домочадец переключил виртуальную лампу… администратор
+     перезапустил HA… после старта лампа в том состоянии, в каком её
+     оставили» — без терминов реализации, названы обе релевантные персоны
+     (`docs/SCOPE.md`: household members, home admin).
+   - **проблема** — описана дважды (верхний уровень + `### Проблема` внутри
+     `## ТЗ`), согласованно, с точными путями/строками кода.
+   - **скоуп/не-скоуп** — явно, дважды (верхний уровень кратко, `### Вне
+     скоупа` подробно), без противоречий между версиями.
+   - **контракт поведения** — 5 пронумерованных пунктов, каждый проверяем
+     независимо (см. п.4 ниже — перепроверены по коду).
+   - **UX** — отдельного заголовка нет, но это оправдано: правка не трогает
+     ни одного видимого интерактивного элемента, весь наблюдаемый эффект уже
+     покрыт сценарием «что человек увидит» (состояние после restart). Нет
+     видимой поверхности, которую полноценный раздел UX мог бы описать сверх
+     этого.
+   - **модель данных и миграция** — явное «нет»: контракт п.5 «Никакой
+     миграции данных или новой настройки нет», формат store/трейлов не
+     меняется (контракт п.4).
+   - **i18n** — не упомянут явно, но обоснованно: изменение не добавляет ни
+     одной пользовательской строки (только серверный `_LOGGER`), других
+     backend-задач в проекте i18n тоже не касается.
+   - **план автотестов** — распределён по AC1–AC5, каждый называет метод
+     доказательства (HA-harness/unit/mutant) — соответствует требованию
+     «указание способа доказательства» для каждого AC.
+   - **риски и откат** — `### Риски и откат`, названа единственная реальная
+     угроза (двойной cleanup / гонка timer-flush) и как она закрыта
+     (идемпотентность + AC3).
+   - **release-артефакты** — `User-Visible: yes`, оба changelog в том же
+     коммите, generated bundle не меняется — явно.
+4. **Перепроверены фактические утверждения ТЗ по коду на SHA `9a7020ce`
+   (== origin/dev), а не приняты на слово:**
+   - `custom_components/houseplan/virtual_lights.py:128-130` (`_schedule_save`)
+     — буквально `asyncio.create_task(self._delayed_save())`, не HA-tracked
+     API. Подтверждено дословно как в issue.
+   - `custom_components/houseplan/__init__.py:258-275` (`async_unload_entry`)
+     — вызывает `rec.async_teardown()` и `virtual_lights.async_flush()`, но
+     других обработчиков `EVENT_HOMEASSISTANT_STOP` не регистрирует.
+   - `grep -rn "EVENT_HOMEASSISTANT_STOP" custom_components/houseplan/` — 0
+     вхождений во всём пакете (только `radar.py:117` слушает
+     `EVENT_CORE_CONFIG_UPDATE`, к делу не относится). Заявление «слушателя
+     нет» подтверждено, не принято на веру.
+   - `custom_components/houseplan/trails.py:472-478` (`async_teardown`) —
+     под `self._refresh_lock` вызывает `_close_subscriptions()` и, если была
+     отложенная запись (`pending`), `await self.store.async_save(self.book.data)`
+     — ровно то поведение, которое ТЗ называет «уже безопасным при
+     reload/unload».
+   - `tests_backend/test_ha_virtual_lights.py:114` —
+     `test_unload_flushes_a_toggle_still_inside_the_debounce_window` существует
+     и называется как в issue; тест `hass.async_stop()`-сценария в проекте не
+     найден (`grep -rn "async_stop" tests_backend/` — 0 совпадений) —
+     подтверждает пробел в покрытии, который и закрывают AC1/AC2.
+   - `custom_components/houseplan/manifest.json:22` — `single_config_entry:
+     true`. Это устраняет риск, который я сам держал в уме при чтении
+     контракта п.1–2 (совместное состояние `TrailRecorder`, ключ
+     `hass.data[DOMAIN]["trail_recorder"]`, не per-entry): при ровно одном
+     возможном entry «один привязанный к entry обработчик» не может
+     коллизировать с другим экземпляром интеграции.
+   - `hass.async_create_task` / `entry.async_create_background_task` уже
+     используются в проекте (`radar.py:115`,
+     `frontend_registration.py:561`, `websocket_api.py` в нескольких местах)
+     — контракт п.3 не изобретает новый паттерн, а переиспользует
+     существующий, что снижает технический риск реализации.
+   - `VirtualLightController.__init__` (`virtual_lights.py:79-86`) не хранит
+     `hass`/`entry` — реализации потребуется протянуть ссылку на `hass` в
+     конструктор или передавать её в `_schedule_save`. Это чисто техническая
+     деталь (модуль/интерфейс класса — то, что ТЗ прямо отдаёт исполнителю),
+     не продуктовый вопрос и не блокер: конструктор вызывается один раз
+     (`store.py:136`) в контексте, где `hass` доступен.
+5. Ветки/коммитов для #655 не существует (`git log --all --oneline | grep 655`
+   — пусто; `git diff origin/dev...HEAD` пуст, `HEAD == origin/dev ==
+   9a7020ce`). Гейты (`tsc`, `test`, `build`, смоки, `golden`, `pytest
+   tests_backend`, инварианты) неприменимы на этапе `spec` — продуктового
+   кода ещё нет, это штатное состояние, а не пропущенный гейт.
+6. HA-рантайм-утверждение («на `EVENT_HOMEASSISTANT_STOP` HA зовёт
+   `entry.async_shutdown`, не `async_unload_entry`») сам автор помечает как
+   «по знанию кода HA, не по прогону» — я не смог перепроверить его
+   исполнением: `python3 -c "import homeassistant"` в песочнице отсутствует
+   (`ModuleNotFoundError`), как и предупреждает `AGENTS.md` о песочнице.
+   Формулировка честно ограничена, доказательство отложено на HA-harness
+   в каноническом Linux CI (AC5 это явно называет) — не считаю это
+   находкой, так как автор сам не выдаёт догадку за факт, а называет её
+   пределы.
+
+## Находки
+
+Нет. Ни одной находки High/Medium/Low после перепроверки кода — все
+утверждения ТЗ подтвердились чтением исходников, а не приняты на слово.
+
+Одно наблюдение, не являющееся находкой (не блокирует, не требует
+действия): AC1/AC2 называют тестовый приём «вызвать штатный
+`hass.async_stop()`». Часть HA-интеграций в тестах вместо полной остановки
+`hass` предпочитают `hass.bus.async_fire(EVENT_HOMEASSISTANT_STOP)` +
+`await hass.async_block_till_done()` — легче для фикстуры, не завершает
+цикл событий раньше, чем тест успеет прочитать файл store после. Это
+стратегия теста, а не продуктовый контракт (§7.1: «стратегия тестов» —
+то, что решает автор/ревьюер, не владелец), и ТЗ уже прямо резервирует
+исполнителю свободу технических решений. Оставляю на усмотрение
+реализации без возврата.
+
+## Что проверено и корректно
+
+- Обязательные разделы §7.1 присутствуют по существу; сценарий и «что
+  человек увидит» — продуктовые, одной фразой, без терминов реализации.
+- Оба факта дефекта (голая `asyncio.create_task` у виртуальных ламп,
+  отсутствие слушателя `EVENT_HOMEASSISTANT_STOP` во всём пакете) —
+  перепроверены построчным чтением кода, а не приняты со слов автора.
+- Контракт из 5 пунктов однозначен, пункты не пересекаются и не
+  противоречат друг другу; «идемпотентно» и «не создаёт новую запись при
+  отсутствии pending-изменений» — прямо адресуют риск двойного cleanup,
+  который сам ТЗ называет в разделе «Риски».
+- AC1–AC5 каждый называет способ доказательства (HA-harness с конкретными
+  шагами / unit-структурный свидетель / мутант) — критерий «указание
+  способа доказательства» выполнен по всем пяти.
+- AC5 заранее формулирует негативное доказательство (мутанты, реестр
+  которых уже существует в проекте, `scripts/mutation-registry.mjs`) — это
+  снижает риск пустой графы «чем краснеет» на код-ревью.
+- `single_config_entry: true` в манифесте снимает единственный
+  архитектурный риск, который я держал в уме при чтении контракта
+  (совместное состояние `TrailRecorder` при нескольких entries).
+- Технический паттерн (`hass.async_create_task`/
+  `entry.async_create_background_task`) уже используется в проекте для
+  аналогичных фоновых задач — реализуемость контракта п.3 подтверждена
+  прецедентом, а не гипотетична.
+- Открытых продуктовых вопросов нет — подтверждаю по тексту: единственное
+  наблюдаемое поведение («последнее принятое состояние переживает
+  штатный stop/restart») уже полностью и однозначно определено сценарием
+  и контрактом, домысливать нечего.
+- Release-артефакты, откат, отсутствие миграции — названы явно, без
+  зазора для догадки.
+
+## Чего не проверял
+
+- Гейты (`tsc --noEmit`, `npm test`, `npm run build`, `pytest
+  tests_backend`, инварианты, смоки, `golden:verify`) — не прогонял: этап
+  `spec`, кода для #655 нет, `git diff origin/dev...HEAD` пуст. Предмет
+  код-ревью после реализации.
+- Не проверял исполнением HA-рантайм-утверждение про
+  `EVENT_HOMEASSISTANT_STOP` vs `entry.async_shutdown` — HA не
+  устанавливается в песочнице (`ModuleNotFoundError: homeassistant`); сам
+  автор ограничивает это утверждение как «по знанию кода, не по прогону»
+  и переносит фактическую проверку на AC5/HA-harness в каноническом Linux
+  CI — это его собственная явная граница, а не пропуск с моей стороны.
+- Не оспаривал конкретный числовой сценарий («увеличить окно debounce») как
+  техническую деталь теста — оставляю технику на усмотрение реализации,
+  как и позволяет §7.1 («стратегия тестов» — не продуктовый вопрос).
+- Не проверял, потребует ли реализация протянуть `hass`/`entry` в
+  `VirtualLightController.__init__` изменения сигнатуры конструктора —
+  архитектурная деталь модуля, прямо отданная исполнителю ТЗ.
+
+## Вердикт
+
+Обязательные разделы полны и по существу, оба факта дефекта и
+архитектурные предпосылки контракта перепроверены чтением кода — расхождений
+не найдено. AC1–AC5 однозначны, каждый называет способ доказательства,
+негативное доказательство (AC5) спланировано заранее. Открытых продуктовых
+вопросов нет. Готово к разработке.
+
+Вердикт: зелёный · заход r1 · блокирующих циклов 0/2 · High: 0 · Medium: 0 → в задаче
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `dev`, коммит `9a7020ce428e` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Дерево материала: `eec9247ab39136663b248226de905169bb3d18bc`
+  ```
+  git log --all --format='%H %T' | grep eec9247ab391
+  ```
+- Тело issue: `9592e83bda3f396fe43ee14eb6bc22b4860f8309d2c5e4e954dc1b90a293c348`
+- Вердикт конвейера: `green` · High 0