diff --git a/docs/reviews/CODE-REVIEW-747-r1.md b/docs/reviews/CODE-REVIEW-747-r1.md new file mode 100644 index 00000000..c7802bf6 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-747-r1.md @@ -0,0 +1,146 @@ +# CODE-REVIEW-747-r1 + +Материал: `51e2f2b510c33bd98a2d278bdbbb983ee292a5db` (issue/747-switch-cycle-hardmax поверх origin/dev `7ce3f662`, один коммит). +Трек: show · заход r1 · блокирующих циклов использовано 0 из 2. + +## Скоуп + +Задача — техдолг перф-гейта (ценность пользователю 1/10, разработке 5/10): после +#735 окно `switchCycle` прогрето, и прежний `hardMaxMs` (7000/8000 мс) в +7,6–10,2 раза выше тёплого уровня, то есть смок между полными прогонами +фактически не ограничивает `switchCycleMs`. ТЗ требует пересчитать `hardMaxMs` +по образцу #675/#692 (+15…20 % над максимумом ряда медиан, кратное 50 мс), +обновить семь budget-файлов, README («CI contracts») и закрепить правило +тестом. Это служит J1/J7 косвенно — удерживает перф-гейт чувствительным, а не +прямой пользовательский сценарий; соответствует заявленной в issue +классификации «техдолг», в SCOPE.md отдельно не описано и не должно быть. + +Изменённые файлы (диапазон `origin/dev..HEAD`): +- 7 × `demo/performance/budgets*.json` — только значение `hardMaxMs` у + `switchCycleMs` (плоская семья → 950, 2.5D → 1550), остальные поля не + тронуты; +- `demo/performance/README.md` — абзац в «CI contracts» + правка фразы в + абзаце #735; +- `test/performance-budget.test.mjs` — новый блок тестов, закрепляющий число, + ряд и правило пересчёта. + +Класс B (гейты/тест/демо-конфиг), `User-Visible: no` — верно, видимого +пользователю поведения нет. Трейлеры коммита: `Issue: #747`, +`User-Visible: no` — оба на месте, changelog не требуется и не тронут. + +## Как проверялось + +1. **Диапазон и материал.** `git log --oneline origin/dev..HEAD` — один + коммит `51e2f2b5`; `git rev-parse HEAD` совпадает с материалом ревью. + `gh run view 36859280322` (Full Performance) — `headSha` этого прогона + буквально равен `51e2f2b5...`, прогон успешен, что подтверждает заявление + AC2 «на ветке задачи», а не на другом SHA. +2. **Дифф по budget-файлам.** Построчно сверил все семь `budgets*.json` — + меняется только `hardMaxMs` у `switchCycleMs`; `maxRegressionRatio` и + `noiseAllowanceMs` полных профилей не тронуты (0,35/0,2, 250 — как в ТЗ). +3. **Тест.** Прочитал новый блок (`test/performance-budget.test.mjs:310–371`), + прогнал файл: `node --test test/performance-budget.test.mjs` — 18/18 + зелёных, включая оба новых теста (`flat`, `isometric`) и ранее + существующие #473 AC4 (`повторяет hardMaxMs … #473 AC4`) и #160 + (`boundary collision search … #585`, `isometric long-task count … #507`) — + прошли **без правки**, как и заявлено. +4. **Тест умеет падать.** Вручную подменил `950` на `7000` в одном файле + плоской семьи (`demo/performance/budgets.json`) — тест `#747 switchCycleMs + (flat)` покраснел (`not ok 14`), второй тест семьи (`isometric`) не + задет — значит, проверка специфична к семье, а не к файлу наугад. + Откатил правку (`git status --porcelain` — чисто). +5. **Арифметика правила.** Для плоской семьи: ряд из 8 медиан (4 прогона × + 2 стороны), максимум 812,7 → `⌈812,7×1,15/50⌉×50 = 950`; для 2.5D: максимум + 1333,9 → `⌈1333,9×1,15/50⌉×50 = 1550`. Оба совпадают с задекларированными + числами в JSON, тесте, README и комментарии issue — «одно число, один + источник» выполнено: пересчитал формулу вручную, не поверил на слово тесту, + который эту же формулу проверяет (тест и независимый пересчёт совпали). +6. **Ряд — не выдумка.** Для прогона AC2 (`36859280322`) вытащил логи шагов + «Enforce relative and absolute performance budget» по всем пяти профилям + (`large-house`, `plan-snap`, `interaction`, `isometric`, + `isometric-stage3`) и сверил пары база/кандидат с таблицей в комментарии + issue — совпадают до десятых: large-house 597/589, plan-snap 567/567, + interaction 764/783, isometric 883/851, isometric-stage3 1202/1233. Все + строки — `✅`, новых максимумов нет. +7. **Смоки.** `node scripts/smoke-select.mjs --base origin/dev --head HEAD` → + «Исполняемого frontend-диффа нет… Browser-smoke этим диффом не + выбираются» — `src/**` не тронут, выбирать нечего, это не пропуск, а + нулевой результат инструмента. +8. **Метки issue** — `tests`, `infra`, `S7-code-review`, `track:show`; меток + `ci:golden`/`ci:full` нет. + +## AC + +| AC | Проверка | Результат | +|---|---|---| +| AC1 — числа и обоснование закреплены | Тест зелёный, формула пересчитана вручную (см. «как проверялось» п.5), контрольный мутант (ручная подмена 950→7000) красит тест | Выполнен | +| AC2 — новые потолки не краснеют на шуме | `gh run view 36859280322` — 9/9 профилей `success`, значения сверены по логам с таблицей комментария (п.6); на самой ветке смок-профили не включаются (`src/**` не тронут) — задокументировано как открытый пункт, с планом проверки на первом кандидате беты | Выполнен в заявленных пределах; открытый пункт не скрыт | +| AC3 — документ | README правлен по образцу #675/#692 (ряд, уровень, шум раннера, запас, что краснеет/ловится); числа JSON (950/1550), теста (`spec.ceiling`) и README совпадают | Выполнен | + +Защитный характер AC1 (гард против «потолок почти не ограничивает»): таблица +«AC · чем доказан · чем краснеет» фактически есть в самом тесте — `red(2 × +level)` обязан покраснеть, и ручная проверка (п.4) подтвердила, что рассинхрон +числа в файле тоже красит. Третий столбец не пуст. + +## Что проверено и корректно + +- Все семь budget-файлов согласованы внутри семей (`budgets.json`, + `budgets-large-house-plan-snap.json`, `budgets-large-house-interaction.json`, + `budgets-interaction-smoke.json` → 950; `budgets-large-house-isometric.json`, + `budgets-isometric-stage3-dense.json`, `budgets-isometric-smoke.json` → + 1550); «смок = полный» (#473 AC4) не нарушено. +- Относительный гейт полных профилей не тронут — заявление ТЗ «относительный + гейт не меняется» подтверждено диффом построчно. +- Числа в JSON, тесте, README и комментарии issue — один источник, независимо + пересчитан и совпадает. +- AC2 подтверждён не на слово: логи реального прогона на точном SHA материала + сверены построчно. +- Риски, названные автором (более тесный абсолютный потолок относительно + относительного лимита; порядок слияния с #743; отсутствие смоковых точек в + ряду), — не скрытые находки, а честно описанные ограничения с планом + проверки на кандидате беты; это соответствует «Риски и откат» самого ТЗ. +- Трейлеры коммита корректны, bundle/golden не тронуты, Release-трейлер не + требуется. + +## Чего не проверял + +- `npx tsc --noEmit`, `npm test` (полный набор), `npm run build` + + сверка копий бандла — не перегонял: Validate на этом же SHA (`51e2f2b5`) + зелёный (ссылка в промпте ревью), гейты уже подтверждены этим прогоном. +- Browser-смоки — не прогонял: `smoke-select.mjs` показал «выбирать нечего» + (`src/**` не тронут). +- `npm run golden:verify` — не прогонял: метки `ci:golden` нет, изменение не + трогает путь отрисовки. +- `python -m pytest tests_backend` — не прогонял: Python не тронут. +- `npm run invariants` — не прогонял: геометрия и ссылки на неё не тронуты. +- Полный 7-образцовый ряд дальнейших прогонов `dev` после слияния — вне + материала этого захода; AC2 сама отмечает это как открытый пункт на первый + кандидат беты. +- Мутанты по диффу на материале не запрашивались (трек show, #709) — + не находка; ручная проверка в п.4 — не замена реестра, а точечная + санити-проверка самого ревьюера. + +## Находки + +Нет. High: 0, Medium: 0, Low: 0. + +## Вердикт + +Зелёный. AC доказаны автотестом (с подтверждённой способностью падать) и +реальным CI-прогоном на материале ревью; числа везде из одного источника и +совпадают при независимом пересчёте; риски описаны автором честно, без +попытки выдать их за закрытые. + +--- + + + +## Материал раунда + +- Ветка: `issue/747-switch-cycle-hardmax`, коммит `51e2f2b510c3` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `df40c326f0c35a40335720491fe0d73eea9e9599` + ``` + git log --all --format='%H %T' | grep df40c326f0c3 + ``` +- Тело issue: `096ac7409cdc7a999fdb3547c362a416f553b8cd98c1e24559343986b805793f` +- Вердикт конвейера: `green` · High 0 · маршрут `fix`