Skip to content

Let the tool-call opener win over a reasoning close, and treat an absent KV cache as already cleared - #30

Open
mcharytoniuk wants to merge 1 commit into
mainfrom
fix-tool-call-open-shadowed-by-reasoning-close
Open

Let the tool-call opener win over a reasoning close, and treat an absent KV cache as already cleared#30
mcharytoniuk wants to merge 1 commit into
mainfrom
fix-tool-call-open-shadowed-by-reasoning-close

Conversation

@mcharytoniuk

Copy link
Copy Markdown

Two regressions found while upgrading Paddler from 0.12.0 to 0.13.0. Both were bisected against 0.12 on the same machine, model files and prompts.

1. A tool-call opener that is also a reasoning close is swallowed

For Qwen3.5, 0.13 reports reasoning_closes: [[248069], [248058]] while tool_call_open is [248058] — the same token is in both lists, which is correct (<tool_call> really does end reasoning). try_consume_marker_at_tail probed reasoning closes before the tool-call open, so the token was consumed as a reasoning terminator and the tool-call section never opened. Downstream the whole call was classified as content and never reached parse_chat_message.

Measured on the same prompt (283 prompt tokens, greedy sampling, byte-identical model output): 0.12 produced 25 tool-call tokens and a parsed call; 0.13 produced 0 tool-call tokens and 26 content tokens.

Probing the tool-call open first fixes it, and is the better answer inside a reasoning section too: <tool_call> there ends reasoning and starts the call, so landing in ToolCall beats landing in Content. A close marker that is only a reasoning close is unaffected, so the existing suppression of a stray </think> in content still holds.

2. clear_kv_cache fails on models that have no KV cache

nomic-embed-text-v1.5 is non-causal and its context has no memory module, so llama_get_memory returns null. 0.12's clear_kv_cache was infallible and no-opped; 0.13 returns MemoryHandleUnavailable, which broke embedding-only agents.

Mutating an absent KV cache is vacuously satisfied, so clear_kv_cache, clear_kv_cache_seq and kv_cache_seq_keep now succeed with nothing to do. copy_kv_cache_seq still errors, and kv_cache_seq_add / kv_cache_seq_div / kv_cache_seq_pos_max keep their own handling — they ask for or move data rather than assert an end state.

Tests

Written first, confirmed failing, then fixed:

  • sampled_token_classifier: two unit tests for a token shared between reasoning_closes and tool_call_open, from both the content and reasoning sections.
  • kv_cache_without_memory_module: an LLM test on nomic-embed-text-v1.5 asserting the three mutations succeed.

make clippy, make test.unit (884 passing) and the full llama-cpp-bindings-tests suite (15 model phases, 0 failures) all pass on CUDA.

One thing worth your call:clear_kv_cache and kv_cache_seq_keep can no longer fail, so their Result is now always Ok. Making them infallible would be the honest signature and would match 0.12, but it is a breaking change, so I left them as-is.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@mcharytoniuk