Skip to content

Commit 1e78c81

Browse files
committed
review comments
Signed-off-by: Akihiko Kuroda <akihikokuroda2020@gmail.com>
1 parent c2963fa commit 1e78c81

3 files changed

Lines changed: 203 additions & 36 deletions

File tree

docs/examples/agents/react/react_using_mellea.py

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,15 +41,15 @@ async def main():
4141
print(out)
4242

4343
# Version that looks up info and formats the final response as an Email object.
44-
out, _ = await react(
45-
goal="Write an email about the Mellea python library to Jake with the subject 'cool library'.",
46-
context=ChatContext(),
47-
backend=m.backend,
48-
tools=[search_tool],
49-
format=Email,
50-
loop_budget=20,
51-
)
52-
print(out)
44+
# out, _ = await react(
45+
# goal="Write an email about the Mellea python library to Jake with the subject 'cool library'.",
46+
# context=ChatContext(),
47+
# backend=m.backend,
48+
# tools=[search_tool],
49+
# format=Email,
50+
# loop_budget=20,
51+
# )
52+
# print(out)
5353

5454

5555
asyncio.run(main())

mellea/backends/tools.py

Lines changed: 26 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
validating/coercing tool arguments against the tool's JSON schema using Pydantic.
88
"""
99

10+
import copy
1011
import inspect
1112
import json
1213
import re
@@ -899,6 +900,30 @@ def _parse_docstring(doc_string: str | None) -> dict[str, str]:
899900
return parsed_docstring
900901

901902

903+
def _resolve_ref(ref_path: str, defs: dict) -> dict:
904+
"""Resolve a $ref path like '#/$defs/Email' to the actual schema."""
905+
if ref_path.startswith("#/$defs/"):
906+
def_name = ref_path.split("/")[-1]
907+
return defs.get(def_name, {})
908+
elif ref_path.startswith("#/definitions/"):
909+
def_name = ref_path.split("/")[-1]
910+
return defs.get(def_name, {})
911+
return {}
912+
913+
914+
def _is_complex_anyof(v: dict) -> bool:
915+
"""Check if anyOf contains complex types (refs or nested objects)."""
916+
any_of_schemas = v.get("anyOf", [])
917+
for sub_schema in any_of_schemas:
918+
# Skip null types - they just indicate optionality
919+
if sub_schema.get("type") == "null":
920+
continue
921+
# Check for references or nested properties
922+
if "$ref" in sub_schema or "properties" in sub_schema:
923+
return True
924+
return False
925+
926+
902927
# https://github.com/ollama/ollama-python/blob/60e7b2f9ce710eeb57ef2986c46ea612ae7516af/ollama/_utils.py#L56-L90
903928
def convert_function_to_ollama_tool(
904929
func: Callable, name: str | None = None
@@ -915,8 +940,6 @@ def convert_function_to_ollama_tool(
915940
An ``OllamaTool`` instance representing the function as an OpenAI-compatible
916941
tool schema.
917942
"""
918-
import copy
919-
920943
doc_string_hash = str(hash(inspect.getdoc(func)))
921944
parsed_docstring = _parse_docstring(inspect.getdoc(func))
922945
schema = type(
@@ -932,37 +955,13 @@ def convert_function_to_ollama_tool(
932955
},
933956
).model_json_schema() # type: ignore
934957

935-
# Helper to resolve $ref references
936-
def resolve_ref(ref_path: str, defs: dict) -> dict:
937-
"""Resolve a $ref path like '#/$defs/Email' to the actual schema."""
938-
if ref_path.startswith("#/$defs/"):
939-
def_name = ref_path.split("/")[-1]
940-
return defs.get(def_name, {})
941-
elif ref_path.startswith("#/definitions/"):
942-
def_name = ref_path.split("/")[-1]
943-
return defs.get(def_name, {})
944-
return {}
945-
946-
# Helper to check if anyOf represents a complex union (not just Optional[primitive])
947-
def _is_complex_anyof(v: dict) -> bool:
948-
"""Check if anyOf contains complex types (refs or nested objects)."""
949-
any_of_schemas = v.get("anyOf", [])
950-
for sub_schema in any_of_schemas:
951-
# Skip null types - they just indicate optionality
952-
if sub_schema.get("type") == "null":
953-
continue
954-
# Check for references or nested properties
955-
if "$ref" in sub_schema or "properties" in sub_schema:
956-
return True
957-
return False
958-
959958
defs = schema.get("$defs", schema.get("definitions", {}))
960959

961960
for k, v in schema.get("properties", {}).items():
962961
# Check if this property has a $ref (reference to a definition)
963962
if "$ref" in v:
964963
# Resolve the reference and inline it
965-
ref_schema = resolve_ref(v["$ref"], defs)
964+
ref_schema = _resolve_ref(v["$ref"], defs)
966965
if ref_schema:
967966
# Inline the referenced schema (deep copy to avoid mutations)
968967
inlined = copy.deepcopy(ref_schema)
Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
"""Tests for schema helper functions in tools.py.
2+
3+
Tests for _resolve_ref and _is_complex_anyof functions that handle
4+
JSON schema resolution and complex type detection.
5+
"""
6+
7+
import pytest
8+
9+
from mellea.backends.tools import _is_complex_anyof, _resolve_ref
10+
11+
12+
class TestResolveRef:
13+
"""Tests for the _resolve_ref helper function."""
14+
15+
def test_resolve_defs_style_ref(self):
16+
"""Test resolving #/$defs/ style references."""
17+
defs = {
18+
"Email": {
19+
"type": "object",
20+
"properties": {"to": {"type": "string"}, "subject": {"type": "string"}},
21+
}
22+
}
23+
result = _resolve_ref("#/$defs/Email", defs)
24+
assert result == defs["Email"]
25+
26+
def test_resolve_definitions_style_ref(self):
27+
"""Test resolving #/definitions/ style references."""
28+
defs = {
29+
"Address": {
30+
"type": "object",
31+
"properties": {
32+
"street": {"type": "string"},
33+
"city": {"type": "string"},
34+
},
35+
}
36+
}
37+
result = _resolve_ref("#/definitions/Address", defs)
38+
assert result == defs["Address"]
39+
40+
def test_resolve_missing_ref_returns_empty_dict(self):
41+
"""Test that resolving a non-existent ref returns empty dict."""
42+
defs = {"Email": {"type": "object"}}
43+
result = _resolve_ref("#/$defs/NotFound", defs)
44+
assert result == {}
45+
46+
def test_resolve_invalid_ref_format_returns_empty_dict(self):
47+
"""Test that invalid ref format returns empty dict."""
48+
defs = {"Email": {"type": "object"}}
49+
result = _resolve_ref("#/invalid/Email", defs)
50+
assert result == {}
51+
52+
def test_resolve_with_empty_defs(self):
53+
"""Test resolving against empty defs dict."""
54+
result = _resolve_ref("#/$defs/Email", {})
55+
assert result == {}
56+
57+
def test_resolve_nested_ref_name(self):
58+
"""Test resolving refs with nested-like names."""
59+
defs = {
60+
"User_v2": {"type": "object", "properties": {"id": {"type": "integer"}}}
61+
}
62+
result = _resolve_ref("#/$defs/User_v2", defs)
63+
assert result == defs["User_v2"]
64+
65+
66+
class TestIsComplexAnyof:
67+
"""Tests for the _is_complex_anyof helper function."""
68+
69+
def test_simple_optional_primitive_not_complex(self):
70+
"""Test that Optional[str] (anyOf with null and string) is not complex."""
71+
schema = {"anyOf": [{"type": "string"}, {"type": "null"}]}
72+
assert _is_complex_anyof(schema) is False
73+
74+
def test_optional_int_not_complex(self):
75+
"""Test that Optional[int] is not complex."""
76+
schema = {"anyOf": [{"type": "integer"}, {"type": "null"}]}
77+
assert _is_complex_anyof(schema) is False
78+
79+
def test_anyof_with_ref_is_complex(self):
80+
"""Test that anyOf with a $ref is complex."""
81+
schema = {"anyOf": [{"$ref": "#/$defs/Email"}, {"type": "null"}]}
82+
assert _is_complex_anyof(schema) is True
83+
84+
def test_anyof_with_nested_object_is_complex(self):
85+
"""Test that anyOf with nested object (properties) is complex."""
86+
schema = {
87+
"anyOf": [
88+
{
89+
"type": "object",
90+
"properties": {
91+
"name": {"type": "string"},
92+
"age": {"type": "integer"},
93+
},
94+
},
95+
{"type": "null"},
96+
]
97+
}
98+
assert _is_complex_anyof(schema) is True
99+
100+
def test_anyof_with_multiple_types_no_ref_not_complex(self):
101+
"""Test that anyOf with multiple primitives (no ref/props) is not complex."""
102+
schema = {"anyOf": [{"type": "string"}, {"type": "integer"}, {"type": "null"}]}
103+
assert _is_complex_anyof(schema) is False
104+
105+
def test_anyof_with_ref_and_primitive_is_complex(self):
106+
"""Test that anyOf with both ref and primitive is complex."""
107+
schema = {"anyOf": [{"$ref": "#/$defs/Email"}, {"type": "string"}]}
108+
assert _is_complex_anyof(schema) is True
109+
110+
def test_anyof_with_only_null_not_complex(self):
111+
"""Test that anyOf with only null type is not complex."""
112+
schema = {"anyOf": [{"type": "null"}]}
113+
assert _is_complex_anyof(schema) is False
114+
115+
def test_empty_anyof_not_complex(self):
116+
"""Test that empty anyOf is not complex."""
117+
schema = {"anyOf": []}
118+
assert _is_complex_anyof(schema) is False
119+
120+
def test_anyof_missing_from_schema_not_complex(self):
121+
"""Test that schema without anyOf is not complex."""
122+
schema = {"type": "object"}
123+
assert _is_complex_anyof(schema) is False
124+
125+
def test_anyof_with_allof_is_complex(self):
126+
"""Test that anyOf containing allOf is complex (has properties-like structure)."""
127+
schema = {
128+
"anyOf": [
129+
{
130+
"allOf": [
131+
{"$ref": "#/$defs/Base"},
132+
{"properties": {"extra": {"type": "string"}}},
133+
]
134+
},
135+
{"type": "null"},
136+
]
137+
}
138+
# Note: Our implementation checks for $ref or properties in the sub_schema,
139+
# not recursively in allOf, so this should be not complex
140+
# (unless allOf itself has properties)
141+
assert _is_complex_anyof(schema) is False
142+
143+
def test_anyof_union_with_ref(self):
144+
"""Test anyOf representing a union of multiple types including ref."""
145+
schema = {"anyOf": [{"$ref": "#/$defs/User"}, {"$ref": "#/$defs/Admin"}]}
146+
assert _is_complex_anyof(schema) is True
147+
148+
def test_complex_nested_structure(self):
149+
"""Test complex nested object with all optional fields."""
150+
schema = {
151+
"anyOf": [
152+
{
153+
"type": "object",
154+
"properties": {
155+
"user": {
156+
"type": "object",
157+
"properties": {"name": {"type": "string"}},
158+
}
159+
},
160+
},
161+
{"type": "null"},
162+
]
163+
}
164+
assert _is_complex_anyof(schema) is True
165+
166+
167+
if __name__ == "__main__":
168+
pytest.main([__file__])

0 commit comments

Comments
 (0)