houseplan-card/docs/reviews/SPEC-REVIEW-251-r1.md

SPEC-REVIEW-251-r1

• Issue: #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 найдены и сняты решением ревьюера с записью выше, без возврата автору) → зелёный. ТЗ готово к разработке.