19 KiB
CODE-REVIEW-694-r1
Issue: #694 «Исследовать и устранить performance-регрессии v1.78 относительно v1.77»
Трек: ask · Заход: r1 · блокирующих циклов израсходовано 0 из 4
Материал: git log --oneline origin/dev..HEAD = 1 коммит,
35c89fc67dc8476dde0aab8c7f4e21ecea4c271f
(perf(card): a floor switch stops re-querying the same subtrees (#694)),
рабочая копия на этом SHA. git diff origin/dev...HEAD — 9 файлов,
246 вставок / 20 удалений.
Скоуп
Три точечных правки без изменения видимого поведения (User-Visible: no,
трейлер Issue: #694 на коммите — оба присутствуют и корректны, changelog не
трогается, что и требуется при no):
- П1
src/device-hit-owner.ts— логика пачки записейMutationObserverнаблюдателя hover-указателя вынесена в чистую функциюdeviceLayerMutated: узел проверяется не больше раза за пачку (Set), а после первого совпадения.devlayerпроверки прекращаются. Добавленные узлы по-прежнему безусловно проходятsyncAdded(_syncPointerHoverSubtree). - П2
src/stairs-view.tsrenderLayer— геттер_modelхоста читается один раз в начале рендера вместо одного чтения на каждую лестницу. - П3
src/i18n/language-runtime.tslanguageRenderGate—setAttribute('lang', …)вызывается только когда значение отличается от текущего (getAttribute).
Работа закрывает J1/J6-смежный сценарий из docs/SCOPE.md («живой план»,
отзывчивость при частом действии — переключении этажей на планшете/кионе);
правки эксплуатационные, не меняют поверхность продукта, доп. вопросов к
docs/USER-GUIDE.ru.md нет — текста интерфейса они не касаются.
Материал: проверка целостности SHA
git log origin/dev..HEAD содержит ровно 1 коммит, совпадающий с
REVIEWER-инструкцией (35c89fc6, рабочая копия уже на нём). Коммит несёт
Issue: #694, User-Visible: no, Co-Authored-By. Мутанты и AC-текст в
issue ссылаются на более ранний SHA ветки — d6ad81120fef23822715cd396976a533ae980401
(из комментария «Сделано»). Это не нарушение #312/#499: d6ad8112 — тот же
логический коммит до pre-review rebase на ушедший вперёд dev (между ними
легли №724, №732, №733 и чисто докс-коммиты, не конфликтующие с файлами
#694). Проверено не на слово автора, а сравнением через GitHub API
(compare/d6ad8112...35c89fc6): патчи src/device-hit-owner.ts,
src/stairs-view.ts, src/i18n/language-runtime.ts,
test/device-hit-owner.test.mjs, test/i18n-runtime.test.mjs,
test/stairs.test.mjs, tsconfig.test.json и добавленные в
scripts/mutation-registry.mjs три записи #694 … — побайтово идентичны в
обоих коммитах. Единственная разница в src/houseplan-card.ts и остальных
записях mutation-registry.mjs между d6ad8112 и 35c89fc6 — не связанный
со стеком #694 рефакторинг оверлеев #724/#732 (wallSilhouettes →
structure), пришедший с dev при ребейзе. Вывод: доказательства AC1–AC5,
собранные на d6ad8112, валидны и для текущего материала 35c89fc6 — код
трёх правок и их тестов не менялся.
Как проверялось
Дёшевые гейты. Validate 35c89fc6 (run 36794107188)
— зелёный только для джоба «Фронтенд: типы, юниты, мутанты, синхрон
бандла» (typecheck + npm test + сверка бандла). Проверено по факту через
gh api .../jobs, а не на слово: джобы «Мутанты по диффу», «Смоки в
браузере», «Golden-кадры», «Бэкенд: pytest», «Перф-смок» в этом прогоне —
skipped, не success. Поэтому npx tsc --noEmit/npm test/npm run build
не перегонял (сошлись по инструкции), а смоки и остальное — ниже, своими
руками, потому что Validate их не покрыл.
AC1 (unit). Прочитан deviceLayerMutated и тест test/device-hit-owner.test.mjs
(#694 AC1 …, 3 теста). Логический разбор эквивалентности: исходный код
пересчитывал inDeviceLayer заново на каждую запись, но ответ детерминирован
текущим DOM на момент колбэка (все синхронные мутации уже произошли до того,
как наблюдатель получает управление), поэтому кэширование через Set в
границах одной пачки не меняет результат — при условии, что syncAdded
внутри пачки не меняет структуру поддерева. Проверил: _syncPointerHoverSubtree
(src/houseplan-card.ts:7120) только переключает атрибут
data-pointer-hover, структуру DOM не трогает — условие выполнено, кэш
безопасен. Тест #694 AC1 a batch into one subtree queries each node at most once жёстко проверяет счётчик (stage.queries === 2, не больше) —
разрушающая мутация в реестре (id: pointer-hover-batch-requeries-shared-subtree, убирает checked.has(node))
эту проверку красит: без мемоизации тот же узел встретится как target
нескольких записей и будет опрошен повторно, счётчик вырастет. Мутант не
гонял (правило §10.4/#709 — ревьюер мутанты не применяет), но прочитал
реестр и тест и убедился, что разрушение действительно ломает конкретное
числовое утверждение теста — не косвенно, а напрямую. Проверено чтением, не
исполнением мутанта.
AC2 (unit). stairs-view.ts renderLayer читает this.owner._model
один раз в локальную const model (строка 66) и переиспользует её и для
spaceIds, и для targetTitle. Тест test/stairs.test.mjs (#694 AC2)
гонит StairViewRuntime(host).renderLayer() при 0/1/2/7/40 лестницах и
проверяет host.reads === 1 через spy-геттер — мутант
stairs-view-reads-model-per-stair возвращает второе чтение через
this.owner._model.find(...), что при ≥1 лестнице подняло бы reads до 2 и
уронило утверждение. Подписи этажа (targetTitle) и tooltip-условия
(active) не меняются — тест это же и проверяет построчно по сценариям
upper/attic/ground/null/gone.
AC3 (unit). languageRenderGate теперь делает
if (host.getAttribute?.('lang') !== lang) host.setAttribute(...).
Опциональная цепочка — у хоста без getAttribute (как старый FakeHost в
уже существующих тестах) undefined !== lang всегда истинно, то есть
поведение для таких вызывающих не меняется — запись как раньше на каждом
рендере; регрессии существующих тестов нет (и это подтверждено зелёным
npm test на Validate). Реальные хосты (houseplan-card.ts, space-card.ts,
space-editor.ts, editor.ts) — настоящие DOM-элементы, getAttribute
есть всегда. Новый тест #694 AC3 (test/i18n-runtime.test.mjs) гоняет
повторный рендер одного языка (langWrites остаётся ['en']), смену языка,
английский fallback и смену языка после внешнего искажения атрибута —
мутант language-gate-rewrites-unchanged-lang убирает условие, тест на
точной последовательности langWrites это ловит.
AC4 (perf). Full Performance на d6ad8112 (run
36790354931,
создан 2026-09-30, head_sha подтверждён через gh api): все 9 джобов
зелёные. Цифры в issue (isometric longTask.countP95 13 vs порог 20,
isometric-stage3 24 vs 30, plan-snap 14 vs 24.3, остальные метрики с
запасом) соответствуют заявленному. Так как содержимое трёх правок
идентично между d6ad8112 и материалом (см. раздел выше), повторный полный
прогон (часы CI-времени, не «дешёвый» гейт) не требуется — его смысла
дельта не меняет. Исключение AC4 (атрибуция к лестницам) не понадобилось,
все профили прошли бюджет без него.
AC5 (smoke + golden) — прогнано лично, Validate это пропустил.
Собрал бандл (npm run bundle:sync, затем npm run bundle:clean после
проверки, чтобы не оставить дифф в dist/) и прогнал:
demo/smoke_stairs.mjs— OK, 37/37 проверок;demo/smoke_device_hit_capsules.mjs— OK, 7/7;demo/smoke_french_locale.mjs— OK, 5/5, включаяlangAttrIsFr: true(атрибутlangвиден корректным и при записи-по-изменению).
node scripts/smoke-select.mjs --base a49f7095 --head HEAD: «прямых
совпадений» и «зарегистрированных связей» нет; «слабая связь» — 24 смока по
символу _model (общий, не новый контракт — _model не меняется по
значению, меняется только частота чтения в stairs-view.ts), решение
ревьюера — не гонять весь список, а точечно проверить два смока, прямо
завязанных на переключение этажей/поведение _model при навигации:
demo/smoke_fixed_floor.mjs (OK, 14/14) и demo/smoke_cold_view_toggle.mjs
(OK, 12/12). Остальные 22 — не гонял: правка не меняет, что возвращает
_model, только сколько раз его читают за рендер, риск для их сценариев
отсутствует. «Визуальный минимум» (8 смоков) в выдаче smoke-select помечен
инструментом как часть gate:small -- --smokes (#690); не перегонял
отдельно — это тот же набор, что покрывает Validate-эквивалент gate:small,
а изменение не трогает ни один из восьми (markup/CSS-слоёв layout, grid,
decor-order и т.д. в диффе нет).
golden:verify не гонял: метки ci:golden на issue нет, а диффу нечего
регенерировать — ни один из трёх файлов не меняет шаблон/вёрстку/пиксели
(П1 — JS-логика обработчика, П2 — порядок чтения геттера, П3 — условие
записи атрибута при той же итоговой строке). python -m pytest tests_backend не гонял: диффа в custom_components/**/*.py нет.
npm run invariants не гонял: диффа в геометрии (space-geometry,
wall-thickness и т.п.) нет.
Мутанты. Три новых записи в scripts/mutation-registry.mjs
(pointer-hover-batch-requeries-shared-subtree,
stairs-view-reads-model-per-stair, language-gate-rewrites-unchanged-lang),
guard-команды нацелены точно на #694 AC1/AC2/AC3. node scripts/mutation-registry-check.mjs — без вывода (реестр структурно
валиден). Мутанты не применял (§10.4/#709 — ловлю мутантов на любом треке
проверяет ночной прогон, не ревью); для каждого прочитал патч и соответствующий
тест и убедился разбором, что патч ломает конкретное числовое/последовательное
утверждение теста — не общую зелёность, а именно то число, которое AC
называет.
Находки
Нет. High: 0. Medium: 0. Low: 0.
Риски, названные автором в ТЗ, закрыты кодом:
core-file-budgetнаsrc/houseplan-card.ts— файл не вырос, а сжался (12874 строки сейчас; диапазон #694 убрал в этом файле больше строк, чем добавил — логика вынесена вdevice-hit-owner.ts).- Этаж с лестницами выше порога — не реализовалось, AC4 прошёл без исключения.
- П1 пропускает узел хотя бы раз — опровергнуто и разбором (кэш безопасен,
syncAddedне меняет структуру), и тестом (syncedсодержит все добавленные узлы в порядке записей, включая узлы после первого хита). - П3
langне восстановится, пока язык не сменится — поведение подтверждено тестом (a foreign value is correctedтолько на смене языка) и соответствует явно принятому предположению в ТЗ.
Что проверено и корректно
- Трейлеры (
Issue: #694,User-Visible: no) на единственном коммите диапазона — присутствуют, changelog не тронут, что и требуется приno. tsconfig.test.jsonдобавляетsrc/stairs-view.tsв проверяемый диапазон — согласовано с новым тестом, который импортирует собранныйstairs-view.jsизtest-build/.- Эквивалентность поведения П1–П3 старому коду доказана по каждому пункту выше (AC1–AC3), не только наличием теста, но и прочтением логики на предмет того, что тест действительно падает на внесённой мутации.
- AC4 (перформанс) и AC5 (смоки) — выполнены, перф подтверждён на побайтово идентичном материале, смоки прогнаны лично на текущем SHA.
- Scope корректен: ровно П1–П3, никакого попутного рефакторинга или расширения (архитектура DOM лестниц и прочие кандидаты профилирования явно вынесены в «Не входит» / #725 и не затронуты).
Чего не проверял
- Полный матричный прогон смоков (283 позиции) — не гейт ревью, предрелизная обязанность; прогнаны только 3 названных в AC5 плюс 2 точечных из «слабой связи».
golden:verify— нет меткиci:golden, диффу нечего регенерировать (аргументация выше).python -m pytest tests_backend,npm run invariants— неприменимо, диффа в соответствующих поверхностях нет.- Повторный Full Performance прогон на точном SHA
35c89fc6— не гонял; вместо этого подтвердил байт-в-байт идентичность содержимого трёх правок между SHA прогона (d6ad8112) и материалом (35c89fc6) через GitHub compare API, так что имеющийся зелёный прогон остаётся доказательством. - Мутанты из реестра — не применял (правило: ревьюер мутанты не гоняет ни на каком треке, ловлю проверяет ночной прогон, #709); проверил разбором кода, что каждый из трёх ломает конкретное утверждение соответствующего теста.
- HA E2E / Hassfest / HACS — не применимо, Python/манифесты не менялись.
Вердикт
Зелёный. AC1–AC5 выполнены и доказаны (автотестами с подтверждённой способностью падать на зарегистрированных мутантах — разбором, не исполнением; плюс перф и смоки). Находок нет.
Материал раунда
- Ветка:
issue/694-floor-switch-cost, коммит35c89fc67dc8— ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. - Дерево материала:
0df6abb0a936a66d2e00d73acdc28bf53363a50cgit log --all --format='%H %T' | grep 0df6abb0a936 - Тело issue:
02419f4111e3d31c9b6850dd135fcd78c7d57892ba54c752a6088ccb070a5a05 - Вердикт конвейера:
green· High 0