mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -0,0 +1,125 @@
|
||||
# CODE-REVIEW-625-r2
|
||||
|
||||
**Issue:** [#625](https://github.com/Matysh/houseplan-card/issues/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-теста)
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/625-backend-io-invariants`, коммит `53b20e8c45a0` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `c8eb775434fdc6139180948ff5ac6b42311228ae`
|
||||
```
|
||||
git log --all --format='%H %T' | grep c8eb775434fd
|
||||
```
|
||||
- Тело issue: `f9d2f583a9c977bd535f6f14de24ad8c8757914db26218bea46875ad89fbaa67`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user