mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -0,0 +1,114 @@
|
||||
# SPEC-REVIEW-588-r2 — «Настройки устройства: отображение „Значение + статичный значок“»
|
||||
|
||||
Issue: [#588](https://github.com/Matysh/houseplan-card/issues/588)
|
||||
Этап: spec (полный трек)
|
||||
Заход: r2 · блокирующих циклов израсходовано на входе 1/4
|
||||
|
||||
## Вердикт
|
||||
|
||||
**Жёлтый.** High: 0. Medium в скоупе: 1 (возвращается автору в этой же задаче, без отдельного issue). Medium вне скоупа: 0 новых (M2 предыдущего раунда закрыт, отдельный issue #589 уже заведён r1 и это ревью его не трогает).
|
||||
|
||||
## Скоуп разбора (по дельте, §2.10)
|
||||
|
||||
Предыдущий вердикт: жёлтый, заход r1, [docs/reviews/SPEC-REVIEW-588-r1.md](../../docs/reviews/SPEC-REVIEW-588-r1.md), материал — тело issue на момент разбора (SHA-256 тела `ee15ac6db7376cfb878034a0fb3bdce2907fc84f45a69cf7c0e9cd0804b7e6ce`, дерево материала `14bf5e35f9bccc2d74e54a390ebf9c9d176f2e8f`, ветка `dev`@`a6185e295d2e`).
|
||||
|
||||
Дельта между r1 и r2 объявлена самим автором в комментарии перехода (`Matysh`, 2026-09-18T08:33:23Z) и подтверждена сверкой с текстом r1-документа (цитаты кода и AC в нём) построчно против текущего тела issue:
|
||||
|
||||
1. новый пункт контракта **К2а** (три ветки живого пылесоса — отдельная, вторая параллельная проверка режима);
|
||||
2. новый **AC6** (живой пылесос: `.vacpuck`/`.vactrail`/`.vacwarn` не рисуются, буфер `_vacRt` не наполняется, обратное переключение восстанавливает puck/trail) со сдвигом прежних AC6…AC11 → AC7…AC12;
|
||||
3. переписан **риск №1** (был про один параллельный путь — `sourceDetails`, стал про два: `sourceDetails` и три ветки пылесоса);
|
||||
4. дополнен блок **«Принято предположительно»** (три ветки пылесоса переводятся на общий предикат «нейтральное лицо», а не на четвёртый литерал);
|
||||
5. обновлены **план автотестов** и **классы риска (async)** под AC6;
|
||||
6. из **К2** и **«Не-скоуп»** убрана фраза про «подсветку убираемой комнаты» пылесоса (M2), в «Не-скоуп» добавлена ссылка на #589.
|
||||
|
||||
Дельта локальна: новая подсистема не затронута (пылесос уже был в скоупе r1 и назван в риске №1), контракт остальных режимов (К1, К3–К6) не менялся, продуктовые ответы владельца от 18.09 не пересматривались. По §2.10 повторно проверялись только AC/разделы, которых касается дельта: К2а/AC6, риск №1, «Принято предположительно», план автотестов, «Скоуп/не-скоуп» (только удалённая фраза), плюс код, на который эти пункты ссылаются. Остальные разделы ТЗ унаследованы из r1 без повторной проверки (раздел ниже).
|
||||
|
||||
Материал: рабочая копия репозитория соответствует `git rev-parse HEAD` = `5b7add94d15a3961adc7f288f00750820cbbba07` — это код, использованный как источник фактов для проверки утверждений ТЗ (фича ещё не реализована, поэтому это не «диапазон коммитов задачи», а актуальное состояние `dev`, ровно как и в r1).
|
||||
|
||||
## Закрытие раунда r1
|
||||
|
||||
| Находка r1 | Чем закрыта | Где это видно |
|
||||
|---|---|---|
|
||||
| **M1** (в скоупе). Три ветки пылесоса (`_vacTick`, рендер puck/trail, `_vacRouteBadge`) названы в скоупе и в К2, но ни один AC/тест/риск их не проверяет — реализация могла честно закрыть AC1–AC11 и забыть один из трёх литералов `=== 'static_icon'`. | Добавлен явный пункт контракта **К2а**, новый **AC6** с тремя фактами (puck/trail/badge) плюс обратное переключение, доказательство — новый блок `demo/smoke_static_icon.mjs`, мутант `value-static-icon-keeps-live-vacuum`; риск №1 переписан и включает обе параллельные проверки; «Принято предположительно» предписывает общий предикат вместо четвёртого литерала. | Тело issue, разделы «Контракт поведения» (К2а), таблица AC (строка AC6, жирным), «Риски» (пункт 1), «Принято предположительно» (пункт про три ветки), «План автотестов» (`demo/smoke_static_icon.mjs: AC5, AC6, AC7`). Код-факты подтверждены построчно: три литерала `normalizeDeviceDisplay(d.marker?.display) === 'static_icon'` реально существуют в `src/houseplan-card.ts:12197` (`_vacTick`), `:12414` (рендер puck/trail), `:12593` (`_vacRouteBadge`); буфер — `_vacRt` (`:2355`); классы — `.vacpuck`/`.vactrail`/`.vacwarn` (`:12486`, `:12497`, `:12599`) — всё совпадает с текстом AC6. **Закрыта не полностью — см. новую находку M3 ниже**: третий факт AC6 (отсутствие `.vacwarn`) заявленным способом доказательства не проверяется. |
|
||||
| **M2** (вне скоупа). `USER-GUIDE.ru.md`/`FILTERING.md` описывают отклонённую фичу «подсветка убираемой комнаты» как существующую; источник должен быть исправлен отдельно. | Фраза убрана из К2 текста ТЗ (продуктовое обещание больше не ссылается на несуществующую фичу); в «Не-скоуп» — прямая ссылка на #589 как место, где чинится источник. | Текст К2 в текущем теле issue (сравнение с цитатой r1 в его §«Как проверялось», п.10, показывает отсутствие фразы); раздел «Не-скоуп»: «формулировка про „подсветку убираемой комнаты“ ... исправление источника вынесено в #589». Issue [#589](https://github.com/Matysh/houseplan-card/issues/589) существует, открыт, метки `bug, docs, vacuum, P3, S1-new` — соответствует требованию §12 (тип, приоритет, S1-new). |
|
||||
|
||||
## Унаследовано из r1
|
||||
|
||||
Без повторной проверки в этом раунде, по [SPEC-REVIEW-588-r1.md](../../docs/reviews/SPEC-REVIEW-588-r1.md) (материал: тело issue SHA-256 `ee15ac6d...b7e6ce`, дерево `14bf5e35f9bc...`, `dev`@`a6185e295d2e`):
|
||||
|
||||
- Сценарий и «что человек увидит до/после» — форма и содержание по §7.1, персона и поверхность названы (r1 §«Что проверено и корректно», п.1). Текст не менялся в дельте.
|
||||
- Скоуп/не-скоуп (за вычетом убранной фразы M2) — соответствует файлам, реально содержащим логику `static_icon`/`value`; не-скоуп корректно исключает `import_export.py`.
|
||||
- Контракт К1, К3–К6 — подтверждён построчным чтением кода в r1 (быстрый путь `sourceDetails`, четыре причины отказа значения, подавление пульсации/бейджа у `static_icon`, порядок `DISPLAY_MODES`, видимость поля «Источник значения» в редакторе). Дельта эти пункты не трогала.
|
||||
- Модель данных, миграция, деградация старого клиента (`normalizeDeviceDisplay` → `badge`) — проверено в r1, текст не менялся.
|
||||
- i18n-ключи, их именование по существующей схеме — не менялись.
|
||||
- AC1–AC5, AC7 (бывший AC6, редактор) — содержательно не изменились, только номер сдвинулся с AC6 на AC7; сверено построчно, текст самих критериев идентичен формулировкам, которые цитирует/пересказывает r1.
|
||||
- AC8–AC12 (бывшие AC7–AC11: backend, golden, регрессия, i18n, документация) — не менялись по содержанию, только по номеру.
|
||||
- Дубликаты (#3, #26, #158, #219) — r1 принял слова автора без проверки текста этих issue; дельта их не касается, поэтому не проверялось и сейчас.
|
||||
- «Обязательные разделы §7.1 — комплектность» — весь список присутствует, r1 подтвердил это один раз; дельта не убрала ни одного обязательного раздела (наоборот, добавила подпункт К2а внутрь «Контракта поведения»).
|
||||
|
||||
## Как проверялось (только по дельте)
|
||||
|
||||
1. Построчно сравнил текущее тело issue с описанием r1 (цитаты AC, К-пунктов, риска №1, «Принято предположительно» в самом документе r1) — дельта соответствует тому, что объявил автор в комментарии перехода, без скрытых расхождений.
|
||||
2. Прочитал `src/houseplan-card.ts` вокруг заявленных мест: `_vacTick` (`:12194-12201`), рендер puck/trail (`:12405-12497`), `_vacRouteBadge` (`:12591-12599`) — подтвердил, что все три сравнивают `normalizeDeviceDisplay(d.marker?.display) === 'static_icon'` напрямую, независимо от `resolveDevicePresentationPolicy`, ровно как заявлено в К2а. Подтвердил имя буфера `_vacRt` (`:2355`) и CSS-классы `.vacpuck`/`.vactrail`/`.vacwarn`.
|
||||
3. Прочитал существующий блок `demo/smoke_static_icon.mjs` (`:86-180`, то же место, что цитирует r1 и что называет ТЗ), на который AC6 ссылается как на образец: он создаёт живой пылесос с `calibration: { m1: [...] }` и камерой, чьи атрибуты (`map_name: 'm1'`) **совпадают** с калиброванной картой — то есть `resolveRoute` возвращает `kind: 'ready'`. Прочитал `src/vacuum-routes.ts:193-224,344-353` (`resolveRoute`, `routeWarningKey`): предупреждение (`unmapped`/`needs_calibration`/`ambiguous`/`missing_space`) возможно только при **несовпадающей/неоднозначной** карте, не при `'ready'`. Значит при точной калибровке, как в образце `:86-180`, бейдж `.vacwarn` не появляется вообще, ни при `static_icon`, ни при `badge`, ни при `value_static_icon` — обнаружил новую находку (M3, ниже).
|
||||
4. Проверил, существует ли где-то в проекте пример, доказывающий, что `static_icon` (существующий режим — не только новый) подавляет именно `.vacwarn`: `grep -rn "vacwarn"` по `demo/**` и `test/**` — единственное совпадение вне `houseplan-card.ts`/стилей — `demo/smoke_vacuum_multifloor.mjs:45`, который проверяет появление бейджа при **несопоставленной** карте (`map_name: 'm9'`, `:70-78`), но не в связке со `static_icon`/`value_static_icon`. Значит подавление бейджа режимом `static_icon` тоже никогда не было доказано автотестом — это существующий (не новый) пробел покрытия, который AC6 унаследует, если его не закрыть явно.
|
||||
5. Проверил issue #589: существует, открыт, метки `bug, docs, vacuum, P3, S1-new`, ссылается на #588 и #12 — соответствует требованиям §12 и заявлению автора.
|
||||
6. Гейты кода не запускал — как и в r1, на этапе ревью ТЗ они неприменимы (фича не реализована, менять нечего).
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium (в скоупе задачи — возвращается автору без отдельного issue)
|
||||
|
||||
**M3. Третий факт AC6 (отсутствие `.vacwarn`) не доказывается способом, который сам же AC6 называет.**
|
||||
|
||||
К2а и AC6 обещают, что для `value_static_icon` бейдж предупреждения о маршруте (`_vacRouteBadge`, `.vacwarn`) не рисуется — наравне с puck и trail. AC6 называет доказательство: «новый блок `demo/smoke_static_icon.mjs` по образцу существующего блока `static_icon` (`:86-180`)». Но фикстура именно этого образца (пылесос `static-vacuum` с `calibration: { m1: [0.001, 0, 0, 0.001, 0, 0] }` и камерой, отдающей `map_name: 'm1'`) заставляет `resolveRoute` вернуть `kind: 'ready'` (карта откалибрована и совпадает) — а `routeWarningKey` (`src/vacuum-routes.ts:344-353`) возвращает `null` для любого `kind`, кроме `unmapped`/`needs_calibration`/`ambiguous`/`missing_space`. То есть при точном повторении образца бейдж `.vacwarn` не появится **ни при каком** значении `display` — ни при `badge`, ни при `static_icon`, ни при `value_static_icon`.
|
||||
|
||||
**Чем это красно на практике.** Мутант `value-static-icon-keeps-live-vacuum`, если он «возвращает одну из трёх строковых веток к `=== 'static_icon'`» применительно именно к строке `_vacRouteBadge`, не будет пойман новым блоком: даже без правки третьей ветки для `value_static_icon` (то есть с сохранённым дефектом — бейдж должен был бы «прорваться») тест на этом образце всё равно увидит отсутствие `.vacwarn`, потому что бейдж и так не должен был появиться при данной калибровке. Проверка окажется зелёной и на дефектном, и на исправленном коде — то есть не проверяет ничего для этой конкретной ветки, при том что для двух других веток (puck/trail) тот же блок действительно чувствителен к мутации (подтверждено чтением: puck/trail рисуются при `moving: true` независимо от состояния маршрута, `:12455-12489`).
|
||||
|
||||
Дополнительно: подавление `.vacwarn` не доказано автотестом даже для уже существующего `static_icon` — значит это не сугубо новый пробел новой фичи, а унаследованный пробел покрытия, который AC6 рискует унаследовать молча, если не исправить формулировку.
|
||||
|
||||
**Почему Medium, не High.** Не требует пересмотра контракта — К2а корректен по существу (бейдж действительно должен подавляться), просто способ доказательства требует другой фикстуры. В проекте уже есть рабочий рецепт: `demo/smoke_vacuum_multifloor.mjs:70-78` создаёт несопоставленную карту (`map_name: 'm9'`) под движущимся роботом и получает `.vacwarn` именно на **не**-статичном маркере (`unmappedWarns`). Правка — минуты: добавить в новый блок AC6 второй под-сценарий (или изменить фикстуру существующего плана-мувинг на несопоставленную карту), убедиться, что при `display: 'badge'` бейдж появляется, а при `display: 'value_static_icon'` — нет.
|
||||
|
||||
**Что нужно на правку:** в тексте AC6 (или в комментарии к нему) уточнить, что доказательство третьего факта требует конфигурации, которая при обычном (не подавляющем) режиме реально порождает `.vacwarn` — например, несопоставленную карту, как в `smoke_vacuum_multifloor.mjs`, а не точную калибровку из образца `:86-180`, которая для этой конкретной проверки бесполезна.
|
||||
|
||||
### Low
|
||||
|
||||
Новых Low-находок в дельте не найдено. Low из r1 (нечёткость «чем краснеет» AC7/бывший AC6 про редактор) не переоткрывается: r1 явно снял её с записью, дельта эту строку не трогала.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- К2а корректно называет все три ветки пылесоса, их независимость от `resolveDevicePresentationPolicy` и необходимость перевода на общий предикат — подтверждено чтением `src/houseplan-card.ts`.
|
||||
- AC6 корректно доказывает два из трёх заявленных фактов (puck, trail) — обе ветки реагируют на `moving`/`matrix` независимо от состояния маршрута, значит мутация одной из них (без учёта badge-ветки) будет поймана.
|
||||
- Риск №1 в новой редакции точно описывает обе параллельные проверки и их независимость от `vacuumLive`.
|
||||
- «Принято предположительно» верно определяет техническое решение (общий предикат вместо четвёртого литерала) и оставляет его на усмотрение реализации, не подменяя продуктовое решение.
|
||||
- M2 закрыт по существу: текст ТЗ больше не выдаёт отклонённую фичу пылесоса за факт; #589 заведён с верными метками.
|
||||
- Нумерация AC1…AC12 после сдвига непротиворечива: не найдено ни дублей, ни пропусков, ни рассинхронизации между таблицей AC и «Планом автотестов»/«Классами риска».
|
||||
- Продуктовые разделы (сценарий, «до/после») не менялись дельтой и по-прежнему соответствуют форме §7.1 (унаследовано из r1, см. выше).
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не проверял повторно AC1–AC5, AC7–AC12 по существу построчным чтением кода второй раз — дельта их не касается, r1 это уже сделал (раздел «Унаследовано из r1»).
|
||||
- Не проверял содержимое #3/#26/#158/#219 — как и в r1, принял на слово; дельта дубликатов не касается.
|
||||
- Не читал `custom_components/houseplan/validation.py` и `scripts/config-schema.json` — как и в r1, это предмет код-ревью, дельта их не трогает.
|
||||
- Не запускал `tsc`/`npm test`/`npm run build` — на этапе ревью ТЗ неприменимо, кода ещё нет.
|
||||
- Не проверял, действительно ли исправление #589 (документация) уже выполнено — оно не входит в скоуп #588 и не блокирует эту задачу.
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Issue: #588, тело на момент разбора (SHA-256 нормализованного тела вычисляется и вписывается конвейером в блок якорей публикации; здесь не дублируется).
|
||||
- Заход r2, циклов ревью ТЗ израсходовано на входе 1 из 4 (лимит для полного трека — 4); жёлтый вердикт этого раунда израсходует цикл 2/4 после публикации.
|
||||
- Рабочая копия репозитория на момент проверки кодовых фактов: `git rev-parse HEAD` = `5b7add94d15a3961adc7f288f00750820cbbba07`.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `dev`, коммит `5b7add94d15a` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `002d320431837447efa84ee6119a95334bac4080`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 002d32043183
|
||||
```
|
||||
- Тело issue: `40a3c8b12af7fe3803ad3fece1a00b54c8fa12bd8fdc36b6bccd6516bf41756f`
|
||||
- Вердикт конвейера: `yellow` · High 0
|
||||
Reference in New Issue
Block a user