From c53702cdfadbb52ce52a4b039f96718205f9686b Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Fri, 4 Sep 2026 11:05:11 +0000 Subject: [PATCH] docs: review document for #54 Issue: #54 User-Visible: no --- docs/reviews/SPEC-REVIEW-54-r1.md | 218 ++++++++++++++++++++++++++++++ 1 file changed, 218 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-54-r1.md diff --git a/docs/reviews/SPEC-REVIEW-54-r1.md b/docs/reviews/SPEC-REVIEW-54-r1.md new file mode 100644 index 00000000..d342dbf2 --- /dev/null +++ b/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 → в задаче + +--- + + + +## Материал раунда + +- Ветка: `issue/54-zigbee-topology-hover`, коммит `a6a3df931079` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `fb166b00299442bc4091dc79538ae54d62106017` + ``` + git log --all --format='%H %T' | grep fb166b002994 + ```