docs: review document for #381

Issue: #381
User-Visible: no

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

@@ -0,0 +1,223 @@
+# SPEC-REVIEW-381-r1
+
+- Issue: https://github.com/Matysh/houseplan-card/issues/381 — «Действие по
+  нажатию: добавить "Ничего не делать"»
+- Этап: ТЗ на ревью (PROCESS.md §2.4), **полный трек** (владелец: нарушен
+  критерий §5 «нет нового UX-контракта» — новое наблюдаемое действие и новое
+  сохраняемое значение публичной конфигурации)
+- ТЗ: `docs/specs/381-no-op-tap-action.md`
+- Материал ревью: ТЗ на ветке `issue/381-no-op-tap-action`, ревизия автора
+  `7a260e8f`; `git diff --stat origin/dev...HEAD` подтверждает, что изменены
+  только `docs/specs/381-no-op-tap-action.md` и `docs/specs/README.md` —
+  продуктовый код не тронут (закономерно для этапа spec)
+- Заход: r1 · блокирующих циклов израсходовано **0 из 4** до этого вердикта
+
+## Скоуп ревью
+
+Первый заход — предыдущего вердикта нет, разбор полный. Проверялось:
+
+- соответствие `docs/SCOPE.md` (job, персона, поверхность);
+- корректность выбора трека (полный, не `small`/`trivial`) по критериям §5;
+- наличие всех обязательных разделов ТЗ по §7.1, включая «риски» и «откат»;
+- однозначность и доказуемость каждого AC1…AC9, указание способа
+  доказательства;
+- отсутствие догадок, выданных за факт: каждое утверждение ТЗ о текущем
+  поведении кода сверено с исходниками, а не принято на слово;
+- согласованность терминологии с `docs/USER-GUIDE.ru.md` и `TOUCH-SUPPORT.md`;
+- полнота списка «Затронутые файлы и модули» — не пропущен ли реальный
+  потребитель `tap_action`.
+
+## Как проверялось
+
+1. Прочитаны `docs/SCOPE.md`, `PROCESS.md` (§2, §4, §5, §7, §8), `AGENTS.md`.
+2. Прочитано тело issue #381 и все три комментария (аналитика, claim,
+   публикация ТЗ) через `gh issue view --json`.
+3. Прочитан ТЗ `docs/specs/381-no-op-tap-action.md` целиком.
+4. Каждое фактическое утверждение ТЗ о текущем коде сверено с `dev`:
+   - `src/logic.ts:910` — `TAP_ACTIONS = ['info', 'more-info', 'toggle',
+     'run']` — подтверждено, ровно четыре текущих действия.
+   - `src/device-toggle.ts:463-475` (`projectedTapAction`) — подтверждено:
+     любой нераспознанный непустой токен возвращает `'info'` («fail closed»),
+     проекция света по умолчанию срабатывает только при реальном отсутствии
+     значения.
+   - `src/houseplan-card.ts:5187-5313` (`_clickDevice`) — подтверждено: после
+     веток `toggle`/`info`/`run`/`more-info` функция заканчивается безусловным
+     `this._infoCard = actionDevice;` — реальный fallback на локальную
+     карточку, который контракт §2.2 обязан обойти до него, а не после
+     (проверен соответствующий пункт мутантов).
+   - `src/houseplan-card.ts:5325-5330` (`_keyDevice`) — подтверждено:
+     `Enter`/`Space` вызывают тот же `_clickDevice()` после `preventDefault()`.
+   - `src/houseplan-card.ts:5178-5185` (`_ctxDevice`) и long-press на 600 мс
+     (`:6360-6364`) — подтверждено: оба жеста не читают `tap_action` вообще,
+     независимы от выбора действия, как заявляет контракт §3.
+   - `src/houseplan-editor-runtime.ts:12346` — маппинг селектора
+     `TAP_ACTIONS.map((v) => [v, 'tap.' + v.replace('-', '_')])` — подтверждено,
+     что `none` автоматически получит ключ `tap.none` без спецкода.
+   - `src/houseplan-editor-runtime.ts:12337,12352,12388,12394,12423` —
+     подтверждено: Toggle-hint, target chooser и confirmation-чекбокс скрыты
+     условием `effectiveTapAction === 'toggle'/'run'`; для `none` они погаснут
+     автоматически, без новой ветки видимости.
+   - `src/houseplan-editor-runtime.ts:7934-7940` (`_markerTapActionFields`) —
+     подтверждено: untouched-путь (`!tapActionTouched`) пишет
+     `originalTapAction` как есть, включая legacy/неизвестный литерал —
+     lossless-контракт AC5 уже существует для этого поля, задача его не ломает.
+   - `src/houseplan-editor-runtime.ts:8034` — `tap_target` пишется `null` для
+     любого `effectiveTapAction !== 'run'` — подтверждает пункт «Принятых
+     предположений» про очистку `tap_target`.
+   - `custom_components/houseplan/validation.py:1693` — `MARKER_SCHEMA`
+     принимает `vol.Any("info", "more-info", "toggle", "run", "cover", None)` —
+     подтверждено, backend действительно отклонит седьмой (`none`) без
+     schema-правки.
+   - `custom_components/houseplan/import_export.py:1386` — подтверждён список
+     полей, которые virtualization дубликата стирает вместе с `tap_action`.
+   - `src/device-presentation.ts:264-338` — подтверждено, что визуальная роль
+     cover-устройства (`coverOwnsFace`) не зависит от выбора `none`: код уже
+     форсирует `tapAction: 'cover'` для presentation independent от
+     персистентного действия, поэтому AC4 (presentation parity) не требует
+     здесь правок.
+   - `src/i18n/{en,ru,de,fr}.json` — подтверждено, что это ровно четыре
+     существующих словаря (пятого/шестого локале нет) и что паттерн ключей
+     `tap.<action>` уже используется (`tap.info`, `tap.toggle`, `tap.run`).
+   - `tests_backend/test_validation.py` — подтверждено существование
+     cross-language parity теста `TAP_ACTIONS`, на который ссылается план
+     автотестов (это не выдуманная будущая инфраструктура).
+5. Сверена терминология: `docs/USER-GUIDE.ru.md:812` подтверждает, что «Действие
+   по нажатию» — существующее название поля редактора, не изобретённое ТЗ.
+6. `docs/TOUCH-SUPPORT.md` — подтверждено, что View/kiosk — гарантированно
+   touch-поверхность; ТЗ явно требует паритет mouse/touch/keyboard в AC2, что
+   закрывает блокирующее требование §2.5 без отдельного раздела «Touch».
+7. Найден прямой прецедент калибровки строгости в этом же репозитории:
+   `docs/reviews/SPEC-REVIEW-376-r1.md` (M1 «раздел откат отсутствует
+   полностью» — классифицирован как **Medium**, не High) и
+   `docs/reviews/SPEC-REVIEW-377-r1.md` (тот же тип находки, вердикт
+   `жёлтый · High: 0 · Medium: 1`). Severity ниже откалибрована по этому
+   прецеденту, а не назначена заново.
+8. Сравнены с шаблоном двух последних full-track ТЗ этого же автора —
+   `docs/specs/377-decor-default-persist.md` и `docs/specs/378-value-face-source.md`
+   — оба содержат отдельные разделы `## Риски` и `## Откат`; это подтверждает,
+   что раздел ожидается предметно, а не просто присутствует в §7.1 формально.
+
+Гейты (`tsc`/`test`/`build`/`invariants`/smoke/golden) не прогонялись: этап —
+ревью ТЗ, продуктовый код в этой ветке не менялся (см. `git diff --stat` выше).
+Они относятся к этапу код-ревью (§2.7) и будут актуальны в следующем цикле.
+
+## Находки
+
+### M1 (Medium, в скоупе) — отсутствуют обязательные разделы «Риски» и «Откат» (§7.1)
+
+`docs/specs/381-no-op-tap-action.md` не содержит `## Риски` и `## Откат`.
+Оба раздела прямо названы обязательными в PROCESS.md §7.1 («Обязательные
+разделы ТЗ: … план автотестов · **риски** · **откат** · release-артефакты») и
+входят в чек-лист DoR §2.5 («риски перечислены», «откат: как выключить или
+вернуть назад»). Оба непосредственно предшествующих full-track ТЗ этого же
+автора — `docs/specs/377-decor-default-persist.md` и
+`docs/specs/378-value-face-source.md` — оба раздела содержат с предметным
+наполнением (конкретные риски со снимающим механизмом; `git revert` +
+анализ потери данных для отката). В 381 часть содержания фактически
+рассеяна по тексту («Известная граница downgrade» в разделе Import/export,
+раздел «Мутанты» как список технических рисков тестирования), но ни то, ни
+другое не отвечает на вопрос §2.5 буквально: как откатить именно этот выпуск,
+если `none` после релиза окажется проблемным (флага Labs нет, отдельной
+миграции нет — значит ответ, вероятно, тривиален: `git revert`
+реализационных коммитов, уже записанный `tap_action: "none"` остаётся в
+конфиге и безопасно читается как `info` старым фронтом — но это нужно
+написать явно, а не оставлять читателю выводить самому), и не перечисляет
+предметные риски конкретно этого контракта (например: неполный список
+потребителей `tap_action` в «Затронутые файлы» — риск того, что найдётся
+шестой потребитель уже после реализации; путаница `none` vs `null` в ручном
+редактировании YAML/API в обход UI).
+
+**Воспроизведение:** `grep -n "^## Риски\|^## Откат" docs/specs/381-no-op-tap-action.md`
+не находит ни одного совпадения; `grep -n "^##"` (полный список выше в
+разделе «Как проверялось» не повторяю) показывает переход прямо от «План
+автотестов» к «Производительность и безопасность» и «Release-артефакты».
+
+**Серьёзность откалибрована по прецеденту.** `SPEC-REVIEW-376-r1.md` нашёл
+структурно идентичную находку («раздел «откат» отсутствует полностью») и
+классифицировал её как Medium, а не High; красный вердикт того раунда был
+вызван отдельной, не связанной High-находкой. Здесь High-находок нет, поэтому
+по правилу §2.4 («без High это жёлтый вердикт») этот Medium один определяет
+вердикт «жёлтый».
+
+**Что делать:** добавить `## Риски` (2–4 пункта: неполный список потребителей
+`tap_action`, путаница `none`/`null` в ручном редактировании конфига в обход
+UI, что ещё именно автор считает риском этого контракта) и `## Откат` (git
+revert реализационных коммитов; судьба уже записанного `tap_action: "none"`
+на старом/новом фронте и бэкенде — эта часть уже фактически написана в разделе
+«Import/export и downgrade», её нужно только процитировать/сослаться под
+правильным заголовком, а не сочинять заново).
+
+### L1 (Low, снимается) — AC4 называет способ доказательства словом вне канонического словаря
+
+AC4 помечен «unit + visual assertion»; §2.5 перечисляет канонические ярлыки
+`unit`/`backend`/`smoke`/`golden`/«ревью кода». «Visual assertion» не входит
+в этот список и по названию мог бы намекать на golden-скриншот, которого
+раздел «Release-артефакты» этого же ТЗ прямо исключает («Новый plan golden не
+требуется…»). Снимаю без правки: раздел «Release-артефакты» и формулировка
+AC4 («совпадают DOM/classes/…») однозначно указывают, что имеется в виду
+DOM/class-assertion в unit-тесте, а не отдельный вид гейта — противоречия по
+существу нет, только словарь. Если автор всё равно правит документ по M1,
+имеет смысл переименовать в «unit (DOM assertion)» заодно, но отдельного
+цикла это не требует.
+
+## Что проверено и корректно
+
+- **Продуктовая рамка.** Job **J3** `docs/SCOPE.md` — точный якорь; пассивный
+  маркер как явный выбор администратора не противоречит ни одному пункту
+  «Out of scope» и не расширяет action-контракт за пределы короткого клика.
+  Сценарий и «что человек увидит» — конкретные, без терминов реализации,
+  отвечают на оба обязательных вопроса §7.1.
+- **Трек выбран верно.** Аргумент «новый UX-контракт» из аналитики
+  подтверждён кодом (новый persisted enum literal, новое наблюдаемое
+  поведение) — не small и не trivial.
+- **Все технические утверждения о текущем коде подтверждены чтением
+  исходников** (полный список — раздел «Как проверялось», пункт 4); догадок,
+  выданных за факт, не найдено.
+- **Контракт поведения однозначен и без домыслов**, включая точку вставки
+  no-op в `_clickDevice()` (до fallback-карточки, а не после) — контракт
+  явно называет актуальный порядок веток и совпадает с реальным кодом.
+- **AC1…AC9 проверяемы**, у каждого указан способ доказательства; AC7
+  (backend) корректно требует cross-language parity test, который уже
+  существует и будет расширен, а не написан с нуля.
+- **План автотестов и раздел «Мутанты»** называют конкретные ошибки
+  реализации (спроецировать `none` в `info`; вставить no-op после
+  fallback-карточки; связать `none` с недоступным Toggle) — это ровно те
+  ошибки, которые нашлись бы при поверхностном прочтении текущего кода,
+  что показывает, что план тестов написан против реального риска, а не
+  формально.
+- **i18n и терминология** — «Действие по нажатию» и паттерн ключей `tap.*`
+  взяты из существующего кода/документации, не изобретены; ровно 4 словаря
+  названы и ровно 4 существуют.
+- **Touch и defaults** решены и не оставлены открытыми: паритет
+  mouse/touch/keyboard явно в AC2, defaults явно не меняются (Скоуп/Не-скоуп).
+- **Release-артефакты** называют оба changelog в скоупе одного коммита с
+  реализацией — соответствует политике `User-Visible: yes` (§2.6).
+- **Открытых продуктовых вопросов действительно нет**: единственная зона
+  неоднозначности («марker остаётся видимым, без disabled-вида») напрямую
+  закрыта формулировкой самого issue («Выбранный маркер остаётся видимым и
+  показывает свои состояния») — это не догадка автора ТЗ, а прямая цитата
+  владельца.
+
+## Чего не проверял
+
+- Не прогонялись `npx tsc --noEmit`, `npm test`, `npm run build`,
+  `node scripts/check-docs.mjs`, `npm run invariants`, браузерные смоки и
+  `golden:verify` — на этапе ревью ТЗ они неприменимы: продуктовый код не
+  менялся (см. `git diff --stat` в шапке документа). Они станут обязательны
+  на этапе код-ревью (§2.7).
+- Не проверялась реализация (её нет) — весь разбор выше про код относится к
+  **текущему**, немодифицированному `dev` и служит только проверке того, что
+  фактические утверждения ТЗ о текущем поведении верны.
+- Не проверялась орфография/стиль переводов DE/FR по существу (нет статуса
+  носителя языка) — только структурное наличие всех четырёх ключей и
+  соответствие паттерну именования.
+
+## Вердикт
+
+Один Medium в скоупе, High нет → по §2.4 жёлтый вердикт, возврат автору на
+доработку двух обязательных разделов; остальной контракт (AC, модель данных,
+i18n, touch, defaults, release-артефакты) готов и не требует повторного
+разбора, если правка ограничится добавлением «Риски»/«Откат» (это будет
+проверено по дельте в r2, PROCESS.md §2.9/§2.10).
+
+`Вердикт: жёлтый · заход r1 · блокирующих циклов 1/4 · High: 0 · Medium: 1 → в задаче · Документ: docs/reviews/SPEC-REVIEW-381-r1.md`