Skip to content

plugins(krisp): add voice isolation telephony mode - #6510

Merged
lukasIO merged 3 commits into
mainfrom
lukas/add-telephony-mode
Jul 23, 2026
Merged

plugins(krisp): add voice isolation telephony mode#6510
lukasIO merged 3 commits into
mainfrom
lukas/add-telephony-mode

Conversation

@lukasIO

@lukasIO lukasIO commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@lukasIO
lukasIO requested a review from a team as a code owner July 22, 2026 15:49

@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.

View 1 additional finding in Devin Review.

Open in Devin Review

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.

🟡 Mode selection is silently ignored for license-key users

The requested mode is dropped for the license-key backend because _build_inner never forwards it to the license processor (_KrispLicenseFrameProcessor(...) at livekit-plugins/livekit-plugins-krisp/livekit/plugins/krisp/viva_filter.py:158-164), so a user who selects a non-default mode gets no effect and no warning.
Impact: A user relying on their own Krisp license who asks for the telephony/voice-isolation mode silently gets the default behavior with no indication that their choice was ignored.

mode parameter not threaded into the license backend

The cloud branch passes mode=mode to the backend (viva_filter.py:151), but the license branch (viva_filter.py:158-164) omits mode, and _KrispLicenseFrameProcessor.__init__ (_krisp.py:155-163) does not accept a mode argument. As a result any mode value supplied by a KrispLicenseAuthProvider user is accepted at the facade but silently discarded. If the license backend genuinely cannot support mode selection, the facade should at least raise or warn when a non-default mode is combined with KrispLicenseAuthProvider, rather than ignoring it.

(Refers to lines 158-164)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this isn't ideal, but I don't know a nicer way around this. For bring-your-own-license use cases it's the caller's responsibility to choose the correct model path

from typing import Any, Literal, Protocol

from livekit import rtc
from livekit.plugins.krisp_internal import VivaMode

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.

🔴 Noise-reduction plugin becomes completely unusable when the bundled backend wheel is missing or broken

The bundled backend package is now loaded when the plugin module is first imported (from livekit.plugins.krisp_internal import VivaMode at livekit-plugins/livekit-plugins-krisp/livekit/plugins/krisp/viva_filter.py:37) before any of the code that was designed to survive a missing backend, so the entire plugin fails to load instead of falling back to the license option.
Impact: A user who relies on their own Krisp license key and model file can no longer use the plugin at all if the bundled closed-source component is missing or fails to load, even though a working fallback exists.

Top-level import defeats the lazy-import fallback in _build_inner

Previously krisp_internal was only imported lazily inside _build_inner (viva_filter.py:134-145), wrapped in a try/except ModuleNotFoundError that raises a friendly RuntimeError and instructs the user to fall back to auth_provider=KrispLicenseAuthProvider(...). The license-mode path (viva_filter.py:155-164) constructs _KrispLicenseFrameProcessor from ._krisp, which does not depend on krisp_internal at all.

By adding the unguarded top-level import at viva_filter.py:37, importing livekit.plugins.krisp (via __init__.py:36) now fails outright whenever the closed-source krisp_internal wheel is absent or fails to load (e.g. a platform-incompatible native binary), so the license-mode fallback path can never be reached. The default parameter value mode: VivaMode = VivaMode.VOICE_ISOLATION at viva_filter.py:206 also forces VivaMode to be resolvable at definition time, so simply deferring the import is not sufficient — the default would need to be restructured (e.g. mode: VivaMode | None = None resolved lazily inside __init__/_build_inner).

Prompt for agents
The new top-level import `from livekit.plugins.krisp_internal import VivaMode` at viva_filter.py:37 makes the whole `livekit.plugins.krisp` package fail to import when the closed-source `krisp_internal` wheel is missing or fails to load. This defeats the graceful fallback that `_build_inner` was designed to provide (it catches ModuleNotFoundError and points users to KrispLicenseAuthProvider, which does not need krisp_internal). Restructure so that VivaMode is not required at module import time / definition time. One approach: import VivaMode lazily inside _build_inner (only on the LiveKitCloudAuthProvider branch), change the public __init__ signature to `mode: VivaMode | None = None` (guarding the type annotation behind TYPE_CHECKING), and resolve the VOICE_ISOLATION default lazily inside _build_inner. Ensure license-mode users can still import and use the plugin when krisp_internal is unavailable.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the module is a direct dependency of the package so this is expected

@lukasIO
lukasIO merged commit 6e67796 into main Jul 23, 2026
26 checks passed
@lukasIO
lukasIO deleted the lukas/add-telephony-mode branch July 23, 2026 12:27
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.

2 participants