psa-systems/lets-chat

Watch

Fork 0

STT cost/load control: bounded concurrency, rate limits, scope gating (LC-592) #579

Merged

longjacksonle merged 1 commit from feat/lc592-stt-load-control into main

2026-07-21 17:21:54 +02:00

longjacksonle commented

2026-07-21 16:43:25 +02:00

Owner

Closes LC-592, and with it the LC-589 transcription-hardening epic (LC-590 reliability, LC-591 accuracy, LC-593 providers, LC-592 cost/load).

What changed

Three controls, deliberately separate because they bound different things:

Concurrency - a process-wide semaphore with LETS_CHAT_STT_WORKERS permits (default 2), held across the read and the transcribe. Protects a CPU-bound self-hosted engine, where the common deployment is whisper.cpp on the same box as lets-chat.
Rate - LETS_CHAT_STT_RATE_GLOBAL (30/min) and LETS_CHAT_STT_RATE_ROOM (10/min). Concurrency alone does not bound cost on a metered engine (two workers can bill all day), so this is a separate axis. Reuses the existing RateLimits on AppState via two new RateLimitKind variants, so it needed no new shared state.
Scope - LETS_CHAT_STT_SCOPE = both (default) / voice / clips / none, so an operator can keep cheap voice notes and drop the expensive video-clip path. Gates stored attachments only; none still leaves live call captions working.

The main deviation, and why

The ticket specced an mpsc worker pool with over-cap jobs left pending for a sweeper to re-enqueue. I asked and we went with a semaphore instead. It needs no AppState field - which would have meant a mechanical one-line edit to the ~117 sites that build an AppState by hand, for a value identical at every one of them - and it has no overflow case at all, so the pending fallback and its polling sweeper both disappear rather than needing to be written, tested, and maintained.

Refusal behaviour differs by path, on purpose

A rate-limited voice message is marked failed, which surfaces LC-590's Retry control once the window rolls over: visible and recoverable rather than silently absent. It is deliberately not left pending - with no queue behind it, nothing would ever drain that, and the UI would show "Transcribing..." forever.

A call clip sheds instead: it takes the permit with try_acquire rather than waiting. A caption is latency-bound and worthless once late, the next clip is already 5 seconds away, and queueing behind a batch of long voice notes would deliver stale captions and hold the engine longer. Both refusals answer non-2xx, which LC-590 already renders as the caption warning.

A gated-out attachment is left completely untouched - no status, so no Retry control offering to do the thing the operator just switched off. Policy is not failure.

Testing

just check clean; full suite green at 185 test binaries (184 plus the new one).

The LC-592 tests live in their own test binary. The gating tests mutate LETS_CHAT_STT_SCOPE, a process-global that scope() reads on every transcription, so sharing a process with transcripts.rs would let a gating test switch transcription off underneath a concurrently-running test that expects it on. Within the file every test takes the lock, including the ones that only read - the rate and concurrency tests failed outright until they did, which is worth knowing if you add one.

MockSttClient gained an optional per-call delay plus in-flight tracking. Without it an instant mock can never have two calls overlap, so max_concurrent() <= WORKERS would have been vacuously true and the concurrency bound would have had no test at all.

All three controls verified non-vacuous: I removed the scope gate, the rate check, and the permit in one pass and confirmed exactly the four intended tests failed - with default_scope_transcribes_both_kinds correctly still passing, since removing the gate should not change default behaviour.

🤖 Generated with Claude Code

https://claude.ai/code/session_019xQaQQJyeJX7ZS9aVECXwF

Closes LC-592, and with it the LC-589 transcription-hardening epic (LC-590 reliability, LC-591 accuracy, LC-593 providers, LC-592 cost/load). ## What changed Three controls, deliberately separate because they bound different things: - **Concurrency** - a process-wide semaphore with `LETS_CHAT_STT_WORKERS` permits (default 2), held across the read and the transcribe. Protects a CPU-bound self-hosted engine, where the common deployment is whisper.cpp on the same box as lets-chat. - **Rate** - `LETS_CHAT_STT_RATE_GLOBAL` (30/min) and `LETS_CHAT_STT_RATE_ROOM` (10/min). Concurrency alone does *not* bound cost on a metered engine (two workers can bill all day), so this is a separate axis. Reuses the existing `RateLimits` on `AppState` via two new `RateLimitKind` variants, so it needed no new shared state. - **Scope** - `LETS_CHAT_STT_SCOPE` = `both` (default) / `voice` / `clips` / `none`, so an operator can keep cheap voice notes and drop the expensive video-clip path. Gates stored attachments only; `none` still leaves live call captions working. ## The main deviation, and why The ticket specced an mpsc worker pool with over-cap jobs left `pending` for a sweeper to re-enqueue. **I asked and we went with a semaphore instead.** It needs no `AppState` field - which would have meant a mechanical one-line edit to the ~117 sites that build an `AppState` by hand, for a value identical at every one of them - and it has no overflow case at all, so the `pending` fallback and its polling sweeper both disappear rather than needing to be written, tested, and maintained. ## Refusal behaviour differs by path, on purpose A rate-limited **voice message** is marked `failed`, which surfaces LC-590's Retry control once the window rolls over: visible and recoverable rather than silently absent. It is deliberately *not* left `pending` - with no queue behind it, nothing would ever drain that, and the UI would show "Transcribing..." forever. A **call clip** sheds instead: it takes the permit with `try_acquire` rather than waiting. A caption is latency-bound and worthless once late, the next clip is already 5 seconds away, and queueing behind a batch of long voice notes would deliver stale captions *and* hold the engine longer. Both refusals answer non-2xx, which LC-590 already renders as the caption warning. A gated-out attachment is left completely untouched - no status, so no Retry control offering to do the thing the operator just switched off. Policy is not failure. ## Testing `just check` clean; full suite green at 185 test binaries (184 plus the new one). The LC-592 tests live in **their own test binary**. The gating tests mutate `LETS_CHAT_STT_SCOPE`, a process-global that `scope()` reads on every transcription, so sharing a process with `transcripts.rs` would let a gating test switch transcription off underneath a concurrently-running test that expects it on. Within the file *every* test takes the lock, including the ones that only read - the rate and concurrency tests failed outright until they did, which is worth knowing if you add one. `MockSttClient` gained an optional per-call delay plus in-flight tracking. Without it an instant mock can never have two calls overlap, so `max_concurrent() <= WORKERS` would have been vacuously true and the concurrency bound would have had no test at all. All three controls verified non-vacuous: I removed the scope gate, the rate check, and the permit in one pass and confirmed exactly the four intended tests failed - with `default_scope_transcribes_both_kinds` correctly still passing, since removing the gate should not change default behaviour. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_019xQaQQJyeJX7ZS9aVECXwF

longjacksonle added 1 commit

2026-07-21 16:43:25 +02:00

feat(stt): bound STT concurrency, rate, and scope (LC-592)

check-secrets / Nosey parker (push) Successful in 6s

Details

check-secrets / Kingfisher (push) Successful in 5s

Details

check-secrets / TruffleHog (push) Successful in 5s

Details

check-secrets / Kingfisher (pull_request) Successful in 4s

Details

Check / clippy + fmt + tests (pull_request) Successful in 13m27s

Details

check-secrets / Nosey parker (pull_request) Successful in 5s

Details

check-secrets / TruffleHog (pull_request) Successful in 8s

Details

Create release / Create release from merged PR (pull_request) Has been skipped

Details

a4b430f6e9

Enabling STT sent every voice message, every video clip, and all call audio to the operator's endpoint with nothing bounding any of it: each send spawned its own unbounded task and call clips ran inline once per 5 seconds per speaker. This adds three controls, kept separate because they bound genuinely different things.

Concurrency: a process-wide semaphore with `LETS_CHAT_STT_WORKERS` permits (default 2), held across the read and the transcribe, so at most N engine calls are ever in flight. That is what protects a CPU-bound self-hosted engine, where the common deployment is whisper.cpp on the same box as lets-chat.

Rate: `LETS_CHAT_STT_RATE_GLOBAL` (default 30/min) and `LETS_CHAT_STT_RATE_ROOM` (default 10/min) as fixed-window counters. Concurrency alone does not bound cost on a metered engine - two workers can bill all day - so these are a separate axis. The global counter is checked first so a server already at its ceiling does not also burn the room's smaller allowance on a submission it is going to refuse. Both reuse the existing `RateLimits` on `AppState` via two new `RateLimitKind` variants, so no new shared state was needed.

Scope: `LETS_CHAT_STT_SCOPE` selects `both` (default, the pre-LC-592 behaviour), `voice`, `clips`, or `none`, letting an operator keep cheap voice notes and drop the expensive video-clip path. It gates stored attachments only; call captions are unaffected, so `none` still leaves live transcription working.

The ticket specified an mpsc worker pool with over-cap jobs left `pending` for a sweeper to re-enqueue. A semaphore was chosen instead: it needs no `AppState` field (which would have meant a mechanical edit to the ~117 sites that build one by hand for a value identical at every one), and it has no overflow case at all, so the `pending` fallback and its polling sweeper both disappear rather than needing to be written and maintained.

Behaviour on refusal differs by path, deliberately. A rate-limited voice message is marked `failed`, which surfaces LC-590's Retry control once the window rolls over - visible and recoverable rather than silently absent. It is not left `pending`, because with no queue behind it nothing would ever drain it. A call clip instead SHEDS: it takes the permit with `try_acquire` rather than waiting, because a caption is latency-bound and worthless once late, the next clip is already 5 seconds away, and queueing behind a batch of long voice notes would deliver stale captions while holding the engine longer. Both refusals answer non-2xx, which LC-590 already renders as the caption warning.

A gated-out attachment is left completely untouched - no status, so no Retry control offering to do the thing the operator just switched off. Policy is not failure.

Tests live in their own binary: the gating tests mutate `LETS_CHAT_STT_SCOPE`, a process-global that `scope()` reads on every transcription, so sharing a process with `transcripts.rs` would let one test switch transcription off underneath another. Within the file every test takes the lock, including the ones that only read - the rate and concurrency tests failed outright until they did. `MockSttClient` gained an optional delay plus in-flight tracking, because an instant mock can never overlap and would have made the concurrency assertion vacuously true. All three controls were verified non-vacuous by removing them and confirming exactly the four intended tests fail, with the default-behaviour test correctly still passing.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019xQaQQJyeJX7ZS9aVECXwF

longjacksonle scheduled this pull request to auto merge when all checks succeed

2026-07-21 16:56:56 +02:00