mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-30 19:58:50 +00:00
@@ -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 решение не вносит.
|
||||
Вердикт: зелёный.
|
||||
Reference in New Issue
Block a user