-
Notifications
You must be signed in to change notification settings - Fork 3k
add tool call support for deepseek-chat、deepseek-reasoner(Only for deepsee… #6236
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
👷 Deploy request for continuedev pending review.Visit the deploys page to approve it
|
All contributors have signed the CLA ✍️ ✅ |
✨ No issues found! Your code is sparkling clean! ✨ |
I have read the CLA Document and I hereby sign the CLA |
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 opening the PR here @xiaohuanxiong3 ! Left a comments on some changes I wasn't sure about.
core/llm/autodetect.ts
Outdated
// deepseek: deepseekTemplateMessages, | ||
deepseek: null, |
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.
What's the reason for this?
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.
The purpose of this line of code is to prevent the program from entering the following code branch in BaseLLM.*streamChat
if (this.templateMessages) {
for await (const chunk of this._streamComplte(
prompt,
signal,
completionOptions,
)) {
completion += chunk;
interaction?.logItem({
kind: "chunk",
chunk: chunk,
});
yield { role: "assistant", content: chunk };
}
}
After the program enters this branch, the tool call information will never be returned, and because the deepseek model will stop responding after the tool call ends, the chat will suddenly end in the user interface.
Maybe I should modify the autodetectTemplateType()
function in autodetect.ts
instead
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.
Maybe I should modify the autodetectTemplateType() function in autodetect.ts instead
I think that seems like the right implementation 👍
core/llm/llms/Deepseek.ts
Outdated
protected useOpenAIAdapterFor: (LlmApiRequestType | "*")[] = []; | ||
|
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.
Do we need this?
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.
Sorry, this is my question, I didn't expect 'openaiAdapter' to implement a similar logic to change-3, so change is not needed here
core/llm/llms/Deepseek.ts
Outdated
options: CompletionOptions, | ||
): AsyncGenerator<ChatMessage, any, any> { | ||
const body = this._convertArgs(options, messages); | ||
|
||
const response = await this.fetch(this._getEndpoint("chat/completions"), { | ||
method: "POST", | ||
headers: this._getHeaders(), | ||
body: JSON.stringify({ | ||
...body, | ||
...this.extraBodyProperties(), | ||
}), | ||
signal, | ||
}); | ||
|
||
// Handle non-streaming response | ||
if (body.stream === false) { | ||
if (response.status === 499) { | ||
return; // Aborted by user | ||
} | ||
const data = await response.json(); | ||
yield data.choices[0].message; | ||
return; | ||
} | ||
|
||
let message: AssistantChatMessage | ThinkingChatMessage | undefined; | ||
let myArguments: string | undefined; | ||
let lastMessageRole: "assistant" | "thinking" | undefined; | ||
|
||
function fromChatCompletionChunk(chunk: any): ChatMessage | undefined { | ||
const delta = chunk.choices?.[0]?.delta; | ||
|
||
if (delta?.content) { | ||
lastMessageRole = "assistant"; | ||
return { | ||
role: "assistant", | ||
content: delta.content, | ||
}; | ||
} else if (delta?.reasoning_content) { | ||
lastMessageRole = "thinking"; | ||
return { | ||
role: "thinking", | ||
content: delta.reasoning_content, | ||
}; | ||
} else if (delta?.tool_calls) { | ||
if (!message) { | ||
message = { | ||
role: "assistant", | ||
content: "", | ||
toolCalls: delta?.tool_calls.map((tool_call: any) => ({ | ||
id: tool_call.id, | ||
type: tool_call.type, | ||
function: { | ||
name: tool_call.function?.name, | ||
arguments: tool_call.function?.arguments, | ||
}, | ||
})), | ||
}; | ||
myArguments = ""; | ||
return message; | ||
} else { | ||
// @ts-ignore | ||
myArguments += delta?.tool_calls[0].function.arguments; | ||
} | ||
return undefined; | ||
} | ||
|
||
if (chunk.choices?.[0]?.finish_reason === "tool_calls") { | ||
if (message) { | ||
message = { | ||
role: message.role, | ||
content: message.content, | ||
toolCalls: [ | ||
{ | ||
id: message.toolCalls?.[0].id, | ||
type: message.toolCalls?.[0].type, | ||
function: { | ||
name: message.toolCalls?.[0].function?.name, | ||
arguments: myArguments, | ||
}, | ||
}, | ||
], | ||
}; | ||
const tempMessage = message; | ||
message = undefined; | ||
return tempMessage; | ||
} else { | ||
return undefined; | ||
} | ||
} else { | ||
return undefined; | ||
} | ||
} | ||
|
||
for await (const value of streamSse(response)) { | ||
const chunk = fromChatCompletionChunk(value); | ||
if (chunk) { | ||
yield chunk; | ||
} | ||
} | ||
} |
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.
Is there a reason we need this when the model is already OpenAI compatible? I believe deepseek-reasoner
was working fine without this
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.
My apologies, this was my mistake. I didn't realize that openaiAdapter
already implemented the similar logic, so this change here is also unnecessary. However, because the fromChatCompletionChunk
function in openaiTypeConverters.ts
doesn't handle the potential ThinkingChatMessage
logic, you're right that deepseek-reasoner
can work fine without the above changes, but the thinking process of deepseek-reasoner
won't be displayed in the chat interface.
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.
The ChatCompletionChunk
returned by the openaiAdapter.chatCompletionStream
function does not have the reasoning_content
field corresponding to the thought process returned by deepseek-reasoner
. Therefore, if it is not necessary to display the thought process of deepseek-reasoner
in the user interface, change-2 and change-3 are unnecessary. If it is necessary to display the thought process of deepseek-reasoner
, I looked at the return type ChatCompletionChunk
of the *chatCompletionStream
function in @continuedev/openai-adapters
and it does not have the reasoning_content
field, so besides adding change-2 and change-3, I don't have any other solutions for the time being.
@xiaohuanxiong3 seems like you rolled back most of the changes, lmk if/when it's ready for another review 👍 |
it's ready |
…k official API )
Description
[ add tool call support for deepseek official api . ]
Checklist
Screenshots
[ For visual changes, include screenshots. Screen recordings are particularly helpful, and appreciated! ]
Tests
[ What tests were added or updated to ensure the changes work as expected? ]