DUX-7485: Capture streaming usage tokens and context management for Bedrock InvokeModel#11
Conversation
…okeModel Add comprehensive token usage tracking for Bedrock InvokeModel streaming: - Capture cache_creation TTL breakdown (ephemeral_5m, ephemeral_1h) - Extract stop_sequence and context_management from responses - Fix input_tokens calculation to properly subtract cached tokens - Add debug logging for context_management applied_edits Why: - Bedrock now returns detailed cache TTL breakdowns in usage metadata - context_management signals when the service auto-edits conversation history - Accurate token accounting requires subtracting cache hits from input_tokens How to apply: - Message and Tokens classes now surface new fields - StreamAccumulator plumbs them through from chunks to final message - Tests verify both non-streaming and streaming code paths Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Co-authored-by: Sam Boland <sam.boland@appfolio.com>
|
Jira Issue: https://appfolio.atlassian.net/browse/DUX-7485 |
|
🤖 Supernova Code Review — View trace |
There was a problem hiding this comment.
Review summary
The PR cleanly wires up cache/ephemeral token extraction, stop_sequence, and context_management through the streaming path and aligns them with the non-streaming path. The test coverage is solid.
One bug: input_tokens can become inconsistent with cache tokens
The concrete issue is a split-responsibility design between build_message_start_chunk and build_message_delta_chunk:
build_message_start_chunkpre-netsinput_tokens(raw − cache_read − cache_creation) and passes only the netted value through the chunk. The raw total is discarded.build_message_delta_chunk(new in this PR) independently extractscached_tokensandcache_creation_tokensfrom themessage_deltaevent, whichStreamAccumulator#count_tokensoverwrites the accumulator's@cached_tokens/@cache_creation_tokenswith.
If the message_delta event sends different cache counts than message_start — which this PR explicitly handles by reading them from both events — @input_tokens stays at its stale netted value while the cache totals change, violating:
input_tokens + cached_tokens + cache_creation_tokens == total_api_input_tokens
See inline comments on streaming.rb lines 183 and 281–284.
Recommended fix: store the raw API input_tokens in the accumulator (un-netted) and compute the net in to_message after all chunks have been accumulated:
# count_tokens: record raw, not netted
@raw_input_tokens = chunk.input_tokens + chunk.cached_tokens.to_i + chunk.cache_creation_tokens.to_i if chunk.input_tokens
# to_message: net down using final cache values
net_input = @raw_input_tokens ? [@raw_input_tokens - @cached_tokens.to_i - @cache_creation_tokens.to_i, 0].max : nilThis also requires build_message_start_chunk to pass the raw input_tokens value rather than the pre-netted one.
Note: StreamAccumulator#count_tokens (lines 165–172) is not in the diff so could not be commented inline — the coupling lives there.
| content: nil, | ||
| model_id: message['model'] || @model&.id, | ||
| input_tokens: input_tok ? [input_tok.to_i, 0].max : nil | ||
| input_tokens: input_tok ? [input_tok.to_i - cache_read.to_i - cache_creation.to_i, 0].max : nil, |
There was a problem hiding this comment.
input_tokens is netted down here using the message_start cache values, and the raw total is discarded. However, build_message_delta_chunk (lines 281–284) also extracts cached_tokens and cache_creation_tokens from the message_delta event, which the StreamAccumulator may overwrite @cached_tokens/@cache_creation_tokens with later. If the message_delta event ever reports different cache counts than message_start, the final message will violate the invariant input + cached + cache_creation == total_api_input_tokens.
Fix: pass the raw input_tokens through the chunk (e.g. as a new raw_input_tokens field) and defer the net-down to StreamAccumulator#to_message, using the final accumulated cache values at assembly time.
| cached_tokens: usage['cache_read_input_tokens'], | ||
| cache_creation_tokens: usage['cache_creation_input_tokens'], | ||
| cache_creation_ephemeral_5m_tokens: cache_creation_detail['ephemeral_5m_input_tokens'], | ||
| cache_creation_ephemeral_1h_tokens: cache_creation_detail['ephemeral_1h_input_tokens'], |
There was a problem hiding this comment.
These lines extract cached_tokens and cache_creation_tokens from message_delta, which StreamAccumulator#count_tokens will then overwrite the values set by message_start. This is the other side of the inconsistency described on line 183: @input_tokens is the pre-netted value from message_start, but @cached_tokens/@cache_creation_tokens can be independently updated here. If the two events ever disagree on cache counts, the accumulator will hold an inconsistent state where the stored input_tokens no longer matches total − cached − cache_creation.
|
Fix all issues |
Queued. The fix has two parts: (1) pass raw (un-netted) |
Bedrock InvokeModel's message_start chunk reports input_tokens before a later message_delta chunk can update the cache fields. Netting at chunk build time missed cache usage that only appeared in message_delta. StreamAccumulator now defers netting to to_message when net_cache_tokens is true, which BedrockInvokeModel streaming now opts into. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
🤖 Supernova Code Review — View trace |
|
Review please |
There was a problem hiding this comment.
Reviewed the full diff across all 6 changed files. No actionable findings.
The design is sound:
- Deferred netting via
net_cache_tokens: true—StreamAccumulatorcorrectly holds@raw_input_tokensfrommessage_startand appliesmax(input − cache_read − cache_creation, 0)only atto_messagetime, so a latermessage_deltathat updates cache fields doesn't get missed. - Ephemeral sub-tokens (
cache_creation_ephemeral_5m,cache_creation_ephemeral_1h) are a breakdown of the existingcache_creation_input_tokenstotal, so excluding them fromnetted_input_tokensis correct — no double-counting. - Non-streaming parity —
chat.rb'sinput_tokenshelper applies the same semantics and the newparse_completion_bodyfields (cache_creation_ephemeral_*,stop_sequence,context_management) are wired up consistently withMessageandTokens. - Zero-value handling — Ruby's truthy semantics for
0incount_tokensmeans amessage_deltathat explicitly sendscache_read_input_tokens: 0would overwrite a prior non-zero value, which is the intended "last value wins" behaviour for Bedrock's cumulative update pattern. - Test coverage is thorough for the new streaming paths: chunk-level extraction, full accumulator round-trips (fully-cached, partial-cache, cache-only-in-delta), and
context_managementpropagation all have specs.
One minor observation (not blocking): there's no StreamAccumulator-level integration test that verifies cache_creation_ephemeral_5m/1h tokens survive the full add → to_message round trip (only the build_chunk unit test at line 530–545 covers the chunk level). The production path in count_cache_creation_ttl_tokens looks correct, but a matching accumulator-level spec alongside the one at line 614 would close the coverage gap and prevent regressions.
The fix is correct and the implementation is clean. Here's what's in the current state: The original bug is resolved. Test coverage is thorough. There are dedicated specs for:
One minor observation: the end-to-end accumulation test at line 594–612 uses Nothing blocking. The PR looks good to merge. |
Summary
This PR captures streaming usage tokens and context management for Bedrock InvokeModel, ensuring consistent token accounting across both streaming and non-streaming paths.
Changes
Token Tracking Improvements
stop_sequenceandcontext_managementto the Message class for richer response metadatacache_creation_ephemeral_5mandcache_creation_ephemeral_1hin addition to existing cache token typesnet_cache_tokensoption to defer input_tokens netting, allowing proper calculation of effective input tokens after cache fields are updated by later stream chunksBedrock InvokeModel Streaming Enhancements
streaming.rb:
net_cache_tokens: trueto handle the Bedrock pattern wheremessage_startcontainsinput_tokensbut latermessage_deltachunks update cache fieldscontext_management.applied_editsfrom message_delta eventschat.rb:
cache_creationusage fields to Messagecontext_managementfrom response dataInput Tokens Semantics
Maintains consistent semantics:
effective_input_tokens = max(input - cache_read - cache_creation, 0)across both streaming and non-streaming code paths.Test Coverage
Added comprehensive specs for:
References
Agent session: https://staging.supernova.dx.appf.io/coders/b96a7d9c-f609-466a-9a8e-e2020ad61ff8