[I] [Bug] [flink-agents]

["rosemarYuan (via GitHub)" <gi...@apache.org>]

rosemarYuan opened a new issue, #868:
URL: https://github.com/apache/flink-agents/issues/868

   ### Search before asking
   
   - [x] I searched in the [issues](https://github.com/apache/flink-agents/issues) and found nothing similar.
   
   ### Description
   
   ## Problem
   
   `ActionStateSerdeTest` currently covers `InputEvent` and `OutputEvent` round-trip only. Those two classes have `@JsonCreator` on their `(UUID, Map<String, Object>)` constructors, so they deserialize correctly.
   
   However, the built-in event classes under `org.apache.flink.agents.api.event.*`, such as `ChatRequestEvent`, `ChatResponseEvent`, `ToolRequestEvent`, `ToolResponseEvent`, `ContextRetrievalRequestEvent`, and `ContextRetrievalResponseEvent`, also have `(UUID, Map<String, Object>)` constructors but do not annotate them with `@JsonCreator/@JsonProperty`.
   
   This matters because durable execution persists events in `ActionState`, including the triggering `taskEvent` and completed `outputEvents`. Both Kafka and Fluss action state stores delegate to `ActionStateSerde`, so recovery/rebuild needs Jackson to reconstruct these concrete event subclasses from the serialized `@class` metadata.
   
   A straightforward fix would be to annotate these `(UUID, Map<String, Object>)` constructors consistently with `InputEvent`/`OutputEvent`, or otherwise teach `ActionStateSerde` how to reconstruct these built-in event subclasses during durable state recovery.
   
   One extra thing worth testing after the constructor fix: nested attributes should also come back as the expected types, e.g. `ChatMessage`, `ToolResponse`, and `Document`, rather than raw `Map` values.
   
   ## Background
   
   This looks related to #631, which introduced the cross-language unified event model. That change moved built-in event data into a common `type` + `attributes` representation and added `(UUID id, Map<String, Object> attributes)` constructors to built-in event subclasses for typed reconstruction via `fromEvent()`.
   
   However, in the #631 merge commit (`071c3a0a`), only `InputEvent` and `OutputEvent` annotated these constructors with `@JsonCreator/@JsonProperty`. Other built-in event subclasses, such as `ChatRequestEvent`, `ChatResponseEvent`, `ToolRequestEvent`, `ToolResponseEvent`, `ContextRetrievalRequestEvent`, and `ContextRetrievalResponseEvent`, have the same `(UUID, Map<String, Object>)` constructors but lack Jackson creator annotations.
   
   That works for explicit `fromEvent()` conversion, but not for `ActionStateSerde`, which relies on Jackson polymorphic deserialization to restore durable `ActionState` records.
   
   
   ### How to reproduce
   
   A minimal regression test could be added to `ActionStateSerdeTest`:
   
   ```java
   @Test
   public void testBuiltinEventSubclassesRoundTrip() throws Exception {
       ChatMessage message = new ChatMessage(MessageRole.USER, "hello");
       UUID requestId = UUID.randomUUID();
   
       assertEventRoundTrips(new ChatRequestEvent("model", List.of(message)));
       assertEventRoundTrips(new ChatResponseEvent(requestId, message));
       assertEventRoundTrips(new ToolRequestEvent("model", List.of(Map.of("name", "tool"))));
       assertEventRoundTrips(
               new ToolResponseEvent(
                       requestId,
                       Map.of("call-1", ToolResponse.success("result")),
                       Map.of("call-1", true),
                       Map.of()));
       assertEventRoundTrips(new ContextRetrievalRequestEvent("query", "vector-store", 5));
       assertEventRoundTrips(
               new ContextRetrievalResponseEvent(
                       requestId,
                       "query",
                       List.of(new Document("content", Map.of("source", "test"), "doc-1"))));
   }
   
   private static void assertEventRoundTrips(Event event) throws Exception {
       ActionState state = new ActionState(event);
       state.addEvent(event);
   
       ActionState restored = ActionStateSerde.deserialize(ActionStateSerde.serialize(state));
   
       assertEquals(event.getClass(), restored.getTaskEvent().getClass());
       assertEquals(event.getClass(), restored.getOutputEvents().get(0).getClass());
   }
   ```
   
   On the current code, the above fails with errors like:
   
   ```text
   Cannot construct instance of `org.apache.flink.agents.api.event.ChatRequestEvent`
   (no Creators, like default constructor, exist): cannot deserialize from Object value
   (no delegate- or property-based Creator)
   ```
   
   
   
   ### Version and environment
   
   0.3.0
   
   ### Are you willing to submit a PR?
   
   - [x] I'm willing to submit a PR!


-- 
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: issues-unsubscribe@flink.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] [Bug] ActionStateSerde cannot deserialize built-in event subclasses during durable recovery [flink-agents]

["wenjin272 (via GitHub)" <gi...@apache.org>]

wenjin272 closed issue #868: [Bug] ActionStateSerde cannot deserialize built-in event subclasses during durable recovery
URL: https://github.com/apache/flink-agents/issues/868


-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org