-
Notifications
You must be signed in to change notification settings - Fork 700
improvement: updated ai21 integration #1228
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
base: main
Are you sure you want to change the base?
Conversation
src/providers/ai21/chatComplete.ts
Outdated
max_tokens: { | ||
param: 'maxTokens', | ||
default: 16, | ||
documents: { |
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.
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.
in the new ai21 spec, there is no separate field for 'system', so you can remove this code block
{
param: 'system',
required: false,
transform: (params: Params) => {
if (
params.messages?.[0]?.role &&
SYSTEM_MESSAGE_ROLES.includes(params.messages?.[0]?.role)
) {
return params.messages?.[0].content;
}
},
},
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.
also remove this chunk from the messages object
if (
params.messages?.[0]?.role &&
SYSTEM_MESSAGE_ROLES.includes(params.messages?.[0]?.role)
) {
inputMessages = params.messages.slice(1);
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.
you will also need to map the text content part into the user and system messages from openai to ai21 format
in openai, a user can send user message like this
{
"role": "user",
"content" : [{"type": "text", "text": "abc"}, ..multiple parts]
}
but ai21 only accepts a string, so you will need to handle that
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 for your review! I'm working on the requested changes and will update the pull request soon.
Also, could you please send me the link for the above-attached image for reference? As in the official API reference for AI21 Chat completions, parameters are defined similarly to the ones in my commit.
Important PR Review SkippedPR review skipped as per the configuration setting. Run a manually review by commenting /matter review 💡Tips to use Matter AICommand List
|
Description
Updated Chat Completions API endpoint, and also modified API configs and response interface
Motivation
This updates the existing outdated ai21 integration and fixes #1215
Type of Change
Checklist
Related Issues
Fixes #1215