From 6372c21eec8b1b72caa4e5b547b06bcf0cc3f3bd Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Thu, 1 Oct 2026 06:41:20 +0000 Subject: [PATCH] docs: review document for #735 Issue: #735 User-Visible: no --- docs/reviews/CODE-REVIEW-735-r1.md | 201 +++++++++++++++++++++++++++++ 1 file changed, 201 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-735-r1.md diff --git a/docs/reviews/CODE-REVIEW-735-r1.md b/docs/reviews/CODE-REVIEW-735-r1.md new file mode 100644 index 00000000..fbcebfd6 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-735-r1.md @@ -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 с числами, +совпадающими с комментарием автора. Находок нет. + +--- + + + +## Материал раунда + +- Ветка: `issue/735-bench-cold-floor3`, коммит `ff8638b6a9ca` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `c7a3cc4364510ebb6ee6453286da468d7f0b4d9e` + ``` + git log --all --format='%H %T' | grep c7a3cc436451 + ``` +- Тело issue: `eb4f5583553212b688d5e458cb0c49dbff43bf09171217dd7278f2844195a094` +- Вердикт конвейера: `green` · High 0 · маршрут `fix`