mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -0,0 +1,189 @@
|
||||
# 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
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `dev`, коммит `e36767773ae0` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `87d7af00903d3fa9b3b71b42f0d277970d781fd7`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 87d7af00903d
|
||||
```
|
||||
- Тело issue: `d4129f3c15c312e0749802a9fb3b87d1be9ceb3dba94b24f33d9243ac4f74323`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user