mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -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 после публикации).
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `dev`, коммит `a6185e295d2e` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `14bf5e35f9bccc2d74e54a390ebf9c9d176f2e8f`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 14bf5e35f9bc
|
||||
```
|
||||
- Тело issue: `ee15ac6db7376cfb878034a0fb3bdce2907fc84f45a69cf7c0e9cd0804b7e6ce`
|
||||
- Вердикт конвейера: `yellow` · High 0
|
||||
Reference in New Issue
Block a user