Issue: #277 User-Visible: no
18 KiB
CODE-REVIEW-277-r2
- Issue: #277 — безопасный Resize без изменения топологии
- Заход: r2 (второй код-ревью-заход; спецификация прошла отдельные заходы SPEC-REVIEW-277-r1 → r2, зелёный, не переоценивается на этой стадии — §10.4 PROCESS.md, «цикл считается по этапу»)
- Ветка:
issue/277-safe-resize - SHA r1 (предыдущий код-ревью):
241821c40e9b2f43b79945af3377fc3b3f4ac084(взят изdocs/reviews/CODE-REVIEW-277-r1.md, раздел «SHA ревью»; в терсовом вердикт-комментарии issue SHA не назван — восстановлен из полного документа, не по времени коммитов. Это повторение процессного L2 изSPEC-REVIEW-277-r2на новой стадии: снимаю с записью, не блокирует, см. «Находки») - SHA этого ревью:
a548d4f17dc5611d4cfd6f3ffe1042f91a365840(HEAD) - Дельта:
git diff 241821c4..HEAD, 2 продуктовых коммита автора между r1 и HEAD:
| Коммит | Тема | Класс | User-Visible |
|---|---|---|---|
9c5b66c3 |
perf: fingerprint Resize handles once per frame | A (правит src/houseplan-card.ts) |
no |
a548d4f1 |
docs: accept Resize performance screenshots | C | no |
(448ceea3 в диапазоне — коммит самого документа CODE-REVIEW-277-r1.md,
положенный шагом публикации r1; не авторская правка, не часть дельты для
разбора.)
Скоуп
Ровно одна Medium-находка предыдущего раунда (M1: eligibility-кэш ручек
Resize пересчитывал полный geometry fingerprint на КАЖДУЮ ручку на КАЖДЫЙ
рендер вместо одного раза на рендер-слой). Дельта — точечный perf-фикс этой
находки плюс тест/бенчмарк/документация на неё. Геометрия (src/resize.ts),
safety-контракт (resolveSafeResize/applySafeResize/validateSafeResize),
i18n и весь остальной код не тронуты. AC1–AC12, AC14–AC17 дельта не задевает
логически (см. «Унаследовано»); AC13 (перформанс) дельта расширяет —
предмет этого раунда.
Разбор — по дельте (§2.9), не полный: дельта локальна (один файл продукта, один метод и один вызывающий цикл), контракт поведения не меняется, новая подсистема не затронута, объём дельты (13 строк продукта) несопоставим с объёмом исходной задачи.
Как проверялось
| Гейт | Команда | Результат |
|---|---|---|
| Typecheck | npx tsc --noEmit |
зелёный |
| Unit | npm test |
1195/1195 passed, 0 fail (было 1194 в r1: +1 новый тест) |
| Build + байт-идентичность 2 отслеживаемых копий бандла | npm run build + cmp dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js |
зелёный, байт-в-байт |
| Docs fingerprint | node scripts/check-docs.mjs |
«Documentation checks passed (7 files, 10 external links)» |
| Выбор смоков по дельте | node scripts/smoke-select.mjs --base 241821c4 --head HEAD |
«Изменено файлов src/**: 1 · символов на изменённых строках: 4» → 6 «прямых совпадений» (все через символ _rszDrag), 0 «зарегистрированных связей», 0 «неопределённостей»; полный вывод ниже |
| Смоки (6 прямых совпадений) | node demo/smoke_<name>.mjs × 6 |
все 6 зелёные (список ниже) |
| Golden (полный) | npm run golden:verify |
86/86 passed, 0 failed, включая safe-resize-handles-clamp-{light,dark} |
| Новый perf-бенчмарк | npm run benchmark:safe-resize-render |
"pass": true; render p95 1.2–1.8 мс (бюджет 25 мс); snapshotCalls: 20 = ровно 1 на кадр при 20 семплах |
| Мутационная проверка бенчмарка (моя, не автора) | вручную откачен фикс (git apply -R на диапазон 9c5b66c3 по src/houseplan-card.ts), npm run bundle:sync, повторный npm run benchmark:safe-resize-render, затем восстановлено git apply + bundle:sync |
на старом коде: snapshotCalls: 1600 (80 на кадр), render p95 27.9 мс > бюджета 25 мс, "pass": false — бенчмарк ловит именно регрессию M1; рабочее дерево восстановлено, git status чист |
| Performance (числа) | см. выше | p95 1.2–1.8 мс против 130–224 мс из воспроизведения M1 в r1 на той же large-house фикстуре (20 комнат/80 ручек) — не просто «в бюджете», а на два порядка быстрее исходной регрессии |
| Model invariants | не запускались | геометрия (src/resize.ts, resolveSafeResize и т.д.) не тронута этой дельтой — только порядок вычисления уже существующего fingerprint в вызывающем коде рендера; наследуется из r1 |
CI-провенанс Reviewed artifact (docs screenshots) |
gh run view 32691836025 |
run зелёный (workflow_dispatch, job capture), соответствует коммиту a548d4f1 |
| Backend | — | не запускался: дельта не трогает custom_components/**/*.py |
Полный вывод smoke-select.mjs:
Изменено файлов src/**: 1 · символов проекта на изменённых строках: 4
Матрица: 177 смоков · порог «широкого» символа: больше 35 смоков
Прямое совпадение (6):
demo/smoke_pan_any_zoom.mjs ← _rszDrag
demo/smoke_resize_audit_1550.mjs ← _rszDrag
demo/smoke_resize_inner_dimensions.mjs ← _rszDrag
demo/smoke_resize_virtual_thick.mjs ← _rszDrag
demo/smoke_resize_wall_thickness.mjs ← _rszDrag
demo/smoke_room_resize.mjs ← _rszDrag
Выборка дополняет AC задачи и суждение ревьюера, а не заменяет их.
Полный прогон матрицы остаётся предрелизной обязанностью на точном SHA.
Решение по строке: прогнать все 6 — узкий диапазон, символ (_rszDrag)
прямо задействован в новом коде (this._rszDrag?.snap || renderSnapshot || this._rszSnapshot()), «слабых» связей нет. Полная матрица (177) не
прогонялась — предрелизный гейт (§8), не гейт этого ревью.
Находки
Low (снимаю с записью, не блокирует)
L1 (процессная, повтор паттерна из SPEC-REVIEW-277-r2/L2). Вердикт-комментарий
CODE-REVIEW-277-r1 в issue не называет SHA, на котором получен зелёный/жёлтый
результат — SHA есть только внутри полного документа
(docs/reviews/CODE-REVIEW-277-r1.md, строка «SHA ревью»). Восстановлено без
труда (сам документ уже закоммичен и однозначен, гадать по времени коммитов не
потребовалось), не стоило дополнительного цикла. Не блокирует: тот же паттерн
уже был отмечен и снят с записью на стадии spec этой же задачи; здесь —
подтверждение, что практика не изменилась на стадии code. Рекомендация на
будущее (не мандат ревьюера): переносить строку «SHA ревью» из документа в
терсовый комментарий issue, раз она уже вычисляется.
Находка предыдущего раунда (M1) закрыта полностью — см. раздел ниже. Новых
находок в продуктовом коде дельты (src/houseplan-card.ts, тест, бенчмарк,
docs) не обнаружено.
Закрытие r1
| Находка r1 | Чем закрыта | Где это видно |
|---|---|---|
M1 (Medium): eligibility-кэш ручек Resize пересчитывал полный JSON.stringify(_geometrySnapshot()) для каждой ручки на каждый рендер, а не один раз на изменение геометрии — противоречило §13 ТЗ и ARCHITECTURE.md |
Коммит 9c5b66c3: _renderResizeLayer() теперь вычисляет renderSnapshot один раз перед циклом по ручкам (const renderSnapshot = this._rszDrag?.snap || this._rszSnapshot();) и передаёт его в _rszResolution(roomId, edge, renderSnapshot); _rszResolution использует переданный снапшот вместо повторного вызова _rszSnapshot(), приоритет this._rszDrag?.snap во время активного жеста сохранён без изменений |
src/houseplan-card.ts:8332-8339 (сигнатура и тело _rszResolution) и :8705-8716 (renderSnapshot вычислен один раз, передан в цикле). Численно: новый npm run benchmark:safe-resize-render даёт snapshotCalls: 20 = 1 на кадр (было бы 1600 = 80 на кадр на старом коде — проверено мной откатом фикса, см. «Как проверялось»); render p95 упал с воспроизведённых в r1 130–224 мс до 1.2–1.8 мс на той же large-house фикстуре (20 комнат/80 ручек) |
L1 (Low, r1): мёртвый _rszSel/устаревший комментарий про удалённую угловую рамку |
Не затронуто этой дельтой — было явно снято с записью в r1 без обязательства править («безопасный кандидат на мелкую уборку», не AC, не блокирует); дельта r2 не касается _rszSel |
src/houseplan-card.ts:7386-7389 не менялся между 241821c4 и HEAD (не входит в diffstat дельты) |
Унаследовано из r1
Без повторной проверки принято из docs/reviews/CODE-REVIEW-277-r1.md
(SHA 241821c40e9b2f43b79945af3377fc3b3f4ac084), так как дельта их не
задевает:
- AC1–AC12, AC14–AC17 — доказаны в r1 автотестами (мутанты 1/1) и живым
pointer-жестом через production-бандл; ни
src/resize.ts, ни safety-порядок причин disabled, ни i18n, ни persist/undo-путь этой дельтой не тронуты. - Полный
golden:verify80/80 из r1 (в этом раунде перепрогнан целиком и дал 86/86 — расширение числа сцен между раундами не относится к дельте #277, само по себе зелёное). - Mutation-gate: 5 новых мутантов
safe-resize-*(r1, дорогой прогон 1/1 каждый) — geometry-функции, которые они бьют, не изменены дельтой r2; не перепрогонялись. Baseline-Reviewed/провенанс golden-baselines коммита241821c4— проверено в r1 черезgh run view --log, не переоценивается.- Риск-таблица §12.1 ТЗ (порядок #276→#277→#278, неучастие #264) — состояние задач не проверялось повторно в этом раунде (продуктовый вопрос, не затронутый дельтой).
- «Одно число — один источник» (
_rszEdgeLabels()берёт данные из того жеres.polys, что коммитится) — код длин/площади не тронут дельтой.
Что проверено и корректно
- Фикс M1 корректен по построению: во время активного драга приоритет
this._rszDrag.snapне изменился (переданныйrenderSnapshotиспользуется только как второй по приоритету источник, после drag-снапшота и до дорогого пересчёта) — поведение при перетаскивании ручки идентично дореформенному, разница только в числе вызовов_rszSnapshot()вне драга. Подтверждено чтением обоих условийsnap = this._rszDrag?.snap || renderSnapshot || this._rszSnapshot()и не найдено ветки, где старое и новое поведение расходятся. - Регрессия из воспроизведения M1 численно устранена на порядки (1.2–1.8 мс против 130–224 мс), а не просто «вошла в бюджет» — проверено собственным контрольным откатом фикса (мутационная проверка вручную, см. таблицу гейтов), а не только заявлением коммита.
- Новый unit-тест (
test/resize-production-path.test.mjs) умеет падать: он требует ровно строку сrenderSnapshotв обоих местах и явно запрещает старую двухаргументную форму вызова — при откате фикса тест обязан упасть (регэксп_rszResolution\(r\.id, i\);совпал бы со старым кодом). - Оба коммита несут
Issue: #277иUser-Visible: noкорректно: изменение не меняет наблюдаемое поведение (то же дерево ручек, то же disabled- состояние, та же геометрия) — только скорость вычисления уже существующего кэша: правки в changelog не требуются и не сделаны. docs/RESIZE.md/docs/TESTING.mdдополнены двумя точными строками про новый бенчмарк — согласуются с реальным поведением скрипта (бюджет 25 мс, один снапшот на кадр), не расходятся с кодом.- Обновление скриншотов (
a548d4f1) — ожидаемое следствие: source-fingerprint документации считается по всемуsrc/**(см. правило проекта), поэтому правка9c5b66c3сделала прежние скриншоты устаревшими;Reviewed artifact-ссылка проверена по факту (gh run view), не только по формату.
Чего не проверял
- Полный набор
demo/smoke_*.mjs(177 файлов) — предрелизный гейт (§8); прогнаны только 6 «прямых совпадений» дельты. python -m pytest tests_backend— дельта не трогаетcustom_components/**/*.py.- Model invariants (
npm test/scripts/model-invariants.mjsна конкретной конфигурации) — геометрия не тронута этой дельтой, наследуется из r1 без повторного прогона. - Мутанты mutation-gate (все ~60, включая 5
safe-resize-*) — не перепрогонялись; дельта не касается функций, которые они бьют. - AC1–AC12, AC14–AC17, риск-таблица §12.1, i18n — не переоценивались, см. «Унаследовано из r1».
- Попиксельный ручной пересмотр всех 86 golden-сцен и двух пересобранных
скриншотов документации — доверился отчётам
golden:verify/check-docsи зелёному CI-runcapture, отдельного визуального ревью пикселей не делал.
Вердикт
Зелёный: единственная Medium-находка r1 (M1) закрыта и проверена численно (включая контрольный откат фикса, а не только чтением диффа), новых находок в продуктовом коде дельты нет. Одна процессная Low (SHA не назван в терсовом вердикте issue) — снята с записью, не блокирует, повторяет уже принятый на стадии spec паттерн.