From dd0715c081cba52f619c49d6fcf0990cf7d209f8 Mon Sep 17 00:00:00 2001 From: Cameron Date: Wed, 18 Mar 2026 16:53:34 -0400 Subject: [PATCH 01/13] feat: add generate_photo_description() to OllamaClient for RAG enrichment Co-Authored-By: Claude Sonnet 4.6 --- src/ai/ollama.rs | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/ai/ollama.rs b/src/ai/ollama.rs index 0c7f36b..48cc220 100644 --- a/src/ai/ollama.rs +++ b/src/ai/ollama.rs @@ -480,6 +480,22 @@ Analyze the image and use specific details from both the visual content and the .await } + /// Generate a brief visual description of a photo for use in RAG query enrichment. + /// Returns 1-2 sentences describing people, location, and activity visible in the image. + /// Only called when the model has vision capabilities. + pub async fn generate_photo_description(&self, image_base64: String) -> Result { + let prompt = "Briefly describe what you see in this image in 1-2 sentences. \ + Focus on the people, location, and activity."; + let system = "You are a scene description assistant. Be concise and factual."; + let images = vec![image_base64]; + + let description = self + .generate_with_images(prompt, Some(system), Some(images)) + .await?; + + Ok(description.trim().to_string()) + } + /// Generate an embedding vector for text using nomic-embed-text:v1.5 /// Returns a 768-dimensional vector as Vec pub async fn generate_embedding(&self, text: &str) -> Result> { @@ -664,3 +680,18 @@ struct OllamaBatchEmbedRequest { struct OllamaEmbedResponse { embeddings: Vec>, } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn generate_photo_description_prompt_is_concise() { + // Verify the method exists and its prompt is sane by checking the + // constant we'll use. This is a compile + smoke check; actual LLM + // calls are integration-tested manually. + let prompt = "Briefly describe what you see in this image in 1-2 sentences. \ + Focus on the people, location, and activity."; + assert!(prompt.len() < 200, "Prompt should be concise"); + } +} -- 2.49.1 From b31b4b903c9b01f44bc8b9fd18c421f4e058d019 Mon Sep 17 00:00:00 2001 From: Cameron Date: Wed, 18 Mar 2026 16:56:27 -0400 Subject: [PATCH 02/13] refactor: use &str for generate_photo_description image parameter Co-Authored-By: Claude Sonnet 4.6 --- src/ai/ollama.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ai/ollama.rs b/src/ai/ollama.rs index 48cc220..0c473d3 100644 --- a/src/ai/ollama.rs +++ b/src/ai/ollama.rs @@ -483,11 +483,11 @@ Analyze the image and use specific details from both the visual content and the /// Generate a brief visual description of a photo for use in RAG query enrichment. /// Returns 1-2 sentences describing people, location, and activity visible in the image. /// Only called when the model has vision capabilities. - pub async fn generate_photo_description(&self, image_base64: String) -> Result { + pub async fn generate_photo_description(&self, image_base64: &str) -> Result { let prompt = "Briefly describe what you see in this image in 1-2 sentences. \ Focus on the people, location, and activity."; let system = "You are a scene description assistant. Be concise and factual."; - let images = vec![image_base64]; + let images = vec![image_base64.to_string()]; let description = self .generate_with_images(prompt, Some(system), Some(images)) -- 2.49.1 From 387ce23afdfeca184a198f139e9343c1bbab7498 Mon Sep 17 00:00:00 2001 From: Cameron Date: Wed, 18 Mar 2026 16:59:39 -0400 Subject: [PATCH 03/13] feat: add tag_dao to InsightGenerator for tag-based context enrichment Threads SqliteTagDao through InsightGenerator and AppState (both default and test_state). Adds Send+Sync bounds to TagDao trait with unsafe impls for SqliteTagDao (always Mutex-protected) and TestTagDao (single-threaded). Co-Authored-By: Claude Sonnet 4.6 --- src/ai/insight_generator.rs | 4 ++++ src/state.rs | 7 +++++++ src/tags.rs | 10 +++++++++- 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/ai/insight_generator.rs b/src/ai/insight_generator.rs index 77ac02a..534c8aa 100644 --- a/src/ai/insight_generator.rs +++ b/src/ai/insight_generator.rs @@ -15,6 +15,7 @@ use crate::database::models::InsertPhotoInsight; use crate::database::{ CalendarEventDao, DailySummaryDao, ExifDao, InsightDao, LocationHistoryDao, SearchHistoryDao, }; +use crate::tags::TagDao; use crate::memories::extract_date_from_filename; use crate::otel::global_tracer; use crate::utils::normalize_path; @@ -45,6 +46,7 @@ pub struct InsightGenerator { calendar_dao: Arc>>, location_dao: Arc>>, search_dao: Arc>>, + tag_dao: Arc>>, base_path: String, } @@ -59,6 +61,7 @@ impl InsightGenerator { calendar_dao: Arc>>, location_dao: Arc>>, search_dao: Arc>>, + tag_dao: Arc>>, base_path: String, ) -> Self { Self { @@ -70,6 +73,7 @@ impl InsightGenerator { calendar_dao, location_dao, search_dao, + tag_dao, base_path, } } diff --git a/src/state.rs b/src/state.rs index 2bf0e7e..578ea78 100644 --- a/src/state.rs +++ b/src/state.rs @@ -5,6 +5,7 @@ use crate::database::{ SqliteLocationHistoryDao, SqliteSearchHistoryDao, }; use crate::database::{PreviewDao, SqlitePreviewDao}; +use crate::tags::{SqliteTagDao, TagDao}; use crate::video::actors::{ PlaylistGenerator, PreviewClipGenerator, StreamActor, VideoPlaylistManager, }; @@ -119,6 +120,8 @@ impl Default for AppState { Arc::new(Mutex::new(Box::new(SqliteLocationHistoryDao::new()))); let search_dao: Arc>> = Arc::new(Mutex::new(Box::new(SqliteSearchHistoryDao::new()))); + let tag_dao: Arc>> = + Arc::new(Mutex::new(Box::new(SqliteTagDao::default()))); // Load base path let base_path = env::var("BASE_PATH").expect("BASE_PATH was not set in the env"); @@ -133,6 +136,7 @@ impl Default for AppState { calendar_dao.clone(), location_dao.clone(), search_dao.clone(), + tag_dao.clone(), base_path.clone(), ); @@ -196,6 +200,8 @@ impl AppState { Arc::new(Mutex::new(Box::new(SqliteLocationHistoryDao::new()))); let search_dao: Arc>> = Arc::new(Mutex::new(Box::new(SqliteSearchHistoryDao::new()))); + let tag_dao: Arc>> = + Arc::new(Mutex::new(Box::new(SqliteTagDao::default()))); // Initialize test InsightGenerator with all data sources let base_path_str = base_path.to_string_lossy().to_string(); @@ -208,6 +214,7 @@ impl AppState { calendar_dao.clone(), location_dao.clone(), search_dao.clone(), + tag_dao.clone(), base_path_str.clone(), ); diff --git a/src/tags.rs b/src/tags.rs index 3a16c2f..360d52b 100644 --- a/src/tags.rs +++ b/src/tags.rs @@ -276,7 +276,7 @@ pub struct AddTagsRequest { pub tag_ids: Vec, } -pub trait TagDao { +pub trait TagDao: Send + Sync { fn get_all_tags( &mut self, context: &opentelemetry::Context, @@ -345,6 +345,10 @@ impl Default for SqliteTagDao { } } +// SAFETY: SqliteTagDao is always accessed through Arc>, +// so concurrent access is prevented by the Mutex. +unsafe impl Sync for SqliteTagDao {} + impl TagDao for SqliteTagDao { fn get_all_tags( &mut self, @@ -735,6 +739,10 @@ mod tests { } } + // SAFETY: TestTagDao is only used in single-threaded tests + unsafe impl Send for TestTagDao {} + unsafe impl Sync for TestTagDao {} + impl TagDao for TestTagDao { fn get_all_tags( &mut self, -- 2.49.1 From 8ecd3c6cf897de67e76fdef3c7a80a7ec4b4c66b Mon Sep 17 00:00:00 2001 From: Cameron Date: Wed, 18 Mar 2026 17:10:11 -0400 Subject: [PATCH 04/13] refactor: use Arc> in SqliteTagDao, remove unsafe impl Sync Aligns SqliteTagDao with the pattern used by SqliteExifDao and SqliteInsightDao. The unsafe impl Sync workaround is no longer needed since Arc> provides safe interior mutability and automatic Sync derivation. Co-Authored-By: Claude Sonnet 4.6 --- src/files.rs | 4 +-- src/tags.rs | 91 +++++++++++++++++++++++++++++++++++++--------------- 2 files changed, 68 insertions(+), 27 deletions(-) diff --git a/src/files.rs b/src/files.rs index 107b16a..a7355fb 100644 --- a/src/files.rs +++ b/src/files.rs @@ -1490,7 +1490,7 @@ mod tests { let request: Query = Query::from_query("path=&tag_ids=1,3&recursive=true").unwrap(); - let mut tag_dao = SqliteTagDao::new(in_memory_db_connection()); + let mut tag_dao = SqliteTagDao::new(std::sync::Arc::new(Mutex::new(in_memory_db_connection()))); let tag1 = tag_dao .create_tag(&opentelemetry::Context::current(), "tag1") @@ -1536,7 +1536,7 @@ mod tests { exp: 12345, }; - let mut tag_dao = SqliteTagDao::new(in_memory_db_connection()); + let mut tag_dao = SqliteTagDao::new(std::sync::Arc::new(Mutex::new(in_memory_db_connection()))); let tag1 = tag_dao .create_tag(&opentelemetry::Context::current(), "tag1") diff --git a/src/tags.rs b/src/tags.rs index 360d52b..5da6d6e 100644 --- a/src/tags.rs +++ b/src/tags.rs @@ -14,8 +14,8 @@ use opentelemetry::KeyValue; use opentelemetry::trace::{Span, Status, TraceContextExt, Tracer}; use schema::{tagged_photo, tags}; use serde::{Deserialize, Serialize}; -use std::borrow::BorrowMut; -use std::sync::Mutex; +use std::ops::DerefMut; +use std::sync::{Arc, Mutex}; pub fn add_tag_services(app: App) -> App where @@ -330,25 +330,23 @@ pub trait TagDao: Send + Sync { } pub struct SqliteTagDao { - connection: SqliteConnection, + connection: Arc>, } impl SqliteTagDao { - pub(crate) fn new(connection: SqliteConnection) -> Self { + pub(crate) fn new(connection: Arc>) -> Self { SqliteTagDao { connection } } } impl Default for SqliteTagDao { fn default() -> Self { - SqliteTagDao::new(connect()) + SqliteTagDao { + connection: Arc::new(Mutex::new(connect())), + } } } -// SAFETY: SqliteTagDao is always accessed through Arc>, -// so concurrent access is prevented by the Mutex. -unsafe impl Sync for SqliteTagDao {} - impl TagDao for SqliteTagDao { fn get_all_tags( &mut self, @@ -357,6 +355,10 @@ impl TagDao for SqliteTagDao { ) -> anyhow::Result> { // select name, count(*) from tags join tagged_photo ON tags.id = tagged_photo.tag_id GROUP BY tags.name ORDER BY COUNT(*); + let mut conn = self + .connection + .lock() + .expect("Unable to lock SqliteTagDao connection"); trace_db_call(context, "query", "get_all_tags", |span| { span.set_attribute(KeyValue::new("path", path.clone().unwrap_or_default())); @@ -367,7 +369,7 @@ impl TagDao for SqliteTagDao { .group_by(tags::id) .select((count_star(), id, name, created_time)) .filter(tagged_photo::photo_name.like(path)) - .get_results(&mut self.connection) + .get_results(conn.deref_mut()) .map::, _>(|tags_with_count: Vec<(i64, i32, String, i64)>| { tags_with_count .iter() @@ -392,6 +394,10 @@ impl TagDao for SqliteTagDao { context: &opentelemetry::Context, path: &str, ) -> anyhow::Result> { + let mut conn = self + .connection + .lock() + .expect("Unable to lock SqliteTagDao connection"); trace_db_call(context, "query", "get_tags_for_path", |span| { span.set_attribute(KeyValue::new("path", path.to_string())); @@ -400,12 +406,16 @@ impl TagDao for SqliteTagDao { .left_join(tagged_photo::table) .filter(tagged_photo::photo_name.eq(&path)) .select((tags::id, tags::name, tags::created_time)) - .get_results::(self.connection.borrow_mut()) + .get_results::(conn.deref_mut()) .with_context(|| "Unable to get tags from Sqlite") }) } fn create_tag(&mut self, context: &opentelemetry::Context, name: &str) -> anyhow::Result { + let mut conn = self + .connection + .lock() + .expect("Unable to lock SqliteTagDao connection"); trace_db_call(context, "insert", "create_tag", |span| { span.set_attribute(KeyValue::new("name", name.to_string())); @@ -414,7 +424,7 @@ impl TagDao for SqliteTagDao { name: name.to_string(), created_time: Utc::now().timestamp(), }) - .execute(&mut self.connection) + .execute(conn.deref_mut()) .with_context(|| format!("Unable to insert tag {:?} in Sqlite", name)) .and_then(|_| { info!("Inserted tag: {:?}", name); @@ -422,7 +432,7 @@ impl TagDao for SqliteTagDao { fn last_insert_rowid() -> Integer; } diesel::select(last_insert_rowid()) - .get_result::(&mut self.connection) + .get_result::(conn.deref_mut()) .with_context(|| "Unable to get last inserted tag from Sqlite") }) .and_then(|id| { @@ -430,7 +440,7 @@ impl TagDao for SqliteTagDao { tags::table .filter(tags::id.eq(id)) .select((tags::id, tags::name, tags::created_time)) - .get_result::(self.connection.borrow_mut()) + .get_result::(conn.deref_mut()) .with_context(|| { format!("Unable to get tagged photo with id: {:?} from Sqlite", id) }) @@ -444,6 +454,10 @@ impl TagDao for SqliteTagDao { tag_name: &str, path: &str, ) -> anyhow::Result> { + let mut conn = self + .connection + .lock() + .expect("Unable to lock SqliteTagDao connection"); trace_db_call(context, "delete", "remove_tag", |span| { span.set_attributes(vec![ KeyValue::new("tag_name", tag_name.to_string()), @@ -452,7 +466,7 @@ impl TagDao for SqliteTagDao { tags::table .filter(tags::name.eq(tag_name)) - .get_result::(self.connection.borrow_mut()) + .get_result::(conn.deref_mut()) .optional() .with_context(|| format!("Unable to get tag '{}'", tag_name)) .and_then(|tag| { @@ -462,7 +476,7 @@ impl TagDao for SqliteTagDao { .filter(tagged_photo::tag_id.eq(tag.id)) .filter(tagged_photo::photo_name.eq(path)), ) - .execute(&mut self.connection) + .execute(conn.deref_mut()) .with_context(|| format!("Unable to delete tag: '{}'", &tag.name)) .map(|_| Some(())) } else { @@ -479,6 +493,10 @@ impl TagDao for SqliteTagDao { path: &str, tag_id: i32, ) -> anyhow::Result { + let mut conn = self + .connection + .lock() + .expect("Unable to lock SqliteTagDao connection"); trace_db_call(context, "insert", "tag_file", |span| { span.set_attributes(vec![ KeyValue::new("path", path.to_string()), @@ -491,7 +509,7 @@ impl TagDao for SqliteTagDao { photo_name: path.to_string(), created_time: Utc::now().timestamp(), }) - .execute(self.connection.borrow_mut()) + .execute(conn.deref_mut()) .with_context(|| format!("Unable to tag file {:?} in sqlite", path)) .and_then(|_| { info!("Inserted tagged photo: {:#} -> {:?}", tag_id, path); @@ -499,13 +517,13 @@ impl TagDao for SqliteTagDao { fn last_insert_rowid() -> diesel::sql_types::Integer; } diesel::select(last_insert_rowid()) - .get_result::(&mut self.connection) + .get_result::(conn.deref_mut()) .with_context(|| "Unable to get last inserted tag from Sqlite") }) .and_then(|tagged_id| { tagged_photo::table .find(tagged_id) - .first(self.connection.borrow_mut()) + .first(conn.deref_mut()) .with_context(|| { format!( "Error getting inserted tagged photo with id: {:?}", @@ -522,6 +540,10 @@ impl TagDao for SqliteTagDao { exclude_tag_ids: Vec, context: &opentelemetry::Context, ) -> anyhow::Result> { + let mut conn = self + .connection + .lock() + .expect("Unable to lock SqliteTagDao connection"); trace_db_call(context, "query", "get_files_with_all_tags", |_| { use diesel::dsl::*; @@ -568,7 +590,7 @@ impl TagDao for SqliteTagDao { .fold(query, |q, id| q.bind::(id)); query - .load::(&mut self.connection) + .load::(conn.deref_mut()) .with_context(|| "Unable to get tagged photos with all specified tags") }) } @@ -579,6 +601,10 @@ impl TagDao for SqliteTagDao { exclude_tag_ids: Vec, context: &opentelemetry::Context, ) -> anyhow::Result> { + let mut conn = self + .connection + .lock() + .expect("Unable to lock SqliteTagDao connection"); trace_db_call(context, "query", "get_files_with_any_tags", |_| { use diesel::dsl::*; // Create the placeholders for the IN clauses @@ -620,7 +646,7 @@ impl TagDao for SqliteTagDao { .fold(query, |q, id| q.bind::(id)); query - .load::(&mut self.connection) + .load::(conn.deref_mut()) .with_context(|| "Unable to get tagged photos") }) } @@ -633,9 +659,13 @@ impl TagDao for SqliteTagDao { ) -> anyhow::Result<()> { use crate::database::schema::tagged_photo::dsl::*; + let mut conn = self + .connection + .lock() + .expect("Unable to lock SqliteTagDao connection"); diesel::update(tagged_photo.filter(photo_name.eq(old_name))) .set(photo_name.eq(new_name)) - .execute(&mut self.connection)?; + .execute(conn.deref_mut())?; Ok(()) } @@ -645,10 +675,14 @@ impl TagDao for SqliteTagDao { ) -> anyhow::Result> { use crate::database::schema::tagged_photo::dsl::*; + let mut conn = self + .connection + .lock() + .expect("Unable to lock SqliteTagDao connection"); tagged_photo .select(photo_name) .distinct() - .load(&mut self.connection) + .load(conn.deref_mut()) .with_context(|| "Unable to get photo names") } @@ -659,6 +693,10 @@ impl TagDao for SqliteTagDao { ) -> anyhow::Result> { use std::collections::HashMap; + let mut conn = self + .connection + .lock() + .expect("Unable to lock SqliteTagDao connection"); trace_db_call(context, "query", "get_tag_counts_batch", |span| { span.set_attribute(KeyValue::new("file_count", file_paths.len() as i64)); @@ -701,7 +739,7 @@ impl TagDao for SqliteTagDao { // Execute query and convert to HashMap query - .load::(&mut self.connection) + .load::(conn.deref_mut()) .with_context(|| "Unable to get batch tag counts") .map(|rows| { rows.into_iter() @@ -739,7 +777,10 @@ mod tests { } } - // SAFETY: TestTagDao is only used in single-threaded tests + // SAFETY: TestTagDao uses RefCell fields which are !Send because they allow + // multiple mutable borrows without coordination. This impl is sound because + // TestTagDao is test-only, used within a single test function, and never moved + // into spawned tasks or shared across threads. unsafe impl Send for TestTagDao {} unsafe impl Sync for TestTagDao {} -- 2.49.1 From c0d27d0b9eba10cc82e6719969295ad7eddc626a Mon Sep 17 00:00:00 2001 From: Cameron Date: Wed, 18 Mar 2026 17:14:00 -0400 Subject: [PATCH 05/13] feat: add Tags section to combine_contexts() for insight context Co-Authored-By: Claude Sonnet 4.6 --- src/ai/insight_generator.rs | 42 ++++++++++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/src/ai/insight_generator.rs b/src/ai/insight_generator.rs index 534c8aa..9efb6cf 100644 --- a/src/ai/insight_generator.rs +++ b/src/ai/insight_generator.rs @@ -15,9 +15,9 @@ use crate::database::models::InsertPhotoInsight; use crate::database::{ CalendarEventDao, DailySummaryDao, ExifDao, InsightDao, LocationHistoryDao, SearchHistoryDao, }; -use crate::tags::TagDao; use crate::memories::extract_date_from_filename; use crate::otel::global_tracer; +use crate::tags::TagDao; use crate::utils::normalize_path; #[derive(Deserialize)] @@ -589,6 +589,7 @@ impl InsightGenerator { calendar: Option, location: Option, search: Option, + tags: Option, ) -> String { let mut parts = Vec::new(); @@ -604,6 +605,9 @@ impl InsightGenerator { if let Some(s) = search { parts.push(format!("## Searches\n{}", s)); } + if let Some(t) = tags { + parts.push(format!("## Tags\n{}", t)); + } if parts.is_empty() { "No additional context available".to_string() @@ -955,6 +959,7 @@ impl InsightGenerator { calendar_context, location_context, search_context, + None, // tags — wired up in Task 5 ); log::info!( @@ -1301,3 +1306,38 @@ Return ONLY the summary, nothing else."#, data.display_name } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn combine_contexts_includes_tags_section_when_tags_present() { + let result = InsightGenerator::combine_contexts( + None, + None, + None, + None, + Some("vacation, hiking, mountains".to_string()), + ); + assert!(result.contains("## Tags"), "Should include Tags section"); + assert!(result.contains("vacation, hiking, mountains"), "Should include tag names"); + } + + #[test] + fn combine_contexts_omits_tags_section_when_no_tags() { + let result = InsightGenerator::combine_contexts( + Some("some messages".to_string()), + None, None, None, + None, // no tags + ); + assert!(!result.contains("## Tags"), "Should not include Tags section when None"); + assert!(result.contains("## Messages"), "Should still include Messages"); + } + + #[test] + fn combine_contexts_returns_no_context_message_when_all_none() { + let result = InsightGenerator::combine_contexts(None, None, None, None, None); + assert_eq!(result, "No additional context available"); + } +} -- 2.49.1 From e58b8fe7437b708f4c353e51feebe1597b6f6b08 Mon Sep 17 00:00:00 2001 From: Cameron Date: Wed, 18 Mar 2026 17:17:21 -0400 Subject: [PATCH 06/13] feat: add enrichment parameter to gather_search_context() replacing weak metadata query Co-Authored-By: Claude Sonnet 4.6 --- src/ai/insight_generator.rs | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/src/ai/insight_generator.rs b/src/ai/insight_generator.rs index 9efb6cf..dbad90b 100644 --- a/src/ai/insight_generator.rs +++ b/src/ai/insight_generator.rs @@ -512,22 +512,29 @@ impl InsightGenerator { timestamp: i64, location: Option<&str>, contact: Option<&str>, + enrichment: Option<&str>, ) -> Result> { let tracer = global_tracer(); let span = tracer.start_with_context("ai.context.search", parent_cx); let search_cx = parent_cx.with_span(span); - // Build semantic query from metadata - let query_text = format!( - "searches about {} {} {}", - DateTime::from_timestamp(timestamp, 0) - .map(|dt| dt.format("%B %Y").to_string()) - .unwrap_or_default(), - location.unwrap_or(""), - contact - .map(|c| format!("involving {}", c)) - .unwrap_or_default() - ); + // Use enrichment (topics + photo description + tags) if available; + // fall back to generic temporal query. + let query_text = if let Some(enriched) = enrichment { + enriched.to_string() + } else { + // Fallback: generic temporal query + format!( + "searches about {} {} {}", + DateTime::from_timestamp(timestamp, 0) + .map(|dt| dt.format("%B %Y").to_string()) + .unwrap_or_default(), + location.unwrap_or(""), + contact + .map(|c| format!("involving {}", c)) + .unwrap_or_default() + ) + }; let query_embedding = match self.ollama.generate_embedding(&query_text).await { Ok(emb) => emb, @@ -948,6 +955,7 @@ impl InsightGenerator { timestamp, location.as_deref(), contact.as_deref(), + None, // enrichment — wired up in Task 5 ) .await .ok() -- 2.49.1 From 8196ef94a099007292c4b437507c4539825d3df3 Mon Sep 17 00:00:00 2001 From: Cameron Date: Wed, 18 Mar 2026 17:23:49 -0400 Subject: [PATCH 07/13] =?UTF-8?q?feat:=20photo-first=20RAG=20enrichment=20?= =?UTF-8?q?=E2=80=94=20early=20vision=20description=20+=20tags=20in=20RAG?= =?UTF-8?q?=20and=20search=20context?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Sonnet 4.6 --- src/ai/insight_generator.rs | 193 +++++++++++++++++++++++++----------- 1 file changed, 134 insertions(+), 59 deletions(-) diff --git a/src/ai/insight_generator.rs b/src/ai/insight_generator.rs index dbad90b..be513e8 100644 --- a/src/ai/insight_generator.rs +++ b/src/ai/insight_generator.rs @@ -153,6 +153,7 @@ impl InsightGenerator { contact: Option<&str>, topics: Option<&[String]>, limit: usize, + extra_context: Option<&str>, ) -> Result> { let tracer = global_tracer(); let span = tracer.start_with_context("ai.rag.filter_historical", parent_cx); @@ -174,7 +175,7 @@ impl InsightGenerator { } let query_results = self - .find_relevant_messages_rag(date, location, contact, topics, limit * 2) + .find_relevant_messages_rag(date, location, contact, topics, limit * 2, extra_context) .await?; filter_cx.span().set_attribute(KeyValue::new( @@ -236,6 +237,7 @@ impl InsightGenerator { contact: Option<&str>, topics: Option<&[String]>, limit: usize, + extra_context: Option<&str>, ) -> Result> { let tracer = global_tracer(); let current_cx = opentelemetry::Context::current(); @@ -250,7 +252,7 @@ impl InsightGenerator { } // Build query string - prioritize topics if available (semantically meaningful) - let query = if let Some(topics) = topics { + let base_query = if let Some(topics) = topics { if !topics.is_empty() { // Use topics for semantic search - these are actual content keywords let topic_str = topics.join(", "); @@ -268,6 +270,12 @@ impl InsightGenerator { Self::build_metadata_query(date, location, contact) }; + let query = if let Some(extra) = extra_context { + format!("{}. {}", base_query, extra) + } else { + base_query + }; + span.set_attribute(KeyValue::new("query", query.clone())); // Create context with this span for child operations @@ -718,6 +726,20 @@ impl InsightGenerator { .set_attribute(KeyValue::new("contact", c.clone())); } + // Fetch file tags (used to enrich RAG and final context) + let tag_names: Vec = { + let mut dao = self.tag_dao.lock().expect("Unable to lock TagDao"); + dao.get_tags_for_path(&insight_cx, &file_path) + .unwrap_or_else(|e| { + log::warn!("Failed to fetch tags for insight {}: {}", file_path, e); + Vec::new() + }) + .into_iter() + .map(|t| t.name) + .collect() + }; + log::info!("Fetched {} tags for photo: {:?}", tag_names.len(), tag_names); + // 4. Get location name from GPS coordinates (needed for RAG query) let location = match exif { Some(ref exif) => { @@ -744,6 +766,90 @@ impl InsightGenerator { None => None, }; + // Check if the model has vision capabilities + let model_to_check = ollama_client.primary_model.clone(); + let has_vision = match OllamaClient::check_model_capabilities( + &ollama_client.primary_url, + &model_to_check, + ) + .await + { + Ok(capabilities) => { + log::info!( + "Model '{}' vision capability: {}", + model_to_check, + capabilities.has_vision + ); + capabilities.has_vision + } + Err(e) => { + log::warn!( + "Failed to check vision capabilities for model '{}', assuming no vision support: {}", + model_to_check, + e + ); + false + } + }; + + insight_cx + .span() + .set_attribute(KeyValue::new("model_has_vision", has_vision)); + + // Load image and encode as base64 only if model supports vision + let image_base64 = if has_vision { + match self.load_image_as_base64(&file_path) { + Ok(b64) => { + log::info!( + "Successfully loaded image for vision-capable model '{}'", + model_to_check + ); + Some(b64) + } + Err(e) => { + log::warn!("Failed to load image for vision model: {}", e); + None + } + } + } else { + log::info!( + "Model '{}' does not support vision, skipping image processing", + model_to_check + ); + None + }; + + // Generate brief photo description for RAG enrichment (vision models only) + let photo_description: Option = if let Some(ref img_b64) = image_base64 { + match ollama_client.generate_photo_description(img_b64).await { + Ok(desc) => { + log::info!("Photo description for RAG enrichment: {}", desc); + Some(desc) + } + Err(e) => { + log::warn!("Failed to generate photo description for RAG enrichment: {}", e); + None + } + } + } else { + None + }; + + // Build enriched context string for RAG: photo description + tags + // (SMS topics are passed separately to RAG functions) + let enriched_query: Option = { + let mut parts: Vec = Vec::new(); + if let Some(ref desc) = photo_description { + parts.push(desc.clone()); + } + if !tag_names.is_empty() { + parts.push(format!("tags: {}", tag_names.join(", "))); + } + if parts.is_empty() { None } else { Some(parts.join(". ")) } + }; + + let mut search_enrichment: Option = enriched_query.clone(); + // 5. Intelligent retrieval: Hybrid approach for better context let mut sms_summary = None; let mut used_rag = false; @@ -782,6 +888,21 @@ impl InsightGenerator { log::info!("Extracted topics for query enrichment: {:?}", topics); + // Build full search enrichment: SMS topics + photo description + tag names + search_enrichment = { + let mut parts: Vec = Vec::new(); + if !topics.is_empty() { + parts.push(topics.join(", ")); + } + if let Some(ref desc) = photo_description { + parts.push(desc.clone()); + } + if !tag_names.is_empty() { + parts.push(format!("tags: {}", tag_names.join(", "))); + } + if parts.is_empty() { None } else { Some(parts.join(". ")) } + }; + // Step 3: Try historical RAG (>30 days ago) using extracted topics let topics_slice = if topics.is_empty() { None @@ -796,6 +917,7 @@ impl InsightGenerator { contact.as_deref(), topics_slice, 10, // Top 10 historical matches + enriched_query.as_deref(), ) .await { @@ -858,7 +980,7 @@ impl InsightGenerator { log::info!("No immediate messages found, trying basic RAG as fallback"); // Fallback to basic RAG even without strong query match self - .find_relevant_messages_rag(date_taken, None, contact.as_deref(), None, 20) + .find_relevant_messages_rag(date_taken, None, contact.as_deref(), None, 20, enriched_query.as_deref()) .await { Ok(rag_messages) if !rag_messages.is_empty() => { @@ -955,19 +1077,25 @@ impl InsightGenerator { timestamp, location.as_deref(), contact.as_deref(), - None, // enrichment — wired up in Task 5 + search_enrichment.as_deref(), ) .await .ok() .flatten(); // 7. Combine all context sources with equal weight + let tags_context = if tag_names.is_empty() { + None + } else { + Some(tag_names.join(", ")) + }; + let combined_context = Self::combine_contexts( sms_summary, calendar_context, location_context, search_context, - None, // tags — wired up in Task 5 + tags_context, ); log::info!( @@ -975,59 +1103,6 @@ impl InsightGenerator { combined_context.len() ); - // 8. Check if the model has vision capabilities - let model_to_check = ollama_client.primary_model.clone(); - let has_vision = match OllamaClient::check_model_capabilities( - &ollama_client.primary_url, - &model_to_check, - ) - .await - { - Ok(capabilities) => { - log::info!( - "Model '{}' vision capability: {}", - model_to_check, - capabilities.has_vision - ); - capabilities.has_vision - } - Err(e) => { - log::warn!( - "Failed to check vision capabilities for model '{}', assuming no vision support: {}", - model_to_check, - e - ); - false - } - }; - - insight_cx - .span() - .set_attribute(KeyValue::new("model_has_vision", has_vision)); - - // 9. Load image and encode as base64 only if model supports vision - let image_base64 = if has_vision { - match self.load_image_as_base64(&file_path) { - Ok(b64) => { - log::info!( - "Successfully loaded image for vision-capable model '{}'", - model_to_check - ); - Some(b64) - } - Err(e) => { - log::warn!("Failed to load image for vision model: {}", e); - None - } - } - } else { - log::info!( - "Model '{}' does not support vision, skipping image processing", - model_to_check - ); - None - }; - // 10. Generate summary first, then derive title from the summary let summary = ollama_client .generate_photo_summary( @@ -1036,7 +1111,7 @@ impl InsightGenerator { contact.as_deref(), Some(&combined_context), custom_system_prompt.as_deref(), - image_base64, + image_base64.clone(), ) .await?; -- 2.49.1 From 5e5a2a31677df2fd6ebfbd2db7709089c25f8896 Mon Sep 17 00:00:00 2001 From: Cameron Date: Wed, 18 Mar 2026 22:55:20 -0400 Subject: [PATCH 08/13] feat: add tool-calling types, chat_with_tools(), and has_tool_calling capability detection Co-Authored-By: Claude Sonnet 4.6 --- src/ai/ollama.rs | 216 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 212 insertions(+), 4 deletions(-) diff --git a/src/ai/ollama.rs b/src/ai/ollama.rs index 0c473d3..481b716 100644 --- a/src/ai/ollama.rs +++ b/src/ai/ollama.rs @@ -1,4 +1,4 @@ -use anyhow::Result; +use anyhow::{Context, Result}; use chrono::NaiveDate; use reqwest::Client; use serde::{Deserialize, Serialize}; @@ -176,10 +176,13 @@ impl OllamaClient { // Check if "vision" is in the capabilities array let has_vision = show_response.capabilities.iter().any(|cap| cap == "vision"); + // Check if "tools" is in the capabilities array + let has_tool_calling = show_response.capabilities.iter().any(|cap| cap == "tools"); Ok(ModelCapabilities { name: model_name.to_string(), has_vision, + has_tool_calling, }) } @@ -206,10 +209,11 @@ impl OllamaClient { Ok(cap) => capabilities.push(cap), Err(e) => { log::warn!("Failed to get capabilities for model {}: {}", model_name, e); - // Fallback: assume no vision if we can't check + // Fallback: assume no vision/tools if we can't check capabilities.push(ModelCapabilities { name: model_name, has_vision: false, + has_tool_calling: false, }); } } @@ -254,7 +258,7 @@ impl OllamaClient { prompt: prompt.to_string(), stream: false, system: system.map(|s| s.to_string()), - options: self.num_ctx.map(|ctx| OllamaOptions { num_ctx: ctx }), + options: self.num_ctx.map(|ctx| OllamaOptions { num_ctx: Some(ctx) }), images, }; @@ -496,6 +500,119 @@ Analyze the image and use specific details from both the visual content and the Ok(description.trim().to_string()) } + /// Send a chat request with tool definitions to /api/chat. + /// Returns the assistant's response message (may contain tool_calls or final content). + /// Uses primary/fallback URL routing same as other generation methods. + pub async fn chat_with_tools( + &self, + messages: Vec, + tools: Vec, + ) -> Result { + // Try primary server first + log::info!( + "Attempting chat_with_tools with primary server: {} (model: {})", + self.primary_url, + self.primary_model + ); + let primary_result = self + .try_chat_with_tools(&self.primary_url, messages.clone(), tools.clone()) + .await; + + match primary_result { + Ok(response) => { + log::info!("Successfully got chat_with_tools response from primary server"); + Ok(response) + } + Err(e) => { + log::warn!("Primary server chat_with_tools failed: {}", e); + + // Try fallback server if available + if let Some(fallback_url) = &self.fallback_url { + let fallback_model = self + .fallback_model + .as_ref() + .unwrap_or(&self.primary_model); + + log::info!( + "Attempting chat_with_tools with fallback server: {} (model: {})", + fallback_url, + fallback_model + ); + match self + .try_chat_with_tools(fallback_url, messages, tools) + .await + { + Ok(response) => { + log::info!( + "Successfully got chat_with_tools response from fallback server" + ); + Ok(response) + } + Err(fallback_e) => { + log::error!( + "Fallback server chat_with_tools also failed: {}", + fallback_e + ); + Err(anyhow::anyhow!( + "Both primary and fallback servers failed. Primary: {}, Fallback: {}", + e, + fallback_e + )) + } + } + } else { + log::error!("No fallback server configured"); + Err(e) + } + } + } + } + + async fn try_chat_with_tools( + &self, + base_url: &str, + messages: Vec, + tools: Vec, + ) -> Result { + let url = format!("{}/api/chat", base_url); + let model = if base_url == self.primary_url { + &self.primary_model + } else { + self.fallback_model.as_deref().unwrap_or(&self.primary_model) + }; + + let options = self.num_ctx.map(|ctx| OllamaOptions { num_ctx: Some(ctx) }); + + let request_body = OllamaChatRequest { + model, + messages: &messages, + stream: false, + tools, + options, + }; + + let response = self + .client + .post(&url) + .json(&request_body) + .send() + .await + .with_context(|| format!("Failed to connect to Ollama at {}", url))?; + + if !response.status().is_success() { + let status = response.status(); + let body = response.text().await.unwrap_or_default(); + anyhow::bail!("Ollama chat request failed with status {}: {}", status, body); + } + + let chat_response: OllamaChatResponse = response + .json() + .await + .with_context(|| "Failed to parse Ollama chat response")?; + + Ok(chat_response.message) + } + /// Generate an embedding vector for text using nomic-embed-text:v1.5 /// Returns a 768-dimensional vector as Vec pub async fn generate_embedding(&self, text: &str) -> Result> { @@ -640,7 +757,97 @@ struct OllamaRequest { #[derive(Serialize)] struct OllamaOptions { - num_ctx: i32, + num_ctx: Option, +} + +/// Tool definition sent in /api/chat requests (OpenAI-compatible format) +#[derive(Serialize, Clone, Debug)] +pub struct Tool { + #[serde(rename = "type")] + pub tool_type: String, // always "function" + pub function: ToolFunction, +} + +#[derive(Serialize, Clone, Debug)] +pub struct ToolFunction { + pub name: String, + pub description: String, + pub parameters: serde_json::Value, +} + +impl Tool { + pub fn function(name: &str, description: &str, parameters: serde_json::Value) -> Self { + Self { + tool_type: "function".to_string(), + function: ToolFunction { + name: name.to_string(), + description: description.to_string(), + parameters, + }, + } + } +} + +/// A message in the chat conversation history +#[derive(Serialize, Deserialize, Clone, Debug)] +pub struct ChatMessage { + pub role: String, // "system" | "user" | "assistant" | "tool" + /// Empty string (not null) when tool_calls is present — Ollama quirk + #[serde(default)] + pub content: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub tool_calls: Option>, + /// Base64 images — only on user messages to vision-capable models + #[serde(skip_serializing_if = "Option::is_none")] + pub images: Option>, +} + +impl ChatMessage { + pub fn system(content: impl Into) -> Self { + Self { role: "system".to_string(), content: content.into(), tool_calls: None, images: None } + } + pub fn user(content: impl Into) -> Self { + Self { role: "user".to_string(), content: content.into(), tool_calls: None, images: None } + } + pub fn tool_result(content: impl Into) -> Self { + Self { role: "tool".to_string(), content: content.into(), tool_calls: None, images: None } + } +} + +/// Tool call returned by the model in an assistant message +#[derive(Serialize, Deserialize, Clone, Debug)] +pub struct ToolCall { + pub function: ToolCallFunction, + #[serde(skip_serializing_if = "Option::is_none")] + pub id: Option, +} + +#[derive(Serialize, Deserialize, Clone, Debug)] +pub struct ToolCallFunction { + pub name: String, + /// Native JSON object (NOT a JSON-encoded string like OpenAI) + pub arguments: serde_json::Value, +} + +#[derive(Serialize)] +struct OllamaChatRequest<'a> { + model: &'a str, + messages: &'a [ChatMessage], + stream: bool, + #[serde(skip_serializing_if = "Vec::is_empty")] + tools: Vec, + #[serde(skip_serializing_if = "Option::is_none")] + options: Option, +} + +#[derive(Deserialize, Debug)] +struct OllamaChatResponse { + message: ChatMessage, + #[allow(dead_code)] + done: bool, + #[serde(default)] + #[allow(dead_code)] + done_reason: String, } #[derive(Deserialize)] @@ -668,6 +875,7 @@ struct OllamaShowResponse { pub struct ModelCapabilities { pub name: String, pub has_vision: bool, + pub has_tool_calling: bool, } #[derive(Serialize)] -- 2.49.1 From 7615b9c99bf3e9ddf690b3bc61c863edb0f2c95e Mon Sep 17 00:00:00 2001 From: Cameron Date: Wed, 18 Mar 2026 23:00:41 -0400 Subject: [PATCH 09/13] feat: add tool executors and generate_agentic_insight_for_photo() to InsightGenerator Add 6 tool executor methods (search_rag, get_sms_messages, get_calendar_events, get_location_history, get_file_tags, describe_photo) and the agentic loop that uses Ollama's chat_with_tools API to let the model decide which context to gather before writing the final photo insight. Co-Authored-By: Claude Sonnet 4.6 --- src/ai/insight_generator.rs | 754 +++++++++++++++++++++++++++++++++++- 1 file changed, 752 insertions(+), 2 deletions(-) diff --git a/src/ai/insight_generator.rs b/src/ai/insight_generator.rs index be513e8..6434314 100644 --- a/src/ai/insight_generator.rs +++ b/src/ai/insight_generator.rs @@ -1,6 +1,6 @@ use anyhow::Result; use base64::Engine as _; -use chrono::{DateTime, Utc}; +use chrono::{DateTime, NaiveDate, Utc}; use image::ImageFormat; use opentelemetry::KeyValue; use opentelemetry::trace::{Span, Status, TraceContextExt, Tracer}; @@ -9,7 +9,7 @@ use std::fs::File; use std::io::Cursor; use std::sync::{Arc, Mutex}; -use crate::ai::ollama::OllamaClient; +use crate::ai::ollama::{ChatMessage, OllamaClient, Tool}; use crate::ai::sms_client::SmsApiClient; use crate::database::models::InsertPhotoInsight; use crate::database::{ @@ -1314,6 +1314,756 @@ Return ONLY the summary, nothing else."#, } } + // ── Tool executors for agentic loop ──────────────────────────────── + + /// Dispatch a tool call to the appropriate executor + async fn execute_tool( + &self, + tool_name: &str, + arguments: &serde_json::Value, + ollama: &OllamaClient, + image_base64: &Option, + cx: &opentelemetry::Context, + ) -> String { + match tool_name { + "search_rag" => self.tool_search_rag(arguments, cx).await, + "get_sms_messages" => self.tool_get_sms_messages(arguments, cx).await, + "get_calendar_events" => self.tool_get_calendar_events(arguments, cx).await, + "get_location_history" => self.tool_get_location_history(arguments, cx).await, + "get_file_tags" => self.tool_get_file_tags(arguments, cx).await, + "describe_photo" => self.tool_describe_photo(ollama, image_base64).await, + unknown => format!("Unknown tool: {}", unknown), + } + } + + /// Tool: search_rag — semantic search over daily summaries + async fn tool_search_rag( + &self, + args: &serde_json::Value, + _cx: &opentelemetry::Context, + ) -> String { + let query = match args.get("query").and_then(|v| v.as_str()) { + Some(q) => q.to_string(), + None => return "Error: missing required parameter 'query'".to_string(), + }; + let date_str = match args.get("date").and_then(|v| v.as_str()) { + Some(d) => d, + None => return "Error: missing required parameter 'date'".to_string(), + }; + let date = match NaiveDate::parse_from_str(date_str, "%Y-%m-%d") { + Ok(d) => d, + Err(e) => return format!("Error: failed to parse date '{}': {}", date_str, e), + }; + let contact = args + .get("contact") + .and_then(|v| v.as_str()) + .map(|s| s.to_string()); + + log::info!( + "tool_search_rag: query='{}', date={}, contact={:?}", + query, + date, + contact + ); + + match self + .find_relevant_messages_rag(date, None, contact.as_deref(), None, 5, Some(&query)) + .await + { + Ok(results) if !results.is_empty() => results.join("\n\n"), + Ok(_) => "No relevant messages found.".to_string(), + Err(e) => format!("Error searching RAG: {}", e), + } + } + + /// Tool: get_sms_messages — fetch SMS messages near a date for a contact + async fn tool_get_sms_messages( + &self, + args: &serde_json::Value, + _cx: &opentelemetry::Context, + ) -> String { + let date_str = match args.get("date").and_then(|v| v.as_str()) { + Some(d) => d, + None => return "Error: missing required parameter 'date'".to_string(), + }; + let contact = match args.get("contact").and_then(|v| v.as_str()) { + Some(c) => c.to_string(), + None => return "Error: missing required parameter 'contact'".to_string(), + }; + let days_radius = args + .get("days_radius") + .and_then(|v| v.as_i64()) + .unwrap_or(4); + + let date = match NaiveDate::parse_from_str(date_str, "%Y-%m-%d") { + Ok(d) => d, + Err(e) => return format!("Error: failed to parse date '{}': {}", date_str, e), + }; + let timestamp = date + .and_hms_opt(12, 0, 0) + .unwrap() + .and_utc() + .timestamp(); + + log::info!( + "tool_get_sms_messages: date={}, contact='{}', days_radius={}", + date, + contact, + days_radius + ); + + // Use the SMS client's existing fetch mechanism + // Build start/end from days_radius + let start_ts = timestamp - (days_radius * 86400); + let end_ts = timestamp + (days_radius * 86400); + + let center_dt = DateTime::from_timestamp(timestamp, 0); + let start_dt = DateTime::from_timestamp(start_ts, 0); + let end_dt = DateTime::from_timestamp(end_ts, 0); + + if center_dt.is_none() || start_dt.is_none() || end_dt.is_none() { + return "Error: invalid timestamp range".to_string(); + } + + match self + .sms_client + .fetch_messages_for_contact(Some(&contact), timestamp) + .await + { + Ok(messages) if !messages.is_empty() => { + let formatted: Vec = messages + .iter() + .take(30) + .map(|m| { + let sender = if m.is_sent { "Me" } else { &m.contact }; + let ts = DateTime::from_timestamp(m.timestamp, 0) + .map(|dt| dt.format("%Y-%m-%d %H:%M").to_string()) + .unwrap_or_else(|| "unknown".to_string()); + format!("[{}] {}: {}", ts, sender, m.body) + }) + .collect(); + format!("Found {} messages:\n{}", messages.len(), formatted.join("\n")) + } + Ok(_) => "No messages found.".to_string(), + Err(e) => format!("Error fetching SMS messages: {}", e), + } + } + + /// Tool: get_calendar_events — fetch calendar events near a date + async fn tool_get_calendar_events( + &self, + args: &serde_json::Value, + cx: &opentelemetry::Context, + ) -> String { + let date_str = match args.get("date").and_then(|v| v.as_str()) { + Some(d) => d, + None => return "Error: missing required parameter 'date'".to_string(), + }; + let days_radius = args + .get("days_radius") + .and_then(|v| v.as_i64()) + .unwrap_or(7); + + let date = match NaiveDate::parse_from_str(date_str, "%Y-%m-%d") { + Ok(d) => d, + Err(e) => return format!("Error: failed to parse date '{}': {}", date_str, e), + }; + let timestamp = date + .and_hms_opt(12, 0, 0) + .unwrap() + .and_utc() + .timestamp(); + + log::info!( + "tool_get_calendar_events: date={}, days_radius={}", + date, + days_radius + ); + + let events = { + let mut dao = self + .calendar_dao + .lock() + .expect("Unable to lock CalendarEventDao"); + dao.find_relevant_events_hybrid(cx, timestamp, days_radius, None, 10) + .ok() + }; + + match events { + Some(evts) if !evts.is_empty() => { + let formatted: Vec = evts + .iter() + .map(|e| { + let dt = DateTime::from_timestamp(e.start_time, 0) + .map(|dt| dt.format("%Y-%m-%d %H:%M").to_string()) + .unwrap_or_else(|| "unknown".to_string()); + let loc = e + .location + .as_ref() + .map(|l| format!(" at {}", l)) + .unwrap_or_default(); + let attendees = e + .attendees + .as_ref() + .and_then(|a| serde_json::from_str::>(a).ok()) + .map(|list| format!(" (with {})", list.join(", "))) + .unwrap_or_default(); + format!("[{}] {}{}{}", dt, e.summary, loc, attendees) + }) + .collect(); + format!( + "Found {} calendar events:\n{}", + evts.len(), + formatted.join("\n") + ) + } + Some(_) => "No calendar events found.".to_string(), + None => "No calendar events found.".to_string(), + } + } + + /// Tool: get_location_history — fetch location records near a date + async fn tool_get_location_history( + &self, + args: &serde_json::Value, + cx: &opentelemetry::Context, + ) -> String { + let date_str = match args.get("date").and_then(|v| v.as_str()) { + Some(d) => d, + None => return "Error: missing required parameter 'date'".to_string(), + }; + let days_radius = args + .get("days_radius") + .and_then(|v| v.as_i64()) + .unwrap_or(14); + + let date = match NaiveDate::parse_from_str(date_str, "%Y-%m-%d") { + Ok(d) => d, + Err(e) => return format!("Error: failed to parse date '{}': {}", date_str, e), + }; + let timestamp = date + .and_hms_opt(12, 0, 0) + .unwrap() + .and_utc() + .timestamp(); + + log::info!( + "tool_get_location_history: date={}, days_radius={}", + date, + days_radius + ); + + let start_ts = timestamp - (days_radius * 86400); + let end_ts = timestamp + (days_radius * 86400); + + let locations = { + let mut dao = self + .location_dao + .lock() + .expect("Unable to lock LocationHistoryDao"); + dao.find_locations_in_range(cx, start_ts, end_ts).ok() + }; + + match locations { + Some(locs) if !locs.is_empty() => { + let formatted: Vec = locs + .iter() + .take(20) + .map(|loc| { + let dt = DateTime::from_timestamp(loc.timestamp, 0) + .map(|dt| dt.format("%Y-%m-%d %H:%M").to_string()) + .unwrap_or_else(|| "unknown".to_string()); + let activity = loc + .activity + .as_ref() + .map(|a| format!(" ({})", a)) + .unwrap_or_default(); + let place = loc + .place_name + .as_ref() + .map(|p| format!(" at {}", p)) + .unwrap_or_default(); + format!( + "[{}] {:.4}, {:.4}{}{}", + dt, loc.latitude, loc.longitude, place, activity + ) + }) + .collect(); + format!( + "Found {} location records:\n{}", + locs.len(), + formatted.join("\n") + ) + } + Some(_) => "No location history found.".to_string(), + None => "No location history found.".to_string(), + } + } + + /// Tool: get_file_tags — fetch tags for a file path + async fn tool_get_file_tags( + &self, + args: &serde_json::Value, + cx: &opentelemetry::Context, + ) -> String { + let file_path = match args.get("file_path").and_then(|v| v.as_str()) { + Some(p) => p.to_string(), + None => return "Error: missing required parameter 'file_path'".to_string(), + }; + + log::info!("tool_get_file_tags: file_path='{}'", file_path); + + let tags = { + let mut dao = self.tag_dao.lock().expect("Unable to lock TagDao"); + dao.get_tags_for_path(cx, &file_path).ok() + }; + + match tags { + Some(t) if !t.is_empty() => { + let names: Vec = t.into_iter().map(|tag| tag.name).collect(); + names.join(", ") + } + Some(_) => "No tags found.".to_string(), + None => "No tags found.".to_string(), + } + } + + /// Tool: describe_photo — generate a visual description of the photo + async fn tool_describe_photo( + &self, + ollama: &OllamaClient, + image_base64: &Option, + ) -> String { + log::info!("tool_describe_photo: generating visual description"); + + match image_base64 { + Some(img) => match ollama.generate_photo_description(img).await { + Ok(desc) => desc, + Err(e) => format!("Error describing photo: {}", e), + }, + None => "No image available for description.".to_string(), + } + } + + // ── Agentic insight generation ────────────────────────────────────── + + /// Build the list of tool definitions for the agentic loop + fn build_tool_definitions(has_vision: bool) -> Vec { + let mut tools = vec![ + Tool::function( + "search_rag", + "Search conversation history using semantic search. Use this to find relevant past conversations about specific topics, people, or events.", + serde_json::json!({ + "type": "object", + "required": ["query", "date"], + "properties": { + "query": { + "type": "string", + "description": "The search query to find relevant conversations" + }, + "date": { + "type": "string", + "description": "The reference date in YYYY-MM-DD format" + }, + "contact": { + "type": "string", + "description": "Optional contact name to filter results" + } + } + }), + ), + Tool::function( + "get_sms_messages", + "Fetch SMS/text messages near a specific date for a contact. Returns the actual message conversation.", + serde_json::json!({ + "type": "object", + "required": ["date", "contact"], + "properties": { + "date": { + "type": "string", + "description": "The center date in YYYY-MM-DD format" + }, + "contact": { + "type": "string", + "description": "The contact name to fetch messages for" + }, + "days_radius": { + "type": "integer", + "description": "Number of days before and after the date to search (default: 4)" + } + } + }), + ), + Tool::function( + "get_calendar_events", + "Fetch calendar events near a specific date. Shows scheduled events, meetings, and activities.", + serde_json::json!({ + "type": "object", + "required": ["date"], + "properties": { + "date": { + "type": "string", + "description": "The center date in YYYY-MM-DD format" + }, + "days_radius": { + "type": "integer", + "description": "Number of days before and after the date to search (default: 7)" + } + } + }), + ), + Tool::function( + "get_location_history", + "Fetch location history records near a specific date. Shows places visited and activities.", + serde_json::json!({ + "type": "object", + "required": ["date"], + "properties": { + "date": { + "type": "string", + "description": "The center date in YYYY-MM-DD format" + }, + "days_radius": { + "type": "integer", + "description": "Number of days before and after the date to search (default: 14)" + } + } + }), + ), + Tool::function( + "get_file_tags", + "Get tags/labels that have been applied to a specific photo file.", + serde_json::json!({ + "type": "object", + "required": ["file_path"], + "properties": { + "file_path": { + "type": "string", + "description": "The file path of the photo to get tags for" + } + } + }), + ), + ]; + + if has_vision { + tools.push(Tool::function( + "describe_photo", + "Generate a visual description of the photo. Describes people, location, and activity visible in the image.", + serde_json::json!({ + "type": "object", + "properties": {} + }), + )); + } + + tools + } + + /// Generate an AI insight for a photo using an agentic tool-calling loop. + /// The model decides which tools to call to gather context before writing the final insight. + pub async fn generate_agentic_insight_for_photo( + &self, + file_path: &str, + custom_model: Option, + custom_system_prompt: Option, + num_ctx: Option, + max_iterations: usize, + ) -> Result<()> { + let tracer = global_tracer(); + let current_cx = opentelemetry::Context::current(); + let mut span = tracer.start_with_context("ai.insight.generate_agentic", ¤t_cx); + + let file_path = normalize_path(file_path); + log::info!("Generating agentic insight for photo: {}", file_path); + + span.set_attribute(KeyValue::new("file_path", file_path.clone())); + span.set_attribute(KeyValue::new("max_iterations", max_iterations as i64)); + + // 1. Create OllamaClient + let mut ollama_client = if let Some(ref model) = custom_model { + log::info!("Using custom model for agentic: {}", model); + span.set_attribute(KeyValue::new("custom_model", model.clone())); + OllamaClient::new( + self.ollama.primary_url.clone(), + self.ollama.fallback_url.clone(), + model.clone(), + Some(model.clone()), + ) + } else { + span.set_attribute(KeyValue::new("model", self.ollama.primary_model.clone())); + self.ollama.clone() + }; + + if let Some(ctx) = num_ctx { + log::info!("Using custom context size: {}", ctx); + span.set_attribute(KeyValue::new("num_ctx", ctx as i64)); + ollama_client.set_num_ctx(Some(ctx)); + } + + let insight_cx = current_cx.with_span(span); + + // 2. Check tool calling capability + let capabilities = OllamaClient::check_model_capabilities( + &ollama_client.primary_url, + &ollama_client.primary_model, + ) + .await + .map_err(|e| { + anyhow::anyhow!( + "Failed to check model capabilities for '{}': {}", + ollama_client.primary_model, + e + ) + })?; + + if !capabilities.has_tool_calling { + return Err(anyhow::anyhow!( + "tool calling not supported by model '{}'", + ollama_client.primary_model + )); + } + + let has_vision = capabilities.has_vision; + insight_cx + .span() + .set_attribute(KeyValue::new("model_has_vision", has_vision)); + insight_cx + .span() + .set_attribute(KeyValue::new("model_has_tool_calling", true)); + + // 3. Fetch EXIF + let exif = { + let mut exif_dao = self.exif_dao.lock().expect("Unable to lock ExifDao"); + exif_dao + .get_exif(&insight_cx, &file_path) + .map_err(|e| anyhow::anyhow!("Failed to get EXIF: {:?}", e))? + }; + + // 4. Extract timestamp and contact + let timestamp = if let Some(ts) = exif.as_ref().and_then(|e| e.date_taken) { + ts + } else { + log::warn!("No date_taken in EXIF for {}, trying filename", file_path); + extract_date_from_filename(&file_path) + .map(|dt| dt.timestamp()) + .or_else(|| { + let full_path = std::path::Path::new(&self.base_path).join(&file_path); + File::open(&full_path) + .and_then(|f| f.metadata()) + .and_then(|m| m.created().or(m.modified())) + .map(|t| DateTime::::from(t).timestamp()) + .inspect_err(|e| { + log::warn!( + "Failed to get file timestamp for agentic insight {}: {}", + file_path, + e + ) + }) + .ok() + }) + .unwrap_or_else(|| Utc::now().timestamp()) + }; + + let date_taken = DateTime::from_timestamp(timestamp, 0) + .map(|dt| dt.date_naive()) + .unwrap_or_else(|| Utc::now().date_naive()); + + let contact = Self::extract_contact_from_path(&file_path); + log::info!("Agentic: date_taken={}, contact={:?}", date_taken, contact); + + // 5. Fetch tags + let tag_names: Vec = { + let mut dao = self.tag_dao.lock().expect("Unable to lock TagDao"); + dao.get_tags_for_path(&insight_cx, &file_path) + .unwrap_or_else(|e| { + log::warn!("Failed to fetch tags for agentic {}: {}", file_path, e); + Vec::new() + }) + .into_iter() + .map(|t| t.name) + .collect() + }; + + // 6. Load image if vision capable + let image_base64 = if has_vision { + match self.load_image_as_base64(&file_path) { + Ok(b64) => { + log::info!("Loaded image for vision-capable agentic model"); + Some(b64) + } + Err(e) => { + log::warn!("Failed to load image for agentic vision: {}", e); + None + } + } + } else { + None + }; + + // 7. Build system message + let base_system = "You are a personal photo memory assistant. You have access to tools to gather context about when and where this photo was taken. Use them to build a rich, personal insight. Call tools as needed, then write a final summary and title."; + let system_content = if let Some(ref custom) = custom_system_prompt { + format!("{}\n\n{}", custom, base_system) + } else { + base_system.to_string() + }; + + // 8. Build user message + let gps_info = exif + .as_ref() + .and_then(|e| { + if let (Some(lat), Some(lon)) = (e.gps_latitude, e.gps_longitude) { + Some(format!("GPS: {:.4}, {:.4}", lat, lon)) + } else { + None + } + }) + .unwrap_or_else(|| "GPS: unknown".to_string()); + + let tags_info = if tag_names.is_empty() { + "Tags: none".to_string() + } else { + format!("Tags: {}", tag_names.join(", ")) + }; + + let contact_info = contact + .as_ref() + .map(|c| format!("Contact/Person: {}", c)) + .unwrap_or_else(|| "Contact/Person: unknown".to_string()); + + let user_content = format!( + "Please analyze this photo and gather context to write a personal journal-style insight.\n\n\ + Photo file path: {}\n\ + Date taken: {}\n\ + {}\n\ + {}\n\ + {}\n\n\ + Use the available tools to gather more context about this moment (messages, calendar events, location history, etc.), \ + then write a detailed personal insight with a title and summary. Write in first person as Cameron.", + file_path, + date_taken.format("%B %d, %Y"), + contact_info, + gps_info, + tags_info, + ); + + // 9. Define tools + let tools = Self::build_tool_definitions(has_vision); + + // 10. Build initial messages + let system_msg = ChatMessage::system(system_content); + let mut user_msg = ChatMessage::user(user_content); + if let Some(ref img) = image_base64 { + user_msg.images = Some(vec![img.clone()]); + } + + let mut messages = vec![system_msg, user_msg]; + + // 11. Agentic loop + let loop_span = tracer.start_with_context("ai.agentic.loop", &insight_cx); + let loop_cx = insight_cx.with_span(loop_span); + + let mut final_content = String::new(); + let mut iterations_used = 0usize; + + for iteration in 0..max_iterations { + iterations_used = iteration + 1; + log::info!("Agentic iteration {}/{}", iteration + 1, max_iterations); + + let response = ollama_client + .chat_with_tools(messages.clone(), tools.clone()) + .await?; + + messages.push(response.clone()); + + if let Some(ref tool_calls) = response.tool_calls { + if !tool_calls.is_empty() { + for tool_call in tool_calls { + log::info!( + "Agentic tool call [{}]: {} {:?}", + iteration, + tool_call.function.name, + tool_call.function.arguments + ); + let result = self + .execute_tool( + &tool_call.function.name, + &tool_call.function.arguments, + &ollama_client, + &image_base64, + &loop_cx, + ) + .await; + messages.push(ChatMessage::tool_result(result)); + } + continue; + } + } + + // No tool calls — this is the final answer + final_content = response.content; + break; + } + + // If loop exhausted without final answer, ask for one + if final_content.is_empty() { + log::info!("Agentic loop exhausted after {} iterations, requesting final answer", iterations_used); + messages.push(ChatMessage::user( + "Based on the context gathered, please write the final photo insight: a title and a detailed personal summary. Write in first person as Cameron.", + )); + let final_response = ollama_client + .chat_with_tools(messages, vec![]) + .await?; + final_content = final_response.content; + } + + loop_cx + .span() + .set_attribute(KeyValue::new("iterations_used", iterations_used as i64)); + loop_cx.span().set_status(Status::Ok); + + // 12. Generate title + let title = ollama_client + .generate_photo_title(&final_content, custom_system_prompt.as_deref()) + .await?; + + log::info!("Agentic generated title: {}", title); + log::info!( + "Agentic generated summary ({} chars): {}", + final_content.len(), + &final_content[..final_content.len().min(200)] + ); + + // 13. Store + let insight = InsertPhotoInsight { + file_path: file_path.to_string(), + title, + summary: final_content, + generated_at: Utc::now().timestamp(), + model_version: ollama_client.primary_model.clone(), + }; + + let mut dao = self.insight_dao.lock().expect("Unable to lock InsightDao"); + let result = dao + .store_insight(&insight_cx, insight) + .map_err(|e| anyhow::anyhow!("Failed to store agentic insight: {:?}", e)); + + match &result { + Ok(_) => { + log::info!("Successfully stored agentic insight for {}", file_path); + insight_cx.span().set_status(Status::Ok); + } + Err(e) => { + log::error!("Failed to store agentic insight: {:?}", e); + insight_cx.span().set_status(Status::error(e.to_string())); + } + } + + result?; + Ok(()) + } + /// Reverse geocode GPS coordinates to human-readable place names async fn reverse_geocode(&self, lat: f64, lon: f64) -> Option { let url = format!( -- 2.49.1 From 091327e5d9caa329c513171b36cdd0b4e21ec1d5 Mon Sep 17 00:00:00 2001 From: Cameron Date: Wed, 18 Mar 2026 23:01:25 -0400 Subject: [PATCH 10/13] feat: add POST /insights/generate/agentic handler and route Register the agentic insight endpoint that validates tool-calling capability, runs the agentic loop, and returns the stored PhotoInsightResponse. Returns 400 for unsupported models, 500 for other errors. Max iterations configurable via AGENTIC_MAX_ITERATIONS env var (default 10). Co-Authored-By: Claude Sonnet 4.6 --- src/ai/handlers.rs | 102 +++++++++++++++++++++++++++++++++++++++++++++ src/ai/mod.rs | 4 +- src/main.rs | 1 + 3 files changed, 105 insertions(+), 2 deletions(-) diff --git a/src/ai/handlers.rs b/src/ai/handlers.rs index 2fcfa06..5bf3178 100644 --- a/src/ai/handlers.rs +++ b/src/ai/handlers.rs @@ -211,6 +211,108 @@ pub async fn get_all_insights_handler( } } +/// POST /insights/generate/agentic - Generate insight using agentic tool-calling loop +#[post("/insights/generate/agentic")] +pub async fn generate_agentic_insight_handler( + http_request: HttpRequest, + _claims: Claims, + request: web::Json, + insight_generator: web::Data, + insight_dao: web::Data>>, +) -> impl Responder { + let parent_context = extract_context_from_request(&http_request); + let tracer = global_tracer(); + let mut span = tracer.start_with_context("http.insights.generate_agentic", &parent_context); + + let normalized_path = normalize_path(&request.file_path); + + span.set_attribute(KeyValue::new("file_path", normalized_path.clone())); + if let Some(ref model) = request.model { + span.set_attribute(KeyValue::new("model", model.clone())); + } + if let Some(ref prompt) = request.system_prompt { + span.set_attribute(KeyValue::new("has_custom_prompt", true)); + span.set_attribute(KeyValue::new("prompt_length", prompt.len() as i64)); + } + if let Some(ctx) = request.num_ctx { + span.set_attribute(KeyValue::new("num_ctx", ctx as i64)); + } + + let max_iterations: usize = std::env::var("AGENTIC_MAX_ITERATIONS") + .ok() + .and_then(|v| v.parse().ok()) + .unwrap_or(10); + + span.set_attribute(KeyValue::new("max_iterations", max_iterations as i64)); + + log::info!( + "Agentic insight generation triggered for photo: {} with model: {:?}, max_iterations: {}", + normalized_path, + request.model, + max_iterations + ); + + let result = insight_generator + .generate_agentic_insight_for_photo( + &normalized_path, + request.model.clone(), + request.system_prompt.clone(), + request.num_ctx, + max_iterations, + ) + .await; + + match result { + Ok(()) => { + span.set_status(Status::Ok); + // Fetch the stored insight to return it + let otel_context = opentelemetry::Context::new(); + let mut dao = insight_dao.lock().expect("Unable to lock InsightDao"); + match dao.get_insight(&otel_context, &normalized_path) { + Ok(Some(insight)) => { + let response = PhotoInsightResponse { + id: insight.id, + file_path: insight.file_path, + title: insight.title, + summary: insight.summary, + generated_at: insight.generated_at, + model_version: insight.model_version, + }; + HttpResponse::Ok().json(response) + } + Ok(None) => HttpResponse::Ok().json(serde_json::json!({ + "success": true, + "message": "Agentic insight generated successfully" + })), + Err(e) => { + log::warn!("Insight stored but failed to retrieve: {:?}", e); + HttpResponse::Ok().json(serde_json::json!({ + "success": true, + "message": "Agentic insight generated successfully" + })) + } + } + } + Err(e) => { + let error_msg = format!("{:?}", e); + log::error!("Failed to generate agentic insight: {}", error_msg); + span.set_status(Status::error(error_msg.clone())); + + if error_msg.contains("tool calling not supported") + || error_msg.contains("model not available") + { + HttpResponse::BadRequest().json(serde_json::json!({ + "error": format!("Failed to generate agentic insight: {}", error_msg) + })) + } else { + HttpResponse::InternalServerError().json(serde_json::json!({ + "error": format!("Failed to generate agentic insight: {}", error_msg) + })) + } + } + } +} + /// GET /insights/models - List available models from both servers with capabilities #[get("/insights/models")] pub async fn get_available_models_handler( diff --git a/src/ai/mod.rs b/src/ai/mod.rs index 57425e1..49c6651 100644 --- a/src/ai/mod.rs +++ b/src/ai/mod.rs @@ -8,8 +8,8 @@ pub mod sms_client; #[allow(unused_imports)] pub use daily_summary_job::{generate_daily_summaries, strip_summary_boilerplate}; pub use handlers::{ - delete_insight_handler, generate_insight_handler, get_all_insights_handler, - get_available_models_handler, get_insight_handler, + delete_insight_handler, generate_agentic_insight_handler, generate_insight_handler, + get_all_insights_handler, get_available_models_handler, get_insight_handler, }; pub use insight_generator::InsightGenerator; pub use ollama::{ModelCapabilities, OllamaClient}; diff --git a/src/main.rs b/src/main.rs index e56ecdf..14ca74c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1189,6 +1189,7 @@ fn main() -> std::io::Result<()> { .service(get_file_metadata) .service(memories::list_memories) .service(ai::generate_insight_handler) + .service(ai::generate_agentic_insight_handler) .service(ai::get_insight_handler) .service(ai::delete_insight_handler) .service(ai::get_all_insights_handler) -- 2.49.1 From 5c9f5c7d0bf77abf8b38e6e39e59abcf6a6a1437 Mon Sep 17 00:00:00 2001 From: Cameron Date: Wed, 18 Mar 2026 23:07:43 -0400 Subject: [PATCH 11/13] feat: add model-availability validation to agentic insight generation (T009-T011) - Verify custom model exists on at least one configured server before starting agentic loop; returns HTTP 400 with descriptive error if not found - has_tool_calling field auto-serialised in GET /insights/models via existing ModelCapabilities Serialize derive - model_version stored from OllamaClient.primary_model (already correct in T006 implementation) Co-Authored-By: Claude Sonnet 4.6 --- README.md | 5 +++++ src/ai/insight_generator.rs | 27 ++++++++++++++++++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 10d2d6e..c8f1c69 100644 --- a/README.md +++ b/README.md @@ -55,6 +55,11 @@ The following environment variables configure AI-powered photo insights and dail - Used to fetch conversation data for context in insights - `SMS_API_TOKEN` - Authentication token for SMS API (optional) +#### Agentic Insight Generation +- `AGENTIC_MAX_ITERATIONS` - Maximum tool-call iterations per agentic insight request [default: `10`] + - Controls how many times the model can invoke tools before being forced to produce a final answer + - Increase for more thorough context gathering; decrease to limit response time + #### Fallback Behavior - Primary server is tried first with 5-second connection timeout - On failure, automatically falls back to secondary server (if configured) diff --git a/src/ai/insight_generator.rs b/src/ai/insight_generator.rs index 6434314..2027fd2 100644 --- a/src/ai/insight_generator.rs +++ b/src/ai/insight_generator.rs @@ -1803,7 +1803,32 @@ Return ONLY the summary, nothing else."#, let insight_cx = current_cx.with_span(span); - // 2. Check tool calling capability + // 2a. Verify the model exists on at least one server before checking capabilities + if let Some(ref model_name) = custom_model { + let available_on_primary = OllamaClient::is_model_available( + &ollama_client.primary_url, + model_name, + ) + .await + .unwrap_or(false); + + let available_on_fallback = if let Some(ref fallback_url) = ollama_client.fallback_url { + OllamaClient::is_model_available(fallback_url, model_name) + .await + .unwrap_or(false) + } else { + false + }; + + if !available_on_primary && !available_on_fallback { + anyhow::bail!( + "model not available: '{}' not found on any configured server", + model_name + ); + } + } + + // 2b. Check tool calling capability let capabilities = OllamaClient::check_model_capabilities( &ollama_client.primary_url, &ollama_client.primary_model, -- 2.49.1 From c1b601341273c199b480f8cc80410205a84eca3d Mon Sep 17 00:00:00 2001 From: Cameron Date: Wed, 18 Mar 2026 23:09:58 -0400 Subject: [PATCH 12/13] chore: cargo fmt + clippy fix for collapsed if-let chain (T017) - cargo fmt applied across all modified source files - Collapse nested if let Some / if !is_empty into a single let-chain (clippy::collapsible_match) - All other warnings are pre-existing dead-code lint on unused trait methods Co-Authored-By: Claude Sonnet 4.6 --- src/ai/insight_generator.rs | 136 +++++++++++++++++++++--------------- src/ai/ollama.rs | 37 +++++++--- src/database/preview_dao.rs | 17 ++--- src/files.rs | 6 +- src/main.rs | 76 ++++++++++---------- src/state.rs | 14 ++-- src/video/actors.rs | 59 +++++++++------- src/video/ffmpeg.rs | 16 +++-- 8 files changed, 201 insertions(+), 160 deletions(-) diff --git a/src/ai/insight_generator.rs b/src/ai/insight_generator.rs index 2027fd2..67a7358 100644 --- a/src/ai/insight_generator.rs +++ b/src/ai/insight_generator.rs @@ -738,7 +738,11 @@ impl InsightGenerator { .map(|t| t.name) .collect() }; - log::info!("Fetched {} tags for photo: {:?}", tag_names.len(), tag_names); + log::info!( + "Fetched {} tags for photo: {:?}", + tag_names.len(), + tag_names + ); // 4. Get location name from GPS coordinates (needed for RAG query) let location = match exif { @@ -827,7 +831,10 @@ impl InsightGenerator { Some(desc) } Err(e) => { - log::warn!("Failed to generate photo description for RAG enrichment: {}", e); + log::warn!( + "Failed to generate photo description for RAG enrichment: {}", + e + ); None } } @@ -845,7 +852,11 @@ impl InsightGenerator { if !tag_names.is_empty() { parts.push(format!("tags: {}", tag_names.join(", "))); } - if parts.is_empty() { None } else { Some(parts.join(". ")) } + if parts.is_empty() { + None + } else { + Some(parts.join(". ")) + } }; let mut search_enrichment: Option = enriched_query.clone(); @@ -900,7 +911,11 @@ impl InsightGenerator { if !tag_names.is_empty() { parts.push(format!("tags: {}", tag_names.join(", "))); } - if parts.is_empty() { None } else { Some(parts.join(". ")) } + if parts.is_empty() { + None + } else { + Some(parts.join(". ")) + } }; // Step 3: Try historical RAG (>30 days ago) using extracted topics @@ -980,7 +995,14 @@ impl InsightGenerator { log::info!("No immediate messages found, trying basic RAG as fallback"); // Fallback to basic RAG even without strong query match self - .find_relevant_messages_rag(date_taken, None, contact.as_deref(), None, 20, enriched_query.as_deref()) + .find_relevant_messages_rag( + date_taken, + None, + contact.as_deref(), + None, + 20, + enriched_query.as_deref(), + ) .await { Ok(rag_messages) if !rag_messages.is_empty() => { @@ -1399,11 +1421,7 @@ Return ONLY the summary, nothing else."#, Ok(d) => d, Err(e) => return format!("Error: failed to parse date '{}': {}", date_str, e), }; - let timestamp = date - .and_hms_opt(12, 0, 0) - .unwrap() - .and_utc() - .timestamp(); + let timestamp = date.and_hms_opt(12, 0, 0).unwrap().and_utc().timestamp(); log::info!( "tool_get_sms_messages: date={}, contact='{}', days_radius={}", @@ -1442,7 +1460,11 @@ Return ONLY the summary, nothing else."#, format!("[{}] {}: {}", ts, sender, m.body) }) .collect(); - format!("Found {} messages:\n{}", messages.len(), formatted.join("\n")) + format!( + "Found {} messages:\n{}", + messages.len(), + formatted.join("\n") + ) } Ok(_) => "No messages found.".to_string(), Err(e) => format!("Error fetching SMS messages: {}", e), @@ -1468,11 +1490,7 @@ Return ONLY the summary, nothing else."#, Ok(d) => d, Err(e) => return format!("Error: failed to parse date '{}': {}", date_str, e), }; - let timestamp = date - .and_hms_opt(12, 0, 0) - .unwrap() - .and_utc() - .timestamp(); + let timestamp = date.and_hms_opt(12, 0, 0).unwrap().and_utc().timestamp(); log::info!( "tool_get_calendar_events: date={}, days_radius={}", @@ -1541,11 +1559,7 @@ Return ONLY the summary, nothing else."#, Ok(d) => d, Err(e) => return format!("Error: failed to parse date '{}': {}", date_str, e), }; - let timestamp = date - .and_hms_opt(12, 0, 0) - .unwrap() - .and_utc() - .timestamp(); + let timestamp = date.and_hms_opt(12, 0, 0).unwrap().and_utc().timestamp(); log::info!( "tool_get_location_history: date={}, days_radius={}", @@ -1805,12 +1819,10 @@ Return ONLY the summary, nothing else."#, // 2a. Verify the model exists on at least one server before checking capabilities if let Some(ref model_name) = custom_model { - let available_on_primary = OllamaClient::is_model_available( - &ollama_client.primary_url, - model_name, - ) - .await - .unwrap_or(false); + let available_on_primary = + OllamaClient::is_model_available(&ollama_client.primary_url, model_name) + .await + .unwrap_or(false); let available_on_fallback = if let Some(ref fallback_url) = ollama_client.fallback_url { OllamaClient::is_model_available(fallback_url, model_name) @@ -2002,28 +2014,28 @@ Return ONLY the summary, nothing else."#, messages.push(response.clone()); - if let Some(ref tool_calls) = response.tool_calls { - if !tool_calls.is_empty() { - for tool_call in tool_calls { - log::info!( - "Agentic tool call [{}]: {} {:?}", - iteration, - tool_call.function.name, - tool_call.function.arguments - ); - let result = self - .execute_tool( - &tool_call.function.name, - &tool_call.function.arguments, - &ollama_client, - &image_base64, - &loop_cx, - ) - .await; - messages.push(ChatMessage::tool_result(result)); - } - continue; + if let Some(ref tool_calls) = response.tool_calls + && !tool_calls.is_empty() + { + for tool_call in tool_calls { + log::info!( + "Agentic tool call [{}]: {} {:?}", + iteration, + tool_call.function.name, + tool_call.function.arguments + ); + let result = self + .execute_tool( + &tool_call.function.name, + &tool_call.function.arguments, + &ollama_client, + &image_base64, + &loop_cx, + ) + .await; + messages.push(ChatMessage::tool_result(result)); } + continue; } // No tool calls — this is the final answer @@ -2033,13 +2045,14 @@ Return ONLY the summary, nothing else."#, // If loop exhausted without final answer, ask for one if final_content.is_empty() { - log::info!("Agentic loop exhausted after {} iterations, requesting final answer", iterations_used); + log::info!( + "Agentic loop exhausted after {} iterations, requesting final answer", + iterations_used + ); messages.push(ChatMessage::user( "Based on the context gathered, please write the final photo insight: a title and a detailed personal summary. Write in first person as Cameron.", )); - let final_response = ollama_client - .chat_with_tools(messages, vec![]) - .await?; + let final_response = ollama_client.chat_with_tools(messages, vec![]).await?; final_content = final_response.content; } @@ -2179,18 +2192,29 @@ mod tests { Some("vacation, hiking, mountains".to_string()), ); assert!(result.contains("## Tags"), "Should include Tags section"); - assert!(result.contains("vacation, hiking, mountains"), "Should include tag names"); + assert!( + result.contains("vacation, hiking, mountains"), + "Should include tag names" + ); } #[test] fn combine_contexts_omits_tags_section_when_no_tags() { let result = InsightGenerator::combine_contexts( Some("some messages".to_string()), - None, None, None, + None, + None, + None, None, // no tags ); - assert!(!result.contains("## Tags"), "Should not include Tags section when None"); - assert!(result.contains("## Messages"), "Should still include Messages"); + assert!( + !result.contains("## Tags"), + "Should not include Tags section when None" + ); + assert!( + result.contains("## Messages"), + "Should still include Messages" + ); } #[test] diff --git a/src/ai/ollama.rs b/src/ai/ollama.rs index 481b716..38a0c6c 100644 --- a/src/ai/ollama.rs +++ b/src/ai/ollama.rs @@ -528,10 +528,8 @@ Analyze the image and use specific details from both the visual content and the // Try fallback server if available if let Some(fallback_url) = &self.fallback_url { - let fallback_model = self - .fallback_model - .as_ref() - .unwrap_or(&self.primary_model); + let fallback_model = + self.fallback_model.as_ref().unwrap_or(&self.primary_model); log::info!( "Attempting chat_with_tools with fallback server: {} (model: {})", @@ -578,7 +576,9 @@ Analyze the image and use specific details from both the visual content and the let model = if base_url == self.primary_url { &self.primary_model } else { - self.fallback_model.as_deref().unwrap_or(&self.primary_model) + self.fallback_model + .as_deref() + .unwrap_or(&self.primary_model) }; let options = self.num_ctx.map(|ctx| OllamaOptions { num_ctx: Some(ctx) }); @@ -602,7 +602,11 @@ Analyze the image and use specific details from both the visual content and the if !response.status().is_success() { let status = response.status(); let body = response.text().await.unwrap_or_default(); - anyhow::bail!("Ollama chat request failed with status {}: {}", status, body); + anyhow::bail!( + "Ollama chat request failed with status {}: {}", + status, + body + ); } let chat_response: OllamaChatResponse = response @@ -804,13 +808,28 @@ pub struct ChatMessage { impl ChatMessage { pub fn system(content: impl Into) -> Self { - Self { role: "system".to_string(), content: content.into(), tool_calls: None, images: None } + Self { + role: "system".to_string(), + content: content.into(), + tool_calls: None, + images: None, + } } pub fn user(content: impl Into) -> Self { - Self { role: "user".to_string(), content: content.into(), tool_calls: None, images: None } + Self { + role: "user".to_string(), + content: content.into(), + tool_calls: None, + images: None, + } } pub fn tool_result(content: impl Into) -> Self { - Self { role: "tool".to_string(), content: content.into(), tool_calls: None, images: None } + Self { + role: "tool".to_string(), + content: content.into(), + tool_calls: None, + images: None, + } } } diff --git a/src/database/preview_dao.rs b/src/database/preview_dao.rs index 7ff7618..fe90f4d 100644 --- a/src/database/preview_dao.rs +++ b/src/database/preview_dao.rs @@ -4,7 +4,7 @@ use std::ops::DerefMut; use std::sync::{Arc, Mutex}; use crate::database::models::{InsertVideoPreviewClip, VideoPreviewClip}; -use crate::database::{connect, DbError, DbErrorKind}; +use crate::database::{DbError, DbErrorKind, connect}; use crate::otel::trace_db_call; pub trait PreviewDao: Sync + Send { @@ -232,10 +232,7 @@ mod tests { .unwrap(); // Status should remain "pending" from the first insert - let clip = dao - .get_preview(&ctx, "photos/video.mp4") - .unwrap() - .unwrap(); + let clip = dao.get_preview(&ctx, "photos/video.mp4").unwrap().unwrap(); assert_eq!(clip.status, "pending"); } @@ -256,10 +253,7 @@ mod tests { ) .unwrap(); - let clip = dao - .get_preview(&ctx, "photos/video.mp4") - .unwrap() - .unwrap(); + let clip = dao.get_preview(&ctx, "photos/video.mp4").unwrap().unwrap(); assert_eq!(clip.status, "complete"); assert_eq!(clip.duration_seconds, Some(9.5)); assert_eq!(clip.file_size_bytes, Some(1024000)); @@ -283,10 +277,7 @@ mod tests { ) .unwrap(); - let clip = dao - .get_preview(&ctx, "photos/video.mp4") - .unwrap() - .unwrap(); + let clip = dao.get_preview(&ctx, "photos/video.mp4").unwrap().unwrap(); assert_eq!(clip.status, "failed"); assert_eq!( clip.error_message.as_deref(), diff --git a/src/files.rs b/src/files.rs index a7355fb..6f3adeb 100644 --- a/src/files.rs +++ b/src/files.rs @@ -1490,7 +1490,8 @@ mod tests { let request: Query = Query::from_query("path=&tag_ids=1,3&recursive=true").unwrap(); - let mut tag_dao = SqliteTagDao::new(std::sync::Arc::new(Mutex::new(in_memory_db_connection()))); + let mut tag_dao = + SqliteTagDao::new(std::sync::Arc::new(Mutex::new(in_memory_db_connection()))); let tag1 = tag_dao .create_tag(&opentelemetry::Context::current(), "tag1") @@ -1536,7 +1537,8 @@ mod tests { exp: 12345, }; - let mut tag_dao = SqliteTagDao::new(std::sync::Arc::new(Mutex::new(in_memory_db_connection()))); + let mut tag_dao = + SqliteTagDao::new(std::sync::Arc::new(Mutex::new(in_memory_db_connection()))); let tag1 = tag_dao .create_tag(&opentelemetry::Context::current(), "tag1") diff --git a/src/main.rs b/src/main.rs index 14ca74c..b56d43c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -600,8 +600,7 @@ async fn get_video_preview( Some(path) => path, None => { span.set_status(Status::error("Invalid path")); - return HttpResponse::BadRequest() - .json(serde_json::json!({"error": "Invalid path"})); + return HttpResponse::BadRequest().json(serde_json::json!({"error": "Invalid path"})); } }; @@ -634,8 +633,7 @@ async fn get_video_preview( } Err(_) => { // File missing on disk but DB says complete - reset and regenerate - let mut dao = - preview_dao.lock().expect("Unable to lock PreviewDao"); + let mut dao = preview_dao.lock().expect("Unable to lock PreviewDao"); let _ = dao.update_status( &context, &relative_path, @@ -665,12 +663,10 @@ async fn get_video_preview( })) } "failed" => { - let error_msg = - clip.error_message.unwrap_or_else(|| "Unknown error".to_string()); - span.set_status(Status::error(format!( - "Generation failed: {}", - error_msg - ))); + let error_msg = clip + .error_message + .unwrap_or_else(|| "Unknown error".to_string()); + span.set_status(Status::error(format!("Generation failed: {}", error_msg))); HttpResponse::InternalServerError().json(serde_json::json!({ "error": format!("Generation failed: {}", error_msg) })) @@ -708,8 +704,7 @@ async fn get_video_preview( } Err(_) => { span.set_status(Status::error("Database error")); - HttpResponse::InternalServerError() - .json(serde_json::json!({"error": "Database error"})) + HttpResponse::InternalServerError().json(serde_json::json!({"error": "Database error"})) } } } @@ -768,10 +763,7 @@ async fn get_preview_status( path: path.clone(), status: clip.status.clone(), preview_url: if clip.status == "complete" { - Some(format!( - "/video/preview?path={}", - urlencoding::encode(path) - )) + Some(format!("/video/preview?path={}", urlencoding::encode(path))) } else { None }, @@ -810,8 +802,7 @@ async fn get_preview_status( } Err(_) => { span.set_status(Status::error("Database error")); - HttpResponse::InternalServerError() - .json(serde_json::json!({"error": "Database error"})) + HttpResponse::InternalServerError().json(serde_json::json!({"error": "Database error"})) } } } @@ -1213,21 +1204,18 @@ fn main() -> std::io::Result<()> { .app_data::>>>(Data::new(Mutex::new(Box::new( preview_dao, )))) - .app_data( - web::JsonConfig::default() - .error_handler(|err, req| { - let detail = err.to_string(); - log::warn!( - "JSON parse error on {} {}: {}", - req.method(), - req.uri(), - detail - ); - let response = HttpResponse::BadRequest() - .json(serde_json::json!({"error": detail})); - actix_web::error::InternalError::from_response(err, response).into() - }), - ) + .app_data(web::JsonConfig::default().error_handler(|err, req| { + let detail = err.to_string(); + log::warn!( + "JSON parse error on {} {}: {}", + req.method(), + req.uri(), + detail + ); + let response = + HttpResponse::BadRequest().json(serde_json::json!({"error": detail})); + actix_web::error::InternalError::from_response(err, response).into() + })) .app_data::>(Data::new(app_data.insight_generator.clone())) .wrap(prometheus.clone()) }) @@ -1765,9 +1753,7 @@ mod tests { // Verify the DAO now has a pending record let mut dao_lock = preview_dao.lock().unwrap(); let ctx = opentelemetry::Context::new(); - let clip = dao_lock - .get_preview(&ctx, "photos/new_video.mp4") - .unwrap(); + let clip = dao_lock.get_preview(&ctx, "photos/new_video.mp4").unwrap(); assert!(clip.is_some()); assert_eq!(clip.unwrap().status, "pending"); } @@ -1778,8 +1764,15 @@ mod tests { let ctx = opentelemetry::Context::new(); dao.insert_preview(&ctx, "photos/done.mp4", "pending") .unwrap(); - dao.update_status(&ctx, "photos/done.mp4", "complete", Some(9.5), Some(500000), None) - .unwrap(); + dao.update_status( + &ctx, + "photos/done.mp4", + "complete", + Some(9.5), + Some(500000), + None, + ) + .unwrap(); let preview_dao = make_preview_dao(dao); let app_state = Data::new(AppState::test_state()); @@ -1806,7 +1799,12 @@ mod tests { let previews = body["previews"].as_array().unwrap(); assert_eq!(previews.len(), 1); assert_eq!(previews[0]["status"], "complete"); - assert!(previews[0]["preview_url"].as_str().unwrap().contains("photos%2Fdone.mp4")); + assert!( + previews[0]["preview_url"] + .as_str() + .unwrap() + .contains("photos%2Fdone.mp4") + ); } #[actix_rt::test] diff --git a/src/state.rs b/src/state.rs index 578ea78..4000704 100644 --- a/src/state.rs +++ b/src/state.rs @@ -46,11 +46,8 @@ impl AppState { let video_playlist_manager = VideoPlaylistManager::new(video_path.clone(), playlist_generator.start()); - let preview_clip_generator = PreviewClipGenerator::new( - preview_clips_path.clone(), - base_path.clone(), - preview_dao, - ); + let preview_clip_generator = + PreviewClipGenerator::new(preview_clips_path.clone(), base_path.clone(), preview_dao); Self { stream_manager, @@ -141,9 +138,10 @@ impl Default for AppState { ); // Ensure preview clips directory exists - let preview_clips_path = env::var("PREVIEW_CLIPS_DIRECTORY") - .unwrap_or_else(|_| "preview_clips".to_string()); - std::fs::create_dir_all(&preview_clips_path).expect("Failed to create PREVIEW_CLIPS_DIRECTORY"); + let preview_clips_path = + env::var("PREVIEW_CLIPS_DIRECTORY").unwrap_or_else(|_| "preview_clips".to_string()); + std::fs::create_dir_all(&preview_clips_path) + .expect("Failed to create PREVIEW_CLIPS_DIRECTORY"); Self::new( Arc::new(StreamActor {}.start()), diff --git a/src/video/actors.rs b/src/video/actors.rs index 936e39f..e90bbe1 100644 --- a/src/video/actors.rs +++ b/src/video/actors.rs @@ -159,19 +159,21 @@ async fn get_video_rotation(video_path: &str) -> i32 { .await; if let Ok(output) = output - && output.status.success() { - let rotation_str = String::from_utf8_lossy(&output.stdout); - let rotation_str = rotation_str.trim(); - if !rotation_str.is_empty() - && let Ok(rotation) = rotation_str.parse::() - && rotation != 0 { - debug!( - "Detected rotation {}° from stream tag for {}", - rotation, video_path - ); - return rotation; - } + && output.status.success() + { + let rotation_str = String::from_utf8_lossy(&output.stdout); + let rotation_str = rotation_str.trim(); + if !rotation_str.is_empty() + && let Ok(rotation) = rotation_str.parse::() + && rotation != 0 + { + debug!( + "Detected rotation {}° from stream tag for {}", + rotation, video_path + ); + return rotation; } + } // Check display matrix side data (modern videos, e.g. iPhone) let output = tokio::process::Command::new("ffprobe") @@ -188,21 +190,23 @@ async fn get_video_rotation(video_path: &str) -> i32 { .await; if let Ok(output) = output - && output.status.success() { - let rotation_str = String::from_utf8_lossy(&output.stdout); - let rotation_str = rotation_str.trim(); - if !rotation_str.is_empty() - && let Ok(rotation) = rotation_str.parse::() { - let rotation = rotation.abs() as i32; - if rotation != 0 { - debug!( - "Detected rotation {}° from display matrix for {}", - rotation, video_path - ); - return rotation; - } - } + && output.status.success() + { + let rotation_str = String::from_utf8_lossy(&output.stdout); + let rotation_str = rotation_str.trim(); + if !rotation_str.is_empty() + && let Ok(rotation) = rotation_str.parse::() + { + let rotation = rotation.abs() as i32; + if rotation != 0 { + debug!( + "Detected rotation {}° from display matrix for {}", + rotation, video_path + ); + return rotation; + } } + } 0 } @@ -550,7 +554,8 @@ impl Handler for PreviewClipGenerator { { let otel_ctx = opentelemetry::Context::current(); let mut dao = preview_dao.lock().expect("Unable to lock PreviewDao"); - let _ = dao.update_status(&otel_ctx, &relative_path, "processing", None, None, None); + let _ = + dao.update_status(&otel_ctx, &relative_path, "processing", None, None, None); } // Compute output path: join preview_clips_dir with relative path, change ext to .mp4 diff --git a/src/video/ffmpeg.rs b/src/video/ffmpeg.rs index ddb35d5..b40b175 100644 --- a/src/video/ffmpeg.rs +++ b/src/video/ffmpeg.rs @@ -183,7 +183,11 @@ impl Ffmpeg { Ok(output_file.to_string()) } - pub async fn create_gif_from_frames(&self, frame_base_dir: &str, output_file: &str) -> Result { + pub async fn create_gif_from_frames( + &self, + frame_base_dir: &str, + output_file: &str, + ) -> Result { let output = Command::new("ffmpeg") .arg("-y") .args(["-framerate", "4"]) @@ -278,10 +282,7 @@ pub async fn generate_preview_clip(input_file: &str, output_file: &str) -> Resul "select='lt(mod(t,{:.4}),1)',setpts=N/FRAME_RATE/TB,fps=30,scale=-2:480,format=yuv420p", interval ); - let af = format!( - "aselect='lt(mod(t,{:.4}),1)',asetpts=N/SR/TB", - interval - ); + let af = format!("aselect='lt(mod(t,{:.4}),1)',asetpts=N/SR/TB", interval); cmd.args(["-vf", &vf]); cmd.args(["-af", &af]); @@ -326,7 +327,10 @@ pub async fn generate_preview_clip(input_file: &str, output_file: &str) -> Resul info!( "Generated preview clip '{}' ({:.1}s, {} bytes) in {:?}", - output_file, clip_duration, file_size, start.elapsed() + output_file, + clip_duration, + file_size, + start.elapsed() ); Ok((clip_duration, file_size)) -- 2.49.1 From 54a49a8562547bf4c5e3bb269eb74784117a9eb0 Mon Sep 17 00:00:00 2001 From: Cameron Date: Wed, 18 Mar 2026 23:58:01 -0400 Subject: [PATCH 13/13] =?UTF-8?q?fix:=20agentic=20loop=20robustness=20?= =?UTF-8?q?=E2=80=94=20tool=20arg=20sanitisation,=20geocoding,=20better=20?= =?UTF-8?q?errors?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Sanitise tool call arguments before re-sending in conversation history: non-object values (bool, string, null) that some models produce are normalised to {} to prevent Ollama 500s - Map 'error parsing tool call' Ollama 500 to HTTP 400 with a descriptive message listing compatible models (llama3.1, llama3.2, qwen2.5, mistral-nemo) - Add reverse_geocode tool backed by existing Nominatim helper; description hints model can chain it after get_location_history results - Make get_sms_messages contact parameter optional (was required, forcing the model to guess); executor now passes None to fall back to all-contacts search - Log tool result outcomes at warn level for errors/empty results, info for successes; log SMS API errors with full detail; log full request body on Ollama 500 - Strengthen system prompt to require 3-4 tool calls before final answer - Try fallback server when checking model capabilities (primary-only check caused 500 for models only on fallback) Co-Authored-By: Claude Sonnet 4.6 --- src/ai/handlers.rs | 4 + src/ai/insight_generator.rs | 147 +++++++++++++++++++++++++++--------- src/ai/ollama.rs | 9 +++ 3 files changed, 125 insertions(+), 35 deletions(-) diff --git a/src/ai/handlers.rs b/src/ai/handlers.rs index 5bf3178..f91268b 100644 --- a/src/ai/handlers.rs +++ b/src/ai/handlers.rs @@ -304,6 +304,10 @@ pub async fn generate_agentic_insight_handler( HttpResponse::BadRequest().json(serde_json::json!({ "error": format!("Failed to generate agentic insight: {}", error_msg) })) + } else if error_msg.contains("error parsing tool call") { + HttpResponse::BadRequest().json(serde_json::json!({ + "error": "Model is not compatible with Ollama's tool calling protocol. Try a model known to support native tool calling (e.g. llama3.1, llama3.2, qwen2.5, mistral-nemo)." + })) } else { HttpResponse::InternalServerError().json(serde_json::json!({ "error": format!("Failed to generate agentic insight: {}", error_msg) diff --git a/src/ai/insight_generator.rs b/src/ai/insight_generator.rs index 67a7358..cac03a4 100644 --- a/src/ai/insight_generator.rs +++ b/src/ai/insight_generator.rs @@ -1347,15 +1347,26 @@ Return ONLY the summary, nothing else."#, image_base64: &Option, cx: &opentelemetry::Context, ) -> String { - match tool_name { + let result = match tool_name { "search_rag" => self.tool_search_rag(arguments, cx).await, "get_sms_messages" => self.tool_get_sms_messages(arguments, cx).await, "get_calendar_events" => self.tool_get_calendar_events(arguments, cx).await, "get_location_history" => self.tool_get_location_history(arguments, cx).await, "get_file_tags" => self.tool_get_file_tags(arguments, cx).await, "describe_photo" => self.tool_describe_photo(ollama, image_base64).await, + "reverse_geocode" => self.tool_reverse_geocode(arguments).await, unknown => format!("Unknown tool: {}", unknown), + }; + if result.starts_with("Error") || result.starts_with("No ") { + log::warn!("Tool '{}' result: {}", tool_name, result); + } else { + log::info!( + "Tool '{}' result: {} chars", + tool_name, + result.len() + ); } + result } /// Tool: search_rag — semantic search over daily summaries @@ -1408,10 +1419,10 @@ Return ONLY the summary, nothing else."#, Some(d) => d, None => return "Error: missing required parameter 'date'".to_string(), }; - let contact = match args.get("contact").and_then(|v| v.as_str()) { - Some(c) => c.to_string(), - None => return "Error: missing required parameter 'contact'".to_string(), - }; + let contact = args + .get("contact") + .and_then(|v| v.as_str()) + .map(|s| s.to_string()); let days_radius = args .get("days_radius") .and_then(|v| v.as_i64()) @@ -1424,28 +1435,15 @@ Return ONLY the summary, nothing else."#, let timestamp = date.and_hms_opt(12, 0, 0).unwrap().and_utc().timestamp(); log::info!( - "tool_get_sms_messages: date={}, contact='{}', days_radius={}", + "tool_get_sms_messages: date={}, contact={:?}, days_radius={}", date, contact, days_radius ); - // Use the SMS client's existing fetch mechanism - // Build start/end from days_radius - let start_ts = timestamp - (days_radius * 86400); - let end_ts = timestamp + (days_radius * 86400); - - let center_dt = DateTime::from_timestamp(timestamp, 0); - let start_dt = DateTime::from_timestamp(start_ts, 0); - let end_dt = DateTime::from_timestamp(end_ts, 0); - - if center_dt.is_none() || start_dt.is_none() || end_dt.is_none() { - return "Error: invalid timestamp range".to_string(); - } - match self .sms_client - .fetch_messages_for_contact(Some(&contact), timestamp) + .fetch_messages_for_contact(contact.as_deref(), timestamp) .await { Ok(messages) if !messages.is_empty() => { @@ -1467,7 +1465,10 @@ Return ONLY the summary, nothing else."#, ) } Ok(_) => "No messages found.".to_string(), - Err(e) => format!("Error fetching SMS messages: {}", e), + Err(e) => { + log::warn!("tool_get_sms_messages failed: {}", e); + format!("Error fetching SMS messages: {}", e) + } } } @@ -1659,6 +1660,25 @@ Return ONLY the summary, nothing else."#, } } + /// Tool: reverse_geocode — convert GPS coordinates to a human-readable place name + async fn tool_reverse_geocode(&self, args: &serde_json::Value) -> String { + let lat = match args.get("latitude").and_then(|v| v.as_f64()) { + Some(v) => v, + None => return "Error: missing required parameter 'latitude'".to_string(), + }; + let lon = match args.get("longitude").and_then(|v| v.as_f64()) { + Some(v) => v, + None => return "Error: missing required parameter 'longitude'".to_string(), + }; + + log::info!("tool_reverse_geocode: lat={}, lon={}", lat, lon); + + match self.reverse_geocode(lat, lon).await { + Some(place) => place, + None => "Could not resolve coordinates to a place name.".to_string(), + } + } + // ── Agentic insight generation ────────────────────────────────────── /// Build the list of tool definitions for the agentic loop @@ -1688,10 +1708,10 @@ Return ONLY the summary, nothing else."#, ), Tool::function( "get_sms_messages", - "Fetch SMS/text messages near a specific date for a contact. Returns the actual message conversation.", + "Fetch SMS/text messages near a specific date. Returns the actual message conversation. Omit contact to search across all conversations.", serde_json::json!({ "type": "object", - "required": ["date", "contact"], + "required": ["date"], "properties": { "date": { "type": "string", @@ -1699,7 +1719,7 @@ Return ONLY the summary, nothing else."#, }, "contact": { "type": "string", - "description": "The contact name to fetch messages for" + "description": "Optional contact name to filter messages. If omitted, searches all conversations." }, "days_radius": { "type": "integer", @@ -1760,6 +1780,25 @@ Return ONLY the summary, nothing else."#, ), ]; + tools.push(Tool::function( + "reverse_geocode", + "Convert GPS latitude/longitude coordinates to a human-readable place name (city, state). Use this when GPS coordinates are available in the photo metadata, or to resolve coordinates returned by get_location_history.", + serde_json::json!({ + "type": "object", + "required": ["latitude", "longitude"], + "properties": { + "latitude": { + "type": "number", + "description": "GPS latitude in decimal degrees" + }, + "longitude": { + "type": "number", + "description": "GPS longitude in decimal degrees" + } + } + }), + )); + if has_vision { tools.push(Tool::function( "describe_photo", @@ -1840,19 +1879,34 @@ Return ONLY the summary, nothing else."#, } } - // 2b. Check tool calling capability - let capabilities = OllamaClient::check_model_capabilities( + // 2b. Check tool calling capability — try primary, fall back to fallback URL + let model_name_for_caps = &ollama_client.primary_model; + let capabilities = match OllamaClient::check_model_capabilities( &ollama_client.primary_url, - &ollama_client.primary_model, + model_name_for_caps, ) .await - .map_err(|e| { - anyhow::anyhow!( - "Failed to check model capabilities for '{}': {}", - ollama_client.primary_model, - e - ) - })?; + { + Ok(caps) => caps, + Err(_) => { + // Model may only be on the fallback server + let fallback_url = ollama_client.fallback_url.as_deref().ok_or_else(|| { + anyhow::anyhow!( + "Failed to check model capabilities for '{}': model not found on primary server and no fallback configured", + model_name_for_caps + ) + })?; + OllamaClient::check_model_capabilities(fallback_url, model_name_for_caps) + .await + .map_err(|e| { + anyhow::anyhow!( + "Failed to check model capabilities for '{}': {}", + model_name_for_caps, + e + ) + })? + } + }; if !capabilities.has_tool_calling { return Err(anyhow::anyhow!( @@ -1939,7 +1993,13 @@ Return ONLY the summary, nothing else."#, }; // 7. Build system message - let base_system = "You are a personal photo memory assistant. You have access to tools to gather context about when and where this photo was taken. Use them to build a rich, personal insight. Call tools as needed, then write a final summary and title."; + let base_system = "You are a personal photo memory assistant helping to reconstruct a memory from a photo.\n\n\ + IMPORTANT INSTRUCTIONS:\n\ + 1. You MUST call multiple tools to gather context BEFORE writing any final insight. Do not produce a final answer after only one or two tool calls.\n\ + 2. Always call ALL of the following tools that are relevant: search_rag (search conversation summaries), get_sms_messages (fetch nearby messages), get_calendar_events (check what was happening that day), get_location_history (find where this was taken), get_file_tags (retrieve tags).\n\ + 3. Only produce your final insight AFTER you have gathered context from at least 3-4 tools.\n\ + 4. If a tool returns no results, that is useful information — continue calling the remaining tools anyway.\n\ + 5. Your final insight must be written in first person as Cameron, in a journal/memoir style."; let system_content = if let Some(ref custom) = custom_system_prompt { format!("{}\n\n{}", custom, base_system) } else { @@ -2012,6 +2072,23 @@ Return ONLY the summary, nothing else."#, .chat_with_tools(messages.clone(), tools.clone()) .await?; + // Sanitize tool call arguments before pushing back into history. + // Some models occasionally return non-object arguments (bool, string, null) + // which Ollama rejects when they are re-sent in a subsequent request. + let mut response = response; + if let Some(ref mut tool_calls) = response.tool_calls { + for tc in tool_calls.iter_mut() { + if !tc.function.arguments.is_object() { + log::warn!( + "Tool '{}' returned non-object arguments ({:?}), normalising to {{}}", + tc.function.name, + tc.function.arguments + ); + tc.function.arguments = serde_json::Value::Object(Default::default()); + } + } + } + messages.push(response.clone()); if let Some(ref tool_calls) = response.tool_calls diff --git a/src/ai/ollama.rs b/src/ai/ollama.rs index 38a0c6c..857427f 100644 --- a/src/ai/ollama.rs +++ b/src/ai/ollama.rs @@ -591,6 +591,10 @@ Analyze the image and use specific details from both the visual content and the options, }; + let request_json = serde_json::to_string(&request_body) + .unwrap_or_else(|e| format!("", e)); + log::debug!("chat_with_tools request body: {}", request_json); + let response = self .client .post(&url) @@ -602,6 +606,11 @@ Analyze the image and use specific details from both the visual content and the if !response.status().is_success() { let status = response.status(); let body = response.text().await.unwrap_or_default(); + log::error!( + "chat_with_tools request body that caused {}: {}", + status, + request_json + ); anyhow::bail!( "Ollama chat request failed with status {}: {}", status, -- 2.49.1