-
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
6b099d6
to
5486906
Compare
- Added StreamingToolCallProcessor to handle tool call data and convert JSON chunks to XML format. - Introduced ToolCallProcessingState to manage the parsing state and track progress. - Enhanced applyDiffTool to support new tool call functionality based on provider state. - Updated generateSystemPrompt to include tool call configuration. - Added supportToolCall utility to determine if a provider supports tool calls. - Created ToolCallSettingsControl component for user interface settings. - Localized new tool call settings in multiple languages.
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. |
4ad947d
to
2c4c145
Compare
…y and conciseness
097b43d
to
e17ca9c
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)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)Important
This PR transitions tool calls from XML to JSON format, enhancing accuracy and efficiency, with updates to backend logic, UI, and tests.
ProviderSettingsManager
to enable/disable JSON tool calls.Task
class to handle JSON tool calls and convert them back to XML if needed.ToolRegistry
to manage tool schemas and support JSON tool calls.apply_diff
,read_file
,write_to_file
, etc.function-call-converter.ts
to handle JSON to XML transformations.ToolCallSettingsControl
component to manage tool call settings in the UI.ApiOptions.tsx
to include tool call settings.settings.json
.base-tool-schema.test.ts
andfunction-call-converter.test.ts
.ToolRegistry
functionality intool-registry.test.ts
.This description was created by
for cd7a21c. You can customize this summary. It will automatically update as commits are pushed.