mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
docs: review document for #509
Проверка (CI) / Предполётные проверки: документация, провенанс, процесс (push) Failing after 51s
Проверка (CI) / Классификация изменённых файлов (push) Successful in 37s
Проверка (CI) / Мутанты по диффу (1/3): затронутые свидетели краснеют (push) Skipped
Проверка (CI) / Мутанты по диффу (2/3): затронутые свидетели краснеют (push) Skipped
Проверка (CI) / Мутанты по диффу (3/3): затронутые свидетели краснеют (push) Skipped
Проверка (CI) / HACS: валидация репозитория (push) Failing after 55s
Проверка (CI) / Переиспользование: это дерево уже проверено (push) Successful in 1m29s
Проверка (CI) / Hassfest: манифест интеграции (push) Failing after 1m3s
Проверка (CI) / Бэкенд: pytest в Home Assistant (push) Failing after 8m21s
Проверка (CI) / Фронтенд: типы, юниты, мутанты, синхрон бандла (push) Failing after 10m42s
Проверка (CI) / Смоки в браузере (шард 1 из 3) (push) Skipped
Проверка (CI) / Смоки в браузере (шард 2 из 3) (push) Skipped
Проверка (CI) / Смоки в браузере (шард 3 из 3) (push) Skipped
Проверка (CI) / Смоки: все шарды зелёные (push) Skipped
Проверка (CI) / Golden-кадры против принятых эталонов (push) Skipped
Проверка (CI) / Перф-смок: бюджет времени кадра (push) Skipped
Проверка (CI) / Предполётные проверки: документация, провенанс, процесс (push) Failing after 51s
Проверка (CI) / Классификация изменённых файлов (push) Successful in 37s
Проверка (CI) / Мутанты по диффу (1/3): затронутые свидетели краснеют (push) Skipped
Проверка (CI) / Мутанты по диффу (2/3): затронутые свидетели краснеют (push) Skipped
Проверка (CI) / Мутанты по диффу (3/3): затронутые свидетели краснеют (push) Skipped
Проверка (CI) / HACS: валидация репозитория (push) Failing after 55s
Проверка (CI) / Переиспользование: это дерево уже проверено (push) Successful in 1m29s
Проверка (CI) / Hassfest: манифест интеграции (push) Failing after 1m3s
Проверка (CI) / Бэкенд: pytest в Home Assistant (push) Failing after 8m21s
Проверка (CI) / Фронтенд: типы, юниты, мутанты, синхрон бандла (push) Failing after 10m42s
Проверка (CI) / Смоки в браузере (шард 1 из 3) (push) Skipped
Проверка (CI) / Смоки в браузере (шард 2 из 3) (push) Skipped
Проверка (CI) / Смоки в браузере (шард 3 из 3) (push) Skipped
Проверка (CI) / Смоки: все шарды зелёные (push) Skipped
Проверка (CI) / Golden-кадры против принятых эталонов (push) Skipped
Проверка (CI) / Перф-смок: бюджет времени кадра (push) Skipped
Issue: #509 User-Visible: no
This commit is contained in:
@@ -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=<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 не тронут, модель данных не тронута, «одно число — один источник»
|
||||
выполняется. Возвращать в скоуп нечего.
|
||||
|
||||
**Вердикт: зелёный.**
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/509-summary-first-paint`, коммит `9b00002a5b09` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `017ac1f24095d4f7009510c19a82a9672984f656`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 017ac1f24095
|
||||
```
|
||||
- Тело issue: `6224b491f7bef6e6962692c3c901bf5121432fd1e0244740514a420aff96ba36`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user