-
Notifications
You must be signed in to change notification settings - Fork 104
support multiple keyterms for v2 listen client #595
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
support multiple keyterms for v2 listen client
WalkthroughThe connect methods in V2Client/AsyncV2Client and RawV2Client/AsyncRawV2Client now accept keyterm as Optional[Union[str, List[str]]]. Query parameter construction branches to handle a single string or multiple strings, expanding lists into repeated keyterm parameters. Docstrings and type hints were updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as Caller
participant Client as V2Client/AsyncV2Client
participant Raw as RawV2Client/AsyncRawV2Client
participant WS as WebSocket Endpoint
App->>Client: connect(model, encoding, sample_rate, keyterm=... )
Client->>Raw: connect(..., keyterm)
alt keyterm is a list
Raw->>Raw: For each item in keyterm list<br/>append query param keyterm=item
else keyterm is a string
Raw->>Raw: Append single query param keyterm=value
else keyterm is None
Raw->>Raw: Do not add keyterm param
end
Raw->>WS: Open websocket with query params
WS-->>Raw: Connection established
Raw-->>Client: Socket client instance
Client-->>App: Socket client instance
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/deepgram/listen/v2/raw_client.py (1)
92-96
: Broaden to sequences (not just list) and skip empty entries (optional).Current check only accepts list; tuples/other sequences won’t work. Also, empty strings become
keyterm=
. Consider accepting any non-string Sequence and skipping empties.Apply this minimal change in both sync and async blocks:
- if isinstance(keyterm, list): + from collections.abc import Sequence # at top of file + if isinstance(keyterm, Sequence) and not isinstance(keyterm, (str, bytes)): for kt in keyterm: - query_params = query_params.add("keyterm", kt) + if kt: # skip empty strings + query_params = query_params.add("keyterm", kt) else: query_params = query_params.add("keyterm", keyterm)Optional: enforce the documented 100‑char limit per keyterm with a simple length check before adding.
Also applies to: 197-201
src/deepgram/listen/v2/client.py (1)
104-108
: Accept any non-string sequence and ignore empty values (optional).Parity with raw client suggestion: support tuples/other sequences and avoid sending
keyterm=
for empty strings.Apply in both sync and async connect paths:
- if keyterm is not None: - if isinstance(keyterm, list): + if keyterm is not None: + from collections.abc import Sequence # at top of file + if isinstance(keyterm, Sequence) and not isinstance(keyterm, (str, bytes)): for kt in keyterm: - query_params = query_params.add("keyterm", kt) + if kt: + query_params = query_params.add("keyterm", kt) else: query_params = query_params.add("keyterm", keyterm)Optionally validate each keyterm length (<=100) per docs.
Also applies to: 220-224
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/deepgram/listen/v2/client.py
(6 hunks)src/deepgram/listen/v2/raw_client.py
(6 hunks)
🔇 Additional comments (2)
src/deepgram/listen/v2/raw_client.py (1)
34-34
: Keyterm type/doc update looks good.Union[str, List[str]] matches the intended multi-keyterm support; doc aligns.
Also applies to: 58-60
src/deepgram/listen/v2/client.py (1)
46-46
: Keyterm type/doc update acknowledged.Public surface now supports single or multiple keyterms; consistent with raw client.
Also applies to: 70-72
https://github.com/orgs/deepgram/discussions/1422#discussioncomment-14619854
Summary by CodeRabbit
New Features
Documentation