Skip to content

Conversation

@Smartsheet-JB-Brown
Copy link
Contributor

@Smartsheet-JB-Brown Smartsheet-JB-Brown commented Mar 12, 2025

Fix AWS Bedrock Provider with Cost Calculations for Prompt Router ARNs

Note - 2nd PR coming in a couple hours to make AWS prices in src/shared/api.ts current. These 2 PRs together will make Roo's cost display more accurate for Bedrock users.

Overview

This PR fixes the AWS Bedrock provider to properly handle and utilize the invokedModelId information returned by AWS Bedrock Prompt Router responses. The code has been updated to use the token cost information from the invokedModelId, because at request time the inference model, and therefore token costs, are not known for prompt router ARNs. This improvement ensures more accurate cost tracking and model configuration when using AWS Bedrock's Prompt Router feature.

Problem

When using AWS Bedrock's Prompt Router feature, the actual model used for inference is not known at request time. The Prompt Router returns the actual model used via the invokedModelId field in the response stream, and the token usage values are located in a different structure than responses from individual models. Previously, our implementation wasn't properly extracting and utilizing this information, which led to:

  1. No cost tracking for prompt router ARNs
  2. Incorrect model configuration for subsequent requests
  3. Missing token usage metrics from Prompt Router responses

Changes

  • Added support for extracting and parsing the invokedModelId from AWS Bedrock Prompt Router responses
  • Implemented logic to update the model configuration based on the actual model used
  • Enhanced region handling for cross-region inference with custom ARNs
  • Added proper extraction of model names from ARNs with regional prefixes (e.g., "us." or "eu.")
  • Improved error handling for custom ARN scenarios with more descriptive error messages
  • Added comprehensive test coverage for the new functionality

Benefits

  • Accurate Cost Tracking: Costs are now calculated based on the actual model used by the Prompt Router
  • Improved Model Configuration: The provider now correctly updates its configuration based on the model selected by the Prompt Router
  • Better Error Handling: More descriptive error messages for common issues with custom ARNs
  • Enhanced Region Support: Better handling of cross-region inference scenarios

Testing

Added new test files and expanded existing tests:

  • bedrock-invokedModelId.test.ts: Tests for proper handling of the invokedModelId field
  • Enhanced bedrock-custom-arn.test.ts: Tests for custom ARN validation and region extraction
  • Updated bedrock.test.ts: Added tests for invokedModelId handling in various scenarios

Technical Details

The key technical changes include:

  1. Enhanced the StreamEvent interface to include the trace.promptRouter.invokedModelId field
  2. Added logic to extract the model name from the invokedModelId ARN
  3. Implemented the costModelConfig property to track the actual model used
  4. Updated the getModel method to return the correct model configuration
  5. Added special handling for region prefixes in model names (us./eu.)
  6. Improved error messages for common ARN-related issues

This implementation ensures that when AWS Bedrock's Prompt Router selects a different model than initially requested, our system correctly identifies and uses that model's configuration for accurate cost tracking and proper functionality.


Important

Enhances AWS Bedrock provider to accurately handle cost calculations and model configurations for prompt router ARNs, with improved error handling and comprehensive testing.

  • Behavior:
    • Updates AwsBedrockHandler in bedrock.ts to handle invokedModelId for accurate cost tracking and model configuration.
    • Adds logic to extract model name from invokedModelId and update costModelConfig.
    • Improves error handling for custom ARNs with descriptive messages.
  • Models:
    • Updates AWS Bedrock model definitions in api.ts with latest pricing and new models.
    • Adds supportsComputerUse flag to relevant models.
  • Testing:
    • Adds bedrock-invokedModelId.test.ts for invokedModelId handling.
    • Updates bedrock-createMessage.test.ts and bedrock.test.ts for custom ARN and cross-region inference scenarios.
  • Misc:
    • Mocks logger in jest.setup.ts for consistent test logging.

This description was created by Ellipsis for d8df9a5. It will automatically update as commits are pushed.

@changeset-bot
Copy link

changeset-bot bot commented Mar 12, 2025

⚠️ No Changeset found

Latest commit: 4ada518

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. bug Something isn't working labels Mar 12, 2025
@mrubens
Copy link
Collaborator

mrubens commented Mar 13, 2025

Think we can clean up some of the logger.debugs? And if the tests that rely on them are important, update them to be based on the behavior or data?

jest.mock("../utils/logging", () => ({
logger: {
debug: jest.fn(),
debug: jest.fn().mockImplementation((message, meta) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we revert the changes to this file and mock debug within specific tests if necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, done

Smartsheet-JB-Brown and others added 2 commits March 13, 2025 08:09
yes, unneeded remnant

Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
agree, sorry old Javascript habits

Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
@Smartsheet-JB-Brown Smartsheet-JB-Brown force-pushed the jbbrown/bedrock_cost_intelligent_prompt_routing branch from 100d0d3 to e866079 Compare March 13, 2025 16:57
@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:XXL This PR changes 1000+ lines, ignoring generated files. labels Mar 13, 2025
@Smartsheet-JB-Brown Smartsheet-JB-Brown force-pushed the jbbrown/bedrock_cost_intelligent_prompt_routing branch from e866079 to 8d30b6f Compare March 13, 2025 17:00
@Smartsheet-JB-Brown Smartsheet-JB-Brown force-pushed the jbbrown/bedrock_cost_intelligent_prompt_routing branch from c8ac41e to 3e15c4e Compare March 13, 2025 18:22
Comment on lines 10 to 12
Copy link
Collaborator

Choose a reason for hiding this comment

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

Think we can revert this one as well?

@Smartsheet-JB-Brown Smartsheet-JB-Brown force-pushed the jbbrown/bedrock_cost_intelligent_prompt_routing branch from 3e15c4e to 6c74e9b Compare March 13, 2025 20:04
Copy link
Collaborator

@mrubens mrubens left a comment

Choose a reason for hiding this comment

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

Looks good!

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Mar 13, 2025
@mrubens mrubens merged commit fbdf758 into RooCodeInc:main Mar 13, 2025
9 checks passed
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Mar 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer size:XL This PR changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants