From ca931b83fe62eb024020b4e99919d0956a8ce1a1 Mon Sep 17 00:00:00 2001 From: sambai-dev <71630146+sambai-dev@users.noreply.github.com> Date: Mon, 24 Aug 2026 19:44:50 +1200 Subject: [PATCH 1/2] fix(llm): recognize raw wrappers as function calls Some models served behind plain OpenAI-compatible endpoints (Kimi K3 via the Synthetic provider) emit their native NAME wrapper instead of our dialect. The non-native function-call converter only stripped those tags when a well-formed call was already present, so the raw syntax leaked into assistant text and the tool never fired. Rewrite the malformed shape (NAME followed by a . Two guards keep this safe: it only fires when no wrapper with the tool name on its own +# line instead of our opener: +# file_editor +# view +# +_TOOL_CALL_WRAPPER_PATTERN = re.compile( + r"\s*([A-Za-z_][\w.-]*)\s*\n(?=\s* str: """Clean up model-specific formatting before parsing function calls. @@ -589,6 +599,21 @@ def _preprocess_model_output(content: str) -> str: content = re.sub(r"\s*(?= when it appears right after content = re.sub(r"(?<=)\s*", "", content) + + # Some models (like Kimi K3 served over a plain OpenAI-compatible endpoint) + # emit their native wrapper instead of our + # dialect. Rewrite it so the call is recognized instead of leaking into + # the assistant text. See: + # https://github.com/OpenHands/software-agent-sdk/issues/4540 + # + # Two guards keep this safe: only fire when the model gave us no + # NAME followed by a \n", content) + if replaced: + content = re.sub(r"\s*", "\n", content, count=1) return content diff --git a/tests/sdk/llm/test_llm_fncall_converter.py b/tests/sdk/llm/test_llm_fncall_converter.py index 71aa0e3029..ae695aea30 100644 --- a/tests/sdk/llm/test_llm_fncall_converter.py +++ b/tests/sdk/llm/test_llm_fncall_converter.py @@ -960,3 +960,80 @@ def test_security_params_excluded_when_flag_is_false(): system_content = result[0]["content"] assert "" not in system_content assert "" not in system_content + + +def test_tool_call_wrapper_normalized_to_function_call(): + """Test that a raw ```` wrapper (Kimi K3 style) is rewritten to + the canonical ```` form so the tool actually fires. + + See https://github.com/OpenHands/software-agent-sdk/issues/4540 + """ + non_fncall_messages = [ + {"role": "user", "content": "Please list the files"}, + { + "role": "assistant", + "content": ( + "I'll list the files for you.\n" + "terminal\n" + "ls\n" + "" + ), + }, + ] + + fncall_messages = convert_non_fncall_messages_to_fncall_messages( + non_fncall_messages, FNCALL_TOOLS + ) + + assistant_msg = next( + msg + for msg in fncall_messages + if msg.get("role") == "assistant" and msg.get("tool_calls") + ) + assert len(assistant_msg["tool_calls"]) == 1 + tool_call = assistant_msg["tool_calls"][0]["function"] + assert tool_call["name"] == "terminal" + assert json.loads(tool_call["arguments"]) == {"command": "ls"} + + +def test_tool_call_wrapper_without_closer_still_parses(): + """A truncated wrapper (no ````) must still be normalized; + ``_fix_stopword`` appends the missing ```` closer.""" + content = "terminal\nls" + messages = [ + {"role": "user", "content": "run ls"}, + {"role": "assistant", "content": content}, + ] + + fncall_messages = convert_non_fncall_messages_to_fncall_messages( + messages, FNCALL_TOOLS + ) + + assistant_msg = next( + msg + for msg in fncall_messages + if msg.get("role") == "assistant" and msg.get("tool_calls") + ) + assert assistant_msg["tool_calls"][0]["function"]["name"] == "terminal" + + +def test_tool_call_mention_in_plain_text_is_not_converted(): + """Prose that merely mentions ```` (e.g. an agent writing docs) + must NOT be turned into a function call.""" + content = ( + "Here is how you write a tool call:\n\n" + " is the opening tag, followed by parameters." + ) + messages = [ + {"role": "user", "content": "explain the format"}, + {"role": "assistant", "content": content}, + ] + + fncall_messages = convert_non_fncall_messages_to_fncall_messages( + messages, FNCALL_TOOLS + ) + + assistant_msg = next( + msg for msg in fncall_messages if msg.get("role") == "assistant" + ) + assert not assistant_msg.get("tool_calls") From 4f3b6473342d320cc720afbbaead1c9e7deec823 Mon Sep 17 00:00:00 2001 From: sambai-dev <71630146+sambai-dev@users.noreply.github.com> Date: Mon, 24 Aug 2026 19:44:51 +1200 Subject: [PATCH 2/2] fix(llm): preserve literal tool-call tags --- .../sdk/llm/mixins/fn_call_converter.py | 20 ++---- tests/sdk/llm/test_llm_fncall_converter.py | 70 ++++++++----------- 2 files changed, 34 insertions(+), 56 deletions(-) diff --git a/openhands-sdk/openhands/sdk/llm/mixins/fn_call_converter.py b/openhands-sdk/openhands/sdk/llm/mixins/fn_call_converter.py index 0cddec095e..a41e088f58 100644 --- a/openhands-sdk/openhands/sdk/llm/mixins/fn_call_converter.py +++ b/openhands-sdk/openhands/sdk/llm/mixins/fn_call_converter.py @@ -574,8 +574,8 @@ def _extract_and_validate_params( return params -# Some models emit their own wrapper with the tool name on its own -# line instead of our opener: +# Kimi K3 can emit a wrapper with the tool name on its own line +# instead of the prompted opener (see issue #4540): # file_editor # view # @@ -600,20 +600,12 @@ def _preprocess_model_output(content: str) -> str: # Strip when it appears right after content = re.sub(r"(?<=)\s*", "", content) - # Some models (like Kimi K3 served over a plain OpenAI-compatible endpoint) - # emit their native wrapper instead of our - # dialect. Rewrite it so the call is recognized instead of leaking into - # the assistant text. See: - # https://github.com/OpenHands/software-agent-sdk/issues/4540 - # - # Two guards keep this safe: only fire when the model gave us no - # NAME followed by a \n", content) + content, replaced = _TOOL_CALL_WRAPPER_PATTERN.subn( + r"\n", content, count=1 + ) if replaced: - content = re.sub(r"\s*", "\n", content, count=1) + content = re.sub(r"(\s*)\Z", r"\1", content, count=1) return content diff --git a/tests/sdk/llm/test_llm_fncall_converter.py b/tests/sdk/llm/test_llm_fncall_converter.py index ae695aea30..0bd18a97b5 100644 --- a/tests/sdk/llm/test_llm_fncall_converter.py +++ b/tests/sdk/llm/test_llm_fncall_converter.py @@ -962,46 +962,41 @@ def test_security_params_excluded_when_flag_is_false(): assert "" not in system_content -def test_tool_call_wrapper_normalized_to_function_call(): - """Test that a raw ```` wrapper (Kimi K3 style) is rewritten to - the canonical ```` form so the tool actually fires. - - See https://github.com/OpenHands/software-agent-sdk/issues/4540 - """ - non_fncall_messages = [ - {"role": "user", "content": "Please list the files"}, - { - "role": "assistant", - "content": ( - "I'll list the files for you.\n" - "terminal\n" - "ls\n" - "" - ), - }, +@pytest.mark.parametrize( + "content", + [ + ( + "I'll list the files.\n" + "terminal\n" + "ls\n" + "" + ), + "terminal\nls", + ], +) +def test_tool_call_wrapper_normalized_to_function_call(content): + messages = [ + {"role": "user", "content": "run ls"}, + {"role": "assistant", "content": content}, ] fncall_messages = convert_non_fncall_messages_to_fncall_messages( - non_fncall_messages, FNCALL_TOOLS + messages, FNCALL_TOOLS ) - assistant_msg = next( - msg - for msg in fncall_messages - if msg.get("role") == "assistant" and msg.get("tool_calls") - ) - assert len(assistant_msg["tool_calls"]) == 1 - tool_call = assistant_msg["tool_calls"][0]["function"] + tool_call = fncall_messages[1]["tool_calls"][0]["function"] assert tool_call["name"] == "terminal" assert json.loads(tool_call["arguments"]) == {"command": "ls"} -def test_tool_call_wrapper_without_closer_still_parses(): - """A truncated wrapper (no ````) must still be normalized; - ``_fix_stopword`` appends the missing ```` closer.""" - content = "terminal\nls" +def test_tool_call_wrapper_preserves_closing_tag_in_parameter(): + content = ( + "terminal\n" + "printf ''\n" + "" + ) messages = [ - {"role": "user", "content": "run ls"}, + {"role": "user", "content": "print the closing tag"}, {"role": "assistant", "content": content}, ] @@ -1009,17 +1004,11 @@ def test_tool_call_wrapper_without_closer_still_parses(): messages, FNCALL_TOOLS ) - assistant_msg = next( - msg - for msg in fncall_messages - if msg.get("role") == "assistant" and msg.get("tool_calls") - ) - assert assistant_msg["tool_calls"][0]["function"]["name"] == "terminal" + tool_call = fncall_messages[1]["tool_calls"][0]["function"] + assert json.loads(tool_call["arguments"]) == {"command": "printf ''"} def test_tool_call_mention_in_plain_text_is_not_converted(): - """Prose that merely mentions ```` (e.g. an agent writing docs) - must NOT be turned into a function call.""" content = ( "Here is how you write a tool call:\n\n" " is the opening tag, followed by parameters." @@ -1033,7 +1022,4 @@ def test_tool_call_mention_in_plain_text_is_not_converted(): messages, FNCALL_TOOLS ) - assistant_msg = next( - msg for msg in fncall_messages if msg.get("role") == "assistant" - ) - assert not assistant_msg.get("tool_calls") + assert not fncall_messages[1].get("tool_calls")