docs: review document for #398

Issue: #398
User-Visible: no

docs/reviews/SPEC-REVIEW-398-r2.md

@@ -0,0 +1,204 @@
+# SPEC-REVIEW-398-r2
+
+- Issue: https://github.com/Matysh/houseplan-card/issues/398
+- Этап: ТЗ на ревью (PROCESS.md §2.4)
+- Материал: `docs/specs/398-sysmodules-guard-scope.md`, коммит `480d202f`
+  («#398 spec revision 2 per SPEC-REVIEW-398-r1»)
+- Заход: r2 · блокирующих циклов израсходовано до этого вердикта: 1 из 4
+- Предыдущий вердикт: `docs/reviews/SPEC-REVIEW-398-r1.md`, жёлтый, на коммите
+  `69dd09a7` (ревизия 1 ТЗ). SHA в вердикте r1 не назван явно текстом — найден
+  сопоставлением: `git show a4b686a7 --stat` (коммит с самим документом r1)
+  датирован позже `69dd09a7` и раньше `480d202f`, а «Материал» в r1 называет
+  файл без ревизии — правки после которой ещё не было, то есть r1 читал ровно
+  содержимое `69dd09a7`.
+
+## Разбор по дельте (PROCESS.md §2.9, #214)
+
+Дельта — `git diff 69dd09a7..480d202f -- docs/specs/398-sysmodules-guard-scope.md`
+(28 добавленных / 22 удалённых строк). Правка не переезжает на другую подсистему,
+не меняет продуктовый контракт (задача остаётся `User-Visible: no`, класс B) и по
+объёму меньше исходной задачи — полный повторный разбор не требуется. Проверены
+только: (а) закрытие находки r1 по цитате нового текста и (б) те AC, чьё
+доказательство дельта задевает — это AC3, AC4 и, как показано ниже, обнаружилось,
+что дельта задевает ещё и совместимость AC1/AC2 с AC3 через код `pure_imports.py`,
+явно оставленный «в скоупе» правки. Разделы, которых дельта не касалась (Сценарий,
+UX, i18n, AC5, AC6, откат, release-артефакты), унаследованы из r1 — см. раздел
+ниже.
+
+## Закрытие раунда r1
+
+| Находка r1 | Чем закрыта | Где это видно |
+|---|---|---|
+| Medium: AC3 явно допускал «путь 2» (объявить `pure_imports.py` вторым легальным исключением), а AC4 требовал отсутствия лишних ключей после прогона — путь 2 конструктивно не мог пройти AC4 | «Путь 2» полностью удалён из текста; раздел «Что делать с `load_pure`» переписан как «один путь: снимать за собой», с явной ссылкой на находку ревьюера | `docs/specs/398-sysmodules-guard-scope.md:71-87` («Ревизия 1 допускала второй исход… вариант снят»); AC3 (строки 120-121) больше не содержит альтернативы «либо два файла»; план автотестов, п.4 (строка 139) убрал условность «если выбран путь 2» |
+
+Текстовое закрытие корректно и однозначно: альтернативный путь убран, а не
+спрятан («не рекомендуется», «как правило» и т.п. отсутствуют — использована
+формулировка «вариант снят»).
+
+## Новая находка r2
+
+### Medium (в скоупе задачи) — устранение «пути 2» открывает новый конфликт: AC1/AC2 не совместимы с AC3 для `tests_backend/pure_imports.py`, оставленного «в скоупе»
+
+Ревизия 2 фиксирует ровно одно исключение из-под гварда —
+`tests_backend/conftest.py` (строка 87: «Единственное исключение остаётся одно»;
+AC3, строки 120-121: «остаётся единственным исключением»; план автотестов,
+п.4, строка 139: «попытка добавить **второй** файл в список исключений →
+красный» — то есть механизм исключения зафиксирован именно как список файлов
+мощности 1, а не как распознавание безопасного паттерна «запись + восстановление»).
+
+Одновременно раздел «Скоуп/не-скоуп» (строка 91) оставляет
+`tests_backend/pure_imports.py` в скоупе как файл, который правка изменяет (а не
+удаляет), и требует (через «Что делать с `load_pure`», строки 78-87, и AC5,
+строки 125-127), чтобы после правки `load_pure` продолжал резолвить
+относительные импорты для существующих вызовов — в частности,
+`tests_backend/test_junction_limits.py:28-30`, который передаёт канонический
+литерал `"custom_components.houseplan.junction_limits"` в качестве аргумента:
+
+```python
+sys.modules[name] = module  # tests_backend/pure_imports.py:31
+```
+
+Это ровно образец, который AC1 называет напрямую: «Файл, записывающий
+`sys.modules[<переменная>] = …`, где переменная может содержать имя пакета
+интеграции, отклоняется гвардом» (строки 112-114). Значение `name` внутри
+`pure_imports.py` — параметр функции, из текста самого файла статически не
+определимо, что оно не начинается с `custom_components`; собственный
+fail-closed принцип ТЗ («значение ключа может быть неизвестно статически —
+тогда гвард обязан отказывать», строки 67-69) требует отклонить именно этот
+файл. Проверка в `test/backend-test-hygiene.test.mjs` статическая, идёт
+файл-за-файлом без исполнения (комментарий в коде, строки 11-14: «Проверка
+статическая — по исходникам, без запуска») — она не может увидеть, что
+`load_pure` восстанавливает `sys.modules` после `exec_module`; восстановление
+— рантайм-факт, а гвард текст не исполняет.
+
+Итог: `pure_imports.py`, оставленный в скоупе и обязанный по AC5 продолжать
+работать для `test_junction_limits.py`, **структурно не может** одновременно (а)
+не быть исключением (по AC3, ровно один файл-исключение — `conftest.py`) и (б)
+избежать отклонения по AC1 (поскольку пишет в `sys.modules` по переменной,
+которая в вызовах из скоупа действительно принимает значения вида
+`custom_components.houseplan.*`). Здесь нет обходного варианта на уровне формы
+записи: регистрация модуля в `sys.modules` под точным каноническим именем —
+обязательное условие резолва относительных импортов в Python
+(`importlib.util.module_from_spec` + `exec_module`), других способов сделать
+`from .wall_segment_model import …` рабочим для этого файла ТЗ не описывает и
+предложить не может без изменения самого механизма загрузки.
+
+**Почему это находка ТЗ, а не код-ревью.** Это тот же класс дефекта, что и
+закрытая находка r1: набор AC совместно невыполним для сценария, который сам
+документ называет обязательным (пункт «в скоупе» + AC5). Разработчик,
+реализующий ТЗ буквально — упрощённая по AC1/AC2 регулярка/AST-проверка плюс
+очистка `sys.modules` в `load_pure` — получит перманентно красный
+`test/backend-test-hygiene.test.mjs` на собственном `pure_imports.py`, то есть
+красный `npm test` сразу после реализации, и упрётся в противоречие, которое
+было видно уже на этапе чтения ТЗ.
+
+**Как править (для автора, не предписание).** Разрешить `pure_imports.py`
+вторым явным исключением в AC3 (список из двух конкретных, поимённо
+зафиксированных файлов — `conftest.py` и `pure_imports.py`, рост списка сверх
+этого по-прежнему красный) — это НЕ реинкарнация снятого «пути 2»: там
+исключение освобождало от очистки и потому ломало AC4; здесь очистка (уже
+специфицированная в ревизии 2 как «снимать всё, что появилось под префиксом
+`custom_components` за время `exec_module`») остаётся обязательной сама по
+себе и закрывает AC4 независимо от статуса файла в списке исключений
+статического гварда. Иными словами: исключение из AC1-проверки (статический
+факт «этому файлу разрешено писать») и обязанность очистки по AC4
+(рантайм-факт «после прогона лишнего не осталось») — два независимых
+требования, и revision 2 верно оставила второе, но по инерции убрала и первое,
+хотя первое было не источником проблемы. Альтернатива — доказать в тексте, что
+`name` в `pure_imports.py` статически ограничен паттерном, не совпадающим с
+`custom_components*` (не просматривается, как это сделать без потери гибкости
+вызова с каноническим именем).
+
+**Воспроизведение** (разбор по коду, не исполнено как эксперимент — гварда,
+описанного ТЗ, ещё не существует в коде): применить контракт AC1 к
+`tests_backend/pure_imports.py:31` буквально даёт «отклонить», а вызов из
+`tests_backend/test_junction_limits.py:28` требует, чтобы файл продолжал
+работать. Оба требования одновременно предъявлены этой же ревизией ТЗ (AC1 +
+скоуп + AC5).
+
+## Унаследовано из r1
+
+Без повторной проверки в этом раунде — дельта не касалась этих участков текста
+и они не входят в цепочку доказательства новой находки:
+
+- Обязательные продуктовые разделы («Сценарий», «Что человек увидит до и
+  после») и корректность персоны — `docs/reviews/SPEC-REVIEW-398-r1.md`,
+  раздел «Что проверено и корректно», п.1, на SHA `69dd09a7` (текст этих
+  разделов дельтой не тронут).
+  Как проверить самостоятельно, что раздел не менялся: сравнить
+  `git diff 69dd09a7..480d202f -- docs/specs/398-sysmodules-guard-scope.md`
+  — «Сценарий» и «Что человек увидит» отсутствуют среди затронутых hunks.
+- Полнота обязательных разделов §7.1 (проблема, скоуп/не-скоуп, UX Н/П, модель
+  данных и миграция Н/П, i18n Н/П, план автотестов, откат, release-артефакты)
+  — там же, п.2. Структура не менялась, менялось только содержимое трёх блоков
+  (проблема/контракт, AC3, план автотестов п.4), уже разобранных выше.
+- Соответствие `docs/SCOPE.md`: задача не претендует на строку Core user jobs,
+  чисто инфраструктурная — там же, п.5. Не переоценивалось: дельта не меняет
+  ни продуктовую рамку, ни `User-Visible`.
+- AC2, AC5, AC6 — однозначны и проверяемы каждый по отдельности — там же, п.4
+  (частично; см. ниже уточнение по AC1). Текст этих критериев дельтой не
+  тронут (см. диф выше — правки только в AC3 и в тексте перед AC-блоком).
+  Уточнение к унаследованному: r1 также назвал AC1 «однозначным» — это верно
+  в изоляции (формулировка ясна), но r1 не проверял совместность AC1 с AC3 и
+  скоупом; это ровно то, что находка r2 закрывает как пробел прошлого раунда,
+  а не противоречие ему.
+- Риски «разбор Python из Node» и «скрытая зависимость от повторного импорта»
+  — назывались по существу уже в r1, текст не менялся (третий риск, «снятие
+  регистрации ломает относительные импорты», получил только уточнение
+  «замер: модуль полностью рабочий после очистки» — редакционное, не меняет
+  сути) — там же, п.6. Новый риск ревизии 2 («соседи, подтянутые
+  относительными импортами») проверен заново в этом раунде, см. «Что
+  проверено и корректно» ниже.
+- Откат и release-артефакты — не изменены дельтой, наследуются из п.7-8 того
+  же документа.
+- Наблюдение 1 (напряжение формулировок «контракт vs риски» про способ
+  разбора) и наблюдение 2 (маршрутизация issue мимо инфраструктурного пути) —
+  не блокировали в r1 и не затронуты дельтой; остаются наблюдениями, не
+  находками.
+
+## Что проверено и корректно (в этом раунде)
+
+- Закрытие находки r1 текстуально полное: «путь 2» не просто ослаблен
+  формулировкой, а вычеркнут вместе со всеми его следами в AC3 и плане
+  автотестов (сверено построчно по диффу выше).
+- Новый риск «Соседи, подтянутые относительными импортами» (строки 163-167)
+  назван конкретно, с перечислением реальных имён модулей
+  (`wall_segment_model`, `coordinate_canonicalization`) и с механизмом
+  смягчения (диф по префиксу вокруг `exec_module`, а не снятие одного имени) —
+  проверено сопоставлением с фактическим поведением, описанным в разделе
+  «Проблема и контракт» (те же три модуля названы там как реально
+  оставшиеся после `import test_junction_limits`).
+- `docs/SCOPE.md` по-прежнему не нарушается: правка не расширяет заявленную
+  поверхность, остаётся полностью инфраструктурной.
+
+## Чего не проверял
+
+- Не гонял `npx tsc --noEmit` / `npm test` / `npm run build` — на этапе ревью
+  ТЗ нет продуктового или тестового кода, есть только правка `docs/specs/*.md`;
+  эти гейты неприменимы. (То же основание, что и в r1.)
+- Не проверял эксперимента «разбор синтаксиса Python из Node» — вопрос остаётся
+  открытым наблюдением из r1, не заново поднятым в этом раунде.
+- Не перечитывал `tests_backend/test_validation.py` целиком повторно — его
+  собственный `_load_pure` использует префикс `hp_pure.*`, статически не
+  совпадающий с `custom_components*` (литеральная часть f-строки/конкатенации
+  проверяема по AC2), и это уже подтверждено в r1; для новой находки этот файл
+  не релевантен, она про `pure_imports.py`.
+- Не проверял, можно ли реализовать распознавание паттерна «запись сразу с
+  последующим восстановлением» как альтернативу пофайловому списку исключений
+  — это один из вариантов правки, adресованных автору, а не факт, который
+  нужно доказывать ревьюеру; сам документ фиксирует именно файловый список
+  (AC3 + план автотестов п.4), и находка оценивает именно этот, явно выбранный
+  автором механизм.
+
+## Вердикт
+
+Жёлтый. Единственная блокирующая находка (Medium, в скоупе задачи) — устраняя
+находку r1 удалением «пути 2», ревизия 2 разорвала совместность AC1/AC2 (гвард
+обязан отклонить запись `pure_imports.py:31` по правилу fail-closed) и AC3
+(единственное разрешённое исключение — только `conftest.py`) при том, что
+`pure_imports.py` явно оставлен в скоупе и обязан по AC5 продолжать работать
+для `test_junction_limits.py`. Правится в тексте ТЗ тем же автором, без выхода
+за рамки текущей задачи — вероятный минимальный фикс: вернуть
+`pure_imports.py` в список статических исключений (не путать со снятым «путём
+2» — обязанность очистки `sys.modules` по AC4 остаётся в силе независимо от
+статуса файла в списке исключений гварда).