-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Native Tool Call #6744
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?
Native Tool Call #6744
Conversation
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.
Thank you for your contribution! I've reviewed the changes and found some issues that need attention. This PR implements a valuable feature for native tool calling, but there are some critical issues that should be addressed before merging.
be6c66f
to
2c0c3b3
Compare
e711649
to
a5df80c
Compare
@roomote-agent |
I'm starting my review of this PR now. I'll examine the Native Tool Call implementation and provide feedback shortly. |
Review Complete ✅I've completed my review of the Native Tool Call implementation. Here's my assessment: ✅ Strengths
📋 Key Components Reviewed
🔍 Observations
✅ Testing Results
💡 Recommendations
✅ VerdictThe implementation is solid and ready for use. The streaming JSON to XML converter is well-designed, and the feature toggle provides a safe rollout path. The code quality is good with proper error handling and test coverage. Great work on this feature! 🎉 |
3dc988a
to
f7eab93
Compare
5486906
to
6e667fc
Compare
Native tool call support is complete. I've found a way to use standard tool results to interact with LLM without major changes. Overall, it's less common to encounter multiple rounds of conversations where subsequent responses mimic XML and abandon the return of tool_calls. I've tested the tools I frequently use in my daily work. There are still a few tools I use less frequently.
For the other tools, I use them frequently in my daily work. Most of the problem have been resolved. |
75d42c1
to
7b7c6f0
Compare
Update on this (From @cte): there's a bug in the tool calling that causes a bunch of the evals to fail, so not quite ready to evaluate. Here's what the errors look like:
|
This involves returning multiple tools data in a single stream, and I did not handle this situation. This issue has now been fixed. Due to network issues, I have not yet run the evaluation locally. I plan to run the evaluation locally, covering models such as GPT-5, GPT-4.1, Gemini-2.5-Pro, Gemini-2.5-Flash, and Deepseek-Chat. Once completed, I will refine the corresponding tests and request a review. Regarding Claude, I have not yet found a solution to address the mixed text returned by OpenRouter, so this issue may be put on hold for now. |
4b5bb0a
to
2f41ec5
Compare
- Introduced a new ToolCallSettingsControl component for enabling/disabling tool calls in the settings UI. - Updated API options to include tool call settings and integrated it into the existing settings view. - Enhanced localization files to support new tool call settings in multiple languages. - Added logic to handle tool call enabled state across different components and tools.
7263f50
to
fcfed58
Compare
Issue
#4047 (comment)
What I Do
Without major refactoring of the existing workflow, the tool call based on XML-formatted text prompts was converted to tool call based on json.
Implementation Approach
Tools
Provider
VSCode-LM API(Flow Break)Copilot(feat: add GitHub Copilot Provider that support agent mode #7010 #7072)Benefits
Problem
Legacy Issues
Not all tools have been migrated yet, which may lead the model to mimic behavior. As XML content accumulates in the chat, incorrect XML outputs may occur.( Abandoned the need to refine parameters and strengthen control (for example, splitting the content of apply_diff into search_str and replace_str), and completely aligned the tool call parameters with the original XML description to avoid parameter errors when LLM refers to tool parameters and large amounts of user XML input for XML output. )(This has been resolved through standard tool calls)New Problem
Streaming processing is disrupted. Although results are still returned via streaming, the actual tool invocation requires converting JSON to XML first. For large file modifications, this causes delays where users see no feedback during processing.(Support streaming conversion of json to xml)Streaming state changes after JSON completion. Due to the current duplicate call detection, the model's retry limit must be adjusted to two or more attempts; otherwise, errors will occur.(See Errors and Repeated Use Limitations #6834)In addition, ask_followup_question uses xml attribute to set mode, and cannot map json stream to xml.(feat: simplify attempt_completion tool description #6888)Screenshots / Videos
Important
Introduces JSON-based tool calls with a toggle in settings, updating API handlers, tool schemas, and UI components to support this new functionality.
toolCallEnabled
toggle inprovider-settings.ts
to enable/disable tool calls.base-provider.ts
,lm-studio.ts
,openai.ts
, andopenrouter.ts
.apply_diff
,insert_content
,list_code_definition_names
,list_files
,read_file
,search_and_replace
,search_files
,write_to_file
, and others.ToolRegistry
intool-registry.ts
to manage tool schemas.ToolCallSettingsControl.tsx
to manage tool call settings in the UI.StreamingToolCallProcessor
intool-call-helper.ts
for streaming JSON to XML conversion.Task.ts
to handle tool call streaming and processing.This description was created by
for b1984b8. You can customize this summary. It will automatically update as commits are pushed.