diff --git a/docs/reviews/SPEC-REVIEW-159-r1.md b/docs/reviews/SPEC-REVIEW-159-r1.md new file mode 100644 index 00000000..13e39455 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-159-r1.md @@ -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 → в задаче`