docs: review document for #173

Issue: #173
User-Visible: no

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

@@ -0,0 +1,306 @@
+# SPEC-REVIEW-173-r1
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/173
+- **ТЗ под ревью:** `docs/specs/173-unified-wall-tool.md` (коммит `d096a02`,
+  ветка `issue/173-unified-wall-tool`)
+- **Роль:** ревьюер ТЗ (не автор), этап `S4-spec-review`
+- **Трек:** обычный (не `small`/`trivial`) — аналитика владельца прямо
+  говорит «`small`/`trivial` неприменимы: это новый UX-контракт, несколько
+  модулей и высокая геометрическая цена ошибки», сложность/риск 9/10.
+  Полный трек и отдельный файл ТЗ выбраны верно.
+- **Цикл:** r1/4
+
+## Скоуп ревью
+
+Проверялось соответствие ТЗ:
+
+- `docs/SCOPE.md` — попадание в Core user jobs (J4/J6), отсутствие
+  расширения скоупа за пределы описанного объединения инструментов;
+- `PROCESS.md` §2.4/§2.5 (DoR), §7.1 (обязательные разделы ТЗ, продуктовые
+  vs технические вопросы), §3/§12 (запрет «догадка вместо решения»);
+- `AGENTS.md` — классы файлов, имя ветки, трейлеры коммита ТЗ;
+- каноническим документам подсистемы: `docs/UX-MODES.md` (toolbar/tool
+  names, режимы, touch-политика), `docs/TOUCH-SUPPORT.md` (обязательная
+  формула деградации), `docs/CANVAS.md`/`docs/WALL-THICKNESS.md`
+  (planar-геометрия, provenance, толщина, island-контракт), `docs/ARCHITECTURE.md`
+  (`polyContainsPoly`/`islandsOf`);
+- `docs/USER-GUIDE.ru.md` — текущая терминология «Контур комнаты» /
+  «Перегородка»;
+- фактическому коду (`src/houseplan-card.ts`, `src/plan-snap-overlay.ts`,
+  `src/i18n/{ru,en}.json`) — чтобы диагноз текущего состояния (§3 ТЗ) не
+  оказался непроверенной догадкой, выданной за факт;
+- полному треду issue #173 — продуктовая идея владельца, аналитика Codex,
+  пакет вопросов Q1–Q7 с default'ами, решения владельца, хендофф автора ТЗ.
+
+## Как проверялось
+
+1. Прочитан весь тред issue #173: исходная продуктовая идея владельца
+   (18.08.2026), аналитика Codex (ценность 9/10, сложность/риск 9/10, P1,
+   явное «`small`/`trivial` неприменимы»), пакет вопросов Q1–Q7 (каждый с
+   предложенным default), ответ владельца «Q1 принят с правкой… Q2–Q7 приняты
+   предложенные defaults без изменений», занятие автора ТЗ и хендофф со
+   ссылкой на `docs/specs/173-unified-wall-tool.md`.
+2. Сверены обязательные разделы ТЗ (§7.1 PROCESS.md) построчно — таблица
+   ниже.
+3. Прочитан код и построчно сверены ключевые фактические утверждения §3 ТЗ
+   о текущем состоянии:
+   - `type MarkupTool = ... | 'draw' | 'partition' | ...`
+     (`src/houseplan-card.ts:497`) — два отдельных инструмента подтверждены;
+   - `markup.add` = «Контур комнаты» / «Room outline»,
+     `markup.partition` = «Перегородка» / «Partition»
+     (`src/i18n/ru.json:13,74`, `src/i18n/en.json:13,74`) — «До:» из §2 ТЗ
+     совпадает с реальными текущими подписями кнопок, а не с уже
+     переименованным текстом канона (см. Low-2 ниже про сам канон);
+   - `MAX_PARTITIONS = 2000` (`src/houseplan-card.ts:531`),
+     `_keepClosedAsPartitions` (`:11647`), `_contourClosed` getter (`:11177`)
+     — существующий механизм ровно такой, как описан в §3 п.2/5;
+   - `src/plan-snap-overlay.ts` существует отдельным модулем (692 строки
+     иначе не совпало бы), собирает `room`/`draft`/`partition` сегменты с
+     opening/open-span cuts (`:115-153`) и строит только endpoint/line-snap
+     кандидаты (`:265-289`) — не строит faces. Это подтверждает §3 п.4 и
+     §13.2 («второй opening resolver запрещён», «делит с detector общий pure
+     collector») как техническую границу, реально существующую в коде, а не
+     придуманную;
+   - `docs/ARCHITECTURE.md:990-991` подтверждает «действующий island-room
+     контракт» (`polyContainsPoly`, `islandsOf`), на который ссылается §10.1
+     ТЗ, — не изобретённое понятие;
+   - `docs/WALL-THICKNESS.md:207` («an interior↔interior X crossing keeps
+     normal…») и `docs/specs/141-wall-junctions.md` подтверждают, что X не
+     становится persisted-узлом в #141 — ровно то ограничение, которое §3 п.7
+     и §9.2.3 ТЗ фиксируют явно («X становится вычисляемым узлом только для
+     face topology; persisted записи и физический join-контракт #141 не
+     меняются»). Технической коллизии с #141 нет.
+   - `toast.room_overlap` (`src/houseplan-card.ts:6442,6692`) подтверждает,
+     что «текущая validation/toast семантика» partial-overlap (§10.1) —
+     реально существующий, а не гипотетический путь.
+4. Прочитан `docs/UX-MODES.md` целиком (раздел «Plan — independent physical
+   objects», строки 164–180) и `docs/TOUCH-SUPPORT.md` целиком. Формулировка
+   ТЗ §12 «Touch editor: best effort / intentionally degraded» — буквальная
+   канон-метка, требуемая `TOUCH-SUPPORT.md` («Documentation rule»). Контракт
+   §12 (pinch/pan/pointercancel/second pointer не завершают цепочку и не
+   создают геометрию) прямо реализует «Safety floor», в частности пункт
+   «saving unintended geometry merely because a pinch, pointer cancellation or
+   second touch was misread as a click» — не изобретение автора.
+5. Проверено разделение продуктовых и технических вопросов (PROCESS.md §7.1:
+   владельцу — только «что видит/делает» и «объём видимых изменений»).
+   Q1–Q7 — все о наблюдаемом поведении (когда завершается цепочка, что
+   считать эквивалентом «замыкания», в каком порядке идут диалоги, нужен ли
+   тихий режим, остаётся ли Split). Ни один не является техническим вопросом,
+   ошибочно вынесенным владельцу. Блок §20 («Принятые технические
+   предположения») отдельно перечисляет 10 технических решений, прямо
+   помеченных как «можно менять свободно», и завершается явным «Открытых
+   продуктовых вопросов нет: … Q2–Q7 — defaults, 18.08.2026» — это корректное
+   разделение, а не спрятанная догадка.
+6. Проверены все 17 AC (§14) на однозначность и явное указание способа
+   доказательства — у каждого в скобках назван метод (`unit`/`smoke`/
+   `golden`/`performance`/`code review`) и исполнитель. Отдельно проверено,
+   что AC1–AC13 покрывают все пункты Scope (§5), AC14–AC17 — совместимость,
+   производительность, touch и гейты реализации; расхождений между Scope и
+   AC не найдено.
+7. Сверена трассируемость: `docs/specs/README.md:96` обновлён тем же
+   коммитом `d096a02`; `git diff --stat origin/dev...HEAD` содержит только
+   `docs/specs/173-unified-wall-tool.md` и `docs/specs/README.md` (класс C,
+   ни одного файла класса A — правило №1 не нарушено на этапе ТЗ). Коммит
+   несёт `Issue: #173`, `User-Visible: no` — верно для документа ТЗ.
+8. Проверено, не дублирует ли задача открытые основания (#137, #138, #141,
+   #150, #172): все перечислены в ТЗ (заголовок «Связано») как готовые
+   строительные блоки (snap, adjacent-autoclose, стыки, толщина), а не как
+   пересекающийся скоуп; #138 (adjacent room autoclose) явно указан как
+   регрессионный сценарий, который должен остаться зелёным (AC7) — граница
+   с #173 (полный planar graph против узкого room-owned-edge контракта #138)
+   проведена явно в §3 п.3.
+
+## Обязательные разделы (§7.1 PROCESS.md)
+
+| Раздел | Есть | Комментарий |
+|---|---|---|
+| Сценарий (персона/поверхность/момент) | ✅ | §1 — администратор дома, desktop Plan editor, момент выбора инструмента перед первым кликом |
+| Что человек увидит до/после | ✅ | §2, «До:»/«После:», проверено по факту i18n (см. «Как проверялось» п.3) |
+| Проблема | ⚠️ | Явного раздела «Проблема» нет — см. Low-1; содержательно проблема сформулирована во вступлении и §1, а §3 «Подтверждённое текущее состояние» несёт функцию причины/диагноза |
+| Скоуп / не-скоуп | ✅ | §5 / §6, 13 и 12 пунктов соответственно, границы явные (Split не трогается, нет persisted node/face, нет новой настройки) |
+| Контракт поведения | ✅ | §7 (термины/инварианты) + §8 (UX) + §9 (planar graph) + §10 (создание комнат) |
+| Модель данных и миграция | ✅ | §11 — явное «новых persisted полей нет», совместимость старых drafts/partitions/warm tool расписана по каждому случаю |
+| UX, i18n, accessibility, touch | ✅ | §12, включая буквальную канон-метку touch-деградации |
+| AC1…ACn с доказательством | ✅ | §14, 17 штук, каждый помечен методом и исполнителем |
+| План автотестов | ✅ | §15, unit / targeted smoke / golden / performance+backend / implementation loop |
+| Риски | ✅ | §18, 12 строк риск → мера |
+| Откат | ✅ | §19 |
+| Release-артефакты | ✅ | §17, конкретный список из 13 файлов/каналов, включая оба changelog в implementation-коммите |
+
+Единственное отклонение от конвенции — отсутствие отдельного заголовка
+«Проблема» (все предыдущие ТЗ, например #138, #172, выделяют её отдельным
+разделом). Содержательно проблема присутствует (см. вступление и §1: «перед
+первым кликом надо решить внутренний вопрос модели… при ошибочном выборе
+приходится менять инструмент либо перерисовывать геометрию»), поэтому это
+Low, не High/Medium — см. находки.
+
+## Находки
+
+Находок уровня **High** нет. Находок уровня **Medium** нет — новых issue не
+требуется.
+
+### Low-1 — нет отдельного раздела «Проблема»
+
+**Файл:** `docs/specs/173-unified-wall-tool.md` (между §2 и §4; текущий §3
+называется «Подтверждённое текущее состояние»)
+
+PROCESS.md §7.1 перечисляет «проблема» отдельным обязательным разделом, и все
+предшествующие ТЗ в репозитории (`138-adjacent-room-autoclose.md:44`,
+`172-zero-divider-taper.md:36`) держат его отдельно от «текущего состояния».
+В ТЗ #173 формулировка проблемы («пользователь должен заранее решить, что он
+рисует… при ошибочном выборе — менять инструмент или перерисовывать») звучит
+только во вступлении к issue и в первом абзаце §1, а раздел §3 сразу
+переходит к построчному разбору кода. По существу пропуска нет — читатель
+получает и мотивацию, и диагноз, — но формально это отход от структуры,
+которая делает разделы машинно/визуально сверяемыми.
+
+**Решение ревьюера:** Low, не блокирует. Можно поправить косметически (дать
+§3 заголовок «Проблема и подтверждённая причина», как в #172) при следующей
+редакции либо снять записью без нового цикла.
+
+### Low-2 — `docs/UX-MODES.md` уже называет текущий инструмент «Walls»
+
+**Файл:** `docs/UX-MODES.md:83, 166` vs `docs/specs/173-unified-wall-tool.md`
+§2 «До:», §8.1
+
+Канонический документ подсистемы в описании **текущего** (до #173) состояния
+Plan-редактора пишет «Toolbar tools: **Walls** / room outline… Partition,
+Column…» и «**Walls** still draws room contours» — то есть уже сейчас
+называет инструмент «Walls», хотя фактический публичный лейбл кнопки
+(`markup.add`) — «Контур комнаты» / «Room outline» (проверено по
+`src/i18n/{ru,en}.json:74`, см. «Как проверялось» п.3). ТЗ #173 корректно
+использует настоящую текущую подпись («Контур комнаты» / «Room outline») в
+§2/§3, но не упоминает, что `UX-MODES.md` уже содержит забежавшую вперёд
+формулировку «Walls» для другого инструмента (сегодняшнего `draw`), что
+создаёт двусмысленность при будущем чтении истории документа. Это дрейф,
+существовавший в `UX-MODES.md` до #173, не внесённый этим ТЗ, и раздел
+release-артефактов (§17) обязывает обновить `UX-MODES.md` в implementation
+коммите — то есть у автора кода будет естественная точка исправить
+формулировку заодно.
+
+**Решение ревьюера:** Low, не блокирует приёмку ТЗ. Фиксирую для code review
+#173: при правке `docs/UX-MODES.md` в рамках этой задачи убедиться, что
+итоговый текст однозначно называет единственный инструмент «Walls»/«Стены» и
+не оставляет старой двусмысленной фразы про «Walls / room outline» рядом с
+отдельным «Partition».
+
+### Low-3 — опечатка в AC17
+
+**Файл:** `docs/specs/173-unified-wall-tool.md:522`
+
+`- **AC17 (`typecheck` + `unit` + `build` + documentation review`; разработчик):**`
+— лишний обратный апостроф после «review» перед точкой с запятой (должно быть
+`... + documentation review; разработчик)`). Чисто косметическая опечатка
+форматирования Markdown, не меняющая смысл критерия.
+
+**Решение ревьюера:** Low, не блокирует. Правится одним символом при
+следующей редакции.
+
+## Что проверено и корректно
+
+- **Соответствие `docs/SCOPE.md`:** задача закрывает **J4** («от нуля до
+  плана без Inkscape/YAML» — рисование стен без знания внутренней модели
+  данных) и **J6** («план остаётся правдивым по мере развития» — merge/split/
+  drag редактируются тем же единым инструментом). Обе строки Closed; это
+  улучшение UX-контракта существующей функциональности, а не расширение
+  продукта за пределы SCOPE.
+- **Легитимность полного трека:** сложность/риск 9/10 по собственной оценке
+  аналитики, новый UX-контракт, минимум четыре затронутых модуля/поверхности
+  (toolbar/tool state, planar-graph detector, room-creation/Split, snap-
+  overlay) — критерии `small`/`trivial` (§5/§5.1 PROCESS.md) не выполняются
+  ни по одному пункту, что аналитика прямо констатирует.
+- **Продуктовые вопросы закрыты по процессу:** пакет Q1–Q7 задан одним
+  комментарием, каждый вопрос — с предложенным default и явной формулировкой
+  «что человек видит/делает»; issue корректно ушёл в `blocked` до ответа
+  владельца и вышел из `blocked` сразу после решения. Ни один вопрос не
+  является техническим, ошибочно вынесенным владельцу — все семь про
+  наблюдаемое поведение (завершение цепочки, эквиваленты замыкания, порядок
+  диалогов, судьба Split).
+- **Диагноз текущего состояния не голословен.** Утверждения §3 о `MarkupTool`,
+  `_contourClosed`, `_keepClosedAsPartitions`, `MAX_PARTITIONS`,
+  `plan-snap-overlay.ts` и реальных подписях кнопок построчно сверены с
+  `src/houseplan-card.ts`, `src/plan-snap-overlay.ts` и `src/i18n/*.json` —
+  см. «Как проверялось» п.3.
+- **Границы с соседними задачами (#137/#138/#141/#150/#172) проведены явно**,
+  без пересечения скоупа: #138 остаётся регрессионным контрактом (AC7), #141
+  не получает нового persisted X-узла (§3 п.7, §9.2.3, сверено с
+  `docs/WALL-THICKNESS.md` и `docs/specs/141-wall-junctions.md`), #150/#172
+  остаются единственным источником физической геометрии стен (§10.4 п.7).
+- **Не-скоуп (§6) корректно отсекает соседние соблазны:** удаление Split,
+  автоматические предложения при reload/move/Align/Undo, фоновая миграция
+  старых drafts, persisted node/face, новая настройка «не предлагать»,
+  создание комнаты через проём — всё явно исключено с обоснованием и
+  соответствует принятым Q2–Q7.
+- **Модель данных и миграция (§11):** корректно заявлено «новых persisted
+  полей нет», отдельно расписана судьба legacy warm-tool значения
+  `partition`, старых drafts/partitions (остаются читаемыми, не
+  конвертируются автоматически), downgrade без rollback — соответствует
+  `docs/CONFIG-COMPATIBILITY.md`.
+- **UX/touch (§12):** дословно использует обязательную канон-формулировку
+  `docs/TOUCH-SUPPORT.md` и отдельно перечисляет touch safety floor
+  (pinch/pan/second-pointer/pointercancel не создают геометрию) — прямая
+  реализация «Safety floor that still applies to touch editors».
+- **Planar-graph контракт (§9) методологически надёжен:** явно запрещён
+  «pairwise любой цикл без выделения faces» (частая ошибка naive-детекторов),
+  явно требуется canonical ring identity, инвариантная к порядку/id/
+  направлению — это именно то, что делает AC5 проверяемым unit-тестом с
+  перестановками, а не подверженным флаки.
+- **Дисциплина «тест должен уметь падать»:** §15.1 прямо требует «минимум
+  один topology test должен краснеть на `origin/dev` без detector», а
+  ревьюеру кода прямо поручено проверить его mutation discipline —
+  соответствует PROCESS.md §2.7/§18.
+- **Performance-контракт (§13, AC15)** ссылается на реально существующий
+  профиль `large-house-plan-snap-v1`
+  (`demo/performance/budgets-large-house-plan-snap.json`) и требует не
+  ослаблять исходные бюджеты — не изобретённая, а существующая точка
+  измерения.
+- **Release-артефакты (§17)** перечисляют оба changelog в implementation-
+  коммите, конкретные документы для обновления (`USER-GUIDE.ru.md`,
+  `UX-MODES.md`, `CANVAS.md`, `ARCHITECTURE.md`, `WALL-THICKNESS.md`,
+  `TESTING.md`), golden matrix и три синхронные копии бандла — golden явно
+  ограничен `golden:accept -- --reviewed` по полному Linux-артефакту
+  (согласуется с PROCESS.md §8/§11.4/правилом 13).
+- **Трассируемость:** `docs/specs/README.md:96` обновлён тем же коммитом
+  `d096a02`; ветка `issue/173-unified-wall-tool` и трейлеры (`Issue: #173`,
+  `User-Visible: no`) корректны для документа ТЗ, который сам не меняет
+  поведение продукта. `git diff --stat origin/dev...HEAD` не содержит ни
+  одного файла класса A — правило №1 не нарушено на этапе ТЗ.
+- **Явный блок технических предположений (§20)** из 10 пунктов корректно
+  отделяет техническую свободу («какой именно алгоритм DCEL», «структура
+  кэша», «имена файлов/smoke/golden id») от продуктового решения и явно
+  фиксирует «открытых продуктовых вопросов нет» со ссылкой на дату и
+  комментарий владельца.
+
+## Чего не проверял
+
+- Не проверял реализуемость конкретного алгоритма half-edge/DCEL как
+  единственно возможного решения planar-graph детектора — §20 п.1 явно
+  оставляет выбор алгоритма технической свободой; код-ревью будет проверять
+  выполнение AC4/AC5, а не конкретную структуру данных.
+- Не запускал автотесты, `golden`, browser-смоки или performance-профили —
+  на этапе `spec` это не требуется и большинство названных файлов
+  (`test/wall-face-graph.test.mjs`, `demo/smoke_unified_wall_tool.mjs`) ещё
+  не существуют по плану ТЗ, а не по недосмотру; существование уже
+  используемых зависимостей (`src/plan-snap-overlay.ts`,
+  `large-house-plan-snap-v1` профиль, `polyContainsPoly`/`islandsOf`)
+  проверено чтением кода/конфигурации, не исполнением.
+- Не проверял корректность числовых оценок аналитики (9/10 · 6/10 · 9/10 ·
+  P1) по существу — это поле владельца (PROCESS.md §2.2), уже принято явным
+  решением до написания ТЗ.
+- Не проверял детально совместимость с #137 (`docs/specs/137-plan-snap-overlay.md`)
+  за пределами того, что понадобилось для подтверждения текущей
+  функциональности snap-коллектора (endpoint/line-snap без face-построения);
+  само #137 не входит в предмет этого ревью.
+- Не оценивал, достаточно ли 17 AC с точки зрения «можно ли перепутать
+  порядок исполнения» на этапе кода — это будет предметом code review, когда
+  появится реализация и её тесты; на этапе ТЗ проверялось только то, что
+  каждый AC однозначен и называет способ доказательства.
+
+## Вердикт
+
+Зелёный. High: 0, Medium: 0. Три находки Low (отсутствие отдельного
+заголовка «Проблема»; дрейф терминологии «Walls» в текущем тексте
+`UX-MODES.md`, не внесённый этим ТЗ, но подлежащий исправлению в рамках
+release-артефактов §17; опечатка-апостроф в AC17) — ни одна не блокирует
+приёмку, все либо правятся косметически при следующей редакции, либо
+снимаются этой записью без нового цикла.