docs: review document for #32

Issue: #32
User-Visible: no

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

@@ -0,0 +1,150 @@
+# SPEC-REVIEW-32-r1
+
+- Issue: https://github.com/Matysh/houseplan-card/issues/32
+- Этап: ревью ТЗ (PROCESS.md §2.4), заход r1
+- Материал: `docs/specs/032-unified-danger-confirmation.md` на SHA `206bcd1a`
+  (коммит «docs: refresh danger confirmation spec»), тело issue #32, все
+  комментарии (актуализации 2026-08-14, 2026-08-30, occupation-комментарий,
+  комментарий о готовности ТЗ на SHA 206bcd1a)
+- Трек: полный (владелец снял `small` 2026-08-30: не проходит критерии «одна
+  поверхность» и «нет нового UX-контракта», задета touch)
+- Бюджет: заход r1, блокирующих циклов израсходовано 0 из 4 (лимит 4, полный трек)
+
+## Скоуп проверки
+
+ТЗ описывает замену восьми прямых browser `confirm()` единым
+`hp-confirm`-компонентом поверх `hp-dialog` с promise-based host API,
+race-safety-контрактом (revalidation после `await`), touch/i18n-требованиями и
+границами со специализированными диалогами (комната, стена с проёмами, декор,
+backup/optimize, Tap confirmation).
+
+Проверялось:
+
+1. `docs/SCOPE.md` — соответствие J3/J6 и lock-инварианту («единственная
+   разрешённая поверхность актуации замка — кнопка в карточке проёма, которая
+   дополнительно подтверждает перед unlock»);
+2. `AGENTS.md`, `PROCESS.md` §2.4/§2.5/§7.1/§4 — обязательные разделы ТЗ,
+   критерии DoR, бюджет циклов;
+3. `docs/ARCHITECTURE.md` § One modal contract / One transient-surface contract;
+4. `docs/TOUCH-SUPPORT.md` § Safety floor;
+5. фактический код: все 8 call sites `confirm(` в `src/**`, существующие
+   специализированные диалоги (`_roomDeleteDialog`, `_partitionDeleteDialog`,
+   `confirm.erase_decor`, backup/optimize), `src/hp-dialog.ts` (focus session,
+   autofocus, Escape), `src/houseplan-editor-runtime.ts::_deleteSpace`
+   (blocker-проверка перед confirm);
+6. два спеки-precedента (`337-lazy-editor-chunk.md`,
+   `348-german-localization.md`) как калибровка того, что в этом репозитории
+   считается «затронутые файлы» и «влияние на производительность» в ТЗ
+   сопоставимого масштаба.
+
+## Находки
+
+### Medium (в скоупе задачи — чинится в этой же правке ТЗ)
+
+**M1. DoR-пункт «влияние на производительность и бюджеты названо» не закрыт,
+хотя задача добавляет новый eager-код.**
+
+- Файл: `docs/specs/032-unified-danger-confirmation.md`
+- Строка: §5.2 (владение состоянием) и §10 (тестовый план)
+- Суть: §5.2 явно решает архитектурный вопрос — «Controller находится в eager
+  root, поэтому onboarding не тянет editor chunk, а View unlock не загружает
+  его». Это осознанное добавление нового кода (`hp-confirm.ts` + controller +
+  разметка для всех 6 сценариев) в **eager root graph**, который в этом
+  проекте является бюджетируемой величиной: `AGENTS.md` называет
+  `npm run run bundle:budget # initial View graph <= 256000 B gzip (#337)` в
+  одном ряду с `bundle:sync` как обязательный после-сборочный гейт, и
+  прецедентный `docs/specs/337-lazy-editor-chunk.md` трактует именно этот
+  initial-view-бюджет как central product constraint с отдельным разделом и
+  AC. Раздел 10 текущего ТЗ (обязательные команды перед S7) перечисляет
+  `bundle:sync`, но не `bundle:budget`; ни один AC (§9) не привязан к размеру
+  eager-бандла.
+- Воспроизведение: `npx tsc --noEmit` / `npm test` этого не поймают —
+  `bundle-budget.mjs` работает только по manifest после `npm run build`, и
+  если его не прогнать явно, регресс initial View graph (пусть и небольшой)
+  останется незамеченным до пре-релизного гейта, где чинить дороже, чем сейчас
+  дописать одну строку в тестовый план.
+- Почему это Medium, а не Low: DoR (PROCESS.md §2.5) прямо требует «влияние на
+  производительность и бюджеты названо (или явно «нет»)» как условие входа в
+  `S5-ready`; сейчас оно не названо ни явно, ни как «нет» — и «нет» здесь было
+  бы неверным утверждением, раз §5.2 сознательно добавляет код в eager graph.
+  Фикс дешёвый: добавить предложение с ожидаемым порядком величины прироста
+  (или явную оценку «в пределах текущего headroom») и добавить
+  `npm run bundle:budget` в список команд §10 рядом с `bundle:sync`.
+- Находка в скоупе issue #32 (сам ТЗ), чинится в этой же правке ТЗ — не
+  отдельный issue (owner's decision 2026-08-19, #202).
+
+## Что проверено и корректно
+
+- **Полнота охвата call sites.** Таблица §4.1 называет ровно 8 сценариев;
+  `rg 'confirm\('` по `src/**` даёт ровно те же 8 строк в
+  `houseplan-card.ts`, `houseplan-editor-runtime.ts`,
+  `houseplan-onboarding-runtime.ts` — расхождений нет.
+- **Границы со специализированными диалогами не выдуманы.** Проверено чтением
+  кода: `_roomDeleteDialog` (houseplan-editor-runtime.ts:11599-11618) — уже
+  `hp-dialog` с двумя вариантами (keep/with walls); `_partitionDeleteDialog`
+  (:11564-11587) — уже `hp-dialog` со списком проёмов; `confirm.erase_decor`
+  (:5539) — уже кастомный `hp-dialog`; backup/import/optimize — многошаговые
+  диалоги с собственными подтверждениями (`backup.confirm_detach`,
+  `backup.replace_warning`). Исключение этих четырёх групп из scope (§4.2)
+  соответствует факту, а не догадке.
+- **Claim о существующем focus/autofocus/Escape-контракте hp-dialog не
+  выдуман.** `src/hp-dialog.ts` действительно реализует focus session,
+  scoped к shadow root, `[autofocus]`-резолюцию и обработку `Escape` (строки
+  237-396) — §6.1 ТЗ корректно ссылается на наследуемое поведение, а не
+  придумывает новое.
+- **Race-safety claim по пространству проверен по коду.**
+  `_deleteSpace` (houseplan-editor-runtime.ts:8758-8784) действительно
+  выполняет dependency/blocker-проверку **до** `confirm()` — ровно то, что
+  §6.3 таблицы утверждает («существующая blocker-проверка остаётся раньше
+  confirm»).
+- **Lock-инвариант SCOPE.md не нарушается и не переопределяется.** §4.2
+  явно оставляет закрытие замка без подтверждения (безопасное действие) и не
+  трогает единственную разрешённую поверхность актуации; AC-9 отдельно требует,
+  чтобы unlock не вызывался после cancel/stale state.
+- **Обязательные разделы §7.1 присутствуют**: сценарий и персона (§1),
+  «до/после» одной фразой без терминов реализации (§2), проблема и цель (§3),
+  scope/не-scope (§4), контракт поведения и API (§5, §7), UX (§6), данные и
+  миграция (§8 — явно «миграция отсутствует»), AC1-12 с доказательством (§9),
+  план автотестов (§10), риски (§11), откат (§12), release-артефакты (§13).
+- **Явных догадок, выданных за факт, не найдено.** Единственное место, где ТЗ
+  сознательно оставляет свободу выбора — §6.1 «action использует warning/on
+  акцент» для unlock-кнопки — не выдаёт решение за факт, а явно формулирует
+  как одно из двух допустимых значений; в проекте нет отдельного `.btn.warning`
+  класса, только warning-баннер, так что оба варианта («on»-акцент или новый
+  warning-акцент) реалистичны и не противоречат существующим стилям. Отношу это
+  к техническому допущению (§14 ТЗ его не называет явно, но по духу оно туда
+  подходит) — не блокирую, ревьюер кода вправе потребовать явного выбора при
+  коде.
+- **Открытых продуктовых вопросов нет** — комментарий владельца от 2026-08-30
+  подтверждает, что контракт достаточно определён; ревьюер не находит
+  продуктового вопроса, который стоило бы вынести отдельно (единственный
+  недостающий пункт, M1, технический и не требует решения владельца).
+- **Ссылки issue ↔ ТЗ на месте** в обоих направлениях (тело issue → файл ТЗ;
+  `docs/specs/README.md:39` → issue).
+
+## Чего не проверял
+
+- Реализацию — её нет, стадия `S4-spec-review`, код не писался.
+- Точный список i18n-ключей (en/ru/de/fr) — ТЗ описывает контент body/title по
+  сценариям (§6.3), но не называет конкретные строковые ключи. Признано
+  приемлемым для этой стадии: AC-11 требует машинной проверки паритета через
+  `test/i18n.test.mjs`, а конкретные имена ключей — техническая деталь
+  (PROCESS.md §7.1: «именование» решают агенты сами), не продуктовый вопрос.
+  Не поднимаю до Medium.
+- Golden/perf профили не запускал — на этой стадии нет кода для прогона;
+  `npm run typecheck`/`npm test`/`npm run build` не прогонялись, так как
+  ревью ТЗ не гейтится кодовыми командами (правки затрагивают только
+  `docs/specs/032-*.md`, класс C).
+- Не сверял ТЗ с `docs/CONFIG-COMPATIBILITY.md` построчно — §8 ТЗ утверждает
+  «persisted schema не меняется, миграция отсутствует», что для чисто
+  presentation-контракта (без изменения config/layout) выглядит правдоподобно
+  и не противоречит ничему найденному в CONFIG-COMPATIBILITY.md; не стал
+  открывать весь документ ради этой проверки, так как diff ТЗ прямо
+  утверждает «вне scope: формат config/layout».
+
+## Вердикт
+
+Единственная находка (M1) — Medium, строго в скоупе этого же ТЗ, чинится
+дополнением двух мест в тексте (не код, не отдельный issue). High-находок нет.
+Итог: **жёлтый**, автор ТЗ правит §5.2/§10, повторный заход разбирается по
+дельте (PROCESS.md §2.10) относительно этого документа и SHA `206bcd1a`.