fix: Enforce strict JSON Schema compliance to prevent OpenAI 400 Bad Request errors

[svetanis]

Enforce strict JSON Schema compliance to prevent OpenAI 400 Bad Request errors

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

• Closes: Tool calls fail with 400 Bad Request due to strict JSON Schema violations (OpenAI) #1265 (Tool calls fail with 400 Bad Request due to strict JSON Schema violations (OpenAI) #1265)
• Related: Tool calls fail with 400 Bad Request due to strict JSON Schema violations (OpenAI) #1265

2. Or, if no issue exists, describe the change:

Problem:
When using strict OpenAI-compatible providers like Groq, empty function arguments or responses are serialized as null or omitted by ChatCompletionsRequest. Furthermore, zero-argument tools fail to declare a parameters schema object. This breaks strict JSON schema parsers, causing them to drop conversation history, which leads to 400 Bad Request exceptions due to models hallucinating raw XML <function> tags. JSON Schema enum types are also improperly serialized in uppercase.

Solution:

• Added a custom schemaNormalizerModule to the ObjectMapper in ChatCompletionsRequest.java to force Type enums to serialize in lowercase (e.g., "string").
• Added enforceJsonObject to ChatCompletionsCommon.java to guarantee that empty arguments and function response payloads fallback to "{}" instead of null or raw strings.
• Updated ChatCompletionsRequest.java to explicitly inject a {"type":"object", "properties":{}} parameters schema for zero-argument functions instead of omitting it.

Testing Plan

Unit Tests:

• I have added or updated unit tests for my change.
• All unit tests pass locally.

Passed: mvn test
Added testFromLlmRequest_withEmptyFunctionArguments to ChatCompletionsRequestTest.java to explicitly test "{}" serialization for zero-argument tools. Updated testFromLlmRequest_withFunctionResponse expectations.

Manual End-to-End (E2E) Tests:

Wired the Google native chat completion client into the external model-prism project (PR #1199). Ran the integration through all the demo tests covering all main ADK features. The applications successfully execute multi-turn multi-tool conversations using strict JSON schema endpoints without throwing 400 Bad Request due to context loss.

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.

Additional context

This change unlocks robust support for the rapidly growing ecosystem of OpenAI-compatible endpoints that enforce strict JSON Schema validation.

[hemasekhar-p]

Hi @svetanis, thank you for your contribution! We appreciate you taking the time to submit this pull request. I noticed that your changes are not covered by the current test cases and the test you included passes even without your changes. Could you please include the corresponding unit tests to verify your changes?

[svetanis]

Hello @hemasekhar-p ,

Thank you for the review and the feedback!

I've updated the PR and added three new dedicated unit tests in ChatCompletionsRequestTest that specifically target and verify the strict JSON Schema compliance fixes. The new tests ensure that:

1. Missing function arguments are correctly serialized as empty objects ({}).
2. Absent tool parameters are automatically populated with the required empty object schema.
3. Gemini's uppercase schema types (like OBJECT and STRING) are safely normalized to lowercase to satisfy strict OpenAI API validation.

I have also successfully rebased the branch on top of the latest main to ensure there are no conflicts with the recent thoughtSignature updates. Everything is folded neatly into a single commit and all tests are passing locally.

Additionally, I ran comprehensive end-to-end testing on the rebased branch, and all of the demos (including a new one validating the thoughtSignature functionality) passed successfully.

Please let me know if there is anything else you need!

[hemasekhar-p]

@svetanis, thank you for addressing the comments. Currently this PR is under review by our team, we will keep you posted if any additional information is required. thank you.

[hemasekhar-p]

@anFatum, Could you please review this PR.

[wikaaaaa]

Thank you for the contribution!

We're on board with the empty/null handling: treating empty arguments/content and zero-argument parameters as valid and serializing them correctly ("{}" / {"type":"object","properties":{}}).

We have two main points we'd like to align on before approving:

• enforceJsonObject function - please see the other comment

• Inbound parsing only covers the non-streaming path. The streaming finalizer (ChatCompletionsResponse.getFinalToolCallParts) parses tool-call args separately and isn't updated, so the same input can behave differently depending on stream=true/false. The PR should align both paths (ideally via one shared parser).

[wikaaaaa]

The raised issue and main problem if I understand correctly is that empty fields are valid (e.g. a zero-arg call legitimately has no args) but ADK serializes them incorrectly (null/omitted) which causes failures.

enforceJsonObject, though, does something broader than the empty case: its !json.trim().startsWith("{") branch silently rewrites non-empty, non-object argument payloads (e.g. malformed/garbage input) to {}. That's really invalid/malformed-input handling — a separate concern from this issue. Silently dropping that content probably isn't what we want, and how invalid args should be handled deserves its own look (and a consistent approach across ADK) rather than being settled implicitly here.

And that startsWith branch is effectively the only thing the method does — without it, enforceJsonObject is just json == null ? "{}" : json.trim(), which is a no-op given the call sites (input is never null, and writeValueAsString/readValue already handle whitespace). So I'd suggest dropping the whole enforceJsonObject helper.

[svetanis]

@wikaaaaa ,

Thank you for the feedback! Both points are addressed in the updated commit.

1. enforceJsonObject — Removed

Dropped entirely, as you suggested. You were right that its startsWith("{") branch was really invalid-input handling masquerading as empty-field handling, and those are separate concerns.

For legitimately absent arguments (null/empty), we now return Map.of() which Jackson serializes to "{}" naturally. For malformed LLM output (hallucinated garbage), we let the parser throw — no silent rewriting.

The one edge case worth noting: Jackson's ObjectMapper.readValue("null", ...) returns Java null rather than throwing. To keep the exception contract clean, we catch that and throw a JsonMappingException (a subclass of JsonProcessingException), so the method has a single, uniform contract — it either returns a valid Map<String, Object> or throws JsonProcessingException. Callers don't need to handle mixed exception types.

2. Unified Inbound Parser

Extracted a shared parseToolCallArguments(String, ObjectMapper) helper in ChatCompletionsCommon and wired it into both:

• Non-streaming: ChatCompletionsCommon.Function.parseArguments() delegates to it
• Streaming: ChatCompletionsResponse.ChatCompletionChunkCollection.buildFinalResponse() delegates to it

Both paths now behave identically for all inputs — stream=true and stream=false produce the same result for the same arguments.

3. Test Coverage

Added ChatCompletionsCommonTest with full branch coverage for the shared parser:

Input	Expected Result	Test
Valid JSON ({"key":"val"})	Parsed Map	withValidJson
null	Map.of()	withNullString
""	Map.of()	withEmptyString
" "	Map.of()	withWhitespaceString
Invalid JSON ("none", "{bad:")	JsonProcessingException	withInvalidJson
Literal "null"	JsonMappingException	withLiteralNullString

The existing ChatCompletionsResponseTest streaming tests (parsesValidJsonArgs, handlesEmptyArgs, throwsOnInvalidJsonArgs) exercise the streaming path through the same shared parser.

Rebased onto latest main, single clean commit, all tests passing.

[wikaaaaa]

Please change Map.of() here and in other places to ImmutableMap.of() from com.google.common.collect.ImmutableMap;

Otherwise our internal tests fail

[svetanis]

@wikaaaaa ,

Thank you for catching this! Completely agree — ImmutableMap is the right choice throughout.

Done. All Map.of() occurrences in the changed files have been replaced with ImmutableMap.of() from com.google.common.collect.ImmutableMap:

• EMPTY_PARAMETERS_SCHEMA — now ImmutableMap.of("type", "object", "properties", ImmutableMap.of())
• parseToolCallArguments — return type changed to ImmutableMap<String, Object>; the null/empty fast-path returns ImmutableMap.of(), and the Jackson-parsed result is wrapped with ImmutableMap.copyOf(result)

I also updated ChatCompletionsCommonTest to use ImmutableMap as the variable type in the valid-JSON test case and added an explicit assertThat(args).isInstanceOf(ImmutableMap.class) assertion to make the immutability contract visible in the test itself.

Rebased onto latest main, single clean commit, all tests passing.