From 2d8e7871f6ac4ee6f5ce6e18fd33a1eb6d98fdf7 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Tue, 8 Sep 2026 11:14:20 +0000 Subject: [PATCH] docs: review document for #485 Issue: #485 User-Visible: no --- docs/reviews/SPEC-REVIEW-485-r1.md | 251 +++++++++++++++++++++++++++++ 1 file changed, 251 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-485-r1.md diff --git a/docs/reviews/SPEC-REVIEW-485-r1.md b/docs/reviews/SPEC-REVIEW-485-r1.md new file mode 100644 index 00000000..355b7963 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-485-r1.md @@ -0,0 +1,251 @@ +# SPEC-REVIEW-485-r1 + +Issue: [#485](https://github.com/Matysh/houseplan-card/issues/485). +Материал: ветка `issue/485-radar-presence`, SHA `6550a69e6857d11aba69b65809f140d239245e88` +(baseline `dev@ed9ee026dc08054e12038b7dbd1b8525e7706d04`, докс-only diff, проверено +`git diff --stat` — только `docs/specs/485-radar-presence*.md` и `docs/specs/README.md`). +Этап: **spec** (S4-spec-review). Заход: r1. Трек: полный (issue не помечен `small`). +Ревьюер ≠ автор (роли закреплены в AGENTS.md). + +## Скоуп разбора + +Комплект из четырёх документов, рассматриваемых как единое ТЗ (общий контракт + +три этапа), в порядке, предписанном §1 общего контракта: + +1. `docs/specs/485-radar-presence.md` — общий контракт (акцептанс-карта C-1…C-8). +2. `docs/specs/485-radar-presence-stage1.md` — источники, калибровка, живое присутствие (S1-1…S1-18). +3. `docs/specs/485-radar-presence-stage2.md` — зоны, запись наблюдений, отражатели (S2-1…S2-22). +4. `docs/specs/485-radar-presence-stage3.md` — покрытие, слияние, HA-сущности, тепловая карта (S3-1…S3-18). + +Задача первого раунда (r1): разбор полный, второй раунд не образует делту (§2.10 +не применяется). Прочитаны целиком все четыре документа, `docs/SCOPE.md`, +`AGENTS.md`, `PROCESS.md`, `docs/TOUCH-SUPPORT.md`, `docs/CONFIG-COMPATIBILITY.md`, +тело issue #485 и все 6 комментариев (полевые данные автора запроса, UX-проработка +владельца, аналитика S2/S3, подтверждение defaults Q1–Q4, публикация комплекта). + +## Как проверялось + +- Продуктовая рамка: сверено с `docs/SCOPE.md` — J1 и явно перечисленный + допустимый gap («Person/presence shown in rooms… pure J1») закрывают живой слой; + запись/облако/тепловая карта заявлены в общем контракте как «bounded exception» + к исключению «History, graphs, statistics» — см. находку M-1. +- Процесс: трек, статус, число заходов и трейлеры issue сверены через + `gh issue view 485` (метки `P2, feature, S4-spec-review`, `small` отсутствует — + полный трек подтверждён). +- Технический разбор: каждый AC C/S1/S2/S3 прочитан вместе со своей строкой + доказательства и негативным свидетелем; проверялась связность между документами + (единая модель source_generation/calibration_revision, единые лимиты 32 радара, + единая канонической проекция §5.1 этапа 1, единый предикат зон/отражателей). +- Cross-check ссылок: `grep` подтвердил, что все четыре документа ссылаются друг + на друга по существующим именам файлов (см. ниже), запись в `docs/specs/README.md` + добавлена и указывает на все четыре файла. +- `docs/TOUCH-SUPPORT.md` построчно сверен с требованием «New editor feature + specifications … must state one of: Touch editor: supported / best effort / + intentionally degraded / not exposed» — см. находку M-2. +- Прецедент оформления SCOPE-исключений сверен по истории: `git log -- docs/SCOPE.md` + показал, что оба предыдущих узких исключения (#89 «revise isometric stage 1 + specification», #53 «address PDF export spec review») были зафиксированы именно + в `docs/SCOPE.md` в рамках цикла ревью/доработки спецификации, а не отложены до + релиза — прямой прецедент для M-1. + +### Гейты + +Диапазон (`ed9ee026`..`6550a69e`) — только четыре файла в `docs/specs/`, докс-only +(class C, `docs/specs/485-radar-presence*.md`), продуктовый код не тронут. +Соответственно раннер-гейты `typecheck/test/build`, `check-docs.mjs`, +`model-invariants`, browser-смоки, `golden:verify`, `pytest tests_backend` и +перфопрофили — **не запускались, потому что неприменимы**: AC этого этапа не +требуют исполнения кода (единственный AC, требующий факта — C-8, «diff restricted +to documentation», проверен `git diff --stat` выше). Ссылка на зелёный Validate на +этом SHA (`https://github.com/Matysh/houseplan-card/actions/runs/34218729737`) +подтверждает, что дешёвые гейты тоже зелёные, хотя для этого раунда они не несут +доказательной нагрузки — С4/АС ревью ТЗ не про исполнение кода. Никакой гейт не +был пропущен «по недосмотру»: полный список выше — это список неприменимых, а не +непрогнанных гейтов. + +## Находки + +### M-1 (Medium, в скоупе) — исключение из SCOPE.md для истории/тепловой карты не зафиксировано в самом SCOPE.md + +`docs/SCOPE.md` явно относит «History, graphs, statistics» к Out of scope: «We +show now, not then» (docs/SCOPE.md:98). Общий контракт (`485-radar-presence.md:40-46`) +признаёт это прямо: «The owner explicitly requested zones, local observations, +day/week heat maps… These are a bounded exception to SCOPE's exclusion of general +historical analytics», но это утверждение живёт только в ТЗ. Сам `docs/SCOPE.md` +никак не упоминает радары/присутствие/тепловые карты — ни в «Out of scope», ни в +«Known gaps», хотя раздел «Known gaps» уже содержит близкую строку («Person/presence +shown in rooms… pure J1», docs/SCOPE.md:64), которая закрывает **живой** слой (этап 1), +но не объясняет исключение для записи/тепловой карты (этапы 2–3). + +Прецедент есть в самом репозитории: оба прежних узких исключения из «Out of scope» +— трёхмерный вид (#89) и печатный PDF-экспорт (#53) — были дописаны в +`docs/SCOPE.md` именно в рамках доработки спецификации по итогам ревью +(`74b08df8 docs: revise isometric stage 1 specification`, `299629d2 docs: address +PDF export spec review`), а не отложены до релиза. Формат этих записей — +короткий абзац «A narrow exception approved for #NN…» с явной границей, что именно +разрешено и что нет (docs/SCOPE.md:101-114). + +Ни один из трёх этапов не перечисляет `docs/SCOPE.md` среди release-артефактов +(stage1 §10, stage2 §12, stage3 §14 называют CHANGELOG, USER-GUIDE, ARCHITECTURE, +CONFIG-COMPATIBILITY, field registry — SCOPE.md нигде). + +**Почему это не мелочь.** `docs/SCOPE.md` — заявленный «guard rail»: «features are +built… only if they serve a job listed here» и «if there is none — it belongs to +HA core, to another card, or nowhere» (docs/SCOPE.md:3-6). Пока запись/тепловая +карта не зафиксированы как узкое исключение непосредственно в этом документе, +следующий читатель SCOPE.md увидит прямое противоречие («history… we show now, not +then») без объяснения, почему радар #485 — не такое же нарушение, каким было бы +любое другое предложение исторической аналитики. Это ровно тот разрыв, который +review #89/#53 закрывал на этой же стадии процесса. + +**Как чинится (в скоупе):** добавить в `docs/SCOPE.md` абзац по образцу +#89/#53 — узкое исключение для #485, ограниченное явно описанными в общем +контракте рамками (запись ≤24 ч по явному согласию, тепловая карта — только +агрегаты ≤7 дней, редактор-only доступ, никакого автопродления/непрерывного +сбора), и добавить `docs/SCOPE.md` в перечень release-артефактов во всех трёх +этапных документах (сейчас перечислены только CHANGELOG/USER-GUIDE/ARCHITECTURE/ +CONFIG-COMPATIBILITY/field registry). + +### M-2 (Medium, в скоупе) — этап 1 не декларирует touch-классификацию нового редактора (обязательное требование TOUCH-SUPPORT.md) + +`docs/TOUCH-SUPPORT.md` («Documentation rule», конец файла) требует: «New editor +feature specifications and code reviews must state one of: `Touch editor: +supported`; `Touch editor: best effort / intentionally degraded`; `Touch editor: +not exposed`.» Это не общее пожелание, а обязательная декларация для каждой новой +функции редактора. + +Этап 2 и этап 3 её дают дословно: +- `485-radar-presence-stage2.md:174` — «Desktop is the reference editor. Touch + editor: best effort / intentionally degraded…»; +- `485-radar-presence-stage3.md:131` — «Touch editor: **best effort / + intentionally degraded** for precise geometry and pair selection…». + +Этап 1 вводит новый мастер настройки радара (§5.2: «Installation» → «Two reference +positions» → «Check») — это новая функция редактора не меньшего масштаба, чем зоны +или анализ покрытия, но нигде в документе (проверено `grep -n -i touch` по всему +файлу) не встречается требуемая формулировка. Единственные следы — общая фраза +общего контракта §6 («Editors remain desktop-first… safe cancellation/permissions +on touch») и AC S1-18 («safe desktop/touch exit meet §2/5/8»), которые описывают +безопасный выход, а не классификацию поддержки самого мастера. + +**Почему это не формальность именно здесь.** Шаг «Two reference positions» +физически требует, чтобы пользователь **отошёл от компьютера** и встал в отмеченную +точку комнаты, пока идёт 10-секундный отсчёт и 5-секундный захват (§5.2). Это, +в отличие от обычного рисования зоны мышью, сценарий, где телефон/планшет в руке — +скорее естественный инструмент, чем компромисс. Продуктовое решение «оставить это +desktop-first best-effort, как и остальные редакторы» может быть правильным, но +оно не сделано явно и не объяснено — редактор просто пропущен в классификации, +а не намеренно отнесён к одному из трёх состояний. Раздел 5 (Compatibility/touch) +из DoR-чеклиста PROCESS §2.5 требует именно этого явного решения, а не умолчания. + +**Как чинится (в скоупе):** добавить в `485-radar-presence-stage1.md` (рядом с §5.2 +или в UX-разделе §6 общего контракта, где он уже цитируется по стадиям 2/3) точную +формулировку `Touch editor: …` для мастера калибровки, с явным решением — либо +«best effort / intentionally degraded» с тем же обоснованием, что у этапов 2–3, либо +отдельно объяснить, почему шаг «дойти и встать» продуктово не требует лучшей +touch-поддержки, чем обычное редактирование геометрии. + +## Что проверено и корректно + +- **Границы скоупа и не-скоупа** каждого этапа (Included/Excluded) не пересекаются + и не противоречат друг другу: живой слой/калибровка (1) → зоны/запись/отражатели + (2) → покрытие/слияние/сущности/тепловая карта (3); каждый этап явно говорит, + что не берёт на себя работу другого («Excluded… specified in stages 2–3»). +- **Единая модель идентичности** (`source_generation`, `calibration_revision`, + `server_session_id`/`seq`) определена один раз в общем контракте §4 и + последовательно используется во всех трёх этапах без переопределения смысла — + проверено построчным сравнением определений в stage1 §6, stage2 §7.1, stage3 §10. +- **AC и негативные свидетели.** Все 8+18+22+18=66 критериев приёмки имеют + колонку доказательства и явно названный негативный сценарий/мутацию, которая + должна дать красный результат (например S2-17: «31 of 100» с конкретной + фикстурой из полевых данных 0.5%→31%, а не абстрактное «score correct»). + Это выше формального минимума ТЗ (PROCESS требует только «чем доказывается»), + но снижает риск немой защиты на код-ревью (#435). +- **«Одно число — один источник»** для чисел, которые пользователь видит дважды: + stage3 §7.2 прямо требует «native state must match the backend room snapshot»; + оценка зоны (stage2 §4.3/§10) явно завязана на одну и ту же выборку для + числителя и знаменателя. Нарушений правила PROCESS §8 не найдено. +- **Совместимость и жизненный цикл** (импорт/дублирование/удаление комнаты/смена + пространства/понижение фронтенда) описаны для каждого нового поля в каждом + документе с явной матрицей «old/new frontend×backend», по образцу существующего + `docs/CONFIG-COMPATIBILITY.md`; ни одного поля не найдено без описанной судьбы + при откате. +- **Продуктовые вопросы закрыты владельцем.** Q1–Q4 из UX-проработки получили + явное подтверждение владельца в комментарии `IC_kwDOTOcLQM8AAAABTNOxFA` + (08.09.2026); ни в одном документе не найдено скрытой догадки, выданной за + факт, кроме двух находок выше — остальные технические допущения (имена + модулей, тайминги, пороги солвера) прямо помечены «assumed, change freely» + и корректно не эскалированы владельцу (PROCESS §7.1). +- **C-8 (докс-only)**: `git diff --stat ed9ee026..6550a69e` подтверждает, что + изменены только четыре файла `docs/specs/*` плюс запись в README реестра + спецификаций — продуктовый код, тесты и версии не тронуты, ревью не требует + прогона исполняемых гейтов. +- **Перекрёстные ссылки.** Все четыре документа ссылаются друг на друга по + существующим относительным путям (`grep` подтвердил отсутствие битых ссылок); + запись в `docs/specs/README.md` добавлена и указывает на все три этапа плюс + общий контракт. +- **Touch-декларация этапов 2 и 3** присутствует дословно в требуемом формате + (см. находку M-2 для контраста с этапом 1). + +## Чего не проверял + +- Не запускал `npx tsc --noEmit`, `npm test`, `npm run build`, + `node scripts/check-docs.mjs`, `npm run invariants`, браузерные смоки, + `golden:verify`, `pytest tests_backend`, перфопрофили — diff не касается + `src/**`, `custom_components/**/*.py` ни какого-либо исполняемого артефакта; + ни один AC этого пакета не требует их прогона для стадии spec-review. Зелёный + Validate на этом SHA (ссылка выше) относится к дешёвым гейтам репозитория в + целом, а не является доказательством для конкретных AC C-1…C-8/S1…S3 — эти AC + про будущую реализацию, а не про текущий (докс-only) коммит. +- Не оценивал реализуемость каждой числовой константы (тайминги, пороги солвера, + лимиты хранения) на предмет технической достижимости — все они прямо помечены + в документах как «engineering assumptions, change freely in review» (общий + контракт §9, stage2 §13, stage3 §15), то есть по PROCESS §7.1 это зона, которую + ревьюер вправе оспорить по существу, но не обязан подтверждать формулами; явных + внутренних противоречий в константах не найдено при построчной сверке. +- Не проверял согласованность предлагаемых имён Python/TS модулей с реальными + соглашениями кодовой базы (это тоже явно помечено как «technical choices… + change freely»), кроме одной точечной проверки существования `admin_only`/ + `CONF_ADMIN_ONLY` в `custom_components/houseplan/{auth,const}.py` — подтверждено, + что константа не выдумана. +- Английская версия `docs/USER-GUIDE.md` не сверялась построчно (только + `docs/USER-GUIDE.ru.md` упомянут как источник терминологии в AGENTS.md); ни один + из трёх документов не вводит видимого текста, требующего сверки терминов, кроме + таблиц i18n `radar.*`, которые самодостаточны и не переиспользуют существующие + строки интерфейса не по смыслу — насколько можно судить без реализации. + +## Вердикт + +**Жёлтый.** Оба High отсутствуют; две находки Medium — обе в скоупе задачи и +чинятся правкой самого ТЗ-комплекта (SCOPE.md-исключение + touch-декларация для +этапа 1), без отдельного issue. Комплект в остальном исключительно полон для +своего масштаба: акцептанс-карта, негативные свидетели, совместимость и +продуктовые границы проработаны заметно выше типичного порога DoR. + +--- + + + +## Материал раунда + +- Ветка: `issue/485-radar-presence`, коммит `6550a69e6857` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `56ba25bd189cba041dbfaca20f6717b8cb9bcb14` + ``` + git log --all --format='%H %T' | grep 56ba25bd189c + ``` +- ТЗ `docs/specs/485-radar-presence-stage1.md`, блоб `2e007f56b5d52fb3537eac42ed134f52f27944c5` + ``` + git log --all --find-object=2e007f56b5d52fb3537eac42ed134f52f27944c5 -- docs/specs/485-radar-presence-stage1.md + ``` +- ТЗ `docs/specs/485-radar-presence-stage2.md`, блоб `46178cfa4c26e1c5b48d766a5a5ba47e909158b7` + ``` + git log --all --find-object=46178cfa4c26e1c5b48d766a5a5ba47e909158b7 -- docs/specs/485-radar-presence-stage2.md + ``` +- ТЗ `docs/specs/485-radar-presence-stage3.md`, блоб `bc4e8cd7675f181dc308ec01de5e6a980c34f9f4` + ``` + git log --all --find-object=bc4e8cd7675f181dc308ec01de5e6a980c34f9f4 -- docs/specs/485-radar-presence-stage3.md + ``` +- ТЗ `docs/specs/485-radar-presence.md`, блоб `8b56afbdbb5923e002513a31e05a83563e5d0c02` + ``` + git log --all --find-object=8b56afbdbb5923e002513a31e05a83563e5d0c02 -- docs/specs/485-radar-presence.md + ```