Skip to content

Fix bug where SecretStr fields defaulting to an empty string were not being instantiated#1298

Merged
rapids-bot[bot] merged 12 commits intoNVIDIA:release/1.4from
dagardner-nv:david-default-secret
Dec 19, 2025
Merged

Fix bug where SecretStr fields defaulting to an empty string were not being instantiated#1298
rapids-bot[bot] merged 12 commits intoNVIDIA:release/1.4from
dagardner-nv:david-default-secret

Conversation

@dagardner-nv
Copy link
Contributor

@dagardner-nv dagardner-nv commented Dec 19, 2025

Description

  • Adds nat serve integration tests for the agent example workflows

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

  • Tests

    • Added parameterized workflow tests to run both direct and REST API server modes; added tests ensuring API key/secret fields are exposed as secrets and unique per instance.
    • Added a helper to exercise workflows via the REST API for test coverage.
  • New Features

    • Support for testing workflows via a REST API server mode alongside direct execution.
  • Refactor

    • Secret credential defaults changed to per-instance secret objects to avoid shared default instances.

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

Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
…zableSecretStr

Signed-off-by: David Gardner <dagardner@nvidia.com>
…olkit into david-default-secret

Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
@dagardner-nv dagardner-nv self-assigned this Dec 19, 2025
@dagardner-nv dagardner-nv requested a review from a team as a code owner December 19, 2025 16:44
@dagardner-nv dagardner-nv added bug Something isn't working non-breaking Non-breaking change labels Dec 19, 2025
@coderabbitai
Copy link

coderabbitai bot commented Dec 19, 2025

Walkthrough

Adds an async serve_workflow test helper to run NAT in server (REST) mode and query /generate; parameterizes two agent tests with a use_rest_api flag to call either serve_workflow or existing run_workflow; and replaces several secret-field defaults with Field(default_factory=lambda: SerializableSecretStr("")).

Changes

Cohort / File(s) Summary
REST API testing helper & test updates
packages/nvidia_nat_test/src/nat/test/utils.py, examples/agents/tests/test_agents.py
Added async def serve_workflow(...) to spawn nat serve, POST a question to /generate, aggregate response text, optionally assert expected answer, and tear down the server. Updated test_rewoo_full_workflow and test_agent_full_workflow to accept use_rest_api: bool and branch between serve_workflow (True) and run_workflow (False).
Secret-field default_factory changes
examples/evaluation_and_profiling/swe_bench/src/nat_swe_bench/config.py, packages/nvidia_nat_langchain/src/nat/plugins/langchain/tools/tavily_internet_search.py, packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py, packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/register.py
Replaced Field(default="") with Field(default_factory=lambda: SerializableSecretStr("")) for multiple secret fields so each instance receives a fresh SerializableSecretStr. Affected fields include SweBenchPredictorFullConfig.openai_api_key, TavilyInternetSearchToolConfig.api_key, telemetry exporter api/keys, and CatalystTelemetryExporter access/secret keys.
Tavily Internet Search tests
packages/nvidia_nat_langchain/tests/test_tavily_internet_search.py
Added tests: test_api_key_is_secret_str (parameterized) to assert api_key is a SecretStr with expected value, and test_default_api_key_is_unique_instance to ensure distinct default SecretStr instances per config object.

Sequence Diagram(s)

sequenceDiagram
    participant Test as Pytest Test
    participant Helper as serve_workflow
    participant Proc as NAT Process
    participant API as NAT REST API (/generate)

    Test->>Helper: call serve_workflow(config_path, question, expected_answer)
    Helper->>Proc: spawn `nat serve --config <path> --port <n>`
    Note over Proc: NAT starts and exposes REST endpoints
    Helper->>API: HTTP POST /generate { "question": "..." }
    API-->>Helper: 200 { value / choices / ... }
    Helper->>Helper: aggregate text from response parts
    alt assert_expected_answer
        Helper->>Test: assert expected_answer in aggregated text
    end
    Helper->>Proc: terminate/kill NAT Process
    Helper-->>Test: return parsed response payload
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Focus review on:
    • packages/nvidia_nat_test/src/nat/test/utils.py: process spawn/teardown correctness, port selection/availability, timeouts, async handling, and HTTP response parsing/aggregation.
    • examples/agents/tests/test_agents.py: pytest parameterization and fixture compatibility.
    • Files using default_factory=lambda: SerializableSecretStr(""): ensure pydantic Field usage is correct and SecretStr serialization/typing remain as intended.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.86% 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 describes the main change: converting SecretStr field defaults from empty strings to default_factory instances, which is the core pattern repeated across multiple files.
✨ 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 a71aaf8 and f20f818.

📒 Files selected for processing (1)
  • packages/nvidia_nat_test/src/nat/test/utils.py (2 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{md,rst,py}

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

Use 'NVIDIA NeMo Agent toolkit' on first use, then 'NeMo Agent toolkit' for subsequent references

Files:

  • packages/nvidia_nat_test/src/nat/test/utils.py
**/*.{py,toml,yaml,yml}

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

Use abbreviations: 'nat' for API namespace and CLI tool, 'nvidia-nat' for package name, 'NAT' for environment variable prefixes and informal comments

Files:

  • packages/nvidia_nat_test/src/nat/test/utils.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 Python formatting
Use ruff check --fix (via pre-commit) for linting using configuration embedded in pyproject.toml, fix warnings unless explicitly ignored
Use snake_case for functions and variables, PascalCase for classes, UPPER_CASE for constants
All public APIs require Python 3.11+ type hints on parameters and return values
Prefer collections.abc / typing abstractions (Sequence over list) for type hints
Use typing.Annotated for units or extra metadata when useful
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 and logger.error() when re-raising
When catching and logging exceptions without re-raising, always use logger.exception() 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
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)
Cache expensive computations with functools.lru_cache or an external cache when appropriate
Leverage NumPy vectorised operations whenever beneficial and feasible

Files:

  • packages/nvidia_nat_test/src/nat/test/utils.py
**/*.{py,js,ts,yaml,yml,json,md,rst}

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

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

Files:

  • packages/nvidia_nat_test/src/nat/test/utils.py
**/*.{py,env,toml,yaml,yml,json}

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

Never commit API keys, credentials or personal data; use environment variables or .env files excluded from Git

Files:

  • packages/nvidia_nat_test/src/nat/test/utils.py
**/*.{py,js,ts,java,cpp,c,go,rb,php}

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

Every file must start with the standard SPDX Apache-2.0 header

Files:

  • packages/nvidia_nat_test/src/nat/test/utils.py
**/*.{py,md,rst}

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

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

Files:

  • packages/nvidia_nat_test/src/nat/test/utils.py
**/*.{py,js,ts,java,cpp,c,go,rb,php,sh}

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

All source files must include the SPDX Apache-2.0 header template

Files:

  • packages/nvidia_nat_test/src/nat/test/utils.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:

  • packages/nvidia_nat_test/src/nat/test/utils.py
packages/**/*

⚙️ CodeRabbit configuration file

packages/**/*: - This directory contains optional plugin packages for the toolkit, each should contain a pyproject.toml file. - The pyproject.toml file should declare a dependency on nvidia-nat or another package with a name starting
with nvidia-nat-. This dependency should be declared using ~=<version>, and the version should be a two
digit version (ex: ~=1.0).

  • Not all packages contain Python code, if they do they should also contain their own set of tests, in a
    tests/ directory at the same level as the pyproject.toml file.
  • When adding a new package, that new package name (as defined in the pyproject.toml file) should
    be added as a dependency to the nvidia-nat-all package in packages/nvidia_nat_all/pyproject.toml

Files:

  • packages/nvidia_nat_test/src/nat/test/utils.py
🧠 Learnings (5)
📚 Learning: 2025-09-15T21:26:29.430Z
Learnt from: saglave
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 726
File: examples/frameworks/adk_demo/src/nat_adk_demo/weather_update_tool.py:0-0
Timestamp: 2025-09-15T21:26:29.430Z
Learning: In NAT's ADK integration, the docstring of registered functions serves as the tool description sent to the LLM, not standard Python function documentation. The docstring should describe the logical tool interface (parameters the LLM will provide) rather than the NAT framework wrapper parameters like tool_config and builder.

Applied to files:

  • packages/nvidia_nat_test/src/nat/test/utils.py
📚 Learning: 2025-09-17T05:34:04.696Z
Learnt from: AnuradhaKaruppiah
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 752
File: packages/nvidia_nat_mcp/src/nat/plugins/mcp/client_base.py:0-0
Timestamp: 2025-09-17T05:34:04.696Z
Learning: The user AnuradhaKaruppiah confirmed that request copying is not needed in the AuthAdapter.async_auth_flow method in packages/nvidia_nat_mcp/src/nat/plugins/mcp/client_base.py, suggesting that httpx handles request mutation safely or there are no concurrent usage concerns for this implementation.

Applied to files:

  • packages/nvidia_nat_test/src/nat/test/utils.py
📚 Learning: 2025-11-24T18:56:53.109Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-11-24T18:56:53.109Z
Learning: Applies to **/*.py : Prefer httpx with SSL verification enabled by default and follow OWASP Top-10 recommendations

Applied to files:

  • packages/nvidia_nat_test/src/nat/test/utils.py
📚 Learning: 2025-11-24T18:56:53.109Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-11-24T18:56:53.109Z
Learning: Applies to **/*.py : Use async/await for I/O-bound work (HTTP, DB, file reads)

Applied to files:

  • packages/nvidia_nat_test/src/nat/test/utils.py
📚 Learning: 2025-11-14T20:33:53.944Z
Learnt from: AnuradhaKaruppiah
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 1181
File: packages/nvidia_nat_test/tests/test_test_llm.py:419-484
Timestamp: 2025-11-14T20:33:53.944Z
Learning: The NVIDIA NeMo-Agent-Toolkit project uses pytest-asyncio in strict mode (the default), which requires pytest.mark.asyncio decorator on all async test functions. All async tests in packages/nvidia_nat_test/tests/test_test_llm.py consistently follow this pattern.

Applied to files:

  • packages/nvidia_nat_test/src/nat/test/utils.py
🪛 Ruff (0.14.8)
packages/nvidia_nat_test/src/nat/test/utils.py

110-110: subprocess call: check for execution of untrusted input

(S603)


124-124: Do not catch blind exception: Exception

(BLE001)

⏰ 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 (1)
packages/nvidia_nat_test/src/nat/test/utils.py (1)

143-156: LGTM: Robust cleanup logic.

The teardown sequence properly ensures the subprocess is cleaned up with graceful termination attempted first, followed by kill if needed. The final assertion confirms successful cleanup.


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

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
packages/nvidia_nat_test/src/nat/test/utils.py (2)

93-151: Useful REST API test utility with minor refinements needed.

The serve_workflow function provides valuable test coverage for the REST API serving mode. However, a few minor issues should be addressed:

  1. Line 98: Spacing inconsistency in parameter definition: port: int= 8000 should be port: int = 8000.
  2. Line 102: Typo in docstring: "Intendted" should be "Intended".
🔎 Proposed fixes
 async def serve_workflow(*,
                          config_path: Path,
                          question: str,
                          expected_answer: str,
                          assert_expected_answer: bool = True,
-                         port: int= 8000) -> dict:
+                         port: int = 8000) -> dict:
     """
     Execute a workflow using `nat serve`, and issue a POST request to the `/generate` endpoint with the given question.
 
-    Intendted to be analogous to `run_workflow` but for the REST API serving mode.
+    Intended to be analogous to `run_workflow` but for the REST API serving mode.
     """

119-120: Consider narrowing exception handling in retry loop.

The bare Exception catch on Line 119 is acceptable for a retry loop waiting for server startup, but could be narrowed to connection-specific exceptions (e.g., requests.exceptions.ConnectionError, requests.exceptions.Timeout) for better error visibility during debugging.

🔎 Suggested refinement
             try:
                 response = requests.post(url=f"{workflow_url}/generate",
                                          json= {"messages": [{"role": "user", "content": question}]},
                                          timeout=60)
-            except Exception:
+            except (requests.exceptions.ConnectionError, requests.exceptions.Timeout):
                 await asyncio.sleep(0.1)
📜 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 eea46dc and b5d84d1.

📒 Files selected for processing (7)
  • examples/agents/tests/test_agents.py (3 hunks)
  • examples/evaluation_and_profiling/swe_bench/src/nat_swe_bench/config.py (1 hunks)
  • packages/nvidia_nat_langchain/src/nat/plugins/langchain/tools/tavily_internet_search.py (1 hunks)
  • packages/nvidia_nat_langchain/tests/test_tavily_internet_search.py (1 hunks)
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py (3 hunks)
  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/register.py (1 hunks)
  • packages/nvidia_nat_test/src/nat/test/utils.py (2 hunks)
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{md,rst,py}

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

Use 'NVIDIA NeMo Agent toolkit' on first use, then 'NeMo Agent toolkit' for subsequent references

Files:

  • packages/nvidia_nat_langchain/src/nat/plugins/langchain/tools/tavily_internet_search.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
  • packages/nvidia_nat_langchain/tests/test_tavily_internet_search.py
  • examples/evaluation_and_profiling/swe_bench/src/nat_swe_bench/config.py
  • examples/agents/tests/test_agents.py
  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/register.py
  • packages/nvidia_nat_test/src/nat/test/utils.py
**/*.{py,toml,yaml,yml}

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

Use abbreviations: 'nat' for API namespace and CLI tool, 'nvidia-nat' for package name, 'NAT' for environment variable prefixes and informal comments

Files:

  • packages/nvidia_nat_langchain/src/nat/plugins/langchain/tools/tavily_internet_search.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
  • packages/nvidia_nat_langchain/tests/test_tavily_internet_search.py
  • examples/evaluation_and_profiling/swe_bench/src/nat_swe_bench/config.py
  • examples/agents/tests/test_agents.py
  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/register.py
  • packages/nvidia_nat_test/src/nat/test/utils.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 Python formatting
Use ruff check --fix (via pre-commit) for linting using configuration embedded in pyproject.toml, fix warnings unless explicitly ignored
Use snake_case for functions and variables, PascalCase for classes, UPPER_CASE for constants
All public APIs require Python 3.11+ type hints on parameters and return values
Prefer collections.abc / typing abstractions (Sequence over list) for type hints
Use typing.Annotated for units or extra metadata when useful
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 and logger.error() when re-raising
When catching and logging exceptions without re-raising, always use logger.exception() 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
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)
Cache expensive computations with functools.lru_cache or an external cache when appropriate
Leverage NumPy vectorised operations whenever beneficial and feasible

Files:

  • packages/nvidia_nat_langchain/src/nat/plugins/langchain/tools/tavily_internet_search.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
  • packages/nvidia_nat_langchain/tests/test_tavily_internet_search.py
  • examples/evaluation_and_profiling/swe_bench/src/nat_swe_bench/config.py
  • examples/agents/tests/test_agents.py
  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/register.py
  • packages/nvidia_nat_test/src/nat/test/utils.py
**/*.{py,js,ts,yaml,yml,json,md,rst}

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

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

Files:

  • packages/nvidia_nat_langchain/src/nat/plugins/langchain/tools/tavily_internet_search.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
  • packages/nvidia_nat_langchain/tests/test_tavily_internet_search.py
  • examples/evaluation_and_profiling/swe_bench/src/nat_swe_bench/config.py
  • examples/agents/tests/test_agents.py
  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/register.py
  • packages/nvidia_nat_test/src/nat/test/utils.py
**/*.{py,env,toml,yaml,yml,json}

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

Never commit API keys, credentials or personal data; use environment variables or .env files excluded from Git

Files:

  • packages/nvidia_nat_langchain/src/nat/plugins/langchain/tools/tavily_internet_search.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
  • packages/nvidia_nat_langchain/tests/test_tavily_internet_search.py
  • examples/evaluation_and_profiling/swe_bench/src/nat_swe_bench/config.py
  • examples/agents/tests/test_agents.py
  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/register.py
  • packages/nvidia_nat_test/src/nat/test/utils.py
**/*.{py,js,ts,java,cpp,c,go,rb,php}

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

Every file must start with the standard SPDX Apache-2.0 header

Files:

  • packages/nvidia_nat_langchain/src/nat/plugins/langchain/tools/tavily_internet_search.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
  • packages/nvidia_nat_langchain/tests/test_tavily_internet_search.py
  • examples/evaluation_and_profiling/swe_bench/src/nat_swe_bench/config.py
  • examples/agents/tests/test_agents.py
  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/register.py
  • packages/nvidia_nat_test/src/nat/test/utils.py
**/*.{py,md,rst}

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

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

Files:

  • packages/nvidia_nat_langchain/src/nat/plugins/langchain/tools/tavily_internet_search.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
  • packages/nvidia_nat_langchain/tests/test_tavily_internet_search.py
  • examples/evaluation_and_profiling/swe_bench/src/nat_swe_bench/config.py
  • examples/agents/tests/test_agents.py
  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/register.py
  • packages/nvidia_nat_test/src/nat/test/utils.py
**/*.{py,js,ts,java,cpp,c,go,rb,php,sh}

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

All source files must include the SPDX Apache-2.0 header template

Files:

  • packages/nvidia_nat_langchain/src/nat/plugins/langchain/tools/tavily_internet_search.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
  • packages/nvidia_nat_langchain/tests/test_tavily_internet_search.py
  • examples/evaluation_and_profiling/swe_bench/src/nat_swe_bench/config.py
  • examples/agents/tests/test_agents.py
  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/register.py
  • packages/nvidia_nat_test/src/nat/test/utils.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:

  • packages/nvidia_nat_langchain/src/nat/plugins/langchain/tools/tavily_internet_search.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
  • packages/nvidia_nat_langchain/tests/test_tavily_internet_search.py
  • examples/evaluation_and_profiling/swe_bench/src/nat_swe_bench/config.py
  • examples/agents/tests/test_agents.py
  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/register.py
  • packages/nvidia_nat_test/src/nat/test/utils.py
packages/**/*

⚙️ CodeRabbit configuration file

packages/**/*: - This directory contains optional plugin packages for the toolkit, each should contain a pyproject.toml file. - The pyproject.toml file should declare a dependency on nvidia-nat or another package with a name starting
with nvidia-nat-. This dependency should be declared using ~=<version>, and the version should be a two
digit version (ex: ~=1.0).

  • Not all packages contain Python code, if they do they should also contain their own set of tests, in a
    tests/ directory at the same level as the pyproject.toml file.
  • When adding a new package, that new package name (as defined in the pyproject.toml file) should
    be added as a dependency to the nvidia-nat-all package in packages/nvidia_nat_all/pyproject.toml

Files:

  • packages/nvidia_nat_langchain/src/nat/plugins/langchain/tools/tavily_internet_search.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
  • packages/nvidia_nat_langchain/tests/test_tavily_internet_search.py
  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/register.py
  • packages/nvidia_nat_test/src/nat/test/utils.py
**/test_*.py

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

**/test_*.py: Use pytest with pytest-asyncio for asynchronous code testing
Test functions should be named using the test_ prefix, using snake_case
Extract frequently repeated code into pytest fixtures, which should be named using the fixture_ prefix and define the name argument in the decorator
Mock external services with pytest_httpserver or unittest.mock instead of hitting live endpoints
Mark slow tests with @pytest.mark.slow so they can be skipped in the default test suite
Mark integration tests requiring external services with @pytest.mark.integration so they can be skipped in the default test suite

Files:

  • packages/nvidia_nat_langchain/tests/test_tavily_internet_search.py
  • examples/agents/tests/test_agents.py
examples/**/*

⚙️ CodeRabbit configuration file

examples/**/*: - This directory contains example code and usage scenarios for the toolkit, at a minimum an example should
contain a README.md or file README.ipynb.

  • If an example contains Python code, it should be placed in a subdirectory named src/ and should
    contain a pyproject.toml file. Optionally, it might also contain scripts in a scripts/ directory.
  • If an example contains YAML files, they should be placed in a subdirectory named configs/. - If an example contains sample data files, they should be placed in a subdirectory named data/, and should
    be checked into git-lfs.

Files:

  • examples/evaluation_and_profiling/swe_bench/src/nat_swe_bench/config.py
  • examples/agents/tests/test_agents.py
🧠 Learnings (2)
📚 Learning: 2025-12-18T18:55:42.993Z
Learnt from: AnuradhaKaruppiah
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 1281
File: packages/nvidia_nat_a2a/src/nat/plugins/a2a/auth/credential_service.py:93-94
Timestamp: 2025-12-18T18:55:42.993Z
Learning: In packages/nvidia_nat_a2a/src/nat/plugins/a2a/auth/credential_service.py, the default_user_id parameter was intentionally removed from A2ACredentialService.__init__ to enhance security by enforcing per-user context. The implementation now exclusively uses Context.get().user_id to obtain user identification, preventing fallback to a shared default user.

Applied to files:

  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/register.py
📚 Learning: 2025-11-14T20:33:53.944Z
Learnt from: AnuradhaKaruppiah
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 1181
File: packages/nvidia_nat_test/tests/test_test_llm.py:419-484
Timestamp: 2025-11-14T20:33:53.944Z
Learning: The NVIDIA NeMo-Agent-Toolkit project uses pytest-asyncio in strict mode (the default), which requires pytest.mark.asyncio decorator on all async test functions. All async tests in packages/nvidia_nat_test/tests/test_test_llm.py consistently follow this pattern.

Applied to files:

  • examples/agents/tests/test_agents.py
  • packages/nvidia_nat_test/src/nat/test/utils.py
🧬 Code graph analysis (2)
packages/nvidia_nat_langchain/tests/test_tavily_internet_search.py (2)
packages/nvidia_nat_langchain/src/nat/plugins/langchain/tools/tavily_internet_search.py (2)
  • tavily_internet_search (37-69)
  • TavilyInternetSearchToolConfig (27-33)
src/nat/data_models/common.py (1)
  • get_secret_value (177-193)
examples/agents/tests/test_agents.py (1)
packages/nvidia_nat_test/src/nat/test/utils.py (2)
  • serve_workflow (93-151)
  • run_workflow (73-91)
🪛 Ruff (0.14.8)
packages/nvidia_nat_test/src/nat/test/utils.py

107-107: subprocess call: check for execution of untrusted input

(S603)


119-119: Do not catch blind exception: Exception

(BLE001)

🔇 Additional comments (10)
packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/register.py (1)

34-35: LGTM! Proper use of default_factory for unique SecretStr instances.

The change from default="" to default_factory=lambda: SerializableSecretStr("") ensures each CatalystTelemetryExporter instance receives its own SerializableSecretStr object, preventing unintended sharing of mutable defaults. This aligns with Pydantic best practices and the PR objective.

examples/evaluation_and_profiling/swe_bench/src/nat_swe_bench/config.py (1)

38-38: LGTM! Correct default_factory pattern for SecretStr.

The change ensures each SweBenchPredictorFullConfig instance gets a unique SerializableSecretStr object, consistent with the PR's bug fix.

packages/nvidia_nat_langchain/src/nat/plugins/langchain/tools/tavily_internet_search.py (1)

33-33: LGTM! Proper default_factory usage with improved field documentation.

The change to default_factory=lambda: SerializableSecretStr("") ensures each instance gets a unique SecretStr object. The addition of the description parameter also improves API documentation.

packages/nvidia_nat_langchain/tests/test_tavily_internet_search.py (1)

1-38: LGTM! Comprehensive test coverage for the SecretStr fix.

The tests validate that:

  1. The api_key field is properly exposed as SecretStr across different initialization scenarios.
  2. Each TavilyInternetSearchToolConfig instance receives a unique SerializableSecretStr object, preventing shared mutable defaults.

This test coverage directly validates the PR's bug fix.

packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py (3)

37-38: LGTM! Correct default_factory pattern for Langfuse keys.

The change ensures each LangfuseTelemetryExporter instance receives unique SerializableSecretStr objects for both public_key and secret_key.


75-75: LGTM! Proper default_factory for Langsmith API key.

Consistent with the pattern applied across other telemetry exporters.


140-140: LGTM! Correct default_factory for Patronus API key.

Aligns with the project-wide fix for SecretStr field initialization.

examples/agents/tests/test_agents.py (3)

23-23: LGTM! Import addition for REST API testing.


61-70: LGTM! Comprehensive dual-mode test coverage.

The parameterization of use_rest_api enables testing both local workflow execution and REST API serving modes. The conditional logic properly routes to serve_workflow or run_workflow based on the test mode.


75-91: LGTM! Consistent dual-mode test pattern.

The parameterization and conditional execution logic align with the pattern established in test_rewoo_full_workflow, providing consistent coverage across both execution modes.

Signed-off-by: David Gardner <dagardner@nvidia.com>
…olkit into david-default-secret

Signed-off-by: David Gardner <dagardner@nvidia.com>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (2)
packages/nvidia_nat_test/src/nat/test/utils.py (2)

105-105: Prefer async httpx over synchronous requests.

The function uses the synchronous requests library, but the coding guidelines state: "Prefer httpx with SSL verification enabled by default" and "Use async/await for I/O-bound work (HTTP, DB, file reads)". Since this is an async function, consider using httpx.AsyncClient for consistency with the async context.

As per coding guidelines, async/await should be used for I/O-bound work, and httpx is preferred over requests.


142-152: Improve process cleanup reliability.

The teardown logic attempts terminate/kill but doesn't log warnings if the process doesn't terminate after 5 attempts. Consider adding logging and handling potential ProcessLookupError exceptions.

🔎 Proposed enhancement
     finally:
         # Teardown
         i = 0
         while proc.poll() is None and i < 5:
-            if i == 0:
-                proc.terminate()
-            else:
-                proc.kill()
-            await asyncio.sleep(0.1)
-            i += 1
+            try:
+                if i == 0:
+                    proc.terminate()
+                else:
+                    proc.kill()
+            except ProcessLookupError:
+                break  # Process already terminated
+            await asyncio.sleep(0.1)
+            i += 1
 
         assert proc.poll() is not None, "NAT server process failed to terminate"
📜 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 b5d84d1 and 0935ce9.

📒 Files selected for processing (7)
  • examples/agents/tests/test_agents.py (3 hunks)
  • examples/evaluation_and_profiling/swe_bench/src/nat_swe_bench/config.py (1 hunks)
  • packages/nvidia_nat_langchain/src/nat/plugins/langchain/tools/tavily_internet_search.py (1 hunks)
  • packages/nvidia_nat_langchain/tests/test_tavily_internet_search.py (1 hunks)
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py (3 hunks)
  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/register.py (1 hunks)
  • packages/nvidia_nat_test/src/nat/test/utils.py (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • packages/nvidia_nat_langchain/src/nat/plugins/langchain/tools/tavily_internet_search.py
  • examples/evaluation_and_profiling/swe_bench/src/nat_swe_bench/config.py
  • packages/nvidia_nat_langchain/tests/test_tavily_internet_search.py
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{md,rst,py}

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

Use 'NVIDIA NeMo Agent toolkit' on first use, then 'NeMo Agent toolkit' for subsequent references

Files:

  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/register.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
  • examples/agents/tests/test_agents.py
  • packages/nvidia_nat_test/src/nat/test/utils.py
**/*.{py,toml,yaml,yml}

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

Use abbreviations: 'nat' for API namespace and CLI tool, 'nvidia-nat' for package name, 'NAT' for environment variable prefixes and informal comments

Files:

  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/register.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
  • examples/agents/tests/test_agents.py
  • packages/nvidia_nat_test/src/nat/test/utils.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 Python formatting
Use ruff check --fix (via pre-commit) for linting using configuration embedded in pyproject.toml, fix warnings unless explicitly ignored
Use snake_case for functions and variables, PascalCase for classes, UPPER_CASE for constants
All public APIs require Python 3.11+ type hints on parameters and return values
Prefer collections.abc / typing abstractions (Sequence over list) for type hints
Use typing.Annotated for units or extra metadata when useful
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 and logger.error() when re-raising
When catching and logging exceptions without re-raising, always use logger.exception() 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
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)
Cache expensive computations with functools.lru_cache or an external cache when appropriate
Leverage NumPy vectorised operations whenever beneficial and feasible

Files:

  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/register.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
  • examples/agents/tests/test_agents.py
  • packages/nvidia_nat_test/src/nat/test/utils.py
**/*.{py,js,ts,yaml,yml,json,md,rst}

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

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

Files:

  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/register.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
  • examples/agents/tests/test_agents.py
  • packages/nvidia_nat_test/src/nat/test/utils.py
**/*.{py,env,toml,yaml,yml,json}

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

Never commit API keys, credentials or personal data; use environment variables or .env files excluded from Git

Files:

  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/register.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
  • examples/agents/tests/test_agents.py
  • packages/nvidia_nat_test/src/nat/test/utils.py
**/*.{py,js,ts,java,cpp,c,go,rb,php}

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

Every file must start with the standard SPDX Apache-2.0 header

Files:

  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/register.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
  • examples/agents/tests/test_agents.py
  • packages/nvidia_nat_test/src/nat/test/utils.py
**/*.{py,md,rst}

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

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

Files:

  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/register.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
  • examples/agents/tests/test_agents.py
  • packages/nvidia_nat_test/src/nat/test/utils.py
**/*.{py,js,ts,java,cpp,c,go,rb,php,sh}

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

All source files must include the SPDX Apache-2.0 header template

Files:

  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/register.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
  • examples/agents/tests/test_agents.py
  • packages/nvidia_nat_test/src/nat/test/utils.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:

  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/register.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
  • examples/agents/tests/test_agents.py
  • packages/nvidia_nat_test/src/nat/test/utils.py
packages/**/*

⚙️ CodeRabbit configuration file

packages/**/*: - This directory contains optional plugin packages for the toolkit, each should contain a pyproject.toml file. - The pyproject.toml file should declare a dependency on nvidia-nat or another package with a name starting
with nvidia-nat-. This dependency should be declared using ~=<version>, and the version should be a two
digit version (ex: ~=1.0).

  • Not all packages contain Python code, if they do they should also contain their own set of tests, in a
    tests/ directory at the same level as the pyproject.toml file.
  • When adding a new package, that new package name (as defined in the pyproject.toml file) should
    be added as a dependency to the nvidia-nat-all package in packages/nvidia_nat_all/pyproject.toml

Files:

  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/register.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
  • packages/nvidia_nat_test/src/nat/test/utils.py
**/test_*.py

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

**/test_*.py: Use pytest with pytest-asyncio for asynchronous code testing
Test functions should be named using the test_ prefix, using snake_case
Extract frequently repeated code into pytest fixtures, which should be named using the fixture_ prefix and define the name argument in the decorator
Mock external services with pytest_httpserver or unittest.mock instead of hitting live endpoints
Mark slow tests with @pytest.mark.slow so they can be skipped in the default test suite
Mark integration tests requiring external services with @pytest.mark.integration so they can be skipped in the default test suite

Files:

  • examples/agents/tests/test_agents.py
examples/**/*

⚙️ CodeRabbit configuration file

examples/**/*: - This directory contains example code and usage scenarios for the toolkit, at a minimum an example should
contain a README.md or file README.ipynb.

  • If an example contains Python code, it should be placed in a subdirectory named src/ and should
    contain a pyproject.toml file. Optionally, it might also contain scripts in a scripts/ directory.
  • If an example contains YAML files, they should be placed in a subdirectory named configs/. - If an example contains sample data files, they should be placed in a subdirectory named data/, and should
    be checked into git-lfs.

Files:

  • examples/agents/tests/test_agents.py
🧠 Learnings (2)
📚 Learning: 2025-12-18T18:55:42.993Z
Learnt from: AnuradhaKaruppiah
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 1281
File: packages/nvidia_nat_a2a/src/nat/plugins/a2a/auth/credential_service.py:93-94
Timestamp: 2025-12-18T18:55:42.993Z
Learning: In packages/nvidia_nat_a2a/src/nat/plugins/a2a/auth/credential_service.py, the default_user_id parameter was intentionally removed from A2ACredentialService.__init__ to enhance security by enforcing per-user context. The implementation now exclusively uses Context.get().user_id to obtain user identification, preventing fallback to a shared default user.

Applied to files:

  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
📚 Learning: 2025-11-14T20:33:53.944Z
Learnt from: AnuradhaKaruppiah
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 1181
File: packages/nvidia_nat_test/tests/test_test_llm.py:419-484
Timestamp: 2025-11-14T20:33:53.944Z
Learning: The NVIDIA NeMo-Agent-Toolkit project uses pytest-asyncio in strict mode (the default), which requires pytest.mark.asyncio decorator on all async test functions. All async tests in packages/nvidia_nat_test/tests/test_test_llm.py consistently follow this pattern.

Applied to files:

  • examples/agents/tests/test_agents.py
  • packages/nvidia_nat_test/src/nat/test/utils.py
🧬 Code graph analysis (1)
examples/agents/tests/test_agents.py (1)
packages/nvidia_nat_test/src/nat/test/utils.py (2)
  • serve_workflow (94-154)
  • run_workflow (73-91)
🪛 Ruff (0.14.8)
packages/nvidia_nat_test/src/nat/test/utils.py

108-108: subprocess call: check for execution of untrusted input

(S603)


122-122: Do not catch blind exception: Exception

(BLE001)

⏰ 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 (8)
packages/nvidia_nat_test/src/nat/test/utils.py (1)

16-16: LGTM: Standard library imports are appropriate.

The asyncio and time imports are correctly placed and necessary for the new serve_workflow function's retry logic and async sleep operations.

Also applies to: 21-21

examples/agents/tests/test_agents.py (2)

23-23: LGTM: Import added for REST API test mode.

The import of serve_workflow is correctly placed and enables the new REST API-based test execution path.


76-76: LGTM: REST API test mode properly integrated.

The parametrization and conditional logic correctly route tests through either serve_workflow or run_workflow based on the use_rest_api flag. The implementation maintains consistency with the existing test patterns.

However, please verify that the port conflict concern mentioned in the previous comment is addressed to ensure reliable test execution.

Also applies to: 88-92

packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py (3)

77-78: Consistent pattern applied for api_key default.

The change correctly applies the same default_factory pattern as the Langfuse fields above. Ensure the truthiness check at line 89 (if config.api_key) works correctly with the empty SerializableSecretStr to preserve fallback to LANGSMITH_API_KEY.


143-144: Consistent pattern applied across all modified exporters.

The default_factory pattern is applied consistently with the previous two exporters. The fallback logic at line 155 follows the same pattern as Langfuse and Langsmith.


37-40: Use default_factory correctly to ensure SerializableSecretStr instantiation.

The change from default="" to default_factory=lambda: SerializableSecretStr("") correctly ensures each model instance receives a proper SecretStr object rather than a plain string. This fixes the Pydantic instantiation issue where plain strings would break the get_secret_value() call.

The fallback logic at lines 52-53 relies on truthiness checks (if config.secret_key), which works correctly because SecretStr("") (empty string wrapped in SecretStr) evaluates to falsy, preserving the environment variable fallback behavior. However, for consistency and clarity, consider adopting the OptionalSecretStr pattern with default=None (as in DBNLTelemetryExporter at line 209) for optional secrets with environment variable fallbacks, which better expresses intent and eliminates reliance on empty object truthiness.

packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/register.py (2)

34-37: LGTM! Correct fix for shared default instance bug.

Using default_factory ensures each CatalystTelemetryExporter instance gets its own SerializableSecretStr("") object, avoiding the shared mutable default problem and ensuring type correctness. This is the idiomatic Pydantic v2 pattern for this scenario.


56-57: No action required. The truthiness checks on lines 56-57 function correctly with the new default_factory pattern. SerializableSecretStr("") objects evaluate to False due to Pydantic's SecretStr implementation of __len__, which returns 0 for empty strings. The fallback to environment variables (os.environ.get()) triggers as intended when the default empty secret is used.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (2)
packages/nvidia_nat_test/src/nat/test/utils.py (2)

102-106: Docstring issues already flagged in previous review.

The typo "Intendted" and missing Args/Returns/Raises sections have been identified in previous review comments.


124-124: Broad exception handling already flagged in previous review.

The bare except Exception: has been identified in previous review comments with specific recommendations to narrow to requests.RequestException or relevant httpx exceptions.

🧹 Nitpick comments (1)
packages/nvidia_nat_test/src/nat/test/utils.py (1)

127-127: Avoid blocking read of process output in assertion.

The proc.stdout.read() call in the assertion may block and will consume all buffered output, potentially leaving nothing available for debugging later failures in the cleanup section.

Consider either:

  1. Using proc.poll() to check process status without reading output
  2. Setting up non-blocking output capture from the start
  3. Reading a limited amount with a timeout
🔎 Proposed fix
-        assert response is not None, f"deadline exceeded waiting for workflow response: {proc.stdout.read()}"
+        assert response is not None, f"deadline exceeded waiting for workflow response. Server may have failed to start or respond (exit code: {proc.poll()})"
📜 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 0935ce9 and a71aaf8.

📒 Files selected for processing (1)
  • packages/nvidia_nat_test/src/nat/test/utils.py (2 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{md,rst,py}

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

Use 'NVIDIA NeMo Agent toolkit' on first use, then 'NeMo Agent toolkit' for subsequent references

Files:

  • packages/nvidia_nat_test/src/nat/test/utils.py
**/*.{py,toml,yaml,yml}

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

Use abbreviations: 'nat' for API namespace and CLI tool, 'nvidia-nat' for package name, 'NAT' for environment variable prefixes and informal comments

Files:

  • packages/nvidia_nat_test/src/nat/test/utils.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 Python formatting
Use ruff check --fix (via pre-commit) for linting using configuration embedded in pyproject.toml, fix warnings unless explicitly ignored
Use snake_case for functions and variables, PascalCase for classes, UPPER_CASE for constants
All public APIs require Python 3.11+ type hints on parameters and return values
Prefer collections.abc / typing abstractions (Sequence over list) for type hints
Use typing.Annotated for units or extra metadata when useful
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 and logger.error() when re-raising
When catching and logging exceptions without re-raising, always use logger.exception() 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
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)
Cache expensive computations with functools.lru_cache or an external cache when appropriate
Leverage NumPy vectorised operations whenever beneficial and feasible

Files:

  • packages/nvidia_nat_test/src/nat/test/utils.py
**/*.{py,js,ts,yaml,yml,json,md,rst}

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

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

Files:

  • packages/nvidia_nat_test/src/nat/test/utils.py
**/*.{py,env,toml,yaml,yml,json}

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

Never commit API keys, credentials or personal data; use environment variables or .env files excluded from Git

Files:

  • packages/nvidia_nat_test/src/nat/test/utils.py
**/*.{py,js,ts,java,cpp,c,go,rb,php}

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

Every file must start with the standard SPDX Apache-2.0 header

Files:

  • packages/nvidia_nat_test/src/nat/test/utils.py
**/*.{py,md,rst}

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

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

Files:

  • packages/nvidia_nat_test/src/nat/test/utils.py
**/*.{py,js,ts,java,cpp,c,go,rb,php,sh}

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

All source files must include the SPDX Apache-2.0 header template

Files:

  • packages/nvidia_nat_test/src/nat/test/utils.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:

  • packages/nvidia_nat_test/src/nat/test/utils.py
packages/**/*

⚙️ CodeRabbit configuration file

packages/**/*: - This directory contains optional plugin packages for the toolkit, each should contain a pyproject.toml file. - The pyproject.toml file should declare a dependency on nvidia-nat or another package with a name starting
with nvidia-nat-. This dependency should be declared using ~=<version>, and the version should be a two
digit version (ex: ~=1.0).

  • Not all packages contain Python code, if they do they should also contain their own set of tests, in a
    tests/ directory at the same level as the pyproject.toml file.
  • When adding a new package, that new package name (as defined in the pyproject.toml file) should
    be added as a dependency to the nvidia-nat-all package in packages/nvidia_nat_all/pyproject.toml

Files:

  • packages/nvidia_nat_test/src/nat/test/utils.py
🧠 Learnings (1)
📚 Learning: 2025-09-15T21:26:29.430Z
Learnt from: saglave
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 726
File: examples/frameworks/adk_demo/src/nat_adk_demo/weather_update_tool.py:0-0
Timestamp: 2025-09-15T21:26:29.430Z
Learning: In NAT's ADK integration, the docstring of registered functions serves as the tool description sent to the LLM, not standard Python function documentation. The docstring should describe the logical tool interface (parameters the LLM will provide) rather than the NAT framework wrapper parameters like tool_config and builder.

Applied to files:

  • packages/nvidia_nat_test/src/nat/test/utils.py
🪛 Ruff (0.14.8)
packages/nvidia_nat_test/src/nat/test/utils.py

110-110: subprocess call: check for execution of untrusted input

(S603)


124-124: Do not catch blind exception: Exception

(BLE001)

⏰ 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 (3)
packages/nvidia_nat_test/src/nat/test/utils.py (3)

16-16: LGTM: Import additions support the new async workflow testing.

The asyncio and time imports are appropriately used for the retry loop and timeout management in the new serve_workflow function.

Also applies to: 21-21


128-142: LGTM: Response handling logic is robust.

The response processing correctly handles multiple response formats (string values and choice arrays) and validates the expected answer when requested. The use of raise_for_status() ensures HTTP errors are properly caught.


143-156: LGTM: Cleanup logic properly handles process termination.

The finally block ensures the NAT server process is terminated even if errors occur. The escalation from terminate() to kill() with retry logic is a robust approach to ensure the process doesn't leak.

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Signed-off-by: David Gardner <96306125+dagardner-nv@users.noreply.github.com>
@dagardner-nv
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit b1f764e into NVIDIA:release/1.4 Dec 19, 2025
17 checks passed
@dagardner-nv dagardner-nv deleted the david-default-secret branch December 19, 2025 19:51
rapids-bot bot pushed a commit that referenced this pull request Dec 19, 2025
This PR fixes the bug that `TypeConverter` not able to handle `Union` type conversion correctly. 

This needs to be merged after [PR-1298](#1298)

Closes AIQ-2600

## By Submitting this PR I confirm:
- I am familiar with the [Contributing Guidelines](https://github.com/NVIDIA/NeMo-Agent-Toolkit/blob/develop/docs/source/resources/contributing/index.md).
- 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.

<sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub>

Authors:
  - Yuchen Zhang (https://github.com/yczhang-nv)
  - David Gardner (https://github.com/dagardner-nv)

Approvers:
  - David Gardner (https://github.com/dagardner-nv)

URL: #1301
yczhang-nv pushed a commit to yczhang-nv/NeMo-Agent-Toolkit that referenced this pull request Dec 22, 2025
…ot being instantiated (NVIDIA#1298)

* Adds `nat serve` integration tests for the agent example workflows

## By Submitting this PR I confirm:
- I am familiar with the [Contributing Guidelines](https://github.com/NVIDIA/NeMo-Agent-Toolkit/blob/develop/docs/source/resources/contributing/index.md).
- 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

* **Tests**
  * Added parameterized workflow tests to run both direct and REST API server modes; added tests ensuring API key/secret fields are exposed as secrets and unique per instance.
  * Added a helper to exercise workflows via the REST API for test coverage.

* **New Features**
  * Support for testing workflows via a REST API server mode alongside direct execution.

* **Refactor**
  * Secret credential defaults changed to per-instance secret objects to avoid shared default instances.

<sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub>

Authors:
  - David Gardner (https://github.com/dagardner-nv)

Approvers:
  - Yuchen Zhang (https://github.com/yczhang-nv)

URL: NVIDIA#1298
Signed-off-by: Yuchen Zhang <yuchenz@nvidia.com>
yczhang-nv added a commit to yczhang-nv/NeMo-Agent-Toolkit that referenced this pull request Dec 22, 2025
…A#1301)

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

This needs to be merged after [PR-1298](NVIDIA#1298)

Closes AIQ-2600

## By Submitting this PR I confirm:
- I am familiar with the [Contributing Guidelines](https://github.com/NVIDIA/NeMo-Agent-Toolkit/blob/develop/docs/source/resources/contributing/index.md).
- 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.

<sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub>

Authors:
  - Yuchen Zhang (https://github.com/yczhang-nv)
  - David Gardner (https://github.com/dagardner-nv)

Approvers:
  - David Gardner (https://github.com/dagardner-nv)

URL: NVIDIA#1301
Signed-off-by: Yuchen Zhang <yuchenz@nvidia.com>
yczhang-nv pushed a commit to yczhang-nv/NeMo-Agent-Toolkit that referenced this pull request Dec 22, 2025
…ot being instantiated (NVIDIA#1298)

* Adds `nat serve` integration tests for the agent example workflows

## By Submitting this PR I confirm:
- I am familiar with the [Contributing Guidelines](https://github.com/NVIDIA/NeMo-Agent-Toolkit/blob/develop/docs/source/resources/contributing/index.md).
- 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

* **Tests**
  * Added parameterized workflow tests to run both direct and REST API server modes; added tests ensuring API key/secret fields are exposed as secrets and unique per instance.
  * Added a helper to exercise workflows via the REST API for test coverage.

* **New Features**
  * Support for testing workflows via a REST API server mode alongside direct execution.

* **Refactor**
  * Secret credential defaults changed to per-instance secret objects to avoid shared default instances.

<sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub>

Authors:
  - David Gardner (https://github.com/dagardner-nv)

Approvers:
  - Yuchen Zhang (https://github.com/yczhang-nv)

URL: NVIDIA#1298
Signed-off-by: Yuchen Zhang <yuchenz@nvidia.com>
yczhang-nv added a commit to yczhang-nv/NeMo-Agent-Toolkit that referenced this pull request Dec 22, 2025
…A#1301)

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

This needs to be merged after [PR-1298](NVIDIA#1298)

Closes AIQ-2600

## By Submitting this PR I confirm:
- I am familiar with the [Contributing Guidelines](https://github.com/NVIDIA/NeMo-Agent-Toolkit/blob/develop/docs/source/resources/contributing/index.md).
- 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.

<sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub>

Authors:
  - Yuchen Zhang (https://github.com/yczhang-nv)
  - David Gardner (https://github.com/dagardner-nv)

Approvers:
  - David Gardner (https://github.com/dagardner-nv)

URL: NVIDIA#1301
Signed-off-by: Yuchen Zhang <yuchenz@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants