23 KiB
SPEC-REVIEW-588-r2 — «Настройки устройства: отображение „Значение + статичный значок“»
Issue: #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, материал — тело issue на момент разбора (SHA-256 тела ee15ac6db7376cfb878034a0fb3bdce2907fc84f45a69cf7c0e9cd0804b7e6ce, дерево материала 14bf5e35f9bccc2d74e54a390ebf9c9d176f2e8f, ветка dev@a6185e295d2e).
Дельта между r1 и r2 объявлена самим автором в комментарии перехода (Matysh, 2026-09-18T08:33:23Z) и подтверждена сверкой с текстом r1-документа (цитаты кода и AC в нём) построчно против текущего тела issue:
- новый пункт контракта К2а (три ветки живого пылесоса — отдельная, вторая параллельная проверка режима);
- новый AC6 (живой пылесос:
.vacpuck/.vactrail/.vacwarnне рисуются, буфер_vacRtне наполняется, обратное переключение восстанавливает puck/trail) со сдвигом прежних AC6…AC11 → AC7…AC12; - переписан риск №1 (был про один параллельный путь —
sourceDetails, стал про два:sourceDetailsи три ветки пылесоса); - дополнен блок «Принято предположительно» (три ветки пылесоса переводятся на общий предикат «нейтральное лицо», а не на четвёртый литерал);
- обновлены план автотестов и классы риска (async) под AC6;
- из К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 существует, открыт, метки bug, docs, vacuum, P3, S1-new — соответствует требованию §12 (тип, приоритет, S1-new). |
Унаследовано из r1
Без повторной проверки в этом раунде, по 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а внутрь «Контракта поведения»).
Как проверялось (только по дельте)
- Построчно сравнил текущее тело issue с описанием r1 (цитаты AC, К-пунктов, риска №1, «Принято предположительно» в самом документе r1) — дельта соответствует тому, что объявил автор в комментарии перехода, без скрытых расхождений.
- Прочитал
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. - Прочитал существующий блок
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, ниже). - Проверил, существует ли где-то в проекте пример, доказывающий, что
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 унаследует, если его не закрыть явно. - Проверил issue #589: существует, открыт, метки
bug, docs, vacuum, P3, S1-new, ссылается на #588 и #12 — соответствует требованиям §12 и заявлению автора. - Гейты кода не запускал — как и в 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.
Материал раунда
- Ветка:
dev, коммит5b7add94d15a— ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. - Дерево материала:
002d320431837447efa84ee6119a95334bac4080git log --all --format='%H %T' | grep 002d32043183 - Тело issue:
40a3c8b12af7fe3803ad3fece1a00b54c8fa12bd8fdc36b6bccd6516bf41756f - Вердикт конвейера:
yellow· High 0