Skip to content

fix: respect final flag in mergeMessages and handle user-llm-text event - #211

Merged
mattieruth merged 1 commit into
pipecat-ai:mainfrom
rahulsolanki001:fix/conversation-merge-final-and-user-llm-text
Jun 11, 2026
Merged

mattieruth merged 1 commit into
pipecat-ai:mainfrom
rahulsolanki001:fix/conversation-merge-final-and-user-llm-text

Conversation

@rahulsolanki001

@rahulsolanki001 rahulsolanki001 commented May 22, 2026

Copy link
Copy Markdown

Fixes #210

Summary

  • mergeMessages final guard: shouldMerge in conversationActions.ts now checks !lastMerged.final, so a finalized bot turn is never merged with the next turn's messages regardless of the 30-second window.
  • user-llm-text event wiring: Added RTVIEvent.UserLlmText to the event enum, a USER_LLM_TEXT case in PipecatClient.handleMessage, onUserLlmText to the callbacks type, and a handler in useConversationEventWiring that calls upsertUserTranscript — making chat-mode user messages visible in the conversation array.

Changes

File Change
client-js/rtvi/events.ts Add UserLlmText = "userLlmText" to RTVIEvent; add userLlmText to RTVIEvents type
client-js/client/client.ts Add onUserLlmText callback; add case USER_LLM_TEXT: in handleMessage
client-react/src/conversation/conversationActions.ts Add !lastMerged.final && to shouldMerge
client-react/src/conversation/useConversationEventWiring.ts Wire RTVIEvent.UserLlmTextupsertUserTranscript(..., true)

Test plan

  • Two consecutive bot turns within 30 seconds produce separate bubbles (not merged)
  • In chat mode, user messages appear in the conversation array via usePipecatConversation
  • In voice mode, user-llm-text still fires and is now surfaced as RTVIEvent.UserLlmText
  • Existing UserTranscript behavior is unchanged
  • TypeScript builds cleanly in both packages (tsc --noEmit)

@markbackman
markbackman requested review from Regaddi and mattieruth May 27, 2026 14:40
Comment thread client-js/client/client.ts Outdated
break;
}
case RTVIMessageType.USER_LLM_TEXT: {
const llmTextData = ev.data as TranscriptData;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the data in a user-llm-text events does not match TranscriptData. It simply includes a text entry (much like bot-llm-text)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

You're right — fixed in the follow-up commit. Defined a proper UserLlmTextData = { text: string } type in messages.ts and used it throughout (callback declaration, RTVIEvents map, handleMessage cast). TranscriptData carries final, timestamp, and user_id which aren't part of the user-llm-text payload.


// Chat mode: server emits user-llm-text instead of user-transcription when text
// input bypasses STT. Treat it as a finalized user message.
useRTVIClientEvent(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i haven't had a chance to test this, but I do not think this is what we want. user-llm-text will fire for EVERY user entry to the llm, spoken or not. If I'm reading this right, the result is that we will duplicate the user's spoken text. Even if not, I'm not sure this is the correct approach to filling in the missing user text. cc @Regaddi for thoughts.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Agreed and reverted in the follow-up commit. You're right that the wiring would cause duplication in voice mode. The exact path:

  1. user-transcription fires with final: trueupsertUserTranscript updates the last part to final: true
  2. user-llm-text fires → upsertUserTranscript checks messages[lastUserIndex].final — still false at the message level (finalization only happens on next UserStartedSpeaking) — and lastPart.final === true, so it pushes a new part with the same text. The user message ends up with the transcript text twice.

Removed the useConversationEventWiring change entirely. RTVIEvent.UserLlmText and onUserLlmText are still exposed so callers can handle the event themselves, but we leave proper chat-mode conversation integration for a follow-up that can avoid this duplication path (e.g. an opt-in flag or a dedicated action that checks whether a user-transcription already exists for the current turn).

@mattieruth mattieruth left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is great. and a good catch. Before I approve, I'd like @Regaddi to weigh in on the client-react change.

Comment thread client-js/rtvi/messages.ts Outdated
text: string;
};

export type UserLlmTextData = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I hate writing nits to people i don't know, but this should be UserLLMTextData for consistency

Suggested change
export type UserLlmTextData = {
export type UserLLMTextData = {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good catch, applied — renamed to UserLLMTextData everywhere to match BotLLMTextData.

@Regaddi

Regaddi commented May 29, 2026

Copy link
Copy Markdown
Collaborator

I think the client-react change is fine: it avoids separate messages being merged into a single text bubble.
The branch only needs a proper rebase to resolve the conflicts.

mergeMessages was merging bot turns that were already finalized
(final: true) into subsequent bot turns within the 30-second window,
causing text from a completed turn to accumulate into the next turn's
bubble. Add !lastMerged.final guard to the shouldMerge condition.

RTVIMessageType.USER_LLM_TEXT had no corresponding RTVIEvent, no
handler in PipecatClient.handleMessage, and no callback type. Add
RTVIEvent.UserLlmText, UserLLMTextData type, onUserLlmText callback,
and a USER_LLM_TEXT case in handleMessage so callers can react to user
LLM text events. Conversation hook integration is intentionally left
for a follow-up: wiring user-llm-text -> upsertUserTranscript
unconditionally would duplicate user text in voice mode because
user-transcription already finalizes a part before user-llm-text fires.

Fixes pipecat-ai#210
@rahulsolanki001
rahulsolanki001 force-pushed the fix/conversation-merge-final-and-user-llm-text branch from 9d3e48e to 6119ada Compare May 30, 2026 06:50
@rahulsolanki001

Copy link
Copy Markdown
Author

Rebased onto current main in a single squashed commit — the intermediate fixup commits are now gone. Resolved the UIJobGroupData/UICancelJobGroupData rename that upstream introduced. Branch shows as mergeable.

@mattieruth mattieruth left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

feel free to hit the button :)

@rahulsolanki001

Copy link
Copy Markdown
Author

Thanks so much for the thorough review and the quick turnaround — really appreciated! 🙏

On the topic of the intentionally-deferred part (chat-mode user messages in usePipecatConversation): would a follow-up PR be welcome? Here's the approach we had in mind to avoid the voice-mode duplication:

useRTVIClientEvent(
  RTVIEvent.UserLlmText,
  useAtomCallback(
    useCallback((get, set, data) => {
      const messages = get(messagesAtom);
      const lastUserIdx = findLastIndex(
        messages,
        (m: ConversationMessage) => m.role === "user"
      );

      if (lastUserIdx === -1 || messages[lastUserIdx].final) {
        // Chat mode: no in-progress user message — create one from the LLM text
        upsertUserTranscript(get, set, data.text ?? "", true);
      }
      // Finalize in both cases:
      // voice mode → finalizes the existing message from user-transcription
      // chat mode → finalizes the one we just created
      finalizeLastMessage(get, set, "user");
    }, [])
  )
);

The key insight: instead of unconditionally calling upsertUserTranscript (which caused the duplicate-part bug), we check whether an in-progress user message already exists. If it does (voice mode — user-transcription already handled it), we skip straight to finalizeLastMessage. If not (chat mode), we create the message first, then finalize.

Happy to open a PR if this direction looks right to you both!

@Regaddi

Regaddi commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

@rahulsolanki001 Feel free to open a follow-up PR. Thank you!

@rahulsolanki001

Copy link
Copy Markdown
Author

Opened the follow-up as #225 — wires RTVIEvent.UserLlmText into usePipecatConversation with the voice/chat mode guard we discussed. It's branched from this fix, so it'll need a rebase onto main once this one is merged.

@mattieruth

Copy link
Copy Markdown
Contributor

i'm going ahead and merging this one in the meantime

@mattieruth
mattieruth merged commit bed406f into pipecat-ai:main Jun 11, 2026
2 checks passed
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.

Two conversation bugs: mergeMessages ignores final flag + user-llm-text has no RTVIEvent mapping

3 participants