From abc1e6568c56d48c12eed23eb9274708b073021a Mon Sep 17 00:00:00 2001 From: vikoabastian <11003051+vianbas@users.noreply.github.com> Date: Wed, 2 Sep 2026 15:35:35 +0700 Subject: [PATCH] fix(server): stop gemma4 tool-call markers leaking into content Gemma4Detector surfaced its own protocol markers as assistant content on every path where a tool-call block failed to parse. detect_and_parse slices strictly before the opener only when it finds the exact `<|tool_call>` byte sequence; when find() misses, the whole text became normal_text, so a partial or malformed opener reached message.content verbatim. finish_streaming had the same blind spot: its `bot_token in residual` guard cannot see a stream that ended mid-marker. And parse_non_stream carried the leak even when the detector got it right -- it discarded the detector's normal_text and re-surfaced full_text whenever no call parsed, and appended the tail after the last closer guarded only by has_tool_call(), which does not recognise a truncated opener. That last path is the shape the report describes: a stray marker in content alongside a correctly parsed tool_calls array. Every path that hands text to a client now runs it through a new BaseFormatDetector.scrub_markup() hook. The base implementation returns the text unchanged, so detectors that surface raw text keep doing so -- scrubbing unconditionally there would blank the response for qwen25, mistral, deepseekv32 and minimax when a block fails to parse. Gemma4Detector overrides it to cut at the `<|tool_call` prefix its opener and closer share. For gemma4 this makes one-shot parsing agree with streaming. Given `<|tool_call>call:get_weather{bad\nHere is the answer: 42.`, streaming already surfaced "" while non-streaming returned the raw block; both now return "". Fixes #203 --- .../freetoken/server/function_call_parser.py | 37 +++++++- tests/server/test_function_call_parser.py | 91 +++++++++++++++++++ 2 files changed, 125 insertions(+), 3 deletions(-) diff --git a/python/freetoken/server/function_call_parser.py b/python/freetoken/server/function_call_parser.py index 0804dcc3f..bed4c9a27 100644 --- a/python/freetoken/server/function_call_parser.py +++ b/python/freetoken/server/function_call_parser.py @@ -259,6 +259,17 @@ class BaseFormatDetector(ABC): # bot_token is a parse trigger, not a uniqueness claim. toolcall_opener: str | None = None + def scrub_markup(self, text: str) -> str: + """Remove this format's markup from text about to be surfaced as content. + + Called on the one-shot parse paths and the end-of-stream drain, so a marker the + parser could not turn into a call cannot ride along. NOT called per streaming + chunk: parse_streaming_increment still releases a partial opener mid-stream. + The default keeps the text untouched, so a detector that does not override this + surfaces raw text as it always has. + """ + return text + def __init__(self): # Streaming state management # Buffer for accumulating incomplete patterns that arrive across multiple streaming chunks @@ -2060,12 +2071,24 @@ def __init__(self): re.DOTALL, ) + # The opener's stable prefix. A block that never completed leaves only part of the + # opener behind, which the exact-match bot_token check cannot catch. The closer + # ("") does not share this prefix; finish_streaming replaces it + # separately, and the one-shot path does not scrub it at all. + _marker_prefix = "<|tool_call" + def has_tool_call(self, text: str) -> bool: return self.bot_token in text + def scrub_markup(self, text: str) -> str: + """Content ends where tool markup begins: an opener the parser could not turn + into a call is still protocol framing, not assistant-visible text.""" + idx = text.find(self._marker_prefix) + return text if idx == -1 else text[:idx] + def detect_and_parse(self, text: str, tools: List[Tool]) -> StreamingParseResult: idx = text.find(self.bot_token) - normal_text = text[:idx].strip() if idx != -1 else text + normal_text = text[:idx].strip() if idx != -1 else self.scrub_markup(text) if idx == -1: return StreamingParseResult(normal_text=normal_text, calls=[]) @@ -2331,6 +2354,9 @@ def finish_streaming(self) -> str: self._g4_reset() if mode != "idle" or self.bot_token in residual: return "" + # A stream that ends mid-marker leaves an opener prefix the bot_token check + # above cannot see; the closer never starts with it, so both scrubs apply. + residual = self.scrub_markup(residual) if self.eot_token in residual: residual = residual.replace(self.eot_token, "") if self.prev_tool_call_arr and residual.strip() == "": @@ -3591,7 +3617,9 @@ def parse_non_stream(self, full_text: str) -> StreamingParseResult: if pos != -1: tail_start = max(tail_start, pos + len(tok)) if tail_start != -1: - tail = full_text[tail_start:] + # has_tool_call() only recognises a COMPLETE opener, so a truncated one + # trailing the last call would ride into content; scrub before appending. + tail = self.detector.scrub_markup(full_text[tail_start:]) normal = parsed_result.normal_text or "" # Only plain text: an unterminated final block would make the # "last closer" precede it and leak markup into content. @@ -3599,7 +3627,10 @@ def parse_non_stream(self, full_text: str) -> StreamingParseResult: parsed_result.normal_text = normal + tail return parsed_result else: - return StreamingParseResult(normal_text=full_text, calls=[]) + # Re-surfacing full_text would undo the detector's own scrub and leak the + # markup. Detectors that do not scrub return it unchanged, so an + # unparseable block never blanks their response. + return StreamingParseResult(normal_text=self.detector.scrub_markup(full_text), calls=[]) def parse_stream_chunk(self, chunk_text: str) -> Tuple[str, list[ToolCallItem]]: """ diff --git a/tests/server/test_function_call_parser.py b/tests/server/test_function_call_parser.py index 34c1c04d1..e7946b320 100644 --- a/tests/server/test_function_call_parser.py +++ b/tests/server/test_function_call_parser.py @@ -343,3 +343,94 @@ def test_streaming_support_flags(): # test_streaming_model_matrix.py::test_non_streaming_detector_falls_back_to_buffered_parse). for name in SUPPORTED_TOOL_CALL_PARSERS: assert FunctionCallParser(TOOLS, tool_call_parser=name).supports_streaming() is True + + +def test_gemma4_partial_opener_does_not_leak_into_content(): + # A call block that never completes leaves an opener prefix in the text. The + # prose before it is real content; the marker is protocol framing and must not + # reach the client. + parser = FunctionCallParser(TOOLS, tool_call_parser="gemma4") + + result = parser.parse_non_stream("Let me check that.<|tool_call") + + assert result.normal_text == "Let me check that." + assert result.calls == [] + + +def test_gemma4_truncated_call_does_not_leak_into_content(): + # Generation cut mid-arguments: a well-formed opener, no closer, nothing + # parseable. Content must be empty rather than the raw block. + parser = FunctionCallParser(TOOLS, tool_call_parser="gemma4") + + result = parser.parse_non_stream('<|tool_call>call:get_weather{city:<|"|>Par') + + assert result.normal_text == "" + assert result.calls == [] + + +def test_gemma4_malformed_opener_does_not_leak_into_content(): + # The opener's closing '>' is missing, so find() misses and the whole block + # used to be surfaced verbatim. + parser = FunctionCallParser(TOOLS, tool_call_parser="gemma4") + + result = parser.parse_non_stream( + '<|tool_call call:get_weather{city:<|"|>Paris<|"|>}' + ) + + assert "<|tool_call" not in result.normal_text + + +def test_gemma4_streaming_partial_opener_does_not_leak_at_finish(): + parser = FunctionCallParser(TOOLS, tool_call_parser="gemma4") + + texts, calls = _feed(parser, ["Let me check that.", "<|tool_call"]) + + surfaced = "".join(texts) + parser.finish_stream() + assert "<|tool_call" not in surfaced + # The prose is real content: scrubbing the marker must not take it with it. + assert surfaced == "Let me check that." + assert calls == [] + + +def test_gemma4_partial_opener_after_a_parsed_call_does_not_leak(): + # The shape #203 reports: a correctly parsed tool_calls array AND a stray marker + # in content. The tail after the last closer is surfaced as content, and + # has_tool_call() misses a partial opener, so the marker rode along. + parser = FunctionCallParser(TOOLS, tool_call_parser="gemma4") + + result = parser.parse_non_stream( + '<|tool_call>call:get_weather{city:<|"|>Paris<|"|>}<|tool_call' + ) + + assert len(result.calls) == 1 + assert "<|tool_call" not in result.normal_text + + +def test_gemma4_prose_after_a_parsed_call_still_surfaces(): + # Guard for the fix above: scrubbing the tail must not swallow real trailing text. + parser = FunctionCallParser(TOOLS, tool_call_parser="gemma4") + + result = parser.parse_non_stream( + '<|tool_call>call:get_weather{city:<|"|>Paris<|"|>}\nAnything else?' + ) + + assert len(result.calls) == 1 + assert "Anything else?" in result.normal_text + + +@pytest.mark.parametrize("parser_name", ["qwen25", "mistral", "deepseekv32", "minimax"]) +def test_unparsed_block_keeps_trailing_prose(parser_name): + # A detector that surfaces raw text for an unparseable block must keep doing so: + # returning "" here would hand the client an empty message instead of the answer. + blocks = { + "qwen25": "\n{not valid json}\n\n", + "mistral": "[TOOL_CALLS] [{bad json}]\n", + "deepseekv32": "<｜DSML｜function_calls>garbage\n", + "minimax": "garbage\n", + } + parser = FunctionCallParser(TOOLS, tool_call_parser=parser_name) + + result = parser.parse_non_stream(blocks[parser_name] + "Here is the answer: 42.") + + assert result.calls == [] + assert "Here is the answer: 42." in result.normal_text