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

SPEC-REVIEW-691-r1

• Issue: https://github.com/Matysh/houseplan-card/issues/691
• Этап: S4-spec-review (ревью ТЗ, PROCESS.md §2.4)
• Трек: полный. Автор сам зафиксировал это в комментарии «Аналитика»: «лёгкий трек: нет», P1, ценность 9/10, сложность и риск 7/10. Разбор по существу подтверждает выбор: задача меняет публичный touch-контракт View и kiosk (владение жестом между pan/swipe и room-fit/double-fit), затрагивает несколько существующих конкурентов (#449, #563, #578, #531, #152) — не мелкая правка одной поверхности.
• Материал: тело issue #691, раздел ## ТЗ, плюс пять комментариев владельца: (1) «Аналитика» — оценка, дубликаты (нет), скоуп (J1), подтверждение по коду (_panLock, _swipeZone, DoubleFitGestureRecognizer, planGestureOwnerFromPath); (2) «Взял: автор ТЗ»; (3) «Вопросы к ТЗ» — три продуктовых вопроса Q1–Q3 с default/альтернативами; (4) «Решения владельца зафиксированы» — Q1 default, Q2 альтернатива, Q3 default, blocked снят; (5) «ТЗ готово» — публикация текста в теле issue. Открытых продуктовых вопросов на момент ревью нет.
• Заход: r1 · блокирующих циклов израсходовано 0 из 4
• Роль: ревьюер ТЗ (не автор)

Скоуп ревью

Арбитраж touch/mouse/pen-жестов в View и kiosk на плане с несколькими пространствами: (1) новый контракт разделения pan и edge-swipe (переключение пространства только из внутренней 48 px краевой зоны, финальность выбора owner до terminal event, отмена вторым пальцем); (2) расширение double-tap/ double-click «Вписать всё» с единственно доступной свободной поверхности на room fill и room label, с задержкой одиночного room-fit на 350 мс и переходом в fit-all без промежуточного кадра камеры. Не-скоуп по тексту ТЗ: настройки жестов, анимация _slideTo()/camera-fit геометрия, View-swipe вне kiosk, жесты редакторов, действия устройств/проёмов/лестниц/ссылок/декора, pinch-zoom/long-press/room-fit-границы/сохранение zoom, backend/WS/config/ i18n, постоянные edge-подсказки.

SCOPE-проверка (docs/SCOPE.md): задача закрывает J1 («at a glance» — надёжный обзор многоэтажного дома) и защищает release-blocking touch-контракт View/kiosk (docs/TOUCH-SUPPORT.md: «View... must support convenient pan, pinch zoom and space switching»; «A touch-only failure in View is a product defect, not an accepted limitation»). Trailer Touch editor: not exposed поставлен корректно — редакторы не получают новый контракт. Ничего из «никогда не строить» (docs/SCOPE.md «Out of scope») не задевается: новых UI-элементов, настроек или геометрии нет.

Как проверялось

1. Прочитаны docs/SCOPE.md, AGENTS.md, docs/process/REVIEWER.md целиком; по ссылкам конспекта открыт PROCESS.md §2.4, §2.5, §4, §7.1, §7.2 (§2.10 не применяется — это r1).
2. Прочитано тело issue #691 целиком (## Проблема, ## Как проявляется, ## Что подтверждено по актуальному коду, ## Ожидаемое поведение, ## Связанные задачи, ## Примечание, затем ## ТЗ §1–§18) и все пять комментариев владельца — сверено, что решения Q1–Q3 из комментария «Решения владельца зафиксированы» дословно попали в §6 п.1, §7 п.3–4 и §7 п.9 итогового текста ТЗ; расхождений не найдено.
3. Сверены обязательные разделы §7.1 в теле issue: Сценарий (§1) → Что человек видит до/после (§2) → Проблема и цель (§3) → Скоуп (§4) / Не-скоуп (§5) → Контракт поведения (§6 pan/edge-swipe, §7 room-fit/double-fit) → UX, доступность, ошибки (§8) → Модель данных/миграция (§9) → i18n (§10) → Производительность (§11, не обязательный, но уместный) → AC1–AC12 с доказательством (§12) → План автотестов (§13) → Затронутые модули (§14) → Риски (§15) → Откат (§16) → Release-артефакты (§17) → Принятые предположения (§18) — все обязательные разделы на месте и в осмысленном порядке.
4. Технические утверждения сверены с реальным кодом на 1751184b (нет ветки issue/691-*, диф пуст — код для #691 ещё не начат, ожидаемо для этапа spec):
   • src/houseplan-card.ts:1912 — private _panLock: 'pan' | 'swipe' | null существует; :6739 — геттер _swipeZone существует с описанным условием (kiosk, swipeTarget-зум, несколько пространств); :6832–6834 — классификация на первом движении > STAGE_TAP_DISTANCE_PX по правилу |dx| > |dy| * 1.5, :6900–6902 — release не переспрашивает swipeTarget() при _panLock === 'pan'. Всё описанное в «Что подтверждено по актуальному коду» и в §6 ТЗ как текущее поведение — точно.
   • src/room-fit.ts:104–106 — DOUBLE_FIT_WINDOW_MS = 350, STAGE_TAP_DISTANCE_PX = 8 существуют как названные константы; :121–136 — planGestureOwnerFromPath() действительно возвращает background только для узлов, дошедших до .stage без interactive/room-owner на пути, подтверждая утверждение issue «распознаватель принимает только owner background»; :151–165 — beginDoubleFitPointer() сегодня жёстко требует input.owner.kind !== 'background' → null, то есть room-owner действительно не участвует — ТЗ корректно называет это ограничение, которое §7 п.2–4 и §14 (src/room-fit.ts) просят снять.
   • docs/CANVAS.md §5 «Zoom and pan» подтверждает текущий контракт слово в слово (владение pointer, _panLock, финальность лока «audit DEV-1DA1-02», free-background double-tap #449, room-fit #152) — ТЗ §17 корректно называет этот документ для обновления, ничего не упущено и не выдумано.
   • docs/TOUCH-SUPPORT.md:42–43 фиксирует именно старое поведение («one tap on a room keeps the immediate room-fit action»), которое §7 ТЗ сознательно меняет (задержка 350 мс, решение владельца Q2-альтернатива) — обновление этого канона корректно названо в §17 release-артефактов.
   • Файлы из §13/§14: test/room-fit.test.mjs, demo/smoke_kiosk.mjs, demo/smoke_room_fit.mjs — существуют. demo/smoke_kiosk_pan_lock.mjs (уже существующий смоук именно на «curved gesture»/финальность лока, сценарий audit DEV-1DA1-02) в списке файлов по имени не назван, но содержательно покрыт пунктом плана автотестов «curved path» (§13 п.4) — не считаю это отдельной находкой, автор явно не выбрасывает и не ослабляет существующую позитивную проверку (§13 п.7 прямо это запрещает).
5. Контракт §6 (pan/edge-swipe) и §7 (room-fit/double-fit) прочитаны пункт за пунктом на внутреннюю непротиворечивость и на покрытие каждым AC1–AC12; расхождений «в контракте есть, а AC нет» не найдено, кроме одной находки ниже.
6. Числовая согласованность: 48 px (edge-зона), 8 px (STAGE_TAP_DISTANCE_PX, переиспользуется, не переопределяется), 350 мс (DOUBLE_FIT_WINDOW_MS, одно и то же окно используется и для парного double-tap, и для задержки одиночного room-fit) — три числа согласованы между собой, между §6/§7 и §18 «Принятые предположения», и совпадают с уже существующими именованными константами в коде (п.4 выше). Расхождений не найдено.
7. Гейты (tsc --noEmit, npm test, npm run build, check-docs.mjs, смоки, инварианты) не прогонялись: этап spec, ветки/коммитов для #691 нет, git diff origin/dev...HEAD пуст — гейты неприменимы к тексту ТЗ, а не пропущены.

Находки

Medium — направление edge-swipe не квантифицировано и не отмечено как допущение

• Файл: тело issue #691, раздел ## ТЗ §6 «Контракт pan и edge-swipe», пункт 6.
• Что не так: «Начатый в краевой зоне жест становится swipe только после существующего порога движения и достаточного преобладания правильного горизонтального направления» — фраза называет поведение (классификация диагонального жеста внутри краевой зоны), но не называет число. Формулировка явно отличает «существующий порог движения» (8 px, переиспользуется) от преобладания направления, для которого слово «существующий» не сказано — то есть не ясно, сохраняется ли текущее отношение |dx| > 1.5·|dy| (src/houseplan-card.ts:6833), меняется ли оно теперь, когда его работу частично взяла на себя краевая зона, или выбирается заново. §18 «Принятые предположения» называет число для 48 px, 8 px и 350 мс, но не для этого порога — то есть ровно тот случай, когда «утверждение о поведении, которого нет ни в одном документе и которое не помечено как предположение», должен фиксировать ревьюер. AC2/AC3 (§12) тестируют только явно горизонтальный и явно вертикальный drag, не саму границу, поэтому по букве ТЗ автор может реализовать любое пороговое отношение (от «чуть больше по горизонтали» до «почти строго горизонтально») и формально пройти оба AC, получив при этом заметно разное поведение на диагональном движении у самого края экрана.
• Почему это продуктовый, а не только технический вопрос: PROCESS.md §7.1 прямо перечисляет «поведение в пограничном случае» как пример вопроса, который остаётся продуктовым, а не техническим усмотрением реализатора — а именно эта задача целиком посвящена тому, что «два визуально похожих жеста выполняют разные действия» (issue, «Как проявляется», п.3). Оставлять неоднозначной ровно ту границу, из-за которой заведена задача, — системно та же проблема, которую ТЗ должно закрыть.
• Смягчающее обстоятельство: сам пункт 6 говорит, что при сомнении («неверное направление или вертикальное движение») жест становится pan, то есть безопасным исходом (не переключает пространство). Это не даёт находке стать High — она не делает AC невыполнимым или в целом непроверяемым, только оставляет одну границу без числа.
• В скоуп задачи: да, правится одной фразой — либо явным числом (можно переиспользовать текущее 1.5), либо явной записью в §18, что порог сохраняется без изменения. Отдельный issue не заводится (#202).

Что проверено и корректно

• Обязательные разделы §7.1 присутствуют полностью и в осмысленном порядке (см. «Как проверялось» п.3); «Сценарий» называет персону (телефон/настенный touch-экран, несколько пространств — Household members/Guests по docs/SCOPE.md), поверхность (View/kiosk) и момент; «Что человек видит до и после» — двумя абзацами, преимущественно без терминов реализации.
• Контракт §6/§7 внутренне согласован (кроме находки выше) и полностью покрыт: п.1–5 (когда доступен edge-swipe, ширина зоны, направление, нет соседа, вне зоны — всегда pan) → AC1/AC2/AC3; п.6–7 (классификация и финальность) → AC2–AC4; п.8 (второй палец) → AC5; п.9 (interactive owner) → AC5/AC9; §7 п.1–3 (модальность, поверхности, задержанный room-fit) → AC6/ AC8; п.4 (double-fit без промежуточного кадра) → AC7; п.5 (одиночный background-тап не делает ничего) → согласуется с уже принятым #449, не меняется; п.6 (interactive owners исключены) → AC9; п.7 (lifecycle-отмена) → AC10; п.9 (модальности не смешиваются, keyboard без задержки) → AC8; п.10 (fit-all переиспользует camera transition) → покрыт §11 (производительность) и не отдельным AC — обоснованно, это не новое поведение.
• Три ключевых числа (48 px, 8 px, 350 мс) согласованы между контрактом, AC и §18, и совпадают с существующими именованными константами в коде (STAGE_TAP_DISTANCE_PX, DOUBLE_FIT_WINDOW_MS) — не изобретены заново.
• AC1–AC12 однозначны (кроме границы в находке выше) и называют способ доказательства: unit (с fake clock для AC6–AC8), browser smoke, mutation witness для четырёх защитных AC (AC1, AC4, AC5, AC9) — ровно там, где защитное поведение (снятие guard, финального лока, owner-исключения) требует не просто падающего теста, а названной мутации; на этапе spec название способа доказательства достаточно, полная таблица «чем краснеет» с результатом прогона — требование этапа code (docs/process/REVIEWER.md «Код-ревью»), не spec.
• Технические решения (расположение guard, разделение room-fit.ts/ houseplan-card.ts, устройство pure state machine) корректно оставлены «на усмотрение реализации» и не эскалированы владельцу — они не наблюдаемы пользователем напрямую.
• Продуктовые развилки Q1–Q3 были заданы владельцу одним комментарием с default/альтернативами, решены явно («Q1 — Default», «Q2 — Альтернатива», «Q3 — Default») до публикации текста ТЗ; в самом тексте ТЗ открытых продуктовых вопросов не осталось — соответствует §7.1 (порог на вопросы, пачкой, с вариантом по умолчанию).
• Риски (§15) называют именно те конкурирующие механизмы, которые фигурируют в «Связанных задачах» issue (#449, #563, #578, #531) и в самом контракте (остаточный палец после pinch, таймер, переживший смену пространства, WebView edge-gesture конкуренция) — не спрятаны внутри AC.
• Откат (§16) — чистый revert продуктового коммита, без данных/миграции; согласуется с §9 «Данные, совместимость и миграция» (новых полей нет).
• Release-артефакты (§17) называют оба changelog, docs/CANVAS.md и docs/TOUCH-SUPPORT.md (сверено выше, что оба документа действительно фиксируют заменяемое поведение и нуждаются в правке), условно — docs/TESTING.md, golden/check-docs — список полон, ничего не упущено и не добавлено лишнего (docs/CONFIG-COMPATIBILITY.md в списке нет и не должен быть — миграции нет).
• i18n (§10) — явное «нет новых строк», согласуется с тем, что весь диф — арбитраж жестов, а не новый UI-текст.

Чего не проверял

• Гейты (tsc --noEmit, npm test, npm run build, check-docs.mjs, смоки, golden:verify, инварианты, performance) — не прогонял: этап spec, кода для #691 нет, git diff origin/dev...HEAD пуст (см. «Как проверялось» п.7). Предмет код-ревью после реализации.
• Не пробовал руками воспроизводить исходный баг (конфликт pan/swipe, недоступный double-tap) на реальном touch-устройстве или в HA Companion — это диагностика владельца и автора ТЗ по коду (issue «Что подтверждено по актуальному коду», подтверждённая мной построчным чтением кода, см. «Как проверялось» п.4), а не то, что проверяется вручную на этапе spec.
• Не оценивал практическую (а не только текстовую) осуществимость pure state machine для edge-arbitration и double-fit при реальной интеграции с Lit-рендером и HA Companion WebView — это по разделу «Принято предположительно» оставлено на усмотрение реализации и подлежит AC1–AC10 на код-ревью.
• Не искал в docs/reviews/INDEX.md строки предыдущих раундов подсистемы — не требуется: это r1, §2.10 (объём по дельте) не применяется.

Вердикт

Обязательные разделы ТЗ полны и в осмысленном порядке, контракт §6/§7 почти полностью непротиворечив и покрыт AC1–AC12 с названным способом доказательства, включая защитные mutation witnesses там, где это нужно. Технические утверждения о текущем коде (константы, геттеры, функции, ограничение beginDoubleFitPointer на background) проверены построчно и оказались точными. Продуктовые развилки Q1–Q3 закрыты владельцем до публикации текста, открытых продуктовых вопросов не осталось. Единственная находка — Medium, в скоупе задачи: направление-преобладание для классификации edge-swipe (§6 п.6) не квантифицировано и не зафиксировано в «Принятых предположениях», хотя именно эта граница — источник исходного дефекта («визуально похожие жесты выполняют разные действия»). Находка не блокирует технически (fallback безопасен — неопределённость разрешается в pan), но нарушает требование однозначности AC и правится одной фразой. Без High-находок это жёлтый вердикт: автор добавляет число или явное допущение в §6 п.6/§18, ревью проходит повторный (нецикловый по факту правки) цикл.

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

---

Материал раунда

• Ветка: dev, коммит 1751184b1247 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: e271ee4d37f31e5b35a219fe7f168aaceeda15f6

  git log --all --format='%H %T' | grep e271ee4d37f3
  

• Тело issue: 4abdb509952fb3ea465e6d6c32a8f6825982bf8533b7f5ff35585db7ee2840c2
• Вердикт конвейера: yellow · High 0