mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-01 04:09:17 +00:00
@@ -0,0 +1,169 @@
|
||||
# SPEC-REVIEW-213-r1
|
||||
|
||||
- **Issue:** https://github.com/Matysh/houseplan-card/issues/213
|
||||
- **Этап:** ТЗ на ревью (S4-spec-review), PROCESS.md §2.4
|
||||
- **Артефакт под ревью:** `docs/specs/213-device-marker-geometry.md`
|
||||
- **Трек:** обычный (не `small`/`trivial` — подтверждено аналитикой в issue:
|
||||
сложность 7/10, несколько поверхностей, изменение принятого визуального
|
||||
контракта, влияние на touch-hover; наличие файла в `docs/specs/` корректно).
|
||||
- **Ревьюер:** Claude, ревью ТЗ (роль отделена от автора — Codex).
|
||||
|
||||
## Вердикт
|
||||
|
||||
**Зелёный.** High: 0 · Medium: 0 · Low: 1 (снят с записью, см. ниже).
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Проверено: тело issue #213 и все три комментария (аналитика, occupancy,
|
||||
готовность ТЗ), ТЗ `docs/specs/213-device-marker-geometry.md` целиком
|
||||
(§1–23), обязательные разделы по PROCESS.md §7.1, соответствие
|
||||
`docs/SCOPE.md` (J1/J2/J7), `docs/UX-MODES.md` (pointer modality/hover
|
||||
ownership), `docs/TOUCH-SUPPORT.md` (hover — mouse-only, instance-local),
|
||||
`docs/USER-GUIDE.ru.md` (терминология LQI/hover/lock), а также фактическое
|
||||
состояние затронутого кода — не как источник вердикта по AC (это код-ревью),
|
||||
а чтобы отличить обоснованное техническое утверждение ТЗ от неотмеченной
|
||||
догадки.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` (процесс, классы
|
||||
изменений, лимит циклов, шаблон вердикта).
|
||||
2. Прочитано тело issue #213 и все комментарии, включая явные решения
|
||||
владельца (6 пунктов) и итог аналитики («Продуктовые вопросы:
|
||||
отсутствуют. Владелец уже зафиксировал…»).
|
||||
3. Прочитан весь файл `docs/specs/213-device-marker-geometry.md`.
|
||||
4. Сверены количественные и технические утверждения ТЗ с фактическим кодом,
|
||||
чтобы отличить обоснованное решение от невыделенной догадки:
|
||||
- `src/styles.ts:1886,2569` — подтверждён отдельный поздний
|
||||
`--device-visual-factor: 0.9` в `.dev` и `.vacpuck` (§3, §8.1 ТЗ);
|
||||
- `src/styles.ts:2039` — подтверждён текущий MDI viewport
|
||||
`0.5 × --dev-size` (§8.2 ТЗ: переход на 0.55, что математически ровно
|
||||
+10%);
|
||||
- `src/houseplan-card.ts:17814-17818`, `src/styles.ts:764-789` —
|
||||
подтверждён «старый компактный круг» oplock и зелёный `#66d17a` для
|
||||
`locked` вместо чёрного/белого пакета #179 (§3, §10 ТЗ);
|
||||
- `src/logic.ts:8-11` (`lqiColor`, непрерывный HSL-градиент) и
|
||||
`src/device-presentation.ts:46-56` (`markerLqiColor`, три
|
||||
дискретные категории) — подтверждено расхождение, которое ТЗ §12
|
||||
предлагает устранить, возвращая маркер к формуле `lqiColor()`;
|
||||
- `src/device-pulse.ts:35-37` — подтверждено, что три цвета пакета #179
|
||||
(`#F0410C/#F0A00C/#1DC21D`) остаются семантическими цветами pulse/alert
|
||||
независимо от маркерного LQI, как утверждает §12 ТЗ («остаются semantic
|
||||
colors других состояний»);
|
||||
- `docs/specs/211-device-icons-visual-parity.md:159-164` — подтверждён
|
||||
источник «amber `#F0A00C`, owner override для Dark Unlock» и
|
||||
`docs/specs/179-device-icons-redesign.md:487-501` — источник термина
|
||||
«No-Blur» (§10.1 ТЗ), то есть оба не являются изобретёнными фактами;
|
||||
- `docs/specs/212-device-icons-polish.md:128,298,382` — подтверждён
|
||||
действующий контракт 44×44 CSS px, который ТЗ переносит как
|
||||
неизменяемый (§8.3, AC4);
|
||||
- `docs/USER-GUIDE.ru.md:853,916` — подтверждено текущее пользовательское
|
||||
описание («три фиксированных диапазона» у маркера против «градиент» у
|
||||
заливки комнаты), что ТЗ явно планирует привести к единой формуле и
|
||||
обновить в release-артефактах (§21).
|
||||
5. Проверена каждая из 13 AC на однозначность и наличие способа
|
||||
доказательства (unit/source, browser smoke, golden, review кода) — см.
|
||||
таблицу ниже.
|
||||
6. Проверено, что предположения автора (не наблюдаемые пользователем детали)
|
||||
явно вынесены в §23 «Принятые технические предположения» отдельным
|
||||
блоком, а не растворены в тексте как факт.
|
||||
|
||||
## Находки
|
||||
|
||||
Нет находок уровня High или Medium.
|
||||
|
||||
### Low — оставлено с запиской (не блокирует)
|
||||
|
||||
**L1. Не явно, распространяется ли рост MDI-viewport на +10% (§8.2) на
|
||||
внутренний глиф `.vacpuck`.** `.vacpuck ha-icon` использует отдельное
|
||||
отношение `puck-size × 0.68` (`src/styles.ts:2598`), которое ТЗ не
|
||||
упоминает явно ни в §8.2 (только «default, custom MDI и state-swapped
|
||||
glyph»), ни в списке «не входит в задачу» (§7). Формально vacuum puck
|
||||
упомянут в Scope (§6) только в контексте базового размера
|
||||
(«base-size resolution… including vacuum puck»), а не MDI-глифа — то есть
|
||||
при внимательном чтении вопрос закрыт (владелец говорил «MDI-иконку внутри
|
||||
круга», имея в виду круглый Icon-маркер, а не puck), но явная фраза
|
||||
«размер MDI-глифа vacuum puck не меняется» сняла бы двойное толкование.
|
||||
**Решение ревьюера:** снимается без правки ТЗ — довод §6 vs §8.2 достаточен,
|
||||
доказательство того же рода уже требуется по AC2 («не клиппится на
|
||||
минимальном размере»), а расширение или нерасширение puck-glyph не меняет
|
||||
ни один AC и не расширяет/сужает скоуп. Автор кода может подтвердить любое
|
||||
из двух прочтений комментарием в реализации без нового продуктового
|
||||
решения.
|
||||
|
||||
## Проверено и корректно
|
||||
|
||||
- **Обязательные разделы §7.1 присутствуют**: сценарий/персона (§1), что
|
||||
человек увидит до/после (§2), проблема (§3), scope/не-scope (§6-7),
|
||||
контракт поведения (§8-12), UX/a11y/touch (§13), данные/миграция/i18n
|
||||
(§14), архитектурный контракт (§15), edge cases (§16), AC1-13 с
|
||||
доказательством (§17), план автотестов (§18), риски (§19), откат (§20),
|
||||
release-артефакты (§21).
|
||||
- **Каждый AC однозначен и несёт способ доказательства** — AC1 (size)
|
||||
unit+browser matrix+reference; AC2 (glyph) source+smoke+reference; AC3
|
||||
(concentric) DOM+pixel-centroid smoke, red-before-fix; AC4
|
||||
(anchor/parity) unit+smoke; AC5/AC6 (opening lock/security)
|
||||
visual matrix+существующие lock smokes; AC7/AC8 (hover/action boundary)
|
||||
real PointerEvent smoke; AC9/AC10 (LQI color/only-color) pure tests +
|
||||
existing room fill regression; AC11 (compat) serialization unit + review;
|
||||
AC12 (perf) source review + pre-beta smoke; AC13 (release artifacts) docs
|
||||
diff + `check-docs`.
|
||||
- **Технические утверждения не являются непроверенными догадками.** Каждое
|
||||
количественное расхождение, которое ТЗ приводит как «подтверждённая
|
||||
причина» (0.9-фактор, MDI 0.5, зелёный oplock.locked, три дискретных
|
||||
цвета LQI против непрерывного `lqiColor()`), прямо соответствует текущему
|
||||
коду — не изобретено и не оставлено голым утверждением.
|
||||
- **Технические предположения корректно отделены от продуктовых решений.**
|
||||
§23 содержит 7 пунктов, помеченных «принято предположительно, ревьюер
|
||||
может оспорить», и все семь — действительно ненаблюдаемые пользователем
|
||||
детали (footprint замка, unknown-asset, hover-vs-action границы,
|
||||
band/color separation, база нормализации, калибровка pixel-threshold,
|
||||
выбор layout-подхода), а не скрытые продуктовые решения.
|
||||
- **Отсутствие открытого продуктового вопроса обосновано, а не
|
||||
замалчивается.** Комментарий аналитики прямо указывает, что все шесть
|
||||
решений уже зафиксированы владельцем в теле issue до начала ТЗ — это
|
||||
единственный легитимный случай нулевого вопроса на сложной задаче
|
||||
(решение уже принято раньше, а не пропущено).
|
||||
- **Совместимость и откат.** §14 верно квалифицирует отсутствие
|
||||
миграции/новых полей (только фактический размер/цвет меняются, схема и
|
||||
сериализация — нет); §20 даёт однокоммитный откат без миграции данных.
|
||||
Соответствует `docs/CONFIG-COMPATIBILITY.md` (нет новых
|
||||
compatibility-полей, ничего материализуется при Open→Save).
|
||||
- **Touch/UX-контракт согласован с канонами.** §11/§13 корректно
|
||||
переносят инвариант `docs/TOUCH-SUPPORT.md`/`docs/UX-MODES.md`: hover
|
||||
instance-local, только настоящая мышь, touch/pen мгновенно снимает,
|
||||
compatibility-мышь не восстанавливает без нового реального ввода;
|
||||
редактор устройств — best effort, View/kiosk — блокирующий.
|
||||
- **Lock invariant не затронут.** §10.2 сохраняет
|
||||
`docs/SCOPE.md` «Лок инвариант»: клик по значку замка только открывает
|
||||
карточку, unlock/lock — только явной кнопкой с подтверждением; AC6
|
||||
отдельно доказывает это существующими lock-smokes.
|
||||
- **Трассируемость.** Issue ↔ ТЗ ссылки в обе стороны на месте; трек и файл
|
||||
ТЗ выбраны верно (обычный трек, не `small`).
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не проверялся реальный код реализации — на этом этапе (S4-spec-review)
|
||||
кода ещё нет, продуктовый код не менялся.
|
||||
- Не прогонялись гейты/тесты/смоки — этап ТЗ этого не требует; гейты
|
||||
относятся к код-ревью (PROCESS.md §2.7, §8).
|
||||
- Не проверялась точность списанных в §23 числовых threshold'ов
|
||||
(«один layout quantum», «0.5 физического пикселя») с точки зрения
|
||||
реальной калибровки — это по определению откладывается на
|
||||
failing-before-fix/mutant доказательство в реализации (сам ТЗ явно это
|
||||
признаёт в §23 п.6), а не на утверждение, которое можно проверить чтением
|
||||
ТЗ.
|
||||
- Отсутствие строки #213 (и #212) в таблице `docs/specs/README.md` не
|
||||
рассматривалось как находка: колонка/таблица помечена в PROCESS.md §7.3
|
||||
как известный расходящийся артефакт на списание, не входит в обязательный
|
||||
DoR-чеклист §2.5, и тот же пробел уже есть у #212 — не регрессия,
|
||||
внесённая этим ТЗ.
|
||||
|
||||
## Итог
|
||||
|
||||
ТЗ #213 добросовестно переводит шесть решений владельца в проверяемый
|
||||
контракт, корректно отделяет продуктовые решения от технических
|
||||
предположений, и все количественные утверждения о текущем поведении
|
||||
подтверждаются чтением актуального кода, а не являются непроверенными
|
||||
догадками. Единственная находка — Low, снята с запиской, не требует правки
|
||||
ТЗ. Задача готова перейти в `S5-ready`.
|
||||
Reference in New Issue
Block a user