docs: review document for #304

Issue: #304
User-Visible: no

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

@@ -0,0 +1,128 @@
+# SPEC-REVIEW-304-r1
+
+Issue: #304 «Редактор плана: режим «Толщина» скрывает часть осевых линий и узлов стен»
+Этап: spec (лёгкий трек, метка `small`)
+Заход: r1 · блокирующих циклов израсходовано 0 из 2 (лимит лёгкого трека — 2)
+Ревьюер: Claude (сессия ревью ТЗ), артефакт — комментарий в issue + этот документ (по правилу §5 документ не обязателен, но публикуется для трассируемости).
+
+## Скоуп
+
+ТЗ живёт в теле issue #304 (комментарий владельца от 2026-08-25 подтверждает `small`).
+Задача: unify статический слой архитектурных осей/узлов Plan editor (`plan-snap-overlay`)
+так, чтобы он не зависел от активного `MarkupTool` — сейчас полный слой рендерится
+только для `draw`, а в остальных девяти инструментах его подменяет
+`hidden-wall-diagnostic-overlay`, который по контракту показывает только скрытые
+перекрывающиеся *независимые* источники (draft/partition), а не полную геометрию
+комнат. AC1–AC7, план проверок, риски, откат и release-артефакты присутствуют.
+
+Продуктовая рамка: J6 SCOPE.md — «Keep the plan true as the home evolves», конкретно
+«достоверная топология при поддержании плана». Owner-комментарий явно ссылается на J6.
+Соответствие подтверждаю: задача не расширяет и не меняет ни один Core user job,
+чинит расхождение представления внутри уже принятого поведения инструмента «Стены».
+
+## Как проверялось
+
+Ревью ТЗ выполнялось не на веру автору, а с чтением текущего кода — задача описывает
+конкретный технический механизм бага, и его нужно было заземлить, а не принять как
+заявление:
+
+1. `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` §2.4/§2.10/§5/§7.1/§7.2/§12 — рамка процесса
+   и лёгкого трека.
+2. Тело issue #304 и оба комментария владельца (аналитика + передача на ревью).
+3. `docs/USER-GUIDE.ru.md:373-379` — подтверждает, что текущая документация уже
+   фиксирует осевые линии как поведение именно инструмента «Стены»; после фикса это
+   предложение придётся расширить на все инструменты Plan editor, что ТЗ и заявляет
+   в Release-артефактах.
+4. `src/houseplan-card.ts`:
+   - `588` — `type MarkupTool = 'select' | 'draw' | 'column' | 'merge' | 'split' |
+     'resize' | 'opening' | 'boundary' | 'wallthick' | 'delroom'` — ровно десять
+     значений, как в AC2, не выдумка;
+   - `1524-1526` — `get _markup(): boolean { return this._mode === 'plan'; }` — статический
+     слой физически не может просочиться в View/Device/Backdrop-editor, потому что все
+     точки рендера overlay обёрнуты в `this._markup ? … : nothing`; AC5 — не гипотеза, а
+     прямое следствие текущей структуры кода;
+   - `17565` `_renderWallBodies`, `17572-17573` `_renderHiddenWallDiagnosticOverlay`
+     (без условия на `_tool`, то есть рендерится во всех markup-инструментах),
+     `17578-17579` `${this._markup && this._tool === 'draw' ? … this._renderPlanSnapOverlay()
+     … : nothing}` — это буквальное подтверждение корневой причины из issue: полный
+     `plan-snap-overlay` гейтится на `draw`, diagnostic-слой остаётся один во всех
+     остальных.
+5. `src/plan-snap-overlay.ts`:
+   - `166-215` `buildHiddenWallDiagnosticGeometry` — `hidden = sources.filter(source =>
+     source.kind !== 'room' && sources.some(other => other !== source &&
+     positiveCollinearOverlap(...)))` — буквально подтверждает контракт «показывает
+     только скрытые перекрывающиеся независимые источники», room-only рёбра без партнёра
+     по перекрытию никогда сюда не попадают. Это и есть причина «части осей нет в
+     Толщина» — не домысел автора ТЗ, а прочитанный код;
+   - `228` `buildPlanSnapGeometry` — существующая каноническая геометрия, которую фикс
+     предлагает переиспользовать для статического слоя во всех инструментах (contract p.3);
+6. `demo/smoke_plan_snap_overlay.mjs:236-239` — `result.otherPlanToolsHaveHiddenDiagnostic =
+   !overlay() && !!diagnostic && …` — буквально закрепляет `!overlay()` (полного слоя нет)
+   для инструмента `select` как ожидаемое поведение. Это подтверждает заявление issue
+   «smoke сейчас закрепляет это расхождение вместо проверки межинструментального
+   инварианта» — не голословно, а строкой теста.
+7. `demo/golden/matrix.mjs`, `demo/golden/harness.mjs` — существуют, ссылка в скоупе ТЗ
+   не на вымышленную инфраструктуру.
+8. `docs/TOUCH-SUPPORT.md` — grep по overlay/snap не дал контрактных ограничений на этот
+   слой; заявление «touch-контракт не расширяется» не противоречит канону (слой был и
+   остаётся `pointer-events: none`, десктоп-first редактор не меняется).
+9. `docs/WALL-THICKNESS.md`, `docs/CANVAS.md`, `docs/UX-MODES.md` — grep по overlay/wallthick
+   не нашёл предписаний, которым ТЗ противоречило бы.
+
+## Находки
+
+Нет находок уровня High или Medium. Задача полностью укладывается в лёгкий трек:
+сложность/риск, заявленные владельцем (3/10 и 3/10), подтверждаются техническим
+разбором — правка изолирована в рендер-гейтинге одного файла плюс возможное
+разделение геометрии в `plan-snap-overlay.ts`, без миграции, новых i18n-ключей,
+нового UX-контракта (видимое поведение просто перестаёт зависеть от инструмента —
+это и есть заявленный "после") и без touch-влияния.
+
+Low, не блокирует, не требует правки:
+- AC1 фиксирует магическое число «шесть узлов» для контрольного участка. Это не
+  домысел: число заземлено в приложенных к issue скриншотах (`2026-08-25_15-46-52.png`
+  — полный вид «Стены») и будет закреплено unit-фикстурой на этапе реализации (план
+  проверок, п.1). Претензий к формулировке нет, отмечаю для полноты «чего не проверял».
+
+## Что проверено и корректно
+
+- Обязательные разделы §7.1 (для лёгкого трека — §5: проблема, контракт, AC1…ACn с
+  доказательством, откат) присутствуют, ТЗ фактически даёт больше, чем требует лёгкий
+  трек (сценарий, эдж-кейсы, риски, release-артефакты).
+- Каждый AC (1–7) однозначен, имеет способ доказательства (`unit`/`smoke`/`golden`/
+  «ревью кода») и указывает конкретный наблюдаемый инвариант, а не реализацию.
+- Корневая причина бага, названная в issue, дословно подтверждается кодом — это не
+  «догадка, выданная за решение»: технический механизм проверен построчно (см. выше).
+- Контракт поведения (пп.1–7) полон: перечисляет все 10 инструментов, явно разделяет
+  статическую топологию и transient-состояния инструмента «Стены», фиксирует
+  presentation-only и `pointer-events: none`, границы режимов (View/Device/Backdrop) и
+  отсутствие промежуточного пустого кадра при переключении.
+- Скоуп/не-скоуп корректно исключают алгоритмы привязки/записи толщины и построения
+  стен — фикс ограничен представлением, не моделью.
+- Совместимость, миграция, i18n, touch, performance разобраны и обоснованы отсутствием
+  влияния, а не пропущены молчанием.
+- Риски (двойная отрисовка, порядок рендера, случайная утечка transient-маркеров,
+  инвалидация кэша на pointermove) названы предметно и совпадают с зонами, которые я
+  сам выявил бы при чтении кода рендер-конвейера — заявка не занижает риск.
+- Откат тривиален и реалистичен (revert рендер-гейтинга, схема/данные не меняются).
+- Owner-аналитика подтверждает соответствие job J6, дубликаты проверены и отклонены
+  предметно (#232 — про другое: hover при рисовании).
+
+## Чего не проверял
+
+- Не проверял на глаз итоговое число узлов/сегментов для контрольной fixture построчным
+  выполнением `buildPlanSnapGeometry`/`buildHiddenWallDiagnosticGeometry` — это задача
+  unit-теста на этапе реализации, а не спецификации; на этапе ТЗ важно было убедиться,
+  что функции и семантика существуют и соответствуют описанному контракту, что сделано.
+- Не запускал `npm test`/`npm run build`/`typecheck` — на этапе ревью ТЗ гейты кода не
+  прогоняются (нечего собирать, код не менялся), это относится к этапу код-ревью (§2.7).
+- Не проверял golden/смоук-набор целиком — задача их ещё не создала (в скоупе только
+  план их появления), это тоже предмет код-ревью.
+- Не оценивал реальный экспорт `houseplan-space-convergence-test-...json`, приложенный к
+  issue (файл не запрашивался и не был нужен: код уже подтверждает механизм независимо
+  от конкретного экспорта).
+
+## Вердикт
+
+Зелёный. ТЗ проверяемо, однозначно, технически заземлено чтением кода, укладывается в
+лёгкий трек и закрывает продуктовую строку J6. High/Medium-находок нет.