docs: review document for #359

Issue: #359
User-Visible: no
This commit is contained in:
claude[bot]
2026-08-28 21:14:24 +00:00
parent 842dc373b1
commit 3c281bb12c
+135
View File
@@ -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 снята записью без правки
текста. Открытых продуктовых вопросов нет. Готово к «Готово к разработке».