mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-06 14:39:22 +00:00
claude[bot]
@@ -0,0 +1,170 @@
|
||||
# SPEC-REVIEW-443-r1
|
||||
|
||||
- Issue: https://github.com/Matysh/houseplan-card/issues/443
|
||||
- Этап: ревью ТЗ (PROCESS.md §2.4)
|
||||
- Заход: r1 (первый; более ранних вердиктов в issue нет)
|
||||
- Материал: `docs/specs/443-vacuum-route-polish.md` на ветке
|
||||
`issue/443-vacuum-route-polish`, коммит `a488e898` (тот же SHA, на котором
|
||||
Validate уже зелёный — https://github.com/Matysh/houseplan-card/actions/runs/33799322137)
|
||||
- Трек: полный (обоснование в аналитике корректно называет нарушенные критерии
|
||||
`small`: второй язык бэкенда, новый визуальный UX-контракт группы, i18n,
|
||||
performance-sensitive путь)
|
||||
|
||||
## Скоуп
|
||||
|
||||
Три узких регресса вокруг маршрутов карт робота, унаследованных от #162/#441:
|
||||
|
||||
1. `map_routes: []` трактуется как «маршрутов не настраивали» и оживляет
|
||||
legacy `calibration` (frontend `effectiveRoutes()`, backend
|
||||
`effective_routes()`), а single-space export превращает пустой список в
|
||||
`null`, теряя эту же явную пустоту при повторном чтении.
|
||||
2. Строки с удалённым пространством в Device editor не образуют явной группы.
|
||||
3. `_renderVacuums` пробегает весь `_renderDevices` (все устройства плана), а
|
||||
не только роботов, на каждый кадр каждого пространства.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Ревью ТЗ, диагноз не берётся на слово — каждое из трёх утверждений сверено с
|
||||
текущим кодом на `origin/dev` (тем же деревом, от которого ветка отпочкована):
|
||||
|
||||
| Утверждение ТЗ | Файл:строка | Результат сверки |
|
||||
|---|---|---|
|
||||
| `explicit.length` вместо `Array.isArray` (frontend) | `src/vacuum-routes.ts:136` | подтверждено: `if (Array.isArray(explicit) && explicit.length)` |
|
||||
| та же ошибка в backend | `custom_components/houseplan/vacuum_routes.py:113` | подтверждено: `if isinstance(explicit, list) and explicit:` |
|
||||
| `kept_routes or None` теряет explicit-empty при экспорте | `custom_components/houseplan/import_export.py:592` | подтверждено дословно |
|
||||
| comparator строк маршрутов уже детерминирован (пункт (б) issue не подтвердился, что сам автор и написал) | `src/editors/vacuum-maps-section.ts:153-155` | подтверждено: sort по space→map_id→source, без `Math.random`/недетерминизма; отдельной группы для missing-space нет |
|
||||
| `_renderVacuums(this._renderDevices, …)` сканирует весь ростер, фильтр `_isVacDev` — внутри цикла | `src/houseplan-card.ts:11794`, `12365-12367` | подтверждено |
|
||||
| capture (`_captureRenderDeviceSnapshot`) уже пробегает все устройства для vacuum facts | `src/houseplan-card.ts:4776-4822` | подтверждено, `O(N)` там неизбежен и ТЗ не пытается его убрать |
|
||||
| `RenderDeviceSnapshot` — место, куда добавить `vacuumDevices`-подобное поле, архитектурно не конфликтует | `src/render-device-snapshot.ts` | подтверждено — интерфейс уже неймспейс read-only коллекций поверх снапшота |
|
||||
| performance-фикстура 60 комнат/200 устройств существует | `demo/performance/README.md:4`, `demo/fixtures/large-house.mjs:132` (1 vacuum) | подтверждено, `V=1 « N=200` — structural witness воспроизводим |
|
||||
| i18n-ключ для «пространство удалено» уже существует построчно | `src/i18n/support/{en,ru,de,fr}.json:64` (`vac.route_status_missing_space`) | подтверждено — новый заголовок группы не изобретает лексику с нуля |
|
||||
| `docs/CONFIG-COMPATIBILITY.md` уже имеет посвящённый `map_routes` раздел, но не описывает explicit-empty | `docs/CONFIG-COMPATIBILITY.md:80-103` | подтверждено — раздел «Vacuum map routes (#162)» молчит именно о том различии, которое чинит эта задача |
|
||||
| `docs/USER-GUIDE.ru.md` уже описывает «Карты и этажи» как раздел Device editor | `docs/USER-GUIDE.ru.md:1504-1511` | подтверждено — это ровно то место, где появится новая группа |
|
||||
|
||||
Дополнительно проверено: `docs/specs/README.md` получил корректную строку
|
||||
таблицы со ссылкой на новый файл (двусторонняя трассируемость §7.1 на месте).
|
||||
|
||||
Гейты (`typecheck`/`test`/`build`) не перегонялись: этап — ревью ТЗ, диапазон
|
||||
изменений — только `docs/specs/**`, а Validate на этом же SHA уже зелёный
|
||||
(см. материал выше). Продуктовый код ещё не написан, гонять его не на чем.
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium (в скоупе) — раздел «Release-артефакты» не называет два канонических документа, которые эта же правка обязана задеть
|
||||
|
||||
ТЗ явно перечисляет release-артефакты (обязательное требование
|
||||
`docs/specs/README.md`: «затронутую пользовательскую документацию») и
|
||||
называет только `docs/VACUUM.md` плюс оба changelog. Но:
|
||||
|
||||
- **`docs/CONFIG-COMPATIBILITY.md`** уже содержит выделенный раздел «Vacuum map
|
||||
routes (#162)» (строки 80–103), который сейчас говорит про `map_routes`
|
||||
только «absence reads as the historical behaviour» и «a single-space export
|
||||
drops routes that point at other spaces…» — то есть описывает ровно ту
|
||||
область, где ТЗ фиксирует новое поведение (explicit-empty против
|
||||
absent/`null`, сохранение `[]` при single-space export), и после реализации
|
||||
этот раздел будет неполным/устаревшим, если его не обновить в том же
|
||||
коммите. AGENTS.md прямо называет `CONFIG-COMPATIBILITY.md` каноническим
|
||||
документом подсистемы, который проверяется при задачах, трогающих
|
||||
геометрию/ссылки/конфиг-семантику — а #443 меняет именно read-semantics
|
||||
сохранённого поля.
|
||||
- **`docs/USER-GUIDE.ru.md`** — раздел «Многоэтажный робот настраивается в
|
||||
блоке «Карты и этажи»» (строки 1504–1511) уже описывает поведение этого же
|
||||
экрана Device editor, где появится новая видимая группа «Пространство
|
||||
удалено». Пользовательская документация интерфейса, которую AGENTS.md прямо
|
||||
требует читать и не изобретать заново, не названа в списке артефактов.
|
||||
|
||||
Почему это Medium, а не Low: раздел «release-артефакты» в ТЗ — это чек-лист
|
||||
DoR (§2.5 PROCESS.md: «release-артефакты по правилу `docs/specs/README.md`»),
|
||||
и его неполнота на этапе ТЗ означает, что задача при переходе в `S5-ready`
|
||||
пройдёт DoR формально невыполненным пунктом, если ревьюер это пропустит.
|
||||
Практический риск — ровно тот, что уже случался в проекте (#237): правка
|
||||
фронтенда/бэкенда меняет поведение, а специализированный канонический документ
|
||||
остаётся молчать про новое поведение до следующей находки постфактум.
|
||||
|
||||
Фикс — чисто текстовый и умещается в этом же ТЗ: добавить оба документа в
|
||||
список «Release-артефакты», без изменения AC, контракта или скоупа.
|
||||
|
||||
### Low — «Карта реализации» называет несуществующий путь для i18n
|
||||
|
||||
ТЗ пишет `src/locales/{en,ru,de,fr}.ts`, но действующие файлы, где уже лежит
|
||||
`vac.route_status_missing_space` и куда естественно ляжет новый ключ
|
||||
заголовка группы, — `src/i18n/support/{en,ru,de,fr}.json` (проверено чтением).
|
||||
Раздел «Карта реализации» — не нормативная часть ТЗ (это ровно то, что
|
||||
PROCESS.md называет решением автора: «раскладка файлов» решают агенты сами),
|
||||
поэтому это не блокирует и не требует отдельного цикла; снимаю с записью —
|
||||
автор может поправить путь при следующей правке ТЗ заодно с Medium-находкой,
|
||||
отдельного возврата это не заслуживает.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Оба продуктовых раздела на месте первыми («Сценарий», «Что человек увидит»)
|
||||
и отвечают на персону/поверхность/момент без терминов реализации.
|
||||
- Скоуп/не-скоуп разделены чётко; не-скоуп явно исключает миграцию, изменение
|
||||
route identity, авто-выбор нового пространства — ровно те решения, которые
|
||||
иначе потребовали бы продуктового вопроса владельцу.
|
||||
- Контракт §1 (таблица authority на 4 состояния поля) однозначен и уже сверен
|
||||
построчно с обоими рантаймами — реализация не оставляет выбора автору.
|
||||
- Контракт §2 (single-space export) точно называет входное/выходное состояние
|
||||
и не путает `null`/`[]`/absent.
|
||||
- Контракт §3 (группа) описывает порядок, локализацию, сохранение построчного
|
||||
статуса и явно разводит новую группу с pending draft #441 — не оставляет
|
||||
открытого вопроса по UX.
|
||||
- Контракт §4 (vacuum-only snapshot) корректно ограничивает область
|
||||
оптимизации только повторным render-сканом, не трогая `O(N)` capture, и
|
||||
явно требует global (не per-floor) subset — иначе межэтажный сценарий #162
|
||||
сломался бы тихо. Технически подтверждено, что `RenderDeviceSnapshot` можно
|
||||
расширить без конфликта архитектуры.
|
||||
- Все восемь AC пронумерованы, у каждого указан способ доказательства (TS
|
||||
unit / Python unit / backend test / browser smoke / unit-contract /
|
||||
существующий smoke / performance smoke / typecheck+build+docs), включая
|
||||
требование к отрицательному прогону в AC2 («минимум один кейс обязан
|
||||
падать при возврате старого условия»).
|
||||
- План автотестов явно называет обновление `scripts/mutation-gate.mjs` двумя
|
||||
новыми мутантами (explicit-empty regression, cross-floor devices leak) —
|
||||
соответствует дисциплине «тест умеет падать» из PROCESS.md §2.7 ещё на
|
||||
этапе планирования.
|
||||
- «Принятые предположения» в конце корректно отделяют техническое решение
|
||||
(`map_routes: null` остаётся legacy-compatible; нет нового latency budget)
|
||||
от уже принятых владельцем фактов (#162 truthiness-authority), и ни одно из
|
||||
них не выдаёт догадку о видимом поведении за факт.
|
||||
- Открытых продуктовых вопросов действительно нет: единственные развилки —
|
||||
технические (где резать snapshot, как называть i18n-ключ), и ТЗ решает их
|
||||
само, как и требует PROCESS.md §7.1.
|
||||
- i18n-строки для группы не изобретены с нуля: предложенный текст
|
||||
«Пространство удалено»/«Deleted space» дословно совпадает с уже
|
||||
существующим построчным `vac.route_status_missing_space`.
|
||||
- Одно число — один источник: у этой задачи нет нового пользовательского
|
||||
числового значения (только authority-переключение, группировка строк и
|
||||
внутренний render-scan), поэтому правило неприменимо — явно проверено и
|
||||
отклонено, а не молча пропущено.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не запускал `typecheck`/`test`/`build` — на этом SHA нет кода класса A/B,
|
||||
только `docs/specs/**`; Validate уже зелёный на этом же SHA (см. материал).
|
||||
- Не проверял golden/смоки/perf — они относятся к предрелизному гейту и коду,
|
||||
которого ещё нет; ТЗ верно относит их к implementation loop, а не к этапу ТЗ.
|
||||
- Не оценивал качество будущей реализации `scripts/mutation-gate.mjs` мутантов
|
||||
— они только описаны в плане, конкретных диффов ещё нет.
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- SHA: `a488e898` (ветка `issue/443-vacuum-route-polish`)
|
||||
- Дерево ТЗ: `docs/specs/443-vacuum-route-polish.md`
|
||||
- Round: r1 (первый заход, budget циклов 0/4)
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/443-vacuum-route-polish`, коммит `a488e8988960` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `76540aeb10d58597b76dff552309e5249b3d23e3`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 76540aeb10d5
|
||||
```
|
||||
- ТЗ `docs/specs/443-vacuum-route-polish.md`, блоб `0da1c9d2318469b2b787d0b282cb11a759ee1e1d`
|
||||
```
|
||||
git log --all --find-object=0da1c9d2318469b2b787d0b282cb11a759ee1e1d -- docs/specs/443-vacuum-route-polish.md
|
||||
```
|
||||
Reference in New Issue
Block a user