From e74823737f519a5b52621b1372aacbf53e49e4c0 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Fri, 11 Sep 2026 16:31:13 +0000 Subject: [PATCH] docs: review document for #533 Issue: #533 User-Visible: no --- docs/reviews/CODE-REVIEW-533-r1.md | 197 +++++++++++++++++++++++++++++ 1 file changed, 197 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-533-r1.md diff --git a/docs/reviews/CODE-REVIEW-533-r1.md b/docs/reviews/CODE-REVIEW-533-r1.md new file mode 100644 index 00000000..83b9ec1a --- /dev/null +++ b/docs/reviews/CODE-REVIEW-533-r1.md @@ -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. + +--- + + + +## Материал раунда + +- Ветка: `issue/533-resize-witness-mapping`, коммит `acea69506907` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `123a41ad67f07b692c0b8e6f0ff9b6da4746510e` + ``` + git log --all --format='%H %T' | grep 123a41ad67f0 + ``` +- Тело issue: `ae465893153dbb7ba1b9d991e2d5cf4a05c8f526b51ae2f2b74e6e2c566805d9` +- Вердикт конвейера: `yellow` · High 0