refactor(llm): pass LLMConfig through the generation paths as one object (#484), and honour the explicit provider on the prose path (#488) #489

Merged
claude-bot merged 10 commits from refactor/484-llm-target into main 2026-09-05 01:09:34 +00:00
Contributor

Closes #484. Closes #488.

Every LLM generation path now takes the configured target as one parameter, target: LLMConfig, instead of the loose endpoint_url, api_key, model, context_tokens, llm_provider quintet destructured at the origin and re-threaded by hand. A new call site can no longer omit a field of an object it is already holding, and adding a sixth attribute to the config is one edit rather than ~58.

What changed

  • LLMConfig moves to app/providers/llm.py (frozen), with DEFAULT_CONTEXT_TOKENS and its rationale block. It had to move: settings_service imports from llm_service, so the transport could not import the config from settings without a cycle, and app.providers is the leaf package. settings_service re-exports the name so settings_service.LLMConfig still resolves.
  • registry.llm_provider_for(target) — the one-object form of resolve_llm_provider.
  • generate_structured_text(prompt, target, *, ...) — loose form removed outright, no shim. All four _structured_* transports take target too, so the destructuring does not just move one frame down.
  • audio_service: 24 LLM-taking functions converted, including the eight hand-rolled _qa_*/_summarise_* transports and test_llm. The eight Whisper ASR functions are untouched (#350's territory). _FALLBACK_CONTEXT_TOKENS deleted; the two callees apply resolve_window(target.context_tokens) themselves, which is the same number.
  • beat_service, generation_service, lore_service, reminder_tasks, planning_tasks, routers/bot.py, routers/users.py: origin sites pass llm_cfg whole.
  • evals/: LiveProvider carries an LLMConfig instead of re-assembling four fields per call — the one caller the app/+tests/ greps missed, with no test covering it. Gained QB_EVAL_PROVIDER so the harness can name the provider rather than being stuck on URL-guessing forever, and --context-tokens now reaches the request as well as the window sizing (previously it moved only one of the two).

The bug fix (#488), its own commit 1b0e273

_dispatch_prose (the session-summary prose path) and test_llm (the admin test button) accepted the explicit provider from #351 and ignored it, still branching on "anthropic.com" in endpoint_url and friends. An operator who pinned anthropic on a gateway of their own got structured generation and /ask routed correctly and session summaries sent to the llama.cpp transport. Both now resolve once through the registry and dispatch on provider.capabilities.name, the way answer_question already did. Two tests set provider="anthropic" on a non-anthropic URL and assert the Anthropic transport is the one awaited; both were verified red against the pre-fix file. Changelog entry under Fixed.

No behaviour change elsewhere

An AST walk over 9a0cc4b found zero production call sites reaching the transport with context_tokens omitted (45 in tests, all migrated), so no production path's effective window moves. The three places the window is now read from provider.capabilities.context_tokens rather than resolve_window(...) of the same value are identical for any positive int, and get_llm_config always resolves one.

After-counts

before after
functions in app/ declaring endpoint_url: str 22 12 (the 8 ASR ones + the ASR/LLM registry resolvers, which take fields loose by design)
llm_provider= in app/ 49 1 (a response-schema field name, not a call argument)

Verification

  • Backend suite: 1716 passed, 0 failed, 0 skipped.
  • ruff format --check / ruff check: clean.
  • scripts/check_version_sync.py: OK.
  • Test doubles are real LLMConfig instances throughout (no SimpleNamespace look-alikes), plus an inspect.signature guard so the loose form cannot creep back.

🤖 Generated with Claude Code

Closes #484. Closes #488. Every LLM generation path now takes the configured target as **one parameter, `target: LLMConfig`**, instead of the loose `endpoint_url, api_key, model, context_tokens, llm_provider` quintet destructured at the origin and re-threaded by hand. A new call site can no longer omit a field of an object it is already holding, and adding a sixth attribute to the config is one edit rather than ~58. ## What changed - **`LLMConfig` moves to `app/providers/llm.py`** (frozen), with `DEFAULT_CONTEXT_TOKENS` and its rationale block. It had to move: `settings_service` imports from `llm_service`, so the transport could not import the config from settings without a cycle, and `app.providers` is the leaf package. `settings_service` re-exports the name so `settings_service.LLMConfig` still resolves. - **`registry.llm_provider_for(target)`** — the one-object form of `resolve_llm_provider`. - **`generate_structured_text(prompt, target, *, ...)`** — loose form removed outright, no shim. All four `_structured_*` transports take `target` too, so the destructuring does not just move one frame down. - **`audio_service`**: 24 LLM-taking functions converted, including the eight hand-rolled `_qa_*`/`_summarise_*` transports and `test_llm`. The eight Whisper ASR functions are untouched (#350's territory). `_FALLBACK_CONTEXT_TOKENS` deleted; the two callees apply `resolve_window(target.context_tokens)` themselves, which is the same number. - **`beat_service`, `generation_service`, `lore_service`, `reminder_tasks`, `planning_tasks`, `routers/bot.py`, `routers/users.py`**: origin sites pass `llm_cfg` whole. - **`evals/`**: `LiveProvider` carries an `LLMConfig` instead of re-assembling four fields per call — the one caller the `app/`+`tests/` greps missed, with no test covering it. Gained `QB_EVAL_PROVIDER` so the harness can name the provider rather than being stuck on URL-guessing forever, and `--context-tokens` now reaches the request as well as the window sizing (previously it moved only one of the two). ## The bug fix (#488), its own commit `1b0e273` `_dispatch_prose` (the session-summary prose path) and `test_llm` (the admin test button) accepted the explicit provider from #351 and ignored it, still branching on `"anthropic.com" in endpoint_url` and friends. An operator who pinned `anthropic` on a gateway of their own got structured generation and `/ask` routed correctly and **session summaries sent to the llama.cpp transport**. Both now resolve once through the registry and dispatch on `provider.capabilities.name`, the way `answer_question` already did. Two tests set `provider="anthropic"` on a non-anthropic URL and assert the Anthropic transport is the one awaited; both were verified red against the pre-fix file. Changelog entry under **Fixed**. ## No behaviour change elsewhere An AST walk over `9a0cc4b` found **zero** production call sites reaching the transport with `context_tokens` omitted (45 in tests, all migrated), so no production path's effective window moves. The three places the window is now read from `provider.capabilities.context_tokens` rather than `resolve_window(...)` of the same value are identical for any positive int, and `get_llm_config` always resolves one. ## After-counts | | before | after | |---|---|---| | functions in `app/` declaring `endpoint_url: str` | 22 | 12 (the 8 ASR ones + the ASR/LLM registry resolvers, which take fields loose by design) | | `llm_provider=` in `app/` | 49 | 1 (a response-schema field name, not a call argument) | ## Verification - Backend suite: **1716 passed**, 0 failed, 0 skipped. - `ruff format --check` / `ruff check`: clean. - `scripts/check_version_sync.py`: OK. - Test doubles are real `LLMConfig` instances throughout (no `SimpleNamespace` look-alikes), plus an `inspect.signature` guard so the loose form cannot creep back. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
LLMConfig moves from settings_service down to app.providers.llm, next to the
provider contract it describes, and DEFAULT_CONTEXT_TOKENS moves with it. It has
to: settings_service imports the default window from llm_service, so llm_service
cannot import the config type back without a cycle. app.providers is the leaf
package and stays that way. settings_service re-exports the name, so
settings_service.LLMConfig keeps working.

Frozen, because it is a target rather than a scratchpad — it is passed unchanged
through a dozen frames, and a function that mutated it would silently change
what every later frame is talking to.

generate_structured_text now takes `target: LLMConfig` in place of the
endpoint_url/api_key/model/context_tokens/llm_provider quintet. No deprecated
shim: the whole point is that a new call site cannot omit a field of an object it
is already holding, which is how seventeen of eighteen sites ended up with no
declared window (#337).

registry.llm_provider_for(target) is the one-liner that resolves a config to a
provider; resolve_llm_provider stays for callers that genuinely have the fields
loose.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Every LLM-taking function in audio_service — the domain generators, the beats
chain, _dispatch_prose, the four prose transports, the four /ask transports,
test_llm and its probes — takes `target: LLMConfig` in place of whatever subset
of endpoint_url/api_key/model/context_tokens/llm_provider it carried, and passes
it on unchanged.

The Whisper functions are untouched: their endpoint_url is the ASR endpoint, not
the LLM one, and belongs to #350.

Two docstrings said the provider is inferred from the endpoint URL. That stopped
being true when #351 made provider identity configuration; they now say what
happens.

_FALLBACK_CONTEXT_TOKENS goes: summarise used to resolve a window and hand it
down, and the two functions that needed the fallback (_summarise_from_beats,
_summarise_chunked) now apply resolve_window to the target's own value instead
of trusting their caller to have done it.

No behaviour change. _dispatch_prose and test_llm still dispatch on the URL
substring — including ignoring an explicit provider, which is a real bug and is
fixed separately so it is reviewable on its own.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
extract_beats takes `target: LLMConfig`; generation_service and lore_service
stop destructuring the llm_cfg they already hold and hand it straight to
generate_structured_text, generate_name_options and merge_lore_entry_body.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The eighteen places that resolve an LLM config — require_llm_config /
get_llm_config in the Celery tasks, /api/bot/ask, and the admin
test-connection endpoint — now pass the config itself instead of unpacking it
into five arguments and hoping the next caller re-packs them the same way.

The admin LLM test button gains the configured provider as a side effect, which
matters: it now probes the adapter generation will actually use rather than the
one the URL looks like.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Every stand-in for the LLM config — SimpleNamespace, an ad-hoc
`type("LLM", (), {...})()`, a dict — becomes a real LLMConfig. A double that
diverges from the dataclass is a test that keeps passing while production
breaks, and these doubles were already one field behind more than once
(see the note in test_recap_email).

Call sites move to the single target argument. The assertions are untouched:
they were on the wire body all along — options.num_ctx, the schema spelling,
the max_tokens cap — which is the request, and still is.

Adds a guard in test_llm_transport asserting from the signature itself that
generate_structured_text takes `target` and none of the five loose fields, so
the shape this issue removed cannot come back unnoticed.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
#351 made provider identity configuration rather than a URL guess, and
generate_structured_text and answer_question resolve it. Two paths did not:
_dispatch_prose and test_llm each kept branching on "anthropic.com" in the URL,
"openai.com", an Ollama-looking host, else llama.cpp — accepting the configured
provider and ignoring it.

So an operator who pinned a provider on a gateway of their own domain — the
exact case the field exists for — got structured generation and /ask routed
correctly while their session summaries went to the llama.cpp transport, with
the key in an Authorization header rather than x-api-key, against an endpoint
that does not speak that contract. And the Test button reported on a connection
nothing else in the product would have made, which is worse than reporting
nothing.

Both now resolve once through llm_provider_for and dispatch on
capabilities.name, exactly as answer_question does, and _dispatch_prose budgets
against the provider's declared window instead of re-deriving one.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
generate_structured_text was still unpacking the target back into
endpoint_url/api_key/model to call its own transports — the destructuring this
issue removes, one frame further down. Each transport now takes the target, and
the Ollama path reads its num_ctx from the resolved provider's declared window
rather than re-deriving it from a separately-threaded number.

No behaviour change: the window the transport was handed was already
``provider.capabilities.context_tokens``, and ``resolve_window`` of a positive
int is that int.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The note above the four /ask transports said folding them together was #484.
#484 turned out to be the parameter refactor instead, so the comment now says
what actually happened and leaves the folding as still-open work rather than
pointing at a closed issue.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
refactor(evals): give the eval harness the same target the app uses (#484)
Some checks failed
CI / Frontend tests, audit, and build (pull_request) Has been cancelled
CI / Backend migration, tests, and audit (pull_request) Has been cancelled
CI / Summarisation accuracy eval harness (stub provider) (pull_request) Has been cancelled
CI / Bot tests and audit (pull_request) Has been cancelled
CI / Backend lint (ruff) (pull_request) Has been cancelled
CI / Bot/backend version sync (pull_request) Has been cancelled
CI / Docker image build (pull_request) Has been cancelled
3ee61545a7
LiveProvider held the endpoint, key, model and window as four fields of its own
and re-assembled them at each call — the only remaining place outside app/ that
did. It now carries an LLMConfig, which fixes two things the loose form was
hiding:

- It had no provider field at all, so every live eval run let the provider be
  guessed from the URL no matter what the deployment under test was configured
  as. QB_EVAL_PROVIDER now names it.
- `--context-tokens` moved the window the runner sizes excerpts against but not
  the window the request declared, so a run could be measured against one number
  and executed against another. Both come from the target now.

No test covered this path — it only runs against a real endpoint — which is why
the suite stayed green while the call sites were stale.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
docs(changelog): cite the bug issue (#488), not the refactor, for the provider fix
All checks were successful
CI / Backend lint (ruff) (pull_request) Successful in 24s
CI / Bot/backend version sync (pull_request) Successful in 20s
CI / Bot tests and audit (pull_request) Successful in 1m21s
CI / Summarisation accuracy eval harness (stub provider) (pull_request) Successful in 1m29s
CI / Frontend tests, audit, and build (pull_request) Successful in 2m49s
CI / Backend migration, tests, and audit (pull_request) Successful in 6m1s
CI / Docker image build (pull_request) Successful in 5m13s
bfd91bb525
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 01:01:03 +00:00
claude-bot deleted branch refactor/484-llm-target 2026-09-05 01:09:36 +00:00
Sign in to join this conversation.
No description provided.