Skip to content
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

Closed

Conversation

chezsmithy
Copy link
Contributor

@chezsmithy chezsmithy commented Mar 7, 2025

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

  • The relevant docs, if any, have been updated or created
  • The relevant tests, if any, have been updated or created

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.

Copy link

netlify bot commented Mar 7, 2025

Deploy Preview for continuedev failed. Why did it fail? →

Name Link
🔨 Latest commit 4d334b3
🔍 Latest deploy log https://app.netlify.com/sites/continuedev/deploys/67cc722499068d0008802bed

@chezsmithy
Copy link
Contributor Author

@ferenci84 This might help our cause to implement anthropic features faster in a shared way?

@ferenci84
Copy link
Contributor

ferenci84 commented Mar 7, 2025

@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
That's why I initially used the InvokeModelWithResponseStreamCommand with a switch, then I found (actually AI found) that the same can be done with the converse api (after you questioned why there is a switch).

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):
claude-aws-test.zip

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?

@ferenci84
Copy link
Contributor

@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.

@chezsmithy
Copy link
Contributor Author

chezsmithy commented Mar 7, 2025

@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.

@sestinj
Copy link
Contributor

sestinj commented Mar 7, 2025

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 bedrock-anthropic provider, as it's much easier for users if they don't need to specify this extra information

@chezsmithy
Copy link
Contributor Author

chezsmithy commented Mar 8, 2025

@sestinj here are some options.

  • we could merge this to give us a new option to improve bedrock right now with Anthropic only.
  • I could make a new combined Anthropic provider that would work with bedrock, api or vertex. You could supply say an optional endpoint parameter to control which option is chosen.
  • I could make three separate providers called Anthropic, Anthropic-bedrock and anthropic-vertex that would all share the same base code (say like 90%) except the client login.
  • I could enhance the bedrock provider to switch to using the Anthropic sdk only when Claude models are selected dynamically. This would have a converse api path for AWS models and a more native Anthropic code path for Anthropic models within streamChat.

Which one would you prefer?

@ferenci84
Copy link
Contributor

@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.

@ferenci84
Copy link
Contributor

@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:

  1. enhance core/llm/llms/Anthropic.ts to use this library
  2. Make it possible to give vertex and bedrock api keys in core/llm/llms/Anthropic.ts provider class in order to test, but keep it hidden from end-users to avoid confusion. For example, dynamically use either one of these clients:
import { AnthropicBedrock } from '@anthropic-ai/bedrock-sdk';
import { AnthropicVertex } from '@anthropic-ai/vertex-sdk';
import Anthropic from '@anthropic-ai/sdk';

// simplified example:
constructor(provider) {
  let client;
  switch (provider) {
    case 'anthropic': client = new Anthropic(); break;
    case 'bedrock': client = new AnthropicBedrock(); break;
    case 'vertex': client = new AnthropicVertex(); break;
  }
  this.client = client;
}
  1. When tests are OK, instantiate this provider with appropriate settings from the core/llm/llms/Bedrock.ts and core/llm/llms/VertexAI.ts provider classes, and call core/llm/llms/Anthropic.ts class' _streamChat method from bedrock/vertex provider's _streamChat, dynamically.

This would align with this:

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.
and this:
b) not creating a separate bedrock-anthropic provider, as it's much easier for users if they don't need to specify this extra information

What do you think @chezsmithy @sestinj ?

@sestinj
Copy link
Contributor

sestinj commented Mar 8, 2025

btw I'm merging in main with the goal of solving these flaky tests

@sestinj
Copy link
Contributor

sestinj commented Mar 8, 2025

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 requestOptions take effect

@chezsmithy
Copy link
Contributor Author

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.

@ferenci84
Copy link
Contributor

ferenci84 commented Mar 8, 2025

@chezsmithy
I was able to make this library with the anthropic provider, I used your bedrock implementation and my implementation with thinking:
https://github.com/ferenci84/continue/blob/anthropic_official_sdk/core/llm/llms/Anthropic.ts

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:
https://github.com/ferenci84/continue/blob/anthropic_official_sdk/core/llm/llms/Bedrock.ts

But I commented that part, because it's not working, it gives me error Failed to communicate with Bedrock API: Truncated event message received. I think the convertMessages part has some differences, but I wasn't able to inspect it further.

@sestinj
Copy link
Contributor

sestinj commented Mar 8, 2025

Yeah I really like that direction

@ferenci84
Copy link
Contributor

ferenci84 commented Mar 9, 2025

@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:

Screenshot 2025-03-09 at 07 14 01 Screenshot 2025-03-09 at 07 13 46

You see that the error is happening just after console.log("processing response"). Did you get similar error before?

Edit:
I was getting the same by checking out PR. This is the my configuration:

{
      "title": "AWS - Claude 3.7 Sonnet",
      "provider": "bedrock-anthropic",
      "model": "arn:aws:bedrock:us-east-1:874087778966:inference-profile/us.anthropic.claude-3-7-sonnet-20250219-v1:0",
      "region": "us-east-1",
      "profile": "default"
    }

@chezsmithy chezsmithy marked this pull request as draft March 10, 2025 21:24
@chezsmithy
Copy link
Contributor Author

I'm going to close this and re-evaluate the best way to proceed.

@chezsmithy chezsmithy closed this Mar 13, 2025
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