From 0a33e87393cff28dd75a9c637f7d80ab7282947e Mon Sep 17 00:00:00 2001 From: Shudipto Trafder Date: Mon, 3 Aug 2026 17:34:26 +0600 Subject: [PATCH 1/2] fix: update version number in pyproject.toml to 0.9.1 --- agentflow/core/graph/tool_node/__init__.py | 10 +- agentflow/core/graph/tool_node/_helpers.py | 43 +- agentflow/core/graph/tool_node/coercion.py | 149 ++++++ agentflow/core/graph/tool_node/local_exec.py | 12 +- agentflow/core/graph/tool_node/schema.py | 513 +++++++++++++++---- agentflow/utils/decorators.py | 16 + pyproject.toml | 2 + tests/graph/test_tool_node.py | 7 +- tests/graph/test_tool_node_helpers.py | 68 +++ tests/graph/test_tool_schema_objects.py | 501 ++++++++++++++++++ 10 files changed, 1206 insertions(+), 115 deletions(-) create mode 100644 agentflow/core/graph/tool_node/coercion.py create mode 100644 tests/graph/test_tool_schema_objects.py diff --git a/agentflow/core/graph/tool_node/__init__.py b/agentflow/core/graph/tool_node/__init__.py index fc182327..740e7c38 100644 --- a/agentflow/core/graph/tool_node/__init__.py +++ b/agentflow/core/graph/tool_node/__init__.py @@ -4,12 +4,20 @@ - ToolNode - HAS_FASTMCP, HAS_MCP +- UnsupportedToolParameterError """ from agentflow.core.state.tool_result import ToolResult from .base import ToolNode from .deps import HAS_FASTMCP, HAS_MCP +from .schema import UnsupportedToolParameterError -__all__ = ["HAS_FASTMCP", "HAS_MCP", "ToolNode", "ToolResult"] +__all__ = [ + "HAS_FASTMCP", + "HAS_MCP", + "ToolNode", + "ToolResult", + "UnsupportedToolParameterError", +] diff --git a/agentflow/core/graph/tool_node/_helpers.py b/agentflow/core/graph/tool_node/_helpers.py index 70af0379..ff1dd5cf 100644 --- a/agentflow/core/graph/tool_node/_helpers.py +++ b/agentflow/core/graph/tool_node/_helpers.py @@ -2,8 +2,13 @@ from __future__ import annotations +import datetime as dt +import decimal +import enum import json +import pathlib import typing as t +import uuid _STATUS_OK: set[str] = {"completed", "success", "ok", "done", "true", "1"} @@ -11,6 +16,34 @@ _ERROR_TRUE: set[str] = {"true", "1", "yes", "error", "failed", "failure"} +def _json_default(obj: t.Any) -> t.Any: + """Render the scalar types a tool can return but JSON cannot hold. + + Mirrors the scalars ``schema.py`` accepts on the way in, so a value the model sent + as ``"2026-01-15T09:30:00"`` comes back in that same textual form rather than as a + Python repr. + + Raises: + TypeError: For anything else, so ``json.dumps`` propagates and the caller falls + through to its existing repr fallback. Unknown objects must not be silently + stringified here, or a genuinely unserializable payload would look clean. + """ + if isinstance(obj, dt.datetime | dt.date | dt.time): + return obj.isoformat() + if isinstance(obj, uuid.UUID | pathlib.PurePath): + return str(obj) + if isinstance(obj, decimal.Decimal): + # str, not float: a money value must not lose precision on the way to the model. + return str(obj) + if isinstance(obj, enum.Enum): + return obj.value + if isinstance(obj, set | frozenset): + return list(obj) + if isinstance(obj, bytes | bytearray): + return obj.decode("utf-8", errors="replace") + raise TypeError(f"Object of type {type(obj).__name__} is not JSON serializable") + + def _safe_serialize(obj: t.Any) -> dict[str, t.Any]: try: json.dumps(obj) @@ -24,7 +57,15 @@ def _safe_serialize(obj: t.Any) -> dict[str, t.Any]: resource["uri"] = str(resource["uri"]) dumped["resource"] = resource return dumped - return {"content": str(obj), "type": "fallback"} + + # Retry with the scalar renderer so a container keeps its shape and only the + # offending leaves become text. Without this a single datetime collapses the + # whole return value into one repr string. + try: + normalized = json.loads(json.dumps(obj, default=_json_default)) + except (TypeError, OverflowError, ValueError): + return {"content": str(obj), "type": "fallback"} + return normalized if isinstance(normalized, dict) else {"content": normalized} def _as_bool(val: t.Any, truthy_set: set[str]) -> bool: diff --git a/agentflow/core/graph/tool_node/coercion.py b/agentflow/core/graph/tool_node/coercion.py new file mode 100644 index 00000000..e685f32a --- /dev/null +++ b/agentflow/core/graph/tool_node/coercion.py @@ -0,0 +1,149 @@ +"""Coercion of raw LLM tool arguments into the types a tool function declares. + +A provider hands back plain JSON, so a parameter annotated with a pydantic model, a +dataclass, or an enum arrives as a ``dict`` / ``str``. Passing that through untouched +means the tool body receives a ``dict`` where it expects a model instance, or the raw +string ``"active"`` where it expects ``Status.ACTIVE``. Both fail silently. + +Validation goes through pydantic in both cases. Notably, a dataclass must **not** be +built with ``Cls(**value)``: a dataclass constructor performs no validation and no +conversion, so enum fields stay strings, nested dataclass fields stay dicts, and int +fields keep whatever the model sent, all without raising. ``TypeAdapter`` handles the +whole tree correctly and reports a precise error when the payload is wrong. + +The same applies to the scalar stdlib types. JSON has no date or UUID, so ``schema.py`` +advertises ``datetime`` as ``{"type": "string", "format": "date-time"}`` and the model +can only answer with a string. Leaving it as one means a body that does ``when.year`` +raises ``AttributeError`` on a ``str``, so every type ``schema.py`` lists in ``_SCALARS`` +must be coerced back here. That list is imported rather than restated: a type advertised +as a formatted string in one module and unknown to the other is exactly the asymmetry +this module exists to prevent. + +Coercion is applied only when the annotation actually contains a structured or scalar +type. Plain primitives (``int``, ``str``, ``float``, ``bool``), ``dict``, and +``list[str]`` are passed through unchanged so existing tools see no behaviour change. +""" + +from __future__ import annotations + +import dataclasses +import enum +import inspect +import json +import typing as t +from functools import lru_cache + +from pydantic import BaseModel, TypeAdapter, ValidationError + +from .schema import _SCALARS + + +_EMPTY = inspect._empty +_MAX_DEPTH = 8 + +# Types that reach the tool as a JSON string (or number) and must be parsed back into +# the annotated type. Sourced from the schema builder so the two cannot drift. +_SCALAR_TYPES: tuple[type, ...] = tuple(k for k in _SCALARS if isinstance(k, type)) + + +def _is_structured(annotation: t.Any) -> bool: + """Return True for the types that need pydantic validation to be usable.""" + if not isinstance(annotation, type): + return False + return ( + issubclass(annotation, BaseModel) + or issubclass(annotation, enum.Enum) + or dataclasses.is_dataclass(annotation) + or annotation in _SCALAR_TYPES + ) + + +def _needs_coercion(annotation: t.Any, depth: int = 0) -> bool: + """Return True when the annotation contains a structured type anywhere inside it.""" + if depth > _MAX_DEPTH: + return False + if _is_structured(annotation): + return True + return any(_needs_coercion(arg, depth + 1) for arg in t.get_args(annotation)) + + +@lru_cache(maxsize=512) +def _cached_needs_coercion(annotation: t.Any) -> bool: + return _needs_coercion(annotation) + + +@lru_cache(maxsize=512) +def _adapter(annotation: t.Any) -> TypeAdapter: + """Build and cache a TypeAdapter. Construction is expensive; this runs per call.""" + return TypeAdapter(annotation) + + +def _maybe_json(value: t.Any) -> t.Any: + """Decode a JSON object/array string. + + Weaker models, and any model whose prompt history was built against the old + ``{"type": "string"}`` schema, hand-serialize nested objects into a single string. + Only strings that clearly look like a JSON object or array are decoded, so an + enum member such as ``"5"`` is never turned into an int. + """ + if not isinstance(value, str): + return value + stripped = value.strip() + if stripped[:1] not in ("{", "["): + return value + try: + return json.loads(stripped) + except ValueError: + return value + + +def coerce_tool_argument( + value: t.Any, + annotation: t.Any, + *, + tool_name: str, + param_name: str, +) -> t.Any: + """Coerce one raw tool argument into its declared type. + + Args: + value: The raw argument value as provided by the model. + annotation: The resolved annotation of the target parameter. + tool_name: Tool name, used only for error messages. + param_name: Parameter name, used only for error messages. + + Returns: + The coerced value, or the original value when no coercion applies. + + Raises: + TypeError: If the value does not satisfy the annotation. The message carries + the pydantic field errors so it can be handed back to the model as a + retryable tool error. + """ + if annotation is _EMPTY or annotation is t.Any or annotation is None: + return value + + try: + needs = _cached_needs_coercion(annotation) + except TypeError: + # Unhashable annotation; fall back to the uncached walk. + needs = _needs_coercion(annotation) + + if not needs: + return value + + candidate = _maybe_json(value) + + try: + return _adapter(annotation).validate_python(candidate) + except TypeError: + # Unhashable annotation could not be cached; build the adapter directly. + return TypeAdapter(annotation).validate_python(candidate) + except ValidationError as exc: + details = "; ".join( + f"{'.'.join(str(p) for p in err['loc']) or param_name}: {err['msg']}" + for err in exc.errors() + ) + raise TypeError( + f"Invalid argument {param_name!r} for tool {tool_name!r}: {details}" + ) from exc diff --git a/agentflow/core/graph/tool_node/local_exec.py b/agentflow/core/graph/tool_node/local_exec.py index 246935c1..f34ef84d 100644 --- a/agentflow/core/graph/tool_node/local_exec.py +++ b/agentflow/core/graph/tool_node/local_exec.py @@ -19,7 +19,9 @@ from agentflow.utils import CallbackContext, CallbackManager, InvocationType, call_sync_or_async from ._helpers import _extract_block_meta, _safe_serialize +from .coercion import coerce_tool_argument from .constants import INJECTABLE_PARAMS, has_injected_default +from .schema import _safe_type_hints if t.TYPE_CHECKING: @@ -39,6 +41,7 @@ def _prepare_input_data_tool( default_data: dict, ) -> dict: sig = inspect.signature(fn) + hints = _safe_type_hints(fn) input_data = {} for param_name, param in sig.parameters.items(): if param.kind in ( @@ -62,7 +65,14 @@ def _prepare_input_data_tool( continue if param_name in args: - input_data[param_name] = args[param_name] + # The provider returns plain JSON, so a model/dataclass/enum parameter + # arrives as a dict or str and must be validated into its real type. + input_data[param_name] = coerce_tool_argument( + args[param_name], + hints.get(param_name, param.annotation), + tool_name=name, + param_name=param_name, + ) elif param.default is inspect.Parameter.empty: raise TypeError(f"Missing required parameter '{param_name}' for function '{name}'") diff --git a/agentflow/core/graph/tool_node/schema.py b/agentflow/core/graph/tool_node/schema.py index dde853c7..7ff0f286 100644 --- a/agentflow/core/graph/tool_node/schema.py +++ b/agentflow/core/graph/tool_node/schema.py @@ -2,21 +2,138 @@ This module provides the SchemaMixin class which handles automatic schema generation for local Python functions, converting their type annotations and signatures into -OpenAI-compatible function schemas. It supports various Python types including -primitives, Optional types, List types, and Literal enums. - -The schema generation process inspects function signatures and converts them to -JSON Schema format suitable for use with language models and function calling APIs. +OpenAI-compatible function schemas. + +Design constraints +------------------ +The generated ``parameters`` dict is forwarded **verbatim** to the provider: Google +receives it as ``FunctionDeclaration(parameters_json_schema=...)`` and OpenAI-style +providers receive it as ``tools[].function.parameters``. Many OpenAI-compatible +endpoints ship weak JSON Schema parsers, so the emitted schema is deliberately +restricted to a portable subset: + +* no ``$ref`` / ``$defs`` (nested models are inlined) +* no ``anyOf`` (``Optional[X]`` collapses to ``X``) +* no ``allOf`` / ``title`` / ``discriminator`` / ``const`` + +That is why nested objects are walked by hand rather than delegating to +``BaseModel.model_json_schema()``, which emits all of the above. The rule is: +**pydantic for validation, hand-rolled for schema.** + +Supported parameter annotations +------------------------------- +``str``, ``int``, ``float``, ``bool``, a few scalar stdlib types (``datetime``, +``date``, ``time``, ``UUID``, ``Path``, ``Decimal``, ``bytes``), ``Optional[X]``, +``list[X]``, ``dict`` / ``dict[str, X]``, ``Literal[...]``, ``enum.Enum`` +subclasses, pydantic ``BaseModel`` subclasses, and dataclasses, plus any nesting +of those. + +Anything else raises :class:`UnsupportedToolParameterError` at schema-build time +rather than silently degrading to ``{"type": "string"}``, which is what caused +malformed function-call arguments in the past. Use ``@tool(parameters=...)`` to +supply a hand-written schema when a parameter cannot be expressed here. """ from __future__ import annotations +import dataclasses +import datetime as dt +import decimal +import enum import inspect +import logging +import pathlib +import types import typing as t +import uuid + +from pydantic import BaseModel from .constants import is_injected_param +logger = logging.getLogger("agentflow.graph.tool_node") + +_EMPTY = inspect._empty + +# Exact-match primitives. Deliberately identity-ish lookups, not issubclass, so that +# `class Status(str, Enum)` is routed to the enum branch instead of being seen as a str. +_PRIMITIVES: dict[t.Any, dict] = { + str: {"type": "string"}, + int: {"type": "integer"}, + float: {"type": "number"}, + bool: {"type": "boolean"}, +} + +# Scalar stdlib types a model can only produce as a JSON string. Only the four +# standard JSON Schema formats are emitted; anything exotic is left off. +_SCALARS: dict[t.Any, dict] = { + dt.datetime: {"type": "string", "format": "date-time"}, + dt.date: {"type": "string", "format": "date"}, + dt.time: {"type": "string", "format": "time"}, + uuid.UUID: {"type": "string", "format": "uuid"}, + pathlib.Path: {"type": "string"}, + pathlib.PurePath: {"type": "string"}, + bytes: {"type": "string"}, + decimal.Decimal: {"type": "number"}, +} + +# Cut-off for pathological nesting. Cycles are caught by the `seen` chain; this is a +# backstop for deeply generic types. +_MAX_DEPTH = 8 + +# dict[K, V] has exactly two type args; anything else is treated as an untyped object. +_DICT_ARG_COUNT = 2 + +_HELP = ( + "Supported: str, int, float, bool, datetime/date/time/UUID/Path/Decimal/bytes, " + "Optional[X], list[X], dict, dict[str, X], Literal[...], enum.Enum, " + "pydantic BaseModel, or a dataclass. " + "To bypass automatic generation entirely, pass an explicit schema: " + "@tool(parameters={...})." +) + + +class UnsupportedToolParameterError(TypeError): + """A tool parameter annotation cannot be expressed as a portable JSON Schema. + + Subclasses :class:`TypeError` so that existing ``except TypeError`` handlers + around tool registration keep working. + """ + + +def _safe_type_hints(obj: t.Any) -> dict[str, t.Any]: + """Resolve PEP 563 string annotations, degrading gracefully. + + ``inspect.signature`` returns raw strings for any module using + ``from __future__ import annotations``, which previously made every parameter + (including plain ``int``) fall through to ``{"type": "string"}``. + + ``typing.get_type_hints`` resolves the whole object at once and therefore fails + outright if *any* annotation is unresolvable, for example a ``TYPE_CHECKING``-only + import on an injected parameter. In that case each annotation is resolved + individually so one bad annotation cannot poison the rest. + """ + try: + return t.get_type_hints(obj) + except Exception as exc: + logger.debug("Bulk annotation resolution failed for %r: %s", obj, exc) + + module = inspect.getmodule(obj) + globalns = getattr(module, "__dict__", {}) + resolved: dict[str, t.Any] = {} + for name, raw in getattr(obj, "__annotations__", {}).items(): + if not isinstance(raw, str): + resolved[name] = raw + continue + try: + resolved[name] = eval(raw, globalns, None) # noqa: S307 # nosec B307 + except Exception as exc: + logger.debug("Could not resolve annotation %r for %r: %s", raw, name, exc) + continue + return resolved + + class SchemaMixin: """Mixin providing schema generation and local tool description building. @@ -24,9 +141,6 @@ class SchemaMixin: from Python function signatures. It handles type annotation conversion, parameter analysis, and OpenAI-compatible function schema generation for local tools. - The mixin is designed to be used with ToolNode to automatically generate tool - schemas without requiring manual schema definition for Python functions. - Attributes: _funcs: Dictionary mapping function names to callable functions. This attribute is expected to be provided by the mixing class. @@ -34,25 +148,33 @@ class SchemaMixin: _funcs: dict[str, t.Callable] + # ---------------------------------------------------------------- primitives + + @staticmethod + def _enum_schema(values: list[t.Any]) -> dict: + """Build an enum schema, typing it from the member values. + + Shared by ``Literal[...]`` and ``enum.Enum`` so the two stay consistent. + An ``IntEnum`` must not be advertised as a string. + """ + if values and all(isinstance(v, str) for v in values): + return {"type": "string", "enum": list(values)} + if values and all(isinstance(v, int) and not isinstance(v, bool) for v in values): + return {"type": "integer", "enum": list(values)} + return {"enum": list(values)} + @staticmethod def _handle_optional_annotation(annotation: t.Any, default: t.Any) -> dict | None: - """Handle Optional type annotations and convert them to appropriate schemas. + """Handle ``Optional[T]`` annotations by generating schema for ``T``. - Processes Optional[T] type annotations (Union[T, None]) and generates - schema for the non-None type. This method handles the common pattern - of optional parameters in function signatures. + Kept for backwards compatibility with callers that used this directly. Args: annotation: The type annotation to process, potentially an Optional type. default: The default value for the parameter, used for schema generation. Returns: - Dictionary containing the JSON schema for the non-None type if the - annotation is Optional, None otherwise. - - Example: - Optional[str] -> {"type": "string"} - Optional[int] -> {"type": "integer"} + Schema for the non-None member if the annotation is Optional, else None. """ args = getattr(annotation, "__args__", None) if args and any(a is type(None) for a in args): @@ -63,85 +185,244 @@ def _handle_optional_annotation(annotation: t.Any, default: t.Any) -> dict | Non @staticmethod def _handle_complex_annotation(annotation: t.Any) -> dict: - """Handle complex type annotations like List, Literal, and generic types. + """Handle generic annotations (list, Literal, dict). - Processes generic type annotations that aren't simple primitive types, - including container types like List and special types like Literal enums. - Falls back to string type for unrecognized complex types. + Retained for backwards compatibility; delegates to the main resolver. Args: - annotation: The complex type annotation to process (e.g., List[str], - Literal["a", "b", "c"]). + annotation: The complex type annotation to process. Returns: - Dictionary containing the appropriate JSON schema for the complex type. - For List types, returns array schema with item type. - For Literal types, returns enum schema with allowed values. - For unknown types, returns string type as fallback. + The JSON schema for the annotation. - Example: - List[str] -> {"type": "array", "items": {"type": "string"}} - Literal["red", "green"] -> {"type": "string", "enum": ["red", "green"]} + Raises: + UnsupportedToolParameterError: If the annotation is not supported. """ - origin = getattr(annotation, "__origin__", None) - if origin is list: - item_type = getattr(annotation, "__args__", (str,))[0] - item_schema = SchemaMixin._annotation_to_schema(item_type, None) - return {"type": "array", "items": item_schema} + return SchemaMixin._schema_for(annotation, (), 0) - Literal = getattr(t, "Literal", None) - if Literal is not None and origin is Literal: - literals = list(getattr(annotation, "__args__", ())) - if all(isinstance(literal, str) for literal in literals): - return {"type": "string", "enum": literals} - return {"enum": literals} + # ------------------------------------------------------------ object walking - return {"type": "string"} + @staticmethod + def _model_fields(model: type[BaseModel]) -> list[tuple]: + """Extract ``(key, annotation, required, default, description)`` from a model.""" + fields = [] + for name, info in model.model_fields.items(): + required = info.is_required() + # A default_factory value must not be called at schema-build time; the + # field is simply not required and carries no advertised default. + skip_default = required or info.default_factory is not None + default = _EMPTY if skip_default else info.default + fields.append( + (info.alias or name, info.annotation, required, default, info.description) + ) + return fields @staticmethod - def _annotation_to_schema(annotation: t.Any, default: t.Any) -> dict: - """Convert a Python type annotation to JSON Schema format. + def _dataclass_fields(owner: type) -> list[tuple]: + """Extract ``(key, annotation, required, default, description)`` from a dataclass. + + ``dataclasses.fields(...).type`` is a raw string whenever the defining module + uses ``from __future__ import annotations``, so hints are resolved separately. + """ + hints = _safe_type_hints(owner) + fields = [] + for f in dataclasses.fields(owner): + if not f.init: + continue + has_default = f.default is not dataclasses.MISSING + has_factory = f.default_factory is not dataclasses.MISSING + fields.append( + ( + f.name, + hints.get(f.name, f.type), + not (has_default or has_factory), + f.default if has_default else _EMPTY, + None, + ) + ) + return fields - Main entry point for type annotation conversion. Handles both simple - and complex types by delegating to appropriate helper methods. - Includes default value handling when present. + @staticmethod + def _object_schema(owner: type, fields: list[tuple], seen: tuple, depth: int) -> dict: + """Build an inlined object schema from extracted field descriptors.""" + if owner in seen: + # Self-referential model. Emit an untyped object at the cut point rather + # than recursing forever; there is no $ref to fall back on. + return {"type": "object"} + + seen = (*seen, owner) + properties: dict[str, dict] = {} + required: list[str] = [] + for key, annotation, is_required, default, description in fields: + sub = SchemaMixin._schema_for(annotation, seen, depth + 1) + if default is not _EMPTY and default is not None: + sub = {**sub, "default": default} + if description: + sub = {**sub, "description": description} + properties[key] = sub + if is_required: + required.append(key) + + schema: dict = {"type": "object", "properties": properties} + if required: + schema["required"] = required + return schema + + # ----------------------------------------------------------- main dispatcher + + @staticmethod + def _schema_for(annotation: t.Any, seen: tuple, depth: int) -> dict: + """Resolve a single annotation to a portable JSON Schema fragment. Args: - annotation: The Python type annotation to convert (e.g., str, int, - Optional[str], List[int]). - default: The default value for the parameter, included in schema - if not inspect._empty. + annotation: The annotation to convert. + seen: Chain of object types currently being expanded, for cycle detection. + depth: Current nesting depth, bounded by ``_MAX_DEPTH``. Returns: - Dictionary containing the JSON schema representation of the type - annotation, including default values where applicable. + A JSON schema fragment containing only portable keywords. - Example: - str -> {"type": "string"} - int -> {"type": "integer"} - str with default "hello" -> {"type": "string", "default": "hello"} + Raises: + UnsupportedToolParameterError: If the annotation is not supported. """ - schema = SchemaMixin._handle_optional_annotation(annotation, default) - if schema: - return schema + if depth > _MAX_DEPTH: + return {"type": "object"} + + # Unannotated or explicitly unconstrained: no schema constraint to express. + if annotation is _EMPTY or annotation is t.Any or annotation is None: + return {"type": "string"} + + # An annotation that is still a string here means resolution failed upstream. + if isinstance(annotation, str | t.ForwardRef): + raise UnsupportedToolParameterError( + f"annotation {annotation!r} is still a string and could not be resolved " + f"to a real type. Under `from __future__ import annotations` a name is " + f"only resolvable if it exists at module scope at runtime, so this " + f"happens when the type is imported under `if TYPE_CHECKING:` or is " + f"defined inside a function body. Move it to module scope, import it " + f"normally, or bypass generation with @tool(parameters={{...}})." + ) + + generic = SchemaMixin._schema_for_generic(annotation, seen, depth) + if generic is not None: + return generic + + if t.is_typeddict(annotation): + raise UnsupportedToolParameterError( + f"TypedDict {getattr(annotation, '__name__', annotation)!r} is not " + f"supported; use a pydantic BaseModel or a dataclass so arguments can " + f"also be validated at call time. {_HELP}" + ) + + if isinstance(annotation, type): + concrete = SchemaMixin._schema_for_class(annotation, seen, depth) + if concrete is not None: + return concrete + + raise UnsupportedToolParameterError(f"annotation {annotation!r} is not supported. {_HELP}") + + @staticmethod + def _schema_for_generic(annotation: t.Any, seen: tuple, depth: int) -> dict | None: + """Resolve unions and generic containers. Returns None if not one of those. + + Raises: + UnsupportedToolParameterError: For containers with no portable schema. + """ + origin = t.get_origin(annotation) + args = t.get_args(annotation) + + # Optional[X] / X | None collapse to X. A union of several real types has no + # portable representation, so it is rejected instead of silently picking one. + if origin is t.Union or origin is types.UnionType: + non_none = [a for a in args if a is not type(None)] + if not non_none: + return {"type": "string"} + if len(non_none) == 1: + return SchemaMixin._schema_for(non_none[0], seen, depth) + raise UnsupportedToolParameterError( + f"union of multiple types {annotation!r} is not supported; a portable " + f"schema cannot express it without anyOf. Use a single type, or " + f"@tool(parameters={{...}})." + ) + + if origin is t.Literal: + return SchemaMixin._enum_schema(list(args)) + + if annotation is list: + return {"type": "array", "items": {"type": "string"}} + if origin is list: + item = args[0] if args else str + # Note: no default is threaded into `items`; a nested item schema must + # never carry a `"default": null`. + return {"type": "array", "items": SchemaMixin._schema_for(item, seen, depth + 1)} + + if annotation is dict: + return {"type": "object"} + if origin is dict: + value_type = args[1] if len(args) == _DICT_ARG_COUNT else t.Any + if value_type is t.Any: + return {"type": "object"} + return { + "type": "object", + "additionalProperties": SchemaMixin._schema_for(value_type, seen, depth + 1), + } + + if origin in (set, frozenset, tuple): + raise UnsupportedToolParameterError( + f"{annotation!r} is not supported; use list[...] instead. {_HELP}" + ) + + return None - primitive_mappings = { - str: {"type": "string"}, - int: {"type": "integer"}, - float: {"type": "number"}, - bool: {"type": "boolean"}, - } + @staticmethod + def _schema_for_class(annotation: type, seen: tuple, depth: int) -> dict | None: + """Resolve a concrete class. Returns None if the class is not supported.""" + if annotation in _PRIMITIVES: + return dict(_PRIMITIVES[annotation]) + if annotation in _SCALARS: + return dict(_SCALARS[annotation]) + if issubclass(annotation, enum.Enum): + return SchemaMixin._enum_schema([m.value for m in annotation]) + if issubclass(annotation, BaseModel): + return SchemaMixin._object_schema( + annotation, SchemaMixin._model_fields(annotation), seen, depth + ) + if dataclasses.is_dataclass(annotation): + return SchemaMixin._object_schema( + annotation, SchemaMixin._dataclass_fields(annotation), seen, depth + ) + return None - if annotation in primitive_mappings: - schema = primitive_mappings[annotation] - else: - schema = SchemaMixin._handle_complex_annotation(annotation) + @staticmethod + def _annotation_to_schema(annotation: t.Any, default: t.Any) -> dict: + """Convert a Python type annotation to portable JSON Schema. - if default is not inspect._empty: - schema["default"] = default + Args: + annotation: The Python type annotation to convert. + default: The default value for the parameter. Included in the schema + unless it is ``inspect._empty``. + Returns: + The JSON schema representation of the annotation. + + Raises: + UnsupportedToolParameterError: If the annotation is not supported. + + Example: + str -> {"type": "string"} + list[int] -> {"type": "array", "items": {"type": "integer"}} + SomeModel -> {"type": "object", "properties": {...}, "required": [...]} + """ + schema = SchemaMixin._schema_for(annotation, (), 0) + # A null default is dropped: it tells the model nothing that absence from + # `required` does not already say, and a `"default": null` on a typed field is + # rejected by some strict schema parsers. + if default is not _EMPTY and default is not None: + schema = {**schema, "default": default} return schema + # -------------------------------------------------------------- tool listing + def get_local_tool( self, tags: set[str] | None = None, @@ -153,9 +434,12 @@ def get_local_tool( Excludes injectable parameters that are provided by the framework. Returns: - List of tool definitions in OpenAI function calling format. Each - definition includes the function name, description (from docstring), - and complete parameter schema with types and required fields. + List of tool definitions in OpenAI function calling format. + + Raises: + UnsupportedToolParameterError: If a tool declares a parameter whose + annotation cannot be expressed as a portable JSON Schema. Attach an + explicit schema with ``@tool(parameters={...})`` to bypass generation. Example: For a function: @@ -194,41 +478,23 @@ def calculate(a: int, b: int, operation: str = "add") -> int: """ tools: list[dict] = [] for name, fn in self._funcs.items(): - sig = inspect.signature(fn) - params_schema: dict = {"type": "object", "properties": {}, "required": []} - - for p_name, p in sig.parameters.items(): - if p.kind in ( - inspect.Parameter.VAR_POSITIONAL, - inspect.Parameter.VAR_KEYWORD, - ): - continue - - if is_injected_param(p_name, p): - continue - - annotation = p.annotation if p.annotation is not inspect._empty else str - prop = SchemaMixin._annotation_to_schema(annotation, p.default) - params_schema["properties"][p_name] = prop - - if p.default is inspect._empty: - params_schema["required"].append(p_name) - - if not params_schema["required"]: - params_schema.pop("required") - # Use decorator metadata if available, otherwise fall back to defaults tool_name = getattr(fn, "_py_tool_name", name) description = getattr(fn, "_py_tool_description", None) if description is None: description = inspect.getdoc(fn) or "No description provided." - # provider = getattr(fn, "_py_tool_provider", None) fun_tags = getattr(fn, "_py_tool_tags", None) capabilities = getattr(fn, "_py_tool_capabilities", None) if tags and fun_tags and tags.isdisjoint(fun_tags): continue + override = getattr(fn, "_py_tool_parameters", None) + if override is not None: + params_schema = override + else: + params_schema = self._build_params_schema(fn, tool_name) + entry = { "type": "function", "function": { @@ -239,16 +505,45 @@ def calculate(a: int, b: int, operation: str = "add") -> int: } if capabilities is not None: entry["function"]["x-function-capabilities"] = capabilities - # meta: dict[str, t.Any] = {} - # if provider: - # meta["provider"] = provider - # if tags: - # meta["tags"] = tags - # if capabilities: - # meta["capabilities"] = capabilities - # if meta: - # entry["x-agentflow"] = meta tools.append(entry) return tools + + @staticmethod + def _build_params_schema(fn: t.Callable, tool_name: str) -> dict: + """Build the ``parameters`` object for one tool function.""" + sig = inspect.signature(fn) + hints = _safe_type_hints(fn) + params_schema: dict = {"type": "object", "properties": {}, "required": []} + + for p_name, p in sig.parameters.items(): + if p.kind in ( + inspect.Parameter.VAR_POSITIONAL, + inspect.Parameter.VAR_KEYWORD, + ): + continue + + if is_injected_param(p_name, p): + continue + + annotation = hints.get(p_name, p.annotation) + if annotation is _EMPTY: + annotation = str + + try: + prop = SchemaMixin._annotation_to_schema(annotation, p.default) + except UnsupportedToolParameterError as exc: + raise UnsupportedToolParameterError( + f"tool {tool_name!r}, parameter {p_name!r}: {exc}" + ) from exc + + params_schema["properties"][p_name] = prop + + if p.default is _EMPTY: + params_schema["required"].append(p_name) + + if not params_schema["required"]: + params_schema.pop("required") + + return params_schema diff --git a/agentflow/utils/decorators.py b/agentflow/utils/decorators.py index 7fa8a909..a11a3c8a 100644 --- a/agentflow/utils/decorators.py +++ b/agentflow/utils/decorators.py @@ -28,6 +28,7 @@ def tool[F: Callable[..., Any]]( provider: str | None = None, capabilities: list[str] | None = None, metadata: dict[str, Any] | None = None, + parameters: dict[str, Any] | None = None, ) -> Callable[[F], F] | F: """Decorator to mark a function as a tool with metadata. @@ -58,6 +59,11 @@ def tool[F: Callable[..., Any]]( Examples: ["read_files", "network_access", "database_write"]. metadata: Additional arbitrary metadata to attach to the tool. This can include custom fields specific to your application or workflow. + parameters: An explicit JSON Schema for the tool's parameters. When provided, + automatic schema generation is skipped and this dict is forwarded to the + provider verbatim. Use it for parameter types the generator rejects, or + when a specific provider needs a hand-tuned schema. Argument coercion + still runs off the real annotations, so keep the two in agreement. Returns: A decorator function that wraps the target function and attaches metadata @@ -101,6 +107,7 @@ def tool[F: Callable[..., Any]]( - _py_tool_provider: The provider string - _py_tool_capabilities: List of capabilities - _py_tool_metadata: Additional metadata dict + - _py_tool_parameters: Explicit parameters JSON Schema, if supplied - These attributes are used by ToolNode and schema generation to build OpenAI-compatible function calling schemas. - Tags are converted to a set for efficient membership testing. @@ -151,6 +158,13 @@ def decorator(func: F) -> F: if metadata is not None: func._py_tool_metadata = metadata # type: ignore[attr-defined] + # Explicit parameters schema bypasses automatic schema generation entirely + if parameters is not None: + if not isinstance(parameters, dict): + msg = f"@tool(parameters=...) must be a JSON Schema dict, got {type(parameters)}" + raise ValueError(msg) + func._py_tool_parameters = parameters # type: ignore[attr-defined] + return func # Handle being called without parentheses (@tool) @@ -178,6 +192,7 @@ def get_tool_metadata(func: Callable) -> dict[str, Any]: - provider: The provider string (or None if not set) - capabilities: List of capabilities (or None if not set) - metadata: Additional metadata dict (or None if not set) + - parameters: Explicit parameters JSON Schema (or None if not set) Example: >>> @tool(name="my_tool", tags=["test"]) @@ -197,6 +212,7 @@ def get_tool_metadata(func: Callable) -> dict[str, Any]: "provider": getattr(func, "_py_tool_provider", None), "capabilities": getattr(func, "_py_tool_capabilities", None), "metadata": getattr(func, "_py_tool_metadata", None), + "parameters": getattr(func, "_py_tool_parameters", None), } diff --git a/pyproject.toml b/pyproject.toml index a0558e64..3c15bcf1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -218,6 +218,8 @@ max-complexity = 10 "agentflow/adapters/llm/openai_responses_converter.py" = ["PLR0912", "PLR0915"] "agentflow/graph/agent.py" = ["PLR0912", "PLR0913"] "agentflow/runtime/publisher/otel_publisher.py" = ["PLR0912"] +# Type dispatchers: one return per supported annotation reads better as a flat table. +"agentflow/core/graph/tool_node/schema.py" = ["PLR0911"] "agentflow/qa/evaluation/reporters/_html_css.py" = ["E501"] "agentflow/qa/evaluation/reporters/_html_js.py" = ["E501"] "agentflow/qa/evaluation/reporters/_html_template.py" = ["E501", "PLR0913"] diff --git a/tests/graph/test_tool_node.py b/tests/graph/test_tool_node.py index 6ce10f59..0c8c4383 100644 --- a/tests/graph/test_tool_node.py +++ b/tests/graph/test_tool_node.py @@ -132,12 +132,13 @@ def test_annotation_to_schema_complex(self): """Test annotation to schema conversion for complex types.""" from typing import Literal + # A nested item schema must not inherit the parameter's default, and a null + # default is dropped entirely since `required` already conveys optionality. schema = ToolNode._annotation_to_schema(list[str], None) - expected = {"type": "array", "items": {"type": "string", "default": None}, "default": None} - assert schema == expected + assert schema == {"type": "array", "items": {"type": "string"}} schema = ToolNode._annotation_to_schema(Literal["a", "b", "c"], None) - assert schema == {"type": "string", "enum": ["a", "b", "c"], "default": None} + assert schema == {"type": "string", "enum": ["a", "b", "c"]} @pytest.mark.asyncio async def test_invoke_local_tool_success(self): diff --git a/tests/graph/test_tool_node_helpers.py b/tests/graph/test_tool_node_helpers.py index a0ab6ac1..1ffab54e 100644 --- a/tests/graph/test_tool_node_helpers.py +++ b/tests/graph/test_tool_node_helpers.py @@ -1,4 +1,8 @@ +import enum +from datetime import date, datetime, time +from decimal import Decimal from pathlib import Path +from uuid import UUID from agentflow.core.graph.tool_node._helpers import ( _as_bool, @@ -7,6 +11,10 @@ ) +class _Priority(str, enum.Enum): + HIGH = "high" + + class _ResourceDump: def model_dump(self): return { @@ -42,6 +50,66 @@ def test_safe_serialize_falls_back_to_string_when_not_serializable(): assert "content" in result +class TestSafeSerializeScalarFormatting: + """A tool returning a datetime must reach the model as a readable date. + + JSON cannot hold a datetime, so the whole return value used to collapse into a + Python repr string: ``{"content": "{'echo': datetime.datetime(2026, 1, 15, 9, 30)}"}``. + The structure must survive, with only the scalar leaves rendered as text. + """ + + def test_datetime_in_dict_keeps_structure_and_is_iso(self): + out = _safe_serialize({"echo": datetime(2026, 1, 15, 9, 30), "ok": True}) + assert out == {"echo": "2026-01-15T09:30:00", "ok": True} + + def test_bare_datetime_is_wrapped_as_content(self): + assert _safe_serialize(datetime(2026, 1, 15, 9, 30)) == { + "content": "2026-01-15T09:30:00" + } + + def test_all_scalar_types_render_as_text(self): + out = _safe_serialize( + { + "when": datetime(2026, 1, 15, 9, 30), + "day": date(2026, 1, 15), + "at": time(9, 30), + "ref": UUID("12345678-1234-5678-1234-567812345678"), + "path": Path("/data/x"), + "amount": Decimal("10.50"), + "blob": b"hello", + "tag": _Priority.HIGH, + } + ) + assert out == { + "when": "2026-01-15T09:30:00", + "day": "2026-01-15", + "at": "09:30:00", + "ref": "12345678-1234-5678-1234-567812345678", + "path": "/data/x", + "amount": "10.50", + "blob": "hello", + "tag": "high", + } + + def test_nested_containers_are_walked(self): + out = _safe_serialize({"rows": [{"when": datetime(2026, 1, 15, 9, 30)}]}) + assert out == {"rows": [{"when": "2026-01-15T09:30:00"}]} + + def test_sets_become_lists(self): + out = _safe_serialize({"tags": {"a"}}) + assert out == {"tags": ["a"]} + + def test_plain_json_values_are_untouched(self): + payload = {"a": 1, "b": "x", "c": [1, 2], "d": None, "e": True} + assert _safe_serialize(payload) == payload + + def test_unknown_object_inside_a_dict_still_falls_back(self): + """The formatter must not swallow genuinely unserializable payloads.""" + out = _safe_serialize({"obj": _NoJsonNoDump()}) + assert out["type"] == "fallback" + assert "" in out["content"] + + def test_as_bool_handles_native_and_string_values(): truthy = {"true", "1", "yes"} assert _as_bool(True, truthy) is True diff --git a/tests/graph/test_tool_schema_objects.py b/tests/graph/test_tool_schema_objects.py new file mode 100644 index 00000000..07426288 --- /dev/null +++ b/tests/graph/test_tool_schema_objects.py @@ -0,0 +1,501 @@ +"""Tests for nested-object tool parameter schemas and argument coercion. + +Covers the regression where any structured parameter (pydantic model, dataclass, +dict, enum) was advertised to the model as ``{"type": "string"}``, forcing it to +hand-serialize JSON into a string argument that frequently came back malformed. +""" + +import dataclasses +import enum +import json +import typing as t +from datetime import date, datetime, time +from decimal import Decimal +from pathlib import Path +from uuid import UUID + +import pytest +from pydantic import BaseModel, Field + +from agentflow.core.graph import ToolNode +from agentflow.core.graph.tool_node import UnsupportedToolParameterError +from agentflow.utils import tool + + +class Priority(str, enum.Enum): + LOW = "low" + HIGH = "high" + + +class Level(enum.IntEnum): + ONE = 1 + TWO = 2 + + +class Address(BaseModel): + city: str + zip_code: str = Field(description="postal code") + + +class Person(BaseModel): + name: str + address: Address + priority: Priority + nickname: str | None = None + tags: list[str] = Field(default_factory=list) + + +class TreeNode(BaseModel): + """Self-referential model; inlining must terminate since there is no $ref.""" + + value: str + children: list["TreeNode"] = Field(default_factory=list) + + +@dataclasses.dataclass +class DcInner: + note: str + + +@dataclasses.dataclass +class DcOuter: + city: str + priority: Priority + inner: DcInner + count: int = 0 + + +def props_for(fn) -> dict: + """Generated properties for a single tool function.""" + return ToolNode([fn]).get_local_tool()[0]["function"]["parameters"]["properties"] + + +class TestObjectSchemas: + def test_pydantic_model_becomes_object_schema(self): + def save(address: Address) -> str: + """Save.""" + return "" + + assert props_for(save)["address"] == { + "type": "object", + "properties": { + "city": {"type": "string"}, + "zip_code": {"type": "string", "description": "postal code"}, + }, + "required": ["city", "zip_code"], + } + + def test_nested_model_is_inlined_recursively(self): + def save(person: Person) -> str: + """Save.""" + return "" + + schema = props_for(save)["person"] + assert schema["properties"]["address"]["type"] == "object" + assert schema["properties"]["address"]["properties"]["city"] == {"type": "string"} + assert schema["properties"]["priority"] == { + "type": "string", + "enum": ["low", "high"], + } + assert schema["properties"]["tags"] == {"type": "array", "items": {"type": "string"}} + # Optional and default-factory fields are not required. + assert schema["required"] == ["name", "address", "priority"] + + def test_dataclass_becomes_object_schema(self): + def save(outer: DcOuter) -> str: + """Save.""" + return "" + + schema = props_for(save)["outer"] + assert schema["properties"]["inner"] == { + "type": "object", + "properties": {"note": {"type": "string"}}, + "required": ["note"], + } + assert schema["properties"]["count"] == {"type": "integer", "default": 0} + assert schema["required"] == ["city", "priority", "inner"] + + def test_list_of_models(self): + def save(people: list[Address]) -> str: + """Save.""" + return "" + + schema = props_for(save)["people"] + assert schema["type"] == "array" + assert schema["items"]["type"] == "object" + assert "city" in schema["items"]["properties"] + + def test_dict_variants(self): + def save(meta: dict, typed: dict[str, int], anyval: dict[str, t.Any]) -> str: + """Save.""" + return "" + + schema = props_for(save) + assert schema["meta"] == {"type": "object"} + assert schema["typed"] == { + "type": "object", + "additionalProperties": {"type": "integer"}, + } + assert schema["anyval"] == {"type": "object"} + + def test_enum_typing_follows_member_values(self): + def save(priority: Priority, level: Level) -> str: + """Save.""" + return "" + + schema = props_for(save) + assert schema["priority"] == {"type": "string", "enum": ["low", "high"]} + assert schema["level"] == {"type": "integer", "enum": [1, 2]} + + def test_scalar_stdlib_types(self): + def save(when: datetime, ident: UUID) -> str: + """Save.""" + return "" + + schema = props_for(save) + assert schema["when"] == {"type": "string", "format": "date-time"} + assert schema["ident"] == {"type": "string", "format": "uuid"} + + def test_optional_collapses_and_drops_null_default(self): + def save(note: str | None = None) -> str: + """Save.""" + return "" + + assert props_for(save)["note"] == {"type": "string"} + + def test_no_unportable_keywords_anywhere(self): + """No $ref/$defs/anyOf/allOf/title reaches the provider.""" + + def save(person: Person, outer: DcOuter) -> str: + """Save.""" + return "" + + raw = json.dumps(ToolNode([save]).get_local_tool()) + for keyword in ("$defs", "$ref", "$schema", "allOf", "anyOf", "title", "discriminator"): + assert f'"{keyword}"' not in raw + + def test_self_referential_model_terminates(self): + def save(node: TreeNode) -> str: + """Save.""" + return "" + + schema = props_for(save)["node"] + assert schema["properties"]["value"] == {"type": "string"} + assert schema["properties"]["children"]["items"] == {"type": "object"} + + def test_stringized_annotations_are_resolved(self): + """PEP 563 turns annotations into strings; they must still resolve.""" + + def save(count: "int", address: "Address") -> str: + """Save.""" + return "" + + schema = props_for(save) + assert schema["count"] == {"type": "integer"} + assert schema["address"]["type"] == "object" + + +class TestUnsupportedParameters: + def test_typeddict_is_rejected(self): + class Plan(t.TypedDict): + title: str + + def save(plan: Plan) -> str: + """Save.""" + return "" + + with pytest.raises(UnsupportedToolParameterError, match="TypedDict"): + ToolNode([save]).get_local_tool() + + def test_multi_type_union_is_rejected(self): + def save(value: int | str) -> str: + """Save.""" + return "" + + with pytest.raises(UnsupportedToolParameterError, match="union of multiple types"): + ToolNode([save]).get_local_tool() + + def test_tuple_is_rejected_with_list_hint(self): + def save(pair: tuple[int, int]) -> str: + """Save.""" + return "" + + with pytest.raises(UnsupportedToolParameterError, match=r"use list\[\.\.\.\]"): + ToolNode([save]).get_local_tool() + + def test_arbitrary_class_is_rejected(self): + class Widget: + pass + + def save(widget: Widget) -> str: + """Save.""" + return "" + + with pytest.raises(UnsupportedToolParameterError, match="not supported"): + ToolNode([save]).get_local_tool() + + def test_unresolvable_string_annotation_is_rejected(self): + def save(thing: "NoSuchTypeAnywhere") -> str: # noqa: F821 + """Save.""" + return "" + + with pytest.raises(UnsupportedToolParameterError, match="still a string"): + ToolNode([save]).get_local_tool() + + def test_error_names_the_tool_and_parameter(self): + def save(pair: tuple[int, int]) -> str: + """Save.""" + return "" + + with pytest.raises(UnsupportedToolParameterError) as exc: + ToolNode([save]).get_local_tool() + assert "'save'" in str(exc.value) + assert "'pair'" in str(exc.value) + + def test_unsupported_error_is_a_type_error(self): + """Existing `except TypeError` handlers around registration keep working.""" + assert issubclass(UnsupportedToolParameterError, TypeError) + + +class TestParametersOverride: + def test_explicit_schema_bypasses_generation(self): + @tool(parameters={"type": "object", "properties": {"raw": {"type": "string"}}}) + def save(payload: dict[str, t.Any]) -> str: + """Save.""" + return "" + + params = ToolNode([save]).get_local_tool()[0]["function"]["parameters"] + assert params == {"type": "object", "properties": {"raw": {"type": "string"}}} + + def test_explicit_schema_rescues_an_unsupported_type(self): + class Plan(t.TypedDict): + title: str + + @tool(parameters={"type": "object", "properties": {"plan": {"type": "object"}}}) + def save(plan: Plan) -> str: + """Save.""" + return "" + + params = ToolNode([save]).get_local_tool()[0]["function"]["parameters"] + assert params["properties"]["plan"] == {"type": "object"} + + def test_non_dict_override_is_rejected(self): + with pytest.raises(ValueError, match="must be a JSON Schema dict"): + + @tool(parameters="not a dict") # type: ignore[arg-type] + def save(x: int) -> str: + """Save.""" + return "" + + +class TestArgumentCoercion: + def _prepare(self, fn, args: dict) -> dict: + return ToolNode([fn])._prepare_input_data_tool(fn, fn.__name__, args, {}) + + def test_dict_becomes_model_instance(self): + def save(address: Address) -> str: + """Save.""" + return "" + + out = self._prepare(save, {"address": {"city": "NYC", "zip_code": "10001"}}) + assert isinstance(out["address"], Address) + assert out["address"].city == "NYC" + + def test_dataclass_fields_are_fully_converted(self): + """A dataclass constructor would leave enum/nested/int fields as raw JSON.""" + + def save(outer: DcOuter) -> str: + """Save.""" + return "" + + out = self._prepare( + save, + { + "outer": { + "city": "NYC", + "priority": "high", + "inner": {"note": "x"}, + "count": "5", + } + }, + ) + outer = out["outer"] + assert isinstance(outer, DcOuter) + assert outer.priority is Priority.HIGH + assert isinstance(outer.inner, DcInner) + assert outer.count == 5 + + def test_enum_argument_becomes_enum_member(self): + def save(priority: Priority) -> str: + """Save.""" + return "" + + assert self._prepare(save, {"priority": "low"})["priority"] is Priority.LOW + + def test_list_of_models_is_coerced(self): + def save(people: list[Address]) -> str: + """Save.""" + return "" + + out = self._prepare(save, {"people": [{"city": "NYC", "zip_code": "1"}]}) + assert all(isinstance(p, Address) for p in out["people"]) + + def test_hand_serialized_json_string_still_works(self): + """Backward compatibility with models trained by the old string schema.""" + + def save(address: Address) -> str: + """Save.""" + return "" + + payload = json.dumps({"city": "NYC", "zip_code": "10001"}) + out = self._prepare(save, {"address": payload}) + assert isinstance(out["address"], Address) + + def test_already_correct_instance_passes_through(self): + def save(address: Address) -> str: + """Save.""" + return "" + + original = Address(city="NYC", zip_code="10001") + assert self._prepare(save, {"address": original})["address"] == original + + def test_primitives_are_untouched(self): + def save(a: int, b: str, c: dict) -> str: + """Save.""" + return "" + + out = self._prepare(save, {"a": 1, "b": "x", "c": {"k": "v"}}) + assert out == {"a": 1, "b": "x", "c": {"k": "v"}} + + def test_invalid_payload_raises_a_readable_type_error(self): + def save(address: Address) -> str: + """Save.""" + return "" + + with pytest.raises(TypeError) as exc: + self._prepare(save, {"address": {"city": "NYC"}}) + message = str(exc.value) + assert "'address'" in message + assert "'save'" in message + assert "zip_code" in message + + def test_optional_model_accepts_none(self): + def save(address: Address | None = None) -> str: + """Save.""" + return "" + + assert self._prepare(save, {"address": None})["address"] is None + + +class TestScalarCoercion: + """A scalar the schema advertises as a formatted string must arrive as its real type. + + JSON has no date/uuid/decimal type, so these go on the wire as strings with a + ``format`` hint. Whatever the schema promises the model, the tool body must receive + the annotated type -- otherwise ``when.year`` raises ``AttributeError`` on a ``str``. + """ + + def _prepare(self, fn, args: dict) -> dict: + return ToolNode([fn])._prepare_input_data_tool(fn, fn.__name__, args, {}) + + def test_top_level_datetime_becomes_datetime(self): + def schedule(when: datetime) -> str: + """Schedule.""" + return "" + + out = self._prepare(schedule, {"when": "2026-01-15T09:30:00"}) + assert out["when"] == datetime(2026, 1, 15, 9, 30) + + def test_every_advertised_scalar_round_trips(self): + """Each type in schema._SCALARS must be coercible; the two lists must not drift.""" + + def take( + when: datetime, + day: date, + at: time, + ref: UUID, + path: Path, + amount: Decimal, + blob: bytes, + ) -> str: + """Take.""" + return "" + + out = self._prepare( + take, + { + "when": "2026-01-15T09:30:00", + "day": "2026-01-15", + "at": "09:30:00", + "ref": "12345678-1234-5678-1234-567812345678", + "path": "/data/x", + "amount": "10.50", + "blob": "hello", + }, + ) + assert out["when"] == datetime(2026, 1, 15, 9, 30) + assert out["day"] == date(2026, 1, 15) + assert out["at"] == time(9, 30) + assert out["ref"] == UUID("12345678-1234-5678-1234-567812345678") + assert out["path"] == Path("/data/x") + assert out["amount"] == Decimal("10.50") + assert out["blob"] == b"hello" + + def test_optional_and_list_scalars_are_coerced(self): + def take(when: datetime | None = None, days: list[date] | None = None) -> str: + """Take.""" + return "" + + out = self._prepare(take, {"when": "2026-01-15T09:30:00", "days": ["2026-01-15"]}) + assert out["when"] == datetime(2026, 1, 15, 9, 30) + assert out["days"] == [date(2026, 1, 15)] + + def test_scalar_inside_a_model_still_works(self): + """The nested path already worked; guard it against regressions.""" + + class Booking(BaseModel): + label: str + when: datetime + + def save(booking: Booking) -> str: + """Save.""" + return "" + + out = self._prepare(save, {"booking": {"label": "x", "when": "2026-01-15T09:30:00"}}) + assert out["booking"].when == datetime(2026, 1, 15, 9, 30) + + def test_already_correct_type_passes_through(self): + def schedule(when: datetime) -> str: + """Schedule.""" + return "" + + original = datetime(2026, 1, 15, 9, 30) + assert self._prepare(schedule, {"when": original})["when"] == original + + def test_optional_scalar_accepts_none(self): + def schedule(when: datetime | None = None) -> str: + """Schedule.""" + return "" + + assert self._prepare(schedule, {"when": None})["when"] is None + + def test_unparseable_scalar_raises_a_readable_type_error(self): + def schedule(when: datetime) -> str: + """Schedule.""" + return "" + + with pytest.raises(TypeError) as exc: + self._prepare(schedule, {"when": "not a date"}) + message = str(exc.value) + assert "'when'" in message + assert "'schedule'" in message + + def test_plain_primitives_remain_untouched(self): + """Widening coercion to scalars must not start coercing int/float/str/bool.""" + + def save(a: int, b: str, c: float, d: bool) -> str: + """Save.""" + return "" + + out = self._prepare(save, {"a": "5", "b": "x", "c": "1.5", "d": "yes"}) + assert out == {"a": "5", "b": "x", "c": "1.5", "d": "yes"} From 47e3d52b3135835c87fc383d29d9053d050491c9 Mon Sep 17 00:00:00 2001 From: Shudipto Trafder Date: Mon, 3 Aug 2026 18:00:50 +0600 Subject: [PATCH 2/2] feat: enhance serialization and validation for tool parameters and nested objects --- agentflow/core/graph/tool_node/_helpers.py | 43 ++++++++++++----- agentflow/core/graph/tool_node/coercion.py | 56 ++++++++++++++++------ tests/graph/test_tool_node_helpers.py | 12 +++-- tests/graph/test_tool_schema_objects.py | 33 +++++++++++++ 4 files changed, 114 insertions(+), 30 deletions(-) diff --git a/agentflow/core/graph/tool_node/_helpers.py b/agentflow/core/graph/tool_node/_helpers.py index ff1dd5cf..e09a5a21 100644 --- a/agentflow/core/graph/tool_node/_helpers.py +++ b/agentflow/core/graph/tool_node/_helpers.py @@ -16,6 +16,33 @@ _ERROR_TRUE: set[str] = {"true", "1", "yes", "error", "failed", "failure"} +def _stable_members(obj: set | frozenset) -> list: + """Order a set so the same result serializes identically on every run. + + Set iteration order is not stable across processes, which would make tool output + flap between runs and tests flaky. + """ + try: + return sorted(obj) + except TypeError: + # Mixed or uncomparable members: order is unavoidably arbitrary here, but + # losing the values entirely would be worse. + return list(obj) + + +# Rendering rules for the scalar types a tool can return but JSON cannot hold. Ordered: +# the first matching entry wins. Decimal renders as str, not float, so a money value +# does not lose precision on the way to the model. +_JSON_ENCODERS: tuple[tuple[t.Any, t.Callable[[t.Any], t.Any]], ...] = ( + ((dt.datetime, dt.date, dt.time), lambda o: o.isoformat()), + ((uuid.UUID, pathlib.PurePath), str), + (decimal.Decimal, str), + (enum.Enum, lambda o: o.value), + ((set, frozenset), _stable_members), + ((bytes, bytearray), lambda o: o.decode("utf-8", errors="replace")), +) + + def _json_default(obj: t.Any) -> t.Any: """Render the scalar types a tool can return but JSON cannot hold. @@ -28,19 +55,9 @@ def _json_default(obj: t.Any) -> t.Any: through to its existing repr fallback. Unknown objects must not be silently stringified here, or a genuinely unserializable payload would look clean. """ - if isinstance(obj, dt.datetime | dt.date | dt.time): - return obj.isoformat() - if isinstance(obj, uuid.UUID | pathlib.PurePath): - return str(obj) - if isinstance(obj, decimal.Decimal): - # str, not float: a money value must not lose precision on the way to the model. - return str(obj) - if isinstance(obj, enum.Enum): - return obj.value - if isinstance(obj, set | frozenset): - return list(obj) - if isinstance(obj, bytes | bytearray): - return obj.decode("utf-8", errors="replace") + for types_, encode in _JSON_ENCODERS: + if isinstance(obj, types_): + return encode(obj) raise TypeError(f"Object of type {type(obj).__name__} is not JSON serializable") diff --git a/agentflow/core/graph/tool_node/coercion.py b/agentflow/core/graph/tool_node/coercion.py index e685f32a..daff5f57 100644 --- a/agentflow/core/graph/tool_node/coercion.py +++ b/agentflow/core/graph/tool_node/coercion.py @@ -85,6 +85,10 @@ def _maybe_json(value: t.Any) -> t.Any: ``{"type": "string"}`` schema, hand-serialize nested objects into a single string. Only strings that clearly look like a JSON object or array are decoded, so an enum member such as ``"5"`` is never turned into an int. + + This is a *fallback*, applied only after the raw value has already been rejected. + Decoding up front would corrupt any value the annotation would have accepted as + it stands, such as an enum member whose value is literally ``"{}"``. """ if not isinstance(value, str): return value @@ -97,6 +101,29 @@ def _maybe_json(value: t.Any) -> t.Any: return value +def _validate(annotation: t.Any, value: t.Any) -> t.Any: + """Validate against the annotation, tolerating annotations the cache cannot key.""" + try: + adapter = _adapter(annotation) + except TypeError: + # Unhashable annotation could not be cached; build the adapter directly. + adapter = TypeAdapter(annotation) + return adapter.validate_python(value) + + +def _readable_error( + exc: ValidationError, + tool_name: str, + param_name: str, +) -> TypeError: + """Flatten pydantic field errors into a message that can go back to the model.""" + details = "; ".join( + f"{'.'.join(str(p) for p in err['loc']) or param_name}: {err['msg']}" + for err in exc.errors() + ) + return TypeError(f"Invalid argument {param_name!r} for tool {tool_name!r}: {details}") + + def coerce_tool_argument( value: t.Any, annotation: t.Any, @@ -132,18 +159,19 @@ def coerce_tool_argument( if not needs: return value - candidate = _maybe_json(value) - try: - return _adapter(annotation).validate_python(candidate) - except TypeError: - # Unhashable annotation could not be cached; build the adapter directly. - return TypeAdapter(annotation).validate_python(candidate) - except ValidationError as exc: - details = "; ".join( - f"{'.'.join(str(p) for p in err['loc']) or param_name}: {err['msg']}" - for err in exc.errors() - ) - raise TypeError( - f"Invalid argument {param_name!r} for tool {tool_name!r}: {details}" - ) from exc + return _validate(annotation, value) + except ValidationError as raw_error: + # Only now consider that the model may have hand-serialized a nested object + # into a string. Trying this first would decode a value the annotation was + # willing to accept, and report a contradictory error about it. + decoded = _maybe_json(value) + if decoded is value: + raise _readable_error(raw_error, tool_name, param_name) from raw_error + + try: + return _validate(annotation, decoded) + except ValidationError as decoded_error: + # The decoded payload is what the model meant, so its field errors are the + # actionable ones ("zip_code: Field required", not "expected a dict"). + raise _readable_error(decoded_error, tool_name, param_name) from decoded_error diff --git a/tests/graph/test_tool_node_helpers.py b/tests/graph/test_tool_node_helpers.py index 1ffab54e..0ba1b1e1 100644 --- a/tests/graph/test_tool_node_helpers.py +++ b/tests/graph/test_tool_node_helpers.py @@ -95,9 +95,15 @@ def test_nested_containers_are_walked(self): out = _safe_serialize({"rows": [{"when": datetime(2026, 1, 15, 9, 30)}]}) assert out == {"rows": [{"when": "2026-01-15T09:30:00"}]} - def test_sets_become_lists(self): - out = _safe_serialize({"tags": {"a"}}) - assert out == {"tags": ["a"]} + def test_sets_become_sorted_lists(self): + """Set iteration order is not stable, so unordered output would flap per run.""" + out = _safe_serialize({"tags": {"c", "a", "b"}}) + assert out == {"tags": ["a", "b", "c"]} + + def test_set_of_uncomparable_values_still_serializes(self): + """Sorting is best-effort; mixed types must not crash the whole result.""" + out = _safe_serialize({"tags": {1, "a"}}) + assert sorted(map(str, out["tags"])) == ["1", "a"] def test_plain_json_values_are_untouched(self): payload = {"a": 1, "b": "x", "c": [1, 2], "d": None, "e": True} diff --git a/tests/graph/test_tool_schema_objects.py b/tests/graph/test_tool_schema_objects.py index 07426288..e03965e5 100644 --- a/tests/graph/test_tool_schema_objects.py +++ b/tests/graph/test_tool_schema_objects.py @@ -32,6 +32,13 @@ class Level(enum.IntEnum): TWO = 2 +class Template(str, enum.Enum): + """Members that look like JSON, to catch over-eager decoding of raw arguments.""" + + EMPTY_OBJ = "{}" + EMPTY_ARR = "[]" + + class Address(BaseModel): city: str zip_code: str = Field(description="postal code") @@ -351,6 +358,32 @@ def save(address: Address) -> str: out = self._prepare(save, {"address": payload}) assert isinstance(out["address"], Address) + def test_json_looking_string_the_annotation_accepts_is_not_decoded(self): + """A value the declared type accepts must never be JSON-decoded out from under it. + + Decoding every ``{``/``[`` string up front turned a legitimate enum member into + a dict and then rejected it, with a message that contradicted itself: + ``Input should be '{}'`` when the input *was* ``"{}"``. + """ + + def render(template: Template) -> str: + """Render.""" + return "" + + assert self._prepare(render, {"template": "{}"})["template"] is Template.EMPTY_OBJ + assert self._prepare(render, {"template": "[]"})["template"] is Template.EMPTY_ARR + + def test_decode_fallback_reports_the_decoded_error(self): + """When a JSON string does decode, its validation error is the useful one.""" + + def save(address: Address) -> str: + """Save.""" + return "" + + with pytest.raises(TypeError) as exc: + self._prepare(save, {"address": json.dumps({"city": "NYC"})}) + assert "zip_code" in str(exc.value) + def test_already_correct_instance_passes_through(self): def save(address: Address) -> str: """Save."""