diff --git a/docs/reviews/CODE-REVIEW-728-r1.md b/docs/reviews/CODE-REVIEW-728-r1.md new file mode 100644 index 00000000..54cdd00b --- /dev/null +++ b/docs/reviews/CODE-REVIEW-728-r1.md @@ -0,0 +1,194 @@ +# CODE-REVIEW-728-r1 + +## Материал раунда + +- Диапазон: `git log --oneline origin/dev..HEAD` → один коммит + `4628300d1a1f4160615f20d58f19233234e23f72` поверх `origin/dev` `52dc08a0`. +- Диф: `git diff origin/dev...HEAD` — 4 файла, `+1252/-31`: + `.github/workflows/_process-metrics.yml` (+13), `PROCESS.md` (+2), + `scripts/process-metrics.mjs` (+858 в основном новый код), `test/process-metrics.test.mjs` (+410). +- Трейлеры коммита: `Issue: #728`, `User-Visible: no` — присутствуют и верны (нет + правки `docs/CHANGELOG*`, так как изменение не продуктовое). +- Это первый код-ревью задачи (round r1 этапа code); цикл ревью ТЗ (отдельный + бюджет этапа) был пройден за r1→r2, в материале кода не разбирается заново. +- Validate на `4628300d` зелёный (ссылка в промпте) — `npx tsc --noEmit`, + `npm test`, `npm run build` + сверка бандла не перегонялись, доверие по + правилу «дешёвые гейты уже подтверждены». + +## Скоуп + +Задача выделена из #707/#637: `scripts/process-metrics.mjs` получает срез по +трекам (`track:ship/show/ask`) поверх еженедельного отчёта о процессе — +К1 трек на момент события, К2 отрезки времени, К3 причины возвратов, К4 +находки пакетного ревью ship, К5 job-минуты по стадиям, К6 токены («нет +данных»), К7 сравнение когорт до/после 28.09, К8 вывод и правка +`_process-metrics.yml` (`fetch-depth: 0`, `timeout-minutes: 30`). Отчёт только +читает снимок GitHub/git, ничего не пишет. Трек `ask` подтверждён владельцем +(issue-комментарий, первая строка ТЗ + метка `track:ask`). + +## Как проверялось + +Код прочитан целиком (`scripts/process-metrics.mjs`, 1110 строк) и сверен с +ТЗ (К1–К8, таблицы АС1–АС9) построчно. Отдельно сверены с реальным деревом: + +- Регэкспы `NOT_RUN_VALIDATE_RE`/`NOT_RUN_CONFLICT_RE` — с шаблонами + `.github/workflows/_process.yml:695` (конфликт) и `:837` (Validate красный, + оба значения `$kind`: `Validate`/`Validate с мутантами`, задаются в + `_process.yml:834-835`); `grep` по файлу подтвердил ровно два шаблона с + префиксом `**Ревью не запускалось:**`, третьего нет. +- Пограничный случай #705 (`push-refused-workflow`, `merge-candidate.mjs:205`) + — жирность закрывается после двоеточия (`**Ревью не запускалось: кандидат + меняет workflow-файл...**`), поэтому `PIPELINE_EVENTS`-регэксп + `/^\*\*Ревью не запускалось:\*\*/m` не матчит, причина — `unknown`, как и + заявлено в ТЗ. +- `labelTrack` (`process-track.mjs`), `verdictDeclaration` + (`review-doc-guard.mjs`), `parseAnchorBlock`/`anchorBlock` (`ship-review.mjs`), + `classify` (`change-classes.mjs`), `PIPELINE_EVENTS` (`wait-verdict.mjs`) — + импортированы, не скопированы; сигнатуры использования совпадают с + определениями. +- `git diff --stat` подтверждает: `wait-verdict.mjs`, `review-doc-guard.mjs`, + `ship-review.mjs`, `change-classes.mjs`, `merge-candidate.mjs`, `_process.yml` + не менялись — ровно как заявлено в «Затронутые файлы». + +### Гейты — что прогнано и результат + +| Гейт | Прогнан | Результат | +|---|---|---| +| `npx tsc --noEmit`, `npm test` (весь набор), `npm run build` + сверка бандла | Нет — подтверждено зелёным Validate на `4628300d` (ссылка в промпте) | — | +| `node --test test/process-metrics.test.mjs` (целевой файл, точечно) | Да | **22/22 зелёных** | +| `node scripts/mutation-gate.mjs --check` | Да | `предупреждений mutation registry: 3` — те же, что на `dev` (`corpus-loses-its-short-edge`, `optimize-reports-work-it-did-not-do`, `nightly-reuse-accepts-stale-marker`, все `#650`, к `process-metrics.mjs` отношения не имеют); новых находок нет | +| `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | Да | «Исполняемого frontend-диффа нет (`src/**/*.ts` не тронут)» — браузерные смоки не выбираются, диффу нечего проверять | +| `actionlint` на `.github/workflows/_process-metrics.yml` | Нет (инструмент недоступен локально) | правка тривиальна — два скалярных значения (`fetch-depth`, `timeout-minutes`) у уже существующих ключей, риск синтаксической ошибки низкий | +| `python -m pytest tests_backend` | Не применимо | диф не трогает `custom_components/**/*.py` | +| `npm run golden:verify` | Не применимо | метки `ci:golden` нет, диф не трогает рендер | +| `npm run invariants` | Не применимо | диф не трогает геометрию модели | +| performance-профили | Не применимо | не названы в AC | + +### Самостоятельная проверка «тест умеет падать» (выборочная мутация) + +Проверены три критических инварианта из ТЗ собственноручной порчей кода (не +мутации автора, независимая проверка) — файлы возвращены в исходное состояние +после каждой проверки, `git diff --stat` после всех проверок пуст: + +1. Откат `_process-metrics.yml` на `fetch-depth: 1`/`timeout-minutes: 15` → + тест `#728 workflow: полная история и потолок 30 минут` красный (AC8). +2. Слияние причин `conflict`/`validate-red` в одну (`NOT_RUN_CONFLICT_RE` ветка + `returnSignal` возвращает `'validate-red'` вместо `'conflict'`) → красные + тест «контракт: шаблоны» и `returnReason` (AC3) — защита, которую в ревью + ТЗ r1 называли отсутствующей, действительно ловит регресс. +3. Отключение ветки `if (blocked) return 'blocked'` в `issueSegments` → красный + тест `issueSegments` (AC2, инвариант «сумма отрезков = lead» и прямые + значения по часам). + +Все три мутации поймались ожидаемым тестом без ложных совпадений в других +тестах файла. + +## Находки + +Нет High. Нет Medium. Нет Low. + +Зафиксированные в ревью ТЗ риски (расхождение классификатора причин с текстом +конвейера, несобираемый тест AC8) были закрыты автором в r2 этапа spec и +проверены здесь заново по коду, а не на слово — см. раздел выше. Новых находок +код-ревью не выявило. + +## Что проверено и корректно + +- **К1 (трек на момент, AC1).** `trackAt`/`trackPath` строят множество меток + строго до момента `t` (`labelsAt`) и делегируют решение `labelTrack` — + единому правилу с конвейером; прежние метки `small`/`trivial` → `show`, + инфраструктура без метки → `show`, продукт без метки → `ask` — всё по §5.1. + `infra` — файлы коммитов задачи без Release-коммита и без класса A + (`issueChanges` отбрасывает `Release:`-коммиты до `isInfra`). +- **К2 (отрезки, AC2).** `issueSegments` корректно сводит `queue`/`spec`/ + `work`/`review`/`rework`/`blocked`, сумма равна `lead` (проверено и тестом, и + четвёртой независимой мутацией). Повторная постановка `S7` не создаёт + возврата и не сдвигает `reviewSince` (комментарий-причина до повторной + постановки остаётся валидным окном для `returnReason`). +- **К3 (причины возврата, AC3).** Признаки с константой конвейера + (`merge`, вердикты) — импорт; признаки без константы (`validate-red`, + `conflict`) — экспортируемые регэкспы, сверенные построчно с реальными + шаблонами `_process.yml` и защищённые контрактным тестом, читающим тот же + файл. Пограничный случай #705 и «третье продолжение» корректно уходят в + `unknown`. `returnReason` берёт последний комментарий с признаком строго в + окне `[since, at]`, посторонние комментарии причину не перекрывают. +- **К4 (находки ship, AC4).** `shipFindings` читает `SHIP-REVIEW-*.md` из + `docs/reviews/` и `legacy/reviews/` через `parseAnchorBlock`, документ на + несколько задач не задваивается в разделе «По трекам» (тест подтверждает + `docs: 1` при двух задачах в одном блоке). +- **К5 (job-минуты, AC5).** Стадии по имени job (`jobStage`) сверены с + реальными именами job конвейера в `_process.yml` контрактным тестом; потолок + 600 прогонов и «усечено: N из M» воспроизведены; недоступные jobs дают `null` + → «нет данных», а не ноль. +- **К6 (токены, AC6).** Без машинной строки — ровно заявленная константа + `TOKENS_NO_DATA`, в секции «Токены» нет ни одной цифры (проверено regex на + срезе секции). +- **К7 (сравнение, AC7).** Объём считается по коммитам `Issue: #NN` в `origin/dev` + без `Release:`-коммитов, без класса D и без `docs/reviews/**` (на временном + git-репозитории: 10 строк из 310 добавленных в `Release`-коммите не вошли — + проверено построчно). Корзины и порог `n < 3` воспроизведены на фикстуре с + пограничными датами (`#12` до окна, `#13` после `until` — оба не в счёт). +- **К8 (вывод и workflow, AC8).** Новые разделы идут строго после прежних + (порядок индексов подстрок проверен тестом), `buildReport` без `jobsByRun` + по-прежнему даёт `jobs: null`/`stages: null` — прежние тесты `#637`/`#682` + зелёные без правки. `_process-metrics.yml`: `fetch-depth: 0`, + `timeout-minutes: 30`, другой строки `fetch-depth:` нет — подтверждено и + тестом, и собственной мутацией. +- **Не-скоуп соблюдён.** `wait-verdict.mjs`, `review-doc-guard.mjs`, + `ship-review.mjs`, `change-classes.mjs`, `merge-candidate.mjs`, `_process.yml`, + тонкий `process-metrics.yml` — не изменены (только читаются тестами). + Отдельных причин-констант в конвейере не добавлено. +- **Трейлеры и один коммит.** `Issue: #728`, `User-Visible: no` на + единственном коммите; `User-Visible: no` корректен — поведение продукта и + видимый пользователю интерфейс не меняются, правка в `docs/CHANGELOG*` не + требуется. +- **Одно число — один источник (§8).** «Job-минуты» (старая строка, + `jobMinutes`) и новый раздел «Job-минуты по стадиям» (`stageMinutes`) + считаются по одному и тому же `jobsByRun`, переданному один раз из CLI — + в тесте оба значения (159 мин) сходятся на одной фикстуре, расхождения + источников нет. Отчёт не продуктовый (`User-Visible: no`), поэтому вопрос + «видимое пользователю число» не применим в прямом смысле §8, но внутренняя + непротиворечивость проверена. + +## Чего не проверял + +- Полные `npx tsc --noEmit`/`npm test`/`npm run build` + сверка трёх копий + бандла — не перегонял, положился на зелёный Validate на материале + (`4628300d`, ссылка в промпте); точечно перегнал только изменённый тестовый + файл и `mutation-gate --check`. +- `actionlint` — не запускал (инструмент недоступен в этой среде); риск низкий + — правка двух скалярных значений у существующих ключей workflow. +- Живой прогон `gh workflow run process-metrics.yml` по реальному API GitHub — + не прогонял (сетевой побочный эффект, к тому же сам автор называет это + «наблюдением после слияния, не AC»); все источники данных проверены на + фикстурах и временном git-репозитории, не на реальном API. +- Не проверял исчерпывающе все 21 ручную поломку, упомянутую автором в + комментарии — выборочно повторил три независимые мутации по ключевым + защитным точкам (AC2, AC3, AC8) и все три поймались; это выборка, не полный + аудит мутационной чувствительности всех 9 AC. +- `python -m pytest tests_backend`, `npm run golden:verify`, + `npm run invariants`, performance-профили — не применимы к этому диффу + (нет правок Python/геометрии/рендера, нет меток `ci:golden`/`ci:full`, + не названы в AC) и не прогонялись. +- Реальное содержимое `demo/**` и визуальная проверка — не применимо, диф не + трогает `src/**` и демо-харнесс. + +## Вывод + +ТЗ выполнено по всем девяти AC с доказательствами, проверяемыми чтением кода и +частично — собственной мутацией. Находок нет. Зелёный вердикт циклов не +расходует. + +--- + + + +## Материал раунда + +- Ветка: `issue/728-process-metrics-by-track`, коммит `4628300d1a1f` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `8a4ab36673b2f1a1e3f2dd1c125d184d0d2b8394` + ``` + git log --all --format='%H %T' | grep 8a4ab36673b2 + ``` +- Тело issue: `b1610752e35e9640f33b40e054e4482a64098837a1564d41654fe796e145c6e8` +- Вердикт конвейера: `green` · High 0