From 5b7add94d15a3961adc7f288f00750820cbbba07 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Fri, 18 Sep 2026 08:29:07 +0000 Subject: [PATCH] docs: review document for #588 Issue: #588 User-Visible: no --- docs/reviews/SPEC-REVIEW-588-r1.md | 118 +++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-588-r1.md diff --git a/docs/reviews/SPEC-REVIEW-588-r1.md b/docs/reviews/SPEC-REVIEW-588-r1.md new file mode 100644 index 00000000..9962e594 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-588-r1.md @@ -0,0 +1,118 @@ +# SPEC-REVIEW-588-r1 — «Настройки устройства: отображение „Значение + статичный значок“» + +Issue: [#588](https://github.com/Matysh/houseplan-card/issues/588) +Этап: spec (полный трек — S2-analysis назвал явно два нарушенных критерия §5: «одна поверхность» и «нет нового UX-контракта») +Заход: r1 (первый; раздела «Унаследовано из r0» и «Закрытие раунда r0» не требуется — §2.10 применяется со второго захода) + +## Вердикт + +**Жёлтый.** High: 0. Medium в скоупе: 1 (возвращается автору в этой же задаче). Medium вне скоупа: 1 → [#589](https://github.com/Matysh/houseplan-card/issues/589). + +## Скоуп разбора + +Полный разбор, как и требует полный трек первого захода: тело issue #588 (`## ТЗ`), единственный комментарий аналитики/автора (сведённые S2+S3 одним ходом), `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` целиком, а также — поскольку почти все содержательные утверждения ТЗ являются проверяемыми фактами о текущем коде, а не предположениями, — сам код и канонические документы, которые ТЗ обязано использовать как источник терминологии и поведения: + +- `src/logic.ts` (`DISPLAY_MODES`, `normalizeDeviceDisplay`); +- `src/device-presentation-policy.ts` (`resolveDevicePresentationPolicy`, `resolvePresentationReason`); +- `src/device-presentation.ts` (быстрый путь `sourceDetails`, `fallbackReason`, `presentationClasses`, `vacuumLive`); +- `src/device-pulse.ts` (`resolveDevicePulse`); +- `src/device-value-badge.ts` (подавление бейджа); +- `src/houseplan-card.ts` (три ветки пылесоса: `_vacTick:12194`, рендер puck/trail:12405-12497, `_vacRouteBadge:12591`; вызовы `sourceDetails: false` на `:4686`, `:5582`); +- `src/space-card.ts:519`, `src/space-render.ts:587` (те же вызовы на других поверхностях плана); +- `src/houseplan-editor-runtime.ts` (метки/подсказки режима, видимость «Источника значения» `:13397`, предупреждение о тревоге `:13430`, секция бейджа `:13447-13458`); +- `docs/USER-GUIDE.ru.md` (раздел «Четыре варианта „Отображение“», `:1280-1334`); +- `demo/smoke_static_icon.mjs` (существующий блок про пылесос); +- `demo/golden/matrix.mjs` (сцены `device-icon-state-table-{light,dark}`); +- `docs/specs/162-vacuum-map-space-routing.md`, GitHub issue #12 (`state_reason`) — для проверки claim про «подсветку убираемой комнаты». + +Продуктовые вопросы не разбирались повторно: владелец принял варианты по умолчанию по всем трём вопросам 18.09.2026 непосредственно в теле issue, до раздела `## ТЗ`. Открытых продуктовых вопросов в тексте не осталось. + +## Как проверялось + +Для каждого утверждения ТЗ, поданного как факт (а не как «принято предположительно»), было выполнено чтение соответствующего участка кода/документа, а не принятие на веру: + +1. Модель данных и совместимость: `normalizeDeviceDisplay` действительно проецирует неизвестный токен в `'badge'` (`src/logic.ts:904-908`) — совпадает с разделом «Модель данных и миграция». +2. Причины отказа значения (К1): все четыре токена `value_no_state` / `value_ambiguous_sources` / `value_non_scalar` / `value_virtual` существуют в `resolveValue` (`src/device-presentation.ts:479-591`) ровно с такой семантикой, как описано. +3. Быстрый путь (К3, риск №1, AC3): подтверждено текстуально — `staticIcon && options.sourceDetails === false` (`src/device-presentation.ts:675-717`), и `sourceDetails: false` передаётся ровно в четырёх местах основного пути плана: `houseplan-card.ts:4686`, `houseplan-card.ts:5582`, `space-card.ts:519`, `space-render.ts:587` — совпадает с числом и местами, названными в комментарии аналитики. +4. Подавление пульсации при тревоге для статичных режимов (К2, AC4): `display === 'static_icon'` действительно проверяется **раньше** ветки `visual.status === 'alarm'` в `resolveDevicePulse` (`src/device-pulse.ts:81-94`) — то есть тревога у `static_icon` реально не пульсирует, и новый режим обязан попасть в то же самое условие. +5. Подавление внешнего бейджа (К2, AC4): `options.display === 'static_icon'` в `device-value-badge.ts:279` — подтверждено. +6. Пятая опция и её ярлык/подсказка (К5): текущий `DISPLAY_MODES` кончается на `static_icon`, добавление в конец действительно не переставляет порядок (`src/logic.ts:898`). +7. Видимость «Источника значения» (К5): реально зависит от `d.display === 'value'` (`src/houseplan-editor-runtime.ts:13397`) — переход на предполагаемый предикат `wantsValue` в «Принято предположительно» корректно закрывает именно эту строку. +8. Три ветки пылесоса, названные в скоупе: подтверждены как `_vacTick` (`houseplan-card.ts:12194-12201`, чистит runtime), рендер puck/trail (`:12405-12497`, две проверки на входе цикла и в списке пуков) и `_vacRouteBadge` (`:12591-12593`) — все три сравнивают `normalizeDeviceDisplay(d.marker?.display) === 'static_icon'` напрямую, минуя `resolveDevicePresentationPolicy`. +9. Golden-сцены `device-icon-state-table-light/dark` существуют в `demo/golden/matrix.mjs:151-152,754` — AC8 не ссылается на несуществующую фикстуру. +10. Проверка claim из К2 «подсветка убираемой комнаты пылесоса не рисуется»: искал реализацию по всему `src/**`/`demo/**` (CSS-класс, метод, поле кроме объявления типа) — не нашёл. `docs/specs/162-vacuum-map-space-routing.md:86` прямо пишет, что `room_highlight` присутствует только в схеме, runtime-потребителя нет, это scope issue #12. `gh api .../issues/12` — `state: closed, state_reason: not_planned` (закрыт 2026-08-27T21:33:18Z). Разбор в §«Находки» ниже. +11. Дубликаты, названные автором (#3, #26, #158, #219) — по названиям и описанию в комментарии аналитики это закрытые задачи про другие грани того же селектора, пересечения с текущим ТЗ по факту нет оснований подозревать. + +## Обязательные разделы §7.1 — комплектность + +Присутствуют все: сценарий · что человек увидит до/после · проблема · скоуп/не-скоуп · контракт поведения · UX · модель данных и миграция · i18n · AC1…AC11 с доказательством · план автотестов · риски · откат · release-артефакты. Дополнительно закрыты «Классы риска §2.6» (все шесть явно разобраны, включимость/неприменимость обоснована). Продуктовые вопросы владельцу заданы пачкой с вариантами по умолчанию и решены до записи ТЗ — процесс §7.1 соблюдён. + +## Находки + +### Medium (в скоупе задачи — возвращается автору без отдельного issue) + +**М1. Три ветки пылесоса, явно включённые в скоуп, не имеют ни одного AC/теста/мутанта.** + +Раздел «Скоуп» прямо называет `src/houseplan-card.ts` (среди прочего — «три ветки пылесоса»), то есть автор уже знает, что `_vacTick`, рендер puck/trail и `_vacRouteBadge` (`houseplan-card.ts:12198`, `:12414`, `:12593`) должны научиться распознавать `value_static_icon` наравне с `static_icon`. Раздел «Контракт поведения» (К2) утверждает как факт: «Живой puck, след ... у пылесоса не рисуются» — то есть это заявленное поведение, не опция на усмотрение реализации. + +Однако ни один из AC1…AC11 и ни один пункт «Плана автотестов» не проверяет это: + +- AC1 проверяет `vacuumLive === false` из чистой функции `resolveDevicePresentationPolicy` (`test/device-presentation-policy.test.mjs`) — но это **не тот код**, который на самом деле управляет пуком/следом/бейджем маршрута. `presentation.vacuumLive` — диагностическое поле, которое сегодня не читается ни одним потребителем в `src/**`/`demo/**` кроме самого смока `smoke_static_icon.mjs`, который сверяет его для *существующего* `static_icon` (`demo/smoke_static_icon.mjs:52,180`). Реальные три ветки сравнивают `normalizeDeviceDisplay(...) === 'static_icon'` напрямую и независимо от `vacuumLive`. +- AC5 (единственный AC про smoke на настоящем плане) описывает только классы состояния, текст значения, пульсацию и LQI — без единого слова про `.vacpuck`/`.vactrail`/route-badge. +- Раздел «Риски» называет риском №1 именно параллельный (второй) путь — быстрый путь `sourceDetails`, ровно потому что «без него дефект был бы виден только на плане и невидим в предпросмотре». Три ветки пылесоса — структурно тот же класс риска (независимая строковая проверка режима, которую легко забыть обновить в одном из трёх мест), но в разделе «Риски» не упомянуты вовсе. + +**Чем это красное на практике.** Реализация может честно выполнить AC1–AC11 «на зелёный», обновив только `neutralFace`/`wantsValue` предикаты в политике и `device-value-badge.ts`/`device-pulse.ts`, и забыть добавить `|| display === 'value_static_icon'` в один из трёх литеральных `=== 'static_icon'` в `houseplan-card.ts` — например, только `_vacRouteBadge`. Результат: маркер `value_static_icon` у живого пылесоса покажет неизменный цвет и число (AC1–AC4 зелёные), но поверх него всё ещё нарисуется предупреждающий бейдж маршрута или продолжит двигаться puck — прямое нарушение К2 и продуктового обещания «маркер никогда не меняется» из тела issue, и ни один автотест, названный в ТЗ, этого не заметит. + +**Почему Medium, не High.** Не блокирует старт разработки: дефект — отсутствие проверки, а не сломанный контракт, дыра закрывается добавлением строки в AC5 (или новым AC12) и одного нового блока в `demo/smoke_static_icon.mjs`, зеркального уже существующему блоку для `static_icon` (`:86-180`, там уже создаётся живой пылесос и проверяются `.vacpuck`/`.vactrail`/`staticSuppressesVacuum`). Работа на десять минут, а не пересмотр контракта. + +**Что нужно на правку:** добавить AC (или явно расширить AC5) вида «На живом пылесосе с `display: value_static_icon` `.vacpuck`, `.vactrail` и route-warning-бейдж не рисуются, как и при `static_icon`» с доказательством `demo/smoke_static_icon.mjs` (новый блок по образцу существующего, `:86-180`) и упомянуть эту ветку риска в разделе «Риски» рядом с риском №1. + +### Medium (вне скоупа — заведён отдельным issue, эта задача его не чинит) + +**М2. `docs/USER-GUIDE.ru.md` и `docs/FILTERING.md` описывают «подсветку убираемой комнаты» пылесоса как существующую фичу, которую подавляет `static_icon` — фича отклонена в issue #12 (`not_planned`, закрыт 2026-08-27) и не имеет в коде ни одного потребителя (`docs/specs/162-vacuum-map-space-routing.md:86` прямо это фиксирует).** + +ТЗ #588 честно скопировало эту формулировку из канонического USER-GUIDE (раздел К2: «Живой puck, след и подсветка убираемой комнаты у пылесоса не рисуются») — как и требует правило «терминология интерфейса берётся оттуда». Проблема не в ТЗ #588, а в источнике: он описывает фичу, которой не существует. Практических последствий для #588 нет (для несуществующей фичи «не рисуется» истинно тривиально, ничего в коде для этого пункта строить не нужно), поэтому это не блокирует и не возвращает #588 — но источник должен быть исправлен, иначе формулировка продолжит попадать в следующие ТЗ как факт. + +Заведён [issue #589](https://github.com/Matysh/houseplan-card/issues/589) (`bug`, `docs`, `vacuum`, `P3`, `S1-new`) со ссылкой на #588 и на #12. + +### Low + +Не найдено значимых Low-находок сверх М1/М2. Формулировка «чем краснеет» AC6 («правка любой из четырёх веток редактора») не называет ветки по номеру строки — но сам AC6 доказывается независимым smoke-тестом (`smoke_device_preview_parity.mjs`/`smoke_static_icon.mjs`) с наблюдаемым результатом, а не косвенным чтением кода, так что размытость формулировки не снимает доказуемость. Не считаю нужным править отдельно. + +## Что проверено и корректно + +- Оба продуктовых раздела (сценарий, «что человек увидит до/после») по форме и содержанию соответствуют §7.1: персона и поверхность названы, разница до/после — одной фразой без терминов реализации. +- Скоуп/не-скоуп перечисляет ровно те файлы, которые реально содержат логику `static_icon`/`value` (проверено построчно по каждому файлу, см. «Как проверялось»); не-скоуп корректно исключает `import_export.py` с объяснением почему. +- Контракт поведения К1–К6 — каждое утверждение о текущем поведении родительских режимов (`value`, `static_icon`) подтверждено чтением кода, а не выдумано; расхождение с реальным кодом обнаружено только по одному пункту (М1/М2 выше). +- Модель данных, миграция и деградация для старого клиента описаны точно и совпадают с существующим механизмом чтения (`normalizeDeviceDisplay`), включая то, что бэкенд отдельно валит незнакомый токен на запись (не проверял сам Python-код валидатора — это не влияет на ТЗ-разбор, будет частью код-ревью). +- i18n-ключи следуют уже существующей схеме именования 1:1 (`display.*`, `marker.display_hint_*`, `marker.preview.reason.*`). +- AC1–AC4, AC7–AC11 однозначны, у каждого назван метод доказательства и (где применимо) мутант/эталон, который должен покраснеть на снятой защите. +- «Принято предположительно» корректно ограничен техническими решениями (имя предиката, форма `presentationClasses`), не подменяет продуктовое решение и явно помечен как свободный для правки. +- Продуктовые вопросы (доступность значения, внешний бейдж, тревога) заданы владельцу пачкой с вариантами по умолчанию и решены до фиксации ТЗ — процесс §7.1 не нарушен. +- Дубликаты (#3, #26, #158, #219) — не проверялись по существу текста (это заняло бы полный повторный анализ старых issue), доверился утверждению автора; расхождения не похоже, что есть, но отдельно не подтверждено чтением этих issue. + +## Чего не проверял + +- Не читал сам `custom_components/houseplan/validation.py` и `scripts/config-schema.json` построчно — доверился тому, что описанный механизм (enum принимает/отвергает токен) стандартен для этого проекта и уже применён к `static_icon`; это предмет код-ревью, а не спецификации. +- Не проверял содержимое старых issue #3/#26/#158/#219 на предмет реального пересечения — принял утверждение автора об отсутствии дублирования на слово. +- Не запускал никаких гейтов (`tsc`, `npm test`, `npm run build`) — на этапе ревью ТЗ они неприменимы: правка кода ещё не начата, гейты кодовой базы не относятся к предмету этого ревью. +- Не проверял `docs/CONFIG-COMPATIBILITY.md` целиком на непротиворечивость — только раздел, прямо релевантный новому токену (проверен через `normalizeDeviceDisplay`). + +## Материал раунда + +- Issue: #588, тело на момент разбора (SHA-256 нормализованного тела вычисляется и вписывается конвейером в блок якорей публикации; здесь не дублируется). +- Заход r1, циклов ревью ТЗ израсходовано 0 из 4 (лимит для полного трека — 4; см. таблицу PROCESS.md §4 — жёлтый вердикт этого раунда израсходует цикл 1/4 после публикации). + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `a6185e295d2e` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `14bf5e35f9bccc2d74e54a390ebf9c9d176f2e8f` + ``` + git log --all --format='%H %T' | grep 14bf5e35f9bc + ``` +- Тело issue: `ee15ac6db7376cfb878034a0fb3bdce2907fc84f45a69cf7c0e9cd0804b7e6ce` +- Вердикт конвейера: `yellow` · High 0