diff --git a/docs/reviews/SPEC-REVIEW-406-r3.md b/docs/reviews/SPEC-REVIEW-406-r3.md new file mode 100644 index 00000000..581688e2 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-406-r3.md @@ -0,0 +1,235 @@ +# SPEC-REVIEW-406-r3 + +- Issue: #406 +- ТЗ: `docs/specs/406-beta2-polish.md`, ветка `issue/406-beta2-polish`, + ревизия 3, SHA `c43051ac` (тот же коммит, что назвал владелец в передаче на + ревью: «Head: `c43051ac`») +- Этап: spec (PROCESS.md §2.4) +- Заход: r3 · блокирующих циклов израсходовано 2 из 4 +- Вердикт: **жёлтый** + +## Почему разбор по дельте, а не заново + +Предыдущий вердикт (SPEC-REVIEW-406-r2) получен на SHA `0d5b52d1`. Этот SHA +существует на текущем дереве (`git cat-file -t 0d5b52d1` → `commit`), и +`git merge-base 0d5b52d1 origin/dev` равен `b04387ce` — тому же коммиту, что +и сам `origin/dev` сейчас: ветка dev не продвинулась с момента ребейза перед +r2, повторного ребейза между r2 и r3 не было. Это не случай §2.10 «ребейз на +ушедший вперёд dev». + +Дельта объявлена как `git diff 0d5b52d1..c43051ac` (плюс два новых файла +`docs/reviews/SPEC-REVIEW-406-r{1,2}.md`, не относящихся к ТЗ). Диапазон +`origin/dev...HEAD` по-прежнему состоит только из документов — продуктового +кода нет, гейты `tsc`/`test`/`build` неприменимы, тот же факт, что в r1/r2. + +Дельта в самом ТЗ ограничена: убран раздел (д) целиком, переписаны контракты +(б) и (в) (они образуют одну технически связанную развилку — HA-ветка против +нативной), скорректированы AC6–AC8, план автотестов, риски, откат, +release-артефакты и скоуп/не-скоуп в частях, которые ссылались на (д). Разделы +(а) и (г) в дифф не попали ни строкой. Контракт поведения меняется — именно +для (б)/(в), и именно поэтому ниже они разобраны заново по существу, а не +по формуле «раз тема та же — переносим вывод r2». (а) и (г) наследуются. + +## Скоуп + +Без изменений по существу, но пересчитан: было пять пунктов на трёх +поверхностях, стало четыре пункта на трёх поверхностях (словари i18n, +`hp-dialog`, снапшоты переезда area) — пункт (д) выведен из скоупа, так как +дефект уже устранён в #409 коммитом `8119c523` до того, как эта ревизия +дошла до ревью. Обоснование трека `small`/полный трек не изменилось. + +## Как проверялось + +Прочитаны `PROCESS.md` §2.4, §2.9, §2.10, §4, §7.1, §7.2; тело issue #406 и +все комментарии, включая передачу на повторное ревью revision 3 (описывает +оба исправления и три проверки: `check-docs.mjs`, `process-gate.mjs +--issues`, `git diff --check` — все green); `docs/reviews/SPEC-REVIEW-406-r1.md` +и `-r2.md` целиком. + +Само ТЗ вычитано целиком построчно на SHA `c43051ac`, не только дельта — +чтобы поймать противоречия между новым текстом (б) и разделами, которые дифф +не тронул (UX, Release-артефакты), что и дало находку №1 ниже. + +Специально для закрытия находки r2 №2 (механизм HA-ветки не назван) — +**внешняя проверка, не чтение только этого репозитория**: скачан реальный +исходник `home-assistant-frontend==20260729.7` (версия подтверждена +`tests_backend/requirements.txt:34`, комментарий там же объясняет, что она +берётся из констрейнтов закреплённого HA 2026.8.3, а не назначена вручную) — +`https://raw.githubusercontent.com/home-assistant/frontend/20260729.7/src/components/ha-dialog.ts`, +560 строк, прочитан целиком. Проверено построчно: + +- `@property({ attribute: 'aria-describedby' }) public ariaDescribedBy?: string` + (строка 92-93) действительно передаётся во внутренний `` (строка 168, внутри + `render()`, строки 158-224); +- `@property({ reflect: true }) public type: 'alert' | 'standard' = 'standard'` + (строка 98-99) отражается только на хосте (`grep -n "this\.type"` по всему + файлу — пусто вне JSDoc/деклараций свойства) и используется только в CSS + через `:host([type="standard"])` (строки 448, 454) — в `render()` тег + `` не получает от `this.type` ни атрибута, ни свойства, роль + нигде явно не устанавливается. + +Это ровно то, что ТЗ теперь утверждает буквально — не пересказ, а +подтверждённый цитированием факт. + +Дополнительно перечитаны `src/hp-dialog.ts` (весь файл, включая шапочный +doc-comment строки 24-31, стили строки 41-216, `render()` строки 437-482, +`connectedCallback` строки 231-242), `src/hp-confirm.ts` (весь файл, 72 +строки) и `src/houseplan-card.ts` вокруг `kind: 'warning'` — не для +подтверждения того, что уже подтвердили r1/r2 (это унаследовано), а чтобы +установить, действительно ли сегодня `hp-confirm` рендерится через HA-ветку в +реальном HA, — это стало основанием находки №1 ниже. + +## Закрытие раунда r2 + +| Находка r2 | Чем закрыта | Где видно | +|---|---|---| +| №1 (д) контракт описывает уже исправленный дефект, расходится с реально смёрженным поведением | Раздел (д) удалён из ТЗ целиком: заголовок, контракт, AC12/AC13 (были — теперь бюджет initial стал AC12), пункт плана автотестов «7. Приёмка с `replace: []`», мутант не заводился и не упоминается. «Не в скоупе» получил явную строку с точной причиной: «история screenshot-acceptance — дефект уже исправлен в #409 коммитом `8119c523`… #406 не меняет и не дублирует этот контракт» | `docs/specs/406-beta2-polish.md:176-190` (скоуп/не-скоуп), diff `0d5b52d1..c43051ac` показывает раздел (д) только в `-`-строках. Перепроверено: `grep -n "AC13\|AC14"` по текущему файлу — пусто; «Откат» пересчитан с пяти правок на четыре (`:321-326`); Release-артефакты — с «остальные четыре пункта» на «остальные три» (`:328-333`) | +| №2 (б) контракт не называет механизм, которым HA-ветка получает `alertdialog`/`aria-describedby` | Раздел (б) получил новый абзац с цитатой закреплённой версии `home-assistant-frontend` (строки 110-118) и переписанный практический абзац (строки 120-129): решение явное — `.ariaDescribedBy` есть и реально долетает до внутреннего диалога, но `type="alert"` роль не передаёт, поэтому alert-подтверждения **всегда** используют нативный ``, даже когда `ha-dialog` зарегистрирован; обычные диалоги вне `hp-confirm` — по-прежнему HA-ветка с `.ariaLabelledBy`/`.ariaDescribedBy`. AC6/AC7/AC8 (`:232-249`) и план автотестов (`:266-273`) переписаны под эту развилку | Независимо перепроверено чтением реального `ha-dialog.ts` тега `20260729.7` (раздел «Как проверялось» выше) — утверждение ТЗ о поведении стороннего компонента подтверждено, а не принято на слово | + +Обе находки r2 закрыты по существу. Разбор (б)/(в) по факту закрытия вскрыл +новую находку — она не переоткрывает находки r1/r2, это независимый дефект, +ставший видимым только после того, как r3 назвал конкретный механизм +(до этого, в r1/r2, контракт был расплывчат, и вопрос «что увидит +пользователь» было не к чему привязать). + +## Унаследовано из r2 + +Разделы (а) «Мёртвые строки в словарях» и (г) «Снапшот копит записи +исчезнувших устройств» дельтой не задеты (не встречаются ни в одной `-`/`+` +строке `git diff 0d5b52d1..c43051ac`, кроме одного упоминания (а) в +перечислении «Приоритет»/«Сценарий», которое не меняет контракт). Их +фактическая проверка (числа по словарю, 13 мёртвых ключей, 19 живых +`*.help.aria`, коллизия с `test/unified-wall-tool-source.test.mjs`, механика +`resolveDeviceAreaRelocations`/`markerAreaSnapshotOf`, номера строк) принята +без повторной перепроверки из `docs/reviews/SPEC-REVIEW-406-r2.md`, разделы +«Что проверено и корректно», на SHA `0d5b52d1`. + +Также унаследована структурная оценка §7.1 (все обязательные разделы +присутствуют, продуктовых технических вопросов владельцу не вынесено, +легитимность полного трека по `docs/SCOPE.md`) — она подтверждена в r2 и не +пересчитывалась заново построчно для разделов, которых дельта не касается; +для (б)/(в)/скоупа/UX/release-артефактов она перепроверена заново (см. ниже). + +## Находки + +### [Medium, в скоупе] №1 — UX и Release-артефакты утверждают «оформление не меняется», хотя раздел (б) в этой же ревизии вводит видимый пользователю переход confirm-диалогов с HA-оформления на нативное + +**Файл**: `docs/specs/406-beta2-polish.md`, раздел (б) «Практически» (строки +120-129) против раздела «UX» (строки 192-197) и «Release-артефакты» (строки +328-333). + +**Что не так**: раздел (б) в этой ревизии впервые называет конкретный +механизм (закрытие находки r2 №2) — и этот механизм состоит в том, что +`hp-confirm` (все виды: и `destructive`, и `warning`) **больше не следует** +общему правилу `hp-dialog._useHaDialog = !!customElements.get('ha-dialog')` +(`hp-dialog.ts:240`), а принудительно рендерит нативный `` даже когда +`ha-dialog` зарегистрирован (строки 124-127: «hp-dialog намеренно выбирает +нативный `` даже при наличии `ha-dialog`»). + +Сегодня, до этой задачи, `hp-confirm.ts` не делает никакого выбора — он +безусловно передаёт `` (`hp-confirm.ts:36-61`), и собственный +doc-comment `hp-dialog.ts:24-31` прямым текстом говорит: «Home Assistant +provides the visual surface and focus trap through ha-dialog. The native +dialog branch keeps the standalone demo usable without mocking HA frontend +internals» — то есть в реальном Home Assistant (где `ha-dialog` всегда +зарегистрирован) все семь `destructive`-подтверждений и один `warning` +сегодня рендерятся через `ha-dialog`: собственный заголовок HA +(`ha-dialog-header`), анимации, `wa-dialog`-обвязку. Нативная ветка сегодня — +исключительно путь standalone-демо без HA. Проверено также, что в коде нет ни +одного прецедента принудительного игнорирования `_useHaDialog` (`grep -rn +"_useHaDialog" src/*.ts` — пять использований, ни одно не форсирует ветку +для конкретного компонента) — то есть это решение (б) вводит новый класс +поведения, а не продолжает существующий. + +После задачи это же множество диалогов — каждое подтверждение удаления и +единственное подтверждение разблокировки замка — у **каждого реального +пользователя** Home Assistant сменит визуальную оболочку с HA-хрома на +собственную вёрстку `hp-dialog` (`.header`/`.close`/`.surface`, +строки 127-216 того же файла): другой DOM/CSS-путь заголовка, другая +анимация появления, другой фокус-трап (нативный `` вместо +`wa-dialog`). Цвета границы/фона/скругления действительно частично совпадают +(`--hp-accent`, `--card-background-color`, `--rad-l` заданы для обеих веток), +но это не то же самое, что «оформление не меняется» — структура заголовка, +анимация и поведение фокуса всё равно другие. + +При этом раздел «UX» (не менявшийся ни в одной из трёх ревизий, `git diff` +подтверждает отсутствие правок этого раздела за весь трек) заявляет: +«Оформление не меняется. Меняется то, что слышит пользователь скринридера…» +— и раздел «Release-артефакты» относит к User-Visible только факт объявления +скринридером, помечая остальные три пункта внутренними, а строку +«Скриншоты не меняются: диалог в статике не открыт» приводит как +единственное следствие для документации. + +**Сценарий отказа**: реализация по тексту ТЗ корректно уходит с HA-хрома на +нативный `` для всех alert-подтверждений (AC6/AC8 зелёные), но +CHANGELOG не получает пункта об этом (Release-артефакты явно требуют только +одну строку — про скринридер), а автор код-ревью следующего раунда, доверяя +разделу UX буквально, не станет искать визуальный регресс на реальном HA — +хотя пользователь при первом же «Удалить комнату?» увидит непривычно +выглядящий диалог вместо знакомого HA-стиля. Отдельный риск: если +несоответствие всплывёт в проде, это будет воспринято как баг (незапланированный +визуальный дрейф), хотя по факту это — осознанное, но нигде не заявленное +инженерное решение. + +**Ожидаемо**: раздел «UX» и «Release-артефакты» приведены в соответствие с +разделом (б) — либо явно признаётся и документируется видимая смена +оформления confirm-диалогов (HA-хром → нативный `hp-dialog`) как +User-Visible-пункт в обоих CHANGELOG, либо владелец фиксирует иной баланс +(например, принять расхождение только для `warning`, а не для всех +`destructive`, если визуальная консистентность важнее для частых операций) — +но текущее молчаливое «не меняется» рядом с разделом, который явно описывает +смену ветки рендера, оставлять нельзя. + +## Что проверено и корректно + +- **Обе находки r2 закрыты по существу**, не косметически — подробности в + разделе «Закрытие раунда r2» выше; находка №2 закрыта с независимой + внешней проверкой (реальный исходник закреплённой версии + `home-assistant-frontend`, а не доверие цитате в ТЗ). +- **§7.1, обязательные разделы** — все присутствуют и после сокращения + документа: сценарий, что человек увидит до/после, проблема+контракт по + каждому из четырёх пунктов, скоуп/не-скоуп, UX, модель данных и миграция, + i18n, AC1-AC12 с доказательством, план автотестов, риски, откат, + release-артефакты. +- **Внутренняя согласованность после удаления (д)**: нет висячих ссылок на + AC13/AC14, счётчики «пять правок» → «четыре», «остальные четыре пункта» → + «остальные три» пересчитаны верно, мутант и юнит-пункт плана, + относившиеся к (д), убраны вместе с разделом, а не оставлены сиротами. +- **AC6-AC8 и обновлённый план автотестов внутренне непротиворечивы**: они + корректно описывают именно ту развилку, которую называет обновлённый (б) — + alert всегда нативный, обычные диалоги вне `hp-confirm` — всегда HA-ветка + при её наличии. +- **Продуктовых технических вопросов владельцу не вынесено** — как и в r1/r2. +- **Три проверки автора** (`check-docs.mjs`, `process-gate.mjs --issues`, + `git diff --check`), названные в передаче на ревью, — дешёвые, применимые к + doc-only дереву; не перепроверял отдельно, так как они не относятся к + предмету находок этого раунда (документная гигиена, не контракт). + +## Чего не проверял + +- Гейты `tsc`/`test`/`build` — не гонял: диапазон `origin/dev...HEAD` + по-прежнему состоит только из doc-файлов (три `.md`), продуктового кода + нет. То же основание, что в r1/r2. +- Не пересчитывал заново инвентарь мёртвых ключей (а) и механику снапшотов + (г) — унаследовано из r2 (SHA `0d5b52d1`), дельта их не касается (см. + раздел «Унаследовано»). +- Не проверял поведение реального `wa-dialog` (`@home-assistant/webawesome`) + на предмет того, передаёт ли *он сам* `role` от `type`, помимо того, что + `ha-dialog.ts` этого не делает явно, — для контракта (б) достаточно + доказать, что `ha-dialog.ts` не передаёт `type`/`role` внутрь, а глубже в + сторонний пакет ТЗ и не заходит (сознательно, см. риск «#406 не патчит + shadow DOM»). +- Не проверял субъективную величину визуального расхождения между HA-хромом + и нативной веткой на глаз (скриншот/рендер) — вывод находки №1 основан на + структурном сравнении кода (разные DOM-пути, разный фокус-трап, разный + элемент заголовка), не на визуальном сравнении пикселей; для целей этого + ревью (обнаружить неучтённое расхождение с разделом UX) этого достаточно, + количественная оценка — дело код-ревью или ручного смотра при реализации. +- Не проверял `scripts/mutation-gate.mjs` на техническую реализуемость трёх + заявленных мутантов — на этапе ТЗ это описание намерения, не код; та же + позиция, что в r1/r2. + +## Вердикт + +Вердикт: жёлтый · заход r3 · блокирующих циклов 2/4 · High: 0 · Medium: 1 → в задаче