-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Feat{providers}: bedrock-anthropic #4522
Conversation
❌ Deploy Preview for continuedev failed. Why did it fail? →
|
@ferenci84 This might help our cause to implement anthropic features faster in a shared way? |
@chezsmithy I think if handling anthropic models are differentiated, it should be part of the bedrock provider. If there is a working LLM class, then it could also be conditionally instantiated called within the bedrock LLM class. Regarding the API, I found that Anthropic with bedrock is well documented here: https://docs.aws.amazon.com/bedrock/latest/userguide/model-parameters-anthropic-claude-37.html I used a simple node.js based test before I implemented them (actually asked Claude to implement a simple example in node.js, and injected the relevant documentation as a context): The bottom line is that whatever is well documented is easy to implement fast, there are at least 3 options, using the InvokeModel API, using the Converse API and using the anthropic-bedrock library, users of this will not care which option we choose, as long as it works. My suggestion is about, what I believe they care about, that if they set up claude 3.7 with the bedrock provider, it should work as well as they had used a specialized provider. But you asked about "shared way": do you think about sharing logic between anthropic provider and bedrock provider? |
@chezsmithy FYI, the thinking blocks also with tool use is already working with the anthropic provider in PR #4426 you may use solutions from there, actually I had to change the UI in order to save the thinking blocks and feed back to the model on the next request. |
@ferenci84 I think if we use the Anthropic SDK for bedrock, and vertex we could refactor down to a single simple to admin code base for all three hosting models. AWS hasn't been very clear where you should use Converse or Invoke. In some discussions they have noted Converse is the future/preferred, in others they have noted both are options without clear direction. https://docs.aws.amazon.com/bedrock/latest/userguide/conversation-inference.html#:~:text=It%20is%20possible%20to%20use%20the%20existing%20base,with%20all%20Amazon%20Bedrock%20models%20that%20support%20messages. I'd prefer to use the Anthropic SDKs for Claude as they "just work". Coding against Claude with Converse and comparing logic with what is in the Anthropic.ts provider has been extremely frustrating to say the least. |
This all looks good, I'd +1 a) eventually moving toward a single AnthropicBaseLLM so we only need to maintain logic with the Anthropic SDK (as much as possible), b) not creating a separate |
… Extension Marketplace timeout
@sestinj here are some options.
Which one would you prefer? |
@chezsmithy I think the last option covers what we are were talking about and I believe @sestinj suggested the same. If a single shared codebase can handle all 3 providers (just as a combined provider you mentioned in the 2nd option), then that single shared code should be called from the 3 providers only for specific models, dynamically. I think of it as a helper class that is called from streamComplete, streamChat and other methods of the anthropic, bedrock and vertex provider, maybe it can be instantiated in the constructor. |
@chezsmithy @sestinj As https://github.com/anthropics/anthropic-sdk-typescript is the official SDK of anthropic why don't we enhance the core/llm/llms/Anthropic.ts provider to use that SDK instead of pure fetch calls? It seems that https://github.com/anthropics/anthropic-sdk-typescript/tree/main/packages/vertex-sdk and https://github.com/anthropics/anthropic-sdk-typescript/tree/main/packages/bedrock-sdk use the same API as their's readme says "For more details on how to use the SDK, see the README.md for the main Anthropic SDK which this library extends.", that way, it's possible to do the following:
This would align with this:
What do you think @chezsmithy @sestinj ? |
btw I'm merging in main with the goal of solving these flaky tests |
I may have slightly mis-typed in my previous comment. I think the most important thing is to keep the "provider" names steady and to not make them complicated. The mental model we want to build is that "provider" is selecting which vendor you are using and "model" is for the model. So providers should be "bedrock", "vertex", and "anthropic". So within the constructor for the "bedrock" provider, we would dynamically decide whether to use the Anthropic SDK or AWS SDK As a side note, if we go with Anthropic SDK we need to be able to inject the custom fetch into the SDK like we do in all of the other providers so that |
I like this idea @ferenci84 So when the bedrock or vertex providers are called with Anthropic models we redirect the calls to the anthropic provider. Seems simple and straightforward. Less cognitive load on the user. @sestinj your thoughts? I'm happy to put this together. |
@chezsmithy You can also see the demonstration how additional provider constructor parameter could allow this to be instantiated from other providers. You can see here, how this could be used from bedrock: But I commented that part, because it's not working, it gives me error |
Yeah I really like that direction |
@chezsmithy I tried to investigate the error I'm getting, used your bedrock implementation with @Anthropic-AI, and I'm consistently getting error from this part, although the same config works with the Bedrock: ![]() ![]() You see that the error is happening just after Edit:
|
I'm going to close this and re-evaluate the best way to proceed. |
Description
Partially solves for #4513
A suggested provider implementation for anthropic models on AWS Bedrock using the anthropic bedrock SDK. The SDK is consistently being updated ahead of the AWS converse API and as such already supports features like prompt caching, and turn by turn thinking modes. By using this new provider just for anthropic models continue will be able to fast follow anthropic innovations on AWS bedrock without waiting for AWS changes.
Further, using this API, we gain alignment from a code perspective with the anthropic provider. This could in the future drive further code sharing reducing time to market for continue features when the back end models are anthropic based.
AWS converse API has renamed tags and APIs making it extremely difficult to implement anthropic innovation because AWS provides extremely poor least common denonminator documentation for their APIs, and often the AWS Javascript SDK lags the python SDK futher reducing time to market for innovation.
NOTE: A similar approach could be taking to build a vertex-anthropic provider using the same SDK from anthropic further gaining code sharing and innovation.
If this merges, I would recommend following on with an agressive refactoring which splits out common code from Anthropic.ts and BedrockAnthropic.ts into a new BaseAnthropicLLM class with shared helper libraries. This could pave the way for the Vertex implementation as well and reduce the number of code changes to implement features across AWS, Vertex and Anthropic API providers.
Checklist
Screenshots
[ For visual changes, include screenshots. ]
Testing instructions
Using aws bedrock, claude 3.5 v2 or 3.7 models note that you can use this new provider to gain advanced support such as prompt caching if your AWS account has that feature enabled in preview. I have validated it against our account which has that feature. This gains significant savings in token use and significantly improves performance.