Fixes to detect optional parameters in tool conversion used by "nat mcp serve"

[AnuradhaKaruppiah]

Description

Parameters with default values were incorrectly marked as required in MCP tool schemas. Now checking for PydanticUndefined and properly extracting default values from Pydantic fields to correctly identify optional parameters.

This PR also adds comprehensive unit tests for the tool schema conversion

By Submitting this PR I confirm:

• I am familiar with the Contributing Guidelines.
• We require that all contributors "sign-off" on their commits. This certifies that the contribution is your original work, or you have rights to submit it under the same license, or a compatible license.
  • Any contribution which contains commits that are not Signed-Off will not be accepted.
• When the PR is ready for review, new or existing tests cover these changes.
• When the PR is ready for review, the documentation is up to date with these changes.

Summary by CodeRabbit

• New Features

  • Improved detection and handling of optional parameters and default/default-factory values in tool wrapper generation, with unified validation and consistent invocation behavior for workflows and regular functions.
  • Ensures parameter descriptions, order, and type annotations are preserved in generated wrappers.

• Tests

  • Added comprehensive tests covering optional/default combinations, wrapper signatures, execution paths, and observability propagation.

[coderabbitai]

Walkthrough

Adds is_field_optional(field: FieldInfo) and a sentinel _USE_PYDANTIC_DEFAULT, integrates them into create_function_wrapper to compute parameter defaults and handle Pydantic default_factory cases, and reworks non-chat invocation flow to strip sentinels, always validate via model_validate, and call functions with the validated model or its dumped dict.

Changes

Cohort / File(s)	Summary
Core implementation src/nat/front_ends/mcp/tool_converter.py	Adds _USE_PYDANTIC_DEFAULT sentinel and is_field_optional(field: FieldInfo) -> tuple[bool, Any]. Integrates optional/default logic into create_function_wrapper, uses computed param_default for inspect.Parameter.default, strips sentinel values from kwargs before validation, always calls model_validate, and routes validated inputs to workflows (function.run) or regular functions (acall_invoke with model_input.model_dump()). Adds imports: FieldInfo, PydanticUndefined, Any.
Tests tests/nat/front_ends/mcp/test_tool_converter.py	Adds tests for is_field_optional across multiple Pydantic schemas (required, optional, mixed, None unions), covers default and default_factory handling (via _USE_PYDANTIC_DEFAULT), verifies wrapper parameter defaults/annotations/order, and integration tests for wrapper execution with omitted/provided/None optional parameters and observability propagation.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant Caller as Caller (tool invocation)
participant Wrapper as Generated wrapper
participant Validator as Pydantic model_validate
participant Func as Target function / workflow
Note over Wrapper,Validator: Pre-call: strip _USE_PYDANTIC_DEFAULT from kwargs
Caller->>Wrapper: invoke(kwargs)
Wrapper->>Validator: model_validate(kwargs)
alt validation success & workflow
Validator-->>Wrapper: validated model instance
Wrapper->>Func: function.run(validated_model)
else validation success & regular function
Validator-->>Wrapper: validated model instance
Wrapper->>Func: acall_invoke(**validated_model.model_dump())
end
alt validation error
Validator-->>Wrapper: ValidationError
Wrapper->>Caller: raise/return error
end
Note over Wrapper: ChatRequest path remains compatible with new default handling

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• Review is_field_optional edge cases: default, default_factory, PydanticUndefined, and Optional/Union with None.
• Verify sentinel _USE_PYDANTIC_DEFAULT is consistently handled (creation, stripping, and not leaked into validation).
• Validate changes to create_function_wrapper parameter construction preserve Parameter order, annotations, descriptions.
• Check runtime flow: model validation, workflow vs regular call paths, and observability/context propagation in tests.

Pre-merge checks and finishing touches

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error)

Check name	Status	Explanation	Resolution
Title Check	❌ Error	The PR title "Fixes to detect optional parameters in tool conversion used by 'nat mcp serve'" is fully related to the main change described in the PR objectives—fixing how optional parameters are detected in MCP tool schemas. However, the title has two issues with the stated requirements: it exceeds the ~72 character limit at 78 characters, and it does not use imperative mood (using "Fixes" in third-person form rather than a command verb like "Fix").

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 96.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 129b58b and 25d3263.

📒 Files selected for processing (2)

• src/nat/front_ends/mcp/tool_converter.py (4 hunks)
• tests/nat/front_ends/mcp/test_tool_converter.py (6 hunks)

🧰 Additional context used

📓 Path-based instructions (8)

**/*.py

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

**/*.py: In code comments use the abbreviations: nat (API namespace/CLI), nvidia-nat (package), NAT (env var prefixes); never use these abbreviations in documentation
Follow PEP 20 and PEP 8 for Python style
Run yapf with column_limit=120; yapf is used for formatting (run second)
Indent with 4 spaces (no tabs) and end each file with a single trailing newline
Use ruff (ruff check --fix) as a linter (not formatter) per pyproject.toml; fix warnings unless explicitly ignored
Respect Python naming schemes: snake_case for functions/variables, PascalCase for classes, UPPER_CASE for constants
When re-raising exceptions, use bare raise to preserve stack trace; log with logger.error(), not logger.exception()
When catching and logging without re-raising, use logger.exception() to capture full stack trace
Provide Google-style docstrings for every public module, class, function, and CLI command
Docstring first line must be a concise description ending with a period
Validate and sanitize all user input, especially in web or CLI interfaces
Prefer httpx with SSL verification enabled by default and follow OWASP Top-10 recommendations
Use async/await for I/O-bound work (HTTP, DB, file I/O)
Cache expensive computations with functools.lru_cache or an external cache when appropriate
Leverage NumPy vectorized operations when beneficial and feasible

Files:

• tests/nat/front_ends/mcp/test_tool_converter.py
• src/nat/front_ends/mcp/tool_converter.py

{tests/**/*.py,examples/*/tests/**/*.py}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

Unit tests live in tests/ or examples/*/tests and use markers defined in pyproject.toml (e.g., integration)

Files:

• tests/nat/front_ends/mcp/test_tool_converter.py

{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}: Use pytest with pytest-asyncio for async code
Test functions should be named with test_ prefix using snake_case
Extract frequently repeated test code into pytest fixtures
Pytest fixtures should define the name argument in @pytest.fixture and the function should be prefixed fixture_ using snake_case
Mock external services with pytest_httpserver or unittest.mock; avoid live endpoints
Mark slow tests with @pytest.mark.slow to skip in default test suite
Mark integration tests requiring external services with @pytest.mark.integration and follow nat-tests guidelines

Files:

• tests/nat/front_ends/mcp/test_tool_converter.py

{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}: Every file must start with the standard SPDX Apache-2.0 header
Confirm copyright years are up to date when a file is changed
All source files must include the SPDX Apache-2.0 header template (copy from an existing file)

Files:

• tests/nat/front_ends/mcp/test_tool_converter.py
• src/nat/front_ends/mcp/tool_converter.py

**/*

⚙️ CodeRabbit configuration file

**/*: # Code Review Instructions

• Ensure the code follows best practices and coding standards. - For Python code, follow
  PEP 20 and
  PEP 8 for style guidelines.
• Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
  Example:

  def my_function(param1: int, param2: str) -> bool:
      pass

• For Python exception handling, ensure proper stack trace preservation:
  • When re-raising exceptions: use bare raise statements to maintain the original stack trace,
    and use logger.error() (not logger.exception()) to avoid duplicate stack trace output.
  • When catching and logging exceptions without re-raising: always use logger.exception()
    to capture the full stack trace information.

Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any

words listed in the ci/vale/styles/config/vocabularies/nat/reject.txt file, words that might appear to be
spelling mistakes but are listed in the ci/vale/styles/config/vocabularies/nat/accept.txt file are OK.

Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,

and should contain an Apache License 2.0 header comment at the top of each file.

• Confirm that copyright years are up-to date whenever a file is changed.

Files:

• tests/nat/front_ends/mcp/test_tool_converter.py
• src/nat/front_ends/mcp/tool_converter.py

tests/**/*.py

⚙️ CodeRabbit configuration file

tests/**/*.py: - Ensure that tests are comprehensive, cover edge cases, and validate the functionality of the code. - Test functions should be named using the test_ prefix, using snake_case. - Any frequently repeated code should be extracted into pytest fixtures. - Pytest fixtures should define the name argument when applying the pytest.fixture decorator. The fixture
function being decorated should be named using the fixture_ prefix, using snake_case. Example:
@pytest.fixture(name="my_fixture")
def fixture_my_fixture():
pass

Files:

• tests/nat/front_ends/mcp/test_tool_converter.py

{src/**/*.py,packages/*/src/**/*.py}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

{src/**/*.py,packages/*/src/**/*.py}: All importable Python code must live under src/ or packages//src/
All public APIs must have Python 3.11+ type hints on parameters and return values
Prefer typing/collections.abc abstractions (e.g., Sequence over list)
Use typing.Annotated for units or metadata when useful
Treat pyright warnings as errors during development

Files:

• src/nat/front_ends/mcp/tool_converter.py

src/nat/**/*

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

src/nat/**/* contains core functionality; changes should prioritize backward compatibility

Files:

• src/nat/front_ends/mcp/tool_converter.py

---

⚙️ CodeRabbit configuration file

This directory contains the core functionality of the toolkit. Changes should prioritize backward compatibility.

Files:

• src/nat/front_ends/mcp/tool_converter.py

🧬 Code graph analysis (2)

tests/nat/front_ends/mcp/test_tool_converter.py (1)

src/nat/front_ends/mcp/tool_converter.py (3)

• create_function_wrapper (74-239)
• get_function_description (242-282)
• is_field_optional (42-71)

src/nat/front_ends/mcp/tool_converter.py (2)

src/nat/builder/workflow.py (1)

• run (97-109)

tests/nat/cli/commands/mcp/test_mcp_cli.py (1)

• acall_invoke (659-661)

🔇 Additional comments (8)

src/nat/front_ends/mcp/tool_converter.py (4)

21-21: LGTM! Clean imports and sentinel pattern.

The new imports support the optional field detection logic, and the sentinel constant follows best practices using object() for identity-based comparisons. The comment clearly explains its purpose.

Also applies to: 25-26, 38-40

---

42-72: Excellent implementation of optional field detection!

The function correctly handles all three cases:

• Required fields returning (False, Parameter.empty)
• Optional fields with explicit defaults returning the actual default value
• Optional fields with factories returning the sentinel for deferred evaluation

The logic properly checks field.is_required(), PydanticUndefined, and default_factory in the right order. The comprehensive docstring and type hints align with coding guidelines.

---

120-121: Clean integration of optional field detection.

The code correctly calls is_field_optional and uses the returned param_default value in the Parameter construction. The underscore prefix on _is_optional appropriately indicates the boolean isn't currently needed here.

Also applies to: 128-128

---

187-203: Well-designed parameter handling refactor.

The new approach correctly:

1. Strips sentinel values (Line 188) so Pydantic can apply default_factory during validation
2. Always validates inputs via model_validate (Line 192) to ensure defaults, factories, nested models, and validators are properly handled
3. Passes the validated model instance directly to workflows (Lines 196-199), which aligns with the Workflow.run(message: InputT) signature
4. Unpacks the validated model for regular functions (Lines 202-203) via model_dump()

The identity check is not _USE_PYDANTIC_DEFAULT is correct for sentinel filtering.

tests/nat/front_ends/mcp/test_tool_converter.py (4)

16-16: Comprehensive test schemas for parameter handling.

The new imports support testing the optional field detection functionality. The four test schemas provide excellent coverage:

• MockAllRequiredSchema: All required fields
• MockMixedRequiredOptionalSchema: Mixed required/optional with concrete defaults and default_factory
• MockAllOptionalSchema: All optional fields with various default types
• MockOptionalTypesSchema: Union types with None defaults

These schemas effectively test edge cases like zero defaults, boolean defaults, and factory defaults.

Also applies to: 27-27, 30-30, 47-77

---

93-205: Thorough unit tests for is_field_optional function.

The test suite comprehensively validates:

• Required field detection returning (False, Parameter.empty)
• Optional fields with various default types (string, int, bool, None)
• Factory default handling returning the _USE_PYDANTIC_DEFAULT sentinel
• Important edge cases: zero and False as valid (but falsy) defaults
• Consistency across multiple fields in a schema

All tests follow the arrange-act-assert pattern with clear docstrings. Excellent coverage!

---

409-556: Comprehensive validation of parameter schema conversion.

The tests ensure create_function_wrapper generates correct MCP tool signatures:

• Required parameters correctly have Parameter.empty as default
• Optional parameters correctly carry their default values
• Fields with default_factory correctly get the _USE_PYDANTIC_DEFAULT sentinel
• Type annotations are preserved in the wrapper signature
• Parameter order matches the Pydantic model field order

The inline comments (lines 460-461, 488-490) correctly explain the distinction between explicit None defaults and factory-generated defaults.

---

607-684: Excellent integration test coverage for optional parameter execution.

The three new integration tests comprehensively validate wrapper execution:

1. test_wrapper_with_optional_parameters_omitted (607-633): Validates that calling a wrapper with only required parameters works correctly, with Pydantic applying defaults for omitted optional parameters.

2. test_wrapper_with_optional_parameters_provided (635-663): Validates that explicitly providing optional parameters (including a list for the factory-based field) correctly overrides the defaults.

3. test_wrapper_with_none_values (665-684): Validates the important edge case of explicitly passing None for optional Union[T, None] fields, ensuring None is treated as a valid value rather than an omission.

All tests properly mock the observability context and verify the function is invoked. Great coverage of the execution paths!

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

src/nat/front_ends/mcp/tool_converter.py (1)

112-113: Consider unpacking only the needed value.

The _is_optional variable is assigned but never used. You could simplify by only unpacking the default value.

Apply this diff:

-            # Check if field is optional and get its default value
-            _is_optional, param_default = is_field_optional(field)
+            # Get the default value for the field (Parameter.empty if required)
+            _, param_default = is_field_optional(field)

Or even more explicit:

-            # Check if field is optional and get its default value
-            _is_optional, param_default = is_field_optional(field)
+            # Get the default value for the field (Parameter.empty if required)
+            param_default = is_field_optional(field)[1]

tests/nat/front_ends/mcp/test_tool_converter.py (1)

604-681: LGTM: Integration tests validate runtime behavior.

These tests effectively validate that the wrapper correctly handles optional parameters at runtime: when omitted, when provided, and when explicitly set to None. The tests use proper async mocking and observability context setup.

Optional: Consider verifying the actual kwargs passed to the function.

While the current tests verify that acall_invoke is called, they don't inspect the actual arguments passed. You could add assertions to verify that defaults are correctly applied:

# In test_wrapper_with_optional_parameters_omitted
call_kwargs = mock_function.acall_invoke.call_args[1]
assert call_kwargs["required_str"] == "test"
assert call_kwargs["required_int"] == 123
# Verify defaults were applied by Pydantic model validation

However, this may be testing Pydantic's behavior rather than your wrapper logic, so it's optional.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1fc7768 and 129b58b.

📒 Files selected for processing (2)

• src/nat/front_ends/mcp/tool_converter.py (3 hunks)
• tests/nat/front_ends/mcp/test_tool_converter.py (6 hunks)

🧰 Additional context used

📓 Path-based instructions (8)

**/*.py

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

**/*.py: In code comments use the abbreviations: nat (API namespace/CLI), nvidia-nat (package), NAT (env var prefixes); never use these abbreviations in documentation
Follow PEP 20 and PEP 8 for Python style
Run yapf with column_limit=120; yapf is used for formatting (run second)
Indent with 4 spaces (no tabs) and end each file with a single trailing newline
Use ruff (ruff check --fix) as a linter (not formatter) per pyproject.toml; fix warnings unless explicitly ignored
Respect Python naming schemes: snake_case for functions/variables, PascalCase for classes, UPPER_CASE for constants
When re-raising exceptions, use bare raise to preserve stack trace; log with logger.error(), not logger.exception()
When catching and logging without re-raising, use logger.exception() to capture full stack trace
Provide Google-style docstrings for every public module, class, function, and CLI command
Docstring first line must be a concise description ending with a period
Validate and sanitize all user input, especially in web or CLI interfaces
Prefer httpx with SSL verification enabled by default and follow OWASP Top-10 recommendations
Use async/await for I/O-bound work (HTTP, DB, file I/O)
Cache expensive computations with functools.lru_cache or an external cache when appropriate
Leverage NumPy vectorized operations when beneficial and feasible

Files:

• src/nat/front_ends/mcp/tool_converter.py
• tests/nat/front_ends/mcp/test_tool_converter.py

{src/**/*.py,packages/*/src/**/*.py}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

{src/**/*.py,packages/*/src/**/*.py}: All importable Python code must live under src/ or packages//src/
All public APIs must have Python 3.11+ type hints on parameters and return values
Prefer typing/collections.abc abstractions (e.g., Sequence over list)
Use typing.Annotated for units or metadata when useful
Treat pyright warnings as errors during development

Files:

• src/nat/front_ends/mcp/tool_converter.py

src/nat/**/*

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

src/nat/**/* contains core functionality; changes should prioritize backward compatibility

Files:

• src/nat/front_ends/mcp/tool_converter.py

---

⚙️ CodeRabbit configuration file

This directory contains the core functionality of the toolkit. Changes should prioritize backward compatibility.

Files:

• src/nat/front_ends/mcp/tool_converter.py

{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}: Every file must start with the standard SPDX Apache-2.0 header
Confirm copyright years are up to date when a file is changed
All source files must include the SPDX Apache-2.0 header template (copy from an existing file)

Files:

• src/nat/front_ends/mcp/tool_converter.py
• tests/nat/front_ends/mcp/test_tool_converter.py

**/*

⚙️ CodeRabbit configuration file

**/*: # Code Review Instructions

• Ensure the code follows best practices and coding standards. - For Python code, follow
  PEP 20 and
  PEP 8 for style guidelines.
• Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
  Example:

  def my_function(param1: int, param2: str) -> bool:
      pass

• For Python exception handling, ensure proper stack trace preservation:
  • When re-raising exceptions: use bare raise statements to maintain the original stack trace,
    and use logger.error() (not logger.exception()) to avoid duplicate stack trace output.
  • When catching and logging exceptions without re-raising: always use logger.exception()
    to capture the full stack trace information.

Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any

words listed in the ci/vale/styles/config/vocabularies/nat/reject.txt file, words that might appear to be
spelling mistakes but are listed in the ci/vale/styles/config/vocabularies/nat/accept.txt file are OK.

Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,

and should contain an Apache License 2.0 header comment at the top of each file.

• Confirm that copyright years are up-to date whenever a file is changed.

Files:

• src/nat/front_ends/mcp/tool_converter.py
• tests/nat/front_ends/mcp/test_tool_converter.py

{tests/**/*.py,examples/*/tests/**/*.py}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

Unit tests live in tests/ or examples/*/tests and use markers defined in pyproject.toml (e.g., integration)

Files:

• tests/nat/front_ends/mcp/test_tool_converter.py

{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}: Use pytest with pytest-asyncio for async code
Test functions should be named with test_ prefix using snake_case
Extract frequently repeated test code into pytest fixtures
Pytest fixtures should define the name argument in @pytest.fixture and the function should be prefixed fixture_ using snake_case
Mock external services with pytest_httpserver or unittest.mock; avoid live endpoints
Mark slow tests with @pytest.mark.slow to skip in default test suite
Mark integration tests requiring external services with @pytest.mark.integration and follow nat-tests guidelines

Files:

• tests/nat/front_ends/mcp/test_tool_converter.py

tests/**/*.py

⚙️ CodeRabbit configuration file

tests/**/*.py: - Ensure that tests are comprehensive, cover edge cases, and validate the functionality of the code. - Test functions should be named using the test_ prefix, using snake_case. - Any frequently repeated code should be extracted into pytest fixtures. - Pytest fixtures should define the name argument when applying the pytest.fixture decorator. The fixture
function being decorated should be named using the fixture_ prefix, using snake_case. Example:
@pytest.fixture(name="my_fixture")
def fixture_my_fixture():
pass

Files:

• tests/nat/front_ends/mcp/test_tool_converter.py

🧬 Code graph analysis (1)

tests/nat/front_ends/mcp/test_tool_converter.py (2)

src/nat/front_ends/mcp/tool_converter.py (2)

• is_field_optional (39-63)
• create_function_wrapper (66-241)

src/nat/utils/type_utils.py (1)

• is_optional (207-218)

🔇 Additional comments (5)

src/nat/front_ends/mcp/tool_converter.py (2)

21-21: LGTM: Necessary imports added.

The new imports (Any, FieldInfo, PydanticUndefined) are correctly added to support the optional field detection logic.

Also applies to: 25-26

---

39-63: LGTM: Well-implemented optional field detection.

The function correctly identifies optional Pydantic fields by checking is_required(), explicit defaults, and default_factory. The logic properly returns the actual default value when available, or None for factory-based defaults, making it suitable for function signature construction.

tests/nat/front_ends/mcp/test_tool_converter.py (3)

16-16: LGTM: Comprehensive test schemas.

The imports and test schemas are well-structured, covering all necessary scenarios: all-required, all-optional, mixed, and Union-with-None types. This provides excellent test coverage for the optional field detection functionality.

Also applies to: 29-29, 46-76

---

92-203: LGTM: Thorough unit test coverage for is_field_optional.

The test class provides comprehensive coverage of the new utility function, including edge cases like falsy defaults (0, False), default_factory handling, and Union types with None. The tests follow pytest conventions and use clear Arrange-Act-Assert patterns.

---

407-553: LGTM: Comprehensive validation of wrapper parameter schemas.

These tests effectively validate that create_function_wrapper correctly translates Pydantic field optionality into function signature defaults. The tests cover all scenarios (required, optional, mixed, Union types) and verify that annotations and order are preserved. The comment at line 486 helpfully explains the default_factory → None mapping.

[yczhang-nv]

LGTM

[AnuradhaKaruppiah]

LGTM

thanks for the review @yczhang-nv!

[AnuradhaKaruppiah]

/merge