houseplan-card/docs/reviews/CODE-REVIEW-509-r1.md

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=<mutant> — все четыре поймали свою поломку («поймано 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=<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