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

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, 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 проверена через 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 → в задаче

---

Материал раунда

• Ветка: issue/739-layout-reads-followup, коммит 20733ac436fa — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: 886102777be1249389fcd2a662dbe551200250fc

  git log --all --format='%H %T' | grep 886102777be1
  

• Тело issue: b4865b454fae1e35d41752478fda842157505fded49f39f20bbaff6fa2794b37
• Вердикт конвейера: green · High 0 · маршрут fix