diff --git a/docs/reviews/CODE-REVIEW-743-r2.md b/docs/reviews/CODE-REVIEW-743-r2.md new file mode 100644 index 00000000..36297e64 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-743-r2.md @@ -0,0 +1,174 @@ +# CODE-REVIEW-743-r2 + +**Issue:** #743 · **Трек:** show · **Заход:** r2 · блокирующих циклов израсходовано 1 из 2 (лимит show) +**Материал:** `12687daa9da834d968a6f8f7e14969bfe4e7aa03` (ветка `issue/743-bench-iso-backdrop`, поверх `dev` `653d94ef`, три коммита: два из r1 плюс `docs: review document for #743`) + +## Скоуп + +Делта r1 → r2 — ровно один коммит `12687daa`, класс C (добавляет +`docs/reviews/CODE-REVIEW-743-r1.md`, 209 строк, публикующий шаг конвейера). +Продуктовый и тестовый код не меняются: `git diff d73b1782..12687daa --stat` +показывает только новый docs-файл. Единственная содержательная правка +раунда — не коммит, а комментарий автора в issue с новым прогоном Full +Performance на финальном SHA, закрывающий Medium r1. + +Задача по-прежнему проходит критерий §5 (разобрано в r1, код не изменился): +сложность низкая, одна поверхность (перф-харнесс), миграций и +compatibility-полей нет, нового UX-контракта нет (`User-Visible: no`), +продуктовой производительности/touch не касается, ожидаемое поведение +зафиксировано в ТЗ issue и `docs/ISOMETRIC.md` (#739 АС2). `route: fix`. + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где это видно | +|---|---|---| +| Medium — АС3 доказан не на финальном материале: единственный прогон Full Performance (`36869442701`) состоялся на `9e323303`, **до** коммита `d73b1782`, ужесточившего `switchCycleMs.hardMaxMs` с `8000` до `1550`; защитный потолок ни разу не проверялся прогоном | Автор запустил `performance.yml` заново на финальном SHA `12687daa` (= `d73b1782` плюс docs-коммит, продукт/тесты не менялись) — прогон [`36883292495`](https://github.com/Matysh/houseplan-card/actions/runs/36883292495), `conclusion: success`, headSha подтверждён. Job `isometric-backdrop` вызывает `compare.mjs --baseline-sha=d73b1782764640da67386c181fcfc62d1d9684a1 --candidate-sha=12687daa9da834d968a6f8f7e14969bfe4e7aa03` — то есть кандидат исполнялся именно на коммите с финальным бюджетом. Таблица отчёта: `✅ timing.switchCycleMs.median | 1070.4 | 1333.7 | 1083.7` — кандидат под потолком `1550` (предел `1333.7` в таблице — это `relativeLimit` от базы, более жёсткий, чем `hardMaxMs`, и он тоже пройден) | `gh run view 36883292495 --json headSha,jobs` (10/10 success, включая `isometric-backdrop`); `gh run view 36883292495 --log` → строка «Enforce relative and absolute performance budget» job `isometric-backdrop`; бюджет-файл `demo/performance/budgets-large-house-isometric-backdrop.json` на материале: `switchCycleMs.hardMaxMs: 1550` | + +Структурный АС2 (`perSwitch === 1` на каждом переключении) тоже подтверждён +этим прогоном, а не только юнит-тестом: `demo/performance/compare.mjs` вызывает +`requireReport`, которая **бросает** исключение `a warm 2.5D floor switch with +a backdrop took update passes`, если хоть один `perSwitch !== 1` +(`demo/performance/evaluate.mjs:65-76`, прочитано построчно). Job `isometric-backdrop` +завершился `success` и напечатал полную ✅-таблицу — то есть `requireReport` +не бросила исключение, то есть `perSwitch === 1` по всем строкам на +финальном коде. Это закрывает находку r1 сильнее, чем просто «автор +подтвердил цифрой в комментарии»: доказательство — исполнение на CI, а не +заявление. + +Новый прогон найден и прочитан мной напрямую (`gh run view`), а не принят +со слов хендоффа — числа и SHA сверены. + +## Унаследовано из r1 + +Код не менялся между r1 и r2 (делта — только docs-коммит), поэтому +нижеследующее из `docs/reviews/CODE-REVIEW-743-r1.md` (материал +`d73b1782764640da67386c181fcfc62d1d9684a1`) принято без повторной +проверки: + +- Профиль `large-house-isometric-backdrop-v1` заведён по закреплённым + образцам (id — README «Changing budgets», вариант фикстуры — как у + `large-house-plan-snap-v1`, бюджет-двойник — как у #160). +- Предусловие подложки `image.hp-backdrop` (`src/houseplan-card.ts:10923`, + геттер `:1169`) и имена `perf-floor-N` сверены с `_pickSpace` + (`src/houseplan-card.ts:1636`) и фикстурой `demo/fixtures/large-house.mjs:230`. +- Структурная проба стоит после окна `switchCycle` и сторожа #735, до + `forceGc`, вне таймеров. +- `evaluate.mjs`: кандидат этого профиля отвергается при `perSwitch !== 1` в + любой строке, при отсутствии `floorSwitchPasses` или `supported !== true`; + база не судится. Таблица «чем краснеет» заполнена юнит-тестом АС2, + мутация (временное отключение проверки) подтверждена исполнением + r1-ревьюером: `performance-budget.test.mjs` стал 18 pass/1 fail именно на + целевом тесте. +- Бюджет-файл — буквальная копия `budgets-large-house-isometric.json` кроме + `profile`; дрейф исключён `deepEqual`-юнит-тестом (приём как у #160). +- `test/performance-workflow.test.mjs`: счётчики `--samples=7 --warmups=1`, + `--baseline-sha=`/`--candidate-sha=`, запись матрицы `isometric-backdrop` — + численно совпадают с реальным `performance.yml` (подтверждено + исполнением теста). +- Trailers `Issue: #743` / `User-Visible: no` на обоих коммитах реализации + (`2f0b608e`, `d73b1782`) — корректно, changelog не требуется. +- Продукт (`src/**`), `demo/fixtures/**`, бандл не меняются. +- «Одно число — один источник»: `switchCycleMs.hardMaxMs=1550` фигурирует в + двух JSON-бюджетах (исторический + новый), тождество гарантирует + `deepEqual`-юнит-тест кроме поля `profile` — не новая находка, принятый в + проекте образец (#160). + +## Как проверялось в этом раунде + +- Прочитаны: `docs/SCOPE.md`, `AGENTS.md`, `docs/process/REVIEWER.md` + (повторный раунд — объём по дельте, §2.10), тело issue #743 и все + комментарии до текущего, включая ответ автора на Medium r1 и его ссылку + на прогон `36883292495`. +- `git log --oneline origin/dev..HEAD` и `git diff origin/dev...HEAD --stat` + — три коммита, из них r2-дельта (`d73b1782..12687daa`) — один файл, + `docs/reviews/CODE-REVIEW-743-r1.md`, только добавления. +- `gh run view 36883292495 --json headSha,headBranch,createdAt,conclusion,jobs` + — `headSha` совпадает с материалом ревью (`12687daa`), `headBranch` + — ветка задачи, `conclusion: success`, все 10 job зелёные. +- `gh run view 36883292495 --log` — построчно найдена и прочитана таблица + job `isometric-backdrop`: `--baseline-sha=d73b1782… --candidate-sha=12687daa…` + и полная ✅-таблица сравнения с бюджетом (`switchCycleMs` 1070.4 ≤ 1333.7 + ≤ скрытый `hardMaxMs=1550`). +- Прочитан построчно `demo/performance/evaluate.mjs:41-87` (`requireReport`) + — подтверждено чтением, что структурная проверка `perSwitch === 1` + бросает исключение и прерывает job при нарушении; раз job зелёный и + таблица напечатана целиком — проверка пройдена исполнением на CI, а не + предположением. +- Сверен бюджет-файл на материале: `demo/performance/budgets-large-house-isometric-backdrop.json` + → `switchCycleMs.hardMaxMs: 1550` — число, названное закрытой находкой, + действительно в файле. +- Trailers коммита `12687daa`: `Issue: #743`, `User-Visible: no` — docs-only + коммит мог обойтись без них (AGENTS.md, «Commits и branches»), но они + поставлены и корректны (ничего пользователю не видно). +- Дешёвые гейты не перегонялись: Validate на материале `12687daa` зелёный — + проверено напрямую (`gh run view 36884365543 --json headSha,conclusion` + → `headSha=12687daa…`, `conclusion: success`), это и прогон, названный в + промпте, и свежая сверка, а не слепое доверие ссылке. + +## Находки + +Нет. Единственная находка предыдущего раунда закрыта прогоном на финальном +материале, прочитанным и сверенным напрямую, новых изменений кода в этом +раунде нет. + +## Что проверено и корректно + +- AC3 (Full Performance на ветке, все профили зелёные, включая + `isometric-backdrop`) — доказан исполнением на финальном SHA `12687daa` + (прогон `36883292495`), а не на промежуточном, как было в r1. +- Структурный контракт «один проход на тёплое переключение при подложке» + (АС1/АС2) подтверждён не только юнит-тестом, но и реальным прогоном + `evaluate.mjs`/`compare.mjs` на CI — `requireReport` не бросила + исключение, job прошёл. +- Защитный потолок `switchCycleMs.hardMaxMs=1550` подтверждён измерением на + финальном коде: кандидат `1070.4` мс, запас ~31% — риск, названный в r1 + (локальный свидетель `1687` мс), не материализовался на CI-раннере. +- Делта раунда (docs-коммит) не нарушает правило 4 (бандл/golden не + трогает), классу C trailers не обязательны и корректны там, где + проставлены. +- Унаследованные из r1 пункты (список выше) остаются в силе — код, который + они описывают, не менялся. + +## Чего не проверял + +- Дешёвые гейты (`npx tsc --noEmit`, `npm test` целиком, `npm run build` + + сверка копий бандла) не перегонял повторно: зелёный Validate на SHA + материала подтверждён напрямую (`gh run view 36884365543`), делта раунда + их не касается (docs-only коммит). +- `npm run gate:small` не запускал — делта раунда не трогает код, который + он проверяет; в r1 он был подтверждён и код с тех пор не менялся. +- `golden:verify` — не запускал; рендер не меняется ни в r1, ни в r2-дельте + (docs-коммит), меток `ci:golden` нет. +- `pytest tests_backend` — не запускал, диффа в `custom_components/**/*.py` + нет ни в одном из трёх коммитов. +- `npm run invariants` — не запускал, геометрия плана не затронута. +- Мутанты реестра по диффу не запрашивались и не прогонялись (трек show, + #709) — решение конвейера, не находка. +- Не повторял ручного исполнения локального бенчмарка (`npm run + benchmark:large-house-isometric-backdrop`) — достаточно CI-прогона на + финальном SHA, он сильнее локального свидетельства и уже сверен напрямую. + +## Вердикт + +Зелёный. Находка r1 закрыта прогоном Full Performance на финальном +материале (`36883292495`, headSha `12687daa`), сверенным напрямую по логам +и JSON, а не принятым со слов хендоффа. Новых находок нет. Цикл зелёный +вердикт не образует; бюджет раунда остаётся израсходованным на 1 из 2 +(от r1). + +--- + +--- + + + +## Материал раунда + +- Ветка: `issue/743-bench-iso-backdrop`, коммит `12687daa9da8` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `2c07d96dd3f492aaa0090c5acfe0c6b1a031fa15` + ``` + git log --all --format='%H %T' | grep 2c07d96dd3f4 + ``` +- Тело issue: `62e82cbccdc0323ae350f6b2353d5c8b2e11b6ebdb4c1febe813bf20a54a0e4e` +- Вердикт конвейера: `green` · High 0 · маршрут `fix` +