docs: review document for #205

Issue: #205
User-Visible: no
This commit is contained in:
claude[bot]
2026-08-19 20:54:57 +03:00
committed by Sergey Matyunin
parent 5c09591ce9
commit d31ad3c562
+137
View File
@@ -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/<NN>-<slug>.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 (снят с записью, см. выше). ТЗ готово к
разработке.