[Backend] Set Ollama num_ctx and verify the prompt was actually consumed #337
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Severity: CRITICAL. Found in the August 2026 session lifecycle review (#319).
The defect
num_ctxis never set anywhere in the repository. Verified by grep across_summarise_ollama(audio_service.py:2089-2101) and_structured_ollama(llm_service.py:181-208).Ollama's default context is 2048-4096 tokens and it silently truncates input beyond it. Any Ollama-configured install is therefore summarising roughly 2-3% of a real session while presenting the result as a summary of the whole thing — with no error, no warning, and a plausible-looking output.
prompt_eval_countcomes back in the response and is ignored.This is not a hypothetical for a self-hosted-first product: Ollama is the most likely local backend a self-hoster reaches for.
Proposed fix
Set
options.num_ctxexplicitly on every Ollama call, sourced from the provider's declared window. Readprompt_eval_countfrom the response and compare it against the preflight estimate; a material shortfall means the prompt was truncated, and that must fail loudly rather than persist a confidently wrong summary.Acceptance criteria
options.num_ctxset on every Ollama requestprompt_eval_countcompared against the estimate on every responsenum_ctxis present in the request bodyDone in
c54d9af. Verified against acceptance criteria below, with one criterion amended rather than met as written.The issue's premise was half out of date
The body says "
num_ctxis never set anywhere in the repository. Verified by grep." That stopped being true inccc29d6, which added it to both_structured_ollamaand_summarise_ollama— andtest_ollama_greedy_decoding_does_not_clobber_num_ctxhas been green ever since.The defect was still real, just one level in: the whole protection sat behind
if context_tokens:, and of the eighteengenerate_structured_textcall sites, exactly one passed a window (beat_service.py:454). So almost every Ollama request in the product still fell back to Ollama's 2-4k default and was silently truncated — the exact failure this issue describes, in a file that looked like it fixed it. A grep fornum_ctxsaid "fixed"; a grep forcontext_tokens=said otherwise.Deviation from the proposed fix, recorded
The issue proposes sourcing
num_ctx"from the provider's declared window", i.e. threadingcontext_tokensthrough the other seventeen call sites. That is now #336's job, but it is not what makes this safe, and shipping only that would have been a mistake: it fixes seventeen call sites and leaves the eighteenth — written next month — unprotected again, with no test able to notice, because "nobody passed a window" and "this call doesn't need one" are indistinguishable.So
Nonenow resolves to a conservative default instead of switching budgeting off (llm_service.py:61-84), andnum_ctxis set unconditionally (llm_service.py:474-490, audio_service.py:3849-3854). #336's plumbing becomes an accuracy improvement rather than the thing standing between a self-hoster and a summary of 3% of their session.32,768 is not a new memory risk on Ollama. It is the value
LLMConfigalready defaults to, so the largest prompt in the product —summariseviaget_llm_config— has been sendingnum_ctx: 32768all along. Applying it to the smaller calls cannot raise the allocation ceiling above one already in production.Acceptance criteria
options.num_ctxset on every Ollama request — llm_service.py:474 (structured) and audio_service.py:3849 (prose). Unconditional on both.prompt_eval_countcompared against the estimate on every response —_warn_if_prompt_truncated, llm_service.py:503 and audio_service.py:3862. Joined now bypreflight_prompt, the input-side check that runs before the request — which matters because a backend with context shift returns an ordinary 200 for a prompt whose front it just discarded, and reports nothing at all.num_ctxis present in the request body — five of them, test_llm_transport.py:722-790, covering structured, prose, an explicit window, and five flavours of junk setting.A truncated prompt raises rather than returning a result→ amended. See below.The one criterion not met as written
"A truncated prompt raises rather than returning a result."
_warn_if_prompt_truncatedwarns and returns, and has sinceccc29d6, for a documented reason: providers that cache a prompt prefix legitimately report only newly-evaluated tokens, so a lowprompt_eval_countmeans either "your prompt was truncated" or "most of it was cached" and from here those are identical. Raising would fail perfectly good requests on a hosted provider.preflight_promptmakes the same call for a different reason —estimate_tokensis characters ÷ 3.4 and deliberately pessimistic, so a raise would refuse prompts that would have worked.Both become hard checks when the provider abstraction can declare a real tokenizer and whether a backend front-truncates. That is #351 / v4.2.0, and this criterion belongs there rather than here. Consistent with the same decision on #338: flag, don't raise, where the signal is ambiguous.
Verified against the old behaviour
All ten new tests were run against the pre-change code with
app/stashed. All ten fail there, and with the right failure mode —KeyError: 'options', i.e. the key was absent from the request body entirely, not merely an assertion mismatch. The two that pass on both sides are regression guards on the explicit-window path and are labelled as such.A latent crash turned up while testing it:
if context_tokens:let a non-numeric value through toint(), so a typo'd context window in Admin raisedValueErrorfrom the transport layer rather than falling back. Covered by the parametrised test.1,273 → 1,284 passing, lint clean.