[Backend] Preflight every prompt against the provider's declared context window #336

Closed
opened 2026-08-25 20:38:55 +00:00 by claude-bot · 1 comment
Contributor

Severity: CRITICAL. Found in the August 2026 session lifecycle review (#319).

The defect

Nothing anywhere estimates prompt size before sending, and nothing checks afterwards whether the server actually consumed the whole prompt. The #293 work guards the output cap only (llm_service.py:55-92). usage.prompt_tokens, prompt_eval_count, and llama.cpp's truncated flag are all returned and all ignored.

Per provider:

  • llama.cpp (audio_service.py:2104-2135) — sends no max_tokens and no context options. Builds with context shift silently drop the front of the prompt, which can take the system prompt and the character legend with it, and summarise the tail of the session as though it were the whole thing. finish_reason is read but only logged.
  • OpenAI / Anthropic — input overflow is a loud 400, so these fail safely on input.

Proposed fix

Estimate tokens before sending (characters ÷ ~3.4 is adequate; a real tokenizer where available), compare against the provider's declared window, and never send a prompt above ~70% of it — route to the chunked path instead. After the call, compare the reported prompt-token count against the estimate and fail loudly on a material shortfall. That is the input-side equivalent of #293 and it is what would have caught a silent truncation.

Depends on the provider abstraction declaring a context limit per provider (v4.2.0).

Acceptance criteria

  • Every LLM call site preflights an estimate against a declared window
  • Prompts above the threshold route to the chunked path rather than being sent and hoped for
  • Reported prompt tokens are compared against the estimate; a shortfall fails loudly
  • The declared window is provider-supplied, not a constant
  • Tests cover under-budget, over-budget, and a provider that reports a truncated prompt
**Severity: CRITICAL.** Found in the August 2026 session lifecycle review (#319). ## The defect **Nothing anywhere estimates prompt size before sending, and nothing checks afterwards whether the server actually consumed the whole prompt.** The #293 work guards the *output* cap only (`llm_service.py:55-92`). `usage.prompt_tokens`, `prompt_eval_count`, and llama.cpp's `truncated` flag are all returned and all ignored. Per provider: - **llama.cpp** (`audio_service.py:2104-2135`) — sends no `max_tokens` and no context options. Builds with context shift **silently drop the front of the prompt**, which can take the system prompt and the character legend with it, and summarise the tail of the session as though it were the whole thing. `finish_reason` is read but only logged. - **OpenAI / Anthropic** — input overflow is a loud 400, so these fail safely on input. ## Proposed fix Estimate tokens before sending (characters ÷ ~3.4 is adequate; a real tokenizer where available), compare against the provider's declared window, and **never send a prompt above ~70% of it** — route to the chunked path instead. After the call, compare the reported prompt-token count against the estimate and fail loudly on a material shortfall. That is the input-side equivalent of #293 and it is what would have caught a silent truncation. Depends on the provider abstraction declaring a context limit per provider (v4.2.0). ## Acceptance criteria - [ ] Every LLM call site preflights an estimate against a declared window - [ ] Prompts above the threshold route to the chunked path rather than being sent and hoped for - [ ] Reported prompt tokens are compared against the estimate; a shortfall fails loudly - [ ] The declared window is provider-supplied, not a constant - [ ] Tests cover under-budget, over-budget, and a provider that reports a truncated prompt
Author
Contributor

Done in ede1ac2, with c54d9af (#337) doing the load-bearing half. One criterion amended, one moved.

What was actually wrong

The body says nothing preflights and nothing checks consumption. That was true when filed and stopped being true in ccc29d6, which built estimate_tokens, prompt_budget_tokens, fits_in_context, LLMConfig.context_tokens, resolve_context_tokens and _warn_if_prompt_truncated. The machinery was all there.

What was missing is that it went almost nowhere. One of eighteen generate_structured_text call sites passed a window (beat_service.py:454). Highlights, the entire lore pipeline, rephrase/promote/statblock/convert_stats, name generation, session titles, journal entries and the GM Workbench tool runner all sent prompts against a window they never declared — and because None meant "skip budgeting entirely", declaring nothing switched off the num_ctx protection too.

Acceptance criteria

  • Every LLM call site preflights an estimate against a declared windowpreflight_prompt runs on all four provider paths in llm_service.py:355-420 and all four in _dispatch_prose. It counts the system prompt too, which shares the window; a preflight that ignores it is the accounting error that drops the speaker legend.
  • The declared window is provider-supplied, not a constantcontext_tokens is now threaded through the 13 domain functions in audio_service and passed from llm_cfg.context_tokens at all 18 call sites (reminder_tasks, lore_service, generation_service, planning_tasks). The constant is the floor for anything undeclared, not the value.
  • Reported prompt tokens are compared against the estimate_warn_if_prompt_truncated, on every provider response.
  • Tests cover under-budget, over-budget, and a provider that reports a truncated prompttest_context_budget.py (including one proving the window survives into the statblock fallback call, not just the first attempt) and test_llm_transport.py:793-830.
  • Prompts above the threshold route to the chunked path rather than being sent and hoped foramended, see below.

The criterion I did not meet as written

"Prompts above the threshold route to the chunked path."

The transport layer cannot route. It has no idea how to split a lore-merge or a statblock prompt — only callers that own a chunked path can, and the two that do (summarise, extraction_windows) already route. Building a generic chunker into the transport would mean inventing a split for prompts whose structure it cannot see.

So over-budget preflights warn loudly and send. It also does not raise, for a second reason: estimate_tokens is characters ÷ 3.4 and deliberately pessimistic, so a raise would refuse prompts that would have worked. Refusing to summarise a session beats summarising a tenth of one — but it does not beat summarising all of it. _warn_if_prompt_truncated reached the same conclusion from the other direction, and both become hard checks once a provider can declare a real tokenizer and whether it front-truncates (#351 / v4.2.0).

Scope moved off this issue

  • /ask — one of the 15 unprotected call sites, and now #426 (v4.2.0). Not fixed here.
  • Parallel map dispatch — belongs to #356, per the note in _summarise_chunked.

One thing worth remembering

Threading context_tokens broke 55 tests across ~20 files, and not visibly. Those files build a fake LLMConfig as a SimpleNamespace or an ad-hoc type(...), none carrying context_tokens. Reading it raised AttributeError inside task bodies that catch exceptions — so the symptom was not a crash but a generation result recorded as failed in tests asserting ready. A hand-rolled double drifts from the dataclass it stands for, and the drift surfaces as a wrong status rather than an error.

1,284 → 1,286 passing, lint clean.

**Done in `ede1ac2`**, with `c54d9af` (#337) doing the load-bearing half. One criterion amended, one moved. ## What was actually wrong The body says nothing preflights and nothing checks consumption. That was true when filed and stopped being true in `ccc29d6`, which built `estimate_tokens`, `prompt_budget_tokens`, `fits_in_context`, `LLMConfig.context_tokens`, `resolve_context_tokens` and `_warn_if_prompt_truncated`. The machinery was all there. What was missing is that it went almost nowhere. **One** of eighteen `generate_structured_text` call sites passed a window ([beat_service.py:454](webapp/backend/app/services/beat_service.py#L454)). Highlights, the entire lore pipeline, rephrase/promote/statblock/convert_stats, name generation, session titles, journal entries and the GM Workbench tool runner all sent prompts against a window they never declared — and because `None` meant "skip budgeting entirely", declaring nothing switched off the `num_ctx` protection too. ## Acceptance criteria - [x] **Every LLM call site preflights an estimate against a declared window** — `preflight_prompt` runs on all four provider paths in [llm_service.py:355-420](webapp/backend/app/services/llm_service.py#L355-L420) and all four in [`_dispatch_prose`](webapp/backend/app/services/audio_service.py#L1888-L1960). It counts the system prompt too, which shares the window; a preflight that ignores it is the accounting error that drops the speaker legend. - [x] **The declared window is provider-supplied, not a constant** — `context_tokens` is now threaded through the 13 domain functions in `audio_service` and passed from `llm_cfg.context_tokens` at all 18 call sites (`reminder_tasks`, `lore_service`, `generation_service`, `planning_tasks`). The constant is the floor for anything undeclared, not the value. - [x] **Reported prompt tokens are compared against the estimate** — `_warn_if_prompt_truncated`, on every provider response. - [x] **Tests cover under-budget, over-budget, and a provider that reports a truncated prompt** — [test_context_budget.py](webapp/backend/tests/test_context_budget.py) (including one proving the window survives into the statblock *fallback* call, not just the first attempt) and [test_llm_transport.py:793-830](webapp/backend/tests/test_llm_transport.py#L793-L830). - [ ] ~~Prompts above the threshold route to the chunked path rather than being sent and hoped for~~ → **amended, see below.** ## The criterion I did not meet as written *"Prompts above the threshold route to the chunked path."* The transport layer cannot route. It has no idea how to split a lore-merge or a statblock prompt — only callers that own a chunked path can, and the two that do (`summarise`, `extraction_windows`) already route. Building a generic chunker into the transport would mean inventing a split for prompts whose structure it cannot see. So over-budget preflights **warn loudly and send**. It also does not raise, for a second reason: `estimate_tokens` is characters ÷ 3.4 and deliberately pessimistic, so a raise would refuse prompts that would have worked. Refusing to summarise a session beats summarising a tenth of one — but it does not beat summarising all of it. `_warn_if_prompt_truncated` reached the same conclusion from the other direction, and both become hard checks once a provider can declare a real tokenizer and whether it front-truncates (**#351 / v4.2.0**). ## Scope moved off this issue - **`/ask`** — one of the 15 unprotected call sites, and now **#426** (v4.2.0). Not fixed here. - **Parallel map dispatch** — belongs to **#356**, per the note in `_summarise_chunked`. ## One thing worth remembering Threading `context_tokens` broke **55 tests** across ~20 files, and not visibly. Those files build a fake `LLMConfig` as a `SimpleNamespace` or an ad-hoc `type(...)`, none carrying `context_tokens`. Reading it raised `AttributeError` *inside* task bodies that catch exceptions — so the symptom was not a crash but a generation result recorded as `failed` in tests asserting `ready`. A hand-rolled double drifts from the dataclass it stands for, and the drift surfaces as a wrong status rather than an error. 1,284 → 1,286 passing, lint clean.
Sign in to join this conversation.
No project
No assignees
1 participant
Notifications
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#336
No description provided.