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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
