docs: review document for #267

Issue: #267
User-Visible: no

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

@@ -0,0 +1,171 @@
+# SPEC-REVIEW-267-r1
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/267
+- **Этап:** ТЗ на ревью (PROCESS.md §2.4)
+- **Заход:** r1 · блокирующих циклов израсходовано 0 из 4 (первый заход)
+- **Материал:** `docs/specs/267-device-presentation-decision-table.md` на SHA
+  `c5e14526` (ветка `issue/267-device-presentation-table`), плюс тело issue и
+  два комментария («Аналитика», «ТЗ готово к независимому ревью»)
+- **Вердикт: зелёный**
+
+## Скоуп ревью
+
+Первый заход — разбор полный. Проверялось:
+
+1. соответствие `docs/SCOPE.md` (какой Core user job закрывает задача);
+2. обязательные разделы ТЗ по PROCESS.md §7.1 и однозначность/доказуемость
+   каждого AC;
+3. отсутствие догадок, выданных за факт — каждое «зафиксированное продуктовое
+   решение» (§3 ТЗ) сверено с первоисточником (`docs/USER-GUIDE.ru.md` §12,
+   issue #251/#274/#98) построчно;
+4. измеренная база (строки/ветвления/имена функций) сверена с текущим
+   `src/device-presentation.ts` на `HEAD`;
+5. технические допущения (§20 ТЗ) — что помечено как «можно менять на
+   ревью», не выдано за продуктовое решение;
+6. инструменты, на которые ссылается план тестов (`scripts/mutation-gate.mjs
+   --check`/`--id=`, `scripts/smoke-select.mjs`, существующие smoke-файлы),
+   существуют и поддерживают заявленный интерфейс.
+
+## Как проверялось
+
+| Что | Команда/действие | Результат |
+|---|---|---|
+| Класс изменения коммита | `git show --stat HEAD` | только `docs/specs/267-*.md` + `docs/specs/README.md` — класс C, соответствует заявленному в шапке ТЗ |
+| Измеренная база §2 ТЗ | `wc -l src/device-presentation.ts`, `grep -n` на `resolvePresentationSources`/`resolveDevicePresentation` | 781 строка, функции на строках 252→404 (152) и 600→781 (181) — совпадает с заявленными 781/152/182 |
+| Существование смежных модулей §7.2 | `ls src/device-*.ts` | `device-visual.ts`, `device-pulse.ts`, `device-value-badge.ts`, `device-face.ts`, `device-toggle.ts` — все существуют, как заявлено |
+| Соответствие §3.2 (#251) | чтение `docs/USER-GUIDE.ru.md:914-921` | текст руководства слово в слово подтверждает решение (доступность контроллера только по своим сущностям, виртуальный контроллер доступен) |
+| Соответствие §3.3 (#274) | чтение `docs/USER-GUIDE.ru.md:922-924` | подтверждено: удалённая цель не делает контроллер недоступным и не расходит план/preview |
+| Соответствие §3.4 (#98, пульсации) | чтение `docs/USER-GUIDE.ru.md:945-963` | подтверждено: обычная пульсация только в «Значок + состояние и активность», статичных колец нет |
+| Соответствие §3.5 (`static_icon`) | чтение `docs/USER-GUIDE.ru.md:1007-1014` | подтверждено дословно, включая исключение hover/controls/Glow/заливки |
+| L05 (`hidden_design_preview`) не выдумка | `grep -rn hidden_design_preview src/ test/` | строка уже существует в `device-presentation.ts:87,728`, `i18n/ru.json:236`, покрыта `test/device-presentation.test.mjs:751` — существующее поведение, не новая догадка |
+| F09–F12 (value fallback reasons) | `grep -rn value_ambiguous_sources\|value_virtual\|activity_display_disabled` | все три reason уже в коде и тестах |
+| F16 (LQI-полосы 0/40, 41/179, 180+) | чтение `markerLqiBand()` (`device-presentation.ts:46-49`) | точное совпадение границ |
+| Инструмент `mutation-gate.mjs --id=`/`--check` | `grep -n "'--check'" `, `grep -n "startsWith('--id="` | оба флага существуют и поддерживаются |
+| Смоки, названные в ТЗ | `ls demo/smoke_*.mjs \| grep -E "wireless\|device_icon"` | `smoke_wireless_controller_parity.mjs`, `smoke_device_icon_design.mjs` существуют |
+| Легаси-совместимость `ripple`→`icon_ripple` (§13) | чтение `normalizeDeviceDisplay()` (`src/logic.ts:852-856`) | подтверждено дословно |
+| Связанные issues реальны и закрыты | `gh issue view 98/251/274/34` | #98, #251, #274 закрыты и по теме совпадают с описанием в ТЗ; #34 открыт как архитектурный umbrella — корректная ссылка, не блокирующая зависимость |
+| Уникальность decision row ID | `grep` по `^\| L0\|^\| S0…` + `uniq -d` | дубликатов ID нет (30 строк: 6 L, 13 S, 17 F, 8 A) |
+| Порядок приоритета §7.3 vs реальный код | чтение `resolveDevicePresentation()` целиком (`device-presentation.ts:600-781`) | заявленный порядок (lifecycle → static → alarm переживает live-gate → live-gate → controller availability → …) соответствует фактической последовательности веток в текущей реализации — не выдумана «идеальная» модель без связи с кодом |
+| Индекс `docs/specs/README.md` | `git diff HEAD~1 HEAD -- docs/specs/README.md` | строка `#267` добавлена в том же коммите, ссылка верна |
+| `node scripts/check-docs.mjs` | прогнан | `Documentation checks passed (7 files, 10 external links)` — зелёный |
+| `git diff --check` на коммит спеки | `git diff --check HEAD~1 HEAD -- docs/specs/267-*.md` | **не зелёный**: `548: new blank line at EOF` (см. находки, Low) |
+
+Гейты `typecheck`/`test`/`build` не гонялись: коммит `c5e14526` — чистый класс C
+(только `docs/specs/**`), продуктовый код не менялся, `src/**` не тронут, и
+`check-docs.mjs` (который зависит от `src/**`) уже прогнан и зелёный.
+Браузерные смоки, golden, инварианты геометрии, backend-тесты — неприменимы:
+diff не касается `src/**`/`custom_components/**` в этом заходе, это ещё
+не реализация.
+
+## Находки
+
+### Low-1 — заявление «`git diff --check` — green» в хендоффе не совпадает с фактом
+
+**Файл:** issue #267, комментарий «ТЗ готово к независимому ревью»
+(2026-08-27T18:17:33Z).
+
+Автор указал `git diff --check` как один из двух прогнанных чеков и заявил
+результат «green». Повторный прогон `git diff --check HEAD~1 HEAD --
+docs/specs/267-device-presentation-decision-table.md` даёт `548: new blank
+line at EOF` (файл заканчивается двумя переводами строки вместо одного),
+exit code 2. Само по себе это не искажает содержание ТЗ и не влияет ни на один
+AC — `check-docs.mjs`, единственный настоящий гейт для доков в этом диффе,
+зелёный. Но правило «"Verified" без названного результата не является
+доказательством» работает в обе стороны: названный результат должен быть
+точным. Ложноположительный «green» в хендоффе — ровно тот тип записи, который
+процесс просит не допускать.
+
+**Решение ревьюера:** снимается без правки. Косметическая лишняя пустая
+строка в конце файла, не блокирует переход в `S5-ready`; будет естественно
+убрана следующим коммитом по ветке (код или доп. правка спеки), отдельного
+возврата на цикл не оправдывает.
+
+### Что не является находкой (проверено и отклонено как ложное подозрение)
+
+- **Отсутствие отдельных заголовков «UX» и «Модель данных и миграция».**
+  PROCESS.md §7.1 требует эти темы по содержанию, а не буквальным заголовком.
+  Содержание есть: §1 «Что человек увидит» прямо формулирует отсутствие
+  видимых изменений, §12 не-скоуп явно исключает новые статусы/цвета/иконки/
+  display-режимы, §13 явно закрывает данные/миграцию («отсутствуют»,
+  `normalizeDeviceDisplay()` остаётся read-gate). Прецедент того же жанра задач
+  — `docs/specs/264-resize-controller.md` — построен по не идентичному, но
+  сопоставимому по духу набору разделов. Не находка.
+- **«Открытых продуктовых вопросов нет» при сложности 7/10.** Проверено
+  предметно, а не на слово: все три продуктовых вопроса из тела issue («что
+  видит человек при доступном контроллере/недоступной цели», «приоритет осей
+  при конфликте», «сколько состояний имеет право быть различимыми») закрыты
+  ссылкой на уже принятые решения (#251, #274, USER-GUIDE §12) и на явный
+  не-скоуп «новых статусов/цветов/иконок нет» — это не отказ отвечать, а
+  предметный ответ «поведение не меняется, только структура кода». Для
+  refactor-only задачи с зафиксированным контрактом это корректно, а не
+  недосмотр.
+- **Достижимость AC2** («renderer и surfaces не принимают альтернативных
+  решений по raw HA state»). В `houseplan-card.ts` действительно остаются
+  прямые обращения к `hass.states[...]` (vacuum-телеметрия, friendly_name для
+  подписей/тултипов, состояние contact/lock для соседних, не face-related
+  элементов). AC2 ограничен явно: «Lifecycle/display/status/content/
+  diagnostics gates **из `resolveDevicePresentation()`**» — то есть про
+  вынос уже существующих внутри функции веток, а не про полный запрет чтения
+  `hass.states` во всём файле. Формулировка не создаёт невыполнимое
+  требование.
+
+## Проверка обязательных разделов (PROCESS.md §7.1)
+
+Сценарий и персона (§1) — есть, персона из `docs/SCOPE.md` (домашний
+администратор + разработчик как второй адресат документа). Что человек
+увидит до/после (§1) — есть, явно «ничего нового». Проблема (§2) — есть,
+с измеренной и подтверждённой базой. Скоуп/не-скоуп (§11/§12) — есть,
+не-скоуп явно закрывает риск декартова произведения и попутных продуктовых
+изменений. Контракт поведения — распределён по §3 (зафиксированные решения) и
+§6 (таблицы рядов), содержательно присутствует. UX/данные и миграция —
+покрыты содержательно (см. «что не является находкой» выше). i18n — явно «нет
+новых строк» (§13), проверено: задача не должна трогать `src/i18n/*.json`,
+её и не трогает вне ТЗ. AC1…AC10 — пронумерованы, у каждого указан способ
+доказательства (§15). План автотестов — есть (§16), с точным списком команд
+перед S7. Риски — есть (§18), адресуют именно те риски, которые релевантны
+этому рефакторингу (декартово произведение, второй resolver, plan/preview
+расхождение, регрессия pixel-parity). Откат — есть (§19), корректен для
+чисто структурного рефакторинга без миграции. Release-артефакты — есть
+(§17), корректно `User-Visible: no`.
+
+## Проверка AC на однозначность и доказуемость
+
+Каждый из AC1–AC10 формулирует наблюдаемое или механически проверяемое
+условие и называет способ доказательства (unit/architecture test/mutation
+registry/smoke/golden/code review), без AC вида «работает корректно» без
+критерия. AC4 и AC5 образуют закрытую петлю «документ ↔ fixture ↔ mutant»,
+которая явно защищает от того самого дрейфа, ради которого затевается задача
+(документ описывает желаемое, а не то, что тест умеет проверить). AC8 —
+единственный жёсткий pixel/config-контракт рефакторинга — корректно замкнут
+на существующий `golden:verify` и docs screenshot gate, а не на новое
+самодельное сравнение.
+
+Отдельно проверено на воспроизводимость до кода: порядок приоритетов §7.3
+не является постулированной автором целевой моделью, оторванной от
+реальности — он совпадает с фактической последовательностью условий в
+`resolveDevicePresentation()` на `HEAD` (эффективная скрытость → static →
+alarm переживает live-gate → live-gate → controller availability →
+value/icon/pulse). Это значит рефакторинг, скорее всего, реализуем без
+скрытого изменения поведения, а не переписывает контракт под видом
+документирования.
+
+## Унаследовано / предыдущие раунды
+
+Неприменимо — это первый заход (r1) по этому этапу.
+
+## Чего не проверял
+
+- Реализацию (класс A/B) — её ещё нет, диапазон `S3-spec`/`S4-spec-review`
+  правок кода не касается.
+- `typecheck`/`npm test`/`npm run build`/браузерные смоки/golden/инварианты
+  геометрии/backend — неприменимы к чисто документационному коммиту этого
+  захода; будут обязательны на код-ревью.
+- Реальную достижимость AC9 (O(1) policy) и AC7 (surface parity) на уровне
+  производительности/поведения — это утверждения о будущей реализации,
+  доказываются на код-ревью, а не на этапе ТЗ.
+- Полноту декомпозиции §6 (все ли нужные строки перечислены, не пропущена ли
+  комбинация) — не пересчитывал декартово произведение осей вручную; проверил
+  выборочно, что перечисленные строки соответствуют реальным веткам кода и
+  реальным issue-регрессиям (#251, #274), и что ID уникальны. Полнота набора
+  рядов — предметная (product) оценка автора и владельца, а не то, что
+  ревьюер ТЗ обязан пересчитать с нуля при отсутствии признаков пропуска.