From 1ce62613edc6786b1024a3776db3ab004f3b565a Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Fri, 4 Sep 2026 14:46:36 +0000 Subject: [PATCH] docs: review document for #449 Issue: #449 User-Visible: no --- docs/reviews/SPEC-REVIEW-449-r1.md | 223 +++++++++++++++++++++++++++++ 1 file changed, 223 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-449-r1.md diff --git a/docs/reviews/SPEC-REVIEW-449-r1.md b/docs/reviews/SPEC-REVIEW-449-r1.md new file mode 100644 index 00000000..0eb79f3e --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-449-r1.md @@ -0,0 +1,223 @@ +# SPEC-REVIEW — issue #449 · заход r1 + +- Issue: https://github.com/Matysh/houseplan-card/issues/449 +- Этап: ревью ТЗ (PROCESS.md §2.4) +- ТЗ: `docs/specs/449-double-fit-all.md` +- Материал: ветка `issue/449-double-fit-all`, SHA `5847bf2b1bb6e6168f6727b26470c28f7f2650ec` +- Трек: полный (issue не помечен `small`); лимит циклов ревью ТЗ — 4 (§2.4) +- Заход r1, блокирующих циклов израсходовано до этого вердикта: 0/4 + +## Скоуп проверки + +Диапазон `git diff origin/dev...HEAD` — два файла, оба класса C: + +``` +docs/specs/449-double-fit-all.md | 412 +++++++++++++++++++++++++++++++++++++++ +docs/specs/README.md | 1 + +``` + +Продуктовый код не менялся (класс A/B пуст в этом диффе) — это чистый этап +«ТЗ в работе → ТЗ на ревью», код-гейты (`typecheck`/`test`/`build`) к этому +раунду не относятся и не прогонялись: разбор — это чтение ТЗ, тела issue и +сверка каждого фактического утверждения ТЗ с действующим кодом/документами. + +## Как проверялось + +1. `docs/SCOPE.md` — мандат и Core user jobs. +2. `AGENTS.md`, `PROCESS.md` целиком (включая §2.4, §2.10, §5, §7.1, §12) — + процесс, обязательные разделы ТЗ, правила Medium в/вне скоупа. +3. Тело issue #449 и все 6 комментариев (`gh issue view 449 --comments`) — + восстановлена хронология Q1/Q2/Q3 и финальное решение владельца. +4. `docs/specs/449-double-fit-all.md` целиком. +5. `docs/CANVAS.md` §5, `docs/TOUCH-SUPPORT.md`, `docs/UX-MODES.md` — + канонические документы подсистемы camera/gesture/touch. +6. `docs/USER-GUIDE.ru.md` §6 (терминология интерфейса) и `src/i18n/ru.json`. +7. Сверка утверждений ТЗ о «текущем поведении по коду» с самим кодом: + `src/houseplan-card.ts` (`_stagePointerUp`, `_stagePointerDown`, `_fitAll`, + `_resetZoom`, `_lastTap`, `_swipeStart`, `_startCameraTransition`), + `src/room-fit.ts` (`roomFitOwnerFromPath`, `acceptedRoomFitGesture`, + `ROOM_FIT_INTERACTIVE_OWNER`), `src/viewport-transition.ts` + (`CameraTransitionReason`, `sameCameraState`, `CameraTransitionController`). +8. Существование всех файлов, упомянутых в «Плане автотестов» и AC + (`demo/smoke_kiosk.mjs`, `demo/smoke_kiosk_pan_lock.mjs`, + `demo/smoke_room_fit.mjs`, `demo/smoke_editor_gestures.mjs`, + `demo/smoke_smooth_zoom.mjs`, `test/room-fit.test.mjs`, + `scripts/mutation-gate.mjs`, `npm run bundle:budget`). +9. Трейлеры коммита `5847bf2b` (`git show -s --format=full`). +10. `docs/specs/README.md` — ссылка issue↔ТЗ в обе стороны. + +## Продуктовая рамка + +Job — J1 «Show the whole home and what's happening right now»: быстрый +возврат к обзору всего дома после zoom/pan — прямая навигационная надобность +внутри уже закрытого job, не новая функциональность вне SCOPE.md. Персона — +Household member/Guest на View, Home admin на kiosk-стенде; оба уже описаны +как основная аудитория View/kiosk (TOUCH-SUPPORT.md: «touch-first product in +View and kiosk»). В скоупе. + +## Находки + +### Medium (в скоупе — чинится в этом ТЗ, без него — жёлтый вердикт) + +**M1. Нет обязательной строки `Touch editor: …`.** + +`docs/TOUCH-SUPPORT.md` §«Documentation rule»: «New editor feature +specifications and code reviews must state one of: `Touch editor: +supported` / `best effort / intentionally degraded` / `not exposed`». +ТЗ #449 явно исключает редакторы из скоупа (раздел «Не-скоуп», п.4; таблица +§6 «Режимы» — План/Устройства/Подложка: «Нет нового действия»), но нигде не +формулирует это классификацией `Touch editor: not exposed`, хотя это +установленная практика проекта даже для задач, вообще не трогающих +редакторы — см. `docs/specs/229-merge-collinear-partitions.md:8`, +`docs/specs/230-hatch-density-normalization.md:7`, +`docs/specs/302-junction-node-material.md:7`, +`docs/specs/220-space-tab-reorder.md:156`, +`docs/specs/243-space-tab-drop-target.md:184` — везде одна явная строка +рядом с обоснованием. + +**Воспроизведение:** `grep -in "touch editor" docs/specs/449-double-fit-all.md` +→ пусто. + +**Почему это находка, а не придирка к форме:** пункт DoR (§2.5) требует +«влияние на touch по `docs/TOUCH-SUPPORT.md` (View и киоск — блокирующие)» +явно названным. Сейчас читатель должен сам собрать этот вывод из трёх разных +мест ТЗ (не-скоуп, таблица режимов, раздел UX); одна строка закрывает вопрос +однозначно и без интерпретации, как это сделано в дюжине предыдущих ТЗ. + +**Как чинится:** добавить одну строку, например «`Touch editor: not +exposed` — задача не добавляет и не убирает ни одного жеста в Плане, +Устройствах и Подложке; двойной жест ограничен `_mode === 'view'` +(включает kiosk)». + +### Вне скоупа — заведён отдельный issue + +**Терминология кнопки «Вписать всё» vs «Показать всё».** ТЗ #449 корректно +использует «Вписать всё» — это буквальный текст тултипа (`title.zoom_fit` в +`src/i18n/ru.json`) и терминология `docs/CANVAS.md`. Но +`docs/USER-GUIDE.ru.md` §6 (три места: строки 270, 289, 297) называет ровно +ту же команду `_fitAll()` «Показать всё» — расхождение, судя по всему, +попавшее из формулировки записи `v1.70.0-beta.1` (#82) в +`docs/CHANGELOG.ru.md:271`. Поскольку `docs/USER-GUIDE.ru.md` — канонический +источник интерфейсной терминологии (AGENTS.md), а сам документ противоречит +себе (гайд ↔ тултип/CANVAS.md), это самостоятельный дефект документации, не +созданный и не обязанный чиниться в ветке #449 (задача не трогает эту +формулировку намеренно). Заведён +[#452](https://github.com/Matysh/houseplan-card/issues/452) со ссылкой на +#449, метки `docs`, `P3`, `S1-new`. + +### Low + +Нет находок уровня Low после разбора: единственный технический вопрос, +который я бы поставил под сомнение (общее окно 350 мс для мыши и тач, +«Принятые предположения» п.2), явно помечен автором как предположение, +свободное к пересмотру ревьюером без цикла — 350 мс лежит в обычном +диапазоне порогов ОС/браузера для double-click, менять не вижу оснований. +Снимаю без правки. + +## Что проверено и корректно + +- **Продуктовые решения Q1/Q2 зафиксированы и непротиворечиво перенесены в + ТЗ.** История комментариев: Q1 сначала переоткрыт в пользу альтернативы + («жест работает и по комнате»), затем явным комментарием владельца + возвращён к default (только свободный фон), Q3 закрыт как потерявший + предмет. Раздел «Контракт поведения» §5 и таблица §6 корректно отражают + именно финальное решение, а не промежуточное. +- **Заявления о «текущем поведении по коду» проверены построчно и точны:** + `_lastTap`/`_swipeStart` действительно живут только внутри + `if (this._kiosk) { … }` в `_stagePointerUp` (`src/houseplan-card.ts:6951-6988`); + stage не имеет `dblclick`-обработчика (единственные `@dblclick` в файле — + на decor-фигурах Background editor и backdrop-диалоге); `acceptedRoom` + вычисляется до kiosk-ветки и не обновляет `_lastTap`, что подтверждает + «room-owned tap намеренно не входит в kiosk double-tap sequence» — + совпадает и с `docs/CANVAS.md:294-295` («room-owned taps never enter the + free-background double-tap sequence»). +- **`_fitAll()` действительно принимает только `'fit' | 'home'` сегодня** + (`src/houseplan-card.ts:6318`), а `CameraTransitionReason` в + `src/viewport-transition.ts:10` уже включает `'double-tap'` — ТЗ корректно + формулирует требуемое расширение сигнатуры `_fitAll`, а не выдаёт его за + существующий факт. +- **Заявление о no-op на совпадающий target подтверждено кодом:** + `_startCameraTransition` (`src/houseplan-card.ts:1156-1173`) содержит два + явных ранних выхода на `sameCameraState`, ровно то, что описывает + контракт §1 п.7. +- **Список исключений «не свободный фон» (§2) не расходится с + `ROOM_FIT_INTERACTIVE_OWNER`** в `src/room-fit.ts:36-40` — совпадает по + составу (`.dev`, `.vacpuck`, `.oplock`, `.op-hit`, `.opening`, `.rlgo`, + `a/button/input/select/textarea`, `role=link/button`, + `[data-room-fit-block]`) плюс `.roomlabel`/`[data-hp="room"]`, + корректно добавленные как «room owner», не «свободный фон». Требование + «один источник selector list для room-fit и нового жеста» — правильная и + проверяемая техническая директива. +- **Режим `view` действительно общий для View и kiosk на уровне + `this._mode`** (`private _mode: 'view' | 'plan' | 'devices' | 'decor'`, + kiosk — отдельный булев флаг) — формулировка §3 п.2 «режим на всём жесте — + `view`» корректно покрывает оба поверхностных режима одним условием. +- **Все файлы, названные в «Плане автотестов» и в AC1–AC11 как + доказательства, существуют** (перечислены выше в «Как проверялось» п.8) — + ни одной ссылки на несуществующий смок/скрипт. +- **Обязательные разделы §7.1 присутствуют все**: сценарий, что человек + увидит до/после, проблема, скоуп/не-скоуп, контракт поведения, UX, модель + данных/миграция/compatibility/i18n, AC1…AC11 с доказательством, план + автотестов, риски, откат, release-артефакты. +- **AC пронумерованы, у каждого указан способ доказательства** (unit / mutation + / production-bundle smoke), включая явные негативные AC (AC4–AC8) — + ни одного «размытого» критерия без проверяемого исхода. +- **Блок «Принятые предположения» отделяет техническое от продуктового** + корректно: ни один пункт там не является продуктовым решением, которое + требовалось бы спрашивать у владельца (только технические — Pointer + Events vs dblclick, отсутствие нового spatial threshold, разделение + modality, отсутствие таймера, вынос selector list, единый reason). +- **Трейлеры коммита корректны:** `Issue: #449`, `User-Visible: no` — верно + для чисто спецификационного коммита без изменения поведения. +- **Ссылка issue ↔ ТЗ на месте в обе стороны:** заголовок ТЗ ссылается на + issue, `docs/specs/README.md` получил новую строку с корректной ссылкой на + файл. +- **Golden/screenshots корректно исключены**: «Golden baseline не должен + меняться» и «Новых UI screenshots не требуется» — верно, статический кадр + View не меняется (только динамика жеста). +- **Formal DoR-пункты** (i18n: явно «нет новых строк», compatibility: явно + «нет новых полей», перф: явно назван бюджет O(длина composed path), откат: + явно описан) все закрыты явными утверждениями, а не молчанием. + +## Чего не проверял и почему + +- **Реализацию** — её нет, диапазон диффа не содержит класса A/B; проверять + нечего до `S5-ready`. +- **`npx tsc --noEmit` / `npm test` / `npm run build`** — не гоняла: раунд не + меняет ни одной строки кода, только `docs/**` (класс C), эти гейты + относятся к код-ревью, а не к ревью ТЗ. +- **`node scripts/check-docs.mjs`** — не гоняла: диапазон не трогает + `src/**`, отпечаток скриншотов документации не мог устареть от этого + диффа. +- **Достижимость всех связанных issue (#82, #152, #183)** — не перечитывала + их код заново сверх того, что нужно для проверки конкретных фактических + утверждений ТЗ (см. выше); ссылки как «уже сделано/подтверждено» приняты + там, где сверены с текущим кодом, и не расширялись дальше необходимого. +- **Возможные будущие AC-конфликты с #82/#152 после реализации** — предмет + код-ревью, не ТЗ: на этапе спецификации проверяется непротиворечивость + контракта, а не факт его будущей корректной реализации. + +## Вердикт + +High: 0 · Medium в скоупе: 1 (M1, см. выше) · Medium вне скоупа: 1 → заведён +[#452](https://github.com/Matysh/houseplan-card/issues/452). + +Без High это жёлтый вердикт (PROCESS.md §2.4, §12): ТЗ возвращается автору на +правку M1, повторный цикл — по дельте (§2.10), лимит цикла израсходован +1/4. + +--- + + + +## Материал раунда + +- Ветка: `issue/449-double-fit-all`, коммит `5847bf2b1bb6` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `0254c870c60fdf2ba6d000a7c0586607a9bf4e52` + ``` + git log --all --format='%H %T' | grep 0254c870c60f + ``` +- ТЗ `docs/specs/449-double-fit-all.md`, блоб `3abb3ba5bb81f1c15be272ddbf1fa858f0e6adb5` + ``` + git log --all --find-object=3abb3ba5bb81f1c15be272ddbf1fa858f0e6adb5 -- docs/specs/449-double-fit-all.md + ```