diff --git a/docs/reviews/SPEC-REVIEW-460-r1.md b/docs/reviews/SPEC-REVIEW-460-r1.md new file mode 100644 index 00000000..d4d4f733 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-460-r1.md @@ -0,0 +1,78 @@ +# SPEC-REVIEW-460-r1 + +Issue: [#460](https://github.com/Matysh/houseplan-card/issues/460) — «Смок мебели краснеет через раз: у живого пути #451 нет точки синхронизации, аналогичной `updateComplete`». +Этап: spec (PROCESS.md §2.4). Заход r1. Блокирующих циклов израсходовано: 0/4. +Материал: `docs/specs/460-live-editor-settlement.md` на SHA `3a566a01` (ветка `issue/460-live-editor-settle`), тело issue #460 и все четыре комментария. + +## Скоуп проверки + +Полный разбор — это первый заход ревью ТЗ, раздел «по дельте» (§2.10) не применяется. + +Проверялось: + +- полнота обязательных разделов ТЗ по PROCESS.md §7.1; +- однозначность и проверяемость AC1…AC4 и указанный способ доказательства для каждого; +- нет ли в тексте догадки, выданной за факт (утверждение о поведении без опоры на код/документ и без пометки «предположение»); +- соответствие описанного технического механизма фактическому состоянию кода — не потому что это код-ревью, а потому что ТЗ, которое просит нереализуемого или описывает несуществующий путь, невозможно проверить как «выполнимое»; +- вопросы, которые ТЗ решает самостоятельно, действительно ли они технические (§7.1), а не продуктовые, ошибочно закрытые автором; +- трек: подтверждение, что `trivial` был обоснованно снят (комментарий владельца) и полный трек сейчас уместен. + +## Как проверялось + +Прочитаны в указанном порядке `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md`, тело issue #460 и все комментарии, `docs/specs/460-live-editor-settlement.md`, `docs/FURNITURE.md` (граница жизни preview), `docs/UX-MODES.md` (mode transition). + +Для проверки правдоподобия контракта (не для код-ревью, а чтобы отличить обоснованное техническое решение от догадки) прочитан целиком `src/live-editor.ts` (411 строк) и точечно — `src/houseplan-editor-runtime.ts` (`routeHouseplanEditorUpdate` вызов, `_commitLiveEditor`), `src/houseplan-card.ts` (`updated()` вокруг строки 4009-4011, `_refitView`/`_refitRaf`, `_modeTransitionBusy`), три смока (`demo/smoke_furniture.mjs`, `demo/smoke_decor.mjs`, `demo/smoke_decor_text.mjs`) и `test/live-editor.test.mjs` (наличие файла, который ТЗ собирается расширять). Гейты (`typecheck`/`test`/`build`) не запускались — стадия ТЗ, продуктовый код ещё не менялся, запуск гейтов ничего не доказывает. + +## Находки + +Находок уровня High и Medium нет. Два наблюдения ниже — Low, обе решены снятием с записью, вердикт не меняют. + +| # | Наблюдение | Где | Решение | +|---|---|---|---| +| L1 | ТЗ не собирает явный список затронутых файлов одним местом (DoR §2.5 требует «перечислены затронутые файлы и модули»). Модули называются россыпью: `live-editor` в разделе «Принятые предположения», три смока — в AC3, `test/live-editor.test.mjs` — в плане автотестов; `src/houseplan-editor-runtime.ts` (где живёт вызов `routeHouseplanEditorUpdate`, что и роутит terminal-null мимо полного Lit-цикла — проверено чтением) не назван нигде явно. | Весь документ | Снято: перечень восстановим по тексту документа (Скоуп + Assumptions + AC3 + план автотестов), двух реально задетых src-файлов (`live-editor.ts`, `houseplan-editor-runtime.ts`) достаточно для суждения о размере правки, а ревью кода получит точный diff. Не блокирует переход в `S5-ready`. | +| L2 | Release-артефакты не называют `docs/DEVELOPMENT.md` как место для новой «грабли»: у живого пути до этой задачи не было точки «я успел покрасить», и это ровно тот класс дефекта, на который ссылается сам issue (аудит 05.09, M1 — восемь модулей #451 без проверки собственных контрактов). Будущий живой модуль имеет тот же риск обойти новый контракт локальным RAF, как это уже произошло трижды в смоках. | Раздел «Release-артефакты» | Снято, не как отдельное AC: `AGENTS.md` уже требует «DEVELOPMENT.md для новых грабель» как стандинг-политику §2.6 реализации, действующую независимо от того, назвал ли её ТЗ явно; не переношу это в блокирующее замечание, но фиксирую здесь, чтобы код-ревью проверило, появилась ли запись. | + +## Что проверено и признано корректным + +- **Все обязательные разделы §7.1 присутствуют**: сценарий, что человек увидит до/после, проблема, скоуп/не-скоуп, контракт поведения, UX, модель данных и миграция, i18n, AC1…AC4 с доказательством, план автотестов, риски, откат, release-артефакты, блок «принято предположительно». +- **Сценарий и «что человек увидит»** названы в пользовательских терминах (админ дома, размещение мебели, preview, которое иногда не исчезает) без терминов реализации — соответствует требованию §7.1. +- **Каждый AC имеет названный способ доказательства**: AC1 — управляемый unit с fake RAF; AC2 — unit + существующая проверка `pointerLeaveClearsPreview` в `smoke_furniture.mjs`; AC3 — фактические прогоны трёх смоков (мебельный — ≥10 подряд); AC4 — `scripts/mutation-gate.mjs` с двумя названными мутантами. Ни один AC не привязан к «проверил локально» без метода. +- **Границы preview совпадают с уже документированным контрактом**: `docs/FURNITURE.md` (строка 65-66) перечисляет ровно то же множество событий очистки (`pointer leave, Escape, palette/tool/editor/space changes and remount`), которое ТЗ повторяет в разделе «UX» — терминология взята из канона, а не изобретена. +- **Технический механизм правдоподобен и опирается на реальный код**, а не на воображаемую архитектуру: + - в `src/live-editor.ts` действительно нет ревизии/промиса — только счётчик `raf: number` (`LiveEditorState`, строка 15) и функции `scheduleHouseplanEditor` / `paintHouseplanEditor` / `commitHouseplanEditor` / `disposeHouseplanEditor` — ровно та точка, где ТЗ предлагает завести awaitable-контракт; + - `commitHouseplanEditor()` действительно вызывается на **каждом** полном Lit `updated()` (`houseplan-card.ts:4011`, безусловно), что делает пункт контракта 3 («полный Lit commit — валидное завершение») не догадкой, а описанием существующего инварианта; + - terminal-null-проблема (пункт 4 контракта) подтверждается чтением `routeHouseplanEditorUpdate`: `hoverProperties.has(name)` делает `route = true` независимо от нового значения свойства, то есть присвоение `null` (`_furnPreviewInput = null` при `pointerleave`) сегодня уходит **только** в лёгкий RAF-путь и не проходит через `commitHouseplanEditor` — ровно то независимое подтверждение, которого требует правило «утверждение о поведении — либо факт из кода, либо помеченное предположение»; + - «mode/refit» — не изобретённые термины: `_modeTransitionBusy` и `_refitRaf`/`_refitView` реально существуют в `houseplan-card.ts` как наблюдаемые внутренние флаги, то есть у смока действительно есть за что зацепиться без выдумывания нового публичного API (что и зафиксировано как предположение — «отдельный публичный settlement API для viewport не вводится»). +- **Не-скоуп корректно исключает** переписывание `live-viewport`/mode-transition и не создаёт нового touch-поведения — совпадает с фактическим распределением кода (`_refitView`/`_modeTransitionBusy` живут в `houseplan-card.ts`, а не в `live-viewport.ts`, который ТЗ обещает не трогать). +- **Три смока, названные в AC3, — исчерпывающий список**: `grep -rl "settleLive" demo/smoke_*.mjs` называет ровно `smoke_furniture.mjs`, `smoke_decor.mjs`, `smoke_decor_text.mjs` — совпадает с ТЗ, зазора по смежным смокам с той же лотереей нет. +- **Продуктовые вопросы не переданы техническими под видом продуктовых** и наоборот: единственные решения, принятые автором самостоятельно («предположено, можно менять») — где живёт ревизия, остаётся ли контракт внутренним, как синхронизируется viewport в smoke-setup — все технические, ни один не требует ответа персоны «что видит/делает». Открытых продуктовых вопросов, которые нужно было бы вынести владельцу, не найдено. +- **Трек**: комментарий владельца корректно снял `trivial` при обнаружении, что правка касается двух продуктовых модулей (нарушение критерия «одна поверхность»), и задача ушла в полноценный ТЗ-цикл — это ранняя диагностика, а не провал, как и описывает §5.1. +- **Откат, модель данных/миграция, i18n** — тривиальны и корректно описаны как «не требуется»/«один revert», без изобретённых сущностей. +- Индекс `docs/specs/README.md:183` действительно содержит строку на новый файл ТЗ — ссылка issue ↔ ТЗ на месте в обе стороны. + +## Чего не проверял + +- Гейты `typecheck`/`test`/`build`/`bundle:sync`/`no-new-any`/`check-docs` — не запускал: стадия ТЗ, продуктовый код не менялся, эти команды к этому SHA неприменимы (диф — только `docs/**`). +- Не проверял осуществимость конкретной реализации awaitable-контракта (какой именно API, где именно резолвится промис) — это решается на код-ревью по фактическому диффу, а ТЗ намеренно оставляет это «принятым предположением, которое можно менять». +- Не проверял `scripts/mutation-gate.mjs` на способность реально поймать два названных в AC4 мутанта — мутанта ещё не существует, это будет предметом код-ревью и таблицы «чем краснеет» (§2.7). +- Не выполнял 10-кратный прогон `smoke_furniture.mjs` и не воспроизводил заявленную частоту падений (7/12) — численные данные взяты из issue как есть, без повторной эмпирической проверки; это не требуется для ревью ТЗ и станет предметом код-ревью через AC3. + +## Вердикт + +Вердикт: зелёный · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 0 + +--- + + + +## Материал раунда + +- Ветка: `issue/460-live-editor-settle`, коммит `3a566a012113` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `45247a6636567f656dd15bdf83793d4b43f819f9` + ``` + git log --all --format='%H %T' | grep 45247a663656 + ``` +- ТЗ `docs/specs/460-live-editor-settlement.md`, блоб `0ab24688d42076145f7a7f69e486d76cb5daa04a` + ``` + git log --all --find-object=0ab24688d42076145f7a7f69e486d76cb5daa04a -- docs/specs/460-live-editor-settlement.md + ```