Skip to content

fix(reasoning): detect bare READY markers and honor the last decision - #6882

Open
CountClaw wants to merge 2 commits into
crewAIInc:mainfrom
CountClaw:fix/reasoning-ready-marker-detection-6204
Open

fix(reasoning): detect bare READY markers and honor the last decision#6882
CountClaw wants to merge 2 commits into
crewAIInc:mainfrom
CountClaw:fix/reasoning-ready-marker-detection-6204

Conversation

@CountClaw

Copy link
Copy Markdown

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 missing workflow OAuth scope on that account).

Closes the same issue intent as #6754 / original fix for ready-marker detection.

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
@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The change adds shared readiness detection for full phrases and line-start READY/NOT READY markers. Planning response paths now use this detector, with precedence determined by the last explicit marker. Unit tests cover detection and fallback behavior.

Changes

Readiness detection

Layer / File(s) Summary
Shared readiness detector
lib/crewai/src/crewai/utilities/reasoning_handler.py
Adds response_indicates_ready, including case-insensitive marker matching, punctuation support, empty-response handling, and last-marker precedence.
Parser integration and tests
lib/crewai/src/crewai/utilities/reasoning_handler.py, lib/crewai/tests/utilities/test_response_indicates_ready.py
Updates function-calling, text fallback, and standard planning parsing to use the shared detector. Tests cover positive markers, negative markers, false positives, and fallback plans.

Suggested reviewers: greysonlalonde, lorenzejay

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: detecting bare readiness markers and honoring the last decision.
Description check ✅ Passed The description directly explains the readiness-marker detection change and its purpose.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Warning

⚠️ This pull request shows signs of AI-generated slop (redundant_comments). It has been flagged by CodeRabbit slop detection and should be reviewed carefully.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between f7ba8e3 and c9922ff.

📒 Files selected for processing (2)
  • lib/crewai/src/crewai/utilities/reasoning_handler.py
  • lib/crewai/tests/utilities/test_response_indicates_ready.py

if last_decision is not None:
return last_decision

return _READY_FULL_PHRASE in response

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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: compare response.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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants