Report bounded planning step outcomes#6618
Conversation
📝 WalkthroughWalkthroughStep execution now reports typed outcomes and termination reasons, preserves partial results for bounded termination, and propagates this metadata through todo state, planning events, audit logs, lite output, and observer handling. ChangesStructured step termination metadata
Sequence Diagram(s)sequenceDiagram
participant AgentExecutor
participant StepExecutor
participant TodoItem
participant PlanStepCompletedEvent
AgentExecutor->>StepExecutor: Execute todo step
StepExecutor-->>AgentExecutor: Return outcome, partial result, termination reason
AgentExecutor->>TodoItem: Store execution metadata
AgentExecutor->>PlanStepCompletedEvent: Emit completion metadata
Suggested reviewers: 🚥 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 196b2a4. Configure here.
| termination_reason=termination.reason, | ||
| tool_calls_made=tool_calls_made, | ||
| execution_time=elapsed, | ||
| ) |
There was a problem hiding this comment.
Bounded exit skips tool validation
Medium Severity
When a step stops for step_timeout or max_step_iterations, _StepExecutionTerminatedError is handled before _validate_expected_tool_usage runs. A step that never invoked a required tool_to_use can be reported as timed_out or iteration_exhausted instead of failed with the expected-tool error.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 196b2a4. Configure here.
| ) | ||
| final_reason = ( | ||
| termination_reason or error or (todo.termination_reason if todo else None) | ||
| ) |
There was a problem hiding this comment.
Observer error replaces execution reason
Low Severity
In _mark_todo_failed, final_reason prefers the observation error argument over an existing todo.termination_reason from step execution. After a bounded failure (for example iteration_exhausted), marking failed with a replan message drops the executor’s termination reason on the todo and emitted events.
Reviewed by Cursor Bugbot for commit 196b2a4. Configure here.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
lib/crewai/src/crewai/utilities/planning_types.py (1)
12-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the public
StepOutcomecontract.It is consumed by the executor and lifecycle-event APIs, so document the meaning of each terminal value.
Proposed change
+# Terminal outcomes for plan-step execution: +# completed, timed_out, iteration_exhausted, and failed. StepOutcome = Literal["completed", "timed_out", "iteration_exhausted", "failed"]As per coding guidelines, “Document public APIs and 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/utilities/planning_types.py` at line 12, Document the public StepOutcome type alias by adding concise Python documentation that defines the meaning of each terminal value: completed, timed_out, iteration_exhausted, and failed. Keep the existing Literal values and contract unchanged.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.
Inline comments:
In `@lib/crewai/src/crewai/experimental/agent_executor.py`:
- Around line 1291-1298: Update the parallel-execution exception branch
alongside _mark_todo_failed to write the step_execution audit entry, using the
same outcome metadata as the normal-result branch, including the failure outcome
and termination reason. Ensure exceptions caught by gather() are recorded before
emitting the completion event.
- Around line 469-502: Update _mark_todo_failed so the existing
todo.termination_reason takes precedence over the observer-provided error when
determining final_reason, preserving the executor’s structured timeout or
iteration-exhaustion reason while retaining error as a fallback when no todo
reason exists.
- Around line 608-623: Update the failed-step handling in the observer path of
the execution method containing _step_success_from_log and
_ensure_planner_observer so a false step_success forces both
step_completed_successfully and goal_already_achieved to false before returning
the observation. Add a high-effort regression test where the observer reports
both fields as true for a failed execution, asserting the bounded failed step is
not marked complete.
---
Nitpick comments:
In `@lib/crewai/src/crewai/utilities/planning_types.py`:
- Line 12: Document the public StepOutcome type alias by adding concise Python
documentation that defines the meaning of each terminal value: completed,
timed_out, iteration_exhausted, and failed. Keep the existing Literal values and
contract unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cea16605-3310-4bbb-91e6-ca04fd08871b
📒 Files selected for processing (7)
lib/crewai/src/crewai/agents/step_executor.pylib/crewai/src/crewai/events/types/observation_events.pylib/crewai/src/crewai/experimental/agent_executor.pylib/crewai/src/crewai/lite_agent_output.pylib/crewai/src/crewai/utilities/planning_types.pylib/crewai/src/crewai/utilities/step_execution_context.pylib/crewai/tests/agents/test_agent_executor.py
| def _mark_todo_failed( | ||
| self, | ||
| step_number: int, | ||
| result: str | None = None, | ||
| error: str | None = None, | ||
| outcome: StepOutcome | None = None, | ||
| termination_reason: str | None = None, | ||
| ) -> None: | ||
| todo = self.state.todos.get_by_step_number(step_number) | ||
| previous_status = todo.status if todo else None | ||
| self.state.todos.mark_failed(step_number, result=result) | ||
| final_outcome = outcome or ( | ||
| todo.outcome | ||
| if todo and todo.outcome not in (None, "completed") | ||
| else "failed" | ||
| ) | ||
| final_reason = ( | ||
| termination_reason or error or (todo.termination_reason if todo else None) | ||
| ) | ||
| self.state.todos.mark_failed( | ||
| step_number, | ||
| result=result, | ||
| outcome=final_outcome, | ||
| termination_reason=final_reason, | ||
| ) | ||
| todo = self.state.todos.get_by_step_number(step_number) | ||
| if todo and previous_status != "failed": | ||
| self._emit_plan_step_completed( | ||
| todo, | ||
| success=False, | ||
| outcome=final_outcome, | ||
| termination_reason=final_reason, | ||
| result=result, | ||
| error=error, | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Preserve the executor termination reason.
At Line 485, an observer-provided error overrides todo.termination_reason. A timed-out or iteration-exhausted step that triggers replanning will therefore emit the planner’s replan message instead of its actual structured termination reason.
Proposed fix
final_reason = (
- termination_reason or error or (todo.termination_reason if todo else None)
+ termination_reason
+ or (todo.termination_reason if todo else None)
+ or error
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _mark_todo_failed( | |
| self, | |
| step_number: int, | |
| result: str | None = None, | |
| error: str | None = None, | |
| outcome: StepOutcome | None = None, | |
| termination_reason: str | None = None, | |
| ) -> None: | |
| todo = self.state.todos.get_by_step_number(step_number) | |
| previous_status = todo.status if todo else None | |
| self.state.todos.mark_failed(step_number, result=result) | |
| final_outcome = outcome or ( | |
| todo.outcome | |
| if todo and todo.outcome not in (None, "completed") | |
| else "failed" | |
| ) | |
| final_reason = ( | |
| termination_reason or error or (todo.termination_reason if todo else None) | |
| ) | |
| self.state.todos.mark_failed( | |
| step_number, | |
| result=result, | |
| outcome=final_outcome, | |
| termination_reason=final_reason, | |
| ) | |
| todo = self.state.todos.get_by_step_number(step_number) | |
| if todo and previous_status != "failed": | |
| self._emit_plan_step_completed( | |
| todo, | |
| success=False, | |
| outcome=final_outcome, | |
| termination_reason=final_reason, | |
| result=result, | |
| error=error, | |
| ) | |
| def _mark_todo_failed( | |
| self, | |
| step_number: int, | |
| result: str | None = None, | |
| error: str | None = None, | |
| outcome: StepOutcome | None = None, | |
| termination_reason: str | None = None, | |
| ) -> None: | |
| todo = self.state.todos.get_by_step_number(step_number) | |
| previous_status = todo.status if todo else None | |
| final_outcome = outcome or ( | |
| todo.outcome | |
| if todo and todo.outcome not in (None, "completed") | |
| else "failed" | |
| ) | |
| final_reason = ( | |
| termination_reason | |
| or (todo.termination_reason if todo else None) | |
| or error | |
| ) | |
| self.state.todos.mark_failed( | |
| step_number, | |
| result=result, | |
| outcome=final_outcome, | |
| termination_reason=final_reason, | |
| ) | |
| todo = self.state.todos.get_by_step_number(step_number) | |
| if todo and previous_status != "failed": | |
| self._emit_plan_step_completed( | |
| todo, | |
| success=False, | |
| outcome=final_outcome, | |
| termination_reason=final_reason, | |
| result=result, | |
| error=error, | |
| ) |
🤖 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/experimental/agent_executor.py` around lines 469 - 502,
Update _mark_todo_failed so the existing todo.termination_reason takes
precedence over the observer-provided error when determining final_reason,
preserving the executor’s structured timeout or iteration-exhaustion reason
while retaining error as a fallback when no todo reason exists.
| if step_success is None: | ||
| step_success = self._step_success_from_log(completed_step.step_number) | ||
|
|
||
| if self._should_observe_steps(): | ||
| observer = self._ensure_planner_observer() | ||
| return observer.observe( | ||
| observation = observer.observe( | ||
| completed_step=completed_step, | ||
| result=result, | ||
| all_completed=all_completed, | ||
| remaining_todos=remaining_todos, | ||
| ) | ||
| if step_success is False and observation.step_completed_successfully: | ||
| return observation.model_copy( | ||
| update={"step_completed_successfully": False} | ||
| ) | ||
| return observation |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Prevent optimistic goal completion after an execution failure.
Line 620 only clears step_completed_successfully. In the high-effort route, goal_already_achieved is handled before the failed-step branch, so an observer returning both fields as true still causes _mark_todo_completed() for a bounded failed step.
Proposed fix
if step_success is False and observation.step_completed_successfully:
return observation.model_copy(
- update={"step_completed_successfully": False}
+ update={
+ "step_completed_successfully": False,
+ "goal_already_achieved": False,
+ }
)Add a high-effort regression where the observer reports both success and early goal completion for a failed execution.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if step_success is None: | |
| step_success = self._step_success_from_log(completed_step.step_number) | |
| if self._should_observe_steps(): | |
| observer = self._ensure_planner_observer() | |
| return observer.observe( | |
| observation = observer.observe( | |
| completed_step=completed_step, | |
| result=result, | |
| all_completed=all_completed, | |
| remaining_todos=remaining_todos, | |
| ) | |
| if step_success is False and observation.step_completed_successfully: | |
| return observation.model_copy( | |
| update={"step_completed_successfully": False} | |
| ) | |
| return observation | |
| if step_success is None: | |
| step_success = self._step_success_from_log(completed_step.step_number) | |
| if self._should_observe_steps(): | |
| observer = self._ensure_planner_observer() | |
| observation = observer.observe( | |
| completed_step=completed_step, | |
| result=result, | |
| all_completed=all_completed, | |
| remaining_todos=remaining_todos, | |
| ) | |
| if step_success is False and observation.step_completed_successfully: | |
| return observation.model_copy( | |
| update={ | |
| "step_completed_successfully": False, | |
| "goal_already_achieved": False, | |
| } | |
| ) | |
| return observation |
🤖 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/experimental/agent_executor.py` around lines 608 - 623,
Update the failed-step handling in the observer path of the execution method
containing _step_success_from_log and _ensure_planner_observer so a false
step_success forces both step_completed_successfully and goal_already_achieved
to false before returning the observation. Add a high-effort regression test
where the observer reports both fields as true for a failed execution, asserting
the bounded failed step is not marked complete.
| todo.outcome = "failed" | ||
| todo.termination_reason = error_msg | ||
| self._mark_todo_failed( | ||
| todo.step_number, | ||
| result=error_msg, | ||
| error=error_msg, | ||
| outcome="failed", | ||
| termination_reason=error_msg, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Log parallel execution exceptions with the same outcome metadata.
This branch updates the todo and emits a completion event, but it omits the step_execution audit entry that the normal-result branch writes. Failures caught by gather() therefore disappear from the execution log despite having an outcome and termination reason.
🤖 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/experimental/agent_executor.py` around lines 1291 -
1298, Update the parallel-execution exception branch alongside _mark_todo_failed
to write the step_execution audit entry, using the same outcome metadata as the
normal-result branch, including the failure outcome and termination reason.
Ensure exceptions caught by gather() are recorded before emitting the completion
event.
1 participant
Summary
completed,timed_out,iteration_exhausted, andfailedoutcomesLiteAgentOutputThis addresses checklist item 1 in OSS-98.
Why
When a planned step reached
step_timeoutormax_step_iterations,StepExecutorreturned a plain result string. Its caller then unconditionally constructed a successfulStepResult, so partial work was reported as completed and could bypass failure/replanning behavior.Real-call before/after
I ran the same
Agent().kickoff()harness against the base commit and this branch usinggpt-4o-miniplus a deterministic local lookup tool. The planner was fixed to one step so the comparison isolates this checklist item; execution and final synthesis were real model calls.17\n2517\n25Step reached max_step_iterations (1) before producing a final answer.The after result intentionally keeps the useful partial tool output and correct final synthesis while making the bounded execution state truthful.
Validation
uv run ruff format --check ...uv run ruff check ...uv run pytest -q lib/crewai/tests/agents/test_agent_executor.py lib/crewai/tests/agents/test_agent_reasoning.py lib/crewai/tests/agents/test_lite_agent.pyAgent().kickoff()probe described aboveNote
Medium Risk
Changes core Plan-and-Execute step result semantics and observation gating; backward-compatible via StepResult.success/error but downstream code relying on implicit success on partial strings may behave differently (more failures/replanning).
Overview
Planning step execution now uses an explicit
StepOutcome(completed,timed_out,iteration_exhausted,failed) instead of treating any returned string as success.StepExecutorraises_StepExecutionTerminatedErrorwhenstep_timeoutormax_step_iterationsis hit, returning partial tool output with atermination_reasonrather than silently succeeding.StepResultis centered onoutcome;success/errorremain as compatibility properties.AgentExecutorcopies outcome metadata onto todos and execution logs, emitsoutcome/termination_reasononPlanStepCompletedEvent, and clamps LLM observations so a failed execution cannot be marked successful.LiteAgentOutputandTodoItemexpose the same fields for callers.Reviewed by Cursor Bugbot for commit 196b2a4. Bugbot is set up for automated code reviews on this repo. Configure here.