Repository navigation
feat(agent_memory): add memory retrieval domain with smart walk parser fix - #3421
Merged
Merged
Conversation
New top-level domain `src/openhuman/agent_memory/` housing the memory retrieval agent, benchmark harness, and performance tracking types. - agent_memory domain: mod.rs, types.rs (WalkBenchmark, BenchmarkSummary), ops.rs (bench_walk, bench_batch wrapping run_smart_walk with timing) - Built-in agent_memory agent: worker tier, read-only sandbox, tools limited to memory_tree/recall/walk/smart_walk/doctor/query_memory - Prompt focused on retrieval strategy, performance contract, and citation-based output format - scripts/bench-memory-walk.sh: CLI benchmark runner against a live memory tree, JSONL output to target/bench-memory/
Move the canonical `memory_loader.rs` (MemoryLoader trait, DefaultMemoryLoader, MemoryCitation, cross-chat context, prior conversations) from `agent/memory_loader.rs` into the new `agent_memory/memory_loader.rs` domain module. The original file becomes a thin re-export shim so all existing harness imports (`agent::memory_loader::*`) continue to resolve.
…o domain - New `call_memory_agent` tool: spawns the agent_memory sub-agent to autonomously walk the memory tree and return cited context for a query. Supports sync (blocking) and async (fire-and-forget) modes. - Move agent definition from agent_registry/agents/agent_memory/ into agent_memory/agent/ so the domain owns its own agent definition. - Switch model hint from "agentic" to "chat" (lightweight/fast tier). - Register call_memory_agent in orchestrator's direct tool list. - Register tool in the global tool registry (tools/ops.rs + mod.rs).
The chat-tier LLM sometimes emits <tool_name>/<parameters> XML inside
<tool_call> blocks instead of the expected JSON {"name","arguments"}
format. The JSON-only parser silently dropped these, causing the walk
to exit after one turn with no evidence.
- Add parse_single_tool_call() with JSON-first + XML fallback parsing
- Add extract_xml_tag() helper for the XML path
- Update system prompt with explicit JSON format example to nudge the
model toward the primary path
- Add bench-memory-retrieval.sh script for local benchmarking
Covers the XML tool-call parsing fix and multi-source memory walking: Parser unit tests: - XML-style <tool_name>/<parameters> inside <tool_call> blocks - Mixed JSON + XML format in same response - XML params with no <parameters> tag - extract_xml_tag helper - parse_single_tool_call JSON/XML/garbage paths E2E walk tests (StubProvider, seeded content tree): - walk_synced_email_with_keyword_and_evidence: 4-turn walk across email + docs, verifies evidence from both sources - walk_with_xml_format_tool_calls: the actual bug scenario — XML calls are parsed, not silently dropped - walk_reads_across_content_types: evidence from raw/episodic/wiki - walk_llm_gives_up_uses_fallback: empty response → LlmGaveUp - walk_direct_answer_without_tools: no tool calls → direct answer - walk_collect_evidence_deduplicates_within_limit: multi-batch evidence collection
Contributor
|
Need the big picture first? Review this PR in Change Stack to see what changed before going file by file. Warning Review limit reached
More reviews will be available in 19 minutes and 22 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 (4)
📝 WalkthroughWalkthroughThis PR introduces a new memory agent subsystem for OpenHuman that consolidates memory retrieval operations into a dedicated agent ( ChangesMemory Agent Feature
Smart Walk Tool Call Parsing
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
coderabbitai
Bot
added
feature
… call_memory_agent All 9 agents that had direct memory tool access now delegate through call_memory_agent instead. This consolidates memory retrieval behind the memory agent, which internally uses the full tool suite (smart walk, keyword search, entity search, tree browse, etc.). Agents updated: orchestrator, planner, researcher, help, presentation_agent, profile_memory_agent, crypto_agent, markets_agent, trigger_reactor. profile_memory_agent retains memory_doctor (diagnostics) and memory_store/memory_forget (write ops) since those are not retrieval.
![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: 18
🧹 Nitpick comments (1)
src/openhuman/agent_memory/agent/prompt.md (1)
32-38: 💤 Low valueAdd language specifier to fenced code block.
The fenced code block at lines 32-38 lacks a language specifier. For clarity and to satisfy linting tools, specify the language (e.g.,
textormarkdown) after the opening backticks.📝 Proposed fix
-``` +```text [Answer text with citations like [1], [2]...] Sources:🤖 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/agent_memory/agent/prompt.md` around lines 32 - 38, The fenced code block in src/openhuman/agent_memory/agent/prompt.md (the snippet starting with ``` and the block showing "Answer text with citations..." and "Sources:") is missing a language specifier; update the opening fence to include a language token (for example change ``` to ```text or ```markdown) so the block is explicitly typed, which satisfies linters and improves rendering.Source: Linters/SAST tools
🤖 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 `@scripts/bench-memory-retrieval.sh`:
- Line 26: The script currently hardcodes CONTENT_ROOT to a single user path;
change CONTENT_ROOT to be set from an environment variable or a positional/flag
argument (e.g., check $CONTENT_ROOT or parse a --content-root/first-arg) and
fall back to a sensible default only if not provided; then validate the
directory exists before proceeding (exit with a clear error if test -d
"$CONTENT_ROOT" fails). Update references to the CONTENT_ROOT variable in the
script so the rest of the benchmark uses the provided path (look for the
CONTENT_ROOT assignment and any usages in bench-memory-retrieval.sh).
- Around line 158-160: The Python one-liner used to set HITS consumes stdin
twice (sys.stdin.read() in both the condition and branch), causing the second
read to be empty; update the HITS assignment to read stdin once into a local
variable (e.g., s = sys.stdin.read()) and then use that variable for the '{'
check and json.loads(...) when extracting result.total, or alternatively replace
the one-liner with a jq-based extraction; ensure the command still falls back to
"parse-error" on failure and that the variable name HITS remains the target of
the assignment.
In `@scripts/bench-memory-walk.sh`:
- Line 163: The echo writes an unescaped "$query" into JSONL causing injection;
change this to build the JSON safely with a JSON serializer (e.g., jq) instead
of string interpolation: use jq -n with --arg for "$query" and "$STATUS" and
--argjson for "$ELAPSED_MS" and include the timestamp via date -u, then append
that safe JSON to "$RESULTS_FILE". Update the line that references
RESULTS_FILE/query/ELAPSED_MS/STATUS to use this jq-based construction so the
query is properly escaped.
- Around line 127-139: The RPC payload currently interpolates the query directly
into the here-doc (RPC_PAYLOAD) causing JSON injection when query contains
quotes/newlines; replace the raw here-doc with a safe JSON builder (e.g., use jq
-n) that sets fields with --arg/--argjson so query, NAMESPACE, MAX_TURNS and the
id ("bench-$idx") are passed as arguments instead of string interpolation;
update the code that assigns RPC_PAYLOAD to use jq (or another proper JSON
serializer) with --arg query "$query", --arg namespace "$NAMESPACE", --arg id
"bench-$idx" and --argjson max_turns "$MAX_TURNS" so the JSON is correctly
escaped and robust.
In `@src/openhuman/agent_memory/agent/agent.toml`:
- Around line 1-28: Add an entry to AGENTS.md documenting the new agent with id
"agent_memory" / display_name "Memory Agent": describe its purpose (memory
retrieval and tree-walking specialist), the when_to_use guidance (copy the
when_to_use text), key config items (delegate_name: retrieve_memory, model hint:
"chat", sandbox_mode: "read_only", agent_tier: "worker", temperature: 0.2,
max_iterations: 15) and list the available tools (memory_recall, memory_tree,
memory_tree_walk, memory_smart_walk, query_memory, memory_doctor,
ask_user_clarification), and include a short example of intended usage and any
important omit_* flags (omit_identity=true, omit_memory_context=false,
omit_safety_preamble=true, omit_skills_catalog=true, omit_profile=false,
omit_memory_md=false).
In `@src/openhuman/agent_memory/memory_loader.rs`:
- Around line 200-207: The current memory.recall(...) calls (e.g., the one that
assigns working_entries) use unwrap_or_default(), which silently drops errors;
update each such call (including the other recall usages that set recent_entries
and longterm_entries) to handle the Result by logging the error with a stable,
grep-friendly prefix (e.g., "MEMORY_RECALL_FAILURE") and relevant context
(query, limit, and backend id) before falling back to an empty vector; keep the
fallback behavior but replace unwrap_or_default() with explicit match/if let
that logs at debug/error level and returns Vec::new() on Err.
In `@src/openhuman/agent_memory/mod.rs`:
- Line 12: Change the exported module from the singular "tool" to the plural
"tools" to match the project contract: replace the `pub mod tool;` export with
`pub mod tools;` and rename the corresponding file from tool.rs to tools.rs (or
create tools.rs if missing), moving any content into tools.rs and updating any
downstream re-exports or uses that reference `openhuman::agent_memory::tool` to
`openhuman::agent_memory::tools`.
In `@src/openhuman/agent_memory/ops.rs`:
- Around line 110-134: The bench_batch loop currently logs per-query failures
but still returns Ok((results, summary)), allowing a completely failed batch to
be treated as success; update bench_batch so that after computing results and
summary it checks if queries is non-empty and results.is_empty() and in that
case returns an Err (e.g., using anyhow::anyhow or converting a descriptive
error) instead of Ok; keep existing behavior when queries is empty or some
results succeeded, and reference the bench_batch function, the results and
queries variables, bench_walk calls, and BenchmarkSummary::from_benchmarks to
locate where to add the check and error return.
- Around line 27-33: The log statements in src/openhuman/agent_memory/ops.rs
(the log::info calls that include the `query` variable in agent_memory::bench
and the other occurrences around the referenced ranges) currently emit raw user
text; replace them with a redacted representation (e.g., compute a stable hash
such as SHA-256 of `query` plus the character length) and log only that
hash+length string instead of the raw `query`. Update every logging site that
references `query` (the log::info calls shown and the ones at the other noted
ranges) to use the redaction helper instead of printing the raw value, ensuring
no raw query or PII is written to logs.
In `@src/openhuman/agent_memory/README.md`:
- Around line 28-43: The fenced code block in README.md (the directory tree
block starting at the shown snippet) is missing a language tag; update the
opening triple-backticks to include a language (for example use ```text) so the
block satisfies MD040 and the docs linter; ensure only the opening fence is
changed (e.g., ``` -> ```text) and no other content is modified.
- Around line 62-65: The README references the wrong registration path for the
built-in agent; update the doc to state that the built-in agent_memory is wired
from the agent's agent.toml (symbol: agent/agent.toml) used by the built-in
loader, and adjust the file list so prompt.rs and prompt.md are described as
living alongside that agent manifest (symbols: prompt.rs, prompt.md,
agent_memory, agent/agent.toml) rather than under the incorrect agent_registry
path. Ensure the single-line entry and bullet list reflect the corrected
location and filenames.
In `@src/openhuman/agent_memory/tool.rs`:
- Around line 65-70: The schema exposes "max_turns" but the execution path never
reads it; update the code that constructs SubagentRunOptions so it reads the
provided "max_turns" parameter (validate against 1..20 per schema) and assigns
it to the corresponding field on SubagentRunOptions instead of relying on
defaults; specifically locate where SubagentRunOptions is built (the code that
sets task_id around the current construction) and add parsing/validation of the
"max_turns" input, clamping/failing if out of range, then pass that value into
SubagentRunOptions so the declared schema is actually enforced at runtime.
- Around line 137-142: The current log in call_memory_agent prints the raw query
at info level; remove or stop logging the full `query` value and instead log
only non-sensitive correlation metadata (e.g., `task_id`, `is_async`) and a safe
diagnostic such as `query.len()` or a redacted snippet; also lower the level to
debug/trace (use log::debug! or log::trace!) for diagnostics. Locate the
log::info! call in the `call_memory_agent` context and replace it so it does not
include `query` (use `query.len()` or a fixed placeholder) and move it to
debug/trace level to avoid exposing PII in production logs. Ensure any remaining
logs follow the guideline to never print full user-provided content.
In `@src/openhuman/agent_memory/types.rs`:
- Around line 45-84: The BenchmarkSummary::from_benchmarks function (which
consumes &[WalkBenchmark] and computes stats) is business logic and should be
moved out of the domain types module into the ops/stats helper module; extract
the entire function body into a new function in ops.rs (or a new stats helper)
and keep BenchmarkSummary as a pure Serde/type definition, then update
references to call the new function, export it if needed, and adjust
imports/visibility for BenchmarkSummary, WalkBenchmark, and the new
from_benchmarks implementation so compilation and access remain correct.
- Around line 67-74: The percentile indexing currently truncates with `as usize`
which biases p50/p95 for even or exact-rank sample sizes; update the computation
around `p50_idx`, `p95_idx` and the assignments to
`p50_elapsed_ms`/`p95_elapsed_ms` to use a proper percentile calculation (e.g.,
compute a float index via (benchmarks.len() - 1) as f64 * quantile and either
round or perform linear interpolation between the two neighboring `elapsed_ms`
values) instead of simple truncation, ensuring you handle small lengths and
clamp indices safely while still setting `runs` and `avg_elapsed_ms` as before.
In `@src/openhuman/memory/query/smart_walk.rs`:
- Around line 1279-1338: Add verbose debug logging in parse_single_tool_call and
extract_xml_tag: log when entering parse_single_tool_call and the input snippet,
log JSON parse failures (including serde error) with a stable prefix like
"[memory::parse]" and a correlation id if available, log selection of the XML
fallback branch ("[memory::parse] using XML-fallback"), log success/failure of
parsing parameters (whether parsed_args came from JSON or heuristic XML parsing)
and any skipped/malformed tags, and log when returning None (dropped call) with
reason; also emit trace-level entry/exit logs from extract_xml_tag when tags are
found/missing using the same prefix. Include function names
parse_single_tool_call and extract_xml_tag in the messages for easy grep.
- Around line 1146-1147: The system-prompt example contains a stray double-quote
at the end of the raw string literal (`</tool_call>"#`) which corrupts the
example; locate the example that contains the tool_call entries `list_sources`
and `keyword_search` and remove the extra `"` so the ending is `</tool_call>#`
(i.e., the raw string delimiter should not be preceded by an extra quote),
ensuring the example string is a valid Rust raw string.
---
Nitpick comments:
In `@src/openhuman/agent_memory/agent/prompt.md`:
- Around line 32-38: The fenced code block in
src/openhuman/agent_memory/agent/prompt.md (the snippet starting with ``` and
the block showing "Answer text with citations..." and "Sources:") is missing a
language specifier; update the opening fence to include a language token (for
example change ``` to ```text or ```markdown) so the block is explicitly typed,
which satisfies linters and improves rendering.
🪄 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: e406ba3c-32c4-4776-acc6-ecd314ea5770
📒 Files selected for processing (19)
scripts/bench-memory-retrieval.shscripts/bench-memory-walk.shsrc/openhuman/agent/memory_loader.rssrc/openhuman/agent_memory/README.mdsrc/openhuman/agent_memory/agent/agent.tomlsrc/openhuman/agent_memory/agent/mod.rssrc/openhuman/agent_memory/agent/prompt.mdsrc/openhuman/agent_memory/agent/prompt.rssrc/openhuman/agent_memory/memory_loader.rssrc/openhuman/agent_memory/mod.rssrc/openhuman/agent_memory/ops.rssrc/openhuman/agent_memory/tool.rssrc/openhuman/agent_memory/types.rssrc/openhuman/agent_registry/agents/loader.rssrc/openhuman/agent_registry/agents/orchestrator/agent.tomlsrc/openhuman/memory/query/smart_walk.rssrc/openhuman/mod.rssrc/openhuman/tools/mod.rssrc/openhuman/tools/ops.rs
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/openhuman/agent_registry/agents/loader.rs (1)
192-195: ⚡ Quick winUpdate loader module docs to match the new built-in layout.
BUILTINSnow includesagent_memoryfrom../../agent_memory/agent/agent.toml, but the file header still says every built-in lives in this folder with only localagent.toml/prompt.rs. Please update that guidance so future edits don’t follow stale assumptions.Based on learnings: “New or changed behavior must ship with matching documentation—at minimum add concise rustdoc / code comments where flow is not obvious, and update AGENTS.md, architecture docs, or feature docs when repository rules or user-visible behavior change.”
🤖 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/agent_registry/agents/loader.rs` around lines 192 - 195, The loader module's documentation is stale because BUILTINS now includes entries that reference agents outside the local folder (e.g., the new "agent_memory" entry pointing at "../../agent_memory/agent/agent.toml"); update the rustdoc/comments in src/openhuman/agent_registry/agents/loader.rs to explain the new built-in layout and that BUILTINS entries may point to external agent dirs, and also update or add a short note in AGENTS.md (or relevant architecture/feature docs) reflecting the rule that built-ins can live in sibling crates and must provide agent.toml and prompt builder functions (reference BUILTINS and the "agent_memory" entry and prompt_fn build function) so future contributors do not assume only local agent.toml/prompt.rs files are allowed.Source: Learnings
🤖 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.
Nitpick comments:
In `@src/openhuman/agent_registry/agents/loader.rs`:
- Around line 192-195: The loader module's documentation is stale because
BUILTINS now includes entries that reference agents outside the local folder
(e.g., the new "agent_memory" entry pointing at
"../../agent_memory/agent/agent.toml"); update the rustdoc/comments in
src/openhuman/agent_registry/agents/loader.rs to explain the new built-in layout
and that BUILTINS entries may point to external agent dirs, and also update or
add a short note in AGENTS.md (or relevant architecture/feature docs) reflecting
the rule that built-ins can live in sibling crates and must provide agent.toml
and prompt builder functions (reference BUILTINS and the "agent_memory" entry
and prompt_fn build function) so future contributors do not assume only local
agent.toml/prompt.rs files are allowed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2ffaeea7-c350-4482-9d2f-0b492a36b18a
📒 Files selected for processing (10)
src/openhuman/agent_registry/agents/crypto_agent/agent.tomlsrc/openhuman/agent_registry/agents/help/agent.tomlsrc/openhuman/agent_registry/agents/loader.rssrc/openhuman/agent_registry/agents/markets_agent/agent.tomlsrc/openhuman/agent_registry/agents/orchestrator/agent.tomlsrc/openhuman/agent_registry/agents/planner/agent.tomlsrc/openhuman/agent_registry/agents/presentation_agent/agent.tomlsrc/openhuman/agent_registry/agents/profile_memory_agent/agent.tomlsrc/openhuman/agent_registry/agents/researcher/agent.tomlsrc/openhuman/agent_registry/agents/trigger_reactor/agent.toml
💤 Files with no reviewable changes (1)
- src/openhuman/agent_registry/agents/orchestrator/agent.toml
- Rename tool.rs → tools.rs for canonical module shape consistency - Fix hardcoded user path in bench-memory-retrieval.sh (auto-detect) - Fix stdin double-read in bench-memory-retrieval.sh Python one-liner - Fix JSON injection in bench-memory-walk.sh via python3 json.dumps - Redact raw queries from log lines (ops.rs, tools.rs) - Return Err when all benchmark queries fail (ops.rs) - Wire max_turns through to subagent prompt (tools.rs) - Fix percentile indexing off-by-one (types.rs) - Add debug logging to XML parser fallback (smart_walk.rs) - Fix stray quote in system prompt example (smart_walk.rs) - Add actual duplicate to dedup test (smart_walk.rs) - Fix README code block language and registration path
…y_agent The e2e test asserted memory_tree was in orchestrator tools — updated to check for call_memory_agent instead, matching the agent TOML migration.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
scripts/bench-memory-retrieval.sh (1)
165-171:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
HITSparsing remains fragile and misses valid RPC response shapes.Line 169 parses only the last line containing
{, which is usually not a complete JSON document for pretty output. It also only checksresult.total, but CLI output can be baretotalwhen logs are empty.Suggested patch
- HITS=$(echo "$OUTPUT" | python3 -c " + HITS=$(printf '%s' "$OUTPUT" | python3 -c " import sys,json -raw = sys.stdin.read() -last_line = [l for l in raw.split('\n') if '{' in l] -d = json.loads(last_line[-1]) if last_line else {} -print(d.get('result',{}).get('total',0)) +raw = sys.stdin.read().strip() +obj = None + +# Try full output first. +try: + obj = json.loads(raw) +except Exception: + # Fallback: parse trailing JSON block if non-JSON text precedes it. + starts = [i for i, ch in enumerate(raw) if ch == '{'] + for i in reversed(starts): + try: + obj = json.loads(raw[i:]) + break + except Exception: + pass + +if not isinstance(obj, dict): + print('parse-error') +else: + payload = obj.get('result', obj) + print(payload.get('total', 0) if isinstance(payload, dict) else 0) " 2>/dev/null || echo "parse-error")🤖 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 `@scripts/bench-memory-retrieval.sh` around lines 165 - 171, The HITS assignment parsing is fragile—update the shell snippet that sets HITS to robustly extract the last complete JSON object from OUTPUT (not just the last line containing "{"), parse it, and read total from either result.total or top-level total (falling back to 0 or a parse-error). Locate the HITS assignment and replace the Python parsing logic (the inline python block) so it finds the final balanced JSON object (e.g., scan from the last '{' and match braces or extract JSON blocks), json.loads that object, then compute hits = d.get('result', {}).get('total') or d.get('total', 0) and print that; ensure non-JSON or parse failures still return "parse-error" as before.
🤖 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 `@scripts/bench-memory-retrieval.sh`:
- Around line 26-32: The pipeline computing CONTENT_ROOT can fail early under
set -euo pipefail when "$WORKSPACE_DIR/users" is missing; before running the
find that sets CONTENT_ROOT, explicitly check that the directory
"$WORKSPACE_DIR/users" exists (guard the path) and emit the same error/exit if
it does not, or alternatively make the find pipeline non-fatal (e.g., ensure it
cannot return a non-zero status) so the subsequent if [[ -z "$CONTENT_ROOT" ]]
check (which references CONTENT_ROOT) still executes; update the logic around
WORKSPACE_DIR and the CONTENT_ROOT assignment to include this guard.
In `@src/openhuman/agent_memory/tools.rs`:
- Around line 103-107: The code builds max_turns from args.get("max_turns") and
currently allows 0 because it only applies an upper bound via .min(20); change
the parsing to clamp into the valid range [1,20] (e.g., map the value to
v.max(1).min(20) as usize or otherwise enforce a minimum of 1) so the schema
minimum is respected and the later message that references max_turns can't
report "use at most 0 retrieval turns"; apply the same fix to the other
identical extraction (the second args.get("max_turns") / map(|v| v.min(20) as
usize) occurrence) and ensure any error/message logic uses the new clamped
value.
---
Duplicate comments:
In `@scripts/bench-memory-retrieval.sh`:
- Around line 165-171: The HITS assignment parsing is fragile—update the shell
snippet that sets HITS to robustly extract the last complete JSON object from
OUTPUT (not just the last line containing "{"), parse it, and read total from
either result.total or top-level total (falling back to 0 or a parse-error).
Locate the HITS assignment and replace the Python parsing logic (the inline
python block) so it finds the final balanced JSON object (e.g., scan from the
last '{' and match braces or extract JSON blocks), json.loads that object, then
compute hits = d.get('result', {}).get('total') or d.get('total', 0) and print
that; ensure non-JSON or parse failures still return "parse-error" as before.
🪄 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: e1000733-879f-4e1c-99ca-0c00964a68bc
📒 Files selected for processing (10)
scripts/bench-memory-retrieval.shscripts/bench-memory-walk.shsrc/openhuman/agent_memory/README.mdsrc/openhuman/agent_memory/mod.rssrc/openhuman/agent_memory/ops.rssrc/openhuman/agent_memory/tools.rssrc/openhuman/agent_memory/types.rssrc/openhuman/memory/query/smart_walk.rssrc/openhuman/tools/mod.rstests/agent_retrieval_e2e.rs
🚧 Files skipped from review as they are similar to previous changes (5)
- src/openhuman/agent_memory/mod.rs
- src/openhuman/tools/mod.rs
- src/openhuman/agent_memory/types.rs
- src/openhuman/agent_memory/ops.rs
- scripts/bench-memory-walk.sh
- Guard CONTENT_ROOT pipeline in bench script so missing users dir gets an explicit error instead of silent exit under set -euo pipefail - Clamp max_turns to [1, 20] to match schema minimum
… wiring test - Add agent_memory entry to RESOURCE_CATALOG so catalog_mirrors_builtins passes - Update presentation_agent grounding tools assertion: memory_tree/query_memory replaced by call_memory_agent
12 tasks done
senamakel
added a commit
to nocstah/openhuman
that referenced
this pull request
Sep 11, 2026
3 tasks done
M3gA-Mind
added a commit
to M3gA-Mind/openhuman
that referenced
this pull request
Sep 18, 2026
Clear every real KNOWN_UNCALLABLE row from the fleet prompt invariant. - Workspace section names shell/pwd only when shell is callable. - planner, profile_memory_agent, trigger_reactor: drop stale memory_recall/memory_forget mentions (tinyhumansai#3421 replaced recall with injected memory). - code_executor: GitHub state ops go back to the caller via Handoff Plan; keep the never-shell-gh rule. - morning_briefing, skill_executor, context_scout: name the packed tools by their use_skill route; record at the workflows pack why skill_executor/context_scout are not owners (run_skill closure). - tools_agent, tool_maker: name shell (and file_write for tool_maker) in their own prompt instead of relying on the shared sentence. - workflow_builder: its shell node kind becomes a collision row in place of the removed wildcard shell row.
M3gA-Mind
added a commit
to M3gA-Mind/openhuman
that referenced
this pull request
Sep 18, 2026
Clear every real KNOWN_UNCALLABLE row from the fleet prompt invariant. - Workspace section names shell/pwd only when shell is callable. - planner, profile_memory_agent, trigger_reactor: drop stale memory_recall/memory_forget mentions (tinyhumansai#3421 replaced recall with injected memory). - code_executor: GitHub state ops go back to the caller via Handoff Plan; keep the never-shell-gh rule. - morning_briefing, skill_executor, context_scout: name the packed tools by their use_skill route; record at the workflows pack why skill_executor/context_scout are not owners (run_skill closure). - tools_agent, tool_maker: name shell (and file_write for tool_maker) in their own prompt instead of relying on the shared sentence. - workflow_builder: its shell node kind becomes a collision row in place of the removed wildcard shell row.
M3gA-Mind
added a commit
to M3gA-Mind/openhuman
that referenced
this pull request
Sep 18, 2026
Clear every real KNOWN_UNCALLABLE row from the fleet prompt invariant. - Workspace section names shell/pwd only when shell is callable. - planner, profile_memory_agent, trigger_reactor: drop stale memory_recall/memory_forget mentions (tinyhumansai#3421 replaced recall with injected memory). - code_executor: GitHub state ops go back to the caller via Handoff Plan; keep the never-shell-gh rule. - morning_briefing, skill_executor, context_scout: name the packed tools by their use_skill route; record at the workflows pack why skill_executor/context_scout are not owners (run_skill closure). - tools_agent, tool_maker: name shell (and file_write for tool_maker) in their own prompt instead of relying on the shared sentence. - workflow_builder: its shell node kind becomes a collision row in place of the removed wildcard shell row.
M3gA-Mind
added a commit
to M3gA-Mind/openhuman
that referenced
this pull request
Sep 18, 2026
Clear every real KNOWN_UNCALLABLE row from the fleet prompt invariant. - Workspace section names shell/pwd only when shell is callable. - planner, profile_memory_agent, trigger_reactor: drop stale memory_recall/memory_forget mentions (tinyhumansai#3421 replaced recall with injected memory). - code_executor: GitHub state ops go back to the caller via Handoff Plan; keep the never-shell-gh rule. - morning_briefing, skill_executor, context_scout: name the packed tools by their use_skill route; record at the workflows pack why skill_executor/context_scout are not owners (run_skill closure). - tools_agent, tool_maker: name shell (and file_write for tool_maker) in their own prompt instead of relying on the shared sentence. - workflow_builder: its shell node kind becomes a collision row in place of the removed wildcard shell row.
M3gA-Mind
added a commit
to M3gA-Mind/openhuman
that referenced
this pull request
Sep 18, 2026
Clear every real KNOWN_UNCALLABLE row from the fleet prompt invariant. - Workspace section names shell/pwd only when shell is callable. - planner, profile_memory_agent, trigger_reactor: drop stale memory_recall/memory_forget mentions (tinyhumansai#3421 replaced recall with injected memory). - code_executor: GitHub state ops go back to the caller via Handoff Plan; keep the never-shell-gh rule. - morning_briefing, skill_executor, context_scout: name the packed tools by their use_skill route; record at the workflows pack why skill_executor/context_scout are not owners (run_skill closure). - tools_agent, tool_maker: name shell (and file_write for tool_maker) in their own prompt instead of relying on the shared sentence. - workflow_builder: its shell node kind becomes a collision row in place of the removed wildcard shell row.
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
agent_memorydomain withcall_memory_agentorchestrator tool for delegated memory retrievalmemory_loader.rsfromagent/into the new domain following canonical module shapememory_recall/memory_tree/query_memorywithcall_memory_agentacross all 9 agentsProblem
memory_loader.rs) lived in the genericagent/directory with no dedicated domain boundary{"name":"...","arguments":{...}}), but chat-tier LLMs sometimes emit XML-style calls (<tool_name>...</tool_name><parameters>...</parameters>). These were silently dropped, causing the walk to exit after one turn with no evidence.Solution
src/openhuman/agent_memory/domain withmod.rs,types.rs,ops.rs,tools.rs,memory_loader.rs, and embedded agent definition (agent/agent.toml+prompt.md)parse_single_tool_call()with JSON-first + XML fallback parsing, andextract_xml_tag()helpercall_memory_agenttool to the orchestrator's tool surface for delegated memory retrievalmemory_recall/memory_tree/query_memorywithcall_memory_agentin all 9 agents (orchestrator, planner, researcher, help, presentation_agent, profile_memory_agent, crypto_agent, markets_agent, trigger_reactor)scripts/bench-memory-retrieval.sh,scripts/bench-memory-walk.sh)Submission Checklist
Impact
Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
pnpm --filter openhuman-app format:checkpnpm typecheck— Rust-only changescargo test smart_walk— 19 passed, 6 ignored (staging-only)Validation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Parity Contract
Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Improvements