From 667613d5941d70243baedd22d43852f7ec27801e Mon Sep 17 00:00:00 2001 From: Barry Wang Date: Thu, 5 Mar 2026 16:51:09 +0800 Subject: [PATCH] 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)