diff --git a/docs/reviews/CODE-REVIEW-761-r1.md b/docs/reviews/CODE-REVIEW-761-r1.md new file mode 100644 index 00000000..af92da66 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-761-r1.md @@ -0,0 +1,174 @@ +# CODE-REVIEW-761-r1 + +**Issue:** #761 · «process-metrics: токены — только за окно отчёта» +**Заход:** r1 · блокирующих циклов израсходовано 0 из 2 +**Трек:** show (PROCESS.md §5) +**Материал:** `7032037a9eb01ac44b09520181bb3f78cbf07962` (ветка `issue/761-tokens-window`, один коммит поверх `dev` `86a301f6`, рабочая копия на нём) +**Validate на этом SHA:** success — https://github.com/Matysh/houseplan-card/actions/runs/36875463085 + +## Скоуп + +ТЗ (тело issue): `tokenUsage` суммировал расход по всем документам ревью в +`HEAD` без окна — каждая неделя повторяла всю историю. Меняется: документ +входит в токены недели, если коммит, добавивший его в `dev`, попал в +`[since, until]`; перенос в архив `legacy/reviews/` (#682) переименованием, а +не добавлением. + +- **AC1.** Два документа со строкой расхода, добавленные внутри и вне окна, — + в сумме только первый. +- **AC2.** Перенос документа в `legacy/reviews` в окне его не добавляет. +- **AC3.** Счёт `missing` (#737) — по тому же окну. + +Единственная поверхность — `scripts/process-metrics.mjs` (отчёт по процессу, +внутренний инструмент конвейера). Продуктового UX не касается, миграций +конфига нет, новых compatibility-полей нет, видимое для пользователя продукта +поведение не меняется (`User-Visible: no` в трейлере коммита — верно). + +## Как проверялось + +1. Прочитан диф `git diff origin/dev...HEAD` целиком (единственный коммит, + 132 строки: `scripts/process-metrics.mjs` + `test/process-metrics.test.mjs`). +2. Прочитано тело issue #761 и все три комментария (оценка, взятие в работу, + отчёт «Сделано»). +3. `reviewDocAddedAt` и логика переноса даты при переименовании проверены + вручную на временном git-репозитории (`git init`, коммит `A` документа, + `git mv` в архив, разбор сырого вывода `git log -M --diff-filter=AR + --reverse --name-status --format=%x1e%cI` построчно) — поведение совпало + с ожидаемым: дата переносится на новый путь, старый путь из карты уходит. +4. Прогнан штатный набор тестов файла: `node --test test/process-metrics.test.mjs` + — 24/24 зелёных, включая новый тест AC1–AC3. +5. Проверено «тест умеет падать» — три целевые мутации кода (не тестов): + - открытая верхняя граница окна (`moment >= from` без `&& moment <= to`) + → тест падает (ловит AC1/окно); + - отключение окна в `trackReport` (`tokens: tokenUsage(reviewDocs)` вместо + ветки с `tokenDocs`) → тест падает; + - снятие флага `-M` у `git log` в `fetchSnapshot` → тест **не** падает (см. + находку Low ниже). +6. Проверена реальная проводка `reviewDocAdded` до вызова `buildReport` в CLI + (`scripts/process-metrics.mjs:1176-1178`) — передаётся через спред снимка, + путь не теряется между `fetchSnapshot` и рендером. +7. Проверены трейлеры коммита: `Issue: #761`, `User-Visible: no` — есть; + правка changelog не требуется, так как видимого пользователю поведения + нет (это внутренний отчёт конвейера, не продукт). +8. Сверена ссылка на зелёный прогон Validate на этом SHA (`gh run view`) — + `headSha` совпадает, `conclusion: success`. + +## Что проверено и корректно + +- **AC1 (окно включения).** `tokenDocs` фильтрует документы по + `added.get(path)` в `[since, until]` включительно по обеим границам; + документ без найденной даты добавления (`at(undefined)` → `NaN`) в + сравнении `NaN >= from` даёт `false` — корректно исключается, как и + описано в JSDoc. Подтверждено тестом и ручной мутацией границы. +- **AC2 (перенос в архив не добавление).** `reviewDocAddedAt` обрабатывает + записи `git log -M --diff-filter=AR --reverse --name-status` построчно: + `A` кладёт дату по текущему пути, `R\d*` при наличии даты у исходного пути + переносит её на новый путь и удаляет старый ключ. Хронология гарантирована + `--reverse`. Разобрано построчно и перепроверено на отдельном + git-репозитории — поведение совпадает с кодом и с новым тестом (перенос + `CODE-REVIEW-700-r1.md` в `legacy/reviews/v1.0.0/...` внутри окна не + добавляет документ, т.к. исходная дата — вне окна). +- **AC3 (missing по тому же окну).** `trackReport` при наличии + `reviewDocAdded` вызывает `tokenUsage(tokenDocs(reviewDocs, {...}))` + целиком — `missing` считается `tokenUsage` над уже отфильтрованным + списком, отдельной фильтрации для `missing` не требуется и её нет; тест + явно проверяет `report.tokens.missing === 1` для документа `SPEC-REVIEW-703` + (без данных, внутри окна), игнорируя `SPEC-REVIEW-701` (без данных, вне + окна, при этом перенесённый в архив). +- **Обратная совместимость юнит-уровня.** Без карты дат (`added: null`, + путь «юнит над готовыми документами») `tokenDocs` возвращает документы как + есть — поведение до #761 не изменилось; явно протестировано + (`tokenDocs(snap.reviewDocs).length === 5`). +- **Проводка в CLI.** `buildReport({ since, until, ...snapshot, compare })` + передаёт `reviewDocAdded` из `fetchSnapshot` без искажений; реальный + еженедельный прогон получит окно, а не только юнит-тест. +- **Рендер.** Новая строка-определение окна в разделе «Токены» выводится + только когда `tokens.window` есть (т.е. только когда у вызывающего была + карта дат) — старый формат вывода для вызовов без карты не меняется. + Подтверждено тестом на срез markdown-секции. +- Диапазон правок (132 строки, один файл логики + тесты) соразмерен оценке + автора (сложность 2/10); новых абстракций или рефакторинга за пределами + задачи нет. + +## Находки + +### Low — заявленная «красная» мутация не воспроизводится в этой среде + +**Файл:** `scripts/process-metrics.mjs:1154-1155` +**Было заявлено в issue:** «Пять ручных поломок, все красные: без окна, +`--no-renames`, переименования игнорируются, `missing` без окна, открытая +верхняя граница.» + +Воспроизвёл мутацию «без `-M`» (убрал флаг у вызова `git log` в +`fetchSnapshot`) и прогнал тест — он остался зелёным. Причина: в git 2.55 +(версия в этом окружении и, видимо, в CI — Validate на этом SHA зелёный) +`git log --name-status` сам детектирует чистое переименование файла без +изменения содержимого даже без `-M`/`--find-renames`, поэтому сценарий +`git mv` в тесте показывает `R100` независимо от флага. Снятие флага других +проверенных мутаций (открытая верхняя граница, отключение окна целиком) тест +ловит штатно. + +Это не функциональный дефект: код явно передаёт `-M`, что делает поведение +детерминированным независимо от версии/конфигурации git, и в реальном +запуске (`npx git log -M ...`) работает правильно — независимо подтверждено +отдельным прогоном на временном репозитории. Но конкретно эта строка из +пяти заявленных «красных» поломок не подтверждена в данной среде — отчёт +автора об исчерпывающей мутационной проверке в этой части неточен. +Серьёзность Low: не блокирует, информация к сведению для доверия к будущим +аналогичным заявлениям в этой задаче. + +**Чем доказано:** ручная мутация + повторный прогон +`node --test test/process-metrics.test.mjs` (24 pass, включая мутированный +прогон — тест, который должен был упасть, не упал). + +## Чего не проверял + +- `npx tsc --noEmit`, `npm test` (весь набор), `npm run build` со сверкой + бандлов — не перегонял: Validate на этом SHA (`7032037a`) зелёный + (https://github.com/Matysh/houseplan-card/actions/runs/36875463085), раунд + первый, дешёвые гейты уже подтверждены. +- Браузерные смоки — диф не трогает рендер/UI/геометрию, `node + scripts/smoke-select.mjs` не запускал: AC задачи смоков не называют, тело + issue смоук не упоминает (#696). +- `npm run golden:verify` — не запускал, в диффе нет меток `ci:golden` и + правок рендера. +- `python -m pytest tests_backend -q` — не запускал, `custom_components/**` + не затронут. +- `npm run invariants -- --config <экспорт>` — не запускал, диф не трогает + геометрию модели дома. +- performance-профили — не названы в AC, не запускал. +- `gate:small`, `mutation-gate --check`, `entry-cost --check`, упомянутые + автором в комментарии «Сделано», — не перегонял отдельно; их область + частично перекрыта моей собственной ручной мутационной проверкой (п. 5 + выше), давшей тот же результат за исключением находки Low. +- Реальную недельную историю репозитория (965 архивных документов, о + которых пишет автор) не прогонял — операция затронула бы `dev`/сетевые + вызовы `gh`, не являющиеся частью материала ревью; поверил ручному + прогону на изолированном синтетическом репозитории, этого достаточно для + проверки алгоритма. + +## Вердикт + +Все три AC доказаны тестом, который действительно падает на целевых +мутациях (кроме одной локальной неточности в самоотчёте автора — Low, не +блокирует). Проводка окна от `fetchSnapshot` до рендера и до CLI корректна, +обратная совместимость для вызовов без карты дат сохранена и +протестирована. High-находок нет, Medium-находок нет, единственная +находка — Low, в скоупе, не требует правки для зелёного вердикта. + +**Зелёный.** + +--- + + + +## Материал раунда + +- Ветка: `issue/761-tokens-window`, коммит `7032037a9eb0` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `c3def681dc104c86a8d04d11e963a83801af348d` + ``` + git log --all --format='%H %T' | grep c3def681dc10 + ``` +- Тело issue: `efa657b88ade6fbe57b0a6004a8c6528a9bc393614800ebbca9fd7f7167cdc24` +- Вердикт конвейера: `green` · High 0 · маршрут `fix` +