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)