From 15c12b752cde52f37842d75bc24304a746c3cd0e Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Wed, 7 Oct 2026 00:26:10 +0000 Subject: [PATCH] docs: review document for #770 Issue: #770 User-Visible: no --- docs/reviews/CODE-REVIEW-770-r1.md | 200 +++++++++++++++++++++++++++++ 1 file changed, 200 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-770-r1.md diff --git a/docs/reviews/CODE-REVIEW-770-r1.md b/docs/reviews/CODE-REVIEW-770-r1.md new file mode 100644 index 00000000..f16ec0a0 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-770-r1.md @@ -0,0 +1,200 @@ +# CODE-REVIEW-770-r1 + +Issue: #770 · Заход: r1 · Трек: show · Блокирующих циклов 0/2 +Материал: `079ab829939687d48f99b81a64595994c7758ef5` (3 коммита поверх `origin/dev`, +`origin/dev..HEAD`: `396625d2`, `63763666`, `079ab829`) + +## Скоуп + +Инфраструктурная задача (B/C), без изменения `src/**`: четыре пункта из тела +issue — + +1. Потолки `longTasks.maxCountP95`/`maxTotalP95Ms` семей `switchCycle` пересчитаны + с учётом тёплого окна после #735 (как #747 для `switchCycleMs`). +2. `scripts/classify-changes.mjs`: правка бюджета профиля (или общего раннера + large-house) теперь сама включает соответствующий perf-smoke профиль в + Validate, а не только диффом `src/**`. +3. `demo/performance/README.md`: актуализированы допуски `resizePreviewMs` и + описана методика пересчёта потолков. +4. Ошибки 2.5D-контракта раннера (`demo/benchmark_large_house.mjs`) теперь + называют измеряемый профиль, а не всегда `large-house-isometric-v1`. + +`User-Visible: no` на всех трёх коммитах — обоснованно: диффа в `src/**` нет, +меняется только CI-харнесс и его документация. + +## Как проверялось + +- Тело issue #770 и три комментария (аналитика 01.10, «Взял» 06.10, «Сделано» + 07.10) — прочитаны через `gh issue view`. +- `git diff origin/dev...HEAD` — прочитан полностью по файлам + (`scripts/classify-changes.mjs`, `demo/benchmark_large_house.mjs`, + `demo/performance/card-contract.mjs`, все `budgets*.json`, + `demo/performance/README.md`, `PROCESS.md`, оба тестовых файла). +- Независимо проверено по живому прогону `gh run view` на точном SHA + материала (workflow_dispatch `37550036250`, push `37549358222`, оба + `success`, оба на `079ab829`): job «Перф-смок» реально выполнил шаги + «Изометрический профиль по диффу» и «Профиль взаимодействия по диффу» (а не + skip) и прогнал их против новых потолков: + - iso: `countP95 17 / 28`, `totalP95Ms 3639 / 6850` — прошло с запасом; + - interaction: `countP95 6 / 18`, `totalP95Ms 2089 / 4350` — прошло с + запасом. + Это прямое доказательство пункта 2 «работает» на материале ревью, а не по + заявлению автора. +- `node scripts/smoke-select.mjs --base origin/dev --head HEAD` — запущен: + «Исполняемого frontend-диффа нет… Browser-smoke этим диффом не выбираются». + Чистое «нечего выбирать», не НЕОПРЕДЕЛЁННОСТЬ. +- Числа README сверены построчно с рядами в `test/performance-budget.test.mjs` + (максимумы по каждому из 6 профилей, формула `⌈1.15×M⌉` для count и + `⌈1.15×M/50⌉×50` для суммы) — совпадают. +- Проверено, что `demo/performance/budgets-isometric-stage3-dense.json` и + `demo/performance/budgets-large-house-isometric-backdrop.json` (тоже + пересчитанные) не используются job `performance_smoke` Validate (их нет ни в + `.github/workflows/validate.yml`, ни в `scripts/classify-changes.mjs`) — они + профили `performance.yml` (Full Performance), не диктуются диффом, так что + отсутствие их путей в новых регэкспах `PERF_PROFILES` — не пробел. + +## Гейты + +| Гейт | Результат | Источник | +|---|---|---| +| `npx tsc --noEmit`, `npm test`, `npm run build` + `bundle-policy --verify` | зелёный | Validate `079ab829`, job «Фронтенд» — success, подтверждено ссылкой в задании ревью, не перегонялось | +| `smoke-select` (диффом) | «нечего выбирать» — src/** не тронут | прогнано в этом раунде | +| Browser-smoke (3 шарда) | зелёные | Validate `079ab829`, не перегонялось | +| `golden:verify` | зелёный, но не обязателен: `ci:golden` не стоит, рендер плана не тронут | Validate `079ab829` | +| `pytest tests_backend` | не прогонялся — Python не тронут | — | +| `npm run invariants` | не прогонялся — геометрия (`src/**`) не тронута | — | +| Перф-смок (iso/interaction профили) | зелёный, реально выполнен (не skip) на новых потолках, с запасом | Validate `079ab829`, job «Перф-смок», лог шагов | +| `ci:full` (владелец/автор запросили) | исполнен — это и есть прогон `37550036250` (workflow_dispatch Validate на материале) | — | + +Не прогонялись намеренно: pytest (нет Python-диффа), invariants (нет +диффа геометрии), Full Performance (`performance.yml`) — задача просила и +получила `ci:full` (полный Validate), не ночной/предрелизный перф-прогон, а +новые ceiling для dense/backdrop профилей всё равно не проверяются Validate ни +до, ни после этого диффа. + +## Находки + +Нет. High: 0, Medium: 0. + +Разобрано отдельно и снято как не-находка: отсутствие +`budgets-isometric-stage3-dense.json` и +`budgets-large-house-isometric-backdrop.json` в новых регэкспах +`PERF_PROFILES` (`scripts/classify-changes.mjs`) — эти бюджеты не судят ни один +шаг `performance_smoke` в Validate (см. выше), так что их включение не нужно +для заявленного AC2; задача явно про диффы, которые «не включают в Validate +профили, которые они судят» (issue, п.2), а не про набор `performance.yml`. + +## Что проверено и корректно + +- **П.1 (потолки Long Task).** `test/performance-budget.test.mjs` + (`LONG_TASK_FAMILIES`) — защитный AC с полной таблицей «доказано · чем + краснеет»: + - доказано: явный пересчёт по ряду (27 отчётов/189 точек на семью, + 14 прогонов Full Performance 01–05.10, один тип раннера) воспроизведён в + самом тесте и сверен построчно с README; + - краснеет: `assert.deepEqual(failures(2*maxCount,1), ['longTask.countP95'])` + и симметрично для суммы — удвоение явно репроduce়тся тестом, не + зависит от истории коммитов; плюс известный медленный вариант v1.78.0 + (`7d4d75bd`) ловится на плотном двойнике (8754 vs 6850). + - Подтверждено исполнением: Validate `079ab829` прогнал реальный perf-smoke + на новых потолках (iso 17/28, 3639/6850; interaction 6/18, 2089/4350) — + числа не близки к границе и не абсурдно далеки, калибровка выглядит + честной. +- **П.2 (classify-changes включает профиль по бюджету/харнессу).** + `test/classify-changes.test.mjs` — защитный AC, таблица «доказано · чем + краснеет»: + - доказано: тест строит ожидаемое сопоставление профиль↔бюджет не из + хардкода, а чтением `validate.yml` (`--budgets=`, `--profile=`) и поля + `profile` в самих JSON — так что он проверяет реально работающую цепочку, + а не копию регэкспа; + - краснеет: по журналу коммита `396625d2` «на прежнем классификаторе оба + теста #770 красные»; я убедился в этом чтением — старый `PERF_PROFILES` не + матчил пути `budgets-*.json`, значит `classify([...budget path])` вернул + бы `'false'`, а тест требует `'true'` — тест обязан падать на коде до + этого изменения. Отдельно подтверждено исполнением: в Validate `079ab829` + шаги «Изометрический профиль по диффу» и «Профиль взаимодействия по + диффу» реально выполнились (а не были пропущены) именно на этом диффе, + где из `src/**` ничего не тронуто — то есть их включил именно новый + предикат по бюджетам/харнессу. +- **П.4 (имя профиля в ошибке 2.5D-контракта).** + `test/performance-contract.test.mjs` — защитный AC: + - доказано: `assertIsometricCandidate` вынесена в `card-contract.mjs`, + вызывается с `profile` на обеих стадиях (`labs-hook`, `renderer`); + - краснеет: `assert.doesNotMatch(runner, /large-house-isometric-v1 + candidate has no/)` — тест ловит буквальный откат к старому хардкоду + профиля в самом раннере, плюс мутация имени стадии + (`'renderr'` → `unknown 2.5D contract stage`). +- **П.3 (README).** Прочитано построчно, сверено с числами из теста и с + `demo/performance/budgets-large-house-interaction.json` (gesture allowances + 150/60/75) — текст больше не описывает отменённые 450/150/120 как текущее + состояние, ссылается на `6f226a5b`/v1.77.0-beta.2. Не защитный AC + (документация), проверено чтением. +- Трейлеры `Issue: #770` и `User-Visible: no` — на всех трёх коммитах, + корректно: диффа, видимого пользователю, нет, changelog не нужен. +- Одно число — один источник: ceiling-значения (18/4350, 28/6850) дублируются + буквально в нескольких JSON (смок + полный + твины), но единственный + логический источник — `LONG_TASK_FAMILIES` в тесте, который держит все копии + в синхроне через явные `assert.equal` по каждому файлу семьи; расхождения + между файлами тест ловит. +- `PERF_PROFILES`/`LARGE_HOUSE_HARNESS` (`scripts/classify-changes.mjs`): + корректно комбинируются через `anyOf`, `demo/benchmark_glow.mjs` и + `demo/performance/compare.mjs` намеренно не включены (их всегда гоняет + glow-смок) — подтверждено тестом «тесты и чужие демо-файлы… не включают». + +## Чего не проверял + +- Python/HA-путь (`pytest tests_backend`) — диффа в Python нет, гейт + неприменим. +- Инварианты модели (`npm run invariants`) — геометрия (`src/**`) не + тронута. +- Полный ночной/предрелизный прогон `performance.yml` (Full Performance) — + не гейт этой задачи; `ci:full`, которую запросил автор, означает полный + Validate (§5.1), не его. Новые ceiling для `isometric-stage3-dense-v1` и + `large-house-isometric-backdrop-v1` тем самым ни разу не исполнялись на + реальном Chromium-прогоне в материале этого ревью (только на историческом + ряду Full Performance 01–05.10, с которого они и взяты) — это ожидаемо: + задача их не могла исполнить иначе, вопрос закрывает плановый ночной + прогон, не этот гейт. +- Golden-эталоны отдельно не перегонял (не требуется: рендер плана не + тронут, `ci:golden` не стоит); использовал готовый зелёный результат из + Validate `079ab829`. + +## Критерии трека `show` (§5) + +- `complexity` — пройден: диффа в `src/**` нет, код изменения механические + (регэкспы, вынос функции, числа в JSON), тяжесть — в обосновании чисел + (README/тест), не в самом коде. +- `surfaces` — пройден: одна поверхность, perf-харнесс Validate + (`classify-changes.mjs` + `demo/performance/*` + budgets). +- `migration` — пройден: нет полей конфига, нет compatibility-слоя. +- `ux-contract` — пройден: нет UX, `User-Visible: no` обоснованно. +- `perf-touch` — пройден: нулевой диф в `src/**`, это CI-измеритель, а не + продуктовая производительность или touch-путь. +- `undocumented` — пройден: ожидаемое поведение — это буквально 4 пункта + тела issue #770, каждый закрыт предъявленным тестом/текстом; открытых + вопросов владельцу код не создаёт. + +Маршрут: `fix` (вердикт зелёный, `route: reclassify` неприменим). + +## Вердикт + +Зелёный. Все четыре пункта issue закрыты, с воспроизводимыми защитными +тестами (явная демонстрация «краснеет»), числа README проверены построчно, +и ключевой риск задачи (новые потолки смока ни разу не исполнялись) закрыт +независимым просмотром лога `ci:full`-прогона на точном SHA материала — оба +смоковых профиля реально выполнились и прошли с разумным запасом. + +--- + + + +## Материал раунда + +- Ветка: `issue/770-perf-budgets-harness`, коммит `079ab8299396` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `3859a3c2b014b5517bfdce649b75f3baa22a6c38` + ``` + git log --all --format='%H %T' | grep 3859a3c2b014 + ``` +- Тело issue: `91895b2844968372ec26698651f6f634f8b5ade6bf1c448177aa37c8764bddd5` +- Вердикт конвейера: `green` · High 0 · маршрут `fix` +