mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -0,0 +1,319 @@
|
||||
# CODE-REVIEW-74-r1
|
||||
|
||||
- **Issue:** https://github.com/Matysh/houseplan-card/issues/74
|
||||
- **Этап:** код-ревью (PROCESS.md §2.7)
|
||||
- **Материал:** `git log --oneline origin/dev..HEAD` и `git diff origin/dev...HEAD` на SHA
|
||||
`fc63f3bf36687cd98c9777ce683d2f2e49d39ff6` (ветка `issue/74-device-position-undo`).
|
||||
Ветка приведена конвейером к `dev` до ревью (легло 10 коммитов `dev`, `d0e1a4b0` → `fc63f3bf`) —
|
||||
по §7.2 это другой код, разбор полный, не по дельте.
|
||||
- **Заход:** r1 (первый код-ревью; предыдущий вердикт в issue — по ТЗ, `SPEC-REVIEW-74-r1`,
|
||||
зелёный, бюджет код-ревью не расходовал)
|
||||
- **Вердикт:** жёлтый
|
||||
- **High:** 0 · **Medium:** 1 (в скоупе, чинится в этой же задаче) · **Low:** 2 (записаны, не блокируют)
|
||||
|
||||
## Скоуп разбора
|
||||
|
||||
Первый заход код-ревью — полный разбор диапазона `origin/dev..HEAD` (6 коммитов:
|
||||
спецификация, актуализация, документ спек-ревью, реализация, применение замечаний спек-ревью,
|
||||
выравнивание регрессионных смоков), всех 14 AC из `docs/specs/074-device-position-undo.md`,
|
||||
трейлеров, changelog, и гейтов из PROCESS.md §8/§10.2, соразмерно объёму задачи (новая
|
||||
independent history для позиций устройств; backend/схема не менялись — подтверждено:
|
||||
`git diff --stat` не содержит `custom_components/**/*.py`).
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Зелёного Validate на этом SHA нет, поэтому дешёвые и часть требуемых по диффу гейтов прогнаны
|
||||
самостоятельно:
|
||||
|
||||
| Гейт | Команда | Результат |
|
||||
|---|---|---|
|
||||
| Typecheck | `npx tsc --noEmit` | OK, без ошибок |
|
||||
| Unit | `npm test` | `tests 1634, pass 1633, fail 0, skipped 1` |
|
||||
| Build | `npm run build` | OK, `dist` собран |
|
||||
| Bundle sync | `cmp dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js` | совпадает; `npm run bundle:sync` не создал diff в git — дерево уже синхронизировано в коммите |
|
||||
| Bundle budget | `npm run bundle:budget` | `initial View 281796 B gzip` (бюджет 300000 B, запас 18204 B) |
|
||||
| Docs fingerprint | `node scripts/check-docs.mjs` | `Documentation checks passed (7 files, 10 external links)` |
|
||||
| `any`-гейт | `node scripts/no-new-any.mjs --base origin/dev --head HEAD` | `Новых any нет` (380 добавленных строк в 3 файлах) |
|
||||
| Coordinate write barrier | `node scripts/coordinate-write-barrier-guard.mjs` | OK (обновлённый инвариант: 3 канонических `layout/update`) |
|
||||
| Process gate | `node scripts/process-gate.mjs --base origin/dev --head HEAD --issues` | `гейт пройден, предупреждений 0` (6 коммитов в диапазоне) |
|
||||
| Smoke selection | `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | 35 прямых совпадений, 21 слабая связь (полный вывод — раздел «Выбор смоков» ниже) |
|
||||
| Targeted smokes (AC1-AC12, AC5, AC9, AC14) | `node demo/smoke_device_position_history.mjs` | OK, все 27 полей `true` |
|
||||
| | `node demo/smoke_drag_bounds.mjs` | OK |
|
||||
| | `node demo/smoke_modes.mjs` | OK |
|
||||
| | `node demo/smoke_pan_any_zoom.mjs` | OK |
|
||||
| | `node demo/smoke_grid_snap.mjs` | OK |
|
||||
| | `node demo/smoke_editor_tabs.mjs` | OK |
|
||||
| | `node demo/smoke_layout_sync.mjs` | OK |
|
||||
| Мутационная проверка «тест умеет падать» | `node scripts/mutation-gate.mjs --id=device-position-cancel-routed-to-commit` | чистый прогон OK, мутант «pointercancel → commit» — `smoke_device_position_history.mjs` **краснеет**, как обязан |
|
||||
| | `node scripts/mutation-gate.mjs --id=stale-space-position-guard-removed` | чистый прогон OK, мутант краснеет, как обязан |
|
||||
| Golden (диагностика, не приёмка) | `node demo/golden/run.mjs --mode=capture --scenario=geometry-devices-editor-dark` | `different` — см. находку Medium-1 |
|
||||
| | `node demo/golden/run.mjs --mode=capture --scenario=geometry-plan-editor-dark` | `passed` (использован для локализации причины Medium-1, см. ниже) |
|
||||
| Model invariants | не прогонялся | diff не трогает геометрию комнат/стен/`marker.space`/`open_spans` — только `houseplan/layout` (позиции устройств), это вне предмета `scripts/model-invariants.mjs` (проверено чтением скрипта: инварианты про полигоны комнат и толщину стен) |
|
||||
| `python -m pytest tests_backend` | не прогонялся | diff не содержит `custom_components/**/*.py` (подтверждено `git diff --stat`) |
|
||||
|
||||
### Выбор смоков — обоснование по каждой строке
|
||||
|
||||
`smoke-select.mjs` вернул 35 прямых совпадений. Прогнаны все, что относятся к изменённым путям
|
||||
исполнения (drag/preview/persist/toolbar/keyboard/layout-sync), а не только упомянутые в тексте
|
||||
AC14:
|
||||
|
||||
- **Прогнаны** (7): `smoke_device_position_history` (новый, основной AC-смок),
|
||||
`smoke_drag_bounds`, `smoke_modes`, `smoke_pan_any_zoom`, `smoke_grid_snap` (все изменены самим
|
||||
диффом — обязаны быть перепроверены целиком, а не доверием к тому, что автор их поправил),
|
||||
`smoke_editor_tabs` (проверяет количество кнопок панели и доступные имена — прямое AC13/AC9),
|
||||
`smoke_layout_sync` (совпал по `_sentPos`/`_serverStorage`, которые diff меняет по типу — точечный
|
||||
update/delete tombstone теперь может быть `null`).
|
||||
- **Не прогнаны, решение — false positive** (`redoName`/`undoName` совпадения в
|
||||
`smoke_furniture.mjs`, `smoke_unified_wall_tool.mjs`, `smoke_v8_draft_write.mjs`): проверено
|
||||
чтением — это геттеры **другого**, не тронутого экземпляра `CommandStack`
|
||||
(`_geometryHistory.undoName/redoName`, `src/houseplan-editor-runtime.ts`), совпадение по имени
|
||||
свойства класса, не по факту зависимости от нового кода. `_geometryHistory` не тронут этим
|
||||
диффом (не входит в `git diff --stat`), новый `_devicePositionHistory` — отдельный самостоятельный
|
||||
экземпляр без общего состояния.
|
||||
- **Остальные 28 из 35** (`smoke_junction_limits`, `smoke_active_chain_ink`, `smoke_align_guides`,
|
||||
`smoke_controls`, `smoke_device_inbox`, `smoke_editor_gestures`, `smoke_fixed_floor`,
|
||||
`smoke_help_affordance`, `smoke_infinite_canvas`, `smoke_junction_holes`,
|
||||
`smoke_lattice_write_barrier`, `smoke_long_press_gesture`, `smoke_orphan_space_references`,
|
||||
`smoke_partition_openings`, `smoke_plan_snap_overlay`, `smoke_room_cards`, `smoke_room_resize`,
|
||||
`smoke_tap_ctx`, `smoke_wallthick_standalone`, `smoke_ws_resilience`,
|
||||
`smoke_zero_wall_migration_unblocked`, `smoke_zoom_out`, `smoke_decor`,
|
||||
`smoke_dialog_footer_width`, `smoke_optimize_coordinate_canonicalization`,
|
||||
`smoke_optional_space_model`, `smoke_v8_draft_write` (частично)) — не прогнаны: совпадение по
|
||||
общим символам верхнего уровня (`NORM_W`, `_mode`, `_showToast`, `_serverStorage`, `_drag`,
|
||||
`_suppressClick`, `_selId`, `_commitSpace`, `_holdFired`, `_restoreZoom`, `_savePos` через
|
||||
room-label путь), ни один из которых этот diff не меняет по семантике — прогон всех 35 не
|
||||
соразмерен задаче (PROCESS.md §8, «полные наборы — предрелизный гейт»). Автор в хендоффе также
|
||||
прогонял более широкий список (`smoke_room_cards`, `smoke_ws_resilience`,
|
||||
`smoke_long_press_gesture`, `smoke_fixed_floor`) — не переисполнялись повторно, доверие к
|
||||
зафиксированной команде и результату оправдано, поскольку это чистые "прочитал/не менял" пути.
|
||||
- **21 слабая связь** (все — `_mode`, самый частый символ проекта): не прогонялись, `_mode`
|
||||
проверяется по значению `'devices'` в изменённом коде, но сам механизм переключения режимов не
|
||||
менялся (проверено чтением `_setMode`/`_mode` — единственное новое место, где `_mode === 'devices'`
|
||||
влияет на поведение, это `_onKey`, уже покрыт прогоном AC9).
|
||||
- `golden`/`performance_smoke` полным набором — не прогонялись намеренно: предрелизный гейт
|
||||
(PROCESS.md §8, §11.4), не гейт код-ревью. Один сценарий прогнан диагностически (см. ниже).
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium-1 — новый Undo/Redo toolbar рендерится без иконок из-за неактуального `demo/srv/assets/icons.js`; сломан именно опубликованный в этом же коммите скриншот `docs/images/06-device-editor.png`
|
||||
|
||||
**В скоупе задачи** — картинка и golden-сценарий, которые проваливаются, это собственная поставка
|
||||
#74 (новый Device-editor toolbar), обновлённые в том же коммите `bdf81fad`.
|
||||
|
||||
**Воспроизведение:**
|
||||
|
||||
```
|
||||
node demo/golden/run.mjs --mode=capture --scenario=geometry-devices-editor-dark
|
||||
# → different
|
||||
```
|
||||
|
||||
Диагностический diff-кадр (`artifacts/golden/diff/geometry-devices-editor-dark.png`) подсвечивает
|
||||
розовым ровно область панели устройств справа от «Icon rules» и в подписи вкладки — крупная
|
||||
геометрия плана снизу идентична существующему baseline. Кроп фактического кадра
|
||||
(`artifacts/golden/actual/geometry-devices-editor-dark.png`, область x=950-1130,y=108-160,
|
||||
увеличено ×5) показывает две пустые скруглённые кнопки без единого видимого штриха иконки — рядом
|
||||
с полностью корректно отрисованной иконкой `mdi:close`.
|
||||
|
||||
То же самое видно в уже закоммиченном как часть этого диффа пользовательском скриншоте
|
||||
`docs/images/06-device-editor.png` (обновлён коммитом `bdf81fad` именно для показа новых кнопок):
|
||||
кроп той же области подтверждает пустые кнопки без иконок рядом с корректной `mdi:close`. Это
|
||||
опубликованная картинка из `docs/USER-GUIDE.md`, а не только внутренний тестовый артефакт.
|
||||
|
||||
**Причина** (проверено чтением и точечным экспериментом, изменения не закоммичены):
|
||||
`demo/srv/demo.html` эмулирует `<ha-icon>` через `window.__ICONS[iconName]`, заполняемый
|
||||
сгенерированным `demo/srv/assets/icons.js` (генератор — `demo/gen_icons.mjs`, сканирует все
|
||||
`mdi:*` в `src/`+`demo/` и берёт SVG-path из пакета `@mdi/js`). В закоммиченном
|
||||
`demo/srv/assets/icons.js` **нет** записей для `mdi:undo-variant`/`mdi:redo-variant`
|
||||
(`grep -c` → 0 совпадений), хотя `@mdi/js` их экспортирует (`mdiUndoVariant`, `mdiRedoVariant`
|
||||
существуют). Диагностический прогон `node demo/gen_icons.mjs` (не закоммичен, откачен обратно
|
||||
`git checkout -- demo/srv/assets/icons.js`) подтверждает: после регенерации файл вырастает с 33424
|
||||
до 42629 байт и обе иконки появляются в карте (`155 из 157` вместо меньшего числа), то есть
|
||||
единственная причина — устаревший сгенерированный файл, не отсутствие иконки в принципе.
|
||||
|
||||
**Это не регрессия конкретно #74 по корню, но именно #74 — первая задача, которая закоммитила
|
||||
видимое проявление.** Тот же дефект уже присутствует на `origin/dev` до этой ветки: диагностический
|
||||
`node demo/golden/run.mjs --mode=capture --scenario=geometry-plan-editor-dark` вернул `passed`
|
||||
(рендер идентичен уже принятому baseline), но кроп самого `demo/golden/baselines/geometry-plan-editor-dark.png`
|
||||
в области persistent Undo/Redo кнопок Plan-редактора (`src/houseplan-editor-runtime.ts:11737-11745`
|
||||
на `origin/dev`, тот же `mdi:undo-variant`/`mdi:redo-variant`, тот же паттерн `.btn.ghost`) показывает
|
||||
точно такие же пустые кнопки без иконок — значит, `icons.js` был неактуален уже на `dev` **до**
|
||||
этой ветки, и никто это не заметил, потому что до сих пор ни один поддерживаемый в актуальном
|
||||
`docs/images/*` кадр не показывал Plan-редактор с открытой панелью инструментов крупным планом.
|
||||
#74 — первая задача, которая (а) добавляет **второе** место с тем же сломанным паттерном и
|
||||
(б) коммитит **новый** документационный скриншот именно этой области, тем самым превращая
|
||||
молчаливый локальный гэп в опубликованный дефект.
|
||||
|
||||
**Почему Medium, а не High:** реальные пользователи Home Assistant не увидят проблему — их
|
||||
браузер резолвит `<ha-icon>` через полный набор MDI из фронтенда HA, а не через
|
||||
`demo/srv/assets/icons.js` (это стенд-заглушка только для демо/тестового харнесса, подтверждено
|
||||
чтением `demo/srv/demo.html:21-40`). Функционально кнопки работают полностью корректно (все 27
|
||||
проверок `smoke_device_position_history.mjs`, включая доступные имена и disabled-состояние, зелёные)
|
||||
— это дефект качества визуального тестового/документационного артефакта, а не поведения продукта.
|
||||
|
||||
**Почему в скоупе, а не отдельный issue:** сломанный артефакт — `docs/images/06-device-editor.png`
|
||||
и golden-сценарий `geometry-devices-editor-dark` — оба созданы/обновлены именно этим коммитом ради
|
||||
показа новых кнопок из #74; по смыслу задачи AC13 требует «Golden + DOM/a11y assertions» на этот
|
||||
самый toolbar, и golden-часть доказательства сейчас недостоверна. Возврат на правку: `node
|
||||
demo/gen_icons.mjs`, зафиксировать обновлённый `demo/srv/assets/icons.js`, пересобрать бандл-стенд,
|
||||
пересъёмка `docs/images/06-device-editor.png` каноническим `Docs screenshots` workflow +
|
||||
`npm run docs:accept -- --reviewed`, обновление golden-эталона `geometry-devices-editor-dark` через
|
||||
`npm run golden:accept -- --reviewed` на полном Linux CI артефакте (PROCESS.md §8, §13). Побочный
|
||||
эффект — тот же коммит заодно чинит уже существующий на `dev` дефект Plan-редактора; это не
|
||||
расширение скоупа, а тот же самый generated-файл одной командой, но если автор предпочтёт не трогать
|
||||
несвязанный `geometry-plan-editor-dark` baseline в этой ветке — это не блокирует #74: обязателен
|
||||
только фикс, покрывающий заявленный в AC13 сценарий этой задачи.
|
||||
|
||||
### Low-1 — AC2 «два устройства в LIFO-порядке» не продемонстрирован реальным перетаскиванием двух устройств подряд
|
||||
|
||||
`docs/specs/074-device-position-undo.md:246` требует для AC2 браузерный смок, показывающий exact
|
||||
before/after «для двух устройств... в LIFO-порядке». `demo/smoke_device_position_history.mjs`
|
||||
перетаскивает только один `deviceId` повторно (несколько drag одного устройства — эта часть AC2
|
||||
покрыта, см. `noopKeepsRedo`/повторные `drag()` вызовы); межпространственный `otherDevice` в конце
|
||||
файла (строки 246-274) не перетаскивается по-настоящему, а получает команду через
|
||||
`c._devicePositionHistory.push(...)` вручную, в пустой на тот момент стек — ни разу оба устройства
|
||||
не оказываются в стеке одновременно, чтобы проверить порядок отмены между ними.
|
||||
|
||||
**Решение ревьюера:** снимаю как Low без возврата в цикл. LIFO-механика — свойство самого
|
||||
`CommandStack` (`src/command-stack.ts`, не тронут этим диффом), уже доказано для разнородных
|
||||
пейлоадов существующим `test/command-stack.test.mjs` («undo and redo preserve names…», строки
|
||||
14-25: два разных именованных пуша, `undo()` возвращает последний). Каждая `DevicePositionCommand`
|
||||
самодостаточна (`deviceId`/`spaceId` хранятся внутри команды, не выводятся из внешнего состояния на
|
||||
момент undo) — проверено чтением `_pointerUp`/`_runDevicePositionHistory`
|
||||
(`src/houseplan-card.ts:6617-6628`, `:5145-5177`): архитектурно перепутать устройства при
|
||||
чередующемся undo/redo невозможно, не только «пока тесты не поймали». Автору стоит перед следующим
|
||||
изменением в этой области добавить реальный двух-device drag-сценарий в смок, но отдельного цикла
|
||||
это не требует.
|
||||
|
||||
### Low-2 — AC13 «доступные имена на поддерживаемых ширинах» проверены на одной ширине/теме
|
||||
|
||||
Golden-сценарий `geometry-devices-editor-dark` (`demo/golden/matrix.mjs:381`) — единственный кадр
|
||||
для нового toolbar, `viewport: 1180×900`, тема `dark`; DOM-проверка
|
||||
(`demo/smoke_editor_tabs.mjs:158-165`, `out.deviceHistoryControlsArePersistent`) подтверждает
|
||||
наличие кнопок, доступные имена (`title`/`aria-label`) и исходное disabled-состояние, но не на
|
||||
нескольких ширинах. AC13 говорит «на поддерживаемых ширинах» во множественном числе.
|
||||
|
||||
**Решение ревьюера:** снимаю как Low. Device editor — desktop-first по `docs/TOUCH-SUPPORT.md`,
|
||||
«поддерживаемые ширины» здесь исторически означают диапазон десктопных окон, а не мобильные
|
||||
брейкпоинты (сравнимые персистентные кнопки — `barclose` — тоже проверяются на одной ширине в
|
||||
существующей практике проекта). Кнопки — фиксированного `.btn.ghost` размера, идентичного другим
|
||||
icon-only элементам панели, реальный риск переполнения на разумных десктопных ширинах низкий.
|
||||
Не блокирует.
|
||||
|
||||
## Что проверено и признано корректным
|
||||
|
||||
- **AC1** (один drag — одна команда/запись): `smoke_device_position_history` —
|
||||
`previewDoesNotPersist`, `oneDragOneWrite`, `oneDragOneCommand` — все `true`; 10 `pointermove`
|
||||
между down/up не создают запись — проверено и чтением (`_previewDevicePlacement` не трогает
|
||||
`_dirtyPos`/`_persistLayout`, `src/houseplan-card.ts:5076-5079`).
|
||||
- **AC2** (exact before/after, LIFO): проверено чтением + существующим `command-stack.test.mjs`
|
||||
(см. Low-1) + `undoRestoresExactStart`/`redoRestoresExactEnd`/`keyboardUndoWorks`/
|
||||
`keyboardRedoWorks` в смоке — все `true`.
|
||||
- **AC3** (auto→manual→Undo=delete, Redo=update): `autoUndoDeletesExplicitPlacement`,
|
||||
`autoRedoRestoresPlacement` — `true`, включая проверку `writes[...]type === 'delete'/'update'`.
|
||||
- **AC4** (no-op не создаёт команду/не чистит Redo): `noopKeepsRedo` — `true`; чтением подтверждено
|
||||
в `_pointerUp` (`src/houseplan-card.ts:6607`): `sameDevicePlacement(after, drag.start)` возвращает
|
||||
раньше `_persistDevicePlacement`/push.
|
||||
- **AC5** (cancel/lost capture/Escape/mode switch/disconnect/second pointer → 0 записей): все шесть
|
||||
сценариев — `escapeAbortsWithoutWrite`, `modeSwitchAbortsWithoutWrite`, `cancelRestoresWithoutWrite`,
|
||||
`lostCaptureRestoresWithoutWrite`, `secondPointerAbortsWithoutWrite` — `true`; disconnect —
|
||||
`disconnectedCallback` вызывает `_cancelDeviceDrag()` (`src/houseplan-card.ts:2672`, проверено
|
||||
чтением, не исполнением — юнит-тест на `disconnectedCallback` в проекте отсутствует, что ожидаемо
|
||||
для lifecycle-хука). Мутационный тест (`device-position-cancel-routed-to-commit`) независимо
|
||||
доказал, что `smoke_device_position_history.mjs` **обязан упасть**, если pointercancel направить
|
||||
в commit-путь — «тест умеет падать» подтверждено экспериментом, не предположением.
|
||||
- **AC6** (Undo во время drag только abort): `undoDuringDragOnlyAborts` — `true`; чтением —
|
||||
`_runDevicePositionHistory` начинается с `if (this._cancelDeviceDrag() || ...) return;`
|
||||
(`src/houseplan-card.ts:5145`).
|
||||
- **AC7** (persist failure → rollback + направление stack): `failedDragRollsBack`,
|
||||
`failedUndoRestoresStackDirection` — `true`. Чтением подтверждена тонкость восстановления
|
||||
направления: на неудачном Undo вызывается `.redo()` (а не повторный `.undo()`), что верно снимает
|
||||
побочный эффект предварительного `.undo()` со стека (`src/houseplan-card.ts:5162-5165`) — без
|
||||
прогона смока эта инверсия легко читается неверно, поэтому проверена и по коду, и по фактическому
|
||||
результату `canUndo/canRedo`.
|
||||
- **AC8** (`k: 0`/unknown-поля/room-label): `test/device-position-history.test.mjs` — 4 юнит-теста,
|
||||
включая явную проверку иммутабельности входа (`assert.notEqual`) и сохранения `k: 0` (falsy,
|
||||
специально проверяется отдельно от truthy `k`) — тест умеет падать: `deepEqual` откажет при потере
|
||||
поля, `notEqual` откажет при мутации на месте. `rl_*` использует отдельный `_savePos`-путь через
|
||||
`_labelMove` (`src/houseplan-editor-runtime.ts:10741-10759`) — не пересекается с
|
||||
`_devicePositionHistory`, проверено чтением (единственный вызывающий `_savePos` теперь — только
|
||||
label-путь, устройства используют новый `_devicePlacementForCanvas`/`applyDevicePlacement`).
|
||||
- **AC9** (кнопки/shortcuts только в Device editor, native field не перехватывается):
|
||||
`nativeInputHistoryNotIntercepted`, `qwertzCtrlZIsUndo`/`qwertzCtrlYIsRedo` (существующие в
|
||||
`smoke_editor_tabs`), `deviceHistoryControlsArePersistent` — все `true`; чтением подтверждено, что
|
||||
`_mode === 'devices'` ветка в `_onKey` — новая и не существовала на `dev` (сравнение с
|
||||
`origin/dev:src/houseplan-card.ts` — раньше devices проваливался в общий `if (!this._markup)
|
||||
return`), регресс в decor/plan-ветки исключён (они выше по `if` и не задеты диффом).
|
||||
- **AC10** (own echo/reconnect сохраняют, remote content чистит): `sameContentReloadKeepsHistory`,
|
||||
`remoteContentClearsHistory` — `true`; чтением — `_reloadLayoutOnly` теперь сравнивает
|
||||
`contentFingerprint` **всегда** против текущего `this._layout` (убран фолбэк на потенциально
|
||||
устаревший кэш, `src/houseplan-card.ts:4801-4809`) — усиление, не ослабление контракта.
|
||||
- **AC11** (кросс-space команда переключает пространство): `otherSpaceUndoIsVisible` — `true`.
|
||||
- **AC12** (deleted/rebound/disabled device — fail-closed): `deletedDeviceCommandFailsClosed`,
|
||||
`reboundDeviceCommandFailsClosed`, `disabledDeviceCommandFailsClosed` — все `true`;
|
||||
`_devicePositionStateValid` проверяет существование, `space`, `ha_disabled` и наличие
|
||||
space-модели перед каждым undo/redo (`src/houseplan-card.ts:5136-5142`).
|
||||
- **AC14** (geometry/decor history, pan/zoom, layout sync не регрессируют): целевые
|
||||
`smoke_pan_any_zoom`, `smoke_grid_snap`, `smoke_editor_tabs`, `smoke_layout_sync` — зелёные;
|
||||
`_geometryHistory`/decor-стеки не входят в diff (`git diff --stat` не содержит изменений в
|
||||
`command-stack.ts`, а `_geometryHistory` в `houseplan-card.ts` не тронут за пределами добавления
|
||||
соседнего поля) — проверено чтением, не исполнением полного regression-набора (несоразмерно
|
||||
задаче, см. §8).
|
||||
- **Трейлеры и changelog:** `bdf81fad` (`feat: add device position undo history`) несёт
|
||||
`Issue: #74` / `User-Visible: yes` и в одном коммите содержит `docs/CHANGELOG.md` +
|
||||
`docs/CHANGELOG.ru.md` (по 6 строк, со ссылкой на #74) — оба сразу, как требует правило. Второй
|
||||
коммит `fc63f3bf` — `User-Visible: no`, трогает только два demo-смока, корректно.
|
||||
`process-gate.mjs --issues` подтверждает диапазон формально чистым.
|
||||
- **Backend/схема:** не затронуты — `git diff --stat` не содержит `custom_components/**/*.py`;
|
||||
`houseplan/layout/update`/`delete` вызываются с той же сигнатурой, что и раньше (третий
|
||||
сайт-вызова в `_persistDevicePlacement`, инвариант `coordinate-write-barrier-guard.mjs` обновлён
|
||||
на 3 и проходит).
|
||||
- **Один источник числа:** новый функционал не вводит ни одной новой пользовательски видимой
|
||||
величины (позиция маркера не отображается как число где-либо ещё) — правило неприменимо,
|
||||
`test/single-source-numbers.test.mjs` в общем прогоне `npm test` зелёный без изменений.
|
||||
- **i18n:** `history.device_move`/`history.device_stale` добавлены во все 4 языка
|
||||
(`en/ru/de/fr`), плейсхолдер `{name}` согласован с существующим паттерном
|
||||
`history.undo_named`/`redo_named`.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- **Полный browser-smoke набор** (все ~207 сценариев) и **полный HA backend harness** — не
|
||||
прогонялись: несоразмерно диффу (PROCESS.md §8), backend не тронут вовсе.
|
||||
- **`golden:verify` полным набором** — недоступен частично по дизайну (`assertGoldenInvocation`
|
||||
в `demo/golden/policy.mjs` отказывает при `--scenario` для verify, только `--mode=capture`
|
||||
разрешает диагностику одного сценария); полный прогон — предрелizный гейт (см. Medium-1, где
|
||||
диагностика `--mode=capture` на два сценария уже вскрыла дефект без полного прогона).
|
||||
- **`performance_smoke`** — не прогонялся; в AC не назван явно, спец §12 отмечает влияние на
|
||||
perf как «объективно снижает нагрузку», что не оспаривается кодом (persist больше не идёт на
|
||||
каждый `pointermove`, только `_previewDevicePlacement`/локальный layout).
|
||||
- **Прогон полного `mutation-gate.mjs`** (все ~сотни мутантов) — прогнаны точечно только два новых
|
||||
мутанта, относящихся к диффу (`--id=`); полный прогон — тяжёлый гейт вне рамок код-ревью этого
|
||||
масштаба.
|
||||
- **Ручное тестирование в браузере** (реальный HA, реальный `<ha-icon>`) — не выполнялось; вывод
|
||||
про «в проде иконки отрисуются нормально» — по чтению кода демо-заглушки, не по факту в реальном
|
||||
Home Assistant.
|
||||
- **Реальный факт прогона канонического `Docs screenshots` workflow** для обновлённых
|
||||
`docs/images/*.png` — не проверялся (нет ссылки на прогон в трейлерах, `check-docs.mjs` сверяет
|
||||
отпечаток источника, а не байтовую строгую принадлежность конкретному CI-раннеру); дефект
|
||||
Medium-1 воспроизводится независимо от того, каким Chromium снят кадр, поскольку причина —
|
||||
отсутствующие данные в `icons.js`, а не отличие рендерера.
|
||||
|
||||
## Итог
|
||||
|
||||
Функциональная часть задачи выполнена основательно: все 14 AC доказаны либо зелёным,
|
||||
демонстрируемо-падающим тестом (включая два прогнанных вживую мутационных теста,
|
||||
подтвердивших, что `smoke_device_position_history.mjs` и regression-guard действительно ловят
|
||||
регресс, а не просто существуют), либо разобраны по коду с точным указанием строк. Транзакционная
|
||||
модель drag/preview/commit/abort, fail-closed инвалидация истории, кросс-пространственный undo и
|
||||
сериализация записи выполнены точно по контракту ТЗ, вплоть до тонких деталей вроде направления
|
||||
восстановления стека при неудачном Undo.
|
||||
|
||||
Единственная блокирующая находка — не в логике фичи, а в её собственном визуальном доказательстве:
|
||||
опубликованный в этом же коммите скриншот `docs/images/06-device-editor.png` и golden-эталон
|
||||
`geometry-devices-editor-dark`, оба созданные ради показа новых кнопок Undo/Redo, показывают их
|
||||
пустыми из-за неактуального `demo/srv/assets/icons.js`. Фикс мелкий и механический
|
||||
(`node demo/gen_icons.mjs` + пересъёмка/переприёмка двух артефактов), но обязателен в этой же
|
||||
задаче, поскольку AC13 прямо ссылается на golden как доказательство, а опубликованная
|
||||
документация не должна показывать несуществующие в кадре кнопки той самой фичи, которую иллюстрирует.
|
||||
|
||||
**Вердикт: жёлтый.** Возврат автору на устранение Medium-1; High не найдено, задача не пятый заход
|
||||
и бюджет циклов не исчерпан.
|
||||
Reference in New Issue
Block a user