From a0a9e4b57964637ba33aaf0c9ae3eee6702a459b Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Tue, 8 Sep 2026 21:45:32 +0000 Subject: [PATCH] docs: review document for #491 Issue: #491 User-Visible: no --- docs/reviews/SPEC-REVIEW-491-r1.md | 165 +++++++++++++++++++++++++++++ 1 file changed, 165 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-491-r1.md diff --git a/docs/reviews/SPEC-REVIEW-491-r1.md b/docs/reviews/SPEC-REVIEW-491-r1.md new file mode 100644 index 00000000..b0ed8dac --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-491-r1.md @@ -0,0 +1,165 @@ +# SPEC-REVIEW-491-r1 + +Issue: [#491](https://github.com/Matysh/houseplan-card/issues/491) — «Optimize/Undo: не терять незавершённую парную транзакцию при следующей записи». +Этап: ревью ТЗ (PROCESS.md §2.4), полный трек (P1/bug, лёгкий трек не проходит по +сложности/риску и числу затронутых backend-эндпоинтов — названо аналитиком). +Заход: r1 · блокирующих циклов израсходовано 0 из 4. + +Материал: `docs/specs/491-optimize-undo-pair-recovery.md` на коммите +`286f11b24efa4b95b26c16619f768ca1cd2c5b75` (рабочая копия уже на нём), тело +issue #491 и оба комментария (аналитика, передача на ревью). + +## Скоуп + +ТЗ описывает защиту SCOPE J6: незавершённая (crash/error) парная запись +config+layout, оставленная `plan/optimize` или `plan/optimize_undo`, не должна +уничтожаться следующим обычным writer'ом (`config/set`, `layout/set`, +`layout/update`, `layout/delete`, `geometry/repair`) до того, как эта пара +доведена до целевого состояния или отката. Предлагается обобщить уже +существующий retry→rollback протокол `_commit_import_pair` (Import, +space-delete) на Optimize/Undo и добавить write fence перед каждым ordinary/ +paired writer. + +## Как проверялось + +Ревью ТЗ на этом этапе кода не существует (задача ещё не реализована), поэтому +предмет проверки — не гейты, а полное чтение текущего backend-кода, на который +ссылается ТЗ, чтобы отличить точный технический разбор от догадки, выданной за +факт (риск, прямо названный в промпте ревью). + +Прочитано целиком и построчно сверено с текстом ТЗ: + +- `custom_components/houseplan/websocket_api.py` — `ws_plan_optimize` (строки + 1890–2063), `ws_plan_optimize_undo` (2074–2148), `ws_config_set` (1516–1687, + включая `_discard_optimizer_snapshot` на 1657), `ws_layout_set` (678–729), + `ws_layout_update` (739–796), `ws_layout_delete` (1307–1328), + `ws_geometry_repair` (810–902), `ws_space_delete` (1789–1877, использует + `_commit_import_pair`), `ws_import_apply` (491–599, тот же helper), + `_persist_pair_intent` / `_converge_pair` / `_commit_import_pair` (311–382), + `_discard_optimizer_snapshot` (180–191), `_layout_metadata` (303–308); +- `custom_components/houseplan/__init__.py` целиком — startup-резолвер + `optimize_pending` (160–222) и compatibility-fallback для pending без + `final_metadata` (189–194); +- `docs/ARCHITECTURE.md` (окрестности 1190–1230) — существующий + задокументированный контракт `optimize_pending`/`optimize_backup` для + Import, который ТЗ предлагает распространить на Optimize/Undo; +- `docs/CONFIG-COMPATIBILITY.md`, `docs/USER-GUIDE.ru.md` — подтверждение, что + ТЗ не описывает несуществующее поведение (существующий обобщённый сценарий + ошибки сохранения на месте, новых терминов нет); +- `tests_backend/test_ha_websocket.py` — существование теста + `test_plan_optimize_pair_and_one_deep_undo_survives_geometry_repair`, + названного в AC8, подтверждено (`grep`, файл найден). + +Инструментальные гейты (typecheck/test/build) на этом этапе неприменимы: ТЗ не +меняет код. Вопрос уместности полного трека и учёта дельты (§2.9/§2.10) не +возникает — это первый заход. + +## Находки + +Ни одной High или Medium находки. Технические расхождения между ТЗ и +фактическим кодом не обнаружены — каждое утверждение раздела «Проблема и +подтверждённая причина» и раздела «Термины и инварианты» проверено чтением +конкретной строки: + +- «`layout/set`, `layout/update` и `layout/delete` намеренно удаляют + `optimize_backup` и `optimize_pending`» — подтверждено, все три вызывают + `async_save_layout_state(..., remove=(_OPTIMIZE_BACKUP, _OPTIMIZE_PENDING))` + безусловно при фактической записи (строки 725, 793, 1324); +- «`config/set` после durable config write вызывает + `_discard_optimizer_snapshot`» — подтверждено, вызов на строке 1657, после + `async_save_config_state` на 1647, best-effort (`except Exception`); +- «Optimize/Undo не используют существующий retry → rollback протокол... уже + защищены Import и удаление пространства» — подтверждено: `ws_plan_optimize` + и `ws_plan_optimize_undo` пишут intent→config→layout напрямую, без обёртки + в `_commit_import_pair`, которую `ws_import_apply` и `ws_space_delete` + используют дословно; +- «Setup умеет завершить пару... старые pending без `final_metadata` + продолжают обрабатываться по compatibility fallback» — подтверждено кодом + `async_setup_entry` (`__init__.py:189-194`): при отсутствии `final_metadata` + используется старое поведение (сохранить/очистить `optimize_backup` по + `clear_backup`). + +Догадок, выданных за решённый факт, не найдено: единственное потенциально +продуктовое следствие — «неразрешимый pending блокирует последующие обычные +writer'ы до восстановления или перезапуска» (AC6, §7 п.5) — не изобретение +автора ТЗ, а дословно названная владельцем граница в теле issue («либо явное +блокирование обычных writers до разрешения»). Раздел 18 «Принятые технические +предположения» корректно ограничен нефункциональными решениями (место +resolver'а, переиспользование `_commit_import_pair`, код ошибки, +read-repair-исключения, порядок permission-check/fence) и явно помечен как +предположения, которые ревьюер вправе оспорить — оспаривать нечего, все пять +следуют из существующего кода Import. + +## Что проверено и корректно + +- Обязательные разделы §7.1 присутствуют все: сценарий, что человек увидит, + проблема, скоуп/не-скоуп, контракт (§§6–9), UX/i18n, модель данных и + совместимость, AC1–AC10 с доказательством, план автотестов, риски, откат, + release-артефакты; +- первые два раздела продуктовые и отвечают на персону/поверхность/момент + (администратор дома, десктоп-редактор, окно между Optimize/Undo и + следующей правкой) и «что человек увидит» одной фразой без терминов + реализации; +- каждый AC1–AC10 указывает способ доказательства (`backend`/HA-harness, + мутационный свидетель либо явное «чтение кода» для AC5/AC9/AC10) — + соответствует требованию §2.5 DoR; +- обоснование полного трека именует нарушенный критерий `small` (сложность/ + риск выше 3, несколько backend-эндпоинтов) — не голословное «обычный + трек»; +- продуктовых вопросов владельцу нет, и оснований считать это неполным не + нашлось: единственное поведенческое следствие (блокировка writer'ов до + разрешения) прямо взято из границ, заданных владельцем в теле issue; +- i18n (нет новых ключей), touch/View/Kiosk (без нового взаимодействия), + производительность (одно дополнительное чтение metadata под уже + существующим `write_lock`), откат (без миграции данных) — все названы + явно, ни один пункт DoR не обойдён молчанием; +- модель данных: новых persisted-полей нет, что подтверждено чтением — + `final_metadata`/`kind`/`clear_backup` уже присутствуют в структуре pending, + используемой Import (строки 568-594, 1837-1857 websocket_api.py), ТЗ лишь + обобщает их на Optimize/Undo, не меняя формат; +- список затронутых файлов и модулей присутствует (в комментарии аналитика, + на который ссылается ТЗ) и совпадает с фактическим расположением кода, + прочитанным при этом ревью. + +## Чего не проверял + +- Не прогонялись `typecheck`/`test`/`build` — на этапе ревью ТЗ продуктовый + код не менялся, дешёвые гейты неприменимы; +- не проверялась реализация write fence, retry/rollback-обобщения или + HA-harness fault-injection тестов — их ещё не существует, это предмет + будущего код-ревью (S6→S7), включая обязательную для защитных AC таблицу + «чем краснеет» (PROCESS §2.7); +- не оценивалась производительность на реальном профиле — ТЗ прямо говорит + «нет» влияния и обоснование (одно доп. чтение под уже держащимся локом) + достаточно для стадии ТЗ; +- не проверялись открытые issue на дубликаты сверх названных автором (#87, + #466) — беглый просмотр их описаний в тексте ТЗ/issue не выявил + противоречий, глубокий поиск дубликатов не проводился. + +## Вывод + +ТЗ технически точное (каждое утверждение о текущем коде проверено чтением и +подтвердилось), полное по составу разделов, каждый AC проверяем и несёт +способ доказательства, продуктовая рамка (сценарий/персона/до-после) на +месте, единственное поведенческое следствие для пользователя прямо +санкционировано владельцем в теле issue, догадок под видом решений не +обнаружено. + +**Вердикт: зелёный.** + +--- + + + +## Материал раунда + +- Ветка: `issue/491-optimize-undo-pair-recovery`, коммит `286f11b24efa` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `3ec8105ab515d6fe60692dda82b4345bfc8d411a` + ``` + git log --all --format='%H %T' | grep 3ec8105ab515 + ``` +- ТЗ `docs/specs/491-optimize-undo-pair-recovery.md`, блоб `1a496fcff52cef3cd9ae35d5f967375278b8cb03` + ``` + git log --all --find-object=1a496fcff52cef3cd9ae35d5f967375278b8cb03 -- docs/specs/491-optimize-undo-pair-recovery.md + ``` +- Вердикт конвейера: `green` · High 0