From 2095a40f41ea298da1a09510eb42b873a0905489 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Thu, 1 Oct 2026 12:18:09 +0000 Subject: [PATCH] docs: review document for #737 Issue: #737 User-Visible: no --- docs/reviews/CODE-REVIEW-737-r1.md | 158 +++++++++++++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-737-r1.md diff --git a/docs/reviews/CODE-REVIEW-737-r1.md b/docs/reviews/CODE-REVIEW-737-r1.md new file mode 100644 index 00000000..ef8d04a4 --- /dev/null +++ b/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 нет. + +--- + + + +## Материал раунда + +- Ветка: `issue/737-review-usage`, коммит `10a1c0013395` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `9682f3784bc6a18bc6327a0b0752e58042a2cc9b` + ``` + git log --all --format='%H %T' | grep 9682f3784bc6 + ``` +- Тело issue: `cd1cf82917e9f72b048ec5382cfacfe8d1d1609e4b972a18c1dad2912a3987c3` +- Вердикт конвейера: `green` · High 0 · маршрут `fix`