docs: spec review r1 for #196

Issue: #196
User-Visible: no

docs/reviews/SPEC-REVIEW-196-r1.md

@@ -0,0 +1,230 @@
+# SPEC-REVIEW-196-r1
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/196
+- **ТЗ под ревью:** тело issue #196 (лёгкий трек `small`, файл в `docs/specs/`
+  отсутствует — корректно, `docs/specs/196-*.md` не создан)
+- **Роль:** ревьюер ТЗ (не автор), этап `S4-spec-review`
+- **Трек:** лёгкий (`small`), лимит циклов ревью ТЗ — 2 (§4, §5 PROCESS.md)
+- **Цикл:** r1/2
+
+## Скоуп ревью
+
+ТЗ #196: room hover tooltip в View получает необязательную строку «средняя
+влажность: N%» между температурой и LQI. Значение берётся из уже существующего
+`_roomHum()` (tier-3 `hum_source` → среднее по HA-зоне из `_climate()`),
+симметрично уже реализованной температуре. Device tooltip, room card/label
+(влажность там уже есть), touch-эквивалент hover, backend, схема, миграция —
+вне скоупа по тексту ТЗ.
+
+Ветки `issue/196-*` в репозитории ещё нет — ожидаемо: лёгкий трек, задача
+только что вышла из аналитики в ревью ТЗ, продуктового кода не существует.
+Гейты (`typecheck`/`test`/`build`, smoke) не прогонялись: на этом этапе им
+неоткуда взяться (PROCESS.md §2.4/§8).
+
+## Как проверялось
+
+1. Прочитаны целиком `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` (действующая
+   редакция, включая §1, §2.4, §5, §7.1, §8).
+2. Прочитано тело issue #196 целиком и все три комментария: аналитика
+   владельца (ценность 4/10 · сложность 2/10 · P2 · `enhancement` ·
+   поверхности `src/houseplan-card.ts` + i18n + smoke/docs · трек `small`,
+   `trivial` явно отклонён из-за нового i18n-ключа) и два «Взял:» на роли
+   аналитика/автора ТЗ.
+3. Построчно сверены все технические утверждения раздела «Сейчас» и
+   «Контракт» с кодом `origin/dev` (`src/houseplan-card.ts`, 19891 строка):
+   - `_tip` / `_showTip()` — `houseplan-card.ts:713` и `:5758`: сигнатура
+     `(ev, title, meta, lqi?, temp?)`, поле `_tip` уже несёт `lqi?: number |
+     null; temp?: number | null`. Добавление `hum?: number | null` последним
+     параметром — прямое расширение без переупорядочивания существующих.
+   - Room hover передаёт `_showTip(e, name, area, lqi, this._roomTemp(r))` —
+     `houseplan-card.ts:15627-15633` (в ТЗ заявлено `:15621`, расхождение на
+     6 строк из-за коммитов после написания ТЗ — не дефект, код найден и
+     совпадает буквально).
+   - Рендер тултипа — `houseplan-card.ts:15863-15874` (в ТЗ `:15860`, то же
+     смещение): строка `tip.temp` идёт первой (`if (this._tip.temp != null)`),
+     затем `tip.lqi` — вставка `hum` между ними ровно там, где просит
+     контракт п.3, синтаксически тривиальна (тот же паттерн `!= null ? html…
+     : nothing`).
+   - Device tooltip — `houseplan-card.ts:16729-16731`: вызывает `_showTip(e,
+     d.name, metrics)` — **без** `lqi`/`temp`/`hum` вовсе. Подтверждает
+     заявление контракта п.1 «existing device call остаётся семантически
+     неизменным»: добавление пятого необязательного параметра ничего здесь
+     не меняет, TS не потребует правки этого call site.
+   - `_roomTemp()` / `_roomHum()` — `houseplan-card.ts:16769-16781`:
+     функции буквально симметричны («tier-3 `temp_source`/`hum_source` →
+     иначе `_climate().get(area)`»). `_roomHum()` уже существует и уже
+     используется в room label (`:17085-17086`, `${hm}%` — тот же формат
+     значения, что просит ТЗ). Ссылка на реальный существующий helper, а не
+     на функцию, которую ещё нужно написать.
+   - `_climate()` — `houseplan-card.ts:16792-16802`: один кеш на `hass`
+     identity, комментарий на `:16790` дословно говорит «two, with humidity
+     on» (в ТЗ упомянут как `:16784`, тот же класс смещения) — подтверждает
+     п.7 контракта: чтение влажности не создаёт нового прохода по registry.
+   - `_noHover` — `houseplan-card.ts:5652-5653`, используется как guard
+     первой строкой `_showTip()` (`:5759`) — подтверждает контракт п.6:
+     `_showTip` уже отказывается строить тултип на no-hover устройствах,
+     новая строка не создаёт новую touch-поверхность просто потому, что
+     `hum` передаётся тем же вызовом.
+   - Значение уже целое: `areaClimateMap` округляет `hum` до `Math.round(...)`
+     (`src/devices.ts:1396`), `sourceValue()` для явного `entity:`/`device:`
+     источника — тоже `Math.round(v)` (`src/devices.ts:1252, humFor`).
+     Предположение ТЗ «значение не пересчитывается, используется готовый
+     результат `_roomHum()` с суффиксом `%`» корректно: дробного значения
+     физически не бывает, формат `${hum}%` без округления в тултипе не
+     потеряет точность.
+4. Прочитан `src/i18n/en.json` и `ru.json`: `tip.temp_avg` (`:374`
+   `"average temperature:"` / `"средняя температура:"`) и `tip.lqi`
+   (`:150`) — предложенный `tip.hum_avg` (`"average humidity:"` /
+   `"средняя влажность:"`) следует тому же формату (двоеточие, строчная
+   буква, `average <metric>:`), новый ключ не выдуман по структуре.
+5. Прочитан `docs/USER-GUIDE.ru.md`: строка 173 (таблица режимов) —
+   «подсказка показывает название, чистую площадь, температуру и LQI» — это
+   ровно то место, которое ТЗ обязано обновить (и заявляет это в разделе
+   Release), и ровно то место, что фиксирует нынешний пробел (влажности в
+   перечислении нет, хотя строка 128/275/1175 подтверждает, что влажность
+   уже показывается в room card). Строка 1310 «Комнатные подсказки основаны
+   на hover» — существующее, не создаваемое этим ТЗ ограничение,
+   распространяется на новую строку тем же образом, что и на текущие.
+6. Прочитан `docs/TOUCH-SUPPORT.md` целиком. Правило «Documentation rule»
+   (`Touch editor: …`) относится к «New editor feature specifications» —
+   тултип комнаты живёт в View, не в редакторе, правило не применяется
+   механически. Контракт «fully supported View» (строка 31-32: «provide a
+   touch path for essential information that desktop exposes through
+   hover») для влажности не создаёт нового риска: `disp.labelHum` уже кладёт
+   влажность в room label (`houseplan-card.ts:17084-17086`), который виден
+   без hover — то есть на touch влажность уже доступна отдельным путём,
+   ТЗ не ухудшает существующий (уже документированный как ограничение,
+   `USER-GUIDE.ru.md:1310`) hover-only статус тултипа для area/temp/LQI.
+7. Проверено, что `docs/specs/196-*.md` не существует (`ls docs/specs/` —
+   пусто по маске) — соответствует лёгкому треку.
+8. Проверено соответствие `docs/SCOPE.md`: задача закрывает J5 «Room climate
+   at a glance» (persona home admin/household, View), не расширяет
+   out-of-scope списки, не касается lock invariant, не создаёт нового job'а.
+9. Проверены оба существующих файла-кандидата для smoke:
+   `demo/smoke_ux_fixes.mjs` (`:21-27`) уже содержит инфраструктуру для
+   проверки `_tip.temp` и текстового содержимого `.tip` («средняя
+   температура»), то есть AC1-AC3 технически осуществимы без нового
+   тестового харнесса — паттерн для новой `hum`-проверки уже есть в файле.
+   `demo/srv/demo.html` не содержит фикстуры-сенсора влажности; но 31 файл
+   в `demo/` (в т.ч. `smoke_room_settings.mjs`, `smoke_sun.mjs` и др.) уже
+   инжектируют синтетические `hass.states` динамически в `page.evaluate` —
+   значит отсутствие штатной humidity-фикстуры не блокирует AC1/AC2,
+   разработчик может ввести сенсор тем же приёмом. Это техническое решение,
+   ТЗ корректно оставляет выбор конкретных имён/assertions разработчику
+   (раздел «Принятые технические предположения»).
+
+## Обязательные разделы для лёгкого трека (§5 PROCESS.md)
+
+| Раздел | Есть | Комментарий |
+|---|---|---|
+| Проблема | ✅ | Подтверждена построчным чтением кода (см. «Как проверялось» п.3): `_roomHum()` существует и используется в label, но не в tooltip — не догадка, факт |
+| Контракт | ✅ | 7 пронумерованных пунктов, включая явную защиту от расширения device tooltip (п.1) и touch-поверхности (п.6) |
+| AC1…ACn с доказательством | ✅ | 5 штук, у каждого назван способ (`smoke`/`unit`+`build`/«ревью кода») |
+| Откат | ✅ | «Совместимость и откат»: revert одного коммита, без миграции, без cleanup |
+
+Дополнительно присутствуют разделы, не обязательные для лёгкого трека, но
+усиливающие ТЗ: «Не входит в задачу» (явный анти-скоуп), «Файлы и проверки»,
+«Release, performance, security, visual», «Принятые технические
+предположения» (3 пункта, все технические — не маскируют продуктовый вопрос).
+
+## Находки
+
+### Low-1 — AC2 частично привязывает `smoke`-доказательство к утверждению, проверяемому только кодом
+
+**Место:** тело issue #196, раздел «Критерии приёмки», AC2: «код tooltip
+вызывает общий `_roomHum()`, а не дублирует resolver».
+
+Первая половина AC2 (значение override совпадает с настроенным
+`hum_source`) действительно проверяется браузерным смоком через видимый
+текст. Вторая половина («не дублирует resolver») — по природе утверждение о
+структуре кода, а не о видимом поведении; смок не может отличить «вызвал
+`_roomHum()`» от «скопировал ту же формулу в другом месте с тем же
+результатом». Это не отменяет проверяемость: AC5 уже требует ревью кода на
+отсутствие новых resolver'ов/scope creep, так что содержательно вторая
+половина AC2 покрыта — реального пробела в доказательстве нет, только
+неточная маркировка способа доказательства одним словом `smoke` на весь
+пункт.
+
+**Обоснование severity:** Low — не блокирует, ничего не остаётся
+непроверенным, дефект чисто редакционный (смешаны behavioral и structural
+части одного AC под одной меткой доказательства).
+
+**Решение ревьюера:** снимаю без возврата ТЗ на правку. Код-ревью (AC5)
+обязан явно подтвердить, что `_roomHum()` вызывается напрямую, без
+дублирования резолвера — этот пункт настоящим решением фиксируется как
+входящий в объём AC5, а не отдельно требующий правки текста ТЗ.
+
+## Что проверено и корректно
+
+- **Диагноз проблемы не голословен.** `_roomHum()` (`:16777-16781`)
+  дословно симметричен `_roomTemp()` (`:16769-16774`), уже используется в
+  room label (`:17085-17086`) и уже настраивается в диалоге комнаты
+  (`_renderRoomSource('hum')`, `:19717-19838`) — но не передаётся в
+  `_showTip()` при hover (`:15627-15633`). Пробел реален, не выдуман.
+- **Соответствие `docs/SCOPE.md`:** закрывает J5, персона home
+  admin/household member, поверхность View; не расширяет lock invariant,
+  out-of-scope список, backend/schema/migration.
+- **Контракт защищает device tooltip от регрессии** (п.1, подтверждено
+  реальным call site `:16729-16731`, который не передаёт `lqi`/`temp`
+  вовсе — пятый необязательный параметр `hum` для него ничего не меняет).
+- **Touch-поверхность не ухудшается.** Room label уже показывает влажность
+  без hover (`disp.labelHum`, `:17084-17086`); hover-only статус самого
+  тултипа — существующее, уже задокументированное ограничение
+  (`USER-GUIDE.ru.md:1310`), не создаваемое этим ТЗ. Правило `Touch editor:
+  …` из `docs/TOUCH-SUPPORT.md` не применяется — это не editor-фича.
+- **AC1-AC5 однозначны, каждый снабжён допустимым способом доказательства**
+  (`smoke`/`unit+build`/«ревью кода»); формат значения (`Math.round`,
+  целое число, суффикс `%`) подтверждён на уровне `devices.ts` — в тултипе
+  не появится дробное число, которое пришлось бы отдельно форматировать.
+- **i18n-ключ следует существующей конвенции** (`tip.temp_avg` →
+  `tip.hum_avg`, тот же шаблон текста на обоих языках).
+- **AC3 корректно защищает границы:** `null`/невалидный источник не
+  показывает строку (тот же паттерн `!= null`, что уже используют
+  `tip.temp`/`tip.lqi`), device tooltip не получает ложную влажность,
+  `_noHover` не создаёт тултип — все три случая проверены по коду и
+  логически вытекают из уже существующих guard'ов, не из нового кода,
+  который ещё предстоит написать правильно.
+- **Откат тривиален** — один user-visible коммит, без миграции схемы/данных,
+  без cleanup.
+- **Принятые технические предположения (3 пункта)** — все технические
+  (порядок параметра `_showTip`, отсутствие пересчёта формата, свобода
+  имён тестов), продуктовый вопрос под видом технического решения не
+  найден.
+- **Не додумано скрытых предположений о поведении.** Ни одно утверждение
+  контракта не описывает поведение, которого нет в коде/канонических
+  документах и не помечено как assumption.
+
+## Чего не проверял
+
+- Реализацию — её нет: ветка `issue/196-*` в репозитории отсутствует,
+  продуктовый код не менялся (`git branch -r` не находит ветку).
+- Гейты `typecheck`/`test`/`build`/smoke — не относятся к этапу ревью ТЗ
+  (PROCESS.md §2.4/§8), выполнить их не над чем.
+- `python -m pytest tests_backend` — задача не касается backend ни кодом,
+  ни текстом ТЗ.
+- Реальный визуальный результат (браузерный рендер строки влажности,
+  точное позиционирование `.tip`) — CSS/разметка ещё не написаны;
+  заявленное («тот же паттерн, что и temp/lqi») — это план, а не
+  исполненный факт, что и должно быть на этапе ТЗ.
+- Точность числовых оценок аналитики (4/10, 2/10, P2) по существу — поле
+  владельца (PROCESS.md §2.2), не предмет ревью ТЗ.
+- Возможность технического спора автор/ревьюер по разделу «Принятые
+  технические предположения» — не возникла, все три пункта разумны без
+  необходимости их оспаривать.
+
+## Вердикт
+
+Зелёный. High: 0, Medium: 0, Low: 1 (AC2 смешивает поведенческую и
+структурную часть под одной меткой `smoke` — не оставляет пробела в
+доказательстве, так как структурная часть уже покрыта AC5; снимается без
+возврата ТЗ на правку, код-ревью проверяет обе половины). ТЗ подтверждено
+построчным чтением кода `origin/dev`: каждое техническое утверждение
+оказалось фактом (с точностью до нормального смещения номеров строк из-за
+коммитов после написания ТЗ), продуктовых вопросов владельцу нет и не
+додумано новых, AC1-AC5 однозначны и снабжены допустимыми способами
+доказательства, откат тривиален, лёгкий трек применён корректно (новый
+i18n-ключ и enhancement-тип верно исключают `trivial`, сложность/риск ≤3 и
+одна поверхность верно подтверждают `small`).
+
+**Вердикт: зелёный · цикл r1/2 · High: 0 · Medium: 0 → в задаче · Документ:
+docs/reviews/SPEC-REVIEW-196-r1.md**