Skip to content

feat(memory): add memory_search module with vector, hybrid, and context tools - #3444

Merged
senamakel merged 4 commits into
tinyhumansai:mainfrom
senamakel-droid:feat/memory-search-module
Jun 6, 2026
Merged

senamakel merged 4 commits into
tinyhumansai:mainfrom
senamakel-droid:feat/memory-search-module

Conversation

@senamakel-droid

@senamakel-droid senamakel-droid commented Jun 6, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Add new src/openhuman/memory_search/ domain module consolidating search/retrieval tools for the memory agent.
  • Implement three new tools: memory_vector_search (direct cosine similarity), memory_chunk_context (neighboring chunk expansion), and memory_hybrid_search (configurable weight profiles over unified memory).
  • Add MMR (Maximal Marginal Relevance) algorithm for diversity-aware result selection.
  • Re-export existing memory_store tools (raw_search, raw_chunks, kinds) through memory_search and register them in the agent tool list.

Problem

  • The memory agent lacked direct semantic search — vector search was only available through the heavy smart_walk LLM loop.
  • Existing retrieval tools (raw_search, raw_chunks, kinds) were defined but never registered in the agent tool list.
  • No diversity mechanism existed to prevent redundant near-duplicate results.
  • No way to tune the scoring balance between vector similarity, keyword matching, graph relevance, and freshness.

Solution

  • memory_vector_search: Pure cosine similarity over stored chunk embeddings. Single embedding call, no LLM. Supports source_kind/time_window_days filters, min_score threshold, and optional MMR diversity mode.
  • memory_chunk_context: Given a chunk_id, returns surrounding chunks from the same source for context expansion (configurable window size).
  • memory_hybrid_search: Exposes UnifiedMemory with four weight profiles (balanced, semantic, lexical, graph_first) and optional per-signal score breakdown.
  • MMR (vector/mmr.rs): Iterative selection maximizing λ·relevance − (1−λ)·max_sim(c, selected) with 5 unit tests.
  • Scoring (scoring.rs): Weight profile definitions with normalization and 5 unit tests.
  • Dependency direction is one-way: memory_search → memory_store, embeddings, config. No circular dependencies.

Submission Checklist

  • Tests added or updated (happy path + at least one failure / edge case) — 10 inline unit tests across mmr.rs and scoring.rs
  • N/A: Diff coverage — new module with unit tests for algorithmic code; tool execute paths require live config/DB
  • N/A: Coverage matrix updated — internal agent tools, no user-facing feature change
  • N/A: All affected feature IDs — no feature matrix row affected
  • No new external network dependencies introduced
  • N/A: Manual smoke checklist — no release-cut surface change
  • N/A: Linked issue closed — no linked issue

Impact

  • Desktop only (agent tools run in Rust core).
  • No performance regression — new tools are additive; existing paths unchanged.
  • No migration needed — reads existing mem_tree_chunk_embeddings and UnifiedMemory stores.
  • Security: tools inherit existing SecurityPolicy gating via the agent tool harness.

Related

  • Follow-up PR(s)/TODOs: Move remaining scattered retrieval code (memory/query/, memory_store/tools/) into memory_search for full consolidation.

AI Authored PR Metadata (required for Codex/Linear PRs)

Linear Issue

  • Key: N/A
  • URL: N/A

Commit & Branch

  • Branch: feat/memory-search-module
  • Commit SHA: 85bba69

Validation Run

  • pnpm --filter openhuman-app format:check
  • N/A: pnpm typecheck — no frontend changes
  • Focused tests: cargo test --lib -- memory_search (build succeeded, tests compile)
  • Rust fmt/check: cargo check --manifest-path Cargo.toml — 0 errors, 0 warnings from new code
  • N/A: Tauri fmt/check — no Tauri shell changes

Validation Blocked

  • command: N/A
  • error: N/A
  • impact: N/A

Behavior Changes

  • Intended behavior change: Agent gains 6 new memory search tools (3 new + 3 previously unregistered)
  • User-visible effect: Memory agent can perform direct vector search, expand chunk context, and run hybrid queries with tunable scoring profiles

Parity Contract

  • Legacy behavior preserved: All existing tools and code paths unchanged
  • Guard/fallback/dispatch parity checks: N/A — additive only

Duplicate / Superseded PR Handling

  • Duplicate PR(s): None
  • Canonical PR: This one
  • Resolution: N/A

Summary by CodeRabbit

  • New Features

    • Unified memory search module exposing multiple agent-invocable retrieval tools.
    • Hybrid search combining vector, keyword, graph, and freshness signals with selectable scoring profiles (Balanced, Semantic, Lexical, Graph-First).
    • Vector semantic search with filtering, scoring thresholds, and optional diversity via MMR.
    • Memory chunk context retrieval showing neighboring chunks and markers.
    • New tools registered for default system access.
  • Tests

    • Unit tests added for scoring presets and MMR selection.

…ity, and hybrid retrieval tools

Introduces src/openhuman/memory_search/ as the consolidated entry point
for agent-facing retrieval tools. Three new tools give the memory agent
direct access to the embedding layer without requiring the heavy
smart_walk LLM loop:

- memory_vector_search: pure cosine-similarity search over chunk
  embeddings with optional MMR diversity, metadata filtering, and
  time-window support.
- memory_chunk_context: expands a chunk_id with neighboring chunks
  from the same source for full conversation/document context.
- memory_hybrid_search: exposes the existing hybrid retrieval engine
  with configurable weight profiles (balanced, semantic, lexical,
  graph_first).

Also registers three previously-unregistered tools from memory_store
(memory_store_raw_search, memory_store_raw_chunks, memory_store_kinds)
in the agent tool list.

New infrastructure:
- vector/mmr.rs: Maximal Marginal Relevance algorithm for diversity-
  aware result selection.
- scoring.rs: WeightProfile constants and compose_score() for hybrid
  re-scoring.
@senamakel-droid
senamakel-droid requested a review from a team June 6, 2026 18:52
@coderabbitai

coderabbitai Bot commented Jun 6, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a top-level memory_search module exposing scoring profiles, MMR/vector utilities, three agent tools (vector search, hybrid search, chunk context), re-exports vector store helpers and existing memory-store/query tools, and registers the tools in the runtime registry.

Changes

Memory Search Module with Hybrid Retrieval Tools

Layer / File(s) Summary
Scoring framework for hybrid retrieval
src/openhuman/memory_search/scoring.rs
WeightProfile struct with preset constants (BALANCED, SEMANTIC, LEXICAL, GRAPH_FIRST), from_name lookup, and compose_score method that weights graph relevance, vector similarity, keyword relevance, and freshness into a composite score. Includes unit tests validating preset sums and score computation.
MMR and vector module surface
src/openhuman/memory_search/vector/mmr.rs, src/openhuman/memory_search/vector/mod.rs, src/openhuman/memory_search/vector/store.rs
Implements mmr_select, MmrCandidate, and MmrResult with tests; exposes vector submodules and re-exports store helpers (bytes_to_vec, vec_to_bytes, cosine_similarity) and SearchResult/VectorStore.
Vector search with MMR diversity selection
src/openhuman/memory_search/tools/vector_search.rs, src/openhuman/memory_search/tools/mod.rs
MemoryVectorSearchTool performs semantic search by embedding the query, fetching candidate chunks, computing cosine similarity, filtering by min_score, and optionally applying MMR selection (mmr_select) when diverse is true.
Chunk context retrieval tool
src/openhuman/memory_search/tools/chunk_context.rs
MemoryChunkContextTool retrieves a target chunk, lists chunks from the same source sorted by seq_in_source, and returns a bounded neighbor window (default 2, clamped to 1–5) with the target marked.
Hybrid multi-signal search tool
src/openhuman/memory_search/tools/hybrid_search.rs
MemoryHybridSearchTool queries unified memory and re-scores results using WeightProfile composition of four signals (graph relevance, vector similarity, keyword relevance, freshness). Filters non-positive scores, sorts by composite score, and optionally includes per-signal breakdown in output.
Module consolidation and registry integration
src/openhuman/memory_search/mod.rs, src/openhuman/memory_search/tools/mod.rs, src/openhuman/mod.rs, src/openhuman/tools/mod.rs, src/openhuman/tools/ops.rs
Adds memory_search module and submodules, re-exports new tool types and existing memory-store/query tools behind unified paths, and registers the memory_search tools in the tool registry.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant VectorSearchTool
  participant EmbeddingProvider
  participant ChunkStore
  participant MmrSelect

  Client->>VectorSearchTool: execute(query, namespace, limit, diverse)
  VectorSearchTool->>EmbeddingProvider: embed(query)
  EmbeddingProvider-->>VectorSearchTool: query_embedding
  VectorSearchTool->>ChunkStore: list_chunks(namespace, filters)
  ChunkStore-->>VectorSearchTool: candidate_chunks
  VectorSearchTool->>ChunkStore: batch_load_embeddings(chunk_ids)
  ChunkStore-->>VectorSearchTool: embeddings
  VectorSearchTool->>VectorSearchTool: compute_cosine_similarity(per chunk)
  VectorSearchTool->>VectorSearchTool: filter_by_min_score
  alt diverse enabled and candidates > limit
    VectorSearchTool->>MmrSelect: mmr_select(query_vec, candidates, limit, lambda)
    MmrSelect-->>VectorSearchTool: selected_results
  else
    VectorSearchTool->>VectorSearchTool: top_k_sort
  end
  VectorSearchTool-->>Client: formatted_results
Loading
sequenceDiagram
  participant Client
  participant HybridSearchTool
  participant UnifiedMemory
  participant WeightProfile

  Client->>HybridSearchTool: execute(query, namespace, mode, limit, include_breakdown)
  HybridSearchTool->>UnifiedMemory: query_namespace_hits(query, limit)
  UnifiedMemory-->>HybridSearchTool: hits_with_breakdown
  loop For each hit
    HybridSearchTool->>WeightProfile: compose_score(graph, vector, keyword, freshness)
    WeightProfile-->>HybridSearchTool: composite_score
  end
  HybridSearchTool->>HybridSearchTool: filter_positive_scores
  HybridSearchTool->>HybridSearchTool: sort_by_score_desc
  HybridSearchTool->>HybridSearchTool: truncate_to_limit
  HybridSearchTool-->>Client: formatted_results_with_optional_breakdown
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • tinyhumansai/openhuman#3382: Adds or modifies tool registry exports similar to this PR's additions to src/openhuman/tools and tool registration.

Suggested reviewers

  • sanil-23

Poem

🐰 A unified search, so neat and bright,
Scoring signals in their composite might,
MMR hops to pick a varied set,
Hybrid ranks what we mustn't forget—
Memory retrieval, bunny-approved delight! 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 63.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: a new memory_search module with three key tools (vector, hybrid, and context search).
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot added feature Net-new user-facing capability or product behavior. rust-core Core Rust runtime in src/: CLI, core_server, shared infrastructure. agent Built-in agents, prompts, orchestration, and agent runtime in src/openhuman/agent/. memory Memory store, memory tree, recall, summarization, and embeddings in src/openhuman/memory/. labels Jun 6, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🧹 Nitpick comments (7)
src/openhuman/tools/mod.rs (1)

33-33: ⚡ Quick win

Narrow this re-export to tool types only.

pub use crate::openhuman::memory_search::*; also re-exports non-tool items (scoring, vector, and module-level symbols) into openhuman::tools. Prefer re-exporting memory_search::tools::* (or explicit tool structs) to keep the tool surface clean.

Suggested change
-pub use crate::openhuman::memory_search::*;
+pub use crate::openhuman::memory_search::tools::*;

As per coding guidelines: “Place domain-owned agent tools in that domain's tools.rs, re-export via src/openhuman/tools/mod.rs.”

🤖 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/tools/mod.rs` at line 33, The current wildcard re-export in
mod.rs exposes non-tool symbols from crate::openhuman::memory_search (e.g.,
scoring, vector, module-level items); narrow it to only tool types by replacing
the glob with a re-export of the memory_search::tools module or explicit tool
structs (e.g., pub use crate::openhuman::memory_search::tools::*; or pub use
crate::openhuman::memory_search::MemorySearchTool, OtherTool) so
openhuman::tools only exposes agent tool types referenced by the domain.

Sources: Coding guidelines, Learnings

src/openhuman/memory_search/scoring.rs (1)

40-48: ⚡ Quick win

Consider case-insensitive matching for profile names.

The from_name method is case-sensitive. If a caller passes "Balanced" or "BALANCED", it will return None and fall back to the default profile. Adding .to_lowercase() would make this more robust:

 pub fn from_name(name: &str) -> Option<Self> {
-    match name {
+    match name.to_lowercase().as_str() {
         "balanced" => Some(Self::BALANCED),
🤖 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_search/scoring.rs` around lines 40 - 48, The from_name
method currently does a case-sensitive match and will return None for inputs
like "Balanced" or "BALANCED"; update from_name to perform a case-insensitive
comparison (prefer using name.eq_ignore_ascii_case(...) to avoid allocating)
before matching so variants like Self::BALANCED, Self::SEMANTIC, Self::LEXICAL,
and Self::GRAPH_FIRST are recognized regardless of input case.
src/openhuman/memory_search/vector/mmr.rs (2)

87-87: ⚡ Quick win

Remove unused query_vec parameter.

The query_vec parameter is never used in the function body (line 87 suppresses the warning). Since MmrCandidate already contains pre-computed relevance, the query vector is not needed here. Removing it would make the signature clearer:

 pub fn mmr_select(
-    query_vec: &[f32],
     candidates: &[MmrCandidate<'_>],
     limit: usize,
     lambda: f64,
 ) -> Vec<MmrResult> {

And remove line 87 entirely. Update all call sites to remove the query_vec argument.

🤖 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_search/vector/mmr.rs` at line 87, The function signature
still includes an unused parameter named query_vec and a suppression line let _
= (query_vec, &selected); — remove the query_vec parameter from the function
declaration in mmr.rs and delete the suppression line, since MmrCandidate
already carries precomputed relevance; then update every call site that passes
query_vec to instead call the function with the remaining arguments (refer to
the function name that accepts MmrCandidate/selected and the MmrCandidate struct
to locate usages) so compilation succeeds without the unused-parameter
workaround.

32-89: ⚡ Quick win

Add debug logging for MMR selection flow.

Per coding guidelines, new/changed flows should include debug logging with entry/exit, branches, and results. Add logging before returning to track selection behavior:

log::debug!(
    "[memory_search][mmr] selected {} items from {} candidates (lambda={:.2}, limit={})",
    results.len(),
    candidates.len(),
    lambda,
    limit
);

As per coding guidelines: "Include debug logging with verbose diagnostics on all new/changed flows: log entry/exit, branches, external calls, retries/timeouts, state transitions, and errors with stable grep-friendly prefixes."

🤖 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_search/vector/mmr.rs` around lines 32 - 89, Add debug
logging to the mmr_select flow: emit an entry log at the start of mmr_select
(include query_vec length or presence, candidates.len(), lambda and limit), and
emit the provided exit log just before returning results to record how many
items were selected, total candidates, lambda and limit (use the existing lambda
and limit variables and results.len()). Place the entry log near the top of
mmr_select and the exit log immediately before the final return so both
entry/exit are present and grep-friendly (refer to function name mmr_select,
variables query_vec, candidates, lambda, limit, and results).

Source: Coding guidelines

src/openhuman/memory_search/tools/hybrid_search.rs (1)

149-167: ⚡ Quick win

Log warning if NaN scores are detected.

Line 166 uses unwrap_or(Equal) which treats NaN scores as equal during sorting. If compose_score produces NaN (e.g., from NaN inputs in score breakdown), results will be silently misordered. Add explicit NaN detection:

 let mut rescored: Vec<(usize, f64)> = hits
     .iter()
     .enumerate()
     .map(|(i, hit)| {
         let bd = &hit.score_breakdown;
         let score = profile.compose_score(
             bd.graph_relevance,
             bd.vector_similarity,
             bd.keyword_relevance,
             bd.freshness,
         );
+        if score.is_nan() {
+            log::warn!(
+                "[tool][memory_hybrid_search] NaN score for hit {} (bd: g={} v={} k={} f={})",
+                i, bd.graph_relevance, bd.vector_similarity, bd.keyword_relevance, bd.freshness
+            );
+        }
         (i, score)
     })
     .filter(|(_, score)| *score > 0.0)
     .collect();
🤖 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_search/tools/hybrid_search.rs` around lines 149 - 167,
The sorting currently uses unwrap_or(Equal) which hides NaNs; before sorting the
Vec<(usize, f64)> named rescored (populated via profile.compose_score using
hit.score_breakdown fields), detect any score.is_nan() values, log a warning
with context (e.g., which hit index or score_breakdown produced NaN), and either
filter those entries out or replace NaN with a defined minimal score; then
perform rescored.sort_by using partial_cmp safely. Update the mapping/filtering
that builds rescored (and the use of compose_score) to perform this NaN check so
NaNs are not silently treated as equal during the sort.
src/openhuman/memory_search/tools/vector_search.rs (2)

202-208: ⚡ Quick win

Fragile unwrap on MMR index lookup.

Line 205 uses .unwrap() to look up the score from scored by index. While this should always succeed given the current logic, it's fragile and could panic if the relationship between scored and mmr_results indices changes. Consider making this safer:

 mmr_results
     .into_iter()
     .map(|r| {
-        (
-            r.index,
-            scored.iter().find(|(i, _, _)| *i == r.index).unwrap().1,
-        )
+        let score = scored.iter().find(|(i, _, _)| *i == r.index)
+            .map(|(_, s, _)| *s)
+            .unwrap_or(r.score);
+        (r.index, score)
     })
     .collect::<Vec<_>>()

Or better yet, use the score already in r.score from MmrResult:

-mmr_results
-    .into_iter()
-    .map(|r| {
-        (
-            r.index,
-            scored.iter().find(|(i, _, _)| *i == r.index).unwrap().1,
-        )
-    })
-    .collect::<Vec<_>>()
+mmr_results.into_iter().map(|r| (r.index, r.score)).collect()

But note: this changes the output to use MMR scores instead of original relevance scores. Verify if that's the intended behavior.

🤖 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_search/tools/vector_search.rs` around lines 202 - 208,
The current mapping in vector_search.rs uses scored.iter().find(...).unwrap(),
which can panic if the lookup fails; replace this fragile unwrap by either using
the MmrResult's own score (r.score) when mapping (if you intend to return MMR
scores), or perform a safe lookup: use scored.iter().find(...).map(|(_, s, _)|
*s).unwrap_or_else(|| /* fallback value or skip entry */) or match the Option
and handle the None case (e.g., skip the entry or return an error). Update the
closure that produces (r.index, ...) to reference r.score or to safely handle
missing entries to avoid panics.

199-199: ⚡ Quick win

Consider making MMR lambda configurable.

The lambda value is hard-coded to 0.7. Since this controls the relevance-diversity tradeoff, exposing it as an optional parameter would give users more control. For example:

#[serde(default = "default_lambda")]
lambda: f64,

And in the schema:

"lambda": {
    "type": "number",
    "minimum": 0.0,
    "maximum": 1.0,
    "description": "MMR diversity parameter: 1.0=pure relevance, 0.0=pure diversity (default 0.7)."
}
🤖 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_search/tools/vector_search.rs` at line 199, The call to
mmr_select currently hardcodes the diversity lambda (mmr_select(&query_vec,
&candidates, limit, 0.7)); make the lambda configurable by adding a field (e.g.,
lambda: f64) with a serde default function (fn default_lambda() -> f64 { 0.7 })
on the struct that owns this logic, validate/clamp it to [0.0,1.0] if needed,
and pass that field into mmr_select instead of 0.7; also update the component's
JSON schema to include a "lambda" numeric property with minimum 0.0, maximum 1.0
and the described documentation string so users can adjust the MMR
relevance-diversity tradeoff.
🤖 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_search/mod.rs`:
- Around line 8-10: The module currently only re-exports scoring, tools, and
vector; update src/openhuman/memory_search/mod.rs to follow the canonical domain
layout by exporting the full sibling set: declare pub mod types; pub mod store;
pub mod ops; pub mod schemas; pub mod tools; pub mod bus; and keep existing pub
mod scoring; pub mod vector; (or rename/merge if content differs) so the module
surface matches the project convention and other domain modules (ensure symbols
like types, store, ops, schemas, tools, bus, scoring, vector are all publicly
re-exported).

In `@src/openhuman/memory_search/tools/chunk_context.rs`:
- Around line 61-155: Add unit tests covering the memory_chunk_context tool's
execute method (the async fn execute in chunk_context.rs) by creating a
#[cfg(test)] mod tests that constructs controlled ListChunksQuery/list_chunks
responses (mock or inject a test config) and calls execute with serialized Args;
include tests for chunk-not-found (expect error), window clamping (verify
parsed.window values <1 and >5 clamp to 1 and 5), boundary windows (target at
first and last positions) and short-source behavior (source with fewer chunks
than window) asserting returned ToolResult contents and markers for target
chunk. Ensure tests reference the Args struct and exercise sorting by
seq_in_source and the output formatting (positions/start/end and "<<<" marker)
so the behavior in execute, list_chunks usage, and window calculation are
validated.
- Around line 100-122: The code silently defaults target_pos to 0 when
parsed.chunk_id is not found in source_chunks (ListChunksQuery + list_chunks)
which can happen because limit is fixed to 500; change this to explicitly handle
the missing target: after computing target_pos from
source_chunks.iter().position(|c| c.id == parsed.chunk_id) return an error
(e.g., anyhow) if None, or modify the fetching logic to ensure the target chunk
is included — e.g., fetch the target chunk separately by id (or
expand/parameterize the ListChunksQuery limit or query by seq_in_source) and
then compute the window from the combined results; update references to
parsed.chunk_id, target_pos, ListChunksQuery and list_chunks accordingly so the
function never silently uses position 0.
- Around line 83-95: Replace the broad list-and-search pattern currently using
ListChunksQuery / list_chunks and scanning for parsed.chunk_id with a direct
call to the existing get_chunk API: call get_chunk(&config, &parsed.chunk_id),
handle the Result and map None to the same error previously returned
("memory_chunk_context: chunk_id not found") and propagate other errors
similarly (e.g., with anyhow::anyhow!); remove the ListChunksQuery, list_chunks
call, and the .find(...) logic so the code uses get_chunk to fetch the target
chunk directly.

In `@src/openhuman/memory_search/tools/hybrid_search.rs`:
- Around line 98-209: Add unit tests for memory_hybrid_search's execute
function: create a #[cfg(test)] mod tests that constructs Args and calls execute
(via the Tool interface or by invoking the function path) to cover empty
query/namespace validation (expect Err), unknown mode mapping to
WeightProfile::BALANCED, score filtering so hits with non-positive or NaN
composite scores are excluded (simulate hits or mock
UnifiedMemory/query_namespace_hits to return controllable score_breakdown
values), and include_breakdown toggling to verify breakdown text appears only
when true; reference the execute method, Args struct, WeightProfile::from_name/
BALANCED, UnifiedMemory::query_namespace_hits, and score composition logic to
locate code to mock/stub for tests.

In `@src/openhuman/memory_search/tools/vector_search.rs`:
- Around line 98-246: Add unit tests for the memory_vector_search tool by
creating a #[cfg(test)] mod tests in the same file (or sibling *_tests.rs) that
exercises execute (via constructing Args and calling the async method or
wrapping it in a runtime). Cover: empty query validation (expect error),
min_score filtering (prepare chunks + embeddings so some scores fall below/above
parsed.min_score), limit clamping (set limit <1 and >50 and assert clamped
behavior), both MMR diversity and non-diverse paths (set parsed.diverse
true/false and verify selection uses mmr_select vs simple sort), and filters:
source_kind and time_window_days (construct ListChunksQuery-like inputs / stub
list_chunks and get_chunk_embeddings_for_signature_batch to return controlled
chunks/embeddings). Use mocks or temporary test helpers to stub
provider_from_config, list_chunks, and get_chunk_embeddings_for_signature_batch
and assert expected ToolResult outputs.

In `@src/openhuman/tools/ops.rs`:
- Around line 221-228: After registering the six memory_search tools
(MemoryVectorSearchTool, MemoryChunkContextTool, MemoryHybridSearchTool,
MemoryStoreRawSearchTool, MemoryStoreRawChunksTool, MemoryStoreKindsTool) add a
debug log statement immediately after that block that emits a stable,
grep-friendly prefix (e.g., "MEMORY_SEARCH_REGISTERED:") followed by the list of
registered tool names or a concise summary; update the same
function/registration area in ops.rs so runtime diagnostics show this event
whenever the memory_search tools are registered.

---

Nitpick comments:
In `@src/openhuman/memory_search/scoring.rs`:
- Around line 40-48: The from_name method currently does a case-sensitive match
and will return None for inputs like "Balanced" or "BALANCED"; update from_name
to perform a case-insensitive comparison (prefer using
name.eq_ignore_ascii_case(...) to avoid allocating) before matching so variants
like Self::BALANCED, Self::SEMANTIC, Self::LEXICAL, and Self::GRAPH_FIRST are
recognized regardless of input case.

In `@src/openhuman/memory_search/tools/hybrid_search.rs`:
- Around line 149-167: The sorting currently uses unwrap_or(Equal) which hides
NaNs; before sorting the Vec<(usize, f64)> named rescored (populated via
profile.compose_score using hit.score_breakdown fields), detect any
score.is_nan() values, log a warning with context (e.g., which hit index or
score_breakdown produced NaN), and either filter those entries out or replace
NaN with a defined minimal score; then perform rescored.sort_by using
partial_cmp safely. Update the mapping/filtering that builds rescored (and the
use of compose_score) to perform this NaN check so NaNs are not silently treated
as equal during the sort.

In `@src/openhuman/memory_search/tools/vector_search.rs`:
- Around line 202-208: The current mapping in vector_search.rs uses
scored.iter().find(...).unwrap(), which can panic if the lookup fails; replace
this fragile unwrap by either using the MmrResult's own score (r.score) when
mapping (if you intend to return MMR scores), or perform a safe lookup: use
scored.iter().find(...).map(|(_, s, _)| *s).unwrap_or_else(|| /* fallback value
or skip entry */) or match the Option and handle the None case (e.g., skip the
entry or return an error). Update the closure that produces (r.index, ...) to
reference r.score or to safely handle missing entries to avoid panics.
- Line 199: The call to mmr_select currently hardcodes the diversity lambda
(mmr_select(&query_vec, &candidates, limit, 0.7)); make the lambda configurable
by adding a field (e.g., lambda: f64) with a serde default function (fn
default_lambda() -> f64 { 0.7 }) on the struct that owns this logic,
validate/clamp it to [0.0,1.0] if needed, and pass that field into mmr_select
instead of 0.7; also update the component's JSON schema to include a "lambda"
numeric property with minimum 0.0, maximum 1.0 and the described documentation
string so users can adjust the MMR relevance-diversity tradeoff.

In `@src/openhuman/memory_search/vector/mmr.rs`:
- Line 87: The function signature still includes an unused parameter named
query_vec and a suppression line let _ = (query_vec, &selected); — remove the
query_vec parameter from the function declaration in mmr.rs and delete the
suppression line, since MmrCandidate already carries precomputed relevance; then
update every call site that passes query_vec to instead call the function with
the remaining arguments (refer to the function name that accepts
MmrCandidate/selected and the MmrCandidate struct to locate usages) so
compilation succeeds without the unused-parameter workaround.
- Around line 32-89: Add debug logging to the mmr_select flow: emit an entry log
at the start of mmr_select (include query_vec length or presence,
candidates.len(), lambda and limit), and emit the provided exit log just before
returning results to record how many items were selected, total candidates,
lambda and limit (use the existing lambda and limit variables and
results.len()). Place the entry log near the top of mmr_select and the exit log
immediately before the final return so both entry/exit are present and
grep-friendly (refer to function name mmr_select, variables query_vec,
candidates, lambda, limit, and results).

In `@src/openhuman/tools/mod.rs`:
- Line 33: The current wildcard re-export in mod.rs exposes non-tool symbols
from crate::openhuman::memory_search (e.g., scoring, vector, module-level
items); narrow it to only tool types by replacing the glob with a re-export of
the memory_search::tools module or explicit tool structs (e.g., pub use
crate::openhuman::memory_search::tools::*; or pub use
crate::openhuman::memory_search::MemorySearchTool, OtherTool) so
openhuman::tools only exposes agent tool types referenced by the domain.
🪄 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: 33f40400-b29f-4e4c-8677-e68c51aa316f

📥 Commits

Reviewing files that changed from the base of the PR and between 7fa3c21 and 55d7490.

📒 Files selected for processing (12)
  • src/openhuman/memory_search/mod.rs
  • src/openhuman/memory_search/scoring.rs
  • src/openhuman/memory_search/tools/chunk_context.rs
  • src/openhuman/memory_search/tools/hybrid_search.rs
  • src/openhuman/memory_search/tools/mod.rs
  • src/openhuman/memory_search/tools/vector_search.rs
  • src/openhuman/memory_search/vector/mmr.rs
  • src/openhuman/memory_search/vector/mod.rs
  • src/openhuman/memory_search/vector/store.rs
  • src/openhuman/mod.rs
  • src/openhuman/tools/mod.rs
  • src/openhuman/tools/ops.rs

Comment thread src/openhuman/memory_search/mod.rs
Comment thread src/openhuman/memory_search/tools/chunk_context.rs
Comment thread src/openhuman/memory_search/tools/chunk_context.rs Outdated
Comment thread src/openhuman/memory_search/tools/chunk_context.rs
Comment thread src/openhuman/memory_search/tools/hybrid_search.rs
Comment thread src/openhuman/memory_search/tools/vector_search.rs
Comment thread src/openhuman/tools/ops.rs
…p_or(0) bug, add debug log

- chunk_context: use get_chunk() for direct ID lookup instead of listing 1000 chunks
- chunk_context: replace unwrap_or(0) with proper error when target not in source
- ops: add debug log after memory_search tool registration
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 6, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/tools/ops.rs`:
- Around line 475-477: After extending the tool registry with scratchpad tools
via
tools.extend(crate::openhuman::subconscious::scratchpad::tools::all_scratchpad_tools()),
add a debug log immediately after that call using the existing logger (or the
module's debug macro) with a stable, grep-friendly prefix like
"SCRATCHPAD-TOOLS-REGISTERED:" and include concise diagnostics such as the count
of tools registered and optionally their identifiers; target the insertion point
in src/openhuman/tools/ops.rs right after the tools.extend(...) line and
reference the functions/tools symbol names (tools.extend and
all_scratchpad_tools) so the message clearly ties to this flow.
🪄 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: 0578a67d-8c00-4eae-b0f5-47f1cc85fc65

📥 Commits

Reviewing files that changed from the base of the PR and between 0ec8018 and cc44e08.

📒 Files selected for processing (1)
  • src/openhuman/tools/ops.rs

Comment thread src/openhuman/tools/ops.rs
@senamakel
senamakel merged commit e445aa4 into tinyhumansai:main Jun 6, 2026
21 of 22 checks passed
senamakel pushed a commit to nocstah/openhuman that referenced this pull request Sep 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent Built-in agents, prompts, orchestration, and agent runtime in src/openhuman/agent/. feature Net-new user-facing capability or product behavior. memory Memory store, memory tree, recall, summarization, and embeddings in src/openhuman/memory/. rust-core Core Rust runtime in src/: CLI, core_server, shared infrastructure.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants