docs: review document for #306

Issue: #306
User-Visible: no

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

@@ -0,0 +1,291 @@
+# SPEC-REVIEW-306-r1
+
+- **Issue:** [#306](https://github.com/Matysh/houseplan-card/issues/306) — Редактор плана: заменить виртуальные стены обычными стенами толщиной 0
+- **Этап:** ревью ТЗ (PROCESS.md §2.4)
+- **Заход:** r1 · блокирующих циклов израсходовано 0 из 4 (до этого раунда)
+- **ТЗ:** `docs/specs/306-zero-thickness-walls.md`, зафиксировано коммитом `904c47e44c7d08ec3e029678226a304fa3a15cc8` («docs: specify zero-thickness wall migration»), это же HEAD ветки на момент ревью
+- **Метка issue:** `S4-spec-review` (не `small`) — полный трек, ТЗ обязано жить в `docs/specs/`, что выполнено
+
+## Скоуп ревью
+
+Первый заход, полный разбор (§2.10 неприменим — это не повторный раунд).
+Проверено: тело issue #306 и все 3 комментария (аналитика автора, дополнение
+владельца о `zero_wall_style`, хендофф автора ТЗ); файл
+`docs/specs/306-zero-thickness-walls.md` целиком (654 строки, 19 разделов,
+18 AC); соответствие `docs/SCOPE.md` (J4/J6), `AGENTS.md`, `PROCESS.md` §7.1 и
+§2.5 (DoR-чеклист); сверка технических утверждений ТЗ с текущим кодом
+(`custom_components/houseplan/validation.py`, наличие модулей из §14) и с
+каноническими документами `docs/LIGHT.md`, `docs/WALL-THICKNESS.md`,
+`docs/TOUCH-SUPPORT.md`, `docs/CONFIG-COMPATIBILITY.md`,
+`docs/USER-GUIDE.ru.md`; сверка структуры документа с прецедентами полного
+трека — `docs/specs/299-mixed-role-wall-records.md` и
+`docs/specs/224-config-coordinate-canonicalization.md` (оба приняты, оба того
+же автора/жанра: геометрия стен/схема данных).
+
+## Как проверялось
+
+- `git show --stat 904c47e4` — подтверждён коммит и его состав (только два
+  файла: сам ТЗ и `docs/specs/README.md`).
+- Построчное чтение ТЗ против обязательных разделов §7.1 и DoR §2.5.
+- Точечная верификация фактических утверждений о текущей системе, а не
+  доверие тексту ТЗ на слово:
+  - `custom_components/houseplan/validation.py:1236,1264,1286` — `cm` для
+    `walls[]`/`room_drafts[].segments[]`/`partitions[]` действительно
+    `Range(min=1, max=100)`; `:1309` — `wall_columns[].cm` действительно
+    `Range(min=1, max=150)`. Оба факта, на которых стоит модель диапазонов
+    §4.1 и §2 п.2, подтверждены кодом, а не только словами автора.
+  - `custom_components/houseplan/const.py:54` — `PLAN_MODEL_VERSION = 7`,
+    совпадает с заявленным переходом «7 → 8» (§4.1, §8).
+  - `custom_components/houseplan/validation.py:911,913,917,921` —
+    `MAX_OPENINGS/MAX_WALLS/MAX_WALL_COLUMNS/MAX_OPEN_SPANS = 500`, совпадает с
+    лимитом 500 в §8 п.9, §11 и рисках §18.
+  - Существование всех перечисленных в §14 существующих модулей
+    (`wall-thickness.ts`, `physical-geometry.ts`, `wall-face-graph.ts`,
+    `wall-merge.ts`, `open-spans.ts`, `houseplan-card.ts`,
+    `light-visibility.ts`, `sun.ts`, `iso-walls.ts`, `plan-optimizer.ts`,
+    `plan-geometry-preflight.ts`, `coordinate-canonicalization.ts`,
+    `coordinate_canonicalization.py`) — все на месте; `zero-walls.ts` заявлен
+    как новый, и ТЗ прямо говорит, что имя можно менять — не выдаётся за
+    существующий факт.
+  - `docs/LIGHT.md` — правило «bare outline of any room edge with no
+    thickness is opaque» и «virtual (open) boundaries are transparent»
+    совпадает с AC3 (отсутствующая запись = легаси физическая стена,
+    непрозрачна) и с fallback `dashed` (прозрачна) — ТЗ не противоречит
+    принятой световой модели.
+  - `docs/TOUCH-SUPPORT.md:9,38,95,114,136,166` — «View/kiosk touch-first,
+    editors best effort» совпадает с §12 ТЗ.
+  - `docs/CONFIG-COMPATIBILITY.md` (раздел «Status meanings», строки 37-46) —
+    сверка предложенных в ТЗ статусов реестра с действующим перечнем. См.
+    находку M3.
+  - `docs/USER-GUIDE.ru.md` — полнотекстовый грep по «Граница/виртуальн»
+    (строки 58, 432, 527-548, 1439, 1473, 1496-1499, 1641, 1649, 1682) —
+    подтверждено, что RU-документ подробно описывает удаляемый инструмент.
+    См. находку M2.
+  - Структурная сверка с `docs/specs/299-*.md` и `docs/specs/224-*.md`
+    (`grep '^## '`) — оба начинаются разделами «Сценарий» и «Что человек
+    увидит до и после», оба содержат отдельный раздел «План автотестов» и
+    «Принятые технические предположения». См. находку M1.
+- `gh issue view 306 --json ... --comments` — прочитаны все три комментария
+  целиком, включая уточнение владельца про световой режим (второй комментарий,
+  15:10:53Z), которое ТЗ полностью учло (таблица §2 п.4 совпадает буква в
+  букву).
+
+## Находки
+
+### M1 (Medium, в скоупе) — отсутствуют два обязательных первых раздела ТЗ
+
+**Файл:** `docs/specs/306-zero-thickness-walls.md`, строки 1–24 (§1 «Цель»
+идёт первым разделом).
+
+PROCESS.md §7.1 называет обязательный список разделов ТЗ и отдельно
+подчёркивает: «Два первых раздела — продуктовые, и они идут первыми не
+случайно. **Сценарий:** какая персона (`docs/SCOPE.md`), на какой
+поверхности, в какой момент это встретит. **Что человек увидит:** одной
+фразой, без терминов реализации. ТЗ, которое не может ответить на эти два
+вопроса, описывает работу, а не изменение продукта.»
+
+В ТЗ #306 таких разделов нет вообще — ни как отдельных заголовков, ни как
+абзаца внутри «Цели». Документ начинается прямо с технической формулировки
+(«Убрать из продукта отдельный инструмент «Граница» и канонические сущности
+`space.open_spans`/`rooms[].open_to`»). Персона (администратор дома,
+`docs/SCOPE.md` J4/J6), поверхность (Plan editor, десктоп) и момент
+(рисование/правка стены, настройка пространства) угадываются по контексту, но
+не зафиксированы явно, и однофразового «до/после» без терминов реализации в
+документе нет вообще — есть только раздел issue «Результат для пользователя»
+(не часть ТЗ-файла) с пятишаговым технически описанным сценарием.
+
+Это не единичное упущение: два предыдущих принятых ТЗ той же линейки —
+`docs/specs/299-mixed-role-wall-records.md` (§1 «Сценарий», §2 «Что человек
+увидит до и после») и `docs/specs/224-config-coordinate-canonicalization.md`
+(§1 «Сценарий, персона и момент», §2 «Что человек увидит до и после») —
+оба следуют этому требованию буквально с теми же заголовками. #306 из этого
+ряда выпадает.
+
+**Сценарий отказа:** без явного «до/после» ревьюер и будущий читатель ТЗ
+проверяют соответствие продукту (`docs/SCOPE.md`) по разрозненным техническим
+формулировкам §1–§2, а не по одному предложению. Для задачи этого размера
+(654 строки, смена модели данных, 18 AC) риск разъехаться с тем, что владелец
+на самом деле утвердил, — не гипотетический: второй комментарий владельца сам
+поправил первую аналитику автора («стиль не влияет на геометрический смысл»
+→ отменено), то есть продуктовая рамка здесь уже была неоднозначной один раз.
+
+Не High: ни один из 18 AC не становится из-за этого неоднозначным или
+недоказуемым — сам контракт поведения (§2, §5–§8) детален и проверяем.
+Дефект структурный, чинится в этом же ТЗ без переоценки AC.
+
+**Почему это Medium, а не High:** реализация не может пойти по неверному пути
+из-за отсутствия этих разделов — все AC самодостаточны; речь о структурной
+полноте документа, а не о содержательной дыре в контракте.
+
+### M2 (Medium, в скоупе) — `docs/USER-GUIDE.ru.md` не входит в список затрагиваемых/release-документов
+
+**Файл:** `docs/specs/306-zero-thickness-walls.md`, строка 424 (§14
+«Затронутые модули») и строка 613 (§17 «Release-артефакты»).
+
+Оба места называют `docs/USER-GUIDE.md`, но не `docs/USER-GUIDE.ru.md`.
+Проект ведёт русский и английский user-guide отдельными файлами (как
+changelog: `docs/CHANGELOG.md` + `docs/CHANGELOG.ru.md`, оба явно
+упомянуты в этом же ТЗ). AGENTS.md прямо требует: «для работы, которая меняет
+видимое поведение, также читай `docs/USER-GUIDE.ru.md` — терминология
+интерфейса берётся оттуда […] или UI начинает говорить на языке разработчика».
+
+`docs/USER-GUIDE.ru.md` сейчас содержит развёрнутое описание удаляемого
+инструмента «Граница»/термина «виртуальная стена»: таблица инструментов
+(строка 432 — «Граница | Виртуальный участок общей стены | …»), отдельный
+раздел «Виртуальные стены» (строки 529–549, с таблицей ситуаций: создание,
+закрытие, объединение соседних участков, поведение проёмов, толщины, света,
+просмотра, редакторов, «внешнюю стену сделать виртуальной нельзя»,
+«ответвление из середины не поддерживается»), упоминания в разделах Optimize
+(строка 1439), удаления комнаты (строка 1447), совпадающей partition (строки
+1496–1499), инвариантов (строка 1641) и лимитов хранилища (строка 1649: «500
+записей стен и 500 виртуальных участков»). После реализации #306 весь этот
+материал станет неверным (инструмента не будет, термин исчезает, лимит
+`MAX_OPEN_SPANS` перестаёт быть отдельным лимитом пользовательского объекта).
+
+**Сценарий отказа:** реализатор берёт §14/§17 как чек-лист файлов для правки
+в том же коммите (правило AGENTS.md «документация — в том же коммите, что
+поведение», правило 11 PROCESS.md §3). Раз файл не назван — обновление RU
+user-guide не гарантировано формально, и продукт получает ту же болезнь, что
+уже отмечена как известный пробел в `docs/SCOPE.md` («README predates the
+two-editor redesign») — только на этот раз с описанием несуществующей более
+функции, а не отставанием скриншотов.
+
+**Почему Medium, не High:** не блокирует ни один AC (AC18 требует
+согласованности RU/EN user docs в общем виде и формально не провалится, если
+проверяющий не заметит отсутствие явного файла в списке) и легко чинится
+добавлением одной строки в §14 и §17.
+
+### M3 (Medium, в скоупе) — статусы compatibility-реестра в §4.2 не совпадают с действующим перечнем `docs/CONFIG-COMPATIBILITY.md`
+
+**Файл:** `docs/specs/306-zero-thickness-walls.md`, строки 135–136 (§4.2):
+«добавляются в `docs/CONFIG-COMPATIBILITY.md` как `deprecated-read /
+project-in-memory / migrate-on-structural-write`».
+
+Действующий раздел «Status meanings» в `docs/CONFIG-COMPATIBILITY.md` (строки
+37–46) определяет ровно пять статусов: `decision-required`, `deprecated-read`,
+`migrate-on-write`, `migrate-on-settings-save`, `drop-on-validation`. Ни
+`project-in-memory`, ни `migrate-on-structural-write` в этом перечне не
+существует. Документ там же называет `scripts/config-field-registry.mjs`
+машиночитаемым источником истины для этих статусов — то есть это не
+просто описательный текст, а контролируемый словарь, который потребляет
+тулинг (`npm run audit:config`).
+
+Показательно, что первый комментарий автора к этому же issue (аналитика,
+раздел «Рекомендуемый целевой контракт», пункт про #33) корректно ссылался на
+существующую пару `deprecated-read/migrate-on-write` — а финальный текст ТЗ
+разошёлся с собственной же более ранней и верной формулировкой.
+
+**Сценарий отказа:** реализатор либо придумывает новый статус в реестре без
+отдельного решения (реестр — «машиночитаемый источник истины», расширение
+словаря — не тривиальная правка одного файла документации, а изменение
+контракта тулинга), либо использует несуществующий статус как текстовую
+метку не в реестре, и `docs/CONFIG-COMPATIBILITY.md` расходится с
+`scripts/config-field-registry.mjs` — то есть ровно то рассогласование, которое
+этот документ существует, чтобы предотвратить. AC15 («`docs/
+CONFIG-COMPATIBILITY.md` описывает оба deprecated поля и downgrade») не
+называет, какой из двух наборов статусов правильный, и в текущем виде
+недоказуем однозначно.
+
+**Предлагаемая правка (техническое решение, не продуктовое, автор решает
+сам):** либо переиспользовать `deprecated-read` (для чтения) и
+`migrate-on-write` (для записи) — оба уже существуют и по смыслу подходят, —
+либо явно завести новый статус в самом реестре `config-field-registry.mjs` и
+задокументировать его добавление как часть этой задачи, а не просто
+упомянуть в prose.
+
+**Почему Medium, не High:** это техническое решение (не продуктовое), решается
+автором/ревьюером без владельца (§7.1), не меняет ни один пользовательский AC
+и не выглядит как факт, выданный за поведение продукта — просто
+несогласованная терминология, дешёвая в правке.
+
+### L1 (Low, снимается с записью) — нет консолидированного блока «принято предположительно»
+
+§7.1 требует явный блок в конце ТЗ для нерешённых технических деталей
+(«принято предположительно, поменять свободно»), и оба предыдущих ТЗ той же
+линейки (#299 §15, #224 §16) его содержат отдельным разделом. В #306 такого
+раздела нет, хотя технические допущения по факту делаются: имя модуля
+`src/zero-walls.ts` («имя можно уточнить без изменения контракта», §3.3),
+конкретные пороги производительности 10%/5% (§11), выбор статусов реестра
+совместимости (см. M3). Часть из них снабжена инлайн-пометкой о свободе
+изменения (§3.3), но не собрана в один блок, который ревьюер мог бы
+предметно оспорить целиком.
+
+**Решение ревьюера:** не блокирует и не требует отдельной правки — все
+затронутые технические решения либо помечены инлайн как свободные к
+изменению, либо уже пойманы отдельными находками (M3). Снимается с этой
+записью; при следующем ТЗ той же линейки стоит вернуться к практике отдельного
+раздела.
+
+## Что проверено и признано корректным
+
+- **Структура AC.** Все 18 AC пронумерованы, у каждого есть однозначный
+  критерий и явный способ доказательства (unit/backend/smoke/golden), как
+  требует DoR §2.5. Ни в одном AC не найдено недоказуемой или расплывчатой
+  формулировки.
+- **Продуктовые решения зафиксированы, открытых вопросов нет.** §2 п.10:
+  «Открытых продуктовых вопросов нет» — подтверждается: оба комментария
+  владельца (включая уточнение про световой режим как настройку пространства,
+  а не свойство стены) полностью учтены текстом §2 п.4 и §7.1 светового
+  раздела; расхождений между тем, что попросил владелец, и тем, что описывает
+  ТЗ, не найдено.
+- **Инвариант «отсутствие записи ≠ нулевая стена» (AC3)** — центральный
+  инвариант всей миграции — сформулирован явно и многократно (§1, §3.2, §8
+  п.5, риски §18) и подтверждён действующей моделью света (`docs/LIGHT.md`:
+  «a wall is still a wall when it is drawn as a line» — оpaque).
+  Ложного автоматического преобразования легаси оси в открытую стену
+  документ не допускает ни в одном месте.
+- **Защита проёмов от потери (AC10)** — контракт «никогда не удалять
+  автоматически» повторяется консистентно в §6.2, §8 п.6, §18 и первом
+  комментарии автора; нет расхождений между разделами.
+- **Диапазоны толщины (0..100 для стен/перегородок, 1..150 для колонн)**
+  проверены по факту в `validation.py` — текущие серверные ограничения
+  совпадают с тем, что автор заявляет как «сейчас», so предлагаемое изменение
+  диапазона корректно описывает дельту, а не выдумывает её.
+  и учитывает существующий предел колонн, не трогая его — согласовано с
+  `docs/WALL-THICKNESS.md`.
+- **Touch/View floor (§12, AC16)** согласован с `docs/TOUCH-SUPPORT.md`:
+  View/kiosk — блокирующий гарантированный уровень, Plan editor — best effort,
+  ровно так, как определяет каноника.
+  Ссылки на существующие модули (§14) — все перечисленные существующие файлы
+  на месте; `zero-walls.ts` заявлен как новый, что не является ложным фактом.
+- **Совместимость и откат (§10)** реалистичны: явно назван unsupported
+  downgrade после записи `cm:0`, явно исключён labs-флаг с обоснованием (две
+  одновременно пишущие модели опаснее фиче-флага) — техническое решение
+  автора, не спрятанное как факт.
+- **Зависимости и вне скоупа (§19)** соответствуют комментарию-аналитике:
+  #282 верно не блокирует эту задачу, но и не дублируется второй новой
+  сущностью; #148/#173 получают superseded-заметку, а не переписывание задним
+  числом (§14) — проверено, что #173 не содержит прямо противоречащего
+  контракта «Границы» (только общеанглийское слово «boundary» = край комнаты),
+  так что заявленное действие соразмерно фактическому конфликту.
+
+## Чего не проверял
+
+- **Реализуемость световой модели «line barrier без площади» в текущем
+  ray-sweep (`splitAtIntersections`/`visibilityPolygon`)** — не запускал
+  никакой код и не писал прототип; сверил только то, что автор ТЗ явно
+  называет эпсилон-риск и ограничивает его границами numeric predicate (§7.1,
+  §11), то есть заранее не выдаёт непроверенное решение за факт. Технический
+  риск реален (это по сути новый тип барьера, а не полигон), но он назван
+  как риск в самом ТЗ (§18: «Glow и sun разойдутся» / «единый resolver»), а
+  не спрятан — на этапе ТЗ этого достаточно, полную проверку сделает
+  код-ревью и `test/light-visibility.test.mjs`/`test/sun.test.mjs`.
+- **Полный список `docs/specs/README.md`** на прочие несоответствия вне
+  #306 — не аудировал.
+- **Гейты/тесты не запускал** — на этапе ревью ТЗ нет кода для типографии/
+  сборки; авторская проверка (`node --test test/docs-accept.test.mjs
+  test/process-gate.test.mjs` — 37 passed, 1 skipped) не перепроверялась
+  повторным запуском, так как класс изменений — только `docs/**` (класс C) и
+  утверждение автора относится к формальным doc-гейтам, не к продукту.
+- **`ADR #282` и его точная граница с #306** — прочитан только через ссылки
+  из issue-комментария и §19 ТЗ, сам ADR-файл не читался целиком.
+
+## Вердикт
+
+Три находки Medium, все в скоупе задачи, все дешёво чинятся правкой текста
+ТЗ без пересмотра AC или продуктовых решений. High-находок нет: ни одна не
+делает какой-либо AC недоказуемым, неоднозначным или основанным на выданной
+за факт догадке — предложенный контракт поведения детален, самосогласован и
+там, где я мог сверить его с кодом/канон-документами, подтверждён.
+
+`Вердикт: жёлтый · заход r1 · блокирующих циклов 1/4 · High: 0 · Medium: 3 → в задаче`