docs: review document for #508

Issue: #508
User-Visible: no

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

@@ -0,0 +1,195 @@
+# SPEC-REVIEW-508-r1
+
+- Issue: #508 «Сводная панель: в диалоге настроек не работает прокрутка — колесом мыши и touch на мобильных»
+- Этап: S4-spec-review (PROCESS.md §2.4)
+- Трек: `small` (лёгкий) — ТЗ живёт в теле issue, файл в `docs/specs/` не создаётся
+- Заход: r1 · блокирующих циклов ревью ТЗ израсходовано 0 из 2 (лимит лёгкого трека, §4)
+- Материал: тело issue #508 + комментарий S2 от 09.09 (аналитика + ТЗ, автор Codex)
+- Ревьюер: Claude (сессия ревью ТЗ), другая роль/модель, чем автор
+
+## Скоуп
+
+Диалог настроек сводной панели (`hp-dialog[data-kind="summary"]`) не прокручивается
+внутри реальной HA (`ha-dialog`/`wa-dialog`) ни колесом, ни тачем, хотя на демо-стенде
+(нативная ветка `<dialog>`) работает. ТЗ предлагает точечный фикс: новый opt-in атрибут
+`flex-content` на `hp-dialog`, форвардящийся как `?flexcontent` на `ha-dialog` в обеих
+HA-ветках рендера; диалог настроек панели получает этот атрибут, остальные — нет.
+Продуктовая ценность — J5/J1 (панель обязана быть управляемой с тем же UX, что и
+остальные View-поверхности); поверхность одна (`src/hp-dialog.ts`,
+`src/summary-panel-editor.ts`), миграции конфига нет, i18n не тронут. Критериям
+лёгкого трека (§5) формально соответствует.
+
+## Как проверялось
+
+Это ревью ТЗ, кода ещё нет (issue в `S4-spec-review`, S5/S6 впереди) — гейты
+`typecheck`/`test`/`build` к этому этапу не относятся, не прогонялись. Проверка —
+чтением: `docs/SCOPE.md`, `docs/TOUCH-SUPPORT.md`, `PROCESS.md` §1, §2.4, §2.5, §5,
+§7.1; чтением исходников, на которые ссылается ТЗ (`src/hp-dialog.ts` — обе ветки
+`ha-dialog` и нативная ветка `<dialog>`/`.surface`/`.content`; `src/summary-panel-editor.ts`,
+`src/summary-panel-editor-style.ts`); проверкой существования файлов и артефактов,
+которые ТЗ называет как доказательство (`demo/smoke_summary_panel_polish.mjs`,
+`demo/golden/baselines/*`, `demo/capture_summary_panel_505.mjs`,
+`demo/helpers/ha-dialog-fixture.mjs`); поиском текста «footer»/«#505» по `docs/*.md`,
+поиском `flexContent`/`flexcontent` по репозиторию (не встречается — новая
+сущность, конфликта имён нет).
+
+## Находки
+
+### Medium (в скоупе задачи — чинится автором в этом же ТЗ)
+
+**M1. AC3 называет несуществующее доказательство: golden-сценариев `summary-*` в
+репозитории нет.**
+
+- Файл: тело issue #508, раздел «AC», пункт AC3.
+- Формулировка: «Нативная ветка (демо) не меняется: `smoke_summary_panel_polish` и
+  golden `summary-*` без отличий.»
+- Проверено: `ls demo/golden/baselines | grep -i summary` → 0 совпадений; ни одного
+  golden-сценария с префиксом `summary` не существует нигде в `demo/golden/`.
+  Единственный скрипт, который вообще снимает пиксели диалога настроек панели —
+  `demo/capture_summary_panel_505.mjs`, и его собственный комментарий в первой
+  строке файла прямо говорит: «Explicit #505 diagnostic evidence, not a
+  golden-baseline producer or smoke». То есть покрытия golden-эталонами у этого
+  диалога нет вовсе — это разовый диагностический скрипт с pinned HA wheel,
+  осознанно выведенный из обычных гейтов (тяжёлая загрузка ~124 МБ).
+- Почему это находка, а не мелочь: команда `npm run golden:verify` (или её часть,
+  отфильтрованная по несуществующему префиксу `summary-*`) в этом случае сравнит
+  ноль сценариев и вернёт «нет отличий» **тривиально**, а не потому что поведение
+  проверено. Это ровно тот антипаттерн, о котором PROCESS.md §2.7 говорит для
+  код-ревью («тест умеет падать» без свидетеля) — только сдвинутый на этап ТЗ: AC,
+  который выглядит проверяемым, ссылается на проверку, которая ничего не
+  проверяет. Код-ревьюер следующего раунда рискует принять «прогнал golden,
+  summary-* чисто» за доказательство, не заметив, что сравнивать было нечего.
+- Что нужно поправить (одно из): (а) убрать golden-часть из AC3 и оставить
+  доказательством только `smoke_summary_panel_polish` (у него, судя по коду,
+  и так есть проверки, что нативная ветка ведёт себя ожидаемо); либо (б) явно
+  снять AC3 до «golden-матрица не даёт отличий» без привязки к
+  несуществующему префиксу — тогда это то же самое, что говорит AC4, и дублирует
+  его без потери смысла. Технический выбор — за автором.
+
+**M2. В ТЗ лёгкого трека нет раздела «откат».**
+
+- Файл: тело issue #508, весь раздел «## ТЗ (лёгкий трек, после S2 09.09)».
+- PROCESS.md §5 задаёт для лёгкого трека фиксированный шаблон тела issue:
+  «проблема · контракт · AC1…ACn с доказательством · **откат**» — откат назван как
+  обязательный, наравне с остальными тремя частями, и не отменяется нигде в §5
+  для этого случая. ТЗ содержит «Причина / Изменения / Тесты / AC», но ни одной
+  фразы о том, как вернуть поведение назад, если фикс скажется неожиданно (флага
+  Labs это не касается — правки штатные, поэтому откат тут дёшев, но он должен
+  быть **назван**, а не подразумеваться).
+- Чем это грозит: это ровно тот пункт DoR (§2.5), который проверяется перед
+  переводом в `S5-ready» — «откат: как выключить или вернуть назад». Без явной
+  строки в ТЗ переход в `S5-ready` формально не проходит чек-лист.
+- Фикс дешёвый: одна фраза, например «откат — обычный ревёрт коммита; атрибут
+  `flex-content` opt-in только у диалога настроек панели, у остальных `hp-dialog`
+  поведение не меняется вообще».
+
+### Low (на усмотрение автора/ревьюера — не блокирует)
+
+**L1. Целевой файл документации для фразы про футер не определён.**
+Пункт 3 «Изменений» называет цель как «`docs/UX-MODES.md` (или где описан контракт
+#505 про футер)». Поиском по `docs/*.md` такого текста про футер сейчас нигде нет
+(проверено `grep -rn footer docs/*.md | grep -i summary` → пусто); ближайшее по духу
+место — `docs/ARCHITECTURE.md:1983` («The #505 designer-aligned surface…»), где уже
+описана техническая сторона диалога настроек. Не блокирует: раздел «Всё, чего
+пользователь не наблюдает, агенты решают сами» (`PROCESS.md §7.1`) явно относит
+«где лежит документация» к техническим решениям автора, а не к продуктовому
+вопросу владельцу. Снимаю с записью: автор выбирает файл сам при реализации.
+
+**L2. Влияние на производительность не названо явно.**
+DoR (§2.5) требует явного «нет», если влияния нет; ТЗ не произносит эту фразу.
+Не блокирует: правка — булев CSS-атрибут на существующем диалоге, ничего не
+добавляет в горячий путь рендера/анимации панели, перф-риска по чтению кода не
+вижу. Снимаю с записью — автору стоит добавить одну строку «перф: нет влияния»
+при реализации, чтобы чек-лист DoR не спотыкался формально.
+
+## Что проверено и корректно
+
+- **Причинно-следственная цепочка фикса правдоподобна и подтверждена не только
+  рассуждением.** Автор S2-комментария воспроизвёл баг на реальном стенде
+  (ha.jbstudio.pro, HA 2026.9.1), измерил `clientHeight`/`scrollHeight` реального
+  `.body` внутри `ha-dialog`/`wa-dialog» и отдельно подтвердил механизм
+  (`overflow-y:auto` + `overscroll-behavior:contain` на контейнере без
+  ограниченной высоты обрывает scroll chaining в Chromium) изолированной
+  Playwright-страницей. Это не догадка, выданная за факт — гипотеза 1
+  (перехват `wheel` обработчиками панели) явно проверена и снята с указанием
+  файлов и строк (`summary-panel-runtime-loaded.ts:207,236`).
+- **Независимое подтверждение в уже существующем коде.** `demo/capture_summary_panel_505.mjs`
+  (написан раньше, для #505) уже содержит комментарий «The genuine HA owns
+  scrolling in its shadow `.body`; the native wrapper owns it in the editor.
+  Follow the composed tree instead of assuming» — то есть проблема с тем, что
+  `.body` владеет скроллом в реальной HA, была на радаре ещё на #505. Диагноз
+  ТЗ #508 согласуется с этим, а не противоречит.
+- **Структура рендера в `src/hp-dialog.ts` соответствует описанной в ТЗ.** Обе
+  HA-ветки (`describedBy` есть/нет, строки 504–517 и 519–531) и нативная ветка
+  (534–557) — ровно то, что называет ТЗ. Нативная ветка действительно уже
+  ограничена по высоте (`.surface{max-height:92vh;overflow:hidden}` →
+  `.content{display:flex;flex-direction:column}` → слотированный
+  `.summary-editor` как единственный скроллер) — заявление «нативной ветке
+  ничего не нужно» подтверждается чтением, а не на слово.
+  Атрибут `flexContent`/`flexcontent` нигде в репозитории пока не существует —
+  конфликта имён с уже используемым нет.
+- **Тесты спроектированы с учётом «тест должен уметь падать».** Пункт (в) в
+  разделе «Тесты» — отдельный сценарий «без `flex-content` у стаба скролл не
+  идёт» — это ровно негативный свидетель, доказывающий, что стенд
+  воспроизводит баг, а не только то, что фикс включён. Оба протективных
+  AC (AC1, AC2) получают названные мутанты
+  (`summary-dialog-drops-flex-content`, `hp-dialog-ignores-flex-content»),
+  что заранее закрывает требование PROCESS.md §2.7 про мутант в
+  `scripts/mutation-gate.mjs» для защиты, проверяемой дорогим (браузерным)
+  гейтом — это сделано на этапе ТЗ, до того как это стало находкой код-ревью.
+- **AC4 и golden-матрица.** В отличие от AC3 (M1), формулировка AC4 «прочие
+  `hp-dialog` без `flex-content` не меняют раскладку (golden-матрица без
+  отличий)» опирается на реально существующий и достаточно широкий набор
+  golden-эталонов с диалогами (`device-dialog-mobile-ru.png`,
+  `optimize-preflight-dialog-light-ru.png`, `tray-*` и др.) — здесь
+  доказательство существует и полный прогон `npm run golden:verify` его
+  даст. Претензия M1 относится только к AC3, не к AC4.
+- **Соответствие `docs/SCOPE.md` и `docs/TOUCH-SUPPORT.md`.** Диалог настроек
+  панели — гарантированная View-поверхность на touch
+  (`TOUCH-SUPPORT.md:48-49`: «The summary panel, both halves of its control,
+  and its simple settings form are supported View surfaces»), поэтому touch-часть
+  AC2 не расширяет скоуп задачи произвольно, а чинит уже гарантированный
+  контракт — включение touch в задачу правомерно, а не самодеятельность
+  автора.
+- **Скоуп не размыт.** Правка ограничена одним атрибутом на одном диалоге;
+  остальные `hp-dialog` явно объявлены неприкосновенными (AC4), новой
+  подсистемы или контракта поведения не появляется.
+- **Трек `small` выбран корректно**: одна поверхность, нет миграции конфига,
+  нет новых i18n-ключей, нет нового UX-контракта (восстанавливается уже
+  обещанный), touch — часть уже гарантированного, а не нового контракта.
+  Названного нарушенного критерия для полного трека нет — и это правильно,
+  такого критерия действительно не видно.
+- Открытых продуктовых вопросов к владельцу нет — весь материал технический
+  и решается автором/ревьюером (что и делает это ревью).
+
+## Чего не проверял
+
+- Не запускал никаких гейтов (`typecheck`/`test`/`build`/`golden`/`smoke`) —
+  на этапе ревью ТЗ кода ещё нет, гейты к этому этапу не относятся.
+- Не проверял факт существования атрибута `flexcontent` у реального `ha-dialog`/
+  `wa-dialog` независимо от стенда — полагаюсь на воспроизведённое автором
+  измерение на живом HA 2026.9.1 (S2-комментарий); отдельного подтверждения из
+  исходников самой HA-фронтенд-сборки в этом репозитории нет и быть не может
+  (внешняя зависимость, pinned wheel только в диагностическом
+  `demo/capture_summary_panel_505.mjs`, не в этом дереве).
+- Не оценивал качество формулировок будущего юнит-теста
+  `test/hp-dialog-contract.test.mjs` — файла ещё нет, ТЗ описывает его
+  проверяемым способом (source-text на оба рендера), к тексту претензий нет.
+
+## Вердикт
+
+Находок High нет. Medium — 2, обе в скоупе задачи, обе дёшево чинятся в теле
+issue до перевода в `S5-ready` (без нового цикла с моей стороны по существу —
+но формально возврат на правки бюджет §4 тратит, раз это не зелёный вердикт).
+
+Вердикт: жёлтый · заход r1 · блокирующих циклов 1/2 · High: 0 · Medium: 2 → в задаче
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `dev`, коммит `` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Якоря снять не удалось: ветки задачи нет, материал читался по `dev`.
+- Вердикт конвейера: `yellow` · High 0