From 6786c48a0e6e6e51f787cf4087bdd8dbc0626ef2 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Thu, 20 Aug 2026 11:43:39 +0000 Subject: [PATCH] docs: review document for #219 Issue: #219 User-Visible: no --- docs/reviews/SPEC-REVIEW-219-r1.md | 187 +++++++++++++++++++++++++++++ 1 file changed, 187 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-219-r1.md diff --git a/docs/reviews/SPEC-REVIEW-219-r1.md b/docs/reviews/SPEC-REVIEW-219-r1.md new file mode 100644 index 00000000..065ff5fd --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-219-r1.md @@ -0,0 +1,187 @@ +# SPEC-REVIEW — issue #219, цикл r1 + +- **Issue:** https://github.com/Matysh/houseplan-card/issues/219 +- **ТЗ:** `docs/specs/219-lock-orange-palette.md`, коммит `28d6b9c` (ветка + `issue/219-lock-orange-palette`) +- **Трек:** обычный полный (не `small`), лимит циклов ревью ТЗ — 4 +- **Ревьюер:** Claude (роль «Ревьюер ТЗ», отдельная сессия от автора) +- **Вердикт:** жёлтый · цикл r1/4 · High: 0 · Medium: 1 → в задаче + +## Скоуп проверки + +Issue #219 фиксирует продуктовое решение владельца: замена палитры +замков (было чёрный/янтарный, стало зелёный/красный) и унификация цвета +glyph на оранжевой подложке между темами (белый в Light, `#252525` в +Dark). Проверялись: + +1. соответствие ТЗ обязательным разделам §7.1 PROCESS.md; +2. однозначность и доказуемость каждого AC1…AC7; +3. отсутствие догадок, выданных за факт, без пометки «предположение»; +4. соответствие ТЗ реальному состоянию кода (`src/styles.ts`) и связанным + канонам (`docs/SCOPE.md`, `docs/USER-GUIDE.ru.md`, `docs/ARCHITECTURE.md`); +5. что вопросы, вынесенные (не)вынесенные владельцу, действительно + технические либо действительно продуктовые. + +Это первый цикл, раздел «объём по дельте» (§2.10 PROCESS.md) не +применяется — разбор ТЗ выполнен полностью. + +## Как проверялось + +- Прочитан `docs/SCOPE.md` (J1/J2 — общая живая карта дома и её базовые + состояния), `AGENTS.md`, `PROCESS.md` целиком (§2.4, §2.9/2.10, §7.1, + §7.2, §12). +- Прочитано тело issue #219 и все три комментария (аналитика, занятие, + публикация ТЗ). Метка на момент ревью — `S4-spec-review`, трек обычный. +- Прочитан `docs/specs/219-lock-orange-palette.md` целиком. +- Сверено текущее состояние `src/styles.ts` построчно с описанием + «Подтверждённая причина» (§3 ТЗ): проверены блоки `.oplock`, + `.oplock.locked/.unlocked`, `.dev.on/.open/.lock-locked/.lock-unlocked`, + `.dev.alarm`, `.dev.sel`, `--hp-open` — совпадает с ТЗ дословно, включая + жёстко заданный `#4a2800` у `.dev.open` (line 2145) и постоянный + `#fff`/black у lock-glyph без theme-aware варианта у `locked`. +- Проверено, что `#F0410C` и `#66d17a` — уже существующие в кодовой базе + semantic-цвета (alert/alarm и comfort-ok/high-LQI соответственно: + `src/device-pulse.ts:37`, `src/logic.ts:1309,1441`, `src/styles.ts:877`, + `test/logic.test.mjs:280-281`), а не изобретённая для этой задачи + палитра — предположение №3 ТЗ подтверждено чтением кода, не на слово. +- Проверено существование golden-сценариев `device-icon-state-table-light` + и `device-icon-state-table-dark` (`demo/golden/matrix.mjs:201`, + `test/golden-matrix.test.mjs:314`) и существующего contract-теста + `test/device-marker-polish-contract.test.mjs`, на которые ссылаются + AC1/AC3/AC5 как доказательство — ссылки не на несуществующие файлы. +- Прочитан `docs/USER-GUIDE.ru.md` целиком в частях «Замок» (524-532), + «Подложка и постоянный статус» (773-784) и таблице «Отображение» — + источник интерфейсной терминологии по AGENTS.md. +- Проверены смежные issue #179/#211/#213/#217, упомянутые как «связанные + контракты, не дубликаты»: #217 — про капсулу text-маркера (не + пересекается), #213 — про центрирование/hover новых маркеров (не + пересекается по предмету, хотя пересекается по CSS-области); дублирования + не найдено. +- Гейты кода в этом цикле не запускались: предмет ревью — ТЗ, не код; + реализации ещё нет (стадия `S4-spec-review`). + +## Находки + +### Medium — в скоупе задачи + +**M1. ТЗ не называет `docs/USER-GUIDE.ru.md` как обязательный +release-артефакт, хотя документ прямо фиксирует заменяемый контракт в +трёх местах.** + +- `docs/USER-GUIDE.ru.md:526-527` — раздел «Замок»: «`locked` показывается + как закрытый чёрный/тёмный замок... `unlocked`/`open` — как открытый, с + жёлтой подложкой дизайн-пакета»; +- `docs/USER-GUIDE.ru.md:778` — таблица «Подложка и постоянный статус», + строка «Жёлтая подложка»: примеры включают «lock unlocked/open»; +- `docs/USER-GUIDE.ru.md:779` — строка «Чёрный значок замка | Замок + заблокирован | lock locked». + +После принятия решения владельца (открыто/разблокировано — красный, +закрыто/заблокировано — зелёный) все три места станут фактически неверны: +разблокированный замок никогда не будет «жёлтой подложкой», заблокированный +— никогда не «чёрным значком». `AGENTS.md` требует: «For work that changes +visible behaviour, also read `docs/USER-GUIDE.ru.md` — interface wording +comes from there and is not invented, or the UI starts speaking developer» — +то есть это канонический источник интерфейсной терминологии, а не +произвольная документация. + +ТЗ (§6 «Scope», §16 «Release-артефакты») называет только `docs/TESTING.md` +и расплывчатое «при необходимости канонический документ device icon +states», не указывая ни одного конкретного файла и не отмечая, что +необходимость уже доказана — три конкретные строки уже противоречат +принимаемому решению. Без явного требования эта правка рискует остаться +несделанной: разработчик увидит в ТЗ только `TESTING.md` и changelog и +не откроет `USER-GUIDE.ru.md`, у которого нет отдельной строки в списке +затронутых файлов (§10). + +**Почему это не Low.** Речь не о стилистической неточности, а о трёх +предложениях канонического пользовательского руководства, которые прямо +и однозначно описывают только что заменяемое поведение, и о явном +процессном требовании (`AGENTS.md`) сверяться с этим документом при +любой правке видимого поведения. Оставленным без указания это — дыра в +DoD задачи, а не косметика. + +**Фикс, ожидаемый в этом же цикле:** добавить в §6 «Scope» и §16 +«Release-артефакты» ТЗ явный пункт — обновление `docs/USER-GUIDE.ru.md`, +строки 526-527, 778, 779 (замена «чёрный/жёлтый» на «зелёный/красный» в +соответствующих формулировках и таблице), с той же нормативной таблицей +цветов, что и в §8.1/§8.2 ТЗ. + +### Low + +Не найдено находок, которые стоило бы фиксировать отдельно и не чинить. + +## Что проверено и корректно + +- Обязательные разделы §7.1 присутствуют и по существу: сценарий и + персона (§1), что человек увидит до/после (§2, без терминов реализации), + проблема (§3), скоуп/не-скоуп (§6/§7), контракт поведения (§8/§9), + архитектура (§10, декларативно, без нового renderer), i18n/a11y/security + (§11), performance (§12), риски (§13), AC1…AC7 с доказательствами (§14), + план проверок (§15), release-артефакты (§16), откат (§17), явный блок + предположений (§18). +- Все AC пронумерованы, у каждого указан способ доказательства + (unit/contract/golden/review), формулировки однозначны — не «сделать + лучше», а точные hex-значения и конкретные CSS-классы/сценарии. +- «Подтверждённая причина» (§3) точно соответствует коду: `.dev.on` уже + theme-aware, `.dev.open` жёстко на `#4a2800`, `.oplock.*`/`.dev.lock-*` + используют старую black/amber палитру — воспроизведено чтением + `src/styles.ts`, а не принято на слово автора. +- Блок «Принятые предположения» (§18) — реальные технические допущения + (где хранится не-продуктовая деталь: конкретные hex уже существующих + semantic-цветов, файловая раскладка), не подмена продуктового решения. + Продуктовое решение (red/green семантика, applies и к badge, и к + marker) уже дано самим текстом issue — ТЗ его не изобретает, а + переносит. +- Открытых продуктовых вопросов владельцу нет и не должно быть: issue + дословно называет обе стороны решения («открыто/разблокировано — + красный, закрыто/заблокировано — зелёный»; правило для glyph по + теме) — решать в ТЗ было нечего, картина полностью дана. +- Приоритет состояний (§9 ТЗ) корректно сохраняет существующий порядок + cascade: alarm (`#F0410C`, уже используемый для alert/pulse) при + наложении на lock-unlocked (тот же `#F0410C`) не создаёт новой + двусмысленности — обе семантики уже красные, а более специфичный + `.dev.alarm`-селектор идёт в файле позже и выигрывает по тому же + правилу, что и сегодня; риск учтён в таблице рисков (§13, строка + «Broad selector перекрасит alert/hover/unavailable»). +- Переиспользование `#F0410C`/`#66D17A` — не новая палитра: оба цвета уже + используются в коде для alert/comfort-ok семантики + (`src/device-pulse.ts:37`, `src/logic.ts:1309,1441`); предположение №3 + ТЗ подтверждено, а не принято на слово. +- Ссылки ТЗ на существующие golden-сценарии (`device-icon-state-table-*`) + и contract-тест (`test/device-marker-polish-contract.test.mjs`) + указывают на реально существующие файлы/сценарии, а не на + выдуманную инфраструктуру. +- Не-скоуп (§7) корректно исключает геометрию, MDI-путь, hit-area, + действия, alarm/unavailable/selected-механику, оранжевые линии + проёмов/Glow, info-card текст — то есть ТЗ не расширяет задачу дальше + цветов состояний, заявленных владельцем. +- Трек (обычный, не `small`) выбран верно: поверхностей несколько (badge, + marker, три представления, тесты, документация), критерий «одна + поверхность» для лёгкого трека не выполняется. +- Откат (§17) осмыслен: один product-коммит, явный запрет частичного + отката одной проекции/темы — обоснованно, так как частичный откат + воспроизвёл бы исходное расхождение. + +## Чего не проверял + +- Golden-артефакты (`npm run golden:verify`, capture) не запускались: + на этапе ревью ТЗ реализации ещё нет, гейты предмет код-ревью (§2.7 и + §8 PROCESS.md), не ревью ТЗ (§2.4). +- Типы/сборка/тесты (`npx tsc --noEmit`, `npm test`, `npm run build`) не + запускались по той же причине — нечего собирать, продуктовый код не + менялся этим коммитом (только `docs/specs/**`, класс C). +- Полный список файлов `docs/` на предмет других упоминаний + чёрно-янтарной палитры замков за пределами `USER-GUIDE.ru.md` не + проверялся построчно per-файл — целевой grep по `4a2800`, «чёрн», + «жёлт» плюс «замок» ограничен `USER-GUIDE.ru.md` и `ARCHITECTURE.md` + (последний упоминает `oplock` только структурно, без цветовых + утверждений — проверено). +- Не проверялась историческая полнота предыдущих SPEC-REVIEW #179/#211/#213 + на предмет собственных незакрытых находок — вне предмета этого ревью. + +## Раздел «Унаследовано» — не применяется + +Это первый цикл ревью ТЗ (r1); раздел «Унаследовано из r» и таблица +закрытия предыдущего раунда не ведутся (§2.10 PROCESS.md действует со +второго цикла).