fix(spring-ai): preserve enum and items in tool schema conversion by caps-xia · Pull Request #1617 · google/adk-java

caps-xia · 2026-10-06T03:04:45Z

Link to Issue or Description of Change

Closes: #1609

Problem:

ToolConverter#convertSchemaToSpringAi maps only type, properties, description and required when converting a genai Schema into the Spring AI JSON-schema map. enum and items are silently dropped:

enum parameters degrade to free-form text — the model receives no value constraint and invents values (verified identical behavior on 1.9.0 and 1.11.0);
array parameters lose their element schema, so nested enums/objects inside items are lost as well.

The drop is silent: the generated schema still looks complete (names/types/descriptions present), which reliably misleads debugging toward prompts or model capability instead of the conversion layer.

Solution:

Map enum and items alongside required, recursing into items so nested schemas survive conversion — the same treatment properties already gets. Minimal and additive:

schemas that don't use these keywords are unaffected (output unchanged);
empty enum lists are skipped, since "enum": [] is an unsatisfiable constraint under JSON Schema semantics;
no existing mapping (type/description/properties/required) is touched.

Testing Plan

Unit Tests:

Added testConvertSchemaToSpringAiPreservesEnumAndItems to ToolConverterTest: an object schema with an enum parameter and an array parameter whose items carry their own enum; asserts both keywords (and the nested item enum) survive conversion while type/required behavior stays unchanged.
Added testConvertSchemaToSpringAiSkipsEmptyEnum: an empty enum list is not forwarded ("enum": [] would make every call to the tool invalid).
Full module suite passes locally: mvn -pl contrib/spring-ai -am test → 217 tests, 0 failures, BUILD SUCCESS.

Manual End-to-End (E2E) Tests:

Reproduced with a real OpenAI-compatible gateway and a tool declaring enum/items:

Before: the model received a schema without enum/items and invented parameter values (behavior verified identical on 1.9.0 and 1.11.0 — probe output attached in the issue).
After: the original schema, including enum/items, reaches the model unchanged. The same pass-through path has been running in a production fork for months via a tool-callback restore workaround, so downstream consumption of these keywords is verified in production.

Checklist

I have read the CONTRIBUTING.md document.
My pull request contains a single commit.
I have performed a self-review of my own code.
I have commented my code, particularly in hard-to-understand areas.
I have added tests that prove my fix is effective or that my feature works.
New and existing unit tests pass locally with my changes.
I have manually tested my changes end-to-end.
Any dependent changes have been merged and published in downstream modules. — N/A, no dependent changes.

Additional context

This follows option 1 from my comment on the issue (small and reviewable). Serializing the original schema wholesale (option 2) can be a follow-up if maintainers prefer it.

wikaaaaa

Thanks for the fix! The change looks correct and safe.

I left a few small comments inline, mostly about tests and comment length. One more point that doesn't fit on a diff line:

Javadoc: convertSchemaToSpringAi is still described as "basic schema conversion". Could you list the keywords it maps (type, description, properties, required, enum, items)? Then it's clear that keywords like nullable, anyOf and format aren't carried over yet.

On option 2: agreed that this PR is the right first step, and a follow-up would be welcome. Note that serializing the genai Schema verbatim won't give valid JSON Schema by itself:

types come out uppercase ("STRING")
nullable needs translating to "type": [..., "null"]
anyOf needs to be converted recursively
Gemini-specific keys like propertyOrdering may be rejected by stricter providers

So it would be "serialize everything, then normalize".

wikaaaaa · 2026-10-08T10:23:37Z

+    // Preserve enum constraints and array element schemas: silently dropping them degrades
+    // enum parameters to free-form text (the model invents values) and loses the element
+    // schema of array parameters.
+    // An empty enum list would serialize to "enum": [] - an unsatisfiable constraint under
+    // JSON Schema semantics; skip it instead of passing through a value that makes every
+    // call to this tool invalid.


Could we keep this to a single line? The reasoning is great but fits better in the commit message / PR description. Something like:

Suggested change

// Preserve enum constraints and array element schemas: silently dropping them degrades

// enum parameters to free-form text (the model invents values) and loses the element

// schema of array parameters.

// An empty enum list would serialize to "enum": [] - an unsatisfiable constraint under

// JSON Schema semantics; skip it instead of passing through a value that makes every

// call to this tool invalid.

// Preserve enum constraints and array item schemas; skip empty enums (unsatisfiable).

Done — adopted your suggestion verbatim.

wikaaaaa · 2026-10-08T10:23:38Z

  }

+  @Test
+  void testConvertSchemaToSpringAiPreservesEnumAndItems() {


These tests exercise convertSchemaToSpringAi directly, but the bug as reported is in what ends up in convertToSpringAiTools(...).get(0).getToolDefinition().inputSchema(). Could you add one test that goes through convertToSpringAiTools and asserts the serialized inputSchema JSON contains enum and items (essentially the repro from the issue)? That guards the user-visible output, including the JSON serialization step.

Done — added testConvertToSpringAiToolsOutputSchemaContainsEnumAndItems: it converts a tool end-to-end and asserts the serialized inputSchema JSON contains enum and items (the repro from the issue, as a regression test).

wikaaaaa · 2026-10-08T10:23:38Z

+            .build();
+    Schema itemsSchema = Schema.builder().type("STRING").enum_("cardio", "dental").build();
+    Schema arrayParam =
+        Schema.builder().type("ARRAY").description("Included services").items(itemsSchema).build();


Nice that the nested item enum is covered. Could you also add a case where items is an OBJECT schema with its own properties (array of objects)? That proves the recursion also flows through properties inside items, which is the shape FunctionTool generates for List<>.

Done — added testConvertSchemaToSpringAiRecursesIntoObjectItemsProperties covering array-of-objects items with their own properties and required.

wikaaaaa · 2026-10-08T10:23:38Z

+
+    Map<String, Object> converted = toolConverter.convertSchemaToSpringAi(objectSchema);
+
+    Map<String, Object> properties = (Map<String, Object>) converted.get("properties");


These (Map<String, Object>) casts produce unchecked-cast warnings. Could you add @SuppressWarnings("unchecked") on the test method (or extract a small typed helper, e.g. private static Map<String, Object> asMap(Object o))?

Done — extracted a small asMap helper. Its cast is guarded by the instanceof assertion, with the suppression added per your second-round comment.

wikaaaaa · 2026-10-08T10:23:38Z

+  void testConvertSchemaToSpringAiSkipsEmptyEnum() {
+    // "enum": [] is unsatisfiable under JSON Schema semantics (no valid value exists);
+    // an empty enum must be skipped rather than forwarded.
+    Schema emptyEnumSchema = Schema.builder().type("STRING").enum_(new String[0]).build();


Nit: List.of() reads a bit more clearly than an empty varargs array.

Suggested change

Schema emptyEnumSchema = Schema.builder().type("STRING").enum_(new String[0]).build();

Schema emptyEnumSchema = Schema.builder().type("STRING").enum_(List.of()).build();

hirematha · 2026-10-08T13:06:33Z

Hi @caps-xia, Thank you for contributing and taking the time to open this PR.Our team is currently reviewing your changes and we will reach out if we need any further information.Thank you.

wikaaaaa · 2026-10-08T15:14:23Z

+          public io.reactivex.rxjava3.core.Single<Map<String, Object>> runAsync(
+              Map<String, Object> args, ToolContext toolContext) {
+            return io.reactivex.rxjava3.core.Single.just(Map.of());


Nit: Single is already imported at the top of the file, so the fully qualified name isn't needed.

Suggested change

public io.reactivex.rxjava3.core.Single<Map<String, Object>> runAsync(

Map<String, Object> args, ToolContext toolContext) {

return io.reactivex.rxjava3.core.Single.just(Map.of());

public Single<Map<String, Object>> runAsync(

Map<String, Object> args, ToolContext toolContext) {

return Single.just(Map.of());

Done — using the existing import.

wikaaaaa · 2026-10-08T15:14:23Z

+    Map<String, Object> schemaJson =
+        new com.fasterxml.jackson.databind.ObjectMapper().readValue(inputSchema, Map.class);


Nit: could you import ObjectMapper instead of using the fully qualified name? Using a TypeReference also avoids the raw Map.class unchecked conversion:

Suggested change

Map<String, Object> schemaJson =

new com.fasterxml.jackson.databind.ObjectMapper().readValue(inputSchema, Map.class);

Map<String, Object> schemaJson =

new ObjectMapper().readValue(inputSchema, new TypeReference<Map<String, Object>>() {});

(needs import com.fasterxml.jackson.core.type.TypeReference; and import com.fasterxml.jackson.databind.ObjectMapper;)

Done — imported ObjectMapper and switched to a TypeReference to avoid the raw Map.class conversion.

wikaaaaa · 2026-10-08T15:14:23Z

+    assertThat(converted).doesNotContainKey("enum");
+  }
+
+  private static Map<String, Object> asMap(Object value) {


Thanks for the helper! The cast inside still raises an unchecked warning; since it's guarded by the isInstanceOf assertion, it's safe to suppress here:

Suggested change

private static Map<String, Object> asMap(Object value) {

@SuppressWarnings("unchecked")

private static Map<String, Object> asMap(Object value) {

Done — suppressed; the cast is guarded by the isInstanceOf assertion.

convertSchemaToSpringAi mapped only type/properties/description/required, silently dropping enum constraints and array item schemas. Enum parameters degraded to free-form text (the model invents values) and array parameters lost their element schema. Map both, recursing into items so nested enums and object arrays survive conversion. Fixes google#1609

caps-xia · 2026-10-09T01:57:13Z

Apologies for the silence on the threads — the fixes went in with each push. Everything is addressed in 7b1a5b8 (single commit): round-one comments in b5eb358 (javadoc now lists the mapped keywords, condensed comment, end-to-end test through convertToSpringAiTools, array-of-objects items case, asMap helper, List.of()), round-two nits applied verbatim (imports + suppressed cast). Full module suite green locally (219 tests).

Good point on option 2 — happy to send the serialize-the-original-schema pass as a follow-up PR once this lands.

hirematha self-assigned this Oct 6, 2026

wikaaaaa requested changes Oct 8, 2026

View reviewed changes

caps-xia force-pushed the fix/1609-tool-converter-enum-items branch from a56645f to b5eb358 Compare October 8, 2026 10:50

hirematha added the needs review label Oct 8, 2026

wikaaaaa requested changes Oct 8, 2026

View reviewed changes

caps-xia force-pushed the fix/1609-tool-converter-enum-items branch from b5eb358 to 7b1a5b8 Compare October 9, 2026 01:17

caps-xia requested a review from wikaaaaa October 9, 2026 02:00


		Map<String, Object> converted = toolConverter.convertSchemaToSpringAi(objectSchema);

		Map<String, Object> properties = (Map<String, Object>) converted.get("properties");

	Schema emptyEnumSchema = Schema.builder().type("STRING").enum_(new String[0]).build();
	Schema emptyEnumSchema = Schema.builder().type("STRING").enum_(List.of()).build();

		Map<String, Object> schemaJson =
		new com.fasterxml.jackson.databind.ObjectMapper().readValue(inputSchema, Map.class);

	private static Map<String, Object> asMap(Object value) {
	@SuppressWarnings("unchecked")
	private static Map<String, Object> asMap(Object value) {

Conversation

caps-xia commented Oct 6, 2026

Link to Issue or Description of Change

Testing Plan

Checklist

Additional context

Uh oh!

wikaaaaa left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hirematha commented Oct 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

caps-xia commented Oct 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants