docs: review document for #54

Issue: #54
User-Visible: no

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

@@ -0,0 +1,218 @@
+# SPEC-REVIEW-54-r1
+
+- Issue: https://github.com/Matysh/houseplan-card/issues/54
+- Этап: spec (PROCESS.md §2.4)
+- Заход: r1 · блокирующих циклов израсходовано 0/4 до этого вердикта
+- Материал: `docs/specs/054-zigbee-topology-overlay.md` на SHA `a6a3df931079a008dd4e7f7d602800e46c353192` (ветка `issue/54-zigbee-topology-hover`), коммит `docs: specify contextual zigbee links`, `Issue: #54 · User-Visible: no`
+- Трек: полный (обоснован автором в шапке ТЗ и в аналитических комментариях владельца — сложность/риск, provider-контракты, новый UX/API-контракт)
+
+## Скоуп ревью
+
+Первый раунд ревью ТЗ. Диапазон изменений: `git diff 6eea4d1c..a6a3df93` показывает
+единственный затронутый файл — `docs/specs/054-zigbee-topology-overlay.md`
+(+509/-50 строк относительно предыдущей редакции документа). Продуктовый код
+(класс A) не менялся, `git show --stat a6a3df93` подтверждает единственный файл
+в коммите. Проверка соответствует этапу spec: судится ТЗ, а не реализация.
+
+## Как проверялось
+
+1. Прочитаны в порядке, заданном инструкцией: `docs/SCOPE.md`, `AGENTS.md`,
+   `PROCESS.md` (§1–§8, §2.4, §2.5, §2.10, §4, §7.1), тело issue #54 и все пять
+   комментариев (аналитика 2026-08-14/2026-08-30, занятие Stage 0, итог Stage 0
+   2026-08-31, решение владельца по UX 2026-09-04).
+2. Прочитан `docs/specs/054-zigbee-topology-overlay.md` целиком (534 строки, 19
+   разделов).
+3. Сверены канонические документы: `docs/UX-MODES.md` (правила View/editor,
+   admin_only editor tabs), `docs/TOUCH-SUPPORT.md` (контракт View на touch,
+   прецедент hover-only фичи без touch-эквивалента — `show_room_tooltip`),
+   `docs/CONFIG-COMPATIBILITY.md` (прецедент нового optional-boolean поля
+   settings без миграции схемы, #426), `docs/USER-GUIDE.ru.md` (текущий словарь
+   «Общие настройки», отсутствие сегодня раздела про Zigbee — ожидаемо для
+   новой фичи).
+4. Сверена конвенция остальных ТЗ репозитория: `grep` по `docs/specs/*.md`
+   нашёл 10+ полнотрековых спеков с отдельным разделом «Затронутые файлы и
+   модули» (пример: `docs/specs/348-german-localization.md:322`).
+5. `node scripts/check-docs.mjs` — зелёный (7 файлов, 12 внешних ссылок),
+   подтверждает то, что уже заявил автор в хендоффе.
+
+### Гейты — что прогнано и что нет
+
+Этап spec: `src/**` не менялся, поэтому код-гейты код-ревью (`typecheck`,
+`test`, `build`, bundle sync/budget, smoke, mutation) к материалу этого раунда
+не относятся и не запускались — запускать их было бы про несуществующий диф.
+Прогнан только релевантный документационный гейт:
+
+| Гейт | Результат |
+|---|---|
+| `node scripts/check-docs.mjs` | зелёный (перепроверено ревьюером) |
+| `git diff --check 6eea4d1c..a6a3df93` | без конфликтных маркеров и trailing whitespace (перепроверено) |
+
+Смоки, golden, backend, performance — неприменимы: диф не касается `src/**`,
+`custom_components/**`, рендера или геометрии.
+
+## Находки
+
+### Medium (в скоупе задачи, чинится в этой же правке ТЗ)
+
+**M1 — нет обязательного раздела «Затронутые файлы и модули».**
+`PROCESS.md` §2.5 (DoR) требует буквально: «перечислены затронутые файлы и
+модули» как отдельный обязательный пункт готовности к разработке. В ТЗ #54 (19
+разделов, см. `grep -n "^## "` вывод) такого раздела нет вообще — ни списком
+путей, ни перечнем логических модулей. Это не вопрос стиля: без него код-ревью
+не имеет опорной точки, чтобы проверить полноту диффа реализации против
+заявленного скоупа, а переход в `S5-ready` формально не может закрыть чек-лист
+DoR.
+
+Прецедент в этом же репозитории: как минимум 10 недавних полнотрековых ТЗ
+(`226-entity-parent-dedup.md`, `238-opening-inner-distances.md`,
+`264-resize-controller.md`, `340-config-set-revision.md`,
+`348-german-localization.md` и другие) содержат отдельный раздел с этим именно
+заголовком, обычно перед «Риски». У #54 раздел пропущен полностью, хотя текст
+ТЗ явно предполагает несколько новых модулей (provider-neutral topology model,
+ZHA adapter, Z2M adapter, resolver mapping/cross-space classification, hover
+render layer, General Settings UI, i18n ключи) — их достаточно перечислить
+логическими именами (директории/модули), точные файлы не обязательны, раздел
+`18` уже разрешает менять внутренние имена свободно.
+
+Воспроизведение: `grep -n "^## " docs/specs/054-zigbee-topology-overlay.md` —
+среди 19 заголовков нет ни «Затронутые файлы», ни «Модули», ни синонима.
+
+Это находка Medium, а не High: она не делает ни один AC невыполнимым или
+непроверяемым, продуктового решения владельца не требует (пункт технический,
+разрешён к свободному выбору автором по §7.1 ТЗ), и чинится добавлением одного
+раздела в этом же документе.
+
+### Low (снимаются данным ревью с запиской, не блокируют)
+
+**L1 — имя персоны не совпадает с таблицей `docs/SCOPE.md`.**
+§1 ТЗ: «Персона — Enthusiast/Power User из `docs/SCOPE.md`». Таблица персон в
+`docs/SCOPE.md` (раздел Target audience) содержит ровно три строки: **Home
+admin** (описан как «HA enthusiast, house/large flat… sets up and maintains
+the plan»), **Household members**, **Guests/kiosk**. Персоны «Enthusiast/Power
+User» как отдельной строки в каноне нет — по содержанию это Home admin,
+описанный через прилагательное «enthusiast», а не самостоятельная персона.
+Смысл ТЗ не меняется (сценарий однозначно про admin с мышью), поэтому не
+блокирую, но при правке рекомендую заменить на точное имя из таблицы: «Home
+admin (HA enthusiast) из `docs/SCOPE.md`». Снимаю с запиской, правка
+необязательна к отдельному циклу.
+
+**L2 — формулировка AC14 «backend/HA contract smoke on Linux CI» не согласована
+с явным заявлением §18, что backend House Plan не добавляется.**
+§18 «Предположения автора»: «House Plan backend не добавляется, пока штатных
+HA WebSocket/MQTT surfaces достаточно». АС14 при этом называет доказательством
+«adapter unit with fake clock/MQTT + backend/HA contract smoke on Linux CI».
+Слово «backend» в доказательстве, скорее всего, значит «контрактный тест
+против реального/эмулированного HA backend» (аналог полного HA harness,
+который по `AGENTS.md` — только Linux/WSL), а не про появление
+`custom_components/houseplan` серверной поверхности — но буквальное чтение
+допускает обе трактовки и способно завести реализацию не в ту сторону при
+планировании гейтов. Не блокирую (не влияет ни на один продуктовый AC), но
+рекомендую на правке заменить формулировку на однозначную, например: «adapter
+unit with fake clock/MQTT + HA MQTT contract fixture on Linux CI (не House
+Plan backend)».
+
+**L3 — численные performance-бюджеты не зафиксированы в ТЗ, откладываются на
+измеренный dev-стенд перед S7 (§14.6).**
+AC18 формально требует «сохраняют установленный performance budget», но сам
+бюджет для нового topology-пути ещё не существует и по плану тестов (§14,
+пункт 6) будет измерен и зафиксирован только перед код-ревью, а не сейчас.
+Это не противоречие: методология (fixtures 20/100/500 nodes, что именно
+измеряется — normalize/map, first/repeated hover, dense-invalid rejection)
+названа, значит требование DoR §2.5 «влияние на производительность… названо»
+выполнено по существу. Формально было бы чище явно занести это как пункт
+предположений в §18 («точные числовые пороги устанавливаются перед S7 на
+измерении, не на этапе ТЗ»), но раздел `14.6` уже говорит то же самое прямым
+текстом. Снимаю без требования правки.
+
+## Что проверено и корректно
+
+- **Структура ТЗ (§7.1).** Все обязательные разделы присутствуют: сценарий,
+  что человек увидит до/после, проблема, скоуп/не-скоуп, контракт поведения,
+  UX, модель данных и миграция, i18n, AC1–AC20 с указанием доказательства,
+  план автотестов, риски, откат, release-артефакты.
+- **Продуктовая рамка.** Сценарий и «что человек увидит» описаны в
+  пользовательских терминах без технической лексики, персона и поверхность
+  названы (см. L1 — неточное имя, но не отсутствие). Задача явно закрывает J7
+  `docs/SCOPE.md` («Is my Zigbee mesh healthy here?»), ссылка на J7 есть в §17.
+- **Открытых продуктовых вопросов нет.** Комментарий владельца от 2026-09-04
+  закрыл все развилки UX (default-off toggle, mouse-only hover, cross-space —
+  только счётчик без перехода, touch/pen без нового жеста), и они дословно
+  перенесены в §5 и §4.2 не-скоупа. Технических вопросов, ошибочно вынесенных
+  владельцу как продуктовые, не найдено — единственный вопрос от аналитика
+  2026-08-14 был «вопросов нет».
+- **Никаких догадок, выданных за факт.** Раздел 7 (provider-контракты)
+  подкреплён конкретными версиями/SHA HA Core, HA Frontend, Zigbee2MQTT и
+  прямыми ссылками на исходники (§19); утверждения о `zha/topology/update` без
+  completion/result и о недоступности произвольного Z2M base topic через
+  registry явно обоснованы источником, а не предположением. Раздел 18 честно
+  маркирует технические решения автора как «assumed, change freely», что
+  соответствует правилу — размытое продуктовое не додумано, техническое явно
+  помечено.
+- **AC1–AC20.** Каждый критерий сформулирован проверяемо (конкретное условие +
+  конкретное отсутствие/наличие эффекта) и имеет указанный способ доказательства
+  (unit/adapter/browser smoke/golden/mutation/reviewer audit). Ни один AC не
+  описывает решение через реализацию без критерия наблюдаемого поведения.
+  Защитные AC (AC1, AC3, AC5, AC9, AC11, AC15, AC17 — все про «не делает X»)
+  имеют названный способ показать, что тест умеет падать (request spy,
+  mutation witness, DOM/computed-style assertions, call-count unit) — это
+  забота код-ревью, но на уровне ТЗ формулировка это уже предусматривает.
+- **Согласованность контракта.** Условия §5.3 (1–5) непротиворечиво стыкуются
+  с §9 (mapping), §10 (fetch/cache/lifecycle) и AC3: неадмин физически не
+  получает snapshot (кэш ключуется по HA-соединению, а runtime вообще не
+  грузится не-админу по §10), поэтому «не видит слой» в AC3 выполняется без
+  отдельного явного admin-гейта внутри условий hover — проверено логически,
+  расхождения нет.
+  Touch/pen carve-out (§4.2, §5.4, §15) прямо разрешён владельцем 2026-09-04 и
+  имеет прецедент в этом же репозитории — `docs/TOUCH-SUPPORT.md` уже
+  документирует ровно такой же паттерн для `show_room_tooltip` (hover-only
+  информация без touch-замены), так что это не непроверенное решение автора, а
+  соответствие канону.
+- **Совместимость конфига.** §8.1 («миграции нет, отсутствие объекта = выкл,
+  downgrade игнорирует неизвестное поле») повторяет уже принятый в проекте
+  паттерн из `docs/CONFIG-COMPATIBILITY.md` (`settings.show_room_tooltip`,
+  #426) — новый optional boolean без миграции схемы. Расхождений с каноном нет.
+- **i18n.** §12 перечисляет все новые видимые строки и синхронность
+  en/ru/de/fr; формулировки прямо избегают вводящих в заблуждение терминов
+  («наблюдаемые связи», не «текущий маршрут»).
+- **Release-артефакты и откат.** §16–17 называют both changelog, обновление
+  `UX-MODES.md`/`SCOPE.md`/user-guide, reviewed golden, и операционный откат
+  (выключить настройку, per-provider независимое отключение) без миграции
+  назад — соответствует правилу «никогда не удалять данные пользователя на
+  догадке» (снапшот memory-only, base topics не удаляются при выключении).
+
+## Чего не проверял и почему
+
+- Код-гейты код-ревью (`typecheck`, `test`, `build`, bundle sync/budget,
+  smoke-select, no-new-any, mutation runs) — неприменимо: диф этого раунда не
+  касается `src/**`/`custom_components/**`, тестировать нечего.
+- Golden/performance/backend прогоны — неприменимо по той же причине; они
+  станут предметом код-ревью, когда появится реализация.
+- Фактическую работоспособность provider WebSocket/MQTT контрактов (`zha/devices`,
+  `bridge/request/networkmap`) против живого HA/ZHA/Z2M стенда — вне периметра
+  ревью ТЗ; принимаю как факт §7 со ссылками на источники (Stage 0 research уже
+  прошёл отдельным этапом с итоговым комментарием владельца «GO с условиями»
+  2026-08-31), это не повторная работа этого ревью.
+- Точное соответствие будущего кода разделу 8.2 (`ZigbeeTopology`,
+  `ZigbeeTopologyNode` и т.д.) — модель помечена как assumed/changeable в §18,
+  оценивать её как контракт рано.
+
+## Вердикт
+
+Единственная блокирующая-в-скоупе находка (M1) — отсутствие обязательного по
+DoR раздела «Затронутые файлы и модули». High-находок нет. Три Low сняты
+записью выше без требования правки. Продуктовая часть ТЗ, провайдерные
+контракты, AC и UX-контракт корректны и не содержат непомеченных догадок.
+
+Вердикт: жёлтый · заход r1 · блокирующих циклов 1/4 · High: 0 · Medium: 1 → в задаче
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `issue/54-zigbee-topology-hover`, коммит `a6a3df931079` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Дерево материала: `fb166b00299442bc4091dc79538ae54d62106017`
+  ```
+  git log --all --format='%H %T' | grep fb166b002994
+  ```