diff --git a/docs/reviews/CODE-REVIEW-483-r1.md b/docs/reviews/CODE-REVIEW-483-r1.md new file mode 100644 index 00000000..664c9c95 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-483-r1.md @@ -0,0 +1,147 @@ +# CODE-REVIEW-483-r1 + +Issue: #483 · Заход r1 · Материал: `git diff origin/dev...HEAD` на SHA `e67244c2a1f6b3e6e609955a26056c9434247cbe` +(`origin/dev` = `862cf151`, ветка = `dev` + один коммит, ребейз не требовался). + +## Скоуп + +Задача — лёгкий трек (`small`, ТЗ в теле issue, автор — владелец). Причина: +абсолютный потолок `interactionSeriesMs.hardMaxMs = 3000` в перф-смоке +(`budgets-interaction-smoke.json`, `budgets-large-house-interaction.json`) +четыре раза подряд ложно завалил Validate релиз-кандидата `v1.73.0-beta.4` +(3017–3093 мс против потолка 3000), при том что чистый `dev` уже давал +3122,6 мс на том же лимите (ревью #476 r3). Решение: поднять только этот +абсолютный потолок до 3300 мс одновременно в обоих профилях, не трогая +остальные лимиты, задокументировать выбор числа, закрепить контракт тестом. + +Класс изменения — **B** (гейты/тесты: `test/**`, `demo/performance/**`), +переиспользует issue #483. Продуктовый код (`src/**`) не тронут. + +Диф (4 файла, 20 добавлений/2 удаления): + +``` +demo/performance/README.md | 7 +++++++ +demo/performance/budgets-interaction-smoke.json | 2 +- +demo/performance/budgets-large-house-interaction.json | 2 +- +test/performance-budget.test.mjs | 11 +++++++++++ +``` + +Один коммит `e67244c2`, трейлеры `Issue: #483` / `User-Visible: no` — +корректно: правка не меняет продуктовое поведение, changelog не требуется и +не тронут. + +## Как проверялось + +Зелёного Validate на `e67244c2` на момент ревью не было (проверено: +`gh api repos/Matysh/houseplan-card/commits/e67244c2.../check-runs` — джоб +«Проверка (CI)» ещё `in_progress`; тяжёлые джобы «Смоки», «Golden», «Перф-смок» +у этого пуша `skipped` — это ожидаемо и не связано с задачей: у коммита нет +трейлера `Release:`, событие не `pull_request`/`schedule`/`workflow_dispatch +full=true`, поэтому `heavyGatesRequested()` в `scripts/classify-changes.mjs` +держит тяжёлый набор выключенным на обычном пуше в task-ветку — это +поведение существует независимо от #483 и не regressed этой правкой). +Дешёвые гейты прогнал сам: + +| Гейт | Команда | Результат | +|---|---|---| +| Typecheck | `npx tsc --noEmit` | чисто, без вывода | +| Юнит-тесты | `npm test` | 2229 тестов, 2228 pass, 0 fail, 1 skip | +| Сборка + сверка бандла | `npm run build && cmp dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js` | собралось, `cmp` без расхождений; третья копия (`demo/srv/assets/**`) в репозитории не хранится по #255 — ожидаемо | +| Выбор смоков | `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | «Исполняемого frontend-диффа нет (src/**/*.ts не тронут). Browser-smoke этим диффом не выбираются — выбирать нечего» | +| Новый `any` | `node scripts/no-new-any.mjs --base origin/dev --head HEAD` | 0 добавленных строк в `src/**/*.ts`, чисто | +| Мутация нового теста | вручную: `budgets-interaction-smoke.json` → `hardMaxMs: 3000` (откат к старому значению), `node --test test/performance-budget.test.mjs` | новый тест из этого диффа падает: `Expected values to be strictly equal: 3000 !== 3300`; попутно падает и существующий тест-контракт «smoke повторяет hardMaxMs полного профиля» (потолки разошлись). Файл восстановлен, `git status` чист | + +`check-docs.mjs` не запускал — diff не трогает `src/**`, гейт по правилу §8 не +относится к задаче (не «правка фронтенда»). `golden:verify`, `pytest +tests_backend`, `model-invariants.mjs` не запускал — визуал, бэкенд и +геометрия/ссылки диффом не задеты. Полный perf-профиль (`performance.yml`, +`benchmark_large_house.mjs` вживую) не гонял: это предрелизный/beta-гейт +(§8, §2.8), он не триггерится на этом SHA по дизайну (нет `Release:`), а +исход уже задокументирован шестью независимыми историческими прогонами, +которые я сверил построчно с текстом README/issue (см. ниже) — все они ниже +нового потолка 3300. + +Дополнительно сверил две цифры, которые автор вписал в README и issue как +факты, а не как догадку: + +- `3122.6` мс (наблюдение на чистом `dev`) — найдено в `docs/reviews/CODE-REVIEW-476-r3.md:132`, совпадает дословно. +- `3501.5` мс (провальный дореоптимизационный результат #451) — найдено в `docs/reviews/CODE-REVIEW-451-r1.md:68`, совпадает. +- Четыре свежих провала беты (3017,1 / 3045,6 / 3011,3 / 3093,2 мс) — из ссылки issue на упавший Validate-run; отдельно не переоткрывал, число согласуется с наблюдаемым диапазоном. + +Обе цифры реальны и уже стояли в дереве до этой задачи — не выдуманы под +обоснование. + +## Разбор по AC + +1. **«Тесты бюджета подтверждают `hardMaxMs = 3300` в обоих профилях и + равенство контрактов».** Доказано: новый тест + `interaction aggregate keeps hosted-runner headroom…` (`test/performance-budget.test.mjs:248`) + проверяет `3300` в обоих JSON явно; существующий параметризованный тест + `${smokeName} повторяет hardMaxMs полного профиля` (тот же файл, выше) + проверяет их взаимное равенство по всем метрикам заново, без изменений — + он и раньше держал этот инвариант и продолжает его держать на новом + значении. Свидетель, что тест умеет падать, приведён в таблице гейтов + выше. **Выполнено.** +2. **«Все индивидуальные лимиты остаются побайтно неизменными».** Проверено + чтением диффа: в каждом JSON-файле изменены ровно две строки — + `interactionSeriesMs.hardMaxMs`, `3000` → `3300` — остальные ключи не + затронуты (полный `git diff` приведён в скоупе). Дополнительно новый тест + фиксирует `hoverSeriesMs`/`panSeriesMs`/`cameraSeriesMs`/`editorSeriesMs` на + старых значениях как регрессионный маячок. **Выполнено.** +3. **«Exact-SHA Validate кандидата проходит без bypass/continue-on-error».** + Не может быть доказано на этом SHA: это условие закрытия issue (§2.8, + «Гейт беты»), а не код-ревью, и тяжёлый Validate-набор на обычном пуше в + task-ветку не запускается по дизайну конвейера (см. выше) — он включится + только на коммите с трейлером `Release:` при подготовке следующей + кандидатуры беты. Это не дефект этой задачи и не то, что реально можно + проверить раньше. Не в скоупе код-ревью, оставляю как открытый пункт + релизного гейта. +4. **«После зелёного CI продолжается публикация v1.73.0-beta.4».** Релизный + процесс, вне скоупа код-ревью. + +## Находки + +Нет. High: 0, Medium: 0, Low: 0. + +## Что проверено и корректно + +- Изменение строго ограничено абсолютным catastrophic-потолком одной + агрегатной метрики; относительные лимиты, Long Task, heap, cache, + структурные инварианты не тронуты ни в одном профиле. +- Документация (`demo/performance/README.md`) обновлена в том же коммите, + вставлена в верном месте (сразу после описания профиля взаимодействия, до + раздела о полном `performance.yml`), числа в ней проверяемы и совпадают с + архивными ревью, а не придуманы. +- Класс изменения (B), трейлеры (`Issue:`, `User-Visible: no`) и отсутствие + правок changelog — согласованы между собой. +- Ветка стоит прямо на текущем `origin/dev` без расхождения — ребейза не + требовалось, полный разбор по этой причине не был нужен и не проводился. + +## Чего не проверял и почему + +- Живой прогон `performance.yml` / `benchmark_large_house.mjs` в браузере — + предрелизный гейт, не триггерится на этом SHA, целевое число уже + подтверждено историческими прогонами (см. таблицу выше). +- `check-docs.mjs`, `golden:verify`, `pytest tests_backend`, + `model-invariants.mjs` — diff не касается `src/**`, визуала, Python или + геометрии/ссылок; гейты к задаче не относятся. +- Browser-smoke (`demo/smoke_*.mjs`) — `smoke-select.mjs` явно вернул «выбирать + нечего», исполняемый frontend-код не менялся. + +## Материал раунда + +- SHA материала: `e67244c2a1f6b3e6e609955a26056c9434247cbe` +- База: `origin/dev` = `862cf151a521b2e65e17a355a9397ba53900788f` (ветка = dev + 1 коммит) +- Дерево: `git diff origin/dev...HEAD` (4 файла) + +--- + + + +## Материал раунда + +- Ветка: `issue/483-performance-smoke`, коммит `e67244c2a1f6` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `a4c06c0c8c268b6ec32a562e3f05c229bfd34537` + ``` + git log --all --format='%H %T' | grep a4c06c0c8c26 + ```