rosemarYuan commented on code in PR #756:
URL: https://github.com/apache/flink-agents/pull/756#discussion_r3355730317
##########
plan/src/main/java/org/apache/flink/agents/plan/actions/Action.java:
##########
@@ -33,36 +33,36 @@
import java.util.Objects;
/**
- * Representation of an agent action with event listening and function
execution.
+ * Representation of an agent action with unified trigger conditions.
*
- * <p>This class encapsulates a named agent action that listens for specific
event types and
- * executes an associated function when those events occur.
+ * <p>Each entry of {@code triggerConditions} is an event type name string.
Multiple entries combine
+ * with OR.
*/
@JsonSerialize(using = ActionJsonSerializer.class)
@JsonDeserialize(using = ActionJsonDeserializer.class)
public class Action {
private final String name;
private final Function exec;
- private final List<String> listenEventTypes;
+ private final List<String> triggerConditions;
Review Comment:
Thanks for the comment — this was actually my first instinct too: a clean
routing-vs-filtering separation. The more I worked through the problem though,
the more I came to think it doesn't quite map onto what users actually want to
express.
Event filtering is just event routing at a finer granularity, not a separate
concern. Each entry in `trigger_conditions` answers exactly one question —
"should this action fire for this event?" — and a bare event type is simply the
most common (and most degenerate) form of that condition.
The case that really pins this down: for a single action, users often want
per-event-type conditions, with different conditions for different types — and
sometimes multiple conditions on the same type. With a unified list this falls
out naturally. [#726](https://github.com/apache/flink-agents/issues/726) shows
the canonical shape:
```
// Mixed triggers. Entries are OR'ed.
@Action({
EventType.InputEvent,
// direct type
"type == EventType.ChatResponseEvent && retryCount > 0",
// metadata condition
"type == EventType.ToolResponseEvent && response.success == false",
// payload condition
"type == EventType.ChatResponseEvent &&
response.plantype.contains('Day')" // metadata + payload
})
```
Split into` listen_event_types + trigger_conditions`, there's no clean way
to express any of this. The two fields are parallel lists with no shared index,
so "this condition belongs to that type" has no representation — and
"ChatResponseEvent has two distinct conditions" is even less expressible. You'd
end up either forcing one CEL expression to gate all listened types (loses
per-type granularity), or inventing a multi-map like `{ChatResponseEvent:
[cond1, cond2], ToolResponseEvent: [cond3]}` — which is just the unified list
with extra ceremony, plus an awkward way to represent the "no condition" case
for direct-type entries. The unified field is what naturally fits the actual
shape of user intent.
Curious if you have a concrete scenario in mind where the two-field split
would be cleaner — happy to revisit if there's a use case I'm missing.
--
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]
