docs: review document for #186

Issue: #186
User-Visible: no

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

@@ -0,0 +1,216 @@
+# SPEC-REVIEW-186-r2
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/186
+- **ТЗ под ревью:** [`docs/specs/186-partition-opening-jamb-margin.md`](https://github.com/Matysh/houseplan-card/blob/issue/186-partition-jamb-margin/docs/specs/186-partition-opening-jamb-margin.md)
+  (коммит `62f73faef737df44ad8bb838aac4beceb74db9a1`), обычный трек — не `small`/`trivial`
+- **Роль:** ревьюер ТЗ (не автор), этап `S4-spec-review`
+- **Трек:** обычный, лимит циклов ревью ТЗ — 4 (§4 PROCESS.md)
+- **Цикл:** r2/4
+- **Предыдущий цикл:** [`SPEC-REVIEW-186-r1.md`](SPEC-REVIEW-186-r1.md) — красный, High-1 (§8/AC4 «full import — всегда strict» противоречил принятому decision #3), Low-1 (нет заголовка «Проблема»)
+
+## Скоуп ревью
+
+Только правка после r1: коммит `62f73fa` («docs: preserve legacy jambs on full
+restore») меняет §3 (пункт 6), §5, §8, UX-таблицу (§9), AC4, AC6 и таблицу
+рисков (§13), плюс добавляет заголовок «Проблема» (§2). Формула margin (§4),
+scope/не-scope (§5/§6 кроме одной строки), frontend-контракт (§7), AC1/AC2/AC3/
+AC5, план проверок (§12), откат (§14), release-артефакты (§15) и принятые
+предположения (§16) не менялись — их точность уже подтверждена в r1 и
+переподтверждается здесь только там, где новая правка их касается.
+
+Продуктовый код по-прежнему не существует: ветка `issue/186-partition-jamb-margin`
+содержит три коммита (`7d4d3f0`, `85f54b4`, `62f73fa`), все класса C
+(документация), все с трейлерами `Issue: #186` / `User-Visible: no`. Гейты
+`typecheck`/`test`/`build`/smoke/golden/pytest не прогонялись — на этапе
+ревью ТЗ они не относятся к предмету (PROCESS.md §2.4/§8).
+
+## Как проверялось
+
+1. Перечитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` (действующая редакция) —
+   без изменений с r1, инварианты те же.
+2. Прочитаны все восемь комментариев issue #186 подряд: аналитика (Q1–Q3 с
+   defaults), решение владельца по Q1–Q3, хендофф «ТЗ готово к ревью», вердикт
+   r1 (красный, High-1), дополнительный вопрос владельцу Q4 (после ревью),
+   решение владельца по Q4, хендофф «Правки ТЗ после ревью r1», и служебный
+   комментарий о несработавшем автоматическом ревью r1 (метка не переставилась —
+   к предмету этого ревью не относится, поднято отдельно перестановкой метки
+   вручную/владельцем).
+3. Построчно сверен `git show 62f73fa -- docs/specs/186-partition-opening-jamb-margin.md`
+   с текстом решения владельца по Q4 и с хендоффом «Правки ТЗ после ревью r1»:
+   правка ограничена ровно перечисленными разделами, посторонних изменений
+   (формулы, AC1-3/5, scope-исключений, отката) нет — «попутных правок» не
+   обнаружено.
+4. Формулировка §3 п.6 и §8 сверена дословно с текстом решения владельца по Q4
+   («полный backup/restore всегда сохраняет legacy near-end проёмы... при
+   восстановлении поверх текущего плана, на пустой и на другой инсталляции.
+   Full import проверяет структурную целостность и попадание проёма внутрь
+   host, но не применяет новый jamb safety margin») — совпадение по существу
+   полное, без добавленных условий и без потерянных случаев (текущий /
+   пустой / чужой инстанс перечислены во всех трёх местах: §3, §8, AC4).
+5. Независимо перепроверена архитектурная посылка исправления (что full
+   import физически не обязан быть strict) чтением текущего кода на этой же
+   ветке:
+   - `custom_components/houseplan/import_export.py:1304-1345` (`prepare_apply`)
+     — для `kind == "full"` вызывается только `CONFIG_SCHEMA(config)` в конце
+     функции (строка 1345); `validate_partition_opening_hosts` в этой ветке не
+     вызывается вовсе.
+   - `grep -n "validate_partition_opening_hosts" custom_components/houseplan/*.py`
+     — ровно три вызова: `import_export.py:995` внутри `build_space_merge()`
+     (путь `kind == "space"`, т.е. частичный merge-импорт, не полный),
+     `websocket_api.py:1260` (`config/set`) и `websocket_api.py:1372`
+     (`optimize`). Ни один не относится к `kind == "full"`.
+   - Вывод: `validate_partition_opening_hosts()` (и, следовательно, будущий
+     jamb-контракт «рядом с ним либо в его расширении», §8) **уже сегодня**
+     структурно не покрывает full import — это не новое послабление ради
+     #186, а естественное продолжение существующей архитектуры. Формулировка
+     §8 не вводит специальный обход, а корректно описывает то, что и так
+     происходит.
+   - Отдельно проверено (не для находки, а чтобы не спутать с общим
+     паттерном): `docs/CONFIG-COMPATIBILITY.md:59` документирует другой
+     прецедент (#157, `invalid_passage_fields`), где `validate_opening_passages`
+     **действительно** вызывается для каждого импорта, включая `full`
+     (`import_export.py:1159`, `create_preview()`, `validate_all=True`, до
+     ветвления по `kind`). Это не противоречие: у #157 и #132/#186 разные
+     валидаторы с разной историей вызова на full-import пути, и decision
+     Q4 явно называет цену этого выбора («вручную изменённый полный backup
+     тоже может содержать near-end geometry») — то есть асимметрия с #157
+     осознанная, а не незамеченная.
+6. Сверено, что merge-импорт (`kind == "space"`, `build_space_merge` вызывает
+   `validate_partition_opening_hosts(merged_config, current_config)` на
+   `import_export.py:995`) остаётся вне пересмотренного decision Q4 (тот
+   называет только «полный backup/restore»). ТЗ (§5/§8/AC4/AC6) не утверждает
+   ничего противоположного про merge-путь и не молчит о нём как о решённом
+   факте — merge и раньше (#132) переприсваивает id и потому уже трактует
+   переносимые записи как новые для семантического валидатора; #186 просто
+   наследует этот действующий паттерн, не решая для него ничего нового.
+   Дополнительных вопросов владельцу это не требует.
+7. Перечитан заново весь текст ТЗ (не только диф) на предмет новых утверждений
+   о поведении, поданных как факт без пометки assumption или ссылки на
+   решение владельца — целенаправленно искал второй High-1. Не найдено:
+   каждое утверждение §3/§5/§8/§9/AC4/AC6 после правки либо дословно повторяет
+   decision Q1–Q4, либо является технической деталью уже покрытой §16
+   (имена helper/reason, previous-параметр).
+8. Перепроверены обязательные разделы §7.1 (таблица ниже) и добавленный
+   заголовок «Проблема» — сравнение старой и новой версии (`git show 62f73fa`)
+   подтверждает: над «До/после» появился отдельный вводный абзац с
+   заголовком `## 2. Проблема`, ровно то, что требовал Low-1.
+9. Проверена запись `docs/specs/README.md:102` — ссылка issue ↔ ТЗ
+   двусторонняя и не менялась в этом цикле, ещё присутствует.
+10. Проверены трейлеры всех трёх коммитов ветки (`git log --format='%H%n%s%n%b'`)
+    — `Issue: #186` и `User-Visible: no` на каждом; `User-Visible: no`
+    корректен, т.к. ни один коммит не меняет продукт.
+
+Гейты (`typecheck`/`test`/`build`, browser smoke, golden, backend pytest) не
+прогонялись — продуктового кода нет, это ожидаемо на этапе ревью ТЗ.
+
+## Обязательные разделы (§7.1 PROCESS.md)
+
+| Раздел | Есть | Комментарий |
+|---|---|---|
+| Сценарий (персона/поверхность/момент) | ✅ | §1, без изменений в r2 |
+| Проблема | ✅ | §2, теперь отдельный заголовок с вводным абзацем — Low-1 закрыт |
+| Что человек увидит до/после | ✅ (см. Low-2) | «До/После» под §2; формулировка не изменилась в r2 и содержит термины реализации, см. находку ниже |
+| Скоуп / не-скоуп | ✅ | §5/§6; один пункт §5 переформулирован под decision Q4, без потери содержания |
+| Контракт поведения | ✅ | §7/§8; §8 переписан под decision Q4, внутренне непротиворечив |
+| UX | ✅ | §9, добавлена строка про full restore |
+| Модель данных и миграция | ✅ | §10, без изменений |
+| i18n | ✅ | §10, без изменений |
+| AC1…ACn с доказательством | ✅ | AC4/AC6 переписаны под decision Q4, доказательство расширено на current/empty/foreign install |
+| План автотестов | ✅ | §12, без изменений |
+| Риски | ✅ | §13, добавлена строка «Full restore ошибочно становится strict» |
+| Откат | ✅ | §14, без изменений |
+| Release-артефакты | ✅ | §15, без изменений |
+
+Блок «Принятые технические предположения» (§16) не менялся в r2; при повторном
+чтении ни один из четырёх пунктов по-прежнему не маскирует продуктовый вопрос.
+
+## Находки
+
+### Low-2 — «что человек увидит» использует термины реализации
+
+**Файл:** `docs/specs/186-partition-opening-jamb-margin.md`, §2, подраздел
+«До/После» (не менялся в r2, существовал и в r1).
+
+PROCESS.md §7.1 требует для этого элемента «одной фразой, без терминов
+реализации». Текущий текст: «До: **frontend** считает валидным любой проём...
+**Backend** повторяет эту границу с одним лишь **floating-point epsilon**...»,
+«После: у обоих торцов host резервируется остаток... математическими
+**endpoints** host». Используются архитектурные/кодовые термины (frontend,
+backend, epsilon, endpoints), а не то, что видит администратор в редакторе
+плана (например: «проём можно было поставить впритык к торцу стены, без места
+под наличник; теперь редактор всегда оставляет отступ, равный половине
+толщины стены»).
+
+Ни один AC не опирается на эту формулировку и не становится двусмысленным —
+дефект чисто редакционный, той же природы, что и снятый в r1 Low-1.
+
+**Решение ревьюера:** Low, не блокирует. Снимаю без возврата ТЗ на правку;
+рекомендую при следующей правке файла (не отдельным циклом) заменить
+«До/После» на формулировку в терминах того, что видит администратор в Plan
+editor, не изобретая новых решений — переформулировка, не новый контракт.
+
+## Что проверено и корректно
+
+- **High-1 из r1 закрыт корректно и полностью.** Коммит `62f73fa` меняет ровно
+  те разделы, что были нужны (§3 п.6, §5, §8, §9, AC4, AC6, §13), дословно
+  соответствует принятому владельцем decision Q4, и не оставляет старой
+  формулировки «full import без trusted previous — strict» ни в одном месте
+  документа (проверено построчным перечитыванием всего файла, не только
+  дифа).
+- **Архитектурная посылка исправления подтверждена независимым чтением кода**,
+  а не принята на слово: `validate_partition_opening_hosts()` сегодня
+  вызывается только в `build_space_merge` (частичный merge), `config/set` и
+  `optimize` — ни разу для `kind == "full"` в `prepare_apply()`. Значит
+  decision Q4 не вводит специальный обход общей архитектуры, а продолжает уже
+  существующее устройство full-import пути. Асимметрия с прецедентом #157
+  (`invalid_passage_fields`, который действительно строг на full import) —
+  реальная и явно принятая владельцем цена, а не незамеченное расхождение.
+- **Merge-импорт (частичный, `kind == "space"`) остаётся вне decision Q4**,
+  и ТЗ не утверждает о нём ничего, что противоречило бы этому — унаследованный
+  из #132 паттерн переприсвоения id продолжает работать без нового решения.
+- **Второй High не найден.** Целенаправленный повторный проход по всему
+  документу (не только по дифу r1→r2) не выявил новых утверждений о
+  поведении, поданных как факт без пометки assumption или ссылки на решение
+  владельца.
+- **Формула margin (§4), AC1/AC2/AC3/AC5, откат (§14), touch-декларация (§10)
+  и release-артефакты (§15)** не менялись в этом цикле; их точность была
+  независимо перепроверена в r1 (арифметика `GRID_N=240`, реальные call sites,
+  существующий паттерн `docs/CONFIG-COMPATIBILITY.md`) и остаётся в силе.
+- **Трейлеры и трассируемость**: все три коммита ветки несут
+  `Issue: #186`/`User-Visible: no`, запись issue ↔ ТЗ в `docs/specs/README.md`
+  на месте, `docs/specs/186-partition-opening-jamb-margin.md` ссылается на
+  issue и на #132.
+- **Трек подтверждён обычным** (не `small`/`trivial`) — критерии §5 PROCESS.md
+  по-прежнему не выполняются (новый видимый граничный контракт,
+  compatibility-решение), лимит цикла — 4, использован r2/4.
+
+## Чего не проверял
+
+- Реализацию — её по-прежнему нет: три коммита ветки все класса C
+  (документация), продуктовый код (`src/**`, `custom_components/houseplan/**/*.py`)
+  не менялся этой веткой — проверено чтением `git show --stat` каждого
+  коммита.
+- Гейты `typecheck`/`test`/`build`/browser smoke/golden/`pytest tests_backend` —
+  не относятся к этапу ревью ТЗ.
+- Полный повторный пересчёт формулы margin и всех восьми call sites
+  `resolvePartitionOpening()` — не требовался: эта часть документа не менялась
+  между r1 и r2, независимая проверка из r1 остаётся в силе и не переделывалась.
+- Оставшиеся детали merge-импорта (`build_space_merge`, duplicate_policy,
+  `same_source`) за пределами вопроса «вызывается ли здесь
+  `validate_partition_opening_hosts`» — не относится к предмету находки High-1
+  и её закрытия.
+- Численные оценки аналитики (5/10 · 6/10 · 5/10 · P2) — поле владельца,
+  решено до написания ТЗ, вне предмета ревью.
+
+## Вердикт
+
+Зелёный. High: 0 (High-1 из r1 закрыт decision Q4 и корректно отражён во всех
+затронутых разделах ТЗ; архитектурная посылка независимо перепроверена
+чтением текущего кода). Medium: 0. Low: 1 новая (Low-2 — «До/После» использует
+термины реализации вместо описания того, что видит администратор; снимается
+ревьюером без возврата, рекомендация — поправить при следующей правке файла).
+Low-1 из r1 (отсутствие заголовка «Проблема») закрыт правкой. ТЗ готово к
+переходу в `S5-ready`.
+
+**Вердикт: зелёный · цикл r2/4 · High: 0 · Medium: 0 → в задаче · Документ:
+docs/reviews/SPEC-REVIEW-186-r2.md**