docs: spec review for #141

Issue: #141
User-Visible: no

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

@@ -0,0 +1,269 @@
+# SPEC-REVIEW-141-r1
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/141
+- **ТЗ под ревью:** `docs/specs/141-wall-junctions.md` (коммит `59c66dd`,
+  ветка `issue/141-wall-joints`)
+- **Роль:** ревьюер ТЗ (не автор), этап `S4-spec-review`
+- **Трек:** обычный (не `small`/`trivial`) — сложность/риск 7/10, две+
+  поверхности (перегородки/drafts и рисование комнатных стен), влияние на
+  golden, свет и производительность; лёгкий/короткий трек владелец и автор
+  корректно не применили.
+- **Цикл:** r1/4
+
+## Скоуп ревью
+
+Проверялось соответствие ТЗ:
+
+- `docs/SCOPE.md` — попадание в Core user jobs, отсутствие расширения скоупа;
+- `PROCESS.md` §2.4, §2.5 (DoR), §7.1 (обязательные разделы), §3/§12 (запреты);
+- `AGENTS.md` — классы файлов, ветка, трейлеры коммита ТЗ;
+- каноническим документам: `docs/WALL-THICKNESS.md` (модель толщины,
+  mitre/bevel, union тел), `docs/LIGHT.md` (барьеры/`_lightBarriers`),
+  `docs/SUN.md` (occluders), `docs/ISOMETRIC.md` (`wallBodiesGeometry` +
+  extra physical bodies), `docs/CANVAS.md` (координаты/масштаб),
+  `docs/TOUCH-SUPPORT.md` (best-effort editors), `docs/CONFIG-COMPATIBILITY.md`
+  (миграция — в задаче её нет, но проверено, что это верно);
+- `docs/USER-GUIDE.ru.md` — терминология «Контур комнаты» / «Перегородка»;
+- фактическому коду (`src/wall-thickness.ts`, `src/physical-geometry.ts`,
+  `src/houseplan-card.ts`) — на предмет того, что технический диагноз ТЗ не
+  является непроверенной догадкой, выданной за факт.
+
+## Как проверялось
+
+1. Прочитан весь тред issue #141: исходное описание с уточнением владельца
+   («проявляется и на прямых, и на непрямых углах»), полная аналитика
+   (ценность 8/10, сложность/риск 7/10, P2, тип `bug`, полный трек) с Q1–Q3 и
+   явными defaults, решение владельца «принимаю все defaults», финальный
+   комментарий автора со ссылкой на коммит ТЗ.
+2. Сверены обязательные разделы ТЗ (§7.1 PROCESS.md) построчно — таблица ниже.
+3. Прочитан код и подтверждены построчно все технические утверждения §3 ТЗ:
+   - `drawWallPreviewD()` (`src/wall-thickness.ts:490`) действительно строит
+     `outset−inset` только для `closed`-контура (:496); открытый путь —
+     независимый quad на каждый сегмент (:512-529). Диагноз ТЗ п.1 точен.
+   - `partitionBody()` (`src/physical-geometry.ts:41`) строит прямоугольник с
+     плоскими торцами без топологии узла; `physicalBodies()` (:89) просто
+     собирает такие прямоугольники в список. Диагноз ТЗ п.2 точен.
+   - `MITRE_LIMIT = 4` (`src/wall-thickness.ts:28`) и его использование в трёх
+     местах (:669, :1429, :1805) существуют — ссылка ТЗ §7.2.3/AC3 не
+     изобретена.
+   - Изометрия (`src/houseplan-card.ts:4606-4611`) уже вызывает
+     `wallBodiesGeometry(..., extras)` с `extras = physicalBodies(...)` —
+     значит full/static/iso уже объединяют независимые тела через
+     `wallBodiesUnionPath`/`wallBodiesGeometry` (тот же приём подтверждён в
+     `src/space-render.ts:338` для статической карточки). Ровно то, что ТЗ
+     утверждает в §3.5 и §8.2 («full/static/isometric render уже объединяют…»).
+   - `_lightBarriers()` (`src/houseplan-card.ts:13556`) вызывает
+     `wallBodiesGeometry()` **без** `extras` (:13623) и затем отдельно кладёт
+     сырые ребра каждого `physical`-тела в occluders (:13637:
+     `for (const body of physical) occluders.push(...polygonSegments(body));`).
+     Солнце (`src/houseplan-card.ts:12705-12716`) вызывает
+     `directionalOccluders(physical, ...)` напрямую на тех же сырых
+     прямоугольниках. Это подтверждает диагноз ТЗ §3.5/§8.2: свет и солнце
+     сегодня действительно обходят raw-прямоугольники отдельно от joined-пути
+     рендера, а не голословное утверждение.
+   - `floorMinusBodies()` вызывается с тем же `_physicalBodiesR()` (сырые
+     тела) и для чистой площади (:7366, :7388), и для проверки source-inside
+     (:11655) — совпадает с AC8.
+4. Прочитан `docs/WALL-THICKNESS.md` целиком: подтверждён существующий
+   контракт mitre/bevel для комнатных стен (§3) и отдельный раздел «9.
+   Independent partitions, drafts and columns» — независимые тела уже
+   юнионятся с комнатными только после вырезов проёмов и участвуют в
+   clean-floor/Glow/sun occlusion. ТЗ §7.3.2, §9 и AC6/AC10 корректно
+   продолжают именно эту, а не новую, модель.
+5. Прочитан `docs/LIGHT.md` и `docs/SUN.md`: подтверждено, что `_lightBarriers`
+   держит один общий барьерный набор на пространство и кэшируется по
+   геометрическому fingerprint, а не по `_cfgEpoch` — ТЗ §8.3.1-2 корректно
+   продолжает существующий кэш-контракт, не изобретая новый механизм
+   инвалидации.
+6. Прочитан `docs/ISOMETRIC.md`: `src/iso-walls.ts` явно описан как
+   потребитель «канонического `wallBodiesGeometry()` MultiPolygon after
+   openings and extra physical bodies have been resolved» — буквально
+   подтверждает ТЗ §8.2 («full/static/isometric не создают разные join
+   алгоритмы»), т.е. это не домысел автора, а прямая цитата канона.
+7. Прочитан `docs/TOUCH-SUPPORT.md`: политика требует, чтобы «новые
+   спецификации editor-фич и код-ревью» явно указывали одно из трёх значений
+   `Touch editor: supported / best effort / not exposed`. ТЗ §10 описывает
+   ровно best-effort-поведение словами, но не использует эту точную метку —
+   см. Low-1.
+8. Прочитан `docs/USER-GUIDE.ru.md` (раздел «Создание комнаты», таблица
+   «Инструменты плана», раздел «Виртуальные стены»): термины «Контур комнаты»
+   и «Перегородка» в ТЗ (§4.1, §5.3 и др.) совпадают с интерфейсным словарём
+   ровно там, где употребляются полностью; там же нашёл существующую строку
+   «Углы | Соседние тела стен соединяются…» (:347) — актуальное описание,
+   которое release-артефакты ТЗ (§15) обязывают дополнить для partitions —
+   корректно учтено.
+9. Прочитан `src/types.ts:45-72` — `RoomDraftCfg`, `PartitionCfg`,
+   `space.room_drafts`, `space.partitions` существуют как реальные поля
+   схемы, а не придуманы для ТЗ; AC4/AC5 про кросс-типовые соединения
+   (partition↔partition, draft↔partition, partition/draft↔room wall)
+   технически имеют смысл на этой модели.
+10. Проверена запись `docs/specs/README.md:85` — строка на #141 добавлена в
+    том же коммите; ссылка issue ↔ ТЗ двусторонняя (ТЗ ссылается на issue в
+    заголовке, issue-комментарий ссылается на файл ТЗ и коммит).
+11. Проверены трейлеры и class-принадлежность: `git diff --stat
+    origin/dev...HEAD` показывает только `docs/specs/141-wall-junctions.md` и
+    `docs/specs/README.md` (класс C, ни одного файла класса A); коммит
+    `59c66dd` несёт `Issue: #141` и `User-Visible: no` — корректно для
+    документа ТЗ, который сам не меняет поведение.
+
+## Обязательные разделы (§7.1 PROCESS.md)
+
+| Раздел | Есть | Комментарий |
+|---|---|---|
+| Сценарий (персона/поверхность/момент) | ✅ | §1 — домашний администратор, desktop Plan editor, момент второго клика |
+| Что человек увидит до/после | ✅ | §2, «До:»/«После:» — см. Low-2 |
+| Проблема (с подтверждённой причиной) | ✅ | §3, шесть пунктов, все проверены по коду (см. выше) |
+| Скоуп / не-скоуп | ✅ | §5 / §6, явные границы (без snap #137, без нового cap, без #138) |
+| Контракт поведения | ✅ | §7 (геометрия) + §8 (архитектура) |
+| Модель данных и миграция | ✅ | §9 — явное «schema не меняется», «читается исправленно, без записи» |
+| UX, i18n, accessibility, touch | ✅ | §10 — см. Low-1 |
+| AC1…ACn с доказательством | ✅ | §12, 13 штук, каждый с типом доказательства и наблюдаемым результатом |
+| План автотестов | ✅ | §13, по подпунктам на unit/smoke/golden/performance |
+| Риски | ✅ | §16, таблица с вероятностью/ущербом/снижением |
+| Откат | ✅ | §17 |
+| Release-артефакты | ✅ | §15, конкретный список файлов документации и обоих changelog |
+
+Все обязательные разделы присутствуют и содержательны. Дополнительно есть
+раздел «Решения владельца» (§4, явно фиксирует принятые Q1–Q3), архитектурный
+контракт (§8), план реализации (§14, помечен как техническая свобода) и явный
+блок «принятые технические предположения» (§18) — соответствует духу
+PROCESS.md §7.1 об отделении продуктового решения от технического.
+
+## Находки
+
+Находок уровня **High** и **Medium** нет — новых issue не требуется.
+
+### Low-1 — нет явной метки touch-контракта по `docs/TOUCH-SUPPORT.md`
+
+**Файл:** `docs/specs/141-wall-junctions.md:252-261` (§10)
+
+`docs/TOUCH-SUPPORT.md` требует: «New editor feature specifications and code
+reviews must state one of: `Touch editor: supported`; `Touch editor: best
+effort / intentionally degraded`; `Touch editor: not exposed`.» §10 ТЗ по
+существу описывает best-effort-контракт словами («Plan editor остаётся
+desktop-first… новый hover parity не обещается») и корректно ссылается на
+safety floor (сохранённая geometry тапнутого сегмента должна совпадать), но не
+использует ни одну из трёх канонических формулировок буквально.
+
+**Решение ревьюера:** Low, не блокирует. При следующей правке ТЗ или в
+хендоффе код-ревью достаточно добавить одну строку `Touch editor: best effort
+/ intentionally degraded` — по содержанию это именно то, что §10 уже
+описывает.
+
+### Low-2 — «что человек увидит» длиннее одной фразы
+
+**Файл:** `docs/specs/141-wall-junctions.md:28-37` (§2)
+
+PROCESS.md §7.1 требует «одной фразой, без терминов реализации». Раздел
+написан двумя короткими абзацами («До:» / «После:»), по одному предложению
+каждый; по существу требование выполнено (без терминов реализации, конкретно
+и однозначно), но формально это не «одна фраза». Тот же класс находки уже
+фиксировался как Low и не блокировал приёмку в `SPEC-REVIEW-137-r1`.
+
+**Решение ревьюера:** Low, не блокирует. Косметическая правка на усмотрение
+автора.
+
+### Low-3 — доказательство AC10 («code review») до кода не проверяемо, но это ожидаемо для code-review-этапа
+
+**Файл:** `docs/specs/141-wall-junctions.md:315-321` (AC10, AC11)
+
+AC10 и AC11 указывают доказательство «`unit` + code review» / «`performance` +
+code review» — это не входит буквально в список §2.5 PROCESS.md
+(`unit`/`backend`/`smoke`/`golden`/«ревью кода»), но «code review» — это то же
+самое «ревью кода» иначе сформулированное, и AC11 корректно называет
+`performance`, обязывая код-ревьюера прогнать performance-профиль по правилу
+«гейты соразмерны AC» (`PROCESS.md` §8). Не дефект по существу, тот же класс
+находки не блокировал `SPEC-REVIEW-137-r1` (Low-3 там).
+
+**Решение ревьюера:** Low, не блокирует. Оставить как есть.
+
+## Что проверено и корректно
+
+- **Соответствие `docs/SCOPE.md`:** задача закрывает **J4** («от нуля до
+  плана без внешнего редактора» — администратор должен доверять форме стены в
+  момент рисования, не после замыкания) и **J6** («план остаётся правдивым по
+  мере развития» — независимые перегородки не деградируют в кривую геометрию
+  навсегда). Обе строки Closed — это исправление дефекта внутри принятой
+  функциональности, не расширение продукта.
+- **Легитимность полного трека:** сложность/риск 7/10, минимум две
+  поверхности (перегородки и рисование стен комнаты), влияние на golden, свет
+  и производительность — критерии `small`/`trivial` (§5/§5.1 PROCESS.md) не
+  выполняются ни по одному пункту; полный трек выбран верно.
+- **Продуктовые вопросы закрыты по процессу:** Q1 (какие соединения входят),
+  Q2 (нормализуется ли rubber-band до клика) и Q3 (что делать со свободными
+  торцами) заданы одним пакетным комментарием, каждый с предлагаемым default,
+  issue корректно ушёл в `blocked`+`S3-spec` до ответа и вышел из `blocked`
+  сразу после решения владельца. Ни одна догадка не выдана за факт без
+  пометки — §18 отдельно и явно перечисляет технически свободные решения и
+  прямо фиксирует «нет открытых продуктовых вопросов» (§18.10).
+- **Технический диагноз не голословен.** Все шесть пунктов §3 (независимый
+  путь `drawWallPreviewD`, плоские торцы `partitionBody`, отсутствие топологии
+  узла в `physicalBodies`, отдельный путь mitre для закрытого контура,
+  частичное объединение в full/static/iso против необъединённых barriers у
+  Glow/sun, неподтверждённость отдельного дефекта свободного торца по
+  скриншоту) построчно проверены по исходному коду и совпадают с ним — см.
+  «Как проверялось» п.3.
+- **Не-скоуп (§6) корректно отсекает соседние соблазны:** snap-tolerance/#137,
+  автоматическое дробление persisted-геометрии, превращение partition в
+  границу комнаты/HA-зону, новый persisted junction-тип и миграция, новый
+  cap/join selector, #138 (замыкание контура по углам существующей комнаты) —
+  все явно исключены с указанием, почему это не эта задача.
+- **Регрессионные гарантии сформулированы явно:** AC10 поимённо защищает
+  проёмы комнатных стен, virtual-T, nested/partial стены, экстерьер #123 и
+  колонны — то есть ровно те механизмы, которые уже используют
+  `MITRE_LIMIT`/union и могли бы негласно пострадать от новой топологии узлов.
+- **Производительность:** §11/AC11 корректно ссылаются на существующую
+  60-partition фикстуру `large-house-v1` и явно запрещают full-house boolean
+  union на каждый pointermove/HA-tick (§8.3.3) — согласуется с
+  `docs/LIGHT.md` («barriers keyed by geometry fingerprint, never `_cfgEpoch`»)
+  и не вводит новый бюджет без решения процесса.
+- **Модель данных и миграция (§9):** корректно заявлено «schema не меняется»,
+  «читается исправленно без записи», без прямой/обратной миграции — сверено с
+  `docs/CONFIG-COMPATIBILITY.md`: задача не создаёт нового
+  compatibility-случая, потому что не меняет persisted-представление.
+- **Release-артефакты (§15)** перечисляют конкретные существующие документы
+  (`WALL-THICKNESS.md`, `LIGHT.md`, `SUN.md`, `ISOMETRIC.md`, `ARCHITECTURE.md`,
+  `TESTING.md`, `STATUS.md`, `USER-GUIDE.ru.md`) и оба changelog в одном
+  implementation-коммите — соответствует §7.1/правилу 11 PROCESS.md. Golden
+  корректно ограничен принятием только через `npm run golden:accept --
+  --reviewed` по полному Linux-артефакту (§13.3), perf/golden/browser-suite
+  верно отнесены к пре-релизному, а не implementation-гейту (§8, §11.4
+  PROCESS.md).
+- **Откат (§17)** корректно опирается на отсутствие миграции данных: revert
+  implementation-коммита восстанавливает прежний (дефектный) визуал без
+  риска для сохранённых данных.
+- **Трассируемость:** `docs/specs/README.md:85` обновлён тем же коммитом
+  (`59c66dd`); ветка `issue/141-wall-joints` и трейлеры (`Issue: #141`,
+  `User-Visible: no`) корректны для документа класса C, который сам не меняет
+  поведение. `git diff --stat origin/dev...HEAD` не содержит ни одного файла
+  класса A — продуктовый код не тронут до `S5-ready` (правило №1).
+
+## Чего не проверял
+
+- Не проверял реализуемость «одного чистого вычисляемого frame» (§8.1) как
+  конкретной структуры данных/API — по правилам ТЗ это явно свободное
+  техническое решение автора кода (§18.2), не предмет ревью ТЗ.
+- Не запускал автотесты, `golden`, `performance` или browser-смоки — на этапе
+  `spec` это не требуется; существование фикстур (`large-house-v1`, реальные
+  поля схемы, конкретные функции и их поведение) проверено чтением кода, а не
+  исполнением.
+- Не проверял, что boolean-decomposition конкретной реализации (mitre patch
+  для узла степени 3+, T-разрез сквозного сегмента) технически осуществима
+  средствами `polyclip-ts` в разумное время — это открытое для автора кода
+  техническое решение, отмеченное в ТЗ как свободно изменяемое (§18.3), и
+  фактическая проверка бюджета (AC11) относится к код-ревью и пре-релизному
+  гейту.
+- Не проверял корректность конкретных числовых оценок аналитики (8/10 · 7/10 ·
+  P2) по существу — это поле владельца (PROCESS.md §2.2), уже принято явным
+  решением владельца до написания ТЗ.
+- Не проверял связанные issue #123/#137/#138 по существу за пределами того,
+  что понадобилось для верификации ссылок ТЗ (регрессия #123, снэп #137,
+  разграничение с #138) — они не входят в предмет этого ревью.
+
+## Вердикт
+
+Зелёный. High: 0, Medium: 0. Три находки Low (отсутствие буквальной метки
+touch-контракта по `docs/TOUCH-SUPPORT.md`; «что человек увидит» длиннее
+одной фразы; типы доказательства AC10/AC11 вне буквального перечня §2.5) — ни
+одна не блокирует приёмку, все либо правятся косметически при следующей
+редакции, либо снимаются этой записью без нового цикла.