Files
2026-09-27 09:02:10 +00:00

11 KiB
Raw Permalink Blame History

CODE-REVIEW-675-r1

Материал раунда: 6183119f1e9e5daa483492567b7c0a2e22a6cfaf (HEAD, детач), база origin/dev @ 7871efb0. Диапазон: git log --oneline origin/dev..HEAD — один коммит; git diff origin/dev...HEAD. Заход: r1 · блокирующих циклов израсходовано 0 из 4.

Скоуп

Issue #675 (класс B, инфраструктура/гейты, S7-code-review): абсолютный потолок spaceSwitchMs изометрии колебался у 1800 мс и красноил performance_smoke без изменения кода этого пути — уровень метрики вырос вместе с 2.5D-линией. Владелец 26.09 выбрал вариант A: перекалибровка потолка с записанным обоснованием (issue, комментарий #issuecomment-5854212474).

Изменённые файлы (все класса B, docs/SCOPE.md не задевается — это чисто CI-гейт, не продуктовая работа):

  • demo/performance/budgets-isometric-smoke.json, demo/performance/budgets-large-house-isometric.json, demo/performance/budgets-isometric-stage3-dense.json — spaceSwitchMs.hardMaxMs 1800 → 2200 во всех трёх;
  • demo/performance/README.md, раздел «CI contracts» — абзац с рядом замеров и обоснованием числа;
  • test/performance-budget.test.mjs — конструктор синтетического отчёта build(firstFrame) поднят на уровень модуля как absoluteSmokeReport (переиспользуется и старым тестом #473 AC4, и новым), плюс новый тест «isometric space switch ceiling covers the 2.5D runner level and still catches #583 (#675)».

Трейлеры коммита: Issue: #675, User-Visible: no — корректно (числа CI-гейта не видны пользователю продукта, changelog не тронут и не нужен).

Как проверялось

  • Прочитан весь диапазон git diff origin/dev...HEAD (5 файлов, +77/−29) — дифф компактный, вписывается в один проход целиком.
  • Прочитано тело issue #675 и оба комментария (постановка + отчёт «Сделано» с рядом замеров и таблицей AC).
  • Сверено обоснование в demo/performance/README.md с текстом issue и с константами внутри нового теста — числа 1867.7 / 2719.6 / 1798.3 совпадают дословно между README, issue-комментарием и телом теста (одно число, три места, но кросс-проверено тестом, не рассинхронизировано).
  • Проверено, что среди JSON-бюджетов с полем spaceSwitchMs остальные четыре файла (budgets-interaction-smoke.json, budgets-large-house-interaction.json, budgets-large-house-plan-snap.json, budgets.json) относятся к другим профилям (large-house-interaction-v1, large-house-plan-snap-v1, large-house-v1 — без 2.5D-геометрии) и сохраняют старое значение 1500 мс; это ожидаемо и вне скоупа задачи, не пропущенное место.
  • grep по репозиторию на предмет забытого «1800» рядом со spaceSwitchMs — чисто, старое число нигде не осталось.
  • Прогнал node --test test/performance-budget.test.mjs — 14/14 зелёных.
  • Проверил дисциплину «тест умеет падать»: временно вернул budgets-isometric-smoke.json → spaceSwitchMs.hardMaxMs: 1800 (мутация, имитирующая старый/забытый потолок) и перезапустил файл — новый тест #675 красит (not ok 12), после чего файл восстановлен из бэкапа; git status подтвердил чистую рабочую копию.
  • Проверил, что рефактор build(firstFrame) → absoluteSmokeReport(smoke, timings) не сломал старый тест #473 AC4 (он использует тот же конструктор с { firstStableRenderMs: 9870 }) — подтверждено тем же прогоном (все 14 тестов, включая оба теста AC4 для isometric/interaction смоков, зелёные).

AC · чем доказан · чем краснеет (из отчёта автора, сверено ревьюером)

AC Чем доказан Чем краснеет Проверка ревьюера
Смок проходит стабильно Ряд из 9 прогонов после #649, максимум 1867.7 ≤ 2200 — (эмпирическая устойчивость, не guard-тест) Число 1867.7 совпадает в README и в тесте; не защитный AC — доп. столбец не обязателен
Ловит «в разы» Тест #675: 2719.6 и 2×1798.3 красные, 1867.7 зелёный hardMaxMs 2800 → not ok; 1800 → not ok (автор прогнал оба) Независимо воспроизвёл: мутация 1800 краснит тест (см. выше)
Одно число в трёх файлах Тест #675 перебирает все три JSON Любое расхождение файлов → not ok Прочитано: цикл for (const file of files) assert.equal(...2200...) действительно покрывает все три пути
Обоснование рядом с числом README, «CI contracts», абзац привязан тестовым комментарием — (существование текста, не guard) Абзац на месте, числа совпадают с тестом

Два пустых третьих столбца («Смок проходит стабильно», «Обоснование рядом с числом») — не находка: это не защитные AC в смысле REVIEWER.md (валидация/ гард/лимит/отказ/инвариант), а описательное утверждение об устойчивости эмпирического ряда и факт наличия текста. Единственный настоящий защитный AC («ловит в разы» / «одно число в трёх файлах») даёт заполненный столбец, и я independently подтвердил его мутацией.

Что проверено и корректно

  • Три файла с потолком spaceSwitchMs синхронны (2200 мс), проверено чтением и тестом.
  • maxRegressionRatio/noiseAllowanceMs полного сравнения не тронуты — рычагом остаётся только абсолютный потолок, как и требовало issue.
  • Классификация изменения (класс B, User-Visible: no) верна; трейлеры на коммите на месте.
  • Рефактор синтетического конструктора отчёта поведение-сохраняющий.
  • Числа в README, issue-комментарии и теле теста согласованы дословно — «одно число — один источник» выполняется через тест-инвариант, а не через единственную константу (это тот же паттерн, что уже принят в #473 AC4).

Чего не проверял и почему

  • npx tsc --noEmit, npm test (полный), npm run build + сверка бандла — Validate на этом же SHA (6183119f) уже зелёный (https://github.com/Matysh/houseplan-card/actions/runs/36307603755); повторный прогон дешёвых гейтов не требуется правилами раунда. Сам прогнал только целевой test/performance-budget.test.mjs — точечно и с мутацией, чтобы лично убедиться, что новый тест умеет падать.
  • node scripts/check-docs.mjs — не требуется, diff не касается src/**.
  • Браузерные смоки / smoke-select.mjs — не требуется: diff не меняет исполняемый frontend-код (только JSON-бюджеты, README и unit-тест), нет затронутого пути рендера. Автор указывает то же в отчёте.
  • npm run golden:verify — не требуется, нет изменений рендера.
  • python -m pytest tests_backend — не требуется, Python не тронут.
  • npm run invariants — не требуется, геометрия модели не меняется, это калибровка performance-бюджета, а не изменение геометрии/ссылок на неё.
  • Достоверность самих исторических цифр CI (прогоны 36306131709, 35070397356 и сводки job summary за 09-06…09-27) — не перепроверял по первоисточнику (страницы конкретных прогонов на GitHub Actions с их retention). Это статистический анализ шума раннера, а не код; доверился прослеживаемости чисел между issue, README и тестом (совпадают дословно) и тому, что решение о самом факте перекалибровки — вариант A — уже принято владельцем 26.09.

Находки

Нет High. Нет Medium. Нет Low.

Вердикт

Зелёный. Задача точечная, полностью в рамках issue и решения владельца, единственный защитный AC доказан тестом, который я лично воспроизвёл как падающий на мутации; дешёвые гейты подтверждены зелёным Validate на этом SHA.


Материал раунда

  • Ветка: issue/675-iso-space-switch-budget, коммит 6183119f1e9e — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
  • Дерево материала: b3aac9a5fbe9b6b5cd38a192868962b8f27c3850
    git log --all --format='%H %T' | grep b3aac9a5fbe9
    
  • Тело issue: b7a671b373053c11b840caf6a585a7c0645df8b7a06e0b713d4c4ad195c49976
  • Вердикт конвейера: green · High 0