docs: review document for #735

Issue: #735 User-Visible: no
2026-10-02 04:38:55 +00:00 · 2026-10-01 06:41:26 +00:00
parent 5e31816881
commit 6372c21eec
1 changed files with 201 additions and 0 deletions
@@ -0,0 +1,201 @@
+# 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 с числами,
+совпадающими с комментарием автора. Находок нет.
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `issue/735-bench-cold-floor3`, коммит `ff8638b6a9ca` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Дерево материала: `c7a3cc4364510ebb6ee6453286da468d7f0b4d9e`
+  ```
+  git log --all --format='%H %T' | grep c7a3cc436451
+  ```
+- Тело issue: `eb4f5583553212b688d5e458cb0c49dbff43bf09171217dd7278f2844195a094`
+- Вердикт конвейера: `green` · High 0 · маршрут `fix`