diff --git a/docs/reviews/SPEC-REVIEW-611-r2.md b/docs/reviews/SPEC-REVIEW-611-r2.md new file mode 100644 index 00000000..7660e5a0 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-611-r2.md @@ -0,0 +1,211 @@ +# SPEC-REVIEW-611-r2 + +**Issue:** [#611](https://github.com/Matysh/houseplan-card/issues/611) — «Импорт пространства не перепривязывает +`vacuum.map_routes[].space`: на чужой установке импорт падает, на своей маршрут робота молча уводит на старый этаж» +**Этап:** spec (PROCESS.md §2.4) +**Трек:** `small` (лёгкий) — лимит циклов ревью ТЗ 2 +**Заход:** r2 · блокирующих циклов израсходовано 1 из 2 (r1 был жёлтым и цикл потратил, зелёный его не тратит, §7.2/#227) +**Ревьюер:** независимая сессия, без контекста автора ТЗ +**Вердикт:** зелёный + +--- + +## Скоуп + +ТЗ живёт в теле issue #611 (раздел `## ТЗ`), не в `docs/specs/`. Предмет ревью r2 — дельта тела issue между +раундами r1 и r2 и то, чего эта дельта касается: контракт п.1–5, AC1–AC3, «Принятые предположения», «Откат» — +проверенные против фактического кода `custom_components/houseplan/import_export.py` и `vacuum_routes.py`. + +Кодовая база не менялась между раундами: HEAD r1 был `61292c9ea05f...` (материал r1), текущий HEAD +`37aed2c9b13b333ba8adb2c97f32158e9606618b` добавляет только сам документ `docs/reviews/SPEC-REVIEW-611-r1.md` +(коммит «docs: review document for #611») — продуктовый код (класс A) не тронут. Значит «дешёвые гейты» (§2.10) +здесь неприменимы по той же причине, что и в r1: артефакта для запуска ещё нет. + +**Разбор по дельте, а не заново (§2.10).** Дельта — правка ровно того, что r1 назвал M1 (Medium, в скоупе): +контракт получил новый п.2 (`_repair_target_space_refs`), AC2 полностью переписан под ремонт целевого маршрута, +добавлена строка в «Принятые предположения» про переиспользование `_lineage_resolver`. Это не ребейз, не смена +подсистемы и не контракта поведения за пределами уже заявленного бага — объём дельты меньше исходной задачи и +локален к одной функции того же файла, поэтому полный разбор не требуется; проверялась дельта и всё, до чего она +дотягивается (AC2 целиком, консолидированный AC1, п.2–4 контракта, ремонт в «Откате»). + +## Как проверялось + +1. Получено тело issue #611 через `gh issue view --json body` и полная история правок тела через GraphQL + `userContentEdits` (`editor`, `editedAt`, `diff`) — 3 правки автора (Matysh): создание 2026-09-22T19:53:51Z, + заполнение ТЗ 2026-09-23T01:14:05Z (материал r1), правка по итогам r1 2026-09-23T01:24:00Z (материал r2, + текущее состояние). Между node[1] (r1) и node[0] (r2) выполнен точный `diff -u` — дельта установлена по + содержимому, а не по пересказу автора. +2. Найден вердикт r1 (комментарий issue, 2026-09-23T01:22:09Z) и документ `docs/reviews/SPEC-REVIEW-611-r1.md` + (уже закоммичен, HEAD `37aed2c9`). Единственная находка r1 — Medium M1: контракт п.1/AC1-AC2 перепривязывали + `vacuum.map_routes[].space` только для входящих маркеров, но не для второго прохода + `_repair_target_space_refs`, который ремонтирует уже существующие целевые маркеры. +3. По дельте построчно сверены с кодом: + - `custom_components/houseplan/import_export.py:1013-1049` (`_lineage_resolver`) — резолвер возвращает ровно + пять исходов: `live`, `exact`, `lineage`, `ambiguous`, `unrelated`. Новый контракт п.2 покрывает все пять + («живая ссылка... не меняется» = live; «доказанная exact-ссылка либо единственная безопасная + lineage-ссылка... заменяется» = exact/lineage; «неоднозначная... сохраняется... `preservedUnresolved`» = + ambiguous; «несвязанная... отклоняется последующей валидацией» = unrelated). Формулировка точна, без + недостающих случаев. + - `custom_components/houseplan/import_export.py:1052-1279` (`_repair_target_space_refs`) прочитан целиком. + Подтверждено: `resolve_space = _lineage_resolver(...)` (строка 1077) уже создан и уже применяется к + `marker.space` безусловно, без гейта `may_rebind_room` (строка 1154) — контракт п.2 требует того же + безусловного применения к `vacuum.map_routes[].space`, что технически корректно (это прямая ссылка на + пространство, как и `marker.space`, а не ссылка на комнату внутри чужого пространства, для которой гейт + нужен). Подтверждено также: сейчас в этой функции **нет** ни одной строки, ремапящей `map_routes` — ровно + тот пробел, который описал M1, и он остаётся открытым до реализации; ТЗ его теперь явно контрактует. + - `custom_components/houseplan/import_export.py:967-1001` (`_empty_reference_report`, `_report_remap`) — + `remapped: {"incoming": {}, "target": {}}` и `preservedUnresolved` уже существуют как открытые словари + произвольных категорий; добавление категории `marker.vacuum.map_routes.space` в `.target` не требует + смены схемы отчёта. `repaired_target_refs` — существующий именованный счётчик, возвращаемый функцией + (строка 1059) и попадающий в `merged_config` (строка 1655) — контракт п.3/AC2 ссылаются на реальные имена. + - `custom_components/houseplan/import_export.py:1415-1466` (цикл по входящим маркерам) — подтверждает, что + соглашение именования категорий (`marker.vacuum.segment_map` для входящих, строка 1463) прямо + аналогично новой `marker.vacuum.map_routes.space` — не изобретённое имя. + - `custom_components/houseplan/vacuum_routes.py:104-142` (`effective_routes`) — не менялся с r1; по-прежнему + подтверждает п.4 (legacy-маршрут вычисляет `space` из параметра `dock_space`, отдельного сохраняемого поля + нет) — эта часть дельты не касалась, но проверена заново, так как п.4 в дельте переформулирован (убрана фраза + «существующая валидация должна по-прежнему отвергать неизвестную ссылку» — не потеря гарантии: та же гарантия + теперь явно и без потери смысла стоит в п.2 для целевого прохода, а для входящего прохода валидация всё так + же выполняется один раз на объединённом `merged_config` после обеих правок, строка 1627, — не задвоена и не + обязана быть задвоена). + - `custom_components/houseplan/import_export.py:1489-1496` (виртуализация теряет `vacuum` целиком) — не + менялся; контракт п.5 (было п.4) по-прежнему точен. +4. Перечитаны имена тестов, на которые ссылается новый AC2: `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` (существуют, + `tests_backend/test_ha_import_export.py:1507,1540,1589`) и `test_issue_162_*`/`test_issue_443_*` + (существуют, строки 2678–2755) — доказательная база AC2 не выдумана, тесты рядом с ней реальны и уже + упражняют именно `_repair_target_space_refs` с `vacuum`-блоком целевого маркера (сейчас — через + `segment_map`; AC2 просит аналогичный тест через `map_routes`). +5. Проверено, не меняет ли дельта классификацию трека `small` (§5): новая категория отчёта — просто ключ в уже + существующем открытом словаре, не смена схемы/версии модели; `small` остаётся верным. +6. `docs/CONFIG-COMPATIBILITY.md` («Vacuum map routes (#162)», строки 163–195) перечитан ещё раз — описывает + только экспорт и downgrade-семантику, не претендует на поведение ремонта ссылок при импорте; заявление ТЗ + «документация не требуется» остаётся самосогласованным и после расширения контракта. + +Дешёвые гейты (`tsc`/`test`/`build`) и `check-docs.mjs` не гонялись — на этапе ревью ТЗ нет ни изменённого кода, +ни изменённого `src/**`; неприменимо ровно как в r1. + +## Находки + +Находок нет. Дельта закрывает M1 полностью и не вводит новых Medium/High. + +Рассмотрен и сознательно не поднят как находка один пограничный момент: AC3 («тест чувствителен к снятию +защиты») остаётся сформулирован в единственном числе и не перечисляет отдельно оба поведения контракта (ремап +входящего маршрута из п.1/AC1 и ремонт целевого маршрута из п.2/AC2). Это не блокирует и не делает ТЗ +неоднозначным: формулировка AC3 общая («перепривязку `map_routes[].space]`»), проект уже регистрирует несколько +мутантов на один файл под соседние issue (r1 отметил несколько адресов в `scripts/mutation-registry.mjs`), и +проверка фактической полноты мутационного покрытия обоих путей — предмет код-ревью (§2.7), а не ревью ТЗ: там +проверяется реестр и его способность падать, здесь — только проверяемость критерия по тексту. Low снят без +правки, с этой записью. + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где это видно | +|---|---|---| +| **M1** (Medium, в скоупе) — контракт п.1/AC1-AC2 перепривязывают `vacuum.map_routes[].space` только для входящих маркеров; второй проход `_repair_target_space_refs` (чинит уже существующие целевые маркеры через lineage/exact-map) пропускает то же поле. | Добавлен контракт **п.2**: `vacuum.map_routes[].space` целевых маркеров проходит через тот же `resolve_space`, что и `marker.space`, с полным перечислением всех пяти исходов резолвера (live/exact/lineage/ambiguous/unrelated). AC2 полностью переписан под «ремонт целевого маршрута» с явной ссылкой на `reference_report.remapped.target`, `repaired_target_refs`, `preservedUnresolved` и тесты #244/#265. Раздел «Принятые предположения» добавляет: для целевого repair используются канонические правила `_lineage_resolver`, новый эвристический поиск не вводится. «Откат» и «Механизм»/«Проблема» переформулированы, чтобы явно называть `_repair_target_space_refs`. | Тело issue #611, разделы «Контракт» п.2, «Критерии приёмки» AC2, «Принятые предположения» (правка автора 2026-09-23T01:24:00Z, подтверждено точным `diff -u` между версией тела на момент r1 и текущей). Код-грaунд: `import_export.py:1013-1049` (`_lineage_resolver`, 5 исходов), `:1077,1154` (`resolve_space`, уже создан и уже безусловно применяется к `marker.space` в том же проходе — контракт просит того же обращения с `map_routes[].space`), `:967-971,1059,1655` (`remapped.target`, `repaired_target_refs`, `preservedUnresolved` — реальные существующие поля отчёта). | + +## Унаследовано из r1 + +Принято без повторной проверки (дельта их не касается): + +- **Механизм и репродукция** (кроме двух предложений, добавленных про `_repair_target_space_refs`, которые + проверены заново выше) — сверены в r1 построчно с кодом на `61292c9ea05f...`; код с тех пор не менялся. +- **AC1, содержательная часть про `same_source=False/True`, `create_export(kind="space")`, `parse_document`, + `build_space_merge`** — существующие публичные точки входа, уже подтверждённые в r1; дельта лишь + консолидировала старые AC1+AC2(r1) в один AC1(r2), не изменив технических утверждений об этих вызовах. +- **П.4 контракта, первое подтверждение** (`vacuum_routes.py:106-146`, legacy-маршрут вычисляется из + `marker.space`, отдельного `route.space` нет) — переподтверждено в этом раунде отдельно (см. «Как + проверялось», п.3), хотя формулировка в тексте не менялась по существу. +- **П.5 контракта (виртуализация теряет `vacuum` целиком; политика дубликатов; экспорт не меняется)** — + переподтверждено (`import_export.py:1489-1496`), текст не менялся по существу. +- **AC3 (мутационная защита) и ссылка на `scripts/mutation-registry.mjs`** — текст не менялся; принято по + выводу r1 («ссылка не выдумана, файл действительно регистрирует backend-мутанты против `import_export.py`»). + Полнота охвата обоих путей — см. «Находки» выше, оставлено на код-ревью. +- **DoR-чеклист §2.5** (i18n — нет новых ключей; touch/performance — без влияния; откат — один revert, + реалистично) — переподтверждён в этом раунде тем, что дельта не добавляет ни новых пользовательских строк, + ни нового UX-контракта, ни миграции; тексты «Release-артефакты» не менялись между r1 и r2 (вне диапазона + правки, подтверждено `diff -u`). +- **Продуктовая рамка и track `small`** — сценарий/что человек увидит (§7.1) и обоснование трека `small` не + менялись; сверено, что дельта не превращает задачу в задачу с новым UX-контрактом или миграцией (см. «Как + проверялось», п.5). + +Документ и материал предыдущего раунда: `docs/reviews/SPEC-REVIEW-611-r1.md`, код-база +`61292c9ea05f0d7d3a98392c81f3be89f2706c7c`, тело issue на момент r1 — правка автора от 2026-09-23T01:14:05Z +(идентифицирована через GraphQL `userContentEdits`, `sha256` содержимого `94e2253da2c3…f1553`, 6811 байт — +уточнение фактического размера: r1 указал 4369 байт при том же хеше, это арифметическая неточность прозы r1, +не расхождение материала). + +## Проверено и корректно + +- **Дельта устанавлена по точному диффу, а не по пересказу.** Через GraphQL `userContentEdits` получены обе + версии тела issue (до и после правки автора 2026-09-23T01:24:00Z) и сопоставлены `diff -u` — видно ровно то, + что изменилось: контракт п.2 (новый), п.3 (расширен на `.target`/`repaired_target_refs`), AC1 (консолидация + двух прежних AC), AC2 (переписан), «Принятые предположения» (новая первая строка), «Откат»/«Механизм»/ + «Проблема» (уточнены упоминанием `_repair_target_space_refs`). Ничего за пределами этого дельта не менялось. +- **Контракт п.2 технически реализуем без нового кода-пути.** `resolve_space` уже создаётся в + `_repair_target_space_refs` и уже используется без доп. условий для `marker.space` — расширение на + `vacuum.map_routes[].space` для существующего маркера не требует новой архитектуры резолвинга, только новый + цикл по списку маршрутов, аналогичный уже существующему циклу по `segment_map` (`:1166-1179`). + Пять исходов резолвера перечислены в контракте без пропусков. +- **AC2 доказуем существующей тестовой инфраструктурой.** Тесты `test_issue_244_*`/`test_issue_265_*` уже + прогоняют целевой маркер с `vacuum`-блоком через тот же путь ремонта; AC2 просит симметричный тест для поля + `map_routes` — не новый вид теста, а естественное расширение существующего паттерна. +- **Единообразие именования сохранено.** `marker.vacuum.map_routes.space` продолжает схему + `marker.vacuum.segment_map`, `marker.space`, `marker.room_id` — не изобретённая категория. +- **Смена состава AC1/AC2 не потеряла содержания.** Всё, что проверял r1 в старых AC1 (импорт на чужую + установку) и AC2 (импорт на исходную установку, `same_source=True`), присутствует в новом AC1 дословно + (оба варианта `same_source` явно перечислены). +- **Раздел «Принятые предположения» не подменяет решение владельца.** Уточнение «используются канонические + exact/lineage/live/ambiguous правила `_lineage_resolver`; новый самостоятельный эвристический поиск не + вводится» — техническое решение в зоне «агенты решают сами» (§7.1), не продуктовый вопрос, и явно записано, + а не домыслено молча. + +## Чего не проверял + +- **Исполнение кода** — по-прежнему не проводилось; кода для AC2 ещё нет, весь разбор — чтением, отмечено как + «проверено чтением, не исполнением». +- **Полнота мутационного покрытия обоих путей (входящий ремап + целевой ремонт) по AC3** — см. «Находки»: + сознательно оставлено код-ревью, поскольку это вопрос фактического количества зарегистрированных мутантов + при реализации, а не однозначности критерия сейчас. +- **Реальная частота срабатывания сценария M1** (существующие у пользователей конфигурации с детачем + пространства и явным `map_routes` на целевом маркере) — не оценивалась, не требуется на этапе ревью ТЗ; тот же + пункт был не проверен и в r1, дельта его не касается. +- **Байтовая точность величин, приведённых в прозе r1** (4369 против фактических 6811 байт при совпадающем + sha256) — не является предметом этого ревью; расхождение зафиксировано выше в «Унаследовано из r1» как + наблюдение, не как находка (сам хеш и, следовательно, содержимое, идентифицированы верно). + +## Материал раунда + +- Issue: #611, тело проверено на состоянии после правки автора 2026-09-23T01:24:00Z (текущее на момент выдачи + задачи ревьюеру r2). +- Тело issue (r2, sha256 реального содержимого через `hashlib.sha256` над UTF-8 текстом, полученным и через + `gh issue view --json body`, и независимо через GraphQL `userContentEdits[0].diff`, оба совпали): + `3c8e0418858130189ed12efb3ba8f2c2c31de4b3dd376453751ed89f91e27189` (9081 байт). +- Тело issue (r1, для дельты, через GraphQL `userContentEdits[1].diff`): + `94e2253da2c31f34919377a64987560e85945a765281dac78e320a29231f1553` (6811 байт). +- Код-база: `61292c9ea05f0d7d3a98392c81f3be89f2706c7c` (не менялась с r1; HEAD `37aed2c9b13b333ba8adb2c97f32158e9606618b` + добавляет только `docs/reviews/SPEC-REVIEW-611-r1.md`, класс A не тронут). +- Комментарий аналитики (2026-09-23T01:13:13Z) и вердикт r1 (2026-09-23T01:22:09Z) учтены. + +## Итог + +**Вердикт: зелёный.** Единственная находка предыдущего раунда (M1, Medium, в скоупе) закрыта точно и полностью: +новый контракт п.2 и переписанный AC2 явно распространяют перепривязку `vacuum.map_routes[].space` на второй +код-путь (`_repair_target_space_refs`), формулировка корректно перечисляет все пять исходов резолвера, ссылается +на реальные имена полей и функций и опирается на уже существующую тестовую инфраструктуру (#244/#265). Новых +Medium/High дельта не вводит. ТЗ готово к разработке. + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `37aed2c9b13b` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `99f40908d3eb60c13a07143e745f366cd2d0b733` + ``` + git log --all --format='%H %T' | grep 99f40908d3eb + ``` +- Тело issue: `bb14ef7839530fe63fc2aedfdcf59e5433cbb80fa20c2a3a84a0971f818fb78e` +- Вердикт конвейера: `green` · High 0