houseplan-card/docs/reviews/CODE-REVIEW-406-r2.md

CODE-REVIEW-406-r2

• Issue: https://github.com/Matysh/houseplan-card/issues/406
• Этап: code (PROCESS.md §2.7)
• Заход: r2 · блокирующих циклов израсходовано 1 из 4 (перед этим раундом)
• Ветка: issue/406-beta2-polish
• Коммит на ревью: f8cfac9c77332d3061b81239fe0c965add54b7df
• ТЗ: docs/specs/406-beta2-polish.md, revision 4, зелёный SPEC-REVIEW-406-r4
• Предыдущий раунд: docs/reviews/CODE-REVIEW-406-r1.md, жёлтый, коммит на ревью fe385cbf (SHA не сохранился — ветка была перебазирована после r1, см. ниже), единственная находка Medium (устаревший отпечаток скриншотов документации)
• База сравнения: origin/dev = 2903374b72a1b16d82eb87acac7233a1db3a3c5e

Почему разбор полный, а не по дельте

PROCESS.md §2.10 требует полного разбора, если «дельта не локальна: ребейз на ушедший вперёд dev». Именно это произошло между r1 и r2:

1. r1 (16:14:53) вынес жёлтый вердикт по коммиту fe385cbf с единственной находкой — устаревший отпечаток docs/images/screenshots.json.
2. Автор (16:16:16) сообщил, что на финальном SHA 6a34396f отпечаток уже обновлён, и попросил повторить ревью.
3. Повторный запуск (16:16:45) не состоялся — при ребейзе ветки на origin/dev обнаружился конфликт в scripts/mutation-gate.mjs (там же независимо приземлился #404). Цикл не израсходован: код никто не читал, вердикта не было.
4. Автор (16:18:41) разрешил конфликт, сохранив мутанты и #404, и #406, перебазировал ветку.

Из-за этого у всех коммитов ветки, начиная со старого fe385cbf, изменились хеши — старый SHA не существует в текущей истории (git cat-file -e fe385cbf9d5… → отсутствует), поэтому прямой git diff fe385cbf..HEAD невозможен. Восстановил дельту по содержимому: сравнил текст находок r1 с текущим деревом и с телом самих коммитов (git show <sha> по каждому коммиту диапазона).

Дельта оказалась не локальной ещё по одной причине, отдельной от ребейза: между fe385cbf (проверено r1) и финальным авторским SHA 6a34396f приземнился не только chore: refresh docs source fingerprint (закрытие находки r1), но и отдельный поведенческий коммит fix: preserve the ordinary dialog path, которого r1 не видел и не мог видеть — он был запушен позже (комментарий 16:14:38, уже после хендоффа на ревью). Это реальная правка кода в src/hp-dialog.ts, а не техническая перестановка. Поэтому разбор веду по всем двенадцати AC заново, а не только по находке r1.

Скоуп

Тот же, что в r1 — четыре независимых пункта ТЗ: (а) удаление 13 мёртвых ключей i18n + AST-гейт; (б) role="alertdialog" + aria-describedby для hp-confirm; (в) смок на обе ветки (HA/native); (г) уборка marker_area_snapshot для исчезнувших устройств при авторитетном реестре + разворот правила усечения по лимиту.

Материал — git log --oneline origin/dev..HEAD (12 коммитов) и git diff origin/dev...HEAD (44 файла).

Что изменилось с r1 (не только rebase)

git show по каждому коммиту диапазона восстанавливает содержательную последовательность после спек-ревью (d3412e6f):

Коммит (текущий хеш)	Содержание	Был ли виден r1
5d2f2957 fix: close beta 2 polish gaps	основная реализация (а)-(г), User-Visible: yes	да, это и есть fe385cbf по содержанию
d5f050fd fix: preserve the ordinary dialog path	новая находка ниже — правка src/hp-dialog.ts, добавляет второй <ha-dialog>-шаблон	нет, запушен после хендоффа r1
13593c1b chore: refresh docs source fingerprint	закрывает Medium r1	нет, запушен в ответ на вердикт r1
f8cfac9c docs: review document for #406	публикация CODE-REVIEW-406-r1.md	—
2903374b (в origin/dev, не в ветке) fix: exception guard… (#404)	не относится к #406, но конфликтовал с #406 в scripts/mutation-gate.mjs при ребейзе	—

Как проверялось

Ручного тестирования в цикле нет. Зелёного Validate на f8cfac9c не найдено — всё ниже прогнано лично.

Гейт	Результат
npx tsc --noEmit	green, без вывода
npm test	1711 passed, 0 failed, 1 skipped (было 1706 в r1 — разница ровно от рёбер #404 в dev)
npm run build	green, 15.7s
npm run bundle:sync (build + bundle-sync.mjs --check эквивалент — прогнал bundle:sync целиком и проверил git status после)	green, git status --porcelain пуст после — три копии бандла синхронны
npm run bundle:budget	initial 287370 B / budget 300000 B, headroom 12630 B (>0 → AC12 выполнен); предупреждение о низком запасе — фон #367, не регрессия
node scripts/mutation-gate.mjs --check	все мутанты ok (331), включая три мутанта #406 (i18n-dead-key-returns, confirm-dialog-loses-alertdialog, area-snapshot-cleanup-ignores-authority) и два мутанта #404, приземлившихся рядом при конфликте — оба набора пережили ребейз
node scripts/no-new-any.mjs --base origin/dev --head HEAD	«Новых any нет» (проверено 55 добавленных строк в 3 файлах)
node scripts/check-docs.mjs	green — находка r1 закрыта
node scripts/check-docs.mjs --external	green, 10 внешних ссылок
node scripts/process-gate.mjs	«гейт пройден, предупреждений 0»
node demo/smoke_danger_confirm_branches.mjs	green, все 24 поля true (AC6–AC8), включая ordinaryDialogForwardsHaAria/ordinaryDialogUsesHaBranch
node demo/smoke_area_relocation.mjs	green, все 20 полей true (AC9–AC10)
node scripts/smoke-select.mjs --base origin/dev --head HEAD	«Прямое совпадение (1)»: demo/smoke_help_affordance.mjs ← _focusInitial — символ появился на изменённой строке именно из-за нового дублированного <ha-dialog>-шаблона (см. находку); прогнал
node demo/smoke_help_affordance.mjs	green, все поля true, включая keyboardFocus

Не прогонял и почему:

• Полная матрица demo/smoke_*.mjs (212 файлов) — диапазон touches только hp-dialog/hp-confirm, device-area-relocation и словари; единственный «прямой» кандидат из smoke-select прогнан, остальные символы (в основном внутренние геттеры device-area-relocation.ts) уже покрыты юнитами построчно (проверено чтением). Полный прогон — предрелизная обязанность.
• npm run golden:verify — demo/golden/matrix.mjs не менялся с r1 (сверил диффом с origin/dev); ни один сценарий (device-dialog-* и др.) не открывает destructive/warning hp-confirm.
• npm run invariants / python -m pytest tests_backend — диапазон не трогает .py, рёбра комнат, layout, marker.space, open_spans (проверено git diff --stat по custom_components/**/*.py — пусто).
• Перфоманс-профили — не названы в AC.
• Полный HA-харнесс / реальный визуальный просмотр в HA — вне цикла ревью.

Находки

Low (в скоупе, снимается с записью) — правка fix: preserve the ordinary dialog path не имеет собственного быстрого регресс-теста

src/hp-dialog.ts:452-467 (текущий диапазон, коммит d5f050fd, ранее не рецензировался): рендер обычного (не-alert) hp-dialog при зарегистрированном ha-dialog теперь разделён на два шаблона:

if (this.describedBy) {
  return html`<ha-dialog … .ariaDescribedBy=${this.describedBy} …>…</ha-dialog>…`;
}
return html`<ha-dialog … (без .ariaDescribedBy вовсе) …>…</ha-dialog>…`;

До этой правки (в версии, которую видел r1, коммит 5d2f2957) была одна ветка с .ariaDescribedBy=${this.describedBy || undefined} — то есть свойство ariaDescribedBy явно выставлялось в undefined на всех ~30 существующих вызовах <hp-dialog> без описания (маркер, калибровка, бэкдроп, импорт/экспорт и т.д. — весь обычный, не-hp-confirm, путь). Автор обнаружил это не тестом, а побочно: обязательная пересъёмка check-docs.mjs дала пиксельную дельту в двух кадрах обычных диалогов (комментарий 16:14:38), и это был реальный визуальный регресс на самом частом пути карточки — большинство диалогов не альтернативны.

Правка верна и подтверждена: канонический прогон «Docs screenshots» (run 33530447151) дал побайтовое совпадение всех 10 PNG после фикса — я сверил docs/images/screenshots.json: imageSha256 всех сценариев не изменился, поменялись только sourceFingerprint/sourceSha256. Это надёжное доказательство отсутствия визуальной регрессии на зафиксированных кадрах.

Но у самого фикса (ветка «нет описания» на HA-пути) нет быстрого автотеста: demo/smoke_danger_confirm_branches.mjs проверяет только случай «с описанием» (ordinaryDialogForwardsHaAria, describedBy = 'ordinary-description' — истинное значение); AC7 в ТЗ тоже требует доказательство только для этого случая. Случай «без описания», который и был реальным дефектом, не покрыт ни юнитом, ни смоком, ни мутантом mutation-gate.mjs — единственная защита от повторного регресса это сам check-docs.mjs (обязателен по механике при любой правке src/**) плюс ручная пиксельная сверка при следующей пересъёмке. Это тот же класс риска, что и «тихий успех» из PROCESS.md §2.9 (#171, #207): если кто-то в будущем снова объединит обе ha-dialog-ветки в одну ради дедупликации кода, npm test/mutation-gate --check/оба browser-смока этой задачи останутся зелёными, и только пиксельный дифф на следующей пересъёмке скриншотов покажет проблему — то есть с существенной задержкой относительно коммита, который её внёс.

Снимаю без возврата в работу: поведение на этом SHA доказанно корректно (канонический скриншот-прогон + факт, что check-docs.mjs green), находка не блокирует AC7 и не относится к сценарию, который ТЗ просило доказать. Рекомендация на будущее — не для этого цикла: добавить в mutation-gate.mjs мутанта, убирающего ветвление if (this.describedBy) (schlagen: замена на старую безусловную форму), guard — node demo/smoke_danger_confirm_branches.mjs с новым полем-утверждением «обычный диалог без описания не передаёт ariaDescribedBy». Дешёво и ловит именно этот класс регрессии за секунды, а не за цикл релиза.

Закрытие раунда r1

Находка r1	Чем закрыта	Где это видно
Medium: node scripts/check-docs.mjs красный на fe385cbf — устаревший отпечаток docs/images/screenshots.json	Коммит 13593c1b (chore: refresh docs source fingerprint) обновил sourceFingerprint/sourceSha256 по каноническому прогону «Docs screenshots» run 33530447151; imageSha256 не изменился ни для одного из 10 сценариев	git diff origin/dev...HEAD -- docs/images/screenshots.json: меняются только два поля-хеша фингерпринта на строку, imageSha256 идентичен. node scripts/check-docs.mjs — green (прогнал лично)

Унаследовано из r1 — с оговоркой

Формально это полный разбор (§2.10, ребейз на ушедший вперёд dev), поэтому все AC перепроверены заново в этом раунде, а не унаследованы бланково. Но там, где текущий файл байт-в-байт совпадает с версией, которую видел и проверил тестом/смоком r1, я не повторяю тот же путь доказательства второй раз, а опираюсь на сочетание «r1 уже прогнал тест X» + «файл не менялся с тех пор, проверено git diff origin/dev...HEAD -- <file> и содержимым коммитов между fe385cbf-эквивалентом и HEAD»:

• src/device-area-relocation.ts (AC9–AC11) — не менялся ни одним из коммитов после 5d2f2957. Логика (авторитетный реестр строит liveIds/ liveBindings, инвариант .slice(-MARKER_AREA_SNAPSHOT_LIMIT)) идентична описанной в CODE-REVIEW-406-r1.md. Перечитал файл и оба продуктовых вызова (houseplan-card.ts:5054/5166) самостоятельно — совпадает с описанием r1. Тесты test/device-area-relocation.test.mjs (14/14) и смок demo/smoke_area_relocation.mjs (20/20 полей) прогнаны мной лично в этом раунде, не по памяти r1.
• AC1–AC5 (мёртвые ключи i18n) — файлы словарей и test/i18n-dead-keys.test.mjs/test/unified-wall-tool-source.test.mjs не менялись после 5d2f2957. Перепрогнал node --test test/i18n-dead-keys.test.mjs (2/2) и полный npm test лично.
• Раздел «одно число — один источник» — диапазон по-прежнему не добавляет видимых пользователю величин (роль/aria — не число); initial View в bundle:budget — единственное число, и оно считается один раз сборкой, что я перепроверил (287370 B совпадает и в выводе bundle:budget, и в houseplan-assets.json). test/single-source-numbers.test.mjs в составе npm test, прошёл.
• Трейлеры/changelog — перепроверил самостоятельно все 12 коммитов диапазона (git show -s --format=%B): Issue: #406 на каждом, User-Visible: yes только на 5d2f2957, оба changelog правятся в этом же коммите (сверил git show --stat 5d2f2957).

Ссылка на документ прежнего раунда: docs/reviews/CODE-REVIEW-406-r1.md (доступен в дереве этой ветки, коммит f8cfac9c).

Что проверено и корректно (AC1–AC12, этот раунд)

• AC1–AC3: test/i18n-dead-keys.test.mjs (AST по src/**/*.ts, derivedHelpAria.size === 19) — 2/2, прогнано лично.
• AC4: коллизия с test/unified-wall-tool-source.test.mjs разрешена той же строкой, что и в r1 — 'history.partition_add' убрана из проверяемого списка в том же коммите, что и ключ из словарей.
• AC5: паритет словарей — npm test включает test/i18n.test.mjs, прошёл; сверил git diff по всем четырём словарям — 13×4=52 строки.
• AC6: оба kind hp-confirm безусловно получают .alert=${true} и .describedBy=${descriptionId} (src/hp-confirm.ts:40-41, перечитал файл целиком) → _usesHaDialog() (this._useHaDialog && !this.alert) всегда false для confirm → нативный <dialog role=${this.alert ? 'alertdialog' : 'dialog'}> (src/hp-dialog.ts:483). Смок: noHa*IsDescribedAlert, ha*StaysNativeAlert, realAccessibilityTreeIncludesConsequence — все true. Мутант confirm-dialog-loses-alertdialog ловится (mutation-gate --check, ok).
• AC7: перечитал новую (двухветочную) реализацию _usesHaDialog()===true пути — новая находка описана выше, но контракт AC7 доказан: ordinaryDialogUsesHaBranch/ordinaryDialogForwardsHaAria — true в смоке, прогнан лично.
• AC8: фокус на «Отмена», Esc → false, клик по «Отмена» → false в обоих окружениях — все соответствующие поля смока true, прогнан лично.
• AC9/AC10: см. раздел «Унаследовано» — перепроверено чтением и тестом заново в этом раунде, не по памяти r1.
• AC11: юнит-тест defensive snapshot reader keeps the newest entries when over its limit — ok 13 в test/device-area-relocation.test.mjs, прогнан лично; символ правки не менялся с r1.
• AC12: npm run bundle:budget → 287370 B / 300000 B, положительный запас; число на 50 Б больше, чем в r1 (287320 → 287370) — ожидаемо: новая ветка if (this.describedBy) добавляет несколько байт разметки.
• Трейлеры и changelog: см. «Унаследовано», перепроверено самостоятельно на всех 12 коммитах диапазона, не только на 5d2f2957.

Чего не проверял

• Полную матрицу demo/smoke_*.mjs (212 файлов) — обоснование в «Как проверялось».
• npm run golden:verify — ни один голден-сценарий не открывает затронутый диалог, сверил demo/golden/matrix.mjs заново на этом SHA.
• npm run invariants / python -m pytest tests_backend — диапазон их не задевает.
• Перфоманс-профили — не названы в AC, не тронуты.
• Подлинность прогона run 33530447151 — доверяю ссылке на GitHub Actions, не открывал сам артефакт; сверил только зафиксированные в docs/images/screenshots.json хеши образов (не изменились) как косвенное, но веское подтверждение.
• Реальный визуальный вид нативного диалога в живом Home Assistant — только по коду, accessibility-дереву Chromium и каноническому скриншот-прогону; ручного просмотра в этом цикле нет и не может быть.

Вердикт

Единственная находка этого раунда — Low, по правке, которая появилась в дельте после r1 (fix: preserve the ordinary dialog path), не покрыта собственным быстрым тестом, но её корректность доказана канонической пересъёмкой скриншотов (побайтовое совпадение) и обычным browser-смоком для того случая, который требует AC7. Не блокирует, снимаю с рекомендацией на будущее (см. находку). Находка r1 (устаревший отпечаток документации) закрыта по существу и проверена лично, а не по слову автора. Все 12 AC доказаны тестом или смоком, который я лично прогнал в этом раунде и который умеет падать (мутанты подтверждают три ключевых контракта, включая оба приземлившихся после ребейза набора — #406 и #404). High-находок нет.

Зелёный.