docs: review document for #178

Issue: #178
User-Visible: no

docs/reviews/SPEC-REVIEW-178-r2.md

@@ -0,0 +1,182 @@
+# SPEC-REVIEW-178-r2
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/178
+- **ТЗ:** [`docs/specs/178-toggle-entity.md`](../specs/178-toggle-entity.md)
+  (commit `76f75f85aafabcf9f698c85de0185e2ee64a98b5`, ветка `issue/178-toggle-entity`)
+- **Ревьюер:** Claude (ревью ТЗ ≠ автор), этап `S4-spec-review`, сессия без
+  контекста написания ТЗ и без контекста r1
+- **Цикл:** r2/4 (обычный трек — issue не `small`/`trivial`; метки `P1`,
+  `feature`, `S4-spec-review`)
+
+## Скоуп ревью
+
+Повторное ревью ТЗ #178 после красного вердикта r1 (High-1: отсутствовали
+обязательные разделы «Риски», «Откат», `Touch editor: …`,
+«Производительность»; Low-1: AC без инлайн-привязки к способу доказательства).
+Автор внёс правки коммитом `76f75f8` (`docs: address review of toggle entity
+spec`) — только документация, продуктовый код не менялся:
+`git diff origin/dev...HEAD --stat` показывает изменения только в
+`docs/reviews/SPEC-REVIEW-178-r1.md`, `docs/specs/178-toggle-entity.md`,
+`docs/specs/README.md`. Задача — проверить (а) что оба High/Low из r1
+действительно закрыты по существу, а не только по названию раздела, (б) что
+переразметка секций (§14→§18, вставка новых §14–17) не сломала внутренние
+ссылки и нумерацию AC↔тестов, (в) что новый текст не содержит догадок,
+выданных за факты, (г) что документ в целом снова проходит §7.1/§2.5
+целиком, а не только по пункту, который был назван в r1.
+
+Не в скоупе: реализация — её по-прежнему нет (тот же `git diff` подтверждает
+пустой `src/**`/`custom_components/**/*.py`), поэтому код-ревью не
+проводилось и не должно.
+
+## Как проверялось
+
+1. `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` целиком (§1, §2.4, §2.5, §5,
+   §7.1, §7.2, §8) — независимо от r1, в свежей сессии.
+2. Issue #178: тело, комментарий аналитики владельца, «взял в работу», «ТЗ
+   готово», вердикт r1, комментарий автора о внесённых правках.
+3. `docs/reviews/SPEC-REVIEW-178-r1.md` целиком — что именно требовалось
+   исправить и какими словами.
+4. `git diff e46ef6f..76f75f8 -- docs/specs/178-toggle-entity.md` — построчный
+   дифф, чтобы увидеть ровно то, что изменилось (только вставка §14–17,
+   переразметка §14→§18/§15→§19/§16→§20, добавление «Доказательство:» к
+   каждому пункту §19).
+5. Текущий полный текст `docs/specs/178-toggle-entity.md` (76f75f8) целиком,
+   не только новые разделы — иначе есть риск подтвердить фикс формально и не
+   заметить, что старые разделы стали противоречить новым.
+6. Перекрёстные ссылки `§N.M` по всему файлу (`grep -n "§[0-9]"`) — проверено,
+   что после переразметки не осталось ссылок на устаревшие номера разделов
+   (например, старое «§14» тестового контракта не должно указывать на новый
+   «§14 Touch»). Не осталось ни одной.
+7. `docs/TOUCH-SUPPORT.md` целиком — точная формулировка обязательной строки
+   (`Touch editor: supported/best effort/intentionally degraded/not exposed`,
+   строки 147–151) и смысл «best-effort editors» / «safety floor».
+8. `docs/CONFIG-COMPATIBILITY.md` (раздел `marker.light_entity`, строки
+   176–191) — сверка формулировок §16 «Откат» ТЗ (lossless doctrine, «старый
+   frontend стирает только при реконструкции marker») с уже принятым каноном
+   для прецедентного поля.
+9. Прецедентные ТЗ этого формата — `docs/specs/174-linked-virtual-light-controller.md`,
+   `docs/specs/164-washer-active-cycle.md`, `docs/specs/084-passive-forced-light-sources.md`,
+   `docs/specs/068-help-affordance.md` — как канон формулирует touch-строку и
+   секцию рисков/отката/performance на практике, и есть ли прецедент отдельной
+   декларации `Touch editor: supported` для одного конкретного контрола
+   внутри в целом best-effort редактора (есть, #068: «поддерживается для
+   самого affordance»).
+10. Выборочная сверка новых технических утверждений §15.1 с реальным кодом:
+    `src/devices.ts:317-325` (`ownControllableEntities`) — подтверждено, что
+    функция работает по `d.entities` одного устройства (`Array.filter`,
+    `Set`), без глобального обхода registry; заявление «линейно по числу
+    сущностей одного устройства, без global registry scan» — не догадка, а
+    факт, подтверждённый чтением кода.
+11. `docs/specs/README.md:98` — трассируемость issue↔ТЗ не нарушена правкой.
+
+Гейты (`typecheck`/`test`/`build`) не прогонялись — на этапе ревью ТЗ
+продуктового кода нет, что подтверждено пустым диффом по классам A/B
+(PROCESS.md §2.7/§8 относят гейты к код-ревью, не к ревью ТЗ).
+
+## Находки
+
+### Low-2 — раздел touch не называет явно runtime-эффект на View/kiosk
+
+**Файл:** `docs/specs/178-toggle-entity.md:336-348` (§14 «Touch и
+accessibility»).
+
+`docs/PROCESS.md` §2.5 требует «влияние на touch по `docs/TOUCH-SUPPORT.md`
+(**View и киоск — блокирующие**)». §14 разбирает только диалог устройства
+(редактор): native `<select>`, отсутствие жестов на плане, безопасность на
+узкой ширине. Но #178 меняет не только UI редактора — §10/§11 меняют, какая
+именно собственная сущность получает `homeassistant.turn_on/turn_off` при
+обычном тапе по маркеру в **View**, то есть именно тот путь, для которого
+`docs/TOUCH-SUPPORT.md` называет touch/kiosk release-blocking, а не best
+effort. Раздел не говорит явно, что резолвинг цели и вызов сервиса идентичны
+независимо от типа указателя (touch tap vs mouse click) и поэтому у View/kiosk
+нет отдельного touch-риска — этот вывод верен (executable-путь `resolveEntity`
+не различает источник события), но в документе он не сформулирован, а
+прецеденты #164 (§11) и #174 (§12) для похожих runtime-изменений явно пишут
+такую строку («Touch View и kiosk: полностью поддержаны и release-blocking»).
+
+**Воспроизведение неоднозначности:** код-ревьюер, ищущий в ТЗ ответ «есть ли
+touch-риск в View из-за этой правки», найдёт только раздел про диалог
+редактора и должен восстанавливать вывод о View самостоятельно по §10–§11,
+а не прочитать его прямо, как в прецедентах.
+
+**Вердикт:** не блокирует. Технического риска нет — резолвер целиком
+device/pointer-независим, что подтверждено чтением `src/device-toggle.ts`
+(`resolveEntity`, `resolveOwnEntity`, вызываемые из общего `_cardToggle` пути
+без учёта типа указателя). Снимаю с запиской: при реализации короткая фраза
+«View/kiosk: не затронуты, resolver и service call одинаковы независимо от
+типа указателя» в §14 закрыла бы это ощущение неполноты и избавила бы код-
+ревью от необходимости восстанавливать вывод самостоятельно.
+
+Других находок, включая High и Medium, не обнаружено.
+
+## Что проверено и корректно
+
+- **High-1 (r1) закрыт по существу, не только по названию.** Новые §14
+  (Touch и accessibility), §15.1 (Производительность), §15.2 (Security),
+  §15.3 (Риски и меры), §16 (Откат), §17 (Release-артефакты) реально
+  присутствуют, содержательны и не являются пустыми заглушками:
+  - §14 содержит буквальную канон-строку `Touch editor: supported` —
+    формат совпадает с требованием `docs/TOUCH-SUPPORT.md:147-151` и с
+    прецедентом #068 (декларация для конкретного контрола внутри в целом
+    best-effort редактора);
+  - §15.3 — таблица из 6 рисков, каждый с мерой и ссылкой на конкретный
+    тест/раздел, плюс явная оценка остаточного риска («низкий/средний»);
+  - §16 — откат без миграции, включая явный запрет автоматически
+    переписывать `toggle_entity` в другие поля при откате;
+  - §17 — release-артефакты перечислены (changelog RU+EN, USER-GUIDE,
+    CONFIG-COMPATIBILITY, golden matrix), с явной оговоркой, почему новый
+    performance budget/security report не создаются (hot path и API surface
+    не расширяются — подтверждено §15.1).
+- **Low-1 (r1) закрыт полностью.** Все 12 пунктов §19 (Acceptance criteria)
+  теперь несут инлайн **«Доказательство:»** с точными номерами
+  `unit §18.1.N` / `smoke §18.2.N` / `golden §18.3` / «code review diff» /
+  «commit trailers/process gate». Сверено вручную AC↔тест для каждого из 12
+  пунктов — расхождений или AC без реального покрывающего теста не найдено
+  (AC8, к примеру, корректно указывает на §18.1.10, а не на первый попавшийся
+  номер).
+- **Переразметка секций не сломала перекрёстные ссылки.** `grep -n "§[0-9]"`
+  по всему файлу показывает только актуальные номера (§7.1, §7.2, §8.1
+  внутри неизменных старых разделов; §15/§18.x/§18.1–3 — внутри новых/
+  переразмеченных). Ни одной ссылки на «осиротевший» номер раздела.
+- **Новые технические утверждения не являются догадками.** Проверено
+  построчно: claim §15.1 о линейности и отсутствии global registry scan
+  подтверждён чтением `ownControllableEntities()` (`src/devices.ts:317-325`)
+  — функция работает строго по `d.entities` одного устройства.
+- **Содержание разделов 1–13 не изменилось** (сверено диффом `e46ef6f..76f75f8`)
+  и уже было построчно проверено против кода в r1 (resolveOwnEntity,
+  ownRoleCandidates, ownControllableEntities, диалоговый паттерн
+  `light_entity`, backend-валидация, virtualize-allowlist) — переносить эту
+  проверку заново нет смысла, т.к. код с r1 не менялся.
+- **Трассируемость issue↔ТЗ↔review на месте**: `docs/specs/README.md:98`,
+  шапка ТЗ, комментарии issue.
+- **Продуктовых вопросов владельцу не осталось** — ни в исходном ТЗ, ни в
+  правке r1→r2 не появилось нового технического вопроса, выданного за
+  продуктовый; §20 «Принятые технические предположения» остаётся
+  корректно отделённым от нормативных решений §4.
+
+## Чего не проверял
+
+- Реализацию — её нет: `git diff origin/dev...HEAD` для `src/**` и
+  `custom_components/**/*.py` пуст (проверено).
+- Гейты `typecheck`/`test`/`build`/browser smoke — не относятся к этапу
+  ревью ТЗ; предмет будущего code-review (PROCESS.md §2.7/§8).
+- `python -m pytest tests_backend` — backend не менялся на этой ветке.
+- Golden/perf-эталоны — сценарии описаны в §18.3/§18.2, реализации для сверки
+  нет.
+- Не повторял вручную полную построчную сверку разделов 1–13 с кодом — это
+  уже сделано в SPEC-REVIEW-178-r1.md и код с тех пор не менялся (подтверждено
+  диффом); повторный полный обход добавил бы задержку без новой информации.
+
+## Вердикт
+
+Оба блокирующих/неблокирующих пункта r1 закрыты содержательно, а не
+формально: обязательные разделы существуют и информативны, AC несут точную
+привязку к доказательству, переразметка не повредила перекрёстные ссылки.
+Новых High/Medium не найдено. Единственная новая находка — Low-2 (раздел
+touch не проговаривает явно, что runtime-эффект правки в View/kiosk
+device/pointer-независим) — не блокирует: риск фактически отсутствует и
+подтверждён чтением резолвера, а сама формулировка дёшево дополняется на
+следующей правке документа без нового цикла ревью.
+
+**Вердикт: зелёный · цикл r2/4 · High: 0 · Medium: 0 → нет · Документ:
+docs/reviews/SPEC-REVIEW-178-r2.md**