From 53f91d974a8401a3c4b82fbb3ff68d15297263b2 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Thu, 3 Sep 2026 17:10:17 +0000 Subject: [PATCH] docs: review document for #162 Issue: #162 User-Visible: no --- docs/reviews/CODE-REVIEW-162-r2.md | 177 +++++++++++++++++++++++++++++ 1 file changed, 177 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-162-r2.md diff --git a/docs/reviews/CODE-REVIEW-162-r2.md b/docs/reviews/CODE-REVIEW-162-r2.md new file mode 100644 index 00000000..1b71921c --- /dev/null +++ b/docs/reviews/CODE-REVIEW-162-r2.md @@ -0,0 +1,177 @@ +# Код-ревью #162 · заход r2 + +Вердикт: зелёный · заход r2 · блокирующих циклов 1/4 · High: 0 · Medium: 0 + +## Скоуп раунда + +Предыдущий код-ревью (r1) прошёл на HEAD `4032b810` по тексту вердикта, но +фактически ревьюемое дерево включало и `8e2892b9` (CI-фиксы, запушены +2026-09-03 19:42, до публикации вердикта в 19:47) — документ `fe18f80f` +(`docs: review document for #162`) физически является потомком `8e2892b9` +(`git log --format='%h %p' -1 fe18f80f` → `fe18f80f 8e2892b9`). Это +несоответствие SHA в тексте вердикта самому дереву — сам факт не критичен +(правки внутри 8e2892b9 не оспаривались), но найден только чтением git-графа, +а не заявлен явно. Базой для дельты этого раунда взят `8e2892b9`, а не +заявленный `4032b810`, как единственный SHA, действительно совпадающий с +деревом, на котором был вынесен вердикт. + +Разбор — по дельте, а не заново: `git diff 8e2892b9..HEAD` (HEAD = `49f69f60`), +две правки-коммита автора (`4b8cf6e0`, `49f69f60`) плюс автоматический +коммит публикации ревью (`fe18f80f`, не код). Дельта закрывает ровно две +находки r1 и не трогает ничего вне их: резолвер маршрутов, серверный +recorder, валидацию config/set, space-deletion, экспорт — не менялись. + +## Как проверялось + +- Прочитаны оба изменённых участка кода целиком: + `src/houseplan-editor-runtime.ts` (`_vacAutoCalibrate`, `_vacApplyCalibrationProposal`), + `src/editors/vacuum-maps-section.ts` (новый блок «Добавить источник карты»). +- Прослежена цепочка `calibrationTarget()` (`src/vacuum-route-edit.ts:153-163`, + не менялась в этой дельте) → `_vacAutoCalibrate` → `_vacCalConfirm.space` → + `_vacApplyCalibrationProposal` — вручную сверено, что `target.space` есть + `route?.space || dockSpace`, то есть пространство маршрута, а не дока. +- Прогнаны гейты лично (см. раздел «Гейты»), включая точечный прогон нового + мутанта и восьми смоков с прямым совпадением символов дельты. + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где это видно | +|---|---|---| +| **High** — `_vacApplyCalibrationProposal` коммитил `dev.space` (пространство дока) вместо пространства маршрута при ручной донастройке после high-residual auto-calibration; AC8 нарушался в штатной ветке | `_vacCalConfirm` получил поле `space?: string`; `_vacAutoCalibrate` кладёт в него `target.space` (`src/houseplan-editor-runtime.ts:11035`); `_vacApplyCalibrationProposal` коммитит `proposal.space \|\| dev.space` вместо голого `dev.space` (`src/houseplan-editor-runtime.ts:11055-11056`) | Код: `houseplan-editor-runtime.ts:11029-11035` (запись `space: target.space`) и `:11054-11058` (чтение с фолбэком). Доказательство: `demo/smoke_vacuum_multifloor.mjs:87-104` — новый сценарий «док на f1, маршрут vr2/m2 на garden, высокий residual → ручная подгонка» проверяет `_space === 'garden'` и `_vacFit.routeId === 'vr2'`. Новый мутант `vacuum-manual-fit-after-proposal-uses-the-dock` (`scripts/mutation-gate.mjs`) возвращает `const space = dev.space` — лично прогнан, красный, как обязан (см. «Гейты») | +| **Medium** — §9.3 ТЗ «Добавить источник карты» (отдельная camera на карту) не реализовано; второй сценарий интеграции из тела issue отсутствовал | В `src/editors/vacuum-maps-section.ts` добавлен раскрывающийся блок «Добавить источник карты» (`
`), список — кандидаты из `sources.candidates` (тот же `VacSourceResolution`, что и у общего picker + ленивой секции «Все камеры», без дублирующего вычисления), источник без читаемого map id показан неактивным с причиной | Код: `vacuum-maps-section.ts:101-120` (функция `addSource`, список `spare`) и `:212-227` (рендер). Переводы: `vac.route_add_source`, `vac.route_add_source_hint`, `vac.route_source_no_map` добавлены во все 4 языковых файла `src/i18n/support/*.json`, синхронность подтверждена `test/i18n.test.mjs` (счётчик ключей 74→77, тест прогнан в составе `npm test`). Ручной сценарий добавлен в `docs/TESTING.md:84-85` | + +Обе находки закрыты по существу, не косметически: High доказан мутантом, +который ловит именно тот регресс, что описан в находке (коммит пространства +дока вместо маршрута); Medium — код читает тот же источник кандидатов, что и +уже проверенный picker, поэтому не вносит второй способ получить тот же +список камер. + +## Унаследовано из r1 (без повторной проверки) + +Со ссылкой на `docs/reviews/CODE-REVIEW-162-r1.md`, вынесенный на SHA `8e2892b9` +(де-факто; в тексте вердикта ошибочно назван `4032b810`, см. «Скоуп раунда»): + +- Резолвер маршрутов и его чистые контракты (`src/vacuum-routes.ts`), включая + 6 исходов резолвинга и `adoptLegacyRun` — не изменялись в дельте, доверяю + прежнему прочтению. +- Питоновское зеркало (`custom_components/houseplan/vacuum_routes.py`) и + серверный recorder/GC прогонов (`trails.py`) — не изменялись. +- Валидация `map_routes` на `config/set` и в обоих import-flow, а также + поведение `space-deletion.ts`/`space-reference-repair.ts` — не изменялись. +- Храповик ядровых файлов и ленивый граф (§3.4 ТЗ) — перепроверены заново в + этом раунде отдельным тестом (см. «Гейты»), так как дельта добавляла код + именно в эти файлы; расхождений с r1 не найдено (потолки соблюдены). +- 13 из 15 мутантов (кроме нового `vacuum-manual-fit-after-proposal-uses-the-dock` + и ранее заявленных восьми из спецификации) — не перепрогонялись, так как + защищаемый ими код не в дельте. +- backend-тесты (`pytest tests_backend`, 359 passed по r1) — не перепрогонялись, + дельта не трогает `custom_components/**/*.py` (подтверждено `git diff --stat`). +- Golden/performance — не гонялись ни в r1, ни здесь: вне AC, canonical запуск + остаётся пре-релизным гейтом. + +## Находки + +Нет. High: 0, Medium: 0. + +Побочное наблюдение, не находка: в комментарии автора после `49f69f60` +заявлено «`npm test` 1856/0/1». Мой личный прогон (дважды, стабильно) +даёт 1854 pass / 0 fail / 1 skip (1855 всего). Расхождение на 2 теста не +объясняется дельтой (в ней нет новых `test(...)`, кроме одной изменённой +числовой проверки в `test/i18n.test.mjs`, которая ассертит количество ключей, +а не добавляет тест-кейс). Похоже на опечатку в отчёте, а не на нестабильный +прогон или пропущенные файлы: `fail: 0` в обоих случаях, набор тестов +одинаков. Не блокирует — актуальный результат зафиксирован мной лично и он +зелёный. + +## Гейты — что прогнал и результат + +- `npx tsc --noEmit` — чисто. +- `npm test` — **1854 pass / 0 fail / 1 skip** (см. наблюдение выше про + расхождение с отчётом автора). +- `npm run build` — собирается; `dist/houseplan-card.js` побайтово совпадает + с `custom_components/houseplan/frontend/houseplan-card.js`; JSON-сверка + `dist/houseplan-assets.json` ↔ `custom_components/.../houseplan-assets.json` + — идентичны. Третья копия (репозиторная `dist/`) — совпадает с только что + собранной, т.е. все копии синхронны. +- `node scripts/check-docs.mjs` — passed (7 файлов, 10 внешних ссылок); + прогнан, так как дельта трогает `src/**`. +- `node --test test/core-file-budget.test.mjs` — 7/7, потолки соблюдены + (`houseplan-card.ts` 13606/13659, `houseplan-editor-runtime.ts` + 14320/14323). +- `npm run bundle:budget` — initial View 292978 Б gzip при потолке 294000±2000; + предупреждение про запас < 15000 Б воспроизводится, но это существующий, + учтённый долг (#367, потолок поднят коммитом `51b4cc7e` в этой же ветке до + начала r1), дельта добавила к нему ~7 байт — не новый долг этого раунда. +- `node scripts/smoke-select.mjs --base 8e2892b9 --head HEAD` — 217 смоков в + матрице; 8 «прямых совпадений» по символам дельты (`_vacCalConfirm`, + `_vacFit`, `_commitSpace`, `DevItem`): `smoke_vacuum_multifloor`, + `smoke_vacuum_firstuse`, `smoke_controls`, `smoke_danger_confirm_branches`, + `smoke_device_position_history`, `smoke_editor_gestures`, `smoke_fixed_floor`, + `smoke_vacuum`. Все восемь прогнаны лично — OK. +- `node scripts/mutation-gate.mjs --id=vacuum-manual-fit-after-proposal-uses-the-dock` + — «чистый прогон» OK, мутант краснеет, как обязан (1 из 1 пойман). Остальные + 14 мутантов не перегонялись — их код не в дельте (унаследованы из r1). +- `node --test test/single-source-numbers.test.mjs` — 3/3 (уже входит в + `npm test`, прогнан отдельно для явности: дельта добавляет пользователю + видимое число — map id рядом с каждым источником в новом блоке — источник + этого числа один: `host._vacObservedMapId(dev, candidate.entityId)`, + вызывается и для отображения, и для записи маршрута, не кешируется отдельно + для каждого места). + +### Что не проверял и почему + +- `python -m pytest tests_backend` — не прогонял в этом раунде: дельта не + касается ни одного `custom_components/**/*.py` файла (подтверждено + `git diff --stat 8e2892b9..HEAD`). Результат r1 (359 passed) наследуется. +- `npm run invariants` — не прогонял: дельта не трогает рёбра комнат, записи + толщины, `layout`, `marker.space` или `open_spans`. Поле `route.space` + внутри `marker.vacuum.map_routes[]` — не то же самое, что `marker.space`, и + не является геометрическим ключом решётки. +- `npm run golden:verify` — не прогонял: дельта не меняет геометрию, стили или + слои плана — только текст/поведение диалога в редакторе устройства и текст + предупреждения. Визуальный рендер плана не затронут. +- Полная матрица `demo/smoke_*.mjs` (217 файлов) — не прогонял целиком: дельта + локальна (2 файла кода + переводы), а `scripts/smoke-select.mjs` явно указал + восемь прямых совпадений без «неопределённостей» и без слабых связей, + требующих отдельного решения. +- Остальные 13 из 15 заявленных мутантов — не перепрогонял: они охраняют код, + не входящий в дельту этого раунда (унаследовано из r1, где они уже были + лично прогнаны и покраснели). +- `docs/USER-GUIDE.ru.md` — сверил текст §«Калибровка» (строки 1497-1512): + формулировка уже достаточно общая («каждую карту нужно сопоставить + пространству и откалибровать именно в нём»), не называет конкретные кнопки + и не требует правки под новую «Добавить источник карты». +- Трейлеры `User-Visible: no` на обоих коммитах (`4b8cf6e0`, `49f69f60`) + проверены на соответствие проектной конвенции: во всей ветке этой задачи + только коммиты, впервые вводящие уже отраженную в `docs/CHANGELOG*.md` + возможность, помечены `yes` (`c38501ef`, `ca6ebc30`, `567b4568`); все + доводочные фиксы после них, включая более ранний `4032b810` (та же + категория бага — пространство калибровки), помечены `no`. Обе правки этого + раунда доводят уже анонсированное в «Не выпущено» поведение + (`docs/CHANGELOG.ru.md:16-19` уже обещает «каждую карту можно сопоставить + пространству и откалибровать её именно в этом пространстве» — без уточнения + механизма) до соответствия описанию, а не добавляют новую строку продукта. + Противоречия конвенции не нашёл. + +## Итог + +Обе находки r1 закрыты по существу и доказаны (одна — мутантом, ловящим +именно описанный регресс; другая — переиспользованием уже проверенного +источника кандидатов без дублирования). Новых находок в дельте нет. Гейты, +соразмерные объёму дельты, зелёные. + +--- + + + +## Материал раунда + +- Ветка: `issue/162-vacuum-map-space-routing`, коммит `49f69f60dbd5` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `60cc4393ee53dac00659790870ee3dcb03ad3248` + ``` + git log --all --format='%H %T' | grep 60cc4393ee53 + ``` +- ТЗ `docs/specs/162-vacuum-map-space-routing.md`, блоб `a8edcfbe6c37700095f6f8582258eac4fcb58ad9` + ``` + git log --all --find-object=a8edcfbe6c37700095f6f8582258eac4fcb58ad9 -- docs/specs/162-vacuum-map-space-routing.md + ```