From 09d04f79f875fc80bacda4962b50b2e17d6fa545 Mon Sep 17 00:00:00 2001 From: Vortiago Date: Mon, 28 Sep 2026 22:22:11 +0200 Subject: [PATCH 1/2] chore: ralph(357-qwen) green progress at iteration 2 model=koishi/qwen3.8-flash-next-mtp gate='python3 -m pytest tests --ignore=tests/e2e && ruff check tapscribe tests && ruff format --check tapscribe tests' --- CONTEXT.md | 8 ++ start.ps1 | 29 ++++--- start.sh | 28 ++++--- tapscribe/__main__.py | 16 +++- tapscribe/bringup_defaults.py | 92 ++++++++++++++++++++++ tests/test_bringup_defaults.py | 137 +++++++++++++++++++++++++++++++++ tests/test_install_matrix.py | 15 ++++ tests/test_preflight.py | 22 +++++- 8 files changed, 323 insertions(+), 24 deletions(-) create mode 100644 tapscribe/bringup_defaults.py create mode 100644 tests/test_bringup_defaults.py diff --git a/CONTEXT.md b/CONTEXT.md index c48bc3f4..d5b3228c 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -17,6 +17,14 @@ Verb/noun split: **the Recorder** writes **a recording**, one WAV per [Utterance](#utterance) per speaker. `recording_enabled` gates new recordings only; live transcription is independent of it. +## Bring-up defaults + +The launch values the start scripts hand the Recorder: bind host, recorder +port, live port, live model, language hint. They have one Python-side owner +(`tapscribe/bringup_defaults.py`) that the scripts read at bring-up, because a +bash and a PowerShell copy of the same values drift silently. An `SX_*` +environment variable overrides each one, and empty counts as unset. + ## Transcriber The protocol for "something that can transcribe one WAV": diff --git a/start.ps1 b/start.ps1 index c0c86175..b3cfe69a 100644 --- a/start.ps1 +++ b/start.ps1 @@ -107,17 +107,26 @@ if (-not (Test-Path ".tapscribe-install.json") -and -not $NonInteractive) { & python -m tapscribe.preflight # --- Configuration ---------------------------------------------------------- -$Model = if ($env:SX_MODEL) { $env:SX_MODEL } else { "tiny.en" } -$LangCode = if ($env:SX_LANG) { $env:SX_LANG } else { "en" } -$PortRec = if ($env:SX_PORT_REC) { $env:SX_PORT_REC } else { "8001" } -$PortWlk = if ($env:SX_PORT_WLK) { $env:SX_PORT_WLK } else { "" } -if ($env:SX_HOST) { - $BindHost = $env:SX_HOST -} elseif ($Lan) { - $BindHost = "0.0.0.0" -} else { - $BindHost = "localhost" +# The bring-up values come from `tapscribe.bringup_defaults` so start.ps1 and +# start.sh cannot re-derive them in parallel (#357). Fatal by design, unlike +# the preflight above: without the launch config there is nothing to launch. +$BringupArgs = @() +if ($Lan) { $BringupArgs += "--lan" } +$BringupLines = @(& python -m tapscribe.bringup_defaults @BringupArgs) +if ($LASTEXITCODE -ne 0) { + Write-Error "[start] Could not read bring-up defaults; aborting." + exit 1 +} +$Bringup = @{} +foreach ($line in $BringupLines) { + $key, $value = $line -split "=", 2 + $Bringup[$key] = $value } +$BindHost = $Bringup["HOST"] +$PortRec = $Bringup["PORT_REC"] +$PortWlk = $Bringup["PORT_WLK"] +$Model = $Bringup["MODEL"] +$LangCode = $Bringup["LANG"] $ExtraArgs = @() if ($NoMlx) { $ExtraArgs += "--no-mlx" } diff --git a/start.sh b/start.sh index bbd29dfd..54161ca1 100755 --- a/start.sh +++ b/start.sh @@ -200,16 +200,26 @@ fi python -m tapscribe.preflight || true # --- Configuration ---------------------------------------------------------- -MODEL="${SX_MODEL:-tiny.en}" -LANG="${SX_LANG:-en}" -PORT_WLK="${SX_PORT_WLK:-}" -PORT_REC="${SX_PORT_REC:-8001}" - +# The bring-up values come from `tapscribe.bringup_defaults` so start.sh and +# start.ps1 cannot re-derive them in parallel (#357). Fatal by design, unlike +# the preflight above: without the launch config there is nothing to exec. +LAN_FLAG=() if [ "$LAN" -eq 1 ]; then - HOST="${SX_HOST:-0.0.0.0}" -else - HOST="${SX_HOST:-localhost}" + LAN_FLAG=(--lan) fi +BRINGUP=$(python -m tapscribe.bringup_defaults "${LAN_FLAG[@]}") || { + echo "[start] Could not read bring-up defaults; aborting." >&2 + exit 1 +} +while IFS='=' read -r _key _val; do + case "$_key" in + HOST) HOST="$_val" ;; + PORT_REC) PORT_REC="$_val" ;; + PORT_WLK) PORT_WLK="$_val" ;; + MODEL) MODEL="$_val" ;; + LANG) LANG="$_val" ;; + esac +done <<< "$BRINGUP" LAN_IP="" if command -v ipconfig >/dev/null 2>&1; then @@ -258,7 +268,7 @@ if [ "$BROWSER_SETUP" -eq 1 ]; then fi echo " Live channel $LIVE_LABEL" echo " Backend $BACKEND_LABEL" -echo " Initial model $MODEL (lang=$LANG; change from the dashboard or via SX_MODEL=…)" +echo " Initial model $MODEL (lang=$LANG; change from the dashboard)" echo "" python -m tapscribe \ diff --git a/tapscribe/__main__.py b/tapscribe/__main__.py index 59fd518c..8e0aaa6b 100644 --- a/tapscribe/__main__.py +++ b/tapscribe/__main__.py @@ -13,7 +13,7 @@ import uvicorn -from . import config, install_target +from . import bringup_defaults, config, install_target from .app import app from .live import LiveConfig from .recorder import Recorder @@ -30,16 +30,24 @@ def build_parser() -> argparse.ArgumentParser: prog="python -m tapscribe", description="TapScribe — local-first transcription recorder + dashboard.", ) - p.add_argument("--host", default="localhost", help="Bind address. Use 0.0.0.0 to expose on LAN.") + # Bring-up defaults are read from `bringup_defaults` (#357): the start scripts + # print the same values, so a launch that skips argparse cannot disagree. + p.add_argument( + "--host", default=bringup_defaults.HOST, help="Bind address. Use 0.0.0.0 to expose on LAN." + ) # Defaulted FROM config, which `main()` then stamps back onto it: one declaration of the # port, rather than a literal here that a launch skipping argparse would disagree with. p.add_argument("--port", type=int, default=config.PORT) p.add_argument( "--live-model", - default="tiny.en", + default=bringup_defaults.MODEL, help="WhisperLiveKit model name (tiny.en, small.en, large-v3, ...). Changeable from the dashboard.", ) - p.add_argument("--live-language", default="en", help="WhisperLiveKit language hint (en, no, auto, ...)") + p.add_argument( + "--live-language", + default=bringup_defaults.LANG, + help="WhisperLiveKit language hint (en, no, auto, ...)", + ) p.add_argument( "--live-host", default=None, diff --git a/tapscribe/bringup_defaults.py b/tapscribe/bringup_defaults.py new file mode 100644 index 00000000..aa7301d9 --- /dev/null +++ b/tapscribe/bringup_defaults.py @@ -0,0 +1,92 @@ +"""Bring-up defaults — the one Python owner of the values the start scripts launch with. + +`start.sh` and `start.ps1` each re-derived the same five launch values — bind +host, recorder port, ephemeral live port, initial live model, language hint — +and the `SX_*` precedence chain above them, once per language. Two owners of +one rule drift silently, so both scripts now run `python -m +tapscribe.bringup_defaults` and parse the five `KEY=value` lines it prints, +and `__main__.build_parser` imports the same constants so the recorder's own +argparse defaults cannot disagree with a scripted launch (#357). + +`resolve` is pure — an env mapping and the `--lan` flag go in, one +`BringupConfig` comes out — following `preflight.plan_steps`: the whole +precedence rule lives here and is testable without a shell. + +Stdlib-only at import, like its bring-up siblings (the one intra-package +import, `config`, is itself stdlib-only), so `-m` works against a venv that +holds nothing but pip, from the repo-root cwd the scripts cd to. A value +containing a newline is truncated by the scripts' line parsers — pathological +for model ids, ports and hosts, and accepted. +""" + +from __future__ import annotations + +import argparse +import os +from collections.abc import Mapping +from dataclasses import dataclass + +from tapscribe import config + +#: Initial live Whisper model; changeable from the dashboard. +MODEL = "tiny.en" +#: Live language hint. +LANG = "en" +#: Bind host: loopback by default, all interfaces under `--lan`. +HOST = "localhost" +HOST_LAN = "0.0.0.0" +#: Empty = ephemeral — the recorder picks a free live port at spawn. +PORT_WLK = "" +#: `config.py` owns the one declaration of the recorder port; this is its str form. +PORT_REC = str(config.PORT) + + +@dataclass(frozen=True) +class BringupConfig: + """The five launch values, resolved. All `str` — they cross a shell boundary.""" + + host: str + port_rec: str + port_wlk: str + model: str + lang: str + + +def resolve(env: Mapping[str, str], *, lan: bool = False) -> BringupConfig: + """Resolve the bring-up values: a set, non-empty `SX_*` var beats the + default, and `SX_HOST` beats `--lan`.""" + return BringupConfig( + host=env.get("SX_HOST") or (HOST_LAN if lan else HOST), + port_rec=env.get("SX_PORT_REC") or PORT_REC, + port_wlk=env.get("SX_PORT_WLK") or PORT_WLK, + model=env.get("SX_MODEL") or MODEL, + lang=env.get("SX_LANG") or LANG, + ) + + +def main(argv: list[str] | None = None) -> int: + p = argparse.ArgumentParser( + prog="python -m tapscribe.bringup_defaults", + description="Print TapScribe's bring-up launch values as KEY=value lines.", + ) + p.add_argument( + "--lan", + action="store_true", + help="Bind host: 0.0.0.0 instead of localhost.", + ) + args = p.parse_args(argv) + + cfg = resolve(os.environ, lan=args.lan) + for key, value in ( + ("HOST", cfg.host), + ("PORT_REC", cfg.port_rec), + ("PORT_WLK", cfg.port_wlk), + ("MODEL", cfg.model), + ("LANG", cfg.lang), + ): + print(f"{key}={value}") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/test_bringup_defaults.py b/tests/test_bringup_defaults.py new file mode 100644 index 00000000..7ad6c727 --- /dev/null +++ b/tests/test_bringup_defaults.py @@ -0,0 +1,137 @@ +"""RED contract for `tapscribe.bringup_defaults` — one Python owner of the launch values (#357). + +`start.sh` and `start.ps1` each re-derived the same five bring-up values — +bind host, recorder port, ephemeral live port, initial live model, language +hint — plus the `SX_*` precedence chain above them, once per language. A bash +`:-` and a PowerShell truthiness test are two owners of one rule, which is +how they drift. Both scripts now read the values from `python -m +tapscribe.bringup_defaults`, and `__main__.build_parser` imports the same +constants, so a launch that skips the scripts cannot disagree either. + +The expected values are literals from the scripts' own header docs +(`start.sh`'s "Configurable via env vars" block), not imported from the +module: test 1 is the contract, and the sharing tests pin that everything +else reads the same source. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +from tapscribe import bringup_defaults +from tapscribe.bringup_defaults import BringupConfig, resolve + +REPO_ROOT = Path(__file__).resolve().parent.parent +_START_SCRIPTS = [REPO_ROOT / "start.sh", REPO_ROOT / "start.ps1"] +_SX_VARS = ("SX_HOST", "SX_PORT_REC", "SX_PORT_WLK", "SX_MODEL", "SX_LANG") + + +def _clear_sx_env(monkeypatch) -> None: + for name in _SX_VARS: + monkeypatch.delenv(name, raising=False) + + +# --- resolve(): the precedence rule ------------------------------------------ + + +def test_defaults_are_the_documented_bring_up_values(): + """The documented bring-up defaults: localhost, 8001, an ephemeral live + port, tiny.en, en. Literals, not module constants — this is the contract + the module must keep.""" + assert resolve({}, lan=False) == BringupConfig( + host="localhost", port_rec="8001", port_wlk="", model="tiny.en", lang="en" + ) + + +def test_lan_makes_the_bind_host_all_interfaces(): + assert resolve({}, lan=True).host == "0.0.0.0" + + +def test_env_vars_beat_every_default(): + env = { + "SX_HOST": "10.0.0.5", + "SX_PORT_REC": "9001", + "SX_PORT_WLK": "9100", + "SX_MODEL": "small.en", + "SX_LANG": "no", + } + assert resolve(env, lan=True) == BringupConfig( + host="10.0.0.5", port_rec="9001", port_wlk="9100", model="small.en", lang="no" + ) + + +def test_sx_host_beats_lan(): + """Both scripts' current rule: an explicit host wins over the LAN flag.""" + assert resolve({"SX_HOST": "10.0.0.5"}, lan=True).host == "10.0.0.5" + + +def test_blank_env_vars_fall_back_to_defaults(): + """Empty means unset in both scripts today (bash `:-`, PowerShell + truthiness) — `SX_MODEL=` must not launch the recorder with no model.""" + assert resolve(dict.fromkeys(_SX_VARS, ""), lan=True) == BringupConfig( + host="0.0.0.0", port_rec="8001", port_wlk="", model="tiny.en", lang="en" + ) + + +# --- main(): the wire format the scripts parse -------------------------------- + + +def test_main_prints_key_value_lines(monkeypatch, capsys): + """Five KEY=value lines in the fixed order both scripts' parsers read.""" + _clear_sx_env(monkeypatch) + + assert bringup_defaults.main([]) == 0 + + out = capsys.readouterr().out + parsed = dict(line.split("=", 1) for line in out.strip().splitlines()) + assert list(parsed) == ["HOST", "PORT_REC", "PORT_WLK", "MODEL", "LANG"] + assert parsed["HOST"] == "localhost" + + +def test_main_lan_flag_switches_the_host_default(monkeypatch, capsys): + _clear_sx_env(monkeypatch) + + bringup_defaults.main(["--lan"]) + + assert "HOST=0.0.0.0" in capsys.readouterr().out.splitlines() + + +# --- the sharing: no second copy of the values anywhere ----------------------- + + +def test_recorder_cli_defaults_share_the_source(monkeypatch): + """`build_parser` must read the module's constants, not restate them: + patch the constants and the parser's defaults move with them. (The port + pairs through `config.PORT`, which `PORT_REC` is derived from.)""" + from tapscribe.__main__ import build_parser + + monkeypatch.setattr(bringup_defaults, "HOST", "patched-host") + monkeypatch.setattr(bringup_defaults, "MODEL", "patched-model") + monkeypatch.setattr(bringup_defaults, "LANG", "patched-lang") + + args = build_parser().parse_args([]) + + assert args.host == "patched-host" + assert args.live_model == "patched-model" + assert args.live_language == "patched-lang" + assert str(args.port) == bringup_defaults.PORT_REC + + +def test_start_scripts_never_restate_bring_up_values(): + """The drift gate: the scripts read the values, they do not restate them. + + Header prose keeps its `SX_*` usage docs, so comment lines are dropped + before the scan; live code must name neither the values nor the variables, + and must invoke the shared module.""" + for script in _START_SCRIPTS: + code = "\n".join( + line + for line in script.read_text(encoding="utf-8").splitlines() + if not line.lstrip().startswith("#") + ) + for token in ("8001", "tiny.en", "SX_"): + assert token not in code, f"{script.name} still restates {token!r} in live code" + assert re.search(r"python -m tapscribe\.bringup_defaults", code), ( + f"{script.name} does not invoke the shared bring-up defaults" + ) diff --git a/tests/test_install_matrix.py b/tests/test_install_matrix.py index 71710fea..3bda4529 100644 --- a/tests/test_install_matrix.py +++ b/tests/test_install_matrix.py @@ -24,6 +24,8 @@ from conftest import atomic_extras +from tapscribe import preflight + REPO_ROOT = Path(__file__).resolve().parent.parent INSTALL_MATRIX_YML = REPO_ROOT / ".github" / "workflows" / "install-matrix.yml" @@ -40,3 +42,16 @@ def test_install_matrix_families_are_valid_pyproject_extras(): # optional-dependencies and names it on failure — the exact check a # family whose extra was silently removed (#263: `canary`) needs. atomic_extras(family) + + +def test_install_matrix_wheel_index_matches_preflight(): + r"""The workflow hand-mirrors the prebuilt llama-cpp wheel index the + preflight passes (its own comment says "Mirror start.sh / start.ps1's"), + but nothing tied the two. `preflight.LLAMA_CPP_WHEEL_INDEX` is the one + declaration; a bumped index must not land in only the workflow or only + the preflight (#357). The `[^\s)]` stops before the bash `)` closing the + array append.""" + workflow_text = INSTALL_MATRIX_YML.read_text() + m = re.search(r"--extra-index-url\s+([^\s)]+)", workflow_text) + assert m, "couldn't find the `--extra-index-url` in install-matrix.yml" + assert m.group(1) == preflight.LLAMA_CPP_WHEEL_INDEX diff --git a/tests/test_preflight.py b/tests/test_preflight.py index 16850215..18a3a259 100644 --- a/tests/test_preflight.py +++ b/tests/test_preflight.py @@ -24,7 +24,7 @@ import pytest -from tapscribe import preflight +from tapscribe import preflight, runtime_probe from tapscribe.preflight import Step, plan_steps _WHEEL_MISSING: frozenset[str] = frozenset() @@ -166,6 +166,26 @@ def test_summarize_extra_is_requested_by_name(): assert any(a.endswith("[summarize]") for a in argv), argv +def test_summarize_probe_agrees_with_resolve_local_backend(): + """`summarize_probe_module` re-derives the routing that + `catalog.resolve_local_backend` owns, from a (system, machine) pair rather + than the live probe — nothing tied the two, so a routing change could move + one and leave the other reinstalling on every boot. Pin both answers on + the two archetypes the mirror keys on.""" + from tapscribe.summarizers import catalog + + try: + runtime_probe.set_available_backends_for_testing(frozenset({"cpu", "mlx"})) + assert catalog.resolve_local_backend() == "mlx" + assert preflight.summarize_probe_module("Darwin", "arm64") == "mlx_lm" + + runtime_probe.set_available_backends_for_testing(frozenset({"cpu"})) + assert catalog.resolve_local_backend() == "gguf" + assert preflight.summarize_probe_module("Linux", "x86_64") == "llama_cpp" + finally: + runtime_probe.set_available_backends_for_testing(None) + + def test_llama_cpp_install_uses_the_prebuilt_wheel_index(): """llama-cpp-python builds from source by default, which needs cmake + a C++ toolchain. A Bundle operator has neither, so the maintainer's prebuilt CPU From 98046056c955f10064651a44ff6acec091cc71f4 Mon Sep 17 00:00:00 2001 From: Vortiago Date: Tue, 29 Sep 2026 07:22:14 +0200 Subject: [PATCH 2/2] refactor(bringup): one Python owner of the start scripts' launch defaults start.sh and start.ps1 no longer derive the host, ports, model and language each on their own. Both run `python -m tapscribe.bringup_defaults` and parse its KEY=value lines. `__main__.build_parser` imports the same constants, so a bare `python -m tapscribe` launch gets the same values. Closes #357 --- .github/workflows/install-matrix.yml | 8 +- CONTEXT.md | 9 +- start.ps1 | 14 ++- start.sh | 39 ++++--- tapscribe/__main__.py | 16 +-- tapscribe/bringup_defaults.py | 51 ++++---- tapscribe/preflight.py | 2 +- tests/conftest.py | 10 ++ tests/test_bringup_defaults.py | 169 ++++++++++++++++++++++----- tests/test_install_matrix.py | 20 ++-- tests/test_preflight.py | 30 +++-- tests/test_summarizers_local.py | 8 -- 12 files changed, 245 insertions(+), 131 deletions(-) diff --git a/.github/workflows/install-matrix.yml b/.github/workflows/install-matrix.yml index 165ad873..747c5dc2 100644 --- a/.github/workflows/install-matrix.yml +++ b/.github/workflows/install-matrix.yml @@ -117,10 +117,10 @@ jobs: - name: Install tapscribe + ${{ matrix.family }} extra # `summarize` on non-macOS pulls llama-cpp-python, which builds from # source by default (needs cmake + a C++ toolchain — see the "Verify - # cmake on PATH" step above). Mirror start.sh / start.ps1's own - # SUMMARIZE_PIP_ARGS logic and pass the maintainer's prebuilt - # CPU-wheel index so this job matches what an operator's bring-up - # actually runs, not a naive install. `shell: bash` so the same + # cmake on PATH" step above). Pass the maintainer's prebuilt CPU-wheel + # index that `tapscribe.preflight.LLAMA_CPP_WHEEL_INDEX` declares, so + # this job matches what an operator's bring-up actually runs, not a + # naive install. tests/test_install_matrix.py pins the two equal. `shell: bash` so the same # conditional runs on the Windows runner too (default shell there is # pwsh). shell: bash diff --git a/CONTEXT.md b/CONTEXT.md index d5b3228c..0839531b 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -20,10 +20,11 @@ only; live transcription is independent of it. ## Bring-up defaults The launch values the start scripts hand the Recorder: bind host, recorder -port, live port, live model, language hint. They have one Python-side owner -(`tapscribe/bringup_defaults.py`) that the scripts read at bring-up, because a -bash and a PowerShell copy of the same values drift silently. An `SX_*` -environment variable overrides each one, and empty counts as unset. +port, live port, live model, language hint. They have one Python-side owner, +`tapscribe/bringup_defaults.py`: the scripts run `python -m +tapscribe.bringup_defaults` at bring-up and parse the `KEY=value` lines it +prints. An `SX_*` environment variable overrides each one, and empty counts as +unset. The module docstring has the precedence rule and the wire format. ## Transcriber diff --git a/start.ps1 b/start.ps1 index b3cfe69a..7208e318 100644 --- a/start.ps1 +++ b/start.ps1 @@ -9,6 +9,14 @@ # .\start.ps1 -NoAuth # disable dashboard auth + /tap token gate (DEV ONLY) # .\start.ps1 -Tls # serve https:// + wss:// (auto self-signed) # .\start.ps1 -NonInteractive # install the saved/default selection in-terminal (no browser) +# +# Configurable via env vars (resolved by `python -m tapscribe.bringup_defaults`): +# SX_HOST bind address (default localhost, 0.0.0.0 under -Lan; a set +# SX_HOST beats -Lan) +# SX_PORT_WLK WhisperLiveKit port (default: ephemeral) +# SX_PORT_REC recorder port (default 8001) +# SX_MODEL initial live Whisper model (default tiny.en; switch live from dashboard) +# SX_LANG language hint (default en) [CmdletBinding()] param( @@ -107,9 +115,9 @@ if (-not (Test-Path ".tapscribe-install.json") -and -not $NonInteractive) { & python -m tapscribe.preflight # --- Configuration ---------------------------------------------------------- -# The bring-up values come from `tapscribe.bringup_defaults` so start.ps1 and -# start.sh cannot re-derive them in parallel (#357). Fatal by design, unlike -# the preflight above: without the launch config there is nothing to launch. +# `tapscribe.bringup_defaults` owns the bring-up values, and start.sh reads +# the same source. Fatal by design, unlike the preflight above: without the +# launch config there is nothing to launch. $BringupArgs = @() if ($Lan) { $BringupArgs += "--lan" } $BringupLines = @(& python -m tapscribe.bringup_defaults @BringupArgs) diff --git a/start.sh b/start.sh index 54161ca1..31acfcf7 100755 --- a/start.sh +++ b/start.sh @@ -32,7 +32,8 @@ # 6. Stream all logs to this terminal; Ctrl+C stops everything cleanly. # # Configurable via env vars: -# SX_HOST bind address (default localhost, overridden by --lan to 0.0.0.0) +# SX_HOST bind address (default localhost, 0.0.0.0 under --lan; a set +# SX_HOST beats --lan) # SX_PORT_WLK WhisperLiveKit port (default: ephemeral — WLK is internal, # only the recorder talks to it; pin only if you have a reason) # SX_PORT_REC recorder port (default 8001) @@ -200,9 +201,25 @@ fi python -m tapscribe.preflight || true # --- Configuration ---------------------------------------------------------- -# The bring-up values come from `tapscribe.bringup_defaults` so start.sh and -# start.ps1 cannot re-derive them in parallel (#357). Fatal by design, unlike -# the preflight above: without the launch config there is nothing to exec. +# `tapscribe.bringup_defaults` owns the bring-up values, and start.ps1 reads +# the same source. Fatal by design, unlike the preflight above: without the +# launch config there is nothing to exec. + +# Reads the KEY=value lines on stdin into HOST, PORT_REC, PORT_WLK, MODEL and +# LANG_CODE. LANG_CODE, not LANG: LANG is the exported locale every child reads. +parse_bringup() { + local key val + while IFS='=' read -r key val; do + case "$key" in + HOST) HOST="$val" ;; + PORT_REC) PORT_REC="$val" ;; + PORT_WLK) PORT_WLK="$val" ;; + MODEL) MODEL="$val" ;; + LANG) LANG_CODE="$val" ;; + esac + done +} + LAN_FLAG=() if [ "$LAN" -eq 1 ]; then LAN_FLAG=(--lan) @@ -211,15 +228,7 @@ BRINGUP=$(python -m tapscribe.bringup_defaults "${LAN_FLAG[@]}") || { echo "[start] Could not read bring-up defaults; aborting." >&2 exit 1 } -while IFS='=' read -r _key _val; do - case "$_key" in - HOST) HOST="$_val" ;; - PORT_REC) PORT_REC="$_val" ;; - PORT_WLK) PORT_WLK="$_val" ;; - MODEL) MODEL="$_val" ;; - LANG) LANG="$_val" ;; - esac -done <<< "$BRINGUP" +parse_bringup <<< "$BRINGUP" LAN_IP="" if command -v ipconfig >/dev/null 2>&1; then @@ -268,14 +277,14 @@ if [ "$BROWSER_SETUP" -eq 1 ]; then fi echo " Live channel $LIVE_LABEL" echo " Backend $BACKEND_LABEL" -echo " Initial model $MODEL (lang=$LANG; change from the dashboard)" +echo " Initial model $MODEL (lang=$LANG_CODE; change from the dashboard or via SX_MODEL=…)" echo "" python -m tapscribe \ --host "$HOST" \ --port "$PORT_REC" \ --live-model "$MODEL" \ - --live-language "$LANG" \ + --live-language "$LANG_CODE" \ "${EXTRA_ARGS[@]}" & REC_PID=$! diff --git a/tapscribe/__main__.py b/tapscribe/__main__.py index 8e0aaa6b..56927e71 100644 --- a/tapscribe/__main__.py +++ b/tapscribe/__main__.py @@ -30,10 +30,12 @@ def build_parser() -> argparse.ArgumentParser: prog="python -m tapscribe", description="TapScribe — local-first transcription recorder + dashboard.", ) - # Bring-up defaults are read from `bringup_defaults` (#357): the start scripts - # print the same values, so a launch that skips argparse cannot disagree. + # Defaulted from `bringup_defaults`, which the start scripts also read: a bare + # `python -m tapscribe` launch gets the same values as a scripted one. p.add_argument( - "--host", default=bringup_defaults.HOST, help="Bind address. Use 0.0.0.0 to expose on LAN." + "--host", + default=bringup_defaults.HOST, + help=f"Bind address. Use {bringup_defaults.HOST_LAN} to expose on LAN.", ) # Defaulted FROM config, which `main()` then stamps back onto it: one declaration of the # port, rather than a literal here that a launch skipping argparse would disagree with. @@ -58,7 +60,7 @@ def build_parser() -> argparse.ArgumentParser: p.add_argument( "--live-port", type=int, - default=0, + default=int(bringup_defaults.PORT_WLK or 0), help="Bind port for the live channel. 0 (default) = pick a free ephemeral " "port at spawn time. WhisperLiveKit is internal — only the recorder talks " "to it — so a stable well-known port is rarely useful, and a fixed 8000 " @@ -243,9 +245,9 @@ def main() -> None: app.state.install_spec = args.install_spec app.state.log_json = bool(args.log_json) - if args.host == "0.0.0.0": + if args.host == bringup_defaults.HOST_LAN: print( - "[tapscribe] WARNING: binding to 0.0.0.0 exposes the recorder to " + f"[tapscribe] WARNING: binding to {bringup_defaults.HOST_LAN} exposes the recorder to " "the LAN. Make sure you trust your network.", flush=True, ) @@ -285,7 +287,7 @@ def main() -> None: print(bar, flush=True) else: print("[tapscribe] WARNING: --no-auth — dashboard AND /tap are UNAUTHENTICATED.", flush=True) - if args.host == "0.0.0.0": + if args.host == bringup_defaults.HOST_LAN: print("[tapscribe] WARNING: combined with LAN binding, anyone on the", flush=True) print("[tapscribe] network can view/delete recordings. Re-enable auth.", flush=True) diff --git a/tapscribe/bringup_defaults.py b/tapscribe/bringup_defaults.py index aa7301d9..71274c0a 100644 --- a/tapscribe/bringup_defaults.py +++ b/tapscribe/bringup_defaults.py @@ -1,22 +1,14 @@ -"""Bring-up defaults — the one Python owner of the values the start scripts launch with. - -`start.sh` and `start.ps1` each re-derived the same five launch values — bind -host, recorder port, ephemeral live port, initial live model, language hint — -and the `SX_*` precedence chain above them, once per language. Two owners of -one rule drift silently, so both scripts now run `python -m -tapscribe.bringup_defaults` and parse the five `KEY=value` lines it prints, -and `__main__.build_parser` imports the same constants so the recorder's own -argparse defaults cannot disagree with a scripted launch (#357). - -`resolve` is pure — an env mapping and the `--lan` flag go in, one -`BringupConfig` comes out — following `preflight.plan_steps`: the whole -precedence rule lives here and is testable without a shell. - -Stdlib-only at import, like its bring-up siblings (the one intra-package -import, `config`, is itself stdlib-only), so `-m` works against a venv that -holds nothing but pip, from the repo-root cwd the scripts cd to. A value -containing a newline is truncated by the scripts' line parsers — pathological -for model ids, ports and hosts, and accepted. +"""Bring-up defaults: the one Python owner of the values the start scripts launch with. + +`start.sh` and `start.ps1` run `python -m tapscribe.bringup_defaults` and parse +the `KEY=value` lines it prints. `__main__.build_parser` imports the same +constants, so a bare `python -m tapscribe` launch gets the same defaults. +`resolve` is pure: an env mapping and the `--lan` flag go in, one +`BringupConfig` comes out. + +Stdlib-only at import (`config` is stdlib-only too), so `-m` works against a +venv that holds nothing but pip. The scripts' line parsers truncate a value +that contains a newline, which no model id, port or host does. """ from __future__ import annotations @@ -24,7 +16,7 @@ import argparse import os from collections.abc import Mapping -from dataclasses import dataclass +from dataclasses import asdict, dataclass from tapscribe import config @@ -35,7 +27,7 @@ #: Bind host: loopback by default, all interfaces under `--lan`. HOST = "localhost" HOST_LAN = "0.0.0.0" -#: Empty = ephemeral — the recorder picks a free live port at spawn. +#: Empty means ephemeral: the recorder picks a free live port at spawn. PORT_WLK = "" #: `config.py` owns the one declaration of the recorder port; this is its str form. PORT_REC = str(config.PORT) @@ -43,7 +35,10 @@ @dataclass(frozen=True) class BringupConfig: - """The five launch values, resolved. All `str` — they cross a shell boundary.""" + """The five launch values, resolved. All `str`, because they cross a shell boundary. + + The field order is the wire order, and a field's upper-cased name is its key. + """ host: str port_rec: str @@ -72,19 +67,13 @@ def main(argv: list[str] | None = None) -> int: p.add_argument( "--lan", action="store_true", - help="Bind host: 0.0.0.0 instead of localhost.", + help=f"Bind host: {HOST_LAN} instead of {HOST}.", ) args = p.parse_args(argv) cfg = resolve(os.environ, lan=args.lan) - for key, value in ( - ("HOST", cfg.host), - ("PORT_REC", cfg.port_rec), - ("PORT_WLK", cfg.port_wlk), - ("MODEL", cfg.model), - ("LANG", cfg.lang), - ): - print(f"{key}={value}") + for name, value in asdict(cfg).items(): + print(f"{name.upper()}={value}") return 0 diff --git a/tapscribe/preflight.py b/tapscribe/preflight.py index 2ebba27a..7216b053 100644 --- a/tapscribe/preflight.py +++ b/tapscribe/preflight.py @@ -103,7 +103,7 @@ def _module_present(name: str) -> bool: def summarize_probe_module(system: str, machine: str) -> str: """Which module proves the `[summarize]` extra is usable on this host. - Mirrors `LocalSummarizer.resolve_local_backend`'s routing: the MLX backend + Mirrors `catalog.resolve_local_backend`'s routing: the MLX backend on Apple Silicon, the GGUF/llama.cpp one everywhere else. Probing the wrong module would either reinstall on every boot or never install at all. """ diff --git a/tests/conftest.py b/tests/conftest.py index dda9f3b3..421c258f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -521,6 +521,16 @@ def fake_wlk() -> Iterator[FakeWlkThread]: wlk.stop() +@pytest.fixture +def reset_available_backends() -> Iterator[None]: + """Restore the catalog's auto-probe after a test forces the backend set, + so a forced {'mlx'}/{'cpu'} cannot leak into another test's routing.""" + from tapscribe.runtime_probe import set_available_backends_for_testing + + yield + set_available_backends_for_testing(None) + + # --------------------------------------------------------------------------- # Lightweight transcriber stub — shared across route + cache tests # --------------------------------------------------------------------------- diff --git a/tests/test_bringup_defaults.py b/tests/test_bringup_defaults.py index 7ad6c727..6e306042 100644 --- a/tests/test_bringup_defaults.py +++ b/tests/test_bringup_defaults.py @@ -1,25 +1,24 @@ -"""RED contract for `tapscribe.bringup_defaults` — one Python owner of the launch values (#357). - -`start.sh` and `start.ps1` each re-derived the same five bring-up values — -bind host, recorder port, ephemeral live port, initial live model, language -hint — plus the `SX_*` precedence chain above them, once per language. A bash -`:-` and a PowerShell truthiness test are two owners of one rule, which is -how they drift. Both scripts now read the values from `python -m -tapscribe.bringup_defaults`, and `__main__.build_parser` imports the same -constants, so a launch that skips the scripts cannot disagree either. +"""RED contract for `tapscribe.bringup_defaults`, the one Python owner of the launch values. The expected values are literals from the scripts' own header docs (`start.sh`'s "Configurable via env vars" block), not imported from the -module: test 1 is the contract, and the sharing tests pin that everything -else reads the same source. +module: the `resolve` tests are the contract. The sharing tests pin that the +recorder's argparse defaults and both start scripts read that one source, and +the round-trip tests pin that each script variable reads its matching key. """ from __future__ import annotations +import os import re +import shutil +import subprocess +import sys from pathlib import Path -from tapscribe import bringup_defaults +import pytest + +from tapscribe import bringup_defaults, config from tapscribe.bringup_defaults import BringupConfig, resolve REPO_ROOT = Path(__file__).resolve().parent.parent @@ -67,8 +66,7 @@ def test_sx_host_beats_lan(): def test_blank_env_vars_fall_back_to_defaults(): - """Empty means unset in both scripts today (bash `:-`, PowerShell - truthiness) — `SX_MODEL=` must not launch the recorder with no model.""" + """Empty means unset: `SX_MODEL=` must not launch the recorder with no model.""" assert resolve(dict.fromkeys(_SX_VARS, ""), lan=True) == BringupConfig( host="0.0.0.0", port_rec="8001", port_wlk="", model="tiny.en", lang="en" ) @@ -102,36 +100,143 @@ def test_main_lan_flag_switches_the_host_default(monkeypatch, capsys): def test_recorder_cli_defaults_share_the_source(monkeypatch): """`build_parser` must read the module's constants, not restate them: - patch the constants and the parser's defaults move with them. (The port - pairs through `config.PORT`, which `PORT_REC` is derived from.)""" + patch the constants and the parser's defaults move with them.""" from tapscribe.__main__ import build_parser monkeypatch.setattr(bringup_defaults, "HOST", "patched-host") monkeypatch.setattr(bringup_defaults, "MODEL", "patched-model") monkeypatch.setattr(bringup_defaults, "LANG", "patched-lang") + monkeypatch.setattr(bringup_defaults, "PORT_WLK", "9100") args = build_parser().parse_args([]) assert args.host == "patched-host" assert args.live_model == "patched-model" assert args.live_language == "patched-lang" - assert str(args.port) == bringup_defaults.PORT_REC + assert args.live_port == 9100 + + +def test_recorder_port_is_derived_from_config(): + """`config.PORT` owns the recorder port. `PORT_REC` is its str form, not + a second literal that happens to agree.""" + assert bringup_defaults.PORT_REC == str(config.PORT) + + +# A read of an `SX_*` variable: bash `$SX_X` / `${SX_X:-…}`, PowerShell `$env:SX_X`. +# A bare `SX_MODEL=` in banner text names the override and reads nothing. +_SX_READ = re.compile(r"\$\{?SX_|\$env:SX_", re.IGNORECASE) + + +def _live_code(script: Path) -> str: + """The script without its comment lines, which keep the `SX_*` usage docs.""" + return "\n".join( + line for line in script.read_text(encoding="utf-8").splitlines() if not line.lstrip().startswith("#") + ) -def test_start_scripts_never_restate_bring_up_values(): +@pytest.mark.parametrize("script", _START_SCRIPTS, ids=lambda p: p.name) +def test_start_scripts_never_restate_bring_up_values(script): """The drift gate: the scripts read the values, they do not restate them. + Live code names no default value, reads no `SX_*` variable, and invokes + the shared module.""" + code = _live_code(script) + for token in ("8001", "tiny.en"): + assert token not in code, f"{script.name} still restates {token!r} in live code" + assert not _SX_READ.search(code), f"{script.name} still reads an SX_* variable itself" + assert re.search(r"python -m tapscribe\.bringup_defaults", code), ( + f"{script.name} does not invoke the shared bring-up defaults" + ) + + +# --- the consumers: each script variable reads its matching key --------------- - Header prose keeps its `SX_*` usage docs, so comment lines are dropped - before the scan; live code must name neither the values nor the variables, - and must invoke the shared module.""" - for script in _START_SCRIPTS: - code = "\n".join( - line - for line in script.read_text(encoding="utf-8").splitlines() - if not line.lstrip().startswith("#") - ) - for token in ("8001", "tiny.en", "SX_"): - assert token not in code, f"{script.name} still restates {token!r} in live code" - assert re.search(r"python -m tapscribe\.bringup_defaults", code), ( - f"{script.name} does not invoke the shared bring-up defaults" - ) + +def _start_sh_parse_bringup() -> str: + body = re.search( + r"^parse_bringup\(\) \{\n.*?^\}\n", + (REPO_ROOT / "start.sh").read_text(encoding="utf-8"), + re.MULTILINE | re.DOTALL, + ) + assert body, "start.sh has no parse_bringup() function" + return body.group(0) + + +def _run_start_sh_parse(env: dict[str, str], lan: bool) -> dict[str, str]: + """Feed the real module output through start.sh's own parser and report + the five variables it sets, plus LANG, which it must leave alone.""" + names = ("HOST", "PORT_REC", "PORT_WLK", "MODEL", "LANG_CODE", "LANG") + script = ( + _start_sh_parse_bringup() + + 'parse_bringup <<< "$("$PY" -m tapscribe.bringup_defaults $LAN_FLAG)"\n' + + "".join(f'printf "{name}=%s\\n" "${name}"\n' for name in names) + ) + # An inherited HOST or MODEL would mask a key the parser failed to set. + base = {k: v for k, v in os.environ.items() if not k.startswith("SX_") and k not in names} + run_env = base | env | {"PY": sys.executable, "LAN_FLAG": "--lan" if lan else ""} + out = subprocess.run( + ["bash", "-c", script], + cwd=REPO_ROOT, + env=run_env, + capture_output=True, + text=True, + check=True, + ).stdout + return dict(line.split("=", 1) for line in out.splitlines()) + + +_needs_bash = pytest.mark.skipif( + sys.platform == "win32" or shutil.which("bash") is None, + reason="start.sh runs under a POSIX bash", +) + + +@_needs_bash +@pytest.mark.parametrize( + ("env", "lan", "expected"), + [ + ({}, False, ("localhost", "8001", "", "tiny.en", "en")), + ({}, True, ("0.0.0.0", "8001", "", "tiny.en", "en")), + ({"SX_HOST": "10.0.0.5"}, True, ("10.0.0.5", "8001", "", "tiny.en", "en")), + ( + { + "SX_PORT_REC": "9001", + "SX_PORT_WLK": "9100", + "SX_MODEL": "small.en", + "SX_LANG": "no", + }, + False, + ("localhost", "9001", "9100", "small.en", "no"), + ), + ], + ids=["defaults", "lan", "sx-host-beats-lan", "every-override"], +) +def test_start_sh_parses_every_bring_up_value(env, lan, expected): + parsed = _run_start_sh_parse(env | {"LANG": "C.UTF-8"}, lan) + + assert ( + parsed["HOST"], + parsed["PORT_REC"], + parsed["PORT_WLK"], + parsed["MODEL"], + parsed["LANG_CODE"], + ) == expected + assert parsed["LANG"] == "C.UTF-8", "start.sh overwrote the exported locale" + + +@pytest.mark.parametrize( + ("variable", "key"), + [ + ("BindHost", "HOST"), + ("PortRec", "PORT_REC"), + ("PortWlk", "PORT_WLK"), + ("Model", "MODEL"), + ("LangCode", "LANG"), + ], +) +def test_start_ps1_reads_each_variable_from_its_key(variable, key): + """A static pin, so the gate needs no PowerShell: the mapping is one line + per variable.""" + code = _live_code(REPO_ROOT / "start.ps1") + assert re.search(rf'^\${variable} = \$Bringup\["{key}"\]$', code, re.MULTILINE), ( + f'start.ps1 does not set ${variable} from $Bringup["{key}"]' + ) diff --git a/tests/test_install_matrix.py b/tests/test_install_matrix.py index 3bda4529..55b7207c 100644 --- a/tests/test_install_matrix.py +++ b/tests/test_install_matrix.py @@ -1,4 +1,5 @@ -"""Meta-test for `.github/workflows/install-matrix.yml`'s `family` axis. +"""Meta-tests for `.github/workflows/install-matrix.yml`: its `family` axis +and its llama-cpp wheel index. The workflow's whole purpose is catching `pip install -e ".[]"` regressions per model family (see the workflow's own header comment). A @@ -45,13 +46,12 @@ def test_install_matrix_families_are_valid_pyproject_extras(): def test_install_matrix_wheel_index_matches_preflight(): - r"""The workflow hand-mirrors the prebuilt llama-cpp wheel index the - preflight passes (its own comment says "Mirror start.sh / start.ps1's"), - but nothing tied the two. `preflight.LLAMA_CPP_WHEEL_INDEX` is the one - declaration; a bumped index must not land in only the workflow or only - the preflight (#357). The `[^\s)]` stops before the bash `)` closing the - array append.""" + r"""The workflow restates the prebuilt llama-cpp wheel index that + `preflight.LLAMA_CPP_WHEEL_INDEX` declares. This test ties every + `--extra-index-url` in the workflow to that one declaration, so a bumped + index cannot land in only one of them. The `[^\s)]` stops before the bash + `)` that closes the array append.""" workflow_text = INSTALL_MATRIX_YML.read_text() - m = re.search(r"--extra-index-url\s+([^\s)]+)", workflow_text) - assert m, "couldn't find the `--extra-index-url` in install-matrix.yml" - assert m.group(1) == preflight.LLAMA_CPP_WHEEL_INDEX + urls = re.findall(r"--extra-index-url\s+([^\s)]+)", workflow_text) + assert urls, "couldn't find an `--extra-index-url` in install-matrix.yml" + assert set(urls) == {preflight.LLAMA_CPP_WHEEL_INDEX}, urls diff --git a/tests/test_preflight.py b/tests/test_preflight.py index 18a3a259..01e667cd 100644 --- a/tests/test_preflight.py +++ b/tests/test_preflight.py @@ -139,13 +139,14 @@ def test_onnxruntime_repair_is_not_fatal(): ("system", "machine", "probe"), [ ("Darwin", "arm64", "mlx_lm"), + ("Darwin", "aarch64", "mlx_lm"), ("Darwin", "x86_64", "llama_cpp"), ("Linux", "x86_64", "llama_cpp"), ("Windows", "AMD64", "llama_cpp"), ], ) def test_summarize_probe_follows_the_routed_backend(system, machine, probe): - """Mirrors LocalSummarizer's resolve_local_backend: mlx_lm on Apple + """Mirrors `catalog.resolve_local_backend`: mlx_lm on Apple Silicon, llama_cpp everywhere else. Probing the wrong module would either reinstall on every boot or never install at all.""" steps = plan_steps( @@ -166,24 +167,21 @@ def test_summarize_extra_is_requested_by_name(): assert any(a.endswith("[summarize]") for a in argv), argv -def test_summarize_probe_agrees_with_resolve_local_backend(): - """`summarize_probe_module` re-derives the routing that - `catalog.resolve_local_backend` owns, from a (system, machine) pair rather - than the live probe — nothing tied the two, so a routing change could move - one and leave the other reinstalling on every boot. Pin both answers on - the two archetypes the mirror keys on.""" +def test_summarize_probe_agrees_with_resolve_local_backend(reset_available_backends): + """`summarize_probe_module` re-derives, from a (system, machine) pair, the + routing that `catalog.resolve_local_backend` owns from the live probe. The + two take different inputs, so this test pins each one's answer on the two + archetypes (Apple Silicon with MLX, a CPU-only x86 box): a routing change + to either one must update this test, which names the other.""" from tapscribe.summarizers import catalog - try: - runtime_probe.set_available_backends_for_testing(frozenset({"cpu", "mlx"})) - assert catalog.resolve_local_backend() == "mlx" - assert preflight.summarize_probe_module("Darwin", "arm64") == "mlx_lm" + runtime_probe.set_available_backends_for_testing(frozenset({"cpu", "mlx"})) + assert catalog.resolve_local_backend() == "mlx" + assert preflight.summarize_probe_module("Darwin", "arm64") == "mlx_lm" - runtime_probe.set_available_backends_for_testing(frozenset({"cpu"})) - assert catalog.resolve_local_backend() == "gguf" - assert preflight.summarize_probe_module("Linux", "x86_64") == "llama_cpp" - finally: - runtime_probe.set_available_backends_for_testing(None) + runtime_probe.set_available_backends_for_testing(frozenset({"cpu"})) + assert catalog.resolve_local_backend() == "gguf" + assert preflight.summarize_probe_module("Linux", "x86_64") == "llama_cpp" def test_llama_cpp_install_uses_the_prebuilt_wheel_index(): diff --git a/tests/test_summarizers_local.py b/tests/test_summarizers_local.py index 79efddf2..00f6c84b 100644 --- a/tests/test_summarizers_local.py +++ b/tests/test_summarizers_local.py @@ -62,14 +62,6 @@ def test_catalog_cross_module_names_are_public(): assert hasattr(catalog, name), f"catalog.{name} should be public" -@pytest.fixture -def reset_available_backends(): - """Restore the catalog's auto-probe after a test forces the backend set, - so a forced {'mlx'}/{'cpu'} can't leak into another test's routing.""" - yield - set_available_backends_for_testing(None) - - @pytest.fixture def extra_present(monkeypatch): """Pretend the `[summarize]` backend module IS importable, so a no-generate_fn