From 9e42ccb51e30817b3720707c9114e13278081206 Mon Sep 17 00:00:00 2001 From: Barry Wang Date: Thu, 5 Mar 2026 16:27:50 +0800 Subject: [PATCH 1/3] feat: auto casting tool params to match schema type --- nanobot/agent/tools/base.py | 114 ++++++++++++++++++++++++++++++ nanobot/agent/tools/registry.py | 4 ++ tests/test_tool_validation.py | 119 ++++++++++++++++++++++++++++++++ 3 files changed, 237 insertions(+) diff --git a/nanobot/agent/tools/base.py b/nanobot/agent/tools/base.py index 051fc9a..b09cfb8 100644 --- a/nanobot/agent/tools/base.py +++ b/nanobot/agent/tools/base.py @@ -3,6 +3,8 @@ from abc import ABC, abstractmethod from typing import Any +from loguru import logger + class Tool(ABC): """ @@ -52,6 +54,118 @@ class Tool(ABC): """ pass + def cast_params(self, params: dict[str, Any]) -> dict[str, Any]: + """ + Attempt to cast parameters to match schema types. + Returns modified params dict. Raises ValueError if casting is impossible. + """ + schema = self.parameters or {} + if schema.get("type", "object") != "object": + return params + + return self._cast_object(params, schema) + + def _cast_object(self, obj: Any, schema: dict[str, Any]) -> dict[str, Any]: + """Cast an object (dict) according to schema.""" + if not isinstance(obj, dict): + return obj + + props = schema.get("properties", {}) + result = {} + + for key, value in obj.items(): + if key in props: + result[key] = self._cast_value(value, props[key]) + else: + result[key] = value + + return result + + def _cast_value(self, val: Any, schema: dict[str, Any]) -> Any: + """Cast a single value according to schema.""" + target_type = schema.get("type") + + # Already correct type + # Note: check bool before int since bool is subclass of int + if target_type == "boolean" and isinstance(val, bool): + return val + if target_type == "integer" and isinstance(val, int) and not isinstance(val, bool): + return val + # For array/object, don't early-return - we need to recurse into contents + if target_type in self._TYPE_MAP and target_type not in ( + "boolean", + "integer", + "array", + "object", + ): + expected = self._TYPE_MAP[target_type] + if isinstance(val, expected): + return val + + # Attempt casting + try: + if target_type == "integer": + if isinstance(val, bool): + # Don't silently convert bool to int + raise ValueError(f"Cannot cast bool to integer") + if isinstance(val, str): + return int(val) + if isinstance(val, (int, float)): + return int(val) + + elif target_type == "number": + if isinstance(val, bool): + # Don't silently convert bool to number + raise ValueError(f"Cannot cast bool to number") + if isinstance(val, str): + return float(val) + if isinstance(val, (int, float)): + return float(val) + + elif target_type == "string": + # Preserve None vs empty string distinction + if val is None: + return val + return str(val) + + elif target_type == "boolean": + if isinstance(val, str): + return val.lower() in ("true", "1", "yes") + return bool(val) + + elif target_type == "array": + if isinstance(val, list): + # Recursively cast array items if schema defines items + if "items" in schema: + return [self._cast_value(item, schema["items"]) for item in val] + return val + # Preserve None vs empty array distinction + if val is None: + return val + # Try to convert single value to array + if val == "": + return [] + return [val] + + elif target_type == "object": + if isinstance(val, dict): + return self._cast_object(val, schema) + # Preserve None vs empty object distinction + if val is None: + return val + # Empty string → empty object + if val == "": + return {} + # Cannot cast to object + raise ValueError(f"Cannot cast {type(val).__name__} to object") + + except (ValueError, TypeError) as e: + # Log failed casts for debugging, return original value + # Let validation catch the error + logger.debug("Failed to cast value %r to %s: %s", val, target_type, e) + + return val + def validate_params(self, params: dict[str, Any]) -> list[str]: """Validate tool parameters against JSON schema. Returns error list (empty if valid).""" if not isinstance(params, dict): diff --git a/nanobot/agent/tools/registry.py b/nanobot/agent/tools/registry.py index 5d36e52..896491f 100644 --- a/nanobot/agent/tools/registry.py +++ b/nanobot/agent/tools/registry.py @@ -44,6 +44,10 @@ class ToolRegistry: return f"Error: Tool '{name}' not found. Available: {', '.join(self.tool_names)}" try: + # Attempt to cast parameters to match schema types + params = tool.cast_params(params) + + # Validate parameters errors = tool.validate_params(params) if errors: return f"Error: Invalid parameters for tool '{name}': " + "; ".join(errors) + _HINT diff --git a/tests/test_tool_validation.py b/tests/test_tool_validation.py index cb50fb0..0083b4e 100644 --- a/tests/test_tool_validation.py +++ b/tests/test_tool_validation.py @@ -106,3 +106,122 @@ def test_exec_extract_absolute_paths_captures_posix_absolute_paths() -> None: paths = ExecTool._extract_absolute_paths(cmd) assert "/tmp/data.txt" in paths assert "/tmp/out.txt" in paths + + +# --- cast_params tests --- + + +class CastTestTool(Tool): + """Minimal tool for testing cast_params.""" + + def __init__(self, schema: dict[str, Any]) -> None: + self._schema = schema + + @property + def name(self) -> str: + return "cast_test" + + @property + def description(self) -> str: + return "test tool for casting" + + @property + def parameters(self) -> dict[str, Any]: + return self._schema + + async def execute(self, **kwargs: Any) -> str: + return "ok" + + +def test_cast_params_string_to_int() -> None: + tool = CastTestTool( + { + "type": "object", + "properties": {"count": {"type": "integer"}}, + } + ) + result = tool.cast_params({"count": "42"}) + assert result["count"] == 42 + assert isinstance(result["count"], int) + + +def test_cast_params_string_to_number() -> None: + tool = CastTestTool( + { + "type": "object", + "properties": {"rate": {"type": "number"}}, + } + ) + result = tool.cast_params({"rate": "3.14"}) + assert result["rate"] == 3.14 + assert isinstance(result["rate"], float) + + +def test_cast_params_string_to_bool() -> None: + tool = CastTestTool( + { + "type": "object", + "properties": {"enabled": {"type": "boolean"}}, + } + ) + assert tool.cast_params({"enabled": "true"})["enabled"] is True + assert tool.cast_params({"enabled": "false"})["enabled"] is False + assert tool.cast_params({"enabled": "1"})["enabled"] is True + + +def test_cast_params_array_items() -> None: + tool = CastTestTool( + { + "type": "object", + "properties": { + "nums": {"type": "array", "items": {"type": "integer"}}, + }, + } + ) + result = tool.cast_params({"nums": ["1", "2", "3"]}) + assert result["nums"] == [1, 2, 3] + + +def test_cast_params_nested_object() -> None: + tool = CastTestTool( + { + "type": "object", + "properties": { + "config": { + "type": "object", + "properties": { + "port": {"type": "integer"}, + "debug": {"type": "boolean"}, + }, + }, + }, + } + ) + result = tool.cast_params({"config": {"port": "8080", "debug": "true"}}) + assert result["config"]["port"] == 8080 + assert result["config"]["debug"] is True + + +def test_cast_params_bool_not_cast_to_int() -> None: + """Booleans should not be silently cast to integers.""" + tool = CastTestTool( + { + "type": "object", + "properties": {"count": {"type": "integer"}}, + } + ) + # Bool input should remain bool (validation will catch it) + result = tool.cast_params({"count": True}) + assert result["count"] is True # Not cast to 1 + + +def test_cast_params_preserves_empty_string() -> None: + """Empty strings should be preserved for string type.""" + tool = CastTestTool( + { + "type": "object", + "properties": {"name": {"type": "string"}}, + } + ) + result = tool.cast_params({"name": ""}) + assert result["name"] == "" From 667613d5941d70243baedd22d43852f7ec27801e Mon Sep 17 00:00:00 2001 From: Barry Wang Date: Thu, 5 Mar 2026 16:51:09 +0800 Subject: [PATCH 2/3] fix edge case casting and more test cases --- nanobot/agent/tools/base.py | 20 +++-- tests/test_tool_validation.py | 136 ++++++++++++++++++++++++++++++++++ 2 files changed, 150 insertions(+), 6 deletions(-) diff --git a/nanobot/agent/tools/base.py b/nanobot/agent/tools/base.py index b09cfb8..fb34fe8 100644 --- a/nanobot/agent/tools/base.py +++ b/nanobot/agent/tools/base.py @@ -57,7 +57,8 @@ class Tool(ABC): def cast_params(self, params: dict[str, Any]) -> dict[str, Any]: """ Attempt to cast parameters to match schema types. - Returns modified params dict. Raises ValueError if casting is impossible. + Returns modified params dict. If casting fails, returns original value + and logs a debug message, allowing validation to catch the error. """ schema = self.parameters or {} if schema.get("type", "object") != "object": @@ -107,7 +108,7 @@ class Tool(ABC): if target_type == "integer": if isinstance(val, bool): # Don't silently convert bool to int - raise ValueError(f"Cannot cast bool to integer") + raise ValueError("Cannot cast bool to integer") if isinstance(val, str): return int(val) if isinstance(val, (int, float)): @@ -116,7 +117,7 @@ class Tool(ABC): elif target_type == "number": if isinstance(val, bool): # Don't silently convert bool to number - raise ValueError(f"Cannot cast bool to number") + raise ValueError("Cannot cast bool to number") if isinstance(val, str): return float(val) if isinstance(val, (int, float)): @@ -130,7 +131,13 @@ class Tool(ABC): elif target_type == "boolean": if isinstance(val, str): - return val.lower() in ("true", "1", "yes") + val_lower = val.lower() + if val_lower in ("true", "1", "yes"): + return True + elif val_lower in ("false", "0", "no"): + return False + # For other strings, raise error to let validation handle it + raise ValueError(f"Cannot convert string '{val}' to boolean") return bool(val) elif target_type == "array": @@ -142,10 +149,11 @@ class Tool(ABC): # Preserve None vs empty array distinction if val is None: return val - # Try to convert single value to array + # Empty string → empty array if val == "": return [] - return [val] + # Don't auto-wrap single values, let validation catch the error + raise ValueError(f"Cannot convert {type(val).__name__} to array") elif target_type == "object": if isinstance(val, dict): diff --git a/tests/test_tool_validation.py b/tests/test_tool_validation.py index 0083b4e..6fb87ea 100644 --- a/tests/test_tool_validation.py +++ b/tests/test_tool_validation.py @@ -225,3 +225,139 @@ def test_cast_params_preserves_empty_string() -> None: ) result = tool.cast_params({"name": ""}) assert result["name"] == "" + + +def test_cast_params_bool_string_false() -> None: + """Test that 'false', '0', 'no' strings convert to False.""" + tool = CastTestTool( + { + "type": "object", + "properties": {"flag": {"type": "boolean"}}, + } + ) + assert tool.cast_params({"flag": "false"})["flag"] is False + assert tool.cast_params({"flag": "False"})["flag"] is False + assert tool.cast_params({"flag": "0"})["flag"] is False + assert tool.cast_params({"flag": "no"})["flag"] is False + assert tool.cast_params({"flag": "NO"})["flag"] is False + + +def test_cast_params_bool_string_invalid() -> None: + """Invalid boolean strings should not be cast.""" + tool = CastTestTool( + { + "type": "object", + "properties": {"flag": {"type": "boolean"}}, + } + ) + # Invalid strings should be preserved (validation will catch them) + result = tool.cast_params({"flag": "random"}) + assert result["flag"] == "random" + result = tool.cast_params({"flag": "maybe"}) + assert result["flag"] == "maybe" + + +def test_cast_params_invalid_string_to_int() -> None: + """Invalid strings should not be cast to integer.""" + tool = CastTestTool( + { + "type": "object", + "properties": {"count": {"type": "integer"}}, + } + ) + result = tool.cast_params({"count": "abc"}) + assert result["count"] == "abc" # Original value preserved + result = tool.cast_params({"count": "12.5.7"}) + assert result["count"] == "12.5.7" + + +def test_cast_params_invalid_string_to_number() -> None: + """Invalid strings should not be cast to number.""" + tool = CastTestTool( + { + "type": "object", + "properties": {"rate": {"type": "number"}}, + } + ) + result = tool.cast_params({"rate": "not_a_number"}) + assert result["rate"] == "not_a_number" + + +def test_cast_params_none_values() -> None: + """Test None handling for different types.""" + tool = CastTestTool( + { + "type": "object", + "properties": { + "name": {"type": "string"}, + "count": {"type": "integer"}, + "items": {"type": "array"}, + "config": {"type": "object"}, + }, + } + ) + result = tool.cast_params( + { + "name": None, + "count": None, + "items": None, + "config": None, + } + ) + # None should be preserved for all types + assert result["name"] is None + assert result["count"] is None + assert result["items"] is None + assert result["config"] is None + + +def test_cast_params_single_value_not_auto_wrapped_to_array() -> None: + """Single values should NOT be automatically wrapped into arrays.""" + tool = CastTestTool( + { + "type": "object", + "properties": {"items": {"type": "array"}}, + } + ) + # Non-array values should be preserved (validation will catch them) + result = tool.cast_params({"items": 5}) + assert result["items"] == 5 # Not wrapped to [5] + result = tool.cast_params({"items": "text"}) + assert result["items"] == "text" # Not wrapped to ["text"] + + +def test_cast_params_empty_string_to_array() -> None: + """Empty string should convert to empty array.""" + tool = CastTestTool( + { + "type": "object", + "properties": {"items": {"type": "array"}}, + } + ) + result = tool.cast_params({"items": ""}) + assert result["items"] == [] + + +def test_cast_params_empty_string_to_object() -> None: + """Empty string should convert to empty object.""" + tool = CastTestTool( + { + "type": "object", + "properties": {"config": {"type": "object"}}, + } + ) + result = tool.cast_params({"config": ""}) + assert result["config"] == {} + + +def test_cast_params_float_to_int() -> None: + """Float values should be cast to integers.""" + tool = CastTestTool( + { + "type": "object", + "properties": {"count": {"type": "integer"}}, + } + ) + result = tool.cast_params({"count": 42.7}) + assert result["count"] == 42 + assert isinstance(result["count"], int) From c3f2d1b01dbf02ec278c7714a85e7cc07f38280c Mon Sep 17 00:00:00 2001 From: Re-bin Date: Sat, 7 Mar 2026 05:28:12 +0000 Subject: [PATCH 3/3] fix(tools): narrow parameter auto-casting --- nanobot/agent/tools/base.py | 113 ++++++++++------------------------ tests/test_tool_validation.py | 54 +++++----------- 2 files changed, 48 insertions(+), 119 deletions(-) diff --git a/nanobot/agent/tools/base.py b/nanobot/agent/tools/base.py index fb34fe8..06f5bdd 100644 --- a/nanobot/agent/tools/base.py +++ b/nanobot/agent/tools/base.py @@ -3,8 +3,6 @@ from abc import ABC, abstractmethod from typing import Any -from loguru import logger - class Tool(ABC): """ @@ -55,11 +53,7 @@ class Tool(ABC): pass def cast_params(self, params: dict[str, Any]) -> dict[str, Any]: - """ - Attempt to cast parameters to match schema types. - Returns modified params dict. If casting fails, returns original value - and logs a debug message, allowing validation to catch the error. - """ + """Apply safe schema-driven casts before validation.""" schema = self.parameters or {} if schema.get("type", "object") != "object": return params @@ -86,91 +80,44 @@ class Tool(ABC): """Cast a single value according to schema.""" target_type = schema.get("type") - # Already correct type - # Note: check bool before int since bool is subclass of int if target_type == "boolean" and isinstance(val, bool): return val if target_type == "integer" and isinstance(val, int) and not isinstance(val, bool): return val - # For array/object, don't early-return - we need to recurse into contents - if target_type in self._TYPE_MAP and target_type not in ( - "boolean", - "integer", - "array", - "object", - ): + if target_type in self._TYPE_MAP and target_type not in ("boolean", "integer", "array", "object"): expected = self._TYPE_MAP[target_type] if isinstance(val, expected): return val - # Attempt casting - try: - if target_type == "integer": - if isinstance(val, bool): - # Don't silently convert bool to int - raise ValueError("Cannot cast bool to integer") - if isinstance(val, str): - return int(val) - if isinstance(val, (int, float)): - return int(val) + if target_type == "integer" and isinstance(val, str): + try: + return int(val) + except ValueError: + return val - elif target_type == "number": - if isinstance(val, bool): - # Don't silently convert bool to number - raise ValueError("Cannot cast bool to number") - if isinstance(val, str): - return float(val) - if isinstance(val, (int, float)): - return float(val) + if target_type == "number" and isinstance(val, str): + try: + return float(val) + except ValueError: + return val - elif target_type == "string": - # Preserve None vs empty string distinction - if val is None: - return val - return str(val) + if target_type == "string": + return val if val is None else str(val) - elif target_type == "boolean": - if isinstance(val, str): - val_lower = val.lower() - if val_lower in ("true", "1", "yes"): - return True - elif val_lower in ("false", "0", "no"): - return False - # For other strings, raise error to let validation handle it - raise ValueError(f"Cannot convert string '{val}' to boolean") - return bool(val) + if target_type == "boolean" and isinstance(val, str): + val_lower = val.lower() + if val_lower in ("true", "1", "yes"): + return True + if val_lower in ("false", "0", "no"): + return False + return val - elif target_type == "array": - if isinstance(val, list): - # Recursively cast array items if schema defines items - if "items" in schema: - return [self._cast_value(item, schema["items"]) for item in val] - return val - # Preserve None vs empty array distinction - if val is None: - return val - # Empty string → empty array - if val == "": - return [] - # Don't auto-wrap single values, let validation catch the error - raise ValueError(f"Cannot convert {type(val).__name__} to array") + if target_type == "array" and isinstance(val, list): + item_schema = schema.get("items") + return [self._cast_value(item, item_schema) for item in val] if item_schema else val - elif target_type == "object": - if isinstance(val, dict): - return self._cast_object(val, schema) - # Preserve None vs empty object distinction - if val is None: - return val - # Empty string → empty object - if val == "": - return {} - # Cannot cast to object - raise ValueError(f"Cannot cast {type(val).__name__} to object") - - except (ValueError, TypeError) as e: - # Log failed casts for debugging, return original value - # Let validation catch the error - logger.debug("Failed to cast value %r to %s: %s", val, target_type, e) + if target_type == "object" and isinstance(val, dict): + return self._cast_object(val, schema) return val @@ -185,7 +132,13 @@ class Tool(ABC): def _validate(self, val: Any, schema: dict[str, Any], path: str) -> list[str]: t, label = schema.get("type"), path or "parameter" - if t in self._TYPE_MAP and not isinstance(val, self._TYPE_MAP[t]): + if t == "integer" and (not isinstance(val, int) or isinstance(val, bool)): + return [f"{label} should be integer"] + if t == "number" and ( + not isinstance(val, self._TYPE_MAP[t]) or isinstance(val, bool) + ): + return [f"{label} should be number"] + if t in self._TYPE_MAP and t not in ("integer", "number") and not isinstance(val, self._TYPE_MAP[t]): return [f"{label} should be {t}"] errors = [] diff --git a/tests/test_tool_validation.py b/tests/test_tool_validation.py index 6fb87ea..c2b4b6a 100644 --- a/tests/test_tool_validation.py +++ b/tests/test_tool_validation.py @@ -210,9 +210,10 @@ def test_cast_params_bool_not_cast_to_int() -> None: "properties": {"count": {"type": "integer"}}, } ) - # Bool input should remain bool (validation will catch it) result = tool.cast_params({"count": True}) - assert result["count"] is True # Not cast to 1 + assert result["count"] is True + errors = tool.validate_params(result) + assert any("count should be integer" in e for e in errors) def test_cast_params_preserves_empty_string() -> None: @@ -283,6 +284,18 @@ def test_cast_params_invalid_string_to_number() -> None: assert result["rate"] == "not_a_number" +def test_validate_params_bool_not_accepted_as_number() -> None: + """Booleans should not pass number validation.""" + tool = CastTestTool( + { + "type": "object", + "properties": {"rate": {"type": "number"}}, + } + ) + errors = tool.validate_params({"rate": False}) + assert any("rate should be number" in e for e in errors) + + def test_cast_params_none_values() -> None: """Test None handling for different types.""" tool = CastTestTool( @@ -324,40 +337,3 @@ def test_cast_params_single_value_not_auto_wrapped_to_array() -> None: assert result["items"] == 5 # Not wrapped to [5] result = tool.cast_params({"items": "text"}) assert result["items"] == "text" # Not wrapped to ["text"] - - -def test_cast_params_empty_string_to_array() -> None: - """Empty string should convert to empty array.""" - tool = CastTestTool( - { - "type": "object", - "properties": {"items": {"type": "array"}}, - } - ) - result = tool.cast_params({"items": ""}) - assert result["items"] == [] - - -def test_cast_params_empty_string_to_object() -> None: - """Empty string should convert to empty object.""" - tool = CastTestTool( - { - "type": "object", - "properties": {"config": {"type": "object"}}, - } - ) - result = tool.cast_params({"config": ""}) - assert result["config"] == {} - - -def test_cast_params_float_to_int() -> None: - """Float values should be cast to integers.""" - tool = CastTestTool( - { - "type": "object", - "properties": {"count": {"type": "integer"}}, - } - ) - result = tool.cast_params({"count": 42.7}) - assert result["count"] == 42 - assert isinstance(result["count"], int)