mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-06 22:49:16 +00:00
claude[bot]
@@ -0,0 +1,206 @@
|
||||
# CODE-REVIEW-714-r1
|
||||
|
||||
**Issue:** #714 «Удалить код поиска места значков #651 — после #713 он не вызывается»
|
||||
**Трек:** show · **Заход:** r1 · блокирующих циклов израсходовано 0/2
|
||||
**Материал:** `git log --oneline origin/dev..HEAD` = `8f09cdfd` (единственный коммит);
|
||||
`git diff origin/dev...HEAD` — 8 файлов, +112/-2561 строк.
|
||||
Рабочая копия проверялась ровно на `8f09cdfd1a6c8af0581e611af047b72b93a5bbbf`
|
||||
(подтверждено `git rev-parse HEAD` до и после прогонов). Ветка к `dev` не
|
||||
приводилась (трек show, #696) — это ожидаемо, не находка.
|
||||
|
||||
## Скоуп
|
||||
|
||||
Чисто техдолг: удаление кода раскладки #651 (`resolveIsoOverlayRigidGroups`,
|
||||
`resolveIsoOverlayCollisions`, поиск сдвига в `resolveIsoOverlayPlacement`,
|
||||
поля `nudge*`/`nearWall*`/`cleared`/`capped`/`status`/`reason`, константы
|
||||
`ISO_OVERLAY_MAX_NUDGE_CSS_PX`/`ISO_OVERLAY_SAFETY_GAP_CSS_PX`, быстрый путь
|
||||
при зуме в `iso-scene-render.ts`), которого живая сцена не вызывает с #713.
|
||||
User-Visible: no, что соответствует «Что видит человек: Ничего» в теле issue.
|
||||
Работа обслуживает не прямую строку `docs/SCOPE.md` Core user jobs, а
|
||||
поддержание J1–J7 читаемым кодом (сам issue и оценка владельца прямо называют
|
||||
это техдолгом ценностью для разработки, не для пользователя — легитимно по
|
||||
`docs/process/REVIEWER.md`, т.к. Rule #1 разрешает изменения класса A при
|
||||
принятой issue, а не только изменения, закрывающие Core user job впрямую).
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Прочитан весь дифф (`git diff origin/dev...HEAD`) файл за файлом, не по
|
||||
диагонали — `src/iso-overlays.ts` (2010→249 строк) прочитан целиком в
|
||||
итоговом виде, не только как дифф.
|
||||
|
||||
### Гейты — что прогнано и результат
|
||||
|
||||
| Гейт | Статус | Результат |
|
||||
|---|---|---|
|
||||
| Validate на `8f09cdfd` (tsc/test/build/bundle-policy) | подтверждён ссылкой | success, https://github.com/Matysh/houseplan-card/actions/runs/36771186918 |
|
||||
| `npx tsc --noEmit` (перепроверка) | прогнан | чисто, без ошибок |
|
||||
| `node --test test/iso-overlays.test.mjs test/iso-scene-render.test.mjs test/isometric-contract.test.mjs test/monolith-text-anchors.test.mjs` | прогнан | 53/53 green |
|
||||
| Точечная мутация в `resolveIsoOverlayPlacement` (`visualOffset → visualOffset*2`) | прогнана и откачена | 6 тестов покраснели — набор умеет падать, не только зеленеть |
|
||||
| Якоря 8 оставшихся мутантов `iso-overlays.ts`/`iso-scene-render.ts` в реестре | сверены скриптом | все 8 находят свой `find`-паттерн в текущем коде |
|
||||
| Якоря 7 удалённых мутантов | сверены | ни один `find`-паттерн больше не существует в `src/**` — удаление мутантов согласовано с удалением кода, не потеряло охват действующего кода |
|
||||
| `npm run golden:capture --scenario=<id>` для всех 21 2.5D-сцены (`isometric-*`, `stage3-*`, `stage6-*`, `wall-union-isolation-view-*`, `stairs-isometric-dark`) | прогнано по одной (`golden:verify` не даёт `--scenario`, полный набор — предрелизный гейт) | все 21 `passed` против принятых эталонов, включая 4 сцены с `requireOneRise` |
|
||||
| `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | прогнан | см. ниже |
|
||||
| `demo/smoke_isometric_live_touch.mjs` (прямое совпадение по `_openingsR`) | прогнан | все 44 поля `true`, `OK` |
|
||||
| `demo/smoke_pan_any_zoom.mjs` (прямое совпадение по `_baseVb`/`_stageEl`/`fitView` — именно то, что диф вынул из вызова сцены оверлеев) | прогнан | все поля `true`, `OK` |
|
||||
| `demo/smoke_iso_flat_parity.mjs` (слабая связь по `_baseVb`, но название прямо про изометрию) | прогнан | все поля `true`, `OK`, включая явный `wideAC3OneStraightUpShift`/`tallAC3OneStraightUpShift` — каждое устройство получает один и тот же вертикальный сдвиг |
|
||||
| `npm run mutation-gate -- --check` (полный реестр) | не прогнан | по правилу трека show мутанты в разработке не гоняются ни автором, ни ревьюером (#709); анкеры сверены вручную (строка выше) |
|
||||
| `npm run golden:verify` (полный набор) | не прогнан | полный набор — предрелизный гейт (§8), не гейт ревью; без метки `ci:golden`; диагностический прогон по всем 21 сценам сделан точечно через `capture` |
|
||||
| `python -m pytest tests_backend` | не прогнан | дифф не касается `custom_components/**/*.py` |
|
||||
| `npm run invariants` | не прогнан | дифф не меняет геометрию модели, только внутреннюю раскладку оверлеев поверх неизменной геометрии |
|
||||
| performance-профили | не прогнан | не названы в AC; `demo/benchmark_large_house.mjs` и `demo/benchmark_safe_resize.mjs` только читают `data-hp-iso-nudged` как константу — не запускались |
|
||||
|
||||
Вывод `smoke-select.mjs`: 22 «прямое совпадение», 31 «слабая связь», 1
|
||||
«зарегистрированная связь» (`demo/smoke_isometric_contract.mjs` ←
|
||||
`unprojectFloorPoint`, с пометкой «#713: … не названы смоком»),
|
||||
плюс несколько `pointInRing`-совпадений, не относящихся к удалённому коду
|
||||
(`pointInRing` используется и осталась в файле). Из 22 прямых прогнаны три
|
||||
самых релевантных диффу (иземетрия live-touch, panorama/zoom+fitView/_baseVb,
|
||||
iso-flat-parity); остальные 19 — по несвязанным символам (`_openingsR`,
|
||||
`pointInRing`, `Axis`) в файлах, к которым этот дифф не прикасается
|
||||
содержательно (класс: touch/drag/wall-thickness/opening-binding смоки, где
|
||||
`_baseVb`/`_openingsR` совпадают по имени символа, а не по изменённой
|
||||
логике) — решение: не гонять, слабая связь не обязывает.
|
||||
|
||||
**Chromium был установлен вручную** (`npx playwright install --with-deps
|
||||
chromium`) — отсутствовал в среде; тело issue явно не называет смоук, но
|
||||
AC1 прямо требует «golden проходит на принятых эталонах», что само по себе
|
||||
требование гейта задачи (не зависит от узкого правила про смоук-триггер).
|
||||
`127.0.0.1 demo.local` был дописан в `/etc/hosts` (требуется `demo/serve.mjs`,
|
||||
прецедент — `legacy/reviews/v1.69.0/CODE-REVIEW-39-r1.md`). После проверки
|
||||
рабочая копия репозитория возвращена в чистое состояние
|
||||
(`npm run bundle:clean`, `git checkout -- demo/srv/assets/icons.js`;
|
||||
`git status --short` пуст, `HEAD` = `8f09cdfd`).
|
||||
|
||||
## AC — разбор
|
||||
|
||||
**AC1 (unit + golden без правок).**
|
||||
Проверено чтением и исполнением. Список из тела issue — `resolveIsoOverlayRigidGroups`,
|
||||
`resolveIsoOverlayCollisions`, поиск сдвига, поля `nudge*`/`nearWall*`/`cleared`/
|
||||
`capped`, обе константы, быстрый путь при зуме в `iso-scene-render.ts` — отсутствует
|
||||
в итоговых файлах (`grep` по всем именам из списка дал 0 совпадений в `src/**`,
|
||||
кроме случайного текстового совпадения `safePointer`/`safePoint` в
|
||||
`demo/benchmark_safe_resize.mjs`, не относящегося к удалённому коду).
|
||||
`resolveIsoOverlayPlacement` теперь просто проецирует `floorAnchor` на
|
||||
`visualOffset` без поиска — независимо подтверждено: до #713 живая сцена уже
|
||||
вызывала резолвер с `wallSilhouettes: []`, из-за чего `nearWallBefore` всегда
|
||||
было `false`, а поисковая ветка — фактически мёртвой в проде ещё до этого
|
||||
коммита; #714 лишь убрал мёртвый код, а не поменял геометрическое поведение.
|
||||
Golden: все 21 2.5D-сцена (включая 4 с `requireOneRise`) проходят против
|
||||
принятых эталонов без новых кадров — перепроверено лично, не только со слов
|
||||
автора. Смоки `smoke_isometric_live_touch`, `smoke_pan_any_zoom`,
|
||||
`smoke_iso_flat_parity` зелёные. **AC1 выполнен, доказан исполнением.**
|
||||
|
||||
**AC2 (реестр мутантов).**
|
||||
7 мутантов удалены из `scripts/mutation-registry.mjs`
|
||||
(`iso-aabb-rejects-touching-wall`, `iso-rigid-groups-use-live-zoom-scale`,
|
||||
`iso-rigid-groups-split-close-row`, `iso-rigid-groups-cross-room-boundary`,
|
||||
`iso-rigid-fallback-drops-room-wall-overlap-priority`,
|
||||
`iso-scene-live-placement-search-returns`,
|
||||
`stage4-w11-nudged-overlay-restores-tether`) — подсчёт `id: '` в файле
|
||||
подтверждает дельту 1094→1087. Для каждого проверено, что его `find`-паттерн
|
||||
физически отсутствует в `src/iso-overlays.ts`/`src/iso-scene-render.ts` (скрипт
|
||||
выше) — значит эти мутанты не просто «показались лишними», они действительно
|
||||
целились в снесённый код, а не в код, который остался без защиты. 8 оставшихся
|
||||
мутантов в этих двух файлах нашли свои якоря. Точечная мутация, внесённая мной
|
||||
в упрощённую функцию (`visualOffset*2`), убила 6 тестов — подтверждает, что
|
||||
оставшееся покрытие не декоративное. `mutation-gate --check` не перезапускался
|
||||
мной: по правилу трека show мутанты в разработке не гоняет ни автор, ни
|
||||
ревьюер (#709, `docs/process/REVIEWER.md` «Трек show»); отсутствие прогона —
|
||||
не находка. **AC2 выполнен, доказан по коду + точечным исполнением.**
|
||||
|
||||
**AC3 (документация).**
|
||||
`docs/ISOMETRIC.md` правки прочитаны целиком: фраза «invisible collision
|
||||
footprints» → «invisible overlay footprints», абзац про #651-резолверы заменён
|
||||
на явную историческую справку «History: #651 used to search a place for every
|
||||
marker … #713 stopped calling that search, and #714 removed it». Текущее
|
||||
поведение описано корректно («a placement depends only on the anchor, its
|
||||
owner room, the footprint and the rise»). Раздел про 1.12×-масштаб маркеров
|
||||
избавлен от слова «collisions». Ни одного места, где раскладка #651 всё ещё
|
||||
описывалась бы как текущая, не найдено. **AC3 выполнен, проверено чтением.**
|
||||
|
||||
## Находки
|
||||
|
||||
Находок нет — ни High, ни Medium, ни Low.
|
||||
|
||||
Отдельно зафиксирую и снимаю два места, которые проверил специально, т.к. они
|
||||
похожи на потенциальные находки, но ими не являются:
|
||||
|
||||
1. **`resolveCollisions`/`collisionMode`/два слота кэша `fit`/`live`** в
|
||||
`iso-scene-render.ts` — это название и разделение кэша остались от эпохи
|
||||
поиска места, хотя результат `fit` и `live` теперь всегда совпадает (автор
|
||||
сам называет это в «Риски»: «лежат в разных слотах кэша; слить их —
|
||||
отдельная задача»). Не находка: вне списка AC1 (issue перечисляет
|
||||
конкретные функции/поля/константы дословно, этого параметра там нет), не
|
||||
меняет наблюдаемое поведение, задокументировано автором как осознанный
|
||||
остаток, слияние — это уже не «удаление», а рефакторинг с более широким
|
||||
риском, разумно оставить отдельной задаче.
|
||||
2. **`test('Stage 4 reuses pure overlay placements …')`** передаёт
|
||||
`view: {...}` и `stageSize: {...}` в `buildIsoOverlayRenderScene`, хотя оба
|
||||
поля убраны из `IsoOverlaySceneInput` — тест по-прежнему проходит, но
|
||||
лишние поля больше ничего не тестируют (сигнатура их не читает ни до, ни
|
||||
после). Не находка уровня Medium/Low: тест не стал ложным (не делает вид,
|
||||
что проверяет то, чего нет, — сами имена тестов описывают corrent
|
||||
поведение «resize/zoom is not a layout event», что верно и без этих полей),
|
||||
просто содержит несколько строк мёртвого фикстурного мусора. Не в скоупе
|
||||
issue (issue просит чистку `src/**`, не `test/**` сверх удалённых
|
||||
#585/#651 кейсов), эффекта на пользователя или регрессию не несёт.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Полное соответствие списка удалений из тела issue фактическому диффу.
|
||||
- Отсутствие мёртвых ссылок на удалённые символы во всём репозитории (кроме
|
||||
случайного текстового совпадения имени, не связанного с удалённым кодом).
|
||||
- `src/houseplan-card.ts`: вызовы `_isoOverlayScene`/`resolveIsoOverlayFitEnvelope`
|
||||
обновлены синхронно с новой сигнатурой (`view`/`stageSize` убраны из
|
||||
раскладки оверлеев, но `stageSize` для `resolveIsoOverlayFitEnvelope` —
|
||||
другого назначения (fit-огибающая K8) — оставлен верно, не удалён по
|
||||
ошибке). `data-hp-iso-nudged` захардкожен в `'false'`, что соответствует
|
||||
реальному поведению (нигде больше `nudged` не вычисляется) и тому, что
|
||||
читают golden/смоук/бенчмарк.
|
||||
- Трейлеры коммита: `Issue: #714`, `User-Visible: no` — корректны, changelog
|
||||
не тронут, как и требуется при `no`.
|
||||
- Мутанты и тесты синхронно вычищены без потери фактического покрытия
|
||||
действующего кода (проверено якорями и точечной мутацией).
|
||||
- Ни один тест, читающий монолит как текст, не добавлен к списку
|
||||
`test/monolith-text-anchors.test.mjs` — правка `isometric-contract.test.mjs`
|
||||
лишь обновляет regex существующего, уже учтённого файла.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Полный `npm run golden:verify` (все сцены матрицы, не только 21
|
||||
изометрическую) и полный `npm run mutation-gate -- --check` — предрелизные
|
||||
гейты, без метки `ci:golden`, объём ревью show их не требует; точечно все
|
||||
21 iso-сцены и все затронутые мутанты всё равно проверены отдельно.
|
||||
- 19 из 22 «прямых совпадений» `smoke-select` (touch/drag/wall-thickness/
|
||||
opening-binding смоки) и все 31 «слабых» — связаны с диффом только общим
|
||||
именем символа (`_baseVb`, `_openingsR`, `pointInRing`), не с изменённой
|
||||
логикой; решение — не гонять, слабая связь не обязывает, широкий символ (39
|
||||
изменённых строк, порог «широкого» — 56) не сработал.
|
||||
- `python -m pytest tests_backend`, `npm run invariants`,
|
||||
performance-профили — гейты не применимы к этому диффу (нет правок Python,
|
||||
геометрии модели или названных в AC перформанс-сценариев).
|
||||
- Ручная проверка в браузере (не headless-смоук/golden) — не выполнялась;
|
||||
доказательства строятся на golden-скриншотах и смоук-ассертах, а не на
|
||||
визуальном осмотре вживую.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Зелёный. Все три AC выполнены и доказаны исполнением (golden — лично
|
||||
перепрогнан на всех 21 сцене, а не принят со слов автора; unit-тесты —
|
||||
перепрогнаны и дополнительно проверены точечной мутацией на способность
|
||||
падать; реестр мутантов — сверен якорями для всех затронутых мутантов, и
|
||||
удалённых, и оставшихся). Находок нет.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/714-drop-651-search`, коммит `8f09cdfd1a6c` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `569e21b91ea4863052ae294726e4bf0111b1fb61`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 569e21b91ea4
|
||||
```
|
||||
- Тело issue: `265208f3d6c426f1a44691f2acb0cf9e77464441e7acf3e9e0cec777615aeefb`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user