docs: review document for #447

Issue: #447
User-Visible: no

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

@@ -0,0 +1,253 @@
+# SPEC-REVIEW-447-r1
+
+- Issue: https://github.com/Matysh/houseplan-card/issues/447
+- Этап: `spec` (PROCESS.md §2.4), полный трек (метка `S4-spec-review`, без `small`)
+- Материал: `docs/specs/447-exterior-furniture-snap-keyboard-nudge.md` на коммите
+  [`a1fbea07`](https://github.com/Matysh/houseplan-card/commit/a1fbea07d2876d51afac3a9df12bfa756424df99)
+  (docs-only, класс C: `docs/specs/447-...md` + `docs/specs/README.md`, `git show --stat a1fbea07`)
+- Заход: r1 · блокирующих циклов до этого раунда: 0/4
+
+## Скоуп ревью
+
+ТЗ описывает две независимые части одной issue:
+
+1. наружная физическая поверхность для внешней стены (`roomFurnitureWallSurfaces`,
+   `snapFurnitureToWall` в `src/furniture-wall-surface.ts` / `src/furniture-placement.ts`);
+2. Arrow-сдвиг выбранного объекта декора на одну ячейку сетки в Редакторе подложки
+   (`_keyHandler`, `_decorSel`, `_decorList` в `src/houseplan-card.ts` /
+   `src/houseplan-editor-runtime.ts`).
+
+Продуктовых вопросов к владельцу нет: тело issue и оба комментария владельца
+(04.09.2026) уже закрывают Shift-шаг, подложку плана, точную величину шага и связь
+с #41. Проверялась не полнота согласования, а исполнимость и однозначность
+контракта и AC.
+
+## Как проверялось
+
+- Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` (§1–§10), тело issue #447 и
+  все три комментария, ТЗ целиком.
+- Прочитан код, который ТЗ описывает как текущее поведение и на который опирается
+  контракт: `src/furniture-wall-surface.ts`, `src/furniture-placement.ts`, фрагмент
+  `_keyHandler` вокруг `src/houseplan-card.ts:3018`, `_decorMoveUpdate`
+  (`src/houseplan-editor-runtime.ts:4336-4376`), `_gridPitch`/`_cellCm`
+  (`src/houseplan-card.ts:7336-7348`), константы `GRID_PITCH`/`GRID_STEP_N`/`NORM_W`
+  (`src/space-geometry.ts:12,212-216`), `segmentCm` (`src/logic.ts:63-66`),
+  `decorCmToUnits`/`decorUnitsToCm` (`src/editors/decor/geometry.ts:76-114`).
+- Сверены утверждения ТЗ о текущем баге (`sideScore` как тай-брейк, а не фильтр) с
+  реальным кодом `snapFurnitureToWall` — подтверждены построчно.
+- Прочитаны `docs/CANVAS.md` §9 (grid/snap contract), `docs/FURNITURE.md`,
+  `docs/USER-GUIDE.ru.md` (разделы «14. Редактор подложки», термин «Декор»,
+  таблица инструмента «Выбрать») на предмет терминологии и уже
+  зафиксированного поведения.
+- Сверена связь с #41 — прочитан текст issue и `docs/specs/041-keyboard-object-editing.md`
+  (`origin/dev`): формулировка «arrows move = один grid node» в #41 не называет
+  конкретную единицу измерения, так что #447 не наследует оттуда готовое решение
+  предмета находки ниже, это самостоятельная неточность #447.
+- Проверено существование файлов, на которые ссылается план тестирования и карта
+  реализации: `docs/FURNITURE.md`, `docs/DECOR-EDITOR.md`, `demo/smoke_furniture.mjs`,
+  `demo/smoke_decor*.mjs` — все существуют.
+- Проверены `DecorKind`/`DecorShape` (`src/editors/decor/types.ts:8-88`) — ровно
+  шесть видов `line | rect | ellipse | text | furniture | image`, как и в ТЗ.
+
+### Гейты
+
+Диапазон изменений на этом SHA — docs-only (класс C по AGENTS.md: только
+`docs/specs/**` и `docs/specs/README.md`), `src/**` не тронут. Условия,
+включающие `typecheck`/`test`/`build`/`check-docs`/`model-invariants`/смоки
+(«если менялся `src/**`», «если менялась геометрия»), не выполнены — ни один из
+них не применим к этому диффу, гонять их бессмысленно: они бы просто
+подтвердили, что `dev`-код (не тронутый) уже проходит эти гейты, и ничего не
+скажут о качестве самого ТЗ. Явно не прогонялись: `npx tsc --noEmit`, `npm test`,
+`npm run build`, `node scripts/check-docs.mjs`, `node scripts/model-invariants.mjs`,
+любые `demo/smoke_*.mjs`, `npm run golden:verify`, `pytest tests_backend` — все по
+причине «diff их не касается», не «пропущено».
+
+## Находки
+
+### H1 — AC4 и §3 путают единицы «шага сетки»: render-space `gridPitch` vs normalised `GRID_STEP_N`, что при буквальной реализации ломает саму суть Part 2
+
+**Файл:** `docs/specs/447-exterior-furniture-snap-keyboard-nudge.md`, разделы
+«3. Сдвиг выбранного декора стрелками» (таблица «Дельта render-координат») и
+«AC4. Каждый вид выбранного декора двигается на одну клетку», плюс комментарий
+владельца от 04.09.2026 («В коде это ровно один `gridPitch`, и никакого деления
+на `cell_cm` не нужно»).
+
+**В чём проблема.** ТЗ и комментарий владельца называют величину шага буквальным
+именем кодовой константы `gridPitch` и требуют применять её к «позиционным
+полям» decor-объекта (`x`, `y`, `x1`, `y1`, `x2`, `y2` — то, что реально лежит в
+`space.decor[]`). Но в текущем коде это два **разных** масштаба, и это не
+случайность, а осознанная граница, задокументированная в самом коде:
+
+- `GRID_PITCH = NORM_W / GRID_N = 1000 / 240 ≈ 4.1667` — шаг сетки в
+  **render-единицах** (`src/space-geometry.ts:212-214`, geometry в масштабе
+  холста 0…1000). Используется там, где считают на уровне пикселей/SVG:
+  `space-render.ts`, `snapR()`, `_gridPitch` getter в `houseplan-card.ts:7336-7342`
+  («NORM_W / GRID_N и ничего больше»).
+- `GRID_STEP_N = 1 / GRID_N ≈ 0.0041667` — тот же шаг в **нормализованных
+  единицах** (`src/space-geometry.ts:216`, комментарий «what the config and the
+  layout store»). Именно в этом масштабе хранятся `x`/`y` room-полигонов, стен,
+  ключей и — что здесь решает — decor: `houseplan-card.ts:9061` прямым текстом
+  «Keys are always stored in normalised space (`GRID_STEP_N`)».
+
+Что фактически хранится в `space.decor[].x/y` подтверждается кодом уже
+существующего mouse-drag: `_decorMoveUpdate`
+(`src/houseplan-editor-runtime.ts:4336-4376`) считает якорь в render-единицах, а
+затем **делит на `NORM_W`** (для x) или на `this.host._decorH` (для y), прежде
+чем записать `x: o.x + dx` — то есть `dx`, попадающий в конфиг, на три порядка
+меньше `GRID_PITCH` (`dx = (anchor[0] - ax0) / NORM_W`, `NORM_W = 1000`,
+`src/space-geometry.ts:12`).
+
+Спецификация же формулирует AC4 буквально как «меняют… позиционные поля на
+`gridPitch`» без единого слова о делении на `NORM_W`/`_decorH` или о разнице
+между координатой в момент пересчёта (render) и координатой на диске
+(normalised). Комментарий владельца ещё и explicit отрицает необходимость
+какого-либо деления («никакого деления на `cell_cm` не нужно») — верно для
+`cell_cm`, но по факту скрывает необходимое деление на `NORM_W`, о котором ни в
+ТЗ, ни в комментарии не сказано ни слова.
+
+**Почему это блокирует, а не техническая мелочь на усмотрение реализации.**
+Раздел «Принятые предположения» разрешает менять «техническую раскладку pure
+helper», но не сам числовой контракт — а именно числовой контракт здесь и
+сформулирован неоднозначно/неверно в AC. Если разработчик реализует AC4 так, как
+она написана — прибавит экспортированную `GRID_PITCH` (≈4.1667) напрямую к
+нормализованному `x` (обычно в диапазоне порядка 0…2–3 для видимого плана) — то
+одно нажатие стрелки сдвинет объект на ~4 ширины холста. Это даже не упрётся в
+`clampCanvasN`/`CANVAS_LIMIT = 5000` (`src/space-geometry.ts:205,240-241`) —
+предмет просто окажется далеко за пределами видимой области без единой ошибки в
+консоли, то есть эффект прямо противоположен заявленному «сдвиг ровно на одну
+клетку сетки, а не число, которое надо помнить». Хуже: если модульный тест по
+AC4 будет (как и написано в тексте AC) утверждать «поле изменилось на
+`GRID_PITCH`», такой тест зафиксирует именно неверную величину и молча пройдёт —
+ровно тот сценарий, о котором предупреждает сам процесс ревью («тест,
+спрашивавший то, чего не проверял», #423) и принцип «одно число — один
+источник»: видимая величина шага (то, что рисует сетку на экране, в
+render-масштабе) и сохранённая величина (то, что пишется в конфиг, в
+normalised-масштабе) обязаны быть одним и тем же физическим шагом, посчитанным
+через одно преобразование, а не два независимых числа с одинаковым именем в
+тексте ТЗ.
+
+**Как воспроизвести рассуждение.** Взять любой существующий decor-объект,
+например `x = 0.4` (типичное значение для видимого плана). Применить
+буквально AC4: `x -= gridPitch` → `x = 0.4 - 4.1667 = -3.7667`. Сравнить с
+ожидаемым «на одну клетку сетки» — при `cell_cm = 5` перемещение на одну клетку
+означает сдвиг на 5 см реального плана, что в normalised-единицах равно
+`GRID_STEP_N ≈ 0.0041667`, а не `4.1667`. Разница — ровно `NORM_W` (1000×).
+
+**Что нужно исправить.** Явно указать в контракте (§3 и AC4), что дельта
+считается в render-координатах (как и озаглавлена таблица), а на «позиционные
+поля» decor записывается **после** того же преобразования, которым уже
+пользуется `_decorMoveUpdate` — деление на `NORM_W` (для x/x1/x2) и на
+высоту холста по Y (для y/y1/y2), либо прямо назвать нормализованный шаг
+`GRID_STEP_N` как то, что фактически прибавляется к `x`/`y`. Это техническое по
+природе уточнение (никакая видимая пользователю величина не меняется — «одна
+клетка» остаётся «одной клеткой» в сантиметрах), поэтому не требует вопроса
+владельцу и решается прямо в тексте ТЗ.
+
+**Категория:** correctness / однозначность AC. **Серьёзность: High** — как
+написано, AC4 не исполним корректно; часть 2 задачи (клавиатурная доводка)
+буквально по тексту ломается на первом же нажатии стрелки.
+
+Других High или Medium в скоупе задачи находок нет.
+
+## Что проверено и корректно
+
+- **Диагноз части 1 подтверждён построчно.** `roomFurnitureWallSurfaces`
+  (`src/furniture-wall-surface.ts:41-82`) действительно строит только одну
+  грань на атом — `axis + inwardNormal * half` — и не создаёт наружного
+  кандидата; `normal` явно документирован как «points from masonry into the
+  owning room» (строка 14). В `snapFurnitureToWall`
+  (`src/furniture-placement.ts:148-153`) `sideScore` участвует в выборе только
+  при `Math.abs(candidate.dist - best.dist) <= tieEps`, первичный критерий —
+  `candidate.dist < best.dist - tieEps`; при единственном кандидате в радиусе
+  сторона не проверяется вовсе. Оба факта, на которых стоит вся первая часть
+  ТЗ, воспроизведены чтением, а не с чужих слов.
+- **Контракт части 1 закрывает найденный баг по существу, а не косметически.**
+  Требование «предмет не переносится через ось на сторону, противоположную
+  точке намерения» — это явный перевод `sideScore` из тай-брейка в фильтр (один
+  из двух вариантов, которые сама issue считает приемлемыми), а не просто
+  добавление наружного кандидата поверх старой логики отбора по `dist` — то
+  есть ТЗ не оставляет прежний баг между старой внутренней и новой внешней
+  поверхностью.
+- **Инварианты общей/нулевой/independent стены названы явно и с AC/mutation**
+  (AC2), включая нечувствительность к порядку комнат и winding — учтён риск,
+  под который заведён отдельный пункт в «Рисках».
+- **Виды decor согласованы с типами.** `DecorKind` (`src/editors/decor/types.ts:10`)
+  — ровно `line | text | rect | ellipse | furniture | image`; ТЗ перечисляет
+  все шесть и не добавляет несуществующих.
+- **Guard-условия Arrow (§3, AC8) переиспользуют уже существующий паттерн.**
+  Текущий `_keyHandler` (`src/houseplan-card.ts:2978-3060`) уже отличает
+  `inField` (composedPath по `input, textarea, select, [contenteditable]`) и
+  `inEditorSecondary` (класс `editor-secondary`) для Delete/Backspace в этом же
+  режиме — ТЗ требует того же для Arrow, что не вводит новый непроверенный
+  guard-паттерн.
+- **Раздельные пути истории названы верно.** `history.decor_move`
+  (`src/i18n/ru.json:124`, использование — `houseplan-card.ts:7018`) —
+  существующий ключ, переиспользуемый и mouse-drag, и клавиатурой; новых
+  i18n-строк ТЗ не требует.
+- **Обязательные разделы §7.1 все присутствуют**: сценарий, что человек увидит
+  до/после, проблема, скоуп/не-скоуп, контракт поведения, UX, данные/миграция/i18n,
+  производительность, крайние случаи, AC1…AC9 с методом доказательства, план
+  автотестов, риски, откат, release-артефакты, принятые предположения. Каждый
+  AC называет способ доказательства (`unit`/`smoke`/`review`).
+- **Трек и лимит согласованы.** Full track обоснован (сложность/риск >3, два
+  независимых поведенческих контракта, новый клавиатурный UX-контракт) —
+  критерии `small` (PROCESS.md §5) действительно не выполнены, названо явно в
+  аналитике; лимит ревью ТЗ для полного трека — 4 цикла, что соответствует
+  «блокирующих циклов 0/4» на входе.
+- **Терминология сверена с USER-GUIDE.ru.md.** «Редактор подложки» — верное имя
+  всего режима (раздел 14, таблица «Декор | Элемент подложки»), не только
+  картинки плана; путаницы между «Редактор подложки» и «декор» в ТЗ нет.
+- **Rollback и release-артефакты адекватны масштабу**: чистый revert
+  frontend/tests/docs/bundle, нет схемы/миграции для отката; changelog RU+EN,
+  `docs/FURNITURE.md`, `docs/DECOR-EDITOR.md`, screenshot fingerprint через
+  `src/**` — корректный список для класса A правки такого объёма.
+
+## Чего не проверял
+
+- Не прогонялись `typecheck`/`test`/`build`/`check-docs`/`model-invariants`/смоки
+  — диапазон изменений на этом SHA docs-only и их не касается (см. «Гейты»
+  выше); они станут релевантны на код-ревью, когда появится диапазон
+  `origin/dev..HEAD` с изменениями в `src/**`.
+- Не проверял, что owner-группировка атома («ровно одной комнате») реализуема
+  за один проход без учёта направления рёбер и winding — это заявлено как
+  требование к реализации (AC1/AC2 + permutation/winding tests), а не как
+  готовый алгоритм в самом ТЗ, поэтому это предмет код-ревью, а не спек-ревью.
+- Не проверял `docs/DECOR-EDITOR.md` и `docs/FURNITURE.md` построчно на предмет
+  того, что именно там придётся дописать — только подтвердил, что оба файла
+  существуют и являются подходящим местом для новых абзацев (release-артефакты
+  ТЗ их и называют).
+- Не оценивал производительность фактическим профилированием — раздел
+  «Производительность» ТЗ содержит качественное, проверяемое утверждение (линейный
+  рост числа кандидатов, отсутствие новых imports в View graph), количественная
+  проверка (`bundle:budget`) относится к код-ревью.
+
+## Вердикт
+
+ТЗ полное, обязательные разделы на месте, диагноз части 1 подтверждён чтением
+кода, риски и инварианты части 1 закрыты явно. Но AC4 (и опирающийся на неё
+раздел 3) содержит невыполнимое буквально требование из-за смешения
+render-масштаба (`gridPitch` ≈4.1667) и normalised-масштаба (`GRID_STEP_N`
+≈0.0041667) constant, в котором фактически хранятся decor-координаты — при
+реализации «как написано» вторая часть задачи (клавиатурная доводка) ломается
+на первом нажатии стрелки, и это ровно то расхождение, которое несколько раз
+уже стоило продукту дефекта («одно число — один источник»). Находка технической
+природы, снимается автором правкой текста ТЗ без обращения к владельцу.
+
+**Вердикт: красный · заход r1 · блокирующих циклов 1/4 · High: 1 · Medium: 0 →
+в задаче**
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `issue/447-exterior-snap-keyboard`, коммит `a1fbea07d287` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Дерево материала: `dd1a85b41c1caba79488df331dd813f5b7522dd0`
+  ```
+  git log --all --format='%H %T' | grep dd1a85b41c1c
+  ```
+- ТЗ `docs/specs/447-exterior-furniture-snap-keyboard-nudge.md`, блоб `5003f5995be9dd06e296a8c0056d63c1d21a8fc6`
+  ```
+  git log --all --find-object=5003f5995be9dd06e296a8c0056d63c1d21a8fc6 -- docs/specs/447-exterior-furniture-snap-keyboard-nudge.md
+  ```