15 KiB
CODE-REVIEW-117-r1
- Issue: https://github.com/Matysh/houseplan-card/issues/117
- ТЗ:
docs/specs/117-registryless-opening-entity.md, зелёноеdocs/reviews/SPEC-REVIEW-117-r1.md(High 0, Medium 0) - Диапазон:
git log --oneline origin/dev..HEAD— три коммита:03cca97fix: support registryless opening entities(Issue: #117,User-Visible: yes)3bd80ffdocs: review document for #117(Issue: #117,User-Visible: no)cc22988docs: specify registryless opening entities(Issue: #117,User-Visible: no)
- Роль: ревьюер кода (не автор), этап
S7-code-review, цикл r1/4
Скоуп ревью
Диф (git diff origin/dev...HEAD --stat, 16 файлов):
| Класс | Файлы |
|---|---|
| A (продукт) | src/ha-binding-status.ts |
| B (гейты/инструменты) | demo/smoke_registryless_opening.mjs (новый), scripts/mutation-gate.mjs, test/ha-binding-status.test.mjs, test/render-device-snapshot.test.mjs |
| C (документация) | docs/ARCHITECTURE.md, docs/CHANGELOG.md, docs/CHANGELOG.ru.md, docs/TESTING.md, docs/USER-GUIDE.ru.md, docs/reviews/SPEC-REVIEW-117-r1.md, docs/specs/117-registryless-opening-entity.md, docs/specs/README.md |
| D (сгенерированное) | dist/houseplan-card.js, custom_components/houseplan/frontend/houseplan-card.js, demo/srv/assets/houseplan-card.js |
Продуктовая правка — ровно одна логическая точка: renderOpeningEntityAvailable()
в src/ha-binding-status.ts:533-545 больше не требует projectedHass.entities[entityId]
одновременно с projectedHass.states[entityId]; сохранена проверка только
наличия state в замороженной активной проекции. _openingAmt() и
_renderOpeningLocks() в src/houseplan-card.ts не менялись — оба уже шли
через единственную точку _renderOpeningEntityAvailable(), поэтому фикс
закрывает contact и lock одновременно без риска рассинхронизации.
Как проверялось
Гейты, которые прогнал я лично (не со слов автора):
| Гейт | Команда | Результат |
|---|---|---|
| Typecheck | npx tsc --noEmit |
зелёный, без вывода |
| Unit | npm test |
804/804 pass |
| Build + сверка бандлов | npm run build && cmp dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js && cmp dist/houseplan-card.js demo/srv/assets/houseplan-card.js |
зелёный; sha256sum всех трёх копий совпадает: 73fffed183e23ab015cb0dd43ee0b3d2b277822bdd2f44813af6a1051398c881 |
| Целевой smoke (AC1/2/3/7/8/9) | node demo/smoke_registryless_opening.mjs |
OK, все 13 подпроверок true |
| Регрессия opening-биндинга | node demo/smoke_opening_binding.mjs |
OK, все 15 подпроверок true |
| Регрессия lock action | node demo/smoke_lock_action.mjs |
OK, все 8 подпроверок true |
| Регрессия lock invariant (SCOPE.md) | node demo/smoke_lock_invariant.mjs |
OK, все 7 подпроверок true |
| Mutation-guard | node scripts/mutation-gate.mjs --id=registryless-opening-requires-registry-row |
поймано 1 из 1 — мутант (возврат .entities[entityId]) красит smoke, чистый прогон зелёный |
Дополнительно — доказательство «тест умеет падать» для unit-покрытия, не
только для smoke (§2.7 требует это явно): временно вернул проверку
!!projectedHass?.entities?.[entityId] в src/ha-binding-status.ts,
пересобрал test-build (npx tsc -p tsconfig.test.json && node scripts/fix-test-build.mjs) и прогнал node --test test/ha-binding-status.test.mjs test/render-device-snapshot.test.mjs — результат 18 pass / 1 fail, упал
именно новый тест 'opening render availability trusts one frozen active projection' (assert на binary_sensor.yaml_only === true). После проверки
восстановил src/ha-binding-status.ts из git, пересобрал test-build и
перепрогнал npm test — снова 804/804, дерево чистое (git status --short
пусто, npx tsc -p tsconfig.test.json — сгенерированный test-build/ не
версионируется).
Что не прогонял и почему (соразмерность гейтов, PROCESS.md §8):
- Полный набор из 127 browser-смоков — diff задевает ровно одну точку
(
renderOpeningEntityAvailable) и один потребитель (opening contact/lock); прогнал новый целевой smoke плюс три регрессионных, напрямую покрывающих opening-биндинг и lock-security — этого достаточно для поверхности изменения, полный прогон здесь не добавил бы сигнала. npm run golden:verify— не прогонял. Обоснование не «со слов автора», а по коду:space-render.ts/logic.ts/любой геометрический или стилевой путь не тронуты; единственная правка убирает одно&&-условие, которое для уже зарегистрированных сущностей (у нихentities[eid]иstates[eid]совпадают одновременно) не меняет результат вообще — registry-backed painted frame буквально не может стать другим. Влияет только на entity без registry row, которых ни в одном golden-сценарии нет (не заведены YAML-only fixtures). Golden-эталоны диф не трогает.python -m pytest tests_backend -q— не прогонял, диф не касаетсяcustom_components/houseplan/**/*.py(нет таких файлов в списке изменений).- Performance-профили — не прогонял. ТЗ §14 явно заявляет «нет», и правка добавляет один and-член в уже читаемый на каждом кадре boolean — не новый проход по данным, не новая структура.
Находки
Находок уровня High и Medium нет.
Low не завожу: код, тесты и документация построены по написанному в ТЗ без отклонений, я не нашёл ни одного места, которое стоило бы поправить или снять с запиской.
Что проверено и корректно
- AC1 (registry-less live contact меняет presentation) — доказано
smoke:
closedContactControlsPresentation(closed) иstateTickSwapsOneAtomicFrame(open) наbinary_sensor.hp117_yaml_contactбез registry row (card.hass.entities[...] == nullподтверждено смоком). - AC2 (registry-less live lock badge) — доказано smoke:
yamlLockBadgeRendersLocked, затем.oplock.unlockedпосле смены состояния,unknownKeepsExistingTypeSemanticsдляunknown. - AC3 (frame — только immutable projection, не raw hass) — доказано и
чтением, и исполнением:
_renderOpeningEntityAvailable(src/houseplan-card.ts:16283) — однострочная делегация вrenderOpeningEntityAvailable(this._renderPlanHass, eid),this.hassне упоминается; source-contract тестtest/render-device-snapshot.test.mjsпроверяет это регексом (assert.doesNotMatch(..., /this\.hass\b/)) и подтверждает черезmethodBody. Мутационный гейт и мой ручной revert показывают, что тест красится при регрессии — не косметическая проверка. - AC4 (active registry entity — прежнее поведение) — не могло
измениться логически (см. «Что не прогонял», golden) и подтверждено
smoke-регрессией
smoke_opening_binding.mjs(exactOpeningReferencesStayActiveи другие). - AC5 (disabled/orphan/missing остаются unavailable) — доказано unit
(
test/ha-binding-status.test.mjs, веткиlock.disabled_entity,lock.disabled_parent,lock.orphan,lock.missing— всеfalse) и smoke (explicitDisabledRowsRemoveStaleStates, включая ожидание паузы на дебаунс реестра220msперед проверкой). Логика опирается на уже существующий (не изменённый этим дифом)activeRegistryHass()(src/ha-binding-status.ts:323-354) — прочитано построчно: state registry-less сущности проходит проекцию, только когда для неё нет соответствующей записиentities[eid]вовсе (значит проверки disabled не применяются), либо запись есть и явно не отключена. - AC6 (limited-registry live exact entity работает) — доказано unit
(
limitedFrameв тесте,authoritative: false). - AC7 (marker tombstone не блокирует opening reference) — доказано unit
(
markerTombstoneIsNotAnInput) и smoke (markerTombstonesDoNotBlockOpening, который дополнительно контрастирует с!card._planEntityAvailable(...)— показывает, что общий marker-путь по-прежнему блокируется, а opening-путь — нет, то есть сужение скоупа из ТЗ §4 не создало разрыва). - AC8 (lock security/unlock confirmation не меняются) — подтверждено
чтением:
_lockAction()(src/houseplan-card.ts:16411-16424) не входит в диф, по-прежнему проверяет live_openingEntityAvailable()передcallService, требуетconfirm()только дляunlock. Регрессия подтверждена исполнением:smoke_lock_action.mjs,smoke_lock_invariant.mjsзелёные, плюс новый smoke явно проверяетplanOpeningAndBadgeNeverActuate(тап по проёму/badge не вызываетcallService) иexplicitInfoActionStillWorks(только явное действие в открытой info-карточке вызываетlock.unlock). Инвариант SCOPE.md «The lock invariant, stated precisely» не ослаблен. - AC9 (state update не пересобирает geometry/config) — доказано smoke:
stateTickDoesNotRebuildGeometryOrConfigсравнивает_physicalBodiesCache,_cfgEpochи сериализованный_serverCfgдо/после тика состояния. - AC10 (существующие opening golden/interactions без регресса) — golden
baseline в дифе не тронут; логическое обоснование неизменности
registry-backed пути дано выше (AC4);
smoke_opening_binding.mjsзелёный подтверждает поведенческую регрессию отдельно от golden. - Трейлеры и changelog: все три коммита несут терминальные
Issue: #117иUser-Visible: yes|no; коммит сUser-Visible: yes(03cca97) правитdocs/CHANGELOG.mdиdocs/CHANGELOG.ru.mdв том же коммите (провереноgit show --stat 03cca97, оба файла присутствуют). - Документация:
docs/USER-GUIDE.ru.mdописывает новое поведение словами, согласующимися с уже принятой терминологией раздела («Контактный датчик», «замок»);docs/ARCHITECTURE.mdфиксирует правило проекции;docs/TESTING.mdдобавляет пункт с явным списком[auto: ...], все перечисленные файлы существуют и были прогнаны выше. - Соответствие ТЗ и SCOPE.md: правка не расширяет скоуп (§4 «не входит» дословно соблюдён — picker, миграция, marker lifecycle, generic registry-less для прочих marker-биндингов, geometry не тронуты), лежит внутри уже закрытых J4/J6, лок-инвариант не ослаблен.
Чего не проверял
- Полный набор из 127 browser-smoke (см. обоснование сужения выше) — предрелизный гейт, не гейт ревью.
npm run golden:verify— обоснование дано выше (логическая неизменность registry-backed пути плюс отсутствие изменений в geometry/render-стилях).python -m pytest tests_backend -q— Python не тронут этим дифом.- Performance-профили — ТЗ явно исключает влияние, изменение не добавляет новый проход по кадру.
- Реальную установку Home Assistant с живой YAML-сущностью без
unique_id— вне возможностей этого ревью; демо-стенд (demo/srv/demo.html) и его фейковыйhass/registry являются каноническим суррогатом поAGENTS.md/docs/DEVELOPMENT.md, и я прогнал его лично, а не со слов автора. - Историю ветки/ребейз на
dev— вне обязанностей код-ревью; это забота конвейера при слиянии (риск отмечен автором в хендоффе).
Вердикт
Зелёный. High: 0, Medium: 0. Каждый AC либо доказан автотестом, который я
лично прогнал и для которого лично подтвердил способность падать (unit —
ручным откатом фикса, smoke — mutation-гейтом), либо разобран чтением кода с
явной пометкой выше. Регрессионные lock-security смоки и SCOPE.md-инвариант
подтверждены исполнением, а не только чтением ТЗ. Продуктовый диф — одна
точка (renderOpeningEntityAvailable), в точности соответствующая описанию
и границам ТЗ; трейлеры, changelog (RU+EN) и три копии бандла в порядке.