From 4a83bd054045d3414894ffa4d15e77d89460ad10 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sun, 30 Aug 2026 08:06:19 +0000 Subject: [PATCH] docs: review document for #383 Issue: #383 User-Visible: no --- docs/reviews/SPEC-REVIEW-383-r1.md | 221 +++++++++++++++++++++++++++++ 1 file changed, 221 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-383-r1.md diff --git a/docs/reviews/SPEC-REVIEW-383-r1.md b/docs/reviews/SPEC-REVIEW-383-r1.md new file mode 100644 index 00000000..b0a9073e --- /dev/null +++ b/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` только у `` (`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`.