Skip to content

Conversation

devin-ai-integration[bot]
Copy link
Contributor

Description

This PR fixes a critical bug in the AWS Bedrock instrumentation where modelId.split(".") fails when using ARN or cross-region model IDs that contain multiple dots. The issue caused a ValueError: too many values to unpack (expected 2) crash when using model IDs like:

  • arn:aws:bedrock:us-east-1:<account_id>:inference-profile/us.anthropic.claude-3-haiku-20240307-v1:0
  • us.anthropic.claude-sonnet-4-20250514-v1:0

Changes Made

  • Fixed patch_converse_stream (line 110): Replaced (vendor, _) = modelId.split(".") with vendor, _ = parse_vendor_and_model_name_from_model_id(modelId)
  • Fixed patch_converse (line 145): Applied the same fix as above
  • Version bump: Incremented from 3.8.20 to 3.8.21 for this bug fix release
  • Consistency improvement: These functions now use the same robust parsing logic as patch_invoke_model and patch_invoke_model_with_response_stream

Root Cause

The patch_converse_stream and patch_converse functions assumed model IDs could always be split into exactly 2 parts using dots, but ARNs and cross-region model IDs contain multiple dots. The codebase already had a robust parse_vendor_and_model_name_from_model_id function that handles these cases correctly, but these two functions weren't using it.

Testing

⚠️ Testing Limitation: Local testing was limited due to missing boto3/botocore dependencies in the development environment. The existing test suite in src/tests/aws_bedrock/test_model_id_parsing.py covers the parser function but couldn't be executed locally.

A test script was created that confirmed:

  • ✅ Original modelId.split(".") method fails with "too many values to unpack" error for ARN formats
  • ✅ The parse_vendor_and_model_name_from_model_id function handles these cases correctly

Review Checklist

  • Critical: Verify fix works with ARN format: arn:aws:bedrock:us-east-1:123456789012:inference-profile/us.anthropic.claude-3-haiku-20240307-v1:0
  • Critical: Verify fix works with cross-region format: us.anthropic.claude-sonnet-4-20250514-v1:0
  • Regression testing: Confirm simple model IDs still work: anthropic.claude-3-opus-20240229
  • Edge cases: Test the parser with foundation model ARNs and custom model ARNs
  • Version: Confirm 3.8.21 is appropriate for this bug fix
  • CI: Ensure all tests pass, especially AWS Bedrock related tests

Risk Assessment

🔴 High Priority Review: This touches critical path code for AWS Bedrock instrumentation. While the fix is minimal and follows existing patterns in the codebase, thorough testing is essential to prevent regressions.


Link to Devin run: https://app.devin.ai/sessions/9b326f6380504040bf8b711ce512ab94
Requested by: karthik@scale3labs.com

obinnascale3 and others added 13 commits May 14, 2025 07:52
Co-authored-by: Obinna Okafor <obinna.okafor01@gmail.com>
* fix aws bedrock streaming bug

* bump version

* add deprecated dependency

* restrict current pinecone instrumentation to v6.0.2

* hard code boto3 dependency version
Added support for embeddings via AWS Bedrock
* Fix AWS Bedrock ARN parsing issue in converse methods

- Replace modelId.split('.') with parse_vendor_and_model_name_from_model_id
- Fixes crash when using ARN or cross-region model IDs with multiple dots
- Makes patch_converse and patch_converse_stream consistent with other methods

Resolves ValueError: too many values to unpack (expected 2) when using:
- ARN format: arn:aws:bedrock:us-east-1:<account_id>:inference-profile/us.anthropic.claude-3-haiku-20240307-v1:0
- Cross-region format: us.anthropic.claude-sonnet-4-20250514-v1:0

Co-Authored-By: karthik@scale3labs.com <karthik@scale3labs.com>

* Bump version to 3.8.21 for ARN parsing bug fix

Co-Authored-By: karthik@scale3labs.com <karthik@scale3labs.com>

---------

Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: karthik@scale3labs.com <karthik@scale3labs.com>
Co-Authored-By: karthik@scale3labs.com <karthik@scale3labs.com>
Copy link
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@karthikscale3 karthikscale3 merged commit 462a04b into main Jul 24, 2025
0 of 4 checks passed
@karthikscale3 karthikscale3 deleted the release/3.8.21-1753390801 branch July 24, 2025 21:02
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.

3 participants