fix: don't drop an empty final task output from crew result - #6783
Open
NishchayMahor wants to merge 1 commit into
Open
fix: don't drop an empty final task output from crew result#6783NishchayMahor wants to merge 1 commit into
NishchayMahor wants to merge 1 commit into
Conversation
_create_crew_output filtered task outputs on truthiness (if t.raw), which was meant to skip empty skipped-conditional outputs (crewAIInc#1937) but also dropped a task that legitimately produced an empty string. The crew then returned an earlier task's output, or crashed with 'No valid task outputs' when the only task returned empty. Add an explicit TaskOutput.skipped marker (set by get_skipped_task_output) and filter on that instead, so skipped conditional tasks are still excluded while genuinely-empty task outputs are preserved.
📝 WalkthroughWalkthroughChangesCrew output selection
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/crewai/src/crewai/tasks/task_output.py (1)
47-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the new public field.
Add
skippedto theTaskOutput.Attributesdocumentation. This field controls crew output selection and is part of the public Pydantic model.Proposed documentation update
output_format: Output format of the task (JSON, PYDANTIC, or RAW) + skipped: Whether the task was skipped rather than executedAs 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/tasks/task_output.py` around lines 47 - 51, Document the public skipped field in TaskOutput.Attributes alongside the existing model attributes, describing that it indicates a task was skipped and controls crew output selection. Keep the documentation aligned with the field’s existing default and semantics.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/crew.py`:
- Around line 1913-1916: Preserve the TaskOutput.skipped flag through the
_store_execution_log persistence path and replay() reconstruction: include
skipped when storing each task output and restore it when creating TaskOutput
instances during replay. Ensure skipped conditional outputs remain excluded by
the valid_outputs filter after replay.
---
Nitpick comments:
In `@lib/crewai/src/crewai/tasks/task_output.py`:
- Around line 47-51: Document the public skipped field in TaskOutput.Attributes
alongside the existing model attributes, describing that it indicates a task was
skipped and controls crew output selection. Keep the documentation aligned with
the field’s existing default and semantics.
🪄 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: 8874e79b-04a9-477f-be1a-b81d69cdefae
📒 Files selected for processing (4)
lib/crewai/src/crewai/crew.pylib/crewai/src/crewai/tasks/conditional_task.pylib/crewai/src/crewai/tasks/task_output.pylib/crewai/tests/test_crew.py
Comment on lines
+1913
to
+1916
| # Exclude skipped tasks (e.g. conditional tasks whose condition was not | ||
| # met) rather than filtering on truthiness, so a task that legitimately | ||
| # produced an empty output is still selected as the final output. | ||
| valid_outputs = [t for t in task_outputs if not t.skipped] |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Confirm that skipped outputs reach persistence and that replay restores the marker.
rg -n -C 8 \
'check_conditional_skip|_store_execution_log|get_skipped_task_output|stored_output|skipped' \
lib/crewai/src/crewai lib/crewai/testsRepository: crewAIInc/crewAI
Length of output: 50372
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Inspect the relevant data-model and path implementations, plus conditional skip behavior,
# without executing repository code.
sed -n '1,220p' lib/crewai/src/crewai/tasks/agent_output.py
printf '\n--- conditional_skip source ---\n'
fd -a 'utils.py' lib/crewai/src/crewai/crews -x sed -n '1,180p' {}
printf '\n--- TaskOutput skipped definitions/usages in source ---\n'
rg -n -C 3 'class TaskOutput|skipped' lib/crewai/src/crewai
printf '\n--- TaskOutput model_json_schema relevant fields ---\n'
python3 - <<'PY'
import ast
from pathlib import Path
p = Path("lib/crewai/src/crewai/tasks/agent_output.py")
tree = ast.parse(p.read_text())
for node in tree.body:
if isinstance(node, ast.ClassDef) and node.name == "TaskOutput":
print("TaskOutput fields:")
for stmt in node.body:
if isinstance(stmt, ast.AnnAssign) and isinstance(stmt.target, ast.Name):
print(f" {stmt.target.id}: {ast.unparse(stmt.annotation)}")
break
PYRepository: crewAIInc/crewAI
Length of output: 241
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Locate TaskOutput and related references in the repository.
printf '%s\n' '--- TaskOutput candidates ---'
fd -a 'agent_output|task_output|TaskOutput' lib/crewai/src lib/crewai/tests 2>/dev/null | sed 's#^\./##' | head -80
printf '\n--- TaskOutput references with file lines ---\n'
rg -n --glob '!**/*.pyc' 'TaskOutput|class TaskOutput|skipped' lib/crewai/src/crewai | head -200Repository: crewAIInc/crewAI
Length of output: 14952
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- TaskOutput model ---'
cat -n lib/crewai/src/crewai/tasks/task_output.py | sed -n '1,80p'
printf '\n--- Store execution log ---'
cat -n lib/crewai/src/crewai/crew.py | sed -n '1469,1550p'
printf '\n--- TaskOutput storage handlers ---'
cat -n lib/crewai/src/crewai/utilities/task_output_storage_handler.py | sed -n '1,180p'
printf '\n--- SQLite storage model/storage ---'
cat -n lib/crewai/src/crewai/memory/storage/kickoff_task_outputs_storage.py | sed -n '1,220p'
printf '\n--- Conditional skip path ---'
cat -n lib/crewai/src/crewai/crews/utils.py | sed -n '170,215p'
cat -n lib/crewai/src/crewai/tasks/conditional_task.py | sed -n '45,68p'
printf '\n--- Replay path ---'
cat -n lib/crewai/src/crewai/crew.py | sed -n '2045,2062p'
printf '\n--- Deterministic schema/data-shape probe ---'
python3 - <<'PY'
import ast, json
from pathlib import Path
path = Path("lib/crewai/src/crewai/tasks/task_output.py")
tree = ast.parse(path.read_text())
fields = []
for node in tree.body:
if isinstance(node, ast.ClassDef) and node.name == "TaskOutput"":
for stmt in node.body:
if isinstance(stmt, ast.AnnAssign) and isinstance(stmt.target, ast.Name):
fields.append(stmt.target.id)
print("TaskOutput fields:", fields)
print("skipped present:", "skipped" in fields)
for filename in [
"lib/crewai/src/crewai/crew.py",
"lib/crewai/src/crewai/utilities/task_output_storage_handler.py",
"lib/crewai/src/crewai/memory/storage/kickoff_task_outputs_storage.py",
]:
text = Path(filename).read_text()
print(f"\n{filename}: TaskOutput field mentions include skipped only inside string/template-like contexts?")
for lineno, line in enumerate(text.splitlines(), 1):
if "skipped" in line:
print(f"{lineno}: {line}")
PYRepository: crewAIInc/crewAI
Length of output: 24958
Preserve skipped in task-output persistence and replay.
_store_execution_log writes TaskOutput fields without skipped, and replay() reconstructs TaskOutput from stored fields without it. A skipped conditional task can already be stored with raw = ""; on replay it becomes skipped = False and this filter can select it as the final crew output. Persist skipped as part of the stored output and restore it when rebuilding task outputs.
🤖 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/crew.py` around lines 1913 - 1916, Preserve the
TaskOutput.skipped flag through the _store_execution_log persistence path and
replay() reconstruction: include skipped when storing each task output and
restore it when creating TaskOutput instances during replay. Ensure skipped
conditional outputs remain excluded by the valid_outputs filter after replay.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When the final task in a crew legitimately produces an empty output, the crew returns an earlier task's output instead — or crashes if the only task returned empty.
Root cause
Crew._create_crew_outputselected the final output with a truthiness filter:That filter was introduced in #1937 to skip the empty
TaskOutputa skipped conditional task produces. But it can't tell a skipped task apart from a task that ran and produced an empty string, so it drops both — andvalid_outputs[-1]then returns the wrong task (or the list is empty and it raises).Fix
Distinguish the two cases explicitly instead of by truthiness:
TaskOutput.skipped: bool = False.ConditionalTask.get_skipped_task_output()setsskipped=True._create_crew_outputfilters onnot t.skipped.Skipped conditional tasks are still excluded (so #1937's behavior is preserved), while a task that genuinely produced an empty output is kept as the crew result.
Testing
Added
test_create_crew_output_keeps_empty_final_task_output(empty final output is returned; single empty task doesn't raise) andtest_create_crew_output_skips_skipped_conditional_task(skipped conditional still excluded). The existingtest_conditional_task_last_task_when_conditional_is_false(#1937) still passes.No new ruff findings vs. main.
This fix was developed with AI assistance; I verified the behavior and the fix (including that #1937's conditional-skip case is preserved) and reviewed every line.