feat: Add AutoMemoryWrapper agent for automatic memory management #1137
Conversation
WalkthroughAdds an Automatic Memory Wrapper for NAT agents (state, graph, register, package export), example README and Zep YAML config, unit tests, and widened Zep backend error handling to treat certain "already exists" responses as idempotent. No public API breaking changes detected. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Wrapper as AutoMemoryWrapper
participant Memory as MemoryEditor
participant Agent as InnerAgent
User->>Wrapper: incoming ChatRequest
rect rgb(230,245,255)
Note over Wrapper: Capture user message (optional)
Wrapper->>Memory: add_items(user_message, user_id, add_params)
Memory-->>Wrapper: add_result
end
rect rgb(245,230,255)
Note over Wrapper: Retrieve memory (optional)
Wrapper->>Memory: search(query, user_id, search_params)
Memory-->>Wrapper: matched_memory
Wrapper->>Wrapper: inject system memory message
end
rect rgb(255,245,230)
Note over Wrapper: Invoke inner agent
Wrapper->>Agent: ChatRequest (messages + injected memory)
Agent-->>Wrapper: response (various shapes)
end
rect rgb(230,255,235)
Note over Wrapper: Capture AI response (optional)
Wrapper->>Memory: add_items(ai_response, user_id, add_params)
Memory-->>Wrapper: add_result
end
Wrapper->>User: final_response_text
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
jackaldenryan
force-pushed
the
auto-memory-wrapper
branch
2 times, most recently
from
949d263 to
4a42c89
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.py (2)
77-85: Consider defensive attribute access for robustness.The error handling correctly addresses concurrent user creation scenarios. However, the direct access to
e.status_codecould raiseAttributeErrorif the attribute is missing. Consider usinggetattr(e, 'status_code', None)for safer access.Additionally, the string matching
"already exists" in str(e).lower()is somewhat fragile. If possible, check whether the Zep Cloud SDK provides a more structured way to inspect error details (e.g., an error code or body attribute).Example refactor:
- except ApiError as e: - # Check if user was created by another request (409 Conflict or 400 with "already exists") - if e.status_code == 409: - logger.info("Zep user already exists - 409, continuing") - elif e.status_code == 400 and "already exists" in str(e).lower(): - logger.info("Zep user already exists - 400, continuing") - else: - logger.error("Failed creating Zep user: %s", str(e)) # noqa: TRY400 - raise + except ApiError as e: + # Check if user was created by another request (409 Conflict or 400 with "already exists") + status = getattr(e, 'status_code', None) + if status == 409: + logger.info("Zep user already exists - 409, continuing") + elif status == 400 and "already exists" in str(e).lower(): + logger.info("Zep user already exists - 400, continuing") + else: + logger.error("Failed creating Zep user: %s", str(e)) # noqa: TRY400 + raise
147-157: Apply the same defensive pattern for consistent error handling.The error handling correctly manages concurrent thread creation in a batch context. The use of
logger.exception()without re-raising (line 156) is appropriate per coding guidelines, as the function continues processing other items.Similar to the user creation logic, consider using
getattr(e, 'status_code', None)for safer attribute access.Example refactor:
except ApiError as create_error: # Check for both 409 (Conflict) and 400 (Bad Request) with "already exists" message - if create_error.status_code == 409: + status = getattr(create_error, 'status_code', None) + if status == 409: logger.info("Zep thread already exists - 409 (thread_id=%s)", thread_id) created_threads.add(thread_id) - elif create_error.status_code == 400 and "already exists" in str(create_error).lower(): + elif status == 400 and "already exists" in str(create_error).lower(): logger.info("Zep thread already exists - 400 (thread_id=%s)", thread_id) created_threads.add(thread_id)tests/nat/agent/test_auto_memory_wrapper.py (1)
288-301: Minor: Unused result variable.The
resultvariable at line 293 is assigned but never used in assertions. Either add assertions onresultor remove the assignment.state = AutoMemoryWrapperState( messages=[SystemMessage(content="Memory context"), HumanMessage(content="User query")]) - result = await wrapper_graph.inner_agent_node(state) + await wrapper_graph.inner_agent_node(state) # Verify inner agent received both messages
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
examples/agents/auto_memory_wrapper/README.md(1 hunks)examples/agents/auto_memory_wrapper/configs/config_zep.yml(1 hunks)packages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.py(3 hunks)src/nat/agent/auto_memory_wrapper/__init__.py(1 hunks)src/nat/agent/auto_memory_wrapper/agent.py(1 hunks)src/nat/agent/auto_memory_wrapper/register.py(1 hunks)src/nat/agent/auto_memory_wrapper/state.py(1 hunks)src/nat/agent/register.py(1 hunks)tests/nat/agent/test_auto_memory_wrapper.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
{docs/source/**/*.md,**/README.@(md|ipynb)}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{docs/source/**/*.md,**/README.@(md|ipynb)}: Use the full name “NVIDIA NeMo Agent toolkit” on first use in documentation, then “NeMo Agent toolkit”; in headings use “NeMo Agent Toolkit” (capital T)
Do not use deprecated names (Agent Intelligence toolkit, aiqtoolkit, AgentIQ, AIQ/aiq) in documentation unless explicitly referring to deprecated names
Never use “NAT”/“nat” abbreviations in documentation
Documentation must be clear/comprehensive; avoid TODOs/FIXMEs/placeholders; avoid offensive/outdated terms; ensure spelling is correct
Files:
examples/agents/auto_memory_wrapper/README.md
{**/*.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/agents/auto_memory_wrapper/README.mdsrc/nat/agent/auto_memory_wrapper/register.pysrc/nat/agent/auto_memory_wrapper/state.pysrc/nat/agent/auto_memory_wrapper/agent.pyexamples/agents/auto_memory_wrapper/configs/config_zep.ymlsrc/nat/agent/register.pypackages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.pysrc/nat/agent/auto_memory_wrapper/__init__.pytests/nat/agent/test_auto_memory_wrapper.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/agents/auto_memory_wrapper/README.mdsrc/nat/agent/auto_memory_wrapper/register.pysrc/nat/agent/auto_memory_wrapper/state.pysrc/nat/agent/auto_memory_wrapper/agent.pyexamples/agents/auto_memory_wrapper/configs/config_zep.ymlsrc/nat/agent/register.pypackages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.pysrc/nat/agent/auto_memory_wrapper/__init__.pytests/nat/agent/test_auto_memory_wrapper.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/agents/auto_memory_wrapper/README.mdexamples/agents/auto_memory_wrapper/configs/config_zep.yml
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: In code comments use the abbreviations: nat (API namespace/CLI), nvidia-nat (package), NAT (env var prefixes); never use these abbreviations in documentation
Follow PEP 20 and PEP 8 for Python style
Run yapf with column_limit=120; yapf is used for formatting (run second)
Indent with 4 spaces (no tabs) and end each file with a single trailing newline
Use ruff (ruff check --fix) as a linter (not formatter) per pyproject.toml; fix warnings unless explicitly ignored
Respect Python naming schemes: snake_case for functions/variables, PascalCase for classes, UPPER_CASE for constants
When re-raising exceptions, use bare raise to preserve stack trace; log with logger.error(), not logger.exception()
When catching and logging without re-raising, use logger.exception() to capture full stack trace
Provide Google-style docstrings for every public module, class, function, and CLI command
Docstring first line must be a concise description ending with a period
Validate and sanitize all user input, especially in web or CLI interfaces
Prefer httpx with SSL verification enabled by default and follow OWASP Top-10 recommendations
Use async/await for I/O-bound work (HTTP, DB, file I/O)
Cache expensive computations with functools.lru_cache or an external cache when appropriate
Leverage NumPy vectorized operations when beneficial and feasible
Files:
src/nat/agent/auto_memory_wrapper/register.pysrc/nat/agent/auto_memory_wrapper/state.pysrc/nat/agent/auto_memory_wrapper/agent.pysrc/nat/agent/register.pypackages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.pysrc/nat/agent/auto_memory_wrapper/__init__.pytests/nat/agent/test_auto_memory_wrapper.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/agent/auto_memory_wrapper/register.pysrc/nat/agent/auto_memory_wrapper/state.pysrc/nat/agent/auto_memory_wrapper/agent.pysrc/nat/agent/register.pypackages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.pysrc/nat/agent/auto_memory_wrapper/__init__.py
src/nat/**/*
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
src/nat/**/* contains core functionality; changes should prioritize backward compatibility
Files:
src/nat/agent/auto_memory_wrapper/register.pysrc/nat/agent/auto_memory_wrapper/state.pysrc/nat/agent/auto_memory_wrapper/agent.pysrc/nat/agent/register.pysrc/nat/agent/auto_memory_wrapper/__init__.py
⚙️ CodeRabbit configuration file
This directory contains the core functionality of the toolkit. Changes should prioritize backward compatibility.
Files:
src/nat/agent/auto_memory_wrapper/register.pysrc/nat/agent/auto_memory_wrapper/state.pysrc/nat/agent/auto_memory_wrapper/agent.pysrc/nat/agent/register.pysrc/nat/agent/auto_memory_wrapper/__init__.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/agents/auto_memory_wrapper/configs/config_zep.yml
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_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.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_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.py
{tests/**/*.py,examples/*/tests/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Unit tests live in tests/ or examples/*/tests and use markers defined in pyproject.toml (e.g., integration)
Files:
tests/nat/agent/test_auto_memory_wrapper.py
{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}: Use pytest with pytest-asyncio for async code
Test functions should be named with test_ prefix using snake_case
Extract frequently repeated test code into pytest fixtures
Pytest fixtures should define the name argument in @pytest.fixture and the function should be prefixed fixture_ using snake_case
Mock external services with pytest_httpserver or unittest.mock; avoid live endpoints
Mark slow tests with @pytest.mark.slow to skip in default test suite
Mark integration tests requiring external services with @pytest.mark.integration and follow nat-tests guidelines
Files:
tests/nat/agent/test_auto_memory_wrapper.py
tests/**/*.py
⚙️ CodeRabbit configuration file
tests/**/*.py: - Ensure that tests are comprehensive, cover edge cases, and validate the functionality of the code. - Test functions should be named using 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/agent/test_auto_memory_wrapper.py
🧬 Code graph analysis (3)
src/nat/agent/auto_memory_wrapper/register.py (7)
src/nat/builder/builder.py (1)
Builder(68-290)src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-25)src/nat/data_models/agent.py (1)
AgentBaseConfig(23-34)src/nat/data_models/component_ref.py (2)
FunctionRef(94-102)MemoryRef(127-135)src/nat/agent/auto_memory_wrapper/agent.py (3)
AutoMemoryWrapperGraph(40-278)get_wrapper_node_count(104-121)build_graph(247-278)src/nat/agent/auto_memory_wrapper/state.py (1)
AutoMemoryWrapperState(21-30)src/nat/builder/function_info.py (2)
FunctionInfo(290-625)from_fn(552-625)
src/nat/agent/auto_memory_wrapper/agent.py (4)
src/nat/builder/context.py (3)
Context(118-316)user_manager(138-149)output(59-60)src/nat/data_models/api_server.py (2)
ChatRequest(125-195)UserMessageContentRoleType(41-47)src/nat/agent/auto_memory_wrapper/state.py (1)
AutoMemoryWrapperState(21-30)packages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.py (2)
add_items(90-173)search(175-233)
tests/nat/agent/test_auto_memory_wrapper.py (5)
src/nat/agent/auto_memory_wrapper/agent.py (9)
AutoMemoryWrapperGraph(40-278)get_wrapper_node_count(104-121)_get_user_id_from_context(70-102)_langchain_message_to_nat_message(124-141)capture_user_message_node(143-160)memory_retrieve_node(162-190)inner_agent_node(192-226)capture_ai_response_node(228-245)build_graph(247-278)src/nat/agent/auto_memory_wrapper/state.py (1)
AutoMemoryWrapperState(21-30)src/nat/builder/context.py (2)
Context(118-316)user_manager(138-149)src/nat/data_models/api_server.py (6)
ChatRequest(125-195)ChatResponse(338-388)ChatResponseChoice(295-297)ChoiceMessage(277-279)Usage(309-312)UserMessageContentRoleType(41-47)packages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.py (2)
add_items(90-173)search(175-233)
🪛 Ruff (0.14.2)
src/nat/agent/auto_memory_wrapper/register.py
148-151: Avoid specifying long messages outside the exception class
(TRY003)
src/nat/agent/auto_memory_wrapper/agent.py
90-90: Do not catch blind exception: Exception
(BLE001)
tests/nat/agent/test_auto_memory_wrapper.py
53-53: Unused function argument: chat_request
(ARG001)
293-293: Local variable result is assigned to but never used
Remove assignment to unused variable result
(F841)
🔇 Additional comments (22)
packages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.py (1)
23-23: Import path updated appropriately.The more specific import path from
zep_cloud.core.api_erroris good practice and aligns with the API changes.examples/agents/auto_memory_wrapper/README.md (1)
39-197: Well-structured documentation with comprehensive examples.The configuration reference, multi-tenant isolation guidance, and examples are thorough and helpful. The priority-based user ID extraction explanation is particularly clear.
src/nat/agent/register.py (1)
19-19: LGTM!The import follows the established pattern for agent registration, consistently exposing the new auto_memory_wrapper workflow alongside existing agent types.
src/nat/agent/auto_memory_wrapper/__init__.py (1)
1-23: LGTM!Clean module initialization with proper header, docstring, and explicit
__all__export. Follows Python packaging best practices.src/nat/agent/auto_memory_wrapper/state.py (1)
21-30: LGTM!Clean Pydantic model with appropriate use of
Fieldanddefault_factory. The docstring clearly explains the separation of concerns between wrapper state and inner agent state.examples/agents/auto_memory_wrapper/configs/config_zep.yml (1)
1-80: LGTM!Well-structured configuration with comprehensive inline documentation. The commented examples for optional parameters (search_params, add_params) provide clear guidance without cluttering the default configuration.
tests/nat/agent/test_auto_memory_wrapper.py (3)
39-87: Well-designed test fixtures.The fixtures provide clean mocks for memory editor, inner agent, and context, enabling comprehensive isolated testing of the wrapper components.
90-188: Thorough state and graph initialization tests.Good coverage of state initialization, graph configuration with different feature flags, node counting, user ID extraction from multiple sources, and message type conversions.
190-382: Comprehensive async node and integration tests.Excellent coverage of all node behaviors (capture_user_message, memory_retrieve, inner_agent, capture_ai_response) with enabled/disabled states, edge cases, and parameter propagation validation.
src/nat/agent/auto_memory_wrapper/register.py (5)
31-117: Excellent configuration model with comprehensive documentation.The
AutoMemoryAgentConfigclass provides well-structured configuration options with detailed docstrings, clear field descriptions, and practical YAML examples. The feature flags and backend-specific parameters offer good flexibility.
144-151: Detailed error message enhances user experience.The comprehensive error message clearly explains the requirement and guides users to fix the configuration. While the static analysis tool flags the long message, this level of detail is valuable for user-facing validation errors.
153-172: Well-reasoned recursion limit calculation.The logic correctly derives
inner_max_callsby checking multiple agent-specific attributes (max_tool_calls, max_iterations, tool_call_max_retries) with appropriate defaults. The comments clearly explain the rationale.
174-191: Transparent recursion budget accounting.The wrapper correctly calculates the total recursion limit by adding wrapper overhead to inner agent needs, with clear debug logging. This ensures the wrapper remains transparent to users configuring only the inner agent.
193-219: Robust response function with proper error handling.The
_response_fncorrectly initializes state, invokes the graph with the calculated recursion limit, and extracts the final response. Exception handling with optional verbose mode is appropriate.src/nat/agent/auto_memory_wrapper/agent.py (8)
50-68: Clean initialization with configurable behavior.The constructor properly stores dependencies and configuration, with sensible defaults for optional parameters (
search_paramsandadd_paramsdefault to empty dicts).
70-102: Robust priority-based user ID extraction.The three-tier fallback (user_manager → X-User-ID header → default_user) provides flexibility for production, testing, and development scenarios. The broad exception catch at line 90 is intentional and appropriate for graceful fallback.
104-141: Useful helper methods.
get_wrapper_node_count()enables transparent recursion limit calculation, and_langchain_message_to_nat_message()correctly maps message types with a safe default for unknown types.
143-160: Correct user message capture.The node properly checks the feature flag, validates message type, extracts user ID, and passes backend-specific parameters to
add_items().
162-190: Memory retrieval with proper context injection.The node correctly retrieves memories using the user message as query, extracts memory strings, and inserts a SystemMessage before the last user message to provide context without disrupting the conversation flow.
192-226: Flexible inner agent invocation.The node builds a proper ChatRequest from all messages and handles multiple response shapes (ChatResponse, GenerateResponse, value field, string) with appropriate fallbacks. This ensures compatibility with various agent types.
228-245: AI response capture mirrors user message handling.Consistent implementation with proper flag checking, type validation, user ID extraction, and parameter passing.
247-278: Graph construction adapts to configuration.The
build_graph()method correctly adds nodes conditionally based on feature flags and wires edges appropriately. The entry and finish point logic ensures proper flow regardless of which features are enabled.
|
@willkill07 here is that auto memory wrapper PR, separated out from other changes |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/nat/agent/auto_memory_wrapper/register.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: In code comments use the abbreviations: nat (API namespace/CLI), nvidia-nat (package), NAT (env var prefixes); never use these abbreviations in documentation
Follow PEP 20 and PEP 8 for Python style
Run yapf with column_limit=120; yapf is used for formatting (run second)
Indent with 4 spaces (no tabs) and end each file with a single trailing newline
Use ruff (ruff check --fix) as a linter (not formatter) per pyproject.toml; fix warnings unless explicitly ignored
Respect Python naming schemes: snake_case for functions/variables, PascalCase for classes, UPPER_CASE for constants
When re-raising exceptions, use bare raise to preserve stack trace; log with logger.error(), not logger.exception()
When catching and logging without re-raising, use logger.exception() to capture full stack trace
Provide Google-style docstrings for every public module, class, function, and CLI command
Docstring first line must be a concise description ending with a period
Validate and sanitize all user input, especially in web or CLI interfaces
Prefer httpx with SSL verification enabled by default and follow OWASP Top-10 recommendations
Use async/await for I/O-bound work (HTTP, DB, file I/O)
Cache expensive computations with functools.lru_cache or an external cache when appropriate
Leverage NumPy vectorized operations when beneficial and feasible
Files:
src/nat/agent/auto_memory_wrapper/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/agent/auto_memory_wrapper/register.py
src/nat/**/*
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
src/nat/**/* contains core functionality; changes should prioritize backward compatibility
Files:
src/nat/agent/auto_memory_wrapper/register.py
⚙️ CodeRabbit configuration file
This directory contains the core functionality of the toolkit. Changes should prioritize backward compatibility.
Files:
src/nat/agent/auto_memory_wrapper/register.py
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}: Every file must start with the standard SPDX Apache-2.0 header
Confirm copyright years are up to date when a file is changed
All source files must include the SPDX Apache-2.0 header template (copy from an existing file)
Files:
src/nat/agent/auto_memory_wrapper/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/agent/auto_memory_wrapper/register.py
🧬 Code graph analysis (1)
src/nat/agent/auto_memory_wrapper/register.py (6)
src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-25)src/nat/data_models/agent.py (1)
AgentBaseConfig(23-34)src/nat/data_models/component_ref.py (2)
FunctionRef(94-102)MemoryRef(127-135)src/nat/agent/auto_memory_wrapper/agent.py (3)
AutoMemoryWrapperGraph(40-278)get_wrapper_node_count(104-121)build_graph(247-278)src/nat/agent/auto_memory_wrapper/state.py (1)
AutoMemoryWrapperState(21-30)src/nat/builder/function_info.py (2)
FunctionInfo(290-625)from_fn(552-625)
🪛 Ruff (0.14.3)
src/nat/agent/auto_memory_wrapper/register.py
142-142: Undefined name inner_agent_config
(F821)
144-144: Undefined name inner_agent_config
(F821)
145-145: Undefined name inner_agent_config
(F821)
147-147: Undefined name inner_agent_config
(F821)
148-148: Undefined name inner_agent_config
(F821)
150-150: Undefined name inner_agent_config
(F821)
🔇 Additional comments (4)
src/nat/agent/auto_memory_wrapper/register.py (4)
1-28: LGTM!License header, copyright year, and imports are all correct and follow the coding guidelines.
31-73: Excellent documentation!The class docstring is comprehensive and provides clear guidance on when to use automatic memory vs. tool-based memory. The YAML example effectively demonstrates integration with other NAT components.
75-111: LGTM!Field definitions are well-structured with appropriate types, sensible defaults, and comprehensive documentation. The search_params and add_params descriptions provide helpful guidance for users working with different memory backends.
179-211: LGTM!The
_response_fnimplementation and cleanup logic follow NAT best practices:
- Proper exception handling with
logger.exception()when catching without re-raising- Appropriate
GeneratorExithandling for generator cleanup- Clean finally block for resource cleanup
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
examples/agents/auto_memory_wrapper/README.md(1 hunks)examples/agents/auto_memory_wrapper/configs/config_zep.yml(1 hunks)src/nat/agent/auto_memory_wrapper/register.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- examples/agents/auto_memory_wrapper/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/agents/auto_memory_wrapper/configs/config_zep.yml
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: In code comments use the abbreviations: nat (API namespace/CLI), nvidia-nat (package), NAT (env var prefixes); never use these abbreviations in documentation
Follow PEP 20 and PEP 8 for Python style
Run yapf with column_limit=120; yapf is used for formatting (run second)
Indent with 4 spaces (no tabs) and end each file with a single trailing newline
Use ruff (ruff check --fix) as a linter (not formatter) per pyproject.toml; fix warnings unless explicitly ignored
Respect Python naming schemes: snake_case for functions/variables, PascalCase for classes, UPPER_CASE for constants
When re-raising exceptions, use bare raise to preserve stack trace; log with logger.error(), not logger.exception()
When catching and logging without re-raising, use logger.exception() to capture full stack trace
Provide Google-style docstrings for every public module, class, function, and CLI command
Docstring first line must be a concise description ending with a period
Validate and sanitize all user input, especially in web or CLI interfaces
Prefer httpx with SSL verification enabled by default and follow OWASP Top-10 recommendations
Use async/await for I/O-bound work (HTTP, DB, file I/O)
Cache expensive computations with functools.lru_cache or an external cache when appropriate
Leverage NumPy vectorized operations when beneficial and feasible
Files:
src/nat/agent/auto_memory_wrapper/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/agent/auto_memory_wrapper/register.py
src/nat/**/*
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
src/nat/**/* contains core functionality; changes should prioritize backward compatibility
Files:
src/nat/agent/auto_memory_wrapper/register.py
⚙️ CodeRabbit configuration file
This directory contains the core functionality of the toolkit. Changes should prioritize backward compatibility.
Files:
src/nat/agent/auto_memory_wrapper/register.py
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}: Every file must start with the standard SPDX Apache-2.0 header
Confirm copyright years are up to date when a file is changed
All source files must include the SPDX Apache-2.0 header template (copy from an existing file)
Files:
src/nat/agent/auto_memory_wrapper/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/agent/auto_memory_wrapper/register.py
🧬 Code graph analysis (1)
src/nat/agent/auto_memory_wrapper/register.py (7)
src/nat/builder/builder.py (1)
Builder(68-290)src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-25)src/nat/data_models/agent.py (1)
AgentBaseConfig(23-34)src/nat/data_models/component_ref.py (2)
FunctionRef(94-102)MemoryRef(127-135)src/nat/agent/auto_memory_wrapper/agent.py (3)
AutoMemoryWrapperGraph(40-278)get_wrapper_node_count(104-121)build_graph(247-278)src/nat/agent/auto_memory_wrapper/state.py (1)
AutoMemoryWrapperState(21-30)src/nat/builder/function_info.py (2)
FunctionInfo(290-625)from_fn(552-625)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/nat/agent/auto_memory_wrapper/register.py (1)
222-223: Avoid logger.exception() for normal cleanup signals.
GeneratorExitis not an error condition—it's a normal signal that the generator is being closed. Usinglogger.exception()here logs unnecessary stack traces for expected cleanup behavior, creating noise in logs.Apply this diff to use an appropriate log level:
except GeneratorExit: - logger.exception("%s Workflow exited early!", AGENT_LOG_PREFIX) + logger.info("%s Workflow exited early!", AGENT_LOG_PREFIX)Alternatively, since line 225 already logs cleanup at debug level, you could remove the
except GeneratorExitblock entirely if early exit doesn't require special handling.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/nat/agent/auto_memory_wrapper/register.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: In code comments use the abbreviations: nat (API namespace/CLI), nvidia-nat (package), NAT (env var prefixes); never use these abbreviations in documentation
Follow PEP 20 and PEP 8 for Python style
Run yapf with column_limit=120; yapf is used for formatting (run second)
Indent with 4 spaces (no tabs) and end each file with a single trailing newline
Use ruff (ruff check --fix) as a linter (not formatter) per pyproject.toml; fix warnings unless explicitly ignored
Respect Python naming schemes: snake_case for functions/variables, PascalCase for classes, UPPER_CASE for constants
When re-raising exceptions, use bare raise to preserve stack trace; log with logger.error(), not logger.exception()
When catching and logging without re-raising, use logger.exception() to capture full stack trace
Provide Google-style docstrings for every public module, class, function, and CLI command
Docstring first line must be a concise description ending with a period
Validate and sanitize all user input, especially in web or CLI interfaces
Prefer httpx with SSL verification enabled by default and follow OWASP Top-10 recommendations
Use async/await for I/O-bound work (HTTP, DB, file I/O)
Cache expensive computations with functools.lru_cache or an external cache when appropriate
Leverage NumPy vectorized operations when beneficial and feasible
Files:
src/nat/agent/auto_memory_wrapper/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/agent/auto_memory_wrapper/register.py
src/nat/**/*
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
src/nat/**/* contains core functionality; changes should prioritize backward compatibility
Files:
src/nat/agent/auto_memory_wrapper/register.py
⚙️ CodeRabbit configuration file
This directory contains the core functionality of the toolkit. Changes should prioritize backward compatibility.
Files:
src/nat/agent/auto_memory_wrapper/register.py
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}: Every file must start with the standard SPDX Apache-2.0 header
Confirm copyright years are up to date when a file is changed
All source files must include the SPDX Apache-2.0 header template (copy from an existing file)
Files:
src/nat/agent/auto_memory_wrapper/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/agent/auto_memory_wrapper/register.py
🧬 Code graph analysis (1)
src/nat/agent/auto_memory_wrapper/register.py (7)
src/nat/builder/builder.py (1)
Builder(68-290)src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-25)src/nat/data_models/agent.py (1)
AgentBaseConfig(23-34)src/nat/data_models/component_ref.py (2)
FunctionRef(94-102)MemoryRef(127-135)src/nat/agent/auto_memory_wrapper/agent.py (3)
AutoMemoryWrapperGraph(40-278)get_wrapper_node_count(104-121)build_graph(247-278)src/nat/agent/auto_memory_wrapper/state.py (1)
AutoMemoryWrapperState(21-30)src/nat/builder/function_info.py (2)
FunctionInfo(290-625)from_fn(552-625)
🔇 Additional comments (4)
src/nat/agent/auto_memory_wrapper/register.py (4)
1-28: LGTM: License header and imports are correct.The Apache 2.0 license header is properly formatted with current copyright years, and all imports are appropriate for the module's functionality.
31-112: LGTM: Well-designed configuration model.The
AutoMemoryAgentConfigclass is properly structured with clear field descriptions, comprehensive docstring including usage examples, and sensible defaults. The multi-tenant user isolation documentation is particularly helpful.
139-172: LGTM: Recursion limit calculation is now robust.The recursion limit calculation correctly addresses previous review concerns by:
- Retrieving
inner_agent_configfrom the builder (line 140)- Validating attribute values are not
Noneand are numeric before use (lines 150, 156, 162)- Providing a safe default when no recognized limits are found (line 167)
- Converting to int before arithmetic to prevent type errors (line 172)
174-191: LGTM: Wrapper initialization and recursion calculation are correct.The wrapper graph is properly configured with all necessary parameters, and the total recursion limit correctly accounts for both wrapper overhead and inner agent requirements. The debug logging is helpful for troubleshooting.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/nat/agent/auto_memory_wrapper/register.py (1)
215-219: Critical: Information disclosure in non-verbose error response.Line 219 still exposes exception details even when
config.verboseis False. This can leak sensitive information such as database connection strings, API keys, file paths, or internal state. This issue was flagged in a previous review and remains unaddressed.Apply this diff to properly restrict information disclosure:
except Exception as ex: logger.exception(f"{AGENT_LOG_PREFIX} Auto-memory agent failed with exception") if config.verbose: return str(ex) - return f"Auto-memory agent failed with exception: {ex}" + return "Auto-memory agent encountered an error processing the request."
🧹 Nitpick comments (2)
src/nat/agent/auto_memory_wrapper/register.py (2)
212-212: Consider defensive check for empty messages list.While the workflow guarantees at least one message (initialized on line 206), adding a defensive check would make the code more robust against unexpected graph behavior.
- output_message = result_state.messages[-1] + if not result_state.messages: + raise RuntimeError("Graph returned empty messages list") + output_message = result_state.messages[-1]
223-224: Consider using logger.debug or logger.info for GeneratorExit.
GeneratorExitis not an error condition but a normal control flow signal when the generator is closed. Usinglogger.exception()will log a stack trace for this non-error case, which may clutter logs. While the current usage follows the coding guideline for non-re-raised exceptions,logger.debug()orlogger.info()might be more appropriate here.except GeneratorExit: - logger.exception("%s Workflow exited early!", AGENT_LOG_PREFIX) + logger.debug("%s Workflow generator closed early.", AGENT_LOG_PREFIX)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/nat/agent/auto_memory_wrapper/register.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: In code comments use the abbreviations: nat (API namespace/CLI), nvidia-nat (package), NAT (env var prefixes); never use these abbreviations in documentation
Follow PEP 20 and PEP 8 for Python style
Run yapf with column_limit=120; yapf is used for formatting (run second)
Indent with 4 spaces (no tabs) and end each file with a single trailing newline
Use ruff (ruff check --fix) as a linter (not formatter) per pyproject.toml; fix warnings unless explicitly ignored
Respect Python naming schemes: snake_case for functions/variables, PascalCase for classes, UPPER_CASE for constants
When re-raising exceptions, use bare raise to preserve stack trace; log with logger.error(), not logger.exception()
When catching and logging without re-raising, use logger.exception() to capture full stack trace
Provide Google-style docstrings for every public module, class, function, and CLI command
Docstring first line must be a concise description ending with a period
Validate and sanitize all user input, especially in web or CLI interfaces
Prefer httpx with SSL verification enabled by default and follow OWASP Top-10 recommendations
Use async/await for I/O-bound work (HTTP, DB, file I/O)
Cache expensive computations with functools.lru_cache or an external cache when appropriate
Leverage NumPy vectorized operations when beneficial and feasible
Files:
src/nat/agent/auto_memory_wrapper/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/agent/auto_memory_wrapper/register.py
src/nat/**/*
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
src/nat/**/* contains core functionality; changes should prioritize backward compatibility
Files:
src/nat/agent/auto_memory_wrapper/register.py
⚙️ CodeRabbit configuration file
This directory contains the core functionality of the toolkit. Changes should prioritize backward compatibility.
Files:
src/nat/agent/auto_memory_wrapper/register.py
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}: Every file must start with the standard SPDX Apache-2.0 header
Confirm copyright years are up to date when a file is changed
All source files must include the SPDX Apache-2.0 header template (copy from an existing file)
Files:
src/nat/agent/auto_memory_wrapper/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/agent/auto_memory_wrapper/register.py
🧬 Code graph analysis (1)
src/nat/agent/auto_memory_wrapper/register.py (6)
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/agent.py (1)
AgentBaseConfig(23-34)src/nat/data_models/component_ref.py (2)
FunctionRef(94-102)MemoryRef(127-135)src/nat/agent/auto_memory_wrapper/agent.py (3)
AutoMemoryWrapperGraph(40-278)get_wrapper_node_count(104-121)build_graph(247-278)src/nat/agent/auto_memory_wrapper/state.py (1)
AutoMemoryWrapperState(21-30)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/nat/agent/auto_memory_wrapper/register.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: In code comments use the abbreviations: nat (API namespace/CLI), nvidia-nat (package), NAT (env var prefixes); never use these abbreviations in documentation
Follow PEP 20 and PEP 8 for Python style
Run yapf with column_limit=120; yapf is used for formatting (run second)
Indent with 4 spaces (no tabs) and end each file with a single trailing newline
Use ruff (ruff check --fix) as a linter (not formatter) per pyproject.toml; fix warnings unless explicitly ignored
Respect Python naming schemes: snake_case for functions/variables, PascalCase for classes, UPPER_CASE for constants
When re-raising exceptions, use bare raise to preserve stack trace; log with logger.error(), not logger.exception()
When catching and logging without re-raising, use logger.exception() to capture full stack trace
Provide Google-style docstrings for every public module, class, function, and CLI command
Docstring first line must be a concise description ending with a period
Validate and sanitize all user input, especially in web or CLI interfaces
Prefer httpx with SSL verification enabled by default and follow OWASP Top-10 recommendations
Use async/await for I/O-bound work (HTTP, DB, file I/O)
Cache expensive computations with functools.lru_cache or an external cache when appropriate
Leverage NumPy vectorized operations when beneficial and feasible
Files:
src/nat/agent/auto_memory_wrapper/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/agent/auto_memory_wrapper/register.py
src/nat/**/*
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
src/nat/**/* contains core functionality; changes should prioritize backward compatibility
Files:
src/nat/agent/auto_memory_wrapper/register.py
⚙️ CodeRabbit configuration file
This directory contains the core functionality of the toolkit. Changes should prioritize backward compatibility.
Files:
src/nat/agent/auto_memory_wrapper/register.py
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}: Every file must start with the standard SPDX Apache-2.0 header
Confirm copyright years are up to date when a file is changed
All source files must include the SPDX Apache-2.0 header template (copy from an existing file)
Files:
src/nat/agent/auto_memory_wrapper/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/agent/auto_memory_wrapper/register.py
🧬 Code graph analysis (1)
src/nat/agent/auto_memory_wrapper/register.py (7)
src/nat/builder/builder.py (1)
Builder(68-290)src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-25)src/nat/builder/function_info.py (2)
FunctionInfo(290-625)from_fn(552-625)src/nat/data_models/agent.py (1)
AgentBaseConfig(23-34)src/nat/data_models/component_ref.py (2)
FunctionRef(94-102)MemoryRef(127-135)src/nat/agent/auto_memory_wrapper/agent.py (3)
AutoMemoryWrapperGraph(40-278)get_wrapper_node_count(104-121)build_graph(247-278)src/nat/agent/auto_memory_wrapper/state.py (1)
AutoMemoryWrapperState(21-30)
🔇 Additional comments (3)
src/nat/agent/auto_memory_wrapper/register.py (3)
1-31: LGTM! License header and imports are properly structured.The SPDX Apache-2.0 header is present with current copyright years, and imports follow the correct organization pattern (standard library → third-party → internal).
33-114: LGTM! Well-documented configuration model.The
AutoMemoryAgentConfigclass has comprehensive field descriptions, clear YAML examples, and proper documentation of multi-tenant behavior. All fields have appropriate type hints and defaults.
116-220: LGTM! Recursion limit calculation and exception handling are robust.The function properly:
- Retrieves required components from the builder
- Calculates recursion limits with proper None and type checking (addresses past review concerns)
- Handles exceptions with appropriate verbose flag control (addresses information disclosure concern)
- Applies consistent formula for recursion limits
The multiplier of 3 for
tool_call_max_retries(line 164) appears to be domain-specific for the ReWOO agent architecture.
willkill07
added
feature request
|
/ok to test c8dbc3d |
|
/ok to test f981bf5 |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/nat/agent/test_auto_memory_wrapper.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: In code comments use the abbreviations: nat (API namespace/CLI), nvidia-nat (package), NAT (env var prefixes); never use these abbreviations in documentation
Follow PEP 20 and PEP 8 for Python style
Run yapf with column_limit=120; yapf is used for formatting (run second)
Indent with 4 spaces (no tabs) and end each file with a single trailing newline
Use ruff (ruff check --fix) as a linter (not formatter) per pyproject.toml; fix warnings unless explicitly ignored
Respect Python naming schemes: snake_case for functions/variables, PascalCase for classes, UPPER_CASE for constants
When re-raising exceptions, use bare raise to preserve stack trace; log with logger.error(), not logger.exception()
When catching and logging without re-raising, use logger.exception() to capture full stack trace
Provide Google-style docstrings for every public module, class, function, and CLI command
Docstring first line must be a concise description ending with a period
Validate and sanitize all user input, especially in web or CLI interfaces
Prefer httpx with SSL verification enabled by default and follow OWASP Top-10 recommendations
Use async/await for I/O-bound work (HTTP, DB, file I/O)
Cache expensive computations with functools.lru_cache or an external cache when appropriate
Leverage NumPy vectorized operations when beneficial and feasible
Files:
tests/nat/agent/test_auto_memory_wrapper.py
{tests/**/*.py,examples/*/tests/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Unit tests live in tests/ or examples/*/tests and use markers defined in pyproject.toml (e.g., integration)
Files:
tests/nat/agent/test_auto_memory_wrapper.py
{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}: Use pytest with pytest-asyncio for async code
Test functions should be named with test_ prefix using snake_case
Extract frequently repeated test code into pytest fixtures
Pytest fixtures should define the name argument in @pytest.fixture and the function should be prefixed fixture_ using snake_case
Mock external services with pytest_httpserver or unittest.mock; avoid live endpoints
Mark slow tests with @pytest.mark.slow to skip in default test suite
Mark integration tests requiring external services with @pytest.mark.integration and follow nat-tests guidelines
Files:
tests/nat/agent/test_auto_memory_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:
tests/nat/agent/test_auto_memory_wrapper.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:
tests/nat/agent/test_auto_memory_wrapper.py
tests/**/*.py
⚙️ CodeRabbit configuration file
tests/**/*.py: - Ensure that tests are comprehensive, cover edge cases, and validate the functionality of the code. - Test functions should be named using 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/agent/test_auto_memory_wrapper.py
🧬 Code graph analysis (1)
tests/nat/agent/test_auto_memory_wrapper.py (5)
src/nat/agent/auto_memory_wrapper/agent.py (9)
AutoMemoryWrapperGraph(40-278)get_wrapper_node_count(104-121)_get_user_id_from_context(70-102)_langchain_message_to_nat_message(124-141)capture_user_message_node(143-160)memory_retrieve_node(162-190)inner_agent_node(192-226)capture_ai_response_node(228-245)build_graph(247-278)src/nat/agent/auto_memory_wrapper/state.py (1)
AutoMemoryWrapperState(21-30)src/nat/builder/context.py (2)
Context(118-316)user_manager(138-149)src/nat/data_models/api_server.py (6)
ChatRequest(125-195)ChatResponse(338-388)ChatResponseChoice(295-297)ChoiceMessage(277-279)Usage(309-312)UserMessageContentRoleType(41-47)packages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.py (2)
add_items(90-173)search(175-233)
🪛 Ruff (0.14.3)
tests/nat/agent/test_auto_memory_wrapper.py
52-52: Unused function argument: chat_request
(ARG001)
🔇 Additional comments (3)
tests/nat/agent/test_auto_memory_wrapper.py (3)
1-14: LGTM: SPDX header is correct.The Apache-2.0 license header is properly formatted and the copyright year is appropriate.
88-102: LGTM: Comprehensive state initialization tests.The tests appropriately cover empty and populated message list scenarios for the
AutoMemoryWrapperStatemodel.
105-376: LGTM: Excellent test coverage for AutoMemoryWrapperGraph.The test suite comprehensively validates:
- Feature flag behavior (enabled/disabled states)
- All async node operations with proper mocking
- User ID extraction from multiple sources (user_manager, headers, default)
- Message type conversions
- Parameter propagation to memory backend
- Edge cases (empty messages, no search results)
The 23 test methods provide strong confidence in the wrapper's correctness.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/nat/agent/test_auto_memory_wrapper.py (1)
78-78: Consider adding type hints to fixture parameters.The
fixture_wrapper_graphparameters (mock_inner_agent,mock_memory_editor,mock_context) lack type hints. Adding them would improve consistency with the coding guidelines that require type hints for all parameters.Apply this diff:
@pytest.fixture(name="wrapper_graph") -def fixture_wrapper_graph(mock_inner_agent, mock_memory_editor, mock_context) -> AutoMemoryWrapperGraph: +def fixture_wrapper_graph(mock_inner_agent: Mock, mock_memory_editor: AsyncMock, mock_context: Mock) -> AutoMemoryWrapperGraph: """Create an AutoMemoryWrapperGraph instance for testing."""
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/nat/agent/test_auto_memory_wrapper.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: In code comments use the abbreviations: nat (API namespace/CLI), nvidia-nat (package), NAT (env var prefixes); never use these abbreviations in documentation
Follow PEP 20 and PEP 8 for Python style
Run yapf with column_limit=120; yapf is used for formatting (run second)
Indent with 4 spaces (no tabs) and end each file with a single trailing newline
Use ruff (ruff check --fix) as a linter (not formatter) per pyproject.toml; fix warnings unless explicitly ignored
Respect Python naming schemes: snake_case for functions/variables, PascalCase for classes, UPPER_CASE for constants
When re-raising exceptions, use bare raise to preserve stack trace; log with logger.error(), not logger.exception()
When catching and logging without re-raising, use logger.exception() to capture full stack trace
Provide Google-style docstrings for every public module, class, function, and CLI command
Docstring first line must be a concise description ending with a period
Validate and sanitize all user input, especially in web or CLI interfaces
Prefer httpx with SSL verification enabled by default and follow OWASP Top-10 recommendations
Use async/await for I/O-bound work (HTTP, DB, file I/O)
Cache expensive computations with functools.lru_cache or an external cache when appropriate
Leverage NumPy vectorized operations when beneficial and feasible
Files:
tests/nat/agent/test_auto_memory_wrapper.py
{tests/**/*.py,examples/*/tests/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Unit tests live in tests/ or examples/*/tests and use markers defined in pyproject.toml (e.g., integration)
Files:
tests/nat/agent/test_auto_memory_wrapper.py
{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}: Use pytest with pytest-asyncio for async code
Test functions should be named with test_ prefix using snake_case
Extract frequently repeated test code into pytest fixtures
Pytest fixtures should define the name argument in @pytest.fixture and the function should be prefixed fixture_ using snake_case
Mock external services with pytest_httpserver or unittest.mock; avoid live endpoints
Mark slow tests with @pytest.mark.slow to skip in default test suite
Mark integration tests requiring external services with @pytest.mark.integration and follow nat-tests guidelines
Files:
tests/nat/agent/test_auto_memory_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:
tests/nat/agent/test_auto_memory_wrapper.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:
tests/nat/agent/test_auto_memory_wrapper.py
tests/**/*.py
⚙️ CodeRabbit configuration file
tests/**/*.py: - Ensure that tests are comprehensive, cover edge cases, and validate the functionality of the code. - Test functions should be named using 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/agent/test_auto_memory_wrapper.py
🧠 Learnings (2)
📚 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} : Pytest fixtures should define the name argument in pytest.fixture and the function should be prefixed fixture_ using snake_case
Applied to files:
tests/nat/agent/test_auto_memory_wrapper.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} : Extract frequently repeated test code into pytest fixtures
Applied to files:
tests/nat/agent/test_auto_memory_wrapper.py
🧬 Code graph analysis (1)
tests/nat/agent/test_auto_memory_wrapper.py (6)
src/nat/agent/auto_memory_wrapper/agent.py (9)
AutoMemoryWrapperGraph(40-278)get_wrapper_node_count(104-121)_get_user_id_from_context(70-102)_langchain_message_to_nat_message(124-141)capture_user_message_node(143-160)memory_retrieve_node(162-190)inner_agent_node(192-226)capture_ai_response_node(228-245)build_graph(247-278)src/nat/agent/auto_memory_wrapper/state.py (1)
AutoMemoryWrapperState(21-30)src/nat/builder/context.py (2)
Context(118-316)user_manager(138-149)src/nat/data_models/api_server.py (6)
ChatRequest(125-195)ChatResponse(338-388)ChatResponseChoice(295-297)ChoiceMessage(277-279)Usage(309-312)UserMessageContentRoleType(41-47)packages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.py (2)
add_items(90-173)search(175-233)src/nat/builder/builder.py (1)
get_id(64-65)
🪛 Ruff (0.14.3)
tests/nat/agent/test_auto_memory_wrapper.py
52-52: Unused function argument: chat_request
(ARG001)
🔇 Additional comments (3)
tests/nat/agent/test_auto_memory_wrapper.py (3)
38-85: Fixture naming conventions properly implemented.The fixtures correctly follow the project's pytest guidelines with
@pytest.fixture(name="...")decorators,fixture_prefixed function names, and return type hints. This addresses the past review feedback.
52-52: Static analysis false positive on line 52.The
chat_requestparameter is correctly used within the function body to construct theChatResponse. The parameter is also necessary to match the expectedainvokesignature. This static analysis warning can be ignored.
88-376: Excellent test coverage and structure.The test suite is comprehensive, well-organized, and follows pytest best practices:
- Complete coverage: Tests cover initialization, all node operations, parameter propagation, feature flags, and edge cases (empty inputs, disabled features, no results).
- Proper async patterns: Async tests correctly use
async defwithAsyncMockfor asynchronous operations.- Clear organization: Logical grouping into
TestAutoMemoryWrapperStateandTestAutoMemoryWrapperGraphclasses with descriptive test names.- Robust assertions: Tests verify both return values and mock interactions, ensuring behavioral correctness.
|
@willkill07 should pass tests now, sorry about that, apparently my pre-commit hooks were not working. |
|
/ok to test e5097a0 |
|
@willkill07 code coverage test seems to only be failing because of being out of date with the base branch. The code coverage patch test passes |
willkill07
left a comment
willkill07
left a comment
There was a problem hiding this comment.
This is looking good!
Some last-minute polishing:
- you add an example but don't update the examples/README.md -- please do this
- For agents like this, it would be good to surface them under the documentation. Because this is memory-related, it could go under
store-and-retrieve/memory(https://docs.nvidia.com/nemo/agent-toolkit/latest/store-and-retrieve/memory.html). Otherwise it should go underworkflowsas a sibling next to ReAct/ReWOO (https://docs.nvidia.com/nemo/agent-toolkit/latest/workflows/about/index.html)
Introduces a new agent wrapper that provides automatic memory management for any NAT agent without requiring LLM tool calls. This ensures guaranteed memory capture and retrieval for consistent context across conversations. Key features: - Wraps any NAT agent type (ReAct, ReWOO, Tool Calling, custom) - Automatic user message capture before agent processing - Automatic memory context retrieval and injection via system messages - Automatic AI response capture after generation - Backend-agnostic design (works with Zep, Mem0, Redis, etc.) - Configurable feature flags and backend-specific parameters - Transparent recursion limit calculation based on inner agent needs Implementation: - AutoMemoryWrapperState: Simple state model tracking conversation messages - AutoMemoryWrapperGraph: LangGraph workflow with conditional memory nodes - AutoMemoryAgentConfig: Pydantic config with feature flags and parameters - Examples with Zep Cloud demonstrating basic and advanced usage The wrapper treats inner agents as black-box functions receiving ChatRequest with multiple messages (including system messages with memory context). Inner agents manage their own state internally while the wrapper handles all memory operations transparently. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Jack Ryan <jackaldenryan@gmail.com> Signed-off-by: jackaldenryan <jackaldenryan@gmail.com>
- Add calculator tool (ReAct agent requires at least one tool) - Remove custom system_prompt to use default - Add required description field to workflow - Remove config_zep_advanced.yml (will be replaced with Zep V3 version) - Simplify config by removing commented optional parameters Signed-off-by: Jack Ryan <jackaldenryan@gmail.com> Signed-off-by: Jack Ryan <jackaldenryan@gmail.com> Signed-off-by: jackaldenryan <jackaldenryan@gmail.com>
- Fix code formatting with yapf - Fix import sorting and style with ruff - Update Zep error handling for v3 API changes - Add comprehensive test coverage for auto_memory_wrapper Signed-off-by: Jack Ryan <jackaldenryan@gmail.com> Signed-off-by: jackaldenryan <jackaldenryan@gmail.com> Signed-off-by: Jack Ryan <jackaldenryan@gmail.com>
Co-authored-by: Will Killian <2007799+willkill07@users.noreply.github.com> Signed-off-by: Jack Ryan <61809814+jackaldenryan@users.noreply.github.com>
Co-authored-by: Will Killian <2007799+willkill07@users.noreply.github.com> Signed-off-by: Jack Ryan <61809814+jackaldenryan@users.noreply.github.com>
Removes all references to the deprecated use_openai_api parameter that was removed in PR NVIDIA#959. The parameter is no longer needed as all agents now accept ChatRequest objects with multiple messages by default through the ChatRequestOrMessage type. Changes: - Fixed missing inner_agent_config variable in register.py - Removed use_openai_api from config_zep.yml example - Removed documentation requiring use_openai_api in README.md - Removed validation check for use_openai_api requirement The auto_memory_wrapper continues to inject system messages with memory context correctly without this parameter. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: jackaldenryan <jackaldenryan@gmail.com>
…ation Changed the logic in register.py (lines 145-172) to properly validate attribute values before performing arithmetic operations. Now treats None or non-numeric attribute values as missing and falls back to the safe default of 15. Key changes: - Initialize inner_max_calls to None to track whether a valid value is found - Validate each attribute is not None and is numeric before using it - Apply the *3 multiplier for tool_call_max_retries only after validation - Convert to int before final calculation to ensure integer arithmetic - Use cascade logic to try each attribute in order This prevents TypeError when attributes contain None or non-numeric values. All 25 auto_memory_wrapper tests pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: jackaldenryan <jackaldenryan@gmail.com>
Added AsyncGenerator[FunctionInfo, None] return type annotation to the auto_memory_agent async function to follow coding guidelines for public API functions. Changes: - Added AsyncGenerator import from collections.abc module - Moved FunctionInfo import to top-level imports - Added return type annotation to auto_memory_agent function signature - Removed duplicate FunctionInfo import from within the function All 25 auto_memory_wrapper tests pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: jackaldenryan <jackaldenryan@gmail.com>
Updated exception handling in auto_memory_agent to only return exception details when config.verbose is true. In non-verbose mode, returns a generic "Auto-memory agent failed" message without exposing exception details. Changes: - Modified except block at line 219 to return generic message - Exception details still logged via logger.exception() for debugging - Only verbose mode returns str(ex) with exception details This prevents sensitive information leakage in production environments while maintaining detailed error logging for troubleshooting. All 25 auto_memory_wrapper tests pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: jackaldenryan <jackaldenryan@gmail.com>
Updated GeneratorExit handler to re-raise the exception after logging, allowing proper generator cleanup. Previously the exception was being swallowed which prevented normal generator termination behavior. Changes: - Changed logger.exception() to logger.debug() for GeneratorExit - Added "raise" statement to re-raise GeneratorExit after logging - This ensures the exception propagates normally per Python best practices The finally block still executes for cleanup while allowing the GeneratorExit to propagate as expected. All 25 auto_memory_wrapper tests pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: jackaldenryan <jackaldenryan@gmail.com>
Replace 'pluggable' with 'interchangeable' to resolve Vale documentation linter error that was causing CI checks to fail. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: jackaldenryan <jackaldenryan@gmail.com>
Remove unused 'result' variable and 'Message' import in test_inner_agent_node_with_memory_context to fix Ruff linting errors. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: jackaldenryan <jackaldenryan@gmail.com>
- Add name= argument to all @pytest.fixture decorators - Rename fixture functions to start with fixture_ prefix - Add precise return type hints (AsyncMock, Mock, AutoMemoryWrapperGraph) - Follow <CHORUS_TAG>pytest.fixture</CHORUS_TAG>(name="...") convention 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: jackaldenryan <jackaldenryan@gmail.com>
Add comprehensive documentation for the auto_memory_wrapper agent: - New auto-memory-wrapper.md guide with configuration and usage - Updated memory.md with automatic memory section and examples - Added auto-memory-wrapper to workflows index - Updated examples README with auto_memory_wrapper entry The automatic memory wrapper provides guaranteed memory capture and retrieval for any NAT agent without requiring LLM memory tool invocation. Signed-off-by: jackaldenryan <jackaldenryan@gmail.com>
jackaldenryan
force-pushed
the
auto-memory-wrapper
branch
from
e5097a0 to
682ba42
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
examples/agents/auto_memory_wrapper/README.md (2)
18-18: Replace "NAT" with proper toolkit name in heading.This issue was already flagged in a previous review. The heading uses "NAT Agents" which violates documentation guidelines requiring the full name "NVIDIA NeMo Agent Toolkit" on first occurrence.
As per coding guidelines.
20-20: Replace "NAT" abbreviation in body text.This issue was already flagged in a previous review. Line 20 and other occurrences use "NAT agent" which is prohibited in documentation per coding guidelines.
As per coding guidelines.
🧹 Nitpick comments (4)
packages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.py (2)
148-157: Same fragile error detection pattern; consider extracting common logic.This error handling block has the same fragility concerns as the user creation logic above (lines 78-85): string matching on error messages and treating 400 Bad Request the same as 409 Conflict.
Additionally, the error handling logic is duplicated between user and thread creation. Consider extracting a helper method to reduce duplication and ensure consistent behavior:
+ def _is_already_exists_error(self, error: ApiError) -> bool: + """Check if an ApiError indicates the resource already exists.""" + if error.status_code == 409: + return True + if error.status_code == 400 and "already exists" in str(error).lower(): + return True + return False + async def _ensure_user_exists(self, user_id: str) -> None: """...""" try: await self._client.user.add(...) except ApiError as e: - if e.status_code == 409: - logger.info("Zep user already exists - 409, continuing") - elif e.status_code == 400 and "already exists" in str(e).lower(): - logger.info("Zep user already exists - 400, continuing") + if self._is_already_exists_error(e): + logger.info("Zep user already exists (status=%s), continuing", e.status_code) else: logger.error(...) raiseThis helper would also be used in the thread creation logic at lines 148-154.
78-85: Add defensive logging and document Zep SDK error handling pattern.The Zep Cloud Python SDK does not expose structured error codes or exception types for "already exists" errors—it only returns HTTP error responses. Your string-matching approach on error messages is therefore the expected pattern given the SDK's limitations, but it remains brittle and could break if the Zep API changes its error message format.
Improve robustness by:
- Add a comment explaining why string matching is necessary and document the expected error message format (e.g.,
"already exists"from Zep API).- Log full error details when the string match succeeds, to help with future debugging if the API response format changes.
- Monitor the Zep Cloud SDK for future improvements to structured error handling.
Example improvement:
elif e.status_code == 400 and "already exists" in str(e).lower(): # Zep API returns "already exists" in error message; SDK does not provide structured error codes logger.info("Zep user already exists - 400 with 'already exists' message, continuing (e=%s)", str(e))docs/source/workflows/about/auto-memory-wrapper.md (1)
166-205: Label conversational fenced blocks with a language.Markdown lint reports MD040 for the conversation snippets; please add a language hint (e.g.,
text) to each fenced block so tooling renders them correctly. As per coding guidelines.-``` +```text User: "My name is Alice and I prefer Python for data analysis." Agent: "Nice to meet you, Alice! I'll remember your preference for Python."@@
-+text
User: "What programming language should I use for my data project?"
Agent: "Based on what you told me earlier, I recommend Python for your data analysis project since that's your preferred language."src/nat/agent/auto_memory_wrapper/agent.py (1)
261-262: Consider refactoring nested ternary for readability.The nested ternary expression for determining the entry point is difficult to parse and violates the KISS principle from PEP 20.
Consider refactoring to an if-elif chain for improved clarity:
- workflow.set_entry_point("capture_user_message" if self.save_user_messages else "memory_retrieve" if self. - retrieve_memory else "inner_agent") + if self.save_user_messages: + entry_point = "capture_user_message" + elif self.retrieve_memory: + entry_point = "memory_retrieve" + else: + entry_point = "inner_agent" + workflow.set_entry_point(entry_point)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
docs/source/store-and-retrieve/memory.md(1 hunks)docs/source/workflows/about/auto-memory-wrapper.md(1 hunks)docs/source/workflows/about/index.md(1 hunks)examples/README.md(1 hunks)examples/agents/auto_memory_wrapper/README.md(1 hunks)examples/agents/auto_memory_wrapper/configs/config_zep.yml(1 hunks)packages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.py(3 hunks)src/nat/agent/auto_memory_wrapper/__init__.py(1 hunks)src/nat/agent/auto_memory_wrapper/agent.py(1 hunks)src/nat/agent/auto_memory_wrapper/register.py(1 hunks)src/nat/agent/auto_memory_wrapper/state.py(1 hunks)src/nat/agent/register.py(1 hunks)tests/nat/agent/test_auto_memory_wrapper.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/source/workflows/about/index.md
🚧 Files skipped from review as they are similar to previous changes (2)
- examples/agents/auto_memory_wrapper/configs/config_zep.yml
- src/nat/agent/auto_memory_wrapper/state.py
🧰 Additional context used
📓 Path-based instructions (14)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: In code comments use the abbreviations: nat (API namespace/CLI), nvidia-nat (package), NAT (env var prefixes); never use these abbreviations in documentation
Follow PEP 20 and PEP 8 for Python style
Run yapf with column_limit=120; yapf is used for formatting (run second)
Indent with 4 spaces (no tabs) and end each file with a single trailing newline
Use ruff (ruff check --fix) as a linter (not formatter) per pyproject.toml; fix warnings unless explicitly ignored
Respect Python naming schemes: snake_case for functions/variables, PascalCase for classes, UPPER_CASE for constants
When re-raising exceptions, use bare raise to preserve stack trace; log with logger.error(), not logger.exception()
When catching and logging without re-raising, use logger.exception() to capture full stack trace
Provide Google-style docstrings for every public module, class, function, and CLI command
Docstring first line must be a concise description ending with a period
Validate and sanitize all user input, especially in web or CLI interfaces
Prefer httpx with SSL verification enabled by default and follow OWASP Top-10 recommendations
Use async/await for I/O-bound work (HTTP, DB, file I/O)
Cache expensive computations with functools.lru_cache or an external cache when appropriate
Leverage NumPy vectorized operations when beneficial and feasible
Files:
src/nat/agent/auto_memory_wrapper/__init__.pysrc/nat/agent/auto_memory_wrapper/register.pysrc/nat/agent/register.pysrc/nat/agent/auto_memory_wrapper/agent.pypackages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.pytests/nat/agent/test_auto_memory_wrapper.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/agent/auto_memory_wrapper/__init__.pysrc/nat/agent/auto_memory_wrapper/register.pysrc/nat/agent/register.pysrc/nat/agent/auto_memory_wrapper/agent.pypackages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.py
src/nat/**/*
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
src/nat/**/* contains core functionality; changes should prioritize backward compatibility
Files:
src/nat/agent/auto_memory_wrapper/__init__.pysrc/nat/agent/auto_memory_wrapper/register.pysrc/nat/agent/register.pysrc/nat/agent/auto_memory_wrapper/agent.py
⚙️ CodeRabbit configuration file
This directory contains the core functionality of the toolkit. Changes should prioritize backward compatibility.
Files:
src/nat/agent/auto_memory_wrapper/__init__.pysrc/nat/agent/auto_memory_wrapper/register.pysrc/nat/agent/register.pysrc/nat/agent/auto_memory_wrapper/agent.py
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}: Every file must start with the standard SPDX Apache-2.0 header
Confirm copyright years are up to date when a file is changed
All source files must include the SPDX Apache-2.0 header template (copy from an existing file)
Files:
src/nat/agent/auto_memory_wrapper/__init__.pyexamples/agents/auto_memory_wrapper/README.mdexamples/README.mddocs/source/store-and-retrieve/memory.mddocs/source/workflows/about/auto-memory-wrapper.mdsrc/nat/agent/auto_memory_wrapper/register.pysrc/nat/agent/register.pysrc/nat/agent/auto_memory_wrapper/agent.pypackages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.pytests/nat/agent/test_auto_memory_wrapper.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/agent/auto_memory_wrapper/__init__.pyexamples/agents/auto_memory_wrapper/README.mdexamples/README.mddocs/source/store-and-retrieve/memory.mddocs/source/workflows/about/auto-memory-wrapper.mdsrc/nat/agent/auto_memory_wrapper/register.pysrc/nat/agent/register.pysrc/nat/agent/auto_memory_wrapper/agent.pypackages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.pytests/nat/agent/test_auto_memory_wrapper.py
{docs/source/**/*.md,**/README.@(md|ipynb)}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{docs/source/**/*.md,**/README.@(md|ipynb)}: Use the full name “NVIDIA NeMo Agent toolkit” on first use in documentation, then “NeMo Agent toolkit”; in headings use “NeMo Agent Toolkit” (capital T)
Do not use deprecated names (Agent Intelligence toolkit, aiqtoolkit, AgentIQ, AIQ/aiq) in documentation unless explicitly referring to deprecated names
Never use “NAT”/“nat” abbreviations in documentation
Documentation must be clear/comprehensive; avoid TODOs/FIXMEs/placeholders; avoid offensive/outdated terms; ensure spelling is correct
Files:
examples/agents/auto_memory_wrapper/README.mdexamples/README.mddocs/source/store-and-retrieve/memory.mddocs/source/workflows/about/auto-memory-wrapper.md
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/agents/auto_memory_wrapper/README.mdexamples/README.md
docs/source/**/*.md
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
docs/source/**/*.md: Documentation sources are Markdown files under docs/source
Surround code entities with backticks in docs to avoid Vale false positives
Do not use words listed in ci/vale/styles/config/vocabularies/nat/reject.txt; accepted words in accept.txt are allowed
Files:
docs/source/store-and-retrieve/memory.mddocs/source/workflows/about/auto-memory-wrapper.md
docs/source/**/*
⚙️ CodeRabbit configuration file
This directory contains the source code for the documentation. All documentation should be written in Markdown format. Any image files should be placed in the
docs/source/_staticdirectory.
Files:
docs/source/store-and-retrieve/memory.mddocs/source/workflows/about/auto-memory-wrapper.md
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_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.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_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.py
{tests/**/*.py,examples/*/tests/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Unit tests live in tests/ or examples/*/tests and use markers defined in pyproject.toml (e.g., integration)
Files:
tests/nat/agent/test_auto_memory_wrapper.py
{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{tests/**/*.py,examples/*/tests/**/*.py,packages/*/tests/**/*.py}: Use pytest with pytest-asyncio for async code
Test functions should be named with test_ prefix using snake_case
Extract frequently repeated test code into pytest fixtures
Pytest fixtures should define the name argument in @pytest.fixture and the function should be prefixed fixture_ using snake_case
Mock external services with pytest_httpserver or unittest.mock; avoid live endpoints
Mark slow tests with @pytest.mark.slow to skip in default test suite
Mark integration tests requiring external services with @pytest.mark.integration and follow nat-tests guidelines
Files:
tests/nat/agent/test_auto_memory_wrapper.py
tests/**/*.py
⚙️ CodeRabbit configuration file
tests/**/*.py: - Ensure that tests are comprehensive, cover edge cases, and validate the functionality of the code. - Test functions should be named using 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/agent/test_auto_memory_wrapper.py
🧠 Learnings (11)
📚 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 {docs/source/**/*.md,**/README.@(md|ipynb)} : Use the full name “NVIDIA NeMo Agent toolkit” on first use in documentation, then “NeMo Agent toolkit”; in headings use “NeMo Agent Toolkit” (capital T)
Applied to files:
examples/agents/auto_memory_wrapper/README.mddocs/source/store-and-retrieve/memory.md
📚 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 : In code comments use the abbreviations: nat (API namespace/CLI), nvidia-nat (package), NAT (env var prefixes); never use these abbreviations in documentation
Applied to files:
examples/agents/auto_memory_wrapper/README.md
📚 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 {docs/source/**/*.md,**/README.@(md|ipynb)} : Do not use deprecated names (Agent Intelligence toolkit, aiqtoolkit, AgentIQ, AIQ/aiq) in documentation unless explicitly referring to deprecated names
Applied to files:
examples/agents/auto_memory_wrapper/README.mdexamples/README.md
📚 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 {docs/source/**/*.md,**/README.@(md|ipynb)} : Never use “NAT”/“nat” abbreviations in documentation
Applied to files:
examples/agents/auto_memory_wrapper/README.md
📚 Learning: 2025-08-22T20:18:28.041Z
Learnt from: CR
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 0
File: .cursor/rules/cursor-rules.mdc:0-0
Timestamp: 2025-08-22T20:18:28.041Z
Learning: Applies to .cursor/rules/**/*.mdc : Use consistent project terminology in descriptions (e.g., NAT workflows, NAT CLI commands)
Applied to files:
examples/agents/auto_memory_wrapper/README.md
📚 Learning: 2025-08-22T20:29:13.433Z
Learnt from: AnuradhaKaruppiah
Repo: NVIDIA/NeMo-Agent-Toolkit PR: 701
File: examples/evaluation_and_profiling/simple_calculator_eval/src/nat_simple_calculator_eval/configs/config-with-custom-post-process.yml:16-18
Timestamp: 2025-08-22T20:29:13.433Z
Learning: In the NeMo-Agent-Toolkit project, configuration consistency across example files is prioritized over removing unused config options, even when those options aren't currently implemented in the runtime.
Applied to files:
examples/agents/auto_memory_wrapper/README.md
📚 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/agents/auto_memory_wrapper/README.md
📚 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:
src/nat/agent/auto_memory_wrapper/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 catching and logging without re-raising, use logger.exception() to capture full stack trace
Applied to files:
src/nat/agent/auto_memory_wrapper/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 {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:
tests/nat/agent/test_auto_memory_wrapper.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} : Extract frequently repeated test code into pytest fixtures
Applied to files:
tests/nat/agent/test_auto_memory_wrapper.py
🧬 Code graph analysis (3)
src/nat/agent/auto_memory_wrapper/register.py (7)
src/nat/builder/builder.py (1)
Builder(68-290)src/nat/builder/framework_enum.py (1)
LLMFrameworkEnum(19-25)src/nat/builder/function_info.py (2)
FunctionInfo(290-625)from_fn(552-625)src/nat/data_models/agent.py (1)
AgentBaseConfig(23-34)src/nat/data_models/component_ref.py (2)
FunctionRef(94-102)MemoryRef(127-135)src/nat/agent/auto_memory_wrapper/agent.py (3)
AutoMemoryWrapperGraph(40-278)get_wrapper_node_count(104-121)build_graph(247-278)src/nat/agent/auto_memory_wrapper/state.py (1)
AutoMemoryWrapperState(21-30)
src/nat/agent/auto_memory_wrapper/agent.py (5)
src/nat/builder/context.py (2)
Context(118-316)user_manager(138-149)src/nat/data_models/api_server.py (2)
ChatRequest(125-195)UserMessageContentRoleType(41-47)src/nat/agent/auto_memory_wrapper/state.py (1)
AutoMemoryWrapperState(21-30)src/nat/builder/builder.py (1)
get_id(64-65)packages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.py (2)
add_items(90-173)search(175-233)
tests/nat/agent/test_auto_memory_wrapper.py (5)
src/nat/agent/auto_memory_wrapper/agent.py (9)
AutoMemoryWrapperGraph(40-278)get_wrapper_node_count(104-121)_get_user_id_from_context(70-102)_langchain_message_to_nat_message(124-141)capture_user_message_node(143-160)memory_retrieve_node(162-190)inner_agent_node(192-226)capture_ai_response_node(228-245)build_graph(247-278)src/nat/agent/auto_memory_wrapper/state.py (1)
AutoMemoryWrapperState(21-30)src/nat/builder/context.py (2)
Context(118-316)user_manager(138-149)src/nat/data_models/api_server.py (6)
ChatRequest(125-195)ChatResponse(338-388)ChatResponseChoice(295-297)ChoiceMessage(277-279)Usage(309-312)UserMessageContentRoleType(41-47)packages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.py (2)
add_items(90-173)search(175-233)
🪛 LanguageTool
docs/source/workflows/about/auto-memory-wrapper.md
[grammar] ~335-~335: Use a hyphen to join words.
Context: ...m_name: nim_llm ### Wrapping a Tool Calling Agent yaml functions: my_t...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.18.1)
docs/source/workflows/about/auto-memory-wrapper.md
166-166: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
177-177: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Ruff (0.14.4)
src/nat/agent/auto_memory_wrapper/agent.py
90-90: Do not catch blind exception: Exception
(BLE001)
tests/nat/agent/test_auto_memory_wrapper.py
52-52: Unused function argument: chat_request
(ARG001)
🔇 Additional comments (4)
tests/nat/agent/test_auto_memory_wrapper.py (1)
1-376: Excellent test coverage and adherence to guidelines.The test suite is comprehensive with 25 unit tests covering:
- State initialization scenarios
- Graph construction with various feature flags
- User ID extraction from multiple sources
- Message conversion utilities
- All async node behaviors (capture, retrieve, invoke, capture response)
- Memory editor parameter propagation
Fixtures correctly follow the project's pytest naming conventions with
@pytest.fixture(name=...)andfixture_prefix as required by coding guidelines.src/nat/agent/auto_memory_wrapper/register.py (1)
1-227: Well-structured registration implementation.The auto_memory_agent registration function correctly:
- Retrieves and validates configuration components (memory editor, inner agent, inner agent config)
- Calculates appropriate recursion limits accounting for wrapper overhead
- Handles multiple inner agent config patterns (max_tool_calls, max_iterations, tool_call_max_retries)
- Provides proper lifecycle management with generator cleanup
- Implements secure error handling (no sensitive data leakage in non-verbose mode)
- Re-raises GeneratorExit to allow proper cleanup
All previously identified critical and major issues have been addressed in prior commits.
src/nat/agent/auto_memory_wrapper/agent.py (2)
70-102: User ID extraction logic is well-designed.The three-tier fallback mechanism (user_manager → X-User-ID header → default_user) provides excellent flexibility for production, testing, and development scenarios. The broad exception catch at line 90 is appropriate defensive programming to ensure graceful fallback if user_manager.get_id() fails for any reason.
143-245: Memory integration nodes are correctly implemented.The four node functions properly handle:
- Conditional execution based on feature flags
- User ID resolution for multi-tenant isolation
- Memory editor interaction with backend-specific parameters
- Graceful handling of empty states and disabled features
- Proper message type checking (HumanMessage, AIMessage)
The integration between wrapper state and inner agent is clean, treating the inner agent as a black box that manages its own internal state.
|
@willkill07 added workflow documentation page, as well as a section to the memory documentation page and updated the examples README. Hopefully good to go! |
|
/ok to test 682ba42 |
|
@willkill07 looks like everything passes, except the codecov/project, likely because the branch is out of date with bass branch, since codecov/patch passes |
|
@jackaldenryan yup! I know. I've just been busy on my end and just want to take one final pass. |
willkill07
left a comment
willkill07
left a comment
There was a problem hiding this comment.
Approving with minor feedback.
…scenarios Update zep_editor.py to use logger.debug() instead of logger.info() for handling expected 'already exists' responses (409/400 errors) when creating users and threads. This aligns with codebase conventions where expected, non-failing behavior should use DEBUG level logging. Also simplify auto_memory_wrapper/__init__.py to match the pattern of other agent packages by removing unnecessary __all__ and imports, leaving only the license header. These changes address code review feedback and follow established patterns in the codebase. Signed-off-by: jackaldenryan <jackaldenryan@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.py (2)
78-85: Good approach for handling concurrent user creation, with a minor robustness note.The enhanced error handling correctly treats HTTP 409 and HTTP 400 with "already exists" as idempotent success cases, which improves robustness for concurrent requests. The use of DEBUG-level logging for these expected cases addresses the earlier reviewer concern about logging verbosity.
However, the string matching
"already exists" in str(e).lower()for detecting HTTP 400 conflicts is somewhat brittle—if Zep modifies their error message format, this detection could fail. Consider checking whetherApiErrorexposes more structured error information (e.g., an error code or type field) that would be more reliable than substring matching.
148-157: Consistent error handling with the same robustness consideration.The thread creation error handling mirrors the user creation logic and correctly treats HTTP 409 and HTTP 400 with "already exists" as success cases. The graceful degradation (setting
thread_ready = Falseand continuing) is appropriate, and the use oflogger.exception()without re-raising follows the coding guidelines.As with the user creation handling, the string-based detection of "already exists" in HTTP 400 responses is functional but potentially fragile. If Zep provides structured error information beyond the message string, leveraging that would improve long-term maintainability.
The error-handling logic is now duplicated between lines 78-85 and lines 148-157. While the duplication is limited and each context has slight differences (one raises, one sets a flag), extracting a shared helper method could reduce repetition if this pattern needs to be extended to other operations in the future.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.py(3 hunks)src/nat/agent/auto_memory_wrapper/__init__.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/nat/agent/auto_memory_wrapper/init.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*
⚙️ 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_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.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_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.py
|
@willkill07 addressed your minor feedback, should be good to go! |
|
/ok to test c33ef46 |
2 participants
Description
This PR introduces an AutoMemoryWrapper component that implements the recommended design pattern for agent memory management. This wrapper automatically handles memory capture and retrieval for any NAT agent without requiring LLM tool calls.
Why AutoMemoryWrapper instead of memory tools?
Raw message capture: Memory services like Zep and Mem0 are designed to create memories from every raw message between the user and the agent, not selective memories chosen by the LLM. Since these providers automatically extract structured memories from raw conversation messages, there's no need for an LLM to decide what to save—the process is deterministic and can be handled automatically by the AutoMemoryWrapper. Delegating this to a memory tool would add unnecessary latency and risk inconsistent memory storage.
Automatic retrieval: Best practice (especially for Zep) is to retrieve relevant memory context automatically for every user message, using the most recent messages as the search query, rather than requiring the agent to explicitly decide when to search and what the search query should be. More advanced memory implementations can involve LLM-decided search queries, but most implementations should start with deterministic memory retrieval (which reduces latency).
Memory tools still supported: For use cases requiring explicit memory search, you can disable automatic retrieval in the AutoMemoryWrapper configuration (
retrieve_memory_for_every_response: false) and use memory search tools instead, while keeping the automatic capture of user and assistant messages.Key Features
MemoryEditorimplementationImplementation Details
Bug Fixes
This PR also includes a minor bug fix to the Zep Cloud editor implementation (
packages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.py) to handle both 409 (Conflict) and 400 (Bad Request with "already exists") error codes when creating users and threads, improving robustness for concurrent requests.Files Changed
src/nat/agent/auto_memory_wrapper/- New agent implementationexamples/agents/auto_memory_wrapper/- Example with comprehensive documentationtests/nat/agent/test_auto_memory_wrapper.py- 25 unit tests (100% passing)packages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.py- Bug fix for error handlingCloses
By Submitting this PR I confirm:
Summary by CodeRabbit
New Features
Documentation
Tests
Bug Fixes