From 047d9d4d7076d86ed55be949eaefa43955bb0a73 Mon Sep 17 00:00:00 2001 From: danielhanchen Date: Mon, 20 Jul 2026 12:58:21 +0000 Subject: [PATCH] Studio: strip orphaned tool-call markup and byte-fallback chars at the tool-call chokepoints On an MTP GGUF model, speculative decoding on a quantized target surfaces byte-fallback garbage as U+FFFD plus an orphaned tool-call close tag whose opener was drained or mangled, which leaked into chat (issue #7084). Centralize the scrub at the tool-call chokepoints instead of scattering it per delta: - strip_tool_markup now sanitizes control chars / U+FFFD and removes a trailing orphan-close run, gated by a style sentinel so code/XML literals survive. Kimi and DeepSeek end-of-turn closers join the sentinel set. - The two streaming-strip entries (safetensors strip_tool_markup_streaming and the GGUF _strip_tool_markup_streaming) sanitize at the top and drop trailing orphan closes on the final segment, so live display matches the finalized answer. - ToolLoopController.record_result scrubs a tool result before it reaches the model or the tool card. Drops the scattered per-delta content, tool-args, and reasoning-channel scrubs and the secondary live-stream scrub, which are now covered by the chokepoints above. --- studio/backend/core/inference/llama_cpp.py | 8 + .../core/inference/safetensors_agentic.py | 8 + .../core/inference/tool_call_parser.py | 61 +++++ .../core/inference/tool_loop_controller.py | 9 +- .../tests/test_mtp_tool_markup_leak.py | 210 ++++++++++++++++++ 5 files changed, 295 insertions(+), 1 deletion(-) create mode 100644 studio/backend/tests/test_mtp_tool_markup_leak.py diff --git a/studio/backend/core/inference/llama_cpp.py b/studio/backend/core/inference/llama_cpp.py index c9ab7eb83b..582e42165d 100644 --- a/studio/backend/core/inference/llama_cpp.py +++ b/studio/backend/core/inference/llama_cpp.py @@ -52,10 +52,12 @@ from core.inference.tool_call_parser import ( _strip_gemma_wrapperless_calls, _strip_glm_calls, _strip_mistral_closed_calls, + _strip_trailing_orphan_close_run, TOOL_XML_SIGNALS as _SHARED_TOOL_XML_SIGNALS, RAG_MAX_SEARCHES_PER_TURN, RAG_SEARCH_CAP_NUDGE, parse_tool_calls_from_text as _shared_parse_tool_calls_from_text, + sanitize_control_chars as _sanitize_control_chars, strip_leading_bare_json_call, strip_llama3_leading_sentinels, strip_tool_markup as _shared_strip_tool_markup, @@ -9428,6 +9430,9 @@ class LlamaCppBackend: ) def _strip_tool_markup_streaming(text: str, *, force: bool = False) -> str: + # Scrub U+FFFD / control chars first, so a mangled opener cannot leave its close + # unmatched and byte-fallback garbage never leaks; mirrors strip_tool_markup. + text = _sanitize_control_chars(text) if not (auto_heal_tool_calls or force): return text @@ -9448,6 +9453,9 @@ class LlamaCppBackend: for pat in pats: seg = pat.sub("", seg) if is_last: + # Trailing orphan closes (drained/U+FFFD-mangled opener); orphan-strip before + # rehearsal-tail to match strip_tool_markup(final=True). + seg = _strip_trailing_orphan_close_run(seg) seg = apply_tool_strip_patterns( seg, [_REHEARSAL_TAIL_STRIP_RE], enabled_tool_names = _enabled_names_gate ) diff --git a/studio/backend/core/inference/safetensors_agentic.py b/studio/backend/core/inference/safetensors_agentic.py index 40731de57b..a0ad99db46 100644 --- a/studio/backend/core/inference/safetensors_agentic.py +++ b/studio/backend/core/inference/safetensors_agentic.py @@ -33,6 +33,7 @@ from core.inference.tool_call_parser import ( _strip_glm_calls, _strip_mistral_closed_calls, _strip_mistral_reasoning, + _strip_trailing_orphan_close_run, BUDGET_EXHAUSTED_NUDGE, MAX_ACT_REPROMPTS, RAG_MAX_SEARCHES_PER_TURN, @@ -41,6 +42,7 @@ from core.inference.tool_call_parser import ( is_short_intent_without_action, parse_tool_calls_from_text, reprompt_to_act_message, + sanitize_control_chars, strip_leading_bare_json_call, strip_llama3_leading_sentinels, strip_tool_markup, @@ -255,6 +257,9 @@ def strip_tool_markup_streaming( not be deleted, else the cumulative text shrinks then regrows). ``enabled_tool_names`` keeps an inactive-name ``foo[ARGS]{..}`` / ``call:NAME{..}`` example visible (it is prose, not a call), matching the parse / detection active-tool gate.""" + # Scrub U+FFFD / control chars first, so a mangled opener cannot leave its close unmatched + # and byte-fallback garbage never leaks into streamed display; mirrors strip_tool_markup. + text = sanitize_control_chars(text) if not (auto_heal_tool_calls or tool_protocol_active): return text @@ -278,6 +283,9 @@ def strip_tool_markup_streaming( for pat in pats: seg = pat.sub("", seg) if is_last: + # Trailing orphan closes (drained/U+FFFD-mangled opener); orphan-strip before + # rehearsal-tail to match strip_tool_markup(final=True). + seg = _strip_trailing_orphan_close_run(seg) seg = apply_tool_strip_patterns( seg, [_REHEARSAL_TAIL_STRIP_RE], enabled_tool_names = enabled_tool_names ) diff --git a/studio/backend/core/inference/tool_call_parser.py b/studio/backend/core/inference/tool_call_parser.py index 9b6b0a7773..c2e7117b79 100644 --- a/studio/backend/core/inference/tool_call_parser.py +++ b/studio/backend/core/inference/tool_call_parser.py @@ -33,6 +33,19 @@ from typing import Any, Optional from core import tool_healing as _tool_healing +# C0 (keep tab/newline/return/ESC), C1, and U+FFFD: never valid in chat/tool output. MTP/ +# speculative GGUF byte-fallback surfaces as U+FFFD (ggml-org/llama.cpp#25618). Same class as +# tools._BINARY_CHAR_RE. +_DISPLAY_CONTROL_CHAR_RE = re.compile("[\x00-\x08\x0b\x0c\x0e-\x1a\x1c-\x1f\x7f-\x9f�]") + + +def sanitize_control_chars(text: str) -> str: + """Drop control chars and U+FFFD from ``text``, keeping ``\\t`` ``\\n`` ``\\r`` and ESC.""" + if not text: + return text + return _DISPLAY_CONTROL_CHAR_RE.sub("", text) + + # Flip the streaming buffer STREAMING->DRAINING so partial markup never leaks. TOOL_XML_SIGNALS = ( "", @@ -118,8 +131,35 @@ _TOOL_ALL_PATS = _TOOL_CLOSED_PATS + [ re.DOTALL, ), # Gemma wrapper-less ``call:NAME{...}`` is handled by ``_strip_gemma_wrapperless_calls`` (enabled-name gate). + # Trailing orphan-close runs are handled by _strip_trailing_orphan_close_run. ] +_ORPHAN_CLOSE_TOKENS = ("", "", "", "", "") +_ORPHAN_SENTINELS = ("", "") + + +def _strip_trailing_orphan_close_run(text: str) -> str: + """Strip a trailing whitespace-separated run of tool close tags, but only when it + carries a or sentinel (a drained/U+FFFD-mangled opener leaves + the close to leak); a lone or is kept as likely code/XML. + Linear scan, so no regex backtracking.""" + i = len(text) + while i > 0 and text[i - 1].isspace(): + i -= 1 + run_start = i + has_sentinel = False + while True: + matched = next((t for t in _ORPHAN_CLOSE_TOKENS if text.endswith(t, 0, i)), None) + if matched is None: + break + if matched in _ORPHAN_SENTINELS: + has_sentinel = True + i -= len(matched) + run_start = i + while i > 0 and text[i - 1].isspace(): + i -= 1 + return text[:run_start] if has_sentinel else text + TOOL_ERROR_PREFIXES = ( "Error", @@ -282,6 +322,23 @@ _KIMI_ARG_BEGIN = "<|tool_call_argument_begin|>" _KIMI_CALL_END = "<|tool_call_end|>" _KIMI_ID_RE = re.compile(r"^(?:functions\.)?([\w\.\-]+)(?::(\d+))?$") +# Kimi and DeepSeek end-of-turn closers are back-to-back special tokens; a drained/U+FFFD- +# mangled opener leaves them to leak as a trailing orphan. Never legit prose, so they join the +# ````-style sentinel set. Extended here (not the tuple defs above) since these +# consts are defined later. +_ORPHAN_CLOSE_TOKENS = _ORPHAN_CLOSE_TOKENS + ( + _KIMI_CALL_END, + _KIMI_SECTION_END, + _DEEPSEEK_CALL_END, + _DEEPSEEK_END, +) +_ORPHAN_SENTINELS = _ORPHAN_SENTINELS + ( + _KIMI_CALL_END, + _KIMI_SECTION_END, + _DEEPSEEK_CALL_END, + _DEEPSEEK_END, +) + # Gemma 4: ``<|tool_call>call:NAME{...}``, ``<|"|>`` wraps strings. _GEMMA_TC_RE = re.compile(r"<\|tool_call>\s*call\s*:\s*([\w\.\-]+)\s*\{") _GEMMA_STR_BEGIN = '<|"|>' @@ -618,6 +675,8 @@ def strip_tool_markup( prose is kept (mirrors the parser gate): the bare reasoning-rehearsal ``name[ARGS]{...}`` and the markerless Gemma ``call:NAME{...}`` strip. ``None`` strips every closed call. """ + # Scrub U+FFFD / control chars first, so a mangled opener cannot leave its close unmatched. + text = sanitize_control_chars(text) if final: # Drop a leading Magistral ``[THINK]...[/THINK]`` at end-of-turn; its bracket # form is not the ```` the reasoning channel renders. @@ -643,6 +702,8 @@ def strip_tool_markup( for pat in pats: seg = pat.sub("", seg) if seg_final: + # Trailing orphan closes whose opener was drained or U+FFFD-mangled upstream. + seg = _strip_trailing_orphan_close_run(seg) # Drop a trailing partial bare rehearsal (``name[ARGS]`` with a truncated or absent # body) the balanced scan cannot close; gated so prose ``foo[ARGS] ...`` survives. seg = _tool_healing.apply_tool_strip_patterns( diff --git a/studio/backend/core/inference/tool_loop_controller.py b/studio/backend/core/inference/tool_loop_controller.py index 61643b5795..44232497fd 100644 --- a/studio/backend/core/inference/tool_loop_controller.py +++ b/studio/backend/core/inference/tool_loop_controller.py @@ -17,7 +17,11 @@ from dataclasses import dataclass, field from typing import Any, Literal, Mapping, Sequence from urllib.parse import urlparse -from core.inference.tool_call_parser import TOOL_ERROR_NUDGE, TOOL_ERROR_PREFIXES +from core.inference.tool_call_parser import ( + TOOL_ERROR_NUDGE, + TOOL_ERROR_PREFIXES, + sanitize_control_chars, +) _CANONICAL_HEAL_ARG = { @@ -403,6 +407,9 @@ class ToolLoopController: def record_result(self, decision: ToolCallDecision, result: Any) -> ToolCallCompletion: """Record a real tool execution and return model/frontend payload helpers.""" result_text = result if isinstance(result, str) else str(result) + # Scrub garbage a fetch/subprocess can leave, so it neither shows on the tool card + # nor poisons the model's next prompt. + result_text = sanitize_control_chars(result_text) failed = is_tool_error(result_text) self._history.append( _ToolCallRecord( diff --git a/studio/backend/tests/test_mtp_tool_markup_leak.py b/studio/backend/tests/test_mtp_tool_markup_leak.py new file mode 100644 index 0000000000..281583ea5a --- /dev/null +++ b/studio/backend/tests/test_mtp_tool_markup_leak.py @@ -0,0 +1,210 @@ +# SPDX-License-Identifier: AGPL-3.0-only +# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0 + +"""Regression tests for the MTP GGUF tool-call garbage bug (issue #7084). + +On an MTP GGUF model, speculative decoding on a quantized target +(ggml-org/llama.cpp#25618) emits byte-fallback garbage that llama-server forwards as +U+FFFD plus an orphaned ```` whose opener was drained/``�``-mangled; +``strip_tool_markup`` had no arm for a bare orphan close, so the reporter saw +``8��� `` in chat. + +The scrub is centralized at the tool-call chokepoints, so these tests pin only those: + + 1. ``strip_tool_markup`` (finalized answer): scrubs U+FFFD / control chars and removes a + trailing orphan-close run, while keeping well-formed stripping and mid-prose literals. + 2. ``strip_tool_markup_streaming`` (streaming display): the same scrub at the streaming + entry, so the live display agrees with the finalized answer. + 3. ``sanitize_control_chars``: drops garbage but keeps ``\t \n \r`` / ESC. + 4. ``ToolLoopController.record_result``: scrubs a tool result before the model/card. +""" + +import pytest + +from core.inference.safetensors_agentic import strip_tool_markup_streaming +from core.inference.tool_call_parser import sanitize_control_chars, strip_tool_markup +from core.inference.tool_loop_controller import ToolCallDecision, ToolLoopController + + +# -- sanitize_control_chars ------------------------------------------ + + +def test_sanitize_drops_replacement_and_control_chars(): + assert sanitize_control_chars("8��� ok") == "8 ok" + assert sanitize_control_chars("a\x00b\x7fc\x9fd") == "abcd" + + +def test_sanitize_keeps_tab_newline_cr_and_esc(): + # ESC (\x1b) is preserved so terminal ANSI in a tool result survives. + assert sanitize_control_chars("a\tb\nc\r\n\x1b[0m") == "a\tb\nc\r\n\x1b[0m" + + +def test_sanitize_noop_on_clean_text(): + s = "Perfectly normal answer with a supplementary-plane char \U00020000 and kanji 美味しい." + assert sanitize_control_chars(s) == s + + +# -- strip_tool_markup (finalized answer): the reporter's exact garbage -- + + +def test_reporter_garbage_is_cleaned(): + # Exact string from issue #7084; before the fix both the U+FFFD and the orphan + # leaked. + out = strip_tool_markup("8��� ", final = True) + assert "�" not in out + assert "" not in out + # is model-hallucinated prose, not a Studio token, so it is left as-is. + assert out == "8 " + + +@pytest.mark.parametrize( + "text,expected", + [ + ("Here is the answer.", "Here is the answer."), + ("x", "x"), + ("answer\n", "answer"), # nested leak run ending in + ("Here is the answer.\n", "Here is the answer."), # trailing newline + ("8��� \n", "8"), # reporter's garbage + trailing newline + ], +) +def test_trailing_orphan_closes_are_stripped_at_final(text, expected): + assert strip_tool_markup(text, final = True) == expected + + +@pytest.mark.parametrize( + "text", + [ + "Done.", # lone close, no sentinel + "value", + "The XML closing tag is ", # a code/XML answer ending on a literal + "In XML you write x inside a tag.", # mid-prose literal + ], +) +def test_trailing_literal_close_without_tool_call_survives(text): + # A trailing / with no sentinel reads as a + # code/XML literal, not a leak, so it survives (a real leak carries ). + assert strip_tool_markup(text, final = True) == text + + +def test_wellformed_call_still_stripped(): + text = ( + "Prefix \n\n\nx\n" + "\n\n suffix" + ) + out = strip_tool_markup(text, final = True) + assert "" not in out and "" not in out + assert out == "Prefix suffix" + + +def test_streaming_pass_final_false_buffers_orphan_but_scrubs(): + # final=False keeps in-progress markup buffered (orphan-run arm is end-of-turn only) + # but still scrubs U+FFFD. + assert strip_tool_markup("Here is the answer.", final = False) == ( + "Here is the answer." + ) + assert "�" not in strip_tool_markup("hi � there", final = False) + + +# -- strip_tool_markup_streaming (streaming display) chokepoint ------- + + +def test_streaming_strips_trailing_orphan_close(): + # The MTP byte-fallback U+FFFD is scrubbed at the streaming entry, so the display strip + # sees a bare orphan close and removes it, matching strip_tool_markup(final=True). + assert strip_tool_markup_streaming("answer�") == "answer" + # A genuine complete call is still fully stripped (no under-strip regression). + assert strip_tool_markup_streaming('{"name":"x","arguments":{}}') == "" + # A lone without a sentinel is kept (likely code/XML), not over-stripped. + assert strip_tool_markup_streaming("see here") == "see here" + + +def test_streaming_entry_scrubs_control_chars_even_when_disabled(): + # The entry scrub runs before the auto-heal gate, so byte-fallback garbage is dropped + # from streamed display even with stripping disabled. + out = strip_tool_markup_streaming( + "hi �\x00 there", auto_heal_tool_calls = False, tool_protocol_active = False + ) + assert "�" not in out and "\x00" not in out + + +def test_gguf_streaming_closure_wires_scrub_and_orphan_strip(): + # The GGUF streaming stripper is a nested closure, so pin the fix by source: its entry must + # scrub control chars and its final-segment block must drop trailing orphan closes, matching + # the safetensors path and strip_tool_markup(final=True). + import inspect + + from core.inference.llama_cpp import LlamaCppBackend + + src = inspect.getsource(LlamaCppBackend.generate_chat_completion_with_tools) + assert "_sanitize_control_chars(text)" in src + assert "_strip_trailing_orphan_close_run(seg)" in src + + +# -- record_result scrubs the tool result on both boundaries --------- + + +def _decision(name = "web_search"): + return ToolCallDecision( + action = "execute", + tool_name = name, + arguments = {"query": "x"}, + tool_call_id = "call_0", + key = f"{name}:{{}}", + ) + + +def test_record_result_scrubs_tool_result_for_model_and_display(): + ctrl = ToolLoopController(tools = [{"function": {"name": "web_search"}}]) + dirty = "Title: Page�� body\x00 text�" + completion = ctrl.record_result(_decision(), dirty) + # Fed back to the model: + assert "�" not in completion.model_message()["content"] + assert "\x00" not in completion.model_message()["content"] + # Shown in the tool card: + assert "�" not in completion.tool_end_payload()["result"] + assert completion.result == "Title: Page body text" + + +def test_record_result_keeps_clean_result_intact(): + ctrl = ToolLoopController(tools = [{"function": {"name": "web_search"}}]) + clean = "Title: Florida ACA 2026\nSilver premium: $1,900/mo" + completion = ctrl.record_result(_decision(), clean) + assert completion.result == clean + + +# -- Kimi + DeepSeek end-of-turn closers belong to the orphan-close set -- +# +# ``_ORPHAN_CLOSE_TOKENS`` / ``_ORPHAN_SENTINELS`` also list the Kimi +# ``<|tool_call_end|><|tool_calls_section_end|>`` and DeepSeek +# ```` closers: back-to-back special tokens, never legit +# prose, so a run whose opener was drained/U+FFFD-mangled is scrubbed like ````. + + +def _kimi_deepseek_tokens(): + from core.inference.tool_call_parser import ( + _DEEPSEEK_CALL_END, + _DEEPSEEK_END, + _KIMI_CALL_END, + _KIMI_SECTION_END, + ) + return _KIMI_CALL_END, _KIMI_SECTION_END, _DEEPSEEK_CALL_END, _DEEPSEEK_END + + +def test_kimi_and_deepseek_trailing_closers_stripped_at_final(): + kimi_end, kimi_section_end, ds_call_end, ds_end = _kimi_deepseek_tokens() + assert strip_tool_markup("answer " + kimi_end + kimi_section_end, final = True) == "answer" + assert strip_tool_markup("answer " + ds_call_end + ds_end, final = True) == "answer" + + +def test_kimi_and_deepseek_closers_stripped_in_streaming(): + kimi_end, kimi_section_end, ds_call_end, ds_end = _kimi_deepseek_tokens() + assert strip_tool_markup_streaming("answer" + kimi_end + kimi_section_end) == "answer" + assert strip_tool_markup_streaming("answer" + ds_call_end + ds_end) == "answer" + + +def test_kimi_closer_in_mid_prose_survives(): + # Only TRAILING orphans are stripped: a token embedded in prose (with real text after) is + # not a trailing run, so a plain answer is never over-stripped. + kimi_end, *_ = _kimi_deepseek_tokens() + text = "the token " + kimi_end + " appears mid sentence" + assert strip_tool_markup(text, final = True) == text