houseplan-card/docs/reviews/SPEC-REVIEW-611-r1.md

SPEC-REVIEW-611-r1

Issue: #611 — «Импорт пространства не перепривязывает vacuum.map_routes[].space: на чужой установке импорт падает, на своей маршрут робота молча уводит на старый этаж» Этап: spec (PROCESS.md §2.4) Трек: small (лёгкий) — лимит циклов ревью ТЗ 2 Заход: r1 · блокирующих циклов израсходовано 0 из 2 Ревьюер: независимая сессия, без контекста автора ТЗ Вердикт: жёлтый

---

Скоуп

ТЗ живёт в теле issue #611 (раздел ## ТЗ), не в docs/specs/ — задача заведена после 2026-09-10 (#517). Предмет ревью — контракт, критерии приёмки AC1–AC3, откат и раздел «Принятые предположения» этого текста, проверенные против фактического кода custom_components/houseplan/import_export.py, validation.py, vacuum_routes.py на HEAD 61292c9e.

Заход первый: раздела «Закрытие предыдущего раунда» и «Унаследовано из r0» не требуются (§2.10 применяется со второго цикла).

Как проверялось

Ручного тестирования и исполнения кода на этапе ревью ТЗ не предусмотрено (артефакта для исполнения ещё нет — код не написан). Проверка велась чтением:

1. Прочитаны docs/SCOPE.md (задача закрывает J4/J6 — многоэтажный план, перенос пространства без потери привязки устройств), PROCESS.md §2.3/§2.4/§7.1, AGENTS.md.
2. Прочитано тело issue #611 целиком и единственный комментарий (аналитика: ценность 8/10, сложность 3/10, P2, трек small, обоснование трека — все критерии §5 выполняются).
3. Построчно сверены три утверждения ТЗ против кода:
   • custom_components/houseplan/import_export.py:1284-1660 (build_space_merge) — цикл по входящим маркерам (1415-1497), где сейчас ремапится marker.space (1428), marker.room_id (1432-1438), radar (1439-1454) и vacuum.segment_map (1455-1466), но не vacuum.map_routes[].space;
   • custom_components/houseplan/validation.py:910-951 (validate_marker_vacuum_routes) — вызывается на merged_config уже после сборки (import_export.py:1627), т.е. до неё контракт п.1 успевает выполниться;
   • custom_components/houseplan/vacuum_routes.py:106-146 (effective_routes) — подтверждает п.3 ТЗ: legacy-маршрут получает space из параметра dock_space, а не из отдельного сохранённого поля, значит ремап marker.space действительно достаточен для legacy-случая.
4. Прочитаны действующие тесты tests_backend/test_ha_import_export.py:
   • test_issue_162_space_export_drops_map_routes_of_other_spaces, test_issue_443_space_export_preserves_explicit_empty_routes, test_issue_162_full_export_round_trips_every_map_route, test_issue_162_space_export_keeps_legacy_calibration_untouched (строки 2678–2755) — подтверждают, что #162/#443 покрывают экспорт, а не перепривязку при импорте, как и заявляет «Механизм» issue;
   • test_space_remap_covers_marker_id_layout_and_vacuum_segment_map, test_issue_244_space_import_repairs_existing_target_refs_with_exact_map, test_issue_265_space_import_repairs_unique_previous_generation_lineage, test_issue_244_space_import_does_not_repair_target_while_source_exists (1491–1600+) — раскрыли путь, не упомянутый в ТЗ (см. находку M1 ниже).
5. Проверена ссылка AC3 на scripts/mutation-registry.mjs: файл действительно регистрирует backend-мутанты против custom_components/houseplan/import_export.py (например, окрестности строк 4938, 5596–5686, 6869–6898, 9474) — ссылка не выдумана, а корректна.
6. Проверено, нет ли в docs/CONFIG-COMPATIBILITY.md (раздел «Vacuum map routes (#162)», строки 163–195) и docs/ARCHITECTURE.md:409-420 утверждений, которые фикс сделал бы ложными, и не описывают ли они уже ремап при импорте пространства отдельно от map_routes (нет — оба документа молчат об идентификаторах при копировании пространства одинаково для marker.space, room_id и map_routes; заявление ТЗ «документация не требуется» самосогласовано с этим прецедентом, а не произвольно).
7. Проверена терминология: docs/USER-GUIDE.ru.md использует «пространство», «этаж», «робот-пылесос» — ТЗ не вводит новых терминов интерфейса (фикс не user-facing поверхность, только сообщение об ошибке импорта и changelog).

Дешёвые гейты (typecheck/test/build) и Validate на этом этапе неприменимы: продуктовый код ещё не написан, класс A не тронут.

Находки

Medium — M1: контракт не называет второй код-путь с идентичным дефектом (_repair_target_space_refs)

Файл/место: тело issue #611, раздел «Контракт» п.1–2 и «Критерии приёмки» AC1–AC2 (текст ТЗ); затрагиваемый код — custom_components/houseplan/import_export.py:1052-1270 (_repair_target_space_refs), вызывается из build_space_merge на import_export.py:1564, то есть до валидации на :1627 — тем же контрактом п.1, который говорит «в build_space_merge», без уточнения.

Формулировка: контракт п.1 и AC1/AC2 описывают перепривязку vacuum.map_routes[].space только для входящих маркеров документа (цикл 1415-1497, marker из incoming.get("markers")). Но build_space_merge вызывает и второй, отдельный проход — _repair_target_space_refs — который чинит уже существующие в целевой конфигурации маркеры, чей marker.space/room_id/vacuum.segment_map ссылались на мёртвый (более не live) ID, если этот ID лineage-разрешим к только что импортированному пространству (тесты test_issue_244_*, test_issue_265_*). В этом проходе (import_export.py:1152-1179) marker.space (1154), marker.room_id (1157-1165) и vacuum.segment_map (1166-1179) чинятся, а vacuum.map_routes[].space — нет: для этого прохода в файле вообще нет строки, аналогичной 1455-1466 из первого цикла. Ни контракт, ни AC1-AC3, ни «Принятые предположения» не упоминают этот путь — не как исключённый явно, а никак.

Почему это тот же дефект, а не гипотетический: тест test_issue_244_space_import_repairs_existing_target_refs_with_exact_map (строки 1507-1537) прогоняет ровно такой маркер — "vacuum": {"segment_map": {"12": "living"}} в целевой конфигурации, чей room_id/space чинятся через лineage/exact-map. Если у такого же целевого маркера вместо (или вместе с) segment_map стоит явный map_routes со space, указывающим на тот же старый ID, после починки marker.space он укажет на новое пространство, а vacuum.map_routes[0].space — на старый, уже мёртвый ID. Дальше — тот же исход, что описывает issue: либо invalid_vacuum_map_route (если старый ID действительно исчез из spaces), либо (если по случайности ID ещё жив в другом контексте — на практике не бывает, used исключает коллизии) молчаливо неверный этаж. Это тот же класс дефекта, что и заявленный в issue, просто триггер — не сам факт копирования пространства, а последующий ре-импорт после того, как исходное пространство уже когда-то было удалено (детач) и лineage-таблица его помнит.

Почему это не придирка к незаявленному сценарию: контракт п.1 буквально говорит «в build_space_merge» (имя функции, а не «в цикле по входящим маркерам»), и _repair_target_space_refs — код внутри той же функции, вызванный до той же строки валидации (:1627), к которой апеллирует контракт. Формулировка «до запуска validate_marker_vacuum_routes» относится к обоим проходам одинаково буквально.

Последствие для процесса: это Medium, поскольку не блокирует реализуемость и проверяемость AC1-AC3 как написано — они по-прежнему однозначны и доказуемы отдельно от этого пробела. Но раз пробел лежит в скоупе той же задачи (тот же баг-класс, тот же файл, тот же вызов внутри build_space_merge, соседний код уже показывает правильный паттерн на segment_map), это Medium в скоупе, а не повод заводить отдельный issue: чинится здесь же — либо явным расширением контракта/AC (AC4 на путь _repair_target_space_refs), либо явной записью в «Принятые предположения», почему этот путь сознательно не покрывается в #611 (например: «до отдельного issue, потому что репродукция требует предварительного детача пространства — двухшаговый сценарий вне аудита 22.09»). Молчание — не то же самое, что решение.

Не High: ни один из AC1-AC3 не становится невыполнимым или недоказуемым без этого расширения; проблема — в полноте контракта относительно собственной формулировки «в build_space_merge», а не в его противоречивости.

---

Проверено и корректно

• Механизм воспроизведён точно. old_space_id/new_space_id (import_export.py:1338,1342), details["space_id"] = new_space_id (:1648) — ТЗ ссылается на реальные имена и реальный порядок вызовов, не выдуманные.
• AC1/AC2 доказуемы уже сейчас, без домыслов о будущем API: create_export(kind="space"), parse_document, build_space_merge(..., same_source=...) — существующие публичные точки входа модуля, тест-файл tests_backend/test_ha_import_export.py уже содержит соседние тесты той же формы (тестовый паттерн _document(tmp_path, "space") + build_space_merge(...) используется многократно, включая test_issue_244_*/test_issue_265_*).
• П.3 контракта (legacy-маршрут вычисляется из marker.space) подтверждён кодом — vacuum_routes.py:106-146, effective_routes() берёт dock_space как параметр, отдельного сохраняемого route.space для legacy нет. Это не домысел, а точное описание существующего инварианта. Первое предложение п.3 («явный список не создаётся из legacy при импорте») тоже подтверждено — этот путь в build_space_merge не создаёт map_routes там, где их не было.
• П.4 (виртуализация теряет vacuum целиком) подтверждён — import_export.py:1489-1496, "vacuum" в списке полей, которые pop-аются при duplicate_policy == "virtual". Ремап map_routes, если он случится раньше проверки на 1467, для виртуализированного маркера станет мёртвым кодом, но не изменит наблюдаемый результат — контракт корректно не создаёт здесь новой обязанности.
• Ссылка на scripts/mutation-registry.mjs в AC3 реальна — файл уже регистрирует backend-мутанты именно против import_export.py, формат (guard: 'python3 -m pytest tests_backend/...') соответствует остальному реестру.
• Раздел «Принятые предположения» — не выдача догадки за факт. Утверждение об отказе перепривязывать «похожие», а не точно совпадающие ID, явно помечено как сохранение существующего fail-closed поведения и обосновано (повреждённые/вручную изменённые документы) — это ровно тот случай, когда решение принято и явно записано, а не спрятано.
• DoR-чеклист §2.5 закрыт по всем пунктам, кроме найденного M1: i18n (нет новых ключей — верно, ошибка использует существующий код invalid_vacuum_map_route), touch/performance (нет влияния — верно, чисто backend-импорт), откат (revert одним коммитом — реалистично, схема хранения не меняется, CONFIG_SCHEMA уже допускает map_routes через существующий vol.Optional), i18n en/ru changelog — план назван.
• Трек small подтверждён. Один backend-контракт (build_space_merge), никакой миграции конфигурации, никакого нового UX-контракта, i18n/touch/perf не затронуты — критерии §5 действительно выполняются все одновременно, а не выборочно.
• Продуктовая рамка отвечает на оба обязательных вопроса §7.1. Сценарий: администратор House Plan переносит/копирует пространство многоэтажного дома, где настроен пылесос с явными маршрутами (J4/J6 из docs/SCOPE.md). Что человек увидит: на чужой установке импорт пространства с настроенными маршрутами вместо падения invalid_vacuum_map_route проходит; на своей — робот на новой копии не «наследует» этаж старого оригинала молча.

Чего не проверял

• Исполнение кода не проводилось — на этапе ревью ТЗ кода ещё нет; все проверки — чтением существующего кода и тестов на HEAD 61292c9e, не запуском. Это ожидаемо для этапа spec (артефакта для запуска нет), отмечается явно по правилу «проверено чтением, не исполнением».
• Не проверялась производительность сборки build_space_merge на больших конфигурациях — ТЗ прямо заявляет «без влияния», и это правдоподобно для точечной правки одного цикла по маркерам без изменения сложности; отдельного перф-профиля для backend-импорта в проекте не предусмотрено.
• Не проверялось, существуют ли реальные, уже сохранённые у пользователей конфигурации, где сработал бы путь M1 (_repair_target_space_refs + явный map_routes на целевом маркере) — это гипотетический, но code-grounded сценарий; фактическая частота не оценивалась и не требуется на этапе ревью ТЗ.

Материал раунда

• Issue: #611, тело проверено на состоянии, зафиксированном при выдаче задачи ревьюеру.
• sha256 нормализованного (UTF-8, как получено через gh issue view --json body) тела issue: 94e2253da2c31f34919377a64987560e85945a765281dac78e320a29231f1553 (4369 байт).
• Код-база: 61292c9ea05f0d7d3a98392c81f3be89f2706c7c (рабочая копия соответствует HEAD).
• Комментарий аналитики учтён (ценность 8/10 · сложность 3/10 · P2 · тип bug · трек small).

Итог

Вердикт: жёлтый. AC1-AC3 сформулированы однозначно и доказуемо, механизм и все технические ссылки в ТЗ точны и проверены построчно против кода — это добротная работа. Единственная находка (M1, Medium, в скоупе) не рушит написанные критерии, но оставляет незакрытым второй код-путь с тем же классом дефекта внутри той же функции. Возврат автору: расширить контракт/AC на _repair_target_space_refs, либо явно и обоснованно исключить этот путь в «Принятых предположениях».

---

Материал раунда

• Ветка: dev, коммит 61292c9ea05f — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: 008da06ab0a721f37308e9af3cf9fbfad2c659de

  git log --all --format='%H %T' | grep 008da06ab0a7
  

• Тело issue: e67628cb6a8a9b34c284381c57eb7ef1105361614b8bf1341c41e4af19b87f19
• Вердикт конвейера: yellow · High 0