diff --git a/docs/reviews/SPEC-REVIEW-330-r2.md b/docs/reviews/SPEC-REVIEW-330-r2.md new file mode 100644 index 00000000..2bb5ec6b --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-330-r2.md @@ -0,0 +1,178 @@ +# SPEC-REVIEW-330-r2 + +Issue: #330 · Этап: spec (ТЗ на ревью, PROCESS.md §2.4) · Заход r2 · +блокирующих циклов 1/4 + +Материал: `docs/specs/330-junction-limits-performance.md` на SHA `36a4aa70` +(HEAD на момент ревью). Предыдущий вердикт: жёлтый, заход r1, документ +`docs/reviews/SPEC-REVIEW-330-r1.md`, получен на SHA `f1b7c237`. + +## Скоуп + +Ревизия 2 правит два High и один Medium из r1: пересчитывает перф-бюджеты §5 +от профилированных чисел (H1), добавляет два новых среза решения — §4.5 +(bucket-индекс для П4 в обеих реализациях) и §4.6 (документ текущей версии +не мигрируется повторно), переписывает AC2–AC7 под новую доказательную базу +и заменяет ссылку AC4 на новый бенч вместо нерелевантного +`benchmark_safe_resize` (H2), добавляет §9 с обязательными разделами i18n/ +touch/риски/release-артефакты (M1). Продуктовая рамка (§1: сценарий, что +человек увидит) и граница «вердикты П1–П5 не меняются» (§3) — без изменений. + +Дельта не локальна: она меняет состав самого технического решения (два +новых среза, §4.5/§4.6), весь перф-контракт §5 и весь набор AC — то есть +затрагивает больше половины документа и требует полной переверки этой части, +а не только «строк, где стоят r1-H1/H2/M1». Разбор ниже — полный по §2–9; +инвентаризация §7.1 и продуктовая рамка §1 унаследованы из r1 без повторной +проверки (раздел «Унаследовано» ниже) — они делтой не задеты. + +## Как проверялось + +Чтением, без исполнения — на этапе spec иное не предусмотрено. Против +каждого нового утверждения решения проверялся код, который оно описывает, +чтобы отделить профилированный факт от догадки: + +- `custom_components/houseplan/wall_segment_model.py:674-684` + (`commit_wall_segment_model`) — подтверждает предпосылку §4.6: функция + прогоняет `_migrate_space`/`_atomize` для КАЖДОГО пространства безусловно, + включая документ, чей `model_version` уже равен `WALL_SEGMENT_MODEL_VERSION` + (флаг `initial_migration` меняет логику присвоения id внутри + `_migrate_space`, но не пропускает атомизацию целиком). Утверждение «деньги + не в deepcopy, а в `_atomize` даже для актуального документа» — не + голословно. +- `custom_components/houseplan/junction_limits.py:150-245` + (`collinear_run_length_units`, `check_node_distances`) — подтверждает, что + П4 — все пары узлов (`O(n²)`) плюс узел×сегмент, отдельный от П3 дефект + архитектуры, а не переиспользуемый индекс, как заявлено в §2 п.3 и §4.5. +- `custom_components/houseplan/websocket_api.py:1280-1341` (`ws_config_set`) + — подтверждает, что `validate_wall_model_transition` (барьер модели) + выполняется безусловно и валидирует документ целиком ДО + `validate_junction_limits` — предпосылка §4.6 «барьер ниже по конвейеру + по-прежнему валидирует документ целиком» верна, риск «клиент прислал + неканонический v9» действительно закрыт независимым барьером, а не только + AC5. +- `src/houseplan-card.ts:7418-7473, 12896, 13044` и + `src/wall-segment-model.ts:821-827` — подтверждают, что фронтенд сам + выставляет `model_version = 9` при клиентской миграции и различает + `_serverCfg.model_version >= 9`: гипотеза «типичный кандидат уже v9 после + первого сохранения» опирается на реальный код, а не на предположение. +- Существующие файлы, на которые ссылаются AC/§7: `test/junction-limits.test.mjs`, + `tests_backend/test_junction_limits.py`, `demo/smoke_junction_limits.mjs`, + `demo/smoke_island_rooms.mjs`, `demo/smoke_room_resize.mjs` — существуют. + `demo/benchmark_junction_limits.mjs` — не существует (и не должен: код ещё + не написан, это предмет реализации). +- `scripts/mutation-gate.mjs:541-580` — три существующих мутанта + `junction-limit-*` подтверждают, что новые имена мутантов в §7 + (`junction-limit-p3-quadratic-again`, `junction-limit-p4-bruteforce-again`, + `junction-limit-baseline-cache-stale`) следуют реальному соглашению + именования, а не придуманы. +- Арифметика §5 сверена построчно с числами §2 (корни) и с числами r1-H1 + (754/376/105 мс) — см. «Что проверено и корректно». + +Не проверялось: исполнение прототипов (44 мс/бакет-П4 и т.д. — числа автора, +принятые как измеренные там, где так и помечено, и как «ожид.» там, где +помечено оценкой). Это работа код-ревью, не этого этапа. + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где видно | +|---|---|---| +| **H1** — бюджеты §5 недостижимы решением §4 (сумма «миграция candidate 754 мс + П4 бэкенд 376 мс» = 1130 мс против заявленных ≤150 мс) | Добавлены два новых среза решения: §4.5 (bucket-индекс П4, прототип 372→44 мс на python, вердикты идентичны) и §4.6 (документ с актуальным `model_version` используется без повторной миграции — снимает 754/69 мс на типичном горячем пути). Бюджеты §5 пересчитаны от чисел этих прототипов, с явным разделением «тёплый путь» (v9-кандидат, rev-кэш previous) и «холодный» (легаси, единственный случай, укладывается в лимит только потому, что цепочка уже в executor согласно §4.1) | `docs/specs/330-junction-limits-performance.md` §4.5, §4.6, §5 (таблица), AC1/AC2/AC5. Код-предпосылки подтверждены чтением: `wall_segment_model.py:674-684`, `websocket_api.py:1325-1334` | +| **H2** — AC4 ссылался на `benchmark_safe_resize`, который не вызывает изменяемый код | AC4 переписан: доказательство — юнит-счётчик на `_junctionLimitsIntroduced` (как и было) плюс НОВЫЙ бенч §5, который явно описан как измеряющий «TS-проверки П1–П5 напрямую и полный `_junctionLimitViolations` на смонтированной карточке»; `benchmark_safe_resize` в тексте AC4 прямо названо как не-доказательство («r1-H2») | `docs/specs/330-junction-limits-performance.md` §5, AC4 | +| **M1** — отсутствуют i18n/touch/риски/release-артефакты (§7.1 DoR) | Добавлен §9 с четырьмя явными пунктами: i18n «нет», touch «не задет», три названных риска (executor/write_lock, эквивалентность §4.6, геометрическая эквивалентность bucket-П4) с указанной для каждого проверкой, release-артефакты (обычная бета, одна строка CHANGELOG RU+EN, без миграции) | `docs/specs/330-junction-limits-performance.md` §9 | + +## Находки + +Нет находок уровня High или Medium. + +**Low-1 (снята решением ревьюера, без правки).** Строка §5 «холодный +(легаси-обе-стороны)» описывает синтетический худший случай для бенча, а не +типичный сценарий «первая запись после обновления» (в котором, по коду +фронтенда, `candidate` уже несёт `model_version: 9`, а легаси остаётся только +`previous`). Название не искажает AC и не меняет бюджет — бенч просто держит +более тяжёлый из двух легаси-случаев с запасом. Не блокирует: ни один AC не +опирается на точное соответствие названия сценарию, и §2 отдельно описывает +реальный случай смешанной версии. + +**Low-2 (снята решением ревьюера, без правки).** Ряд §5 «TS `checkNodeDistances` +(П4, 576) ~15 мс» помечен как «ожид.», но в тексте §4.5 измерен прототип +только для python (372→44 мс); TS-число — экстраполяция по той же +пропорции, не отдельный замер. Это не «догадка, выданная за факт»: колонка +честно озаглавлена «После фикса (ожид.)», и AC2/AC7 всё равно требуют +подтверждения бенчем и мутантом на этапе код-ревью — оценка не подменяет +доказательство. + +## Что проверено и корректно + +- Арифметика бюджетов §5 сходится с числами §2 и r1: тёплый путь (линейный + П3 + bucket-П4 44 мс + отсутствие повторной миграции v9-кандидата) даёт + порядок «около 100 мс» бэкенд / «около 40 мс» фронт — это прямо снимает + расхождение на порядок, из-за которого r1 поставил H1. Холодный путь + (~1.7 с) остаётся только в executor (AC1 требует loop-времени, а не + общего CPU-времени), поэтому попадание в ≤50 мс loop-бюджета не зависит от + того, насколько медленна миграция легаси-документа. +- §4.6 не противоречит #329: барьер `validate_wall_model_transition` + (`websocket_api.py:1325`) выполняется независимо от лимитов и валидирует + документ целиком, так что пропуск повторной миграции в путях лимитов не + открывает дыру для непроверенного «поддельного v9». +- AC5 (эквивалентность §4.6) сформулирован проверяемо: конкретные фикстуры + границ (14°/15°, 19/20 см, T-стык, доборный атом), существующий механизм — + паритет-тест, который уже есть в `test/junction-limits.test.mjs`/ + `tests_backend/test_junction_limits.py`. +- AC4 больше не создаёт видимость покрытия: новый бенч явно вызывает именно + изменяемый код, а не соседний примитив ресайза. +- §9 закрывает все четыре пункта M1 явными и по существу корректными + ответами (в частности, риск «неканонический v9» получает named + mitigation, а не декларацию). +- Имена новых мутантов и ссылки на существующие смоки/тесты в §7 + соответствуют реальному дереву репозитория — не выдуманы. +- Открытых продуктовых вопросов владельцу по-прежнему нет; ревизия 2 — + целиком технический ответ на технические находки r1, что и требовалось + (§7.1: технический спор автора и ревьюера решается вердиктом ревью, не + эскалируется). + +## Чего не проверял + +- Не запускал `npm test`/`pytest tests_backend`/`typecheck`/бенчи — на этапе + spec-review это не предусмотрено PROCESS.md §2.4 (кода этой задачи ещё + нет; тронут только текст ТЗ). +- Не выполнял прототипы автора (cProfile/bucket-индекс) — их числа приняты + как измеренные там, где это прямо написано («по профилю», «прототип на + python»), и как оценка там, где написано «ожид.» (см. Low-2). Воспроизвести + эти числа под нагрузкой — работа код-ревью по AC1/AC2/AC7. +- Не проверял детали будущей структуры bucket-индекса П4 (шаг сетки, + 9-окрестность) глубже, чем нужно для проверки правдоподобия заявленного + ускорения — §7.1 оставляет конструкцию реализации на усмотрение авторов + кода. +- Не проверял `AUDIT-2026-08-28.md` (упомянут в теле issue) — не найден в + дереве репозитория; как и в r1, не блокирует: цифры продублированы и + подтверждены в аналитике и в этом документе напрямую по коду. + +## Унаследовано из r1 + +Без повторной проверки в этом раунде — делта их не касается: + +- Продуктовая рамка §7.1 («сценарий», «что человек увидит», персона home + admin на десктопе выводится из контекста) — принята в + `docs/reviews/SPEC-REVIEW-330-r1.md` на SHA `f1b7c237`, текст §1 не менялся + в ревизии 2 (сверено диффом `f1b7c237..36a4aa70`). +- Корень П3 (`collinear_run_length_units` строит индекс заново на каждый + сегмент) и синхронность цепочки `ws_config_set` под `write_lock` в event + loop — подтверждены чтением кода в r1 (`junction_limits.py`, + `websocket_api.py:1303-1334`), в этом раунде код не менялся, повторно не + перечитывал построчно (кроме диапазона 1280-1341, проверенного заново для + H1/§4.6 — см. «Как проверялось»). +- Граница §3 «вердикты П1–П5 не меняются ни на бит» и механизм её проверки + (паритет-тест + три мутанта #329) — принята в r1, текст границы в ревизии 2 + расширен (добавлена одна строка про атомизацию вне скоупа), но сама + граница и её проверяемость не изменились. +- Откат §8 (чистый revert, кэши только в памяти) — не менялся, принят в r1. +- Отсутствие открытых продуктовых вопросов владельцу — подтверждено в r1 и + остаётся верным: оба цикла (r1→r2) были чисто техническими. + +## Вывод + +Оба High и Medium из r1 закрыты по существу, с кодовым подтверждением +ключевых новых предпосылок (§4.6 — барьер модели независим и валидирует +документ целиком; премисса «типичный кандидат уже v9» опирается на реальный +код фронтенда, а не на догадку). Новых High/Medium решение не вносит. +Вердикт: зелёный.