-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Fix Kimi K2 Thinking native tool calls for OpenAI-compatible servers #9557
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
Conversation
Review complete. No issues found. The previous concern about inconsistent argument handling is no longer applicable - the code now uses
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
|
Tested this PR, no issues with Roo Code v3.35.5. Thanks for the PR. |
|
@roomote can you please run merge resolver mode on this? |
…_llama.cpp - Modified base-openai-compatible-provider.ts to better handle tool calls - Tool calls are now yielded when any finish_reason is present, not just "tool_calls" - Added fallback to yield tool calls at stream end if not already yielded - Added tracking to prevent duplicate tool call yields - Ensures tool calls with valid ID and name are always yielded - Fixes issue #9551 where Kimi K2 Thinking native tool calls were not working
f310921 to
2177a8c
Compare
Successfully resolved merge conflicts for PR #9557 "Fix Kimi K2 Thinking native tool calls for OpenAI-compatible servers". Resolution Summary:
All type checks passed. The rebase completed successfully and the branch is now up-to-date with main. |
|
This issue was already fixed by PR #9542 which was merged the same day this PR was opened. The real fix added NativeToolCallParser.finalizeRawChunks() which is called at the end of stream processing regardless of finish_reason. This handles the case where servers don't send finish_reason = "tool_calls" more elegantly at the consumer level. This PR modifies the provider layer to emit tool_call_end events, but those events aren't actually consumed by Task.ts - there's no case handler for them in the stream processing switch. The events emitted by this code would be silently ignored. Closing since the underlying issue was resolved by #9542. |
This PR attempts to address Issue #9551 regarding Kimi K2 Thinking native tool calls not working properly with ik_llama.cpp and other OpenAI-compatible servers.
Problem
When using Kimi K2 with ik_llama.cpp, tool calls were being detected but the results were not being sent back to the model properly. The issue was that:
finish_reasonto "tool_calls" specificallySolution
Modified
base-openai-compatible-provider.tsto:finish_reasonis present (not just "tool_calls")hasYieldedToolCallsflag to prevent duplicatesTesting
Feedback and guidance are welcome!
Important
Fixes tool call handling in
base-openai-compatible-provider.tsfor OpenAI-compatible servers by yielding tool calls on anyfinish_reasonand adding a fallback mechanism.base-openai-compatible-provider.ts, tool calls are now yielded when anyfinish_reasonis present, not just "tool_calls".hasEmittedToolCallEndflag to prevent duplicate tool call end events.This description was created by
for 2177a8c. You can customize this summary. It will automatically update as commits are pushed.