mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -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) снята с записью как не создающая риска на практике. ТЗ
|
||||
готово к статусу «Готово к разработке».
|
||||
|
||||
Вердикт: зелёный.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/613-scroll-hit-pinch-persist`, коммит `d36b7f80c320` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `58cda05f89d57c19a721dd12800ec32654231dcb`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 58cda05f89d5
|
||||
```
|
||||
- Тело issue: `98d86d7e8cd4c13b732452c7b4c391e31cf302d3636fbe1ef0c8a7aeb5dac9e3`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user