joeyutong commented on code in PR #1185:
URL: https://github.com/apache/flink-agents/pull/1185#discussion_r4237219617
##########
api/src/main/java/org/apache/flink/agents/api/tools/ToolResponse.java:
##########
@@ -42,52 +59,59 @@ public class ToolResponse {
@JsonCreator
private ToolResponse(
- @JsonProperty("result") Object result,
+ @JsonProperty("blocks") List<? extends DataContentBlock> blocks,
+ @JsonProperty("metadata") Map<String, Object> metadata,
@JsonProperty("success") boolean success,
@JsonProperty("error") String error,
@JsonProperty("execution_time_ms") long executionTimeMs,
@JsonProperty("tool_name") String toolName) {
- this.result = result;
+ if (success != (error == null)) {
+ throw new IllegalArgumentException("ToolResponse success and error
disagree");
+ }
+ // Kryo restores collections by adding elements to the backing list.
+ this.blocks = blocks == null ? new ArrayList<>() : new
ArrayList<>(List.copyOf(blocks));
+ this.metadata = metadata == null ? Map.of() : metadata;
Review Comment:
`Map.of()` has a Flink-version compatibility caveat: **Flink 1.20.5 uses
Kryo 2.24.0**, which does not correctly round-trip JDK immutable maps with its
default serializers. **Flink 2.3.0 uses Kryo 5.6.2**, which has dedicated
serializers for these maps. This is worth considering for `metadata` here when
supporting Flink 1.20.
--
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]
