mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-07 23:19:14 +00:00
@@ -0,0 +1,190 @@
|
||||
# CODE-REVIEW-778-r2
|
||||
|
||||
Материал: `6a9fdee0be45a2cc3a5e7dc556738b08070262b2` (HEAD, `origin/dev..HEAD` = три коммита:
|
||||
`6b8a51a2` test(perf) — повтор r1-коммита после ребейза на слитый #770, `59527ee0`
|
||||
docs — коммит документа ревью r1, `6a9fdee0` docs(perf) — закрытие Medium r1).
|
||||
Трек: show. Заход r2. Блокирующих циклов израсходовано 1 из 2 (до этого вердикта).
|
||||
|
||||
## Скоуп
|
||||
|
||||
Предмет r2 — дельта после жёлтого вердикта r1 (`docs/reviews/CODE-REVIEW-778-r1.md`):
|
||||
единственная находка Medium (п.3 собственного ТЗ — «если продуктовая задержка,
|
||||
вынести конкретную оптимизацию в продуктовую задачу» — не закрыт, ссылка «в
|
||||
документе находок волны» не резолвилась ни в материале, ни в репозитории, ни в
|
||||
issues).
|
||||
|
||||
Закрывающий коммит автора (`6a9fdee0`) меняет один файл:
|
||||
`demo/performance/README.md`, +23/−0, без строки продуктового кода, без
|
||||
бюджетов, без окон измерения. Трейлеры `Issue: #778`, `User-Visible: no` —
|
||||
корректны (`demo/**` — класс B, трейлеры обязательны; «no» корректно: правка
|
||||
не видна пользователю продукта).
|
||||
|
||||
Промежуточный коммит `6b8a51a2` (test(perf): attribute…) — тот же патч, что был
|
||||
материалом r1 (423 вставки/2 удаления, тот же набор из 6 файлов), реплеенный
|
||||
поверх влившегося `#770`: `git show 6b8a51a2 --stat` даёт идентичный диффстат
|
||||
r1-коммита. Автор заявляет конфликт только в импортах
|
||||
`demo/benchmark_large_house.mjs`, обе стороны сохранены — проверено: в шапке
|
||||
файла сейчас присутствуют и `summarizeLongTasks, summarizeTimings` (из `#770`),
|
||||
и `attributeResizeLongTask` (из `#778`), оба блока импортов на месте. Делаю
|
||||
вывод: содержательно это не новый код для ревью, а материал r1, перенесённый
|
||||
через ребейз — ниже инвентаризирую его как «унаследовано», а не разбираю
|
||||
заново.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
- Прочитан `git diff 114c171a...HEAD` (три коммита дельты r2) и `git show
|
||||
6a9fdee0` целиком — единственная содержательная правка этого раунда.
|
||||
- Исходный SHA материала r1 (`68fc5d1f4b35…`) и его дерево (`2c52f7e78071…`) не
|
||||
резолвятся в этом (полном, не shallow) чекауте — ожидаемо: ребейз на `#770`
|
||||
его осиротил, сам r1-документ предупреждал об этом заранее («ребейз его
|
||||
осиротит, и это нормально»). Признаков того, что SHA был мёртв уже в момент
|
||||
публикации r1 (а не стал таким после последующего ребейза), нет — трактую
|
||||
как обычное дело (`docs/process/REVIEWER.md`, «Повторный раунд»), не находка.
|
||||
- Проверены технические утверждения нового абзаца README построчным чтением
|
||||
текущего `src/`:
|
||||
- `_rszEdgeLabels` (`src/houseplan-editor-runtime.ts:3417`) действительно
|
||||
вызывает `geometryArea(floorMinusBodies(floor, physical))` внутри цикла
|
||||
`for (const id of ids)` (`:3475–3493`), где `ids = plan.roomIds` — то есть
|
||||
объединение тел этажа действительно пересчитывается по разу на каждую
|
||||
задействованную комнату (обычно две при резайзе общей стены) — заявление
|
||||
«дважды за шаг» подтверждено чтением, не исполнением.
|
||||
- `_checkSpacePhysicalGeometry` → `_checkSpacePhysicalGeometryImpl`
|
||||
(`:8996`) действительно делегирует в `checkSpacePhysicalGeometry`,
|
||||
передавая `captureWallGeometry` поверх `wallBodiesGeometry` — имена и
|
||||
маршрут вызова совпадают с описанием в README. Доли времени (70 %/29 %/
|
||||
45 %/14 %) — результат локального CPU-профиля Chrome, не код; как и в r1,
|
||||
эта часть принята как документированная диагностика, не перепроверяется
|
||||
исполнением (отдельно и явно помечена в README как «diagnostic only»).
|
||||
- Имена функций и путь файла (`src/houseplan-editor-runtime.ts`) в тексте
|
||||
README совпадают с реальными объявлениями (`grep` по `_rszEdgeLabels`,
|
||||
`floorMinusBodies`, `_checkSpacePhysicalGeometry`, `wallBodiesGeometry`,
|
||||
`geometryArea`) — не придуманный адрес.
|
||||
- Ссылка «в документе находок волны (F23)» в README и в закрывающем
|
||||
комментарии автора по-прежнему не резолвится в репозитории (внешний
|
||||
документ владельца) — но в отличие от r1 это уже не единственный след
|
||||
находки: конкретные причины (функция, файл, доля времени, путь вызова)
|
||||
теперь зафиксированы в самом репозитории, в каноническом перф-документе
|
||||
подсистемы, а не только в прозе закрытого issue.
|
||||
- `node scripts/smoke-select.mjs --base origin/dev --head HEAD` — «Браузер-смоки
|
||||
этим диффом не выбираются: src/**/*.ts не тронут» — согласуется с тем, что
|
||||
вся дельта (все 3 коммита раунда, не только r2-коммит) не трогает продуктовый
|
||||
код.
|
||||
- `gh run view 37555015347 --json headSha,conclusion` — `headSha` совпадает с
|
||||
HEAD (`6a9fdee0`), `conclusion: success`: дешёвые гейты подтверждены на
|
||||
точном материале этого раунда, не на приближённом SHA.
|
||||
- Точечно прогнано мной: `node --test test/performance-contract.test.mjs
|
||||
test/performance-resize-attribution.test.mjs` — 17/17 зелёных (совпадает с
|
||||
числом, которое называет автор в закрывающем комментарии).
|
||||
|
||||
## Гейты: что прогнано и почему
|
||||
|
||||
- **`npx tsc --noEmit`, `npm test` (полный), `npm run build` + сверка бандла —
|
||||
не прогонялись.** Validate зелёный на точном SHA материала (`6a9fdee0`,
|
||||
прогон `37555015347`, подтверждено `gh run view` выше) — дешёвые гейты уже
|
||||
закрыты, повтор не добавляет доказательств.
|
||||
- **Прогнано мной:** `node --test` по двум тестовым файлам, относящимся к
|
||||
диапазону (17/17); `node scripts/smoke-select.mjs` — подтверждает «нечего
|
||||
выбирать».
|
||||
- **Golden, pytest, инварианты модели, performance (AC-named) — не
|
||||
прогонялись.** Весь диапазон раунда (включая унаследованный `6b8a51a2`) не
|
||||
трогает `src/**`, геометрию, рендер или Python; AC не называет конкретный
|
||||
смок; задача инфраструктурная.
|
||||
- **Негативный свидетель кода харнесса (`attributeResizeLongTask`,
|
||||
`optionalMethodsOf`) не перепроверялся заново** — это содержимое
|
||||
унаследовано из r1 (см. ниже), где он уже был лично подтверждён ручной
|
||||
мутацией; в r2 этот код не менялся.
|
||||
- **Полный `ci:full` / Full Performance — не прогонялся и не нужен ревью**:
|
||||
как и в r1, это дорогой гейт для диагностики, не защищаемый продуктом AC
|
||||
этого коммита.
|
||||
|
||||
## Находки
|
||||
|
||||
Нет ни одной находки High или Medium в r2.
|
||||
|
||||
## Закрытие раунда r1
|
||||
|
||||
| Находка r1 | Чем закрыта | Где это видно |
|
||||
|---|---|---|
|
||||
| Medium: п.3 ТЗ («вынести продуктовую оптимизацию в продуктовую задачу») не закрыт — ссылка «в документе находок волны» не резолвится нигде | Коммит `6a9fdee0`: README получил раздел «Product causes, left for a product task» с двумя конкретными причинами (`_rszEdgeLabels`/`floorMinusBodies`, ~24 %; `_checkSpacePhysicalGeometry`/`wallBodiesGeometry`+polyclip-ts, ~70 %/~29 %), названными файлом, функцией и долей времени; явно признано, что отдельный issue не заведён из-за запрета владельца на новые issue на время волны (сообщение коммита и комментарий закрытия). Причины и адрес проверены чтением текущего `src/` (раздел «Как проверялось» выше) — не вымысел | `demo/performance/README.md`, раздел «Product causes, left for a product task» (между «synthetic 30 ms stall» и «## Private card contract»); коммит `6a9fdee0`, сообщение коммита; комментарий автора от 2026-10-07 01:00:55Z |
|
||||
|
||||
Оцениваю это как достаточное закрытие: буквальное требование АС («завести
|
||||
задачу») временно невыполнимо по внешней причине (директива владельца), автор
|
||||
это не скрывает, а компенсирует тем единственным доступным способом, который
|
||||
не теряет находку — переносит её с точным адресом в код в канонический
|
||||
документ подсистемы, откуда её возьмёт следующая продуктовая задача, и
|
||||
привязывает к записи в процессе триажа владельца (F23, внешний документ, не
|
||||
проверяется, но и не является обязательной частью доказательства — обязательная
|
||||
часть, сами причины с адресом, в репозитории и проверена).
|
||||
|
||||
## Унаследовано из r1
|
||||
|
||||
Без повторной проверки принято содержимое коммита `6b8a51a2` (test(perf):
|
||||
attribute the resize Long Task…) — тот же патч, что был материалом r1
|
||||
(`68fc5d1f4b35`, дерево `2c52f7e78071`, оба не резолвятся после ребейза —
|
||||
ожидаемо, см. «Как проверялось»), реплеенный на `#770` без содержательных
|
||||
изменений (идентичный диффстат, конфликт только в заголовочных импортах
|
||||
`benchmark_large_house.mjs`, обе стороны сохранены — проверено чтением
|
||||
текущего файла):
|
||||
|
||||
- `attributeResizeLongTask` (`demo/performance/resize-attribution.mjs`) —
|
||||
построчный разбор, ручной пересчёт на всех позитивных кейсах, негативный
|
||||
свидетель (ручная мутация красит 3/4 теста) — документ r1, раздел «Как
|
||||
проверялось» и «Находки».
|
||||
- Обёртка `ResizeController.move` в `demo/benchmark_large_house.mjs` —
|
||||
установка/снятие строго вокруг окна, восстановление через
|
||||
`hasOwnProperty` — документ r1, тот же раздел.
|
||||
- `optionalMethodsOf` в `demo/performance/card-contract.mjs` — не ломает
|
||||
базовую сборку без `_resize`, красит только неверный тип метода —
|
||||
документ r1.
|
||||
- Корректность трейлеров `Issue: #778`/`User-Visible: no` на этом коммите —
|
||||
документ r1.
|
||||
|
||||
Основание: дельта раунда — только README; код этого коммита не менялся между
|
||||
r1 и r2, только сменил родителя при ребейзе.
|
||||
|
||||
## Что проверено и корректно (r2)
|
||||
|
||||
- Единственная правка раунда — документация (README), не продуктовый код;
|
||||
класс B, трейлеры корректны.
|
||||
- Технические утверждения нового абзаца (имена функций, файл, кратность
|
||||
вызова union) совпадают с текущим `src/houseplan-editor-runtime.ts` —
|
||||
проверено чтением, не исполнением (доли времени — внешняя диагностика,
|
||||
не код).
|
||||
- Ни бюджеты, ни окна измерения, ни продуктовый код не затронуты ни в одном
|
||||
из трёх коммитов дельты раунда.
|
||||
- Validate зелёный ровно на SHA материала (`gh run view` подтверждает
|
||||
`headSha`).
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Сам Chrome CPU-профиль/трассу, из которых взяты доли 24 %/70 %/29 %/45 %/
|
||||
14 % — не переснимал; README само помечает их как «diagnostic only»,
|
||||
принимаю как документированный анализ, не как код, который может
|
||||
покраснеть (как и в r1 для аналогичных числовых утверждений).
|
||||
- Внешний документ владельца «находок волны (F23)» — не существует в
|
||||
репозитории, не резолвится и не обязан: он не является доказательной базой
|
||||
закрытия, роль играет запись в README, которая проверена.
|
||||
- Полные `tsc`/`npm test`/`npm run build`+bundle — не гонял, Validate зелёный
|
||||
точно на SHA `6a9fdee0`.
|
||||
- Golden/pytest/инварианты модели/Full Performance — не прогонял: диапазон
|
||||
раунда не трогает рендер, Python или геометрию модели живым кодом.
|
||||
|
||||
## Вердикт
|
||||
|
||||
High: 0. Medium: 0 (находка r1 закрыта предъявленным адресом причины в
|
||||
репозитории). Зелёный вердикт, цикла не образует.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/778-resize-longtask-attribution`, коммит `6a9fdee0be45` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `60238c18a0fb0897c6a67f055dc5aa9d9726d626`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 60238c18a0fb
|
||||
```
|
||||
- Тело issue: `c5a06e8f343baa4956489a535a51db06cfaee2bc8b1e55d26e4996b42021afb5`
|
||||
- Вердикт конвейера: `green` · High 0 · маршрут `fix`
|
||||
<!-- hp:usage input_tokens=4356 output_tokens=16497 cache_creation_input_tokens=69816 cache_read_input_tokens=1179579 num_turns=24 -->
|
||||
Reference in New Issue
Block a user