mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-03 21:28:59 +00:00
@@ -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 дельта не вводит. ТЗ готово к разработке.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `dev`, коммит `37aed2c9b13b` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `99f40908d3eb60c13a07143e745f366cd2d0b733`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 99f40908d3eb
|
||||
```
|
||||
- Тело issue: `bb14ef7839530fe63fc2aedfdcf59e5433cbb80fa20c2a3a84a0971f818fb78e`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user