diff --git a/docs/reviews/CODE-REVIEW-443-r1.md b/docs/reviews/CODE-REVIEW-443-r1.md new file mode 100644 index 00000000..3d89c7a4 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-443-r1.md @@ -0,0 +1,243 @@ +# CODE-REVIEW-443-r1 + +- Issue: https://github.com/Matysh/houseplan-card/issues/443 +- Ветка: `issue/443-vacuum-route-polish` +- Материал: `git diff origin/dev...HEAD`, `HEAD = 1e5f33aa6413cb68659d5d1dcdb2b374a7404bda` +- ТЗ: `docs/specs/443-vacuum-route-polish.md`, ревью ТЗ зелёное на r2 + (`docs/reviews/SPEC-REVIEW-443-r2.md`) +- Заход: r1 (первый код-ревью цикл этой задачи) + +## Скоуп диффа + +Один коммит поведения `cebd20a9` (`fix: polish vacuum map routes`) плюс +promotion-коммит `1e5f33aa` (пересборка бандла и канонических скриншотов). +Три независимых правки, всё согласно контракту ТЗ §1–§4: + +1. `map_routes` authority — `src/vacuum-routes.ts::effectiveRoutes()`, + `custom_components/houseplan/vacuum_routes.py::effective_routes()`: + условие `Array.isArray(x) && x.length` → `Array.isArray(x)`, отдельно на + frontend и backend. +2. Single-space export — `custom_components/houseplan/import_export.py`: + `kept_routes or None` → `kept_routes` (пустой список пишется как есть). +3. Группа «Пространство удалено» — `src/editors/vacuum-maps-section.ts`: + разбиение `routes` на `validRows`/`missingRows`, отдельная секция с + локализованным заголовком; `src/i18n/support/{en,ru,de,fr}.json`, + `src/styles/dialogs.styles.ts`. +4. Vacuum-only render snapshot — `src/render-device-snapshot.ts` (`vacuumDevices` + subset, заморожен), `src/houseplan-card.ts` (`_renderVacuumDevices` getter и + новый call site `_renderVacuums(this._renderVacuumDevices, …)`). + +Плюс тесты/мутанты (`test/vacuum-routes.test.mjs`, +`test/render-device-snapshot.test.mjs`, `test/isometric-contract.test.mjs`, +`tests_backend/test_vacuum_routes.py`, `tests_backend/test_ha_import_export.py`, +`scripts/mutation-gate.mjs`, `demo/smoke_vacuum_route_draft.mjs`) и документация +(`docs/VACUUM.md`, `docs/CONFIG-COMPATIBILITY.md`, `docs/USER-GUIDE.ru.md`, оба +changelog) — все пять release-артефактов, названных ревью ТЗ, на месте. + +## Как проверялось + +**Дешёвые гейты подтверждены на этом SHA, не перегонялись.** Validate зелёный +на точном `1e5f33aa` +(https://github.com/Matysh/houseplan-card/actions/runs/33802298798): +джобы `frontend` (typecheck/unit/mutation-gate/bundle:sync), +`backend` (pytest в HA), `golden`, `performance_smoke`, все 3 шарда `smoke`, +`docs`, `provenance`, `process-gate`, `hassfest`, `hacs` — все `success`, я +сверил через `gh run view --json jobs` лично, не только со слов автора. + +Отдельно проверены с сохранением headSha: +- полный performance-прогон (7 профилей, включая large-house 60/200) — + `33802485595`, `headSha = 1e5f33aa…`, success; +- каноническая съёмка документации — `33802095608`, success (на SHA + промежуточного дерева `1fd042d9`, откуда получены новые PNG, зафиксированные + затем в promotion-коммите `1e5f33aa`; сам `docs` job Validate подтверждает + fingerprint против финального дерева отдельно и тоже зелёный). + +Гейты, которые прогнал сам ревьюер (дёшево, воспроизводимо): + +| Гейт | Команда | Результат | +|---|---|---| +| smoke-select | `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | 6 прямых совпадений: `smoke_cold_view_vacuum`, `smoke_controls`, `smoke_glow_fail_dark`, `smoke_glow_geometry_resilience`, `smoke_vacuum_firstuse`, `smoke_vacuum` — все входят в полный набор, уже прогнанный зелёным в Validate (3/3 шарда) | +| no-new-any | `node scripts/no-new-any.mjs --base origin/dev --head HEAD` | «Новых any нет» (83 добавленные строки в 5 файлах) | + +Не прогонял: `npx tsc --noEmit`, `npm test`, `npm run build` — покрыты Validate +на этом же SHA (условие задачи: не гонять то, что уже зелёное на точном SHA). +`npm run model-invariants` не запускался — дифф не трогает рёбра комнат, записи +толщины, `layout`, `marker.space` (сравнивается `route.space`, поле маршрута +робота, не геометрия) или `open_spans`; гейт неприменим. `python -m pytest +tests_backend -q` не гонял отдельно — покрыт зелёным backend-job Validate на +этом SHA. Полный `golden:capture`/локальный HA-harness не запускал — канон +Linux CI зелёный на точном SHA, второй прогон ничего не добавляет. + +## Разбор по AC + +**AC1 (единая route authority).** `test/vacuum-routes.test.mjs` добавляет +`explicit empty routes remain authoritative over legacy calibration` — три +состояния: absent (legacy), `null` (legacy), `[]` (пусто, без legacy). Защитный +AC, доказан таблицей «чем краснеет»: + +| AC | Чем доказан | Чем краснеет | +|---|---|---| +| AC1 (frontend) | `node --test --test-name-pattern="explicit empty routes remain authoritative" test/vacuum-routes.test.mjs` | мутант `vacuum-empty-routes-revive-legacy-frontend` возвращает `explicit.length`, гейт красный (проверено по определению мутанта в `scripts/mutation-gate.mjs:384-396`, сам mutation-gate job зелёный в Validate — значит применённая мутация действительно ловится) | +| AC2 (backend) | `tests_backend/test_vacuum_routes.py::test_explicit_empty_routes_remain_authoritative` | мутант `vacuum-empty-routes-revive-legacy-backend` (то же условие в Python) | +| AC3 (export) | `tests_backend/test_ha_import_export.py::test_issue_443_space_export_preserves_explicit_empty_routes` — экспортирует marker с explicit routes, полностью отфильтрованными по space, и уцелевшей legacy `calibration`; проверяет `map_routes == []`, `effective_routes(...) == []` после round-trip | мутант `vacuum-space-export-drops-empty-authority` возвращает `kept_routes or None` | + +Читкой подтверждено: `writeRoutes()` в `vacuum-maps-section.ts:129` +(`explicit ? (vacuum.map_routes ?? null) : null`) при `map_routes: []` берёт +пустой массив как базу (`![]` есть `false` в JS — ветка `convertLegacyRoutes` +не выполняется), то есть UI-редактирование поверх уже-пустого explicit-состояния +не оживляет legacy при следующей записи. Отдельного unit на этот конкретный +путь нет, но `demo/smoke_vacuum_route_draft.mjs:50-72` стартует именно с +`map_routes: []` и успешно проводит через него draft-flow — риск низкий, тот же +`explicit`-флаг, что доказан AC1/AC2. + +**AC2 (backend parity).** См. таблицу выше; TS и Python проверяют одну и ту же +четырёхстрочную матрицу independently, значения совпадают (absent/null → 1 +маршрут, `[]` → 0). + +**AC3 (export).** См. таблицу; дополнительно `assert +document["transfer"]["dropped_marker_links"] == 1` — счётчик не подменяется +побочно. + +**AC4 (группа «Пространство удалено»).** Контракт §3 — не защитный AC +(расположение/текст/группировка), доказательство обычным сравнением достаточно. +`demo/smoke_vacuum_route_draft.mjs:150-192` (часть полного зелёного smoke-прогона +в Validate) проверяет: ровно одна группа +(`[data-hp="vacuum-route-missing-group"]`), заголовок `"Deleted space"`, +валидные строки идут первыми и не входят в группу, три missing-строки +отсортированы по идентичности (`missing-a,missing-b,missing-z` — по +`map_id`→`source`, а не по `space`, соответствует §3.2), каждая строка группы +сохраняет `