-
Notifications
You must be signed in to change notification settings - Fork 140
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for Bedrock Converse API (Anthropic Messages API, Claude 3.5 Sonnet) #2851
Conversation
I need a jpeg file and a pdf file for testing image and document contents and they are failing the utf-8 content checks. I see that in the core, the ingest plugin has this:
I can add that to plugin/build.gradle. |
@austintlee just a few comments left but LGTM overall. |
static class DummyStreamOutput extends StreamOutput { | ||
|
||
List<String> list = new ArrayList<>(); | ||
List<Integer> intValues = new ArrayList<>(); | ||
|
||
@Override | ||
public void writeString(String str) { | ||
System.out.println("Adding string: " + str); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a leftover? remove this line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's just for debugging. This is only in a test.
@@ -136,6 +136,19 @@ protected Map<String, String> getInputParameters(ChatCompletionInput chatComplet | |||
chatCompletionInput.getContexts() | |||
) | |||
); | |||
} else if (chatCompletionInput.getModelProvider() == ModelProvider.BEDROCK_CONVERSE) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of keep adding new "else if" blocks here, why not just define an annotation for different methods and use reflection to process different models servers at runtime? In that way I think we could reduce the code complexity here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the time constraint, I would do the refactoring at a later time. I have been thinking about a good way to avoid this style of handling different cases of LLM vendors and APIs, but I was waiting for some general patterns to emerge which I think is this Message API, but again there are still small differences between LLM providers.
Yes, I'll produce some documentation, but I don't have time this week. You can look at the IT tests to get a sense of the syntax for now if that helps with reviewing and making sense of the code. |
} | ||
|
||
JsonArray messageArray = new JsonArray(); | ||
MessageArrayBuilder bldr = new MessageArrayBuilder(provider); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bldr
is hard to understand , use more meaningful variable name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure.
|
Is this an issue with GH setup? |
This is not my test. Is this a known issue? Some (internal) model got removed? Am I the only one seeing this issue? |
Also, all these time-outs we were seeing - I don't see this happening on Windows or Mac. Should I try to repro it on linux? It's really frustrating that I can't repro it locally. |
That test flaky is because of the congestion in the ITs running environment. We should remove some outdated or duplicate ITs to unchoke. That multi modal IT should update to use auto deploy so it won’t fail due to model not found because it will be auto deployed in that case. |
Can we merge this? |
The backport to
To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-2851-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 17e81ae618bebc72b8d3cb76d57f1556b1d8c8e1
# Push it to GitHub
git push --set-upstream origin backport/backport-2851-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x Then, create a pull request where the |
The backport to
To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.17 2.17
# Navigate to the new working tree
cd .worktrees/backport-2.17
# Create a new branch
git switch --create backport/backport-2851-to-2.17
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 17e81ae618bebc72b8d3cb76d57f1556b1d8c8e1
# Push it to GitHub
git push --set-upstream origin backport/backport-2851-to-2.17
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.17 Then, create a pull request where the |
…3.5 Sonnet) (opensearch-project#2851) * Add support for Anthropic Message API (Issue 2826) Signed-off-by: Austin Lee <austin@aryn.ai> * Fix a bug. Signed-off-by: Austin Lee <austin@aryn.ai> * Add unit tests, improve coverage, clean up code. Signed-off-by: Austin Lee <austin@aryn.ai> * Allow pdf and jpg files for IT tests for multimodel conversation API testing. Signed-off-by: Austin Lee <austin@aryn.ai> * Fix spotless check issues. Signed-off-by: Austin Lee <austin@aryn.ai> * Update IT to work with session tokens. Signed-off-by: Austin Lee <austin@aryn.ai> * Fix MLRAGSearchProcessorIT not to extend RestMLRemoteInferenceIT. Signed-off-by: Austin Lee <austin@aryn.ai> * Use suite specific model group name. Signed-off-by: Austin Lee <austin@aryn.ai> * Disable tests that require futher investigation. Signed-off-by: Austin Lee <austin@aryn.ai> * Skip two additional tests with time-outs. Signed-off-by: Austin Lee <austin@aryn.ai> * Restore a change from RestMLRemoteInferenceIT. Signed-off-by: Austin Lee <austin@aryn.ai> --------- Signed-off-by: Austin Lee <austin@aryn.ai> (cherry picked from commit 17e81ae)
…3.5 Sonnet) (opensearch-project#2851) * Add support for Anthropic Message API (Issue 2826) Signed-off-by: Austin Lee <austin@aryn.ai> * Fix a bug. Signed-off-by: Austin Lee <austin@aryn.ai> * Add unit tests, improve coverage, clean up code. Signed-off-by: Austin Lee <austin@aryn.ai> * Allow pdf and jpg files for IT tests for multimodel conversation API testing. Signed-off-by: Austin Lee <austin@aryn.ai> * Fix spotless check issues. Signed-off-by: Austin Lee <austin@aryn.ai> * Update IT to work with session tokens. Signed-off-by: Austin Lee <austin@aryn.ai> * Fix MLRAGSearchProcessorIT not to extend RestMLRemoteInferenceIT. Signed-off-by: Austin Lee <austin@aryn.ai> * Use suite specific model group name. Signed-off-by: Austin Lee <austin@aryn.ai> * Disable tests that require futher investigation. Signed-off-by: Austin Lee <austin@aryn.ai> * Skip two additional tests with time-outs. Signed-off-by: Austin Lee <austin@aryn.ai> * Restore a change from RestMLRemoteInferenceIT. Signed-off-by: Austin Lee <austin@aryn.ai> --------- Signed-off-by: Austin Lee <austin@aryn.ai> (cherry picked from commit 17e81ae)
I opened #2914 to bring back the rest of the tests. |
Doc - opensearch-project/documentation-website#8195. I don't have a PR, yet. Do I need to prepare a PR or is it someone from the docs team? |
…3.5 Sonnet) (#2851) (#2912) * Add support for Anthropic Message API (Issue 2826) Signed-off-by: Austin Lee <austin@aryn.ai> * Fix a bug. Signed-off-by: Austin Lee <austin@aryn.ai> * Add unit tests, improve coverage, clean up code. Signed-off-by: Austin Lee <austin@aryn.ai> * Allow pdf and jpg files for IT tests for multimodel conversation API testing. Signed-off-by: Austin Lee <austin@aryn.ai> * Fix spotless check issues. Signed-off-by: Austin Lee <austin@aryn.ai> * Update IT to work with session tokens. Signed-off-by: Austin Lee <austin@aryn.ai> * Fix MLRAGSearchProcessorIT not to extend RestMLRemoteInferenceIT. Signed-off-by: Austin Lee <austin@aryn.ai> * Use suite specific model group name. Signed-off-by: Austin Lee <austin@aryn.ai> * Disable tests that require futher investigation. Signed-off-by: Austin Lee <austin@aryn.ai> * Skip two additional tests with time-outs. Signed-off-by: Austin Lee <austin@aryn.ai> * Restore a change from RestMLRemoteInferenceIT. Signed-off-by: Austin Lee <austin@aryn.ai> --------- Signed-off-by: Austin Lee <austin@aryn.ai> (cherry picked from commit 17e81ae)
…3.5 Sonnet) (#2851) (#2913) * Add support for Anthropic Message API (Issue 2826) Signed-off-by: Austin Lee <austin@aryn.ai> * Fix a bug. Signed-off-by: Austin Lee <austin@aryn.ai> * Add unit tests, improve coverage, clean up code. Signed-off-by: Austin Lee <austin@aryn.ai> * Allow pdf and jpg files for IT tests for multimodel conversation API testing. Signed-off-by: Austin Lee <austin@aryn.ai> * Fix spotless check issues. Signed-off-by: Austin Lee <austin@aryn.ai> * Update IT to work with session tokens. Signed-off-by: Austin Lee <austin@aryn.ai> * Fix MLRAGSearchProcessorIT not to extend RestMLRemoteInferenceIT. Signed-off-by: Austin Lee <austin@aryn.ai> * Use suite specific model group name. Signed-off-by: Austin Lee <austin@aryn.ai> * Disable tests that require futher investigation. Signed-off-by: Austin Lee <austin@aryn.ai> * Skip two additional tests with time-outs. Signed-off-by: Austin Lee <austin@aryn.ai> * Restore a change from RestMLRemoteInferenceIT. Signed-off-by: Austin Lee <austin@aryn.ai> --------- Signed-off-by: Austin Lee <austin@aryn.ai> (cherry picked from commit 17e81ae)
@austintlee Our workflow is that the feature developer puts up a documentation PR in the doc repo and then the doc team reviews. Please feel free to ask any questions. Thank you! |
… Claude 3.5 Sonnet) (opensearch-project#2851) (opensearch-project#2913)" This reverts commit ed37690.
@@ -168,6 +207,7 @@ public GenerativeQAParameters(StreamInput input) throws IOException { | |||
this.interactionSize = input.readInt(); | |||
this.timeout = input.readInt(); | |||
this.llmResponseField = input.readOptionalString(); | |||
this.llmMessages.addAll(input.readList(MessageBlock::new)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are introducing a new field into bytestream here, which needs to do a version control here to prevent bwc failing
@@ -197,6 +238,7 @@ public void writeTo(StreamOutput out) throws IOException { | |||
out.writeInt(interactionSize); | |||
out.writeInt(timeout); | |||
out.writeOptionalString(llmResponseField); | |||
out.writeList(llmMessages); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
Description
Adds support for latest Message API
Related Issues
Resolves #2826
Check List
--signoff
.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.