Stop presenting a truncated /ask answer as a complete one (#426) #486

Merged

claude-bot merged 2 commits from fix/426-ask-context-hygiene into main 2026-09-01 21:07:59 +00:00

Conversation 0 Commits 2 Files changed 9 +487 -32

claude-bot commented 2026-09-01 21:00:44 +00:00

Contributor

Copy link

/ask received none of the context hygiene the summarisation path did. Three defects, one root cause: four near-duplicate transports, and a fix that only ever reached some of them.

What was wrong

function	max_tokens	prompt window	truncation checked
_qa_anthropic	hardcoded 1024	—	no
_qa_openai	hardcoded 1024	—	no
_qa_llamacpp	absent	—	no
_qa_ollama	—	no options at all	no

_qa_ollama is the serious one. With no options dict, Ollama applied its own 2–4k default to the prompt, so the model answered a question it had only been shown part of. That is exactly the defect #337 was filed to fix, still live on this path a release later, on the provider a self-hoster is most likely to reach for.

The others degrade rather than destroy: an answer cut at the output cap is prose, so it simply stops, and there was no finish_reason check to notice. 1024 tokens is small for an answer drawn from ten session summaries plus a 40-entry wiki slice.

Where the flag goes — the design question this issue owned

Truncation flags rather than raises, per your call on #338. That leniency is only defensible while the truncation is visible, and unlike the summary path there is no SummarisationRun to hang it on. So:

answer_question returns a QaAnswer → AskResponse.truncated → the bot renders it in the embed description, not the footer. A GM may act on a partial answer, and footer text is small grey print. The answer budget shrinks to 3900 characters to make room, so the notice cannot push the embed past Discord's 4096-character description cap — otherwise a degraded answer becomes no answer, which is strictly worse.

No BOT_CONTRACT_VERSION bump: the field is additive with a default. An older bot ignores it; a newer bot against an older backend reads it as absent. There's a test for that direction. Both sides change in one commit per the cross-component rule.

Why the transports stay separate

The four _qa_* functions are near-duplicates and folding them into the shared transport is #484 — I'm not smuggling that refactor in here. But the checks now go through _check_usage, the same helper the structured paths use. Duplicated transports are precisely how one of them got a fix the other three did not; sharing the checks means they can't drift apart again even while the transports remain separate.

/ask also repeated the URL sniff independently of llm_service, which is part of how it drifted — it now shares the registry, and gets the preflight it was one of fifteen call sites to miss (#336).

Two test doubles fixed rather than worked around

Both were passing while diverging from reality, which is #441's whole concern:

• test_recap_email used a type("Cfg", ...) shim that lacked context_tokens, so it failed on an AttributeError raised before the code under test. Replaced with a real LLMConfig, which cannot drift.
• test_context_bounds mocked _qa_llamacpp with a bare "ok". Against the new tuple return that would have unpacked into two characters and passed.

Verification

• 1712 backend tests (14 new), 278 bot tests (4 new), ruff check + format clean.
• New tests assert the flag is set on a cut-off response and clear on a clean stop on all four providers — a flag that is always on tells a GM nothing.
• One test asserts a truncated 8000-character answer still fits Discord's embed limit, since that failure mode would swallow the answer entirely.

`/ask` received none of the context hygiene the summarisation path did. Three defects, one root cause: four near-duplicate transports, and a fix that only ever reached some of them. ## What was wrong | function | `max_tokens` | prompt window | truncation checked | |---|---|---|---| | `_qa_anthropic` | hardcoded **1024** | — | **no** | | `_qa_openai` | hardcoded **1024** | — | **no** | | `_qa_llamacpp` | **absent** | — | **no** | | `_qa_ollama` | — | **no `options` at all** | **no** | **`_qa_ollama` is the serious one.** With no `options` dict, Ollama applied its own 2–4k default to the *prompt*, so the model answered a question it had only been shown part of. That is exactly the defect #337 was filed to fix, still live on this path a release later, on the provider a self-hoster is most likely to reach for. The others degrade rather than destroy: an answer cut at the output cap is prose, so it simply stops, and there was no `finish_reason` check to notice. 1024 tokens is small for an answer drawn from ten session summaries plus a 40-entry wiki slice. ## Where the flag goes — the design question this issue owned Truncation **flags rather than raises**, per your call on #338. That leniency is only defensible while the truncation is *visible*, and unlike the summary path there is no `SummarisationRun` to hang it on. So: `answer_question` returns a `QaAnswer` → `AskResponse.truncated` → the bot renders it **in the embed description, not the footer**. A GM may act on a partial answer, and footer text is small grey print. The answer budget shrinks to 3900 characters to make room, so the notice cannot push the embed past Discord's 4096-character description cap — otherwise a degraded answer becomes *no* answer, which is strictly worse. **No `BOT_CONTRACT_VERSION` bump**: the field is additive with a default. An older bot ignores it; a newer bot against an older backend reads it as absent. There's a test for that direction. Both sides change in one commit per the cross-component rule. ## Why the transports stay separate The four `_qa_*` functions are near-duplicates and folding them into the shared transport is #484 — I'm not smuggling that refactor in here. But the **checks** now go through `_check_usage`, the same helper the structured paths use. Duplicated transports are precisely how one of them got a fix the other three did not; sharing the checks means they can't drift apart again even while the transports remain separate. `/ask` also repeated the URL sniff independently of `llm_service`, which is part of how it drifted — it now shares the registry, and gets the preflight it was one of fifteen call sites to miss (#336). ## Two test doubles fixed rather than worked around Both were passing while diverging from reality, which is #441's whole concern: - `test_recap_email` used a `type("Cfg", ...)` shim that lacked `context_tokens`, so it failed on an `AttributeError` raised *before* the code under test. Replaced with a real `LLMConfig`, which cannot drift. - `test_context_bounds` mocked `_qa_llamacpp` with a bare `"ok"`. Against the new tuple return that would have unpacked into two characters and **passed**. ## Verification - 1712 backend tests (14 new), 278 bot tests (4 new), ruff check + format clean. - New tests assert the flag is set on a cut-off response *and clear on a clean stop* on all four providers — a flag that is always on tells a GM nothing. - One test asserts a truncated 8000-character answer still fits Discord's embed limit, since that failure mode would swallow the answer entirely.

claude-bot added 2 commits 2026-09-01 21:00:44 +00:00

fix(ask): stop presenting a truncated /ask answer as a complete one (#426) ... 0a5bd35c61

/ask received none of the context hygiene the summarisation path did. All four
_qa_* functions skipped _reject_if_truncated, so an answer cut off at the output
cap was indistinguishable from a complete one — it is prose, so it simply stops
and there was no finish_reason check to notice.

_qa_ollama was worse. It sent no `options` dict whatsoever, so Ollama applied
its own 2-4k default to the *prompt* and the model answered a question it had
only been shown part of. That is precisely the defect #337 was filed to fix,
still live on this path a release later, on the provider a self-hoster is most
likely to reach for.

Output caps were 1024 on Anthropic and OpenAI — small for prose drawn from ten
sessions plus a wiki slice — and absent entirely on llama.cpp, which meant
whatever the server happened to default to. All four now send _PROSE_MAX_TOKENS,
the same bound the prose summarisation paths use.

Truncation flags rather than raises, per Ryan's call on #338. That leniency is
only defensible while the truncation is *visible*, and unlike the summary path
there is no SummarisationRun to hang a flag on — so answer_question returns a
QaAnswer, AskResponse carries `truncated`, and the bot renders it in the embed
description rather than the footer, because a GM may act on a partial answer and
footer text is small grey print. The answer budget shrinks to 3900 to make room,
so the notice cannot push the embed past Discord's 4096-character cap and turn a
degraded answer into no answer at all.

The field is additive with a default, so no BOT_CONTRACT_VERSION bump: an older
bot ignores it, and a newer bot against an older backend reads it as absent.
Both sides change in this commit, per the cross-component rule.

/ask also repeated the URL sniff independently of llm_service, which is part of
how it drifted; it now shares the registry, and gets the preflight it was one of
fifteen call sites to miss (#336).

The four transports stay separate — folding them into the shared one is #484 —
but the *checks* now go through _check_usage, the same helper the structured
paths use. Duplicated transports are how one of them got a fix the other three
did not; sharing the checks means they cannot drift apart again.

Two test doubles were fixed rather than worked around. test_recap_email's
type("Cfg", ...) shim lacked context_tokens, so it failed on an AttributeError
raised before the code under test — replaced with a real LLMConfig. And
test_context_bounds mocked _qa_llamacpp with a bare string, which would have
unpacked into two characters and passed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

docs(changelog): record the /ask truncation fixes (#426) ... 95fa9cc1ad

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

claude-bot merged commit 233dc71f5e into main 2026-09-01 21:07:59 +00:00

claude-bot deleted branch fix/426-ask-context-hygiene 2026-09-01 21:07:59 +00:00

claude-bot referenced this pull request from a commit 2026-09-01 21:08:01 +00:00

Merge PR #486: stop presenting a truncated /ask answer as a complete one (#426)

claude-bot referenced this pull request 2026-09-01 21:08:17 +00:00

[Backend] /ask is unprotected on every provider — no context budget, no num_ctx, no truncation check #426

claude-bot referenced this pull request 2026-09-05 00:11:57 +00:00

chore(release): v4.1.1 #487

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!486

No description provided.