fix(engine,condition): stop starving a downstream node after a single-branch condition#5
Conversation
…ition B15: a condition node wired with only a `true` edge (no `false` edge — the common gate/filter shape) was lowered as an UNCONDITIONAL plain edge in build_graph, so the successor ran on every execution regardless of which port the condition actually emitted. collect_input then port-matched the edge's `true` against the emitted `false` and silently dropped the items, so the successor ran with an EMPTY input and downstream `=item`/`=item.<field>` expressions resolved to null instead of the run simply ending. Fix: when a node's single outgoing edge has from_port != "main", lower it as a conditional edge (mirroring the existing multi-edge branch) whose router returns from_port when the emitted port matches, and END otherwise. A `true`-only condition now behaves as a FILTER: it runs the successor with items on `true`, and terminates the run to END on `false`. Also fixes condition.rs: `field` was read as a raw literal key, so an `=`-expression field (e.g. `field: "=item.assignee"`) was looked up as a literal key named "=item.assignee" and always missed, always routing `false`. Now an expression field is resolved against the node's expression scope first and the resolved value's truthiness is checked directly; non-expression fields keep the existing literal-key-lookup behavior. Tests: condition_single_true_edge_filters_on_false, condition_single_true_edge_item_flows_through (engine.rs), expression_in_field_resolves_and_checks_truthiness (condition.rs). All existing condition/engine tests remain green.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughGraph lowering in engine.rs now emits conditional edges (instead of unconditional edges) for nodes with a single outgoing edge on a non-"main" port, routing based on the emitted port or terminating at END. ConditionNode's field evaluation now supports ChangesCondition Node Filtering Fix
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Engine
participant ConditionNode
participant Successor
Engine->>ConditionNode: execute(item)
ConditionNode->>ConditionNode: resolve field (expression or key lookup)
ConditionNode-->>Engine: emitted port ("true"/"false")
Engine->>Engine: compare emitted port to edge.from_port
alt port matches edge
Engine->>Successor: run with item
else port does not match
Engine->>Engine: terminate to END
end
Poem
✨ Finishing Touches📝 Generate docstrings
Comment |
Bug: a single-branch
conditionsilently starves its downstream nodeTwo coupled bugs made a downstream node (e.g. an agent with
input_context:"=item") receive null when its predecessor was acondition:A1 — engine drops items on a single-branch condition (
engine.rs,HandlerRouting::Plainsingle-edge branch). A condition with only atrueedge was lowered as an unconditionaladd_edge(ignoringfrom_port), so the successor ran on every execution — butcollect_inputport-matchestruevs the emittedfalseand drops the items, so the successor ran with empty input. Fix: when the single outgoing edge'sfrom_port != "main", lower it as a conditional edge with anENDfallback — atrue-only condition now acts as a filter (runs with items on true, terminates to END on false).A2 — condition node didn't resolve
=-expressions (nodes/control_flow/condition.rs).fieldwas read as a raw literal key, sofield:"=item.assignee"didget("=item.assignee")→ alwaysNone→ alwaysfalse. Fix: iffieldis_expression(), resolve it and check the resolved value's truthiness; non-expression fields keep the literal-key path (backward compatible).Tests
condition_single_true_edge_filters_on_false,condition_single_true_edge_item_flows_through,expression_in_field_resolves_and_checks_truthiness. Full suite green (cargo test304 lib + 12 doctests,--all-featurese2e all pass).Summary by CodeRabbit
New Features
Bug Fixes
nullinputs from appearing in downstream branches when a condition is not met.Tests