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

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

Не применимо — это первый заход (r1), возвратов на правки не было.

---

Вердикт: зелёный · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 0

Обе находки — Low, обе решены ревьюером на месте (не блокируют, не требуют повторного цикла). Issue может перейти в S5-ready.