docs: review document for #229

Issue: #229
User-Visible: no

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

@@ -0,0 +1,246 @@
+# SPEC-REVIEW-229-r1
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/229
+- **ТЗ:** `docs/specs/229-merge-collinear-partitions.md`
+- **Ветка / SHA:** `issue/229-merge-collinear-partitions` @ `81210f7`
+- **Этап:** spec (PROCESS.md §2.4) · трек: обычный (не `small`)
+- **Заход:** r1 · блокирующих циклов израсходовано 0/4 до этого вердикта
+- **Ревьюер:** Claude, роль «ревьюер ТЗ» (отдельная от роли аналитика/автора)
+
+## Скоуп
+
+Оценивалось только ТЗ issue #229 — сращивание коллинеарных независимых
+перегородок одинаковой толщины при завершении цепочки стен, плюс аналогичное
+слияние в «Оптимизировать планы» для уже нарисованных планов. Код не менялся и
+не оценивался (правильный этап — S4, а не S7); объектом ревью является
+исполнимость и проверяемость ТЗ, а не корректность отсутствующей реализации.
+
+## Как проверялось
+
+1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` (полностью, включая §2.10,
+   §4, §7.1, §8) — это первый цикл (r1), правило «по дельте» не применяется.
+2. Прочитано тело issue #229 и оба комментария (аналитика → `S3-spec`, автор ТЗ
+   → `S4-spec-review`), продуктовые решения владельца от 2026-08-21 (§4 ТЗ).
+3. Прочитан сам файл ТЗ целиком (`docs/specs/229-merge-collinear-partitions.md`).
+4. Проверены построчно все фактические ссылки на код, которые ТЗ приводит как
+   подтверждение причины и контракта: `houseplan-card.ts:6538` (`_finishWallChain`,
+   реальный номер строки метода — 6538, не 6558 как в теле issue; в самом файле
+   ТЗ строка не называется, см. находку L1), `wall-thickness.ts:1258`
+   (`normalizeWallIntervals`), `plan-optimizer.ts:402-530` (`wallsMerged`,
+   `spansMerged`, отчёт), `align-grid.ts:262` (только snap независимых
+   перегородок, без слияния), `partition-openings.ts:43-90`
+   (`resolvePartitionOpening`, `host.t`, `missing-partition`),
+   `wall-thickness.ts:743` и `:2256` (два разных допуска коллинеарности/смежности).
+   Все ссылки, кроме одной, точны.
+5. Прочитан canonical `docs/WALL-THICKNESS.md` §9 (Independent partitions,
+   drafts and columns) и `docs/CONFIG-COMPATIBILITY.md` (Independent-wall opening
+   host, #132) — они определяют существующий контракт хостов проёмов, с которым
+   должно согласовываться §8.4 ТЗ.
+6. Прочитан прецедентный код записи, который уже пересчитывает материализованную
+   проекцию проёма при геометрическом изменении хозяина: перетаскивание
+   перегородки (`houseplan-card.ts:7809-7826`) и прямое редактирование проёма
+   (`:11996-12007`) — оба вызывают `materializePartitionOpening` после
+   `resolvePartitionOpeningCompat`.
+7. Проверена корректность строки `Touch editor: not exposed` по
+   `docs/TOUCH-SUPPORT.md` (документационное правило, строка 153) — значение из
+   допустимого набора, обоснование («не добавляет жестов») соответствует тому, что
+   вся фича живёт только на десктопном приёме цепочки в режиме «Стены».
+8. Проверена согласованность с `docs/USER-GUIDE.ru.md` (раздел 8, «Комнаты и
+   стены») — существующее описание поведения («сегменты сохранятся обычными
+   независимыми стенами») не противоречит будущему («меньше независимых стен»),
+   план обновления документации (§15 ТЗ, одна строка) достаточен.
+9. Код не запускался, гейты не гонялись — реализации нет, оценивать нечего;
+   на этапе spec-review это не требуется (PROCESS.md §2.4/§8 говорят о коде).
+
+## Находки
+
+### Medium-1 — момент слияния при завершении цепочки шире, чем решение владельца о старых планах
+
+**Файл:** `docs/specs/229-merge-collinear-partitions.md`, §17 «Принятые
+предположения», п.4.
+
+**Формулировка ТЗ:** «Слияние при завершении цепочки применяется ко всему
+пространству, а не только к новым записям: цепочка могла примкнуть к
+нарисованному ранее отрезку, и шов на стыке — тот же самый случай.»
+
+**Проблема.** Владелец явно разделил два случая (§4 ТЗ, решения 2026-08-21):
+(1) слияние новой цепочки — сразу; (2) слияние **уже нарисованных планов** —
+только явным действием «Оптимизировать планы», с отчётом и отменой. Пункт 4
+раздела «Принятые предположения» помечен как техническое решение («менять
+свободно»), но по факту расширяет действие правила 1 на данные, подпадающие под
+правило 2: он не ограничивает слияние компонентой, связанной с только что
+нарисованной цепочкой, а сканирует **всё пространство**.
+
+**Сценарий воспроизведения.** В пространстве уже лежат 9 перегородок из экспорта
+владельца (#228, `1.json`), среди них коллинеарная пара `#0`/`#4` — то есть ровно
+тот случай, который ТЗ само приводит как мотивирующий пример и который решение
+владельца п.2 явно откладывает до «Оптимизировать планы». Администратор рисует
+новую, никак не связанную стену в другом углу того же пространства и завершает
+цепочку. По формулировке п.4 слияние пройдёт по всему пространству и сотрёт пару
+`#0`/`#4` тоже — без отчёта и без выделенной отмены, которые п.2 обещает именно
+для этого случая. Единственная защита — общая история Ctrl+Z, которая отменит
+весь акт завершения цепочки (включая саму новую стену), а не только неожиданный
+побочный эффект.
+
+**Почему это находка, а не варьируемая деталь реализации.** Оправдание в тексте
+(«цепочка могла примкнуть к нарисованному ранее отрезку») описывает узкий случай
+— новый сегмент касается старого и продолжает его. Формулировка реализации шире
+оправдания: она не привязана к связности с новой цепочкой. Это меняет объём
+видимых изменений (§7.1 PROCESS.md: «какой объём видимых изменений входит в этот
+issue» — ровно тот класс, который решает владелец), а не техническую деталь типа
+имени модуля или выбора выжившего `id`.
+
+**Что нужно.** Одно из двух: (a) сузить п.4 до компоненты связности, содержащей
+концы только что завершённой цепочки (транзитивно через общие концы), тогда
+поведение полностью укладывается в решение владельца п.1 и вопрос снимается без
+эскалации; (b) если задуман действительно широкий охват (он и полезнее — план
+чище), явно спросить владельца одним вопросом с предложенным дефолтом («слияние
+при завершении цепочки задевает не только новый сегмент, а всё пространство —
+ок?»), как требует §7.1.
+
+### Medium-2 — допуск «есть причина оставить узел» не определён для трёх из четырёх причин
+
+**Файл:** `docs/specs/229-merge-collinear-partitions.md`, §8.2 и §8.3.
+
+**Проблема.** §8.3 заявляет «один источник истины» для допусков, но называет
+только два: `EPS_ANGLE` (коллинеарность) и `EPS_JOIN` (совпадение концов двух
+*перегородок*). §8.2 перечисляет четыре причины оставить узел: третья
+перегородка, **ребро комнаты**, **колонна**, **конец черновика**. Для первой
+причины допуск неявно тот же `EPS_JOIN` (стык двух перегородок и есть предмет
+§8.1.3). Для трёх остальных ТЗ не говорит, каким допуском и по какому
+геометрическому критерию («точка на точке» vs «точка на отрезке» для ребра
+комнаты) устанавливается совпадение.
+
+**Почему это блокирует однозначность AC.** AC2 требует четыре отдельных unit-теста
+(«ребро комнаты», «колонна», «конец черновика», третья перегородка) и заявлен как
+доказываемый `unit`, но без зафиксированного критерия автор теста выбирает допуск
+по своему усмотрению — один из тех самых «плавающих» допусков, от которых §8.3
+явно старается защититься («иначе на границе слияние станет непредсказуемым»).
+Конкретный риск: если для колонны или ребра комнаты возьмут допуск строже
+`EPS_JOIN`, легитимный ULP-шум (риск, явно описанный в самом ТЗ со ссылкой на
+#218/#223/#224) заставит узел остаться там, где физически есть только шум
+координат, а не колонна; если допуск шире `EPS_JOIN`, слияние может пройти сквозь
+примыкание, которое реально существует, но чуть-чуть не долетело до совпадения
+концов — прямое нарушение риска №3 из §11.
+
+**Что нужно.** Одна фраза в §8.2 или §8.3: причины «ребро комнаты», «колонна»,
+«конец черновика» проверяются тем же `EPS_JOIN` (точка-в-допуске к общему концу
+перегородки), либо явно называется другой, но такой же единый допуск с
+обоснованием, почему он должен отличаться.
+
+### Medium-3 — §8.4 не упоминает материализованную legacy-проекцию проёма (`x/y/angle`)
+
+**Файл:** `docs/specs/229-merge-collinear-partitions.md`, §8.4, §9, AC3.
+
+**Проблема.** `docs/CONFIG-COMPATIBILITY.md` («Independent-wall opening host,
+#132») документирует контракт: у хостованного проёма легacy-поля `x/y/angle`
+— это **материализованная проекция** для старых фронтендов, которая должна
+оставаться синхронной с `host.id`/`host.t`. Это не гипотеза, а действующий
+прецедент в коде: перетаскивание перегородки (`houseplan-card.ts:7809-7826`) и
+прямое редактирование проёма (`:11996-12007`) оба вызывают
+`resolvePartitionOpeningCompat` → `materializePartitionOpening` сразу после
+изменения геометрии хозяина, специально чтобы не оставить проекцию устаревшей.
+
+§8.4 ТЗ описывает пересчёт `host.t` и переписывание `host.id`, но ни словом не
+упоминает пересчёт `x/y/angle`. §9 («Данные...») утверждает «формат перегородки
+не меняется; меняется их количество и `host.id` части проёмов» — и тоже не
+называет legacy-проекцию. AC3 проверяет «координаты центра проёма в единицах
+плана» и `host.t ∈ [0,1]` — то есть **резолвленную**, а не материализованную
+позицию; тест по AC3 в его текущей формулировке может стать зелёным при устаревших
+`x/y/angle`.
+
+**Сценарий воспроизведения.** Слияние пересчитывает `host.t` двери на новую длину
+хозяина, но материализованные `x/y/angle` остаются от старой перегородки.
+Дверь физически на месте для текущего фронтенда (он всегда резолвит через
+`host`), но откат на старый фронтенд (или любой код, который по прецеденту
+CONFIG-COMPATIBILITY.md вправе читать `x/y/angle` напрямую) увидит дверь там, где
+она была до слияния — ровно тот класс проблемы, для которого прецедентный код
+уже существует и который #132 называет явно.
+
+**Что нужно.** Добавить в §8.4 шаг «для каждого пересчитанного проёма
+материализовать `x/y/angle` тем же вызовом, что использует перетаскивание
+перегородки», и явно упомянуть это в §9. Не обязательно расширять сам AC3 (там
+всё ещё резолвленная позиция — правильный инвариант), но стоит добавить это
+как отдельную строку либо в AC3, либо отдельным AC, иначе ревьюер кода не будет
+знать, что искать в реализации.
+
+## Что проверено и корректно
+
+- Все требуемые по §7.1 разделы по существу присутствуют: сценарий и персона
+  (§1), видимое изменение (§2), причина/проблема (§3), скоуп/не-скоуп (§6–7),
+  контракт поведения (§8), данные/i18n/a11y (§9), performance (§10), риски
+  (§11), AC1…AC9 с доказательством (§12), план автотестов (§13), мутационный
+  гейт (§14), release-артефакты (§15), откат (§16), явный блок предположений
+  (§17). Заголовки не совпадают буквально с шаблоном, но это стандартная для
+  репозитория практика (см. `223-optimize-coordinate-canonicalization.md`,
+  `226-entity-parent-dedup.md`) — не находка.
+- Продуктовая рамка (§1–2) отвечает на оба обязательных вопроса — какая персона
+  на какой поверхности и что человек видит без терминов реализации.
+- Заявленная причина и асимметрия (§3) — не догадка: обе половины (наличие
+  `normalizeWallIntervals`/`wallsMerged` для стен комнат и его отсутствие для
+  независимых перегородок) подтверждены чтением `wall-thickness.ts`,
+  `plan-optimizer.ts`, `align-grid.ts`; номера строк точны, кроме одной (см.
+  выше — L1, не блокирует, разобрано отдельно как низкая находка).
+- Три продуктовых решения владельца (§4) взяты из issue буквально, не
+  переинтерпретированы — кроме расширения решения №2 через техническое
+  предположение №4 (Medium-1 выше).
+- AC1, AC4, AC5, AC6, AC7, AC8, AC9 однозначны, проверяемы и указывают способ
+  доказательства; AC3 явно требует «тест красный до реализации §8.4» — то самое
+  требование «тест должен уметь падать», зафиксированное заранее, а не постфактум.
+- Допуски (§8.3) для собственно решения «сращивать/не сращивать» две
+  перегородки заданы через единые именованные константы, выраженные в долях шага
+  сетки (а не абсолютно) — правильная реакция на разброс `cell_cm` 1–25 (#230) и
+  на ULP-шум (#218/#223/#224).
+- Границы применения (§8.5): транзитивное слияние до стабилизации и
+  независимость от порядка обхода — оба явно проверяются отдельными AC
+  (входят в AC1/AC6), а не декларируются без доказательства.
+- Мутационный гейт (§14) покрывает по одной репрезентативной мутации на каждый
+  из четырёх названных рисков (§11) — стандартный для репозитория уровень
+  строгости, не исчерпывающий по всем под-случаям AC2, и это нормально (не
+  находка).
+- `Touch editor: not exposed` — верно выбранное значение из
+  `docs/TOUCH-SUPPORT.md` §153 при отсутствии новых жестов.
+- Не входит в задачу (§7): стены комнат, колонны, черновики контуров,
+  промахи примыкания (#228 п.2) — верно отнесены к соседним причинам/задачам, не
+  присвоены этой.
+- Release-артефакты (§15) называют оба changelog, обновление
+  `docs/USER-GUIDE.ru.md` и явно отмечают отсутствие влияния на golden — сверено
+  с текущим содержанием гайда (раздел 8), противоречия нет.
+
+## Мелкая находка (Low, снимаю с записью, не блокирует)
+
+**L1.** Тело issue цитирует `_finishWallChain` как `houseplan-card.ts:6558`; в
+файле метод объявлен на `6538`. Разница в 20 строк — комментарии/боевой код
+сдвинулись между моментом анализа и моментом чтения. Не влияет на понимание или
+проверяемость ТЗ (сам файл ТЗ конкретную строку не называет), исполнитель найдёт
+метод по имени. Снимаю без действия.
+
+## Продуктовые вопросы владельцу
+
+Формально ни один из вопросов выше не обязан идти владельцу — оба фиксируемых
+пункта (Medium-1, Medium-2) решаются техническим сужением формулировки в ТЗ, без
+изменения того, что видит пользователь. Единственная точка, где владелец мог бы
+захотеть высказаться, — Medium-1, если авторы предпочтут вариант (b) (широкий
+охват всего пространства) вместо сужения (a); в этом случае вопрос будет
+сформулирован автором ТЗ во втором заходе с предложенным дефолтом, как того
+требует §7.1. Ревьюер вопросов владельцу сейчас не задаёт (согласно инструкции —
+такие вопросы снимаются и решаются по существу в вердикте).
+
+## Чего не проверял
+
+- Реализацию — её не существует на этом этапе; код-ревью будет отдельной сессией
+  после `S5-ready` → `S6-in-progress` → `S7-code-review`.
+- Содержимое `1.json` из #228 (конкретные координаты пары `#0`/`#4`) — принято на
+  веру как результат анализа предыдущего этапа (аналитика уже это подтвердила
+  чтением экспорта), пересчёт координат вручную не входит в задачу ревью ТЗ.
+- `docs/specs/README.md` регистрацию — автор заявил, что запись добавлена;
+  не влияет на вердикт содержания ТЗ, отдельно не сверял.
+- Существующие смоки (`smoke_subarea` и смоки рисования, упомянутые в §13) —
+  их код не читал; ТЗ верно указывает, что они должны быть прогнаны без правок
+  на этапе реализации, это забота код-ревью, не ТЗ-ревью.
+
+## Вердикт
+
+Медиум-находки все в скоупе задачи и чинятся правкой ТЗ, без блокирующих (High)
+находок. Итог — жёлтый: возврат автору на правку ТЗ, фикс проходит повторный
+цикл ревью (лимит для обычного трека — 4, израсходовано после этого раунда — 1).