Usability and test infrastructure updates.
This commit is contained in:
1 parent
99b1732003
commit
7040f031a6
12 files changed
+549
-127
No files matched your search
@@ -836,3 +836,72 @@ baked-in models, either accept runtime downloads for the extra ones (point
|
||||
`FASTER_WHISPER_DOWNLOAD_ROOT` at `/app/data` for those, per the README's
|
||||
override note) or extend the Dockerfile's pre-download `RUN` line —
|
||||
re-measure and re-document the image size either way.
|
||||
|
||||
## D-032 — Server-live WebSocket brought in line with the HTTP engine's DI/validation/test conventions
|
||||
**Status:** Accepted (2026-07-29)
|
||||
D-019's server-engine live transcription (`GET /api/transcribe/live` →
|
||||
`src/server/live.ts`) had landed and was wired end-to-end in `App.tsx`, but
|
||||
three things about it lagged the standard the rest of M5 set with
|
||||
`src/server/transcribe.ts`:
|
||||
1. **Not injectable.** `live.ts` imported the real `sidecar` singleton
|
||||
directly and called it from inside the `LiveTranscriber`'s `transcribe`
|
||||
closure, unlike `createTranscribeHandlers(deps)`, whose whole point (per
|
||||
its own comment) is that `Sidecar` is "injectable — tests use a fake".
|
||||
There was no way to unit-test the WebSocket bridge without a real Python
|
||||
sidecar.
|
||||
2. **Unvalidated query params.** `POST /api/transcribe`'s `parseParams`
|
||||
validates `language` against `LANGUAGE_RE` and `task` against
|
||||
`"transcribe" | "translate"` before it ever reaches the sidecar protocol.
|
||||
The WS upgrade route in `src/index.ts` skipped both checks —
|
||||
`url.searchParams.get("task") || undefined` forwarded an arbitrary string
|
||||
straight into `LiveSessionData.task` and on to the Python sidecar's
|
||||
JSON-lines protocol. Also skipped: the upfront `sidecar.status()` check the HTTP
|
||||
route makes before doing any work (D-013's "degrade gracefully" —
|
||||
opening a socket that can only report unavailability after the first
|
||||
segment reaches the sidecar is a worse failure mode than a prompt 503).
|
||||
3. **Zero test coverage.** No `tests/server-live.*` file existed; every
|
||||
other stateful server module in the app (`transcribe.ts`, `auth.ts`,
|
||||
`db.ts`) has one.
|
||||
**Fix:**
|
||||
1. `src/server/transcribe.ts` exports `parseLanguageAndTask(url)` (extracted
|
||||
from `parseParams`, same validation, same 400-text-on-string-return
|
||||
contract); `src/index.ts`'s `GET /api/transcribe/live` case now calls it
|
||||
and 400s on an invalid value, then calls `sidecar.status()` and 503s with
|
||||
the reason before upgrading — matching `POST /api/transcribe` exactly.
|
||||
2. `src/server/live.ts` exports `createLiveWebSocket(deps: { sidecar:
|
||||
Sidecar })`; `liveWebSocket` (the instance `src/index.ts` wires into
|
||||
`Bun.serve`) is just `createLiveWebSocket({ sidecar })` against the real
|
||||
singleton. `LiveSessionData.user`/`task` are now typed `SessionUser`/
|
||||
`WhisperTask` instead of `any`/`string | undefined` (task always has a
|
||||
value now that the WS route defaults it like the HTTP route does), which
|
||||
let the pre-existing `options.websocket = liveWebSocket as any` cast in
|
||||
`src/index.ts` drop to a plain assignment — it was papering over the
|
||||
untyped `user`/`task` fields, not a real incompatibility with `Bun.serve`'s
|
||||
generic `websocket` option.
|
||||
3. `tests/server-live.test.ts` exercises `createLiveWebSocket` against a fake
|
||||
`Sidecar` (same shape as `tests/transcribe.test.ts`'s): a committed
|
||||
segment writes exactly its own PCM bytes to the temp file and cleans up
|
||||
after, model/language/task pass through, a sidecar error surfaces via the
|
||||
`update` message's `error` field instead of crashing the session, a
|
||||
`{"type":"stop"}` frame drains and closes with a `result` message, and a
|
||||
malformed text frame is ignored. `tests/transcribe.test.ts` gained direct
|
||||
cases for `parseLanguageAndTask`.
|
||||
**Incidental correctness fix found while rewriting the `transcribe` closure:**
|
||||
the original code wrote `new Uint8Array(audio.buffer as ArrayBuffer)` to the
|
||||
temp file — the whole underlying `ArrayBuffer`, ignoring `audio.byteOffset`/
|
||||
`audio.byteLength`. Every real caller today happens to pass a fresh
|
||||
`Float32Array` from `lib/live.ts`'s `concat()` helper (always offset 0, full
|
||||
length), so this never manifested, but `LiveTranscribeFn`'s type signature
|
||||
(`(audio: Float32Array) => ...`) makes no such promise to callers. Restored
|
||||
the original's `audio.byteOffset, audio.byteLength` slice (which the
|
||||
pre-refactor code already had — it would have been an actual regression to
|
||||
drop it during the DI rewrite).
|
||||
**Consequence:** the server-live path now has the same testability and
|
||||
input-validation posture as the HTTP path; no behavior change for a valid
|
||||
client, since `App.tsx` already only ever sends catalog model ids and
|
||||
`"transcribe" | "translate"` task values.
|
||||
**Alternatives rejected:** leaving `live.ts` untested and testing only
|
||||
through a real spawned sidecar (matches `createSidecar`'s own integration
|
||||
tests, but that suite already covers the sidecar process itself — the gap
|
||||
was specifically the WebSocket/`LiveTranscriber` glue, which needs no Python
|
||||
process to exercise).
|
||||
Reference in new issue
Block a user