mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
committed by
Sergey Matyunin
Sergey Matyunin
parent
aac9232bbc
commit
fe0c2e0c83
@@ -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)
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `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
|
||||
```
|
||||
Reference in New Issue
Block a user