houseplan-card/docs/reviews/SPEC-REVIEW-44-r1.md

SPEC-REVIEW-44-r1

• Issue: https://github.com/Matysh/houseplan-card/issues/44
• Этап: spec (ревью ТЗ, PROCESS.md §2.4)
• Заход: r1 · блокирующих циклов израсходовано 0/4
• ТЗ: docs/specs/044-filter-grouping-policy.md, ревизия 2, зафиксирована коммитом 3d4c5090 (только этот файл, docs/specs/044-filter-grouping-policy.md, 180 добавлено / 56 удалено — см. git show --stat 3d4c5090)
• Вердикт: красный

Скоуп ревью

Полный разбор ТЗ #44 ревизии 2 (первый заход, дельты нет). Сверялись:

1. docs/SCOPE.md — задача закрывает пункт J6 issue-текста («третий вариант» — скрытая настройка с видимым эффектом — запрещён), не конфликтует со SCOPE.
2. AGENTS.md, PROCESS.md §7.1, §5 — трек полный (не small), файл ТЗ обязателен и существует; проверены обязательные разделы.
3. Тело issue #44 и три комментария (актуализация 2026-08-14, вход в работу и ревизия 2 от 2026-08-30) — сверка фактов кода, на которые ссылается автор.
4. docs/USER-GUIDE.ru.md §10 «Устройства» — терминология вкладок инбокса.
5. Код, который ТЗ описывает как «факты»: src/devices.ts, src/rules.ts, src/houseplan-card.ts, src/space-render.ts, src/device-inbox.ts, src/houseplan-editor-runtime.ts, src/i18n/*.json, scripts/config-field-registry.mjs.

Как проверялось

Построчная сверка каждого числового/файлового факта из раздела «Проблема», «Скоуп» и «Контракт поведения» ТЗ с текущим деревом (git log подтверждает, что рабочая копия — это коммит 3d4c5090, единственный докс-коммит #44 в этой ветке). Инструменты: Grep/Read по указанным файлам и строкам, git log -S для датировки происхождения кода, на который ссылается ТЗ как на «уже существующее».

Находки

H1 — Сценарий и AC4 построены на неверном факте: причина «исключена

интеграция» и появление кандидата в инбоксе уже существуют сегодня, но не там, куда их помещает ТЗ

Файл: docs/specs/044-filter-grouping-policy.md, раздел «Что человек увидит до и после» (строка 23) и «Скоуп → 2. Причина „Исключена интеграция X“» (строки 69–75), AC4 (строка 128).

Утверждение ТЗ: «До: … понять „почему этого устройства нет в списке“ нельзя. […] Сегодня такие устройства не попадают в инбокс вовсе — они появляются на «Скрытых» именно с этой причиной» и AC4: «скрытый фильтром кандидат виден на «Скрытых» с причиной «Исключена интеграция X»».

Что показывает код:

• Значение excluded_integration типа DeviceInboxReason и одноимённый i18n- ключ device_inbox.reason_excluded_integration уже существуют и не новые: добавлены коммитом cab8d128 feat: add device lifecycle catalog (issue #29), задолго до ревизии 2 этого ТЗ (git log -S excluded_integration → cab8d128). Тексты уже переведены на все 4 языка (src/i18n/en.json:484, de.json:484, fr.json:484, ru.json:484), сегодняшний текст — не с плейсхолдером: «Integration excluded by device filters» / «Интеграция исключена фильтрами устройств» и т.д. — обобщённая фраза без имени интеграции.
• Уже сегодня src/houseplan-editor-runtime.ts:7635-7637 считает excluded = […].some((domain) => this.host._excluded.has(domain)) (тот же резолвер _excluded, что ТЗ описывает как «единственный источник» в Контракте №3) и присваивает reasonByBinding[binding] = 'excluded_integration' ещё до всякой доработки — то есть кандидат УЖЕ размечен этой причиной.
• Эта причина уже рендерится пользователю безусловно для любой категории строки (src/houseplan-editor-runtime.ts:11848-11851, ${this.host._t(\device_inbox.reason_${row.reason}` as any)}— вызов не зависит отrow.category`).
• Категория строки для такого кандидата — не «Скрытые», а «Доступны»: buildDeviceInbox (src/device-inbox.ts:218-223) присваивает category = 'hidden' только когда есть маркер live?.hidden === true; кандидат без runtime/live и без маркера получает category = 'available' (строка 222), а reasonByBinding[binding] подставляется именно в ветке «иначе» для этой категории (src/device-inbox.ts:234-239). Категория 'hidden' в принципе не рассматривает reasonByBinding для кандидатов без маркера — код это структурно не допускает.

Воспроизведение (проверено чтением, не исполнением): возьмите конфигурацию без маркера на устройство с платформой из EXCLUDED_DOMAINS (или из settings.exclude_integrations). Откройте диалог «Устройства» уже на сегодняшнем dev. Кандидат уже виден на вкладке «Доступны» (не «Скрытые») со строкой причины «Integration excluded by device filters» — это происходит без единой строки нового кода, только из-за существующей связки houseplan-editor-runtime.ts:7637 → device-inbox.ts:238 → houseplan-editor-runtime.ts:11849.

Почему это блокирует: ТЗ выдаёт неверное описание сегодняшнего поведения за факт (не за предположение) и строит на нём и сценарий, и AC4. Раздел «Принятые предположения» (строка 183: «причина показывается на «Скрытых», НЕ в отдельной новой вкладке») маскирует ровно эту развилку решением, но не называет техническое противоречие: сегодняшняя категоризация структурно не кладёт такого кандидата на «Скрытые» без отдельного изменения контракта buildDeviceInbox, которого раздел «Контракт поведения» не содержит (там только 5 пунктов, ни один не про категорию строки). AC4 в текущей формулировке невыполним без незаявленного изменения категоризации, а смок, написанный по AC4 «как есть», либо провалится на реальной категории («available», не «hidden»), либо будет молча ослаблен под фактическое поведение — оба исхода авторами не решены, а решение здесь ровно продуктовое («на какой вкладке пользователь видит причину») и должно быть либо переписано по факту, либо вынесено владельцу одним вопросом с вариантом по умолчанию (PROCESS.md §7.1).

Что нужно поправить: переписать «Проблема»/сценарий по фактическому сегодняшнему поведению (причина и её текст уже есть, только обобщённые и на вкладке «Доступны»), решить и явно записать: 1) остаётся ли причина на «Доступны» (тогда AC4 меняет текст на плейсхолдерный, но не трогает buildDeviceInbox) или 2) вводится изменение категоризации, переносящее такие строки на «Скрытые» (тогда это новый пункт Контракта поведения и отдельный риск — категория hidden сегодня зарезервирована за маркерами hidden: true, смешение с «нет маркера вовсе» меняет смысл вкладки для остальных находок).

H2 — Контракт №3 («оба значения читаются из одного источника всеми

потребителями») не выполняется уже сегодня для exclude_integrations, и ТЗ не замечает единственного потребителя-исключения

Файл: docs/specs/044-filter-grouping-policy.md, «Проблема» (строка 34: «потребители houseplan-card.ts:3924, space-render.ts:245, discovery devices.ts:1491») и «Контракт поведения» п.3 (строка 98).

Что показывает код: src/devices.ts:1491 — это НЕ потребитель настраиваемого ctx.excluded/_excluded. Строка лежит внутри roomClimateMap (объявление src/devices.ts:1435, сигнатура roomClimateMap(hass, rules?, markers?) — параметра settings/excluded в ней нет вовсе) и хардкодит продуктовый EXCLUDED_DOMAINS, импортированный напрямую из rules.ts (src/devices.ts:5):

if (EXCLUDED_DOMAINS.has(reg.platform)) continue; // filtered-out integrations

Это единственное место в src/**, где EXCLUDED_DOMAINS.has( вызывается напрямую, минуя настраиваемый резолвер (grep -rn "EXCLUDED_DOMAINS\.has\(" src → одно вхождение). lightGroups() (второй ключ, group_lights) такой проблемы не имеет — оба его вызова (devices.ts:1035, :1100) читают settings.group_lights честно.

Почему это в скоупе, а не соседний баг: issue #44 прямо требует «для каждого ключа выбрать: 1) поддерживаемая настройка с понятным эффектом […] 2) фиксированное поведение. Нельзя оставлять третий вариант — скрытую настройку, которая влияет на результат, но не видна пользователю». roomClimateMap (агрегирование температуры/влажности комнаты, J5) — это ровно третий вариант для exclude_integrations: после того как пользователь через новый UI уберёт интеграцию из исключений, устройство появится в списке комнаты, но его температурные/влажностные показания по-прежнему будут молча исключаться (или не исключаться) по продуктовому дефолту — независимо от выбора пользователя. И наоборот: добавление интеграции в исключения через UI не уберёт её показания из climate-агрегата. Это именно то расхождение, ради ликвидации которого заведён #44, только оно остаётся необнаруженным самим ТЗ.

Воспроизведение (проверено чтением, не исполнением): установить settings.exclude_integrations: [] (пользователь явно снял все исключения через «Вернуть рекомендуемые» → нет, это удаляет ключ; для эффекта нужно явно задать [] — «ничего не исключать», валидное значение по «Принятым предположениям» ТЗ). После этого _excluded в houseplan-card.ts:3925 вернёт пустой Set, устройства всех интеграций попадут в buildDevices. Но roomClimateMap по-прежнему исключит их показания температуры/влажности по жёстко зашитому EXCLUDED_DOMAINS, потому что в её сигнатуре нет входа для пользовательского значения. Пользователь увидит устройство на плане, но не увидит его вклад в комнатный climate-показатель, и UI, который ТЗ обещает («честные представления», «понятный эффект»), не объяснит почему.

Почему это блокирует: Контракт поведения — раздел, на который ссылаются AC (AC7 — «отсутствие ключей → выдача байт-в-байт», проверяет только buildDevices, не climate-агрегат) и release-текст «нет скрытых discovery-ключей» (Release-артефакты, ARCHITECTURE.md). Ложное утверждение «читается из одного источника всеми потребителями» — это ровно тот случай, когда «утверждение о поведении, которого нет ни в одном документе, выдано за факт»: инвентаризация (тот самый пункт «инвентаризация exclude_integrations… на реальных конфигах», который issue требует первым делом) пропустила единственного потребителя-нарушителя. ТЗ должно либо явно перевести roomClimateMap на настраиваемый резолвер (тогда это новый пункт контракта/AC и новая поверхность — roomClimateMap, J5), либо осознанно задокументировать расхождение как принятое ограничение с явным «не-скоуп» и предупреждением в UI/USER-GUIDE («фильтр не влияет на климат-агрегацию») — сейчас не сделано ни то, ни другое.

L1 — Название вкладки инбокса не совпадает с интерфейсным словарём

Файл: docs/specs/044-filter-grouping-policy.md, «Сценарий» (строка 13), «Скоуп → 1» (строка 48) и везде далее — «вкладка «Доступные»».

Канонический термин из docs/USER-GUIDE.ru.md:785 и живого src/i18n/ru.json:457 — «Доступны» (device_inbox.tab_available), не «Доступные». AGENTS.md требует брать терминологию интерфейса из USER-GUIDE, а не изобретать. Не блокирует (Low, по решению ревьюера правится автором при следующей правке текста ТЗ, отдельного цикла не требует).

Что проверено и корректно

• Резолвер group_lights (devices.ts:1033, :1098 — !== false, default TRUE) и exclude_integrations (houseplan-card.ts:3924-3925, space-render.ts:245-246 — replace-семантика, не additive) описаны точно, номера строк совпадают день в день.
• EXCLUDED_DOMAINS (rules.ts:12) — 13 доменов, совпадает с описанием.
• scripts/config-field-registry.mjs:42-70 — оба ключа сегодня status: 'decision-required', ui: 'hidden', паспорта allow-extra — ТЗ переносит их в current с ui-путём в инбоксе; статус current используется в реестре многократно (6 других полей), формат совпадает.
• expected_rev (оптимистическая блокировка, #340) реально существует в houseplan-card.ts/houseplan-editor-runtime.ts; шаблон удаления ключа при возврате к дефолту уже применён для settings.weather_entity (enforcedBy в реестре) — техническое решение ТЗ опирается на существующий прецедент, а не выдумано.
• buildDevices(ctx: BuildCtx) (devices.ts:1092) — чистая функция, принимает settings/excluded через ctx, не делает I/O; план AC6 («превью — тот же вход, что у боевого buildDevices, без копии логики фильтра») технически реализуем как заявлено.
• [] как валидное «ничего не исключать», отличное от отсутствия ключа — подтверждено кодом (list ? new Set(list) : EXCLUDED_DOMAINS, строка разбора верна).
• Обязательные разделы §7.1 присутствуют все (сценарий, что увидит человек, проблема, скоуп/не-скоуп, контракт, UX/i18n, модель данных и миграция, AC, план автотестов, риски, откат, release-артефакты); «Принятые предположения» оформлены отдельным блоком, как требует PROCESS.md, хотя (H1) не покрывают реальное техническое противоречие.
• Трек (полный, не small) выбран правильно и соответствует решению аналитики от 2026-08-15, зафиксированному в issue.

Чего не проверял

• Гейты typecheck/test/build не запускались: на этом этапе (S4, ревизия ТЗ) изменён только docs/specs/044-filter-grouping-policy.md (коммит 3d4c5090, git show --stat подтверждает единственный файл); продуктовый код не тронут, гейты неприменимы к чистому докс-коммиту. node scripts/check-docs.mjs не запускался по той же причине (diff не касается src/**).
• Смоки/golden/backend/perf — неприменимо, кода нет.
• Не проверялась истинность утверждения «инфраструктура для UI появилась #29 … паспорта уже выданы в #33» сверх того, что напрямую процитировано выше (паспорта allow-extra подтверждены реестром; сам факт наличия вкладок/DeviceInboxReason подтверждён и лёг в основу H1).
• Не оценивалась производительность превью-диффа (буст buildDevices на больших registry) — вопрос реализации, не ТЗ; риск назван самим автором и выглядит разумно (пересчёт по явному действию, не на тик).

Итог

Вердикт: красный. Два High: ложное описание сегодняшнего поведения инбокса (H1, делает AC4 невыполнимым как написано и маскирует нерешённый продуктовый вопрос про вкладку) и незамеченный потребитель EXCLUDED_DOMAINS в roomClimateMap, нарушающий заявленный контракт «единственный источник» (H2, оставляет ровно тот «третий вариант», ради ликвидации которого заведён issue). Оба — фактические ошибки инвентаризации кода, а не вкусовщина; обе воспроизведены чтением конкретных строк, обе in-scope issue #44. Low (L1) — терминология вкладки, правится попутно.