mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -0,0 +1,231 @@
|
||||
# SPEC-REVIEW-613-r1
|
||||
|
||||
Материал: тело issue #613 (первая редакция, комментарий-аналитика от Matysh
|
||||
2026-09-23T02:02:47Z уже приложен, ТЗ после него не менялся). Дерево кода на
|
||||
момент проверки — `1241b9499c7b33d4a4b905d884f60ae6db74690c` (`dev`), читано
|
||||
только для проверки утверждений ТЗ о существующем коде, не как материал
|
||||
код-ревью.
|
||||
|
||||
Заход r1, блокирующих циклов израсходовано 0 из 4 (ещё не потрачены — это
|
||||
первый заход, полный трек).
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Issue #613: дефект #1 (устаревание hit-индекса устройств после прокрутки
|
||||
дашборда/страницы) и дефект #2 (`_saveZoom()` синхронно пишет `localStorage`
|
||||
на каждый `pointermove` обоих pinch-путей). ТЗ полного трека, тело issue,
|
||||
раздел `## ТЗ`. Проверялись: наличие обязательных разделов §7.1, однозначность
|
||||
и доказуемость AC1…AC6, соответствие описания текущему коду
|
||||
(`src/device-hit-owner.ts`, `src/houseplan-card.ts`), соответствие
|
||||
`docs/SCOPE.md` (J1/J3) и `docs/TOUCH-SUPPORT.md` (View/kiosk —
|
||||
release-blocking), а также техническая реализуемость заявленного подхода к
|
||||
scroll-инвалидации.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
- Прочитан `docs/SCOPE.md`, релевантные разделы `PROCESS.md` (§1–§9, лимит
|
||||
циклов §4, лёгкий трек §5, шаблоны §7.2) и `docs/TOUCH-SUPPORT.md` целиком.
|
||||
- Прочитано тело issue #613 и единственный комментарий (аналитика).
|
||||
- Прочитан `src/device-hit-owner.ts` целиком (288 строк) — подтверждён
|
||||
механизм `DeviceHitIndex`/`DeviceHitController`/`DevicePointerOwnerLatch`,
|
||||
на который ссылается ТЗ (арбитраж #564).
|
||||
- В `src/houseplan-card.ts` найдены и прочитаны оба pinch-пути: обычный stage
|
||||
(`_stagePointerMove` / `_stagePointerUp` / `_stagePointerCancel`, строки
|
||||
~6903–7113, 7964–7981) и capture-guard для жеста, начатого над устройством
|
||||
(`_guardTouchGesture`, строки ~7329–7413) — подтверждено, что `_saveZoom()`
|
||||
(строка 6968 и 7388) вызывается на каждый `pointermove` в обоих путях и что
|
||||
ни один терминальный обработчик (`pointerup`/`pointercancel`/
|
||||
`lostpointercapture`) сейчас не делает единственную финальную запись.
|
||||
- Поиском по `src/houseplan-card.ts` подтверждено отсутствие любого текущего
|
||||
`scroll`/`visualViewport` слушателя — инвалидация индекса сейчас идёт только
|
||||
через `ResizeObserver(.stage)` и явные вызовы `_invalidateDeviceHitGeometry()`
|
||||
при изменении геометрии устройств. Заявление ТЗ «сейчас он инвалидируется при
|
||||
ResizeObserver... но не при прокрутке» подтверждено кодом, не домыслено.
|
||||
- Подтверждено существование `_deviceHitOwnerAt` (строка 5718) и обоих
|
||||
smoke-файлов, которые ТЗ предлагает расширить: `demo/smoke_device_hit_capsules.mjs`,
|
||||
`demo/smoke_editor_gestures.mjs` — план автотестов ссылается на реальные
|
||||
файлы, а не на выдуманные имена.
|
||||
- Найдено на будущее свойство: `connectedCallback`/`disconnectedCallback` в
|
||||
`src/houseplan-card.ts` (2672, 2780) существуют и уже несут прецедент
|
||||
«таймеры/подписки умирают на disconnect» (комментарий AUD-1552-01) — AC2
|
||||
(lifecycle слушателей) реализуем на существующем паттерне.
|
||||
- **Ключевая проверка — техническая реализуемость принятого предположения
|
||||
«capture-listener `scroll` на `ownerDocument`».** Эмпирически проверено в
|
||||
реальном Chromium проекта (`playwright`, тот же движок, что и в
|
||||
`demo/serve.mjs`): скролл элемента внутри `shadowRoot` **не долетает** до
|
||||
`document`-уровневого слушателя даже с `{capture: true}` — см. находку H1
|
||||
ниже, вывод теста приведён там же дословно.
|
||||
- Проверено по коду: в проекте уже есть прецедент подъёма по цепочке теневых
|
||||
хостов через `getRootNode()` (`src/hp-dialog.ts:329,464-465`,
|
||||
`src/hp-zigbee-topology-overlay.ts:80,165`,
|
||||
`src/summary-panel-runtime-loaded.ts:396`) — то есть команда уже решала
|
||||
именно эту категорию проблемы в других местах кода, просто не в этом ТЗ.
|
||||
- Автотесты/гейты в этом раунде не гонялись: на стадии `spec` не существует
|
||||
ни кода, ни коммитов для проверки — гонять `tsc`/`test`/`build` не на чем.
|
||||
Единственная фактическая проверка — приведённый выше playwright-эксперимент,
|
||||
подтверждающий конкретное утверждение о поведении браузера.
|
||||
|
||||
## Находки
|
||||
|
||||
### H1 — High. Принятое предположение о scroll-инвалидации не работает в основном заявленном сценарии (HA-дашборд), и это не покрыто ни одним AC
|
||||
|
||||
**Файл:** тело issue #613, разделы «Принятые технические предположения» и
|
||||
«Риски».
|
||||
|
||||
**Сценарий отказа:** Раздел «Принятые технические предположения» выбирает:
|
||||
«Использовать capture-listener `scroll` на `ownerDocument` плюс
|
||||
`visualViewport.scroll/resize`, а не подписываться на динамический список
|
||||
предков». Раздел «Риски» тут же, тремя строками выше, называет ровно
|
||||
противоположный факт: «Прокрутка может происходить внутри shadow DOM/
|
||||
HA-контейнера, поэтому слушатель только на `window` не покрывает все случаи» —
|
||||
но противоречие не разрешается: assumption выбирает именно ту схему, которую
|
||||
risk описывает как недостаточную, и не говорит, почему это приемлемо или как
|
||||
будет проверено.
|
||||
|
||||
Это не гипотетическое опасение. Проверено экспериментально в реальном
|
||||
Chromium (том же движке, что использует `demo/serve.mjs`/`playwright`):
|
||||
|
||||
```js
|
||||
const host = document.getElementById('host');
|
||||
const root = host.attachShadow({ mode: 'open' });
|
||||
root.innerHTML = '<div id="scroller" style="overflow:auto;height:100px">'
|
||||
+ '<div style="height:1000px"></div></div>';
|
||||
document.addEventListener('scroll', () => window.__docScrollSeen++, true);
|
||||
// прокрутка #scroller внутри теневого корня:
|
||||
root.getElementById('scroller').scrollTop = 50;
|
||||
// → window.__docScrollSeen остаётся 0
|
||||
```
|
||||
|
||||
Результат: `{ doc: 0, win: undefined }` — `document`/`window` capture-слушатель
|
||||
**не видит** scroll, произошедший на элементе внутри отдельного
|
||||
`shadowRoot`, даже с `capture: true`. Причина: событие `scroll` не
|
||||
`composed`, поэтому его путь диспетчеризации не покидает теневое дерево, в
|
||||
котором оно возникло, — не помогает ни capture, ни bubble.
|
||||
|
||||
Сценарий issue («карточка занимает часть прокручиваемого дашборда Home
|
||||
Assistant») — это именно многоуровневая Shadow DOM структура
|
||||
(`home-assistant` → `home-assistant-main` → `ha-panel-lovelace` → `hui-view` и
|
||||
т.д., каждый уровень — свой `shadowRoot`), и сама кодовая база это признаёт
|
||||
косвенно (в ней уже есть прецеденты подъёма по `getRootNode()` именно для
|
||||
обхода этой границы: `src/hp-dialog.ts:329,464-465`,
|
||||
`src/hp-zigbee-topology-overlay.ts:80,165`). Если реальный скроллящийся
|
||||
предок дашборда находится в чужом (не `document`) теневом дереве — что для HA
|
||||
Lovelace типично, — выбранный слушатель на `ownerDocument` его не увидит, и
|
||||
дефект №1 («после прокрутки тап переключает соседнее устройство») в реальном
|
||||
эксплуатационном сценарии останется неисправленным.
|
||||
|
||||
**Почему это блокирует, а не «инженерная деталь на усмотрение реализации».**
|
||||
AC1 и AC2 доказываются smoke-тестом на «прокручиваемой странице/контейнере»
|
||||
без указания, что этот контейнер обязан быть за пределами `document`-дерева
|
||||
самой карточки (внутри отдельного `shadowRoot`, как в реальном HA). Смок,
|
||||
построенный на простом `<div style="overflow:auto">` в light DOM, зазеленеет
|
||||
и «докажет» AC1/AC2 — но не докажет исправление сценария, ради которого issue
|
||||
заведён: пользователь листает Lovelace-дашборд HA, где скролл-контейнер
|
||||
почти наверняка находится в стороннем теневом дереве. Это ровно тот случай,
|
||||
когда «AC формально выполнены» и «сценарий не решён» расходятся (жёлтый
|
||||
вердикт допустим и при выполненных AC).
|
||||
|
||||
**Что нужно от автора при возврате:** либо (а) заменить/дополнить механизм
|
||||
инвалидации на способ, не зависящий от `composed`-пути `scroll` — например,
|
||||
подъём по реальной цепочке скроллящихся предков через `getRootNode()`
|
||||
(прецедент уже есть в проекте) или наблюдение за геометрией независимо от
|
||||
scroll-событий (`IntersectionObserver`, который штатно пересекает границы
|
||||
теневых деревьев), — либо (б) явно ограничить AC1/AC2 договорённой
|
||||
доказуемой моделью и вынести недостающее покрытие в названный, а не молчаливый
|
||||
риск с решением, что с ним делать. Раздел «Риски» такой решённости сейчас не
|
||||
даёт — он просто называет факт и переходит к следующему пункту.
|
||||
|
||||
### M1 — Medium (в скоупе), сопутствует H1. AC1/AC2 не требуют, чтобы smoke-контейнер пересекал границу shadow DOM
|
||||
|
||||
**Файл:** тело issue #613, раздел «Критерии приёмки», AC1 и AC2; раздел «План
|
||||
автотестов».
|
||||
|
||||
Даже если H1 будет закрыт правильным техническим решением, формулировка
|
||||
AC1 («два реальных маркера... в прокручиваемой странице/контейнере») и AC2
|
||||
не требуют, чтобы хотя бы один тестовый сценарий воспроизводил скролл именно
|
||||
**внутри стороннего `shadowRoot`**, имитирующего реальное вложение карточки в
|
||||
Home Assistant. Без этого требования разработчик может закрыть AC на light-DOM
|
||||
контейнере, интуитивно решить, что «прокрутка есть — тест прошёл», и не
|
||||
заметить, что боевой сценарий не покрыт. Это отдельная от H1 находка, потому
|
||||
что даже правильная реализация нуждается в тесте, который умеет упасть именно
|
||||
на этом классе регрессии — иначе AC6 (мутационная защита) будет мутировать
|
||||
код, который сам тест не отличает от рабочего в реалистичном вложении.
|
||||
|
||||
Правится вместе с H1 в этом же цикле: формулировка AC1/AC2 должна явно
|
||||
называть, что хотя бы один прогон использует scroll-контейнер за пределами
|
||||
`ownerDocument` дерева карточки (внутри отдельного `attachShadow`), а не
|
||||
только «прокручиваемую страницу».
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Все обязательные разделы §7.1 присутствуют: сценарий, что человек увидит до
|
||||
и после, проблема, скоуп/не-скоуп, контракт поведения (7 пунктов), UX,
|
||||
модель данных и миграция, i18n, AC1…AC6 с указанием способа доказательства,
|
||||
план автотестов, риски, откат, release-артефакты.
|
||||
- Оба технических утверждения о текущем поведении кода (два pinch-пути пишут
|
||||
`_saveZoom()` на каждый `pointermove`; индекс не инвалидируется на scroll)
|
||||
подтверждены чтением `src/device-hit-owner.ts` и `src/houseplan-card.ts`, а
|
||||
не приняты на слово автора.
|
||||
- Ссылка на арбитраж #564 (латч владельца pointer-последовательности) —
|
||||
подтверждена структурой `DevicePointerOwnerLatch` в `device-hit-owner.ts`;
|
||||
контракт «прокрутка не меняет владельца уже начатой последовательности»
|
||||
реализуем на существующем механизме без структурных изменений.
|
||||
- AC1–AC6 однозначны и каждый называет способ доказательства (`smoke`,
|
||||
`unit/contract + smoke`, `mutation`); AC6 корректно требует двух раздельных
|
||||
мутантов на две независимые защиты (scroll-инвалидация и terminal-only
|
||||
persistence), что соответствует §2.7 (таблица «чем краснеет»).
|
||||
- Не-скоуп сформулирован явно и исключает ровно те смежные контракты (#563,
|
||||
#578, размеры капсул, порядок перекрытий, рендер), которые действительно не
|
||||
должны меняться этой задачей.
|
||||
- «Принятые технические предположения» оформлены отдельным блоком с пометкой
|
||||
«можно поменять свободно» — процессуально корректно; H1 — это ровно
|
||||
реализация права ревьюера оспорить такое предположение (§7.1), а не
|
||||
претензия к оформлению.
|
||||
- Продуктовых вопросов к владельцу нет и не требуется: всё найденное —
|
||||
техническое, разрешается на этом ревью, не эскалируется.
|
||||
- Track (полный) обоснован верно: две поверхности/подсистемы (hit-index +
|
||||
zoom persistence) и прямое влияние на touch-контракт/perf горячего пути —
|
||||
критерии `small` действительно не выполняются, «лёгкий трек» здесь был бы
|
||||
неверным выбором.
|
||||
- Влияние на i18n, миграцию, откат — корректно и полно названы как «нет»/
|
||||
«revert коммита», без домыслов.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не запускал `tsc`/`test`/`build`/смоки — на стадии ТЗ нет кода для прогона;
|
||||
единственный практический тест в этом раунде — точечный playwright-скрипт,
|
||||
подтверждающий утверждение о поведении `scroll`-событий относительно
|
||||
`shadowRoot` (см. H1), не гейт продукта.
|
||||
- Не проверял реальную DOM-структуру актуальной версии Home Assistant
|
||||
Lovelace (`hui-view`/`ha-panel-lovelace`) вживую — вывод о «типичной
|
||||
многоуровневой Shadow DOM» опирается на общеизвестную архитектуру HA
|
||||
frontend и на то, что сама кодовая база уже решает эту границу в других
|
||||
местах (`getRootNode()` прецеденты), а не на прямое наблюдение внутри этого
|
||||
ревью.
|
||||
- Не оценивал производительность предложенного IntersectionObserver-подобного
|
||||
fallback (это было бы решением за автора, не моя роль на этом этапе).
|
||||
- Не проверял issues #563/#578/#582 построчно — они прочитаны только по
|
||||
ссылкам из аналитики #613 для проверки отсутствия дублирования; сам их код
|
||||
вне скоупа этой задачи (заявлено ТЗ явно, и это корректно).
|
||||
|
||||
## Вывод
|
||||
|
||||
Один High (H1) и один сопутствующий Medium-в-скоупе (M1), оба технические,
|
||||
оба возвращаются автору ТЗ в этом же issue. Остальная часть ТЗ методологически
|
||||
и процессуально готова — переработка ожидается точечной (раздел
|
||||
«Принятые технические предположения» + формулировка AC1/AC2), не переписыванием
|
||||
задачи целиком.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/613-scroll-hit-pinch-persist`, коммит `1241b9499c7b` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `9a4518ab2014ca6190468cd459762dedbcf1ff0f`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 9a4518ab2014
|
||||
```
|
||||
- Тело issue: `40b48efe32b7ca1cbc2bf41d1b3da9d1f55bf042cb41d7fa691c9453fa563e79`
|
||||
- Вердикт конвейера: `yellow` · High 1
|
||||
Reference in New Issue
Block a user