From e5bda27cc395cb0560f791ee211d9e9d6d48bdcc Mon Sep 17 00:00:00 2001
From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com>
Date: Sun, 23 Aug 2026 04:00:05 +0000
Subject: [PATCH] docs: review document for #251

Issue: #251
User-Visible: no
---
 docs/reviews/SPEC-REVIEW-251-r1.md | 237 +++++++++++++++++++++++++++++
 1 file changed, 237 insertions(+)
 create mode 100644 docs/reviews/SPEC-REVIEW-251-r1.md

diff --git a/docs/reviews/SPEC-REVIEW-251-r1.md b/docs/reviews/SPEC-REVIEW-251-r1.md
new file mode 100644
index 00000000..e6b1d094
--- /dev/null
+++ b/docs/reviews/SPEC-REVIEW-251-r1.md
@@ -0,0 +1,237 @@
+# SPEC-REVIEW-251-r1
+
+- Issue: [#251](https://github.com/Matysh/houseplan-card/issues/251) — «доступность контроллера не наследуется от управляемой цели»
+- Этап: ТЗ на ревью (PROCESS.md §2.4)
+- Заход: r1 · блокирующих циклов израсходовано 0 из 4 (первый раунд, разбор полный)
+- Артефакт ТЗ: `docs/specs/251-controller-target-availability.md` (355 строк), запись в `docs/specs/README.md:130`
+- Ветка: `issue/251-controller-target-availability`
+- Ревьюер получил issue и ТЗ без устных пояснений автора; ниже — независимая проверка по коду `dev`.
+
+## Скоуп ревью
+
+Диапазон `origin/dev..HEAD` — один коммит `ba0c787` («docs(spec): separate
+controller and target availability»), трейлеры `Issue: #251` /
+`User-Visible: no` на месте, класс C (документация), правка сама по себе не
+требует `User-Visible: yes`. Изменены только `docs/specs/251-*.md` и
+`docs/specs/README.md`. Продуктовый код (`src/**`) не тронут — это ожидаемо
+для этапа ТЗ.
+
+Предмет ревью: полный текст ТЗ против §7.1 PROCESS.md (обязательные разделы),
+против диагноза в теле issue и трёх комментариев владельца/аналитика, и против
+реального кода `src/device-presentation.ts` / `src/device-toggle.ts` /
+`src/houseplan-card.ts` — верификация того, что заявленные в ТЗ факты о
+текущем поведении не догадка, а точное описание существующего кода.
+
+## Как проверялось
+
+1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` §2.4/§2.9/§7.1/§7.2,
+   `docs/USER-GUIDE.ru.md` (разделы «Жесты маркера», «Действия», «Визуальные
+   состояния устройств», §12), `docs/TOUCH-SUPPORT.md`.
+2. Прочитано тело issue #251 и все 6 комментариев (диагноз владельца,
+   актуализация аналитики, claim-комментарий, вопрос Q1, решение владельца,
+   хендофф на ревью).
+3. Гейты этапа spec:
+   - `node scripts/check-docs.mjs --external` → `Documentation checks passed
+     (7 files, 10 external links)` — green;
+   - `node scripts/process-gate.mjs --range origin/dev..HEAD --issues` →
+     `гейт пройден, предупреждений 0` — green.
+   Оба совпадают с тем, что заявил автор в хендоффе.
+4. Построчная сверка технических утверждений ТЗ с кодом:
+   - `resolvePresentationSources()` в `src/device-presentation.ts:233-369` —
+     подтверждено, что для маркера с `controls` `visualSources`/`samples`
+     строятся исключительно из `lights` (controls-derived + owned light
+     sources) и **никогда** не читают `d.entities` устройства-контроллера
+     (battery/LQI/event туда не попадают в принципе, кроме ветки `alarm` в
+     `criticalSources`). Это точное описание дефекта, не догадка.
+   - `resolveDevicePresentation()` (там же, 580-711) — `visual =
+     combineVisualSamples(sources.samples)`, класс `unavail` берётся из
+     `visual.availability === 'unavailable'` (строка 564) раньше, чем
+     `working`/`open` — подтверждает утверждение ТЗ §6.1 о приоритете
+     `unavail` над `working`.
+   - `src/device-toggle.ts`: `SkippedToggleTarget{ref, entityId, name, reason}`
+     (37-58), `toggleOperation()` (763-767) возвращает `null`, когда нет
+     `command`/`operation` — подтверждает опору AC3/AC4/AC5 на реальную,
+     уже существующую классификацию, а не на новый API.
+   - `resolveOwnEntity()`/`reasonForSingle()` (405-497) — для одиночной цели
+     `noneReason` уже мапит `missing → 'unavailable'`, `skippedTargets`
+     сохраняет причину и имя — ровно то, что нужно для локализованного
+     сообщения без нового запроса к HA (заявление аналитика подтверждено).
+   - `_clickDevice()` в `src/houseplan-card.ts:4930-4979` — строка 4932,
+     `if (!initial || !toggleOperation(initial)) return; // ... quiet no-op`
+     — это именно тот тихий `return`, который ТЗ называет дефектом второй
+     части (п.3 «Актуализации аналитики»). Подтверждено чтением, не
+     исполнением.
+   - Токст toasta: `src/houseplan-card.ts:16473,17069` —
+     `<div class="toast" role="alert" aria-live="assertive">` — ТЗ §7.1/§8
+     точно описывает существующую разметку (после первичной проверки я
+     нашёл в коде другой, необязательный `role="status" aria-live="polite"`
+     у `_renderRecoveryOverlay()`, 3389-3399 — это другой компонент,
+     оверлей восстановления связи, не toast; ложная тревога снята).
+   - `confirm.unavailable_targets` (`src/i18n/ru.json:722`,
+     `src/houseplan-card.ts:19805`) — существующий паттерн «Недоступно:
+     {count}» без словоформ; новый текст ТЗ («Цели недоступны: {names}»)
+     грамматически не зависит от числа и не наследует эту проблему —
+     формулировка ТЗ безопасна для RU-множественного числа.
+   - `isControllable()` (`src/logic.ts:1657-1658`) ограничивает прямые
+     ссылки `controls` доменами `light`/`switch`; `secureEntity()`
+     (`device-toggle.ts:209-218`) относится к `lock`/`alarm_control_panel`/
+     охраняемым классам `cover` — по коду смешение `secure` с
+     `unavailable/missing` в одной группе `controls` технически достижимо
+     только через `marker:`-алиас на **другой** маркер, помеченный
+     `is_light: true` (`_controlCandidates`, `houseplan-card.ts:19975-19992`
+     фильтрует кандидатов именно так) — узкий, нетипичный путь конфигурации.
+   - Виртуальные маркеры (`devices.ts:1246-1265`) всегда получают
+     `entities: []` — значит строка матрицы §6.1 «virtual controller без
+     own entities» — это не частный случай, а единственно возможный для
+     `virtual: true`; formulировка §5 «виртуальные контроллеры... как
+     всегда доступные» не создаёт скрытого противоречия с матрицей.
+   - `demo/smoke_controls.mjs`, `test/device-presentation.test.mjs`,
+     `test/device-toggle.test.mjs` существуют — ссылки AC1/AC3/AC4 на них
+     не повисают в воздухе.
+5. Сверка терминологии с `docs/USER-GUIDE.ru.md`: текущая документированная
+   формулировка «Если все сущности, реально описывающие маркер, недоступны,
+   маркер бледнеет» (строка 855-857) и «Бледный маркер | Данные недоступны...
+   | Все рабочие сущности маркера unknown, unavailable или отсутствуют»
+   (871-875) — это ровно контракт, который ТЗ №251 сознательно меняет
+   (разделяет «рабочие сущности» на own-controller и target). ТЗ п.14
+   («Release-артефакты») правильно требует правку `USER-GUIDE.md`/`.ru.md` в
+   том же коммите реализации — иначе документация и код разойдутся, как уже
+   было отмечено process-риском в самом ТЗ (риск 6).
+
+## Находки
+
+Обе найденные точки — низкой серьёзности; ни одна не блокирует и не
+требует возврата автору. Записываю решение ревьюера по каждой (PROCESS.md
+§3.8: Low либо правится, либо снимается решением ревьюера с записью).
+
+### L1 — не проговорено соответствие toast'а формулировке владельца «стандартными средствами HA»
+
+Первый комментарий владельца в issue: «...выводить корректное сообщение...
+**стандартными средствами HA** (как пишется, что соединение с HA потеряно)».
+ТЗ §7.1/§8 отвечает на это локальным тостом самой карточки House Plan
+(`role="alert" aria-live="assertive"`), а не нативным механизмом HA
+(например, событием `hass-notification`, которым сама HA показывает баннер
+о потере соединения). ТЗ не проговаривает это расхождение и не отмечает
+выбор как намеренную интерпретацию.
+
+**Почему не Medium:** в кодовой базе уже существует устоявшийся паттерн —
+все аналогичные «действие не выполнено» сообщения (`toast.error`,
+`toast.tap_target_changed`, `toast.no_entity`, `toast.ha_disabled_action`,
+`toast.run_target_missing`) используют именно локальный тост карточки, а не
+нативный HA-механизм. Новое сообщение, реализованное иначе, было бы
+единственным исключением и визуально разошлось бы с остальной картой.
+Фраза владельца «как пишется, что соединение с HA потеряно» по контексту
+описывает регистр/тон сообщения («это системное объяснение, а не тихая
+ошибка»), а не требует буквально вызвать HA-нативный баннер; последующие
+три коммента владельца (Q1 и решение) не возражают против уже
+предложенного тостового решения аналитика.
+
+**Решение ревьюера:** снимаю как Low. Прошу автора добавить в §7.1 или §17
+одну строку, явно фиксирующую, что «стандартные средства HA» реализуются
+локальным тостом карточки по образцу уже существующих `toast.*`-сообщений —
+это превратит implicit-решение в явно записанное предположение и закроет
+дорогу к спору на код-ревью.
+
+### L2 — не решён смешанный `secure` + `unavailable` случай в группе `controls`
+
+§7.1 формулирует условие тоста как «все пропуски объясняются `unavailable`,
+`missing` или `ha-disabled`» и отдельно говорит, что no-op из-за `secure`
+сохраняет существующее (тихое) поведение. Не описан случай, когда в одной
+группе `controls` одновременно есть и `secure`-цель, и `unavailable`/
+`missing`-цели, при нулевом исполняемом множестве: он не подходит под
+условие «все пропуски объясняются unavailable/missing/ha-disabled» (раз
+среди них есть secure), но и не подходит под «no-op вызван secure»
+(раз среди них есть не-secure причины). Естественная реализация на базе
+существующего `noneReason` (`device-toggle.ts:536-537`, который различает
+только `'secure'` при **всех** secure и `'configured-targets-missing'` во
+всех остальных случаях) молча покажет тост и для этого смешанного случая —
+поведение, о котором ТЗ прямо не высказывается.
+
+**Почему не Medium:** по коду смешение достижимо только через `marker:`-
+алиас на другой маркер с `is_light: true`, а не через прямую ссылку на
+`lock.*`/`alarm_control_panel.*` (те отфильтрованы `isControllable()` уже на
+этапе выбора в редакторе). Обычный охраняемый `cover` (garage/door/gate)
+почти никогда не выставляется как `is_light: true`, так что путь требует
+нетипичной ручной конфигурации. Независимо от исхода замок/охранная панель
+не получает command — инвариант `SCOPE.md` («никогда не по тапу на плане»)
+не затрагивается ни при каком прочтении.
+
+**Решение ревьюера:** снимаю как Low. Достаточно одной уточняющей строки в
+§7.1 или §16 (риски): смешанная группа classифицируется так же, как
+«все пропуски unavailable/missing/ha-disabled» — тост показывается, имена
+secure-целей в тексте тоста не участвуют отдельным исключением. Не блокирует
+разработку; можно поправить в рамках реализации без повторного цикла ревью
+ТЗ.
+
+## Что проверено и корректно
+
+- Обязательные разделы §7.1 PROCESS.md присутствуют и в правильном порядке:
+  сценарий/персона (§1) → что человек видит до/после (§2) → диагноз (§3) →
+  решение владельца (§4) → скоуп/не-скоуп (§5) → контракт presentation (§6) →
+  контракт действия (§7) → UX/a11y/touch (§8) → данные/compat/migration (§9) →
+  perf/security (§10) → AC1-AC9 с доказательством (§11) → план реализации и
+  автотестов (§12) → mutation guards (§13) → release-артефакты (§14) →
+  откат (§15) → риски (§16) → явный блок принятых технических предположений
+  (§17).
+- Продуктовые разделы 1-2 отвечают на «какая персона/поверхность/момент» и
+  «что человек видит одной фразой без терминов реализации» — оба
+  недвусмысленны.
+- Единственный продуктовый вопрос (Q1 — как считать доступность самого
+  контроллера при отсутствии device-level state в HA) задан владельцу пачкой,
+  с предлагаемым default, `blocked` был выставлен и снят после ответа —
+  процесс §2.3/§7.1 соблюдён буквально. «Продуктовых вопросов не осталось»
+  (§4 ТЗ) подтверждается тем, что все решения §4 действительно восходят к
+  цитируемым словам владельца, а не к догадке автора.
+- Матрица §6.1 однозначна и проверяема: каждая строка задаёт конкретный вход
+  (состояния own/target) и конкретный выход (availability/status/класс),
+  привязана к AC1 и к перечисленному mutation guard
+  (`controller-availability-follows-target`,
+  `controller-diagnostics-do-not-prove-online`).
+- Скоуп/не-скоуп (§5) взаимно непротиворечивы и совпадают с решением
+  владельца (нет нового бейджа, Glow не меняется, групповая семантика «any on
+  → all off» не меняется, secure-инвариант не трогается).
+- Модель данных/миграция (§9) — корректно «нет миграции»: правка чисто
+  presentation/action, персистентная схема `marker.controls` не меняется,
+  подтверждено тем, что все затронутые функции читают, а не переписывают
+  `d.entities`/`d.controls`.
+- Все AC (1-9) пронумерованы и у каждого указан способ доказательства
+  (`unit`/`smoke`/`golden`/«ревью кода»/config diff) — требование DoR §2.5
+  выполнено.
+- i18n: точный RU/EN текст обеих реплик дан в §7.1; именование конкретных
+  ключей оставлено автору как техническая деталь (не наблюдается
+  пользователем) — это в рамках §7.1 PROCESS.md («именование... агенты
+  решают сами»), не дефект готовности к разработке.
+- Touch/a11y (§8): не вводит новых жестов, использует существующий
+  keyboard/tap путь и объявление screen reader — согласуется с
+  `TOUCH-SUPPORT.md` (View/kiosk — гарантированная touch-поверхность) и не
+  создаёт editor-специфичного расхождения.
+- Откат (§15) реалистичен: одна code revision, без обратной миграции данных.
+- Release-артефакты (§14) перечисляют оба changelog, `ARCHITECTURE.md`,
+  оба `USER-GUIDE`, `TESTING.md` — учитывая, что USER-GUIDE.ru.md сейчас
+  прямо документирует старый (заменяемый) контракт «Бледный маркер», это
+  необходимое требование, а не избыточное.
+- Гейты spec-этапа (`check-docs`, `process-gate`) перезапущены ревьюером
+  независимо и совпали с заявленными автором результатами.
+
+## Чего не проверял
+
+- Не запускал `npm run typecheck` / `npm test` / `npm run build` — диапазон
+  `origin/dev..HEAD` не содержит изменений `src/**`/`test/**` (только
+  `docs/specs/**`), эти гейты на этапе ТЗ не применимы к нулевому
+  код-диффу; они станут обязательными в код-ревью реализации.
+- Не проверял `demo/smoke_controls.mjs` и `test/device-presentation.test.mjs`
+  / `test/device-toggle.test.mjs` на способность падать под будущими
+  мутантами — на этапе ТЗ кода мутантов ещё нет; это станет предметом
+  код-ревью (раздел §13 «Mutation guards» ТЗ уже задаёт, что именно должно
+  покраснеть).
+- Не проверял golden/скриншот-провенанс — на этапе ТЗ `src/**` не менялся,
+  `check-docs.mjs --external` уже подтвердил, что текущий provenance зелёный.
+- Не оценивал реальную Zigbee/HA-инсталляцию из исходного репорта (батарея
+  100, LQI 164, группа из четырёх лампочек) — это внешний живой стенд
+  владельца, недоступный ревьюеру; диагноз проверен по коду, а не повторным
+  воспроизведением на инсталляции.
+
+## Вердикт
+
+High: 0 · Medium: 0 (два Low найдены и сняты решением ревьюера с записью
+выше, без возврата автору) → **зелёный**. ТЗ готово к разработке.