diff --git a/docs/reviews/SPEC-REVIEW-490-r1.md b/docs/reviews/SPEC-REVIEW-490-r1.md new file mode 100644 index 00000000..0b9111b0 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-490-r1.md @@ -0,0 +1,168 @@ +# SPEC-REVIEW-490-r1 + +- **Issue:** #490 — «Атомарный recovery и live-состояние сводной панели» +- **Этап:** ТЗ на ревью (S4-spec-review), PROCESS.md §2.4 +- **Заход:** r1 · блокирующих циклов израсходовано 0 из 4 +- **Артефакт ТЗ:** `docs/specs/490-summary-recovery-live-state.md`, + коммит `95a34f730a572c06c732e1e029b2c8dc719428b9`, + ветка `issue/490-summary-recovery-live-state` +- **Трек:** полный (не `small`) — обоснование в S2-комментарии проверено и + корректно: больше одной поверхности (save-контракт панели + render + dependency projection в `houseplan-card.ts`), multi-client + concurrency/state contract, влияние на высокочастотный render scheduling + +## Скоуп + +Issue заведён по прямой команде владельца из независимого аудита +(2026-09-08, §11 п.1/12) по уже опубликованному в `dev` (но не +зарелиженному) #437. Два подтверждённых дефекта: + +- **F1** — `saveDialog()` в lost-ACK ветке принимает только `rev` из + повторного `config/get`, но затем безусловно перезаписывает + `_serverCfg` старым `candidate`, поэтому параллельная правка другого + клиента к несвязанному полю документа теряется при следующей записи; +- **F2** — резолвер зависимостей рендера (`_captureRenderDeviceSnapshot`) + не включает entity-источники `summary_panel.blocks[].values[]`, поэтому + summary-only датчик не обновляется на обычном HA-tick. + +Это соответствует `docs/SCOPE.md` J1 (live-обзор), J2, J6 (multi-client +sync, optimistic locking) — ТЗ не расширяет функциональность #437, а чинит +регресс уже принятого контракта. + +## Как проверялось + +Ревью читал ТЗ и сверял каждое заявленное поведение с: + +- телом issue #490 и обоими комментариями (аналитика S2, готовность ТЗ S3); +- исходным ТЗ #437 (`docs/specs/437-summary-panel.md`, §7.1, §7.4, §9.1, + §9.2) — контракт, который #490 исправляет, а не переопределяет; +- фактическим кодом на SHA `95a34f73` (ветка задачи): `src/summary-panel-runtime-loaded.ts` + (`saveDialog()`), `src/houseplan-card.ts` + (`_captureRenderDeviceSnapshot`, `_cfgEpoch`/`willUpdate`, + `_setAreaLifecycleConfig`), `custom_components/houseplan/validation.py` + (`SUMMARY_PANEL_SCHEMA` — `max=10` блоков × `max=20` значений); +- существующими тестами, которые ТЗ обещает расширить: + `test/summary-panel.test.mjs`, `test/render-device-snapshot.test.mjs`, + `test/render-invalidation.test.mjs`, `demo/smoke_summary_panel.mjs` — все + существуют, план тестирования не ссылается на несуществующие файлы. + +Продуктовый код не менялся и не оценивался на корректность реализации — +это код-ревью следующего этапа. Проверялось только то, что ТЗ описывает +проверяемый и технически достижимый контракт. + +### Проверка root cause (F1) + +`saveDialog()` (`src/summary-panel-runtime-loaded.ts`, ветка `catch +(writeError)`): при совпадении `saved`/`draft` принимает +`this.host._cfgRev = authoritative.rev`, но `this.host._serverCfg = +candidate` (старый локальный кандидат) стоит **вне** `catch`-блока и +выполняется безусловно после любого исхода `try`. Соответствует +описанию §3 п.1 ТЗ дословно. + +### Проверка root cause (F2) + +`entityIds` в `_captureRenderDeviceSnapshot` собирается из +`room.settings.temp_source/hum_source`, `openingEntityReferences`, +decor-текстов и `_vacFit.source` — обхода `settings.summary_panel` нет +нигде в файле (`grep summary_panel src/houseplan-card.ts` — 0 +совпадений). Соответствует §3 п.2 ТЗ. + +### Проверка «200 values» (§14, принятое предположение №2) + +Не голословно: backend-схема `SUMMARY_PANEL_SCHEMA` ограничивает `blocks` +`vol.Length(max=10)`, `values` внутри блока — `vol.Length(max=20)`, +итог 10×20=200 — число в ТЗ точное, не придуманное. + +### Проверка терминологии AC5 (identity/epoch) + +`_cfgEpoch` — реальный существующий механизм (`houseplan-card.ts`, +`willUpdate`, комментарий `DEV-B701-01`), уже используемый именно как +сигнал «геометрию нужно пересчитать» отдельно от `_cfgRev` +(concurrency-номер). Существует прецедент атомарной подмены `_serverCfg` +без бампа эпохи (`_setAreaLifecycleConfig`) — доказывает, что контракт +§6.2 «принять весь authoritative config через общий adoption seam» и +§7.2 «relevant tick не меняет geometry epoch» технически реализуем в +существующей архитектуре, а не выдуман. + +### Проверка словаря AC5 (relevant/irrelevant tick) + +`test/render-invalidation.test.mjs` уже использует ровно эти термины +(`an unrelated HA state row does not invalidate...`, `a dependency row +identity or presence change invalidates state...`) — ТЗ говорит на языке +существующего теста, план тестирования реалистичен. + +## Находки + +Нет High. Нет Medium (ни в скоупе, ни вне скоупа). Нет Low. + +## Что проверено и корректно + +- Обязательные разделы §7.1 присутствуют по существу: сценарий (§1), + до/после (§2), проблема как «подтверждённые причины» (§3), скоуп (§4), + не-скоуп (§5), контракт поведения (§6–7), совместимость/данные/i18n + (§8, явно «миграции нет»), AC1–AC6 с доказательством (§9), план + автотестов (§10), риски (§11), откат (§12), release-артефакты (§13). +- Каждый AC — проверяемое утверждение с указанным способом доказательства + (unit/integration/smoke), не общая фраза; AC3 и негативная матрица явно + отделяют «настоящий конфликт» от «потерянного ACK», что закрывает + главный риск регрессии #102-типа (правка по одному AC не должна ломать + соседний). +- Не-скоуп (§5) корректно исключает #493 (picker/scale/mobile/lifecycle) и + не переопределяет backend/API/схему — F1/F2 действительно локальны к + save-контракту и render-dependency-проекции. +- Раздел «Принятые предположения» (§14) размечен явно и корректно: все + четыре пункта — технические решения (детектирование lost ACK, bounded + dependency set, семантика unavailable/missing, отсутствие нового + UX-контракта), ни один не маскирует продуктовый вопрос под факт. + Проверка по существу (см. «200 values» выше) показала, что предположения + не являются догадками, а выводимы из уже существующего кода/схемы. +- Продуктовых вопросов владельцу нет и не должно быть: ожидаемое + поведение уже зафиксировано контрактом #437 (§7.1, §7.4, §9.1) — + задача восстанавливает, а не придумывает поведение. Никакого + технического вопроса, ошибочно адресованного владельцу, не найдено. +- Откат (§12) специфичен для этой задачи: явно запрещает частичный + rollback одной из двух половин фикса, если тесты по-прежнему обещают + общий контракт — не шаблонная фраза. +- Release-артефакты (§13) корректно называют «golden не требуется» с + обоснованием (нет визуальных изменений) вместо молчаливого пропуска. + +## Чего не проверял + +- Не проверялась реализация — код по этой задаче ещё не написан + (issue в `S4-spec-review`, не `S6`/`S7`). Проверка «работает ли + реализация» — предмет код-ревью следующего этапа. +- Не запускались гейты (`typecheck`/`test`/`build`) — на этапе ТЗ они не + относятся к предмету ревью (нет изменений в `src/**`/`test/**` этой + задачи; коммит `95a34f73` — только `docs/specs/**`, класс C). +- Не проверялась производительность существующего `render-invalidation` + пути вне контекста этой задачи — вне скоупа. + +## Материал раунда + +- SHA: `95a34f730a572c06c732e1e029b2c8dc719428b9` + (ветка `issue/490-summary-recovery-live-state`) +- ТЗ: `docs/specs/490-summary-recovery-live-state.md` на этом SHA + +## Вердикт + +**Зелёный.** ТЗ технически обоснованно, каждое заявленное расхождение с +контрактом #437 подтверждено чтением кода на указанных строках, все AC +проверяемы и снабжены способом доказательства, план тестов ссылается на +существующие файлы, предположения размечены и не подменяют продуктовые +решения. High: 0, Medium: 0. + +--- + + + +## Материал раунда + +- Ветка: `issue/490-summary-recovery-live-state`, коммит `95a34f730a57` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `17238622e9af7346270d88a5100e8b6260789ca6` + ``` + git log --all --format='%H %T' | grep 17238622e9af + ``` +- ТЗ `docs/specs/490-summary-recovery-live-state.md`, блоб `d4bfd4ea63f1b2ce1cb255b2764fe14d25661df7` + ``` + git log --all --find-object=d4bfd4ea63f1b2ce1cb255b2764fe14d25661df7 -- docs/specs/490-summary-recovery-live-state.md + ```