weiqingy commented on code in PR #919:
URL: https://github.com/apache/flink-agents/pull/919#discussion_r3688540218


##########
integrations/chat-models/openai/src/main/java/org/apache/flink/agents/integrations/chatmodels/openai/OpenAICompletionsConnection.java:
##########
@@ -119,11 +122,53 @@ public OpenAICompletionsConnection(
         this.client = builder.build();
     }
 
+    // Models for which OpenAI documents json_schema strict Structured Outputs 
support.
+    // Source of truth: 
https://platform.openai.com/docs/guides/structured-outputs — json_schema is
+    // supported on the gpt-4o-mini and gpt-4o-2024-08-06 snapshots "and 
later"; gpt-4-turbo,
+    // earlier models, and gpt-3.5-turbo get JSON mode only.
+    //
+    // "and later" is temporal, not a name prefix: gpt-4o-2024-05-13 predates 
the cutoff and does
+    // NOT support Structured Outputs, so matching a bare "gpt-4o" prefix 
would misclassify it as
+    // capable and fail silently at the provider. Prefix matching is therefore 
used only for the
+    // gpt-4o-mini family, whose entire lifetime post-dates the cutoff; every 
other capable model is
+    // matched exactly. An unrecognized model reports not-capable and degrades 
to the prompt
+    // fallback rather than failing at the provider.
+    private static final String NATIVE_STRUCTURED_OUTPUT_FAMILY_PREFIX = 
"gpt-4o-mini";
+    private static final Set<String> NATIVE_STRUCTURED_OUTPUT_MODELS =
+            Set.of("gpt-4o", "gpt-4o-2024-08-06", "gpt-4o-2024-11-20");
+
+    @Override
+    protected boolean supportsNativeStructuredOutput(String effectiveModel) {
+        if (effectiveModel == null) {
+            return false;
+        }
+        return 
effectiveModel.startsWith(NATIVE_STRUCTURED_OUTPUT_FAMILY_PREFIX)
+                || NATIVE_STRUCTURED_OUTPUT_MODELS.contains(effectiveModel);

Review Comment:
   Good catch, you were right in both directions. Fixed in f01e9443.
   
   The predicate now rejects any name carrying a non-text modality marker 
(`-audio`, `-realtime`, `-tts`, `-transcribe`) first, then matches a capable 
family by prefix or a capable name exactly. Checking against the model pages 
turned up three more false positives than the two you named: 
`gpt-4o-audio-preview`, `gpt-4o-mini-tts` and `gpt-4o-mini-transcribe`.
   
   Two cases are why prefix and exact matching are split:
   
   - `gpt-4o-search-preview` does support Structured Outputs, so rejecting on 
`-preview` would have swapped your false positives for a new false negative.
   - `o1` supports them but `o1-mini` does not, and `o1-mini` is served on Chat 
Completions. So `o1` is matched exactly. A prefix there would have reintroduced 
the bug you reported, and there is a test pinning `o1-mini` as rejected.
   
   Tests cover both directions in Java and Python, including every model you 
named.
   
   One tradeoff worth flagging: with prefix matching, an unknown name inside a 
listed family now reports capable, so unknown names are no longer uniformly 
not-capable. The comment in both files says so.
   



##########
integrations/chat-models/openai/src/main/java/org/apache/flink/agents/integrations/chatmodels/openai/OpenAICompletionsConnection.java:
##########
@@ -170,6 +220,13 @@ private ChatCompletionCreateParams buildRequest(
             builder.tools(convertTools(tools, strictMode));
         }
 
+        // Native structured output applies only for a POJO Class schema on a 
model the provider
+        // documents as capable; a RowTypeInfo (wrapped in OutputSchema) or an 
incapable model keeps
+        // the prompt-engineering fallback.
+        if (outputSchema instanceof Class && 
supportsNativeStructuredOutput(modelName)) {

Review Comment:
   Added in e7639ed9, both languages.
   
   It sits at the capability re-check, in `buildRequest` on the Java side and 
`chat` on the Python side. It records that the connection cannot tell an 
explicit `NATIVE` from an `AUTO` that merely resolved to native, so `NATIVE` on 
a model the predicate rejects degrades to the prompt fallback instead of 
erroring, and that #912 has to either bypass this re-check or fail explicitly.
   



-- 
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]

Reply via email to