diff --git a/docs/reviews/CODE-REVIEW-740-r2.md b/docs/reviews/CODE-REVIEW-740-r2.md new file mode 100644 index 00000000..1eddb0c8 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-740-r2.md @@ -0,0 +1,143 @@ +# CODE-REVIEW-740-r2 + +Issue: #740 · «Производительность: слой лестниц — ≈50 мс на переключение на этаж 1» +Трек: ask (перф, §5) · Этап: code · Заход: r2 · блокирующих циклов израсходовано 1 из 4 +Материал: `8e4f417f51f46dc49239deb508a4fd341dc72f06` (ветка `issue/740-stairs-floor-switch`) + +## Скоуп + +Продуктовый код не менялся с r1: К1 (ступени одной лестницы — один +`` в View и в редакторе плана, строки разметки +кэшируются `WeakMap` по объекту `cachedStairRenderGeometry`) уже проверен и +принят в `docs/reviews/CODE-REVIEW-740-r1.md`. Дельта этого раунда — три +коммита над материалом r1 (`1b46c3f4`): + +- `d709ea11` perf(stairs) — тот же код К1, что ревизовался в r1 (байт-в-байт, + см. «Как проверялось»); +- `b8f662cd` docs(screenshots) — сдвиг `sourceFingerprint`/`sourceSha256` + после очередного ребейза на ушедший вперёд `dev`, `imageSha256` не + изменился ни у одного из 11 кадров (не визуальная правка); +- `8e4f417f` docs: публикация `CODE-REVIEW-740-r1.md` в репозиторий (шаг + конвейера, не авторский код). + +Единственная находка r1 была одна — Medium в скоупе, AC4 (Full Performance +не был переизмерен против актуального merge-base после трёх ребейзов). +Автор ответил в issue (18:26 UTC) новым прогоном против верного +merge-base. Этот раунд целиком посвящён проверке закрытия этой находки и +тому, что дельта раунда не внесла ничего нового. + +## Как проверялось + +| Гейт | Статус | Источник | +|---|---|---| +| `npx tsc --noEmit`, `npm test`, `npm run build` + сверка бандла | зелёные | Validate на точном SHA материала `8e4f417f`: [36907029311](https://github.com/Matysh/houseplan-card/actions/runs/36907029311) — приняты как дешёвые по инструкции прогона, не перегонял | +| Байт-в-байт сравнение продуктовых файлов между коммитом, на котором измерен Full Performance (`c1466c13`), и материалом ревью (`8e4f417f`) | идентичны: `src/stairs.ts`, `src/stairs-view.ts`, `src/stairs-editor.ts`, `test/stairs.test.mjs`, `demo/smoke_stairs.mjs`, `scripts/mutation-registry.mjs`, `scripts/smoke-links.mjs`, `docs/STAIRS.md`, `docs/testing-notes/mutation-browser-guards.md` | прогнал сам: `gh api .../contents/?ref=c1466c13` построчно против `git show 8e4f417f:` | +| Перф-значимость коммитов `dev` между `0606a366` (база, на которой автор переизмерил) и `9f695d8a` (фактический merge-base материала `8e4f417f`) | ровно один коммит, `9f695d8a` — чистый docs (ревью-документ #762), `src/`, `demo/performance`, `scripts/` не тронуты | `git log --oneline 0606a366..9f695d8a` + `git diff 0606a366..9f695d8a --stat -- src/ demo/performance scripts` — пусто | +| Full Performance автора — реальность и база прогона | прогон [36905707715](https://github.com/Matysh/houseplan-card/actions/runs/36905707715): `conclusion=success`, `headSha=c1466c13…`, событие `workflow_dispatch` («Полные бенчмарки производительности») | прогнал `gh run view --json`, не поверил заявлению из комментария | +| Comparison base прогона `36905707715` | лог job «Resolve comparison SHA»: `Comparison base: 0606a3664a6c819a501316f478840d6c3a5b456c (manual comparison ref 0606a366…)` — именно та база, что требовалась находкой r1 | прочитал реальный вывод job через `gh run view --log` | +| Все 10 job'ов прогона `36905707715` | все `success` (`interaction`, `blend`, `isometric-stage3`, `overlay`, `large-house`, `space-glow`, `isometric`, `isometric-backdrop`, `space-default`, `plan-snap`) | `gh run view --json jobs` | +| Бюджеты/`hardMaxMs` | не тронуты диффом | `git diff origin/dev...HEAD --stat -- demo/performance` — пусто | +| AC4 условие по `switchCycleMs` | `large-house-v1` 617.1→566.5 (−8.2 %, ниже базы ✅); `large-house-isometric-v1` 1003.9→899.9 (−10.4 %, не выше ✅); `large-house-plan-snap-v1` 749.9→699.9 (−6.7 %, не выше ✅) | цифры из комментария автора, сверены с условием AC4 issue дословно | +| Локальные числа «заход на этаж 1» / «тёплый цикл» против базы | опубликованы в более раннем комментарии «Сделано» (09:51 UTC): +46→+26 мс (плоский), +45→+25 мс (2.5D); остаются в силе, т.к. продуктовый код с тех пор не менялся (см. байт-в-байт сравнение выше) | прочитан исходный комментарий, сверен с таблицей «Проблемы» issue | +| Трейлеры всех трёх коммитов дельты | `Issue: #740`, `User-Visible: no` — на месте; changelog не правился, корректно (поведение не видно) | `git show -s --format='%B'` на каждый коммит | + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где это видно | +|---|---|---| +| Medium (в скоупе) — AC4 не доказан: единственный Full Performance прогон стоял на устаревшей базе `7ff2b5ae`, между ней и фактической `0606a366` попали перф-значимые коммиты #739/#742/#745/#747 | Автор перезапустил `performance.yml` с `comparison_ref=0606a366` (кандидат `c1466c13`, который содержит тот же код К1, что уже лежит на ветке) | Прогон [36905707715](https://github.com/Matysh/houseplan-card/actions/runs/36905707715), job «Resolve comparison SHA» называет точную запрошенную r1 базу; 10/10 job'ов `success`; числа по трём профилям AC4 опубликованы в issue (комментарий 18:26 UTC) и совпадают с условием AC4 дословно | + +## Унаследовано из r1 + +Без повторной проверки, т.к. продуктовый код байт-в-байт тот же (см. таблицу +выше), а r1 проверил эти пункты чтением с доказанной способностью тестов +падать: + +- **AC1** — `stairTreadPath`/`cachedStairMarkup`, `WeakMap`-кэш с `===`-идентичностью, + нулевой случай (0 ступеней). Документ: `docs/reviews/CODE-REVIEW-740-r1.md`, + раздел «Что проверено и корректно», на SHA `1b46c3f4` (код идентичен `8e4f417f`). +- **AC2** — разметка `path.hp-stair-tread` вместо `line`, остальной символ не + менялся, `smoke-select` подтверждает единственную и зарегистрированную + связь. Тот же документ/SHA. +- **AC3** — golden всех 4 сцен лестниц `passed`, проверено исполнением через + цепочку переиспользования CI (`source_sha=5b94c09e…`). Тот же документ/SHA. +- **AC5** — мутант `stairs-view-tread-lines` зарегистрирован с верным гардом, + `mutation-gate.mjs --check` зелёный. Тот же документ/SHA. +- Риск-класс «geometry» (`src/stairs.ts:345,346,347,349,356` и остальные + строки блока `StairMarkup`/`stairTreadPath`/`cachedStairMarkup`, + строки 331–366) — весь этот участок и есть предмет AC1/AC2 из r1; новых + строк вне уже проверенного блока дельта не добавила. + +## Находки + +Нет. + +## Что проверено и корректно + +- Дельта раунда (`b8f662cd`, `8e4f417f`) не трогает продуктовый код, тесты, + смоки, реестр мутаций и `STAIRS.md` — подтверждено байт-в-байт. +- `b8f662cd` меняет только `sourceFingerprint`/`sourceSha256` в + `docs/images/screenshots.json`; `imageSha256` всех 11 кадров не изменился + ни на бит — это сдвиг входного отпечатка после ребейза на новый `dev`, а + не визуальная правка. Не затрагивает 4 golden-сцены лестниц (AC3), + которые вообще не входят в этот список из 11 кадров документации. +- AC4 выполнен на материале ревью: Full Performance пересчитан против того + самого `comparison_ref`, который требовала находка r1 (`0606a366`), + доказано чтением логов самого прогона, а не доверием к тексту комментария; + код, измеренный прогоном (`c1466c13`), идентичен коду материала (`8e4f417f`); + `dev` между измеренной и фактической базой не содержит перф-значимых + изменений — измерение валидно для `8e4f417f`, а не только для `c1466c13`. + Все три условия AC4 (`large-house` ниже базы, `isometric`/`plan-snap` не + выше, бюджеты/`hardMaxMs` не меняются) выполнены. +- `stateUpdateMs` вырос на +10…+17 % в четырёх профилях — это не часть + условия AC4 (которое завязано на `switchCycleMs`), бюджет не нарушен + (все 10 job'ов `success`, запас до потолка 2–3×), и автор сам вынес цифры + в issue, а не скрыл. Находкой не считаю: AC4 не требует отсутствия любых + колебаний соседних метрик, только отсутствия нарушений бюджета и + заданного условия по `switchCycleMs`. +- Трейлеры всех коммитов дельты корректны, `User-Visible: no` обосновано + (markup-рефакторинг и сдвиг отпечатка документации не видны пользователю), + changelog не правился — верно. + +## Чего не проверял + +- **`npx tsc --noEmit`, `npm test`, `npm run build`** — не перегонял отдельно, + принял зелёный Validate на точном SHA материала `8e4f417f` + ([36907029311](https://github.com/Matysh/houseplan-card/actions/runs/36907029311)) + согласно инструкции прогона; продуктовый код в дельте раунда отсутствует, + так что это не ослабляет гарантию. +- **Full Performance** сам не запускал — дорогой гейт; вместо повторного + прогона прочитал реальные логи и JSON чужого прогона (`gh run view`, + `--json jobs`, `--log`) как доказательство исполнением, не как заявление + автора. +- **`golden:verify` локально** — не прогонял; код не изменился с r1, где уже + проверена цепочка переиспользования CI на конкретном SHA с `passed` по + всем 4 сценам лестниц. +- **`npm run invariants`, `pytest tests_backend`** — не прогонял: дельта + раунда не код, геометрия лестниц не меняется (то же, что в r1). +- **Ручной визуальный осмотр скриншотов** — не делал; `imageSha256` + неизменность по всем 11 кадрам — более строгая проверка, чем осмотр глазом. + +## Вердикт + +Зелёный. High 0, Medium 0. Единственная находка r1 (Medium, AC4) закрыта +проверяемым прогоном Full Performance против верного merge-base; дельта +раунда — два некодовых коммита (сдвиг отпечатка скриншотов и публикация +документа r1), оба без влияния на поведение или на уже принятый код К1. + +--- + +--- + + + +## Материал раунда + +- Ветка: `issue/740-stairs-floor-switch`, коммит `8e4f417f51f4` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `fd367dd826f421ded4b32294553c4eb0386d14e4` + ``` + git log --all --format='%H %T' | grep fd367dd826f4 + ``` +- Тело issue: `9cfcfc9e8157d5a25961e793ed530de257848fb5d870dee885fbd71103ffb3bd` +- Вердикт конвейера: `green` · High 0 · маршрут `fix` +