docs: spec review document for #131 (r1, green)

Issue: #131
User-Visible: no

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

@@ -0,0 +1,226 @@
+# SPEC-REVIEW-131-r1
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/131
+- **ТЗ под ревью:** `docs/specs/131-readonly-cold-start.md` (коммит `0164e65`)
+- **Роль:** ревьюер ТЗ (не автор), этап `S4-spec-review`
+- **Трек:** обычный (не `small`/`trivial`) — владелец явно принял D1/D2
+  (оценка сложности 4/10, но затрагивает async boot, cache/navigation
+  precedence, reload и warm continuity; лёгкий трек корректно не применён)
+- **Цикл:** r1/4
+
+## Скоуп ревью
+
+Проверялось соответствие ТЗ:
+
+- `docs/SCOPE.md` — попадание в Core user jobs, отсутствие расширения скоупа,
+  сохранение lock-инварианта и правила «никогда не удалять файл по догадке»
+  (задача файлов не касается);
+- `PROCESS.md` §2.4, §2.5 (DoR), §7.1 (обязательные разделы), §12 (запреты);
+- `AGENTS.md` — классы файлов (класс A: `src/houseplan-card.ts`; класс B:
+  тесты/demo; класс C: документация/changelog), ветка `issue/131-readonly-cold-start`,
+  трейлеры;
+- `docs/TOUCH-SUPPORT.md` — View/kiosk как гарантированные touch-поверхности;
+- `docs/USER-GUIDE.ru.md` — терминология «пространство», «вкладка», «киоск»;
+- фактическому состоянию кода `src/houseplan-card.ts` — на предмет того, что
+  технические утверждения ТЗ (диагноз причины, поведение hash/`LS_NAV`/warm
+  viewport, no-op клика активной вкладки) не являются непроверенной догадкой,
+  а описывают код, который действительно существует.
+
+## Как проверялось
+
+1. Прочитан весь тред issue #131: исходный отчёт с двумя скриншотами,
+   аналитика S2 (таблица диагностики на `dev` SHA `0e69c4a18337`, два
+   пространства `home`/`upstairs`), решение владельца о defaults D1–D3
+   (https://github.com/Matysh/houseplan-card/issues/131#issuecomment-5287280361),
+   комментарий «Взял» и комментарий «ТЗ готово».
+2. Сверены обязательные разделы ТЗ (§7.1 `PROCESS.md`) построчно — таблица ниже.
+3. Прочитан `setConfig()` (`src/houseplan-card.ts:2258-2321`): подтверждено, что
+   `default_floor` и cache-приоритет (`hash → LS_NAV → default_floor → model[0]`)
+   применяются здесь только когда есть валидный `LS_CFG`; без кэша `_space`
+   остаётся дефолтным `'f1'` до серверного ответа — совпадает с §3 ТЗ.
+4. Прочитан `_loadFromServer()` (`src/houseplan-card.ts:3193-3316`): подтверждено
+   дословно то, что описывает §3/§8 ТЗ —
+   - `_adoptStructuralResponses()` (принятие config/layout/`can_write`) вызывается
+     **до** трёх последовательных `await this.hass.connection.subscribeEvents(...)`
+     (`houseplan_config_updated`, `houseplan_trail_updated`, `houseplan_layout_updated`);
+   - выбор `_space` по hash/`LS_NAV`/`_norm`-fallback и `_cacheSnapshot()`
+     находятся **после** этих трёх `await`, внутри того же `try`;
+   - отклонённый `subscribeEvents` уходит во внешний `catch`, который при уже
+     установленном `_serverCfg` вызывает `_scheduleLoadRetry(true)` — то есть
+     ошибка необязательной подписки трактуется как отказ всей загрузки и
+     провоцирует непрерывный retry. Это ровно то, что ТЗ называет в §3 п.4 и
+     в риске §14.2.
+5. Прочитан `_warmAdoptViewport()` (`src/houseplan-card.ts:2451-2499`) и
+   окружающие флаги `_hashApplied`/`_navApplied` (`:1521-1522`, `:2296-2299`,
+   `:2466`, `:3266-3277`): подтверждено, что explicit `#space=` hash уже сегодня
+   выигрывает у warm-viewport (`if (this._hashApplied || …) { this._warmVp = null; return; }`),
+   а принятый warm viewport не сбрасывается менее точным `LS_NAV`/`default_floor`
+   (`_navApplied = true` защищает ветку в `_loadFromServer`). Формулировка §7.2
+   ТЗ об этом взаимодействии технически точна, а не придумана заново.
+6. Прочитан `_pickSpace()` (`src/houseplan-card.ts:1087-1097`): `if (id === this._space) return;`
+   — клик по уже активной вкладке уже сегодня no-op. AC4 ТЗ («после исправления
+   клик активной вкладки — no-op») не вводит новое поведение клика, а лишь
+   требует, чтобы `_space` корректно совпадал с реальным выбором к моменту клика.
+7. Прочитана генерация id пространства (`src/houseplan-card.ts:12067`,
+   `spaceId = 's' + Date.now().toString(36)`): реальные id никогда не выглядят
+   как `f1`/`f2`, то есть легаси-дефолт `_space = 'f1'` (`:611`) — синтетический
+   сентинел, а не потенциально валидный id; диагностическая таблица ТЗ с
+   `home`/`upstairs` репрезентативна, коллизии не подразумевается.
+8. Проверено `docs/CONFIG-COMPATIBILITY.md` — ни `LS_CFG`, ни server-config
+   schema там не упомянуты в связи с этим изменением; заявление ТЗ §9 «миграция
+   не нужна» не противоречит канону.
+9. Проверено существующее browser-smoke покрытие: `demo/smoke_warm_remount.mjs`,
+   `demo/smoke_kiosk.mjs`, `demo/smoke_warm_owners.mjs`, `demo/smoke_warm_dialogs.mjs`
+   реально существуют — предположение §17 п.5 («smoke может расширить
+   существующий WS/warm lifecycle сценарий») опирается на реальную
+   инфраструктуру, а не на вымышленный файл.
+10. Проверена запись в `docs/specs/README.md` — строка на #131 добавлена в том
+    же коммите `0164e65`, ссылка issue ↔ ТЗ двусторонняя.
+11. Сверена терминология с `docs/USER-GUIDE.ru.md` (§7 «Пространства», §17
+    «Киоск-режим», обычный просмотр «не отдельная вкладка») — ТЗ использует
+    «пространство», «вкладка», «киоск» ровно в этом значении, ничего не
+    изобретает.
+12. Сверено с `docs/TOUCH-SUPPORT.md` — «View is fully supported/must be
+    convenient and reliable» и «kiosk is the primary supported environment»:
+    формулировка ТЗ §10 «View на touch блокирующий… в киоске требование ещё
+    строже» дословно соответствует канону, а не собственная эскалация автора.
+
+## Обязательные разделы (§7.1 PROCESS.md)
+
+| Раздел | Есть | Комментарий |
+|---|---|---|
+| Сценарий (персона/поверхность/момент) | ✅ | §1 |
+| Что человек увидит до/после (без терминов реализации) | ✅ | §2, одной фразой каждое состояние |
+| Проблема | ✅ | §3, с подтверждённой диагностикой и таблицей на конкретном SHA |
+| Скоуп / не-скоуп | ✅ | §5 / §6 |
+| Контракт поведения | ✅ | §7 (инвариант, приоритет, mandatory/optional, деградация, reload/cache) |
+| UX / i18n / accessibility / touch | ✅ | §10 |
+| Модель данных и миграция | ✅ | §9 |
+| AC1…ACn с доказательством | ✅ | §11, 12 штук, каждый с типом (`unit`/`smoke`/«ревью кода»/`build`) |
+| План автотестов | ✅ | §12 |
+| Риски | ✅ | §14, 5 пунктов, каждый со ссылкой на закрывающий AC |
+| Откат | ✅ | §16 |
+| Release-артефакты | ✅ | §15 |
+
+Все обязательные разделы присутствуют и содержательны. Дополнительно есть
+раздел §8 «Архитектурный контракт реализации» и §17 «Принятые технические
+предположения» — оба корректно отделены от продуктового контракта.
+
+## Находки
+
+Находок уровня **High** и **Medium** нет.
+
+### Low-1 — плотная формулировка §7.2 про hash/warm-viewport взаимодействие
+
+**Файл:** `docs/specs/131-readonly-cold-start.md:146-160`
+
+Абзац описывает две ветки (без принятого warm viewport / с ним) одним плотным
+текстом: «применяется существующий порядок… Explicit hash по-прежнему
+выигрывает. Уже принятый валидный same-route warm viewport остаётся
+существующим continuity-исключением… и не сбрасывается менее точным
+saved/default значением». При беглом чтении можно ошибочно понять, что
+`explicit hash` всегда переопределяет уже принятый warm viewport — в
+реальности (проверено чтением `_warmAdoptViewport`, `src/houseplan-card.ts:2466`)
+hash блокирует **принятие** нового warm viewport, но не выбивает уже
+принятый в текущем цикле рендера; вторая фраза говорит о независимой ветке
+«when warm viewport already accepted», а не о конкуренции с первой веткой в
+рамках одного и того же кадра. Технически формулировка точна (я сверил её с
+кодом и противоречия не нашёл), но растянута до состояния, что имплементатор
+может перечитать её как конфликт правил.
+
+**Почему не блокирует:** AC1/AC5/AC6 explicitly не расширяют матрицу на
+конкуренцию «hash vs уже принятый warm viewport» — этот случай прямо в
+не-скоупе (§6: «изменение выбора пространства… deep link… не входит»), то
+есть задача обязана лишь не сломать существующее поведение, а не
+переописывать его заново. Формулировка описывает уже существующий, а не
+новый контракт, и не создаёт риска неверной реализации, потому что §8 п.2
+и риск §14.1 уже требуют «одного resolver» и защищают его тем же AC1/AC5/AC6.
+
+**Решение ревьюера:** Low, не блокирует. На усмотрение автора — можно
+перефразировать двумя явными пунктами («без warm viewport: …», «warm viewport
+уже принят: …») при следующей правке или оставить как есть.
+
+## Что проверено и корректно
+
+- Соответствие `docs/SCOPE.md`: задача закрывает J1 («план должен быть читаемым
+  и полным сразу при открытии») и J6-грань «reload/reconnect/техническое
+  перемонтирование не должны менять состав видимого плана» — регрессия внутри
+  уже закрытых Core user jobs, не новая функциональность и не расширение
+  скоупа; ни один пункт «Out of scope» не задет.
+- Гарантированный View/touch/kiosk-контракт (`docs/TOUCH-SUPPORT.md`) учтён
+  верно и процитирован дословно, не переизобретён.
+- Владелец лично принял defaults D1–D3 и приоритет P1 (комментарий
+  2026-08-14) — открытых продуктовых вопросов в финальной редакции ТЗ нет, и
+  это корректно: вопросы (триггер cold start vs прав, поведение при
+  нескольких пространствах, переживание reload) были заданы и закрыты на
+  этапе аналитики диагностическим прогоном, а не додуманы автором ТЗ.
+  Раздел §17 «Принятые технические предположения» отделяет свободно
+  изменяемые технические решения (имя resolver'а, форма orchestration
+  подписок, механизм retry, файл browser smoke) от продуктовых решений D1–D3,
+  которые пересмотру не подлежат — ни одна догадка не выдана за факт без
+  пометки.
+- Технический диагноз причины (порядок `_adoptStructuralResponses` → три
+  `await subscribeEvents` → выбор пространства/`_cacheSnapshot`, отказ
+  подписки уходит во внешний `catch` и триггерит `_scheduleLoadRetry(true)`)
+  подтверждён построчным чтением `_loadFromServer()` — не голословное
+  утверждение автора, а точное описание существующего кода.
+- Поведение hash/`LS_NAV`/warm-viewport и no-op клика активной вкладки
+  (AC4) подтверждено чтением `setConfig`, `_warmAdoptViewport`, `_pickSpace`
+  — расхождений с ТЗ не найдено (см. Low-1 только по ясности формулировки,
+  не по фактической корректности).
+- AC1–AC12 однозначны, у каждого указан тип доказательства из допустимого по
+  DoR перечня (`unit`/`smoke`/«ревью кода»/`build`); план автотестов (§12)
+  даёт конкретный маршрут, включая явное требование покрыть матрицу
+  valid/stale hash × `LS_NAV` × `default_floor` × 0/1/N пространств (AC1) и
+  проверять «умеет падать» через фиксированный HA snapshot до/после no-op
+  клика (AC4).
+- Не-скоуп (§6) корректно отсекает смежные соблазны: не менять backend
+  permissions/websocket API, не добавлять новый toast/recovery overlay, не
+  трогать порядок вкладок/deep-link/формат cache, не менять
+  `houseplan-space-card`, не переписывать весь boot lifecycle — типичные
+  места, где скоуп мог бы незаметно расшириться из-за близости к async boot.
+- Раздел §13 (Производительность и security) обоснованно закрывает вопрос
+  «не ослабляет ли исправление read-only-границу»: подписка на запрещённые
+  события не эмулируется, write API не вызывается — прямо отражает
+  требование AC10.
+- Release-артефакты (§15) перечисляют реальные файлы:
+  `docs/CHANGELOG.md`/`docs/CHANGELOG.ru.md` (существуют), `docs/ARCHITECTURE.md`
+  (существует), `test/*.test.mjs` и `demo/smoke_*.mjs` (существующая
+  инфраструктура, см. «Как проверялось» п.9), три bundle snapshot
+  (существующая практика синхронизации по `AGENTS.md`).
+- Реестр `docs/specs/README.md` обновлён тем же коммитом, ссылка issue ↔ ТЗ
+  двусторонняя (`PROCESS.md` §7.1).
+- Трейлеры коммита `0164e65` (`Issue: #131`, `User-Visible: no`) корректны:
+  изменение — только документация ТЗ, продуктовый код не тронут, что явно
+  подтверждено и в тексте самого ТЗ, и в комментарии «ТЗ готово».
+
+## Чего не проверял
+
+- Не проверял, что предложенная в §8 архитектурная декомпозиция (единый pure
+  resolver cold precedence) реализуема без побочных эффектов на смежные поля
+  `_hashApplied`/`_navApplied`/`_warmVpArmed` — по правилам ТЗ (§17 п.1–2) это
+  свободно изменяемое техническое предположение автора кода, предмет
+  код-ревью, а не ревью ТЗ.
+- Не запускал никаких автотестов и не собирал бандл — на этапе `spec` это не
+  требуется; существование упомянутых тестовых файлов и smoke-инфраструктуры
+  проверено чтением файловой системы, не исполнением.
+- Не проверял golden/скриншоты — ТЗ §12 явно и обоснованно заявляет их
+  ненужность (новый визуал совпадает с уже принятым состоянием после клика);
+  это будущий предмет пре-релизного гейта, не ревью ТЗ.
+- Не проверял поведение `houseplan-space-card` (статическая карточка) —
+  явно вынесено в не-скоуп §6, и это верно: она получает пространство
+  отдельным обязательным параметром, а не через выбор, который чинит эта
+  задача.
+- Не проверял, действительно ли трёх-подписочный orchestration в текущем
+  коде уже идемпотентен при повторном вызове `_loadFromServer` после отказа
+  (авторитет `_unsubCfg`/`_unsubTrail`/`_unsubLayout`, AC7/AC11) —
+  прочитал структуру `if (!this._unsubX) { this._unsubX = await … }`
+  (`src/houseplan-card.ts:3232-3263`) и она выглядит согласованной с
+  требованием, но полное покрытие гонок (двойной параллельный вызов
+  `_loadFromServer`) — предмет код-ревью на реализованном коде, не ревью ТЗ.
+
+## Вердикт
+
+Зелёный. High: 0, Medium: 0. Одна находка Low (плотность формулировки §7.2
+про hash/warm-viewport) — не блокирует, технически формулировка проверена и
+корректна, оставлена автору на усмотрение с записью в этом документе.