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

SPEC-REVIEW-609-r1

Скоуп

Issue #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, снимаются ревьюером с запиской, цикл не тратят.

Вердикт: зелёный.

---

Материал раунда

• Ветка: dev, коммит 84cffb3f92bb — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: 3e2a1d462abbbb8e06d11d07851c6e928e814330

  git log --all --format='%H %T' | grep 3e2a1d462abb
  

• Тело issue: b08b9257ed688a47f3a0f422cdddc1bfc7cf7c511e81ada3ceeac8c2ce354d51
• Вердикт конвейера: green · High 0