mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-01 04:09:17 +00:00
@@ -0,0 +1,194 @@
|
||||
# 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`, жёлтый, проверен на
|
||||
SHA `905d4847a3797a8830eb6c5c509351f142193b73` (эквивалент текущего
|
||||
`4b3b71e73bc369d04f57ef000a30465571932bf1` — см. «Скоуп разбора» ниже, это
|
||||
тот же коммит после ребейза, содержимое подтверждено идентичным)
|
||||
|
||||
## Скоуп разбора: почему разбор полный, а не по дельте
|
||||
|
||||
Заголовок задачи говорит: между r1 и этим раундом ветка приведена конвейером
|
||||
к `dev` — «поверх легло 1 коммит(ов) dev, `11c0cbd4 -> 8572d43c`». Это ровно
|
||||
условие §7.2 «после ребейза это другой код» — по умолчанию разбор полный.
|
||||
Я не поверил этому на слово и проверил, что ребейз действительно ничего не
|
||||
подмешал:
|
||||
|
||||
- коммит `4b3b71e7` («feat: add device lifecycle catalog») — это тот самый
|
||||
коммит, что рецензировался в r1 под SHA `905d4847` (сообщение коммита то
|
||||
же самое, дифф-статистика совпадает построчно: `src/device-inbox.ts` — ровно
|
||||
281 строка, как в тексте r1, `src/houseplan-card.ts` — 536 вставок против
|
||||
536 в свежем `git show --stat`); сам SHA `905d4847` в дереве больше не
|
||||
существует именно потому, что рецензируемая история была переиграна поверх
|
||||
нового `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_picker` 24/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`/legacy `settings.show_all` — не менялся.
|
||||
|
||||
## Чего не проверял и почему
|
||||
|
||||
- **Полный набор `demo/smoke_*.mjs` (194 файла, `ls demo/smoke_*.mjs | wc -l`)**
|
||||
— не прогонял целиком локально; CI на точном SHA `8572d43c`
|
||||
(run `33125028109`) уже прогнал все 3 шарда браузерных смоков полностью и
|
||||
все зелёные — это закрывает вопрос лучше, чем повторный локальный прогон.
|
||||
- **`npm run golden:verify` / приёмка baseline** — не прогонял; три новых
|
||||
golden-ID по-прежнему без базовых кадров (`missing-baseline` в CI job
|
||||
«Golden», run `33125028109`, лог подтверждён) — то же ожидаемое состояние,
|
||||
что и в 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` не обнаружено. Рекомендация:
|
||||
зелёный вердикт.
|
||||
Reference in New Issue
Block a user