mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -0,0 +1,221 @@
|
||||
# SPEC-REVIEW-606-r1 — «Показать тренажёр и исправить перепутанные bookshelf/shelf_floor»
|
||||
|
||||
Issue: [#606](https://github.com/Matysh/houseplan-card/issues/606)
|
||||
Этап: spec (полный трек — автор в S2-analysis назвал нарушенный критерий §5:
|
||||
«одна поверхность» не выполняется, затронуты `assets/furniture/**`, генератор,
|
||||
`src/furniture-*`, каталог палитры, i18n четырёх языков, PDF/static-путь, golden
|
||||
и совместимость публичного ID; `small`/`trivial` неприменимы)
|
||||
Заход: r1 (первый; разделы «Унаследовано из r0» и «Закрытие раунда r0» не нужны — §2.10
|
||||
применяется со второго захода)
|
||||
|
||||
## Вердикт
|
||||
|
||||
**Зелёный.** High: 0. Medium в скоупе: 0. Medium вне скоупа: 0. Low: 2 (обе
|
||||
сняты решением ревьюера с записью ниже, доработка не требуется).
|
||||
|
||||
## Скоуп разбора
|
||||
|
||||
Полный разбор: тело issue #606 целиком — исходное описание проблемы/объёма
|
||||
работ/критериев приёмки (написано до раздела `## ТЗ`, ТЗ явно ссылается на него
|
||||
как на действующий текст) и раздел `## ТЗ`; оба комментария issue (аналитика S2
|
||||
и хендофф автора ТЗ); `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` (§1, §2.2–§2.5,
|
||||
§2.10, §4, §5, §7.1, §7.2); родительская задача [#593](https://github.com/Matysh/houseplan-card/issues/593)
|
||||
(поставка `houseplan-0.4.0`, откуда унаследован текущий баг) — прочитана как
|
||||
источник происхождения пакета, а не как материал этого ревью.
|
||||
|
||||
Поскольку почти все содержательные утверждения ТЗ — проверяемые факты о текущем
|
||||
состоянии репозитория, а не только предположения о будущем, разбор включает код
|
||||
и данные на `dev` (`0d30dde1`, ветка `issue/606-furniture-catalog-corrections`
|
||||
опубликована от того же коммита, продуктовый код не менялся — чистая проверка ТЗ):
|
||||
|
||||
- `assets/furniture/houseplan-0.4.0/pack.json` — фактические записи `cactus`
|
||||
(`menu_icon: plant`, `70×120`, `back: top`, `operation: add`), `bookshelf`
|
||||
(`file: svg/plan/bookshelf.svg`, `menu_icon: wardrobe`, `100×35`), `shelf_floor`
|
||||
(`file: svg/plan/shelf_floor.svg`, `menu_icon: shelving`, `100×35`), `exercise`
|
||||
в `menu_icons` (уже существует, `group: other`, `file: svg/menu/exercise.svg`) —
|
||||
33 `menu_icons`, 60 `symbols`;
|
||||
- содержимое `svg/plan/bookshelf.svg`, `svg/plan/shelf_floor.svg`,
|
||||
`svg/plan/cactus.svg` — форма путей подтверждает описанную в issue путаницу
|
||||
(первый рисует перекрёстный «ящик»-стеллаж, второй — раскрытую книгу);
|
||||
- `scripts/generate-furniture-assets.mjs` — правило `path.basename(entry.file,
|
||||
'.svg') !== entryId → fail`, проверка `boxFillDeviation` против `viewBox`,
|
||||
проверка `menu_icon` → группа менюшки/символа, жёсткая проверка версии
|
||||
манифеста (`pack_version !== '0.4.0' → fail`) и происхождения (`author`,
|
||||
`license`);
|
||||
- `src/furniture.ts` — `BY_ID`/`furnitureSymbol`/`furnitureArtIsLazy`/
|
||||
`furnitureGraphic`: неизвестный (не в каталоге) id возвращает `null`/`false`
|
||||
на всех трёх входах, то есть простое удаление `cactus` из каталога без
|
||||
резолвера действительно гасит старый объект — именно то, что ТЗ называет
|
||||
«Риск 1»;
|
||||
- `src/decor-image-editor.ts:407-446` (`renderShapeDialog`) — диалог свойств
|
||||
строит `<option>` предметной формы строго перечислением
|
||||
`GENERATED_FURNITURE_MENU` × `furnitureOfGroup(...)`, то есть только по
|
||||
видимому каталогу, и отмечает текущий выбор через `?selected=${symbol.id ===
|
||||
dialog.symbol}` без запасного варианта; при этом `_decorSaveShape` в
|
||||
`src/houseplan-editor-runtime.ts:4443` пишет `symbol: d.symbol` только если
|
||||
`d.symbol` truthy и он не читается из DOM `<select>`, а из JS-состояния
|
||||
диалога, инициализированного из самого объекта (`houseplan-card.ts:3670`) —
|
||||
то есть ID не переписывается молча, но текстовая подпись варианта у диалога
|
||||
свойств для `cactus` без резолвера покажет не «Тренажёр», а первый пункт
|
||||
списка;
|
||||
- `test/furniture-assets.test.mjs` — тест «`cactus` живёт в `plant`, `exercise`
|
||||
пуста и скрыта» закрепляет именно то отображение, которое issue называет
|
||||
ошибочным (подтверждает названный в ТЗ «Риск 4»);
|
||||
- `demo/smoke_furniture.mjs:106-107` — тот же паттерн в браузерном смоке
|
||||
(`out.menuOnlyCategoriesStayHidden = !category('exercise')`), не назван в ТЗ
|
||||
явно, но подпадает под ту же категорию находок Риска 4 и под обязательный
|
||||
прогон мебельных smoke в AC6;
|
||||
- `src/i18n/{ru,en,fr,de}.json` — во всех четырёх файлах есть `furn.sym_cactus`,
|
||||
нет `furn.cat_exercise`/`furn.sym_exercise`; `furn.group_other` = «Прочее» —
|
||||
терминология ТЗ совпадает с уже принятой в интерфейсе;
|
||||
- `docs/FURNITURE.md:24-29` — текущий текст прямо фиксирует прежнее (ныне
|
||||
отменяемое) решение «кактус — под `plant`, `exercise` пуста и скрыта
|
||||
(owner's decision в #593)»; ТЗ явно требует переписать этот раздел (п. 5
|
||||
объёма работ), несоответствие не противоречит ТЗ, а подтверждает, зачем
|
||||
правка документации в объёме работ обязательна;
|
||||
- `docs/CONFIG-COMPATIBILITY.md` — статус `deprecated-read` («текущая запись
|
||||
использует другое представление, чтение сохраняет старые данные/визуал»)
|
||||
структурно совпадает с описанным в ТЗ поведением псевдонима `cactus`; ТЗ не
|
||||
ссылается на этот документ напрямую, но не противоречит его модели.
|
||||
|
||||
## Проверка §7.1 — комплектность
|
||||
|
||||
Присутствуют все обязательные разделы: сценарий и что человек увидит до/после
|
||||
(«Сценарий и результат для человека») · проблема · скоуп и не-скоуп · контракт
|
||||
поведения · UX · модель данных, i18n и производительность («Данные, локализация,
|
||||
производительность») · миграция (явно: «Схема и версия конфигурации не
|
||||
меняются, миграции нет») · критерии приёмки AC1–AC6 с доказательством и
|
||||
столбцом «что должно краснеть» · план автотестов (распределён по столбцу
|
||||
доказательства AC-таблицы и по предположениям 2 и 4 — тот же формат, что
|
||||
ревьюеры этого репозитория принимали в SPEC-REVIEW-588-r1 и SPEC-REVIEW-402-r1,
|
||||
отдельного одноимённого заголовка не требовалось и там) · риски · откат ·
|
||||
release-артефакты.
|
||||
|
||||
Два продуктовых раздела (кто/где/когда и что видно до/после) идут первыми и
|
||||
отвечают на оба вопроса без терминов реализации.
|
||||
|
||||
Блок «Принятые предположения» (4 пункта) корректно ограничен техническими
|
||||
решениями — механизм резолвера псевдонима, стратегия теста-оракула по 0.4.0,
|
||||
трактовка поля `operation`, стратегия smoke/golden — ни один пункт не
|
||||
подменяет продуктовое решение владельца.
|
||||
|
||||
## Проверка AC1–AC6 на однозначность и доказуемость
|
||||
|
||||
Все шесть критериев формулируют наблюдаемый пользователем результат (видимая
|
||||
категория, рисунок, поведение старого объекта, локализация, проверки) и
|
||||
называют способ доказательства. Ни один не описывает *решение*, для которого в
|
||||
issue/каноне нет опоры:
|
||||
|
||||
- AC1/AC2 опираются на точные, уже проверенные мной значения `pack.json`
|
||||
(33/60, `cactus.menu_icon = plant`, размеры 70×120 и 100×35) — не догадка,
|
||||
а формализация текущего фактического состояния плюс однозначно описанное
|
||||
целевое;
|
||||
- AC3/AC4 корректно называют независимый геометрический факт (`viewBox`,
|
||||
размеры) как границу неизменности при обмене рисунков — измеримо;
|
||||
- AC4 явно требует раздельного покрытия «Риска 1» (исчезновение из View) и
|
||||
«Риска 2» (пустой/неверный вариант в свойствах) — оба риска подтверждены
|
||||
чтением кода (см. «Скоуп разбора» выше) и реальны, а не гипотетичны;
|
||||
- AC5 проверяет отсутствие английского фallback во всех языках — сейчас все
|
||||
четыре файла синхронны по составу ключей `furn.*`, добавление двух новых
|
||||
ключей в каждый — механическая, проверяемая операция;
|
||||
- AC6 называет полный обязательный набор гейтов плюс smoke/golden по
|
||||
необходимости, с прямым требованием не принимать эталоны автоматически.
|
||||
|
||||
Защитный принцип «эталон — не текущий каталог, а независимый источник» назван
|
||||
явно (предположение 2: пакет 0.4.0 остаётся в дереве как oracle) — это
|
||||
закрывает главный риск подмены теста «тест сравнивает код с самим собой».
|
||||
|
||||
## Находки
|
||||
|
||||
### Low (сняты решением ревьюера, доработка не требуется)
|
||||
|
||||
1. **Предположение 1 («резолвер покрывает арт и lazy-gate») по буквальному
|
||||
прочтению не называет третий вход — рендер `<select>` в диалоге свойств
|
||||
(`src/decor-image-editor.ts:430-446`), который перечисляет варианты только
|
||||
по видимому каталогу и не имеет запасной записи для `symbol`, отсутствующего
|
||||
в каталоге.** Я прочитал реальный код (см. «Скоуп разбора») и подтверждаю,
|
||||
что без явного расширения резолвера на эту точку диалог свойств у
|
||||
существующего `cactus`-объекта не покажет «Тренажёр», хотя ID при обычном
|
||||
сохранении не перепишется (значение читается из состояния диалога, не из
|
||||
DOM). Не поднимаю до Medium, потому что ТЗ уже называет ровно этот сценарий
|
||||
как «Риск 2» («свойства старого предмета показывают пустой/неверный вариант
|
||||
— проверить несохраняющее и явное сохраняющее действия») и требует его
|
||||
проверки в AC4; расширение предположения 1 на этот третий вход — техническое
|
||||
решение в рамках «принято предположительно, поменять свободно», а не
|
||||
пробел в контракте. Снимаю с записью: автору стоит явно расширить
|
||||
формулировку предположения 1 при переносе в реализацию, чтобы риск не
|
||||
переоткрывался как находка код-ревью.
|
||||
2. **`demo/smoke_furniture.mjs:106-107` содержит тот же обратный
|
||||
assert (`!category('exercise')`), что и мутационная фикстура #593 в
|
||||
`test/furniture-assets.test.mjs`, но ТЗ называет («Риск 4») только
|
||||
последнюю.** Обе фикстуры одного класса дефекта и обе будут красными сразу
|
||||
после правки каталога, поэтому пропуск в тексте не создаёт риска пропуска в
|
||||
реализации — `npm test` и обязательный локальный прогон мебельных smoke
|
||||
(AC6) заведомо укажут на оба места. Снимаю без действия.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Трек, оценка и продуктовая рамка (J4/J6 по `docs/SCOPE.md`) в комментарии
|
||||
S2-analysis — корректны и обоснованы по критериям §5.
|
||||
- Численные и структурные утверждения ТЗ о текущем пакете (33/60, конкретные
|
||||
поля `cactus`/`bookshelf`/`shelf_floor`, `operation: add`×4/`replace`×56)
|
||||
сверены с `pack.json` и генератором — совпадают без расхождений.
|
||||
- Технический путь «переименовать файл под новый id + поменять местами
|
||||
содержимое двух файлов с прежними именами» согласован с жёсткими правилами
|
||||
генератора (`filename === id`, `boxFillDeviation` против неизменного
|
||||
`viewBox`, согласие групп символа/иконки) — обе операции проходят эти правила
|
||||
без конфликта.
|
||||
- Терминология («Прочее», «Тренажёр» как перевод `Exercise equipment`)
|
||||
совпадает с уже принятой в `src/i18n/*.json`, отдельного изобретения текста
|
||||
интерфейса в ТЗ нет.
|
||||
- Откат, совместимость конфигурации и attribution-требования к новому пакету
|
||||
описаны explicitly и не противоречат `docs/CONFIG-COMPATIBILITY.md` и модели
|
||||
происхождения, уже принятой для 0.4.0.
|
||||
- Ни одно утверждение о будущем поведении не подано как факт без пометки:
|
||||
всё, что не зафиксировано документами, вынесено в явный блок «Принятые
|
||||
предположения» и помечено как техническое.
|
||||
- Продуктовых вопросов владельцу нет и не требуется: оба обязательных
|
||||
вопроса (что видно/делает человек, объём видимых изменений) в issue уже
|
||||
закрыты однозначным текстом.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не запускал `npm run furniture:check`, `npm test`, `npm run typecheck`,
|
||||
`npm run build` и другие гейты кода — на этапе ТЗ продуктового/тестового
|
||||
диффа ещё нет (ветка от `dev` без изменений), гонять их не над чем.
|
||||
- Не открывал SVG-рисунки визуально в браузере/рендерере — идентификация
|
||||
«файл `cactus.svg` на самом деле рисует тренажёр» это продуктовое суждение
|
||||
владельца о содержании конкретной иллюстрации (в его полномочиях, не
|
||||
техническая проверяемая деталь), я лишь убедился, что путь SVG синтаксически
|
||||
валиден и его `viewBox`/масштаб (70×120) соответствуют заявленным размерам
|
||||
тренажёра.
|
||||
- Не проверял golden-каталог `demo/golden/matrix.mjs` на предмет того, какие
|
||||
именно существующие golden-сцены задействуют `cactus`/`bookshelf`/
|
||||
`shelf_floor` — это часть исполнения AC6 («релевантные golden-сцены»), а не
|
||||
предмет ревью ТЗ.
|
||||
- Не сверял `scripts/config-field-registry.mjs` на предмет регистрации
|
||||
`symbol: cactus` как записи `deprecated-read` — модель `pack.json`-значений
|
||||
(перечислимые id, а не структурные поля конфигурации) может быть вне области
|
||||
этого реестра; это техническое решение реализации, а не пробел контракта.
|
||||
|
||||
## Материал раунда
|
||||
|
||||
Issue #606, тело на момент вынесения вердикта (раздел `## ТЗ` плюс
|
||||
предшествующее ему описание, на которое ТЗ явно ссылается). Ветка
|
||||
`issue/606-furniture-catalog-corrections` от `dev`@`0d30dde17e6b3924bc4a19add800bd852586107e`
|
||||
без продуктовых изменений — код читался с этого дерева как контекст для
|
||||
проверки утверждений ТЗ, не как материал стадии.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/606-furniture-catalog-corrections`, коммит `0d30dde17e6b` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `7fccacda2e293d7222d3c8dbea6c5db52cc65f54`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 7fccacda2e29
|
||||
```
|
||||
- Тело issue: `96bd494bbf2f992639dc27711a166b327427c83505722e10119c467c66dbbfd5`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user