Skip to content

fix(flow): don't double-append the turn reply when a handler trims history#6510

Open
joaomdmoura wants to merge 1 commit into
mainfrom
joao/epd-181-conversational-flow-handle_turn-double-appends-the-reply
Open

fix(flow): don't double-append the turn reply when a handler trims history#6510
joaomdmoura wants to merge 1 commit into
mainfrom
joao/epd-181-conversational-flow-handle_turn-double-appends-the-reply

Conversation

@joaomdmoura

@joaomdmoura joaomdmoura commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes EPD-181: in the experimental Conversational Flow, handle_turn()'s safety net — "append the handler's return value if the handler didn't append an assistant message itself" — inferred that by comparing assistant-message counts before and after kickoff. A handler that appends its reply and trims state.messages to a cap (a normal bounded-context pattern) leaves the count unchanged, so the fallback appended the reply a second time — on every turn once trimming engaged. The duplicates then crowd real turns out of the capped window. Silent, starts mid-conversation, nasty to debug.

Fix

Replace the count heuristic with an explicit per-turn signal, as the ticket suggests:

  • append_assistant_message() sets an _assistant_reply_appended instance flag.
  • handle_turn() and stream_turn() clear the flag right before kickoff and only run the fallback when no assistant message was appended during the turn — robust to any history mutation (trimming, rewriting, compaction).
  • This also covers append_agent_result(..., visibility="public") and the built-in handlers, which all append through append_assistant_message().
  • The now-unused _assistant_message_count() helper is removed.

Testing

  • The ticket's clean-room repro (echo handler with a 4-message cap) now prints NOT REPRODUCED: no duplicate assistant messages across 4 turns — where 1.15.2 duplicated from turn 3 onward — and the capped window correctly holds the last two full turns instead of stale echo copies.
  • New TestHandleTurnReplyFallback regression tests: the trim scenario across 4 turns (no duplicates + exact window contents), and the fallback still appending exactly once for handlers that return without appending.
  • Green: full test_flow_conversation.py (44), broader flow suites (305: flow, definition, from-definition, persistence, human-input integration), ruff, and mypy.

🤖 Generated with Claude Code


Note

Low Risk
Targeted fix in experimental conversational mixin with regression tests; behavior change only affects when the turn reply fallback runs.

Overview
Fixes EPD-181: experimental conversational handle_turn() / stream_turn() no longer infer “did the handler already reply?” by comparing assistant message counts before and after kickoff.

append_assistant_message() now sets a per-turn _assistant_reply_appended flag; each turn resets it before kickoff and only runs the return-value fallback when the flag is still false. That stays correct when handlers cap or rewrite state.messages mid-turn (where count-based logic falsely triggered a second append).

Removes _assistant_message_count(). Adds TestHandleTurnReplyFallback for trim-with-cap and silent-handler fallback behavior.

Reviewed by Cursor Bugbot for commit 8d2d6b1. Bugbot is set up for automated code reviews on this repo. Configure here.

…story

handle_turn() (and stream_turn) decided "did the handler append its
reply?" by snapshotting the assistant-message count before kickoff and
appending the stringified result when the count came back unchanged. A
handler that appends its reply and then trims state.messages to a cap —
a normal bounded-context pattern — left the count unchanged, so the
fallback appended the reply a second time on every turn once trimming
engaged, and the duplicates then crowded real turns out of the capped
window.

Replace the count heuristic with an explicit per-turn flag:
append_assistant_message() sets _assistant_reply_appended, handle_turn
and stream_turn clear it before kickoff and only fall back when no
assistant message was appended during the turn. The now-unused
_assistant_message_count() helper is removed.

Fixes EPD-181.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@linear

linear Bot commented Jul 10, 2026

Copy link
Copy Markdown

EPD-181

@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 50b5b6dd-f34e-43ee-8f2d-efd58d8c536a

📥 Commits

Reviewing files that changed from the base of the PR and between 7baf8f9 and 8d2d6b1.

📒 Files selected for processing (2)
  • lib/crewai/src/crewai/experimental/conversational_mixin.py
  • lib/crewai/tests/test_flow_conversation.py

📝 Walkthrough

Walkthrough

The conversational mixin now uses an explicit per-turn flag to detect appended assistant replies. Both regular and streaming turns avoid duplicate fallback messages, while silent handlers still receive fallback replies. Regression tests cover capped-history and non-appending handlers.

Changes

Conversational reply fallback

Layer / File(s) Summary
Reply tracking and fallback behavior
lib/crewai/src/crewai/experimental/conversational_mixin.py, lib/crewai/tests/test_flow_conversation.py
Adds _assistant_reply_appended, resets it for each turn, marks it when replies are appended, and uses it for regular and streaming fallback decisions. Removes the message-count helper and adds regression tests for trimmed replies and silent handlers.

Suggested reviewers: vinibrsl, lucasgomide

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main fix: preventing duplicate turn replies when handlers trim history.
Description check ✅ Passed The description is directly related to the changeset and accurately explains the bug, fix, and tests.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch joao/epd-181-conversational-flow-handle_turn-double-appends-the-reply

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.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant