diff --git a/docs/reviews/SPEC-REVIEW-578-r1.md b/docs/reviews/SPEC-REVIEW-578-r1.md new file mode 100644 index 00000000..29a0d725 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-578-r1.md @@ -0,0 +1,191 @@ +# SPEC-REVIEW-578-r1 + +- Issue: #578 — «Touch: второй палец pinch на устройстве запускает long-press и HA more-info» +- Этап: spec (PROCESS.md §2.4), заход r1, блокирующих циклов израсходовано 0 из 4 +- Материал: тело issue #578, раздел `## ТЗ` (сборка 14 разделов, план автотестов, принятые предположения), состояние `S4-spec-review`, комментариев к issue нет — это первый заход +- Проверено на `dev` @ `ad4000f95a152d2cdd0fc41f3cb1707df121a47e` (рабочая копия уже на этом SHA) + +## Скоуп ревью + +Ревью ТЗ, не кода: продукт ещё не изменён. Вопрос — выполнимо ли и однозначно ли +ТЗ, доказуем ли каждый AC, не выдана ли догадка за факт, укладывается ли трек +(полный) и не требуется ли продуктовый вопрос владельцу. + +## Как проверялось + +1. Прочитан `docs/SCOPE.md` — задача чинит нарушение touch-контракта в View/kiosk + (персоны «Household members», «Guests/kiosk», §«View mode is the product»); + лежит внутри уже принятой рамки, нового job не открывает и не расширяет её. +2. Прочитаны `AGENTS.md` и разделы PROCESS.md §2.4, §2.9/2.10 (не применимо — r1), + §4, §7.1, §7.2. +3. Прочитано тело issue #578 целиком (`## Проблема` → `## ТЗ`, 14 разделов). + Комментариев нет. +4. Прочитан `docs/TOUCH-SUPPORT.md` (контракт View/kiosk, разделы «Product + contract», «What "fully supported View" means», «Safety floor»). +5. Прочитан `docs/USER-GUIDE.ru.md` (табл. §на строках 1009/1132/1133/1147) — + термины «долгое нажатие, 600 мс», «внутренняя карточка House Plan», «HA + more-info», «правый клик» использованы в ТЗ так же, как в интерфейсной + документации; не изобретены. +6. **Технический разбор аналитики сверен с кодом на названном SHA**, а не принят + на веру (обе претензии автора — не догадка): + - `src/houseplan-card.ts:7126-7146` (`_pointerDown`, ветка `view`): + `_holdTimer` действительно взводится безусловно, без проверки + `_touchSequenceMultitouch`/`_touchClickGuard.sequenceMultitouch` — + соответствует претензии «второй палец на маркере заново вооружает + long-press». + - Capture-guard `_touchGestureGuard` (`src/houseplan-card.ts:2090-2093`) + подписан только на `pointerdown/pointermove/pointerup/pointercancel/ + lostpointercapture/click` (`:11451-11456`); `@contextmenu` навешен прямо на + маркер (`:12555`) и вызывает `_ctxDevice()` (`:5751-5759`) в обход + capture-фазы — соответствует претензии «touch contextmenu обходит защиту + pinch». + - `src/touch-gesture-click-guard.ts` (класс `TouchGestureClickGuard`, + проверен целиком) и `test/touch-gesture-click-guard.test.mjs` показывают, + что существующий guard #563 закрывает только `click`, не имеет понятия о + long-press/contextmenu, и уже сейчас корректно снимает блокировку на любой + новый `pointerDown` иного `pointerType` (тест «a new mouse sequence on a + hybrid device is not held by an old pinch», строки 50-63) — то есть + примитив, который ТЗ предлагает обобщить (§14), уже имеет нужное свойство + для гибридного случая, и его расширение с высокой вероятностью наследует + это свойство бесплатно. + - `_keyDevice` (`:5903-5908`) подтверждает существование клавиатурной + активации Enter/Space, на которую ссылается контракт п.9 раздела 5 — не + придуманная фича. +7. Сверены названные в плане автотестов файлы/инструменты: + `demo/smoke_editor_gestures.mjs`, `demo/smoke_long_press_gesture.mjs`, + `scripts/mutation-gate.mjs`, `docs/TESTING.md` (раздел про + `scripts/mutation-registry.mjs`) — все существуют, инфраструктура для + AC8/AC9 реальна, а не гипотетична. +8. Проверено согласование трека: у `small` (PROCESS.md §5) требуется ровно одна + поверхность, отсутствие нового UX-контракта и touch-эффекта — задача правит + `pointerdown`/long-press/`contextmenu`/click одновременно и меняет + `docs/TOUCH-SUPPORT.md`, то есть полный трек выбран верно, автор обосновал + это явно, а не промолчал. + +Код не менялся, гейты (`typecheck`/`test`/`build`) на этой стадии не +прогонялись — они относятся к этапу code-review, спецификация не трогает `src/**`. + +## Находки + +### Low-1 — i18n закрыт только косвенно + +`§7.1` требует отдельно отраслью i18n. В ТЗ единственное упоминание — пункт +«вне scope»: «изменение конфигурации, backend/WebSocket протокола, HA +entity/device model **либо i18n**» (issue, раздел «4. Вне scope»). Явного +«новых ключей нет» как отдельного вывода не сформулировано. + +По существу пункт закрыт: раздел «7. UX и доступность» прямо запрещает любые +новые сообщения/тосты/overlay при pinch, а вся правка — про подавление событий, +не про текст интерфейса. Второй независимый источник (перечень «Затронутые +модули», раздел 10) не содержит ни одного i18n-файла. Открытого вопроса это не +создаёт. + +**Решение ревьюера:** Low, снимается без правки — вывод «новых i18n-ключей нет» +следует из §7 и §10 однозначно, дублировать его отдельной строкой не обязательно +для однозначности исполнения. + +### Low-2 — сценарий «новый mouse pointerdown сразу после pinch» не назван явно ни в одном AC + +Контракт п.8 раздела 5 (второе предложение): «Mouse pointerdown новой +последовательности на гибридном устройстве не наследует блокировку +закончившегося pinch». Ни один AC (AC1–AC9) не называет этот сценарий по имени; +ближе всего AC6 («сохраняет mouse click, правый клик и Enter/Space без ложного +подавления») и общая фраза плана автотестов п.4 («сохранить контрольные +позитивные сценарии... mouse right-click»), но явного «мышь сразу после +завершённого pinch» там нет. + +Смягчающее обстоятельство, подтверждённое чтением кода (см. выше, п.6): для уже +существующего click-guard это свойство уже реализовано и покрыто тестом +`test/touch-gesture-click-guard.test.mjs:50-63`, а обобщение того же примитива +на long-press/contextmenu (что и предлагает §14 ТЗ) с высокой вероятностью +наследует то же поведение той же веткой кода (`if +(this._activeTouchPointers.size === 0) this._postGestureClickBlocked = false;` +не зависит от `pointerType`). Риск регрессии этого конкретного угла невысок, но +он не нулевой, если реализация заведёт для long-press/contextmenu отдельный, +не переиспользованный примитив. + +**Решение ревьюера:** Low, не блокирует. Рекомендация автору (не обязательна к +исполнению до кода): при реализации явно включить проверку «мышиный +`pointerdown` сразу после конца pinch не наследует блокировку» в +unit/browser-доказательство AC6 или AC7, раз уж контракт её формулирует. +Формально снимаю с записью — General wording AC6 достаточно для однозначности +приёмки, кодревьюер вправе потребовать явный тест-кейс при реализации. + +Ни High, ни Medium-находок нет. + +## Что проверено и корректно + +- Обязательные разделы §7.1 присутствуют все: сценарий и «что человек увидит» + (раздел 1), проблема (2), скоуп/не-скоуп (3–4), контракт поведения (5), + данные/совместимость/миграция (6), UX (7), AC1–AC9 с указанным способом + доказательства (8), план автотестов (9), риски и откат (12), release-артефакты + (13). Плюс необязательные, но полезные «затронутые модули» (10) и + «производительность/touch-impact» (11). +- Каждый AC имеет наблюдаемый oracle (счётчики `_infoCard`/`hass-more-info`/ + confirmation/service, факт изменения zoom, число срабатываний нового + tap/long-press) и назван способ доказательства (`browser smoke`, `unit`, + `mutation witness`, стандартные гейты) — ни один не сводится к «проверить, что + код скомпилировался». +- AC8 заранее требует «отрицательных свидетелей» (mutation witness, красный при + снятии каждой из двух защит по отдельности) — снимает типовой риск теста, + который не умеет падать, до того, как код написан. +- Технический анализ причины дефекта (два независимых пробела: bubble-phase + `_pointerDown` не проверяет multitouch; `contextmenu` не входит в + capture-guard) подтверждён построчным чтением кода на точном SHA, а не + принят на веру — обе претензии верны. +- Раздел 14 («принятые предположения») корректно выделяет ровно то, что + пользователь не наблюдает (внутреннее представление barrier, способ отличить + synthetic touch-`contextmenu` от настоящего), помечает это как «свободно + меняется» и не выдаёт технические догадки за продуктовые факты нигде за + пределами этого раздела — я не нашёл утверждений о поведении, которого нет ни + в `TOUCH-SUPPORT.md`, ни в `USER-GUIDE.ru.md`, ни в коде, без такой пометки. +- Терминология («долгое нажатие, 600 мс», «внутренняя карточка House Plan», + «HA more-info», «правый клик») дословно совпадает с `docs/USER-GUIDE.ru.md`, + не изобретена заново. +- Вне-scope (раздел 4) корректно исключает изменение таймаутов, самих жестов + pan/pinch/double-tap, действий устройств, конфигурации, i18n и общей event + architecture — граница задачи не расползается за пределы двух названных + пробелов. +- Выбор полного трека вместо `small` обоснован явно и по существу (несколько + путей активации, изменение touch-контракта), а не декларативно. +- «Вопросы владельцу: нет» — оправдано: ожидаемое поведение уже зафиксировано в + действующем `docs/TOUCH-SUPPORT.md` («once a second touch joins... whole + sequence is navigation only»), новых продуктовых решений задача не требует; + открытых продуктовых вопросов (что видит/делает человек, объём изменений) я + тоже не нашёл — оба найденных Low технические, не продуктовые, и решаю их + сам, не выношу владельцу. +- Откат (revert коммита, без данных/миграции) и риски (§12) покрыты + перечисленными AC3–AC7 по существу, а не общей фразой. + +## Чего не проверял + +- Код ещё не написан — не проверялись типы гейтов (`typecheck`/`test`/`build`), + golden, smoke-запуски: это стадия code-review, не spec-review. +- Не проверял `scripts/mutation-registry.mjs` на предмет точного формата записи + двух будущих witness-мутаций — техническая деталь реализации, не предмет + ревью ТЗ. +- Не запускал `node scripts/smoke-select.mjs` — на этой стадии диффа кода нет, + инструмент неприменим. + +## Вердикт + +Зелёный. High — 0, Medium — 0. Обе Low-находки решены самим ревьюером с +записью (без правки ТЗ и без возврата автору), как предусмотрено PROCESS.md +§2.4 для Low. + +**Готово к разработке** — но перевод статуса делает конвейер (эта роль его не +меняет вручную). + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `ad4000f95a15` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `752738456c32b4d4f015527719edba343c434395` + ``` + git log --all --format='%H %T' | grep 752738456c32 + ``` +- Тело issue: `ec0f61cba0a9ebb0afa611f400145755e567714b91b307e18928aed57bbb61be` +- Вердикт конвейера: `green` · High 0