From 9c5e1e27499340bab9e27e8f4de4436b915f8588 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Tue, 25 Aug 2026 13:06:34 +0000 Subject: [PATCH] docs: review document for #304 Issue: #304 User-Visible: no --- docs/reviews/SPEC-REVIEW-304-r1.md | 128 +++++++++++++++++++++++++++++ 1 file changed, 128 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-304-r1.md diff --git a/docs/reviews/SPEC-REVIEW-304-r1.md b/docs/reviews/SPEC-REVIEW-304-r1.md new file mode 100644 index 00000000..99c25fbd --- /dev/null +++ b/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-находок нет.