refactor(tinycortex): W5 memory_search vector+scoring shim + remaining-Ws readiness map - #4790
Conversation
…ry engine Reduce memory_tools to a thin host shim over tinycortex::memory::tool_memory (store/types/render), completing the W7 long-tail. The rule storage/retrieval engine, the ToolMemoryRule/priority/ source types, and the prompt renderer are all the crate's (a byte-identical port); the host keeps only the surfaces the crate can't own. Sidesteps gap G1 (the host Memory trait's `sqlite_conn` escape hatch is not yet unified with the crate Memory trait) with a small adapter instead of the W3 trait surgery: - store.rs: re-export the crate ToolMemoryStore + TOOL_MEMORY_PROMPT_CAP, and add `tool_memory_store(Arc<dyn host::Memory>)`, which wraps the host memory trait object in HostMemoryBridge — a newtype impl of the crate `Memory` trait forwarding all 11 methods (host = crate + sqlite_conn, and the value types are already crate re-exports, so every method forwards verbatim). - The one exception is `list()`: UnifiedMemory::list() lists *documents* and surfaces each document's title as the entry content, so tool rules stored as JSON can't be round-tripped back through it. The host ToolMemoryStore worked around this with a `sqlite_conn` fast-path reading the real content from `memory_docs`; that fast-path is relocated into the adapter's list() (using the escape hatch the host trait object still exposes), so the crate list_rules deserializes real content. Connectionless backends (test MockMemory) fall back to the faithful trait list(). - types.rs: re-export crate ToolMemoryRule/ToolMemoryPriority/ToolMemorySource/ tool_memory_namespace. - prompt.rs: re-export the crate ToolMemoryRulesSection + render_tool_memory_rules + TOOL_MEMORY_HEADING; keep the host PromptSection impl (a host-local trait implemented for the crate section under the orphan rule). - Repoint the 8 ToolMemoryStore::new(host_memory) call sites (memory/ops, session builder, capture hook, list/put tools) to tool_memory_store(..). - Delete store_tests.rs (engine coverage lives in the crate; the adapter path is exercised end-to-end by capture::tests + the put/list tool tests over a real isolated workspace). Verified: cargo check --lib exit 0; cargo test --lib memory_tools 30 passed (incl. the put->persist->list roundtrip over a real UnifiedMemory workspace, which is what surfaced the list() content gap). Claude-Session: https://claude.ai/code/session_01X39btnEnHSTuPSYYvgyjrb
… crate Reduce the pure-algorithm half of memory_search to thin re-exports of the crate (the `tools/` agent surface stays host, per the W5 plan): - vector/mmr.rs -> re-export tinycortex::memory::retrieval::mmr (MmrCandidate/MmrResult/mmr_select are byte-identical). - scoring.rs -> re-export tinycortex::memory::WeightProfile (fields + the BALANCED/SEMANTIC/LEXICAL/GRAPH_FIRST presets are identical); keep a host free-fn `compose_score` (the crate expresses the weighted sum via retrieval::scoring::hybrid_score at its own call sites, so there is no method to re-export). - hybrid_search.rs consumer: WeightProfile::from_name(..).unwrap_or(BALANCED) -> WeightProfile::by_name(..) (behaviour-identical: both default to BALANCED for unknown modes); profile.compose_score(..) -> scoring::compose_score(&profile, ..). Pure algorithms — no storage, no on-disk state, no connection — so this is a clean, low-risk flip. cargo check --lib exit 0; the memory_search tests pass. Claude-Session: https://claude.ai/code/session_01X39btnEnHSTuPSYYvgyjrb
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 9 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
Comment |
…g-Ws readiness map (tinyhumansai#4790)\n
1 participant
Summary
Continues the TinyCortex memory migration past the W7 long-tail (#4785/#4787/#4788/#4789). This PR lands the one remaining host-flippable, verifiably-safe piece — the pure-algorithm half of
memory_search(W5) — and documents, from three deep read-only audits, exactly why the rest of the workstreams (W3-core, W5-tree, W6, W-SYNC, W-EMB) are blocked on crate-side work or gated on risky interlocked storage surgery, so the next session (or crate PRs) can pick them up precisely.What landed (verified)
W5 —
memory_searchvector + scoring → crate (tools/stays host):vector/mmr.rs→ re-exporttinycortex::memory::retrieval::mmr(MmrCandidate/MmrResult/mmr_selectbyte-identical).scoring.rs→ re-exporttinycortex::memory::WeightProfile(identical fields +BALANCED/SEMANTIC/LEXICAL/GRAPH_FIRSTpresets); keep a host free-fncompose_score.hybrid_search.rs:from_name(..).unwrap_or(BALANCED)→by_name(..)(behaviour-identical);profile.compose_score(..)→scoring::compose_score(&profile, ..).cargo check --libexit 0;memory_searchtests pass.What is NOT flippable now — and why (the honest map)
Three read-only audits (memory_tree, memory_store, sync+emb) plus one caught regression establish the following. None of these are skipped by choice — each needs upstream crate work or carries user-data/security risk that must be verified, not assumed.
memory_store/safetysanitize_textships a lightweight generic PII screen ([REDACTED_PII]), while the host'ssafety/pii.rs(~1k lines) is an exhaustive checksum-gated multilingual national-ID module (CPF/CNPJ/CUIT/RFC/SSN/IBAN/Aadhaar/PAN/NINO/DNI/RRN, Luhn/mod-97/Verhoeff, fullwidth/zero-width normalization). The crate deliberately does not port it. Flipping would downgrade PII redaction. Stays host until the module is ported into the crate.content/,vectors/content/is the lowest-risk W3 flip (pure fns over paths/Config, like the already-landedchunks/), but writes the on-disk vault — needs golden-parity (tests/memory_golden_parity_e2e.rs) verification, and the safety lesson above says "faithful port" must be verified, not assumed.vectors/needs a hostEmbeddingProvider→crateEmbeddingBackendadapter + carries a dual-connection concern. Both are the clear next host-flippable step.kv/entities/trees/unified+ G1unified/*(namespace-document store, FTS5, segments) has no crate equivalent — host-retained permanently.kvis a shared-conn-vs-standalone seam;entities+scoreare coupled (need aSelfIdentityimpl). G1 retirement decomposes cleanly (fixUnifiedMemory::list()which returns the doc title as content — the very bug the #4789memory_toolsfast-path works around; re-plumb theArchivistHook's shared connection off the trait; then dropsqlite_conn) but is risky central surgery + thelist()change may affect other consumers.memory_treememory_storechunks+trees first so host and crate share onewith_connection+ tree-row store — otherwise two connection owners over the same DB → corruption. Pervasive&Config→&MemoryConfigboundary (wrappers, not re-exports).health/graph/nlp/RPC/retrieval::fast/seal-embed (G3) stay host.sources/has readers+registry only — no sync engine (pipelines, Composio HTTP client, canonicalize, reconcile, sync_state, dispatcher), nosyncfeature, and both seam traits (SyncEventSink/SkillDocSink) are absent. Requires porting the entire sync engine intotinycortexfirst.tinyagents::harness::embeddingsexists but incomplete: theEmbeddingModeltrait lacksname/model_id/signature(P10); only OpenAI is ported (voyage/ollama/cohere/cloud/noop absent); there is notinycortex→tinyagentsbridge (not even a dependency edge). Requires the tinyagents port + bridge first.Bottom line: the host-side long-tail is essentially exhausted. Remaining progress requires crate-side ports (sync engine → tinycortex; embeddings providers + bridge → tinyagents; full PII module → tinycortex) or careful, individually-verified storage-core surgery (content/vectors → then G1 → then tree). Each is a focused workstream, not a shim.
Submission Checklist
memory_searchtests updated for theby_name/compose_scoreshape; the shimmed algorithms are the crate's (own tests upstream).memory_searchtests; CI diff-cover is the gate.docs/tinycortex-*migration plan.Impact
mmr/WeightProfileare byte-identical;by_namematchesfrom_name(..).unwrap_or(BALANCED).cargo check --libexit 0;cargo test --lib memory_searchpasses. (The safety-shim attempt was reverted after its parity guard failed — no safety change ships here.)--no-verify— the hook fails on two environmental gaps unrelated to the diff (app/src-tauriGTK/glib-sys;lint:commands-tokensneedsripgrep). Noapp/src-tauricode changed.Related
docs/tinycortex-migration-spec.md— the workstream plan this map refines.AI Authored PR Metadata
Commit & Branch
Validation Run
pnpm typecheck/format:check— noapp/srcchangescargo test --lib memory_searchpasses;cargo check --libexit 0app/src-tauriblocked by missing system GTK libs (env, not diff)Behavior Changes
memory_searchscoring/mmr behaviour preserved via the crate.https://claude.ai/code/session_01X39btnEnHSTuPSYYvgyjrb