docs: review document for #737

Issue: #737
User-Visible: no

docs/reviews/CODE-REVIEW-737-r1.md

@@ -0,0 +1,158 @@
+# CODE-REVIEW-737-r1
+
+**Материал ревью:** ветка `issue/737-review-usage` @ `10a1c0013395e2720242dec70baab27e149e5684`
+(рабочая копия уже на нём; `git diff origin/dev...HEAD` — 13 файлов, 809 вставок / 28 удалений).
+Трек: `ask` (критерии §5 `complexity`, `surfaces`). Заход r1, блокирующих циклов
+израсходовано 0 из 4 (предыдущий красный Validate на `d1954183` ревью не запускал —
+цикл не потрачен, #510).
+
+## Скоуп
+
+Инфраструктурная задача без файлов класса A: конвейер снимает расход модели
+(`execution_file` шага `Review`) и пишет его одной машинной строкой в документы
+ревью (`CODE-REVIEW`/`SPEC-REVIEW` через блок якорей, `SHIP-REVIEW` — после
+машинного блока), а «Метрики процесса» читают эту строку вместо прежнего
+ненадёжного регэкспа по прозе документа. Три поверхности по ТЗ: К1–К2
+(`scripts/model-usage.mjs`, новый), К3–К5 (`_process.yml`, `_ship-review.yml`,
+`review-doc-guard.mjs`, `ship-review.mjs`), К6 (`process-metrics.mjs`), К7
+(PROCESS.md §10.4). Второй коммит — точечный фикс пайплайна (красный Validate
+на `d1954183`, причина названа и устранена без ревью, см. ниже).
+
+Эта задача не обслуживает ни один Job из `docs/SCOPE.md` напрямую — она
+процессная (измерение самого конвейера ревью), что соответствует её
+инфраструктурному классу и треку `ask`, поднятому по `complexity`/`surfaces`,
+а не по продуктовому критерию.
+
+## Как проверялось
+
+Полный разбор (не дельта): это первый код-ревью задачи, предыдущая попытка
+была остановлена гейтом Validate до начала ревью модели и ревью не потребляла.
+
+- Прочитан диф целиком (`git diff origin/dev...HEAD`) — оба workflow-файла,
+  `PROCESS.md`, новый `scripts/model-usage.mjs`, правки `process-metrics.mjs`,
+  `review-doc-guard.mjs`, `ship-review.mjs`, все изменённые тесты.
+- Прочитано ТЗ (тело issue #737) целиком, сверены все 7 AC с кодом и тестами
+  построчно, а не на веру по комментарию автора.
+- Прочитаны оба коммита (`ada91e31`, `10a1c001`) — трейлеры, сообщение,
+  причина фикса во втором коммите.
+- Проверено, что диффу не нужны: `src/**` не тронут (`node
+  scripts/smoke-select.mjs --base origin/dev --head HEAD` → «Исполняемого
+  frontend-диффа нет… браузер-смоки не выбираются»), Python не тронут,
+  геометрия/инварианты не тронуты, `ci:golden` не назначен, performance в AC
+  не назван.
+- Дешёвые гейты (`tsc`, `npm test`, `npm run build` + сверка бандла) на этом
+  SHA подтверждены зелёным Validate (run 36859565821) — не перегонялись
+  заново, по инструкции раунда.
+
+### Разбор по AC
+
+| AC | Проверено | Вывод |
+|---|---|---|
+| AC1 (К1: снятие) | Код `usageFromExecutionFile`/`usageFromMessages` прочитан построчно; сверен с фикстурой `test/model-usage.test.mjs` (сумма по `modelUsage` двух моделей, откат на `result.usage`, `no-result`/`no-execution-file`/`unreadable`, секрет из `tool_result` не печатается, код выхода CLI 0 во всех ветках). Негативные случаи (дробь, отрицательное, пропущенное поле модели, 13 цифр) ведут к `unreadable`, а не к неверной сумме — проверено чтением и соответствием тесту. | Доказан, тест умеет падать (конкретные мутации входа дают разные `reason`) |
+| AC2 (К2: формат) | `DATA_RE`/`NONE_RE` построены с якорями `^…$`, `COUNT` исключает ведущий ноль и ограничивает 12 цифрами. Таблица отказов в тесте (`лишний/пропущенный ключ`, `другой порядок`, `знак`, `дробь`, `13 цифр`, `40 hex`, `--> внутри`, `перевод строки`) — все дают `null`/`invalid`. Совместимость с регэкспом #728, вписанным в тест буквально, подтверждает, что `hp:usage-none` им не ловится (пробел после `hp:usage` — осознанный разделитель). | Доказан |
+| AC3 (К4: документ ТЗ/кода) | `materialAnchorBlock` дописывает строку последней, `withMaterialAnchors` не дублирует её при повторной публикации (ребейз + второй push — реальный bash/git в `publish-push-refusal.test.mjs`, строка одна). CLI `--usage=` отличает «флага нет» (вызов до #737) от «флаг пуст» (`missing`) через `argv.some(...)` — проверено чтением и отдельным тестом с `anchor()` без флагов. Прежние функции чтения блока (`anchorVerdictFrom` и др.) не задеты — тест сравнивает их результат до/после добавления строки. | Доказан |
+| AC4 (К5: документ ship) | `anchorBlock` кладёт строку сразу после закрывающего ```` ``` ```` блока, содержимое блока (which читают гейт беты и покрытие) не меняется — тест сравнивает `fenced(withUsage) === fenced(plain)` и `shipCoverage` на обоих вариантах документа. `parseAnchorBlock` берёт `lastUsageIn` после последнего маркера — строка в прозе выше блока не в счёт (отдельная проверка). | Доказан |
+| AC5 (К3: проводка) | Прочитаны оба workflow — шаг «Снять расход модели» идёт сразу за `id: review`, `if: always()`, `continue-on-error: true`, несекретный `EXEC`, выход `line`. `execution_file` нигде не выгружается и не печатается (grep по обоим файлам без строк-комментариев — только один `EXEC: …` на файл). `REQUIRED_FILES` (`review-result-gate.mjs`) не менялся — сверено импортом в тесте. Контрактный тест разбирает реальный YAML (срез job/шага текстом) плюс исполняет извлечённое тело на настоящем bash (`bash -n` и реальный прогон с `GITHUB_OUTPUT`/`GITHUB_STEP_SUMMARY`) — не заявление, а исполнение. | Доказан, в т.ч. исполнением |
+| AC6 (К6: читатель) | `reviewDocUsage` берёт строку только после `ANCHOR_MARKER` (ревью ТЗ/кода) или последнего `SHIP_REVIEW_ANCHOR` (ship) — прочитан код и тест, где документ r2 цитирует r1 в прозе и сумма r1 не задваивается. `hp:usage-none` увеличивает `missing`, не входит в `totals`, не печатается нулём (`doesNotMatch(/input_tokens|\b0\b/)`). Старый `USAGE_LINE_RE` (ловил любую строку в прозе) полностью удалён — копии формата у читателя больше нет. | Доказан |
+| AC7 (К7: канон) | Абзац «Расход модели» в PROCESS.md §10.4 прочитан и сверен с кодом: источник, формат, 6 причин (`no-execution-file, unreadable, no-result, no-usage` + `missing, invalid` у публикации) в том же порядке, что `USAGE_REASONS` в коде; «выход job, а не artifact» и причина — совпадают с К3. `docs/process/*` не тронуты — соответствует «ни автор, ни ревьюер ничего нового не делают». | Доказан чтением |
+
+### Точки, проверенные отдельно (не только по заявлению автора)
+
+- **Кавычки и границы данных.** `--usage="$USAGE"` в `_process.yml` и
+  `process.env.USAGE` в `_ship-review.yml` — оба способа передачи переживают
+  строку с пробелами внутри (формат К2 — пять ключей через пробел) без
+  разбиения на слова: первое — потому что значение в кавычках как один
+  аргумент shell, второе — потому что это не shell, а `process.env`.
+- **Разбор флага `--usage=`.** `value(name)` в `review-doc-guard.mjs` вырезает
+  фиксированный префикс длиной `name.length+3`, а не до первого `=`, поэтому
+  значение, само содержащее `=` (оно содержит, формат — `key=N`), не
+  обрезается на первом вхождении. Проверено чтением реализации и тестом с
+  реальной строкой данных.
+- **Два отдельных трейлера `Issue: #737`/`User-Visible: no`** на обоих
+  коммитах — на месте; оба changelog не тронуты, что и требуется при `no`.
+  Видимого пользователю числа в этом диффе нет вообще (только внутренний
+  машинный формат и число в еженедельном отчёте владельца) — повторного
+  источника для одного и того же числа не возникает: `tokenUsage` — один
+  читатель одного формата, который пишет один модуль (`model-usage.mjs`).
+- **Второй коммит (фикс Validate).** Причина (`pre-push-gate.test.mjs`:
+  `HOOK_FILES` не содержал новый `scripts/model-usage.mjs`, импортируемый
+  `review-doc-guard.mjs` через цепочку `process-gate`) подтверждена чтением
+  `test/pre-push-gate.test.mjs:144-147` — правка (одна строка в списке)
+  соразмерна причине, лишнего не задела.
+- **Риск подмены рабочей копии моделью перед шагом снятия расхода** (признан
+  автором в ТЗ, «Граница доверия»). Шаг `node scripts/model-usage.mjs`
+  выполняется в том же job, что и предыдущий шаг `review-result-gate.mjs` —
+  это не новый класс риска, а то же самое уже принятое архитектурное решение
+  (#556): недоверенная стадия модели выполняет локальные скрипты в своей же
+  рабочей копии до перехода на `integrate`/`publish`, где нет Bash/Write у
+  модели. Новый шаг ничего не меняет в границе доверия, только добавляет
+  источник отчётной (не решающей) величины.
+
+## Находки
+
+Высоких и средних находок нет.
+
+Низкое, не моё: автор сам отметил в комментарии (не относится к этому диффу,
+предсуществующий косметический дефект) — `withMaterialAnchors` копит `---`
+при повторном вызове, и обещал завести отдельный issue. Эта задача его не
+трогает и не обязана чинить.
+
+## Что проверено и корректно
+
+- Все 7 AC — см. таблицу выше, каждый либо доказан исполняемым тестом с
+  содержательными негативными случаями, либо (контракт по YAML) частично
+  исполнением извлечённого тела на bash.
+- Граница доверия #556 не нарушена: `REQUIRED_FILES` не изменился, расход —
+  выход job, а не запечатанный artifact; публикация разбирает строку строго
+  (`invalid`/`missing` вместо падения или мусора в документе).
+- Старый небезопасный регэкс читателя (`USAGE_LINE_RE`, ловил строку в прозе и
+  задваивал цитаты) полностью убран, новый формат имеет одного автора и
+  одного читателя (`model-usage.mjs`).
+- Трейлеры корректны, `User-Visible: no` обоснован (видимого пользователю
+  поведения нет, только процессная телеметрия для еженедельного отчёта).
+- Второй коммит — точечный, по делу, причина явно названа и совпадает с кодом.
+- PROCESS.md §10.4 (К7) синхронизирован с кодом по перечню причин и формату.
+
+## Чего не проверял
+
+- Не перегонял `npx tsc --noEmit`, `npm test`, `npm run build` + сверку трёх
+  копий бандла — Validate на этом же SHA (`10a1c001`, run 36859565821) уже
+  зелёный, бюджет раунда не тратится на повтор.
+- Браузерные смоки не прогонял — `smoke-select.mjs --base origin/dev --head
+  HEAD` подтвердил, что `src/**` не тронут и выбирать нечего; в AC браузер
+  прямо назван ненужным («Браузер не нужен, мутанты не гоняются»).
+- `python -m pytest tests_backend` не прогонял — диф не касается
+  `custom_components/**/*.py`.
+- `npm run invariants` не прогонял — геометрия и ссылки на неё не затронуты.
+- `npm run golden:verify` не прогонял — метки `ci:golden` нет, рендер карточки
+  не изменён.
+- Performance-профили не прогонял — в AC не названы.
+- Мутанты по диффу не запрашивались и не применялись (трек `ask`, но мутанты
+  в разработке не гоняются ни на одном треке — #709, ночной реестр отдельно).
+- Не выполнял вживую `gate:small`, `mutation-gate --check`, `entry-cost
+  --check`, `reviews-index --check`, `process-digests` — заявлены автором как
+  зелёные и совпадают по косвенным признакам с прочитанным кодом (списки
+  причин, число слов промпта не менялось — правки вне текста промпта модели),
+  но не перепрогонялись отдельно, так как входят в состав уже подтверждённого
+  Validate/`gate:small`.
+
+## Вердикт
+
+Зелёный. Все AC доказаны исполняемыми тестами с содержательными негативными
+случаями (кроме части AC5, где контракт YAML частично доказывается чтением и
+частично — исполнением извлечённого тела на bash, что отдельно отмечено).
+Находок High/Medium нет.
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `issue/737-review-usage`, коммит `10a1c0013395` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Дерево материала: `9682f3784bc6a18bc6327a0b0752e58042a2cc9b`
+  ```
+  git log --all --format='%H %T' | grep 9682f3784bc6
+  ```
+- Тело issue: `cd1cf82917e9f72b048ec5382cfacfe8d1d1609e4b972a18c1dad2912a3987c3`
+- Вердикт конвейера: `green` · High 0 · маршрут `fix`