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

Anthropic prompt caching #104

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ChrisB-TL
Copy link

@ChrisB-TL ChrisB-TL commented Dec 13, 2024

Implements Anthropic prompt caching

Choices made

  • Ended up opting for ->withProviderMeta() on UserMessage, SystemMessage and AssistantMessage - (1) to keep consistent with Tools, and (2) as it felt a bit clumsy as a parameter on the constructor due to the "provider" wrapper (e.g. providerMeta: [Provider::Anthropic->value -> [...]]. It ended up easy enough to do with a trait, shared with Tool.
  • Have used a "beta_features" config rather than prompt caching, to future proof and allow for enabling other beta features (e.g. PDF processing).

I don't think providerMeta is the nicest API for the user, and it might get messy in the map methods if more features become reliant on it. But it should be easy to add a nicer API in a backward compatible way in future if needed. My gut thought is a ->withFeature() method on Messages and Tools, which accepts an implementation of a contract with a handle method, which are then applied with a pipeline to mutate the payload.

To-do

  • Once the multiple system prompts PR is merged, fix mapSystemMessage() and write a test for it.
  • Update docs.

P.S. could do away with the AnthropicCacheType enum - however if anyone else is like me that can never spell "ephemeral" right - it is quite handy!

Copy link

netlify bot commented Dec 13, 2024

Deploy Preview for stunning-cannoli-238aa5 canceled.

Name Link
🔨 Latest commit a7b32b3
🔍 Latest deploy log https://app.netlify.com/sites/stunning-cannoli-238aa5/deploys/676087cab59b360008cbbf6a


enum AnthropicCacheType
{
case ethemeral;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see value of keeping the enum here but Imo keeping provider related settings in the prisms code can make maintenance way harder.

@sixlive can think different.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hilariously, having added the enum to avoid spelling mistakes, I misspelt it in the enum...! Now fixed.

Definitely not a huge loss if we don't include the enum. Really just a convenience thing.

@ChrisB-TL ChrisB-TL marked this pull request as draft December 16, 2024 18:28
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.

2 participants