-
Notifications
You must be signed in to change notification settings - Fork 2.8k
feat: implement dynamic tool protocol resolution with proper precedence hierarchy #9286
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
Re-review completed. The previous issue has been resolved.
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
| MARKDOWN RULES | ||
| ALL responses MUST show ANY \`language construct\` OR filename reference as clickable, exactly as [\`filename OR language.declaration()\`](relative/file/path.ext:line); line is required for \`syntax\` and optional for filename links. This applies to ALL markdown responses and ALSO those in <attempt_completion>` | ||
| ALL responses MUST show ANY \`language construct\` OR filename reference as clickable, exactly as [\`filename OR language.declaration()\`](relative/file/path.ext:line); line is required for \`syntax\` and optional for filename links. This applies to ALL markdown responses and ALSO those in attempt_completion` |
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.
It appears there's a typographical error in the inline code formatting in this line. The fragment "in attempt_completion" ends with a backtick that isn't balanced by a corresponding opening backtick. Should it be formatted as "attempt_completion`" or corrected in some other way?
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.
Are you ok buddy?
Fixed the support validation issue. All local checks passed. |
Overview
This PR implements dynamic tool protocol resolution that properly respects the precedence hierarchy: Support > Preference > Defaults.
Precedence Hierarchy
The tool protocol is now resolved in the following order:
toolProtocolsetting) - highest prioritydefaultToolProtocolin ModelInfo) - model-specific preferenceKey Changes
Type Definitions
defaultToolProtocolfield toModelInfotypetoolProtocolfield toProviderSettingstyperooModelDefaultsconfiguration object for Roo providerCore Resolution Logic
resolveToolProtocol()utility function that centralizes all resolution logicIntegration Points Updated
Bug Fixes
apiConfigurationpropertyTesting
resolveToolProtocolcovering all scenariosConfiguration Example
Roo Model Defaults
Provider Defaults
Currently all providers default to XML (empty list in
getProviderDefaultProtocol()). To add native-preferred providers:What This Enables
defaultToolProtocolImportant
Introduces dynamic tool protocol resolution with a precedence hierarchy, updating core logic and tests to ensure correct protocol selection based on user, model, and provider settings.
resolveToolProtocol()to determine tool protocol based on precedence: User Profile > Global Setting > Model Default > Provider Default > XML Fallback.Task.ts,presentAssistantMessage.ts, andgenerateSystemPrompt.tsto useresolveToolProtocol().applyDiffTool()andReadFileToolto handle protocol resolution.resolveToolProtocol.spec.tswith 33 tests covering all precedence scenarios and edge cases.applyDiffTool.experiment.spec.tsandmultiApplyDiffTool.spec.tsto reflect new protocol logic.defaultToolProtocoltoModelInfoandtoolProtocoltoProviderSettingstypes.This description was created by
for ae21f2a. You can customize this summary. It will automatically update as commits are pushed.