Files
houseplan-card/docs/reviews/CODE-REVIEW-33-r2.md
T
claude[bot] 8819e390c9
Проверка (CI) / Классификация изменённых файлов (push) Successful in 28s
Проверка (CI) / Предполётные проверки: документация, провенанс, процесс (push) Failing after 1m16s
Проверка (CI) / Переиспользование: это дерево уже проверено (push) Successful in 1m24s
Проверка (CI) / HACS: валидация репозитория (push) Failing after 25s
Проверка (CI) / Hassfest: манифест интеграции (push) Failing after 16s
Проверка (CI) / Бэкенд: pytest в Home Assistant (push) Failing after 8m17s
Проверка (CI) / Фронтенд: типы, юниты, мутанты, синхрон бандла (push) Failing after 10m20s
Проверка (CI) / Смоки в браузере (шард 1 из 3) (push) Skipped
Проверка (CI) / Смоки в браузере (шард 2 из 3) (push) Skipped
Проверка (CI) / Смоки в браузере (шард 3 из 3) (push) Skipped
Проверка (CI) / Смоки: все шарды зелёные (push) Skipped
Проверка (CI) / Golden-кадры против принятых эталонов (push) Skipped
Проверка (CI) / Перф-смок: бюджет времени кадра (push) Skipped
docs: review document for #33
Issue: #33
User-Visible: no
2026-08-30 10:09:14 +00:00

18 KiB
Raw Blame History

CODE-REVIEW-33-r2

  • Issue: https://github.com/Matysh/houseplan-card/issues/33
  • ТЗ: docs/specs/033-config-schema-lifecycle.md (ревизия 3, зелёное ревью SPEC-REVIEW-33-r2)
  • Ветка: issue/33-config-schema-lifecycle
  • SHA материала ревью: 15fa33b9d965ee463a8e61e8de2c13dec41a1832 (git rev-parse HEAD)
  • Заход: r2 · блокирующих циклов израсходовано 1 из 4
  • Предыдущий раунд: CODE-REVIEW-33-r1, жёлтый, на SHA 572cf928969fd8849299170ba3d42ff61d7ef423
  • Дельта этого раунда: git diff 572cf928..15fa33b9 — один коммит 15fa33b9 («fix: close CODE-REVIEW-33-r1 M1-M3»), трейлеры Issue: #33, User-Visible: no

Скоуп раунда

Разбор ПО ДЕЛЬТЕ, не заново — согласно PROCESS.md §2.9/§2.7 и правилу этого цикла. Дельта локальна: 6 файлов, ни один не src/**, ни один не геометрия/схема:

Файл Изменение
scripts/config-audit.mjs сужение MIGRATION_STATUSES (закрывает M3)
test/config-schema-parity.test.mjs переписан AC7-тест: весь src/**, актуальное имя дампа (закрывает M1)
docs/CHANGELOG.md, docs/CHANGELOG.ru.md, docs/ARCHITECTURE.md удалён задвоенный абзац/раздел (закрывает M2)
docs/reviews/CODE-REVIEW-33-r1.md публикация документа предыдущего раунда (артефакт конвейера, не код)

Рантайм-поведение конфига не менялось (не менялось и в r1); дельта — это ровно три точечные правки по числу находок r1, без побочных изменений скоупа. Условие «разбор остаётся полным» (ребейз, новый контракт поведения, новая подсистема, объём дельты сопоставим с задачей) не выполняется ни по одному пункту — сокращаю объём разбора до дельты и наследую остальное.

Закрытие раунда r1

Находка r1 Чем закрыта Где это видно
M1 — AC7-тест искал строку 'config-schema-manifest', которой нет нигде после переименования файла в f4318721; тест не мог падать. Тест переписан: рекурсивный обход всего src/** (readdirSync с withFileTypes, фильтр по .ts/.js/.mjs/.json), поиск актуальной строки 'config-schema.json'. test/config-schema-parity.test.mjs:96-106. Проверено исполнением заново в этом раунде: дописал в src/houseplan-card.ts строку fetch('./scripts/config-schema.json') (тот же приём, что в r1) и перезапустил тест — теперь not ok 3, тест красный с точным указанием файла; после отката файла — снова зелёный (node --test test/config-schema-parity.test.mjs → 3/3).
M2 — абзац/раздел #33 задвоен в CHANGELOG.md, CHANGELOG.ru.md, ARCHITECTURE.md (второй feat-коммит повторил правки первого). Вторая копия удалена в каждом из трёх файлов. Диф 15fa33b9: docs/ARCHITECTURE.md −24 строки, docs/CHANGELOG.md −7, docs/CHANGELOG.ru.md −7. Проверено чтением и подсчётом в этом раунде: grep -c на характерную фразу в каждом файле даёт 1 (было 2). Заголовок ## Schema as the source of truth (#33, 2026-08-30) встречается в ARCHITECTURE.md один раз (docs/ARCHITECTURE.md:1503).
M3 — config-audit.mjs расширил exit-код 3 на drop-on-validation и decision-required; для decision-required (group_lights/exclude_integrations — живые поддерживаемые поля, ждущие решения #44) это семантически неверно и вводит в заблуждение. MIGRATION_STATUSES сужен ровно до контракта ТЗ Блока 3: migrate-on-write, migrate-on-settings-save, deprecated-read. Новый комментарий объясняет, почему оба статуса исключены. scripts/config-audit.mjs:80-88. Проверено исполнением заново, своими фикстурами (тест задачи это не покрывает — см. ниже): {settings:{group_lights:true}} → exit=0 (было бы 3 до фикса); {spaces:[{...,aspect:1.2}],...} → exit=0. Контракт спеки (docs/specs/033-config-schema-lifecycle.md:102-105, «3 — migration available (найдены поля со статусом migrate-*/deprecated-read)») теперь воспроизведён буквально.

Ни одна находка не закрыта «на словах»: по каждой — либо перегон теста с воспроизведением регрессии (M1), либо построчная сверка файла (M2), либо собственный ad hoc прогон, раз штатный test/config-audit.test.mjs фикстуры с decision-required/drop-on-validation не содержит и не мог бы отличить старое поведение от нового (M3 — см. «Находки» ниже, это Low, не блокирует раунд).

Унаследовано из r1

Из CODE-REVIEW-33-r1 (SHA 572cf928) принимается без повторной проверки — дельта этого раунда не касается доказательной базы этих пунктов:

  • AC1 (свежесть/детерминизм/полнота манифеста, 265 путей) — доказано в r1 исполнением (dump-config-schema.py --check, pytest freshness). Файлы манифеста и дамп-скрипт не менялись между 572cf928 и 15fa33b9.
  • AC2/AC3 (parity 8 пар + анти-гниение allow-list) — доказано в r1 мутантом schema-manifest-enum-drift (1/1). scripts/schema-compat-allowlist.mjs и validation.py не менялись в дельте; сам parity-блок теста (строки 1-92) не тронут — правка коснулась только AC7-блока в конце файла.
  • AC4 (полнота registry) — доказано в r1 мутантом registry-selector-dead-decision (1/1). scripts/config-field-registry.mjs не менялся в дельте.
  • AC5 (lossless-фикстуры, round-trip будущих полей) — доказано в r1 pytest на трёх фикстурах test/fixtures/config-lifecycle/. Фикстуры не менялись в дельте.
  • enforcedBy для 4 реализованных механизмов (show_all, weather_entity, ripple, aspect/segments) — подтверждено чтением кода в r1, код этих механизмов не менялся.
  • Трейлеры и разнесение классов (build-коммит 572cf928 отдельно от feat) — оценено в r1, не относится к дельте этого раунда (у неё свой коммит со своими трейлерами, проверен отдельно выше).
  • Не-скоуп (поведение конфига, судьба group_lights/exclude_integrations как таковая — не паспорт, а решение #44) — подтверждено в r1, дельта его не расширяет и не сужает.

Что проверено в этом раунде дополнительно к таблице закрытия

  • AC6 (exit-коды 0/3/2) пересмотрен целиком, не только по факту закрытия M3: штатный test/config-audit.test.mjs (не менялся в дельте) по-прежнему зелёный — current.json → 0, oldest-supported.json → 3 (за счёт show_all/weather_entity/ ripple, которые остались в MIGRATION_STATUSES; aspect в той же фикстуре имеет статус drop-on-validation и больше не участвует, но фикстура всё равно даёт 3 по другим находкам — тест не мог бы заметить сужение сам по себе), broken.json → 2. Отсутствие штатного теста именно на decision-required/drop-on-validation в изоляции — Low-разрыв в покрытии AC6, не блокирует раунд (см. «Находки»).
  • AC7 пересмотрен целиком (это и есть предмет M1): тест теперь читает весь src/** (102 файла верхнего уровня плюс поддиректории editors/, render/, i18n/ — итого 115 файлов по find src -type f), а не 2 файла, как раньше. Проверил, что обход не пропускает поддиректории (реализация рекурсивна, entry.isDirectory() идёт в walk снова) и что фильтр расширений (.ts/.js/.mjs/.json) не даёт ложноположительных срабатываний на бинарные/генерируемые артефакты.

Гейты, прогнанные в этом раунде

Зелёного Validate на SHA 15fa33b9 нет — прогнал дешёвые гейты лично.

Гейт Команда Результат
typecheck npx tsc --noEmit чисто
unit (полный) npm test 1608 pass / 0 fail / 1 skipped — то же число, что в r1, дельта не сдвинула счётчик
docs fingerprint node scripts/check-docs.mjs «Documentation checks passed (7 files, 10 external links)» — не обязателен (дельта не трогает src/**), прогнан из осторожности, т.к. дёшев
AC7-тест изолированно, с воспроизведением регрессии node --test test/config-schema-parity.test.mjs (дважды: с посаженным fetch('./scripts/config-schema.json') в src/houseplan-card.ts, и после отката) красный с посаженной регрессией → зелёный (3/3) после отката — тест умеет падать, доказано в этом раунде заново
M3 руками, две ad hoc фикстуры node scripts/config-audit.mjs <config с только group_lights> и <config с только aspect> оба exit=0 (было бы 3 до фикса)

Не прогонял и почему:

  • npm run build + сверка трёх копий бандла — дельта не трогает ни один файл src/** и ни один бандлируемый скрипт; dist/**/custom_components/.../frontend/** не менялись (git diff --stat 572cf928..15fa33b9 подтверждает — их нет в списке). Билд не мог измениться, гонять его — тратить время без нового сигнала.
  • npm run bundle:budget — по той же причине (нет изменений в бандлируемом коде).
  • npm run invariants — дельта не трогает геометрию/layout/marker.space/open_spans; повторный прогон унаследован из r1 (фикстуры lifecycle не менялись).
  • Мутанты schema-manifest-enum-drift/registry-selector-dead-decision — код, который они бьют (validation.py, config-field-registry.mjs, allowlist), не менялся в дельте; результат унаследован из r1.
  • python -m pytest tests_backend -q — дельта не трогает custom_components/**/*.py и не трогает tests_backend/**; ноль оснований перегонять.
  • Браузерные смоки / smoke-select.mjs — дельта не содержит ни одного изменённого символа src/** (там вообще нет правок src/**, кроме временной ad hoc-мутации, откаченной после проверки); инструмент нечего сравнивать, не запускал.
  • golden:verify — дельта не меняет рендер/геометрию/стили.

Находки

Три находки r1 закрыты состоятельно (см. таблицу выше). Новых Medium/High дельта не принесла. Одна Low-находка, заводимая как заметка, а не блокирующая:

L1 — AC6 не имеет штатного регрессионного теста на изолированные decision-required/drop-on-validation

test/config-audit.test.mjs не менялся в этой правке и по-прежнему проверяет только три lifecycle-фикстуры Блока 3, ни одна из которых не изолирует «только decision-required» или «только drop-on-validation». Штатный прогон npm test зелёный и до, и (гипотетически) после отката M3-фикса — потому что oldest-supported.json всё равно даёт exit 3 через show_all/weather_entity/ripple. То есть M3 закрыта фактически (проверено вручную выше), но не закреплена тестом задачи — будущая правка может случайно вернуть decision-required в MIGRATION_STATUSES, и штатный набор этого не заметит. Не блокирует: серьёзность Low, находка технической полноты покрытия, а не дефект поведения на HEAD. Можно снять правкой (добавить кейс с изолированным decision-required-конфигом в test/config-audit.test.mjs) либо принять с записью — решение автора.

Что проверено и корректно (сводно, r1 + r2)

  • Все три Medium из r1 закрыты и перепроверены исполнением в этом раунде (таблица «Закрытие раунда r1»).
  • AC1-AC5 унаследованы из r1 без повторной проверки (см. «Унаследовано из r1») — доказательная база не затронута дельтой.
  • AC6 пересмотрен целиком в контексте M3-фикса: контракт exit-кодов теперь буквально соответствует ТЗ (docs/specs/033-config-schema-lifecycle.md:102-105).
  • AC7 пересмотрен целиком в контексте M1-фикса: тест теперь способен поймать регрессию, которую называет своей целью, и покрывает весь src/**, а не 2 файла.
  • Дублирование документации (M2) устранено без остатка в трёх файлах.
  • Трейлеры коммита 15fa33b9 корректны: Issue: #33, User-Visible: no — правки не вводят новое видимое поведение (дедуп уже анонсированного changelog-абзаца плюс внутренняя коррекция dev-инструмента и теста), отдельного изменения в changelog не требуется.
  • Один источник числа: у этой дельты нет ни одной новой пользовательской величины (правки — тест, CLI dev-инструмент, дедуп документации) — правило не применяется.

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

  • Полный tests_backend с реальной Home Assistant — не установлена в этой песочнице; дельта в любом случае не трогает custom_components/**/*.py.
  • Полный набор мутантов и golden/браузерные смоки — не относятся к дельте (см. таблицу гейтов выше с обоснованием по каждому пункту).
  • Судьба group_lights/exclude_integrations как продуктовое решение (#44) — вне скоупа #33, не судил.

Вердикт

Зелёный. High: 0, Medium: 0. Все три Medium-находки r1 закрыты состоятельно и перепроверены исполнением заново в этом раунде (не только чтением диффа): M1 — тест подтверждённо умеет падать на реальном воспроизведении старой регрессии; M2 — дублирующиеся абзацы отсутствуют во всех трёх файлах; M3 — контракт exit-кодов воспроизведён буквально по ТЗ и проверен на изолированных ad hoc конфигурациях, которых штатный набор задачи не покрывает (отсюда одна Low-заметка о неполноте покрытия AC6, не блокирует). Дешёвые гейты (tsc, npm test 1608/0/1, check-docs) зелёные на HEAD; остальные гейты обоснованно унаследованы из r1 или признаны неприменимыми к локальной дельте.