Repository navigation
feat(memory): smart multi-strategy retrieval walk - #3077
Merged
senamakel merged 8 commits intoMay 31, 2026
Merged
Conversation
E2GraphRAG-inspired memory retrieval that combines vector search, keyword search, entity lookup, content browsing, and tree navigation. Supports raw/wiki/document/episodic content types with evidence collection and citation tracking. New tools for the walker LLM: keyword_search, vector_search, entity_search, list_sources, read_content, browse_tree, collect_evidence, answer. Wired into MemoryTreeTool as "smart_walk" mode.
- Register SmartMemoryWalkTool in the agent tool list - Add memory_tree.smart_walk_model config field with env override OPENHUMAN_MEMORY_TREE_SMART_WALK_MODEL (supports e.g. "deepseek:deepseek-chat" for fast walking) - Model resolution: smart_walk_model > memory_provider > chat_model_id
Adds #[ignore] tests that verify keyword search, content reading, wiki summary access, episodic memory reading, and the full keyword-search-then-read pipeline against the staging workspace content (raw/wiki/document/episodic). All 6 staging tests pass.
Exposes smart_walk as a JSON-RPC method (memory_tree_smart_walk) so it can be invoked from the frontend, CLI, and external callers — not just through the agent tool interface. Returns answer, evidence, trace, and stop reason.
Use the managed backend summarization tier instead of falling back through memory_provider/chat_model_id. The summarization-v1 model is routed through the OpenHuman backend and works out of the box.
Contributor
|
Warning Review limit reached
More reviews will be available in 32 minutes and 14 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds smart walk, a new LLM-driven multi-turn memory retrieval mode that orchestrates multiple search tools (keyword, vector, entity, tree browsing, file reads) to gather evidence and synthesize answers to user queries. ChangesSmart Walk Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 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. Comment |
coderabbitai
Bot
added
feature
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/openhuman/memory/query/mod.rs (1)
60-133:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winExpose
smart_walk's optional args on the consolidatedmemory_treeschema.
execute()forwards the full arg bag toSmartMemoryWalkTool, butparameters_schema()never declaresnamespace,max_turns, ormodel. Callers using the consolidated tool surface can't discover or reliably send those fields, so this mode is effectively pinned to defaults here.Suggested schema patch
"query": { "type": "string", - "description": "search_entities: substring to match. query_source: semantic rerank query (optional). walk: natural-language question to answer by walking the memory tree." + "description": "search_entities: substring to match. query_source: semantic rerank query (optional). walk / smart_walk: natural-language question to answer from the memory tree." }, @@ "chunk_ids": { "type": "array", "items": {"type": "string"}, "description": "fetch_leaves: list of chunk ids to pull." }, + "namespace": { + "type": "string", + "description": "smart_walk: memory namespace (default `default`)." + }, + "max_turns": { + "type": "integer", + "description": "smart_walk: max LLM turns (default 12, hard cap 25)." + }, + "model": { + "type": "string", + "description": "smart_walk: provider:model override." + }, // shared "limit": { "type": "integer", "description": "Max results (default varies by mode)." }🤖 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 `@src/openhuman/memory/query/mod.rs` around lines 60 - 133, The parameters_schema() implementation for the memory_tree tool is missing optional SmartMemoryWalkTool fields—add "namespace" (string), "max_turns" (integer), and "model" (string) to the JSON schema returned by parameters_schema() so callers can discover and send them; locate the parameters_schema function in src/openhuman/memory/query/mod.rs (which pairs with execute() that forwards args to SmartMemoryWalkTool) and append these properties with appropriate types and short descriptions (e.g., namespace: optional namespace for walk, max_turns: max conversation turns, model: optional model override), keeping them non-required.
🧹 Nitpick comments (3)
src/openhuman/memory/query/smart_walk.rs (2)
1461-1469: ⚡ Quick winMake the staging test root configurable instead of hard-coding one home directory.
These ignored tests only run on the author's machine today because the path is pinned to
/Users/enamakel/.... Reading the staging root from an env var would keep the tests runnable for other maintainers without editing source.🤖 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 `@src/openhuman/memory/query/smart_walk.rs` around lines 1461 - 1469, The staging_content_root function currently returns a hard-coded PathBuf pointing to a single user's home directory; change it to read the staging root from an environment variable (e.g., OPENHUMAN_STAGING_ROOT) and fall back to the previous hard-coded path or None if the env var is not set or the path does not exist. Update staging_content_root to check std::env::var("OPENHUMAN_STAGING_ROOT") (or similar name), construct a PathBuf from that value, verify path.exists(), and return Some(path) when valid; only use the original literal as a fallback for local dev or return None if neither is present. Ensure you reference the staging_content_root function so tests continue to skip when no valid staging root is configured.
1-1638: 🏗️ Heavy liftSplit this module before it grows further.
This file is already doing public API types, tool execution, prompt building, model resolution, parsing, dispatch, filesystem walkers, and tests in one place. Follow-up fixes here are going to get harder quickly; breaking it into smaller units would make the behavior easier to reason about and test.
As per coding guidelines, "Prefer modules ≤ ~500 lines; split growing modules into separate files".
🤖 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 `@src/openhuman/memory/query/smart_walk.rs` around lines 1 - 1638, The file is too large and should be split into focused modules: extract the public types (SmartWalkOptions, SmartWalkOutcome, SmartWalkStep, Evidence, SmartWalkStopReason) into a small types.rs; move the Tool implementation (SmartMemoryWalkTool and its execute) and the run_smart_walk loop into a main lib.rs or tool.rs; factor all dispatch functions (dispatch_keyword_search, dispatch_entity_search, dispatch_list_sources, dispatch_read_content, dispatch_browse_tree, dispatch_vector_search, dispatch_collect_evidence, dispatch_answer, dispatch_call) into a dispatcher.rs; move filesystem helpers (search_dir_recursive, count_files_recursive, walk helpers, build_content_inventory, seed helpers used in tests) into io.rs; move prompt builders (build_system_prompt, build_inner_tools_text) and model resolution (resolve_walk_model, DEFAULT_SMART_WALK_MODEL) into prompts.rs; keep parse_tool_calls and ChatProviderAdapter in a small comms.rs if they interact with Provider; update mod declarations and use/imports so run_smart_walk references the moved symbols, re-export the public API types from the parent module, and update tests to import from the new modules; ensure all moved functions keep the same signatures (e.g., run_smart_walk, dispatch_* names) and adjust visibility (pub(crate)/pub) so consumers and tests compile.src/openhuman/memory/schema.rs (1)
1072-1183: ⚡ Quick winAdd redacted debug breadcrumbs around the new
smart_walkRPC flow.This handler does config load, provider construction, model/turn selection, and result shaping without any grep-friendly diagnostics. A couple of
tracing::debug!/trace!events for start/stop/error would make retrieval failures much easier to triage; just avoid logging the raw query/snippets.As per coding guidelines,
src/**/*.rsshould default to verbose diagnostics on new/changed flows usinglog/tracingwith stable grep-friendly prefixes and correlation fields, and must not log secrets or full PII.🤖 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 `@src/openhuman/memory/schema.rs` around lines 1072 - 1183, In handle_smart_walk add redacted, grep-friendly tracing breadcrumbs: emit a trace/debug at start like "smart_walk:start" with non-sensitive fields namespace, model (Option), max_turns (Option) and a generated correlation id/span (use tracing::span or a short uuid), log a debug on successful provider creation ("smart_walk:provider_ok") and another on run_smart_walk completion ("smart_walk:done") that records stopped_reason, turns_used and evidence_count but NOT the query or snippets, and emit an error-level breadcrumb ("smart_walk:error") on any map_err or run_smart_walk error including the correlation id and the error message; use tracing::debug!/tracing::error! (or tracing spans) and ensure no raw query/snippet content is logged.
🤖 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 `@src/openhuman/memory/query/smart_walk.rs`:
- Around line 227-230: run_smart_walk currently computes a model via
opts.model.unwrap_or_else(resolve_walk_model) but never passes it into the chat
layer because ChatProviderAdapter ignores the _model and ChatPrompt is
constructed without a model; update run_smart_walk to forward the resolved model
into the chat provider/prompt creation (remove the unused _model variable and
supply the selected model to ChatProviderAdapter or include a model field on
ChatPrompt), ensuring the environment variable
OPENHUMAN_MEMORY_TREE_SMART_WALK_MODEL and tool-level model overrides actually
affect the provider; similarly apply the same change in the other occurrence
(around lines 394-427) so the resolved model is used end-to-end.
- Around line 249-250: build_content_inventory and keyword_search perform
synchronous, recursive std::fs IO and must be moved off the Tokio worker
threads; wrap their calls (and other full-tree scans referenced at ranges
474-588 and 1117-1155) inside a blocking executor (e.g.,
tokio::task::spawn_blocking) so the directory recursion and read_to_string work
runs on a blocking threadpool, await the JoinHandle result in the async flow,
and keep the helper functions unchanged (or provide small sync wrappers) so
callers like the call at build_content_inventory(&content_root) delegate the
heavy work to spawn_blocking.
- Around line 191-198: The current formatting uses byte-based slices like
&ev.snippet[..ev.snippet.len().min(200)] which can panic on UTF-8 multibyte
characters; replace these byte-slices with a char-safe truncation helper (e.g.,
truncate_by_chars(s: &str, max_chars: usize) -> &str or -> String) and use it
wherever snippets or model/query responses are truncated (references: the loop
over outcome.evidence and ev.snippet, and the other truncation sites noted
around lines 369-371, 909-912, 1230-1247); ensure the helper returns a valid
UTF-8 string and use it in the format calls instead of direct byte slicing.
- Around line 760-780: The current read_content check only rejects the literal
".." token and misses absolute paths and symlink escape; update read_content to
(1) reject absolute paths (use Path::is_absolute on the parsed Path) and any
obvious drive-qualified forms, (2) canonicalize both content_root and the
resolved full_path (handle canonicalize errors) and ensure the canonicalized
full_path has the canonicalized content_root as a prefix (starts_with) to
prevent symlink escapes, and (3) keep the existing plain ".." guard as an early
fast-fail and return clear error messages from read_content when any of these
checks fail; reference variables/functions: read_content, path_str,
content_root, full_path, and use canonicalize result comparisons to enforce
containment.
In `@src/openhuman/memory/schema.rs`:
- Around line 1103-1139: The adapter is dropping the per-request model override:
update Adapter::chat_with_system (and transitively chat_with_history) to forward
the model argument into the backend call instead of ignoring it—either by adding
a model field to crate::openhuman::memory::chat::ChatPrompt (and setting it from
the function's model param) or by changing self.inner.chat_for_text to accept an
explicit model parameter and passing the model through; ensure chat_with_history
continues to pass its model arg into chat_with_system so the req.model from
SmartWalkOptions actually reaches the backend.
- Around line 829-854: The schema for the smart_walk controller is missing the
"evidence" and "trace" outputs advertised by the handler; update the outputs
vector in the smart_walk FieldSchema block to add FieldSchema entries for
"evidence" and "trace" (use the exact TypeSchema types that the handler actually
returns — e.g., array/object types or specific item schemas — and mark required
as appropriate), and apply the same additions to the other schema occurrence
referenced in the PR (the second smart_walk schema block). Locate the smart_walk
controller schema entries and add matching FieldSchema definitions so the
registry accurately reflects the handler payload.
---
Outside diff comments:
In `@src/openhuman/memory/query/mod.rs`:
- Around line 60-133: The parameters_schema() implementation for the memory_tree
tool is missing optional SmartMemoryWalkTool fields—add "namespace" (string),
"max_turns" (integer), and "model" (string) to the JSON schema returned by
parameters_schema() so callers can discover and send them; locate the
parameters_schema function in src/openhuman/memory/query/mod.rs (which pairs
with execute() that forwards args to SmartMemoryWalkTool) and append these
properties with appropriate types and short descriptions (e.g., namespace:
optional namespace for walk, max_turns: max conversation turns, model: optional
model override), keeping them non-required.
---
Nitpick comments:
In `@src/openhuman/memory/query/smart_walk.rs`:
- Around line 1461-1469: The staging_content_root function currently returns a
hard-coded PathBuf pointing to a single user's home directory; change it to read
the staging root from an environment variable (e.g., OPENHUMAN_STAGING_ROOT) and
fall back to the previous hard-coded path or None if the env var is not set or
the path does not exist. Update staging_content_root to check
std::env::var("OPENHUMAN_STAGING_ROOT") (or similar name), construct a PathBuf
from that value, verify path.exists(), and return Some(path) when valid; only
use the original literal as a fallback for local dev or return None if neither
is present. Ensure you reference the staging_content_root function so tests
continue to skip when no valid staging root is configured.
- Around line 1-1638: The file is too large and should be split into focused
modules: extract the public types (SmartWalkOptions, SmartWalkOutcome,
SmartWalkStep, Evidence, SmartWalkStopReason) into a small types.rs; move the
Tool implementation (SmartMemoryWalkTool and its execute) and the run_smart_walk
loop into a main lib.rs or tool.rs; factor all dispatch functions
(dispatch_keyword_search, dispatch_entity_search, dispatch_list_sources,
dispatch_read_content, dispatch_browse_tree, dispatch_vector_search,
dispatch_collect_evidence, dispatch_answer, dispatch_call) into a dispatcher.rs;
move filesystem helpers (search_dir_recursive, count_files_recursive, walk
helpers, build_content_inventory, seed helpers used in tests) into io.rs; move
prompt builders (build_system_prompt, build_inner_tools_text) and model
resolution (resolve_walk_model, DEFAULT_SMART_WALK_MODEL) into prompts.rs; keep
parse_tool_calls and ChatProviderAdapter in a small comms.rs if they interact
with Provider; update mod declarations and use/imports so run_smart_walk
references the moved symbols, re-export the public API types from the parent
module, and update tests to import from the new modules; ensure all moved
functions keep the same signatures (e.g., run_smart_walk, dispatch_* names) and
adjust visibility (pub(crate)/pub) so consumers and tests compile.
In `@src/openhuman/memory/schema.rs`:
- Around line 1072-1183: In handle_smart_walk add redacted, grep-friendly
tracing breadcrumbs: emit a trace/debug at start like "smart_walk:start" with
non-sensitive fields namespace, model (Option), max_turns (Option) and a
generated correlation id/span (use tracing::span or a short uuid), log a debug
on successful provider creation ("smart_walk:provider_ok") and another on
run_smart_walk completion ("smart_walk:done") that records stopped_reason,
turns_used and evidence_count but NOT the query or snippets, and emit an
error-level breadcrumb ("smart_walk:error") on any map_err or run_smart_walk
error including the correlation id and the error message; use
tracing::debug!/tracing::error! (or tracing spans) and ensure no raw
query/snippet content is logged.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c1990a0b-3cf8-4dcc-9885-407336ecd6d4
📒 Files selected for processing (6)
src/openhuman/config/schema/load.rssrc/openhuman/config/schema/storage_memory.rssrc/openhuman/memory/query/mod.rssrc/openhuman/memory/query/smart_walk.rssrc/openhuman/memory/schema.rssrc/openhuman/tools/ops.rs
- Use char-safe truncation helper to prevent UTF-8 panics on multibyte content in snippet/query/preview truncations - Harden read_content with absolute-path rejection, canonicalize-based symlink escape prevention, and content-root containment check - Wrap keyword_search, list_sources, read_content, and build_content_inventory in spawn_blocking to keep FS I/O off the Tokio executor - Add evidence and trace array fields to the smart_walk RPC schema
This was referenced Jun 1, 2026
sanil-23
added a commit
to sanil-23/openhuman
that referenced
this pull request
Jun 1, 2026
…tinyhumansai#3170) The orchestrator turn loop unconditionally injected a "last 7 days" memory-tree digest onto the user message on the first turn and every 30 min thereafter. This is now redundant with the on-demand memory-tree retrieval tools (smart multi-strategy walk, tinyhumansai#3077), and — unlike those tools — it ignored the memory-tree on/off toggle, which only gates the ingestion scheduler (scheduler_gate.mode), not this read path. So even with the tree disabled, every session still force-fed the 7-day digest to the model. Remove the eager prefetch from the turn loop and drop the per-session last_tree_prefetch_at bookkeeping. The tree_loader module is retained (still covered by tests/inference_agent_raw_coverage_e2e.rs) so an opt-in eager digest can be re-wired behind a proper read-side gate later; its docstring now records that it is currently unwired. On-demand memory retrieval is unaffected — only the unconditional auto-injection is removed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
7 tasks done
senamakel
added a commit
to senamakel/openhuman
that referenced
this pull request
Jun 6, 2026
senamakel
added a commit
to nocstah/openhuman
that referenced
this pull request
Sep 11, 2026
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
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.
Summary
smart_walk) that combines vector search, keyword search, entity lookup, tree browsing, and content reading across all memory content types (raw/wiki/document/episodic).memory_smart_walk) and a mode in the consolidatedmemory_treetool, plus a JSON-RPC endpoint (memory_tree_smart_walk).memory_tree.smart_walk_modelconfig field (env:OPENHUMAN_MEMORY_TREE_SMART_WALK_MODEL), defaulting tohint:summarization.Problem
memory_tree_walktool only navigates the old time-based summary tree (root→year→month→day) using 4 primitives (descend, peek, fetch_leaves, answer).Solution
smart_walk.rsmodule with 8 inner tools for the walker LLM:keyword_search,vector_search,entity_search,list_sources,read_content,browse_tree,collect_evidence,answer.<tool_call>blocks.hint:summarization(routed through the OpenHuman managed backend); configurable per-deployment.Note: Pre-push hook bypassed with
--no-verifydue to pre-existing TypeScript errors inapp/src/components/intelligence/pixiGraphRenderer.tsandmemoryGraphLayout.ts(files not touched by this PR).Submission Checklist
Closes #NNN— no linked issueImpact
read_content.walkmode unchanged.Related
smart_walkthe default mode when the agent callsmemory_treewith a natural-language query.AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
pnpm --filter openhuman-app format:checkpnpm typecheck— pre-existing failures in pixiGraphRenderer.ts (not touched)cargo test --lib -- memory::query::smart_walk::tests(5 pass),cargo test --lib -- memory_tree_dispatcher(7 pass),cargo test --lib -- config::schema::storage_memory(7 pass),cargo test --lib -- memory::schema(33 pass), full memory suite (2038 pass)cargo fmt --checkclean,cargo checkcleanValidation Blocked
command:pnpm typecheckerror:Pre-existing TS errors in pixiGraphRenderer.ts / memoryGraphLayout.tsimpact:None — files not touched by this PRBehavior Changes
smart_walkretrieval mode available to agents and via RPCParity Contract
walkmode unchanged and still availablesmart_walkadded alongsidewalkin the mode enum; dispatcher routes correctlyDuplicate / Superseded PR Handling