diff --git a/docs/reviews/CODE-REVIEW-692-r1.md b/docs/reviews/CODE-REVIEW-692-r1.md new file mode 100644 index 00000000..91a8152a --- /dev/null +++ b/docs/reviews/CODE-REVIEW-692-r1.md @@ -0,0 +1,129 @@ +# CODE-REVIEW-692-r1 + +Материал раунда: `git log --oneline origin/dev..HEAD` = 1 коммит +(`bb33626f3806fbde5ecdfb3e65e99669c99eae71`), ветка `issue/692-perf-first-stable` +от `dev` @ `4c74e80a`. Заход r1, трек `track:show`, блокирующих циклов +израсходовано 0 из 2. + +## Скоуп + +Инфраструктурная задача класса B: поднять `hardMaxMs` метрики +`firstStableRenderMs` профиля `large-house-interaction-v1` с 3000 до 3400 мс +(и в его smoke-двойнике), задокументировать обоснование в +`demo/performance/README.md` и закрепить число тестом. Повод — находка ревью +#689 (CODE-REVIEW-689-r1): бюджет краснел на 2.4 мс на честном Validate-прогоне +из-за того, что уровень `firstStableRenderMs` вырос между линиями 1.77 и +1.78.0, а не из-за кода #689. Задача обслуживает J6 (SCOPE.md: «Keep the plan +true as the home evolves») косвенно — держит CI performance-гейт полезным +(ловит регрессии), а не шумящим ложными красными. + +Файлы диффа (4, все класса B): + +| Файл | Изменение | +|---|---| +| `demo/performance/budgets-large-house-interaction.json` | `firstStableRenderMs.hardMaxMs` 3000 → 3400 | +| `demo/performance/budgets-interaction-smoke.json` | то же число, синхронно (смок = полный профиль, #473 AC4) | +| `demo/performance/README.md` | абзац с рядом замеров и обоснованием, по образцу существующего абзаца про `spaceSwitchMs` (#675) | +| `test/performance-budget.test.mjs` | новый тест `#692`, закрепляющий число и точки ряда | + +Файлов класса A нет (`src/**`, Python не тронуты) — подтверждено диффом. +`User-Visible: no` в трейлере коммита корректен: изменение не меняет ничего, +что видит пользователь карточки, только CI-гейт. Оба changelog поэтому +не требуются и не тронуты — правильно. + +## Как проверялось + +| Гейт | Прогнан | Результат | +|---|---|---| +| `typecheck` / `npm test` / `npm run build` + bundle-policy | нет, повторно | Validate на этом точном SHA `bb33626f` зелёный (run [36673058928](https://github.com/Matysh/houseplan-card/actions/runs/36673058928)) — дешёвые гейты подтверждены, перегонять не стал | +| `node --test test/performance-budget.test.mjs` | да, локально | 15/15 зелёных, включая новый тест `#692` | +| Тест `#692` умеет падать | да, проверено мутацией | `hardMaxMs` полного профиля вручную возвращён 3400 → 3000, тест `#692` красный (`not ok 11`, `testCodeFailure`); файл восстановлен, `git status` чист | +| `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | да | «Исполняемого frontend-диффа нет (`src/**/*.ts` не тронут)… смоки не выбираются — выбирать нечего» — браузерные смоки не нужны | +| `golden:verify` | нет | метки `ci:golden` на issue нет, golden-баз не тронуто | +| `pytest tests_backend` | нет | Python не тронут | +| `npm run invariants` | нет | геометрия не тронута | +| Свежий прогон `large-house-interaction-v1` (7 сэмплов) | нет | не требуется AC; авторитетный ряд — прогоны `performance.yml` (перечислены в оценке владельца и в README), не разовый повторный замер этой веткой, которая рантайм не меняет | +| Мутанты в реестре | не гонял (и не должен) | track `show`: мутанты в разработке не гоняются ни на каком треке (#709); отсутствие прогона не находка | + +Кросс-проверка «одно число — один источник» (§8): `firstStableRenderMs` +встречается в других бюджетных файлах (`budgets.json`, +`budgets-isometric-smoke.json`, `budgets-large-house-plan-snap.json`, +`budgets-large-house-isometric.json`, `budgets-isometric-stage3-dense.json`), +но это другие профили (default/plan-snap/isometric) с собственными +потолками (3000/3500), не дубликаты значения `large-house-interaction-v1` — +их не трогали и не должны были. Синхронизированы ровно два файла, которые и +названы владельцем в оценке как «одно число». + +## AC · чем доказан · чем краснеет + +| AC | Чем доказан | Чем краснеет | +|---|---|---| +| AC1 — `hardMaxMs` = 3400 в обоих файлах (полный профиль и смок), коэффициент/допуск полного сравнения не ослаблены | `test/performance-budget.test.mjs:254-269`, прогнан локально, 15/15 | Проверено мутацией: ручной откат `hardMaxMs` на 3000 красит тест `#692` (`not ok`, см. таблицу выше); тест также проверяет `smoke.hardMaxMs === full.hardMaxMs`, `maxRegressionRatio === 0.3`, `noiseAllowanceMs === 250` — ослабление любого из них тоже красит соответствующий `assert.equal` | +| AC2 — ряд и обоснование в README, тест закрепляет число и точки ряда | Прочитан текст README (строки 102–111): числа (2769.0, 2838.2, 2777.3, 2801.7, 2925.0, 2925.2, 3144.8, 3400) совпадают построчно с оценкой владельца в issue и с константами в тесте `#692`. Проверено чтением, не исполнением — README не исполняется | Тест `#692` пинит те же числа рядов (`level177`, `level178`, `worst`) отдельно от prose — рассинхрон README/теста не будет пойман автоматически, но это документационный AC, не защитный; риск низкий (числа скопированы дословно, сверены вручную) | + +Оба AC выполнены. AC1 — защитный (лимит/потолок гейта), таблица заполнена +результатом реального прогона мутации, третий столбец не пуст. + +## Проверено и корректно + +- Арифметика обоснования сходится: 3400 / 2925.2 (макс. уровня 1.78) ≈ 1.162 + → «+16 %», 3400 / 3144.8 (худший прогон) ≈ 1.081 → «+8 %» — оба числа из + текста issue/README подтверждены пересчётом. +- Тест `#692` — не просто фиксация числа: он же проверяет полосу 15–20 % над + уровнем 1.78 (`ceiling >= max(level178)*1.15` и `<= max(level178)*1.2`), + то есть страхует от будущего чрезмерного расширения потолка тем же + тестом, что фиксирует его сегодняшнее значение. +- Формулировка в README и коммите продолжает точно тот же стиль и структуру + доказательства, что и предыдущий прецедент того же файла — абзац про + `spaceSwitchMs`/#675 (строки 113–124): уровень, шум раннера, запас, что + ловит меньшие регрессии. +- Трейлеры коммита корректны: `Issue: #692`, `User-Visible: no` (верно — + поведение продукта не меняется), `Claude-Session` присутствует. +- Рабочая копия после моей мутационной проверки восстановлена в исходное + состояние (`git status --porcelain` пуст). + +## Чего не проверял + +- Не гонял новый 7-сэмпловый прогон `large-house-interaction-v1` в CI — + задача сознательно опирается на уже существующий авторитетный ряд + `performance.yml` (перечислен владельцем в оценке), а не производит новое + измерение; AC этого не требует. +- Не гонял `golden:verify`, `pytest tests_backend`, `npm run invariants` — + ни один не применим к этому диффу (нет golden-меток, Python, геометрии). +- Не перегонял `tsc --noEmit` / `npm test` (полный) / `npm run build` — + зелёный Validate на этом точном SHA (`bb33626f`, run 36673058928) уже их + подтвердил. +- Не запускал реестр мутантов — по правилам track `show` (#709) он не + гоняется на этом этапе вообще; заменил его точечной ручной мутацией + одного значения, чтобы лично убедиться, что новый тест умеет падать. +- Не проверял шаг +4.5 % между линиями 1.77 и 1.78.0 по существу — задача + явно выносит бисект за скоуп («Шаг… здесь не разбираю… если нужен бисект — + отдельная задача»), и это соответствует объёму track `show` (до трёх AC). + +## Находки + +Нет. High: 0, Medium: 0, Low: 0. + +## Вердикт + +Зелёный. AC1 и AC2 выполнены и доказаны — AC1 автотестом с подтверждённой +способностью падать (проверено мутацией самим ревьюером), AC2 чтением с +явной пометкой «проверено чтением, не исполнением». Дешёвые гейты подтверждены +зелёным Validate на этом SHA, гейты по диффу (смоки, golden, pytest, +инварианты) не применимы и это обосновано в разделе «Чего не проверял». +Расхождений с `docs/SCOPE.md` или `docs/USER-GUIDE.ru.md` нет — видимого +поведения нет вовсе. + +--- + + + +## Материал раунда + +- Ветка: `issue/692-perf-first-stable`, коммит `bb33626f3806` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `0d4b3fe50cf39c6d21fae4dca3f1ce18db57fcf6` + ``` + git log --all --format='%H %T' | grep 0d4b3fe50cf3 + ``` +- Тело issue: `0450569e776cc3dadca6ebd0463bf615cf328185a6d8dfae9ca0af3acb4c062f` +- Вердикт конвейера: `green` · High 0