From 02a523045b2aeef364e0f792d4fe3f93a6367bcb Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sat, 22 Aug 2026 16:09:16 +0000 Subject: [PATCH] docs: review document for #243 Issue: #243 User-Visible: no --- docs/reviews/SPEC-REVIEW-243-r1.md | 188 +++++++++++++++++++++++++++++ 1 file changed, 188 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-243-r1.md diff --git a/docs/reviews/SPEC-REVIEW-243-r1.md b/docs/reviews/SPEC-REVIEW-243-r1.md new file mode 100644 index 00000000..3056dfda --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-243-r1.md @@ -0,0 +1,188 @@ +# SPEC-REVIEW-243-r1 + +- Issue: [#243](https://github.com/Matysh/houseplan-card/issues/243) — «Перетаскивание вкладок пространств не работает: захват указателя съедает + цель. Плюс нужен указатель места вставки» +- Этап: spec (PROCESS.md §2.4) +- Заход: r1 · блокирующих циклов израсходовано 0 из 4 +- ТЗ: `docs/specs/243-space-tab-drop-target.md`, коммит `7ac1934ac5002f915c2e1f885c9142daaee645a8` + (`docs(spec): define reliable space tab drop target`, `Issue: #243`, `User-Visible: no`) +- Трек: обычный (не `small`) — подтверждено: задача вводит видимый UX-контракт + (вертикальный разделитель before/after), не подпадает под критерии §5. +- Ревьюер этого раунда контекста автора не имеет; ТЗ и issue разобраны как + состязательный материал. + +## Скоуп ревью + +Первый заход, предыдущего вердикта нет — раздел «Унаследовано из r0» не +применяется, разбор ниже полный по PROCESS.md §2.4 и §7.1. + +## Как проверялось + +1. Прочитан `docs/SCOPE.md` — задача чинит регрессию выпущенной функции J6 + («keep the plan true as the home evolves», порядок пространств/вкладок), + персона — home admin, поверхность — desktop editors (plan/devices/decor). + Новый UX-контракт (разделитель) не расширяет мандат: это правка уже + принятой функции #220, не новая возможность. +2. Прочитаны `AGENTS.md`, `PROCESS.md` целиком (классы изменений, §7.1 + обязательные разделы, §5 критерии `small`, §4 лимит циклов, шаблон + вердикта §7.2). +3. Прочитано тело issue #243 и все три комментария (аналитика владельца, + занятие автором, сдача ТЗ). Открытых продуктовых вопросов автор не + поднимал — сверено, что действительно нечего было спрашивать (см. ниже). +4. Прочитан `docs/USER-GUIDE.ru.md` (§ вкладки/перетаскивание, строки + 276–282) — терминология «вкладка», «перетаскивание», используемая ТЗ, + совпадает с зафиксированной там, новых терминов не изобретено. +5. Прочитан канонический документ затронутой функции — + `docs/specs/220-space-tab-reorder.md` (диагноз, контракт `reorderSpaceIds`, + продуктовые решения владельца, границы touch/View) — и + `docs/TOUCH-SUPPORT.md` (§ «Safety floor», формулировка `Touch editor: not + exposed` — ТЗ цитирует её дословно, не изобретает). +6. Диагноз ТЗ сверен построчно с актуальным `dev` (не с описанием в issue): + - `src/houseplan-card.ts:1302-1329` (`_tabPointerDown`) — `capturePointer` + вызывается, `_tabDrag.overId` инициализируется значением `id` источника; + - `src/houseplan-card.ts:1331-1340` (`_tabPointerMove`) — обработчик + навешен на каждую вкладку отдельно (`houseplan-card.ts:16426`, + `@pointermove=${(e) => this._tabPointerMove(e, s.id)}`) и получает + фиксированный `overId = s.id` того элемента, на который повешен; + - после `setPointerCapture` браузер ретаргетирует все `pointermove` на + захвативший элемент — значит, только обработчик, навешенный на + исходную вкладку, продолжает получать события, и передаёт в него + собственный `overId` (= `drag.id`). Логический вывод ТЗ «`overId` + остаётся равен `drag.id`» подтверждён чтением, не только заявлением + автора issue; + - `_tabPointerUp` (`:1342-1348`) → `_commitTabOrder(drag.id, drag.overId)` + → при `drag.id === drag.overId` `reorderSpaceIds` (`src/space-order.ts:55-65`) + закономерно возвращает тот же массив — запись не идёт. Диагноз ТЗ §3 + воспроизводится по коду, а не только по логу автора issue. +7. Проверена математика `reorderSpaceIds` на нескольких комбинациях + (перемещение к более ранней/более поздней вкладке, соседние позиции, + перемещение в начало/конец массива) — во всех прогнанных вручную случаях + результат совпадает с заявленным в ТЗ §4.3 правилом «`to < from` → before, + `to > from` → after»: сторона индикатора однозначно соответствует + итоговой позиции после `splice`. Расхождений не найдено. +8. Проверены факты, на которые ТЗ опирается как на данность: лимит + `MAX_SPACES = 50` (`custom_components/houseplan/validation.py:478`, + задокументирован в `docs/USER-GUIDE.ru.md:1520`) — используется в ТЗ §11 + как обоснование «отдельный performance-профиль не нужен»; существующие + CSS-классы `.tab[data-reorderable]`, `.tab.dragging`, `.tab.droptarget` + (`src/styles.ts:1725-1728`) — ТЗ корректно описывает их как + «текущее, недостаточное» состояние (единственный `.droptarget` без + стороны). +9. Проверены команды гейтов из ТЗ §15 на существование: `node + scripts/mutation-gate.mjs --check` и `--id=` реализованы + (`scripts/mutation-gate.mjs:1608,1620`); `node scripts/smoke-select.mjs + --base --head ` — существующий скрипт. Три новых + mutation-id (`tab-drag-target-follows-captured-source`, + `tab-drop-indicator-always-before`, `tab-drop-outside-commits-last-target`) + не пересекаются с уже зарегистрированными (`tab-reorder-not-persisted`, + `tab-drag-survives-release-outside`, `tab-drag-outlives-the-card`, + `tab-reorder-eats-click`, `tab-reorder-ignores-pointer-type`). +10. Проверено наличие прецедента в golden-харнесе для утверждаемого способа + доказательства AC2/AC8: `demo/golden/harness.mjs` уже использует + `page.mouse`, то есть захват мыши в момент капчура — не новый для + инфраструктуры приём, а не голая гипотеза автора. +11. Проверены обязательные разделы §7.1: сценарий и персона (§1) · что + человек увидит до/после (§2) · диагноз/проблема (§3) · продуктовые + решения (§4) · скоуп и не-скоуп (§6) · контракт поведения и + hit-testing (§7–8) · UX/визуальный контракт (§9) · данные/i18n/a11y/touch + (§10) · перф/безопасность (§11) · AC1–AC9 с доказательством (§12) · план + автотестов (§13) · mutation guards (§14) · гейты (§15) · release- + артефакты (§16) · откат (§17) · риски (§18) · явный блок принятых + технических предположений (§19). Все присутствуют, ничего не + зачёркнуто «TBD». +12. Проверен трейлерный коммит: `Issue: #243`, `User-Visible: no` — + корректно для чисто документационного изменения; diff коммита + ограничен `docs/specs/243-space-tab-drop-target.md` и + `docs/specs/README.md` (класс C), продуктовый код не тронут — Rule #1 + соблюдено. + +## Находки + +Находок нет — ни High, ни Medium, ни Low. + +Отдельно проверенные потенциальные риски ложной уверенности, снятые +разбором: + +- **Не является ли диагноз просто пересказом автора issue?** Нет: диагноз + сверен построчно с `dev` (см. п.6 выше) независимо от текста issue — + вывод «`overId` не меняется после capture» логически следует из + сочетания «per-tab listener + retargeting событий при `setPointerCapture`», + а не принят на веру. +- **Не выдана ли догадка за решение?** Технические решения (расположение + обработчика на `.tabs`, форма transient-state, способ рисования линии, + вынос hit-test helper в `space-order.ts` или нет) явно помечены в §19 + «принято предположительно, поменять свободно» — ревьюер вправе оспорить, + но это не блокер: ни одно из них не является пользовательским поведением. + Нормативные же требования (реальный `page.mouse`, сохранение capture, + точная сторона before/after, отмена drop вне вкладки, отсутствие drag во + View/touch/fixedFloor) явно исключены из категории предположений в конце + §19 и присутствуют как факт в основном тексте — обоснованно, они прямо + унаследованы от продуктовых решений #220 и явного ТЗ #243. +- **Нет ли скрытого расширения скоупа?** «Не входит» (§6) явно исключает + touch/pen drag, клавиатурную сортировку, смену модели порядка, схему + `config.spaces`, `expected_rev`, материализацию маркеров, `floor`-toast, + свайп/карусель и `fixedFloor`-карточку — совпадает с P0-границами #220, + которые #243 обязан не трогать. +- **Проверяем ли AC однозначно и с доказательством?** Да, AC1–AC9 (§12) + пронумерованы, у каждого указан способ доказательства (`browser smoke`, + `unit`, `golden`, `gates`), совпадающий с допустимыми видами из DoR §2.5. +- **Продуктовые вопросы владельцу нужны?** Нет открытых: сторона + разделителя (before/after), обязательность реального `page.mouse` и + сохранение capture — всё это уже зафиксировано либо в теле issue + (`AC2`, «мой вариант по умолчанию — первый»), либо в комментарии + аналитики владельца («действующие границы #220 сохраняются»). Технические + вопросы (где хранить transient-state, чем рисовать линию) ТЗ не выносит + владельцу и решает само — корректно по §7.1. + +## Что проверено и корректно + +- Соответствие `docs/SCOPE.md`: правка регрессии J6, персона/поверхность + верны, новый видимый элемент (разделитель) не создаёт новой функции — + только делает существующий контракт #220 рабочим и honest. +- Диагноз причины бага — подтверждён чтением кода независимо от заявления + issue (см. «Как проверялось», п.6). +- Математика `reorderSpaceIds` и заявленное правило стороны индикатора — + проверены вручную на нескольких комбинациях перестановки, расхождений с + ТЗ §4.3/§7 не найдено. +- Все 12 обязательных разделов §7.1 присутствуют и по существу. +- AC1–AC9 однозначны, у каждого назван способ доказательства из + допустимого набора; ни один не сформулирован как нефальсифицируемый. +- Явный блок принятых технических предположений (§19) отделён от + нормативных требований — соответствует правилу «догадка, выданная за + факт, — худший вид дефекта». +- Терминология («вкладка», «перетаскивание», `Touch editor: not exposed`) + взята из `docs/USER-GUIDE.ru.md` и `docs/TOUCH-SUPPORT.md`, а не + изобретена. +- Упомянутые в ТЗ команды гейтов (`mutation-gate.mjs --check/--id=`, + `smoke-select.mjs --base --head`) существуют в репозитории с такими + флагами; новые mutation-id не пересекаются с реестром. +- Класс изменения (C — только `docs/**`) и трейлеры коммита корректны; + продуктовый код в этом коммите не тронут. +- Лимит циклов ТЗ на обычном треке — 4 (не 2, трек не `small`) — учтён при + формировании вердикта. + +## Чего не проверял + +- Не запускал никакие браузерные смоки, unit, golden или mutation-gate — + на этапе ревью ТЗ кода ещё нет (ТЗ описывает будущую реализацию), гейты + §15 относятся к стадии «В разработке» и будут материалом код-ревью. +- Не проверял golden-харнесс на техническую способность реально удержать + `page.mouse.down` до `screenshot` и корректно освободить кнопку между + сценами (риск #18 п.5 ТЗ) — это техническая реализуемость, а не + продуктовая ambiguity, и относится к зоне ответственности автора и + будущего код-ревью. +- Не проверял, действительно ли `ShadowRoot.elementFromPoint()` даёт тот же + результат, что ручной hit-test по `getBoundingClientRect()` во всех + браузерах — ТЗ прямо оставляет выбор реализации автору (§19 п.4), + вопрос не продуктовый. + +## Вердикт + +Спецификация полна, диагноз проверен независимо от слов автора issue, +контракт стороны индикатора математически согласован с существующей +семантикой `reorderSpaceIds`, скоуп и не-скоуп чётко отделяют регрессионный +фикс от нового поведения, все обязательные разделы §7.1 на месте, AC +проверяемы и снабжены допустимым способом доказательства, открытых +продуктовых вопросов нет. + +**Вердикт: зелёный · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 0 → в задаче**