docs: review document for #713

Issue: #713
User-Visible: no
This commit is contained in:
claude[bot]
2026-09-30 14:23:03 +00:00
parent 80e3dba700
commit 5c0e8668f1
+242
View File
@@ -0,0 +1,242 @@
# CODE-REVIEW-713-r1
## Материал раунда
- Issue: [#713](https://github.com/Matysh/houseplan-card/issues/713) · трек `track:ask` · заход **r1** · блокирующих циклов израсходовано 0 из 4.
- Ветка `issue/713-oblique-projection`, вершина **`80e3dba70024d91aee7c019ca5159534cea58f3d`** (рабочая копия уже на нём).
- Диапазон материала: `git log --oneline origin/dev..HEAD` — 4 коммита:
- `0d641ffb` feat(iso): the 2.5D floor is the Flat plane, one wall-top rise for tiles (#713)
- `6da6d9c6` test(golden): Stage 4 overlay scenes check the one wall-top rise (#713)
- `c2c806fa` docs(screenshots): source fingerprint for #713 — all 11 frames pixel-identical
- `80e3dba7` test(golden): accept the 19 reviewed 2.5D frames of #713
- ТЗ ревьюировано ранее (`SPEC-REVIEW-713-r1/r2`, зелёный вердикт r2). Это первый код-ревью раунд задачи — предыдущего раунда код-ревью нет, раздела «Закрытие раунда r0» и «Унаследовано» не открываю.
## Скоуп
Проекция 2.5D переведена с орфографической камеры (пол сжимался по `cos 20°`
к центру `[500,500]`) на «третий путь»: пол — тождественная плоскость,
высота откладывается строго вверх на `z·sin 20°`. Значки устройств и замков
получили единый подъём на высоту стен вместо поиска места #651 (жёсткие
кластеры, вектор до 48 px). Камера при смене проекции сохраняет viewBox,
пересчитывается только скалярный zoom. Порядок глубины проёмов переведён на
ключ нового направления проецирования. Golden, changelog RU/EN,
`docs/ISOMETRIC.md`, `docs/USER-GUIDE.{md,ru.md}` обновлены.
Первая строка `docs/SCOPE.md`, которую задача обслуживает: J1/J2/J3 (плоский
и объёмный планы — один и тот же «взгляд на дом и действие», исправление
бага восприятия геометрии) в рамках узкого исключения #89 (2.5D-презентация
без второй модели и свободной камеры) — задача не расширяет это исключение,
только чинит его геометрию.
## Как проверялось
Читал в порядке из промпта: `docs/SCOPE.md` → `AGENTS.md` →
`docs/process/REVIEWER.md` → тело issue #713 и все комментарии (включая три
решения владельца 30.09 — вариант Б, третий путь, ответы на четыре вопроса)
→ `docs/USER-GUIDE.ru.md` (после правки) → `docs/ISOMETRIC.md` (канонический
документ подсистемы, целиком, до и после правки по диффу).
Дальше — построчное чтение диффа `git diff origin/dev...HEAD` по каждому
файлу (`src/iso-projection.ts`, `src/iso-openings.ts`, `src/iso-overlays.ts`,
`src/iso-scene-render.ts`, `src/houseplan-card.ts`,
`src/houseplan-editor-runtime.ts`, `src/live-editor.ts`, все тесты, смоуки,
`scripts/mutation-registry.mjs`, `scripts/smoke-links.mjs`,
`demo/golden/harness.mjs`, `demo/golden/matrix.mjs`, changelog, user-guide).
Затем — исполнение: пересобрал `dist` (`npm run build`, `npm run
bundle:sync`) на материале `80e3dba7` и прогнал названные в плане
автотестов и по `smoke-select` смоуки headless-Chromium'ом
(`playwright chromium`, уже установлен в окружении). Отдельно поднял
`git worktree` на `origin/dev` (без изменения текущего `HEAD` рабочей
копии — правило про `git checkout`/`fetch` не нарушено: материал ревью не
трогался), скопировал в него **новый** файл `demo/smoke_iso_flat_parity.mjs`
и прогнал его на старом коде — чтобы лично убедиться, что свидетель умеет
падать, а не поверить заявлению автора на слово.
### Гейты — что прогнал, что нет и почему
| Гейт | Статус | Как |
|---|---|---|
| `npx tsc --noEmit`, `npm test`, `npm run build` + `bundle-policy --verify` | **Не перегонял** | Validate на `80e3dba7` зелёный (run 36725334300, ссылка в задаче ревью) — дешёвые гейты подтверждены на этом SHA (#343). Косвенно перепроверил: `npm run build` внутри `bundle:sync` прошёл чисто на этом же SHA дважды в ходе моих прогонов. |
| `node scripts/mutation-registry.mjs` / `mutation-gate.mjs --check` | **Прогнал** | `browser guards: 200/200`, exit 0, 3 предупреждения — все дочерние #650 (шаблонные имена тестов, не резолвятся статически), ни одно не касается 4 новых/1 перепрофилированного мутанта #713. Реестр согласован с кодом. |
| `demo/smoke_iso_flat_parity.mjs` (АС2–АС5, АС11, план автотестов) | **Прогнал дважды** | На `80e3dba7` — **OK**, все 23 проверки true. На `origin/dev` (тот же файл смока, скопированный в чистый worktree, старый код собран отдельно) — **12 красных** ровно по тем, что заявил автор: `AC2`×2, `AC3`×2 (+large-house), `AC4`, `AC5`×3, `AC11`×2. Свидетель умеет падать — доказательство состоятельно. |
| `demo/smoke_isometric_live_touch.mjs` (AC6, К9 ввод) | **Прогнал** | OK, 13/13. |
| `demo/smoke_isometric_contract.mjs` (AC9, отказоустойчивость) | **Прогнал** | OK, 13/13. |
| `demo/smoke_room_fit.mjs` (не-скоуп «Вписать всё» не меняется) | **Прогнал** | OK, 13/13, включая `isoUsesProjectedBounds`. |
| `demo/smoke_iso_first_frame.mjs` (зарегистрированная связь в `smoke-links.mjs`) | **Прогнал** | OK, 7/7. |
| `demo/smoke_daycycle_raster.mjs` (зарегистрированная связь) | **Прогнал** | OK, 14/14. |
| `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | **Прогнал**, решение по строкам ниже | 34 прямых совпадения, 42 слабые связи, 2 зарегистрированные. |
| 30 смоков из «прямого совпадения» (кроме уже перечисленных выше по AC) | **Прогнал все** | `smoke_pan_any_zoom`, `smoke_smooth_zoom`, `smoke_furniture`, `smoke_infinite_canvas`, `smoke_warm_dialogs`, `smoke_zoom_out`, `smoke_audit_1490`, `smoke_canvas_frame`, `smoke_editor_gestures`, `smoke_houseplan_panel`, `smoke_kiosk_pan_lock`, `smoke_static_zoom_sharpness`, `smoke_warm_owners`, `smoke_zigbee_topology_hover`, `smoke_daycycle_layer_budget`, `smoke_decor`, `smoke_grid_scale_invariance`, `smoke_kiosk`, `smoke_live_pan_coverage`, `smoke_sections_resize`, `smoke_warm_remount`, `smoke_zoom_flash`, `smoke_backdrop_guard`, `smoke_danger_confirmation`, `smoke_help_affordance`, `smoke_post_write_adoption`, `smoke_radar_live`, `smoke_radar_setup`, `smoke_space_scale_defaults`, `smoke_space_settings_form` — **все 30 зелёные** (включая `smoke_infinite_canvas` и `smoke_live_pan_coverage`, которые автор пометил красными в песочнице CI как окружение-зависимые — в моей среде они прошли, что не противоречит заявлению «окружение», просто другая песочница). |
| 42 «слабые связи» smoke-select (`_baseVb`, `_zoom`, `cellCm`, `_booting`, `_view`, `_applyView` — распространённые имена) | **Не прогонял** | Судебное решение: все они совпали по одному общему символу без прямого попадания в изменённую строку; риск-поверхность (камера/zoom/проекция) уже покрыта 34 прямыми смоками, все зелёные. Инструмент сам называет это «решает ревьюер», не обязанностью. |
| `npm run golden:verify` | **Не перегонял** | Метка `ci:golden` уже закрыта автором по процессу §3 п.13: коммит `80e3dba7` несёт `Release:` + `Baseline-Reviewed:` со ссылкой на Validate run 36721827715 (артефакт `c2c806fa`, Linux, Chromium 151.0.7922.34), 19 изменившихся 2.5D-сцен просмотрены попарно (см. текст коммита), 171 сцена сохранена байт-в-байт. Golden-приёмка — не обязанность код-ревью при уже закрытой метке; проверил только формальные условия (трейлеры, ссылка валидна, SHA артефакта совпадает с предыдущим пушем). |
| `python -m pytest tests_backend` | **Не запускал** | Диф не касается `custom_components/**/*.py` — гейт не применим. |
| `npm run invariants -- --config <export>` | **Не запускал** | Задача меняет ГЕОМЕТРИЮ ПРОЕКЦИИ (камера/экран), а не геометрию модели плана (стены/комнаты/проёмы как данные); координаты, хранимые в конфиге, не меняются («Модель данных и миграция: Нет» в ТЗ, подтверждено чтением — ни один диф не трогает `logic.ts`/схему). Инварианты модели не относятся к этому диффу. |
| Performance-профили (`large-house-isometric`, `isometric-stage3-dense`, `isometric-smoke`) | **Не запускал** | Не названы ни в одном AC (AC10 — про бюджет стартового JS-графа, это другой гейт и он уже часть `npm test`). ТЗ ожидает только улучшения (поиск #651 ушёл), автор привёл число (300130 против 300328, предел 300500) — правдоподобно, но полный профиль — предрелизная обязанность, а не гейт ревью. |
| `node scripts/entry-cost.mjs` / бюджет стартового графа (AC10) | **Покрыт `npm test`** | `test/bundle-assets.test.mjs` входит в `npm test`, уже зелёный на этом SHA по Validate. |
## Находки
### Low — устаревший комментарий, не влияет на поведение
`src/houseplan-card.ts:8439` (строка не тронута этим диффом, соседствует с
изменённой подсистемой): комментарий к `furnitureScreenScale` утверждает
*«The 2.5D floor matrix only foreshortens y by cos 20°»*. После #713 это
неверно: К1/АС1 явно требуют и код подтверждает, что для пола `cos 20°`
нигде не осталось (`grep` по всем изменённым файлам подтверждает — единственное
совпадение это и есть данный комментарий). Комментарий не проверяется
исполнением, поведение декора корректно (AC2 покрывает совпадение мебели
пиксель-в-пиксель, зелёный), риска для AC9 нет — сам AC9 про код, а не про
текст комментария. Снимаю с записью, возврата не требует: строка достаточно
заметна (`grep -n cos.20` найдёт её первой), стоимость правки тривиальна и
может уйти вместе с #714 (снос кода #651), либо мелким follow-up.
Других находок — Medium или High — не обнаружено.
## Что проверено и корректно
- **К1/AC1 (проекция).** `isoPlaneMatrix()` — тождество на полу, подъём
`z·sin 20°`, никакого `cos(tilt)` в матрице пола и в `unprojectFloorPoint`
(было — убрано вместе с проверкой `Math.abs(Math.cos(tilt)) > 1e-12` из
`finiteCamera`; вырожденный тест `tiltDeg: 90` заменён на `xyScale: 0` —
корректно, старый сценарий вырождения физически ушёл вместе с `cos(tilt)`).
`test/iso-projection.test.mjs` добавляет прямую и обратную проверку по
четырём точкам плюс отрицательную проверку «старый ключ `y·sin+z·cos`
не совпадает» — не тавтология.
- **К2 (слой пола не меняется).** `<g class="iso-floor-scene">` больше не
несёт `transform` (было `isoFloorMatrixCss()`); `_scenePoint`,
`_floorView` со сжатием через `unprojectFloorPoint` удалены как отдельный
путь для vacuum/radar точек — они используют сырые координаты. AC2
подтверждён и на исполнении (0.00 px расхождения на wide/tall стендах для
rooms/decor/labels/backdrop), и на регрессии от старого кода (17–26 px
расхождение, совпадает по порядку величины с числами из тела issue,
15–23 px).
- **К3 (`show_borders: false`).** `_baseVb('iso', …)` для этой ветки строит
кадр через `projectedFrame({rect: flat, wallHeight: 0})` — геометрически
совпадает с плоским. AC4 подтверждён исполнением (0 px расхождение по
rooms/decor/labels/devices/locks).
- **К4/AC3 (единый сдвиг значков).** `visualOffset = kind === 'room-label' ?
0 : wallHeight` в `buildIsoOverlayRenderScene`; поиск места убран из
живого пути (`resolveIsoOverlayRigidGroups`/`resolveIsoOverlayCollisions`
не вызываются нигде в `src/**`, только определены — их снос корректно
вынесен в отдельный #714, как требует «Не-скоуп»). Тест `#713 AC3`
проверяет и позицию у «дальней» точки, и у «пришпиленной к стене» — обе
дают тот же вертикальный сдвиг и не пересчитываются при зуме/пане (`zoom
и pan are not layout events`). Подтверждено и на демо (11 значков, один
и тот же `Δy`, `Δx=0`), и на `large-house` этаж 1 (80 устройств,
`AC3LargeHouseOneRise: true`, `bad: 0`).
- **К5 (названия комнат).** `visualOffset` для `room-label` — 0; тест
`#713 K5` и `smoke_iso_flat_parity` (`labels` в maxDelta) подтверждают:
название остаётся на полу, `nudged: false`.
- **К6 (перенос камеры).** Разобрал все три пути:
- (а) сохранение настройки — `_syncVolumetricSetting` →
`_convertProjectionView(from, to)`. Ключевой нюанс: `_effectiveProjection()`
— не чистый геттер, а метод с побочным эффектом (строит
`_renderIsoScene`, мемоизирует через `_isoProjectionSnapshot`); вызывающий
код (`install()`-колбэк лоадера, `_syncVolumetricSetting`) явно обнуляет
`_isoProjectionSnapshot` перед повторным вызовом, поэтому к моменту
`_rezoom()` реальная 2.5D-сцена (с учётом высоты стен) уже построена, а
не берётся из промежуточного плоского фолбэка. Подтверждено исполнением:
`AC5SettingOnKeepsCamera` (после полного оседания настройки zoom точно
равен `fit'.w/view.w`, отклонение < 1e-6) — если бы пересчёт цеплялся за
промежуточный кадр без стен, эта проверка бы не сошлась.
- (б) вход в редактор из 2.5D — `houseplan-editor-runtime.ts`:
`targetZoom = fitView(host._baseVb('flat'), v.w/v.h).w / v.w`, тот же
вид, что при входе из Flat с той же картинкой. `AC5EditorEntryMatchesFlat`
зелёный на исполнении.
- (в) тёплый памятник из другой проекции — `_warmAdoptViewport`:
`this._view = vp.view ? {...vp.view} : null;` теперь не зависит от
`sameProjection` (было — зависело), zoom пересчитывается через
`_rezoom()`. `AC11WarmFlatToIsoKeepsCamera`/`AC11WarmIsoToFlatKeepsCamera`
зелёные на исполнении (ремаунт карточки, сравнение viewBox и zoom).
- Холодный старт без памятника — `AC11ColdIsoStartsAtHome` зелёный.
- Клэмп `[1/3, 8]` — `_rezoom()` проверяет `z >= MIN_ZOOM && z <= 8` и при
выходе за диапазон перестраивает `_view` через `_applyView` (который
клэмпит внутри), а не просто отбрасывает значение — граничный случай не
покрыт отдельным тестом, но по чтению корректен и не противоречит К6.
- **К7/AC7 (порядок глубины проёма).** `cameraDepth` в `iso-openings.ts`
переписан на `s·y + z` (без `rotDeg` слагаемых лишних не осталось при
`rotDeg=0`, что и есть единственная используемая камера). Новый тест
`#713 AC7` проверяет и совпадение с новым ключом, и **несовпадение** со
старым (`y·sin+z·cos`) — не тавтология.
- **К8 (кадр без резерва 48 px).** `resolveIsoOverlayFitEnvelope` — весь
бинарный поиск масштаба под нудж убран, теперь просто `unionRect(base,
overlay)` без паддинга. Тест `#713 K8` подтверждает точную геометрию (105
вместо прежнего варьируемого бюджета), golden-harness (`demo/golden/harness.mjs`)
требует `data-hp-iso-nudged="false"` везде — 4 golden-сцены, ранее
требовавшие бюджет, обновлены на контракт «один подъём» отдельным
коммитом `6da6d9c6` с ясным обоснованием.
- **AC9 (ревью кодом, не исполнением).** Прочитан весь дифф `grep -rn
"cos(tilt)|Math.cos(20"` по затронутым файлам — единственное совпадение
это комментарий (Low, см. находки), не код. `resolveIsoOverlayRigidGroups`/
`resolveIsoOverlayCollisions` не вызываются нигде в `src/**` вне своего
файла определения. Кадр не резервирует 48 px (см. К8). Названия без
коррекции (см. К5). Все четыре утверждения AC9 подтверждены чтением.
- **Трейлеры.** `Issue: #713` на всех трёх нетривиальных коммитах,
`User-Visible: yes` только на `0d641ffb` — оба changelog (`docs/CHANGELOG.md`,
`docs/CHANGELOG.ru.md`) отредактированы в этом же коммите, текст на
русском и английском согласован по формулировкам с `USER-GUIDE`.
`c2c806fa` — docs-only (`docs/images/screenshots.json`), трейлеры не
нужны по правилу «докс-коммит без трейлеров» (AGENTS.md §«Commits»).
`80e3dba7` трогает `demo/golden/baselines/**` — несёт `Release:
v1.79.0-beta.1` и `Baseline-Reviewed: <run URL>`, оба требования §10.1
выполнены, ссылка на реальный Validate-прогон.
- **Одно число — один источник (§8).** Числа, видимые пользователю дважды:
подъём значка «высота стен» — единственный источник `ISO_WALL_HEIGHT=84`
через `gridVisualUnits`, используется и в `iso-scene-render.ts`
(построение сцены), и в тексте руководства («на одно и то же
расстояние — высоту стен»), не задублирован вторым магическим числом.
Клэмп zoom `[1/3, 8]` — источник `MIN_ZOOM`/`HouseplanCard.ZOOM_MAX=8`,
переиспользуется в `_rezoom()`, `_applyView()`; в тексте руководства
числа не упоминаются. Расхождений не нашёл.
- **Документация.** `docs/ISOMETRIC.md` перечитан целиком после правки:
«Coordinate systems», «Camera and overlay placement», «Failure boundary»
(`nudged` всегда `false` с #713), «Limits» (маркеры могут перекрывать
стены — решение владельца зафиксировано) — согласовано с кодом и с ТЗ,
без противоречий. `docs/USER-GUIDE.{md,ru.md}` — терминология совпадает
(«один и тот же сдвиг — высота стен», «сохраняют масштаб и положение»),
подсказка к опции (упомянутая в ТЗ как «уже обещает верное») не менялась
и текстом диффа не тронута — корректно, текст не требовал правки.
## Чего не проверял
- Полный `npm run golden:verify` (артефакт) и попиксельный личный просмотр
19 принятых 2.5D-кадров — принял по формальному условию §3 п.13
(`Release:`+`Baseline-Reviewed:` с валидным run URL, авторское
покадровое сравнение зафиксировано в тексте коммита). Не открывал сами
PNG.
- 42 «слабые связи» smoke-select — решение задокументировано в таблице
гейтов, не запускал.
- Полные performance-профили (`large-house-isometric` и т. д.) — не
названы в AC, предрелизная обязанность.
- `python -m pytest tests_backend` — не применимо, Python не тронут.
- `npm run invariants` — не применимо, геометрия модели (данные) не
изменена.
- Пограничный случай клэмпа `_rezoom()` при выходе `fit'.w/view.w` за
`[1/3, 8]` (например, экстремальная смена соотношения сторон окна в
момент переключения проекции) — не покрыт отдельным тестом и не
прогонялся мной вручную; по чтению кода корректен (см. «Что проверено»),
но эмпирически не подтверждён.
- Мутанты реестра #713 не прогонялись по прогону (только `--check`,
синтаксическая связность с тестами) — по правилу трека `ask` это не
обязанность ревью, ловит ночной прогон (#709).
## Вердикт
Зелёный. Все 11 AC либо подтверждены исполнением (2 юнита, 4 смоук-набора,
witness на старом коде лично воспроизведён с тем же числом падений, что
заявил автор), либо разобраны чтением с явной пометкой (AC9). Единственная
находка — Low (устаревший комментарий), снята с записью, цикла не образует.
---
<!-- material-anchors: сгенерировано конвейером (#414) -->
## Материал раунда
- Ветка: `issue/713-oblique-projection`, коммит `80e3dba70024` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
- Дерево материала: `8a10b6de3e2be01995b1e037de92302b3f499c5c`
```
git log --all --format='%H %T' | grep 8a10b6de3e2b
```
- Тело issue: `2305e229564d42ff52bc6d6672c579214fcd49ec80df28e2f111ea726179e7ab`
- Вердикт конвейера: `green` · High 0