Skip to content

(expressive mode): pass expressions at segment start#6359

Open
tinalenguyen wants to merge 1 commit into
mainfrom
tina/fix-expressive-attributes
Open

(expressive mode): pass expressions at segment start#6359
tinalenguyen wants to merge 1 commit into
mainfrom
tina/fix-expressive-attributes

Conversation

@tinalenguyen

Copy link
Copy Markdown
Member

No description provided.

@tinalenguyen tinalenguyen requested a review from a team as a code owner July 9, 2026 00:05

@devin-ai-integration devin-ai-integration Bot 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.

Devin Review found 2 potential issues.

Open in Devin Review

Comment on lines 605 to +608
if self._is_delta_stream:
remaining = self._stripper.flush()
tags = self._stripper.tags
if not self._writer_expression_sent:
extra_attributes = expression_attribute(self._pending_expressions())

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.

πŸ” Late expression tags arriving without subsequent text are silently dropped

When an expression tag arrives after text has been written (_writer_has_text = True) but the stripper buffers it (e.g. partial tag split across chunks), and then flush() is called before more text arrives, the expression is lost. In flush() at livekit-agents/livekit/agents/voice/room_io/_output.py:607, self._writer_expression_sent is True (from the opening header), so _pending_expressions() is never consulted for the closing header. The buffered tag completes during self._stripper.flush() and its expression is added to self._stripper.tags, but it's never attached to any wire segment. This is arguably correct (an expression with no following text is semantically empty), but it means a tag that was split across the last two chunks of a turn is silently discarded rather than riding the closing header.

Open in Devin Review

Was this helpful? React with πŸ‘ or πŸ‘Ž to provide feedback.

Comment on lines +484 to +497
async def _rotate_writer(self, pending: list[ExpressiveTag]) -> None:
"""Finalize the current wire segment and open a new one led by the pending expression."""
assert self._writer is not None
attributes = {ATTRIBUTE_TRANSCRIPTION_FINAL: "true"}
if self._track_id:
attributes[ATTRIBUTE_TRANSCRIPTION_TRACK_ID] = self._track_id
await self._writer.aclose(attributes=attributes)

self._current_id = utils.shortuuid("SG_")
self._writer = await self._create_text_writer(
extra_attributes=expression_attribute(pending)
)
self._writer_expression_sent = True
self._writer_has_text = False

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.

πŸ” If _rotate_writer fails mid-way, the old writer is closed but self._writer is stale

In _rotate_writer at line 484-497, the old writer is closed (await self._writer.aclose(...)) before the new one is created. If _create_text_writer raises (e.g. room disconnected), self._writer still references the old, now-closed writer. The exception propagates to capture_text's try/except (line 564), which logs a warning. On the next capture_text call, self._writer is the closed writer, and self._writer is None is False, so the code may attempt to write to the closed writer. This is an error-recovery edge case that also existed in the old code (writer creation failure left inconsistent state), so it's pre-existing rather than newly introduced.

Open in Devin Review

Was this helpful? React with πŸ‘ or πŸ‘Ž to provide feedback.

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.

1 participant