Skip to content

Conversation

@dagardner-nv
Copy link
Contributor

@dagardner-nv dagardner-nv commented Oct 3, 2025

Description

  • Work around Implicit namespace packages do not generate documentation readthedocs/sphinx-autoapi#298 by copying the contents of plugin packages into the staged API tree. Also refactor this logic into a method.
  • Rename the span_to_dfw_record module to span_to_dfw this fixes an issue where the span_to_dfw_record contained a function of the same name, which was also imported into the package, making the rest of the module difficult to import.
  • Rename dataflyweel's Request class to ESRequest as this was conflicting with the nat.data_models.api_server.Request class
  • Fix Sphinx docstring parsing errors such as **kwargs in a parameter list is interpreted as a bold open, without a closing **.
  • Add missing __init__.py files
  • Since dataflywheel was not included in v1.2, I'm claiming this to be a non-breaking change.

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

  • Refactor
    • Streamlined docs build with dynamic API tree generation, improved path handling, and path logging.
    • Renamed Elasticsearch request model to ESRequest; updated related components accordingly.
    • OTLP span redaction exporter now accepts an otlp_kwargs dict instead of variadic kwargs.
  • Documentation
    • Standardized docstrings across plugins: kwargs naming, “Example::” formatting, and spacing; no behavioral changes.
  • Tests
    • Updated tests to new trace conversion module paths; coverage preserved.
  • Chores
    • Added licensing boilerplate to a utilities package initializer.

@dagardner-nv dagardner-nv self-assigned this Oct 3, 2025
@dagardner-nv dagardner-nv added doc Improvements or additions to documentation skip-ci Optionally Skip CI for this PR non-breaking Non-breaking change labels Oct 3, 2025
@coderabbitai
Copy link

coderabbitai bot commented Oct 3, 2025

Walkthrough

Introduces a new API tree builder in Sphinx conf.py using pathlib and filesystem operations. Updates Data Flywheel Elasticsearch schema to use ESRequest, adapts converter and tests accordingly. Adjusts a constructor signature in OTLP span redaction exporter. Numerous docstring-only edits across multiple plugins. Adds a boilerplate init.py in utils.

Changes

Cohort / File(s) Summary of changes
Docs Sphinx API tree builder
docs/source/conf.py
Replaced hard-coded path setup with _build_api_tree() using pathlib.Path; cleans and rebuilds _api_tree, ensures __init__.py, copies NAT plugin packages; assigns API_TREE = _build_api_tree(); sets autoapi_dirs to str(API_TREE.absolute()); added a print of built path.
Data Flywheel ES schema and adapters
packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/schema/sink/elasticsearch/dfw_es_record.py, .../processor/trace_conversion/adapter/elasticsearch/openai_converter.py, .../processor/trace_conversion/__init__.py, packages/nvidia_nat_data_flywheel/tests/observability/processor/trace_conversion/test_span_to_dfw_record.py
Introduced ESRequest model; switched DFWESRecord.request from Request to ESRequest; updated OpenAI converter to construct ESRequest; changed import source of span_to_dfw_record to .span_to_dfw; updated tests and patches to new module paths and types.
OpenTelemetry exporters/mixins
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/otlp_span_redaction_adapter_exporter.py, .../otlp_span_adapter_exporter.py, .../mixin/otlp_span_exporter_mixin.py
Changed OTLPSpanHeaderRedactionAdapterExporter.__init__ signature from **otlp_kwargs to named otlp_kwargs (dict) and forwarded with **otlp_kwargs; docstring formatting updates (Example::, parameter docs).
Boilerplate addition
packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/utils/__init__.py
New file with SPDX license header; no code.
Docstring-only adjustments (no logic changes)
packages/nvidia_nat_adk/src/nat/plugins/adk/adk_callback_handler.py, packages/nvidia_nat_agno/src/nat/plugins/agno/tool_wrapper.py, packages/nvidia_nat_crewai/src/nat/plugins/crewai/crewai_callback_handler.py, packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/exporter/dfw_elasticsearch_exporter.py, packages/nvidia_nat_mcp/src/nat/plugins/mcp/client_impl.py, packages/nvidia_nat_mem0ai/src/nat/plugins/mem0ai/mem0_editor.py, packages/nvidia_nat_phoenix/src/nat/plugins/phoenix/mixin/phoenix_mixin.py, packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/mixin/ragaai_catalyst_mixin.py, packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/ragaai_catalyst_exporter.py, packages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.py
Formatting and parameter name text changes in docstrings (e.g., **kwargskwargs, “Example:” → “Example::”, spacing). No code or API behavior changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Importer as Sphinx conf.py import
  participant Conf as conf.py
  participant FS as Filesystem
  Importer->>Conf: Import module
  activate Conf
  Conf->>Conf: _build_api_tree()
  activate Conf
  Conf->>FS: Remove existing _api_tree (if exists)
  FS-->>Conf: Removed/Not found
  Conf->>FS: Create dirs: _api_tree/nat, _api_tree/plugins, build, etc.
  Conf->>FS: Ensure __init__.py in nat/ and plugins/
  Conf->>FS: Copy NVIDIA NAT hook plugin dirs to plugins/
  Conf->>FS: Copy plugin subpackages into destination\n(Create empty __init__.py if missing)
  Conf-->>Importer: API_TREE Path
  deactivate Conf
  Conf->>Conf: Set autoapi_dirs = [str(API_TREE.absolute())]
  Conf->>Importer: Print "API tree built at {API_TREE}"
  deactivate Conf
Loading
sequenceDiagram
  autonumber
  participant Conv as openai_converter.convert_langchain_openai
  participant Models as dfw_es_record.ESRequest
  participant Record as DFWESRecord
  Conv->>Models: ESRequest(**payload)
  Models-->>Conv: ESRequest instance
  Conv->>Record: DFWESRecord(request=ESRequest, ...)
  Record-->>Conv: DFWESRecord instance
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

breaking

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly summarizes the main change of copying and including plugin packages into the staged API tree for documentation builds, uses imperative mood, is concise, and falls within the character limit.
Docstring Coverage ✅ Passed Docstring coverage is 93.10% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ 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

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

Signed-off-by: David Gardner <[email protected]>
Signed-off-by: David Gardner <[email protected]>
Signed-off-by: David Gardner <[email protected]>
Signed-off-by: David Gardner <[email protected]>
This reverts commit a20826b.

Signed-off-by: David Gardner <[email protected]>
Signed-off-by: David Gardner <[email protected]>
Signed-off-by: David Gardner <[email protected]>
…e had a function of the same name and because the function was imported into the package, this made the rest of the module difficult to access

Signed-off-by: David Gardner <[email protected]>
Signed-off-by: David Gardner <[email protected]>
Signed-off-by: David Gardner <[email protected]>
Signed-off-by: David Gardner <[email protected]>
Signed-off-by: David Gardner <[email protected]>
@dagardner-nv dagardner-nv removed the skip-ci Optionally Skip CI for this PR label Oct 3, 2025
@dagardner-nv dagardner-nv marked this pull request as ready for review October 3, 2025 23:11
@dagardner-nv dagardner-nv requested a review from a team as a code owner October 3, 2025 23:11
@coderabbitai coderabbitai bot added the breaking Breaking change label Oct 3, 2025
@dagardner-nv dagardner-nv removed the breaking Breaking change label Oct 3, 2025
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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/nvidia_nat_mem0ai/src/nat/plugins/mem0ai/mem0_editor.py (1)

105-115: Add missing return type hint and docstring.

This public method lacks a return type hint and docstring, which violates the coding guidelines requiring type hints for all public API parameters and return values, as well as Google-style docstrings for all public functions.

Apply this diff to add the missing annotations and documentation:

-    async def remove_items(self, **kwargs):
+    async def remove_items(self, **kwargs) -> None:
+        """
+        Remove memory items based on provided identifiers.
+
+        Args:
+            kwargs: Keyword arguments specifying items to remove.
+                memory_id (str, optional): Remove a specific memory by ID.
+                user_id (str, optional): Remove all memories for a user.
+
+        Returns:
+            None
+        """
🧹 Nitpick comments (3)
packages/nvidia_nat_mem0ai/src/nat/plugins/mem0ai/mem0_editor.py (1)

44-48: Consider enhancing the docstring with Args and Returns sections.

While the current docstring is concise, the coding guidelines specify Google-style docstrings for all public functions. Adding Args: and Returns: sections would improve clarity and consistency across the codebase.

Example enhancement:

     async def add_items(self, items: list[MemoryItem]) -> None:
         """
         Insert Multiple MemoryItems into the memory.
         Each MemoryItem is translated and uploaded.
+
+        Args:
+            items (list[MemoryItem]): The memory items to add.
+
+        Returns:
+            None
         """
docs/source/conf.py (2)

82-83: Consider logging instead of print for build output.

The print() statement on line 83 works but is not consistent with typical Sphinx extension practices, which often use Sphinx's logging mechanism or warnings for build-time messages.

Consider replacing the print statement with a logging call for better integration with Sphinx's output handling:

-print(f"API tree built at {API_TREE}")
+import logging
+logger = logging.getLogger(__name__)
+logger.info(f"API tree built at {API_TREE}")

Or keep the print but add more context:

-print(f"API tree built at {API_TREE}")
+print(f"[sphinx-autoapi] API tree built at {API_TREE}")

41-79: Validate directories and optionally add error handling in _build_api_tree

  • Check that nat_dir and plugins_dir exist before attempting to copy
  • (Optional) Wrap rmtree, makedirs, and copytree calls in try/except to emit clearer errors and clean up partially-built trees on failure
📜 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 4ee0dfd and 2de0a22.

📒 Files selected for processing (19)
  • docs/source/conf.py (2 hunks)
  • packages/nvidia_nat_adk/src/nat/plugins/adk/adk_callback_handler.py (1 hunks)
  • packages/nvidia_nat_agno/src/nat/plugins/agno/tool_wrapper.py (1 hunks)
  • packages/nvidia_nat_crewai/src/nat/plugins/crewai/crewai_callback_handler.py (1 hunks)
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/exporter/dfw_elasticsearch_exporter.py (1 hunks)
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/processor/trace_conversion/__init__.py (1 hunks)
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/processor/trace_conversion/adapter/elasticsearch/openai_converter.py (3 hunks)
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/schema/sink/elasticsearch/dfw_es_record.py (3 hunks)
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/utils/__init__.py (1 hunks)
  • packages/nvidia_nat_data_flywheel/tests/observability/processor/trace_conversion/test_span_to_dfw_record.py (15 hunks)
  • packages/nvidia_nat_mcp/src/nat/plugins/mcp/client_impl.py (3 hunks)
  • packages/nvidia_nat_mem0ai/src/nat/plugins/mem0ai/mem0_editor.py (1 hunks)
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/mixin/otlp_span_exporter_mixin.py (1 hunks)
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/otlp_span_adapter_exporter.py (2 hunks)
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/otlp_span_redaction_adapter_exporter.py (2 hunks)
  • packages/nvidia_nat_phoenix/src/nat/plugins/phoenix/mixin/phoenix_mixin.py (1 hunks)
  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/mixin/ragaai_catalyst_mixin.py (2 hunks)
  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/ragaai_catalyst_exporter.py (1 hunks)
  • packages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.py (1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{py,yaml,yml}

📄 CodeRabbit inference engine (.cursor/rules/nat-test-llm.mdc)

**/*.{py,yaml,yml}: Configure response_seq as a list of strings; values cycle per call, and [] yields an empty string.
Configure delay_ms to inject per-call artificial latency in milliseconds for nat_test_llm.

Files:

  • packages/nvidia_nat_crewai/src/nat/plugins/crewai/crewai_callback_handler.py
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/utils/__init__.py
  • packages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.py
  • packages/nvidia_nat_agno/src/nat/plugins/agno/tool_wrapper.py
  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/mixin/ragaai_catalyst_mixin.py
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/exporter/dfw_elasticsearch_exporter.py
  • packages/nvidia_nat_mcp/src/nat/plugins/mcp/client_impl.py
  • packages/nvidia_nat_mem0ai/src/nat/plugins/mem0ai/mem0_editor.py
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/schema/sink/elasticsearch/dfw_es_record.py
  • docs/source/conf.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/otlp_span_adapter_exporter.py
  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/ragaai_catalyst_exporter.py
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/processor/trace_conversion/__init__.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/otlp_span_redaction_adapter_exporter.py
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/processor/trace_conversion/adapter/elasticsearch/openai_converter.py
  • packages/nvidia_nat_data_flywheel/tests/observability/processor/trace_conversion/test_span_to_dfw_record.py
  • packages/nvidia_nat_phoenix/src/nat/plugins/phoenix/mixin/phoenix_mixin.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/mixin/otlp_span_exporter_mixin.py
  • packages/nvidia_nat_adk/src/nat/plugins/adk/adk_callback_handler.py
**/*.py

📄 CodeRabbit inference engine (.cursor/rules/nat-test-llm.mdc)

**/*.py: Programmatic use: create TestLLMConfig(response_seq=[...], delay_ms=...), add with builder.add_llm("", cfg).
When retrieving the test LLM wrapper, use builder.get_llm(name, wrapper_type=LLMFrameworkEnum.) and call the framework’s method (e.g., ainvoke, achat, call).

**/*.py: In code comments/identifiers use NAT abbreviations as specified: nat for API namespace/CLI, nvidia-nat for package name, NAT for env var prefixes; do not use these abbreviations in documentation
Follow PEP 20 and PEP 8; run yapf with column_limit=120; use 4-space indentation; end files with a single trailing newline
Run ruff check --fix as linter (not formatter) using pyproject.toml config; fix warnings unless explicitly ignored
Respect naming: snake_case for functions/variables, PascalCase for classes, UPPER_CASE for constants
Treat pyright warnings as errors during development
Exception handling: use bare raise to re-raise; log with logger.error() when re-raising to avoid duplicate stack traces; use logger.exception() when catching without re-raising
Provide Google-style docstrings for every public module, class, function, and CLI command; first line concise and ending with a period; surround code entities with backticks
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; profile CPU-heavy paths with cProfile or mprof before optimizing; cache expensive computations with functools.lru_cache or external cache; leverage NumPy vectorized operations when beneficial

Files:

  • packages/nvidia_nat_crewai/src/nat/plugins/crewai/crewai_callback_handler.py
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/utils/__init__.py
  • packages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.py
  • packages/nvidia_nat_agno/src/nat/plugins/agno/tool_wrapper.py
  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/mixin/ragaai_catalyst_mixin.py
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/exporter/dfw_elasticsearch_exporter.py
  • packages/nvidia_nat_mcp/src/nat/plugins/mcp/client_impl.py
  • packages/nvidia_nat_mem0ai/src/nat/plugins/mem0ai/mem0_editor.py
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/schema/sink/elasticsearch/dfw_es_record.py
  • docs/source/conf.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/otlp_span_adapter_exporter.py
  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/ragaai_catalyst_exporter.py
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/processor/trace_conversion/__init__.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/otlp_span_redaction_adapter_exporter.py
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/processor/trace_conversion/adapter/elasticsearch/openai_converter.py
  • packages/nvidia_nat_data_flywheel/tests/observability/processor/trace_conversion/test_span_to_dfw_record.py
  • packages/nvidia_nat_phoenix/src/nat/plugins/phoenix/mixin/phoenix_mixin.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/mixin/otlp_span_exporter_mixin.py
  • packages/nvidia_nat_adk/src/nat/plugins/adk/adk_callback_handler.py
packages/*/src/**/*.py

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

Importable Python code inside packages must live under packages//src/

Files:

  • packages/nvidia_nat_crewai/src/nat/plugins/crewai/crewai_callback_handler.py
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/utils/__init__.py
  • packages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.py
  • packages/nvidia_nat_agno/src/nat/plugins/agno/tool_wrapper.py
  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/mixin/ragaai_catalyst_mixin.py
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/exporter/dfw_elasticsearch_exporter.py
  • packages/nvidia_nat_mcp/src/nat/plugins/mcp/client_impl.py
  • packages/nvidia_nat_mem0ai/src/nat/plugins/mem0ai/mem0_editor.py
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/schema/sink/elasticsearch/dfw_es_record.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/otlp_span_adapter_exporter.py
  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/ragaai_catalyst_exporter.py
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/processor/trace_conversion/__init__.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/otlp_span_redaction_adapter_exporter.py
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/processor/trace_conversion/adapter/elasticsearch/openai_converter.py
  • packages/nvidia_nat_phoenix/src/nat/plugins/phoenix/mixin/phoenix_mixin.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/mixin/otlp_span_exporter_mixin.py
  • packages/nvidia_nat_adk/src/nat/plugins/adk/adk_callback_handler.py
{src/**/*.py,packages/*/src/**/*.py}

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

All public APIs must have Python 3.11+ type hints on parameters and return values; prefer typing/collections.abc abstractions; use typing.Annotated when useful

Files:

  • packages/nvidia_nat_crewai/src/nat/plugins/crewai/crewai_callback_handler.py
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/utils/__init__.py
  • packages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.py
  • packages/nvidia_nat_agno/src/nat/plugins/agno/tool_wrapper.py
  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/mixin/ragaai_catalyst_mixin.py
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/exporter/dfw_elasticsearch_exporter.py
  • packages/nvidia_nat_mcp/src/nat/plugins/mcp/client_impl.py
  • packages/nvidia_nat_mem0ai/src/nat/plugins/mem0ai/mem0_editor.py
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/schema/sink/elasticsearch/dfw_es_record.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/otlp_span_adapter_exporter.py
  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/ragaai_catalyst_exporter.py
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/processor/trace_conversion/__init__.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/otlp_span_redaction_adapter_exporter.py
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/processor/trace_conversion/adapter/elasticsearch/openai_converter.py
  • packages/nvidia_nat_phoenix/src/nat/plugins/phoenix/mixin/phoenix_mixin.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/mixin/otlp_span_exporter_mixin.py
  • packages/nvidia_nat_adk/src/nat/plugins/adk/adk_callback_handler.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:

  • packages/nvidia_nat_crewai/src/nat/plugins/crewai/crewai_callback_handler.py
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/utils/__init__.py
  • packages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.py
  • packages/nvidia_nat_agno/src/nat/plugins/agno/tool_wrapper.py
  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/mixin/ragaai_catalyst_mixin.py
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/exporter/dfw_elasticsearch_exporter.py
  • packages/nvidia_nat_mcp/src/nat/plugins/mcp/client_impl.py
  • packages/nvidia_nat_mem0ai/src/nat/plugins/mem0ai/mem0_editor.py
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/schema/sink/elasticsearch/dfw_es_record.py
  • docs/source/conf.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/otlp_span_adapter_exporter.py
  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/ragaai_catalyst_exporter.py
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/processor/trace_conversion/__init__.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/otlp_span_redaction_adapter_exporter.py
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/processor/trace_conversion/adapter/elasticsearch/openai_converter.py
  • packages/nvidia_nat_data_flywheel/tests/observability/processor/trace_conversion/test_span_to_dfw_record.py
  • packages/nvidia_nat_phoenix/src/nat/plugins/phoenix/mixin/phoenix_mixin.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/mixin/otlp_span_exporter_mixin.py
  • packages/nvidia_nat_adk/src/nat/plugins/adk/adk_callback_handler.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.

Files:

  • packages/nvidia_nat_crewai/src/nat/plugins/crewai/crewai_callback_handler.py
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/utils/__init__.py
  • packages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.py
  • packages/nvidia_nat_agno/src/nat/plugins/agno/tool_wrapper.py
  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/mixin/ragaai_catalyst_mixin.py
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/exporter/dfw_elasticsearch_exporter.py
  • packages/nvidia_nat_mcp/src/nat/plugins/mcp/client_impl.py
  • packages/nvidia_nat_mem0ai/src/nat/plugins/mem0ai/mem0_editor.py
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/schema/sink/elasticsearch/dfw_es_record.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/otlp_span_adapter_exporter.py
  • packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/ragaai_catalyst_exporter.py
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/processor/trace_conversion/__init__.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/otlp_span_redaction_adapter_exporter.py
  • packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/processor/trace_conversion/adapter/elasticsearch/openai_converter.py
  • packages/nvidia_nat_data_flywheel/tests/observability/processor/trace_conversion/test_span_to_dfw_record.py
  • packages/nvidia_nat_phoenix/src/nat/plugins/phoenix/mixin/phoenix_mixin.py
  • packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/mixin/otlp_span_exporter_mixin.py
  • packages/nvidia_nat_adk/src/nat/plugins/adk/adk_callback_handler.py
docs/source/**/*

⚙️ CodeRabbit configuration file

This directory contains the source code for the documentation. All documentation should be written in Markdown format. Any image files should be placed in the docs/source/_static directory.

Files:

  • docs/source/conf.py
packages/*/tests/**/*.py

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

If a package contains Python code, include tests in a tests/ directory at the same level as pyproject.toml

Files:

  • packages/nvidia_nat_data_flywheel/tests/observability/processor/trace_conversion/test_span_to_dfw_record.py
🧠 Learnings (1)
📚 Learning: 2025-09-15T21:26:29.430Z
Learnt from: saglave
PR: NVIDIA/NeMo-Agent-Toolkit#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_agno/src/nat/plugins/agno/tool_wrapper.py
🧬 Code graph analysis (4)
packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/schema/sink/elasticsearch/dfw_es_record.py (1)
packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/schema/sink/elasticsearch/contract_version.py (1)
  • ContractVersion (23-31)
packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/processor/trace_conversion/__init__.py (1)
packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/processor/trace_conversion/span_to_dfw.py (1)
  • span_to_dfw_record (99-119)
packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/processor/trace_conversion/adapter/elasticsearch/openai_converter.py (1)
packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/schema/sink/elasticsearch/dfw_es_record.py (1)
  • ESRequest (139-151)
packages/nvidia_nat_data_flywheel/tests/observability/processor/trace_conversion/test_span_to_dfw_record.py (4)
packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/processor/trace_conversion/span_to_dfw.py (2)
  • get_trace_container (45-96)
  • span_to_dfw_record (99-119)
packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/processor/trace_conversion/trace_adapter_registry.py (1)
  • list_registered_types (242-248)
packages/nvidia_nat_data_flywheel/tests/observability/schema/test_trace_container.py (1)
  • MockTraceSource (28-31)
packages/nvidia_nat_data_flywheel/tests/observability/processor/test_dfw_record_processor.py (1)
  • MockDFWRecord (29-35)
🔇 Additional comments (30)
packages/nvidia_nat_ragaai/src/nat/plugins/ragaai/mixin/ragaai_catalyst_mixin.py (2)

188-188: LGTM! Correct reStructuredText formatting for Sphinx.

The change from "Example:" to "Example::" is the proper reStructuredText syntax for introducing code blocks in Sphinx documentation, resolving the parsing issue mentioned in the PR description.


215-217: LGTM! Improved docstring readability.

The line wrapping of the long parameter descriptions improves readability while maintaining the same semantic content. This aligns with PEP 8 line length recommendations.

packages/nvidia_nat_mem0ai/src/nat/plugins/mem0ai/mem0_editor.py (2)

79-79: LGTM!

The docstring change correctly prevents Sphinx from interpreting **kwargs as bold markers while preserving the semantic meaning.


86-86: No direct calls to .search( found in codebase. The review concern remains valid: search requires user_id via kwargs.pop, but neither the signature nor docstring specifies it. Add explicit validation with a clear error or update the docstring to state that user_id is required.

packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/mixin/otlp_span_exporter_mixin.py (1)

38-39: LGTM!

The docstring formatting change to use Example:: aligns with reStructuredText conventions for Sphinx documentation.

packages/nvidia_nat_phoenix/src/nat/plugins/phoenix/mixin/phoenix_mixin.py (1)

38-39: LGTM!

The docstring formatting change to use Example:: aligns with reStructuredText conventions for Sphinx documentation.

packages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.py (1)

67-67: LGTM!

The docstring parameter name change from **kwargs to kwargs prevents Sphinx from interpreting the double asterisks as bold markdown, which is consistent with the PR's objective to fix docstring parsing errors.

packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/otlp_span_adapter_exporter.py (2)

46-47: LGTM!

The docstring formatting change to use Example:: aligns with reStructuredText conventions for Sphinx documentation.


83-83: LGTM!

The docstring parameter name change from **otlp_kwargs to otlp_kwargs prevents Sphinx from interpreting the double asterisks as bold markdown, which is consistent with the PR's objective to fix docstring parsing errors.

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

58-59: LGTM!

The docstring formatting change to use Example:: aligns with reStructuredText conventions for Sphinx documentation.


120-120: LGTM!

The docstring parameter name change from **otlp_kwargs to otlp_kwargs prevents Sphinx from interpreting the double asterisks as bold markdown, which is consistent with the PR's objective to fix docstring parsing errors.


99-99: Incorrect signature-change concern: the constructor still uses **otlp_kwargs, so no breaking change was introduced.

Likely an incorrect or invalid review comment.

docs/source/conf.py (3)

30-30: LGTM!

The import of glob is necessary for finding plugin directories and is used appropriately on line 66.


35-35: LGTM!

The import of Path from pathlib modernizes path handling and is used consistently throughout the new _build_api_tree() function.


118-118: LGTM!

Converting the Path object to a string is necessary for autoapi_dirs, which expects string paths. Using .absolute() ensures a fully qualified path.

packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/exporter/dfw_elasticsearch_exporter.py (1)

46-50: LGTM! Sphinx docstring formatting fixed.

The docstring change correctly addresses the Sphinx parsing issue by removing the ** prefix from elasticsearch_kwargs and adjusting the bullet format. This prevents Sphinx from interpreting the parameter name as bold markers.

packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/processor/trace_conversion/adapter/elasticsearch/openai_converter.py (3)

27-27: LGTM! Import updated to use ESRequest.

The import correctly references the renamed ESRequest class, which resolves the naming conflict mentioned in the PR description between the data flywheel's Request and nat.data_models.api_server.Request.


80-80: LGTM! Sphinx docstring formatting fixed.

The docstring parameter name change from **kwargs to kwargs prevents Sphinx from misinterpreting the asterisks as bold markers, consistent with the PR's Sphinx parsing error fixes.


317-321: LGTM! Request construction updated to ESRequest.

The construction correctly uses ESRequest with all the same parameters as before. This completes the rename to resolve the naming conflict with nat.data_models.api_server.Request.

packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/processor/trace_conversion/__init__.py (1)

19-19: LGTM! Module rename resolves import conflict.

The import source change from .span_to_dfw_record to .span_to_dfw correctly addresses the import conflict where a function and module shared the same name. The public API remains unchanged, maintaining backward compatibility.

packages/nvidia_nat_data_flywheel/src/nat/plugins/data_flywheel/observability/schema/sink/elasticsearch/dfw_es_record.py (3)

30-31: LGTM! Import refactored to use relative path.

The change to a relative import for ContractVersion is appropriate for internal module references and improves code organization.


139-152: LGTM! Class renamed to ESRequest to resolve naming conflict.

The rename from Request to ESRequest addresses the conflict with nat.data_models.api_server.Request mentioned in the PR description. All field definitions remain unchanged, making this a straightforward rename with no behavioral changes.


203-203: LGTM! Field type updated to ESRequest.

The field type update from Request to ESRequest correctly reflects the class rename and maintains consistency throughout the codebase.

packages/nvidia_nat_data_flywheel/tests/observability/processor/trace_conversion/test_span_to_dfw_record.py (7)

26-27: LGTM! Test imports updated for module rename.

The import statements correctly reference the renamed module span_to_dfw while maintaining the same function names, ensuring test continuity after the module rename.


208-208: LGTM! Mock patch path updated.

The mock patch path correctly references the renamed module span_to_dfw, ensuring the test continues to mock the logger from the correct module.


218-226: LGTM! Mock patch paths updated for registry and container.

Both mock patch paths correctly reference the renamed module span_to_dfw, ensuring mocks are applied to the correct module locations after the rename.


239-258: LGTM! Additional mock patch paths updated.

The mock patches for TraceAdapterRegistry and TraceContainer are consistently updated to reference the renamed module span_to_dfw.


310-311: LGTM! All test method mock patches updated.

All mock patch paths in test methods are consistently updated to reference the renamed module span_to_dfw, ensuring comprehensive test coverage after the module rename.

Also applies to: 332-334, 348-350, 360-362, 373-373, 382-384, 402-404


451-451: LGTM! Integration test mock patches updated.

The integration test mock patches for TraceAdapterRegistry correctly reference the renamed module span_to_dfw.

Also applies to: 467-467


555-560: LGTM! Reimport test paths updated.

The import statements within the test function correctly reference the renamed module span_to_dfw, ensuring the signature compatibility test validates imports from the correct module location.

@dagardner-nv
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit d9eb559 into NVIDIA:release/1.3 Oct 3, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc Improvements or additions to documentation non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants