From 0f7ec0235218a4a57cdc767941c6ea233c1a37f2 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Wed, 30 Sep 2026 23:26:42 +0000 Subject: [PATCH] docs: review document for #732 Issue: #732 User-Visible: no --- docs/reviews/CODE-REVIEW-732-r1.md | 163 +++++++++++++++++++++++++++++ 1 file changed, 163 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-732-r1.md diff --git a/docs/reviews/CODE-REVIEW-732-r1.md b/docs/reviews/CODE-REVIEW-732-r1.md new file mode 100644 index 00000000..6b0a2fb4 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-732-r1.md @@ -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). Диффа, который +менял бы видимую пользователем величину, нет — правка внутренняя. + +**Зелёный.** + +--- + + + +## Материал раунда + +- Ветка: `issue/732-iso-overlay-stubs`, коммит `9b2bb26d06eb` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `be4c320efecaf530c61c44d4a75a0ce30623e632` + ``` + git log --all --format='%H %T' | grep be4c320efeca + ``` +- Тело issue: `6c8a69ec6edbe327dfd35032e6913e49f538f1b7e99dc7f45ea3774f3c5f18cf` +- Вердикт конвейера: `green` · High 0