From 4366a1c79635696a9d8b6c67d854bef42a6bbc94 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Wed, 23 Sep 2026 08:57:40 +0000 Subject: [PATCH] docs: review document for #637 Issue: #637 User-Visible: no --- docs/reviews/CODE-REVIEW-637-r2.md | 197 +++++++++++++++++++++++++++++ 1 file changed, 197 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-637-r2.md diff --git a/docs/reviews/CODE-REVIEW-637-r2.md b/docs/reviews/CODE-REVIEW-637-r2.md new file mode 100644 index 00000000..ed319943 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-637-r2.md @@ -0,0 +1,197 @@ +# CODE-REVIEW · issue #637 · заход r2 + +Материал: `44aa35806050e3f3c663b317aab6016b60f88888` (рабочая копия на нём, +`git status --porcelain` пуст). Ветка `issue/637-process-metrics`, 3 коммита +поверх текущего `origin/dev` (`0b55163d`): `46edcb1f` (ребейз-эквивалент +материала r1), `5df8cfc0` (документ ревью r1, публикация), `44aa3580` (фикс +находки r1). Класс B: `scripts/process-metrics.mjs`, +`test/process-metrics.test.mjs`. Трейлеры `Issue: #637`, `User-Visible: no` — +верно, видимого поведения продукта нет. + +Трек — инфраструктурный (#562, §1): ни одного файла класса A, задача входит в +общий флоу сразу на `S7-code-review`. + +## Раунд по дельте (§2.10) + +1. **Вердикт и материал r1.** `docs/reviews/CODE-REVIEW-637-r1.md` (закоммичен + в `5df8cfc0`) — жёлтый, заход r1, блокирующих циклов 0/4, High 0, Medium 1 + в скоупе. Материал раунда объявлен в самом документе: + SHA `966928135b6a40fd6a73c7631bd514a79015d48a`, дерево + `0687eddba719e669212123bf80bf8c6d683bbf50`, хеш тела issue + `187c1bf4bbdea65f4114112ea0ee62c313cb015d343fbb44d68507420f204e41`. + +2. **SHA не резолвится** — `git cat-file -t 966928135b6a...` и поиск дерева + `0687eddba719` по `git log --all --format='%H %T'` дают пустой результат. + Это обычное дело (§2.10, п.2): автор явно пишет «Материал: + `44aa3580…` после rebase на актуальный `dev`». Проверил, что это чистый + ребейз, а не скрытая правка: родитель `46edcb1f` (эквивалент старого + материала на новой базе) — это ровно текущий `origin/dev` (`0b55163d`, + `git merge-base origin/dev 46edcb1f` совпадает), диапазон файлов и их + размер (`process-metrics.mjs` 291 строка, `test` 116 строк до фикса) + соответствуют тому, что описано и процитировано в документе r1 + (`buildReport`, строки 162–165, тот же код). Значит дельта раунда — это + ровно коммит `44aa3580` (`scripts/process-metrics.mjs` +8/−4, + `test/process-metrics.test.mjs` +23), а не результат смены поведения из-за + рейбейза. Разбор по дельте оправдан: `dev` не «ушёл вперёд» относительно + материала — ребейз тривиальный, чужой скоуп не задет, новая подсистема не + затронута, объём дельты (31 строка) несопоставим с исходной задачей. + +3. **Дельта объявлена:** `git diff 46edcb1f..44aa3580` = сам коммит + `44aa3580` (см. ниже «Закрытие раунда r1»). + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где это видно | +|---|---|---| +| Medium: `hours(median(...) ?? NaN) \|\| null` в `buildReport` схлопывает настоящую медиану 0 ч в `null`/«—» (`scripts/process-metrics.mjs:162-165` на материале r1) | Введена `medianHours(values)` (`scripts/process-metrics.mjs:35-38`): различает «нет данных» (`median` вернул `null` → `null`) и любой числовой результат, включая 0, через `value === null ? null : hours(value)` — без финального `\|\| null`, который путал `0` (falsy) с отсутствием данных. Все четыре медианы (`medianLeadToS7Hours`, `medianReviewToMergeHours`, `medianLeadToS8Hours`, `medianSpecLeadHours`, `scripts/process-metrics.mjs:166-169`) переведены на неё | `scripts/process-metrics.mjs:35-38,166-169`; новый тест `test/process-metrics.test.mjs:108-129` — воспроизводит ровно сценарий из репро r1 (переходы короче ≈3 минут) и проверяет `[0, 0, 0.1, 0]` вместо `[null, null, null, null]`, плюс отдельно проверяет, что пустая выборка по-прежнему даёт `null`/«—» (не путает две ветки местами) | + +Мутационное доказательство сделал сам (мутация не входит в реестр — фикс в +чистом юните, п.8 §2.7 разрешает прогон со снятой защитой прямо в ревью, без +`scripts/mutation-gate.mjs`): вручную вернул старое выражение +`hours(median(...) ?? NaN) || null` во все четыре поля, прогнал +`node --test test/process-metrics.test.mjs` — новый тест красный +(`AssertionError [ERR_ASSERTION]` на `deepStrictEqual`, строка 118), остальные +7 тестов проходят. Откатил патч (`cp` из бэкапа), `git status --porcelain` +снова пуст, повторный прогон — 8/8 pass. Тест умеет падать. + +## Унаследовано из r1 + +Без повторной проверки приняты (документ `docs/reviews/CODE-REVIEW-637-r1.md`, +материал которого эквивалентен текущему `46edcb1f`, см. п.2 выше) — дельта +`44aa3580` этих путей кода не касается: + +- `issueMetrics` — вход по первой статусной метке, различение `entered`/S4/S5/ + S7/S8, счётчики повторных постановок. +- `reviewRounds` — строгий регэксп имён файлов ревью, раунды по документам, а + не по событиям S7-метки (мутант `metrics-rounds-by-s7-events`, пойман). +- `runMetrics` — группировка прогонов конвейера `process #NN · …` в одну + строку; остальные workflow — по имени. +- `pipelineMetrics` — skipped-прогоны не считаются в wall-time (мутант + `metrics-count-skipped-pipeline-runs`, пойман). +- `jobMinutes` — доля «Мутанты» по имени job, устойчивость к сломанным полям + времени. +- `fetchSnapshot` — только чтение (`gh api` issues/timeline/actions/runs + + `git ls-tree`), пагинация, фильтр по `closed_at` внутри окна. +- `.github/workflows/process-metrics.yml` — `permissions` только на чтение, + расписание понедельник 05:00 UTC + `workflow_dispatch`, код и `ref: dev`. +- Трейлеры коммита и структура §2.10-блока материала. + +Инвариант этого наследования: сама дельта (`buildReport`, только четыре поля +медиан плюс новый хелпер) не может задеть ни один из перечисленных путей — +они не вызывают и не вызываются из изменённых строк. Проверил чтением диффа +`44aa3580`, других правок в `scripts/process-metrics.mjs` нет. + +## AC — что дельта задевает + +- **AC1** (скрипт воспроизводит цифры аудита ±10 %): фикстура аудита + (`test/process-metrics.test.mjs:60-106`, unchanged) по-прежнему проходит — + значения там не нулевые, фикс их не касается численно, только путь для + нулевого случая. Дельта расширяет доказательство AC1 на нулевой случай, + который сама формулировка AC подразумевает («самый быстрый переход» — + ровно то, что баг маскировал). Живой прогон против реального `gh api` за + 15–22.09 остаётся не выполненным — это унаследовано из r1 (см. ниже «Чего + не проверял»), делта на это не влияет. +- **AC2** (тест на парсинг фикстур): дельта добавляет восьмой тест, число + тестов выросло с 7 до 8, все проходят — доказательство AC2 усилилось, не + ослабло. +- **AC3** (первый отчёт опубликован автоматически): не задета дельтой, + наследуется из r1 как открытая до пост-мерж действия владельца + (зеркалирование workflow в `main`) — это явно вне контроля код-ревью и не + являлось находкой в r1. + +## Гейты + +- `node --test test/process-metrics.test.mjs` — прогнал сам: **8 pass, 0 + fail** (7 из r1 + новый регрессионный). +- Мутация находки r1 (снятие `medianHours`, возврат старого выражения) — + прогнал сам: новый тест краснеет, остальные держатся; откатил, дерево чистое. +- Мутанты `metrics-count-skipped-pipeline-runs`, + `metrics-rounds-by-s7-events` — не перепрогонял: код, который они проверяют + (`pipelineMetrics`, `reviewRounds`), дельтой не тронут (см. «Унаследовано»), + а сам реестр `scripts/mutation-registry.mjs` не менялся между `46edcb1f` и + `44aa3580` (дифф двух файлов, реестра нет). Инвариант r1 переносится. +- `npx tsc --noEmit`, `npm test` (полный), `npm run build` со сверкой копий + бандла — не гонял: Validate на этом самом SHA (`44aa3580`) зелёный + (https://github.com/Matysh/houseplan-card/actions/runs/35839128726), дифф с + тех пор не менялся (рабочая копия чистая на этом SHA). +- `node scripts/check-docs.mjs` — не требуется: дифф не трогает `src/**` + (только `scripts/`, `test/`). +- `npm run invariants` — не требуется: геометрия, `layout`, толщина стен, + `marker.space`/`open_spans` не затронуты. +- `demo/smoke_*.mjs`, `npm run golden:verify` — не требуется: дифф не меняет + рендер, карточку, стили или слои; это чистые функции над снимком GitHub. +- `python -m pytest tests_backend -q` — не требуется: `custom_components/**` + не тронут. +- `test/single-source-numbers.test.mjs` — не относится: метрики процесса не + пользовательские величины карточки. + +## Что проверено и корректно + +- Фикс сохраняет форму данных: `medianHours` возвращает `null` только когда + `median()` сам вернул `null` (пустая выборка после `filter(Number.isFinite)` + в `median`, `scripts/process-metrics.mjs:29-34`), в остальных случаях — + всегда `hours(value)`, включая 0. Прочитано и подтверждено регрессионным + тестом на обеих ветках (пустая выборка и нулевая медиана в одном тесте, не + раздельно — так тест не может «повезти» и спутать ветки местами). +- `renderMarkdown`/`fmt` (`scripts/process-metrics.mjs:185`) корректно + отображает `0` как `0 ч`, а не как «—»: `value == null` ложно для `0`, + `Number.isNaN(0)` ложно — код `fmt` не менялся в этой дельте и уже был + правильным, баг был только в вычислении значения выше по цепочке, не в + форматировании. Подтверждено новым тестом + (`/Медиана вход → S7 \| 0 ч/`). +- Округление `hours()` (`Math.round((ms/3_600_000)*10)/10`) — проверил вручную + арифметику теста: 2 мин → 0.0(3) ч → округление до 0; 3 мин → 0.05 ч → + `Math.round(0.5)=1` (JS всегда вверх на `.5`) → 0.1 ч. Ожидаемый вектор + `[0, 0, 0.1, 0]` в тесте математически верен, не «подогнан» под реализацию. +- Трейлеры `44aa3580`: `Issue: #637`, `User-Visible: no` — соответствуют + `git show -s --format=full`. +- Хендофф-комментарий владельца/исполнителя точно называет материал раунда + (`44aa35806050e3f3c663b317aab6016b60f88888`) и он совпадает с + `git rev-parse HEAD` рабочей копии — соответствует §2.7 (вердикт привязан к + SHA, сверено непосредственно перед итогом). + +## Чего не проверял + +- Живой прогон `fetchSnapshot`/CLI против реального `gh api` — наследуется из + r1, не задет дельтой; автор по-прежнему планирует сверку с таблицей аудита + первым живым запуском после зеркалирования workflow в `main`. +- Мутанты `metrics-count-skipped-pipeline-runs`, + `metrics-rounds-by-s7-events` — не перепрогонял в этом раунде, обоснование + выше («Гейты»): код, который они проверяют, дельтой не задет. +- `npx tsc --noEmit`/`npm test` (полный)/`npm run build` — не гонял, опираюсь + на зелёный Validate на точном SHA материала (ссылка выше), как разрешает + сокращение гейтов ревью. + +## Вывод + +Единственная находка r1 (Medium, в скоупе) закрыта точечным фиксом: +`medianHours` корректно различает «данных нет» и «медиана равна 0», +регрессионный тест воспроизводит исходное репро и умеет падать (проверено +мутацией вручную). Делта локальна, не задевает ни один из путей, признанных +корректными в r1, AC1/AC2 доказательство усилено, AC3 не в скоупе дельты. +High-находок нет, новых Medium/Low не нашёл. + +**Вердикт: зелёный.** + +## Материал раунда + +- Ветка: `issue/637-process-metrics`, коммит `44aa35806050e3f3c663b317aab6016b60f88888`. +- Рабочая копия ревью — на этом SHA, `git status --porcelain` пуст. +- Предыдущий материал (r1) осиротел ребейзом (ожидаемо, §2.10 п.2): SHA + `966928135b6a40fd6a73c7631bd514a79015d48a` не резолвится; рабочий эквивалент + на текущей базе — `46edcb1f37a7bad93de97e04e2307f67d0d310d4` + (`git merge-base origin/dev 46edcb1f` = `origin/dev` = `0b55163d`). + +--- + + + +## Материал раунда + +- Ветка: `issue/637-process-metrics`, коммит `44aa35806050` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `c5dd9f105b40966232d871cb5b2a8dfbb5cf54fb` + ``` + git log --all --format='%H %T' | grep c5dd9f105b40 + ``` +- Тело issue: `187c1bf4bbdea65f4114112ea0ee62c313cb015d343fbb44d68507420f204e41` +- Вердикт конвейера: `green` · High 0