fix(flow): report the real CEL error for failures inside map literals - #6793
Conversation
📝 WalkthroughWalkthroughThe CEL evaluator detects nested ChangesCEL error propagation
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/crewai/src/crewai/flow/expressions.py (1)
24-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the recursive error scan.
_find_cel_eval_errorrecursively inspects dictionary keys and values, lists, and tuples. Add a docstring that states it returns the first nestedCELEvalError.As per coding guidelines, “Document public APIs and complex logic in Python code.”
Proposed documentation
def _find_cel_eval_error(value: Any) -> Exception | None: + """Return the first CELEvalError found in nested CEL collections.""" from celpy.evaluation import CELEvalError🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/src/crewai/flow/expressions.py` around lines 24 - 40, Add a concise docstring to _find_cel_eval_error documenting that it recursively scans dictionary keys and values, lists, and tuples, returning the first nested CELEvalError found or None when absent.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@lib/crewai/src/crewai/flow/expressions.py`:
- Around line 24-40: Add a concise docstring to _find_cel_eval_error documenting
that it recursively scans dictionary keys and values, lists, and tuples,
returning the first nested CELEvalError found or None when absent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 36b1b5ec-5cbd-4759-a308-008bf440db59
📒 Files selected for processing (2)
lib/crewai/src/crewai/flow/expressions.pylib/crewai/tests/test_flow_from_definition.py
fd58603 to
167911f
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/crewai/src/crewai/flow/expressions.py (1)
24-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the recursive error search.
This helper contains non-obvious recursive logic. Add a docstring that defines the first-error behavior, the supported container traversal, and why the search runs before JSON serialization.
Suggested documentation
def _find_cel_eval_error(value: Any) -> Exception | None: + """Find the first nested CELEvalError before JSON serialization. + + Search mapping keys and values and list or tuple elements. + """As per coding guidelines, document complex logic in Python code.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/src/crewai/flow/expressions.py` around lines 24 - 40, Add a docstring to _find_cel_eval_error describing that it returns the first CELEvalError found, recursively traverses dictionary keys and values plus list and tuple elements, and must run before JSON serialization so nested evaluation errors remain identifiable.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@lib/crewai/src/crewai/flow/expressions.py`:
- Around line 24-40: Add a docstring to _find_cel_eval_error describing that it
returns the first CELEvalError found, recursively traverses dictionary keys and
values plus list and tuple elements, and must run before JSON serialization so
nested evaluation errors remain identifiable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c4e92070-9e41-4640-8708-a181a8382df2
📒 Files selected for processing (2)
lib/crewai/src/crewai/flow/expressions.pylib/crewai/tests/test_flow_from_definition.py
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/crewai/tests/test_flow_from_definition.py
What
Flow CEL expressions that fail inside a map literal now report the actual error.
Before, a bad value in a map reported an internal serialization error:
→ failed to evaluate CEL expression:
Object of type CELEvalError is not JSON serializableNow it reports the cause, matching what the same expression reports on its own:
→ failed to evaluate CEL expression: ('modulus or divide by zero', ...)
This affects any failure nested in a map — division by zero, a missing
statefield, a bad conversion — which previously gave no indication of what was wrong.Why
celpyraises evaluation errors that land at the top level of a result, but a map literal keeps the failed value in place and returns it. List literals propagate the error; maps don't. That returned error object reachedjson.dumps, which turned a clear CEL failure into an opaque serialization message.Note
Low Risk
Localized change to CEL error handling in
expressions.pywith focused tests; no auth, persistence, or API surface changes.Overview
Flow CEL evaluation now detects
CELEvalErrorobjects nested inside map (and list) results fromcelpyand re-raises them before JSON serialization, so failures like divide-by-zero or missingstatefields surface asExpressionErrorwith the underlying cause instead of an opaque “not JSON serializable” message.A recursive
_find_cel_eval_errorhelper walks dict keys/values and list/tuple elements afterprogram.evaluate(). Short-circuited failures (&&,||,exists) that CEL never evaluates are unchanged and still succeed.Tests add coverage for map/nested/list error shapes, message content for nested divide-by-zero, and guards that intentional short-circuiting still evaluates.
Reviewed by Cursor Bugbot for commit 167911f. Bugbot is set up for automated code reviews on this repo. Configure here.