wenjin272 commented on code in PR #930:
URL: https://github.com/apache/flink-agents/pull/930#discussion_r3701061737
##########
integrations/chat-models/openai/src/main/java/org/apache/flink/agents/integrations/chatmodels/openai/AzureOpenAIChatModelConnection.java:
##########
@@ -151,84 +194,226 @@ public AzureOpenAIChatModelConnection(
this.client = clientBuilder.build();
}
+ /**
+ * Whether Azure documents json_schema strict support for {@code
effectiveModel}.
+ *
+ * <p>{@code effectiveModel} is the model backing an Azure deployment, not
the deployment name.
+ * See the allowlist above for the source of truth and for why the match
is exact. An
+ * unrecognized model reports {@code false} so it degrades to the
prompt-engineering fallback
+ * rather than failing at the provider.
+ *
+ * <p>Reads no instance state, so capability stays answerable
independently of how the
+ * connection was configured.
+ */
+ @Override
+ protected boolean supportsNativeStructuredOutput(String effectiveModel) {
+ if (effectiveModel == null || effectiveModel.isEmpty()) {
+ return false;
+ }
+ return NATIVE_STRUCTURED_OUTPUT_MODELS.contains(effectiveModel);
+ }
+
+ /**
+ * Whether the configured api-version reaches the structured-output floor.
+ *
+ * <p>Azure documents {@code 2024-08-01-preview} as the first api-version
supporting structured
+ * outputs, and whether an older version rejects {@code response_format}
or silently ignores it
+ * is not documented. The request therefore never carries {@code
response_format} below the
+ * floor, which is safe under either behavior.
+ *
+ * <p>The comparison assumes the documented api-version form, a
zero-padded {@code YYYY-MM-DD}
+ * date optionally suffixed {@code -preview}; over that form comparing the
leading date
+ * lexicographically is exact. The GA {@code v1} literal sorts above the
floor, which matches
+ * Azure documenting {@code v1} as supporting structured outputs. This is
not a validator: a
+ * value of any other shape is not classified reliably, and the service
rejects an api-version
+ * it does not recognize. The constructor rejects a null or blank
api-version, so no value of
+ * that shape reaches here.
+ */
+ private boolean apiVersionSupportsStructuredOutput() {
+ String datePrefix =
+ apiVersion.length() >
MIN_STRUCTURED_OUTPUT_API_VERSION.length()
+ ? apiVersion.substring(0,
MIN_STRUCTURED_OUTPUT_API_VERSION.length())
+ : apiVersion;
+ return datePrefix.compareTo(MIN_STRUCTURED_OUTPUT_API_VERSION) >= 0;
+ }
+
@Override
public ChatMessage chat(
List<ChatMessage> messages, List<Tool> tools, Map<String, Object>
modelParams) {
+ return doChat(messages, tools, modelParams, null);
+ }
+
+ /**
+ * Translates {@code outputSchema} into Azure's native strict {@code
response_format}
+ * json_schema when it is a POJO {@link Class}, the model backing the
deployment is one Azure
+ * documents json_schema strict support for, and the configured
api-version reaches {@code
+ * 2024-08-01-preview}. Any other combination leaves the request
unconstrained so that the
+ * prompt-engineering fallback still governs the response, rather than
failing at the provider.
+ *
+ * <p>Capability is keyed on the {@code model_of_azure_deployment} model
parameter rather than
+ * on the deployment the request targets, because a deployment name is
chosen by the user and
+ * carries no model information. Leaving that parameter unset therefore
keeps even a capable
+ * deployment on the fallback.
+ *
+ * @throws IllegalArgumentException if the schema is applied natively
while {@code
+ * additional_kwargs} also carries a {@code response_format}, since
the two would otherwise
+ * compete on the same request
+ */
+ @Override
+ public ChatMessage chat(
+ List<ChatMessage> messages,
+ List<Tool> tools,
+ Map<String, Object> modelParams,
+ Object outputSchema) {
+ return doChat(messages, tools, modelParams, outputSchema);
+ }
+
+ private ChatMessage doChat(
+ List<ChatMessage> messages,
+ List<Tool> tools,
+ Map<String, Object> modelParams,
+ Object outputSchema) {
try {
- Map<String, Object> mutableArgs =
- modelParams != null ? new HashMap<>(modelParams) : new
HashMap<>();
+ ChatCompletionCreateParams params =
+ buildRequest(messages, tools, modelParams, outputSchema);
+ return toResponse(client.chat().completions().create(params),
modelParams);
+ } catch (IllegalArgumentException e) {
+ throw e;
+ } catch (Exception e) {
+ throw new RuntimeException("Failed to call Azure OpenAI chat
completions API.", e);
+ }
+ }
- String azureDeployment = (String) mutableArgs.remove("model");
- if (azureDeployment == null || azureDeployment.isBlank()) {
- throw new IllegalArgumentException("model is required for
Azure OpenAI API calls");
- }
- String modelOfAzureDeployment =
- (String) mutableArgs.remove("model_of_azure_deployment");
+ // Package-private so response handling can be asserted against a
constructed completion without
+ // issuing a live API call through the final OpenAI client.
+ ChatMessage toResponse(ChatCompletion completion, Map<String, Object>
modelParams) {
+ // Read from the caller's map rather than the copy buildRequest
consumed, and read without
+ // consuming: a caller may reuse the same map across calls. The map is
assembled fresh for
+ // each call and no one retains it, so reading it once the response
has arrived yields the
+ // same value as reading it before the request was issued. Token
metrics report the model
+ // backing the deployment, which buildRequest only uses to decide
capability.
+ String modelOfAzureDeployment =
+ modelParams != null ? (String)
modelParams.get("model_of_azure_deployment") : null;
+
+ ChatMessage response =
+ OpenAIChatCompletionsUtils.convertFromOpenAIMessage(
+ completion.choices().get(0).message());
+
+ if (modelOfAzureDeployment != null
+ && !modelOfAzureDeployment.isBlank()
+ && completion.usage().isPresent()) {
+ response.getExtraArgs().put("model_name", modelOfAzureDeployment);
+ response.getExtraArgs().put("promptTokens",
completion.usage().get().promptTokens());
+ response.getExtraArgs()
+ .put("completionTokens",
completion.usage().get().completionTokens());
+ }
- ChatCompletionCreateParams.Builder builder =
- ChatCompletionCreateParams.builder()
- .model(ChatModel.of(azureDeployment))
-
.messages(OpenAIChatCompletionsUtils.convertToOpenAIMessages(messages));
+ return response;
+ }
- if (tools != null && !tools.isEmpty()) {
- builder.tools(convertTools(tools));
- }
+ // Package-private so the request body (including the native
response_format) can be asserted
+ // without issuing a live API call through the final OpenAI client.
+ ChatCompletionCreateParams buildRequest(
+ List<ChatMessage> messages,
+ List<Tool> tools,
+ Map<String, Object> rawModelParams,
+ Object outputSchema) {
+ Map<String, Object> mutableArgs =
+ rawModelParams != null ? new HashMap<>(rawModelParams) : new
HashMap<>();
+
+ String azureDeployment = (String) mutableArgs.remove("model");
+ if (azureDeployment == null || azureDeployment.isBlank()) {
+ throw new IllegalArgumentException("model is required for Azure
OpenAI API calls");
+ }
+ String modelOfAzureDeployment = (String)
mutableArgs.remove("model_of_azure_deployment");
- Object temperature = mutableArgs.remove("temperature");
- if (temperature instanceof Number) {
- builder.temperature(((Number) temperature).doubleValue());
- }
+ ChatCompletionCreateParams.Builder builder =
+ ChatCompletionCreateParams.builder()
+ .model(ChatModel.of(azureDeployment))
+
.messages(OpenAIChatCompletionsUtils.convertToOpenAIMessages(messages));
- Object maxTokens = mutableArgs.remove("max_tokens");
- if (maxTokens instanceof Number) {
- builder.maxCompletionTokens(((Number) maxTokens).longValue());
- }
+ if (tools != null && !tools.isEmpty()) {
+ builder.tools(convertTools(tools));
+ }
- Object logprobs = mutableArgs.remove("logprobs");
- if (Boolean.TRUE.equals(logprobs)) {
- builder.logprobs(true);
- }
+ // Capability belongs to the model backing the deployment, so it is
the input to the check;
+ // the deployment name is chosen by the user and carries none. Native
structured output
+ // applies only for a POJO Class schema — a RowTypeInfo (wrapped in
OutputSchema) keeps the
+ // prompt-engineering fallback, as do an incapable model and an
api-version below the floor.
+ String nativeSchemaName = null;
+ if (outputSchema instanceof Class
Review Comment:
On every non-native branch, this omits response_format and still sends the
request. The connection neither adds a schema prompt nor signals the caller to
fall back, so a non-null outputSchema is silently dropped and direct callers
receive an unconstrained response instead of the previous exception. Could we
reject the schema when native format cannot be applied, leaving the caller to
invoke the prompt path with a null schema?
##########
python/flink_agents/integrations/chat_models/azure/azure_openai_chat_model.py:
##########
@@ -114,6 +185,41 @@ def client(self) -> AzureOpenAI:
)
return self._client
+ @override
+ def supports_native_structured_output(self, effective_model: str | None)
-> bool:
+ """Whether Azure documents json_schema strict support for
``effective_model``.
+
+ ``effective_model`` is the model backing an Azure deployment, not the
deployment
+ name. See the module-level allowlist for the source of truth and for
why the
+ match is exact. An unrecognized model reports ``False`` so it degrades
to the
+ prompt-engineering fallback rather than failing at the provider.
+
+ Reads no instance state, so it stays answerable on an instance that
was never
+ initialized, where any field access would raise.
+ """
+ if not effective_model:
+ return False
+ return effective_model in _NATIVE_STRUCTURED_OUTPUT_MODELS
+
+ def _api_version_supports_structured_output(self) -> bool:
+ """Whether the configured api-version reaches the structured-output
floor.
+
+ Azure documents ``2024-08-01-preview`` as the first api-version
supporting
+ structured outputs, and whether an older version rejects
``response_format`` or
+ silently ignores it is not documented. The request therefore never
carries
+ ``response_format`` below the floor, which is safe under either
behavior.
+
+ The comparison assumes the documented api-version form, a zero-padded
+ ``YYYY-MM-DD`` date optionally suffixed ``-preview``; over that form
comparing
+ the leading date lexicographically is exact. The GA ``v1`` literal
sorts above
+ the floor, which matches Azure documenting ``v1`` as supporting
structured
+ outputs. This is not a validator: a value of any other shape is not
classified
+ reliably, and the service rejects an api-version it does not recognize.
+ """
+ if not self.api_version:
+ return False
+ return self.api_version[:10] >= _MIN_STRUCTURED_OUTPUT_API_VERSION
Review Comment:
Could we avoid treating v1 as supported until this connection uses the Azure
unified endpoint? The current AzureOpenAI client still calls
/openai/deployments/{model}/chat/completions?api-version=v1, while Azure v1
uses /openai/v1/chat/completions. The mocked test does not cover the final URL,
so we should either remove v1 from the gate/test or add unified-endpoint
handling in both languages.
##########
python/flink_agents/integrations/chat_models/azure/azure_openai_chat_model.py:
##########
@@ -40,6 +47,70 @@
{"model", "model_of_azure_deployment", "temperature", "max_tokens",
"logprobs"}
)
+# Models with documented json_schema strict Structured Outputs support. Source
of truth:
+#
https://learn.microsoft.com/en-us/azure/ai-foundry/openai/how-to/structured-outputs
+#
+# Matching is exact, never by prefix: Azure exposes a deployment's model name
and model
+# version as separate properties, so a name carries no version to discriminate
on. The
+# documented list includes gpt-4o only at versions 2024-08-06 and 2024-11-20
while
+# version 2024-05-13 is unsupported, so a bare "gpt-4o" is ambiguous and is
deliberately
+# absent from the set below. An unrecognized name reports not-capable and
degrades to
+# the prompt fallback rather than failing at the provider.
+#
+# The source list prints "gpt-5.1-codex mini" with a space; it is transcribed
hyphenated
+# here because Azure model identifiers do not contain spaces.
+_NATIVE_STRUCTURED_OUTPUT_MODELS = frozenset(
+ {
+ "gpt-5.1-codex",
Review Comment:
This allowlist includes Responses-only models such as gpt-5.1-codex,
gpt-5.1-codex-mini, and gpt-5-codex, but this connection calls
chat.completions.create. Could we intersect the Structured Outputs list with
the models that Azure supports on the Chat Completions API?
--
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]
