fix(reasoning): accept short-form READY keyword from model responses#6261
fix(reasoning): accept short-form READY keyword from model responses#6261s3ak6i-dev wants to merge 2 commits into
Conversation
Prompt templates instruct models to conclude with "READY" or "NOT READY", but the detection logic only matched the legacy phrase "READY: I am ready to execute the task." -- so models following the current prompts always triggered a NOT READY result and looped indefinitely. Adds AgentReasoning._is_ready() which accepts both the short keyword form and the legacy phrase, while correctly rejecting "NOT READY". Updates _parse_planning_response and both fallback branches in _call_with_function to use the new helper. Fixes crewAIInc#6204 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![corridor-security[bot]](https://avatars.githubusercontent.com/in/1177339?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary: This PR updates agent reasoning readiness parsing to accept both short-form and legacy READY indicators in model responses, with tests covering READY and NOT READY cases. No exploitable security vulnerabilities were identified.
Risk: Low risk. The change affects internal model-response parsing and does not introduce a new public endpoint, authentication/authorization change, external integration, file/network access, or dependency/workflow supply-chain risk.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthrough
ChangesAgentReasoning readiness detection
🚥 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 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 |
|
Note: this PR was authored with AI assistance (Claude Code). |
![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/tests/agents/test_agent_reasoning.py (1)
41-43: ⚡ Quick winAdd a regression test for READY-as-substring false positives.
Please add a case like
assert not AgentReasoning._is_ready("I already need more info.")so marker detection stays boundary-based and doesn’t regress.🤖 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/tests/agents/test_agent_reasoning.py` around lines 41 - 43, Add an additional assertion to the test_no_keyword_returns_false method in the AgentReasoning test class to check for a boundary-based false positive case. Include a new assertion that verifies AgentReasoning._is_ready returns False when the string contains "READY" as a substring within another word, such as in "I already need more info", to ensure the marker detection stays boundary-based and prevents substring matches from being incorrectly identified as the READY keyword.
🤖 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`:
- Around line 592-594: The substring matching for "READY" in the return
statement is causing false positives by matching unrelated words like "ALREADY".
Replace the simple substring check `"READY" in upper` with marker-aware matching
that treats "READY" as a complete word boundary (such as using word boundary
markers like space or punctuation around it) to ensure it only matches the
standalone word "READY" and not as part of other words, while maintaining the
"NOT READY" exclusion logic.
---
Nitpick comments:
In `@lib/crewai/tests/agents/test_agent_reasoning.py`:
- Around line 41-43: Add an additional assertion to the
test_no_keyword_returns_false method in the AgentReasoning test class to check
for a boundary-based false positive case. Include a new assertion that verifies
AgentReasoning._is_ready returns False when the string contains "READY" as a
substring within another word, such as in "I already need more info", to ensure
the marker detection stays boundary-based and prevents substring matches from
being incorrectly identified as the READY keyword.
🪄 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: 1d30298e-0293-497e-929d-87fca88ba11b
📒 Files selected for processing (2)
lib/crewai/src/crewai/utilities/reasoning_handler.pylib/crewai/tests/agents/test_agent_reasoning.py
Substring check "READY" in upper also matched words like "ALREADY" and "UNREADY", causing agents to prematurely exit planning loops. Switch to re.search(r"\bREADY\b") and short-circuit on NOT READY first. Add two regression tests covering the substring false-positive cases. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2 participants
Prompt templates instruct models to conclude with "READY" or "NOT READY", but the detection logic only matched the legacy phrase "READY: I am ready to execute the task." -- so models following the current prompts always triggered a NOT READY result and looped indefinitely.
Adds AgentReasoning._is_ready() which accepts both the short keyword form and the legacy phrase, while correctly rejecting "NOT READY". Updates _parse_planning_response and both fallback branches in _call_with_function to use the new helper.
Note: pre-commit hooks were skipped locally due to a Windows path
incompatibility (.venv/bin/activate). ruff and mypy were run manually
and passed. CI will confirm on Linux.
Fixes #6204
Summary by CodeRabbit
New Features
Bug Fixes
Tests