From 59fdae817a9bbc0322a9feaa9120be2f22f07d33 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Wed, 23 Sep 2026 02:25:10 +0000 Subject: [PATCH] docs: review document for #613 Issue: #613 User-Visible: no --- docs/reviews/SPEC-REVIEW-613-r2.md | 234 +++++++++++++++++++++++++++++ 1 file changed, 234 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-613-r2.md diff --git a/docs/reviews/SPEC-REVIEW-613-r2.md b/docs/reviews/SPEC-REVIEW-613-r2.md new file mode 100644 index 00000000..2dc84c2b --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-613-r2.md @@ -0,0 +1,234 @@ +# SPEC-REVIEW-613-r2 + +Материал: тело issue #613, редакция после правки автора 2026-09-23T02:14:56Z +(последняя `userContentEdit`, применена через 88 секунд после вердикта r1, +опубликованного 2026-09-23T02:13:28Z). Хэш тела на момент этого ревью: +`sha256:71ea7980f0a9d84820d118ab5bcc6e7fa71f7e0be9368498f58a4c30c197af5e` +(вычислен с `\r\n` как в живом API; без завершающего перевода строки — +`1115bb4c836ca6529c546e6d13af5dc515be93553a8b6c468a2e3d6f47dfac89`). Дерево +кода на момент проверки — `d36b7f80c3208eab7fdaca6fdfd6c4d131a86cba` (рабочая +копия); код относительно материала r1 (`1241b9499c7b`) не менялся — единственный +коммит между раундами добавил только `docs/reviews/SPEC-REVIEW-613-r1.md` +(подтверждено: `git diff 1241b949..d36b7f80 --stat -- src/ demo/ scripts/` +пуст). Код читан только для проверки утверждений ТЗ, не как материал +код-ревью, и в этом раунде повторно не читался — см. «Унаследовано из r1». + +Заход r2, блокирующих циклов израсходовано 1 из 4 (r1 — жёлтый, потратил 1; +зелёный цикла не образует и бюджет не тратит, §4/#227). + +**Примечание к процессу поиска материала.** Вердикт r1 в комментарии issue не +называет SHA явно (только «Документ: docs/reviews/SPEC-REVIEW-613-r1.md») — +это соответствует описанному в задании шаблону «SHA в вердикте не назван». +SHA и весь материал раунда найдены в блоке «Материал раунда» в конце самого +документа `SPEC-REVIEW-613-r1.md` (машинный блок, подставлен конвейером). +Дополнительно раунд r1 не назвал SHA/blob тела issue *до* правки в виде, +удобном для машинной сверки, — этот пробел закрыт вручную через GraphQL +`userContentEdits` (три исторические редакции тела; см. ниже), поскольку +`docs/specs/*.md` для issue #613 не существует (задача заведена после +2026-09-10, ТЗ живёт только в теле issue, архив не создаётся — верно по +`PROCESS.md` §7.3 п.1). + +## Скоуп ревью + +Раунд r2 разбирает **только дельту тела issue между материалом r1 и текущим +состоянием** — это локальная точечная правка (переформулировка технического +решения и AC в ответ на H1/M1 r1), без ребейза, без смены подсистемы, без +смены поведенческого контракта в остальных разделах. Условия «разбор остаётся +полным» (ребейз на ушедший вперёд `dev`, смена контракта, новая подсистема, +объём дельты сопоставим с исходной задачей) не выполнены ни одно — код не +менялся вообще, а дельта текста ограничена 6 из 22 разделов ТЗ. Полный +повторный разбор всего ТЗ не требуется; см. «Унаследовано из r1» для +остального. + +Проверялось в этом раунде: +- дельта тела issue между материалом r1 (`sha256`-снимок редакции от + 2026-09-23T02:03:28Z, 9278 байт) и текущим состоянием (10685 байт, 2026-09-23T02:14:56Z), + раздел за разделом; +- закрывает ли новая формулировка H1 (нерабочее предположение о + `scroll`-инвалидации через `ownerDocument`) и M1 (AC1/AC2 не требовали + witness за пределами `document`-дерева карточки) технически, а не + косметически; +- не породила ли переформулировка новое противоречие «Контракт ↔ Риски», + аналогичное найденному в r1; +- не разошлись ли изменённые AC (AC1, AC2) с неизменными соседними AC + (AC3–AC6) и с планом автотестов. + +## Как проверялось + +- Получено тело issue #613 текущей редакции через `gh issue view --json body`. +- Получена полная история редактирования тела через GraphQL + `repository.issue.userContentEdits` (3 записи: исходный короткий баг-репорт + 19:54:25Z → полное ТЗ 02:03:28Z, которое и было материалом r1 → финальная + правка 02:14:56Z, материал r2). Снимок 02:03:28Z побайтово сверен с цитатами + в `docs/reviews/SPEC-REVIEW-613-r1.md` (раздел «Находки», H1/M1) — + совпадает дословно, значит найден правильный «до»-снимок, а не соседняя + редакция. +- Тело до/после разбито на секции по заголовкам `##`/`###`; секции сравнены + попарно (`old[k] != new[k]`) — изменившимися оказались ровно 6 из 22: + «Скоуп», «Контракт поведения», «Критерии приёмки», «План автотестов», + «Риски», «Принятые технические предположения». Остальные 16 (Сценарий, Что + человек увидит, Проблема, Не-скоуп, UX, Модель данных и миграция, i18n, AC5/AC3/AC4/AC6 + как текст, Производительность, Touch, Откат, Release-артефакты и вводные + абзацы «Подтверждённые дефекты») побайтово идентичны материалу r1. +- Прочитан полностью PROCESS.md §2.4, §2.9 (правильное название — фактически + §2.10 в файле, «Повторный раунд ревью — объём по дельте»), §4, §7.2 — + сверены правила бюджета циклов, формат вердикта и обязательные разделы + документа для повторного раунда. +- Прочитан `docs/TOUCH-SUPPORT.md` на предмет упоминаний scroll/shadow — + канонический документ не фиксирует конкретный механизм инвалидации, новое + техническое решение ему не противоречит (поиск `shadow|scroll|composed` — + два несвязанных совпадения, оба про другой контекст). +- Автотесты/гейты не гонялись: стадия `spec`, код не менялся между раундами + (проверено `git diff 1241b949..d36b7f80 --stat -- src/ demo/ scripts/` — + пусто), гонять `tsc`/`test`/`build` не на чем и незачем — эти гейты относятся + к этапу `code`, не к этапу `spec`. + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где это видно | +|---|---|---| +| **H1 (High).** Предположение «capture-listener `scroll` на `ownerDocument` + `visualViewport`» не работает, если реальный scroll-предок карточки находится в чужом shadow-root (событие `scroll` не `composed`, подтверждено playwright-экспериментом в r1) — противоречие с разделом «Риски» не разрешалось. | Механизм заменён: раздел «Принятые технические предположения» теперь описывает **обход composed ancestor-chain** (`parentElement`, затем `ShadowRoot.host`, до document/window) с прямой подпиской на каждый источник — то есть выбран вариант (а) из требования r1 («подъём по реальной цепочке скроллящихся предков через `getRootNode()`»), а не документированный компромисс (б). Раздел «Риски» больше не содержит противоречащего утверждения «слушатель только на `window` не покрывает все случаи» — эта фраза удалена, вместо неё — риск нового рода («composed-цепочка может измениться после remount»), не конфликтующий с контрактом. | Тело issue, раздел «Скоуп» (буллет 1–2, переформулирован под composed-цепочку), раздел «Контракт поведения» п.2 (переписан целиком: «`scroll` не считается `composed`... обходит composed-цепочку вверх... Поэтому scroll-контейнер в любом внешнем shadow-root инвалидирует кэш»), раздел «Принятые технические предположения» (первый пункт заменён целиком на lifecycle-helper по composed ancestor-chain), раздел «Риски» (второй пункт заменён). | +| **M1 (Medium, в скоупе).** AC1/AC2 не требовали, чтобы хотя бы один smoke-прогон воспроизводил scroll-контейнер **за пределами `ownerDocument`-дерева карточки** (внутри стороннего `attachShadow`) — иначе light-DOM смок мог «доказать» AC и не поймать реальный дефект HA-вложения. | AC1 переименован в «scroll hit correctness **across Shadow DOM**» и теперь явно требует: «карточка помещена внутрь прокручиваемого контейнера во **внешнем** `attachShadow`... прокрутки **внешнего контейнера**... Дополнительный light-DOM вариант допустим, **но не заменяет этот witness**». AC2 добавляет «включая ancestor **за границей внешнего shadow-root**». План автотестов явно требует «монтировать карточку... внутри внешнего `attachShadow` и прокручиваемого ancestor» и отдельно уточняет, что «browser smoke обязан отдельно покрыть реальный scroll через внешний `attachShadow`». | Тело issue, раздел «Критерии приёмки» AC1 и AC2 (переформулированы), раздел «План автотестов» (пункты 1 и 3 переписаны). | + +Обе находки закрыты в тексте, а не заявлением — переформулировка называет +конкретный механизм (обход composed-цепочки через `ShadowRoot.host`) и +конкретное требование к тесту (witness обязан пересекать границу стороннего +`attachShadow`), а не общие слова о «доработке». + +Проверка на новое противоречие (по образцу самого H1, где «Контракт» +противоречил «Рискам»): в новой редакции «Контракт поведения» п.2 и «Риски» +п.2 согласованы — контракт обещает полный обход ancestor-chain, риск честно +называет цену этого обещания (пропуск ancestor/remount), не отрицая при этом +работоспособность самого механизма для заявленного сценария. Противоречия +того же типа, что в H1, не возникло. + +Технической стороной решение также опирается на прецедент, уже +существующий в кодовой базе (`getRootNode()`-обход теневых границ в +`src/hp-dialog.ts` и `src/hp-zigbee-topology-overlay.ts`, зафиксированный в +r1) — то есть команда предлагает не новый непроверенный паттерн, а +переиспользование уже работающего в проекте приёма. Код не менялся, повторно +не читан в этом раунде — доверие к этому факту наследуется из r1, не +проверяется заново (см. ниже). + +## Унаследовано из r1 + +Без повторной проверки в этом раунде — код между раундами не менялся, +а перечисленные разделы ТЗ дельтой r1→r2 не задеты: + +- Наличие обязательных разделов §7.1 (сценарий, до/после, проблема, + скоуп/не-скоуп, UX, модель данных, i18n, план автотестов, риски, откат, + release-артефакты) — документ `SPEC-REVIEW-613-r1.md`, раздел «Что проверено + и корректно». +- Соответствие двух утверждений ТЗ о текущем поведении кода (`_saveZoom()` на + каждый `pointermove` в обоих pinch-путях; индекс не инвалидируется на + scroll) — подтверждено чтением `src/device-hit-owner.ts` и + `src/houseplan-card.ts` на дереве `1241b9499c7b` (`SPEC-REVIEW-613-r1.md`, + раздел «Как проверялось»). Дерево не менялось (см. «Как проверялось» выше). +- Реализуемость контракта арбитража #564 (`DevicePointerOwnerLatch`) на + существующем механизме без структурных изменений — `SPEC-REVIEW-613-r1.md`. +- Существование обоих smoke-файлов (`demo/smoke_device_hit_capsules.mjs`, + `demo/smoke_editor_gestures.mjs`) и `scripts/mutation-registry.mjs` — + `SPEC-REVIEW-613-r1.md`. +- Выбор полного трека (а не лёгкого) — обоснование в аналитике автора и + подтверждение r1 не изменились: дельта не меняет число затронутых + поверхностей. +- Отсутствие продуктовых вопросов к владельцу — дельта r1→r2 полностью + техническая (механизм инвалидации, формулировка AC), продуктовая рамка + (сценарий, до/после, UX) не тронута. +- AC3, AC4, AC5, AC6 как формулировки — не менялись текстом; их согласованность + с AC1/AC2 проверена в этом раунде заново (см. «Скоуп ревью» и «Находки»), + поскольку AC6 ссылается на AC1/AC2 напрямую («мутант... падает на AC1/AC2»). + +Документ и материал первоисточника: `docs/reviews/SPEC-REVIEW-613-r1.md`, +блок «Материал раунда» — дерево `9a4518ab2014ca6190468cd459762dedbcf1ff0f`, +ветка `issue/613-scroll-hit-pinch-persist` на `1241b9499c7b`, вердикт `yellow`, +High 1. + +## Находки + +Нет находок High или Medium. Переформулировка точечно закрывает обе находки +r1 корректным техническим решением, не вносит новых противоречий и не +затрагивает разделы, не относящиеся к H1/M1. + +**Low (снята с записью, не блокирует).** Формулировка AC2 «включая ancestor +за границей внешнего shadow-root» синтаксически двусмысленна при первом +прочтении отдельно от AC1 (можно прочесть как «ancestor, который находится +снаружи внешнего shadow-root», а не как задумано — «ancestor на границе или +за пределами внешнего shadow-root в сторону ещё более глубокой вложенности»). +Двусмысленность снимается контекстом: AC1 уже фиксирует ровно один уровень +вложенности («внешний `attachShadow`»), а «Контракт поведения» п.2 и +«Принятые технические предположения» однозначно описывают полный обход +цепочки произвольной глубины. Разработчик, реализующий AC2 в паре с AC1 и +контрактом, не может ошибиться в объёме требования. Снимаю без правки текста +ТЗ — стоимость уточнения формулировки выше цены двусмысленности, которая +разрешается соседними разделами того же документа. + +## Что проверено и корректно + +- Обе находки r1 (H1, M1) закрыты по существу, не косметически: изменившийся + раздел «Принятые технические предположения» выбирает именно тот вариант + решения, который r1 назвал технически реализуемым (обход composed-цепочки + через `ShadowRoot.host`), а не переформулировку прежнего нерабочего + предположения другими словами. +- Новая формулировка AC1/AC2 требует witness, пересекающий границу стороннего + `attachShadow`, и явно не позволяет заменить его light-DOM вариантом + («Дополнительный light-DOM вариант допустим, но не заменяет этот witness») — + ровно то, что M1 требовал явно записать. +- «Контракт поведения» и «Риски» в новой редакции не противоречат друг другу + (проверено по образцу конфликта, найденного в H1) — контракт обещает то, что + риски признают дорогим, но не невозможным. +- План автотестов согласован с изменёнными AC: явно называет, какой смок + доказывает cross-shadow сценарий, а что допустимо доказывать управляемым + contract-фикстуром (`visualViewport`/reconnect) — граница между + «обязательный browser witness» и «допустимый unit-fixture» проведена по + тому же принципу, что и в AC1. +- Дельта не тронула ни одного из шести AC по существу их идентификаторов и + число (AC1…AC6 те же), не расширила и не сузила скоуп/не-скоуп, не добавила + и не убрала пункты модели данных, миграции, отката, release-артефактов — + никакого «попутного» изменения задачи вместе с точечным фиксом не произошло. +- Код между раундами не менялся (`git diff` по `src/`, `demo/`, `scripts/` + пуст) — утверждения ТЗ о текущем поведении кода, подтверждённые в r1, + остаются в силе без повторной проверки. + +## Чего не проверял + +- Не читал повторно `src/device-hit-owner.ts` и `src/houseplan-card.ts` — + код не менялся, доверие к прошлому чтению унаследовано из r1 (см. + «Унаследовано из r1»). +- Не проверял реальную DOM-структуру Home Assistant Lovelace вживую в этом + раунде — вывод об архитектуре HA frontend не менялся между раундами и не + является предметом дельты; проверялось в r1. +- Не гонял `tsc`/`test`/`build`/смоки/инварианты — стадия `spec`, кода для + прогона нет (код между раундами не менялся, что подтверждено `git diff`, + а не просто предположено). +- Не оценивал производительность или корректность конкретной реализации + lifecycle-helper (сколько слушателей, какая структура данных) — это решение + за автором на этапе реализации, явно помечено в ТЗ как «можно свободно + изменить на ревью», и не предмет спецификационного ревью. +- Не проверял issues #563/#578/#582 повторно — вне скоупа, не задеты дельтой. + +## Вывод + +Обе находки r1 (H1 High, M1 Medium-в-скоупе) закрыты точечно и по существу: +изменившиеся 6 из 22 разделов ТЗ выбирают технически реализуемый механизм +(composed ancestor-chain вместо `ownerDocument`-capture) и требуют witness, +пересекающий границу стороннего `attachShadow`, — ровно то, что r1 назвал +условием возврата. Новых High/Medium не найдено, одна Low-находка (двусмысленность +формулировки AC2) снята с записью как не создающая риска на практике. ТЗ +готово к статусу «Готово к разработке». + +Вердикт: зелёный. + +--- + + + +## Материал раунда + +- Ветка: `issue/613-scroll-hit-pinch-persist`, коммит `d36b7f80c320` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `58cda05f89d57c19a721dd12800ec32654231dcb` + ``` + git log --all --format='%H %T' | grep 58cda05f89d5 + ``` +- Тело issue: `98d86d7e8cd4c13b732452c7b4c391e31cf302d3636fbe1ef0c8a7aeb5dac9e3` +- Вердикт конвейера: `green` · High 0