Add Redis Agent Memory Server as memory type#1702
Open
justin-cechmanek wants to merge 45 commits intoNVIDIA:developfrom
Open
Add Redis Agent Memory Server as memory type#1702justin-cechmanek wants to merge 45 commits intoNVIDIA:developfrom
justin-cechmanek wants to merge 45 commits intoNVIDIA:developfrom
Conversation
Merge develop to main for rc-10
Merge develop to main
Merge 1.0.0: upstream/develop to upstream/main
Merge pull request NVIDIA#92 from NVIDIA/develop
Updated changelog with another bug fix (NVIDIA#93)
[RELEASE] AIQToolkit v1.1.0
Changelog updated for 1.1.0 (NVIDIA#291)
## Description <!-- Note: The pull request title will be included in the CHANGELOG. --> <!-- Provide a standalone description of changes in this PR. --> <!-- Reference any issues closed by this PR with "closes NVIDIA#1234". All PRs should have an issue they close--> <!-- SPDX-FileCopyrightText: Copyright (c) 2025, NVIDIA CORPORATION & AFFILIATES. All rights reserved. SPDX-License-Identifier: Apache-2.0 Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. --> ## ❄️ Code freeze for `develop` and `v1.2.0` release ### What does this mean? Only critical/hotfix level issues should be merged into `develop` until release (merging of this PR). ### What is the purpose of this PR? - Update documentation - Allow testing for the new release - Enable a means to merge `develop` into `main` for the release ## By Submitting this PR I confirm: - I am familiar with the [Contributing Guidelines](https://github.com/NVIDIA/NeMo-Agent-Toolkit/blob/develop/docs/source/resources/contributing.md). - We require that all contributors "sign-off" on their commits. This certifies that the contribution is your original work, or you have rights to submit it under the same license, or a compatible license. - Any contribution which contains commits that are not Signed-Off will not be accepted. - When the PR is ready for review, new or existing tests cover these changes. - When the PR is ready for review, the documentation is up to date with these changes.
<!-- SPDX-FileCopyrightText: Copyright (c) 2025, NVIDIA CORPORATION & AFFILIATES. All rights reserved. SPDX-License-Identifier: Apache-2.0 Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. --> ## ❄️ Code freeze for `v1.2.1` release ### What does this mean? Only critical/hotfix level issues should be merged into `develop` until release (merging of this PR). ### What is the purpose of this PR? - Update documentation - Allow testing for the new release - Enable a means to merge `develop` into `main` for the release
Signed-off-by: David Gardner <dagardner@nvidia.com>
<!-- SPDX-FileCopyrightText: Copyright (c) 2025, NVIDIA CORPORATION & AFFILIATES. All rights reserved. SPDX-License-Identifier: Apache-2.0 Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. --> ## ❄️ Code freeze for `release/1.3` and `v1.3.0` release ### What does this mean? Only critical/hotfix level issues should be merged into `release/1.3` until release (merging of this PR). ### What is the purpose of this PR? - Update documentation - Allow testing for the new release - Enable a means to merge `release/1.3` into `main` for the release
Signed-off-by: David Gardner <dagardner@nvidia.com>
<!-- SPDX-FileCopyrightText: Copyright (c) 2025, NVIDIA CORPORATION & AFFILIATES. All rights reserved. SPDX-License-Identifier: Apache-2.0 Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. --> ## ❄️ Code freeze for `release/1.3` and `v1.3.1` release ### What does this mean? Only critical/hotfix level issues should be merged into `release/1.3` until release (merging of this PR). ### What is the purpose of this PR? - Update documentation - Allow testing for the new release - Enable a means to merge `release/1.3` into `main` for the release
Signed-off-by: David Gardner <dagardner@nvidia.com>
<!-- SPDX-FileCopyrightText: Copyright (c) 2025-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. SPDX-License-Identifier: Apache-2.0 Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. --> ## ❄️ Code freeze for `v1.4.0` release ### What does this mean? Only critical/hotfix level issues should be merged into `release/1.4` until release (merging of this PR). ### What is the purpose of this PR? - [RELEASE] NeMo-Agent-Toolkit v1.4.0
Signed-off-by: Michael Demoret <mdemoret@nvidia.com>
- Add package pyproject.toml, LICENSE, and LICENSE-3rd-party - Package will provide Redis Agent Memory Server as a NAT memory type Made-with: Cursor Signed-off-by: Justin Cechmanek <justin.cechmanek@redis.com> Made-with: Cursor
- Register packages/nvidia_nat_agent_memory_server in [tool.uv.workspace] members Made-with: Cursor Signed-off-by: Justin Cechmanek <justin.cechmanek@redis.com>
- Add nvidia-nat[agent-memory-server] extra and include in 'most' bundle Made-with: Cursor Signed-off-by: Justin Cechmanek <justin.cechmanek@redis.com>
- Add AgentMemoryServerEditor (add_items, search, remove_items) using agent-memory-client - Add AgentMemoryServerMemoryConfig with base_url and default_namespace - Register memory type via @register_memory and nat.components entry point - Add agent-memory-client dependency and setuptools/entry-point config in pyproject.toml Made-with: Cursor Signed-off-by: Justin Cechmanek <justin.cechmanek@redis.com>
- Add tests/test_agent_memory_editor.py following nvidia_nat_redis structure - Test add_items (success, empty list, conversation-derived text, skip no-text) - Test search (success, missing user_id, empty memories) - Test remove_items (by memory_id, by user_id, no-op when client lacks methods) - Fix editor to import ClientMemoryRecord/MemoryTypeEnum from agent_memory_client.models - Add nvidia-nat-agent-memory-server to tool.uv.sources (workspace = true) for resolution Signed-off-by: Justin Cechmanek <justin.cechmanek@redis.com> Made-with: Cursor
Signed-off-by: Justin Cechmanek <justin.cechmanek@redis.com>
ClientMemoryRecord has no metadata attribute; remove invalid assignment to fix type error and runtime. Signed-off-by: Justin Cechmanek <justin.cechmanek@redis.com> Made-with: Cursor
Signed-off-by: Justin Cechmanek <justin.cechmanek@redis.com>
WalkthroughAdds a new package Changes
Sequence DiagramsequenceDiagram
actor NAT as NAT Framework
participant Builder
participant Factory as Memory Factory
participant Client as Agent Memory Server Client
participant Editor as AgentMemoryServerEditor
NAT->>Builder: build(AgentMemoryServerMemoryConfig)
Builder->>Factory: call async factory
Factory->>Client: create_memory_client(base_url, namespace)
Client-->>Factory: client instance
Factory->>Editor: wrap client
Editor-->>Factory: editor instance
Factory-->>Builder: yield editor
Builder-->>NAT: ready
rect rgba(100, 200, 100, 0.5)
NAT->>Editor: add_items(items)
Editor->>Client: create_long_term_memory(records)
Client-->>Editor: success
end
rect rgba(100, 150, 200, 0.5)
NAT->>Editor: search(query, top_k, user_id=...)
Editor->>Client: search_long_term_memory(text, limit, filters)
Client-->>Editor: memories (with dist)
Editor-->>NAT: MemoryItems
end
rect rgba(200, 150, 100, 0.5)
NAT->>Editor: remove_items(memory_id=... or user_id=...)
Editor->>Client: delete_long_term_memory() or forget()
Client-->>Editor: success
end
NAT->>Factory: cleanup
Factory->>Client: close()
Client-->>Factory: closed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
2 tasks
<!-- SPDX-FileCopyrightText: Copyright (c) 2025-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. SPDX-License-Identifier: Apache-2.0 Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. --> ## ❄️ Code freeze for `develop` and `v1.5.0` release ### What does this mean? Only critical/hotfix level issues should be merged into `develop` until release (merging of this PR). ### What is the purpose of this PR? - Update documentation - Allow testing for the new release - Enable a means to merge `develop` into `main` for the release
Signed-off-by: Anuradha Karuppiah <26330987+AnuradhaKaruppiah@users.noreply.github.com>
|
This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 14 days. |
github-actions
Bot
added
the
Stale
- Add package pyproject.toml, LICENSE, and LICENSE-3rd-party - Package will provide Redis Agent Memory Server as a NAT memory type Made-with: Cursor Signed-off-by: Justin Cechmanek <justin.cechmanek@redis.com> Made-with: Cursor
- Register packages/nvidia_nat_agent_memory_server in [tool.uv.workspace] members Made-with: Cursor Signed-off-by: Justin Cechmanek <justin.cechmanek@redis.com>
- Add nvidia-nat[agent-memory-server] extra and include in 'most' bundle Made-with: Cursor Signed-off-by: Justin Cechmanek <justin.cechmanek@redis.com>
- Add AgentMemoryServerEditor (add_items, search, remove_items) using agent-memory-client - Add AgentMemoryServerMemoryConfig with base_url and default_namespace - Register memory type via @register_memory and nat.components entry point - Add agent-memory-client dependency and setuptools/entry-point config in pyproject.toml Made-with: Cursor Signed-off-by: Justin Cechmanek <justin.cechmanek@redis.com>
- Add tests/test_agent_memory_editor.py following nvidia_nat_redis structure - Test add_items (success, empty list, conversation-derived text, skip no-text) - Test search (success, missing user_id, empty memories) - Test remove_items (by memory_id, by user_id, no-op when client lacks methods) - Fix editor to import ClientMemoryRecord/MemoryTypeEnum from agent_memory_client.models - Add nvidia-nat-agent-memory-server to tool.uv.sources (workspace = true) for resolution Signed-off-by: Justin Cechmanek <justin.cechmanek@redis.com> Made-with: Cursor
Signed-off-by: Justin Cechmanek <justin.cechmanek@redis.com>
ClientMemoryRecord has no metadata attribute; remove invalid assignment to fix type error and runtime. Signed-off-by: Justin Cechmanek <justin.cechmanek@redis.com> Made-with: Cursor
…into feat/add-agent-memory-server Signed-off-by: Justin Cechmanek <justin.cechmanek@redis.com> Made-with: Cursor # Conflicts: # uv.lock
…ry-server' into feat/add-agent-memory-server Signed-off-by: Justin Cechmanek <justin.cechmanek@redis.com> Made-with: Cursor # Conflicts: # uv.lock
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/nvidia_nat_agent_memory_server/src/nat/plugins/agent_memory_server/register.py (1)
16-20: Narrow linter suppression to the import line only.
Line 16andLine 17disable checks for the entire file. Since the only lint-sensitive statement is the side-effect import onLine 20, use a targeted suppression there instead.♻️ Proposed change
-# flake8: noqa -# isort:skip_file - # Import to register the agent_memory_server memory type with NAT. -from . import memory +from . import memory # noqa: F401As per coding guidelines: “Use ruff via ruff check --fix as configured in pyproject.toml; fix warnings unless explicitly ignored in pyproject.toml.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nvidia_nat_agent_memory_server/src/nat/plugins/agent_memory_server/register.py` around lines 16 - 20, Remove the file-level flake8/isort suppressions and instead apply a targeted inline suppression to the side-effect import statement "from . import memory"; specifically delete the top "# flake8: noqa" and "# isort:skip_file" lines and add an inline noqa (and/or isort:skip comment) on the "from . import memory" line so only that import is ignored by linters, then run ruff check --fix to validate no other warnings remain.packages/nvidia_nat_agent_memory_server/src/nat/plugins/agent_memory_server/memory.py (1)
39-50: Add return typing and a public docstring to the async memory factory.
Line 39defines a public API without an explicit return type, and the function is missing a docstring.♻️ Proposed change
+from collections.abc import AsyncIterator + `@register_memory`(config_type=AgentMemoryServerMemoryConfig) -async def agent_memory_server_memory_client(config: AgentMemoryServerMemoryConfig, builder: Builder): +async def agent_memory_server_memory_client( + config: AgentMemoryServerMemoryConfig, + builder: Builder, +) -> AsyncIterator[AgentMemoryServerEditor]: + """Create and yield an `AgentMemoryServerEditor` backed by Agent Memory Server.""" from agent_memory_client import create_memory_clientAs per coding guidelines: “All public APIs require Python 3.11+ type hints on parameters and return values” and “Provide Google-style docstrings for every public module, class, function and CLI command.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nvidia_nat_agent_memory_server/src/nat/plugins/agent_memory_server/memory.py` around lines 39 - 50, The async factory agent_memory_server_memory_client lacks an explicit return type and a public Google-style docstring; add a Python 3.11+ return annotation (an async generator type that yields AgentMemoryServerEditor, e.g., AsyncGenerator[AgentMemoryServerEditor, None]) and add a top-of-function Google-style docstring describing purpose, parameters (config: AgentMemoryServerMemoryConfig, builder: Builder) and return value; update imports if necessary to reference typing.AsyncGenerator and the AgentMemoryServerEditor type for the annotation so the function signature and docstring satisfy the public API guidelines.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/nvidia_nat_agent_memory_server/src/nat/plugins/agent_memory_server/agent_memory_editor.py`:
- Around line 91-98: The code currently converts m.topics into tags using "tags
if isinstance(tags, list) else list(tags)" which will split a string into
characters; update the logic in agent_memory_editor.py around the MemoryItem
construction (the tags variable and the MemoryItem(...) call) to detect strings
and wrap them as a single-element list (e.g., if isinstance(tags, str) then
[tags]) while still converting other iterables to list and defaulting to [] when
None so topics returned as a string become a single tag instead of a character
list.
In `@packages/nvidia_nat_agent_memory_server/tests/test_agent_memory_editor.py`:
- Line 121: Remove the redundant `@pytest.mark.asyncio` decorator in
tests/test_agent_memory_editor.py: locate the test decorated with
`@pytest.mark.asyncio` (the decorator shown on line 121) and delete that decorator
so the async test relies on the repo's async test runner; ensure no other tests
in this file add `@pytest.mark.asyncio` and keep the async test function (the
function immediately following the removed decorator) unchanged.
---
Nitpick comments:
In
`@packages/nvidia_nat_agent_memory_server/src/nat/plugins/agent_memory_server/memory.py`:
- Around line 39-50: The async factory agent_memory_server_memory_client lacks
an explicit return type and a public Google-style docstring; add a Python 3.11+
return annotation (an async generator type that yields AgentMemoryServerEditor,
e.g., AsyncGenerator[AgentMemoryServerEditor, None]) and add a top-of-function
Google-style docstring describing purpose, parameters (config:
AgentMemoryServerMemoryConfig, builder: Builder) and return value; update
imports if necessary to reference typing.AsyncGenerator and the
AgentMemoryServerEditor type for the annotation so the function signature and
docstring satisfy the public API guidelines.
In
`@packages/nvidia_nat_agent_memory_server/src/nat/plugins/agent_memory_server/register.py`:
- Around line 16-20: Remove the file-level flake8/isort suppressions and instead
apply a targeted inline suppression to the side-effect import statement "from .
import memory"; specifically delete the top "# flake8: noqa" and "#
isort:skip_file" lines and add an inline noqa (and/or isort:skip comment) on the
"from . import memory" line so only that import is ignored by linters, then run
ruff check --fix to validate no other warnings remain.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bed22676-3a0f-426d-b7a8-29fe892fc400
⛔ Files ignored due to path filters (49)
examples/A2A/currency_agent_a2a/uv.lockis excluded by!**/*.lockexamples/A2A/math_assistant_a2a/uv.lockis excluded by!**/*.lockexamples/A2A/math_assistant_a2a_protected/uv.lockis excluded by!**/*.lockexamples/HITL/por_to_jiratickets/uv.lockis excluded by!**/*.lockexamples/HITL/simple_calculator_hitl/uv.lockis excluded by!**/*.lockexamples/MCP/kaggle_mcp/uv.lockis excluded by!**/*.lockexamples/MCP/service_account_auth_mcp/uv.lockis excluded by!**/*.lockexamples/MCP/simple_auth_mcp/uv.lockis excluded by!**/*.lockexamples/MCP/simple_calculator_fastmcp/uv.lockis excluded by!**/*.lockexamples/MCP/simple_calculator_fastmcp_protected/uv.lockis excluded by!**/*.lockexamples/MCP/simple_calculator_mcp/uv.lockis excluded by!**/*.lockexamples/MCP/simple_calculator_mcp_protected/uv.lockis excluded by!**/*.lockexamples/RAG/simple_rag/uv.lockis excluded by!**/*.lockexamples/advanced_agents/alert_triage_agent/uv.lockis excluded by!**/*.lockexamples/agents/uv.lockis excluded by!**/*.lockexamples/control_flow/hybrid_control_flow/uv.lockis excluded by!**/*.lockexamples/control_flow/parallel_executor/uv.lockis excluded by!**/*.lockexamples/control_flow/router_agent/uv.lockis excluded by!**/*.lockexamples/control_flow/sequential_executor/uv.lockis excluded by!**/*.lockexamples/custom_functions/automated_description_generation/uv.lockis excluded by!**/*.lockexamples/custom_functions/plot_charts/uv.lockis excluded by!**/*.lockexamples/documentation_guides/uv.lockis excluded by!**/*.lockexamples/documentation_guides/workflows/text_file_ingest/uv.lockis excluded by!**/*.lockexamples/dynamo_integration/latency_sensitivity_demo/uv.lockis excluded by!**/*.lockexamples/dynamo_integration/react_benchmark_agent/uv.lockis excluded by!**/*.lockexamples/evaluation_and_profiling/email_phishing_analyzer/uv.lockis excluded by!**/*.lockexamples/evaluation_and_profiling/simple_calculator_eval/uv.lockis excluded by!**/*.lockexamples/evaluation_and_profiling/simple_web_query_eval/uv.lockis excluded by!**/*.lockexamples/finetuning/dpo_tic_tac_toe/uv.lockis excluded by!**/*.lockexamples/finetuning/rl_with_openpipe_art/uv.lockis excluded by!**/*.lockexamples/frameworks/adk_demo/uv.lockis excluded by!**/*.lockexamples/frameworks/agno_personal_finance/uv.lockis excluded by!**/*.lockexamples/frameworks/haystack_deep_research_agent/uv.lockis excluded by!**/*.lockexamples/frameworks/multi_frameworks/uv.lockis excluded by!**/*.lockexamples/frameworks/nat_autogen_demo/uv.lockis excluded by!**/*.lockexamples/frameworks/semantic_kernel_demo/uv.lockis excluded by!**/*.lockexamples/frameworks/strands_demo/uv.lockis excluded by!**/*.lockexamples/front_ends/per_user_workflow/uv.lockis excluded by!**/*.lockexamples/front_ends/simple_auth/uv.lockis excluded by!**/*.lockexamples/front_ends/simple_calculator_custom_routes/uv.lockis excluded by!**/*.lockexamples/getting_started/simple_calculator/uv.lockis excluded by!**/*.lockexamples/getting_started/simple_web_query/uv.lockis excluded by!**/*.lockexamples/memory/redis/uv.lockis excluded by!**/*.lockexamples/notebooks/uv.lockis excluded by!**/*.lockexamples/object_store/user_report/uv.lockis excluded by!**/*.lockexamples/observability/simple_calculator_observability/uv.lockis excluded by!**/*.lockexamples/prompt_from_file/uv.lockis excluded by!**/*.lockexamples/safety_and_security/retail_agent/uv.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
packages/nvidia_nat_agent_memory_server/LICENSE.mdpackages/nvidia_nat_agent_memory_server/pyproject.tomlpackages/nvidia_nat_agent_memory_server/src/nat/__init__.pypackages/nvidia_nat_agent_memory_server/src/nat/meta/pypi.mdpackages/nvidia_nat_agent_memory_server/src/nat/plugins/__init__.pypackages/nvidia_nat_agent_memory_server/src/nat/plugins/agent_memory_server/__init__.pypackages/nvidia_nat_agent_memory_server/src/nat/plugins/agent_memory_server/agent_memory_editor.pypackages/nvidia_nat_agent_memory_server/src/nat/plugins/agent_memory_server/memory.pypackages/nvidia_nat_agent_memory_server/src/nat/plugins/agent_memory_server/register.pypackages/nvidia_nat_agent_memory_server/tests/test_agent_memory_editor.pypyproject.toml
github-actions
Bot
removed
the
Stale
Signed-off-by: Justin Cechmanek <justin.cechmanek@redis.com>
Signed-off-by: Justin Cechmanek <justin.cechmanek@redis.com>
Signed-off-by: Justin Cechmanek <justin.cechmanek@redis.com>
Signed-off-by: Justin Cechmanek <justin.cechmanek@redis.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/nvidia_nat_agent_memory_server/src/nat/plugins/agent_memory_server/agent_memory_editor.py`:
- Around line 123-130: When both memory_id and user_id may be passed, avoid
falling back from deleting a single memory to calling forget (which deletes all
user memories). Update the branching in the remove/delete routine so it first
checks if memory_id is provided and self._client exposes delete_long_term_memory
and only then calls await self._client.delete_long_term_memory(memory_id);
otherwise, if memory_id is provided but delete_long_term_memory is not
available, log a warning (use logger.warning) that single-item deletion by
memory_id is unsupported and do NOT call self._client.forget; only if memory_id
is not provided and user_id is provided and self._client exposes forget should
you call await self._client.forget(user_id=user_id).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: add7750c-eebc-4555-b1e0-db077f71f12c
⛔ Files ignored due to path filters (5)
examples/agents/uv.lockis excluded by!**/*.lockexamples/finetuning/rl_with_openpipe_art/uv.lockis excluded by!**/*.lockexamples/frameworks/agno_personal_finance/uv.lockis excluded by!**/*.lockexamples/frameworks/multi_frameworks/uv.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
packages/nvidia_nat_agent_memory_server/src/nat/plugins/agent_memory_server/agent_memory_editor.pypackages/nvidia_nat_agent_memory_server/tests/test_agent_memory_editor.py
Signed-off-by: Justin Cechmanek <justin.cechmanek@redis.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/nvidia_nat_agent_memory_server/src/nat/plugins/agent_memory_server/agent_memory_editor.py`:
- Around line 75-91: The search method forwards kwargs to
_client.search_long_term_memory but currently only removes "user_id", which can
cause TypeError when kwargs also contain "text" or "limit"; update the kwargs
filtering in AgentMemoryEditor.search to exclude "user_id", "text", and "limit"
(i.e., only forward keys where k not in {"user_id","text","limit"}) so the
explicit text=query and limit=top_k parameters are not duplicated when calling
_client.search_long_term_memory.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1a95616f-911a-46bf-8ddc-baf4b4cc4afb
📒 Files selected for processing (1)
packages/nvidia_nat_agent_memory_server/src/nat/plugins/agent_memory_server/agent_memory_editor.py
Comment on lines
+75
to
+91
| async def search(self, query: str, top_k: int = 5, **kwargs: Any) -> list[MemoryItem]: | ||
| """Search long-term memory; user_id should be passed in kwargs.""" | ||
| from agent_memory_client.filters import UserId | ||
|
|
||
| user_id = kwargs.get("user_id") | ||
| if user_id is None: | ||
| raise ValueError("search() requires user_id in kwargs for Agent Memory Server") | ||
| user_filter = UserId(eq=user_id) | ||
| results = await self._client.search_long_term_memory( | ||
| text=query, | ||
| limit=top_k, | ||
| user_id=user_filter, | ||
| **{ | ||
| k: v | ||
| for k, v in kwargs.items() if k != "user_id" | ||
| }, | ||
| ) |
There was a problem hiding this comment.
Guard forwarded kwargs to prevent duplicate-keyword failures.
If kwargs includes text or limit, this call can fail with TypeError due to duplicate keyword arguments (text/limit are already explicitly set).
Suggested fix
results = await self._client.search_long_term_memory(
text=query,
limit=top_k,
user_id=user_filter,
**{
k: v
- for k, v in kwargs.items() if k != "user_id"
+ for k, v in kwargs.items() if k not in {"user_id", "text", "limit"}
},
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/nvidia_nat_agent_memory_server/src/nat/plugins/agent_memory_server/agent_memory_editor.py`
around lines 75 - 91, The search method forwards kwargs to
_client.search_long_term_memory but currently only removes "user_id", which can
cause TypeError when kwargs also contain "text" or "limit"; update the kwargs
filtering in AgentMemoryEditor.search to exclude "user_id", "text", and "limit"
(i.e., only forward keys where k not in {"user_id","text","limit"}) so the
explicit text=query and limit=top_k parameters are not duplicated when calling
_client.search_long_term_memory.
|
We will need to do some cleanup on this branch to remove other commits. I can handle this while Justin is out. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR adds a new package
nvidia_nat_agent_memory_serverwhich enables using Redis Agent Memory Server as a memory type.Closes #1635
By Submitting this PR I confirm:
Summary by CodeRabbit
New Features
Documentation
Tests
Chores