docs: review document for #282

Issue: #282
User-Visible: no
This commit is contained in:
claude[bot]
2026-08-26 02:21:15 +03:00
committed by Matysh
parent 0f54daf37b
commit c75c77d571
+312
View File
@@ -0,0 +1,312 @@
# CODE-REVIEW-282-r2
- **Issue:** #282 — стабильная идентичность сегментов стен (ADR Stage 1)
- **Ветка:** `issue/282-wall-geometry-model`, HEAD `3588284f` (после ребейза на `origin/dev` `30698d6e` — merge-base подтверждён, ветка полностью впереди `dev`)
- **ТЗ:** `docs/specs/282-stable-wall-segment-identity.md` (spec-review зелёный на r2, `2f30c481`)
- **Этап:** код-ревью (PROCESS.md §2.7)
- **Заход:** r2 · блокирующих циклов израсходовано 1 из 4 до этого вердикта
- **Предыдущий раунд:** `docs/reviews/CODE-REVIEW-282-r1.md`, красный, HEAD на тот момент `5adfdb5d` (SHA назван в документе — не находка). Этот SHA более не существует в истории ветки: после r1 ветка дважды уходила на ребейз (issue-комментарии `#282`, 19:33 и 21:12/21:21), причём второй ребейз (на `dev` `30698d6e`) потребовал реального разрешения конфликта — по словам автора, «post-rebase fixes also make direct backend Optimize cross the v7→v8 barrier, retain authored active-chain coordinates during conversion, and keep a rejected wall transaction atomic» — то есть новую продуктовую логику, а не только смену родителя.
- **Вердикт:** **красный**
## 1. Скоуп проверки
Раунд не локален по критериям PROCESS.md §2.10: ребейз на ушедший вперёд `dev`
с конфликтом, разрешённым новым кодом в самом identity-барьере — ровно та
подсистема, где r1 нашёл H1/H2. Поэтому разбор полный, но выполнен по факту
через дельту `9fc213c6..HEAD` (27 файлов, +1793/−1020: коммит `9fc213c6` — это
сам документ `CODE-REVIEW-282-r1.md`, его родитель `466a6765` — переименованный
рикбейзом эквивалент `5adfdb5d`, на котором получен вердикт r1). Дельта
покрывает: `src/houseplan-card.ts` (`_commitPhysicalGeometry`,
`_applyGeometryState`), `src/wall-segment-model.ts` (lineage hints, off-grid
guard), `src/plan-geometry-preflight.ts`, backend
`validation.py`/`wall_segment_model.py`/`websocket_api.py`, тесты и
mutation-gate. Остальные ~45 файлов исходного 62-файлового диффа r1 (import/export
remap, i18n, документация, backend-зеркало валидации помимо перечисленного)
дельтой не задеты — раздел «Унаследовано из r1» ниже.
## 2. Как проверялось — таблица гейтов
| Гейт | Команда | Результат |
|---|---|---|
| Typecheck | `npx tsc --noEmit` | зелёный |
| Unit/frontend | `npm test` | 1331 тестов, **1329 passed, 1 failed, 1 skipped** — см. H1 |
| Build + sync бандла | `npm run build && npm run bundle:sync` + `cmp` трёх копий | зелёный, `dist/`, `custom_components/houseplan/frontend/`, `demo/srv/assets/` побайтово идентичны |
| Docs fingerprint | `node scripts/check-docs.mjs` | зелёный (7 файлов, 10 внешних ссылок) — обязателен, дельта трогает `src/**` |
| Model invariants (синтетика) | часть `npm test` (`test/model-invariants.test.mjs`, `test/wall-segment-model.test.mjs`) | зелёный |
| Model invariants (реальная нагрузка) | `node scripts/model-invariants.mjs --config <large-house.mjs, до и после `commitWallSegmentModel`>` | зелёный до миграции («ссылки разрешимы, записи толщины находятся»), зелёный после; `--lattice`: 0.00% шума у узла |
| Smoke selection | `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | 31 прямое совпадение, 5 зарегистрированных связей, 7 слабых — вывод см. §4 |
| Golden | `npm run golden:verify` | **зелёный**, 129/129 сценариев, в т.ч. оба сценария из M1(r1) |
| Mutation gate | `node scripts/mutation-gate.mjs --check` | **зелёный**, 226/226 мутантов пойманы, включая 3 мутанта #282 (editor-commit/history-restore/optimize barrier) |
| Named/registered smokes | 31 прямых + 5 зарегистрированных = 36 запущено | **1 красный** (`smoke_open_passage.mjs`) — см. H2, остальные 35 зелёные |
| Backend pure (`tests_backend`) | не прогонялся мной — среда без `homeassistant`/`voluptuous`/`.venv-backend` (см. §6) | проверено чтением диффа `validation.py`/`wall_segment_model.py` + нового backend-теста, не исполнением |
| `pytest tests_backend` (полный, Linux CI) | не прогонялся | пре-релизный гейт, вне объёма код-ревью (§8) |
| Performance-профили (полный `performance_smoke`) | не прогонялся | пре-релизный гейт; целевой `npm run benchmark:wall-model` дельтой не задет (файл вне диффа) |
Дешёвые гейты (`tsc`, `build`, `check-docs`) зелёные. Но `npm test` сам по себе
**красный** — это не пре-релизный, а обязательный на каждом раунде гейт (§8,
§2.10), и он не проходит на предъявленном HEAD. Расширенные гейты (invariants,
smoke-select, golden, mutation-gate, целевые smoke) прогнаны, потому что дельта
меняет геометрию/ссылки на неё и барьер идентичности — именно то сочетание,
для которого §8 требует их «по необходимости». Golden и mutation-gate теперь
полностью зелёные (закрывают M1(r1) и M2(r1) — см. §5 «Закрытие раунда r1»); но
один из 36 запущенных смоков нашёл новую, не связанную с r1 регрессию (H2).
## 3. High — блокирующие находки
### H1. `npm test` красный: диагностика больше не соответствует контракту, который сама же проверяет
**Файл:** `test/open-passage-contract.test.mjs:70-74`, тест «all write/import
paths invoke the semantic passage validator».
**Воспроизведение:**
```
npm test
# ...
not ok 688 - all write/import paths invoke the semantic passage validator
Expected values to be strictly equal:
3 !== 2
location: test/open-passage-contract.test.mjs:71
```
Тест жёстко ожидает ровно 2 вхождения `validate_opening_passages(` в
`custom_components/houseplan/websocket_api.py`. Последний коммит дельты,
`3588284f` («fix: preserve passage validation during wall migration»),
осознанно добавил третий вызов — `validate_opening_passages(candidate_config,
config_data.get("config"))` в `ws_plan_optimize` **до** барьера
`commit_wall_segment_model`, рядом с уже существующим вызовом **после** барьера
(строки 1655 и 1672; третий, старый вызов на 1327 — в `ws_config_set`). Это
осмысленное и корректное по существу исправление (см. §5, closure M-out
comment #16 в issue): без него Optimize возвращал общий
`wall_model_migration_blocked` вместо конкретного
`invalid_passage_fields`-кода на конфиге с некорректным passage ещё до миграции.
Но тест, фиксирующий количество мест вызова, не обновлён вместе с фиксом — он
писался под контракт «ровно 2 вызова» и не отражает намеренно добавленный
третий.
Это прямое нарушение дисциплины: `npm test` — гейт, обязательный **в каждом
раунде** (§2.10, §8), и он сейчас красный на предъявленном коде. Хендофф-
комментарий автора к `3588284f` (issue, 22:04:14) называет только узкий прогон
14 pure-passage-кейсов и process-gate, не полный `npm test` — расхождение
между «verified» и фактическим состоянием gate.
**Доказательство:** тест уже красный на HEAD, механически (`node --test
test/open-passage-contract.test.mjs`); падение детерминировано, не флуктуация.
### H2. AC7 нарушен: opening на обычной стене исчезает со Static-карточки после миграции в v8
**Файл:** `demo/smoke_open_passage.mjs` — не тронут этой дельтой (0 строк в
`git diff --stat origin/dev...HEAD`). Корень — `src/space-render.ts:217-278`
(`renderSpaceStatic`, единственный потребитель — `space-card.ts:770`, т. е.
`houseplan-space-card`, документированная в `ARCHITECTURE.md` «Second card»);
файл не тронут диффом #282 вовсе (`git diff --stat origin/dev...HEAD --
src/space-render.ts` → пусто).
**Воспроизведение:** на `origin/dev` (`30698d6e`, отдельный git-worktree, тот
же `npm run bundle:sync`):
```
node demo/smoke_open_passage.mjs → OK (все 13 проверок true)
```
На HEAD (`3588284f`), дважды подряд, детерминированно:
```
"staticCutsAndFillsPassage": false
FAILED (1)
```
Целевая диагностика (см. приложенный debug-прогон) показывает точный механизм:
после того как `_saveOpening()` проводит объект через
`_commitPhysicalGeometry` (v7→v8 baseline-replay из `c694f6ea`), опция
`passage-main` корректно получает `host: {kind:'wall', id:'wall-…', t:0.5}` —
барьер `hostRoomOpenings` (`src/wall-segment-model.ts:530-556`) тегирует
**любой** незакреплённый за partition opening (door/window/gate/passage
одинаково, без разбора по типу) таким `host` при миграции. Но
`renderSpaceStatic` (`space-render.ts:217-222`, `resolvedHosted`) резолвит
`opening.host` только через `resolvePartitionOpeningCompat` — функцию,
понимающую исключительно `host.kind === 'partition'`. Для нашего
`host.kind === 'wall'` резолюция не находится, `resolvedHosted` пуст для этой
записи, и `resolvedRawOpenings` (строки 274-278) на строке `return resolved ?
[materializePartitionOpening(...)] : []` **молча роняет** opening из всего
дальнейшего рендера Static-карточки — не только из tunnel-заливки прохода, а
из любой отрисовки (символ, вырез в стене, hit-box).
Сам код `src/houseplan-card.ts` (полный/Plan/View card) везде корректно
различает `host?.kind === 'partition'` от прочих случаев (8295, 8325, 8480,
12766, 12804, 12958) — регрессия локализована именно в `space-render.ts`,
единственном месте, которое ADR Stage 1 должен был обновить и не обновил, хотя
сам ТЗ прямо обещает это в AC7: «v8 → legacy… projection даёт текущим full,
**static**… consumers прежние inputs» с доказательством «projection snapshots
+ **existing geometry suite**» — а именно этот existing-geometry смок
(`smoke_open_passage.mjs`, часть suite с #157) и ловит нарушение AC7.
Blast radius не ограничен синтетическим passage из смока: `hostRoomOpenings`
единообразно тегирует **все** типы openings без partition-host при миграции,
значит после Optimize/любой структурной правки, переводящей пространство в
v8, **любой** door/window/gate/passage на обычной (не-partition) стене
перестаёт отображаться на `houseplan-space-card` — символ, вырез, hit-box.
Это прямой конфликт с J1/J2 из `docs/SCOPE.md` (план должен показывать, что
происходит и где) на реальной, документированной с v1.16.0 поверхности
продукта.
**Доказательство:** тест умеет падать — зафиксирована пара «зелёный на
`origin/dev` `30698d6e` / красный на HEAD `3588284f`» на байт-идентичном файле
теста, дважды воспроизведено. Причина локализована до конкретных строк (в
отличие от недиагностированных H1/H2 из r1) через целевой debug-прогон,
приложенный к разбору находки, а не только «зелёный/красный».
**Итог по H1+H2:** оба блокируют. H1 — процессный, лечится синхронизацией
числа в тесте с фактическим количеством вызовов (тривиально, но не сделано).
H2 — продуктовый: явное нарушение собственного AC7 задачи, широкий blast
radius на документированной поверхности продукта, которую эта дельта не
касалась и не тестировала.
## 4. Medium — по существу задачи
Новых находок Medium в этом раунде нет. Все четыре Medium из r1 закрыты — см.
§5 «Закрытие раунда r1». Отдельных находок вне скоупа нет — новый issue не
заводился.
## 5. Закрытие раунда r1
| Находка r1 | Чем закрыта | Где это видно |
|---|---|---|
| **H1** — guard #278 не отклоняет правку деградированного пространства | `_commitPhysicalGeometry` (`c694f6ea`, `src/houseplan-card.ts:7296-7313`) теперь вызывает `_checkSpacePhysicalGeometry` на `liveCandidate` **до** прохождения identity-барьера (`legacySafe`), а не после — восстановлен исходный порядок guard-а #278 | `node demo/smoke_wall_union_isolation.mjs` → OK, все 11 проверок `true` (перепроверено мной на HEAD, включая три ранее красные: `degradedPhysicalEditRejected`, `rejectedEditHasNoHistoryOrWrite`, `rejectedEditHasLocalizedToast`) |
| **H2** — 8 именованных structural-smoke красные | Переработан `_commitPhysicalGeometry`: для v7-пространства identity выводится из pre-edit baseline (`baselineSource`/`historyBefore`), а не из уже отредактированной геометрии, плюс lineage hints (`fixedTopologyWallLineageHints`) для сохранения ID при чистом split | Все 8 смоков (`smoke_resize_pointer_real_plan`, `smoke_wall_chain_thickness`, `smoke_unified_wall_tool`, `smoke_wall_face_overlap`, `smoke_edit_walk`, `smoke_editor_tabs`, `smoke_plan_snap_overlay`, `smoke_partition_openings`) перепрогнаны мной на HEAD — все зелёные |
| **M1** — golden baseline не обновлён под AC15-строку | Baseline не потребовался: фикстура `optimize-orphan-references-*` (`demo/golden/matrix.mjs:695-720`) уже v8/canonical, `Optimize` на ней не мигрирует ни одного сегмента, `r.wallSegmentsMigrated` = 0 → строка `gs.wall_segments_migrated` не рендерится, визуального различия нет | `npm run golden:verify` → 129/129 passed, включая `optimize-orphan-references-dark-en`/`-light-ru`; `demo/golden/baselines/**` не тронут диффом (подтверждено `git log` — пусто), что согласуется, а не противоречит |
| **M2** — AST/choke-point guard уже, чем заявлено в AC5/`TESTING.md` | Добавлены два новых именованных мутанта: `wall-identity-history-restore-barrier-bypassed` (Undo/Redo restore) и `wall-identity-optimize-barrier-bypassed` (`plan-optimizer.ts`), плюс переименован исходный в `wall-identity-editor-commit-barrier-bypassed` — все три известных writer-пути теперь поимённо покрыты | `node scripts/mutation-gate.mjs --check` → 226/226 `ok`, включая все три `wall-identity-*` мутанта (перепрогнано мной) |
| **M3** — `wall_model_client_outdated` не срабатывает на реалистичном echo-сценарии | `validate_wall_model_transition` (`custom_components/houseplan/validation.py:185-217`) получил новую ветку `old_model >= 8 and new_model >= 8 and geometry_changed`, сверяющую `_wall_catalog_projection` (только `wall_segments`) — расхождение поднимает `WallModelClientOutdatedError` с текстом «unchanged wall catalogue» | Новый тест `tests_backend/test_wall_segment_model.py::test_stale_client_echoing_v8_catalog_gets_the_named_error` — прочитан, логика соответствует описанному сценарию; **не исполнен** (см. §6, нет `homeassistant`/`voluptuous` в этой среде) — проверено чтением, не исполнением |
| **M4** — регрессионный тест #248 ослаблен без обновления фикстуры | `test/fixtures/optimize-storage-roundtrip.json` обновлена под полный v8-выход, `test/plan-optimizer.test.mjs:366` вернул строгий `assert.deepEqual(first.config, storageRoundtripFixture.expected.config)` вместо частичного сравнения только `poly` | Тест прогнан мной в составе `npm test` — проходит (часть 1329 passed); дифф `test/plan-optimizer.test.mjs` (`9fc213c6..HEAD`) показывает возврат полного `deepEqual` |
Все шесть находок r1 (H1, H2, M1-M4) закрыты предметно, не декларативно. Ни
одна из них не осталась открытой или не переоткрылась при моей перепроверке.
## 6. Унаследовано из r1
Из полного 62-файлового диффа r1 (`git diff origin/dev...5adfdb5d` — уже
недоступен напрямую как SHA, но содержательно эквивалентен диапазону, который
описывает `docs/reviews/CODE-REVIEW-282-r1.md`), эта дельта не касается и
поэтому **не переразобрано**, а принято на основании выводов r1 (документ
`CODE-REVIEW-282-r1.md`, SHA на котором получен вывод — `5adfdb5d`/содержательно
`466a6765` после ребейза):
- **Migration determinism/idempotence** (`deterministicWallSegmentId`,
суффиксы при коллизии) — r1 §5, не задето дельтой (`test/wall-segment-model.test.mjs`
дельта затрагивает только новые тест-кейсы, не эту логику).
- **Backend-зеркало валидации** — unique-id, owner-count, dangling refs,
edge-geometry parity кроме нового `_wall_catalog_projection`-блока (см. §5
M3) — r1 §5, файл `wall_segment_model.py` в дельте затронут (+48/-х строк),
но это тот же lineage-код, что и H1/H2-фикс на фронтенде; backend-инварианты
(owner-count/dangling refs) не изменены текстуально в дельте.
- **Import/export remap** (`import_export.py`, `secrets.token_hex(4)`) — не в
дельте (`git diff --stat 9fc213c6..HEAD` не содержит `import_export.py`),
наследуется из r1 §5 без переразбора.
- **i18n-ключи** (`toast.wall_model_migration_blocked`,
`toast.wall_model_client_outdated`, `gs.wall_segments_migrated`,
`wall_model.reason.*`) — не в дельте, наследуется из r1 §5.
- **Документация** (`WALL-THICKNESS.md`, `CANVAS.md`, `USER-GUIDE.{ru,}.md` —
кроме `TESTING.md`, который в дельте, см. §5 M2) — наследуется из r1 §5.
- **`config-field-registry.mjs`** — не в дельте, наследуется.
- **Трейлеры и changelog структуры коммитов r1** — наследуется; трейлеры
каждого нового коммита дельты проверены заново мной отдельно (см. §7).
Обоснование, почему это не требует переразбора несмотря на общий полный охват
раунда (§1): дельта физически не касается перечисленных файлов
(`git diff --stat 9fc213c6..HEAD`, приведён в §1), а находки этого раунда (H1,
H2) обнаружены и локализованы именно в затронутых дельтой файлах
(`test/open-passage-contract.test.mjs` тестирует код `websocket_api.py`,
который в дельте; `space-render.ts` хоть и не в дельте, но находка H2 —
следствие данных, которые генерирует именно delta-код `wall-segment-model.ts`,
т.е. вызвана дельтой, а не унаследованным кодом).
## 7. Что проверено и корректно
- **Трейлеры** — все 6 коммитов дельты (`c694f6ea`, `41d7a63f`, `c24792b6`,
`35fce64a`, `38b03549`, `3588284f`) несут `Issue: #282`; четыре
`User-Visible: yes` (`c694f6ea`, `35fce64a`, `38b03549`, `3588284f`) —
каждый правит оба changelog в том же коммите (проверено `git show --stat`
на каждом).
- **Backend-барьер Optimize** (`35fce64a`) — `ws_plan_optimize` теперь ведёт
тот же v7→v8-барьер, что и frontend `_commitPhysicalGeometry`
(`commit_wall_segment_model` + `too_large`-проверка после миграции +
`WallSegmentMigrationError` в списке перехватываемых) — прочитано полностью,
логика симметрична фронтенду.
- **Guard-переупорядочивание #278** (H1(r1)) — `legacySafe`-проверка на
`liveCandidate` до барьера, restore на отказ — прочитано и подтверждено
прогоном (см. §5).
- **Lineage hints** (`fixedTopologyWallLineageHints`,
`adoptWallSegmentModelCandidateInPlace`) — прочитаны; логика различения
`preferredIds`/`positionalIds` через `collinearOverlap` (защита от
«застарелого ординала» при сплите) выглядит осмысленной и подтверждена всем
доступным smoke/mutation набором.
- **Off-grid guard с authored path** (`38b03549`) — `wallModelOffGridValueCount`
теперь учитывает `_path` (активная рисуемая цепочка) как базовую линию
роста, не как новое отклонение — юнит-тест на это добавлен и проходит.
- **Три мутанта #282** (editor-commit/history-restore/optimize barrier) —
реально пойманы (§2, §5 M2).
- **`npm run invariants`-эквивалент на реальной нагрузке** — прогнан на
`demo/fixtures/large-house.mjs` (3 пространства, 1788 записей по прошлым
измерениям Stage 0) до и после `commitWallSegmentModel`: ссылки разрешимы,
записи толщины находятся, 0% шума у узла.
- **35 из 36 запущенных смоков** зелёные (см. §2, §4).
## 8. Чего не проверял и почему
- **`pytest tests_backend`, даже pure-subset** — среда исполнения не содержит
`homeassistant`, а `custom_components/houseplan/__init__.py` безусловно
импортирует `homeassistant.components.frontend`, поэтому даже
не-`test_ha_*.py` модули (`test_validation.py`, `test_wall_segment_model.py`)
не собираются без полного HA-пакета — это не селективный пропуск
`conftest.py`, а полный отказ коллекции. `.venv-backend` в этой среде не
существует (в отличие от cloud-agent окружения, описанного в `AGENTS.md`).
M3 (§5) и общая корректность `validation.py`/`wall_segment_model.py`-диффа
проверены **чтением**, не исполнением — честно назвать это ограничением, не
«verified».
- **Полный `performance_smoke`** — пре-релизный гейт (§8); целевой
`benchmark:wall-model` дельтой не затронут (файл вне диффа), поэтому не
перепрогонялся отдельно в этом раунде — цифры из r1/хендоффа автора (167-284
мс на p95 при бюджете 500 мс) остаются в силе, так как измеряемый код не в
дельте.
- **7 «слабая связь» смоков** (`smoke_align_guides`, `smoke_card_tool_conflict`,
`smoke_editor_gestures`, `smoke_esc_dialogs`, `smoke_island_rooms`,
`smoke_optional_space_model`, `smoke_pan_any_zoom` — связь только по
`_path`) — не прогонялись: методология идентична r1 (§6 её документа) — уже
найденных блокирующих находок достаточно для возврата, и один из 36 более
сильно связанных смоков (`open_passage`) уже красный. Автору стоит
прогнать все 7 вместе с полным набором перед следующей сдачей.
- **Глубокий line-by-line аудит `buildAtoms`/lineage-логики
(`src/wall-segment-model.ts`, весь диапазон 218 изменённых строк)** — не
выполнен formal-proof, только чтение ключевых веток плюс полное
black-box-покрытие (36 смоков, 226 мутантов, 129 golden, полный `npm test`
кроме H1). Смок `smoke_open_passage.mjs` — прямое доказательство, что
black-box-покрытие не исчерпывающее; глубже эту логику не разбирал, так как
найденных H1/H2 достаточно, чтобы вернуть задачу автору (тот же принцип, что
в r1 §3 «проверять глубже уже не требуется по методологии»).
## 9. Резюме
Красный вердикт, второй подряд, но по существу другим находкам, чем r1: все
шесть находок r1 (2 High + 4 Medium) закрыты предметно и перепроверены
исполнением (кроме M3 — чтением, backend-харнесс недоступен). Однако именно
дельта, закрывшая r1, внесла две новые блокирующие находки:
- **H1** — `npm test` красный прямо сейчас: тест на количество вызовов
`validate_opening_passages(` не обновлён под намеренно добавленный третий
вызов из последнего коммита дельты. Механически тривиально, но нарушает
обязательный на каждом раунде гейт.
- **H2** — нарушение собственного AC7 задачи: `houseplan-space-card` теряет
**любой** wall-hosted opening (не только passage) после миграции
пространства в v8, потому что `src/space-render.ts` — единственный
нетронутый диффом #282 потребитель `opening.host` — не различает
`host.kind === 'wall'` от `host.kind === 'partition'` и молча роняет
нерезолвленные openings из рендера. Реальный blast radius на
документированной поверхности продукта, конфликтующий с J1/J2
`docs/SCOPE.md`.
Задача возвращается в «В разработке». Оба фикса локальны и не должны требовать
повторного полного разбора всей подсистемы в r3, если дельта останется
локальной к `websocket_api.py`-тесту и `space-render.ts`; но `space-render.ts`
до сих пор не в диффе #282 вовсе — стоит проверить на этом же файле и другие
пути, потребляющие `opening.host` (hidden-isometric, если он существует
отдельно), а не только `houseplan-space-card`.