Files
houseplan-card/docs/reviews/SPEC-REVIEW-563-r1.md
2026-09-13 15:34:08 +00:00

16 KiB
Raw Permalink Blame History

SPEC-REVIEW-563-r1

Issue: #563 — «Pinch-жест на touch-устройстве вызывает действие маркера устройства во время/после масштабирования» (bug, P1, полный трек). Заход: r1 · блокирующих циклов израсходовано 0 из 4.

Скоуп ревью

ТЗ живёт в теле issue #563, раздел ## ТЗ (решение #517). Ревьюер читает только тело issue и комментарий аналитики, без устных пояснений автора. Материал: тело issue на момент чтения (2026-09-13), метки bug, P1, tests, S4-spec-review. Аналитика уже отклонила лёгкий трек (нарушены критерии §5 «нет влияния на touch» и «сложность/риск ≤3») — корректно: задача переписывает центральную capture-guard машину жестов, общую для View, kiosk и редакторов, и снимает гарантированное свойство touch-контракта (docs/TOUCH-SUPPORT.md: «saving unintended... a second touch was misread as a click» / lock invariant docs/SCOPE.md CR‑1). Дубликаты #396 (плавная камера/якорь зума, закрыт) и #449 (двойной тап — room fit, закрыт) — оба о другом; не дубликаты, аналитика права.

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

  • Прочитаны docs/SCOPE.md, AGENTS.md, PROCESS.md целиком (порядок из инструкции), docs/TOUCH-SUPPORT.md (канонический документ подсистемы), выдержки docs/TESTING.md, docs/USER-GUIDE.ru.md (раздел «Навигация, масштаб и жесты»).
  • Прочитан весь текст issue #563 (before-ТЗ секции Проблема / Как воспроизвести / Ожидаемое поведение / Фактическое поведение / Обязательная регрессия, сам ## ТЗ, оба комментария — аналитика и «Взял»).
  • Каждое фактическое утверждение ТЗ о текущем коде сверено с src/houseplan-card.ts построчно (не поверил на слово): _guardTouchGesture (строки 7239–7326), _touchSequenceMultitouch, _touchClickBlockUntil = Date.now() + 500 (7311–7312), _suppressClick/его сброс через setTimeout(…, 0) (6797, 6892, 6991–6992, 7029, 7144–7145), _doubleFit, _holdTimer/_kioskHoldTimer.
  • Проверен существующий demo/smoke_editor_gestures.mjs: подтверждено, что сценарий pinchCannotMisclickInteractiveChild сегодня ждёт 520 мс и шлёт click без нового pointerdown, засчитывая его как «обычный тап» (ordinaryTapStillWorks) — именно та дыра, которую описывает issue, и именно тот тест, который новый контракт (AC3/AC4) обязан заставить измениться.
  • Проверено существование scripts/mutation-gate.mjs (реестр именованных мутантов, issue #85) и его конвенции (id, find/replace, привязка к конкретному тесту) — AC7 просит ровно то, что механизм уже умеет; не голословное требование.
  • Проверено существование docs/TESTING.md (ссылка в §5 ТЗ не выдумана).
  • Проверены #396 и #449 через gh issue view — оба закрыты, оба про другое.
  • Ревью выполнялось состязательно: цель — не согласиться с автором, а найти, где ТЗ невыполнимо или непроверяемо.

Этап spec: тяжёлые/browser-гейты не запускались и не нужны — на этой стадии проверяется текст, а не код; ни один AC ещё не реализован.

Находки

Low — сценарий и «что человек увидит» не выделены отдельными первыми

разделами ## ТЗ (§7.1)

PROCESS.md §7.1 требует, чтобы первые два раздела ТЗ были: «сценарий» (персона по docs/SCOPE.md, поверхность, момент) и «что человек увидит до и после» одной фразой без терминов реализации. Раздел ## ТЗ в #563 начинается сразу с «1. Проблема и цель» и не содержит отдельного раздела-ответа на эти два вопроса.

По существу вопрос всё же закрыт — но текстом до ## ТЗ (Как воспроизвести / Ожидаемое поведение), а не в самом ТЗ: персона — Household member/Guest на touch-поверхности View/kiosk (docs/SCOPE.md: «View mode is the product for two of the three personas»), момент — pinch рядом с маркером, до — маркер получает случайное действие, после — pinch только масштабирует, обычный тап работает как прежде. Ни одна деталь не придумана, всё уже было точно сформулировано в issue.

Поскольку содержание присутствует и однозначно и второй агент (код-ревью) получит его как часть того же тела issue, снимаю находку без цикла правок — Low, снята решением ревьюера с записью (§2.4): не блокирует и не требует возврата автору. Рекомендация на будущее: копировать эти два абзаца форматом «Сценарий» / «Что видит пользователь» в начало ## ТЗ, чтобы страховка §7.1 читалась механически, а не «по всему телу issue».

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

  • Полный трек выбран верно и обоснован названным нарушенным критерием §5, а не общей фразой «обычный трек» (соответствует правилу «отказ от лёгкого трека обосновывается явно», issue #338).
  • Каждое утверждение о текущем поведении читаемо в коде и подтвердилось: двухфазный guard, замена бессрочного блока на Date.now()+500 после отпускания последнего пальца, немедленный сброс _touchSequenceMultitouch при _touchContacts.size===0 — ни одной догадки, выданной за факт, не найдено; предположения, которые действительно являются предположениями, вынесены в явный раздел «Принятые предположения» (событийная vs временная форма блока; общий ли guard для потомков карточки; свобода технической формы состояния) — ровно то, что требует §7.1.
  • AC1…AC8 однозначны и у каждого указан способ доказательства (browser smoke / unit / mutation / стандартные гейты), без смешения. AC1–AC3 воспроизводят все пять пунктов «Обязательной регрессии» из тела issue дословно (палец на маркере, реальное изменение zoom, нулевые счётчики во время и после жеста, ровно одно срабатывание при следующем осознанном тапе, отдельно pointercancel/lostpointercapture и оба порядка отпускания пальцев).
  • AC6 корректно бьёт по корню проблемы: явно требует, чтобы корректность состояния guard не зависела от Date.now()/setTimeout — это ровно тот механизм (500 мс окно), который сегодня и создаёт дыру. Заменить его на времязависимый вариант с другим числом прямо запрещено (§3 ТЗ, п.7) — типичная лазейка «увеличить таймер» закрыта текстом ТЗ, а не оставлена на усмотрение реализации.
  • AC7 — защитный AC с названным свидетелем (табличное требование §2.7 «чем краснеет» будет закрыто на код-ревью): ТЗ называет ровно два кандидата мутации (снять блок по последнему pointerup; разрешить click без нового pointerdown) и требует красноты у AC1–AC3. Механизм scripts/mutation-gate.mjs уже поддерживает добавление такого мутанта по существующей конвенции — требование выполнимо, не голословно.
  • Не-скоуп сформулирован явно и не размыт: touch-паритетность редакторов и изменение их жестов — не входят (п.6 «Пользовательский контракт»); поведение вне Pointer Events и нативный page zoom — не входят (§6 ТЗ); существующая защита комнат/проёмов/controls не ослабляется (принятое предположение).
  • Совместимость/данные/миграция/i18n закрыты явным «нет»: конфиг, layout, backend/WebSocket API, сущности, i18n не меняются — соответствует DoR-пункту (§2.5), не требует docs/CONFIG-COMPATIBILITY.md.
  • Touch-влияние по docs/TOUCH-SUPPORT.md названо и совместимо: «Touch View/kiosk — release-blocking» указано прямо (§6 ТЗ), соответствует таблице политики («View/kiosk: fully supported / primary environment»); задача не вводит новую editor-фичу, поэтому явная метка «Touch editor: …» из раздела «Documentation rule» не требуется — существующий editor safety floor прямо сохраняется предположением, а не переопределяется.
  • Откат назван и реалистичен: вернуть прежний времязависимый guard и связанные тесты; данные/конфигурация не меняются исправлением, поэтому откат безопасен технически (риск отката — возврат старой уязвимости, что прямо признано).
  • Release-артефакты названы: оба changelog в User-Visible: yes коммите, docs/TOUCH-SUPPORT.md (и при необходимости docs/TESTING.md) обновляются в том же коммите, что и поведение (§2.6).
  • Риски перечислены содержательно (навсегда заблокировать при отсутствии compatibility-click; снять блок слишком рано новой точкой старой последовательности; пропустить cancel/lost-capture; задеть мышь на гибридном устройстве) — и на каждый есть закрывающий AC (AC4 — мышь и клавиатура; AC2 — cancel/lost-capture и оба порядка; AC6 — машина состояний без времени).
  • Производительность: явно названо «O(1) на событие плюс существующий Map», без новых таймеров/re-render — удовлетворяет DoR-пункту «влияние на производительность названо».

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

  • Реализуемость самого guard-кода (это работа код-ревью после S6); ТЗ проверялось на выполнимость и непротиворечивость с текущим кодом, не на то, что будущий diff будет именно таким.
  • Тяжёлые/browser/golden/performance-гейты — не запускались: не нужны на этапе ревью ТЗ (код ещё не написан) и не входят в §2.4.
  • Правильность существующего смока demo/smoke_editor_gestures.mjs целиком (pinchZoomsInPlanEditor, pinchDoesNotZoomBelowVacuumFit, _markupClick-сценарий) — прочитан только фрагмент, относящийся к найденной несовместимости с новым контрактом; остальные сценарии файла к ТЗ #563 не относятся и не разбирались.
  • Свежесть SHA HEAD (e3676777) для целей этого этапа не имеет значения: этап spec смотрит на текст issue, а не на дерево репозитория; дерево использовано только для чтения src/houseplan-card.ts, scripts/mutation-gate.mjs, demo/smoke_editor_gestures.mjs и канонов — файлы, которые задача не редактирует на этом этапе.

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

  • Issue: #563, метки на момент ревью: bug, P1, tests, S4-spec-review.
  • ТЗ: раздел ## ТЗ тела issue #563 (единственная редакция, r1).
  • Комментарии учтены: аналитика (полный трек, дубликаты проверены), «Взял: автор ТЗ».
  • Рабочее дерево репозитория на момент чтения кода: e3676777 (справочно, не является материалом самого ТЗ).

Вердикт

Вердикт: зелёный · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 0 → в задаче | — · Документ: docs/reviews/SPEC-REVIEW-563-r1.md


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

  • Ветка: dev, коммит e36767773ae0 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
  • Дерево материала: 87d7af00903d3fa9b3b71b42f0d277970d781fd7
    git log --all --format='%H %T' | grep 87d7af00903d
    
  • Тело issue: d4129f3c15c312e0749802a9fb3b87d1be9ceb3dba94b24f33d9243ac4f74323
  • Вердикт конвейера: green · High 0