diff --git a/docs/reviews/SPEC-REVIEW-359-r2.md b/docs/reviews/SPEC-REVIEW-359-r2.md new file mode 100644 index 00000000..6162afdd --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-359-r2.md @@ -0,0 +1,135 @@ +# SPEC-REVIEW-359-r2 + +Issue: [#359 — Предпросмотр мебели на плане перед размещением](https://github.com/Matysh/houseplan-card/issues/359) +ТЗ: [docs/specs/359-furniture-placement-preview.md](https://github.com/Matysh/houseplan-card/blob/issue/359-furniture-placement-preview/docs/specs/359-furniture-placement-preview.md) +Материал: коммит `842dc373` "docs: prove furniture preview edge cases" (HEAD на +момент разбора; `git rev-parse HEAD` = `842dc373b193acfc8ed2c8f5940f69c400e4cf3f`). +Предыдущий раунд: [SPEC-REVIEW-359-r1](https://github.com/Matysh/houseplan-card/blob/dev/docs/reviews/SPEC-REVIEW-359-r1.md), +материал `392ef22c` "docs: specify furniture placement preview" — SHA назван +явно в самом документе r1 (строка «Материал: коммит `392ef22c`»), так что +находки «SHA в вердикте не назван» здесь нет. +Заход: r2. Блокирующих циклов израсходовано 1/4 (израсходован r1 — жёлтый +вердикт с Medium в скоупе). + +## Скоуп разбора + +Второй раунд, дельта не задевает продуктовую рамку: `git diff +392ef22c..842dc373 -- docs/specs/359-furniture-placement-preview.md` — 24 +добавленные и 7 удалённых строк, только в разделах «Критерии приёмки» (добавлены +AC9, AC10), «План автотестов» (пункты 2 и 4, старое обоснование «golden не +требуется» удалено) и «Release-артефакты» (одна строка). Сценарий, проблема, +скоуп/не-скоуп, контракт поведения пп.1-8 и 10, UX, модель данных, i18n, AC1-AC8, +риски (кроме перечисленных правок) и раздел «Принято предположительно» не +менялись — они наследуются из r1 без повторной проверки (раздел ниже). + +Это ровно тот случай PROCESS.md §2.10, где объём разбора сокращается до +дельты: правки автора точечно отвечают на обе Medium-находки r1 и не меняют +контракт, не задевают новую подсистему и по объёму несопоставимы с исходной +задачей. + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где это видно | +|---|---|---| +| Medium 1 — контракт п.9 (invalid/unknown `symbol`) не имел ни одного AC/теста-доказательства; риск «Fail dark» был декларацией, не проверкой | Добавлен **AC9** — «неизвестный символ»: явно требует «не создаёт `.furniture-placement-preview`», «не добавляет предмет в `space.decor`», «не вызывает исключение», с доказательством «unit-тест resolver на неизвестный id и `demo/smoke_furniture.mjs` с принудительным invalid palette state» | `docs/specs/359-furniture-placement-preview.md:142-146`; синхронно расширен пункт 2 «Плана автотестов» — «fail-dark неизвестного symbol с последующим восстановлением валидного выбора» (строки 157-160) | +| Medium 2 — «golden не требуется» обосновано общей фразой «transient = шум», которая опровергается уже существующими `hoverRoom`/`hoverDevice`/`junction-draft-end-node-dark` сценариями того же файла; AC7 (DOM/computed-style) не ловит дефекты z-order/композитинга — ядро ценности фичи | Добавлен **AC10** — «композиция»: ghost виден поверх сохранённого decor и ниже стен в одном composition layer, доказательство — один детерминированный golden-сценарий `furniture-placement-preview-light`, состояние которого «вооружается» программно (тот же приём, что `hoverRoom`/`draft: true` в `demo/golden/matrix.mjs`), без реального hover-таймера | `docs/specs/359-furniture-placement-preview.md:147-151`; план автотестов п.4 (строки 164-170, конкретизирует сценарий и приёмку эталона только через `npm run golden:accept -- --reviewed` по полному Linux-артефакту); Release-артефакты (строки 202-205) заменили «golden/screenshots не требуются» на обязательство добавить сценарий | + +Проверка нового текста, а не только его наличия: + +- **AC9** технически исполним: `furnitureGraphic(id)` в `src/furniture.ts:362` + уже сегодня возвращает `null` на неизвестный id — единый resolver, который + контракт п.5 требует построить, естественно оборачивает эту проверку. Способ + доказательства «принудительное invalid palette state» уже является рабочим + паттерном smoke-тестов: `demo/smoke_furniture.mjs:49` прямо присваивает + `c._furnPalette = null`/объект, то есть `c._furnPalette = { symbol: 'unknown-id', ... }` + для нового кейса не требует новой инфраструктуры теста. +- **AC10** технически исполним: `demo/golden/matrix.mjs` уже строит сценарии + через декларативные флаги без реального pointer-таймера — + `hoverRoom`/`hoverDevice` (строки 452, 542, 628, 630) и `draft: true` + (строка 668, `junction-draft-end-node-dark`). Новый флаг для preview-состояния + ложится в тот же механизм. Одиночный `-light`-сценарий без парного `-dark` + не противоречит практике: `decor-over-opaque-hover-light` в том же файле тоже + без тёмной пары. Правило принятия эталона (`golden:accept --reviewed` по + полному Linux CI, не локально «ради зелёного CI») сформулировано в + «Плане автотестов» и в «Release-артефактах» одинаково и совпадает с §12 + PROCESS.md и `demo/golden/README.md`. + +Обе находки закрыты по существу, не декларативно: новый текст называет +конкретный AC, конкретный способ доказательства и опирается на уже +существующий в репозитории код/паттерн, а не на новое обещание. + +## Унаследовано из r1 + +Без повторной проверки в этом раунде принято (документ +`docs/reviews/SPEC-REVIEW-359-r1.md`, SHA `392ef22c`, раздел «Что проверено и +корректно»): + +- обязательные разделы §7.1 присутствуют полностью, включая обе продуктовые + вставки («Сценарий», «Что человек увидит») из AGENTS.md; +- персона/поверхность/сценарий верны и привязаны к J4/J6 `docs/SCOPE.md`; лок- + инвариант, View/kiosk и «Out of scope» не задеты; +- touch-контракт (п.10, «Не-скоуп») дословно совпадает с `docs/TOUCH-SUPPORT.md`; +- магнит к стене и роль `Shift` (контракт п.3) совпадают с `docs/CANVAS.md` + §9.4, `docs/USER-GUIDE.ru.md:1305-1311` и текущим кодом (`_furnPlace`, + `_furnMoveUpdate`, `snapFurnitureToWall`); +- единый resolver (контракт п.5) реализуем без новой архитектуры — `_furnPlace` + уже сегодня короткая цепочка чистых вызовов; +- условие видимости preview (контракт п.1) и его touch-симметрия (п.10) + опираются на существующий `src/pointer-modality.ts` и + `docs/TOUCH-SUPPORT.md`, а не на новый механизм; +- терминология UX-раздела («decor composition layer») и значение opacity + `0.55` — существующий код/визуальный язык проекта, не догадки; +- i18n, модель данных/миграция, откат — корректны и не требуют пересмотра; +- открытых продуктовых вопросов не найдено (проверялось отдельно: opacity, + отсутствие рамки, гибридные touch+mouse устройства); +- не-скоуп ограничивает риск расползания (multi-stamp, размерные плашки, + collision detection, миграция схемы, move/resize/rotate существующей мебели). + +Ни один из этих пунктов дельта r1→r2 не задевает: правки лежат только в +AC9/AC10, соседних строках плана автотестов и одной строке release-артефактов. + +## Находки нового раунда + +Ни одной находки уровня High или Medium в дельте не найдено. + +**Low (снята записью, не требует правки).** План автотестов, пункт 1 +(«…покрыть его unit-тестами для обычной точки, стены, `Shift`-пути и clamp у +границы», строка 155-156) не упоминает кейс неизвестного `symbol`, хотя AC9 +обещает именно «unit-тест resolver на неизвестный id». Перечисление в пункте 1 +не полное относительно AC9, но пункт 2 того же плана прямо называет +smoke-часть этого же кейса, а сам AC9 достаточно однозначен как источник +истины о необходимости unit-теста. Реализация не может «не заметить» этот +случай — он назван в AC. Снимаю без правки текста: расширять перечисление +ради полноты списка, который и так избыточен по отношению к AC, — не стоит +второго цикла ревью. + +## Что проверено и корректно (новый текст раунда) + +- AC9 и AC10 однозначны, каждый называет способ доказательства и не + подменяет продуктовое решение техническим вопросом владельцу; +- AC9/AC10 не открывают новых продуктовых вопросов — оба чисто технические + (что не создаётся при невалидном вводе; каким тестом ловится z-order); +- новый golden-сценарий не нарушает §12/§11.4: явно назван путь принятия + через `golden:accept --reviewed` по полному CI-артефакту, а не локальное + принятие; +- нумерация AC (1-10) и плана автотестов (1-5) внутренне непротиворечива, + перекрёстные ссылки (AC9→план п.2, AC10→план п.4) на месте. + +## Чего не проверял + +- исполняемость AC9/AC10 в реальном коде — кода ещё нет, это код-ревью на S7; +- всё, что перечислено в «Чего не проверял» документа r1 (Playwright-прогон, + typecheck/test/build/bundle/smoke, `check-docs.mjs`, точные формулировки + будущих i18n/changelog-записей, реальный перф render-цикла) — дельта этого + раунда не меняет применимость этих пунктов, они по-прежнему предмет + код-ревью; +- `docs/specs/README.md` не содержит записи про #359 — не проверялось как + находка: колонка «Статус ТЗ» в этом файле помечена PROCESS.md §7.3 как + уходящая сама по себе, вне рамок этого ревью. + +## Вердикт + +Зелёный: 0 High, 0 Medium. Обе Medium-находки r1 закрыты предметно (AC9, AC10 +с исполнимым способом доказательства, опирающимся на существующий код и +существующую практику golden-сценариев). Одна Low снята записью без правки +текста. Открытых продуктовых вопросов нет. Готово к «Готово к разработке».