diff --git a/docs/reviews/SPEC-REVIEW-505-r1.md b/docs/reviews/SPEC-REVIEW-505-r1.md new file mode 100644 index 00000000..bad7262b --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-505-r1.md @@ -0,0 +1,290 @@ +# SPEC-REVIEW-505-r1 + +Issue: https://github.com/Matysh/houseplan-card/issues/505 +Стадия: ТЗ на ревью (`S4-spec-review`), полный трек (лёгкий отклонён аналитиком — +сложность/риск >3, три поверхности, поддерживаемый touch-контракт). +Заход: r1 (первый прогон ревью ТЗ для этой задачи). +Материал: `docs/specs/505-summary-panel-design-parity.md` на SHA +`8defee4ce1cb0f2794495c03173348c7a6c34e13` (ветка +`issue/505-summary-panel-polish`, коммит «docs: specify summary panel designer +parity»). Рабочая копия уже стоит на этом SHA. + +## 1. Скоуп ревью + +Задача расширена владельцем 2026-09-09: к четырём исходным UX-исправлениям +(плавное скрытие панели, широкий диалог настроек, удаление `Sizes on this +screen`, зависимость mobile-опции от master-переключателя) добавлено полное +визуальное/поведенческое соответствие трёх поверхностей (шапка, панель, диалог +настроек) прототипу Dashboard 5 из архива `макет.zip` +(`sha256:bc754c16c9…357e15`). + +Ревью охватывает: +- само ТЗ `docs/specs/505-summary-panel-design-parity.md`; +- вспомогательные документы `docs/design/505-summary-panel/**` (референс, + handoff, README); +- обновление индекса `docs/specs/README.md`; +- соответствие ТЗ телу issue #505 и обоим комментариям аналитики (r1 «первая + версия задачи», r2 «возобновление после расширения scope»); +- соответствие `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` §7.1/§7.2/§5, + `docs/USER-GUIDE.ru.md`, `docs/TOUCH-SUPPORT.md`, `docs/CONFIG-COMPATIBILITY.md`. + +Это первый заход — раздела «Унаследовано из r0» не требуется, разбор полный. + +## 2. Как проверялось + +- Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` целиком (включая §2.4, + §2.10, §4, §5, §7.1, §7.2, §9) перед оценкой. +- Прочитано тело issue #505 и все три комментария (аналитика r1, пауза + владельца, возобновление аналитики r2 после расширения scope). +- Прочитан ТЗ-документ целиком (`git show + origin/issue/505-summary-panel-polish:docs/specs/505-summary-panel-design-parity.md`). +- Проверены фактические утверждения ТЗ о текущем состоянии `dev` чтением кода: + - `renderPanel()` в `src/summary-panel-runtime-loaded.ts:182-192` действительно + возвращает `nothing` сразу при `!visible` — exit-анимация невозможна, + подтверждает описанную причину дефекта №1; + - `src/summary-panel-style.ts` содержит только `@keyframes hp-summary-in-right` + и `hp-summary-in-bottom` (строки 144–152) — входных keyframes для скрытия нет; + - `min-width` в `src/summary-panel-style.ts:156` навешен на `.summary-editor` + (внутренний контент), а не на оболочку `hp-dialog` — подтверждает причину + переполнения диалога; + - `src/summary-panel-editor.ts` не связывает `disabled` mobile-чекбокса + (строка ~102) с `dialog.localShow` — подтверждает независимость от + master-переключателя; + - `summary-sizes-title`/`summary.sizes_title` (строка 58) по-прежнему + рендерится — раздел `Sizes on this screen` не удалён. + Все пять фактических предпосылок ТЗ верны на названном SHA. +- Проверен механизм wide-диалога: `--hp-dialog-wide-width` уже используется + (`src/styles/dialogs.styles.ts:1592`, `device-inbox-dialog`), `hp-dialog.ts:163` + читает эту переменную для `.surface` при `wide` — предпочтение ТЗ «переиспользовать + системный wide-dialog contract» технически реализуемо без новой инфраструктуры. +- Проверено соответствие терминологии `docs/USER-GUIDE.ru.md` (раздел «Сводная + панель», строки 261–314) — используемые в ТЗ названия настроек и поведение + совпадают с зафиксированным пользовательским контрактом (общий/локальный + scope настроек, ресайз-скрытие, mobile-политика). Обновление самого гайда + корректно отнесено к §9 «Release artifacts» реализации, а не к этапу ТЗ. +- Проверено `docs/TOUCH-SUPPORT.md:48-56` — сводная панель и её простая форма + настроек явно объявлены полноценной View-поверхностью с 44×44 px целями; + требования ТЗ по touch-таргетам этому не противоречат. +- Проверено `docs/CONFIG-COMPATIBILITY.md:100-132` («Summary panel namespace + (#437)») — утверждение ТЗ «Config namespace stays version1; compatibility + registry/storage shapes do not change» и утверждение, что локальные + size-preferences — браузерные runtime-данные вне серверного конфига, + соответствуют канону; удаление UI-раздела `Sizes on this screen` не требует + правки этого документа. +- Проверена целостность референс-архива: `docs/design/505-summary-panel/reference/` + содержит воспроизводимый `index.html`, CSS/JS/SVG и три handoff-документа; + README задачи и README референса указывают SHA-256 архива и совпадают между + собой и с телом issue. +- Сверено обновление `docs/specs/README.md` — строка для #505 добавлена. +- Прогнан `node scripts/process-gate.mjs --range origin/dev..origin/issue/505-summary-panel-polish` + → «гейт пройден, предупреждений 0» (офлайн-часть; статус issue не проверялся + флагом `--issues`, т.к. факт статуса уже виден в метках issue: `S4-spec-review`, + без `blocked`). +- Проверены трейлеры коммита `8defee4c`: `Issue: #505`, `User-Visible: no` — + корректно для документации без изменения продуктового поведения. +- Проверен CI: `gh run list --branch issue/505-summary-panel-polish` → + прогон `34316129681`, `completed/success`, `headSha == 8defee4ce1cb…` — точный + SHA материала. (Уточнение к вводным этого ревью: там указано, что зелёного + Validate на этом SHA не найдено — это было неверно, прогон найден и зелёный; + дешёвые гейты дополнительно вручную не гонялись, так как класс изменений — + только C (документация), продуктовый код не тронут, и зафиксированный CI + прогон уже покрывает `docs`/`process-gate`/`provenance`.) +- Сверена диффа `git diff origin/dev..origin/issue/505-summary-panel-polish + --stat` — только `docs/**`, класс C; продуктовый код (`src/**`, + `custom_components/**`) не изменён, что и требуется на этапе ТЗ. + +## 3. Находки + +### Medium — отсутствуют обязательные первые разделы «Сценарий» и «Что человек увидит до и после» (PROCESS.md §7.1) + +`docs/specs/505-summary-panel-design-parity.md`, раздел 1 «Problem, value and +scope» объединяет проблему, ценность и скоуп, но нигде явно не называет: какая +персона (`docs/SCOPE.md`: home admin / household members / kiosk) на какой +поверхности и в какой момент встречает это изменение, и не даёт отдельной фразы +«что человек увидит до и после» без терминов реализации. PROCESS.md §7.1 требует +эти два раздела **первыми** и явно: «ТЗ, которое не может ответить на эти два +вопроса, описывает работу, а не изменение продукта». Это не абстрактное +требование формата — непосредственно предшествующий и тематически связанный ТЗ +того же автора, `docs/specs/493-summary-panel-hardening.md` (issue #493, та же +подсистема, тот же день), содержит именно эти два раздела первыми +(`## 1. Сценарий`, `## 2. Что человек увидит до и после`, строки 11 и 24), то +есть автору конвенция известна и обычно соблюдается — здесь она пропущена +целиком, а не сокращена. + +**Сценарий проявления:** ревьюер (или будущий читатель ТЗ, включая ревьюера +кода на следующем этапе) не может быстро проверить, что изменение решает +заявленный пользовательский сценарий, а не просто список визуальных +требований — именно такой класс дефекта («AC выполнены, но сценарий не решён») +явно разрешён как основание для жёлтого вердикта в PROCESS.md §2.4/§7.2. +Материал, впрочем, косвенно восстановим (персоны и поверхности разбросаны по +§2–3 и по комментарию аналитики), поэтому находка — дефект формы и +трассируемости, а не скрытая двусмысленность поведения; поэтому Medium, а не +High. + +**Как чинится:** добавить в начало документа (перед текущим «1. Problem, value +and scope», со сдвигом нумерации) два раздела по образцу #493 — «Сценарий» +(admin/household member/kiosk-пользователь на View/kiosk/диалоге настроек, в +момент открытия карточки, включения показа или редактирования блоков) и «Что +человек увидит до и после» (одной фразой, без терминов реализации: сейчас +составной контрол и панель визуально не совпадают с согласованным прототипом +и не позволяют держать диалог настроек без горизонтальной прокрутки; после — +шапка/панель/диалог выглядят и ведут себя как согласованный дизайн, функционал +не теряется). + +### Medium — отсутствует обязательный раздел «Принятые технические предположения» (PROCESS.md §7.1/§7) + +ТЗ #505 не содержит финального блока «принято предположительно, поменять +свободно», хотя делает по ходу текста ряд односторонних технических решений, +которые пользователь не наблюдает и которые PROCESS.md §7 прямо требует +выносить в такой явный блок, чтобы ревьюер мог их оспорить: точное имя CSS +custom property для реального HA-диалога (`--ha-dialog-width-md`, §6, +помечено «verify actual pinned HA frontend contract», то есть уже осознаётся +как непроверенное), состав и расположение новых файлов реализации (§7, +«Expected files: …»), новые i18n-ключи и их точные имена (§6), решение +переиспользовать `--hp-dialog-wide-width` вместо отдельного механизма. Ни +один из этих пунктов не проходил как продуктовый вопрос владельцу (и не +должен был), но без явного блока они читаются как окончательные решения ТЗ, а +не как предположения, которые исполнитель волен скорректировать при сохранении +AC. Тот же #493 (строки 462 и далее, «## 20. Принятые технические +предположения», 7 пунктов) — прямой прецедент того же автора для той же +подсистемы, показывающий, что формат достижим без дополнительной информации: +большая часть содержимого уже есть в тексте ТЗ, недостаёт только явного +заголовка и итоговой фразы «эти пункты не меняют видимый контракт и могут +быть скорректированы ревьюером при сохранении AC». + +**Как чинится:** добавить раздел «Принятые технические предположения» в конец +документа (после текущего раздела 9), перечислив как минимум: точное имя +CSS-переменной ширины real-HA диалога подлежит проверке при реализации; состав +файлов из §7 ориентировочный; конкретные ключи i18n, кроме перечисленных как +обязательные, могут быть переименованы при сохранении семантики; способ +хранения generation-guard для анимации (упомянут в §4, но не в отдельном +пункте — имя/модуль структуры state machine не названы явно как свободные). + +### Low — неполный список визуальных пар в разделе доказательств (§8) + +Абзац «Visual evidence is mandatory» в конце §8 перечисляет +«control+open right panel and settings, dark desktop, bottom portrait, kiosk; +plus narrow native/real HA settings screenshots». Чек-лист самого issue +(«Дополнительные критерии приёмки», пункт 4) требует пары «desktop light/dark, +bottom, kiosk, mobile» — пять явно названных контекстов. В тексте ТЗ явно +назван «dark desktop», но не назван отдельно «light desktop» (light, +предположительно, подразумевается базовым/неотмеченным вариантом «control+open +right panel and settings»), и «mobile» из чек-листа issue визуально не +отделён от «narrow native/real HA settings screenshots» — неясно, требуется ли +отдельная пара для мобильного отображения самой панели (не только диалога +настроек) в узкой View. Это не блокирует понимание общего требования (AC10 +дополнительно требует смок-проверку 320/390/desktop, light/dark, RU/EN/DE/FR), +и не меняет ни одного AC по существу, поэтому не блокирует зелёный вердикт — +достаточно явно перечислить все пять контекстов из чек-листа issue при правке +двух Medium-находок выше, либо оставить как есть с запиской, что реализация +интерпретирует this list по образцу issue буквально. Снимается автором на своё +усмотрение с пометкой в ТЗ. + +## 4. Что проверено и корректно + +- **Формат этапа:** issue не помечен `small`/`trivial`, полный трек выбран и + обоснован дважды (первичная аналитика — сложность/риск >3 и touch-контракт; + повторная аналитика после расширения scope — три поверхности плюс touch), + файл ТЗ существует по правильному пути и имени + (`docs/specs/505-summary-panel-design-parity.md`), ссылки issue↔ТЗ на месте. +- **Продуктовая рамка:** задача остаётся внутри одобренного исключения #437 из + `docs/SCOPE.md` (read-only summary overlay); ТЗ явно перечисляет, что НЕ + переносится из демо (fake data, localStorage, HA-хром, сворачивание блоков, + отдельные экранные размеры) — совпадает и с §2 SCOPE.md, и с телом issue. +- **Приоритет источников истины** задан явно и в правильном порядке: owner + requirements > текущие контракты House Plan (данные/безопасность/a11y) > + визуальный референс > детали демо-реализации — снимает риск слепого + копирования прототипа поверх контрактов. +- **Технический анализ причин** всех четырёх исходных дефектов подтверждён + чтением актуального кода (см. §2 выше) — не голословен. +- **Явное разведение прототипной и продуктовой логики** там, где они + расходятся: `IMPLEMENTATION-NOTES.md` референса описывает выбор позиции по + `matchMedia("orientation: portrait")`, тогда как ТЗ §3 прямо фиксирует, что + действующий resolver «stage width >= height => right» авторитетен и **не** + является viewport-ориентацией — корректное решение конфликта в пользу + существующего контракта, а не слепое копирование демо. +- **AC1–AC11** пронумерованы, у каждого явно указан способ доказательства + (smoke/визуальная сверка/runtime-тест/gate), не перекрываются бессмысленно и + покрывают все пункты обоих чек-листов issue (четыре исходных + расширенные). + Для дорогих защитных проверок (AC3 exit-DOM-retention, AC5 shell-wide sizing) + явно требуется отрицательный свидетель и mutation-gate — соответствует + PROCESS.md §2.7 «чем краснеет». +- **Анимационная state machine** (§4) описана как явные состояния + hidden→entering→visible→exiting→hidden с generation-guard против «призраков», + явно перечисленными граничными случаями (reduced motion, resize/anchor change, + скрытый документ, смена identity, editor entry) — не оставляет открытых + вопросов по логике жизненного цикла. +- **Config-совместимость и данные** не меняются (namespace version 1, + local-only size prefs вне серверного конфига) — подтверждено + `docs/CONFIG-COMPATIBILITY.md`, миграция не требуется, откат тривиален. +- **i18n**: новые ключи перечислены явно (en/ru/de/fr), с условием на переиспользование + существующих и удаление `summary.sizes_title` только при отсутствии других + потребителей — проверяемо статическим поиском на этапе код-ревью. +- **Touch/a11y**: 44×44 px, disabled-семантика, keyboard/focus traversal — + согласовано с `docs/TOUCH-SUPPORT.md`, который прямо относит эту поверхность + к гарантированному View-контракту, а не к best-effort editor. +- **Продуктовых вопросов владельцу не осталось** — оба комментария аналитики + подтверждают это, и текст ТЗ действительно не содержит скрытых догадок о + видимом поведении, выданных за факт (единственное место, где остаётся + неопределённость — имя HA CSS-переменной — прямо помечено как требующее + проверки, а не заявлено как факт). +- **Процесс/гейты:** коммит несёт корректные трейлеры, `process-gate.mjs` + проходит без предупреждений, CI зелёный на точном материале ревью, диапазон + диффа ограничен классом C (документация), продуктовый код не тронут — как и + должно быть на этапе ТЗ. +- **Индекс** `docs/specs/README.md` обновлён. + +## 5. Чего не проверял + +- Пиксельное сравнение реализации с прототипом — на этапе ТЗ реализации не + существует, сверка станет обязанностью код-ревью (AC1/AC2/AC6/AC10 явно + требуют парные скриншоты как доказательство). +- Тяжёлые браузерные гейты (`golden`, `smoke_*`, `performance_smoke`, + `pytest tests_backend`) не запускались — не относится к этапу ТЗ, продуктовый + код и тесты не изменены этим коммитом. +- Флаг `--issues` у `process-gate.mjs` не использовался (нужен токен с широким + доступом к Issues API сверх текущего); статус подтверждён напрямую через + `gh issue view` — ровно одна метка `S4-spec-review`, без `blocked`. +- Не проверялось содержимое самого архива `макет.zip` за пределами + зафиксированного SHA-256 и распакованных `docs/design/505-summary-panel/reference/**` + файлов — оригинальный архив в репозиторий не входит, воспроизводимость + обеспечена самими распакованными файлами. + +## 6. Вердикт + +High: 0. Medium: 2 (обе — дефект формата ТЗ по PROCESS.md §7.1, в скоупе +текущей задачи, чинятся правкой того же документа без нового технического +исследования). Low: 1 (снимается автором на своё усмотрение). + +Обязательные разделы присутствуют по существу содержания, но не по форме, +предписанной PROCESS.md §7.1 и подтверждённой прецедентом #493 того же автора; +это возврат на доработку ТЗ, а не отклонение — формат восстановим без нового +анализа. + +Вердикт: жёлтый · заход r1 · блокирующих циклов 1/4 · High: 0 · Medium: 2 → в задаче + +## Материал раунда + +- Ветка: `issue/505-summary-panel-polish` +- SHA материала: `8defee4ce1cb0f2794495c03173348c7a6c34e13` +- Дерево ТЗ: `docs/specs/505-summary-panel-design-parity.md` на указанном SHA +- Поиск при устаревании ветки: + `git log --all --format='%H %T' | grep <дерево HEAD^{tree}>` + `git log --all --find-object=<блоб docs/specs/505-summary-panel-design-parity.md> -- docs/specs/505-summary-panel-design-parity.md` + +--- + + + +## Материал раунда + +- Ветка: `issue/505-summary-panel-polish`, коммит `8defee4ce1cb` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `eddbf58ae62fc94849148a7d3881d5543cf16870` + ``` + git log --all --format='%H %T' | grep eddbf58ae62f + ``` +- ТЗ `docs/specs/505-summary-panel-design-parity.md`, блоб `61a52163e5a30f3b86f24de3ace9fbd6ebb4e0a5` + ``` + git log --all --find-object=61a52163e5a30f3b86f24de3ace9fbd6ebb4e0a5 -- docs/specs/505-summary-panel-design-parity.md + ``` +- Вердикт конвейера: `yellow` · High 0