weiqingy opened a new issue, #936:
URL: https://github.com/apache/flink-agents/issues/936
### Background
The OpenAI-family chat model connections (OpenAI and Azure OpenAI, both
languages) converge on a shared message conversion helper. Comparing the two
implementations, the non-happy-path branches diverge in three places. All three
predate any current work, and all three are invisible on the happy path, which
is why they have survived.
Native structured output makes the first two materially more reachable. With
`strict: true` a model refuses rather than emitting non-conforming JSON, so the
refusal branch stops being an edge case and becomes the primary non-happy path.
### 1. A refusal is preserved in Java and dropped in Python
Java, `OpenAIChatCompletionsUtils.convertFromOpenAIMessage`
(`integrations/chat-models/openai/src/main/java/org/apache/flink/agents/integrations/chatmodels/openai/OpenAIChatCompletionsUtils.java:113`):
```java
String content = message.content().orElse("");
ChatMessage response = ChatMessage.assistant(content);
message.refusal().ifPresent(refusal ->
response.getExtraArgs().put("refusal", refusal));
```
Python, `convert_from_openai_message`
(`python/flink_agents/integrations/chat_models/openai/openai_utils.py:217`):
```python
return ChatMessage(
role=MessageRole(message.role),
content=message.content or "",
tool_calls=tool_calls,
extra_args=extra_args,
)
```
There is no refusal handling on the Python side. `grep -rn "refusal"
python/flink_agents/` returns zero hits across the whole tree, and `git log
-S"refusal" -- python/flink_agents/` returns no commit, so the field has never
been read there.
When a provider refuses, `content` is empty on both sides. A Java caller can
recover the reason from `extraArgs["refusal"]`. A Python caller receives an
empty assistant message with the reason discarded, and has no way to
distinguish a refusal from a genuinely empty completion.
### 2. `finish_reason` is never inspected in either language
`grep -rni "finishReason\|finish_reason"` returns zero hits in both
`integrations/chat-models/openai/src/main/java/` and
`python/flink_agents/integrations/chat_models/`.
A completion truncated by a token limit therefore returns as ordinary
partial content, with nothing indicating it is incomplete. Under structured
output this is the more damaging case of the two, because truncated JSON fails
to parse downstream with an error that points at the parser rather than at the
truncation.
Unlike case 1 this one is symmetric, so it is a behavior question rather
than a parity question: should a truncated completion raise, or surface the
reason to the caller and let them decide?
### 3. Provider errors are wrapped in Java and raw in Python
Java wraps non-argument exceptions from the call in
`RuntimeException("Failed to call ... API.", e)`. The corresponding Python
paths have no `try`/`except`, so an SDK exception, including a provider 400,
propagates to the caller unchanged.
Neither behavior is obviously wrong on its own. The divergence is the issue,
since a caller writing against both languages sees different failure types for
the same provider condition.
### Proposed handling
Case 1 is a clear parity fix with an established reference implementation,
and is small and self-contained: mirror the Java behavior in the Python
converter and cover it with a test on both sides. Worth doing on its own.
Cases 2 and 3 are design questions rather than mechanical fixes, and I would
rather not fold them into the same change. Raising them here so the decision is
recorded in one place.
I'm willing to submit a PR for case 1, and happy to take 2 and 3 as
follow-ups once there is a decision on the intended behavior.
--
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]
