llm_chat_respond() errors and corrupts the chat when the context fills, instead of stopping #28

Summary

When a chat fills its context, llm_chat_respond() returns a SQL error, discards the text it already generated, and leaves the conversation in a state where every subsequent turn also fails. A full context is a normal terminal condition for a turn, not an error, and should be reported the way every other LLM API reports it: return what was generated, plus a reason.

Found while stabilising CI (#27), where this surfaced as flaky test failures across arm64 and Vulkan jobs.

Reproduction

.load ./dist/ai
SELECT llm_model_load('tests/models/unsloth/gemma-3-270m-it-GGUF/gemma-3-270m-it-UD-IQ2_M.gguf');
SELECT llm_context_create_chat('context_size=64');
SELECT llm_chat_create();
SELECT 'turn1=[' || llm_chat_respond('Write a long detailed essay about the history of computing.') || ']';
SELECT 'turn2=[' || llm_chat_respond('Hi') || ']';

Error near line 5: Failed to decode prompt batch (1: could not find a KV slot for the batch, context is full)
Error near line 6: Context size exceeded (256, 280)

Note turn 2 fails too, with a different error, on a prompt of two characters.

Current behaviour

Two exits in llm_chat_generate_response() (src/sqlite-ai.c:1805), both return false:

A — the guard (:1812)

uint32_t n_ctx      = llama_n_ctx(ctx);
int32_t  n_ctx_used = llama_memory_seq_pos_max(llama_get_memory(ctx), 0);
if (n_ctx_used + batch.n_tokens > n_ctx) {
    sqlite_common_set_error(..., "Context size exceeded (%d, %d)", ...);
    return false;
}

B — the decode (:1820)

int32_t drc = llama_decode(ctx, batch);
if (drc != 0) { ... "Failed to decode prompt batch (%d: %s)" ... return false; }

They are the same condition. Which one fires is decided by a bug in A:

• llama_memory_seq_pos_max() returns the highest position; occupancy is pos_max + 1. The guard treats a position as a count, so it permits exactly one token past capacity — and llama_decode catches what it let through, returning 1. That is why the reproduction hits B on a fresh turn.
• The comparison is also int32_t against uint32_t, so the left side converts to unsigned. On an empty cache seq_pos_max returns -1; with a zero-token batch that becomes 0xFFFFFFFF and the guard trips falsely. Latent today (batches are ≥ 1 token), but a trap.

Why the failure is sticky

In llm_chat_run() (:1975):

while (1) {
    if (!llm_chat_generate_response(ai, NULL, &is_eog)) return false;   // bails
    if (is_eog) break;
}
if (llm_chat_save_response(ai, messages, template) == false) return false;

1. The generated tokens sit in ai->chat.response but sqlite3_result_text() never runs — the caller gets an error instead of the partial reply.
2. The user message was appended at :1922, before generation. llm_chat_save_response() never runs, so no assistant turn is appended and prev_len is not advanced. The history keeps an orphan user turn.
3. Because prev_len is stale, the next turn's template delta re-includes the orphan and is re-fed into a KV cache that is still full — so it fails too. The chat is permanently unusable, not just that one turn. That is turn2 above: 280 tokens against a 256 context, for the prompt 'Hi'.

There is also an inconsistency: the streaming vtab path does keep the partial reply. llm_chat_cursor_next() returns SQLITE_ERROR, but xClose still calls llm_chat_save_response(). Streaming and non-streaming disagree about what a full context means.

Proposed fix

Treat "context is full" as a terminal condition of the turn, in the same class as EOG.

1. Make the guard exact, so the condition is decided in one place:

   int32_t n_ctx      = (int32_t)llama_n_ctx(ctx);
   int32_t n_ctx_used = llama_memory_seq_pos_max(llama_get_memory(ctx), 0) + 1;  // pos -> count
   if (n_ctx_used + batch.n_tokens > n_ctx) { /* stop */ }

   Both operands signed; the +1 fixes the off-by-one. llama_decode then never has to catch an over-large batch, so a non-zero return goes back to meaning a genuine error (2, -1, < -1) and keeps erroring.

2. Stop instead of erroring. Where the guard trips, set *is_eog / c->is_eog = true and return true without setting an error.

3. Record why — ai->chat.stop_reason ∈ { STOP_EOG, STOP_CONTEXT_FULL }.

4. Let the normal path finish. llm_chat_run() breaks out of its loop, llm_chat_save_response() runs, the partial reply is appended to history, prev_len advances, and llm_chat_respond() returns the text generated so far. The sticky corruption disappears.

5. Expose the reason — llm_chat_stop_reason() returning 'eog' or 'context_full'. Not optional: without it, step 4 turns a loud error into a silent truncation, which is worse for anyone storing the output from a trigger.

This matches OpenAI's finish_reason: "length" and Anthropic's stop_reason: "max_tokens" — partial text plus a flag — and makes the non-streaming path agree with what the vtab already does.

Non-goals

• Multi-turn is not fixed. After a truncated turn the context is still full, so the next turn stops almost immediately with a near-empty reply. Fixing that needs context shifting (evicting the oldest turns and re-basing positions), which is a feature, not a bug fix. This makes the wall graceful, not absent.
• This is a behaviour change. Callers relying on a SQL error to detect overflow will get a short string instead. That is why step 5 should land in the same commit.

Regression test

test_chat_context_full_stops_gracefully in tests/c/unittest.c:

load model
llm_context_create_chat('context_size=1024')      -- NOT 512, see below
install a seeded sampler                          -- install_seeded_chat_sampler(), from #27
llm_chat_create()

turn 1: a prompt long enough to exhaust the context
  assert  llm_chat_respond() succeeds (no SQL error)
  assert  the returned text is non-empty
  assert  llm_chat_stop_reason() = 'context_full'

  assert  ai_chat_messages contains BOTH the user turn and the assistant turn
          (today the assistant turn is missing)

turn 2: llm_chat_respond('Hi') on the same connection
  assert  it does not return "Context size exceeded" — the chat is still usable
          (this is the part that fails today, and the real point of the fix)

control: a short prompt on a fresh chat
  assert  llm_chat_stop_reason() = 'eog'

The turn-2 assertion is the important one: turns 1 and 3 only check the new surface, but turn 2 is what proves the state corruption is gone.

Use context_size=1024, not 512 — see below.

Related: context_size=512 is silently ignored

Not the same bug, but it made this one much worse and should probably be fixed alongside. src/sqlite-ai.c:2736:

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 n_ctx_train"
}

The intent is "auto-size when the caller did not set context_size", but it detects that by comparing the value against llama's default, which is 512. So an explicit context_size=512 is indistinguishable from unset:

Same on llm_context_create(), llm_context_create_chat() and llm_context_create_textgen().

Consequence: anyone asking for a deliberately small 512-token context gets the model's full window instead — 64× more KV cache than requested. It is also why a runaway generation in the test suite cost minutes instead of failing fast, and the likely explanation for the ~16-minute arm64 jobs in #27.

Fix: track explicitly whether the key was present in the options string (a bool context_size_set in llm_options, or a sentinel of 0/UINT32_MAX that a caller cannot plausibly pass) rather than inferring it from the value.