rosemarYuan commented on PR #756:
URL: https://github.com/apache/flink-agents/pull/756#issuecomment-4687049436
**YAML `trigger_conditions`: should CEL also accept YAML event aliases?**
I have a local implementation for the YAML API path, but there is one syntax
question worth aligning on. When the condition-expression support was
originally designed, the YAML API path had not been added yet, so the
interaction between YAML aliases and CEL expressions was not considered at that
time.
Today the YAML loader already supports short aliases for built-in event
types. For example:
```yaml
trigger_conditions:
- input
```
is resolved by the YAML layer to the built-in event type string before it
reaches `plan.Action`.
For CEL conditions, the current Java/Python condition implementation uses
the `EventType.X` namespace instead:
```yaml
trigger_conditions:
- >-
type == EventType.ChatResponseEvent
&& priority == 'high'
```
This means a mixed list may contain two notations for built-in event types:
```yaml
actions:
- name: handle_priority_input
type: java
function: com.example.MyActions:handlePriorityInput
trigger_conditions:
- input
- >-
type == EventType.ChatResponseEvent
&& priority == 'high'
```
So `input` and `EventType.InputEvent` refer to the same conceptual event
type, but they appear in different syntactic forms.
I also considered allowing CEL expressions to use YAML short aliases
directly. However, I do not think we should add this. Alias names such as
`input`, `output`, and `chat_response` are common words that users may also use
as event attribute names. If we inject them as top-level CEL variables, they
could conflict with user-defined fields and make the expression semantics less
clear.
So my preference is to keep the boundary simple:
* short aliases are only supported by the YAML loader for single-token type
matches;
* CEL expressions should continue to use `EventType.X` for built-in event
types.
Does this trade-off look acceptable for the YAML API, or do we want a
different shape before finalizing it? The main alternatives I see are either
standardizing YAML entirely on `EventType.X` as well, which would be a breaking
change for existing YAMLs, or splitting plain type matches and CEL filters into
separate YAML fields, although that would lose the unified per-condition list
shape discussed earlier.
Concrete feedback welcome — particularly from anyone who has written
non-trivial YAML agents and has an opinion on which form they reach for first.
/cc @wenjin272
--
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]
