From 759111beb3b72892d8b2f38b85e1ed962707abf3 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sun, 23 Aug 2026 06:18:02 +0000 Subject: [PATCH] docs: review document for #252 Issue: #252 User-Visible: no --- docs/reviews/SPEC-REVIEW-252-r1.md | 168 +++++++++++++++++++++++++++++ 1 file changed, 168 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-252-r1.md diff --git a/docs/reviews/SPEC-REVIEW-252-r1.md b/docs/reviews/SPEC-REVIEW-252-r1.md new file mode 100644 index 00000000..0e4557e5 --- /dev/null +++ b/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_` — реальный префикс 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_` и 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).