docs: review document for #265

Issue: #265
User-Visible: no

docs/reviews/SPEC-REVIEW-265-r1.md

@@ -0,0 +1,251 @@
+# SPEC-REVIEW-265-r1
+
+- Issue: [#265](https://github.com/Matysh/houseplan-card/issues/265) — «Рефакторинг 2/5: один контракт шва импорта — ремап ссылок за собой и идемпотентная уборка»
+- Этап: `S4-spec-review` (ревью ТЗ, PROCESS.md §2.4)
+- Заход: r1 · блокирующих циклов израсходовано 0/4 (до вердикта)
+- ТЗ: `docs/specs/265-import-reference-seam.md`, ветка `issue/265-import-seam-contract`, SHA `2cf47fc1`
+- Ревьюер: Claude (свежая сессия, без переписки с автором)
+
+## Скоуп ревью
+
+Первый заход — разбор полный, дельта не применяется (§2.10 относится со второго
+цикла). Материал: тело issue #265, оба комментария аналитика/автора, файл ТЗ
+целиком (`docs/specs/265-import-reference-seam.md`, 454 строки) и правка
+`docs/specs/README.md` (+1 строка — таблица ссылок).
+
+## Как проверялось
+
+1. Прочитаны `docs/SCOPE.md`, `PROCESS.md` (целиком, включая §2.4/§2.5/§2.9/§7.1),
+   `AGENTS.md`.
+2. Прочитано тело issue #265 и оба комментария (`Взял: …`, аналитика,
+   «ТЗ готово»).
+3. Проверены утверждения §2 «Подтверждённое состояние кода» против реального
+   `custom_components/houseplan/import_export.py`: `_fresh()` (строка 804),
+   `_repair_target_space_refs()` (825), `build_space_merge()` (875),
+   `create_preview()`/`revalidate_candidate()`/`prepare_apply()` (1214, 1330,
+   1381). Все пять пунктов §2 подтверждены построчно — это не догадки:
+   `_fresh()` действительно строит stem из текущего `old` (без канонизации),
+   `_repair_target_space_refs()` действительно ремонтирует только exact-map
+   пяти перечисленных полей, preview/apply действительно каждый раз вызывают
+   `build_space_merge()` заново со случайным `secrets.token_hex(4)`.
+4. Проверено утверждение §2.4 о `scripts/model-invariants.mjs`: функция
+   `checkReferences()` (строки 118–194) действительно не проверяет
+   `marker.room_id`, `vacuum.segment_map`, `rooms[].open_to`, `marker:*`
+   controls/value badge и partition opening host — совпадает с текстом ТЗ.
+5. Проверены поля матрицы §7 против кода: `controls[]` с префиксом `marker:`,
+   `value_badge.ref`, `vacuum.segment_map`, `rooms[].open_to`,
+   `openings[].host.id` при `host.kind == "partition"` — все существуют в
+   `import_export.py` (строки 482–497, 750–780, 848–853, 916–927, 970–1014).
+   Матрица не изобретает несуществующие поля и не молчит про существующие,
+   которых коснулась выборка.
+6. Проверены три «унаследованных решения владельца» (§3) против статуса и
+   текста связанных issue/спеков: #244, #248, #252, #258, #262 — все
+   `CLOSED` (`gh issue view`), тексты их ТЗ (`docs/specs/244-*.md`,
+   `252-*.md`, `262-*.md`) подтверждают ровно те формулировки, которые #265
+   им приписывает (tombstone не собирается по сроку — #262 §6.2; отчёт живёт
+   внутри «Оптимизировать планы» — #252 §1–2; неразрешимая ссылка не
+   угадывается — #244 AC3/AC5/AC6). Ни один из трёх вопросов, которые исходный
+   текст issue адресовал владельцу, не решён произвольно: все три закрыты уже
+   принятыми (закрытыми) задачами, а не мнением автора.
+7. Сверена терминология: «Оптимизировать планы», «Общие настройки» —
+   совпадает с `docs/USER-GUIDE.ru.md` (строки 402, 1425 и др.). «carrier» —
+   совпадает с `docs/WALL-THICKNESS.md`. Новый текст отчёта (§9) — это
+   черновик новой строки, не существующей ранее ни в одном каноне; это
+   ожидаемо для новой фичи и не является нарушением («термин из
+   USER-GUIDE» требование касается уже существующей терминологии интерфейса).
+8. Сверена структура документа с двумя прямыми «братьями» той же серии
+   рефакторинга — `docs/specs/244-orphan-space-references.md` и
+   `docs/specs/252-optimize-orphan-layout-report.md` (тот же автор, тот же
+   класс задачи, ссылаются друг на друга) — чтобы отличить «этот документ так
+   устроен потому что задача другая» от реального отступления от конвенции.
+9. Технически проверена состоятельность алгоритма канонизации lineage (§5.1,
+   edge case 1): смоделирован случай, когда пользовательский id случайно
+   принимает форму `<namespace>_<stem>_<8 lowercase hex>` и совпадает root'ом
+   с реальным импортным id. Риск ложного совпадения существует, но заперт
+   пятью одновременными условиями §5.2 (мертва исходная ссылка + ровно один
+   живой кандидат + совместимый тип + отсутствие exact id + отсутствие второго
+   кандидата с тем же root) и прямо назван эвристикой, а не идентичностью
+   (§17). Блокирующим не считаю.
+10. `node scripts/check-docs.mjs` не запускался: diff не касается `src/**`
+    (только `docs/specs/**`), гейт неприменим по своему собственному условию
+    («любая правка фронтенда» — здесь правки фронтенда нет). Остальные гейты
+    (`typecheck`/`test`/`build`/смоки/инварианты) относятся к этапу код-ревью
+    (S7), а не спек-ревью (S4); на этом этапе кода еще нет.
+
+## Находки
+
+Все находки — **Medium, в скоупе задачи**: чинятся тем же автором в этом же
+документе ТЗ, без нового цикла обсуждения продукта и без отдельного issue.
+High-находок нет.
+
+### M1 — «что человек увидит» слито со сценарием и написано языком реализации; поверхность/touch не названы
+
+**Файл:** `docs/specs/265-import-reference-seam.md`, §1 (строки 19–51).
+
+PROCESS.md §7.1 требует два отдельных обязательных раздела: сценарий (какая
+персона, на какой поверхности, в какой момент) и «что человек увидит»
+**одной фразой, без терминов реализации** — и прямо предупреждает: «ТЗ, которое
+не может ответить на эти два вопроса, описывает работу, а не изменение
+продукта». Оба прямых родственника этой серии соблюдают разделение и
+дают термин поверхности явно: `244-orphan-space-references.md` §1–2 и
+`252-optimize-orphan-layout-report.md` §1–2 оба содержат фразу вида «Optimize —
+desktop-first административная поверхность; View и kiosk только читают уже
+сохранённый результат» и отдельное «До/После» на бытовом языке («видит список
+внутренних id и не понимает, безопасно ли нажимать Apply»).
+
+В §265 «До»/«После» — это фактически список внутренних инвариантов: «id
+строится от канонического корня lineage», «матрица plan-id ссылок»,
+«неизменяемый candidate», «Apply пишет ровно тот кандидат». Ни одна из этих
+пяти строк не отвечает на вопрос, что увидит администратор в диалоге импорта.
+Единственная строка, которая касается видимого экрана («Optimize остаётся
+единственным инспектором») — про архитектурное решение, не про экран импорта.
+Заодно нигде в документе не упомянут `docs/TOUCH-SUPPORT.md` и статус
+поверхности (desktop-only админ-функция) — обязательный пункт DoR §2.5
+(«влияние на touch… названо»).
+
+**Почему это не мелочь:** без явного «что человек увидит» ревьюер не может
+проверить AC10 на полноту (см. M2) — граница «это то же самое ТЗ, просто
+описано плохо» и «здесь спрятано непродуманное продуктовое решение» стирается
+именно тем, что раздел не выполняет свою функцию.
+
+**Как чинится:** вынести из §9 уже готовые пользовательские формулировки
+(«сохранены без изменений…», «Убрано забытых записей: N») в отдельное «После»
+одним-двумя предложениями без «lineage»/«canonical root»/«candidate»; добавить
+фразу про desktop-only поверхность и отсутствие touch-влияния, как в
+#244/#252.
+
+### M2 — AC10 хеджирует «если отчёт станет виден пользователю», хотя §9 безусловно описывает новый видимый текст
+
+**Файл:** `docs/specs/265-import-reference-seam.md`, §9 (264–291) против AC10
+(405–412).
+
+§9 не оставляет пространства для сомнения: «Import preview показывает:
+количество восстановленных target-ссылок; количество сохранённых неразрешимых
+ссылок с формулировкой «сохранены без изменений; после импорта запустите
+"Оптимизировать планы"»; количество отброшенных incoming links…». Это
+конкретный, безусловный список новых пользовательских строк.
+
+AC10 при этом формулирует i18n/changelog/screenshot обязательства условно:
+«Если новый report виден пользователю... Если итоговая реализация не меняет
+видимый UI, changelog фиксирует только "small fixes"». Это внутреннее
+противоречие: собственный §9 уже утверждает, что UI меняется, а AC10
+оставляет автору лазейку объявить это невидимым. DoR §2.5 требует, чтобы
+«i18n: ключи en + ru перечислены» было решено **до** перехода в
+«Готово к разработке», а не отложено условием, которое своим же текстом
+документа уже снято.
+
+**Как чинится:** снять условность из AC10 (текст отчёта уже зафиксирован в
+§9 — значит UI меняется точно) и либо перечислить конкретные i18n-ключи
+(даже черновые: `import.report.remapped`, `import.report.preserved_unresolved`,
+`import.report.dropped_incoming` и т.п.), либо явно сказать, что ключи
+называет реализация, а ТЗ фиксирует только текст — но без слова «если» там,
+где сам документ уже решил вопрос.
+
+### M3 — AC1–AC9 не называют способ доказательства (в отличие от требования DoR и от прямого соседа #244)
+
+**Файл:** `docs/specs/265-import-reference-seam.md`, §13 (352–412), ср.
+`docs/specs/244-orphan-space-references.md` §14 (355–369).
+
+PROCESS.md §2.5 требует: «AC1…ACn — пронумерованные проверяемые критерии
+приёмки; у каждого указано, чем он доказывается: `unit`/`backend`/`smoke`/
+`golden`/«ревью кода»». `#244` (тот же автор, та же неделя, тот же кластер
+задач) соблюдает это буквально — таблица АС там имеет отдельный столбец
+«доказательство» для каждой строки («Table-driven optimizer unit + idempotence
+unit.», «Backend import/export tests preview/revalidate/apply.» и т.д.).
+
+В §265 из десяти AC явный метод доказательства называет только AC10
+(«canonical-doc screenshots/golden review»). AC1–AC9 — чистый текст критерия
+без указания, каким тестом он проверяется; §14/§15 дают только общий,
+не привязанный к номеру AC список («Python import/export unit/backend tests;
+frontend unit tests…»). Из этого списка не восстанавливается однозначно, что,
+например, AC2 (cross-generation repair) проверяется backend unit, а AC7
+(идемпотентность) — какой конкретно раз запускаемым тестом.
+
+**Как чинится:** добавить к каждому AC1–AC9 короткую пометку способа
+доказательства по образцу §14 из #244 — минимальная правка формата, не
+меняющая ни один критерий по содержанию.
+
+### M4 — нет отдельного обязательного раздела «Риски»
+
+**Файл:** весь документ; ср. `docs/specs/244-orphan-space-references.md` §16
+(«Риски, производительность и security»).
+
+PROCESS.md §7.1 перечисляет «риски» как самостоятельный обязательный раздел
+ТЗ, отдельно от AC и от edge cases. В §265 нет раздела с таким содержанием:
+§12 «Edge cases» — про корректность алгоритма на конкретных входах, не про
+риски проекта (например: риск ложного совпадения lineage при совпадающей
+пользовательской строке — уже частично обсуждён в edge case 1, но не назван
+явно как принятый риск с последствием; риск расхождения Python/TS реализаций
+общего lineage-алгоритма, который §5.3 признаёт возможным и лечит только
+conformance fixture — сам факт «может расходиться, лечим тестом» это и есть
+риск, который нигде не сформулирован как риск).
+
+**Как чинится:** добавить раздел «Риски» (можно короткий, 3–5 пунктов):
+ложноположительное совпадение lineage у пользовательского id; расхождение
+Python/TS реализации до появления conformance fixture; неполнота матрицы §7,
+если найдётся ещё не учтённое plan-id поле; стоимость независимого прогона
+инвариантов на больших конфигурациях (уже частично закрыто AC9, достаточно
+сослаться).
+
+## Что проверено и корректно
+
+- Диагноз §2 (five claims про `_fresh`/`_repair_target_space_refs`/preview↔apply
+  desync/`model-invariants.mjs` gaps) — точен, построчно сверен с кодом, не
+  является догадкой.
+- Матрица внутренних ссылок §7 — все перечисленные поля существуют в коде;
+  не нашёл полей, которые код трогает, а матрица замалчивает бы, в пределах
+  выборки, которую успел свести (`marker.space`, `marker.room_id`,
+  `vacuum.segment_map`, `rooms[].open_to`, `markers[].controls[]`,
+  `value_badge.ref`, `openings[].host.id`).
+- Три «унаследованных решения владельца» (§3) действительно наследуются от
+  закрытых issue #244/#248/#252/#262, а не выданы автором за решение владельца
+  без основания — все три вопроса, которые исходный текст #265 адресовал
+  владельцу, закрыты предыдущими, уже принятыми задачами.
+- Скоуп/не-скоуп (§4) явный и не расширяет задачу дальше контракта шва импорта;
+  явно исключены full-import semantics, merge пространств, HA-registry remap,
+  удаление записей — совпадает с §SCOPE.md «never delete a user's file on an
+  inference» и с принципом lossless Optimize.
+- Алгоритм канонизации lineage (§5) внутренне согласован, ограничен по глубине,
+  не путает namespace при разборе (проверено на примере
+  `marker_room_x_deadbeef`), и явно называет себя эвристикой, а не
+  криптографическим доказательством происхождения — риск ложного совпадения
+  признан и заперт пятью условиями §5.2.
+- Терминология («Оптимизировать планы», «carrier») соответствует
+  `docs/USER-GUIDE.ru.md` и `docs/WALL-THICKNESS.md`, не изобретена.
+- Открытых продуктовых вопросов владельцу нет; вопросов, которые стоило бы
+  туда вынести, я не нашёл — то, что можно было решить самостоятельно
+  (алгоритм lineage, формат id, порядок remap), решено автором и явно помечено
+  как допущение в §17, подлежащее спору с ревьюером, а не эскалации.
+- AC5/AC8 корректно защищают инвариант «lossless по умолчанию» и явно
+  перечисляют regression-контракты пяти связанных задач, что закрывает риск
+  повторной регрессии одной из них при рефакторинге общего шва.
+- Изменение файла `docs/specs/README.md` — верно оформленная строка таблицы,
+  без добавления второго источника статуса (колонка «Статус ТЗ» не трогается).
+
+## Чего не проверял
+
+- Не запускал `typecheck`/`test`/`build`/смоки/инварианты — на этапе спек-ревью
+  кода еще нет, эти гейты относятся к S7 (код-ревью) и будут релевантны при
+  следующем заходе после реализации.
+- Не проверял `node scripts/check-docs.mjs` — diff не касается `src/**`
+  (только `docs/specs/**`), условие гейта не выполнено, прогон не даёт
+  дополнительной информации.
+- Не оценивал производительность реального lineage-индекса на больших
+  инсталляциях эмпирически — только прочитал заявление AC9 о линейности и не
+  нашёл в описанном алгоритме (§8) шагов, требующих декартова сравнения;
+  оценка «читал, не исполнял».
+- Не проверял конкретные формулировки будущих i18n-строк на корректность
+  русского/английского — они ещё не существуют, будет предметом код-ревью.
+
+## Вердикт
+
+Технический диагноз и дизайн решения обоснованы, привязаны к реальному коду и
+к уже принятым решениям владельца по связанным задачам; ни одна находка не
+касается корректности алгоритма или заявленного продуктового поведения.
+Все четыре находки — про полноту обязательных разделов ТЗ по PROCESS.md §7.1 и
+DoR §2.5 (пользовательское «до/после» без языка реализации, безусловность
+i18n-обязательств, способ доказательства на каждый AC, отдельный раздел
+рисков), и все устраняются правкой текста того же документа без пересмотра
+дизайна.
+
+**Вердикт: жёлтый · заход r1 · блокирующих циклов 1/4 · High: 0 · Medium: 4 → в задаче**