yunfengzhou-hub opened a new pull request, #1184: URL: https://github.com/apache/flink-agents/pull/1184
Linked issue: #1125 ### Purpose of change **Caller-visible outcome.** `Event.fromJson` (Java) and `Event.from_json` (Python) now return the concrete built-in subclass (`ChatRequestEvent`, `ChatResponseEvent`, `ToolRequestEvent`, the memory events, …) instead of a base `Event`, for any registered built-in type. Unknown and user-defined types are unchanged. **Why.** Built-in events cross the Python/Java boundary as JSON and were deserialized into the base `Event`, whose `attributes` is a generic map/dict, so nested typed values (e.g. a multimodal `ChatMessage`) degraded to plain maps. The infrastructure that runs *before* an Action — event router, Event Log, listeners — therefore saw an untyped event even for a known type. One concrete consequence: the Event Log's `ChatMessage` sanitizer is registered for `ChatMessage.class`, so a cross-language chat request whose messages arrived as maps bypassed sanitization and could log inline media payloads and signed-URL credentials verbatim. **Runtime flow.** 1. One side serializes a built-in event to wire JSON (e.g. Python `model_dump_json`). 2. The other side deserializes at the single lowest-level entry point: `Event.fromJson` → `MAPPER.readValue(json, Event.class)` gives a base `Event` with generic-map attributes. 3. `BuiltInEvents.restore(event)` looks up `event.getType()` in a central registry: registered → the type's existing `fromEvent`/`from_event` rebuilds the concrete subclass (re-validating nested types); unregistered → the base `Event` is returned unchanged. 4. Router, Event Log, listeners and actions all see the typed event; the Event Log sanitizer now engages for cross-language chat messages. **Key decisions.** - One hook at the lowest-level deserialization point, not a patch per consumer. - Reuse each type's existing `fromEvent`/`from_event` as the reconstructor, keeping validation in one place per type. - Central registry keyed by event-type string, with a drift test against `EventType` constants; the 7 memory types dispatch through `MemoryEvent.fromEvent`. - Python imports the registry lazily inside `from_json` (the registry imports the subclasses, which import `event` — a top-level import would be circular). - `ModelRoutingEvent` is registered Java-side only (no Python counterpart). ### Behavioral Semantics **Interaction decisions** (registered? × already-concrete?): | Event type | Incoming shape | Result | |---|---|---| | Registered built-in | Generic base `Event` | Reconstructed to concrete subclass; nested typed values rebuilt | | Registered built-in | Already concrete | Reconstructed to same subclass; id preserved (idempotent) | | Unknown / user-defined | Generic base `Event` | Same instance returned unchanged, stays generic | | Registered built-in | Malformed attributes | Raises `IllegalArgumentException` (Java) / `ValueError` (Python) | | Any | `null` (Java `restore`) | Returns `null` | **Behavioral contracts.** 1. A registered built-in type yields an instance of its concrete subclass, with nested typed values (e.g. `ChatMessage` content blocks) reconstructed, not left as maps. 2. Restoration preserves event id, source timestamp, upstream event id, upstream action name, and attachments. 3. An unknown/user-defined type is returned as the same generic `Event` instance. 4. Restoration is idempotent, so actions that still call `fromEvent`/`from_event` keep working. 5. A cross-language multimodal chat request, once restored, has its inline media payload and signed-URL credentials/query sanitized out of the Event Log at STANDARD and VERBOSE. **Failure behavior.** - Malformed registered event (e.g. a memory event missing its value; an `OutputEvent` carrying attachments): `restore` rethrows as `IllegalArgumentException`/`ValueError` — message `Malformed built-in event of type '<type>'`, original cause chained. It does not degrade to a generic event. - Missing/empty `type`: unchanged — `fromJson`/`from_json` already raised before the registry lookup. - Unknown type: not a failure; returned unchanged. ### Tests | Contract | Java (`BuiltInEventsTest`) | Python (`test_built_in_events`) | |---|---|---| | Registry drift guard | `registryCoversEveryBuiltInEventTypeConstant` | `test_registry_covers_every_builtin_event_type_constant` | | Memory → shared base | (via category test) | `test_registry_maps_memory_types_to_shared_base` | | (1) Concrete type + typed nested values | `restoreReconstructsChatRequestWithTypedMessages`, `restoreReconstructsEveryBuiltInCategoryToItsConcreteType`, `restoreDispatchesMemorySubtypeToConcreteClass` | `test_restore_reconstructs_chat_request_with_typed_messages`, `test_restore_reconstructs_every_builtin_category`, `test_restore_dispatches_memory_subtype_to_concrete_class` | | (2) Preserves id/lineage/attachments | `restorePreservesLineageAndAttachments` | `test_restore_preserves_lineage_and_attachments` | | (3) Unknown unchanged | `restoreReturnsUnknownTypeUnchanged`, `fromJsonKeepsUserDefinedTypeGeneric` | `test_restore_returns_unknown_type_unchanged`, `test_from_json_keeps_user_defined_type_generic` | | (4) Idempotent | `restoreIsIdempotentForAlreadyTypedEvents` | `test_restore_is_idempotent_for_already_typed_events` | | Malformed → clear error | `restoreThrowsForMalformedMemoryEvent`, `restoreRejectsOutputEventCarryingAttachments` | `test_restore_raises_for_malformed_memory_event`, `test_restore_rejects_output_event_carrying_attachments` | | Boundary restores | `fromJsonRestoresBuiltInTypeAtTheBoundary` | `test_from_json_restores_builtin_type_at_the_boundary` | | `null` input | `restoreReturnsNullForNullInput` | n/a (`from_json` always builds an `Event`) | | (5) Cross-language media/URL sanitized | `FileEventLoggerTest.testCrossLanguageMediaPayloadsNeverReachTheLogAtStandard` / `…AtVerbose` | Java-side Event Log | **Coverage by risk.** The highest-risk path — a Python-shaped multimodal chat request crossing into Java, restored, then logged — is pinned end-to-end by the two new `FileEventLoggerTest` cases: they assert the base64 payload, the URL userinfo (`secret`), and the query (`X-Amz-Signature`) never appear at either level, while sanitized metadata (media type, `source.type=base64` without `data`, stripped URL) is retained. Registry/boundary contracts are pinned symmetrically in both languages. `CrossLanguageEventSnapshotTest` exercises `Event.fromJson` against real Python fixtures and passes unchanged. **Not verified.** - No Python-side Event Log media-sanitization test; sanitization is asserted only on the Java Event Log path, where the serializer lives. - `ModelRoutingEvent` restoration is covered only by the registry drift guard, not a dedicated round-trip test. - No full Flink-runtime test (operator → Python executor → Java) for this change; verification is at the `fromJson`/`from_json` unit boundary plus the Event Log. <details> <summary>Implementation invariants and local verification evidence (checker detail)</summary> - `restore` is a pure function of the event + registry; it holds no state and adds no mutation beyond each type's own `fromEvent`/`from_event`. - Java registry: 17 entries incl. `ModelRoutingEvent`; the 7 memory types → `MemoryEvent::fromEvent`. Python registry: 16 entries (`ModelRoutingEvent` absent). Java `restore(null)` returns `null` before lookup. - Reconstruction reuses the existing `@JsonCreator`/`fromEvent` paths, so nested `ChatMessage` blocks and `ChatResponseEvent` status validation are handled by those types, not the registry. - The Event Log sanitizer is registered via `addSerializer(ChatMessage.class, …)` on the loggers' own mapper, so it engages only for real `ChatMessage` instances — which is why restoring the concrete type matters for cross-language events. - Local runs: `api` 530 green; runtime `eventlog` green (`FileEventLoggerTest` 18, incl. 2 new); runtime condition/cross-language 112 green; Python `api` 391 green (2 pre-existing `watsonx` failures from a missing optional `ibm_watsonx_ai` dependency, unrelated); Python `runtime` 208 green; `spotless:check` + `ruff` clean. </details> ### API Yes. For a registered built-in type, `Event.fromJson(String)` / `Event.from_json(str)` now return the concrete subclass instead of a base `Event`. Callers reading only `getType()`/attributes are unaffected; `instanceof`/`isinstance` checks and casts now see the concrete type (strictly more information); actions calling `fromEvent`/`from_event` themselves keep working (idempotent). A malformed built-in event now raises `IllegalArgumentException`/`ValueError` at the boundary instead of returning a base `Event`. Made within the 0.4 breaking window. The only new type is the internal `BuiltInEvents` registry helper. ### Documentation - [ ] `doc-needed` - [x] `doc-not-needed` - [ ] `doc-included` ### Was this patch authored or co-authored using generative AI tooling? - [x] Yes - [ ] No Generated-by: Qoder 1.32.1 (Qwen3.8-Max) -- 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]
