From 3ab601c717d29586454f4a48d07d4cabc9aee7b2 Mon Sep 17 00:00:00 2001 From: Micah Date: Sun, 15 Mar 2026 22:58:38 -0700 Subject: [PATCH] Fix additional memory retrieval issues beyond PR #74 - Add porter stemming to FTS5 tokenizer (tokenize='porter unicode61') so plurals like 'medications' match 'Medication' - Strip punctuation from FTS5 queries so question marks don't break MATCH - Expand tilde (~) in db_path before opening SQLite connection - Remove 'id' from FTS5 columns (UUIDs shouldn't be full-text indexed) - Add memory context injection to server chat_completions route (routes.py had no memory integration - UI chat was never grounded) - Fix agent name-to-ID resolution in agent_cmd.py (foreign key error) - Align Python SQLiteMemory schema with Rust backend --- .gitignore | 2 + .../openjarvis-tools/src/storage/sqlite.rs | 60 ++++++++++++++++--- src/openjarvis/cli/agent_cmd.py | 22 +++++++ src/openjarvis/server/routes.py | 45 ++++++++++++++ src/openjarvis/tools/storage/sqlite.py | 27 +-------- 5 files changed, 121 insertions(+), 35 deletions(-) diff --git a/.gitignore b/.gitignore index dcdb7447..8843988e 100644 --- a/.gitignore +++ b/.gitignore @@ -88,3 +88,5 @@ CLAUDE.md # Research output research_mining_* +.python-version +openjarvis-bugfix-spec-v2.md diff --git a/rust/crates/openjarvis-tools/src/storage/sqlite.rs b/rust/crates/openjarvis-tools/src/storage/sqlite.rs index bfa9f97c..abe70eee 100644 --- a/rust/crates/openjarvis-tools/src/storage/sqlite.rs +++ b/rust/crates/openjarvis-tools/src/storage/sqlite.rs @@ -15,6 +15,19 @@ pub struct SQLiteMemory { impl SQLiteMemory { pub fn new(db_path: &Path) -> Result { + // Expand leading ~ to the user's home directory + let db_path = if db_path.starts_with("~") { + let home = std::env::var("HOME").map_err(|_| { + OpenJarvisError::Io(std::io::Error::other( + "HOME environment variable not set", + )) + })?; + PathBuf::from(home).join(db_path.strip_prefix("~").unwrap()) + } else { + db_path.to_path_buf() + }; + let db_path = db_path.as_path(); + if let Some(parent) = db_path.parent() { std::fs::create_dir_all(parent).map_err(|e| { OpenJarvisError::Io(std::io::Error::other(e)) @@ -36,7 +49,7 @@ impl SQLiteMemory { created_at REAL DEFAULT (julianday('now')) ); CREATE VIRTUAL TABLE IF NOT EXISTS documents_fts USING fts5( - id, content, source, tokenize='unicode61' + content, source, tokenize='porter unicode61' );", ) .map_err(|e| { @@ -110,9 +123,10 @@ impl MemoryBackend for SQLiteMemory { )) })?; + let rowid = conn.last_insert_rowid(); conn.execute( - "INSERT INTO documents_fts (id, content, source) VALUES (?1, ?2, ?3)", - rusqlite::params![doc_id, content, source], + "INSERT INTO documents_fts (rowid, content, source) VALUES (?1, ?2, ?3)", + rusqlite::params![rowid, content, source], ) .map_err(|e| { OpenJarvisError::Io(std::io::Error::other( @@ -130,9 +144,13 @@ impl MemoryBackend for SQLiteMemory { ) -> Result, OpenJarvisError> { let conn = self.conn.lock(); - let words: Vec<&str> = query.split_whitespace().collect(); + let words: Vec = query + .split_whitespace() + .map(|w| w.trim_matches(|c: char| "?.,!;:'\"()[]{}/ ".contains(c)).to_string()) + .filter(|w| !w.is_empty()) + .collect(); let fts_query = if words.len() == 1 { - words[0].to_string() + words[0].clone() } else { words.join(" OR ") }; @@ -140,11 +158,11 @@ impl MemoryBackend for SQLiteMemory { let mut stmt = conn .prepare( "SELECT d.content, d.source, d.metadata, - bm25(documents_fts, 0.0, 1.0, 0.5) * -1 as score + bm25(documents_fts, 1.0, 0.5) * -1 as score FROM documents_fts f - JOIN documents d ON d.id = f.id + JOIN documents d ON d.rowid = f.rowid WHERE documents_fts MATCH ?1 - ORDER BY bm25(documents_fts, 0.0, 1.0, 0.5) + ORDER BY bm25(documents_fts, 1.0, 0.5) LIMIT ?2", ) .map_err(|e| { @@ -179,8 +197,9 @@ impl MemoryBackend for SQLiteMemory { fn delete(&self, doc_id: &str) -> Result { let conn = self.conn.lock(); + // Delete from FTS5 using the rowid from the documents table conn.execute( - "DELETE FROM documents_fts WHERE id = ?1", + "DELETE FROM documents_fts WHERE rowid = (SELECT rowid FROM documents WHERE id = ?1)", rusqlite::params![doc_id], ) .map_err(|e| { @@ -238,6 +257,29 @@ mod tests { let results = mem.retrieve("Rust programming", 5).unwrap(); assert!(!results.is_empty()); assert!(results[0].content.contains("Rust")); + assert!(results[0].score > 0.0, "score should be positive, got {}", results[0].score); + } + + #[test] + fn test_sqlite_porter_stemming() { + let mem = SQLiteMemory::in_memory().unwrap(); + mem.store("Medication list for patient", "health", None).unwrap(); + + // Plural form should match via porter stemming + let results = mem.retrieve("medications", 5).unwrap(); + assert!(!results.is_empty(), "porter stemming should match 'medications' to 'Medication'"); + assert!(results[0].score > 0.0); + } + + #[test] + fn test_sqlite_punctuation_stripping() { + let mem = SQLiteMemory::in_memory().unwrap(); + mem.store("Medication list for patient Micah", "health", None).unwrap(); + + // Natural language query with trailing punctuation should not break FTS5 + let results = mem.retrieve("What medications does Micah take?", 5).unwrap(); + assert!(!results.is_empty(), "query with punctuation should still return results"); + assert!(results[0].score > 0.0); } #[test] diff --git a/src/openjarvis/cli/agent_cmd.py b/src/openjarvis/cli/agent_cmd.py index 47c3ba94..0958ca4e 100644 --- a/src/openjarvis/cli/agent_cmd.py +++ b/src/openjarvis/cli/agent_cmd.py @@ -24,6 +24,26 @@ def _get_manager(): return AgentManager(db_path=db_path) +def _resolve_agent_id(manager, agent_id_or_name: str) -> str: + """Resolve an agent name or ID to the actual agent ID. + + Accepts either the hex ID or the agent name. Raises SystemExit if + no matching agent is found. + """ + # Try direct ID lookup first + agent = manager.get_agent(agent_id_or_name) + if agent is not None: + return agent["id"] + + # Fall back to name lookup + for a in manager.list_agents(include_archived=True): + if a["name"] == agent_id_or_name: + return a["id"] + + click.echo(f"Agent not found: {agent_id_or_name}", err=True) + raise SystemExit(1) + + @click.group("agents") def agent() -> None: """Manage persistent agents — create, inspect, chat, bind channels.""" @@ -680,6 +700,7 @@ def errors(): def ask(agent_id, message): """Ask an agent a question (immediate response).""" manager = _get_manager() + agent_id = _resolve_agent_id(manager, agent_id) manager.send_message(agent_id, message, mode="immediate") click.echo("Asking agent...") _, executor, _ = _get_scheduler_and_executor() @@ -701,6 +722,7 @@ def ask(agent_id, message): def instruct(agent_id, message): """Queue an instruction for the agent's next tick.""" manager = _get_manager() + agent_id = _resolve_agent_id(manager, agent_id) msg = manager.send_message(agent_id, message, mode="queued") click.echo(f"Instruction queued (ID: {msg['id'][:8]})") diff --git a/src/openjarvis/server/routes.py b/src/openjarvis/server/routes.py index 7838daac..b59ec919 100644 --- a/src/openjarvis/server/routes.py +++ b/src/openjarvis/server/routes.py @@ -46,6 +46,51 @@ async def chat_completions(request_body: ChatCompletionRequest, request: Request agent = getattr(request.app.state, "agent", None) model = request_body.model + # Inject memory context into messages before dispatching + config = getattr(request.app.state, "config", None) + memory_backend = getattr(request.app.state, "memory_backend", None) + if ( + config is not None + and memory_backend is not None + and config.agent.context_from_memory + and request_body.messages + ): + try: + from openjarvis.tools.storage.context import ContextConfig, inject_context + + # Extract query from the last user message + query_text = "" + for m in reversed(request_body.messages): + if m.role == "user" and m.content: + query_text = m.content + break + + if query_text: + messages = _to_messages(request_body.messages) + ctx_cfg = ContextConfig( + top_k=config.memory.context_top_k, + min_score=config.memory.context_min_score, + max_context_tokens=config.memory.context_max_tokens, + ) + enriched = inject_context( + query_text, messages, memory_backend, config=ctx_cfg, + ) + # Rebuild request messages from enriched Message objects + if len(enriched) > len(messages): + from openjarvis.server.models import ChatMessage + + new_msgs = [] + for msg in enriched: + new_msgs.append(ChatMessage( + role=msg.role.value, + content=msg.content, + name=msg.name, + tool_call_id=getattr(msg, "tool_call_id", None), + )) + request_body.messages = new_msgs + except Exception: + pass # Don't break chat if memory retrieval fails + if request_body.stream: bus = getattr(request.app.state, "bus", None) # Use the agent stream bridge only when tools are present (the diff --git a/src/openjarvis/tools/storage/sqlite.py b/src/openjarvis/tools/storage/sqlite.py index cd166643..708ca5a4 100644 --- a/src/openjarvis/tools/storage/sqlite.py +++ b/src/openjarvis/tools/storage/sqlite.py @@ -56,33 +56,8 @@ class SQLiteMemory(MemoryBackend): USING fts5( content, source, - content=documents, - content_rowid=rowid + tokenize='porter unicode61' ); - - CREATE TRIGGER IF NOT EXISTS documents_ai - AFTER INSERT ON documents BEGIN - INSERT INTO documents_fts(rowid, content, source) - VALUES (new.rowid, new.content, new.source); - END; - - CREATE TRIGGER IF NOT EXISTS documents_ad - AFTER DELETE ON documents BEGIN - INSERT INTO documents_fts( - documents_fts, rowid, content, source - ) - VALUES ('delete', old.rowid, old.content, old.source); - END; - - CREATE TRIGGER IF NOT EXISTS documents_au - AFTER UPDATE ON documents BEGIN - INSERT INTO documents_fts( - documents_fts, rowid, content, source - ) - VALUES ('delete', old.rowid, old.content, old.source); - INSERT INTO documents_fts(rowid, content, source) - VALUES (new.rowid, new.content, new.source); - END; """) def store(