wenjin272 commented on issue #936:
URL: https://github.com/apache/flink-agents/issues/936#issuecomment-5226331269

   For cases 2 and 3, I suggest handling them as follows:
   
   **Case 2: `finish_reason` handling**
   
   We should preserve the provider's `finish_reason` in `ChatMessage` metadata 
in both Java and Python. For normal, non-structured responses, the connection 
should still return the message even when the completion is truncated, since 
the partial content may be useful and the caller can decide how to handle it.
   
   For structured output, however, we should inspect `finish_reason` before 
parsing the content:
   
   - `length`: fail with a clear error indicating that the response was 
truncated, instead of exposing a misleading JSON parsing error.
   - `content_filter`: fail with an explicit content-filtering error.
   - `stop` and `tool_calls`: treat them as normal completion reasons.
   - Unknown reasons: preserve them without failing automatically.
   
   There is also a related Python serialization issue that should be addressed 
before storing `finish_reason` in `extra_args`. `convert_to_openai_message()` 
currently merges all `extra_args` into outbound messages. This means 
response-only metadata such as `finish_reason` could be sent back to the 
provider during a later tool-call round. The existing `model_name`, 
`promptTokens`, and `completionTokens` fields have the same problem. We should 
either separate response metadata from provider request fields or explicitly 
allowlist the fields that may be serialized into an outbound request.
   
   **Case 3: exception handling**
   
   For now, I suggest removing the generic `RuntimeException` wrappers in the 
Java OpenAI connections and allowing the original SDK exceptions to propagate, 
which would also align Java with the current Python behavior. The SDK 
exceptions already expose useful details such as the HTTP status and provider 
error information, while the generic wrapper forces callers to inspect the 
cause.
   
   Local validation failures can continue using `IllegalArgumentException` or 
`ValueError`. If we later want a provider-neutral exception hierarchy, it would 
be better to design and apply it consistently across all chat-model 
integrations rather than introducing it only for OpenAI.
   


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