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): Integrate anthropic models #27

Merged
merged 5 commits into from
Sep 23, 2024
Merged

Conversation

0xMochan
Copy link
Contributor

@0xMochan 0xMochan commented Sep 19, 2024

This PR implements Claude models from Anthropic. This provider is structured in multiple files as a test of organization for providers.

anthropic::ClientBuilder

The Anthropic client has a couple of extra fields that need to be kept track of:

  • anthropic_betas
  • anthropic_version

These are used by the anthropic api within the headers to determine certain things such as the specific API version or which specific beta features (currently only prompt-caching) is used. This became a bit odd to work with via a normal client constructor, so a builder pattern was a bit more natural here.

Specifically looking for feedback on whether I should just use normal String rather than &str with lifetimes for code readability. Also, I've set anthropic_beta to be chained as a way to add multiple items to a list.

max_tokens

For some reason, Anthropic requires max_tokens to be a property set on every call to the api and every claude model has a different max value for this field. Since the other providers have a max_token property, I've added this directly to the generic builders (such as AgentBuilder) so that it can be used for every model. It's only required for anthropic models.

The implementation of max_tokens for other models is unset in this PR but could easily be included as a separate PR if needed.

Anthropic Version

Currently, I have this set as a const similar to models. TBH, I'm not a fan of this (especially the naming isn't great) but I'm also not a fan of including the full date in the const name either (like what would be a point of a const name if the entire date was in it). I'm not exactly sure what should be the design here.

This is used for API version stability. Why this is a custom header rather than the v1, v2 pattern akin to traditional APIs is beyond me.

@marieaurore123
Copy link
Contributor

For you first question, I think it is reasonable to keep the ClientBuilder's fields as &str. The data defined in the builder struct is "short term" since it's only objective is to create a Client struct (which itself owns it's fields).

For the versioning, I think what you did makes sense given the circumstances.

@0xMochan
Copy link
Contributor Author

Question: with chat history, is the last item in the list the newest member (is it sorted older to newest where the last element in the list is the prompt).

rig-core/src/providers/anthropic/client.rs Outdated Show resolved Hide resolved
rig-core/src/providers/anthropic/completion.rs Outdated Show resolved Hide resolved
@0xMochan
Copy link
Contributor Author

@cvauclair The last thing with this,

pub const ANTHROPIC_VERSION_1: &str = "2023-01-01";
pub const ANTHROPIC_VERSION_2: &str = "2023-06-01";
pub const ANTHROPIC_VERSION_LATEST: &str = ANTHROPIC_VERSION_2;

This is how I've done the specific anthropic versioning constants naming. I'm assuming you agree with this. If so, ready to merge 🚀

@cvauclair
Copy link
Contributor

@cvauclair The last thing with this,

pub const ANTHROPIC_VERSION_1: &str = "2023-01-01";
pub const ANTHROPIC_VERSION_2: &str = "2023-06-01";
pub const ANTHROPIC_VERSION_LATEST: &str = ANTHROPIC_VERSION_2;

This is how I've done the specific anthropic versioning constants naming. I'm assuming you agree with this. If so, ready to merge 🚀

I hesitated to flag this, but seeing that you're also unsure, I think it may be best to change to it reflect to actual name of the version, i.e.:

pub const ANTHROPIC_VERSION_2023_01_01: &str = "2023-01-01";

@0xMochan
Copy link
Contributor Author

@cvauclair The last thing with this,

pub const ANTHROPIC_VERSION_1: &str = "2023-01-01";
pub const ANTHROPIC_VERSION_2: &str = "2023-06-01";
pub const ANTHROPIC_VERSION_LATEST: &str = ANTHROPIC_VERSION_2;

This is how I've done the specific anthropic versioning constants naming. I'm assuming you agree with this. If so, ready to merge 🚀

I hesitated to flag this, but seeing that you're also unsure, I think it may be best to change to it reflect to actual name of the version, i.e.:

pub const ANTHROPIC_VERSION_2023_01_01: &str = "2023-01-01";

Yea, I was also unsure about putting the whole date in the const name but I guess you get auto-complete w/ rust, so it makes sense.

@cvauclair cvauclair merged commit 278ea1e into main Sep 23, 2024
2 checks passed
@github-actions github-actions bot mentioned this pull request Sep 23, 2024
@0xMochan 0xMochan deleted the feat/anthropic branch September 23, 2024 19:16
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