From 8e4396bb32c0f0638d05403445b9b39d9d8ea2a8 Mon Sep 17 00:00:00 2001 From: diego-coder <39010417+diego-coder@users.noreply.github.com> Date: Mon, 21 Jul 2025 09:11:22 -0700 Subject: [PATCH] fix(ollama): robustly parse single-quoted JSON in tool calls (#32109) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Description:** This PR makes argument parsing for Ollama tool calls more robust. Some LLMs—including Ollama—may return arguments as Python-style dictionaries with single quotes (e.g., `{'a': 1}`), which are not valid JSON and previously caused parsing to fail. The updated `_parse_json_string` method in `langchain_ollama.chat_models` now attempts standard JSON parsing and, if that fails, falls back to `ast.literal_eval` for safe evaluation of Python-style dictionaries. This improves interoperability with LLMs and fixes a common usability issue for tool-based agents. **Issue:** Closes #30910 **Dependencies:** None **Tests:** - Added new unit tests for double-quoted JSON, single-quoted dicts, mixed quoting, and malformed/failure cases. - All tests pass locally, including new coverage for single-quoted inputs. **Notes:** - No breaking changes. - No new dependencies introduced. - Code is formatted and linted (`ruff format`, `ruff check`). - If maintainers have suggestions for further improvements, I’m happy to revise! Thank you for maintaining LangChain! Looking forward to your feedback. --- libs/langchain/pyproject.toml | 2 +- .../ollama/langchain_ollama/chat_models.py | 39 +++++++---- .../tests/unit_tests/test_chat_models.py | 65 +++++++++++++++++-- 3 files changed, 88 insertions(+), 18 deletions(-) diff --git a/libs/langchain/pyproject.toml b/libs/langchain/pyproject.toml index a373c842cdd..1b6c4f055b2 100644 --- a/libs/langchain/pyproject.toml +++ b/libs/langchain/pyproject.toml @@ -5,7 +5,7 @@ build-backend = "pdm.backend" [project] authors = [] license = { text = "MIT" } -requires-python = ">=3.9" +requires-python = ">=3.9, <4.0" dependencies = [ "langchain-core<1.0.0,>=0.3.66", "langchain-text-splitters<1.0.0,>=0.3.8", diff --git a/libs/partners/ollama/langchain_ollama/chat_models.py b/libs/partners/ollama/langchain_ollama/chat_models.py index b987e8492d4..4fbe9bce062 100644 --- a/libs/partners/ollama/langchain_ollama/chat_models.py +++ b/libs/partners/ollama/langchain_ollama/chat_models.py @@ -2,6 +2,7 @@ from __future__ import annotations +import ast import json from collections.abc import AsyncIterator, Iterator, Mapping, Sequence from operator import itemgetter @@ -77,33 +78,45 @@ def _get_usage_metadata_from_generation_info( def _parse_json_string( json_string: str, + *, raw_tool_call: dict[str, Any], - skip: bool, # noqa: FBT001 + skip: bool, ) -> Any: """Attempt to parse a JSON string for tool calling. + It first tries to use the standard json.loads. If that fails, it falls + back to ast.literal_eval to safely parse Python literals, which is more + robust against models using single quotes or containing apostrophes. + Args: json_string: JSON string to parse. - skip: Whether to ignore parsing errors and return the value anyways. raw_tool_call: Raw tool call to include in error message. + skip: Whether to ignore parsing errors and return the value anyways. Returns: - The parsed JSON string. + The parsed JSON string or Python literal. Raises: - OutputParserException: If the JSON string wrong invalid and skip=False. + OutputParserException: If the string is invalid and skip=False. """ try: return json.loads(json_string) - except json.JSONDecodeError as e: - if skip: - return json_string - msg = ( - f"Function {raw_tool_call['function']['name']} arguments:\n\n" - f"{raw_tool_call['function']['arguments']}\n\nare not valid JSON. " - f"Received JSONDecodeError {e}" - ) - raise OutputParserException(msg) from e + except json.JSONDecodeError: + try: + # Use ast.literal_eval to safely parse Python-style dicts + # (e.g. with single quotes) + return ast.literal_eval(json_string) + except (SyntaxError, ValueError) as e: + # If both fail, and we're not skipping, raise an informative error. + if skip: + return json_string + msg = ( + f"Function {raw_tool_call['function']['name']} arguments:\n\n" + f"{raw_tool_call['function']['arguments']}" + "\n\nare not valid JSON or a Python literal. " + f"Received error: {e}" + ) + raise OutputParserException(msg) from e except TypeError as e: if skip: return json_string diff --git a/libs/partners/ollama/tests/unit_tests/test_chat_models.py b/libs/partners/ollama/tests/unit_tests/test_chat_models.py index e7ac5a29957..3a856a48833 100644 --- a/libs/partners/ollama/tests/unit_tests/test_chat_models.py +++ b/libs/partners/ollama/tests/unit_tests/test_chat_models.py @@ -8,10 +8,15 @@ from unittest.mock import patch import pytest from httpx import Client, Request, Response +from langchain_core.exceptions import OutputParserException from langchain_core.messages import ChatMessage from langchain_tests.unit_tests import ChatModelUnitTests -from langchain_ollama.chat_models import ChatOllama, _parse_arguments_from_tool_call +from langchain_ollama.chat_models import ( + ChatOllama, + _parse_arguments_from_tool_call, + _parse_json_string, +) MODEL_NAME = "llama3.1" @@ -49,13 +54,11 @@ def test_arbitrary_roles_accepted_in_chatmessages( monkeypatch: pytest.MonkeyPatch, ) -> None: monkeypatch.setattr(Client, "stream", _mock_httpx_client_stream) - llm = ChatOllama( model=MODEL_NAME, verbose=True, format=None, ) - messages = [ ChatMessage( role="somerandomrole", @@ -64,7 +67,6 @@ def test_arbitrary_roles_accepted_in_chatmessages( ChatMessage(role="control", content="thinking"), ChatMessage(role="user", content="What is the meaning of life?"), ] - llm.invoke(messages) @@ -83,3 +85,58 @@ def test_validate_model_on_init(mock_validate_model: Any) -> None: # Test that validate_model is NOT called by default ChatOllama(model=MODEL_NAME) mock_validate_model.assert_not_called() + + +# Define a dummy raw_tool_call for the function signature +dummy_raw_tool_call = { + "function": {"name": "test_func", "arguments": ""}, +} + + +# --- Regression tests for tool-call argument parsing (see #30910) --- + + +@pytest.mark.parametrize( + "input_string, expected_output", + [ + # Case 1: Standard double-quoted JSON + ('{"key": "value", "number": 123}', {"key": "value", "number": 123}), + # Case 2: Single-quoted string (the original bug) + ("{'key': 'value', 'number': 123}", {"key": "value", "number": 123}), + # Case 3: String with an internal apostrophe + ('{"text": "It\'s a great test!"}', {"text": "It's a great test!"}), + # Case 4: Mixed quotes that ast can handle + ("{'text': \"It's a great test!\"}", {"text": "It's a great test!"}), + ], +) +def test_parse_json_string_success_cases( + input_string: str, expected_output: Any +) -> None: + """Tests that _parse_json_string correctly parses valid and fixable strings.""" + raw_tool_call = {"function": {"name": "test_func", "arguments": input_string}} + result = _parse_json_string(input_string, raw_tool_call=raw_tool_call, skip=False) + assert result == expected_output + + +def test_parse_json_string_failure_case_raises_exception() -> None: + """Tests that _parse_json_string raises an exception for truly malformed strings.""" + malformed_string = "{'key': 'value',,}" + raw_tool_call = {"function": {"name": "test_func", "arguments": malformed_string}} + with pytest.raises(OutputParserException): + _parse_json_string( + malformed_string, + raw_tool_call=raw_tool_call, + skip=False, + ) + + +def test_parse_json_string_skip_returns_input_on_failure() -> None: + """Tests that skip=True returns the original string on parse failure.""" + malformed_string = "{'not': valid,,,}" + raw_tool_call = {"function": {"name": "test_func", "arguments": malformed_string}} + result = _parse_json_string( + malformed_string, + raw_tool_call=raw_tool_call, + skip=True, + ) + assert result == malformed_string