docs: review document for #463

Issue: #463
User-Visible: no

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

@@ -0,0 +1,82 @@
+# SPEC-REVIEW-463-r1
+
+**Issue:** #463 — «Диалоги открываются немодальными и прижимаются к левому краю: showModal() теряется, ветка ha-dialog выбирается на удачу»
+**Этап:** ТЗ на ревью (S4-spec-review), лёгкий трек (`small`)
+**Заход:** r1 · блокирующих циклов израсходовано 0 из 2 (лимит лёгкого трека — 2)
+**Материал:** тело issue #463 на момент ревью (канонический блок «ТЗ лёгкого трека» после разделителя `---`, включая правку-комментарий владельца от 2026-09-05T14:20:02Z) + `src/hp-dialog.ts` на `HEAD=fc0c6166` (dev) + принятые golden-baselines в `demo/golden/baselines/**` на том же SHA.
+
+## Скоуп
+
+Задача чинит общий modal-контракт `hp-dialog` (`src/hp-dialog.ts`), используемый во всех 31 местах карточки: native `<dialog>`-ветка (обязательна для `alert`-подтверждений и как fallback без `ha-dialog`) не должна растягиваться на весь viewport и должна самовосстанавливать `:modal` после потери top-layer членства (detach/reattach, повторный апдейт). Изменение не входит ни в один Core user job SCOPE.md напрямую, но является предпосылкой к работе J3/J4/J6 (диалоги подтверждения и редакторы, которыми пользуется единственная персона, работающая с редакторами, — Home admin). Это баг-фикс, восстанавливающий уже описанный контракт («диалог модален, центрирован, имеет scrim и focus containment»), не новая функциональность — конфликта со SCOPE.md нет.
+
+Не-скоуп ТЗ подтверждён явно: редизайн диалогов, размеры/отступы, переезд на `ha-md-dialog` — не трогаются.
+
+## Как проверялось
+
+- Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` целиком (лимиты циклов, формат вердикта, требования §7.1/§2.5/§5).
+- Прочитано тело issue #463 полностью, включая исходный «Симптом» (не помечен как заменённый) и оба комментария владельца (аналитика + правка после воспроизведения на `origin/dev`).
+- Прочитан `src/hp-dialog.ts` целиком: проверены технические утверждения контракта против фактического кода — ветвление `_usesHaDialog()`, единичный вызов `showModal()` в `firstUpdated()`, отсутствие какой-либо повторной проверки модальности при апдейтах/переподключении, CSS `dialog { width: auto; max-width: none; margin: auto; }` и `.surface { width: min(360px, 92vw); }`.
+- Проверено, регистрирует ли демо/golden-окружение `ha-dialog` (`grep -rn "ha-dialog" demo/srv/demo.html demo/serve.mjs demo/golden/*.mjs`) — не регистрирует нигде.
+- Открыты и визуально изучены три принятых golden-baseline с открытым `hp-dialog` разных сценариев: `device-dialog-desktop-en.png`, `backup-full-preview-desktop-en.png`, `optimize-preflight-dialog-dark-en.png`.
+- Прочитан существующий `demo/smoke_danger_confirm_branches.mjs`, чтобы исключить дублирование с новым «отдельным browser smoke общего modal-контракта» — покрытие геометрии/`:modal` там отсутствует, дублирования нет.
+
+Кода для гейтов (`typecheck`/`test`/`build`) на этом этапе нет — задача ещё не реализована, стадия ТЗ. Гейты не прогонялись: неприменимо к этапу.
+
+## Находки
+
+### [High] Раздел «Release-артефакты» содержит опровергнутое фактом утверждение: golden обязателен, не опционален
+
+**Файл:** тело issue #463, раздел «Release-артефакты» канонического ТЗ.
+
+**Утверждение ТЗ:** «Новый golden не обязателен, если восстановленный вид побайтово совпадает с действующими эталонами; browser smoke обязателен».
+
+**Почему это неверно.** `demo/golden/harness.mjs`, `demo/serve.mjs` и `demo/srv/demo.html` нигде не регистрируют кастомный элемент `ha-dialog`. Значит `HpDialog._useHaDialog = !!customElements.get('ha-dialog')` равно `false` в среде golden **всегда**, и абсолютно каждый диалог, попавший в golden-матрицу (`dialog: 'device'`, `'backup-full'`, `'backup-export-plan-only'`, `'backup-space'`, `'optimize-preflight'` ×2, `'optimize-orphan-references'` ×2, `'decor-color'`, `'general-color'`, `'general-help'` ×2, `'support'` ×6, `'device-ripple-color'`, `'space-room-color'`), рендерится нативной веткой — той самой, которую чинит эта задача.
+
+Открыл три принятых эталона напрямую: **все три уже показывают ровно тот дефект, который описан в issue** — диалог прижат к верхнему левому углу страницы, фон снаружи не затемнён:
+
+- `demo/golden/baselines/device-dialog-desktop-en.png` — диалог «Device on the plan» у левого края, план справа не затемнён;
+- `demo/golden/baselines/backup-full-preview-desktop-en.png` — диалог «Import House Plan» у левого верхнего угла, план не затемнён;
+- `demo/golden/baselines/optimize-preflight-dialog-dark-en.png` — диалог «Optimize plans» у левого верхнего угла, план не затемнён.
+
+Три независимых сценария, три разных `space`/режима — совпадение исключено; это системное свойство текущей связки код+тестовая среда, а не случайность одного скриншота. После того как контракт починят (диалог центрирован, backdrop виден), пиксели этих (и, по всей вероятности, ещё ~15 родственных `dialog:`-сценариев из `matrix.mjs`) эталонов **обязаны** измениться. Заявление «побайтово совпадает» в разделе Release-артефакты неверно для практически любого сценария, где вообще открыт видимый диалог.
+
+**Почему это блокирует, а не мелкая правка текста.** Раздел «Release-артефакты» — часть DoR (§2.5 PROCESS.md: «release-артефакты по правилу `docs/specs/README.md`» обязателен пункт для перехода в `S5-ready`). Ложное «golden не нужен» — ровно тот тип догадки, который "проходит ревью, потому что выглядит решением" (формулировка §7.1): код-ревьюер следующего этапа, доверяя этому пункту ТЗ, может по праву не прогнать `golden:verify` при код-ревью (правило §8: гоняется «если diff может изменить видимый результат» — а ТЗ прямо утверждает, что не изменит). Результат — красный `golden` job на пре-релизе (не на код-ревью), то есть находка того же класса, что уже стоила `dev` красного `docs`-job в #230/#234. Объём переоценки нетривиален: это не один эталон, а десяток+ сцен с диалогами — сопоставимо по труду с самой правкой CSS.
+
+**Требуемая правка (в скоупе, без выхода за лёгкий трек):** заменить пункт на «golden-эталоны для сцен с открытым `hp-dialog` (не менее ~15 сцен: `device-dialog-*`, `backup-*`, `optimize-preflight-*`, `optimize-orphan-references-*`, `*-color-popover-*`, `general-help-*`, `support-*`, `device-help-popover-*`) обязаны быть пересняты и приняты через `npm run golden:accept -- --reviewed` вместе с этой задачей — они уже фиксируют дефект, который контракт исправляет». Это не расширяет скоуп и не меняет пригодность лёгкого трека (новый модуль/миграция/UX-контракт не добавляются), только корректирует раздел DoR и реалистичную оценку объёма.
+
+### [Low] AC1: формулировка «backdrop непрозрачен» неоднозначна относительно действующей CSS
+
+**Файл:** тело issue #463, AC1.
+
+Действующий `dialog::backdrop { background: rgb(0 0 0 / 0.45); }` (`src/hp-dialog.ts:142-144`) — сознательно полупрозрачный scrim (45%), не сплошной. Формулировка AC1 «вычисленный backdrop непрозрачен» при буквальном прочтении («opaque», alpha = 1) требует несуществующего и не входящего в скоуп визуального решения («Не входит: редизайн... размеры и отступы» этого не покрывает явно, но подразумевает отсутствие внешних правок вида). При прочтении как «не полностью прозрачен» (alpha > 0, то есть просто виден) формулировка верна и совпадает с фактическим дефектом («Затемнения фона нет» в исходном «Симптоме»).
+
+Не блокирует: смысл читается однозначно из контекста остального ТЗ (проблема описана как «нет затемнения», а не «затемнение недостаточно тёмное»), и разработчик/ревьюер кода поймут это верно. Рекомендация — при правке High-находки заодно уточнить формулировку до «backdrop виден (computed alpha > 0)», чтобы мутационный тест не проверял случайно alpha === 1.
+
+## Что проверено и корректно
+
+- **Техническая база контракта достоверна, не догадка.** Оба диагноза из правки-комментария владельца («native dialog растягивается, отчего inner surface у левого края» и «после detach/reattach `open=true`, `:modal=false`, повторный `showModal()` бросает `InvalidStateError`») согласуются с прочитанным кодом: `firstUpdated()` вызывает `showModal()` **ровно один раз** (нет повторной проверки при `updated()`/переподключении — ни `willUpdate`, ни `connectedCallback` второй раз это не делают), а браузерный контракт top-layer действительно теряется при detach независимо от `open`. Это не выдано за факт без основания — оба поведения напрямую видны в коде и в `HTMLDialogElement`.
+- **AC1–AC3** сформулированы как проверяемые критерии со связанным мутационным свидетелем («мутант, возвращающий X, обязан краснеть») — соответствует требованию §2.5/§2.7 «доказательство + свидетель».
+- **AC4** корректно разводит два случая (поздняя регистрация `ha-dialog` для уже открытого native-fallback экземпляра vs новый обычный экземпляр) и не молчит о выборе — это явный пункт контракта (п.5), не пропущенный пограничный случай.
+- **Не-скоуп, откат, совместимость/touch/производительность, i18n** — все присутствуют, согласованы с фактическим состоянием (backend/config/i18n действительно не тронуты этим модулем), откат — простой revert без миграции. Замечаний нет.
+- **Дублирование со `smoke_danger_confirm_branches.mjs` отсутствует** — прочитан файл целиком: он проверяет ветвление, ARIA, фокус и жизненный цикл `_confirmDanger`, но нигде не проверяет геометрию `.surface`, `:modal` или `::backdrop`. Новый отдельный смок обоснован тем же принципом, что уже используется в проекте (`smoke_danger_confirmation.mjs` vs `smoke_danger_confirm_branches.mjs` — разделение по типу оснастки).
+- **Продуктовые разделы (сценарий, «что человек увидит»)** формально не продублированы под заголовками канонического блока, но присутствуют дословно в разделе «Симптом» того же тела issue (не помечен как заменённый) — персона (Home admin, plan editor, desktop) и наблюдаемая картина «прижато к левому краю, без затемнения» читаются однозначно. Не считаю это отдельной находкой.
+- **Пограничное решение п.5 контракта** (уже открытый native-fallback экземпляр не пересоздаётся при поздней регистрации `ha-dialog`) — технически это единственный разумный выбор (пересоздание открытого диалога дало бы мерцание/потерю фокуса хуже любой альтернативы), и хотя формально это «поведение в пограничном случае» из списка продуктовых вопросов §7.1, эскалировать его владельцу избыточно: обе стороны выбора недвусмысленно хуже или эквивалентны для пользователя. Не считаю находкой, но отмечаю как решение без явного маркера «принято предположительно» — на усмотрение автора при следующей правке.
+
+## Чего не проверял
+
+- Не запускал `typecheck`/`test`/`build`/`golden:verify` — кода ещё нет, стадия ТЗ, гейты неприменимы.
+- Не проверял golden-сцены за пределами трёх открытых образцов пиксельно (не сверял точные координаты центра), визуальной оценки «прижато к углу, фон не затемнён» достаточно для находки; полный список затронутых сцен при реализации должен быть пересчитан самим автором по `dialog:`-полю `matrix.mjs`, а не переписан с моих слов.
+- Не проверял поведение реального Home Assistant frontend (когда `ha-dialog` регистрируется рано) — вне материала ревью ТЗ, будет видно в код-ревью через смоки.
+
+## Вердикт
+
+Один High: заявление о release-артефактах в текущем ТЗ фактически неверно и вводит в заблуждение относительно объёма golden-работ и условий пропуска `golden:verify` на код-ревью. Возврат автору для правки этого раздела (плюс желательно уточнение формулировки AC1 по backdrop, Low). Остальной контракт, AC1–AC4 и структура ТЗ — корректны и не требуют переписывания.
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `dev`, коммит `` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Якоря снять не удалось: ветки задачи нет, материал читался по `dev`.