mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-05 06:08:59 +00:00
claude[bot]
@@ -0,0 +1,197 @@
|
||||
# CODE-REVIEW — issue #533, заход r1
|
||||
|
||||
**Материал:** `acea695069075fb1bd7592f3f2b38853bc9f4c32` (единственный коммит
|
||||
поверх `origin/dev`, ветка `issue/533-resize-witness-mapping`, приведена к
|
||||
`dev` конвейером: `97666f9b -> acea6950`, +2 коммита dev). Разбор полный
|
||||
(после ребейза — другой код, §7.2), но это первый заход ревью, поэтому
|
||||
разделы «Закрытие раунда» и «Унаследовано» не применяются.
|
||||
|
||||
**Диапазон:** `git diff origin/dev...HEAD --stat` → один файл,
|
||||
`demo/smoke_room_resize.mjs` (+69/-64). Продуктовый код не тронут — класс B
|
||||
(гейты и инструментарий), трейлеры коммита: `Issue: #533`, `User-Visible: no`
|
||||
— соответствует (чейнджлог не требуется).
|
||||
|
||||
## Скоуп
|
||||
|
||||
Трек `trivial`. Свидетель `demo/smoke_room_resize.mjs` считал экранные точки
|
||||
жеста один раз, заранее, через `svg.getScreenCTM()`, а карточка переводит их
|
||||
обратно в момент события — при рассинхронизации раскладки между замером и
|
||||
жестом (раннер против локальной машины) резайз коммитил не ту величину, и
|
||||
чистый прогон падал четырьмя немыми `expected true, got false`. Три AC из
|
||||
тела issue:
|
||||
|
||||
- **AC1** — точка каждого события берётся непосредственно перед отправкой (не
|
||||
один раз заранее); `pointermove`/`pointerup` целятся в ту же ручку, что и
|
||||
`pointerdown`; смок падает, если событие не доставлено.
|
||||
- **AC2** — новая проверка `safe_resize.mapping_stable`: масштаб
|
||||
`getScreenCTM` на `pointerdown` и `pointermove` совпадает в пределах 1e-6,
|
||||
с обоими масштабами и размерами стейджа в сообщении.
|
||||
- **AC3** — смок зелёный локально и на раннере на материале кандидата;
|
||||
штатный мутант `safe-resize-commit-preflight-bypassed` по-прежнему
|
||||
ловится.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
**Прогнал сам:**
|
||||
- `node demo/smoke_room_resize.mjs` локально (после `npm run build` +
|
||||
`npm run bundle:sync`, свежий бандл): **зелёный**, `OK`, 0 провалов; вывод
|
||||
подтверждает `mapping_stable` (`scale` совпадает на `down`/`move`) и
|
||||
правильный `safeResizeResult` (`[450, 450, 450, 0.45]`).
|
||||
- Эксперимент на «умеет ли тест падать» для найденного пробела (см.
|
||||
находку ниже): временно испортил цель `pointermove` в сценарии
|
||||
`mixed_role` (`cx: 999, cy: 999` — заведомо не существующая ручка),
|
||||
прогнал смок повторно — он остался зелёным (`OK`), хотя событие физически
|
||||
не было доставлено. Файл восстановлен из бэкапа сразу после эксперимента,
|
||||
`git status` — чисто.
|
||||
- `node scripts/smoke-select.mjs --base origin/dev --head HEAD` →
|
||||
«Исполняемого frontend-диффа нет (`src/**/*.ts` не тронут). Browser-smoke
|
||||
этим диффом не выбираются — выбирать нечего».
|
||||
|
||||
**Не прогонял, и почему:**
|
||||
- `npx tsc --noEmit`, `npm test`, `npm run build` (сверка бандла),
|
||||
`node scripts/check-docs.mjs` — дёшевые гейты уже зелёные на этом SHA:
|
||||
Validate [run 34621275434](https://github.com/Matysh/houseplan-card/actions/runs/34621275434)
|
||||
завершился `success`, job «Переиспользование: это дерево уже проверено» —
|
||||
`success`, `check-docs` не применим — diff не трогает `src/**`.
|
||||
- `npm run invariants` — diff не трогает геометрию, `layout`,
|
||||
`marker.space`, `open_spans`, толщины стен: правок продуктового кода нет
|
||||
вообще.
|
||||
- `npm run golden:verify` — diff не может изменить рендер (только текст
|
||||
свидетеля).
|
||||
- `python -m pytest tests_backend -q` — `custom_components/**/*.py` не
|
||||
тронут.
|
||||
- performance-профили — не названы в AC, чувствительные пути не тронуты.
|
||||
- Полный набор `demo/smoke_*.mjs` — не запускал; `smoke-select` явно сказал
|
||||
«выбирать нечего» (нет диффа в `src/**`), а AC называют только
|
||||
`smoke_room_resize.mjs`, который я прогнал напрямую.
|
||||
|
||||
Дополнительно проверил на самом SHA `acea6950` через
|
||||
`gh api .../commits/.../check-runs`: все шесть шардов «Мутанты по диффу»
|
||||
(включая «Мутанты по диффу (2/6)» — именно тот шард, что падал в исходном
|
||||
инциденте) — **success**. Это прямое подтверждение AC3 на раннере: этот job
|
||||
сначала гоняет чистый прогон затронутых свидетелей, потом мутанты по ним —
|
||||
зелёный исход означает и «чистый прогон больше не мигает», и «мутант
|
||||
по-прежнему ловится» одновременно.
|
||||
|
||||
## Разбор по AC
|
||||
|
||||
**AC1 — частично.** Перевод координат теперь происходит внутри того же
|
||||
`page.evaluate`, что и отправка события (`pointer()`, строки 63–75) — единый
|
||||
кадр для всех 22 вызовов в файле, `screenPt` удалён полностью. Целевая ручка:
|
||||
во всех местах, где раньше `pointermove`/`pointerup` вызывались без `cx/cy`
|
||||
(искали «первую незаблокированную ручку»), теперь передаётся то же `cx/cy`,
|
||||
что и в `pointerdown` — проверил построчно все 8 сценариев (главный,
|
||||
`disabled_no_drag`, `mixed_role`, `owner_boundary`, `corner_clamped`,
|
||||
`preflight`, `commit_preflight`, `cancel`) — везде консистентно. Отдельно
|
||||
проверил по коду (`_rszRooms()` → `houseplan-editor-runtime.ts:3250`), что
|
||||
ручки рендерятся от персистентной модели (`space.rooms`), а не от live-превью
|
||||
резайза, — значит `cx/cy` ручки не плывёт в процессе перетаскивания и поиск
|
||||
по исходным координатам остаётся корректным на всём жесте (проверено
|
||||
чтением, не исполнением).
|
||||
|
||||
Требование «смок падает, если событие не было доставлено» выполнено только
|
||||
для **трёх** из 22 вызовов `pointer()` — обёрнуты в `sent()` только
|
||||
`down_sent`/`move_sent`/`up_sent` главного сценария (строки 129, 131, 143).
|
||||
Остальные 19 вызовов (`disabled_no_drag`, все три события `mixed_role`,
|
||||
`owner_boundary`, `corner_clamped`, все четыре `preflight`, оба
|
||||
`commit_preflight`, все три `cancel`) по-прежнему отбрасывают возвращаемое
|
||||
значение — тот же паттерн, который AC1 прямо называет дефектом. Экспериментом
|
||||
подтвердил: подмена `cx/cy` на заведомо несуществующую ручку в `mixed_role`
|
||||
(`pointermove`) делает событие недоставленным, но смок остаётся зелёным. Это
|
||||
не гипотетический риск — именно так текущая версия способна молча
|
||||
пропустить регресс доставки событий в любом из этих 7 сценариев, ровно то,
|
||||
от чего должен защищать AC1.
|
||||
|
||||
**AC2 — выполнено.** `safe_resize.mapping_stable` (строки 132–137) сравнивает
|
||||
`grab.scale` и `moved.scale` с допуском `1e-6`, имя проверки содержит оба
|
||||
масштаба и оба размера стейджа. Локальный прогон подтверждает: `scale`
|
||||
идентичен на `down`/`move` (`0.6144444290587165` в обоих), проверка проходит.
|
||||
|
||||
**AC3 — выполнено.** Локально три прогона (де-факто больше — при отладке
|
||||
находки) стабильно зелёные. На раннере: все шесть шардов «Мутанты по диффу»
|
||||
на `acea6950` зелёные, включая ранее падавший 2/6 — свидетельство того, что
|
||||
раннер больше не мигает и что мутанты (в т.ч.
|
||||
`safe-resize-commit-preflight-bypassed`) по-прежнему отлавливаются.
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium (в скоупе задачи) — AC1 закрыт частично: 19 из 22 вызовов `pointer()` не проверяют доставку события
|
||||
|
||||
**Файл:** `demo/smoke_room_resize.mjs`, сценарии `mixed_role` (244–246),
|
||||
`owner_boundary` (263–265), `corner_clamped` (292–294), `preflight`
|
||||
(315–323), `commit_preflight` (342, 352), `cancel` (368–370),
|
||||
`disabled_no_drag` (212).
|
||||
|
||||
**Симптом:** только `down_sent`/`move_sent`/`up_sent` в главном сценарии
|
||||
(129, 131, 143) оборачивают `pointer()` в `sent()`. Остальные 19 вызовов
|
||||
по всему файлу игнорируют `result.sent`, как это делал старый код.
|
||||
|
||||
**Воспроизведение:** временно заменил `cx: 100, cy: 928` на `cx: 999, cy: 999`
|
||||
во втором `pointer('pointermove', …)` сценария `mixed_role` (строка 245) —
|
||||
целевая ручка перестаёт находиться, `dispatchEvent` не вызывается
|
||||
(`target?.dispatchEvent` с `target === undefined`). Смок тем не менее
|
||||
выводит `OK` без единого провала: последующие проверки
|
||||
(`safe_resize.mixed_role_no_drag`, `safe_resize.mixed_role_geometry_exact`)
|
||||
случайно совпадают с «жест не состоялся», потому что drag и так не должен
|
||||
двигать геометрию в этом сценарии. Изменение отменено сразу после проверки,
|
||||
рабочее дерево чистое.
|
||||
|
||||
**Почему это в скоупе, а не отдельный issue:** AC1 сформулирован в issue
|
||||
как общее требование к свидетелю («смок падает, если событие не было
|
||||
доставлено»), без ограничения одним сценарием, и правка находится в том же
|
||||
файле и в той же задаче. Это ровно тот класс дефекта, который спровоцировал
|
||||
инцидент (немой булев результат вместо явного провала) — в оставшихся семи
|
||||
сценариях `pointerdown`/`pointermove`/`pointerup` может провалиться молча, и
|
||||
тест этого не заметит. Цена исправления — обернуть оставшиеся вызовы в
|
||||
`sent()`, как уже сделано для главного сценария.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Перевод координат «в момент события» — for всех 22 вызовов, `screenPt`
|
||||
удалён без остатка (`grep` по репозиторию не находит других мест
|
||||
использования).
|
||||
- Консистентность цели `pointermove`/`pointerup` с `pointerdown` — проверено
|
||||
построчно по всем 8 сценариям.
|
||||
- `mapping_stable` — реализация и допуск (`1e-6`) соответствуют AC2, имя
|
||||
проверки печатает диагностику, как требуется.
|
||||
- Ручки не двигаются во время live-резайза (рендерятся от персистентной
|
||||
модели, не от превью) — значит поиск ручки по исходным `cx/cy` безопасен
|
||||
на протяжении всего жеста, включая многошаговые сценарии.
|
||||
- `nudge` — корректно реализованная замена прежнего «второй `pointermove` на
|
||||
чуть другую точку»: теперь это явно тот же план-таргет плюс пиксельный
|
||||
дребезг, что точнее соответствует намерению проверки
|
||||
`preflight_reason_once` (дребезг указателя не должен размножать toast).
|
||||
- Удаление поля `safeResizePoints` из `finish()` — не используется больше
|
||||
нигде в репозитории, безопасно.
|
||||
- Трейлеры коммита корректны: `Issue: #533`, `User-Visible: no`, чейнджлоги
|
||||
не тронуты и не требуются (класс B, без пользовательского эффекта).
|
||||
- Продуктовый код не тронут ни единой строкой — соответствует «Вне объёма»
|
||||
из ТЗ.
|
||||
|
||||
## Чего не проверял и почему
|
||||
|
||||
Список дан выше, в разделе «Как проверялось» → «Не прогонял, и почему»:
|
||||
typecheck/test/build/check-docs (уже зелёные на этом SHA, дешёвые гейты),
|
||||
инварианты модели, golden, pytest, перф-профили, полный набор смоков — по
|
||||
всем diff не даёт оснований их гонять.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Находка Medium — в скоупе задачи, блокирующего High нет. По PROCESS.md §7.2
|
||||
это жёлтый вердикт: правка возвращается автору для дозакрытия AC1 (обернуть
|
||||
оставшиеся 19 вызовов `pointer()` в `sent()` тем же способом, что и три уже
|
||||
обёрнутых), без создания отдельного issue.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/533-resize-witness-mapping`, коммит `acea69506907` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `123a41ad67f07b692c0b8e6f0ff9b6da4746510e`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 123a41ad67f0
|
||||
```
|
||||
- Тело issue: `ae465893153dbb7ba1b9d991e2d5cf4a05c8f526b51ae2f2b74e6e2c566805d9`
|
||||
- Вердикт конвейера: `yellow` · High 0
|
||||
Reference in New Issue
Block a user