From b87322ed5f6126b42ca5cc13234cfd89cdd9175a Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Thu, 27 Aug 2026 18:26:00 +0000 Subject: [PATCH] docs: review document for #267 Issue: #267 User-Visible: no --- docs/reviews/SPEC-REVIEW-267-r1.md | 171 +++++++++++++++++++++++++++++ 1 file changed, 171 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-267-r1.md diff --git a/docs/reviews/SPEC-REVIEW-267-r1.md b/docs/reviews/SPEC-REVIEW-267-r1.md new file mode 100644 index 00000000..7e2ed7e1 --- /dev/null +++ b/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) оценка автора и владельца, а не то, что + ревьюер ТЗ обязан пересчитать с нуля при отсутствии признаков пропуска.