Skip to content

feat: use deepgram-sdk for the Deepgram WebSocket bridge - #8

Open
GregHolmes wants to merge 8 commits into
mainfrom
feat/use-deepgram-sdk
Open

GregHolmes wants to merge 8 commits into
mainfrom
feat/use-deepgram-sdk

Conversation

@GregHolmes

@GregHolmes GregHolmes commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

What

Migrate the Deepgram-facing Voice Agent bridge in starter/consumers.py to the official deepgram-sdk.

Follow-up fixes

  • Pin deepgram-sdk to >=7.7.0,<8.0.0 because this bridge imports SDK internals.
  • Forward FunctionCallResponse, KeepAlive, UpdateListen, UpdateThink, and InjectAgentMessage through typed SDK senders.
  • Preserve unmodeled Agent events as JSON instead of replacing them with an Unknown frame.
  • Add a regression test proving API-error authorization headers cannot reach browser-safe error text.

Verified

  • python manage.py check passes.
  • python -m unittest discover -s tests passes.
  • A live function-call round trip remains required before merge.

@GregHolmes GregHolmes self-assigned this Jul 30, 2026
GregHolmes and others added 2 commits August 14, 2026 16:07
A deepgram-sdk ApiError stringifies its request headers, which include
Authorization: Token <api-key>. The agent error paths forwarded str(e) to
the browser and logs, so a failed Deepgram connect could leak the API key.
Route every Deepgram error path through _safe_error_detail(), which exposes
only the HTTP status (ApiError) or the exception type name. Mirrors the fix
already applied to the flux / live-transcription / live-tts starters.
@GregHolmes
GregHolmes marked this pull request as ready for review September 7, 2026 11:13

@dg-coreylweathers dg-coreylweathers left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What this PR does

It replaces the hand-rolled websockets pass-through in starter/consumers.py with the official Python SDK, parsing each browser JSON control message into the SDK's typed model and calling the matching send_* method. Later commits route error paths through a sanitizer so the ApiError string form, which carries the Authorization: Token <key> header, never reaches the browser or the log, and add forwarding for FunctionCallResponse, KeepAlive, UpdateListen, UpdateThink and InjectAgentMessage.

The key-leak fix is real and I verified it live. One defect in the new typed conversion blocks merge.

What I checked

  • Does UpdateListen in its documented shape survive the conversion? No. starter/consumers.py:163-166 converts the browser message with construct_type(AgentV1UpdateListen, data). In SDK 7.8.1 the listen provider is a union keyed on a version field, and construct_type skips the validator that fills version in when the browser omits it, so the provider resolves to None.

    Expected: {"type":"UpdateListen","listen":{"provider":{"type":"deepgram","model":"nova-3"}}} reaches Deepgram unchanged and Deepgram answers ListenUpdated. I confirmed live that the same message with "version":"v1" added does return ListenUpdated.

    Observed: the message captured at a stub upstream is {"type": "UpdateListen", "listen": {"provider": null}}, and against the live API the browser receives {"type":"Error","code":"UNPARSABLE_CLIENT_MESSAGE",...} followed by the socket closing with code 3000. The conversation is over. The pass-through this PR replaces forwarded the message intact, so mid-call listen updates are a regression.

    Recommended fix, default version from the model name before constructing:

    elif msg_type == "UpdateListen":
        provider = (data.get("listen") or {}).get("provider")
        if isinstance(provider, dict) and "version" not in provider:
            provider["version"] = "v2" if str(provider.get("model", "")).startswith("flux") else "v1"
        await self.connection.send_update_listen(
            construct_type(type_=AgentV1UpdateListen, object_=data)
        )

    Do not switch to a validated parse instead: the SDK's own fallback labels nova-3 as version: "v2", which the live API rejects with INVALID_SETTINGS ("Model must have exactly 3 parts separated by hyphens"). The robust alternative is to forward the browser dict for UpdateListen without going through the model at all, plus a wire-level test asserting the provider survives.

  • Would the new test have caught this? No. tests/test_safe_error_detail.py:55 replaces construct_type with an identity lambda, so it stubs out the exact layer where the defect lives and every control message passes regardless of what the SDK puts on the wire. Remove the patch and assert on the captured payload, for example calls[2][1].dict()["listen"]["provider"]["model"] == "nova-3".

  • Is the key leak closed? Yes, verified live: the browser frames and the server log carry only an HTTP status.

Also worth fixing

  • AGENTS.md documents an UpdateSpeak shape that ends the call. The "Live Updates" example { "type": "UpdateSpeak", "model": "aura-2-luna-en" } (around line 120) returns UNPARSABLE_CLIENT_MESSAGE from the live API and closes the session, while the shipped frontend's nested {"type":"UpdateSpeak","speak":{"provider":{...}}} returns SpeakUpdated. Pre-existing text, but this PR edits AGENTS.md and the bridge now forwards the message unchanged.
  • AGENTS.md still calls the backend a pure WebSocket proxy; it is now a typed dispatcher. Name the nine browser message types that are forwarded and say anything else is dropped with a log line, and add the DEEPGRAM_BASE_URL row that _build_client() reads to the environment table.

Smaller items: an unknown message type is now dropped silently at starter/consumers.py:184 where the old proxy let Deepgram answer with an Error; starter/consumers.py:67 renders every non-ApiError as "Failed to connect to Deepgram" even mid-call; and the SDK adds "version": "v1" to agent.listen.provider in Settings when the browser omits it, which the live API accepts but is worth a comment so the next reader knows the wire message is not byte-identical to the browser's.

Note for whoever merges: this repo auto-deploys to Fly on push to main.

@dg-coreylweathers dg-coreylweathers left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What this PR does

The backend of this starter sits between the browser and Deepgram's Voice Agent API, passing audio and control messages in both directions. This PR rewrites it to use Deepgram's official Python package instead of a hand-written websocket relay, so each browser message is parsed into a typed object and sent through a matching method. The two commits from 2026-09-16 close both blockers from the last round.

What I checked

  • Does a mid-call UpdateListen in the documented browser shape still work? Yes, fixed. starter/consumers.py:174-183 now fills in the version field from the model name. Live against production: nova-3 goes out as version: "v1" and Deepgram answers ListenUpdated; flux-general-en goes out as version: "v2" and also answers ListenUpdated. Last round this message ended the call with UNPARSABLE_CLIENT_MESSAGE.
  • Would the test catch it now? Yes. construct_type is no longer patched anywhere; tests/test_safe_error_detail.py:80-108 drives a real AsyncV1SocketClient and asserts the exact bytes for both models.
  • Does the sample.env line for the new DEEPGRAM_BASE_URL setting work? No — see below.
  • Does a wrong API key leak the key? No. Live fake-key probe: the exception prints Authorization: Token [REDACTED] and the browser is told only "Deepgram rejected the connection (HTTP 401)". The >=7.7.0 floor is doing real work — the same probe on 7.6.0 prints the real key, so don't lower it.
  • Everything else passed: python manage.py check, all 5 tests (python:3.12 container, resolved deepgram-sdk 7.9.0), deploy/Dockerfile builds, the corrected nested UpdateSpeak example returns SpeakUpdated live, and known Agent events survive re-serialization with no fields dropped.

What to fix

🚫 Blocking — the DEEPGRAM_BASE_URL example in sample.env cannot connect. sample.env:6 ships wss://agent.deepgram.com/v1/agent/converse, but the SDK appends /v1/agent/converse itself, so the path doubles. Verified in production and staging: that value returns HTTP 404 and the browser is told "Deepgram rejected the connection (HTTP 404)", which points a developer at their API key rather than the URL. The value is the host only:

# Deepgram Agent API endpoint override (for example, a staging host)
# DEEPGRAM_BASE_URL=wss://agent.deepgram.com

Please also add "(host only, no path)" to the AGENTS.md:178 row — the sibling django-live-transcription#8 documents this correctly at its AGENTS.md:146.

Should fix

  • AGENTS.md:201 says python -m unittest discover -s tests, which reports "Ran 1 test … FAILED" because deepgram.toml:13-17 installs into venv/. Use ./venv/bin/python -m unittest discover -s tests, which runs all 5 and passes.
  • starter/consumers.py:73 reads the SDK's private connection._websocket, which carries no promise inside the <8.0.0 range — a fresh install already resolves to 7.9.0. Narrowing to <7.10.0 matches what's been tested.

Three nits are in the full review: an unknown message type is dropped with no reply to the browser (starter/consumers.py:201); AGENTS.md:115 lists only nova-3/nova-2 though the code has a flux-* path; and the Settings message Deepgram receives gains a version field the browser never sent.

What still needs you

Merging pushes to main, which deploys to Fly.io via .github/workflows/deploy.yml.

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