docs: review document for #402

Issue: #402
User-Visible: no

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

@@ -0,0 +1,222 @@
+# SPEC-REVIEW-402-r1
+
+- Issue: https://github.com/Matysh/houseplan-card/issues/402
+- Артефакт ТЗ: `docs/specs/402-confirm-outside-main-branch.md` (полный трек, класс A)
+- Заход: r1 · лимит циклов ревью ТЗ для полного трека — 4 (§4), израсходовано 0
+- Ревьюер: Claude (роль «ревьюер ТЗ»), независимая сессия, без устных пояснений автора
+- База сравнения: HEAD `d94db87e` (коммит, добавивший спецификацию)
+
+## Скоуп ревью
+
+Первый раунд ревью ТЗ для #402: регресс подтверждения опасного действия
+(`hp-confirm`), которое рендерится только в финальной ветке `render()`
+(`src/houseplan-card.ts`) и поэтому недоступно/теряется в остальных ветках
+(онбординг «нет пространств», `fixed_floor` pending/invalid, `!space`).
+Задача полного трека (P1, bug, класс A) — файл ТЗ обязателен, что и сделано.
+
+## Как проверялось
+
+1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` целиком (разделы §2.4,
+   §2.5, §5, §7.1, §7.2), тело issue #402 и единственный комментарий (аналитика
+   S2).
+2. Прочитан весь текст ТЗ `docs/specs/402-confirm-outside-main-branch.md`.
+3. Каждое фактическое утверждение ТЗ о коде сверено с текущим деревом на SHA
+   `d94db87e`:
+   - структура `render()` (`src/houseplan-card.ts:11165-11884`) — все семь
+     ранних веток и их точки возврата;
+   - `HpConfirmController` (`src/danger-confirm.ts`) — поведение `confirm` /
+     `resolve` / `cancel`;
+   - вызов `_confirmDanger` из `_deleteServerPlan`
+     (`src/houseplan-onboarding-runtime.ts:218-224`, клик на `:273`);
+   - все восемь call site'ов `_confirmDanger` (`houseplan-card.ts`,
+     `houseplan-editor-runtime.ts` ×5, `houseplan-onboarding-runtime.ts` ×2);
+   - расположение `_tapConfirm` (`:11852-11868`) и `_vacCalConfirm`
+     (`:11802-11812`) относительно `_dangerConfirm` (`:11872-11878`);
+   - импорт `hp-confirm` — проверено, что он не ленивый (`houseplan-card.ts:14`,
+     верхнеуровневый `import`), что подтверждает заявление ТЗ об отсутствии
+     влияния на бюджет (AC7);
+   - использование `noChange` во всём `src/**` — подтверждено, что сентинел
+     нигде не оборачивается в шаблон (`grep noChange`), это делает техническое
+     утверждение границы ТЗ («noChange нельзя обернуть») фактом, а не догадкой.
+4. Проверено соответствие терминологии `docs/USER-GUIDE.ru.md` (раздел про
+   общий диалог подтверждения) — видимых текстов ТЗ не меняет, конфликта нет.
+5. Проверено `docs/TOUCH-SUPPORT.md` и DoR-чек-лист §2.5 на предмет
+   обязательного пункта про touch/kiosk — см. находку H1.
+6. Проверены обязательные разделы §7.1 — присутствуют все (сценарий · что
+   человек увидит · проблема/контракт · скоуп/не-скоуп · UX · модель данных ·
+   i18n · AC1-7 с доказательством · план автотестов · риски · откат ·
+   release-артефакты).
+7. Гейты кода (`tsc`, `test`, `build`) не гонялись: на этапе ТЗ продуктового
+   диффа ещё нет, гонять их не над чем (см. «Чего не проверял»).
+
+## Находки
+
+### H1 (High, блокирует). ТЗ не называет влияние на touch/kiosk — обязательный, объявленный блокирующим пункт DoR
+
+**Где**: весь файл `docs/specs/402-confirm-outside-main-branch.md` — ни разу
+не упоминает touch, kiosk или `TOUCH-SUPPORT.md` (`grep -i touch|kiosk` по
+файлу — ноль совпадений).
+
+**Почему это находка, а не формальность**. PROCESS.md §2.5 перечисляет пункты
+«Готово к разработке» и явно помечает один из них как блокирующий отдельно от
+остальных: «влияние на touch по `docs/TOUCH-SUPPORT.md` (**View и киоск —
+блокирующие**)», и там же: «Если хоть один пункт не выполнен — статус не
+«Готово к разработке»». `TOUCH-SUPPORT.md` со своей стороны требует того же
+прямым текстом: «New editor feature specifications … must state one of:
+`Touch editor: supported` / `best effort / intentionally degraded` / `not
+exposed`».
+
+Затронутая этим ТЗ поверхность — не абстрактный служебный код: это ровно
+диалог `<hp-confirm>` в состоянии «нет пространств» (онбординг) и в
+`fixed_floor` pending/invalid, то есть экран, который согласно
+`TOUCH-SUPPORT.md` относится к категории «View dialogs and safe device
+actions» — «Fully supported» на touch, без исключений best-effort. Часть
+затронутых call site'ов (`houseplan-editor-runtime.ts`, 5 мест) при этом
+принадлежит редакторам, для которых допустима best-effort деградация — но
+только если она **явно объявлена**, а не подразумевается молчанием.
+
+**Симптом отсутствия анализа**: без явного утверждения нельзя отличить
+«автор проверил — влияния нет» от «автор не думал про touch вовсе». Это
+особенно значимо здесь, потому что рефакторинг двигает `hp-confirm` из
+внутреннего дочернего узла одной `<ha-card>` в узел, рендерящийся отдельно от
+конкретной ветки разметки (см. Риск «двойной рендер диалога» в самом ТЗ) —
+у смены места крепления DOM-узла есть техническая возможность задеть
+touch-специфичные вещи (portal/DOM-scope для `dialog.showModal()`,
+`pointer-events`, `:focus-trap`), даже если по факту не заденет.
+
+**Фактическая оценка (для экономии цикла)**: содержательно последствий,
+скорее всего, нет — `<hp-confirm>` остаётся тем же кастомным элементом с тем
+же shadow DOM и той же логикой модального диалога независимо от того, чьим
+прямым потомком в основном дереве он является; ни один AC1-7 не описывает
+изменение верстки, жестов или поведения `hp-confirm` самого по себе. Но это
+вывод ревьюера, а не факт, зафиксированный автором в ТЗ — а фиксировать его
+обязан автор (правило #163 «New editor feature specifications … must state
+one of»).
+
+**Требуемая правка**: добавить в ТЗ явную строку по образцу
+`Touch: View/kiosk — fully supported, без изменений (тот же <hp-confirm>,
+маршрут рендера не меняет разметку/жесты диалога)`, и отдельно —
+`Touch editor: supported` для пяти call site'ов из `houseplan-editor-runtime.ts`,
+если они действительно не деградируют. Это правка одной-двух строк текста, не
+кода.
+
+### M1 (Medium, в скоупе задачи). «Не-скоуп»-обоснование для `_tapConfirm`/`_vacCalConfirm` содержит фактическую неточность
+
+**Где**: `docs/specs/402-confirm-outside-main-branch.md`, раздел «Скоуп /
+не-скоуп»: «Не в скоупе: … `_tapConfirm` и `_vacCalConfirm` — у них своя
+механика и свои ветки.»
+
+**Проверено чтением кода**: `_tapConfirm` рендерится в
+`src/houseplan-card.ts:11852-11868`, `_vacCalConfirm` — в `:11802-11812`,
+`_dangerConfirm` — в `:11872-11878`. Все три блока лежат в **одной и той же**
+финальной ветке `render()` (последний `<ha-card>`, тот же самый шаблон, между
+ними нет ни одного `return`). Утверждение «свои ветки» в буквальном
+прочтении неверно: у них нет собственных веток `render()` — они делят ровно
+ту же ветку, что и `_dangerConfirm` до этого фикса, и подвержены тому же
+классу дефекта («открытое подтверждение исчезает при смене ветки», вторая
+половина дефекта из аналитики S2), например если многоклиентская
+синхронизация (J6, `docs/SCOPE.md`) уводит карточку в `fixed_floor` pending
+или обнуляет `model.length` **пока** открыт `_tapConfirm`/`_vacCalConfirm`.
+
+Отличие от `_dangerConfirm`, которое у ревьюера НЕ вызывает вопросов и,
+похоже, и есть настоящая причина исключения: `_tapConfirm`/`_vacCalConfirm`
+не используют `HpConfirmController` и не отдают вызывающему `Promise` —
+`exec()` вызывается синхронно по клику, поэтому «вечно висящего промиса» у
+них в принципе не бывает (при потере диалога они не блокируют await
+вызывающего, а просто становятся недоступны до следующего рендера основной
+ветки). А сами точки входа (`marker.tap_confirm`, календарь пылесоса)
+физически недостижимы из веток онбординга/`fixed_floor`/`!space` — там нет
+устройств на плане, значит буквальный сценарий issue их не касается.
+
+Это делает исключение из скоупа **разумным по существу**, но
+**обоснование в тексте — неверным**: «свои ветки» вместо настоящей причины
+(«синхронный exec без промиса» + «недостижимость точки входа из веток вне
+основной»). Ложное обоснование опасно не абстрактно: тот, кто будет
+реализовывать вынос `hp-confirm` из финальной ветки, должен точно знать, что
+`_tapConfirm`/`_vacCalConfirm` физически стоят в той же ветке и их нельзя
+случайно утащить вместе с `_dangerConfirm` при выносе блока — а формулировка
+«у них свои ветки» наводит на обратное представление.
+
+**Требуемая правка**: заменить формулировку на техническую причину исключения
+(синхронный `exec`, недостижимость входа) и явно предупредить реализацию не
+трогать расположение этих двух блоков при выносе `_dangerConfirm`. Опционально
+(не обязательно для этого issue) — упомянуть остаточный риск «уже открытый
+`_tapConfirm`/`_vacCalConfirm` теряется при смене ветки во время
+многоклиентской синхронизации» как кандидата в отдельный issue, если владелец
+сочтёт его достаточно вероятным; в скоупе #402 фиксировать не обязательно —
+issue именно про мёртвую кнопку и висящий промис, которых у этих двух объектов
+нет.
+
+## Что проверено и признано корректным
+
+- **Диагноз дефекта** — точен и воспроизводим по строкам: `render()` действительно
+  возвращает `hp-confirm` только в финальной ветке (`:11872`), все более ранние
+  `return` (`:11166`, `:11170`, `:11171`, `fixed_floor` pending/invalid,
+  `!model.length`, `!space`) реально существуют и реально не содержат
+  `hp-confirm`.
+- **`HpConfirmController` описан верно**: `cancel()` действительно резолвит
+  `false` через `resolve()`, `resolve()` сверяет токен, новый `confirm()`
+  отменяет предыдущий запрос (`src/danger-confirm.ts:47-72`) — контроллер не
+  участвует в дефекте, претензия ТЗ обоснована.
+- **Буквальный сценарий issue** (`_deleteServerPlan` из онбординга) сверен
+  построчно: `src/houseplan-onboarding-runtime.ts:218-224` вызывает
+  `this.host._confirmDanger`, кнопка в `:273` — реальная причина «мёртвой
+  корзины» из issue.
+- **Техническое утверждение о `noChange`** («нельзя обернуть в шаблон») —
+  подтверждено: во всём `src/**` `noChange` возвращается только как значение
+  `render()` целиком (`houseplan-card.ts`, `editor.ts`, `space-card.ts`,
+  `space-editor.ts`), ни разу не встречается внутри `${…}` — граница ТЗ не
+  придумана, а списана с уже действующего паттерна кодовой базы.
+- **AC1-AC4, AC6** — проверяемы, у каждого назван способ доказательства
+  (смок), и способ реалистичен: `demo/smoke_danger_confirmation.mjs` уже
+  существует и уже покрывает контракт диалога, дополнение под новые ветки —
+  органичное расширение того же файла.
+- **AC5** (иммедиат-отказ в `!_config||!hass` и `warm`) — продуман
+  корректно: `!hass`/`!_config` не могут стать `false→true→false` в течение
+  жизни примонтированной карточки (Lovelace не сбрасывает `hass`/`config` в
+  falsy после первичной установки), то есть эта ветка достижима только до
+  первого рендера — отказывать там немедленно безопасно и не теряет открытый
+  диалог (терять нечего). Ветка `warm` реально достижима в течение сессии
+  (смена языка), и там `noChange` **сохраняет** уже показанный диалог
+  нетронутым (Lit не трогает DOM) — то есть AC5 верно ограничивает
+  немедленный отказ только НОВЫМИ запросами, не описывая (и не должен
+  описывать) отмену уже открытых.
+- **AC7 (бюджет)** — `hp-confirm` импортируется в шапке файла обычным
+  (не ленивым) `import` (`houseplan-card.ts:14`), поэтому перемещение точки
+  рендера не добавляет новый чанк/зависимость — заявление «правка структурная,
+  бюджет не растёт» подтверждается фактическим импортом.
+- **Скоуп/не-скоуп в остальном** — граница с #32 (содержимое диалога,
+  ревалидация после `await`) и с #406 (`alertdialog`/`aria-describedby`)
+  названа явно и не пересекается с контрактом этого ТЗ.
+- **Обязательные разделы §7.1** — все присутствуют, включая «откат» и
+  «release-артефакты» (changelog RU+EN).
+- **Соответствие `docs/SCOPE.md`**: чинит J4 (онбординг, «zero to a working
+  plan») и снимает риск для J6 (multi-client sync может вызвать смену ветки
+  под открытым диалогом) — задача не расширяет продукт, а восстанавливает
+  ранее рабочее поведение (регресс против до-#32).
+- Метки issue (`bug`, `P1`, `S4-spec-review`, без `small`/`trivial`)
+  согласуются с заявленным в ТЗ полным треком.
+
+## Чего не проверял
+
+- **Гейты кода** (`npx tsc --noEmit`, `npm test`, `npm run build`,
+  `check-docs`, смоки, `bundle:budget`) — не гонялись: на этапе ТЗ
+  продуктового кода ещё нет, диффа для гейтов не существует. Это будет
+  предметом код-ревью после реализации.
+- **`scripts/mutation-gate.mjs` / `demo/smoke_danger_confirmation.mjs`** —
+  не запускал, только убедился, что оба файла существуют и их формат
+  (реестр мутантов, browser-smoke на реальной карточке) совместим с планом
+  автотестов ТЗ.
+- **Таблицу `docs/specs/README.md`** — строка для #402 в неё не добавлена,
+  но это не регрессия этого ТЗ: специфика #395-#401 туда тоже не занесена
+  (таблица не поддерживается систематически, известный долг §7.3 п.1
+  документа процесса), поэтому не поднимаю отдельной находкой.
+- **Реальный рендер в браузере** — на этапе ТЗ кода нет, воспроизведение
+  дефекта в описании ТЗ и issue взято на веру как «воспроизведено исполнением»
+  автором аналитики; независимо не перепроверял (это будет предметом
+  смок-доказательства в код-ревью).
+- **`_tapConfirm`/`_vacCalConfirm` как отдельный дефект** — не завожу
+  отдельный issue: по результату разбора (см. M1) это не тот же класс
+  дефекта (нет висящего промиса, нет недостижимости из веток issue), поэтому
+  это не «Medium вне скоупа» по #202, а неточность формулировки внутри
+  текущего ТЗ — чинится правкой текста в этом же документе.