docs: review document for #609

Issue: #609
User-Visible: no

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

@@ -0,0 +1,178 @@
+# SPEC-REVIEW-609-r1
+
+## Скоуп
+
+Issue [#609](https://github.com/Matysh/houseplan-card/issues/609) — оболочка
+`hp-dialog[form-shell]` (канва `.content`/`.body`, ширина 560 px, максимальная
+высота `min(940px, 100vh − 48px)`, единственный скроллер, fullscreen ≤480 px),
+принятая референсом #600, реализована только в нативной ветке `<dialog>`
+(`src/hp-dialog.ts:194-212`). Ветка настоящего `ha-dialog`, которую видит любой
+реальный пользователь Home Assistant (`_usesHaDialog()` включается всегда,
+когда кастомный элемент `ha-dialog` зарегистрирован — то есть внутри HA
+всегда), рендерит form-shell без этой оболочки: `width=medium` (580 px),
+никакой канвы, штатный HA-fullscreen только при ширине ≤450 px или высоте
+≤500 px. ТЗ живёт в теле issue под `## ТЗ` (решение владельца 2026-09-10,
+#517), трек — полный (аналитик назвал нарушенный критерий `small`: риск >3,
+затронуты desktop и mobile/touch, контракт проверяется на внешней оболочке
+настоящего HA). Это первый заход ревью ТЗ (r1); возврата на правки нет,
+раздел «Унаследовано» неприменим.
+
+Разбирается вся текстовая часть тела issue: предварительные «Что не так» / 
+«Правка» / `## AC`, затем `## ТЗ` целиком — сценарий, что человек увидит,
+проблема, скоуп/не-скоуп, контракт поведения (8 пунктов), UX и доступность,
+модель данных и миграция, i18n, критерии приёмки AC1–AC7 (каждый с
+доказательством), план автотестов, риски, откат, release-артефакты, блок
+«принято предположительно».
+
+## Как проверялось
+
+Ревью ТЗ, не код: продуктового кода к этой задаче ещё нет. Сверка каждого
+фактического утверждения ТЗ с деревом на SHA `84cffb3f92bba469a987584b262ef6d9efee0d44`
+(рабочая копия), чтение канонических документов и трассировка терминологии.
+
+| Проверка | Результат |
+|---|---|
+| `docs/SCOPE.md` — попадание в Core user jobs | Администратор (primary persona), правит #600-диалоги настроек в реальном HA — J4/J6 (`zero-to-plan`/`keep the plan true`, два встроенных редактора). Задача чинит регресс уже принятого продуктового решения (#600), а не добавляет новое — конфликтов с lock-инвариантом и «never delete a user's file» нет (геометрия/канва диалога, не данные и не actuation) |
+| `src/hp-dialog.ts:197-211` (нативная form-shell оболочка) | Совпадает: `:host([form-shell])` задаёт `--hp-dialog-wide-width: 560px` (198), `max-height: min(940px, calc(100vh - 48px))` (199), канву `--secondary-background-color` (200-206), `@media (max-width: 480px)` fullscreen (209-212) |
+| `dialogs.styles.ts:577-582` (`.body` без собственного скроллера/padding) | Совпадает буква в букву: `hp-dialog[form-shell] .body { max-height: none; overflow: visible; padding: 16px; ... }`, мобильный `padding: 12px` на 584 |
+| `src/hp-dialog.ts` — ветка `ha-dialog` (:558/574 в тексте issue) | **Устарело на ~8 строк**: `width=medium/small` сейчас на строках 566/582, а не 558/574 — см. «Находки», Low. По существу утверждение верно: ни один form-shell-специфичный стиль/CSS-переменная в эту ветку не идёт, `.content`/`.surface`-правил нет вообще |
+| Пять потребителей `form-shell` (space, settings, room, marker, onboarding) | `src/editors/space-settings-dialog.ts:33` (`data-kind="space" form-shell wide`), `general-settings-dialog.ts:82` (`data-kind="settings"`), `room-settings-dialog.ts:146` (`data-kind="room"`), `marker-dialog.ts:848` (`data-kind="marker"`), `houseplan-onboarding-runtime.ts:627` (`data-kind="onboarding"`) — все пять существуют, названия `data-kind` в АC3 ТЗ («Space, General, Room, Device и onboarding») — синонимы этих же пяти значений, не новая номенклатура |
+| Реальный HA-fixture для AC1/AC2/AC6/AC7 | `demo/helpers/ha-dialog-fixture.mjs` + `ha-dialog-assets.mjs` существуют, `HA_DIALOG_PIN.version === '20260729.7'` — совпадает с версией, названной в АС1 ТЗ. Офлайн (WebSocket выбрасывает `Error`, `hassConnection` никогда не резолвится) — соответствует АС7 «без внешних запросов/WebSocket». Прецедент использования — `capture_summary_panel_505.mjs` (#505) и `demo/verify_ha_dialog_discard_recovery.mjs` (#607, kind: marker/room/space/settings) |
+| `--ha-dialog-width-md` — заявлено в §15 как «принято предположительно» | Не голая догадка: переменная уже используется в продукте (`src/summary-panel-editor-style.ts:17`, `test/fixtures/summary-panel-editor.css:4`) и разбиралась в `CODE-REVIEW-505-r1/r2.md` как реальный, наследуемый publicly CSS custom property настоящего `ha-dialog`. Корректно помечено как техническое решение автора, не продуктовый вопрос |
+| Золотой набор — «нативная ветка без изменений» (AC4) | `demo/golden/harness.mjs`, `demo/golden/run.mjs`, `demo/srv/demo.html` нигде не регистрируют кастомный элемент `ha-dialog` — golden всегда идёт через нативный `<dialog>`-fallback. Стаб `ha-dialog` регистрируют только отдельные smoke-скрипты (`smoke_dialog_help_clipping.mjs`, `smoke_dialog_modal_recovery.mjs`, `smoke_free_walls.mjs`, `smoke_danger_confirm_branches.mjs`, `smoke_summary_dialog_scroll.mjs`), golden они не касаются. Значит риск того, что CSS-правки для `ha-dialog`-ветки случайно сдвинут пиксели 13 golden-сцен, реально нулевой — заявление AC4 технически обосновано, а не на честном слове |
+| «13 golden-сцен диалогов #600» (AC4) | Подтверждено: `docs/reviews/CODE-REVIEW-600-r2.md` — «11 из ТЗ + 2 `room-temperature-dialog-*`» = 13, принято `ed2af1b1`. Число не выдумано для этой задачи, унаследовано из закрытой |
+| `docs/USER-GUIDE.ru.md:389-394` | Уже описывает целевое поведение как факт: «Четыре диалога настроек... собраны одинаково. Это одна форма шириной 560 px с одной полосой прокрутки». Это ровно то, что #609 обязано сделать истинным и в реальном HA — задача не меняет документированный контракт, а устраняет расхождение между ним и продакшеном; правка USER-GUIDE не нужна, и ТЗ её обоснованно не требует |
+| `docs/TOUCH-SUPPORT.md` — editors best-effort на touch | §7 ТЗ требует не уменьшать существующие 44×44 px и не создавать horizontal overflow — это non-regression, а не новое обещание touch-паритета; согласуется с «editors are desktop-first, best effort on touch» |
+| §7.1 обязательные разделы | Все присутствуют под `## ТЗ`: сценарий, что человек увидит до/после, проблема, скоуп, не-скоуп, контракт поведения, UX и доступность, модель данных и миграция, i18n, критерии приёмки AC1–AC7 (у каждого — «Доказательство: …»), план автотестов, риски, откат, release-артефакты, плюс отдельный блок «Принято предположительно, можно менять свободно» |
+| Трек и критерий §5 | Аналитик прямо назвал нарушенный критерий (риск >3, desktop+mobile/touch, контракт на внешней оболочке HA) — не «обычный трек» без обоснования |
+| Продуктовые вопросы владельцу | Отсутствуют, и обоснованно: автор комментария подтвердил («onboarding/create уже входят в контракт #600, штатный fullscreen сохраняется»). Все технические развилки (`--ha-dialog-width-md` vs альтернативный публичный механизм, где именно задавать канву, имена diagnostic/smoke-файлов) вынесены в §15 как решения автора, не продуктовые |
+| Защитные AC и будущее код-ревью | АС6 сформулирован в терминах §2.7 («мутация делает свидетеля красным»), план автотестов п.3 заранее требует регистрации мутантов на потерю ширины/канвы/fullscreen — закрывает требование «AC · чем доказан · чем краснеет» до того, как код-ревью его спросит |
+
+## Находки
+
+### Low — строки `:558/574` для ветки `ha-dialog` в преамбуле устарели на ~8 строк
+
+Раздел «Что не так» (часть тела issue до `## ТЗ`, тоже входит в sha256-якорь)
+пишет: `Ветка ha-dialog (:558/574) использует width=medium (580 px)...`. На
+проверяемом SHA `84cffb3f` атрибут `width=${this.wide ? 'medium' : 'small'}`
+находится на строках **566** и **582**, а не 558/574. Причина — коммит
+`9bbf6fb2` («fix(dialog): restore HA modal after rejected close», #607),
+слитый в `dev` в тот же день, добавил в файл 8 строк (импорт `live`,
+комментарии к `_requestClose`/`rejectClose`) выше проверяемого блока; аудит
+#609, судя по всему, писался на снимке до этого слияния.
+
+По существу утверждение не пострадало: обе `<ha-dialog>`-ветки (describedBy и
+без него) по-прежнему не содержат ни одного form-shell-специфичного правила —
+только `width=`, `flexcontent`, `preventScrimClose` и aria-биндинги. Раздел
+`## ТЗ` (контракт поведения, AC1–AC7, план автотестов) номера строк вообще не
+использует — цитата живёт только в преамбуле и не является частью
+исполняемого контракта.
+
+**Снимается решением ревьюера с записью**, цикл не тратит: устаревшая
+строчная ссылка в диагностическом вступлении не меняет ни одного AC и не
+вводит разработчика в заблуждение — реализация будет читать живой код, а не
+цитату из issue. Автору стоит (не обязательно, при следующей правке тела
+issue) поправить `:558/574` → `:566/582`, чтобы преамбула не расходилась с
+дальнейшими код-ревью того же issue.
+
+### Low — план автотестов не называет явно «влияние на производительность: нет»
+
+DoR (`PROCESS.md` §2.5) требует, чтобы влияние на производительность и
+бюджеты было названо явно, включая явное «нет». Раздел «Риски» перечисляет
+пять рисков (CSS custom properties HA, padding, overflow/скроллер, mobile
+overrides не должны задеть generic-диалоги, забытый onboarding), но нигде в
+ТЗ нет отдельной строки о performance/bundle-budget — притом что правка чисто
+CSS-декларативная (новые селекторы и custom properties, ни одного нового
+вычисления в рантайме), риск объективно нулевой.
+
+**Снимается решением ревьюера с записью**: правка ограничена статическими
+CSS-правилами на уже существующих селекторах, `npm run bundle:budget`
+(входит в `gate:small`) и так поймает любой неожиданный рост бандла
+автоматически. Отсутствие явной строки не создаёт риска, который не был бы
+уже закрыт существующим гейтом — но пункт DoR формально не закрыт текстом, и
+автору стоит добавить одну фразу («Performance: нет — только CSS») при
+следующей правке тела issue, если такая случится по другой причине.
+
+## Что проверено и корректно
+
+- Оба продуктовых вопроса §7.1 (какая персона/поверхность/момент и что человек
+  увидит до/после) отвечены одной фразой без терминов реализации; персона и
+  задача из `docs/SCOPE.md` названы явно.
+- Контракт поведения (8 пунктов) закрывает все обсуждаемые в «Рисках» кейсы:
+  сжатие ширины на insufficient viewport, безопасная высота с учётом
+  safe-area, источник канвы и её токен, отсутствие двойного padding,
+  единственный владелец скролла в обеих ветках, точную границу ≤480 px и её
+  сосуществование со штатной HA-политикой `height ≤ 500px`, ограничение
+  правки атрибутом `form-shell` (явный негативный контроль generic-диалогов),
+  токены темы без хардкода цвета.
+- AC1–AC7 — у каждого назван способ доказательства (адресная диагностика на
+  закреплённой офлайн-фикстуре #505, дешёвый HA-подобный smoke с таблицей
+  пяти `data-kind` и отрицательным generic-контролем, golden 13 сцен,
+  фокусные smokes четырёх форм, мутанты на потерю ширины/канвы/fullscreen,
+  изоляционные проверки самой фикстуры). AC6 заранее сформулирован в
+  терминах будущего код-ревью («чем краснеет»), что закрывает требование
+  §2.7 до того, как код-ревью его спросит.
+- Скоуп/не-скоуп разделены по существу: содержимое форм, backend, миграции,
+  generic `hp-dialog`, приватный shadow DOM настоящего HA, штатная широкая-
+  но-низкая HA-fullscreen-политика — явно вне скоупа, что отсекает
+  расползание задачи на #600 повторно.
+- Технические решения (какой именно публичный механизм CSS-переменных
+  использовать, где физически задавать канву, имена diagnostic/smoke-файлов)
+  вынесены в явный блок §15 «принято предположительно» — ровно как требует
+  §7.1; продуктовых вопросов владельцу нет, и это обоснованно: все развилки
+  видимого поведения уже зафиксированы принятым #600 и текущим кодом.
+- i18n («новых строк нет»), модель данных/миграция («не нужна, изменение
+  представительное и откатываемое»), откат (revert коммита + пересборка
+  bundle-копий) закрыты явно, не пропущены молчанием.
+- Golden-риск для AC4 проверен по инфраструктуре, а не на честном слове:
+  demo/golden харнес нигде не регистрирует `ha-dialog`, поэтому изменения
+  CSS для этой ветки физически не могут задеть 13 принятых сцен.
+- `docs/USER-GUIDE.ru.md` уже описывает целевое поведение (560 px, один
+  скроллер) как факт — задача синхронизирует продакшен с уже документированным
+  контрактом, а не меняет его; корректно, что release-артефакты не требуют
+  правки USER-GUIDE.
+
+## Чего не проверял
+
+- Не проверял исполнением ни один тест/smoke/diagnostic — на этапе ревью ТЗ
+  кода задачи ещё нет, оценивался только текст спецификации против дерева на
+  `84cffb3f`.
+- Не проверял актуальность закреплённой версии `home-assistant-frontend==20260729.7`
+  против последней реальной версии HA — вне скоупа и ТЗ, и этого ревью
+  (обновление pin не входит в «Скоуп» задачи).
+- Не проверял `docs/ARCHITECTURE.md`/`docs/UX-MODES.md` построчно на предмет
+  описания модального контракта form-shell — целевой grep по `form-shell`
+  совпадений вне уже проверенных мест не дал; риска расхождения с
+  архитектурной документацией не нашёл, но не исключаю, что она просто не
+  описывает этот уровень детализации CSS.
+- Не оценивал итоговую визуальную корректность (действительно ли 560 px +
+  канва будут выглядеть идентично `pairs/room-light.png` в реальном HA) —
+  это предмет исполнения и последующего код-ревью с самой диагностикой, а не
+  текста ТЗ.
+
+## Вывод
+
+ТЗ полно по §7.1, каждый AC однозначен и снабжён способом доказательства,
+технические предположения явно выделены в §15 и не требуют решения
+владельца, продуктовые вопросы отсутствуют обоснованно. Фактические
+утверждения о коде проверены построчно на текущем SHA и совпадают, за
+единственным исключением — устаревшая ссылка на номера строк в преамбуле
+(Low, не в `## ТЗ`, не входит в исполняемый контракт). Риск для golden-набора
+(AC4) проверен по инфраструктуре и реально нулевой. Обе находки — Low,
+снимаются ревьюером с запиской, цикл не тратят.
+
+**Вердикт: зелёный.**
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `dev`, коммит `84cffb3f92bb` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Дерево материала: `3e2a1d462abbbb8e06d11d07851c6e928e814330`
+  ```
+  git log --all --format='%H %T' | grep 3e2a1d462abb
+  ```
+- Тело issue: `b08b9257ed688a47f3a0f422cdddc1bfc7cf7c511e81ada3ceeac8c2ce354d51`
+- Вердикт конвейера: `green` · High 0