mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-30 11:49:16 +00:00
@@ -0,0 +1,228 @@
|
||||
# SPEC-REVIEW-487-r1
|
||||
|
||||
- **Issue:** https://github.com/Matysh/houseplan-card/issues/487
|
||||
- **Этап:** ревью ТЗ (PROCESS.md §2.4)
|
||||
- **Заход:** r1 · блокирующих циклов израсходовано 0 из 4 (лимит лёгкого трека
|
||||
не применяется — трек полный)
|
||||
- **Артефакт ТЗ:** `docs/specs/487-room-temperature-thresholds.md`,
|
||||
зафиксирован коммитом `09328760` («docs: specify room temperature
|
||||
thresholds», `Issue: #487 · User-Visible: no`, только `docs/**` — класс C).
|
||||
- **Материал ревью:** тело issue #487, три комментария (аналитика S2, занятие
|
||||
автора, готовность ТЗ), файл ТЗ на SHA `09328760`.
|
||||
|
||||
## Скоуп
|
||||
|
||||
Комнате разрешается задать собственный диапазон комфортной температуры
|
||||
(`room.settings.temp_min/temp_max`), с независимым наследованием каждой
|
||||
границы от `space.settings.temp_min/temp_max` (умолчания 20/25 °C). Единственный
|
||||
потребитель — цвет температурной заливки пола комнаты (включая продолжение в
|
||||
откосах проёмов). Room-card, tooltip, badge, источник температуры и алгоритм
|
||||
«холодно/комфорт/жарко» не меняются. HA-helper-ссылки и глобальный уровень
|
||||
диапазона — явно вне скоупа по решению владельца от 07.09.2026, зафиксированному
|
||||
в теле issue.
|
||||
|
||||
Трек — полный, обоснованно: новые compatibility-поля конфигурации и новый
|
||||
UX-контракт наследования — два независимых критерия §5, каждого из которых
|
||||
достаточно, чтобы трек не был `small`. Аналитик (комментарий S2) назвал оба.
|
||||
Задача попадает в J5 «Room climate at a glance» из `docs/SCOPE.md` — фильтр
|
||||
пройден, нового вида функциональности вне списка Core user jobs нет.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Ревью читает ТЗ состязательно: без устных пояснений автора, с проверкой
|
||||
каждого фактического утверждения о текущем коде по самому коду — раздел 3 ТЗ
|
||||
(«Подтверждённое текущее состояние») сплошь состоит из проверяемых утверждений
|
||||
о конкретных функциях, и именно они подлежали проверке в первую очередь, а не
|
||||
принятию на слово.
|
||||
|
||||
Построчно сверено с деревом на `09328760` (HEAD этой ветки, код не менялся —
|
||||
это чисто документационный коммит):
|
||||
|
||||
| Утверждение ТЗ (§3) | Где проверено | Результат |
|
||||
|---|---|---|
|
||||
| `spaceDisplayOf()` проецирует `tempMin/tempMax` с default 20/25 | `src/logic.ts:1286-1327` | Совпадает дословно |
|
||||
| `roomFillStyle()` нормализует границы через `Math.min/Math.max` | `src/logic.ts:1481-1490` (внутри режима `temp`) | Совпадает |
|
||||
| `_resolvedRoomFills()` и `renderSpaceStatic()` передают всем комнатам одну пару пространства | `src/houseplan-card.ts:9276-9309` (`disp.tempMin, disp.tempMax` в цикле по `space.rooms`); `src/space-render.ts:497-505` (то же в `renderSpaceStatic`) | Совпадает — оба render path читают `disp.tempMin/tempMax`, не `room.settings` |
|
||||
| `RoomCfg.settings` уже хранит `fill_mode/custom_fill/glow/temp_source/hum_source/name_scale/label_scale`, но не диапазон | `src/types.ts:9-22` | Совпадает, полей диапазона нет |
|
||||
| `_saveRoomEdit()` бережно сохраняет неизвестные/future room settings через `{...previous}` | `src/houseplan-editor-runtime.ts:10959-10991` | Совпадает: `next = {...previous}`, известные поля точечно перезаписываются/удаляются |
|
||||
| `_roomSettingsFromDialog()` строит настройки новой комнаты только из известных полей | `src/houseplan-editor-runtime.ts:10948-10957` | Совпадает: собирает `st` с нуля из 6 известных полей |
|
||||
| Backend `ROOM_SCHEMA` разрешает extra-поля, диапазона в схеме нет | `custom_components/houseplan/validation.py:1267-1288` (`extra=vol.ALLOW_EXTRA`, список known settings без temp_min/max) | Совпадает |
|
||||
| Plan-only проекция переносит только allowlist `_ROOM_DISPLAY_FIELDS` | `custom_components/houseplan/import_export.py:98-100` (`fill_mode, custom_fill, glow, name_scale, label_scale`) | Совпадает, диапазона в списке действительно нет — задача обязана его добавить (§7.5), и в скоупе это названо |
|
||||
|
||||
Дополнительно проверено:
|
||||
|
||||
- `roomFillModeOf()` (`src/logic.ts:1897-1904`) — существующий helper эффективного
|
||||
режима заливки комнаты, на который ТЗ опирается при определении видимости
|
||||
блока (§10.1 «действующий режим заливки — Температура»). Существует и делает
|
||||
ровно то, что нужно.
|
||||
- `strictNumber()` — общий парсер числового поля, используется в нескольких
|
||||
местах (`src/houseplan-editor-runtime.ts:2914, 5793, 7945` и др.). ТЗ (§10.3)
|
||||
предполагает reuse этого контракта для валидации — паттерн существует, это не
|
||||
придуманная функция.
|
||||
- `temp_source`/`hum_source` уже присутствуют в общем диалоге создания/
|
||||
редактирования комнаты (`src/houseplan-editor-runtime.ts:10916-10952`) — то
|
||||
есть паттерн «один диалог на create/edit с полем комнаты» уже действует
|
||||
сегодня для соседнего поля, а не изобретается для этой задачи.
|
||||
- `docs/USER-GUIDE.ru.md` §13 «Заливки комнат и свет» → таблица «Наследование»
|
||||
(строки 1335-1343) сегодня не упоминает температурные границы комнаты вовсе —
|
||||
ТЗ корректно определяет это как пробел для закрытия release-артефактом (§17), а
|
||||
не как расхождение с уже описанным контрактом.
|
||||
- `docs/CONFIG-COMPATIBILITY.md` не содержит записи про temp-thresholds — ТЗ
|
||||
корректно планирует новую запись (§17), не претендуя на то, что она уже
|
||||
существует.
|
||||
- i18n: проект поддерживает ровно `en/ru/de/fr` (`src/i18n/{en,ru,de,fr}.json`) —
|
||||
ТЗ требует все четыре (§12), ничего не упущено и не придумано лишнего языка.
|
||||
- `scripts/config-field-registry.mjs`, `scripts/config-schema.json` существуют и
|
||||
имеют заявленную в ТЗ форму — ссылки на реальную инфраструктуру, не на
|
||||
вымышленные скрипты.
|
||||
|
||||
Ни одного утверждения о текущем поведении, которое не подтвердилось бы кодом,
|
||||
не найдено. Раздел 3 ТЗ — это в буквальном смысле код-грамотный аудит, а не
|
||||
пересказ по памяти.
|
||||
|
||||
## Проверка §7.1 (обязательные разделы) и однозначности AC
|
||||
|
||||
Все обязательные разделы присутствуют: сценарий (§1), что человек увидит до/
|
||||
после (§2, одной фразой, без терминов реализации — выполнено), проблема
|
||||
(растворена в §3, но по существу присутствует — текущее поведение и его
|
||||
недостаточность названы явно), скоуп и не-скоуп (§7, §8), контракт поведения
|
||||
(§4, §6, §11), UX (§10), модель данных и миграция (§9), i18n (§12), AC1…AC12 с
|
||||
доказательством (§13), план автотестов (§14), риски (§15), откат (§16),
|
||||
release-артефакты (§17).
|
||||
|
||||
AC1–AC12 пронумерованы, для каждого указан способ доказательства (unit /
|
||||
backend / browser+golden) и — сверх формального минимума §7.1 — колонка «что
|
||||
краснеет» (конкретный мутант или снятая защита), которая по факту в этом
|
||||
процессе обязательна только на этапе код-ревью (§2.7 «Защитный AC доказывается
|
||||
таблицей»), но её присутствие уже в ТЗ убирает половину работы будущего
|
||||
код-ревью и делает критерии проверяемыми уже сейчас, а не только на словах.
|
||||
Ни один AC не описывает решение только на словах «работает корректно» —
|
||||
каждый называет конкретную входную комбинацию (полное/частичное/отсутствующее
|
||||
наследование, перестановка границ, blank/zero/invalid, скрытие при смене
|
||||
режима, parity рендеров, backend/compatibility, i18n).
|
||||
|
||||
Проверка на «догадку, выданную за факт» (задача явно требуется процессом):
|
||||
каждое утверждение о существующем поведении в §3 и §6 привязано к конкретной
|
||||
функции/файлу и подтвердилось чтением кода (таблица выше). Новое поведение
|
||||
(§4, §9-§12) последовательно отделено фразой «Принятые решения» и разделом 5
|
||||
«Принятые предположения» — то есть авторские технические решения размечены как
|
||||
решения, а не незаметно подмешаны к описанию текущего состояния. Продуктовые
|
||||
развилки (глобальный уровень диапазона, индикатор «свой диапазон», HA-helper
|
||||
ссылки) не додуманы ТЗ, а взяты из уже состоявшихся решений владельца
|
||||
07.09.2026, процитированных в теле issue дословно — свежих открытых
|
||||
продуктовых вопросов ТЗ не содержит и не требуется задавать их отдельно.
|
||||
|
||||
Единственное явно не проверяемое человеком-ревьюером (без запуска кода)
|
||||
место — фактическое визуальное поведение узкого диалога (§10.4, AC8). Это
|
||||
корректно вынесено на browser smoke + reviewed golden, а не описано как
|
||||
самоочевидный факт.
|
||||
|
||||
## Находки
|
||||
|
||||
### Low — расхождение с уже принятой формулировкой «наследовать» (в скоупе, не блокирует)
|
||||
|
||||
`docs/specs/487-room-temperature-thresholds.md` §10.2 вводит текст кнопки
|
||||
сброса «Как в пространстве». Действующий i18n-ключ `fill.inherit` (используется
|
||||
для аналогичного смысла — опции «унаследовать от пространства» в выборе режима
|
||||
заливки комнаты, `src/houseplan-editor-runtime.ts:13926`) уже переведён как
|
||||
«Как у пространства». Строка ТЗ не является ошибкой (это новая кнопка для
|
||||
нового явления, а не переиспользование существующего ключа), но два почти
|
||||
идентичных по смыслу элемента интерфейса с разной формулировкой создают
|
||||
объективно и неопределённость терминологии, а не языковую вариативность.
|
||||
|
||||
**Решение ревьюера:** не блокирует зелёный вердикт. Автор реализации решает
|
||||
свободно — либо переиспользует существующую формулировку «Как у пространства»
|
||||
для нового элемента ради консистентности словаря интерфейса, либо оставляет
|
||||
предложенную и коротко фиксирует причину. Технический выбор строки локализации
|
||||
не относится к продуктовым вопросам §7.1 и не требует владельца.
|
||||
|
||||
Других Low, Medium или High находок нет.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Соответствие `docs/SCOPE.md`: задача закрывает J5, не расширяет и не создаёт
|
||||
новый Core user job, не задевает out-of-scope список.
|
||||
- Наследование двухуровневое, независимое по каждой границе, без глобального
|
||||
уровня и без HA helper-ссылок — совпадает с решениями владельца 07.09.2026 из
|
||||
тела issue.
|
||||
- Единственный потребитель диапазона — цвет заливки; room-card/tooltip/badge
|
||||
явно исключены и в §4.6, и в §8, и в AC11 (с названным способом падения —
|
||||
появление нового climate-pass или влияние на нетемпературный fixture).
|
||||
- «Одно число — один источник» (требование PROCESS.md §8) выполнено намеренно:
|
||||
§11 явно запрещает читать локальные пороги внутри `roomFillStyle()`, требует
|
||||
ровно один вызов `roomTempRangeOf()` за кадр и запрещает расхождение между
|
||||
full View, static card и dialog preview; AC7 отдельно доказывает parity.
|
||||
Здесь ТЗ не просто не создаёт дефект «второе число без синхронизации» — оно
|
||||
проектирует резолвер именно для того, чтобы такой дефект стал структурно
|
||||
невозможным.
|
||||
- Совместимость: §9 корректно описывает forward/backward поведение (null
|
||||
трактуется как «наследовать» и не пишется текущим UI, legacy-конфиги без
|
||||
полей не мигрируют, plan-only allowlist получает два новых поля без утечки
|
||||
HA-идентичности) и соответствует стилю уже существующих записей
|
||||
`docs/CONFIG-COMPATIBILITY.md`.
|
||||
- Откат (§16) корректно называет асимметрию: frontend откатить можно всегда,
|
||||
backend обязан продолжать принимать поле в течение заявленного окна
|
||||
совместимости, иначе откат стал бы разрушающим — это тот самый принцип,
|
||||
на котором уже стоят все записи `CONFIG-COMPATIBILITY.md`.
|
||||
- Release-артефакты (§17) перечисляют оба changelog, оба User Guide, обновление
|
||||
`config-schema.json`/registry и reviewed golden — полностью по чек-листу DoR
|
||||
(PROCESS.md §2.5).
|
||||
- Трейлеры коммита ТЗ (`Issue: #487`, `User-Visible: no`) корректны для
|
||||
документационного коммита без изменения поведения; запись в
|
||||
`docs/specs/README.md` — одна строка, без дублирующего словаря статуса
|
||||
(PROCESS.md §7.3 п.1 не нарушен).
|
||||
|
||||
## Чего не проверял и почему
|
||||
|
||||
- **Гейты `typecheck`/`test`/`build`/`check-docs` не гонялись.** Диапазон
|
||||
коммита ветки — только `docs/specs/487-room-temperature-thresholds.md` и
|
||||
`docs/specs/README.md` (класс C, `git show --stat 09328760`); `src/**` и
|
||||
`custom_components/**` не тронуты, код не менялся с последнего зелёного
|
||||
Validate на `dev`. Гонять дорогие гейты ради документационного коммита без
|
||||
единой строки продуктового кода было бы прогоном ради прогона — сам ТЗ-этап
|
||||
этого не требует (§2.4 PROCESS.md), а `check-docs.mjs` реагирует только на
|
||||
диффы `src/**`, которых здесь нет.
|
||||
- **Browser smoke / golden / backend pytest не гонялись** — по той же причине:
|
||||
на этапе ТЗ нет кода, который они могли бы проверить. Это станет предметом
|
||||
код-ревью, когда появится реализация; уже сейчас в ТЗ для каждого AC назван
|
||||
конкретный смок/golden-сценарий и мутант — их станет чем проверять на
|
||||
следующем этапе.
|
||||
- **Не проверялась связь с #56 и #68 по факту кода** (наследование `custom_fill`,
|
||||
паттерн подсказки «?») глубже, чем нужно для оценки
|
||||
консистентности: ТЗ ссылается на них как на прецедент паттерна, а не как на
|
||||
зависимость, которую эта задача меняет — их код не тронут и не должен быть
|
||||
тронут этим ТЗ.
|
||||
- **Не оценивалось качество будущей реализации** — ТЗ не описывает код, а
|
||||
описывает контракт; реализация и её защитные тесты — предмет код-ревью.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Зелёный. High: 0. Medium: 0. Low: 1 (в скоупе, не блокирует, решение — за
|
||||
автором реализации).
|
||||
|
||||
---
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `origin/issue/487-room-temperature-thresholds`
|
||||
- SHA: `09328760e6259da94aac83dff38009a641917e32`
|
||||
- Дерево ТЗ: `docs/specs/487-room-temperature-thresholds.md` (483 строки, тот же
|
||||
SHA)
|
||||
- Команда поиска материала при разрыве SHA:
|
||||
`git log --all --find-object=<blob> -- docs/specs/487-room-temperature-thresholds.md`
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/487-room-temperature-thresholds`, коммит `09328760e625` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `eb4d00d27a3e2a3d403e2b68b5298bdbc3f74aa7`
|
||||
```
|
||||
git log --all --format='%H %T' | grep eb4d00d27a3e
|
||||
```
|
||||
- ТЗ `docs/specs/487-room-temperature-thresholds.md`, блоб `d1d33e782d3edd6fd7e393c6e372a7d76439d05e`
|
||||
```
|
||||
git log --all --find-object=d1d33e782d3edd6fd7e393c6e372a7d76439d05e -- docs/specs/487-room-temperature-thresholds.md
|
||||
```
|
||||
Reference in New Issue
Block a user