Split flow DSL monolith into focused decorator modules#6040
Conversation
📝 WalkthroughWalkthroughEstablishes crewi.flow.dsl as the DSL entrypoint, moves start/listen/router/human_feedback into dsl submodules, implements condition primitives and combinators, refactors DSL utilities to use the new condition module, updates runtime imports, and adds a compatibility shim plus a test asserting exports. ChangesFlow DSL Package Restructuring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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/dsl/__init__.py (1)
1-8: ⚡ Quick winUpdate docstring to document
human_feedbackdecorator andHumanFeedbackResult.The module docstring describes
@start,@listen,@routerdecorators and theor_/and_condition combinators, but omits@human_feedbackandHumanFeedbackResulteven though both are exported in__all__(lines 34, 32). Users consulting the package docstring won't discover these public exports.📝 Suggested docstring enhancement
"""Flow DSL: the Python authoring layer for Flows. -Provides the ``@start`` / ``@listen`` / ``@router`` decorators and the -``or_`` / ``and_`` condition combinators used to write Flow classes in -Python. The DSL is one way to produce a Flow Structure: this package -extracts a :class:`~crewai.flow.flow_definition.FlowDefinition` from a -Python Flow class. Execution is handled by ``runtime``. +Provides the ``@start`` / ``@listen`` / ``@router`` / ``@human_feedback`` +decorators and the ``or_`` / ``and_`` condition combinators used to write +Flow classes in Python. The DSL is one way to produce a Flow Structure: +this package extracts a :class:`~crewai.flow.flow_definition.FlowDefinition` +from a Python Flow class. Execution is handled by ``runtime``. + +The ``HumanFeedbackResult`` dataclass captures the outcome of human-in-the-loop +feedback interactions. """🤖 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/dsl/__init__.py` around lines 1 - 8, The module docstring is missing documentation for the public exports `@human_feedback` and HumanFeedbackResult (they're listed in __all__), so update the top docstring in __init__.py to mention the human_feedback decorator and the HumanFeedbackResult type: briefly state that `@human_feedback` marks steps requiring human input/annotation and that HumanFeedbackResult represents the structured result returned by such steps (how it integrates with Flow execution and where to look for examples); keep the style consistent with the existing descriptions of `@start/`@listen/@router and the or_/and_ combinators.
🤖 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/dsl/__init__.py`:
- Around line 1-8: The module docstring is missing documentation for the public
exports `@human_feedback` and HumanFeedbackResult (they're listed in __all__), so
update the top docstring in __init__.py to mention the human_feedback decorator
and the HumanFeedbackResult type: briefly state that `@human_feedback` marks steps
requiring human input/annotation and that HumanFeedbackResult represents the
structured result returned by such steps (how it integrates with Flow execution
and where to look for examples); keep the style consistent with the existing
descriptions of `@start/`@listen/@router and the or_/and_ combinators.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 20a766ca-6a72-4a5e-8934-86575f3d5967
📒 Files selected for processing (9)
lib/crewai/src/crewai/flow/__init__.pylib/crewai/src/crewai/flow/dsl/__init__.pylib/crewai/src/crewai/flow/dsl/human_feedback.pylib/crewai/src/crewai/flow/dsl/listen.pylib/crewai/src/crewai/flow/dsl/router.pylib/crewai/src/crewai/flow/dsl/start.pylib/crewai/src/crewai/flow/dsl/utils.pylib/crewai/src/crewai/flow/human_feedback.pylib/crewai/tests/test_flow_definition.py
4c63f01 to
fa590b2
Compare
The Flow DSL lived in one 1033-line `dsl.py` that mixed every decorator (`@start`/`@listen`/`@router`), the `human_feedback` decorator, condition combinators, and FlowDefinition extraction helpers in a single file. Split it into a `dsl/` package where each decorator gets its own module (`start.py` 68 lines, `listen.py` 55, `router.py` 164, `human_feedback.py` 98) and the shared extraction/condition helpers stay in `utils.py`. The public API is re-exported from `dsl/__init__.py`, so import paths are unchanged. This is simpler because each decorator is now read and changed in isolation instead of scanning a 1000-line file to find one of them, and router-specific annotation parsing no longer sits next to unrelated start/listen logic.
fa590b2 to
366e7b9
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
lib/crewai/src/crewai/flow/dsl/_conditions.py (2)
182-184: 💤 Low valueSilent fallback may mask unexpected input types.
The fallback
return str(condition)at line 184 silently converts any unhandled type to a string. If an unexpected type reaches this branch (e.g., a malformed nested condition or wrong object type), it will produce potentially invalid output rather than failing fast.Consider raising a
ValueErrorfor truly unexpected types to surface bugs early.🛡️ Suggested defensive handling
if isinstance(condition, list): return {"or": [_definition_condition_from_runtime(item) for item in condition]} - return str(condition) + raise ValueError(f"Unexpected condition type: {type(condition).__name__}")🤖 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/dsl/_conditions.py` around lines 182 - 184, The current fallback in _definition_condition_from_runtime silently converts unexpected types to strings (return str(condition)), which can mask bugs; instead, validate the input type and raise a ValueError for truly unexpected types. Update _definition_condition_from_runtime to only accept the expected types (e.g., dict, list, str — match the function's handled branches) and replace the final return str(condition) with raising ValueError(f"Unsupported condition type: {type(condition)!r}, value={condition!r}") so callers fail fast and errors include the offending value and type.
38-67: 💤 Low valueType guard allows incomplete condition dicts.
is_flow_condition_dictvalidates"conditions"and"methods"keys independently but doesn't require at least one of them. A dict like{"type": "AND"}passes validation, which will cause the downstream consumer in_set_trigger_metadata(context snippet 1) to raiseValueError("Condition dict must contain 'conditions' or 'methods'").While the consumer handles this defensively, stricter validation at the type-guard level would catch invalid shapes earlier and provide clearer semantics.
🛡️ Suggested stricter validation
allowed_keys = {"type", "conditions", "methods"} if not set(obj).issubset(allowed_keys): return False + # Require at least one of "conditions" or "methods" + if "conditions" not in obj and "methods" not in obj: + return False + return True🤖 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/dsl/_conditions.py` around lines 38 - 67, is_flow_condition_dict currently allows a dict like {"type":"AND"} because it validates "conditions" and "methods" independently; update is_flow_condition_dict to require that at least one of "conditions" or "methods" is present and valid (i.e., after the existing validation for "conditions" being a list of str/dict and "methods" being a list of str, add a final check that obj contains "conditions" or "methods" and that the corresponding validated value is present/non-empty), so the function rejects incomplete condition dicts and matches the expectation of the downstream _set_trigger_metadata consumer.
🤖 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/dsl/_conditions.py`:
- Around line 182-184: The current fallback in
_definition_condition_from_runtime silently converts unexpected types to strings
(return str(condition)), which can mask bugs; instead, validate the input type
and raise a ValueError for truly unexpected types. Update
_definition_condition_from_runtime to only accept the expected types (e.g.,
dict, list, str — match the function's handled branches) and replace the final
return str(condition) with raising ValueError(f"Unsupported condition type:
{type(condition)!r}, value={condition!r}") so callers fail fast and errors
include the offending value and type.
- Around line 38-67: is_flow_condition_dict currently allows a dict like
{"type":"AND"} because it validates "conditions" and "methods" independently;
update is_flow_condition_dict to require that at least one of "conditions" or
"methods" is present and valid (i.e., after the existing validation for
"conditions" being a list of str/dict and "methods" being a list of str, add a
final check that obj contains "conditions" or "methods" and that the
corresponding validated value is present/non-empty), so the function rejects
incomplete condition dicts and matches the expectation of the downstream
_set_trigger_metadata consumer.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 17fa3a8f-0ec2-4b75-950f-dd2e6e0c2a1a
📒 Files selected for processing (11)
lib/crewai/src/crewai/flow/__init__.pylib/crewai/src/crewai/flow/dsl/__init__.pylib/crewai/src/crewai/flow/dsl/_conditions.pylib/crewai/src/crewai/flow/dsl/_human_feedback.pylib/crewai/src/crewai/flow/dsl/_listen.pylib/crewai/src/crewai/flow/dsl/_router.pylib/crewai/src/crewai/flow/dsl/_start.pylib/crewai/src/crewai/flow/dsl/_utils.pylib/crewai/src/crewai/flow/human_feedback.pylib/crewai/src/crewai/flow/runtime.pylib/crewai/tests/test_flow_definition.py
🚧 Files skipped from review as they are similar to previous changes (8)
- lib/crewai/tests/test_flow_definition.py
- lib/crewai/src/crewai/flow/dsl/_listen.py
- lib/crewai/src/crewai/flow/dsl/init.py
- lib/crewai/src/crewai/flow/dsl/_router.py
- lib/crewai/src/crewai/flow/init.py
- lib/crewai/src/crewai/flow/dsl/_human_feedback.py
- lib/crewai/src/crewai/flow/human_feedback.py
- lib/crewai/src/crewai/flow/dsl/_utils.py
The Flow DSL lived in one 1033-line
dsl.pythat mixed every decorator (@start/@listen/@router), thehuman_feedbackdecorator, condition combinators, and FlowDefinition extraction helpers in a single file.Split it into a
dsl/package where each decorator gets its own module (start.py68 lines,listen.py55,router.py164,human_feedback.py98) and the shared extraction/condition helpers stay inutils.py. The public API is re-exported fromdsl/__init__.py, so import paths are unchanged.This is simpler because each decorator is now read and changed in isolation instead of scanning a 1000-line file to find one of them, and router-specific annotation parsing no longer sits next to unrelated start/listen logic.
Note
Medium Risk
Large structural move across Flow authoring, runtime wiring, and HITL decorator layering; behavior is intended to stay compatible via re-exports, but regressions in metadata stamping or imports would affect flow execution and visualization.
Overview
Replaces the monolithic Flow DSL module with a
crewai.flow.dslpackage:_conditionsholdsor_/and_and condition normalization,_start,_listen, and_routereach own their decorator, and_utilskeeps FlowDefinition extraction and shared metadata helpers.human_feedbackis split sohuman_feedback.pyexposes_build_human_feedback_runtime_decorator(execution only) whiledsl._human_feedbackapplies the public decorator and stamps router/definition metadata.crewai.flow.human_feedback.human_feedbackremains a thin forwarder to the DSL implementation, andcrewai.flownow importsHumanFeedbackResultandhuman_feedbackfromcrewai.flow.dsl.runtime.pyimports condition helpers fromdsl._conditionsand definition builders fromdsl._utils. DSL__all__and tests are updated to includehuman_feedbackandHumanFeedbackResultalongside the existing decorators.Reviewed by Cursor Bugbot for commit 366e7b9. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Refactor
Tests