Apps/ImageApi
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

fix: agentic loop robustness — tool arg sanitisation, geocoding, better errors

Browse Source 
- 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 <noreply@anthropic.com>
This commit is contained in:
Cameron
2026-03-18 23:58:01 -04:00
parent c1b6013412
commit 54a49a8562
3 changed files with 125 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
src/ai/handlers.rs
View File

@@ -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)

147
src/ai/insight_generator.rs
View File

@@ -1347,15 +1347,26 @@ Return ONLY the summary, nothing else."#,
image_base64: &Option<String>,
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

9
src/ai/ollama.rs
View File

@@ -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!("<serialization error: {}>", 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,