Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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.
*
Expand All @@ -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(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears that the BaseChatModelSetup has never invoked this chat interface. Since ChatModelAction actually calls BaseChatModelSetup#chat, should we also add the corresponding interface to BaseChatModelSetup?

Copy link
Copy Markdown
Collaborator Author

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 chat in 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 auto resolve 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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make sense

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);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When NATIVE is configured, resolvesToNative(false) still returns true, so the caller may invoke the four-argument chat() even when the connection does not support native structured output. With the current default implementation, outputSchema is silently ignored and an unconstrained response may be returned. I think the default implementation should throw UnsupportedOperationException for a non-null outputSchema; supporting connections can override it, while null can continue delegating to the three-argument overload.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, fixed in aead976.

The default overload now throws UnsupportedOperationException for a non-null outputSchema and still delegates to the three-argument overload for null, so a connection that translates a schema overrides it and nothing changes for schemaless calls.

Python needed a different shape for the same contract. chat is abstract there, so there is no inherited body that could refuse a schema. A guard on the connection base raises NotImplementedError, and every connection without a native translation calls it as the first statement of chat, before any client access. The connection subclass walk in test_output_schema_param_declared.py now asserts the rejection as well, so a connection added later is held to 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 BaseChatModelSetup, and on the Python side chat_model_action.py calls chat without a schema and applies the prompt fallback separately.

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ public abstract class BaseChatModelSetup extends Resource {
@Nullable protected String skillDiscoveryPrompt;
protected List<String> allowedCommands;
protected List<String> allowedScriptDirs;
protected StructuredOutputStrategy structuredOutputStrategy;

@Nullable protected BaseChatModelConnection connection;
protected final List<Tool> tools = new ArrayList<>();
Expand All @@ -67,6 +68,10 @@ public BaseChatModelSetup(ResourceDescriptor descriptor, ResourceContext resourc
declaredScriptDirs == null
? new ArrayList<>()
: new ArrayList<>(declaredScriptDirs);
this.structuredOutputStrategy =
StructuredOutputStrategy.fromArgument(
descriptor.getArgument("structured_output_strategy"),
StructuredOutputStrategy.AUTO);
}

/**
Expand Down Expand Up @@ -219,4 +224,15 @@ public List<String> getAllowedCommands() {
public List<String> getAllowedScriptDirs() {
return allowedScriptDirs;
}

/**
* The configured intent about how an output schema should be applied, defaulting to {@link
* StructuredOutputStrategy#AUTO}. Whether native structured output is actually applied combines
* this policy with the connection's model-dependent capability.
*
* @return the structured output strategy
*/
public StructuredOutputStrategy getStructuredOutputStrategy() {
return structuredOutputStrategy;
}
}
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()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ void testChatResponseFormat() {
/** Connection that captures the messages passed to it for assertions. */
private static class RecordingConnection extends BaseChatModelConnection {
List<ChatMessage> capturedMessages;
Map<String, Object> capturedModelParams;

RecordingConnection() {
super(
Expand All @@ -249,6 +250,7 @@ private static class RecordingConnection extends BaseChatModelConnection {
public ChatMessage chat(
List<ChatMessage> messages, List<Tool> tools, Map<String, Object> modelParams) {
this.capturedMessages = new ArrayList<>(messages);
this.capturedModelParams = new HashMap<>(modelParams);
return new ChatMessage(MessageRole.ASSISTANT, "ok");
}
}
Expand Down Expand Up @@ -320,6 +322,124 @@ void testChatRefillsTemplateOnSubsequentInvocations() {
assertEquals("tool result", connection.capturedMessages.get(1).getContent());
}

@Test
@DisplayName("Default chat() overload rejects an outputSchema it cannot translate")
void testDefaultChatOverloadRejectsOutputSchema() {
RecordingConnection connection = new RecordingConnection();

// Dropping the schema instead would return an unconstrained response that the
// caller has no way to tell apart from a schema-conforming one.
assertThrows(
UnsupportedOperationException.class,
() ->
connection.chat(
List.of(new ChatMessage(MessageRole.USER, "hi")),
List.of(),
new HashMap<>(),
new Object()));

// The rejection has to precede the delegation: a delegate-then-throw ordering
// would still issue a real provider request before failing.
assertNull(connection.capturedMessages);
}

@Test
@DisplayName("Default chat() overload delegates to the 3-arg chat() for a null outputSchema")
void testDefaultChatOverloadDelegatesForNullOutputSchema() {
RecordingConnection connection = new RecordingConnection();
Map<String, Object> modelParams = new HashMap<>();
modelParams.put("temperature", 0.5);

ChatMessage response =
connection.chat(
List.of(new ChatMessage(MessageRole.USER, "hi")),
List.of(),
modelParams,
null);

// The 3-arg chat() ran (it is what produces "ok") and the overload added nothing
// to modelParams that could travel on to a provider SDK request.
assertEquals("ok", response.getContent());
assertEquals(Map.of("temperature", 0.5), connection.capturedModelParams);
}

@Test
@DisplayName("Default capability predicate reports no native structured output for any model")
void testDefaultCapabilityPredicateIsFalse() {
RecordingConnection connection = new RecordingConnection();

assertFalse(connection.supportsNativeStructuredOutput("gpt-4o"));
assertFalse(connection.supportsNativeStructuredOutput("gpt-3.5-turbo"));
assertFalse(connection.supportsNativeStructuredOutput(null));
}

@Test
@DisplayName("Structured-output strategy defaults to AUTO when the descriptor omits it")
void testStructuredOutputStrategyDefaultsToAuto() {
RecordingChatModelSetup setup =
new RecordingChatModelSetup(new RecordingConnection(), null);

assertEquals(StructuredOutputStrategy.AUTO, setup.getStructuredOutputStrategy());
}

@Test
@DisplayName("Structured-output strategy defaults to AUTO when the descriptor argument is null")
void testStructuredOutputStrategyDefaultsToAutoForNullArgument() {
// A descriptor argument present with a null value is indistinguishable from an
// absent one here, so it resolves to the same default rather than failing.
TestChatModel model =
new TestChatModel(
new ResourceDescriptor(
TestChatModel.class.getName(),
Collections.singletonMap("structured_output_strategy", null)),
null);

assertEquals(StructuredOutputStrategy.AUTO, model.getStructuredOutputStrategy());
}

@Test
@DisplayName("Structured-output strategy is read from the descriptor argument")
void testStructuredOutputStrategyReadFromDescriptor() {
TestChatModel model =
new TestChatModel(
new ResourceDescriptor(
TestChatModel.class.getName(),
Map.of("structured_output_strategy", "native")),
null);

assertEquals(StructuredOutputStrategy.NATIVE, model.getStructuredOutputStrategy());
}

@Test
@DisplayName("An unrecognized structured-output strategy is rejected instead of defaulting")
void testUnknownStructuredOutputStrategyRejected() {
ResourceDescriptor descriptor =
new ResourceDescriptor(
TestChatModel.class.getName(),
Map.of("structured_output_strategy", "bogus"));

assertThrows(IllegalArgumentException.class, () -> new TestChatModel(descriptor, null));
}

@Test
@DisplayName("AUTO resolves to native only when the effective model is capable")
void testAutoStrategyResolvesToNativeOnlyWhenCapable() {
assertTrue(StructuredOutputStrategy.AUTO.resolvesToNative(true));
assertFalse(StructuredOutputStrategy.AUTO.resolvesToNative(false));
}

@Test
@DisplayName("NATIVE forces native even when the model is not capable")
void testNativeStrategyForcesNativeRegardlessOfCapability() {
assertTrue(StructuredOutputStrategy.NATIVE.resolvesToNative(false));
}

@Test
@DisplayName("PROMPT never resolves to native even when the model is capable")
void testPromptStrategyNeverResolvesToNative() {
assertFalse(StructuredOutputStrategy.PROMPT.resolvesToNative(true));
}

@Test
@DisplayName("Test chat with long input")
void testChatWithLongInput() {
Expand Down
Loading
Loading