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/09/16 23:21:08 UTC
[PR] [api][integrations][java][python] Ask each connection whether it could apply a schema natively [flink-agents]
weiqingy opened a new pull request, #1129:
URL: https://github.com/apache/flink-agents/pull/1129
Linked issue: #912
Stacked on #1128, which adds the effective-model hook this builds beside. Review that one first; the diff here shows its two commits until it merges.
### 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_schema` through 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
| Connection | Conditions the query answers | Kept in the branch |
|---|---|---|
| OpenAI, Ollama, Bedrock, watsonx, Tongyi | schema form | capability |
| Azure | schema form, api version floor | capability |
| Anthropic | schema form, no caller supplied output config | capability |
| Gemini | schema form, no bound tools | capability |
#### Behavioral contracts
1. The query answers whether the connection could apply that schema to such a request, with the effective model's capability excluded.
2. It accepts a missing schema, missing tools and missing parameters without raising, so a caller can ask speculatively, and it reads the parameters without consuming them.
3. A `false` answer is not an error: the caller keeps the prompt fallback.
4. A `true` answer 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.
5. An override answers from the same logic its own request path uses.
#### 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, four Python 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. 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
| Contract | Tests |
|---|---|
| 1, 2, 3 | the base-class tests in both languages: default, missing inputs accepted, parameters not consumed |
| 4 | each connection's contract states it; the conflicting-format raises are pinned by existing tests |
| 5 | a binding test per connection, capturing what the branch actually does and comparing it with the answer |
| the exclusion | a dedicated test per connection that capability is not folded into the answer |
| the unstripped parameters | a test per stripping connection that the query sees keys the request path later removes |
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, `protected` in 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-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.272 (Claude Opus 5)
--
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] [api][integrations][java][python] Ask each connection whether it could apply a schema natively [flink-agents]
Posted by "wenjin272 (via GitHub)" <gi...@apache.org>.
wenjin272 commented on code in PR #1129:
URL: https://github.com/apache/flink-agents/pull/1129#discussion_r4140394607
##########
api/src/main/java/org/apache/flink/agents/api/chat/model/BaseChatModelConnection.java:
##########
@@ -99,6 +99,48 @@ protected String effectiveModelFor(@Nullable Map<String, Object> modelParams) {
return modelParams == null ? null : (String) modelParams.get("model");
}
+ /**
+ * Whether this connection could apply {@code outputSchema} natively to a request built from
+ * these tools and parameters, leaving the effective model's capability out of the answer.
+ *
+ * <p>Feasibility, not capability: the answer covers everything this connection's native branch
+ * requires of a request apart from the effective model, and says nothing about whether the
+ * model the request names would honor a native schema, which is the separate question {@link
+ * #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.
+ *
+ * <p>An override must answer from the same logic its own request builder uses to decide the
+ * native branch, so that the answer cannot drift from what the request ends up carrying.
+ *
+ * <p>A {@code false} answer is not an error: it reports that the request would carry no native
+ * schema, so the caller keeps the prompt-engineering fallback rather than losing the schema.
+ *
+ * <p>The default {@code false} is safe only for a connection that translates no schema at all.
+ * A connection whose request builder has a native branch but which leaves this unoverridden
+ * reports every request infeasible: a caller that degrades to the prompt-engineering fallback
+ * then silently never reaches that branch, and one that refuses an unapplicable schema instead
+ * fails on a request the connection could in fact have applied.
+ *
+ * <p>Answers about the request rather than validating it. A null {@code outputSchema} is an
+ * unconstrained request, a null {@code tools} is a request binding no tools, and a null {@code
+ * modelParams} is accepted; none of the three may raise. The parameters must be read without
+ * being consumed, so that the same map still builds the request the answer was about.
+ *
+ * @param outputSchema the schema the request would carry, or null for an unconstrained request
+ * @param tools the tools the request would bind, may be null or empty for none
+ * @param modelParams the parameters the request would be built from, may be null
+ * @return true if these inputs satisfy every condition the native branch imposes apart from the
+ * effective model's capability
+ */
+ protected boolean canApplyNativeStructuredOutput(
Review Comment:
That makes sense. I missed the timing difference from LangChain: because Flink only sends the schema on the finalization call, deferring deterministic validation to the request builder would discover the failure after the tool rounds and would also send it through the generic retry path.
The three-state result sounds like a good compromise. It preserves early failure for `NATIVE + INFEASIBLE`, while still reducing the base-class surface to one request-level method and keeping effective-model resolution, capability, and feasibility as provider-internal details.
For naming, I would prefer to keep the unified base method as:
```java
supportsNativeStructuredOutput(schema, tools, modelParams)
```
and rename the internal model-capability helper to:
```java
modelSupportsNativeStructuredOutput(effectiveModel)
```
with `_model_supports_native_structured_output` in Python. Since none of this has shipped, we do not need to retain the old capability-helper name, and this also avoids the Python overloading conflict.
--
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] [api][integrations][java][python] Ask each connection whether it could apply a schema natively [flink-agents]
Posted by "weiqingy (via GitHub)" <gi...@apache.org>.
weiqingy commented on code in PR #1129:
URL: https://github.com/apache/flink-agents/pull/1129#discussion_r4136680371
##########
api/src/main/java/org/apache/flink/agents/api/chat/model/BaseChatModelConnection.java:
##########
@@ -99,6 +99,48 @@ protected String effectiveModelFor(@Nullable Map<String, Object> modelParams) {
return modelParams == null ? null : (String) modelParams.get("model");
}
+ /**
+ * Whether this connection could apply {@code outputSchema} natively to a request built from
+ * these tools and parameters, leaving the effective model's capability out of the answer.
+ *
+ * <p>Feasibility, not capability: the answer covers everything this connection's native branch
+ * requires of a request apart from the effective model, and says nothing about whether the
+ * model the request names would honor a native schema, which is the separate question {@link
+ * #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.
+ *
+ * <p>An override must answer from the same logic its own request builder uses to decide the
+ * native branch, so that the answer cannot drift from what the request ends up carrying.
+ *
+ * <p>A {@code false} answer is not an error: it reports that the request would carry no native
+ * schema, so the caller keeps the prompt-engineering fallback rather than losing the schema.
+ *
+ * <p>The default {@code false} is safe only for a connection that translates no schema at all.
+ * A connection whose request builder has a native branch but which leaves this unoverridden
+ * reports every request infeasible: a caller that degrades to the prompt-engineering fallback
+ * then silently never reaches that branch, and one that refuses an unapplicable schema instead
+ * fails on a request the connection could in fact have applied.
+ *
+ * <p>Answers about the request rather than validating it. A null {@code outputSchema} is an
+ * unconstrained request, a null {@code tools} is a request binding no tools, and a null {@code
+ * modelParams} is accepted; none of the three may raise. The parameters must be read without
+ * being consumed, so that the same map still builds the request the answer was about.
+ *
+ * @param outputSchema the schema the request would carry, or null for an unconstrained request
+ * @param tools the tools the request would bind, may be null or empty for none
+ * @param modelParams the parameters the request would be built from, may be null
+ * @return true if these inputs satisfy every condition the native branch imposes apart from the
+ * effective model's capability
+ */
+ protected boolean canApplyNativeStructuredOutput(
Review Comment:
Yes, one method on the base connection makes sense to me. `effectiveModelFor`, capability and feasibility can become provider-internal helpers. vLLM can still override capability alone through a protected helper on `OpenAICompletionsConnection`, so my inheritance concern goes away. None of this has shipped, so renaming is free.
One question on NATIVE though. In the follow-up wiring, the schema is only sent on a final call after the tool rounds. If NATIVE skips the check and lets the builder fail, the failure shows up after every tool has already run. It would also go through the generic retry catch (`ChatModelInvoker.java:211`), so a failure that can never succeed gets retried. LangChain doesn't hit this because it binds the schema on every call, so it fails on the first one.
What do you think about having the method return three answers instead of a boolean? Something like `nativeStructuredOutputSupport(schema, tools, modelParams)` returning `INFEASIBLE`, `FEASIBLE` or `NATIVE_RECOMMENDED`. AUTO goes native only on `NATIVE_RECOMMENDED`, NATIVE fails up front on `INFEASIBLE`, and PROMPT stays as is. It also means the builders keep skipping a schema they can't apply, instead of all 13 of them starting to throw. It needs a new name either way, since Python can't overload `supports_native_structured_output`.
If that shape works for you, I'll make the change on this PR in place of the two predicates.
--
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] [api][integrations][java][python] Ask each connection whether it could apply a schema natively [flink-agents]
Posted by "weiqingy (via GitHub)" <gi...@apache.org>.
weiqingy commented on code in PR #1129:
URL: https://github.com/apache/flink-agents/pull/1129#discussion_r4055469668
##########
python/flink_agents/integrations/chat_models/tongyi_chat_model.py:
##########
@@ -247,34 +296,29 @@ def chat(
# popped on the line above, so a kwargs lookup would yield None on every call
# and report every model incapable.
#
- # TODO(#912): the requested strategy is not visible here, so this check
- # cannot tell an explicit NATIVE request apart from one that merely
- # resolved to native. A caller asking for NATIVE on a model this predicate
- # rejects therefore gets an unconstrained response instead of an error.
- # Once strategy resolution is wired up, NATIVE must either bypass this
- # capability check or fail explicitly.
- if output_schema is not None and self.supports_native_structured_output(
- model_name
- ):
- # Resolved before the conflict test, so a payload with no native
- # translation does not raise over a response_format this branch was
- # never going to write. Tested before the schema is rendered, because a
- # caller who supplies both a schema and a response_format has a conflict
- # to resolve whatever the schema turns out to render to, and reporting a
- # render failure instead would describe the wrong problem. The name is
- # read off the model class, so this needs no rendered document.
+ # The feasibility half is asked rather than restated, so a caller asking the
+ # same question gets the answer this branch acts on. A payload with no native
+ # 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(
Review 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?
--
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] [api][integrations][java][python] Ask each connection whether it could apply a schema natively [flink-agents]
Posted by "wenjin272 (via GitHub)" <gi...@apache.org>.
wenjin272 commented on code in PR #1129:
URL: https://github.com/apache/flink-agents/pull/1129#discussion_r4056383274
##########
api/src/main/java/org/apache/flink/agents/api/chat/model/BaseChatModelConnection.java:
##########
@@ -99,6 +99,48 @@ protected String effectiveModelFor(@Nullable Map<String, Object> modelParams) {
return modelParams == null ? null : (String) modelParams.get("model");
}
+ /**
+ * Whether this connection could apply {@code outputSchema} natively to a request built from
+ * these tools and parameters, leaving the effective model's capability out of the answer.
+ *
+ * <p>Feasibility, not capability: the answer covers everything this connection's native branch
+ * requires of a request apart from the effective model, and says nothing about whether the
+ * model the request names would honor a native schema, which is the separate question {@link
+ * #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.
+ *
+ * <p>An override must answer from the same logic its own request builder uses to decide the
+ * native branch, so that the answer cannot drift from what the request ends up carrying.
+ *
+ * <p>A {@code false} answer is not an error: it reports that the request would carry no native
+ * schema, so the caller keeps the prompt-engineering fallback rather than losing the schema.
+ *
+ * <p>The default {@code false} is safe only for a connection that translates no schema at all.
+ * A connection whose request builder has a native branch but which leaves this unoverridden
+ * reports every request infeasible: a caller that degrades to the prompt-engineering fallback
+ * then silently never reaches that branch, and one that refuses an unapplicable schema instead
+ * fails on a request the connection could in fact have applied.
+ *
+ * <p>Answers about the request rather than validating it. A null {@code outputSchema} is an
+ * unconstrained request, a null {@code tools} is a request binding no tools, and a null {@code
+ * modelParams} is accepted; none of the three may raise. The parameters must be read without
+ * being consumed, so that the same map still builds the request the answer was about.
+ *
+ * @param outputSchema the schema the request would carry, or null for an unconstrained request
+ * @param tools the tools the request would bind, may be null or empty for none
+ * @param modelParams the parameters the request would be built from, may be null
+ * @return true if these inputs satisfy every condition the native branch imposes apart from the
+ * effective model's capability
+ */
+ protected boolean canApplyNativeStructuredOutput(
Review 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?
- `AUTO` calls `supportsNativeStructuredOutput(schema, tools, modelParams)` to choose native or prompt.
- `NATIVE` directly enters the native request path.
- `PROMPT` uses 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.
--
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] [api][integrations][java][python] Ask each connection whether it could apply a schema natively [flink-agents]
Posted by "weiqingy (via GitHub)" <gi...@apache.org>.
weiqingy commented on code in PR #1129:
URL: https://github.com/apache/flink-agents/pull/1129#discussion_r4043052645
##########
api/src/main/java/org/apache/flink/agents/api/chat/model/BaseChatModelConnection.java:
##########
@@ -99,6 +99,48 @@ protected String effectiveModelFor(@Nullable Map<String, Object> modelParams) {
return modelParams == null ? null : (String) modelParams.get("model");
}
+ /**
+ * Whether this connection could apply {@code outputSchema} natively to a request built from
+ * these tools and parameters, leaving the effective model's capability out of the answer.
+ *
+ * <p>Feasibility, not capability: the answer covers everything this connection's native branch
+ * requires of a request apart from the effective model, and says nothing about whether the
+ * model the request names would honor a native schema, which is the separate question {@link
+ * #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.
+ *
+ * <p>An override must answer from the same logic its own request builder uses to decide the
+ * native branch, so that the answer cannot drift from what the request ends up carrying.
+ *
+ * <p>A {@code false} answer is not an error: it reports that the request would carry no native
+ * schema, so the caller keeps the prompt-engineering fallback rather than losing the schema.
+ *
+ * <p>The default {@code false} is safe only for a connection that translates no schema at all.
+ * A connection whose request builder has a native branch but which leaves this unoverridden
+ * reports every request infeasible: a caller that degrades to the prompt-engineering fallback
+ * then silently never reaches that branch, and one that refuses an unapplicable schema instead
+ * fails on a request the connection could in fact have applied.
+ *
+ * <p>Answers about the request rather than validating it. A null {@code outputSchema} is an
+ * unconstrained request, a null {@code tools} is a request binding no tools, and a null {@code
+ * modelParams} is accepted; none of the three may raise. The parameters must be read without
+ * being consumed, so that the same map still builds the request the answer was about.
+ *
+ * @param outputSchema the schema the request would carry, or null for an unconstrained request
+ * @param tools the tools the request would bind, may be null or empty for none
+ * @param modelParams the parameters the request would be built from, may be null
+ * @return true if these inputs satisfy every condition the native branch imposes apart from the
+ * effective model's capability
+ */
+ protected boolean canApplyNativeStructuredOutput(
Review 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.
--
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] [draft][api][integrations][java][python] Ask each connection whether it could apply a schema natively [flink-agents]
Posted by "wenjin272 (via GitHub)" <gi...@apache.org>.
wenjin272 commented on code in PR #1129:
URL: https://github.com/apache/flink-agents/pull/1129#discussion_r4035303043
##########
api/src/main/java/org/apache/flink/agents/api/chat/model/BaseChatModelConnection.java:
##########
@@ -99,6 +99,48 @@ protected String effectiveModelFor(@Nullable Map<String, Object> modelParams) {
return modelParams == null ? null : (String) modelParams.get("model");
}
+ /**
+ * Whether this connection could apply {@code outputSchema} natively to a request built from
+ * these tools and parameters, leaving the effective model's capability out of the answer.
+ *
+ * <p>Feasibility, not capability: the answer covers everything this connection's native branch
+ * requires of a request apart from the effective model, and says nothing about whether the
+ * model the request names would honor a native schema, which is the separate question {@link
+ * #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.
+ *
+ * <p>An override must answer from the same logic its own request builder uses to decide the
+ * native branch, so that the answer cannot drift from what the request ends up carrying.
+ *
+ * <p>A {@code false} answer is not an error: it reports that the request would carry no native
+ * schema, so the caller keeps the prompt-engineering fallback rather than losing the schema.
+ *
+ * <p>The default {@code false} is safe only for a connection that translates no schema at all.
+ * A connection whose request builder has a native branch but which leaves this unoverridden
+ * reports every request infeasible: a caller that degrades to the prompt-engineering fallback
+ * then silently never reaches that branch, and one that refuses an unapplicable schema instead
+ * fails on a request the connection could in fact have applied.
+ *
+ * <p>Answers about the request rather than validating it. A null {@code outputSchema} is an
+ * unconstrained request, a null {@code tools} is a request binding no tools, and a null {@code
+ * modelParams} is accepted; none of the three may raise. The parameters must be read without
+ * being consumed, so that the same map still builds the request the answer was about.
+ *
+ * @param outputSchema the schema the request would carry, or null for an unconstrained request
+ * @param tools the tools the request would bind, may be null or empty for none
+ * @param modelParams the parameters the request would be built from, may be null
+ * @return true if these inputs satisfy every condition the native branch imposes apart from the
+ * effective model's capability
+ */
+ protected boolean canApplyNativeStructuredOutput(
Review 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.
##########
api/src/main/java/org/apache/flink/agents/api/chat/model/BaseChatModelConnection.java:
##########
@@ -99,6 +99,48 @@ protected String effectiveModelFor(@Nullable Map<String, Object> modelParams) {
return modelParams == null ? null : (String) modelParams.get("model");
}
+ /**
+ * Whether this connection could apply {@code outputSchema} natively to a request built from
+ * these tools and parameters, leaving the effective model's capability out of the answer.
+ *
+ * <p>Feasibility, not capability: the answer covers everything this connection's native branch
+ * requires of a request apart from the effective model, and says nothing about whether the
+ * model the request names would honor a native schema, which is the separate question {@link
+ * #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.
Review 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`.
--
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] [api][integrations][java][python] Ask each connection whether it could apply a schema natively [flink-agents]
Posted by "weiqingy (via GitHub)" <gi...@apache.org>.
weiqingy commented on code in PR #1129:
URL: https://github.com/apache/flink-agents/pull/1129#discussion_r4204090507
##########
api/src/main/java/org/apache/flink/agents/api/chat/model/BaseChatModelConnection.java:
##########
@@ -99,6 +99,48 @@ protected String effectiveModelFor(@Nullable Map<String, Object> modelParams) {
return modelParams == null ? null : (String) modelParams.get("model");
}
+ /**
+ * Whether this connection could apply {@code outputSchema} natively to a request built from
+ * these tools and parameters, leaving the effective model's capability out of the answer.
+ *
+ * <p>Feasibility, not capability: the answer covers everything this connection's native branch
+ * requires of a request apart from the effective model, and says nothing about whether the
+ * model the request names would honor a native schema, which is the separate question {@link
+ * #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.
+ *
+ * <p>An override must answer from the same logic its own request builder uses to decide the
+ * native branch, so that the answer cannot drift from what the request ends up carrying.
+ *
+ * <p>A {@code false} answer is not an error: it reports that the request would carry no native
+ * schema, so the caller keeps the prompt-engineering fallback rather than losing the schema.
+ *
+ * <p>The default {@code false} is safe only for a connection that translates no schema at all.
+ * A connection whose request builder has a native branch but which leaves this unoverridden
+ * reports every request infeasible: a caller that degrades to the prompt-engineering fallback
+ * then silently never reaches that branch, and one that refuses an unapplicable schema instead
+ * fails on a request the connection could in fact have applied.
+ *
+ * <p>Answers about the request rather than validating it. A null {@code outputSchema} is an
+ * unconstrained request, a null {@code tools} is a request binding no tools, and a null {@code
+ * modelParams} is accepted; none of the three may raise. The parameters must be read without
+ * being consumed, so that the same map still builds the request the answer was about.
+ *
+ * @param outputSchema the schema the request would carry, or null for an unconstrained request
+ * @param tools the tools the request would bind, may be null or empty for none
+ * @param modelParams the parameters the request would be built from, may be null
+ * @return true if these inputs satisfy every condition the native branch imposes apart from the
+ * effective model's capability
+ */
+ protected boolean canApplyNativeStructuredOutput(
Review Comment:
Done in `b860b4cf` (Java) and `6af3e898` (Python). The base connection now has only `supportsNativeStructuredOutput(schema, tools, modelParams)`, returning `NativeStructuredOutputSupport` (`INFEASIBLE` / `FEASIBLE` / `NATIVE_RECOMMENDED`). The capability helper uses your names, and effective-model resolution and feasibility are private to each connection. vLLM still overrides the capability helper alone. `resolvesToNative` takes the three-state answer and raises on `NATIVE` with `INFEASIBLE`. I updated the PR description to match.
--
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] [api][integrations][java][python] Ask each connection whether it could apply a schema natively [flink-agents]
Posted by "wenjin272 (via GitHub)" <gi...@apache.org>.
wenjin272 merged PR #1129:
URL: https://github.com/apache/flink-agents/pull/1129
--
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] [api][integrations][java][python] Ask each connection whether it could apply a schema natively [flink-agents]
Posted by "weiqingy (via GitHub)" <gi...@apache.org>.
weiqingy commented on code in PR #1129:
URL: https://github.com/apache/flink-agents/pull/1129#discussion_r4037675717
##########
api/src/main/java/org/apache/flink/agents/api/chat/model/BaseChatModelConnection.java:
##########
@@ -99,6 +99,48 @@ protected String effectiveModelFor(@Nullable Map<String, Object> modelParams) {
return modelParams == null ? null : (String) modelParams.get("model");
}
+ /**
+ * Whether this connection could apply {@code outputSchema} natively to a request built from
+ * these tools and parameters, leaving the effective model's capability out of the answer.
+ *
+ * <p>Feasibility, not capability: the answer covers everything this connection's native branch
+ * requires of a request apart from the effective model, and says nothing about whether the
+ * model the request names would honor a native schema, which is the separate question {@link
+ * #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.
+ *
+ * <p>An override must answer from the same logic its own request builder uses to decide the
+ * native branch, so that the answer cannot drift from what the request ends up carrying.
+ *
+ * <p>A {@code false} answer is not an error: it reports that the request would carry no native
+ * schema, so the caller keeps the prompt-engineering fallback rather than losing the schema.
+ *
+ * <p>The default {@code false} is safe only for a connection that translates no schema at all.
+ * A connection whose request builder has a native branch but which leaves this unoverridden
+ * reports every request infeasible: a caller that degrades to the prompt-engineering fallback
+ * then silently never reaches that branch, and one that refuses an unapplicable schema instead
+ * fails on a request the connection could in fact have applied.
+ *
+ * <p>Answers about the request rather than validating it. A null {@code outputSchema} is an
+ * unconstrained request, a null {@code tools} is a request binding no tools, and a null {@code
+ * modelParams} is accepted; none of the three may raise. The parameters must be read without
+ * being consumed, so that the same map still builds the request the answer was about.
+ *
+ * @param outputSchema the schema the request would carry, or null for an unconstrained request
+ * @param tools the tools the request would bind, may be null or empty for none
+ * @param modelParams the parameters the request would be built from, may be null
+ * @return true if these inputs satisfy every condition the native branch imposes apart from the
+ * effective model's capability
+ */
+ protected boolean canApplyNativeStructuredOutput(
Review 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.
##########
api/src/main/java/org/apache/flink/agents/api/chat/model/BaseChatModelConnection.java:
##########
@@ -99,6 +99,48 @@ protected String effectiveModelFor(@Nullable Map<String, Object> modelParams) {
return modelParams == null ? null : (String) modelParams.get("model");
}
+ /**
+ * Whether this connection could apply {@code outputSchema} natively to a request built from
+ * these tools and parameters, leaving the effective model's capability out of the answer.
+ *
+ * <p>Feasibility, not capability: the answer covers everything this connection's native branch
+ * requires of a request apart from the effective model, and says nothing about whether the
+ * model the request names would honor a native schema, which is the separate question {@link
+ * #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.
Review 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.
--
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] [api][integrations][java][python] Ask each connection whether it could apply a schema natively [flink-agents]
Posted by "purushah (via GitHub)" <gi...@apache.org>.
purushah commented on code in PR #1129:
URL: https://github.com/apache/flink-agents/pull/1129#discussion_r4054289843
##########
api/src/main/java/org/apache/flink/agents/api/chat/model/StructuredOutputStrategy.java:
##########
@@ -25,8 +25,8 @@
*
* <p>This expresses <b>policy</b> only. Whether a connection <i>can</i> apply the provider's native
* structured-output API is a separate, model-dependent <b>capability</b> question answered by
- * {@link BaseChatModelConnection#supportsNativeStructuredOutput(String)}. Policy and capability are
- * combined at request-build time.
+ * {@link BaseChatModelConnection#supportsNativeStructuredOutput(String)}. {@link
+ * #resolvesToNative(boolean)} combines the two.
Review 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.
```suggestion
* #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.
```
##########
python/flink_agents/integrations/chat_models/openai/openai_chat_model.py:
##########
@@ -82,6 +82,26 @@
)
+def _native_output_model(output_schema: Any) -> type[BaseModel] | None:
Review 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.
##########
integrations/chat-models/ollama/src/test/java/org/apache/flink/agents/integrations/chatmodels/ollama/OllamaChatModelConnectionTest.java:
##########
@@ -207,6 +217,91 @@ void buildRequestLeavesFormatUnsetForRowTypeInfo() {
assertThat(request.getFormat()).isNull();
}
+ /** A connection that records what the feasibility query answered on each request it built. */
+ private static OllamaChatModelConnection recordingConnection(
+ AtomicReference<Boolean> answered) {
+ ResourceDescriptor desc =
+ ResourceDescriptor.Builder.newBuilder(OllamaChatModelConnection.class.getName())
+ .addInitialArgument("endpoint", "http://localhost:11434")
+ .build();
+ return new OllamaChatModelConnection(desc, NOOP) {
+ @Override
+ protected boolean canApplyNativeStructuredOutput(
+ Object outputSchema, List<Tool> tools, Map<String, Object> modelParams) {
+ boolean answer =
+ super.canApplyNativeStructuredOutput(outputSchema, tools, modelParams);
+ answered.set(answer);
+ return answer;
+ }
+ };
+ }
+
+ @Test
+ @DisplayName("The feasibility query answers exactly what the native branch decides")
+ void feasibilityQueryAgreesWithTheNativeBranch() {
Review 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.
##########
integrations/chat-models/watsonx/src/test/java/org/apache/flink/agents/integrations/chatmodels/watsonx/WatsonxChatModelConnectionTest.java:
##########
@@ -812,6 +818,120 @@ void buildPayloadOmitsResponseFormatForRowTypeInfo() {
assertThat(payload.has("response_format")).isFalse();
}
+ /** Minimal tool carrying only metadata; never invoked in these tests. */
+ private static final class SchemaOnlyTool extends Tool {
+ SchemaOnlyTool() {
+ super(new ToolMetadata("add", "Add two numbers.", "{\"type\":\"object\"}"));
+ }
+
+ @Override
+ public ToolType getToolType() {
+ return ToolType.FUNCTION;
+ }
+
+ @Override
+ public ToolResponse call(ToolParameters parameters) {
+ throw new UnsupportedOperationException("not invoked in this test");
+ }
+ }
+
+ /** A connection that records what the feasibility query answered on each payload it built. */
+ private static WatsonxChatModelConnection recordingConnection(
+ AtomicReference<Boolean> answered) {
+ return new WatsonxChatModelConnection(
+ descriptor("https://us-south.ml.cloud.ibm.com", "test-key", "test-project"),
+ NOOP,
+ NO_ENVIRONMENT) {
+ @Override
+ protected boolean canApplyNativeStructuredOutput(
+ Object outputSchema, List<Tool> tools, Map<String, Object> modelParams) {
+ boolean answer =
+ super.canApplyNativeStructuredOutput(outputSchema, tools, modelParams);
+ answered.set(answer);
+ return answer;
+ }
+ };
+ }
+
+ @Test
+ @DisplayName("The feasibility query answers exactly what the native branch decides")
+ void feasibilityQueryAgreesWithTheNativeBranch() {
Review 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.
##########
python/flink_agents/integrations/chat_models/openai/openai_chat_model.py:
##########
@@ -262,6 +316,12 @@ def chat(
Model response message. When the response carries a finish reason,
it is available as ``extra_args["finish_reason"]``.
"""
+ # Snapshotted before the native branch below, so the feasibility query is
+ # 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)
Review 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.
##########
python/flink_agents/integrations/chat_models/tongyi_chat_model.py:
##########
@@ -247,34 +296,29 @@ def chat(
# popped on the line above, so a kwargs lookup would yield None on every call
# and report every model incapable.
#
- # TODO(#912): the requested strategy is not visible here, so this check
- # cannot tell an explicit NATIVE request apart from one that merely
- # resolved to native. A caller asking for NATIVE on a model this predicate
- # rejects therefore gets an unconstrained response instead of an error.
- # Once strategy resolution is wired up, NATIVE must either bypass this
- # capability check or fail explicitly.
- if output_schema is not None and self.supports_native_structured_output(
- model_name
- ):
- # Resolved before the conflict test, so a payload with no native
- # translation does not raise over a response_format this branch was
- # never going to write. Tested before the schema is rendered, because a
- # caller who supplies both a schema and a response_format has a conflict
- # to resolve whatever the schema turns out to render to, and reporting a
- # render failure instead would describe the wrong problem. The name is
- # read off the model class, so this needs no rendered document.
+ # The feasibility half is asked rather than restated, so a caller asking the
+ # same question gets the answer this branch acts on. A payload with no native
+ # 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(
Review 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`.
--
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] [api][integrations][java][python] Ask each connection whether it could apply a schema natively [flink-agents]
Posted by "weiqingy (via GitHub)" <gi...@apache.org>.
weiqingy commented on PR #1129:
URL: https://github.com/apache/flink-agents/pull/1129#issuecomment-6052734297
Thanks for the review, @wenjin272 !
--
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