diff --git a/docs/reviews/SPEC-REVIEW-211-r1.md b/docs/reviews/SPEC-REVIEW-211-r1.md new file mode 100644 index 00000000..c029d5f6 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-211-r1.md @@ -0,0 +1,226 @@ +# Ревью ТЗ — issue #211, цикл r1 + +- **Issue:** https://github.com/Matysh/houseplan-card/issues/211 +- **ТЗ:** `docs/specs/211-device-icons-visual-parity.md` (коммит `93200eb`, ветка + `issue/211-device-icons-visual-parity`) +- **Трек:** обычный (аналитика явно исключила `small`/`trivial`) +- **Вердикт:** жёлтый · цикл r1/4 · High: 0 · Medium: 1 (в скоупе задачи) · Low: 2 + +## Скоуп ревью + +Проверялось только ТЗ как артефакт этапа `S4-spec-review`: обязательные разделы +§7.1 PROCESS.md, однозначность и доказуемость AC1…AC10, соответствие +`docs/SCOPE.md` (J1/J2), `docs/USER-GUIDE.ru.md`, `docs/TOUCH-SUPPORT.md`, и +отдельно — фактическое соответствие числовых утверждений ТЗ нормативному +источнику: тому же архиву дизайнера, что и в #179, плюс текущей реализации в +`src/styles.ts` (описанной как «до»). Продуктовый код по #211 не существует +(issue не дошёл до `S5-ready`, диапазон `origin/dev..HEAD` содержит только +`docs/specs/211-*.md` и правку `docs/specs/README.md`), поэтому код не +проверялся и гейты §8 PROCESS.md к этому циклу неприменимы. + +## Как проверялось + +1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` целиком. +2. Прочитано тело issue #211 и оба комментария (аналитика + «ТЗ готово»), а + также issue #179 целиком — тело и все 15 комментариев обоих циклов ревью + ТЗ и код-ревью, чтобы знать, какие решения владельца уже приняты и что + именно было принято/исправлено там (`M1–M5`, `L1–L2` в + `SPEC-REVIEW-179-r1.md`/`r2.md`). +3. Прочитан файл ТЗ `211-device-icons-visual-parity.md` целиком (354 строки) и + `docs/specs/179-device-icons-redesign.md` целиком — как источник приоритета + №4 по собственной цепочке разрешения расхождений ТЗ #211. +4. **Архив дизайнера скачан заново и распакован** (`curl` по той же ссылке из + тела issue #179, `sha256sum` совпал с указанным в ТЗ #211 + `63670C73E25D1E59DDAF1BE236F3D7F2FAC827B9B5D6DD4B77125EA9BC012025`) — + независимая проверка, не пересказ ТЗ #179: + - подтверждена геометрия shell/core (``/`` в `Light/Dark Icon + Default*.svg`: shell — круг диаметром `101.5`, core — `` + `80×80`, отношение `1.26875`); + - подтверждён pill value-badge (`Double Default Right.svg`: `Frame_2` — + путь `y: 24…87`, высота `63 = 0.7875×80`, радиус скруглений `31.5 = + 0.39375×80`, то есть ровно половина высоты); + - подтверждены цвета состояний построчным `grep` по `fill`/`stroke` в + `Icon Hover/Active/Alert Value/Selected/Focus Visible/Lock/Unlock.svg` + для обеих тем; + - подтверждён `stroke="#252525" stroke-opacity="0.75"` в + `Dark/Icon Default No Blur.svg` (Dark shell default) и `stroke-dasharray + "6 6"` в `Virtual Device *.svg` (Virtual shell); + - подтверждена сама регрессия, зафиксированная в Dark/Unlock.svg архива + (`fill="#1DC21D"`, зелёный) — и то, что ТЗ №211 корректно, вслед за ТЗ + #179, отвергает этот файл в пользу решения владельца («Unlock — янтарный + в обеих темах»), а не проецирует его буквально; + - измерена реальная bounding box глифа (`svgpathtools`, путь `id="Vector"`) + в `Icon Default/Hover/Active/Default No Blur.svg` (общий «колокольчик») + и в `Lock.svg`/`Unlock.svg` — см. M1 ниже; + - проверено существование каждого документа, который ТЗ №211 §4 называет + нормативным источником (`SPECIFICATION.md`, `DEVELOPER_HANDOFF.md`, + `ACTIVE_ANIMATION_SPEC.md`, `COMPARISON_NOTES.md`) — см. L1. +5. Сверено с текущей реализацией `src/styles.ts` (строки 1860–2214): каждое из + шести «подтверждённых расхождений» ТЗ §3 сопоставлено построчно с кодом — + `border-radius: 28%` (:1961), `--mdc-icon-size: … * 0.62` (:1984), + value-section `border-radius: … * 0.18` при высоте `.7875` (:2180,:2188), + безусловный `border: 1px solid #BCBCBC` без тёмного варианта (:1912). +6. Прочитаны релевантные фрагменты `docs/USER-GUIDE.ru.md` (термин «маркер», + отсутствие конфликтующей терминологии) и целиком `docs/TOUCH-SUPPORT.md` + («Documentation rule», канонические теги `Touch editor: …`). +7. Код не запускался, гейты не прогонялись — на этапе ревью ТЗ кода нет. + +## Находки + +### M1 — целевое соотношение «MDI viewport = 0.5 × core» не подтверждено ни одним источником архива и расходится с прямым измерением SVG + +**Файл:** `docs/specs/211-device-icons-visual-parity.md:44-45` (§3, п.2), +`:123` (§7.1, таблица), AC1 (`:241-244`) + +ТЗ утверждает как факт: «пакет использует `40 × 40` внутри core `80 × 80`, то +есть `0.5 × core`» и заносит `MDI viewport | width = height = 0.5` в таблицу +геометрического контракта §7.1 с допуском **не более 0,5 CSS px** на 32/56/96 +px (тот же допуск, что и для проверенно точных чисел вроде `1.26875` и +`0.7875`). Это число прямо используется в AC1 как критерий приёмки. + +Проверка источников: + +- `SPECIFICATION.md`, `DEVELOPER_HANDOFF.md`, `ACTIVE_ANIMATION_SPEC.md`, + `PACKAGE_ANNOTATION.txt` — ни один не называет размер/долю icon-viewport + внутри core; `SPECIFICATION.md` §8 говорит только о shell/core/border/shadow. +- Прямое измерение bounding box самого глифа (`svgpathtools`, путь + `id="Vector"`) в реальных SVG даёт **не 0.5**: `Icon + Default/Hover/Active/Default No Blur.svg` (общий глиф-колокольчик) — + `33.33 × 33.33` = `0.417 × core`; `Lock.svg`/`Unlock.svg` — + `26.67 × 35.0` = `0.333–0.4375 × core`. +- Единственное место архива, где встречается ровно `50%`, — файл + `Index.html` (`.new .glyph { width: 50%; height: 50%; }`), интерактивный + demo-компаратор «старый/новый» дизайн. Но: (а) он не входит в список + источников §4 ТЗ (там перечислены только owner-решения, три `.md`-документа, + «SVG архива» и ТЗ #179 — без `Index.html`); (б) сам этот демо-файл + использует приближённые круглые px-значения (`--core-size: 42px` при + `--s: 56px`, то есть core/marker `≈0.75`, тогда как в реальных экспортных + SVG это отношение `0.7882` — расхождение ~5%), то есть он иллюстративный + прототип, а не пиксель-точная копия экспортных assets. + +Итог: `0.5` — правдоподобное **производное** значение (bbox глифа + типичный +внутренний padding самих MDI-иконок ≈10–15% могли бы дать assigned-frame +≈39–41), но ТЗ подаёт его как прямую цитату архива наравне с числами, которые +я независимо проверил и подтвердил буквально (`1.26875`, `0.7875`, +`0.39375`, все цвета состояний). Ни один текстовый документ этого числа не +называет, а единственный SVG-источник с буквальным `50%` не упомянут в §4 и +сам не является пиксель-точным. + +**Почему это важно:** AC1 — единственный количественный критерий именно для +формы/масштаба глифа, то есть сердцевина задачи «маркер визуально совпадает с +пакетом». Раздел §11 требует построить независимую таблицу «Reference SVG ↔ +Runtime» именно для того, чтобы новый golden не мог сам стать себе эталоном +(это и есть причина, по которой заведён #211). Если строка «Reference SVG» +для glyph viewport унаследует непроверенное число `0.5`, фикстура будет +защищать неверную/неподтверждённую цель с той же уверенностью, что и +корректные строки — то есть повторит тот самый механизм регрессии, из-за +которого возник этот баг. + +**Требуется** одно из: +1. явно сослаться на `Index.html` как источник и показать расчёт (bbox + + допущение о внутреннем padding MDI) в §4/§7.1, чтобы ревьюер кода видел + основание числа, а не готовый вывод; либо +2. перенести именно эту строку в §16 «Принятые технические предположения» — + как единственную геометрическую величину, не имеющую прямого текстового + или однозначного SVG-подтверждения, — и явно разрешить реализации/ревью + скорректировать её при визуальном сравнении с реальным MDI-глифом (в + отличие от статичного примера-«колокольчика» из пакета). + +Находка в скоупе задачи (это её собственный AC1); блокирующей не является — +одна Medium без High даёт жёлтый вердикт и правку в этом же ТЗ. + +### L1 — §4 называет нормативным источником несуществующий файл `COMPARISON_NOTES.md` + +**Файл:** `docs/specs/211-device-icons-visual-parity.md:62-63` + +Порядок приоритета источников перечисляет «`SPECIFICATION.md`, +`DEVELOPER_HANDOFF.md`, `ACTIVE_ANIMATION_SPEC.md` и `COMPARISON_NOTES.md` +архива #179». В реально скачанном и распакованном архиве (тот же SHA-256, +что и в ТЗ) присутствуют `README.md`, `SPECIFICATION.md`, +`DEVELOPER_HANDOFF.md`, `ACTIVE_ANIMATION_SPEC.md`, `PACKAGE_ANNOTATION.txt`, +`manifest.json`, `Index.html` — файла `COMPARISON_NOTES.md` нет и не было ни +в одной версии архива, которую видел я или которую цитировал разбор #179. + +Не блокирует ни один AC (все содержательные факты, которые могли бы там +быть, взяты из реально существующих документов и подтверждены), но при +реализации кто-то потратит время, разыскивая несуществующий файл как +«третий по приоритету» источник истины. Снять правкой — убрать имя файла из +списка либо заменить на реально существующий (вероятно, опечатка/перенос из +не-#179 контекста). + +### L2 — обязательный тег `Touch editor: …` присутствует по смыслу, но не дословно + +**Файл:** `docs/specs/211-device-icons-visual-parity.md:183-186` (§8) + +`docs/TOUCH-SUPPORT.md` («Documentation rule») требует, чтобы спецификации, +затрагивающие редактор, буквально называли один из трёх канонических тегов: +`Touch editor: supported` / `best effort / intentionally degraded` / `not +exposed`. §8 ТЗ №211 говорит «Device editor остаётся desktop-first / touch +best effort» — по смыслу корректно (совпадает с уже принятым в #179 «best +effort / intentionally degraded») и задача не меняет интерактивность +(в отличие от #179, где отсутствие этого тега было Medium именно потому, что +там вводился новый keyboard/focus контракт), поэтому не блокирует. Для +единообразия с остальными спецификациями и grep-совместимости стоит заменить +на буквальный `Touch editor: best effort / intentionally degraded.` — на +решение автора. + +## Проверено и признано корректным + +- **SHA-256 архива** совпадает с указанным в ТЗ; архив идентичен тому, что + проверялся в цикле #179. +- **Геометрия shell/core** (`1.26875`) и **value-badge pill** (`0.7875` + высота, `0.39375` радиус = половина высоты) — оба подтверждены прямым + измерением реальных ``/`` координат, не переписаны на глаз. +- **Описание регрессии** (§3, пп. 1, 3, 4) сверено построчно с текущим + `src/styles.ts` и подтверждено буквально: `border-radius: 28%` (core не + круг), `border-radius: calc(... * 0.18)` при pill-высоте `0.7875` (радиус + должен быть `0.39375`), безусловный светлый `#BCBCBC` shell-stroke без + тёмного варианта. +- **Цвета состояний** (§7.3): hover `#0C82F0`, active/unlock `#F0A00C`, + alert `#F0410C`, Dark shell `#252525`/`opacity .75`, virtual + `stroke-dasharray 6 6`, selected `#F0A00C` ring `r=41.5`, focus `#0C82F0` + ring `r=41.5` поверх видимого base shell — все сверены построчным `grep` + по реальным SVG обеих тем и совпадают с таблицей §7.2/§7.3. +- **Корректно разрешённая ошибка пакета**: `Dark/Unlock.svg` в архиве + реально закрашен зелёным (`#1DC21D`, устаревшая dark-ревизия) — ТЗ №211 + правильно, вслед за уже принятым решением владельца в #179, отвергает этот + файл в пользу «янтарный в обеих темах», а не проецирует его буквально. +- **Обязательные разделы §7.1 PROCESS.md** — все присутствуют: сценарий, что + видит пользователь до/после, проблема, скоуп/не-скоуп, контракт поведения, + UX/touch/доступность, данные/совместимость/i18n, AC1…AC10 с доказательством, + план автотестов, риски, откат, release-артефакты. +- **Скоуп корректно ограничен** восстановлением уже принятого дизайна #179 — + никаких новых состояний, resolvers, LQI-порогов, actions или config-модели + не вводится (§6); это отличает задачу от #179 и обосновывает отсутствие + открытых продуктовых вопросов владельцу без `blocked`. +- **Трассируемость**: `docs/specs/README.md` обновлён тем же коммитом с + двусторонней ссылкой issue↔ТЗ; коммит `93200eb` несёт трейлеры + `Issue: #211` и `User-Visible: no` — корректно для документации без + изменения продукта. +- **Секьюрность и touch-инвариант** не задеты: §8 явно называет secure + confirmation/keyboard/dialog focus return как поведение, которое обязано + остаться без изменений, а не переопределяет его. + +## Чего не проверял + +- **Figma-фреймы** (`99:1290`, `104:1539`) напрямую не открывались — ревью + ограничено содержимым скачанного архива, тем же источником, которым + пользовался автор ТЗ. +- **Полная построчная сверка всех Double/Text/LQI SVG всех четырёх сторон** — + выборочно проверены `Double Default Right.svg` (value pill) и + сопутствующий `Frame_2`; остальные три стороны и `Text *.svg` не + измерялись отдельно, поскольку геометрия pill и текста не зависит от + стороны по самому контракту §7.1 ТЗ. +- **PNG-визуализации архива** не открывались — не источник геометрических + фактов, только иллюстрация. +- **Производительность** не измерялась (кода нет). +- Гейты `typecheck`/`test`/`build`/smoke/golden не запускались — на этапе + ревью ТЗ продуктового кода не существует, раздел неприменим. + +## Итог + +Находка блокирующей силы не имеет: единственный Medium (M1) устраняется +точечной правкой текста ТЗ — либо явной ссылкой на источник и расчёт, либо +переносом числа в блок технических предположений §16, — без изменения +скоупа, AC или архитектурного решения. Два Low — на усмотрение автора. +Вердикт — жёлтый, возврат автору в рамках текущего issue.