mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -1,9 +1,10 @@
|
||||
# Индекс ревью
|
||||
|
||||
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 168, issue: 79. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
|
||||
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 169, issue: 80. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
|
||||
|
||||
| Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы |
|
||||
|---|---|---|---|---:|---:|---|---|
|
||||
| #691 | [SPEC-REVIEW-691-r1.md](SPEC-REVIEW-691-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 1 | направление edge-swipe не квантифицировано и не отмечено как допущение | `src/houseplan-card.ts` |
|
||||
| #689 | [SPEC-REVIEW-689-r1.md](SPEC-REVIEW-689-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
| #688 | [SPEC-REVIEW-688-r1.md](SPEC-REVIEW-688-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
| #688 | [CODE-REVIEW-688-r1.md](CODE-REVIEW-688-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | AC6 не покрывает спиральную лестницу и второй масштаб печати на уровне PDF-сцены | `test/pdf-scene.test.mjs` `test/stairs.test.mjs` `src/pdf/pdf-scene.ts` |
|
||||
|
||||
@@ -0,0 +1,260 @@
|
||||
# SPEC-REVIEW-691-r1
|
||||
|
||||
- **Issue:** https://github.com/Matysh/houseplan-card/issues/691
|
||||
- **Этап:** `S4-spec-review` (ревью ТЗ, PROCESS.md §2.4)
|
||||
- **Трек:** полный. Автор сам зафиксировал это в комментарии «Аналитика»:
|
||||
«лёгкий трек: нет», P1, ценность 9/10, сложность и риск 7/10. Разбор по
|
||||
существу подтверждает выбор: задача меняет публичный touch-контракт View и
|
||||
kiosk (владение жестом между pan/swipe и room-fit/double-fit), затрагивает
|
||||
несколько существующих конкурентов (#449, #563, #578, #531, #152) —
|
||||
не мелкая правка одной поверхности.
|
||||
- **Материал:** тело issue #691, раздел `## ТЗ`, плюс пять комментариев
|
||||
владельца: (1) «Аналитика» — оценка, дубликаты (нет), скоуп (J1), подтверждение
|
||||
по коду (`_panLock`, `_swipeZone`, `DoubleFitGestureRecognizer`,
|
||||
`planGestureOwnerFromPath`); (2) «Взял: автор ТЗ»; (3) «Вопросы к ТЗ» — три
|
||||
продуктовых вопроса Q1–Q3 с default/альтернативами; (4) «Решения владельца
|
||||
зафиксированы» — Q1 default, Q2 альтернатива, Q3 default, `blocked` снят;
|
||||
(5) «ТЗ готово» — публикация текста в теле issue. Открытых продуктовых
|
||||
вопросов на момент ревью нет.
|
||||
- **Заход:** r1 · блокирующих циклов израсходовано 0 из 4
|
||||
- **Роль:** ревьюер ТЗ (не автор)
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Арбитраж touch/mouse/pen-жестов в View и kiosk на плане с несколькими
|
||||
пространствами: (1) новый контракт разделения pan и edge-swipe (переключение
|
||||
пространства только из внутренней 48 px краевой зоны, финальность выбора
|
||||
owner до terminal event, отмена вторым пальцем); (2) расширение double-tap/
|
||||
double-click «Вписать всё» с единственно доступной свободной поверхности на
|
||||
room fill и room label, с задержкой одиночного room-fit на 350 мс и переходом
|
||||
в fit-all без промежуточного кадра камеры. Не-скоуп по тексту ТЗ: настройки
|
||||
жестов, анимация `_slideTo()`/camera-fit геометрия, View-swipe вне kiosk,
|
||||
жесты редакторов, действия устройств/проёмов/лестниц/ссылок/декора,
|
||||
pinch-zoom/long-press/room-fit-границы/сохранение zoom, backend/WS/config/
|
||||
i18n, постоянные edge-подсказки.
|
||||
|
||||
**SCOPE-проверка (`docs/SCOPE.md`):** задача закрывает J1 («at a glance» —
|
||||
надёжный обзор многоэтажного дома) и защищает release-blocking touch-контракт
|
||||
View/kiosk (`docs/TOUCH-SUPPORT.md`: «View... must support convenient pan,
|
||||
pinch zoom and space switching»; «A touch-only failure in View is a product
|
||||
defect, not an accepted limitation»). Trailer `Touch editor: not exposed`
|
||||
поставлен корректно — редакторы не получают новый контракт. Ничего из
|
||||
«никогда не строить» (`docs/SCOPE.md` «Out of scope») не задевается: новых
|
||||
UI-элементов, настроек или геометрии нет.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `docs/process/REVIEWER.md` целиком;
|
||||
по ссылкам конспекта открыт `PROCESS.md` §2.4, §2.5, §4, §7.1, §7.2
|
||||
(§2.10 не применяется — это r1).
|
||||
2. Прочитано тело issue #691 целиком (`## Проблема`, `## Как проявляется`,
|
||||
`## Что подтверждено по актуальному коду`, `## Ожидаемое поведение`,
|
||||
`## Связанные задачи`, `## Примечание`, затем `## ТЗ` §1–§18) и все пять
|
||||
комментариев владельца — сверено, что решения Q1–Q3 из комментария
|
||||
«Решения владельца зафиксированы» дословно попали в §6 п.1, §7 п.3–4 и
|
||||
§7 п.9 итогового текста ТЗ; расхождений не найдено.
|
||||
3. Сверены обязательные разделы §7.1 в теле issue: Сценарий (§1) → Что
|
||||
человек видит до/после (§2) → Проблема и цель (§3) → Скоуп (§4) / Не-скоуп
|
||||
(§5) → Контракт поведения (§6 pan/edge-swipe, §7 room-fit/double-fit) → UX,
|
||||
доступность, ошибки (§8) → Модель данных/миграция (§9) → i18n (§10) →
|
||||
Производительность (§11, не обязательный, но уместный) → AC1–AC12 с
|
||||
доказательством (§12) → План автотестов (§13) → Затронутые модули (§14) →
|
||||
Риски (§15) → Откат (§16) → Release-артефакты (§17) → Принятые
|
||||
предположения (§18) — все обязательные разделы на месте и в осмысленном
|
||||
порядке.
|
||||
4. Технические утверждения сверены с реальным кодом на `1751184b` (нет ветки
|
||||
`issue/691-*`, диф пуст — код для #691 ещё не начат, ожидаемо для этапа
|
||||
spec):
|
||||
- `src/houseplan-card.ts:1912` — `private _panLock: 'pan' | 'swipe' | null`
|
||||
существует; `:6739` — геттер `_swipeZone` существует с описанным
|
||||
условием (kiosk, `swipeTarget`-зум, несколько пространств); `:6832–6834`
|
||||
— классификация на первом движении `> STAGE_TAP_DISTANCE_PX` по правилу
|
||||
`|dx| > |dy| * 1.5`, `:6900–6902` — release не переспрашивает
|
||||
`swipeTarget()` при `_panLock === 'pan'`. Всё описанное в «Что
|
||||
подтверждено по актуальному коду» и в §6 ТЗ как текущее поведение —
|
||||
точно.
|
||||
- `src/room-fit.ts:104–106` — `DOUBLE_FIT_WINDOW_MS = 350`,
|
||||
`STAGE_TAP_DISTANCE_PX = 8` существуют как названные константы; `:121–136`
|
||||
— `planGestureOwnerFromPath()` действительно возвращает `background`
|
||||
только для узлов, дошедших до `.stage` без interactive/room-owner на
|
||||
пути, подтверждая утверждение issue «распознаватель принимает только
|
||||
owner `background`»; `:151–165` — `beginDoubleFitPointer()` сегодня
|
||||
жёстко требует `input.owner.kind !== 'background'` → `null`, то есть
|
||||
room-owner действительно не участвует — ТЗ корректно называет это
|
||||
ограничение, которое §7 п.2–4 и §14 (`src/room-fit.ts`) просят снять.
|
||||
- `docs/CANVAS.md` §5 «Zoom and pan» подтверждает текущий контракт слово в
|
||||
слово (владение pointer, `_panLock`, финальность лока «audit DEV-1DA1-02»,
|
||||
free-background double-tap #449, room-fit #152) — ТЗ §17 корректно
|
||||
называет этот документ для обновления, ничего не упущено и не выдумано.
|
||||
- `docs/TOUCH-SUPPORT.md:42–43` фиксирует именно старое поведение («one
|
||||
tap on a room keeps the immediate room-fit action»), которое §7 ТЗ
|
||||
сознательно меняет (задержка 350 мс, решение владельца Q2-альтернатива)
|
||||
— обновление этого канона корректно названо в §17 release-артефактов.
|
||||
- Файлы из §13/§14: `test/room-fit.test.mjs`, `demo/smoke_kiosk.mjs`,
|
||||
`demo/smoke_room_fit.mjs` — существуют. `demo/smoke_kiosk_pan_lock.mjs`
|
||||
(уже существующий смоук именно на «curved gesture»/финальность лока,
|
||||
сценарий audit DEV-1DA1-02) в списке файлов по имени не назван, но
|
||||
содержательно покрыт пунктом плана автотестов «curved path» (§13 п.4) —
|
||||
не считаю это отдельной находкой, автор явно не выбрасывает и не
|
||||
ослабляет существующую позитивную проверку (§13 п.7 прямо это
|
||||
запрещает).
|
||||
5. Контракт §6 (pan/edge-swipe) и §7 (room-fit/double-fit) прочитаны пункт за
|
||||
пунктом на внутреннюю непротиворечивость и на покрытие каждым AC1–AC12;
|
||||
расхождений «в контракте есть, а AC нет» не найдено, кроме одной находки
|
||||
ниже.
|
||||
6. Числовая согласованность: 48 px (edge-зона), 8 px (`STAGE_TAP_DISTANCE_PX`,
|
||||
переиспользуется, не переопределяется), 350 мс (`DOUBLE_FIT_WINDOW_MS`,
|
||||
одно и то же окно используется и для парного double-tap, и для задержки
|
||||
одиночного room-fit) — три числа согласованы между собой, между §6/§7 и
|
||||
§18 «Принятые предположения», и совпадают с уже существующими именованными
|
||||
константами в коде (п.4 выше). Расхождений не найдено.
|
||||
7. Гейты (`tsc --noEmit`, `npm test`, `npm run build`, `check-docs.mjs`,
|
||||
смоки, инварианты) не прогонялись: этап `spec`, ветки/коммитов для #691
|
||||
нет, `git diff origin/dev...HEAD` пуст — гейты неприменимы к тексту ТЗ, а
|
||||
не пропущены.
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium — направление edge-swipe не квантифицировано и не отмечено как допущение
|
||||
|
||||
- **Файл:** тело issue #691, раздел `## ТЗ` §6 «Контракт pan и edge-swipe»,
|
||||
пункт 6.
|
||||
- **Что не так:** «Начатый в краевой зоне жест становится swipe только после
|
||||
существующего порога движения и **достаточного преобладания правильного
|
||||
горизонтального направления**» — фраза называет поведение (классификация
|
||||
диагонального жеста внутри краевой зоны), но не называет число. Формулировка
|
||||
явно отличает «существующий порог движения» (8 px, переиспользуется) от
|
||||
преобладания направления, для которого слово «существующий» не сказано —
|
||||
то есть не ясно, сохраняется ли текущее отношение `|dx| > 1.5·|dy|`
|
||||
(`src/houseplan-card.ts:6833`), меняется ли оно теперь, когда его работу
|
||||
частично взяла на себя краевая зона, или выбирается заново. §18 «Принятые
|
||||
предположения» называет число для 48 px, 8 px и 350 мс, но не для этого
|
||||
порога — то есть ровно тот случай, когда «утверждение о поведении, которого
|
||||
нет ни в одном документе и которое не помечено как предположение», должен
|
||||
фиксировать ревьюер. AC2/AC3 (§12) тестируют только явно горизонтальный и
|
||||
явно вертикальный drag, не саму границу, поэтому по букве ТЗ автор может
|
||||
реализовать любое пороговое отношение (от «чуть больше по горизонтали» до
|
||||
«почти строго горизонтально») и формально пройти оба AC, получив при этом
|
||||
заметно разное поведение на диагональном движении у самого края экрана.
|
||||
- **Почему это продуктовый, а не только технический вопрос:** PROCESS.md
|
||||
§7.1 прямо перечисляет «поведение в пограничном случае» как пример вопроса,
|
||||
который остаётся продуктовым, а не техническим усмотрением реализатора —
|
||||
а именно эта задача целиком посвящена тому, что «два визуально похожих
|
||||
жеста выполняют разные действия» (issue, «Как проявляется», п.3).
|
||||
Оставлять неоднозначной ровно ту границу, из-за которой заведена задача, —
|
||||
системно та же проблема, которую ТЗ должно закрыть.
|
||||
- **Смягчающее обстоятельство:** сам пункт 6 говорит, что при сомнении
|
||||
(«неверное направление или вертикальное движение») жест становится `pan`,
|
||||
то есть безопасным исходом (не переключает пространство). Это не даёт
|
||||
находке стать High — она не делает AC невыполнимым или в целом
|
||||
непроверяемым, только оставляет одну границу без числа.
|
||||
- **В скоуп задачи:** да, правится одной фразой — либо явным числом (можно
|
||||
переиспользовать текущее `1.5`), либо явной записью в §18, что порог
|
||||
сохраняется без изменения. Отдельный issue не заводится (#202).
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Обязательные разделы §7.1 присутствуют полностью и в осмысленном порядке
|
||||
(см. «Как проверялось» п.3); «Сценарий» называет персону (телефон/настенный
|
||||
touch-экран, несколько пространств — Household members/Guests по
|
||||
`docs/SCOPE.md`), поверхность (View/kiosk) и момент; «Что человек видит до
|
||||
и после» — двумя абзацами, преимущественно без терминов реализации.
|
||||
- Контракт §6/§7 внутренне согласован (кроме находки выше) и полностью
|
||||
покрыт: п.1–5 (когда доступен edge-swipe, ширина зоны, направление, нет
|
||||
соседа, вне зоны — всегда pan) → AC1/AC2/AC3; п.6–7 (классификация и
|
||||
финальность) → AC2–AC4; п.8 (второй палец) → AC5; п.9 (interactive owner)
|
||||
→ AC5/AC9; §7 п.1–3 (модальность, поверхности, задержанный room-fit) → AC6/
|
||||
AC8; п.4 (double-fit без промежуточного кадра) → AC7; п.5 (одиночный
|
||||
background-тап не делает ничего) → согласуется с уже принятым #449, не
|
||||
меняется; п.6 (interactive owners исключены) → AC9; п.7 (lifecycle-отмена)
|
||||
→ AC10; п.9 (модальности не смешиваются, keyboard без задержки) → AC8;
|
||||
п.10 (fit-all переиспользует camera transition) → покрыт §11
|
||||
(производительность) и не отдельным AC — обоснованно, это не новое
|
||||
поведение.
|
||||
- Три ключевых числа (48 px, 8 px, 350 мс) согласованы между контрактом, AC
|
||||
и §18, и совпадают с существующими именованными константами в коде
|
||||
(`STAGE_TAP_DISTANCE_PX`, `DOUBLE_FIT_WINDOW_MS`) — не изобретены заново.
|
||||
- AC1–AC12 однозначны (кроме границы в находке выше) и называют способ
|
||||
доказательства: unit (с fake clock для AC6–AC8), browser smoke, mutation
|
||||
witness для четырёх защитных AC (AC1, AC4, AC5, AC9) — ровно там, где
|
||||
защитное поведение (снятие guard, финального лока, owner-исключения)
|
||||
требует не просто падающего теста, а названной мутации; на этапе spec
|
||||
название способа доказательства достаточно, полная таблица «чем краснеет»
|
||||
с результатом прогона — требование этапа code (`docs/process/REVIEWER.md`
|
||||
«Код-ревью»), не spec.
|
||||
- Технические решения (расположение guard, разделение `room-fit.ts`/
|
||||
`houseplan-card.ts`, устройство pure state machine) корректно оставлены
|
||||
«на усмотрение реализации» и не эскалированы владельцу — они не наблюдаемы
|
||||
пользователем напрямую.
|
||||
- Продуктовые развилки Q1–Q3 были заданы владельцу одним комментарием с
|
||||
default/альтернативами, решены явно («Q1 — Default», «Q2 — Альтернатива»,
|
||||
«Q3 — Default») до публикации текста ТЗ; в самом тексте ТЗ открытых
|
||||
продуктовых вопросов не осталось — соответствует §7.1 (порог на вопросы,
|
||||
пачкой, с вариантом по умолчанию).
|
||||
- Риски (§15) называют именно те конкурирующие механизмы, которые фигурируют
|
||||
в «Связанных задачах» issue (#449, #563, #578, #531) и в самом контракте
|
||||
(остаточный палец после pinch, таймер, переживший смену пространства,
|
||||
WebView edge-gesture конкуренция) — не спрятаны внутри AC.
|
||||
- Откат (§16) — чистый revert продуктового коммита, без данных/миграции;
|
||||
согласуется с §9 «Данные, совместимость и миграция» (новых полей нет).
|
||||
- Release-артефакты (§17) называют оба changelog, `docs/CANVAS.md` и
|
||||
`docs/TOUCH-SUPPORT.md` (сверено выше, что оба документа действительно
|
||||
фиксируют заменяемое поведение и нуждаются в правке), условно —
|
||||
`docs/TESTING.md`, golden/check-docs — список полон, ничего не упущено и
|
||||
не добавлено лишнего (`docs/CONFIG-COMPATIBILITY.md` в списке нет и не
|
||||
должен быть — миграции нет).
|
||||
- i18n (§10) — явное «нет новых строк», согласуется с тем, что весь диф —
|
||||
арбитраж жестов, а не новый UI-текст.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Гейты (`tsc --noEmit`, `npm test`, `npm run build`, `check-docs.mjs`,
|
||||
смоки, `golden:verify`, инварианты, performance) — не прогонял: этап
|
||||
`spec`, кода для #691 нет, `git diff origin/dev...HEAD` пуст (см. «Как
|
||||
проверялось» п.7). Предмет код-ревью после реализации.
|
||||
- Не пробовал руками воспроизводить исходный баг (конфликт pan/swipe,
|
||||
недоступный double-tap) на реальном touch-устройстве или в HA Companion —
|
||||
это диагностика владельца и автора ТЗ по коду (issue «Что подтверждено по
|
||||
актуальному коду», подтверждённая мной построчным чтением кода, см. «Как
|
||||
проверялось» п.4), а не то, что проверяется вручную на этапе spec.
|
||||
- Не оценивал практическую (а не только текстовую) осуществимость pure state
|
||||
machine для edge-arbitration и double-fit при реальной интеграции с
|
||||
Lit-рендером и HA Companion WebView — это по разделу «Принято
|
||||
предположительно» оставлено на усмотрение реализации и подлежит AC1–AC10
|
||||
на код-ревью.
|
||||
- Не искал в `docs/reviews/INDEX.md` строки предыдущих раундов подсистемы —
|
||||
не требуется: это r1, §2.10 (объём по дельте) не применяется.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Обязательные разделы ТЗ полны и в осмысленном порядке, контракт §6/§7 почти
|
||||
полностью непротиворечив и покрыт AC1–AC12 с названным способом доказательства,
|
||||
включая защитные mutation witnesses там, где это нужно. Технические
|
||||
утверждения о текущем коде (константы, геттеры, функции, ограничение
|
||||
`beginDoubleFitPointer` на `background`) проверены построчно и оказались
|
||||
точными. Продуктовые развилки Q1–Q3 закрыты владельцем до публикации текста,
|
||||
открытых продуктовых вопросов не осталось. Единственная находка — Medium,
|
||||
в скоупе задачи: направление-преобладание для классификации edge-swipe
|
||||
(§6 п.6) не квантифицировано и не зафиксировано в «Принятых предположениях»,
|
||||
хотя именно эта граница — источник исходного дефекта («визуально похожие
|
||||
жесты выполняют разные действия»). Находка не блокирует технически (fallback
|
||||
безопасен — неопределённость разрешается в `pan`), но нарушает требование
|
||||
однозначности AC и правится одной фразой. Без High-находок это жёлтый
|
||||
вердикт: автор добавляет число или явное допущение в §6 п.6/§18, ревью
|
||||
проходит повторный (нецикловый по факту правки) цикл.
|
||||
|
||||
Вердикт: жёлтый · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 1 → в задаче
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `dev`, коммит `1751184b1247` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `e271ee4d37f31e5b35a219fe7f168aaceeda15f6`
|
||||
```
|
||||
git log --all --format='%H %T' | grep e271ee4d37f3
|
||||
```
|
||||
- Тело issue: `4abdb509952fb3ea465e6d6c32a8f6825982bf8533b7f5ff35585db7ee2840c2`
|
||||
- Вердикт конвейера: `yellow` · High 0
|
||||
Reference in New Issue
Block a user