Files
houseplan-card/docs/reviews/SPEC-REVIEW-564-r1.md
2026-09-14 02:57:32 +03:00

16 KiB
Raw Permalink Blame History

SPEC-REVIEW-564-r1

Скоуп

Issue #564 (bug, P1, S4-spec-review, полный трек — S2-аналитика прямо назвала сложность/риск 6/10 и влияние на touch/security-контракт как критерии, не пройденные small). Предмет: единая модель определения владельца pointer-точки (click/tap, hover/tooltip, long-press, contextmenu, drag в редакторе устройств) для маркеров устройств, когда фиксированный невидимый 44‑px hit-circle (.dev::before, src/styles/devices.styles.ts) одного маркера перекрывает соседний на плотной раскладке (390 px, шаг центров 11,8 px — воспроизведено на реальной установке владельца, найдено при полевой проверке #560). Сейчас побеждает DOM-order, а не визуально ближайший маркер. ТЗ живёт в теле issue, раздел ## ТЗ, как того требует #517.

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

  • Прочитаны docs/SCOPE.md (J3 «безопасно действовать», персоны, touch-контракт), AGENTS.md, PROCESS.md целиком (обе части файла), docs/UX-MODES.md (раздел «Devices — placement and marker configuration»), docs/TOUCH-SUPPORT.md.
  • Прочитано тело issue #564 целиком (постановка дефекта + ## ТЗ, 217 строк) и единственный комментарий issue через gh issue view 564 --json body,comments и повторно через GraphQL (comments.totalCount: 1, isMinimized: false) — чтобы исключить скрытый/удалённый ответ владельца.
  • Восстановлен полный timeline меток (gh api .../timeline): S1-new → S2-analysis (22:22:46) → комментарий аналитика с Q1/Q2 (22:26:16) → S3-spec+blocked (22:26:17) → blocked снят (22:34:57) → S4-spec-review (22:35:06). Ни одного дополнительного комментария между снятием blocked и переходом нет.
  • Прочитан текущий код, который описывает баг: src/styles/devices.styles.ts (строки 173–183, подтверждён .dev::before { width/height: max(44px, var(--device-shell-size)); pointer-events: auto; z-index: 3; }) и src/houseplan-card.ts (_clickDevice, строка 5686, привязан к маркеру через замыкание на строке 12465) — технический диагноз в аналитике не выдуман, а совпадает с кодом.
  • Прочитан docs/specs/213-device-marker-geometry.md — подтверждён контракт «44×44 минимум + вся видимая capsule — hover/action область», на который ссылается ТЗ #564 (нормативный источник №3). Проверен статус #563 (S8-merged, «Touch: pinch-to-zoom случайно активирует устройства под пальцами») — существующий контракт, который #564 обязано не сломать.
  • Сверено docs/UX-MODES.md: «Icon dragging (ONLY here). Click on a device opens the edit dialog directly» в Devices editor — не противоречит модели «владелец фиксируется на pointerdown» из §6 ТЗ.
  • Отдельно сверено, как этот же паттерн (аналитика + Q1/Q2 в одном/двух комментариях, без отдельного видимого ответа владельца, default подхвачен прямо в тексте ТЗ) был оценён в последнем прецеденте этого репозитория — docs/reviews/SPEC-REVIEW-561-r1.md. Тот раунд прямо отказался считать это находкой: «процесс не требует, чтобы решение владельца обязательно приходило тем же каналом, что вопрос». Не переоткрывал этот вопрос заново ради #564 — см. раздел «Чего не проверял».
  • Пункты DoR (PROCESS.md §2.5) сверены построчно с текстом ТЗ.

Находки

M1 — DoR: «затронутые файлы и модули» не названы (Medium, в скоупе)

PROCESS.md §2.5 требует для перехода в «Готово к разработке»: «перечислены затронутые файлы и модули» — пункт обязателен наравне с остальными девятью. Раздел «7. Архитектурные ограничения» ТЗ #564 описывает требуемую архитектуру (один общий resolver, запрет z-index/DOM-order разрешения, кэш экранной геометрии), но не называет ни одного реального пути: ни src/styles/devices.styles.ts (источник .dev::before), ни src/houseplan-card.ts (_clickDevice, hover/contextmenu handlers), ни src/houseplan-editor-runtime.ts (drag в Devices editor), ни ожидаемое имя нового модуля-резолвера, ни houseplan-space-card/hp-device-preview.ts, которые §3 Scope упоминает как затронутые поверхности. Без этого списка ни исполнитель, ни код-ревью не могут заранее сверить, что реализация не расползлась на непредусмотренную поверхность — а сама проверка DoR-пункта §2.5 буквально не проходит.

Как воспроизвести: полнотекстовый поиск по телу issue (grep -i "файл\|модул") не даёт ни одного совпадения — проверено отдельно.

Как чинится: автор добавляет короткий список ожидаемых файлов/модулей (по образцу docs/specs/213-device-marker-geometry.md §15 «Ожидаемые зоны изменений») тем же абзацем ТЗ, без пересмотра модели или AC.

L1 — «Сценарий» и «Что человек увидит» объединены без явной персоны на

поверхность (Low, снимаю с записью)

PROCESS.md §7.1 требует два первых раздела ТЗ отдельно: какая персона, на какой поверхности, в какой момент — и одной фразой, что человек увидит. ТЗ #564 объединяет это в «1. Цель и пользовательский результат» одним общим описанием, не разделяя персону/поверхность по каждой из нескольких реально затронутых поверхностей (View/kiosk — домочадец/гость, docs/SCOPE.md; Devices editor drag — администратор). Содержательно оба вопроса покрыты исходным текстом бага и §3 Scope, формулировка проверяема, персоны восстанавливаются однозначно из контекста — поэтому не блокирую, а снимаю с записью, а не как повод для правки текста.

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

  • Технические предпосылки ТЗ (44‑px круг .dev::before, click handler, замкнутый на конкретный маркер, отсутствие арбитража перекрытий) подтверждены прямым чтением текущего кода, а не только заявлением автора аналитики.
  • Ссылки на #213 (44×44 минимум + вся видимая capsule — hover/action) и #563 (S8-merged, pinch suppression) точны, не выдуманы и не спутаны между собой — #564 воспроизводится обычным неподвижным нажатием, а не через multi-touch/pinch, поэтому дубликатом не является.
  • 11 AC пронумерованы, у каждого назван способ доказательства (unit / browser smoke / mutation witness / source review), формулировки проверяемы и привязаны к конкретным артефактам — например AC1 к существующей J7-фикстуре и именованным маркерам dense1…dense5, а не к абстрактному «работает корректно».
  • Compatibility-матрица (§8) покрывает все шесть классов риска из PROCESS.md §2.6: async (pointer lifecycle latch до pointerup/cancel), данные/права (unavailable/virtual/hidden/removed/другое пространство не участвуют в арбитраже), геометрия (flat/3D, pan/zoom, DPR 1/1.25/1.5/2), визуал (Icon/Text/Double/legacy, четыре направления capsule), объём/perf (200 маркеров, §7 явно запрещает full per-frame scan), host/input (mouse/touch/pen/keyboard/long-press/contextmenu).
  • Не-скоуп (§4) корректно исключает визуальное раздвигание/кластеризацию, изменение сохранённых координат/размера/действий/подтверждений и правку механизма #563 — согласовано и с docs/SCOPE.md (нет автоматического изменения геометрии без отдельного запроса), и с уже принятым #563.
  • Откат (§12) реалистичен: полный implementation revert, без миграции данных и без скрытого риска для хранимой конфигурации.
  • Release-артефакты (§11) называют все реально релевантные документы (ARCHITECTURE.md, TOUCH-SUPPORT.md, TESTING.md, changelog RU+EN) — соразмерно объёму задачи.
  • i18n, схема конфигурации и backend API явно не меняются (§4, AC11) — этим закрыты DoR-пункты «i18n» и «миграция/compatibility» явным «нет», а не молчанием.
  • Q1/Q2 (модель арбитража перекрытий; единство владельца между click/hover/long-press/contextmenu/drag) — ровно те примеры продуктовых вопросов, что перечисляет PROCESS.md §7.1 («поведение в пограничном случае», «объём видимых изменений»); их default последовательно и без внутренних противоречий проведён через весь текст ТЗ (§1, §5, §6), а отклонённые альтернативы обоснованы (визуальное раздвигание уже отдельно исключено из скоупа; частичное исправление только click оставило бы hover/long-press/contextmenu уязвимыми к тому же риску J3, ради которого заведена задача).

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

  • Не прогонял ни один гейт (typecheck/test/build/смоки) — кода ещё нет, это предмет код-ревью (PROCESS.md §2.7), не ревью ТЗ.
  • Не проверял, существует ли отдельный ответ владельца на Q1/Q2 вне GitHub (Telegram и т.п.) — по прецеденту этого же репозитория (docs/reviews/SPEC-REVIEW-561-r1.md, идентичный паттерн: аналитика и вопрос в одном заходе, ни одного отдельного ответного комментария, default зафиксирован прямо текстом ТЗ) процесс не требует, чтобы решение владельца приходило тем же каналом или отдельным сообщением, что вопрос. Не переоткрываю этот вопрос повторно для #564 — трактую единообразно с прошлым раундом.
  • Не проверял построчно исходники #563/#449 на предмет точного механизма pinch-suppression — доверился статусу issue (#563 S8-merged) и связке, описанной в аналитике (разные классы дефекта: synthetic click после multi-touch у #563, статическое перекрытие неподвижного tap у #564).
  • Не проверял docs/ISOMETRIC.md построчно на предмет того, что hit-test в 3D действительно уже выполняется в финальных экранных координатах, как утверждает §5 ТЗ, — это заявление о существующей архитектуре, не новое продуктовое решение; если оно неточно, это находка код-ревью на этапе реализации, а не предмет ревью ТЗ.
  • Не оценивал производительность профилированием — AC8 доказывается source review плюс pre-beta performance gate, что не требует прогона на этапе ТЗ.

Вывод

Единственная блокирующая (в бюджете цикла) находка — M1, формально пропущенный обязательный пункт DoR (PROCESS.md §2.5 «затронутые файлы и модули»), не задевающий модель арбитража, AC или план тестов. High-находок нет. Вердикт — жёлтый: автор дописывает список ожидаемых файлов/модулей в тот же раздел ТЗ (по образцу #213 §15), без пересмотра остального текста, и документ уходит на второй заход.

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


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

  • Ветка: issue/564-dense-marker-hit, коммит dbb1da5d2511 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
  • Дерево материала: 3b54d2227e673ed2d387850c927987f1c6fafb0c
    git log --all --format='%H %T' | grep 3b54d2227e67
    
  • Тело issue: 32ca611dc6bd99561abbb7cee1295dd63d195a2f70469dd4a4922a5ebc46a498
  • Вердикт конвейера: yellow · High 0