From 1805ce8e67881b75eac7b6f8cb2030e88d8718a3 Mon Sep 17 00:00:00 2001 From: Jacob Lasky Date: Sat, 25 Jul 2026 13:21:37 -0400 Subject: [PATCH] fix(params): gate params by mode per Deepgram's docs, and report real errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live-testing the keyterm fix against production turned up four separate defects, all of them the same shape: the app sends something Deepgram refuses, and the user is told nothing useful. **Mode gating was materially wrong.** STREAMING_ONLY and BATCH_ONLY were partial. Deepgram's docs carry a machine-readable capability matrix — each STT feature page includes `stt-stream-unavailable` / `stt-batch-unavailable` markers — and transcribing it adds filler_words, measurements, utt_split and detect_language to BATCH_ONLY, and channels, encoding and sample_rate to STREAMING_ONLY. This matters because a batch-only param sent to a stream is SILENTLY IGNORED: the feature you believe you are testing was never applied, and the run still looks valid. Same failure mode as the keyterms bug. **The UI's own defaults produced a live 400.** sample_rate and channels render as 0 when unset and were forwarded verbatim; Deepgram returns 400 for `sample_rate=0`. A test actually asserted this was correct ("callers are responsible for omitting defaults") — no caller did. Fixed with an explicit ZERO_MEANS_UNSET set rather than a blanket drop-all-zeroes rule, because `endpointing=0` is meaningful: it disables endpointing. **Errors were replaced with a link to MDN.** The batch handler returned str(httpx.HTTPStatusError), i.e. "Client error '400 Bad Request' for url ... For more information check: ". That names neither the parameter nor the reason, in an app whose entire purpose is explaining why Deepgram rejected a request. _clean_error now surfaces Deepgram's response body (err_msg/err_code). Response bodies are safe to show; the existing header stripping, which keeps the server's API key out of responses, is unchanged and now has a test. **An immediate stream close looked like a clean shutdown.** Deepgram refuses an unsupported param on a WebSocket by completing the handshake and then closing with code 1000 and no reason, so the user saw only "received 1000 (OK); then sent 1000 (OK)". _clean_error now explains that a reasonless close means a rejected parameter and tells the user to re-run in batch mode, where Deepgram states the actual reason. Batch-only controls (Filler Words, Utterances, Paragraphs) are now hidden while streaming instead of offered and silently ignored. Live verification against production, real Deepgram key: batch keyterms=[perineorrhaphy] -> "a perineorrhaphy" (correct) batch no keyterms (control) -> "a perineurope" (wrong) That is the delta the earlier -rrhaphy CSVs could never show, since both of their arms ran with no keyterms applied. alternatives=2 nova-3 -> 400 alternatives=2 base -> 200 alternatives=2 nova-2 -> 400 alternatives=2 enhanced -> 200 `alternatives` is MODEL-dependent, not mode-dependent, in both batch and streaming. It is deliberately NOT mode-gated and NOT removed: it is valid on legacy models, which replicating a legacy customer config requires. An earlier draft of this change had it batch-only, which the model sweep disproved; the test now pins that distinction so it is not "corrected" back. UI artifact: scripts/probe_ui.py gains --mode and --expect-removed, splits "hidden from the user" from "gone entirely", and asserts on VISIBLE labels rather than DOM presence, since a mode-gated control is still in the DOM. Verified in both modes: the three controls are hidden in streaming and present in batch, no console errors. 112 passed, 1 skipped. --- app.py | 49 ++++++++++++++++- scripts/probe_ui.py | 104 ++++++++++++++++++++++++++++-------- stt/options.py | 49 +++++++++++++++-- templates/index.html | 13 +++-- tests/test_options.py | 66 ++++++++++++++++++++--- tests/test_wire_contract.py | 77 +++++++++++++++++++++++++- 6 files changed, 317 insertions(+), 41 deletions(-) diff --git a/app.py b/app.py index 823be59..5390f18 100644 --- a/app.py +++ b/app.py @@ -324,7 +324,50 @@ def _safe_temp_path(filename: str) -> Path: app = socketio.ASGIApp(sio, fastapi_app) def _clean_error(e: Exception) -> str: - """Strip SDK request headers (including auth token) from Deepgram exception messages.""" + """Turn a Deepgram failure into the most specific message we can safely show. + + DO NOT weaken the header stripping below. SDK exception messages embed the + full request, Authorization header included, so returning str(e) verbatim + would put the server's API key in an HTTP response body. + + For httpx failures we substitute Deepgram's RESPONSE body, which is where the + actual reason lives. str(HTTPStatusError) is only + Client error '400 Bad Request' for url '...' + For more information check: https://developer.mozilla.org/... + which names neither the offending parameter nor the reason, and an MDN link + to "what is a 400" is worse than useless in a diagnostic tool: the whole + point of this app is to tell someone WHY Deepgram rejected their request. + A response body is safe to surface — credentials travel in request headers. + """ + if isinstance(e, httpx.HTTPStatusError): + status = e.response.status_code + try: + body = e.response.json() + detail = body.get("err_msg") or body.get("error") or body.get("message") + code = body.get("err_code") + if detail: + return f"Deepgram {status}: {detail}" + (f" ({code})" if code else "") + return f"Deepgram {status}: {json_mod.dumps(body)[:500]}" + except ValueError: + text = (e.response.text or "").strip() + return f"Deepgram {status}: {text[:500]}" if text else f"Deepgram {status}" + + if isinstance(e, websockets.exceptions.ConnectionClosed): + # Deepgram rejects an unsupported parameter on a STREAM by completing the + # handshake and then closing with code 1000 and NO reason, so websockets + # reports only "received 1000 (OK); then sent 1000 (OK)". That names + # neither the parameter nor the cause, and it reads like a clean shutdown + # rather than a rejection. Verified live: alternatives=2 on nova-3 closes + # this way, while the same request in batch mode returns a 400. + return ( + f"Deepgram closed the stream immediately without transcribing ({e}). " + "A close with no reason is what an unsupported parameter looks like on " + "a stream — most often a parameter the chosen model does not accept " + "(e.g. alternatives>1 on nova-*, which is valid on base and enhanced). " + "Re-run the same parameters in batch mode: there Deepgram returns a " + "400 stating the actual reason." + ) + msg = str(e) m = re.search(r'status_code:\s*(\d+),\s*body:\s*(.+)$', msg, re.DOTALL) if m: @@ -696,7 +739,9 @@ async def _batch_pipeline(): return JSONResponse({"batch": batch_result, "streaming": stream_result}) except httpx.HTTPStatusError as e: - return JSONResponse({"error": str(e)}, status_code=e.response.status_code) + # _clean_error, never str(e): it surfaces Deepgram's reason and strips + # request headers. This handler used str(e) and returned an MDN link. + return JSONResponse({"error": _clean_error(e)}, status_code=e.response.status_code) except Exception as e: return JSONResponse({"error": _clean_error(e)}, status_code=500) diff --git a/scripts/probe_ui.py b/scripts/probe_ui.py index 303bd7c..661f95f 100644 --- a/scripts/probe_ui.py +++ b/scripts/probe_ui.py @@ -12,7 +12,12 @@ DEEPGRAM_API_KEY=probe-key uv run uvicorn app:app --port 8899 # Terminal 2 uv run python scripts/probe_ui.py - uv run python scripts/probe_ui.py --expect-absent Keywords --expect-present Keyterms + uv run python scripts/probe_ui.py --expect-removed Keywords --expect-present Keyterms + uv run python scripts/probe_ui.py --mode streaming \ + --expect-absent "Filler Words" Utterances Paragraphs + +Exits non-zero when an expectation is unmet, so it works as a gate rather than +just a screenshot printer. Requires the browser binary once: `uv run playwright install chromium`. @@ -30,7 +35,18 @@ ARTIFACTS = Path(__file__).resolve().parent / "probe-artifacts" -async def probe(url: str, expect_present: list[str], expect_absent: list[str]) -> int: +def _param_key(label: str) -> str: + """UI label -> Alpine params key. "Filler Words" -> "filler_words".""" + return label.strip().lower().replace(" ", "_") + + +async def probe( + url: str, + expect_present: list[str], + expect_absent: list[str], + expect_removed: list[str], + mode: str | None, +) -> int: ARTIFACTS.mkdir(exist_ok=True) failures: list[str] = [] @@ -58,19 +74,36 @@ async def probe(url: str, expect_present: list[str], expect_absent: list[str]) - await page.wait_for_timeout(1200) # Expand every collapsible param section, or a field can be "absent" - # only because its accordion happened to be shut. - await page.evaluate("""() => { + # only because its accordion happened to be shut. Some params are + # mode-gated (batch-only features are hidden while streaming), so --mode + # picks which surface is being asserted. + await page.evaluate( + """(m) => { const d = Alpine.$data(document.querySelector('[x-data]')); + if (m) d.mode = m; Object.keys(d.sections || {}).forEach(k => d.sections[k] = true); - }""") - await page.wait_for_timeout(700) - - labels = await page.eval_on_selector_all( - ".field-label", "els => els.map(e => e.textContent.trim())" - ) - bindings = await page.eval_on_selector_all( - "[x-model]", "els => els.map(e => e.getAttribute('x-model'))" + }""", + mode, ) + await page.wait_for_timeout(700) + if mode: + print(f"mode: {mode}") + + # VISIBLE labels only, and both label flavours (field rows use + # .field-label, checkboxes use