fix(params): gate params by mode per Deepgram's docs, and report real errors#9
Merged
Merged
Conversation
… errors
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: <MDN 400 page>". 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Live-testing PR #8 against production turned up four defects, all the same shape: the app sends something Deepgram refuses, and the user is told nothing useful.
Mode gating was materially wrong
STREAMING_ONLYandBATCH_ONLYwere partial. Deepgram's docs carry a machine-readable capability matrix — each STT feature page includesstt-stream-unavailable/stt-batch-unavailablemarkers — and transcribing it adds:BATCH_ONLYfiller_words,measurements,utt_split,detect_languageSTREAMING_ONLYchannels,encoding,sample_rateThis 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 — the same failure mode as the keyterms bug, which is why it's worth fixing properly rather than one param at a time.
The UI's own defaults produced a live 400
sample_rateandchannelsrender as0when unset and were forwarded verbatim:A test asserted this was correct — "callers are responsible for omitting defaults". No caller did. Fixed with an explicit
ZERO_MEANS_UNSETset rather than a blanket drop-all-zeroes rule, becauseendpointing=0is meaningful: it disables endpointing.Errors were replaced with a link to MDN
The batch handler returned
str(httpx.HTTPStatusError):That names neither the parameter nor the reason, in an app whose entire purpose is explaining why Deepgram rejected a request.
_clean_errornow surfaces Deepgram's response body (err_msg/err_code). Response bodies are safe to show — credentials travel in request headers — and the existing header stripping that keeps the server's API key out of responses is unchanged, now with a test pinning it.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:
_clean_errornow 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.Live verification
Against production with the real Deepgram key.
Keyterm prompting works, and this is the delta the void CSVs could never show:
Both arms of the earlier
-rrhaphyrun had no keyterms applied, so "keyterms don't help" was never a finding.alternativesis model-dependent, not mode-dependent:True in both batch and streaming. So it is deliberately not mode-gated and not removed — it's valid on legacy models, which replicating a legacy customer config requires. An earlier draft of this change had it batch-only; the model sweep disproved that before it shipped, and a test now pins the distinction so it doesn't get "corrected" back.
UI artifact.
scripts/probe_ui.pygains--modeand--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 withx-showfalse. Verified in both modes:112 passed, 1 skipped.