weiqingy commented on issue #936: URL: https://github.com/apache/flink-agents/issues/936#issuecomment-5229639247
Thanks for the feedback, this unblocks both cases. Agreed on the shape of case 2 and on aligning Java to Python for case 3. Here is how I plan to land it. For the outbound serialization I will go with the allowlist rather than a separate metadata container. Java already works that way, its outbound converter reads only `refusal` (`OpenAIChatCompletionsUtils.java:88`) and `externalId` (`:96`) and never merges the whole map, so this becomes a parity fix. A hard split would also break Anthropic, which deliberately round-trips response data through `extra_args`: `anthropic_chat_model.py:337` stores `anthropic_content_blocks` off the response and `:68` reads it back when building the next request. One side effect worth flagging before I do it. Today a Python caller can put any field in `extra_args` and have it forwarded to the provider. An allowlist takes that away. Java never had it so I am treating this as parity, but say the word if you would rather keep a passthrough for caller-supplied fields. For `finish_reason` I will write it in the OpenAI-family connections and put the structured-output check in the shared path (`_generate_structured_output` in Python, `ChatModelAction` in Java), gated on the field being present. Providers that do not write it are simply not checked, so nothing changes for them today, and Anthropic picks the behavior up for free once it writes the same key. There is already a TODO for that at `anthropic_chat_model.py:345`. For case 3 I will unwrap the three wrappers around the API call itself and leave the ones around tool schema and tool argument JSON handling, since those wrap a checked `JsonProcessingException` from local work rather than a provider error. Unwrapping helps more than it first looks: the try block at `OpenAICompletionsConnection.java:186` also covers `buildRequest`, so a local validation failure like the `IllegalArgumentException` at `OpenAIChatCompletionsUtils.java:99` currently surfaces as "Failed to call OpenAI chat completions API". Unrelated to the decision, just for the record: Gemini already fails on this in every call rather than only for structured output, via the SDK's `checkFinishReason()` at `GeminiChatModelConnection.java:472`. Not touching it here, but the two integrations will behave differently. I will split this into two PRs, the Python outbound allowlist plus `finish_reason` capture in one, and the Java exception unwrapping in the other. -- 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]
