docs: review document for #398

Issue: #398
User-Visible: no

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

@@ -0,0 +1,183 @@
+# SPEC-REVIEW-398-r1
+
+- Issue: https://github.com/Matysh/houseplan-card/issues/398
+- Этап: ТЗ на ревью (PROCESS.md §2.4)
+- Материал: `docs/specs/398-sysmodules-guard-scope.md`, коммит `69dd09a7` (тело
+  issue #398 без изменений)
+- Заход: r1 · блокирующих циклов израсходовано до этого вердикта: 0 из 4
+- Комментариев в issue к моменту ревью: 0 (первый заход, полный разбор)
+
+## Скоуп проверки
+
+Issue #398 просит расширить гвард `test/backend-test-hygiene.test.mjs`,
+который сегодня ловит запись стаба интеграции в `sys.modules` только по
+строковому литералу и поэтому не видит `tests_backend/pure_imports.py:31`
+(`sys.modules[name] = module`, запись через переменную). Изменение полностью
+внутри класса B (`test/**`, `tests_backend/**`), не трогает `src/**` и
+`custom_components/**/*.py`, `User-Visible: no`. ТЗ оформлено полным треком —
+`docs/specs/398-sysmodules-guard-scope.md`, файл на месте, ссылка issue↔ТЗ
+двусторонняя.
+
+## Как проверялось
+
+1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` целиком.
+2. Прочитано тело issue #398 и лента меток (`gh api .../timeline`) — задача
+   ушла `S1-new → S3-spec → S4-spec-review` без публичного комментария
+   аналитики (см. «Наблюдения», п.2).
+3. Прочитан ТЗ `docs/specs/398-sysmodules-guard-scope.md` целиком.
+4. Сверены с реальным кодом все цитаты из ТЗ: `test/backend-test-hygiene.test.mjs`
+   (регэксп на строке 39), `tests_backend/pure_imports.py` (строка 31),
+   `tests_backend/conftest.py` (условная подмена под `if not HAS_HA:`),
+   `tests_backend/test_junction_limits.py` (безусловный вызов `load_pure` с
+   каноническим именем `custom_components.houseplan.junction_limits`),
+   `tests_backend/test_validation.py` (собственный `_load_pure`, пишет в
+   `sys.modules` под префиксом `hp_pure.*`, вне описанного скоупа гварда).
+5. Канонические документы подсистем (`SUN.md`, `LIGHT.md`, `CANVAS.md`,
+   `WALL-THICKNESS.md`, `UX-MODES.md`, `CONFIG-COMPATIBILITY.md`,
+   `TOUCH-SUPPORT.md`) — не применимы, задача не меняет видимое поведение
+   продукта.
+6. `docs/USER-GUIDE.ru.md` — не применим, `User-Visible: no`, персона —
+   разработчик, а не пользователь продукта.
+7. Гейты кода не гонялись: на этапе ревью ТЗ продуктового/тестового кода ещё
+   нет, есть только новый markdown-файл — `typecheck`/`test`/`build` к нему
+   неприменимы.
+
+## Находки
+
+### Medium (в скоупе задачи) — AC3 разрешает «путь 2», AC4 делает его невозможным
+
+`docs/specs/398-sysmodules-guard-scope.md:73-84` называет два одинаково
+допустимых исхода для `load_pure`:
+
+1. снимать регистрацию за собой после `exec_module` (самоочистка);
+2. объявить `pure_imports.py` вторым легальным исключением рядом с
+   `conftest.py`, с тестом, фиксирующим список ровно из двух файлов.
+
+Текст явно оставляет выбор реализации: «Предпочтителен (1)… — но завтра
+станет прецедентом» (строка 83) описывает предпочтение, не запрет второго
+пути, и **AC3** (строки 117-119) прямо предусматривает путь 2: «единственное
+исключение (**либо два файла, если выбран путь 2** — тогда список
+зафиксирован тестом и его рост краснеет)».
+
+Но **AC4** (строки 120-122) требует: «После прогона всего `tests_backend/` в
+`sys.modules` нет ключей `custom_components*` **сверх тех, что положил
+conftest**». Это не совместимо с путём 2 по конструкции: если `pure_imports.py`
+объявлен вторым легальным исключением и не убирает за собой запись, то после
+прогона `tests_backend/` в `sys.modules` останутся ключи
+`custom_components.houseplan.junction_limits`,
+`custom_components.houseplan.wall_segment_model`,
+`custom_components.houseplan.coordinate_canonicalization` — ровно то, что
+issue проверил исполнением как текущее наблюдаемое поведение — и они не
+«положены conftest», а положены вторым разрешённым файлом. AC4 в буквальной
+формулировке тогда красный при полностью корректной, явно допустимой
+реализации.
+
+**Почему это находка ТЗ, а не мелочь для код-ревью.** DoR требует
+«пронумерованные проверяемые критерии приёмки» — набор AC должен быть
+одновременно выполним. Сейчас AC3 и AC4 совместно выполнимы только при выборе
+пути (1); реализатор, прочитавший ТЗ и выбравший путь (2) как более простой
+(сам документ признаёт его допустимым и даже кладёт под него отдельный
+тест-пункт «план автотестов», п.4), гарантированно упрётся в красный AC4 при
+исполняемой проверке — и потратит цикл код-ревью на то, что было видно уже на
+этапе ТЗ.
+
+**Как править (для автора, не предписание):** либо явно исключить путь 2 из
+допустимых исходов (тогда «выбор — за реализацией» в контракте лишний — есть
+только путь 1), либо переформулировать AC4 так, чтобы он допускал оба исхода:
+«…нет ключей `custom_components*` сверх положенных `conftest.py` **и**, если
+выбран путь 2, кроме предъявленных явным списком исключений в
+`pure_imports.py`».
+
+**Воспроизведение** (не гипотеза — уже исполнено issue-автором и
+переподтверждено при разборе): `cd tests_backend && python3 -c "import
+conftest, test_junction_limits, sys; print([k for k in sys.modules if
+k.startswith('custom_components')])"` печатает три ключа
+`junction_limits`/`wall_segment_model`/`coordinate_canonicalization` сверх
+пустышек `custom_components`/`custom_components.houseplan`, поставленных
+`conftest.py`. При выборе пути 2 эти три ключа остаются легальными по AC3, но
+проваливают AC4 буквально.
+
+## Наблюдения (не блокируют, не находки к ТЗ)
+
+1. **Контракт vs риски о способе разбора.** Раздел «Проблема и контракт»
+   говорит «Форма проверки меняется с регулярного выражения на разбор
+   синтаксиса Python» (что совпадает с просьбой issue: «разбор AST вместо
+   регулярки»), а раздел «Риски» тут же ограничивает: «Полноценный парсер
+   тянуть не нужно и нельзя: достаточно узкого разбора строк… с fail-closed на
+   всё непонятое». Формулировки не запрещают друг друга буквально (узкий
+   разбор конкретно конструкции присваивания — тоже «разбор синтаксиса», не
+   обязательно generic-грамматика), но провисает связка: если ключевая
+   гарантия ТЗ — «отказ на всё недоказуемое» — реализована самим fail-closed
+   по умолчанию, то выбор между вызовом Python `ast` через subprocess (0 новых
+   зависимостей, репозиторий и так тянет Python 3.13) и узким текстовым
+   разбором в Node не влияет на корректность настолько, насколько намекает
+   контраст формулировок. Технический вопрос — не продуктовый, решается
+   автором/ревьюером по существу, не эскалируется владельцу; не блокирую,
+   но рекомендую снять формулировочное противоречие в следующей редакции.
+2. **Маршрутизация issue мимо инфраструктурного пути.** Диапазон правки —
+   исключительно класс B (`test/**`, `tests_backend/**`), ни одного файла
+   класса A. По `AGENTS.md` («Infrastructure-only work runs outside this
+   flow… The test… is mechanical: not a single class A file») и
+   `PROCESS.md` §1 такая задача идёт «без ТЗ, ревью ТЗ, код-ревью и без
+   прохода по статусам». Лента меток issue (`gh api .../timeline`) показывает,
+   что все переходы `S1-new → S3-spec → S4-spec-review` проставил лично
+   владелец (`Matysh`) без публичного комментария аналитики S2 — то есть это
+   не сбой автоматики, а его собственное решение вести задачу полным треком.
+   Это не дефект ТЗ и не встаёт в оценку документа: владелец вправе выбрать
+   больше строгости для третьего по счёту случая одного класса дефектов.
+   Фиксирую как наблюдение для гигиены процесса, не как находку ревью.
+
+## Что проверено и корректно
+
+- Оба обязательных продуктовых раздела на месте: «Сценарий» называет персону
+  верно (разработчик, добавляющий backend-тест — не пользователь продукта по
+  `docs/SCOPE.md`, что для чисто инфраструктурной задачи ожидаемо) и
+  поверхность (backend test harness, `tests_backend/`); «Что человек увидит до
+  и после» отвечает без терминов реализации.
+- Все обязательные разделы §7.1 присутствуют: проблема, скоуп/не-скоуп,
+  контракт, UX (Н/П), модель данных и миграция (Н/П), i18n (Н/П), AC1…AC6 с
+  указанием доказательства, план автотестов, риски, откат, release-артефакты.
+- Цитаты кода в ТЗ (регэксп `test/backend-test-hygiene.test.mjs:39`, запись
+  `pure_imports.py:31`) дословно совпадают с текущим содержимым файлов — не
+  придуманы задним числом.
+- AC1, AC2, AC5, AC6 однозначны и проверяемы: у каждого назван способ
+  доказательства (тест-контракт по форме записи, backend-прогон, мутант).
+  AC2 корректно перечисляет конкретные проверяемые формы (f-строка,
+  конкатенация, `setdefault`/`update`) вместо общего «и другие похожие».
+- Скоуп/не-скоуп разграничены точно: `tests_backend/conftest.py` (контракт
+  #394) и правило про `sys.path` (#393) явно исключены из скоупа этой задачи —
+  граница, названная в issue #394, не ослабляется этим ТЗ.
+- `docs/SCOPE.md` не нарушается: задача не претендует ни на одну строку Core
+  user jobs, это ожидаемо для чисто инфраструктурного изменения, и ТЗ не
+  делает вид, что решает продуктовую задачу.
+- Риски названы по существу (разбор Python из Node, снятие регистрации ломает
+  относительные импорты, скрытая зависимость от повторного импорта) со
+  смягчениями, а не общими словами.
+- Откат дешёвый и конкретный: «два коммита назад, продуктовый код не
+  затронут» — соответствует факту (диапазон правки — исключительно class B).
+- Ни одна догадка о поведении не выдана за факт: утверждения о текущем
+  дефекте («переживает свой тест», перечень трёх модулей) сопровождаются
+  пометкой «проверено исполнением» и совпадают с тем, что показал сам issue.
+
+## Чего не проверял
+
+- Не гонял `npx tsc --noEmit` / `npm test` / `npm run build` — на этапе
+  ревью ТЗ нет ни строки продуктового или тестового кода, есть только новый
+  `docs/specs/*.md`; эти гейты неприменимы к чтению документа.
+- Не проверял технической реализуемости «разбора синтаксиса Python из Node»
+  экспериментом (не писал прототип) — вопрос вынесен в наблюдение 1 как
+  формулировочный, а не как проверенный тупик.
+- Не проверял `tests_backend/test_validation.py` построчно за пределами
+  найденного вызова `_load_pure`/`sys.modules[f"hp_pure...`] — этого было
+  достаточно, чтобы подтвердить корректность утверждения ТЗ «при
+  необходимости» про этот файл (префикс `hp_pure` вне заявленного скоупа
+  гварда `custom_components*`); полный разбор файла оставляю код-ревью, если
+  реализация всё же тронет этот файл.
+
+## Вердикт
+
+Жёлтый. Единственная блокирующая находка (Medium, в скоупе задачи) —
+несовместимость AC3 и AC4 для явно допущенного документом «пути 2»: описанный
+как разрешённый исход не может пройти собственный исполняемый критерий
+приёмки. Правится в тексте ТЗ этим же автором, без выхода за рамки текущей
+задачи.