mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-04 05:41:34 +00:00
@@ -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`:
|
||||
подложка на каждом этаже фикстуры, предусловие на `<image>` в первом кадре,
|
||||
структурная проба `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`, либо скорректировать число по реальному
|
||||
прогону).
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/743-bench-iso-backdrop`, коммит `d73b17827646` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `05034ee1adba3b82302d5ce071c62f1b35363dd6`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 05034ee1adba
|
||||
```
|
||||
- Тело issue: `62e82cbccdc0323ae350f6b2353d5c8b2e11b6ebdb4c1febe813bf20a54a0e4e`
|
||||
- Вердикт конвейера: `yellow` · High 0 · маршрут `fix`
|
||||
<!-- hp:usage input_tokens=5080 output_tokens=24492 cache_creation_input_tokens=85891 cache_read_input_tokens=2722177 num_turns=49 -->
|
||||
Reference in New Issue
Block a user