fix(memory): handle mem_src scopes in retrieval - #124
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR adds an asynchronous retrieval test for dense fallback scope filtering. The test verifies that a bare ChangesDense fallback scope filtering
Estimated code review effort: 2 (Simple) | ~5 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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. Comment |
|
Ready for review. This is the focused mem_src retrieval fix split out from #123 so OpenHuman source-scoped retrieval can depend on a small, independently reviewable TinyCortex change. |
|
| Filename | Overview |
|---|---|
| src/memory/retrieval/fast.rs | Replaces direct HashSet containment checks with source_scope_allows, adds extract_mem_src_id; new mem_src code path is untested and has a case-sensitivity mismatch vs. source.rs |
| src/memory/retrieval/source.rs | Adds mem_src: prefix recognition inside scope_matches_kind so source trees with composite mem_src scopes are classified as document kind; logic is correct and narrowly scoped |
| src/memory/retrieval/source_tests.rs | Adds a single assertion for mem_src scope in scope_prefix_matching_known_platforms; covers the source.rs change but not the fast.rs change |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[fast_retrieve called\nwith source_scope] --> B{entity_ids\nempty?}
B -- yes --> C[dense path:\nquery_source no filter]
B -- no --> D{graph pairs\nfound?}
D -- no --> E[global_occurrence\ncalls dense]
D -- yes --> F[resolve_local\nfrom candidates]
C --> G[source_scope_allows\nfor each hit]
E --> G
F --> G
G --> H{scope.contains\ntree_scope?}
H -- yes --> I[Allow hit]
H -- no --> J{mem_src: prefix?}
J -- yes --> K[extract_mem_src_id]
K --> L{scope.contains\ncollection_id?}
L -- yes --> I
L -- no --> M[Reject hit]
J -- no --> M
Reviews (1): Last reviewed commit: "Handle mem_src scopes in source retrieva..." | Re-trigger Greptile
Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
# Conflicts: # src/memory/retrieval/fast.rs # src/memory/retrieval/source.rs # src/memory/retrieval/source_tests.rs
Summary
mem_src:<source_id>:<item>scopes as document/source scopes during source retrievalmem_srccollection idsmem_srcdocument scope classificationWhy
OpenHuman stores reader-backed source trees with scopes such as
mem_src:src_vault_hob_local:path/to/note.md. Source-scoped retrieval can be asked for the collection id (src_vault_hob_local), so TinyCortex needs to recognize the collection id embedded inside the compositemem_srctree scope. Without this, host source retrieval can miss indexed summaries even when the source is correctly ingested.Current state (updated)
Upstream #125 already shipped the
source_scope_allows/extract_mem_src_idimplementation and thesource.rskind classification onmain. This branch has since been merged withmainand is conflict-free. The surviving delta vsmainis the end-to-endfast_retrieveregression test (dense_fallback_filters_by_mem_src_collection_idinfast_tests.rs) that exercises the collection-id-onlysource_scopepath through the dense retrieval pipeline — the piece #125's unit-only coverage did not include.Validation
cargo fmt --all -- --checkcleancargo test --lib memory::retrieval— 99 passed / 0 failedcargo test --lib dense_fallback_filters_by_mem_src_collection_id— passesRelation to PR #123
This is a focused PR for only the
mem_srcretrieval fix. The same change was previously pushed onto the broader legacy chunk repair branch in #123, but keeping it separate makes the OpenHuman source-retrieval dependency easier to review and merge independently.