From d66cd2ebab9f4eea1e464ad4d5d78c952224dde3 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Fri, 14 Aug 2026 12:33:14 +0000 Subject: [PATCH] docs: review document for #138 Issue: #138 User-Visible: no --- docs/reviews/SPEC-REVIEW-138-r2.md | 165 +++++++++++++++++++++++++++++ 1 file changed, 165 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-138-r2.md diff --git a/docs/reviews/SPEC-REVIEW-138-r2.md b/docs/reviews/SPEC-REVIEW-138-r2.md new file mode 100644 index 00000000..03845bd4 --- /dev/null +++ b/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`).