mirror of
https://github.com/open-jarvis/OpenJarvis.git
synced 2026-08-14 00:47:52 +00:00
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
This commit is contained in:
@@ -88,3 +88,5 @@ CLAUDE.md
|
||||
|
||||
# Research output
|
||||
research_mining_*
|
||||
.python-version
|
||||
openjarvis-bugfix-spec-v2.md
|
||||
|
||||
@@ -15,6 +15,19 @@ pub struct SQLiteMemory {
|
||||
|
||||
impl SQLiteMemory {
|
||||
pub fn new(db_path: &Path) -> Result<Self, OpenJarvisError> {
|
||||
// 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<Vec<RetrievalResult>, OpenJarvisError> {
|
||||
let conn = self.conn.lock();
|
||||
|
||||
let words: Vec<&str> = query.split_whitespace().collect();
|
||||
let words: Vec<String> = 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<bool, OpenJarvisError> {
|
||||
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]
|
||||
|
||||
@@ -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]})")
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user