diff --git a/docs/reviews/SPEC-REVIEW-213-r1.md b/docs/reviews/SPEC-REVIEW-213-r1.md new file mode 100644 index 00000000..821df5c2 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-213-r1.md @@ -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`.