mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-07 23:19:14 +00:00
@@ -0,0 +1,194 @@
|
||||
# CODE-REVIEW-778-r1
|
||||
|
||||
Материал: `68fc5d1f4b3570bedf6955fbb66612b99c7c4d55` (HEAD, `origin/dev..HEAD` = один коммит).
|
||||
Трек: show. Заход r1. Блокирующих циклов израсходовано 0 из 2 (до этого вердикта).
|
||||
|
||||
## Скоуп
|
||||
|
||||
Задача #778 — инфраструктурное расследование нестабильного превышения
|
||||
`longTask.editorSeries.maxSingleMs` (163 мс против потолка 150 мс на одном
|
||||
Validate-прогоне #762). Предложенная автором работа (тело issue, раздел
|
||||
«Предлагаемая работа») — три пункта:
|
||||
|
||||
1. снять сопоставимую base/candidate серию, явно сохранить SHA/окружение/
|
||||
распределение, отличить шум раннера от реальной синхронной работы;
|
||||
2. разобрать длинную задачу ресайза на фазы (preflight, публикация, DOM/paint,
|
||||
GC), проверить, что окно и календарь событий воспроизводят ввод пользователя;
|
||||
3. если найден дефект измерения — исправить его с отрицательным свидетелем;
|
||||
если продуктовая задержка — вынести конкретную оптимизацию в продуктовую
|
||||
задачу. Не убирать из окна реальную работу, не поднимать лимиты.
|
||||
|
||||
Диапазон правок — только класс B (`demo/**`, `test/**`), `src/**` не тронут:
|
||||
`demo/benchmark_large_house.mjs`, `demo/performance/resize-attribution.mjs`
|
||||
(новый), `demo/performance/card-contract.mjs`, `demo/performance/README.md`,
|
||||
`test/performance-contract.test.mjs`, `test/performance-resize-attribution.test.mjs`
|
||||
(новый). Трейлеры коммита: `Issue: #778`, `User-Visible: no` — корректно для
|
||||
правки только харнесса измерения, без изменения продукта или бюджетов.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
- Прочитан диапазон `git diff origin/dev...HEAD` целиком (6 файлов, 423/−2).
|
||||
- Прочитано тело issue #778 и оба комментария автора (взятие в работу,
|
||||
итоговый комментарий с выводом и списком изменений).
|
||||
- Прочитан `docs/SCOPE.md`, `AGENTS.md`, `docs/process/REVIEWER.md`.
|
||||
- Код `attributeResizeLongTask` разобран вручную построчно и пересчитан на
|
||||
всех трёх позитивных юнит-кейсах теста (первый CI-образец 135 мс, задача без
|
||||
шага ресайза, задержка вне тайм-вызовов) — результаты функции совпали с
|
||||
ручным пересчётом по формулам модуля, расхождений не найдено.
|
||||
- Прочитан `demo/benchmark_large_house.mjs` вокруг `startResizeAttribution`
|
||||
(строки ~884–978): подтверждено, что `card._resize` уже существует к моменту
|
||||
обёртки (создаётся `_rszEdgeDown` до неё), что обёртка ставится и снимается
|
||||
строго вокруг окна, что восстановление `move` учитывает `hasOwnProperty`
|
||||
(не плодит метод на прототипе), и что окно Long Task открывается уже после
|
||||
обёртки (как заявлено в комментарии кода).
|
||||
- Прочитан `card-contract.mjs`: новая ветка `optionalMethodsOf` пропускает
|
||||
отсутствующий `_resize` и отсутствующий `move` целиком (отличает «нет
|
||||
владельца» от «владелец без метода»), красит только метод неправильного
|
||||
типа — соответствует описанию в README и тесту.
|
||||
|
||||
## Гейты: что прогнано и почему
|
||||
|
||||
- **`npx tsc --noEmit`, `npm test` (полный), `npm run build` + сверка бандла —
|
||||
не прогонялись мной.** Validate на этом же SHA `68fc5d1f` зелёный
|
||||
(https://github.com/Matysh/houseplan-card/actions/runs/37550065681) — дешёвые
|
||||
гейты уже подтверждены этим прогоном, повторный прогон ничего не добавил бы.
|
||||
- **Точечно прогнано мной** (дёшево, для собственной уверенности, не замена
|
||||
Validate): `node --test test/performance-resize-attribution.test.mjs
|
||||
test/performance-contract.test.mjs` — 16/16 зелёных.
|
||||
- **Негативный свидетель проверен лично.** Внёс ручную мутацию в
|
||||
`demo/performance/resize-attribution.mjs` (`task.duration += 50` после
|
||||
`if (!task) return empty;`), прогнал
|
||||
`test/performance-resize-attribution.test.mjs` — 3 из 4 тестов покраснели
|
||||
(`parts add up`, «без шага ресайза», «задержка вне тайм-вызовов»; тест на
|
||||
`supported:false` не затронут мутацией — ожидаемо). Файл восстановлен из
|
||||
копии, `git status` чист. Это и есть требуемое «тест умеет падать» для
|
||||
AC этой задачи — для чистого юнита, без продукта и без дорогого гейта,
|
||||
мутант в `scripts/mutation-registry.mjs` не обязателен (`PROCESS.md` о
|
||||
защитном AC: «для чистых юнитов достаточно отрицательного случая в самом
|
||||
тесте»), и тест действительно содержит отдельные негативные кейсы (задача
|
||||
без шага ресайза, отсутствие Long Task/spans).
|
||||
- **Смоки, golden, pytest, инварианты геометрии, performance (AC-named) — не
|
||||
прогонялись и не нужны.** Изменения не трогают `src/**`, никакой геометрии,
|
||||
рендера или Python-кода; AC задачи не называют конкретный смок. `ci:full`
|
||||
уже стоит на issue — Full Performance увидит новое поле
|
||||
`interactionDiagnostics.resizeLongTask` в отчёте по строке CI, это вне
|
||||
ревью кода (сама задача просит не гонять полные наборы ради ревью).
|
||||
- **Я не прогонял Full Performance и не проверял реальный CI-образец** —
|
||||
это дорогой гейт, задача явно не требует от ревью его повторения; численные
|
||||
утверждения README (202 образца, распределение по раннерам) проверены
|
||||
чтением текста и логики кода, не повторным сбором серии — это диагностика
|
||||
инфраструктурной задачи уровня анализа данных, а не защищаемый продуктом AC.
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium (в скоупе — AC п.3, вторая ветка не закрыта)
|
||||
|
||||
Собственная постановка задачи (issue, «Предлагаемая работа», п.3) требует:
|
||||
*«если продуктовая задержка — вынести конкретную оптимизацию в продуктовую
|
||||
задачу»*. Итоговый комментарий автора называет конкретные продуктовые причины
|
||||
длинной задачи резайза:
|
||||
|
||||
> «`_rszEdgeLabels` → `floorMinusBodies` дважды за шаг заново объединяет тела
|
||||
> этажа, ~24 % задачи; живой preflight `wallBodiesGeometry` — ~70 %, из них
|
||||
> BigNumber polyclip-ts — ~29 % self time»
|
||||
|
||||
— то есть продуктовая задержка **найдена**, а не исключена (вывод «нет
|
||||
дефекта измерения» относится к дефекту *измерения*, не к вопросу о наличии
|
||||
продуктовой задержки вообще). Но ссылка на её разбор — «Продуктовые причины —
|
||||
в документе находок волны» — не резолвится ни в материале ревью (только
|
||||
`demo/**`/`test/**`, никакого нового документа), ни в репозитории: поиск по
|
||||
`wallBodiesGeometry`, `floorMinusBodies`, `polyclip-ts`, `rszEdgeLabels` не
|
||||
находит такого документа ни в `docs/`, ни в `docs/reviews/`, ни в открытых
|
||||
issues (проверены issues, созданные 2026-10-01…2026-10-06, включая #789 —
|
||||
не про это; никакого issue про оптимизацию preflight ресайза после #778 не
|
||||
заведено). Формулировка «находок волны» также не совпадает с принятым в этом
|
||||
репозитории словоупотреблением «волна» (многоэтапные ТЗ вроде #680/#677, не
|
||||
имеющие отношения к #778).
|
||||
|
||||
**Воспроизведение:** `gh issue list --repo Matysh/houseplan-card --search
|
||||
"polyclip OR wallBodiesGeometry OR floorMinusBodies OR rszEdgeLabels" --state
|
||||
all` и `git grep -l "wallBodiesGeometry\|floorMinusBodies\|polyclip-ts" docs/`
|
||||
— ни один результат не указывает на задачу, выносящую найденную продуктовую
|
||||
причину в работу.
|
||||
|
||||
**Почему это Medium, а не Low.** Это невыполненный пункт AC задачи, а не
|
||||
бухгалтерия: трек show прямо называет «невыполненный AC» критерием Medium.
|
||||
Без видимого адресата находка рискует остаться только прозой в закрытом
|
||||
issue — ровно то, против чего написан п.3 ТЗ («вынести… в продуктовую
|
||||
задачу», а не «упомянуть»).
|
||||
|
||||
**Что нужно для зелёного:** либо автор называет SHA/ссылку на уже
|
||||
существующий документ находок (и я его перечитаю), либо заводит отдельное
|
||||
issue с конкретной оптимизацией (`wallBodiesGeometry`/preflight,
|
||||
`floorMinusBodies`/`_rszEdgeLabels`) и ссылкой на #778, либо правит
|
||||
формулировку итогового комментария, если это не было фактическим
|
||||
обязательством (но тогда п.3 ТЗ не закрыт и это нужно явно признать).
|
||||
|
||||
Находка в скоупе #778 — отдельный issue не завожу.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Нет изменений в `src/**`, бюджетах, окнах измерения или лимитах — соответствует
|
||||
запрету «не поднимать лимиты ради зелёного CI» и классу B (не A).
|
||||
- `attributeResizeLongTask` (новый модуль) — чистая функция, независимо
|
||||
пересчитана вручную на всех заявленных кейсах, расхождений с кодом нет.
|
||||
Разделение на `preflightMs`/`projectOtherMs`/`publishMs`/`labelsMs`/
|
||||
`moveOtherMs`/`updateMs`/`otherMs` корректно не задваивает preflight,
|
||||
вложенный в `project`, и не приписывает фазы задаче, в которой нет шага
|
||||
ресайза (`geometryMoves: 0` ветка проверена тестом и пересчётом).
|
||||
- Обёртка `ResizeController.move` в раннере ставится и снимается вокруг ровно
|
||||
одного окна; «неизвестная форма» (`project`/`publish`/`measure` не функции)
|
||||
репортится как `supported:false` с причиной, а не считается нулевыми
|
||||
долями — проверено чтением и тестом `performance-contract.test.mjs`.
|
||||
- `card-contract.mjs`: `optionalMethodsOf` — новый необязательный метод
|
||||
контракта, не ломает прохождение базовой сборки v1.68.1 (`_rszDrag`, без
|
||||
`_resize`) и красит только случай «метод есть, но не функция» — доказано
|
||||
`assert.doesNotThrow`/`assert.throws` в тесте.
|
||||
- Защитные AC (неизвестная форма move; невалидный тип необязательного метода)
|
||||
доказаны негативными кейсами в самом тесте — для чистых юнитов без
|
||||
дорогого гейта этого достаточно, реестр мутантов не требуется.
|
||||
- Трейлеры `Issue: #778` и `User-Visible: no` на коммите корректны: изменение
|
||||
не видно пользователю продукта, changelog не требуется.
|
||||
- `demo/performance/README.md` корректно описывает состав поля
|
||||
`interactionDiagnostics.resizeLongTask` и логику чтения следующего отказа —
|
||||
текст совпадает с кодом (проверено построчно).
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Полные `tsc`/`npm test`/`npm run build`+bundle — не гонял: Validate уже
|
||||
зелёный на этом SHA (см. «Гейты» выше), бюджет раунда не трачу на повтор.
|
||||
- Сами 202 исторических CI-образца (числа из прошлых прогонов Full
|
||||
Performance/Validate, которые пересказывает README) — не пересчитывал
|
||||
заново из артефактов CI; доверяю им как документированному историческому
|
||||
анализу вне материала этого коммита (они не являются кодом, который может
|
||||
«покраснеть»).
|
||||
- Поведение в реальном браузере (Long Animation Frames, Chrome-трассировка,
|
||||
профиль GC) — не воспроизводил; это заявлено автором как ручная локальная
|
||||
диагностика (Chromium 141), не автоматизированная часть диапазона ревью, и
|
||||
honest-метка «проверено чтением, не исполнением» применяется к этой части.
|
||||
- `ci:full` (Full Performance на ветке задачи) — ещё не обязан быть виден на
|
||||
момент этого ревью; его не запускал и не ждал, задача сама пометила issue
|
||||
этой меткой для последующего прогона.
|
||||
- Golden/pytest/инварианты модели — не прогонял: диапазон не трогает рендер,
|
||||
Python-бэкенд или геометрию модели.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Один High отсутствует, один Medium в скоупе (невыполненный пункт AC —
|
||||
эскалация продуктовой причины). Жёлтый вердикт возвращает задачу автору;
|
||||
цикл расходуется (1 из 2 на track:show).
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/778-resize-longtask-attribution`, коммит `68fc5d1f4b35` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `2c52f7e78071c2afaf88614dbe7558783c84c230`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 2c52f7e78071
|
||||
```
|
||||
- Тело issue: `c5a06e8f343baa4956489a535a51db06cfaee2bc8b1e55d26e4996b42021afb5`
|
||||
- Вердикт конвейера: `yellow` · High 0 · маршрут `fix`
|
||||
<!-- hp:usage input_tokens=4455 output_tokens=28736 cache_creation_input_tokens=89820 cache_read_input_tokens=1759388 num_turns=34 -->
|
||||
Reference in New Issue
Block a user