Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions src/memory/retrieval/fast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ fn resolve_local(
let mut hits = Vec::new();
for (id, _) in ordered {
if let Some(mut hit) = by_id.remove(&id) {
if source_scope.is_some_and(|scope| !scope.contains(&hit.tree_scope)) {
if source_scope.is_some_and(|scope| !source_scope_allows(scope, &hit.tree_scope)) {
continue;
}
hit.score = coverage.get(&id).copied().unwrap_or_default();
Expand Down Expand Up @@ -239,7 +239,9 @@ async fn dense(
)
.await?;
if let Some(scope) = source_scope {
response.hits.retain(|hit| scope.contains(&hit.tree_scope));
response
.hits
.retain(|hit| source_scope_allows(scope, &hit.tree_scope));
}
let total = response.hits.len();
response.hits.truncate(limit);
Expand Down Expand Up @@ -282,6 +284,19 @@ fn dedup_ids(ids: impl Iterator<Item = String>) -> Vec<String> {
ids.filter(|id| seen.insert(id.clone())).collect()
}

fn source_scope_allows(scope: &HashSet<String>, tree_scope: &str) -> bool {
if scope.contains(tree_scope) {
return true;
}
extract_mem_src_id(tree_scope).is_some_and(|id| scope.contains(id))
}

fn extract_mem_src_id(value: &str) -> Option<&str> {
let rest = value.strip_prefix("mem_src:")?;
let (id, _) = rest.split_once(':')?;
(!id.is_empty()).then_some(id)
}
Comment on lines +287 to +298

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Missing test coverage for mem_src path in fast retrieval

source_scope_allows and extract_mem_src_id are not exercised by any test in fast_tests.rs. The existing tests (dense_fallback_filters_scope_before_limit, resolve_local_hydrates_leaf_and_summary_and_skips_missing_or_out_of_scope) always use scopes like "slack:#allowed" that match via the direct scope.contains(tree_scope) branch, so they never reach the new extract_mem_src_id code path. A test seeding a tree with scope mem_src:src_vault_hob_local:path/to/note.md and a source_scope containing only "src_vault_hob_local" would confirm the core fix behaves correctly end-to-end through fast_retrieve.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Comment on lines +294 to +298

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Case-sensitivity inconsistency with scope_matches_kind

extract_mem_src_id uses a case-sensitive strip_prefix("mem_src:"), while the parallel function scope_matches_kind in source.rs normalises the scope via to_lowercase() before checking lower.starts_with("mem_src:"). A tree_scope stored with any capitalisation variant (e.g. Mem_Src:...) would be classified as document kind by scope_matches_kind but would silently fall through the fast-retrieval filter because extract_mem_src_id returns None. Lowercasing the prefix portion before the strip_prefix call would align both paths.


#[cfg(test)]
#[path = "fast_tests.rs"]
mod tests;
3 changes: 3 additions & 0 deletions src/memory/retrieval/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,9 @@ fn scope_matches_kind(scope: &str, kind_prefix: &str) -> bool {
if lower.starts_with(&format!("{kind_prefix}:")) {
return true;
}
if kind_prefix == SourceKind::Document.as_str() && lower.starts_with("mem_src:") {
return true;
}
PLATFORM_KINDS
.iter()
.any(|(platform, kind)| *kind == kind_prefix && lower.starts_with(&format!("{platform}:")))
Expand Down
4 changes: 4 additions & 0 deletions src/memory/retrieval/source_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,10 @@ fn scope_prefix_matching_known_platforms() {
assert!(scope_matches_kind("gmail:alice", "email"));
assert!(scope_matches_kind("notion:page123", "document"));
assert!(scope_matches_kind("linear:conn-1:issue-abc", "document"));
assert!(scope_matches_kind(
"mem_src:src-folder-9:Slides_Notes/example.md",
"document"
));
assert!(!scope_matches_kind("slack:#eng", "email"));
assert!(scope_matches_kind("chat:custom", "chat"));
}