Agentling/feature/tools#2
Conversation
Turn a plain function into a model-ready tool. @tool builds a JSON Schema from type hints (Optional, Literal -> enum, list[str] -> array+items) and a Google-style docstring; Tool.call validates arguments against that schema before running, raising ToolCallError on bad input. - Tool base class (name/description/parameters, async forward, to_schema, call) - @tool decorator wrapping sync or async functions - _build_schema (hints -> JSON Schema) + _parse_docstring (per-arg descriptions) - schema hardening: additionalProperties:false, default exposure, and a param.kind guard rejecting *args/**kwargs/positional-only tools - arg validation: required/unknown/type/enum + non-dict guard -> ToolCallError - FinalAnswerTool for explicit termination - JsonSchema vs ToolSpec type split (parameters object vs full function spec)
PR Summary by QodoAdd JSON-Schema-backed tools framework, tests, and CI pipeline
AI Description
Diagram
High-Level Assessment
Files changed (6)
|
Code Review by Qodo
1. Optional default schema mismatch
|
| # Treat Optional[X] as "the field may be omitted". If the field is supplied, | ||
| # it must still match X. Null is intentionally not advertised as valid. | ||
| if origin is Union or origin is types.UnionType: | ||
| non_none = [arg for arg in args if arg is not type(None)] | ||
| if len(non_none) == 1: | ||
| return _schema_for_type(non_none[0]) | ||
|
|
||
| return {"type": "string"} | ||
|
|
||
| # list[str] becomes an array with typed items. Bare list becomes an array | ||
| # without item constraints. | ||
| if origin is list: | ||
| if args: | ||
| return { | ||
| "type": "array", | ||
| "items": _schema_for_type(args[0]), | ||
| } | ||
|
|
||
| return {"type": "array"} | ||
|
|
||
| # dict[...] is represented as an object without per-key constraints. | ||
| if origin is dict: | ||
| return {"type": "object"} | ||
|
|
||
| # For other generic aliases, fall back to the origin's base type. | ||
| if origin is not None: | ||
| return {"type": _JSON_TYPES.get(origin, "string")} | ||
|
|
||
| # Plain types: str, int, float, bool, list, dict, etc. | ||
| return {"type": _JSON_TYPES.get(python_type, "string")} | ||
|
|
||
|
|
||
| _ARGS_HEADERS = { | ||
| "args", | ||
| "arguments", | ||
| "parameters", | ||
| "keyword args", | ||
| "keyword arguments", | ||
| } | ||
|
|
||
| _HEADER_RE = re.compile(r"^([A-Za-z][A-Za-z ]*):$") | ||
| _ARG_RE = re.compile(r"^(\w+)\s*(?:\([^)]*\))?:\s*(.*)$") | ||
|
|
||
|
|
||
| def _parse_docstring(func: Callable[..., Any]) -> tuple[str, dict[str, str]]: | ||
| """Extract a summary and argument descriptions from a docstring. | ||
|
|
||
| This is a pragmatic Google-style parser, not a complete docstring parser. | ||
| It recognizes Args/Arguments/Parameters sections and stops parsing argument | ||
| descriptions when another section header is encountered. | ||
| """ | ||
|
|
||
| doc = inspect.getdoc(func) or "" | ||
| summary_lines: list[str] = [] | ||
| arg_docs: dict[str, str] = {} | ||
|
|
||
| section: str | None = None | ||
| current_arg: str | None = None | ||
|
|
||
| for raw in doc.splitlines(): | ||
| stripped = raw.strip() | ||
|
|
||
| header = _HEADER_RE.match(stripped) | ||
| if header: | ||
| key = header.group(1).strip().lower() | ||
| section = "args" if key in _ARGS_HEADERS else "other" | ||
| current_arg = None | ||
| continue | ||
|
|
||
| if section is None: | ||
| summary_lines.append(stripped) | ||
|
|
||
| elif section == "args": | ||
| match = _ARG_RE.match(stripped) | ||
|
|
||
| if match: | ||
| current_arg = match.group(1) | ||
| arg_docs[current_arg] = match.group(2).strip() | ||
|
|
||
| elif current_arg and stripped: | ||
| arg_docs[current_arg] += " " + stripped | ||
|
|
||
| # Other sections, such as Returns or Raises, are ignored. | ||
|
|
||
| summary = " ".join(line for line in summary_lines if line).strip() | ||
| return summary, arg_docs | ||
|
|
||
|
|
||
| def _build_schema(func: Callable[..., Any]) -> JsonSchema: | ||
| """Build a JSON Schema parameters object from a function signature.""" | ||
|
|
||
| hints = get_type_hints(func) | ||
| signature = inspect.signature(func) | ||
| _, arg_docs = _parse_docstring(func) | ||
|
|
||
| properties: dict[str, Any] = {} | ||
| required: list[str] = [] | ||
|
|
||
| for name, param in signature.parameters.items(): | ||
| if name in {"self", "cls"}: | ||
| continue | ||
|
|
||
| # Tools are invoked with a JSON object of named arguments. Variadic and | ||
| # positional-only parameters cannot be represented safely, so reject | ||
| # them during tool registration rather than failing during a model run. | ||
| if param.kind in { | ||
| inspect.Parameter.VAR_POSITIONAL, | ||
| inspect.Parameter.VAR_KEYWORD, | ||
| inspect.Parameter.POSITIONAL_ONLY, | ||
| }: | ||
| raise TypeError( | ||
| f"Tool {func.__name__!r} has unsupported parameter {name!r}. " | ||
| "Tools support only normal or keyword-only parameters " | ||
| "(no *args, **kwargs, or positional-only parameters)." | ||
| ) | ||
|
|
||
| schema = _schema_for_type(hints.get(name, str)) | ||
|
|
||
| if name in arg_docs: | ||
| schema["description"] = arg_docs[name] | ||
|
|
||
| if param.default is inspect.Parameter.empty: | ||
| required.append(name) | ||
| else: | ||
| schema["default"] = param.default | ||
|
|
||
| properties[name] = schema |
There was a problem hiding this comment.
1. Optional default schema mismatch 🐞 Bug ≡ Correctness
In agentling.tools, Optional[T]/Union[T, None] is converted to the schema for T, but _build_schema still emits default = None, yielding an internally inconsistent JSON Schema (e.g., type: integer with default: null) that can be rejected by tool-schema validators/providers.
Agent Prompt
### Issue description
`_schema_for_type()` unwraps `Optional[T]` into a schema for `T` (int/string/etc), but `_build_schema()` always copies the Python default into `schema["default"]`, including `None`. This produces a JSON Schema where the declared `type` conflicts with the `default` value (`null`), which can cause provider-side schema validation failures when sending tool specs.
### Issue Context
This happens for any tool parameter annotated as `Optional[...]` (or `T | None`) with a default of `None`.
### Fix Focus Areas
- src/agentling/tools.py[177-184]
- src/agentling/tools.py[293-303]
### Implementation notes
Prefer one of:
1) If `param.default is None`, **omit** the `default` key from the JSON Schema property (keeps schema consistent and still allows omission at call-time because Python will use its default).
2) Alternatively, preserve nullability in schema (e.g., `type: [<type>, "null"]` or `anyOf`) and adjust `_matches_json_type()` / validation accordingly to accept `None`.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
No description provided.