NVIDIA / NVIDIA/NeMo-Agent-Toolkit
ADK tool wrapper drops optional-parameter defaults, so every field is declared required
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 2.6k
- Forks
- 762
- Avg merge
- 21h 28m
- Merged PRs (30d)
- 27
Description
Version
1.8.0 (also on develop @ 2d2e342)
Which installation method(s) does this occur on?
PyPi
Describe the bug.
google_adk_tool_wrapper builds the wrapped callable's __signature__ with
inspect.Parameter(name, POSITIONAL_OR_KEYWORD, annotation=...) and no default. Every
input-schema field therefore becomes a required parameter, even when the schema says it is
optional with a default.
The code, packages/nvidia_nat_adk/src/nat/plugins/adk/tool_wrapper.py on develop @ 2d2e342:
inspect.Parameter(
param_name,
inspect.Parameter.POSITIONAL_OR_KEYWORD,
annotation=resolve_type(param_annotation),
)
Google ADK derives its function declaration from that signature, so it tells the model that
optional parameters are required. The model must then invent a value for each one on every
call, and the tool's own defaults never apply. For a tool like
search(query, max_results=5, region="global") the model is forced to supply max_results
and region it was never asked about - quietly changing the tool's behaviour rather than
failing loudly.
Expected: a field that is optional in the input schema is optional in the ADK declaration.
The information needed is already in hand at that point. #2166 changed this same loop to
iterate Pydantic's model_fields, and the FieldInfo it now yields carries both default
and is_required() - so the fix reads two more attributes off the object the loop already
has.
One ordering detail for whoever fixes it: a parameter with a default cannot precede one
without, and Pydantic preserves declaration order, which may interleave required and optional
fields. The parameters need sorting (required first) or inspect.Signature raises
ValueError. NAT invokes the wrapped function with keyword arguments, so reordering is safe.
Related to #2161 / #2166 - same function, but a separate defect. That crash masked this one;
with #2166 merged it is now directly observable on develop.
Suggested fix, against develop @ 2d2e342. A helper that reads the default off the
FieldInfo the loop already has:
def _field_default(field_info: Any) -> Any:
"""Return a Pydantic field's default, or ``inspect.Parameter.empty`` if required."""
if field_info.is_required():
return inspect.Parameter.empty
if field_info.default_factory is not None:
try:
return field_info.default_factory()
except TypeError:
# Factories that need validated data cannot be evaluated here.
return None
return field_info.default
Then carry the default through into the signature:
params: list[inspect.Parameter] = []
if input_schema is not None:
model_fields = getattr(input_schema, "model_fields", None)
if model_fields is not None:
field_items = ((n, f.annotation, _field_default(f)) for n, f in model_fields.items())
else:
field_items = ((n, a, getattr(input_schema, n, inspect.Parameter.empty))
for n, a in getattr(input_schema, "__annotations__", {}).items())
for param_name, param_annotation, param_default in field_items:
params.append(
inspect.Parameter(
param_name,
inspect.Parameter.POSITIONAL_OR_KEYWORD,
annotation=resolve_type(param_annotation),
default=param_default,
))
# A parameter with a default cannot precede one without, and Pydantic
# preserves declaration order, which may interleave them. NAT invokes the
# wrapped function with keyword arguments, so reordering is safe.
params.sort(key=lambda p: p.default is not inspect.Parameter.empty)
setattr(func_to_wrap, "__signature__", inspect.Signature(parameters=params))
default_factory is evaluated where it can be, and falls back to None for factories that
need validated data, rather than leaking a factory object into the signature. The non-Pydantic
branch keeps working - a class attribute is used as the default if one exists, otherwise the
field stays required, which matches today's behaviour. The sort is what stops
inspect.Signature raising ValueError when a schema interleaves required and optional
fields.
With this applied the reproducer above prints:
signature : (query: str, max_results: int = 5, region: str = 'global')
ADK required : ['query']
Minimum reproducible example
import asyncio
from nat.builder.builder import Builder
from nat.builder.framework_enum import LLMFrameworkEnum
from nat.builder.function_info import FunctionInfo
from nat.builder.workflow_builder import WorkflowBuilder
from nat.cli.register_workflow import register_function
from nat.data_models.function import FunctionBaseConfig
from nat.runtime.loader import PluginTypes
from nat.runtime.loader import discover_and_register_plugins
class SearchConfig(FunctionBaseConfig, name="search_tool"):
pass
@register_function(config_type=SearchConfig)
async def search_tool(config: SearchConfig, builder: Builder):
async def search(query: str, max_results: int = 5, region: str = "global") -> str:
"""Search. query is required; max_results and region are optional."""
return f"{query}|{max_results}|{region}"
yield FunctionInfo.from_fn(search, description="Search with optional filters.")
async def main():
discover_and_register_plugins(PluginTypes.ALL)
async with WorkflowBuilder() as builder:
await builder.add_function("search", SearchConfig())
schema = (await builder.get_function("search")).input_schema
print("schema required:", sorted(n for n, f in schema.model_fields.items() if f.is_required()))
tool = await builder.get_tool("search", wrapper_type=LLMFrameworkEnum.ADK)
print("signature :", tool.func.__signature__)
print("ADK required :", sorted(tool._get_declaration().parameters.required or []))
asyncio.run(main())
Relevant log output
schema required: ['query']
signature : (query: str, max_results: int, region: str)
ADK required : ['max_results', 'query', 'region']
Other/Misc.
max_results and region are optional in the schema and required in the declaration. With the defaults preserved the same script prints:
signature : (query: str, max_results: int = 5, region: str = 'global')
ADK required : ['query']
Run it against develop (or any build including #2166). On the released 1.8.0 wheel the KeyError from #2161 fires first and masks this.
Environment:
nvidia-nat-core 1.8.0
nvidia-nat-adk 1.8.0
google-adk 1.38.0
python 3.11.9
OS Windows 11
Code of Conduct
- I agree to follow the NeMo Agent Toolkit Code of Conduct
- I have searched the open bugs and have found no duplicates for this bug report
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in packages/nvidia_nat_adk/src/nat/plugins/adk/tool_wrapper.py and inspect the signature-building loop over Pydantic model_fields. Run the supplied minimal reproducer on develop, then preserve field defaults and required status in the ADK declaration, including default factories and interleaved required fields; the reproducer should report only query as required.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100