Skip to content

Conversation

@tzolov
Copy link
Contributor

@tzolov tzolov commented Feb 13, 2025

  • Extends the ToolCallingAutoConfiguration to support both FunctionCallback and ToolCallback types.
  • The toolCallbackResolver bean now handles both callback types through ObjectProvider injection.
  • Added comprehensive tests to verify the resolution of multiple function and tool callbacks.

@tzolov tzolov force-pushed the extend-tool-calling-auto-conf branch from cc9a5ec to c945741 Compare February 13, 2025 12:49
@tzolov tzolov changed the title feat: Support both FunctionCallback and ToolCallback in ToolCallingAutoConfiguration feat(autoconfigure): Support both FunctionCallback and ToolCallback in ToolCallingAutoConfiguration Feb 13, 2025
@tzolov tzolov added this to the 1.0.0-M6 milestone Feb 13, 2025
@tzolov tzolov marked this pull request as ready for review February 13, 2025 12:51
List<FunctionCallback> toolCallbacks) {
var staticToolCallbackResolver = new StaticToolCallbackResolver(toolCallbacks);
ObjectProvider<List<FunctionCallback>> functionCallbacksProvider,
ObjectProvider<List<ToolCallback>> toolCallbacksProvider) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ObjectProvider<List<FunctionCallback>> will provide both FunctionCallback and ToolCallback.
I think we can keep just ObjectProvider<List<FunctionCallback>> here, and in the next release we can replace it with ObjectProvider<List<ToolCallback>>.

Copy link
Contributor Author

@tzolov tzolov Feb 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ObjectProvider<List> will provide both FunctionCallback and ToolCallback.

I'm afraid this is not true.

Run the ToolCallingAutoConfigurationTests and put a breakpoint at ToolCallingAutoConfiguration line 59. You will find that the size of the List<FunctionCallback> allFunctionAndToolCallbacks is 2, e.g. the total number of the FunctionCallbacks defined in the test.
Then the ObjectProvider<List<ToolCallback>> toolCallbacksProvider will provide the remaining 3 ToolCallback definitions

}

public StaticToolCallbackProvider(List<? extends FunctionCallback> toolCallbacks) {
Assert.notNull(toolCallbacks, "ToolCallbacks must not be null");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding

Assert.noNullElements(toolCallbacks, "toolCallbacks cannot contain null elements");

@ThomasVitale
Copy link
Contributor

LGTM

…n ToolCallingAutoConfiguration

- Extends the ToolCallingAutoConfiguration to support both FunctionCallback and ToolCallback types.
- The toolCallbackResolver bean now handles both callback types through ObjectProvider injection.
- Added comprehensive tests to verify the resolution of multiple function and tool callbacks.

Signed-off-by: Christian Tzolov <[email protected]>
- Update ToolCallbackProvider to return FunctionCallback[]
- Migrate from List to ToolCallbackProvider in configurations
- Update tests to use new provider pattern

Signed-off-by: Christian Tzolov <[email protected]>
…ients

Refactor AsyncMcpToolCallbackProvider and SyncMcpToolCallbackProvider to handle multiple MCP clients
Add ToolCallbackProvider support to ChatClient API
Deprecate direct tool callback list methods in favor of providers
Fix typos in Closeable class names
Update MCP documentation with new examples and usage patterns

Signed-off-by: Christian Tzolov <[email protected]>
@tzolov tzolov force-pushed the extend-tool-calling-auto-conf branch from d5412a7 to d18262e Compare February 13, 2025 19:06

ChatClientRequestSpec tools(Object... toolObjects);

ChatClientRequestSpec tools(ToolCallbackProvider... toolCallbackProvider);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ChatClientRequestSpec tools(ToolCallbackProvider... toolCallbackProvider);
ChatClientRequestSpec tools(ToolCallbackProvider... toolCallbackProviders);


Builder defaultTools(Object... toolObjects);

Builder defaultTools(ToolCallbackProvider... toolCallbackProvider);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Builder defaultTools(ToolCallbackProvider... toolCallbackProvider);
Builder defaultTools(ToolCallbackProvider... toolCallbackProviders);

import org.springframework.ai.model.function.FunctionCallback;
import org.springframework.util.Assert;

/**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider JavaDoc here


Additionally, the registered MCP Tools with all MCP clients are provided as a list of ToolCallback instances:
Additionally, the registered MCP Tools with all MCP clients are provided as a list of ToolCallback
throurgh a ToolCallbackProvider instance:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
throurgh a ToolCallbackProvider instance:
through a ToolCallbackProvider instance:

----
@Bean
public List<ToolCallback> myTools(...) {
public ToolCallbackProvier myTools(...) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public ToolCallbackProvier myTools(...) {
public ToolCallbackProvider myTools(...) {

public ToolCallbackProvier myTools(...) {
List<ToolCallback> tools = ...
return tools;
return ToolCallbackProvier.from(tools);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return ToolCallbackProvier.from(tools);
return ToolCallbackProvider.from(tools);

Signed-off-by: Christian Tzolov <[email protected]>
@tzolov
Copy link
Contributor Author

tzolov commented Feb 13, 2025

Rebased, squashed and merged at 1fdda61

@tzolov tzolov closed this Feb 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants