docs: spec review for #210

Issue: #210
User-Visible: no

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

@@ -0,0 +1,212 @@
+# Ревью ТЗ — issue #210, цикл r1
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/210
+- **ТЗ:** `docs/specs/210-fixed-floor-card.md` (коммит `0bd6094`, ветка
+  `issue/210-fixed-floor`)
+- **Трек:** обычный (аналитика явно исключила `small`/`trivial`; сложность
+  6/10, риск 6/10, метка `small` на issue отсутствует)
+- **Вердикт:** жёлтый · цикл r1/4 · High: 0 · Medium: 1 (в скоупе задачи) ·
+  Low: 1
+
+## Скоуп ревью
+
+Проверялось ТЗ `docs/specs/210-fixed-floor-card.md` как артефакт этапа
+`S4-spec-review`: обязательные разделы §7.1 PROCESS.md, однозначность и
+доказуемость AC1…AC12, соответствие `docs/SCOPE.md`, `docs/UX-MODES.md`,
+`docs/TOUCH-SUPPORT.md`, `docs/CONFIG-COMPATIBILITY.md` и
+`docs/USER-GUIDE.ru.md`, а также фактическая проверка причинных утверждений
+ТЗ (§3, §7) против текущей реализации в `src/initial-load.ts`,
+`src/houseplan-card.ts` и `src/editor.ts` — то есть не пересказ автора, а
+независимое чтение кода, который ТЗ описывает как «до». Продуктового кода по
+#210 не существует (issue в `S4-spec-review`, диапазон коммитов веток —
+только документация), поэтому гейты §8 PROCESS.md к этому циклу неприменимы.
+
+## Как проверялось
+
+1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` целиком.
+2. Прочитано тело issue #210 и все три комментария (аналитика,
+   «взял в работу», «ТЗ готово»).
+3. Прочитан файл ТЗ `210-fixed-floor-card.md` целиком (327 строк).
+4. Прочитаны `docs/CONFIG-COMPATIBILITY.md` и `docs/UX-MODES.md` целиком, а
+   также `docs/TOUCH-SUPPORT.md` целиком — как канонические документы
+   затронутых подсистем (навигация, конфигурация, touch/kiosk-контракт).
+5. Прочитан `docs/USER-GUIDE.ru.md` в месте, описывающем `default_floor` и
+   несколько карточек (строки 122-125, 1327-1339) — подтверждает, что именно
+   это описание текущего (ошибочного) поведения ТЗ обязано заменить/уточнить
+   в release-артефактах (§15 ТЗ это учитывает).
+6. Независимо сверены причинные утверждения ТЗ §3/§7 с кодом:
+   - `resolveInitialSpace()` (`src/initial-load.ts:27-42`) — подтверждён
+     буквальный холодный приоритет `hash → current → saved → default → first`;
+   - общий ключ `houseplan_card_nav_v1` (`LS_NAV`, `src/houseplan-card.ts:482`,
+     `_savedNav`/`_saveNav` :5866-5881) — подтверждён как единственный,
+     без per-instance scoping;
+   - `_warmAdoptViewport()` (`src/houseplan-card.ts:2625-2644`) — подтверждено
+     восстановление пространства из viewport-memo поверх saved/default;
+   - множественные прямые присваивания `this._space = …` вне единой точки
+     (grep дал >15 мест: :1150, :1188, :1820, :2430, :2635, :2925, :3179,
+     :13522, :13531, :13573, :13644, :14297, :16459, :16487 и др.) —
+     подтверждает риск «новый guard пропустит прямое `_space =`» из таблицы
+     §14 ТЗ как реальный, а не гипотетический;
+   - kiosk swipe (`src/houseplan-card.ts:5405`) и cycle (`:1191-1193`,
+     `:1951-1953`) — подтверждены как условные пути, которые можно
+     дополнительно закрыть проверкой fixed-authority без переписывания их
+     структуры;
+   - GUI editor (`src/editor.ts:42-73`) — подтверждён текущий паттерн
+     `default_floor` (dropdown со stable ID при доступных `spaces`, textbox
+     fallback при недоступности WS) как прямой прецедент для нового поля
+     `floor`, на который ссылается §6.2 ТЗ.
+7. Проверено соответствие `docs/SCOPE.md`: задача закрывает возможность
+   «Home admin» держать несколько выделенных fullscreen/kiosk-панелей по
+   этажам без взаимного вмешательства навигации — часть уже закрытых J1/J6
+   («live spatial overview», «keep the plan true», несколько карточек с
+   собственным стартовым этажом уже документировано как поддерживаемый
+   сценарий в `USER-GUIDE.ru.md:1327-1332`); задача не расширяет и не
+   нарушает ни одну строку scope.
+8. Код не запускался, гейты не прогонялись — на этапе ревью ТЗ продуктового
+   кода нет.
+
+## Находки
+
+### M1 — контракт не говорит явно, что очистка `floor` в GUI должна удалять ключ, а не записывать невалидную пустую строку
+
+**Файл:** `docs/specs/210-fixed-floor-card.md:101-111` (§6.2), в противоречии
+с `:92` (§6.1, п.3) и AC8 (`:239`)
+
+§6.1 п.3 явно объявляет пустую строку невалидным значением `floor` — при её
+наличии в конфиге карточка обязана уйти в fail-closed error state (§9, AC3:
+«Unknown ID и все невалидные типы/числа fail closed… без чужого
+плана/fallback»). При этом §6.2 описывает GUI-контроль как «dropdown со
+stable ID и пустым вариантом «не закреплять»», а AC8 требует, чтобы GUI «умел
+очистить `floor`» — но ни §6.2, ни §7, ни AC8 не говорят явно, **что именно**
+происходит в конфиге при выборе этого пустого варианта: должен ли
+`floor`-ключ удаляться из объекта конфигурации целиком (что вернёт legacy
+navigation, как того требует «не закреплять»), или контрол просто запишет
+`floor: ''` (что по буквальной формулировке §6.1 п.3 является невалидным
+значением и включит error state).
+
+Это не абстрактный риск: `_valueChanged()` в текущем `src/editor.ts:99-104`
+делает `{ ...this._config, ...ev.detail.value }` — то есть механически
+переносит то, что вернул `ha-form`, без специальной зачистки пустых полей;
+никакого существующего прецедента «optional selector с явной очисткой ключа»
+в этом редакторе сейчас нет (`default_floor` never being cleared — у него нет
+пустого варианта). Если реализация просто повторит текущий паттерн, самое
+частое и ожидаемое админом действие — «я больше не хочу закреплять этот
+экран» — приведёт не к обычной навигации, а к видимой ошибке ровно того типа,
+который вся задача обязана устранить.
+
+**Почему это Medium, а не High:** ни один AC не сформулирован неверно и не
+описывает ошибочное поведение как целевое — пробел чисто в определении, какую
+именно мутацию конфигурации должен произвести один конкретный UI-контрол;
+это устраняется одной уточняющей фразой в тексте ТЗ, не меняя ни архитектуру,
+ни один AC, ни скоуп. Находка в скоупе собственного AC8 задачи — правится в
+текущем ТЗ.
+
+**Требуется:** явно зафиксировать в §6.2 (и, для симметрии, в §16 как принятое
+техническое решение), что выбор пустого варианта удаляет ключ `floor` из
+конфигурации целиком, а не заменяет его на пустую строку/`null` — то есть
+GUI обязан обеспечивать инвариант §6.1 «отсутствует, только если свойства нет
+в конфиге» на собственном пути записи, а не полагаться на то, что позже это
+поле случайно совпадёт с одним из уже описанных invalid-путей.
+
+### L1 — обязательный раздел «i18n» присутствует по содержанию, но не оформлен отдельным заголовком
+
+**Файл:** `docs/specs/210-fixed-floor-card.md` (содержание разбросано по
+§6.2 — подписи полей GUI на двух языках, и §9 — требования к тексту ошибки)
+
+PROCESS.md §7.1 перечисляет i18n отдельным обязательным разделом ТЗ. Формально
+такого заголовка в документе нет: русские/английские подписи полей GUI даны
+прямой строкой в §6.2 («Fixed space / Закреплённое пространство», «Initial
+space / Стартовое пространство»), а требования к содержимому error-текста —
+в §9, без перечисления конкретных ключей `en.json`/`ru.json`. Это не создаёт
+двусмысленности для реализации (именование ключей — техническая деталь,
+которую агенты решают сами, PROCESS.md §7.1) и не блокирует ни один AC, но
+формально отклоняется от структуры, которую требует процесс, и усложняет
+беглую проверку «оба языка учтены» без чтения всего документа целиком.
+
+Не блокирует. Снимается на решение автора — либо выделить короткий
+одноимённый раздел (даже как ссылку на уже написанные §6.2/§9), либо оставить
+как есть с этой пометкой в документе ревью.
+
+## Проверено и признано корректным
+
+- **Причина дефекта (§3)** — построчно подтверждена кодом: единый
+  `houseplan_card_nav_v1`, приоритет `resolveInitialSpace()`, влияние
+  `_warmAdoptViewport()`; это не пересказанная, а самостоятельно
+  верифицированная причинно-следственная цепочка.
+- **Обязательные разделы §7.1 PROCESS.md** — все присутствуют по содержанию:
+  сценарий (§1), что видит пользователь до/после (§2, без терминов
+  реализации), проблема (§3), скоуп/не-скоуп (§4/§5), контракт поведения
+  (§6-§9), UX (§8), данные/совместимость/безопасность (§10), архитектурный
+  контракт (§11), AC1…AC12 с доказательством для каждого (§12), план
+  автотестов (§13), риски и откат (§14), release-артефакты (§15).
+- **Продуктовая часть контракта не содана как догадка**: аналитика (комментарий
+  владельца) прямо сказала «продуктовых вопросов по заданному контракту нет»,
+  и §16 «Принятые предположения» корректно отделяет технические решения (7
+  пунктов) от уже отвеченного владельцем продуктового контракта, помечая
+  каждое как решение автора, открытое для оспаривания ревьюером — ни одно не
+  подано как установленный факт без основания.
+- **Предположение №1 (§16)** — расширение «floor выше всех источников
+  навигации» на URL hash, хотя исходная формулировка issue перечисляла только
+  saved nav/tabs/swipe/cycle — признано обоснованным: это техническая деталь,
+  которую issue прямо разрешает решать на этапе ревью («Technical details are
+  assumed and may change freely»), и оно последовательно с продуктовым
+  утверждением «After: … не участвует в общей истории выбора этажей».
+- **Риск «новый guard пропустит прямое `_space =`»** (§14 таблицы) —
+  подтверждён как реальный, не гипотетический: код действительно содержит
+  более 15 разрозненных прямых присваиваний `this._space =`; выбранная мера
+  («инвентаризация всех assignments + единая mutation boundary», §11) —
+  адекватный ответ.
+- **Соответствие `docs/TOUCH-SUPPORT.md`** — §8 несёт буквальный канонический
+  тег `Touch editor: best effort / intentionally degraded.`, требуемый
+  «Documentation rule»; удаление kiosk swipe/cycle/dots для fixed-инстанса не
+  нарушает гарантию «View/kiosk — fully supported touch surface», поскольку
+  pan/pinch/zoom и безопасные действия текущего пространства прямо сохранены,
+  а «переключение между этажами» перестаёт быть релевантным действием там,
+  где второго этажа для переключения не предусмотрено конфигурацией.
+- **Соответствие `docs/UX-MODES.md`** — правило «Header в View: только
+  вкладки пространств, счётчик устройств, zoom-кластер» и принцип «редакторы
+  не просачиваются в View» не нарушаются: §8 ТЗ убирает лишние
+  навигационные элементы (другие вкладки, Add, kiosk dots), но не трогает
+  editors/gear текущего пространства и не вводит новых интерактивных
+  поверхностей.
+- **`docs/CONFIG-COMPATIBILITY.md`** — корректно не упомянут как файл для
+  правки: `floor` — Lovelace card config, а не персистентное серверное поле
+  House Plan (§10 ТЗ), и уже существующие card-level опции (`default_floor`,
+  `cycle`, `kiosk`) в этом реестре также не зарегистрированы — задача не
+  создаёт исключения из этого паттерна.
+- **AC1…AC12** — каждый однозначен и указывает способ доказательства
+  (unit/browser smoke/DOM assertions/config refresh test), включая AC12
+  («новый guard доказан failing-before-fix») — это прямое выполнение
+  требования code review «тест умеет падать» уже на этапе постановки ТЗ.
+- **Совместимость (§10, §16 п.6)** — отсутствие `floor` не меняет ни один
+  существующий путь навигации; `default_floor` остаётся ровно тем полем,
+  которое описывает действующий `USER-GUIDE.ru.md`; невалидный `floor` прямо
+  не откатывается на `default_floor`, что снимает риск «молча показать чужой
+  этаж» — именно то, из-за чего заведён issue.
+- **Откат (§14)** — единый revert без миграции хранилища; отсутствие поля
+  на старом фронтенде корректно описано как «fixed guarantee отсутствует», а
+  не как повреждение данных.
+
+## Чего не проверял
+
+- **Фактическая работоспособность будущей реализации** — на этапе ревью ТЗ
+  продуктового кода не существует; проверка ограничена текстом ТЗ и текущим
+  («до») кодом.
+- **`ha-form`/`ha-selector` внутреннее поведение при очистке optional select**
+  не тестировалось эмпирически (нет собранной карточки с новым полем) — M1
+  сформулирована как пробел в тексте ТЗ, а не как утверждение о конкретном
+  поведении HA-компонента.
+- **`demo/smoke_fixed_floor.mjs`, `smoke_nav_persist.mjs`, `smoke_kiosk.mjs`**
+  не запускались — файл нового смока пока не существует (создаётся в
+  реализации), а существующие смоки не относятся к этому циклу (нет кода для
+  проверки).
+- Гейты `typecheck`/`test`/`build`/smoke/golden не прогонялись — на этапе
+  ревью ТЗ раздел неприменим (PROCESS.md §8: гейты код-ревью, не ТЗ-ревью).
+
+## Итог
+
+Блокирующих находок нет. Единственный Medium (M1) устраняется точечной
+правкой текста §6.2 (и, по желанию автора, зеркальным пунктом в §16) — без
+изменения архитектуры, скоупа или какого-либо AC. Low (L1) — на решение
+автора, можно снять с запиской без правки. Вердикт — жёлтый, возврат автору
+в рамках текущего issue.