mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-01 04:09:17 +00:00
claude[bot]
@@ -0,0 +1,88 @@
|
||||
# SPEC-REVIEW-51-r1 — Пользовательские изображения в декоративном слое
|
||||
|
||||
- Issue: https://github.com/Matysh/houseplan-card/issues/51
|
||||
- Этап: ТЗ на ревью (PROCESS.md §2.4), заход **r1**, блокирующих циклов израсходовано 0 из 4 (лимит на полном треке — 4)
|
||||
- ТЗ: `docs/specs/051-custom-decor-images.md`
|
||||
- Ветка: `issue/51-custom-decor-images`
|
||||
- Материал ревью: SHA `ee658c5d6d9c00bad475e2ed59826b1c01415143` (= `HEAD`, проверено `git rev-parse HEAD`; совпадает с SHA, названным автором в хендофф-комментарии)
|
||||
- Вердикт: **зелёный**
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Первый заход по этой задаче: предыдущих документов `SPEC-REVIEW-51-*` в истории репозитория нет (`git log --all -- docs/reviews/SPEC-REVIEW-51*` пуст), поэтому раздел «Унаследовано из r0» не применяется и разбор — полный, как того требует §2.9 при первом заходе.
|
||||
|
||||
Задача идёт полным треком (в issue явно названы критерии `small`, которые не проходит: несколько поверхностей, новый UX-контракт, новые compatibility-поля/persisted-схема) — ТЗ корректно лежит в `docs/specs/051-custom-decor-images.md`, а не в теле issue.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Прочитан `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` полностью.
|
||||
2. Прочитано тело issue #51 и все 6 комментариев: закрытый дубль #46 (требование к file lifecycle), три раунда аналитики (2026-08-15, 2026-08-30, 2026-09-02 с 6 продуктовыми вопросами Q1–Q6), решения владельца по всем Q1–Q6 (2026-09-02), финальный хендофф автора.
|
||||
3. Прочитан весь текст `docs/specs/051-custom-decor-images.md` (629 строк) от начала до конца.
|
||||
4. Сверены с реальным кодом на этом дереве ключевые фактические утверждения ТЗ, а не приняты на веру:
|
||||
- `src/editors/decor/types.ts` — `DecorKind` действительно заканчивается на `'furniture'`, `DecorImageTransform` — действительно неиспользуемая заготовка `{x,y,w,h,angle?,opacity?}`, как заявляет «Проблема»;
|
||||
- `docs/DECOR-EDITOR.md` — секция «Deferred custom images» подтверждает оба ограничения, которые ТЗ соблюдает: не переиспользовать backdrop file lifecycle implicitly, и контракт transform уже совпадает с мебелью;
|
||||
- таблица «Tools» / «Magnet targets» в `DECOR-EDITOR.md` подтверждает, что wall magnet — специфика мебели («Furniture: wall magnet unless Shift»), а общий decor/room magnet — отдельный, более широкий механизм; ТЗ (§4 контракта) корректно различает эти два понятия и просит убрать только первое;
|
||||
- `src/backdrop-probe.ts` подтверждает точные числа, которые ТЗ называет как переиспользуемые: `WARN_DECODED_BYTES = 128 * 1024 * 1024`, `DOWNSCALE_TARGET_PX = 4096`, `HARD_DIMENSION = 16384`;
|
||||
- `custom_components/houseplan/websocket_api.py:1057-1082` подтверждает `MAX_SIGN_PATHS = 200` и существующий паттерн `content/sign` — ТЗ переиспользует то же число для лимита `resolve`, не выдумывает новое;
|
||||
- `custom_components/houseplan/http_api.py:125-182` (`HouseplanContentView`) подтверждает существующий `sandbox`-CSP для `.svg` и паттерн `{kind}/{sub}/{name}` с плейсхолдером `_` для плоских директорий (как у `plans`) — ТЗ предлагает `assets/_/<name>` тем же паттерном, не новым;
|
||||
- `custom_components/houseplan/plans.py:235-239` подтверждает существующий `low_disk_space`-guard, который ТЗ обещает переиспользовать, а не изобретать;
|
||||
- `custom_components/houseplan/import_export.py` подтверждает реальные имена полей `content_manifest`, `exists_at_export`, `export_version`, и что `EXPORT_VERSION = 1` в `const.py` сегодня; апгрейд до v2 с обратной читаемостью v1 — реальная работа, не фикция, и прямо помечен в ТЗ как «принято предположительно, поменять свободно»;
|
||||
- `custom_components/houseplan/validation.py:1278` подтверждает существование поля `hide_decor`, которым ТЗ оперирует;
|
||||
- `support_package.py`/`import_export.py` подтверждают, что SHA-256 content-addressing — уже используемый в проекте приём, а не новый паттерн без прецедента;
|
||||
- `demo/benchmark_large_house.mjs`, `demo/performance/budgets-large-house-*.json` подтверждают, что «large-house fixture» и связанный performance-гейт — существующая практика, на которую ТЗ (AC14) корректно опирается;
|
||||
- `custom_components/houseplan/validation.py:1066` подтверждает `MAX_DECOR = 1000` на пространство — фикстура AC14 на 1000 image records бьёт в реальный, а не придуманный потолок.
|
||||
5. Сверены все 6 продуктовых вопросов Q1–Q6 и defaults из аналитики 2026-09-02 построчно с соответствующими местами контракта (§1 «Единая кнопка», §4 «Первичное размещение», §4/§5 «wall magnet», §8 «Явное удаление», «Import/export», §3 «Безопасный SVG») — все решения владельца перенесены в контракт без искажений и без дополнительных недосказанных допущений.
|
||||
6. Проверены обязательные разделы §7.1: сценарий, что человек увидит, проблема, скоуп/не-скоуп, контракт поведения, модель данных, i18n, AC1–AC16 с доказательством, план автотестов, риски, откат, release-артефакты — все присутствуют.
|
||||
7. Проверено соответствие `docs/TOUCH-SUPPORT.md` — см. находку L1 ниже.
|
||||
|
||||
## Находки
|
||||
|
||||
### L1 (Low) — отсутствует обязательная литеральная строка `Touch editor: …`
|
||||
|
||||
`docs/TOUCH-SUPPORT.md` («Documentation rule») требует, чтобы каждая спецификация новой функциональности редактора явно содержала одну из трёх фраз: `Touch editor: supported`; `Touch editor: best effort / intentionally degraded`; `Touch editor: not exposed`. Это не стилистическое пожелание, а соблюдаемая по факту конвенция — она обнаружена буквально в ~15 других файлах `docs/specs/*.md` (132, 137, 138, 150, 159, 164, 172, 173, 174, 178, 179, 180, 186, 193, 199, 210 и др.).
|
||||
|
||||
Раздел «UX, accessibility, touch и kiosk» ТЗ #51 содержит по смыслу правильное утверждение — «Background editing on touch остаётся best effort по `TOUCH-SUPPORT.md`, но safety floor обязателен: …» — но не использует требуемую литеральную форму `**Touch editor: best effort / intentionally degraded.**`, из-за чего DoR-чеклист §2.5 («влияние на touch по `docs/TOUCH-SUPPORT.md`») и автоматизированный грep по будущим ревью не находят её механически.
|
||||
|
||||
**Воспроизведение:** `grep -n "Touch editor:" docs/specs/051-custom-decor-images.md` — 0 совпадений, при том что `docs/TOUCH-SUPPORT.md` требует ровно такую строку.
|
||||
|
||||
**Серьёзность:** Low — по существу вопрос решён верно (best effort, safety floor описан), это дефект оформления, а не решения. Правится добавлением одной строки, не влияет ни на один AC.
|
||||
|
||||
**Решение ревьюера:** не блокирует зелёный вердикт. Правится либо в ходе реализации (замена формулировки на литеральную), либо мелкой правкой ТЗ вне цикла — записываю как принятое с пометкой «поправить при следующем прикосновении к файлу», отдельного возврата на цикл ревью не требует по §3 правило 8 (Low либо правится, либо снимается решением ревьюера с записью).
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- **Продуктовая рамка (SCOPE.md).** Фича закрывает J4/J6 («from zero to a working plan… no external editors» / «keep the plan true as the home evolves»); View остаётся пассивным (никаких HA-состояний, действий, hover/click) — соответствует «View mode is the product» и правилу lock/actuation (изображение никогда не actuates).
|
||||
- **Правило «никогда не удалять файл по предположению»** (SCOPE.md, зафиксировано 2026-07-28) соблюдено буквально: §8 контракта запрещает автоматическое удаление при замене/удалении объекта/пространства/импорте, разрешает только явное действие с подтверждением и disabled-состоянием при наличии ссылок.
|
||||
- **Требование из закрытого #46** («до реализации нужно закрыть file lifecycle: удаление пространства, копирование декора, экспорт») закрыто: refcount вычисляется динамически из authoritative config (удаление пространства снижает его само по себе), copy/paste сохраняет `asset_id`, экспорт покрыт `content_manifest`/`exists_at_export`/repair-placeholder веткой.
|
||||
- **Все 6 owner-decisions (Q1–Q6, комментарий 2026-09-02) перенесены в контракт без отклонений** — кнопка+palette (§1), 100 см / cap 200 см (§4, AC2), transform мебели без wall magnet (§4–§5, AC3), запрет auto-delete + явное удаление только для asset с нулём ссылок (§8, AC9), JSON без bytes + repair-placeholder (Import/export, AC11), полный отказ небезопасного SVG без молчаливой вырезки (§3, AC7).
|
||||
- **AC1–AC16** пронумерованы, у каждого явно назван способ доказательства (unit/backend/smoke/golden/performance/review), что удовлетворяет §2.5 DoR-чеклист.
|
||||
- **Раздел «Принято предположительно, поменять свободно»** корректно отделяет технические решения (content-addressed id, quota, export v2, точные имена модулей) от продуктовых — соответствует §7.1: «всё, чего пользователь не наблюдает, агенты решают сами».
|
||||
- **Not-scope** явно исключает WYSIWYG-редактирование, drag-and-drop, авто-удаление orphan-файлов, вложение bytes в архив, wall magnet — устраняет пространство для скрытого расползания скоупа при реализации.
|
||||
- **Backend security-контракт (§2–§3)** — signature/decode-confirmed проверка форматов, строгий XML parser без DTD/entities, allowlist элементов, canonical reserialize перед хешированием, `nosniff`+sandbox CSP, `<image>`-only рендер SVG (никогда inline) — согласуется с существующим прецедентом sandbox-CSP в `http_api.py` и не ослабляет его.
|
||||
- **Rollback/откат** описан по обеим осям (frontend/backend) и явно запрещает lossy-миграцию или автоматическую очистку при откате — соответствует стандартному разделу «Откат» DoR.
|
||||
- **Одно число — один источник.** Единственная пользователю видимая величина, которую вводит эта фича при размещении, — физический размер нового изображения (100 см / aspect-preserving height, cap 200 см). ТЗ явно требует (§4, AC2): «preview и committed bounds совпадают при разных `cell_cm`, zoom и intrinsic ratios» и что значения «переводятся в normalized geometry через текущие `cell_cm`/grid helpers» — то есть единственный источник преобразования один и тот же для preview и записи, регрессии класса #234/#233 контракт исключает на уровне формулировки.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- **Код не читался и не запускался** — на этом этапе (ревью ТЗ) кода ещё нет: `git diff` по `src/**`/`custom_components/**` для этого issue пуст, это ожидаемо для `S4-spec-review`. Гейты `typecheck`/`test`/`build`/смоки/golden/performance/backend не прогонялись и не применимы: класс изменения — только `docs/specs/051-custom-decor-images.md` (класс C), кодовых гейтов задача не требует.
|
||||
- **`git diff --check` и «проверка обязательных разделов §7.1»**, которые упоминает хендофф автора, я не перезапускал — визуальная проверка всех разделов §7.1 сделана вручную построчно (см. «Как проверялось», п.6) и даёт тот же результат.
|
||||
- Не проверялась реализуемость точных backend-имён (`houseplan/assets/upload` и т.д.) на предмет коллизий с уже существующими websocket command id — это заявлено автором как техническая деталь реализации («Точные transport names являются частью реализации и покрываются contract tests»), законно оставлено на усмотрение реализации согласно §7.1.
|
||||
- Не проверялись содержательно 4 канонических документа, которые ТЗ обещает обновить (`DECOR-EDITOR.md`, `ARCHITECTURE.md`, `CONFIG-COMPATIBILITY.md`, `USER-GUIDE(.ru).md`) — они меняются в реализации, не в ТЗ, и будут предметом код-ревью.
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/51-custom-decor-images`
|
||||
- SHA: `ee658c5d6d9c00bad475e2ed59826b1c01415143` (= `HEAD` на момент ревью, дерево чистое)
|
||||
- Файл ТЗ: `docs/specs/051-custom-decor-images.md`
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/51-custom-decor-images`, коммит `ee658c5d6d9c` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `42b442d3b9dd4bb4de9e8061e6b5120f13c7d68b`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 42b442d3b9dd
|
||||
```
|
||||
Reference in New Issue
Block a user