docs: review document for #138

Issue: #138
User-Visible: no

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

@@ -0,0 +1,218 @@
+# Ревью ТЗ — issue #138, цикл r1
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/138
+- **ТЗ:** `docs/specs/138-adjacent-room-autoclose.md`, коммит `611c5a76f56ce83587eafbd00dbd6ad1a0f8c9d8`
+- **Этап:** spec (PROCESS.md §2.4)
+- **Вердикт:** красный · цикл r1/4 · High: 1 · Medium: 0
+
+## Скоуп ревью
+
+Прочитаны в указанном порядке: `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` (§1,
+§2, §3, §4, §7, §12), тело issue #138 и оба комментария владельца (аналитика
+Q1–Q5 с defaults, подтверждение ТЗ), `docs/USER-GUIDE.ru.md` (терминология
+диалога комнаты, кнопка «Оставить замкнутыми стенами», секции про соседние
+комнаты и общую стену), и связанный `docs/specs/137-plan-snap-overlay.md` как
+основа, на которой строится #138.
+
+Задача не помечена `small`, ТЗ корректно лежит в `docs/specs/138-adjacent-room-autoclose.md`
+и зарегистрировано в `docs/specs/README.md:49`. Формат ревью — полный документ,
+не комментарий.
+
+## Как проверялось
+
+Ревью читало не только текст ТЗ, но и код, на который оно опирается, чтобы
+отличить обоснованное техническое утверждение от догадки, выданной за факт:
+
+- `src/houseplan-card.ts`: `_markupClick()` (6454–6575), `_closeRoomContour()`
+  (6426–6452), `_resolvePlanDrawPoint()` (6011–6031), `_draftEndAt()`
+  (6580–6594) — сверка с §3 «Причина дефекта», §7.1 «Порядок разрешения
+  клика», §8 «Контракт замыкания».
+- `src/plan-snap-overlay.ts` (1–180): структура `PlanSnapSegment`,
+  `sourceKind: 'room'|'draft'|'partition'`, `cutSegments`/`roomEdges`,
+  дедупликация по `axisKey` — сверка с §7.2 «Допустимый существующий
+  интервал» и терминологией «room-owned solid interval».
+- `src/wall-thickness.ts:461` `applyWallThicknessToNewRoom()` — сверка с §9
+  (наследование толщины общей стены) и AC7.
+- `src/i18n/ru.json:9,267–269` — существующие `btn.keep_as_walls`,
+  `toast.room_overlap`, `toast.contour_min_edges`, `toast.contour_cannot_close`
+  — сверка с заявлением «новых i18n-ключей не требуется» (§4.4, §6).
+- `docs/specs/README.md` — регистрация spec-файла.
+
+Каждое утверждение ТЗ о текущем поведении, которое удалось проверить чтением
+кода, подтвердилось: терминология (`sourceKind`, `_markupClick`,
+`_closeRoomContour`, `_resolvePlanDrawPoint`, `applyWallThicknessToNewRoom`,
+«Оставить замкнутыми стенами») — не изобретена, а взята из реального кода.
+Отдельно оценивалась полнота контракта: не пропущен ли граничный случай,
+который спецификация не проговаривает и не относит явно в non-scope.
+
+## Находки
+
+### High-1 — контракт автозамыкания перехватывает и блокирует обычный второй клик по той же стене
+
+**Где:** `docs/specs/138-adjacent-room-autoclose.md` §7.1 (строки 105–119),
+§7.2 (121–139), §8.1 (155–173), §8.2 (175–186).
+
+**Формулировка:** §7.1 п.3 требует проверять на автозамыкание **любой**
+обычный клик, отличный от клика в собственную первую точку контура — без
+условия «в контуре уже есть хотя бы одно нарисованное ребро». §7.2 требует
+только геометрических условий (общий room-owned solid interval, содержит `A`
+и финальную точку, положительная длина, точки различны) — тоже без условия на
+уже накопленное число вершин. §8.1 требует **минимум три различные вершины**
+у prospective polygon `[..., P, B] + B—A`, иначе (§8.2) диалог не открывается,
+`B`/`P—B`/`B—A` не записываются, **и клик не превращается в обычное
+добавление `B`** — то есть точка не появляется в drafting вообще, только
+показывается existing validation toast.
+
+Отсюда прямое следствие: если пользователь ставит **первую** точку `A` на
+существующем углу (`this._path = [A]`, `houseplan-card.ts:6544`), а **второй**
+клик кладёт на другой конец **той же самой** прямой существующей стены (что
+является совершенно естественным способом начать соседнюю комнату — сначала
+отметить оба конца общей стены, а затем обвести остальной контур), —
+`P` на этот момент равен `A` (в пути одна точка), и prospective polygon равен
+`[A, B] + B—A`: **две** различные вершины, не три. Проверка §8.1 обязана
+провалиться, и по букве §8.2 клик обязан быть отклонён с toast, а `B` —
+**не добавлен в draft вообще**.
+
+Сегодня, до этой задачи, тот же клик работает штатно: `_markupClick()` не
+делает такой проверки и просто добавляет `B` как обычную точку контура
+(`houseplan-card.ts:6573–6574`, путь `this._path = [...this._path, pt];
+this._persistActiveDraftSegment();`). Показательно, что для симметричного
+случая — клика в собственную первую точку — система уже сегодня специально
+избегает этой ловушки: строка `houseplan-card.ts:6522`
+(`const closing = this._path.length >= 3 && this._samePt(pt, this._path[0]);`)
+трактует клик как попытку замкнуть контур **только когда уже накоплено
+достаточно вершин**; при недостатке вершин точка добавляется как обычная —
+ошибка «нужно минимум два ребра» никогда не показывается на пустом контуре.
+Новый контракт для автозамыкания (§7.1 п.3) не воспроизводит этот защитный
+гейт: он назначает автозамыкание кандидатом для проверки независимо от того,
+сколько вершин уже нарисовано, а §8.2 явно запрещает деградацию к обычному
+добавлению точки при провале.
+
+**Сценарий воспроизведения (по тексту ТЗ, до реализации — логическая проверка контракта):**
+
+1. Существует завершённая комната с прямой стеной `A—C` (общая длина, `A` и
+   `C` — её концы).
+2. Пользователь выбирает «Контур комнаты», кликает в `A` — `this._path = [A]`.
+3. Следующим кликом отмечает `C` — противоположный конец **той же** стены,
+   намереваясь провести по ней первую грань новой комнаты, а затем обвести
+   остальной периметр и замкнуть контур явным кликом в `A` (существующий,
+   неизменяемый способ, приоритет 1 из §7.1).
+4. По §7.1 п.3 клик в `C` не равен `_path[0]=A`, не является Ctrl/Cmd — значит
+   проверяется как кандидат на автозамыкание. По §7.2 `A` и `C` лежат на одном
+   `sourceKind: room` сплошном интервале, точки различны, длина положительна —
+   eligibility выполнена.
+5. По §8.1 prospective polygon `[A, C] + C—A` содержит 2 различные вершины,
+   что меньше требуемых трёх → невалиден.
+6. По §8.2 диалог не открывается, `C` не добавляется в draft, показывается
+   toast «Чтобы замкнуть контур, сначала нарисуйте минимум две грани»
+   (`toast.contour_min_edges`), а **клик не превращается в обычное добавление
+   точки**.
+7. Результат: пользователь не может продолжить рисовать — второй клик по
+   общей стене, который сегодня работает и является естественным способом
+   начать примыкающую комнату, теперь ничего не делает, кроме показа
+   сообщения об ошибке, не имеющего отношения к тому, что человек пытался
+   сделать.
+
+Это прямая регрессия текущего, работающего поведения — причём именно в той
+области (рисование вдоль общей стены соседней комнаты), которую задача #138
+должна улучшить, а не ухудшить. Ни один AC (§13) и ни один пункт плана
+unit-тестов (§14.1, пп.1–9) не покрывает этот случай: пункт 7
+(«одинаковые A/B, zero-length и current anchor не подходят») — про **совпадающие**
+точки, а не про две **различные** точки на одном интервале при пустом ранее
+контуре. Риск не упомянут и в таблице §17.
+
+**Почему это High, а не Medium:** правило §3.8 и §2.4 требует, чтобы контракт
+поведения был однозначным и не содержал воспроизводимой логической ошибки;
+здесь ошибка выводится напрямую из текста ТЗ без домысливания реализации, и
+затрагивает основной, ежедневный workflow редактора (первые же клики при
+разметке соседней комнаты — ровно сценарий из тела issue). Это не пограничный
+кейс, а типичный способ начать рисовать общую стену.
+
+**Что нужно поправить в ТЗ (не решение реализации, а контракт):**
+Автозамыкание должно проверяться только когда в контуре уже достаточно
+вершин, чтобы в принципе замкнуться (аналогично существующему гейту
+`this._path.length >= 3` для клика в первую точку — т.е. до клика в `B` уже
+должно быть отрисовано **хотя бы одно** дополнительное ребро, не лежащее
+целиком на том же интервале что и `A—B`), либо явно указать, что при провале
+именно по причине «меньше трёх вершин» (в отличие от self-intersection/overlap/
+zero-area) клик **деградирует к обычному добавлению точки**, а не блокируется
+toast'ом. Выбор между этими двумя вариантами — продуктовый (что видит
+пользователь при клике по второй точке общей стены сразу после первой):
+либо клик тихо добавляет обычную точку (как сегодня), либо показывает
+ошибку. Это ровно тот вопрос, который стоило задать владельцу вместе с Q1–Q5,
+и он остался незамеченным.
+
+## Что проверено и корректно
+
+- **Терминология и техническая база не выдуманы.** Все ссылки на
+  `_resolvePlanDrawPoint`, `_markupClick`, `_closeRoomContour`, `_draftEndAt`,
+  `_persistActiveDraftSegment`, `sourceKind: 'room'|'draft'|'partition'`,
+  `applyWallThicknessToNewRoom`, `plan-snap-overlay.ts`, кнопку «Оставить
+  замкнутыми стенами» и существующие toast-ключи — соответствуют
+  действительному коду. §3 «Причина дефекта» описывает код, каким он есть
+  сегодня, а не предположение.
+- **Обязательные разделы §7.1 PROCESS.md на месте:** сценарий/персона/
+  поверхность (§1), «что человек увидит до/после» одной фразой без терминов
+  реализации (§2), проблема (§3), scope/non-scope (§5–6), контракт поведения
+  (§7–9), UX и touch-деградация (§10), модель данных/миграция (§11),
+  AC1…AC12 с указанием способа доказательства (§13), план автотестов (§14),
+  риски (§17), откат (§18), release-артефакты (§16).
+- **i18n корректно закрыт:** новых ключей нет, переиспользуются существующие
+  `toast.contour_min_edges`, `toast.contour_cannot_close`, `toast.room_overlap`,
+  `btn.keep_as_walls` — подтверждено чтением `src/i18n/ru.json`/`en.json`.
+- **Наследование толщины общей стены (AC7, §9)** грамотно опирается на уже
+  существующий `applyWallThicknessToNewRoom()` (`wall-thickness.ts:461–484`),
+  который и сегодня пропускает интервалы с уже ненулевой толщиной (`cms[i] >
+  0`) — новый код для этого не нужен, утверждение ТЗ верно.
+- **Проёмы и cuts (§7.3, AC4)** корректно опираются на уже существующий
+  `cutSegments`/canonical cuts из #137 (`plan-snap-overlay.ts`), не вводят
+  второй resolver — соответствует Non-scope и архитектурному контракту §12.
+- **Дедупликация общей стены между двумя комнатами (§9, «не создаёт вторую
+  физическую стену»)** согласуется с найденной в `plan-snap-overlay.ts`
+  дедупликацией сегментов по `axisKey` (один `PlanSnapSegment` на общую ось
+  независимо от числа комнат-источников) — заявление обосновано, не догадка.
+- **Открытых продуктовых вопросов действительно не осталось** для того, что
+  было явно задано: Q1–Q5 из комментария аналитики и все технические defaults
+  подтверждены владельцем 2026-08-14 (issue-комментарий), ссылка есть в ТЗ
+  §4 и §19.7. «Ни одного открытого вопроса» здесь не является тревожным
+  сигналом самим по себе — вопросы были заданы и закрыты пакетом, а не
+  обойдены.
+- **Release-артефакты и трек соответствуют процессу:** `User-Visible: yes`,
+  оба changelog в одном коммите, `docs/CANVAS.md`/`ARCHITECTURE.md`/
+  `USER-GUIDE.ru.md` в списке правок (§16), полный трек (не `small`/`trivial`)
+  обоснован в аналитике — сложность и число задетых инвариантов оправдывают
+  выбор.
+- **Non-scope сформулирован конкретно и исключает реальные соблазны**
+  расширения (путь по нескольким рёбрам, второй resolver проёмов, новая
+  визуальная индикация, hit-radius #137) — не оставляет скрытого расширения
+  скоупа.
+
+## Чего не проверял
+
+- Реализацию — её ещё нет; это ревью ТЗ, а не кода (PROCESS.md §2.4 не
+  предполагает исполнения на этом этапе).
+- Golden/визуальные артефакты — ТЗ корректно утверждает, что новых пикселей
+  нет (§14.3), баз для сверки нет и не требовалось.
+- Производительность вживую — утверждение об O(S)-поиске только на click
+  (§12, AC10) проверено чтением архитектуры snap-геометрии (кэшированный
+  snapshot в `_planSnapGeometrySnapshot()`), не профилированием — на этапе ТЗ
+  профилирование не требуется.
+- Backend/schema — ТЗ утверждает отсутствие изменений (§11, AC12); код
+  `custom_components/houseplan/**/*.py` не затрагивается по scope ТЗ, отдельно
+  не сверялся построчно, поскольку заявленный scope его не касается.
+
+## Вывод
+
+Один блокирующий (High) дефект контракта: связка §7.1 п.3 + §7.2 + §8.1 + §8.2
+без защитного гейта по минимальному числу уже нарисованных вершин превращает
+обычный, сегодня работающий второй клик по общей стене в блокирующую ошибку —
+регрессия ровно того сценария, который issue #138 должен исправить. Остальной
+контракт, включая проёмы, толщину, drafts/partitions, historyCancel/Save,
+touch-деградацию и release-артефакты, проверен по коду и корректен.
+
+Возврат в «ТЗ в работе» (`S3-spec`), цикл r1/4. Исправление: явно решить (и
+записать в ТЗ), что происходит при клике на другую точку того же интервала,
+когда в контуре ещё недостаточно вершин для валидного замыкания — деградация к
+обычному добавлению точки (по аналогии с текущим гейтом `path.length >= 3` для
+закрытия по первой точке) либо иной явно обоснованный вариант, покрытый
+отдельным AC и unit-тестом.