docs: add spec review r1 for toggle confirmation state

Issue: #103
User-Visible: no

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

@@ -0,0 +1,257 @@
+# SPEC-REVIEW-103-r1
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/103
+- **ТЗ под ревью:** `docs/specs/103-toggle-confirmation-state.md` (коммит
+  `49c0c3d`, ветка `issue/103-toggle-confirmation-state`)
+- **Роль:** ревьюер ТЗ (не автор), этап `S4-spec-review`
+- **Трек:** обычный (не `small`/`trivial`) — согласно аналитике владельца от
+  2026-08-14: сложность 2/10, риск 3/10, но `trivial` намеренно не применён,
+  так как confirmation copy — UX-контракт, потенциально затрагивающий
+  несколько доменов (power/cover/valve/group/virtual light)
+- **Цикл:** r1/4
+
+## Скоуп ревью
+
+Проверялось соответствие ТЗ:
+
+- `docs/SCOPE.md` — попадание задачи в Core user jobs, отсутствие расширения
+  скоупа и конфликта с «lock invariant»;
+- `PROCESS.md` §2.4, §2.5 (DoR), §7.1 (обязательные разделы), §3/§12
+  (запреты), §5 (легкий трек — не применяется, но проверено, что признаки
+  `small` действительно отсутствуют);
+- `AGENTS.md` — классы файлов коммита, ветка, трейлеры;
+- фактическому состоянию кода `src/device-toggle.ts` и `src/houseplan-card.ts`
+  — технические утверждения ТЗ о существующем резолвере #94
+  (`ResolvedToggleIntent`, `formatToggleIntent`, `sameToggleOperationTargets`,
+  `_tapConfirm`) сверены построчно, чтобы отличить факт от догадки;
+- `docs/USER-GUIDE.ru.md` — терминология («Переключить состояние», hint под
+  селектором действия, `tap_confirm`);
+- `docs/TOUCH-SUPPORT.md` — категория «View dialogs and safe device actions»
+  (fully supported на touch, не best-effort).
+
+## Как проверялось
+
+1. Прочитан весь тред issue #103: исходное тело (уже содержит нормативную
+   матрицу, i18n-ключи и race-контракт от владельца), комментарий аналитики
+   2026-08-15 (`P3`, сложность 2/10, риск 3/10, «вопросов нет»), комментарий
+   автора ТЗ («продуктовых вопросов нет, метка остаётся `S3-spec` по поручению
+   владельца» — на момент ревью метка уже `S4-spec-review`, расхождение не
+   процессное: `docs/specs/README.md` подтверждает коммит и ветку).
+2. Сверены обязательные разделы ТЗ (§7.1 PROCESS.md) построчно — таблица ниже.
+3. Прочитан `src/device-toggle.ts` целиком: подтверждено существование
+   `ResolvedToggleIntent`, `ToggleNextEffect`, `formatToggleIntent()`,
+   `sameToggleOperationTargets()`, `toggleOperation()` — ровно тот резолвер
+   #94, на который ссылается ТЗ, а не придуманный интерфейс.
+4. Прочитан `src/houseplan-card.ts` (окрестности `_clickDevice`, `:4336-4400`
+   и `:14838-14846`): подтверждено дословно, что текущий `_tapConfirm` —
+   `{ text: string; exec: () => void }`, а `text` строится только как
+   `this._t('confirm.tap_toggle', { name })` — то есть претензия ТЗ «диалог
+   показывает только имя» верна для текущего кода, не выдумана.
+5. Прочитан `_toggleHintLines()` (`:17314-17358`) и соответствующие ключи
+   `marker.toggle_hint_current` / `marker.toggle_effect_*` /
+   `marker.toggle_hint_group_current` / `marker.toggle_hint_skipped` в
+   `src/i18n/ru.json` и `en.json`. Эта существующая инфраструктура уже строит
+   «текущее → эффект» для редакторского hint под селектором действия
+   (`docs/USER-GUIDE.ru.md:489`). ТЗ не игнорирует её: §5 явно допускает
+   расширение `formatToggleIntent()` либо добавление соседней pure-функции, а
+   разделение current/expected на отдельные строки в §8 обосновано
+   собственным accessibility-требованием (порядок чтения screen reader'ом),
+   которого нет у однострочного hint. Дублирования семантики без причины не
+   обнаружено.
+6. Проверена логика `coverLikeService()` (`:307-331`) против нормативной
+   матрицы §6 ТЗ — см. Low-3.
+7. Прочитан `test/device-toggle.test.mjs` и список `demo/smoke_*.mjs`:
+   подтверждено существование `smoke_ha_controls.mjs`, `smoke_controls.mjs`,
+   `smoke_virtual_light_toggle.mjs`, на которые ссылается план регрессии §11 —
+   не выдуманные имена.
+8. Прочитан `docs/TOUCH-SUPPORT.md`: строка «View dialogs and safe device
+   actions» — «Fully supported» на touch (не best-effort), то есть touch-
+   влияние здесь блокирующее по DoR; ТЗ §8 адресует это узкой mobile-footer и
+   narrow-viewport smoke — соответствует канону.
+9. Прочитан `docs/USER-GUIDE.ru.md:489` и «Краткая памятка безопасности»
+   (:1206): термин «Переключить состояние» и рекомендация включать
+   подтверждение для Toggle/Run/Cover совпадают с ТЗ дословно.
+10. Проверена трассируемость: `docs/specs/README.md` получил секцию `## P3`
+    со строкой на #103 в том же коммите; `git diff --stat origin/dev...HEAD`
+    показывает только два файла класса C; коммит `49c0c3d` несёт
+    `Issue: #103` и `User-Visible: no` — корректно для документа ТЗ, который
+    сам не меняет поведение.
+
+## Обязательные разделы (§7.1 PROCESS.md)
+
+| Раздел | Есть | Комментарий |
+|---|---|---|
+| Сценарий (персона/поверхность/момент) | Частично | §1 объясняет проблему технически (только имя в диалоге), но ни разу не называет персону/поверхность явной фразой — см. Low-1 |
+| Что человек увидит до/после | ✅ | §2, буквально в виде текста диалога «до/после» |
+| Проблема | ✅ | §1 |
+| Скоуп / не-скоуп | ✅ | §3 / §4 |
+| Контракт поведения | ✅ | §5, §6, §9 |
+| Модель данных и миграция | ✅ | §7 (TS-интерфейс `_tapConfirm`) + §9 («config не меняются», миграции нет) |
+| UX и i18n | ✅ | §8 |
+| AC1…ACn с доказательством | Частично | §10 — 10 пронумерованных AC, но без построчной метки типа доказательства; тип восстанавливается однозначно из §11 (см. Low-2) |
+| План автотестов | ✅ | §11, разбит на unit/integration/регрессию |
+| Риски | ✅ | §14 |
+| Откат | ✅ | §14 (последний абзац) |
+| Release-артефакты | ✅ | §13 |
+
+Дополнительно есть раздел «Принятые технические предположения» (§15) —
+не требуется §7.1 буквально, но прямо соответствует духу PROCESS.md §7.1 о
+записи технических решений отдельным блоком.
+
+## Находки
+
+Находок уровня **High** и **Medium** нет — отдельные issue заводить не
+требуется.
+
+### Low-1 — сценарий не называет персону/поверхность явной фразой
+
+**Файл:** `docs/specs/103-toggle-confirmation-state.md` (§1)
+
+AGENTS.md требует, чтобы первый раздел ТЗ отвечал: какая персона (из
+`docs/SCOPE.md`), на какой поверхности, в какой момент встретит изменение.
+§1 объясняет техническую проблему («текущий диалог показывает только имя»),
+но не пишет explicit: диалог tap-confirm — это «View dialogs and safe device
+actions» (`TOUCH-SUPPORT.md`), доступный любой персоне на любой поверхности
+(desktop/kiosk/companion app) в момент нажатия на маркер с `tap_confirm`.
+Смысл восстанавливается из §2 (пример текста диалога) и общего контекста
+J3 `SCOPE.md`, поэтому неоднозначности для читателя нет, но формальное
+требование выполнено не буквально.
+
+**Решение ревьюера:** Low, не блокирует. Косметическая правка одним
+предложением на усмотрение автора при следующей редакции.
+
+### Low-2 — AC1–AC10 не промаркированы построчно типом доказательства
+
+**Файл:** `docs/specs/103-toggle-confirmation-state.md` (§10, AC1–AC10)
+
+DoR (`PROCESS.md` §2.5) и цепочка §7.1 требуют, чтобы «у каждого [AC] указано,
+чем он доказывается: unit / backend / smoke / golden / ревью кода». §10
+перечисляет десять проверяемых критериев, но ни один не несёт явной метки
+доказательства — план тестирования (§11) даёт эти метки только группами
+(Unit / Integration-browser / Регрессия), и сопоставление 1:1 нужно
+восстанавливать вручную. Я проверил это сопоставление вручную: AC1–AC6
+однозначно закрываются перечисленными в §11 unit-кейсами (formatter per
+`ToggleNextEffect`, single/group/partial/unknown/no-operation), AC7–AC8 —
+race-кейсами из Integration/browser, AC9 — DOM order/narrow viewport/keyboard
+smoke, AC10 — «run confirmation unchanged» той же секции. Ни один AC не
+остаётся недоказуемым по существу; отсутствует только явная метка в самом
+тексте §10. Тот же класс находки (типы доказательства AC вне буквального
+формата) уже фиксировался как Low и не блокировал приёмку в
+`SPEC-REVIEW-137-r1` (Low-3).
+
+**Решение ревьюера:** Low, не блокирует. Рекомендуется при следующей правке
+приписать к каждому AC1–AC10 короткую метку (`unit`/`integration`/`review
+кода`), чтобы код-ревью могло сверяться механически, а не восстанавливать
+сопоставление заново — но проверяемость критериев уже подтверждена этим
+документом.
+
+### Low-3 — нормативная матрица §6 группирует cover-состояния не так, как резолвер
+
+**Файл:** `docs/specs/103-toggle-confirmation-state.md:78-79` (§6, строки
+«cover `closed/closing` + `open`» и «cover + `stop`»)
+**Код:** `src/device-toggle.ts:307-331` (`coverLikeService`)
+
+Таблица группирует «closed/closing» в одну строку с ожидаемым `open`. Но по
+факту резолвера состояние `closed` действительно даёт `nextEffect: 'open'`,
+а `closing` — отдельную ветку `nextEffect: 'stop'` (наравне с `opening`),
+то есть реальная комбинация «`closing` → next `open`» в резолвере никогда не
+возникает: она относится к строке «cover + `stop`» этой же таблицы. Так как
+формула §5 явно требует брать current-строку из HA-formatted state, а не
+пересчитывать её из этой таблицы, и направление всегда берётся из
+`nextEffect` резолвера (а не выводится реализацией по названию строки), риска
+для рантайма нет — но группировка вводит в заблуждение при написании unit-
+теста по этой таблице буквально (можно ошибочно закодировать несуществующую
+комбинацию `state=closing + effect=open` как «нормальный» кейс вместо
+`state=closing + effect=stop`).
+
+**Решение ревьюера:** Low, не блокирует. При следующей редакции стоит убрать
+`closing` из строки `open` (оставить только `closed`) и явно отметить, что
+строка `stop` покрывает оба переходных состояния (`opening`/`closing`) — но
+это не меняет ни один AC и не требует возврата цикла.
+
+## Что проверено и корректно
+
+- Соответствие `docs/SCOPE.md`: задача уточняет содержимое существующего
+  Toggle-confirmation guard'а — это J3 («Let me act on the obvious right from
+  the plan», guarded quick actions), статус **Closed**; расширения продукта
+  нет, задача улучшает уже принятую функциональность и не создаёт нового
+  actuation-пути. Явно проверено, что «lock invariant» `SCOPE.md` не задет:
+  secure-цели (`lock.*`, `alarm_control_panel.*`, guarded cover) уже
+  фильтруются резолвером на уровне `secureEntity()` и никогда не попадают в
+  `targets`, следовательно новый confirmation-текст не может *обещать*
+  переключение замка.
+- Легитимность полного трека: несмотря на низкую оценку сложности (2/10) от
+  владельца, автор корректно не применил `trivial`/`small` — confirmation
+  copy реально затрагивает пять разных семантик (power/cover/valve/group/
+  virtual light) и i18n; критерий «одна поверхность» (§5 PROCESS.md) не
+  выполняется буквально, обычный трек оправдан.
+- Продуктовых вопросов владельцу нет, и это корректно: почти вся нормативная
+  матрица уже была продиктована владельцем в теле issue до написания ТЗ,
+  автор её только формализовал и уточнил (см. Low-3 как единственное реальное
+  уточнение, не вопрос). Ни одна догадка не выдана за факт без пометки —
+  §15 отдельно фиксирует три технических предположения (no live-update
+  snapshot, HA-formatter приоритет, `stop` как честный ожидаемый эффект),
+  все они соответствуют либо явному тексту issue, либо существующему коду.
+- Технические утверждения о текущем коде подтверждены чтением, а не
+  голословны: `ResolvedToggleIntent`/`formatToggleIntent`/
+  `sameToggleOperationTargets` (#94, `src/device-toggle.ts`), текущий
+  однострочный `_tapConfirm.text` (`src/houseplan-card.ts:4388-4389`),
+  существующий toast `toast.tap_target_changed`, существующие тесты
+  `test/device-toggle.test.mjs` и смоки `smoke_ha_controls.mjs` /
+  `smoke_controls.mjs` / `smoke_virtual_light_toggle.mjs`.
+- Не-скоуп (§4) корректно отсекает смежные соблазны: изменение resolver/
+  target-selection/service-call, прогноз scripts/scenes, история состояний,
+  redesign confirmation-диалогов удаления/unlock/run, live-анимация в
+  открытом диалоге, изменение схемы `tap_confirm` — типичные места, где скоуп
+  мог бы незаметно расшириться, явно исключены.
+- Touch-контракт: диалог tap-confirm — категория «View dialogs and safe
+  device actions» (`TOUCH-SUPPORT.md`), полностью поддерживаемая на touch
+  (не best-effort). ТЗ §8 адресует это явно (narrow footer с двумя кнопками,
+  перенос длинного текста без horizontal scroll) и §11 добавляет narrow-
+  viewport smoke — соответствует блокирующей категории канона.
+- Совместимость: `stored tap_confirm` и схема config не меняются, миграции
+  нет — корректно для confirmation copy, не меняющей контракт хранения.
+  Откат (§14) корректно опирается на то, что `_tapConfirm` возвращается к
+  одной строке без отдельного data rollback.
+- i18n: ключи перечислены раздельно для RU/EN в одном release-коммите (§13);
+  plural rules осознанно не вводятся, что соответствует «counter-safe style»
+  уже принятому в существующих `marker.toggle_*` строках.
+- Release-артефакты (§13) корректно относят golden/screenshot к точечному
+  необязательному кадру, а backend/migration/performance/security — явно
+  «не требуются», что соответствует характеру изменения (чистый UI-текст без
+  новых сервис-вызовов).
+- Трассируемость: `docs/specs/README.md` обновлён тем же коммитом (новая
+  секция `## P3` со строкой на #103), ссылка issue ↔ ТЗ двусторонняя; ветка
+  `issue/103-toggle-confirmation-state` и трейлеры коммита (`Issue: #103`,
+  `User-Visible: no`) корректны для документа класса C, который сам не
+  меняет поведение.
+- `git diff --stat origin/dev...HEAD` не содержит ни одного файла класса A —
+  код не тронут до `S5-ready`, что соответствует правилу №1.
+
+## Чего не проверял
+
+- Не проверял реализуемость `_tapConfirm` как размеченного union
+  (`{kind:'toggle', ...} | {kind:'run', ...}`) на уровне TypeScript-типов —
+  это по правилам ТЗ свободно изменяемое техническое решение автора кода
+  (§15), не предмет ревью ТЗ.
+- Не запускал автотесты, build или browser-смоки — на этапе `spec` это не
+  требуется; существование резолвера, hint-инфраструктуры, i18n-ключей и
+  тестовых/смоук-файлов, на которые ссылается ТЗ, проверено чтением
+  исходников, а не исполнением.
+- Не проверял корректность численных оценок аналитики (ценность 5/10 и 3/10,
+  сложность 2/10, риск 3/10, P3) по существу — это поле владельца
+  (PROCESS.md §2.2), уже принятое явным решением до написания ТЗ.
+- Не проверял, как именно `hass.formatEntityState` поведёт себя для каждого
+  конкретного домена/state на реальном HA — ТЗ корректно требует safe
+  fallback на raw state, и это уже проверяемый unit-кейс по плану §11, а не
+  вопрос ревью ТЗ.
+
+## Вердикт
+
+Зелёный. High: 0, Medium: 0. Три находки Low (сценарий не называет
+персону/поверхность явной фразой; AC1–AC10 не промаркированы построчно типом
+доказательства, хотя проверяемость подтверждена этим документом; нормативная
+матрица §6 вводит в заблуждение группировкой cover `closed/closing` в строке
+`open`, хотя резолвер направляет `closing` в строку `stop`) — ни одна не
+блокирует приёмку и не меняет ни один AC; все три либо правятся косметически
+при следующей редакции, либо снимаются этой записью без нового цикла.