docs: review document for #739

Issue: #739
User-Visible: no

docs/reviews/CODE-REVIEW-739-r1.md

@@ -0,0 +1,225 @@
+# CODE-REVIEW-739-r1
+
+**Issue:** #739 · «perf(iso): keep the theme paper across floor switches in 2.5D»
+**Трек:** ask (перф, §5) · **Заход:** r1 · блокирующих циклов использовано 0/4
+**Материал:** `20733ac436faa574837812698bb17e98ebd09383` (один коммит поверх `origin/dev`,
+merge-base `5a49c8c32df959a5f5450e856b204e0120b5125f`)
+**Вердикт:** зелёный
+
+## Скоуп
+
+Единственная цель — К1 из ТЗ: в 2.5D с подложкой цвет «бумаги» (фон карточки
+темы под изображением плана) должен определяться один раз на идентичность
+темы+режима, а не на каждое переключение этажа. До правки каждое переключение
+роняло кэш из-за `space` в ключе `isoPaperContext`, что давало второй полный
+проход рендера (невидимую, но дорогую заставку `.bootveil` + `getComputedStyle`
++ `requestUpdate()`). Эффект — чисто перформансный; экран не меняется
+(`User-Visible: no`, подтверждено в ТЗ и трейлерах коммита).
+
+Файлы диффа: `src/iso-first-frame.ts` (единственный продуктовый файл),
+`test/iso-stage6.test.mjs`, `demo/smoke_iso_floor_switch.mjs` (новый),
+`scripts/mutation-registry.mjs`, `scripts/smoke-links.mjs`, `docs/ISOMETRIC.md`.
+`src/houseplan-card.ts` не тронут — соответствует заявлению автора и ТЗ
+(«Затронутые файлы»).
+
+Закрывает: J1 (`docs/SCOPE.md`) — «живой обзор дома», частое действие
+переключения этажей в киоске/на телефоне; ускоряет путь, которым пользуется
+каждый дом с подложкой в 2.5D.
+
+## Как проверялось
+
+Дешёвые гейты подтверждены Validate на этом SHA (прогон
+[36845529191](https://github.com/Matysh/houseplan-card/actions/runs/36845529191),
+success): `npx tsc --noEmit`, `npm test`, `npm run build` + сверка бандла — не
+перегонялись повторно.
+
+Сверх Validate прогнано лично в этом раунде:
+
+| Гейт | Команда | Результат |
+|---|---|---|
+| `smoke-select` по диффу | `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | 2 «зарегистрированные связи»: `demo/smoke_iso_first_frame.mjs`, `demo/smoke_iso_floor_switch.mjs` (символы `commitPaper`, `themePaper`) — оба названы в AC2/AC3, оба прогнаны ниже |
+| Мутант AC5, гард | `npx tsc -p tsconfig.test.json && node scripts/fix-test-build.mjs && node --test --test-name-pattern="#654\|#739" test/iso-stage6.test.mjs` | 4/4 passed |
+| `scripts/mutation-gate.mjs --check` | — | `ok iso-paper-resolved-per-floor`, предупреждения те же, что на `dev` (4 общих + browser guards 201/200, те же, что были) |
+| `demo/smoke_iso_floor_switch.mjs` (AC2) | `node demo/smoke_iso_floor_switch.mjs` | **OK** на материале; **красный** (`everySwitchRendersOnce`, `noSwitchProbesThePaperAgain`, `noSwitchInsertsTheVeil` = false, 2 обновления/1 проба/1 веил на переключение) после отката `src/iso-first-frame.ts` к версии `origin/dev` — тест умеет падать, проверено лично (не со слов автора) |
+| `demo/smoke_iso_first_frame.mjs` (AC3) | `node demo/smoke_iso_first_frame.mjs` | OK, все 7 проверок true |
+| `demo/smoke_isometric_contract.mjs` (AC3) | `node demo/smoke_isometric_contract.mjs` | OK |
+| `demo/smoke_iso_flat_parity.mjs` (AC3) | `node demo/smoke_iso_flat_parity.mjs` | OK |
+| `demo/smoke_iso_theme_walls.mjs` (AC3) | `node demo/smoke_iso_theme_walls.mjs` | OK |
+| `node scripts/no-new-private-writes.mjs --base origin/dev --head HEAD` | — | «Новых записей в приватное состояние карточки нет» (перехват `card.update`/`card._cssColor` в смоке не считается нарушением; у `_cssColor` стоит `// private-ok: #739`) |
+| `npm run bundle:budget` | — | initial View headroom 541 Б (ceiling не превышен); предупреждение про низкий запас — долг #367/#474, не новый, не из этого диффа |
+
+Перед смоками потребовалось `npm run build && node scripts/bundle-sync.mjs`,
+чтобы `demo/srv/assets` увидел свежий бандл (сам по себе `dist/` на диске при
+старте раунда не был синхронизирован с `demo/srv`). После проверок рабочая
+копия возвращена `npm run bundle:clean` — `git status` чист.
+
+Полная матрица смоков, `golden:verify`, `pytest tests_backend`, performance —
+не гонялись (обоснование ниже, «Чего не проверял»).
+
+## AC — разбор
+
+**AC1 (состояние, `test/iso-stage6.test.mjs`).** Тест добавлен и покрывает все
+шаги (а)–(е) плюс отдельно прогон «уход в редактор и обратно». Прочитан код
+`src/iso-first-frame.ts` построчно и прослежен исполнением:
+
+- `isoPaperContext` больше не кладёт `space` ни в `theme` (идентичность темы),
+  ни в `key` (теперь `key = [imagePlan, theme]`). Из-за этого `key` у двух
+  разных этажей с подложкой и одинаковой темой **совпадает** — это и есть
+  механизм К1: в `prepare()` условие `context.key === this.paperContext`
+  становится `true` уже на первом же сравнении, без обращения к
+  `themePaper`-кэшу вообще.
+- Переход «подложка → нарисованный план → снова подложка» — другой случай:
+  `key` меняется ({imagePlan:false}≠{imagePlan:true}), `paperReady` сбрасывается,
+  но `prepare()` находит `this.themePaper?.theme === context.theme` и мгновенно
+  коммитит закэшированный rgb без похода в DOM — это шаг (г) теста.
+  `commitPaper` при этом помечает `floorMemo = null` (т.к. `!paperReady` на
+  момент вызова истинно), так что `lightFloors()` пересчитывается на новом
+  (не белом) paper — тест проверяет это явно (`[...]` → `[]` вместо
+  `['unfilled']`).
+- `memoIsoLightFloorRooms` (не тронут диффом) сам ключует на `fills`+`paper`
+  (`src/iso-materials.ts:118`), поэтому коллапс `key` между разными этажами
+  с одинаковой темой не ломает пересчёт светлых комнат при смене `fills` —
+  проверено чтением, отдельного теста на эту комбинацию в АС нет, но риска
+  регрессии не вижу: `floorMemo` переиспользуется только если оба входа
+  (цвет и fills) совпали.
+- Смена темы или `_mode` входит в `context.theme`, поэтому шаги (д)/(е)
+  сбрасывают `themePaper` (строка `if (this.themePaper.theme !== context.theme)
+  this.themePaper = null`) и идут путём #654 — подтверждено тестом и чтением.
+
+Проверено исполнением (`node --test`, фильтр `#654|#739`): 4/4 зелёные.
+Проверено также, что тест красный на коде `origin/dev` конкретно на той
+проверке, которую заявляет коммит-сообщение («a floor switch shows no veil»,
+`pending()` ожидаемо `true` на dev вместо `false`) — убедился чтением логики
+(dev-версия не содержит `themePaper`, значит `pending()` после `prepare()` на
+шаге (б) обязана остаться `true`).
+
+**AC2 (один рендер на переключение, `demo/smoke_iso_floor_switch.mjs`).**
+Смок прогнан лично: на материале — `OK`, 1 `update`, 0 `_cssColor`, 0 вставок
+`.bootveil` на каждое из 6 переключений, `readiness` всегда `ready`, светлые
+комнаты совпадают с первым показом этажа. После отката файла до `origin/dev` —
+смок **красный** ровно на тех трёх счётчиках, которые и должен был поймать
+(`everySwitchRendersOnce`, `noSwitchProbesThePaperAgain`, `noSwitchInsertsTheVeil`
+= false, 2/1/1 вместо 1/0/0) — дисциплина «тест умеет падать» выполнена не со
+слов автора, а исполнением.
+
+**AC3 (холодный путь и смежные контракты не изменились).** Четыре названных
+смока (`smoke_iso_first_frame`, `smoke_isometric_contract`,
+`smoke_iso_flat_parity`, `smoke_iso_theme_walls`) прогнаны — все зелёные.
+`smoke-select` по диффу вернул только эти два файла как связанные с диффом
+(«зарегистрированная связь», не «прямое совпадение» и не «неопределённость») —
+оба в списке прогнанных. Красные песочницы (`smoke_summary_*`,
+`smoke_infinite_canvas`, `smoke_live_pan_coverage`) из AC3 не проверялись
+повторно — судит Validate на этом SHA, как и прописано в самом AC.
+
+**AC4 (без регрессий, Full Performance).** Ссылка на прогон
+[36839009721](https://github.com/Matysh/houseplan-card/actions/runs/36839009721)
+проверена через `gh run view`: `conclusion: success`, `headSha:
+31464bb69125ec8eaa9df8c67b97f7e08ac9074f`. Эта SHA — более ранняя точка ветки
+(до ребейза на текущий `dev`, когда база была `7ff2b5ae`); сам объект-коммит в
+локальном репозитории недоступен (история переписана ребейзом), поэтому
+побайтово дифф между `31464bb6` и `20733ac4` не сверить без `git fetch`,
+который запрещён инструкцией материала. Компенсирующая проверка: `git diff
+--stat 7ff2b5ae..origin/dev` показывает, что все 4 коммита, которые легли
+между старой базой прогона и текущей, — `AGENTS.md`, `PROCESS.md`,
+`docs/process/*`, `scripts/process-gate.mjs`, `scripts/task-packet.mjs`,
+`test/moon.test.mjs`, `test/process-*`, `test/task-packet.test.mjs` — не
+пересекаются ни с одним файлом диффа #739 (`src/iso-first-frame.ts`,
+`test/iso-stage6.test.mjs`, `demo/smoke_iso_floor_switch.mjs`,
+`scripts/mutation-registry.mjs`, `scripts/smoke-links.mjs`,
+`docs/ISOMETRIC.md`). Значит ребейз не мог изменить проверяемый диф, и
+прогон на `31464bb6` доказывает то же самое, что доказал бы прогон на
+`20733ac4`. Числа в прогоне (9/9 профилей зелёные, `switchCycleMs` в пределах
+±1% на плоских профилях, в isometric `modelReady` −9%, `switchCycleMs` −4% —
+улучшение, не регрессия) совпадают с заявленным. Сам Full Performance заново
+не гонялся — дорогой гейт, ссылка уже даёт нужное доказательство.
+
+**AC5 (гейт задачи).** `mutation-gate.mjs --check`: `ok
+iso-paper-resolved-per-floor`, патч мутанта (добавляет `space` обратно в
+идентичность темы) и гард (`#654|#739` в `test/iso-stage6.test.mjs`)
+осмысленны — я прочитал: мутант ломает ровно ту строку, из-за которой правка
+вообще нужна. `gate:small` не перегонялся отдельно — он часть Validate,
+подтверждённого на этом SHA. `test/core-file-budget.test.mjs` не актуален —
+`houseplan-card.ts` диффом не тронут.
+
+## Трейлеры и changelog
+
+Коммит несёт `Issue: #739` и `User-Visible: no` — соответствует ТЗ («экран не
+меняется»). Правки обоих CHANGELOG не требуются и отсутствуют — корректно.
+Единственное число, которое «видно дважды» в материале — бюджетный потолок
+бандла View (не меняется диффом, только фактическое значение растёт в
+пределах полосы 2000 Б, подтверждено `bundle:budget`, источник один —
+`scripts/bundle-budget.mjs`). Других дублирующихся чисел в диффе нет (нет
+новых UI-текстов, нет новых констант, видимых пользователю).
+
+## Отклонения ТЗ → реализация
+
+Три отклонения автор назвал сам в хендоффе, все проверены чтением и
+совпадают с кодом:
+1. `space` остался параметром `isoPaperContext`, но не входит в `theme`/`key` —
+   подтверждено (см. AC1 выше); тесты #654 действительно не менялись
+   (`git diff` в тестовом файле — только добавление нового теста).
+2. Сброс кэша при смене `_mode` — действительно строже «принято
+   предположительно» ТЗ (там говорилось только про тему); обоснование
+   (редактор между подложками другого типа плана не должен брать старый
+   цвет) логично и не противоречит «Режимам» ТЗ («уход в редактор... цвет
+   ищется заново»).
+3. `commitPaper`, `themePaper` добавлены и в запись `#654` `smoke-links` —
+   подтверждено в диффе `scripts/smoke-links.mjs`.
+
+## Что проверено и корректно
+
+- Устранение второго прохода рендера реализовано корректно и доказано и
+  юнитом, и смоком, с фактическим «тест умеет падать» (не теория).
+- Коллапс ключа между разными этажами с одной темой — найден чтением,
+  совместим с `memoIsoLightFloorRooms`, не ломает пересчёт светлых комнат.
+- Холодный путь (#654), смена темы/режима, отказ чанка — не затронуты
+  (код `pending`/`readiness`/`runtimeFailure` не менялся).
+- Гейт приватных записей и бюджет бандла — зелёные, без новых предупреждений
+  сверх тех, что уже были на `dev`.
+- Документация (`docs/ISOMETRIC.md`) обновлена в том же коммите, формулировка
+  соответствует новому поведению.
+
+## Чего не проверял
+
+- **Golden.** Диф меняет путь отрисовки 2.5D (`src/iso-first-frame.ts`),
+  видимый риск по правилу §5.1 реален в общем случае, но здесь не
+  реализуется: кадр не меняется — то же решение «нет» подтверждено ТЗ
+  («На экране ничего не меняется: та же сцена... заставки при переключении
+  нет — ни сейчас, ни после правки») и структурно: правка не трогает ни
+  геометрию, ни материалы/заливки (`iso-materials.ts`, `iso-scene-render.ts`
+  не в диффе), а только то, сколько раз и когда резолвится *тот же самый*
+  цвет и нужен ли промежуточный `.bootveil`, который и до, и после невидим
+  глазу (кадр с заставкой и без неё идёт в одной задаче до `requestAnimationFrame`).
+  Метки `ci:golden` на задаче нет, `golden:verify` не гонял.
+- **Full Performance заново.** Не гонял — использовал существующий зелёный
+  прогон (см. AC4) с обоснованием, что ребейз его не инвалидировал.
+- **`pytest tests_backend`.** Диф не трогает `custom_components/**/*.py` —
+  не применимо.
+- **Инварианты модели.** Диф не трогает геометрию комнат/устройств — не
+  применимо.
+- **Полная матрица смоков.** Не гонял — только смоки, названные в AC2/AC3 и
+  подтверждённые `smoke-select`; полный прогон — предрелизная обязанность.
+- **Мутации вне реестра/регистрация нового мутанта в CI-прогоне.** Мутанты в
+  разработке не гоняются (§2.7, #709); якорь патча проверен `--check`.
+
+## Находки
+
+Нет. High: 0, Medium: 0.
+
+---
+
+Вердикт: зелёный · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 0 → в задаче
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `issue/739-layout-reads-followup`, коммит `20733ac436fa` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Дерево материала: `886102777be1249389fcd2a662dbe551200250fc`
+  ```
+  git log --all --format='%H %T' | grep 886102777be1
+  ```
+- Тело issue: `b4865b454fae1e35d41752478fda842157505fded49f39f20bbaff6fa2794b37`
+- Вердикт конвейера: `green` · High 0 · маршрут `fix`