feat: Added support for Claude 3+ Chat API in Bedrock #2870
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bizob2828
requested changes
Jan 13, 2025
There was a problem hiding this comment.
thanks for the PR @RyanKadri! A few tiny suggestions but you also have merge conflicts so I didn't do a thorough review yet.
| const chatCompletionMessage = new LlmChatCompletionMessage({ | ||
| agent, | ||
| segment, | ||
| bedrockCommand, | ||
| bedrockResponse, | ||
| isResponse: true, | ||
| index: index + 1, | ||
| index: index, |
There was a problem hiding this comment.
Suggested change
| index: index, | |
| index, |
There was a problem hiding this comment.
this is a little confusing. what about splitting vars so the prompt index can be promptIndex and then in here you have completionIndex, then this index is promptIndex + completionIndex
| content: contextMessage.content, | ||
| role: contextMessage.role, | ||
| bedrockResponse, | ||
| index: index, |
There was a problem hiding this comment.
Suggested change
| index: index, | |
| index, |
bizob2828
changed the title
|
please run |
|
Sounds good 👍. I have a couple other changes incoming for another unit test scenario I want to cover |
RyanKadri
force-pushed
the
claude-3-messages
branch
from
23c37dc to
c31dace
Compare
|
(Looking into unit tests) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
The current version of the Bedrock instrumentation is mostly geared toward supporting the Claude Text Completions API but may not split messages as expected in multi-turn conversations with the Messages API.
The Text Completions API supports a single prompt message but the Messages API, lets you provide multiple distinct messages (context and prompt) to the model. Currently the agent combines those separate context messages into a single prompt but it probably should follow the pattern we use in OpenAI instrumentation of splitting them into multiple
LlmChatCompletionMessageevents (still oneLlmChatCompletionSummaryper API call)To make things more complicated, the Messages API also allows messages to have sub-chunks to support multi-modal use-cases (Anthropic docs for how this works). For example, one conceptual multi-modal message might be
What is this animal <picture of an elephant> and where does it live?. For these cases, if a single message has multiple chunks, this PR attempts to join those together into one string representation with reasonable placeholders for non-text components. Further down the road, the AI Monitoring team needs to do a bit of work to see if we can represent those non-text chunks as placeholders in a string. For now though, we just want something to show.How to Test
The AI Monitoring sample app is running this version of the instrumentation for Anthropic models. The AI Monitoring team can help you out with where to see the data it produces
Related Issues
N/A