GH-5877: Use ToolExecutionEligibilityPredicate in ToolCallAdvisor for tool call detection#5878
Open
ArpanC6 wants to merge 1 commit intospring-projects:mainfrom
Open
GH-5877: Use ToolExecutionEligibilityPredicate in ToolCallAdvisor for tool call detection#5878ArpanC6 wants to merge 1 commit intospring-projects:mainfrom
ArpanC6 wants to merge 1 commit intospring-projects:mainfrom
Conversation
…CallAdvisor for tool call detection Signed-off-by: Arpan Chakraborty <chakrabortyarpan151@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
We’ve made improvements to the ToolCallAdvisor by adding a ToolExecutionEligibilityPredicate. This new approach replaces the previous hardcoded chatResponse.hasToolCalls() check for detecting tool calls in both the non streaming (adviseCall) and streaming (handleToolCallRecursion) processes.
Additionally a toolExecutionEligibilityPredicate() method has been added to the Builder to allow users to inject a custom predicate if needed.
Why
Previously ToolCallAdvisor was using a hardcoded check (hasToolCalls()) to detect tool calls which didn't take into account more complex mechanisms used by models like Anthropic and Bedrock. These models rely on additional checks such as the finish reason (tool_use) to correctly detect tool calls. This made ToolCallAdvisor inconsistent with how other models handle tool call detection.
How
We added a ToolExecutionEligibilityPredicate field to ToolCallAdvisor, which defaults to DefaultToolExecutionEligibilityPredicate.
A new constructor was added to accept a custom predicate.
We replaced the old chatResponse.hasToolCalls() check with toolExecutionEligibilityPredicate.isToolExecutionRequired() in both the non streaming and streaming tool call detection paths.
We also added the toolExecutionEligibilityPredicate() method to the Builder allowing for easy customization.
This change addresses the issue and ensures that ToolCallAdvisor works seamlessly with different models that rely on custom tool call detection.
Closes #5877.