-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix(gemini): preserve nullable JSON Schema tool declarations #3051
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
0f77913
8312022
a37e8e5
847f3a0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,162 @@ | ||
| /* | ||
| * Copyright 2024-2026 the original author or authors. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package io.agentscope.extensions.model.gemini; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.assertNotNull; | ||
| import static org.junit.jupiter.api.Assertions.assertNull; | ||
| import static org.junit.jupiter.api.Assertions.assertTrue; | ||
| import static org.junit.jupiter.api.Assumptions.assumeTrue; | ||
|
|
||
| import io.agentscope.core.agent.test.TestUtils; | ||
| import io.agentscope.core.e2e.E2ETestCondition; | ||
| import io.agentscope.core.message.Msg; | ||
| import io.agentscope.core.message.ToolUseBlock; | ||
| import io.agentscope.core.model.ChatResponse; | ||
| import io.agentscope.core.model.GenerateOptions; | ||
| import io.agentscope.core.model.ToolChoice; | ||
| import io.agentscope.core.model.ToolSchema; | ||
| import io.agentscope.extensions.model.gemini.formatter.GeminiChatFormatter; | ||
| import java.time.Duration; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import org.junit.jupiter.api.DisplayName; | ||
| import org.junit.jupiter.api.Tag; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.junit.jupiter.api.extension.ExtendWith; | ||
|
|
||
| /** Provider-specific acceptance test for nullable Gemini tool schemas. */ | ||
| @Tag("e2e") | ||
| @Tag("tools") | ||
| @ExtendWith(E2ETestCondition.class) | ||
| class GeminiNullableToolSchemaE2ETest { | ||
|
|
||
| private static final String GOOGLE_API_KEY = "GOOGLE_API_KEY"; | ||
| private static final String GOOGLE_GEMINI_MODEL = "GOOGLE_GEMINI_MODEL"; | ||
| private static final String DEFAULT_MODEL = "gemini-3.6-flash"; | ||
| private static final String TOOL_NAME = "nullable_echo"; | ||
| private static final Duration TEST_TIMEOUT = Duration.ofSeconds(90); | ||
|
|
||
| @Test | ||
| @DisplayName("Gemini accepts a nullable JSON Schema tool parameter") | ||
| void acceptsNullableToolSchema() { | ||
| String apiKey = System.getenv(GOOGLE_API_KEY); | ||
| assumeTrue(apiKey != null && !apiKey.isBlank(), "GOOGLE_API_KEY is required"); | ||
| String modelName = System.getenv(GOOGLE_GEMINI_MODEL); | ||
| if (modelName == null || modelName.isBlank()) { | ||
| modelName = DEFAULT_MODEL; | ||
| } | ||
|
|
||
| GeminiChatModel model = | ||
| GeminiChatModel.builder() | ||
| .apiKey(apiKey) | ||
| .modelName(modelName) | ||
| .formatter(new GeminiChatFormatter()) | ||
| .build(); | ||
|
|
||
| Map<String, Object> parameters = | ||
| Map.of( | ||
| "type", | ||
| "object", | ||
| "properties", | ||
| Map.of("value", Map.of("type", List.of("string", "null")))); | ||
| ToolSchema toolSchema = | ||
| ToolSchema.builder() | ||
| .name(TOOL_NAME) | ||
| .description("Echo an optional value") | ||
| .parameters(parameters) | ||
| .build(); | ||
|
|
||
| Msg request = TestUtils.createUserMessage("User", "Call nullable_echo with value hello."); | ||
| GenerateOptions options = | ||
| GenerateOptions.builder().toolChoice(new ToolChoice.Specific(TOOL_NAME)).build(); | ||
|
|
||
| List<ChatResponse> responses = | ||
| model.stream(List.of(request), List.of(toolSchema), options) | ||
| .collectList() | ||
| .block(TEST_TIMEOUT); | ||
|
|
||
| assertNotNull(responses, "Gemini should return a response stream"); | ||
| assertTrue( | ||
| responses.stream() | ||
| .flatMap(response -> response.getContent().stream()) | ||
| .filter(ToolUseBlock.class::isInstance) | ||
| .map(ToolUseBlock.class::cast) | ||
| .anyMatch(toolUse -> TOOL_NAME.equals(toolUse.getName())), | ||
| "Gemini should return a tool-use block for the forced nullable tool"); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Gemini accepts a nullable anyOf JSON Schema tool parameter") | ||
| void acceptsNullableAnyOfToolSchema() { | ||
| String apiKey = System.getenv(GOOGLE_API_KEY); | ||
| assumeTrue(apiKey != null && !apiKey.isBlank(), "GOOGLE_API_KEY is required"); | ||
| String modelName = System.getenv(GOOGLE_GEMINI_MODEL); | ||
| if (modelName == null || modelName.isBlank()) { | ||
| modelName = DEFAULT_MODEL; | ||
| } | ||
|
|
||
| GeminiChatModel model = | ||
| GeminiChatModel.builder() | ||
| .apiKey(apiKey) | ||
| .modelName(modelName) | ||
| .formatter(new GeminiChatFormatter()) | ||
| .build(); | ||
|
|
||
| Map<String, Object> parameters = | ||
| Map.of( | ||
| "type", | ||
| "object", | ||
| "properties", | ||
| Map.of( | ||
| "value", | ||
| Map.of( | ||
| "anyOf", | ||
| List.of(Map.of("type", "string"), Map.of("type", "null")))), | ||
| "required", | ||
| List.of("value")); | ||
| ToolSchema toolSchema = | ||
| ToolSchema.builder() | ||
| .name(TOOL_NAME) | ||
| .description("Echo an optional value") | ||
| .parameters(parameters) | ||
| .build(); | ||
|
|
||
| Msg request = | ||
| TestUtils.createUserMessage( | ||
| "User", "Call nullable_echo with the value field set to JSON null."); | ||
| GenerateOptions options = | ||
| GenerateOptions.builder().toolChoice(new ToolChoice.Specific(TOOL_NAME)).build(); | ||
|
|
||
| List<ChatResponse> responses = | ||
| model.stream(List.of(request), List.of(toolSchema), options) | ||
| .collectList() | ||
| .block(TEST_TIMEOUT); | ||
|
|
||
| assertNotNull(responses, "Gemini should return a response stream"); | ||
| ToolUseBlock toolUse = | ||
| responses.stream() | ||
| .flatMap(response -> response.getContent().stream()) | ||
| .filter(ToolUseBlock.class::isInstance) | ||
| .map(ToolUseBlock.class::cast) | ||
| .filter(tool -> TOOL_NAME.equals(tool.getName())) | ||
| .findFirst() | ||
| .orElse(null); | ||
|
|
||
| assertNotNull(toolUse, "Gemini should return the nullable anyOf tool call"); | ||
| assertTrue(toolUse.getInput().containsKey("value")); | ||
| assertNull(toolUse.getInput().get("value")); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,6 +51,8 @@ | |
| public class GeminiToolsHelper { | ||
|
|
||
| private static final Logger log = LoggerFactory.getLogger(GeminiToolsHelper.class); | ||
| private static final String MULTI_TYPE_ANY_OF_ERROR = | ||
| "JSON Schema cannot combine a multi-type array with anyOf"; | ||
|
|
||
| /** | ||
| * Creates a new GeminiToolsHelper. | ||
|
|
@@ -116,17 +118,22 @@ public Tool convertToGeminiTool(List<ToolSchema> tools) { | |
| * @return Gemini Schema object | ||
| */ | ||
| protected Schema convertParametersToSchema(Map<String, Object> parameters) { | ||
| // Normalize nullable unions before converting nested schema branches. | ||
| parameters = normalizeNullableAnyOf(parameters); | ||
| Schema.Builder schemaBuilder = Schema.builder(); | ||
|
|
||
| // Set type (default to OBJECT) | ||
| boolean typeUsesAnyOf = false; | ||
| if (parameters.containsKey("type")) { | ||
| String typeStr = (String) parameters.get("type"); | ||
| Type type = convertJsonTypeToGeminiType(typeStr); | ||
| schemaBuilder.type(type); | ||
| } else { | ||
| typeUsesAnyOf = applyJsonType(schemaBuilder, parameters.get("type")); | ||
| } else if (!parameters.containsKey("anyOf")) { | ||
| schemaBuilder.type(new Type(Type.Known.OBJECT)); | ||
| } | ||
|
|
||
| if (Boolean.TRUE.equals(parameters.get("nullable"))) { | ||
| schemaBuilder.nullable(true); | ||
| } | ||
|
|
||
| // Set description | ||
| if (parameters.containsKey("description")) { | ||
| schemaBuilder.description((String) parameters.get("description")); | ||
|
|
@@ -160,6 +167,20 @@ protected Schema convertParametersToSchema(Map<String, Object> parameters) { | |
| schemaBuilder.items(convertParametersToSchema(itemsSchema)); | ||
| } | ||
|
|
||
| // Set anyOf schemas | ||
| if (parameters.containsKey("anyOf")) { | ||
| if (typeUsesAnyOf) { | ||
| throw new IllegalArgumentException(MULTI_TYPE_ANY_OF_ERROR); | ||
| } | ||
| List<Schema> anyOfSchemas = new ArrayList<>(); | ||
| for (Object anyOfSchema : (List<?>) parameters.get("anyOf")) { | ||
| @SuppressWarnings("unchecked") | ||
| Map<String, Object> anyOfSchemaMap = (Map<String, Object>) anyOfSchema; | ||
| anyOfSchemas.add(convertParametersToSchema(anyOfSchemaMap)); | ||
| } | ||
| schemaBuilder.anyOf(anyOfSchemas); | ||
| } | ||
|
|
||
| // Set enum values | ||
| if (parameters.containsKey("enum")) { | ||
| @SuppressWarnings("unchecked") | ||
|
|
@@ -170,6 +191,132 @@ protected Schema convertParametersToSchema(Map<String, Object> parameters) { | |
| return schemaBuilder.build(); | ||
| } | ||
|
|
||
| private Map<String, Object> normalizeNullableAnyOf(Map<String, Object> schema) { | ||
| Object rawAnyOf = schema.get("anyOf"); | ||
| if (!(rawAnyOf instanceof List<?> anyOfSchemas)) { | ||
| return schema; | ||
| } | ||
|
|
||
| if (hasMultipleNonNullTypes(schema.get("type"))) { | ||
| throw new IllegalArgumentException(MULTI_TYPE_ANY_OF_ERROR); | ||
| } | ||
|
|
||
| List<Map<String, Object>> nonNullSchemas = new ArrayList<>(); | ||
| for (Object anyOfSchema : anyOfSchemas) { | ||
| if (!(anyOfSchema instanceof Map<?, ?> schemaMap)) { | ||
| return schema; | ||
| } | ||
| if (!isNullSchema(schemaMap)) { | ||
| Map<String, Object> typedSchema = new HashMap<>(); | ||
| for (Map.Entry<?, ?> entry : schemaMap.entrySet()) { | ||
| if (entry.getKey() instanceof String key) { | ||
| typedSchema.put(key, entry.getValue()); | ||
| } | ||
| } | ||
| nonNullSchemas.add(typedSchema); | ||
| } | ||
| } | ||
|
|
||
| if (nonNullSchemas.size() == anyOfSchemas.size()) { | ||
| return schema; | ||
| } | ||
|
|
||
| Map<String, Object> normalized = new HashMap<>(schema); | ||
| if (nonNullSchemas.isEmpty()) { | ||
| normalized.remove("anyOf"); | ||
| } else if (nonNullSchemas.size() == 1) { | ||
| // Keep the non-null branch authoritative and fill in missing outer metadata. | ||
| Map<String, Object> merged = new HashMap<>(nonNullSchemas.get(0)); | ||
| for (Map.Entry<String, Object> entry : schema.entrySet()) { | ||
| if (!"anyOf".equals(entry.getKey())) { | ||
| merged.putIfAbsent(entry.getKey(), entry.getValue()); | ||
| } | ||
| } | ||
| normalized = merged; | ||
| } else { | ||
| normalized.put("anyOf", nonNullSchemas); | ||
| } | ||
| normalized.put("nullable", true); | ||
| return normalized; | ||
| } | ||
|
|
||
| private boolean hasMultipleNonNullTypes(Object jsonType) { | ||
| if (!(jsonType instanceof List<?> typeValues)) { | ||
| return false; | ||
| } | ||
|
|
||
| int nonNullTypeCount = 0; | ||
| for (Object typeValue : typeValues) { | ||
| if (typeValue instanceof String typeString && !"null".equals(typeString)) { | ||
| nonNullTypeCount++; | ||
| } | ||
| } | ||
| return nonNullTypeCount > 1; | ||
| } | ||
|
|
||
| private boolean isNullSchema(Map<?, ?> schema) { | ||
| Object jsonType = schema.get("type"); | ||
| if ("null".equals(jsonType)) { | ||
| return true; | ||
| } | ||
| return jsonType instanceof List<?> typeValues | ||
| && typeValues.size() == 1 | ||
| && "null".equals(typeValues.get(0)); | ||
| } | ||
|
|
||
| private boolean applyJsonType(Schema.Builder schemaBuilder, Object jsonType) { | ||
| if (jsonType instanceof String typeString) { | ||
| if ("null".equals(typeString)) { | ||
| // Gemini rejects standalone null function parameter types. | ||
| schemaBuilder.type(new Type(Type.Known.OBJECT)); | ||
| return false; | ||
| } | ||
| schemaBuilder.type(convertJsonTypeToGeminiType(typeString)); | ||
| return false; | ||
| } | ||
|
|
||
| if (!(jsonType instanceof List<?> typeValues)) { | ||
| throw new IllegalArgumentException("JSON Schema type must be a string or an array"); | ||
| } | ||
|
|
||
| List<String> nonNullTypes = new ArrayList<>(); | ||
| boolean nullable = false; | ||
| for (Object typeValue : typeValues) { | ||
| if (!(typeValue instanceof String typeString)) { | ||
| throw new IllegalArgumentException("JSON Schema type array must contain strings"); | ||
| } | ||
| if ("null".equalsIgnoreCase(typeString)) { | ||
| nullable = true; | ||
| } else { | ||
| nonNullTypes.add(typeString); | ||
| } | ||
| } | ||
|
|
||
| if (nonNullTypes.isEmpty()) { | ||
| // Preserve the compatibility fallback for a null-only type array. | ||
| schemaBuilder.type(new Type(Type.Known.OBJECT)); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for catching this edge case. You are right that the null-only Fixed in commit 847f3a0 by preserving nullable=true on that fallback, and GeminiToolsHelperTest (20 tests), the full Gemini module tests (148 tests), |
||
| if (nullable) { | ||
| schemaBuilder.nullable(true); | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| if (nonNullTypes.size() == 1) { | ||
| schemaBuilder.type(convertJsonTypeToGeminiType(nonNullTypes.get(0))); | ||
| } else { | ||
| List<Schema> schemas = new ArrayList<>(); | ||
| for (String type : nonNullTypes) { | ||
| schemas.add(Schema.builder().type(convertJsonTypeToGeminiType(type)).build()); | ||
| } | ||
| schemaBuilder.anyOf(schemas); | ||
| } | ||
|
|
||
| if (nullable) { | ||
| schemaBuilder.nullable(true); | ||
| } | ||
| return nonNullTypes.size() > 1; | ||
| } | ||
|
|
||
| /** | ||
| * Convert JSON Schema type string to Gemini Type. | ||
| * | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing the
nullbranch here changes the schema semantics without marking the resulting Gemini schema as nullable. For example,anyOf: [{type: "string"}, {type: "null"}]becomes a non-nullableSTRINGschema, so validnullarguments are no longer accepted. Please preserve this by settingnullable(true)whenever a null branch is removed, and add assertions for both the single- and multi-branch nullableanyOfcases.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the careful review and for pointing out this nullable semantics issue.
Fixed in commit 8312022.
The nullable schema normalization now preserves nullable=true whenever null
branches are removed from anyOf. Added regression assertions for single-branch,
multi-branch, and all-null nullable anyOf cases, plus a guard against
nullable=false overriding type-array nullability.
Validation:
single-non-null-branch anyOf schemas with a JSON null argument.