From 0612e1bc41f98a7b8dac66dc8afdf6c472d44996 Mon Sep 17 00:00:00 2001 From: Kent Overstreet Date: Sun, 12 Apr 2026 13:30:00 -0400 Subject: [PATCH] query: MCP tool uses execute_query, add double-quote strings - MCP memory_query tool now uses execute_query path instead of parse_stages, enabling full expression support (content ~, AND/OR, neighbors, etc.) instead of just Expr::All - Parser now accepts double-quoted strings ("foo") in addition to single quotes ('foo') - Added tests for double-quote syntax - Removed dead resolve_field_str function from memory.rs Co-Authored-By: Proof of Concept --- src/agent/tools/memory.rs | 62 ++------------- src/hippocampus/query/parser.rs | 134 +++++++++++++++++++++++++++++++- 2 files changed, 138 insertions(+), 58 deletions(-) diff --git a/src/agent/tools/memory.rs b/src/agent/tools/memory.rs index 588721b..c606c68 100644 --- a/src/agent/tools/memory.rs +++ b/src/agent/tools/memory.rs @@ -276,73 +276,23 @@ async fn query(args: &serde_json::Value) -> Result { let store = arc.lock().await; let graph = store.build_graph(); - let stages = crate::query_parser::parse_stages(query_str) - .map_err(|e| anyhow::anyhow!("{}", e))?; - let results = crate::search::run_query(&stages, vec![], &graph, &store, false, 100); - let keys: Vec = results.into_iter().map(|(k, _)| k).collect(); - match format { "full" => { // Rich output with full content, graph metrics, hub analysis + let results = crate::query_parser::execute_query(&store, &graph, query_str) + .map_err(|e| anyhow::anyhow!("{}", e))?; + let keys: Vec = results.into_iter().map(|r| r.key).collect(); let items = crate::subconscious::defs::keys_to_replay_items(&store, &keys, &graph); Ok(crate::subconscious::prompts::format_nodes_section(&store, &items, &graph)) } _ => { - // Compact output: check for count/select stages, else just list keys - use crate::search::{Stage, Transform}; - let has_count = stages.iter().any(|s| matches!(s, Stage::Transform(Transform::Count))); - if has_count { - return Ok(keys.len().to_string()); - } - if keys.is_empty() { - return Ok("no results".to_string()); - } - let select_fields: Option<&Vec> = stages.iter().find_map(|s| match s { - Stage::Transform(Transform::Select(f)) => Some(f), - _ => None, - }); - if let Some(fields) = select_fields { - let mut out = String::from("key\t"); - out.push_str(&fields.join("\t")); - out.push('\n'); - for key in &keys { - out.push_str(key); - for f in fields { - out.push('\t'); - out.push_str(&resolve_field_str(&store, &graph, key, f)); - } - out.push('\n'); - } - Ok(out) - } else { - Ok(keys.join("\n")) - } + // Compact output: handles count, select, and all expression types + crate::query_parser::query_to_string(&store, &graph, query_str) + .map_err(|e| anyhow::anyhow!("{}", e)) } } } -fn resolve_field_str(store: &crate::store::Store, graph: &crate::graph::Graph, key: &str, field: &str) -> String { - let node = match store.nodes.get(key) { - Some(n) => n, - None => return "-".to_string(), - }; - match field { - "key" => key.to_string(), - "weight" => format!("{:.3}", node.weight), - "node_type" => format!("{:?}", node.node_type), - "provenance" => node.provenance.clone(), - "emotion" => format!("{}", node.emotion), - "retrievals" => format!("{}", node.retrievals), - "uses" => format!("{}", node.uses), - "wrongs" => format!("{}", node.wrongs), - "created" => format!("{}", node.created_at), - "timestamp" => format!("{}", node.timestamp), - "degree" => format!("{}", graph.degree(key)), - "content_len" => format!("{}", node.content.len()), - _ => "-".to_string(), - } -} - // ── Journal tools ────────────────────────────────────────────── async fn journal_tail(args: &serde_json::Value) -> Result { diff --git a/src/hippocampus/query/parser.rs b/src/hippocampus/query/parser.rs index 6c2e826..b84935d 100644 --- a/src/hippocampus/query/parser.rs +++ b/src/hippocampus/query/parser.rs @@ -201,9 +201,22 @@ peg::parser! { rule value() -> Value = f:fn_call() { Value::FnCall(f) } - / n:number() { Value::Num(n) } / s:string() { Value::Str(s) } - / i:ident() { Value::Ident(i) } + / t:token() { t } + + // Token: number or identifier, with alphanumeric fallback (e.g., "27b") + rule token() -> Value + = n:$(['0'..='9']+ ("." ['0'..='9']+)?) !['a'..='z' | 'A'..='Z'] { + Value::Num(n.parse().unwrap()) + } + / s:$(['a'..='z' | 'A'..='Z' | '0'..='9' | '_' | '-' | '.']+) { + // Try as number first, fall back to string + if let Ok(n) = s.parse::() { + Value::Num(n) + } else { + Value::Str(s.to_string()) + } + } rule fn_call() -> FnCall = "community" _ "(" _ k:string() _ ")" { FnCall::Community(k) } @@ -216,12 +229,19 @@ peg::parser! { rule string() -> String = "'" s:$([^ '\'']*) "'" { s.to_string() } + / "\"" s:$([^ '"']*) "\"" { s.to_string() } rule ident() -> String = s:$(['a'..='z' | 'A'..='Z' | '_']['a'..='z' | 'A'..='Z' | '0'..='9' | '_' | '-' | '.']*) { s.to_string() } + // Bare word for matching (allows digits at start, e.g. "27b") + rule word() -> String + = s:$(['a'..='z' | 'A'..='Z' | '0'..='9' | '_' | '-' | '.']+) { + s.to_string() + } + // Glob pattern for key matching (allows * and ?) rule glob_pattern() -> String = s:$(['a'..='z' | 'A'..='Z' | '0'..='9' | '_' | '-' | '.' | '*' | '?']+) { @@ -830,3 +850,113 @@ fn print_connectivity(results: &[QueryResult], graph: &Graph) { } } } + +// -- Tests -- + +#[cfg(test)] +mod tests { + use super::*; + + // Helper to check if a query parses successfully + fn parses(s: &str) -> bool { + query_parser::query(s).is_ok() + } + + // Helper to get parse error for debugging + fn parse_err(s: &str) -> String { + query_parser::query(s).err().map(|e| format!("{}", e)).unwrap_or_default() + } + + #[test] + fn test_generators() { + assert!(parses("all")); + assert!(parses("*")); + assert!(parses("all | limit:10")); + } + + #[test] + fn test_pipeline_filters() { + assert!(parses("all | type:semantic")); + assert!(parses("all | type:episodic")); + assert!(parses("all | provenance:observe")); + assert!(parses("all | key:journal-*")); + assert!(parses("all | !key:_*")); // negated key glob + assert!(parses("all | age:>7d")); + assert!(parses("all | not-visited:organize,86400")); + } + + #[test] + fn test_pipeline_transforms() { + assert!(parses("all | sort:weight")); + assert!(parses("all | sort:timestamp")); + assert!(parses("all | sort:degree")); + assert!(parses("all | limit:20")); + assert!(parses("all | sort:weight | limit:10")); + } + + #[test] + fn test_composite_sort() { + // Weighted composite sort expressions (require 2+ terms with +) + assert!(parses("all | sort:degree*0.5+isolation*0.3")); + assert!(parses("all | sort:degree*0.5+isolation*0.3+recency(organize)*0.2")); + assert!(parses("all | sort:weight*0.5+degree*0.5")); + // Single field (no weight) falls back to simple sort + assert!(parses("all | sort:weight")); + } + + #[test] + fn test_expression_syntax() { + // Expression comparisons (legacy syntax) + assert!(parses("weight > 0.5")); + assert!(parses("degree >= 10")); + assert!(parses("key ~ 'journal.*'")); + assert!(parses("content ~ 27b"), "alphanumeric pattern: {}", parse_err("content ~ 27b")); + assert!(parses("content ~ qwen35")); + // Both single and double quotes work for strings + assert!(parses("content ~ '27b'")); + assert!(parses("content ~ \"27b\""), "double quotes: {}", parse_err("content ~ \"27b\"")); + assert!(parses("neighbors(\"my-key\")")); + } + + #[test] + fn test_boolean_expressions() { + assert!(parses("weight > 0.5 AND degree > 10")); + assert!(parses("key ~ 'a' OR key ~ 'b'")); + assert!(parses("NOT weight < 0.1")); + } + + #[test] + fn test_duration_parsing() { + assert!(parses("all | age:>1d")); + assert!(parses("all | age:>=24h")); + assert!(parses("all | age:<30m")); + assert!(parses("all | age:=3600s")); + assert!(parses("all | age:>86400")); // raw seconds + } + + #[test] + fn test_glob_patterns() { + assert!(parses("all | key:*")); + assert!(parses("all | key:journal-*")); + assert!(parses("all | key:*-2026-*")); + assert!(parses("all | key:dream-cycle-?")); + assert!(parses("all | !key:subconscious-*")); + } + + #[test] + fn test_complex_pipelines() { + assert!(parses("all | type:semantic | sort:weight | limit:50")); + assert!(parses("all | !key:_* | sort:degree*0.5+isolation*0.5 | limit:10")); + assert!(parses("all | provenance:observe | age:>1d | sort:timestamp | limit:20")); + } + + #[test] + fn test_parse_stages_output() { + // Ensure parse_stages produces expected Stage types + let stages = parse_stages("all | type:semantic | limit:10").unwrap(); + assert_eq!(stages.len(), 3); + assert!(matches!(stages[0], Stage::Generator(Generator::All))); + assert!(matches!(stages[1], Stage::Filter(Filter::Type(_)))); + assert!(matches!(stages[2], Stage::Transform(Transform::Limit(10)))); + } +}