From 6bb44322be3135654c8f5b911e28b4fe82759b29 Mon Sep 17 00:00:00 2001 From: Buddhika Godakanda <54194807+hacksics@users.noreply.github.com> Date: Sun, 26 Jul 2026 23:56:06 +0530 Subject: [PATCH] Recover truncated tool calls, and never pass a leaked one off as an answer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Small local models drift off the tool-call format, especially with a large tool schema in play. Two failures followed from that, both of which ended the turn looking like the model had simply stopped mid-sentence. Salvage handled well-formed Qwen/Hermes XML but gave up entirely on a call cut off partway through. It now takes the function name plus every parameter that actually closed. A trailing unterminated `` is dropped rather than guessed, so a half-written path or file body can never reach a tool; if that leaves a required argument missing, the call fails validation and the model gets a corrective tool error, which is the agent loop working. Worse, a call that never parsed at all was reported as `status: completed` — indistinguishable from the model deciding it was done, leaving the user with narration trailing off into stray closing tags. It now ends on the error path, so the GUI offers Retry. That is the right affordance here: the drift is probabilistic, not deterministic, so the same model usually succeeds on a second attempt. Detection ignores fenced and inline code, so a model *explaining* tool-call syntax is still a real answer, and requires that tools were offered at all. Reported against qwen3.5-9b on LM Studio, 2026-07-26. Co-Authored-By: Claude --- coworker/engine.py | 20 ++++++++++ coworker/providers/openai_provider.py | 55 +++++++++++++++++++++++++++ tests/test_engine.py | 36 ++++++++++++++++++ tests/test_provider_router.py | 53 +++++++++++++++++++++++++- 4 files changed, 163 insertions(+), 1 deletion(-) diff --git a/coworker/engine.py b/coworker/engine.py index 668d51e6..97237327 100644 --- a/coworker/engine.py +++ b/coworker/engine.py @@ -24,6 +24,7 @@ from .progress import bind_sink, reset_sink, wants_progress from .providers import AssistantTurn, ProviderClient, ToolCall from .providers.errors import friendly_model_error +from .providers.openai_provider import looks_like_unparsed_tool_call from .tools import ToolRegistry @@ -367,6 +368,25 @@ def _partial_turn() -> AssistantTurn: if self._steering: self._inject_steering() continue + # The model tried to call a tool and the syntax never parsed — salvage already + # had its go. Ending as "completed" here would present a half-written call as + # the answer, which is indistinguishable from the model deciding it was done; + # the user just sees narration trailing off into stray tags. Fail loudly + # instead, on the error path so the GUI offers Retry — this is drift, not a + # deterministic failure, so retrying the same model usually works. + if looks_like_unparsed_tool_call(turn.text, self.registry.schemas() or None): + message = ( + f"{self.model} replied with a tool call this endpoint couldn't parse, " + "so the turn was stopped rather than answered from a partial call. " + "Retry, or switch to a larger model — smaller local models drift off " + "the tool-call format, especially with many tools in play." + ) + self._append_notice("error", message) + yield Event( + EventType.ERROR, + {"error": message, "error_type": "UnparsedToolCall"}, + ) + return yield Event( EventType.TURN_END, {"status": "completed", "iterations": iterations}, diff --git a/coworker/providers/openai_provider.py b/coworker/providers/openai_provider.py index 42edec85..790391ce 100644 --- a/coworker/providers/openai_provider.py +++ b/coworker/providers/openai_provider.py @@ -306,6 +306,45 @@ def _parse_tool_calls(raw_tool_calls: Any) -> list[ToolCall]: re.IGNORECASE | re.DOTALL, ) +# A `` that never closes — the model ran out of tokens (or drifted) partway +# through writing the call. Anchored to end-of-text so it only matches a genuinely unfinished +# tail, never a well-formed block earlier in the message. Small local models hit this often on +# a large tool schema, and the turn used to end silently on the leftover text. +_FUNCTION_OPEN_TRUNCATED = re.compile( + r"[^>\s]+)\s*>(?P(?:(?!).)*)$", + re.IGNORECASE | re.DOTALL, +) + +# Markers that mean "this text IS a tool call the endpoint failed to parse", used to tell a +# real answer from a leaked one. Fenced code is stripped first: a model *explaining* tool-call +# syntax in a ``` block is answering, not calling. +_LEAKED_TOOL_SYNTAX = ( + "", + "", + "", + "", + "", + " bool: + """True when assistant text still carries tool-call markup that salvage couldn't turn into + a call — i.e. the model tried to call a tool and the syntax was mangled or cut off. + + Only meaningful when tools were actually offered, and only over OpenAI-compatible endpoints + that parse tool calls out of the model's raw output (LM Studio, Ollama, vLLM). The caller + uses it to end the turn as a retriable error instead of presenting the fragment as an answer. + """ + if not tools or not text: + return False + return any(m in _FENCED.sub("", text).lower() for m in _LEAKED_TOOL_SYNTAX) + def _coerce_param(raw: str) -> Any: """Keep free-text verbatim (the common case: file content), but recover real JSON values when @@ -478,6 +517,22 @@ def _salvage_tool_calls_from_text( if calls: return _renumber(calls) + # 1c) A TRUNCATED XML call: `` with no closing tag, because the model ran + # out of tokens mid-call. Take the name plus every parameter that DID close; a trailing + # unterminated `` is dropped rather than guessed, so a half-written path or + # file body can never reach a tool. If that leaves a required argument missing the call + # fails validation and the model gets a corrective tool error — which is the agent loop + # working, and strictly better than the turn ending on the leftover fragment. + tm = _FUNCTION_OPEN_TRUNCATED.search(text) + if tm: + name = tm.group("name").strip() + if names is None or name in names: + args = { + pm.group("key").strip(): _coerce_param(pm.group("val")) + for pm in _PARAM_BLOCK.finditer(tm.group("body")) + } + return _renumber([ToolCall(id="", name=name, arguments=args)]) + # 2) Embedded {"name": …, "arguments": …} objects, even surrounded by prose. for sub in _iter_top_objects(text): d = _loads(sub) diff --git a/tests/test_engine.py b/tests/test_engine.py index 90e38fdb..3c4c24e2 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -439,3 +439,39 @@ def capabilities(self, model): assert all(p["type"] != "image_url" for p in parts) assert "not viewable" in parts[-1]["text"] assert engine.messages[-1]["content"][1]["type"] == "image_url" # history untouched + + +def test_leaked_tool_call_ends_the_turn_as_a_retriable_error(tmp_path): + """A tool call the endpoint couldn't parse must not pass as an answer. Ending "completed" + made a half-written call indistinguishable from the model deciding it was done — the user + saw narration trailing off into stray tags (owner report 2026-07-26, qwen3.5-9b on LM + Studio). It ends on the error path so the GUI offers Retry; the drift is probabilistic, so + retrying the same model usually works.""" + leaked = "Let me read the key files.\n\n\n\n```\nThat's it.")], + ) + events = _collect(engine, "how does qwen format tool calls?") + assert EventType.ERROR not in _types(events) + assert next(ev for ev in events if ev.type == EventType.TURN_END).data["status"] == "completed" diff --git a/tests/test_provider_router.py b/tests/test_provider_router.py index b02027ef..09bd7df6 100644 --- a/tests/test_provider_router.py +++ b/tests/test_provider_router.py @@ -19,7 +19,10 @@ _normalize_ollama_url, build_provider_client, ) -from coworker.providers.openai_provider import _salvage_tool_calls_from_text +from coworker.providers.openai_provider import ( + _salvage_tool_calls_from_text, + looks_like_unparsed_tool_call, +) # -- base_url passthrough ------------------------------------------------------- @@ -274,6 +277,21 @@ def test_salvage_ignores_prose(): ] +_GREP_TOOL = [ + { + "type": "function", + "function": { + "name": "grep", + "parameters": { + "type": "object", + "properties": {"pattern": {"type": "string"}, "path": {"type": "string"}}, + "required": ["pattern"], + }, + }, + } +] + + def test_salvage_mixed_prose_and_object(): # The model wrote prose THEN a bare-JSON tool call in one message. text = 'It seems the workspace is empty. {"name": "list_files", "arguments": {"recursive": true}}' @@ -309,6 +327,39 @@ def test_salvage_filters_unknown_tool_name(): assert _salvage_tool_calls_from_text(text, _TODO_TOOLS) == [] +def test_salvage_truncated_xml_call_keeps_only_complete_parameters(): + """A local model that runs out of tokens mid-call leaves `` unclosed. Take the + name and every parameter that DID close; NEVER the half-written trailing one — a truncated + path or file body reaching a tool is worse than no call at all.""" + text = "\n\nTODO\nsr" + calls = _salvage_tool_calls_from_text(text, _TODO_TOOLS + _GREP_TOOL) + assert len(calls) == 1 and calls[0].name == "grep" + assert calls[0].arguments == {"pattern": "TODO"} # the partial `path` is gone + + +def test_salvage_truncated_xml_prefers_a_complete_call_and_filters_unknown_names(): + complete_then_cut = ( + "true" + "\n\n" + ) + calls = _salvage_tool_calls_from_text(complete_then_cut, _TODO_TOOLS + _GREP_TOOL) + assert [c.name for c in calls] == ["list_files"] # the finished one wins + # An unfinished call naming something we never offered stays text (no false positives). + assert _salvage_tool_calls_from_text("\n/", _TODO_TOOLS) == [] + + +def test_looks_like_unparsed_tool_call_ignores_code_and_needs_tools(): + """Distinguishes a leaked call from a model *explaining* tool syntax — the latter is a real + answer and must not be turned into an error.""" + leaked = "Let me read the files.\n\n\n" + assert looks_like_unparsed_tool_call(leaked, _TODO_TOOLS) is True + assert looks_like_unparsed_tool_call("A CLI that greets people.", _TODO_TOOLS) is False + fenced = "Qwen writes calls like:\n```\n\n```\nThat's the shape." + assert looks_like_unparsed_tool_call(fenced, _TODO_TOOLS) is False + assert looks_like_unparsed_tool_call("The `` wrapper.", _TODO_TOOLS) is False + assert looks_like_unparsed_tool_call(leaked, None) is False # no tools offered → not a call + + def test_salvage_nested_braces_in_tag(): text = '{"name": "todo_write", "arguments": {"items": [{"content": "a", "status": "pending"}]}}' calls = _salvage_tool_calls_from_text(text, _TODO_TOOLS)