From 3d31d65735841590d563750efd722af295492589 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Wed, 26 Aug 2026 15:11:38 +0000 Subject: [PATCH] docs: review document for #319 Issue: #319 User-Visible: no --- docs/reviews/SPEC-REVIEW-319-r1.md | 119 +++++++++++++++++++++++++++++ 1 file changed, 119 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-319-r1.md diff --git a/docs/reviews/SPEC-REVIEW-319-r1.md b/docs/reviews/SPEC-REVIEW-319-r1.md new file mode 100644 index 00000000..13c1971d --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-319-r1.md @@ -0,0 +1,119 @@ +# SPEC-REVIEW-319-r1 + +Issue: #319 · Этап: spec (лёгкий трек `small`) · Заход r1 · блокирующих циклов 0/2 + +## Скоуп + +Правка одной ветки условия в `validate_wall_model_transition` +(`custom_components/houseplan/validation.py:169-203`): гард «unchanged wall +catalogue» (строки 193-203) должен применяться только при `new_model <= +old_model`, а не безусловно при `old_model >= 8 and new_model >= 8`. Цель — +пропускать первую запись клиента v9 поверх v8-документа с осиротевшим +`open_span`/`open_to`, которую миграция #306 делает нередактируемой навсегда. +ТЗ — в теле issue (small-трек), файла в `docs/specs/` нет, что соответствует +§5 PROCESS.md. + +## Как проверялось + +Ревью полное (заход r1, дельты нет). Материал: тело issue #319 + комментарий +`S2-analysis` владельца (решение зафиксировано 2026-08-26), текущий код +`custom_components/houseplan/validation.py`, `src/wall-segment-model.ts`, +`src/plan-optimizer.ts`, существующие тесты +`tests_backend/test_wall_segment_model.py`, канон `docs/SCOPE.md`, +`docs/WALL-THICKNESS.md`. + +1. Прочитан `validate_wall_model_transition` целиком (строки 169-203) — + условие и текст ошибки в коде дословно совпадают с тем, что описывает + issue («причина, доказана исполнением»). Ветка `old_model >= 8 and + new_model < 8` (легаси-клиент) действительно отдельная и правкой не + затрагивается. +2. Проверена посылка контракта «устаревший клиент не может поднять + `model_version`»: `WALL_SEGMENT_MODEL_VERSION = 9` + (`src/wall-segment-model.ts:21`) — константа кода, `commitWallSegmentModel` + всегда пишет `config.model_version = WALL_SEGMENT_MODEL_VERSION` + (`src/wall-segment-model.ts:724`), независимо от того, что было прочитано. + `plan-optimizer.ts` («Оптимизировать планы») использует ту же константу + (`PLAN_MODEL_VERSION = 9`, `src/plan-optimizer.ts:38`) и сам вызывает + `commitWallSegmentModelInPlace`. Значит клиент старой сборки (константа 8) + физически не может отправить `new_model=9` — посылка верна, это не догадка. +3. Проверено, не открывает ли ослабление гарда дыру в целостности данных: + независимая схемная проверка `_config_wall_segment_invariants` + (`validation.py:1766-1817`) при `model_version >= 8` жёстко сверяет + `room.poly` edge-by-edge с координатами `wall_segments` + (`_canonical_segment_key`, строка 1793-1795) и `space.walls` с + `wall_segments` (строка 1811-1817) — независимо от + `validate_wall_model_transition`. Реальное изменение геометрии контура без + соответствующего обновления каталога `wall_segments` отклоняется этой + схемной проверкой в любом случае, даже если гард транзита ослаблен. Гард + транзита ловит только специфический случай: `_catalog_coupled_wall_ + geometry_projection` включает `open_spans`/`open_to`, поэтому просто + *удаление* легаси-поля миграцией меняет «проекцию контура», не трогая ни + `poly`, ни `wall_ids`, ни `wall_segments`. Спуфинг `model_version` наверх + не даёт обойти геометрическую проверку — вывод: ослабление гарда не + создаёт находки Medium/High по целостности данных. +4. Сверены AC1-AC4 с существующей тестовой инфраструктурой + `tests_backend/test_wall_segment_model.py`: фикстура + `test/fixtures/282-wall-identity-parity.json` существует и уже используется + (строка 36-46); паттерн «stored/previous → submitted/candidate → + `validate_wall_model_transition`» уже используется в 4 существующих + тестах (строки 184-298), включая ровно тот сценарий AC2/#314 + (`test_current_wall_model_independent_geometry_does_not_require_contour_ + catalog_change`, строка 226). AC1/AC2/AC3/AC4 доказуемы предложенным + способом без дополнительных догадок об инфраструктуре. +5. Проверено, что `test_stale_client_echoing_v8_catalog_gets_the_named_error` + (строка 210-223, `old_model=9, new_model=9`) и + `test_stale_client_round_trip_is_hydrated_but_structural_change_is_rejected` + (строка 184-207, `new_model<8` ветка) остаются зелёными под новым условием + `new_model <= old_model` — оба явно соответствуют AC3/AC4. +6. Проверен канон `docs/WALL-THICKNESS.md` — «canonical model v9 never writes + compatibility `open_spans` or `open_to`» (строка 76), противоречий с ТЗ + нет. +7. `docs/SCOPE.md`: это баг-фикс, восстанавливающий J6 («Keep the plan true + as the home evolves» — drag/resize, merge/split), а не новая функция; + решение владельца по направлению фикса уже зафиксировано в + `S2-analysis`-комментарии 2026-08-26. Открытых продуктовых вопросов нет. + +## Находки + +Нет. Ни одной High/Medium/Low. + +## Что проверено и корректно + +- Соответствие §5 (лёгкий трек): сложность/риск, одна поверхность + (`validation.py`), нет миграции конфига/новых полей/UX-контракта, нет + влияния на touch/perf — подтверждено чтением кода, а не только заявлением + автора оценки. +- Структура ТЗ в теле issue соответствует упрощённому шаблону §5: проблема · + контракт · AC1…AC4 с доказательством · откат — все разделы на месте. +- Контракт («гард применяется только при `new_model <= old_model`») — + однозначная булева формулировка, прямо переносимая в код без дополнительной + интерпретации; не риторика, а точное условие. +- Посылка «устаревший клиент не может поднять `model_version`» проверена по + фронтенд-коду, а не принята на веру. +- Отсутствие дыры в целостности данных при ослаблении гарда проверено по + независимой схемной проверке `_config_wall_segment_invariants`. +- Каждый AC указывает способ доказательства (регрессионный тест, зелёные + существующие тесты) и воспроизводим по существующей тестовой + инфраструктуре проекта. +- Откат: ревертом одного коммита, правка не трогает данные (только + валидацию записи) — корректно и достаточно для этого класса изменений. +- Ветка `old_model >= 8 and new_model < 8` (легаси-клиент, AC4) явно не + меняется — подтверждено чтением кода. + +## Чего не проверял + +- Не запускал `pytest tests_backend` — на этапе spec кода фикса ещё нет, + запускать нечего; это часть код-ревью следующего этапа. +- Не проверял вручную UI/тост «Обновите карточку» в браузере — это + поведенческое доказательство, а не спецификационное; относится к + код-ревью/смокам. +- Не выяснял в глубину, обрабатывает ли `plan-optimizer.ts` путь отдельно от + обычной структурной записи в бэкенде — не требуется: бэкенд-валидация одна + на все пути записи, и AC1 формулирует «любая структурная запись», что + корректно покрывает и «Оптимизировать планы» без отдельного AC. + +## Вердикт + +Зелёный. ТЗ технически обосновано, контракт однозначен и реализуем буквально, +все AC проверяемы предложенным способом, открытых продуктовых вопросов нет, +скоуп совпадает с job J6 из `docs/SCOPE.md`. Готово к разработке.