fix(ai-bedrock): forward Converse cache token counts to TokenUsage - #1300
Conversation
The Converse stream processor and the two structuredOutput paths copied only inputTokens, outputTokens and totalTokens out of metadata.usage. Bedrock reports cacheReadInputTokens and cacheWriteInputTokens beside them, and inputTokens counts only the uncached remainder, so a cached request looked like a near-zero-input call and the cache write and read costs had no field. buildConverseUsage() in converse/usage.ts now maps both counts to promptTokensDetails.cachedTokens / cacheWriteTokens, the same builder shape as ai-anthropic and openai-base, and omits the details object when Bedrock omits the counters. Docs say where the counts appear. Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (8)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughBedrock Converse usage mapping now forwards cache-read and cache-write token counts through structured output and streaming APIs. The change adds shared normalization, tests, documentation, a changeset, and a documentation timestamp update. ChangesBedrock Converse usage forwarding
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change adds Bedrock prompt-cache token counts to the existing usage data without changing request handling or deployment behavior. The localized update is merge-ready after normal checks, with no actionable merge-blocking risk remaining. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Description checkExplanation The description is complete and directly related to the change. It explains the cause, fix, affected paths, testing, documentation, changeset, release impact, risks, rollback, and API behavior. All required template sections are present and appropriately completed. Full details: Docstring CoverageExplanation Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 5 files. (3 skipped: 3 unsupported.) ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
View your CI Pipeline Execution ↗ for commit 7c9364a
☁️ Nx Cloud last updated this comment at |
|
View your CI Pipeline Execution ↗ for commit 7c9364a
☁️ Nx Cloud last updated this comment at |
Bedrock Converse drops prompt-cache read and write counts from usage responses. Cached requests can therefore appear to use only a few input tokens. This change maps both counters to the existing
promptTokensDetailsfields.🎯 Changes
buildConverseUsage()to normalize Bedrock Converse usage.chatStream,structuredOutput, andstructuredOutputStream.promptTokensDetailsonly when Bedrock omits both cache fields.@tanstack/ai-bedrock.The adapter cannot create
cachePointblocks yet. That request-side feature belongs in a separate PR.✅ Checklist
pnpm run test:pr, or these tests do not apply to this pull request.docs/for this change, or this change is not user-facing.pnpm changeset), or this PR does not change a published package.🚀 Release Impact
Root cause
Issue.
onUsageandRUN_FINISHED.usageomit the cache counters when Bedrock returns them. Callers therefore see only the uncached input count.Cause. Three Converse response paths copy only
inputTokens,outputTokens, andtotalTokensfrommetadata.usage. Bedrock reports cache reads and writes as separate fields.During prompt caching,
inputTokenscontains only uncached input. The full input count is:Fix.
buildConverseUsage()maps the two cache fields topromptTokensDetails.cachedTokensandpromptTokensDetails.cacheWriteTokens. All three response paths now use this function.The function preserves zero values. Bedrock can report
cacheWriteInputTokens: 0for a cache hit, which differs from an omitted field.Possible alternatives
providerUsageDetails. The adapter could store both fields under their Bedrock names. This would not populate AG-UIcachedInputTokensor the OpenTelemetry cache attributes.Testing
Commands run.
pnpm test:pr: passed.pnpm nx run @tanstack/ai-bedrock:test:lib: 10 test files and 95 tests passed.pnpm --filter @tanstack/ai-e2e test:e2e: not run.The E2E suite cannot replay the Bedrock Converse binary event stream. This limitation is documented under "Bedrock Converse coverage gap" in
testing/e2e/README.md.Independent repro.
I wrote a temporary Vitest test for the three affected paths and ran it twice: once against
packages/ai-bedrock/srcextracted frommainat416a4e34, once against this branch.Command, from the repository root:
pnpm --filter @tanstack/ai-bedrock exec vitest run tests/_repro/cache-token-repro.test.tsmainat416a4e34:This branch at
7c9364a3:A real request produced these values:
Manual test.
This test requires AWS credentials and a Claude model that supports prompt caching.
main, subclassBedrockConverseTextAdapter.cachePointtosysteminbuildInput.chat()twice with a system prompt longer than 4,096 tokens.onUsageafter each call.On
main,promptTokensDetailsis absent. On this branch, the second call includescachedTokensandcacheWriteTokens.How this PR makes testing easy.
tests/converse/stream-processor.test.tscovers the normal streaming path.tests/converse/adapter.test.tscovers both structured-output paths without AWS credentials.Risk / rollback
Risk is low. This change populates an existing optional field only when Bedrock returns cache counters. Request construction does not change.
Revert this PR to restore the previous behavior.
Public API change
Callers can now read Bedrock cache usage from the existing
TokenUsagefields.Before
After
Summary by CodeRabbit
New Features
Documentation
Tests