Skip to content

common/chat, server: refactor, move all conversion functions to common, add tests - #20690

Merged
pwilkin merged 2 commits into
ggml-org:masterfrom
pwilkin:refactor-chat-conversion
Apr 22, 2026
Merged

common/chat, server: refactor, move all conversion functions to common, add tests#20690
pwilkin merged 2 commits into
ggml-org:masterfrom
pwilkin:refactor-chat-conversion

Conversation

@pwilkin

@pwilkin pwilkin commented Mar 17, 2026

Copy link
Copy Markdown
Member

Unifies the conversion for Chat Completions, Responses and Anthropic Messages endpoints by moving the conversion functions to a dedicated conversion file in common.

Adds test for Responses API (currently one test for #20280) + fixes issue with type: message string-only content.

@pwilkin
pwilkin requested review from a team as code owners March 17, 2026 16:36
@github-actions github-actions Bot added testing Everything test related examples server labels Mar 17, 2026
@Naster17

Copy link
Copy Markdown

hm, give a shot for this

@Naster17

Copy link
Copy Markdown

Finally its work Thank you. Dziękuję ;)

@pwilkin

pwilkin commented Mar 19, 2026

Copy link
Copy Markdown
Member Author

Bumpy bump? @ngxson @CISC maybe?

@ngxson ngxson left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

IMO it's a bit strange to move the conversion functions to common, as they are technically only used by server.

Just wondering, it is possible to confine them inside server, maybe: server/server-chat.cpp ?

test-chat can be linked against libserver, the way as llama-cli

@aldehir

aldehir commented Mar 19, 2026

Copy link
Copy Markdown
Contributor

I'm in agreement with @ngxson. I would expect interface choices to be made at the API boundary, instead of propagating internally.

In a way the chat completions API is our "internal" representation, for better or worse.

What are we trying to solve?

@pwilkin

pwilkin commented Mar 19, 2026

Copy link
Copy Markdown
Member Author

I'm trying to make the conversion clearer (and more testable), possibly in preparation for migration to a different system than Chat Completions as our base.

Okay, I'll make a separate class in server/ instead for the other conversions.

@pwilkin
pwilkin force-pushed the refactor-chat-conversion branch from 4d6942c to 8686b80 Compare March 19, 2026 20:17
@pwilkin
pwilkin requested a review from ggerganov as a code owner March 19, 2026 20:51
Comment thread tools/server/server-chat.h Outdated
using json = nlohmann::ordered_json;

// Convert OpenAI Responses API format to OpenAI Chat Completions API format
json common_chat_convert_responses_to_chatcmpl(const json & body);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
json common_chat_convert_responses_to_chatcmpl(const json & body);
json server_chat_convert_responses_to_chatcmpl(const json & body);

Comment thread tools/server/server-chat.h Outdated

#include "chat.h"

#include <nlohmann/json.hpp>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It should be possible to use json_fwd here, right?

@pwilkin
pwilkin force-pushed the refactor-chat-conversion branch from fcad53c to 2438739 Compare March 31, 2026 20:07
@pwilkin
pwilkin requested review from CISC, aldehir and ngxson March 31, 2026 20:08
@pwilkin
pwilkin force-pushed the refactor-chat-conversion branch from 76212e4 to d0bf6ac Compare April 3, 2026 11:21
@pwilkin

pwilkin commented Apr 3, 2026

Copy link
Copy Markdown
Member Author

Okay, errors fixed, bumping since there are already further fixes to the Anthropic API people want.

Comment thread common/chat.cpp Outdated
@pwilkin pwilkin added the merge ready A maintainer can use this label to indicate that they consider the changes final and ready to merge. label Apr 3, 2026
@alejrobinson

Copy link
Copy Markdown

This commit seems to solve the 400 error currently happening in N8N, are there plans to merge it soon?

Thanks and great work!

@pwilkin
pwilkin force-pushed the refactor-chat-conversion branch from 0a5bf62 to 531b71b Compare April 17, 2026 14:01
@pwilkin
pwilkin force-pushed the refactor-chat-conversion branch from 531b71b to 889121d Compare April 17, 2026 14:05
@pwilkin

pwilkin commented Apr 17, 2026

Copy link
Copy Markdown
Member Author

@ggml-org/maintainers need two approvals after merge please.

@alejrobinson

Copy link
Copy Markdown

Just had the time to properly pull and build this PR to test it and it works! All problems I had with N8N are gone.

Hope it get's merged soon, thanks again for the great work!

@pwilkin

pwilkin commented Apr 20, 2026

Copy link
Copy Markdown
Member Author

@ggml-org/maintainers bumpy bump PTAL :)

@pwilkin

pwilkin commented Apr 22, 2026

Copy link
Copy Markdown
Member Author

@CISC could I get a reapproval pls? This has been stuck for so long now :/

@CISC CISC 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.

@CISC could I get a reapproval pls? This has been stuck for so long now :/

Sure, but it won't help, you still need a second approval. :P

@pwilkin

pwilkin commented Apr 22, 2026

Copy link
Copy Markdown
Member Author

@ggml-org/maintainers pretty please :)

@pwilkin
pwilkin merged commit 134d6e5 into ggml-org:master Apr 22, 2026
49 of 51 checks passed
mudler added a commit to mudler/LocalAI that referenced this pull request Apr 23, 2026
Upstream llama.cpp refactor (ggml-org/llama.cpp#20690) moved the
OAI/Anthropic/Responses and transcription conversion helpers out of
server-common.cpp into a new server-chat.cpp, and server-task.cpp and
server-context.cpp now call those symbols (convert_transcriptions_to_chatcmpl,
server_chat_convert_responses_to_chatcmpl, server_chat_convert_anthropic_to_oai,
server_chat_msg_diff_to_json_oaicompat) via server-chat.h.

grpc-server.cpp builds as a single translation unit by #include-ing the
upstream .cpp files directly. Without including server-chat.cpp, the
declarations are satisfied at compile time via server-chat.h but the
link step fails with undefined references once LLAMA_VERSION crosses
the refactor commit (134d6e54).

Guard the include with __has_include so the same source stays buildable
on older LLAMA_VERSION pins that predate the refactor (where prepare.sh
won't copy server-chat.cpp into tools/grpc-server/).

Assisted-by: Claude:claude-opus-4-7 [Claude Code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
mudler added a commit to mudler/LocalAI that referenced this pull request Apr 23, 2026
…it (#9511)

* fix(llama-cpp): include server-chat.cpp in grpc-server translation unit

Upstream llama.cpp refactor (ggml-org/llama.cpp#20690) moved the
OAI/Anthropic/Responses and transcription conversion helpers out of
server-common.cpp into a new server-chat.cpp, and server-task.cpp and
server-context.cpp now call those symbols (convert_transcriptions_to_chatcmpl,
server_chat_convert_responses_to_chatcmpl, server_chat_convert_anthropic_to_oai,
server_chat_msg_diff_to_json_oaicompat) via server-chat.h.

grpc-server.cpp builds as a single translation unit by #include-ing the
upstream .cpp files directly. Without including server-chat.cpp, the
declarations are satisfied at compile time via server-chat.h but the
link step fails with undefined references once LLAMA_VERSION crosses
the refactor commit (134d6e54).

Guard the include with __has_include so the same source stays buildable
on older LLAMA_VERSION pins that predate the refactor (where prepare.sh
won't copy server-chat.cpp into tools/grpc-server/).

Assisted-by: Claude:claude-opus-4-7 [Claude Code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>

* chore(llama-cpp): bump LLAMA_VERSION to 0d0764dfd

Bump to ggml-org/llama.cpp@0d0764d.
Paired with the preceding grpc-server server-chat.cpp include so the
refactor at 134d6e54 links cleanly. Supersedes PR #9494.

Assisted-by: Claude:claude-opus-4-7 [Claude Code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>

---------

Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
my-other-github-account pushed a commit to my-other-github-account/llama.cpp that referenced this pull request May 15, 2026
baramofme pushed a commit to baramofme/llama-cpp-turboquant that referenced this pull request May 23, 2026
fewtarius pushed a commit to fewtarius/CachyLLama that referenced this pull request May 30, 2026
MrLordCat referenced this pull request in MrLordCat/llama.cpp-rdna-lab Jul 16, 2026
…n, add tests (#20690)

* Refactor conversion functions
zommiommy pushed a commit to zommiommy/llama.cpp that referenced this pull request Aug 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

examples merge ready A maintainer can use this label to indicate that they consider the changes final and ready to merge. server testing Everything test related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants