mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -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) действительно передаётся во внутренний `<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 → в задаче
|
||||
Reference in New Issue
Block a user