houseplan-card/docs/reviews/SPEC-REVIEW-613-r1.md

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):

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), не переписыванием задачи целиком.

---

Материал раунда

• Ветка: issue/613-scroll-hit-pinch-persist, коммит 1241b9499c7b — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: 9a4518ab2014ca6190468cd459762dedbcf1ff0f

  git log --all --format='%H %T' | grep 9a4518ab2014
  

• Тело issue: 40b48efe32b7ca1cbc2bf41d1b3da9d1f55bf042cb41d7fa691c9453fa563e79
• Вердикт конвейера: yellow · High 1