diff --git a/PLAN.md b/PLAN.md index fc338e1..a105591 100644 --- a/PLAN.md +++ b/PLAN.md @@ -1,13 +1,74 @@ # Refactoring Plan to Reduce Code Duplication -## 1. Create Core Service Layer with Clear Boundaries +## Implementation Order: +1. Create core module structure and error types +2. Create common data structures +3. Update database layer for batch operations +4. Create core services with clear boundaries (synchronous) +5. Add async wrappers for API use +6. Refactor CLI modes to use services +7. Refactor REST API to use async services +8. Refactor MCP tools to use services +9. Create unified error handling +10. Add integration tests +11. Add performance optimization guidelines + +## 1. Create Core Module Structure and Error Types +**Files:** +- Add: `src/core/error.rs` +- Add: `src/core/mod.rs` + +**Functions:** +- Add: `CoreError` enum with comprehensive variants +- Add: Conversion traits for all error types + +**Reason:** Establish foundation for consistent error handling +**Implementation:** +- Define base error enum with conversions to all interface-specific error types +- Use `#[derive(thiserror::Error)]` for easy `Display` and `Error` implementations +- Provide user-friendly error messages with error codes + +## 2. Create Common Data Structures +**Files:** +- Add: `src/core/types.rs` + +**Functions:** +- Add: `ItemWithMeta` struct +- Add: `ItemWithContent` struct +- Add: `From` for `ItemWithMeta` +- Add: `From` for `ItemInfo` (API response) +- Add: Serialization/deserialization implementations + +**Reason:** Standardize data structures used across modes and APIs +**Implementation:** +- Define structs for common data types +- Include conversion functions from database types +- Add serialization/deserialization support for JSON/YAML +- Ensure all fields are properly documented + +## 3. Update Database Layer for Batch Operations +**Files:** +- Change: `src/db.rs` + +**Functions:** +- Add: `get_items_with_meta_batch` +- Add: `get_items_with_tags_batch` +- Add: `insert_item_with_meta_transaction` +- Add: `delete_item_with_meta_transaction` +- Change: Optimize existing queries for batch operations + +**Reason:** Support efficient batch operations needed by services +**Implementation:** +- Add functions to get multiple items with their metadata and tags +- Add batch insertion/updates for tags and metadata +- Add transaction support for atomic operations +- Optimize queries for common access patterns + +## 4. Create Core Service Layer with Clear Boundaries **Files:** - Add: `src/core/item_service.rs` -- Add: `src/core/async_item_service.rs` - Add: `src/core/compression_service.rs` - Add: `src/core/meta_service.rs` -- Add: `src/core/mod.rs` -- Add: `src/core/error.rs` **Functions:** - Add: `get_item_full` in `item_service.rs` @@ -16,7 +77,6 @@ - Add: `delete_item` in `item_service.rs` - Add: `get_compressed_content` in `compression_service.rs` - Add: `process_metadata` in `meta_service.rs` -- Add: Async wrappers for all core functions in `async_item_service.rs` **Reason:** Extract common business logic from modes and APIs into reusable services **Implementation:** @@ -24,116 +84,30 @@ - Services should return structured data, not format output - Handle compression, metadata, and database operations - Keep core services **synchronous** for CLI performance -- Provide async wrappers using `tokio::task::spawn_blocking` for API use - Use streaming for pipeline efficiency (process data in chunks) **Layer Division:** - **`db.rs`**: Low-level SQL operations (e.g., `insert_item`, `query_all_items`) - **`item_service.rs`**: Higher-level business logic (e.g., "get item with metadata and tags," "validate item before save") -- Example: - ```rust - // db.rs (low-level) - pub fn get_item_with_tags(conn: &Connection, id: i64) -> Result<(Item, Vec)> - // item_service.rs (high-level) - pub fn get_item_full(conn: &Connection, id: i64) -> Result { - let (item, tags) = db::get_item_with_tags(conn, id)?; - let meta = db::get_item_meta(conn, &item)?; - Ok(ItemWithMeta { item, tags, meta }) - } - ``` - -## 2. Thread Safety and Resource Management +## 5. Add Async Wrappers for API Use **Files:** -- Change: `src/modes/server/api/item.rs` -- Change: `src/modes/server/api/status.rs` -- Change: `src/modes/server/mcp/tools.rs` +- Add: `src/core/async_item_service.rs` **Functions:** - Add: `get_item_async` in `async_item_service.rs` - Add: `save_item_async` in `async_item_service.rs` -- Change: All API handlers to use async services +- Add: `list_items_async` in `async_item_service.rs` +- Add: `delete_item_async` in `async_item_service.rs` -**Reason:** Ensure services can be safely used in both synchronous and asynchronous contexts +**Reason:** Ensure services can be safely used in asynchronous contexts **Implementation:** - Document thread-safety guarantees for all core services -- Use `Arc>` or connection pooling for shared resources like database connections -- Provide examples for safe async/sync boundaries: - ```rust - pub async fn get_item_async( - state: &AppState, - id: i64, - ) -> Result { - let conn = state.db.clone(); // Arc> - tokio::task::spawn_blocking(move || { - let conn = conn.lock().unwrap(); - item_service::get_item_full(&conn, id) - }) - .await? - } - ``` +- Use `Arc>` or connection pooling for shared resources +- Provide examples for safe async/sync boundaries - Use `tokio::task::spawn_blocking` for CPU-bound or blocking I/O operations -- Benchmark thread pool sizes for CPU-bound tasks (e.g., compression) -## 3. Create Common Data Structures -**Files:** -- Add: `src/core/types.rs` - -**Functions:** -- Add: `From` for `ItemWithMeta` -- Add: `From` for `ItemInfo` (API response) -- Add: Serialization/deserialization implementations - -**Reason:** Standardize data structures used across modes and APIs -**Implementation:** -- Define structs for `Item`, `ItemWithContent`, `ItemWithMeta`, `Response` -- Include conversion functions from database types (`From`) -- Add serialization/deserialization support for JSON/YAML -- Ensure all fields are properly documented -- Use zero-copy patterns where possible (slicing instead of copying) - -## 4. Unified Error Handling with Conversions -**Files:** -- Add: `src/core/error.rs` -- Change: `src/modes/server/api/item.rs` -- Change: `src/modes/server/mcp/tools.rs` -- Change: All mode files (`get.rs`, `save.rs`, etc.) - -**Functions:** -- Add: `CoreError` enum with comprehensive variants -- Add: `From` for `StatusCode` -- Add: `From` for `ToolError` -- Add: `From` for `anyhow::Error` -- Change: All error handling to use `CoreError` - -**Reason:** Standardize error types across CLI, API, and MCP interfaces -**Implementation:** -- Define a base error enum (`CoreError`) with conversions to all interface-specific error types -- Example: - ```rust - #[derive(Debug, thiserror::Error)] - pub enum CoreError { - #[error("DB error: {0}")] - Database(#[from] rusqlite::Error), - #[error("IO error: {0}")] - Io(#[from] std::io::Error), - // ... - } - - // Auto-convert to HTTP status - impl From for StatusCode { - fn from(err: CoreError) -> Self { - match err { - CoreError::Database(_) => StatusCode::INTERNAL_SERVER_ERROR, - // ... - } - } - } - ``` -- Use `#[derive(thiserror::Error)]` for easy `Display` and `Error` implementations -- Provide user-friendly error messages with error codes for programmatic handling - -## 5. Refactor CLI Modes to Use Services +## 6. Refactor CLI Modes to Use Services **Files:** - Change: `src/modes/get.rs` - Change: `src/modes/save.rs` @@ -160,7 +134,7 @@ - Use synchronous services directly - Implement streaming for stdin/stdout to maintain pipeline performance -## 6. Refactor REST API to Use Async Services +## 7. Refactor REST API to Use Async Services **Files:** - Change: `src/modes/server/api/item.rs` - Change: `src/modes/server/api/status.rs` @@ -181,7 +155,7 @@ - Use common error handling with conversions to HTTP responses - Wrap synchronous service calls in `tokio::task::spawn_blocking` -## 7. Refactor MCP Tools to Use Services +## 8. Refactor MCP Tools to Use Services **Files:** - Change: `src/modes/server/mcp/tools.rs` @@ -199,13 +173,11 @@ - Use synchronous services directly (MCP is typically local/short-lived) - Standardize response format to match API/CLI -## 8. Create Common Error Handling +## 9. Create Unified Error Handling **Files:** -- Add: `src/core/error.rs` - Change: All files that handle errors **Functions:** -- Add: Comprehensive error handling in `core/error.rs` - Add: Conversion traits for all error types - Change: All error handling to use new error system @@ -223,25 +195,6 @@ - Provide user-friendly error messages - Include error codes for programmatic handling -## 9. Update Database Layer for Batch Operations -**Files:** -- Change: `src/db.rs` - -**Functions:** -- Add: `get_items_with_meta_batch` -- Add: `get_items_with_tags_batch` -- Add: `insert_item_with_meta_transaction` -- Add: `delete_item_with_meta_transaction` -- Change: Optimize existing queries for batch operations - -**Reason:** Support efficient batch operations needed by services -**Implementation:** -- Add functions to get multiple items with their metadata and tags -- Add batch insertion/updates for tags and metadata -- Add transaction support for atomic operations -- Optimize queries for common access patterns -- Ensure all batch operations are properly documented - ## 10. Add Integration Tests **Files:** - Add: `tests/integration/core_tests.rs` @@ -284,18 +237,6 @@ - Benchmark critical operations before/after refactoring - Document performance characteristics and tradeoffs -## Implementation Order: -1. Create core module structure and error types (`core/error.rs`, `core/types.rs`) -2. Implement core services with basic functionality (sync first) -3. Add async wrappers for API use -4. Refactor one mode (e.g., `get`) to use services and validate approach -5. Refactor corresponding API endpoints to use async services -6. Repeat for other modes and APIs -7. Refactor MCP tools to use services -8. Add comprehensive tests and benchmarks -9. Clean up removed code from original files -10. Document performance characteristics and tradeoffs - ## Benefits: - Reduced code duplication between CLI, API, and MCP - Easier maintenance with clear separation of concerns