Files
optimclaw/src/cli/memory.rs
T
86ae12747b feat: LRU embedding cache for workspace search (#1423)
* feat: LRU embedding cache for workspace search (#165)

Add CachedEmbeddingProvider that wraps any EmbeddingProvider with an
in-memory LRU cache keyed by SHA-256(model_name + text). This avoids
redundant HTTP calls when the same text is embedded multiple times
(common during reindexing and repeated searches).

- Cache uses HashMap + last_accessed tracking with manual LRU eviction
  (same pattern as llm::response_cache::CachedProvider)
- Lock is never held during HTTP calls to prevent blocking
- embed_batch() partitions into hits/misses and only fetches misses
- Default 10,000 entries (~58 MB for 1536-dim vectors)
- Configurable via EMBEDDING_CACHE_SIZE env var
- Workspace.with_embeddings() auto-wraps; with_embeddings_uncached()
  available for tests

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address review comments on embedding cache

- Validate embed_batch return count matches expected miss count
- Replace unwrap_or_default() with proper error propagation
- Fix batch eviction: run final eviction pass after insert to enforce cap
- Fix test: use different-length inputs to verify ordering correctness
- Reject EMBEDDING_CACHE_SIZE=0 in config validation (minimum is 1)

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: replace .expect() with proper error handling in embed_batch

The all-cache-hits early-return path used .expect("all cache hits") which
violates the project convention of no .unwrap()/.expect() in production
code. Replaced with the same ok_or_else pattern used in the normal path.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: clarify memory sizing docs and use saturating_add for eviction

- Update memory comments in embedding_cache.rs, config/embeddings.rs,
  and workspace/mod.rs to note the ~58 MB figure is payload-only
  (actual memory is higher due to HashMap/key/allocation overhead)
- Use saturating_add(1) instead of + 1 for eviction threshold to
  prevent overflow if max_entries is usize::MAX

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address Copilot review on embedding cache

- Avoid double-clone per miss in embed_batch: move embedding into
  results, clone only for the cache entry
- Evict per-insert instead of after all inserts to keep peak memory
  bounded during large batches
- Clamp max_entries to at least 1 in constructor to prevent unexpected
  eviction behavior when set to 0 via the public API

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: reduce embedding_cache module visibility to private

Types are already re-exported via `pub use`, so the module itself
doesn't need to be public. Reduces unnecessary API surface.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address serrrfirat review feedback on embedding cache

- Add TODO comment for O(n) LRU eviction scalability
- Add thundering herd note at lock release in embed()
- Warn when cache max_entries exceeds 100k
- Use with_embeddings_uncached() in integration test
- Add tests: error_does_not_pollute_cache, embed_batch_empty_input
- Update README with cache-aware with_embeddings() docs

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: prevent u32 wrapping in FailThenSucceedMock failure counter

fetch_sub(1) wraps to u32::MAX when called past zero, silently
breaking the mock for 3+ calls. Switch to load-then-store to avoid
the wrapping bug in both embed() and embed_batch().

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address Copilot and serrrfirat review findings on embedding cache

- Switch tokio::sync::Mutex to std::sync::Mutex (lock never held across
  .await — cheaper synchronous lock)
- Extract DEFAULT_EMBEDDING_CACHE_SIZE constant to avoid 10_000 duplication
  between EmbeddingCacheConfig and EmbeddingsConfig

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* test: add all-misses batch test for embedding cache

Adds embed_batch_all_misses test covering the case where every text in a
batch is a cache miss — fulfilling the commitment from serrrfirat's review.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* chore: trigger CI re-check after rebase

* fix: use raw [u8;32] cache keys and pre-allocate HashMap capacity

Address Copilot review findings:
- cache_key() now returns [u8; 32] instead of hex String, avoiding a
  64-byte allocation per lookup
- HashMap::with_capacity(max_entries) avoids incremental reallocation
- Fix pre-existing staging compilation error in cli/routines.rs
  (missing max_tool_rounds/use_tools fields)

[skip-regression-check]

* fix: make cache accessors sync and update doc for [u8;32] keys

Address Copilot review:
- len(), is_empty(), clear() are now sync since they only take a
  std::sync::Mutex lock with no .await points
- Update cache_size doc comment to reflect [u8;32] keys instead of
  String keys

[skip-regression-check]

* fix: remove clone_on_copy for [u8; 32] cache keys

[skip-regression-check]

* ci: add safety comments to test code for no-panics check

The CI no-panics grep check cannot distinguish test code inside
src/ files from production code. Add // safety: test annotations
to .unwrap(), .expect(), and assert!() calls in #[cfg(test)] modules.

* fix: correct cache doc and demote hit/miss logs to trace

- Fix misleading "String keys" in memory comment (cache uses [u8; 32])
- Demote per-request hit/miss logs from debug to trace to reduce noise
  on hot paths (batch summary stays at trace too)

* docs: add missing Arc import in workspace README example

* perf: batch eviction in embed_batch to avoid O(n×m) cost

Replace per-insert evict_lru call with a single evict_k_oldest pass
that computes eviction count upfront and removes the k oldest entries
in one O(n) scan. Avoids O(n×m) HashMap iterations while holding the
mutex during batch inserts.

* fix: cap batch cache inserts at max_entries and use O(n) selection

- evict_k_oldest now uses select_nth_unstable_by_key for O(n) average
  partial selection instead of O(n log n) full sort
- embed_batch caps cached entries at max_entries when misses exceed
  capacity, preventing the cache from growing unbounded
- Added test: batch_exceeding_capacity_respects_max_entries

* fix: flatten test assert for fmt compatibility

Shorten assert message to fit single line so cargo fmt doesn't
split the safety annotation onto a separate line.

* fix: address review feedback and improve embedding cache (takeover #235)

- Fix merge conflict: add missing allow_always field in PendingApproval
- Thread EmbeddingCacheConfig through CLI memory commands so they respect
  EMBEDDING_CACHE_SIZE instead of silently using default (fixes #235 review)
- Cap HashMap pre-allocation at min(max_entries, 1024) to avoid upfront
  memory waste at large cache sizes
- Fix FailThenSucceedMock race: replace load+store with atomic fetch_update
- Remove noisy '// safety: test' comments (40+ lines of diff noise)
- Fix collapsed lines from comment removal
- Simplify redundant Ok(...collect()?) to just collect()

Co-Authored-By: ztsalexey <[email protected]>
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* fix(embedding-cache): skip eviction on concurrent duplicate insert

When the lock is released for the HTTP call, another caller may insert
the same key. Re-check under lock and just update the existing entry
without evicting, avoiding unnecessary cache churn under concurrency.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

---------

Co-authored-by: ztsalexey <[email protected]>
Co-authored-by: Claude Opus 4.6 <[email protected]>
Co-authored-by: ztsalexey <[email protected]>
2026-03-19 13:37:55 -07:00

296 lines
8.2 KiB
Rust

//! Memory/workspace CLI commands.
//!
//! Exposes the workspace system for direct CLI use without starting the agent.
use std::io::Read;
use std::sync::Arc;
use clap::Subcommand;
use crate::workspace::{EmbeddingCacheConfig, EmbeddingProvider, SearchConfig, Workspace};
/// Run a memory command using the Database trait (works with any backend).
pub async fn run_memory_command_with_db(
cmd: MemoryCommand,
db: std::sync::Arc<dyn crate::db::Database>,
embeddings: Option<Arc<dyn EmbeddingProvider>>,
cache_config: EmbeddingCacheConfig,
) -> anyhow::Result<()> {
let mut workspace = Workspace::new_with_db("default", db);
if let Some(emb) = embeddings {
workspace = workspace.with_embeddings_cached(emb, cache_config);
}
match cmd {
MemoryCommand::Search { query, limit } => search(&workspace, &query, limit).await,
MemoryCommand::Read { path } => read(&workspace, &path).await,
MemoryCommand::Write {
path,
content,
append,
} => write(&workspace, &path, content, append).await,
MemoryCommand::Tree { path, depth } => tree(&workspace, &path, depth).await,
MemoryCommand::Status => status(&workspace).await,
}
}
#[derive(Subcommand, Debug, Clone)]
pub enum MemoryCommand {
/// Search workspace memory (hybrid full-text + semantic)
Search {
/// Search query
query: String,
/// Maximum number of results
#[arg(short, long, default_value = "5")]
limit: usize,
},
/// Read a file from the workspace
Read {
/// File path (e.g., "MEMORY.md", "daily/2024-01-15.md")
path: String,
},
/// Write content to a workspace file
Write {
/// File path (e.g., "notes/idea.md")
path: String,
/// Content to write (omit to read from stdin)
content: Option<String>,
/// Append instead of overwrite
#[arg(short, long)]
append: bool,
},
/// Show workspace directory tree
Tree {
/// Root path to start from
#[arg(default_value = "")]
path: String,
/// Maximum depth to traverse
#[arg(short, long, default_value = "3")]
depth: usize,
},
/// Show workspace status (document count, index health)
Status,
}
/// Run a memory command (PostgreSQL backend).
#[cfg(feature = "postgres")]
pub async fn run_memory_command(
cmd: MemoryCommand,
pool: deadpool_postgres::Pool,
embeddings: Option<Arc<dyn EmbeddingProvider>>,
cache_config: EmbeddingCacheConfig,
) -> anyhow::Result<()> {
let mut workspace = Workspace::new("default", pool);
if let Some(emb) = embeddings {
workspace = workspace.with_embeddings_cached(emb, cache_config);
}
match cmd {
MemoryCommand::Search { query, limit } => search(&workspace, &query, limit).await,
MemoryCommand::Read { path } => read(&workspace, &path).await,
MemoryCommand::Write {
path,
content,
append,
} => write(&workspace, &path, content, append).await,
MemoryCommand::Tree { path, depth } => tree(&workspace, &path, depth).await,
MemoryCommand::Status => status(&workspace).await,
}
}
async fn search(workspace: &Workspace, query: &str, limit: usize) -> anyhow::Result<()> {
let config = SearchConfig::default().with_limit(limit.min(50));
let results = workspace.search_with_config(query, config).await?;
if results.is_empty() {
println!("No results found for: {}", query);
return Ok(());
}
println!("Found {} result(s) for \"{}\":\n", results.len(), query);
for (i, result) in results.iter().enumerate() {
let score_bar = score_indicator(result.score);
println!("{}. [{}] (score: {:.3})", i + 1, score_bar, result.score);
// Show a content preview (first 200 chars)
let preview = truncate_content(&result.content, 200);
for line in preview.lines() {
println!(" {}", line);
}
println!();
}
Ok(())
}
async fn read(workspace: &Workspace, path: &str) -> anyhow::Result<()> {
match workspace.read(path).await {
Ok(doc) => {
println!("{}", doc.content);
}
Err(crate::error::WorkspaceError::DocumentNotFound { .. }) => {
anyhow::bail!("File not found: {}", path);
}
Err(e) => return Err(e.into()),
}
Ok(())
}
async fn write(
workspace: &Workspace,
path: &str,
content: Option<String>,
append: bool,
) -> anyhow::Result<()> {
let content = match content {
Some(c) => c,
None => {
// Read from stdin
let mut buf = String::new();
std::io::stdin().read_to_string(&mut buf)?;
buf
}
};
if append {
workspace.append(path, &content).await?;
println!("Appended to {}", path);
} else {
workspace.write(path, &content).await?;
println!("Wrote to {}", path);
}
Ok(())
}
async fn tree(workspace: &Workspace, path: &str, max_depth: usize) -> anyhow::Result<()> {
let root = if path.is_empty() { "." } else { path };
println!("{}/", root);
print_tree(workspace, path, "", max_depth, 0).await?;
Ok(())
}
async fn print_tree(
workspace: &Workspace,
path: &str,
prefix: &str,
max_depth: usize,
current_depth: usize,
) -> anyhow::Result<()> {
if current_depth >= max_depth {
return Ok(());
}
let entries = workspace.list(path).await?;
let count = entries.len();
for (i, entry) in entries.iter().enumerate() {
let is_last = i == count - 1;
let connector = if is_last { "└── " } else { "├── " };
let child_prefix = if is_last { " " } else { "│ " };
if entry.is_directory {
println!("{}{}{}/", prefix, connector, entry.name());
Box::pin(print_tree(
workspace,
&entry.path,
&format!("{}{}", prefix, child_prefix),
max_depth,
current_depth + 1,
))
.await?;
} else {
println!("{}{}{}", prefix, connector, entry.name());
}
}
Ok(())
}
async fn status(workspace: &Workspace) -> anyhow::Result<()> {
let all_paths = workspace.list_all().await?;
let file_count = all_paths.len();
// Count directories by collecting unique parent paths
let mut dirs: std::collections::HashSet<String> = std::collections::HashSet::new();
for path in &all_paths {
if let Some(parent) = path.rsplit_once('/') {
dirs.insert(parent.0.to_string());
}
}
println!("Workspace Status");
println!(" User: {}", workspace.user_id());
println!(" Files: {}", file_count);
println!(" Directories: {}", dirs.len());
// Check key files
let key_files = [
"MEMORY.md",
"HEARTBEAT.md",
"IDENTITY.md",
"SOUL.md",
"AGENTS.md",
"USER.md",
];
println!("\n Identity files:");
for path in &key_files {
let exists = workspace.exists(path).await.unwrap_or(false);
let marker = if exists { "+" } else { "-" };
println!(" [{}] {}", marker, path);
}
Ok(())
}
fn truncate_content(s: &str, max_len: usize) -> String {
if s.len() <= max_len {
s.to_string()
} else {
format!("{}...", &s[..max_len])
}
}
fn score_indicator(score: f32) -> &'static str {
if score > 0.8_f32 {
"=====>"
} else if score > 0.5_f32 {
"====>"
} else if score > 0.3_f32 {
"===>"
} else if score > 0.1_f32 {
"==>"
} else {
"=>"
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test_score_indicator() {
assert_eq!(score_indicator(0.9_f32), "=====>");
assert_eq!(score_indicator(0.6_f32), "====>");
assert_eq!(score_indicator(0.4_f32), "===>");
assert_eq!(score_indicator(0.2_f32), "==>");
assert_eq!(score_indicator(0.05_f32), "=>");
}
#[test]
fn test_truncate_content() {
assert_eq!(truncate_content("hello", 10), "hello");
assert_eq!(truncate_content("hello world", 5), "hello...");
}
}