fix(react): Keep an assistant turn in one message on RTVI 2.0.0 by markbackman · Pull Request #246 · pipecat-ai/pipecat-client-web

markbackman · 2026-08-24T21:01:23Z

The bug

On RTVI 2.0.0+, a single assistant turn renders as one bubble per sentence instead of one contiguous block:

assistant: Hi there!
assistant: I'm your helpful AI assistant, here to answer your questions...
assistant: How can I help you today?

Cause

useConversationEventWiring.ts passed aggregated_by === "sentence" as the message's final flag. aggregated_by describes how the text was chunked, not whether the turn is over — a turn contains many sentences — so the message was finalized after its first sentence.

That becomes a visible split through three steps:

updateAssistantBotOutput applies final: final ? true : lastMessage.final, so the bubble is final once sentence 1 lands.
Sentence 2 arrives with spoken_status: "new" and calls ensureAssistantMessage. There is a guard that reopens a prematurely-finalized message, but it requires hasUnspokenContent. Sentence 1's completed progress event has already flagged every part spoken, so the guard returns false and a new message is added instead.
mergeConsecutiveMessages cannot rejoin them, because merging requires !lastMerged.final.

Fix

Pass false on the 2.0.0 path and let the turn be finalized where turn boundaries are actually known — the BotStoppedSpeaking timer, or the user starting a new turn. Both paths already exist and are unchanged.

Backwards compatibility

The change is scoped to the v2 branch. The 1.4.x branch keeps aggregated_by === "sentence" unchanged.

The asymmetry is deliberate: the two protocols reopen a finalized message by different means. On 1.4.x an unspoken event precedes the spoken one for each sentence, so hasUnspokenContent is true and ensureAssistantMessage reopens the message rather than splitting — the per-sentence final is effectively inert there. On 2.0.0 the completed progress event flags every part spoken, which is what defeats the guard. Changing 1.4.x too would alter shipped behavior with no bug to justify it.

This reasoning now lives in a comment on the legacy branch itself, so a later cleanup of the "inconsistency" doesn't quietly reintroduce the bug on 1.4.x.

Testing

265 tests pass, up from 253 on main — 12 added. Within tests/conversation alone that is 212, up from 200.

A failing test was written first, reproducing the reported event sequence (segment #167 new → completed, then #271 new). Before the fix it produced two final: true assistant messages; after, one message with parts ["Hi there!", "How can I help you today?"].

The added tests cover:

v2 turn continuity — one message when a sentence finishes speaking mid-turn; the turn stays open until explicitly finalized; a genuine turn boundary still starts a new message.
Timer re-arming — a BotOutput trailing BotStoppedSpeaking pushes the finalize deadline back rather than letting it fire mid-turn.
1.4.x compatibility — a sentence-aggregated message is still marked final; non-sentence aggregations still are not.

eventWiring.test.tsx drives the real hook through a fake event bus, rather than reimplementing its logic. The harness-based v2 tests are worth migrating in that direction over time.

storeHarness.ts duplicated the same aggregated_by === "sentence" logic, so its v2 path was updated to mirror the wiring — otherwise the suite would have kept exercising the old behavior. The harness's legacy emitBotOutput is untouched.

Typecheck is clean, as is eslint on the changed source files.

Follow-up: unspoken turns are never finalized (not addressed here)

A v2 turn that is never spoken has no finalize path except UserStartedSpeaking, because the BotOutput handler only re-arms an already-pending timer and BotStoppedSpeaking never fires for such a turn. On a text-input-only client with no VAD, UserStartedSpeaking never fires either, so the message stays non-final indefinitely.

Two distinct symptoms, only one of which this PR touches:

The final flag — new here. Sentence aggregation used to finalize such a turn immediately; now it stays open. Cosmetic: it may render as still in progress.
Cross-turn merging — pre-existing. ensureAssistantMessage reopens the finalized message via hasUnspokenContent, so two consecutive unspoken turns merge into one bubble. This happens identically on main, verified by probing both branches, so it is not a regression.

The follow-up should be scoped to the merging, which is the larger half of the problem; the flag alone would not fix it.

BotOutput passed `aggregated_by === "sentence"` as the message's `final` flag. But `aggregated_by` describes how the text was chunked, not whether the turn is over, so the assistant message was finalized after its first sentence. On 2.0.0 that split the turn across bubbles: once a sentence finished speaking its parts are all flagged spoken, so the premature-finalization guard in ensureAssistantMessage no longer recognised the message as resumable and opened a new one for the next segment. mergeConsecutiveMessages could not rejoin them either, since merging requires a non-final message. Pass false on the 2.0.0 path and let the turn be finalized where turn boundaries are actually known: the BotStoppedSpeaking timer, or the user starting a new turn. The 1.4.x path keeps its existing per-sentence behavior, which relies on unspoken content arriving before spoken content to reopen the message. That makes the delayed finalize the only thing ending a 2.0.0 turn, and BotOutput used to cancel it outright. Content events cannot arrive after bot-stopped-speaking -- the server queues them until the next BotStartedSpeaking -- but progress events are not gated that way, so a trailing one would cancel the finalize with nothing left to re-arm it and strand the message non-final until the user spoke. Extract the timer into armBotStoppedFinalizeTimer and postpone the deadline rather than dropping it. The handler is synchronous, so a pending timer cannot have fired before the re-arm. Also collapse the repeated clearTimeout/undefined pairs into cancelFinalizeTimer, since that duplication is what made the timer easy to drop by accident. Tests render the real hook via useConversationEventWiring rather than asserting against the store harness, which mirrors the hook by hand and would pass whether or not the production fix was present.

Regaddi

Approving. The diagnosis is right and the fix is minimal: aggregated_by describes how text was chunked, not whether the turn is over, so using it as final was straightforwardly wrong.

I ran the suite locally. tests/conversation gives 212 passing on this branch against 200 on main, 12 added, no regressions.

I also probed the limitation you disclosed, running each scenario against both this branch and main so I could separate what this PR changes from what was already there.

A. TTS-disabled turn (will_be_spoken: false, no Bot*Speaking events), two consecutive turns:

	`final`	bubbles
this branch	`false` indefinitely	1 (merged)
`main`	`true`	1 (merged)

So the delta really is only the final flag, exactly as you describe. The cross-turn merging is pre-existing: ensureAssistantMessage reopens the finalized message through hasUnspokenContent. Your "cosmetic" framing holds up.

B. Two spoken turns 1s apart (function-call round trip, inside the 2500ms delay): this branch and main both merge into one bubble. No regression.

Two things worth fixing inline, neither blocking.

Regaddi · 2026-08-25T06:54:39Z

              set,
              data.text,
-              isFinal,
+              false,


This is the right call, and the comment above it earns its length.

Confirming the third step of the chain in your description: I read updateAssistantBotOutput, and final: final ? true : lastMessage.final means passing false can never un-finalize a message. So a late progress event landing on an already-finalized turn just advances the cursor and does nothing else. No side effect from the looser flag.

One small thing: this comment explains why v2 passes false, but the legacy branch below has nothing saying why it deliberately keeps isFinal. Your reasoning for the asymmetry (on 1.4.x an unspoken event precedes the spoken one for each sentence, so hasUnspokenContent is true, ensureAssistantMessage reopens rather than splits, and the per-sentence final is effectively inert) is convincing, but right now it lives only in the PR description. A line of it on the legacy branch would stop someone from "fixing" the inconsistency later and quietly reintroducing this bug on 1.4.x.

Done in 27607e2 — the reasoning is now a comment on the legacy branch itself, right above const isFinal = data.aggregated_by === "sentence". You are right that leaving it only in the PR description was an invitation to "fix" the inconsistency later and quietly reintroduce this on 1.4.x.

Thanks for independently confirming step 3. final: final ? true : lastMessage.final being monotonic is what makes the looser flag safe, and it is worth having that verified by someone other than me.

Regaddi · 2026-08-25T06:54:39Z

+          // the message non-final until the user speaks again. This handler is
+          // synchronous, so a pending timer cannot have fired before here.
+          // Re-arming is a no-op once the message is final (legacy path).
+          if (botStoppedSpeakingTimeoutRef.current) {


This guard is the crux of the change and the comment is worth having.

One scoping note for the follow-up. Because the timer is only re-armed when one is already pending, a v2 turn that is never spoken has no finalize path at all except UserStartedSpeaking. On a text-input-only client with no VAD, that event never fires.

Per probe A in my review comment, main has the same merging behavior, so this is not a regression and I am not asking for it here. But it does mean the follow-up should be scoped to the merging, not only to the final flag. The flag is the smaller half of that problem.

Good catch on the scoping, and I have rewritten the limitation section to match. It now separates the two symptoms explicitly: the final flag is new here and cosmetic, while the cross-turn merging via hasUnspokenContent is pre-existing and identical on main — which your probe A is what established. The follow-up is scoped to the merging, with a note that fixing the flag alone would not address it.

The text-input-only-client-with-no-VAD case is called out by name, since that is the configuration where there is genuinely no finalize path at all.

Regaddi · 2026-08-25T06:54:39Z

+
+        finalizeLastMessage(get, set, "assistant");
+      }, BOT_STOPPED_FINALIZE_DELAY_MS);
+    }, [])


react-hooks/exhaustive-deps flags this array for a missing cancelFinalizeTimer, and the same warning fires at line 102 on finalizeLastAssistantMessageIfPending. main is clean on this file, so both are new here.

cancelFinalizeTimer is useCallback(..., []) and therefore stable, so adding it to both arrays is behaviorally a no-op. It would also match how you already updated Connected and BotStartedSpeaking in this same diff, which is what made the inconsistency visible.

Worth noting nothing will catch this for you: CI does not run eslint on this repo, and npm run lint already fails on main with 828 no-undef errors on jest globals in the tests, so the two real warnings are buried.

Fixed in 27607e2 — cancelFinalizeTimer added to both arrays. It is useCallback(..., []) so this is a no-op behaviorally, and it matches what I had already done to Connected and BotStartedSpeaking; the inconsistency was mine.

npx eslint on the two source files is now silent. Your point about nothing catching this stands and is worth its own issue: CI does not run eslint here, and npm run lint fails on main with 828 no-undef errors on jest globals, so any real warning is buried. Not fixing that in this PR.

Regaddi · 2026-08-25T06:54:39Z

+  function finalizeAssistantAfterBotStoppedSpeaking() {
+    const messages = store.get(messagesAtom);
+    const cursorMap = new Map(store.get(botOutputMessageStateAtom));
+    const last = [...messages]
+      .reverse()
+      .find((m: ConversationMessage) => m.role === "assistant");
+    if (last) {
+      const cursor = cursorMap.get(last.createdAt);
+      if (cursor && last.parts && last.parts.length > 0) {
+        const lastPartIdx = last.parts.length - 1;
+        const lastPartText = last.parts[lastPartIdx]?.text;
+        cursor.currentPartIndex = lastPartIdx;
+        cursor.currentCharIndex =
+          typeof lastPartText === "string" ? lastPartText.length : 0;
+        for (let i = 0; i <= lastPartIdx; i++) {
+          cursor.partFinalFlags[i] = true;
+        }
+        store.set(botOutputMessageStateAtom, cursorMap);
+      }
+    }
+    finalizeAssistant();
+  }


This is now the second copy of the cursor-snap block, the first being inside armBotStoppedFinalizeTimer.

A test helper that mirrors production logic drifts silently, and the drift shows up as tests that keep passing while the behavior changes, which is the exact failure this harness exists to catch. Extracting the snap into an exported helper in conversationActions.ts and calling it from both sides would make the harness exercise the real thing instead of a lookalike.

Worth doing now, while the two copies are still identical and the diff is trivial.

More broadly: eventWiring.test.tsx driving the real hook through a fake event bus is the right direction for this file, and it is what let me verify the behavior above with confidence. The harness-based v2 tests would be better off migrating there over time.

Done in 27607e2. Extracted as snapSpeechCursorToEnd(get, set) in conversationActions.ts; both armBotStoppedFinalizeTimer and the harness now call it, so the harness exercises the real function. Net -33 lines.

You were right that it was worth doing now — the copies were still byte-identical, so the diff was mechanical.

Verified behavior-neutral: 265 passing both before and after the extraction (stashed and re-ran to be sure), against 253 on main. That cross-check also caught that the PR description understated its own numbers — 12 tests added, not 5, which matches your 212 vs 200. Description corrected.

Agreed on the direction for eventWiring.test.tsx. Driving the real hook is what made the v2 tests worth trusting, and migrating the harness-based ones there over time is the right call.

Extract the speech-cursor snap that runs on a normal turn boundary into snapSpeechCursorToEnd in conversationActions, and call it from both armBotStoppedFinalizeTimer and the test harness. The harness previously held a byte-identical copy, which could drift from production and would fail silently: tests would keep passing while behavior changed. Document why the 1.4.x path deliberately keeps its per-sentence `final` flag. The asymmetry with the 2.0.0 path is intentional, but the reasoning lived only in the PR description, so a later cleanup could "fix" the inconsistency and reintroduce the bug on 1.4.x. Add the stable cancelFinalizeTimer to the dep arrays of finalizeLastAssistantMessageIfPending and armBotStoppedFinalizeTimer, clearing two new react-hooks/exhaustive-deps warnings and matching the Connected and BotStartedSpeaking handlers. No behavior change.

Disable commit and PR attribution trailers for this repo. Ignore .claude/settings.local.json, which holds per-machine permission grants and is not meant to be shared.

markbackman requested review from Regaddi and filipi87 August 24, 2026 21:18

markbackman force-pushed the mb/fix-assistant-turn-splitting branch from 134916b to d42e831 Compare August 24, 2026 21:56

Regaddi approved these changes Aug 25, 2026

View reviewed changes

markbackman added 2 commits August 25, 2026 09:03

chore: Add shared Claude Code settings

6d6dbe5

Disable commit and PR attribution trailers for this repo. Ignore .claude/settings.local.json, which holds per-machine permission grants and is not meant to be shared.

markbackman force-pushed the mb/fix-assistant-turn-splitting branch from 27607e2 to 6d6dbe5 Compare August 25, 2026 13:04

markbackman merged commit de2fac4 into main Aug 25, 2026
3 checks passed

jptaylor mentioned this pull request Aug 28, 2026

fix(react): End the assistant turn when a message is injected #249

Open

Navigation Menu

Uh oh!

fix(react): Keep an assistant turn in one message on RTVI 2.0.0 - #246

fix(react): Keep an assistant turn in one message on RTVI 2.0.0#246
markbackman merged 3 commits into
mainfrom
mb/fix-assistant-turn-splitting

markbackman commented Aug 24, 2026 •

edited

Loading

Uh oh!

Regaddi left a comment

Uh oh!

Regaddi Aug 25, 2026

Uh oh!

markbackman Aug 25, 2026

Uh oh!

Regaddi Aug 25, 2026

Uh oh!

markbackman Aug 25, 2026

Uh oh!

Regaddi Aug 25, 2026

Uh oh!

markbackman Aug 25, 2026

Uh oh!

Regaddi Aug 25, 2026

Uh oh!

markbackman Aug 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

markbackman commented Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The bug

Cause

Fix

Backwards compatibility

Testing

Follow-up: unspoken turns are never finalized (not addressed here)

Uh oh!

Regaddi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

markbackman commented Aug 24, 2026 •

edited

Loading