mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-01 20:29:00 +00:00
claude[bot]
@@ -0,0 +1,163 @@
|
||||
# CODE-REVIEW #732 · заход r1
|
||||
|
||||
Материал: `9b2bb26d06eba8c81bde1600117bfeed6eda08fa` (рабочая копия на нём же, `git log --oneline origin/dev..HEAD` — один коммит).
|
||||
Трек: show (до 3 AC, лёгкое код-ревью). Мутанты по диффу не запрашивались.
|
||||
|
||||
## Скоуп
|
||||
|
||||
После #714 и #724 в 2.5D-оверлеях остались пустышки: `renderIsoOverlayGrounds` и
|
||||
`renderIsoRaisedOverlays` безусловно возвращали пустой SVG, `groundRadius`
|
||||
вычислялся и только сравнивался, тестовые фикстуры носили пять полей
|
||||
(`view`, `referenceView`, `stageSize`, `layers`, `selectedDeviceId`), которых
|
||||
`IsoOverlaySceneInput`/`IsoOverlayRenderEntry` не читают.
|
||||
|
||||
Три AC:
|
||||
- **AC1** — пустые рендереры, вызовы и `groundRadius` удалены; DOM 2.5D не
|
||||
меняется.
|
||||
- **AC2** — фикстуры не носят непрочитанных полей, typecheck тестов это
|
||||
закрепляет.
|
||||
- **AC3** — мутанты не привязаны к удалённому коду, `mutation-gate --check`
|
||||
без новых предупреждений.
|
||||
|
||||
Правка внутренняя (`Нечего видит человек: Ничего`), трейлер `User-Visible: no`
|
||||
стоит — изменений CHANGELOG не требуется и нет.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Прочитан весь дифф (`src/iso-scene-render.ts`, `src/houseplan-card.ts`, 4
|
||||
тестовых файла). Дополнительно:
|
||||
|
||||
1. **AC1 доказан чтением, не только заявлением.** Сверил версию на
|
||||
`origin/dev`: `renderIsoOverlayGrounds`/`renderIsoRaisedOverlays` уже ДО
|
||||
этого коммита безусловно возвращали `emptySvg()` независимо от входа
|
||||
(src/iso-scene-render.ts:1117-1128 на dev). Значит удаление этих функций и
|
||||
двух интерполяций `${isoFrame?.grounds ?? nothing}` /
|
||||
`${isoFrame?.raised ?? nothing}` в `houseplan-card.ts` не может изменить
|
||||
отрисованный DOM — там и раньше ничего не рендерилось. Это сильнее, чем
|
||||
повторный прогон golden: логически исключает регрессию по построению, а не
|
||||
по выборке эталонов.
|
||||
Прогнал `node --test test/isometric-contract.test.mjs` — 14/14 зелёных,
|
||||
включая переписанные проверки «ничего не рендерится в overlay-поверхность»
|
||||
и «grounds/raised/groundRadius нигде не остались».
|
||||
2. **AC2.** Прогнал `node --test test/iso-overlay-fixture-types.test.mjs` —
|
||||
3/3 зелёных. Проверил дисциплину «тест умеет падать»: временно вернул
|
||||
`view: {...}` в один из литералов `overlayScene({...})` в
|
||||
`test/iso-scene-render.test.mjs` — первый чек теста немедленно покраснел с
|
||||
точной локацией (`test/iso-scene-render.test.mjs:346 ... 'view' does not
|
||||
exist in type 'OverlaySceneFixture'`), откатил правку, `git status`
|
||||
— чисто. Также прогнал `test/iso-scene-render.test.mjs` и
|
||||
`test/iso-stage6.test.mjs` целиком (после `npx tsc -p tsconfig.test.json`
|
||||
+ `fix-test-build.mjs`) — 46/46 зелёных.
|
||||
3. **AC3.** Прогнал `node scripts/mutation-gate.mjs --check` — `browser
|
||||
guards: 200/200`, `предупреждений mutation registry: 3`, ни одного `FAIL`.
|
||||
Проверил содержание трёх предупреждений — все про несовпадение
|
||||
`--test-name-pattern` с именами, содержащими `${…}` (#650), никак не
|
||||
связаны с `groundRadius`/`renderIsoOverlayGrounds`/`renderIsoRaisedOverlays`.
|
||||
Совпадает с заявлением автора «3, как на dev».
|
||||
4. Проверил по всему репозиторию (`src/`, `test/`, `docs/`), что удалённые
|
||||
имена (`renderIsoOverlayGrounds`, `renderIsoRaisedOverlays`,
|
||||
`groundRadius`) не остались висящими ссылками нигде, кроме архивного
|
||||
`docs/specs/471-*.md` (исторический документ, не правится) и самих
|
||||
регрессионных тестов, которые проверяют их отсутствие.
|
||||
5. Прогнал `node scripts/smoke-select.mjs --base origin/dev --head HEAD`:
|
||||
«Прямое совпадение» — 10 смоков по символу `cellCm` (строки диффа рядом со
|
||||
стёртыми полями просто содержат этот параметр по соседству, функционально
|
||||
он не менялся). Символы `grounds`/`raised`/`groundRadius` в выборке не
|
||||
всплыли — ожидаемо, эти смоки ходят по DOM, а не по именам символов.
|
||||
AC1 отдельно называет конкретные iso-смоки
|
||||
(`smoke_iso_flat_parity`, `smoke_isometric_contract`,
|
||||
`smoke_isometric_live_touch`, `smoke_iso_tiles`) и golden — автор в
|
||||
комментарии заявляет их зелёными; учитывая пункт 1 (DOM гарантированно не
|
||||
меняется по построению), решение не перегонять браузерные смоки и golden
|
||||
самостоятельно.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- `IsoFramePresentation.grounds`/`.raised` и обе привязки в карточке убраны
|
||||
вместе; `IsoFramePresentation.overlays` — отдельное поле, используется
|
||||
(`isoFrame?.overlays` → `isoOverlaySceneBounds`/fit, houseplan-card.ts:10719),
|
||||
дальше живёт не тронутым.
|
||||
- `emptySvg()` остаётся нужной — используется в `renderIsoUnderlay`,
|
||||
`renderIsoShadows`, `renderIsoWalls`; не мёртвый код.
|
||||
- `isoRaisedOverlayHalfSize` (читается в `iso-stage6.test.mjs`) не removed
|
||||
— проверено, что функция осталась и не была спутана с удаляемыми.
|
||||
- Переписанные тесты `#724 AC2` и `#713 AC3` осмысленно переносят проверку
|
||||
«зум/ресайз не двигают раскладку» на продакшн-путь (через `resolveIsoScene`
|
||||
→ `liveFrame`), а не просто удаляют покрытие — заменяющий кейс
|
||||
(`zoomedIn`/`zoomed` в `test/iso-stage6.test.mjs`) сам по себе проверяет то
|
||||
же инвариант через структурный кэш.
|
||||
- Трейлеры `Issue: #732`, `User-Visible: no` на месте; CHANGELOG не правится
|
||||
— согласовано с «Нечего видит человек: Ничего».
|
||||
|
||||
## Чего не проверял и почему
|
||||
|
||||
- `npx tsc --noEmit`, `npm test`, `npm run build` целиком — не перегонял:
|
||||
Validate на этом же SHA зелёный
|
||||
(https://github.com/Matysh/houseplan-card/actions/runs/36790189760),
|
||||
бюджет §4 не тратится повторно. Частично (`tsc -p tsconfig.test.json` +
|
||||
конкретные тестовые файлы) прогнал точечно ради проверки AC, не как замену
|
||||
гейту.
|
||||
- `npm run golden:verify` (капча браузером) — не прогонял: метки `ci:golden`
|
||||
на issue нет, и (см. п.1 выше) отрисовка `iso-overlays-svg` была пустой уже
|
||||
на dev, так что 19/19 эталонов, заявленные автором, логически не могут
|
||||
измениться этим диффом. Останавливаться на повторном браузерном прогоне
|
||||
ради этого посчитал избыточным для трека show.
|
||||
- Остальные iso-смоки (`smoke_iso_flat_parity`, `smoke_isometric_contract`,
|
||||
`smoke_isometric_live_touch`, `smoke_iso_tiles`) браузером сам не гонял —
|
||||
Chromium не поднимал (#696: тело issue явно смоук не называет; AC1
|
||||
использует их как способ доказательства, но доказательство DOM-инварианта
|
||||
получено чтением кода надёжнее любого выборочного прогона). Решение по
|
||||
строке из smoke-select: все 10 «прямых совпадений» по `cellCm` — не
|
||||
прогонял отдельно, полагаюсь на п.1 (grounds/raised уже были инертны) и на
|
||||
зелёный Validate.
|
||||
- `npm run invariants` — не прогонял: дифф не трогает геометрию модели и
|
||||
ссылки на неё, только убирает неиспользуемое поле и мёртвые функции
|
||||
рендера; расчёт раскладки (`buildIsoOverlayRenderScene`,
|
||||
`isoRaisedOverlayHalfSize`) не менялся.
|
||||
- `python -m pytest tests_backend` — не прогонял: дифф не касается
|
||||
`custom_components/**/*.py`.
|
||||
- Ручного тестирования в браузере не делал (вне бюджета трека show и не
|
||||
требуется отдельным AC сверх перечисленного).
|
||||
|
||||
## Находки
|
||||
|
||||
Блокирующих (High) и находок в скоупе (Medium) нет.
|
||||
|
||||
Вне скоупа, не заводил отдельным issue — оценил как Low, не Medium: автор
|
||||
сам явно называет в комментарии два кандидата на будущую уборку —
|
||||
`IsoOverlayFitEnvelopeInput.stageSize` (объявлено, передаётся из карточки,
|
||||
не читается — проверил чтением: `resolveIsoOverlayFitEnvelope`,
|
||||
src/iso-scene-render.ts:646-666, поле действительно нигде не используется в
|
||||
теле функции) и пустой `iso-overlays-svg` (можно убрать ценой смены DOM,
|
||||
требует отдельного решения). Оба — нулевой функциональный риск (один
|
||||
неиспользуемый опциональный параметр, один инертный DOM-узел, на который
|
||||
явно опираются смоки), явно зафиксированы в публичном комментарии к issue, и
|
||||
их самостоятельная правка означала бы расширение скоупа трека show сверх
|
||||
заявленных 3 AC. Открывать отдельный issue ради поля, которое ничего не
|
||||
стоит и не рискует, счёл избыточным для этого захода; если ревьюер
|
||||
следующего раунда сочтёт иначе — материала для S1-new достаточно
|
||||
(см. абзац выше).
|
||||
|
||||
## Вердикт
|
||||
|
||||
Все три AC доказаны: автотестом с подтверждённой способностью падать (AC2),
|
||||
прогоном с нулевым числом FAIL и предупреждениями, не связанными с диффом
|
||||
(AC3), и разбором по коду с чтением прежней и новой версии функций,
|
||||
подкреплённым прогоном обновлённых regression-тестов (AC1). Диффа, который
|
||||
менял бы видимую пользователем величину, нет — правка внутренняя.
|
||||
|
||||
**Зелёный.**
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/732-iso-overlay-stubs`, коммит `9b2bb26d06eb` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `be4c320efecaf530c61c44d4a75a0ce30623e632`
|
||||
```
|
||||
git log --all --format='%H %T' | grep be4c320efeca
|
||||
```
|
||||
- Тело issue: `6c8a69ec6edbe327dfd35032e6913e49f538f1b7e99dc7f45ea3774f3c5f18cf`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user