17 KiB
CODE-REVIEW-450-r1
Issue: #450 · трек trivial · заход r1 · блокирующих циклов 0/2
Материал: git diff origin/dev...HEAD, git log --oneline origin/dev..HEAD
SHA на момент вывода: a8ccb2b90fa72a808e818de51d0af1697febadda
Ветка приведена к dev конвейером до ревью (1 коммит dev поверх, 41612974 -> a8ccb2b9).
Разбор — полный, как и предписано при таком ребейзе (§7.2), но фактическая
дельта тривиальна: единственный добавленный коммит-надстройка —
a8ccb2b9 docs: refresh screenshot source fingerprint (класс C, User-Visible: no,
собственный doc-коммит публикации, что и разрешено правилом «допустим только
собственный doc-коммит поверх»). Продуктовый код не менялся между 41612974
и текущим SHA.
Скоуп
Баг: normalizeZ2mTopology в src/zigbee-topology.ts разбирает
bridge/response/networkmap только в соглашении bridge/devices
(ieee_address/network_address), а реальный raw-ответ Zigbee2MQTT
(zigbee-herdsman) отдаёт camelCase (ieeeAddr/networkAddress) и плоские
sourceIeeeAddr/targetIeeeAddr на связи. Итог — 0 узлов, 0 связей,
error_invalid_payload у каждого реального пользователя Z2M. Закрывает J7
(«Zigbee mesh health») из docs/SCOPE.md — без парсера не работает вся ветка
Z2M диагностики.
Один коммит продуктовых изменений (8935a736, класс A+B+C+D в одном
коммите): src/zigbee-topology.ts, test/zigbee-topology.test.mjs,
новая фикстура test/fixtures/zigbee2mqtt-networkmap-real-anonymized.json,
мутант в scripts/mutation-gate.mjs, чек-лист в docs/TESTING.md, оба
changelog. Трейлеры Issue: #450 / User-Visible: yes на месте, оба
changelog — в этом же коммите (проверено git show --stat).
Как проверялось
Гейты (прогнаны лично на a8ccb2b9)
| Гейт | Команда | Результат |
|---|---|---|
| Typecheck | npx tsc --noEmit |
green, без ошибок |
| Unit | npm test |
green: tests 1909 · pass 1908 · fail 0 · skipped 1 (совпадает с заявленным в хендоффе; 1 skip — известный #89-класс окружения, не связан с диффом) |
| Build + сверка бандла | npm run build затем cmp dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js |
green, идентичны. Копия demo/srv/assets/** не коммитится (#255) — сверка неприменима |
| Docs freshness | node scripts/check-docs.mjs (diff трогает src/**) |
green: «Documentation checks passed (7 files, 12 external links)» |
Новый any |
node scripts/no-new-any.mjs --base origin/dev --head HEAD |
green: 18 добавленных строк в 1 файле, новых any нет |
| Бюджет бандла | npm run bundle:budget |
green: initial View 295079 B, headroom 4921 B — тот же известный долг #367, не новый регресс (диф не добавляет UI/рендер) |
| Мутант автора | node scripts/mutation-gate.mjs --id=zigbee-topology-z2m-camelcase-node-rejected |
green: «тест покраснел, как обязан», поймано 1 из 1 |
Не гонялись и почему:
npm run golden:verify— diff не меняет рендер/геометрию/стили/слои: правка только в чистом нормализаторе данных (hp-zigbee-topology-overlayв исходниках не тронут, изменился лишь его собранный чанк — сама сборка, а не логика оверлея). Визуальных последствий нет.python -m pytest tests_backend -q— Python не тронут.- Инварианты модели (
npm run invariants) — diff не касается геометрии,layout,marker.space,open_spans, толщины стен. - Performance-профили — не названы в AC, чувствительных путей не касается.
Браузерные смоки
node scripts/smoke-select.mjs --base origin/dev --head HEAD:
Изменено файлов src/**: 1 · символов на изменённых строках: 2
НЕОПРЕДЕЛЁННОСТЬ: дифф исполняемый, но ни один смок не связан доказуемо.
Символы, которых нет ни в одном смоке: endpointIeee, normalizeIeee
Решение: смок не гоняю. Обе неопознанные функции — внутренние помощники
чистого нормализатора MQTT-payload, без пути к DOM/canvas; сама UI-часть
(hp-zigbee-topology-overlay) в этом диффе не меняется вовсе, а её
рендер-контракт (что и как рисуется при наведении) не затронут — меняется
только то, какие узлы/связи долетают до неё живыми. Наблюдаемое поведение
целиком укладывается в существующий runtime-тест
(«Z2M runtime verifies retained bridge info…», низкоуровневая проверка
refreshZ2mTopology на реальном MQTT envelope, доходит до phase: 'ready').
Браузерный смок добавил бы дублирующее, не более сильное доказательство того
же контракта. AC задачи browser-смок не называет.
Защитные AC — таблица «чем краснеет» (§2.7)
Обе строки, требующие мутации/снятия защиты, проверены лично поверх зелёного
npm test (чистый юнит, без дорогого гейта — по правилу достаточно прогона
со снятой защитой):
| AC | Чем доказан | Чем краснеет |
|---|---|---|
AC1: camelCase-узлы (ieeeAddr) не отбрасываются |
node --test --test-name-pattern="real anonymized camelCase" test/zigbee-topology.test.mjs |
Мутант zigbee-topology-z2m-camelcase-node-rejected (убирает ieeeAddr из чтения узла) — node scripts/mutation-gate.mjs --id=... → «тест покраснел, как обязан» |
AC1: плоские sourceIeeeAddr/targetIeeeAddr имеют приоритет над вложенными source/target |
node --test --test-name-pattern="prefers flat link IEEE fields" test/zigbee-topology.test.mjs (тест намеренно конфликтует: flat указывает на узлы 1↔2, nested на тот же узел 3↔3) |
Ручная инверсия приоритета (endpointIeee(...) ?? normalizeIeee(flat) вместо normalizeIeee(flat) ?? endpointIeee(...)) на пересобранном test-build → 0 !== 1, тест красный (nested self-link «съедает» оба звена) |
AC2: массив failed не помечает узел недоступным (available не должен стать false) |
node --test --test-name-pattern="real anonymized camelCase" test/zigbee-topology.test.mjs (реальная фикстура: у End device failed: ["routingTable"]) |
Ручное снятие защиты (трактовка непустого массива как available: false) на пересобранном test-build → ожидание {ieee, role} без available, факт available: true/false для всех трёх узлов — тест красный |
AC2: legacy boolean fallback остаётся совместимым |
node --test test/zigbee-topology.test.mjs (тест «keeps snake_case compatibility…», узел с failed: true → available: false) |
Обычное сравнение ожидаемого/фактического — AC не заявляет защиту сверх уже проверенного выше, третий столбец не нужен |
AC3: runtime доходит до ready на реальной raw-схеме, malformed/ZHA не меняются |
npm test целиком (весь test/zigbee-topology.test.mjs, 11/11 green) |
Тест «rejects a malformed response immediately» и ZHA-тесты не тронуты диффом (сверено git diff) и остаются зелёными в полном прогоне — регрессии нет |
Оба ручных мутанта откачены после проверки (src/zigbee-topology.ts
восстановлен из git, test-build пересобран обратно перед сдачей отчёта).
Проверка AC по существу
- AC1 (реальный raw-payload нормализуется без
invalid_payload, приоритет плоских IEEE-полей при конфликте) — доказано автотестомZ2M normalization accepts a real anonymized camelCase raw network mapна фикстуреtest/fixtures/zigbee2mqtt-networkmap-real-anonymized.json(снята с реального payload из тела issue, IEEE обезличены) и тестом...prefers flat link IEEE fieldsс намеренным конфликтом flat/nested. Мутационно проверено (см. таблицу выше). Выполнено. - AC2 (snake_case + nested endpoints остаются рабочими;
failed-массив не превращается в недоступность; старыйbooleanсовместим) — доказано тем же тестом (три узла сfailed: []/["routingTable"]→availableне выставлен) и тестом snake_case-совместимости (failed: true→available: false). Мутационно проверено. Выполнено. - AC3 (runtime-тест на фактической raw-схеме доходит до
ready; malformed payload и ветка ZHA не меняются) — тестZ2M runtime verifies retained bridge info, correlates transaction and cleans subscriptionsпереведён наz2mNetworkmapFixtureцеликом (через MQTT-подпискуrefreshZ2mTopology), проверяетphase === 'ready',nodes.length === 3,links.length === 2. Ветка ZHA (normalizeZhaTopology,readZhaTopology) и malformed-тест не затронуты диффом — проверено чтениемgit diff(нет ни одной строки в ZHA-функциях) и подтверждено зелёным полным прогоном. Выполнено.
Примечание не по коду, а по тексту issue: раздел «Проверка» в теле issue
называет команду node --test test/zigbee-runtime.test.mjs — такого файла в
репозитории нет, runtime-тесты Z2M/ZHA живут в test/zigbee-topology.test.mjs
и покрывают ровно то же поведение (см. AC3 выше). Это неточность в тексте
issue, а не пробел кода: эквивалентная проверка существует и зелёная. Не
блокирует, не заводится отдельно — правка тела issue вне полномочий этого
ревью.
Побочная проверка: последствие available: false
Проверено чтением: mapTopologyNodes (src/zigbee-topology.ts:282) исключает
узел из размещения на плане при node.available === false
(provider_scan_failure). Это делает AC2 не косметическим: до правки любой
реальный узел с частичным сбоем опроса (failed: ["lqi"] и т.п. — обычное
дело в живой сети) при доверчивом мэппинге массива в false тихо пропадал бы
с карты. Мутационная проверка выше воспроизводит именно этот сценарий на
реальном узле End device (failed: ["routingTable"]).
Не проверял
golden:verify,pytest tests_backend,npm run invariants, performance-профили — обоснование см. таблицу гейтов выше.- Полный набор
demo/smoke_*.mjs— не запускал ни одного: diff не задевает ни один визуальный/интерактивный контракт,smoke-select.mjsне нашёл прямых совпадений. - Внешний CI-прогон
docs(ссылка из хендоффа) — не открывал; воспроизвёл тот же гейт локально (check-docs.mjs, зелёный) и этого достаточно. - Ветку ZHA на настоящих данных (упомянута в issue как отдельная забота) — сознательно исключена из скоупа автором на этапе аналитики («Ветка ZHA использует отдельный фактический контракт… не требует изменения»), в этом issue не проверяется.
Находки
Нет High. Нет Medium — ни в скоупе, ни вне скоупа. Low нет.
Единственное отмеченное выше расхождение (несуществующий файл в тексте «Проверка» issue) — не дефект кода и ниже порога Low: не требует правки, не маскирует непроверенное поведение.
Один источник числа
Диф не добавляет и не меняет ни одной пользовательски видимой величины
(площадь, размер, подпись с единицей измерения). LQI попадает в модель как
было (lqiOf), способ отображения не менялся. Правило неприменимо к этому
диффу.
Вердикт
Все три AC доказаны автотестами, для защитных претензий (camelCase-приоритет,
приоритет плоских IEEE-полей, семантика failed-массива) лично проверено,
что тест умеет падать — мутацией автора и двумя ручными мутациями. Дешёвые
гейты зелёные. Диф укладывается в объявленный скоуп (trivial, одна
поверхность — Z2M-нормализатор), ветка ZHA не тронута, откат тривиален
(файл конфигурации не меняется).
Вердикт: зелёный · заход r1 · блокирующих циклов 0/2 · High: 0 · Medium: 0
Материал раунда
- Ветка/SHA материала:
a8ccb2b90fa72a808e818de51d0af1697febadda(HEAD на момент вывода, свереноgit rev-parse HEADнепосредственно перед подведением итогов). - Дерево материала: вывод
git diff origin/dev...HEAD --statвыше, 33 файла (13 продуктовых/тестовых/документных + генерированныеdist/**иcustom_components/houseplan/frontend/**). - Диапазон:
git log --oneline origin/dev..HEAD=8935a736(реализация),a8ccb2b9(doc-коммит публикации конвейера). - Команды поиска (если ветка задачи будет перебазирована/удалена до r2):
git log --all --find-object=<blob> -- src/zigbee-topology.ts.
Материал раунда
- Ветка:
issue/450-z2m-networkmap-camelcase, коммит416129744b3a— ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. - Дерево материала:
d305da400327b44c81c69a8467ffab58fb02f6f7git log --all --format='%H %T' | grep d305da400327