20 KiB
CODE-REVIEW-29-r2
- Issue: #29 «[HP-UX-02] inbox и жизненный цикл устройств»
- Этап: code (PROCESS.md §2.7)
- Заход: r2 · блокирующих циклов израсходовано 1 из 4
- Проверено на SHA
8572d43c9192ace04b8e7d305d5a3589eee80071(HEAD, веткаissue/29-device-inbox-lifecycle, приведена конвейером кorigin/dev@2b1964f9) - Предыдущий раунд:
docs/reviews/CODE-REVIEW-29-r1.md, жёлтый, проверен на SHA905d4847a3797a8830eb6c5c509351f142193b73(эквивалент текущего4b3b71e73bc369d04f57ef000a30465571932bf1— см. «Скоуп разбора» ниже, это тот же коммит после ребейза, содержимое подтверждено идентичным)
Скоуп разбора: почему разбор полный, а не по дельте
Заголовок задачи говорит: между r1 и этим раундом ветка приведена конвейером
к dev — «поверх легло 1 коммит(ов) dev, 11c0cbd4 -> 8572d43c». Это ровно
условие §7.2 «после ребейза это другой код» — по умолчанию разбор полный.
Я не поверил этому на слово и проверил, что ребейз действительно ничего не
подмешал:
- коммит
4b3b71e7(«feat: add device lifecycle catalog») — это тот самый коммит, что рецензировался в r1 под SHA905d4847(сообщение коммита то же самое, дифф-статистика совпадает построчно:src/device-inbox.ts— ровно 281 строка, как в тексте r1,src/houseplan-card.ts— 536 вставок против 536 в свежемgit show --stat); сам SHA905d4847в дереве больше не существует именно потому, что рецензируемая история была переиграна поверх новогоdev-коммита — это и есть смена родителя, о которой предупреждает §7.2, а не смысловое расхождение; - единственный коммит
dev, который лёг поверх (2b1964f9, «ci: бандл собирается один раз…») — это перестройка CI-джобов, не трогает ни один файл вsrc/**,docs/specs/**,demo/**; пересечения с темой задачи нет; npx tsc --noEmit/npm test(1409 pass / 0 fail / 1 skip) /npm run build+ сверка трёх копий бандла — зелёные на текущем HEAD, то есть переигранный код компилируется и проходит тот же набор тестов, что и до ребейза.
Итог: контент коммита, который рецензировал r1, не изменился — изменился
только его родитель. Поэтому этот раунд не повторяет построчный разбор всего
src/houseplan-card.ts/src/device-inbox.ts заново (это было бы тем самым
бесполезным полным прогоном ради нуля новой информации, о котором
предупреждает §2.9), а: (а) независимо перепроверяет, что ребейз не подменил
код тихо (сделано выше), (б) разбирает новый коммит 8572d43c («fix: align
device catalog with plan rooms») целиком — это и есть фактическая дельта
поведения, отвечающая на находки r1, (в) заново прогоняет весь набор дешёвых
гейтов §8 на итоговом HEAD, а не только на дельте, (г) перепроверяет каждый
AC, довод которого лежит в изменённых строках (AC2, AC6, AC9 — источник
имени комнаты; AC1 — счётчик кнопок devbar), остальные наследует из r1 с
независимым подтверждением по гейтам (раздел «Унаследовано» ниже).
Скоуп изменения (коммит 8572d43c, единственный новый относительно r1)
Один коммит, Issue: #29, User-Visible: yes, оба CHANGELOG в этом же
коммите:
src/houseplan-card.ts(_deviceInboxRows()):areaNamesтеперь строится изthis._areaToSpace[id]?.room?.name(имя комнаты плана), а не изthis.hass?.areas[id].name(имя зоны HA); второй проход поareaMapгарантирует это и для комнат, чья зона отсутствует в текущем (возможно урезанном)hass.areas-снапшоте;src/device-inbox.ts: удалено мёртвое полеDeviceInboxRow.canOpenHa(не читалось нигде, рендер использует независимый_bindingHasHaPage);demo/smoke_editor_tabs.mjs: ожидаемое число кнопок devbar исправлено с 3 на 2 (комментарий обновлён на#29);demo/smoke_device_inbox.mjs: добавлены 3 проверки — строка каталога берёт имя из комнаты плана, а не из зоны HA, переименование комнаты на плане отражается в каталоге и в поисковой строке немедленно;- скриншот
docs/images/06-device-editor.pngиscreenshots.jsonпересобраны (следствие смены отображаемого текста, docs-check зелёный).
Как проверялось
| Гейт | Команда | Результат |
|---|---|---|
| Типы | npx tsc --noEmit |
зелёный, без вывода |
| Юниты | npm test |
# tests 1410 / pass 1409 / fail 0 / skipped 1 |
| Сборка | npm run build |
собран dist/houseplan-card.js за 13.4s |
| Три копии бандла | npm run bundle:sync + md5sum dist/… custom_components/… |
идентичны; git status после сборки чист — закоммиченные копии уже актуальны |
| Документация | node scripts/check-docs.mjs |
Documentation checks passed (7 files, 10 external links) |
| Выборка смоков по дельте | node scripts/smoke-select.mjs --base 4b3b71e7 --head HEAD |
«Прямое совпадение (1): demo/smoke_device_inbox.mjs ← _areaToSpace» — единственный релевантный смок, других слабых связей нет |
| Тематический смок (H1) | node demo/smoke_editor_tabs.mjs |
OK |
| Тематический смок (M2) | node demo/smoke_device_inbox.mjs |
OK, включая новые hasCatalogRowInPlanRoom/planRoomNameWins/planRoomNameIsSearchable |
| Тематический смок (не тронут этим коммитом, перепроверен как в r1) | node demo/smoke_hidden_flag.mjs |
OK, 27/27 |
| Тематический смок (не тронут этим коммитом, перепроверен как в r1) | node demo/smoke_binding_picker.mjs |
OK, 24/24 |
| Single-source-numbers | node --test test/single-source-numbers.test.mjs |
3/3 pass (не входит в скоуп изменения — в каталоге нет дублирующихся чисел, badge-счётчиков нет) |
| Единый источник чисел (продуктовое) | — | этот диф не добавляет и не меняет ни одной видимой пользователю величины (счётчика/площади/etc.) — есть только текстовое имя комнаты; проверять нечего |
| Инварианты модели | не прогонялись | diff не трогает рёбра комнат, layout, marker.space, open_spans, толщину стен — geometry не затронута (grep по diff подтверждает отсутствие этих символов) |
| CI на точном SHA задачи | gh run view 33125028109 (run триггернут этим же коммитом 8572d43c) |
«Фронтенд» ✓, «Перф-смок» ✓, все 3 шарда браузерных смоков ✓ (включая шард 2, где в r1 падал smoke_editor_tabs — теперь зелёный), «Golden» ✗ — missing-baseline для device-inbox-desktop-en-light/ru-dark, device-inbox-narrow-ru-dark (лог job подтверждает: missing-baseline device-inbox-desktop-en-light и т.д.) — то же самое ожидаемое состояние, что и в r1, не новая находка |
Полный node scripts/smoke-select.mjs --base origin/dev --head HEAD уже
выполнялся и разобран в r1 (75 прямых совпадений, обоснование см. там); для
этого раунда достаточно delta-режима (--base 4b3b71e7), так как только он
отвечает на вопрос «что изменилось между r1 и r2» — полная выборка от dev
не даёт новой информации, потому что нерассмотренная r1 часть дифа не
менялась.
Прочитан полный diff коммита 8572d43c (все 10 файлов) построчно.
Закрытие раунда r1
| Находка r1 | Чем закрыта | Где это видно |
|---|---|---|
H1 (High, в скоупе) — demo/smoke_editor_tabs.mjs ожидал 3 кнопки в devbar (add/show-all/rules), а ТЗ §10.1 оставляет 2 («Устройства», «Правила иконок») — существующий смок был красным на SHA задачи, в т.ч. в реальном CI |
Ассерт исправлен на === 2, комментарий переписан на devices catalog + icon rules (#29); фактическая разметка _renderDevicesBar() (houseplan-card.ts:20950-20970) рендерит ровно 2 .btn:not(.barclose) (кнопка каталога + кнопка правил, _editorToolbarGroups в демо-конфиге пуст) |
demo/smoke_editor_tabs.mjs:159; локальный прогон OK; CI run 33125028109, job «Смоки в браузере (шард 2 из 3)» — зелёный (был красным на SHA r1) |
M2 (Medium, в скоупе) — источник имени «комната» в каталоге — hass.areas[id].name (имя зоны HA), а не RoomCfg.name (имя комнаты плана); расходится с остальным UI и с текстом USER-GUIDE.ru.md («поиск… по комнате») |
areaNames теперь строится в первую очередь из this._areaToSpace[id]?.room?.name; второй проход по areaMap дополнительно покрывает комнаты, чья HA-зона отсутствует в (возможно урезанном) hass.areas-снапшоте текущего пользователя (комментарий в коде это явно объясняет) |
src/houseplan-card.ts:14100-14110; новый смок-сценарий переименовывает room.name на живом конфиге и проверяет, что строка каталога и searchText немедленно отражают новое имя (demo/smoke_device_inbox.mjs, planRoomNameWins/planRoomNameIsSearchable — оба true) |
L3 (Low, снято автором) — DeviceInboxRow.canOpenHa — мёртвое поле, нигде не читается |
Поле удалено из интерфейса и построения строки | src/device-inbox.ts — оба места (interface, push) отсутствуют; grep -rn canOpenHa по репозиторию не находит использований (только упоминание в тексте r1) |
Все три находки r1 закрыты по существу (проверено чтением кода и исполнением тестов/смоков, а не заявлением автора).
Проверено (полный охват AC, с указанием источника доказательства)
Диф 8572d43c не трогает резолвер жизненного цикла, read-only контракт,
пагинацию, accessibility-разметку и compatibility-слой — они не менялись со
времени r1, где были разобраны построчно и подтверждены тестами/mutation-gate.
Ниже — не слепое доверие, а повторное подтверждение тем же набором
тестов/гейтов на итоговом HEAD (см. таблицу гейтов выше: npm test
1409/1410, все тематические смоки зелёные на 8572d43c), с отдельной пометкой,
где что-то реально поменялось в этом раунде.
- AC1 (единая точка входа, счётчик кнопок): изменилось в этом раунде —
подтверждено чтением разметки
_renderDevicesBarиsmoke_editor_tabs(см. «Закрытие r1 → H1»). - AC2 (детерминированная классификация): не менялось; наследуется из r1
(unit-тест «full lifecycle matrix»,
npm testзелёный). - AC3 (auto/new): не менялось; наследуется из r1.
- AC4 (lifecycle/HA-status независимы): не менялось; наследуется из r1.
- AC5 (exact binding/re-add): не менялось; наследуется из r1
(
smoke_binding_picker24/24 перепрогнан на HEAD, зелёный). - AC6 (действия строки, включая «комнату» в мета-строке): источник имени
комнаты изменился в этом раунде — закрывает M2, подтверждено новым смок-
сценарием (
planRoomNameWins/planRoomNameIsSearchable). - AC7 (read-only): не менялось; наследуется из r1
(
smoke_device_inbox.browsingIsReadOnlyперепрогнан,true). - AC8 (возврат/refresh): не менялось; наследуется из r1
(
nestedCancelReturnsContextперепрогнан,true). - AC9 (поиск/большие реестры): поисковая строка теперь включает имя
комнаты плана, а не зоны HA — это и есть предмет M2; unit-тест на 260
сущностях (не менялся, не завязан на имя комнаты) и
searchUsesFullSnapshotперепрогнаны, зелёные. - AC10 (accessibility/responsive): не менялось; наследуется из r1
(
arrowChangesTab,noHorizontalOverflowперепрогнаны,true). - AC11 (compatibility):
npm testперепрогнан целиком на HEAD, 1409/1410 зелёных (то же соотношение, что и в r1, плюс 10 новых утверждений в смоках). - Трейлеры всех 7 коммитов диапазона
origin/dev..HEAD:Issue: #29везде;User-Visible: yesу8572d43cи4b3b71e7(единственные, меняющие видимое поведение) — в обоих оба CHANGELOG правились в том же коммите (провереноgit show --statи содержимым диффов).
Унаследовано из r1 (без повторного построчного чтения)
Документ: docs/reviews/CODE-REVIEW-29-r1.md, SHA 905d4847 (= текущий
4b3b71e7 после ребейза, содержимое идентично — проверено дифф-статистикой
и повторным прогоном тестов, см. «Скоуп разбора» выше). Принято без
повторного посимвольного чтения (но с независимым перепрогоном тестов на
HEAD):
- построчный разбор
src/houseplan-card.ts(диалог каталога, ~15 приватных методов рендера/действий строк) и полного модуляsrc/device-inbox.ts— логика приоритета статусов,bindingCandidates, пагинация; - сверка терминологии с
docs/USER-GUIDE.ru.mdиdocs/SCOPE.md(J4/J6, admin persona, desktop-first) — не менялась в этом раунде за пределами правки M2, которая саму терминологию подтверждает, а не опровергает; - разбор 75 «прямых совпадений»
smoke-selectотorigin/dev— обоснование (общие символы_markerDialog/_saveConfigи т.п., не относящиеся к теме) не пересматривалось, так как ни один из этих участков кода не тронут коммитом8572d43c; - вывод о переходе
_showAll/legacysettings.show_all— не менялся.
Чего не проверял и почему
- Полный набор
demo/smoke_*.mjs(194 файла,ls demo/smoke_*.mjs | wc -l) — не прогонял целиком локально; CI на точном SHA8572d43c(run33125028109) уже прогнал все 3 шарда браузерных смоков полностью и все зелёные — это закрывает вопрос лучше, чем повторный локальный прогон. npm run golden:verify/ приёмка baseline — не прогонял; три новых golden-ID по-прежнему без базовых кадров (missing-baselineв CI job «Golden», run33125028109, лог подтверждён) — то же ожидаемое состояние, что и в r1, приёмка baseline — не обязанность код-ревью (PROCESS.md).python -m pytest tests_backend— diff не трогаетcustom_components/**/*.py.- Инварианты модели (
npm run invariants) — diff не трогает геометрию (нет изменений рёбер комнат,layout,marker.space,open_spans, записей толщины стен) — проверено grep'ом по диффу, гейт не применим. - Performance-профили — не затронуты, не названы в AC/§17 ТЗ.
- Ручной keyboard/screen-reader проход — не менялся этим коммитом, унаследовано из r1 (роли/aria не тронуты).
Вывод
Оба блокирующих/скоуповых замечания r1 (H1 — сломанный существующий смок,
M2 — комната каталога называлась по зоне HA, а не по имени комнаты плана)
закрыты по существу: чтением кода, перепрогоном тестов/смоков и реальным CI
на точном SHA задачи (шард с падением H1 теперь зелёный). L3 снято автором
чисто. Новых находок в коммите 8572d43c не обнаружено. Рекомендация:
зелёный вердикт.