Files
houseplan-card/docs/reviews/SPEC-REVIEW-213-r1.md
2026-08-20 07:25:11 +00:00

14 KiB
Raw Permalink Blame History

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.