docs: review document for #329

Issue: #329
User-Visible: no
This commit is contained in:
claude[bot]
2026-08-27 21:01:48 +00:00
parent 7017896eb2
commit e1df015d93
+337
View File
@@ -0,0 +1,337 @@
# CODE-REVIEW-329-r1
Issue: #329 · этап: code (PROCESS.md §2.7) · заход r1 · блокирующих циклов 0/4 (до этого вердикта)
## Скоуп
Диапазон: `origin/dev...HEAD` на ветке `issue/329-junction-limits`, HEAD =
`7017896e` (докоммит после ребейза на `dev`, который вобрал #264 —
извлечённый `ResizeController`). Ветка чисто ребейзится на `origin/dev`
(`git merge-base origin/dev HEAD` == tip `origin/dev`), конфликта нет — два
предыдущих запуска конвейера, отказавшихся ревьюить из-за конфликта, здесь не
актуальны, ветка перебазирована и запушена автором (комментарий
2026-08-27T20:00:24Z).
Задача обычного трека (не `small`): ТЗ — `docs/specs/329-junction-limits.md`,
ревизия 6, спек-ревью прошло r1→r3 (зелёный на r3), затем автор внёс правки
владельца §4/П3 напрямую (ревизии 4–6) без дополнительного раунда
спек-ревью — это первый code-review заход для итогового кода.
43 изменённых файла, из них продуктовый код: `src/junction-limits.ts` (новый),
`src/houseplan-card.ts`, `src/wall-thickness.ts`, `src/resize-controller.ts`,
`src/i18n/{en,ru}.json`, `custom_components/houseplan/junction_limits.py`
(новый), `custom_components/houseplan/websocket_api.py`. Остальное — тесты,
golden, документация, сгенерированный бандл.
## Как проверялось
Дешёвые гейты (обязательные всегда):
- `npx tsc --noEmit` — чисто.
- `npm test` — 1388/1389 pass, 1 skip (питоновский parity-тест живёт в
`tests_backend`, не в node-раннере). Ни одного red.
- `npm run build` — собрался; `dist/houseplan-card.js` побайтово совпадает с
`custom_components/houseplan/frontend/houseplan-card.js` (сверено `diff -q`
после `npm run bundle:sync`).
- `node scripts/check-docs.mjs` — «Documentation checks passed (7 files, 10
external links)», отпечаток скриншотов свежий (последний коммит ветки как
раз чинит это после ребейза).
Геометрия/инварианты (диф трогает рёбра стен, `wall_segments`, толщину,
`layout`):
- `npm run invariants -- --config test/fixtures/329-sharp-apex.json` —
«Инварианты выполнены: ссылки разрешимы, записи толщины находятся» (ключ
записи толщины и ключ решёточного ребра совпадают на самой фикстуре issue).
- `npm run golden:verify` — весь текущий набор (полная матрица, не только
новая сцена) прошёл, включая новую `sharp-apex-legacy-dark` (AC6) и уже
существующую `junction-owner-repro-dark`; ни одна старая сцена не
изменилась — риск §10 закрыт исполнением, а не заявлением.
Бэкенд (диф трогает `custom_components/houseplan/*.py`):
- `python -m pytest tests_backend/test_junction_limits.py -q` — 7 passed,
включая `test_parity_with_the_frontend_checks` (реально прогнал TS-функции
через `node --eval` и сравнил с питоновским вердиктом — не skip, `test-build/`
был собран `npm run invariants` до этого).
- Полный `python -m pytest tests_backend -q` в этой среде не поднимается:
`homeassistant` не установлен (это облачная CI-среда без
`.venv-backend`, установить полный HA недостаточно оправдано ради одного
ревью) — падает на импорте в трёх модулях, не в затронутых #329 файлах.
Ограничение окружения, не решение сузить проверку: описано в AGENTS.md
(«Локально только pytest без HA — silently skips `test_ha_*.py`»). Здесь
импорт вообще падает раньше skip-логики `conftest.py`, поэтому я запустил
ИМЕННО целевой файл отдельно (см. выше) вместо того, чтобы полагаться на
«зелёный», который ничего не доказывает.
- Дополнительно: написал и выполнил собственный python-репро (см. находку
H1) — прямой вызов `validate_junction_limits` на сконструированных
fixtures, не полагаясь на чтение кода.
Мутанты (диф добавляет/меняет 4 мутанта в `scripts/mutation-gate.mjs`) —
прогнаны точечно, не весь гейт (несоразмерно объёму ревью):
- `junction-limit-angle-not-enforced` — поймано 1/1.
- `junction-limit-write-gate-removed` — поймано 1/1.
- `degenerate-apex-bevelled-again` — поймано 1/1.
- `resize-preview-reject-silent` (изменённый якорь) — поймано 1/1.
Браузерные смоки — выбраны через `node scripts/smoke-select.mjs --base
origin/dev --head HEAD` (43 символа на изменённых строках `src/**` в 4
файлах, порог широкого символа 38 смоков — под порог не попал ни один,
инструмент не потребовал расширять выборку до всей матрицы 193). Прогнаны:
оба смока, названных в AC/изменённых диффом (`smoke_junction_limits.mjs` —
AC1/AC3/AC5a/AC7a/AC7b/§3, `smoke_island_rooms.mjs` — П3 на комнатах-островах),
плюс все 13 «прямых совпадений» и 1 «зарегистрированная связь»
(`smoke_real_plan_masonry` — инструмент сам напомнил, что на реальном плане
кладка рвётся там, где синтетика цела, и восемь прошлых задач по стыкам уже
входили в бету с необнаруженным разрывом; учитывая, что диф правит
`insetContour`/`outsetContour`, счёл прогон обязательным, а не факультативным).
Итог — 15/15 OK, ни один не покраснел.
## Находки
### H1 (High, в скоупе). Бэкенд отказывает в легитимной правке легаси-плана — прямое нарушение §3
`custom_components/houseplan/junction_limits.py::validate_junction_limits`
сравнивает «before» и «after» по СЫРЫМ документам (`msg["config"]` и
`data.get("config")`), не пропуская ни один из них через
`commit_wall_segment_model`. Фронтенд ровно эту ошибку уже нашёл и
исправил в коммите `4758767e` («junction limits judge both sides after the
same migration») — коммит-сообщение прямым текстом объясняет механику:
«The baseline for inheritance was the raw previous document, which for a
legacy space carries no wall catalogue at all — so every inherited short
segment of a real plan looked new». На бэкенде аналогичная миграция
(`commit_wall_segment_model`) вызывается только в `import_export.py` и в
`ws_plan_optimize` (и то — условно, при `submitted_model < WALL_SEGMENT_MODEL_VERSION`),
но НЕ в `ws_config_set`, где живёт вызов `validate_junction_limits`
(websocket_api.py:1334).
Следствие: хранимый (`previous`) документ пространства, которое ещё ни разу
не проходило структурную запись после введения каталога `wall_segments`
(`model_version < 8`), не содержит `wall_segments` вовсе —
`limit_segments()` читает только `wall_segments`/`partitions`/`room_drafts`,
поэтому `space_violations(old_space)` возвращает `[]` для такого
пространства НЕЗАВИСИМО от реальной геометрии. Кандидат же (`msg["config"]`)
современный клиент уже мигрировал на клиенте до отправки — там
`wall_segments` есть. Любое унаследованное нарушение (в т.ч. безобидное,
годами живущее в реальном плане пользователя — ровно повод, по которому
заведён #329) на первой же структурной правке после обновления карточки
читается как «новое» и запись ЦЕЛИКОМ отклоняется — даже если правка не
касалась нарушающего узла. Это прямое нарушение §3 ТЗ («миграция...
не блокируются никогда», «правка, не трогающая нарушающий элемент,
проходит как обычно») и AC8/AC9 по факту (тест паритета в
`test_junction_limits.py` не ловит это, потому что в его фикстурах ОБЕ
стороны уже содержат `wall_segments` — сценарий «previous без каталога»
не фигурирует нигде в тестах).
**Воспроизведено исполнением** (не только чтением кода) — прямой вызов
модуля:
```python
previous = {"spaces": [{
"id": "s", "cell_cm": 5.0, "model_version": 0, "rooms": [],
"walls": [ # легаси-хранение, БЕЗ ключа "wall_segments"
{"key": "w0", "a": [0.0, 0.0], "b": [...], "cm": 15},
{"key": "w1", "a": [0.0, 0.0], "b": [...], "cm": 15}, # угол 9°
],
}]}
candidate = {"spaces": [{
"id": "s", "cell_cm": 5.0, "model_version": 9,
"rooms": [{"id": "r1", "name": "Renamed"}], # НЕСВЯЗАННАЯ правка
"wall_segments": [ # тот же угол 9°, клиент уже мигрировал
{"id": "w0", ...}, {"id": "w1", ...},
],
}]}
jl.validate_junction_limits(candidate, previous)
# → JunctionLimitError: junction_limit_angle; actual=9; limit=15
```
Результат: `REFUSED: junction_limit_angle` — легитимная переименование
комнаты на давно существующем плане с историческим острым углом отклоняется
целиком. Реальный масштаб поражения велик: сама мотивация #329 — план
РЕАЛЬНОГО пользователя с давним острым углом; после этой задачи такой
пользователь не сможет сохранить вообще никакую правку своего плана, пока
не прогонит Optimize (о котором он может не знать) — притом что §3 прямо
обещает обратное, и фронтенд этот же случай уже умеет обрабатывать
корректно (проверено смоком `§3: унаследованное нарушение не блокирует
несвязанную запись` внутри `smoke_junction_limits.mjs`, который проходит
именно потому, что бьёт только по фронтенд-пути).
Чинится по образцу уже принятого решения: перед сравнением пропустить
`old_space` (и, для симметрии, `space`) через `commit_wall_segment_model`
— тем же приёмом, каким `_junctionLimitsIntroduced` в `houseplan-card.ts`
уже чинит идентичный дефект на фронтенде. `docs/CONFIG-COMPATIBILITY.md`
(этот же дифф) уже ОБЕЩАЕТ это поведение текстом «both have gone through
the same commitWallSegmentModel migration» — то есть документация описывает
контракт, которого бэкенд не реализует.
### M1 (Medium, в скоупе). Мёртвый код в `wall-thickness.ts`, чьё удаление коммит уже задекларировал
Коммит `002795f7` («fix: no jags on the edges of a degenerate apex»)
убрал оба места, где вызывались `degenerateApexCaps()`/`clipPolygonOutsideCap()`,
и сообщение коммита прямо утверждает: «The clip helper and its cap plumbing
are gone». На деле в текущем HEAD остаются:
- `apexCaps?: number[][][]` — поле интерфейса `MultiWallRoomRing`
(`src/wall-thickness.ts:2815`), никогда не устанавливается и не читается;
- `export function clipPolygonOutsideCap(...)` (строка 3637, ~30 строк);
- `export function degenerateApexCaps(...)` (строка 3696, ~60 строк).
Ни один из них не вызывается из продуктового кода (`grep` по `src/` даёт
только сами определения) и не покрыт тестами (`grep` по `test/` — ноль
совпадений). Реальный переход на «острие вместо фаски» сделан иначе —
через `isDegenerateApexCorner` прямо в `insetContour`/`outsetContour` — эти
функции остались как нетронутый, непроверяемый и вводящий в заблуждение
довесок: следующий читающий код решит, что это часть работающего конвейера
клиппинга (интерфейс даже комментирует поле как «#329 §4: clip regions...»),
хотя оно мертво с коммита, заявившего обратное.
### M2 (Medium, в скоупе). Дублированный раздел в `docs/USER-GUIDE.ru.md`
Раздел «### Ограничения стыков стен» вставлен ДВАЖДЫ подряд, слово в слово
(строки 420–443 и 445–468 идентичны по всему тексту, включая таблицу и оба
абзаца после неё). Английская версия (`docs/USER-GUIDE.md`, «### Wall
junction limits») дублирования не имеет — это чисто копипаст-дефект русской
версии, попавший в тот же коммит, что и остальная документация задачи
(`User-Visible: yes` требует правки обоих changelog и, по духу того же
правила, аккуратной документации на обоих языках).
### M3 (Medium, в скоупе). USER-GUIDE описывает канал Resize неполно — читатель не узнает про тост
И `docs/USER-GUIDE.md`, и `docs/USER-GUIDE.ru.md` в новом разделе пишут:
«рисование и «Толщина» показывают тост с названием правила, а Resize
останавливает стену в последней разрешённой позиции» (RU, строки 441–443/
466–468; аналогично EN). Формулировка явно противопоставляет тост (для
рисования/Толщины) и «просто останавливается» (для Resize) — то есть
читатель разумно поймёт, что при Resize тоста нет.
По факту `src/houseplan-card.ts` (строки ~9181–9188) показывает тост и для
Resize: `resize.limit_stopped` + `_junctionLimitLabel(...)`, что подтверждено
исполнением — смок `smoke_junction_limits.mjs` проверяет
`resizeRefusalNamesRule` (тост содержит «5») и `resizeRefusalOnce === 1`, и
оба зелёные. Это ровно тот класс дефекта, который проверяет
`test/single-source-numbers.test.mjs` для чисел, но здесь смысловая
сторона: сам факт наличия канала обратной связи задокументирован неверно.
AC7a (ревизия 6 спека) корректно описывает тост-канал — расхождение
только в изданном для пользователя `USER-GUIDE`, который вправляли отдельно
и не сверили с финальным текстом AC7a.
Смежное наблюдение (не отдельная находка, отметка для автора): §2 самого
ТЗ (нормативный текст) тоже не обновлён вслед за AC7a ревизии 6 и всё ещё
говорит «тост не используется» для Resize — но ТЗ на этапе code-review не
правится, отмечаю для полноты картины, поскольку именно расхождение §2 с
AC уже один раз обсуждалось на спек-ревью (r2-M1) и, похоже, вернулось в
другом месте документа.
### M4 (Medium, в скоупе). AC10 (постусловие Optimize) не доказан ни тестом, ни явным разбором
AC10: «Optimize на легаси-фикстуре не создаёт новых нарушений
(AC-постусловие §3)». В диффе нет ни юнита, ни смока, который прогоняет
`optimizePlans`/`ws_plan_optimize` на фикстуре с существующим нарушением и
сравнивает набор нарушений до/после. `grep` по «Optimize»/«optimize» во
всех тестовых файлах, тронутых этой задачей, ничего не находит.
Это не тривиально верно «по построению»: `optimizePlans` вызывает
`alignAllToGrid` и `repairNearAxisRoomWalls`, которые двигают узлы к
решётке и выпрямляют почти-осевые стены — оба потенциально смещают
геометрию на доли сантиметра. Ни один из порогов П1–П5 не имеет запаса
шире шага решётки в общем случае (например, узел ровно на 5.0 см от
чужой стены после сдвига на привязку может уйти ниже порога). Формально
проверить это чтением я не могу (нужно было бы доказать, что снап
монотонно не уменьшает ни один из пяти инвариантов, что для П1/П4 не
очевидно), поэтому по правилу «либо тест, либо разбор с явной пометкой»
AC10 сейчас не доказан никак.
## Что проверено и корректно
- П1–П5 (`src/junction-limits.ts`) — юнит-покрытие точное, включая границы
(14°/15°/16°, 6/7 стен, 19/20/25 см, 4/5 см, пустой/непустой просвет),
T-стык как легальная инцидентность, компенсацию перепада толщин (АС3b) —
числа владельца (30/20 см → 5 см доборный атом) воспроизведены и на
фронте, и на бэке идентично.
- §4 (честная вершина легаси) — `isDegenerateApexCorner`,
`insetContour`/`outsetContour` дают ровно одну точку в вершине плана и
снаружи, и внутри; внешнее кольцо тела — треугольник без ступенек и
микровершин; golden-сцена `sharp-apex-legacy-dark` подтверждает это на
пиксельном уровне, вся остальная golden-матрица не сдвинулась.
- Наследование нарушений считается ПО ПРАВИЛУ (`increasedViolations`), а не
по subject id — комментарий и тест прямо объясняют, почему id нестабилен
через структурную запись (переатомизация); фронтенд-часть барьера
(`_junctionLimitsIntroduced`) верно мигрирует ОБЕ стороны через
`commitWallSegmentModel` перед сравнением — это именно то, чего не хватает
на бэкенде (H1).
- AC7a (Resize) — проверено настоящим pointer-жестом (не вызовом
внутренних методов): сетка 2 см, две комнаты в 10 см, тяга 6 см
останавливается на 6 см (не доезжает до нарушающих 4 см), ровно один тост
с «5». `_rszLimitViolation` сбрасывается в начале КАЖДОЙ проекции —
проверил код: без этого сброса тост от предыдущего шага мог бы объяснить
отказ другого рода; тест на это (`resize-preview-reject-silent` мутант)
ловит немоту, но не проверяет свежесть причины — сам код читаем и
корректен (обнуление на первой строке `_rszProjectPreview`).
- AC7b («Толщина») — значение не применяется, тост с названием правила,
план байт-неизменен (смок).
- AC1/AC3/AC5a — смоком, с проверкой, что тост называет конкретное число
(не просто безмолвный отказ).
- AC2/AC4 — юнитами на обеих сторонах (фронт+бэк), включая T-стык.
- AC6 — см. выше, golden + юниты на контурах и кольце.
- AC8 (миграция/импорт/restore не блокируются) — проверено ЧТЕНИЕМ, не
исполнением: `validate_junction_limits` вызывается только из
`ws_config_set`; ни `ws_import_apply`, ни `ws_files_migrate` его не
вызывают. На фронте `_junctionLimitsIntroduced` вызывается только из
`_commitPhysicalGeometry` и `_rszProjectPreview` — оба это операции
редактирования, не импорта/миграции.
- AC9 (паритет) — исполнено: `test_parity_with_the_frontend_checks`
реально прогоняет TS через `node --eval` и сравнивает с питоновским
результатом на 8 фикстурах, 7/7 pass.
- Backend exception plumbing в `ws_plan_optimize`: `JunctionLimitError`
добавлен в `except`-кортеж, хотя `validate_junction_limits` там не
вызывается (сознательно, по §5 — Optimize не должен упираться в новые
правила) — код от этого не ломается, исключение просто никогда не
возбуждается в этой ветке; не поднимаю до отдельной находки, так как
безвредно и не вводит в заблуждение так же сильно, как M1 (нет
комментария, утверждающего обратное).
- AC5b — формула П5 (`checkRoomClearance`) шейп-агностична (это чистая
формула площади по шнурку), поэтому квадратные фикстуры теста
математически эквивалентны треугольнику из спека; но буквально
«юнит с этими числами» (равносторонний 60/33 см, inradius 17.3/34.6 см)
отсутствует — фиксирую как Low, не поднимаю до Medium: код, который
доказывал бы обратное, был бы тем же самым.
- Трейлеры: `Issue: #329` на каждом коммите; `User-Visible: yes` только там,
где меняется видимое поведение (214355f4), и в том же коммите правки
обоих changelog — проверено `git show --stat`.
- Одно число — один источник: `_junctionLimitLabel` — единственное место,
формирующее текст отказа что для тоста рисования/Толщины, что для
Resize; `actual`/`limit` берутся из одного и того же объекта
`JunctionLimitViolation`, не пересчитываются отдельно для тоста и для
ручки — проверено чтением, не вижу второго источника чисел.
## Чего не проверял и почему
- Полный `python -m pytest tests_backend -q` (HA-harness) — недоступен в
этой среде (`homeassistant` не установлен, нет `.venv-backend`); прогнан
только целевой файл задачи (полностью, 7/7) плюс попытка полного набора
зафиксирована как падение импорта, не как «зелёный, но неполный».
- Полная браузерная матрица (193 смока) — не прогонял, диф не потребовал
этого по `smoke-select` (43 символа, порог 38 не превышен); прогнал все
прямые совпадения, зарегистрированную связь и оба AC-смока (15 итого).
- Полный `scripts/mutation-gate.mjs` (весь набор мутантов) — несоразмерно
ревью; прогнал точечно только 4 мутанта, которые диф добавил/изменил.
- `performance_smoke` — не прогонял: в AC не назван, диф не трогает горячий
путь рендера (П1–П4 — O(узлы+сегменты) на запись, П5 — на commit, не на
каждый кадр курсора, это подтверждается чтением: вызовы
`_junctionLimitViolations`/`_junctionLimitsIntroduced` находятся в
`_commitPhysicalGeometry` и в `_rszProjectPreview`, оба — не per-frame
рендер-путь).
- Ручное тестирование в браузере — не проводил (этап code-review, ручного
тестирования в цикле нет по регламенту; браузерные смоки — единственная
замена).
- AC5b буквально «с этими числами» — не поднимаю до отдельного прогона,
формула шейп-агностична (см. выше), различие признано как Low и не
требует отдельного гейта.
## Итог
Один High (H1) — реальный, воспроизведённый исполнением дефект, прямо
нарушающий §3 ТЗ и обесценивающий центральное обещание задачи («легаси не
блокируется») для бэкенд-пути записи. Плюс четыре Medium в скоупе (M1–M4),
все чинятся в этой же задаче без отдельного issue (владелец, #202).