docs: review document for #406

Issue: #406
User-Visible: no

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

@@ -0,0 +1,198 @@
+# SPEC-REVIEW-406-r1
+
+- Issue: #406
+- ТЗ: `docs/specs/406-beta2-polish.md`, ветка `issue/406-beta2-polish`, SHA `8248f9e4`
+- Этап: spec (PROCESS.md §2.4)
+- Заход: r1 · блокирующих циклов израсходовано 0 из 4
+- Вердикт: **жёлтый**
+
+## Скоуп
+
+Пять независимых мелочей (полный трек, критерий «одна поверхность» для `small`
+не выполняется — согласен, обоснование корректно и подкреплено прецедентами
+#385/#400):
+
+- (а) мёртвые ключи `confirm.*` и смежных семейств в 4 словарях + гейт;
+- (б) роль `alertdialog`/`aria-describedby` в `hp-dialog`/`hp-confirm`;
+- (в) смок HA-ветки диалога подтверждения;
+- (г) уборка `marker_area_snapshot` для исчезнувших устройств + порядок
+  обрезки при лимите;
+- (д) сохранение `acceptance.declared` при пустой замене в `docs-accept.mjs`.
+
+## Как проверялось
+
+Ревью аналитическое (этап spec, кода ещё нет). Читал тело issue #406, оба
+комментария владельца (S2-разбор, ссылка на ТЗ), сам файл ТЗ на SHA `8248f9e4`,
+`PROCESS.md` (§2.4, §7.1, §7.2), `docs/SCOPE.md`. Затем **перепроверил
+фактические утверждения ТЗ по текущему коду** (`git archive` того же SHA,
+`src/i18n/en.json`, `src/hp-dialog.ts`, `src/hp-confirm.ts`,
+`src/danger-confirm.ts`, `src/device-area-relocation.ts`,
+`scripts/docs-acceptance.mjs`, `demo/smoke_*`), поскольку ТЗ строит контракт
+(а) на конкретных числах и списке ключей, а разошедшееся число здесь — не
+стилистика, а входные данные для гейта, который эта же задача пишет.
+
+Обязательные разделы §7.1 присутствуют все: сценарий · что человек увидит до/
+после · проблема+контракт (по каждому из пяти пунктов) · скоуп/не-скоуп · UX ·
+модель данных и миграция · i18n · AC1…AC14 с доказательством · план автотестов
+· риски · откат · release-артефакты.
+
+## Находки
+
+### [Medium, в скоупе] (а) Инвентарь мёртвых ключей `*.help.aria` неверен — все 19, а не 6 из 19, используются
+
+**Файл**: `docs/specs/406-beta2-polish.md`, раздел «(а) Мёртвые строки в
+словарях», строка со списком «13 ключей `*.help.aria`».
+
+**Что не так**: ТЗ утверждает, что из ключей семейства `*.help.aria` 13 (из 19
+в словаре) — «задел, не подключённый ни одним потребителем». Проверка кода на
+том же SHA это опровергает: **все 19** ключей `*.help.aria` в `en.json`
+используются — не литералом, а через паттерн `` `${key}.aria` `` в
+`_help(key)` (`houseplan-editor-runtime.ts:1194`: `const ariaKey =
+\`${key}.aria\``). Для каждого из 19 базовых `*.help` ключей нашёлся ровно один
+литеральный вызов `this._help('xxx.help')` в `houseplan-editor-runtime.ts`
+и/или `houseplan-onboarding-runtime.ts` (`space.cell_cm.help`,
+`space.zero_wall_style.help`, `space.fill_mode.help`,
+`device_inbox.show_hidden.help`, `gs.glow_radius.help`, `marker.controls.help`,
+`marker.glow_radius.help`, `marker.value_badge.help`,
+`marker.value_badge_source.help`, `marker.value_badge_position.help`,
+`marker.value_source.help`, `marker.light_role.help`,
+`marker.light_entity.help`, `marker.toggle_entity.help`,
+`marker.glow_mode.help`, `gs.bg_mode.help`, `gs.north.help`,
+`space.bg_mode.help`, `space.north.help`) — то есть ровно тот самый паттерн
+«динамический суффикс от литерального ключа», который ТЗ признаёт законным
+классом (`furn.cat_${id}` и «48 семейств»), но почему-то не признал для этого
+конкретного случая.
+
+Отдельно: список «Остальные двадцать три» в этом же разделе перечисляет только
+19 позиций (13 `.help.aria` + `marker.display_hint` + `marker.display_hint_icon`
++ `history.delete_room` + `markup.delete` + `title.markup` +
+`history.partition_add` = 19), а не 23 — четыре ключа из заявленных 23 нигде не
+названы.
+
+**Сценарий отказа**: если реализация будет опираться на нарратив ТЗ (список
+«30 доказанно мёртвых ключей», явно включающий 13 `.help.aria`) вместо
+самостоятельной перепроверки, из словарей будут удалены 13 реально
+показываемых строк подсказки-для-скринридера в панели настроек — то есть
+задача, соседний пункт которой (б) *чинит* доступность для скринридера,
+пунктом (а) её же и *поломает*. Если же реализация полагается только на
+механический гейт (AC1) — риск переносится на сам гейт: правило распознавания
+«динамических семейств» (AC3) не называет паттерн `${key}.help → ${key}.aria`
+явно (назван только пример `furn.cat_${id}`), и создатель гейта имеет
+основания не покрыть его — а именно это и произошло у автора ТЗ при ручном
+аудите.
+
+**Ожидаемо**: пересчитать список действительно мёртвых ключей раздела (а)
+(проверено выше: 6 из 19 `confirm.*`-ключей подтверждаются, `marker.
+display_hint*`, `history.delete_room`, `markup.delete`, `title.markup`,
+`history.partition_add` подтверждаются — эти 12 факт-чекнуты и мертвы; ни один
+`*.help.aria` мёртвым не подтверждён при ручной проверке) и явно включить
+правило «литеральный вызов `_help('x.help')` делает живыми оба ключа, `x.help`
+и `x.help.aria`» в контракт AC3 как отдельно названное семейство, а не
+подразумевать его в «и прочие 48».
+
+### [Medium, в скоупе] (б) AC6/AC7 не решают роль для `HpConfirmKind.warning` (диалог разблокировки)
+
+**Файл**: `docs/specs/406-beta2-polish.md`, раздел «(б) Роль диалога и текст
+последствий», AC6/AC7.
+
+**Что не так**: `hp-confirm` — общий компонент для двух видов запросов,
+`kind: 'destructive'` (используется 7 раз, все — удаления) и `kind: 'warning'`
+(используется ровно один раз — подтверждение разблокировки двери,
+`houseplan-card.ts:13001-13006`, с собственным текстом последствий
+`confirm.unlock_body`). Контракт ТЗ формулирует дихотомию «destructive →
+alertdialog» / «обычный → dialog», а в качестве примера «обычных» диалогов
+называет только диалоги, вообще не проходящие через `hp-confirm` (маркер,
+калибровка пылесоса). Про `kind: 'warning'` контракт молчит: буквальное
+прочтение AC6 («Подтверждение **разрушающего** действия объявляется как
+alertdialog») позволяет реализовать признак как `destructive =
+request.kind === 'destructive'` и оставить диалог разблокировки на
+`role="dialog"` без `aria-describedby` — то есть ровно тот же дефект, ради
+починки которого заведён пункт (б), сохранится на диалоге, который
+`docs/SCOPE.md` называет единственной санкционированной поверхностью опасного
+действия (лок/разлок).
+
+**Сценарий отказа**: AC6 и AC7 оба зелёные (пройдены буквально по тексту), но
+пользователь со скринридером по-прежнему не услышит «Устройство разблокируется»
+при нажатии Unlock — потому что это `warning`, а не `destructive`, и контракт
+не сказал, что с ним делать.
+
+**Ожидаемо**: явное решение в контракте (б) — получает ли `kind: 'warning'`
+тот же `alertdialog` + `aria-describedby`, что и `destructive` (агент вправе
+решить сам, это не видимая пользователю форма, а объявляемая роль ARIA, но
+решение должно быть явным и внесено в AC6/AC7, а не оставлено читателю
+угадывать по бинарной формулировке), и соответствующий пункт в AC8/плане
+автотестов, если ветка `warning` должна тоже проверяться смоком.
+
+## Что проверено и корректно
+
+- **Числа по словарю в целом**: 1201 ключ в `en.json` — подтверждено точным
+  подсчётом. 7 ключей `confirm.*` (delete_draft, delete_draft_segment,
+  delete_plan, delete_room, delete_space, remove_marker, unlock) — подтверждено
+  отсутствием любого литерального использования в `src/*.ts` на SHA
+  `8248f9e4`. `marker.display_hint`, `marker.display_hint_icon`,
+  `history.delete_room`, `markup.delete`, `title.markup`,
+  `history.partition_add` — подтверждено тем же способом, действительно мертвы.
+- **(а) коллизия с `test/unified-wall-tool-source.test.mjs`** — реальна и
+  корректно описана как открытая: тест требует `history.partition_add`, ключ
+  в `src/` не используется; ТЗ явно требует разрешить конфликт, а не тихо
+  оставить.
+- **(в) HA-ветка не покрыта** — подтверждено: `smoke_free_walls.mjs` — 
+  единственный смок, определяющий `ha-dialog`, и он стабит `_confirmDanger`,
+  так что сам диалог не рисуется; в `smoke_danger_confirmation.mjs` и
+  `smoke_danger_confirm_branches.mjs` слова `ha-dialog` нет. Прецедент стаба
+  (`smoke_free_walls.mjs:20-22`), на который ссылается AC8, реален и рабочий.
+- **(г) утечка снапшота** — подтверждено чтением `device-area-relocation.ts`:
+  `resolveDeviceAreaRelocations` строит `decisions` только по
+  `options.devices`; устройство, отсутствующее в этом списке целиком, никогда
+  не попадёт ни в `decisions`, ни, соответственно, в `removeSnapshot` — запись
+  `marker_area_snapshot[id]` переживёт его исчезновение бессрочно.
+  `removeMarkerAreaSnapshots` действительно вызывается только из ручного
+  редактирования/удаления маркера (`houseplan-editor-runtime.ts:8326,8448`) —
+  номера строк точны. Термин «авторитетный реестр» — не изобретение ТЗ, это
+  существующее в коде понятие (`device-area-relocation.ts:50,133,142`,
+  `ha-binding-status.ts`), контракт AC10 (не трогать при неавторитетном
+  реестре) корректно продолжает уже действующий ранний `return` в
+  `resolveDeviceAreaRelocations` при `!authoritative`.
+  `.slice(0, MARKER_AREA_SNAPSHOT_LIMIT)` при чтении — подтверждено
+  (`device-area-relocation.ts:64-75`), обрезка по вставке действительно режет
+  новые записи, а не старые; контракт на переворот правила (AC11) реализуем.
+- **(д) `acceptance.declared`** — подтверждено чтением
+  `scripts/docs-acceptance.mjs` и `scripts/docs-accept.mjs:141-145`:
+  `docsAcceptancePlan` при пустом `declared` не отказывает (не тот класс
+  ошибок, что «объявлены, но не изменились»), и текущий код действительно
+  безусловно пишет `declared: [...decision.replace]` — при `replace: []` это
+  стирает предыдущий список. AC12/AC13 реализуемы без правки существующих
+  тестовых утверждений — `docsAcceptancePlan` не меняется, меняется только
+  сборка `accepted.acceptance` в `docs-accept.mjs`.
+- **Скоуп/не-скоуп** — границы названы явно и по делу (например, отказ от
+  подключения `*.help.aria`-задела к интерфейсу — не в этой задаче), пересечение
+  с #403/#405 разведено по разным контрактам того же файла.
+- **Продуктовые вопросы владельцу** — ни одного технического вопроса не
+  вынесено владельцу; UX-раздел корректно ссылается на терминологию
+  `USER-GUIDE.ru.md` вместо изобретения новой.
+- Release-артефакты: изменение User-Visible (скринридер объявляет
+  подтверждение как alertdialog) корректно привязано к обоим CHANGELOG,
+  остальные четыре пункта верно помечены внутренними.
+
+## Чего не проверял
+
+- Не пересчитывал вручную полный список из «48 динамических семейств» и
+  итоговые «937 использовано / 30 не используется» построчно — точечно
+  перепроверил категории, названные в тексте (`confirm.*`, `*.help.aria`,
+  единичные ключи), этого достаточно, чтобы найти расхождение; исчерпывающий
+  пересчёт всех 1201 ключей — задача самого гейта (AC1), не ревью ТЗ.
+- Не проверял, останется ли запись `marker_area_snapshot` живой, если
+  устройство пропало из `_devices`, но по тому же id всё ещё существует
+  маркер (контракт (г) говорит «пока жив маркер ИЛИ устройство», AC9
+  формулирует доказательство только через отсутствие в `_devices`, без явной
+  проверки маркера) — не смог дёшево установить, входят ли такие маркеры в
+  `options.devices` по построению; если нет — AC9 в текущей формулировке может
+  требовать удаления записи, которую контракт просит сохранить. Не поднимаю
+  до отдельной находки: не смог воспроизвести конкретный сценарий отказа за
+  разумное время, это кандидат на внимание в код-ревью, а не блокер ТЗ.
+- Гейты (`tsc`/`test`/`build`) не гонял — на этапе spec кода ещё нет, дерево
+  ветки состоит из одного doc-коммита.
+
+## Вердикт
+
+Вердикт: жёлтый · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 2 → в задаче