Ashfaqbs commented on PR #1342: URL: https://github.com/apache/flink-agents/pull/1342#issuecomment-6610802475
Thanks for the detailed review. - Fixed the 5 failing `ActionExecutionOperatorTest` fixtures: `seedActionState`/`getStoredActionState` and the direct lookup in `testUnsupportedObservationValueIsSkippedWithoutChangingActionSuccess` now assign the same root occurrence identity the operator itself computes, instead of leaving it unset. All green locally (`ActionExecutionOperatorTest`, `ActionStateUtilTest`, `KafkaActionStateStoreTest`, `FlussActionStateStoreTest`, `ActionStateKeyEncoderTest`, `KafkaActionStateStoreRecoveryTest`, `ActionStateDivergenceRecoveryTest`). - Moved identity generation into a shared path independent of durable storage: the operator now derives a root event's business-key identity directly from its own keyed-state serializer (a new `ActionStateKeyEncoder` field it owns), instead of delegating through `DurableExecutionManager`/`ActionStateStore` — so this is no longer a no-op when durable execution is disabled. An action execution's identity is now `hash(triggeringEventOccurrenceId, actionIdentity)` via a new `ActionStateUtil.deriveActionExecutionId`, needing neither a key encoder nor a store. Removed `generateBusinessKeyIdentity`/`generateActionExecutionId` from the `ActionStateStore` interface and the Kafka/Fluss implementations. - Cleaned up the `FLINK-AGENTS-1084`/"before this fix" references from the production-code comments this patch touches. - One partial item: I kept the attributes-only fallback in `ActionStateUtil.generateUUIDForEvent` rather than making a missing occurrence identity throw. Removing it outright breaks a large pre-existing lower-level test surface (`ActionStateUtilTest`, `KafkaActionStateStoreTest`, `FlussActionStateStoreTest`, `ActionStateKeyEncoderTest`) that predates occurrence identity and constructs events without assigning one — happy to do that migration as a follow-up if you'd rather not carry the fallback at all. - Left the chat/internal-subagent identity fix (#1220) for a follow-up PR as you suggested — this PR now has the shared component in place for it to build on. Pushed as bc042eb6..8a5d3f5e. -- 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]
