docs: review document for #29

Issue: #29
User-Visible: no

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

@@ -0,0 +1,191 @@
+# SPEC-REVIEW-29-r1
+
+- Issue: https://github.com/Matysh/houseplan-card/issues/29
+- Этап: spec (PROCESS.md §2.4)
+- Заход: r1 · блокирующих циклов израсходовано 0 из 4
+- ТЗ: `docs/specs/029-device-inbox-lifecycle.md`
+- SHA материала ревью: `a6ce1ae7e534fa4b41fcc0ab93e5ed95a693c4c1` (ветка `issue/29-device-inbox-lifecycle`)
+- Трек: обычный (не `small`), лимит циклов ревью ТЗ — 4.
+
+Это первый заход ревью по этой задаче — предыдущего вердикта в issue нет,
+раздел «Унаследовано» не применяется, разбор полный.
+
+## Скоуп разбора
+
+Проверено:
+
+1. `docs/SCOPE.md` — соответствие job'ам J4/J6, отсутствие выхода за
+   продуктовую рамку (out-of-scope список, standing rules).
+2. `AGENTS.md` и `PROCESS.md` §2.4, §7.1 — формальные требования к ТЗ и
+   формат вердикта.
+3. Тело issue #29 и все комментарии, включая повторную актуализацию анализа
+   от 2026-08-28 и решение владельца по Q1.
+4. `docs/USER-GUIDE.ru.md` (терминология «Скрытые и деактивированные»,
+   «Редактор устройств», раздел про скрытие/удаление) и `docs/FILTERING.md`
+   (канонический контракт hidden/removed/tombstone/entity-ownership).
+5. Исходный код: `src/houseplan-card.ts` (`_bindingCandidates`,
+   `new_device_ids`, `_maybeRebuildDevices`, `.slice(0, 200)` дважды),
+   `src/ha-binding-status.ts` (`HaBindingStatus.kind`), `src/types.ts`
+   (`marker.hidden`, `marker.removed`), `src/i18n/ru.json` и `en.json`.
+6. Существующие тесты, упомянутые в AC11/§16: `test/devices.test.mjs`,
+   `test/ha-binding-status.test.mjs`, `test/device-presentation*.test.mjs`,
+   `demo/smoke_hidden_flag.mjs`, `demo/smoke_binding_picker.mjs` —
+   существуют, содержимое `smoke_hidden_flag.mjs` прочитано целиком.
+7. `docs/CONFIG-COMPATIBILITY.md` — беглая проверка на отсутствие
+   противоречий заявлению «нет новых persisted-полей».
+
+Ревью — на этапе ТЗ: продуктового кода нет, автотесты не запускались (их
+ещё не существует), гейты §8 к этому этапу не относятся.
+
+## Метод проверки
+
+Каждое фактическое утверждение ТЗ о текущем поведении продукта
+(«До», §3, §7, §8, §9, §10.1) сверено с кодом или каноническим документом, а
+не принято на слово автора. Отдельно проверено, есть ли в тексте
+утверждение о поведении, которого нет ни в одном документе и которое не
+помечено как предположение (раздел 21 ТЗ).
+
+## Находки
+
+### M1 (Medium, в скоупе) — исчезновение призрачного показа скрытых/HA-disabled маркеров на плане не названо как продуктовое решение
+
+**Файл:** `docs/specs/029-device-inbox-lifecycle.md`, §10.1 (строки 216–222).
+
+Сегодня редактор устройств может показать скрытые и HA-disabled маркеры
+прямо на плане, в их реальной позиции, призраками — режим переключается
+кнопкой «Скрытые и деактивированные» (`docs/FILTERING.md` строки 84–91,
+`docs/USER-GUIDE.ru.md:178` — «доступны скрытые маркеры», перетаскиваются;
+подтверждено в `demo/smoke_hidden_flag.mjs`, где ghost рендерится в
+`_setMode('devices')` с `_showHidden = true` и клик по нему открывает
+диалог). Это единственный способ увидеть, ГДЕ на плане сидит скрытое или
+деактивированное устройство, не отменяя его скрытость.
+
+ТЗ прямо убирает этот режим: «Скрытые markers больше не рисуются поверх
+плана постоянным локальным режимом: доступ к ним даёт каталог» (§10.1).
+Но каталог (§10.2–10.4) не даёт эквивалента: `Find` явно доступен «только
+если marker реально отрисовывается» (§10.4), а для строк категории
+«Скрытые» и для «На плане, временно не отображается из-за HA status»
+primary/secondary действия — это «Показать»/«Настроить», не позиция на
+плане. Значит, чтобы увидеть, где стоит скрытый маркер, администратору
+придётся сначала его показать (что меняет конфиг), посмотреть, и при
+необходимости скрыть обратно — вместо непосредственного просмотра.
+
+Это видимое пользователю изменение объёма функциональности («какая
+персона что видит и делает» — ровно тот класс вопросов, который согласно
+PROCESS.md §7.1 задаётся владельцу или явно фиксируется как принятое
+предположение в §21). В тексте ТЗ оно подано как самоочевидный
+технический побочный эффект объединения кнопок, а не как решение,
+которое можно оспорить: обоснование «устраняет состояние панели,
+неочевидное после возврата в редактор» — это плюс нового дизайна, но оно
+не адресует потерю прямого просмотра позиции.
+
+**Не High**, потому что: обходной путь существует (Показать → посмотреть/
+перетащить → Скрыть), это не потеря данных и не поломка AC — просто
+непроверенное продуктовое допущение, которое дёшево закрыть на этом этапе.
+
+**Как закрыть в этом же цикле (на выбор автора):** либо явно вынести это в
+блок §21 как предположение, которое ревьюер/владелец может оспорить, с
+описанием обходного пути; либо задать это владельцу одним пакетным
+вопросом с предложенным дефолтом (например: «Find для скрытой/disabled
+строки временно подсвечивает позицию на плане не снимая hidden» как
+альтернативный дизайн); либо сознательно сохранить упрощённый вариант, но
+явно назвать компромисс и обходной путь в §2 «До/После» и в §18 «Риски».
+
+### M2 (Medium, в скоупе) — устаревающая строка i18n не включена в план обновления
+
+**Файлы:** `src/i18n/ru.json:673`, `src/i18n/en.json:673` (`marker.hide_tip`).
+
+Текущий текст подсказки при скрытии маркера дословно ссылается на кнопку,
+которую это ТЗ удаляет: RU — «Вернуть его можно через кнопку «Скрытые и
+деактивированные» в редакторе устройств»; EN — `Restore it through "Hidden
+and disabled" in the device editor`. §10.1 заменяет обе кнопки («Добавить»
+и «Скрытые и деактивированные») одной кнопкой «Устройства» (`devbar.add`,
+`devbar.show_all` перестают существовать в UI в текущем виде).
+
+Раздел 14 (i18n) перечисляет только **новые** ключи и не содержит пункта
+«обновить существующие ключи, ссылающиеся на удаляемые элементы
+интерфейса». Если реализовать ТЗ как написано, `marker.hide_tip` останется
+нетронутым и после релиза будет указывать пользователю нажать
+несуществующую кнопку — конкретный, проверяемый дефект, а не гипотетический.
+
+**Как закрыть:** добавить в §14 явный пункт «`marker.hide_tip` (en+ru)
+обновляется, чтобы указывать на новую точку входа «Устройства»» (и
+проверить, нет ли других строк с той же ссылкой — быстрый `grep` по
+`show_all`/«Скрытые и деактивированные» в `src/i18n/*.json` показывает,
+что это единственная пара строк такого рода, кроме самих `devbar.*`
+/`title.show_all`, которые и так меняются по §10.1).
+
+Обе находки Medium, в скоупе задачи (правки в самом файле ТЗ) — по
+PROCESS.md §2.4/§2.7 они не создают отдельный issue (решение владельца
+2026-08-19, #202) и возвращают ТЗ автору с жёлтым вердиктом.
+
+## Что проверено и признано корректным
+
+- **Формальные разделы §7.1** — сценарий, «что человек увидит до/после»,
+  проблема, скоуп/не-скоуп, контракт поведения, UX, модель данных и
+  миграция, i18n, AC1…AC11 с указанием способа доказательства, план
+  автотестов, риски, откат, release-артефакты — все присутствуют.
+- **AC1–AC11** однозначны и у каждого назван способ доказательства
+  (unit/smoke/golden/review); ни один не описывает недоказуемое поведение.
+- **Q1 владельца** (классификация автообнаруженного видимого устройства
+  без marker) корректно отражена в §7.2 (категория `on_plan`) и в AC3 —
+  соответствует решению владельца в комментарии issue.
+- **Заявления о текущем поведении, использованные как база для «До»,
+  подтверждены кодом и канонoм**, а не додуманы:
+  - `_bindingCandidates` в `src/houseplan-card.ts:14032` — существующий
+    eligibility-код, который ТЗ предлагает извлечь в общий helper (§12);
+    не выдумка.
+  - `HaBindingStatus.kind` (`active`/`ha_disabled`/`orphaned`/`unverified`)
+    в `src/ha-binding-status.ts:14-17` — точное совпадение с таблицей §7.3.
+  - `marker.hidden`/`marker.removed` в `src/types.ts:118,121` — существуют,
+    семантика совпадает с `docs/FILTERING.md`.
+  - Жёсткий кап на 200 элементов в существующих списках кандидатов
+    (`src/houseplan-card.ts:14106`, `19114` — `.slice(0, 200)`) —
+    подтверждает описанную в §8.1/AC9 проблему, которую каталог обязан не
+    унаследовать.
+  - `duplicate_name_area` действительно устарела: `docs/FILTERING.md:145`
+    прямо говорит «Duplicate names are still numbered», не скрываются.
+  - `settings.new_device_ids` фильтруется от уже скрытых id при seed
+    (`src/houseplan-card.ts:3679–3684`) — подтверждает §8.2 «Первично
+    отфильтрованный скрытый кандидат не получает badge, как и сегодня».
+  - Терминология «Добавить», «Скрытые и деактивированные», «Правила
+    иконок» — точное совпадение с `src/i18n/ru.json:455-457` и
+    `docs/USER-GUIDE.ru.md`.
+- **Технические предположения (§21)** промаркированы явно и корректно
+  отделены от продуктовых решений — кроме пробела, описанного в M1.
+- **Не входит (§6)** корректно исключает #44 (discovery-настройки), #126
+  (смена area), #109 (multi-channel), bulk-операции, историю/графики,
+  touch-паритет — всё по `docs/SCOPE.md` и `docs/TOUCH-SUPPORT.md`, без
+  расширения скоупа.
+- **Откат (§19)** реалистичен: нет новых persisted-полей и версии модели,
+  откат — вернуть старые кнопки; совместимость с созданными через «Скрыть
+  из списка» обычными `hidden:true` маркерами сохраняется.
+- Названные тестовые артефакты существуют: `test/devices.test.mjs`,
+  `test/ha-binding-status.test.mjs`, `test/device-presentation.test.mjs`,
+  `test/device-presentation-policy.test.mjs`, `demo/smoke_hidden_flag.mjs`,
+  `demo/smoke_binding_picker.mjs`, `scripts/smoke-select.mjs` — ни один не
+  является выдумкой.
+
+## Чего не проверял
+
+- Не запускал никакие гейты (`typecheck`/`test`/`build`/смоки) — на этапе
+  ТЗ продуктового кода нет, это не применимо (§8 относится к
+  код-ревью).
+- Не проверял `docs/ARCHITECTURE.md` и `docs/CANVAS.md` целиком постранично
+  — только точечно то, что касается заявленных в ТЗ архитектурных решений
+  (pure resolver, eligibility helper); полный аудит этих документов не
+  требуется для ревью ТЗ.
+- Не оценивал производительность реализации (её ещё нет); проверил только,
+  что заявленная асимптотика (§12, §17) не противоречит существующему коду
+  (`.slice(0, 200)` факт капа подтверждён).
+- Не проверял golden/скриншот-инфраструктуру предметно — она релевантна
+  коду, а не ТЗ.
+
+## Вердикт
+
+Полностью выполненные формальные требования ТЗ (§7.1) не отменяют жёлтый
+вердикт: две находки Medium в скоупе (M1, M2) — реальные пробелы, оставить
+их «в тексте ревью» без правки запрещено §12 PROCESS.md. High-находок нет.
+
+**Вердикт: жёлтый.** ТЗ возвращается автору на правку M1 и M2; после
+правки — новый заход ревью по дельте (PROCESS.md §2.10).