Files
2026-09-23 11:49:34 +00:00

20 KiB
Raw Permalink Blame History

CODE-REVIEW-625-r2

Issue: #625 — backend I/O, конкурентность и целостность конфигурации Стадия: код-ревью, заход r2 (r1 — жёлтый, единственная Medium-находка M1) Материал: 53b20e8c45a072e72bbbbcac3be68cacce039a18 (рабочая копия уже на нём) Класс изменения: B (tests_backend/**, scripts/mutation-registry.mjs) — продуктовый код (класс A) в этом раунде не тронут Трек: унаследован из r1 (полный, обоснование см. в CODE-REVIEW-625-r1.md) — сам r2 дельту не переоценивает, она ýже, чем порог смены трека

Дельта раунда

Находка предыдущего раунда: CODE-REVIEW-625-r1.md, материал 15bc6bcf5a69ef3423badfc61311dd0efc072030, вердикт жёлтый, Medium M1. Между материалом r1 и текущим HEAD ровно два коммита:

15bc6bcf..53b20e8c:
  f10dd1eb docs: review document for #625      (публикация документа r1 — не код)
  53b20e8c test(backend): покрыть marker-id write boundaries (#625)

git diff 15bc6bcf..53b20e8c по содержательным файлам:

scripts/mutation-registry.mjs      | 39 +++++++++++++++++
tests_backend/test_ha_websocket.py | 107 ++++++++++++++++++++++++++++++++++

Ни один файл custom_components/houseplan/** (продуктовый код) в дельте не тронут. Дельта строго локальна: только новые тесты и новые мутанты, целящиеся ровно в находку M1. Смены подсистемы, ребейза на ушедший вперёд dev, изменения контракта — ничего из перечисленного в критериях «разбирай полностью» нет. Объём разбора этого раунда — дельта плюс закрытие M1, остальное наследуется из r1.

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

Находка r1 Чем закрыта Где это видно
M1 (Medium, в скоупе). Три из четырёх точек вызова validate_active_marker_ids (ws_config_set websocket_api.py:1692, ws_space_delete :1939, ws_plan_optimize :2085) не имели ни одного теста, отправляющего дублирующийся id через сам WS-хендлер; пустой третий столбец таблицы «чем краснеет». Добавлены три HA-tier WS-теста, по одному на каждую названную точку: test_config_set_rejects_duplicate_active_marker_ids, test_space_delete_rejects_changed_legacy_duplicate_marker_ids, test_plan_optimize_rejects_duplicate_active_marker_ids. Каждой точке сопоставлен собственный мутант в mutation-registry.mjs, патчащий именно эту строку вызова (config-set-skips-active-marker-id-invariant, space-delete-skips-active-marker-id-invariant, plan-optimize-skips-active-marker-id-invariant). Материал r1 просил минимум одного теста на ws_config_set и «по возможности» — на остальные два; сделаны все три. tests_backend/test_ha_websocket.py:1095-1201; scripts/mutation-registry.mjs (3 новых блока после якоря quota-reserves-staged-bytes-twice-on-disk). Реальный прогон — см. «Как проверялось» ниже: все три мутанта пойманы в CI на этом же SHA.
L1 (Low, снят записью в r1, не требовал правки) Код не тронут этой задачей — дельта r2 его не касается, ничего не изменилось. —
L2 (Low, снят записью в r1, не требовал правки) Код не тронут этой задачей — дельта r2 его не касается. —

Процессное замечание (не блокирует, фиксирую по требованию инструкции ревью): вердикт r1 в комментарии issue (2026-09-23T11:20:42Z) не называет SHA материала — это само по себе находка правила «SHA обязателен в вердикте». Сам документ CODE-REVIEW-625-r1.md SHA называет (15bc6bcf..., раздел «Материал раунда»), поэтому цепочка восстановима и трасса не потеряна; на итог ревью не влияет.

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

Прочитан полный диф дельты (git diff 15bc6bcf..53b20e8c) построчно — оба файла целиком аддитивны (только +-строки, ни одной существующей строки не тронуто/удалено).

Проверка соответствия мутантов коду. Для каждого из трёх новых мутантов сверил find-якорь с текущим содержимым websocket_api.py (grep -n -C4 validate_active_marker_ids):

  • websocket_api.py:1692 — validate_active_marker_ids(msg["config"], data.get("config")) — совпадает с якорем config-set-skips-active-marker-id-invariant дословно;
  • websocket_api.py:1939 — validate_active_marker_ids(target_config, current_config) — совпадает с якорем space-delete-skips-active-marker-id-invariant дословно;
  • websocket_api.py:2085 — validate_active_marker_ids(msg["config"], config_data.get("config")) — совпадает с якорем plan-optimize-skips-active-marker-id-invariant дословно.

Расхождений нет, все три патча бьют ровно в названные в M1 строки, не в соседние.

Проверка логики новых тестов (чтением).

  • test_config_set_rejects_duplicate_active_marker_ids — шлёт config/set с двумя активными марkerами одного id, ожидает success: false, error.code == "invalid_config", error.message == "duplicate active marker id", затем читает config/get и проверяет rev == 0, markers == [] — подтверждает, что отклонённая запись не изменила состояние. Прямой позитивный сценарий: новый дубликат всегда отклоняется.
  • test_space_delete_rejects_changed_legacy_duplicate_marker_ids — сначала пишет напрямую в Store (в обход валидации) legacy-пару дублей, привязанных к удаляемому пространству, затем шлёт space/delete; ожидает отказ и что обе Store (config и layout) остались побайтово равны состоянию до запроса. Это именно тот сценарий из ТЗ («любое изменение конфликтующей группы обязано оставить ≤1 активный, иначе отклонить») и проверяет атомарность отката по двум хранилищам, а не только по одному.
  • test_plan_optimize_rejects_duplicate_active_marker_ids — сидирует пустую конфигурацию через config/set, затем шлёт plan/optimize с новым (ранее не существовавшим) дублирующимся id; ожидает отказ и что config/layout и их rev не изменились. Новый дубликат при optimize отклоняется так же, как и при обычной записи — тест закрывает именно «optimize не обходной путь».

Все три ассерта на состояние делают то, что требует §2.7: тест не может пройти молча при снятом guard — без вызова validate_active_marker_ids запрос вернул бы success: true и состояние бы изменилось, оба assert'а на это отреагируют.

Проверка «тест умеет падать» — реальным прогоном в CI, а не декларацией. HA-tier тесты (test_ha_*.py) в этой песочнице не выполняются (нет homeassistant, нет .venv-backend — то же ограничение, что и в r1). Вместо повторной ссылки на заявление автора нашёл и прочитал логи джобов мутационного гейта Validate на точном материале 53b20e8c (run 35855432979, тот самый, что процитирован в системном контексте задачи как «дешёвые гейты подтверждены»):

  • job «Мутанты по диффу (3/6)»: ok config-set-skips-active-marker-id-invariant: заявленный тест покраснел на мутанте, поймано 27 из 27;
  • job «Мутанты по диффу (1/6)»: ok space-delete-skips-active-marker-id-invariant: заявленный тест покраснел на мутанте, поймано 28 из 28;
  • job «Мутанты по диффу (6/6)»: ok plan-optimize-skips-active-marker-id-invariant: заявленный тест покраснел на мутанте, поймано 27 из 27.

mutation-gate.mjs перед применением патча гоняет runCleanGuards — то есть каждый из трёх новых тестов сначала прошёл зелёным на чистом дереве в реальном Home Assistant (иначе гейт упал бы на этапе clean guard, до применения мутанта), а затем красным — после патча, снимающего вызов валидатора. Это ровно тот эксперимент «guard снят → тест падает, guard на месте → тест проходит», который §2.7 требует от ревьюера, выполненный не мной локально (technически недоступно), а настоящим CI-раннером с Home Assistant на именно этом SHA. Все три шарда завершились success, ни одного FAIL/unverifiable/предсуществующий рядом с этими тремя id.

Это закрывает M1 сильнее, чем требовал минимум r1 (минимум был — один тест на ws_config_set; сделаны все три названные точки), и с более весомым доказательством, чем «личная проба» — потому что личная проба здесь физически невозможна (нет HA в песочнице), а CI-проба — настоящий прогон на материале ревью, а не по ссылке на заявление автора.

Гейты

Гейт Статус Как получен
npx tsc --noEmit, npm test, npm run build+сверка бандлов зелёные Validate на 53b20e8c (run 35855432979) — переиспользован по инструкции задачи, не перегонялся: дельта не трогает src/** и не тестонезависима от этих гейтов
Мутационный гейт по диффу (6 шардов, весь диапазон origin/dev...53b20e8c) зелёный, лично проверено по логам тот же run 35855432979; отдельно вычитаны логи трёх шардов, содержащих три новых мутанта — все пойманы, все шарды поймано N из N
python -m pytest tests_backend -q (полный, включая HA-tier) не переисполнялся отдельно недоступен локально (нет homeassistant/.venv-backend); заменён более сильным доказательством — реальным прогоном именно этих трёх тестов в мутационном гейте CI (см. выше), это строже, чем просто «зелёный набор»
node scripts/check-docs.mjs не требуется 0 файлов src/** в дельте — отпечаток скриншотов не мог устареть
npm run golden:verify, browser smokes, performance-профили не требуются дельта не трогает src/**, визуальной поверхности нет; подтверждено и самим Validate (Смоки, Golden, Перф-смок, «Бэкенд: pytest в Home Assistant» — все skipped через job «Переиспользование: это дерево уже проверено», т.к. production backend-код в дельте не менялся)
python -m pytest tests_backend/test_validation.py и другие pure-юниты не переисполнялись отдельно в этом раунде дельта их не трогает; логика самого валидатора (validation.py) не менялась — только точки вызова покрыты тестами; наследуется из r1, где эти файлы лично прогонялись
node scripts/smoke-select.mjs не переисполнялся 0 файлов src/**, тот же вывод, что и в r1

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

Документ: docs/reviews/CODE-REVIEW-625-r1.md (закоммичен как f10dd1eb), материал 15bc6bcf5a69ef3423badfc61311dd0efc072030. Дельта этого раунда не трогает продуктовый код и не задевает ни одного из перечисленных ниже доказательств — принимаются без повторной проверки:

  • Скоуп и трек — полный трек, обоснование (несколько поверхностей, конкурентность, изменение контракта валидации, сложность 8/10) не пересматривается.
  • AC1, AC2, AC4–AC13 — разобраны в r1 построчным чтением диффа и (для «чем краснеет») тремя личными отрицательными пробами на чистых pure-юнитах (test_atomic_write_keeps_destination_and_cleans_temp_when_replace_fails, test_runtime_controller_coalesces_rapid_toggles_into_one_durable_write, test_async_teardown_flushes_pending_debounced_state_and_closes_handles) плюс сверкой 46 мутационных якорей. Ни один из этих файлов не изменился между 15bc6bcf и 53b20e8c.
  • AC3 (кроме witness-пробела, закрытого выше) — семантика delta-aware валидатора (validation.py:37-62), tombstone-fix в ws_layout_update (websocket_api.py:829-832) и unit-тесты самого валидатора (test_validation.py) — код не менялся в этом раунде, разбор из r1 действителен.
  • «Найдено и корректно» — deepcopy под write_lock в ws_export_create, upload-preflight с payload_floor, VirtualLightController (единственная _save_task, корректный flush), redaction diagnostics через async_redact_data, quality_scale.yaml — ни один из этих файлов не в дельте r2.
  • Трейлеры и changelog — терминальный коммит 965bbb05 (User-Visible: yes, оба changelog в этом же коммите) проверен в r1; коммиты 4051899b, 15bc6bcf — User-Visible: no, корректно. Новый коммит 53b20e8c — тоже User-Visible: no (только тесты и мутанты, новой видимой пользователю поверхности нет) — согласуется с той же логикой, отдельной проверки не потребовалось.
  • Low L1, L2 — оба сняты записью в r1 без требования правки; дельта их не касается.

Находки

Нет. High: 0. Medium: 0.

Единственная находка предыдущего раунда (M1) закрыта точно по названному в r1 рецепту («добавить минимум один WS-уровневый тест на ws_config_set; по возможности — на остальные три точки») — сделаны все три, с собственным мутантом на каждую и подтверждённой поимкой в реальном CI-прогоне на материале ревью.

Итог

High: 0. Medium: 0. Low: 0 новых (два унаследованных из r1 остаются сняты записью).

Дельта раунда — узкая, чисто тестовая правка, точно закрывающая единственную блокирующую находку r1. Мутационный гейт на точном SHA 53b20e8c подтверждает реальным прогоном в Home Assistant, что все три новых теста и падают при снятом guard, и проходят при включённом — это сильнее минимального требования r1.

Вердикт: зелёный.


Материал раунда

  • SHA: 53b20e8c45a072e72bbbbcac3be68cacce039a18
  • Дельта: git diff 15bc6bcf..53b20e8c — 2 файла, +146/−0 (плюс некодовый docs/reviews/CODE-REVIEW-625-r1.md, +140, публикация предыдущего документа)
  • Полное дерево: git diff origin/dev...HEAD — унаследовано из r1 (26 файлов, +1341/−138 на материале r1; в r2 к этому добавлены только два файла дельты выше)
  • Ветка: issue/625-backend-io-invariants
  • Validate на точном SHA: run 35855432979 — success (мутационный гейт по диффу, 6/6 шардов, включает три новых witness-теста)

Материал раунда

  • Ветка: issue/625-backend-io-invariants, коммит 53b20e8c45a0 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
  • Дерево материала: c8eb775434fdc6139180948ff5ac6b42311228ae
    git log --all --format='%H %T' | grep c8eb775434fd
    
  • Тело issue: f9d2f583a9c977bd535f6f14de24ad8c8757914db26218bea46875ad89fbaa67
  • Вердикт конвейера: green · High 0