Skip to content

Commit 762672e

Browse files
LearningGpLearningGp
authored
fix(tool): hoist $defs to schema root for nested object $ref resolution (#893) (#1077)
## AgentScope-Java Version [The version of AgentScope-Java you are working on, e.g. 1.0.9, check your pom.xml dependency version or run `mvn dependency:tree | grep agentscope-parent:pom`(only mac/linux)] ## Description close #893 ## Checklist Please check the following items before code is ready to be reviewed. - [x] Code has been formatted with `mvn spotless:apply` - [x] All tests are passing (`mvn test`) - [x] Javadoc comments are complete and follow project conventions - [x] Related documentation has been updated (e.g. links, examples, etc.) - [x] Code is ready for review
1 parent f090a05 commit 762672e

3 files changed

Lines changed: 248 additions & 0 deletions

File tree

‎agentscope-core/src/main/java/io/agentscope/core/tool/ToolSchemaGenerator.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,14 @@ class ToolSchemaGenerator {
4545
* be null or empty)
4646
* @return JSON Schema map in OpenAI format
4747
*/
48+
@SuppressWarnings("unchecked")
4849
Map<String, Object> generateParameterSchema(Method method, Set<String> excludeParams) {
4950
Map<String, Object> schema = new HashMap<>();
5051
schema.put("type", "object");
5152

5253
Map<String, Object> properties = new HashMap<>();
5354
List<String> required = new ArrayList<>();
55+
Map<String, Object> allDefs = new HashMap<>();
5456

5557
Parameter[] parameters = method.getParameters();
5658
for (Parameter param : parameters) {
@@ -67,6 +69,11 @@ Map<String, Object> generateParameterSchema(Method method, Set<String> excludePa
6769
continue;
6870
}
6971

72+
// Hoist $defs from per-parameter schema to the root level so that
73+
// $ref pointers like "#/$defs/TypeName" resolve against the document root.
74+
hoistDefs(info.schema, "$defs", allDefs);
75+
hoistDefs(info.schema, "definitions", allDefs);
76+
7077
properties.put(info.name, info.schema);
7178
if (info.required) {
7279
required.add(info.name);
@@ -77,10 +84,25 @@ Map<String, Object> generateParameterSchema(Method method, Set<String> excludePa
7784
if (!required.isEmpty()) {
7885
schema.put("required", required);
7986
}
87+
if (!allDefs.isEmpty()) {
88+
schema.put("$defs", allDefs);
89+
}
8090

8191
return schema;
8292
}
8393

94+
// TODO: putAll silently overwrites when different parameters define the same def key
95+
// (e.g. different classes with the same simple name under PLAIN_DEFINITION_KEYS).
96+
// Add value-equality check and fail-fast on true conflicts in a follow-up.
97+
@SuppressWarnings("unchecked")
98+
private void hoistDefs(
99+
Map<String, Object> paramSchema, String key, Map<String, Object> target) {
100+
Object raw = paramSchema.remove(key);
101+
if (raw instanceof Map<?, ?> defs && !defs.isEmpty()) {
102+
target.putAll((Map<String, Object>) defs);
103+
}
104+
}
105+
84106
/**
85107
* Extract parameter information from a Parameter with @ToolParam annotation.
86108
*

‎agentscope-core/src/test/java/io/agentscope/core/tool/ToolValidatorTest.java‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,102 @@ void testNestedObjectValidation() {
529529
}
530530
}
531531

532+
@Nested
533+
@DisplayName("validateInput - Nested Object with $ref (Issue #893)")
534+
class ValidateInputNestedObjectWithRef {
535+
536+
/**
537+
* Regression test for https://github.com/agentscope-ai/agentscope-java/issues/893
538+
*
539+
* After the fix, ToolSchemaGenerator hoists $defs to the schema root so that
540+
* $ref "#/$defs/Material" resolves correctly against the document root.
541+
*/
542+
@Test
543+
@DisplayName("Should resolve $ref when $defs is at schema root (Issue #893 fix)")
544+
void testRootDefsRefResolution() {
545+
Map<String, Object> materialDef =
546+
Map.of(
547+
"type",
548+
"object",
549+
"properties",
550+
Map.of(
551+
"key", Map.of("type", "string"),
552+
"value", Map.of("type", "string")));
553+
554+
Map<String, Object> requestSchema =
555+
Map.of(
556+
"type",
557+
"object",
558+
"properties",
559+
Map.of(
560+
"materialList",
561+
Map.of(
562+
"type",
563+
"array",
564+
"items",
565+
Map.of("$ref", "#/$defs/Material"))));
566+
567+
// After the fix, $defs is hoisted to the tool schema root.
568+
Map<String, Object> toolSchema =
569+
Map.of(
570+
"type", "object",
571+
"$defs", Map.of("Material", materialDef),
572+
"properties", Map.of("request", requestSchema),
573+
"required", List.of("request"));
574+
575+
String validInput =
576+
"{\"request\":{\"materialList\":[{\"key\":\"aaa\",\"value\":\"bbb\"}]}}";
577+
578+
String result = ToolValidator.validateInput(validInput, toolSchema);
579+
assertNull(result, "Validation should pass with $defs at root, but got: " + result);
580+
}
581+
582+
/**
583+
* Demonstrates the original bug: nested $defs cannot be resolved.
584+
*/
585+
@Test
586+
@DisplayName("Should fail when $defs is nested inside a property (original bug)")
587+
void testNestedDefsRefFailsWithoutFix() {
588+
Map<String, Object> materialDef =
589+
Map.of(
590+
"type",
591+
"object",
592+
"properties",
593+
Map.of(
594+
"key", Map.of("type", "string"),
595+
"value", Map.of("type", "string")));
596+
597+
Map<String, Object> requestSchema =
598+
Map.of(
599+
"$defs", Map.of("Material", materialDef),
600+
"type", "object",
601+
"properties",
602+
Map.of(
603+
"materialList",
604+
Map.of(
605+
"type",
606+
"array",
607+
"items",
608+
Map.of("$ref", "#/$defs/Material"))));
609+
610+
// Before the fix, $defs was nested inside properties.request
611+
Map<String, Object> toolSchema =
612+
Map.of(
613+
"type", "object",
614+
"properties", Map.of("request", requestSchema),
615+
"required", List.of("request"));
616+
617+
String validInput =
618+
"{\"request\":{\"materialList\":[{\"key\":\"aaa\",\"value\":\"bbb\"}]}}";
619+
620+
String result = ToolValidator.validateInput(validInput, toolSchema);
621+
assertNotNull(result, "Nested $defs should fail to resolve");
622+
assertTrue(
623+
result.contains("cannot be resolved"),
624+
"Error should mention unresolved reference, but got: " + result);
625+
}
626+
}
627+
532628
// ==================== validateToolResultMatch Tests ====================
533629

534630
@Nested

‎agentscope-core/src/test/java/io/agentscope/core/tool/ToolkitTest.java‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -899,6 +899,136 @@ public Mono<ToolResultBlock> testToolMethod(
899899
}
900900
}
901901

902+
// ==================== Nested Object $ref Tests (Issue #893) ====================
903+
904+
/**
905+
* POJO that victools will generate $defs/$ref for when used in a List.
906+
*/
907+
public static class Material {
908+
private String key;
909+
private String value;
910+
911+
public Material() {}
912+
913+
public Material(String key, String value) {
914+
this.key = key;
915+
this.value = value;
916+
}
917+
918+
public String getKey() {
919+
return key;
920+
}
921+
922+
public void setKey(String key) {
923+
this.key = key;
924+
}
925+
926+
public String getValue() {
927+
return value;
928+
}
929+
930+
public void setValue(String value) {
931+
this.value = value;
932+
}
933+
}
934+
935+
/**
936+
* Nested request object containing lists of Material,
937+
* which triggers victools to produce $defs + $ref in the schema.
938+
*/
939+
public static class CreateRequest {
940+
private java.util.List<Material> baseMaterialList;
941+
private java.util.List<Material> searchMaterialList;
942+
943+
public java.util.List<Material> getBaseMaterialList() {
944+
return baseMaterialList;
945+
}
946+
947+
public void setBaseMaterialList(java.util.List<Material> baseMaterialList) {
948+
this.baseMaterialList = baseMaterialList;
949+
}
950+
951+
public java.util.List<Material> getSearchMaterialList() {
952+
return searchMaterialList;
953+
}
954+
955+
public void setSearchMaterialList(java.util.List<Material> searchMaterialList) {
956+
this.searchMaterialList = searchMaterialList;
957+
}
958+
}
959+
960+
/**
961+
* Tool class that uses a nested object parameter, reproducing the scenario in Issue #893.
962+
*/
963+
public static class NestedObjectTool {
964+
965+
@Tool(name = "createTheme", description = "Create a theme with material lists")
966+
public String create(
967+
@ToolParam(name = "request", description = "The creation request")
968+
CreateRequest request) {
969+
return "Created with "
970+
+ (request.getSearchMaterialList() != null
971+
? request.getSearchMaterialList().size()
972+
: 0)
973+
+ " search materials";
974+
}
975+
}
976+
977+
@Test
978+
@DisplayName("Should handle nested object with $ref in schema (Issue #893)")
979+
void testNestedObjectWithRefIssue893() {
980+
// Register tool with nested POJO parameter
981+
NestedObjectTool nestedTool = new NestedObjectTool();
982+
toolkit.registerTool(nestedTool);
983+
984+
// Verify the tool is registered
985+
AgentTool tool = toolkit.getTool("createTheme");
986+
assertNotNull(tool, "Tool should be registered");
987+
988+
// Print the generated schema for debugging
989+
Map<String, Object> parameters = tool.getParameters();
990+
String schemaJson = JsonUtils.getJsonCodec().toJson(parameters);
991+
992+
// Verify the schema contains $defs (victools should generate it for Material)
993+
assertTrue(
994+
schemaJson.contains("$defs") || schemaJson.contains("$ref"),
995+
"Schema should contain $defs/$ref for nested Material type");
996+
997+
// Simulate agent calling the tool with nested object input (matching Issue #893 scenario)
998+
Map<String, Object> input =
999+
Map.of(
1000+
"request",
1001+
Map.of(
1002+
"searchMaterialList",
1003+
List.of(
1004+
Map.of("key", "aaa", "value", "123"),
1005+
Map.of("key", "bbb", "value", "456"))));
1006+
1007+
ToolUseBlock toolCall =
1008+
ToolUseBlock.builder()
1009+
.id("toolu_test_893")
1010+
.name("createTheme")
1011+
.input(input)
1012+
.content(JsonUtils.getJsonCodec().toJson(input))
1013+
.build();
1014+
1015+
// This is the bug: callTool triggers ToolValidator.validateInput which fails
1016+
// with "Schema validation error: Reference /$defs/Material cannot be resolved"
1017+
ToolResultBlock result =
1018+
toolkit.callTool(ToolCallParam.builder().toolUseBlock(toolCall).build()).block();
1019+
1020+
assertNotNull(result, "Result should not be null");
1021+
assertFalse(
1022+
isErrorResult(result),
1023+
"Tool call with nested objects should succeed, but got error: "
1024+
+ getResultText(result));
1025+
1026+
String resultText = getResultText(result);
1027+
assertTrue(
1028+
resultText.contains("2 search materials"),
1029+
"Should have processed 2 search materials. Got: " + resultText);
1030+
}
1031+
9021032
// ==================== Converter Tests ====================
9031033

9041034
/**

0 commit comments

Comments
 (0)