From 78094a34e3d9b8a7eec2faeca7153109690a34be Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Wed, 8 Jul 2026 12:06:18 -0700 Subject: [PATCH 1/3] fix: preserve tool_calls in multi-turn history and skip malformed tool-arg JSON (OpenAI-compatible backends) Two bugs broke multi-turn agent loops (start_session + repeated instruct(..., tool_calls=True)) on OpenAI-compatible backends: 1. History serialization dropped tool calls. A tool-only assistant turn was stored as str(msg['tool_calls']) in Message.content, and message_to_openai_message emitted only {role, content} with that repr and no tool_calls key. Spec-strict providers (Cohere via OpenRouter, etc.) rejected the replayed history with '400 ... must have non-empty content or tool calls'. Message now carries an optional tool_calls list; _parse passes the raw OpenAI-shape list through (and builds it for the ollama branch), and the serializer emits tool_calls with content: null for tool-only turns. 2. Truncated streamed tool arguments raised an uncaught json.JSONDecodeError in extract_model_tool_requests that killed the agent. The parse is now wrapped to warn and skip the bad call, mirroring the existing unknown-tool skip. Scoped to the two reported bugs; the role:'tool'/tool_call_id feedback half is left as a follow-up. Closes #1349 --- mellea/helpers/openai_compatible_helpers.py | 38 ++++---- mellea/stdlib/components/chat.py | 31 +++++- .../helpers/test_openai_compatible_helpers.py | 57 ++++++++++- test/stdlib/components/test_chat.py | 97 +++++++++++++++++-- 4 files changed, 184 insertions(+), 39 deletions(-) diff --git a/mellea/helpers/openai_compatible_helpers.py b/mellea/helpers/openai_compatible_helpers.py index df1c09771..67dcf77ec 100644 --- a/mellea/helpers/openai_compatible_helpers.py +++ b/mellea/helpers/openai_compatible_helpers.py @@ -78,7 +78,14 @@ def extract_model_tool_requests( args = {} if tool_args is not None: # Returns the args as a string. Parse it here. - args = json.loads(tool_args) + try: + args = json.loads(tool_args) + except json.JSONDecodeError: + MelleaLogger.get_logger().warning( + f"model returned malformed JSON arguments for tool {tool_name!r} " + f"(possibly truncated during streaming); skipping this tool call: {tool_args!r}" + ) + continue # Validate and coerce argument types validated_args = validate_tool_arguments(func, args, strict=False) @@ -189,7 +196,7 @@ def message_to_openai_message(msg: Message, formatter: Formatter | None = None) Returns: A dict with `"role"` and `"content"` fields. When the message carries images, `"content"` is a list of text and image-URL dicts; otherwise it - is a plain string. + is a plain string. Assistant messages may also include `"tool_calls"`. """ # NOTE: `self.formatter.to_chat_messages` explicitly skips `Message` objects. However, we need # to print `Message`s to correctly serialize any documents with the message. Do the printing here. @@ -205,28 +212,19 @@ def message_to_openai_message(msg: Message, formatter: Formatter | None = None) url = raw if raw.startswith("data:") else f"data:image/png;base64,{raw}" img_list.append({"type": "image_url", "image_url": {"url": url}}) - return { + result = { "role": msg.role, "content": [{"type": "text", "text": content}, *img_list], } else: - return {"role": msg.role, "content": content} - # Target format: - # { - # "role": "user", - # "content": [ - # { - # "type": "text", - # "text": "What's in this picture?" - # }, - # { - # "type": "image_url", - # "image_url": { - # "url": "data:image/jpeg;base64," - # } - # } - # ] - # } + result = {"role": msg.role, "content": content} + + tool_calls = getattr(msg, "tool_calls", None) + if tool_calls: + result["tool_calls"] = tool_calls + if msg.images is None and not content: + result["content"] = None + return result def messages_to_docs(msgs: list[Message]) -> list[dict[str, str]]: diff --git a/mellea/stdlib/components/chat.py b/mellea/stdlib/components/chat.py index cb0be5823..61585e4d9 100644 --- a/mellea/stdlib/components/chat.py +++ b/mellea/stdlib/components/chat.py @@ -11,7 +11,7 @@ import logging from collections.abc import Callable, Iterable, Mapping -from typing import Any, Literal, get_args +from typing import Any, Literal, cast, get_args from ...core import ( CBlock, @@ -42,6 +42,8 @@ class Message(Component["Message"]): backends that require base64 will raise a ``ValueError``). documents (list[Document] | None): Optional documents associated with the message. + tool_calls (list[dict[str, Any]] | None): Optional OpenAI-compatible + assistant tool calls associated with the message. Attributes: Role (type): Type alias for the allowed role literals: `"system"`, @@ -57,6 +59,7 @@ def __init__( *, images: None | list[ImageBlock | ImageUrlBlock] = None, documents: None | Iterable[str | Document] = None, + tool_calls: list[dict[str, Any]] | None = None, ): """Initialize a Message with a role, text content, and optional images and documents.""" if role not in get_args(Message.Role): @@ -68,12 +71,18 @@ def __init__( self._content_cblock = CBlock(self.content) self._images = images self._docs = _coerce_to_documents(documents) + self._tool_calls = tool_calls @property def images(self) -> None | list[ImageBlock | ImageUrlBlock]: """Returns the images associated with this message.""" return self._images + @property + def tool_calls(self) -> list[dict[str, Any]] | None: + """Returns the OpenAI-compatible tool calls associated with this message.""" + return self._tool_calls + def parts(self) -> list[Component | CBlock | ModelOutputThunk]: """Return the constituent parts of this message, including content, documents, and images. @@ -128,18 +137,30 @@ def _parse(self, computed: ModelOutputThunk) -> "Message": if computed.tool_calls is not None: # A tool was successfully requested. - # Assistant responses for tool calling differ by backend. For the default formatter, - # we put all of the function data into the content field in the same format we received it. + # Assistant responses for tool calling differ by backend. Preserve + # OpenAI-compatible tool calls separately from message content when + # the provider gives us structured tool-call data. if provider == "ollama" and response is not None: + from ...helpers.openai_compatible_helpers import build_tool_calls + + tool_calls = cast( + list[dict[str, Any]], build_tool_calls(computed) or [] + ) return Message( - role=response.message.role, content=str(response.message.tool_calls) + role=response.message.role, + content=getattr(response.message, "content", "") or "", + tool_calls=tool_calls, ) if provider in ("openai", "watsonx", "litellm") and isinstance( response, dict ): choice = response["choices"][0] if "choices" in response else response msg = choice["message"] - return Message(role=msg["role"], content=str(msg.get("tool_calls", []))) + return Message( + role=msg["role"], + content=msg.get("content") or "", + tool_calls=msg.get("tool_calls", []), + ) # Hugging Face (or others). There are no guarantees on how the model represented the function calls. # Output it in the same format we received the tool call request. assert computed.value is not None diff --git a/test/helpers/test_openai_compatible_helpers.py b/test/helpers/test_openai_compatible_helpers.py index db9847c3f..508365f81 100644 --- a/test/helpers/test_openai_compatible_helpers.py +++ b/test/helpers/test_openai_compatible_helpers.py @@ -135,13 +135,42 @@ def test_mixed_known_and_unknown(self): assert len(result) == 1 assert "get_weather" in result - def test_malformed_json_arguments_raises(self): - """Malformed JSON from the model propagates as JSONDecodeError.""" + def test_malformed_json_arguments_skipped(self, caplog): + """Malformed JSON from the model skips that tool call.""" tool = _make_tool("get_weather") tools = {"get_weather": tool} - response = _response_with_tool_calls([_tool_call("get_weather", '{"broken')]) - with pytest.raises(json.JSONDecodeError): - extract_model_tool_requests(tools, response) + response = _response_with_tool_calls([_tool_call("get_weather", '{"path": "')]) + + with caplog.at_level("WARNING", logger="mellea"): + result = extract_model_tool_requests(tools, response) + + assert result is None + assert any( + "malformed JSON arguments for tool 'get_weather'" in record.message + for record in caplog.records + ) + + def test_mixed_valid_and_malformed_json_arguments(self, caplog): + """Valid tool calls are extracted while malformed calls are skipped.""" + tool = _make_tool("get_weather") + tools = {"get_weather": tool} + response = _response_with_tool_calls( + [ + _tool_call("get_weather", json.dumps({"location": "LA"})), + _tool_call("get_weather", '{"location": "'), + ] + ) + + with caplog.at_level("WARNING", logger="mellea"): + result = extract_model_tool_requests(tools, response) + + assert result is not None + assert len(result) == 1 + assert result["get_weather"].args["location"] == "LA" + assert any( + "malformed JSON arguments for tool 'get_weather'" in record.message + for record in caplog.records + ) # --- chat_completion_delta_merge --- @@ -295,6 +324,24 @@ def test_text_only(self): result = message_to_openai_message(msg) assert result == {"role": "user", "content": "hello"} + def test_tool_calls_with_empty_content(self): + tool_calls = [ + { + "id": "call_123", + "type": "function", + "function": {"name": "get_weather", "arguments": "{}"}, + } + ] + msg = Message(role="assistant", content="", tool_calls=tool_calls) + + result = message_to_openai_message(msg) + + assert result == { + "role": "assistant", + "content": None, + "tool_calls": tool_calls, + } + def test_with_images(self): img = ImageBlock(_B64_PNG) msg = Message(role="user", content="describe this", images=[img]) diff --git a/test/stdlib/components/test_chat.py b/test/stdlib/components/test_chat.py index 3d43bdd6c..80725e76e 100644 --- a/test/stdlib/components/test_chat.py +++ b/test/stdlib/components/test_chat.py @@ -2,12 +2,15 @@ import pytest +from mellea.backends.tools import MelleaTool from mellea.core import ( CBlock, ModelOutputThunk, + ModelToolCall, RawProviderResponse, TemplateRepresentation, ) +from mellea.formatters.chat_formatter import ChatFormatter from mellea.formatters.template_formatter import TemplateFormatter from mellea.helpers import message_to_openai_message, messages_to_docs from mellea.stdlib.components import Document, Message @@ -19,6 +22,16 @@ from mellea.stdlib.context import ChatContext +def _make_tool_call(name: str, args: dict[str, object] | None = None) -> ModelToolCall: + """Create a ModelToolCall with a simple backing tool for tests.""" + + def test_tool(location: str = "LA") -> str: + return f"result for {location}" + + tool = MelleaTool.from_callable(test_tool, name) + return ModelToolCall(name=name, func=tool, args=args or {"location": "LA"}) + + def test_message_with_docs(): doc = Document("I'm text!", "Im a title!") msg = Message("user", "hello", documents=[doc]) @@ -185,22 +198,43 @@ def test_parse_openai_streamed_choice_shape(): def test_parse_tool_calls_ollama(): msg = Message("user", "q") - mot = ModelOutputThunk(value="v", tool_calls={"some_fn": None}) + mot = ModelOutputThunk( + value="v", tool_calls={"some_fn": _make_tool_call("some_fn")} + ) fake_calls = [{"name": "some_fn"}] fake_response = type( "Resp", (), - {"message": type("Msg", (), {"role": "assistant", "tool_calls": fake_calls})()}, + { + "message": type( + "Msg", + (), + {"role": "assistant", "content": None, "tool_calls": fake_calls}, + )() + }, )() mot.raw = RawProviderResponse(provider="ollama", response=fake_response) result = msg._parse(mot) assert result.role == "assistant" - assert "some_fn" in result.content + assert result.content == "" + assert result.tool_calls is not None + assert len(result.tool_calls) == 1 + assert result.tool_calls[0]["id"].startswith("call_") + assert result.tool_calls[0]["type"] == "function" + assert result.tool_calls[0]["function"]["name"] == "some_fn" + assert result.tool_calls[0]["function"]["arguments"] == '{"location": "LA"}' def test_parse_tool_calls_openai(): msg = Message("user", "q") mot = ModelOutputThunk(value="v", tool_calls={"fn": None}) + tool_calls = [ + { + "id": "call_openai", + "type": "function", + "function": {"name": "fn", "arguments": "{}"}, + } + ] mot.raw = RawProviderResponse( provider="openai", response={ @@ -208,7 +242,8 @@ def test_parse_tool_calls_openai(): { "message": { "role": "assistant", - "tool_calls": [{"function": {"name": "fn"}}], + "content": None, + "tool_calls": tool_calls, } } ] @@ -216,6 +251,8 @@ def test_parse_tool_calls_openai(): ) result = msg._parse(mot) assert result.role == "assistant" + assert result.content == "" + assert result.tool_calls == tool_calls def test_parse_tool_calls_openai_streamed_choice_shape(): @@ -226,19 +263,61 @@ def test_parse_tool_calls_openai_streamed_choice_shape(): """ msg = Message("user", "q") mot = ModelOutputThunk(value="v", tool_calls={"fn": None}) + tool_calls = [ + { + "id": "call_streamed", + "type": "function", + "function": {"name": "fn", "arguments": "{}"}, + } + ] mot.raw = RawProviderResponse( provider="openai", response={ "finish_reason": "tool_calls", - "message": { - "role": "assistant", - "tool_calls": [{"function": {"name": "fn"}}], - }, + "message": {"role": "assistant", "content": None, "tool_calls": tool_calls}, }, ) result = msg._parse(mot) assert result.role == "assistant" - assert "fn" in result.content + assert result.content == "" + assert result.tool_calls == tool_calls + + +def test_tool_call_message_survives_formatter_to_openai_history(): + """Tool-only assistant turns survive chat formatting and OpenAI serialization.""" + tool_calls = [ + { + "id": "call_history", + "type": "function", + "function": {"name": "fn", "arguments": "{}"}, + } + ] + prompt = Message("user", "call fn") + mot = ModelOutputThunk(value="", tool_calls={"fn": None}) + mot.raw = RawProviderResponse( + provider="openai", + response={ + "finish_reason": "tool_calls", + "message": {"role": "assistant", "content": None, "tool_calls": tool_calls}, + }, + ) + mot.parsed_repr = prompt._parse(mot) + ctx = ChatContext().add(prompt).add(mot) + + # ChatFormatter is abstract (print()); to_chat_messages is what this test + # exercises, so use a minimal concrete subclass that stubs print(). + class _ChatFormatter(ChatFormatter): + def print(self, c): + return "" + + messages = _ChatFormatter().to_chat_messages(ctx.as_list()) + serialized = [message_to_openai_message(message) for message in messages] + + assert serialized[1] == { + "role": "assistant", + "content": None, + "tool_calls": tool_calls, + } def test_parse_tool_calls_fallback_uses_value(): From b97798759a542f360f2ab9f364acef714d399691 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Thu, 9 Jul 2026 21:52:31 -0700 Subject: [PATCH 2/3] fix: address review nits on tool_calls handling Normalize absent/empty tool_calls to None when reconstructing Messages, and document the tool-only assistant turn shape in the helper docstring. Co-Authored-By: Claude Fable 5 Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> --- mellea/helpers/openai_compatible_helpers.py | 4 +++- mellea/stdlib/components/chat.py | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/mellea/helpers/openai_compatible_helpers.py b/mellea/helpers/openai_compatible_helpers.py index 67dcf77ec..4d3cc82ad 100644 --- a/mellea/helpers/openai_compatible_helpers.py +++ b/mellea/helpers/openai_compatible_helpers.py @@ -196,7 +196,9 @@ def message_to_openai_message(msg: Message, formatter: Formatter | None = None) Returns: A dict with `"role"` and `"content"` fields. When the message carries images, `"content"` is a list of text and image-URL dicts; otherwise it - is a plain string. Assistant messages may also include `"tool_calls"`. + is a plain string. For tool-only assistant turns, `"content"` is `None` + and `"tool_calls"` carries the structured call list. When content is + present alongside tool calls, both keys are included. """ # NOTE: `self.formatter.to_chat_messages` explicitly skips `Message` objects. However, we need # to print `Message`s to correctly serialize any documents with the message. Do the printing here. diff --git a/mellea/stdlib/components/chat.py b/mellea/stdlib/components/chat.py index 61585e4d9..3f09e4a4d 100644 --- a/mellea/stdlib/components/chat.py +++ b/mellea/stdlib/components/chat.py @@ -159,7 +159,7 @@ def _parse(self, computed: ModelOutputThunk) -> "Message": return Message( role=msg["role"], content=msg.get("content") or "", - tool_calls=msg.get("tool_calls", []), + tool_calls=msg.get("tool_calls") or None, ) # Hugging Face (or others). There are no guarantees on how the model represented the function calls. # Output it in the same format we received the tool call request. From 9bbd0ee262a533f7faa0810c67b96343aeb62ce4 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Sat, 11 Jul 2026 12:19:28 -0700 Subject: [PATCH 3/3] fix: satisfy mypy on openai_compatible_helpers Small typing fix requested in review; mypy, ruff, and targeted tests pass locally. --- mellea/helpers/openai_compatible_helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mellea/helpers/openai_compatible_helpers.py b/mellea/helpers/openai_compatible_helpers.py index 4d3cc82ad..a37c20953 100644 --- a/mellea/helpers/openai_compatible_helpers.py +++ b/mellea/helpers/openai_compatible_helpers.py @@ -214,7 +214,7 @@ def message_to_openai_message(msg: Message, formatter: Formatter | None = None) url = raw if raw.startswith("data:") else f"data:image/png;base64,{raw}" img_list.append({"type": "image_url", "image_url": {"url": url}}) - result = { + result: dict[str, Any] = { "role": msg.role, "content": [{"type": "text", "text": content}, *img_list], }