feat: Add usage for Bedrock Anthropic Claude models #157
Conversation
Introduce a `usage` variable to track usage information throughout the `AnthropicClaude3ChatModel` class. Add a `GetUsage` method to extract usage data from JSON nodes. Initialize `usage` to `null` and update it with data from JSON responses, ensuring it is non-null before use with a fallback to `Usage.Empty`. Update `usage` incrementally in streaming responses and include it in `ChatResponse`, passing it to `AddUsage` and `provider.AddUsage` methods. Remove `CONTENT_BLOCK_STOP` type handling from streaming response processing.
Modified the assignment of the `usage` variable to use `GetUsage(response?["usage"])` instead of `GetUsage(response)`. This change ensures that the `usage` variable is assigned from the "usage" field of the `response` object, if it exists, preventing potential null reference exceptions.
|
|
WalkthroughThe changes focus on improving usage tracking in the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/Amazon.Bedrock/src/Chat/AnthropicClaude3ChatModel.cs (2)
98-105: Consider combining usage initialization and property updates.The current implementation has multiple assignments to
usage. Consider combining them for better readability.-usage ??= Usage.Empty; -usage = usage.Value with -{ - Time = watch.Elapsed, - Messages = messages.Count, -}; +usage = (usage ?? Usage.Empty) with +{ + Time = watch.Elapsed, + Messages = messages.Count, +};
175-179: Enhance XML documentation with parameter constraints and return value details.The current documentation could be more descriptive about parameter constraints and return value composition.
/// <summary> -/// Extracts usage information from the provided JSON node. +/// Extracts usage information from the provided JSON node. Returns Usage.Empty with updated token counts if the node is null. /// </summary> /// <param name="usageNode">The JSON node containing usage information.</param> +/// <remarks> +/// The method expects the following JSON structure: +/// { +/// "input_tokens": number, +/// "output_tokens": number +/// } +/// </remarks> /// <returns>A <see cref="Usage"/> object with the extracted usage data.</returns>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Amazon.Bedrock/src/Chat/AnthropicClaude3ChatModel.cs(4 hunks)
🔇 Additional comments (2)
src/Amazon.Bedrock/src/Chat/AnthropicClaude3ChatModel.cs (2)
91-91: LGTM!The usage extraction from non-streaming response is implemented correctly.
180-193: 🛠️ Refactor suggestionConsider making the method static and implement price calculation.
The method doesn't use any instance members and could be static. Also, the price calculation is currently hardcoded to 0.0.
-private Usage GetUsage(JsonNode? usageNode) +private static Usage GetUsage(JsonNode? usageNode) { var inputTokens = usageNode?["input_tokens"]?.GetValue<int>() ?? 0; var outputTokens = usageNode?["output_tokens"]?.GetValue<int>() ?? 0; - var priceInUsd = 0.0; + // TODO: Implement price calculation based on token usage and model pricing + var priceInUsd = CalculatePrice(inputTokens, outputTokens); return Usage.Empty with { InputTokens = inputTokens, OutputTokens = outputTokens, Messages = 0, PriceInUsd = priceInUsd, }; } +private static double CalculatePrice(int inputTokens, int outputTokens) +{ + // Implement pricing logic based on Anthropic Claude 3 model rates + return 0.0; +}Would you like me to help implement the pricing calculation based on the current Anthropic Claude 3 model rates?
✅ Verification successful
The method should be static and price calculation should be implemented
The review comment is correct on both points:
- The
GetUsagemethod is purely functional and doesn't use any instance members, making it safe to be static.- Price calculation should be implemented as:
- It's a standard feature across other models in the codebase
- Other Anthropic models implement proper price calculation
- Usage tracking and billing require accurate price calculation
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Find usages of GetUsage method in the codebase echo "=== GetUsage method usages ===" rg "GetUsage\(" -A 2 -B 2 echo -e "\n=== Price calculation in chat models ===" # Look for price calculation in other chat models rg "PriceInUsd|CalculatePrice" --type cs -A 2 -B 2 echo -e "\n=== Pricing constants or utilities ===" # Search for pricing-related code rg "price|cost|rate" --type cs -A 2 -B 2Length of output: 116611
3 participants
Summary by CodeRabbit
New Features
Refactor