docs: review document for #138

Issue: #138
User-Visible: no

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

@@ -0,0 +1,165 @@
+# Ревью ТЗ — issue #138, цикл r2
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/138
+- **ТЗ:** `docs/specs/138-adjacent-room-autoclose.md`, коммит `6704cfc9258d78688e83f13233cf67a12eae7bea`
+- **Предыдущий цикл:** `docs/reviews/SPEC-REVIEW-138-r1.md`, вердикт красный, High: 1
+- **Этап:** spec (PROCESS.md §2.4)
+- **Вердикт:** зелёный · цикл r2/4 · High: 0 · Medium: 0
+
+## Скоуп ревью
+
+Прочитаны заново: `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` (§1, §2, §2.4,
+§4, §7.1–7.2, §12), тело issue #138 и все комментарии владельца, включая
+аналитику Q1–Q5 с defaults, `SPEC-REVIEW-138-r1.md` целиком, ответ владельца
+на r1 (коммит `6704cfc`), и полный текущий текст
+`docs/specs/138-adjacent-room-autoclose.md` (не только диф r1→r2— весь
+документ прочитан заново, чтобы не подтверждать чужой вывод не глядя).
+Дополнительно: `docs/USER-GUIDE.ru.md` (терминология «Оставить замкнутыми
+стенами», секции про соседние комнаты и общую стену), `docs/TOUCH-SUPPORT.md`
+(safety floor, «best effort», список из шести условий деградации),
+`docs/specs/137-plan-snap-overlay.md` как основа контракта.
+
+Задача не помечена `small`, лимит цикла — 4 (полный трек), это r2. ТЗ лежит в
+`docs/specs/138-adjacent-room-autoclose.md`, зарегистрировано в
+`docs/specs/README.md:49`.
+
+## Как проверялось
+
+Между r1 и r2 изменился только сам файл ТЗ — `git diff 611c5a7 6704cfc --
+docs/specs/138-adjacent-room-autoclose.md` подтверждает, что коммит `6704cfc`
+не трогает `src/**` или что-либо ещё; продуктовый код с r1 не менялся, поэтому
+все выводы r1 о соответствии терминологии реальному коду остаются в силе без
+повторной построчной сверки там, где текст ТЗ не изменился.
+
+Специально перечитан и построчно сверен с кодом именно тот фрагмент, который
+правит High-1 из r1:
+
+- `src/houseplan-card.ts:6454–6575` (`_markupClick`) и `6426–6452`
+  (`_closeRoomContour`) — чтобы проверить, действительно ли новый
+  minimum-vertex gate (§7.1 п.3, §7.2, §8.1–8.3) воспроизводит существующий
+  защитный гейт `this._path.length >= 3 && this._samePt(pt, this._path[0])`
+  (`houseplan-card.ts:6522`) по аналогии, а не только по формулировке.
+- Прослежены все достижимые состояния `this._path` в момент клика: путь длины
+  `0` обрабатывается отдельной веткой (`houseplan-card.ts:6527–6545`, resume
+  или `this._path = [pt]`) и никогда не доходит до строки 6522 / новой
+  eligibility-проверки; значит единственное «недостаточное» состояние на
+  входе в новый контракт — `this._path.length === 1` (только точка `A`).
+  Именно это состояние и описывает новый AC3/§7.1/§8.3, без пробелов.
+- Пересчитана арифметика минимума: у ручного замыкания на входе требуется
+  `path.length >= 3` (уже 3 узла), клик не добавляет новый узел — итоговый
+  полигон имеет 3 разных вершины. У автозамыкания на входе требуется
+  `path.length >= 2` (2 узла: `A`, `P`), клик добавляет `B` как новый узел —
+  итоговый полигон `[A, P, B]` тоже 3 разных вершины. Это тот же инвариант
+  «минимум треугольник», выраженный симметрично для двух разных путей входа
+  (совпадение с первой точкой vs. новая точка на общей стене), а не
+  произвольное число.
+- Проверено, что порядок проверок в тексте (§8.2: «недостаточное количество
+  вершин... обрабатывается как обычный клик по §8.3 без toast») не
+  противоречит §8.1/§8.2 для случая, когда вершин уже достаточно, но prospective
+  polygon невалиден по другой причине (self-intersection/overlap/zero-area) —
+  эти два условия (мало вершин / геометрически невалидно при достаточном числе
+  вершин) взаимно исключающие и оба покрыты отдельными ветками контракта и
+  отдельными AC (AC3 и AC6).
+- Перечитаны переномерованные AC1…AC13 (§13) и пункты плана автотестов §14.1
+  (1–10) и §14.2 (1–9) на согласованность номеров и полноту — ссылок на старые
+  номера, пропущенных или задвоенных пунктов не найдено.
+- `docs/specs/README.md:49` — запись на ТЗ не устарела после переименования
+  редакции.
+- `docs/TOUCH-SUPPORT.md:58–70` (safety floor) — сверка с §10 нового текста:
+  все пять перечисленных touch-инвариантов (`pinch`, `pan`, `pointercancel`,
+  второй touch, suppressed synthetic click) прямо соответствуют
+  канонической формулировке «saving unintended geometry merely because a
+  pinch, pointer cancellation or second touch was misread as a click».
+- `docs/USER-GUIDE.ru.md:281,294,324` — «Оставить замкнутыми стенами»,
+  «Объединить», «общая стена» — термины ТЗ не изобретены.
+- `src/plan-snap-overlay.ts:1–180` (`cutSegments`, `roomEdges`, `sourceKind`,
+  `axisKey`-дедупликация) — повторно проверено, что §7.2/§7.3 нового текста не
+  расходятся с уже проверенной в r1 структурой; правки r1→r2 в эти секции не
+  меняли геометрический контракт, только добавляли gate по числу вершин перед
+  ним.
+
+Отдельно рассмотрен вопрос, не следовало ли решение (второй клик по общей
+стене при недостающих вершинах тихо добавляет обычную точку, без toast)
+вынести владельцу как продуктовый вопрос, а не решить технически в ответ на
+red-вердикт. Вывод: нет — выбранный вариант **не меняет никакое видимое
+поведение** относительно состояния до issue #138: это ровно то, что делает
+сегодняшний код (`houseplan-card.ts:6573–6574`) для того же клика. Автор не
+принял новое продуктовое решение, а сузил новый контракт так, чтобы он не
+трогал случай, для которого никакого изменения не было заявлено ни в issue,
+ни в Q1–Q5. Эскалация «оставить работающий сегодня клик работающим и дальше»
+была бы вопросом без реального разночтения.
+
+## Находки
+
+Нет. Единственный High из r1 устранён без побочных эффектов; новых High или
+Medium при повторном прочтении всего документа не найдено.
+
+### High-1 из r1 — статус: исправлено
+
+Гейт «недостаточное число вершин» теперь явно предшествует eligibility-проверке
+(§7.1 п.3, §7.2 первая строка, §8.2 последний абзац, §8.3 первый пункт списка),
+получил собственный AC3, unit-пункт §14.1.3, smoke-пункт §14.2.2 и строку риска
+в §17. Проверено по коду: единственное достижимое «недостаточное» состояние —
+`this._path.length === 1` — теперь однозначно закрыто и ведёт к тому же
+поведению, что и сегодня (обычное добавление точки, без toast, без диалога).
+Сценарий из r1 (первая точка `A` на угле, второй клик `B` на другом конце той
+же стены) больше не регрессирует: он explicit-но отнесён к §8.3, не к
+eligibility.
+
+## Что проверено и корректно
+
+- Всё, что было подтверждено в r1 («Что проверено и корректно» этого
+  документа) и не менялось между r1 и r2, остаётся в силе: терминология не
+  изобретена, обязательные разделы §7.1 PROCESS.md на месте, i18n не меняется,
+  наследование толщины общей стены (`wall-thickness.ts:461`) обосновано,
+  проёмы/cuts переиспользуют #137 без второго resolver, дедупликация общей
+  стены по `axisKey` подтверждена, вопросы Q1–Q5 закрыты владельцем пакетом,
+  release-артефакты и трек соответствуют процессу, non-scope конкретен.
+- Новый minimum-vertex gate арифметически и логически симметричен
+  существующему гейту ручного замыкания (`path.length >= 3` для клика в
+  первую точку ↔ `path.length >= 2` перед добавлением новой точки `B` для
+  автозамыкания) — оба требуют ровно 3 разные вершины итогового полигона,
+  бez произвольных чисел.
+- AC1–AC13 переномерованы согласованно; каждый AC называет способ
+  доказательства (`unit`, `smoke`, `code review`, `performance review`,
+  `schema/security review`); ни один не сформулирован как открытый вопрос.
+- Риски (§17) содержат отдельную строку для нового защитного гейта с указанием
+  меры (unit + smoke), т.е. класс регрессии из r1 не остался незамеченным в
+  таблице.
+- План автотестов §14.1/§14.2 покрывает именно новый граничный случай (пункт
+  3 unit, пункт 2 smoke) отдельно от общих negative-кейсов (пункты 4–5/3–4),
+  так что AC3 доказуем автоматически, а не только по тексту.
+- Решение не escalировать выбор поведения при недостатке вершин владельцу —
+  обосновано выше: вариант сохраняет статус-кво без нового видимого
+  поведения, то есть не является продуктовой развилкой.
+
+## Чего не проверял
+
+- Реализацию — её ещё нет; код `src/**` не менялся с r1 (подтверждено
+  `git diff 611c5a7 6704cfc --stat` — только spec-файл), проверка кода
+  на этом этапе не требуется PROCESS.md §2.4.
+- Golden/визуальные артефакты и живой перфоманс — не изменились относительно
+  r1: заявления §14.3/§12 не изменились по смыслу, только переномерованы;
+  повторная сверка архитектуры snap-геометрии не требовалась, так как её текст
+  между r1 и r2 не менялся.
+- Backend/schema — как и в r1, `custom_components/houseplan/**/*.py` вне
+  scope ТЗ, отдельно не сверялся.
+- Полный повторный аудит секций 7.2/7.3/8.1/9–12/15/18/19, не затронутых
+  диффом r1→r2, — прочитан целиком в рамках этого цикла (не только
+  построчный git diff), но без повторного code-review каждого отдельного
+  утверждения, ранее уже подтверждённого в r1 по неизменному коду; проверялась
+  внутренняя согласованность с новым текстом §7.1/§8, а не повторная сверка с
+  исходниками с нуля.
+
+## Вывод
+
+High-1 из r1 устранён без побочных эффектов и без появления новых High/Medium
+находок. Новый minimum-vertex gate — прямая и арифметически верная аналогия
+уже существующего защитного гейта ручного замыкания, покрыта собственным AC,
+unit- и smoke-пунктом и строкой риска. Решение не деградирует ни один
+соседний сценарий и не меняет поведение, для которого issue #138 не заявляла
+изменений. Остальной контракт (проёмы, толщина, drafts/partitions,
+Cancel/Save/history, touch-деградация, release-артефакты) не менялся с r1 и
+остаётся проверенным и корректным.
+
+Готово к разработке (`S5-ready`).