Conversation
Whether a native schema reaches a provider depends on more than the model. The schema form decides it, and so do conditions that differ per provider: an api version floor, a caller supplied output config, whether tools are bound. A caller outside the connection cannot see any of that, and the capability predicate answers a different and independent question. Add canApplyNativeStructuredOutput, which a connection answers from the same logic its own request builder uses, so the two cannot drift. The default is false, which suits a connection that translates no schema at all, and the contract records the hazard for a connection that has a native branch and leaves the query unoverridden. No connection overrides it yet and nothing calls it, so behavior is unchanged. Generated-by: Claude Code 2.1.272 (Claude Opus 5)
…anch Every connection that can apply a native schema repeated its branch conditions in two places once the query existed: the branch itself, and the answer a caller would get. Those two can drift, and the drift is invisible, because a caller sees only the answer. Each of the seven connections now implements the query, and its native branch calls that query rather than restating the conditions. The conditions themselves are unchanged: the schema form everywhere, an api version floor on Azure, a caller supplied output config on Anthropic, bound tools on Gemini. Capability stays a separate conjunct of each branch. The query is asked with the parameters the caller supplied, not a copy a request builder has already stripped, since two connections read keys that copy no longer holds. The tests capture what each branch actually does and compare it with the answer, so a branch that stops agreeing fails. The comments marking these re-checks are removed. Behavior is unchanged: nothing asks the query outside the connections themselves. Generated-by: Claude Code 2.1.272 (Claude Opus 5)
Mirror the Java query on the Python connection base. Whether a native schema reaches a provider depends on more than the model: the schema form decides it, and so do conditions that differ per provider, including ones fixed by a connection's own configuration rather than carried by the request. The default is False, which suits a connection that translates no schema at all, and the contract records what happens to a connection that has a native branch and leaves the query unoverridden. It also records that a False is not an error, the caller keeps the prompt fallback, and that a True is not a promise the call succeeds, since a connection may still raise once its branch has decided to apply the schema. No connection overrides it yet and nothing calls it, so behavior is unchanged. Generated-by: Claude Code 2.1.272 (Claude Opus 5)
…branch Each Python connection that can apply a native schema repeated its branch conditions in two places once the query existed: the branch itself, and the answer a caller would get. Those two can drift, and the drift is invisible, because a caller sees only the answer. The six connections now implement the query, and each native branch calls it rather than restating the conditions. The conditions are unchanged: the schema form everywhere, an api version floor on Azure, a caller supplied output config on Anthropic. Capability stays a separate conjunct of each branch. Three connections gain a form helper that does not render, so asking the question cannot raise where rendering would. The query is asked with the parameters the caller supplied, before a request path strips its own keys. Anthropic's JSON prefill keeps reading the live parameters instead, because it has to see the config the branch derives, which the caller never supplied. One narrow behavior change, on invalid input only. A branch consults the query before the capability predicate, so a request the query reports infeasible no longer evaluates that predicate. Where the model is not a string, four connections raised there and now send the request without a schema: OpenAI, Anthropic, Tongyi and Azure. Anthropic reaches that path with a translatable schema when the caller supplied an output config, and Azure when the api version is below its floor, since both conditions moved into the query. Ollama and watsonx are unaffected, their predicates answering unconditionally. Every request carrying a valid model behaves exactly as before. The comments marking these re-checks are removed, in both languages. Generated-by: Claude Code 2.1.272 (Claude Opus 5)
0c5adee to
0430219
Compare
wenjin272
left a comment
There was a problem hiding this comment.
Thanks for the thorough Java/Python implementation and tests. I left two API-design comments: one on consolidating the connection-level predicates, and one non-blocking follow-up for flat RowTypeInfo native support.
| * @return true if these inputs satisfy every condition the native branch imposes apart from the | ||
| * effective model's capability | ||
| */ | ||
| protected boolean canApplyNativeStructuredOutput( |
There was a problem hiding this comment.
Do we need two connection-level predicates here? Although model capability and request/schema feasibility are independent dimensions internally, the framework ultimately needs one answer: whether native structured output can be applied to this request. Could canApplyNativeStructuredOutput resolve the effective model internally and incorporate capability, replacing supportsNativeStructuredOutput as a connection-level hook? Providers could still keep model-specific allowlists as private helpers. I do not see a production caller that needs to consume the two dimensions independently, so exposing both seems to add API and composition complexity without a caller requirement.
There was a problem hiding this comment.
You're right that nothing consumes the two independently today, and the combining happens inside each connection rather than in the framework: canApplyNativeStructuredOutput(...) && supportsNativeStructuredOutput(...) at OpenAICompletionsConnection.java:303-304, and the same pair in the Anthropic, Bedrock, Gemini, Ollama, Azure and watsonx connections.
The caller that would read capability on its own is StructuredOutputStrategy.resolvesToNative (StructuredOutputStrategy.java:65-75), and it is not wired to anything yet. The setup parses and stores the strategy without reading it (BaseChatModelSetup.java:71-74), and it still calls the three-argument chat (:200), so no schema reaches a connection through the framework at all on this branch. The independence is a designed contract here rather than observed behavior.
I would still keep them separate, and the reason that already shows up in code is provider inheritance. VLLMChatModelConnection extends OpenAICompletionsConnection and overrides capability only (VLLMChatModelConnection.java:80-83), because the inherited allowlist rejects served models such as Qwen/Qwen2.5-7B-Instruct, while it inherits feasibility unchanged. Merge the two and super stops being usable, since the parent's answer has already applied the allowlist the subclass exists to escape. vLLM would have to restate the parent's feasibility rule locally: one line in Java, and in Python also a reach into a module-private helper in the parent module (vllm_chat_model.py:77-85).
Your point holds for the other providers though. Ollama and watsonx answer capability with an unconditional true (OllamaChatModelConnection.java:203, WatsonxChatModelConnection.java:265), so under a merged predicate their override collapses into the feasibility test and disappears.
The contract does state that the two are independent (BaseChatModelConnection.java:106-112), but not why they are separate hooks rather than one, which is a fair gap. I will state the asymmetry at the declaration: capability is advisory, so a policy is allowed to override it, while feasibility bounds what the connection can encode at all.
If you would rather have one hook, the shape that keeps both behaviors is a three-valued answer rather than a boolean, so that "cannot encode this" and "this model probably will not honor it" stay distinguishable at a single call site. That is more API surface than the split rather than less, which is why I have not taken it, but it is a real option if the single call site is worth it.
There was a problem hiding this comment.
Pushed as 7f6417d7. Both declarations now state the asymmetry in each language: capability is advisory and a configured policy may overrule it in either direction, while feasibility is binding, since a request whose schema the connection cannot encode has no native form to send.
Writing it turned up a related error in four places, all claiming that policy and capability are combined at request-build time. That is wrong on both halves: policy is parsed and stored but combined nowhere yet, and what a connection combines while building a request is capability with feasibility. Each site now names resolvesToNative as where policy meets capability, which is a declared method rather than a call path, so it stays accurate while nothing calls it.
One of the four is a pydantic field description rather than a comment, so that one change is a runtime string. Nothing reads it today.
There was a problem hiding this comment.
Following up on my earlier concern: I agree that effective-model resolution, model capability, and request/schema feasibility are all necessary concepts. My concern is not about removing those checks, but about exposing all three as independent hooks on BaseChatModelConnection. That makes the internal steps of one structured-output decision part of every connection’s extension contract.
Comparing this with LangChain and AgentScope Java suggests a simpler boundary. LangChain uses a request-shaped capability check only for AutoStrategy; an explicit ProviderStrategy bypasses auto-detection and lets native schema binding or request construction fail when it cannot be applied. AgentScope keeps the check coarse at the model level (supportsNativeStructuredOutput and its WithTools variant), with runtime fallback when native execution fails.
Could Flink expose only one request-shaped method on the base connection?
AUTOcallssupportsNativeStructuredOutput(schema, tools, modelParams)to choose native or prompt.NATIVEdirectly enters the native request path.PROMPTuses the existing prompt path.
The native request builder can still fail fast on deterministic incompatibilities—such as an unencodable schema, an unsupported API version, or conflicting request configuration—before sending anything to the provider. Effective-model resolution and the separate capability/feasibility checks could then remain provider-internal helpers, rather than three hooks on the common base class.
| * #supportsNativeStructuredOutput(String)} answers. Neither answer bounds the other, in either | ||
| * direction. A POJO on a model the connection does not classify as capable is feasible here and | ||
| * not capable there; a {@code RowTypeInfo} on a connection whose capability predicate is | ||
| * unconditionally true is capable there and not feasible here. |
There was a problem hiding this comment.
Non-blocking follow-up: RowTypeInfo does not necessarily have to remain on the prompt fallback permanently. Since OutputSchema already restricts it to basic field types, we could render a flat RowTypeInfo directly as JSON Schema in Java and Python, while leaving nested rows unsupported initially. This would allow it to use provider-native structured output without generating runtime POJO/Pydantic classes, and the result could still be parsed into a Flink Row.
There was a problem hiding this comment.
Agreed that a flat RowTypeInfo is renderable without a runtime POJO or Pydantic class, since OutputSchema already restricts the fields to basic types, and the response still parses back into a Row. Nested rows are the part that would need a decision, so leaving those unsupported initially sounds right.
Worth doing as its own change rather than widening this one.
…ooks The two predicates a connection answers are documented as independent, but not as different in kind, which leaves unanswered why they are separate hooks rather than one. Capability is advisory: it states something about the model that a configured policy is permitted to overrule, in either direction. Feasibility is binding: a request whose schema the connection cannot encode has no native form to send, so no policy can overrule it. Say so on both declarations, in both languages. Correct four claims that policy and capability are combined at request-build time. Policy is parsed and stored but combined nowhere yet, and what a connection combines while building a request is capability with feasibility. Each site now names the resolver as where policy meets capability, which is a declared method rather than a call path, so it stays true while nothing calls it. One of the four is a field description rather than a comment, so this changes one runtime string. Nothing reads it. Generated-by: Claude Code 2.1.272 (Claude Opus 5)
purushah
left a comment
There was a problem hiding this comment.
Thanks for the careful work here. I read both sides, ran the suites, and checked each override against its branch. It all lines up, and I couldn't find a valid request that behaves differently than before. I left a few comments on the code. None of them block this one.
What I verified
- Java on JDK 17: api 428, openai 131, anthropic 103, bedrock 90, gemini 71, ollama 18, watsonx 51. All green, matching the description.
- Python: 322 passed, 1 skipped across the seven changed test files. Pinned ruff is clean.
- Every override reads without consuming, is null-safe, and matches its branch's conditions. The only load-bearing raw-parameters pass is Java Anthropic, and it is wired correctly.
On the open thread
On the two-predicates question: I think the asymmetry argument is right, and vLLM only overriding capability is a real reason not to merge them. If a combining method comes later it has to take the strategy into account too, something like feasible && strategy.resolvesToNative(capable), otherwise NATIVE still can't override capability. That keeps the distinction you two already worked out in the thread. Happy either way, that's for @wenjin272 to decide.
Looks good to me overall. Everything below is non-blocking for this extraction.
| * {@link BaseChatModelConnection#supportsNativeStructuredOutput(String)}. Policy and capability are | ||
| * combined at request-build time. | ||
| * {@link BaseChatModelConnection#supportsNativeStructuredOutput(String)}. {@link | ||
| * #resolvesToNative(boolean)} combines the two. |
There was a problem hiding this comment.
Could we retain one TODO for #912 here noting that strategy resolution is not wired into production yet? When it is wired, the native branches need to honor the resolved policy without independently vetoing NATIVE through another capability check. Non-blocking for this extraction, it's unfinished #912 integration rather than something this PR changed.
| * #resolvesToNative(boolean)} combines the two. | |
| * #resolvesToNative(boolean)} combines the two. | |
| * | |
| * TODO(#912): strategy resolution is not wired into production yet. Once it is, the | |
| * native branches must honor the resolved policy rather than vetoing NATIVE through | |
| * their own capability check. |
|
|
||
| @Test | ||
| @DisplayName("The feasibility query answers exactly what the native branch decides") | ||
| void feasibilityQueryAgreesWithTheNativeBranch() { |
There was a problem hiding this comment.
Ollama's capability check is always true, so this test can't tell if someone folds capability into the override later, both sides move together and it still passes. The Python test gets around that with an _IncapableConnection subclass that returns false for capability and checks the hook still says yes for a Pydantic model. Worth doing the same here with a POJO.
|
|
||
| @Test | ||
| @DisplayName("The feasibility query answers exactly what the native branch decides") | ||
| void feasibilityQueryAgreesWithTheNativeBranch() { |
There was a problem hiding this comment.
Same as Ollama, capability is always true here so there's no way to test that it isn't folded into the hook. These two are the ones the "dedicated test per connection" line in the description doesn't cover yet.
| # translation is reported infeasible there, so it never reaches the conflict | ||
| # test below and cannot raise over a response_format this branch was never | ||
| # going to write. | ||
| if self.can_apply_native_structured_output( |
There was a problem hiding this comment.
Small thing on the description. Tongyi and Azure use plain set membership for capability, so a non-string model like 123 was already returning False quietly before this change, only unhashable values raised. OpenAI did raise, at its containment check, and Anthropic at .startswith. Might be worth softening "four connections raised" and adding one test for the new fallback so it's pinned. Not a real risk in practice since the setup declares model: str.
There was a problem hiding this comment.
Thanks for the review, and for running both suites. All six are addressed in dc8bfb03, along with a description fix.
One correction worth surfacing: on the non-string model it's three connections, not four. vLLM raises too. It inherits the query from OpenAI, so it didn't show up in the first sweep.
The one I left alone is the raw_kwargs copy. It keeps all six connections reading the same way, though it's a no-op on that path today. Does that trade seem worth it to you?
| ) | ||
|
|
||
|
|
||
| def _native_output_model(output_schema: Any) -> type[BaseModel] | None: |
There was a problem hiding this comment.
This is now the same function in anthropic, ollama and openai, and azure, tongyi and watsonx have their own versions too. If it lived next to render_output_schema in types.py, the schema-form check would have one home when the flat RowTypeInfo follow-up comes. Optional, fine as a follow-up.
| # asked with the parameters as they arrived. This path strips nothing today, | ||
| # and the snapshot is what keeps the query's view of them accurate if it ever | ||
| # does, rather than leaving that to whoever adds the first pop. | ||
| raw_kwargs = dict(kwargs) |
There was a problem hiding this comment.
Nothing gets popped before the hook on this path, so this copy is the same map as kwargs. The other connections do strip keys before an overridable hook, so their snapshots make sense as a contract for subclasses. Here it could just pass kwargs. Tiny, non-blocking.
…nto the feasibility query Ollama and watsonx report every model capable, so no model name can tell their two answers apart, and the existing agreement test moves both sides together and cannot see a fold. A subclass reporting nothing capable separates them: the query must still answer true, which fails the moment a capability conjunct is folded into the override. Pin the non-string model case as well. Three of the seven Python connections raise on one and the others do not, so tolerating it belongs to the connection rather than to the contract, and the test now says so instead of claiming a general property. Record in both languages that strategy resolution is not wired into production yet, since the native branches would still veto a forced NATIVE through their own capability check. Generated-by: Claude Code 2.1.272 (Claude Opus 5)
Linked issue: #912
Builds on #1128, now merged, which added the effective-model hook this uses. Rebased onto it, so the diff here is this change alone.
Purpose of change
Nothing changes for a caller yet: no production code asks what this adds. It is the second step of wiring an agent
output_schemathrough to the providers that can enforce it, and it removes a drift hazard the first step exposed.Whether a native schema reaches a provider depends on more than the model. The schema form decides it, and so do conditions that differ per provider: an api version floor on Azure, a caller supplied output config on Anthropic, bound tools on Gemini. Each connection already made that decision inside its own request path, where a caller cannot see it. Asking a connection to answer the same question separately would mean writing those conditions twice, and two copies drift silently, because a caller only ever sees the answer.
Runtime flow
Both connection bases gain a feasibility query: can this connection apply this schema, to a request built from these tools and parameters, with the effective model's capability set aside. The default is
false, which suits a connection that translates no schema at all.Every connection with a native branch implements it, and each branch now calls the query instead of restating its conditions, so the branch and the answer cannot disagree. Capability stays a separate conjunct of each branch. The query is asked with the parameters the caller supplied, before a request path strips its own keys, because two connections read keys that a stripped copy no longer holds.
Key decisions
Feasibility is asked separately from capability, and neither bounds the other. A schema form a connection cannot translate is infeasible on a model it calls capable; a translatable form is feasible on a model it calls incapable. Merging them would have hidden one behind the other.
Each override answers from its own request path rather than from a copy of its conditions. That is what the tests pin, and it is why the query is asked with the unstripped parameters.
Anthropic's JSON prefill keeps reading the live parameters rather than sharing the query's snapshot, because prefill has to see the output config the branch derives, which the caller never supplied.
Behavioral Semantics
Interaction decisions
Behavioral contracts
falseanswer is not an error: the caller keeps the prompt fallback.trueanswer is not a promise the call succeeds. A connection may still raise once its branch has decided to apply, where the caller supplied a conflicting format, or where the schema cannot be rendered.Failure behavior
No new failure path, and one narrow change on invalid input. A branch consults the query before the capability predicate, so a request the query reports infeasible no longer evaluates that predicate. Where the model is not a string, three Python connections raised there and now send the request without a schema: OpenAI at its modality check, Anthropic at its alias scan, and vLLM, which inherits the query and classifies by trimming the name. Tongyi and Azure already answered
Falsequietly, since both match by set membership. Anthropic reaches that path with a translatable schema when the caller supplied an output config, and Azure when the api version is below its floor, since both conditions moved into the query. Ollama and watsonx are unaffected. Measured by driving every connection across 1032 request shapes on both trees: 70 cells differ, none of them carrying a valid or absent model, and all 696 that do are byte-identical.Tests
Coverage by risk. The failure that matters is an answer that stops matching the branch, which no assertion against a literal can catch. The binding tests capture the model or the request the branch produced and compare it with the query, so a branch that stops agreeing fails. The exclusion tests exist because a binding test is structurally blind to capability being folded into an override: it moves both sides together, which was measured on both languages.
Suite results: Java api 428, chat-models common 6, openai 131, anthropic 103, bedrock 90, gemini 71, ollama 18, watsonx 51. Python 797 passed, 12 skipped.
Not verified. No live provider was called.
API
New method on each chat-model connection base,
protectedin Java and public in Python, plus overrides. No user-facing API changes, and no behavior change for an existing caller beyond the invalid-model case above.Documentation
doc-neededdoc-not-neededdoc-includedWas this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code 2.1.272 (Claude Opus 5)