From 0bc62878bb854b2aa19309606341096b152b38f9 Mon Sep 17 00:00:00 2001 From: Layne Penney Date: Sun, 8 Feb 2026 05:40:18 -0600 Subject: [PATCH 1/2] feat(phase-2): implement file cleanup and fix warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove unused load_session function from main.rs - Add get_all_files() method to SymbolDatabase - Implement cleanup_deleted() to remove stale file entries - Update PRODUCTION_READINESS_PLAN with Phase 2 progress Progress on Phase 2: - ✅ Clean build with zero warnings - ✅ HIGH priority TODO: File cleanup for deleted/renamed files --- codi-rs/docs/PRODUCTION_READINESS_PLAN.md | 154 +++++---- codi-rs/src/main.rs | 10 +- codi-rs/src/symbol_index/database.rs | 380 ++++++++++++++-------- codi-rs/src/symbol_index/indexer.rs | 36 +- 4 files changed, 361 insertions(+), 219 deletions(-) diff --git a/codi-rs/docs/PRODUCTION_READINESS_PLAN.md b/codi-rs/docs/PRODUCTION_READINESS_PLAN.md index dd1891bf..82da5ca4 100644 --- a/codi-rs/docs/PRODUCTION_READINESS_PLAN.md +++ b/codi-rs/docs/PRODUCTION_READINESS_PLAN.md @@ -2,7 +2,7 @@ **Objective:** Bring codi/codi-rs from 88% to 100% production readiness **Timeline:** 2-3 weeks -**Branch:** `feat/production-readiness-phase-1` +**Current Branch:** `feat/production-readiness-phase-2` **Priority:** Fix critical panics first, then polish --- @@ -16,24 +16,24 @@ This plan addresses all production readiness issues identified in the assessment --- -## Phase 1: Critical Issues (Week 1) - ELIMINATE PRODUCTION PANICS +## Phase 1: Critical Issues (Week 1) - ELIMINATE PRODUCTION PANICS ✅ COMPLETE -### 1.1 Priority Files (Fix First) +### 1.1 Priority Files (Fixed) **File 1: `src/tui/app.rs:1683-1687`** - **Issue:** Two `panic!` calls in production app logic -- **Fix:** Replace with proper error handling and user notification -- **Lines:** 1683, 1687 +- **Fix:** Replaced with proper error handling and user notification +- **Status:** ✅ Complete **File 2: `src/orchestrate/ipc/transport.rs:133-152`** - **Issue:** 9 `.expect()` calls in IPC transport layer -- **Fix:** Convert to `Result` with descriptive errors -- **Impact:** IPC failures crash the entire application +- **Fix:** Converted to `Result` with descriptive errors +- **Status:** ✅ Complete **File 3: `src/orchestrate/ipc/client.rs:347, 488`** - **Issue:** `.unwrap()` and `.expect()` in message handling - **Fix:** Graceful degradation on malformed messages -- **Impact:** Worker communication failures +- **Status:** ✅ Complete ### 1.2 Implementation Strategy @@ -49,15 +49,15 @@ This plan addresses all production readiness issues identified in the assessment ``` **Pattern:** -1. Identify all panics/unwraps/expects (100 total) -2. Replace with proper error types -3. Add context with `tracing::error!` -4. Ensure graceful degradation -5. Test error paths +1. ✅ Identify all panics/unwraps/expects (100 total) +2. ✅ Replace with proper error types +3. ✅ Add context with `tracing::error!` +4. ✅ Ensure graceful degradation +5. ✅ Test error paths ### 1.3 Error Type Design -Add comprehensive error types to affected modules: +Added comprehensive error types to affected modules: ```rust // src/orchestrate/ipc/error.rs @@ -79,35 +79,71 @@ pub enum IpcError { HandshakeFailed(String), #[error("Permission request failed: {0}")] PermissionFailed(String), + #[error("Worker not connected: {0}")] + WorkerNotConnected(String), + #[error("Invalid handshake")] + InvalidHandshake, + #[error("Channel closed")] + ChannelClosed, + #[error("Server not started")] + NotStarted, + #[error("Invalid message: {0}")] + InvalidMessage(String), + #[error("Serialization error: {0}")] + Serialization(String), } ``` -### 1.4 Acceptance Criteria +### 1.4 Files Modified in Phase 1 -- [ ] Zero `panic!` calls in production code (tests OK) -- [ ] Zero `expect()` calls in production code -- [ ] Zero `unwrap()` calls in production code paths -- [ ] All errors properly propagated with context -- [ ] No behavioral regressions +- `src/orchestrate/ipc/error.rs` (NEW - comprehensive error type) +- `src/orchestrate/ipc/mod.rs` (exports) +- `src/orchestrate/ipc/client.rs` (fixed unwraps, added InvalidMessage) +- `src/orchestrate/ipc/server.rs` (uses unified IpcError) +- `src/orchestrate/commander.rs` (fixed socket path handling) +- `src/orchestrate/worktree.rs` (fixed 3 unwraps) +- `src/orchestrate/griptree.rs` (fixed 4 unwraps) +- `src/tui/terminal_ui.rs` (removed unused import) + +### 1.5 Acceptance Criteria + +- [x] Zero `panic!` calls in production code (tests OK) +- [x] Zero `expect()` calls in production code +- [x] Zero `unwrap()` calls in production code paths +- [x] All errors properly propagated with context +- [x] No behavioral regressions +- [x] All 516 tests pass --- -## Phase 2: Code Quality (Week 1-2) +## Phase 2: Code Quality (Week 1-2) - IN PROGRESS ### 2.1 Clean Up Warnings **Issues:** -- Unused import `Stylize` in `src/tui/terminal_ui.rs:18` -- Unused function `load_session` in `src/main.rs:505` +- ~~Unused import `Stylize` in `src/tui/terminal_ui.rs:18`~~ ✅ Fixed via cargo fix +- ~~Unused function `load_session` in `src/main.rs:505`~~ ✅ Removed -**Action:** Remove or implement +**Status:** Clean build with zero warnings ✅ ### 2.2 Address TODOs by Priority **HIGH (Complete in Phase 2):** -1. `src/symbol_index/indexer.rs:561` - File cleanup for deleted/renamed files -2. `src/symbol_index/service.rs:206, 229` - Usage detection & dependency graph -3. `src/tui/syntax/highlighter.rs:49` - Tree-sitter-markdown compatibility + +1. **`src/symbol_index/indexer.rs:561` - File cleanup for deleted/renamed files** + - **Status:** ✅ IMPLEMENTED + - **Changes:** + - Added `get_all_files()` method to `SymbolDatabase` + - Implemented `cleanup_deleted()` to remove stale entries + - Files are checked against disk and deleted from DB if missing + +2. **`src/symbol_index/service.rs:206` - Usage detection** + - **Status:** 🔄 IN PROGRESS + - **Description:** Find where symbols are used across the codebase + +3. **`src/symbol_index/service.rs:229` - Dependency graph** + - **Status:** 🔄 IN PROGRESS + - **Description:** Build file dependency graph from imports **LOW (Defer to Phase 4):** - `src/tui/app.rs:1355` - Worktree listing exposure @@ -211,29 +247,29 @@ async fn test_ipc_bind_failure() { ## Success Criteria -### Phase 1 (Critical) ✅ -- [ ] Zero `panic!` in production code -- [ ] Zero `expect()` in production code -- [ ] Zero `unwrap()` in production code paths -- [ ] All IPC errors handled gracefully -- [ ] Comprehensive error types implemented +### Phase 1 (Critical) ✅ COMPLETE +- [x] Zero `panic!` in production code +- [x] Zero `expect()` in production code +- [x] Zero `unwrap()` in production code paths +- [x] All IPC errors handled gracefully +- [x] Comprehensive error types implemented -### Phase 2 (Quality) ✅ -- [ ] Clean build with zero warnings +### Phase 2 (Quality) 🔄 IN PROGRESS +- [x] Clean build with zero warnings - [ ] High-priority TODOs resolved - [ ] Remaining TODOs documented in GitHub issues -### Phase 3 (Testing) ✅ +### Phase 3 (Testing) ⏳ PENDING - [ ] Error path coverage >80% - [ ] Performance benchmarks established - [ ] Performance budgets defined -### Phase 4 (Docs) ✅ +### Phase 4 (Docs) ⏳ PENDING - [ ] Deployment guide complete - [ ] Security audit passed - [ ] Configuration reference complete -### Phase 5 (Monitoring) ✅ +### Phase 5 (Monitoring) ⏳ PENDING - [ ] Health check implemented - [ ] Metrics collection comprehensive - [ ] Production-ready telemetry @@ -242,30 +278,22 @@ async fn test_ipc_bind_failure() { ## Implementation Log -### Day 1: Setup & Priority Files -- [ ] Create error types for IPC layer -- [ ] Fix `src/orchestrate/ipc/transport.rs` -- [ ] Fix `src/orchestrate/ipc/client.rs` - -### Day 2: TUI & App Layer -- [ ] Fix `src/tui/app.rs` panics -- [ ] Fix remaining IPC unwraps -- [ ] Add error context with tracing - -### Day 3: Remaining Unwraps -- [ ] Audit all 100 unwrap/panic locations -- [ ] Fix remaining production code issues -- [ ] Verify tests still pass - -### Day 4: Testing & Validation -- [ ] Add error path tests -- [ ] Run full test suite -- [ ] Performance sanity check - -### Day 5: Documentation & Cleanup -- [ ] Update docs -- [ ] Create GitHub issues for TODOs -- [ ] Final review +### Phase 1: Production Panics (COMPLETE) +- **Date:** 2026-02-07 +- **Branch:** feat/production-readiness-phase-1 +- **PR:** #284 +- **Commits:** 5 +- **Files Changed:** 9 (+473/-43 lines) +- **Tests:** All 516 passing + +### Phase 2: Code Quality (IN PROGRESS) +- **Date:** 2026-02-07 +- **Branch:** feat/production-readiness-phase-2 +- **Completed:** + - ✅ Fixed unused function warning in main.rs + - ✅ Implemented file cleanup in symbol_index + - 🔄 Implementing usage detection + - 🔄 Implementing dependency graph --- @@ -281,4 +309,4 @@ async fn test_ipc_bind_failure() { **Last Updated:** 2026-02-07 **Author:** Codi AI Assistant -**Branch:** feat/production-readiness-phase-1 +**Current Branch:** feat/production-readiness-phase-2 diff --git a/codi-rs/src/main.rs b/codi-rs/src/main.rs index 5c483e90..e285131d 100644 --- a/codi-rs/src/main.rs +++ b/codi-rs/src/main.rs @@ -12,7 +12,7 @@ use codi::agent::AgentConfig; use codi::config::{self, CliOptions}; use codi::providers::{create_provider_from_config, ProviderType}; use codi::tools::ToolRegistry; -use codi::tui::{App, build_system_prompt_from_config}; +use codi::tui::build_system_prompt_from_config; use codi::tui::terminal_ui::run_terminal_repl; /// Codi version string. @@ -501,11 +501,3 @@ async fn run_repl(config: &config::ResolvedConfig, auto_approve: bool, verbose: let debug_mode = verbose || std::env::var("CODI_DEBUG").is_ok(); run_terminal_repl(config, auto_approve, debug_mode).await } - -async fn load_session(app: &mut App, session_name: &str) -> anyhow::Result<()> { - // Try to load by exact name first, then by fuzzy match - if let Err(_) = app.load_session(session_name).await { - return Err(anyhow::anyhow!("Session not found: {}", session_name)); - } - Ok(()) -} diff --git a/codi-rs/src/symbol_index/database.rs b/codi-rs/src/symbol_index/database.rs index 76079837..a75a5f7d 100644 --- a/codi-rs/src/symbol_index/database.rs +++ b/codi-rs/src/symbol_index/database.rs @@ -8,8 +8,8 @@ use std::path::{Path, PathBuf}; use std::time::Instant; -use rusqlite::{Connection, params, OptionalExtension}; -use sha2::{Sha256, Digest}; +use rusqlite::{params, Connection, OptionalExtension}; +use sha2::{Digest, Sha256}; use crate::error::ToolError; @@ -17,9 +17,8 @@ use crate::error::ToolError; use crate::telemetry::metrics::GLOBAL_METRICS; use super::types::{ - CodeSymbol, ExtractionMethod, ImportStatement, - IndexStats, IndexedFile, SymbolKind, SymbolSearchResult, - SymbolVisibility, + CodeSymbol, ExtractionMethod, ImportStatement, IndexStats, IndexedFile, SymbolKind, + SymbolSearchResult, SymbolVisibility, }; /// Current index format version. @@ -66,19 +65,17 @@ impl SymbolDatabase { })?; // Open database - let conn = Connection::open(&db_path).map_err(|e| { - ToolError::ExecutionFailed(format!("Failed to open database: {}", e)) - })?; + let conn = Connection::open(&db_path) + .map_err(|e| ToolError::ExecutionFailed(format!("Failed to open database: {}", e)))?; // Set pragmas for performance conn.execute_batch( "PRAGMA journal_mode = WAL; PRAGMA synchronous = NORMAL; PRAGMA foreign_keys = ON; - PRAGMA cache_size = -64000;" // 64MB cache - ).map_err(|e| { - ToolError::ExecutionFailed(format!("Failed to set pragmas: {}", e)) - })?; + PRAGMA cache_size = -64000;", // 64MB cache + ) + .map_err(|e| ToolError::ExecutionFailed(format!("Failed to set pragmas: {}", e)))?; let mut db = Self { conn, @@ -111,7 +108,8 @@ impl SymbolDatabase { let start = Instant::now(); // Check if schema exists - let table_exists: bool = self.conn + let table_exists: bool = self + .conn .query_row( "SELECT 1 FROM sqlite_master WHERE type='table' AND name='files'", [], @@ -134,7 +132,9 @@ impl SymbolDatabase { /// Create the database schema. fn create_schema(&self) -> Result<(), ToolError> { // Create tables and indexes first (no parameters needed) - self.conn.execute_batch(r#" + self.conn + .execute_batch( + r#" -- Files table CREATE TABLE IF NOT EXISTS files ( id INTEGER PRIMARY KEY AUTOINCREMENT, @@ -200,13 +200,17 @@ impl SymbolDatabase { CREATE INDEX IF NOT EXISTS idx_imports_resolved ON imports(resolved_file_id); CREATE INDEX IF NOT EXISTS idx_deps_from ON file_dependencies(from_file_id); CREATE INDEX IF NOT EXISTS idx_deps_to ON file_dependencies(to_file_id); - "#).map_err(|e| ToolError::ExecutionFailed(format!("Failed to create schema: {}", e)))?; + "#, + ) + .map_err(|e| ToolError::ExecutionFailed(format!("Failed to create schema: {}", e)))?; // Insert metadata with parameters (separate statements) - self.conn.execute( - "INSERT OR REPLACE INTO metadata (key, value) VALUES ('version', ?1)", - params![INDEX_VERSION], - ).map_err(|e| ToolError::ExecutionFailed(format!("Failed to set version: {}", e)))?; + self.conn + .execute( + "INSERT OR REPLACE INTO metadata (key, value) VALUES ('version', ?1)", + params![INDEX_VERSION], + ) + .map_err(|e| ToolError::ExecutionFailed(format!("Failed to set version: {}", e)))?; self.conn.execute( "INSERT OR REPLACE INTO metadata (key, value) VALUES ('last_full_rebuild', datetime('now'))", @@ -231,22 +235,27 @@ impl SymbolDatabase { let start = Instant::now(); // Try to insert, on conflict update - self.conn.execute( - "INSERT INTO files (path, hash, extraction_method, last_indexed) + self.conn + .execute( + "INSERT INTO files (path, hash, extraction_method, last_indexed) VALUES (?1, ?2, ?3, datetime('now')) ON CONFLICT(path) DO UPDATE SET hash = excluded.hash, extraction_method = excluded.extraction_method, last_indexed = datetime('now')", - params![path, hash, method.as_str()], - ).map_err(|e| ToolError::ExecutionFailed(format!("Failed to upsert file: {}", e)))?; + params![path, hash, method.as_str()], + ) + .map_err(|e| ToolError::ExecutionFailed(format!("Failed to upsert file: {}", e)))?; // Always query for the actual ID (last_insert_rowid is unreliable for upsert) - let file_id: i64 = self.conn.query_row( - "SELECT id FROM files WHERE path = ?1", - params![path], - |row| row.get(0), - ).map_err(|e| ToolError::ExecutionFailed(format!("Failed to get file id: {}", e)))?; + let file_id: i64 = self + .conn + .query_row( + "SELECT id FROM files WHERE path = ?1", + params![path], + |row| row.get(0), + ) + .map_err(|e| ToolError::ExecutionFailed(format!("Failed to get file id: {}", e)))?; #[cfg(feature = "telemetry")] GLOBAL_METRICS.record_operation("symbol_index.db.upsert_file", start.elapsed()); @@ -258,19 +267,23 @@ impl SymbolDatabase { pub fn get_file(&self, path: &str) -> Result, ToolError> { let start = Instant::now(); - let result = self.conn.query_row( - "SELECT id, path, hash, extraction_method, last_indexed FROM files WHERE path = ?1", - params![path], - |row| { - Ok(IndexedFile { - id: row.get(0)?, - path: row.get(1)?, - hash: row.get(2)?, - extraction_method: ExtractionMethod::from_str(&row.get::<_, String>(3)?), - last_indexed: row.get(4)?, - }) - }, - ).optional().map_err(|e| ToolError::ExecutionFailed(format!("Failed to get file: {}", e)))?; + let result = self + .conn + .query_row( + "SELECT id, path, hash, extraction_method, last_indexed FROM files WHERE path = ?1", + params![path], + |row| { + Ok(IndexedFile { + id: row.get(0)?, + path: row.get(1)?, + hash: row.get(2)?, + extraction_method: ExtractionMethod::from_str(&row.get::<_, String>(3)?), + last_indexed: row.get(4)?, + }) + }, + ) + .optional() + .map_err(|e| ToolError::ExecutionFailed(format!("Failed to get file: {}", e)))?; #[cfg(feature = "telemetry")] GLOBAL_METRICS.record_operation("symbol_index.db.get_file", start.elapsed()); @@ -283,10 +296,9 @@ impl SymbolDatabase { let start = Instant::now(); // Cascading deletes handle symbols, imports, etc. - self.conn.execute( - "DELETE FROM files WHERE id = ?1", - params![file_id], - ).map_err(|e| ToolError::ExecutionFailed(format!("Failed to delete file: {}", e)))?; + self.conn + .execute("DELETE FROM files WHERE id = ?1", params![file_id]) + .map_err(|e| ToolError::ExecutionFailed(format!("Failed to delete file: {}", e)))?; #[cfg(feature = "telemetry")] GLOBAL_METRICS.record_operation("symbol_index.db.delete_file", start.elapsed()); @@ -299,10 +311,11 @@ impl SymbolDatabase { let start = Instant::now(); // Delete existing symbols for this file - self.conn.execute( - "DELETE FROM symbols WHERE file_id = ?1", - params![file_id], - ).map_err(|e| ToolError::ExecutionFailed(format!("Failed to delete old symbols: {}", e)))?; + self.conn + .execute("DELETE FROM symbols WHERE file_id = ?1", params![file_id]) + .map_err(|e| { + ToolError::ExecutionFailed(format!("Failed to delete old symbols: {}", e)) + })?; // Insert new symbols let mut stmt = self.conn.prepare( @@ -323,7 +336,8 @@ impl SymbolDatabase { symbol.signature, symbol.doc_summary, metadata_json, - ]).map_err(|e| ToolError::ExecutionFailed(format!("Failed to insert symbol: {}", e)))?; + ]) + .map_err(|e| ToolError::ExecutionFailed(format!("Failed to insert symbol: {}", e)))?; } #[cfg(feature = "telemetry")] @@ -333,43 +347,69 @@ impl SymbolDatabase { } /// Insert imports for a file. - pub fn insert_imports(&self, file_id: i64, imports: &[ImportStatement]) -> Result<(), ToolError> { + pub fn insert_imports( + &self, + file_id: i64, + imports: &[ImportStatement], + ) -> Result<(), ToolError> { let start = Instant::now(); // Delete existing imports for this file - self.conn.execute( - "DELETE FROM imports WHERE file_id = ?1", - params![file_id], - ).map_err(|e| ToolError::ExecutionFailed(format!("Failed to delete old imports: {}", e)))?; - - let mut import_stmt = self.conn.prepare( - "INSERT INTO imports (file_id, source_path, is_type_only, line) - VALUES (?1, ?2, ?3, ?4)" - ).map_err(|e| ToolError::ExecutionFailed(format!("Failed to prepare import statement: {}", e)))?; - - let mut symbol_stmt = self.conn.prepare( - "INSERT INTO import_symbols (import_id, name, alias, is_default, is_namespace) - VALUES (?1, ?2, ?3, ?4, ?5)" - ).map_err(|e| ToolError::ExecutionFailed(format!("Failed to prepare import symbol statement: {}", e)))?; + self.conn + .execute("DELETE FROM imports WHERE file_id = ?1", params![file_id]) + .map_err(|e| { + ToolError::ExecutionFailed(format!("Failed to delete old imports: {}", e)) + })?; + + let mut import_stmt = self + .conn + .prepare( + "INSERT INTO imports (file_id, source_path, is_type_only, line) + VALUES (?1, ?2, ?3, ?4)", + ) + .map_err(|e| { + ToolError::ExecutionFailed(format!("Failed to prepare import statement: {}", e)) + })?; + + let mut symbol_stmt = self + .conn + .prepare( + "INSERT INTO import_symbols (import_id, name, alias, is_default, is_namespace) + VALUES (?1, ?2, ?3, ?4, ?5)", + ) + .map_err(|e| { + ToolError::ExecutionFailed(format!( + "Failed to prepare import symbol statement: {}", + e + )) + })?; for import in imports { - import_stmt.execute(params![ - file_id, - import.source, - import.is_type_only as i32, - import.line, - ]).map_err(|e| ToolError::ExecutionFailed(format!("Failed to insert import: {}", e)))?; + import_stmt + .execute(params![ + file_id, + import.source, + import.is_type_only as i32, + import.line, + ]) + .map_err(|e| { + ToolError::ExecutionFailed(format!("Failed to insert import: {}", e)) + })?; let import_id = self.conn.last_insert_rowid(); for sym in &import.symbols { - symbol_stmt.execute(params![ - import_id, - sym.name, - sym.alias, - sym.is_default as i32, - sym.is_namespace as i32, - ]).map_err(|e| ToolError::ExecutionFailed(format!("Failed to insert import symbol: {}", e)))?; + symbol_stmt + .execute(params![ + import_id, + sym.name, + sym.alias, + sym.is_default as i32, + sym.is_namespace as i32, + ]) + .map_err(|e| { + ToolError::ExecutionFailed(format!("Failed to insert import symbol: {}", e)) + })?; } } @@ -380,7 +420,11 @@ impl SymbolDatabase { } /// Search for symbols by name (fuzzy search). - pub fn find_symbols(&self, query: &str, limit: usize) -> Result, ToolError> { + pub fn find_symbols( + &self, + query: &str, + limit: usize, + ) -> Result, ToolError> { let start = Instant::now(); // Use LIKE for simple fuzzy search @@ -402,35 +446,35 @@ impl SymbolDatabase { let exact_pattern = query; let starts_with_pattern = format!("{}%", query); - let results = stmt.query_map( - params![pattern, exact_pattern, starts_with_pattern, limit as i64], - |row| { - let name: String = row.get(0)?; - let score = if name == query { - 1.0 - } else if name.starts_with(query) { - 0.8 - } else { - 0.5 - }; - - Ok(SymbolSearchResult { - name, - kind: SymbolKind::from_str(&row.get::<_, String>(1)?), - file: row.get(2)?, - line: row.get(3)?, - end_line: row.get(4)?, - visibility: SymbolVisibility::from_str(&row.get::<_, String>(5)?), - signature: row.get(6)?, - doc_summary: row.get(7)?, - score, - }) - }, - ).map_err(|e| ToolError::ExecutionFailed(format!("Query failed: {}", e)))?; + let results = stmt + .query_map( + params![pattern, exact_pattern, starts_with_pattern, limit as i64], + |row| { + let name: String = row.get(0)?; + let score = if name == query { + 1.0 + } else if name.starts_with(query) { + 0.8 + } else { + 0.5 + }; + + Ok(SymbolSearchResult { + name, + kind: SymbolKind::from_str(&row.get::<_, String>(1)?), + file: row.get(2)?, + line: row.get(3)?, + end_line: row.get(4)?, + visibility: SymbolVisibility::from_str(&row.get::<_, String>(5)?), + signature: row.get(6)?, + doc_summary: row.get(7)?, + score, + }) + }, + ) + .map_err(|e| ToolError::ExecutionFailed(format!("Query failed: {}", e)))?; - let results: Vec<_> = results - .filter_map(|r| r.ok()) - .collect(); + let results: Vec<_> = results.filter_map(|r| r.ok()).collect(); #[cfg(feature = "telemetry")] GLOBAL_METRICS.record_operation("symbol_index.db.find_symbols", start.elapsed()); @@ -442,32 +486,55 @@ impl SymbolDatabase { pub fn get_stats(&self) -> Result { let start = Instant::now(); - let total_files: u32 = self.conn + let total_files: u32 = self + .conn .query_row("SELECT COUNT(*) FROM files", [], |row| row.get(0)) .map_err(|e| ToolError::ExecutionFailed(format!("Failed to count files: {}", e)))?; - let total_symbols: u32 = self.conn + let total_symbols: u32 = self + .conn .query_row("SELECT COUNT(*) FROM symbols", [], |row| row.get(0)) .map_err(|e| ToolError::ExecutionFailed(format!("Failed to count symbols: {}", e)))?; - let total_imports: u32 = self.conn + let total_imports: u32 = self + .conn .query_row("SELECT COUNT(*) FROM imports", [], |row| row.get(0)) .map_err(|e| ToolError::ExecutionFailed(format!("Failed to count imports: {}", e)))?; - let total_dependencies: u32 = self.conn - .query_row("SELECT COUNT(*) FROM file_dependencies", [], |row| row.get(0)) - .map_err(|e| ToolError::ExecutionFailed(format!("Failed to count dependencies: {}", e)))?; - - let version: String = self.conn - .query_row("SELECT value FROM metadata WHERE key = 'version'", [], |row| row.get(0)) + let total_dependencies: u32 = self + .conn + .query_row("SELECT COUNT(*) FROM file_dependencies", [], |row| { + row.get(0) + }) + .map_err(|e| { + ToolError::ExecutionFailed(format!("Failed to count dependencies: {}", e)) + })?; + + let version: String = self + .conn + .query_row( + "SELECT value FROM metadata WHERE key = 'version'", + [], + |row| row.get(0), + ) .unwrap_or_else(|_| INDEX_VERSION.to_string()); - let last_full_rebuild: String = self.conn - .query_row("SELECT value FROM metadata WHERE key = 'last_full_rebuild'", [], |row| row.get(0)) + let last_full_rebuild: String = self + .conn + .query_row( + "SELECT value FROM metadata WHERE key = 'last_full_rebuild'", + [], + |row| row.get(0), + ) .unwrap_or_else(|_| "never".to_string()); - let last_update: String = self.conn - .query_row("SELECT value FROM metadata WHERE key = 'last_update'", [], |row| row.get(0)) + let last_update: String = self + .conn + .query_row( + "SELECT value FROM metadata WHERE key = 'last_update'", + [], + |row| row.get(0), + ) .unwrap_or_else(|_| "never".to_string()); let index_size_bytes = std::fs::metadata(&self.db_path) @@ -492,26 +559,60 @@ impl SymbolDatabase { /// Update the last_update timestamp. pub fn touch_update(&self) -> Result<(), ToolError> { - self.conn.execute( - "UPDATE metadata SET value = datetime('now') WHERE key = 'last_update'", - [], - ).map_err(|e| ToolError::ExecutionFailed(format!("Failed to update timestamp: {}", e)))?; + self.conn + .execute( + "UPDATE metadata SET value = datetime('now') WHERE key = 'last_update'", + [], + ) + .map_err(|e| { + ToolError::ExecutionFailed(format!("Failed to update timestamp: {}", e)) + })?; Ok(()) } + /// Get all indexed file paths. + pub fn get_all_files(&self) -> Result, ToolError> { + let start = Instant::now(); + + let mut stmt = self + .conn + .prepare("SELECT id, path FROM files") + .map_err(|e| ToolError::ExecutionFailed(format!("Failed to prepare query: {}", e)))?; + + let files = stmt + .query_map([], |row| { + Ok((row.get::<_, i64>(0)?, row.get::<_, String>(1)?)) + }) + .map_err(|e| ToolError::ExecutionFailed(format!("Failed to query files: {}", e)))?; + + let mut result = Vec::new(); + for file in files { + if let Ok((_, path)) = file { + result.push(path); + } + } + + #[cfg(feature = "telemetry")] + GLOBAL_METRICS.record_operation("symbol_index.db.get_all_files", start.elapsed()); + + Ok(result) + } + /// Clear the entire index. pub fn clear(&self) -> Result<(), ToolError> { let start = Instant::now(); - self.conn.execute_batch( - "DELETE FROM file_dependencies; + self.conn + .execute_batch( + "DELETE FROM file_dependencies; DELETE FROM import_symbols; DELETE FROM imports; DELETE FROM symbols; DELETE FROM files; UPDATE metadata SET value = datetime('now') WHERE key = 'last_full_rebuild'; - UPDATE metadata SET value = datetime('now') WHERE key = 'last_update';" - ).map_err(|e| ToolError::ExecutionFailed(format!("Failed to clear index: {}", e)))?; + UPDATE metadata SET value = datetime('now') WHERE key = 'last_update';", + ) + .map_err(|e| ToolError::ExecutionFailed(format!("Failed to clear index: {}", e)))?; #[cfg(feature = "telemetry")] GLOBAL_METRICS.record_operation("symbol_index.db.clear", start.elapsed()); @@ -521,21 +622,24 @@ impl SymbolDatabase { /// Begin a transaction. pub fn begin_transaction(&mut self) -> Result<(), ToolError> { - self.conn.execute("BEGIN TRANSACTION", []) - .map_err(|e| ToolError::ExecutionFailed(format!("Failed to begin transaction: {}", e)))?; + self.conn.execute("BEGIN TRANSACTION", []).map_err(|e| { + ToolError::ExecutionFailed(format!("Failed to begin transaction: {}", e)) + })?; Ok(()) } /// Commit the current transaction. pub fn commit(&mut self) -> Result<(), ToolError> { - self.conn.execute("COMMIT", []) + self.conn + .execute("COMMIT", []) .map_err(|e| ToolError::ExecutionFailed(format!("Failed to commit: {}", e)))?; Ok(()) } /// Rollback the current transaction. pub fn rollback(&mut self) -> Result<(), ToolError> { - self.conn.execute("ROLLBACK", []) + self.conn + .execute("ROLLBACK", []) .map_err(|e| ToolError::ExecutionFailed(format!("Failed to rollback: {}", e)))?; Ok(()) } @@ -570,7 +674,9 @@ mod tests { let db = SymbolDatabase::open(project_root).unwrap(); // Insert file - let file_id = db.upsert_file("src/main.rs", "abc123", ExtractionMethod::TreeSitter).unwrap(); + let file_id = db + .upsert_file("src/main.rs", "abc123", ExtractionMethod::TreeSitter) + .unwrap(); assert!(file_id > 0); // Get file @@ -579,7 +685,9 @@ mod tests { assert_eq!(file.hash, "abc123"); // Update file - let file_id2 = db.upsert_file("src/main.rs", "def456", ExtractionMethod::TreeSitter).unwrap(); + let file_id2 = db + .upsert_file("src/main.rs", "def456", ExtractionMethod::TreeSitter) + .unwrap(); assert_eq!(file_id, file_id2); let file = db.get_file("src/main.rs").unwrap().unwrap(); @@ -593,7 +701,9 @@ mod tests { let db = SymbolDatabase::open(project_root).unwrap(); - let file_id = db.upsert_file("src/lib.rs", "hash", ExtractionMethod::TreeSitter).unwrap(); + let file_id = db + .upsert_file("src/lib.rs", "hash", ExtractionMethod::TreeSitter) + .unwrap(); let symbols = vec![ CodeSymbol { diff --git a/codi-rs/src/symbol_index/indexer.rs b/codi-rs/src/symbol_index/indexer.rs index 4fa2a4a1..99163ab1 100644 --- a/codi-rs/src/symbol_index/indexer.rs +++ b/codi-rs/src/symbol_index/indexer.rs @@ -550,18 +550,30 @@ impl Indexer { ) -> Result { let start = Instant::now(); - let _project_root = Path::new(&self.options.project_root); - let _db_lock = db.lock().await; - - // Get all indexed files - // Note: We'd need to add a method to get all files from db - // For now, this is a placeholder - let deleted_count = 0u32; - - // TODO: Implement file cleanup - // 1. Get all indexed file paths from database - // 2. Check which ones no longer exist on disk - // 3. Delete those from the database + let project_root = Path::new(&self.options.project_root); + let db_lock = db.lock().await; + + // Get all indexed files from database + let indexed_files = db_lock.get_all_files()?; + + let mut deleted_count = 0u32; + + // Check which files no longer exist on disk + for file_path in indexed_files { + let full_path = if file_path.starts_with('/') { + PathBuf::from(&file_path) + } else { + project_root.join(&file_path) + }; + + if !full_path.exists() { + // Get file ID and delete + if let Ok(Some(file)) = db_lock.get_file(&file_path) { + db_lock.delete_file(file.id)?; + deleted_count += 1; + } + } + } #[cfg(feature = "telemetry")] GLOBAL_METRICS.record_operation("symbol_index.indexer.cleanup_deleted", start.elapsed()); From a7eb93d5db9e22e5a062f13b632b50438e29ab3a Mon Sep 17 00:00:00 2001 From: Layne Penney Date: Sun, 8 Feb 2026 08:05:22 -0600 Subject: [PATCH 2/2] feat(phase-2): implement usage detection and dependency graph MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add database methods: find_imports_with_symbol(), get_file_dependencies(), get_file_dependents() - Implement usage detection in find_references() by finding imports - Implement dependency graph traversal in get_dependencies() with BFS - Add support for both Imports and ImportedBy directions - Fix unused variable warnings Phase 2 complete: - ✅ Clean build with zero warnings - ✅ HIGH priority TODOs resolved (file cleanup, usage detection, dependency graph) - ✅ All 516 tests pass --- codi-rs/src/symbol_index/database.rs | 126 +++++++++++++++++++++++++++ codi-rs/src/symbol_index/service.rs | 80 ++++++++++++++--- 2 files changed, 193 insertions(+), 13 deletions(-) diff --git a/codi-rs/src/symbol_index/database.rs b/codi-rs/src/symbol_index/database.rs index a75a5f7d..0ccb3eac 100644 --- a/codi-rs/src/symbol_index/database.rs +++ b/codi-rs/src/symbol_index/database.rs @@ -598,6 +598,132 @@ impl SymbolDatabase { Ok(result) } + /// Find all imports that reference a symbol name. + pub fn find_imports_with_symbol( + &self, + symbol_name: &str, + ) -> Result, ToolError> { + let start = Instant::now(); + + let mut stmt = self + .conn + .prepare( + "SELECT f.path, i.source_path, i.line + FROM import_symbols s + JOIN imports i ON s.import_id = i.id + JOIN files f ON i.file_id = f.id + WHERE s.name = ?1", + ) + .map_err(|e| ToolError::ExecutionFailed(format!("Failed to prepare query: {}", e)))?; + + let rows = stmt + .query_map(params![symbol_name], |row| { + Ok(( + row.get::<_, String>(0)?, + row.get::<_, String>(1)?, + row.get::<_, u32>(2)?, + )) + }) + .map_err(|e| ToolError::ExecutionFailed(format!("Failed to query imports: {}", e)))?; + + let mut result = Vec::new(); + for row in rows { + if let Ok((file_path, source, line)) = row { + result.push((file_path, source, line)); + } + } + + #[cfg(feature = "telemetry")] + GLOBAL_METRICS + .record_operation("symbol_index.db.find_imports_with_symbol", start.elapsed()); + + Ok(result) + } + + /// Get file dependencies (imports) for a file. + pub fn get_file_dependencies( + &self, + file_id: i64, + ) -> Result, ToolError> { + let start = Instant::now(); + + let mut stmt = self + .conn + .prepare( + "SELECT f.id, f.path, i.source_path + FROM imports i + JOIN files f ON i.resolved_file_id = f.id + WHERE i.file_id = ?1 AND i.resolved_file_id IS NOT NULL", + ) + .map_err(|e| ToolError::ExecutionFailed(format!("Failed to prepare query: {}", e)))?; + + let rows = stmt + .query_map(params![file_id], |row| { + Ok(( + row.get::<_, i64>(0)?, + row.get::<_, String>(1)?, + row.get::<_, String>(2)?, + )) + }) + .map_err(|e| { + ToolError::ExecutionFailed(format!("Failed to query dependencies: {}", e)) + })?; + + let mut result = Vec::new(); + for row in rows { + if let Ok((id, path, source)) = row { + result.push((id, path, source)); + } + } + + #[cfg(feature = "telemetry")] + GLOBAL_METRICS.record_operation("symbol_index.db.get_file_dependencies", start.elapsed()); + + Ok(result) + } + + /// Get files that depend on a given file (reverse dependencies). + pub fn get_file_dependents( + &self, + file_id: i64, + ) -> Result, ToolError> { + let start = Instant::now(); + + let mut stmt = self + .conn + .prepare( + "SELECT f.id, f.path, i.source_path + FROM imports i + JOIN files f ON i.file_id = f.id + WHERE i.resolved_file_id = ?1", + ) + .map_err(|e| ToolError::ExecutionFailed(format!("Failed to prepare query: {}", e)))?; + + let rows = stmt + .query_map(params![file_id], |row| { + Ok(( + row.get::<_, i64>(0)?, + row.get::<_, String>(1)?, + row.get::<_, String>(2)?, + )) + }) + .map_err(|e| { + ToolError::ExecutionFailed(format!("Failed to query dependents: {}", e)) + })?; + + let mut result = Vec::new(); + for row in rows { + if let Ok((id, path, source)) = row { + result.push((id, path, source)); + } + } + + #[cfg(feature = "telemetry")] + GLOBAL_METRICS.record_operation("symbol_index.db.get_file_dependents", start.elapsed()); + + Ok(result) + } + /// Clear the entire index. pub fn clear(&self) -> Result<(), ToolError> { let start = Instant::now(); diff --git a/codi-rs/src/symbol_index/service.rs b/codi-rs/src/symbol_index/service.rs index e7378b78..c1e660b8 100644 --- a/codi-rs/src/symbol_index/service.rs +++ b/codi-rs/src/symbol_index/service.rs @@ -203,10 +203,19 @@ impl SymbolIndexService { }); } - // TODO: Implement usage detection by: - // 1. Finding all imports that reference the symbol - // 2. Searching for usages in code (would need full text search) - // For now, we return just the definition + // Find all imports that reference this symbol + let imports = db.find_imports_with_symbol(symbol_name)?; + for (file_path, _source, line) in imports { + // Skip if this is the definition file (already added above) + if file_path != def_symbol.file { + results.push(ReferenceResult { + file: file_path, + line, + reference_type: ReferenceType::Import, + context: None, + }); + } + } } #[cfg(feature = "telemetry")] @@ -218,19 +227,64 @@ impl SymbolIndexService { /// Get the dependency graph for a file. pub async fn get_dependencies( &self, - _file_path: &str, - _direction: DependencyDirection, - _max_depth: Option, + file_path: &str, + direction: DependencyDirection, + max_depth: Option, ) -> Result, ToolError> { let start = Instant::now(); + let mut results = Vec::new(); + let mut visited = std::collections::HashSet::new(); + let max_depth = max_depth.unwrap_or(3); // Default to 3 levels deep + + let db = self.db.lock().await; - let results = Vec::new(); + // Get the starting file ID + if let Ok(Some(file)) = db.get_file(file_path) { + let file_id = file.id; + visited.insert(file_id); - // TODO: Implement dependency graph traversal - // This requires: - // 1. Getting imports from the file - // 2. Resolving import paths to actual files - // 3. Recursively following the graph + // BFS traversal + let mut queue = vec![(file_id, file_path.to_string(), 0u32)]; + + while let Some((current_id, _current_path, depth)) = queue.pop() { + if depth >= max_depth { + continue; + } + + match direction { + DependencyDirection::Imports => { + // Get files that this file imports + let deps = db.get_file_dependencies(current_id)?; + for (dep_id, dep_path, _source) in deps { + if visited.insert(dep_id) { + results.push(DependencyResult { + file: dep_path.clone(), + direction: DependencyDirection::Imports, + depth: depth + 1, + dependency_type: super::types::DependencyType::Import, + }); + queue.push((dep_id, dep_path, depth + 1)); + } + } + } + DependencyDirection::ImportedBy => { + // Get files that import this file + let dependents = db.get_file_dependents(current_id)?; + for (dep_id, dep_path, _source) in dependents { + if visited.insert(dep_id) { + results.push(DependencyResult { + file: dep_path.clone(), + direction: DependencyDirection::ImportedBy, + depth: depth + 1, + dependency_type: super::types::DependencyType::Import, + }); + queue.push((dep_id, dep_path, depth + 1)); + } + } + } + } + } + } #[cfg(feature = "telemetry")] GLOBAL_METRICS.record_operation("symbol_index.service.get_dependencies", start.elapsed());