Feat/personaplex plugin #4660
Feat/personaplex plugin #4660
Conversation
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Guard audio/text channel sends with ChanClosed suppression - Wrap recv_task message handlers in try/except to prevent single malformed frame from breaking the receive loop - Replace deprecated asyncio.get_event_loop() with get_running_loop() - Add response_id to GenerationCreatedEvent emissions - Add exponential backoff (1s -> 30s max) on reconnect instead of fixed 1s delay - Add 32 unit tests covering init, URL building, audio conversion, retry backoff, options, and data structures Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove unused imports in tests (ruff F401) - Fix import sorting (ruff I001) - Apply ruff format to realtime_model.py - Add __pdoc__ dict to realtime/__init__.py for docs generation Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
|
📝 WalkthroughWalkthroughAdds a new PersonaPlex LiveKit plugin: packaging and README, plugin bootstrap (registration, logger, version), voice type definitions, a full RealtimeModel/RealtimeSession implementation (WebSocket + Opus audio I/O, generation lifecycle, retry/backoff, metrics), and tests. Changes
Sequence DiagramsequenceDiagram
participant Agent as LiveKit Agent
participant Session as RealtimeSession
participant Encoder as Opus Encoder
participant WS as WebSocket
participant Server as PersonaPlex Server
participant Decoder as Opus Decoder
Agent->>Session: push_audio(AudioFrame)
Session->>Session: resample to 24kHz mono
Session->>Encoder: encode(PCM)
Encoder-->>Session: Opus bytes
Session->>WS: send(MSG_AUDIO, Opus bytes)
WS->>Server: WebSocket message
Server->>WS: send(MSG_TEXT / MSG_AUDIO)
WS->>Session: receive(MSG_TEXT / MSG_AUDIO)
alt MSG_TEXT
Session->>Session: filter special tokens
Session->>Agent: emit(text token)
else MSG_AUDIO
Session->>Decoder: decode(Opus bytes)
Decoder-->>Session: PCM samples
Session->>Agent: emit(AudioFrame `@24kHz`)
end
Note over Session: silence timeout triggers finalization
Session->>Session: _finalize_generation()
Session->>Agent: emit(GenerationCreatedEvent + metrics)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (2)📓 Common learnings📚 Learning: 2026-01-30T12:53:12.738ZApplied to files:
🧬 Code graph analysis (1)livekit-plugins/livekit-plugins-personaplex/tests/test_realtime_model.py (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (8)
✏️ Tip: You can disable this entire section by setting 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@livekit-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/__init__.py`:
- Around line 19-21: The import statements for Plugin and logger currently
appear after the module-level __all__, which triggers ruff E402; move the
imports "from livekit.agents import Plugin" and "from .log import logger" so
they are placed above the __all__ definition (ensure __all__ references Plugin
and logger remain correct) to satisfy the linter and keep import ordering
correct.
In
`@livekit-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/realtime/realtime_model.py`:
- Around line 327-337: _build_ws_url currently hardcodes "ws://" which prevents
TLS; update it to choose "wss://" when the provided base URL indicates TLS.
Parse self._opts.base_url (or inspect its original string) for a scheme: if it
starts with "wss://" or "https://" use "wss://", if it starts with "ws://" or
"http://" use that scheme, otherwise default to "wss://" when a separate SSL
flag is present or "ws://" otherwise; ensure you strip any existing scheme
before joining host/path so the returned URL is either
"wss://{host}/api/chat?{query}" or "ws://{host}/api/chat?{query}" accordingly.
Use the _build_ws_url function and self._opts.base_url to locate and implement
this change.
🧹 Nitpick comments (5)
livekit-plugins/livekit-plugins-personaplex/README.md (1)
13-13: Consider using a proper markdown link for the URL.The bare URL works but wrapping it in markdown link syntax improves rendering consistency across different markdown parsers.
📝 Suggested fix
-You need a running PersonaPlex server on a GPU machine. See https://github.com/NVIDIA/personaplex for setup. +You need a running PersonaPlex server on a GPU machine. See [NVIDIA PersonaPlex](https://github.com/NVIDIA/personaplex) for setup.livekit-plugins/livekit-plugins-personaplex/pyproject.toml (1)
25-34: Consider adding Python 3.11 and 3.12 classifiers.The
requires-python = ">=3.9.0"allows Python 3.11 and 3.12, but the classifiers only list 3.9 and 3.10. Adding these classifiers improves discoverability on PyPI.📝 Suggested addition
"Programming Language :: Python :: 3.9", "Programming Language :: Python :: 3.10", + "Programming Language :: Python :: 3.11", + "Programming Language :: Python :: 3.12", "Programming Language :: Python :: 3 :: Only", ]livekit-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/realtime/realtime_model.py (3)
239-243: Consider usinglogger.warningfor unsupported feature notification.When a user provides
instructionsthat won't be applied, a warning level is more appropriate than info to ensure visibility.📝 Suggested change
if is_given(instructions): - logger.info( + logger.warning( "PersonaPlex does not support dynamic instructions. " "Instruction changes require reconnection via update_instructions()." )
487-491: Minor: Redundant length check.
if opus_bytes and len(opus_bytes) > 0is redundant since a truthybytesobject always has length > 0.📝 Suggested simplification
- if opus_bytes and len(opus_bytes) > 0: + if opus_bytes: # Prepend audio message type message = bytes([MSG_AUDIO]) + opus_bytes
710-712: Accessing private attribute_input_ratefromrtc.AudioResampler.Accessing
self._input_resampler._input_raterelies on an internal implementation detail. Iflivekit-rtcchanges its internals, this could break. Consider storing the input rate alongside the resampler or checking if there's a public property.📝 Suggested approach
+ self._input_resampler_rate: int | None = None + def _resample_audio(self, frame: rtc.AudioFrame) -> Iterator[rtc.AudioFrame]: if self._input_resampler: - if frame.sample_rate != self._input_resampler._input_rate: + if frame.sample_rate != self._input_resampler_rate: self._input_resampler = None + self._input_resampler_rate = None if self._input_resampler is None and ( frame.sample_rate != SAMPLE_RATE or frame.num_channels != NUM_CHANNELS ): self._input_resampler = rtc.AudioResampler( input_rate=frame.sample_rate, output_rate=SAMPLE_RATE, num_channels=NUM_CHANNELS, ) + self._input_resampler_rate = frame.sample_rate
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
livekit-plugins/livekit-plugins-personaplex/README.mdlivekit-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/__init__.pylivekit-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/log.pylivekit-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/models.pylivekit-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/py.typedlivekit-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/realtime/__init__.pylivekit-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/realtime/realtime_model.pylivekit-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/version.pylivekit-plugins/livekit-plugins-personaplex/pyproject.tomllivekit-plugins/livekit-plugins-personaplex/tests/__init__.pylivekit-plugins/livekit-plugins-personaplex/tests/test_realtime_model.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Format code with ruff
Run ruff linter and auto-fix issues
Run mypy type checker in strict mode
Maintain line length of 100 characters maximum
Ensure Python 3.9+ compatibility
Use Google-style docstrings
Files:
livekit-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/version.pylivekit-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/log.pylivekit-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/__init__.pylivekit-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/models.pylivekit-plugins/livekit-plugins-personaplex/tests/test_realtime_model.pylivekit-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/realtime/__init__.pylivekit-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/realtime/realtime_model.py
🧠 Learnings (1)
📚 Learning: 2026-01-16T07:44:56.353Z
Learnt from: CR
Repo: livekit/agents PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T07:44:56.353Z
Learning: Implement Model Interface Pattern for STT, TTS, LLM, and Realtime models with provider-agnostic interfaces, fallback adapters for resilience, and stream adapters for different streaming patterns
Applied to files:
livekit-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/realtime/realtime_model.py
🧬 Code graph analysis (3)
livekit-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/__init__.py (2)
livekit-agents/livekit/agents/llm/realtime.py (1)
realtime_model(143-144)livekit-agents/livekit/agents/plugin.py (2)
Plugin(13-56)register_plugin(31-36)
livekit-plugins/livekit-plugins-personaplex/tests/test_realtime_model.py (1)
livekit-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/realtime/realtime_model.py (5)
RealtimeModel(73-161)_PersonaplexOptions(49-54)_ResponseGeneration(58-70)model(141-142)provider(145-146)
livekit-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/realtime/realtime_model.py (11)
livekit-agents/livekit/agents/metrics/base.py (4)
Metadata(8-10)RealtimeModelMetrics(91-132)InputTokenDetails(97-102)OutputTokenDetails(104-107)livekit-agents/livekit/agents/utils/misc.py (1)
is_given(25-26)livekit-agents/livekit/agents/utils/aio/channel.py (2)
Chan(49-178)ChanClosed(17-18)livekit-agents/livekit/agents/llm/realtime.py (5)
MessageGeneration(30-34)RealtimeCapabilities(57-63)realtime_model(143-144)RealtimeSessionReconnectedEvent(133-134)RealtimeModelError(47-53)livekit-agents/livekit/agents/llm/chat_context.py (3)
FunctionCall(179-192)ChatContext(218-656)add_message(234-267)livekit-agents/livekit/agents/utils/http_context.py (1)
http_session(40-51)livekit-agents/livekit/agents/llm/tool_context.py (2)
ToolContext(326-449)message(99-100)livekit-agents/livekit/agents/utils/audio.py (1)
AudioByteStream(41-157)livekit-agents/livekit/agents/utils/log.py (1)
log_exceptions(9-41)livekit-agents/livekit/agents/voice/agent.py (1)
instructions(99-104)livekit-agents/livekit/agents/voice/speech_handle.py (1)
add_done_callback(163-164)
🪛 markdownlint-cli2 (0.20.0)
livekit-plugins/livekit-plugins-personaplex/README.md
[warning] 13-13: Bare URL used
(MD034, no-bare-urls)
[warning] 66-66: Table column style
Table pipe does not align with header for style "aligned"
(MD060, table-column-style)
[warning] 66-66: Table column style
Table pipe does not align with header for style "aligned"
(MD060, table-column-style)
[warning] 66-66: Table column style
Table pipe does not align with header for style "aligned"
(MD060, table-column-style)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: type-check (3.9)
- GitHub Check: type-check (3.13)
- GitHub Check: unit-tests
🔇 Additional comments (32)
livekit-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/version.py (1)
1-1: LGTM!Standard version module pattern that integrates correctly with the Hatch build system.
livekit-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/log.py (1)
1-3: LGTM!Standard logger setup following the package naming convention.
livekit-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/models.py (1)
1-24: LGTM!Clean type alias definition providing compile-time type safety for voice selection.
livekit-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/realtime/realtime_model.py (16)
32-45: LGTM!Constants are well-documented with clear naming. The message type prefixes and retry parameters are sensibly configured.
48-71: LGTM!Well-structured dataclasses with appropriate field defaults and type annotations. The
_ResponseGenerationproperly tracks generation lifecycle state.
73-162: LGTM!The
RealtimeModelclass follows the Model Interface Pattern well. URL resolution with fallback chain (parameter → env var → default) is user-friendly, and protocol prefix stripping handles various input formats gracefully.
171-202: LGTM!Clean initialization with proper resource setup. The 100ms frame size (2400 samples at 24kHz) is appropriate for real-time audio streaming. Starting
_main_taskin__init__follows the established pattern for realtime sessions.
215-226: LGTM!Audio pipeline is well-structured with resampling and the
@utils.log_exceptionsdecorator ensures errors are logged without breaking the caller.
259-278: LGTM!Methods appropriately handle PersonaPlex limitations with clear comments explaining the full-duplex streaming model.
281-295: LGTM!The
update_instructionsmethod correctly triggers a reconnection since PersonaPlex requires instruction changes at connection time. Other update methods gracefully handle unsupported features.
298-313: LGTM!Clean shutdown sequence with proper idempotency check and resource cleanup.
338-416: LGTM!The connection management with exponential backoff (1s → 30s) is well-implemented. Task coordination using
asyncio.waitwithFIRST_COMPLETEDproperly handles both normal operation and reconnection scenarios.
417-475: LGTM!The send/receive tasks have proper error handling. Wrapping message handlers in try/except (lines 447-461) prevents malformed frames from breaking the receive loop, which is good defensive coding per the commit message.
493-531: LGTM!Audio decoding correctly converts Opus to PCM with proper clipping and channel handling. The silence timer reset on each audio frame implements the generation boundary detection.
532-561: LGTM!Text token handling correctly filters special tokens and accumulates output for the chat context.
564-611: LGTM!Generation lifecycle management properly distinguishes between server-initiated (
user_initiated=False) and user-requested (user_initiated=True) generations. The UUID-based response IDs aid traceability.
612-639: LGTM!Clean finalization with proper channel cleanup ordering and defensive close checks. The local chat context update preserves conversation history for potential future use.
691-706: LGTM!Silence detection using
asyncio.get_running_loop().call_lateris the correct approach. Usinginterrupted=Falsefor silence-based finalization correctly indicates natural generation completion.
663-663: No changes needed. Thelabelproperty is properly defined in the base classllm.RealtimeModel(livekit-agents/livekit/agents/llm/realtime.py:88-90) and returnsself._label. PersonaPlex'sRealtimeModelcorrectly callssuper().__init__()which initializes the inherited_labelattribute, then customizes it. The code at line 663 will work without errors.livekit-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/realtime/__init__.py (1)
1-12: LGTM!Clean module re-exports with proper
__all__definition and pdoc3 documentation hygiene for unexported symbols.livekit-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/__init__.py (3)
1-17: Clear module docstring and explicit exports.The module docstring and
__all__make the public API straightforward.
24-29: Confirm import-time registration always runs on the main thread.
Plugin.register_pluginraises if called off the main thread; import-time registration can crash if this module is imported from a worker thread. If that’s possible in your loading flow, consider deferring registration or adding a main-thread guard with a clear error/log message.
31-38: Doc cleanup for non-exported symbols looks good.
__pdoc__suppression aligns with__all__.livekit-plugins/livekit-plugins-personaplex/tests/test_realtime_model.py (9)
1-20: Imports and internal symbol usage in tests are clear.The test module setup is straightforward.
25-101: Strong init/metadata coverage.Default, override, and capability assertions give good confidence in constructor behavior.
106-130: URL/options coverage is focused and sensible.The opts validation provides a lightweight sanity check.
135-145: Token filter tests are concise and clear.Good minimal checks around special-token handling.
150-161: Audio constant assertions are straightforward.These sanity checks are valuable.
164-182: Audio conversion tests cover roundtrip and clipping well.Nice validation of edge values and clipping semantics.
188-213: ResponseGeneration defaults and cleanup look good.The test verifies defaults and closes channels explicitly.
218-232: Backoff sequence test is crisp and deterministic.This matches the expected exponential cap behavior.
237-260: Options dataclass tests cover both seed cases.Good validation for
seedbeing set orNone.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
livekit-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/__init__.py
Show resolved
Hide resolved
...t-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/realtime/realtime_model.py
Show resolved
Hide resolved
- Preserve wss:// / https:// scheme for TLS deployments - Use logger.warning for unsupported dynamic instructions - Remove redundant length check on opus_bytes Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@livekit-plugins/livekit-plugins-personaplex/tests/test_realtime_model.py`:
- Around line 248-271: The test_all_fields in TestPersonaplexOptions fails to
assert the dataclass's use_ssl field; update the test to pass a non-default
value for use_ssl when constructing _PersonaplexOptions and add an assertion
verifying opts.use_ssl equals that value (e.g., True), so the
_PersonaplexOptions constructor and field are covered alongside base_url, voice,
text_prompt, seed, and silence_threshold_ms.
🧹 Nitpick comments (3)
livekit-plugins/livekit-plugins-personaplex/tests/test_realtime_model.py (3)
117-141: Consider improving type annotations to avoidtype: ignore.The
**kwargs: objectannotation combined with# type: ignore[arg-type]on line 127 suggests type system friction. UsingAnyor explicit keyword arguments would provide better type safety.Also, as the comment on lines 133-134 notes,
test_basic_urlandtest_seed_in_urldon't actually test URL building—they only verify that options are set correctly. Consider either renaming this test class to reflect what it actually tests (e.g.,TestSessionOptions) or adding actual URL construction tests if that's testable.♻️ Suggested improvement for type annotations
+from typing import Any + class TestBuildWsUrl: - def _make_session_opts(self, **kwargs: object) -> _PersonaplexOptions: + def _make_session_opts(self, **kwargs: Any) -> _PersonaplexOptions: defaults = { "base_url": "localhost:8998", "voice": "NATF2", "text_prompt": "You are helpful.", "seed": None, "silence_threshold_ms": 500, } defaults.update(kwargs) - return _PersonaplexOptions(**defaults) # type: ignore[arg-type] + return _PersonaplexOptions(**defaults)
146-156: Tests verify set membership rather than filtering behavior.The docstrings describe filtering behavior ("should be filtered out", "should pass through"), but the tests only verify which values are in
_SPECIAL_TOKENS. Consider either:
- Renaming the class to
TestSpecialTokensSetto reflect what's actually being tested- Adding tests that exercise the actual token handling logic (e.g., calling the method that filters tokens and verifying output)
199-224: Consider using a pytest fixture for channel cleanup.The manual cleanup at lines 220-223 won't execute if an assertion fails earlier in the test. Using a fixture with
yieldensures cleanup runs regardless of test outcome.♻️ Suggested fixture-based approach
`@pytest.fixture` def response_generation(): from livekit import rtc from livekit.agents import llm, utils gen = _ResponseGeneration( message_ch=utils.aio.Chan[llm.MessageGeneration](), function_ch=utils.aio.Chan[llm.FunctionCall](), response_id="test-123", text_ch=utils.aio.Chan[str](), audio_ch=utils.aio.Chan[rtc.AudioFrame](), ) yield gen gen.message_ch.close() gen.function_ch.close() gen.text_ch.close() gen.audio_ch.close() class TestResponseGeneration: def test_defaults(self, response_generation: _ResponseGeneration) -> None: gen = response_generation assert gen.response_id == "test-123" assert gen._first_token_timestamp is None # ... rest of assertions
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
livekit-plugins/livekit-plugins-personaplex/tests/test_realtime_model.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Format code with ruff
Run ruff linter and auto-fix issues
Run mypy type checker in strict mode
Maintain line length of 100 characters maximum
Ensure Python 3.9+ compatibility
Use Google-style docstrings
Files:
livekit-plugins/livekit-plugins-personaplex/tests/test_realtime_model.py
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: livekit/agents PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T07:44:56.353Z
Learning: Follow the Plugin System pattern where plugins in livekit-plugins/ are separate packages registered via the Plugin base class
📚 Learning: 2026-01-30T12:53:12.738Z
Learnt from: milanperovic
Repo: livekit/agents PR: 4660
File: livekit-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/__init__.py:19-21
Timestamp: 2026-01-30T12:53:12.738Z
Learning: In plugin __init__.py files under the livekit-plugins or similar plugin directories, place internal imports (for example, from .log import logger) after the __all__ definition. These imports are used for plugin registration and are not part of the public API. This pattern is used across plugins (e.g., openai, deepgram, ultravox) and helps avoid E402 violations while keeping the public API surface clean.
Applied to files:
livekit-plugins/livekit-plugins-personaplex/tests/test_realtime_model.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: type-check (3.13)
- GitHub Check: type-check (3.9)
- GitHub Check: unit-tests
🔇 Additional comments (5)
livekit-plugins/livekit-plugins-personaplex/tests/test_realtime_model.py (5)
1-21: LGTM!Imports are well-organized with clear separation between standard library, third-party, and internal plugin modules. Testing private internals (
_PersonaplexOptions,_ResponseGeneration,_SPECIAL_TOKENS) is appropriate for thorough unit testing of implementation details.
25-112: LGTM!Excellent coverage of initialization scenarios including:
- URL prefix stripping and SSL inference from
ws://,wss://,http://,https://- Environment variable handling with proper precedence
- All configurable options and capability flags
The use of
monkeypatchfor environment variable tests is clean and idiomatic.
161-172: LGTM!Constant verification tests are valuable for catching accidental changes to protocol-critical values.
174-194: LGTM!Solid numerical tests covering:
- Round-trip conversion with critical edge cases (min/max int16 boundaries)
- Clipping behavior for out-of-range float values
Using
np.testing.assert_array_equalis the correct approach for array comparisons.
229-243: LGTM!Good validation of retry constants and the exponential backoff sequence. The sequence test effectively documents the expected backoff behavior (1→2→4→8→16→30→30).
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
livekit-plugins/livekit-plugins-personaplex/tests/test_realtime_model.py
Show resolved
Hide resolved
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Rename TestBuildWsUrl to TestSessionOptions with proper typed helper - Rename TestHandleTextToken to TestSpecialTokens to reflect actual scope - Wrap ResponseGeneration assertions in try/finally for channel cleanup Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Added by upstream in AGT-2474 (livekit#4622). PersonaPlex is full-duplex and does not support explicit user turn commits. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Use ws:// scheme in all example URLs (README, docstrings) - Sync uv.lock after upstream merge Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Thanks for the PR! |
# Conflicts: # uv.lock
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
...t-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/realtime/realtime_model.py
Outdated
Show resolved
Hide resolved
...t-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/realtime/realtime_model.py
Show resolved
Hide resolved
...t-plugins/livekit-plugins-personaplex/livekit/plugins/personaplex/realtime/realtime_model.py
Show resolved
Hide resolved
| async def aclose(self) -> None: | ||
| if self._closed: | ||
| return | ||
|
|
||
| self._closed = True | ||
| self._msg_ch.close() | ||
| self._session_should_close.set() |
There was a problem hiding this comment.
🟡 _main_task loop exits prematurely on aclose() before cancel_and_wait takes effect, causing _closing race with _recv_task
When aclose() is called, it first closes _msg_ch (line 309) and sets _session_should_close (line 310), then calls cancel_and_wait on the main task (line 312). However, the _send_task and _recv_task interact through the _closing flag in a racy way.
Detailed race condition explanation
When aclose() closes _msg_ch, the _send_task's async for msg in self._msg_ch loop ends and sets self._closing = True at line 436. Meanwhile, _recv_task could receive a WebSocket CLOSE frame from the server (since we're closing the connection). At line 475, it checks if self._closing: to decide whether to return cleanly or raise an error.
The race occurs because _recv_task could receive the CLOSE message before _send_task has finished iterating and set _closing = True. In that case, _recv_task raises APIConnectionError("PersonaPlex connection closed unexpectedly") at line 477. This exception is re-raised by task.result() at line 381, which gets caught by the outer except handler at line 395. This triggers spurious error logging ("PersonaPlex WebSocket error") and emits an error event with recoverable=True, potentially causing an unnecessary reconnection attempt before cancel_and_wait actually cancels the task.
Impact: During normal shutdown, the session may log spurious WebSocket errors and emit unnecessary error events.
Prompt for agents
In aclose(), set self._closing = True before closing the message channel and setting session_should_close. This way, if _recv_task receives a CLOSE frame from the server during shutdown, it sees _closing is True and returns cleanly instead of raising APIConnectionError. Change aclose() to:
async def aclose(self) -> None:
if self._closed:
return
self._closed = True
self._closing = True # <-- set _closing BEFORE triggering shutdown
self._msg_ch.close()
self._session_should_close.set()
await utils.aio.cancel_and_wait(self._main_atask)
...
Was this helpful? React with 👍 or 👎 to provide feedback.
| # Filter special tokens | ||
| try: | ||
| token_id = int(text) | ||
| if token_id in _SPECIAL_TOKENS: | ||
| return | ||
| except ValueError: | ||
| pass # Not a numeric token, process as text |
There was a problem hiding this comment.
🔴 _handle_text_token silently drops legitimate numeric text tokens like "0" and "3"
The _handle_text_token method at line 556 parses the decoded text as an integer, and if the integer matches a value in _SPECIAL_TOKENS ({0, 3}), the token is silently discarded. This means if the model generates the literal text "0" or "3" as actual conversational content (e.g., counting, giving a number), those tokens are filtered out and never added to the output.
Root Cause
At lines 555-560:
try:
token_id = int(text)
if token_id in _SPECIAL_TOKENS:
return
except ValueError:
passThe code treats the UTF-8 decoded text payload as a potential token ID. If the text is "0" or "3", int(text) succeeds and the token is discarded. However, the binary protocol already distinguishes message types via the first byte (MSG_TEXT = 0x02), so the payload is explicitly text content. The special token filtering conflates protocol-level token IDs with text content.
Impact: The assistant's text output will silently lose any occurrence of the standalone strings "0" or "3", resulting in corrupted transcripts. For example, if the model says "I have 3 suggestions", the "3" token may be dropped, producing "I have suggestions".
Was this helpful? React with 👍 or 👎 to provide feedback.
3 participants
Summary
Add new livekit-plugins-personaplex plugin for NVIDIA PersonaPlex full-duplex conversational AI
Implements RealtimeModel / RealtimeSession as a WebSocket client connecting to a separately-deployed PersonaPlex server
Binary WebSocket protocol with Opus audio encoding/decoding via sphn
Silence-based generation boundary detection, exponential backoff reconnection, and metrics emission
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.