Skip to content

Conversation

narengogi
Copy link
Collaborator

@narengogi narengogi commented Jul 4, 2025

#1210

Here is an example usage object from bedrock (anthropic model) when cache control is used

"usage": {
  "prompt_tokens": 368,
  "completion_tokens": 19,
  "total_tokens": 10923,
  "cache_read_input_tokens": 10536,
  "cache_creation_input_tokens": 0
}

untransformed

            "usage": {
                "cacheReadInputTokenCount": 10536,
                "cacheReadInputTokens": 10536,
                "cacheWriteInputTokenCount": 0,
                "cacheWriteInputTokens": 0,
                "inputTokens": 368,
                "outputTokens": 19,
                "totalTokens": 10923
            }

openai sends all the input tokens together in prompt_tokens, so the current implementation is not compliant and could cause problems with cost calculation

reference:
https://docs.aws.amazon.com/bedrock/latest/APIReference/API_runtime_TokenUsage.html
https://platform.openai.com/docs/api-reference/chat/object

@narengogi narengogi requested review from VisargD and b4s36t4 July 4, 2025 10:34
Copy link
Contributor

matter-code-review bot commented Jul 4, 2025

Code Quality bug fix

Description

Summary By MatterAI MatterAI logo

🔄 What Changed

This Pull Request addresses an issue with token calculation for Bedrock models, specifically when cache tokens are present. The BedrockChatCompleteResponseTransform and BedrockChatCompleteStreamChunkTransform functions have been updated to correctly include cacheReadInputTokens and cacheWriteInputTokens in the prompt_tokens calculation. Additionally, a new prompt_tokens_details object, including cached_tokens, has been introduced to provide more granular information. The src/providers/types.ts file has been modified to include these new usage detail fields, ensuring type safety and consistency.

🔍 Impact of the Change

The primary impact of this change is to ensure that Bedrock model token usage reporting is compliant with OpenAI's prompt_tokens definition, which aggregates all input tokens. This correction prevents potential inaccuracies in cost calculation and provides a more consistent token usage reporting experience across different models, especially when caching mechanisms are utilized.

📁 Total Files Changed

2 files changed: src/providers/bedrock/chatComplete.ts, src/providers/types.ts.

🧪 Test Added

N/A - No explicit test details were provided in the pull request description.

🔒Security Vulnerabilities

No security vulnerabilities were detected in the provided code changes.

Motivation

openai sends all the input tokens together in prompt_tokens, so the current implementation is not compliant and could cause problems with cost calculation

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)

How Has This Been Tested?

  • Unit Tests
  • Integration Tests
  • Manual Testing

Screenshots (if applicable)

N/A

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Related Issues

#1210

Tip

Quality Recommendations

  1. Ensure that the comment regarding 'only for anthropic models and this is not openai compliant' accurately reflects the current logic. The code now sends these fields if cache tokens are present, which might contradict the 'not openai compliant' part if the goal is strict OpenAI compliance for all fields. If the intent is to provide Bedrock-specific cache details when available, the comment could be rephrased for clarity.

Sequence Diagram

sequenceDiagram
    participant B as Bedrock API
    participant R as BedrockChatCompleteResponseTransform
    participant S as BedrockChatCompleteStreamChunkTransform
    participant C as Client/Consumer

    B-->>R: Response Object (response)
    activate R
    R->>R: Extract cacheReadInputTokens, cacheWriteInputTokens from response.usage
    R->>R: Calculate prompt_tokens = response.usage.inputTokens + cacheReadInputTokens + cacheWriteInputTokens
    R->>R: Add prompt_tokens_details.cached_tokens = cacheReadInputTokens
    R->>R: Conditionally add cache_read_input_tokens, cache_creation_input_tokens if cache tokens > 0
    R-->>C: Transformed Usage Object (OpenAI compliant)
    deactivate R

    B-->>S: Stream Chunk (parsedChunk)
    activate S
    S->>S: Extract cacheReadInputTokens, cacheWriteInputTokens from parsedChunk.usage
    S->>S: Calculate prompt_tokens = parsedChunk.usage.inputTokens + cacheReadInputTokens + cacheWriteInputTokens
    S->>S: Add prompt_tokens_details.cached_tokens = cacheReadInputTokens
    S->>S: Conditionally add cache_read_input_tokens, cache_creation_input_tokens if cache tokens > 0
    S-->>C: Transformed Stream Chunk (OpenAI compliant)
    deactivate S

    Note over C: Updated types.ts for new usage fields
Loading

Copy link
Contributor

@matter-code-review matter-code-review bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR correctly fixes token calculation for Bedrock models when cache tokens are present. The changes look good overall, but I have a few suggestions to improve the implementation.

Copy link
Contributor

Important

PR Review Skipped

PR review skipped as per the configuration setting. Run a manually review by commenting /matter review

💡Tips to use Matter AI

Command List

  • /matter summary: Generate AI Summary for the PR
  • /matter review: Generate AI Reviews for the latest commit in the PR
  • /matter review-full: Generate AI Reviews for the complete PR
  • /matter release-notes: Generate AI release-notes for the PR
  • /matter : Chat with your PR with Matter AI Agent
  • /matter remember : Generate AI memories for the PR
  • /matter explain: Get an explanation of the PR
  • /matter help: Show the list of available commands and documentation
  • Need help? Join our Discord server: https://discord.gg/fJU5DvanU3

Copy link
Contributor

Important

PR Review Skipped

PR review skipped as per the configuration setting. Run a manually review by commenting /matter review

💡Tips to use Matter AI

Command List

  • /matter summary: Generate AI Summary for the PR
  • /matter review: Generate AI Reviews for the latest commit in the PR
  • /matter review-full: Generate AI Reviews for the complete PR
  • /matter release-notes: Generate AI release-notes for the PR
  • /matter : Chat with your PR with Matter AI Agent
  • /matter remember : Generate AI memories for the PR
  • /matter explain: Get an explanation of the PR
  • /matter help: Show the list of available commands and documentation
  • Need help? Join our Discord server: https://discord.gg/fJU5DvanU3

Copy link
Contributor

Important

PR Review Skipped

PR review skipped as per the configuration setting. Run a manually review by commenting /matter review

💡Tips to use Matter AI

Command List

  • /matter summary: Generate AI Summary for the PR
  • /matter review: Generate AI Reviews for the latest commit in the PR
  • /matter review-full: Generate AI Reviews for the complete PR
  • /matter release-notes: Generate AI release-notes for the PR
  • /matter : Chat with your PR with Matter AI Agent
  • /matter remember : Generate AI memories for the PR
  • /matter explain: Get an explanation of the PR
  • /matter help: Show the list of available commands and documentation
  • Need help? Join our Discord server: https://discord.gg/fJU5DvanU3

@narengogi narengogi linked an issue Jul 9, 2025 that may be closed by this pull request
@VisargD VisargD merged commit a17cf29 into main Jul 10, 2025
2 checks passed
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.

Bedrock usage object is incorrect when cache read tokens are present

3 participants