Skip to content

chat : refactor API - #30210

Merged
aldehir merged 21 commits into
ggml-org:masterfrom
aldehir:chat-refactoring
Oct 9, 2026
Merged

aldehir merged 21 commits into
ggml-org:masterfrom
aldehir:chat-refactoring

Conversation

@aldehir

@aldehir aldehir commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Overview

Refactored the interaction between the server and chat components through a common_chat_session object:

// Short snippet of the API
class common_chat_session {
  public:
    common_chat_session(const common_chat_templates *        tmpls,
                        const llama_vocab *                  vocab, // vocab is used only on construction, not stored internally.
                        const common_chat_templates_inputs & inputs,
                        const common_chat_session_params &   params = {});

    const common_chat_msg & feed(const common_chat_input & chunk);

    const common_chat_msg & finish(const common_chat_input & chunk = {});
};

Moved various operations (chat template detection, autoparser analysis, tokenization) to chat_common_template construction so they only run once per template and not on every request.

Removed the PEG parser's JSON serialization that was put in place to integrate with the existing code.

The underlying flow in common/ is still the same: init + parse, but this opens up the opportunity to refactor that in isolation.

Requirements

  • I have read and agree with the contributing guidelines
  • AI usage disclosure: Yes, provided a high level design and iterated w/ Opus 5.5 and Fable 5.1

@aldehir
aldehir requested review from a team, ggerganov and pwilkin as code owners October 9, 2026 07:27
@github-actions github-actions Bot added documentation Improvements or additions to documentation testing Everything test related server labels Oct 9, 2026

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

Looking good :) gonna run the bot review just in case.

@pwilkin

pwilkin commented Oct 9, 2026

Copy link
Copy Markdown
Member

/bot review

@ggml-gh-bot

ggml-gh-bot Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

❌ Code review failed.

Error: pi agent produced an empty review

@ServeurpersoCom

ServeurpersoCom commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Nice refactor, I tested it in my Debian container with Qwen3 30B A3B Thinking (autoparser) and Gemma 4 E4B (dedicated handler) across plain, tools, streaming, n=2, continuation with and without echo, and everything behaves as expected.

The CI failures are unrelated to this PR.

A few leftovers from the old JSON path are now dead code. Outputs are byte identical with them removed, here is the patch:

diff --git a/common/chat.cpp b/common/chat.cpp
index bfa9d2bc73..87f1e99a2b 100644
--- a/common/chat.cpp
+++ b/common/chat.cpp
@@ -115,9 +115,7 @@ const char * common_chat_role_to_string(common_chat_role role) {

 void common_chat_msg_delimiters::tokenize(const llama_vocab * vocab) {
     for (auto & d : delimiters) {
-        if (d.tokens.empty()) {
-            d.tokens = common_tokenize(vocab, d.delimiter, false, true);
-        }
+        d.tokens = common_tokenize(vocab, d.delimiter, false, true);
     }
 }

diff --git a/common/chat.h b/common/chat.h
index 522fb9c109..7753349242 100644
--- a/common/chat.h
+++ b/common/chat.h
@@ -317,16 +317,10 @@ common_chat_input common_chat_input_tokenize(const llama_vocab * vocab, const st
 // per-message parsing syntax
 // should be derived from common_chat_params
 struct common_chat_parser_params {
-    common_chat_format      format               = COMMON_CHAT_FORMAT_CONTENT_ONLY;
-    common_reasoning_format reasoning_format     = COMMON_REASONING_FORMAT_NONE; // TODO: refactor this to "bool parse_reasoning"
-    // Whether reasoning_content should be inlined in the content (e.g. for reasoning_format=deepseek in stream mode)
-    bool                    reasoning_in_content = false;
-    common_chat_input       generation_prompt;
-    bool                    parse_tool_calls     = true;
-    bool                    is_continuation      = false;
-    bool                    echo                 = false;  // Include assistant prefilled msg in output
-    bool                    debug                = false;  // Enable debug output for PEG parser
-    common_peg_arena        parser               = {};
+    common_chat_format format = COMMON_CHAT_FORMAT_CONTENT_ONLY;
+    common_chat_input  generation_prompt;
+    bool               debug  = false; // Enable debug output for PEG parser
+    common_peg_arena   parser = {};
     common_chat_parser_params() = default;
     common_chat_parser_params(const common_chat_params & chat_params) {
         format  = chat_params.format;
diff --git a/common/parsers/gpt-oss.cpp b/common/parsers/gpt-oss.cpp
index 61bcdb8557..b91c61d2ae 100644
--- a/common/parsers/gpt-oss.cpp
+++ b/common/parsers/gpt-oss.cpp
@@ -45,8 +45,7 @@ common_chat_params common_chat_params_init_gpt_oss(const common_chat_template &
     data.thinking_start_tag = "<|channel|>analysis<|message|>";
     data.thinking_end_tags  = {"<|end|>"};

-    // These special tokens are required to parse properly, so we include them
-    // even if parse_tool_calls is false.
+    // These special tokens are required to parse properly
     data.preserved_tokens = {
         "<|channel|>", "<|constrain|>", "<|message|>", "<|start|>", "<|end|>",
     };
diff --git a/common/parsers/llm-jp-harmony.cpp b/common/parsers/llm-jp-harmony.cpp
index b7d142b4b3..aebceb1260 100644
--- a/common/parsers/llm-jp-harmony.cpp
+++ b/common/parsers/llm-jp-harmony.cpp
@@ -48,8 +48,7 @@ common_chat_params common_chat_params_init_llm_jp_harmony(const common_chat_temp
     data.thinking_start_tag = "<|channel|>analysis<|message|>";
     data.thinking_end_tags  = {"<|end|>"};

-    // These special tokens are required to parse properly, so we include them
-    // even if parse_tool_calls is false.
+    // These special tokens are required to parse properly
     data.preserved_tokens = {
         "<|channel|>", "<|constrain|>", "<|message|>", "<|start|>", "<|end|>",
     };

@aldehir

aldehir commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

@ServeurpersoCom good catch, thank you.

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

The common/sampling changes are OK.

@aldehir
aldehir merged commit 8b54361 into ggml-org:master Oct 9, 2026
20 of 26 checks passed
feal87 added a commit to feal87/myllama.cpp that referenced this pull request Oct 9, 2026
Upstream brings the chat API refactor into common_chat_session (PR ggml-org#30210),
the UI models manager (ggml-org#29583), server default port 9931 (ggml-org#30159), and CUDA /
metal / sycl / vulkan / musa fixes.

Conflicts and resolution:
- server-context.cpp: upstream moved message spans into task.apply_chat_session()
  and dropped the per-request message_delimiters JSON round-trip. Keep the fork
  deferred-mtmd loop bound (n_inputs) and pass chat_session.message_delimiters()
  to task.mtmd_delims.
- server-queue.cpp: keep the fork reported_tool_names / base_tool_names /
  markdown_fences bookkeeping and adapt it to upstream's task_result_state()
  free constructor.

Kept the fork MoE expert cache (src/llama-moecache.*); scripts/fork-post-merge.sh
reports no upstream MoE cache leftovers.

Assisted-by: pi (deepseek-flash)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation server testing Everything test related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants