From f07e3c54ffea88b0d77b12065bf318fc820d0ebf Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Mon, 31 Aug 2026 05:06:43 +0000 Subject: [PATCH] docs: review document for #400 Issue: #400 User-Visible: no --- docs/reviews/CODE-REVIEW-400-r1.md | 196 +++++++++++++++++++++++++++++ 1 file changed, 196 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-400-r1.md diff --git a/docs/reviews/CODE-REVIEW-400-r1.md b/docs/reviews/CODE-REVIEW-400-r1.md new file mode 100644 index 00000000..e1d35dec --- /dev/null +++ b/docs/reviews/CODE-REVIEW-400-r1.md @@ -0,0 +1,196 @@ +# CODE-REVIEW-400-r1 + +- Issue: https://github.com/Matysh/houseplan-card/issues/400 +- ТЗ: `docs/specs/400-beta-polish.md` (r1), ревью ТЗ зелёное (комментарий 2026-08-31T04:44:56Z) +- Диапазон: `origin/dev..HEAD`, HEAD на момент разбора и на момент вердикта — `73ecfbd1d74a6d194f62e8e83766294018f8eb81` +- Заход r1 (первый код-ревью этой задачи), блокирующих циклов израсходовано 0 из 4 + +## Скоуп + +Три несвязанные правки из ТЗ: + +1. Угловые ручки рамки декора (`.dtframe`) рисуются последними, чтобы забирать хит у осевых на мелкой мебели (AC1/AC2). +2. `_alignCandidates` в режиме `devices` исключает перетаскиваемый маркер по `_deviceDrag`, а не по мёртвому `_drag` (AC4/AC5). +3. Решение по 38 help-строкам #86 зафиксировано в `docs/ARCHITECTURE.md`: остаются в initial-чанке, с числом и причиной (AC3/AC6). + +Коммиты: `60c211c8` (ТЗ), `532743b0` (документ ревью ТЗ), `fe3b85c0` (реализация, +оба changelog в одном коммите с `User-Visible: yes`), `73ecfbd1` (пересборка +бандлов + пересъёмка двух doc-скриншотов, `User-Visible: no`). Трейлеры на всех +четырёх в порядке. + +## Как проверялось + +Материал — `git diff origin/dev...HEAD`, без ручного тестирования. Ниже — +таблица гейтов с результатами; часть прогнана заново, часть свежих чисел взята +из хендоффа и перепроверена. + +| Гейт | Команда | Результат | +|---|---|---| +| typecheck | `npx tsc --noEmit` | чисто | +| unit | `npm test` | 1668 tests, 1667 pass, 0 fail, 1 skipped — совпадает с хендоффом | +| build + сверка бандлов | `npm run build && cmp dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js` | совпадают; `npm run bundle:sync` не внёс изменений (дерево уже синхронизировано в `73ecfbd1`) | +| any-гейт | `node scripts/no-new-any.mjs --base origin/dev --head HEAD` | 24 добавленные строки, новых `any` нет | +| check-docs | `node scripts/check-docs.mjs` | обязателен, т.к. diff трогает `src/**`: чисто (7 файлов, 10 внешних ссылок) | +| бюджет | `npm run bundle:budget` | initial 285 425 / 300 000 — не вырос против заявленного; отдельный варнинг про запас < 15 000 Б — фоновый (#367), не следствие этой задачи | +| смок-выборка | `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | 8 «прямых совпадений» по символам `_drag`/`_deviceDrag`: `smoke_align_guides`, `smoke_drag_bounds`, `smoke_decor`, `smoke_device_position_history`, `smoke_grid_snap`, `smoke_modes`, `smoke_optional_space_model`, `smoke_pan_any_zoom` | +| смоки (прямое совпадение + целевые AC) | `node demo/smoke_{align_guides,drag_bounds,decor,device_position_history,grid_snap,modes,optional_space_model,pan_any_zoom,furniture_polish}.mjs` | все 9 зелёные, прогнаны заново вживую | +| мутанты (полный, дорогой прогон — оправдан, т.к. это прямая проверка «тест умеет падать» для AC1/AC4) | `node scripts/mutation-gate.mjs --id=furniture-edge-handles-steal-the-corner` | **красный**: «тест остался зелёным на сломанном коде», поймано 0 из 1 — находка ниже | +| мутанты | `node scripts/mutation-gate.mjs --id=align-guides-exclude-dead-source` | зелёный: «тест покраснел, как обязан», поймано 1 из 1 | + +Не прогонялось: `npm run golden:verify` (обоснование — см. «Что проверено»), +`python -m pytest tests_backend` (diff не трогает `custom_components/**/*.py`), +`node scripts/model-invariants.mjs` (diff не трогает геометрию/ссылки на неё — +ни рёбер комнат, ни толщины, ни `layout`, ни `marker.space`, ни `open_spans`), +performance-профили (не названы в AC, diff не на горячем пути рендера плана), +полный набор смоков (диф — два файла с точечными правками, широкая выборка +`scripts/smoke-select.mjs` уже покрывает все прямые совпадения; расширять до +всего дерева здесь непропорционально задаче). + +## Находки + +### Medium (в скоупе задачи — чинится в этом issue) + +**M1. Мутант `furniture-edge-handles-steal-the-corner` не воспроизводит регрессию, которую обязан ловить (AC1).** + +`scripts/mutation-gate.mjs:749-759`. Патч мутанта: + +``` +find: " ${/* #400: corners LAST." +replace: " ${/* mutant: corners no longer last." +``` + +Это правка **внутри JS-комментария** в `src/houseplan-card.ts:8475` — она не +меняет порядок отрисовки `sides.map(...)` / `corners.map(...)`, только текст +внутри `/* ... */`. Комментарии не влияют на поведение шаблона: физический +порядок двух блоков в исходнике остаётся прежним (`sides` перед `corners`), +и «сломанный» бандл рендерит рамку так же, как исправленный. + +**Воспроизведено исполнением**, не расчётом: + +``` +$ node scripts/mutation-gate.mjs --id=furniture-edge-handles-steal-the-corner +ok чистый прогон: node demo/smoke_furniture_polish.mjs +FAIL furniture-edge-handles-steal-the-corner: тест остался зелёным на сломанном коде +поймано 0 из 1 +``` + +Второй мутант (`align-guides-exclude-dead-source`) свою регрессию ловит верно +(«поймано 1 из 1», проверено тем же способом) — дефект локален к первому пункту. + +Коммит `fe3b85c0` утверждает «Both mutants run by hand: reverting the paint +order reddens the 40 cm probe» — это, видимо, было проверено вручную корректной +правкой (перестановкой блоков), но то, что легло в постоянный реестр +`scripts/mutation-gate.mjs` (файл класса B, который будет гонять +`.github/workflows/mutation-gate.yml` перед каждым стабильным релизом без +участия человека), эту регрессию не ловит. AC1 в ТЗ прямо называет мутант как +доказательство «тест умеет падать» — сейчас это неверно для постоянного +артефакта, а не только для разового ручного прогона. + +**Почему Medium, а не High**: сама функциональная правка верна и доказана +живым браузерным смоком на реальном DOM (`smallFurnitureCornerWinsTheHit: true`, +`largeFurnitureKeepsBothHandles: true` — прогнано заново вживую, не только по +хендоффу). Пользователь получает исправленное поведение уже сейчас. Ломается +не продукт, а будущая защита от регресса — обнаружится не раньше, чем перед +следующим стабильным релизом, когда её прогонят по-настоящему, и к тому +моменту будет поздно понять, что она никогда не работала. + +**Ожидаемо**: патч мутанта должен реально возвращать `corners` перед `sides` +(или иным способом менять фактический порядок рендера), а не текст комментария. +Проверка приёмки: `node scripts/mutation-gate.mjs --id=furniture-edge-handles-steal-the-corner` → «поймано 1 из 1». + +### Low (снимаются с записью, доработки не требуют) + +**L1. AC4 доказан не тем видом теста, что назван в ТЗ.** + +План тестов ТЗ называет для AC4 «юнит на `_alignCandidates`» +(`test/align-candidates.test.mjs` либо существующий файл). Такого юнит-теста в +диффе нет — `git diff --stat -- test/` пуст. Вместо этого AC4 проверяется +браузерным смоком `demo/smoke_align_guides.mjs` (`devGuideComesFromAnotherMarker`), +который вызывает тот же `_alignCandidates()` напрямую на живом инстансе и +сравнивает списки кандидатов с активным `_deviceDrag` и без — по существу то +же утверждение, что просило ТЗ, просто через e2e, а не через изолированный +юнит. Проверено исполнением: смок зелёный, а отрицательный прогон мутанта +`align-guides-exclude-dead-source` подтверждает, что проверка умеет падать. +Снимаю без правок: доказательство сильнее, чем просило ТЗ (реальный DOM вместо +изолированного вызова), просто не в той форме файла. + +**L2. Число «2 654 Б gzip» для 38 help-строк в `docs/ARCHITECTURE.md` не +перепроверено независимо.** + +Прочитано и признано правдоподобным (38 ключей, общий вес словарей 47 754 Б +gzip, итоговый бюджет 285 425 — совпадает с хендоффом), но точный вес именно +этих 38 ключей отдельно не измерялся мной — потребовал бы отдельного скрипта +подсчёта, которого в репозитории нет. AC3 требует записанного решения с +причиной, а не независимого замера ревьюером; решение и причина на месте. +Снимаю без правок. + +## Что проверено и корректно + +- **AC1/AC2** (приоритет угловой ручки на мелкой мебели, отсутствие регрессии + на крупной) — доказаны исполнением: `node demo/smoke_furniture_polish.mjs` + зелёный, диагностика `handleProbeDiagnostics` показывает ровно одно + срабатывание на угловой ручке и ноль на осевой для 40 см и для 160 см. + Порядок отрисовки в `src/houseplan-card.ts:8467-8485` подтверждён чтением: + `sides.map` идёт раньше `corners.map`, радиусы хита идентичны (`hr` общий на + `:8440`). +- **AC4** — доказан исполнением через смок (см. L1); чтением подтверждено, что + `_alignCandidates` (`src/houseplan-editor-runtime.ts:10970-10979`) теперь + берёт `this.host._deviceDrag?.id ?? this.host._drag?.id`, и что в режиме + `devices` `_drag` действительно остаётся `null` (не тронуто этим диффом, + проверено чтением `houseplan-card.ts:2547` и присваиваний `_drag` — ни одно + не относится к ветке `devices`). +- **AC5** — доказан исполнением, включая отрицательный прогон мутанта + («поймано 1 из 1»): смок различает направляющую от другого маркера и от + себя, сравнением списков кандидатов, а не по заранее посчитанной точке — + учтён риск гонки координат, названный в ТЗ. +- **AC3** — решение (2) «en/ru неделимы» записано в `docs/ARCHITECTURE.md` с + причиной (стоимость раздельной поставки против экономии), проверено чтением. +- **AC6** — бюджет initial не вырос: `npm run bundle:budget` = 285 425 / 300 000, + прогнано заново, совпадает с хендоффом. +- Интерфейс `HouseplanEditorHostPort._deviceDrag` — новое поле только в типе, + реальное приватное поле `_deviceDrag` в `HouseplanCard` существовало и до + этого диффа (`houseplan-card.ts:2548`, не изменено) — подтверждено чтением, + `tsc --noEmit` подтверждает совместимость типов. +- Трейлеры: все четыре коммита несут `Issue: #400` и корректный + `User-Visible:`; `User-Visible: yes` (`fe3b85c0`) правит оба changelog в том + же коммите — подтверждено `git show --stat`. +- Видимых числовых значений, которые пользователь видел бы дважды, в этом + диффе нет (правки — хит-тестинг и внутренний список кандидатов + направляющих, не отображаемые величины) — «одно число, один источник» не + применимо. +- Диф не трогает геометрию комнат/толщины/`layout`/`marker.space`/`open_spans` + — инварианты модели не требуются. + +## Чего не проверял и почему + +- **`npm run golden:verify`** — не прогонял. Обоснование: правка (1) меняет + DOM-порядок двух групп невидимых (прозрачных) хит-кругов и парных с ними + `.dtknob`; сами бусины позиционно не пересекаются друг с другом (углы и + середины сторон — разные точки), поэтому перестановка групп не может менять + итоговые пиксели рамки редактирования декора. Риск назван и разобран в самом + ТЗ («перестановка не должна менять картинку»). Golden-баз для оверлея рамки + редактирования декора в матрице предположительно нет (оверлей — состояние + выделения в редакторе, а не View); не проверял это утверждение отдельным + прогоном golden, полагаюсь на разбор по коду. +- **`python -m pytest tests_backend`** — не прогонял, diff не касается + `custom_components/**/*.py`. +- **`node scripts/model-invariants.mjs`** — не прогонял, diff не касается + геометрии. +- **Полный `ls demo/smoke_*.mjs` (209 смоков)** — не прогонял; прогнаны 9, + названные `scripts/smoke-select.mjs` как прямые совпадения, плюс целевые + `align_guides`/`furniture_polish` из AC. Полный набор — предрелизный гейт. +- **Точный байтовый вес 38 help-строк отдельно** — не перемерял (см. L2). +- **Ручное DOM-тестирование в браузере вне смоков** — не проводилось; в этом + процессе фазы ручного тестирования нет, автотесты и код-ревью её заменяют + (см. таблицу гейтов выше). + +## Итог + +Один Medium в скоупе задачи (M1), без High — жёлтый вердикт. Функциональные +правки (1) и (2) верны и доказаны исполнением; постоянный регресс-гейт для +пункта (1) сломан и не защищает от отката ровно той регрессии, которую +описывает ТЗ. Правка ожидается в этом же issue — заменить патч мутанта +`furniture-edge-handles-steal-the-corner` на такой, что реально возвращает +`corners` перед `sides`, и подтвердить прогоном +`node scripts/mutation-gate.mjs --id=furniture-edge-handles-steal-the-corner` +(«поймано 1 из 1»).