Skip to content

server: improve user message detection and create checkpoints at every user message#24176

Merged
ggerganov merged 12 commits into
ggml-org:masterfrom
aldehir:improve-messages-spans
Jun 23, 2026
Merged

server: improve user message detection and create checkpoints at every user message#24176
ggerganov merged 12 commits into
ggml-org:masterfrom
aldehir:improve-messages-spans

Conversation

@aldehir

@aldehir aldehir commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Overview

Scan for user message boundaries directly on the tokens and avoid doing a second pass that translates from byte offsets.

Create checkpoints at the start of every user message, as opposed to only the last message.

cc. @ggerganov @jacekpoplawski

Additional information

The checkpoint logic makes sense to me, but I haven't thoroughly tested it for edge cases.

Requirements

  • I have read and agree with the contributing guidelines
  • AI usage disclosure: Yes - used to update the chat params for every model. Also used to review the checkpoint changes.

@aldehir

This comment has been minimized.

@ggerganov ggerganov self-assigned this Jun 5, 2026
@aldehir aldehir force-pushed the improve-messages-spans branch from eb3f6bb to 68bcfa3 Compare June 6, 2026 03:55
@aldehir aldehir force-pushed the improve-messages-spans branch from 68bcfa3 to fd324b1 Compare June 16, 2026 03:27
@aldehir aldehir marked this pull request as ready for review June 16, 2026 03:28
@aldehir aldehir requested review from a team and pwilkin as code owners June 16, 2026 03:28
@jacekpoplawski

Copy link
Copy Markdown
Contributor

Could you explain the use case where I could observe the behavior this PR fixes?

I think my problem is that I implemented #22929 for my own use case, and I don't see any issues with the current implementation. So I assume this is either for a different kind of model (not Qwen 3.6 27B) or for different behavior (not agentic coding in pi).

I remember ggerganov mentioned /tree in pi, but I use /tree in pi often and there is no reprocessing, because each step is stored in the checkpoint.

@aldehir

aldehir commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

I remember ggerganov mentioned /tree in pi, but I use /tree in pi often and there is no reprocessing, because each step is stored in the checkpoint.

If you resume an older chat session, or restart the server, then using /tree after sending a message will trigger it.

@ggerganov

Copy link
Copy Markdown
Member

I remember ggerganov mentioned /tree in pi, but I use /tree in pi often and there is no reprocessing, because each step is stored in the checkpoint.

If you resume an older chat session, or restart the server, then using /tree after sending a message will trigger it.

Yes, that's the main use case.

@ggerganov

Copy link
Copy Markdown
Member

I am testing and something I didn't realize before is that each tool response is actually a "user" message. So this results in a checkpoint created for each tool call. Instead, I was thinking about making the checkpoints only at the points where the user manually provides input. But I realize that maybe we cannot distinguish between the two?

@aldehir

aldehir commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

But I realize that maybe we cannot distinguish between the two?

A tool response message in this case contains the user message preamble as a prefix, so we simply need to check for a tool response first. I can easily do this with the specialized parsers, but @pwilkin will need to assist in providing it for the autoparser variants.

@ggerganov

Copy link
Copy Markdown
Member

But I realize that maybe we cannot distinguish between the two?

A tool response message in this case contains the user message preamble as a prefix, so we simply need to check for a tool response first. I can easily do this with the specialized parsers, but @pwilkin will need to assist in providing it for the autoparser variants.

@aldehir An simpler alternative approach would be:

  • Keep the logic as it is - i.e. checkpoint on all user and tool response messages
  • Bump the default checkpoint_min_step from 256 -> 8192
  • Add extra condition to ignore the min-step condition for the last user message

@pwilkin

pwilkin commented Jun 19, 2026

Copy link
Copy Markdown
Member

I'd tune it to 4096, I think 8192 is a bit too much with agentic sessions with large turnover, but yeah, definitely 256 is too little, the way it works currently is that if the agent calls a lot of small tools at once, then that immediately clobbers the checkpoints pool even at the large default size of 32. The way I see it, a rule of "always keep last user checkpoint, after that, respect the hard limit" would solve most of these problems.

@aldehir

aldehir commented Jun 20, 2026

Copy link
Copy Markdown
Contributor Author
  • Keep the logic as it is - i.e. checkpoint on all user and tool response messages

  • Bump the default checkpoint_min_step from 256 -> 8192

  • Add extra condition to ignore the min-step condition for the last user message

Sounds good, the latest commit should reflect that.

I'd tune it to 4096, I think 8192 is a bit too much with agentic sessions with large turnover

I set it 8192, can change if @ggerganov agrees.

@rankaiyx

rankaiyx commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

I rebased this PR onto the main branch—which includes #24746 —and put it through a full day of intensive testing. Everything seems to be working fine.

I'll set it to 256; the harness I use trims old tool messages, which can improve the KV cache reuse rate.

@ggerganov

Copy link
Copy Markdown
Member

I'd tune it to 4096, I think 8192 is a bit too much with agentic sessions with large turnover

I set it 8192, can change if @ggerganov agrees.

With 8192, on average we will be reprocessing half of the step (i.e. 4096) tokens when restoring a checkpoint. I think reprocessing 4096 tokens should be OK in majority of cases so default of 8192 is better IMO - we are more interested in sparing memory usage.

@ggerganov ggerganov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restoring an old session now works correctly.

Note that when running a current session and we do many small tool calls one after the other, the logic creates a checkpoint for each one because it is always the last user message on each turn. To improve that, we should add a logic to remove the penultimate checkpoint if it does not satisfy the step-size condition. This should make the checkpoints to be both distributed sparsely and also respect the user boundaries. With this improvement I think we should be gucci. Can also do it in a follow-up PR.

@aldehir

aldehir commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

@ggerganov let's do it in a follow up PR.

@ggerganov ggerganov merged commit 73618f2 into ggml-org:master Jun 23, 2026
21 of 25 checks passed
papamoose pushed a commit to papamoose/llama.cpp that referenced this pull request Jun 27, 2026
…y user message (ggml-org#24176)

* server : improve message span logic

* cont : cast size_t to int32_t in comparisons

* server : create checkpoints before every user msg

* chat : remove \n in gemma4 delimiters

* chat : merge msg delimiter structs into one

* cont : reword comment

* cont : initialize tokens in delimiter

* cont : add server_tokens::get_raw_tokens() for mtmd

* cont : move message finding to server_tokens and skip mtmd tokens

* cont : update cohere2moe parser

* cont : increase min-step to 8192 and always produce a chkpt for last user message
adrianhoehne pushed a commit to adrianhoehne/llama.cpp that referenced this pull request Jul 5, 2026
…y user message (ggml-org#24176)

* server : improve message span logic

* cont : cast size_t to int32_t in comparisons

* server : create checkpoints before every user msg

* chat : remove \n in gemma4 delimiters

* chat : merge msg delimiter structs into one

* cont : reword comment

* cont : initialize tokens in delimiter

* cont : add server_tokens::get_raw_tokens() for mtmd

* cont : move message finding to server_tokens and skip mtmd tokens

* cont : update cohere2moe parser

* cont : increase min-step to 8192 and always produce a chkpt for last user message
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

examples server testing Everything test related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants