Fix auto memory recall scoping#1968
Conversation
Signed-off-by: Will Killian <wkillian@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughThis PR moves user and conversation ID extraction to runtime/front-ends and SessionManager.session(), adds conversation_id and user_message_id context-var lifecycle management, reorders auto-memory-wrapper to retrieve memory before capturing the current message, and implements deterministic per-user Zep thread-id fallback with corresponding tests and docs. ChangesRuntime user/conversation ID and session context
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/source/reference/cli.md (1)
536-538:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSync
nat run --helpoptions withnat start console --help.Line 536-Line 538 still describe
--input_fileas JSON input and do not include--user_id/--conversation_id, which now appear in Line 238-Line 247. Sincenat runis documented as an alias, this section should match to avoid conflicting CLI guidance.As per coding guidelines, “Ensure the
nat runCLI docs remain consistent with the console front-end options.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/reference/cli.md` around lines 536 - 538, Update the `nat run` help block so it matches the `nat start console` options: change the `--input_file` description to the same wording used in the console docs (do not restrict it to "json" if console accepts other structured formats), and add the `--user_id` and `--conversation_id` option entries with the same descriptions as in the `nat start console --help` section; ensure the help text for `nat run` (alias) references the same flags `--input_file`, `--user_id`, and `--conversation_id` and uses identical wording to avoid conflicting CLI guidance.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/nvidia_nat_core/tests/nat/runtime/test_session_manager.py`:
- Line 466: Remove the unnecessary `@pytest.mark.asyncio` decorator from the new
async test (the line showing "`@pytest.mark.asyncio`") so the test relies on the
repo's auto-detection for async tests; if that decorator import is now unused,
also remove the unused pytest import/marker reference to keep imports clean.
---
Outside diff comments:
In `@docs/source/reference/cli.md`:
- Around line 536-538: Update the `nat run` help block so it matches the `nat
start console` options: change the `--input_file` description to the same
wording used in the console docs (do not restrict it to "json" if console
accepts other structured formats), and add the `--user_id` and
`--conversation_id` option entries with the same descriptions as in the `nat
start console --help` section; ensure the help text for `nat run` (alias)
references the same flags `--input_file`, `--user_id`, and `--conversation_id`
and uses identical wording to avoid conflicting CLI guidance.
🪄 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: Enterprise
Run ID: ad8754c2-2b7e-4f3c-9b58-002324754b57
📒 Files selected for processing (12)
docs/source/build-workflows/memory.mddocs/source/components/agents/auto-memory-wrapper/auto-memory-wrapper.mddocs/source/reference/cli.mdexamples/agents/auto_memory_wrapper/README.mdpackages/nvidia_nat_core/src/nat/front_ends/console/console_front_end_config.pypackages/nvidia_nat_core/src/nat/front_ends/console/console_front_end_plugin.pypackages/nvidia_nat_core/src/nat/runtime/session.pypackages/nvidia_nat_core/tests/nat/runtime/test_session_manager.pypackages/nvidia_nat_langchain/src/nat/plugins/langchain/agent/auto_memory_wrapper/agent.pypackages/nvidia_nat_langchain/tests/agent/test_auto_memory_wrapper.pypackages/nvidia_nat_zep_cloud/src/nat/plugins/zep_cloud/zep_editor.pypackages/nvidia_nat_zep_cloud/tests/test_zep_editor.py
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Will Killian <2007799+willkill07@users.noreply.github.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
|
/merge |
Description
Closes NAT-270
Fix automatic memory recall for memory-backed agent workflows run across CLI invocations.
This updates the auto-memory wrapper to retrieve existing memory before storing the current user message, so recall questions do not pollute memory search before lookup. It also passes the console front end
user_idandconversation_idinto the runtime session, ensures direct session conversation metadata is scoped and reset correctly, and updates the Zep Cloud adapter to use query-based graph search before falling back to thread context. When no conversation ID is supplied, Zep now uses a deterministic per-user default thread instead of a single global fallback thread.The root cause was a combination of memory operation ordering and runtime scoping gaps:
nat rundid not propagate the configured console user or conversation IDdefault_zep_threadwhile retrieval depended on thread context.Docs were updated to describe
nat run --user_id,nat run --conversation_id, and the runtime scoping model for memory-backed workflows.By Submitting this PR I confirm:
Validation
Automated checks run locally:
Provided manual validation:
Seeding
Retrieval
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests