Files
houseplan-card/docs/reviews/CODE-REVIEW-277-r2.md
claude[bot] e68b494555
Validate / process-workflow-sync (push) Successful in 47s
Validate / docs (push) Successful in 52s
Validate / provenance (push) Successful in 45s
Validate / process-gate (push) Failing after 32s
Validate / changes (push) Successful in 32s
Validate / hacs (push) Skipped
Validate / hassfest (push) Skipped
Validate / frontend (push) Skipped
Validate / reuse (push) Successful in 43s
Validate / smoke (1) (push) Skipped
Validate / smoke (2) (push) Skipped
Validate / smoke (3) (push) Skipped
Validate / smoke_done (push) Skipped
Validate / performance_smoke (push) Skipped
Validate / backend (push) Skipped
Validate / golden (push) Skipped
docs: review document for #277
Issue: #277
User-Visible: no
2026-08-24 05:15:13 +00:00

18 KiB
Raw Permalink Blame History

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:verify 80/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-run capture, отдельного визуального ревью пикселей не делал.

Вердикт

Зелёный: единственная Medium-находка r1 (M1) закрыта и проверена численно (включая контрольный откат фикса, а не только чтением диффа), новых находок в продуктовом коде дельты нет. Одна процессная Low (SHA не назван в терсовом вердикте issue) — снята с записью, не блокирует, повторяет уже принятый на стадии spec паттерн.