Add proof of concept of transcription ordering fix#1346
Conversation
🦋 Changeset detectedLatest commit: 9ee3932 The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
size-limit report 📦
|
There was a problem hiding this comment.
generally makes sense to me!
For agent use cases, I believe in theory there's still a timing issue wrt speech timing vs agent transcript timing. The sender timing should be consistent for these cases anyways, so we gain little from the firstReceivedTime timestamp unless the textstream were to open before actual data is being sent. I don't think that's the case though: https://github.com/livekit/agents/blob/main/livekit-agents/livekit/agents/voice/room_io/_output.py#L454-L464
we only stream_text once the transcription is available
|
@lukasIO - for context, In the original issue, it seemed like the reporter mentioned that empty transcription chunks can get sent before the actual "content containing" transcriptions get sent. Reading that agents snippet you pasted though, it makes me wonder if maybe this isn't really right, and a better direction here maybe could be to keep with the old strategy of ordering transcriptions based on the first received transcription chunk, but ONLY start aggregating chunks once the first non empty chunk is received (I'm assuming non empty means So that would look something like:
And then the transcriptions would get ordered Thoughts on that? |
Previously, transcriptions were ordered by the first actual transcription data stream packet arrival time.
What this meant was that in a situation like the below:
The transcriptions would be ordered in
Agent->Userorder, notUser->Agent.To fix this, change transcription ordering so it is based off of when transcriptions data streams are initially opened, and push all this ordering logic further up into the
components-corepackage so it is more easily testable (and then bothuseTranscriptionsANDuseSessionMessagesget this same fix). Finally, add some tests to assert both this behavior and the related clock offset behavior which @davidliu reported and was fixed in #1229.Fixes #1280 (I am pretty sure, but would like to get confirmation from the reporter first)
Todo