docs: review document for #545

Issue: #545
User-Visible: no

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

@@ -0,0 +1,194 @@
+# SPEC-REVIEW-545-r1
+
+Issue: #545 «Радары: range/zone entity_id теряются из списка источников для
+подписок и проверки прав»
+Этап: ревью ТЗ (PROCESS.md §2.4)
+Заход: r1 · трек: full (не `small`) · блокирующих циклов израсходовано 0 из 4
+Ревьюер: Claude (роль «ревьюер ТЗ», §6)
+Материал: тело issue #545, раздел `## ТЗ`, на состоянии на момент разбора;
+issue-комментарий «S2 — аналитика» (тот же материал, не решение по существу
+вопроса).
+
+## Скоуп ревью
+
+Проверить, что ТЗ в теле issue #545 удовлетворяет §7.1 (обязательные разделы,
+однозначность AC1…AC7 и указанный способ доказательства для каждого), что
+продуктовые разделы («Сценарий», «Что человек увидит») отвечают на свои
+вопросы, и что ни одно утверждение о существующем поведении не выдано за
+факт без проверки. Это первый заход по этому ТЗ — версия для r2+ «разбор по
+дельте» (§2.10) здесь неприменима.
+
+## Как проверялось
+
+Материал — исключительно текст: код смотрел не для того, чтобы утвердить
+реализацию (её ещё нет), а чтобы отличить утверждение о факте от догадки,
+которую §7.1 требует помечать явно. Прочитано и сверено построчно:
+
+- `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` (§1, §2.3–2.5, §5, §7.1, §7.2);
+- `docs/RADAR.md` — канонический документ подсистемы, раздел «Privacy,
+  permissions and storage» и таблица профилей;
+- `custom_components/houseplan/radar_validation.py` — `_source_ids()`,
+  `radar_source_entity_ids()`, `_validate_sources()` для всех шести профилей;
+- `custom_components/houseplan/radar.py` — `_resubscribe_sources()`,
+  `source_ids()`, `teardown()`, `_ensure_tick()`;
+- `custom_components/houseplan/radar_websocket.py` — `_can_read()`,
+  `ws_radar_subscribe`, `ws_radar_setup_subscribe`, `source_restricted`/
+  `health: restricted` пути;
+- `tests_backend/test_ha_radar_websocket.py` — существующий `_Permissions`
+  fake (булев allow/deny, не per-entity) — сверка риска, названного в ТЗ;
+- `scripts/mutation-gate.mjs`, `docs/TESTING.md` — подтверждение, что реестр
+  поддерживает Python-мутанты с `pytest`-гвардом (не только фронтенд), и что
+  формат идентификатора мутанта из ТЗ соответствует принятой конвенции;
+- список файлов `tests_backend/test_radar_validation.py`,
+  `test_ha_radar.py`, `test_ha_radar_websocket.py` — существование
+  подтверждено (`ls`).
+
+Гейты не гонялись: на этапе ревью ТЗ кода ещё нет, гонять `typecheck`/`test`/
+`build` не над чем — это ожидаемо для S4-spec-review, а не пропуск.
+
+## Проверка утверждений ТЗ о текущем поведении (не догадка ли это)
+
+Раздел «Проблема» и «Контракт поведения» делают несколько фактических
+заявлений о существующем коде. Каждое сверено с источником, а не принято на
+слово:
+
+| Заявление ТЗ | Где проверено | Результат |
+|---|---|---|
+| `_source_ids()` собирает вложенные поля только по суффиксу `_entity` | `radar_validation.py:99-105` | Подтверждено: `if key.endswith("_entity")` |
+| `range_v1`/`zones_v1` называют основной источник `entity_id`, а не `*_entity` | `radar_validation.py:244, 259` | Подтверждено: `unique_role(item.get("entity_id"), ...)` для ranges, `_entity(item.get("entity_id"), ...)` для zones. `"entity_id".endswith("_entity")` — ложно (сравнил посимвольно) |
+| Coordinator реально читает `entity_id` при построении frame | `radar.py:502, 531` | Подтверждено: `self.hass.states.get(source.get("entity_id"))` |
+| Подписки коллектора берутся из того же неполного `radar_source_entity_ids()` | `radar.py:189-193` | Подтверждено: `entity_ids = sorted({... for entity_id in radar_source_entity_ids(...)})`, передаётся в `async_track_state_report_event`/`_change_event` |
+| `_can_read()`/ACL получает тот же неполный список | `radar_websocket.py:56, 128, 184, 224-225` | Подтверждено: все проверки идут через `coordinator.source_ids()` → `radar_source_entity_ids()` либо `validate_radar_draft()` → та же функция |
+| Setup subscribe не имеет секундного tick-фолбэка, в отличие от live | `radar_websocket.py:196-251` vs `radar.py:258-268` (`_ensure_tick`) | Подтверждено: `_ensure_tick`/периодическая публикация существует только у `RadarCoordinator` (для активных `add_listener`), у `ws_radar_setup_subscribe` publish идёт исключительно по `source_reported` на пересчитанном `source_ids` |
+| `restricted`/`source_restricted` — уже существующее fail-closed поведение, а не новое | `radar_websocket.py:57,132,185,226,239` | Подтверждено, задача переиспользует существующий путь, не изобретает новый |
+| Существующий тест использует общий allow/deny boolean, а не per-entity fake (риск в разделе «Риски») | `tests_backend/test_ha_radar_websocket.py:23` (`_Permissions`, `connection.user.permissions.allowed = False/True`) | Подтверждено: это ровно тот недостаточный дизайн, который ТЗ требует не повторять в AC4 |
+| `mutation-gate.mjs` умеет патчить backend `.py` и гонять `pytest`-гвард | `scripts/mutation-gate.mjs` (например записи с `guard: 'python3 -m pytest tests_backend/...'`, `file: 'custom_components/houseplan/*.py'`) | Подтверждено, AC5 технически осуществим на этом реестре |
+| Перечисленные роли по профилям (общие optional + по группам `slots`/`ranges`/`zones`) | `radar_validation.py:198-266` | Построчно совпадает с контрактом ТЗ п.2–3 для всех шести профилей, включая отсутствие `presence_entity` у zones и обязательность `occupancy_entity` только для `presence_v1` |
+
+Ни одно из проверенных утверждений не оказалось догадкой, выданной за факт:
+там, где ТЗ формулирует именно новое поведение (например, п.6–7 контракта —
+«saved coordinator подписывается на оба HA event stream для полного
+инвентаря»), это явно нормативное требование к реализации, а не описание
+того, что уже так работает.
+
+## Обязательные разделы §7.1
+
+Все присутствуют: сценарий · что человек увидит до/после · проблема · скоуп
+и не-скоуп · контракт поведения · UX · модель данных/миграция/совместимость ·
+затронутые файлы · AC1…AC7 с доказательством · план автотестов · перф ·
+безопасность · риски · откат · release-артефакты · блок «принято
+предположительно». Пункт DoR по i18n закрыт явным «frontend/i18n не
+меняются» — это корректная форма пустого списка, а не пропуск раздела.
+
+## Проверка AC1…AC7 на однозначность и доказуемость
+
+- **AC1** (backend unit) — таблица для всех 6 профилей, включая точное имя
+  роли `entity_id` у range/zone и отрицательный случай произвольного
+  future-поля. Проверяемо механически, способ доказательства назван.
+- **AC2** (backend HA) — саму развилку «что считается обходом» ТЗ снимает
+  прямым запретом полагаться на occupancy/count/presence в фикстуре; это
+  устраняет ровно тот тип ложного прохождения теста, что PROCESS.md требует
+  предотвращать (§2.7, «двух источников с разными списками»).
+- **AC3** (backend HA) — три отдельных факта (subscribe range/zone id,
+  rebind заменяет набор целиком, unload снимает всё) — все три проверяемы по
+  отдельности, не слиты в одно расплывчатое утверждение.
+- **AC4** (backend HA, security) — явно требует записывающий per-entity fake
+  permission (не общий boolean) и раздельно saved/draft путь — снимает
+  собственный риск, названный в разделе «Риски», а не просто декларирует его.
+- **AC5** (mutation) — конкретный id мутанта, конкретная механика удаления
+  роли, обязательный `--check`. Соответствует формату существующего реестра.
+- **AC6** (backend + ревью кода) — числовой инвариант («два listener»)
+  сверяемый по факту: сейчас уже два (`state_report`+`state_change`) и на
+  коордиторе, и на setup — AC фиксирует неизменность этого числа при
+  расширении набора entity, а не декларирует что-то новое и непроверяемое.
+- **AC7** (backend + docs review) — стандартный набор доказательств
+  совместимости и артефактов, ничего скрытого.
+
+Ни один AC не сформулирован как «неявное намерение», каждый называет способ
+доказательства (`unit`/`backend HA`/`mutation`/`ревью кода`), как того
+требует §2.5.
+
+## Продуктовые разделы и вопрос владельцу
+
+«Сценарий» называет персону/поверхность/момент (home admin, мастер настройки
+или обычный план, изменение основного датчика диапазона/зоны), «Что человек
+увидит» — одной фразой без терминов реализации. Автор аналитики (S2)
+заявила, что открытых продуктовых вопросов нет, потому что ожидаемое
+поведение уже зафиксировано существующим контрактом `docs/RADAR.md`
+(«House Plan subscribes only to the exact configured entity IDs») и не
+вводит нового UI/UX. Прочитав материал независимо, соглашаюсь: расхождение
+между валидацией и inventory — техническая ошибка одной реализации, а не
+неопределённость продукта, поэтому пачка вопросов владельцу здесь была бы
+избыточной (§7.1 «вопрос, не блокирующий написание ТЗ, не задаётся вовсе»).
+
+## Скоуп и не-скоуп
+
+Не-скоуп корректно исключает Stage 2/3, историю, heatmap, изменения
+`may_write`/прав HA и миграцию — всё это действительно посторонние
+подсистемы. Скоуп ограничен одной точкой истины (profile → source roles) и
+её тремя потребителями (validation, subscriptions, ACL), что соответствует
+корню проблемы, а не симптомам по отдельности.
+
+## Откат
+
+Явно запрещён частичный откат «только ACL» или «только подписок» — важная
+деталь: такой частичный откат воссоздал бы именно тот дефект, который
+задача чинит (расхождение списков), и явный запрет предотвращает случайную
+регрессию через невнимательный cherry-revert.
+
+## Что проверено и корректно
+
+- Все шесть заявленных факта о текущем коде подтверждены построчным чтением
+  (таблица выше), включая тонкий момент: `"entity_id".endswith("_entity")`
+  действительно ложно.
+- Все файлы из «Затронутые файлы» существуют.
+- Роли по профилям в контракте поведения дословно совпадают с
+  `_validate_sources()`.
+- Мутационный гейт поддерживает заявленный тип мутанта на Python-файле.
+- Откат, риски и не-скоуп внутренне непротиворечивы.
+
+## Чего не проверял
+
+- Не запускал автотесты и не собирал бандл — на этапе ревью ТЗ кода ещё нет,
+  проверять нечего; это не гейт этого этапа.
+- Не оценивал качество будущей реализации — вне скоупа ревью ТЗ.
+- Не проверял `docs/specs/485-radar-presence-stage1.md` построчно на полное
+  совпадение (упомянут в `docs/RADAR.md` как нормативный контракт Stage 1);
+  сверка ограничилась текущим кодом, что для проверки *этого* багфикса
+  достаточно.
+
+## Находки
+
+Нет. Ни одной High, ни одной Medium, ни одной Low.
+
+## Вердикт
+
+Зелёный. ТЗ полное по §7.1, каждый AC однозначен и снабжён способом
+доказательства, продуктовые разделы отвечают на свои вопросы, ни одно
+техническое утверждение не оказалось непроверенной догадкой — напротив,
+каждое фактическое заявление о текущем коде подтвердилось при чтении
+источника, включая нетривиальные детали (dead-code ветка zones в
+`_resubscribe_sources`, недостаточность существующего permission-fake).
+Открытых продуктовых вопросов владельцу нет и не требуется.
+
+---
+
+## Материал раунда
+
+- SHA issue-материала: тело issue #545 на момент разбора (заход r1, первый
+  документ ревью для этой задачи — предыдущего раунда нет).
+- Ветка/код: не применимо (S4-spec-review, кода ещё нет).
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `dev`, коммит `ce6650b1e05a` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Дерево материала: `8aa210d76f61c0edd53530a6f88f7d98567918dd`
+  ```
+  git log --all --format='%H %T' | grep 8aa210d76f61
+  ```
+- Тело issue: `8b063fe89abbf48179a894847dff1b5c223f394fa4f6e98ec284302624b71437`
+- Вердикт конвейера: `green` · High 0