rbrooks/Quest-Board

Watch

Fork 0

LLM transport: a truncated response (finish_reason=length) is silently repaired into a partial result #293

New issue

Closed

opened 2026-08-07 00:26:38 +00:00 by claude-bot · 1 comment

claude-bot commented

2026-08-07 00:26:38 +00:00

Collaborator

Found while running the #281 spike. Independent of whether we adopt json_schema, and probably higher-value.

The bug

_structured_llamacpp inspects finish_reason only when content is empty (webapp/backend/app/services/llm_service.py:196-212):

content = choice.get("message", {}).get("content") or ""
if not content.strip():
    log.warning("llama.cpp returned empty content (finish_reason=%r, ...)", ...)
    raise RuntimeError(...)
return content.strip()

When the model hits max_tokens and returns finish_reason=length with a non-empty body, that body is a JSON object cut off mid-structure. It is returned as if complete. repair_json_object then does exactly what it is designed to do — preserves the longest parsing prefix — and the caller receives a well-formed object containing some of the results, with no way to tell it is partial.

_structured_openai has the same gap (llm_service.py:283-286).

Reproduced

Same ~33k-token prompt, production-shaped lore-proposal schema, max_tokens forced to 256:

mode	n=4
`json_object`	3/4 `finish_reason=length` → invalid JSON → repaired to 3 proposals
`json_schema`	2/4 `finish_reason=length` → invalid JSON → repaired to 2–3 proposals

Constrained decoding gives no protection — a grammar constrains what gets emitted, not whether generation is cut off. Both modes fail identically.

Why it matters

This is the exact hazard repair_json_object's docstring warns about:

a repaired result can contain an element that is structurally valid but semantically incomplete … callers must validate required fields, not just shape

Here that hazard is reached through a path nothing detects. The failure is silent by construction: the WARNING says "recovered by structural repair", which reads like a successful recovery, and the count that reaches the DB looks like a legitimate short result. A session producing 12 proposals that gets truncated to 3 is indistinguishable from a session that genuinely had 3.

Production default is max_tokens=2048. Observed completions on synthetic prompts ran 300–1,200 tokens, so real sessions with richer content plausibly reach the cap — and would do so more often the better the lore pipeline gets.

Suggested fix

Treat finish_reason=length as a first-class outcome in _structured_llamacpp and _structured_openai: at minimum log it at WARNING with the completion-token count; preferably surface it to the caller so a truncated structured result can be rejected or retried rather than persisted.
Consider raising max_tokens for the proposal-generating paths, or making it caller-specified — 2048 is a guess that predates knowing output sizes.
Where a partial result is unacceptable, extract_json_object(..., strict=True) already exists; a truncation signal makes it usable at the right call sites.

Not in scope

Adopting json_schema (#281) — orthogonal, and does not fix this.

Found while running the #281 spike. Independent of whether we adopt `json_schema`, and probably higher-value. ## The bug `_structured_llamacpp` inspects `finish_reason` **only** when content is empty (`webapp/backend/app/services/llm_service.py:196-212`): ```python content = choice.get("message", {}).get("content") or "" if not content.strip(): log.warning("llama.cpp returned empty content (finish_reason=%r, ...)", ...) raise RuntimeError(...) return content.strip() ``` When the model hits `max_tokens` and returns `finish_reason=length` with a **non-empty** body, that body is a JSON object cut off mid-structure. It is returned as if complete. `repair_json_object` then does exactly what it is designed to do — preserves the longest parsing prefix — and the caller receives a well-formed object containing *some* of the results, with no way to tell it is partial. `_structured_openai` has the same gap (`llm_service.py:283-286`). ## Reproduced Same ~33k-token prompt, production-shaped lore-proposal schema, `max_tokens` forced to 256: | mode | n=4 | |---|---| | `json_object` | 3/4 `finish_reason=length` → invalid JSON → repaired to **3 proposals** | | `json_schema` | 2/4 `finish_reason=length` → invalid JSON → repaired to **2–3 proposals** | Constrained decoding gives **no protection** — a grammar constrains what gets emitted, not whether generation is cut off. Both modes fail identically. ## Why it matters This is the exact hazard `repair_json_object`'s docstring warns about: > a repaired result can contain an element that is *structurally* valid but *semantically* incomplete … callers must validate required fields, not just shape Here that hazard is reached through a path nothing detects. The failure is silent by construction: the WARNING says "recovered by structural repair", which reads like a successful recovery, and the count that reaches the DB looks like a legitimate short result. A session producing 12 proposals that gets truncated to 3 is indistinguishable from a session that genuinely had 3. Production default is `max_tokens=2048`. Observed completions on synthetic prompts ran 300–1,200 tokens, so real sessions with richer content plausibly reach the cap — and would do so more often the better the lore pipeline gets. ## Suggested fix 1. Treat `finish_reason=length` as a first-class outcome in `_structured_llamacpp` and `_structured_openai`: at minimum log it at WARNING with the completion-token count; preferably surface it to the caller so a truncated structured result can be rejected or retried rather than persisted. 2. Consider raising `max_tokens` for the proposal-generating paths, or making it caller-specified — 2048 is a guess that predates knowing output sizes. 3. Where a partial result is unacceptable, `extract_json_object(..., strict=True)` already exists; a truncation signal makes it usable at the right call sites. ## Not in scope Adopting `json_schema` (#281) — orthogonal, and does not fix this.

claude-bot added the

backend

label

2026-08-07 00:29:24 +00:00

rbrooks referenced this issue from a commit

2026-08-07 03:44:54 +00:00

fix(backend): reject an LLM response truncated at the output cap (#293)

claude-bot commented

2026-08-07 03:45:21 +00:00

Author

Collaborator

Fixed in f3264d9 on main.

Correction to the issue body first

I overstated the production exposure when I filed this. The body says "Production default is max_tokens=2048 … real sessions with richer content plausibly reach the cap", using the lore-proposal path as the example. That path is not capped at 2048.

Auditing every structured call site:

call site	cap	output size
`extract_highlights`	2048 (default)	bounded by `highlights_max_quotes`
`generate_session_titles`	2048 (default)	a handful of short titles
`_propose_lore_relationships_async`	2048 (default)	small relationship list
lore extract / dedup / match	`max_tokens=None`	field omitted entirely
`merge_lore_entry_body`	2048 explicit	`json_mode=False`, so prose

Only three structured sites inherit the 2048 default, and all three produce naturally bounded output well under it. The lore pipeline — the one I used to illustrate "12 proposals truncated to 3" — passes max_tokens=None, which omits the field, so llama.cpp generates until EOS or the context window fills.

The bug is real and the code path is exactly as described; its current reach is narrower than I implied. Those uncapped calls can still reach this guard by exhausting the 131k context, and finish_reason=length covers that case too — so the fix is worth having on its own merits, but I don't want a misleading exposure claim sitting in the tracker.

This also means suggested fix #2 (raise max_tokens) is not needed: nothing is being squeezed by the current cap, so raising it would be tuning a constraint that isn't binding.

What landed

Rejects truncated structured output; returns truncated prose. A response cut off at the cap is malformed rather than partial, and the caller cannot detect the loss on its own — whereas shortened prose is degraded but usable.

Covers all four providers, not just the two named here — each signals the condition differently:

provider	signal
llama.cpp / OpenAI	`finish_reason == "length"`
Anthropic	`stop_reason == "max_tokens"`
Ollama	`done_reason == "length"`

Neither prompt nor response is logged — only the token count.

On raising being safe: I checked this against the #287 stuck-state hazard before choosing to raise. LoreGenerationTask.on_failure fires once after retries are exhausted and refuses to clobber a terminal state a later phase already reached, so a rejected response surfaces as failed with a recorded error rather than stranding a session mid-pipeline.

Tests — 7 new in test_llm_transport.py, one per provider plus a negative case (finish_reason=stop must not trip the guard) and a prose case. The last one pins why the guard exists: it feeds the same truncated body to repair_json_object and asserts it comes back as a well-formed object holding one proposal — the silent loss this prevents.

888 backend tests pass (up from 881), ruff clean.

Closing.

Fixed in `f3264d9` on `main`. ## Correction to the issue body first I overstated the production exposure when I filed this. The body says "Production default is `max_tokens=2048` … real sessions with richer content plausibly reach the cap", using the lore-proposal path as the example. That path is **not** capped at 2048. Auditing every structured call site: | call site | cap | output size | |---|---|---| | `extract_highlights` | 2048 (default) | bounded by `highlights_max_quotes` | | `generate_session_titles` | 2048 (default) | a handful of short titles | | `_propose_lore_relationships_async` | 2048 (default) | small relationship list | | lore extract / dedup / match | **`max_tokens=None`** | field omitted entirely | | `merge_lore_entry_body` | 2048 explicit | `json_mode=False`, so prose | Only three structured sites inherit the 2048 default, and all three produce naturally bounded output well under it. The lore pipeline — the one I used to illustrate "12 proposals truncated to 3" — passes `max_tokens=None`, which omits the field, so llama.cpp generates until EOS or the context window fills. The bug is real and the code path is exactly as described; its **current** reach is narrower than I implied. Those uncapped calls can still reach this guard by exhausting the 131k context, and `finish_reason=length` covers that case too — so the fix is worth having on its own merits, but I don't want a misleading exposure claim sitting in the tracker. This also means suggested fix #2 (raise `max_tokens`) is **not** needed: nothing is being squeezed by the current cap, so raising it would be tuning a constraint that isn't binding. ## What landed Rejects truncated structured output; returns truncated prose. A response cut off at the cap is malformed rather than partial, and the caller cannot detect the loss on its own — whereas shortened prose is degraded but usable. Covers **all four** providers, not just the two named here — each signals the condition differently: | provider | signal | |---|---| | llama.cpp / OpenAI | `finish_reason == "length"` | | Anthropic | `stop_reason == "max_tokens"` | | Ollama | `done_reason == "length"` | Neither prompt nor response is logged — only the token count. **On raising being safe:** I checked this against the #287 stuck-state hazard before choosing to raise. `LoreGenerationTask.on_failure` fires once after retries are exhausted and refuses to clobber a terminal state a later phase already reached, so a rejected response surfaces as `failed` with a recorded error rather than stranding a session mid-pipeline. **Tests** — 7 new in `test_llm_transport.py`, one per provider plus a negative case (`finish_reason=stop` must not trip the guard) and a prose case. The last one pins *why* the guard exists: it feeds the same truncated body to `repair_json_object` and asserts it comes back as a well-formed object holding one proposal — the silent loss this prevents. 888 backend tests pass (up from 881), ruff clean. Closing.

claude-bot closed this issue

2026-08-07 03:45:26 +00:00

rbrooks referenced this issue from a commit

2026-08-07 03:49:11 +00:00

style(backend): ruff format test_llm_transport.py (#293 follow-up)

claude-bot referenced this issue

2026-08-07 06:03:06 +00:00

chore(release): v3.11.4 #295

claude-bot referenced this issue

2026-08-25 20:38:55 +00:00

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

claude-bot referenced this issue