From 20f17fee5453d53014d08e0f3c2fb92ba69cbe3e Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Mon, 28 Sep 2026 12:51:01 +0000 Subject: [PATCH] docs: review document for #691 Issue: #691 User-Visible: no --- docs/reviews/INDEX.md | 3 +- docs/reviews/SPEC-REVIEW-691-r1.md | 260 +++++++++++++++++++++++++++++ 2 files changed, 262 insertions(+), 1 deletion(-) create mode 100644 docs/reviews/SPEC-REVIEW-691-r1.md diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index cfdae234..c5e6cdf7 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -1,9 +1,10 @@ # Индекс ревью -Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 168, issue: 79. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. +Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 169, issue: 80. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. | Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы | |---|---|---|---|---:|---:|---|---| +| #691 | [SPEC-REVIEW-691-r1.md](SPEC-REVIEW-691-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 1 | направление edge-swipe не квантифицировано и не отмечено как допущение | `src/houseplan-card.ts` | | #689 | [SPEC-REVIEW-689-r1.md](SPEC-REVIEW-689-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | — | — | | #688 | [SPEC-REVIEW-688-r1.md](SPEC-REVIEW-688-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | — | — | | #688 | [CODE-REVIEW-688-r1.md](CODE-REVIEW-688-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | AC6 не покрывает спиральную лестницу и второй масштаб печати на уровне PDF-сцены | `test/pdf-scene.test.mjs` `test/stairs.test.mjs` `src/pdf/pdf-scene.ts` | diff --git a/docs/reviews/SPEC-REVIEW-691-r1.md b/docs/reviews/SPEC-REVIEW-691-r1.md new file mode 100644 index 00000000..ce0468a0 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-691-r1.md @@ -0,0 +1,260 @@ +# 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