houseplan-card/docs/reviews/SPEC-REVIEW-491-r1.md

SPEC-REVIEW-491-r1

Issue: #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