mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-07 15:09:30 +00:00
@@ -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 материала — оба
|
||||
смоковых профиля реально выполнились и прошли с разумным запасом.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/770-perf-budgets-harness`, коммит `079ab8299396` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `3859a3c2b014b5517bfdce649b75f3baa22a6c38`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 3859a3c2b014
|
||||
```
|
||||
- Тело issue: `91895b2844968372ec26698651f6f634f8b5ade6bf1c448177aa37c8764bddd5`
|
||||
- Вердикт конвейера: `green` · High 0 · маршрут `fix`
|
||||
<!-- hp:usage input_tokens=4468 output_tokens=22986 cache_creation_input_tokens=87326 cache_read_input_tokens=2393534 num_turns=39 -->
|
||||
Reference in New Issue
Block a user