[Dgarc359]

Overview

/v1/messages does not pass the full set of llama.cpp sampling settings, only temperature, top_k, top_p, stream, and chat_template_kwargs. llama's sampling settings are dropped if passed to this endpoint. This PR forwards the 24 sampling names from server-schema.cpp when the request includes them. Without them, the endpoint behaves the same as before. As far as the implementation, there is a better way to do this which would require more changes to the codebase, I wanted to refrain from that since this is my first PR. Ideally the server-schema.cpp file would mark the right fields as a sampling field and we can import that for the endpoint, so the set of sampling settings is always dynamic. If there is interest in that, I can pursue further in a follow up PR. Thanks!

Additional information

To reporduce this issue send seed: 4242, min_p: 0.33, presence_penalty: 1.5 to each endpoint, then read GET /slots

After v1/messages: seed 4294967295, min_p 0.05, presence_penalty 0.0. After /v1/chat/completions: 4242, 0.33, 1.5

edit: as an addition, on master, /v1/chat/completions and /v1/responses both pass but /v1/messages fails, with the patch, all three are now passing with the added test

Requirements

• I have read and agree with the contributing guidelines
• AI usage disclosure: YES - Used for debugging and testing

[ServeurpersoCom]

Hi, I think the existing mechanism already covers this: the Responses conversion copies the whole body and only rewrites its own keys, so every llama.cpp param passes through for free. I suggested doing the same for Anthropic on #28554, which would avoid a second hardcoded list duplicating server-schema.cpp. Would you like to try that approach here instead?

[Dgarc359]

@ServeurpersoCom I see what you mean, after reviewing your recommendation I adjusted the PR. Let me know how it looks. If you need me to squash the commits down to one I can also do that

[ServeurpersoCom]

@ServeurpersoCom I see what you mean, after reviewing your recommendation I adjusted the PR. Let me know how it looks. If you need me to squash the commits down to one I can also do that

No need to worry about squashing (combining commits); it’s pointless:) Because the process is automated for all PRs. Instead, use Git in a clean way, with atomic commits.

[ServeurpersoCom]

That's much simpler and it also covers id_slot, so #28554 becomes redundant once this lands. One small nit: I think the comment could be one short line, see AGENTS.md on code comments.

[Dgarc359]

That's much simpler and it also covers id_slot, so #28554 becomes redundant once this lands. One small nit: I think the comment could be one short line, see AGENTS.md on code comments.

Reduced the comment to a shorter line, thanks for the feedback!

[ServeurpersoCom]

LGTM, I wonder why this filter was there in the first place, the OAI chat endpoint already documents llama.cpp specific params as supported (https://github.com/ggml-org/llama.cpp/blob/master/tools/server/README.md#post-v1chatcompletions-openai-compatible-chat-completions-api) and nothing in the Anthropic one (https://github.com/ggml-org/llama.cpp/blob/master/tools/server/README.md#post-v1messages-anthropic-compatible-messages-api) needed them dropped. Tested on my side: seed, min_p, presence_penalty, dry_multiplier and id_slot now reach the slot on /v1/messages, streamed or not, and a full Anthropic request ends up with the same slot params as master. This supersedes #28554.

[ServeurpersoCom]

@Dgarc359 #28554 just got merged and conflicts with this one, could you rebase? It's only the pass-through list, which now has id_slot: just drop it as your PR does, and keep the new test_anthropic_id_slot since it now covers your change too.

@ngxson what do you think about removing the Anthropic pass-through filter entirely, so the conversion copies the whole body like the Responses one does?

[ngxson]

not sure, the risk is that there can be (or will be) conflict params between llama.cpp schema vs anthropic schema

maybe better to add a new field extra_params and put all llama.cpp relate params there, leave the original code for compat

example:

"extra_params": {
  "slot_id": ...
}

[ServeurpersoCom]

not sure, the risk is that there can be (or will be) conflict params between llama.cpp schema vs anthropic schema

maybe better to add a new field extra_params and put all llama.cpp relate params there, leave the original code for compat

example:

"extra_params": {
  "slot_id": ...
}

Makes sense, there's no conflict today but a namespaced field keeps us safe from future Anthropic keys. @Dgarc359 could you switch the PR to that: standard Anthropic params stay at the top level through the current pass-through list, and only llama.cpp specific params go into an extra_params object merged into the converted body?

[ServeurpersoCom]

@ngxson I wonder if it's worth it though, a future clash would be a one line fix in the converter, while the list needs a PR for every new param. Your call, both work for me...

[ngxson]

I'd prefer a namespaced extra_params since it's more future-proof and less maintenance burden, also it's more explicitly what's being override

and OAI /v1/responses API should also do the same

[ServeurpersoCom]

Makes sense @ngxson. @Dgarc359 so the pass-through list stays as it is, llama.cpp params go into an extra_params object merged into the converted body, and /v1/responses can get the same extra_params in a follow-up PR.

[Dgarc359]

@ServeurpersoCom it's worth taking another look now, top level keys are kept while the extra keys come in extra_params I can follow up with a separate PR for /v1/responses shortly.

@ngxson thanks for the suggestion! Im on the same page and would like to make sure maintenance burden doesnt increase with my changes. I think latest changes slot in better now

[ServeurpersoCom]

Changes requested by ngxson re-tested OK on my side