docs: review document for #330

Issue: #330
User-Visible: no

docs/reviews/CODE-REVIEW-330-r2.md

@@ -0,0 +1,257 @@
+# CODE-REVIEW-330-r2
+
+Issue: #330 — «Ограничения стыков (#329) блокируют event loop HA и дёргают ресайз на больших планах»
+Этап: code (PROCESS.md §2.7)
+Заход: r2 · блокирующих циклов израсходовано 1 из 4 (до этого раунда)
+Ветка: `issue/330-junction-limits-performance`, HEAD ревью — `85636a65`
+
+## 0. Почему разбор полный, а не по дельте
+
+Предыдущий вердикт (code-review r1, 2026-08-28T00:00:48Z) не назвал SHA, на
+котором он получен — это само по себе процессное упущение (см. ниже). Автор
+в ответном комментарии называет два SHA до финального ребейза: `a7500973`
+(HEAD на момент публикации r1) и `7513f93d` (докс-фикс, слитый до публикации
+вердикта). Оба объекта в текущем дереве отсутствуют:
+
+```
+$ git cat-file -t a7500973   → fatal: Not a valid object name
+$ git cat-file -t 7513f93d   → fatal: Not a valid object name
+```
+
+Ветка была перебазирована на ушедший вперёд `dev` через мердж #62 («ребейз
+над мерджем i18n-registry», коммит `85636a65`) уже ПОСЛЕ публикации r1. Это
+ровно исключение §7.2 из брифинга: «ребейз на ушедший вперёд dev — после
+ребейза это другой код». Пословный `git diff <r1 SHA>..HEAD` невозможен и не
+имел бы смысла — базовые деревья разошлись. Разбор ниже полный, по
+`git diff origin/dev...HEAD`, с явным закрытием находок r1 отдельным
+разделом.
+
+**Процессное наблюдение (не находка в код, не считается в M/H):** ни в
+вердикте r1, ни в вердикте предыдущего code-review-раунда SHA не был назван
+текстом. Формат §7.2 этого не требует явно, но по факту это стоило времени
+на этот раунд — рекомендую в следующих раундах явно указывать SHA в теле
+вердикта, а не только в документе.
+
+## 1. Скоуп
+
+Диапазон `origin/dev...HEAD`, 31 файл, +1731/-258:
+
+- `custom_components/houseplan/junction_limits.py` — линейный byNode-индекс
+  П3, bucket-решётка П4, `_migrated_spaces` без ремиграции v9 (§4.6),
+  `space_violation_counts`/`baseline_counts` (§4.2).
+- `custom_components/houseplan/websocket_api.py` — цепочка валидаторов
+  `ws_config_set`/`ws_plan_optimize` в `async_add_executor_job` (§4.1),
+  запись/чтение `rt.junction_baseline` по rev (§4.2).
+- `custom_components/houseplan/store.py` — новое поле рантайма
+  `junction_baseline`.
+- `src/junction-limits.ts` — те же три среза на фронте (byNode-индекс,
+  bucket-решётка, зеркальная логика).
+- `src/houseplan-card.ts` — `_junctionLimitViolations` принимает
+  `sharedGeometry` (§4.7, общий проход топологии/кладки на все комнаты),
+  `_junctionLimitsIntroduced` — кэш baseline по (identity, `_cfgEpoch`)
+  (§4.4) и путь «v9 как есть» (§4.6 на фронте).
+- `demo/benchmark_junction_limits.mjs`, `package.json`,
+  `.github/workflows/validate.yml` — новый перф-контракт §5/AC7, вшитый в
+  job `performance_smoke` (не только в еженедельный `mutation-gate.yml`).
+- `demo/smoke_junction_limits.mjs` — счётчик вызовов `_junctionLimitViolations`
+  за один жест ресайза (заявленная поведенческая часть AC4).
+- `scripts/mutation-gate.mjs` — три новых мутанта (П3, П4, «протухший
+  baseline-кэш»).
+- `test/junction-limits.test.mjs`, `tests_backend/test_junction_limits.py`,
+  `tests_backend/test_ha_websocket.py` — юниты эквивалентности и AC1.
+- `docs/CHANGELOG.md`/`.ru.md` — одна строка, User-Visible: yes, в том же
+  коммите (`c90f5bf0`), что и код.
+- Скриншоты/`screenshots.json` — механический пересчёт отпечатка
+  (src/** изменился), содержимое интерфейса не менялось.
+
+Вердикты П1–П5 не меняются нигде в дифф — заявление автора подтверждено
+чтением: единственная содержательная логика (`_length`, `_distance_to_segment`,
+условия нарушений, epsilons) не тронута; менялись только структуры данных
+вокруг неё (индексация, кэш, порядок обхода).
+
+## 2. Как проверялось
+
+Все команды выполнены на этом SHA в этой рабочей копии; результат — вывод
+ниже, не «verified» без числа.
+
+| Гейт | Команда | Результат |
+|---|---|---|
+| Типы | `npx tsc --noEmit` | чисто, без вывода |
+| Юниты фронта | `npm test` | **1419 passed**, 1 skipped, 0 failed |
+| Юниты бэкенда (чистый python, без HA) | `python3 -m pytest tests_backend/test_junction_limits.py -q` | **13 passed** |
+| Юниты бэкенда (HA-харнесс) | `python3 -m pytest tests_backend -q` (pytest-homeassistant-custom-component установлен в этой сессии) | **423 passed, 1 skipped, 1 error** — ошибка теardown (`_run_safe_shutdown_loop` thread-leak assert) воспроизводится и на НЕ тронутом этой веткой тесте (`test_issue_244_...`) при отдельном прогоне того же файла — окружение, не диффа. Тест **AC1** (`test_330_config_set_validators_run_in_the_executor`) прошёл исполнением: `1 passed`. |
+| Перф-контракт §5/AC7 | `npm run benchmark:junction-limits` | `pass: true`; tsFullCandidateMs 150.7/400 (2.65×), pyWarm 67.1/300, pyCold 2463/5000 |
+| Сборка + идентичность бандла | `npm run build` + `git status --short` (чисто) + `diff dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js` | идентичны, `git status` чист после ребилда |
+| Докс-гейт | `node scripts/check-docs.mjs` | «Documentation checks passed (7 files, 10 external links)» |
+| Инварианты модели | не гонял отдельной командой — `npm test` уже включает `test/model-invariants.test.mjs` на всех моделях проекта (зелёный); задача не вводит новую конфигурацию для точечной проверки `npm run invariants -- --config` |
+| Смок-выборка | `node scripts/smoke-select.mjs --base origin/dev --head HEAD` → 7 прямых совпадений + 1 «зарегистрированная связь» (см. §3) | все 8 + 3 названных в AC6 (`island_rooms`, `room_resize`, `resize_pointer_real_plan`) прогнаны — **11/11 OK** (после `node scripts/bundle-sync.mjs`, без которого `demo/srv/assets/houseplan-card.js` отсутствовал и смоки падали таймаутом на билд, не на код) |
+| Мутационная проверка (ручная, не полный `mutation-gate.mjs`) | вручную применил 3 патча из `scripts/mutation-gate.mjs` (П3-снова-квадратичен, П4-снова-перебор, протухший baseline-кэш) и прогнал их guard-команды | П3 и П4 **ловятся бенчем** (`pass:false`, tsSegmentLengthsMs 402.6/60 и tsNodeDistancesMs 207.4/80 соответственно); третий мутант — см. находку M1 ниже, там дисциплина «тест умеет падать» провалилась на смоке |
+| golden | не гонял | diff не трогает рендер/геометрию/стили/слои — только порядок вычислений и кэш валидатора; визуальный результат не может измениться этим кодом |
+| performance-профили сверх §5 | не гонял | не названы в AC сверх §5/AC7 |
+
+## 3. Смок-выборка — вывод инструмента
+
+```
+Изменено файлов src/**: 2 · символов на изменённых строках: 17
+Прямое совпадение (7): smoke_junction_holes, smoke_decor, smoke_glow_fail_dark,
+  smoke_glow, smoke_grid_scale_invariance, smoke_junction_limits, smoke_space_scale_defaults
+Зарегистрированная связь (1): smoke_real_plan_masonry ← wallBodiesGeometry
+```
+Плюс три смока, названных в AC6 автором (`island_rooms`, `room_resize`,
+`resize_pointer_real_plan`) — не входят в прямые совпадения инструмента для
+этого diff, но названы в AC, поэтому прогнаны. Итог: 11 смоков, все **OK**.
+
+## 4. Находки
+
+### M1 (в скоупе) — заявленная «поведенческая половина» AC4 не различает наличие и отсутствие кэша
+
+Файл: `demo/smoke_junction_limits.mjs:185-217`; коммит `c90f5bf0` и
+подтверждающий комментарий автора («Поведенческая половина AC4 (N move →
+N+1 вычислений на настоящем pointer-жесте)»).
+
+Смок считает вызовы `_junctionLimitViolations` за весь жест ресайза и
+проверяет `resizeBaselineCachedPerGesture = jlCalls > 0 && jlCalls <= 12`.
+Я вручную воспроизвёл ровно тот мутант, что заведён в
+`scripts/mutation-gate.mjs` под id `junction-limit-baseline-cache-stale`
+(отключил запись в `_junctionBaselineCache` — кэш никогда не срабатывает,
+baseline пересчитывается на каждый move) и прогнал **сам смок** (не юнит,
+который guard мутанта вызывает вместо него):
+
+```
+DEBUG jlCalls= 12    (кэш полностью отключён)
+DEBUG jlCalls= 11    (немодифицированный код, кэш работает)
+OK                   (оба раза)
+```
+
+Разница между «кэш работает» и «кэш полностью выключен» — **один вызов из
+двенадцати**, и порог `<= 12` пропускает оба случая. Смок физически не может
+отличить рабочий кэш от полностью удалённого — он зелёный в обоих случаях.
+Причина в том, что счётчик `window.__jlCalls` ловит вообще все обращения к
+`_junctionLimitViolations` за время жеста (включая вызовы вне
+`_junctionLimitsIntroduced`, например из рендера/подсветки), а не только
+экономию на baseline — сигнал тонет в шуме, и заявленная арифметика
+«N+1 против 2N» на практике не воспроизводится этим жестом ни при каком
+разумном пороге.
+
+Это ровно тот класс дефекта, который автор сам поймал для ЮНИТ-теста тем же
+приёмом в `7aaf5a72` («тест умеет падать» — контракт перепрятали в
+проверку по исходнику именно потому, что первая версия юнита не ловила
+мутанты). Для смока тот же самоконтроль не был применён: заявление в
+коммите, что поведенческая защита AC4 «живёт в смоке», не подтверждается.
+
+Фактическая защита AC4 сейчас — только regex-проверка по исходнику
+(`test/junction-limits.test.mjs:380-399`, ищет литеральную строку
+`cached.epoch === this._cfgEpoch`). Она реально работает как барьер (я
+проверял: мой мутант меняет именно эту подстроку и юнит падает), но это
+защита от одной конкретной формы регресса (удаление/подмена этого сравнения
+в тексте), а не от логической. Сама РЕАЛИЗАЦИЯ кэша в продакшен-коде
+корректна — я прочитал `_junctionLimitsIntroduced` (houseplan-card.ts:7503-7549)
+и логика верна: WeakMap по identity `previousConfig`, инвалидация по
+`_cfgEpoch`, который сам ведёт себя как установившийся паттерн в этом файле
+(бампается в ~30 местах при любом реальном изменении `_serverCfg`/эпизода
+редактирования). Это находка про КАЧЕСТВО ТЕСТА, а не про баг в фиче.
+
+**Почему в скоупе и Medium:** AC4 ТЗ требует доказательства «жест ресайза
+считает baseline один раз»; заявленное доказательство decorative. Без
+High это возвращается автору жёлтым, а не заводится отдельным issue —
+находка про код и тесты ЭТОЙ задачи, не соседнего поведения.
+
+**Что нужно поправить:** либо ужесточить порог до значения, которое
+реально разделяет 11 и 12 (например, точное равенство ожидаемому числу
+вызовов для конкретной синтетической последовательности `mouse.move` — тогда
+и сам тест обязан быть проверен на этом же мутанте), либо честно
+переформулировать комментарий/заявление AC4: поведенческая защита не
+добавляет покрытия сверх юнита по исходнику, и это нужно признать в
+документе, а не заявлять как отдельную гарантию.
+
+## 5. Закрытие раунда r1
+
+| Находка r1 | Чем закрыта | Где видно |
+|---|---|---|
+| H1 — докс-гейт красный (screenshots.json/PNG не из одного прогона) | Уже было закрыто `7513f93d` до публикации вердикта; после второго ребейза (`85636a65`) фингерпринт и PNG пересобраны ещё раз тем же правилом | `node scripts/check-docs.mjs` → «Documentation checks passed» на этом SHA (проверено мной заново) |
+| H2 — бюджеты §5 калиброваны с запасом 1.14× вместо 2-3×, бенч не в CI-воркфлоу | Таблица замеров с обеих машин, бюджеты пересчитаны от худшей; шаг `npm run benchmark:junction-limits` добавлен в job `performance_smoke` в `validate.yml` | `.github/workflows/validate.yml:543-547`; сам прогон на этом SHA — `pass:true`, запас 2.65× (150.7/400) |
+| M1 — обещанный AC1-тест на самом деле мерил только тёплый `validate_junction_limits`, не `websocket_api.py`/`hass` | Новый тест `test_330_config_set_validators_run_in_the_executor` патчит `validate_junction_limits` обёрткой-шпионом внутри реального HA-харнесса, вердикты (принято/отклонено `junction_limit_angle`) сверены | `tests_backend/test_ha_websocket.py:2322-2385`; **я прогнал его исполнением** (`pytest -k 330` → `1 passed`), не только чтением |
+| M2 — §4.6-эквивалентность фронта проверена только текстовым паттерном; python-паритет на одной фикстуре | Три граничные фикстуры (as-is==through-migration) на TS (`test/junction-limits.test.mjs:401`) и на python (`tests_backend/test_junction_limits.py:423`, три полигона: spike/box/narrow) | Оба набора прогнаны: TS — часть зелёного `npm test`; python — часть зелёного `pytest tests_backend/test_junction_limits.py` (13/13) |
+
+Все четыре находки r1 закрыты по существу, подтверждено исполнением (не
+только чтением) там, где это было в моих силах в этой песочнице.
+
+## 6. Унаследовано из r1 (без повторной проверки)
+
+- Общая архитектура срезов §4.1–§4.6 (rev-кэш, byNode-индекс П3,
+  «v9 как есть» §4.6) — признана в code-review r1 корректной по коду и
+  эквивалентной по вердиктам; в этом раунде я перечитал тот же код заново
+  (диф не менялся с r1 в этой части, кроме docs/CHANGELOG и бенч-калибровки)
+  и подтверждаю тот же вывод самостоятельно, а не по доверию к r1 — это не
+  наследование, а совпавший результат независимой проверки.
+- Ничего из r1 не наследуется слепо: поскольку исходный SHA r1 недостижим
+  после ребейза (см. §0), весь код перечитан заново в этом раунде, а не
+  предположен на основании старого вердикта.
+
+## 7. Что проверено и корректно
+
+- **Эквивалентность вердиктов П1–П5** до и после — не изменилась ни в одной
+  строке содержательной логики; подтверждено чтением и параллельно
+  зелёными паритет-тестами TS↔Python (`test_parity_with_the_frontend_checks`,
+  уже существовавший до #330, прошёл в составе 13/13).
+- **Bucket-решётка П4**: прочитал построчно оба зеркала (`junction_limits.py:223-291`,
+  `junction-limits.ts:188-247|`); граничный случай (узел и сегмент в соседних
+  ячейках решётки, но ближе порога) закрыт паддингом bbox сегмента на
+  `min_units` и проверен тестом `test_330_p4_bucket_matches_bruteforce_on_cell_borders`
+  (4 кейса, включая ровно 4 и 5 см от порога) — прогнан, зелёный.
+  Ассиметрия «узел проверяется в 9 соседних ячейках, сегмент — только в
+  своей» корректна по построению: паддинг сегмента компенсирует то, что для
+  узлов (точек) эквивалентного паддинга нет.
+- **§4.6 «v9 как есть»**: перечитан барьер в `websocket_api.py:1325` (schema
+  всё равно валидирует документ целиком независимо от пропуска ремиграции) —
+  риск «подделать v9 без каталога» закрыт независимо от нового теста.
+- **Executor-обёртка** в `ws_config_set`/`ws_plan_optimize`: `write_lock`
+  держится вокруг `await`, поэтому сериализация записей не нарушена; ошибка
+  `too_large` в `ws_plan_optimize` корректно вынесена из executor-функции
+  обратно в вызывающий код перед `connection.send_error` (сеть — дело event
+  loop, не executor-потока).
+- **`rt.junction_baseline`**: инвалидация «по факту», а не по всем путям
+  записи — `ws_plan_optimize` не обновляет кэш вовсе (и не обязан: он не
+  проверяет ограничения стыков ни до, ни после #330 — не регрессия), поэтому
+  следующий `config/set` просто промахнётся по rev и пересчитает `previous`
+  с нуля; корректность не зависит от того, кто ещё пишет rev.
+- **§4.7 общий проход**: `preflight.wallGeometry`, переданный в
+  `_junctionLimitsIntroduced` как `candidateGeometry`, построен
+  (`_rszSpaceCandidateGeometry`) из ТОЙ ЖЕ пары (`spaceId`, `sp`), что и
+  `limitCandidate` — не подмена по случайному кэшу; проверено чтением обеих
+  функций (houseplan-card.ts:9104-9121, 9156-9175).
+- **CHANGELOG** — одна строка в обоих файлах, в том же коммите (`c90f5bf0`),
+  что и код, при `User-Visible: yes`.
+- **Одно число — один источник**: diff не добавляет и не меняет ни одной
+  видимой пользователю величины (только внутренние тайминги/пороги
+  бенчмарка, невидимые пользователю) — раздел неприменим.
+
+## 8. Чего не проверял и почему
+
+- **golden:verify** — не гонял; diff не трогает рендер, геометрию,
+  стили или слои, только порядок/кэш вычислений валидатора стыков.
+- **Полная матрица `demo/smoke_*.mjs` (194 шт.)** — не гонял; задача не
+  меняет ничего, что затрагивает широкие символы (`_cfgEpoch`, `_serverCfg`,
+  `_space` инструмент явно исключил как «не широкие» в данном случае), 11
+  выбранных/названных в AC смоков — зелёные.
+- **`benchmark_safe_resize`** — не гонял; это тот самый бенч, про который
+  spec-review r1 (H2) установил, что он не вызывает код лимитов вообще —
+  нерелевантен этой задаче.
+- **Полный `scripts/mutation-gate.mjs` прогон (все ~40+ мутантов)** — не
+  гонял; это еженедельный гейт (PROCESS.md §8), не гейт ревью. Три НОВЫХ
+  мутанта проверил вручную построчно (см. §2, §4) — этого достаточно для
+  вопроса «умеет ли тест, который я прогонял, падать».
+- **`npm run invariants -- --config <...>`** отдельной командой — не
+  гонял; задача не вводит новую конфигурацию/фикстуру геометрии за
+  пределами того, что уже покрыто `npm test` (который включает
+  `model-invariants.test.mjs` на моделях проекта).
+- **Ручное тестирование в браузере** — вне цикла ревью по регламенту; браузерные
+  смоки — единственная замена, они прогнаны.
+
+## 9. Итог
+
+Один Medium в скоупе (M1 — заявленная поведенческая защита AC4 не
+дискриминирует регресс, эмпирически подтверждено), High нет. Основная
+перф-архитектура (§4.1–§4.7), эквивалентность вердиктов и перф-контракт §5
+подтверждены исполнением, а не только чтением или доверием к предыдущему
+раунду.