docs: review document for #252

Issue: #252
User-Visible: no

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

@@ -0,0 +1,168 @@
+# SPEC-REVIEW-252-r1
+
+- Issue: [#252](https://github.com/Matysh/houseplan-card/issues/252) — «Отчёт
+  "Оптимизировать" перечисляет внутренние id вместо того, чтобы починить или
+  сказать, что делать»
+- Этап: spec (PROCESS.md §2.4)
+- Заход: r1 · блокирующих циклов израсходовано 0 из 4
+- ТЗ: `docs/specs/252-optimize-orphan-layout-report.md`, коммит `883a95a`,
+  ветка `issue/252-optimize-orphan-layout-report`
+- Трек: обычный (не `small`/`trivial`, файл ТЗ обязателен — подтверждено)
+
+## Скоуп проверки
+
+Полный разбор — первый заход. Читал в порядке из инструкции: `docs/SCOPE.md`,
+`AGENTS.md`/`PROCESS.md` §2.4/§5/§7.1, тело issue #252 и все три комментария
+(аналитика, занятие, ТЗ готово), `docs/USER-GUIDE.ru.md` (термин «групповой
+маркер»), канонические `docs/CANVAS.md` и `docs/CONFIG-COMPATIBILITY.md`.
+
+## Как проверялось
+
+Автор — не я; устных пояснений не было, работал только с issue и текстом ТЗ.
+
+1. Сверил обязательные разделы ТЗ (PROCESS.md §7.1) — все 12 присутствуют, плюс
+   раздел «Принятые технические предположения» (§14), как того требует §7.1 для
+   решений, не наблюдаемых пользователем.
+2. Прочитал текущий код, на который ссылается диагноз ТЗ (§3), чтобы отличить
+   проверенный факт от догадки:
+   - `src/space-reference-repair.ts` — подтверждает `positionsUnresolved`,
+     `nestedRefsUnresolved`, `deadSpaceIds`, порядок remap/detach-проходов,
+     трактовку `removed:true` как «unresolved, не удаляется» **сегодня**.
+   - `src/houseplan-card.ts:16065–16132` (`_renderAlignDialog`) — подтверждает
+     текст `gs.optimize_reference_warning` и вывод сырых `deadSpaceIds` ровно
+     как описано в issue и в ТЗ.
+   - `src/i18n/ru.json:796,808` — подтверждает дословный текст текущих строк.
+   - `src/logic.ts:728-738`, `src/devices.ts:1151` — подтверждают, что
+     `lg_<entity_id>` — реальный префикс marker id для групп света, а не
+     придуманный термин.
+   - `src/ha-binding-status.ts` — подтверждает, что `HaRegistrySnapshot` с
+     полем `authoritative`, кэшем на соединение и подписками на
+     device/entity registry **уже существует и уже используется**
+     (`houseplan-card.ts:4542`). План ТЗ «расширить runtime context
+     авторитетным roster» (§11.1) переиспользует готовый механизм, а не
+     придумывает новый с нуля — техническая осуществимость подтверждена.
+   - `src/align-grid.ts:375-390`, `src/houseplan-card.ts:18133-18201` —
+     подтверждают, что `rl_<roomId>` и marker-id-ключ — единственные два
+     вида layout-ключей в коде; категория «неизвестный namespace» в §6.1
+     действительно исчерпывающий catch-all, а не дыра в классификации.
+   - `src/houseplan-card.ts:15006-15028` (`_openAlignDialog`) — подтверждает,
+     что `_devices` сегодня используется как отфильтрованный
+     presentation-снимок (`effectiveAreaByMarker` строится с
+     `.filter(d => !d.virtual && !!d.area)`), что оправдывает требование ТЗ не
+     считать отсутствие в `_devices` доказательством отсутствия владельца.
+3. Сверил §6/§7 (классификация и её доказательства) с этими файлами построчно
+   — расхождений не нашёл; диагноз не является догадкой, выданной за факт.
+4. Проверил `docs/CANVAS.md` (канонический документ подсистемы) и
+   `docs/CHANGELOG.md` на противоречия с новым контрактом — здесь нашлась
+   единственная содержательная находка, см. ниже.
+5. Проверил соответствие персоне/скоупу: `docs/SCOPE.md` J6 («keep the plan
+   true as the home evolves») и стоящее правило «never delete a user's file on
+   an inference» (SCOPE.md, строки 81-91) — новый контракт **согласован** с
+   духом этого правила (удаляет только доказанно-мёртвое, при неполном
+   registry сохраняет), но затрагивает соседний инвариант, см. находку.
+6. Проверил, что автор не оставил владельцу технических вопросов под видом
+   продуктовых — комментарий «Продуктовых вопросов не осталось» (2026-08-23)
+   подтверждён: все решения в §14 действительно не наблюдаемы пользователем
+   (внутреннее имя `lg_`, устройство opt-in внутри существующего диалога,
+   лимиты 3/10 в UI, трактовка tombstone-маркера).
+
+## Находки
+
+### Medium (в скоупе — правится в этом же ТЗ)
+
+**M1. ТЗ меняет задокументированный и опубликованный инвариант Optimize, не
+называя его.**
+
+`docs/CANVAS.md:494-497` фиксирует как **намеренное** архитектурное решение:
+
+> The optimizer deliberately does **not** alter backdrop calibration or saved
+> view boxes, **delete unattached layout entries (a device may only be
+> temporarily unavailable)**, deduplicate markers, or delete files.
+
+То же самое опубликовано пользователю в `docs/CHANGELOG.md:1378-1385` (релиз
+v1.59.0-rc.1, English changelog): «Backdrop calibration, saved views,
+**unattached layout entries** and user files are left alone.»
+
+ТЗ #252 (§4, цель 1; §7.1) вводит ровно обратное для категории `absent`:
+Optimize **начинает** удалять unattached layout entries — при условии, что
+отсутствие владельца доказано по авторитетному registry. Технически это
+разумное сужение старого правила (закрывает ту же дыру «temporarily
+unavailable», которую защищала старая формулировка, но через доказательство,
+а не через полный запрет), и §13 корректно включает `docs/CANVAS.md` в список
+файлов на обновление. Но нигде в теле ТЗ (§3 «диагноз», §4 «цели» или отдельным
+пунктом §14) не сказано прямо: *«это меняет существующий инвариант CANVAS.md
+[494-497] / обещание CHANGELOG v1.59.0-rc.1 — старое правило было
+консервативным приближением, новое доказывает отсутствие, а не предполагает
+его»*.
+
+Почему это не Low и не «само собой закроется веткой §13»: без явного указания,
+**что именно** в CANVAS.md заменяется, а не просто дополняется, есть риск, что
+реализация допишет новый абзац рядом со старым «deliberately does not delete
+unattached layout entries» — и канонический документ подсистемы станет
+внутренне противоречивым (ровно то, чего требует избегать сам жанр
+канонического документа). Такой же явный след нужен в записи CHANGELOG для
+#252: старое обещание «unattached layout entries... are left alone» не должно
+молча стать ложным без указания, что оно сужено, а не отменено.
+
+**Как чинится в ТЗ:** одним абзацем в §3 или отдельным пунктом рядом с §14 —
+явная ссылка на `docs/CANVAS.md:494-497` и на changelog-запись v1.59.0-rc.1,
+формулировка «заменяет», а не «дополняет», и требование к release-артефакту в
+§13 переписать (не приписать к) старое предложение в CANVAS.md.
+
+**Воспроизведение:** `docs/CANVAS.md` строки 494-497 vs
+`docs/specs/252-optimize-orphan-layout-report.md` §4 п.1 и §7.1 — прямое
+текстовое противоречие без ссылки друг на друга.
+
+## Что проверено и корректно
+
+- Все обязательные разделы ТЗ (§7.1) присутствуют, включая блок принятых
+  предположений (§14) для всего, что не наблюдает пользователь.
+- Диагноз (§3) построчно подтверждён кодом (`space-reference-repair.ts`,
+  `houseplan-card.ts`, i18n) — не догадка.
+- Классификация владельцев (§6) исчерпывающая: `rl_`, marker-id (device/`lg_`),
+  unknown — других префиксов layout-ключей в коде нет.
+- Техническая база для fail-closed authority (`HaRegistrySnapshot.
+  authoritative`) уже существует и уже используется в проде — план не полагается
+  на код, которого нет.
+- AC1–AC8 однозначны, у каждого указан способ доказательства (unit fixture,
+  browser smoke, semantic/golden), включая mutation guard — термин уже принят
+  в этом репозитории (`docs/TESTING.md`, множество прежних ревью), не изобретён.
+- Три исхода классификации (`absent` / `live-in-missing-space` / `unverified`)
+  соответствуют трём случаям из AC4 issue и корректно не пересекаются.
+- Safety-контракт согласован с духом стоящего правила SCOPE.md «never delete a
+  user's file on an inference»: удаление разрешено только по доказательству,
+  неполный registry — fail-closed в сторону сохранения.
+- Не-скоуп (§5) корректно исключает миграцию схемы, автоперенос живых объектов,
+  очистку vacuum segment map и новый экран диагностики — совпадает с тем, что
+  реально решает issue.
+- Release-артефакты (§13) перечисляют оба changelog, User-Visible: yes,
+  обновление docs screenshot fingerprint (обязательно, т.к. меняется `src/**`)
+  и golden-решение — ничего не забыто по чек-листу §13 самого PROCESS.md.
+- Владельцу не оставлено технических вопросов под видом продуктовых; все пункты
+  §14 действительно ненаблюдаемы пользователем.
+- small/trivial треки корректно не применены (меняется UX-контракт и i18n).
+
+## Чего не проверял
+
+- Не проверял production-код реализации — задачи ещё нет, стадия `S4-spec-review`.
+- Не запускал гейты (`tsc`, `npm test`, `npm run build`, smoke) — на этапе
+  ревью ТЗ они неприменимы, кода нет.
+- Не проверял полноту `docs/ARCHITECTURE.md` на предмет уже существующего
+  языка для «runtime authority boundary» — доверился формулировке ТЗ §13, это
+  файл, а не поведенческий контракт, и ревью ТЗ не требует вычитывать все
+  канонические документы построчно, если задача их не переопределяет.
+- Не проверял точные будущие RU/EN i18n-ключи и их plural-формы — ТЗ не обязано
+  фиксировать их дословно на этапе ТЗ, это область реализации, а не контракта.
+- Не оценивал `docs/UX-MODES.md`/`docs/TOUCH-SUPPORT.md` построчно на
+  противоречия — §9 ТЗ (touch/accessibility) ссылается на уже существующий
+  паттерн диалога Optimize, новых touch-механизмов не вводит.
+
+## Вердикт
+
+Один Medium-дефект в скоупе — документируемое расхождение с существующим
+каноническим инвариантом, устраняется правкой текста ТЗ (без изменения
+технического контракта). High-находок нет.
+
+**Жёлтый.** Возврат автору на правку ТЗ (добавить явную ссылку на
+`docs/CANVAS.md:494-497` / CHANGELOG v1.59.0-rc.1 и требование заменить, а не
+дополнить, старую формулировку в release-артефактах §13).