-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Add AWS Bedrock AI support #174
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
Conversation
368346c
to
810d2be
Compare
59dc188
to
6d0364f
Compare
@@ -0,0 +1,83 @@ | |||
# Bedrock AI Chat and Embedding Clients |
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.
The documentation for these base clients still needs to go into our reference documentation, which is in adoc format. I will create a separate issue for that.
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.
agree
/** | ||
* Converts a list of messages to a prompt for particular model. | ||
* | ||
* TODO: consider factoring out this into an interface and provide different |
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.
Create an issue instead of a TODO. Will do on merge.
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.
Rename MessageToPromptStrategy to MessageToPromptConverter
.map(this::messageToString) | ||
.collect(Collectors.joining("\n")); | ||
|
||
final String prompt = String.format("%s\n\n%s\n%s", systemMessages, userMessages, ASSISTANT_PROMPT); |
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.
use %n
instead of \n
and can return directly instead of storing in a string first.
.build(); | ||
|
||
InvokeModelResponse response = this.client.invokeModel(invokeRequest); | ||
// BedrockRuntimeResponseMetadata metadata = response.responseMetadata(); |
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.
remove commented line
* In search use-cases, use search_document when you encode documents for embeddings that you store in a | ||
* vector database. | ||
*/ | ||
search_document, |
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.
shouldn't these all be upper case names?
/** | ||
* The model has finished generating text for the input prompt. | ||
*/ | ||
stop, |
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.
should these be upper case to follow existing conventions?
* | ||
* @param embedding The embedding vector. | ||
* @param inputTextTokenCount The number of tokens in the input text. | ||
* @param message TODO |
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.
extraneous TODO
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.
these jpg should to into some spot in the antora docs. ATM we don't yet have many diagrams in the docs, e.g. ETL pipeline, which would greatly enhance the docs.
@@ -42,6 +42,11 @@ | |||
|
|||
|
|||
<!-- Spring Framework --> | |||
<dependency> |
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.
why is this dependency necessary to add?
@@ -27,6 +27,11 @@ | |||
<version>${parent.version}</version> | |||
</dependency> | |||
|
|||
<dependency> |
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.
why was this added?
- Add spring-ai-bedrock project with support for Cohere, Llama2, Ai21 Jurassic 2, Titan and Anthropic LLM models. - Add native API clients for CohereChat CohereEmbedding , Llama2Chat, JurassicChat, TitanChat and TitanEmbedding models, supporting both single shot and streaming completions (for the models that allows it) - Add ITs tests for the native API clients. - Implement Chat (AiClient) and ChatStreaming (AiStreamingClient) and EmbeddingClients (according to the models’ support for those) for Cohere, Llama2, and Anthropica. Titan and Jurassic2 are WIP. - Add ITs for the ChatClient, ChatStreamingClient and EmbeddingClient implementations. - Add Spring Boot Auto-configurations with flexible properties for the Llama2, Anthropic and Cohere modes + ITs - Add Spring Boot Starter configurations for all Bedrock models. - Add README documentations for all models. Resolves spring-projects#66
@@ -0,0 +1,83 @@ | |||
# Bedrock AI Chat and Embedding Clients |
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.
agree
/** | ||
* Converts a list of messages to a prompt for particular model. | ||
* | ||
* TODO: consider factoring out this into an interface and provide different |
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.
Rename MessageToPromptStrategy to MessageToPromptConverter
.withTemperature(0.8f) | ||
.withMaxTokensToSample(300) | ||
.withTopK(10) | ||
// .withStopSequences(List.of("\n\nHuman:")) |
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.
remove
6d0364f
to
289bc8d
Compare
merged in 0e3192a |
Resolves #66