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

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; все три либо правятся косметически при следующей редакции, либо снимаются этой записью без нового цикла.