From 0c748282408cbc5fb8ce397fcc4153db2a885db3 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Thu, 10 Sep 2026 09:12:51 +0000 Subject: [PATCH] docs: review document for #509 Issue: #509 User-Visible: no --- docs/reviews/CODE-REVIEW-509-r1.md | 221 +++++++++++++++++++++++++++++ 1 file changed, 221 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-509-r1.md diff --git a/docs/reviews/CODE-REVIEW-509-r1.md b/docs/reviews/CODE-REVIEW-509-r1.md new file mode 100644 index 00000000..da3c5d49 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-509-r1.md @@ -0,0 +1,221 @@ +# CODE-REVIEW-509-r1 + +Заход r1 · блокирующих циклов израсходовано 0 из 4 (первый прогон дispatch на `172d9d5a` покраснел на собственном смоке автора, до чтения кода ревьюером — цикл не считается, PROCESS.md §2.9/#510). + +Материал: диапазон `origin/dev..HEAD`, `HEAD` = `9b00002a5b09f64a4fea0b51bc03ca6be317ec7b` (рабочая копия уже на нём, `git fetch`/`checkout` не выполнялись). + +## Скоуп + +Одна поверхность — runtime сводной панели (`src/summary-panel-runtime-loaded.ts`, +`src/summary-panel-metrics.ts`, `src/summary-panel-style.ts`) плюс новый смок +`demo/smoke_summary_first_paint.mjs`, четыре записи в `scripts/mutation-gate.mjs` +и оба changelog. Модель данных, конфигурация и i18n не менялись — подтверждено +чтением диффа (`git diff origin/dev...HEAD --stat`): изменений в `src/i18n/**` +или в схеме конфигурации нет. + +Контракт задан ТЗ (issue #509, полный трек, зелёный спек-ревью r2, +`docs/reviews/SPEC-REVIEW-509-r2.md`): AC1–AC9 (AC7 — оба changelog). + +## Как проверялось + +Прочитан весь диф построчно (`git diff origin/dev...HEAD -- src/... test/... +demo/smoke_summary_first_paint.mjs scripts/mutation-gate.mjs docs/CHANGELOG*`), +сверены сигнатуры `wallBodiesGeometry` / `multiWallNodesForGeometry` / +`innerContourForRoom` (`src/wall-thickness.ts:2434,2750,3662`) с новым вызовом +`spaceWallGeometry` — аргументы и их порядок совпадают со схемой, которой уже +пользуется карточка (`_innerContour`). + +Прослежена машина состояний `valueState()` / `scheduleMetrics()` / +`advanceMetrics()` / `computeDeviceCount()` вручную по всем ветвям: модуль не +загружен → `pending`; entity-источник резолвится синхронно, минуя агрегаты; +`device_count`/`total_area` без мемо → `pending` + планирование; с устаревшим +мемо → `ready` со старым числом + фоновый пересчёт (AC9); `datetime` не +зависит от агрегатов вовсе. Прослежен `resetLifecycle()`/`current(generation)` +на предмет использования устаревшего таймера/генератора после отключения +панели — `run()` первым делом обнуляет `metricsRefresh`, затем проверяет +`current(generation)` и выходит без побочных эффектов; утечки не нашёл. + +Дисциплина «тест умеет падать» проверена не на слово: каждый из четырёх новых +мутантов прогнан ЛОКАЛЬНО в этой рабочей копии командой `node +scripts/mutation-gate.mjs --id=` — все четыре поймали свою поломку +(«поймано 1 из 1»): + +- `summary-first-paint-shows-unavailable` (AC1, guard — смок) +- `summary-metrics-block-first-frame` (AC4, guard — смок) +- `summary-area-recomputes-walls-per-room` (AC3, guard — юнит) +- `summary-stale-metric-falls-back-to-skeleton` (AC9, guard — юнит) + +Дополнительно эти же четыре мутанта независимо подтверждены в CI на этом же +SHA: два — прямым прогоном dispatch-джобов `Мутанты по диффу (1/3)` и `(3/3)` +(runs/34457268966), два — валидным переиспользованием ledger-кеша с прогона +`(2/3)` того же диспатча, отпечаток входов (файл патча + файл гарда) не +менялся между `172d9d5a` и `9b00002a` (единственная правка между ними — +`demo/smoke_summary_first_paint.mjs`, к мутантам шарда 2 отношения не имеющая +— проверено `git diff 172d9d5a..9b00002a --stat`). У самого мутанта +`summary-stale-metric-falls-back-to-skeleton` найден «поймано» ещё и на +исходном красном прогоне `34456100054` (шард 2 там был зелёным сам по себе, +красным был только шард 1/3 из-за флейка в смоке, не в мутанте) — см. +job `102802793874`. + +## Что проверено и корректно + +- **AC1/AC2** — `valueState()` возвращает `pending` пока `metricsModule` не + загружен (для ЛЮБОГО источника, включая entity), и не путает это с + `unavailable`; для реально отсутствующей сущности — честный `unavailable`. + Юнит `#509 AC1/AC2/AC9` (`test/summary-panel-runtime.test.mjs`) гоняет обе + ветки, включая живую и мёртвую сущность. Локально: `node --test + test/summary-panel-runtime.test.mjs` — 5/5 ok. +- **AC3** — `spaceWallGeometry` считает `wallBodiesGeometry` + + `multiWallNodesForGeometry` один раз на пространство и передаёт в + `innerContourForRoom` как `sharedRoomWallGeometry`/`sharedMultiWallNodes`; + сигнатуры совпадают построчно с уже принятым способом, которым карточка + передаёт `_innerContour`. Юнит со счётчиком вызовов подтверждает «один + вызов на пространство, не на комнату» и что результат не меняется + (`4.32` что со spy, что без); второй юнит подтверждает ускорение на + фикстуре `large-house` (порог 2500 мс, грубый, не флейкует). Оба зелёные + локально. +- **AC4** — тяжёлый агрегат больше не считается внутри `renderPanel()`: + `valueState()` только читает мемо и планирует фоновый пересчёт + (`scheduleMetrics()`), сам расчёт стартует через + `setTimeout(0)→rAF→setTimeout(0)`, как записано в «Принятых технических + предположениях» ТЗ. Мутант, возвращающий синхронный расчёт в путь + рендера (`summary-metrics-block-first-frame`), ловится смоком. Локально + пересобранный бандл + `node demo/smoke_summary_first_paint.mjs`: первый + кадр не заблокирован, 3 строки со скелетами, ни одного «unavailable». +- **AC5** — анимация входа/выхода (190 мс, #505) реально проигрывается, + потому что главный поток свободен к моменту показа; смок читает + `getAnimations()` на обоих переходах. Проверено локально — `enterAnimated` + и `exitAnimated` подтверждены смоком (полный вывод: `OK`). +- **AC6** — `@media (prefers-reduced-motion: reduce)` глушит и пульсацию + скелета, и fade-in значения, прямоугольник остаётся. Смок проверяет + вычисленный `animationName === 'none'` и видимость плашки — оба true в + локальном прогоне. +- **AC8** — расчёт стартует сразу после показанного кадра (не отложен до + простоя), скелет сменяется значением за разумное время: локально + `значения через 2822 мс` (порог смока 6000 мс; машина ревьюера медленнее + CI-раннера автора, где было ~1555 мс, — оба варианта далеко от порога). +- **AC9** — устаревшее мемо не сбрасывается в скелет: `valueState()` + возвращает `ready` со старым числом и планирует пересчёт в фоне, пока + `metricsFresh()` не станет `true`. Юнит меняет `host._cfgEpoch` и + проверяет ровно эту последовательность; мутант, убирающий проверку + свежести, ловится. +- **AC7** — оба changelog правлены в этом же диффе, синхронно по смыслу + (en/ru), со ссылкой на #509; новых i18n-ключей нет (проверено — diff не + трогает `src/i18n/**`); `docs/images/screenshots.json` меняет только + `sourceFingerprint`/`sourceSha256`, все `imageSha256` — те же самые, то + есть кадры документации побитово не изменились (`docs:accept --identical`, + подтверждено логом job `docs` в CI на этом SHA). +- **Одно число — один источник.** `totalCleanFloorAreaM2` остаётся + единственной функцией, вычисляющей площадь для панели; никто другой в + `src/**` её не вызывает (проверено `grep` по собранному бандлу и + исходникам) — карточка и PDF используют отдельный путь через + `_innerContour`, что и раньше, и ТЗ явно не заявляет это в скоуп. Новый + путь (генератор) не создаёт второго источника числа — `totalCleanFloorAreaM2` + остался тонкой синхронной обёрткой над тем же генератором, который + использует и панель. +- Типы: `npx tsc --noEmit` (через `npm run build`) — зелёный локально; + `npm run bundle:sync` пересобрал бандл и синхронизировал три копии без + расхождений. + +## Гейты: что прогнал и почему + +**Зелёный Validate уже на этом SHA (`9b00002a`), переиспользован без +повторного прогона:** +- push-прогон `34457195661` — предполётные проверки (докс/провенанс/процесс). +- push-прогон на предыдущем коммите `172d9d5a` (`34455999479`, тот же диф по + продукту, следующий коммит менял только смок) — job «Фронтенд: типы, + юниты, мутанты, синхрон бандла» зелёный целиком: `npm run typecheck`, + `no-new-any`, `npm test` (полный набор), `npm run build`, сверка + бандл-деревьев, `bundle:budget`. +- dispatch-прогон с мутантами `34457268966` — все три шарда `Мутанты по + диффу` зелёные на точном `9b00002a`, включая чистый прогон + `node demo/smoke_summary_first_paint.mjs`, `node --test + test/summary-panel.test.mjs`, `node --test test/summary-panel-runtime.test.mjs`. + +**Прогнал сам, локально, на этой же рабочей копии** (Validate не покрывает +браузерные смоки для обычного пуша, а «работает ли оно» — моя ответственность): +- `npm run build` — зелёный (typecheck включён). +- `npm run bundle:sync` — зелёный, три копии бандла синхронны. +- `npx tsc -p tsconfig.test.json && node scripts/fix-test-build.mjs && node + --test test/summary-panel-runtime.test.mjs test/summary-panel.test.mjs` — + 25 + 5 тестов, все ok (в т.ч. новые `#509 AC1/AC2/AC9` и `#509 AC3` × 2). +- `node demo/smoke_summary_first_paint.mjs` — `OK` (все 14 проверок). +- `node demo/smoke_summary_panel.mjs` — `OK` (прямое совпадение по + `_config/_layoutRev/_model`, `scripts/smoke-select.mjs` тоже его называет). +- `node demo/smoke_summary_panel_polish.mjs`, `node + demo/smoke_summary_warm_attach.mjs` — `OK` (заявлены автором как + прогнанные; перепроверил сам вместо того, чтобы поверить на слово). +- Каждый из 4 новых мутантов по отдельности: `node scripts/mutation-gate.mjs + --id=` — все 4 «поймано 1 из 1» (список выше). + +**Не прогонял и почему:** +- `npm test` целиком у себя — не требуется: уже зелёный в CI на диффе, + который отличается от `HEAD` только смоком (не входит в `npm test`); + прогнал точечно изменённые файлы вместо этого. +- `npm run golden:verify` — в `demo/golden/**` нет ни одного сценария со + сводной панелью (`grep -rl summary demo/golden/` — пусто); диф не может + изменить ни один принятый кадр. AC7 отдельно ожидает «не ожидается» + изменения golden. +- `npm run invariants` — диф не трогает геометрическую модель (рёбра, + записи толщины, `layout`, `marker.space`, `open_spans`): он лишь + переиспользует уже существующие геометрические функции, не меняя, что + они возвращают (доказано юнитом AC3 — результат идентичен). Инвариантов + ребра/толщины это не касается. +- `python -m pytest tests_backend` — диф не трогает + `custom_components/**/*.py`. +- `smoke_summary_dialog_scroll.mjs` (#508) и остальные «прямые совпадения» + из `smoke-select` (`smoke_junction_holes`, `smoke_glow_fail_dark`, + `smoke_wallthick_standalone` и т.п.) — все они совпали по предельно общим + символам (`cellCm`, `NORM_W`, `GRID_PITCH`, `wallBodiesGeometry` как имя + функции, не как поведение) и не имеют отношения к загрузочному состоянию + панели или к переносу расчёта во времени — единственному, что меняет этот + диф в геометрическом коде. Посмотрел список, прогонять не стал. +- performance-профили — не названы в AC; AC4/AC8 доказываются собственным + смоком задачи. + +## Находки + +**Low-1 (не блокирует, снимаю с записью).** Комментарий в +`scheduleMetrics()` (`src/summary-panel-runtime-loaded.ts:~735`): «Каждая +порция короче кадра, между порциями браузер обрабатывает ввод и рисует» — +неточен для одного случая. Переход генератора `cleanFloorAreaSteps` между +двумя ПРОСТРАНСТВАМИ выполняет `spaceWallGeometry` (единый polyclip-union) +целиком внутри одного вызова `.next()`, без промежуточного `yield` — это +именно тот «неделимый» кусок, из-за которого автор в этом же заходе поднял +порог смока `noLongTaskWhileComputing` с 250 до 450 мс (коммит `9b00002a`, +сам объясняет это в сообщении коммита и в комментарии смока). Итог по +существу верный (AC4/AC8 выполняются, мутант «весь расчёт целиком» +по-прежнему падает — перепроверено локально), но приведённая фраза +обещает больше, чем код гарантирует: разработчик, который прочитает только +её, ожидает, что ни один кусок не превышает ~16 мс, а по факту до трёх +кусков по дому могут занять до ~450 мс каждый. Прошу поправить формулировку +в следующей правке этого файла (не отдельным заходом ради одной фразы) — +не блокирует, продуктовое поведение проверено и корректно. + +## Итог + +High: 0. Medium: 0 (обе Medium-находки спек-ревью были закрыты AC8/AC9 ещё +на этапе ТЗ и подтверждены здесь тестами, которые умеют падать). Low: 1, +снята с записью выше. + +Все девять AC (AC1–AC9) доказаны либо тестом, который я лично проверил на +способность падать (мутационным гейтом или прогоном красной/зелёной ветки), +либо прямым чтением с перепроверкой сигнатур. Оба changelog на месте, +i18n не тронут, модель данных не тронута, «одно число — один источник» +выполняется. Возвращать в скоуп нечего. + +**Вердикт: зелёный.** + +--- + + + +## Материал раунда + +- Ветка: `issue/509-summary-first-paint`, коммит `9b00002a5b09` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `017ac1f24095d4f7009510c19a82a9672984f656` + ``` + git log --all --format='%H %T' | grep 017ac1f24095 + ``` +- Тело issue: `6224b491f7bef6e6962692c3c901bf5121432fd1e0244740514a420aff96ba36` +- Вердикт конвейера: `green` · High 0