wenjin272 commented on code in PR #1185:
URL: https://github.com/apache/flink-agents/pull/1185#discussion_r4236463843
##########
python/flink_agents/runtime/python_java_utils.py:
##########
@@ -248,6 +245,7 @@ def _encode_python_tool_result(result: Any) -> Dict[str,
Any]:
"error": result.error_message,
"execution_time_ms": result.execution_time_ms,
"tool_name": result.tool_name,
+ "blocks": result.model_dump(mode="json")["blocks"],
Review Comment:
Thanks for catching this. We refactored `ToolResponse` to use `blocks` for
model-facing content and `metadata` for application data, removing `result`.
The bridge now converts only the blocks and passes metadata through Pemja
without JSON serialization. Added a regression test covering non-UTF-8 bytes in
metadata across both bridge directions.
##########
integrations/chat-models/openai/src/main/java/org/apache/flink/agents/integrations/chatmodels/openai/OpenAICompletionsConnection.java:
##########
@@ -258,29 +260,21 @@ private ChatMessage doChat(
ChatMessage response =
OpenAIChatCompletionsUtils.convertFromOpenAIMessage(choice.message());
- // ChatCompletion.Choice#finishReason throws
OpenAIInvalidDataException when the member is
- // absent or null, so the value is read through the raw field.
- choice._finishReason()
- .asKnown()
- .ifPresent(
- reason -> response.getExtraArgs().put("finish_reason",
reason.asString()));
-
- // Stash token usage
- if (completion.usage().isPresent()) {
- String modelName = modelParams != null ? (String)
modelParams.get("model") : null;
- if (modelName == null || modelName.isBlank()) {
- modelName = this.defaultModel;
- }
- if (modelName != null && !modelName.isBlank()) {
- response.getExtraArgs().put("model_name", modelName);
- response.getExtraArgs()
- .put("promptTokens",
completion.usage().get().promptTokens());
- response.getExtraArgs()
- .put("completionTokens",
completion.usage().get().completionTokens());
- }
- }
-
- return response;
+ return new ChatResult(
+ response,
+ (modelParams != null && modelParams.get("model") != null
+ ? (String) modelParams.get("model")
+ : this.defaultModel),
Review Comment:
Fixed: request construction and `ChatResult.model` now use the same
`effectiveModelFor()` resolution. Added local HTTP tests for OpenAI and vLLM
covering missing, null, empty, whitespace and explicit model overrides,
checking both the outgoing request and result attribution.
##########
api/src/main/java/org/apache/flink/agents/api/chat/messages/ContentBlock.java:
##########
@@ -27,28 +27,34 @@
/**
* A single, typed part of a {@link ChatMessage}'s content.
*
- * <p>Blocks are ordered within a message and are immutable value objects:
every construction path,
- * including Jackson deserialization, runs the same validation, so sharing a
block instance never
- * shares mutable state. The concrete type answers how providers route the
content ({@link
+ * <p>Blocks are ordered within a message. Block fields are read-only, while
metadata and tool
+ * inputs use ordinary maps. Every construction path, including Jackson
deserialization, runs the
+ * same structural validation. The concrete type answers how providers route
the content ({@link
* TextBlock}, {@link ImageBlock}, {@link AudioBlock}, {@link VideoBlock},
{@link DocumentBlock}),
* while media encoding is carried by the media type on {@link MediaBlock}.
*
* <p>The serialized form carries a {@code type} discriminator with fixed
values ({@code text},
* {@code image}, {@code audio}, {@code video}, {@code document}) shared with
the Python API, so
* blocks cross the Java/Python boundary as plain JSON.
*/
-@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY,
property = "type")
+@JsonTypeInfo(
+ use = JsonTypeInfo.Id.NAME,
+ include = JsonTypeInfo.As.EXISTING_PROPERTY,
+ property = "type")
@JsonSubTypes({
@JsonSubTypes.Type(value = TextBlock.class, name = "text"),
@JsonSubTypes.Type(value = ImageBlock.class, name = "image"),
@JsonSubTypes.Type(value = AudioBlock.class, name = "audio"),
@JsonSubTypes.Type(value = VideoBlock.class, name = "video"),
- @JsonSubTypes.Type(value = DocumentBlock.class, name = "document")
+ @JsonSubTypes.Type(value = DocumentBlock.class, name = "document"),
+ @JsonSubTypes.Type(value = ReasoningBlock.class, name = "reasoning"),
+ @JsonSubTypes.Type(value = ToolCallBlock.class, name = "tool_call"),
+ @JsonSubTypes.Type(value = ToolResultBlock.class, name = "tool_result")
Review Comment:
Thanks, agreed that tool-result content should have a narrower type. We
introduced `DataContentBlock` for text/media and restricted
`ToolResultBlock.blocks` and `ToolResponse.blocks` to that subset in both Java
and Python.
We kept a single ordered `ContentBlock` list in `ChatMessage` to preserve
content order and avoid separate content fields. This broad content-block model
is also used by
[AgentScope](https://github.com/agentscope-ai/agentscope/blob/main/src/agentscope/message/_block.py#L219-L226)
and
[LangChain](https://github.com/langchain-ai/langchain/blob/master/libs/core/langchain_core/messages/content.py#L832-L853),
whose unions include text/media, reasoning and tool-related blocks.
--
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]
