feat(memory): multi-query expansion with error-safe recall#2592
Merged
theonlyhennygod merged 2 commits intomainfrom Mar 5, 2026
Merged
feat(memory): multi-query expansion with error-safe recall#2592theonlyhennygod merged 2 commits intomainfrom
theonlyhennygod merged 2 commits intomainfrom
Conversation
Contributor
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Memory Retrieval Enhancement src/memory/retrieval.rs, src/memory/mod.rs |
New module introducing enhanced_recall() function that wraps Memory::recall() with multi-query expansion. Extracts significant keywords (β₯4 characters) from queries β₯30 chars, performs secondary recall, merges results by deduplicating on key (highest score preserved), sorts by score descending, and returns top-N entries. Includes keyword extraction utility, merge logic, and comprehensive test suite covering keyword parsing, deduplication, and integration scenarios. |
Retrieval Integration src/agent/loop_/context.rs, src/agent/memory_loader.rs |
Updated two retrieval call sites to use enhanced_recall() instead of direct mem.recall(). context.rs gates on early return if no entries found; memory_loader.rs adds new test helper FailingRecallMemory to verify error propagation from primary memory backend. Maintains downstream processing (time-decay, Core boost, min_relevance filtering, truncation, context emission). |
Sequence Diagram(s)
sequenceDiagram
actor Agent
participant ER as enhanced_recall()
participant Mem as Memory (Primary)
participant KE as Keyword Extractor
participant Mem2 as Memory (Secondary)
participant Merger as Dedup & Sort
Agent->>ER: enhanced_recall(query, limit)
ER->>Mem: recall(full query)
Mem-->>ER: primary_results
alt query length >= 30 chars
ER->>KE: extract_keywords(query)
KE-->>ER: keyword string
alt keywords differ from original
ER->>Mem2: recall(keywords)
Mem2-->>ER: secondary_results
ER->>Merger: merge_entries(primary, secondary)
else keywords same as original
Merger-->>ER: primary_results
end
else short query
Merger-->>ER: primary_results
end
Merger->>Merger: sort by score descending
Merger->>Merger: truncate to limit
Merger-->>ER: merged_results
ER-->>Agent: sorted_entries
Estimated code review effort
π― 3 (Moderate) | β±οΈ ~20 minutes
Possibly related issues
- [Feature]: Core memory category boost in context retrievalΒ #2378: Modifies memory retrieval in
build_context()andDefaultMemoryLoaderwith Core-category score boost during post-retrieval ranking; this PR refactors those same call sites to use enhanced multi-query recall before downstream re-ranking.
Suggested labels
size: S, risk: medium, agent
Suggested reviewers
- chumyin
π₯ Pre-merge checks | β 4 | β 1
β Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Description check | The PR description covers key sections (Summary, Why, Validation, linked issues) but omits most required template fields (Label Snapshot, Change Metadata, Security Impact, Compatibility, i18n, Human Verification, Side Effects, Rollback Plan, Risks). | Complete the PR description using the repository template: add Label Snapshot (risk/size/scope/module labels), Change Metadata, Security Impact, Compatibility/Migration, Human Verification, Side Effects/Blast Radius, Rollback Plan, and Risks/Mitigations sections. |
β Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | β Passed | The title 'feat(memory): multi-query expansion with error-safe recall' accurately summarizes the main feature: multi-query keyword expansion for memory retrieval with error-safe behavior. |
| Linked Issues check | β Passed | The code changes directly implement all requirements from #2472: enhanced_recall with primary/secondary queries, keyword extraction, deduplication, sorting, truncation, integration into both retrieval paths, and error-safe behavior. |
| Out of Scope Changes check | β Passed | All changes align with #2472 scope: memory retrieval module, keyword expansion logic, integration points, and regression tests. No unrelated modifications detected. |
| Docstring Coverage | β Passed | Docstring coverage is 88.24% which is sufficient. The required threshold is 80.00%. |
βοΈ Tip: You can configure your own custom pre-merge checks in the settings.
β¨ Finishing Touches
- π Generate docstrings (stacked PR)
- π Generate docstrings (commit on current branch)
π§ͺ Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
issue-2472-enhanced-recall-safe
Comment @coderabbitai help to get the list of available commands and usage tips.
PR intake checks found warnings (non-blocking)Fast safe checks found advisory issues. CI lint/test/build gates still enforce merge quality.
Action items:
Detected Linear keys: none Run logs: https://github.com/zeroclaw-labs/zeroclaw/actions/runs/22601223025 Detected blocking line issues (sample):
Detected advisory line issues (sample):
|
2 tasks
github-actions
Bot
added
agent
|
Thanks for contributing to ZeroClaw. For faster review, please ensure:
See |
github-actions
Bot
added
size: M
![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: 1
π§Ή Nitpick comments (1)
src/memory/retrieval.rs (1)
28-30: Make secondary recall fallback observableAt Line 28, secondary recall failures are silently swallowed. Keeping primary results is correct, but logging this fallback will make production debugging safer.
π‘ Proposed improvement
- if let Ok(extra) = mem.recall(&keywords, limit, session_id).await { - merge_entries(&mut results, extra); - } + match mem.recall(&keywords, limit, session_id).await { + Ok(extra) => merge_entries(&mut results, extra), + Err(err) => { + tracing::debug!( + error = %err, + "keyword expansion recall failed; continuing with primary recall results" + ); + } + }As per coding guidelines, "document fallback behavior when fallback is intentional and safe".
π€ Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/memory/retrieval.rs` around lines 28 - 30, The secondary recall call to mem.recall(&keywords, limit, session_id).await currently ignores Err cases; update the block around mem.recall so that failures are logged (including error details and context like keywords/ session_id/limit) while still preserving primary results and calling merge_entries(&mut results, extra) on Ok; reference the mem.recall invocation and merge_entries/results to locate the change and use the existing logging facility (or add one nearby) to record the fallback error.
π€ Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/memory/retrieval.rs`:
- Around line 25-52: The length checks use byte counts instead of character
counts: replace usages of query.len() in the expansion check (the block that
compares to MIN_EXPANSION_LENGTH) and clean.len() inside fn extract_keywords to
use character counts (query.chars().count() and clean.chars().count()) so
Unicode characters (e.g., emoji, CJK) are measured correctly; keep the same
comparisons to MIN_EXPANSION_LENGTH and MIN_KEYWORD_LENGTH and preserve the
surrounding logic in extract_keywords and the recall/merge flow.
---
Nitpick comments:
In `@src/memory/retrieval.rs`:
- Around line 28-30: The secondary recall call to mem.recall(&keywords, limit,
session_id).await currently ignores Err cases; update the block around
mem.recall so that failures are logged (including error details and context like
keywords/ session_id/limit) while still preserving primary results and calling
merge_entries(&mut results, extra) on Ok; reference the mem.recall invocation
and merge_entries/results to locate the change and use the existing logging
facility (or add one nearby) to record the fallback error.
βΉοΈ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
π Files selected for processing (4)
src/agent/loop_/context.rssrc/agent/memory_loader.rssrc/memory/mod.rssrc/memory/retrieval.rs
Comment on lines
+25
to
+52
| if query.len() >= MIN_EXPANSION_LENGTH { | ||
| let keywords = extract_keywords(query); | ||
| if !keywords.is_empty() && keywords != query.trim() { | ||
| if let Ok(extra) = mem.recall(&keywords, limit, session_id).await { | ||
| merge_entries(&mut results, extra); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Sort by score descending, take top `limit` | ||
| results.sort_by(|a, b| { | ||
| b.score | ||
| .unwrap_or(0.0) | ||
| .partial_cmp(&a.score.unwrap_or(0.0)) | ||
| .unwrap_or(std::cmp::Ordering::Equal) | ||
| }); | ||
| results.truncate(limit); | ||
|
|
||
| Ok(results) | ||
| } | ||
|
|
||
| /// Extract significant keywords (length >= 4) from a message. | ||
| fn extract_keywords(msg: &str) -> String { | ||
| msg.split_whitespace() | ||
| .filter_map(|w| { | ||
| let clean = w.trim_matches(|c: char| !c.is_alphanumeric()); | ||
| if clean.len() >= MIN_KEYWORD_LENGTH { | ||
| Some(clean) |
Contributor
There was a problem hiding this comment.
𧩠Analysis chain
π Script executed:
# First, let's examine the exact code at the flagged lines
head -60 src/memory/retrieval.rs | tail -40Repository: zeroclaw-labs/zeroclaw
Length of output: 1244
π Script executed:
# Find the constant definitions
rg -n 'MIN_EXPANSION_LENGTH|MIN_KEYWORD_LENGTH' src/memory/retrieval.rs -B2 -A2Repository: zeroclaw-labs/zeroclaw
Length of output: 782
π Script executed:
# Check if there are any Unicode/non-ASCII related tests
rg -n 'unicode|emoji|cjk|multibyte|non.?ascii|utf|δΈζ|ζ₯ζ¬θͺ|νκ΅μ΄|π¨π»πΆπ°πΌπ±π²' src/memory/retrieval.rsRepository: zeroclaw-labs/zeroclaw
Length of output: 48
π Script executed:
# Look at the complete extract_keywords function
sed -n '47,60p' src/memory/retrieval.rsRepository: zeroclaw-labs/zeroclaw
Length of output: 430
Use character-based thresholds instead of byte length
Constants MIN_EXPANSION_LENGTH and MIN_KEYWORD_LENGTH are documented as character-based, but lines 25 and 51 use len() which counts UTF-8 bytes. This causes incorrect behavior with non-ASCII input (e.g., emoji, CJK characters take 2β4 bytes per character but should count as 1).
Replace query.len() with query.chars().count() and clean.len() with clean.chars().count().
π€ Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/memory/retrieval.rs` around lines 25 - 52, The length checks use byte
counts instead of character counts: replace usages of query.len() in the
expansion check (the block that compares to MIN_EXPANSION_LENGTH) and
clean.len() inside fn extract_keywords to use character counts
(query.chars().count() and clean.chars().count()) so Unicode characters (e.g.,
emoji, CJK) are measured correctly; keep the same comparisons to
MIN_EXPANSION_LENGTH and MIN_KEYWORD_LENGTH and preserve the surrounding logic
in extract_keywords and the recall/merge flow.
This was referenced Mar 3, 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
1 participant
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
Why
mainValidation
cargo test --lib memory::retrieval::tests -- --nocapturecargo test --lib agent::memory_loader::tests -- --nocapturecargo test --lib agent::loop_::context::tests -- --nocaptureNo space left on device, preventing a final rerun in this worktree.Closes #2472
Supersedes #2473
Summary by CodeRabbit
New Features
Tests