Skip to content

Commit 07aaca3

Browse files
committed
Fix to make the property internalToolExecutionEnabled readable when options are merged
- In DefaultToolCallingChatOptions, the property internalToolExecutionEnabled was set via deprecated proxyTools property which is now removed. - Add getter method with the prefix "get" to make the BeanWrapperImpl recognize the reader method for internalToolExecutionEnabled - Fix ModelOptionsUtils to recognize the correct interface method for boolean property names
1 parent 62a455b commit 07aaca3

File tree

12 files changed

+112
-104
lines changed

12 files changed

+112
-104
lines changed

models/spring-ai-anthropic/src/main/java/org/springframework/ai/anthropic/AnthropicChatOptions.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,12 @@ public Boolean isInternalToolExecutionEnabled() {
220220
return internalToolExecutionEnabled;
221221
}
222222

223+
// This getter is used by the BeanWrapper when merging options.
224+
@Nullable
225+
public Boolean getInternalToolExecutionEnabled() {
226+
return isInternalToolExecutionEnabled();
227+
}
228+
223229
@Override
224230
@JsonIgnore
225231
public void setInternalToolExecutionEnabled(@Nullable Boolean internalToolExecutionEnabled) {

models/spring-ai-azure-openai/src/main/java/org/springframework/ai/azure/openai/AzureOpenAiChatOptions.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,12 @@ public Boolean isInternalToolExecutionEnabled() {
234234
return internalToolExecutionEnabled;
235235
}
236236

237+
// This getter is used by the BeanWrapper when merging options.
238+
@Nullable
239+
public Boolean getInternalToolExecutionEnabled() {
240+
return isInternalToolExecutionEnabled();
241+
}
242+
237243
@Override
238244
@JsonIgnore
239245
public void setInternalToolExecutionEnabled(@Nullable Boolean internalToolExecutionEnabled) {

models/spring-ai-mistral-ai/src/main/java/org/springframework/ai/mistralai/MistralAiChatOptions.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,12 @@ public Boolean isInternalToolExecutionEnabled() {
290290
return internalToolExecutionEnabled;
291291
}
292292

293+
// This getter is used by the BeanWrapper when merging options.
294+
@Nullable
295+
public Boolean getInternalToolExecutionEnabled() {
296+
return isInternalToolExecutionEnabled();
297+
}
298+
293299
@Override
294300
@JsonIgnore
295301
public void setInternalToolExecutionEnabled(@Nullable Boolean internalToolExecutionEnabled) {

models/spring-ai-ollama/src/main/java/org/springframework/ai/ollama/api/OllamaOptions.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -721,6 +721,12 @@ public Boolean isInternalToolExecutionEnabled() {
721721
return internalToolExecutionEnabled;
722722
}
723723

724+
// This getter is used by the BeanWrapper when merging options.
725+
@Nullable
726+
public Boolean getInternalToolExecutionEnabled() {
727+
return isInternalToolExecutionEnabled();
728+
}
729+
724730
@Override
725731
@JsonIgnore
726732
public void setInternalToolExecutionEnabled(@Nullable Boolean internalToolExecutionEnabled) {

models/spring-ai-openai/src/main/java/org/springframework/ai/openai/OpenAiChatModel.java

Lines changed: 46 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -212,16 +212,16 @@ public ChatResponse internalCall(Prompt prompt, ChatResponse previousChatRespons
212212
}
213213

214214
// @formatter:off
215-
List<Generation> generations = choices.stream().map(choice -> {
216-
Map<String, Object> metadata = Map.of(
217-
"id", chatCompletion.id() != null ? chatCompletion.id() : "",
218-
"role", choice.message().role() != null ? choice.message().role().name() : "",
219-
"index", choice.index(),
220-
"finishReason", choice.finishReason() != null ? choice.finishReason().name() : "",
221-
"refusal", StringUtils.hasText(choice.message().refusal()) ? choice.message().refusal() : "");
222-
return buildGeneration(choice, metadata, request);
223-
}).toList();
224-
// @formatter:on
215+
List<Generation> generations = choices.stream().map(choice -> {
216+
Map<String, Object> metadata = Map.of(
217+
"id", chatCompletion.id() != null ? chatCompletion.id() : "",
218+
"role", choice.message().role() != null ? choice.message().role().name() : "",
219+
"index", choice.index(),
220+
"finishReason", choice.finishReason() != null ? choice.finishReason().name() : "",
221+
"refusal", StringUtils.hasText(choice.message().refusal()) ? choice.message().refusal() : "");
222+
return buildGeneration(choice, metadata, request);
223+
}).toList();
224+
// @formatter:on
225225

226226
RateLimit rateLimit = OpenAiResponseHeaderExtractor.extractAiResponseHeaders(completionEntity);
227227

@@ -308,19 +308,19 @@ public Flux<ChatResponse> internalStream(Prompt prompt, ChatResponse previousCha
308308
String id = chatCompletion2.id();
309309

310310
List<Generation> generations = chatCompletion2.choices().stream().map(choice -> { // @formatter:off
311-
if (choice.message().role() != null) {
312-
roleMap.putIfAbsent(id, choice.message().role().name());
313-
}
314-
Map<String, Object> metadata = Map.of(
315-
"id", chatCompletion2.id(),
316-
"role", roleMap.getOrDefault(id, ""),
317-
"index", choice.index(),
318-
"finishReason", choice.finishReason() != null ? choice.finishReason().name() : "",
319-
"refusal", StringUtils.hasText(choice.message().refusal()) ? choice.message().refusal() : "");
320-
321-
return buildGeneration(choice, metadata, request);
322-
}).toList();
323-
// @formatter:on
311+
if (choice.message().role() != null) {
312+
roleMap.putIfAbsent(id, choice.message().role().name());
313+
}
314+
Map<String, Object> metadata = Map.of(
315+
"id", chatCompletion2.id(),
316+
"role", roleMap.getOrDefault(id, ""),
317+
"index", choice.index(),
318+
"finishReason", choice.finishReason() != null ? choice.finishReason().name() : "",
319+
"refusal", StringUtils.hasText(choice.message().refusal()) ? choice.message().refusal() : "");
320+
321+
return buildGeneration(choice, metadata, request);
322+
}).toList();
323+
// @formatter:on
324324
OpenAiApi.Usage usage = chatCompletion2.usage();
325325
Usage currentChatResponseUsage = usage != null ? getDefaultUsage(usage) : new EmptyUsage();
326326
Usage accumulatedUsage = UsageUtils.getCumulativeUsage(currentChatResponseUsage,
@@ -361,30 +361,30 @@ public Flux<ChatResponse> internalStream(Prompt prompt, ChatResponse previousCha
361361

362362
// @formatter:off
363363
Flux<ChatResponse> flux = chatResponse.flatMap(response -> {
364-
if (toolExecutionEligibilityPredicate.isToolExecutionRequired(prompt.getOptions(), response)) {
365-
return Flux.defer(() -> {
366-
// FIXME: bounded elastic needs to be used since tool calling
367-
// is currently only synchronous
368-
var toolExecutionResult = this.toolCallingManager.executeToolCalls(prompt, response);
369-
if (toolExecutionResult.returnDirect()) {
370-
// Return tool execution result directly to the client.
371-
return Flux.just(ChatResponse.builder().from(response)
372-
.generations(ToolExecutionResult.buildGenerations(toolExecutionResult))
373-
.build());
374-
} else {
375-
// Send the tool execution result back to the model.
376-
return this.internalStream(new Prompt(toolExecutionResult.conversationHistory(), prompt.getOptions()),
377-
response);
364+
if (toolExecutionEligibilityPredicate.isToolExecutionRequired(prompt.getOptions(), response)) {
365+
return Flux.defer(() -> {
366+
// FIXME: bounded elastic needs to be used since tool calling
367+
// is currently only synchronous
368+
var toolExecutionResult = this.toolCallingManager.executeToolCalls(prompt, response);
369+
if (toolExecutionResult.returnDirect()) {
370+
// Return tool execution result directly to the client.
371+
return Flux.just(ChatResponse.builder().from(response)
372+
.generations(ToolExecutionResult.buildGenerations(toolExecutionResult))
373+
.build());
374+
} else {
375+
// Send the tool execution result back to the model.
376+
return this.internalStream(new Prompt(toolExecutionResult.conversationHistory(), prompt.getOptions()),
377+
response);
378+
}
379+
}).subscribeOn(Schedulers.boundedElastic());
378380
}
379-
}).subscribeOn(Schedulers.boundedElastic());
380-
}
381-
else {
382-
return Flux.just(response);
383-
}
384-
})
385-
.doOnError(observation::error)
386-
.doFinally(s -> observation.stop())
387-
.contextWrite(ctx -> ctx.put(ObservationThreadLocalAccessor.KEY, observation));
381+
else {
382+
return Flux.just(response);
383+
}
384+
})
385+
.doOnError(observation::error)
386+
.doFinally(s -> observation.stop())
387+
.contextWrite(ctx -> ctx.put(ObservationThreadLocalAccessor.KEY, observation));
388388
// @formatter:on
389389

390390
return new MessageAggregator().aggregate(flux, observationContext::setResponse);

models/spring-ai-openai/src/main/java/org/springframework/ai/openai/OpenAiChatOptions.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,12 @@ public Boolean isInternalToolExecutionEnabled() {
492492
return internalToolExecutionEnabled;
493493
}
494494

495+
// This getter is used by the BeanWrapper when merging options.
496+
@Nullable
497+
public Boolean getInternalToolExecutionEnabled() {
498+
return isInternalToolExecutionEnabled();
499+
}
500+
495501
@Override
496502
@JsonIgnore
497503
public void setInternalToolExecutionEnabled(@Nullable Boolean internalToolExecutionEnabled) {

models/spring-ai-vertex-ai-gemini/src/main/java/org/springframework/ai/vertexai/gemini/VertexAiGeminiChatOptions.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,12 @@ public Boolean isInternalToolExecutionEnabled() {
263263
return internalToolExecutionEnabled;
264264
}
265265

266+
// This getter is used by the BeanWrapper when merging options.
267+
@Nullable
268+
public Boolean getInternalToolExecutionEnabled() {
269+
return isInternalToolExecutionEnabled();
270+
}
271+
266272
@Override
267273
public void setInternalToolExecutionEnabled(@Nullable Boolean internalToolExecutionEnabled) {
268274
this.internalToolExecutionEnabled = internalToolExecutionEnabled;

spring-ai-model/src/main/java/org/springframework/ai/model/ModelOptionsUtils.java

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import java.beans.PropertyDescriptor;
2020
import java.lang.reflect.Field;
21+
import java.lang.reflect.ParameterizedType;
2122
import java.lang.reflect.Type;
2223
import java.util.ArrayList;
2324
import java.util.Arrays;
@@ -61,6 +62,7 @@
6162
*
6263
* @author Christian Tzolov
6364
* @author Thomas Vitale
65+
* @author Ilayaperumal Gopinathan
6466
* @since 0.8.0
6567
*/
6668
public abstract class ModelOptionsUtils {
@@ -325,7 +327,7 @@ public static <I, S extends I, T extends S> T mergeBeans(S source, T target, Cla
325327
for (PropertyDescriptor descriptor : sourceBeanWrap.getPropertyDescriptors()) {
326328

327329
if (!BEAN_MERGE_FIELD_EXCISIONS.contains(descriptor.getName())
328-
&& interfaceNames.contains(toGetName(descriptor.getName()))) {
330+
&& interfaceNames.contains(toGetName(descriptor))) {
329331

330332
String propertyName = descriptor.getName();
331333
Object value = sourceBeanWrap.getPropertyValue(propertyName);
@@ -344,6 +346,17 @@ public static <I, S extends I, T extends S> T mergeBeans(S source, T target, Cla
344346
return target;
345347
}
346348

349+
private static String toGetName(PropertyDescriptor descriptor) {
350+
String name = descriptor.getName();
351+
Class propertyType = descriptor.getPropertyType();
352+
if (propertyType == Boolean.class) {
353+
return "is" + Character.toUpperCase(name.charAt(0)) + name.substring(1);
354+
}
355+
else {
356+
return "get" + Character.toUpperCase(name.charAt(0)) + name.substring(1);
357+
}
358+
}
359+
347360
private static String toGetName(String name) {
348361
return "get" + name.substring(0, 1).toUpperCase() + name.substring(1);
349362
}

spring-ai-model/src/main/java/org/springframework/ai/model/tool/DefaultToolCallingChatOptions.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,12 @@ public Boolean isInternalToolExecutionEnabled() {
113113
return this.internalToolExecutionEnabled;
114114
}
115115

116+
// This getter is used by the BeanWrapper when merging options.
117+
@Nullable
118+
public Boolean getInternalToolExecutionEnabled() {
119+
return isInternalToolExecutionEnabled();
120+
}
121+
116122
@Override
117123
public void setInternalToolExecutionEnabled(@Nullable Boolean internalToolExecutionEnabled) {
118124
this.internalToolExecutionEnabled = internalToolExecutionEnabled;

spring-ai-model/src/main/java/org/springframework/ai/model/tool/ToolCallingChatOptions.java

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
* including tool calling.
3939
*
4040
* @author Thomas Vitale
41+
* @author Ilayaperumal Gopinathan
4142
* @since 1.0.0
4243
*/
4344
public interface ToolCallingChatOptions extends ChatOptions {
@@ -77,9 +78,17 @@ public interface ToolCallingChatOptions extends ChatOptions {
7778
*/
7879
void setInternalToolExecutionEnabled(@Nullable Boolean internalToolExecutionEnabled);
7980

81+
/**
82+
* Get the configured tool context.
83+
* @return the tool context map.
84+
*/
8085
Map<String, Object> getToolContext();
8186

82-
void setToolContext(Map<String, Object> tooContext);
87+
/**
88+
* Set the tool context values as map.
89+
* @param toolContext
90+
*/
91+
void setToolContext(Map<String, Object> toolContext);
8392

8493
/**
8594
* A builder to create a new {@link ToolCallingChatOptions} instance.
@@ -172,10 +181,6 @@ static boolean isInternalToolExecutionEnabled(ChatOptions chatOptions) {
172181
&& toolCallingChatOptions.isInternalToolExecutionEnabled() != null) {
173182
internalToolExecutionEnabled = Boolean.TRUE.equals(toolCallingChatOptions.isInternalToolExecutionEnabled());
174183
}
175-
else if (chatOptions instanceof FunctionCallingOptions functionCallingOptions
176-
&& functionCallingOptions.getProxyToolCalls() != null) {
177-
internalToolExecutionEnabled = Boolean.TRUE.equals(!functionCallingOptions.getProxyToolCalls());
178-
}
179184
else {
180185
internalToolExecutionEnabled = DEFAULT_TOOL_EXECUTION_ENABLED;
181186
}

spring-ai-model/src/test/java/org/springframework/ai/model/tool/DefaultToolExecutionEligibilityPredicateTests.java

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -94,38 +94,6 @@ void whenToolExecutionDisabledAndNoToolCalls() {
9494
assertThat(result).isFalse();
9595
}
9696

97-
@Test
98-
void whenFunctionCallingOptionsAndToolExecutionEnabled() {
99-
// Create a FunctionCallingOptions with proxy tool calls disabled (which means
100-
// internal tool execution is enabled)
101-
FunctionCallingOptions options = FunctionCallingOptions.builder().proxyToolCalls(false).build();
102-
103-
// Create a ChatResponse with tool calls
104-
AssistantMessage.ToolCall toolCall = new AssistantMessage.ToolCall("id1", "function", "testTool", "{}");
105-
AssistantMessage assistantMessage = new AssistantMessage("test", Map.of(), List.of(toolCall));
106-
ChatResponse chatResponse = new ChatResponse(List.of(new Generation(assistantMessage)));
107-
108-
// Test the predicate
109-
boolean result = predicate.test(options, chatResponse);
110-
assertThat(result).isTrue();
111-
}
112-
113-
@Test
114-
void whenFunctionCallingOptionsAndToolExecutionDisabled() {
115-
// Create a FunctionCallingOptions with proxy tool calls enabled (which means
116-
// internal tool execution is disabled)
117-
FunctionCallingOptions options = FunctionCallingOptions.builder().proxyToolCalls(true).build();
118-
119-
// Create a ChatResponse with tool calls
120-
AssistantMessage.ToolCall toolCall = new AssistantMessage.ToolCall("id1", "function", "testTool", "{}");
121-
AssistantMessage assistantMessage = new AssistantMessage("test", Map.of(), List.of(toolCall));
122-
ChatResponse chatResponse = new ChatResponse(List.of(new Generation(assistantMessage)));
123-
124-
// Test the predicate
125-
boolean result = predicate.test(options, chatResponse);
126-
assertThat(result).isFalse();
127-
}
128-
12997
@Test
13098
void whenRegularChatOptionsAndHasToolCalls() {
13199
// Create regular ChatOptions (not ToolCallingChatOptions or

spring-ai-model/src/test/java/org/springframework/ai/model/tool/ToolCallingChatOptionsTests.java

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -56,26 +56,6 @@ void whenToolCallingChatOptionsAndExecutionEnabledDefault() {
5656
assertThat(ToolCallingChatOptions.isInternalToolExecutionEnabled(options)).isTrue();
5757
}
5858

59-
@Test
60-
void whenFunctionCallingOptionsAndExecutionEnabledTrue() {
61-
FunctionCallingOptions options = FunctionCallingOptions.builder().build();
62-
options.setProxyToolCalls(false);
63-
assertThat(ToolCallingChatOptions.isInternalToolExecutionEnabled(options)).isTrue();
64-
}
65-
66-
@Test
67-
void whenFunctionCallingOptionsAndExecutionEnabledFalse() {
68-
FunctionCallingOptions options = FunctionCallingOptions.builder().build();
69-
options.setProxyToolCalls(true);
70-
assertThat(ToolCallingChatOptions.isInternalToolExecutionEnabled(options)).isFalse();
71-
}
72-
73-
@Test
74-
void whenFunctionCallingOptionsAndExecutionEnabledDefault() {
75-
FunctionCallingOptions options = FunctionCallingOptions.builder().build();
76-
assertThat(ToolCallingChatOptions.isInternalToolExecutionEnabled(options)).isTrue();
77-
}
78-
7959
@Test
8060
void whenMergeRuntimeAndDefaultToolNames() {
8161
Set<String> runtimeToolNames = Set.of("toolA");

0 commit comments

Comments
 (0)