-
Notifications
You must be signed in to change notification settings - Fork 149
[api][python] Add explicit output schema parameter to the chat path (structured-output foundation) #843
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[api][python] Add explicit output schema parameter to the chat path (structured-output foundation) #843
Changes from all commits
679078a
aead976
e537e4d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,6 +25,8 @@ | |
| import org.apache.flink.agents.api.resource.ResourceType; | ||
| import org.apache.flink.agents.api.tools.Tool; | ||
|
|
||
| import javax.annotation.Nullable; | ||
|
|
||
| import java.util.List; | ||
| import java.util.Map; | ||
|
|
||
|
|
@@ -45,6 +47,26 @@ public ResourceType getResourceType() { | |
| return ResourceType.CHAT_MODEL_CONNECTION; | ||
| } | ||
|
|
||
| /** | ||
| * Whether this connection can apply the provider's native structured-output API for the given | ||
| * model. | ||
| * | ||
| * <p>Capability is <b>model-dependent</b>, not connection-wide: a single provider connection | ||
| * commonly serves both models that accept a native schema parameter and models that do not. It | ||
| * is therefore evaluated against the <i>effective</i> model at request-build time — the model | ||
| * actually being called, which per-request parameters may override. | ||
| * | ||
| * <p>The default {@code false} keeps a connection on the prompt-engineering fallback. An | ||
| * unrecognized model must report {@code false} so that it degrades to the fallback rather than | ||
| * failing at the provider. | ||
| * | ||
| * @param effectiveModel the model the request will be issued against, may be null | ||
| * @return true if a schema can be applied natively for {@code effectiveModel} | ||
| */ | ||
| protected boolean supportsNativeStructuredOutput(String effectiveModel) { | ||
| return false; | ||
| } | ||
|
|
||
| /** | ||
| * Process a chat request and return a chat response. | ||
| * | ||
|
|
@@ -55,4 +77,44 @@ public ResourceType getResourceType() { | |
| */ | ||
| public abstract ChatMessage chat( | ||
| List<ChatMessage> messages, List<Tool> tools, Map<String, Object> modelParams); | ||
|
|
||
| /** | ||
| * Process a chat request that carries an output schema, and return a chat response. | ||
| * | ||
| * <p>{@code outputSchema} is framework-level execution metadata, kept off {@code modelParams} | ||
| * so that it can never reach a provider SDK request as a generation parameter. It is either a | ||
| * POJO {@link Class} or an {@link org.apache.flink.agents.api.agents.OutputSchema} (a {@code | ||
| * RowTypeInfo} wrapper); the two cases are distinguished by the connection that consumes it. | ||
| * | ||
| * <p>A schema must not be handed to a connection that has no native translation for it: this | ||
| * default implementation rejects a non-null {@code outputSchema} rather than dropping it, so an | ||
| * unconstrained response can never be mistaken for a schema-conforming one. A null {@code | ||
| * outputSchema} delegates to {@link #chat(List, List, Map)}. A connection that does translate a | ||
| * schema into a native provider parameter overrides this overload, and reports its capability | ||
| * via {@link #supportsNativeStructuredOutput(String)}. | ||
| * | ||
| * @param messages the input chat messages | ||
| * @param tools the tools can be called by the model | ||
| * @param modelParams the additional arguments passed to the model | ||
| * @param outputSchema the schema the response should conform to, or null for an unconstrained | ||
| * response | ||
| * @return the chat response containing model outputs | ||
| * @throws UnsupportedOperationException if {@code outputSchema} is non-null and this connection | ||
| * has no native structured-output translation | ||
| */ | ||
| public ChatMessage chat( | ||
| List<ChatMessage> messages, | ||
| List<Tool> tools, | ||
| Map<String, Object> modelParams, | ||
| @Nullable Object outputSchema) { | ||
| if (outputSchema != null) { | ||
| throw new UnsupportedOperationException( | ||
| getClass().getName() | ||
| + " has no native structured-output translation, so it cannot honor" | ||
| + " the given output schema. Override chat(List, List, Map, Object) to" | ||
| + " translate the schema natively, or pass no schema so the caller" | ||
| + " applies the prompt-engineering fallback."); | ||
| } | ||
| return chat(messages, tools, modelParams); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, fixed in The default overload now throws Python needed a different shape for the same contract. One thing worth flagging since it is not visible from the diff: the throw has no production caller in either language yet, so it stays dormant until the native path lands. Java's only connection call is the three-argument one in |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one | ||
| * or more contributor license agreements. See the NOTICE file | ||
| * distributed with this work for additional information | ||
| * regarding copyright ownership. The ASF licenses this file | ||
| * to you under the Apache License, Version 2.0 (the | ||
| * "License"); you may not use this file except in compliance | ||
| * with the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.apache.flink.agents.api.chat.model; | ||
|
|
||
| import java.util.Locale; | ||
|
|
||
| /** | ||
| * User intent about how an output schema should be applied to a chat request. | ||
| * | ||
| * <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. | ||
| */ | ||
| public enum StructuredOutputStrategy { | ||
| /** | ||
| * Use the provider's native structured-output API when the effective model is capable of it, | ||
| * and fall back to prompt engineering otherwise. This is the default. | ||
| */ | ||
| AUTO, | ||
|
|
||
| /** | ||
| * Always use the provider's native structured-output API, without consulting the capability | ||
| * predicate. | ||
| */ | ||
| NATIVE, | ||
|
|
||
| /** | ||
| * Never use the provider's native structured-output API; rely on prompt engineering alone. This | ||
| * matches the behavior of connections that have no native translation. | ||
| */ | ||
| PROMPT; | ||
|
|
||
| /** | ||
| * Resolves this policy against a connection's model-dependent capability into whether the | ||
| * provider's native structured-output API should be used. | ||
| * | ||
| * <ul> | ||
| * <li>{@code AUTO} defers to {@code modelCapable}: native when the effective model can, else | ||
| * the prompt-engineering fallback. | ||
| * <li>{@code NATIVE} always resolves to native, ignoring {@code modelCapable}, so an explicit | ||
| * user intent surfaces a provider error rather than silently degrading. | ||
| * <li>{@code PROMPT} never resolves to native. | ||
| * </ul> | ||
| * | ||
| * @param modelCapable whether the connection reports the effective model as natively capable | ||
| * @return true if native structured output should be applied | ||
| */ | ||
| public boolean resolvesToNative(boolean modelCapable) { | ||
| switch (this) { | ||
| case NATIVE: | ||
| return true; | ||
| case PROMPT: | ||
| return false; | ||
| case AUTO: | ||
| default: | ||
| return modelCapable; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Resolves a strategy from a descriptor argument, which may arrive either as a {@code | ||
| * StructuredOutputStrategy} or — across the Python bridge, where arguments are carried as JSON | ||
| * — as its case-insensitive name. | ||
| * | ||
| * @param value the raw descriptor argument, may be null | ||
| * @param defaultValue the strategy to use when {@code value} is null | ||
| * @return the resolved strategy | ||
| * @throws IllegalArgumentException if {@code value} is neither null, a {@code | ||
| * StructuredOutputStrategy}, nor the name of one | ||
| */ | ||
| public static StructuredOutputStrategy fromArgument( | ||
| Object value, StructuredOutputStrategy defaultValue) { | ||
| if (value == null) { | ||
| return defaultValue; | ||
| } | ||
| if (value instanceof StructuredOutputStrategy) { | ||
| return (StructuredOutputStrategy) value; | ||
| } | ||
| if (value instanceof String) { | ||
| try { | ||
| return valueOf(((String) value).toUpperCase(Locale.ROOT)); | ||
| } catch (IllegalArgumentException e) { | ||
| throw new IllegalArgumentException( | ||
| String.format( | ||
| "Unknown structured output strategy '%s'. Expected one of: AUTO, NATIVE, PROMPT.", | ||
| value), | ||
| e); | ||
| } | ||
| } | ||
| throw new IllegalArgumentException( | ||
| String.format( | ||
| "Unsupported structured output strategy type '%s'. Expected a StructuredOutputStrategy or its name.", | ||
| value.getClass().getName())); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It appears that the
BaseChatModelSetuphas never invoked thischatinterface. SinceChatModelActionactually callsBaseChatModelSetup#chat, should we also add the corresponding interface toBaseChatModelSetup?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question, and yes, nothing calls the four-argument
chatin this PR.That is the scope here: the mechanism, with no framework path passing a schema yet. The setup-level overload is the point where the policy on the setup meets the connection-side capability, and where that resolution should live is still open. #912 lists it as an open question: "Where should
autoresolve policy into a concrete strategy, given capability is a connection-side predicate over the effective model at request-build time?"#912 is also the consumer. It issues a dedicated structured call at
ChatModelAction's no-tool-calls branch, so it is the change that both needs the setup-level entry point and can actually exercise it. Adding the overload here would settle that open question with nothing able to validate the choice, and it has to move together with normalizing effective-model resolution across Java and Python.If you would rather see the complete path in one PR, I am happy to pull it forward here instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make sense