docs: review document for #316

Issue: #316
User-Visible: no

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

@@ -0,0 +1,238 @@
+# SPEC-REVIEW-316-r1
+
+Issue: #316 · ТЗ: `docs/specs/316-opening-host-auto-resolution.md` ·
+ветка `issue/316-migration-auto-resolve` · SHA материала ревью: `36f735ae`
+(`docs: spec #316 — migration auto-resolves opening-host conflicts`).
+Трек: обычный (метка `small` отсутствует) → требуется файл ТЗ и отдельный
+документ ревью, что и сделано.
+
+## Скоуп
+
+Владелец решил в чате (комментарий S2→S4, 2026-08-26): конфликт
+«проём ↔ нулевая стена» миграция v9 разрешает автоматически, рисование
+нигде не блокируется. Настоящее ТЗ формализует правила авто-разрешения
+(§3.1–3.4) и сужает контракт #306 §8.2 п.4. Отдельная задача #319
+(бэкенд-гард против stale-клиента) прямо исключена из скоупа — правильно,
+это независимый инвариант.
+
+## Как проверялось
+
+Ревью документа без исполнения кода (стадия ТЗ, код ещё не написан —
+commit `36f735ae` содержит только спецификацию). Проверено:
+
+1. Полное чтение `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` (см. системный
+   промпт) — обязательные разделы §7.1 и чек-лист DoR §2.5 сверены построчно.
+2. Тело issue #316 и оба комментария (аналитика с воспроизведением,
+   решение владельца S2→S4) — сверено, что ТЗ не расходится с зафиксированной
+   причиной и решением.
+3. Чтение текущего (до фикса) `src/wall-segment-model.ts`: `buildAtoms`,
+   `resolveRoomOpeningHost`, `hostRoomOpenings`, `migrateSpace`,
+   `commitWallSegmentModel` — сверка каждого нормативного правила §3 ТЗ с
+   реальным кодом, который оно должно изменить.
+4. Проверка ссылок ТЗ на прежние документы: `docs/specs/306-zero-thickness-walls.md`
+   §8.2 п.4 и п.8 (ровно то, что ТЗ #316 объявляет заменяемым/сужаемым —
+   подтверждено построчно), `docs/WALL-THICKNESS.md` §9 (независимая кладка,
+   `opening cannot punch a coincident independent wall`), issue #313 (полный
+   текст).
+5. Проверка мест использования `resolveRoomOpeningHost` в `houseplan-card.ts`
+   (drag/placement путь, строки ~12680–12684 и ~12828–12834) — подтверждает
+   claim §3.3 «picker/placement уже умеет переставлять безхостовый проём».
+6. Проверка `scripts/config-field-registry.mjs` (запись
+   `spaces[].openings[].host=wall`) и backend `validation.py`
+   (`vol.Optional("host")`) — сверка, меняется ли контракт совместимости,
+   который ТЗ должно менять/документировать.
+7. Поиск i18n-ключей `toast.zero_wall_migration_blocked` /
+   `wall_model.reason.opening-host` в `src/i18n/{ru,en}.json` — проверка,
+   остаются ли они актуальны для пост-v9 fail-closed случая (AC5).
+8. `ls demo/smoke_*.mjs` — проверка существующего покрытия по теме (нашёлся
+   `smoke_inert_openings.mjs`, но по не относящемуся термину — см. находку L1).
+
+Гейты `typecheck`/`test`/`build` не прогонялись: на этапе ТЗ продуктового
+кода ещё нет, диапазон изменений — исключительно `docs/specs/316-*.md`
+(класс C). Это не пропуск гейта — гейтов для класса C на этой стадии не
+предусмотрено.
+
+## Находки
+
+### M1 (Medium, в скоупе) — ТЗ не закрывает несколько обязательных разделов §7.1 / чек-листа DoR §2.5
+
+Отсутствуют полностью, не как «неявно покрыто», а буквально нет ни строки:
+
+- **i18n** — не сказано даже «изменений нет». Между тем это не тривиально:
+  AC5 подразумевает, что `wall_model.reason.opening-host` и
+  `toast.zero_wall_migration_blocked` остаются в строю для пост-v9
+  fail-closed случая — я проверил, ключи в `ru.json`/`en.json` есть и не
+  тронуты, но ТЗ обязано сказать это явно, а не оставлять ревьюеру
+  реконструировать.
+- **Release-артефакты** — changelog RU+EN, документация, golden/скриншоты
+  не упомянуты вовсе. Это не формальность: правило 3.1 меняет зонирование
+  `cm:0` относительно beta.3 (сам документ признаёт это в разделе «Риски»),
+  то есть меняет видимую геометрию стен в местах с легаси-проёмами — а
+  значит потенциально golden-эталоны, где такая геометрия зафиксирована.
+  ТЗ не говорит, требуется ли пересъёмка/сверка golden, и не резервирует
+  правку `docs/CHANGELOG.md`/`docs/CHANGELOG.ru.md`, хотя описанное
+  поведение прямо user-visible (пользователь больше не видит блокирующий
+  тост, комната рисуется).
+- **Затронутые файлы/модули** — DoR §2.5 требует их перечисления
+  (`docs/specs/README.md` дальше по цепочке это использует); в ТЗ нет
+  списка (ожидаемо: `wall-segment-model.ts`, возможно
+  `scripts/config-field-registry.mjs`, `docs/specs/306-*.md` cross-ref).
+- **Влияние на производительность и на touch** — ни разу не названо, даже
+  как «нет» с обоснованием. Оба пункта DoR §2.5 сформулированы как
+  обязательные к явному ответу («или явно "нет"»), не как опциональные.
+- **Compatibility-поле по `docs/CONFIG-COMPATIBILITY.md`** — ТЗ не
+  упоминает этот документ вовсе, хотя правило 3.3 меняет реальный
+  контракт совместимости: `scripts/config-field-registry.mjs`, запись
+  `spaces[].openings[].host=wall` (строки 291–303), сейчас гласит
+  *«required for contour openings in model v8»* и *«materialize only when
+  exactly one carrier is proven»* — то есть текущий машиночитаемый канон
+  прямо противоречит новому §3.3 («без host» становится валидным
+  персистентным v9-состоянием при отсутствии кандидата). Правило
+  «документация — в том же коммите, что поведение» (PROCESS.md, правило
+  11) требует, чтобы эта запись реестра тоже обновлялась; ТЗ должно было
+  назвать это явно как часть контракта, а не оставить открытие
+  ревьюеру.
+
+Отдельно: раздел «принято предположительно, поменять свободно»
+(обязателен по PROCESS.md §7.1 для всего, что автор решает сам без
+вопроса владельцу) в документе отсутствует полностью, хотя решений
+уровня «непродуктовых, но не очевидных» несколько: порядок
+tie-break в 3.2 (текущий host → расстояние → `cm` → id), и — что важнее —
+семантика «инертного» проёма в 3.3 (проём остаётся в данных, но перестаёт
+резать стену/создавать тоннель) — это уже граничит с «что считать
+приемлемой деградацией», один из двух видов вопросов, которые процесс
+резервирует за владельцем (PROCESS.md §7.1). Формально владелец уже дал
+общую директиву «рисование нигде не блокируется», и 3.3 — разумное
+техническое следствие, но фиксация этого решения явным блоком (а не
+внутри нормативного правила) — то, что требует процесс, и то, что даёт
+ревьюеру предмет для несогласия, а не факт.
+
+**Почему Medium, а не High:** ни один из этих пробелов не делает уже
+написанные правила §3 невыполнимыми или непроверяемыми — рассуждение
+корректно и обосновано кодом (проверено чтением `wall-segment-model.ts`,
+см. «Как проверялось» п.3–6). Но по букве DoR §2.5 «если хоть один пункт
+не выполнен — статус не "Готово к разработке"», и здесь не выполнено
+сразу пять пунктов чек-листа. Чинится в этом же ТЗ без изменения
+контракта §3 — ревизия 2 добавляет разделы, не переписывает правила.
+
+### M2 (Medium, в скоупе) — AC3 описывает фикстуру, не достижимую в названном ею механизме
+
+AC3: «Два коллинеарных совпадающих кандидата (**контурный + независимый**,
+кейс #313)». Формулировка правила 3.2 сама ссылается на
+`resolveRoomOpeningHost.eligible`, а функция (проверено чтением
+`src/wall-segment-model.ts:588–615`) принимает единственный параметр
+`segments: readonly WallSegmentEntry[]` — это `space.wall_segments`,
+собранный `buildAtoms` **только из полигонов комнат** (контурные атомы).
+Независимая кладка (`partitions`, `room_drafts`, `wall_columns`) в эту
+коллекцию не попадает вообще, а `hostRoomOpenings` сама явно пропускает
+`opening.host?.kind === 'partition'` (строка 620) — партиционные проёмы
+резолвятся другим путём, не через это правило. Значит «контурный +
+независимый» кандидат для `resolveRoomOpeningHost` в принципе не может
+возникнуть так, как описано.
+
+Ссылка на «кейс #313» тоже не подтверждает сценарий: issue #313 — это
+инструмент «Толщина» (`_wallThickHit`), не умеющий выбирать отдельно
+стоящие стены/драфты в UI; он не о неоднозначности хоста проёма и не
+про два коллинеарных кандидата в `wall_segments`. Единственная найденная
+опора для «coincident»-сценария — `docs/WALL-THICKNESS.md` §9
+(«opening cannot punch a coincident independent wall», строки 480–509,
+`smoke_optimize_coincident_partition.mjs`) — но это Optimize-конкретный
+путь материализации, отдельный от `resolveRoomOpeningHost`, и там для
+партиционных проёмов другой резолвер (`host:{kind:'partition'}`).
+
+Реальный источник неоднозначности внутри `resolveRoomOpeningHost` скорее
+находится на стыке двух **соседних контурных атомов** (например, на
+границе смены толщины — `smoke_wall_thickness_transition.mjs`), где
+проём у самого стыка формально укладывается в допуск `EPS`/
+`GRID_STEP_N * 0.02` сразу для двух атомов. Это не то же самое, что
+заявлено в AC3.
+
+**Что нужно от автора:** заменить фикстуру AC3 (и её обоснование) на
+реально достижимую внутри `resolveRoomOpeningHost` — два коллинеарных
+контурных атома на стыке (thickness-transition или аналогичный кейс), с
+корректной ссылкой на источник, либо явно показать код-путь, которым
+независимая кладка всё же попадает в `segments` (если такой существует и
+я его не нашёл — тогда снимаю находку).
+
+**Почему Medium:** правило 3.2 (сам алгоритм tie-break) звучит разумно и
+тестируемо — есть исполнимая, реально достижимая фикстура для той же
+цели (стык двух контурных атомов). Дефект локален к описанию AC3, не к
+нормативному правилу.
+
+### L1 (Low) — коллизия термина «инертный»
+
+§3.3 называет безхостовый проём «инертным» («проём... становится
+инертным: не создаёт тоннель/вырез/тело, не участвует в физике»). Термин
+уже занят: `docs/UX-MODES.md` (строки 18, 207) использует «inert» для
+конкретного контракта — блокировки взаимодействия в View/kiosk по lock
+guard, установленного `docs/SCOPE.md`. В коде есть отдельный смок
+`demo/smoke_inert_openings.mjs`, который проверяет именно
+View-инертность (pointer-events/cursor), а не отсутствие хоста — это
+подтверждено чтением файла. Новое состояние (проём без host, при этом
+по-прежнему интерактивный и перетаскиваемый в Plan mode — см.
+`houseplan-card.ts:12680–12684`) описывает нечто иное, и повторное
+использование того же русского слова создаёт риск, что будущий автор
+кода/теста перепутает два разных инварианта по имени.
+
+**Предложение:** выбрать другой термин («безхостовый», «unhosted», «проём
+без носителя» — как и названо в остальном тексте ТЗ) и не использовать
+«инертный» применительно к этому состоянию.
+
+## Что проверено и корректно
+
+- Причинно-следственная цепочка (§1) точно совпадает с воспроизведением
+  из аналитического комментария владельца — не домысел, а прямая
+  трассировка к `commitWallSegmentModel`/`hostRoomOpenings` в текущем коде.
+- Границы (§2): корректно и дословно сверено с #306 §8.2 — заменяется
+  ровно пункт 4 («legacy-конфликт... блокирует миграцию всего
+  пространства»), пункты 1–3 (picker не предлагает 0 как host, запрет
+  перевода host-атома в 0, отсутствие tunnel/fill у нулевой стены) и
+  пункт 7 (§8, строка 68: «новый проём на нулевой стене запрещён»)
+  остаются в силе как пост-миграционные — согласуется с исходным текстом
+  #306.
+- Правило 3.1 (проём удерживает стену) — физический смысл обоснован,
+  критерий (интервал проёма пересекает интервал атома, допуск
+  `GRID_STEP_N * 0.02`, тот же угловой критерий) идентичен уже
+  существующему `eligible()` в `resolveRoomOpeningHost` — заимствование
+  проверенного критерия, а не новое изобретение. Технически потребует
+  перестановки порядка вычислений в `buildAtoms` (сейчас cuts считаются
+  до того, как известны `cm`/host), но это вопрос реализации, не
+  корректности ТЗ.
+- Правило 3.3 (проём без кандидатов → безхостовое состояние, рендер по
+  x/y) — подтверждено чтением `space-render.ts:285` и
+  `plan-geometry-preflight.ts:224/276`: обе точки уже трактуют
+  `!opening.host` как проходной случай («return [opening]»/«return
+  [fallback]»), то есть рендер по x/y для безхостового проёма — не
+  придуманное поведение, а уже существующий код-путь. Claim про
+  picker/placement также подтверждён (см. «Как проверялось» п.5).
+- AC1, AC2, AC4–AC6 — сформулированы однозначно, с указанным способом
+  доказательства (`smoke`/`unit`/`backend`), фикстуры проверяемы по
+  описанному коду.
+- Обратной совместимости данных, уже мигрировавших на beta.3, дано явное
+  и честное признание в разделе «Риски» — без попытки выдать частичный
+  охват за полный.
+- Откат (§6) конкретен и проверяем: ревёрт ветки восстанавливает
+  прежнее поведение, уже мигрировавшие v9-документы остаются валидными.
+
+## Чего не проверял
+
+- Не проверял осуществимость реализации на уровне алгоритма (порядок
+  вычислений в `buildAtoms`/`hostRoomOpenings` для правил 3.1–3.3) —
+  это задача автора кода, не предмет ревью ТЗ; я лишь убедился, что
+  используемые в правилах критерии (допуски, угловой тест) уже есть в
+  кодовой базе и не придуманы с нуля.
+- Не прогонял никаких гейтов (typecheck/test/build/смоки/golden) — на
+  этой стадии продуктового кода нет, класс изменений C (документация),
+  гейты для класса C не предусмотрены до кода.
+- Не проверял #319 (бэкенд-гард) — прямо исключён из скоупа этим ТЗ и
+  корректно помечен как отдельная задача.
+- Не оценивал точную реализуемость AC6 (байтовая идемпотентность) —
+  доверяю существующему паттерну (`commitWallSegmentModel`
+  уже задуман идемпотентным для Optimize, судя по комментариям в коде),
+  предметно не проверял иначе как чтением.
+
+## Вывод
+
+High-находок нет. M1 и M2 — в скоупе задачи, чинятся в этом же ТЗ без
+пересмотра нормативных правил §3 (кроме фикстуры AC3). Вердикт —
+жёлтый, документ возвращается автору на ревизию 2.