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

SPEC-REVIEW-126-r1

• Issue: https://github.com/Matysh/houseplan-card/issues/126
• ТЗ: docs/specs/126-ha-area-marker-relocation.md
• Коммит материала: 8bc8ba66 (ветка issue/126-ha-area-marker-relocation, поверх origin/dev@6bf39ee9)
• Этап: ТЗ на ревью (PROCESS.md §2.4)
• Заход: r1 (первый прогон ревью ТЗ по этому issue — прежних SPEC-REVIEW-126-* документов в истории репозитория нет, git log --all по этому пути пуст)
• Трек: полный (владелец явно закрыл small в комментарии 2026-08-30)

Скоуп ревью

Диапазон материала — только диф docs/specs/126-ha-area-marker-relocation.md + одна строка в docs/specs/README.md (git diff origin/dev...HEAD --stat). Продуктовый код не менялся — это подтверждено и текстом хендоффа автора, и самим дифом. Ревью оценивает: соответствие docs/SCOPE.md (J6), выполнимость и однозначность AC1–AC12, отсутствие догадок, выданных за факт, соответствие разделу §7.1 и полноту DoR-чеклиста §2.5 (это ревью — единственный гейт перед переводом в «Готово к разработке»).

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

Гейт	Команда	Результат
Typecheck	npx tsc --noEmit	green, без вывода
Unit-тесты	npm test	green: tests 1668, pass 1667, fail 0, skipped 1 (пропуск — известный environment-sensitive кейс smoke_opening_measure, не относится к этому диффу)
Build	npm run build	green, dist собран без ошибок
node scripts/check-docs.mjs	не запускал	диф не трогает src/** — фингерпринт скриншотов не мог протухнуть от этого коммита
npm run invariants / model invariants	не запускал	диф не трогает геометрию, layout, marker.space, open_spans — только текст ТЗ
Browser smoke (demo/smoke_*)	не запускал	диф не содержит продуктового кода; смоки проверяют бандл, а бандл не менялся этим коммитом
golden:verify, performance_smoke, pytest tests_backend	не запускал	не тронуты визуальные/perf/backend поверхности — диф docs-only

Зелёного Validate на SHA 8bc8ba66 не найдено (по условиям задачи), поэтому дешёвые гейты прогнаны вручную; все три зелёные. Тяжёлые гейты не применимы — объём гейта соразмерен объёму диффа (только ТЗ), как и предписано.

Дополнительно я сверил ключевые технические утверждения ТЗ с реальным кодом на dev@6bf39ee9 (агент Explore, плюс собственные grep/Read), поскольку ТЗ в разделе 4 заявляет несколько «выпущенных соседних контрактов» как факты — а такие заявления обязаны быть фактом, а не пересказом с чужих слов.

Находки

Medium — в скоупе задачи (возврат автору)

M1. §9 «Persisted provenance»: заявленный бюджет known_devices не существует в коде — факт, поданный как решённый, на деле является догадкой.

ТЗ утверждает: «максимум совпадает с действующим budget known_devices и общим config-size limit» (docs/specs/126-ha-area-marker-relocation.md:198). Проверка кода это не подтверждает:

• settings.known_devices и settings.new_device_ids не объявлены в ServerConfig['settings'] (src/types.ts — там только exclude_integrations, group_lights, show_all, filter_seeded, icon_rules); оба поля читаются через any-каст в src/houseplan-card.ts (например, строки ~5071, ~5086);
• явного per-field ограничения на длину/количество записей known_devices в коде нет — массив растёт без отдельного капа;
• единственный реальный числовой лимит — общий MAX_CONFIG_BYTES = 2 * 1024 * 1024 в custom_components/houseplan/validation.py:1101, применяемый ко всему конфигу, а не к этому полю отдельно.

Раздел «модель данных и миграция" — один из обязательных по §7.1, и именно в нём остаётся утверждение о несуществующем ограничении. Это ровно тот случай, про который прямо предупреждает этап ревью: «утверждение о поведении, которого нет ни в одном документе и которое не помечено как предположение — замечание». Практическое следствие: реализатор может решить, что отдельный бюджет для marker_area_snapshot уже где-то согласован и просто скопировать несуществующее число, вместо того чтобы принять его как самостоятельное техническое решение.

Как воспроизвести: grep -n "known_devices\|new_device_ids" src/types.ts — пусто; grep -n "MAX_CONFIG_BYTES" custom_components/houseplan/validation.py — единственный найденный числовой лимит, общий для всего конфига.

Требуется: переписать §9, убрав ссылку на несуществующий бюджет known_devices. Либо явно принять размер marker_area_snapshot как самостоятельное техническое решение (в раздел §23 «принято предположительно», например «не больше активных зарегистрированных direct-биндингов, живёт по той же lifecycle-гигиене, что и known_devices»), либо прямо написать, что единственная защита — общий MAX_CONFIG_BYTES.

Low — снимаются с записью (реализатор решает сам, без возврата цикла)

L1. §8 «Eligibility»: нет формального поля, отличающего composite light group от direct entity marker.

ТЗ требует исключить «markerless auto entity: light group» из eligibility (строка 166), подразумевая чистое различение по типу привязки. По факту в src/types.ts есть только bindingKind?: 'device' | 'entity' | 'virtual', и у light group это тоже 'entity' (src/devices.ts, блок построения групп) — то есть на уровне типа обычная entity-маркер и light group неотличимы. Реальное различение в кодовой базе делается по побочным признакам (id с префиксом lg_, icon === 'mdi:lightbulb-group', локализованная модель device.light_group).

Это решаемо (признак уже используется в других местах того же файла), и это чисто техническая деталь — по §7.1 «всё, чего пользователь не наблюдает, агенты решают сами». Формально не блокирует ТЗ, но раздел §23 «принято предположительно» такую деталь не называет, а без неё AC5 («binding scope») не самоочевиден для будущего код-ревью. Снимается с записью: реализация обязана явно задокументировать выбранный признак (например, в комментарии resolver-модуля или в §23 при следующей редакции), код-ревью проверит это по факту.

L2. §15/DoR: явного предложения «влияние на производительность — Х» нет отдельной фразой.

DoR (§2.5) требует явно называть влияние на производительность и бюджеты (или явное «нет»). Отдельного предложения об этом в ТЗ нет. При этом по существу пункт закрыт — AC11 требует bounded/no-loop поведение с performance-тестом, а resolver по тексту §10 запускается «после каждого authoritative registry rebuild», а не в рендер-цикле. Учитывая объём диффа (маленький pure-модуль) и явные гарантии AC11/§14 против дублирующей работы, риск для бюджета bundle:budget (256000 B gzip) и рендер-пути пренебрежимо мал. Снимается с записью: не блокирует ревью; рекомендую добавить одну явную строку в раздел рисков при следующей правке ТЗ ради дисциплины DoR-чеклиста, но отдельного цикла ради этого не открываю.

Что проверено и признано корректным

• Обязательные разделы §7.1 присутствуют полностью: сценарий и персона (§1, Home admin/View+Devices editor+hosted Static), «что человек увидит до и после» одной фразой без терминов реализации (§1), проблема/диагноз (§2), скоуп и не-скоуп (§5–6), контракт поведения (§7–14), UX/touch/a11y (§15), модель данных и миграция (§16), i18n (§17, корректно «новых строк нет»), AC1–AC12 с указанием способа доказательства (§18), план тестов (§19), риски и откат (§21), release-артефакты (§22).
• J6 верно назван и обоснован (docs/SCOPE.md): marker, оставшийся в прежней комнате после переезда устройства в HA, — ровно случай «план перестаёт быть правдой» из J6.
• Причина дефекта подтверждена кодом, а не заявлена на веру: _livePos() (src/houseplan-card.ts:5174-5190) действительно сравнивает только saved.s === d.space, без комнаты/Area; тот же паттерн дублирован в markerPos() (src/space-geometry.ts:629-638) — ТЗ верно указывает, что дефект есть и в hosted Static. diffNewDevices() (src/logic.ts:2092-2100) и _syncNewDevices() действительно сравнивают только множества id, без учёта area — заявление о причине отсутствия метки подтверждено.
• Правило приоритета explicit override (раздел «#317») процитировано кодом, а не придумано: src/devices.ts:1421-1424 — if (marker.area == null && marker.space && marker.room_id) return roomClimateKey(...); return marker.area || null; — это буквально то трёхветочное правило, которое ТЗ формулирует в разделе 4. Отдельно подтверждено resolveExplicitMarkerPlacement() (src/devices.ts:1066-1086, const area = marker.area || registryArea || '').
• Эндпойнт houseplan/layout/delete, на который опирается сценарий переноса (§11), реален, а не выдуман: backend custom_components/houseplan/websocket_api.py:1099-1106, фронтенд уже вызывает его в существующем коде удаления layout (src/houseplan-card.ts:5240-5243).
• DevItem.area/.space действительно берутся из device.area_id/ entity.area_id реестра HA в buildDevices() (src/devices.ts:1112-1113, 1157-1158, 1240-1241) — заявление раздела 2 ТЗ о механике перестройки корректно.
• Механизм очистки истории при авторитетном layout-событии другого клиента, на который ссылается контракт «#74», реально существует: _geometryHistory.clear(); this._devicePositionHistory.clear(); при адопции структурных ответов и при layoutChanged (src/houseplan-card.ts:4215-4218, ~5245) — переиспользование, а не придуманный факт.
• Механизм point-in-room для backfill (§12) опирается на существующий pointInPolygon() (src/logic.ts:431), уже используемый для аналогичных проверок принадлежности точки комнате в нескольких модулях (houseplan-card.ts:7566, sun.ts:239,259) — не изобретение несуществующей возможности.
• Терминология согласована с docs/USER-GUIDE.ru.md: красная метка «Новое» и правило «поиск/Найти на плане её не снимают, открытие настроек — снимает» (docs/USER-GUIDE.ru.md:810-812) совпадают с тем, что ТЗ переиспользует в разделе «#29» и в §13 без выдумывания второго badge.
• Владельцу не задано ни одного нового вопроса внутри ТЗ — все Q1–Q4 закрыты предыдущими комментариями, что и зафиксировано в разделе 3.
• docs/specs/README.md обновлён корректно — строка со ссылкой на ТЗ добавлена на своё алфавитное место.

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

• Не проверял выполнимость самого резолвера — кода ещё нет, это спек-ревью, а не код-ревью; технические имена по §23 прямо разрешено менять свободно.
• Не прогонял golden/performance/browser smoke/backend pytest — диф их не касается (см. таблицу гейтов и обоснование в каждой строке).
• Не проверял по коду каждую строчку раздела «#29» (полный lifecycle каталога устройств) — свойства hide/remove/HA-disabled ТЗ заявляет как «не меняются», и это утверждение о неизменности, не о новом поведении; выборочно свёл через USER-GUIDE.ru.md, глубокого код-аудита всего #29 не делал, так как это чужой уже выпущенный контракт вне предмета этой задачи.
• Не оценивал реалистичность оценки сложности/риска из комментариев owner (7/10, 8/10) — это управленческая оценка, не предмет ревью ТЗ.

Вывод

High-находок нет. Одна Medium-находка (M1) — в скоупе задачи, требует правки текста ТЗ (не архитектуры и не AC), поэтому отдельный issue не заводится. Две Low-находки сняты с записью и не блокируют. Раздел «Унаследовано из r0» не пишется — предыдущего раунда ревью ТЗ по этому issue не существует.

Вердикт: жёлтый. Автору — поправить §9 (M1), при желании закрыть L1/L2 в той же правке, и вернуть на повторное ревью ТЗ.