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

fix(persona-chat): review fixes before merge

Browse Source 
- Thread tools are bound server-side to the chat's own thread. The model
  used to pass contact_id, so practice mode could read any conversation it
  named; a model-supplied contact_id is now only checked, never trusted, and
  the prompts no longer hand it one.
- Practice chats need no persona. They're stored under PRACTICE_PERSONA_ID
  and run with an inert stand-in, so deleting or editing personas can't hide
  or break them. persona_id is optional on create for as_contact.
- The per-thread list filters in SQL (list_persona_chats_for_contact, served
  by idx_persona_chat_anchor_contact) instead of decoding every transcript.
- fetch_mms_image checks the content type and a 20 MB cap before reading
  the body, so a video attachment isn't downloaded in full.
- get_photos_near_time unions tags over every path sharing the content, per
  the multi-library model.
- look_at_photo only opens indexed photos, so an excluded folder can't be
  read by guessing a path.
- One date parser: get_photos_near_time uses date_info::parse_center.
- Practice prompt source rewrapped.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Cameron CordesandClaude Opus 5.5 committed 2026-10-20 19:38:08 -04:00
1 parent 185a1b561a
commit e029c81274
8 files changed
+383 -116
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

src/ai/insight_generator.rs
+94 -66
View File
@@ -1891,6 +1891,7 @@ Return ONLY the summary, nothing else."#,
file_path: &str,
user_id: i32,
persona_id: &str,
thread_contact: Option<i64>,
) -> String {
let model = backend.model();
let backend_label = backend.kind.as_str();
@@ -1940,9 +1941,12 @@ Return ONLY the summary, nothing else."#,
"date_info" => {
crate::ai::date_info::date_info(arguments, Utc::now(), self.display_zone())
}
"get_thread_messages" => self.tool_get_thread_messages(arguments).await,
"get_thread_messages" => {
self.tool_get_thread_messages(arguments, thread_contact)
.await
}
"look_at_message_image" => {
self.tool_look_at_message_image(arguments, backend.local())
self.tool_look_at_message_image(arguments, thread_contact, backend.local())
.await
}
"get_photos_near_time" => self.tool_get_photos_near_time(arguments).await,
@@ -2411,9 +2415,14 @@ Return ONLY the summary, nothing else."#,
/// Tool: get_thread_messages — page the anchored SMS thread from a
/// message id. Lines are rendered exactly like the anchor snapshot so
/// the model can keep paging from any id it sees.
async fn tool_get_thread_messages(&self, args: &serde_json::Value) -> String {
let Some(contact_id) = args.get("contact_id").and_then(|v| v.as_i64()) else {
return "Error: missing required parameter 'contact_id'".to_string();
async fn tool_get_thread_messages(
&self,
args: &serde_json::Value,
thread_contact: Option<i64>,
) -> String {
let contact_id = match Self::bound_thread(args, thread_contact) {
Ok(c) => c,
Err(e) => return e,
};
let Some(message_id) = args.get("message_id").and_then(|v| v.as_i64()) else {
return "Error: missing required parameter 'message_id'".to_string();
@@ -2461,10 +2470,12 @@ Return ONLY the summary, nothing else."#,
async fn tool_look_at_message_image(
&self,
args: &serde_json::Value,
thread_contact: Option<i64>,
local: &dyn LlmClient,
) -> String {
let Some(contact_id) = args.get("contact_id").and_then(|v| v.as_i64()) else {
return "Error: missing required parameter 'contact_id'".to_string();
let contact_id = match Self::bound_thread(args, thread_contact) {
Ok(c) => c,
Err(e) => return e,
};
let Some(message_id) = args.get("message_id").and_then(|v| v.as_i64()) else {
return "Error: missing required parameter 'message_id'".to_string();
@@ -2476,11 +2487,15 @@ Return ONLY the summary, nothing else."#,
let bytes = match self
.sms_client
.fetch_mms_media(contact_id, message_id, media_index)
.fetch_mms_image(contact_id, message_id, media_index)
.await
{
Ok(b) => b,
Err(e) => return format!("Error fetching attachment: {}", e),
Err(e) => {
return format!(
"Error: couldn't look at that attachment ({e}); only photos can be viewed."
);
}
};
let image_base64 = match Self::encode_image_bytes_for_vision(&bytes) {
Ok(b64) => b64,
@@ -2501,7 +2516,12 @@ Return ONLY the summary, nothing else."#,
async fn tool_get_photos_near_time(&self, args: &serde_json::Value) -> String {
let zone = self.display_zone();
let server_zone = crate::camera_tz::server_zone();
let Some(center) = Self::parse_moment(args, zone) else {
let center_arg = args
.get("timestamp")
.filter(|v| !v.is_null())
.or_else(|| args.get("datetime"));
let Some(center) = center_arg.and_then(|v| crate::ai::date_info::parse_center(v, zone))
else {
return "Error: pass 'timestamp' (unix seconds) or 'datetime' (YYYY-MM-DD HH:MM)"
.to_string();
};
@@ -2553,11 +2573,22 @@ Return ONLY the summary, nothing else."#,
let when = crate::date_view::row_display_wall(&row, zone, server_zone)
.map(|w| w.format("%Y-%m-%d %H:%M").to_string())
.unwrap_or_else(|| "unknown time".to_string());
// Tags follow the bytes: union over every path holding the same
// content, so a copy in another library shows its tags too.
let sibling_paths = {
let mut dao = self.exif_dao.lock().expect("Unable to lock ExifDao");
dao.get_rel_paths_sharing_content(row.library_id, &row.file_path)
.unwrap_or_else(|_| vec![row.file_path.clone()])
};
let tags: Vec<String> = {
let mut dao = self.tag_dao.lock().expect("Unable to lock TagDao");
dao.get_tags_for_path(&row.file_path)
let mut names: Vec<String> = dao
.get_tags_for_paths(&sibling_paths)
.map(|t| t.into_iter().map(|t| t.name).collect())
.unwrap_or_default()
.unwrap_or_default();
names.sort();
names.dedup();
names
};
let people: Vec<String> = match &row.content_hash {
Some(hash) => {
@@ -2602,6 +2633,14 @@ Return ONLY the summary, nothing else."#,
if crate::file_types::is_video_file(std::path::Path::new(&file_path)) {
return "Error: that's a video — only photos can be looked at.".to_string();
}
// Only photos the library has indexed: excluded folders (EXCLUDED_DIRS,
// e.g. `private`) are never indexed, so a guessed path into one is
// refused rather than read off disk.
if self.fetch_exif(&file_path).is_none() {
return "Error: that isn't a photo in the library — use a file_path from \
get_photos_near_time."
.to_string();
}
let image_base64 = match self.load_image_as_base64(&file_path) {
Ok(b) => b,
Err(e) => return format!("Error loading photo: {}", e),
@@ -2612,6 +2651,26 @@ Return ONLY the summary, nothing else."#,
}
}
/// The thread a thread tool may read: the one the chat was started from,
/// fixed server-side. A `contact_id` the model passes is only checked
/// against it — never trusted — so a chat (practice mode especially) can
/// never be talked into reading another conversation.
fn bound_thread(args: &serde_json::Value, thread_contact: Option<i64>) -> Result<i64, String> {
let Some(bound) = thread_contact else {
return Err(
"Error: thread tools are only available in a chat started from a \
text thread."
.to_string(),
);
};
match args.get("contact_id").and_then(|v| v.as_i64()) {
Some(asked) if asked != bound => {
Err("Error: only this chat's own text thread can be read here.".to_string())
}
_ => Ok(bound),
}
}
/// A model-supplied path is joined onto each library root, so refuse
/// anything that could climb out of it.
fn photo_path_is_safe(path: &str) -> bool {
@@ -2622,28 +2681,6 @@ Return ONLY the summary, nothing else."#,
.all(|c| matches!(c, Component::Normal(_) | Component::CurDir))
}
/// A tool's centre moment: `timestamp` (unix seconds) wins, else
/// `datetime` read as a wall clock in the owner's display zone. A bare
/// date centres on midday.
fn parse_moment(args: &serde_json::Value, zone: chrono_tz::Tz) -> Option<i64> {
if let Some(ts) = args.get("timestamp").and_then(|v| v.as_i64()) {
return Some(ts);
}
let raw = args.get("datetime").and_then(|v| v.as_str())?.trim();
let naive = chrono::NaiveDateTime::parse_from_str(raw, "%Y-%m-%d %H:%M")
.or_else(|_| chrono::NaiveDateTime::parse_from_str(raw, "%Y-%m-%dT%H:%M"))
.or_else(|_| chrono::NaiveDateTime::parse_from_str(raw, "%Y-%m-%dT%H:%M:%S"))
.ok()
.or_else(|| {
NaiveDate::parse_from_str(raw, "%Y-%m-%d")
.ok()
.and_then(|d| d.and_hms_opt(12, 0, 0))
})?;
zone.from_local_datetime(&naive)
.earliest()
.map(|dt| dt.timestamp())
}
/// Decode arbitrary image bytes (an MMS attachment), shrink to the
/// vision models' 1024px working size and re-encode as base64 JPEG —
/// the same shape `load_image_as_base64` produces for library files.
@@ -4062,12 +4099,11 @@ Return ONLY the summary, nothing else."#,
thread (the #numbers in the transcript) — `before` for older messages, `after` for what \
came next, `around` for both sides. Lines carry their own #id, so call again from the \
oldest/newest id to keep paging. \
Example: `{contact_id: 42, message_id: 812, direction: \"before\", limit: 30}`.",
Example: `{message_id: 812, direction: \"before\", limit: 30}`.",
serde_json::json!({
"type": "object",
"required": ["contact_id", "message_id"],
"required": ["message_id"],
"properties": {
"contact_id": { "type": "integer", "description": "The thread's contact_id (given in the system prompt)." },
"message_id": { "type": "integer", "description": "Message id to page from." },
"direction": { "type": "string", "enum": ["before", "after", "around"], "description": "Which side of message_id to read (default before)." },
"limit": { "type": "integer", "description": "Max messages (default 30, max 80)." }
@@ -4080,12 +4116,11 @@ Return ONLY the summary, nothing else."#,
"Look at a photo that was sent in the SMS thread, using the local vision model. Returns a \
visual description. Attachments appear in the transcript as `image #N` on a message line — \
pass that message's id and N as media_index. Videos and audio can't be viewed. \
Example: `{contact_id: 42, message_id: 815, media_index: 0}`.",
Example: `{message_id: 815, media_index: 0}`.",
serde_json::json!({
"type": "object",
"required": ["contact_id", "message_id"],
"required": ["message_id"],
"properties": {
"contact_id": { "type": "integer", "description": "The thread's contact_id." },
"message_id": { "type": "integer", "description": "Id of the message carrying the photo." },
"media_index": { "type": "integer", "description": "Which attachment on that message (default 0)." }
}
@@ -4843,6 +4878,7 @@ Return ONLY the summary, nothing else."#,
&file_path,
user_id,
&persona_id,
None,
)
.instrument(loop_span.clone())
.await;
@@ -4895,6 +4931,7 @@ Return ONLY the summary, nothing else."#,
&file_path,
user_id,
&persona_id,
None,
)
.instrument(loop_span.clone())
.await;
@@ -5083,6 +5120,7 @@ Return ONLY the summary, nothing else."#,
"",
0,
"",
None,
)
.await;
messages.push(ChatMessage::tool_result(result));
@@ -5124,6 +5162,7 @@ Return ONLY the summary, nothing else."#,
"",
0,
"",
None,
)
.await;
messages.push(ChatMessage::tool_result(result));
@@ -5294,6 +5333,21 @@ mod tests {
assert!(names.contains(&"look_at_photo"));
}
#[test]
fn thread_tools_only_read_the_chats_own_thread() {
use serde_json::json;
// The bound thread is used; the model needn't (and can't) choose.
assert_eq!(InsightGenerator::bound_thread(&json!({}), Some(42)), Ok(42));
assert_eq!(
InsightGenerator::bound_thread(&json!({"contact_id": 42}), Some(42)),
Ok(42)
);
// Another thread is refused, even if the model names it.
assert!(InsightGenerator::bound_thread(&json!({"contact_id": 7}), Some(42)).is_err());
// No bound thread (not a thread chat): nothing to read.
assert!(InsightGenerator::bound_thread(&json!({"contact_id": 42}), None).is_err());
}
#[test]
fn photo_path_is_safe_rejects_escapes() {
assert!(InsightGenerator::photo_path_is_safe("2019/06/IMG_1.jpg"));
@@ -5303,32 +5357,6 @@ mod tests {
assert!(!InsightGenerator::photo_path_is_safe(""));
}
#[test]
fn parse_moment_accepts_unix_and_local_wall_clock() {
let tz = chrono_tz::Tz::America__New_York;
assert_eq!(
InsightGenerator::parse_moment(&serde_json::json!({"timestamp": 1_700_000_000}), tz),
Some(1_700_000_000)
);
// 2023-11-14 17:13 in New York (EST) = 22:13 UTC.
assert_eq!(
InsightGenerator::parse_moment(
&serde_json::json!({"datetime": "2023-11-14 17:13"}),
tz
),
Some(1_699_999_980)
);
// A bare date means midday, the least-wrong centre for a whole day.
assert_eq!(
InsightGenerator::parse_moment(&serde_json::json!({"datetime": "2023-11-14"}), tz),
Some(1_699_981_200)
);
assert_eq!(
InsightGenerator::parse_moment(&serde_json::json!({}), tz),
None
);
}
fn place(name: &str, description: &str) -> ApolloPlace {
ApolloPlace {
id: 1,