From fe0c2e0c83c928c3d22ae787a0f2a2f2f27eb309 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sat, 5 Sep 2026 05:21:17 +0000 Subject: [PATCH] docs: review document for #456 Issue: #456 User-Visible: no --- docs/reviews/SPEC-REVIEW-456-r1.md | 168 +++++++++++++++++++++++++++++ 1 file changed, 168 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-456-r1.md diff --git a/docs/reviews/SPEC-REVIEW-456-r1.md b/docs/reviews/SPEC-REVIEW-456-r1.md new file mode 100644 index 00000000..daea20b5 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-456-r1.md @@ -0,0 +1,168 @@ +# SPEC-REVIEW-456-r1 + +- **Issue:** https://github.com/Matysh/houseplan-card/issues/456 +- **ТЗ:** `docs/specs/456-copy-space.md` (611 строк) +- **Материал:** тело issue #456 + все комментарии на момент ревью, ТЗ на SHA + `197853c4d48bb0800df306f6e0d33eb18a23eb6e` (ветка `issue/456-copy-space`, + идентична текущему `dev`/`main` плюс один документ). +- **Этап:** spec (PROCESS.md §2.4), заход r1, блокирующих циклов израсходовано 0/4. +- **Трек:** полный (в S2-analysis явно назван критерий, который задача не + проходит: новый UX-контракт и составная запись с предварительной + оптимизацией — не `small`). + +## Скоуп проверки + +Issue #456 просит кнопку «Копировать» в диалоге настроек пространства: +копия стен (включая независимые перегородки), проёмов, колонн, декора, вида и +подложки под новым именем, без комнат и без привязок к устройствам, с +опциональной предварительной оптимизацией всего плана перед копированием. + +ТЗ полного трека обязано пройти §7.1: сценарий, что человек увидит, +проблема/подтверждённое состояние, скоуп/не-скоуп, контракт поведения (UX + +данные), модель данных и миграция, i18n, AC1…ACn с доказательством, план +автотестов, риски, откат, release-артефакты — все разделы присутствуют +(`docs/specs/456-copy-space.md` §1–20). + +## Как проверялось + +Ревью кода в этом заходе не требуется — диапазон `origin/dev...HEAD` содержит +только `docs/specs/456-copy-space.md` и одну строку в `docs/specs/README.md` +(проверено `git diff --stat origin/dev...HEAD`), то есть класс C. Гейты §8 +(`typecheck`/`test`/`build`) к этому коммиту неприменимы: продуктовый код не +менялся, изменённых поверхностей `src/**`/`custom_components/**/*.py` нет. + +Вместо гейтов — состязательная проверка каждого фактического утверждения ТЗ +против реального состояния репозитория (не «выглядит правдоподобно», а +конкретный grep/read): + +| Утверждение ТЗ | Проверено | Результат | +|---|---|---| +| `wall_segments` не могут существовать без 1–2 владельцев-комнат | `validation.py:1902-1903` | подтверждено: `raise vol.Invalid("wall segment must have one or two room owners")` | +| ID геометрии уникальны в пространстве не только внутри своего массива | `validation.py:1581-1604`, `_space_geometry_invariants` | подтверждено: один общий `seen`-set на `rooms/openings/decor/room_drafts/partitions/wall_columns/wall_segments` | +| `MAX_SPACES=50`, `MAX_PARTITIONS=2000`, `MAX_OPENINGS=500`, `MAX_DECOR=1000`, `MAX_WALL_COLUMNS=500` | `validation.py:1107-1120` | подтверждено дословно | +| `MAX_JUNCTION_VALENCE=6` (лимит стыков, «8 вместо 6») | `junction_limits.py:35` | подтверждено | +| `rooms: []` — валидный model-v9 space (нет `vol.Length(min=...)`) | `validation.py:1677` | подтверждено | +| Потолки ядра исчерпаны полностью: `houseplan-card.ts` 13659, `houseplan-editor-runtime.ts` 14323 | `test/core-file-budget.test.mjs:21-24`, фактический `wc -l` файлов (13658/14322, мера теста — `split('\n').length`, на 1 больше `wc -l`) | подтверждено: оба файла ровно на потолке, запас 0 | +| `validate_partition_opening_hosts`, `validate_junction_limits`, `validate_opening_passages`, `validate_wall_model_transition` существуют | `validation.py:175,630,737`, `junction_limits.py` | подтверждено | +| `_unique_title`, `plan_refs`, decor asset accounting | `import_export.py:906,1393`, `plans.py:249,299` | подтверждено | +| `_checkOptimizeGeometry`, `_reportPreflightFailure`, `houseplan/plan/optimize`, `PARTITION_OPENING_HOST_SCHEMA`, `gs.align_all` | grep по `src/**` | все символы существуют | +| Кнопка Delete сегодня — единственная в footer edit-режима, вне danger-группы места для Copy пока нет | `houseplan-editor-runtime.ts:14110-14120`, `houseplan-onboarding-runtime.ts:889-897` | подтверждено, оба места дословно совпадают с описанием ТЗ | +| Layout-ключ подписи комнаты `'rl_' + room.id` — плоский, без пространства (мотивация не копировать комнаты) | `houseplan-card.ts:12660,12688,12916` | подтверждено | +| `src/space-dialog.ts`, `src/coincident-partitions.ts`, `src/plan-optimizer.ts` существуют как заявлено | `ls` | подтверждено | +| «Сегодня повтор этажа делается только перерисовкой с нуля» — не забыт ли уже существующий экспорт/импорт «Текущее пространство» + «Только планировка» (#167)? | `docs/USER-GUIDE.ru.md:1789-1810`, `docs/specs/167-plan-only-export.md` | проверено отдельно — расхождения с реальностью не нашёл, см. ниже | + +### Проверка на дубль: #167 «Только планировка» + +Существующий экспорт `Current space` + `Plan only` действительно переносит +стены/проёмы/декор/подложку без устройств — на первый взгляд похоже на то, что +просит #456. Но этот механизм **сохраняет комнаты** (`docs/specs/167-plan-only-export.md` +§1–2: «комнаты, стены, проёмы, декор и фон остаются»), а модель v9 не разрешает +`wall_segment` без владеющей комнаты (см. таблицу выше) — то есть импортированную +копию нельзя превратить в «стены без комнат» существующими средствами: удаление +старых комнат потребовало бы предварительно решить ровно ту же задачу +преобразования в `partitions`, которую и решает #456. Сценарий #456 +(«комнаты в новом этаже другие») этим путём сегодня не закрывается. Дублирования +не нашёл; различие можно было бы явно назвать в §3 ТЗ, но отсутствие этого +предложения не создаёт риска неоднозначности AC — не поднимаю как находку. + +### Не проверялось (осознанно, вне гейтов spec-review) + +- Автотесты, mutation-тесты, browser smoke — кода ещё нет, впервые появятся в + `S6-in-progress`; спецификация лишь обязана назвать способ доказательства + (проверено — назван для всех AC1–AC13). +- `npx tsc --noEmit` / `npm test` / `npm run build` / `check-docs` / + `model-invariants` — не прогонялись: диапазон диффа не содержит `src/**` и + `custom_components/**/*.py`, только `docs/specs/**` (класс C), гейты §8 к + этому коммиту не применимы. +- Golden/performance/backend harness — неприменимо на этапе spec. + +## Находки + +Блокирующих (High) и находок Medium в скоупе или вне скоупа не обнаружено. + +**Low (снята с записью, не правится):** §3 ТЗ («подтверждённое текущее +состояние») не упоминает уже существующий экспорт «Текущее пространство → +Только планировка» (#167) и не объясняет явно, почему он не закрывает +сценарий. Разбор выше показывает, что реального дублирования нет (существующий +путь сохраняет комнаты, а модель не разрешает стены без владельца, то есть +не даёт «стены без комнат» без решения той же задачи преобразования). Автору +не нужно возвращать ТЗ ради одной поясняющей фразы — снимаю находку решением +ревьюера, содержательного риска для AC она не несёт. + +## Что проверено и корректно + +- Все AC1–AC13 однозначны, у каждого назван способ доказательства + (unit/integration/backend/browser smoke) и назван хотя бы один мутант, + который должен покраснеть — раздел «Чем краснеет» в теле issue и + «Доказательство» в ТЗ совпадают по существу. +- Технический анализ (`wall_segments` невозможны без комнат → `room_drafts` + ломает лимит стыков → `partitions` проходит все четыре валидатора) — + не декларация, а результат реального прогона схемы/валидаторов на + `demo/fixtures/large-house.mjs`; цифры (49 перегородок, 34 проёма, 8 vs + лимит 6) сверены с кодом и совпадают. +- Продуктовые вопросы Q1–Q4 (существующие перегородки источника, когда именно + нужен второй confirmation, куда переходит пользователь после успеха, + поведение при геометрическом долге после Optimize) заданы владельцу пачкой + с предлагаемым default и явно решены им — открытых продуктовых вопросов не + осталось (§7.1 требование выполнено). +- Раздел «Принятые предположения» (§21) отделяет технические/мелкие решения + от продуктовых и явно помечен как свободно оспоримый ревьюером — ни одна + догадка не выдана за факт без пометки. +- Не-скоуп (§5) корректно исключает перенос комнат/устройств/markers/vacuum + routes, перенос между установками HA (уже закрыт #167), изменение самой + логики Optimize и Copy в onboarding. +- Ограничение по потолку core-файлов (M2 аудита 04.09) учтено прямо и + корректно: потолки исчерпаны полностью (подтверждено измерением), и ТЗ + требует либо вынос эквивалентного объёма, либо отдельное решение владельца + — не игнорирует блокирующее ограничение. +- Модель данных: `rooms:[]`/`wall_segments:[]` как обязательные пустые + массивы — валидный model v9 space, миграции и новых config keys нет (§12); + совместимость со старой версией card/integration («пространство без комнат + с partitions») корректно вытекает из существующей схемы. +- i18n, performance, откат, release-артефакты — разделы присутствуют и + содержательны, а не формальные заглушки. + +## Чего не проверял + +- Реализуемость части «инъецируемая фабрика ID» и точную раскладку + `src/space-copy.ts` — это архитектурная заметка ТЗ (§13), не факт о текущем + коде; она специально помечена как решаемая свободно на код-ревью. +- Точные строки i18n-ключей — ТЗ называет категории строк, а не финальные + ключи; это осознанно оставлено на реализацию (см. §21 п.7, техническое + решение) и не блокирует AC13 (гейты `i18n`/`i18n-dead-keys` проверят факт + наличия во всех языках, а не конкретное имя ключа). +- Все побочные ветки backend-валидаторов (`validate_wall_model_transition` и + др.) построчно — прочитаны сигнатуры и место вызова, не построчный разбор + всей логики; для этапа spec этого достаточно, вопрос «работает ли» встанет + на код-ревью, когда появится реализация. + +## Вердикт + +Зелёный. ТЗ полное, все обязательные разделы §7.1 на месте, каждый AC +однозначен и снабжён способом доказательства, продуктовые вопросы закрыты +владельцем, технические утверждения проверены чтением кода и не разошлись с +реальностью. High: 0. Medium: 0. Low: 1, снята решением ревьюера с записью +выше. + +## Материал раунда + +- SHA материала: `197853c4d48bb0800df306f6e0d33eb18a23eb6e` +- Дерево: `git show 197853c4:docs/specs/456-copy-space.md` (611 строк) +- Диапазон: `origin/dev...197853c4` = `docs/specs/456-copy-space.md`, + `docs/specs/README.md` (2 файла, класс C) + +--- + + + +## Материал раунда + +- Ветка: `issue/456-copy-space`, коммит `197853c4d48b` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `ec1f135705c26ee5bcd3081516ebaa9b75ba5e1d` + ``` + git log --all --format='%H %T' | grep ec1f135705c2 + ``` +- ТЗ `docs/specs/456-copy-space.md`, блоб `a1def3eeefa15cac8b1bfe0542eb8d1ad0515fcf` + ``` + git log --all --find-object=a1def3eeefa15cac8b1bfe0542eb8d1ad0515fcf -- docs/specs/456-copy-space.md + ```