Files
houseplan-card/docs/reviews/SPEC-REVIEW-162-r1.md
2026-09-03 15:26:51 +00:00

17 KiB
Raw Permalink Blame History

SPEC-REVIEW-162-r1

  • Issue: #162
  • ТЗ: docs/specs/162-vacuum-map-space-routing.md, редакция 2, коммит 086b3682 (ветка issue/162-vacuum-map-space-routing, HEAD detached на момент ревью)
  • Трек: полный (P2 bug, сложность 8/10, риск 9/10 — критерии small §5 PROCESS.md не выполнены: миграция конфига, несколько поверхностей, новый UX-контракт)
  • Заход: r1 (первый прогон S4-spec-review для этого issue — предыдущие редакции ТЗ его не проходили)
  • Вердикт: жёлтый

Скоуп

Многоэтажный робот-пылесос: House Plan хранит калибровку по map_id, но marker несёт одно space, поэтому дока и live-оверлей фактически привязаны к одному этажу. ТЗ вводит marker.vacuum.map_routes[] — явное сопоставление (source, map_id) → target space, независимое от положения дока, с resolution-контрактом, editor UI, backend trail routing, lifecycle/export и release-артефактами. Продукт: J1 (достоверное местоположение), J6 (конфигурация остаётся корректной при нескольких пространствах) — подтверждено в docs/SCOPE.md.

Задача проверялась целиком (заход r1, документ первый), раздел «Унаследовано из r0» не применим.

Как проверялось

  1. Прочитан issue #162 целиком (тело + 4 комментария аналитики/актуализации/ТЗ) через gh issue view.
  2. Прочитаны docs/SCOPE.md, AGENTS.md, PROCESS.md (весь файл, включая §2.9/§2.10, §4, §7.1–7.2, §12) и docs/USER-GUIDE.ru.md §16 (роботы-пылесосы).
  3. Прочитан канонический документ подсистемы docs/VACUUM.md целиком.
  4. Прочитано ТЗ docs/specs/162-vacuum-map-space-routing.md целиком (21 раздел).
  5. Каждое фактическое утверждение §3.2 «Подтверждённая техническая база» проверено чтением кода на текущем HEAD (086b3682, dev слит):
    • src/houseplan-card.ts:11334 — фильтр devs по d.space === space.id — подтверждено буквально;
    • src/houseplan-card.ts:12347 — выбор матрицы по calibration?.[mapNow] и silent continue — подтверждено буквально (строка if (!matrix || matrix.length !== 6) continue;);
    • src/types.ts — состав полей Marker.vacuum (семь полей, без ссылки на space) — подтверждено;
    • src/vacuum.ts:310,322 — vacMapIdFromAttrs/vacMapIdWithFallback, nullish-цепочка — подтверждено;
    • custom_components/houseplan/trails.py — run как {"map_id", "started", "ended", "points"}, can_resume_trail_run() сравнивает только map_id, поле source в записи run отсутствует вовсе — подтверждено чтением всего файла;
    • docs/USER-GUIDE.ru.md:1504 — «Для каждого map_id хранится собственная калибровка, поэтому многоэтажный робот может работать с несколькими пространствами» — подтверждено, это и есть завышенное обещание, которое ТЗ обязано снять (§18 release-артефактов).
  6. Проверено существование каждого инструмента/модуля, на который ссылается ТЗ и не которые не выдумка: scripts/mutation-gate.mjs, scripts/smoke-select.mjs, scripts/model-invariants.mjs, scripts/bundle-budget.mjs (поля initialViewFiles/lazyEditorFiles в нём есть), src/space-deletion.ts (collectSpaceMarkerDependencies, createSpaceDeletionCandidate), src/space-reference-repair.ts (repairSpaceReferences, доктрина «не трогает вложенные calibration-данные» — подтверждена дословно в шапке файла), src/render-device-snapshot.ts.
  7. Проверен «храповик» test/core-file-budget.test.mjs: потолки 13659/14323 подтверждены; фактическая длина файлов на HEAD 086b3682 — wc -l даёт 13544/14319 (в ТЗ 13545/14320 — расхождение на 1 объясняется разной трактовкой конечного перевода строки, не находка).
  8. Проверен прецедент общей TS/Python JSON-фикстуры (test/vacuum.test.mjs:446 + tests_backend/test_trail_recorder.py:425 + test/fixtures/vacuum-attrs/map-id.json) — план §16.2 «shared TS/Python route fixture» — не новый паттерн, а продолжение существующего.
  9. Прогнаны дешёвые гейты (зелёного Validate на 086b3682 нет):
    • npx tsc --noEmit → чисто, без ошибок;
    • npm test → 1819 pass / 0 fail / 1 skip — совпадает с базовой линией, заявленной в ТЗ §3.4 буквально;
    • npm run build → бандл собран без ошибок (16.1s). check-docs.mjs/model-invariants.mjs/смоки/golden не прогонялись — на этом коммите нет изменений в src/** (диф спец-ревью — только docs/specs/162-*.md), эти гейты относятся к код-ревью после реализации, не к ревью ТЗ.

Находки

Medium (в скоупе) — M1: «compatible source» в §11.3.2 не определено и не проверяемо по факту схемы run

Файл: docs/specs/162-vacuum-map-space-routing.md:537 (§11.3, пункт 2)

Формулировка ТЗ: «Run без route_id: … 2. при explicit routes сопоставляется только если ровно один route имеет тот же map_id и compatible source».

Почему это находка. Термин «compatible source» встречается в документе ровно один раз и нигде не раскрыт — ни определением, ни примером, ни блоком «принято предположительно». При этом текущая (и не меняемая этим ТЗ) схема серверного run в custom_components/houseplan/trails.py — {"map_id", "started", "ended", "points"} — не содержит поля source вообще (TrailBook.on_point, can_resume_trail_run, весь файл прочитан целиком). Легаси-run физически не хранит, с каким источником он записан — только map_id. Значит по факту хранимых данных «совместимость источника» нельзя вычислить сравнением полей run и route: это либо (а) требует ещё не описанного косвенного механизма (например: «совместим», если единственный кандидат-route на данный map_id в момент показа одновременно проходит resolver §8.2 как ready/needs_calibration, то есть реально наблюдается на этом source прямо сейчас), либо (б) реализация обязана будет угадать конкретную трактовку самостоятельно — то есть ровно тот случай, который раздел «Ambiguity is asked, not guessed» и §8.2 этого же ТЗ («Exact source/map identity никогда не ретаргетится молча») запрещают в остальных местах документа.

Это не отвлечённая формальность: от этого определения зависит AC14 («Legacy run без route id отображается только при unique match; ambiguity fail-closed | TS/backend unit») — как написано, эту проверку нельзя детерминированно закодировать unit-тестом, потому что неизвестно, что именно тест обязан признать «совместимым».

Почему Medium, не High. Вопрос не продуктовый (пользователь не видит слово «source» — он видит, показался путь или нет) и не требует владельца: это техническая формулировка алгоритма, которую автор ТЗ решает сам по §7.1 PROCESS.md («всё, чего пользователь не наблюдает, агенты решают сами»). Фикс — одна-две фразы в §11.3, например: «под "compatible source" понимается X» либо явная привязка к уже описанному в §8.2 механизму. Он не расширяет скоуп и не меняет ни одного другого раздела ТЗ.

Что нужно от автора при возврате: заменить «compatible source» точным правилом, вычислимым из данных, которые run реально хранит (или из результата resolver §8.2 на момент показа), и явно указать это правило рядом с AC14.

Что проверено и признано корректным

  • Обязательные разделы §7.1 PROCESS.md присутствуют и в правильном порядке: сценарий/персоны (§2), что видно до/после (§2), проблема (§1), скоуп/не-скоуп (§5–6), контракт поведения (§4, §7–12), UX (§9), модель данных и миграция (§7, §7.3, §20), i18n (§13), AC1…AC20 с указанным способом доказательства (§15), план автотестов (§16), риски (§19), откат (§20), release-артефакты (§18).
  • Все 20 AC пронумерованы, для каждого назван способ доказательства (unit/backend/smoke/golden/комбинации); ни один не оставлен без доказательства.
  • §16.5 «Мутанты защитных контрактов» заранее перечисляет 7 мутантов (M-A…M-G) с точным тестом-свидетелем на каждый — это ровно тот формат, который потребует код-ревью (#435/PROCESS §2.7), заложен уже на этапе ТЗ.
  • Технический раздел §3.2 «Подтверждённая техническая база» — не голословен: каждая строка кода, на которую ссылается ТЗ, проверена чтением и совпадает дословно (см. «Как проверялось», п.5). Расхождений между заявленным и фактическим поведением текущего dev не найдено.
  • §3.4 (ограничения после первой редакции — храповик core-file-budget, ленивый граф/бюджет бандла, требование мутантов, взаимодействие с space-deletion.ts/space-reference-repair.ts, снимок фактов) — все упомянутые файлы, скрипты и потолки существуют и имеют указанные значения; план реализации (§17) корректно распределяет новый код по отдельным модулям, не расширяя houseplan-card.ts/houseplan-editor-runtime.ts, что обязательно при остатке бюджета в единицы строк.
  • Overpromise, который ТЗ обязуется снять (docs/USER-GUIDE.ru.md:1504), подтверждён буквально — раздел §18 «Release-артефакты» верно называет docs/USER-GUIDE.ru.md для правки.
  • Продуктовая рамка соответствует docs/SCOPE.md: J1/J6, View/kiosk — блокирующие поверхности, editor touch — best effort (согласуется с docs/TOUCH-SUPPORT.md), Static space card корректно исключена (§6, вне скоупа по существующей доктрине — docs/VACUUM.md не описывает live-оверлей для неё).
  • Ни одного продуктового вопроса, вынесенного на автора вместо ревьюера, не обнаружено — все технические развилки (имена модулей, route id формат, раскладка файлов) явно помечены в §21 «assumed, change freely», как того требует §7.1 PROCESS.md.
  • Downgrade/rollback (§20) явно называет небезопасный путь (полный откат backend после canonical-записи) и safe-путь — соответствует требованию «откат» из чек-листа DoR §2.5.
  • Дешёвые гейты — тесты в разделе выше — зелёные и совпадают с зафиксированной в ТЗ базовой линией npm test (1819/0/1) один-в-один.

Чего не проверял

  • Смоки (demo/smoke_*.mjs), golden:verify, pytest tests_backend, performance-профили и check-docs.mjs — не прогонялись: на этом SHA нет изменений в src/**/custom_components/**/*.py (диф ревью — только текст ТЗ), эти гейты относятся к циклу код-ревью после реализации (PROCESS.md §8 прямо разделяет: «полные наборы — предрелизный/код-ревью гейт, не гейт ТЗ»).
  • Модельные инварианты (npm run invariants) — геометрия комнат/стен этим ТЗ не затрагивается (маршруты пылесоса не являются рёбрами/толщиной), поэтому не запускались.
  • Не проверялась реализация — её ещё нет, ТЗ находится на входе в S5-ready.
  • Мутанты M-A…M-G из §16.5 не прогонялись — они относятся к будущему коду, которого на этом коммите нет; на этапе ТЗ проверялось только то, что для каждого защитного контракта назван мутант и тест-свидетель (это выполнено).

Итог

Один Medium-дефект в скоупе задачи: термин «compatible source» в §11.3.2 не определён и, с учётом того что легаси-run вообще не хранит поле source, не может быть проверен буквальным сравнением данных — AC14 в текущей формулировке не поддаётся детерминированному тесту. High-находок нет, остальные 19 AC однозначны и проверяемы, техническая база ТЗ подтверждена чтением кода без расхождений. Вердикт — жёлтый: правка ограничивается одним разделом ТЗ, повторный цикл может быть точечным.


Материал раунда

  • Ветка: issue/162-vacuum-map-space-routing, коммит 086b3682e96e — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
  • Дерево материала: 2b5694be2febfde87f514b1261d14525b54a3c90
    git log --all --format='%H %T' | grep 2b5694be2feb
    
  • ТЗ docs/specs/162-vacuum-map-space-routing.md, блоб 22176219387e22288dcb3197806937edd7ad073c
    git log --all --find-object=22176219387e22288dcb3197806937edd7ad073c -- docs/specs/162-vacuum-map-space-routing.md