-
Notifications
You must be signed in to change notification settings - Fork 3
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 (Claude) provider #22
Conversation
I added a first version of code completion.
|
Thanks @brichet for looking into this 👍 I just tried this PR locally and there seems to be many requests made to the anthropic API, so maybe there is indeed something not expected happening: jupyterlite-anthropic.webm |
Thanks for testing it @jtpio Using Anthropic completion with jupyter-ai do not trigger error on my side, but the suggestions are not really relevant (most of the time): Indeed, you're right about the extra requests, I'll take a closer look. |
Indeed according to your screenshot there seems to be an issue with how Jupyter AI handles the inline completer suggestions. So likely an issue in Jupyter AI (we could open one if non exists already) |
Looks like the default model is Maybe we should revive #14 to get all the settings for each providers. |
Yeah the JupyterLab related change should now be available in |
Indeed, I just tried it locally and it seems to be faster with Claude 3.5 Sonnet. There seems to be some issues with the quality of completion being returned sometimes, but it's something that can likely be fixed by improving the prompt or the use of the langchain primitives: |
fbb3833
to
54f96ca
Compare
Unfortunately #14 does not work out of the box for Anthropic provider. |
Looks like the settings generator script needs to be updated for each new provided being added? |
Yes, but it would be great if we could have a generic way to retrieve the available settings. If The other problem is about the use of ts-json-schema-generator, which raises an error when trying to build the settings for
In conclusion, for now I think we'll have to update the generator, but hopefully it should be painless next time 😄 |
3096bc5
to
b74d481
Compare
let schema; | ||
|
||
// Workaround to exclude some properties from a type or interface. | ||
if (desc.excludedProps) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's probably fine to have exclusion clauses like this for now as we figure things out with several providers 👍
Also found that some props had to be excluded over in #27.
// Anthropic does not allow whitespace at the end of the AIMessage | ||
const trimmedPrompt = prompt.trim(); | ||
|
||
const messages = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering if this list of messages should be recreated each time, or it should be persisted per provider?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some other open questions: should the list of messages be shared across providers? Or should it be reset when a user switches to another provider?
Maybe a reset would be fine to keep things tidy. Also users probably wouldn't change providers often in practice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened #29
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Testing locally and it seems to be working fine 👍
By default the inline completion does seem to be quite slow:
jupyterlite-ai-anthropic-default.mov
But switching to claude-3-5-sonnet-latest
as the model does seem to be making things faster:
jupyterlite-ai-anthropic-35.mov
Left a couple of inline comments, that we can also track separately.
Thanks for the review @jtpio |
This PR adds the Anthropic provider as a chat model.
It does not include completion.