mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-05 06:08:59 +00:00
claude[bot]
@@ -0,0 +1,262 @@
|
||||
# SPEC-REVIEW-159-r1
|
||||
|
||||
- **Issue:** https://github.com/Matysh/houseplan-card/issues/159
|
||||
- **ТЗ под ревью:** [`docs/specs/159-furniture-pack.md`](https://github.com/Matysh/houseplan-card/blob/issue/159-furniture-pack/docs/specs/159-furniture-pack.md), commit `a9d4d9e6`, обычный трек (не `small`/`trivial`)
|
||||
- **Роль:** ревьюер ТЗ (не автор), этап `S4-spec-review`
|
||||
- **Лимит циклов:** обычный трек — 4 (PROCESS.md §4)
|
||||
- **Заход:** r1 · блокирующих циклов израсходовано 0/4
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
ТЗ #159 описывает вендоринг присланного дизайнерского набора мебели (33
|
||||
фронтальные SVG-иконки категорий + 44 top-view SVG: 18 `replace` + 26 `add`),
|
||||
детерминированный compile-time генератор безопасной path-геометрии из этого
|
||||
набора в два TS-каталога (`plan-art` для View, `menu-art` только для
|
||||
ленивого editor-графа) и новую двухуровневую палитру «категории → варианты»
|
||||
в редакторе подложки. Итоговая библиотека — 56 top-view символов, persisted
|
||||
schema `decor[].symbol` не меняется, миграции нет.
|
||||
|
||||
Не в скоупе ревью: продуктовый код — на ветке `issue/159-furniture-pack`
|
||||
лежит только ТЗ (плюс правка `docs/specs/README.md`), реализации нет. Гейты
|
||||
(`typecheck`/`test`/`build`) не прогонялись — на этапе ревью ТЗ продуктового
|
||||
кода не существует, что вне скоупа этапа (PROCESS.md §2.4/§8).
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Прочитаны целиком `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` (действующая
|
||||
редакция — §1, §2.2–§2.10, §4, §5, §7.1, §7.2, §8).
|
||||
2. Прочитано тело issue #159 и все четыре комментария: требования владельца к
|
||||
handoff, комментарий `nikitaevfz-commits` (association `NONE`) с архивом,
|
||||
аналитика владельца (P2, feature, полный трек, ссылка на #159 job'ы
|
||||
J4/J6), передача на ревью.
|
||||
3. Прочитан весь текст ТЗ (365 строк) и сверены обязательные разделы §7.1:
|
||||
сценарий и персона, что человек увидит до/после, проблема, проверенный
|
||||
вход и права, scope/не-scope, каталог и совместимость, контракт UX, рендер
|
||||
и безопасность, модель данных и миграция, генерация/source of truth, i18n,
|
||||
производительность и bundle, AC1…AC9 с доказательством, план автотестов,
|
||||
release-артефакты, риски, откат, блок «принято предположительно». Все
|
||||
присутствуют. Продуктовые разделы (персона `docs/SCOPE.md`, поверхность,
|
||||
момент; «что увидит» без терминов реализации) — на месте (строки 7–21).
|
||||
4. Сверена арифметика каталога: 18 `replace` + 12 retained = 30 прежних ID
|
||||
(совпадает с `FURNITURE_GROUPS`/списком в `src/furniture.ts` и таблицей
|
||||
`docs/FURNITURE.md` §3: 12+8+6+4=30); + 26 новых = 56 (AC2). Перечисленные
|
||||
18/26/12 ID построчно сосчитаны и не пересекаются.
|
||||
5. Прочитан `docs/FURNITURE.md` целиком и сверены цитируемые в ТЗ инварианты:
|
||||
backend не валидирует `symbol` по closed-list (§8, «An id this build has
|
||||
never heard of simply renders as nothing») — ТЗ (раздел «Модель данных и
|
||||
миграция») цитирует это точно.
|
||||
6. Прочитан `docs/UX-MODES.md` (принцип «context tray» как общая
|
||||
second-level поверхность для будущих согласованных групп инструментов) —
|
||||
двухуровневая палитра ТЗ (категории → варианты) укладывается в этот
|
||||
принцип, а не изобретает новый паттерн навигации.
|
||||
7. Прочитан `docs/USER-GUIDE.ru.md` (строки 1227–1305, разделы
|
||||
«14. Редактор подложки» и «Мебель») — текущая терминология («Мебель»,
|
||||
«Символ», «Свойства», группы) не противоречит формулировкам ТЗ.
|
||||
8. Проверено импортирование: `src/furniture.ts` импортируется и
|
||||
`houseplan-card.ts` (initial View), и `houseplan-editor-runtime.ts`
|
||||
(ленивый editor) — заявление ТЗ «plan-art нужен обычному View, menu-art —
|
||||
только editor runtime» технически обосновано существующей архитектурой
|
||||
лениво загружаемого редактора, а не догадкой.
|
||||
9. Прочитан `scripts/bundle-budget.mjs` — общий `INITIAL_VIEW_GZIP_BUDGET`
|
||||
(282 000 B gzip) и его печать `headroom` независимы от
|
||||
task-specific цели «≤18 KiB» из ТЗ; это разные величины (см. Medium-1).
|
||||
10. **Проверена цитируемая провенанс-цепочка** (см. High-1): сопоставлены
|
||||
ссылки в ТЗ (строки 42–45) и в аналитике владельца с фактическим текстом
|
||||
комментария, на который они ссылаются
|
||||
(`https://github.com/Matysh/houseplan-card/issues/159#issuecomment-5449707137`),
|
||||
полученным напрямую через `gh issue view 159 --json comments`.
|
||||
11. Проверена трассируемость: `docs/specs/README.md:126` содержит строку
|
||||
`#159 ↔ 159-furniture-pack.md`, issue ссылается на файл ТЗ и наоборот —
|
||||
связь двусторонняя.
|
||||
12. AC1–AC9 прочитаны на однозначность и способ доказательства — у каждого
|
||||
указан метод (`unit`/`smoke`/`golden`/`build`/«ревью кода»); формулировки
|
||||
не допускают двух прочтений, кроме зависимости AC8 от неоднозначно
|
||||
определённого порога (см. Medium-1).
|
||||
|
||||
## Находки
|
||||
|
||||
### High-1 — провенанс и лицензия 77 вендоруемых SVG опираются на ссылку, которая не подтверждает заявленное
|
||||
|
||||
ТЗ, строки 42–45 (раздел «Проверенный вход и права»):
|
||||
|
||||
> Автор и владелец репозитория явно подтвердил в запросе на реализацию, что
|
||||
> все иконки нарисованы им собственноручно. В репозиторий попадает
|
||||
> нормализованная копия с `author: Matysh`, `license: MIT`; исходный `TBD`
|
||||
> из архива не переносится как релизная метаинформация.
|
||||
|
||||
Дословно та же формулировка — в аналитике владельца (комментарий issue
|
||||
`#5453879505`, пункт «Проверенный входной пакет», последний буллет), со
|
||||
ссылкой `[явно подтвердил](.../issues/159#issuecomment-5449707137)`.
|
||||
|
||||
**Воспроизведение.** Комментарий `#issuecomment-5449707137`, на который
|
||||
указывает ссылка, получен напрямую:
|
||||
|
||||
```
|
||||
gh issue view 159 --repo Matysh/houseplan-card --json comments \
|
||||
-q '.comments[] | select(.author.login=="nikitaevfz-commits")'
|
||||
```
|
||||
|
||||
Его полный текст:
|
||||
|
||||
> Внутри:
|
||||
> - 33 фронтальные SVG для меню;
|
||||
> - 44 SVG вида сверху для плана;
|
||||
> - manifest.json;
|
||||
> - светлое и тёмное превью;
|
||||
> - пример размещения на плане;
|
||||
> - документация и скрипт проверки.
|
||||
>
|
||||
> [houseplan-furniture-custom-0.3.0.zip](...)
|
||||
|
||||
Этот комментарий — только опись содержимого архива. В нём нет ни слова о
|
||||
том, кто рисовал иконки, и нет заявления о лицензии. `includesCreatedEdit:
|
||||
false` — комментарий не редактировался, то есть подтверждение не было
|
||||
стёрто задним числом.
|
||||
|
||||
Дополнительно: автор этого комментария, `nikitaevfz-commits`, имеет
|
||||
`authorAssociation: NONE` — GitHub-гарантированный признак того, что у
|
||||
аккаунта нет прав записи и никакой формальной связи с репозиторием. Формула
|
||||
ТЗ «автор и владелец репозитория» either отождествляет постороннего
|
||||
контрибьютора с владельцем репозитория (`Matysh`, `OWNER`), либо утверждает,
|
||||
что оба они что-то подтвердили — но единственная процитированная ссылка
|
||||
ведёт на комментарий постороннего аккаунта без единого слова о том, кто
|
||||
рисовал символы. Сам архив, по признанию той же аналитики, содержит
|
||||
`author/license: TBD` — то есть отправитель тоже не заявил лицензию явно.
|
||||
|
||||
**Почему это High, а не бумажная придирка.** House Plan распространяется под
|
||||
MIT и включает мебель прямо в bundle/HACS — `docs/FURNITURE.md` §2 посвящает
|
||||
лицензии отдельный аудит именно потому, что ошибка здесь означает поставку
|
||||
чужого или недостаточно лицензированного контента под чужим именем и под MIT
|
||||
без разрешения правообладателя. ТЗ прямо предписывает записать в репозиторий
|
||||
`author: Matysh`, `license: MIT` (Scope п.1, `assets/furniture/…/manifest.json`
|
||||
+ README/provenance) — то есть закоммитить фактическое юридическое
|
||||
утверждение, для которого процитированное доказательство не подтверждает ни
|
||||
авторство, ни передачу прав. AC1 («Vendored source соответствует
|
||||
зафиксированному архиву... нормализованы только метаданные author/license»)
|
||||
не может быть закрыт кодревью так, как написано: единственная проверяемая
|
||||
вещь — что метаданные *изменены* на `Matysh`/`MIT`, а не что это изменение
|
||||
корректно.
|
||||
|
||||
**Это не продуктовый вопрос** (не «что видит пользователь»), поэтому не
|
||||
подлежит вынесению владельцу как есть — но фактическое заявление,
|
||||
которого нет ни в одном процитированном источнике и которое не помечено как
|
||||
предположение, само по себе является замечанием ревью
|
||||
(инструкция к этапу spec, PROCESS.md §7.1 «догадка, записанная как факт —
|
||||
худший вид дефекта»).
|
||||
|
||||
**Рекомендация автору.** Либо процитировать реальный источник подтверждения
|
||||
(например, отдельный комментарий-заявление от `nikitaevfz-commits` с явной
|
||||
передачей прав/лицензии, либо иной документированный канал — Telegram
|
||||
из `docs/SCOPE.md`, если разговор был там, с точной ссылкой/цитатой), либо
|
||||
запросить такое заявление до того, как ТЗ фиксирует `author: Matysh,
|
||||
license: MIT` как решённый факт. Пока подтверждения нет, AC1/Scope п.1
|
||||
описывают недоказуемое действие.
|
||||
|
||||
### Medium-1 (в скоупе) — AC8 не согласован с описанием бюджета: неясно, «≤18 KiB» — блокирующий порог или цель для пересогласования
|
||||
|
||||
ТЗ, строки 247–249 (раздел «Производительность и bundle»):
|
||||
|
||||
> Целевой дополнительный initial View gzip для 44 plan SVG — не более 18 KiB;
|
||||
> превышение требует упростить представление либо отдельно пересогласовать
|
||||
> продуктовую цену, а не поднять общий бюджет.
|
||||
|
||||
ТЗ, строки 300–302 (AC8):
|
||||
|
||||
> Budget проходит; initial View gzip delta ≤18 KiB, menu-art отсутствует в
|
||||
> initial graph; один мебельный объект создаёт один основной plan path и
|
||||
> один erase-hit только в активном erase-mode.
|
||||
|
||||
Первая формулировка называет 18 KiB «целевым» значением с явным путём выхода
|
||||
(«либо... либо отдельно пересогласовать продуктовую цену») — то есть
|
||||
превышение не обязательно провал, а повод вернуться к владельцу. Вторая
|
||||
формулировка (AC8) перечисляет то же число как обычный проверяемый критерий
|
||||
приёмки наравне с «Budget проходит» (уже существующий жёсткий гейт
|
||||
`bundle:budget`, 282 000 B). Кодревью не может одновременно считать AC8
|
||||
единственным источником истины для «прошёл/не прошёл» и знать, что порог на
|
||||
самом деле мягкий. Если фактическая дельта окажется, скажем, 22 KiB —
|
||||
это провал AC8 (задача не готова) или основание вернуться к аналитике за
|
||||
пересмотром продуктовой цены (задача в порядке, просто дороже)? Текст не
|
||||
говорит, какой из двух путей выбрать, а это меняет весь исход код-ревью.
|
||||
|
||||
Не является продуктовым вопросом (per PROCESS.md §7.1 второй критерий —
|
||||
только про то, что видит пользователь) — это техническое расхождение внутри
|
||||
одного документа, автор решает сам.
|
||||
|
||||
**Рекомендация:** либо снять эскейп-хэтч и оставить AC8 жёстким порогом,
|
||||
либо явно написать в AC8, что «превышение возвращает задачу на пересмотр
|
||||
scope, а не проваливает код-ревью формально» — так, чтобы у код-ревью на
|
||||
следующем этапе была одна интерпретация, а не две.
|
||||
|
||||
### Low-1 (снят с запиской) — непоследовательное форматирование метода доказательства в заголовке AC6
|
||||
|
||||
Заголовок «### AC6 — локализации (`unit`, smoke)» — из всех девяти AC только
|
||||
здесь второй метод не взят в обратные кавычки (везде остальные —
|
||||
`` `unit` ``, `` `smoke` `` и т.д. одинаково оформлены). Чисто косметическое,
|
||||
не влияет на однозначность критерия. Снимаю без правки: автор может поправить
|
||||
заодно с Medium-1, отдельного действия не требую.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Все обязательные разделы §7.1 присутствуют, включая оба продуктовых
|
||||
(сценарий/персона, «что увидит»).
|
||||
- Арифметика каталога (18 replace + 12 retained = 30; +26 add = 56) сходится
|
||||
построчно и совпадает с кодом (`src/furniture.ts`, `FURNITURE_GROUPS`) и
|
||||
`docs/FURNITURE.md`.
|
||||
- Технический план (generated TS вместо runtime SVG import, раздельные
|
||||
plan-art/menu-art графы, `--check` детерминированность) обоснован
|
||||
существующей архитектурой ленивого редактора (`houseplan-editor-runtime.ts`)
|
||||
и правилом «no user markup in config», уже установленным
|
||||
`docs/FURNITURE.md` §9 («Not a user-extensible library»).
|
||||
- Цитата про backend forward-compatibility (символ не проверяется по
|
||||
closed-list) дословно совпадает с `docs/FURNITURE.md` §8.
|
||||
- Модель данных: persisted schema действительно не меняется (никаких новых
|
||||
полей), что соответствует `docs/CONFIG-COMPATIBILITY.md` (символ — opaque
|
||||
regex-валидируемая строка, не версия модели).
|
||||
- Не-скоуп корректно исключает то, что `docs/SCOPE.md` уже запрещает
|
||||
(3D/изометрия, произвольная загрузка пользовательских SVG — прямое
|
||||
повторение «Not a user-extensible library»).
|
||||
- Двухуровневая палитра «категории → варианты» укладывается в принцип
|
||||
context tray из `docs/UX-MODES.md» («shared second-level surface for future
|
||||
explicitly approved tool groups»), а не вводит незнакомый паттерн.
|
||||
- Решение «даже категория с одним вариантом проходит второй экран» явно
|
||||
помечено как предположение в блоке «Принято предположительно» (строки
|
||||
361–363) — соответствует процессу флагирования решений, подлежащих
|
||||
оспариванию ревьюером, а не эскалации владельцу.
|
||||
- Трассируемость issue ↔ ТЗ двусторонняя и зафиксирована в
|
||||
`docs/specs/README.md:126`.
|
||||
- AC1–AC9 в остальном однозначны и у каждого назван метод доказательства.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Содержимое самого архива (`houseplan-furniture-custom-0.3.0.zip`) и
|
||||
77 SVG внутри — на этапе ревью ТЗ продуктового кода/вендоринга ещё нет,
|
||||
архив не скачивался и не разбирался файл за файлом; технический
|
||||
валидатор из отчёта аналитики («TECHNICAL VALIDATION PASSED») не
|
||||
перезапускался.
|
||||
- Фактическая величина gzip-прироста (18 KiB) не пересчитывалась —
|
||||
на этапе ТЗ бандла с новыми SVG не существует.
|
||||
- Golden/визуальные сцены — не создавались и не оценивались, сцены не
|
||||
существует до реализации.
|
||||
- Смоки/юнит-тесты — не запускались: реализации нет, `npm test`/`tsc`/`build`
|
||||
вне скоупа этапа `S4-spec-review` (PROCESS.md §2.4/§8).
|
||||
- Достоверность самого факта «иконки нарисованы дизайнером лично» не
|
||||
проверялась и не может быть проверена ревьюером ТЗ — оценивалась только
|
||||
связь между заявлением ТЗ и процитированным им источником (см. High-1).
|
||||
|
||||
## Вердикт
|
||||
|
||||
High: 1 (провенанс/лицензия 77 SVG опираются на ссылку, которая не
|
||||
подтверждает заявленное — Scope п.1 и AC1 фиксируют недоказанный факт как
|
||||
решённый) · Medium: 1 → в задаче (AC8 внутренне противоречив: «цель с путём
|
||||
пересогласования» против «жёсткого критерия приёмки») · Low: 1, снят с
|
||||
запиской (форматирование в заголовке AC6).
|
||||
|
||||
**Жёлтый.** Находка High-1 не опровергает техническую часть ТЗ (каталог,
|
||||
генератор, UX-контракт, модель данных выполнимы и проверяемы) — она
|
||||
блокирует только раздел провенанса/лицензии, который правится точечно:
|
||||
процитировать реальное подтверждение авторства и лицензии либо получить его
|
||||
до фиксации `author: Matysh, license: MIT` как факта. Вместе с Medium-1
|
||||
(уточнить, что означает превышение 18 KiB) это возвращается автору на один
|
||||
цикл, без пересмотра архитектуры.
|
||||
|
||||
`Вердикт: жёлтый · заход r1 · блокирующих циклов 0/4 · High: 1 · Medium: 1 → в задаче`
|
||||
Reference in New Issue
Block a user