Files
houseplan-card/docs/reviews/SPEC-REVIEW-406-r3.md
2026-09-01 16:32:25 +00:00

23 KiB
Raw Permalink Blame History

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) действительно передаётся во внутренний <wa-dialog aria-describedby=${ifDefined(this.ariaDescribedBy)}> (строка 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() тег <wa-dialog> не получает от 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-подтверждения всегда используют нативный <dialog role="alertdialog">, даже когда 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), а принудительно рендерит нативный <dialog> даже когда ha-dialog зарегистрирован (строки 124-127: «hp-dialog намеренно выбирает нативный <dialog role="alertdialog"> даже при наличии ha-dialog»).

Сегодня, до этой задачи, hp-confirm.ts не делает никакого выбора — он безусловно передаёт <hp-dialog> (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-путь заголовка, другая анимация появления, другой фокус-трап (нативный <dialog> вместо wa-dialog). Цвета границы/фона/скругления действительно частично совпадают (--hp-accent, --card-background-color, --rad-l заданы для обеих веток), но это не то же самое, что «оформление не меняется» — структура заголовка, анимация и поведение фокуса всё равно другие.

При этом раздел «UX» (не менявшийся ни в одной из трёх ревизий, git diff подтверждает отсутствие правок этого раздела за весь трек) заявляет: «Оформление не меняется. Меняется то, что слышит пользователь скринридера…» — и раздел «Release-артефакты» относит к User-Visible только факт объявления скринридером, помечая остальные три пункта внутренними, а строку «Скриншоты не меняются: диалог в статике не открыт» приводит как единственное следствие для документации.

Сценарий отказа: реализация по тексту ТЗ корректно уходит с HA-хрома на нативный <dialog> для всех 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 → в задаче