Ignore tool calls emitted inside thinking

Refs https://github.com/antirez/ds4/issues/167.

DSML tool calls are executable only after the assistant closes the thinking stanza. The live decoder now skips tool-marker detection while thinking is open, and the final parser searches for executable DSML only after the last </think> when thinking mode is active. This prevents a DSML block inside reasoning from being duplicated as both thinking text and a structured tool call.

We were not able to prove from traces that the model actually made this exact mistake in the reported case, because the trace gives text rather than the original sampled-token/KV state. It is nevertheless possible, and it matches the reported shape of the bug. We did see evidence that the model can discuss tool usage and DSML syntax inside the thinking stanza, so this is a conservative protocol fix: it does not force the end of thinking, and it does not execute tool calls while still inside thinking.

It remains unclear what the model would do if it truly mis-emits a tool call before </think>: it might recover by closing thinking later, or it might not. A follow-up commit should address the likely root source by changing sampler behavior itself instead of relying only on protocol filtering.
This commit is contained in:
antirez committed 2026-05-16 15:31:56 +02:00
1 parent 011aa677a7
commit 037ee39aa6
2 files changed
+158 -52

No files matched your search

+156 -50
View File
@@ -4246,6 +4246,17 @@ static const char *skip_ascii_ws(const char *p) {
return p;
}
static const char *find_last_substr(const char *s, const char *needle) {
if (!s || !needle || !needle[0]) return NULL;
const char *last = NULL;
const char *p = s;
while ((p = strstr(p, needle)) != NULL) {
last = p;
p++;
}
return last;
}
/* The prompt renderer escapes DSML text so a tool argument can safely contain
* shell operators or closing tags. The generated-DSML parser must undo exactly
* those entities before it turns parameters back into JSON; otherwise
@@ -4401,29 +4412,49 @@ static void split_reasoning_content(const char *text, size_t n, char **content_o
free(s);
}
static bool parse_generated_message(const char *text, char **content_out,
char **reasoning_out, tool_calls *calls) {
const char *start = strstr(text, "\n\n" DS4_TOOL_CALLS_START);
static bool parse_generated_message_ex(const char *text, bool require_thinking_closed,
char **content_out, char **reasoning_out,
tool_calls *calls) {
text = text ? text : "";
const char *tool_search = text;
/* When thinking mode is enabled the model is expected to close
* </think> before it enters the executable assistant surface. DSML inside
* reasoning is just model text: it may be a mistaken attempt, a quotation,
* or an explanation of the protocol. Treating it as a real tool call
* duplicates it into both reasoning and structured tool_calls, and can make
* clients execute something the assistant had not actually emitted as its
* post-thinking action. */
if (require_thinking_closed) {
const char *think_end = find_last_substr(text, "</think>");
if (!think_end) {
split_reasoning_content(text, strlen(text), content_out, reasoning_out);
return true;
}
tool_search = think_end + 8;
}
const char *start = strstr(tool_search, "\n\n" DS4_TOOL_CALLS_START);
int style = 0; /* 0: DSML, 1: plain XML, 2: DSML with the first vertical bar omitted. */
if (!start) start = strstr(text, DS4_TOOL_CALLS_START);
if (!start) start = strstr(tool_search, DS4_TOOL_CALLS_START);
if (!start) {
start = strstr(text, "\n\n" DS4_TOOL_CALLS_START_SHORT);
start = strstr(tool_search, "\n\n" DS4_TOOL_CALLS_START_SHORT);
style = start ? 2 : style;
}
if (!start) {
start = strstr(text, DS4_TOOL_CALLS_START_SHORT);
start = strstr(tool_search, DS4_TOOL_CALLS_START_SHORT);
style = start ? 2 : style;
}
if (!start) {
start = strstr(text, "\n\n<tool_calls>");
start = strstr(tool_search, "\n\n<tool_calls>");
style = start ? 1 : style;
}
if (!start) {
start = strstr(text, "<tool_calls>");
start = strstr(tool_search, "<tool_calls>");
style = start ? 1 : style;
}
if (!start) {
split_reasoning_content(text, text ? strlen(text) : 0, content_out, reasoning_out);
split_reasoning_content(text, strlen(text), content_out, reasoning_out);
return true;
}
@@ -4571,6 +4602,7 @@ static const char *tool_parse_failure_recovery_finish(const char *finish) {
static bool parse_generated_message_for_response(const char *text,
bool has_tools,
bool saw_tool_start,
bool require_thinking_closed,
const char **finish_io,
char *err,
size_t errlen,
@@ -4580,8 +4612,10 @@ static bool parse_generated_message_for_response(const char *text,
bool *recovered_out) {
if (recovered_out) *recovered_out = false;
bool parsed_ok = parse_generated_message(text ? text : "", content_out,
reasoning_out, calls);
bool parsed_ok = parse_generated_message_ex(text ? text : "",
require_thinking_closed,
content_out, reasoning_out,
calls);
if (parsed_ok) return true;
free(*content_out);
@@ -10829,6 +10863,9 @@ static void generate_job(server *s, job *j) {
double last_decode_log_t = decode_t0;
int last_decode_log_completion = 0;
thinking_state thinking = thinking_state_from_prompt(&j->req);
const bool thinking_gates_tool_markers = ds4_think_mode_enabled(j->req.think_mode);
bool tool_scan_waiting_for_think_close =
thinking_gates_tool_markers && thinking.inside;
dsml_decode_tracker dsml_tracker;
dsml_decode_tracker_init(&dsml_tracker);
@@ -10966,36 +11003,52 @@ static void generate_job(server *s, job *j) {
free(piece);
if (j->req.kind == REQ_CHAT && j->req.has_tools) {
const char *tool_scan = text.ptr ? text.ptr + tool_scan_from : "";
bool orphan_end = false;
bool old_start = saw_tool_start;
bool old_end = saw_tool_end;
observe_tool_markers(tool_scan, &saw_tool_start, &saw_tool_end, &orphan_end);
if (orphan_end && !saw_orphan_tool_end) {
saw_orphan_tool_end = true;
server_log(DS4_LOG_WARNING,
"ds4-server: chat ctx=%s%s%s ignored orphan tool-call end marker after %d generated tokens",
ctx_span,
req_flags[0] ? " " : "",
req_flags,
completion);
trace_event(s, trace_id,
"ignored orphan tool-call end marker after %d generated tokens",
completion);
}
if (saw_tool_start && !old_start) {
trace_event(s, trace_id, "entered tool-call block after %d generated tokens", completion);
}
if (saw_tool_end && !old_end) {
trace_event(s, trace_id, "closed tool-call block after %d generated tokens", completion);
}
const size_t marker_hold = 80;
tool_scan_from = text.len > marker_hold ? text.len - marker_hold : 0;
if (s->trace && completion >= next_tool_progress) {
trace_event(s, trace_id,
"progress gen=%d dsml_start=%d dsml_end=%d",
completion, saw_tool_start ? 1 : 0, saw_tool_end ? 1 : 0);
next_tool_progress += 128;
if (thinking_gates_tool_markers && thinking.inside) {
/* A DSML block inside reasoning is not executable. This is
* the live guard: do not let a quoted or mistaken marker in
* <think> stop decoding as a real tool call. */
tool_scan_waiting_for_think_close = true;
tool_scan_from = text.len;
} else {
if (tool_scan_waiting_for_think_close) {
const char *think_end = find_last_substr(text.ptr, "</think>");
tool_scan_from = think_end ? (size_t)((think_end + 8) - text.ptr) : text.len;
if (tool_scan_from > text.len) tool_scan_from = text.len;
tool_scan_waiting_for_think_close = false;
}
if (tool_scan_from > text.len) tool_scan_from = text.len;
const char *tool_scan = text.ptr ? text.ptr + tool_scan_from : "";
bool orphan_end = false;
bool old_start = saw_tool_start;
bool old_end = saw_tool_end;
observe_tool_markers(tool_scan, &saw_tool_start, &saw_tool_end, &orphan_end);
if (orphan_end && !saw_orphan_tool_end) {
saw_orphan_tool_end = true;
server_log(DS4_LOG_WARNING,
"ds4-server: chat ctx=%s%s%s ignored orphan tool-call end marker after %d generated tokens",
ctx_span,
req_flags[0] ? " " : "",
req_flags,
completion);
trace_event(s, trace_id,
"ignored orphan tool-call end marker after %d generated tokens",
completion);
}
if (saw_tool_start && !old_start) {
trace_event(s, trace_id, "entered tool-call block after %d generated tokens", completion);
}
if (saw_tool_end && !old_end) {
trace_event(s, trace_id, "closed tool-call block after %d generated tokens", completion);
}
const size_t marker_hold = 80;
size_t hold_from = text.len > marker_hold ? text.len - marker_hold : 0;
if (hold_from > tool_scan_from) tool_scan_from = hold_from;
if (s->trace && completion >= next_tool_progress) {
trace_event(s, trace_id,
"progress gen=%d dsml_start=%d dsml_end=%d",
completion, saw_tool_start ? 1 : 0, saw_tool_end ? 1 : 0);
next_tool_progress += 128;
}
}
}
@@ -11074,6 +11127,7 @@ static void generate_job(server *s, job *j) {
text.ptr ? text.ptr : "",
j->req.has_tools,
saw_tool_start,
ds4_think_mode_enabled(j->req.think_mode),
&final_finish,
err,
sizeof(err),
@@ -12569,8 +12623,8 @@ static void test_anthropic_tool_stream_sends_live_tool_use(void) {
char *parsed_content = NULL;
char *parsed_reasoning = NULL;
tool_calls calls = {0};
TEST_ASSERT(parse_generated_message(raw_complete, &parsed_content,
&parsed_reasoning, &calls));
TEST_ASSERT(parse_generated_message_ex(raw_complete, false, &parsed_content,
&parsed_reasoning, &calls));
TEST_ASSERT(calls.len == 1);
apply_anthropic_stream_tool_ids(&calls, &st);
TEST_ASSERT(calls.v[0].id != NULL);
@@ -12906,7 +12960,7 @@ static void test_openai_tool_stream_sends_partial_arguments(void) {
char *parsed_content = NULL;
char *parsed_reasoning = NULL;
tool_calls calls = {0};
TEST_ASSERT(parse_generated_message(raw_complete, &parsed_content, &parsed_reasoning, &calls));
TEST_ASSERT(parse_generated_message_ex(raw_complete, false, &parsed_content, &parsed_reasoning, &calls));
TEST_ASSERT(calls.len == 1);
apply_openai_stream_tool_ids(&calls, &st);
TEST_ASSERT(calls.v[0].id != NULL);
@@ -13452,7 +13506,7 @@ static void test_parse_short_dsml_and_canonical_suffix(void) {
char *content = NULL;
char *reasoning = NULL;
tool_calls calls = {0};
TEST_ASSERT(parse_generated_message(generated, &content, &reasoning, &calls));
TEST_ASSERT(parse_generated_message_ex(generated, false, &content, &reasoning, &calls));
TEST_ASSERT(reasoning && !strcmp(reasoning, "need a tool"));
TEST_ASSERT(content && content[0] == '\0');
TEST_ASSERT(calls.len == 1);
@@ -13492,7 +13546,7 @@ static void test_dsml_parser_recovers_loose_nested_parameters(void) {
char *content = NULL;
char *reasoning = NULL;
tool_calls calls = {0};
TEST_ASSERT(parse_generated_message(generated, &content, &reasoning, &calls));
TEST_ASSERT(parse_generated_message_ex(generated, false, &content, &reasoning, &calls));
TEST_ASSERT(content && !strcmp(content, "review done"));
TEST_ASSERT(calls.len == 1);
TEST_ASSERT(calls.v[0].name && !strcmp(calls.v[0].name, "edit"));
@@ -13523,6 +13577,7 @@ static void test_tool_parse_failure_returns_recoverable_finish(void) {
TEST_ASSERT(!parse_generated_message_for_response(generated,
true,
true,
false,
&finish,
err,
sizeof(err),
@@ -13542,6 +13597,55 @@ static void test_tool_parse_failure_returns_recoverable_finish(void) {
tool_calls_free(&calls);
}
static void test_thinking_dsml_is_not_executable_before_think_close(void) {
const char *generated =
"<think>I might mention a malformed or tentative tool call here:\n\n"
DS4_TOOL_CALLS_START "\n"
DS4_INVOKE_START " name=\"bash\">\n"
DS4_PARAM_START " name=\"command\" string=\"true\">true" DS4_PARAM_END "\n"
DS4_INVOKE_END "\n"
DS4_TOOL_CALLS_END
"\nBut it is still reasoning, not an assistant action.</think>Final answer.";
char *content = NULL;
char *reasoning = NULL;
tool_calls calls = {0};
TEST_ASSERT(parse_generated_message_ex(generated, true,
&content, &reasoning, &calls));
TEST_ASSERT(calls.len == 0);
TEST_ASSERT(reasoning && strstr(reasoning, DS4_TOOL_CALLS_START) != NULL);
TEST_ASSERT(content && !strcmp(content, "Final answer."));
free(content);
free(reasoning);
tool_calls_free(&calls);
}
static void test_thinking_dsml_after_think_close_is_executable(void) {
const char *generated =
"<think>need a shell check</think>\n\n"
DS4_TOOL_CALLS_START "\n"
DS4_INVOKE_START " name=\"bash\">\n"
DS4_PARAM_START " name=\"command\" string=\"true\">pwd" DS4_PARAM_END "\n"
DS4_INVOKE_END "\n"
DS4_TOOL_CALLS_END;
char *content = NULL;
char *reasoning = NULL;
tool_calls calls = {0};
TEST_ASSERT(parse_generated_message_ex(generated, true,
&content, &reasoning, &calls));
TEST_ASSERT(calls.len == 1);
TEST_ASSERT(reasoning && !strcmp(reasoning, "need a shell check"));
TEST_ASSERT(content && content[0] == '\0');
TEST_ASSERT(calls.v[0].name && !strcmp(calls.v[0].name, "bash"));
TEST_ASSERT(strstr(calls.v[0].arguments, "\"command\": \"pwd\"") != NULL);
free(content);
free(reasoning);
tool_calls_free(&calls);
}
static void test_tool_checkpoint_suffix_is_future_prompt_canonical(void) {
tool_schema_orders orders = make_bash_order();
const char *tool_schemas =
@@ -13567,7 +13671,7 @@ static void test_tool_checkpoint_suffix_is_future_prompt_canonical(void) {
char *content = NULL;
char *reasoning = NULL;
tool_calls calls = {0};
TEST_ASSERT(parse_generated_message(generated, &content, &reasoning, &calls));
TEST_ASSERT(parse_generated_message_ex(generated, false, &content, &reasoning, &calls));
TEST_ASSERT(calls.len == 1);
TEST_ASSERT(strstr(calls.v[0].arguments, "cd /tmp && git diff 2>/dev/null") != NULL);
TEST_ASSERT(strstr(calls.v[0].arguments, "&amp;&amp;") == NULL);
@@ -13646,7 +13750,7 @@ static void test_tool_checkpoint_minifies_json_parameters(void) {
char *content = NULL;
char *reasoning = NULL;
tool_calls calls = {0};
TEST_ASSERT(parse_generated_message(generated, &content, &reasoning, &calls));
TEST_ASSERT(parse_generated_message_ex(generated, false, &content, &reasoning, &calls));
TEST_ASSERT(calls.len == 1);
request r;
@@ -13703,7 +13807,7 @@ static void test_tool_memory_replays_sampled_dsml(void) {
char *content = NULL;
char *reasoning = NULL;
tool_calls sampled = {0};
TEST_ASSERT(parse_generated_message(generated, &content, &reasoning, &sampled));
TEST_ASSERT(parse_generated_message_ex(generated, false, &content, &reasoning, &sampled));
TEST_ASSERT(sampled.len == 1);
server s;
@@ -14313,7 +14417,7 @@ static void test_tool_separator_whitespace_is_not_content(void) {
char *content = NULL;
char *reasoning = NULL;
tool_calls calls = {0};
TEST_ASSERT(parse_generated_message(generated, &content, &reasoning, &calls));
TEST_ASSERT(parse_generated_message_ex(generated, false, &content, &reasoning, &calls));
TEST_ASSERT(reasoning && !strcmp(reasoning, "need a tool"));
TEST_ASSERT(content && !strcmp(content, "I will inspect the files."));
TEST_ASSERT(calls.len == 1);
@@ -15400,6 +15504,8 @@ static void ds4_server_unit_tests_run(void) {
test_parse_short_dsml_and_canonical_suffix();
test_dsml_parser_recovers_loose_nested_parameters();
test_tool_parse_failure_returns_recoverable_finish();
test_thinking_dsml_is_not_executable_before_think_close();
test_thinking_dsml_after_think_close_is_executable();
test_tool_checkpoint_suffix_is_future_prompt_canonical();
test_tool_checkpoint_minifies_json_parameters();
test_tool_memory_replays_sampled_dsml();
+2 -2
View File
@@ -606,8 +606,8 @@ static void test_tool_call_quality_one(bool quality) {
char *content = NULL;
char *reasoning = NULL;
tool_calls calls = {0};
bool parsed = parse_generated_message(text.ptr ? text.ptr : "",
&content, &reasoning, &calls);
bool parsed = parse_generated_message_ex(text.ptr ? text.ptr : "",
false, &content, &reasoning, &calls);
TEST_ASSERT(decode_ok);
TEST_ASSERT(parsed);
TEST_ASSERT(calls.len > 0);