fix(backends): fold Claude cache tokens into prompt_tokens total#262
Conversation
…ens total Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Correct the remaining D-15 comments/docstrings to state that prompt_tokens/input_tokens is the total input (backends fold cache read+creation) and cached_tokens is the cache-read hit subset: trajectory.py MetricsEvent handling and the CostEvent/MetricsEvent cached_tokens field docstrings in backends/__init__.py. Gate fallout: wrap the pr_comment_renderer clamp comment under 120 cols, and correct test_per_phase_rollup_distinguishes_phases which still encoded the old uncached-remainder premise (assert folded totals 5,500 / 1,500 instead of 4,000 / 1,000). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThis change updates Claude backend token accounting so cache-read and cache-creation input tokens are folded into reported total input and prompt token values. It adds a helper used for 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/test_backend_claude_metrics.py (1)
172-241: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMissing coverage for simultaneous cache-read + cache-creation.
All new/updated scenarios exercise cache-read-only or cache-creation-only usage, but never both non-zero in the same turn — a case that's realistically possible per Anthropic's API (e.g. one breakpoint hit, another written) and directly relevant given the folding logic being tested. Worth adding a case with both buckets non-zero to lock in that the sum still folds correctly and
cached_tokensstill reports only the read subset.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_backend_claude_metrics.py` around lines 172 - 241, Add a test in test_prompt_tokens_include_cache_read_and_creation that covers a single turn where both cache_read_input_tokens and cache_creation_input_tokens are non-zero, using the existing _collect_events helper and MockAssistantMessageWithUsage/MockResultMessageWithUsage flow. Verify that MetricsEvent.prompt_tokens and CostEvent.input_tokens include the sum of input_tokens + cache_read_input_tokens + cache_creation_input_tokens, while MetricsEvent.cached_tokens and CostEvent.cached_tokens only reflect the cache_read_input_tokens value.daydream/backends/claude.py (1)
85-99: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocstring's "mutually exclusive buckets" claim is inaccurate.
Anthropic's own docs show
cache_read_input_tokensandcache_creation_input_tokenscan both be non-zero in the same response (e.g. a request that hits one cache breakpoint but writes another). The summation logic here is correct regardless (it doesn't depend on exclusivity), but the docstring's claim could mislead future maintainers into assuming these are always disjoint.📝 Suggested docstring fix
- Anthropic reports `input_tokens` as the *uncached remainder* only, with - cache hits and writes split into `cache_read_input_tokens` and - `cache_creation_input_tokens` (mutually exclusive buckets). ATIF's - `Metrics.prompt_tokens` is the total input, so fold all three. Returns - None when `input_tokens` is absent (preserves the no-token-count gate). + Anthropic reports `input_tokens` as the *uncached remainder* only, with + cache hits and writes split into `cache_read_input_tokens` and + `cache_creation_input_tokens` (both can be non-zero on the same + request — e.g. one breakpoint hit, another written). ATIF's + `Metrics.prompt_tokens` is the total input, so fold all three. Returns + None when `input_tokens` is absent (preserves the no-token-count gate).Based on external documentation confirming cache read and cache-creation buckets can co-occur on a single request.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@daydream/backends/claude.py` around lines 85 - 99, The `_total_input_tokens` docstring in `daydream/backends/claude.py` incorrectly states that `cache_read_input_tokens` and `cache_creation_input_tokens` are mutually exclusive. Update the docstring for `_total_input_tokens` to describe that Anthropic may return both cache buckets in the same response, and that the function simply sums `input_tokens`, `cache_read_input_tokens`, and `cache_creation_input_tokens` when present.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@daydream/backends/claude.py`:
- Around line 85-99: The `_total_input_tokens` docstring in
`daydream/backends/claude.py` incorrectly states that `cache_read_input_tokens`
and `cache_creation_input_tokens` are mutually exclusive. Update the docstring
for `_total_input_tokens` to describe that Anthropic may return both cache
buckets in the same response, and that the function simply sums `input_tokens`,
`cache_read_input_tokens`, and `cache_creation_input_tokens` when present.
In `@tests/test_backend_claude_metrics.py`:
- Around line 172-241: Add a test in
test_prompt_tokens_include_cache_read_and_creation that covers a single turn
where both cache_read_input_tokens and cache_creation_input_tokens are non-zero,
using the existing _collect_events helper and
MockAssistantMessageWithUsage/MockResultMessageWithUsage flow. Verify that
MetricsEvent.prompt_tokens and CostEvent.input_tokens include the sum of
input_tokens + cache_read_input_tokens + cache_creation_input_tokens, while
MetricsEvent.cached_tokens and CostEvent.cached_tokens only reflect the
cache_read_input_tokens value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e3c37a3e-7c87-45b8-9f4a-62cb75499e2c
📒 Files selected for processing (7)
daydream/backends/__init__.pydaydream/backends/claude.pydaydream/pr_comment_renderer.pydaydream/trajectory.pytests/test_backend_claude_metrics.pytests/test_pr_comment_integration.pytests/test_pr_comment_renderer.py
The _total_input_tokens docstring wrongly described cache_read_input_tokens and cache_creation_input_tokens as mutually exclusive. A single Anthropic response can read one cache breakpoint while writing another, so both may be non-zero at once. The summing code was already correct; only the docstring misled. Add a test covering the both-buckets-non-zero turn. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Problem
Token counts in review comments showed impossibly-small input values (e.g. "22 in"). The Claude backend emitted Anthropic's raw
input_tokens— the uncached remainder only — as ATIFprompt_tokens, and droppedcache_creation_input_tokensentirely. On a cache-heavy turn the displayed input collapsed to the tiny uncached tail.Fix
Fold all three Anthropic input buckets into the ATIF-defined total at both Claude backend emission sites:
This restores the invariant
cached ⊆ prompt_tokensthat the recorder and renderer clamp already assume, turning the downstreammin()clamp into a correct defensive guard rather than a value-destroyer.Changes
daydream/backends/claude.py— new module-level_total_input_tokens(usage)helper (returnsNonewheninput_tokensis absent, preserving the "no MetricsEvent without token counts" gate); wired into both theMetricsEvent.prompt_tokensandCostEvent.input_tokenssites; rewrote the emission-site comment block.daydream/pr_comment_renderer.py— reframed the clamp comment as a defensive guard (no logic change).daydream/trajectory.py,daydream/backends/__init__.py— corrected the D-15 comments/docstrings to stateprompt_tokens/input_tokensis the total input andcached_tokensis the read subset.test_prompt_tokens_include_cache_read_and_creation), new renderer regression lock (test_cached_turn_renders_total_input), and rewrote 4 tests acrosstest_backend_claude_metrics.pyandtest_pr_comment_integration.pythat encoded the old uncached-remainder premise to the corrected total-input contract.Out of scope (unchanged)
codex.py/pi.py— Codex'sinput_tokensis already a total; its comment is correct.pricing.compute_cost— no cache-creation write premium. Claude cost comes fromtotal_cost_usd, so no cost regression.Verification
make checkgreen: ruff clean,mypy daydream testsclean (251 files), 2086 passed / 5 skipped.🤖 Generated with Claude Code