mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-01 12:18:51 +00:00
@@ -0,0 +1,233 @@
|
||||
# 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.ts` `renderLayer` — геттер `_model` хоста читается
|
||||
один раз в начале рендера вместо одного чтения на каждую лестницу.
|
||||
- **П3** `src/i18n/language-runtime.ts` `languageRenderGate` — `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](https://github.com/Matysh/houseplan-card/actions/runs/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](https://github.com/Matysh/houseplan-card/actions/runs/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.
|
||||
|
||||
Риски, названные автором в ТЗ, закрыты кодом:
|
||||
1. `core-file-budget` на `src/houseplan-card.ts` — файл не вырос, а сжался
|
||||
(12874 строки сейчас; диапазон #694 убрал в этом файле больше строк, чем
|
||||
добавил — логика вынесена в `device-hit-owner.ts`).
|
||||
2. Этаж с лестницами выше порога — не реализовалось, AC4 прошёл без
|
||||
исключения.
|
||||
3. П1 пропускает узел хотя бы раз — опровергнуто и разбором (кэш безопасен,
|
||||
`syncAdded` не меняет структуру), и тестом (`synced` содержит все
|
||||
добавленные узлы в порядке записей, включая узлы после первого хита).
|
||||
4. П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 выполнены и доказаны (автотестами с подтверждённой
|
||||
способностью падать на зарегистрированных мутантах — разбором, не
|
||||
исполнением; плюс перф и смоки). Находок нет.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/694-floor-switch-cost`, коммит `35c89fc67dc8` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `0df6abb0a936a66d2e00d73acdc28bf53363a50c`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 0df6abb0a936
|
||||
```
|
||||
- Тело issue: `02419f4111e3d31c9b6850dd135fcd78c7d57892ba54c752a6088ccb070a5a05`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user