mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-02 12:49:56 +00:00
committed by
Sergey Matyunin
parent
fd364c57a8
commit
5c9e79081b
@@ -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.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `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
|
||||
```
|
||||
Reference in New Issue
Block a user