diff --git a/docs/reviews/SPEC-REVIEW-192-r1.md b/docs/reviews/SPEC-REVIEW-192-r1.md new file mode 100644 index 00000000..910987fd --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-192-r1.md @@ -0,0 +1,223 @@ +# SPEC-REVIEW-192-r1 + +- **Issue:** https://github.com/Matysh/houseplan-card/issues/192 +- **ТЗ под ревью:** тело issue #192 (лёгкий трек, файл `docs/specs/` не + создаётся — метка `small` подтверждена) +- **Роль:** ревьюер ТЗ (не автор), этап `S4-spec-review` +- **Трек:** лёгкий (`small`), лимит циклов ревью ТЗ — 2 (§4 PROCESS.md) +- **Цикл:** r1/2 + +## Скоуп ревью + +ТЗ #192: шкала «Оттенок» в общем компоненте `hp-color-opacity` (используется +в 8 call sites House Plan) получает полноценный циклический hue-gradient на +треке вместо нативной серой дорожки; `accent-color` продолжает управлять +только thumb. Изменение затрагивает один файл (`src/hp-color-opacity.ts`), +без миграции, i18n, backend или расширения touch-контракта. + +Не в скоупе ревью: продуктовый код не написан (issue в `S4-spec-review`; +ветки `issue/192-*` в репозитории нет — проверено `git branch -r`), поэтому +реализация, тесты и гейты — предмет будущего код-ревью (PROCESS.md §2.7). + +## Как проверялось + +1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` целиком (актуальная + редакция, включая §2.4/§2.5/§5/§7.1/§8). +2. Прочитано тело issue #192 и единственный комментарий — аналитика + владельца (`Matysh`, `authorAssociation: OWNER`, 2026-08-19): оценка + 4/10 · 2/10 · 3/10 · P3 · `feature`/`polish`, обоснование легитимности + лёгкого трека, явное решение владельца по опциональному объёму + («меняется только шкала «Оттенок»; отдельные градиенты S/V/opacity не + нужны — это принятое предположение, продуктового вопроса нет»). +3. Прочитан `src/hp-color-opacity.ts` целиком (741 строка) и построчно + сверены технические утверждения ТЗ: + - Проблема (§«Проблема» ТЗ) подтверждена: `.hue-range` (строка 234-236) + содержит только `accent-color: var(--hp-picker-hue, #f00)`, трек + остаётся нативным серым; `input[type='range']` (строка 225-233) задаёт + `height: 40px` — заявленная в ТЗ существующая интерактивная область + подтверждена, а не выдумана. + - `.hue-range` — единственный range с этим классом; `saturation`/`value` + (строки 682, 689) и `opacity` (707) не имеют отдельного класса — + scope `.hue-range`-селектором из §«Скоуп»/Risks реально изолирует + только hue-трек. + - `accent-color: var(--hp-picker-hue, ...)` и переменная `hueColor` + (строка 653, 676) — существующий механизм, ТЗ верно ссылается на его + сохранение, а не переизобретает. +4. Проверено, нет ли других hue-подобных range-элементов вне + `hp-color-opacity.ts`, которые ТЗ должно было бы затронуть, но не + затрагивает (по прецеденту Medium-1 из `SPEC-REVIEW-57-r1`, где ТЗ #57 + молча пропустило часть call sites): `grep -rn "type=\"range\"" src/*.ts` + вне `hp-color-opacity.ts` находит только `_rangeInput()` + (`houseplan-card.ts:14482-14495`, обобщённый opacity/numeric slider для + диалога «Общие настройки», без hue) и `.colorrow input[type='range']` + (`styles.ts:2157`, тот же класс). Ни один не является hue-контролом — + скоуп ТЗ (только `.hue-range` внутри `hp-color-opacity`) реально полон, + пропущенных hue-поверхностей нет. +5. Прочитан `test/color-picker.test.mjs` (50 строк) и `demo/smoke_color_picker.mjs` + (144 строки) — оба существуют, названы в плане автотестов точно; текущий + smoke уже проверяет `hue.value = '120'` → `picker.color === '#00ff00'` и + `shiftArrowUsesTenStep` — заявленные в AC2 факты («120° даёт зелёный», + «Shift+Arrow меняет на 10°») подтверждены существующим кодом теста, а не + придуманы для ТЗ. +6. Проверен `demo/golden/matrix.mjs`: сцена `decor-color-popover-mobile-ru` + (строка 269, `theme: 'dark'`) существует и совпадает с описанием ТЗ; + отдельной light-сцены этого picker пока нет — ТЗ корректно относит её к + «при необходимости» в разделе «Затронутые файлы», а не выдаёт за уже + существующую. +7. Прочитан `docs/USER-GUIDE.ru.md:189-193` — термин «Оттенок» и описание + палитры («Оттенок, насыщенность, яркость, точное значение HEX и + прозрачность... доступны вместе») совпадают с текстом ТЗ; обещанное + release-уточнение («шкала оттенка показывает spectrum») не противоречит + и не переопределяет существующую формулировку. +8. Прочитан `docs/TOUCH-SUPPORT.md`, раздел «Documentation rule» + (строки 142-154): требование буквальной строки `Touch editor: …`. ТЗ + содержит `### Performance и touch` → «**Touch editor:** picker + поддерживается как в #57; hit area и события не меняются» — по факту + покрывает требование (см. «Что проверено» ниже). +9. Проверен прецедент `SPEC-REVIEW-137-r1.md` (Low-4): golden-харнесс + (`demo/golden/matrix.mjs`/`harness.mjs`) параметризует тему только как + `light`/`dark` (`emulateMedia({ colorScheme })`), отдельного + `forced-colors` emulation-пути нет — сверено с тем же выводом, актуальным + и для AC3 этого ТЗ (см. Low-1 ниже). +10. Проверены `docs/CHANGELOG.md`, `docs/CHANGELOG.ru.md` — существуют, + ссылки ТЗ на release-артефакты не ведут в никуда. +11. Проверено, что issue не помечен `trivial`: тип `feature`/`polish`, а не + `bug` — `trivial` неприменим по критериям §5.1 корректно, как и пишет + сам автор аналитики. + +Гейты (`typecheck`/`test`/`build`) не прогонялись — на этапе ревью ТЗ +продуктового кода нет; прогон гейтов не относится к этому этапу (PROCESS.md +§2.7/§8). + +## Обязательные разделы (§7.1 PROCESS.md, лёгкий трек — тело issue) + +| Раздел | Есть | Комментарий | +|---|---|---| +| Сценарий (персона/поверхность/момент) | ✅ | персона, поверхность, момент названы явно | +| Что человек увидит до/после | ✅ | одна фраза «До»/«После», без терминов реализации | +| Проблема | ✅ | подтверждена чтением `hp-color-opacity.ts` (см. «Как проверялось» п.3) | +| Скоуп / не-скоуп | ✅ | скоуп полон — подтверждено отсутствием иных hue-поверхностей (п.4) | +| Контракт поведения и UX | ✅ | включает touch-строку (п.5 контракта) | +| Модель данных и миграция | ✅ | «нет изменений», обоснованно для presentation-only CSS | +| i18n | ✅ | «новых строк нет», используется существующий ключ | +| AC1…ACn с доказательством | ✅ | 5 штук, у каждого назван способ доказательства | +| План автотестов | ✅ | unit/smoke/golden по отдельности, ссылается на реальные файлы | +| Риски | ✅ | таблица риск/мера, 4 строки, включая thumb-контраст и селектор-scope | +| Откат | ✅ | revert engine-specific стилей, без миграции | +| Release-артефакты | ✅ | commit/changelog/USER-GUIDE/golden — все ссылки на существующие механизмы | + +Присутствует также блок «Принятые предположения», корректно отделяющий +технические допущения (равномерные CSS stops, Chromium-only browser smoke) +от уже принятого владельцем продуктового решения по объёму (см. «Как +проверялось» п.2). + +## Находки + +### Low-1 — AC3 не разделяет явно, какая часть критерия доказывается golden, а какая — только чтением кода + +**Файл:** тело issue #192, раздел «Критерии приёмки», AC3. + +AC3 требует одновременно: (a) spectrum/thumb читаемы в light/dark, (b) +forced-colors не получает нечитаемую заливку, (c) reduced-motion не +меняется — и называет одно доказательство на все три: «dark/light golden +candidates + source-contract/code review». Как подтверждено в «Как +проверялось» п.9, golden-харнесс параметризует тему только как +`light`/`dark`; отдельного `forced-colors` emulation-пути нет. Формально ТЗ +не лжёт: «dark/light golden» относится к пункту (a), а «source-contract/code +review» — к (b) и (c), но эта граница не проведена явно внутри самого AC3. +Тот же класс находки уже фиксировался как Low в `SPEC-REVIEW-137-r1` (Low-4) +и не блокировал приёмку. + +**Воспроизведение неоднозначности:** код-ревьюер, сверяя «AC3 доказан +golden-ом целиком?», должен самостоятельно вывести, что golden покрывает +только light/dark-часть, а `forced-colors`/`reduced-motion` проверяются +исключительно чтением CSS, а не эмуляцией браузера. + +**Решение ревьюера:** Low, не блокирует. При реализации/код-ревью явно +зафиксировать в хендоффе, что forced-colors и reduced-motion доказаны +«проверено чтением, не исполнением», а golden покрывает только light/dark +readability — снимаю находку без правки текста ТЗ, тем же способом, что и +прецедент. + +## Что проверено и корректно + +- **Соответствие `docs/SCOPE.md`:** задача — editor/settings usability + polish общего компонента, используемого в J4/J6-поверхностях + (настройка декора/комнат/устройств); не создаёт нового job'а, View/kiosk + не затрагиваются. +- **Легитимность лёгкого трека:** сложность 3/10, одна поверхность + (`hp-color-opacity.ts`), без миграции/i18n/backend/touch-контракта — + критерии §5 PROCESS.md выполнены все одновременно; `trivial` корректно + признан неприменимым (не `bug`). +- **Продуктовый вопрос закрыт до написания ТЗ.** Единственный потенциально + продуктовый вопрос — объём видимых изменений («только Hue или все шкалы») + — уже решён самим владельцем текстом комментария аналитики + («В рамках #192 меняется только шкала «Оттенок»... продуктового вопроса + владельцу нет»), а не додуман автором задним числом. +- **Технический диагноз проблемы не голословен** — подтверждён прямым + чтением `src/hp-color-opacity.ts:225-236` (см. «Как проверялось» п.3): + трек `.hue-range` действительно остаётся нативным серым, `accent-color` + красит только thumb. +- **Скоуп реально полон** — построчный поиск по `src/*.ts` не находит + других hue-подобных range-элементов вне `.hue-range`; в отличие от + прецедента `SPEC-REVIEW-57-r1` (Medium-1, пропущенные call sites), здесь + пропущенных hue-поверхностей нет. +- **Существующие тестовые файлы и golden-сцена, на которые ссылается ТЗ, + реальны** — `test/color-picker.test.mjs`, `demo/smoke_color_picker.mjs`, + `decor-color-popover-mobile-ru` в `demo/golden/matrix.mjs:269` — не + выдуманы, факты AC2 (120°→зелёный, Shift+Arrow→10°) уже покрыты текущим + smoke-кодом. +- **Touch-контракт корректно ограничен**: ТЗ явно относит touch-поддержку + picker к уже реализованной в #57, не расширяя её, и явно называет + родительские редакторы desktop-first по `docs/TOUCH-SUPPORT.md`; строка + `**Touch editor:** picker поддерживается как в #57` по существу закрывает + требование буквальной декларации из «Documentation rule» этого документа. +- **AC1-AC5 однозначны**, каждый снабжён допустимым по §2.5 PROCESS.md + способом доказательства (unit/smoke/golden/code review), включая + корректное делегирование полной изоляции (AC4) предрелизному golden-гейту, + а не локальному прогону — соответствует PROCESS.md §8 («полные наборы — + предрелизный гейт»). +- **Откат тривиален и корректен** — CSS-only изменение без миграции данных. +- **Release-артефакты** ссылаются на существующие файлы + (`docs/CHANGELOG.md`, `docs/CHANGELOG.ru.md`, `docs/USER-GUIDE.ru.md`), + формулировка уточнения не противоречит текущему тексту гайда. +- **«Принятые предположения»** корректно отделяют свободные технические + решения реализации (интерполяция stops, Chromium-only smoke) от уже + принятого продуктового решения по объёму — не маскируют продуктовый + вопрос под техническое решение. + +## Чего не проверял + +- Реализацию — её нет: issue в `S4-spec-review`, ветки `issue/192-*` не + существует (`git branch -r` пуст по этому номеру) — проверено. +- Гейты `typecheck`/`test`/`build`/browser smoke/golden — не относятся к + этапу ревью ТЗ; предмет будущего код-ревью (PROCESS.md §2.7/§8). +- `python -m pytest tests_backend` — backend не затрагивается ни кодом, ни + ТЗ этой задачи. +- Реальный визуальный результат градиента в браузере (Chromium/Firefox + track pseudo-elements, контраст thumb на разных секторах) — на этом этапе + CSS не написан; заявленные риски (thumb теряется на ярком секторе, + расхождение WebKit/Gecko) зафиксированы в ТЗ как риски с мерами, а не + проверены исполнением. +- Точность числовых оценок аналитики (ценность 4/10, сложность 3/10, P3) по + существу — поле владельца (PROCESS.md §2.2), уже принятое явным решением + до написания ТЗ. +- Полный список всех `` в `demo/` и `custom_components/` + — поиск ограничен `src/*.ts`, как единственным местом, где мог бы + находиться продуктовый hue-контрол; `demo/`/backend не содержат + собственных UI-контролов picker'а. + +## Вердикт + +Зелёный. High: 0, Medium: 0, Low: 1 (неявное разделение доказательства AC3 +между golden и code review для forced-colors/reduced-motion — тот же класс, +что прецедент `SPEC-REVIEW-137-r1` Low-4; снимается без правки текста ТЗ, +фиксируется в хендоффе код-ревью). ТЗ корректно и проверяемо решает +заявленный узкий скоуп: единственный потенциально продуктовый вопрос +(объём видимых изменений) уже закрыт владельцем в комментарии аналитики, +технический диагноз проблемы подтверждён чтением кода, скоуп реально полон +(других hue-поверхностей вне `.hue-range` не существует), AC однозначны и +снабжены допустимыми способами доказательства. + +**Вердикт: зелёный · цикл r1/2 · High: 0 · Medium: 0 → нет · Документ: +docs/reviews/SPEC-REVIEW-192-r1.md**