common/parser: fix nasty bug causing subtle corruption of generation prompt (#20825)

This commit is contained in:
Piotr Wilkin (ilintar) 2026-03-21 00:19:04 +01:00 committed by GitHub
parent e6ec21e62f
commit b1c70e2e54
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 52 additions and 1 deletions

View File

@ -188,6 +188,21 @@ diff_split calculate_diff_split(const std::string & left, const std::string & ri
result.suffix = "";
// pick prefix = all as representation
}
// When left has no unique content (result.left is empty), left is entirely
// shared with right. The simultaneous prefix/suffix segment matching can
// incorrectly consume trailing segments of left as suffix when those same
// segments also appear at the end of right (e.g. "\n" at the end of both
// the shared content and the generation prompt). This rotates the diff.
// Fix: if left is a prefix of right, enforce that directly.
if (result.left.empty() && !result.right.empty() &&
left.size() <= right.size() &&
right.substr(0, left.size()) == left) {
result.prefix = left;
result.suffix = "";
result.right = right.substr(left.size());
}
return result;
}

View File

@ -22,6 +22,7 @@ static void test_calculate_diff_split_no_common(testing & t);
static void test_calculate_diff_split_single_char(testing & t);
static void test_calculate_diff_split_overlaps(testing & t);
static void test_calculate_diff_split_tag_boundaries(testing & t);
static void test_calculate_diff_split_generation_prompt(testing & t);
static void test_calculate_diff_split(testing & t);
static void test_until_common_prefix_basic(testing & t);
@ -179,6 +180,7 @@ static void test_calculate_diff_split(testing & t) {
t.test("calculate_diff_split single char", test_calculate_diff_split_single_char);
t.test("calculate_diff_split overlaps", test_calculate_diff_split_overlaps);
t.test("calculate_diff_split tag boundaries", test_calculate_diff_split_tag_boundaries);
t.test("calculate_diff_split generation prompt", test_calculate_diff_split_generation_prompt);
}
static void test_calculate_diff_split_basic(testing & t) {
@ -502,6 +504,39 @@ static void test_calculate_diff_split_tag_boundaries(testing & t) {
}
}
static void test_calculate_diff_split_generation_prompt(testing & t) {
// ChatML thinking template: left is a prefix of right, generation_prompt is the appended part.
// The trailing \n in left matches the trailing \n in the generation_prompt, causing
// the suffix matcher to steal it and rotate the diff result.
{
// Simplified reproduction: left ends with \n, right = left + "<|im_start|>assistant\n<think>\n"
std::string left = "<|im_start|>user\nHello<|im_end|>\n";
std::string right = left + "<|im_start|>assistant\n<think>\n";
diff_split result = calculate_diff_split(left, right);
t.assert_equal("chatml prefix", left, result.prefix);
t.assert_equal("chatml left", "", result.left);
t.assert_equal("chatml right should be generation prompt",
"<|im_start|>assistant\n<think>\n", result.right);
t.assert_equal("chatml suffix", "", result.suffix);
}
{
// More realistic: longer conversation ending with tool_response
std::string common =
"<|im_start|>system\nYou are a helpful assistant.<|im_end|>\n"
"<|im_start|>user\nSearch for files<|im_end|>\n"
"<|im_start|>assistant\n<think>\nLet me search.\n</think>\n\n"
"<tool_call>\n<function=search>\n</function>\n</tool_call><|im_end|>\n"
"<|im_start|>user\n<tool_response>\nNo files found\n</tool_response><|im_end|>\n";
std::string left = common;
std::string right = common + "<|im_start|>assistant\n<think>\n";
diff_split result = calculate_diff_split(left, right);
t.assert_equal("tool_response left", "", result.left);
t.assert_equal("tool_response right should be generation prompt",
"<|im_start|>assistant\n<think>\n", result.right);
}
}
static void test_until_common_prefix(testing & t) {
t.test("until_common_prefix basic", test_until_common_prefix_basic);
}

View File

@ -1337,7 +1337,7 @@ static void test_template_output_peg_parsers(bool detailed_debug) {
tst.test("I'm\nthinking\n</think>\nHello, world!\nWhat's up?")
.enable_thinking(true)
.reasoning_format(COMMON_REASONING_FORMAT_NONE)
.expect_content("<think>I'm\nthinking\n</think>\nHello, world!\nWhat's up?")
.expect_content("<think>\nI'm\nthinking\n</think>\nHello, world!\nWhat's up?")
.run();
tst.test("I'm\nthinking\n</think>\nHello, world!\nWhat's up?")

View File

@ -415,6 +415,7 @@ task_params server_task::params_from_json_cmpl(
params.chat_parser_params.reasoning_in_content = params.stream && (reasoning_format == COMMON_REASONING_FORMAT_DEEPSEEK_LEGACY);
params.chat_parser_params.generation_prompt = json_value(data, "generation_prompt", std::string());
params.sampling.generation_prompt = params.chat_parser_params.generation_prompt;
SRV_DBG("Generation prompt: '%s'\n", params.chat_parser_params.generation_prompt.c_str());
params.chat_parser_params.parse_tool_calls = json_value(data, "parse_tool_calls", false);
if (data.contains("chat_parser")) {
params.chat_parser_params.parser.load(data.at("chat_parser").get<std::string>());