mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
Port from cline/cline#11803: recursively normalize JSON-string tool args by schema (#52220)
coerce_tool_args only repaired the outermost value, so JSON-encoded
*elements* of array properties (and nested object sub-fields) were left
as strings. Three core tools have array<object> schemas — todo.todos,
delegate_task.tasks, memory.operations — so a model emitting
{"todos": ["{...}"]} would pass raw JSON strings into the tool and fail
downstream on item["id"]/item["goal"] access.
Adds a schema-guided recursive pass (_normalize_json_strings_for_schema)
that parses JSON-string array items and nested object fields only when
the matching schema position expects an array/object, preserving
legitimate JSON-looking string fields (type: string).
Adapted from cline/cline#11803 to hermes-agent's existing coercion layer.
This commit is contained in:
parent
b3b1e58ad6
commit
8a04b516a8
2 changed files with 249 additions and 0 deletions
112
model_tools.py
112
model_tools.py
|
|
@ -718,16 +718,128 @@ def coerce_tool_args(tool_name: str, args: Dict[str, Any]) -> Dict[str, Any]:
|
|||
continue
|
||||
|
||||
if not isinstance(value, str):
|
||||
# Recurse into already-native containers so JSON-encoded
|
||||
# *elements* (array items) and *sub-fields* (nested object
|
||||
# properties) get normalized too — e.g. ``todos: ['{"id":...}']``
|
||||
# or ``tasks: [{"goal": "..."}]`` where an element was emitted as
|
||||
# a JSON string. The top-level coercion above only repairs the
|
||||
# outermost value.
|
||||
if expected == "array" and isinstance(value, (list, tuple)):
|
||||
args[key] = _normalize_json_strings_for_schema(value, prop_schema)
|
||||
elif expected == "object" and isinstance(value, dict):
|
||||
args[key] = _normalize_json_strings_for_schema(value, prop_schema)
|
||||
continue
|
||||
if not expected and not _schema_allows_null(prop_schema):
|
||||
continue
|
||||
coerced = _coerce_value(value, expected, schema=prop_schema)
|
||||
if coerced is not value:
|
||||
args[key] = coerced
|
||||
# If we just JSON-parsed a string into a container, recurse so
|
||||
# nested JSON-encoded elements/fields get normalized as well.
|
||||
if isinstance(coerced, (list, tuple, dict)):
|
||||
args[key] = _normalize_json_strings_for_schema(coerced, prop_schema)
|
||||
|
||||
return args
|
||||
|
||||
|
||||
def _schema_accepts_kind(schema: Any, kind: str) -> bool:
|
||||
"""Return True when *schema* permits a value of JSON type *kind*.
|
||||
|
||||
Looks at ``type`` (string or list) and recurses through
|
||||
``anyOf``/``oneOf``/``allOf`` branches — matching the JSON-Schema shapes
|
||||
open-weight models emit against. ``kind`` is ``"array"`` or ``"object"``.
|
||||
"""
|
||||
if not isinstance(schema, dict):
|
||||
return False
|
||||
t = schema.get("type")
|
||||
if t == kind or (isinstance(t, list) and kind in t):
|
||||
return True
|
||||
for union_key in ("anyOf", "oneOf", "allOf"):
|
||||
branches = schema.get(union_key)
|
||||
if isinstance(branches, list) and any(
|
||||
_schema_accepts_kind(b, kind) for b in branches
|
||||
):
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def _normalize_json_strings_for_schema(value: Any, schema: Any) -> Any:
|
||||
"""Recursively parse JSON-encoded string values that a schema expects to
|
||||
be arrays or objects, including nested array items and object properties.
|
||||
|
||||
Open-weight models (DeepSeek, Qwen, GLM, and others) sometimes emit a
|
||||
structured field — or an *element* of a structured field — as a
|
||||
JSON-encoded string instead of a native value. The top-level
|
||||
:func:`coerce_tool_args` pass repairs the outermost value; this helper
|
||||
walks the rest of the tree so cases like::
|
||||
|
||||
{"todos": ["{\\"id\\": \\"1\\", \\"content\\": \\"x\\"}"]}
|
||||
|
||||
(a list whose elements are JSON strings) and nested object sub-fields are
|
||||
repaired too. Parsing is schema-guided: a string is only parsed when the
|
||||
matching schema position actually expects an array or object, so
|
||||
legitimate JSON-looking string fields (``type: string``) are preserved.
|
||||
|
||||
Ported from cline/cline#11803, adapted to hermes-agent's coercion layer.
|
||||
Returns the original value object when nothing changed (identity preserved
|
||||
so callers can cheaply detect no-ops).
|
||||
"""
|
||||
if not isinstance(schema, dict):
|
||||
return value
|
||||
|
||||
# Parse a JSON-encoded string into the container the schema expects.
|
||||
if isinstance(value, str):
|
||||
trimmed = value.strip()
|
||||
expects_array = _schema_accepts_kind(schema, "array")
|
||||
expects_object = _schema_accepts_kind(schema, "object")
|
||||
if (expects_array and trimmed.startswith("[")) or (
|
||||
expects_object and trimmed.startswith("{")
|
||||
):
|
||||
try:
|
||||
parsed = json.loads(trimmed)
|
||||
except (ValueError, TypeError):
|
||||
return value
|
||||
if isinstance(parsed, list) and expects_array:
|
||||
value = parsed
|
||||
elif isinstance(parsed, dict) and expects_object:
|
||||
value = parsed
|
||||
else:
|
||||
return value
|
||||
else:
|
||||
return value
|
||||
|
||||
# Recurse into list items using the ``items`` schema.
|
||||
if isinstance(value, list):
|
||||
items_schema = schema.get("items")
|
||||
if not isinstance(items_schema, dict):
|
||||
return value
|
||||
changed = False
|
||||
out = []
|
||||
for item in value:
|
||||
nxt = _normalize_json_strings_for_schema(item, items_schema)
|
||||
changed = changed or (nxt is not item)
|
||||
out.append(nxt)
|
||||
return out if changed else value
|
||||
|
||||
# Recurse into object properties using each property's schema.
|
||||
if isinstance(value, dict):
|
||||
props = schema.get("properties")
|
||||
if not isinstance(props, dict):
|
||||
return value
|
||||
changed = False
|
||||
out = dict(value)
|
||||
for k, prop_schema in props.items():
|
||||
if k not in value or not isinstance(prop_schema, dict):
|
||||
continue
|
||||
nxt = _normalize_json_strings_for_schema(value[k], prop_schema)
|
||||
if nxt is not value[k]:
|
||||
out[k] = nxt
|
||||
changed = True
|
||||
return out if changed else value
|
||||
|
||||
return value
|
||||
|
||||
|
||||
def _coerce_value(value: str, expected_type, schema: dict | None = None):
|
||||
"""Attempt to coerce a string *value* to *expected_type*.
|
||||
|
||||
|
|
|
|||
|
|
@ -13,6 +13,8 @@ from model_tools import (
|
|||
_coerce_value,
|
||||
_coerce_number,
|
||||
_coerce_boolean,
|
||||
_schema_accepts_kind,
|
||||
_normalize_json_strings_for_schema,
|
||||
)
|
||||
|
||||
|
||||
|
|
@ -408,3 +410,138 @@ class TestCoerceToolArgs:
|
|||
assert isinstance(result["offset"], int)
|
||||
assert result["limit"] == 100
|
||||
assert isinstance(result["limit"], int)
|
||||
|
||||
|
||||
# ── Schema-guided nested JSON-string normalization (cline/cline#11803) ─────
|
||||
|
||||
|
||||
class TestSchemaAcceptsKind:
|
||||
"""Unit tests for _schema_accepts_kind."""
|
||||
|
||||
def test_plain_type(self):
|
||||
assert _schema_accepts_kind({"type": "array"}, "array") is True
|
||||
assert _schema_accepts_kind({"type": "object"}, "object") is True
|
||||
assert _schema_accepts_kind({"type": "string"}, "array") is False
|
||||
|
||||
def test_type_list(self):
|
||||
assert _schema_accepts_kind({"type": ["array", "null"]}, "array") is True
|
||||
assert _schema_accepts_kind({"type": ["string", "null"]}, "array") is False
|
||||
|
||||
def test_union_branches(self):
|
||||
schema = {"anyOf": [{"type": "string"}, {"type": "array"}]}
|
||||
assert _schema_accepts_kind(schema, "array") is True
|
||||
assert _schema_accepts_kind(schema, "object") is False
|
||||
|
||||
def test_non_dict(self):
|
||||
assert _schema_accepts_kind(None, "array") is False
|
||||
|
||||
|
||||
class TestNormalizeJsonStringsForSchema:
|
||||
"""Unit tests for _normalize_json_strings_for_schema (the recursive pass)."""
|
||||
|
||||
def test_parses_json_string_array_when_schema_expects_array(self):
|
||||
schema = {"type": "array", "items": {"type": "string"}}
|
||||
out = _normalize_json_strings_for_schema('["git status", "bun test"]', schema)
|
||||
assert out == ["git status", "bun test"]
|
||||
|
||||
def test_preserves_json_looking_string_when_schema_expects_string(self):
|
||||
schema = {"type": "string"}
|
||||
text = '{"keep": "as text"}'
|
||||
assert _normalize_json_strings_for_schema(text, schema) == text
|
||||
|
||||
def test_normalizes_array_item_json_strings(self):
|
||||
schema = {
|
||||
"type": "array",
|
||||
"items": {"type": "object", "properties": {"id": {"type": "string"}}},
|
||||
}
|
||||
out = _normalize_json_strings_for_schema(['{"id": "1"}', '{"id": "2"}'], schema)
|
||||
assert out == [{"id": "1"}, {"id": "2"}]
|
||||
|
||||
def test_normalizes_nested_object_field(self):
|
||||
schema = {
|
||||
"type": "object",
|
||||
"properties": {"cfg": {"type": "object", "properties": {"k": {"type": "string"}}}},
|
||||
}
|
||||
out = _normalize_json_strings_for_schema({"cfg": '{"k": "v"}'}, schema)
|
||||
assert out == {"cfg": {"k": "v"}}
|
||||
|
||||
def test_native_list_preserved_identity(self):
|
||||
schema = {"type": "array", "items": {"type": "object", "properties": {}}}
|
||||
value = [{"id": "1"}]
|
||||
# Nothing to change — same object back (no-op identity preserved).
|
||||
assert _normalize_json_strings_for_schema(value, schema) is value
|
||||
|
||||
def test_non_dict_schema_returns_value(self):
|
||||
assert _normalize_json_strings_for_schema("x", None) == "x"
|
||||
|
||||
|
||||
class TestCoerceToolArgsNested:
|
||||
"""Integration: nested JSON-string elements/fields are normalized via the
|
||||
registry schema, while legitimate string fields are preserved."""
|
||||
|
||||
def _array_of_objects_schema(self):
|
||||
return {
|
||||
"name": "test_tool",
|
||||
"description": "test",
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"items": {
|
||||
"type": "array",
|
||||
"items": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"id": {"type": "string"},
|
||||
"content": {"type": "string"},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
def test_array_elements_as_json_strings_are_parsed(self):
|
||||
schema = self._array_of_objects_schema()
|
||||
with patch("model_tools.registry.get_schema", return_value=schema):
|
||||
args = {"items": ['{"id": "1", "content": "x"}']}
|
||||
result = coerce_tool_args("test_tool", args)
|
||||
assert result["items"] == [{"id": "1", "content": "x"}]
|
||||
|
||||
def test_mixed_native_and_string_elements(self):
|
||||
schema = self._array_of_objects_schema()
|
||||
with patch("model_tools.registry.get_schema", return_value=schema):
|
||||
args = {"items": [{"id": "1", "content": "a"}, '{"id": "2", "content": "b"}']}
|
||||
result = coerce_tool_args("test_tool", args)
|
||||
assert result["items"] == [
|
||||
{"id": "1", "content": "a"},
|
||||
{"id": "2", "content": "b"},
|
||||
]
|
||||
|
||||
def test_string_subfield_with_json_content_preserved(self):
|
||||
"""A string-typed sub-field whose value looks like JSON must NOT be parsed."""
|
||||
schema = self._array_of_objects_schema()
|
||||
with patch("model_tools.registry.get_schema", return_value=schema):
|
||||
args = {"items": [{"id": "1", "content": '{"not": "parsed"}'}]}
|
||||
result = coerce_tool_args("test_tool", args)
|
||||
assert result["items"][0]["content"] == '{"not": "parsed"}'
|
||||
|
||||
def test_whole_array_string_still_works(self):
|
||||
schema = self._array_of_objects_schema()
|
||||
with patch("model_tools.registry.get_schema", return_value=schema):
|
||||
args = {"items": '[{"id": "1", "content": "x"}]'}
|
||||
result = coerce_tool_args("test_tool", args)
|
||||
assert result["items"] == [{"id": "1", "content": "x"}]
|
||||
|
||||
def test_native_array_preserved(self):
|
||||
schema = self._array_of_objects_schema()
|
||||
with patch("model_tools.registry.get_schema", return_value=schema):
|
||||
args = {"items": [{"id": "1", "content": "keep"}]}
|
||||
result = coerce_tool_args("test_tool", args)
|
||||
assert result["items"] == [{"id": "1", "content": "keep"}]
|
||||
|
||||
def test_real_todo_schema_element_strings(self):
|
||||
"""Against the real todo schema from the registry."""
|
||||
import json as _json
|
||||
args = {"todos": [_json.dumps({"id": "1", "content": "x", "status": "pending"})]}
|
||||
result = coerce_tool_args("todo", args)
|
||||
assert result["todos"][0] == {"id": "1", "content": "x", "status": "pending"}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue