docs: review document for #383

Issue: #383
User-Visible: no

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

@@ -0,0 +1,221 @@
+# SPEC-REVIEW-383-r1
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/383
+- **Этап:** ревью ТЗ (PROCESS.md §2.4)
+- **Заход:** r1 · блокирующих циклов израсходовано 0 из 4 (полный трек, лимит 4)
+- **Материал:** `docs/specs/383-furniture-transform.md`, ревизия 1, коммит
+  `aa1b9e39` (`docs: specify furniture transforms`), ветка
+  `issue/383-furniture-transform`.
+- **Вердикт: зелёный**
+
+## Скоуп ревью
+
+Первый заход — разбор полный, дельты нет. Проверялись: соответствие
+`docs/SCOPE.md`, наличие всех обязательных разделов §7.1, однозначность и
+доказуемость каждого AC1–AC13, отсутствие непомеченных догадок о поведении,
+согласованность с каноническими документами подсистемы (`CANVAS.md`,
+`FURNITURE.md`, `UX-MODES.md`, `CONFIG-COMPATIBILITY.md`, `TOUCH-SUPPORT.md`,
+`USER-GUIDE.ru.md`) и — там, где ТЗ описывает *текущую* реализацию как базу для
+дельты — сверка этих утверждений с фактическим кодом на `aa1b9e39`.
+
+## Как проверялось
+
+1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` целиком.
+2. Прочитано тело issue #383 и все шесть комментариев (аналитика, два решения
+   владельца по вопросам, уточнение про галочки отражения, взятие в работу,
+   готовность ТЗ). Открытых продуктовых вопросов на момент ревью нет — владелец
+   явно закрыл оба вопроса аналитики (Q1: свободный поворот без `Shift`, 45° с
+   `Shift`; Q2: crossing через ноль разрешён; Q3: расширение hit-area на
+   физические 10 см от штриха, а не весь bounding box).
+3. Прочитан `docs/specs/383-furniture-transform.md` целиком.
+4. Прочитаны канонические документы: `docs/FURNITURE.md`, фрагменты
+   `docs/CANVAS.md`, `docs/UX-MODES.md`, `docs/TOUCH-SUPPORT.md`,
+   `docs/CONFIG-COMPATIBILITY.md`, соответствующие разделы
+   `docs/USER-GUIDE.ru.md` (мебель, толстые стены/`invert`, Optimize).
+5. Технические утверждения ТЗ и аналитики сверены с кодом на `aa1b9e39`
+   (не для оценки реализации — реализации ещё нет, — а чтобы убедиться, что
+   база, от которой считается контракт, не выдумана):
+   - `resizeDecorBox()` (`src/editors/decor/geometry.ts:222`) действительно
+     получает `step`/`minSize` и округляет обе оси к `gridPitch` —
+     подтверждает «Проблема» и корректность выбранной base-line для AC1/AC3.
+   - Общий поворот (`_dtMove`, `src/houseplan-editor-runtime.ts:4596-4601`)
+     сейчас округляет к `DT_ANGLE_STEP` (5°) без `Shift` и свободен с `Shift`
+     для всех decor kinds одной веткой — подтверждает нужность отдельной ветки
+     для мебели (AC4) и корректность «остальные decor kinds без изменений».
+   - `pointer-events: visiblePainted` на `.dshape` в режиме decor
+     (`src/styles/plan.styles.ts:665-667`) и увеличенный select hit-path
+     `.dselecthit` только у `<line>` (`plan.styles.ts:719-729`,
+     `houseplan-card.ts:8130`) — подтверждает «у мебели своего hit-path нет» и
+     что образец для «прежнего hit contract» существует только для линии.
+   - Существующий Save-обработчик свойств декора
+     (`houseplan-editor-runtime.ts:4459-4466`) действительно вызывает
+     `snapToGrid` для `rect|ellipse|furniture` одной веткой — AC5.5 корректно
+     называет цель правки (убрать вызов только для furniture).
+   - Дисплей/клэмп числового поля размера в диалоге свойств
+     (`_decorLargeField`/`_decorLargeCm`, `houseplan-card.ts:7618-7625`) уже
+     имеет пол 0,1 см — заявленный в ТЗ технический минимум (§3) не
+     противоречит существующему клэмпу этого диалога. Отдельный `FURN_MIN_CM=1`
+     в `src/furniture.ts:309` относится к другому пути — палитре *новой*
+     мебели перед стемпом (`_furnFieldToCm`, используется только для
+     `_furnPalette.w/h`), а не к диалогу свойств уже размещённого объекта; путаницы
+     между двумя путями в тексте ТЗ нет.
+   - Backend `_FURN_SIZE = vol.Range(min=0.0000001, max=CANVAS_LIMIT)`
+     (`custom_components/houseplan/validation.py:1314`) — совместим с новым
+     0,1-см полом (он строже, чем текущий backend-минимум, значит не открывает
+     новый диапазон, отклонённый сейчас).
+   - `flip_h`/`flip_v` уже существуют в схеме — но для `openings`
+     (`validation.py:1631-1633`, `PASSAGE_FORBIDDEN_FIELDS` line 728,
+     `import_export.py:280-284`) с другим смыслом: не отражение размера, а
+     направление створки/направление ворот (`USER-GUIDE.ru.md:721-722`, «Флаг
+     „Открывается в другую сторону“»). Коллизии по факту нет — decor и openings
+     обрабатываются раздельными циклами в `coordinate_canonicalization.py:143-159`
+     и раздельным allowlist-кодом в `import_export.py`, поэтому смешения полей
+     не будет. Отмечено ниже как Low-наблюдение, не блокирует.
+6. `docs/specs/README.md` — задача добавлена; ссылки issue↔ТЗ на месте в обе
+   стороны (issue → блоб файла в комментарии автора, файл → issue в шапке).
+
+## Проверка §7.1 (обязательные разделы)
+
+Все обязательные разделы присутствуют и в правильном порядке: **Сценарий**
+(персона — Home admin, десктоп, Редактор подложки, момент — подгонка размера
+мебели под реальный объект) → **Что человек увидит до/после** (одной фразой,
+без терминов реализации) → Проблема → Скоуп/Не-скоуп → Контракт поведения
+(7 подсекций) → История/перенос/совместимость → UX/i18n → Затронутые файлы →
+Критерии приёмки AC1–AC13 с доказательством для каждого → План автотестов →
+Риски → Откат → Производительность и безопасность → Release-артефакты →
+Принятые предположения. Два продуктовых раздела не описывают реализацию —
+проверено.
+
+## Проверка AC1–AC13
+
+Каждый AC — проверяемое утверждение с названным способом доказательства
+(`unit`/`smoke`/`golden`/`backend`/`DOM`/`integration`/ручной скриншот, где это
+уместно — AC8 сознательно называет «manual screenshot» для курсора, что
+корректно: программно проверить визуальный SVG data-URI cursor можно только
+косвенно). Ни один AC не описывает реализацию вместо наблюдаемого контракта.
+Мутанты в «Плане автотестов» покрывают все 13 AC по одному-два мутанта на
+критерий — сцепка AC↔мутант явная, а не общая фраза «тесты будут».
+
+Не найдено ни одного AC, доказательство которого требовало бы теста, не
+умеющего упасть (например, AC3 «crossing и minimum» явно требует таблицы по
+осям/углам, а не общего «works»).
+
+## Проверка на непомеченные догадки
+
+Единственные места, где ТЗ фиксирует поведение, которое не следует напрямую из
+явного решения владельца, вынесены в «Принятые предположения» и помечены как
+таковые (масштаб crossing на обеих осях одновременно; false как каноническое
+отсутствие; точный ноль как невалидное transient-состояние; 10 см — от внешней
+границы уже видимого stroke, а не от centerline). Все четыре — технические, не
+продуктовые, и ревьюер вправе их принять или оспорить без обращения к
+владельцу (PROCESS.md §7.1). Возражений по существу нет: все четыре
+согласуются с §3/§6/§7 контракта и с уже принятым в проекте паттерном
+«optional-флаг, absence = историческое поведение» (`CONFIG-COMPATIBILITY.md`,
+многократно, напр. `marker.value_source`, `space.zero_wall_style`).
+
+Продуктовых вопросов, которые следовало задать владельцу, но не задали, не
+найдено — оба вопроса из аналитики (модификатор поворота, поведение hit-area)
+уже закрыты явными решениями владельца в комментариях, а не додуманы автором.
+
+## Находки
+
+### Low-1 — i18n-ключи галочек отражения не в том namespace
+
+`furn.flip_h`/`furn.flip_v` (спецификация, раздел UX/i18n) попадут в диалог
+свойств **уже размещённого** объекта (там же, где `decor.size`, `decor.angle`,
+`decor.fill` — все ключи этого диалога, включая существующие поля размера
+мебели, уже используют namespace `decor.*`, см. `src/i18n/ru.json:520-521` и
+Save-обработчик `houseplan-editor-runtime.ts:4459` — furniture лежит в одной
+ветке с rect/ellipse). Namespace `furn.*` в проекте зарезервирован за диалогом
+**палитры новой** мебели (`furn.title`, `furn.width`, `furn.depth`,
+`furn.pick_hint` и т.д., `src/i18n/ru.json:907-1002`) — другой диалог, другой
+момент взаимодействия. Ключи галочек стоит назвать `decor.flip_h`/
+`decor.flip_v`, чтобы не заводить третий смешанный источник именования в одном
+диалоге.
+
+**Почему Low, не Medium:** чисто наименование, не влияет ни на один AC, чинится
+переименованием двух ключей в четырёх словарях без побочных эффектов.
+
+**Решение ревьюера:** не блокирует, правится при реализации без возврата на
+повторное ревью ТЗ; если автор оставит `furn.*` — тоже не дефект AC, только
+стилистическая непоследовательность, которую тогда фиксируем без действия.
+
+### Low-2 — переиспользование имён `flip_h`/`flip_v` без ссылки на существующий смысл
+
+Поля `flip_h`/`flip_v` уже существуют в backend-схеме для `openings` с иным
+значением — направление створки двери/окна (`docs/USER-GUIDE.ru.md:721-722`,
+`validation.py:1631-1633`). Раздел «Рекомендуемая модель» (комментарий
+аналитики) и раздел 6 ТЗ вводят те же имена для мебели как будто с нуля, не
+упоминая этот прецедент. Функциональной коллизии нет: `decor` и `openings`
+обрабатываются раздельными циклами в `coordinate_canonicalization.py` и
+раздельным allowlist-кодом в `import_export.py`, поля физически в разных
+записях. Это наблюдение, а не риск для реализации.
+
+**Почему Low:** не блокирует ни один AC и не создаёт технического противоречия
+— два независимых пространства имён случайно тёзки. Стоит одной строкой
+упомянуть в `docs/CONFIG-COMPATIBILITY.md` при обновлении (AC12 всё равно
+трогает этот файл), чтобы будущий читатель не тратил время на тот же вопрос,
+который потратил ревьюер.
+
+**Решение ревьюера:** снимается с записью; не требует правки ТЗ или кода.
+
+## Что проверено и корректно
+
+- Трек `full` обоснован по §5: новый UX-контракт (реверс модификатора
+  вращения для мебели, новые ручки, отражение), задета сохраняемая
+  модель/backend-схема, публичный контракт (знаковые размеры в свойствах) —
+  минимум три из пяти критериев лёгкого трека нарушены одновременно, что и
+  требуется для отказа от `small`.
+- Скоуп/не-скоуп разделены чётко и по кодовой границе: явно исключены
+  rect/ellipse/text/backdrop, wall magnet, размещение, скос/деформация линий
+  символа, touch UX. Это соответствует «Отдельный furniture-путь без изменения
+  rect/ellipse/text/backdrop» и снижает риск регрессии на общем контроллере.
+- Persisted-модель (§6) — обратимо-добавочная, `false` не материализуется,
+  старый рендер/бэкенд не ломается — соответствует установленному в проекте
+  паттерну optional-полей (`CONFIG-COMPATIBILITY.md`).
+- AC10 корректно требует отклонения не-boolean для `flip_h`/`flip_v` и
+  сохранения отказа backend на неположительных `w/h` — это уже сегодняшнее
+  поведение `_FURN_SIZE`, и ТЗ его не трогает, только добавляет два optional
+  boolean рядом.
+- Область выбора (§7) осознанно отличается от готового паттерна `.derasehit`/
+  `.dselecthit` (экранные 16px, `vector-effect: non-scaling-stroke`): ТЗ
+  требует физические 10 см, зависящие от `cell_cm`/zoom, и в «Рисках» отдельно
+  прописан план на случай, если один stroked path не даёт корректный офсет при
+  неравномерном масштабе символа. Это не оставлено на «как получится».
+- i18n-таблица (не считая Low-1) содержит все четыре словаря сразу, без
+  «дозаполним EN/RU, а DE/FR потом».
+- Откат описан предметно: что можно откатить (frontend UX/render), что нельзя
+  без отдельной data-fix (снятие полей), и явно запрещён «слепой одновременный
+  revert frontend+backend после публичного сохранения флагов» — типичное место,
+  где специи обычно останавливаются на «миграции нет» и не договаривают.
+
+## Чего не проверял
+
+- **Не проверялся код** — его не существует на этот SHA (задача ещё в
+  `S3-spec`/`S4-spec-review`, до `S5-ready`). Гейты `typecheck`/`test`/`build`
+  на этом этапе неприменимы: нечего собирать.
+- Не проверялась реализуемость «дискриминации доминирующей оси» (§1.4) на
+  реальных furniture-символах с сильно неравномерным aspect ratio за пределами
+  чтения формулы — это войдёт в код-ревью вместе с unit-таблицей.
+- Не проверялся визуальный контраст новых средних ручек в тёмной теме —
+  проверяемо только golden-эталоном, которого нет до реализации.
+- Не проверялась точность SVG data-URI курсора вращения — AC8 сам называет
+  способ доказательства «manual screenshot», то есть не автотест; это
+  сознательное решение автора, не пробел ревью.
+
+## Гейты этого ревью
+
+Ревью ТЗ гейтов сборки не требует (нет продуктового кода на этом SHA).
+Прогонялся только `git`-осмотр репозитория (чтение файлов, `git rev-parse
+HEAD`); ни один build/test/typecheck не запускался и не нужен для этого этапа.
+
+## Унаследовано из r<N-1>
+
+Не применимо — это первый заход (r1), возвратов на правки не было.
+
+---
+
+**Вердикт: зелёный · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 0**
+
+Обе находки — Low, обе решены ревьюером на месте (не блокируют, не требуют
+повторного цикла). Issue может перейти в `S5-ready`.