Strands integration #1063
Strands integration #1063
Conversation
WalkthroughAdds Strands framework integration: a new nvidia-nat-strands plugin (LLM handlers, tool wrapper, profiler instrumentation), a nat_strands_demo example with configs and tools, updates to packaging/workspace and optional dependencies, a new STRANDS enum, and extensive tests for LLMs, tool wrapping, and profiler hooks. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Demo as Strands Demo Workflow
participant LLM as Strands LLM Handler
participant Tools as NAT Tools\n(url_directory, current_datetime)
participant HTTP as Strands\nhttp_request Tool
participant Prof as Profiler
Client->>Demo: invoke(query)
Demo->>LLM: request(response with system_prompt)
Prof->>Prof: emit LLM_START
LLM->>Tools: request knowledge_base (url_directory)
Prof->>Prof: emit TOOL_START
Tools-->>LLM: return approved URL directory
LLM->>HTTP: http_request(selected_url)
HTTP-->>LLM: return fetched content
Prof->>Prof: emit TOOL_END
LLM->>LLM: assemble final response
Prof->>Prof: emit LLM_END (outputs & tokens)
LLM-->>Demo: return response
Demo-->>Client: deliver answer
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
mdemoret-nv
added
feature request
|
@ronjer30 Thanks for the PR adding strands support. It was on our roadmap for next release so this is a big help. Can you fix up the DCO check and we will review shortly? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (18)
examples/frameworks/strands_demo/src/nat_strands_demo/register.py (3)
28-29: Clean up unnecessarynoqadirectives.The
noqa: F401directives are unused since F401 is not enabled in the current ruff configuration. Thepylint: disable=unused-importdirectives are sufficient for these side-effect imports.Apply this diff:
-from . import ping_tool # noqa: F401, pylint: disable=unused-import -from . import url_directory # noqa: F401, pylint: disable=unused-import +from . import ping_tool # pylint: disable=unused-import +from . import url_directory # pylint: disable=unused-import
77-77: Consider using unpacking for list concatenation.The unpacking syntax is more idiomatic in modern Python.
Apply this diff:
- all_tools = nat_tools + [http_request] + all_tools = [*nat_tools, http_request]
89-91: Remove redundant exception object from log message.When using
logger.exception(), the exception information is automatically captured. Including the exception object in the message is redundant.As per coding guidelines
Apply this diff:except Exception as exc: - logger.exception("Strands demo failed: %s", exc) + logger.exception("Strands demo failed") return f"Error: {exc}"examples/frameworks/strands_demo/src/nat_strands_demo/url_directory.py (1)
103-105: Clean up exception handling.Two minor improvements: remove redundant exception object from
logger.exception()call and use explicit conversion flag in f-string.As per coding guidelines
Apply this diff:except Exception as e: - logger.exception("Error generating URL directory: %s", e) - return f"Error accessing URL directory: {str(e)}" + logger.exception("Error generating URL directory") + return f"Error accessing URL directory: {e!s}"packages/nvidia_nat_strands/src/nat/plugins/strands/llm.py (4)
37-93: Add docstrings and keep injector robust to arg forms.
- Public helpers should have concise Google-style docstrings;
_patch_llm_based_on_configand the nested injector lack them.- Injector logic is good; keep it tolerant to both positional/kw system_prompt (it already is). No behavior change needed.
Apply docstrings and import typing for later return annotations:
@@ -from typing import TypeVar +from typing import TypeVar, AsyncGenerator, Any @@ -def _patch_llm_based_on_config(client: ModelType, llm_config: LLMBaseConfig) -> ModelType: +def _patch_llm_based_on_config(client: ModelType, llm_config: LLMBaseConfig) -> ModelType: + """Patch a Strands client per NAT config (retries/thinking) and return it. + + Args: + client: Concrete Strands model client instance. + llm_config: NAT LLM config with Retry/Thinking mixins. + + Returns: + The patched client instance. + """ @@ - class StrandsThinkingInjector(BaseThinkingInjector): + class StrandsThinkingInjector(BaseThinkingInjector): + """Inject thinking system prompt for Strands stream/structured_output.""" @@ - def inject(self, messages, *args, **kwargs) -> FunctionArgumentWrapper: + def inject(self, messages, *args, **kwargs) -> FunctionArgumentWrapper: thinking_prompt = self.system_prompt if not thinking_prompt: return FunctionArgumentWrapper(messages, *args, **kwargs)As per coding guidelines.
95-117: Annotate return type and document the OpenAI wrapper.Decorated functions are public API; add return type and docstring.
-@register_llm_client(config_type=OpenAIModelConfig, wrapper_type=LLMFrameworkEnum.STRANDS) -async def openai_strands(llm_config: OpenAIModelConfig, _builder: Builder): +@register_llm_client(config_type=OpenAIModelConfig, wrapper_type=LLMFrameworkEnum.STRANDS) +async def openai_strands(llm_config: OpenAIModelConfig, _builder: Builder) -> AsyncGenerator[Any, None]: + """Build a Strands OpenAIModel from NAT OpenAIModelConfig and yield it."""As per coding guidelines.
119-171: Annotate return type; NIM compatibility override is solid.
- Add return type and docstring.
- The
format_request_messagesoverride correctly flattens text arrays for NIM.-@register_llm_client(config_type=NIMModelConfig, wrapper_type=LLMFrameworkEnum.STRANDS) -async def nim_strands(llm_config: NIMModelConfig, _builder: Builder): +@register_llm_client(config_type=NIMModelConfig, wrapper_type=LLMFrameworkEnum.STRANDS) +async def nim_strands(llm_config: NIMModelConfig, _builder: Builder) -> AsyncGenerator[Any, None]: + """Build a Strands OpenAI-compatible client for NIM and yield it."""As per coding guidelines.
173-187: Annotate return type and guard Bedrock region “None” string.
- Add return type/docstring.
- Some configs default
region_nameto the literal "None" string; coerce to None for Bedrock SDKs.-@register_llm_client(config_type=AWSBedrockModelConfig, wrapper_type=LLMFrameworkEnum.STRANDS) -async def bedrock_strands(llm_config: AWSBedrockModelConfig, _builder: Builder): +@register_llm_client(config_type=AWSBedrockModelConfig, wrapper_type=LLMFrameworkEnum.STRANDS) +async def bedrock_strands(llm_config: AWSBedrockModelConfig, _builder: Builder) -> AsyncGenerator[Any, None]: + """Build a Strands BedrockModel from NAT AWSBedrockModelConfig and yield it.""" @@ - client = BedrockModel( + region = None if llm_config.region_name in (None, "None") else llm_config.region_name + client = BedrockModel( model_id=llm_config.model_name, max_tokens=llm_config.max_tokens, temperature=llm_config.temperature, top_p=llm_config.top_p, - region_name=llm_config.region_name, + region_name=region, endpoint_url=llm_config.base_url, )Please confirm whether
AWSBedrockModelConfig.region_namecan be the literal "None" in your configs; if not, skip the coercion.packages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py (4)
32-42: Tighten exception logging in schema converter.Use
logger.exception()without formatting the exception object; optional else block appeases TRY300.def _json_schema_from_pydantic(model: type[BaseModel]) -> dict[str, Any]: try: schema = model.model_json_schema() for k in ("title", "additionalProperties"): if k in schema: del schema[k] - return {"json": schema} + return {"json": schema} except Exception as exc: # noqa: BLE001 - logger.exception("Failed to generate JSON schema: %s", exc) - return {"json": {}} + logger.exception("Failed to generate JSON schema") + return {"json": {}}If you want to silence broad-except warnings, narrow to
Exception(already used) and keep as is; tests expect fallback on failure. Based on static analysis hints.
56-64: Prefer explicit conversion flag in error payload and keep stack trace correct.
- Use
!sin f-string per ruff RUF010.logger.exceptionalready captures the stack; avoid passingexcin other calls too.def _to_error_result(tool_use_id: str, err: Exception) -> dict[str, Any]: return { "toolUseId": tool_use_id, "status": "error", "content": [{ - "text": f"{type(err).__name__}: {str(err)}" + "text": f"{type(err).__name__}: {err!s}" }], }As per coding guidelines.
92-114: Annotate return type; silence unused args; correct exception logging.
streamshould declare-> AsyncGenerator[Any, None].- Prefix unused
invocation_state/kwargsto satisfy linters without changing signature expectations.- Use
logger.exceptionwithout formattingexc.-from typing import Any +from typing import Any, AsyncGenerator @@ - async def stream(self, tool_use: ToolUse, invocation_state: dict[str, Any], **kwargs: Any): + async def stream(self, tool_use: ToolUse, _invocation_state: dict[str, Any], **_kwargs: Any) -> AsyncGenerator[Any, None]: @@ - except Exception as exc: # noqa: BLE001 - logger.exception("Strands tool '%s' failed: %s", self.tool_name, exc) + except Exception: # noqa: BLE001 + logger.exception("Strands tool '%s' failed", self.tool_name) yield ToolResultEvent(_to_error_result(tool_use_id, exc))Double-check Strands
ToolStreamEventandToolResultEventsignatures for compatibility with the yielded payloads in your version.
116-123: Docstring and unused-arg fix for wrapper; validate presence of input schema.
- Public factory should have a docstring and return type.
- Mark
builderunused to silence ruff ARG001.-@register_tool_wrapper(wrapper_type=LLMFrameworkEnum.STRANDS) -def strands_tool_wrapper(name: str, fn: Function, builder: Builder): +@register_tool_wrapper(wrapper_type=LLMFrameworkEnum.STRANDS) +def strands_tool_wrapper(name: str, fn: Function, _builder: Builder) -> NATFunctionAgentTool: + """Create a Strands `AgentTool` wrapper for a NAT `Function`.""" if fn.input_schema is None: raise ValueError(f"Tool '{name}' must define an input schema")As per coding guidelines.
packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py (6)
56-99: Use logger.exception when not re-raising; remove unused noqa.Guidelines require
logger.exception()to capture stack traces when swallowing exceptions.- except Exception as exc: # noqa: BLE001 - logger.error("Error in before_tool_invocation: %s", exc, exc_info=True) + except Exception: + logger.exception("Error in before_tool_invocation")As per coding guidelines.
100-155: Same logging rule for after_tool_invocation.Use
logger.exceptionto capture trace.- except Exception as exc: # noqa: BLE001 - logger.warning("Failed to handle after_tool_invocation: %s", exc, exc_info=True) + except Exception: + logger.exception("Failed to handle after_tool_invocation")As per coding guidelines.
198-246: Instrument(): prefer logger.exception for failures; keep info logs.Non-fatal, but stack traces help troubleshooting.
- except Exception as exc: # noqa: BLE001 - logger.warning( - "Failed to instrument Strands models: %s", - exc, - exc_info=True, - ) + except Exception: + logger.exception("Failed to instrument Strands models")As per coding guidelines.
247-299: Agent.init wrapper: exception logging per guideline.- except Exception as exc: # noqa: BLE001 - logger.warning("Failed to auto-register hooks: %s", exc, exc_info=True) + except Exception: + logger.exception("Failed to auto-register hooks") @@ - except Exception as exc: # noqa: BLE001 - logger.warning("Failed to instrument Agent.__init__: %s", exc, exc_info=True) + except Exception: + logger.exception("Failed to instrument Agent.__init__")As per coding guidelines.
333-445: Stream wrapper: avoid silent except; keep END guarantee.
- Replace
try/except/passinside the loop with debug-level logging to aid diagnosis without noise.- No behavior change to End-push logic.
- except Exception: # noqa: BLE001 - pass + except Exception: + logger.debug("Failed to extract streaming fields from event", exc_info=True)Consider using
functools.wraps(original)onwrappedto preserve introspection.
464-491: Usage extraction looks good; minor robustness.Consider
.get("usage") or ev.get("usage")if upstream versions relocate the field; optional.No change required now.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (28)
examples/frameworks/strands_demo/README.md(1 hunks)examples/frameworks/strands_demo/configs(1 hunks)examples/frameworks/strands_demo/data(1 hunks)examples/frameworks/strands_demo/pyproject.toml(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/__init__.py(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/configs/agentcore_config.yml(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/configs/config.yml(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/configs/eval_config.yml(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/configs/sizing_config.yml(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/configs/tracing_config.yml(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/data/strands.json(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/ping_tool.py(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/register.py(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/url_directory.py(1 hunks)packages/nvidia_nat_all/pyproject.toml(2 hunks)packages/nvidia_nat_strands/pyproject.toml(1 hunks)packages/nvidia_nat_strands/src/nat/plugins/strands/__init__.py(1 hunks)packages/nvidia_nat_strands/src/nat/plugins/strands/llm.py(1 hunks)packages/nvidia_nat_strands/src/nat/plugins/strands/register.py(1 hunks)packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py(1 hunks)packages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py(1 hunks)packages/nvidia_nat_strands/tests/test_callback_handler.py(1 hunks)packages/nvidia_nat_strands/tests/test_llm.py(1 hunks)packages/nvidia_nat_strands/tests/test_tool_wrapper.py(1 hunks)pyproject.toml(6 hunks)src/nat/builder/framework_enum.py(1 hunks)src/nat/profiler/decorators/framework_wrapper.py(2 hunks)tests/nat/profiler/test_callback_handler.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (18)
**/*.{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:
src/nat/builder/framework_enum.pytests/nat/profiler/test_callback_handler.pyexamples/frameworks/strands_demo/src/nat_strands_demo/configs/tracing_config.ymlpackages/nvidia_nat_strands/tests/test_tool_wrapper.pyexamples/frameworks/strands_demo/src/nat_strands_demo/configs/config.ymlexamples/frameworks/strands_demo/src/nat_strands_demo/ping_tool.pyexamples/frameworks/strands_demo/src/nat_strands_demo/configs/sizing_config.ymlexamples/frameworks/strands_demo/src/nat_strands_demo/__init__.pysrc/nat/profiler/decorators/framework_wrapper.pypackages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.pyexamples/frameworks/strands_demo/src/nat_strands_demo/configs/eval_config.ymlpackages/nvidia_nat_strands/src/nat/plugins/strands/llm.pyexamples/frameworks/strands_demo/src/nat_strands_demo/configs/agentcore_config.ymlpackages/nvidia_nat_strands/tests/test_llm.pyexamples/frameworks/strands_demo/src/nat_strands_demo/register.pyexamples/frameworks/strands_demo/src/nat_strands_demo/url_directory.pypackages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.pypackages/nvidia_nat_strands/src/nat/plugins/strands/__init__.pypackages/nvidia_nat_strands/src/nat/plugins/strands/register.pypackages/nvidia_nat_strands/tests/test_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:
src/nat/builder/framework_enum.pytests/nat/profiler/test_callback_handler.pypackages/nvidia_nat_strands/tests/test_tool_wrapper.pyexamples/frameworks/strands_demo/src/nat_strands_demo/ping_tool.pyexamples/frameworks/strands_demo/src/nat_strands_demo/__init__.pysrc/nat/profiler/decorators/framework_wrapper.pypackages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.pypackages/nvidia_nat_strands/src/nat/plugins/strands/llm.pypackages/nvidia_nat_strands/tests/test_llm.pyexamples/frameworks/strands_demo/src/nat_strands_demo/register.pyexamples/frameworks/strands_demo/src/nat_strands_demo/url_directory.pypackages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.pypackages/nvidia_nat_strands/src/nat/plugins/strands/__init__.pypackages/nvidia_nat_strands/src/nat/plugins/strands/register.pypackages/nvidia_nat_strands/tests/test_callback_handler.py
src/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
All importable Python code must live under src/ (or packages//src/)
Files:
src/nat/builder/framework_enum.pysrc/nat/profiler/decorators/framework_wrapper.py
src/nat/**/*
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Changes in src/nat should prioritize backward compatibility
Files:
src/nat/builder/framework_enum.pysrc/nat/profiler/decorators/framework_wrapper.py
⚙️ CodeRabbit configuration file
This directory contains the core functionality of the toolkit. Changes should prioritize backward compatibility.
Files:
src/nat/builder/framework_enum.pysrc/nat/profiler/decorators/framework_wrapper.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:
src/nat/builder/framework_enum.pysrc/nat/profiler/decorators/framework_wrapper.pypackages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.pypackages/nvidia_nat_strands/src/nat/plugins/strands/llm.pypackages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.pypackages/nvidia_nat_strands/src/nat/plugins/strands/__init__.pypackages/nvidia_nat_strands/src/nat/plugins/strands/register.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
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.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.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile are OK.Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,
and should contain an Apache License 2.0 header comment at the top of each file.
- Confirm that copyright years are up-to date whenever a file is changed.
Files:
src/nat/builder/framework_enum.pytests/nat/profiler/test_callback_handler.pypackages/nvidia_nat_all/pyproject.tomlexamples/frameworks/strands_demo/src/nat_strands_demo/configs/tracing_config.ymlexamples/frameworks/strands_demo/README.mdpackages/nvidia_nat_strands/tests/test_tool_wrapper.pyexamples/frameworks/strands_demo/src/nat_strands_demo/configs/config.ymlexamples/frameworks/strands_demo/configsexamples/frameworks/strands_demo/datapackages/nvidia_nat_strands/pyproject.tomlexamples/frameworks/strands_demo/src/nat_strands_demo/ping_tool.pyexamples/frameworks/strands_demo/src/nat_strands_demo/configs/sizing_config.ymlexamples/frameworks/strands_demo/src/nat_strands_demo/data/strands.jsonexamples/frameworks/strands_demo/src/nat_strands_demo/__init__.pysrc/nat/profiler/decorators/framework_wrapper.pypackages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.pyexamples/frameworks/strands_demo/src/nat_strands_demo/configs/eval_config.ymlpackages/nvidia_nat_strands/src/nat/plugins/strands/llm.pypyproject.tomlexamples/frameworks/strands_demo/src/nat_strands_demo/configs/agentcore_config.ymlpackages/nvidia_nat_strands/tests/test_llm.pyexamples/frameworks/strands_demo/src/nat_strands_demo/register.pyexamples/frameworks/strands_demo/src/nat_strands_demo/url_directory.pypackages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.pypackages/nvidia_nat_strands/src/nat/plugins/strands/__init__.pypackages/nvidia_nat_strands/src/nat/plugins/strands/register.pyexamples/frameworks/strands_demo/pyproject.tomlpackages/nvidia_nat_strands/tests/test_callback_handler.py
tests/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Unit tests reside under tests/ and should use markers defined in pyproject.toml (e.g., integration)
Files:
tests/nat/profiler/test_callback_handler.py
⚙️ CodeRabbit configuration file
tests/**/*.py: - Ensure that tests are comprehensive, cover edge cases, and validate the functionality of the code. - Test functions should be named using thetest_prefix, using snake_case. - Any frequently repeated code should be extracted into pytest fixtures. - Pytest fixtures should define the name argument when applying the pytest.fixture decorator. The fixture
function being decorated should be named using thefixture_prefix, using snake_case. Example:
@pytest.fixture(name="my_fixture")
def fixture_my_fixture():
pass
Files:
tests/nat/profiler/test_callback_handler.py
{tests/**/*.py,examples/*/tests/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{tests/**/*.py,examples/*/tests/**/*.py}: Use pytest (with pytest-asyncio for async); name test files test_*.py; test functions start with test_; extract repeated code into fixtures; fixtures must set name in decorator and be named with fixture_ prefix
Mock external services with pytest_httpserver or unittest.mock; do not hit live endpoints
Mark expensive tests with @pytest.mark.slow or @pytest.mark.integration
Files:
tests/nat/profiler/test_callback_handler.py
packages/*/pyproject.toml
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
packages/*/pyproject.toml: Each package must contain a pyproject.toml
In packages, declare a dependency on nvidia-nat or packages starting with nvidia-nat-
Use ~= version constraints (e.g., ~=1.0) for dependencies
Files:
packages/nvidia_nat_all/pyproject.tomlpackages/nvidia_nat_strands/pyproject.toml
{packages/*/pyproject.toml,uv.lock}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Add new dependencies to both pyproject.toml (alphabetically) and uv.lock via uv pip install --sync
Files:
packages/nvidia_nat_all/pyproject.tomlpackages/nvidia_nat_strands/pyproject.toml
packages/**/*
⚙️ CodeRabbit configuration file
packages/**/*: - This directory contains optional plugin packages for the toolkit, each should contain apyproject.tomlfile. - Thepyproject.tomlfile should declare a dependency onnvidia-nator another package with a name starting
withnvidia-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 thepyproject.tomlfile.
Files:
packages/nvidia_nat_all/pyproject.tomlpackages/nvidia_nat_strands/tests/test_tool_wrapper.pypackages/nvidia_nat_strands/pyproject.tomlpackages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.pypackages/nvidia_nat_strands/src/nat/plugins/strands/llm.pypackages/nvidia_nat_strands/tests/test_llm.pypackages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.pypackages/nvidia_nat_strands/src/nat/plugins/strands/__init__.pypackages/nvidia_nat_strands/src/nat/plugins/strands/register.pypackages/nvidia_nat_strands/tests/test_callback_handler.py
**/*.{yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/nat-test-llm.mdc)
In workflow/config YAML, set llms.._type: nat_test_llm to stub responses.
Files:
examples/frameworks/strands_demo/src/nat_strands_demo/configs/tracing_config.ymlexamples/frameworks/strands_demo/src/nat_strands_demo/configs/config.ymlexamples/frameworks/strands_demo/src/nat_strands_demo/configs/sizing_config.ymlexamples/frameworks/strands_demo/src/nat_strands_demo/configs/eval_config.ymlexamples/frameworks/strands_demo/src/nat_strands_demo/configs/agentcore_config.yml
**/configs/**
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Configuration files consumed by code must be stored next to that code in a configs/ folder
Files:
examples/frameworks/strands_demo/src/nat_strands_demo/configs/tracing_config.ymlexamples/frameworks/strands_demo/src/nat_strands_demo/configs/config.ymlexamples/frameworks/strands_demo/src/nat_strands_demo/configs/sizing_config.ymlexamples/frameworks/strands_demo/src/nat_strands_demo/configs/eval_config.ymlexamples/frameworks/strands_demo/src/nat_strands_demo/configs/agentcore_config.yml
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 apyproject.tomlfile. Optionally, it might also contain scripts in ascripts/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 nameddata/, and should
be checked into git-lfs.
Files:
examples/frameworks/strands_demo/src/nat_strands_demo/configs/tracing_config.ymlexamples/frameworks/strands_demo/README.mdexamples/frameworks/strands_demo/src/nat_strands_demo/configs/config.ymlexamples/frameworks/strands_demo/configsexamples/frameworks/strands_demo/dataexamples/frameworks/strands_demo/src/nat_strands_demo/ping_tool.pyexamples/frameworks/strands_demo/src/nat_strands_demo/configs/sizing_config.ymlexamples/frameworks/strands_demo/src/nat_strands_demo/data/strands.jsonexamples/frameworks/strands_demo/src/nat_strands_demo/__init__.pyexamples/frameworks/strands_demo/src/nat_strands_demo/configs/eval_config.ymlexamples/frameworks/strands_demo/src/nat_strands_demo/configs/agentcore_config.ymlexamples/frameworks/strands_demo/src/nat_strands_demo/register.pyexamples/frameworks/strands_demo/src/nat_strands_demo/url_directory.pyexamples/frameworks/strands_demo/pyproject.toml
**/README.@(md|ipynb)
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Ensure READMEs follow the naming convention; avoid deprecated names; use “NeMo Agent Toolkit” (capital T) in headings
Files:
examples/frameworks/strands_demo/README.md
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_strands/tests/test_tool_wrapper.pypackages/nvidia_nat_strands/tests/test_llm.pypackages/nvidia_nat_strands/tests/test_callback_handler.py
**/data/**
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Large/binary assets must be committed with Git-LFS and placed in a neighbouring data/ folder
Files:
examples/frameworks/strands_demo/src/nat_strands_demo/data/strands.json
packages/*/src/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Importable Python code inside packages must live under packages//src/
Files:
packages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.pypackages/nvidia_nat_strands/src/nat/plugins/strands/llm.pypackages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.pypackages/nvidia_nat_strands/src/nat/plugins/strands/__init__.pypackages/nvidia_nat_strands/src/nat/plugins/strands/register.py
🧠 Learnings (2)
📚 Learning: 2025-09-23T18:39:15.023Z
Learnt from: CR
PR: NVIDIA/NeMo-Agent-Toolkit#0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-09-23T18:39:15.023Z
Learning: Applies to packages/*/pyproject.toml : In packages, declare a dependency on nvidia-nat or packages starting with nvidia-nat-
Applied to files:
packages/nvidia_nat_all/pyproject.tomlpackages/nvidia_nat_strands/pyproject.tomlpyproject.tomlexamples/frameworks/strands_demo/pyproject.toml
📚 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_strands/src/nat/plugins/strands/tool_wrapper.py
🧬 Code graph analysis (12)
tests/nat/profiler/test_callback_handler.py (4)
packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py (3)
StrandsProfilerHandler(172-491)on_before_tool_invocation(56-98)on_after_tool_invocation(100-154)src/nat/builder/intermediate_step_manager.py (1)
subscribe(180-188)src/nat/data_models/intermediate_step.py (4)
event_type(265-266)IntermediateStepType(42-58)framework(277-278)UUID(301-302)src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-26)
packages/nvidia_nat_strands/tests/test_tool_wrapper.py (1)
packages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py (9)
_json_schema_from_pydantic(32-41)_to_error_result(56-63)_to_tool_result(44-53)strands_tool_wrapper(117-123)tool_spec(85-86)NATFunctionAgentTool(66-113)tool_name(81-82)stream(92-113)tool_type(89-90)
examples/frameworks/strands_demo/src/nat_strands_demo/ping_tool.py (3)
src/nat/builder/builder.py (1)
Builder(68-290)src/nat/builder/function_info.py (2)
FunctionInfo(290-625)from_fn(552-625)src/nat/data_models/function.py (1)
FunctionBaseConfig(26-27)
src/nat/profiler/decorators/framework_wrapper.py (2)
src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-26)packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py (2)
StrandsProfilerHandler(172-491)instrument(184-245)
packages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py (3)
src/nat/builder/builder.py (1)
Builder(68-290)src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-26)packages/nvidia_nat_strands/tests/test_tool_wrapper.py (3)
model_json_schema(61-62)tool_spec(136-138)acall_invoke(42-43)
packages/nvidia_nat_strands/src/nat/plugins/strands/llm.py (10)
src/nat/builder/builder.py (1)
Builder(68-290)src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-26)src/nat/data_models/llm.py (1)
LLMBaseConfig(30-38)src/nat/data_models/thinking_mixin.py (2)
ThinkingMixin(29-86)thinking_system_prompt(49-86)src/nat/llm/aws_bedrock_llm.py (1)
AWSBedrockModelConfig(33-63)src/nat/llm/nim_llm.py (1)
NIMModelConfig(34-52)src/nat/llm/openai_llm.py (2)
openai_llm(53-55)OpenAIModelConfig(32-49)src/nat/llm/utils/thinking.py (3)
BaseThinkingInjector(57-81)FunctionArgumentWrapper(34-53)patch_with_thinking(121-215)src/nat/utils/exception_handlers/automatic_retries.py (1)
patch_with_retry(412-493)src/nat/utils/type_utils.py (1)
override(56-57)
packages/nvidia_nat_strands/tests/test_llm.py (5)
src/nat/builder/builder.py (1)
Builder(68-290)src/nat/llm/aws_bedrock_llm.py (1)
AWSBedrockModelConfig(33-63)src/nat/llm/nim_llm.py (1)
NIMModelConfig(34-52)src/nat/llm/openai_llm.py (2)
openai_llm(53-55)OpenAIModelConfig(32-49)packages/nvidia_nat_strands/src/nat/plugins/strands/llm.py (4)
_patch_llm_based_on_config(37-92)bedrock_strands(174-187)nim_strands(120-170)openai_strands(96-116)
examples/frameworks/strands_demo/src/nat_strands_demo/register.py (6)
src/nat/builder/builder.py (1)
Builder(68-290)src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-26)src/nat/builder/function_info.py (2)
FunctionInfo(290-625)from_fn(552-625)src/nat/data_models/component_ref.py (2)
FunctionRef(94-102)LLMRef(116-124)src/nat/data_models/function.py (1)
FunctionBaseConfig(26-27)examples/frameworks/strands_demo/src/nat_strands_demo/url_directory.py (1)
url_directory(55-107)
examples/frameworks/strands_demo/src/nat_strands_demo/url_directory.py (3)
src/nat/builder/builder.py (1)
Builder(68-290)src/nat/builder/function_info.py (2)
FunctionInfo(290-625)from_fn(552-625)src/nat/data_models/function.py (1)
FunctionBaseConfig(26-27)
packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py (4)
src/nat/builder/context.py (3)
Context(118-316)intermediate_step_manager(172-183)output(59-60)src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-26)src/nat/data_models/intermediate_step.py (11)
IntermediateStepPayload(130-232)IntermediateStepType(42-58)StreamEventData(67-77)TraceMetadata(114-127)UsageInfo(80-83)event_type(265-266)framework(277-278)UUID(301-302)data(293-294)usage_info(297-298)span_event_timestamp(273-274)src/nat/profiler/callbacks/token_usage_base_model.py (1)
TokenUsageBaseModel(20-29)
packages/nvidia_nat_strands/src/nat/plugins/strands/register.py (1)
tests/nat/builder/test_builder.py (1)
tool_wrapper(554-564)
packages/nvidia_nat_strands/tests/test_callback_handler.py (4)
src/nat/builder/context.py (3)
Context(118-316)intermediate_step_manager(172-183)output(59-60)src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-26)src/nat/data_models/intermediate_step.py (5)
IntermediateStepType(42-58)event_type(265-266)UUID(301-302)framework(277-278)data(293-294)packages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py (3)
tool_name(81-82)tool_spec(85-86)NATFunctionAgentTool(66-113)
🪛 Biome (2.1.2)
examples/frameworks/strands_demo/src/nat_strands_demo/data/strands.json
[error] 1-1: String values must be double quoted.
(parse)
[error] 1-1: String values must be double quoted.
(parse)
[error] 1-2: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 2-2: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 2-2: String values must be double quoted.
(parse)
[error] 2-3: String values must be double quoted.
(parse)
[error] 3-3: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
🪛 Ruff (0.14.1)
packages/nvidia_nat_strands/tests/test_tool_wrapper.py
42-42: Unused method argument: kwargs
(ARG002)
62-62: Create your own exception
(TRY002)
62-62: Avoid specifying long messages outside the exception class
(TRY003)
212-212: Unused function argument: kwargs
(ARG001)
249-249: Unused function argument: kwargs
(ARG001)
251-251: Avoid specifying long messages outside the exception class
(TRY003)
packages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py
38-38: Consider moving this statement to an else block
(TRY300)
40-40: Redundant exception object included in logging.exception call
(TRY401)
61-61: Use explicit conversion flag
Replace with conversion flag
(RUF010)
92-92: Unused method argument: invocation_state
(ARG002)
92-92: Unused method argument: kwargs
(ARG002)
111-111: Unused noqa directive (unused: BLE001)
Remove unused noqa directive
(RUF100)
112-112: Redundant exception object included in logging.exception call
(TRY401)
117-117: Unused function argument: builder
(ARG001)
119-119: Avoid specifying long messages outside the exception class
(TRY003)
examples/frameworks/strands_demo/src/nat_strands_demo/register.py
28-28: Unused noqa directive (non-enabled: F401)
Remove unused noqa directive
(RUF100)
29-29: Unused noqa directive (non-enabled: F401)
Remove unused noqa directive
(RUF100)
77-77: Consider [*nat_tools, http_request] instead of concatenation
Replace with [*nat_tools, http_request]
(RUF005)
88-88: Consider moving this statement to an else block
(TRY300)
90-90: Redundant exception object included in logging.exception call
(TRY401)
examples/frameworks/strands_demo/src/nat_strands_demo/url_directory.py
50-50: Avoid specifying long messages outside the exception class
(TRY003)
104-104: Redundant exception object included in logging.exception call
(TRY401)
105-105: Use explicit conversion flag
Replace with conversion flag
(RUF010)
packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py
97-97: Unused noqa directive (unused: BLE001)
Remove unused noqa directive
(RUF100)
328-329: try-except-pass detected, consider logging the exception
(S110)
432-433: try-except-pass detected, consider logging the exception
(S110)
packages/nvidia_nat_strands/tests/test_callback_handler.py
173-173: Avoid specifying long messages outside the exception class
(TRY003)
246-246: Unused method argument: args
(ARG002)
246-246: Unused method argument: kwargs
(ARG002)
256-256: Avoid specifying long messages outside the exception class
(TRY003)
259-259: Avoid specifying long messages outside the exception class
(TRY003)
419-419: Unused function argument: model_self
(ARG001)
419-419: Unused function argument: messages
(ARG001)
419-419: Unused function argument: tool_specs
(ARG001)
419-419: Unused function argument: system_prompt
(ARG001)
419-419: Unused function argument: kwargs
(ARG001)
468-468: Unused function argument: model_self
(ARG001)
468-468: Unused function argument: args
(ARG001)
468-468: Unused function argument: kwargs
(ARG001)
470-470: Avoid specifying long messages outside the exception class
(TRY003)
499-499: Unused function argument: model_self
(ARG001)
499-499: Unused function argument: args
(ARG001)
499-499: Unused function argument: kwargs
(ARG001)
527-527: Unused function argument: model_self
(ARG001)
527-527: Unused function argument: messages
(ARG001)
527-527: Unused function argument: tool_specs
(ARG001)
527-527: Unused function argument: system_prompt
(ARG001)
527-527: Unused function argument: kwargs
(ARG001)
576-576: Unused method argument: args
(ARG002)
576-576: Unused method argument: kwargs
(ARG002)
593-593: Consider moving this statement to an else block
(TRY300)
600-600: Avoid specifying long messages outside the exception class
(TRY003)
655-655: Unused method argument: args
(ARG002)
655-655: Unused method argument: kwargs
(ARG002)
671-671: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (36)
packages/nvidia_nat_strands/src/nat/plugins/strands/__init__.py (1)
1-14: Verify trailing newline per PEP 8.The Apache-2.0 license header is correct, copyright year (2025) is up-to-date, and the file location follows the required package structure. However, per PEP 8 and coding guidelines, all Python files should end with a single trailing newline. Ensure line 14 is followed by exactly one newline character.
As per coding guidelines: "end files with a single trailing newline"
examples/frameworks/strands_demo/data (1)
1-1: Clarify the purpose of this path reference file.This file contains a single line with a path string. It's unclear whether this is intended as a symlink, a configuration reference, or a packaging artifact. Consider using a proper symlink if the goal is to reference the actual data directory, or document the purpose if this is an intentional pattern.
Can you clarify the purpose of this file structure? If it's meant to reference the data directory, a symlink might be more appropriate.
examples/frameworks/strands_demo/src/nat_strands_demo/__init__.py (1)
1-14: LGTM!The license header is correct and the copyright year is up to date.
examples/frameworks/strands_demo/configs (1)
1-1: Clarify the purpose of this path reference file.Similar to the
datafile, this contains a single path string. The purpose and mechanism of these path reference files should be clarified or documented. If this is meant to reference the configs directory, consider using a proper symlink or documenting this pattern.Can you clarify the purpose of this file structure? Is this a standard pattern in the repository?
src/nat/builder/framework_enum.py (1)
26-26: LGTM!The STRANDS enum member follows the established pattern and is correctly implemented.
examples/frameworks/strands_demo/src/nat_strands_demo/data/strands.json (1)
1-3: LGTM! Static analysis errors are false positives.This is a valid Git LFS pointer file. The Biome parse errors can be ignored—they occur because the tool is attempting to parse this as JSON when it's actually a Git LFS pointer with a specific format. The file is correctly placed in a
data/directory and properly tracked by Git LFS as per coding guidelines.packages/nvidia_nat_strands/src/nat/plugins/strands/register.py (1)
1-20: LGTM!The plugin registration module is correctly structured with appropriate license headers and imports. The linter directives indicate intentional import ordering.
packages/nvidia_nat_all/pyproject.toml (1)
39-39: LGTM!The
nvidia-nat-strandsdependency is correctly added to both the dependencies list and workspace sources, following the established pattern for other NAT packages. The alphabetical ordering is maintained.Also applies to: 84-84
tests/nat/profiler/test_callback_handler.py (1)
616-696: LGTM! Comprehensive Strands tool execution test.The test thoroughly validates the Strands profiler handler's tool instrumentation by:
- Simulating Strands before/after tool invocation events with realistic mock objects
- Verifying TOOL_START and TOOL_END events are emitted with correct framework, UUID, and metadata
- Asserting tool_inputs and tool_outputs are captured accurately
The test follows established patterns in the file and integrates well with the reactive stream testing approach.
packages/nvidia_nat_strands/pyproject.toml (1)
1-40: LGTM! Package configuration follows guidelines.The pyproject.toml correctly:
- Uses
~=1.3constraint for the nvidia-nat dependency (as per coding guidelines)- Declares exact versions for external Strands dependencies
- Sets appropriate Python version requirements (>=3.11,<3.14)
- Defines the entry point for component registration
src/nat/profiler/decorators/framework_wrapper.py (2)
37-37: LGTM! Registry initialization for Strands.The addition of
"strands": Falseto the instrumentation registry follows the established pattern for tracking framework instrumentation state.
135-148: LGTM! Strands profiler registration follows framework patterns.The Strands instrumentation setup correctly:
- Checks framework presence and instrumentation state to avoid duplicate setup
- Handles import failures gracefully with informative installation guidance
- Instantiates and instruments the handler
- Updates the instrumentation flag and logs registration
The implementation is consistent with other framework handlers (ADK, Agno, etc.) in this file.
examples/frameworks/strands_demo/pyproject.toml (1)
1-25: LGTM! Demo project configuration is well-structured.The pyproject.toml correctly:
- Uses
~=1.4constraint for the nvidia-nat dependency with appropriate extras [strands,profiling]- Sets Python version requirements consistent with the main package
- Defines the demo entry point for component registration
- Configures local editable source for development
The configuration is appropriate for an example/demo project.
examples/frameworks/strands_demo/src/nat_strands_demo/configs/tracing_config.yml (1)
1-53: LGTM! Tracing configuration is well-defined.The configuration appropriately:
- Enables console and file logging with appropriate levels
- Configures Phoenix tracing for span visualization
- Defines knowledge_base and current_datetime functions
- Sets up multiple LLM options (NIM, Bedrock, OpenAI)
- Configures the strands_demo workflow with appropriate system prompt
The telemetry setup will enable effective observability for the Strands integration.
examples/frameworks/strands_demo/src/nat_strands_demo/configs/config.yml (1)
1-46: LGTM! Base configuration is well-structured.The configuration provides:
- Properly defined knowledge_base function with curated documentation URLs
- Multiple LLM options with reasonable parameters
- Clear workflow definition with appropriate system prompt
This serves as a solid baseline configuration for the Strands demo.
examples/frameworks/strands_demo/src/nat_strands_demo/configs/agentcore_config.yml (1)
1-66: LGTM! AgentCore configuration is properly structured.The configuration correctly:
- Defines FastAPI endpoints compatible with AWS AgentCore requirements
- Includes health check endpoint at /ping using simple_agentcore_ping function
- Configures main workflow endpoint at /invocations
- Provides WebSocket and OpenAI API paths
- Reuses the same functions, LLMs, and workflow definition from base config
The setup aligns with AWS AgentCore integration patterns (as noted in the comment referencing AWS docs).
examples/frameworks/strands_demo/src/nat_strands_demo/configs/sizing_config.yml (1)
1-44: LGTM! Sizing configuration is appropriate for evaluation.The configuration correctly:
- Sets temperature to 0.0 for all LLMs to ensure deterministic behavior during evaluation
- Defines eval section with dataset path and output directory
- Enables cleanup flag to manage evaluation artifacts
- Reuses the same functions and workflow structure for consistency
This configuration is well-suited for evaluation and sizing experiments.
examples/frameworks/strands_demo/src/nat_strands_demo/ping_tool.py (2)
26-27: LGTM!The minimal configuration class follows NAT patterns correctly.
30-50: LGTM!The health check implementation is clean and appropriate for AgentCore compatibility. The docstring is comprehensive and follows Google style.
packages/nvidia_nat_strands/tests/test_callback_handler.py (3)
28-139: LGTM!The test coverage for
StrandsToolInstrumentationHookis comprehensive, covering initialization, tool invocation lifecycle, and NAT-wrapped tool instrumentation.
271-394: LGTM!Excellent edge-case coverage for event extraction methods, including valid data, missing fields, invalid values, and partial data scenarios.
327-338: Remove duplicate assertions.Lines 331-338 contain duplicated test code that repeats the same assertion three times without adding value.
Apply this diff to remove the duplication:
assert result_prompt == "Think step by step" - result_prompt = (thinking_prompt - if not existing_system_prompt else f"{thinking_prompt}\n\n{existing_system_prompt}") - - assert result_prompt == "Think step by step" - result_prompt = (thinking_prompt - if not existing_system_prompt else f"{thinking_prompt}\n\n{existing_system_prompt}") - - assert result_prompt == "Think step by step"Likely an incorrect or invalid review comment.
examples/frameworks/strands_demo/src/nat_strands_demo/configs/eval_config.yml (1)
1-86: LGTM!The evaluation configuration is comprehensive and well-structured, featuring multiple LLM providers, appropriate profiler settings, and a good mix of RAG and trajectory evaluators.
packages/nvidia_nat_strands/tests/test_llm.py (2)
31-93: LGTM!The test coverage for OpenAI and Bedrock Strands integrations is appropriate, using proper mocking patterns and covering both basic and parameterized cases.
95-222: LGTM!Comprehensive test coverage for NIM Strands integration, including environment variable handling, default base URL, message formatting for NIM compatibility, and proper exclusion of NAT-specific parameters.
packages/nvidia_nat_strands/tests/test_tool_wrapper.py (2)
46-307: LGTM!Comprehensive test coverage for the Strands tool wrapper, including JSON schema generation, result formatting, error handling, streaming support, and various edge cases. The test structure is clean and well-organized.
309-361: LGTM!Excellent edge-case coverage, including schema validation, field sanitization, various result types, and custom exception handling.
examples/frameworks/strands_demo/src/nat_strands_demo/register.py (2)
34-40: LGTM!The configuration class is well-defined with appropriate field types and helpful descriptions.
93-96: LGTM!Proper usage of
FunctionInfo.from_fnto wrap the agent execution function.pyproject.toml (5)
49-49: LGTM!The loosened version constraint for
richprovides better flexibility while maintaining compatibility.
93-93: LGTM!The Strands optional dependency follows the established pattern and naming convention.
165-169: LGTM!The tenacity version override appropriately resolves the dependency conflict between google-adk and strands-agents-tools. The comments clearly explain the rationale and API compatibility.
192-192: LGTM!The workspace member configurations for
nvidia-nat-strandsandnat_strands_demofollow established patterns and are properly ordered.Also applies to: 222-222
129-129: LGTM!The
nat_strands_demoexample is correctly added to the examples list in alphabetical order.examples/frameworks/strands_demo/src/nat_strands_demo/url_directory.py (2)
30-51: LGTM!The configuration class is well-designed with appropriate field validation. The URL validator ensures all URLs have proper scheme and netloc before being accepted.
107-107: LGTM!Proper usage of
FunctionInfo.from_fnto wrap the URL directory function.
packages/nvidia_nat_strands/tests/test_strands_callback_handler.py
Outdated
Show resolved
Hide resolved
ronjer30
force-pushed
the
strands_integration
branch
from
6349102 to
8459762
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/nvidia_nat_strands/tests/test_callback_handler.py (1)
678-685: Remove duplicate assertion.Lines 684-685 contain a duplicate assertion that repeats the check on line 682.
Apply this diff:
# Should have attempted to register hooks despite the error assert agent.hooks.add_callback.called - - # Should have attempted to register hooks despite the error - assert agent.hooks.add_callback.calledpackages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py (1)
44-53: CRITICAL: Fixisinstance()with PEP 604 union syntax.This was flagged in a previous review and must be fixed. The
isinstance()call withdict | list | tupleraises aTypeErrorat runtime. Use a tuple of types instead.Apply this diff:
def _to_tool_result(tool_use_id: str, value: Any) -> dict[str, Any]: - if isinstance(value, dict | list | tuple): + if isinstance(value, (dict, list, tuple)): content_item = {"json": value}
🧹 Nitpick comments (10)
examples/frameworks/strands_demo/src/nat_strands_demo/url_directory.py (1)
103-105: Fix logging and f-string usage.Two minor issues:
logger.exception()already includes the exception object, so passing it again is redundant.- Using
str(e)in an f-string is unnecessary; the f-string will automatically convert it.Apply this diff:
except Exception as e: - logger.exception("Error generating URL directory: %s", e) - return f"Error accessing URL directory: {str(e)}" + logger.exception("Error generating URL directory") + return f"Error accessing URL directory: {e}"examples/frameworks/strands_demo/src/nat_strands_demo/register.py (3)
28-29: Remove unusednoqadirectives.Ruff reports that the
F401rule is not enabled, so thenoqa: F401directives are unnecessary. Keep thepylint: disable=unused-importcomments as they're still needed for pylint users.Apply this diff:
-from . import ping_tool # noqa: F401, pylint: disable=unused-import -from . import url_directory # noqa: F401, pylint: disable=unused-import +from . import ping_tool # pylint: disable=unused-import +from . import url_directory # pylint: disable=unused-import
77-77: Consider using unpacking syntax for list construction.The unpacking syntax
[*nat_tools, http_request]is more idiomatic in modern Python and recommended by static analysis.Apply this diff:
- all_tools = nat_tools + [http_request] + all_tools = [*nat_tools, http_request]
82-91: Remove redundant exception parameter fromlogger.exception().
logger.exception()automatically captures the exception and stack trace, so passingexcas a parameter is redundant per coding guidelines.Apply this diff:
except Exception as exc: - logger.exception("Strands demo failed: %s", exc) + logger.exception("Strands demo failed") return f"Error: {exc}"packages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py (3)
32-41: Remove redundant exception parameter fromlogger.exception().
logger.exception()automatically captures the exception and stack trace, so passingexcas a parameter is redundant per coding guidelines.Apply this diff:
except Exception as exc: - logger.exception("Failed to generate JSON schema: %s", exc) + logger.exception("Failed to generate JSON schema") return {"json": {}}
56-63: Use f-string conversion flag for cleaner string formatting.Use the explicit
!sconversion flag instead of callingstr()in the f-string.Apply this diff:
return { "toolUseId": tool_use_id, "status": "error", "content": [{ - "text": f"{type(err).__name__}: {str(err)}" + "text": f"{type(err).__name__}: {err!s}" }], }
111-113: Remove redundant exception parameter fromlogger.exception().
logger.exception()automatically captures the exception and stack trace, so passingexcas a parameter is redundant per coding guidelines.Apply this diff:
except Exception as exc: # noqa: BLE001 - logger.exception("Strands tool '%s' failed: %s", self.tool_name, exc) + logger.exception("Strands tool '%s' failed", self.tool_name) yield ToolResultEvent(_to_error_result(tool_use_id, exc))packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py (3)
97-98: Remove unusednoqadirective if BLE001 is not enabled.Ruff reports that the
BLE001rule is not enabled in your configuration, making thenoqadirective unnecessary. You can safely remove it unless you plan to enable this rule.Apply this diff:
- except Exception as exc: # noqa: BLE001 + except Exception as exc: logger.error("Error in before_tool_invocation: %s", exc, exc_info=True)
316-329: Consider logging exceptions during parameter extraction.The
try-except-passblock silently suppresses exceptions when extracting model parameters. While this prevents instrumentation from breaking the main flow, logging these exceptions at debug level would help diagnose instrumentation issues.Apply this diff:
try: params = getattr(model_instance, "params", {}) if isinstance(params, dict): model_params = { "temperature": params.get("temperature"), "max_tokens": params.get("max_tokens"), "top_p": params.get("top_p"), "stream": params.get("stream", True), } # Remove None values model_params = {k: v for k, v in model_params.items() if v is not None} - except Exception: # noqa: BLE001 - pass + except Exception as exc: + logger.debug("Failed to extract model params: %s", exc)
416-434: Consider logging exceptions during event extraction.The
try-except-passblock silently suppresses exceptions when extracting text and usage from stream events. While this prevents instrumentation from breaking the stream, logging these exceptions at debug level would help diagnose why events aren't being captured properly.Apply this diff:
async for ev in agen: # type: ignore try: # Extract text content text_content = self._extract_text_from_event(ev) if text_content: output_text += text_content # Extract usage information usage_info = self._extract_usage_from_event(ev) if usage_info and not ended: token_usage = TokenUsageBaseModel(**usage_info) _push_end_if_needed() - except Exception: # noqa: BLE001 - pass + except Exception as exc: + logger.debug("Failed to extract data from stream event: %s", exc) yield ev
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (28)
examples/frameworks/strands_demo/README.md(1 hunks)examples/frameworks/strands_demo/configs(1 hunks)examples/frameworks/strands_demo/data(1 hunks)examples/frameworks/strands_demo/pyproject.toml(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/__init__.py(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/configs/agentcore_config.yml(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/configs/config.yml(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/configs/eval_config.yml(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/configs/sizing_config.yml(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/configs/tracing_config.yml(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/data/strands.json(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/ping_tool.py(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/register.py(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/url_directory.py(1 hunks)packages/nvidia_nat_all/pyproject.toml(2 hunks)packages/nvidia_nat_strands/pyproject.toml(1 hunks)packages/nvidia_nat_strands/src/nat/plugins/strands/__init__.py(1 hunks)packages/nvidia_nat_strands/src/nat/plugins/strands/llm.py(1 hunks)packages/nvidia_nat_strands/src/nat/plugins/strands/register.py(1 hunks)packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py(1 hunks)packages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py(1 hunks)packages/nvidia_nat_strands/tests/test_callback_handler.py(1 hunks)packages/nvidia_nat_strands/tests/test_llm.py(1 hunks)packages/nvidia_nat_strands/tests/test_tool_wrapper.py(1 hunks)pyproject.toml(6 hunks)src/nat/builder/framework_enum.py(1 hunks)src/nat/profiler/decorators/framework_wrapper.py(2 hunks)tests/nat/profiler/test_callback_handler.py(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- examples/frameworks/strands_demo/configs
- packages/nvidia_nat_strands/src/nat/plugins/strands/init.py
- examples/frameworks/strands_demo/data
🚧 Files skipped from review as they are similar to previous changes (11)
- examples/frameworks/strands_demo/src/nat_strands_demo/configs/sizing_config.yml
- examples/frameworks/strands_demo/src/nat_strands_demo/ping_tool.py
- examples/frameworks/strands_demo/README.md
- packages/nvidia_nat_strands/src/nat/plugins/strands/register.py
- src/nat/builder/framework_enum.py
- packages/nvidia_nat_strands/src/nat/plugins/strands/llm.py
- examples/frameworks/strands_demo/src/nat_strands_demo/configs/tracing_config.yml
- src/nat/profiler/decorators/framework_wrapper.py
- examples/frameworks/strands_demo/pyproject.toml
- packages/nvidia_nat_strands/tests/test_llm.py
- examples/frameworks/strands_demo/src/nat_strands_demo/init.py
🧰 Additional context used
📓 Path-based instructions (15)
packages/*/pyproject.toml
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
packages/*/pyproject.toml: Each package must contain a pyproject.toml
In packages, declare a dependency on nvidia-nat or packages starting with nvidia-nat-
Use ~= version constraints (e.g., ~=1.0) for dependencies
Files:
packages/nvidia_nat_all/pyproject.tomlpackages/nvidia_nat_strands/pyproject.toml
{packages/*/pyproject.toml,uv.lock}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Add new dependencies to both pyproject.toml (alphabetically) and uv.lock via uv pip install --sync
Files:
packages/nvidia_nat_all/pyproject.tomlpackages/nvidia_nat_strands/pyproject.toml
**/*
⚙️ 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
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.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.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile 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_all/pyproject.tomlexamples/frameworks/strands_demo/src/nat_strands_demo/configs/config.ymlexamples/frameworks/strands_demo/src/nat_strands_demo/configs/agentcore_config.ymlpackages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.pypackages/nvidia_nat_strands/tests/test_tool_wrapper.pyexamples/frameworks/strands_demo/src/nat_strands_demo/data/strands.jsonexamples/frameworks/strands_demo/src/nat_strands_demo/url_directory.pyexamples/frameworks/strands_demo/src/nat_strands_demo/register.pytests/nat/profiler/test_callback_handler.pypackages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.pyexamples/frameworks/strands_demo/src/nat_strands_demo/configs/eval_config.ymlpackages/nvidia_nat_strands/tests/test_callback_handler.pypackages/nvidia_nat_strands/pyproject.tomlpyproject.toml
packages/**/*
⚙️ CodeRabbit configuration file
packages/**/*: - This directory contains optional plugin packages for the toolkit, each should contain apyproject.tomlfile. - Thepyproject.tomlfile should declare a dependency onnvidia-nator another package with a name starting
withnvidia-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 thepyproject.tomlfile.
Files:
packages/nvidia_nat_all/pyproject.tomlpackages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.pypackages/nvidia_nat_strands/tests/test_tool_wrapper.pypackages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.pypackages/nvidia_nat_strands/tests/test_callback_handler.pypackages/nvidia_nat_strands/pyproject.toml
**/*.{yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/nat-test-llm.mdc)
In workflow/config YAML, set llms.._type: nat_test_llm to stub responses.
Files:
examples/frameworks/strands_demo/src/nat_strands_demo/configs/config.ymlexamples/frameworks/strands_demo/src/nat_strands_demo/configs/agentcore_config.ymlexamples/frameworks/strands_demo/src/nat_strands_demo/configs/eval_config.yml
**/*.{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:
examples/frameworks/strands_demo/src/nat_strands_demo/configs/config.ymlexamples/frameworks/strands_demo/src/nat_strands_demo/configs/agentcore_config.ymlpackages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.pypackages/nvidia_nat_strands/tests/test_tool_wrapper.pyexamples/frameworks/strands_demo/src/nat_strands_demo/url_directory.pyexamples/frameworks/strands_demo/src/nat_strands_demo/register.pytests/nat/profiler/test_callback_handler.pypackages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.pyexamples/frameworks/strands_demo/src/nat_strands_demo/configs/eval_config.ymlpackages/nvidia_nat_strands/tests/test_callback_handler.py
**/configs/**
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Configuration files consumed by code must be stored next to that code in a configs/ folder
Files:
examples/frameworks/strands_demo/src/nat_strands_demo/configs/config.ymlexamples/frameworks/strands_demo/src/nat_strands_demo/configs/agentcore_config.ymlexamples/frameworks/strands_demo/src/nat_strands_demo/configs/eval_config.yml
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 apyproject.tomlfile. Optionally, it might also contain scripts in ascripts/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 nameddata/, and should
be checked into git-lfs.
Files:
examples/frameworks/strands_demo/src/nat_strands_demo/configs/config.ymlexamples/frameworks/strands_demo/src/nat_strands_demo/configs/agentcore_config.ymlexamples/frameworks/strands_demo/src/nat_strands_demo/data/strands.jsonexamples/frameworks/strands_demo/src/nat_strands_demo/url_directory.pyexamples/frameworks/strands_demo/src/nat_strands_demo/register.pyexamples/frameworks/strands_demo/src/nat_strands_demo/configs/eval_config.yml
**/*.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_strands/src/nat/plugins/strands/strands_callback_handler.pypackages/nvidia_nat_strands/tests/test_tool_wrapper.pyexamples/frameworks/strands_demo/src/nat_strands_demo/url_directory.pyexamples/frameworks/strands_demo/src/nat_strands_demo/register.pytests/nat/profiler/test_callback_handler.pypackages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.pypackages/nvidia_nat_strands/tests/test_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_strands/src/nat/plugins/strands/strands_callback_handler.pypackages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.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_strands/src/nat/plugins/strands/strands_callback_handler.pypackages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.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_strands/tests/test_tool_wrapper.pypackages/nvidia_nat_strands/tests/test_callback_handler.py
**/data/**
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Large/binary assets must be committed with Git-LFS and placed in a neighbouring data/ folder
Files:
examples/frameworks/strands_demo/src/nat_strands_demo/data/strands.json
tests/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Unit tests reside under tests/ and should use markers defined in pyproject.toml (e.g., integration)
Files:
tests/nat/profiler/test_callback_handler.py
⚙️ CodeRabbit configuration file
tests/**/*.py: - Ensure that tests are comprehensive, cover edge cases, and validate the functionality of the code. - Test functions should be named using thetest_prefix, using snake_case. - Any frequently repeated code should be extracted into pytest fixtures. - Pytest fixtures should define the name argument when applying the pytest.fixture decorator. The fixture
function being decorated should be named using thefixture_prefix, using snake_case. Example:
@pytest.fixture(name="my_fixture")
def fixture_my_fixture():
pass
Files:
tests/nat/profiler/test_callback_handler.py
{tests/**/*.py,examples/*/tests/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{tests/**/*.py,examples/*/tests/**/*.py}: Use pytest (with pytest-asyncio for async); name test files test_*.py; test functions start with test_; extract repeated code into fixtures; fixtures must set name in decorator and be named with fixture_ prefix
Mock external services with pytest_httpserver or unittest.mock; do not hit live endpoints
Mark expensive tests with @pytest.mark.slow or @pytest.mark.integration
Files:
tests/nat/profiler/test_callback_handler.py
🧠 Learnings (1)
📚 Learning: 2025-09-23T18:39:15.023Z
Learnt from: CR
PR: NVIDIA/NeMo-Agent-Toolkit#0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-09-23T18:39:15.023Z
Learning: Applies to packages/*/pyproject.toml : In packages, declare a dependency on nvidia-nat or packages starting with nvidia-nat-
Applied to files:
packages/nvidia_nat_all/pyproject.tomlpackages/nvidia_nat_strands/pyproject.tomlpyproject.toml
🧬 Code graph analysis (7)
packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py (4)
src/nat/builder/context.py (3)
Context(118-316)intermediate_step_manager(172-183)output(59-60)src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-26)src/nat/data_models/intermediate_step.py (11)
IntermediateStepPayload(130-232)IntermediateStepType(42-58)StreamEventData(67-77)TraceMetadata(114-127)UsageInfo(80-83)event_type(265-266)framework(277-278)UUID(301-302)data(293-294)usage_info(297-298)span_event_timestamp(273-274)src/nat/profiler/callbacks/token_usage_base_model.py (1)
TokenUsageBaseModel(20-29)
packages/nvidia_nat_strands/tests/test_tool_wrapper.py (1)
packages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py (9)
_json_schema_from_pydantic(32-41)_to_error_result(56-63)_to_tool_result(44-53)strands_tool_wrapper(117-123)tool_spec(85-86)NATFunctionAgentTool(66-113)tool_name(81-82)stream(92-113)tool_type(89-90)
examples/frameworks/strands_demo/src/nat_strands_demo/url_directory.py (3)
src/nat/builder/builder.py (1)
Builder(68-290)src/nat/builder/function_info.py (2)
FunctionInfo(290-625)from_fn(552-625)src/nat/data_models/function.py (1)
FunctionBaseConfig(26-27)
examples/frameworks/strands_demo/src/nat_strands_demo/register.py (6)
src/nat/builder/builder.py (1)
Builder(68-290)src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-26)src/nat/builder/function_info.py (2)
FunctionInfo(290-625)from_fn(552-625)src/nat/data_models/component_ref.py (2)
FunctionRef(94-102)LLMRef(116-124)src/nat/data_models/function.py (1)
FunctionBaseConfig(26-27)examples/frameworks/strands_demo/src/nat_strands_demo/url_directory.py (1)
url_directory(55-107)
tests/nat/profiler/test_callback_handler.py (3)
packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py (3)
StrandsProfilerHandler(172-491)on_before_tool_invocation(56-98)on_after_tool_invocation(100-154)src/nat/data_models/intermediate_step.py (4)
event_type(265-266)IntermediateStepType(42-58)framework(277-278)UUID(301-302)src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-26)
packages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py (3)
src/nat/builder/builder.py (1)
Builder(68-290)src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-26)packages/nvidia_nat_strands/tests/test_tool_wrapper.py (2)
model_json_schema(61-62)tool_spec(136-138)
packages/nvidia_nat_strands/tests/test_callback_handler.py (5)
src/nat/builder/context.py (3)
Context(118-316)intermediate_step_manager(172-183)output(59-60)src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-26)src/nat/data_models/intermediate_step.py (5)
IntermediateStepType(42-58)event_type(265-266)UUID(301-302)framework(277-278)data(293-294)packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py (10)
StrandsProfilerHandler(172-491)StrandsToolInstrumentationHook(39-169)on_before_tool_invocation(56-98)on_after_tool_invocation(100-154)instrument(184-245)_extract_model_info(300-331)_extract_text_from_event(447-462)_extract_usage_from_event(464-491)_wrap_stream_method(333-445)_instrument_agent_init(247-298)packages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py (3)
tool_name(81-82)tool_spec(85-86)NATFunctionAgentTool(66-113)
🪛 Biome (2.1.2)
examples/frameworks/strands_demo/src/nat_strands_demo/data/strands.json
[error] 1-1: String values must be double quoted.
(parse)
[error] 1-1: String values must be double quoted.
(parse)
[error] 1-2: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 2-2: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 2-2: String values must be double quoted.
(parse)
[error] 2-3: String values must be double quoted.
(parse)
[error] 3-3: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
🪛 Ruff (0.14.1)
packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py
97-97: Unused noqa directive (unused: BLE001)
Remove unused noqa directive
(RUF100)
328-329: try-except-pass detected, consider logging the exception
(S110)
432-433: try-except-pass detected, consider logging the exception
(S110)
packages/nvidia_nat_strands/tests/test_tool_wrapper.py
42-42: Unused method argument: kwargs
(ARG002)
62-62: Create your own exception
(TRY002)
62-62: Avoid specifying long messages outside the exception class
(TRY003)
212-212: Unused function argument: kwargs
(ARG001)
249-249: Unused function argument: kwargs
(ARG001)
251-251: Avoid specifying long messages outside the exception class
(TRY003)
examples/frameworks/strands_demo/src/nat_strands_demo/url_directory.py
50-50: Avoid specifying long messages outside the exception class
(TRY003)
104-104: Redundant exception object included in logging.exception call
(TRY401)
105-105: Use explicit conversion flag
Replace with conversion flag
(RUF010)
examples/frameworks/strands_demo/src/nat_strands_demo/register.py
28-28: Unused noqa directive (non-enabled: F401)
Remove unused noqa directive
(RUF100)
29-29: Unused noqa directive (non-enabled: F401)
Remove unused noqa directive
(RUF100)
77-77: Consider [*nat_tools, http_request] instead of concatenation
Replace with [*nat_tools, http_request]
(RUF005)
88-88: Consider moving this statement to an else block
(TRY300)
90-90: Redundant exception object included in logging.exception call
(TRY401)
packages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py
38-38: Consider moving this statement to an else block
(TRY300)
40-40: Redundant exception object included in logging.exception call
(TRY401)
61-61: Use explicit conversion flag
Replace with conversion flag
(RUF010)
92-92: Unused method argument: invocation_state
(ARG002)
92-92: Unused method argument: kwargs
(ARG002)
111-111: Unused noqa directive (unused: BLE001)
Remove unused noqa directive
(RUF100)
112-112: Redundant exception object included in logging.exception call
(TRY401)
117-117: Unused function argument: builder
(ARG001)
119-119: Avoid specifying long messages outside the exception class
(TRY003)
packages/nvidia_nat_strands/tests/test_callback_handler.py
173-173: Avoid specifying long messages outside the exception class
(TRY003)
246-246: Unused method argument: args
(ARG002)
246-246: Unused method argument: kwargs
(ARG002)
256-256: Avoid specifying long messages outside the exception class
(TRY003)
259-259: Avoid specifying long messages outside the exception class
(TRY003)
419-419: Unused function argument: model_self
(ARG001)
419-419: Unused function argument: messages
(ARG001)
419-419: Unused function argument: tool_specs
(ARG001)
419-419: Unused function argument: system_prompt
(ARG001)
419-419: Unused function argument: kwargs
(ARG001)
468-468: Unused function argument: model_self
(ARG001)
468-468: Unused function argument: args
(ARG001)
468-468: Unused function argument: kwargs
(ARG001)
470-470: Avoid specifying long messages outside the exception class
(TRY003)
499-499: Unused function argument: model_self
(ARG001)
499-499: Unused function argument: args
(ARG001)
499-499: Unused function argument: kwargs
(ARG001)
527-527: Unused function argument: model_self
(ARG001)
527-527: Unused function argument: messages
(ARG001)
527-527: Unused function argument: tool_specs
(ARG001)
527-527: Unused function argument: system_prompt
(ARG001)
527-527: Unused function argument: kwargs
(ARG001)
576-576: Unused method argument: args
(ARG002)
576-576: Unused method argument: kwargs
(ARG002)
593-593: Consider moving this statement to an else block
(TRY300)
600-600: Avoid specifying long messages outside the exception class
(TRY003)
655-655: Unused method argument: args
(ARG002)
655-655: Unused method argument: kwargs
(ARG002)
671-671: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (11)
packages/nvidia_nat_all/pyproject.toml (1)
39-39: LGTM!The nvidia-nat-strands integration is properly added to both the dependencies list (alphabetically sorted) and workspace sources.
Also applies to: 84-84
pyproject.toml (3)
49-49: Dependency constraint deviates from coding guidelines.The rich dependency was changed from
~=13.9to>=13.9,<15.0.0. Per coding guidelines, use~=<two-digit>version constraints to keep version requirements as open as possible while maintaining safety. The new constraint allows major version 14.x, whereas~=13.9would limit to the 13.x series.If intentional to support rich 14.x, please document the rationale. Otherwise, revert to
~=13.9to follow guidelines.
93-93: LGTM!The Strands integration is properly added to optional-dependencies (line 93), workspace sources (line 192), and examples list (line 222). All additions are alphabetically sorted and follow existing patterns.
Also applies to: 192-192, 222-222
165-169: Dependency conflict resolution is well-justified.The tenacity override is a reasonable approach to resolve the conflict between google-adk (<9.0.0) and strands-agents-tools (>=9.1.2). The comment correctly notes API compatibility between 8.x and 9.x versions.
Please confirm that google-adk functions correctly with tenacity 9.1.2+ in testing, to ensure the API compatibility assumption holds in practice.
examples/frameworks/strands_demo/src/nat_strands_demo/data/strands.json (1)
1-3: LGTM! Git LFS pointer is correctly formatted.This is a properly formatted Git LFS pointer file for managing the large dataset. The static analysis errors from Biome are false positives - Biome is attempting to parse the LFS pointer as JSON when it's actually a text file with Git LFS metadata.
examples/frameworks/strands_demo/src/nat_strands_demo/configs/eval_config.yml (1)
1-86: LGTM! Comprehensive evaluation configuration.The configuration is well-structured with proper license headers, multiple LLM providers (NIM, Bedrock, OpenAI), and comprehensive evaluation settings including profiler features and RAGAS-based metrics.
tests/nat/profiler/test_callback_handler.py (1)
616-696: LGTM! Comprehensive Strands handler test.The test properly validates that the Strands profiler handler emits TOOL_START and TOOL_END events with correct payloads, tool_inputs, and tool_outputs. The mock event structure accurately simulates Strands' tool invocation flow.
examples/frameworks/strands_demo/src/nat_strands_demo/configs/agentcore_config.yml (1)
1-66: LGTM! AWS AgentCore integration configuration.The configuration properly defines FastAPI endpoints following the AWS AgentCore contract, including the /invocations workflow endpoint and /ping health check. The functions, LLMs, and workflow are well-configured for the Strands demo.
examples/frameworks/strands_demo/src/nat_strands_demo/configs/config.yml (1)
1-46: LGTM! Clean Strands demo configuration.The configuration is well-structured with proper license headers, multiple LLM options (NIM, Bedrock, OpenAI), and a clear workflow definition that leverages the knowledge_base URL directory tool.
packages/nvidia_nat_strands/tests/test_tool_wrapper.py (1)
1-361: LGTM! Comprehensive tool wrapper test coverage.The test suite thoroughly validates:
- JSON schema generation from Pydantic models
- Tool result and error result serialization for various data types
- NATFunctionAgentTool initialization and properties
- Successful and error execution paths
- Streaming function support
- Edge cases (empty input, missing schema, custom exceptions)
The unused argument warnings from static analysis are acceptable for test stub functions.
packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py (1)
172-299: Well-designed instrumentation approach.The auto-registration of tool hooks via Agent.init patching is an elegant solution that provides seamless instrumentation without requiring user code changes. The closure-based approach for capturing the handler reference is correct and avoids common pitfalls with monkey-patching.
examples/frameworks/strands_demo/src/nat_strands_demo/url_directory.py
Outdated
Show resolved
Hide resolved
Thanks @mdemoret-nv, addressed the DCO check. |
Signed-off-by: RanjitR <[email protected]>
Signed-off-by: RanjitR <[email protected]>
ronjer30
force-pushed
the
strands_integration
branch
from
8459762 to
cabfdeb
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pyproject.toml (1)
1-1: Add SPDX header to pyproject.toml.Required by repository guidelines for *.toml files.
+## SPDX-FileCopyrightText: Copyright (c) 2025, NVIDIA CORPORATION & AFFILIATES. All rights reserved. +## SPDX-License-Identifier: Apache-2.0 + [build-system]
♻️ Duplicate comments (4)
packages/nvidia_nat_strands/tests/test_strands_llm.py (1)
376-387: Remove duplicate test assertions.Lines 380-387 contain duplicate code that repeats the same assertion twice without adding value. This was flagged in a previous review.
Apply this diff:
assert result_prompt == "Think step by step" - result_prompt = (thinking_prompt - if not existing_system_prompt else f"{thinking_prompt}\n\n{existing_system_prompt}") - - assert result_prompt == "Think step by step" - result_prompt = (thinking_prompt - if not existing_system_prompt else f"{thinking_prompt}\n\n{existing_system_prompt}") - - assert result_prompt == "Think step by step"examples/frameworks/strands_demo/src/nat_strands_demo/url_directory.py (1)
16-26: Add missing return type, tighten URL validation to http/https, and fix logging per ruff.
- Annotate url_directory to return AsyncGenerator[FunctionInfo, None]; import AsyncGenerator.
- Type the validator parameter/return and restrict schemes to http/https.
- Use logger.exception without "%s", and prefer {e!s} for explicit conversion.
Apply:
@@ -import logging -from urllib.parse import urlparse +import logging +from urllib.parse import urlparse +from collections.abc import AsyncGenerator @@ - @field_validator("urls") - @classmethod - def validate_urls(cls, v): + @field_validator("urls") + @classmethod + def validate_urls(cls, v: dict[str, str]) -> dict[str, str]: """Validate that all URLs are properly formatted.""" for name, url in v.items(): parsed = urlparse(url) - if not all([parsed.scheme, parsed.netloc]): + if not all([parsed.scheme, parsed.netloc]): raise ValueError(f"Invalid URL for '{name}': {url}") + if parsed.scheme not in ("http", "https"): + raise ValueError(f"Unsupported scheme for '{name}': {parsed.scheme} (only http/https allowed)") return v @@ -@register_function(config_type=URLDirectoryConfig) -async def url_directory(config: URLDirectoryConfig, _: Builder): +@register_function(config_type=URLDirectoryConfig) +async def url_directory(config: URLDirectoryConfig, _: Builder) -> AsyncGenerator[FunctionInfo, None]: @@ - except Exception as e: - logger.exception("Error generating URL directory: %s", e) - return f"Error accessing URL directory: {str(e)}" + except Exception as e: # noqa: BLE001 + logger.exception("Error generating URL directory") + return f"Error accessing URL directory: {e!s}"Also applies to: 43-51, 54-56, 103-105
packages/nvidia_nat_strands/tests/test_strands_callback_handler.py (1)
684-685: Remove duplicate assertion.Already asserted on Line 682; drop the repetition.
- # Should have attempted to register hooks despite the error - assert agent.hooks.add_callback.calledpackages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py (1)
44-54: Bug:isinstance(value, dict | list | tuple)raises at runtime.The
isinstancecall cannot accept a PEP 604 union (|syntax). Use a tuple of types instead.Apply this diff:
- if isinstance(value, dict | list | tuple): + if isinstance(value, (dict, list, tuple)):This is a runtime fix and must be applied before release.
🧹 Nitpick comments (7)
examples/frameworks/strands_demo/src/nat_strands_demo/register.py (3)
28-29: Clarify import purpose or remove noqa directives.The
noqa: F401directives appear unnecessary per Ruff. If these imports are needed for side effects (e.g., registering functions), consider adding a comment explaining why they're imported but not directly used.Apply this diff to either remove the noqa or add clarifying comments:
-from . import ping_tool # noqa: F401, pylint: disable=unused-import -from . import url_directory # noqa: F401, pylint: disable=unused-import +# Import for side effects: registers ping_tool and url_directory functions +from . import ping_tool # pylint: disable=unused-import +from . import url_directory # pylint: disable=unused-import
77-77: Consider unpacking syntax for list extension.Apply this diff for more idiomatic list construction:
- all_tools = nat_tools + [http_request] + all_tools = [*nat_tools, http_request]
89-91: Simplify exception logging.Line 90 passes
excredundantly tologger.exception(), which already includes exception info viaexc_info=Trueby default.Apply this diff:
- logger.exception("Strands demo failed: %s", exc) + logger.exception("Strands demo failed")Additionally, consider moving the return statement to an
elseblock per TRY300:try: text: str = "" async for ev in agent.stream_async(inputs): if "data" in ev: text += ev["data"] - return text or "" + else: + return text or "" except Exception as exc: - logger.exception("Strands demo failed: %s", exc) + logger.exception("Strands demo failed") return f"Error: {exc}"packages/nvidia_nat_strands/tests/test_strands_tool_wrapper.py (1)
102-114: Align fixture naming and explicit names with repo test conventions.Optional, but recommended for consistency.
- @pytest.fixture - def mock_function(self): + @pytest.fixture(name="mock_function") + def fixture_mock_function(self): @@ - @pytest.fixture - def tool_spec(self): + @pytest.fixture(name="tool_spec") + def fixture_tool_spec(self):Also applies to: 128-140
packages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py (3)
32-42: Remove redundant exception object from logger.exception call.The
logger.exception()call automatically captures exception information, so passingexcas a parameter is redundant.Apply this diff:
- logger.exception("Failed to generate JSON schema: %s", exc) + logger.exception("Failed to generate JSON schema")
56-63: Consider using explicit conversion flag in f-string.The
str(err)can be replaced with an explicit!sconversion flag for clarity.Apply this diff:
- "text": f"{type(err).__name__}: {str(err)}" + "text": f"{type(err).__name__}: {err!s}"
92-114: Add return type hint and remove redundant exception object.The
streammethod is missing a return type hint, and thelogger.exception()call includes a redundant exception object.Apply this diff:
- async def stream(self, tool_use: ToolUse, invocation_state: dict[str, Any], **kwargs: Any): + async def stream(self, tool_use: ToolUse, invocation_state: dict[str, Any], **kwargs: Any) -> typing.AsyncGenerator[Any, None]:Also, remove the redundant exception object:
- logger.exception("Strands tool '%s' failed: %s", self.tool_name, exc) + logger.exception("Strands tool '%s' failed", self.tool_name)Note: The
noqa: BLE001directive on line 111 appears to be unused according to ruff; consider removing it if the rule is not active in your configuration.Additionally, verify whether the unused arguments
invocation_stateandkwargsare required by theAgentToolinterface. If they are interface requirements, consider prefixing with underscore (e.g.,_invocation_state) to indicate they're intentionally unused.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (26)
examples/frameworks/strands_demo/README.md(1 hunks)examples/frameworks/strands_demo/configs(1 hunks)examples/frameworks/strands_demo/data(1 hunks)examples/frameworks/strands_demo/pyproject.toml(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/__init__.py(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/configs/agentcore_config.yml(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/configs/config.yml(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/configs/eval_config.yml(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/data/strands.json(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/ping_tool.py(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/register.py(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/url_directory.py(1 hunks)packages/nvidia_nat_all/pyproject.toml(2 hunks)packages/nvidia_nat_strands/pyproject.toml(1 hunks)packages/nvidia_nat_strands/src/nat/plugins/strands/__init__.py(1 hunks)packages/nvidia_nat_strands/src/nat/plugins/strands/llm.py(1 hunks)packages/nvidia_nat_strands/src/nat/plugins/strands/register.py(1 hunks)packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py(1 hunks)packages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py(1 hunks)packages/nvidia_nat_strands/tests/test_strands_callback_handler.py(1 hunks)packages/nvidia_nat_strands/tests/test_strands_llm.py(1 hunks)packages/nvidia_nat_strands/tests/test_strands_tool_wrapper.py(1 hunks)pyproject.toml(6 hunks)src/nat/builder/framework_enum.py(1 hunks)src/nat/profiler/decorators/framework_wrapper.py(2 hunks)tests/nat/profiler/test_callback_handler.py(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- examples/frameworks/strands_demo/configs
- packages/nvidia_nat_strands/src/nat/plugins/strands/init.py
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/nvidia_nat_strands/src/nat/plugins/strands/register.py
- packages/nvidia_nat_all/pyproject.toml
- packages/nvidia_nat_strands/pyproject.toml
- examples/frameworks/strands_demo/README.md
- tests/nat/profiler/test_callback_handler.py
- examples/frameworks/strands_demo/src/nat_strands_demo/configs/eval_config.yml
- examples/frameworks/strands_demo/data
- examples/frameworks/strands_demo/src/nat_strands_demo/init.py
- examples/frameworks/strands_demo/pyproject.toml
- examples/frameworks/strands_demo/src/nat_strands_demo/ping_tool.py
🧰 Additional context used
📓 Path-based instructions (13)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: In code comments use the abbreviations: nat (API namespace/CLI), nvidia-nat (package), NAT (env var prefixes); never use these abbreviations in documentation
Follow PEP 20 and PEP 8 for Python style
Run yapf with column_limit=120; yapf is used for formatting (run second)
Indent with 4 spaces (no tabs) and end each file with a single trailing newline
Use ruff (ruff check --fix) as a linter (not formatter) per pyproject.toml; fix warnings unless explicitly ignored
Respect Python naming schemes: snake_case for functions/variables, PascalCase for classes, UPPER_CASE for constants
When re-raising exceptions, use bare raise to preserve stack trace; log with logger.error(), not logger.exception()
When catching and logging without re-raising, use logger.exception() to capture full stack trace
Provide Google-style docstrings for every public module, class, function, and CLI command
Docstring first line must be a concise description ending with a period
Validate and sanitize all user input, especially in web or CLI interfaces
Prefer httpx with SSL verification enabled by default and follow OWASP Top-10 recommendations
Use async/await for I/O-bound work (HTTP, DB, file I/O)
Cache expensive computations with functools.lru_cache or an external cache when appropriate
Leverage NumPy vectorized operations when beneficial and feasible
Files:
packages/nvidia_nat_strands/tests/test_strands_llm.pysrc/nat/profiler/decorators/framework_wrapper.pysrc/nat/builder/framework_enum.pypackages/nvidia_nat_strands/src/nat/plugins/strands/llm.pypackages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.pypackages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.pypackages/nvidia_nat_strands/tests/test_strands_callback_handler.pypackages/nvidia_nat_strands/tests/test_strands_tool_wrapper.pyexamples/frameworks/strands_demo/src/nat_strands_demo/url_directory.pyexamples/frameworks/strands_demo/src/nat_strands_demo/register.py
{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}: Use pytest with pytest-asyncio for async code
Test functions should be named with test_ prefix using snake_case
Extract frequently repeated test code into pytest fixtures
Pytest fixtures should define the name argument in @pytest.fixture and the function should be prefixed fixture_ using snake_case
Mock external services with pytest_httpserver or unittest.mock; avoid live endpoints
Mark slow tests with @pytest.mark.slow to skip in default test suite
Mark integration tests requiring external services with @pytest.mark.integration and follow nat-tests guidelines
Files:
packages/nvidia_nat_strands/tests/test_strands_llm.pypackages/nvidia_nat_strands/tests/test_strands_callback_handler.pypackages/nvidia_nat_strands/tests/test_strands_tool_wrapper.py
{tests/test_*.py,examples/*/tests/test_*.py,packages/*/tests/test_*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Name test files as test_*.py and store them in a tests/ folder
Files:
packages/nvidia_nat_strands/tests/test_strands_llm.pypackages/nvidia_nat_strands/tests/test_strands_callback_handler.pypackages/nvidia_nat_strands/tests/test_strands_tool_wrapper.py
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}: Every file must start with the standard SPDX Apache-2.0 header
Confirm copyright years are up to date when a file is changed
All source files must include the SPDX Apache-2.0 header template (copy from an existing file)
Files:
packages/nvidia_nat_strands/tests/test_strands_llm.pyexamples/frameworks/strands_demo/src/nat_strands_demo/configs/agentcore_config.ymlsrc/nat/profiler/decorators/framework_wrapper.pysrc/nat/builder/framework_enum.pypackages/nvidia_nat_strands/src/nat/plugins/strands/llm.pyexamples/frameworks/strands_demo/src/nat_strands_demo/configs/config.ymlpackages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.pypackages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.pypackages/nvidia_nat_strands/tests/test_strands_callback_handler.pypackages/nvidia_nat_strands/tests/test_strands_tool_wrapper.pyexamples/frameworks/strands_demo/src/nat_strands_demo/url_directory.pypyproject.tomlexamples/frameworks/strands_demo/src/nat_strands_demo/data/strands.jsonexamples/frameworks/strands_demo/src/nat_strands_demo/register.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
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.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.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile 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_strands/tests/test_strands_llm.pyexamples/frameworks/strands_demo/src/nat_strands_demo/configs/agentcore_config.ymlsrc/nat/profiler/decorators/framework_wrapper.pysrc/nat/builder/framework_enum.pypackages/nvidia_nat_strands/src/nat/plugins/strands/llm.pyexamples/frameworks/strands_demo/src/nat_strands_demo/configs/config.ymlpackages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.pypackages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.pypackages/nvidia_nat_strands/tests/test_strands_callback_handler.pypackages/nvidia_nat_strands/tests/test_strands_tool_wrapper.pyexamples/frameworks/strands_demo/src/nat_strands_demo/url_directory.pypyproject.tomlexamples/frameworks/strands_demo/src/nat_strands_demo/data/strands.jsonexamples/frameworks/strands_demo/src/nat_strands_demo/register.py
packages/**/*
⚙️ CodeRabbit configuration file
packages/**/*: - This directory contains optional plugin packages for the toolkit, each should contain apyproject.tomlfile. - Thepyproject.tomlfile should declare a dependency onnvidia-nator another package with a name starting
withnvidia-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 thepyproject.tomlfile.
Files:
packages/nvidia_nat_strands/tests/test_strands_llm.pypackages/nvidia_nat_strands/src/nat/plugins/strands/llm.pypackages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.pypackages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.pypackages/nvidia_nat_strands/tests/test_strands_callback_handler.pypackages/nvidia_nat_strands/tests/test_strands_tool_wrapper.py
**/configs/**
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Configuration files consumed by code should be stored next to that code in a configs/ folder
Files:
examples/frameworks/strands_demo/src/nat_strands_demo/configs/agentcore_config.ymlexamples/frameworks/strands_demo/src/nat_strands_demo/configs/config.yml
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 apyproject.tomlfile. Optionally, it might also contain scripts in ascripts/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 nameddata/, and should
be checked into git-lfs.
Files:
examples/frameworks/strands_demo/src/nat_strands_demo/configs/agentcore_config.ymlexamples/frameworks/strands_demo/src/nat_strands_demo/configs/config.ymlexamples/frameworks/strands_demo/src/nat_strands_demo/url_directory.pyexamples/frameworks/strands_demo/src/nat_strands_demo/data/strands.jsonexamples/frameworks/strands_demo/src/nat_strands_demo/register.py
{src/**/*.py,packages/*/src/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{src/**/*.py,packages/*/src/**/*.py}: All importable Python code must live under src/ or packages//src/
All public APIs must have Python 3.11+ type hints on parameters and return values
Prefer typing/collections.abc abstractions (e.g., Sequence over list)
Use typing.Annotated for units or metadata when useful
Treat pyright warnings as errors during development
Files:
src/nat/profiler/decorators/framework_wrapper.pysrc/nat/builder/framework_enum.pypackages/nvidia_nat_strands/src/nat/plugins/strands/llm.pypackages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.pypackages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py
src/nat/**/*
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
src/nat/**/* contains core functionality; changes should prioritize backward compatibility
Files:
src/nat/profiler/decorators/framework_wrapper.pysrc/nat/builder/framework_enum.py
⚙️ CodeRabbit configuration file
This directory contains the core functionality of the toolkit. Changes should prioritize backward compatibility.
Files:
src/nat/profiler/decorators/framework_wrapper.pysrc/nat/builder/framework_enum.py
packages/*/src/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
If a package contains Python code, it must have tests in a tests/ directory at the same level as pyproject.toml
Files:
packages/nvidia_nat_strands/src/nat/plugins/strands/llm.pypackages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.pypackages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py
{**/pyproject.toml,uv.lock}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
New dependencies must be added to both pyproject.toml (alphabetically) and uv.lock via ‘uv pip install --sync’
Files:
pyproject.toml
**/data/**
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Large/binary assets must be committed with Git-LFS and placed in a neighbouring data/ folder
Files:
examples/frameworks/strands_demo/src/nat_strands_demo/data/strands.json
🧬 Code graph analysis (9)
packages/nvidia_nat_strands/tests/test_strands_llm.py (6)
src/nat/builder/builder.py (1)
Builder(68-290)src/nat/data_models/llm.py (1)
APITypeEnum(25-27)src/nat/llm/aws_bedrock_llm.py (1)
AWSBedrockModelConfig(33-63)src/nat/llm/nim_llm.py (1)
NIMModelConfig(34-52)src/nat/llm/openai_llm.py (2)
openai_llm(53-55)OpenAIModelConfig(32-49)packages/nvidia_nat_strands/src/nat/plugins/strands/llm.py (4)
_patch_llm_based_on_config(38-93)bedrock_strands(179-203)nim_strands(123-175)openai_strands(97-119)
src/nat/profiler/decorators/framework_wrapper.py (2)
src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-26)packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py (2)
StrandsProfilerHandler(172-490)instrument(184-245)
packages/nvidia_nat_strands/src/nat/plugins/strands/llm.py (11)
src/nat/builder/builder.py (1)
Builder(68-290)src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-26)src/nat/data_models/llm.py (1)
LLMBaseConfig(30-38)src/nat/data_models/thinking_mixin.py (2)
ThinkingMixin(29-86)thinking_system_prompt(49-86)src/nat/llm/aws_bedrock_llm.py (1)
AWSBedrockModelConfig(33-63)src/nat/llm/nim_llm.py (1)
NIMModelConfig(34-52)src/nat/llm/openai_llm.py (2)
openai_llm(53-55)OpenAIModelConfig(32-49)src/nat/llm/utils/thinking.py (3)
BaseThinkingInjector(57-81)FunctionArgumentWrapper(34-53)patch_with_thinking(121-215)src/nat/utils/exception_handlers/automatic_retries.py (1)
patch_with_retry(412-493)src/nat/utils/responses_api.py (1)
validate_no_responses_api(21-26)src/nat/utils/type_utils.py (1)
override(56-57)
packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py (5)
src/nat/builder/context.py (3)
Context(118-316)intermediate_step_manager(172-183)output(59-60)src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-26)src/nat/data_models/intermediate_step.py (11)
IntermediateStepPayload(130-232)IntermediateStepType(42-58)StreamEventData(67-77)TraceMetadata(114-127)UsageInfo(80-83)event_type(265-266)framework(277-278)UUID(301-302)data(293-294)usage_info(297-298)span_event_timestamp(273-274)src/nat/profiler/callbacks/token_usage_base_model.py (1)
TokenUsageBaseModel(20-29)packages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py (1)
tool_name(81-82)
packages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py (2)
src/nat/builder/builder.py (1)
Builder(68-290)src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-26)
packages/nvidia_nat_strands/tests/test_strands_callback_handler.py (5)
src/nat/builder/context.py (3)
Context(118-316)intermediate_step_manager(172-183)output(59-60)src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-26)src/nat/data_models/intermediate_step.py (5)
IntermediateStepType(42-58)event_type(265-266)UUID(301-302)framework(277-278)data(293-294)packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py (10)
StrandsProfilerHandler(172-490)StrandsToolInstrumentationHook(39-169)on_before_tool_invocation(56-98)on_after_tool_invocation(100-154)instrument(184-245)_extract_model_info(299-330)_extract_text_from_event(446-461)_extract_usage_from_event(463-490)_wrap_stream_method(332-444)_instrument_agent_init(247-297)packages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py (2)
tool_name(81-82)NATFunctionAgentTool(66-113)
packages/nvidia_nat_strands/tests/test_strands_tool_wrapper.py (1)
packages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py (7)
_json_schema_from_pydantic(32-41)_to_error_result(56-63)_to_tool_result(44-53)strands_tool_wrapper(117-123)NATFunctionAgentTool(66-113)tool_name(81-82)tool_type(89-90)
examples/frameworks/strands_demo/src/nat_strands_demo/url_directory.py (3)
src/nat/builder/builder.py (1)
Builder(68-290)src/nat/builder/function_info.py (2)
FunctionInfo(290-625)from_fn(552-625)src/nat/data_models/function.py (1)
FunctionBaseConfig(26-27)
examples/frameworks/strands_demo/src/nat_strands_demo/register.py (5)
src/nat/builder/builder.py (1)
Builder(68-290)src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-26)src/nat/builder/function_info.py (2)
FunctionInfo(290-625)from_fn(552-625)src/nat/data_models/component_ref.py (2)
FunctionRef(94-102)LLMRef(116-124)examples/frameworks/strands_demo/src/nat_strands_demo/url_directory.py (1)
url_directory(55-107)
🪛 Biome (2.1.2)
examples/frameworks/strands_demo/src/nat_strands_demo/data/strands.json
[error] 1-1: String values must be double quoted.
(parse)
[error] 1-1: String values must be double quoted.
(parse)
[error] 1-2: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 2-2: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 2-2: String values must be double quoted.
(parse)
[error] 2-3: String values must be double quoted.
(parse)
[error] 3-3: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
🪛 Ruff (0.14.1)
packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py
97-97: Unused noqa directive (unused: BLE001)
Remove unused noqa directive
(RUF100)
327-328: try-except-pass detected, consider logging the exception
(S110)
431-432: try-except-pass detected, consider logging the exception
(S110)
packages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py
38-38: Consider moving this statement to an else block
(TRY300)
40-40: Redundant exception object included in logging.exception call
(TRY401)
61-61: Use explicit conversion flag
Replace with conversion flag
(RUF010)
92-92: Unused method argument: invocation_state
(ARG002)
92-92: Unused method argument: kwargs
(ARG002)
111-111: Unused noqa directive (unused: BLE001)
Remove unused noqa directive
(RUF100)
112-112: Redundant exception object included in logging.exception call
(TRY401)
117-117: Unused function argument: builder
(ARG001)
119-119: Avoid specifying long messages outside the exception class
(TRY003)
packages/nvidia_nat_strands/tests/test_strands_callback_handler.py
173-173: Avoid specifying long messages outside the exception class
(TRY003)
246-246: Unused method argument: args
(ARG002)
246-246: Unused method argument: kwargs
(ARG002)
256-256: Avoid specifying long messages outside the exception class
(TRY003)
259-259: Avoid specifying long messages outside the exception class
(TRY003)
419-419: Unused function argument: model_self
(ARG001)
419-419: Unused function argument: messages
(ARG001)
419-419: Unused function argument: tool_specs
(ARG001)
419-419: Unused function argument: system_prompt
(ARG001)
419-419: Unused function argument: kwargs
(ARG001)
468-468: Unused function argument: model_self
(ARG001)
468-468: Unused function argument: args
(ARG001)
468-468: Unused function argument: kwargs
(ARG001)
470-470: Avoid specifying long messages outside the exception class
(TRY003)
499-499: Unused function argument: model_self
(ARG001)
499-499: Unused function argument: args
(ARG001)
499-499: Unused function argument: kwargs
(ARG001)
527-527: Unused function argument: model_self
(ARG001)
527-527: Unused function argument: messages
(ARG001)
527-527: Unused function argument: tool_specs
(ARG001)
527-527: Unused function argument: system_prompt
(ARG001)
527-527: Unused function argument: kwargs
(ARG001)
576-576: Unused method argument: args
(ARG002)
576-576: Unused method argument: kwargs
(ARG002)
593-593: Consider moving this statement to an else block
(TRY300)
600-600: Avoid specifying long messages outside the exception class
(TRY003)
655-655: Unused method argument: args
(ARG002)
655-655: Unused method argument: kwargs
(ARG002)
671-671: Avoid specifying long messages outside the exception class
(TRY003)
packages/nvidia_nat_strands/tests/test_strands_tool_wrapper.py
42-42: Unused method argument: kwargs
(ARG002)
62-62: Create your own exception
(TRY002)
62-62: Avoid specifying long messages outside the exception class
(TRY003)
212-212: Unused function argument: kwargs
(ARG001)
249-249: Unused function argument: kwargs
(ARG001)
251-251: Avoid specifying long messages outside the exception class
(TRY003)
examples/frameworks/strands_demo/src/nat_strands_demo/url_directory.py
50-50: Avoid specifying long messages outside the exception class
(TRY003)
104-104: Redundant exception object included in logging.exception call
(TRY401)
105-105: Use explicit conversion flag
Replace with conversion flag
(RUF010)
examples/frameworks/strands_demo/src/nat_strands_demo/register.py
28-28: Unused noqa directive (non-enabled: F401)
Remove unused noqa directive
(RUF100)
29-29: Unused noqa directive (non-enabled: F401)
Remove unused noqa directive
(RUF100)
77-77: Consider [*nat_tools, http_request] instead of concatenation
Replace with [*nat_tools, http_request]
(RUF005)
88-88: Consider moving this statement to an else block
(TRY300)
90-90: Redundant exception object included in logging.exception call
(TRY401)
🔇 Additional comments (23)
packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py (7)
97-98: Exception handling is appropriate.The broad exception catch with
exc_info=Truelogging is acceptable here since this is a telemetry hook that should not disrupt the main workflow.
172-246: Instrumentation design is sound.The dynamic import and monkey-patching approach is appropriate for optional framework integration. The use of
setattrto replace methods ensures instrumentation applies to all instances.Note: The static analysis hint about unused
noqaat line 97 appears to be a false positive —BLE001is the correct suppression for the broad exception catch.
247-298: Auto-registration pattern is well-implemented.The Agent.init patching ensures hooks are registered automatically on every agent instance without requiring user code changes. The closure-based design correctly captures the handler reference.
299-330: Model info extraction is robust.The fallback logic for extracting model name and parameters handles various Strands model implementations gracefully. Silent exception handling at lines 327-328 is appropriate since params extraction is optional.
332-444: Stream wrapper ensures proper span lifecycle.The
finallyblock at line 440-442 guarantees thatLLM_ENDis emitted even if streaming fails or is interrupted. The use ofcopy.deepcopyprevents mutation of message data.
446-490: Helper methods are well-implemented.Both extraction methods use appropriate type guards and return safe defaults. The nested dictionary navigation for usage info is defensive and handles missing keys gracefully.
39-54: Verify if step_manager caching is safe by understanding tool invocation execution context.The cached
_step_managerat line 54 assumes the NAT context remains stable for all tool invocations. However, if tool invocations execute in different asyncio tasks or contexts (e.g., concurrent requests), the cached reference may stale and miss the active context.Note that the codebase shows mixed patterns: line 54 caches the manager in this class, but line 342 in
StrandsProfilerHandler._wrap_stream_methodfetches it fresh. This inconsistency suggests the relative safety of caching is context-dependent.Verify the execution model: Do all tool invocations within a single agent request execute in the same asyncio task/context, or can they span different contexts? If contexts can differ, fetch
intermediate_step_managerfresh in each callback (lines 79, 137) rather than caching in__init__.src/nat/builder/framework_enum.py (1)
26-26: LGTM!The STRANDS enum member follows the established naming pattern and integrates cleanly with the existing framework enum.
examples/frameworks/strands_demo/src/nat_strands_demo/data/strands.json (1)
1-3: Git LFS pointer is correctly formatted.This is a valid Git LFS pointer file for managing the large dataset. The static analysis errors from Biome are false positives — this is not a JSON file but a Git LFS pointer format.
src/nat/profiler/decorators/framework_wrapper.py (2)
37-37: LGTM!The "strands" key follows the established pattern for framework instrumentation flags.
135-148: Strands integration follows established pattern.The conditional import with helpful error messaging matches the approach used for ADK integration. The handler instrumentation is correctly guarded by the instrumentation flag.
examples/frameworks/strands_demo/src/nat_strands_demo/configs/agentcore_config.yml (1)
1-66: Configuration is well-structured.The AgentCore configuration correctly defines the FastAPI endpoints, functions, LLMs, and workflow. The URL directory provides documentation references, and the system prompt appropriately guides the agent to source information from the knowledge base.
examples/frameworks/strands_demo/src/nat_strands_demo/configs/config.yml (1)
1-46: Standalone configuration is clean.This simplified config provides a clear example for running the Strands demo without AgentCore endpoints. The switch to
nim_llmoffers an alternative LLM provider choice.packages/nvidia_nat_strands/tests/test_strands_llm.py (5)
32-80: OpenAI test coverage is comprehensive.The tests validate async context manager usage, parameter handling, and API type validation. Good use of fixtures and mocking.
82-124: Bedrock tests are appropriate.The tests cover basic functionality and API validation. Clean and concise.
126-271: NIM integration tests are thorough.Tests cover environment variables, default URLs, parameter exclusion, and message formatting. The NIM-specific message formatting test at lines 203-242 validates an important compatibility layer.
273-335: Patching tests validate mixin behavior.The tests for
_patch_llm_based_on_configcorrectly verify that retry and thinking mixins are applied when present. Clear test structure.
337-369: Thinking injector tests are clear.Tests validate prompt combination logic for various scenarios. Well-organized test cases.
examples/frameworks/strands_demo/src/nat_strands_demo/register.py (1)
34-96: Strands demo implementation is solid.The workflow correctly integrates NAT tools with Strands' http_request tool and properly handles streaming responses. The error handling ensures graceful degradation.
pyproject.toml (1)
93-99: All workspace/package wiring and lock synchronization verified successfully—no issues found.
- ✓ uv.lock is present and updated: tenacity override properly synced (
>=9.1.2,<10.0.0at uv.lock:39; version 9.1.2 locked at uv.lock:9717)- ✓ packages/nvidia_nat_strands/pyproject.toml declares
nvidia-nat~=1.3(correct two-digit constraint format)- ✓ examples/frameworks/strands_demo contains both README.md and pyproject.toml per examples/ rules
- Override rationale documented with explanatory comment at pyproject.toml:166-168
packages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py (3)
1-15: LGTM: License header is correct.The SPDX header and copyright year are properly formatted.
16-30: LGTM: Imports are well-organized.The imports are properly structured with appropriate type ignore comments for the Strands library.
66-91: LGTM: Class structure and properties are well-defined.The
NATFunctionAgentToolclass correctly extendsAgentTooland provides appropriate properties.
… reasoning models in NIMs, and addresses other PR feedback Signed-off-by: RanjitR <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (1)
packages/nvidia_nat_strands/tests/test_strands_tool_wrapper.py (1)
149-311: Add@pytest.mark.asynciodecorator to async test methods.All async test methods in this class are missing the required
@pytest.mark.asynciodecorator. The following methods need the decorator:
test_nat_function_agent_tool_stream_success(line 150)test_nat_function_agent_tool_stream_error(line 179)test_nat_function_agent_tool_streaming_function(line 206)test_nat_function_agent_tool_streaming_with_error(line 245)test_nat_function_agent_tool_empty_input(line 294)This was already noted in a past review. Apply this diff:
+ @pytest.mark.asyncio async def test_nat_function_agent_tool_stream_success(self, mock_function, tool_spec): """Test successful tool execution.""" from nat.plugins.strands.tool_wrapper import NATFunctionAgentTool tool = NATFunctionAgentTool("test_tool", "Test desc", tool_spec, mock_function) # Mock the tool use - need to use get method properly tool_use = MagicMock() tool_use.get.return_value = {"param": "value"} tool_use.id = "tool_123" # Execute the tool results = [] async for result in tool.stream(tool_use, {}): results.append(result) # Verify the function was called with the input mock_function.acall_invoke.assert_called_once() # Verify we got a result assert len(results) == 1 # Check the result structure - it should contain tool_result result_event = results[0] assert "tool_result" in result_event tool_result = result_event["tool_result"] assert tool_result["status"] == "success" assert "content" in tool_result + @pytest.mark.asyncio async def test_nat_function_agent_tool_stream_error(self, mock_function, tool_spec): """Test tool execution with error.""" from nat.plugins.strands.tool_wrapper import NATFunctionAgentTool # Make the function raise an error mock_function.acall_invoke = AsyncMock(side_effect=ValueError("Test error")) tool = NATFunctionAgentTool("test_tool", "Test desc", tool_spec, mock_function) tool_use = MagicMock() tool_use.get.return_value = {"param": "value"} tool_use.id = "tool_456" # Execute the tool results = [] async for result in tool.stream(tool_use, {}): results.append(result) # Should get an error result assert len(results) == 1 result_event = results[0] assert "tool_result" in result_event tool_result = result_event["tool_result"] assert tool_result["status"] == "error" assert "ValueError" in tool_result["content"][0]["text"] + @pytest.mark.asyncio async def test_nat_function_agent_tool_streaming_function(self, tool_spec): """Test tool with streaming function.""" from nat.plugins.strands.tool_wrapper import NATFunctionAgentTool # Create a mock streaming function mock_function = MagicMock(spec=Function) mock_function.has_streaming_output = True mock_function.has_single_output = False async def mock_stream(**kwargs): yield "chunk1" yield "chunk2" yield "final_chunk" mock_function.acall_stream = mock_stream tool = NATFunctionAgentTool("streaming_tool", "Streaming desc", tool_spec, mock_function) tool_use = MagicMock() tool_use.get.return_value = {"param": "value"} tool_use.__getitem__ = MagicMock(return_value="stream_tool_123") # For toolUseId access # Execute the streaming tool results = [] async for result in tool.stream(tool_use, {}): results.append(result) # Should get stream events plus final result assert len(results) > 1 # Last result should be the final tool result final_result = results[-1] assert "tool_result" in final_result tool_result = final_result["tool_result"] assert tool_result["status"] == "success" # toolUseId comes from tool_use.get("toolUseId", "unknown") assert "toolUseId" in tool_result + @pytest.mark.asyncio async def test_nat_function_agent_tool_streaming_with_error(self, tool_spec): """Test streaming tool with error.""" from nat.plugins.strands.tool_wrapper import NATFunctionAgentTool mock_function = MagicMock(spec=Function) mock_function.has_streaming_output = True mock_function.has_single_output = False async def mock_stream_error(**kwargs): yield "chunk1" raise RuntimeError("Streaming error") mock_function.acall_stream = mock_stream_error tool = NATFunctionAgentTool("error_stream_tool", "Error stream desc", tool_spec, mock_function) tool_use = MagicMock() tool_use.get.return_value = {"param": "value"} tool_use.__getitem__ = MagicMock(return_value="error_stream_456") # For toolUseId access # Execute the tool results = [] async for result in tool.stream(tool_use, {}): results.append(result) # Should get stream events and then an error result assert len(results) >= 1 # Last result should be the error final_result = results[-1] assert "tool_result" in final_result tool_result = final_result["tool_result"] assert tool_result["status"] == "error" assert "RuntimeError" in tool_result["content"][0]["text"] + @pytest.mark.asyncio async def test_nat_function_agent_tool_empty_input(self, mock_function, tool_spec): """Test tool execution with empty input.""" from nat.plugins.strands.tool_wrapper import NATFunctionAgentTool tool = NATFunctionAgentTool("empty_input_tool", "Empty input test", tool_spec, mock_function) tool_use = MagicMock() tool_use.get.return_value = None # Empty input tool_use.__getitem__ = MagicMock(return_value="empty_123") # For toolUseId access # Execute the tool results = [] async for result in tool.stream(tool_use, {}): results.append(result) # Should still work with empty input assert len(results) == 1 mock_function.acall_invoke.assert_called_once_with() # Called with no argsBased on learnings.
🧹 Nitpick comments (6)
examples/frameworks/strands_demo/src/nat_strands_demo/register.py (2)
28-29: Remove unnecessarynoqadirectives.The
noqa: F401directives are unnecessary since the F401 check appears to be disabled in the Ruff configuration.Apply this diff:
-from . import ping_tool # noqa: F401, pylint: disable=unused-import -from . import url_directory # noqa: F401, pylint: disable=unused-import +from . import ping_tool # pylint: disable=unused-import +from . import url_directory # pylint: disable=unused-import
48-62: Revise docstring to describe the logical tool interface for the LLM.The docstring should describe the logical tool interface (what the LLM will call: the
_runfunction) rather than the framework wrapper parameters. The LLM will interact with a function that takes a string input and returns a string response.Based on learnings.
Apply this diff:
- """ - Create a Strands agent workflow that queries documentation URLs. - - This workflow demonstrates NeMo Agent toolkit's Strands integration - by creating an agent that uses a URL directory and HTTP request tool - to answer questions about Strands documentation. - - Args: - config: Configuration specifying LLM, tools, and system prompt - builder: NeMo Agent toolkit builder for resolving components - - Yields: - FunctionInfo wrapping the agent execution function that processes - user inputs and returns agent responses as strings - """ + """ + Answer questions using a Strands agent with access to documentation URLs. + + This agent uses a URL directory to find relevant documentation and can + fetch content from approved URLs to answer your questions. + + Args: + inputs: Your question or request + + Returns: + The agent's response as a string + """examples/frameworks/strands_demo/src/nat_strands_demo/url_directory.py (3)
44-54: Consider extracting exception messages to constants.The validator logic is correct and appropriately restrictive. Optionally, you could extract the exception messages to module-level constants to address the TRY003 style hints.
Example refactor:
+INVALID_URL_MSG = "Invalid URL for '{name}': {url}" +UNSUPPORTED_SCHEME_MSG = "Unsupported scheme for '{name}': {scheme} (only http/https allowed)" + class URLDirectoryConfig(FunctionBaseConfig, name="url_directory"):@field_validator("urls") @classmethod def validate_urls(cls, v: dict[str, str]) -> dict[str, str]: """Validate that all URLs are properly formatted.""" for name, url in v.items(): parsed = urlparse(url) if not all([parsed.scheme, parsed.netloc]): - raise ValueError(f"Invalid URL for '{name}': {url}") + raise ValueError(INVALID_URL_MSG.format(name=name, url=url)) if parsed.scheme not in ("http", "https"): - raise ValueError(f"Unsupported scheme for '{name}': {parsed.scheme} (only http/https allowed)") + raise ValueError(UNSUPPORTED_SCHEME_MSG.format(name=name, scheme=parsed.scheme)) return v
57-66: Return type annotation correctly added. Consider revising docstring for LLM clarity.The return type annotation addresses the past review comment—well done!
However, similar to
register.py, the docstring should describe the logical tool interface (the_get_url_directoryfunction that the LLM will call) rather than the wrapper function. The LLM needs to understand that it can call this with a query string to get a directory of URLs.Based on learnings.
Consider this revision:
- """ - Create a URL directory tool that provides vetted URLs for specific topics. - - This tool acts as a knowledge base directory, providing approved URLs that - the agent can then fetch using the Strands http_request tool. This prevents - URL hallucination and ensures the agent only accesses approved domains. - """ + """ + Get a directory of vetted URLs for specific topics or documentation. + + Use this tool to discover available URLs in the knowledge base before + fetching content with http_request. This ensures you only access + approved domains. + + Args: + query: The topic or type of URL you're looking for + + Returns: + A formatted directory with available URLs, domains, and usage instructions + """
67-110: LGTM! Minor note on exception handling.The directory formatting logic is clear and well-structured. The exception handling correctly uses
logger.exception()when catching and logging without re-raising.The
noqa: BLE001directive may be unnecessary if broad exception catching is already allowed in your Ruff configuration.Optionally remove the unused noqa directive:
- except Exception as e: # noqa: BLE001 + except Exception as e:packages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py (1)
46-55: Remove unusednoqadirective.Line 47 has
# noqa: UP038but UP038 is not enabled in the Ruff configuration. This directive can be removed.Apply this diff:
def _to_tool_result(tool_use_id: str, value: Any) -> dict[str, Any]: - if isinstance(value, (dict, list, tuple)): # noqa: UP038 + if isinstance(value, (dict, list, tuple)): content_item = {"json": value} else: content_item = {"text": str(value)}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
examples/frameworks/strands_demo/README.md(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/register.py(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/url_directory.py(1 hunks)packages/nvidia_nat_strands/pyproject.toml(1 hunks)packages/nvidia_nat_strands/src/nat/plugins/strands/llm.py(1 hunks)packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py(1 hunks)packages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py(1 hunks)packages/nvidia_nat_strands/tests/test_strands_callback_handler.py(1 hunks)packages/nvidia_nat_strands/tests/test_strands_llm.py(1 hunks)packages/nvidia_nat_strands/tests/test_strands_tool_wrapper.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/frameworks/strands_demo/README.md
🧰 Additional context used
📓 Path-based instructions (11)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: In code comments use the abbreviations: nat (API namespace/CLI), nvidia-nat (package), NAT (env var prefixes); never use these abbreviations in documentation
Follow PEP 20 and PEP 8 for Python style
Run yapf with column_limit=120; yapf is used for formatting (run second)
Indent with 4 spaces (no tabs) and end each file with a single trailing newline
Use ruff (ruff check --fix) as a linter (not formatter) per pyproject.toml; fix warnings unless explicitly ignored
Respect Python naming schemes: snake_case for functions/variables, PascalCase for classes, UPPER_CASE for constants
When re-raising exceptions, use bare raise to preserve stack trace; log with logger.error(), not logger.exception()
When catching and logging without re-raising, use logger.exception() to capture full stack trace
Provide Google-style docstrings for every public module, class, function, and CLI command
Docstring first line must be a concise description ending with a period
Validate and sanitize all user input, especially in web or CLI interfaces
Prefer httpx with SSL verification enabled by default and follow OWASP Top-10 recommendations
Use async/await for I/O-bound work (HTTP, DB, file I/O)
Cache expensive computations with functools.lru_cache or an external cache when appropriate
Leverage NumPy vectorized operations when beneficial and feasible
Files:
packages/nvidia_nat_strands/src/nat/plugins/strands/llm.pypackages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.pypackages/nvidia_nat_strands/tests/test_strands_tool_wrapper.pypackages/nvidia_nat_strands/tests/test_strands_callback_handler.pypackages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.pypackages/nvidia_nat_strands/tests/test_strands_llm.pyexamples/frameworks/strands_demo/src/nat_strands_demo/url_directory.pyexamples/frameworks/strands_demo/src/nat_strands_demo/register.py
{src/**/*.py,packages/*/src/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{src/**/*.py,packages/*/src/**/*.py}: All importable Python code must live under src/ or packages//src/
All public APIs must have Python 3.11+ type hints on parameters and return values
Prefer typing/collections.abc abstractions (e.g., Sequence over list)
Use typing.Annotated for units or metadata when useful
Treat pyright warnings as errors during development
Files:
packages/nvidia_nat_strands/src/nat/plugins/strands/llm.pypackages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.pypackages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py
packages/*/src/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
If a package contains Python code, it must have tests in a tests/ directory at the same level as pyproject.toml
Files:
packages/nvidia_nat_strands/src/nat/plugins/strands/llm.pypackages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.pypackages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}: Every file must start with the standard SPDX Apache-2.0 header
Confirm copyright years are up to date when a file is changed
All source files must include the SPDX Apache-2.0 header template (copy from an existing file)
Files:
packages/nvidia_nat_strands/src/nat/plugins/strands/llm.pypackages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.pypackages/nvidia_nat_strands/tests/test_strands_tool_wrapper.pypackages/nvidia_nat_strands/tests/test_strands_callback_handler.pypackages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.pypackages/nvidia_nat_strands/pyproject.tomlpackages/nvidia_nat_strands/tests/test_strands_llm.pyexamples/frameworks/strands_demo/src/nat_strands_demo/url_directory.pyexamples/frameworks/strands_demo/src/nat_strands_demo/register.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
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.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.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile 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_strands/src/nat/plugins/strands/llm.pypackages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.pypackages/nvidia_nat_strands/tests/test_strands_tool_wrapper.pypackages/nvidia_nat_strands/tests/test_strands_callback_handler.pypackages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.pypackages/nvidia_nat_strands/pyproject.tomlpackages/nvidia_nat_strands/tests/test_strands_llm.pyexamples/frameworks/strands_demo/src/nat_strands_demo/url_directory.pyexamples/frameworks/strands_demo/src/nat_strands_demo/register.py
packages/**/*
⚙️ CodeRabbit configuration file
packages/**/*: - This directory contains optional plugin packages for the toolkit, each should contain apyproject.tomlfile. - Thepyproject.tomlfile should declare a dependency onnvidia-nator another package with a name starting
withnvidia-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 thepyproject.tomlfile.
Files:
packages/nvidia_nat_strands/src/nat/plugins/strands/llm.pypackages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.pypackages/nvidia_nat_strands/tests/test_strands_tool_wrapper.pypackages/nvidia_nat_strands/tests/test_strands_callback_handler.pypackages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.pypackages/nvidia_nat_strands/pyproject.tomlpackages/nvidia_nat_strands/tests/test_strands_llm.py
{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}: Use pytest with pytest-asyncio for async code
Test functions should be named with test_ prefix using snake_case
Extract frequently repeated test code into pytest fixtures
Pytest fixtures should define the name argument in @pytest.fixture and the function should be prefixed fixture_ using snake_case
Mock external services with pytest_httpserver or unittest.mock; avoid live endpoints
Mark slow tests with @pytest.mark.slow to skip in default test suite
Mark integration tests requiring external services with @pytest.mark.integration and follow nat-tests guidelines
Files:
packages/nvidia_nat_strands/tests/test_strands_tool_wrapper.pypackages/nvidia_nat_strands/tests/test_strands_callback_handler.pypackages/nvidia_nat_strands/tests/test_strands_llm.py
{tests/test_*.py,examples/*/tests/test_*.py,packages/*/tests/test_*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Name test files as test_*.py and store them in a tests/ folder
Files:
packages/nvidia_nat_strands/tests/test_strands_tool_wrapper.pypackages/nvidia_nat_strands/tests/test_strands_callback_handler.pypackages/nvidia_nat_strands/tests/test_strands_llm.py
packages/*/pyproject.toml
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
packages/*/pyproject.toml: Each package under packages/ must contain a pyproject.toml
packages/*/pyproject.toml must declare a dependency on nvidia-nat or a package starting with nvidia-nat-
Dependencies in pyproject.toml should use ~= with two-digit versions (e.g., ~=1.0)
Files:
packages/nvidia_nat_strands/pyproject.toml
{**/pyproject.toml,uv.lock}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
New dependencies must be added to both pyproject.toml (alphabetically) and uv.lock via ‘uv pip install --sync’
Files:
packages/nvidia_nat_strands/pyproject.toml
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 apyproject.tomlfile. Optionally, it might also contain scripts in ascripts/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 nameddata/, and should
be checked into git-lfs.
Files:
examples/frameworks/strands_demo/src/nat_strands_demo/url_directory.pyexamples/frameworks/strands_demo/src/nat_strands_demo/register.py
🧠 Learnings (9)
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
PR: NVIDIA/NeMo-Agent-Toolkit#0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to **/*.py : Provide Google-style docstrings for every public module, class, function, and CLI command
Applied to files:
packages/nvidia_nat_strands/src/nat/plugins/strands/llm.py
📚 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_strands/src/nat/plugins/strands/llm.pypackages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.pyexamples/frameworks/strands_demo/src/nat_strands_demo/register.py
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
PR: NVIDIA/NeMo-Agent-Toolkit#0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py} : Use pytest with pytest-asyncio for async code
Applied to files:
packages/nvidia_nat_strands/tests/test_strands_tool_wrapper.py
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
PR: NVIDIA/NeMo-Agent-Toolkit#0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py} : Mark integration tests requiring external services with pytest.mark.integration and follow nat-tests guidelines
Applied to files:
packages/nvidia_nat_strands/tests/test_strands_tool_wrapper.py
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
PR: NVIDIA/NeMo-Agent-Toolkit#0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to **/*.py : Use async/await for I/O-bound work (HTTP, DB, file I/O)
Applied to files:
packages/nvidia_nat_strands/tests/test_strands_tool_wrapper.py
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
PR: NVIDIA/NeMo-Agent-Toolkit#0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py} : Pytest fixtures should define the name argument in pytest.fixture and the function should be prefixed fixture_ using snake_case
Applied to files:
packages/nvidia_nat_strands/tests/test_strands_tool_wrapper.py
📚 Learning: 2025-10-09T22:21:15.944Z
Learnt from: onkkul
PR: NVIDIA/NeMo-Agent-Toolkit#859
File: packages/nvidia_nat_autogen/src/nat/plugins/autogen/tool_wrapper.py:115-118
Timestamp: 2025-10-09T22:21:15.944Z
Learning: In `packages/nvidia_nat_autogen/src/nat/plugins/autogen/tool_wrapper.py`, when extracting parameters from input_schema at line 136, the code must handle both dataclasses (standard and Pydantic) and Pydantic BaseModels differently. Pydantic dataclasses have `__pydantic_fields__` (not `model_fields`), so use `dataclasses.fields()` for dataclasses and `model_fields` for BaseModels to properly extract function parameters.
Applied to files:
packages/nvidia_nat_strands/tests/test_strands_tool_wrapper.pypackages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
PR: NVIDIA/NeMo-Agent-Toolkit#0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to packages/*/pyproject.toml : packages/*/pyproject.toml must declare a dependency on nvidia-nat or a package starting with nvidia-nat-
Applied to files:
packages/nvidia_nat_strands/pyproject.toml
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
PR: NVIDIA/NeMo-Agent-Toolkit#0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to packages/*/pyproject.toml : Each package under packages/ must contain a pyproject.toml
Applied to files:
packages/nvidia_nat_strands/pyproject.toml
🧬 Code graph analysis (8)
packages/nvidia_nat_strands/src/nat/plugins/strands/llm.py (9)
src/nat/builder/builder.py (1)
Builder(68-290)src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-26)src/nat/llm/aws_bedrock_llm.py (1)
AWSBedrockModelConfig(33-63)src/nat/llm/nim_llm.py (1)
NIMModelConfig(34-52)src/nat/llm/openai_llm.py (2)
openai_llm(53-55)OpenAIModelConfig(32-49)src/nat/llm/utils/thinking.py (3)
BaseThinkingInjector(57-81)FunctionArgumentWrapper(34-53)patch_with_thinking(121-215)src/nat/utils/exception_handlers/automatic_retries.py (1)
patch_with_retry(412-493)src/nat/utils/responses_api.py (1)
validate_no_responses_api(21-26)src/nat/utils/type_utils.py (1)
override(56-57)
packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py (4)
src/nat/builder/context.py (3)
Context(118-316)intermediate_step_manager(172-183)output(59-60)src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-26)src/nat/data_models/intermediate_step.py (11)
IntermediateStepPayload(130-232)IntermediateStepType(42-58)StreamEventData(67-77)TraceMetadata(114-127)UsageInfo(80-83)event_type(265-266)framework(277-278)UUID(301-302)data(293-294)usage_info(297-298)span_event_timestamp(273-274)src/nat/profiler/callbacks/token_usage_base_model.py (1)
TokenUsageBaseModel(20-29)
packages/nvidia_nat_strands/tests/test_strands_tool_wrapper.py (1)
packages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py (9)
_json_schema_from_pydantic(34-43)_to_error_result(58-65)_to_tool_result(46-55)strands_tool_wrapper(120-127)tool_spec(87-88)NATFunctionAgentTool(68-116)tool_name(83-84)stream(94-116)tool_type(91-92)
packages/nvidia_nat_strands/tests/test_strands_callback_handler.py (5)
src/nat/builder/context.py (3)
Context(118-316)intermediate_step_manager(172-183)output(59-60)src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-26)src/nat/data_models/intermediate_step.py (5)
IntermediateStepType(42-58)event_type(265-266)UUID(301-302)framework(277-278)data(293-294)packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py (10)
StrandsProfilerHandler(174-489)StrandsToolInstrumentationHook(39-171)on_before_tool_invocation(56-99)on_after_tool_invocation(101-156)instrument(186-244)_extract_model_info(298-329)_extract_text_from_event(445-460)_extract_usage_from_event(462-489)_wrap_stream_method(331-443)_instrument_agent_init(246-296)packages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py (3)
tool_name(83-84)tool_spec(87-88)NATFunctionAgentTool(68-116)
packages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py (3)
src/nat/builder/builder.py (1)
Builder(68-290)src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-26)packages/nvidia_nat_strands/tests/test_strands_tool_wrapper.py (3)
model_json_schema(61-62)tool_spec(136-138)acall_invoke(42-43)
packages/nvidia_nat_strands/tests/test_strands_llm.py (6)
src/nat/builder/builder.py (1)
Builder(68-290)src/nat/data_models/llm.py (1)
APITypeEnum(25-27)src/nat/llm/aws_bedrock_llm.py (1)
AWSBedrockModelConfig(33-63)src/nat/llm/nim_llm.py (1)
NIMModelConfig(34-52)src/nat/llm/openai_llm.py (2)
openai_llm(53-55)OpenAIModelConfig(32-49)packages/nvidia_nat_strands/src/nat/plugins/strands/llm.py (4)
_patch_llm_based_on_config(40-104)bedrock_strands(216-242)nim_strands(135-212)openai_strands(108-131)
examples/frameworks/strands_demo/src/nat_strands_demo/url_directory.py (3)
src/nat/builder/builder.py (1)
Builder(68-290)src/nat/builder/function_info.py (2)
FunctionInfo(290-625)from_fn(552-625)src/nat/data_models/function.py (1)
FunctionBaseConfig(26-27)
examples/frameworks/strands_demo/src/nat_strands_demo/register.py (6)
src/nat/builder/builder.py (1)
Builder(68-290)src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-26)src/nat/builder/function_info.py (2)
FunctionInfo(290-625)from_fn(552-625)src/nat/data_models/component_ref.py (2)
FunctionRef(94-102)LLMRef(116-124)src/nat/data_models/function.py (1)
FunctionBaseConfig(26-27)examples/frameworks/strands_demo/src/nat_strands_demo/url_directory.py (1)
url_directory(58-110)
🪛 Ruff (0.14.2)
packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py
97-97: Unused noqa directive (unused: BLE001)
Remove unused noqa directive
(RUF100)
154-154: Unused noqa directive (unused: BLE001)
Remove unused noqa directive
(RUF100)
242-242: Unused noqa directive (unused: BLE001)
Remove unused noqa directive
(RUF100)
287-287: Unused noqa directive (unused: BLE001)
Remove unused noqa directive
(RUF100)
295-295: Unused noqa directive (unused: BLE001)
Remove unused noqa directive
(RUF100)
326-326: Do not catch blind exception: Exception
(BLE001)
430-430: Do not catch blind exception: Exception
(BLE001)
packages/nvidia_nat_strands/tests/test_strands_tool_wrapper.py
42-42: Unused method argument: kwargs
(ARG002)
62-62: Create your own exception
(TRY002)
62-62: Avoid specifying long messages outside the exception class
(TRY003)
215-215: Unused function argument: kwargs
(ARG001)
253-253: Unused function argument: kwargs
(ARG001)
255-255: Avoid specifying long messages outside the exception class
(TRY003)
packages/nvidia_nat_strands/tests/test_strands_callback_handler.py
177-177: Avoid specifying long messages outside the exception class
(TRY003)
250-250: Unused method argument: args
(ARG002)
250-250: Unused method argument: kwargs
(ARG002)
260-260: Avoid specifying long messages outside the exception class
(TRY003)
263-263: Avoid specifying long messages outside the exception class
(TRY003)
423-423: Unused function argument: model_self
(ARG001)
423-423: Unused function argument: messages
(ARG001)
423-423: Unused function argument: tool_specs
(ARG001)
423-423: Unused function argument: system_prompt
(ARG001)
423-423: Unused function argument: kwargs
(ARG001)
472-472: Unused function argument: model_self
(ARG001)
472-472: Unused function argument: args
(ARG001)
472-472: Unused function argument: kwargs
(ARG001)
474-474: Avoid specifying long messages outside the exception class
(TRY003)
503-503: Unused function argument: model_self
(ARG001)
503-503: Unused function argument: args
(ARG001)
503-503: Unused function argument: kwargs
(ARG001)
531-531: Unused function argument: model_self
(ARG001)
531-531: Unused function argument: messages
(ARG001)
531-531: Unused function argument: tool_specs
(ARG001)
531-531: Unused function argument: system_prompt
(ARG001)
531-531: Unused function argument: kwargs
(ARG001)
580-580: Unused method argument: args
(ARG002)
580-580: Unused method argument: kwargs
(ARG002)
597-597: Consider moving this statement to an else block
(TRY300)
604-604: Avoid specifying long messages outside the exception class
(TRY003)
659-659: Unused method argument: args
(ARG002)
659-659: Unused method argument: kwargs
(ARG002)
675-675: Avoid specifying long messages outside the exception class
(TRY003)
packages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py
40-40: Consider moving this statement to an else block
(TRY300)
47-47: Unused noqa directive (non-enabled: UP038)
Remove unused noqa directive
(RUF100)
114-114: Unused noqa directive (unused: BLE001)
Remove unused noqa directive
(RUF100)
123-123: Avoid specifying long messages outside the exception class
(TRY003)
packages/nvidia_nat_strands/tests/test_strands_llm.py
283-283: Do not assert False (python -O removes these calls), raise AssertionError()
Replace assert False
(B011)
examples/frameworks/strands_demo/src/nat_strands_demo/url_directory.py
51-51: Avoid specifying long messages outside the exception class
(TRY003)
53-53: Avoid specifying long messages outside the exception class
(TRY003)
106-106: Unused noqa directive (unused: BLE001)
Remove unused noqa directive
(RUF100)
examples/frameworks/strands_demo/src/nat_strands_demo/register.py
28-28: Unused noqa directive (non-enabled: F401)
Remove unused noqa directive
(RUF100)
29-29: Unused noqa directive (non-enabled: F401)
Remove unused noqa directive
(RUF100)
88-88: Consider moving this statement to an else block
(TRY300)
⏰ 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 (13)
examples/frameworks/strands_demo/src/nat_strands_demo/register.py (1)
82-91: LGTM! Consider minor simplification.The streaming logic and exception handling are correct. The use of
logger.exception()is appropriate for catching and logging without re-raising.Optionally, you could move line 88 to an
elseblock as suggested by TRY300, but the current form is clear and readable.packages/nvidia_nat_strands/src/nat/plugins/strands/llm.py (4)
40-104: LGTM! Well-structured patching logic.The
_patch_llm_based_on_configfunction properly applies conditional patches based on mixin presence, and the nestedStrandsThinkingInjectorcorrectly handles all system prompt scenarios (positional, keyword, and missing). The sequential patching order (retry → thinking) is appropriate.
107-131: LGTM! Clean OpenAI integration.The function properly validates the API type, sanitizes parameters by excluding NAT-specific keys, creates the OpenAI model with appropriate client args, and applies the common patching logic. The environment variable fallback for
api_keyis appropriate.
134-212: LGTM! NIM-specific compatibility layer is well-implemented.The
NIMCompatibleOpenAIModelclass appropriately handles NIM's requirements:
- Extracts reasoning content from
reasoningContentblocks- Flattens single-item text content arrays to strings
- Joins multiple text blocks into single strings
- Ensures empty content is replaced with a space (NIM rejects empty strings)
The parameter sanitization and patching logic mirrors the OpenAI implementation correctly.
215-242: LGTM! Bedrock integration properly handles provider-specific configuration.The function correctly:
- Validates API type
- Excludes NAT-specific and Bedrock-specific parameters
- Handles the
region_nameedge case (treating "None" string as None)- Applies the common patching logic
packages/nvidia_nat_strands/tests/test_strands_callback_handler.py (1)
418-563: LGTM! Async tests properly decorated.All async test methods in
TestStrandsProfilerHandlerStreamWrappercorrectly have the@pytest.mark.asynciodecorator. The tests comprehensively cover streaming behavior, exception handling, non-async responses, and message formatting.packages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py (2)
94-116: LGTM! Robust streaming implementation with proper error handling.The
streammethod correctly:
- Extracts tool use ID and input with appropriate defaults
- Handles streaming functions by yielding
ToolStreamEventfor each chunk and finalizing withToolResultEvent- Falls back to non-streaming execution with
acall_invokewhen appropriate- Catches and logs all exceptions, yielding error results
The broad exception catch (line 114) is appropriate here since all tool errors should be converted to error results rather than propagating.
119-127: LGTM! Clean factory function with proper validation.The function correctly:
- Validates that the function has an input schema (raises ValueError if missing)
- Converts the Pydantic model to JSON schema
- Uses function description or falls back to name
- Returns a properly initialized
NATFunctionAgentToolThe docstring is clear and complete.
packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py (5)
1-15: LGTM!The Apache 2.0 license header is properly formatted with the correct copyright year.
16-36: LGTM!Imports are properly organized and all necessary dependencies are included.
158-171: LGTM!The helper method properly extracts tool information with sensible fallbacks for missing data.
331-443: LGTM!The streaming wrapper correctly implements the telemetry lifecycle:
- Emits LLM_START before streaming begins
- Collects output text and token usage during streaming
- Ensures LLM_END is always emitted via the finally block
The use of a
finallyblock to guarantee the END span is particularly important for telemetry integrity.
46-54: No changes needed—context caching is intentional and appropriate.The step_manager caching at initialization (line 54) is a deliberate design decision, explicitly documented in the code comment at line 137: "Use stored step_manager to avoid context isolation issues." This approach is sound because:
- One hook instance handles all tool invocations for a single handler
- Tool execution occurs within the same Context established at initialization
- The
_tool_start_timesdict withtool_use_idkeys properly isolates concurrent tool tracking- Tests validate this pattern by mocking
Context.get()during hook instantiationThe design correctly handles the execution model and requires no changes.
examples/frameworks/strands_demo/src/nat_strands_demo/register.py
Outdated
Show resolved
Hide resolved
packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py
Show resolved
Hide resolved
packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py
Outdated
Show resolved
Hide resolved
- Fix tool call extraction logic in StrandsCallbackHandler - Add optimizer configuration example to demonstrate parameter optimization - Update test cases to cover improved tool call handling - Address review comments and code quality improvements Signed-off-by: RanjitR <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/nvidia_nat_strands/tests/test_strands_llm.py (1)
35-48: Align fixtures with project pytest conventions.Please update the fixtures in this file to follow the documented testing guideline: use function names prefixed with
fixture_…and supply thename=argument on@pytest.fixture. That applies to each fixture here (e.g.,mock_builder,openai_config,openai_config_wrong_api, etc.).packages/nvidia_nat_strands/tests/test_strands_callback_handler.py (1)
31-48: Rename fixtures and set explicit names per pytest guideline.The repository testing rules require fixtures to be declared as
def fixture_…()and to passname=on@pytest.fixture. Please update the fixtures starting here (mock_handler,mock_step_manager,tool_hook, etc.) to follow that convention.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
examples/frameworks/strands_demo/src/nat_strands_demo/configs/config.yml(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/configs/eval_config.yml(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/configs/optimizer_config.yml(1 hunks)examples/frameworks/strands_demo/src/nat_strands_demo/register.py(1 hunks)packages/nvidia_nat_strands/pyproject.toml(1 hunks)packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py(1 hunks)packages/nvidia_nat_strands/tests/test_strands_callback_handler.py(1 hunks)packages/nvidia_nat_strands/tests/test_strands_llm.py(1 hunks)tests/nat/profiler/test_callback_handler.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- examples/frameworks/strands_demo/src/nat_strands_demo/configs/config.yml
- packages/nvidia_nat_strands/pyproject.toml
- tests/nat/profiler/test_callback_handler.py
🧰 Additional context used
📓 Path-based instructions (10)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: In code comments use the abbreviations: nat (API namespace/CLI), nvidia-nat (package), NAT (env var prefixes); never use these abbreviations in documentation
Follow PEP 20 and PEP 8 for Python style
Run yapf with column_limit=120; yapf is used for formatting (run second)
Indent with 4 spaces (no tabs) and end each file with a single trailing newline
Use ruff (ruff check --fix) as a linter (not formatter) per pyproject.toml; fix warnings unless explicitly ignored
Respect Python naming schemes: snake_case for functions/variables, PascalCase for classes, UPPER_CASE for constants
When re-raising exceptions, use bare raise to preserve stack trace; log with logger.error(), not logger.exception()
When catching and logging without re-raising, use logger.exception() to capture full stack trace
Provide Google-style docstrings for every public module, class, function, and CLI command
Docstring first line must be a concise description ending with a period
Validate and sanitize all user input, especially in web or CLI interfaces
Prefer httpx with SSL verification enabled by default and follow OWASP Top-10 recommendations
Use async/await for I/O-bound work (HTTP, DB, file I/O)
Cache expensive computations with functools.lru_cache or an external cache when appropriate
Leverage NumPy vectorized operations when beneficial and feasible
Files:
examples/frameworks/strands_demo/src/nat_strands_demo/register.pypackages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.pypackages/nvidia_nat_strands/tests/test_strands_callback_handler.pypackages/nvidia_nat_strands/tests/test_strands_llm.py
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}: Every file must start with the standard SPDX Apache-2.0 header
Confirm copyright years are up to date when a file is changed
All source files must include the SPDX Apache-2.0 header template (copy from an existing file)
Files:
examples/frameworks/strands_demo/src/nat_strands_demo/register.pyexamples/frameworks/strands_demo/src/nat_strands_demo/configs/optimizer_config.ymlpackages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.pypackages/nvidia_nat_strands/tests/test_strands_callback_handler.pyexamples/frameworks/strands_demo/src/nat_strands_demo/configs/eval_config.ymlpackages/nvidia_nat_strands/tests/test_strands_llm.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
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.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.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile 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:
examples/frameworks/strands_demo/src/nat_strands_demo/register.pyexamples/frameworks/strands_demo/src/nat_strands_demo/configs/optimizer_config.ymlpackages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.pypackages/nvidia_nat_strands/tests/test_strands_callback_handler.pyexamples/frameworks/strands_demo/src/nat_strands_demo/configs/eval_config.ymlpackages/nvidia_nat_strands/tests/test_strands_llm.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 apyproject.tomlfile. Optionally, it might also contain scripts in ascripts/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 nameddata/, and should
be checked into git-lfs.
Files:
examples/frameworks/strands_demo/src/nat_strands_demo/register.pyexamples/frameworks/strands_demo/src/nat_strands_demo/configs/optimizer_config.ymlexamples/frameworks/strands_demo/src/nat_strands_demo/configs/eval_config.yml
**/configs/**
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Configuration files consumed by code should be stored next to that code in a configs/ folder
Files:
examples/frameworks/strands_demo/src/nat_strands_demo/configs/optimizer_config.ymlexamples/frameworks/strands_demo/src/nat_strands_demo/configs/eval_config.yml
{src/**/*.py,packages/*/src/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{src/**/*.py,packages/*/src/**/*.py}: All importable Python code must live under src/ or packages//src/
All public APIs must have Python 3.11+ type hints on parameters and return values
Prefer typing/collections.abc abstractions (e.g., Sequence over list)
Use typing.Annotated for units or metadata when useful
Treat pyright warnings as errors during development
Files:
packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py
packages/*/src/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
If a package contains Python code, it must have tests in a tests/ directory at the same level as pyproject.toml
Files:
packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py
packages/**/*
⚙️ CodeRabbit configuration file
packages/**/*: - This directory contains optional plugin packages for the toolkit, each should contain apyproject.tomlfile. - Thepyproject.tomlfile should declare a dependency onnvidia-nator another package with a name starting
withnvidia-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 thepyproject.tomlfile.
Files:
packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.pypackages/nvidia_nat_strands/tests/test_strands_callback_handler.pypackages/nvidia_nat_strands/tests/test_strands_llm.py
{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}: Use pytest with pytest-asyncio for async code
Test functions should be named with test_ prefix using snake_case
Extract frequently repeated test code into pytest fixtures
Pytest fixtures should define the name argument in @pytest.fixture and the function should be prefixed fixture_ using snake_case
Mock external services with pytest_httpserver or unittest.mock; avoid live endpoints
Mark slow tests with @pytest.mark.slow to skip in default test suite
Mark integration tests requiring external services with @pytest.mark.integration and follow nat-tests guidelines
Files:
packages/nvidia_nat_strands/tests/test_strands_callback_handler.pypackages/nvidia_nat_strands/tests/test_strands_llm.py
{tests/test_*.py,examples/*/tests/test_*.py,packages/*/tests/test_*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Name test files as test_*.py and store them in a tests/ folder
Files:
packages/nvidia_nat_strands/tests/test_strands_callback_handler.pypackages/nvidia_nat_strands/tests/test_strands_llm.py
🧠 Learnings (6)
📚 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:
examples/frameworks/strands_demo/src/nat_strands_demo/register.py
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to **/*.py : When re-raising exceptions, use bare raise to preserve stack trace; log with logger.error(), not logger.exception()
Applied to files:
packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to **/*.py : When catching and logging without re-raising, use logger.exception() to capture full stack trace
Applied to files:
packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py} : Use pytest with pytest-asyncio for async code
Applied to files:
packages/nvidia_nat_strands/tests/test_strands_llm.py
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to {tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py} : Mark integration tests requiring external services with pytest.mark.integration and follow nat-tests guidelines
Applied to files:
packages/nvidia_nat_strands/tests/test_strands_llm.py
📚 Learning: 2025-10-22T22:02:12.903Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-10-22T22:02:12.903Z
Learning: Applies to **/*.py : Use async/await for I/O-bound work (HTTP, DB, file I/O)
Applied to files:
packages/nvidia_nat_strands/tests/test_strands_llm.py
🧬 Code graph analysis (4)
examples/frameworks/strands_demo/src/nat_strands_demo/register.py (7)
src/nat/builder/builder.py (1)
Builder(68-290)src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-26)src/nat/builder/function_info.py (2)
FunctionInfo(290-625)from_fn(552-625)src/nat/data_models/component_ref.py (2)
FunctionRef(94-102)LLMRef(116-124)src/nat/data_models/function.py (1)
FunctionBaseConfig(26-27)src/nat/data_models/optimizable.py (1)
OptimizableMixin(199-208)examples/frameworks/strands_demo/src/nat_strands_demo/url_directory.py (1)
url_directory(58-110)
packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py (5)
src/nat/builder/context.py (3)
Context(118-316)intermediate_step_manager(172-183)output(59-60)src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-26)src/nat/data_models/intermediate_step.py (11)
IntermediateStepPayload(130-232)IntermediateStepType(42-58)StreamEventData(67-77)TraceMetadata(114-127)UsageInfo(80-83)event_type(265-266)framework(277-278)UUID(301-302)data(293-294)usage_info(297-298)span_event_timestamp(273-274)src/nat/profiler/callbacks/token_usage_base_model.py (1)
TokenUsageBaseModel(20-29)packages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py (2)
tool_name(83-84)tool_spec(87-88)
packages/nvidia_nat_strands/tests/test_strands_callback_handler.py (5)
src/nat/builder/context.py (3)
Context(118-316)intermediate_step_manager(172-183)output(59-60)src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-26)src/nat/data_models/intermediate_step.py (5)
IntermediateStepType(42-58)event_type(265-266)UUID(301-302)framework(277-278)data(293-294)packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py (13)
StrandsProfilerHandler(178-615)StrandsToolInstrumentationHook(40-175)on_before_tool_invocation(57-100)on_after_tool_invocation(102-157)instrument(188-246)_extract_model_info(302-317)_extract_text_from_event(500-534)_extract_usage_from_event(588-615)_wrap_stream_method(319-498)_instrument_agent_init(248-300)_extract_tool_call_from_event(552-586)_finalize_tool_call(536-550)_extract_tool_info(159-175)packages/nvidia_nat_strands/src/nat/plugins/strands/tool_wrapper.py (3)
tool_name(83-84)tool_spec(87-88)NATFunctionAgentTool(68-116)
packages/nvidia_nat_strands/tests/test_strands_llm.py (7)
src/nat/builder/builder.py (1)
Builder(68-290)src/nat/data_models/llm.py (1)
APITypeEnum(25-27)src/nat/llm/aws_bedrock_llm.py (1)
AWSBedrockModelConfig(33-63)src/nat/llm/nim_llm.py (1)
NIMModelConfig(34-52)src/nat/llm/openai_llm.py (2)
openai_llm(53-55)OpenAIModelConfig(32-49)packages/nvidia_nat_strands/src/nat/plugins/strands/llm.py (4)
_patch_llm_based_on_config(40-104)bedrock_strands(216-242)nim_strands(135-212)openai_strands(108-131)src/nat/data_models/thinking_mixin.py (1)
thinking_system_prompt(49-86)
🪛 Ruff (0.14.3)
examples/frameworks/strands_demo/src/nat_strands_demo/register.py
30-30: Unused noqa directive (non-enabled: F401)
Remove unused noqa directive
(RUF100)
31-31: Unused noqa directive (non-enabled: F401)
Remove unused noqa directive
(RUF100)
96-96: Consider moving this statement to an else block
(TRY300)
packages/nvidia_nat_strands/src/nat/plugins/strands/strands_callback_handler.py
98-98: Unused noqa directive (unused: BLE001)
Remove unused noqa directive
(RUF100)
99-99: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
155-155: Unused noqa directive (unused: BLE001)
Remove unused noqa directive
(RUF100)
156-156: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
244-244: Unused noqa directive (unused: BLE001)
Remove unused noqa directive
(RUF100)
245-245: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
291-291: Unused noqa directive (unused: BLE001)
Remove unused noqa directive
(RUF100)
299-299: Unused noqa directive (unused: BLE001)
Remove unused noqa directive
(RUF100)
packages/nvidia_nat_strands/tests/test_strands_callback_handler.py
175-175: Avoid specifying long messages outside the exception class
(TRY003)
243-243: Unused method argument: args
(ARG002)
243-243: Unused method argument: kwargs
(ARG002)
253-253: Avoid specifying long messages outside the exception class
(TRY003)
256-256: Avoid specifying long messages outside the exception class
(TRY003)
416-416: Unused function argument: model_self
(ARG001)
416-416: Unused function argument: messages
(ARG001)
416-416: Unused function argument: tool_specs
(ARG001)
416-416: Unused function argument: system_prompt
(ARG001)
416-416: Unused function argument: kwargs
(ARG001)
465-465: Unused function argument: model_self
(ARG001)
465-465: Unused function argument: args
(ARG001)
465-465: Unused function argument: kwargs
(ARG001)
467-467: Avoid specifying long messages outside the exception class
(TRY003)
496-496: Unused function argument: model_self
(ARG001)
496-496: Unused function argument: args
(ARG001)
496-496: Unused function argument: kwargs
(ARG001)
524-524: Unused function argument: model_self
(ARG001)
524-524: Unused function argument: messages
(ARG001)
524-524: Unused function argument: tool_specs
(ARG001)
524-524: Unused function argument: system_prompt
(ARG001)
524-524: Unused function argument: kwargs
(ARG001)
579-579: Unused method argument: args
(ARG002)
579-579: Unused method argument: kwargs
(ARG002)
596-596: Consider moving this statement to an else block
(TRY300)
603-603: Avoid specifying long messages outside the exception class
(TRY003)
666-666: Unused method argument: args
(ARG002)
666-666: Unused method argument: kwargs
(ARG002)
682-682: Avoid specifying long messages outside the exception class
(TRY003)
packages/nvidia_nat_strands/tests/test_strands_llm.py
291-291: Avoid specifying long messages outside the exception class
(TRY003)
| from . import ping_tool # noqa: F401, pylint: disable=unused-import | ||
| from . import url_directory # noqa: F401, pylint: disable=unused-import |
There was a problem hiding this comment.
Drop unused # noqa directives to satisfy ruff.
Ruff is emitting RUF100 because # noqa: F401 references a rule that isn’t enabled. Since the Pylint suppression is the only one needed, remove the # noqa portion so lint passes. Based on static analysis hints.
-from . import ping_tool # noqa: F401, pylint: disable=unused-import
-from . import url_directory # noqa: F401, pylint: disable=unused-import
+from . import ping_tool # pylint: disable=unused-import
+from . import url_directory # pylint: disable=unused-import📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from . import ping_tool # noqa: F401, pylint: disable=unused-import | |
| from . import url_directory # noqa: F401, pylint: disable=unused-import | |
| from . import ping_tool # pylint: disable=unused-import | |
| from . import url_directory # pylint: disable=unused-import |
🧰 Tools
🪛 Ruff (0.14.3)
30-30: Unused noqa directive (non-enabled: F401)
Remove unused noqa directive
(RUF100)
31-31: Unused noqa directive (non-enabled: F401)
Remove unused noqa directive
(RUF100)
🤖 Prompt for AI Agents
In examples/frameworks/strands_demo/src/nat_strands_demo/register.py around
lines 30 to 31, the imports use an unnecessary "# noqa: F401" which triggers
RUF100; remove the "# noqa: F401," portion and keep the pylint suppression only
(i.e., change the trailing comment to "pylint: disable=unused-import") so the
unused-import import remains intentionally kept but Ruff no longer flags an
invalid noqa reference.
| self.handler = handler | ||
| self._tool_start_times: dict[str, float] = {} | ||
| self._step_manager = Context.get().intermediate_step_manager | ||
|
|
||
| def on_before_tool_invocation(self, event: Any) -> None: | ||
| """Handle tool invocation start. | ||
|
|
||
| Called by Strands before a tool is executed. | ||
| Emits a TOOL_START span. | ||
|
|
||
| Args: | ||
| event: BeforeToolInvocationEvent from Strands | ||
| """ | ||
| try: | ||
| tool_use = event.tool_use | ||
| selected_tool = event.selected_tool | ||
|
|
||
| if not selected_tool: | ||
| logger.debug("Tool hook: no selected_tool, skipping") | ||
| return | ||
|
|
||
| # Extract tool information | ||
| tool_name, tool_use_id, tool_input = self._extract_tool_info(selected_tool, tool_use) | ||
|
|
||
| # Store start time for duration calculation | ||
| self._tool_start_times[tool_use_id] = time.time() | ||
|
|
||
| step_manager = self._step_manager | ||
|
|
||
| start_payload = IntermediateStepPayload( | ||
| event_type=IntermediateStepType.TOOL_START, | ||
| framework=LLMFrameworkEnum.STRANDS, | ||
| name=tool_name, | ||
| UUID=tool_use_id, | ||
| data=StreamEventData(input=str(tool_input), output=""), | ||
| metadata=TraceMetadata( | ||
| tool_inputs=copy.deepcopy(tool_input), | ||
| tool_info=copy.deepcopy(getattr(selected_tool, 'tool_spec', {})), | ||
| ), | ||
| usage_info=UsageInfo(token_usage=TokenUsageBaseModel()), | ||
| ) | ||
|
|
||
| step_manager.push_intermediate_step(start_payload) | ||
|
|
||
| logger.debug("TOOL_START: %s (ID: %s)", tool_name, tool_use_id) | ||
| except Exception: # noqa: BLE001 | ||
| logger.error("Error in before_tool_invocation") | ||
| raise | ||
|
|
There was a problem hiding this comment.
Do not capture the step manager at hook construction time.
Context.get() is invoked in __init__, and its intermediate_step_manager is cached on the hook. When the agent is instantiated outside an active NAT context (which is the common case—hooks are registered during Agent.__init__), Context.get() raises, the hook constructor blows up, and wrapped_init silently logs “Failed to auto-register hooks.” Tool spans never fire. Even when it succeeds, the cached manager belongs to the context present at construction, so subsequent runs under a different context push telemetry into the wrong stream.
Please grab the step manager lazily inside on_before_tool_invocation / on_after_tool_invocation, guard the lookup (skip instrumentation if no context is active), and drop the cached _step_manager field. A small helper such as:
def _current_step_manager() -> IntermediateStepManager | None:
try:
return Context.get().intermediate_step_manager
except LookupError:
logger.debug("No active NAT context; skipping Strands tool instrumentation")
return Nonewould let each callback resolve the correct manager (and avoid fatal errors). This also protects _wrap_stream_method, which should similarly fall back to original if the context lookup fails.
🧰 Tools
🪛 Ruff (0.14.3)
98-98: Unused noqa directive (unused: BLE001)
Remove unused noqa directive
(RUF100)
99-99: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
2 participants
Description
This PR adds AWS Strands framework integration, including a new nvidia_nat_strands package with LLM wrapper, tool adapter, and callback handler for observability.
Includes a complete strands_demo example with custom tools, multiple configuration options, and test coverage.
By Submitting this PR I confirm:
Summary by CodeRabbit
New Features
Documentation
Tests
Chores