mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-30 19:58:50 +00:00
+165
-212
@@ -1,237 +1,190 @@
|
||||
# CODE-REVIEW — issue #441 · заход r1
|
||||
# CODE-REVIEW — issue #441 · заход r2
|
||||
|
||||
Ветка `issue/441-vacuum-route-draft`, коммит на ревью `fa41d298` (после
|
||||
приведения к dev конвейером; исходный коммит автора — `014b7480`,
|
||||
логически тот же диф). Заход первый, разбор полный.
|
||||
Ветка `issue/441-vacuum-route-draft`, HEAD на ревью `75478905` (проверено
|
||||
`git rev-parse HEAD` непосредственно перед выводом, §2.7).
|
||||
|
||||
## Скоуп
|
||||
## Почему это заход r2, а не r1
|
||||
|
||||
Баг: блок «Карты и этажи» писал второй и последующий маршрут пылесоса
|
||||
с `space: ''`, что отвергают и семантический валидатор, и
|
||||
voluptuous-схема. Кнопка «Добавить текущую карту» при этом оставалась
|
||||
активной, а отклонённая запись ничего не откатывала.
|
||||
Заголовок задачи, переданный этой сессии, утверждает «заход r1 ·
|
||||
блокирующих циклов 0 из 2». Это не подтверждается материалом:
|
||||
|
||||
Три AC (trivial-трек, ТЗ = §9.2 из `docs/specs/162-vacuum-map-space-routing.md`):
|
||||
- AC1 — черновик до выбора этажа;
|
||||
- AC2 — только валидная атомарная запись;
|
||||
- AC3 — отмена/отказ не оставляют фантом.
|
||||
- в комментариях issue #441 уже есть терминальный вердикт код-ревью:
|
||||
`Вердикт: жёлтый · заход r1 · блокирующих циклов 1/2 · High: 0 ·
|
||||
Medium: 1 → в задаче` (комментарий `claude`,
|
||||
`IC_kwDOTOcLQM8AAAABSa2lLw`, 19:35:18 UTC);
|
||||
- следом (19:35:28 UTC) владелец подтвердил: «Автоматическое ревью не
|
||||
отработало… Статусная метка не менялась» — то есть вердикт реальный,
|
||||
но шаг конвейера, переставляющий метку после вердикта (§10.4), не
|
||||
выполнился, и задача осталась висеть в `S7-code-review`, откуда её
|
||||
забрала эта сессия повторно;
|
||||
- полный документ того раунда уже закоммичен в ветку самим конвейером
|
||||
(`75478905 docs: review document for #441`, автор `claude[bot]`) —
|
||||
это штатный механизм публикации (§10.4: «кладёт документ в
|
||||
`docs/reviews/` ветки задачи»), не постороннее вмешательство;
|
||||
- SHA того раунда назван в документе — `fa41d298` — и он не осиротел:
|
||||
`git diff fa41d298..HEAD` резолвится и виден ниже.
|
||||
|
||||
Диф: `src/editors/vacuum-maps-section.ts`, `src/vacuum-route-edit.ts`
|
||||
(новый черновичный API), `test/vacuum-routes.test.mjs`,
|
||||
`demo/smoke_vacuum_route_draft.mjs` (новый), `scripts/smoke-links.mjs`,
|
||||
`docs/CHANGELOG.md` / `.ru.md`, плюс однострочная правка вызова в
|
||||
`src/houseplan-editor-runtime.ts` (убран неиспользуемый параметр
|
||||
`setVac`). Backend, i18n, View, модель геометрии не тронуты — совпадает
|
||||
с заявлением автора и с оценкой владельца.
|
||||
Таким образом это второй заход ревью, разбор — по дельте (§2.10), а не
|
||||
заново. Файл `docs/reviews/CODE-REVIEW-441-r1.md` уже существует, второй
|
||||
документ с тем же номером затёр бы его, поэтому этот — `r2`.
|
||||
Присвоенный этой сессии заголовок «r1 · 0/2», видимо, следствие того же
|
||||
сбоя конвейера, который не переставил метку: трекер раундов не увидел
|
||||
уже опубликованный r1. Само по себе это не код-находка issue #441 и не
|
||||
блокирует вердикт, но чтобы бюджет §4 не потерялся — ниже он посчитан по
|
||||
факту (см. «Вердикт»), а не по переданному заголовку.
|
||||
|
||||
## Как проверялось
|
||||
## Материал предыдущего раунда
|
||||
|
||||
Ручного тестирования в цикле нет, поэтому доказательства — тест
|
||||
(с проверкой, что он умеет падать) и чтение кода.
|
||||
- Документ: `docs/reviews/CODE-REVIEW-441-r1.md` (в этой же ветке, коммит `75478905`).
|
||||
- SHA раунда: `fa41d298` (`fix: keep vacuum routes pending until a floor is chosen`).
|
||||
- Вердикт r1: жёлтый, High 0, Medium 1 в скоупе — красный `check-docs.mjs`
|
||||
на `fa41d298` (устаревший отпечаток скриншотов документации).
|
||||
|
||||
**AC1** — `beginVacuumRouteDraft` (`src/vacuum-route-edit.ts:30-35`):
|
||||
`space: routes.length ? '' : dockSpace`. Первый маршрут (routes.length
|
||||
== 0, включая виртуальные legacy-маршруты через `effectiveRoutes`)
|
||||
наследует этаж дока, второй и следующие — пустая строка. Кнопка
|
||||
«Сохранить» в черновике: `?disabled=${!!draft.saving ||
|
||||
!spaceIds.has(draft.space)}` (`vacuum-maps-section.ts:283`) — недоступна,
|
||||
пока `space` не входит в множество существующих этажей. Доказано
|
||||
юнит-тестом `test/vacuum-routes.test.mjs:279` (АC1) — **проверил, что
|
||||
тест умеет падать**: временно вернул старую формулу
|
||||
`space: dockSpace` (без `routes.length ?`), тест 25 упал
|
||||
(`not ok 25`), после отката — снова зелёный (`node --test
|
||||
test/vacuum-routes.test.mjs`: 32/32). Дополнительно смоком
|
||||
`demo/smoke_vacuum_route_draft.mjs` (`firstRoutePreselectsDock`,
|
||||
`secondCurrentStartsBlank`, `separateSourceStartsBlank`,
|
||||
`blankDraftCannotSave`) — прогнан, зелёный.
|
||||
## Дельта r1 → r2
|
||||
|
||||
**AC2** — `commitVacuumRouteDraft` (`vacuum-route-edit.ts:45-56`)
|
||||
возвращает `null`, если `space` не входит в `spaceIds`, либо если пара
|
||||
`source`+`map_id` уже существует среди маршрутов; иначе — ровно один
|
||||
новый маршрут через `addRoute`. `writeRoutes` в `vacuum-maps-section.ts:126-137`
|
||||
конвертирует legacy весь целиком (`convertLegacyRoutes`, не тронут
|
||||
этим дифом) и передаёт кандидат в `persistRoutes` только если он не
|
||||
`null` — невалидная запись никогда не доходит до сохранения. Доказано
|
||||
юнит-тестом `test/vacuum-routes.test.mjs:287` (AC2, включая проверку
|
||||
«одну карту нельзя добавить дважды») и смоком
|
||||
(`noEmptySpaceEverSubmitted`, `rejectedPayloadWasValid`,
|
||||
`retryKeepsExactIdentity`).
|
||||
```
|
||||
git diff fa41d298..HEAD --stat
|
||||
docs/images/screenshots.json | 25 ++--
|
||||
docs/reviews/CODE-REVIEW-441-r1.md | 246 +++++++++++++
|
||||
```
|
||||
|
||||
**AC3** — `persistRoutes` (`vacuum-maps-section.ts:89-123`) сохраняет
|
||||
`previous = host._serverCfg` до применения, использует
|
||||
`optimisticAttempt`/`rollbackOptimistic` (`src/serialized-write-queue.ts`)
|
||||
для отката именно этой попытки (защищено сверкой `_cfgRev` — чужая
|
||||
более новая правка откатом не будет затёрта). `confirmDraft`
|
||||
(`vacuum-maps-section.ts:240-255`) при отказе восстанавливает исходный
|
||||
`current` (без `saving: true`), при успехе — удаляет черновик; в обоих
|
||||
случаях `pendingRoute` не трогается, если за время ожидания
|
||||
изменилась identity черновика (гонка с параллельным закрытием
|
||||
диалога). Отмена (`cancelDraft`) не вызывает `writeRoutes` вовсе —
|
||||
конфигурация не меняется. Доказано смоком: `cancelLeavesAcceptedRoutes`,
|
||||
`rejectionRestoresAcceptedMarker` (после синтетического отказа
|
||||
бэкенда первый маршрут `r1` не изменился), `rejectionKeepsEditorUsable`
|
||||
(черновик остаётся с выбранным этажом, повторное сохранение проходит
|
||||
без переоткрытия диалога).
|
||||
Два файла, оба класса C (документация/артефакт ревью). Продуктовый код
|
||||
(`src/**`), тесты и смоки, тронутые в `fa41d298`, дельтой r1→r2 не
|
||||
задеты — `git diff fa41d298..HEAD -- src test demo scripts
|
||||
docs/CHANGELOG.md docs/CHANGELOG.ru.md` пуст. Это делает дельту
|
||||
локальной по §2.10: полный повторный разбор AC не требуется, только
|
||||
проверка закрытия находки r1 плюс дешёвые гейты.
|
||||
|
||||
Прочитал also `docs/SCOPE.md` (J4/J6 — штатная настройка через GUI и
|
||||
поддержание валидности конфига), `AGENTS.md`/`PROCESS.md` (формат
|
||||
вердикта, трейлеры), issue #441 целиком и все три комментария
|
||||
(аналитика, взятие в работу, отчёт о реализации).
|
||||
`docs/reviews/CODE-REVIEW-441-r1.md` — служебный артефакт публикации
|
||||
предыдущего раунда, не правка автора; содержательного разбора не
|
||||
требует.
|
||||
|
||||
## Гейты — что прогнал и с каким результатом
|
||||
## Закрытие раунда r1
|
||||
|
||||
- `npx tsc --noEmit` — чисто, без вывода.
|
||||
- `npm test` — 1857 тестов, 1856 passed, 1 skipped, 0 fail (совпадает
|
||||
с заявлением автора). Отдельно перепрогнал
|
||||
`test/vacuum-routes.test.mjs` и подтвердил, что новые тесты AC1/AC2
|
||||
падают на буквальном откате фикса (см. выше) — дисциплина
|
||||
«тест умеет падать» соблюдена для прогнанных тестов.
|
||||
- `npm run build` и `npm run bundle:sync` — зелёные, три копии бандла
|
||||
(`dist/`, `custom_components/houseplan/frontend/`,
|
||||
`demo/srv/assets`) синхронны и байт-в-байт совпадают с
|
||||
закоммиченными после пересборки — working tree после прогона чист.
|
||||
- `node scripts/check-docs.mjs` — **КРАСНЫЙ**: `ERROR screenshot
|
||||
source fingerprint is stale; run npm run build && node
|
||||
demo/docs/capture.mjs`. Перепроверил на `origin/dev` (тот же
|
||||
чекаут скриптов) — там гейт зелёный («Documentation checks passed»),
|
||||
то есть красный статус — следствие именно этого дифа (правка
|
||||
`src/editors/vacuum-maps-section.ts` затронула отпечаток всего
|
||||
`src/**`), а не унаследованная проблема. Автор не называет этот шаг
|
||||
в своём списке проверок. См. находку ниже.
|
||||
- `node scripts/smoke-select.mjs --base origin/dev --head HEAD` —
|
||||
32 символа на изменённых строках, 23 «прямых совпадения» + 23 «слабые
|
||||
связи», НЕОПРЕДЕЛЁННОСТИ нет. Все прямые совпадения — это общие
|
||||
символы записи конфига (`_cfgRev`, `_saveConfigDebounced`,
|
||||
`_cfgContentFingerprint`, `_showToast`, `DevItem`, `Marker`),
|
||||
которые встречаются в диффе только потому что `persistRoutes`
|
||||
впервые применяет к путям retarget/drop уже существующий разделяемый
|
||||
механизм `optimisticAttempt`/`rollbackOptimistic` (сам механизм этим
|
||||
дифом не тронут — правка только в вызывающем коде одного файла).
|
||||
Прогнал точечно, а не весь список: `demo/smoke_vacuum_route_draft.mjs`
|
||||
(новый, основной), `demo/smoke_vacuum_firstuse.mjs` и
|
||||
`demo/smoke_vacuum_multifloor.mjs` (тема диффа — vacuum-раздел
|
||||
редактора), `demo/smoke_save_race.mjs` и `demo/smoke_config_writer.mjs`
|
||||
(тестируют именно `optimisticAttempt`/`rollbackOptimistic`/debounce,
|
||||
которые `persistRoutes` теперь использует). Все пять — зелёные.
|
||||
Остальные 18 прямых и 23 слабых совпадения не прогонял: правка не
|
||||
трогает их собственную логику (decor, sun, discovery, room resize,
|
||||
area relocation и т.д. используют те же общие поля конфигурации, но
|
||||
не код, который правка меняет), а общий примитив записи проверен
|
||||
выше двумя целевыми смоками.
|
||||
- `npm run golden:verify` — не прогонял. Диф ограничен диалогом
|
||||
редактора устройства (лениво загружаемый блок); ни один golden-сценарий
|
||||
(`demo/golden/matrix.mjs`) не рендерит этот диалог — они снимают
|
||||
View/canvas. Видимый результат View не меняется.
|
||||
- `python -m pytest tests_backend -q` — не прогонял, диф не трогает
|
||||
`custom_components/**/*.py` (только generated JS-бандл).
|
||||
Perf-профили — не прогонял, в AC не названы, путь не
|
||||
perf-чувствительный.
|
||||
- `npm run invariants -- --config ...` — не прогонял. Диф пишет
|
||||
`marker.vacuum.map_routes[].space` — по форме похоже на «ссылку на
|
||||
геометрию», но `scripts/model-invariants.mjs` эту ссылку не
|
||||
проверяет вовсе (там только `marker.vacuum.segment_map` →
|
||||
`roomId`, см. `model-invariants.mjs:150-155`); рёбра комнат, записи
|
||||
толщины, `layout`, `open_spans`, `marker.space` (то есть верхнеуровневая
|
||||
привязка маркера к этажу, а не маршрут пылесоса) этим дифом не
|
||||
затронуты. Собственная защита от dangling-ссылки — как раз предмет
|
||||
AC2 (`spaceIds.has(draft.space)`), проверена юнит-тестом и падением
|
||||
выше.
|
||||
- `test/single-source-numbers.test.mjs` — прогнан отдельно (входит в
|
||||
`npm test`), 3/3 зелёных. Дублирующегося пользовательского числа
|
||||
диф не добавляет: `mapId`/`space` — идентификаторы, а не измеряемая
|
||||
величина, и в черновике, и в подтверждённой строке читаются из
|
||||
одного и того же значения (`currentMapId`/`host._vacObservedMapId`),
|
||||
второго источника нет.
|
||||
- Трейлеры коммита `fa41d298`: `Issue: #441`, `User-Visible: yes` —
|
||||
оба CHANGELOG (`docs/CHANGELOG.md`, `docs/CHANGELOG.ru.md`) правлены
|
||||
в этом же коммите, формулировки соответствуют реализации.
|
||||
| Находка r1 | Чем закрыта | Где это видно |
|
||||
|---|---|---|
|
||||
| Medium: `node scripts/check-docs.mjs` красный на `fa41d298` — отпечаток скриншотов документации устарел после правки `src/editors/vacuum-maps-section.ts` | Коммит `0ea9c0f5` `docs: refresh canonical screenshots for #441` обновил `sourceFingerprint`/`sourceSha256` в `docs/images/screenshots.json` на все 10 сценариев; `imageSha256` каждого сценария не изменился (диалог, который правит #441, ни в одном сценарии не рендерится — пиксели те же), манифест получил `acceptance.lastWriteWasFingerprintOnly: true` — след легитимной приёмки `scripts/docs-accept.mjs` (fingerprint-only ветка `acceptedDocsManifest`, docs-accept.mjs:107), а не самодельной правки JSON | `git diff fa41d298..HEAD -- docs/images/screenshots.json`; перепрогнан `node scripts/check-docs.mjs` на HEAD `75478905` → `Documentation checks passed (7 files, 10 external links)` (было `ERROR screenshot source fingerprint is stale`) |
|
||||
|
||||
Находка закрыта корректно и через штатный инструмент приёмки
|
||||
(`docs-accept.mjs`, а не ручная правка `screenshots.json` мимо
|
||||
проверки совпадения пикселей) — именно та дисциплина, которую требует
|
||||
§8 («скриншоты снимаются только… принимаются локально `docs:accept`»).
|
||||
|
||||
## Унаследовано из r1
|
||||
|
||||
Без повторной проверки принято (документ `docs/reviews/CODE-REVIEW-441-r1.md`,
|
||||
материал — SHA `fa41d298`, тот же SHA подтверждён этой сессией как
|
||||
резолвящийся предок HEAD):
|
||||
|
||||
- **AC1** (черновик до выбора этажа) — доказательство `beginVacuumRouteDraft`
|
||||
(`src/vacuum-route-edit.ts:34`) и `test/vacuum-routes.test.mjs:279` —
|
||||
код не менялся дельтой; для собственного спокойствия эта сессия
|
||||
всё же **независимо повторила** мутацию r1 (не обязательна по §2.10,
|
||||
сделана как дешёвая проверка): временно заменила
|
||||
`space: routes.length ? '' : dockSpace` на `space: dockSpace`,
|
||||
пересобрала `test-build` (`npx tsc -p tsconfig.test.json &&
|
||||
node scripts/fix-test-build.mjs`) и прогнала
|
||||
`node --test test/vacuum-routes.test.mjs` → `not ok 25 - новый
|
||||
маршрут живёт в черновике до выбора этажа (#441 AC1)`, `# fail 1`;
|
||||
откат мутации вернул `# pass 32 / # fail 0`. Рабочее дерево после
|
||||
отката чисто (`git status --short` пуст). AC1 подтверждён на текущем
|
||||
SHA, а не только унаследован по документу.
|
||||
- **AC2** (только валидная атомарная запись) — `commitVacuumRouteDraft`
|
||||
(`src/vacuum-route-edit.ts:39-56`), `test/vacuum-routes.test.mjs:287`,
|
||||
смоки `noEmptySpaceEverSubmitted`/`rejectedPayloadWasValid`/
|
||||
`retryKeepsExactIdentity` — код не менялся, наследуется по документу r1.
|
||||
- **AC3** (отмена/отказ не оставляют фантом) — `persistRoutes`/`confirmDraft`
|
||||
(`src/editors/vacuum-maps-section.ts`), смоки
|
||||
`cancelLeavesAcceptedRoutes`/`rejectionRestoresAcceptedMarker`/
|
||||
`rejectionKeepsEditorUsable` — код не менялся, наследуется по документу r1.
|
||||
- Наблюдение r1 про побочное усиление `retarget`/`drop` (переход на общий
|
||||
`persistRoutes`/`rollbackOptimistic`) и про отсутствие новых ключей i18n —
|
||||
без изменений, код не тронут дельтой.
|
||||
- Оценка `docs/SCOPE.md` (J4/J6) — без изменений, продуктовый скоуп не
|
||||
менялся.
|
||||
|
||||
## Гейты — что прогнал в этом раунде и с каким результатом
|
||||
|
||||
Дельта r1→r2 не трогает `src/**`/`test/**`, поэтому по §2.10 достаточно
|
||||
дешёвых гейтов; они прогнаны заново на HEAD `75478905`, а не унаследованы:
|
||||
|
||||
| Гейт | Команда | Результат |
|
||||
|---|---|---|
|
||||
| Typecheck | `npx tsc --noEmit` | чисто, без вывода |
|
||||
| Тесты | `npm test` | `# tests 1857 · # pass 1856 · # fail 0 · # skipped 1` — совпадает с заявленным в r1 |
|
||||
| Сборка + синхронизация бандла | `npm run build`, затем `git status --short` | `dist` пересобран, working tree чист после сборки — `dist/houseplan-card.js` и `custom_components/houseplan/frontend/houseplan-card.js` совпадают в коммите (`cmp` → без вывода, идентичны) |
|
||||
| Документация | `node scripts/check-docs.mjs` | **зелёный**: `Documentation checks passed (7 files, 10 external links)` — находка r1 закрыта (см. выше) |
|
||||
| Мутация AC1 (доп. проверка этого раунда) | временный откат `src/vacuum-route-edit.ts:34` → `space: dockSpace`, `npx tsc -p tsconfig.test.json && node scripts/fix-test-build.mjs && node --test test/vacuum-routes.test.mjs` | `not ok 25` на мутанте, `# pass 32 / # fail 0` после отката — дисциплина «тест умеет падать» подтверждена независимо |
|
||||
| Смок AC1–AC3 | `node demo/smoke_vacuum_route_draft.mjs` (после `npm run bundle:sync`, иначе смок падает на отсутствующем `demo/srv/assets/houseplan-card.js` — сгенерированный, не коммитится, #255) | все 14 проверок `true`, `OK` |
|
||||
|
||||
Не прогонялись повторно (дельта их не задевает, обоснование см. в
|
||||
документе r1, наследуется): `smoke-select.mjs` по полному диффу,
|
||||
`golden:verify`, `pytest tests_backend`, `npm run invariants`,
|
||||
performance-профили, остальные точечные смоки (`smoke_vacuum_firstuse`,
|
||||
`smoke_vacuum_multifloor`, `smoke_save_race`, `smoke_config_writer`) —
|
||||
их результат на `fa41d298` уже зафиксирован в r1 и код, который они
|
||||
покрывают, не менялся.
|
||||
|
||||
Трейлеры дельты: `0ea9c0f5` — `Issue: #441`, `User-Visible: no` (верно:
|
||||
обновление служебного отпечатка, а не пользовательского поведения,
|
||||
changelog не требуется); `75478905` — `Issue: #441`, `User-Visible: no`
|
||||
(служебный артефакт публикации ревью, класс C).
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium (в скоупе, чинится в этой же ветке) — стал причиной жёлтого вердикта
|
||||
|
||||
**Отпечаток скриншотов документации устарел, `check-docs.mjs` красный
|
||||
на `fa41d298`.**
|
||||
- Файл: `docs/images/screenshots.json` (не обновлён), причина —
|
||||
`src/editors/vacuum-maps-section.ts`.
|
||||
- Воспроизведение: `node scripts/check-docs.mjs` на текущем HEAD →
|
||||
`ERROR screenshot source fingerprint is stale; run npm run build &&
|
||||
node demo/docs/capture.mjs`; тот же скрипт на `origin/dev` проходит
|
||||
чисто — красный статус специфичен для этого дифа.
|
||||
- Почему это не мелочь: ровно этот пропуск в #230 и #234 оставил
|
||||
`dev` с красным job `docs` до следующей задачи (#237) — то есть при
|
||||
мёрже без правки CI-джоб `docs` станет красным на `dev` для всех
|
||||
последующих задач, пока кто-то не поймает и не почини́т его
|
||||
отдельно.
|
||||
- Что нужно: `npm run build && node demo/docs/capture.mjs`, закоммитить
|
||||
обновлённые `docs/images/screenshots.json` и (если изменились пиксели)
|
||||
сами PNG, затем перепрогнать `node scripts/check-docs.mjs` до
|
||||
зелёного.
|
||||
|
||||
Больше High/Medium в скоупе не найдено. AC1–AC3 выполнены и доказаны;
|
||||
рассмотренный продуктовый сценарий (второй и следующие маршруты через
|
||||
штатный UI) решается, регрессии в соседний сценарий (первый маршрут,
|
||||
retarget, drop, легаси-конверсия, `trail_mode`/`live`) не внесено —
|
||||
подтверждено чтением и прогоном смоков.
|
||||
Нет. Единственная находка r1 (Medium, в скоупе) закрыта корректно и
|
||||
через штатный инструмент. Новых находок дельта r1→r2 не вносит — она
|
||||
состоит из точечного обновления отпечатка документации и служебного
|
||||
коммита самого конвейера.
|
||||
|
||||
## Что проверено и корректно (без отдельной находки)
|
||||
|
||||
- Сигнатура `renderVacuumMapsSection` лишилась неиспользуемого
|
||||
параметра `setVac`; единственный вызов в
|
||||
`src/houseplan-editor-runtime.ts:10973` обновлён синхронно, `tsc`
|
||||
подтверждает отсутствие расхождений.
|
||||
- Побочный эффект дифа: `retarget`/`drop` (переещё раньше существовавшие
|
||||
операции) раньше писали через `this._saveConfig()` (дебаунс, без
|
||||
отката при отказе бэкенда — тот самый пробел, который отдельно
|
||||
трекает #439) и теперь используют тот же атомарный
|
||||
`persistRoutes`/`rollbackOptimistic`, что и новый draft-флоу. Это не
|
||||
требовалось ни одним AC #441, но не является регрессией: это
|
||||
строгое усиление (retarget/drop теперь тоже откатываются при
|
||||
отказе), сам примитив уже покрыт `smoke_save_race.mjs` и
|
||||
`smoke_config_writer.mjs` (оба прогнаны и зелены выше). Отмечаю как
|
||||
наблюдение: выделенного браузерного смока именно на отказ
|
||||
бэкенда при retarget/drop нет ни до, ни после этого дифа — не
|
||||
блокирует, т.к. общий примитив протестирован, а AC #441 этого пути
|
||||
не касаются.
|
||||
- `.vacroute`/`.pending`/`.vacroute-draft-*` не имеют выделенных CSS-правил
|
||||
нигде в дереве (`grep -rn vacroute src --include=*.ts` — только сам
|
||||
файл раздела) — это унаследовано от #162, не введено этим дифом.
|
||||
- Использование `host._saveConfigDebounced.cancel()` (а не `.flush()`,
|
||||
как в большинстве других мест `houseplan-card.ts`/`houseplan-editor-runtime.ts`)
|
||||
— корректно: `persistRoutes` сразу после отмены сам вызывает
|
||||
`_saveConfigNow()` с уже актуальным `_serverCfg` (включающим любые
|
||||
ожидавшие правки), поэтому `cancel()` избегает второго избыточного
|
||||
сетевого вызова, а не теряет данные.
|
||||
- Новых ключей i18n нет (`git diff` по `src/i18n/**` пуст) — совпадает
|
||||
с заявлением аналитики в комментарии владельца.
|
||||
- `docs/SCOPE.md`: задача закрывает J4 (штатная GUI-настройка без
|
||||
YAML) и J6 (конфиг не должен становиться невалидным) — соответствует
|
||||
заявке автора, продуктовое обоснование в скоупе.
|
||||
- `docs/images/screenshots.json`: у всех 10 сценариев `imageSha256`
|
||||
не изменился при обновлении `sourceFingerprint` — подтверждает
|
||||
заявление r1, что правка не задевает ни один заскриншоченный экран
|
||||
(диалог второго маршрута пылесоса нигде не снимается для документации).
|
||||
- Приёмка прошла через `scripts/docs-accept.mjs` (ветка
|
||||
`lastWriteWasFingerprintOnly`), а не через ручную правку JSON —
|
||||
соответствует требованию §8 «коммит скриншотов делает человек» и
|
||||
«приёмка отказывает, если кандидат снят не с этого дерева»: сам
|
||||
скрипт это проверяет (`verifyDocsCandidate`), а не декларация автора.
|
||||
- `docs/reviews/CODE-REVIEW-441-r1.md` корректно оформлен как документ
|
||||
своего раунда и не требует правок задним числом (документы ревью не
|
||||
переписываются, только дополняются новым заходом).
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Ручной запуск карточки в браузере вне смоков (нет такого шага в
|
||||
этом цикле по правилам процесса) — заменено чтением кода и
|
||||
прогоном `demo/smoke_vacuum_route_draft.mjs`, который управляет
|
||||
теми же DOM-элементами, что и живой пользователь.
|
||||
- 18 из 23 «прямых совпадений» и все 23 «слабые связи» из
|
||||
`smoke-select.mjs` — см. обоснование в разделе «Гейты» (общий
|
||||
механизм записи, а не специфика этого дифа, уже покрыт двумя
|
||||
целевыми смоками).
|
||||
- `npm run golden:verify`, `pytest tests_backend`, `npm run
|
||||
invariants`, perf-профили — не запускал, обоснование по каждому — в
|
||||
разделе «Гейты».
|
||||
- Многопользовательский конфликт (два клиента редактируют один и тот
|
||||
же маркер одновременно, оба добавляют разные маршруты) — не тестовый
|
||||
сценарий этого issue; общая защита от такого конфликта — существующий
|
||||
`_cfgRev`/`rollbackOptimistic`, не изменённый этим дифом.
|
||||
- Полный повторный разбор AC1–AC3 по коду и всем смокам — не требуется
|
||||
по §2.10, т.к. дельта не трогает `src/**`/`test/**`/`demo/**`; AC1
|
||||
дополнительно перепроверен мутацией (см. «Гейты»), AC2/AC3 унаследованы.
|
||||
- `npm run golden:verify`, `pytest tests_backend`, `npm run invariants`,
|
||||
performance-профили — не прогонял; обоснование то же, что в r1
|
||||
(View, backend `.py`, геометрия/ссылки на неё не затронуты ни
|
||||
`fa41d298`, ни дельтой r1→r2).
|
||||
- `node scripts/smoke-select.mjs --base origin/dev --head HEAD` по
|
||||
полному диффу — не перегонял: набор символов на изменённых строках
|
||||
не изменился с r1 (дельта не трогает код), решение по каждому пункту
|
||||
из r1 остаётся в силе.
|
||||
- Реальность прогона CI-джобы `Docs screenshots` для коммита `0ea9c0f5`
|
||||
(не видна из репозитория) — доверие основано на структурном
|
||||
доказательстве самого `scripts/docs-accept.mjs` (совпадение
|
||||
`imageSha256` с уже закоммиченными файлами и наличие
|
||||
`acceptance.lastWriteWasFingerprintOnly`), а не на слепом заявлении
|
||||
автора; независимый пересчёт `visualFingerprint(root)` на HEAD не
|
||||
делал отдельно от `check-docs.mjs`, который эту сверку и выполняет.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Жёлтый: AC выполнены и доказаны, но `check-docs.mjs` красный на
|
||||
проверяемом SHA из-за пропущенного шага пересъёмки — Medium в скоупе,
|
||||
возврат автору на прогон `npm run build && node demo/docs/capture.mjs`
|
||||
и коммит обновлённого манифеста/скриншотов.
|
||||
Зелёный. AC1–AC3 подтверждены (AC1 — независимо на HEAD этой сессией,
|
||||
AC2/AC3 — унаследованы из r1 по неизменному коду), единственная
|
||||
Medium-находка r1 закрыта штатным инструментом и подтверждена зелёным
|
||||
`check-docs.mjs` на HEAD `75478905`. Новых находок нет.
|
||||
|
||||
Бюджет циклов (§4): r1 был жёлтым и потратил 1 цикл из 2 (лёгкий/короткий
|
||||
трек, лимит код-ревью — 2, §5.1). Этот заход (r2) зелёный и цикл не
|
||||
образует (#227) — израсходовано остаётся **1 из 2**, а не 0 из 2, как
|
||||
было в заголовке этой сессии; расхождение объяснено в разделе «Почему
|
||||
это заход r2».
|
||||
|
||||
---
|
||||
|
||||
@@ -239,8 +192,8 @@ retarget, drop, легаси-конверсия, `trail_mode`/`live`) не вн
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/441-vacuum-route-draft`, коммит `014b74805533` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `a372d3d387b4eefe464eda9079fa930f4d3f896c`
|
||||
- Ветка: `issue/441-vacuum-route-draft`, коммит `75478905d5eb` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `6e2f7f4813e3b45927eb282b1e5fdd62b872d019`
|
||||
```
|
||||
git log --all --format='%H %T' | grep a372d3d387b4
|
||||
git log --all --format='%H %T' | grep 6e2f7f4813e3
|
||||
```
|
||||
|
||||
Reference in New Issue
Block a user