mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -0,0 +1,182 @@
|
||||
# SPEC-REVIEW-317-r1
|
||||
|
||||
- Issue: [#317](https://github.com/Matysh/houseplan-card/issues/317) — «Метрики под названием комнаты и тултип комнаты не показывают температуру и влажность, хотя датчики в комнате есть»
|
||||
- Этап: spec (S4-spec-review)
|
||||
- Заход: r1 · блокирующих циклов израсходовано 0 из 4
|
||||
- Трек: полный (аналитик сам назвал критерий, который задача не проходит: «изменение затрагивает единый контракт данных сразу для подписи комнаты, тултипа и температурной заливки»)
|
||||
- Артефакт ТЗ: `docs/specs/317-room-climate-placement.md` на SHA `f19ed10f` (ветка `issue/317-room-climate-placement`)
|
||||
- Ревьюер: Claude (сессия ревью ТЗ), состязательно, без пояснений автора
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Issue #317 — баг: `_roomTemp`/`_roomHum`, общие для подписи комнаты, tooltip и
|
||||
температурной заливки, читают климат только из `areaClimateMap()`, которая
|
||||
группирует сенсоры исключительно по HA registry `area_id`. Ручное размещение
|
||||
маркера датчика в комнате House Plan (`marker.area`/`marker.room_id`) в
|
||||
климатический агрегат не попадает, а у комнаты без HA Area автоматического
|
||||
среднего нет вовсе. Владелец уже принял продуктовое решение по единственному
|
||||
открытому вопросу (комнаты без HA Area — да, правило распространяется), поэтому
|
||||
на входе в ревью открытых продуктовых вопросов не осталось.
|
||||
|
||||
Разбирался только этот документ: файла предыдущего раунда нет, дельта не
|
||||
считается (§2.10 к первому заходу не применяется).
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Ревью читает ТЗ состязательно: каждое утверждение о текущем поведении и о
|
||||
существующей инфраструктуре сверено с кодом на дереве этого SHA, а не принято
|
||||
на слово автора.
|
||||
|
||||
1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` — трек, разделы §7.1,
|
||||
лимит циклов и формат вердикта.
|
||||
2. Прочитаны тело issue #317 и все комментарии, включая явное решение
|
||||
владельца («да, распространяем автоматическое правило также на комнаты без
|
||||
HA Area», 2026-08-28).
|
||||
3. Прочитан `docs/specs/317-room-climate-placement.md` целиком.
|
||||
4. Диагноз ТЗ сверен построчно с кодом:
|
||||
- `src/houseplan-card.ts:11956-11968` (`_roomTemp`/`_roomHum`, gate `r.area
|
||||
? this._climate().get(r.area) : null`) — подтверждает §3;
|
||||
- `src/houseplan-card.ts:12093` (внешний gate `_renderRoomLabel`, блокирует
|
||||
метрики area-less комнаты без `temp_source`/`hum_source`/`labelLight`) —
|
||||
подтверждает §3 и требование §8;
|
||||
- `src/houseplan-card.ts:11058-11068` (tooltip вызывает `_roomTemp`/`_roomHum`
|
||||
без gate) — подтверждает диагноз «дефект воспроизводится сразу на трёх
|
||||
поверхностях»;
|
||||
- `src/houseplan-card.ts:8502` (`fill_mode: temp` читает тот же `_roomTemp`) —
|
||||
подтверждает единственность резолвера;
|
||||
- `src/devices.ts:1394-1478` (`areaClimateMap`) — подтверждает группировку
|
||||
только по `reg.area_id || dev?.area_id`, существующий tombstone/`use_climate_temp`
|
||||
механизм, `NON_AIR_RE`, `entity_category` gate — контракт §6.3 списывает их
|
||||
как «сохраняются», а не выдумывает заново;
|
||||
- `src/devices.ts:1056-1076` (`resolveExplicitMarkerPlacement`) и
|
||||
`src/devices.ts:350-357` (`lightSourceBelongsToRoom`) — подтверждают, что
|
||||
приоритет «явный marker → `room_id`+`area:null` → registry» уже существует
|
||||
как паттерн для визуального размещения и света; ТЗ не изобретает новый
|
||||
контракт, а переносит существующий на климат;
|
||||
- `src/logic.ts:2016-2030` (`parseRoomRef`) — подтверждает формат
|
||||
`space#area` / `space#@roomId`, на который ссылается §9;
|
||||
- `docs/ARCHITECTURE.md:1148-1153` — текущее описание one-pass climate
|
||||
совпадает с диагнозом дословно;
|
||||
- `docs/USER-GUIDE.ru.md:468` и `:1194` — подтверждают, что документация уже
|
||||
фиксирует нынешнее (дефектное) поведение и уже содержит прецедент
|
||||
«`room_id` точнее HA-зоны» для света, на который ТЗ ссылается по аналогии.
|
||||
5. Проверено существование целевой тестовой/гейт-инфраструктуры, названной в
|
||||
критериях приёмки, а не выдуманной:
|
||||
- `test/devices.test.mjs` уже содержит матрицу `areaClimateMap` (one-pass,
|
||||
tombstone, `use_climate_temp`) — AC1 расширяет реальный файл;
|
||||
- `demo/smoke_climate_once.mjs` существует и уже фиксирует «44/60 rooms» —
|
||||
число в AC9 не с потолка;
|
||||
- `demo/smoke_climate_temp.mjs` существует — задел для AC7;
|
||||
- `scripts/mutation-gate.mjs` существует и поддерживает `--id=<mutant>` —
|
||||
механизм AC10 реален;
|
||||
- `docs/specs/262-readd-child-entity-after-device-delete.md` действительно
|
||||
фиксирует binding-scoped tombstone contract, на который ссылается §6.2.
|
||||
6. Проверены обязательные разделы ТЗ по §7.1 — все на месте (см. таблицу ниже).
|
||||
7. Проверена трассируемость: issue ↔ ТЗ ссылки в обе стороны, `docs/specs/README.md`
|
||||
обновлён той же строкой, ветка/трейлеры коммита (`Issue: #317`,
|
||||
`User-Visible: no`) корректны для чисто документационного коммита.
|
||||
|
||||
Гейты (typecheck/test/build) на этом шаге не прогонялись и не должны: стадия —
|
||||
ревью ТЗ, продуктовый код не менялся (коммит `f19ed10f` — только
|
||||
`docs/specs/**`), unit/build ничего нового не проверили бы. Это соответствует
|
||||
«гейты соразмерны задаче»: полный набор гейтов относится к код-ревью, не к
|
||||
ревью ТЗ.
|
||||
|
||||
## Обязательные разделы §7.1
|
||||
|
||||
| Раздел | Есть | Где |
|
||||
|---|---|---|
|
||||
| Сценарий (персона, поверхность, момент) | ✅ | §1, ссылается на J1/J5 `SCOPE.md` |
|
||||
| Что человек увидит до/после | ✅ | §2, без терминов реализации |
|
||||
| Проблема | ✅ | §3 (подтверждённый диагноз) |
|
||||
| Скоуп и не-скоуп | ✅ | §5 |
|
||||
| Контракт поведения | ✅ | §§6-7 |
|
||||
| UX | ✅ | §8 |
|
||||
| Модель данных и миграция | ✅ | §9 (миграции нет, обосновано) |
|
||||
| i18n | ✅ | §11 (новых строк нет, обосновано) |
|
||||
| AC1…ACn с доказательством | ✅ | §12, для каждого назван unit/smoke/mutation/review |
|
||||
| План автотестов | ✅ | §13 |
|
||||
| Риски | ✅ | §14 |
|
||||
| Откат | ✅ | §16 |
|
||||
| Release-артефакты | ✅ | §16 |
|
||||
|
||||
Плюс дополнительные разделы сверх минимума (архитектура/perf §10, security/privacy
|
||||
§15, явный блок принятых технических предположений §17) — это хорошо
|
||||
документированное ТЗ для полного трека с реальным риском перфоманса.
|
||||
|
||||
## Находки
|
||||
|
||||
Blocking (High): нет.
|
||||
В скоупе (Medium): нет.
|
||||
Вне скоупа (Medium → issue): нет.
|
||||
|
||||
### Low (снято ревьюером с записью, правки не требуют)
|
||||
|
||||
1. **Терминология HA Area/HA-зона в плане документации.** ТЗ систематически
|
||||
использует «HA Area», тогда как `docs/USER-GUIDE.ru.md` в основном пишет
|
||||
«HA-зона» и лишь один раз (строка 1194) — «HA Area». Несогласованность уже
|
||||
существует в каноне до этого ТЗ (не внесена им), новых UI-строк ТЗ не
|
||||
добавляет, и §11 прямо поручает автору синхронизировать формулировку при
|
||||
правке `USER-GUIDE.ru.md` в реализации. Снимаю без правки ТЗ — это
|
||||
редакторская забота реализации, не пробел контракта.
|
||||
2. **AC8 называет доказательством «diff audit», а не один из канонических
|
||||
`unit/backend/smoke/golden`.** По содержанию это ревью кода («проверено
|
||||
чтением, не исполнением»): совпадение snapshot/resolver между View и hosted
|
||||
Static и отсутствие мутации config естественно проверяются чтением диффа, а
|
||||
не отдельным раннером. Не считаю это находкой, требующей правки текста ТЗ —
|
||||
отмечаю для код-ревьюера следующего этапа: при проверке AC8 явно указать,
|
||||
что это чтение, а не автотест.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Диагноз в §3 воспроизведён по коду один в один, включая три точки отказа
|
||||
(label gate, `_roomTemp`/`_roomHum`, `areaClimateMap`) — не разошёлся ни в
|
||||
одной детали с деревом на SHA `f19ed10f`.
|
||||
- Решение владельца по единственному продуктовому вопросу зафиксировано в
|
||||
issue и корректно перенесено в §4 ТЗ дословно по смыслу.
|
||||
- Контракт приоритета источников (§6.2, §7) не изобретает новый паттерн, а
|
||||
переносит уже существующий в проекте прецедент точного/родительского marker
|
||||
(`resolveExplicitMarkerPlacement`, `lightSourceBelongsToRoom`) на климат —
|
||||
ни одно утверждение о поведении не помечено как факт без опоры на код или
|
||||
канон, а там, где авторы вводят технические решения, они явно вынесены в §17
|
||||
«принято предположительно, поменять свободно» и открыты для оспаривания.
|
||||
Единственный продуктовый вопрос (комнаты без HA Area) был задан владельцу
|
||||
именно как продуктовый вопрос — не технический вопрос, замаскированный под
|
||||
продуктовый.
|
||||
- Каждый AC (1–11) однозначен и указывает способ доказательства; для AC1, AC7,
|
||||
AC9, AC10 целевые файлы/скрипты уже существуют в дереве, значит критерии не
|
||||
ссылаются на несуществующую инфраструктуру.
|
||||
- Не-скоуп (§5) явно исключает эвристику air-sensor, миграцию, новый UI,
|
||||
`use_climate_temp` semantics кроме placement — граница с соседними задачами
|
||||
проведена, а не подразумевается.
|
||||
- Откат, release-артефакты, security/privacy покрыты и согласуются с
|
||||
«никогда не удалять файл пользователя на догадке» и «одно число — один
|
||||
источник» (§6, §8: label/tooltip/fill читают один и тот же `_roomTemp/_roomHum`,
|
||||
что и есть образцовое соблюдение принципа единственного источника числа).
|
||||
- Трейлеры коммита (`Issue: #317`, `User-Visible: no`) корректны для
|
||||
docs-only коммита; `docs/specs/README.md` обновлён той же строкой.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не прогонял `npm run typecheck`/`npm test`/`npm run build` — на этом шаге
|
||||
нет продуктового кода для проверки (коммит правит только `docs/specs/**`);
|
||||
прогон ничего не доказал бы и не входит в объём ревью ТЗ.
|
||||
- Не проверял поведение реального HA-инстанса и golden/browser smokes — они
|
||||
относятся к код-ревью следующего этапа, когда появится код и заявленные в
|
||||
§13 команды (`node demo/smoke_climate_once.mjs` и т.д.).
|
||||
- Не проверял `docs/ARCHITECTURE.md`, `docs/USER-GUIDE(.ru).md`,
|
||||
`docs/TESTING.md`, оба changelog на предмет уже внесённых правок — по плану
|
||||
реализации (§13, §11) они меняются в коде, а не в этом ТЗ-коммите, и их
|
||||
редактирование — предмет DoD код-ревью, а не спецификации.
|
||||
- Не проверял корректность будущего мутанта `--id=<new-room-climate-placement-mutant>`
|
||||
— он ещё не существует, ТЗ лишь называет обязательство его завести.
|
||||
|
||||
## Вердикт
|
||||
|
||||
ТЗ технически обосновано, каждое фактическое утверждение о текущем коде и
|
||||
существующей инфраструктуре подтверждено чтением дерева, единственный продуктовый
|
||||
вопрос уже решён владельцем, все обязательные разделы §7.1 на месте, критерии
|
||||
приёмки пронумерованы и проверяемы. Blocking-находок нет, Medium нет ни в скоупе,
|
||||
ни вне скоупа. Задача может переходить в «Готово к разработке».
|
||||
|
||||
Вердикт: зелёный · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 0 · Документ: docs/reviews/SPEC-REVIEW-317-r1.md
|
||||
Reference in New Issue
Block a user