houseplan-card/docs/reviews/SPEC-REVIEW-737-r1.md

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 проверяем и привязан к названному автотесту с указанным способом провала, технические допущения отделены от продуктовых и не требуют эскалации владельцу, а фактические утверждения о текущем состоянии репозитория и о пином внешней зависимости подтверждены построчным чтением обоих источников, а не приняты на веру.

---

Материал раунда

• Ветка: dev, коммит 3b9f25ea659c — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: e94ca566360e026e50ecff7594279f9db87cfbbc

  git log --all --format='%H %T' | grep e94ca566360e
  

• Тело issue: cd1cf82917e9f72b048ec5382cfacfe8d1d1609e4b972a18c1dad2912a3987c3
• Вердикт конвейера: green · High 0 · маршрут fix