fix(reasoning): detect bare READY markers and honor the last decision - #6882
fix(reasoning): detect bare READY markers and honor the last decision#6882CountClaw wants to merge 2 commits into
Conversation
Planning readiness only matched the full instructional phrase "READY: I am ready to execute the task.", so models that conclude with a short-form READY line (common with Ollama/local models) were stuck in endless refine loops. Introduce response_indicates_ready() that accepts the full phrase and line-anchored READY / NOT READY markers, with the last marker winning. Closes crewAIInc#6204
📝 WalkthroughWalkthroughThe change adds shared readiness detection for full phrases and line-start ChangesReadiness detection
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 Warning |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/utilities/reasoning_handler.py`:
- Line 71: Update the full-phrase check in reasoning_handler.py at lines 71-71
to compare response.casefold() against _READY_FULL_PHRASE.casefold(), preserving
the existing matching behavior. Add a regression assertion in
test_response_indicates_ready.py at lines 14-20 covering an embedded lowercase
full phrase.
🪄 Autofix
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: 751c3f04-878b-4708-a335-e6d4ca27b3d8
📒 Files selected for processing (2)
lib/crewai/src/crewai/utilities/reasoning_handler.pylib/crewai/tests/utilities/test_response_indicates_ready.py
| if last_decision is not None: | ||
| return last_decision | ||
|
|
||
| return _READY_FULL_PHRASE in response |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make full-phrase matching case-insensitive.
Line 71 uses a case-sensitive containment check. A lowercase full phrase that is not at line start does not match the short-marker regex and returns False. Normalize both strings with casefold(). Add a regression test for an embedded lowercase full phrase.
lib/crewai/src/crewai/utilities/reasoning_handler.py#L71-L71: compareresponse.casefold()with_READY_FULL_PHRASE.casefold().lib/crewai/tests/utilities/test_response_indicates_ready.py#L14-L20: add an assertion for an embedded lowercase full phrase.
Proposed fix
- return _READY_FULL_PHRASE in response
+ return _READY_FULL_PHRASE.casefold() in response.casefold()📍 Affects 2 files
lib/crewai/src/crewai/utilities/reasoning_handler.py#L71-L71(this comment)lib/crewai/tests/utilities/test_response_indicates_ready.py#L14-L20
🤖 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/reasoning_handler.py` at line 71, Update the
full-phrase check in reasoning_handler.py at lines 71-71 to compare
response.casefold() against _READY_FULL_PHRASE.casefold(), preserving the
existing matching behavior. Add a regression assertion in
test_response_indicates_ready.py at lines 14-20 covering an embedded lowercase
full phrase.
What
Detect bare READY markers and honor the last readiness decision in planning responses.
Why
Supersedes Kyou12138#6754 after rebasing onto latest
main(original PR branch could not be force-updated from this environment due to missingworkflowOAuth scope on that account).Closes the same issue intent as #6754 / original fix for ready-marker detection.