mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
committed by
Sergey Matyunin
Sergey Matyunin
parent
ca2a00231b
commit
570da3dd1a
@@ -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
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `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
|
||||
```
|
||||
Reference in New Issue
Block a user