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

CODE-REVIEW-735-r1

Issue: #735 · Этап: code · Заход: r1 · Трек: show · Материал: ff8638b6a9ca3577ff2e28a768e7d88ff2bf164a (origin/dev..HEAD — один коммит)

Скоуп

Перф-харнесс demo/benchmark_large_house.mjs до этой правки включал в тёплый 12-шаговый цикл switchCycle первый (холодный) заход на этаж 3 — каждый образец монтирует новую карточку, успевшую посетить только этажи 1 и 2. Холодная сборка чистого пола (а в 2.5D — ещё геометрии и структурная сборка) занимала примерно половину метрики, документированной как «тёплая навигация». В large-house-interaction-v1 холодным оказывался и этаж 1: серия редактора двигает _cfgEpoch, который входит в ключ кэша чистого пола.

Правка (класс B, харнесс, продукт не меняется):

1. Прогрев всех этажей фикстуры по порядку цикла после закрытия диалога настроек и до окна switchCycle, вне любого замеряемого окна и окна Long Task; возврат на этаж 2 перед стартом цикла.
2. Сторож окна: cacheSnapshot/isoStructuralBuildCount до и после цикла; рост любого кэша или счётчика структурных сборок роняет образец.
3. demo/performance/README.md — абзац о ступеньке абсолютного уровня switchCycleMs, явно: сравнение база/кандидат не задето (обе стороны меряет раннер кандидата), бюджеты и hardMaxMs не менялись.
4. test/performance-workflow.test.mjs — unit-якорь #735.

Это закрытая задача из «Не-скоуп» ТЗ #725, track: show, оценка автора — ценность 4/10, сложность 2/10. User-Visible: no, продуктовый код (src/**) не тронут — диф целиком в demo/** и test/**.

Маршрут §5

Критерий	Проходит
complexity ≤3	да — один точечный механизм (прогрев + сторож) в одном файле харнесса
surfaces — одна поверхность	да — перф-раннер demo/benchmark_large_house.mjs; README и unit-тест обслуживают ту же поверхность
migration	да — нет конфига, нет compatibility-полей
ux-contract	да — продуктовое поведение не меняется, User-Visible: no подтверждён диффом (нет правок src/**, changelog, USER-GUIDE.ru.md)
perf-touch	да — меняется только методика измерения, не продуктовая производительность и не touch-контракт; бюджеты/hardMaxMs/workflow/фикстура не тронуты (проверено git diff --stat по этим путям — пусто)
undocumented	да — контракт «тёплый цикл» уже был заявлен в README/комментарии кода; правка приводит измерение в соответствие уже описанному, а не вводит новое поведение

Все критерии пройдены → route: fix.

Как проверялось

• Полностью прочитан дифф (git diff origin/dev...HEAD, 3 файла, 80 вставок/1 удаление) и окружающий код demo/benchmark_large_house.mjs (прогрев, сторож, cacheSnapshot, isoStructuralBuildCount, duration, frame) — построчно.
• Прочитано тело issue #735 (ТЗ, предположения, риски) и итоговый комментарий автора с отчётом по AC1–AC3.
• Структурная сверка: других вызовов _pickSpace между spaceSwitch (:477–480, этаж 2) и закрытием диалога настроек (:1121) нет (grep -n "_pickSpace") — подтверждает заявление «до прогрева посещены только этажи 1 и 2» и что прогрев/возврат на этаж 2 не меняют состояние перед другими уже существующими окнами.
• npx tsc --noEmit — чисто (не обязателен по §8 для этого диффа, прогнан попутно при локальной сборке).
• node --test test/performance-workflow.test.mjs — 7/7 зелёных, включая новый #735 switchCycle times warmed navigation and fails on a floor build inside its window.
• Проверка «тест умеет падать»: прогнал тот же regex-блок якоря против текста раннера на origin/dev (git show origin/dev:demo/benchmark_large_house.mjs) — падает на первой же проверке (every fixture floor must be visited once before the switchCycle window), как и заявлено в AC3 («На dev якорь красный»).
• git diff --stat по demo/performance/budgets*.json, .github/workflows/performance.yml, demo/fixtures/large-house.mjs — пусто; подтверждает заявление «бюджеты, hardMaxMs, профили, фикстура не меняются».
• Сверены оба трейлера коммита: Issue: #735, User-Visible: no — присутствуют; при no changelog не требуется и не тронут (проверено).
• AC2 — реальный прогон, не пересказ. Проверил оба упомянутых прогона через gh run view:
  • 36821241343 («Полные бенчмарки производительности») — headSha = материал ревью ff8638b6…, conclusion: success, все 9 job зелёные. В логах шага «Enforce relative and absolute performance budget» для large-house/isometric/interaction/plan-snap/isometric-stage3 медианы switchCycleMs (683.3 / 1051 / 754.1 / 700.1 / 983.2) совпадают с числами из комментария автора день-в-день; строки switchCycle built a floor inside the window в логах нет ни на одной стороне — сторож ни разу не сработал на зелёном материале.
  • 36821012618 («Проверка (CI)», Validate) — headSha = ff8638b6…, conclusion: success. Это и есть «дешёвые гейты уже подтверждены», упомянутые в задаче на ревью; tsc/npm test/npm run build с бандл-политикой на этом SHA повторно не гонял.
  • Issue #747 и #744 (объявленные «кандидаты в новые issue») существуют и открыты — не декларация без следа.
• AC1 — попытка прогнать смок сама. Issue называет конкретный смок (npm run benchmark:large-house -- --samples=3 --warmups=1), поэтому попытался прогнать его в своей песочнице (npm run build, затем npm run benchmark:large-house -- --samples=1 --warmups=0). Браузер Chromium в окружении есть, но прогон падает на Failed to fetch dynamically imported module: http://demo.local/assets/... / TimeoutError при ожидании window.__card — это сетевое ограничение песочницы ревью (перехват маршрутов Playwright работает только для самого документа, динамический импорт модуля блокируется), а не дефект кода: то же самое происходит при --no-sandbox. Записано в «Чего не проверял». Поведение AC1 принято по (а) чтению кода сторожа — логика симметрична описанной в ТЗ и сторож технически не может не сработать при росте любого из восьми отслеживаемых ключей; (б) реальному зелёному прогону AC2 на точном SHA, который исполняет тот же код сторожа на 9 профилях и 0 раз его не срабатывает; (в) сообщённому автором witness-прогону (раннер без прогрева падает на первом образце с cleanFloor +20 и т. п.) — это заявление автора, не перепроверено исполнением.

AC · чем доказан · чем краснеет

AC	Чем доказан	Чем краснеет
AC1 (прогрев + сторож)	Код прочитан построчно; реальный зелёный прогон AC2 на материале SHA исполняет тот же путь 9×; witness-прогон автора (не переисполнен мной)	Сторож: switchCycleCachesAfter[key] > …Before[key] по любому из 8 ключей cacheSnapshot, либо isoStructuralBuildsAfter !== …Before при не-null счётчике — рост бросает Error. Независимо перепроверено regex-сравнением с origin/dev: без прогрева заявленный автором витнес-прогон обязан упасть (структурно подтверждено по коду: без прогрева шаг 2 цикла строит этаж 3 внутри окна)
AC2 (CI, вся матрица)	Исполнено реально — прогон 36821241343, headSha и числа сверены мной напрямую через gh run view, не из пересказа	Любой профиль с нарушением бюджета или строкой сторожа в логе красит job; conclusion каждого из 9 job — success, строки сторожа нет
AC3 (гейт + якорь)	node --test локально, 7/7 зелёных; regex-блок якоря самостоятельно прогнан против origin/dev и упал — тест умеет падать	Любое смещение прогрева относительно card._settingsDialog = null;/const switchCycle = …, пропуск одного из полей сторожа в тексте раннера или выпадение из порядка 2 -> 1 красят якорь

Находки

Нет. Ни High, ни Medium, ни Low.

Что проверено и корректно

• Прогрев обходит этажи 1..fixture.counts.floors (не хардкод "3"), переживёт смену FLOOR_COUNT, как заявлено в «Принято предположительно» п.2.
• Прогрев и возврат на этаж 2 не входят ни в duration(), ни в окно Long Task (структурно вне startLongTaskWindow/duration блоков — проверено по расположению кода и подтверждено третьей частью regex-якоря !warmup.includes('duration(') && !warmup.includes('startLongTaskWindow(')).
• isoStructuralBuildCount корректно возвращает null для профилей без 2.5D (Number.isFinite на undefined → false), и сторож явно пропускает проверку счётчика при null — старая база (без 2.5D инструментирования) не ложно красится.
• cacheSnapshot читает отсутствующие у старой базы кэши как 0 (?.size ?? 0 / ? 1 : 0), рост от 0 возможен только при реальном появлении кэша — не создаёт ложных срабатываний на старой базе; это и подтвердил прогон AC2 против v1.78.0 со стороны base (сторож молчал). (в этом коммите сторож проверен только по работе на кандидате, со стороны base это уже проверка AC2, исполненная раннером кандидата по дизайну задачи — не предмет этой правки).
• README-абзац не вводит число, у которого есть второй источник истины: budgets-*.json не менялись (проверено диффом), числа в README — историческая справка с явной ссылкой на issue, а не гейтуемая величина; «одно число — один источник» не нарушено.
• Трейлеры корректны: Issue: #735, User-Visible: no; changelog и USER-GUIDE.ru.md не тронуты — согласовано.
• Диапазон диффа ограничен тремя поверхностями, названными в оценке автора (demo/benchmark_large_house.mjs, demo/performance/README.md, test/performance-workflow.test.mjs) — лишних файлов нет.
• Временный зонд, упомянутый в ТЗ («в ветку не идёт»), в диффе действительно отсутствует.

Чего не проверял

• Браузерный смок AC1 не исполнен в этой сессии: сетевая песочница ревью блокирует динамический импорт бандла демо-сервером (demo.local), что не зависит от диффа (см. «Как проверялось»). Принято по чтению кода и по реальному зелёному прогону AC2 на точном материале SHA, который исполняет тот же путь на 9 профилях.
• npx tsc --noEmit / npm test (полный) / npm run build с сверкой бандла — не перегонялись по новой: Validate на материале SHA (36821012618) зелёный, сверено напрямую через gh run view (headSha совпадает). npx tsc --noEmit всё же прогнан попутно (чисто) и node --test test/performance-workflow.test.mjs — прицельно по изменённому файлу.
• npm run mutation-gate -- --check не прогонял: задача явно освобождена от мутационных гейтов на ветке (§2.7, #709), автор сообщил «3 предупреждения, как на dev» — не перепроверено, не требуется по заголовку ревью («мутанты по диффу на материале: не запрашивались»).
• golden:verify — не применим, нет метки ci:golden, дифф не трогает рендер продукта.
• pytest tests_backend — не применим, custom_components/**/*.py не тронут.
• npm run invariants — не применим, геометрия модели и ссылки на неё не тронуты (дифф целиком в demo/**/test/**).
• Полный npm run benchmark:* матрица (9 профилей, несколько образцов) локально не воспроизводилась — дорогой прогон, уже воспроизведён в CI (AC2) на точном материале; повтор не добавил бы доказательной силы сверх уже исполненного прогона на том же SHA.
• Старая (прошлая) switchCycleMs-история на dev до #735 (прогон 36803711867) — проверено только то, что workflow существует, зелёный и датирован раньше ветки; содержимое его логов (конкретные медианы «до») не сверялось построчно — не является частью AC этой задачи, только контекст README-абзаца.

Вердикт

Зелёный. Диапазон диффа узкий, строго по заявленным трём поверхностям, логика сторожа и прогрева прочитана и подтверждена независимым прогоном unit-якоря против dev (красный) и против ветки (зелёный), AC2 подтверждён прямым обращением к реальному CI-прогону на точном SHA с числами, совпадающими с комментарием автора. Находок нет.

---

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

• Ветка: issue/735-bench-cold-floor3, коммит ff8638b6a9ca — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: c7a3cc4364510ebb6ee6453286da468d7f0b4d9e

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

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