Skip to content

Commit 4969d5b

Browse files
committed
Review changes
1 parent 2b9a7cb commit 4969d5b

File tree

11 files changed

+134
-72
lines changed

11 files changed

+134
-72
lines changed
Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,10 @@
4040
@SpringBootTest(classes = AnthropicTestConfiguration.class, properties = "spring.ai.retry.on-http-codes=429")
4141
@EnabledIfEnvironmentVariable(named = "ANTHROPIC_API_KEY", matches = ".+")
4242
@ActiveProfiles("logging-test")
43-
class AnthropicChatClientMethodFunctionCallbackIT {
43+
class AnthropicChatClientMethodInvokingFunctionCallbackIT {
4444

45-
private static final Logger logger = LoggerFactory.getLogger(AnthropicChatClientMethodFunctionCallbackIT.class);
45+
private static final Logger logger = LoggerFactory
46+
.getLogger(AnthropicChatClientMethodInvokingFunctionCallbackIT.class);
4647

4748
public static Map<String, Object> arguments = new ConcurrentHashMap<>();
4849

models/spring-ai-openai/src/test/java/org/springframework/ai/openai/chat/OpenAiChatModelProxyToolCallsIT.java

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343
import org.springframework.ai.chat.prompt.Prompt;
4444
import org.springframework.ai.model.ModelOptionsUtils;
4545
import org.springframework.ai.model.function.FunctionCallback;
46-
import org.springframework.ai.model.function.ToolCallHelper;
46+
import org.springframework.ai.model.function.FunctionCallingHelper;
4747
import org.springframework.ai.openai.OpenAiChatModel;
4848
import org.springframework.ai.openai.OpenAiChatOptions;
4949
import org.springframework.ai.openai.api.OpenAiApi;
@@ -64,7 +64,7 @@ class OpenAiChatModelProxyToolCallsIT {
6464

6565
private static final String DEFAULT_MODEL = "gpt-4o-mini";
6666

67-
FunctionCallback functionDefinition = new ToolCallHelper.FunctionDefinition("getWeatherInLocation",
67+
FunctionCallback functionDefinition = new FunctionCallingHelper.FunctionDefinition("getWeatherInLocation",
6868
"Get the weather in location", """
6969
{
7070
"type": "object",
@@ -87,7 +87,7 @@ class OpenAiChatModelProxyToolCallsIT {
8787

8888
// Helper class that reuses some of the {@link AbstractToolCallSupport} functionality
8989
// to help to implement the function call handling logic on the client side.
90-
private ToolCallHelper toolCallHelper = new ToolCallHelper();
90+
private FunctionCallingHelper functionCallingHelper = new FunctionCallingHelper();
9191

9292
@SuppressWarnings("unchecked")
9393
private static Map<String, String> getFunctionArguments(String functionArguments) {
@@ -139,7 +139,7 @@ void functionCall() throws JsonMappingException, JsonProcessingException {
139139

140140
// Note that the tool call check could be platform specific because the finish
141141
// reasons.
142-
isToolCall = this.toolCallHelper.isToolCall(chatResponse,
142+
isToolCall = this.functionCallingHelper.isToolCall(chatResponse,
143143
Set.of(OpenAiApi.ChatCompletionFinishReason.TOOL_CALLS.name(),
144144
OpenAiApi.ChatCompletionFinishReason.STOP.name()));
145145

@@ -176,7 +176,7 @@ void functionCall() throws JsonMappingException, JsonProcessingException {
176176

177177
ToolResponseMessage toolMessageResponse = new ToolResponseMessage(toolResponses, Map.of());
178178

179-
List<Message> toolCallConversation = this.toolCallHelper
179+
List<Message> toolCallConversation = this.functionCallingHelper
180180
.buildToolCallConversation(prompt.getInstructions(), assistantMessage, toolMessageResponse);
181181

182182
assertThat(toolCallConversation).isNotEmpty();
@@ -236,7 +236,7 @@ private Flux<ChatResponse> processToolCall(Prompt prompt, Set<String> finishReas
236236

237237
return chatResponses.flatMap(chatResponse -> {
238238

239-
boolean isToolCall = this.toolCallHelper.isToolCall(chatResponse, finishReasons);
239+
boolean isToolCall = this.functionCallingHelper.isToolCall(chatResponse, finishReasons);
240240

241241
if (isToolCall) {
242242

@@ -261,7 +261,7 @@ private Flux<ChatResponse> processToolCall(Prompt prompt, Set<String> finishReas
261261

262262
ToolResponseMessage toolMessageResponse = new ToolResponseMessage(toolResponses, Map.of());
263263

264-
List<Message> toolCallConversation = this.toolCallHelper
264+
List<Message> toolCallConversation = this.functionCallingHelper
265265
.buildToolCallConversation(prompt.getInstructions(), assistantMessage, toolMessageResponse);
266266

267267
assertThat(toolCallConversation).isNotEmpty();
@@ -285,7 +285,7 @@ void functionCall2() throws JsonMappingException, JsonProcessingException {
285285

286286
var prompt = new Prompt(messages, promptOptions);
287287

288-
ChatResponse chatResponse = this.toolCallHelper.processCall(this.chatModel, prompt,
288+
ChatResponse chatResponse = this.functionCallingHelper.processCall(this.chatModel, prompt,
289289
Set.of(OpenAiApi.ChatCompletionFinishReason.TOOL_CALLS.name(),
290290
OpenAiApi.ChatCompletionFinishReason.STOP.name()),
291291
toolCall -> {
@@ -319,7 +319,7 @@ void functionStream2() throws JsonMappingException, JsonProcessingException {
319319

320320
var prompt = new Prompt(messages, promptOptions);
321321

322-
Flux<ChatResponse> responses = this.toolCallHelper.processStream(this.chatModel, prompt,
322+
Flux<ChatResponse> responses = this.functionCallingHelper.processStream(this.chatModel, prompt,
323323
Set.of(OpenAiApi.ChatCompletionFinishReason.TOOL_CALLS.name(),
324324
OpenAiApi.ChatCompletionFinishReason.STOP.name()),
325325
toolCall -> {
Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,10 @@
4040
@SpringBootTest(classes = OpenAiTestConfiguration.class)
4141
@EnabledIfEnvironmentVariable(named = "OPENAI_API_KEY", matches = ".+")
4242
@ActiveProfiles("logging-test")
43-
class OpenAiChatClientMethodFunctionCallbackIT {
43+
class OpenAiChatClientMethodInvokingFunctionCallbackIT {
4444

45-
private static final Logger logger = LoggerFactory.getLogger(OpenAiChatClientMethodFunctionCallbackIT.class);
45+
private static final Logger logger = LoggerFactory
46+
.getLogger(OpenAiChatClientMethodInvokingFunctionCallbackIT.class);
4647

4748
public static Map<String, Object> arguments = new ConcurrentHashMap<>();
4849

models/spring-ai-openai/src/test/java/org/springframework/ai/openai/chat/client/OpenAiChatClientProxyFunctionCallsIT.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
import org.springframework.ai.chat.model.Generation;
4040
import org.springframework.ai.model.ModelOptionsUtils;
4141
import org.springframework.ai.model.function.FunctionCallback;
42-
import org.springframework.ai.model.function.ToolCallHelper;
42+
import org.springframework.ai.model.function.FunctionCallingHelper;
4343
import org.springframework.ai.openai.OpenAiChatModel;
4444
import org.springframework.ai.openai.OpenAiChatOptions;
4545
import org.springframework.ai.openai.OpenAiTestConfiguration;
@@ -64,7 +64,7 @@ class OpenAiChatClientProxyFunctionCallsIT extends AbstractIT {
6464
@Value("classpath:/prompts/system-message.st")
6565
private Resource systemTextResource;
6666

67-
FunctionCallback functionDefinition = new ToolCallHelper.FunctionDefinition("getWeatherInLocation",
67+
FunctionCallback functionDefinition = new FunctionCallingHelper.FunctionDefinition("getWeatherInLocation",
6868
"Get the weather in location", """
6969
{
7070
"type": "object",
@@ -87,7 +87,7 @@ class OpenAiChatClientProxyFunctionCallsIT extends AbstractIT {
8787

8888
// Helper class that reuses some of the {@link AbstractToolCallSupport} functionality
8989
// to help to implement the function call handling logic on the client side.
90-
private ToolCallHelper toolCallHelper = new ToolCallHelper();
90+
private FunctionCallingHelper functionCallingHelper = new FunctionCallingHelper();
9191

9292
// Function which will be called by the AI model.
9393
private String getWeatherInLocation(String location, String unit) {
@@ -130,7 +130,7 @@ void toolProxyFunctionCall() throws JsonMappingException, JsonProcessingExceptio
130130

131131
// Note that the tool call check could be platform specific because the finish
132132
// reasons.
133-
isToolCall = this.toolCallHelper.isToolCall(chatResponse,
133+
isToolCall = this.functionCallingHelper.isToolCall(chatResponse,
134134
Set.of(OpenAiApi.ChatCompletionFinishReason.TOOL_CALLS.name(),
135135
OpenAiApi.ChatCompletionFinishReason.STOP.name()));
136136

@@ -167,7 +167,7 @@ void toolProxyFunctionCall() throws JsonMappingException, JsonProcessingExceptio
167167

168168
ToolResponseMessage toolMessageResponse = new ToolResponseMessage(toolResponses, Map.of());
169169

170-
messages = this.toolCallHelper.buildToolCallConversation(messages, assistantMessage,
170+
messages = this.functionCallingHelper.buildToolCallConversation(messages, assistantMessage,
171171
toolMessageResponse);
172172

173173
assertThat(messages).isNotEmpty();

spring-ai-core/src/main/java/org/springframework/ai/chat/client/DefaultChatClient.java

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -836,11 +836,7 @@ public <T extends ChatOptions> ChatClientRequestSpec options(T options) {
836836
return this;
837837
}
838838

839-
/**
840-
* @deprecated in favor of
841-
* {@link #function(String, String, Class, java.util.function.Function)}
842-
*/
843-
@Deprecated
839+
@Override
844840
public <I, O> ChatClientRequestSpec function(String name, String description,
845841
java.util.function.Function<I, O> function) {
846842
Assert.hasText(name, "name cannot be null or empty");
@@ -856,6 +852,7 @@ public <I, O> ChatClientRequestSpec function(String name, String description,
856852
return this;
857853
}
858854

855+
@Override
859856
public <I, O> ChatClientRequestSpec function(String name, String description,
860857
java.util.function.BiFunction<I, ToolContext, O> biFunction) {
861858

@@ -872,6 +869,7 @@ public <I, O> ChatClientRequestSpec function(String name, String description,
872869
return this;
873870
}
874871

872+
@Override
875873
public <I, O> ChatClientRequestSpec function(String name, String description, @Nullable Class<I> inputType,
876874
java.util.function.Function<I, O> function) {
877875

spring-ai-core/src/main/java/org/springframework/ai/model/function/DefaultFunctionCallbackBuilder.java

Lines changed: 36 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.util.function.BiFunction;
2121
import java.util.function.Function;
2222

23+
import com.fasterxml.jackson.core.JsonProcessingException;
2324
import com.fasterxml.jackson.databind.DeserializationFeature;
2425
import com.fasterxml.jackson.databind.ObjectMapper;
2526
import com.fasterxml.jackson.databind.SerializationFeature;
@@ -30,8 +31,8 @@
3031
import org.springframework.ai.chat.model.ToolContext;
3132
import org.springframework.ai.model.ModelOptionsUtils;
3233
import org.springframework.ai.model.function.FunctionCallback.Builder;
33-
import org.springframework.ai.model.function.FunctionCallback.FunctionInvokerBuilder;
34-
import org.springframework.ai.model.function.FunctionCallback.MethodInvokerBuilder;
34+
import org.springframework.ai.model.function.FunctionCallback.FunctionInvokingSpec;
35+
import org.springframework.ai.model.function.FunctionCallback.MethodInvokingSpec;
3536
import org.springframework.ai.model.function.FunctionCallbackContext.SchemaType;
3637
import org.springframework.ai.util.JacksonUtils;
3738
import org.springframework.ai.util.ParsingUtils;
@@ -64,8 +65,8 @@ public class DefaultFunctionCallbackBuilder implements FunctionCallback.Builder
6465
* The function to convert the response object to a string. The default is to convert
6566
* the response to a JSON string.
6667
*/
67-
private Function<Object, String> responseConverter = input -> (input instanceof String) ? "" + input
68-
: ModelOptionsUtils.toJsonString(input);
68+
private Function<Object, String> responseConverter = response -> (response instanceof String) ? "" + response
69+
: this.toJsonString(response);
6970

7071
/**
7172
* (Optional) Instead of generating the input type schema from the input type or
@@ -80,6 +81,15 @@ public class DefaultFunctionCallbackBuilder implements FunctionCallback.Builder
8081
.disable(SerializationFeature.FAIL_ON_EMPTY_BEANS)
8182
.build();
8283

84+
private String toJsonString(Object object) {
85+
try {
86+
return this.objectMapper.writeValueAsString(object);
87+
}
88+
catch (JsonProcessingException e) {
89+
throw new RuntimeException(e);
90+
}
91+
}
92+
8393
@Override
8494
public Builder description(String description) {
8595
Assert.hasText(description, "Description must not be empty");
@@ -116,21 +126,21 @@ public Builder objectMapper(ObjectMapper objectMapper) {
116126
}
117127

118128
@Override
119-
public <I, O> FunctionInvokerBuilder<I, O> function(String name, Function<I, O> function) {
120-
return new FunctionInvokerBuilderImpl<>(name, function);
129+
public <I, O> FunctionInvokingSpec<I, O> function(String name, Function<I, O> function) {
130+
return new DefaultFunctionInvokingSpec<>(name, function);
121131
}
122132

123133
@Override
124-
public <I, O> FunctionInvokerBuilder<I, O> function(String name, BiFunction<I, ToolContext, O> biFunction) {
125-
return new FunctionInvokerBuilderImpl<>(name, biFunction);
134+
public <I, O> FunctionInvokingSpec<I, O> function(String name, BiFunction<I, ToolContext, O> biFunction) {
135+
return new DefaultFunctionInvokingSpec<>(name, biFunction);
126136
}
127137

128138
@Override
129-
public MethodInvokerBuilder method(String methodName, Class<?>... argumentTypes) {
130-
return new MethodInvokerBuilderImpl(methodName, argumentTypes);
139+
public MethodInvokingSpec method(String methodName, Class<?>... argumentTypes) {
140+
return new DefaultMethodInvokingSpec(methodName, argumentTypes);
131141
}
132142

133-
public class FunctionInvokerBuilderImpl<I, O> implements FunctionInvokerBuilder<I, O> {
143+
class DefaultFunctionInvokingSpec<I, O> implements FunctionInvokingSpec<I, O> {
134144

135145
private final String name;
136146

@@ -140,15 +150,15 @@ public class FunctionInvokerBuilderImpl<I, O> implements FunctionInvokerBuilder<
140150

141151
private final Function<I, O> function;
142152

143-
private FunctionInvokerBuilderImpl(String name, BiFunction<I, ToolContext, O> biFunction) {
153+
private DefaultFunctionInvokingSpec(String name, BiFunction<I, ToolContext, O> biFunction) {
144154
Assert.hasText(name, "Name must not be empty");
145155
Assert.notNull(biFunction, "BiFunction must not be null");
146156
this.name = name;
147157
this.biFunction = biFunction;
148158
this.function = null;
149159
}
150160

151-
private FunctionInvokerBuilderImpl(String name, Function<I, O> function) {
161+
private DefaultFunctionInvokingSpec(String name, Function<I, O> function) {
152162
Assert.hasText(name, "Name must not be empty");
153163
Assert.notNull(function, "Function must not be null");
154164
this.name = name;
@@ -157,14 +167,14 @@ private FunctionInvokerBuilderImpl(String name, Function<I, O> function) {
157167
}
158168

159169
@Override
160-
public FunctionInvokerBuilder<I, O> inputType(Class<?> inputType) {
170+
public FunctionInvokingSpec<I, O> inputType(Class<?> inputType) {
161171
Assert.notNull(inputType, "InputType must not be null");
162172
this.inputType = inputType;
163173
return this;
164174
}
165175

166176
@Override
167-
public FunctionInvokerBuilder<I, O> inputType(ParameterizedTypeReference<?> inputType) {
177+
public FunctionInvokingSpec<I, O> inputType(ParameterizedTypeReference<?> inputType) {
168178
Assert.notNull(inputType, "InputType must not be null");
169179
this.inputType = inputType.getType();
170180
;
@@ -187,8 +197,8 @@ public FunctionCallback build() {
187197
BiFunction<I, ToolContext, O> finalBiFunction = (this.biFunction != null) ? this.biFunction
188198
: (request, context) -> this.function.apply(request);
189199

190-
return new FunctionCallbackWrapper(this.name, this.getDescription(), inputTypeSchema, this.inputType,
191-
(Function<I, String>) responseConverter, objectMapper, finalBiFunction);
200+
return new FunctionInvokingFunctionCallback(this.name, this.getDescription(), inputTypeSchema,
201+
this.inputType, (Function<I, String>) responseConverter, objectMapper, finalBiFunction);
192202
}
193203

194204
private String getDescription() {
@@ -200,7 +210,7 @@ private String getDescription() {
200210

201211
}
202212

203-
public class MethodInvokerBuilderImpl implements FunctionCallback.MethodInvokerBuilder {
213+
class DefaultMethodInvokingSpec implements FunctionCallback.MethodInvokingSpec {
204214

205215
private String name;
206216

@@ -212,27 +222,27 @@ public class MethodInvokerBuilderImpl implements FunctionCallback.MethodInvokerB
212222

213223
private final Class<?>[] argumentTypes;
214224

215-
private MethodInvokerBuilderImpl(String methodName, Class<?>... argumentTypes) {
225+
private DefaultMethodInvokingSpec(String methodName, Class<?>... argumentTypes) {
216226
Assert.hasText(methodName, "Method name must not be null");
217227
Assert.notNull(argumentTypes, "Argument types must not be null");
218228
this.methodName = methodName;
219229
this.argumentTypes = argumentTypes;
220230
}
221231

222-
public MethodInvokerBuilder name(String name) {
232+
public MethodInvokingSpec name(String name) {
223233
Assert.hasText(name, "Name must not be empty");
224234
this.name = name;
225235
return this;
226236
}
227237

228-
public MethodInvokerBuilder targetClass(Class<?> targetClass) {
238+
public MethodInvokingSpec targetClass(Class<?> targetClass) {
229239
Assert.notNull(targetClass, "Target class must not be null");
230240
this.targetClass = targetClass;
231241
return this;
232242
}
233243

234244
@Override
235-
public MethodInvokerBuilder targetObject(Object methodObject) {
245+
public MethodInvokingSpec targetObject(Object methodObject) {
236246
Assert.notNull(methodObject, "Method object must not be null");
237247
this.targetObject = methodObject;
238248
this.targetClass = methodObject.getClass();
@@ -246,8 +256,8 @@ public FunctionCallback build() {
246256
var method = ReflectionUtils.findMethod(targetClass, methodName, argumentTypes);
247257
Assert.notNull(method,
248258
"Method: '" + methodName + "' with arguments:" + Arrays.toString(argumentTypes) + " not found!");
249-
return new MethodFunctionCallback(this.targetObject, method, this.getDescription(), objectMapper, this.name,
250-
responseConverter);
259+
return new MethodInvokingFunctionCallback(this.targetObject, method, this.getDescription(), objectMapper,
260+
this.name, responseConverter);
251261
}
252262

253263
private String getDescription() {
@@ -264,9 +274,9 @@ private String generateDescription(String fromName) {
264274

265275
String generatedDescription = ParsingUtils.reConcatenateCamelCase(fromName, " ");
266276

267-
logger.warn("Description is not set! A best effort attempt to generate a description:'{}' from the:'{}'",
277+
logger.info("Description is not set! A best effort attempt to generate a description:'{}' from the:'{}'",
268278
generatedDescription, fromName);
269-
logger.warn("It is recommended to set the Description explicitly! Use the 'description()' method!");
279+
logger.info("It is recommended to set the Description explicitly! Use the 'description()' method!");
270280

271281
return generatedDescription;
272282
}

0 commit comments

Comments
 (0)