docs: review document for #602

Issue: #602
User-Visible: no

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

@@ -0,0 +1,283 @@
+# SPEC-REVIEW-602-r1
+
+**Issue:** [#602](https://github.com/Matysh/houseplan-card/issues/602) — «Полировка редизайна диалогов пространства и устройства»
+**Этап:** ТЗ на ревью (PROCESS.md §2.4)
+**Трек:** full (аналитика #602 называет нарушенные критерии `small`: больше одной поверхности, новый UX-контракт radar-toggle, отдельная touch/mobile проверка — обоснование корректно)
+**Заход:** r1 · блокирующих циклов израсходовано 0 из 4
+
+Ревьюер работает без контекста реализации, материал — тело issue (раздел `## ТЗ`) и три комментария (аналитика, вопросы владельцу, решения владельца).
+
+---
+
+## Скоуп проверки
+
+ТЗ описывает точечную полировку четырёх диалогов на общем form-kit («Настройки
+пространства», «Общие настройки», «Настройки комнаты», «Настройки устройства»):
+удаление избыточных подписей/статусов, выравнивание переключателя и числовых
+полей шкал, однострочные футеры с icon-only деградацией на узких экранах,
+перенос настройки climate-температуры и замену кнопки объявления radar на
+form-kit toggle с раскрытием настроек под ним.
+
+Проверялось:
+
+1. Соответствие `docs/SCOPE.md` (персона/job) — task служит J4/J6, поверхность
+   редакторская, персона — Home admin.
+2. Полнота обязательных разделов ТЗ по PROCESS.md §7.1.
+3. Однозначность и проверяемость AC1–AC16.
+4. Заземление фактических утверждений ТЗ (аналитика + §6) в реальном коде —
+   не выданы ли догадки за факты.
+5. Согласованность с `docs/USER-GUIDE.ru.md` (терминология, уже
+   задокументированное поведение) и с `docs/TOUCH-SUPPORT.md` (мобильный
+   контракт редакторов).
+6. Процесс: корректно снятый `blocked`, зафиксированные решения владельца по
+   Q1/Q2, метки (`bug`, `P2`, `polish`, `S4-spec-review`, без `small`/`trivial`
+   — согласуется с выбором полного трека).
+
+## Как проверялось
+
+Ревью велось состязательно: каждое фактическое утверждение ТЗ и аналитики
+сверялось с исходником, а не принималось на веру.
+
+- `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` (§1–§9) — прочитаны целиком.
+- `docs/TOUCH-SUPPORT.md` — прочитан целиком (мобильный контракт редакторов,
+  правило «Touch editor: …» из раздела Documentation rule).
+- `docs/USER-GUIDE.ru.md` — прочитаны разделы про футер/`dialog.unsaved`
+  (строки 395–414), про radar (1158–1179), про climate-бейдж (1365–1383).
+- i18n: подтверждено существование и точный текст ключей
+  `space.title_hint`, `space.scale_hint`, `marker.name_hint`,
+  `dialog.unsaved`, `dialog.review_fields`, `radar.declare`,
+  `dialog.discard_title/confirm/keep` в `src/i18n/settings/ru.json` и
+  `src/i18n/ru.json`.
+- Код диалогов: `src/styles/form-kit.styles.ts` (геометрия тумблера, `4em`/
+  `5.8em` полей, `padding-right: 122px`/`125px` подписей шкалы),
+  `src/editors/marker-dialog.ts`, `src/editors/radar-section.ts`,
+  `src/radar-editor.ts`, `src/editors/room-settings-dialog.ts`,
+  `src/editors/general-settings-dialog.ts`, `src/hp-confirm.ts` — прочитаны
+  целиком/выборочно по каждому пункту §6 ТЗ.
+- Предшественники: тела issue #591, #600 (через `gh issue view`) — сверка,
+  что мобильный контракт диалогов (320/390/480/560/768/1280/1920,
+  «touch остаётся best effort по SCOPE») уже установлен в #600 и не вводится
+  заново.
+- Поиск прецедентов формулировки «Touch editor: …» по корпусу
+  `docs/reviews/*` (`SPEC-REVIEW-563-r1.md`, `SPEC-REVIEW-454-r1.md`) —
+  чтобы откалибровать, когда эта метка обязательна.
+- Файловая проверка: `docs/CHANGELOG.md`, `docs/CHANGELOG.ru.md`,
+  `docs/USER-GUIDE.md` существуют; корневого `CHANGELOG.md` в репозитории
+  нет (`ls` вернул «No such file or directory»).
+- Гейты кода в этом раунде не гонялись: этап — ревью ТЗ, продуктовый код не
+  менялся, для стадии `S4-spec-review` PROCESS.md §8/§10.4 не требует
+  прогона `typecheck`/`test`/`build` — они относятся к код-ревью (§2.7) и
+  реализации (§2.6). Это явное решение, а не пропуск: гонять их не по чему,
+  диапазон коммитов на ветке кода не менялся.
+
+## Находки
+
+### Medium (в скоупе) — неоднозначность «уже настроенного» radar-toggle
+
+**Файл:** тело issue #602, раздел `## ТЗ` §6.6 (радар toggle).
+
+**Формулировка ТЗ:** «Включение создаёт/возвращает radar draft и сразу
+раскрывает radar-настройки непосредственно под тумблером в «Дополнительных
+действиях»… **Для уже настроенного радара тумблер отображается включённым.**
+Выключение сворачивает настройки и ставит `radarRemove=true`; фактическое
+удаление происходит только после Save.»
+
+**Что проверено по коду.** `renderRadarSection` (`src/editors/radar-section.ts`)
+рендерит toggle/кнопку `radar.declare` только при
+`manualEntry = !savedUnsupported && bindingMode !== 'virtual' && (!d.radar || d.radarRemove) && !recognition.eligible`
+(строка 77). `recognizeRadar()` (`src/radar-editor.ts:105-127`) для ЛЮБОГО
+валидного сохранённого radar-конфига (независимо от того, был ли он объявлен
+вручную через `radar.declare` или распознан по модели устройства) возвращает
+`eligible: true, reason: 'saved'` — единственный признак, отличающий
+auto-recognized от manually-declared, это **значение профиля** (`profile`),
+а не отдельное поле происхождения: в схеме `MarkerRadar`/`RadarEditorDraft`
+такого поля нет (`src/radar-editor.ts:7-49`, `src/radar-model.ts`).
+
+Следствие: как только у устройства **уже есть сохранённый** radar-конфиг
+(любого происхождения), `recognition.eligible` = `true` → `manualEntry` = `false`
+→ ветка `placement === 'additional'` возвращает `html\`\`` (ничего) → диалог
+показывает не toggle, а старую карточку `<fieldset>` с `legend`
+`radar.title` («Присутствие на плане») прямо в «Основных параметрах» —
+то есть **ровно то поведение, которое задача называет дефектом** (форма
+радара не там, где ожидает пользователь), просто для другого триггера
+(повторное открытие диалога, а не текущая сессия объявления).
+
+**Не хватает:** явного ответа, что означает «уже настроенный» —
+(a) радар, объявленный **в рамках текущей открытой сессии диалога**
+(`d.radar` уже создан через `begin()`, Save ещё не нажат) — тогда для этого
+не нужна миграция данных, но текущая реализация `manualEntry` (условие
+`!d.radar`) должна измениться так, чтобы toggle не исчезал сразу после
+создания драфта, иначе он «мигает» и снова показывает форму сверху — это,
+похоже, и есть цель задачи, но словами о «правках `manualEntry`» ТЗ не
+говорит и оставляет это угадывать; или
+(b) радар, **сохранённый на сервере в прошлой сессии** и подгруженный при
+повторном открытии диалога — тогда для manually-declared радаров нужно
+различать происхождение (например, по `profile !== 'esphome_ld2450_v1'`),
+чтобы не сломать уже принятую и не входящую в скоуп («не входит:
+…распознавания поддерживаемых моделей радара», §5) карточку `radar.title`
+для по-настоящему распознанных устройств LD2450. Такое различение — решение,
+видимое пользователю (что именно он увидит, открыв диалог заново), а не
+техническая деталь: PROCESS.md §7.1 относит подобные вопросы к продуктовым.
+
+**Почему это не High.** ACn (AC11, AC12) описывают только поведение внутри
+одной открытой сессии (создание/восстановление/удаление до Save) и
+проходят проверку независимо от выбранной трактовки; сценарий повторного
+открытия ранее сохранённого вручную-объявленного радара **не выражен ни
+одним AC явно** — то есть задача не станет невыполнимой при любой из двух
+трактовок, но реализация без уточнения рискует либо (a) не решить часть
+исходной жалобы (форма при повторном открытии всё ещё «не там»), либо (b)
+выйти за заявленный §5 «не входит» и потребовать различения происхождения
+конфигурации, о котором ТЗ не упоминает вовсе.
+
+**Предлагаемая правка (пример, не решение за автора):** одно предложение в
+§6.6 — «Для устройства, у которого radar уже сохранён из предыдущей сессии,
+диалог продолжает использовать существующую карточку «Присутствие на плане»
+без изменений (см. §5); toggle отражает состояние **только внутри текущей
+открытой сессии диалога**, начиная с момента объявления/восстановления
+драфта и до Save/закрытия.» — или явное продуктовое решение в обратную
+сторону, если владелец действительно хочет перенести и уже сохранённые
+вручную-объявленные радары под toggle.
+
+### Low (в скоупе, не блокирует) — неверная пара changelog-файлов
+
+**Файл:** тело issue #602, §10 «Затрагиваемые модули» и §16 «Release
+artifacts».
+
+ТЗ дважды называет пару `CHANGELOG.md` + `docs/CHANGELOG.md`. В репозитории
+корневого `CHANGELOG.md` не существует (`ls CHANGELOG.md` →
+No such file or directory); настоящая пара — `docs/CHANGELOG.md` +
+`docs/CHANGELOG.ru.md` (так и в `AGENTS.md`: «`User-Visible: yes` requires
+edits to **both** changelogs — `docs/CHANGELOG.md` и `docs/CHANGELOG.ru.md`»,
+и в гейте `scripts/validate-commit-provenance.mjs:64`, который проверяет
+именно эти два пути). RU-changelog в списке ТЗ вообще не назван.
+
+**Почему не блокирует.** Коммит-хук/`provenance` job проверяют реальные пути
+(`docs/CHANGELOG.md`, `docs/CHANGELOG.ru.md`) независимо от текста ТЗ —
+механически пропустить обновление RU-changelog не получится, коммит с
+`User-Visible: yes` без него будет отклонён. Опечатка не создаёт риска для
+AC16 (доказательство — «review»), но может на секунду сбить с толку человека,
+сверяющего release-артефакты по тексту ТЗ.
+
+**Решение ревьюера:** снимается с записью — правка тривиальна (заменить
+`CHANGELOG.md` на `docs/CHANGELOG.ru.md` в двух местах), настоящий гейт уже
+защищает от практического следствия; отдельного цикла ради одной опечатки не
+требуется, но автор может поправить текст заодно с ответом на находку выше.
+
+## Что проверено и признано корректным
+
+- **Обязательные разделы §7.1** — все присутствуют: сценарий (§1), что
+  человек видит до/после (§2), проблема (§3), скоуп/не-скоуп (§4/§5),
+  контракт (§6), UX/доступность (§7), данные и миграция (§8), i18n (§9),
+  AC1–AC16 с «Свидетелем» (§11), план автотестов (§12), риски (§14), откат
+  (§15), release-артефакты (§16).
+- **Продуктовые вопросы отделены от технических корректно.** Q1 (icon-only
+  footer на узких экранах) и Q2 (семантика выключения radar-toggle для уже
+  сохранённого радара **внутри той же сессии**) — обе действительно
+  продуктовые («что видит/делает человек»), обе закрыты явным решением
+  владельца, `blocked` снят по факту. Технические детали (i18n-ключи можно
+  оставить или убрать по результатам поиска потребителей; какой модуль
+  держит guard) оставлены автору — соответствует PROCESS.md §7.1.
+- **Ни одна фактическая формулировка ТЗ/аналитики не оказалась догадкой,
+  выданной за факт** — все проверенные утверждения подтвердились чтением
+  кода:
+  - фиксированные ширины `4em`/`5.8em` и `padding-right: 122px/125px`
+    (`src/styles/form-kit.styles.ts:388,421-422,496`) — источник дефектов
+    прозрачности/шкалы назван верно;
+  - `useClimateTemp`-тумблер сейчас в карточке света
+    (`marker-dialog.ts:501-509`, `marker.card_light`) и должен переехать в
+    `marker.card_details` после подраздела `radar.additional_actions`
+    (`marker-dialog.ts:842`, `radar-section.ts:82-89`) — подраздел реально
+    существует под этим именем и рендерится для типичного (не virtual,
+    не сохранённо-неподдерживаемого, не auto-recognized) устройства, в т.ч.
+    climate-bound, поскольку `_bindingHasClimate` (`houseplan-card.ts:5621-5623`)
+    для `binding === 'virtual'` всегда `false` — конфликта climate+virtual
+    не возникает;
+  - разрыв «кнопка в Additional actions → форма в Main params» воспроизведён
+    по коду именно так, как описан в §3 (два вызова `_renderRadarSection` с
+    разным `placement`, `marker-dialog.ts:319,842`);
+  - `hp-confirm` (`src/hp-confirm.ts`) — единственный компонент подтверждений
+    во всём приложении, всегда ровно две кнопки; риск «Confirm используется
+    не только discard» в §14 корректно закрыт этим наблюдением — новый
+    однорядный layout не может сломать иной by-design confirm, потому что
+    другого layout не существует;
+  - все процитированные i18n-строки (`space.title_hint`, `space.scale_hint`,
+    `marker.name_hint`, `dialog.unsaved`, `dialog.review_fields`,
+    `radar.declare`, `dialog.discard_title/confirm/keep`) существуют
+    дословно в `ru.json`/`settings/ru.json`.
+- **Touch/mobile контракт не вводится заново.** Мобильная ширина от 320 px
+  для этих диалогов и статус «touch остаётся best effort по SCOPE» уже
+  зафиксированы в #600 (подтверждено чтением тела #600: таблица ширин
+  320/390/480/560/768/1280/1920 и явная фраза про best effort). #602 —
+  точечное исправление внутри уже принятого контракта, не новая
+  editor-фича; по прецеденту `SPEC-REVIEW-563-r1.md` («задача не вводит
+  новую editor-фичу, поэтому явная метка «Touch editor: …» не требуется»)
+  отдельная декларация `Touch editor: …` не обязательна. Icon-only-переход
+  Copy/Delete — новая деталь по сравнению с #600, но она строго внутри уже
+  описанного «best effort»-контракта и решена продуктовым вопросом Q1, а
+  не угадана.
+- **Область задачи (4 диалога) не раздута.** Room/General settings попали в
+  скоуп не произвольно: оба реально импортируют общие хелперы `toggleRow`/
+  `rangeEnds`/`footerStatus` из `src/editors/form-kit.ts`
+  (`room-settings-dialog.ts:19-22`, `general-settings-dialog.ts:13-16`) —
+  общий CSS-баг действительно задевает все четыре потребителя, объединение
+  оправдано тем же корнем причины, а не удобством.
+- **Соответствие SCOPE.md.** Job — J4/J6, поверхность — только editor-диалоги
+  (админ), View/kiosk не затронуты; лок-инвариант и out-of-scope список не
+  задеты.
+- **Метки/процесс.** `S4-spec-review` без `small`/`trivial` соответствует
+  названному в аналитике отказу от лёгкого трека; `blocked` корректно снят
+  после ответов владельца в третьем комментарии.
+
+## Чего не проверял
+
+- **Golden/скриншот-покрытие Room/General диалогов.** План автотестов (§12
+  п.4) явно называет «минимум» голден-сцен для Space/Device/Confirm, но не
+  называет отдельных сцен для Room/General, хотя оба используют те же
+  `rangeEnds('50%','300%')` (подтверждено: `room-settings-dialog.ts:276`).
+  Это оставлено на усмотрение автора («минимум» допускает добавление) и не
+  формулирует нового продуктового решения — не поднимаю как находку, но
+  ревьюер кода должен свериться, что регресс в Room/General действительно
+  покрыт хотя бы layout-смоком, если не голденом.
+- **Точная реализация центрирования шарика тумблера** (какая именно строка
+  CSS изменится) — не разбирал до пиксельной арифметики box-sizing,
+  поскольку ТЗ не обязано называть implementation-решение, а описывает
+  наблюдаемый результат («визуально центрирован… одинаковые внутренние
+  отступы») — тестируемо через layout-смок и golden, этого достаточно на
+  этапе ТЗ.
+- **Автотесты, гейты `typecheck`/`test`/`build`/`golden:verify` и т.п.** —
+  не запускались: продуктовый код в этом раунде не менялся, предмет
+  ревью — исключительно текст ТЗ. Они относятся к код-ревью (§2.7) после
+  реализации.
+- **Полный текст docs/USER-GUIDE.md (EN)** — сверял только русскую версию
+  построчно по конкретным терминам; английскую версию не читал целиком,
+  считаю её производной (перевод) и не источником терминологии по
+  правилам AGENTS.md.
+
+## Вердикт
+
+High: 0 · Medium: 1 (в скоупе — уточнение семантики «уже настроенного»
+radar-toggle, §6.6) · Low: 1 (в скоупе, снят с запиской — неверная пара
+changelog-файлов в §10/§16).
+
+Оба найденных дефекта чинятся правкой текста ТЗ (одно-два предложения),
+без изменения архитектуры или AC. Medium-находка не позволяет поставить
+зелёный вердикт по правилу §2.4: «Medium в скоупе задачи чинится в текущем
+issue: без High это жёлтый вердикт, автор правит ТЗ, фикс проходит
+повторный цикл.»
+
+**Вердикт: жёлтый · заход r1 · блокирующих циклов 1/4 (полный трек, лимит
+ревью ТЗ — 4) · High: 0 · Medium: 1 → в задаче**
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `dev`, коммит `3701c7bd201d` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Дерево материала: `3fe0f0cd2ce99828382138829f2d4ca0eccc9cce`
+  ```
+  git log --all --format='%H %T' | grep 3fe0f0cd2ce9
+  ```
+- Тело issue: `345487371ab35ce7a540ebb03b9eeb09cda56792a7322d0783036edb5866f78e`
+- Вердикт конвейера: `yellow` · High 0