docs: review document for #229

Issue: #229
User-Visible: no

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

@@ -0,0 +1,196 @@
+# SPEC-REVIEW-229-r2
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/229
+- **ТЗ:** `docs/specs/229-merge-collinear-partitions.md`
+- **SHA на момент этого ревью:** `ecc3d6a88ba259d3446f77ab3a60170337bc068f`
+  (коммит `docs: close M1, M2 and M3 from the spec review of #229`)
+- **Этап:** spec (PROCESS.md §2.4) · трек: обычный (не `small`)
+- **Заход:** r2 · блокирующих циклов израсходовано 1/4 до этого вердикта
+- **Предыдущий раунд:** `docs/reviews/SPEC-REVIEW-229-r1.md`, вердикт жёлтый на
+  SHA `81210f7` (ветка `issue/229-merge-collinear-partitions`)
+- **Ревьюер:** Claude, роль «ревьюер ТЗ»
+
+## Скоуп этого раунда
+
+Дельта — один коммит `ecc3d6a`, который правит только
+`docs/specs/229-merge-collinear-partitions.md` (62 добавленных / 15 удалённых
+строк) в ответ на три Medium-находки r1 (M1, M2, M3). Правка локальна: не
+ребейз, не смена контракта, не новая подсистема, объём дельты (одна секция
+плюс точечные правки соседних) заметно меньше исходного ТЗ. По правилу §2.9/§2.10
+разбор идёт по дельте: полная повторная проверка — только для того, что дельта
+задевает (AC2, AC3, AC8, §8.2, §8.4, §8.6, §14, §17.4); остальное наследуется из
+r1 без повторного прогона.
+
+Тело issue #229 не менялось между раундами (сравнил текущее содержимое issue с
+цитатами в r1 — совпадает буквально), поэтому продуктовая рамка (§1–§4 ТЗ) не
+пересматривалась заново.
+
+## Как проверялось
+
+1. Прочитан вердикт r1 (`docs/reviews/SPEC-REVIEW-229-r1.md`) — три Medium
+   (M1, M2, M3) и одна снятая Low (L1).
+2. `git diff 81210f7..HEAD -- docs/specs/229-merge-collinear-partitions.md` —
+   единственный файл дельты; построчно сверен с формулировками M1/M2/M3.
+3. Для каждой из трёх находок проверено, чем именно она закрыта (не заявлением
+   коммита, а текстом ТЗ) — см. таблицу «Закрытие раунда r1».
+4. Так как правка M2 вводит новый нормативный текст («ребро комнаты — ближайшая
+   вершина полигона»), он сверен с каноническим прецедентом продукта:
+   - `docs/specs/141-wall-junctions.md` §4.1: «Исправление охватывает точные
+     endpoint↔endpoint и endpoint↔line (**T**) соединения... между active/saved
+     draft, partition и **готовой комнатной стеной**» — T-стык партиции с
+     ЛИНИЕЙ комнатной стены (не только с её вершиной) прямо назван как канон
+     этого продукта, а не гипотеза;
+   - `docs/specs/141-wall-junctions.md` §13.1, п.5 — regression-тест явно требует
+     «T partition→partition, draft→partition **и partition→solid room wall**»;
+   - `src/plan-snap-overlay.ts:313` вызывает `distToSegment(...)` от
+     `roomEdges` (`src/logic.ts`) — существующий код-примитив для «точка близко
+     к стороне комнаты» уже оперирует расстоянием до **отрезка**, а не только до
+     вершины.
+5. Проверены построчные ссылки на код, добавленные/исправленные дельтой:
+   `houseplan-card.ts:6538` (`_finishWallChain`, теперь верно и в §3, и в §6 —
+   было `:6558` в r1), `:7824` и `:12007` (`materializePartitionOpening`,
+   оба вызова подтверждены чтением файла). Все точны.
+6. Заново прочитаны AC2, AC3, AC8, §14 (мутационный гейт) — они напрямую задеты
+   дельтой. AC1, AC4, AC5, AC6, AC7, AC9 дельта не касается — наследуются из r1.
+7. Код не запускался, гейты не гонялись — на этапе spec-review реализации нет,
+   оценивать нечего (как и в r1).
+
+## Закрытие раунда r1
+
+| Находка r1 | Чем закрыта | Где это видно |
+|---|---|---|
+| **M1** — слияние при завершении цепочки шире решения владельца о старых планах | Добавлен §8.6 «Что именно сращивается при завершении цепочки»: скоуп сужен до компоненты связности по общим концам, содержащей хотя бы один новый сегмент цепочки; явно сказано «не всё пространство» с тем же примером `#0`/`#4` из #228, который в r1 был контрпримером | `docs/specs/229-merge-collinear-partitions.md` §8.6 (новая секция); AC8 дополнен строкой про перегородки вне компоненты связности; в мутационный гейт (§14) добавлена строка `chain-merge-sweeps-whole-space` → guard «юнит AC8»; §17.4 переписан и явно ссылается на «находку M1 ревью r1» |
+| **M2** — допуск «есть причина оставить узел» не определён для трёх из четырёх причин | В §8.2 добавлен абзац: все четыре причины проверяются одним `EPS_JOIN`; для каждой названа точка стыка (третья перегородка — её конец; ребро комнаты — «ближайшая вершина полигона»; колонна — центр; черновик — конец сохранённого контура) | `docs/specs/229-merge-collinear-partitions.md` §8.2, абзац «Все четыре причины проверяются одним допуском `EPS_JOIN`». **Закрыта частично — см. Medium-1 ниже**: сам факт единого допуска и явного критерия для всех четырёх причин снимает исходную находку (было — критерий не назван вовсе, стало — назван), но выбранный критерий для «ребра комнаты» противоречит канону #141 |
+| **M3** — §8.4 не упоминает материализованную legacy-проекцию `x/y/angle` | В §8.4 добавлен шаг 4 «пересчитывается материализованная проекция `x/y/angle`», сослан на `#132`/`CONFIG-COMPATIBILITY.md` и на тот же прецедентный код (`:7824`, `:12007`), что r1 приводил как образец. Инвариант и AC3 переписаны: явно требуют совпадения «и в резолвленном виде, и в материализованной проекции», AC3 теперь явно «остаётся красным, если пересчитан только `host.t`» | `docs/specs/229-merge-collinear-partitions.md` §8.4 п.4, §12 AC3, §14 добавлена строка `partition-merge-skips-materialization` → guard «юнит AC3» |
+| **L1** (снята в r1 без действия) — расхождение номера строки `_finishWallChain` `:6558` vs `:6538` | Попутно исправлено и в §3, и в §6, хотя это не требовалось (была снята без действия) | `docs/specs/229-merge-collinear-partitions.md` §3, §6 |
+
+## Находки этого раунда
+
+### Medium-1 — критерий «ребро комнаты» для допуска `EPS_JOIN` (правка M2) сам по себе противоречит канону T-стыков продукта
+
+**Файл:** `docs/specs/229-merge-collinear-partitions.md`, §8.2 (новый абзац,
+внесён дельтой r2).
+
+**Формулировка ТЗ (текущая):** «Точка стыка берётся у ребра комнаты — ближайшая
+вершина полигона, у колонны — её центр, у черновика — конец сохранённого
+контура.»
+
+**Проблема.** Эта фраза отвечает на вопрос, поднятый находкой M2 r1 («точка на
+точке» vs «точка на отрезке» для ребра комнаты), но отвечает на него
+геометрически неверно относительно уже принятого канона продукта. Слово
+«ребро» здесь означает сторону комнатного полигона — отрезок, а не точку. ТЗ же
+проверяет близость конца перегородки к **ближайшей вершине** этого полигона, то
+есть только к его углам. Между тем `docs/specs/141-wall-junctions.md` (принятое
+и не отменённое ТЗ этого же репозитория, канон стыков стен) §4.1 прямо называет
+«endpoint↔line (T)» соединение партиции с **готовой комнатной стеной** одним из
+двух типов стыков, которые продукт обязан поддерживать наравне с
+endpoint↔endpoint, а §13.1 п.5 требует отдельного regression-теста именно на
+«partition→solid room wall». T-стык по определению происходит **вдоль стороны**
+комнаты, а не в её углу — иначе это было бы endpoint↔endpoint, а не
+endpoint↔line, и отдельной категории в #141 не потребовалось бы. Существующий
+код-примитив `distToSegment`/`roomEdges` (`src/plan-snap-overlay.ts:313`,
+`src/logic.ts`), уже используемый в редакторе для похожей задачи «точка близко
+к стороне комнаты», тоже оперирует расстоянием до отрезка, а не до вершины —
+то есть естественный примитив для этой проверки в кодовой базе уже есть, и он не
+«ближайшая вершина».
+
+**Почему это блокирует однозначность AC2, а не деталь реализации.** AC2 требует
+unit-тест именно на случай «ребро комнаты» как причину не сращивать. По
+буквальной формулировке текущего §8.2 автор теста обязан проверять близость к
+вершине полигона, а не к стороне — то есть тест для типичного T-стыка
+(перегородка примыкает к середине комнатной стены, что и есть основной мотив
+всей категории «endpoint↔line» из #141) при точном следовании ТЗ **не найдёт
+причину оставить узел**, и AC2 в этой части будет доказывать не то поведение,
+которое нужно.
+
+**Сценарий воспроизведения.** Комнатная стена — длинная сторона (10 м), её углы
+далеко. Администратор дорисовывает независимую перегородку, конец которой
+касается этой стены точно посередине (T-стык, ровно случай из #141 §4.1/§13.1
+п.5), и на этом же конце сходится вторая независимая перегородка, коллинеарная
+первой и той же толщины — классический кейс §8.1. По текущему §8.2 расстояние
+от точки стыка до **ближайшей вершины** полигона (≈5 м) заведомо больше
+`EPS_JOIN` (доли шага сетки), значит «причины оставить узел» формально нет, и
+слияние по §8.1 проходит — стирая узел ровно там, где комнатная стена
+T-образно примыкает к перегородке. Это прямое попадание в риск №3 из §11
+(«Потеря узла, который был нужен») — риск, который сама находка M2 в r1 и
+призывала закрыть однозначным критерием, но выбранный критерий закрывает его
+только для углового случая, оставляя открытым основной, T-образный.
+
+**Что нужно.** Заменить «ближайшая вершина полигона» на «ближайшая точка на
+любой стороне (ребре) полигона» — то есть на point-to-segment расстояние по
+всем сторонам комнаты (`roomEdges` + `distToSegment`, тот же примитив, что уже
+используется в `plan-snap-overlay.ts`), а не point-to-vertex. Это одна фраза в
+§8.2, не расширяет и не сужает продуктовое решение владельца — чисто техническое
+уточнение критерия, которое сам ревьюер может закрыть по инструкции ревью (не
+продуктовый вопрос: пользователь не видит разницы между «стык у угла» и «стык
+посередине стены» — оба должны одинаково сохранять узел).
+
+## Унаследовано из r1 (без повторной проверки)
+
+Документ: `docs/reviews/SPEC-REVIEW-229-r1.md`, SHA `81210f7`. Принято на веру,
+так как дельта r2 эти места не касается:
+
+- Продуктовая рамка §1–§4 (персона, сценарий, три решения владельца
+  2026-08-21) — не менялась, тело issue не менялось.
+- §7 «Не входит в задачу», §9 (данные/i18n/a11y), §10 (performance), §16
+  (откат) — не менялись дельтой.
+- AC1, AC4, AC5, AC6, AC7, AC9 — их формулировки и способ доказательства не
+  затронуты правками M1/M2/M3, признаны однозначными и проверяемыми в r1.
+- Соответствие обязательным разделам §7.1 PROCESS.md, отсутствие непомеченных
+  догадок за пределами §8.2/§8.4/§8.6 (проверено в r1 постранично) и
+  корректность `Touch editor: not exposed` по `docs/TOUCH-SUPPORT.md` §153 — не
+  переоценивались, дельта этих разделов не касается.
+- Строки кода `wall-thickness.ts:1258`, `plan-optimizer.ts:402-530`,
+  `align-grid.ts:262`, `partition-openings.ts:43-90`, `wall-thickness.ts:743`,
+  `:2256`, проверенные в r1, дельтой не менялись — не перечитывались повторно.
+- Согласованность с `docs/USER-GUIDE.ru.md` §8 — не переоценивалась, план
+  документации (§15 ТЗ) дельтой не менялся.
+
+## Что проверено и корректно (дельта r2)
+
+- Все три Medium-находки r1 закрыты по существу текстом ТЗ, а не заявлением
+  коммита — см. таблицу выше. M1 и M3 закрыты полностью и корректно.
+- M2 закрыта частично: сам факт объявления единого допуска и явного критерия
+  для всех четырёх причин — правильный ответ на форму находки M2 («допуск не
+  назван»); ошибка — в конкретном выборе критерия для одной из четырёх причин
+  (см. Medium-1 этого раунда).
+- Новые строки мутационного гейта (`partition-merge-skips-materialization` →
+  AC3, `chain-merge-sweeps-whole-space` → AC8) корректно привязаны к тем AC,
+  которые проверяют соответствующее поведение.
+- AC3 переписан так, что явно требует падения теста при пересчёте только
+  `host.t` без материализации проекции — сохраняет дисциплину «тест должен
+  уметь падать», зафиксированную в r1.
+- AC8 корректно расширен пунктом про перегородки вне компоненты связности —
+  соответствует новому §8.6.
+- §17.4 честно помечен как «Ревизия r2» со ссылкой на находку M1 r1 — не
+  выдаёт правку за исходное решение.
+- Построчные ссылки на код, тронутые дельтой (`:6538`, `:7824`, `:12007`),
+  точны на текущем SHA.
+
+## Чего не проверял
+
+- Продуктовую рамку и решения владельца §1–§4 — не менялись, наследуются из r1
+  (см. выше).
+- Реализацию — её не существует на этом этапе.
+- Существующие смоки и `docs/specs/README.md` — как и в r1, вне зоны этого
+  ревью и не затронуты дельтой.
+- Значения `EPS_ANGLE`/`EPS_JOIN` как конкретные числа — ТЗ намеренно не
+  фиксирует их (принятое предположение §17.3), это не находка.
+- Критерий «колонна — её центр»: не нашёл прямого прецедента (существующего
+  кода снаппинга партиции к поверхности колонны, а не к центру), но и прямого
+  противоречия канону — данные модели (`WallColumnCfg.center`) и то, что
+  `align-grid.ts` снаппит и центр колонны, и концы перегородок на одну и ту же
+  сетку, делают «конец партиции == центр колонны» правдоподобной рабочей
+  конвенцией, в отличие от «ребра комнаты», где есть прямое, явное
+  противоречие канону (#141) и существующему код-примитиву. Не эскалирую как
+  находку — это осталось предположением, которое ревьюер не смог ни
+  подтвердить, ни опровергнуть уверенно; если оно неверно, всплывёт на
+  код-ревью через AC2.
+
+## Вердикт
+
+Одна Medium-находка в скоупе задачи (Medium-1), без High. Она — прямое
+следствие правки M2 этого же раунда, а не унаследованная проблема, и чинится
+одной фразой в §8.2 без изменения продуктового решения владельца. Итог —
+жёлтый: возврат автору на правку ТЗ, фикс проходит r3 (лимит для обычного
+трека — 4, израсходовано после этого раунда — 2).