Add glm 4.6 bis - bis (#1046)

Co-authored-by: openhands <[email protected]>

Author: simonrosenberg

openhands-sdk/openhands/sdk/agent/agent.py

@@ -4,6 +4,7 @@
 
 import openhands.sdk.security.risk as risk
 from openhands.sdk.agent.base import AgentBase
+from openhands.sdk.agent.utils import fix_malformed_tool_arguments
 from openhands.sdk.context.view import View
 from openhands.sdk.conversation import (
     ConversationCallbackType,
@@ -351,6 +352,9 @@ def _get_action_event(
         try:
             arguments = json.loads(tool_call.arguments)
 
+            # Fix malformed arguments (e.g., JSON strings for list/dict fields)
+            arguments = fix_malformed_tool_arguments(arguments, tool.action_type)
+
             # if the tool has a security_risk field (when security analyzer is set),
             # pop it out as it's not part of the tool's action schema
             if (

openhands-sdk/openhands/sdk/agent/utils.py

@@ -0,0 +1,98 @@
+import json
+import types
+from typing import Annotated, Any, Union, get_args, get_origin
+
+from openhands.sdk.tool import Action
+
+
+def fix_malformed_tool_arguments(
+    arguments: dict[str, Any], action_type: type[Action]
+) -> dict[str, Any]:
+    """Fix malformed tool arguments by decoding JSON strings for list/dict fields.
+
+    This function handles cases where certain LLMs (such as GLM 4.6) incorrectly
+    encode array/object parameters as JSON strings when using native function calling.
+
+    Example raw LLM output from GLM 4.6:
+    {
+        "role": "assistant",
+        "content": "I'll view the file for you.",
+        "tool_calls": [{
+            "id": "call_ef8e",
+            "type": "function",
+            "function": {
+                "name": "str_replace_editor",
+                "arguments": '{
+                    "command": "view",
+                    "path": "/tmp/test.txt",
+                    "view_range": "[1, 5]"
+                }'
+            }
+        }]
+    }
+
+    Expected output: `"view_range" : [1, 5]`
+
+    Note: The arguments field is a JSON string. When decoded, view_range is
+    incorrectly a string "[1, 5]" instead of the proper array [1, 5].
+    This function automatically fixes this by detecting that view_range
+    expects a list type and decoding the JSON string to get the actual array.
+
+    Args:
+        arguments: The parsed arguments dict from json.loads(tool_call.arguments).
+        action_type: The action type that defines the expected schema.
+
+    Returns:
+        The arguments dict with JSON strings decoded where appropriate.
+    """
+    if not isinstance(arguments, dict):
+        return arguments
+
+    fixed_arguments = arguments.copy()
+
+    # Use model_fields to properly handle aliases and inherited fields
+    for field_name, field_info in action_type.model_fields.items():
+        # Check both the field name and its alias (if any)
+        data_key = field_info.alias if field_info.alias else field_name
+        if data_key not in fixed_arguments:
+            continue
+
+        value = fixed_arguments[data_key]
+        # Skip if value is not a string
+        if not isinstance(value, str):
+            continue
+
+        expected_type = field_info.annotation
+
+        # Unwrap Annotated types - only the first arg is the actual type
+        if get_origin(expected_type) is Annotated:
+            type_args = get_args(expected_type)
+            expected_type = type_args[0] if type_args else expected_type
+
+        # Get the origin of the expected type (e.g., list from list[str])
+        origin = get_origin(expected_type)
+
+        # For Union types, we need to check all union members
+        if origin is Union or origin is types.UnionType:
+            # For Union types, check each union member
+            type_args = get_args(expected_type)
+            expected_origins = [get_origin(arg) or arg for arg in type_args]
+        else:
+            # For non-Union types, just check the origin
+            expected_origins = [origin or expected_type]
+
+        # Check if any of the expected types is list or dict
+        if any(exp in (list, dict) for exp in expected_origins):
+            # Try to parse the string as JSON
+            try:
+                parsed_value = json.loads(value)
+                # json.loads() returns dict, list, str, int, float, bool, or None
+                # Only use parsed value if it matches expected collection types
+                if isinstance(parsed_value, (list, dict)):
+                    fixed_arguments[data_key] = parsed_value
+            except (json.JSONDecodeError, ValueError):
+                # If parsing fails, leave the original value
+                # Pydantic will raise validation error if needed
+                pass
+
+    return fixed_arguments

tests/sdk/agent/test_fix_malformed_tool_arguments.py

@@ -0,0 +1,274 @@
+"""Tests for fix_malformed_tool_arguments helper function.
+
+This module tests the fix_malformed_tool_arguments helper that automatically
+decodes JSON strings for list/dict fields. This handles cases where LLMs
+(like GLM-4) return array/object values as JSON strings instead of native
+JSON arrays/objects.
+"""
+
+from typing import Annotated
+
+import pytest
+from pydantic import Field, ValidationError
+
+from openhands.sdk.agent.utils import fix_malformed_tool_arguments
+from openhands.sdk.tool.schema import Action
+
+
+class JsonDecodingTestAction(Action):
+    """Test action with list and dict fields."""
+
+    items: list[str] = Field(description="A list of items")
+    config: dict[str, int] = Field(description="Configuration dictionary")
+    name: str = Field(description="A regular string field")
+
+
+class JsonDecodingAnnotatedAction(Action):
+    """Test action with Annotated types."""
+
+    items: Annotated[list[str], Field(description="A list of items")]
+    config: Annotated[dict[str, int], Field(description="Configuration dictionary")]
+
+
+class JsonDecodingAliasAction(Action):
+    """Test action with field aliases."""
+
+    my_list: list[int] = Field(alias="myList", description="A list with alias")
+    my_dict: dict[str, str] = Field(alias="myDict", description="A dict with alias")
+
+
+class JsonDecodingOptionalAction(Action):
+    """Test action with optional list/dict fields."""
+
+    items: list[str] | None = Field(default=None, description="Optional list")
+    config: dict[str, int] | None = Field(default=None, description="Optional dict")
+
+
+def test_decode_json_string_list():
+    """Test that JSON string lists are decoded to native lists."""
+    data = {
+        "items": '["a", "b", "c"]',
+        "config": '{"x": 1, "y": 2}',
+        "name": "test",
+    }
+    fixed_data = fix_malformed_tool_arguments(data, JsonDecodingTestAction)
+    action = JsonDecodingTestAction.model_validate(fixed_data)
+
+    assert action.items == ["a", "b", "c"]
+    assert action.config == {"x": 1, "y": 2}
+    assert action.name == "test"
+
+
+def test_decode_json_string_dict():
+    """Test that JSON string dicts are decoded to native dicts."""
+    data = {
+        "items": '["item1", "item2"]',
+        "config": '{"key1": 10, "key2": 20}',
+        "name": "dict_test",
+    }
+    fixed_data = fix_malformed_tool_arguments(data, JsonDecodingTestAction)
+    action = JsonDecodingTestAction.model_validate(fixed_data)
+
+    assert action.items == ["item1", "item2"]
+    assert action.config == {"key1": 10, "key2": 20}
+    assert action.name == "dict_test"
+
+
+def test_native_list_dict_passthrough():
+    """Test that native lists and dicts pass through unchanged."""
+    data = {
+        "items": ["direct", "list"],
+        "config": {"direct": 42},
+        "name": "native_test",
+    }
+    fixed_data = fix_malformed_tool_arguments(data, JsonDecodingTestAction)
+    action = JsonDecodingTestAction.model_validate(fixed_data)
+
+    assert action.items == ["direct", "list"]
+    assert action.config == {"direct": 42}
+    assert action.name == "native_test"
+
+
+def test_regular_string_not_decoded():
+    """Test that regular string fields are not affected by JSON decoding."""
+    data = {
+        "items": "[]",
+        "config": "{}",
+        "name": "this is not json but a regular string",
+    }
+    fixed_data = fix_malformed_tool_arguments(data, JsonDecodingTestAction)
+    action = JsonDecodingTestAction.model_validate(fixed_data)
+
+    assert action.items == []
+    assert action.config == {}
+    # Regular string field should NOT be decoded
+    assert action.name == "this is not json but a regular string"
+
+
+def test_annotated_types():
+    """Test that Annotated types are properly handled."""
+    data = {
+        "items": '["x", "y", "z"]',
+        "config": '{"a": 1, "b": 2}',
+    }
+    fixed_data = fix_malformed_tool_arguments(data, JsonDecodingAnnotatedAction)
+    action = JsonDecodingAnnotatedAction.model_validate(fixed_data)
+
+    assert action.items == ["x", "y", "z"]
+    assert action.config == {"a": 1, "b": 2}
+
+
+def test_field_aliases():
+    """Test that field aliases are properly handled."""
+    data = {
+        "myList": "[1, 2, 3]",
+        "myDict": '{"key": "value"}',
+    }
+    fixed_data = fix_malformed_tool_arguments(data, JsonDecodingAliasAction)
+    action = JsonDecodingAliasAction.model_validate(fixed_data)
+
+    assert action.my_list == [1, 2, 3]
+    assert action.my_dict == {"key": "value"}
+
+
+def test_optional_fields_with_json_strings():
+    """Test that optional list/dict fields work with JSON strings."""
+    data = {
+        "items": '["opt1", "opt2"]',
+        "config": '{"opt": 99}',
+    }
+    fixed_data = fix_malformed_tool_arguments(data, JsonDecodingOptionalAction)
+    action = JsonDecodingOptionalAction.model_validate(fixed_data)
+
+    assert action.items == ["opt1", "opt2"]
+    assert action.config == {"opt": 99}
+
+
+def test_optional_fields_with_none():
+    """Test that optional fields can be None."""
+    data = {}
+    fixed_data = fix_malformed_tool_arguments(data, JsonDecodingOptionalAction)
+    action = JsonDecodingOptionalAction.model_validate(fixed_data)
+
+    assert action.items is None
+    assert action.config is None
+
+
+def test_optional_fields_with_native_values():
+    """Test that optional fields work with native values."""
+    data = {
+        "items": ["native1", "native2"],
+        "config": {"native": 100},
+    }
+    fixed_data = fix_malformed_tool_arguments(data, JsonDecodingOptionalAction)
+    action = JsonDecodingOptionalAction.model_validate(fixed_data)
+
+    assert action.items == ["native1", "native2"]
+    assert action.config == {"native": 100}
+
+
+def test_invalid_json_string_rejected():
+    """Test that invalid JSON strings are rejected with validation error."""
+    data = {
+        "items": "not valid json",
+        "config": "{}",
+        "name": "test",
+    }
+    fixed_data = fix_malformed_tool_arguments(data, JsonDecodingTestAction)
+
+    with pytest.raises(ValidationError) as exc_info:
+        JsonDecodingTestAction.model_validate(fixed_data)
+
+    # Should fail validation because "not valid json" can't be parsed as list
+    assert "items" in str(exc_info.value)
+
+
+def test_json_string_with_wrong_type_rejected():
+    """Test that JSON strings with wrong types are rejected."""
+    # Field expects list but JSON string contains dict
+    data = {
+        "items": '{"not": "a list"}',
+        "config": "{}",
+        "name": "test",
+    }
+    fixed_data = fix_malformed_tool_arguments(data, JsonDecodingTestAction)
+
+    with pytest.raises(ValidationError) as exc_info:
+        JsonDecodingTestAction.model_validate(fixed_data)
+
+    assert "items" in str(exc_info.value)
+
+
+def test_nested_structures():
+    """Test that nested lists and dicts in JSON strings work."""
+
+    class NestedAction(Action):
+        nested_list: list[list[int]] = Field(description="Nested list")
+        nested_dict: dict[str, dict[str, str]] = Field(description="Nested dict")
+
+    data = {
+        "nested_list": "[[1, 2], [3, 4]]",
+        "nested_dict": '{"outer": {"inner": "value"}}',
+    }
+    fixed_data = fix_malformed_tool_arguments(data, NestedAction)
+    action = NestedAction.model_validate(fixed_data)
+
+    assert action.nested_list == [[1, 2], [3, 4]]
+    assert action.nested_dict == {"outer": {"inner": "value"}}
+
+
+def test_empty_collections():
+    """Test that empty lists and dicts work."""
+    data = {
+        "items": "[]",
+        "config": "{}",
+        "name": "empty",
+    }
+    fixed_data = fix_malformed_tool_arguments(data, JsonDecodingTestAction)
+    action = JsonDecodingTestAction.model_validate(fixed_data)
+
+    assert action.items == []
+    assert action.config == {}
+
+
+def test_mixed_native_and_json_strings():
+    """Test mixing native values and JSON strings in same model."""
+    data = {
+        "items": ["native", "list"],  # Native list
+        "config": '{"from": 1, "json": 2}',  # JSON string
+        "name": "mixed",
+    }
+    fixed_data = fix_malformed_tool_arguments(data, JsonDecodingTestAction)
+    action = JsonDecodingTestAction.model_validate(fixed_data)
+
+    assert action.items == ["native", "list"]
+    assert action.config == {"from": 1, "json": 2}
+    assert action.name == "mixed"
+
+
+def test_unicode_in_json_strings():
+    """Test that unicode characters in JSON strings are handled correctly."""
+    data = {
+        "items": '["hello", "世界", "🌍"]',
+        "config": '{"greeting": 1, "你好": 2}',
+        "name": "unicode",
+    }
+    fixed_data = fix_malformed_tool_arguments(data, JsonDecodingTestAction)
+    action = JsonDecodingTestAction.model_validate(fixed_data)
+
+    assert action.items == ["hello", "世界", "🌍"]
+    assert action.config == {"greeting": 1, "你好": 2}
+
+
+def test_whitespace_in_json_strings():
+    """Test that JSON strings with extra whitespace work."""
+    data = {
+        "items": '  [ "a" , "b" , "c" ]  ',
+        "config": '  { "x" : 1 , "y" : 2 }  ',
+        "name": "whitespace",
+    }
+    fixed_data = fix_malformed_tool_arguments(data, JsonDecodingTestAction)
+    action = JsonDecodingTestAction.model_validate(fixed_data)
+
+    assert action.items == ["a", "b", "c"]
+    assert action.config == {"x": 1, "y": 2}