diff --git a/docs/reviews/SPEC-REVIEW-205-r1.md b/docs/reviews/SPEC-REVIEW-205-r1.md new file mode 100644 index 00000000..a48e246d --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-205-r1.md @@ -0,0 +1,137 @@ +# SPEC-REVIEW-205-r1 + +- Issue: [#205](https://github.com/Matysh/houseplan-card/issues/205) — след пылесоса обнуляется после мойки швабр +- ТЗ: [docs/specs/205-vacuum-trail-resume-grace.md](../specs/205-vacuum-trail-resume-grace.md) +- Ветка: `issue/205-vacuum-trail-grace`, коммит спеки: `f3472de` ("docs: specify vacuum trail resume grace") +- Этап: ТЗ на ревью (PROCESS.md §2.4) +- Цикл: r1/4 (лёгкий трек не применяется — `small` не выставлен, сложность 4, риск 7) +- Ревьюер: Claude, свежая сессия, без переписки с автором + +## Скоуп ревью + +Проверялось только ТЗ (`docs/specs/205-vacuum-trail-resume-grace.md`) и вход в него: +тело issue #205, комментарий аналитики с Q1–Q4 и ответами владельца, комментарий +«ТЗ готово к ревью». Продуктовый код не менялся веткой (единственный коммит на +ветке — сам файл спеки плюс строка в `docs/specs/README.md`), поэтому диапазон +`git diff origin/dev...HEAD` для этого цикла тривиален и не проверяется как код. + +## Как проверялось + +1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` целиком (обязательные + разделы §7.1, лимиты циклов, формат вердикта, правило Medium-вне-скоупа #202). +2. Прочитаны тело issue #205 и оба комментария (аналитика с Q1–Q4, ссылка на ТЗ). +3. Прочитан канонический документ подсистемы `docs/VACUUM.md` целиком. +4. Каждое фактическое утверждение ТЗ о текущем поведении сверено с реальным кодом + на `origin/dev`, а не принято на слово: + - `custom_components/houseplan/trails.py` — `MOVING_STATES`, `TrailBook.on_point`, + `TrailBook.end_run`, `TrailRecorder._sample` (нейтральность `unavailable`/ + `unknown`, идемпотентность `end_run`, epoch-таймстемпы, `async_refresh` → + немедленный `_sample` после рестарта); + - `src/vacuum.ts:592-599` — `trail_mode` по умолчанию `'cleaning'`; + - `src/houseplan-card.ts:16658-16667` — `showCur = tmode==='always' || + (tmode==='cleaning' && moving)` и то, что `previous` рисуется только в + `'always'`; убедился, что решение «показывать/скрывать» зависит от `moving` + (состояние вакуума), а не от поля `ended` в серверном `current` — то есть + фронтенд действительно не требует правок для этой задачи, как заявляет ТЗ; + - `tests_backend/test_trails.py` — текущий стиль pure-тестов `TrailBook`, + подтверждает, что план тестирования (раздел 10) реалистичен и совпадает с + существующей практикой; + - существование `demo/smoke_vacuum.mjs`, названного в разделе 10 как кандидат + для AC8. +5. Проверены §7.1 (обязательные разделы), однозначность и способ доказательства + каждого AC1…AC11, наличие явного блока принятых предположений (раздел 13), + соответствие `docs/CONFIG-COMPATIBILITY.md` (новых персистентных полей нет, + формат Store не меняется — отдельная запись в реестре не требуется). +6. Проверено попадание в `docs/SCOPE.md`: явной строки про пылесосы в Core user + jobs нет, но это не новая функция — Stage 1 vacuum уже принят и описан в + `VACUUM.md` («implemented contract»), а непрерывность следа — прямое продолжение + J1 («live spatial overview… what's happening right now»). Это починка регресса + принятой возможности, а не расширение скоупа продукта. + +Что не проверялось (не применимо на этом этапе): исполнение кода, автотесты, +гейты (`typecheck`/`test`/`build`) — продуктовый код не менялся, спецификация +не содержит рантайм-артефактов для прогона. + +## Находки + +### Low — раздел 1 не называет персону и поверхность по имени (снято ревьюером) + +**Файл:** `docs/specs/205-vacuum-trail-resume-grace.md`, раздел 1. + +PROCESS.md §7.1 требует, чтобы первый раздел ТЗ явно называл персону из +`docs/SCOPE.md` и поверхность. Раздел 1 говорит «владелец робота» и не привязывает +это к строке персон (`Home admin` / `Household members` / `Guests`) и не называет +поверхность (десктоп, настенный планшет, телефон). + +**Почему это не блокирует:** след пылесоса рисуется одинаково для любой персоны и +любой поверхности View — рендер не зависит от того, кто смотрит и с какого +устройства (подтверждено чтением `_renderVacuums`: решение о показе зависит только +от `trail_mode` и `moving`, не от персоны/поверхности). Формальное называние +персоны не изменило бы ни один AC и не создаёт риска разночтения между автором и +реализацией. + +**Решение:** снимаю без правки ТЗ — добавление одной фразы ничего не меняет по +существу, а цикл ревью на лёгкой правке дороже её отсутствия. + +Других находок — High или Medium, в скоупе или вне скоупа — не выявлено. + +## Что проверено и признано корректным + +- **Причина (раздел 3)** — точное совпадение с кодом: `_sample` зовёт `end_run` + для любого state вне `MOVING_STATES = {cleaning, returning, on}`; `on_point` + трактует любой truthy `ended` как безусловную границу ротации `current → + previous`, независимо от длительности и `map_id`. `unavailable`/`unknown` + уже нейтральны (`continue` до вызова `end_run`) — заявлено верно, не догадка. +- **Контракт (раздел 6)** — все 8 пунктов однозначны и непротиворечивы: + граница `0 <= now-ended <= 1800` резолвится в пользу продолжения ровно на + 30:00 (AC2 проверяет обе стороны границы); смена `map_id` — жёсткая граница + независимо от времени (AC3); дубликат первой точки после resume не + добавляется, но `changed=True` (AC4) — согласовано с тем, как + `_on_state`/`_schedule_save`/`bus.async_fire` используют возвращаемое + значение `on_point` в текущем коде; idempotent `end_run` и нейтральность + `unavailable`/`unknown` — не новое поведение, а точная фиксация уже + существующего. +- **Scope/Non-scope (разделы 4–5)** — граница чистая: vendor dialect + (`washing`/`drying`/`emptying`), task/session id, эвристики по координатам, + UI-настройка grace, миграция Store и изменение `MOVING_STATES`/`trail_mode` + явно исключены и совпадают с Q4-default владельца. +- **Persistence и frontend (раздел 7)** — заявление «фронтенд не требует + изменений» подтверждено чтением `_renderVacuums`: `showCur` зависит от + `moving`, не от `ended`; `srvCur`/`srvPrev` фильтруются только по `map_id` и + наличию `points`. AC8 корректно указывает на существующий + `demo/smoke_vacuum.mjs` как на реалистичный носитель доказательства. +- **AC1–AC11 (раздел 9)** — каждый снабжён способом доказательства + (`unit`/`backend`/`smoke`), формулировки проверяемы буквально (конкретные + входные последовательности состояний, конкретные пороги времени, конкретные + ожидаемые поля). AC1–AC6, AC8 — это ровно надмножество исходных шести AC из + тела issue (сверено построчно), AC7/AC9/AC10/AC11 — обоснованные усиления + (рестарт, регресс, mutation-guard, гейты). +- **Продуктовая неоднозначность (Q1–Q4)** — корректно вынесена автору ТЗ + анализом-агентом до написания спеки, с default-вариантами и явным + trade-off; ни одна догадка не выдана в тексте спеки за факт — раздел 13 + («принятые предположения») содержит только технические решения (module-level + константа, fail-closed на malformed timestamp, судьба существующего + `previous` при resume), которые ревьюер имеет право оспорить, но не находит + спорными. +- **DoR-пригодность** — миграция/compatibility (нет новых полей, формат Store + неизменен), i18n (нет новых ключей), touch/accessibility («не затрагиваются», + подтверждено — новых controls нет), производительность («O(1) на точку»), + откат (revert коммита, Store совместим) — все пункты §2.5 закрыты текстом ТЗ. +- **Формальности** — файл лежит по конвенции `docs/specs/-.md`, + двусторонняя ссылка issue ↔ ТЗ на месте, `docs/specs/README.md` обновлён в + верной секции (`P1`), трейлеры коммита (`Issue: #205`, `User-Visible: no`) + корректны для документационного коммита. + +## Чего не проверял + +- Исполнение/падение автотестов — их не существует, писать код на этом этапе + запрещено (Rule #1). +- Реальное поведение живого робота/симулятора демо-стенда — вне этапа spec. +- Будущие правки `docs/VACUUM.md`/`docs/USER-GUIDE.ru.md`/`docs/TESTING.md` — + раздел 12 корректно относит их к release-артефактам реализации, а не к этому + документу. + +## Вердикт + +Зелёный. High: 0. Medium: 0. Low: 1 (снят с записью, см. выше). ТЗ готово к +разработке.