Fix TypeConverter not able to handle Union type conversion

[yczhang-nv]

Description

This PR fixes the bug that TypeConverter not able to handle Union type conversion correctly.

This needs to be merged after PR-1298

Closes AIQ-2600

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

Release Notes

• Bug Fixes

  • Improved type converter to correctly handle union types in both direct and indirect conversion paths, including support for Python 3.10+ parameterized union syntax.

• Tests

  • Added comprehensive test coverage for union type conversion scenarios across multiple conversion paths and edge cases.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

The changes enhance union-type handling in TypeConverter by distinguishing union from non-union types in direct and indirect conversion paths, enabling correct isinstance checks for union-parameterized types (Python 3.10+). Comprehensive test coverage is added.

Changes

Cohort / File(s)	Summary
Union-type conversion logic src/nat/utils/type_converter.py	Enhanced _try_direct_conversion_ to compute decomposed_from and select check_type based on whether from_type is a union. Enhanced _try_indirect_conversion_ to use isinstance for union types and DecomposedType for non-union types. Added explanatory comments for union handling.
Union-type test coverage tests/nat/utils/test_converter.py	Added TargetSchema class with __init__, __eq__, and __hash__ methods. Added 11 test functions covering direct conversions from union-typed from_type, indirect paths through converters, failure paths, three-member unions, None handling, bidirectional conversions, and try_convert behavior variations.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• Areas requiring extra attention:
  • _try_direct_conversion_ logic: Verify the decomposed_from computation and check_type selection correctly handles all union and non-union type combinations
  • _try_indirect_conversion_ conditional logic: Ensure isinstance vs DecomposedType branching properly distinguishes union behavior
  • Test assertions: Review the TargetSchema class equality and hashing behavior to ensure test scenarios accurately validate conversion results
  • Edge cases: Verify coverage of nested unions, None unions, and parameterized generic targets

Suggested labels

DO NOT MERGE

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 46.67% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately and concisely describes the main fix - handling Union type conversion in TypeConverter. It uses imperative mood and is 62 characters, well within the ~72 character limit.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

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

---

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 (1)

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

187-190: Clarify the comment for better readability.

The comment on line 187 is grammatically incomplete. It should start with a verb or subject to form a complete sentence.

🔎 Suggested improvement

-                    # union types correctly in Python 3.10+ (e.g., isinstance("x", str | int))
+                    # Handle union types correctly in Python 3.10+ (e.g., isinstance("x", str | int))
                     decomposed_from = DecomposedType(convert_from_type)
                     check_type = convert_from_type if decomposed_from.is_union else decomposed_from.root
                     if isinstance(data, check_type):

📜 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 b1f764e and 39d060e.

📒 Files selected for processing (2)

• src/nat/utils/type_converter.py (2 hunks)
• tests/nat/utils/test_converter.py (1 hunks)

🧰 Additional context used

📓 Path-based instructions (10)

**/*.{py,md,toml,yaml,yml}

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

Use technical identifiers: 'nat' for API namespace and CLI tool, 'nvidia-nat' for package name, 'NAT_' prefix for environment variables, 'NeMo-Agent-Toolkit' for URLs and directory names

Files:

• src/nat/utils/type_converter.py
• tests/nat/utils/test_converter.py

src/nat/**/*.py

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

Source code in 'src/nat/**/*' contains core functionality; changes should prioritize backward compatibility

Files:

• src/nat/utils/type_converter.py

**/*.py

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

**/*.py: Follow PEP 20 and PEP 8 for Python style guidelines
Run yapf second (PEP 8 base, 'column_limit = 120') for code formatting
Use ruff (via 'ruff check --fix') for linting with configuration in 'pyproject.toml'; fix warnings unless explicitly ignored
Respect naming schemes: 'snake_case' for functions & variables, 'PascalCase' for classes, 'UPPER_CASE' for constants in Python code
All public APIs require Python 3.11+ type hints on parameters and return values
Prefer 'collections.abc' / 'typing' abstractions ('Sequence' over 'list') in type hints
Use 'typing.Annotated' for units or extra metadata when useful in type hints
Treat 'pyright' warnings (configured in 'pyproject.toml') as errors during development
Preserve stack traces and prevent duplicate logging when handling exceptions; use bare 'raise' statements to maintain original stack trace, and use 'logger.error()' for logging (not 'logger.exception()') when re-raising
When catching and logging exceptions without re-raising, always use 'logger.exception()' (equivalent to 'logger.error(exc_info=True)') to capture the full stack trace information
Provide Google-style docstrings for every public module, class, function and CLI command
The first line of docstrings must be a concise description ending with a period (Vale checks this)
Surround code entities with backticks in docstrings to avoid Vale false-positives
Validate and sanitise all user input, especially in web or CLI interfaces in Python code
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 reads) in Python code
Profile CPU-heavy paths with 'cProfile' or 'mprof' before optimising
Cache expensive computations with 'functools.lru_cache' or an external cache when appropriate in Python code
Leverage NumPy vectorised operations whenever beneficial and feasible in Python code

Files:

• src/nat/utils/type_converter.py
• tests/nat/utils/test_converter.py

**/*.{py,yaml,yml,md,json,toml}

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

Indent with 4 spaces, never tabs, and ensure every file ends with a single newline

Files:

• src/nat/utils/type_converter.py
• tests/nat/utils/test_converter.py

**/*.{py,js,ts,yaml,yml,sh,md}

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

**/*.{py,js,ts,yaml,yml,sh,md}: Every file must start with the standard SPDX Apache-2.0 header
Confirm that copyright years are up-to-date whenever a file is changed
All source files must include the SPDX Apache-2.0 header template; binary assets committed via Git-LFS must have licensing info recorded in 'LICENSE-3rd-party.txt'

Files:

• src/nat/utils/type_converter.py
• tests/nat/utils/test_converter.py

**/*.{py,md}

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

Version numbers are derived automatically by 'setuptools-scm'; never hard-code them in code or docs

Files:

• src/nat/utils/type_converter.py
• tests/nat/utils/test_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 (except for return values of None,
  in that situation no return type hint is needed).
  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/utils/type_converter.py
• tests/nat/utils/test_converter.py

src/nat/**/*

⚙️ CodeRabbit configuration file

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

Files:

• src/nat/utils/type_converter.py

**/test_*.py

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

**/test_*.py: Use pytest with 'pytest-asyncio' for testing asynchronous code
Test functions should be named using the 'test_' prefix, using snake_case
Extract frequently repeated code in tests into pytest fixtures
Pytest fixtures should define the name argument when applying the pytest.fixture decorator; the fixture function should be named using the 'fixture_' prefix, using snake_case (e.g., '@pytest.fixture(name="my_fixture") def fixture_my_fixture():')
Maintain ≥ 80% test coverage; add or update tests when introducing changes
Mock external services with 'pytest_httpserver' or 'unittest.mock' instead of hitting live endpoints in tests
Mark slow tests with '@pytest.mark.slow' so they can be skipped in the default test suite
Mark integration tests which require external services with '@pytest.mark.integration' so they can be skipped in the default test suite; follow the integration testing guidelines in nat-tests/integration-tests.mdc rules

Files:

• tests/nat/utils/test_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

• Do NOT add @pytest.mark.asyncio to any test. Async tests are automatically detected and run by the
  async runner - the decorator is unnecessary clutter.

Files:

• tests/nat/utils/test_converter.py

🧬 Code graph analysis (2)

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

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

• is_union (173-184)
• root (101-111)
• is_instance (341-356)

tests/nat/utils/test_converter.py (1)

src/nat/utils/type_converter.py (5)

• TypeConverter (33-282)
• convert (119-148)
• convert (297-298)
• try_convert (150-172)
• try_convert (301-302)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: CI Pipeline / Check

🔇 Additional comments (2)

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

246-254: LGTM! Well-implemented union type handling.

The logic correctly uses isinstance directly for union types (Python 3.10+ support) and falls back to DecomposedType.is_instance for non-union types. The comment is clear and the implementation is consistent with the direct conversion path.

tests/nat/utils/test_converter.py (1)

573-756: Excellent test coverage for union type handling!

The new TargetSchema class and comprehensive union type tests thoroughly validate the TypeConverter changes. The tests cover:

• Direct conversions with various union members (str, int, classes)
• Indirect conversion paths
• Failure scenarios with proper error handling
• Edge cases (three-member unions, None/optional types, subclass matching)
• Bidirectional conversions

All tests follow pytest conventions and naming guidelines. The test coverage ensures the union type handling works correctly across multiple scenarios.

[yczhang-nv]

/merge