feat(tool): Add ToolExecutionEligibilityPredicate interface #2585
Conversation
| * @param chatResponse The response from the chat model | ||
| * @return true if tool execution should be performed, false otherwise | ||
| */ | ||
| default boolean isToolExecutionRequired(ChatOptions promptOptions, ChatResponse chatResponse) { |
There was a problem hiding this comment.
I would suggest keeping only this method in the interface and make it a Predicate, returning just true/false.
The other two methods are already implemented elsewhere and the implementation should never change. And if customisations are needed, this method here would allow that, without having to change the other two.
@tzolov what do you think?
There was a problem hiding this comment.
Following that train of thought, I would name this interface ToolExecutionEligibilityPredicate. The default implementation can simply consist of the following:
return ToolCallingChatOptions.isInternalToolExecutionEnabled(prompt.getOptions()) && response != null
&& response.hasToolCalls()There was a problem hiding this comment.
The "internalToolExecutionEnabled" is an inner state of ToolCallingChatOptions. It shouldn't be changed.
Similarly, having toolCalls is an inner state of ChatResponse. It shouldn't be changed either.
What can be changed is if and when to use those two pieces of information, possibly in combination with additional factors. And a ToolExecutionEligibilityPredicate interface with just one method should be sufficient.
tzolov
force-pushed
the
toolExecutionEligibilityChecker
branch
from
72a271c to
a66a673
Compare
|
Thanks for the suggestions @ThomasVitale . I've adjusted the PR. |
tzolov
changed the title
Introduce a new ToolExecutionEligibilityChecker interface to provide a more flexible way to determine when tool execution should be performed based on model responses. This abstraction replaces the hardcoded logic previously scattered across the codebase. - Adds a new ToolExecutionEligibilityChecker interface in spring-ai-core - Integrates the checker into OpenAiChatModel with appropriate defaults - Updates OpenAiChatAutoConfiguration to support the new interface - Provides a default implementation that maintains backward compatibility Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
…ligibilityPredicate - Replacing ToolExecutionEligibilityChecker with ToolExecutionEligibilityPredicate - Changing from Function<ChatResponse, Boolean> to BiPredicate<ChatOptions, ChatResponse> - Adding a DefaultToolExecutionEligibilityPredicate implementation - Updating AnthropicChatModel and OpenAiChatModel to use the new predicate - Updating auto-configurations to inject the new predicate - Adding comprehensive tests for the new predicate implementation The new approach provides a cleaner and more consistent way to determine when tool execution should be performed based on both prompt options and chat responses. Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
tzolov
force-pushed
the
toolExecutionEligibilityChecker
branch
from
f7f0387 to
9999675
Compare
Introduce a new ToolExecutionEligibilityChecker interface to provide a more flexible way to determine when tool execution should be performed based on model responses. This abstraction replaces the hardcoded logic previously scattered across the codebase.