# Neomemx Architecture Review
## Summary of the Project
**neomemx** is a high-performance memory library for AI agents built in Rust with Python bindings. It provides:
- **Semantic memory storage** with vector embeddings (ChromaDB, Milvus)
- **LLM-assisted fact extraction** and consolidation (Groq, OpenAI)
- **Multi-tenant scoping** (user/agent/session hierarchy)
- **Change history tracking** (SQLite)
- **Graph-based relationship extraction** (optional)
- **Reranking** for search results (optional HuggingFace)
The project follows a fluent builder API pattern and uses trait-based abstractions for pluggable providers.
---
## Strengths
### 1. **Clean Architecture & Separation of Concerns**
- Well-organized module structure (`core/`, `engine/`, `storage/`, `vector_store/`, etc.)
- Clear trait abstractions (`LlmBase`, `EmbeddingBase`, `VectorStoreBase`, `HistoryStore`)
- Domain-driven design with core types (`StoredFact`, `ScopeIdentifiers`, `ChangeLog`)
### 2. **Excellent Error Handling**
- Structured error codes with retryability hints (`ErrorCode`)
- Comprehensive error types using `thiserror`
- Good error context and suggestions
### 3. **Fluent Builder API**
- Intuitive API: `engine.store("...").with_scope(...).execute().await?`
- Consistent pattern across operations
- Good ergonomics for Rust users
### 4. **Multi-Tenant Safety**
- Hierarchical scoping (user → agent → session)
- Scope validation and filtering
- Proper isolation between tenants
### 5. **Observability**
- Uses `tracing` for structured logging
- Good debug/info logging in critical paths
### 6. **Test Coverage**
- Integration tests with proper setup/teardown
- Unit tests for core types
- Test helpers for engine creation
---
## Major Areas for Improvement
### 1. **Performance & Memory Issues**
#### Unnecessary Clones
- **`engine.rs:107`**: `previous_content.clone()` - could use references or move
- **`engine.rs:421`**: `StoredFact::new()` creates fact then mutates - inefficient
- **`engine.rs:476`**: Multiple `clone()` calls in `output_to_fact`
- **`query.rs:44`**: `scope: ScopeIdentifiers` - takes ownership when could borrow
- **`storage.rs:35`**: `scope: ScopeIdentifiers` - same issue
#### Inefficient Vector Operations
- **`vector_store/base.rs:71-75`**: `delete_batch` loops sequentially - should batch
- **`vector_store/base.rs:87-94`**: `get_batch` loops sequentially - should batch
- **`engine.rs:183`**: `list()` then `collect()` into `Vec<String>` - could stream
#### Unnecessary Allocations
- **`engine.rs:113-114`**: Creates `HashMap` for every update payload
- **`engine.rs:282-287`**: Creates new `HashMap` for each update in consolidation loop
- **`core/scope.rs:83-94`**: `to_filter_map()` allocates new HashMap every call
#### Inefficient String Operations
- **`core/fact.rs:78-79`**: Uses `format!()` for hex string - could use `hex` crate
- **`engine.rs:414`**: `Uuid::new_v4().to_string()` - could use `compact_str` or `&str` where possible
### 2. **Code Quality & Rust Best Practices**
#### Unnecessary `Arc` Cloning
- **`engine.rs:32-33`**: `Arc<EngineConfig>` and `Arc<dyn MemorySearch>` - `NeomemxEngine` is already `Clone`, but cloning `Arc` is cheap, so this is fine. However, consider if `Clone` is necessary.
#### Missing `Send + Sync` Bounds
- All trait objects properly have `Send + Sync`, which is good.
#### Error Handling Gaps
- **`engine.rs:468-474`**: Date parsing uses `unwrap_or_else` with fallback - should log warning
- **`sqlite.rs:72`**: `pragma_update` errors are ignored - should handle gracefully
- **`python.rs:130-136`**: `get()` method returns `None` - incomplete implementation
#### Missing Documentation
- Many public functions lack doc comments
- Module-level docs are sparse
- No examples in doc comments for complex operations
### 3. **API Design Issues**
#### Inconsistent Naming
- `store()` vs `add()` (Python uses `add`)
- `retrieve_all()` vs `get_all()` (Python uses `get_all`)
- `clear()` vs `delete_all()` (Python uses `delete_all`)
#### Missing API Methods
- **`python.rs:130`**: `get(memory_id)` not implemented
- **`python.rs:213`**: `update()` requires scope but Python API doesn't provide it
- **`python.rs:222`**: `delete()` same issue
#### Builder Pattern Issues
- **`query.rs:38`**: `query()` takes `impl Into<String>` but could accept `&str` for zero-copy
- **`storage.rs:23`**: Same issue with `content: impl Into<String>`
### 4. **Python Integration Problems**
#### Runtime Management
- **`python.rs:30`**: Each `PyMemory` creates its own `Runtime` - inefficient
- Should use a shared runtime or `tokio::runtime::Handle::current()`
#### GIL Safety
- **`python.rs:123-127`**: Uses `Python::with_gil()` correctly
- However, blocking async operations in `block_on` can deadlock if called from async Python code
#### Error Propagation
- All errors converted to `PyRuntimeError` - loses error codes
- Should preserve error types for Python users
#### Incomplete Implementation
- **`python.rs:67`**: TODO comment - builder pattern not supported
- **`python.rs:130-136`**: `get()` returns `None` always
- **`python.rs:213-220`**: `update()` and `delete()` not implemented
### 5. **Architecture & Design Issues**
#### Over-Engineering
- **`ScopeFilter`** vs **`ScopeIdentifiers`** - two similar types that could be unified
- **`MemoryKind`** enum defined but only `Semantic` is used
- **`GraphBackend`** trait exists but only `NoOpGraphBackend` implemented
#### Configuration Complexity
- **`EngineConfig`** has many `Arc<dyn Trait>` fields - hard to construct
- **`EngineBuilder`** helps but still verbose
- Default config creation in `engine.rs:533-574` is very long
#### Unused Code
- Many unused structs/methods (see linter warnings)
- `HuggingFaceEmbedding`, `OpenAIEmbedderConfig`, etc. defined but never used
- Suggests incomplete feature implementation
### 6. **Storage & Database Issues**
#### SQLite Performance
- **`sqlite.rs:76`**: Cache size hardcoded to 64MB - should be configurable
- **`sqlite.rs:72`**: WAL mode enabled but errors ignored
- No connection pooling - each operation locks mutex
#### History Store Design
- **`sqlite.rs:231-268`**: `get_history()` reconstructs `ChangeLog` with empty scope
- Loses scope information in history - should store it
### 7. **Testing & Documentation**
#### Missing Tests
- No tests for error paths
- No tests for edge cases (empty strings, very long content)
- No benchmarks for performance-critical paths
- No tests for Python bindings
#### Documentation Gaps
- README doesn't explain architecture
- No API documentation examples
- No migration guide for breaking changes
- No performance characteristics documented
---
## Detailed File-by-File Review
### `src/lib.rs`
**Status**: ✅ Good
- Clean module organization
- Good prelude module
- Proper feature gates
**Issues**:
- Missing module-level documentation examples
### `src/error.rs`
**Status**: ✅ Excellent
- Well-structured error codes
- Good retryability logic
- Comprehensive error types
**Issues**: None significant
### `src/engine/engine.rs`
**Status**: ⚠️ Needs Improvement
- **Line 107**: Unnecessary clone of `previous_content`
- **Line 183**: Inefficient `list()` + `collect()` pattern
- **Line 282-287**: Creates new HashMap in loop
- **Line 414**: UUID string allocation
- **Line 468-474**: Silent date parsing failures
- **Line 533-574**: Default config creation is too long - should be in builder
**Recommendations**:
- Extract default config to `EngineBuilder::default()`
- Use `Cow<str>` or references where possible
- Batch vector store operations
- Add logging for date parsing failures
### `src/engine/builder.rs`
**Status**: ✅ Good
- Clean builder pattern
- Good validation
**Issues**:
- Could provide `from_env()` helper for common configs
### `src/engine/query.rs`
**Status**: ✅ Good
- Clean builder API
**Issues**:
- **Line 44**: Takes ownership of `scope` - could use `&ScopeIdentifiers`
### `src/engine/storage.rs`
**Status**: ✅ Good
- Clean builder API
**Issues**:
- **Line 35**: Same ownership issue as query.rs
### `src/core/fact.rs`
**Status**: ✅ Good
- Well-designed core type
**Issues**:
- **Line 78-79**: `format!()` for hex - use `hex` crate or `format!("{:x}")` is fine but could be optimized
### `src/core/scope.rs`
**Status**: ⚠️ Needs Improvement
- **Line 83-94**: `to_filter_map()` allocates every call
- **`ScopeFilter`** and **`ScopeIdentifiers`** are redundant
**Recommendations**:
- Unify `ScopeFilter` and `ScopeIdentifiers`
- Consider `Cow<str>` for string fields
- Cache filter maps if frequently accessed
### `src/storage/sqlite.rs`
**Status**: ⚠️ Needs Improvement
- **Line 72**: Ignores pragma errors
- **Line 76**: Hardcoded cache size
- **Line 231-268**: Loses scope in history reconstruction
**Recommendations**:
- Handle pragma errors properly
- Make cache size configurable
- Store scope in history table
### `src/python.rs`
**Status**: ❌ Critical Issues
- **Line 30**: Creates new runtime per instance
- **Line 67**: TODO - incomplete builder support
- **Line 130-136**: `get()` not implemented
- **Line 213-220**: `update()`/`delete()` not implemented
- Error types lost in conversion
**Recommendations**:
- Use shared runtime or `Handle::current()`
- Implement missing methods
- Preserve error types in Python
- Add proper GIL handling for async operations
### `src/vector_store/base.rs`
**Status**: ⚠️ Needs Improvement
- **Line 71-75**: Sequential `delete_batch` - should be parallel/batched
- **Line 87-94**: Sequential `get_batch` - should be parallel/batched
**Recommendations**:
- Make default implementations use `futures::future::join_all` or similar
- Or mark as `unimplemented!()` and require trait implementors to provide efficient versions
### `src/vector_store/chroma.rs`
**Status**: ✅ Good
- Well-structured ChromaDB client
- Good error handling
**Issues**:
- Some repetitive code that could be extracted
### `src/search/mod.rs`
**Status**: ✅ Good
- Clean trait design
**Issues**:
- **`MemoryKind`** enum only has `Semantic` used - remove or implement others
### `src/extraction/extractor.rs`
**Status**: ✅ Good
- Clean trait design
- Good error handling
**Issues**: None significant
---
## Recommended Refactor Plan
### Phase 1: Critical Fixes (Week 1)
1. **Fix Python Bindings**
- [ ] Implement missing `get()`, `update()`, `delete()` methods
- [ ] Use shared runtime or `Handle::current()`
- [ ] Preserve error types in Python
- [ ] Add proper async/GIL handling
2. **Fix Performance Issues**
- [ ] Remove unnecessary clones in `engine.rs`
- [ ] Batch vector store operations
- [ ] Use references where possible in builders
3. **Fix SQLite Issues**
- [ ] Handle pragma errors properly
- [ ] Make cache size configurable
- [ ] Store scope in history table
### Phase 2: API Improvements (Week 2)
4. **Unify Scope Types**
- [ ] Merge `ScopeFilter` and `ScopeIdentifiers`
- [ ] Use `Cow<str>` for string fields
- [ ] Cache filter maps
5. **Improve Builder API**
- [ ] Accept `&str` where possible (zero-copy)
- [ ] Add `from_env()` helper
- [ ] Extract default config to builder
6. **Complete Missing Features**
- [ ] Implement `MemoryKind::Episodic` and `Procedural` or remove enum
- [ ] Implement `GraphBackend` properly or remove
- [ ] Clean up unused code
### Phase 3: Documentation & Testing (Week 3)
7. **Add Documentation**
- [ ] Module-level docs with examples
- [ ] Function-level docs
- [ ] Architecture documentation
- [ ] Performance characteristics
8. **Improve Tests**
- [ ] Add error path tests
- [ ] Add edge case tests
- [ ] Add Python binding tests
- [ ] Add benchmarks
### Phase 4: Polish (Week 4)
9. **Code Cleanup**
- [ ] Remove unused code (fix linter warnings)
- [ ] Standardize naming (store vs add, etc.)
- [ ] Add clippy lints to CI
10. **Performance Optimization**
- [ ] Profile and optimize hot paths
- [ ] Add caching where appropriate
- [ ] Optimize string operations
---
## Improved Architecture Diagram
```
┌─────────────────────────────────────────────────────────────┐
│ Public API │
│ ┌──────────────┐ ┌──────────────┐ ┌──────────────┐ │
│ │ Rust API │ │ Python API │ │ Builder API │ │
│ └──────┬───────┘ └──────┬───────┘ └──────┬───────┘ │
└─────────┼─────────────────┼─────────────────┼──────────────┘
│ │ │
└─────────────────┼─────────────────┘
│
┌──────────▼──────────┐
│ NeomemxEngine │
│ (Orchestration) │
└──────────┬──────────┘
│
┌────────────────────┼────────────────────┐
│ │ │
┌───────▼────────┐ ┌────────▼────────┐ ┌───────▼────────┐
│ Fact Storage │ │ Fact Extraction │ │ Fact Search │
│ │ │ │ │ │
│ ┌────────────┐ │ │ ┌──────────────┐ │ │ ┌────────────┐ │
│ │Extractor │ │ │ │ LLM Provider │ │ │ │Embedding │ │
│ │Consolidator│ │ │ │ (Groq/OpenAI)│ │ │ │Provider │ │
│ └────────────┘ │ │ └──────────────┘ │ │ └────────────┘ │
│ │ │ │ │ │
│ ┌────────────┐ │ │ │ │ ┌────────────┐ │
│ │Vector Store│ │ │ │ │ │Reranker │ │
│ │(Chroma/Milv│ │ │ │ │ │(Optional) │ │
│ └────────────┘ │ │ │ │ └────────────┘ │
│ │ │ │ │ │
│ ┌────────────┐ │ │ │ │ ┌────────────┐ │
│ │History │ │ │ │ │ │Graph │ │
│ │(SQLite) │ │ │ │ │ │(Optional) │ │
│ └────────────┘ │ │ │ │ └────────────┘ │
└────────────────┘ └──────────────────┘ └────────────────┘
```
### Key Improvements:
1. **Clear separation** between storage, extraction, and search
2. **Pluggable providers** via traits
3. **Optional features** (reranker, graph) clearly marked
4. **Single orchestration point** (`NeomemxEngine`)
---
## Additional Recommendations
### 1. **Add Metrics/Telemetry**
- Consider adding `metrics` crate for performance monitoring
- Track operation latencies, error rates, etc.
### 2. **Add Configuration File Support**
- Support TOML/YAML config files
- Environment variable overrides
- Validation on startup
### 3. **Add Migration System**
- Version database schemas
- Provide migration utilities
- Document breaking changes
### 4. **Improve CI/CD**
- Add GitHub Actions for:
- Clippy linting
- Format checking (rustfmt)
- Test coverage
- Python wheel building
- Documentation generation
### 5. **Consider Async Traits**
- Currently uses `async-trait` - consider waiting for native async traits (Rust 1.75+)
- Or use `async fn` in traits when stable
### 6. **Add Caching Layer**
- Cache embeddings for identical content
- Cache LLM responses (with TTL)
- Reduce API calls
### 7. **Add Batch Operations**
- `store_batch()` for multiple facts
- `search_batch()` for multiple queries
- More efficient than individual calls
---
## Conclusion
**neomemx** is a well-architected project with good separation of concerns and clean abstractions. The main issues are:
1. **Incomplete Python bindings** - critical for the project's goals
2. **Performance optimizations** - unnecessary clones and allocations
3. **Missing documentation** - especially for Python users
4. **Unused code** - suggests incomplete features
The refactor plan prioritizes critical fixes first, then API improvements, then polish. The architecture is sound and just needs refinement.
**Overall Grade: B+** (Good foundation, needs polish)