From a2578e6f14c16f0b435c40b2eece25227c648b11 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Thu, 1 Oct 2026 14:34:12 +0000 Subject: [PATCH] docs: review document for #743 Issue: #743 User-Visible: no --- docs/reviews/CODE-REVIEW-743-r1.md | 209 +++++++++++++++++++++++++++++ 1 file changed, 209 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-743-r1.md diff --git a/docs/reviews/CODE-REVIEW-743-r1.md b/docs/reviews/CODE-REVIEW-743-r1.md new file mode 100644 index 00000000..07063e31 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-743-r1.md @@ -0,0 +1,209 @@ +# CODE-REVIEW-743-r1 + +**Issue:** #743 · **Трек:** show · **Заход:** r1 · блокирующих циклов израсходовано 0 из 2 (лимит show) +**Материал:** `d73b1782764640da67386c181fcfc62d1d9684a1` (ветка `issue/743-bench-iso-backdrop`, поверх `dev` `653d94ef`, два коммита) + +## Скоуп + +Задача закрывает техдолг перф-гейта (не продуктовую строку SCOPE.md напрямую — +это харнесс, класс B): ни один профиль Full Performance не ходит путём +2.5D-подложки (`imagePlan`), поэтому регрессию К1 #739 (двойной рендер на +переключении этажа) CI не видел ни по времени, ни структурно. Диапазон правок: + +- `demo/benchmark_large_house.mjs` — новый профиль `large-house-isometric-backdrop-v1`: + подложка на каждом этаже фикстуры, предусловие на `` в первом кадре, + структурная проба `floorSwitchPasses` после окна `switchCycle`/сторожа #735, + до `forceGc`. +- `demo/performance/evaluate.mjs` — кандидат этого профиля отвергается, если + хоть одно переключение заняло не 1 проход или проба отсутствует; база не + судится (до #739 — 2 прохода). +- `demo/performance/budgets-large-house-isometric-backdrop.json` — бюджет-двойник + `budgets-large-house-isometric.json`, отличается только `profile`. +- `.github/workflows/performance.yml`, `package.json` — новая запись матрицы и + npm-скрипт. +- `test/performance-budget.test.mjs`, `test/performance-workflow.test.mjs` — + юнит-контракты нового профиля/бюджета. +- `demo/performance/README.md` — абзац о профиле. + +Продукт (`src/**`) не меняется; `demo/fixtures/**` не меняется (фикстура +собирается в раннере, как у `plan-snap`); бандл не затронут. + +Трек show проходит критерий §5: сложность низкая (один новый тестовый профиль +по готовому образцу #160/#137), одна поверхность (перф-харнесс), миграций и +новых compatibility-полей нет, нового UX-контракта нет (ничего не видимо +пользователю — `User-Visible: no`, верно), влияния на продуктовую +производительность/touch нет (это CI-измерение, не продуктовый путь), ожидаемое +поведение зафиксировано в теле issue (ТЗ) и `docs/ISOMETRIC.md` (однопроходное +правило #739 АС2). `route: fix`. + +## Как проверялось + +- Прочитаны: `docs/SCOPE.md`, `AGENTS.md`, `docs/process/REVIEWER.md`, тело + issue #743 и комментарии (постановка, исследование, ТЗ, хендофф), фрагмент + `docs/ISOMETRIC.md` (ссылка на #739), `docs/TESTING.md` правило 3. +- Полный `git diff origin/dev...HEAD` прочитан по каждому файлу (не только + `--stat`); оба коммита и их trailers — построчно. +- Сверено чтением: признак подложки `image.hp-backdrop` в `src/houseplan-card.ts` + (строка рендера `10923`, геттер сравнения `1169`) — предусловие раннера + (`.stage svg image.hp-backdrop`) бьёт по реальному классу, а не по + выдуманному. +- Сверено чтением: `_pickSpace` (`src/houseplan-card.ts:1636`) и наименование + `perf-floor-${floor + 1}` в `demo/fixtures/large-house.mjs:230` — цикл пробы + `perf-floor-${(index % fixture.counts.floors) + 1}` обращается к + существующим id, `fixture.counts.floors` соответствует `FLOOR_COUNT`. +- Бюджетный файл сверен `jq 'del(.profile)'` против + `budgets-large-house-isometric.json` — идентичен (как и заявлено), что + согласуется с юнит-тестом `#743 backdrop budget is the historical isometric + budget under its own profile id`. +- Запущены оба изменённых тест-файла локально: + `node --test test/performance-budget.test.mjs` — 19/19, + `node --test test/performance-workflow.test.mjs` — 9/9 (28 всего, + согласуется с заявленными автором числами по файлам). +- Дисциплина «тест умеет падать» проверена исполнением: временно отключена + проверка `extra = passes.perSwitch.find(...)` в `evaluate.mjs` (заменена на + `undefined`) → `performance-budget.test.mjs` стал 18 pass / **1 fail** именно + на новом тесте `#743 backdrop candidate renders a warm 2.5D floor switch in + one update pass`; откат мутации восстановил зелёный прогон. Мутант по + диффу не требовался (трек show, #709) — это разовая проверка собственной + добросовестности теста, не прогон реестра. +- Проверены trailers обоих коммитов: `Issue: #743` на каждом, + `User-Visible: no` на каждом — корректно, changelog не нужен. +- Проверено по AC3 отдельно (см. находку ниже): `gh run list --workflow + performance.yml --branch issue/743-bench-iso-backdrop` → + ровно один прогон, `gh run view 36869442701 --json headSha,createdAt`: + `headSha=9e3233032119a734c2fc7d609d6a0edbc69b72c0`, + `createdAt=2026-10-01T13:32:35Z`; коммиты материала — + `2f0b608e` в `2026-10-01T14:21:14+03:00` (=`11:21:14Z`, до прогона) и + `d73b1782` в `2026-10-01T14:21:36+03:00`→`17:21:36+03:00`=`14:21:36Z` + (**после** прогона). Попытка перезапустить сам (`gh workflow run + performance.yml --ref issue/743-bench-iso-backdrop`) закономерно + отклонена: `HTTP 403: Resource not accessible by integration` — у + ревьюера нет права на workflow (AGENTS.md: «Push делает автор либо + главный агент с правом на workflow»), так что находка ниже не может быть + закрыта мной прогоном. +- Дешёвые гейты (`typecheck`, `npm test`, `npm run build` + сверка бандла) — + не перегонялись: Validate на этом SHA зелёный + (https://github.com/Matysh/houseplan-card/actions/runs/36875781664), + решение принято по правилу раунда. + +## Находки + +### Medium — AC3 доказан не на финальном материале + +**Файл/контекст:** хендофф-комментарий issue #743, раздел «AC3»; коммит +`d73b1782` (`demo/performance/budgets-large-house-isometric-backdrop.json`). + +**Воспроизведение.** АС3 требует: «Full Performance на ветке … все профили +зелёные, включая isometric-backdrop». Автор ссылается на прогон +`36869442701`, сам же помечая его «на `9e323303` (до второго коммита)». +Второй коммит `d73b1782` — не косметика: он меняет `hardMaxMs` метрики +`switchCycleMs` бюджета этого профиля с `8000` на `1550`, то есть ужесточает +именно тот защитный потолок, который АС3 должен подтвердить зелёным. Прогон +`36869442701` исполнялся на SHA `9e323303`, когда в бюджете ещё стоял +`8000` — почти на три порядка мягче. Единственный прогон Full Performance +этой ветки (подтверждено `gh run list`) состоялся **до** правки, +закрывающей собственно эту защиту; на `d73b1782` (материал ревью) AC3 не +исполнялся ни разу. + +**Почему это не бухгалтерия (не Low).** `switchCycleMs.hardMaxMs` — +защитный AC в терминах промпта (лимит/отказ): граница, которую новый +прогон мог бы не пройти, а никто не проверил. Риск не гипотетический: +собственный локальный свидетель автора по АС1 (до бюджетного коммита) +называет `switchCycleMs` ветки `1687` мс — уже выше итогового `1550`, +пусть и с оговоркой «локально, 2 CPU, рядом другие агенты — диагностика, +не доказательство» (так помечено в исследовательской части самого issue). +На CI-раннере показатель может оказаться другим в любую сторону, но именно +поэтому нужен реальный прогон на `d73b1782`, а не экстраполяция. + +**Чем закрывается.** Один свежий `workflow_dispatch` `performance.yml` на +`issue/743-bench-iso-backdrop` (ветка не меняется, ≈4 минуты — прогон +`36869442701` занял `3m50s`) с зелёным `isometric-backdrop` на `d73b1782` +(или более новом коммите того же содержания). Если он красный по +`switchCycleMs` — обсуждаемо: вернуть `hardMaxMs` к значению, +подтверждённому прогоном, либо прогнать достаточно образцов, чтобы медиана +легла в `1550` с варьируемым `noiseAllowanceMs=250`. + +**Серьёзность:** Medium, в скоупе задачи — чинится автором в этой же ветке +без выделения отдельного issue (§12). Без High это жёлтый вердикт и +повторный цикл. + +## Что проверено и корректно + +- Профиль заведён по закреплённым образцам: id — новый (README, + «Changing budgets»), вариант фикстуры в раннере — как у + `large-house-plan-snap-v1`, бюджет-двойник — как у #160, правило «один + проход на переключение» — ссылка на #739 АС2 и `docs/ISOMETRIC.md` + (ссылка подтверждена). +- Предусловие подложки (`image.hp-backdrop`) и имена `perf-floor-N` сверены + с реальным кодом/фикстурой, а не с воображаемыми именами. +- Структурная проба стоит после окна `switchCycle` и сторожа #735, до + `forceGc`, вне таймеров — подтверждено и исполнением юнит-теста + `#743 backdrop profile walks the imagePlan path…`, и чтением тела пробы. +- `evaluate.mjs`: кандидат этого профиля отвергается при `perSwitch !== 1` + в любом элементе любой строки, при отсутствии `floorSwitchPasses` или + `supported !== true`; база не судится. Таблица «чем краснеет» заполнена + (АС2 юнит-тест), мутация подтверждена исполнением (см. «Как + проверялось»). +- Бюджет — буквальная копия `budgets-large-house-isometric.json` кроме + `profile`; второй коммит привёл `switchCycleMs.hardMaxMs` к актуальному + потолку семьи 2.5D (#747, уже слит в `dev` и используется профилем + `large-house-isometric-v1`) — направление правки верное, вопрос только в + том, что её не подтвердили прогоном (находка выше). +- `test/performance-workflow.test.mjs` — счётчики `--samples=7 --warmups=1` + (18→20), `--baseline-sha=`/`--candidate-sha=` (2→3) и список матрицы + обновлены и численно совпадают с реальным текстом `performance.yml` + (проверено исполнением теста, не только чтением). +- Trailers `Issue: #743` и `User-Visible: no` на обоих коммитах; changelog + не требуется и не тронут — корректно. +- Продукт (`src/**`), `demo/fixtures/**`, бандл не меняются — заявление + подтверждено `git diff --stat`. +- «Одно число — один источник»: `switchCycleMs.hardMaxMs=1550` фигурирует в + двух JSON (историческом и новом бюджете), но дрейф исключён юнит-тестом + `deepEqual` на их тождественность кроме `profile` — тот же приём, что у + #160; не новая находка, это принятый в проекте образец. +- `docs/benchmark/README.md` (`demo/performance/README.md`) получил абзац + о профиле и команду локального прогона — раздел «Что меняется» п.5 ТЗ + выполнен. + +## Чего не проверял + +- Дешёвые гейты (`npx tsc --noEmit`, `npm test` целиком, `npm run build` + + сверка копий бандла) не перегонялись повторно — зелёный Validate на этом + SHA принят как основание (ссылка выше). +- Полный `npm run gate:small` не запускал — доверяю заявлению автора по + этому пункту вкупе с собственным прогоном изменённых тест-файлов. +- Full Performance **на финальном SHA** не прогонялся (ни автором, ни мной + — нет прав на `workflow_dispatch`); это и есть находка выше, а не просто + пропуск. +- `golden:verify` — не запускал, в диффе нет меток `ci:golden` и нет правок + пути отрисовки плана/`demo/golden/**`; рендер не меняется (подложка + существующая, геометрия комнат не меняется) — запись в «чего не + проверял» по указанию раздела «Трек show» (§5.1), без находки. +- `pytest tests_backend` — не запускал, диффа в `custom_components/**/*.py` + нет. +- `npm run invariants` — не запускал, геометрия/ссылки на неё не + затронуты (бюджет и раннер не меняют модель плана). +- Мутанты реестра по диффу не запрашивались и не прогонялись (трек show, + #709) — это решение конвейера, не находка. + +## Вердикт + +Один Medium в скоупе, High нет → жёлтый вердикт, возврат автору на +доработку (прогнать Full Performance на финальном SHA и либо подтвердить +`switchCycleMs.hardMaxMs=1550`, либо скорректировать число по реальному +прогону). + +--- + + + +## Материал раунда + +- Ветка: `issue/743-bench-iso-backdrop`, коммит `d73b17827646` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `05034ee1adba3b82302d5ce071c62f1b35363dd6` + ``` + git log --all --format='%H %T' | grep 05034ee1adba + ``` +- Тело issue: `62e82cbccdc0323ae350f6b2353d5c8b2e11b6ebdb4c1febe813bf20a54a0e4e` +- Вердикт конвейера: `yellow` · High 0 · маршрут `fix` +