mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -0,0 +1,170 @@
|
||||
# CODE-REVIEW · issue #637 · заход r1
|
||||
|
||||
Материал: `966928135b6a40fd6a73c7631bd514a79015d48a` (единственный коммит поверх
|
||||
`dev`=`ee0f97c8`). Класс B: `scripts/process-metrics.mjs`,
|
||||
`test/process-metrics.test.mjs`, `.github/workflows/process-metrics.yml`,
|
||||
`scripts/mutation-registry.mjs`. Файлов класса A и документов — нет.
|
||||
Трейлеры: `Issue: #637`, `User-Visible: no` — верно, видимого поведения
|
||||
продукта изменение не касается, правки changelog не требуются.
|
||||
|
||||
## Скоуп
|
||||
|
||||
Разовый инструмент вместо ручного сбора метрик процесса (аудит 22.09):
|
||||
чистые функции над снимком GitHub (issue + таймлайн меток, прогоны Actions,
|
||||
имена документов ревью) → Markdown-отчёт; тонкий CLI на `gh`; workflow —
|
||||
понедельник 05:00 UTC + ручной запуск, только чтение, отчёт в step summary
|
||||
и артефакт. Это инфраструктура процесса, не продуктовая фича — к
|
||||
`docs/SCOPE.md` (Core user jobs) не апеллирует, что для класса B корректно.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
- Полное чтение `scripts/process-metrics.mjs`, `test/process-metrics.test.mjs`,
|
||||
`.github/workflows/process-metrics.yml`, диффа `scripts/mutation-registry.mjs`.
|
||||
- `node --test test/process-metrics.test.mjs` — 7 pass (см. ниже, «Гейты»).
|
||||
- Оба новых мутанта применены вручную к рабочей копии и прогнаны под своим
|
||||
guard-тестом: оба ловятся (тест падает), рабочая копия возвращена в
|
||||
исходное состояние (`git status --porcelain` пуст после отката).
|
||||
- Точечный ручной прогон `buildReport` на синтетическом таймлайне с
|
||||
быстрыми (< 3 мин) переходами — искал регресс в вычислении медиан.
|
||||
- Сверка трейлеров коммита и путей файлов с хендофф-комментарием
|
||||
(§2.10 material/blobs совпадают с `git show`).
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium (в скоупе задачи) — ложный «—» вместо настоящего «0 ч» в медианах
|
||||
|
||||
`scripts/process-metrics.mjs:162-165` (`buildReport`):
|
||||
|
||||
```js
|
||||
medianLeadToS7Hours: hours(median(completed.map((i) => i.leadToS7Ms)) ?? NaN) || null,
|
||||
medianReviewToMergeHours: hours(median(completed.map((i) => i.reviewToMergeMs)) ?? NaN) || null,
|
||||
medianLeadToS8Hours: hours(median(completed.map((i) => i.leadToS8Ms)) ?? NaN) || null,
|
||||
medianSpecLeadHours: hours(median(completed.map((i) => i.specLeadMs)) ?? NaN) || null,
|
||||
```
|
||||
|
||||
`??` корректно отличает «нет данных» (`median` вернул `null`) от валидного
|
||||
результата, но финальное `|| null` — нет: если медиана и правда равна 0 ч
|
||||
(переход занял меньше ≈3 минут, что при автоматической простановке меток
|
||||
конвейером — обычное дело для быстрых/тривиальных задач), `hours(0)` даёт
|
||||
`0`, а `0 || null` схлопывает его в `null`. В отчёте (и в Markdown, и в
|
||||
JSON-артефакте) настоящий «0 ч» неотличим от «данных нет» — оба рендерятся
|
||||
как «—».
|
||||
|
||||
**Воспроизведение** (не тестовый файл, ручной прогон в песочнице ревью):
|
||||
|
||||
```js
|
||||
const T = (h) => new Date(Date.UTC(2026,8,15,0,Math.round(h*60))).toISOString();
|
||||
const issues = [{number:1, title:'t', closed_at:T(1), state_reason:'completed'}];
|
||||
const timelines = new Map([[1,[
|
||||
{event:'labeled', label:{name:'S1-new'}, created_at:T(0)},
|
||||
{event:'labeled', label:{name:'S7-code-review'}, created_at:T(0.01)},
|
||||
{event:'labeled', label:{name:'S8-merged'}, created_at:T(0.02)},
|
||||
]]]);
|
||||
buildReport({since:T(0), until:T(1), issues, timelines, reviewFiles:[], runs:[]}).issues.medianLeadToS7Hours
|
||||
// → null (ожидалось 0)
|
||||
```
|
||||
|
||||
Существующие тесты этот путь не задевают: во всех фикстурах (`buildReport`,
|
||||
аудит-фикстура) переходы занимают часы, медиана никогда не равна 0.
|
||||
|
||||
Почему это релевантно именно здесь: единственная цель инструмента — «без
|
||||
регулярного замера решения об ускорении (#620, #636) нельзя проверить»
|
||||
(текст issue). Если ускорение конвейера доходит до долей часа, самый
|
||||
успешный случай — самый быстрый — окажется замаскирован под «нет данных»,
|
||||
то есть баг бьёт ровно по метрике, ради которой скрипт написан. Это не
|
||||
крайний случай на бумаге: автоматическая простановка меток конвейером
|
||||
(#636) делает переходы короче секунд для части событий уже сегодня.
|
||||
|
||||
В скоупе задачи (правка внутри тех же чистых функций, что уже покрыты
|
||||
тестами), High-находок нет → жёлтый вердикт, возврат автору.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- `issueMetrics`: вход по первой статусной метке (не любой), различение
|
||||
`entered`/`s4`/`s5`/`s7`/`s8`, счётчик повторных постановок S7/S4 отдельно
|
||||
от времени первой — тест воспроизводит ровно кейс с `unlabeled`+повтором,
|
||||
прочитан и логика соответствует.
|
||||
- `reviewRounds`: регэксп `^(CODE|SPEC)-REVIEW-(\d+)-r(\d+)\.md$` строгий,
|
||||
не матчит `README.md`/`CODE-REVIEW-issue-5.md` — верно по тесту.
|
||||
- `runMetrics`: прогоны конвейера (`display_title` вида `process #NN · …`)
|
||||
сведены в одну строку `process`, остальные — по имени workflow; отменённый
|
||||
прогон без длительности не учитывается в wall-time — прочитано и
|
||||
соответствует правке, которая решает проблему из хендоффа («имена
|
||||
прогонов конвейера — на каждое событие метки, не сотни строк»).
|
||||
Здесь та же группировка использована для оценки инструмента:
|
||||
ложноположительного объединения разных workflow с совпадающим префиксом
|
||||
не нашёл, `process #\d+ · ` — достаточно специфичный якорь для этого
|
||||
репозитория.
|
||||
- `pipelineMetrics`: skipped-прогоны не считаются — подтверждено мутантом
|
||||
`metrics-count-skipped-pipeline-runs` (тест падает при снятии условия,
|
||||
проверено запуском).
|
||||
- `jobMinutes`: доля «Мутанты» по имени job, сломанные (`started_at: 'x'`)
|
||||
записи не портят сумму — по тесту и чтению кода.
|
||||
- `buildReport`/`renderMarkdown`: раунды считаются по документам, а не по
|
||||
событиям S7-метки — подтверждено мутантом
|
||||
`metrics-rounds-by-s7-events` (тест падает при подмене источника,
|
||||
проверено запуском). Причина в хендоффе (конвейер #636 переставляет S7
|
||||
сам) соответствует коду.
|
||||
- `fetchSnapshot`: только чтение (`gh api` на issues/timeline/actions/runs
|
||||
+ `git ls-tree`), пагинация с разумными пределами (5×100 issue, 3×100
|
||||
событий таймлайна на issue, 15×100 прогонов); фильтр по `closed_at` внутри
|
||||
окна — корректен, поскольку `since` в GitHub Issues API фильтрует по
|
||||
`updated_at`, а закрытие всегда обновляет `updated_at`, так что ни один
|
||||
закрытый в окне issue не потеряется на этом шаге.
|
||||
- `.github/workflows/process-metrics.yml`: `permissions` — только
|
||||
`contents/actions/issues: read`, нет `issues: write`; код и `ref: dev` —
|
||||
согласовано с хендоффом (расписание сработает после зеркалирования в
|
||||
`main`, что явно оставлено как задача владельцу после мержа); экшены
|
||||
запиненны по SHA. Тест `test/process-metrics.test.mjs:108` проверяет это
|
||||
структурно (регэкспы по тексту workflow) — само по себе слабое
|
||||
доказательство, но здесь избыточно: я прочитал файл и подтвердил то же
|
||||
глазами.
|
||||
- Трейлеры коммита (`Issue: #637`, `User-Visible: no`) и заявленные в
|
||||
§2.10-блоке SHA/blob-хэши соответствуют `git show`.
|
||||
|
||||
## Гейты
|
||||
|
||||
- `node --test test/process-metrics.test.mjs` — прогнал сам: 7 pass, 0 fail.
|
||||
- `npx tsc --noEmit`, `npm test` (полный), `npm run build` — не гонял:
|
||||
Validate на этом самом SHA (`96692813`) уже зелёный
|
||||
(https://github.com/Matysh/houseplan-card/actions/runs/35828853731),
|
||||
дифф с тех пор не менялся.
|
||||
- `node scripts/check-docs.mjs` — не требуется: диф не трогает `src/**`
|
||||
(только `scripts/`, `test/`, `.github/workflows/`).
|
||||
- `npm run invariants` — не требуется: диф не касается геометрии, `layout`,
|
||||
толщины стен, `marker.space`/`open_spans`.
|
||||
- `demo/smoke_*.mjs`, `npm run golden:verify` — не требуется: диф не
|
||||
меняет рендер, геометрию, стили или слои; смок-селектор не запускал —
|
||||
никакой продуктовый рендер-путь не тронут (правка целиком в
|
||||
CI-инструментарии, работающем на `gh api`-снимках, а не на карточке).
|
||||
- `python -m pytest tests_backend -q` — не требуется: `custom_components/**`
|
||||
не тронут.
|
||||
- Мутанты `metrics-count-skipped-pipeline-runs` и `metrics-rounds-by-s7-events`
|
||||
— проверил лично (патч → тест красный → откат), оба ловятся.
|
||||
- `test/single-source-numbers.test.mjs` — не относится: диф не добавляет
|
||||
пользовательских чисел (превью/подпись/подсветка), это внутренние
|
||||
метрики процесса, а не карточка.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Живой прогон `fetchSnapshot`/CLI против реального `gh api` за 15–22.09 —
|
||||
не выполнял: это ручное тестирование сети, вне цикла ревью, и сам автор
|
||||
прямо пишет, что сверка с таблицей аудита §7.1 состоится первым живым
|
||||
запуском после мержа и зеркалирования в `main`. Логика самой функции
|
||||
(пагинация, фильтрация, только-чтение) прочитана и корректна.
|
||||
- Точность `wallMs`/`job-минут` на реальных исторических прогонах — не
|
||||
считал вручную, доверяю арифметике, подтверждённой тестами на фикстурах.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/637-process-metrics`, коммит `966928135b6a` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `0687eddba719e669212123bf80bf8c6d683bbf50`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 0687eddba719
|
||||
```
|
||||
- Тело issue: `187c1bf4bbdea65f4114112ea0ee62c313cb015d343fbb44d68507420f204e41`
|
||||
- Вердикт конвейера: `yellow` · High 0
|
||||
Reference in New Issue
Block a user