fix(chat): keep the chat usable when it runs out of context - #30
Merged
Conversation
Running a chat out of context did not just fail the turn, it corrupted the
conversation: every later turn failed too, and the failure got worse each time.
turn 11: Context size exceeded (256, 266)
turn 12: Context size exceeded (256, 276)
turn 13: Context size exceeded (256, 286)
turn 14: Context size exceeded (256, 296)
llm_chat_run() appends the user message to the history before generating, then
bails straight out of the token loop on failure - never reaching
llm_chat_save_response(). So the user turn is stranded with no assistant reply
and prev_len is never advanced, which makes the template delta for the next turn
re-include the stranded message. Hence the number climbing by a turn's worth of
tokens on every retry, and a chat that can never be used again.
Commit the partial turn before reporting the failure. The streaming cursor path
already does this in xClose for the same reason; the non-streaming path
disagreeing with it was the bug. Same run now reports a stable
Context size exceeded (256, 267)
on turn 11 and on every attempt after it, and the saved history has an assistant
row for every user row instead of trailing orphans (28 rows vs 24).
Also fixes the guard that decides this. llama_memory_seq_pos_max() returns the
highest position in the sequence, so occupancy is that + 1; treating it as a
count let exactly one over-large batch through for llama_decode() to reject with
"could not find a KV slot" instead - which is why the failure used to surface
from the decode rather than from the guard meant to prevent it. That is the 266
-> 267 difference above. The comparison also promoted int32_t to uint32_t, so an
empty cache (seq_pos_max == -1) with a zero-token batch tripped it falsely.
This is the recoverable half of #28. The turn still returns an error rather than
the partial reply plus a stop reason; that part is deliberately left open, since
it changes the contract and deserves its own decision.
test_chat_context_full_is_recoverable talks until the context fills, then keeps
going, and asserts the guard reports it, that the requirement does not grow
across retries, and that the history does not end on a user turn with no reply.
Verified to fail against the pre-fix build with "failure grew across retries".
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012KRojTrn4Q4SaZQpqcwpAt
The new test asserted that every failure after the context fills reports the
same requirement. That held on macOS but failed on all four Linux jobs:
first: Context size exceeded (256, 257)
last: Context size exceeded (256, 269)
Both numbers are correct, and the assertion was wrong to compare them. 257 is
one token over: the guard tripped mid-generation, where the batch is a single
sampled token. 269 is thirteen over: a retry, where the batch is the whole
re-sent user message against a cache that is already full. Two different batch
sizes, so two different numbers - nothing to do with the corruption the test is
meant to catch. On macOS generation happened to stop at EOG before filling the
cache, so the first failure was also a prompt-batch one and the numbers matched.
Record every failure instead of just the first and last, and compare only from
the second onward - those are all prompt-batch retries sending the same message
against the same full cache, so they must agree. Still catches the regression;
against the pre-fix build the four failures read
266, 276, 286, 296
which is the stranded user turn being re-sent on every retry.
Also prints all of them on failure, which is what made the CI result diagnosable
in the first place.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012KRojTrn4Q4SaZQpqcwpAt
andinux
added a commit
that referenced
this pull request
Aug 24, 2026
llm_context_create*() decided whether the caller had asked for a context size by
comparing the parsed value against llama_context_default_params().n_ctx:
struct llama_context_params defaults = llama_context_default_params();
if (ai->model && ctx_params.n_ctx == defaults.n_ctx) {
ctx_params.n_ctx = 0; // 0 = use the model's training window
}
That default is 512 (llama-context.cpp:2772), a value a caller can perfectly
well pass, so an explicit context_size=512 was indistinguishable from unset and
was silently replaced by the model's full training window:
context_size=64 -> n_ctx=256
context_size=512 -> n_ctx=32768 <-- asked for 512, got 64x that
context_size=513 -> n_ctx=768
n_ctx=512 hit it too, since the check looked at the resolved value rather than
at which key was written.
The intent was right, only the detection was wrong. llama.cpp already defines
n_ctx = 0 as "use the training window", so start from that sentinel instead of
inferring it after the fact: the caller's value now always survives, and 0 keeps
its documented meaning. Also stops context_size=0 driving n_batch to 0, which
llama will not accept.
Behaviour change, and a quiet one: anyone passing context_size=512 or n_ctx=512
was getting the model's whole window and now gets 512. Nothing errors - the
context simply becomes what was asked for, so a conversation that used to fit
may now reach the limit. That is the point: silently ignoring the configuration
is the bug. #30 makes reaching the limit recoverable rather than fatal to the
chat; the remaining half of #28 - returning the partial reply plus a stop reason
instead of an error - is still open.
API.md said llm_context_create_chat() and llm_context_create_textgen() were
equivalent to context_size=4096. Their presets are empty, so both inherit the
model's training window; documented as such, along with 0 on context_size/n_ctx.
test_context_size_is_honoured covers 256/512/1024 exactly (llama pads n_ctx to a
multiple of 256), both spellings, and that omitting the key - or passing 0 -
still auto-sizes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012KRojTrn4Q4SaZQpqcwpAt
This was referenced Aug 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The recoverable half of #28. Companion to #29 — that PR makes a small
context_sizeactually take effect, which makes this path far easier to reach.The bug
Running a chat out of context didn't just fail the turn, it corrupted the conversation. Every later turn failed too, and the failure got worse each time:
llm_chat_run()appends the user message to the history before generating, then bails straight out of the token loop on failure — never reachingllm_chat_save_response(). So the user turn is stranded with no assistant reply andprev_lenis never advanced, which makes the template delta for the next turn re-include the stranded message. Hence the requirement climbing by a turn's worth of tokens on every retry, and a chat that can never be used again.The fix
Commit the partial turn before reporting the failure. The streaming cursor path already does exactly this in
xClose; the non-streaming path disagreeing with it was the bug.Same 14-turn run, after:
Stable. And the saved history has an assistant row for every user row instead of trailing orphans — 28 rows vs 24.
Also fixes the guard that decides this.
llama_memory_seq_pos_max()returns the highest position, so occupancy is that+ 1. Treating it as a count let exactly one over-large batch through forllama_decode()to reject withcould not find a KV slot— which is why the failure used to surface from the decode rather than from the guard meant to prevent it. That's the266 → 267difference above. The comparison also promotedint32_ttouint32_t, so an empty cache (seq_pos_max == -1) with a zero-token batch tripped it falsely.What is deliberately not in this PR
The turn still returns an error, not the partial reply plus a stop reason. That half of #28 changes the contract —
llm_chat_respond()would start returning short strings where it used to raise — and deserves its own decision. #28 stays open for it.Worth knowing about the current behaviour: when the guard trips before anything is decoded (a single prompt larger than the whole context), the committed turn is an empty assistant message. That keeps
prev_lenand the history mutually consistent, which is the goal here, but a rollback of the user turn would be more precise. Doing it properly means distinguishing "nothing entered the KV cache" from "generation stopped part-way", which fits naturally with the stop-reason work rather than ahead of it.Testing
test_chat_context_full_is_recoverabletalks until the context fills, then keeps going, and asserts:Context size exceeded), notllama_decode;Verified to fail against a pre-fix build:
make test: 44/44 pass.Merge order: this PR must go in before #29
Not a preference — the
releasejob runs on every push tomainand gates onSQLITE_AI_VERSIONdiffering from the latest published release:mainand the latest release are both 1.0.5, and #29 carries the bump to 1.0.6.context_sizefix; this PR then lands with the version already equal to the latest release, so it is never released until someone bumps again — and 1.0.6 would make the context limit far easier to hit without the fix that keeps the chat recoverable when you do.The version bump stays in #29 only.
🤖 Generated with Claude Code
https://claude.ai/code/session_012KRojTrn4Q4SaZQpqcwpAt