feat: provider-aware concurrency caps — queue behind a busy provider instead of failing against it (#356) #495

Merged

claude-bot merged 13 commits from feat/356-provider-concurrency into main 2026-09-05 03:38:32 +00:00

Conversation 0 Commits 13 Files changed 24 +3392 -124

claude-bot commented 2026-09-05 03:28:28 +00:00

Contributor

Copy link

Closes #356.

A burst of Friday-night sessions now costs latency, not failures. Each provider declares what it will actually serve at once; the Celery layer enforces it; a GM whose session lands in a busy window sees "queued behind N, about M minutes" instead of silence.

What

• Declared limits + operator override — Ollama 1, llama.cpp 2 (matches the documented --parallel 2), bundled WhisperX 1 (its v2 advertises exactly that), Anthropic/OpenAI None (rely on 429 + Retry-After). .env overrides QB_LLM_CONCURRENCY_LIMIT / QB_ASR_CONCURRENCY_LIMIT win over the declaration.
• A Redis lease semaphore (app/services/provider_slots.py, DB 0 only): leases with TTL so an OOM-killed worker cannot pin a GPU; rank taken from a monotonic admission counter rather than expiry (renewing a lease must not re-sort it behind a newcomer); rolling hold durations for the estimate; fails open (logged, degraded) if Redis is unreachable.
• Enforcement where it belongs: process_audio acquires the ASR slot where whisper_cfg is read and, if none is free, self.retry(countdown=20–40 s jittered, max_retries=240) so the worker returns to the pool; released before the LLM phase. Every LLM path acquires through one run_with_slot around the transport dispatch in generate_structured_text and _dispatch_prose with a bounded in-task wait (ceiling 900 s), because retrying a whole task after transcription would repeat the ASR. Request-path callers (/ask, admin Test, the two Workbench generators that run inside an HTTP request) never wait: a RequestScopeMiddleware contextvar marks request scope so a browser cannot hang on the queue — the rule is a property of where code runs, not of each call site.
• 429 handling: release the slot, honour Retry-After (clamped to 10 min), bounded backoff, typed error afterwards. An ASR 429 re-queues the session rather than failing it (_AsrShouldRequeue), and a re-queued run writes no session_usage row — a charge for work that never happened, rewritten every 20 s, would be wrong. An LLM 429 after transcription still fails after three attempts, deliberately: the ASR half was really paid for and re-running would repeat it.
• Observability: GET /api/admin/ai/queue per {kind, provider, host} → limit, in use, waiting, estimated wait; processing_wait on the session response; ProcessingQueueNote on the session page.
• Unified with #359: ProviderRateLimited is now an alias of app.providers.errors.ProviderRateLimitError; #359's xfail placeholder is replaced by a real conformance case asserting peak == limit for every adapter (a cap that never admits its full quota would pass <= while halving throughput). The gate is in the caller by design and the test docstring says so.

Rebased over #357: the usage meter, _prose_usage and the ASR timer coexist with the slot try/finally; three import/phase conflicts resolved.

Verification

• Burst test: 8 concurrent sessions, limit 2 → peak provider concurrency exactly 2, all 8 complete, 0 rate-limit failures, 0 leases or waiters left behind. Mutation-verified (removing the fake Redis fails the enforcement cases; disabling the re-queue guard fails the 429 test).
• Backend: 1929 passed, 8 skipped. Frontend: 489 passed, eslint clean apart from the pre-existing warning. ruff clean. Version sync OK; no migration; no bot contract change.

🤖 Generated with Claude Code

Closes #356. A burst of Friday-night sessions now costs latency, not failures. Each provider declares what it will actually serve at once; the Celery layer enforces it; a GM whose session lands in a busy window sees "queued behind N, about M minutes" instead of silence. ## What - **Declared limits + operator override** — Ollama 1, llama.cpp 2 (matches the documented `--parallel 2`), bundled WhisperX 1 (its v2 advertises exactly that), Anthropic/OpenAI `None` (rely on 429 + `Retry-After`). `.env` overrides `QB_LLM_CONCURRENCY_LIMIT` / `QB_ASR_CONCURRENCY_LIMIT` win over the declaration. - **A Redis lease semaphore** (`app/services/provider_slots.py`, DB 0 only): leases with TTL so an OOM-killed worker cannot pin a GPU; rank taken from a monotonic admission counter rather than expiry (renewing a lease must not re-sort it behind a newcomer); rolling hold durations for the estimate; **fails open** (logged, `degraded`) if Redis is unreachable. - **Enforcement where it belongs**: `process_audio` acquires the ASR slot where `whisper_cfg` is read and, if none is free, `self.retry(countdown=20–40 s jittered, max_retries=240)` so the worker returns to the pool; released before the LLM phase. Every LLM path acquires through one `run_with_slot` around the transport dispatch in `generate_structured_text` and `_dispatch_prose` with a bounded in-task wait (ceiling 900 s), because retrying a whole task after transcription would repeat the ASR. Request-path callers (`/ask`, admin Test, the two Workbench generators that run inside an HTTP request) never wait: a `RequestScopeMiddleware` contextvar marks request scope so a browser cannot hang on the queue — the rule is a property of where code runs, not of each call site. - **429 handling**: release the slot, honour `Retry-After` (clamped to 10 min), bounded backoff, typed error afterwards. **An ASR 429 re-queues the session** rather than failing it (`_AsrShouldRequeue`), and a re-queued run writes no `session_usage` row — a charge for work that never happened, rewritten every 20 s, would be wrong. An LLM 429 after transcription still fails after three attempts, deliberately: the ASR half was really paid for and re-running would repeat it. - **Observability**: `GET /api/admin/ai/queue` per `{kind, provider, host}` → limit, in use, waiting, estimated wait; `processing_wait` on the session response; `ProcessingQueueNote` on the session page. - **Unified with #359**: `ProviderRateLimited` is now an alias of `app.providers.errors.ProviderRateLimitError`; #359's `xfail` placeholder is replaced by a real conformance case asserting **peak == limit** for every adapter (a cap that never admits its full quota would pass `<=` while halving throughput). The gate is in the caller by design and the test docstring says so. Rebased over #357: the usage meter, `_prose_usage` and the ASR timer coexist with the slot try/finally; three import/phase conflicts resolved. ## Verification - Burst test: 8 concurrent sessions, limit 2 → peak provider concurrency **exactly 2**, all 8 complete, 0 rate-limit failures, 0 leases or waiters left behind. Mutation-verified (removing the fake Redis fails the enforcement cases; disabling the re-queue guard fails the 429 test). - Backend: **1929 passed, 8 skipped**. Frontend: **489 passed**, eslint clean apart from the pre-existing warning. ruff clean. Version sync OK; no migration; no bot contract change. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-bot added 13 commits 2026-09-05 03:28:28 +00:00

feat(providers): declare what each provider will actually serve at once ... 987c62f491

Every adapter carried a `concurrency_limit` field and only one of them set it,
so the pipeline's only real control was the worker's `--concurrency=2` — a cap
on tasks, which is not the same thing as a cap on provider requests and is
unrelated to what any endpoint can serve.

The declarations split on how a provider fails when oversubscribed. Ollama (1)
and llama.cpp (2, matching the documented `--parallel 2`) and the bundled
WhisperX server (1, one model on one GPU) all queue internally and slow every
in-flight request down together, which turns a burst into a pile of requests
approaching their timeout at once. Anthropic and OpenAI declare nothing, on
purpose: their real limit is per-account, per-model and changes without notice,
so a guessed number could only throttle a paying account below what it bought.
There the 429 is the signal.

QB_LLM_CONCURRENCY_LIMIT / QB_ASR_CONCURRENCY_LIMIT override any of it. Env
vars rather than admin settings because the right value is a fact about how the
operator started their inference server, same class of thing as RESTART_POLICY.
0 reads as unset — taken literally it would stop the pipeline and leave the
queue as the only explanation.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

feat(services): a Redis lease semaphore for provider slots ... ac97ab7b7e

The counting half of #356. A slot is a lease rather than a lock because the
holder is a Celery worker that can be OOM-killed mid-transcription: a lock would
then be held forever by a process that no longer exists, and the only recovery
would be an operator flushing a Redis key by hand.

Rank comes from a separate ZSET scored by a monotonic counter, not from the
expiry ZSET. That distinction is the whole correctness argument: renewing a
long-running lease pushes its expiry forward, and if rank followed expiry the
renewing holder would be re-sorted behind a newcomer and both would believe they
held the same slot. Scoring admission separately from expiry makes "the first N
ids in" the holders, stably, under renewal.

No Lua script. It would make acquire atomic in one round trip and untestable
without a real Redis, which this suite does not have. It is not needed anyway —
every process derives the same ordering from the same counter, so two racing
acquirers get two distinct ranks and only one can be under the limit.

Redis being unreachable fails open: the lease is marked degraded, logged, and
the work runs uncapped. The cap governs how fast a backlog drains, not whether
the output is correct, and an outage that also froze every transcription would
be far worse than a briefly oversubscribed GPU.

Also here: a rolling window of recent hold durations per key (median, so one
four-hour session does not quadruple every estimate for the evening), a waiting
count that doubles as a queue position, and the `qb:session_wait:{id}` record a
GM's session page reads.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

feat(tasks): queue behind a busy provider instead of failing against it ... 154bbcfa7f

Enforcement at three granularities, because the right answer differs by how
much work is already spent when the slot turns out to be taken.

`process_audio` takes the ASR slot at the point the endpoint becomes known and
gives the worker straight back to the pool when there is none — `self.retry`
with a flat, jittered 20-40 s countdown and a much larger max_retries. Flat and
not exponential on purpose: exponential backoff is the right shape for a
dependency that is failing and the wrong one for a dependency that is merely
busy, because it reorders the burst so whoever waited longest waits longest
again, and it idles the transcriber while every waiter sleeps. The slot is
handed back the moment transcription finishes, before summarisation — holding
it through the LLM phase would idle a GPU for the length of a summary and turn
a cap meant to protect throughput into the thing that destroys it.

LLM calls wait in place instead, up to fifteen minutes. By then the
transcription is paid for and re-running the task to get a slot would repeat the
expensive half to save the cheap one. The acquire sits in the two functions
every LLM path already resolves its provider through — `generate_structured_text`
and `_dispatch_prose` — rather than at eighteen call sites, seventeen of which
would be right.

`/ask` and the admin Test button take a slot and never wait: somebody is
watching a spinner, and fifteen minutes of silence is worse than being told the
queue is full. They still count against the endpoint's capacity rather than
sneaking past it, and ProviderBusy's message says in as many words that nothing
was sent and the endpoint is not the problem.

A 429 now raises a typed error rather than a generic RuntimeError, so a provider
that is rate limiting us stops being indistinguishable from one that is broken.
The slot is released before the backoff — sitting on it while a provider has
told us to stop starves callers who could have used it — and Retry-After is
honoured when given. Three attempts, then the typed error surfaces. Everything
else passes straight through: a broken provider should look broken.

Running out of ASR retries (roughly two hours of a permanently full queue, which
is not a busy Saturday) marks the session failed with a capacity message rather
than leaving it on "processing" forever.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

feat(api): show the provider queue, to an admin and to a waiting GM ... 3408a7f990

Two views of the same Redis state, aimed at the two people who ask about it.

`GET /api/admin/ai/queue` answers the operator's question during a burst: is
anything stuck, or is it just busy? Those looked identical from outside — every
session on "Processing…" either way — and the only way to tell them apart was to
read worker logs. Per endpoint it reports the cap, what is in use, what is
waiting and how long a new arrival should expect to wait. It reads Redis only
and never contacts a provider, so it stays honest and instant when a provider is
exactly what has gone wrong.

`processing_wait` on the session detail response answers the GM's: an additive
field, null whenever nothing is queued, shaped {kind, position,
estimated_seconds, since}. Read from Redis rather than the session row, so there
is no migration and nothing to clean up when the wait ends. Only the detail
endpoint fills it — the list endpoints would need one Redis read per session for
a note nobody reads in a list.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

feat(frontend): tell a GM their session is queued, not stuck ... 9f0226c0a1

Recordings arrive in a burst against a transcriber that serves one at a time,
which is fine — a backlog costs latency and nobody needs their recap within
thirty seconds. What was not fine is that the wait looked exactly like a hang:
the same "Processing…" pill and the same silence for twenty minutes whether the
pipeline was working through a queue or had died. GMs reasonably concluded the
second and hit Retry, which put another session into the same queue.

One line on the session page, rendering only while the backend reports an actual
wait. The page already re-polls every five seconds while processing, so the
estimate updates itself with no timer of its own.

The estimate is deliberately vague — a median of recent hold times against a
queue position, rounded to minutes and hedged with "about". A precise number
that turns out wrong is worse than an approximate one that never claimed to be
precise.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

test: pin the cap, the queue, and the burst it exists for ... b108e9f81d

The property under test throughout is the issue's own: a burst costs latency,
not failures. Eight fake sessions against a limit of two, and the assertions
are the two halves of that — the provider never sees more than two concurrent
calls, and all eight finish. It also asserts peak *equals* two, because a cap
that is respected by never using it would pass the first assertion and be
useless.

There is no Redis in this suite and none of this is observable through a mock
that records calls: "no more than two at once" is a claim about state shared
between concurrent callers. So there is a small in-process Redis implementing
the dozen commands the slot service uses, with real zrank ordering — rank is
the whole admission decision, so approximating it would test nothing. fakeredis
is not in the dev dependencies and adding a dependency to test one module is a
poor trade for a surface this small.

The renewal test is the one worth reading: it is the sequence that would put two
workers on a one-slot GPU if rank came from expiry rather than from admission
order. Also covered — expiry reclaiming a crashed worker's slot, the override
beating the declared default in both directions (and 0 meaning "unset", not
"never run anything"), process_audio re-queuing with a flat countdown and
touching nothing on the wire, the in-task wait resolving as a slot frees, and
a 429 releasing the slot *before* it backs off for exactly as long as the
provider asked.

Writing them turned up one real hole: a window full of instant failures made
the estimate confidently promise no wait at all. Non-positive holds are now
discarded, which is fixed here rather than separately because the test is the
only reason anyone knew.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

docs: what the caps are, how to change them, and what "queued" means ... b47f8d226f

An operator needs three things here and the code cannot tell them any of it:
what each provider's declared cap is and why that number, the two `.env`
settings that override it (and that `0` means "unset" rather than "never run
anything"), and what actually happens at each granularity when a slot is
busy — a re-queue for transcription, an in-place wait for summarisation, an
immediate refusal for `/ask` and the Test button.

Also documents `GET /api/admin/ai/queue` in API.md, the additive
`processing_wait` field on the session detail response, and the honest reading
of the estimate: a median against a queue length, right to the order of
magnitude and no further, which is why the UI rounds it to minutes.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

fix(api): never let an HTTP request block on the provider queue ... 8c58636468

Two GM Workbench endpoints run a generation synchronously inside the request —
the name generators on the campaign and session pages — through the same
`generate_structured_text` a Celery task uses. With the in-task wait added they
would sit on a busy provider for up to fifteen minutes, which the browser, the
reverse proxy and the GM all abandon long before, while the request pins a
worker throughout.

Fixed at the boundary rather than at each caller. "Do not block on a queue" is a
property of where the code is running, not of the call site: threading
`wait_for_slot=False` down from today's callers would work today and silently
hang the fifth one somebody adds next month, which is exactly the
eighteen-call-sites failure the provider package exists to prevent. A request-
scope flag set once in middleware cannot be forgotten by code that does not know
it exists.

Same mechanism as RequestIDMiddleware, which has been setting a request-scoped
context variable read deep inside handlers since the logging work — proven here
rather than new. The test asserts it from inside a handler on a real request,
because the thing that could quietly break is context propagation across
Starlette's task boundary, and only a real request exercises that.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

refactor(services): recognise a rate limit by shape, not by class ... 716a644e38

#359 has since landed `ProviderRateLimitError` under `app.providers` with the
same `retry_after` this module carries, and says in as many words that honouring
it is #356's job. More adapters will bring more types.

So the backoff now matches on `status_code == 429` rather than on a tuple of
imported classes. Anything carrying it is a provider saying "too often",
whoever built the exception, and the provider's own `Retry-After` survives
whichever type carried it. A tuple would need every new adapter to remember to
be added to it, and forgetting silently converts "wait thirty seconds" into a
failed session.

That also makes unifying the two types a rename rather than a behaviour change,
which is the point — `ProviderRateLimited` should become an alias of
`app.providers.errors.ProviderRateLimitError` once the two branches are in the
same tree.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

refactor(providers): one typed 429, not two ... c4efe611b6

#356 and #359 were built in parallel and each needed a typed rate limit, so the
tree briefly had two classes meaning the same thing with the same retry_after.
One is obviously right: the adapters raise ProviderRateLimitError, app.providers
is the lower layer, and app.services may import from it but not the reverse.

So `provider_slots.ProviderRateLimited` is now a strict alias of it — not a
subclass, because the only property worth guaranteeing is that
`except ProviderRateLimited` and `except ProviderRateLimitError` catch exactly
the same thing. The name stays because it reads better at the call sites here,
where the subject is the queue rather than the adapter.

The shape check in `as_rate_limit` stays as the belt. It is not made redundant
by there being one class: anything carrying `status_code == 429` is a provider
saying "too often" whoever built the exception, so a gateway wrapper or a
managed SDK's own error is honoured without being added to a tuple somebody has
to remember. Forgetting silently converts "wait thirty seconds" into a failed
session.

errors.py no longer says nothing honours retry_after yet; it names what does.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

test(conformance): the declared limit is enforced, for every adapter ... 6f1d1acb40

#359 wrote this case for real and marked it xfail against #356, so the suite
would report a known unimplemented property rather than quietly not testing it.
#356 has landed; the marker comes off and the case is rewritten against where
the gate actually is.

It is deliberately not an adapter-level assertion, and the docstring says so at
length so nobody re-adds one. The limit belongs to the endpoint, not to the
object: two providers pointed at the same box share its slots and one pointed
elsewhere does not, which an adapter holding its own semaphore cannot express —
nor can it express a Celery worker handing itself back to the pool instead of
sleeping on a slot, or a wait the queued GM can see. So the gate is
`provider_slots`, between the caller and `submit`, and what this proves is that
every adapter is covered by it — including the stub, which speaks no HTTP at
all, so the property survives a new adapter without that adapter opting in.

`limit + 4` jobs launched together, asserting peak == limit rather than <=: a
gate that never admits its full quota satisfies an upper bound while quietly
halving a self-hoster's throughput. Adapters that speak HTTP get a second
counter at the wire, which would also catch one that took a slot and then fanned
out behind it. Verified by mutation — removing the fake Redis (so the slot
service fails open) fails all three cases.

The in-process Redis moves to tests/fake_redis.py so both suites can use it,
and `_LLM_POLL_*` becomes `_SLOT_POLL_*_SECONDS`: the ceiling above it is an LLM
policy but the polling gates every kind, and reading `_LLM_POLL_MIN` in an ASR
test is the kind of small lie that costs someone an afternoon.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

fix(tasks): a 429 from the transcriber re-queues the session, not fails it ... b5f0aae2a0

Our cap said there was room and the provider disagreed. That is not a
misconfiguration and not hypothetical: a shared GPU box, a gateway in front of
one, or a managed endpoint's per-account limit all look exactly like this, and
none of them are things the adapter's declared figure can know about.

Before this, a 429 from the ASR phase reached the generic handler, marked the
session `failed` and put a red banner and a Retry button in front of a GM with
nothing to fix — and pressing it added a second copy of the recording to the
same queue that was already too full. #359's ProviderRateLimitError made the
case distinguishable; this acts on it.

So it is the same answer as a full slot arriving from the other direction. The
sentinel that carried "wait your turn" out of the async body now carries both
causes and a countdown, and the handler that turns it into a Celery retry is
one place. The provider's own Retry-After wins when it sent one — it knows when
it will take us and we do not — bounded at 10 minutes, because it is the one
number in this path chosen by somebody else's server and a gateway answering
`Retry-After: 86400` would park a session for a day. Same retry budget, same
terminal handling: two hours of a queue that never opens is a transcriber that
is not coming back, and the session is failed with a message naming which wall
it kept hitting, because "our cap was full" and "it kept saying 429" point an
admin at different settings.

The slot goes back before the wait, so a session that would have been allowed
through is not stuck behind one the provider has told us to stop sending.

Three cases: the honoured Retry-After (7 s, through the real adapter against a
scripted wire), the hostile one (a day, clamped), and none at all (the flat
jittered interval). Each asserts the session is *not* marked failed and the slot
was released — verified by mutation: removing the guard ahead of the generic
handler fails the first.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

fix(tasks): a re-queued run writes no usage row ... f1e181af12

Rebasing onto #357 put a `_record_usage(USAGE_STATUS_FAILED)` inside the generic
handler this branch already has to step around, so the two features now have to
agree about what a re-queue means.

They do, and in #357's favour: a re-queued run has consumed nothing. No
transcription completed, no LLM call was made, and the meter is empty. A
`failed` row for it would be a charge for work that never happened — written
afresh every twenty seconds for as long as the queue stays full, which is
exactly when an operator is most likely to be reading the cost page and least
able to afford it being wrong. The attempt that eventually runs records what it
actually cost, and `retry_count` on that row is where the waiting shows up.

The `except _AsrShouldRequeue: raise` that already sat ahead of the generic
handler gives this for free; what it needed was for the comment to say so,
because "why is there no row here" is the question a reader of #357 will bring.

Asserted for both re-queue causes, with the failing run as the control: it
*does* write its row, so "no row" is a real distinction rather than a spy that
never fired.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

claude-bot scheduled this pull request to auto merge when all checks succeed 2026-09-05 03:28:34 +00:00

claude-bot merged commit b462f051b5 into main 2026-09-05 03:38:32 +00:00

claude-bot deleted branch feat/356-provider-concurrency 2026-09-05 03:38:32 +00:00

claude-bot referenced this pull request from a commit 2026-09-05 03:38:34 +00:00

Merge PR #495: provider-aware concurrency caps (#356)

claude-bot referenced this pull request 2026-09-05 03:41:44 +00:00

[Backend] Provider-aware concurrency caps in the Celery queue #356

claude-bot referenced this pull request 2026-09-05 06:04:34 +00:00

chore(release): v4.2.0 — AI Provider Abstraction & BYO-AI #504

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

backend

Issue related to the backend

  

bot

Issue related to the bot

  

bug

Something is not working

  

duplicate

This issue or pull request already exists

  

enhancement

New feature

  

frontend

Issue related to the frontend

  

help wanted

Need some help

  

invalid

Something is wrong

  

question

More information is needed

  

recording

Issue related to the recording system

  

security

Security hardening / vulnerability work

  

wontfix

This won't be fixed

No labels

backend

bot

bug

duplicate

enhancement

frontend

help wanted

invalid

question

recording

security

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

rbrooks/Quest-Board!495

No description provided.