From 90a3313d9270d9aa9ef58ecf01c22f274f3aea6c Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Fri, 28 Aug 2026 00:49:56 +0000 Subject: [PATCH] docs: review document for #330 Issue: #330 User-Visible: no --- docs/reviews/CODE-REVIEW-330-r3.md | 226 +++++++++++++++++++++++++++++ 1 file changed, 226 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-330-r3.md diff --git a/docs/reviews/CODE-REVIEW-330-r3.md b/docs/reviews/CODE-REVIEW-330-r3.md new file mode 100644 index 00000000..b47905c2 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-330-r3.md @@ -0,0 +1,226 @@ +# CODE-REVIEW-330-r3 + +Issue: #330 — «Ограничения стыков (#329) блокируют event loop HA и дёргают ресайз на больших планах» +Этап: code (PROCESS.md §2.7) +Заход: r3 · блокирующих циклов израсходовано 2 из 4 (до этого раунда) +Ветка: `issue/330-junction-limits-performance`, HEAD ревью — `04bb54ae` + +## 0. Разбор по дельте — почему и от какого SHA + +Предыдущий вердикт (code-review r2, 2026-08-28T00:30:25Z) назван на HEAD +`85636a65`. Проверено, что этот SHA — прямой предок текущего HEAD и что +между ними ребейза не было: + +``` +$ git merge-base --is-ancestor 85636a65 HEAD && echo ancestor +ancestor +$ git log --oneline 85636a65..HEAD +04bb54ae fix: the baseline cache keys on geometry, and the smoke tells all three worlds apart (#330 r2-M1) +294d7047 docs: review document for #330 +``` + +Два коммита, оба ожидаемые: `294d7047` — публикация документа r2 (не +код), `04bb54ae` — правка автора на единственную находку r2 (M1). Дельта +локальна: один файл продуктового кода, один смок, один юнит-тест, один +мутант-гвард. Контракт поведения не менялся, новая подсистема не задета, +объём дельты много меньше исходной задачи. Условия §2.10/§7.2 для полного +разбора не выполнены — разбираю по дельте `git diff 85636a65..HEAD`. + +## 1. Скоуп дельты + +`git diff 85636a65..HEAD --stat`: + +``` + .../houseplan/frontend/houseplan-card.js | 4 +- + demo/smoke_junction_limits.mjs | 56 ++++- + dist/houseplan-card.js | 4 +- + docs/reviews/CODE-REVIEW-330-r2.md | 257 +++++++++++++ + scripts/mutation-gate.mjs | 9 +- + src/houseplan-card.ts | 25 +- + test/junction-limits.test.mjs | 6 +- + 7 files changed, 336 insertions(+), 25 deletions(-) +``` + +Только r2-M1 (АС4 «поведенческая часть кэша не различает рабочий кэш от +удалённого»): + +- `src/houseplan-card.ts:7501-7553` (`_junctionLimitsIntroduced`) — ключ + кэша `_junctionBaselineCache` заменён с `(epoch, spaceId)` на + `(spaceId, fingerprint)`, где `fingerprint = spacePhysicalGeometryFingerprint(previousSpace)` + — существующая, уже применяемая в этом же файле утилита (строки 7212, + 7569-7570, 7676, 7751-7752, 7800, 16031), а не новый механизм. +- `demo/smoke_junction_limits.mjs` — счётчик теперь считает ТОЛЬКО вызовы + `_junctionLimitViolations` с документом `=== card._serverCfg` (вычисления + baseline, не кандидата), плюс второй жест ресайза с коммитом между ними, + чтобы отличить «кэш никогда не инвалидируется» от «кэш инвалидируется + честно». +- `test/junction-limits.test.mjs` — regex источника обновлён под новое имя + полей (`cached.fingerprint === fingerprint`, `spacePhysicalGeometryFingerprint(previousSpace)`). +- `scripts/mutation-gate.mjs` — guard мутанта `junction-limit-baseline-cache-stale` + теперь запускает и смок (поведенческая часть), и юнит (исходник), было — + только юнит. +- `dist/**`/frontend-копия — механический пересчёт бандла тем же коммитом. + +## 2. Закрытие раунда r2 + +| Находка r2 | Чем закрыта | Где это видно | +|---|---|---| +| M1 (в скоупе) — заявленная «поведенческая половина» AC4 не различала рабочий и удалённый кэш (11 против 12 вызовов тонули в шуме); реальный баг вскрылся тем же разбором: кэш ключевался на `_cfgEpoch`, который тикает на каждом принятом превью-оверлее без изменения хранимого документа, поэтому baseline пересчитывался ~4 раза за жест, а не 1 | Ключ кэша заменён на identity документа + `spacePhysicalGeometryFingerprint` его пространства — содержание, не счётчик; превью-оверлей fingerprint не трогает, in-place structural commit меняет геометрию и инвалидирует честно. Смок считает отдельно вычисления baseline (документ `===_serverCfg`) через два жеста с коммитом между ними: ожидание строго 1, затем строго 2 | `src/houseplan-card.ts:7518-7553`; `demo/smoke_junction_limits.mjs:186-254`; я применил ровно мутант `junction-limit-baseline-cache-stale` (условие кэша обратно на `cached.spaceId === spaceId` без fingerprint) и прогнал сам смок — `resizeBaselineRecomputedAfterCommit: expected true, got false` (см. §3); без мутанта смок и `mutation-gate.mjs --id=junction-limit-baseline-cache-stale` — оба зелёные (см. §3) | + +Находка закрыта по существу и подтверждена исполнением, не только чтением. + +## 3. Как проверялось (дельта, r3) + +Зелёного Validate на `04bb54ae` не найдено — гоняю дешёвые гейты сам, плюс +`check-docs` (диф трогает `src/**`) и целевой смок/мутант по дельте. + +| Гейт | Команда | Результат | +|---|---|---| +| Типы | `npx tsc --noEmit` | чисто, без вывода | +| Юниты | `npm test` | `# tests 1420 / pass 1419 / fail 0 / skipped 1` | +| Сборка + идентичность 3 копий бандла | `npm run build && npm run bundle:sync`, затем `cmp` попарно всех трёх файлов | все три `cmp` — молча, идентичны; `git status --short` после сборки чист | +| Докс-гейт (диф трогает `src/**`) | `node scripts/check-docs.mjs` | **`exit 1`** — `ERROR screenshot source fingerprint is stale; run npm run build && node demo/docs/capture.mjs`. Проверил напрямую: записанный `sourceFingerprint` в `docs/images/screenshots.json` — `2a5ae6c1…`, реальный `visualFingerprint()` на этом дереве — `dbac4a84…`. Последний коммит, трогавший `screenshots.json`, — `85636a65` (ДО фикса); `04bb54ae` поменял `src/houseplan-card.ts` и не пересчитал фингерпринт — см. находку H1 | +| Целевой смок AC4 | `node demo/smoke_junction_limits.mjs` | `OK` (после свежей сборки и `bundle:sync`) | +| Тест умеет падать (AC4/мутант) | вручную заменил условие кэша в `src/houseplan-card.ts` на мутант `junction-limit-baseline-cache-stale` (`if (cached && cached.spaceId === spaceId)`, без fingerprint), пересобрал бандл, прогнал смок | `FAILED (1): resizeBaselineRecomputedAfterCommit: expected true, got false` — мутант ловится; источник восстановлен, пересобран, смок снова `OK`, `git status --short` пуст | +| Тот же мутант через гвард | `node scripts/mutation-gate.mjs --id=junction-limit-baseline-cache-stale` | `ok junction-limit-baseline-cache-stale: тест покраснел, как обязан`; «поймано 1 из 1» | +| Выборка смоков по дельте | `node scripts/smoke-select.mjs --base 85636a65 --head HEAD` | 1 файл `src/**` изменён, 3 символа на изменённых строках. Одна **зарегистрированная связь**: `demo/smoke_wall_union_isolation.mjs` ← `spacePhysicalGeometryFingerprint`. Прогнан отдельно (см. ниже) | +| `smoke_wall_union_isolation.mjs` (по связи из smoke-select) | `node demo/smoke_wall_union_isolation.mjs` | `OK`, все 11 полей `true` | +| Перф-контракт §5/AC7 (не трогается дельтой, но это перф-задача — перепроверил) | `npm run benchmark:junction-limits` | `pass: true`, `tsFullCandidateMs: 152` (бюджет 400, запас 2.6×) | + +### Отдельная проверка: не добавила ли дельта скрытую перф-регрессию + +`_junctionLimitsIntroduced` теперь считает `spacePhysicalGeometryFingerprint` +на КАЖДЫЙ вызов (и при попадании в кэш, и при промахе) — раньше сравнение +было целочисленным O(1). Измерил стоимость самой функции на фикстуре §5 +(576 атомов, 12×12) отдельно от бенчей (ни `benchmark_junction_limits.mjs`, +ни `benchmark_safe_resize.mjs` не проходят через этот метод класса — оба +вызывают чистые функции/`resize.js` напрямую, это отмечено ещё в r1/r2): + +``` +$ node --input-type=module -e '... spacePhysicalGeometryFingerprint(space) × 200 ...' +avg ms per call: 1.71 +``` + +~1.7 мс на вызов на самой крупной фикстуре задачи — на два порядка меньше +бюджета кадра и на два порядка меньше стоимости, которую §4.4 устраняет +(до фикса пересчёт baseline стоил сотни мс — 4.2 с на полный набор до §4.7). +Не блокирует, отмечаю как проверенное наблюдение: этот путь остаётся не +покрыт числовым перф-бюджетом ни в одном бенче — не находка этой задачи +(оба бенча так же не покрывали его в r1/r2), но стоит иметь в виду, если +§330 продолжится следующим срезом. + +## 4. Находки + +### H1 — докс-гейт красный на HEAD ревью (в скоупе, блокирует) + +`node scripts/check-docs.mjs` завершается `exit 1`: + +``` +ERROR screenshot source fingerprint is stale; run npm run build && node demo/docs/capture.mjs +``` + +Коммит `04bb54ae` меняет `src/houseplan-card.ts` (комментарий + логика +кэша), но не пересчитывает `docs/images/screenshots.json`. Отпечаток +считается по всему `src/**` (PROCESS.md §8), поэтому любая правка +фронтенда делает его устаревшим механически, без вариантов трактовки — +ровно класс дефекта, который уже стоил `dev` красного джоба `docs` на +#230/#234 (найдено только на следующей задаче, #237). Сама задача #330 +уже дважды проходила через этот гейт (r1-H1 в code-review, тот же класс) +и знает цену пропуска. UI не изменился (изменение сугубо внутреннее, +кэш-логика), поэтому PNG, вероятно, останутся байт-в-байт те же — но +запись `sourceFingerprint` обязана быть пересчитана командой `npm run +build && node demo/docs/capture.mjs` и принята `npm run docs:accept +-- --reviewed --from=<артефакт>` тем же правилом, что и раньше в этой +задаче; самому пересобирать и коммитить в рабочей копии не в праве +(ревьюер не правит код). + +**Что нужно поправить:** пересчитать фингерпринт/скриншоты и закоммитить +вместе (или отдельным коммитом класса C поверх), убедиться, что +`check-docs` зелёный на итоговом SHA. + +Находок Medium/Low, не закрытых в тексте, нет. + +## 5. Унаследовано из r2 (без повторной проверки) + +- **Архитектура срезов §4.1–§4.7** (executor-цепочка, rev-кэш `previous`, + byNode-индекс П3, bucket-решётка П4, §4.6 «v9 как есть», общий проход + §4.7) — признана верной по коду и по вердиктам в code-review r2 + (`docs/reviews/CODE-REVIEW-330-r2.md`, HEAD `85636a65`, разделы 3 и 7). + Дельта r3 их не касается ни одной строкой (см. §1) — принимаю без + повторного чтения. +- **AC1 (event loop/executor)** — HA-харнесс тест + `test_330_config_set_validators_run_in_the_executor`, прогнан исполнением + в r2 (`pytest -k 330` → `1 passed`, документ r2 §5/§6). Дельта r3 не + трогает `custom_components/**`. +- **AC2/AC3/AC5 (линейный П3/П4, rev-кэш бэкенда, эквивалентность §4.6)** — + подтверждены исполнением в r2 (паритет-тесты TS↔Python, границы ячеек, + `pytest tests_backend -q` 423 passed). Ни один из файлов бэкенда не в + дельте r3. +- **AC7/перф-контракт §5, включение в `validate.yml`** — подтверждено в r2 + чтением воркфлоу (`validate.yml:543-547`) и исполнением + (`pass:true`, запас 2.65×). Я перепрогнал `benchmark:junction-limits` + заново в этом раунде (см. §3) — тот же результат в пределах шума + раннера (152 мс против 150.7 мс в r2), воркфлоу-файл не в дельте, не + перечитывал. +- **golden:verify** — не гонял, как и в r2: дельта не трогает рендер, + геометрию, стили, слои, только ключ кэша и подсчёт вызовов в смоке. +- **Полная матрица `demo/smoke_*.mjs` (194 шт.)** и **полный + `mutation-gate.mjs`** — не гонял целиком, как и в r2: это предрелизные + гейты (PROCESS.md §8), не гейт ревью. По дельте прогнал точечно — + `smoke_junction_limits.mjs` (прямое совпадение) и + `smoke_wall_union_isolation.mjs` (зарегистрированная связь по + `spacePhysicalGeometryFingerprint`, см. §3) — плюс единственный + релевантный дельте мутант через `--id=`. +- **`pytest tests_backend`** — не гонял; дельта не содержит ни одного + файла `.py` (см. `git diff --name-only 85636a65..HEAD`). +- **`npm run invariants -- --config <...>`** отдельной командой — не + гонял; дельта не вводит новую геометрию/фикстуру, только ключ кэша по + уже существующему `spacePhysicalGeometryFingerprint`; `npm test` + прогоняет инварианты модели на существующих моделях проекта и прошёл + (см. §3). +- **Одно число — один источник** — неприменимо: дельта не вводит и не + меняет ни одной пользователю видимой величины (внутренний счётчик + вызовов в смоке и внутренний кэш-ключ невидимы пользователю). +- **CHANGELOG** — не требуется: коммит `04bb54ae` несёт `User-Visible: no` + (внутренняя правка кэша, поведение для пользователя не меняется); + трейлеры коммита корректны (`Issue: #330`, `User-Visible: no`). + +## 6. Что проверено и корректно (эта дельта) + +- Новый ключ кэша (`spaceId + fingerprint`) читается из `previousConfig`, + вычисляется безопасно через `try/catch` с пустой строкой на ошибке; + пустой fingerprint явно исключён из условия попадания в кэш И из записи + в кэш (`&& fingerprint`) — сбой вычисления fingerprint отключает + кэширование для этого вызова, а не выдаёт ложное попадание. + Прочитано построчно (`src/houseplan-card.ts:7526-7553`). +- `spacePhysicalGeometryFingerprint` — не новый механизм, а уже + используемый в этом же файле барьер физической геометрии (6 других + мест), корректность и покрытие которого не в скоупе этой находки. +- Смок теперь физически различает три мира (рабочий кэш = 1 затем 2, + вечный кэш = 1 затем 1, отключённый кэш = ~10 затем ~20) — проверено + и мутантом (см. §3), и обычным прогоном. +- Трейлеры коммита `04bb54ae` — `Issue: #330`, `User-Visible: no`, + корректны для внутренней правки. + +## 7. Чего не проверял и почему + +- `pytest tests_backend` — дельта не трогает `.py`, наследую r2 (§5). +- `golden:verify` — дельта не трогает визуал, наследую r2 (§5). +- Полная матрица смоков (194) и полный `mutation-gate.mjs` (все мутанты) — + предрелизные гейты, не гейт ревью; прогнал точечно по выводу + `smoke-select.mjs` плюс единственный релевантный мутант. +- Перф-бюджет самой `spacePhysicalGeometryFingerprint` внутри + `_junctionLimitsIntroduced` — не заведён отдельным числом ни в одном + бенче (см. §3, «отдельная проверка»); измерил вручную (1.7 мс/вызов на + 576-атомной фикстуре) вместо гона несуществующего гейта — не блокирует, + записано как наблюдение. +- Ручное тестирование в браузере — вне цикла ревью по регламенту; + браузерный смок прогнан, это и есть замена. + +## 8. Итог + +High: 1 (докс-гейт красный на этом SHA — механический пропуск +`check-docs` после правки `src/**`, в скоупе, чинится в этой же задаче). +Medium: 0. Единственная находка r2 (M1) закрыта по существу и +подтверждена мутантом лично. Архитектурная часть решения (§4.1–§4.7) +унаследована из r2 без повторной проверки — дельта её не касается.