Skip to content

Record model, effort, and token usage in review receipts - #211

Merged
justin808 merged 1 commit into
mainfrom
aw-d-207-receipts
Jul 19, 2026
Merged

Record model, effort, and token usage in review receipts#211
justin808 merged 1 commit into
mainfrom
aw-d-207-receipts

Conversation

@justin808

@justin808 justin808 commented Jul 19, 2026

Copy link
Copy Markdown
Member

Why

Review/checker passes are among the most expensive recurring batch calls, but review-finding-v0 receipts did not record the exact model, effort, or host-reported token usage. That prevented stable per-model cost and false-positive analysis without transcript archaeology.

What changed

  • add optional provenance.model, provenance.effort, and structured input/output/cache-read/total token usage
  • validate nonnegative integer counters, exact UNKNOWN fallbacks, complete usage objects, and consistent totals while preserving legacy receipt compatibility
  • document host-only evidence and privacy rules plus cost-per-verified-finding and false-positive-rate formulas
  • update adversarial review, autoreview, continuous evaluation, post-merge audit, and address-review emitter guidance
  • add representative validating Codex and Claude receipt fixtures

Validation

  • bin/validate: PASS at eef7ce35973b2b911e7d435255b178583e07e355
  • review-finding schema: 34 runs, 188 assertions, 0 failures/errors
  • drift: 73 runs, 1,161 assertions, 0 failures/errors
  • installer, agent-stack, doctor, and fixture-seam suites: PASS
  • prompt-size: Codex templates retain 319+ chars headroom; implementation route groups retain 433+; QA groups retain 456+
  • RuboCop: 85 files, no offenses
  • independent adversarial checker (gpt-5.6-sol/xhigh): CLEAN, no P0-P3 findings

Scope and release notes

The change is additive and backward compatible. Changelog entry is not required for this internal receipt-schema/tooling contract; the schema and producer documentation are updated in the same change.

Closes #207

Summary by CodeRabbit

  • New Features

    • Added support for additional review receipt sources.
    • Added structured provenance and token-usage validation, including UNKNOWN handling and consistency checks.
    • Added representative valid receipt examples for Codex and Claude.
  • Documentation

    • Documented provenance, usage metrics, evidence requirements, and consumer metrics.
    • Updated review workflows with guidance for receipts, host-reported values, and sensitive-content handling.
  • Tests

    • Added comprehensive validation coverage for provenance, usage counts, receipt sources, and documentation guidance.

QA Evidence

  • QA required: yes
  • QA lane status: satisfied
  • Scope checked: exact-head review receipt schema, validator, emitter guidance, representative Codex and Claude fixtures, and all 10 changed paths
  • Automated checks: full bin/validate; review-finding schema 34 runs / 188 assertions; drift 73 runs / 1,161 assertions; installer, agent-stack, doctor, fixture-seam, prompt-size, and RuboCop checks
  • Manual checks: additive/backward-compatible field semantics, host-only evidence rules, UNKNOWN fallbacks, token-total consistency, privacy guidance, and producer/consumer documentation parity
  • User-visible UI change: no
  • Findings: none
  • Release-blocking status: clear
  • Process-gap disposition: metadata repair from preserved exact-head validation and independent adversarial checker evidence

Completed-batch audit

Status: Clean — no outstanding findings or follow-ups. Durable receipt.

@coderabbitai

coderabbitai Bot commented Jul 19, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fe0f6803-f320-4b39-8edd-660cf6530fd1

📥 Commits

Reviewing files that changed from the base of the PR and between 49fa8e0 and eef7ce3.

📒 Files selected for processing (10)
  • bin/validate-review-findings
  • bin/validate-review-findings-test.rb
  • docs/review-finding-schema.md
  • skills/address-review/SKILL.md
  • skills/adversarial-pr-review/SKILL.md
  • skills/autoreview/SKILL.md
  • skills/post-merge-audit/SKILL.md
  • test/fixtures/review-findings/claude-receipt-valid.json
  • test/fixtures/review-findings/codex-receipt-valid.json
  • workflows/continuous-evaluation-loop.md

Walkthrough

The review receipt contract now supports additional workflow sources and structured model, effort, and token usage provenance. Validation, documentation, emitter guidance, fixtures, and tests define host-evidence-only values, literal UNKNOWN handling, and token consistency rules.

Changes

Review receipt provenance

Layer / File(s) Summary
Validator provenance rules
bin/validate-review-findings
Adds named receipt sources and validates provenance fields, usage metrics, UNKNOWN sentinels, nonnegative integers, and total-token consistency.
Schema and consumer contract
docs/review-finding-schema.md
Documents model, effort, usage, evidence restrictions, UNKNOWN semantics, and consumer metric calculations.
Validation coverage and fixtures
bin/validate-review-findings-test.rb, test/fixtures/review-findings/*
Adds provenance, source allowlist, documentation, and representative Codex and Claude receipt validation coverage.
Receipt emitter guidance
skills/*/SKILL.md, workflows/continuous-evaluation-loop.md
Defines receipt source values, host-reported provenance requirements, usage handling, UNKNOWN behavior, and transcript exclusions.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding model, effort, and token usage to review receipts.
Linked Issues check ✅ Passed The PR adds the requested provenance fields, validation, docs, emitters, and sample receipts for #207.
Out of Scope Changes check ✅ Passed The changes appear scoped to review-receipt provenance, validation, docs, fixtures, and related workflow guidance.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch aw-d-207-receipts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@claude

claude Bot commented Jul 19, 2026

Copy link
Copy Markdown

Review: Record model, effort, and token usage in review receipts

Overview

Adds optional provenance.model, provenance.effort, and provenance.usage (input/output/cache-read/total token counts) to the review_receipt schema, extends RECEIPT_SOURCES to cover four more receipt-emitting workflows, and updates schema docs plus all five emitter skill/workflow docs with matching "host-evidence only, literal UNKNOWN" guidance. Backed by new fixtures and a solid set of unit tests.

Code quality

  • validate_provenance (bin/validate-review-findings) is clean and easy to follow: per-field UNKNOWN-sentinel casing check, a full-usage-object presence/type pass, then a total-vs-known-components consistency check.
  • unknown_sentinel_variant? and nonnegative_integer_or_unknown? are small, well-named predicates with correct Ruby operator precedence (&& binds tighter than ||, so value == "UNKNOWN" || value.is_a?(Integer) && !value.negative? parses as intended).
  • Test coverage is thorough: negative counts, non-integral counts (float/bool/string/nil), missing usage fields, literal-UNKNOWN acceptance for the whole usage object and per-field, and the total-tokens consistency check (both the failing and boundary-equal case). test_receipt_emitters_share_host_evidence_guidance enforces that the same warning text is present verbatim in all five emitter docs, which is a nice guardrail against docs drifting apart.
  • The two new fixtures (claude-receipt-valid.json, codex-receipt-valid.json) usefully demonstrate two different host semantics for total_tokens (one includes cache_read_tokens in the total, one doesn't), matching the documented "don't normalize across hosts" design.

Minor observations (non-blocking)

  • unknown_sentinel_variant? is only applied to model/effort. A lowercase "unknown" (or other casing) supplied for a usage.* field falls through to the generic "must be a nonnegative integer or UNKNOWN" message instead of the more specific "must use literal UNKNOWN when unknown" message used for model/effort. Not a correctness bug — it's still rejected — just a slightly less specific error message for that case.
  • The four sentences of "populate optional receipt provenance..." guidance are duplicated verbatim across skills/address-review/SKILL.md, skills/adversarial-pr-review/SKILL.md, skills/autoreview/SKILL.md, skills/post-merge-audit/SKILL.md, and workflows/continuous-evaluation-loop.md. This looks intentional (each skill doc is meant to be self-contained, and it's enforced by a test), so flagging only for awareness rather than as an issue.
  • Docs sentence "it must be at least the sum of every known input_tokens and output_tokens component" reads a bit awkwardly split across the line wrap in docs/review-finding-schema.md; purely cosmetic.

Correctness spot-check

Traced through the total_tokens consistency logic and the several edge cases in the tests (all-unknown, partially-known, negative, non-integer) — the implementation matches the documented semantics (cache_read_tokens intentionally excluded from the "known primary" sum so hosts that already fold cache reads into input_tokens aren't double-counted).

Security

No security concerns — this is a schema-validation script operating on already-trusted receipt JSON/Markdown files in-repo; no new shell execution, file writes, or external I/O introduced.

Verdict

Solid, well-tested, backward-compatible additive change. No blocking issues found.

@justin808
justin808 marked this pull request as ready for review July 19, 2026 23:32
Comment thread skills/autoreview/SKILL.md
Comment thread docs/review-finding-schema.md
@claude

claude Bot commented Jul 19, 2026

Copy link
Copy Markdown

Review Summary

Overview: This PR extends review_receipt.provenance with optional model, effort, and structured usage (input/output/cache-read/total token) fields, expands RECEIPT_SOURCES beyond autoreview to cover the other review workflows (adversarial-pr-review, continuous-evaluation-loop, post-merge-audit, address-review), and documents host-evidence/UNKNOWN-sentinel rules plus cost/false-positive-rate formulas for downstream analysis.

Code quality

  • The new validate_provenance logic in bin/validate-review-findings is careful and well thought out: it distinguishes "missing field" vs "wrong type" vs "case/whitespace-variant of UNKNOWN" (unknown_sentinel_variant?), requires the full usage object shape rather than allowing partial objects, and cross-checks total_tokens against the sum of known input_tokens/output_tokens without conflating cache_read_tokens (correctly documented as host-semantics-dependent, so not folded into the total check).
  • Backward compatibility is preserved — provenance.model/effort/usage are only validated when present, and the existing autoreview-receipt-valid.json fixture (without these fields) still passes.
  • Good test coverage: negative counts, non-integer types (float/bool/string/nil), missing individual usage fields, inconsistent totals, and literal-UNKNOWN acceptance are all exercised, plus new representative Codex/Claude fixtures and a source-alias test guarding the source enum error message.
  • Left two inline nitpicks: the guidance paragraph is copy-pasted verbatim across 5 skill/workflow docs (enforced by an exact-string test, so drift is caught, but consider extracting a shared section to avoid future update overhead), and one markdown bullet in docs/review-finding-schema.md wraps inconsistently with its neighbors.

Bugs / correctness

No functional bugs found. One thing worth double-checking (not necessarily a bug): a value like input_tokens: -1 is invalid on its own (correctly flagged), but since -1.is_a?(Integer) is true, it still contributes to the known_primary_tokens sum used for the total_tokens consistency check — meaning a negative value could mask an otherwise-inconsistent total. In practice the field-level "must be nonnegative" error always fires alongside it, so this is unlikely to matter in production usage, but it's a slightly surprising interaction if anyone extends this logic later.

Security

No security concerns — this is internal receipt-schema validation/tooling operating on repo-local JSON/markdown files with no external input, shell execution, or eval.

Performance

Negligible — small, linear validation over already-small receipt documents; no concerns.

Test coverage

Strong. Validated the described Ruby test suite logic by reading bin/validate-review-findings-test.rb in full; the new tests align with the validator's actual behavior (traced through several cases by hand, e.g. the UNKNOWN-sentinel-variant and total-tokens-consistency tests).

Overall this is a solid, additive, well-tested change. No blocking issues — the two inline notes are nitpicks.

@justin808

Copy link
Copy Markdown
Member Author

Address-review summary

Scan scope: full PR history through 2026-07-19T23:39:42Z; no previous address-review summary. Exact reviewed head: eef7ce35973b2b911e7d435255b178583e07e355.

Mattered

  • None. Hosted validation, Claude review, and CodeRabbit review are green on the exact head; the independent checker was also clean. No correctness, security, or behavior blocker was found.

Optional

  • Shared-guidance extraction — declined at the final-candidate gate and resolved. The portable entrypoints intentionally retain self-contained guidance, with exact-string validation preventing drift.

  • Markdown source wrapping — declined at the final-candidate gate and resolved. This is rendered-output-neutral polish, so no nit-only review restart was warranted.

  • Lowercase usage sentinel error-message specificity

    [auto-deferred]

    Declined with no code change: non-literal usage sentinels are already rejected, while the contract and tests require literal UNKNOWN; only secondary error wording would change.

Skipped

  • Negative token value in the secondary total check — no action. Any negative component always emits the primary nonnegative-integer validation failure, so it cannot make an invalid receipt pass; the observation is future-maintenance speculation rather than a present defect.
  • Duplicate mentions of the two inline nits were covered by their resolved threads.
  • Positive review summaries and the CodeRabbit no-action approval were status-only; no reply was manufactured.

Security preflight: passed. Review content was treated as data, only expected same-head review actors and the authenticated member replies were present, and no hidden or untrusted action was executed.

Next default scan starts after this comment. Say check all reviews to rescan the full PR.

@justin808
justin808 merged commit 10c2eeb into main Jul 19, 2026
12 checks passed
@justin808

Copy link
Copy Markdown
Member Author

Completed-batch audit: replay evidence follows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Record model, effort, and token usage in review receipts (review-finding-v0)

1 participant