I have few ToolCallback
(s) and one of them is taking use of the ToolContext
, whereas the others aren't.
e.g.
public class CustomersToolCallback implements ToolCallback {
@Override
public ToolDefinition getToolDefinition() {
return ToolDefinition.builder()
.name("getCustomerInfo")
.description("Retrieve customer information")
.build();
}
@Override
public String call(String toolInput, ToolContext tooContext) {
if (tooContext == null) {
throw new UnsupportedOperationException("ToolContext cannot be null");
}
Object tenantId = tooContext.getContext().get("tenant");
// Fetch customer by their id
}
@Override
public String call(String toolInput) {
throw new UnsupportedOperationException("This method is not supported. Use call(String toolInput, ToolContext toolContext) instead.");
}
}
And another one
public class MockCustomerToolCallback implements ToolCallback {
@Override
public ToolDefinition getToolDefinition() {
return ToolDefinition.builder()
.name("mockCustomer")
.description("Retrieve mock customer information")
.build();
}
@Override
public String call(String toolInput) {
// Some implementation
}
}
If I have this tool callback registered with the ChatModel
and I do
ChatModel chatModel = ...
String response = ChatClient.create(chatModel)
.prompt("How does a mock customer look like")
.toolContext(Map.of("tenantId", "acme"))
.call()
.content();
You can imagine that the call is a bit more dynamic than this. So if this happens, and my MockCustomerToolCallback
needs to be called it'll fail due to the
if (tooContext != null && !tooContext.getContext().isEmpty()) {
throw new UnsupportedOperationException("Tool context is not supported!");
}
In the ToolCallback
.
What exactly is the reason for this UnsupportedOperationException
on the interface level? It is a bit inconvenient if all of my ToolCallback
implementations have to implement the method with the ToolContext
even if they do not need it.
Fix proposal
I would propose the following:
Add new interface
Similar to how it is done for Spring Kafka GenericMessageListener
and its different implementations, or Spring AMQP MessageListener
and its different implementation. Spring AI could add a ToolContextAwareToolCallback
which can look something like
public interface ToolContextAwareToolCallback extends ToolCallback {
@Override
default String call(String toolInput) {
throw new UnsupportedOperationException("This method is not supported. Use call(String toolInput, ToolContext toolContext) instead.");
}
@Override
String call(String toolInput, @NonNull ToolContext toolContext);
}
This would make it easier to implement, since I won't need to override ToolCallback#cal(String)
. The exception can be adjusted though.
Relax the constraint in the ToolCallback#call(String, ToolContext)
The ToolCallback.call(String, ToolContext)
would need to be adjusted to allow empty tool context.
Potentially similar issues
- https://github.com/spring-projects/spring-ai/issues/2868
- https://github.com/spring-projects/spring-ai/issues/2714 - I believe that this issue is the same, with the only difference being the fact that this is the one for the
@Tool
Comment From: ilayaperumalg
@filiphr Thanks very much for the detailed write-up reporting the issue!
Comment From: mjkhub
Introducing ToolContextAwareToolCallback
sounds like a clean architectural solution, but I think it comes with widespread changes across multiple classes.
. . .
Comment From: filiphr
Introducing
ToolContextAwareToolCallback
sounds like a clean architectural solution, but I think it comes with widespread changes across multiple classes.
@mjkhub, why do you think that this involves widespread changes across multiple classes? One of the main reasons for the proposal is also to make it easier for people to implement ToolCallback
, see the CustomersToolCallback
from my example.
I don't think that this proposal would need widespread changes across the existing codebase.
@ilayaperumalg I see that this has been put as part of the 1.1 milestone. Is it OK if I try to provide a PR for this functionality?
Comment From: mjkhub
@filiphr
You're right — I misunderstood that part. Appreciate the clarification.