Skip to content

chat: replace output string with parts - #30389

Merged
ngxson merged 3 commits into
masterfrom
xsn/chat_input_parts
Oct 13, 2026
Merged

ngxson merged 3 commits into
masterfrom
xsn/chat_input_parts

Conversation

@ngxson

@ngxson ngxson commented Oct 12, 2026 •

Copy link
Copy Markdown
Collaborator

Overview

Second item of #28249

Formatted chat will now be an array of parts, not just a simple string. This allows selectively parsing special tokens for only certain parts (will be a follow-up PR)

Requirements

@ngxson
ngxson requested review from a team, CISC, am17an, ggerganov and pwilkin as code owners October 12, 2026 12:54
@github-actions github-actions Bot added documentation Improvements or additions to documentation testing Everything test related server jinja parser Issues related to the jinja parser labels Oct 12, 2026
Comment thread common/chat.cpp
Comment on lines +1507 to +1510
std::string out;
for (const auto & part : parts) {
out += part.text;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we do a reserve here first? To something reasonable, since I assume you can't find the length without traversing the parts anyway.

Comment on lines 70 to +77
const auto & msg = inputs.continue_msg;

data.generation_prompt = GEN_PREFIX + THINK_START + msg.reasoning_content;
if (inputs.continue_final_message == COMMON_CHAT_CONTINUATION_CONTENT) {
data.generation_prompt += THINK_END + TEXT_START + msg.render_content();
}

data.prompt += data.generation_prompt;
data.prompt.append(data.generation_prompt);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In a follow up, maybe we can add another append() or a + operator to concat common_chat_prompt values. I can update the parsers to provide a common_chat_prompt value with the proper input marking for assistant prefilled messages.

Comment on lines 24 to 28
if (inputs.is_inference && !inputs.add_generation_prompt) {
static constexpr std::string_view return_token = "<|return|>";
static constexpr std::string_view end_token = "<|end|>";
if (size_t pos = prompt.rfind(return_token); pos != std::string::npos) {
prompt.replace(pos, return_token.length(), end_token);
}
prompt.replace_last(std::string(return_token), std::string(end_token));
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If we derive the generation prompt during rendering of the template, this entire block goes away. Actually, since the refactoring of llama-cli, I'm not sure it's even needed anymore. I'll revisit this later.

@aldehir

aldehir commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

A heads up for the actual tokenization: Qwen's thinking tags are not marked as special but user defined. There is no way to avoid parsing user defined tokens like there is for special tokens, but that's what is needed to tokenize those thinking tags into multiple pieces for user input.

@pwilkin

pwilkin commented Oct 12, 2026

Copy link
Copy Markdown
Member

@aldehir sounds like we need some extra predicate to mark some tokens as "special" for the purposes of the parser - might be something to add to the parser output (already have "reserved tokens", which basically serve this purpose).

@ngxson
ngxson merged commit ff026f8 into master Oct 13, 2026
15 checks passed
xarillian added a commit to xarillian/chorus-llm that referenced this pull request Oct 23, 2026
Upstream ggml-org/llama.cpp#30389 turned common_chat_params::prompt into template and input parts. Chorus tokenizes the rendered prompt as one string, as upstream's own callers do, so it flattens the parts. Moves llama.cpp to 184f65360 and refreshes the resource-cleanup patch (offsets only).
auRose94 pushed a commit to auRose94/llama.cpp that referenced this pull request Oct 28, 2026
* chat: replace output string with parts

* reserve first
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 jinja parser Issues related to the jinja parser server testing Everything test related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants