mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -0,0 +1,198 @@
|
||||
# SPEC-REVIEW-584-r1
|
||||
|
||||
**Issue:** #584 · **Этап:** ТЗ на ревью (S4-spec-review) · **Заход:** r1 ·
|
||||
**Трек:** полный (аналитика от 2026-09-15 назвала критерий: сложность/риск >3,
|
||||
несколько модулей и поверхностей, меняется публичный контракт «см. в
|
||||
свойствах = видимый габарит»).
|
||||
|
||||
**Вердикт: жёлтый**
|
||||
|
||||
---
|
||||
|
||||
## Скоуп проверки
|
||||
|
||||
ТЗ живёт в теле issue #584 под заголовком `## ТЗ` (комментарий владельца от
|
||||
2026-09-16T19:33:08Z объявляет переход в `S4-spec-review`). Материал ревью —
|
||||
текст issue на момент чтения (последний комментарий владельца, тот же, что
|
||||
поставил статус); отдельного файла в `docs/specs/` нет и не создаётся (архив
|
||||
закрыт с #517).
|
||||
|
||||
Прочитано перед разбором: `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` целиком
|
||||
(включая §2.4, §2.5, §2.7, §7.1, §12), тело issue #584 и все 6 комментариев
|
||||
(аналитика → вопросы Q1/Q2 → ответ владельца → полное ТЗ дизайнеру →
|
||||
присланный архив → приёмка пака дизайнером-исполнением). `docs/TOUCH-SUPPORT.md`
|
||||
и канонические документы подсистем (SUN/LIGHT/CANVAS/WALL-THICKNESS/UX-MODES/
|
||||
CONFIG-COMPATIBILITY) просмотрены на предмет пересечения — задача их не
|
||||
затрагивает (чистая геометрия SVG-артворка мебели, не стены/не свет/не солнце).
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Поскольку задача целиком про фактическую геометрию, а не про рассказ, каждое
|
||||
фактическое утверждение ТЗ сверено с текущим состоянием репозитория (SHA
|
||||
`7c5fd32a`, `dev`), а не принято на слово:
|
||||
|
||||
| Утверждение ТЗ | Проверка | Результат |
|
||||
|---|---|---|
|
||||
| 44 плановых SVG в `svg/plan`, 33 иконки в `svg/menu` | `ls assets/furniture/houseplan-0.3.0/svg/plan \| wc -l`, `svg/menu` | 44 и 33 — совпадает |
|
||||
| `furnitureRenderTransform` существует | `grep -r furnitureRenderTransform src/` | найден в `houseplan-card.ts`, `houseplan-editor-runtime.ts`, `furniture.ts` |
|
||||
| `parseSvgPath` — производственный парсер экспорта | `grep -r parseSvgPath src/` | единственное определение в `src/pdf/svg-path.ts` |
|
||||
| `kitchen_floor` — дизайнерский символ 60×60, `svg/plan/kitchen_floor.svg` | `pack.json` строки 580-600 | `width_cm: 60, depth_cm: 60, file: svg/plan/kitchen_floor.svg` — совпадает |
|
||||
| `dishwasher` — legacy-примитив 60×60, заполняющий unit box | `pack.json` (единственная запись — `svg/menu/dishwasher.svg`, без `width_cm`) + `src/furniture.ts:155` (`w: 60, h: 60, g: [box(), …]`) | подтверждено: `dishwasher` не входит в дизайнерский пак, это legacy-примитив из `RETAINED_IDS`, `box()` рисует полный `0..1×0..1` |
|
||||
| `plant` — единственный legacy-символ с полями 2% | `src/furniture.ts:226-231` (`e 0.5,0.5,0.22,0.22` + лучи-линии до `0.02`/`0.98`) | внешний bbox линий действительно `0.02…0.98` — 2% с каждой стороны, совпадает |
|
||||
| Генератор склеивает `path` простой конкатенацией `paths.join(' ')` | `scripts/generate-furniture-assets.mjs:97` | `return { d: paths.join(' '), … }` — подтверждён ровно описанный дефект |
|
||||
| `furniture-plan-art.generated.ts`, `furniture-plan-catalog.generated.ts` существуют | `ls src/*.generated.ts` | оба файла на месте |
|
||||
| `furniture:check` — существующий гейт | `package.json:47` | `"furniture:check": "node scripts/generate-furniture-assets.mjs --check"` |
|
||||
| `test/furniture-*.test.mjs` — свидетели AC1–AC5 | прочитаны `furniture-stroke-contract.test.mjs`, `furniture-transform-contract.test.mjs` | оба существуют, но проверяют другие контракты (#361/#376/#383 — экранный stroke-resolver, resize/rotate editor-контракт), а не bounds SVG-геометрии; конфликта с новыми AC нет, но новые unit-тесты AC1–AC4, скорее всего, лягут в отдельный новый файл — это техническая деталь, не продуктовая, решать разработчику |
|
||||
|
||||
Все фактические утверждения ТЗ подтвердились чтением кода — ни одной догадки,
|
||||
выданной за факт, не найдено. Отдельно ценно, что владелец не просто принял
|
||||
присланный дизайнером пак на слово, а перепроверил его **тем же
|
||||
production-парсером**, которым будет проверяться AC1/AC2 (комментарий от
|
||||
2026-09-16T19:33:08Z: 44 из 44, худшее отклонение `0.000000`) — то есть контракт
|
||||
уже был воспроизведён на реальных файлах ещё до передачи в разработку.
|
||||
|
||||
## §7.1 — обязательные разделы
|
||||
|
||||
| Раздел §7.1 | Есть в ТЗ | Комментарий |
|
||||
|---|---|---|
|
||||
| Сценарий | ✅ «Пользовательский сценарий» | персона не названа явно (home admin), но контекст (свойства мебели, план) её однозначно определяет |
|
||||
| Что человек увидит до/после | ✅ | числа названы (14% по стороне, 30% по площади), формулировка «это исправление, а не регрессия» — по делу |
|
||||
| Проблема | ✅ | в теле issue выше `## ТЗ` («Что происходит») |
|
||||
| Скоуп и не-скоуп | ⚠️ распределено | явного заголовка нет; не-скоуп фактически задан AC7 («редактор, hit-area, привязка к стенам, схема конфига и i18n не меняются») — содержание есть, форма нет. Low, не блокирует |
|
||||
| Контракт поведения | ✅ «Контракт», пп. 1–8 | однозначен, проверен по коду (см. таблицу выше) |
|
||||
| UX | ⚠️ отсутствует явно | по существу — «нет нового UX-контракта» (задача не трогает интеракции), это следует из AC7, но отдельного раздела нет. Low, не блокирует |
|
||||
| Модель данных и миграция | ✅ «Совместимость и миграция» | миграции нет, обосновано |
|
||||
| i18n | ✅ | явное «новых ключей нет» |
|
||||
| AC1…ACn с доказательством | ✅ AC1–AC8 | у каждого назван способ (unit/golden/review/гейты) |
|
||||
| План автотестов | ⚠️ распределено | не оформлен отдельным разделом, но каждый AC называет тип теста и файлы перечислены в «Затронутые файлы» — по существу план есть |
|
||||
| Риски | ✅ «Риски» | 3 пункта, по делу (маскировка регрессии golden-пересъёмкой, склейка путей, рост ленивого чанка) |
|
||||
| Откат | ✅ | «возврат коммита» — корректно для непереносимой миграции |
|
||||
| Release-артефакты | ⚠️ частично | changelog RU+EN и golden названы (в «Затронутые файлы»), перф — отдельным разделом; **security явно не упомянут** (ни разу, даже «нет») |
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium (в скоупе) — влияние на touch не названо
|
||||
|
||||
**Файл:** тело issue #584, раздел `## ТЗ`.
|
||||
|
||||
DoR §2.5 явно требует пункт «влияние на touch по `docs/TOUCH-SUPPORT.md` (View
|
||||
и киоск — блокирующие)» как обязательный, отдельно от i18n и производительности,
|
||||
которые в ТЗ присутствуют. В тексте ТЗ слова «touch», «тач», «киоск» не
|
||||
встречаются вообще — раздел не просто краток, он отсутствует.
|
||||
|
||||
**Почему это не тривиальная придирка:** задача меняет физический видимый
|
||||
размер мебели **в View/kiosk тоже**, не только в редакторе (мебель рендерится
|
||||
и там, и там одним и тем же `furnitureRenderTransform`). AC7 отдельно
|
||||
фиксирует «hit-area… не меняются», что фактически закрывает главный
|
||||
touch-риск (позиция resize/rotate-хэндлов на touch не сдвигается), но это
|
||||
написано как граница скоупа редактора, а не как ответ на явно
|
||||
предусмотренный процессом пункт. Реальный риск, скорее всего, нулевой — но
|
||||
DoR требует **явного** «нет», а не выводимого из соседнего пункта.
|
||||
|
||||
**Как закрыть:** одно предложение в ТЗ, например: «Touch: изменение
|
||||
непосредственно не затрагивает — View/kiosk остаются readonly-поверхностью,
|
||||
позиции hit-area и resize/rotate-хэндлов не пересчитываются (AC7); мебель в
|
||||
редакторе визуально крупнее/меньше на ту же величину, что и в View, новых
|
||||
touch-жестов не добавляется.» Правки кода не требует.
|
||||
|
||||
**Серьёзность:** Medium, в скоупе — блокирует переход в DoR (§2.5 явно требует
|
||||
пункт), но правится добавлением одного абзаца в тот же issue, без нового цикла
|
||||
имплементации. Без High это жёлтый вердикт.
|
||||
|
||||
### Low (снимается с записью, не блокирует)
|
||||
|
||||
1. **Раздел «UX» и «скоуп/не-скоуп» не оформлены отдельными заголовками.**
|
||||
Содержание по факту присутствует: AC7 однозначно перечисляет, что НЕ
|
||||
меняется (редактор, hit-area, привязка к стенам, схема конфига, i18n), а
|
||||
отсутствие нового UX-контракта следует из того же AC7 и из того, что задача
|
||||
правит только артворк и генератор. Формального заголовка нет, но
|
||||
переспрашивать автора ради двух заголовков дороже, чем цена неясности —
|
||||
снимаю с записью.
|
||||
2. **Security как release-артефакт не назван даже «нет».** Задача не
|
||||
затрагивает ни одну поверхность из OWASP-класса рисков (правятся SVG-пути
|
||||
мебели и генератор ассетов, без сетевых вызовов, парсинга пользовательского
|
||||
ввода на новом пути или новых прав) — риска не вижу, но пункт DoR формально
|
||||
пуст. Снимаю с записью, а не как Medium: в отличие от touch, здесь нет
|
||||
спорной поверхности (View/kiosk), которую процесс называет блокирующей
|
||||
явно.
|
||||
3. **AC3 «допуск, пропорциональный масштабу» не даёт формулу.** Это разумно
|
||||
оставить на усмотрение разработчика (технический, не продуктовый вопрос —
|
||||
§7.1: «то, чего пользователь не наблюдает, агенты решают сами») при
|
||||
условии, что реализация зафиксирует конкретную формулу и она останется
|
||||
проверяемой (не «плавает» между прогонами). Ревьюер кода должен убедиться,
|
||||
что тест «умеет падать» на конкретной, а не абстрактной величине допуска —
|
||||
отмечаю на будущее, не как находку ТЗ.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- **AC1–AC8 однозначны и снабжены способом доказательства** (unit / unit-generator
|
||||
/ golden / review / гейты) — ни один не пришлось домысливать.
|
||||
- **AC2 конкретен**: именно пара `kitchen_floor`/`dishwasher`, оба подтверждены
|
||||
в `pack.json`/`furniture.ts` как реальные `60×60` записи — тест будет
|
||||
проверять то, что реально описано в баг-репорте, а не абстракцию.
|
||||
- **AC4 и контракт п.6** точно описывают дефект склейки путей — сверено с
|
||||
реальным кодом генератора (`paths.join(' ')`), включая упомянутые
|
||||
`coffee_table_round`/`table_round`.
|
||||
- **Ни одной догадки, выданной за факт**: все количественные утверждения
|
||||
(44 символа, доля полей 2,7–3,9 у дизайнерских, 2% у `plant`, конкретные id
|
||||
и размеры) подтверждаются либо кодом, либо независимым измерением владельца
|
||||
тем же production-парсером, которым будет доказываться AC1.
|
||||
- **Продуктовые вопросы закрыты владельцем предметно** (Q1 — видимый габарит
|
||||
= осевая линия контура, обводка не считается; Q2 — без миграции,
|
||||
центрируется в прежнем физическом боксе), открытых продуктовых вопросов в
|
||||
тексте не осталось; технические решения (склейка путей, `plant`) вынесены в
|
||||
явный блок «Принятые предположения», как требует §7.1.
|
||||
- **Границы скоупа (AC7)** явно исключают редактор/hit-area/привязку к
|
||||
стенам/схему конфига/i18n — правка не расползается на соседние подсистемы.
|
||||
- **Откат и совместимость** описаны корректно для правки без миграции: `x/y`,
|
||||
`w/h`, угол, зеркалирование сохраняются, откат — реверт коммита.
|
||||
- **Риски названы предметно**, включая специфичный для этой задачи риск —
|
||||
маскировку чужой регрессии golden-пересъёмкой.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не проверял сам присланный дизайнером ZIP-архив (`fix-584-1`) —
|
||||
на этапе ревью ТЗ кода/ассетов ещё нет, они появятся в ветке реализации;
|
||||
их проверка (в том числе повторное измерение AC1/AC2 после факта) —
|
||||
предмет код-ревью.
|
||||
- Не запускал `furniture:check`, `npm test`, `golden:verify` и т.п. — на
|
||||
этом этапе нет ни одной строки кода/ассета, гейты нечего гонять; это
|
||||
предмет §8 на код-ревью, не ревью ТЗ.
|
||||
- Не проверял `docs/CONFIG-COMPATIBILITY.md` построчно на предмет
|
||||
дополнительных полей совместимости сверх того, что решает п.8 контракта —
|
||||
задача явно не трогает схему конфига (AC7), риск счёл пренебрежимым для
|
||||
ревью ТЗ.
|
||||
|
||||
## Итог
|
||||
|
||||
ТЗ содержательно сильное: контракт точен, AC проверяемы и уже частично
|
||||
эмпирически подтверждены владельцем на реальном присланном паке тем же
|
||||
инструментом, которым будет доказываться AC1. Единственная находка,
|
||||
требующая правки текста, — отсутствующий явный пункт про touch/View/kiosk,
|
||||
обязательный по DoR §2.5. High-находок нет, поэтому вердикт жёлтый:
|
||||
одно предложение в ТЗ, без нового раунда имплементации.
|
||||
|
||||
---
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- **Issue:** https://github.com/Matysh/houseplan-card/issues/584
|
||||
- **Материал ТЗ:** тело issue #584, раздел `## ТЗ`, редакция комментария от
|
||||
2026-09-16T19:33:08Z (тот же комментарий ставит `S4-spec-review`).
|
||||
- **Репозиторий на момент ревью:** `dev` @ `7c5fd32a08f07177d55905d65990180a354dd0ec`
|
||||
(кода по задаче ещё нет — ревью ТЗ смотрит только текст issue и сверяет его
|
||||
утверждения с текущим `dev`).
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `dev`, коммит `7c5fd32a08f0` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `2b57427cbed60f2f3b46aaf1b2c9a84147a8ed88`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 2b57427cbed6
|
||||
```
|
||||
- Тело issue: `a4ec41f212d36f3ec2d313b534161c25a38e58c118ed2f7b97ab437de1593eb3`
|
||||
- Вердикт конвейера: `yellow` · High 0
|
||||
Reference in New Issue
Block a user