Skip to content

Commit 52ec056

Browse files
Aias00Aias00
authored
feat: update McpSchema.CallToolResult to use a builder pattern and refine OpenAI seed parameter application. (#201)
1 parent 18c0aac commit 52ec056

7 files changed

Lines changed: 152 additions & 42 deletions

File tree

‎agentscope-core/src/main/java/io/agentscope/core/formatter/openai/OpenAIToolsHelper.java‎

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,11 @@
3535
/**
3636
* Handles tool registration and options application for OpenAI API.
3737
*
38-
* <p>This class provides utility methods for:
38+
* <p>
39+
* This class provides utility methods for:
3940
* <ul>
40-
* <li>Applying generation options to OpenAI request parameters
41-
* <li>Converting AgentScope tool schemas to OpenAI tool definitions
41+
* <li>Applying generation options to OpenAI request parameters
42+
* <li>Converting AgentScope tool schemas to OpenAI tool definitions
4243
* </ul>
4344
*/
4445
public class OpenAIToolsHelper {
@@ -48,18 +49,20 @@ public class OpenAIToolsHelper {
4849
/**
4950
* Apply GenerateOptions to OpenAI ChatCompletionCreateParams.Builder.
5051
*
51-
* @param paramsBuilder OpenAI request parameters builder
52-
* @param options Generation options to apply
52+
* @param paramsBuilder OpenAI request parameters builder
53+
* @param options Generation options to apply
5354
* @param defaultOptions Default options to use if options parameter is null
54-
* @param optionGetter Function to get option value with fallback
55+
* @param optionGetter Function to get option value with fallback
5556
*/
57+
@SuppressWarnings("deprecation")
5658
public void applyOptions(
5759
ChatCompletionCreateParams.Builder paramsBuilder,
5860
GenerateOptions options,
5961
GenerateOptions defaultOptions,
6062
Function<Function<GenerateOptions, ?>, ?> optionGetter) {
6163

62-
// Apply each option individually, falling back to defaultOptions if the specific field is
64+
// Apply each option individually, falling back to defaultOptions if the
65+
// specific field is
6366
// null
6467
applyDoubleOption(
6568
optionGetter,
@@ -90,9 +93,25 @@ public void applyOptions(
9093

9194
// Apply seed parameter
9295
applyLongOption(
93-
optionGetter, GenerateOptions::getSeed, defaultOptions, paramsBuilder::seed);
96+
optionGetter,
97+
GenerateOptions::getSeed,
98+
defaultOptions,
99+
val -> {
100+
if (val > Integer.MAX_VALUE || val < Integer.MIN_VALUE) {
101+
throw new IllegalArgumentException(
102+
"Seed value "
103+
+ val
104+
+ " is out of int range ("
105+
+ Integer.MIN_VALUE
106+
+ " to "
107+
+ Integer.MAX_VALUE
108+
+ ")");
109+
}
110+
paramsBuilder.seed(val.intValue());
111+
});
94112

95-
// Apply additional parameters (merge defaultOptions first, then options to override)
113+
// Apply additional parameters (merge defaultOptions first, then options to
114+
// override)
96115
// Apply additional headers
97116
applyAdditionalHeaders(
98117
paramsBuilder,
@@ -239,7 +258,7 @@ private void applyLongOption(
239258
* Apply tool schemas to OpenAI ChatCompletionCreateParams.Builder.
240259
*
241260
* @param paramsBuilder OpenAI request parameters builder
242-
* @param tools List of tool schemas to apply (may be null or empty)
261+
* @param tools List of tool schemas to apply (may be null or empty)
243262
*/
244263
public void applyTools(
245264
ChatCompletionCreateParams.Builder paramsBuilder, List<ToolSchema> tools) {
@@ -291,7 +310,7 @@ public void applyTools(
291310
* Apply tool choice configuration to OpenAI request parameters.
292311
*
293312
* @param paramsBuilder OpenAI request parameters builder
294-
* @param toolChoice Tool choice configuration (null means auto)
313+
* @param toolChoice Tool choice configuration (null means auto)
295314
*/
296315
public void applyToolChoice(
297316
ChatCompletionCreateParams.Builder paramsBuilder, ToolChoice toolChoice) {

‎agentscope-core/src/test/java/io/agentscope/core/formatter/openai/OpenAIToolsHelperMockTest.java‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/*
2+
* Copyright 2024-2025 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package io.agentscope.core.formatter.openai;
17+
18+
import static org.mockito.ArgumentMatchers.anyInt;
19+
import static org.mockito.Mockito.mock;
20+
import static org.mockito.Mockito.verify;
21+
import static org.mockito.Mockito.when;
22+
23+
import com.openai.models.chat.completions.ChatCompletionCreateParams;
24+
import io.agentscope.core.model.GenerateOptions;
25+
import org.junit.jupiter.api.BeforeEach;
26+
import org.junit.jupiter.api.Test;
27+
28+
@SuppressWarnings("deprecation")
29+
class OpenAIToolsHelperMockTest {
30+
31+
private OpenAIToolsHelper helper;
32+
private ChatCompletionCreateParams.Builder mockBuilder;
33+
34+
@BeforeEach
35+
void setUp() {
36+
helper = new OpenAIToolsHelper();
37+
mockBuilder = mock(ChatCompletionCreateParams.Builder.class);
38+
// Ensure builder methods return the builder for chaining if needed
39+
when(mockBuilder.temperature(org.mockito.ArgumentMatchers.anyDouble()))
40+
.thenReturn(mockBuilder);
41+
when(mockBuilder.seed(anyInt())).thenReturn(mockBuilder);
42+
}
43+
44+
@Test
45+
void testApplyOptions_SetsSeed() {
46+
long seedValue = 12345L;
47+
GenerateOptions options = GenerateOptions.builder().seed(seedValue).build();
48+
49+
helper.applyOptions(mockBuilder, options, null, getter -> getter.apply(options));
50+
51+
verify(mockBuilder).seed((int) seedValue);
52+
}
53+
}

‎agentscope-core/src/test/java/io/agentscope/core/tool/mcp/McpAsyncClientWrapperTest.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,10 @@ void testCallTool_Success() {
161161
McpSchema.TextContent resultContent =
162162
new McpSchema.TextContent("Tool executed successfully");
163163
McpSchema.CallToolResult callResult =
164-
new McpSchema.CallToolResult(List.of(resultContent), false);
164+
McpSchema.CallToolResult.builder()
165+
.content(List.of(resultContent))
166+
.isError(false)
167+
.build();
165168

166169
when(mockClient.callTool(any(McpSchema.CallToolRequest.class)))
167170
.thenReturn(Mono.just(callResult));

‎agentscope-core/src/test/java/io/agentscope/core/tool/mcp/McpClientWrapperTest.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,11 @@ public Mono<McpSchema.CallToolResult> callTool(
227227

228228
// Return a simple success result
229229
McpSchema.TextContent content = new McpSchema.TextContent("Success");
230-
return Mono.just(new McpSchema.CallToolResult(List.of(content), false));
230+
return Mono.just(
231+
McpSchema.CallToolResult.builder()
232+
.content(List.of(content))
233+
.isError(false)
234+
.build());
231235
}
232236

233237
@Override

‎agentscope-core/src/test/java/io/agentscope/core/tool/mcp/McpContentConverterTest.java‎

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,10 @@ void testConvertCallToolResult_Success() {
5353
// Create successful MCP result with text content
5454
McpSchema.TextContent textContent = new McpSchema.TextContent("Operation successful");
5555
McpSchema.CallToolResult mcpResult =
56-
new McpSchema.CallToolResult(List.of(textContent), false);
56+
McpSchema.CallToolResult.builder()
57+
.content(List.of(textContent))
58+
.isError(false)
59+
.build();
5760

5861
ToolResultBlock result = McpContentConverter.convertCallToolResult(mcpResult);
5962

@@ -71,7 +74,10 @@ void testConvertCallToolResult_Error() {
7174
// Create error MCP result
7275
McpSchema.TextContent errorContent = new McpSchema.TextContent("File not found");
7376
McpSchema.CallToolResult mcpResult =
74-
new McpSchema.CallToolResult(List.of(errorContent), true);
77+
McpSchema.CallToolResult.builder()
78+
.content(List.of(errorContent))
79+
.isError(true)
80+
.build();
7581

7682
ToolResultBlock result = McpContentConverter.convertCallToolResult(mcpResult);
7783

@@ -100,7 +106,8 @@ void testConvertCallToolResult_NullResult() {
100106
@Test
101107
void testConvertCallToolResult_EmptyContent() {
102108
// Create result with empty content list
103-
McpSchema.CallToolResult mcpResult = new McpSchema.CallToolResult(List.of(), false);
109+
McpSchema.CallToolResult mcpResult =
110+
McpSchema.CallToolResult.builder().content(List.of()).isError(false).build();
104111

105112
ToolResultBlock result = McpContentConverter.convertCallToolResult(mcpResult);
106113

@@ -119,7 +126,10 @@ void testConvertCallToolResult_MultipleErrorMessages() {
119126
McpSchema.TextContent error1 = new McpSchema.TextContent("Error 1");
120127
McpSchema.TextContent error2 = new McpSchema.TextContent("Error 2");
121128
McpSchema.CallToolResult mcpResult =
122-
new McpSchema.CallToolResult(List.of(error1, error2), true);
129+
McpSchema.CallToolResult.builder()
130+
.content(List.of(error1, error2))
131+
.isError(true)
132+
.build();
123133

124134
ToolResultBlock result = McpContentConverter.convertCallToolResult(mcpResult);
125135

@@ -340,7 +350,8 @@ void testConvertContent_NullContent() {
340350
void testExtractErrorMessage_EmptyContent() {
341351
// Test indirect through convertCallToolResult with empty error content
342352
List<McpSchema.Content> emptyList = List.of();
343-
McpSchema.CallToolResult mcpResult = new McpSchema.CallToolResult(emptyList, true);
353+
McpSchema.CallToolResult mcpResult =
354+
McpSchema.CallToolResult.builder().content(emptyList).isError(true).build();
344355

345356
ToolResultBlock result = McpContentConverter.convertCallToolResult(mcpResult);
346357

@@ -356,19 +367,10 @@ void testExtractErrorMessage_EmptyContent() {
356367

357368
@Test
358369
void testExtractErrorMessage_NullContent() {
359-
// Test indirect through convertCallToolResult with null content
360-
List<McpSchema.Content> nullList = null;
361-
McpSchema.CallToolResult mcpResult = new McpSchema.CallToolResult(nullList, true);
362-
363-
ToolResultBlock result = McpContentConverter.convertCallToolResult(mcpResult);
364-
365-
assertNotNull(result);
366-
List<ContentBlock> outputs = result.getOutput();
367-
assertEquals(1, outputs.size());
368-
assertTrue(outputs.get(0) instanceof TextBlock);
369-
String text = ((TextBlock) outputs.get(0)).getText();
370-
assertTrue(text.startsWith("Error:"));
371-
assertTrue(text.contains("Unknown error"));
370+
// Validation: CallToolResult content cannot be null in newer MCP SDK versions
371+
assertThrows(
372+
IllegalArgumentException.class,
373+
() -> McpSchema.CallToolResult.builder().content(null));
372374
}
373375

374376
@Test
@@ -380,7 +382,8 @@ void testComplexScenario_MultipleContentTypes() {
380382
McpSchema.AudioContent audio = new McpSchema.AudioContent(null, "audiodata", "audio/wav");
381383

382384
List<McpSchema.Content> contents = List.of(text1, image, text2, audio);
383-
McpSchema.CallToolResult mcpResult = new McpSchema.CallToolResult(contents, false);
385+
McpSchema.CallToolResult mcpResult =
386+
McpSchema.CallToolResult.builder().content(contents).isError(false).build();
384387

385388
ToolResultBlock result = McpContentConverter.convertCallToolResult(mcpResult);
386389

@@ -397,7 +400,8 @@ void testComplexScenario_MultipleContentTypes() {
397400
}
398401

399402
// Note: Cannot test unknown ResourceContents type because it's a sealed class
400-
// The MCP SDK only allows TextResourceContents and BlobResourceContents implementations
403+
// The MCP SDK only allows TextResourceContents and BlobResourceContents
404+
// implementations
401405

402406
@Test
403407
void testContentList_AllNullElements() {

‎agentscope-core/src/test/java/io/agentscope/core/tool/mcp/McpSyncClientWrapperTest.java‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,10 @@ void testCallTool_Success() {
183183
McpSchema.TextContent resultContent =
184184
new McpSchema.TextContent("Tool executed successfully");
185185
McpSchema.CallToolResult callResult =
186-
new McpSchema.CallToolResult(List.of(resultContent), false);
186+
McpSchema.CallToolResult.builder()
187+
.content(List.of(resultContent))
188+
.isError(false)
189+
.build();
187190

188191
when(mockClient.callTool(any(McpSchema.CallToolRequest.class))).thenReturn(callResult);
189192

@@ -200,7 +203,10 @@ void testCallTool_ReturnsError() {
200203

201204
McpSchema.TextContent errorContent = new McpSchema.TextContent("Tool execution failed");
202205
McpSchema.CallToolResult callResult =
203-
new McpSchema.CallToolResult(List.of(errorContent), true);
206+
McpSchema.CallToolResult.builder()
207+
.content(List.of(errorContent))
208+
.isError(true)
209+
.build();
204210

205211
when(mockClient.callTool(any(McpSchema.CallToolRequest.class))).thenReturn(callResult);
206212

@@ -305,7 +311,10 @@ void testCallTool_WithNullArguments() {
305311

306312
McpSchema.TextContent resultContent = new McpSchema.TextContent("Success");
307313
McpSchema.CallToolResult callResult =
308-
new McpSchema.CallToolResult(List.of(resultContent), false);
314+
McpSchema.CallToolResult.builder()
315+
.content(List.of(resultContent))
316+
.isError(false)
317+
.build();
309318

310319
when(mockClient.callTool(any(McpSchema.CallToolRequest.class))).thenReturn(callResult);
311320

‎agentscope-core/src/test/java/io/agentscope/core/tool/mcp/McpToolTest.java‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,10 @@ void testCallAsync_Success() {
128128

129129
McpSchema.TextContent resultContent = new McpSchema.TextContent("Success result");
130130
McpSchema.CallToolResult mcpResult =
131-
new McpSchema.CallToolResult(List.of(resultContent), false);
131+
McpSchema.CallToolResult.builder()
132+
.content(List.of(resultContent))
133+
.isError(false)
134+
.build();
132135

133136
when(mockClientWrapper.callTool(eq("test-tool"), any())).thenReturn(Mono.just(mcpResult));
134137

@@ -148,7 +151,10 @@ void testCallAsync_WithEmptyInput() {
148151

149152
McpSchema.TextContent resultContent = new McpSchema.TextContent("Success");
150153
McpSchema.CallToolResult mcpResult =
151-
new McpSchema.CallToolResult(List.of(resultContent), false);
154+
McpSchema.CallToolResult.builder()
155+
.content(List.of(resultContent))
156+
.isError(false)
157+
.build();
152158

153159
when(mockClientWrapper.callTool(eq("test-tool"), any())).thenReturn(Mono.just(mcpResult));
154160

@@ -166,7 +172,10 @@ void testCallAsync_WithNullInput() {
166172

167173
McpSchema.TextContent resultContent = new McpSchema.TextContent("Success");
168174
McpSchema.CallToolResult mcpResult =
169-
new McpSchema.CallToolResult(List.of(resultContent), false);
175+
McpSchema.CallToolResult.builder()
176+
.content(List.of(resultContent))
177+
.isError(false)
178+
.build();
170179

171180
when(mockClientWrapper.callTool(eq("test-tool"), any())).thenReturn(Mono.just(mcpResult));
172181

@@ -192,7 +201,10 @@ void testCallAsync_MergesPresetArguments() {
192201

193202
McpSchema.TextContent resultContent = new McpSchema.TextContent("Success");
194203
McpSchema.CallToolResult mcpResult =
195-
new McpSchema.CallToolResult(List.of(resultContent), false);
204+
McpSchema.CallToolResult.builder()
205+
.content(List.of(resultContent))
206+
.isError(false)
207+
.build();
196208

197209
when(mockClientWrapper.callTool(eq("test-tool"), any())).thenReturn(Mono.just(mcpResult));
198210

@@ -218,7 +230,10 @@ void testCallAsync_PresetArgsOnly() {
218230

219231
McpSchema.TextContent resultContent = new McpSchema.TextContent("Success");
220232
McpSchema.CallToolResult mcpResult =
221-
new McpSchema.CallToolResult(List.of(resultContent), false);
233+
McpSchema.CallToolResult.builder()
234+
.content(List.of(resultContent))
235+
.isError(false)
236+
.build();
222237

223238
when(mockClientWrapper.callTool(eq("test-tool"), any())).thenReturn(Mono.just(mcpResult));
224239

@@ -384,7 +399,10 @@ void testMergeArguments_BothEmpty() {
384399

385400
McpSchema.TextContent resultContent = new McpSchema.TextContent("Success");
386401
McpSchema.CallToolResult mcpResult =
387-
new McpSchema.CallToolResult(List.of(resultContent), false);
402+
McpSchema.CallToolResult.builder()
403+
.content(List.of(resultContent))
404+
.isError(false)
405+
.build();
388406

389407
when(mockClientWrapper.callTool(eq("test-tool"), any())).thenReturn(Mono.just(mcpResult));
390408

0 commit comments

Comments
 (0)