0xPlaygrounds / rig

⚙️🦀 Build portable, modular & lightweight Fullstack Agents
https://rig.rs
MIT License
153 stars 9 forks source link

feat(providers): Integrate anthropic models #27

Closed 0xMochan closed 1 month ago

0xMochan commented 1 month ago

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:

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.

garance-buricatu commented 1 month ago

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 commented 1 month ago

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

0xMochan commented 1 month ago

@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 commented 1 month ago

@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 commented 1 month ago

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