You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "weiqingy (via GitHub)" <gi...@apache.org> on 2026/08/08 17:53:47 UTC

[PR] [integrations][java][python] Apply Ollama native structured output [flink-agents]

weiqingy opened a new pull request, #981:
URL: https://github.com/apache/flink-agents/pull/981

   Linked issue: #280
   
   ### Purpose of change
   
   Ollama has constrained decoding to a JSON schema since server v0.5.0, through the `format` request field. This connection refused it: Java inherited the base 4-arg `chat`, which throws `UnsupportedOperationException` for a non-null schema, and Python called `_reject_unsupported_output_schema`, which raises `NotImplementedError`. Callers were held on the prompt-engineering fallback, where the model is asked in prose for conforming JSON and the reply is scraped for code fences.
   
   This adds the native path in both languages, following the OpenAI, Azure and Anthropic connections.
   
   #### Runtime flow
   
   Java: `chat` delegates to `doChat`, which calls the new package-private `buildRequest` inside its existing try block and then issues the call. `buildRequest` assembles the request as before, and if the schema is a `Class` and the capability predicate agrees, derives a JSON schema with victools and sets it as the request's `format`. The seam is new, so it is extracted in a separate commit with no behavior change.
   
   Python: `chat` converts messages and tools, pops `model`, then consults the predicate and `_native_format`. A translated schema becomes a `format=` argument spliced into the SDK call. With none, no `format` key is passed.
   
   #### Key decisions
   
   The capability predicate reports capable for every model, in both languages. Ollama applies the grammar in the server's sampler rather than the model, and exposes no model-level signal to key on: `/api/show` reports a capability set carrying nothing schema-related, and `/api/version` reports only a version string. A server also runs arbitrary local models, so an allowlist of the kind the hosted providers use would be invented, and would report not-capable for models that work. Probing at `open()` was rejected because it detects only one of the three deployments that break the guarantee.
   
   Java derives the schema itself with victools, the first connection here not to hand a class to a provider SDK. `ObjectMapper.generateJsonSchema` needs no dependency and already serves the prompt fallback, but it is deprecated and draft-03 flavored, which would put the two languages on different dialects.
   
   victools moves from transitive runtime to direct compile scope. It already ships in the distribution through two provider SDKs, so no artifact is added and no notice changes. Its version comes from importing the victools BOM: a version on the module declaration would win nearest-wins mediation and become the global victools version while pinning only one of its three artifacts, letting the siblings drift on a later SDK upgrade with no build warning.
   
   ### Implementation Description
   
   #### Behavioral contracts
   
   1. A POJO `Class` in Java, or an `OutputSchema` wrapping a `BaseModel` in Python, is sent as the request's native `format`.
   2. Any other schema form, notably a `RowTypeInfo`, leaves `format` unset and keeps the prompt-engineering fallback.
   3. Without a schema the request is what it was before this change. `format` is omitted from the body rather than sent as null.
   4. The Java schema declares draft 2020-12, keeps properties in declaration order, gives a `Map` its value schema, excludes accessor-derived properties, and marks every field required except an `Optional` one.
   5. The Python payload is `model_json_schema()` verbatim, `$defs` included.
   6. The schema travels as a request field, never as a sampling option.
   7. Capability is reported for every model name, including an absent or empty one.
   8. A schema this connection cannot translate natively is answered rather than refused.
   
   #### Failure behavior
   
   A server below v0.5.0 rejects `format` with HTTP 400. Java rewraps it as `RuntimeException` through the existing catch, Python propagates the SDK error. This is the intended loud failure.
   
   Ollama Cloud accepts the request without enforcing the schema, and the MLX runner accepts the field and discards it. Both return an unconstrained response and no error, and neither is distinguishable from a model name, so neither is detected. The escape hatch for all three is `structured_output_strategy: PROMPT`, which is not yet dispatched.
   
   A recursive Java class generates a self reference the server rejects with HTTP 400. The victools option that rewrites it is deliberately unset: it makes the server drop the grammar silently for any class carrying a nested type twice, trading a loud failure on a rare shape for a silent one on a common shape.
   
   A `BaseModel` pydantic cannot render raises instead of falling back, matching the merged OpenAI and Azure connections.
   
   An explicit `NATIVE` strategy is not visible at this layer, so a caller requesting it for a schema form this branch skips receives an unconstrained response rather than an error. A `TODO(#912)` marks the site in both languages.
   
   ### Tests
   
   | Contract | Tests |
   |---|---|
   | 1 | `buildRequestSetsFormatForPojoSchema`, `test_native_applied_for_base_model` |
   | 2 | `buildRequestLeavesFormatUnsetForRowTypeInfo`, `test_native_not_applied_for_row_type_info` |
   | 3 | `buildRequestOmitsFormatWithoutSchema`, `test_format_absent_without_schema` |
   | 4 | `generatedSchemaShapeIsConstraining` |
   | 5 | `test_schema_is_model_json_schema` |
   | 6 | `test_schema_not_passed_as_sampling_option`. No Java test: `setFormat` is a typed request field, so there is no equivalent way to misroute it |
   | 7 | `supportsNativeStructuredOutputIsServerNotModelGated`, `test_supports_native_structured_output`, both parameterized over several names plus null and empty |
   | 8 | `test_schema_accepted_not_rejected`. Java is covered by contract 2's test, which returns a request rather than throwing |
   
   The Java tests are this connection's first, asserting the built request through `buildRequest` with no network and no mocking framework. The Python tests assert the SDK call kwargs against a mocked client. Both run in the unit arm. The Java `RowTypeInfo` test substitutes a non-`Class` object exercising the same gate, because Flink is a `provided` dependency of the api module and so is off this module's test classpath. The Python test uses a real `RowTypeInfo`.
   
   ### API
   
   `supportsNativeStructuredOutput` and the 4-arg `chat` are existing extension points, overridden here. `buildRequest` is package-private, and `toNativeFormat` and `_native_format` are private.
   
   For a caller that does nothing differently, nothing changes. No framework path passes a schema to a connection today, so the native branch is reachable only from a direct call, and a request without a schema is byte-identical to before. For a caller that does pass one, a previously raised error becomes a constrained response for a POJO or `BaseModel`, and a silent prompt fallback for any other form.
   
   victools changes scope from runtime to compile in dependent modules. Resolved versions are unchanged and no packaged bytes change.
   
   ### Documentation
   
   - [ ] `doc-needed`
   - [x] `doc-not-needed`
   - [ ] `doc-included`
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   - [x] Yes
   - [ ] No
   
   `Generated-by: Claude Code 2.1.226`
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [integrations][java][python] Apply Ollama native structured output [flink-agents]

Posted by "wenjin272 (via GitHub)" <gi...@apache.org>.
wenjin272 commented on code in PR #981:
URL: https://github.com/apache/flink-agents/pull/981#discussion_r3849405131


##########
integrations/chat-models/ollama/src/main/java/org/apache/flink/agents/integrations/chatmodels/ollama/OllamaChatModelConnection.java:
##########
@@ -245,6 +276,100 @@ public ChatMessage chat(
         }
     }
 
+    // Package-private so the request body (including the native format) can be asserted without
+    // issuing a live call through the Ollama endpoint caller.
+    OllamaChatRequest buildRequest(
+            List<ChatMessage> messages,
+            List<Tool> tools,
+            Map<String, Object> modelParams,
+            Object outputSchema) {
+        // convert think to think mode.
+        final Object think = modelParams.getOrDefault("think", true);
+        ThinkMode thinkMode = ThinkMode.ENABLED;
+        for (ThinkMode mode : ThinkMode.values()) {
+            if (mode.getValue().equals(think)) {
+                thinkMode = mode;
+                break;
+            }
+        }
+
+        final List<Tools.Tool> ollamaTools = this.convertToOllamaTools(tools);
+        final List<OllamaChatMessage> ollamaChatMessages =
+                messages.stream()
+                        .map(this::convertToOllamaChatMessages)
+                        .collect(Collectors.toList());
+
+        final String modelName = (String) modelParams.get("model");
+        final OllamaChatRequest chatRequest =
+                OllamaChatRequest.builder()
+                        .withMessages(ollamaChatMessages)
+                        .withModel(modelName)
+                        .withThinking(thinkMode)
+                        .withUseTools(false)
+                        .build();
+
+        chatRequest.setTools(ollamaTools);
+
+        // Native structured output applies only for a POJO Class schema; any other schema form,
+        // such as a RowTypeInfo wrapped in OutputSchema, keeps the prompt-engineering fallback.
+        // The schema is a request field of its own rather than a sampling option, so it is set as
+        // the request's format, which is left unset when no native translation applies and is then
+        // omitted from the serialized body rather than serialized as null.
+        //
+        // TODO(#912): the requested strategy is not visible here, so this re-check cannot tell an
+        // explicit NATIVE request apart from one that merely resolved to native. A caller asking
+        // for NATIVE on a schema form this branch skips therefore gets an unconstrained response
+        // instead of an error. Once strategy resolution is wired up, NATIVE must either bypass
+        // this capability re-check or fail explicitly.
+        if (outputSchema instanceof Class && supportsNativeStructuredOutput(modelName)) {
+            chatRequest.setFormat(toNativeFormat((Class<?>) outputSchema));
+        }
+
+        return chatRequest;
+    }
+
+    // Derives the JSON schema Ollama's format field expects from a POJO class. Every setting below
+    // addresses a concrete way the generated schema otherwise fails to constrain generation:
+    //
+    //   - DRAFT_2020_12 is the draft pydantic generates on the Python side, so a schema derived
+    //     from a Java class states the same contract in the same dialect.
+    //   - The PLAIN_JSON preset keeps generation to fields. Without a preset, getters surface as
+    //     properties of their own, named after the accessor call, e.g. "getSummary()".
+    //   - MAP_VALUES_AS_ADDITIONAL_PROPERTIES gives a Map its value schema. Without it the map
+    //     admits any value, and a model does emit values that the declared value type then fails
+    //     to deserialize.
+    //   - Sorting fields before methods and applying no further comparison leaves properties in
+    //     declaration order. Ollama's grammar fixes generation order to the order the schema
+    //     declares its properties, so the default alphabetical order would condition generation on
+    //     an order the class does not read in.
+    //   - The required check marks every field required except an Optional one. The default marks
+    //     nothing required, which lets a model omit fields at will, while marking everything
+    //     required would force the fields a caller declared omissible.
+    //
+    // Two settings are deliberately absent:
+    //
+    //   - FORBIDDEN_ADDITIONAL_PROPERTIES_BY_DEFAULT gains nothing: Ollama's grammar already
+    //     refuses a key the schema does not declare, even one a prompt explicitly asks for, and
+    //     only an explicit additionalProperties: true admits one.
+    //   - DEFINITION_FOR_MAIN_SCHEMA lets a recursive type generate a schema, but when the
+    //     document root is a $ref and one $defs entry references another, the server drops the
+    //     grammar and returns a free-form object. Any nested type used twice is extracted into
+    //     $defs, so enabling it would silently unconstrain a common shape to rescue a rare one. A
+    //     recursive type instead fails loudly, with HTTP 400 from the server.
+    private static ObjectNode toNativeFormat(Class<?> schemaClass) {
+        SchemaGeneratorConfigBuilder configBuilder =
+                new SchemaGeneratorConfigBuilder(
+                                SchemaVersion.DRAFT_2020_12, OptionPreset.PLAIN_JSON)

Review Comment:
   **[P1] Keep the generated schema aligned with Jackson deserialization**
   
   `OptionPreset.PLAIN_JSON` ignores Jackson's property model and annotations, so the generated schema can diverge from the contract used to deserialize the response. For example, `@JsonProperty("full_name") public String name` is emitted as `name`, while Jackson expects `full_name`; a response valid against the generated schema can therefore fail to deserialize. The same mismatch applies to ignored and visibility-controlled properties. Could we configure victools with its Jackson module (or otherwise use the same `ObjectMapper` semantics) and add a regression test covering renamed and ignored properties?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [integrations][java][python] Apply Ollama native structured output [flink-agents]

Posted by "weiqingy (via GitHub)" <gi...@apache.org>.
weiqingy commented on code in PR #981:
URL: https://github.com/apache/flink-agents/pull/981#discussion_r3888631101


##########
integrations/chat-models/ollama/src/main/java/org/apache/flink/agents/integrations/chatmodels/ollama/OllamaChatModelConnection.java:
##########
@@ -245,6 +276,100 @@ public ChatMessage chat(
         }
     }
 
+    // Package-private so the request body (including the native format) can be asserted without
+    // issuing a live call through the Ollama endpoint caller.
+    OllamaChatRequest buildRequest(
+            List<ChatMessage> messages,
+            List<Tool> tools,
+            Map<String, Object> modelParams,
+            Object outputSchema) {
+        // convert think to think mode.
+        final Object think = modelParams.getOrDefault("think", true);
+        ThinkMode thinkMode = ThinkMode.ENABLED;
+        for (ThinkMode mode : ThinkMode.values()) {
+            if (mode.getValue().equals(think)) {
+                thinkMode = mode;
+                break;
+            }
+        }
+
+        final List<Tools.Tool> ollamaTools = this.convertToOllamaTools(tools);
+        final List<OllamaChatMessage> ollamaChatMessages =
+                messages.stream()
+                        .map(this::convertToOllamaChatMessages)
+                        .collect(Collectors.toList());
+
+        final String modelName = (String) modelParams.get("model");
+        final OllamaChatRequest chatRequest =
+                OllamaChatRequest.builder()
+                        .withMessages(ollamaChatMessages)
+                        .withModel(modelName)
+                        .withThinking(thinkMode)
+                        .withUseTools(false)
+                        .build();
+
+        chatRequest.setTools(ollamaTools);
+
+        // Native structured output applies only for a POJO Class schema; any other schema form,
+        // such as a RowTypeInfo wrapped in OutputSchema, keeps the prompt-engineering fallback.
+        // The schema is a request field of its own rather than a sampling option, so it is set as
+        // the request's format, which is left unset when no native translation applies and is then
+        // omitted from the serialized body rather than serialized as null.
+        //
+        // TODO(#912): the requested strategy is not visible here, so this re-check cannot tell an
+        // explicit NATIVE request apart from one that merely resolved to native. A caller asking
+        // for NATIVE on a schema form this branch skips therefore gets an unconstrained response
+        // instead of an error. Once strategy resolution is wired up, NATIVE must either bypass
+        // this capability re-check or fail explicitly.
+        if (outputSchema instanceof Class && supportsNativeStructuredOutput(modelName)) {
+            chatRequest.setFormat(toNativeFormat((Class<?>) outputSchema));
+        }
+
+        return chatRequest;
+    }
+
+    // Derives the JSON schema Ollama's format field expects from a POJO class. Every setting below
+    // addresses a concrete way the generated schema otherwise fails to constrain generation:
+    //
+    //   - DRAFT_2020_12 is the draft pydantic generates on the Python side, so a schema derived
+    //     from a Java class states the same contract in the same dialect.
+    //   - The PLAIN_JSON preset keeps generation to fields. Without a preset, getters surface as
+    //     properties of their own, named after the accessor call, e.g. "getSummary()".
+    //   - MAP_VALUES_AS_ADDITIONAL_PROPERTIES gives a Map its value schema. Without it the map
+    //     admits any value, and a model does emit values that the declared value type then fails
+    //     to deserialize.
+    //   - Sorting fields before methods and applying no further comparison leaves properties in
+    //     declaration order. Ollama's grammar fixes generation order to the order the schema
+    //     declares its properties, so the default alphabetical order would condition generation on
+    //     an order the class does not read in.
+    //   - The required check marks every field required except an Optional one. The default marks
+    //     nothing required, which lets a model omit fields at will, while marking everything
+    //     required would force the fields a caller declared omissible.
+    //
+    // Two settings are deliberately absent:
+    //
+    //   - FORBIDDEN_ADDITIONAL_PROPERTIES_BY_DEFAULT gains nothing: Ollama's grammar already
+    //     refuses a key the schema does not declare, even one a prompt explicitly asks for, and
+    //     only an explicit additionalProperties: true admits one.
+    //   - DEFINITION_FOR_MAIN_SCHEMA lets a recursive type generate a schema, but when the
+    //     document root is a $ref and one $defs entry references another, the server drops the
+    //     grammar and returns a free-form object. Any nested type used twice is extracted into
+    //     $defs, so enabling it would silently unconstrain a common shape to rescue a rare one. A
+    //     recursive type instead fails loudly, with HTTP 400 from the server.
+    private static ObjectNode toNativeFormat(Class<?> schemaClass) {
+        SchemaGeneratorConfigBuilder configBuilder =
+                new SchemaGeneratorConfigBuilder(
+                                SchemaVersion.DRAFT_2020_12, OptionPreset.PLAIN_JSON)

Review Comment:
   Good catch. Fixed in `5230d2ce`.
   
   The schema now goes through victools' `JacksonModule`, so `@JsonProperty` renames and `@JsonIgnore` drops show up in it. Before, `@JsonProperty("full_name") String name` came out as `name`, and an ignored field came out required, so the model was being pushed to write exactly what the mapper would then reject.
   
   I passed no `JacksonOption`. The bare module only affects naming and visibility, which leaves the property order and the required check alone, and both are load-bearing here.
   
   Turns out the OpenAI and Anthropic SDKs already derive their schemas the same way, `PLAIN_JSON` plus `JacksonModule`, so Ollama was the odd one out. Does that match what you had in mind?
   
   The test is `generatedSchemaFollowsJacksonPropertyNames`, covering the renamed and the ignored property. Drop the module and it is the only test that fails.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [integrations][java][python] Apply Ollama native structured output [flink-agents]

Posted by "weiqingy (via GitHub)" <gi...@apache.org>.
weiqingy commented on PR #981:
URL: https://github.com/apache/flink-agents/pull/981#issuecomment-5467101438

   Both findings are fixed, thanks for catching them.
   
   **Jackson naming** (`5230d2ce`): answered inline.
   
   **Live coverage** (`6f10f490`): `test_ollama_chat_with_output_schema` sends a `BaseModel` schema down the native path and reads the reply back with `model_validate_json`, in the integration arm where CI already runs Ollama. It reuses the marker, model pull and skip check already in that file, so nothing new is gated.
   
   It checks the shape, not the values. The server enforces the grammar, so `{"name": "", "age": 0}` from a 1.7b model is a fair pass, and asserting a value would only make it flaky. To be sure it was not just passing on a model that emits JSON anyway, I reran the same call with the schema removed and got prose back, which fails the test.
   
   On POJO/Pydantic: only the Pydantic side is live. The Java connector's own tests do not run in a lane that has Ollama, so a Java version would have to sit in the e2e integration module.
   
   **`supportsNativeStructuredOutput()`**: agreed. I will put the user-facing documentation on #912, next to the strategy resolution and fallback behavior.
   
   On the guide: yes, let's start it. I will open a PR for it and ask for your review soon.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [integrations][java][python] Apply Ollama native structured output [flink-agents]

Posted by "weiqingy (via GitHub)" <gi...@apache.org>.
weiqingy commented on PR #981:
URL: https://github.com/apache/flink-agents/pull/981#issuecomment-5228734382

   Hi @wenjin272 , could you take a look at this PR when you get a chance? A fourth sample for the Implementation Description experiment on #894, native structured output for Ollama.
   
   Two things worth reporting from writing it, both about size.
   
   It came out around 7k characters against the 6k target, and #965 was 7.5k. Two in a row over, for the same reason: this one has eight behavioral contracts and six distinct failure paths, and the format asks for every one of them. I could only reach 6k by dropping a contract or a failure path, which seemed worse than being a screen longer. Is the target something that should flex with the size of the change, or does two consecutive overruns say the field list is too much for a provider PR?
   
   The description also caught something the review had missed: one contract has no Java test. The contracts-to-tests table made it visible because an empty cell sits next to seven filled ones. Worth noting #965's description surfaced nothing new about its code, so this is not consistent, and that difference might itself be the useful signal about when the format pays off.
   
   If it is proving useful on your side, would it make sense to start on the "Author-side Implementation Descriptions + two-stage review for AI-assisted PRs" guide? Happy to keep producing samples first if you would rather see more before committing to a format.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [integrations][java][python] Apply Ollama native structured output [flink-agents]

Posted by "wenjin272 (via GitHub)" <gi...@apache.org>.
wenjin272 merged PR #981:
URL: https://github.com/apache/flink-agents/pull/981


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org