docs: review document for #278

Issue: #278
User-Visible: no

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

@@ -0,0 +1,165 @@
+# SPEC-REVIEW — Issue #278 · заход r1
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/278
+- **ТЗ:** `docs/specs/278-wall-union-isolation.md` (SHA `d906e5d8`, ветка
+  `issue/278-wall-union-isolation`)
+- **Трек:** обычный (small/trivial явно исключены автором) — файл ТЗ обязателен,
+  что соблюдено.
+- **Заход:** r1 · блокирующих циклов израсходовано 0 из 4.
+
+## Скоуп ревью
+
+Ревью ТЗ по PROCESS.md §2.4/§7.1: выполнимость и проверяемость AC1–AC15,
+отсутствие догадок, выданных за факт, полнота обязательных разделов, согласие
+с `docs/SCOPE.md` и каноническими документами подсистемы (`docs/WALL-THICKNESS.md`),
+непротиворечивость связанным issue (#141, #197, #199, #276, #277).
+
+Продуктовый код не менялся (`git diff --stat origin/dev...HEAD` — только
+`docs/specs/278-wall-union-isolation.md` и запись в `docs/specs/README.md`),
+поэтому гейты `tsc`/`test`/`build`/`check-docs` к этому заходу не применяются:
+на этапе spec трогается только документ, дерево кода идентично `dev`.
+
+## Как проверялось
+
+1. Прочитан текст issue #278 и оба комментария автора (аналитика 10/9/7/9,
+   P1, уточнение доказательной базы после повторного запуска preflight #199
+   на `payload.config`).
+2. Прочитан `docs/SCOPE.md` — задача закрывает J1/J4/J6 (не даёт локальному
+   дефекту физической геометрии визуально стирать этаж; план должен
+   "keep the plan true").
+3. Прочитан `docs/WALL-THICKNESS.md` §9 (Independent partitions, drafts and
+   columns) — сверка терминологии `extraBodies`, `physicalBodySet()`,
+   joined/raw body set, Glow/sun/clean-floor consumers.
+4. Прочитан код `src/wall-thickness.ts` (`wallBodiesGeometry`,
+   `wallBodiesUnionPath`, циклы `roomRings`/`wallEdgeBodies`/`extraBodies`,
+   строки 2743–2880) — проверка центрального технического утверждения ТЗ.
+5. Проверены issue #197 (закрыт) и #199 (закрыт) — согласованность границ
+   "не дубликат".
+6. Проверены #276 и #277 — оба сейчас также в `S4-spec-review` (не смержены),
+   что важно для оценки принятого предположения о порядке merge (§18.4).
+7. Проверено существование файлов, названных в плане тестов:
+   `test/wall-thickness.test.mjs`, `test/plan-geometry-preflight.test.mjs`,
+   `scripts/model-invariants.mjs` — существуют; `demo/smoke_wall_union_isolation.mjs`
+   — не существует (ожидаемо, создаётся в разработке).
+8. Проверено использование `evenodd` fill-rule в `houseplan-card.ts`/
+   `space-render.ts`/`physical-geometry.ts` — утверждение §5.7 о риске
+   схлопывания coincident-компонента подтверждено существующим кодом и
+   существующим комментарием в `physical-geometry.ts` про запрет
+   конкатенации противоположно ориентированных rings в один evenodd path.
+9. Сверены со смежными принятыми ТЗ той же подсистемы —
+   `docs/specs/253-resize-wall-thickness.md` и
+   `docs/specs/275-multiwall-strip-containment.md` — на состав обязательных
+   разделов по PROCESS.md §7.1.
+
+## Что проверено и корректно
+
+- **Технический диагноз не догадка, а прочитанный код.** Утверждение §2/§12.4
+  "цикл `extraBodies` в `wallBodiesGeometry()` не имеет per-piece try/catch,
+  в отличие от `roomRings` и `wallEdgeBodies`, и исключение улетает во внешний
+  catch, обнуляя весь результат" — подтверждено построчно
+  (`src/wall-thickness.ts:2815-2823` и `:2830-2840` оба оборачивают тело цикла
+  в `try/catch`, `:2872-2875` — нет). Это именно то различие, которое делает
+  ТЗ проверяемым, а не спекулятивным.
+- **`roomGeom` действительно исключает extras** (`:2851` вычисляется до
+  `for (const extra of extraBodies)` на `:2872`) — соответствует AC5
+  ("room area не включает extras").
+- Границы с #197/#199 объяснены и согласуются с фактическим текстом закрытых
+  issue: #197 — единичный virtual-junction patch, здесь другой узел отказа;
+  #199 — preflight Optimize, здесь — barrier для обычных commit-путей, которых
+  у #199 не было. Автор дополнительно подтвердил во втором комментарии issue,
+  что #199 не регрессировал (`wall-null` при повторном прогоне на точном
+  `payload.config`), и перенёс эту находку в границу задачи (§3.5) — хорошая
+  практика: уточнение доказательной базы отражено в ТЗ, а не осталось только
+  в комментарии.
+- Типизированный контракт §4 (`ok`/`degraded-extra`/`failed-core`/
+  `not-applicable`) и разделение strict/render-safe режимов дают однозначный,
+  проверяемый словарь для AC1–AC3, AC6.
+- Canonical-consumers claim (§6) подтверждён кодом: `houseplan-card.ts`,
+  `space-render.ts` и `plan-geometry-preflight.ts` — все три реальных
+  потребителя `wallBodiesGeometry`/`wallBodiesUnionPath` в `src/`.
+- Явно помечены принятые технические предположения (§18) с правом ревьюера их
+  оспорить — соответствует PROCESS.md §7.1: имена classes/reasons, бюджет
+  bounded-toast, отсутствие отдельного repair UI, порядок merge #276→#277→#278.
+  Открытых продуктовых вопросов действительно нет ни одного размытого места,
+  которое требовало бы решения владельца, а не инженерного суждения.
+- AC1–AC15 в целом привязаны к конкретному способу доказательства (unit,
+  property/permutation matrix, production-bundle smoke, mutation, benchmark) —
+  ни один AC не оставлен без названного способа проверки.
+- Non-scope (§11) корректно выносит #276 (устранение конкретного redundant
+  partition) и #277 (safe Resize) за периметр, не пытаясь тихо взять на себя их
+  скоуп.
+
+## Находки
+
+### Medium (в скоупе, блокирует зелёный вердикт)
+
+**M1 — отсутствует обязательный раздел «Риски».**
+PROCESS.md §7.1 перечисляет обязательные разделы ТЗ: «…критерии приёмки
+AC1…ACn с указанием доказательства · план автотестов · **риски** · откат ·
+release-артефакты». В `docs/specs/278-wall-union-isolation.md` слово «риск»
+встречается один раз — как число в общей оценке заголовка
+(«риск 9/10», строка 7) — и это всё; отдельного раздела с анализом рисков и
+мер нет вообще (структура документа: §1 Сценарий → …→ §13 Производительность →
+§14 Touch → §15 AC → §16 План тестов → §17 Release-артефакты и rollback → §18
+Принятые предположения — раздела «Риски» между планом тестов и rollback, где
+он стоит в соседних ТЗ этой же подсистемы, нет).
+
+Это не формальность: два родственных ТЗ той же геометрической подсистемы,
+написанные тем же автором, содержат именно такой раздел —
+`docs/specs/253-resize-wall-thickness.md` (§14 «Риски и защита») и
+`docs/specs/275-multiwall-strip-containment.md` (§11 «Риски и меры»). #278 —
+не менее рискованная задача (та же авторская оценка «риск 9/10», плюс явная
+кросс-issue зависимость), и именно здесь отсутствие анализа заметно:
+
+- §18.4 фиксирует предположение о порядке merge #276→#277→#278, но на момент
+  этого ревью **#276 и #277 оба ещё в `S4-spec-review`**, то есть все три
+  задачи идут параллельно, а не последовательно, как предполагает пункт.
+  Что произойдёт, если #278 будет реализован и смержен раньше #277 (временный
+  adapter, который #278 должен удалить, ещё не существует), или #276 не будет
+  готов к моменту, когда #278 нужен для AC6 (barrier зелёный «после valid #276
+  reconciliation») — не разобрано ни как риск, ни как assumption с планом на
+  случай отклонения от предполагаемого порядка.
+- Риск деградации UX для реальных легаси-планов (после этой задачи план с
+  повреждённым extra навсегда останется в `degraded-extra` и будет виден как
+  изолированный компонент до ручного #276-ремонта) заявлен как продуктовый
+  контракт (§18.5), но не оценён как риск с мерой (например: сколько таких
+  планов может существовать в проде, что видит пользователь до вмешательства).
+- Риск того, что «reusable strict geometry transaction barrier» (§7) станет
+  тем самым единственным источником правды, но при интеграции с уже
+  реализуемым #277 останется временный дублирующий adapter дольше одного
+  цикла — упомянут как admission (§18.4 «удаляет временный adapter»), но не
+  как риск с последствием, если удаление не произойдёт до релиза.
+
+**Почему Medium, а не High:** ни один AC не становится невыполнимым или
+непроверяемым из-за этого пропуска — сам контракт §3–§9 самодостаточен и
+проверяем без раздела «Риски». Это дефект полноты документа, а не дефект
+проверяемости критериев, поэтому не блокирует по критерию «ТЗ невыполнимо»,
+но прямо нарушает обязательный список разделов §7.1 → жёлтый вердикт, правка
+остаётся в этом ТЗ (Medium в скоупе, решение владельца 2026-08-19 / #202: не
+заводится отдельный issue).
+
+**Как закрыть:** добавить раздел «Риски» (после §13/§14 или перед §17,
+по образцу 253/275), явно разобрав минимум три пункта выше — параллельность
+#276/#277/#278, судьбу legacy `degraded-extra` планов без #276-ремонта и
+жизненный цикл временного adapter #277.
+
+## Что не проверял
+
+- Реализуемость `union()`/`polyclip-ts` конкретно для сценария «сохранить
+  primary geometry при исключении на N-м extra и продолжить с N+1» —
+  прочитано только текущее поведение (общий catch), не пробовалась
+  экспериментальная реализация; на этапе spec это ожидаемо не требуется.
+- Полный список файлов/модулей, которые будут затронуты в реализации — не
+  входит в обязательные разделы ТЗ §7.1 (это чек-лист DoR §2.5), поэтому не
+  оценивался как находка.
+- Golden/perf/smoke наборы не запускались — код не менялся, гейты §8 к spec-этапу
+  неприменимы; таблица гейтов не прикладывается по той же причине.
+- Не проверялось содержимое приложенного `33.json` (не публикуется по
+  политике приватности, согласно самому issue) — доверился описанию автора и
+  перепроверил через код, что описанный механизм отказа (`extraBodies` без
+  try/catch) реален и достаточен для воспроизведения заявленного симптома.
+
+## Вердикт
+
+Жёлтый. Единственная находка — M1, в скоупе задачи, не заводится отдельным
+issue. После добавления раздела «Риски» документ готов к повторному циклу.