docs: review document for #737

Issue: #737
User-Visible: no
This commit is contained in:
claude[bot]
2026-10-01 05:07:59 +00:00
parent b27ae06ef8
commit f3cfb93e94
2 changed files with 210 additions and 1 deletions
+2 -1
View File
@@ -1,10 +1,11 @@
# Индекс ревью
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 230, issue: 110. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 231, issue: 111. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
| Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы |
|---|---|---|---|---:|---:|---|---|
| бета v1.79.0-beta.1 | [SHIP-REVIEW-v1.79.0-beta.1.md](SHIP-REVIEW-v1.79.0-beta.1.md) | пакетное ревью ship · — | ⚪ — | 0 | 0 | — | — |
| #737 | [SPEC-REVIEW-737-r1.md](SPEC-REVIEW-737-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | — | — |
| #732 | [CODE-REVIEW-732-r1.md](CODE-REVIEW-732-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
| #730 | [CODE-REVIEW-730-r1.md](CODE-REVIEW-730-r1.md) | code · r1 | 🟡 жёлтый | 0 | 1 | текст сводки «повтор и ребейз не помогут» вводит в заблуждение именно в сценарии, котор… | `scripts/merge-candidate.mjs` |
| #730 | [CODE-REVIEW-730-r2.md](CODE-REVIEW-730-r2.md) | code · r2 | 🟢 зелёный | 0 | 0 | — | — |
+208
View File
@@ -0,0 +1,208 @@
# SPEC-REVIEW-737-r1
## Материал раунда
- Issue #737, тело на момент ревью (раздел `## ТЗ`, полный текст прочитан и
процитирован ниже построчно).
- Рабочая копия репозитория — `3b9f25ea659c645baded44a82c15ed635798b977`
(`origin/dev`), использована только для проверки фактических утверждений ТЗ
о текущем состоянии кода (К1–К7, Проблема, Затронутые файлы). Продуктовый код
этой задачей ещё не тронут — в `dev` его нет, и это ожидаемо для этапа spec.
- Внешняя зависимость, факты по которой сверены: `anthropics/claude-code-action`
на пине `9cdae7f0d995e3ba7c33f226087fdf82a59cd520` (указан в
`.github/workflows/_process.yml:1142` и `_ship-review.yml:191` — совпадает с
тем, что назвал автор ТЗ).
## Скоуп
Задача (трек `ask`, инфраструктура, `User-Visible: no`) учит три стадии модели
(`spec`/`code`/`ship`-ревью) снимать расход токенов из `execution_file`
`claude-code-action` и публиковать его одной машинной строкой в документе
ревью; читатель `process-metrics.mjs` (#728, К6) получает настоящий источник
данных вместо фиктивного регэкспа. Семь контрактных пунктов К1–К7, семь AC,
план автотестов и граничные случаи — все описаны в теле issue.
## Как проверялось
Ревьюер ≠ исполнитель, устных пояснений нет — только текст issue. Для ТЗ,
которое утверждает конкретные факты о чужом API (а не о продукте House Plan),
проверка «не бывает дешёвым гейтом» означает: свериться с реальным источником,
а не поверить формулировке. Сделано построчно:
1. Прочитаны требуемые §7.1 разделы, сверена нумерация AC↔К и их полнота.
2. Каждая ссылка на текущий код (`scripts/review-doc-guard.mjs`,
`scripts/ship-review.mjs`, `scripts/process-metrics.mjs`, оба workflow,
`scripts/review-result-gate.mjs`) открыта и сверена построчно с номерами
строк, которые называет ТЗ.
3. Внешние факты о `claude-code-action` на названном пине и о пакетах
`@anthropic-ai/claude-agent-sdk` / `@anthropic-ai/sdk` получены `curl` с
`raw.githubusercontent.com` / `registry.npmjs.org` (сеть в среде ревью
доступна) — это единственный способ отличить проверенный факт от догадки,
выглядящей как факт (§7.1).
4. Проверено, что новые файлы (`scripts/model-usage.mjs`,
`test/model-usage.test.mjs`) в дереве действительно отсутствуют, а файлы,
которые ТЗ обещает только расширить, существуют.
5. Трек и зависимости (#726, #727, #728) сверены по `git log`: все три
`S8-merged` до этого issue, предположения ТЗ о их API актуальны на HEAD.
Гейты (`typecheck`/`npm test`/`npm run build`) на этом этапе неприменимы: кода
ещё нет, стадия — ревью ТЗ, а не ревью кода (§2.4). Это не сокращение объёма, а
корректный объём для spec review.
## Находки
Находок уровня High или Medium нет.
Low — ни одной с записью на снятие: формулировки избыточно подробны местами
(например, повтор одного и того же тезиса в «Проблема» и «Принято
предположительно» п.5), но это не дефект проверяемости, а стиль; снимать нечего
чинить не в праве ревьюера менять текст.
## Что проверено и корректно
- **Обязательные разделы §7.1 — все на месте**: сценарий, что человек увидит до
и после, проблема, скоуп/не-скоуп, контракт поведения, UX·данные·i18n·
миграция (один комбинированный раздел, содержание покрывает все подпункты
явно словом «нет» там, где неприменимо), критерии приёмки AC1–AC7 с
доказательством и oracle, план автотестов, риски, откат, release-артефакты.
DoR (§2.5) по каждому пункту закрывается однозначно: i18n — нет, миграция —
нет, touch/perf — нет и обоснованно («секунда на шаг CI»), release-артефакты
— названы явным «нет» + `PROCESS.md §10.4`, откат — описан (revert коммитов,
публикации не трогаются), открытых продуктовых вопросов нет.
- **Факты о собственном коде репозитория верны**, построчно:
- `materialAnchorBlock`, `withMaterialAnchors`, `anchorVerdictFrom`,
`anchorTreeFrom`, `anchorIssueBodyFrom`, `materialAnchorsFrom`,
`reusableGreenVerdict` — все существуют в `scripts/review-doc-guard.mjs`
под теми именами, что называет ТЗ (строки 117, 281, 315, 385, 517, 526,
556).
- `materialAnchorsFrom` действительно ищет 40-символьные hex-подстроки после
`ANCHOR_MARKER` (`scripts/review-doc-guard.mjs:110–121`) — формат К2,
запрещающий 40 hex-символов и `-->`/перевод строки внутри строки `hp:usage`,
предотвращает ложное распознавание якоря именно этим механизмом.
- `anchorBlock`/`parseAnchorBlock`/`SHIP_REVIEW_ANCHOR` в
`scripts/ship-review.mjs` — на месте (строки 47, 345, 372), содержимое
совпадает с описанием К5.
- `_ship-review.yml`: `model_review` (строка 122) **действительно не имеет**
`outputs:` — ровно то расхождение, которое К3 называет проблемой и которое
К3/AC5 обещают исправить.
- `_process.yml`: шаг `Review` имеет `id: review` (строка 1141), `outputs:`
job несёт только `duration_seconds` (строки 1013–1014) — совпадает с
текстом ТЗ «рядом с `duration_seconds`».
- `scripts/process-metrics.mjs`: `TOKENS_NO_DATA`, `USAGE_LINE_RE`,
`tokenUsage` — присутствуют (строки 198, 217, 448), и воспроизведён именно
тот дефект, который ТЗ описывает как текущую проблему: `found` ставится
в `true` по самому факту совпадения регэкспа независимо от того, нашлись
ли внутри пары `ключ=число` — то есть «строка без чисел» сегодня
действительно считается документом с пустой суммой, как и написано в ТЗ.
- `fetchSnapshot`/`git grep -l -e hp:usage` (строки ~1073–1080) при
совпадении включает в выборку документов и `hp:usage-none` — подстрока
`hp:usage` в нём есть; К6 в курсе этого и опирается на это явно.
- `REQUIRED_FILES` в `scripts/review-result-gate.mjs:25` действительно не
содержит ничего про расход — граница доверия (#556), которую К3 выбирает
не трогать, описана верно.
- Тестовые файлы, которые ТЗ обещает расширить
(`test/review-doc-guard.test.mjs`, `test/publish-push-refusal.test.mjs`,
`test/ship-review.test.mjs`, `test/process-metrics.test.mjs`,
`test/default-branch-workflows.test.mjs`) существуют; новые
(`scripts/model-usage.mjs`, `test/model-usage.test.mjs`) отсутствуют —
ровно то, что ожидается до реализации.
- `STEP_SCRIPTS`/`importClosure` в `test/publish-push-refusal.test.mjs:57–58`
действительно включает `review-doc-guard.mjs` в замыкание импортов — если
реализация подключит новый модуль оттуда, тест подхватит его сам, как и
заявляет АС3.
- `npm run gate:small`, `node scripts/mutation-gate.mjs --check`,
`node scripts/reviews-index.mjs --check` (AC7) — все существуют.
- В `PROCESS.md` §10.4 абзаца «Расход модели» пока нет — задача действительно
добавляет новый текст, а не дублирует существующий.
- **Факты о внешней зависимости `claude-code-action` на названном пине —
верны**, сверено по исходнику на GitHub и по пакетам npm:
- `base-action/action.yml` (используется top-level `action.yml`, который его
оборачивает) отдаёт `outputs.execution_file` — «Path to the Claude Code
execution output file»; совпадает с ТЗ.
- `execution-file.ts`: файл — `$RUNNER_TEMP/claude-execution-output.json`,
`writeExecutionFile` вызывается и при нормальном завершении, и в `catch`
при ошибке SDK (`run-claude-sdk.ts`, строки ~208–222) — совпадает с
формулировкой «пишется после конца сессии, и при ошибке SDK тоже».
- `sanitizeModelUsage`/`sanitizeSdkOutput` и комментарий «без утечки token
usage or cost details» — на месте, строка комментария ровно та, что
процитирована в ТЗ.
- Тип `SDKResultSuccess` (пакет `@anthropic-ai/claude-agent-sdk`, актуальная
версия 0.3.286) несёт оба поля: `modelUsage: Record<string, ModelUsage>`
(по моделям, camelCase: `inputTokens`, `outputTokens`,
`cacheReadInputTokens`, `cacheCreationInputTokens`) и `usage:
NonNullableUsage` — тип на основе `BetaUsage` из `@anthropic-ai/sdk`, где
поля снэйк-кейсом: `cache_creation_input_tokens`,
`cache_read_input_tokens` (и стандартные `input_tokens`/`output_tokens`
рядом). Ровно то разделение camelCase/snake_case, на которое опирается К1.
- Тестовая фикстура `base-action/test/run-claude-sdk.test.ts` (строки
83–108) использует именно эти ключи и `num_turns` — К1 не придумывает
формат, а списывает его с реального контракта action на этом пине.
- Пин `9cdae7f0d995e3ba7c33f226087fdf82a59cd520` в workflow совпадает с тем,
что называет ТЗ, в обоих файлах.
- **AC однозначны и проверяемы**: каждый из AC1–AC7 называет конкретный тестовый
файл, точку входа и ожидаемый результат; у защитных случаев (AC1 — коды
причин «нет данных», AC2 — отклонение искажённого формата, AC3/AC4 —
неизменность старых якорей/блока, AC6 — счёт только по машинному блоку)
названо «чем краснеет» прямо в таблице и/или в разделе «План автотестов» —
требование §2.7 о непустом третьем столбце для защитного AC выполнено
текстуально, хотя формально это код-ревью просит таблицу «AC · доказано ·
краснеет»; здесь эквивалент есть в объединённом виде.
- **Явный блок «Принято предположительно»** разделяет продуктовые решения (нет:
весь интерфейс — внутренний, процессный) от технических допущений (имена
ключей, канал передачи, формат «нет данных», позиция строки в ship-документе,
источник чтения в process-metrics, `if: always()`, свобода в именовании
модуля/функций) — ревьюер эти допущения не оспаривает: они согласуются с уже
принятыми прецедентами (`duration_seconds` как job output, `#556` граница
доверия, принцип «один модуль — одна сборка/разбор формата» уже применяется
в `review-doc-guard.mjs`/`ship-review.mjs`).
- **Открытых продуктовых вопросов владельцу нет** — и это оправдано: весь
интерфейс задачи процессный (комментарий HTML в машинном блоке, который
GitHub не рендерит, плюс строка отчёта `process-metrics`), персон
`docs/SCOPE.md` она не касается никак; ни один вопрос ТЗ не маскирует
техническое решение под продуктовое.
- **Откат и риски реалистичны**: откат — ревертом коммитов в `dev`, уже
опубликованные строки остаются (миграции нет, это явно принято), риск
«тело конвейера читается из dev в момент события» снят `continue-on-error` и
тем, что публикация на любой строке не падает, а пишет `reason=invalid`.
## Чего не проверял
- **Не проверялось исполнением** — кода ещё нет, это ревью ТЗ, а не ревью кода;
не было ни одного вызова `npx tsc`, `npm test`, `npm run build`,
`smoke-select.mjs`, `golden:verify`, `pytest tests_backend`,
`invariants` — ни один из них не применим на этапе spec (нет диффа продукта),
это не пропуск гейта, а корректный для этапа объём (§8, «Объём гейтов
соразмерен задаче»).
- Не проверялась эксплуатационная точность комментария `run-claude-sdk.ts:94` в
построчной нумерации — проверена по содержимому (grep -n), не по точному
индексу символа; расхождение на ±1 строку не меняет сути цитаты.
- Не проверялось поведение `claude-code-action` в проде (реальный прогон на CI
с реальной сессией) — только статический разбор исходника на пине. Первый
живой прогон — наблюдение, а не AC, как сама задача и указывает в разделе
«Риски».
- Не оценивалась цена «секунда на шаг CI» эмпирически — принято как заявленная
экспертная оценка автора, проверить её может только первый живой прогон.
- Не проверялись архивные спецификсации в `docs/specs/` — задача создана после
2026-09-10, ТЗ по правилу живёт только в теле issue.
## Вердикт
Зелёный. ТЗ полно по §7.1, каждый AC проверяем и привязан к названному
автотесту с указанным способом провала, технические допущения отделены от
продуктовых и не требуют эскалации владельцу, а фактические утверждения о
текущем состоянии репозитория и о пином внешней зависимости подтверждены
построчным чтением обоих источников, а не приняты на веру.
---
<!-- material-anchors: сгенерировано конвейером (#414) -->
## Материал раунда
- Ветка: `dev`, коммит `3b9f25ea659c` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
- Дерево материала: `e94ca566360e026e50ecff7594279f9db87cfbbc`
```
git log --all --format='%H %T' | grep e94ca566360e
```
- Тело issue: `cd1cf82917e9f72b048ec5382cfacfe8d1d1609e4b972a18c1dad2912a3987c3`
- Вердикт конвейера: `green` · High 0 · маршрут `fix`