wenjin272 commented on code in PR #950:
URL: https://github.com/apache/flink-agents/pull/950#discussion_r3725525928


##########
api/src/main/java/org/apache/flink/agents/api/Event.java:
##########
@@ -125,7 +149,19 @@ public static Event fromEvent(Event event) {
      * @throws IOException if JSON parsing fails or the 'type' field is 
missing or empty
      */
     public static Event fromJson(String json) throws IOException {
-        return MAPPER.readValue(json, Event.class);
+        Event event = MAPPER.readValue(json, Event.class);
+        for (Map.Entry<String, Object> entry : 
event.getAttachments().entrySet()) {

Review Comment:
   **[P1] Preserve `MemoryRef` when restoring `ActionState`**
   
   Thanks for adding JSON support for `MemoryRef`. This conversion only runs 
when `Event.fromJson()` is called explicitly. Durable recovery instead 
deserializes the enclosing `ActionState` directly through `ActionStateSerde`, 
so attachment values declared as `Object` are restored as `LinkedHashMap` 
rather than `MemoryRef`. `loadEventAttachments()` then skips them, and the 
recovered action receives the reference-shaped map instead of the original 
payload.
   
   A possible implementation sketch would be to bind a content deserializer 
directly to the `attachments` values:
   
   ```java
   @JsonCreator
   public Event(
           // ...
           @JsonProperty("attachments")
           @JsonDeserialize(contentUsing = AttachmentValueDeserializer.class)
           Map<String, Object> attachments) {
       // ...
   }
   
   static final class AttachmentValueDeserializer extends 
JsonDeserializer<Object> {
       @Override
       public Object deserialize(JsonParser p, DeserializationContext ctxt)
               throws IOException {
           JsonNode node = p.getCodec().readTree(p);
           if (node.isObject()
                   && "memory_ref".equals(node.path("@type").asText())) {
               return p.getCodec().treeToValue(node, MemoryRef.class);
           }
           return p.getCodec().treeToValue(node, Object.class);
       }
   }
   ```
   
   The `MemoryRef` serializer would emit a language-neutral discriminator, for 
example `{"@type": "memory_ref", "memory_type": "sensory", "path": "..."}`. 
Merely adding the discriminator to the JSON is not sufficient: because the 
declared map value type is still `Object`, Jackson also needs this 
property-level deserialization hook (or an equivalent central hook) to select 
`MemoryRef.Deserializer`. This should use the same discriminator on the Python 
side, and an `ActionStateSerde` round-trip test should verify that the restored 
attachment is still a `MemoryRef`.



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