From b3df25ab8583c325bd97d16a6f0234ab9b13e560 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Fri, 28 Aug 2026 13:33:25 +0000 Subject: [PATCH] docs: review document for #317 Issue: #317 User-Visible: no --- docs/reviews/SPEC-REVIEW-317-r1.md | 182 +++++++++++++++++++++++++++++ 1 file changed, 182 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-317-r1.md diff --git a/docs/reviews/SPEC-REVIEW-317-r1.md b/docs/reviews/SPEC-REVIEW-317-r1.md new file mode 100644 index 00000000..d6fe6f56 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-317-r1.md @@ -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=` — + механизм 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=` + — он ещё не существует, ТЗ лишь называет обязательство его завести. + +## Вердикт + +ТЗ технически обосновано, каждое фактическое утверждение о текущем коде и +существующей инфраструктуре подтверждено чтением дерева, единственный продуктовый +вопрос уже решён владельцем, все обязательные разделы §7.1 на месте, критерии +приёмки пронумерованы и проверяемы. Blocking-находок нет, Medium нет ни в скоупе, +ни вне скоупа. Задача может переходить в «Готово к разработке». + +Вердикт: зелёный · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 0 · Документ: docs/reviews/SPEC-REVIEW-317-r1.md