diff --git a/.gitignore b/.gitignore index ee88d90..5b5f9fb 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,7 @@ outputs/* # Generated evidence from scripts/. Regenerable; keep deliberately, not by accident. scripts/*.csv +scripts/probe-artifacts/ # Claude Code local overrides .claude/settings.local.json diff --git a/app.py b/app.py index db16442..823be59 100644 --- a/app.py +++ b/app.py @@ -1,4 +1,5 @@ import asyncio +import inspect import json as json_mod import logging import os @@ -759,12 +760,42 @@ async def transcribe(request: Request): # --- Helper functions --- +# Parameter names deepgram-sdk's connect() actually enumerates as keywords. +# Computed from the installed SDK, never hardcoded: the set grows between SDK +# releases, and a hardcoded copy silently rots into the bug below. +_SDK_CONNECT_KWARGS = frozenset( + inspect.signature(AsyncDeepgramClient(api_key="_").listen.v1.connect).parameters +) - {"self", "request_options"} + + def _params_to_sdk_kwargs(raw_params: dict) -> dict: - """Convert frontend params dict to deepgram-sdk 6.x keyword args. - model is required by connect() — default to nova-2 if not provided. + """Convert a frontend params dict into deepgram-sdk 6.x connect() arguments. + + Splits into two buckets, because the SDK enumerates only ~28 of Deepgram's + query parameters as keywords and raises TypeError on anything else: + + AsyncV1Client.connect() got an unexpected keyword argument 'keyterms' + + That killed the stream for `keyterms`, and equally for `filler_words`, + `no_delay`, `word_confidence`, `alternatives`, `diarize_version` and + `entity_prompt`, all of which are real Deepgram params the UI exposes. + Anything the SDK does not name is forwarded verbatim through + RequestOptions.additional_query_parameters, so it still reaches the wire. + + DO NOT "fix" a future occurrence by deleting the param from the UI. Add it + to nothing: the split handles unknown names automatically, and widens on its + own when a newer SDK starts naming them. + + model is required by connect(), so default it. """ - kwargs = serialize_params(raw_params, Mode.STREAMING) - kwargs.setdefault("model", "nova-2") + serialized = serialize_params(raw_params, Mode.STREAMING) + serialized.setdefault("model", "nova-2") + + kwargs = {k: v for k, v in serialized.items() if k in _SDK_CONNECT_KWARGS} + passthrough = {k: v for k, v in serialized.items() if k not in _SDK_CONNECT_KWARGS} + if passthrough: + logger.debug("params not named by the SDK, sent as query: %s", sorted(passthrough)) + kwargs["request_options"] = {"additional_query_parameters": passthrough} return kwargs diff --git a/config/defaults.json b/config/defaults.json index 276d8a5..8a411ba 100644 --- a/config/defaults.json +++ b/config/defaults.json @@ -23,7 +23,6 @@ "redact": [], "keyterms": [], "entity_prompt": "", - "keywords": "", "search": "", "replace": "", "topics": false, diff --git a/pyproject.toml b/pyproject.toml index bcd75eb..00256d5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -31,6 +31,11 @@ dev = [ # [project] dependencies: the async app must never ship the sync # websocket-client path again (see test_websocket_client_not_a_runtime_dep). "websocket-client>=1.9.0", + # scripts/probe_ui.py only. A UI change needs a real-browser artifact; a + # passing suite says nothing about the rendered DOM. The pip package is all + # CI installs — the browser binary is a separate opt-in + # (`uv run playwright install chromium`), so this does not slow the test job. + "playwright>=1.61.0", ] [tool.pytest.ini_options] diff --git a/scripts/probe_ui.py b/scripts/probe_ui.py new file mode 100644 index 0000000..303bd7c --- /dev/null +++ b/scripts/probe_ui.py @@ -0,0 +1,124 @@ +#!/usr/bin/env python3 +"""Visual/DOM probe for UI changes: proves a field rendered, or is really gone. + +WHY THIS EXISTS: a passing test suite is not evidence about the DOM. Contract +tests assert on fixtures whose shape we wrote ourselves, so they pass while the +rendered page differs. Any change to templates/index.html or static/app.js needs +an artifact from a real browser, and without a committed harness every such +change re-invents this setup from scratch. + +Usage: + # Terminal 1 + 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 + +Requires the browser binary once: `uv run playwright install chromium`. + +Writes screenshots next to this script under probe-artifacts/, which is +gitignored: attach them to the PR, do not commit them. +""" +import argparse +import asyncio +import json +import sys +from pathlib import Path + +from playwright.async_api import async_playwright + +ARTIFACTS = Path(__file__).resolve().parent / "probe-artifacts" + + +async def probe(url: str, expect_present: list[str], expect_absent: list[str]) -> int: + ARTIFACTS.mkdir(exist_ok=True) + failures: list[str] = [] + + async with async_playwright() as p: + browser = await p.chromium.launch() + page = await browser.new_page(viewport={"width": 1500, "height": 1400}) + + console_errors: list[str] = [] + page.on("console", lambda m: m.type == "error" and console_errors.append(m.text)) + page.on("pageerror", lambda e: console_errors.append(f"pageerror: {e}")) + + try: + await page.goto(url, wait_until="networkidle") + except Exception as exc: + # A raw ERR_CONNECTION_REFUSED traceback reads like a probe bug. It + # almost always means the app is not running; say so. + await browser.close() + print(f"could not load {url}: {exc}", file=sys.stderr) + print( + "\nIs the app running? Start it with:\n" + " DEEPGRAM_API_KEY=probe-key uv run uvicorn app:app --port 8899", + file=sys.stderr, + ) + return 1 + 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("""() => { + const d = Alpine.$data(document.querySelector('[x-data]')); + 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'))" + ) + params = await page.evaluate( + "() => JSON.parse(JSON.stringify(" + "Alpine.$data(document.querySelector('[x-data]')).params))" + ) + + print(f"fields rendered: {len(labels)}") + print(json.dumps(sorted(labels), indent=1)) + print(f"\nconsole errors: {console_errors or 'none'}") + if console_errors: + failures.append(f"console errors on load: {console_errors}") + + for name in expect_present: + if name not in labels: + failures.append(f"expected field {name!r} to render; it did not") + + for name in expect_absent: + # Check the label, the x-model binding, AND the live Alpine state. + # A removed label with a live binding left behind still ships the + # param, which is the failure this triple-check catches. + key = name.lower() + if name in labels: + failures.append(f"{name!r} still has a rendered label") + if f"params.{key}" in bindings: + failures.append(f"{name!r} still has an x-model binding") + if key in params: + failures.append(f"{key!r} is still in the live Alpine params state") + + await page.screenshot(path=str(ARTIFACTS / "ui_full.png"), full_page=True) + print(f"\nartifact: {ARTIFACTS / 'ui_full.png'}") + await browser.close() + + if failures: + print("\nFAILED:") + for f in failures: + print(f" - {f}") + return 1 + print("\nOK: all DOM expectations met") + return 0 + + +def main() -> int: + ap = argparse.ArgumentParser() + ap.add_argument("--url", default="http://127.0.0.1:8899/") + ap.add_argument("--expect-present", nargs="*", default=[]) + ap.add_argument("--expect-absent", nargs="*", default=[]) + a = ap.parse_args() + return asyncio.run(probe(a.url, a.expect_present, a.expect_absent)) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/test_rrhaphy_keyterms.py b/scripts/test_rrhaphy_keyterms.py index 2edc067..ced8e6d 100644 --- a/scripts/test_rrhaphy_keyterms.py +++ b/scripts/test_rrhaphy_keyterms.py @@ -4,14 +4,20 @@ Runs triplicates without keyterms, then triplicates with keyterms. Uses the /api/tts-transcribe endpoint on deepgram-python-stt.fly.dev. """ -import os -import httpx -import json import csv +import json +import os import sys from datetime import datetime from pathlib import Path +import httpx +from dotenv import load_dotenv + +# Reads APP_ACCESS_TOKEN from the repo's gitignored .env so the harness gets the +# privileged (un-rate-limited) tier. Must run before the getenv calls below. +load_dotenv() + # Output lands next to this script. DO NOT hardcode an absolute /coding path: # it pins the harness to one machine. SCRIPT_DIR = Path(__file__).resolve().parent @@ -59,8 +65,11 @@ def run_test(use_keyterms: bool) -> list[dict]: rows = [] stt_params = {"model": STT_MODEL, "smart_format": True} if use_keyterms: - # keyterms with intensifier boost - stt_params["keyterms"] = list(TERMS) + # Keyterm Prompting: bare terms, no intensifiers (that is `keywords`). + # Canonical Deepgram wire name. stt.options.PARAM_ALIASES also maps the + # UI's plural "keyterms", but be explicit here: sending the wrong name + # to the batch API is silently ignored, which invalidated an earlier run. + stt_params["keyterm"] = list(TERMS) label = "WITH keyterms" if use_keyterms else "WITHOUT keyterms" print(f"\n{'='*60}") diff --git a/scripts/test_rrhaphy_multivoice.py b/scripts/test_rrhaphy_multivoice.py index 3f71e71..1b73276 100644 --- a/scripts/test_rrhaphy_multivoice.py +++ b/scripts/test_rrhaphy_multivoice.py @@ -4,13 +4,19 @@ 10 Deepgram TTS voices × 8 terms × 3 trials × 2 conditions (no keyterms / with keyterms). Uses /api/tts-transcribe on deepgram-python-stt.fly.dev. """ -import os -import httpx import csv +import os import sys +from concurrent.futures import ThreadPoolExecutor, as_completed from datetime import datetime from pathlib import Path -from concurrent.futures import ThreadPoolExecutor, as_completed + +import httpx +from dotenv import load_dotenv + +# Reads APP_ACCESS_TOKEN from the repo's gitignored .env so the harness gets the +# privileged (un-rate-limited) tier. Must run before the getenv calls below. +load_dotenv() # Output lands next to this script. DO NOT hardcode an absolute /coding path: # it pins the harness to one machine. diff --git a/static/app.js b/static/app.js index 8f059c3..6044708 100644 --- a/static/app.js +++ b/static/app.js @@ -313,7 +313,6 @@ function appData() { redact: [], keyterms: [], entity_prompt: '', - keywords: '', search: '', replace: '', topics: false, @@ -1098,7 +1097,6 @@ function appData() { redact: [], keyterms: [], entity_prompt: '', - keywords: '', search: '', replace: '', topics: false, diff --git a/stt/options.py b/stt/options.py index f042b76..ad584eb 100644 --- a/stt/options.py +++ b/stt/options.py @@ -17,49 +17,120 @@ class Mode(str, Enum): # Params that should never be sent to Deepgram (handled by client) INTERNAL_PARAMS = {"base_url"} +# UI-internal name -> Deepgram wire name. +# +# The frontend models these as plural lists (`keyterms`, `tags`) but Deepgram's +# parameters are singular and repeated once per value (`keyterm`, `tag`). +# Translating here, at the single parameter gate, makes every call site correct +# at once. +# +# `keyterm` is KEYTERM PROMPTING (Nova-3 and Flux). It improves Keyword Recall +# Rate and takes BARE TERMS ONLY. It is NOT keyword boosting and has NO +# intensifier syntax: a value of "term:2" prompts for the literal string +# "term:2". Weighted boosting is the separate, legacy `keywords` parameter +# (Nova-2 and older), which this app deliberately does not support. +# Join words with %20 or + to prompt a multi-word phrase as one cohesive unit. +# +# This bug was live and it failed in two different ways, which is why the alias +# belongs in code and not in a comment telling people to remember: +# - Streaming raised `AsyncV1Client.connect() got an unexpected keyword +# argument 'keyterms'`, because the SDK only accepts `keyterm`. +# - Batch SILENTLY IGNORED it. Deepgram drops unknown query params without +# erroring, so a "with keyterms" experiment ran with no keyterms applied at +# all and looked like evidence that keyterm prompting does not help. +# DO NOT rename the UI field to work around this; the alias is the fix. +PARAM_ALIASES = {"keyterms": "keyterm", "tags": "tag"} + +# Wire params Deepgram accepts more than once per request (the key is repeated, +# once per value). Two sources can land on the SAME wire name: the UI's plural +# alias (`keyterms`) and a caller-supplied `extra={"keyterm": ...}`. A plain +# assignment drops one of them, and WHICH one depends on dict iteration order, +# so these are unioned instead. Deepgram repeats the key per value, so a union +# is exactly what the caller asked for. DO NOT collapse this back to an +# overwrite: silently dropping half a keyterm list is the same class of bug as +# the plural/singular mismatch above, and just as invisible in a transcript. +REPEATABLE_PARAMS = {"keyterm", "tag", "redact", "search", "replace"} + # Params a caller must never be able to set, on any endpoint. # `callback` makes Deepgram POST the finished transcript to a URL of the # caller's choosing, so on a public unauthenticated app it is a data-exfil # primitive AND a way to run arbitrary async jobs on the server's account. # DO NOT move this into INTERNAL_PARAMS: those are stripped because the client # consumes them, these are stripped because forwarding them is a vulnerability. -DENIED_PARAMS = {"callback"} +DENIED_PARAMS = { + "callback", + # Legacy Nova-2-and-older keyword BOOSTING (`keywords=TERM:INTENSIFIER`). + # Deliberately unsupported: this app targets Nova-3 and Flux, where the + # equivalent is Keyterm Prompting via `keyterm`. Keeping a Nova-2-only + # parameter in the UI invites sending it on a Nova-3 request, where Deepgram + # silently ignores it and the user concludes the feature does not work. + "keywords", +} + + +def _is_blocked(caller_name: str, wire_name: str) -> bool: + """True if a param must not reach Deepgram. + + Tests BOTH the name the caller used and the wire name it maps to. Checking + only the caller's name would let any future alias whose target is denied + smuggle that target past the gate, because the deny list is enforced on the + name that actually goes on the wire. + """ + return bool({caller_name, wire_name} & (INTERNAL_PARAMS | DENIED_PARAMS)) + + +def _put(result: dict, wire_name: str, value) -> None: + """Write one already-aliased param, unioning repeatable ones (see REPEATABLE_PARAMS).""" + if wire_name in result and wire_name in REPEATABLE_PARAMS: + existing = result[wire_name] if isinstance(result[wire_name], list) else [result[wire_name]] + incoming = value if isinstance(value, list) else [value] + # dict.fromkeys dedupes while preserving order; a repeated keyterm is + # noise on the wire, and order is what the user typed. + result[wire_name] = list(dict.fromkeys([*existing, *incoming])) + return + result[wire_name] = value def clean_params(params: dict, mode: Mode) -> dict: """ Remove internal params, mode-incompatible params, empty/falsy values, - and handle special cases (keyterms list, redact list, etc.) + and map UI-internal names to Deepgram wire names (see PARAM_ALIASES). Returns clean dict ready to send to Deepgram as query params. """ - result = {} + result: dict = {} for key, value in params.items(): - if key in INTERNAL_PARAMS or key in DENIED_PARAMS: + wire = PARAM_ALIASES.get(key, key) + if _is_blocked(key, wire): continue - if mode == Mode.STREAMING and key in BATCH_ONLY: + if mode == Mode.STREAMING and wire in BATCH_ONLY: continue - if mode == Mode.BATCH and key in STREAMING_ONLY: + if mode == Mode.BATCH and wire in STREAMING_ONLY: continue # Skip falsy values (but not 0 for numeric params, not False for booleans that are explicitly set) if value is None or value == "" or value == [] or value == {}: continue if isinstance(value, bool) and not value: continue - result[key] = value - - # Handle keyterms: list of "term" or "term:weight" strings -> repeated keyterm= params - # (handled by requests library when value is a list) - - # Handle extra params: merge into result. - # Re-apply the deny list here. This merge runs AFTER the filter loop above, - # so without it a caller smuggles a blocked param straight through as - # extra={"callback": "..."} and the deny list above does nothing. + _put(result, wire, value) + + # Handle extra params: merge into result. `extra` is the deliberate escape + # hatch for params this app does not model, so it skips the mode and falsy + # filters above — but NOT the deny list. + # + # Re-applying the deny list here is load-bearing. This merge runs AFTER the + # filter loop, so without it a caller smuggles a blocked param straight + # through as extra={"callback": "..."} and the deny list above does nothing. + # + # NOTE: Deepgram also has its own unrelated `extra` query param (arbitrary + # metadata echoed back in the response). A non-dict `extra` is left alone + # above and forwarded as that param; only a dict means "merge these". if "extra" in result and isinstance(result["extra"], dict): extra = result.pop("extra") - result.update({ - k: v for k, v in extra.items() - if k not in DENIED_PARAMS and k not in INTERNAL_PARAMS - }) + for k, v in extra.items(): + wire = PARAM_ALIASES.get(k, k) + if _is_blocked(k, wire): + continue + _put(result, wire, v) return result diff --git a/templates/index.html b/templates/index.html index 2649959..f5c7b80 100644 --- a/templates/index.html +++ b/templates/index.html @@ -1361,10 +1361,6 @@
Replace
-
-
Keywords
- -
diff --git a/tests/test_options.py b/tests/test_options.py index 9947651..ca80b48 100644 --- a/tests/test_options.py +++ b/tests/test_options.py @@ -60,7 +60,176 @@ def test_extra_dict_merged(): def test_list_values_kept(): - params = {"redact": ["pci", "ssn"], "keyterms": ["hello:2", "world"]} + # Bare terms only. "hello:2" would be a keyWORDS intensifier, which is a + # different (Nova-2-only) parameter; as a keyterm it prompts for the literal + # string "hello:2". + params = {"redact": ["pci", "ssn"], "keyterms": ["hello", "world"]} result = clean_params(params, Mode.STREAMING) assert result["redact"] == ["pci", "ssn"] - assert result["keyterms"] == ["hello:2", "world"] + # keyterms is aliased to Deepgram's singular wire name; see PARAM_ALIASES. + assert result["keyterm"] == ["hello", "world"] + + +# --------------------------------------------------------------------------- +# keyterms -> keyterm. This was a live bug that failed two different ways: +# streaming raised "unexpected keyword argument 'keyterms'", and batch silently +# ignored it so a keyterm experiment ran with no keyterms applied at all. +# --------------------------------------------------------------------------- + +def test_keyterms_is_aliased_to_the_wire_name(): + from stt.options import clean_params, Mode + out = clean_params({"model": "nova-3", "keyterms": ["alpha", "beta"]}, Mode.STREAMING) + assert "keyterms" not in out, "the UI-internal plural must never reach Deepgram" + assert out["keyterm"] == ["alpha", "beta"] + + +def test_keyterms_alias_applies_in_batch_mode_too(): + """Batch is where the wrong name was silently dropped rather than erroring.""" + from stt.options import clean_params, Mode + out = clean_params({"keyterms": ["alpha"]}, Mode.BATCH) + assert out == {"keyterm": ["alpha"]} + + +def test_keyterms_alias_applies_through_extra(): + from stt.options import clean_params, Mode + out = clean_params({"extra": {"keyterms": ["alpha"]}}, Mode.BATCH) + assert out == {"keyterm": ["alpha"]} + + +def test_keyterm_passed_directly_is_untouched(): + from stt.options import clean_params, Mode + out = clean_params({"keyterm": ["alpha"]}, Mode.STREAMING) + assert out == {"keyterm": ["alpha"]} + + +def test_query_string_repeats_keyterm_per_item(): + from stt.options import query_string, Mode + qs = query_string({"keyterms": ["one", "two words"]}, Mode.STREAMING) + assert qs.count("keyterm=") == 2 + assert "keyterms=" not in qs + + +def test_sdk_accepts_every_serialized_param_name(): + """Guards the whole class of bug: a param name the SDK rejects raises + TypeError at connect() and kills the stream. Assert every name we emit for + streaming is a real connect() keyword.""" + import inspect + from deepgram import AsyncDeepgramClient + from stt.options import serialize_params, Mode + + accepted = set(inspect.signature( + AsyncDeepgramClient(api_key="x").listen.v1.connect + ).parameters) + + import json + from pathlib import Path + defaults = json.loads( + (Path(__file__).resolve().parents[1] / "config" / "defaults.json").read_text() + ) + # Give every default a truthy value so none are dropped as falsy. + probe = {} + for k, v in defaults.items(): + if isinstance(v, bool): + probe[k] = True + elif isinstance(v, list): + probe[k] = ["x"] + elif isinstance(v, dict): + continue # `extra` is a passthrough bag, not a param itself + elif isinstance(v, (int, float)): + probe[k] = v or 1 + else: + probe[k] = "x" + + emitted = set(serialize_params(probe, Mode.STREAMING)) + + # Every emitted param must reach the wire by one of the two routes, and the + # ones the SDK does not name must go through the passthrough rather than + # being dropped or raising TypeError at connect(). + import app + built = app._params_to_sdk_kwargs(probe) + via_kwargs = set(built) - {"request_options"} + via_query = set( + built.get("request_options", {}).get("additional_query_parameters", {}) + ) + + assert via_kwargs <= accepted, f"would raise TypeError at connect(): {sorted(via_kwargs - accepted)}" + dropped = emitted - via_kwargs - via_query + assert not dropped, f"params silently dropped, never reaching Deepgram: {sorted(dropped)}" + # These are real Deepgram params the SDK does not enumerate; they must be + # routed, not lost. Regression guard for the keyterms crash class. + for name in ("filler_words", "no_delay", "word_confidence"): + assert name in via_query, f"{name} is not being forwarded" + + +def test_nova2_keywords_is_not_supported(): + """Deliberately dropped. `keywords` is Nova-2-and-older keyword BOOSTING + (`keywords=TERM:INTENSIFIER`); this app targets Nova-3 and Flux, where the + equivalent is Keyterm Prompting via `keyterm`. Leaving a Nova-2-only param + in the UI invites sending it on a Nova-3 request, where Deepgram silently + ignores it and the user concludes the feature is broken.""" + from stt.options import clean_params, Mode + for mode in (Mode.BATCH, Mode.STREAMING): + assert "keywords" not in clean_params({"keywords": "term:2"}, mode) + assert "keywords" not in clean_params({"extra": {"keywords": "term:2"}}, mode) + + +def test_keyterm_takes_bare_terms_not_intensifiers(): + """Keyterm Prompting has NO intensifier syntax. Documented so nobody + reintroduces `term:2` thinking it weights the term; it would prompt for the + literal string. Weights belong to `keywords`, which is unsupported here.""" + from stt.options import query_string, Mode + qs = query_string({"keyterms": ["perineorrhaphy"]}, Mode.STREAMING) + assert qs == "keyterm=perineorrhaphy" + + +def test_keywords_gone_from_ui_defaults(): + import json + from pathlib import Path + d = json.loads((Path(__file__).resolve().parents[1] / "config" / "defaults.json").read_text()) + assert "keywords" not in d + + +def test_alias_and_wire_name_from_two_sources_are_unioned_not_dropped(): + """A caller can reach one wire param by two names at once: the UI's plural + `keyterms` and an explicit `extra={"keyterm": ...}`. A plain assignment + dropped one of them depending on dict order, which is invisible in a + transcript. Repeatable params union instead.""" + from stt.options import clean_params, Mode + out = clean_params( + {"keyterms": ["alpha"], "extra": {"keyterm": ["beta"]}}, Mode.STREAMING + ) + assert out["keyterm"] == ["alpha", "beta"] + + # Order preserved, duplicates collapsed — a repeated keyterm is wire noise. + out = clean_params( + {"keyterms": ["alpha", "beta"], "extra": {"keyterm": "alpha"}}, Mode.STREAMING + ) + assert out["keyterm"] == ["alpha", "beta"] + + +def test_non_repeatable_collision_still_takes_one_value(): + """Only params Deepgram accepts more than once are unioned. `model` is + single-valued: unioning it would put a list where a string belongs.""" + from stt.options import clean_params, Mode + out = clean_params({"model": "nova-3", "extra": {"model": "nova-2"}}, Mode.STREAMING) + assert out["model"] == "nova-2" # extra is the escape hatch and wins + + +def test_alias_cannot_smuggle_a_denied_wire_name(): + """The deny list is enforced on the name that actually goes on the wire, not + just the name the caller typed, so a future alias pointing at a denied param + cannot bypass it. Guards the invariant, not today's alias table.""" + from stt.options import clean_params, Mode, PARAM_ALIASES, DENIED_PARAMS + import stt.options as options + + original = dict(PARAM_ALIASES) + try: + options.PARAM_ALIASES = {**original, "harmless": "callback"} + out = clean_params({"harmless": "https://evil.example/x"}, Mode.BATCH) + assert out == {}, f"denied wire name reached the request: {out}" + out = clean_params({"extra": {"harmless": "https://evil.example/x"}}, Mode.BATCH) + assert out == {}, f"denied wire name reached the request via extra: {out}" + finally: + options.PARAM_ALIASES = original + + assert "callback" in DENIED_PARAMS diff --git a/tests/test_security.py b/tests/test_security.py index 36f2843..ffd2569 100644 --- a/tests/test_security.py +++ b/tests/test_security.py @@ -153,8 +153,13 @@ def test_require_auth_without_a_token_is_a_hard_error(): "REQUIRE_AUTH": "true", "DEEPGRAM_API_KEY": "test-key", "PYTHONPATH": str(Path(__file__).resolve().parents[1]), + # Set EMPTY rather than popping it. app.py calls load_dotenv(), which + # would repopulate a missing key from the repo's gitignored .env and + # make this assertion depend on the developer's local file. dotenv does + # not override a key already present in the environment, even when its + # value is the empty string, so this is hermetic either way. + "APP_ACCESS_TOKEN": "", } - env.pop("APP_ACCESS_TOKEN", None) proc = subprocess.run( [sys.executable, "-c", "import app"], capture_output=True, text=True, env=env, @@ -206,10 +211,16 @@ def _reset_rate_limits(): app._anon_global_hits.clear() -def test_everyone_is_privileged_when_no_token_is_configured(): - """Local dev and this suite run with the limits off.""" - assert app.APP_ACCESS_TOKEN == "" +def test_everyone_is_privileged_when_no_token_is_configured(monkeypatch): + """With no token configured, the limits are off. + + DO NOT assert on the ambient app.APP_ACCESS_TOKEN here. app.py calls + load_dotenv() at import, so a developer with APP_ACCESS_TOKEN in the repo's + gitignored .env would fail this test for no real reason. Pin the value. + """ + monkeypatch.setattr(app, "APP_ACCESS_TOKEN", "") assert app._is_privileged("") is True + assert app._is_privileged("anything") is True req = _FakeRequest() assert app._enforce_access(req) is None assert req.state.privileged is True diff --git a/tests/test_wire_contract.py b/tests/test_wire_contract.py new file mode 100644 index 0000000..bbaaf64 --- /dev/null +++ b/tests/test_wire_contract.py @@ -0,0 +1,159 @@ +"""What actually arrives at Deepgram's WebSocket, asserted against a real handshake. + +Every other test in this suite checks the dict we hand to the SDK. That is the +producer half of the contract. This file checks the consumer half: it stands up a +real WebSocket server on loopback, points the SDK's environment at it, and reads +the request path the SDK actually sent. + +Why this is not redundant with test_options.py: `_params_to_sdk_kwargs` splits +params into named kwargs and `request_options.additional_query_parameters`, and +nothing in a dict-shaped assertion proves the SDK does anything with the second +bucket. It could accept the kwarg and drop it. The keyterms bug was exactly this +class of failure — the value was present in our dict and absent on the wire — and +it survived because no test ever looked at a URL. + +No Deepgram credential is needed, and none should ever be added here: the server +is ours, the handshake is local, and the assertion is on the path string. +""" +import pytest +from deepgram import AsyncDeepgramClient +from deepgram.environment import DeepgramClientEnvironment +from websockets.asyncio.server import serve + +from app import _params_to_sdk_kwargs + + +async def _handshake_path(params: dict) -> str: + """Connect the SDK to a local WS server; return the path it requested.""" + seen: list[str] = [] + + async def handler(ws): + seen.append(ws.request.path) + await ws.close() + + async with await serve(handler, "127.0.0.1", 0) as server: + port = server.sockets[0].getsockname()[1] + url = f"ws://127.0.0.1:{port}" + dg = AsyncDeepgramClient( + api_key="wire-contract-test-key", + environment=DeepgramClientEnvironment( + base=f"http://127.0.0.1:{port}", production=url, agent=url + ), + ) + try: + async with dg.listen.v1.connect(**_params_to_sdk_kwargs(params)): + pass + except TypeError: + # DO NOT swallow this. A TypeError from connect() IS the reported + # bug ("got an unexpected keyword argument 'keyterms'"): it fires + # before the handshake, so the assertion below would otherwise + # report a confusing "server never saw a handshake" and hide the + # symptom the user actually saw. + raise + except Exception: + # Anything else is the server closing on us immediately, which is + # what it is written to do. By this point the handshake, and + # therefore the path, has already happened. + pass + + assert seen, "server never saw a handshake" + return seen[0] + + +def _pairs(path: str) -> list[tuple[str, str]]: + from urllib.parse import parse_qsl, urlsplit + + return parse_qsl(urlsplit(path).query, keep_blank_values=True) + + +@pytest.mark.asyncio +async def test_sdk_named_params_reach_the_wire(): + pairs = _pairs(await _handshake_path({"model": "nova-3", "smart_format": True})) + assert ("model", "nova-3") in pairs + # Booleans must be the strings Deepgram accepts, never Python's "True". + assert ("smart_format", "true") in pairs + + +@pytest.mark.asyncio +async def test_passthrough_params_reach_the_wire(): + """The six real Deepgram params the SDK does not enumerate as keywords. + + Before the kwarg split these raised TypeError at connect() and killed the + stream. Asserting they are ON THE URL is the only proof that routing them + through additional_query_parameters actually works. + """ + params = { + "model": "nova-3", + "filler_words": True, + "no_delay": True, + "word_confidence": True, + "alternatives": 3, + "diarize_version": "v2", + "entity_prompt": "patient names", + } + pairs = _pairs(await _handshake_path(params)) + for expected in ( + ("filler_words", "true"), + ("no_delay", "true"), + ("word_confidence", "true"), + ("alternatives", "3"), + ("diarize_version", "v2"), + ("entity_prompt", "patient names"), + ): + assert expected in pairs, f"{expected[0]} never reached the wire: {pairs}" + + +@pytest.mark.asyncio +async def test_keyterm_is_singular_and_repeated_per_term(): + """The reported bug, asserted at the wire. + + `keyterms` (plural) is a UI-internal name. Deepgram's param is `keyterm`, + repeated once per term. A plural key on this URL means the alias regressed; + a single comma-joined value means list encoding regressed. + """ + pairs = _pairs( + await _handshake_path( + {"model": "nova-3", "keyterms": ["perineorrhaphy", "herniorrhaphy"]} + ) + ) + assert ("keyterm", "perineorrhaphy") in pairs + assert ("keyterm", "herniorrhaphy") in pairs + assert not any(k == "keyterms" for k, _ in pairs), f"plural leaked: {pairs}" + assert [v for k, v in pairs if k == "keyterm"] == [ + "perineorrhaphy", + "herniorrhaphy", + ] + + +@pytest.mark.asyncio +async def test_multiword_keyterm_stays_one_term(): + """A phrase must arrive as ONE keyterm value, not split into two params. + Deepgram treats a single keyterm as one cohesive unit; splitting it changes + what is being prompted for.""" + pairs = _pairs( + await _handshake_path({"model": "nova-3", "keyterms": ["cerebral palsy"]}) + ) + assert [v for k, v in pairs if k == "keyterm"] == ["cerebral palsy"] + + +@pytest.mark.asyncio +async def test_denied_params_never_reach_the_wire(): + """`callback` is an exfil primitive and `keywords` is an unsupported + Nova-2-only param. Neither may appear on the URL, including via `extra`.""" + params = { + "model": "nova-3", + "callback": "https://evil.example/collect", + "keywords": "term:2", + "extra": {"callback": "https://evil.example/collect2", "keywords": "x:3"}, + } + keys = [k for k, _ in _pairs(await _handshake_path(params))] + assert "callback" not in keys + assert "keywords" not in keys + + +@pytest.mark.asyncio +async def test_base_url_is_never_forwarded_to_deepgram(): + """`base_url` selects the host client-side. Forwarding it as a query param + would leak internal endpoint names into Deepgram's request logs.""" + path = await _handshake_path({"model": "nova-3", "base_url": "api.deepgram.com"}) + assert "base_url" not in [k for k, _ in _pairs(path)] diff --git a/uv.lock b/uv.lock index 43a936d..00d32b3 100644 --- a/uv.lock +++ b/uv.lock @@ -197,6 +197,7 @@ dependencies = [ [package.dev-dependencies] dev = [ { name = "aiohttp" }, + { name = "playwright" }, { name = "pytest" }, { name = "pytest-asyncio" }, { name = "websocket-client" }, @@ -219,6 +220,7 @@ requires-dist = [ [package.metadata.requires-dev] dev = [ { name = "aiohttp", specifier = ">=3.9.0" }, + { name = "playwright", specifier = ">=1.61.0" }, { name = "pytest", specifier = ">=8.0" }, { name = "pytest-asyncio", specifier = ">=0.23,<1.0" }, { name = "websocket-client", specifier = ">=1.9.0" }, @@ -265,6 +267,24 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/9a/9a/e35b4a917281c0b8419d4207f4334c8e8c5dbf4f3f5f9ada73958d937dcc/frozenlist-1.8.0-py3-none-any.whl", hash = "sha256:0c18a16eab41e82c295618a77502e17b195883241c563b00f0aa5106fc4eaa0d", size = 13409, upload-time = "2025-10-06T05:38:16.721Z" }, ] +[[package]] +name = "greenlet" +version = "3.5.4" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/a3/74/b13368064b09053253555d3f2839cc2684d22d5aed0d2ccffbf7a6736558/greenlet-3.5.4.tar.gz", hash = "sha256:0232ae1de90a8e07867bb127d7a6ba2301e859145489f25cda8a6096dabe1d20", size = 206538, upload-time = "2026-07-22T12:47:14.468Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/f3/04/81bd731d6d1e3a469d9a4c36f5eb069bcf0cbb2d5d342c9fec22245b91fc/greenlet-3.5.4-cp312-cp312-macosx_11_0_universal2.whl", hash = "sha256:3d66250e8b09f182ede05490998c818b5961f7a3640332d44c4927caec7bbfe4", size = 295909, upload-time = "2026-07-22T11:38:09.261Z" }, + { url = "https://files.pythonhosted.org/packages/cc/dd/f5f22903a6ae70f5ea328ed0beaec92ad903f0e3b7d2845133b354abc4b8/greenlet-3.5.4-cp312-cp312-manylinux_2_24_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:c90e930c9c192e5b3ee9fb8bcd920ea3926155e2e3ded39fc697323addecee17", size = 612011, upload-time = "2026-07-22T12:26:40.69Z" }, + { url = "https://files.pythonhosted.org/packages/8e/10/92a4a88d12b915d74ea5b6d288e4afefda4771647caa34442c156f7a454f/greenlet-3.5.4-cp312-cp312-manylinux_2_24_ppc64le.manylinux_2_28_ppc64le.whl", hash = "sha256:791fdfeeb9c6e0c7b10fa151bf110d2a6974866f13dcb5b1c7efae698245893a", size = 624299, upload-time = "2026-07-22T12:29:02.089Z" }, + { url = "https://files.pythonhosted.org/packages/6c/f9/03e26be3487c5238e81f2b84714959a86ea8515a869828cf41f4fc54b34e/greenlet-3.5.4-cp312-cp312-manylinux_2_24_s390x.manylinux_2_28_s390x.whl", hash = "sha256:b7c895310363f310361e0fe2072af85269d2a2a285cd04c0c59e79a5e3670dcf", size = 629603, upload-time = "2026-07-22T12:43:43.456Z" }, + { url = "https://files.pythonhosted.org/packages/50/6d/0b14bb9db2989f32cd9fe7f76afedea01ee8bee3f87c07e69f24adfe7e63/greenlet-3.5.4-cp312-cp312-manylinux_2_24_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:f88193799d43dbf8c8a806d6405c9c52fe2af40bf75072a606357b33cc336c7f", size = 621541, upload-time = "2026-07-22T11:51:09.464Z" }, + { url = "https://files.pythonhosted.org/packages/57/6b/7c55ca72ef80d57c16c4a55210f82582622462dc4485799a30f4ec6f3372/greenlet-3.5.4-cp312-cp312-manylinux_2_39_riscv64.whl", hash = "sha256:13b980043cb1b3134e81ea469da1250ddcc6bfe6d245bbaa59168d9cdc8f228f", size = 432554, upload-time = "2026-07-22T12:39:51.379Z" }, + { url = "https://files.pythonhosted.org/packages/48/3d/25e9a2d9eb6b2e8b7ca4e80a3a26cb887cce6c8e0a87c921164f11bc5574/greenlet-3.5.4-cp312-cp312-musllinux_1_2_aarch64.whl", hash = "sha256:b7a5f095767c4493afcd06067f2bb3b8716e3f3f9e92b99c88e7e99f885b3d4d", size = 1581444, upload-time = "2026-07-22T12:25:03.818Z" }, + { url = "https://files.pythonhosted.org/packages/b9/96/4c9bf2e2c408dcc0556edce69efa9f802e82223573c53240136a086821f1/greenlet-3.5.4-cp312-cp312-musllinux_1_2_x86_64.whl", hash = "sha256:42afdc1ab5f66da8c586c32af9224a74a706b4f0ea0dc3a4188a0860a09c65c9", size = 1645842, upload-time = "2026-07-22T11:51:12.295Z" }, + { url = "https://files.pythonhosted.org/packages/b5/41/303ecb26a3a56122c0f4d4073ee078881847bd6b6f463ae0ec57ec20223b/greenlet-3.5.4-cp312-cp312-win_amd64.whl", hash = "sha256:60149df8f462d1b230038e6590c23c3b4768bb5d6c022b3b6e82532b34b0b8a3", size = 247169, upload-time = "2026-07-22T11:38:19.893Z" }, + { url = "https://files.pythonhosted.org/packages/a4/e3/ef56864b4c35fcb3eb3b41b869f6cc46f4cd3f5e2c68e74acde8ac433951/greenlet-3.5.4-cp312-cp312-win_arm64.whl", hash = "sha256:77d6ce04fed0d9aeed42e0f37923cc43eba9b027bdd9c34546bb4ccd143d0fe0", size = 245565, upload-time = "2026-07-22T11:38:27.061Z" }, +] + [[package]] name = "h11" version = "0.16.0" @@ -365,6 +385,25 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/20/12/38679034af332785aac8774540895e234f4d07f7545804097de4b666afd8/packaging-25.0-py3-none-any.whl", hash = "sha256:29572ef2b1f17581046b3a2227d5c611fb25ec70ca1ba8554b24b0e69331a484", size = 66469, upload-time = "2025-04-19T11:48:57.875Z" }, ] +[[package]] +name = "playwright" +version = "1.61.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "greenlet" }, + { name = "pyee" }, +] +wheels = [ + { url = "https://files.pythonhosted.org/packages/44/ee/31e4e0db36588b817a10b299a0285082545fde7d36543c2abe498bb3d61a/playwright-1.61.0-py3-none-macosx_10_13_x86_64.whl", hash = "sha256:ff138c3a604f69911e9d42fd036e55c2a171e5616edf04c1e7f60a2a285540b0", size = 43421877, upload-time = "2026-06-29T10:32:48.428Z" }, + { url = "https://files.pythonhosted.org/packages/42/35/71395dd3ecc798965be4a3ef8c443217d4abca168e7cb34536304f9489e6/playwright-1.61.0-py3-none-macosx_11_0_arm64.whl", hash = "sha256:009588c2a7e499bc5a8b425b61fa65490968bbda9cd69e0cf2cff10f8304659a", size = 42205016, upload-time = "2026-06-29T10:32:52.104Z" }, + { url = "https://files.pythonhosted.org/packages/f4/44/323164cf5cd1647bdefce76ffce27651aadb959d089b48f53ea40918276e/playwright-1.61.0-py3-none-macosx_11_0_universal2.whl", hash = "sha256:9f7de4536088d12037c13a52b7ea34b59270b78926bb56935070597ffac6b1af", size = 43421884, upload-time = "2026-06-29T10:32:55.773Z" }, + { url = "https://files.pythonhosted.org/packages/ab/f8/a35bf179e4ba2522c1893635094a64e407572547bd61528820fc0abc87fe/playwright-1.61.0-py3-none-manylinux1_x86_64.whl", hash = "sha256:54f3b39f6eab832e33458c1dd7da0b5682aedab3b09ae731b5c59fa12fd2024e", size = 47421381, upload-time = "2026-06-29T10:32:59.903Z" }, + { url = "https://files.pythonhosted.org/packages/b7/eb/e3f922348ec17c315f98c463f72faa1181a1c3de0bfe31a8d2edf6561723/playwright-1.61.0-py3-none-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:93454322ade8c11d5d6c211bfd91bdfb9ffb4810e3e026371bcbc4bec1b7ee4c", size = 47120545, upload-time = "2026-06-29T10:33:03.574Z" }, + { url = "https://files.pythonhosted.org/packages/c2/a6/5be4e52b40a9c0c8a073e7c5b0785c05cf5a9ea8f8a7b5b260e32d970342/playwright-1.61.0-py3-none-win32.whl", hash = "sha256:372d55a6f1248fa1dd47599686980cb8fb5bbe6fcda59eab793eb657c11d8a9b", size = 37844841, upload-time = "2026-06-29T10:33:07.361Z" }, + { url = "https://files.pythonhosted.org/packages/6c/fd/2b78036e5fbe9d5f5645bbe08a1eac7160c51243c0093963edbcf67c35d9/playwright-1.61.0-py3-none-win_amd64.whl", hash = "sha256:35c6cc4589a5d00964a59d7b3e59641e0aac0c02f15479a7af77d20f6bc79597", size = 37844846, upload-time = "2026-06-29T10:33:10.637Z" }, + { url = "https://files.pythonhosted.org/packages/27/0d/1b0f3c4ee4eb0514bc805b5c2f9a223e5b6de4f11a926f5235d51d0fc81b/playwright-1.61.0-py3-none-win_arm64.whl", hash = "sha256:e9fcbffcf557a8620fdedd92491eb59a32d18e23d6f3b4f6214b952be324fe51", size = 33955127, upload-time = "2026-06-29T10:33:14.008Z" }, +] + [[package]] name = "pluggy" version = "1.6.0" @@ -460,6 +499,18 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/a6/53/d78dc063216e62fc55f6b2eebb447f6a4b0a59f55c8406376f76bf959b08/pydub-0.25.1-py2.py3-none-any.whl", hash = "sha256:65617e33033874b59d87db603aa1ed450633288aefead953b30bded59cb599a6", size = 32327, upload-time = "2021-03-10T02:09:53.503Z" }, ] +[[package]] +name = "pyee" +version = "13.0.1" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "typing-extensions" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/8b/04/e7c1fe4dc78a6fdbfd6c337b1c3732ff543b8a397683ab38378447baa331/pyee-13.0.1.tar.gz", hash = "sha256:0b931f7c14535667ed4c7e0d531716368715e860b988770fc7eb8578d1f67fc8", size = 31655, upload-time = "2026-02-14T21:12:28.044Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/a0/c4/b4d4827c93ef43c01f599ef31453ccc1c132b353284fc6c87d535c233129/pyee-13.0.1-py3-none-any.whl", hash = "sha256:af2f8fede4171ef667dfded53f96e2ed0d6e6bd7ee3bb46437f77e3b57689228", size = 15659, upload-time = "2026-02-14T21:12:26.263Z" }, +] + [[package]] name = "pygments" version = "2.19.2"