Fix KV cache reuse when changing reasoning_effort

#8
by dormosh - opened

The original template included reasoning_instructions in the system prompt. Since these instructions depend on reasoning_effort, changing the effort changed tokens near the beginning of the prompt, preventing llama.cpp from reusing the existing KV cache.

The fix moves the dynamic reasoning instructions from the system prompt to the final user message. This keeps the conversation prefix unchanged when reasoning_effort changes, allowing llama.cpp, for example, to reuse the existing KV cache and avoid re-evaluating the entire conversation.

That's a great idea!

Thanks! Measured on a three-turn conversation switching low → xhigh (character-prefix reuse, as a proxy for tokens):

scenario current this PR
effort change, same conversation 1.9% 93.9%
constant effort, turn 1 to 2 100% 80.9%
constant effort, turn 2 to 3 100% 89.9%

The bottom two rows are why I don't want it as the default, though: today each turn's prompt is a strict prefix of the next, and putting the instructions on whichever message happens to be last gives that up. Every turn then pays a little to save a lot on effort changes, and for most people turns are much more frequent than effort changes. That's a trade, which makes me think it should be opt-in or not implemented at all.

Two things I'd want fixed either way:

  1. The instructions vanish when the last message isn't a user turn. The new block only fires under message.role == 'user', so in a tool-calling loop ending on a tool result:

last = tool result current -> instructions present: True
last = tool result this PR -> instructions present: False

reasoning_effort would appear to work and quietly do nothing for the whole agentic loop. Emitting after the message loop rather than inside the user branch would cover tool results and assistant prefills uniformly.

  1. chat_template_oneline.txt wasn't regenerated. The minified file is what gets embedded into GGUFs, so as it stands published builds would behave differently from the source here. scripts/minify_jinja.py regenerates it.

Also, unrelated to the KV work: the diff changes _think_end from '\n' to '' in one branch. It's harmless in practice (both split results getrstrip('\n') / lstrip('\n') immediately) but it breaks the invariant every sibling branch holds (set _think_end to the string you matched), so I'd revert it, unless it was deliberate?

But you already helped me: your PR passed our verifier 22/22. The round-trip checefault path, where medium injects nothing and there is nothing for source and minified to disagree about. I've rewritten it to sweep the kwarg and to assert that effort steering survives every message shape so it catches both issues above now. So thanks for that!

If you're up for it: implement the same idea, gated behind a chat_template_kwargs (we shipped "terse" that way in v22.3.2), emitted after the loop, oneline regenerated. I'd take that :)

Ready to merge
This branch is ready to get merged automatically.

Sign up or log in to comment