From a699c940b8cc9fa3809beefc4f3263360a0d74eb Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Sun, 27 Sep 2026 19:48:45 -0400 Subject: [PATCH 1/5] Prefs: owner defaults (model, backend, persona_id, reel span/length) Closes #127. Grows user_ai_prefs with five nullable owner-default columns and makes them the server-side fallback layer: request param -> stored pref -> hard default, at insight generation (agentic + standard), all three insight-chat turn variants, and reel creation. - PUT /prefs accepts the fields with the existing three-state partial update semantics (TriState generalized to TriState for the numeric reel_max_segments); validation (canonical BackendKind::parse, MemoriesSpan::from_str) runs before any write. - GET /prefs returns the new fields. - capture_prefs reads the existing row and carries the AI owner-defaults through unchanged (the upsert overwrites all columns), mirroring only the effective reel span/segments. - MemoriesSpan gains Display + FromStr so stored spans round-trip losslessly. 1463/0 tests (+27), clippy clean. --- CLAUDE.md | 5 + .../down.sql | 5 + .../up.sql | 8 + src/ai/handlers.rs | 172 +++++++ src/database/models.rs | 18 + src/database/schema.rs | 5 + src/database/user_ai_prefs_dao.rs | 39 ++ src/memories.rs | 25 + src/prefs.rs | 482 +++++++++++++++--- src/reels/mod.rs | 189 ++++++- 10 files changed, 863 insertions(+), 85 deletions(-) create mode 100644 migrations/2026-09-27-000000_add_owner_defaults/down.sql create mode 100644 migrations/2026-09-27-000000_add_owner_defaults/up.sql diff --git a/CLAUDE.md b/CLAUDE.md index d1234c2..fac6136 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -533,6 +533,11 @@ POST /insights/chat (single-turn reply, non-streaming) POST /insights/chat/stream (SSE: text / tool_call / tool_result / truncated / done) GET /insights/chat/history?path=... (rendered transcript with tool invocations) POST /insights/chat/rewind (truncate transcript at a rendered index) + +// User AI Prefs (owner defaults + passive mirror; src/prefs.rs) +GET /prefs (current row; auth) +PUT /prefs (partial update: omitted=unchanged, null=clear, value=set; auth) +GET /timezones (IANA zone list from chrono-tz; unauthenticated) ``` **Request Types:** diff --git a/migrations/2026-09-27-000000_add_owner_defaults/down.sql b/migrations/2026-09-27-000000_add_owner_defaults/down.sql new file mode 100644 index 0000000..b85a5cf --- /dev/null +++ b/migrations/2026-09-27-000000_add_owner_defaults/down.sql @@ -0,0 +1,5 @@ +ALTER TABLE user_ai_prefs DROP COLUMN default_model; +ALTER TABLE user_ai_prefs DROP COLUMN default_backend; +ALTER TABLE user_ai_prefs DROP COLUMN default_persona_id; +ALTER TABLE user_ai_prefs DROP COLUMN reel_span; +ALTER TABLE user_ai_prefs DROP COLUMN reel_max_segments; diff --git a/migrations/2026-09-27-000000_add_owner_defaults/up.sql b/migrations/2026-09-27-000000_add_owner_defaults/up.sql new file mode 100644 index 0000000..d1d9479 --- /dev/null +++ b/migrations/2026-09-27-000000_add_owner_defaults/up.sql @@ -0,0 +1,8 @@ +-- Owner defaults: recurring per-user picks that were previously re-sent on +-- every request. All nullable — NULL = keep the current per-request / +-- hard-default behavior. +ALTER TABLE user_ai_prefs ADD COLUMN default_model TEXT; +ALTER TABLE user_ai_prefs ADD COLUMN default_backend TEXT; +ALTER TABLE user_ai_prefs ADD COLUMN default_persona_id TEXT; +ALTER TABLE user_ai_prefs ADD COLUMN reel_span TEXT; +ALTER TABLE user_ai_prefs ADD COLUMN reel_max_segments INTEGER; diff --git a/src/ai/handlers.rs b/src/ai/handlers.rs index 1a98d35..b00c7b2 100644 --- a/src/ai/handlers.rs +++ b/src/ai/handlers.rs @@ -384,6 +384,36 @@ pub struct ServerModels { pub default_model: String, } +/// Apply the stored owner defaults to any request field the client omitted. +/// Resolution order: request param → stored pref → hard default (applied +/// downstream). The request always wins; the pref only fills gaps. One DAO +/// read per request; a failed read silently falls back to hard defaults. +fn apply_owner_defaults( + app_state: &AppState, + model: &mut Option, + backend: &mut Option, + persona_id: &mut Option, +) { + if model.is_some() && backend.is_some() && persona_id.is_some() { + return; // nothing to fill — skip the DB read + } + let Some(mut dao) = app_state.user_ai_prefs_dao.lock().ok() else { + return; + }; + let Ok(Some(prefs)) = dao.get_prefs(&opentelemetry::Context::new()) else { + return; + }; + if model.is_none() { + *model = prefs.default_model; + } + if backend.is_none() { + *backend = prefs.default_backend; + } + if persona_id.is_none() { + *persona_id = prefs.default_persona_id; + } +} + /// POST /insights/generate - Generate insight for a specific photo (async) #[post("/insights/generate")] pub async fn generate_insight_handler( @@ -396,6 +426,16 @@ pub async fn generate_insight_handler( let tracer = global_tracer(); let mut span = tracer.start_with_context("http.insights.generate", &parent_context); + let mut request = request.0; + // Stored owner defaults fill what the client omitted (request → pref → + // hard default; the hard default still applies downstream). + apply_owner_defaults( + &app_state, + &mut request.model, + &mut request.backend, + &mut request.persona_id, + ); + let normalized_path = normalize_path(&request.file_path); let library = app_state.primary_library(); let gen_type = InsightGenerationType::Standard; @@ -714,6 +754,16 @@ pub async fn generate_agentic_insight_handler( let tracer = global_tracer(); let mut span = tracer.start_with_context("http.insights.generate_agentic", &parent_context); + let mut request = request.0; + // Stored owner defaults fill what the client omitted (request → pref → + // hard default; the hard default still applies downstream). + apply_owner_defaults( + &app_state, + &mut request.model, + &mut request.backend, + &mut request.persona_id, + ); + let normalized_path = normalize_path(&request.file_path); let library = app_state.primary_library(); let gen_type = InsightGenerationType::Agentic; @@ -1231,6 +1281,16 @@ pub async fn chat_turn_handler( let mut span = tracer.start_with_context("http.insights.chat", &parent_context); span.set_attribute(KeyValue::new("file_path", request.file_path.clone())); + let mut request = request.0; + // Stored owner defaults fill what the client omitted (request → pref → + // hard default; the hard default still applies downstream). + apply_owner_defaults( + &app_state, + &mut request.model, + &mut request.backend, + &mut request.persona_id, + ); + let library = match libraries::resolve_library_param_state(&app_state, request.library.as_deref()) { Ok(Some(lib)) => lib, @@ -1611,6 +1671,16 @@ pub async fn chat_stream_handler( request: web::Json, app_state: web::Data, ) -> HttpResponse { + let mut request = request.0; + // Stored owner defaults fill what the client omitted (request → pref → + // hard default; the hard default still applies downstream). + apply_owner_defaults( + &app_state, + &mut request.model, + &mut request.backend, + &mut request.persona_id, + ); + let library = match libraries::resolve_library_param_state(&app_state, request.library.as_deref()) { Ok(Some(lib)) => lib, @@ -1758,6 +1828,16 @@ pub async fn turn_async_handler( let mut span = tracer.start_with_context("http.insights.chat_turn_async", &parent_context); span.set_attribute(KeyValue::new("file_path", request.file_path.clone())); + let mut request = request.0; + // Stored owner defaults fill what the client omitted (request → pref → + // hard default; the hard default still applies downstream). + apply_owner_defaults( + &app_state, + &mut request.model, + &mut request.backend, + &mut request.persona_id, + ); + let library = match libraries::resolve_library_param_state(&app_state, request.library.as_deref()) { Ok(Some(lib)) => lib, @@ -2277,3 +2357,95 @@ mod turn_replay_tests { assert_eq!(info.total_events_pushed, 2); } } + +#[cfg(test)] +mod owner_defaults_tests { + use super::apply_owner_defaults; + use crate::database::models::UpsertUserAiPrefs; + use crate::database::{SqliteUserAiPrefsDao, UserAiPrefsDao}; + use crate::state::AppState; + use std::sync::{Arc, Mutex}; + + fn prefs_state(prefs: Option) -> AppState { + let conn = Arc::new(Mutex::new(crate::database::test::in_memory_db_connection())); + let mut state = AppState::test_state(); + state.user_ai_prefs_dao = Arc::new(Mutex::new(Box::new( + SqliteUserAiPrefsDao::from_connection(conn.clone()), + ))); + if let Some(p) = prefs { + let mut dao = SqliteUserAiPrefsDao::from_connection(conn); + dao.upsert_prefs(&opentelemetry::Context::new(), &p) + .expect("seed prefs"); + } + state + } + + fn seeded_defaults() -> UpsertUserAiPrefs { + UpsertUserAiPrefs { + voice: None, + tz_offset_minutes: None, + library: None, + updated_at: 0, + display_tz: None, + default_model: Some("llama3.2:3b".into()), + default_backend: Some("hybrid".into()), + default_persona_id: Some("cabin".into()), + reel_span: None, + reel_max_segments: None, + } + } + + #[actix_rt::test] + async fn request_wins_over_stored_pref() { + let state = prefs_state(Some(seeded_defaults())); + let mut model = Some("explicit".to_string()); + let mut backend = Some("local".to_string()); + let mut persona = Some("explicit-p".to_string()); + apply_owner_defaults(&state, &mut model, &mut backend, &mut persona); + assert_eq!(model.as_deref(), Some("explicit")); + assert_eq!(backend.as_deref(), Some("local")); + assert_eq!(persona.as_deref(), Some("explicit-p")); + } + + #[actix_rt::test] + async fn pref_fills_omitted_fields() { + let state = prefs_state(Some(seeded_defaults())); + let mut model = None; + let mut backend = None; + let mut persona = None; + apply_owner_defaults(&state, &mut model, &mut backend, &mut persona); + assert_eq!(model.as_deref(), Some("llama3.2:3b")); + assert_eq!(backend.as_deref(), Some("hybrid")); + assert_eq!(persona.as_deref(), Some("cabin")); + } + + #[actix_rt::test] + async fn no_pref_row_leaves_fields_none() { + let state = prefs_state(None); + let mut model = None; + let mut backend = None; + let mut persona = None; + apply_owner_defaults(&state, &mut model, &mut backend, &mut persona); + // Hard defaults apply downstream; the pref layer fills nothing. + assert!(model.is_none()); + assert!(backend.is_none()); + assert!(persona.is_none()); + } + + #[actix_rt::test] + async fn pref_fills_only_the_fields_it_has() { + let state = prefs_state(Some(UpsertUserAiPrefs { + default_backend: None, + default_persona_id: Some("journal".into()), + ..seeded_defaults() + })); + let mut model = None; + let mut backend = Some("local".to_string()); + let mut persona = None; + apply_owner_defaults(&state, &mut model, &mut backend, &mut persona); + assert_eq!(model.as_deref(), Some("llama3.2:3b")); + // Explicit request backend is untouched; pref had none anyway. + assert_eq!(backend.as_deref(), Some("local")); + assert_eq!(persona.as_deref(), Some("journal")); + } +} diff --git a/src/database/models.rs b/src/database/models.rs index 9c5502e..22919ce 100644 --- a/src/database/models.rs +++ b/src/database/models.rs @@ -579,6 +579,19 @@ pub struct UserAiPrefs { /// default (`camera_tz::DEFAULT_DISPLAY_TZ`). The legacy /// `tz_offset_minutes` column stays frozen for rollback. pub display_tz: Option, + /// Last/explicit insight + chat model choice (local id or OpenRouter id). + /// NULL = current per-request / env default behavior. + pub default_model: Option, + /// `"local"` | `"hybrid"` — insight/chat backend. NULL = per-request / + /// hard default ("local"). + pub default_backend: Option, + /// Active persona for persona chat. NULL = hard default ("default"). + pub default_persona_id: Option, + /// Reel span: `"day"` | `"week"` | `"month"`. NULL = per-request / + /// hard default (day). + pub reel_span: Option, + /// Reel segment cap. NULL = per-request / hard default. + pub reel_max_segments: Option, } #[derive(Insertable, Debug, Clone, serde::Deserialize, serde::Serialize)] @@ -589,4 +602,9 @@ pub struct UpsertUserAiPrefs { pub library: Option, pub updated_at: i64, pub display_tz: Option, + pub default_model: Option, + pub default_backend: Option, + pub default_persona_id: Option, + pub reel_span: Option, + pub reel_max_segments: Option, } diff --git a/src/database/schema.rs b/src/database/schema.rs index 30336b8..051fb20 100644 --- a/src/database/schema.rs +++ b/src/database/schema.rs @@ -293,6 +293,11 @@ diesel::table! { library -> Nullable, updated_at -> BigInt, display_tz -> Nullable, + default_model -> Nullable, + default_backend -> Nullable, + default_persona_id -> Nullable, + reel_span -> Nullable, + reel_max_segments -> Nullable, } } diff --git a/src/database/user_ai_prefs_dao.rs b/src/database/user_ai_prefs_dao.rs index 772e49c..a3427a4 100644 --- a/src/database/user_ai_prefs_dao.rs +++ b/src/database/user_ai_prefs_dao.rs @@ -100,6 +100,11 @@ impl UserAiPrefsDao for SqliteUserAiPrefsDao { dsl::tz_offset_minutes.eq(&prefs.tz_offset_minutes), dsl::library.eq(&prefs.library), dsl::display_tz.eq(&prefs.display_tz), + dsl::default_model.eq(&prefs.default_model), + dsl::default_backend.eq(&prefs.default_backend), + dsl::default_persona_id.eq(&prefs.default_persona_id), + dsl::reel_span.eq(&prefs.reel_span), + dsl::reel_max_segments.eq(&prefs.reel_max_segments), dsl::updated_at.eq(&prefs.updated_at), )) .execute(connection.deref_mut()) @@ -147,6 +152,11 @@ mod tests { library: Some("1".to_string()), updated_at: now, display_tz: Some("America/New_York".to_string()), + default_model: Some("llama3.2:3b".to_string()), + default_backend: Some("hybrid".to_string()), + default_persona_id: Some("cabin".to_string()), + reel_span: Some("week".to_string()), + reel_max_segments: Some(24), }; dao.upsert_prefs(&ctx(), &prefs).unwrap(); @@ -156,6 +166,11 @@ mod tests { assert_eq!(row.tz_offset_minutes, Some(-480)); assert_eq!(row.library, Some("1".to_string())); assert_eq!(row.display_tz, Some("America/New_York".to_string())); + assert_eq!(row.default_model, Some("llama3.2:3b".to_string())); + assert_eq!(row.default_backend, Some("hybrid".to_string())); + assert_eq!(row.default_persona_id, Some("cabin".to_string())); + assert_eq!(row.reel_span, Some("week".to_string())); + assert_eq!(row.reel_max_segments, Some(24)); assert_eq!(row.updated_at, now); } @@ -171,6 +186,11 @@ mod tests { library: Some("1".to_string()), updated_at: now1, display_tz: Some("America/New_York".to_string()), + default_model: Some("llama3.2:3b".to_string()), + default_backend: Some("local".to_string()), + default_persona_id: None, + reel_span: Some("week".to_string()), + reel_max_segments: Some(24), }; dao.upsert_prefs(&ctx(), &prefs1).unwrap(); @@ -180,6 +200,11 @@ mod tests { library: None, updated_at: now2, display_tz: None, + default_model: None, + default_backend: None, + default_persona_id: None, + reel_span: None, + reel_max_segments: None, }; dao.upsert_prefs(&ctx(), &prefs2).unwrap(); @@ -188,6 +213,11 @@ mod tests { assert_eq!(row.tz_offset_minutes, Some(-300)); assert!(row.library.is_none()); assert!(row.display_tz.is_none()); + // Owner-default columns are overwritten to NULL like the rest. + assert!(row.default_model.is_none()); + assert!(row.default_backend.is_none()); + assert!(row.reel_span.is_none()); + assert!(row.reel_max_segments.is_none()); assert_eq!(row.updated_at, now2); } @@ -202,6 +232,11 @@ mod tests { library: None, updated_at: now, display_tz: Some("Europe/Berlin".to_string()), + default_model: None, + default_backend: None, + default_persona_id: Some("default".to_string()), + reel_span: Some("month".to_string()), + reel_max_segments: None, }; dao.upsert_prefs(&ctx(), &prefs).unwrap(); @@ -210,5 +245,9 @@ mod tests { assert!(row.voice.is_none()); assert!(row.library.is_none()); assert_eq!(row.display_tz, Some("Europe/Berlin".to_string())); + assert!(row.default_model.is_none()); + assert_eq!(row.default_persona_id, Some("default".to_string())); + assert_eq!(row.reel_span, Some("month".to_string())); + assert!(row.reel_max_segments.is_none()); } } diff --git a/src/memories.rs b/src/memories.rs index 1411ce7..8f1af68 100644 --- a/src/memories.rs +++ b/src/memories.rs @@ -108,6 +108,31 @@ pub enum MemoriesSpan { Month, } +/// Wire/storage string for the span (`"day"` | `"week"` | `"month"`) — the +/// same lowercase names as the serde rename, so a stored pref round-trips +/// through `MemoriesRequest.span` unchanged. +impl std::fmt::Display for MemoriesSpan { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(match self { + MemoriesSpan::Day => "day", + MemoriesSpan::Week => "week", + MemoriesSpan::Month => "month", + }) + } +} + +impl std::str::FromStr for MemoriesSpan { + type Err = String; + fn from_str(s: &str) -> Result { + match s.trim().to_ascii_lowercase().as_str() { + "day" => Ok(MemoriesSpan::Day), + "week" => Ok(MemoriesSpan::Week), + "month" => Ok(MemoriesSpan::Month), + other => Err(format!("invalid span {other:?} (expected day|week|month)")), + } + } +} + #[derive(Deserialize)] pub struct MemoriesRequest { pub span: Option, diff --git a/src/prefs.rs b/src/prefs.rs index 32b8737..02c57ba 100644 --- a/src/prefs.rs +++ b/src/prefs.rs @@ -8,15 +8,19 @@ //! `GET /timezones` — the IANA zone list from chrono-tz, for picker UIs //! (unauthenticated; static data). //! -//! Scope: existing columns only (voice, library, display_tz) — no migration. -//! The legacy `tz_offset_minutes` column is preserved verbatim on every -//! write. `POST /reels`'s `capture_prefs` stays as the incidental writer; -//! both writers hit the same single row, so they're idempotent. Owner -//! defaults (model/backend/persona/reels) are a separate follow-on ticket -//! that adds the columns. +//! Surface: all persisted prefs — the passive mirror (voice, library, +//! display_tz) and the owner defaults (default_model, default_backend, +//! default_persona_id, reel_span, reel_max_segments). Consumers resolve +//! `request param → stored pref → hard default`; this endpoint is the only +//! place the owner-default fields are written. `POST /reels`'s +//! `capture_prefs` stays as the incidental writer for the mirror fields +//! (plus the effective reel span/segments); both writers hit the same +//! single row, so they're idempotent. The legacy `tz_offset_minutes` +//! column is preserved verbatim on every write. use actix_web::{HttpResponse, web}; use chrono_tz::Tz; +use serde::de::IntoDeserializer; use serde::{Deserialize, Serialize}; use serde_json::json; use std::str::FromStr; @@ -24,6 +28,7 @@ use std::time::{SystemTime, UNIX_EPOCH}; use crate::data::Claims; use crate::database::models::{UpsertUserAiPrefs, UserAiPrefs}; +use crate::memories::MemoriesSpan; use crate::state::AppState; // --------------------------------------------------------------------------- @@ -35,6 +40,18 @@ pub struct PrefsResponse { pub voice: Option, pub library: Option, pub display_tz: Option, + /// Last/explicit insight + chat model choice. NULL = per-request / + /// env default behavior. + pub default_model: Option, + /// `"local"` | `"hybrid"`. NULL = per-request / hard default. + pub default_backend: Option, + /// Active persona for persona chat. NULL = hard default ("default"). + pub default_persona_id: Option, + /// Reel span: `"day"` | `"week"` | `"month"`. NULL = per-request / + /// hard default (day). + pub reel_span: Option, + /// Reel segment cap. NULL = per-request / hard default. + pub reel_max_segments: Option, pub updated_at: Option, } @@ -43,41 +60,59 @@ pub struct PrefsResponse { /// explicit null and a missing key apart on `Option>` (both come /// out as outer `None`), so this carries a hand-rolled visitor. #[derive(Clone, Debug, Default)] -pub enum TriState { +pub enum TriState { #[default] Absent, Null, - Val(String), + Val(T), } -impl<'de> Deserialize<'de> for TriState { - fn deserialize(deserializer: D) -> Result +impl<'de, T: Deserialize<'de>> Deserialize<'de> for TriState { + fn deserialize(deserializer: D) -> Result, D::Error> where D: serde::Deserializer<'de>, { - struct Visitor; - impl<'de> serde::de::Visitor<'de> for Visitor { - type Value = TriState; + struct Visitor(std::marker::PhantomData); + impl<'de, T: Deserialize<'de>> serde::de::Visitor<'de> for Visitor { + type Value = TriState; fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - f.write_str("a string or null") + f.write_str("a value or null") } // serde_json routes an explicit JSON null to visit_unit/visit_none; // a missing key never calls deserialize — `#[serde(default)]` - // supplies Absent instead. - fn visit_unit(self) -> Result { + // supplies Absent instead. Concrete values are re-deserialized into + // T through single-value deserializers (serde_json's + // deserialize_any dispatches directly to visit_str/visit_i64/… + // — visit_some only fires on the deserialize_option path). + fn visit_unit(self) -> Result, E> { Ok(TriState::Null) } - fn visit_none(self) -> Result { + fn visit_none(self) -> Result, E> { Ok(TriState::Null) } - fn visit_str(self, v: &str) -> Result { - Ok(TriState::Val(v.to_string())) + fn visit_some(self, d: D2) -> Result, D2::Error> + where + D2: serde::Deserializer<'de>, + { + Ok(TriState::Val(T::deserialize(d)?)) } - fn visit_string(self, v: String) -> Result { - Ok(TriState::Val(v)) + // `into_deserializer()` gives a single-value deserializer whose + // error type is the visitor's E, so T::deserialize lands on + // Result with no conversion. + fn visit_str(self, v: &str) -> Result, E> { + T::deserialize(v.into_deserializer()).map(TriState::Val) + } + fn visit_string(self, v: String) -> Result, E> { + T::deserialize(v.as_str().into_deserializer()).map(TriState::Val) + } + fn visit_i64(self, v: i64) -> Result, E> { + T::deserialize(v.into_deserializer()).map(TriState::Val) + } + fn visit_u64(self, v: u64) -> Result, E> { + T::deserialize(v.into_deserializer()).map(TriState::Val) } } - deserializer.deserialize_any(Visitor) + deserializer.deserialize_any(Visitor(std::marker::PhantomData)) } } @@ -86,11 +121,21 @@ impl<'de> Deserialize<'de> for TriState { #[derive(Deserialize, Debug, Default)] pub struct UpdatePrefsRequest { #[serde(default)] - pub voice: TriState, + pub voice: TriState, #[serde(default)] - pub library: TriState, + pub library: TriState, #[serde(default)] - pub display_tz: TriState, + pub display_tz: TriState, + #[serde(default)] + pub default_model: TriState, + #[serde(default)] + pub default_backend: TriState, + #[serde(default)] + pub default_persona_id: TriState, + #[serde(default)] + pub reel_span: TriState, + #[serde(default)] + pub reel_max_segments: TriState, } #[derive(Serialize, Debug)] @@ -109,6 +154,18 @@ fn is_valid_display_tz(name: &str) -> bool { Tz::from_str(name.trim()).is_ok() } +/// A stored `default_backend` must be consumable downstream, i.e. accepted +/// by the canonical `BackendKind::parse` ("local"/"hybrid", case-insensitive). +/// Delegating to that parser keeps the write-time validator and the read-time +/// consumer from drifting apart; anything else stored here would 500 on every +/// insight/chat request instead of failing once at write time. The blank +/// string the parser tolerates (empty request field = app default) is +/// rejected here — a stored default is an explicit choice, and NULL already +/// means "no default". +fn is_valid_backend(name: &str) -> bool { + !name.trim().is_empty() && crate::ai::backend::BackendKind::parse(name).is_ok() +} + /// The full set of zone names `PUT /prefs` will accept — `chrono_tz::TZ_VARIANTS` /// is exactly the domain of `Tz::from_str`, so the picker list and the /// validation check can never drift apart. @@ -119,14 +176,13 @@ pub fn all_timezones() -> Vec { .collect() } -/// Merge a partial update onto the existing row. Returns -/// `(voice, library, display_tz, changed)`. An omitted field keeps the -/// stored value (or `None` when there is no row); a present field wins. -fn tri_value( - field: &TriState, +/// Merge a partial update onto the existing row. An omitted field keeps +/// the stored value (or `None` when there is no row); a present field wins. +fn tri_value( + field: &TriState, existing: Option<&UserAiPrefs>, - keep: impl FnOnce(&UserAiPrefs) -> Option, -) -> Option { + keep: impl FnOnce(&UserAiPrefs) -> Option, +) -> Option { match field { TriState::Absent => existing.and_then(keep), TriState::Null => None, @@ -134,17 +190,43 @@ fn tri_value( } } -fn merge_patch( - existing: Option<&UserAiPrefs>, - patch: &UpdatePrefsRequest, -) -> (Option, Option, Option, bool) { - let voice = tri_value(&patch.voice, existing, |e| e.voice.clone()); - let library = tri_value(&patch.library, existing, |e| e.library.clone()); - let display_tz = tri_value(&patch.display_tz, existing, |e| e.display_tz.clone()); +/// The eight partial-update fields after a merge (everything except the +/// frozen `tz_offset_minutes`, which the handler copies through verbatim). +struct MergedPrefs { + voice: Option, + library: Option, + display_tz: Option, + default_model: Option, + default_backend: Option, + default_persona_id: Option, + reel_span: Option, + reel_max_segments: Option, +} + +fn merge_patch(existing: Option<&UserAiPrefs>, patch: &UpdatePrefsRequest) -> (MergedPrefs, bool) { + let merged = MergedPrefs { + voice: tri_value(&patch.voice, existing, |e| e.voice.clone()), + library: tri_value(&patch.library, existing, |e| e.library.clone()), + display_tz: tri_value(&patch.display_tz, existing, |e| e.display_tz.clone()), + default_model: tri_value(&patch.default_model, existing, |e| e.default_model.clone()), + default_backend: tri_value(&patch.default_backend, existing, |e| { + e.default_backend.clone() + }), + default_persona_id: tri_value(&patch.default_persona_id, existing, |e| { + e.default_persona_id.clone() + }), + reel_span: tri_value(&patch.reel_span, existing, |e| e.reel_span.clone()), + reel_max_segments: tri_value(&patch.reel_max_segments, existing, |e| e.reel_max_segments), + }; let changed = !matches!(patch.voice, TriState::Absent) || !matches!(patch.library, TriState::Absent) - || !matches!(patch.display_tz, TriState::Absent); - (voice, library, display_tz, changed) + || !matches!(patch.display_tz, TriState::Absent) + || !matches!(patch.default_model, TriState::Absent) + || !matches!(patch.default_backend, TriState::Absent) + || !matches!(patch.default_persona_id, TriState::Absent) + || !matches!(patch.reel_span, TriState::Absent) + || !matches!(patch.reel_max_segments, TriState::Absent); + (merged, changed) } fn row_to_response(row: Option<&UserAiPrefs>) -> PrefsResponse { @@ -153,12 +235,22 @@ fn row_to_response(row: Option<&UserAiPrefs>) -> PrefsResponse { voice: row.voice.clone(), library: row.library.clone(), display_tz: row.display_tz.clone(), + default_model: row.default_model.clone(), + default_backend: row.default_backend.clone(), + default_persona_id: row.default_persona_id.clone(), + reel_span: row.reel_span.clone(), + reel_max_segments: row.reel_max_segments, updated_at: Some(row.updated_at), }, None => PrefsResponse { voice: None, library: None, display_tz: None, + default_model: None, + default_backend: None, + default_persona_id: None, + reel_span: None, + reel_max_segments: None, updated_at: None, }, } @@ -192,7 +284,8 @@ pub async fn put_prefs( ) -> HttpResponse { let patch = body.0; - // Validate before any write: an invalid zone name → 400, nothing written. + // Validate before any write: an invalid value → 400, nothing written + // (not even the other fields in the same body). if let TriState::Val(name) = &patch.display_tz && !is_valid_display_tz(name) { @@ -200,6 +293,20 @@ pub async fn put_prefs( "error": format!("invalid display_tz: {name}") })); } + if let TriState::Val(name) = &patch.default_backend + && !is_valid_backend(name) + { + return HttpResponse::BadRequest().json(json!({ + "error": format!("invalid default_backend: {name} (expected local|hybrid)") + })); + } + if let TriState::Val(name) = &patch.reel_span + && MemoriesSpan::from_str(name).is_err() + { + return HttpResponse::BadRequest().json(json!({ + "error": format!("invalid reel_span: {name} (expected day|week|month)") + })); + } let mut dao = match app_state.user_ai_prefs_dao.lock() { Ok(dao) => dao, @@ -219,7 +326,7 @@ pub async fn put_prefs( } }; - let (voice, library, display_tz, changed) = merge_patch(existing.as_ref(), &patch); + let (merged, changed) = merge_patch(existing.as_ref(), &patch); if !changed { // Empty body: no-op, return the current row without bumping updated_at. return HttpResponse::Ok().json(row_to_response(existing.as_ref())); @@ -231,18 +338,28 @@ pub async fn put_prefs( .as_secs() as i64; let resp = PrefsResponse { - voice: voice.clone(), - library: library.clone(), - display_tz: display_tz.clone(), + voice: merged.voice.clone(), + library: merged.library.clone(), + display_tz: merged.display_tz.clone(), + default_model: merged.default_model.clone(), + default_backend: merged.default_backend.clone(), + default_persona_id: merged.default_persona_id.clone(), + reel_span: merged.reel_span.clone(), + reel_max_segments: merged.reel_max_segments, updated_at: Some(now), }; let upsert = UpsertUserAiPrefs { - voice, + voice: merged.voice, // Frozen legacy column: preserved verbatim, never written by this // surface. tz_offset_minutes: existing.as_ref().and_then(|e| e.tz_offset_minutes), - library, - display_tz, + library: merged.library, + display_tz: merged.display_tz, + default_model: merged.default_model, + default_backend: merged.default_backend, + default_persona_id: merged.default_persona_id, + reel_span: merged.reel_span, + reel_max_segments: merged.reel_max_segments, updated_at: now, }; match dao.upsert_prefs(&ctx, &upsert) { @@ -311,6 +428,27 @@ mod tests { library: library.map(str::to_string), updated_at: 1_700_000_000, display_tz: display_tz.map(str::to_string), + default_model: None, + default_backend: None, + default_persona_id: None, + reel_span: None, + reel_max_segments: None, + } + } + + /// A row with owner defaults populated, for the owner-default tests. + fn seeded_row_with_defaults() -> UpsertUserAiPrefs { + UpsertUserAiPrefs { + voice: Some("grandma".to_string()), + tz_offset_minutes: Some(-240), + library: Some("1".to_string()), + updated_at: 1_700_000_000, + display_tz: Some("America/New_York".to_string()), + default_model: Some("llama3.2:3b".to_string()), + default_backend: Some("hybrid".to_string()), + default_persona_id: Some("cabin".to_string()), + reel_span: Some("week".to_string()), + reel_max_segments: Some(24), } } @@ -363,6 +501,11 @@ mod tests { "voice": null, "library": null, "display_tz": null, + "default_model": null, + "default_backend": null, + "default_persona_id": null, + "reel_span": null, + "reel_max_segments": null, "updated_at": null }) ); @@ -682,43 +825,242 @@ mod tests { // --- merge_patch (pure) -------------------------------------------------- - #[test] - fn merge_patch_omitted_keeps_existing() { - let row = UserAiPrefs { + fn row_with_defaults() -> UserAiPrefs { + UserAiPrefs { id: 1, voice: Some("a".into()), tz_offset_minutes: None, library: Some("b".into()), updated_at: 0, display_tz: Some("c".into()), - }; - let (voice, library, display_tz, changed) = - merge_patch(Some(&row), &UpdatePrefsRequest::default()); - assert_eq!(voice.as_deref(), Some("a")); - assert_eq!(library.as_deref(), Some("b")); - assert_eq!(display_tz.as_deref(), Some("c")); + default_model: Some("m1".into()), + default_backend: Some("local".into()), + default_persona_id: Some("p1".into()), + reel_span: Some("week".into()), + reel_max_segments: Some(24), + } + } + + #[test] + fn merge_patch_omitted_keeps_existing() { + let row = row_with_defaults(); + let (merged, changed) = merge_patch(Some(&row), &UpdatePrefsRequest::default()); + assert_eq!(merged.voice.as_deref(), Some("a")); + assert_eq!(merged.library.as_deref(), Some("b")); + assert_eq!(merged.display_tz.as_deref(), Some("c")); + assert_eq!(merged.default_model.as_deref(), Some("m1")); + assert_eq!(merged.default_backend.as_deref(), Some("local")); + assert_eq!(merged.default_persona_id.as_deref(), Some("p1")); + assert_eq!(merged.reel_span.as_deref(), Some("week")); + assert_eq!(merged.reel_max_segments, Some(24)); assert!(!changed); } #[test] fn merge_patch_present_field_wins_including_null() { - let row = UserAiPrefs { - id: 1, - voice: Some("a".into()), - tz_offset_minutes: None, - library: None, - updated_at: 0, - display_tz: None, - }; + let row = row_with_defaults(); let patch = UpdatePrefsRequest { voice: TriState::Null, // clear library: TriState::Val(String::from("x")), // set display_tz: TriState::Absent, + default_backend: TriState::Null, // clear an owner default + reel_span: TriState::Val(String::from("month")), + reel_max_segments: TriState::Val(12), + ..UpdatePrefsRequest::default() }; - let (voice, library, display_tz, changed) = merge_patch(Some(&row), &patch); - assert!(voice.is_none()); - assert_eq!(library.as_deref(), Some("x")); - assert!(display_tz.is_none()); + let (merged, changed) = merge_patch(Some(&row), &patch); + assert!(merged.voice.is_none()); + assert_eq!(merged.library.as_deref(), Some("x")); + // Absent field keeps the stored value. + assert_eq!(merged.display_tz.as_deref(), Some("c")); + assert!(merged.default_backend.is_none()); + assert_eq!(merged.reel_span.as_deref(), Some("month")); + assert_eq!(merged.reel_max_segments, Some(12)); + // Untouched owner defaults survive. + assert_eq!(merged.default_model.as_deref(), Some("m1")); + assert_eq!(merged.default_persona_id.as_deref(), Some("p1")); assert!(changed); } + + // --- PUT /prefs: owner defaults ------------------------------------------ + + #[actix_rt::test] + async fn put_prefs_owner_defaults_set_and_persist() { + let (state, mut seed_dao) = prefs_state(); + seed( + &mut seed_dao, + seeded_row(Some("grandma"), Some("1"), Some("America/New_York")), + ); + let app = init_service( + App::new() + .service( + web::resource("/prefs") + .route(web::get().to(get_prefs)) + .route(web::put().to(put_prefs)), + ) + .service(web::resource("/timezones").route(web::get().to(list_timezones))) + .app_data(state.clone()), + ) + .await; + let (header, value) = auth(); + let req = TestRequest::put() + .uri("/prefs") + .insert_header((header, value)) + .set_json(serde_json::json!({ + "default_model": "llama3.2:3b", + "default_backend": "hybrid", + "default_persona_id": "cabin", + "reel_span": "week", + "reel_max_segments": 24 + })) + .to_request(); + let resp = call_service(&app, req).await; + assert_eq!(resp.status(), 200); + + let mut dao = state.user_ai_prefs_dao.lock().unwrap(); + let row = dao + .get_prefs(&opentelemetry::Context::new()) + .unwrap() + .unwrap(); + assert_eq!(row.default_model.as_deref(), Some("llama3.2:3b")); + assert_eq!(row.default_backend.as_deref(), Some("hybrid")); + assert_eq!(row.default_persona_id.as_deref(), Some("cabin")); + assert_eq!(row.reel_span.as_deref(), Some("week")); + assert_eq!(row.reel_max_segments, Some(24)); + // Mirror fields untouched by the owner-default patch. + assert_eq!(row.voice.as_deref(), Some("grandma")); + assert_eq!(row.library.as_deref(), Some("1")); + } + + #[actix_rt::test] + async fn put_prefs_owner_defaults_null_clears() { + let (state, mut seed_dao) = prefs_state(); + seed(&mut seed_dao, seeded_row_with_defaults()); + let app = init_service( + App::new() + .service( + web::resource("/prefs") + .route(web::get().to(get_prefs)) + .route(web::put().to(put_prefs)), + ) + .service(web::resource("/timezones").route(web::get().to(list_timezones))) + .app_data(state.clone()), + ) + .await; + let (header, value) = auth(); + let req = TestRequest::put() + .uri("/prefs") + .insert_header((header, value)) + .set_json(serde_json::json!({ + "default_model": null, + "default_backend": null, + "default_persona_id": null, + "reel_span": null, + "reel_max_segments": null + })) + .to_request(); + let resp = call_service(&app, req).await; + assert_eq!(resp.status(), 200); + + let mut dao = state.user_ai_prefs_dao.lock().unwrap(); + let row = dao + .get_prefs(&opentelemetry::Context::new()) + .unwrap() + .unwrap(); + assert!(row.default_model.is_none()); + assert!(row.default_backend.is_none()); + assert!(row.default_persona_id.is_none()); + assert!(row.reel_span.is_none()); + assert!(row.reel_max_segments.is_none()); + // Sibling fields survive a clear patch. + assert_eq!(row.voice.as_deref(), Some("grandma")); + } + + #[actix_rt::test] + async fn put_prefs_invalid_backend_returns_400_and_writes_nothing() { + let (state, mut seed_dao) = prefs_state(); + seed( + &mut seed_dao, + seeded_row(Some("grandma"), Some("1"), Some("America/New_York")), + ); + let app = init_service( + App::new() + .service( + web::resource("/prefs") + .route(web::get().to(get_prefs)) + .route(web::put().to(put_prefs)), + ) + .service(web::resource("/timezones").route(web::get().to(list_timezones))) + .app_data(state.clone()), + ) + .await; + let (header, value) = auth(); + let req = TestRequest::put() + .uri("/prefs") + .insert_header((header, value)) + .set_json(serde_json::json!({"default_backend": "quantum", "voice": "dad"})) + .to_request(); + let resp = call_service(&app, req).await; + assert_eq!(resp.status(), 400); + + let mut dao = state.user_ai_prefs_dao.lock().unwrap(); + let row = dao + .get_prefs(&opentelemetry::Context::new()) + .unwrap() + .unwrap(); + // Nothing written — not even the valid field in the same body. + assert_eq!(row.voice.as_deref(), Some("grandma")); + assert!(row.default_backend.is_none()); + assert_eq!(row.updated_at, 1_700_000_000); + } + + #[test] + fn backend_validator_delegates_to_canonical_parser() { + // Case-insensitive like BackendKind::parse (a stored pref must always + // be consumable downstream), but the blank string the parser tolerates + // for request fields is rejected — NULL already means "no default". + assert!(is_valid_backend("local")); + assert!(is_valid_backend("hybrid")); + assert!(is_valid_backend("Hybrid")); + assert!(is_valid_backend(" LOCAL ")); + assert!(!is_valid_backend("quantum")); + assert!(!is_valid_backend("")); + assert!(!is_valid_backend(" ")); + } + + #[actix_rt::test] + async fn put_prefs_invalid_reel_span_returns_400_and_writes_nothing() { + let (state, mut seed_dao) = prefs_state(); + seed( + &mut seed_dao, + seeded_row(Some("grandma"), Some("1"), Some("America/New_York")), + ); + let app = init_service( + App::new() + .service( + web::resource("/prefs") + .route(web::get().to(get_prefs)) + .route(web::put().to(put_prefs)), + ) + .service(web::resource("/timezones").route(web::get().to(list_timezones))) + .app_data(state.clone()), + ) + .await; + let (header, value) = auth(); + let req = TestRequest::put() + .uri("/prefs") + .insert_header((header, value)) + .set_json(serde_json::json!({"reel_span": "decade"})) + .to_request(); + let resp = call_service(&app, req).await; + assert_eq!(resp.status(), 400); + + let mut dao = state.user_ai_prefs_dao.lock().unwrap(); + let row = dao + .get_prefs(&opentelemetry::Context::new()) + .unwrap() + .unwrap(); + assert!(row.reel_span.is_none()); + assert_eq!(row.updated_at, 1_700_000_000); + } } diff --git a/src/reels/mod.rs b/src/reels/mod.rs index c46b0c6..cf5dceb 100644 --- a/src/reels/mod.rs +++ b/src/reels/mod.rs @@ -18,6 +18,7 @@ pub mod selector; use std::collections::HashMap; use std::path::{Path, PathBuf}; +use std::str::FromStr; use std::sync::{LazyLock, Mutex, Mutex as StdMutex}; use std::time::{Duration, Instant}; @@ -71,13 +72,38 @@ pub fn normalize_library_key(libs: &[Library], param: Option<&str>) -> String { } } +/// Reel span / segment-cap resolution: explicit request param → stored +/// owner pref → hard default. A stored span that no longer parses (e.g. +/// hand-edited) is skipped rather than erroring — the hard default still +/// applies. Non-positive stored segment caps are ignored the same way. +fn resolve_reel_params( + req_span: Option, + req_max_segments: Option, + prefs: Option<&crate::database::models::UserAiPrefs>, +) -> (MemoriesSpan, usize) { + let pref_span = prefs + .and_then(|p| p.reel_span.as_deref()) + .and_then(|s| MemoriesSpan::from_str(s).ok()); + let pref_max_segments = prefs + .and_then(|p| p.reel_max_segments) + .filter(|v| *v > 0) + .map(|v| v as usize); + let span = req_span.or(pref_span).unwrap_or(MemoriesSpan::Day); + let max_segments = req_max_segments + .or(pref_max_segments) + .unwrap_or(selector::DEFAULT_MAX_SEGMENTS); + (span, max_segments) +} + /// Best-effort: mirror the latest client reel params into `user_ai_prefs` /// so the nightly pre-gen scheduler can pick them up. Never fails the /// caller regardless of DB errors. fn capture_prefs( app_state: &AppState, - req: &web::Json, + req: &CreateReelRequest, library_param: Option<&str>, + effective_span: MemoriesSpan, + effective_max_segments: usize, ) -> Result<(), anyhow::Error> { use crate::database::models::UpsertUserAiPrefs; let now = std::time::SystemTime::now() @@ -93,6 +119,21 @@ fn capture_prefs( }; let mut dao = app_state.user_ai_prefs_dao.lock().expect("lock"); let ctx = opentelemetry::Context::new(); + // The row passively mirrors the latest request's EFFECTIVE reel params + // (after the request → pref → default resolution done by the caller), so + // the stored reel_span / reel_max_segments track what was last used — + // idempotent when the pref itself supplied the effective value. + // + // The owner-default fields (model/backend/persona) are owned by PUT /prefs, + // not by reel requests. The upsert overwrites every column, so we must + // carry the existing values through unchanged — writing None here would + // silently wipe the owner's stored defaults on every reel creation. If the + // read fails we bail without writing: this is best-effort, and clobbering + // owner defaults on a transient error is worse than skipping a mirror. + let existing = match dao.get_prefs(&ctx) { + Ok(row) => row, + Err(e) => return Err(anyhow::anyhow!("failed to read prefs before mirror: {e}")), + }; dao.upsert_prefs( &ctx, &UpsertUserAiPrefs { @@ -104,6 +145,11 @@ fn capture_prefs( library: Some(library), updated_at: now as i64, display_tz: req.display_tz.clone().filter(|s| !s.is_empty()), + default_model: existing.as_ref().and_then(|e| e.default_model.clone()), + default_backend: existing.as_ref().and_then(|e| e.default_backend.clone()), + default_persona_id: existing.as_ref().and_then(|e| e.default_persona_id.clone()), + reel_span: Some(effective_span.to_string()), + reel_max_segments: Some(effective_max_segments as i32), }, ) .map_err(|e| anyhow::anyhow!("failed to upsert user_ai_prefs: {e}")) @@ -415,20 +461,21 @@ pub async fn create_reel_handler( })); } - let span = req.span.unwrap_or(MemoriesSpan::Day); - let max_segments = req.max_segments.unwrap_or(selector::DEFAULT_MAX_SEGMENTS); - // Grouping zone: explicit request param → stored pref → app default - // (ADR-0001). The legacy device-offset param is still mirrored into the - // frozen prefs column by `capture_prefs` for rollback but no longer - // drives selection. + // Owner defaults: explicit request param → stored pref → hard default. + // The stored row is read once and reused for the span, the segment cap, + // and the display zone. The legacy device-offset param is still mirrored + // into the frozen prefs column by `capture_prefs` for rollback but no + // longer drives selection. + let stored_prefs = app_state + .user_ai_prefs_dao + .lock() + .ok() + .and_then(|mut dao| dao.get_prefs(&opentelemetry::Context::new()).ok().flatten()); + let (span, max_segments) = + resolve_reel_params(req.span, req.max_segments, stored_prefs.as_ref()); let display_zone_name = { let req_tz = req.display_tz.clone().filter(|s| !s.trim().is_empty()); - let stored = app_state - .user_ai_prefs_dao - .lock() - .ok() - .and_then(|mut dao| dao.get_prefs(&opentelemetry::Context::new()).ok().flatten()) - .and_then(|p| p.display_tz); + let stored = stored_prefs.as_ref().and_then(|p| p.display_tz.clone()); crate::camera_tz::display_zone_for(req_tz.as_deref().or(stored.as_deref())) }; let selector = ReelSelector::Memories { @@ -489,7 +536,7 @@ pub async fn create_reel_handler( }, ); // Capture params for passive prefs mirror (best-effort, never fails). - let _ = capture_prefs(&app_state, &req, req.library.as_deref()); + let _ = capture_prefs(&app_state, &req, req.library.as_deref(), span, max_segments); return HttpResponse::Accepted().json(ReelJobCreatedResponse { job_id: job_id.to_string(), status: ReelJobStatus::Done, @@ -549,7 +596,7 @@ pub async fn create_reel_handler( with_job(job_id, |job| job.abort = Some(handle.abort_handle())); // Capture params for passive prefs mirror (best-effort, never fails). - let _ = capture_prefs(&app_state, &req, req.library.as_deref()); + let _ = capture_prefs(&app_state, &req, req.library.as_deref(), span, max_segments); HttpResponse::Accepted().json(ReelJobCreatedResponse { job_id: job_id.to_string(), @@ -1333,9 +1380,121 @@ fn sweep_reel_cache(app_state: &AppState) -> usize { #[cfg(test)] mod tests { use super::*; + use crate::database::models::UserAiPrefs; use crate::libraries::Library; use chrono::TimeZone; + fn prefs_row(span: Option<&str>, max_segments: Option) -> UserAiPrefs { + UserAiPrefs { + id: 1, + voice: None, + tz_offset_minutes: None, + library: None, + updated_at: 0, + display_tz: None, + default_model: None, + default_backend: None, + default_persona_id: None, + reel_span: span.map(str::to_string), + reel_max_segments: max_segments, + } + } + + #[test] + fn resolve_reel_params_request_wins_over_pref() { + let prefs = prefs_row(Some("month"), Some(10)); + let (span, max) = resolve_reel_params(Some(MemoriesSpan::Day), Some(5), Some(&prefs)); + assert_eq!(span, MemoriesSpan::Day); + assert_eq!(max, 5); + } + + #[test] + fn resolve_reel_params_pref_fills_omitted_request() { + let prefs = prefs_row(Some("week"), Some(24)); + let (span, max) = resolve_reel_params(None, None, Some(&prefs)); + assert_eq!(span, MemoriesSpan::Week); + assert_eq!(max, 24); + } + + #[test] + fn resolve_reel_params_no_pref_falls_to_hard_defaults() { + let (span, max) = resolve_reel_params(None, None, None); + assert_eq!(span, MemoriesSpan::Day); + assert_eq!(max, selector::DEFAULT_MAX_SEGMENTS); + } + + #[test] + fn resolve_reel_params_invalid_or_nonpositive_pref_skipped() { + // Hand-edited / corrupt stored values fall through to hard defaults + // instead of erroring the request. + let bad_span = prefs_row(Some("decade"), Some(0)); + let (span, max) = resolve_reel_params(None, None, Some(&bad_span)); + assert_eq!(span, MemoriesSpan::Day); + assert_eq!(max, selector::DEFAULT_MAX_SEGMENTS); + } + + /// Regression: reel creation is a PASSIVE mirror. The upsert overwrites + /// every column, so `capture_prefs` must carry the PUT /prefs-owned + /// default_model / default_backend / default_persona_id through unchanged + /// — a reel request has no business wiping the owner's AI defaults. + #[actix_rt::test] + async fn reel_capture_preserves_owner_defaults() { + use crate::database::models::UpsertUserAiPrefs; + use crate::database::{SqliteUserAiPrefsDao, UserAiPrefsDao}; + use crate::state::AppState; + use std::sync::{Arc, Mutex}; + + let conn = Arc::new(Mutex::new(crate::database::test::in_memory_db_connection())); + let mut state = AppState::test_state(); + state.user_ai_prefs_dao = Arc::new(Mutex::new(Box::new( + SqliteUserAiPrefsDao::from_connection(conn.clone()), + ))); + { + let mut dao = SqliteUserAiPrefsDao::from_connection(conn.clone()); + dao.upsert_prefs( + &opentelemetry::Context::new(), + &UpsertUserAiPrefs { + voice: None, + tz_offset_minutes: None, + library: None, + updated_at: 0, + display_tz: None, + default_model: Some("llama3.2:3b".into()), + default_backend: Some("hybrid".into()), + default_persona_id: Some("cabin".into()), + reel_span: None, + reel_max_segments: None, + }, + ) + .expect("seed owner defaults"); + } + + let req = CreateReelRequest { + span: Some(MemoriesSpan::Week), + timezone_offset_minutes: Some(-240), + display_tz: None, + library: None, + voice: Some("grandma".into()), + max_segments: Some(12), + }; + capture_prefs(&state, &req, None, MemoriesSpan::Week, 12).expect("capture"); + + let row = state + .user_ai_prefs_dao + .lock() + .expect("lock") + .get_prefs(&opentelemetry::Context::new()) + .expect("read") + .expect("row"); + // The mirrored reel params landed... + assert_eq!(row.reel_span.as_deref(), Some("week")); + assert_eq!(row.reel_max_segments, Some(12)); + // ...and the owner defaults survived untouched. + assert_eq!(row.default_model.as_deref(), Some("llama3.2:3b")); + assert_eq!(row.default_backend.as_deref(), Some("hybrid")); + assert_eq!(row.default_persona_id.as_deref(), Some("cabin")); + } + fn photo(p: &str, lib: i32) -> SegmentMedia { SegmentMedia::Photo { rel_path: p.to_string(), -- 2.54.0 From e84ac82f865f3d1dc24c103f2fb89aeee0a1aa5e Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Sun, 27 Sep 2026 22:50:28 -0400 Subject: [PATCH 2/5] ADR-0002: owner defaults live in the DB row; .env keeps deployment config and secrets Records the config boundary the #127 owner-default columns establish: .env = boot-time deployment config + secrets (never DB), user_ai_prefs single row = per-request owner defaults (hot-tunable, no secrets), resolution request param -> owner default -> hard default, and the safe-by-default privacy posture for default_backend. Adds the Configuration glossary section to CONTEXT.md (owner default, deployment config, hard default). --- CONTEXT.md | 22 ++++++++ docs/adr/0002-owner-defaults-vs-env.md | 78 ++++++++++++++++++++++++++ 2 files changed, 100 insertions(+) create mode 100644 docs/adr/0002-owner-defaults-vs-env.md diff --git a/CONTEXT.md b/CONTEXT.md index cd59bf4..51c5040 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -62,3 +62,25 @@ Insights and pre-computed artifacts grouped by calendar day (daily message summa reel pre-generation). Because it is keyed on the day, re-dating a photo across a day boundary invalidates the derived data for that day, which regenerates on its normal schedule. _Avoid_: cache, nightly output + +### Configuration + +**Owner default**: +A per-request default the owner sets for the whole deployment — the model, backend, +persona, reel span, or segment cap a client gets when it omits the field. Stored in the +single `user_ai_prefs` row, hot-tunable without a redeploy, and global: every client that +omits the field gets it, on every device. Never holds secrets. +_Avoid_: user prefs, user settings, preferences (ambiguous with device-local client state) + +**Deployment config**: +Values the server needs before it can start serving — bind address, database path, media +roots, the URLs used to build clients — plus every secret. Lives in `.env` only; changing +it is a redeploy, by design. +_Avoid_: environment config, system config + +**Hard default**: +The env/code fallback a request lands on when neither an explicit param nor a stored +owner default supplies the field (e.g. backend `local`, persona `default`, reel span +day). It is the floor under the owner-default layer: env changes keep working whenever a +default column is NULL. +_Avoid_: fallback value, env default diff --git a/docs/adr/0002-owner-defaults-vs-env.md b/docs/adr/0002-owner-defaults-vs-env.md new file mode 100644 index 0000000..6dc5910 --- /dev/null +++ b/docs/adr/0002-owner-defaults-vs-env.md @@ -0,0 +1,78 @@ +# ADR-0002: Owner defaults live in the DB row; .env keeps deployment config and secrets + +**Status**: accepted (2026-09-27) + +## Context + +All configuration lived in `.env`: bind address, media paths, Ollama/llama-swap/OpenRouter +URLs, model names, `LLM_BACKEND`, the embedding contract, TTS voices. Two friction points: +values the owner actually *tunes* (which model, which backend, which persona, how long a +reel is) needed an edit + redeploy to change, and clients had no server-side fallback — +"use the default" was expressed by omitting a field, and the default itself was invisible +and untouchable at runtime. The `user_ai_prefs` single row (`id = 1`) already existed as +a passive mirror of the last client reel request for the nightly pre-gen. #126/#127 gave +it an explicit `PUT /prefs` surface and five owner-default columns (`default_model`, +`default_backend`, `default_persona_id`, `reel_span`, `reel_max_segments`) — the first +batch of non-private configuration moved out of `.env` and into the database. + +## Decision + +Configuration splits into two homes by one rule — *does the server need the value before +it can start serving?* + +- **`.env` (deployment config)**: everything read at boot to bind, connect, or open — + `BIND_URL`, `DATABASE_URL`, media paths, `OLLAMA_*_URL` / `LLAMA_SWAP_URL`, + `LLM_BACKEND`, the embedding contract — and **all secrets** (`OPENROUTER_API_KEY`, + `SECRET_KEY`, `SMS_API_TOKEN`). Secrets never go in the database: the SQLite file gets + backed up, copied, and attached to bug reports. +- **`user_ai_prefs` (owner defaults)**: per-request defaults resolved at request time — + the model/backend/persona a client gets when it omits the field, reel span and + segment cap, display zone, TTS voice. Hot-tunable via `PUT /prefs`, effective on the + next request, no redeploy. This row is the designated home for future non-private, + per-request configuration that outgrows `.env`. The table is a **single row per + deployment, not a per-user profile** — `user_ai_prefs` is a historical name; the + canonical term is *owner defaults* (single-user tool: owner = admin). + +Resolution order at every consumer: **explicit request param → stored owner default → +hard default** (the env/code fallback). An explicit request always wins, so any client +can override the owner's default per request. The env value stays the floor under the +DB: a NULL `default_model` means "whatever the Ollama/llama-swap primary is", so env +changes keep working. + +**Privacy posture is safe-by-default.** `default_backend` is the one owner default with +network-egress consequences: `hybrid` routes insight generation and chat to OpenRouter. +With no row or a NULL column, the hard default is `local` — data stays on the LAN. +Leaving the network requires an explicit owner action (storing `hybrid`), and any client +can still pin itself by sending `backend` explicitly. Owner defaults therefore split +into two sensitivity classes: **harmless tuning** (span, segments, display zone, voice, +model, persona) and **privacy/security** (`default_backend`), which any admin UI must +surface with a warning, not a quiet dropdown. + +## Considered options + +- **Everything in `.env`** (status quo): safe, but tuning a model default means edit + + redeploy, and clients have no server-side default to fall back to — a picker's + "server default" is just the env value, invisible and unchangeable at runtime. + Rejected for per-request defaults. +- **Everything in the DB**: boot-time values (bind address, DB path, the URLs used to + build the HTTP clients) become chicken-and-egg, and secrets sit next to tuning values + in a file that gets copied around. Rejected. +- **A separate `system_config` table**: cleaner name, but a second single-row table + would be a synonym with extra migrations; the row and the `PUT /prefs` surface already + exist. Rejected — the row keeps its name, its *term* becomes owner defaults. + +## Non-obvious rules worth remembering + +- **Owner defaults are global to the deployment.** Every client that omits a field gets + the stored value, on every device. Storing `default_backend=hybrid` changes the + mobile app's behavior even for users who never touch the setting; a client that must + stay local should send `backend` explicitly (tracked for FileViewer-React #33). +- **A stored value that is no longer consumable fails at write time, not request time** — + `PUT /prefs` validates `default_backend` against the canonical `BackendKind::parse` + and `reel_span` against the span enum before writing, so a bad default cannot 500 + every request later. +- **The passive reel mirror (`capture_prefs`) may only write the fields it owns** (voice, + library, display zone, effective span/segments); it carries the AI owner-defaults + through unchanged, because the upsert overwrites every column. +- **NULL means "use the hard default", not "unset the env"** — the DB layer sits + *above* env, never instead of it. -- 2.54.0 From 696d9fa8f3496c5f97bd590923d067216135a0cf Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Mon, 28 Sep 2026 01:21:46 -0400 Subject: [PATCH 3/5] Otel: llm.request client spans + traceparent on LLM calls (#130) Add OpenTelemetry tracing to every LLM client call so an insight/chat turn is a single trace from HTTP entry to each downstream inference. - LlmClient trait: thread a &opentelemetry::Context through all 5 methods (generate, chat_with_tools, chat_with_tools_stream, generate_embeddings, describe_image), matching the DAO convention of explicit context threading rather than ambient Context::current(). - otel::start_llm_request_span: single shared helper that every provider (openrouter / llamacpp / ollama) funnels through, so the span shape is identical: name llm.request, SpanKind::Client, llm.provider / llm.operation / llm.model. - Per-method spans in all three clients with the spec's operation enum (chat | chat_stream | embeddings | describe_image | generate); stream spans are owned and moved into the async_stream generator so they end when the response drains, not when headers arrive. - llm.tokens.input / llm.tokens.output recorded on non-stream and stream completion; llm.cost_usd from usage.cost (OpenRouter); http.status_code + error status on failure. - W3C traceparent injected on outgoing LLM HTTP requests (llama-swap, Ollama, OpenRouter) gated on span_context.is_valid() so debug no-op-tracer builds don't send an all-zero parent. - Turn-level spans (ai.chat.turn.execute, ai.persona_chat.turn.execute) now carry llm.backend + llm.model; with_entry streaming paths thread the turn context (turn_cx) to child describe_image / agentic-loop calls instead of the ambient context. clippy clean; 690 lib + 775 integration + 4 doc tests green. --- src/ai/daily_summary_job.rs | 12 +- src/ai/insight_chat.rs | 49 +++-- src/ai/insight_generator.rs | 100 +++++++---- src/ai/llamacpp.rs | 159 +++++++++++++--- src/ai/llm_client.rs | 14 +- src/ai/local_llm.rs | 29 ++- src/ai/mod.rs | 14 +- src/ai/nl_query.rs | 5 +- src/ai/ollama.rs | 328 ++++++++++++++++++++++++++++------ src/ai/openrouter.rs | 307 ++++++++++++++++++++++--------- src/ai/persona_chat.rs | 16 +- src/ai/sms_client.rs | 2 + src/bin/test_daily_summary.rs | 8 +- src/knowledge.rs | 3 +- src/otel.rs | 93 +++++++++- src/reels/script.rs | 3 +- src/unified_search.rs | 2 +- 17 files changed, 910 insertions(+), 234 deletions(-) diff --git a/src/ai/daily_summary_job.rs b/src/ai/daily_summary_job.rs index 3fede71..acf7451 100644 --- a/src/ai/daily_summary_job.rs +++ b/src/ai/daily_summary_job.rs @@ -360,11 +360,17 @@ async fn generate_and_store_daily_summary( span.set_attribute(KeyValue::new("date", date.to_string())); span.set_attribute(KeyValue::new("contact", contact.to_string())); span.set_attribute(KeyValue::new("message_count", messages.len() as i64)); + // Parent context for the child llm.request spans; the span itself stays + // borrowed here for the attribute updates below. + let llm_cx = + opentelemetry::Context::new().with_remote_span_context(span.span_context().clone()); let (prompt, system_prompt) = build_daily_summary_prompt(contact, date, messages); // Generate summary with LLM - let summary = ollama.generate(&prompt, Some(system_prompt)).await?; + let summary = ollama + .generate(&llm_cx, &prompt, Some(system_prompt)) + .await?; log::debug!( "Generated summary for {}: {}", @@ -382,7 +388,9 @@ async fn generate_and_store_daily_summary( ); // Embed the stripped summary (store original summary in DB) - let embedding = ollama.generate_embedding(&stripped_summary).await?; + let embedding = ollama + .generate_embedding(&llm_cx, &stripped_summary) + .await?; span.set_attribute(KeyValue::new( "embedding_dimensions", diff --git a/src/ai/insight_chat.rs b/src/ai/insight_chat.rs index 66b0449..f4dd3da 100644 --- a/src/ai/insight_chat.rs +++ b/src/ai/insight_chat.rs @@ -377,7 +377,7 @@ impl InsightChatService { let (response, prompt_tokens, eval_tokens) = backend .chat() - .chat_with_tools(messages.clone(), tools.clone()) + .chat_with_tools(&loop_cx, messages.clone(), tools.clone()) .await?; last_prompt_eval_count = prompt_tokens; last_eval_count = eval_tokens; @@ -434,7 +434,7 @@ impl InsightChatService { let synthetic_idx = push_synthetic_final_prompt(&mut messages); let (final_response, prompt_tokens, eval_tokens) = backend .chat() - .chat_with_tools(messages.clone(), vec![]) + .chat_with_tools(&loop_cx, messages.clone(), vec![]) .await?; last_prompt_eval_count = prompt_tokens; last_eval_count = eval_tokens; @@ -492,6 +492,7 @@ impl InsightChatService { let title_raw = backend .chat() .generate( + &loop_cx, &title_prompt, Some( "You are my long term memory assistant. Use only the information provided. Do not invent details.", @@ -852,7 +853,7 @@ impl InsightChatService { span.set_attribute(KeyValue::new("library_id", library_id as i64)); let result = svc - .run_streaming_turn_with_entry(req, entry_clone.clone()) + .run_streaming_turn_with_entry(req, entry_clone.clone(), &mut span) .await; if let Err(ref e) = result { span.set_attribute(KeyValue::new("status", "error")); @@ -882,6 +883,7 @@ impl InsightChatService { self: Arc, req: ChatTurnRequest, entry: Arc, + turn_span: &mut opentelemetry::global::BoxedSpan, ) -> Result<()> { if req.user_message.trim().is_empty() { bail!("user_message must not be empty"); @@ -911,11 +913,11 @@ impl InsightChatService { if req.regenerate || existing_insight.is_none() { return self - .run_bootstrap_streaming_with_entry(req, normalized, entry) + .run_bootstrap_streaming_with_entry(req, normalized, entry, turn_span) .await; } let insight = existing_insight.expect("just checked Some above"); - self.run_continuation_streaming_with_entry(req, normalized, insight, entry) + self.run_continuation_streaming_with_entry(req, normalized, insight, entry, turn_span) .await } @@ -926,6 +928,7 @@ impl InsightChatService { normalized: String, insight: crate::database::models::PhotoInsight, entry: Arc, + turn_span: &mut opentelemetry::global::BoxedSpan, ) -> Result<()> { let active_persona = req .persona_id @@ -981,6 +984,12 @@ impl InsightChatService { }; let backend = self.generator.resolve_backend(kind, &overrides).await?; let model_used = backend.model().to_string(); + // llm.* attributes on the turn-level span (issue #130): clients can't + // know local-vs-hybrid, but the turn knows its resolved backend. + turn_span.set_attribute(KeyValue::new("llm.backend", kind.as_str())); + turn_span.set_attribute(KeyValue::new("llm.model", model_used.clone())); + let turn_cx = opentelemetry::Context::new() + .with_remote_span_context(turn_span.span_context().clone()); let local_first_user_has_image = messages .iter() @@ -1030,6 +1039,7 @@ impl InsightChatService { &active_persona, max_iterations, &entry, + &turn_cx, ) .await?; let AgenticLoopOutcome { @@ -1146,6 +1156,7 @@ impl InsightChatService { req: ChatTurnRequest, normalized: String, entry: Arc, + turn_span: &mut opentelemetry::global::BoxedSpan, ) -> Result<()> { let active_persona = req .persona_id @@ -1171,6 +1182,11 @@ impl InsightChatService { }; let backend = self.generator.resolve_backend(kind, &overrides).await?; let model_used = backend.model().to_string(); + // llm.* attributes on the turn-level span (issue #130). + turn_span.set_attribute(KeyValue::new("llm.backend", kind.as_str())); + turn_span.set_attribute(KeyValue::new("llm.model", model_used.clone())); + let turn_cx = opentelemetry::Context::new() + .with_remote_span_context(turn_span.span_context().clone()); let image_base64: Option = self.generator.load_image_as_base64(&normalized).ok(); @@ -1190,7 +1206,7 @@ impl InsightChatService { let visual_block = if !backend.images_inline { match image_base64.as_deref() { - Some(b64) => match backend.local().describe_image(b64).await { + Some(b64) => match backend.local().describe_image(&turn_cx, b64).await { Ok(desc) => { format!("Visual description (from local vision model):\n{}\n", desc) } @@ -1245,6 +1261,7 @@ impl InsightChatService { &active_persona, max_iterations, &entry, + &turn_cx, ) .await?; let AgenticLoopOutcome { @@ -1333,6 +1350,7 @@ impl InsightChatService { active_persona: &str, max_iterations: usize, entry: &Arc, + cx: &opentelemetry::Context, ) -> Result { crate::ai::insight_chat::run_streaming_agentic_loop_with_entry( &self.generator, @@ -1345,6 +1363,7 @@ impl InsightChatService { active_persona, max_iterations, entry, + cx, ) .await } @@ -1500,6 +1519,7 @@ impl InsightChatService { &active_persona, max_iterations, &tx, + &opentelemetry::Context::current(), ) .await?; let AgenticLoopOutcome { @@ -1667,7 +1687,11 @@ impl InsightChatService { // images_inline backends send images directly to the chat model. let visual_block = if !backend.images_inline { match image_base64.as_deref() { - Some(b64) => match backend.local().describe_image(b64).await { + Some(b64) => match backend + .local() + .describe_image(&opentelemetry::Context::current(), b64) + .await + { Ok(desc) => { format!("Visual description (from local vision model):\n{}\n", desc) } @@ -1734,6 +1758,7 @@ impl InsightChatService { &active_persona, max_iterations, &tx, + &opentelemetry::Context::current(), ) .await?; let AgenticLoopOutcome { @@ -1816,6 +1841,7 @@ impl InsightChatService { active_persona: &str, max_iterations: usize, tx: &tokio::sync::mpsc::Sender, + cx: &opentelemetry::Context, ) -> Result { let mut tool_calls_made = 0usize; let mut iterations_used = 0usize; @@ -1834,7 +1860,7 @@ impl InsightChatService { let mut stream = backend .chat() - .chat_with_tools_stream(messages.clone(), tools.clone()) + .chat_with_tools_stream(cx, messages.clone(), tools.clone()) .await?; let mut final_message: Option = None; @@ -1928,7 +1954,7 @@ impl InsightChatService { let synthetic_idx = push_synthetic_final_prompt(messages); let mut stream = backend .chat() - .chat_with_tools_stream(messages.clone(), vec![]) + .chat_with_tools_stream(cx, messages.clone(), vec![]) .await?; let mut final_message: Option = None; while let Some(ev) = stream.next().await { @@ -2308,6 +2334,7 @@ pub async fn run_streaming_agentic_loop_with_entry( active_persona: &str, max_iterations: usize, entry: &Arc, + cx: &opentelemetry::Context, ) -> Result { let mut tool_calls_made = 0usize; let mut iterations_used = 0usize; @@ -2337,7 +2364,7 @@ pub async fn run_streaming_agentic_loop_with_entry( let mut stream = backend .chat() - .chat_with_tools_stream(messages.clone(), tools.clone()) + .chat_with_tools_stream(cx, messages.clone(), tools.clone()) .await?; let mut final_message: Option = None; @@ -2420,7 +2447,7 @@ pub async fn run_streaming_agentic_loop_with_entry( let synthetic_idx = push_synthetic_final_prompt(messages); let mut stream = backend .chat() - .chat_with_tools_stream(messages.clone(), vec![]) + .chat_with_tools_stream(cx, messages.clone(), vec![]) .await?; let mut final_message: Option = None; while let Some(ev) = stream.next().await { diff --git a/src/ai/insight_generator.rs b/src/ai/insight_generator.rs index be44108..1546eb3 100644 --- a/src/ai/insight_generator.rs +++ b/src/ai/insight_generator.rs @@ -646,7 +646,8 @@ impl InsightGenerator { // (`LLM_BACKEND` switch). Must match the backend that populated the // daily-summary embeddings or similarity search will be garbage. let query_embedding = - crate::ai::embed_query(&self.ollama, self.llamacpp.as_deref(), &query).await?; + crate::ai::embed_query(&search_cx, &self.ollama, self.llamacpp.as_deref(), &query) + .await?; // Search for similar daily summaries with time-based weighting // This prioritizes summaries temporally close to the query date @@ -712,7 +713,8 @@ impl InsightGenerator { // Must use the same backend that populated the daily-summary // embeddings or similarity search is garbage (see embed_one docs). let query_embedding = - crate::ai::embed_query(&self.ollama, self.llamacpp.as_deref(), query).await?; + crate::ai::embed_query(&search_cx, &self.ollama, self.llamacpp.as_deref(), query) + .await?; let mut summary_dao = self .daily_summary_dao @@ -798,7 +800,9 @@ impl InsightGenerator { let calendar_cx = parent_cx.with_span(span); let query_embedding = if let Some(loc) = location { - match crate::ai::embed_query(&self.ollama, self.llamacpp.as_deref(), loc).await { + match crate::ai::embed_query(&calendar_cx, &self.ollama, self.llamacpp.as_deref(), loc) + .await + { Ok(emb) => Some(emb), Err(e) => { log::warn!("Failed to generate embedding for location '{}': {}", loc, e); @@ -969,18 +973,23 @@ impl InsightGenerator { ) }; - let query_embedding = - match crate::ai::embed_query(&self.ollama, self.llamacpp.as_deref(), &query_text).await - { - Ok(emb) => emb, - Err(e) => { - log::warn!("Failed to generate search embedding: {}", e); - search_cx.span().set_status(Status::Error { - description: e.to_string().into(), - }); - return Ok(None); - } - }; + let query_embedding = match crate::ai::embed_query( + &search_cx, + &self.ollama, + self.llamacpp.as_deref(), + &query_text, + ) + .await + { + Ok(emb) => emb, + Err(e) => { + log::warn!("Failed to generate search embedding: {}", e); + search_cx.span().set_status(Status::Error { + description: e.to_string().into(), + }); + return Ok(None); + } + }; let searches = { let mut dao = self @@ -1254,7 +1263,10 @@ impl InsightGenerator { // 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 { + match ollama_client + .generate_photo_description(&insight_cx, img_b64) + .await + { Ok(desc) => { log::info!("Photo description for RAG enrichment: {}", desc); Some(desc) @@ -1323,7 +1335,7 @@ impl InsightGenerator { if !immediate_messages.is_empty() { // Step 2: Extract topics from immediate messages to enrich RAG query let topics = self - .extract_topics_from_messages(&immediate_messages, &ollama_client) + .extract_topics_from_messages(&immediate_messages, &ollama_client, &insight_cx) .await; log::info!("Extracted topics for query enrichment: {:?}", topics); @@ -1379,6 +1391,7 @@ impl InsightGenerator { &immediate_messages, &ollama_client, custom_system_prompt.as_deref(), + &insight_cx, ) .await .unwrap_or_else(|| String::from("No immediate context")); @@ -1388,6 +1401,7 @@ impl InsightGenerator { &historical_messages, &ollama_client, custom_system_prompt.as_deref(), + &insight_cx, ) .await .unwrap_or_else(|| String::from("No historical context")); @@ -1406,6 +1420,7 @@ impl InsightGenerator { &immediate_messages, &ollama_client, custom_system_prompt.as_deref(), + &insight_cx, ) .await; } @@ -1416,6 +1431,7 @@ impl InsightGenerator { &immediate_messages, &ollama_client, custom_system_prompt.as_deref(), + &insight_cx, ) .await; } @@ -1441,6 +1457,7 @@ impl InsightGenerator { &rag_messages, &ollama_client, custom_system_prompt.as_deref(), + &insight_cx, ) .await; } @@ -1473,7 +1490,7 @@ impl InsightGenerator { if !sms_messages.is_empty() { match self .sms_client - .summarize_context(&sms_messages, &ollama_client) + .summarize_context(&sms_messages, &ollama_client, &insight_cx) .await { Ok(summary) => { @@ -1557,6 +1574,7 @@ impl InsightGenerator { // 10. Generate summary first, then derive title from the summary let summary = ollama_client .generate_photo_summary( + &insight_cx, date_taken, location.as_deref(), contact.as_deref(), @@ -1567,7 +1585,7 @@ impl InsightGenerator { .await?; let title = ollama_client - .generate_photo_title(&summary, custom_system_prompt.as_deref()) + .generate_photo_title(&insight_cx, &summary, custom_system_prompt.as_deref()) .await?; log::info!("Generated title: {}", title); @@ -1634,6 +1652,7 @@ impl InsightGenerator { &self, messages: &[crate::ai::SmsMessage], ollama: &OllamaClient, + cx: &opentelemetry::Context, ) -> Vec { if messages.is_empty() { return Vec::new(); @@ -1669,7 +1688,7 @@ Return ONLY the comma-separated list, nothing else."#, ); match ollama - .generate(&prompt, Some("You are an entity extraction assistant. Extract proper nouns, people, places, and domain-specific terms that provide context.")) + .generate(cx, &prompt, Some("You are an entity extraction assistant. Extract proper nouns, people, places, and domain-specific terms that provide context.")) .await { Ok(response) => { @@ -1704,6 +1723,7 @@ Return ONLY the comma-separated list, nothing else."#, messages: &[String], ollama: &OllamaClient, custom_system: Option<&str>, + cx: &opentelemetry::Context, ) -> Option { if messages.is_empty() { return None; @@ -1724,7 +1744,7 @@ Return ONLY the summary, nothing else."#, let system = custom_system .unwrap_or("You are a context summarization assistant. Be concise and factual."); - match ollama.generate(&prompt, Some(system)).await { + match ollama.generate(cx, &prompt, Some(system)).await { Ok(summary) => Some(summary), Err(e) => { log::warn!("Failed to summarize messages: {}", e); @@ -1740,6 +1760,7 @@ Return ONLY the summary, nothing else."#, messages: &[crate::ai::SmsMessage], ollama: &OllamaClient, custom_system: Option<&str>, + cx: &opentelemetry::Context, ) -> Option { if messages.is_empty() { return None; @@ -1785,7 +1806,7 @@ Return ONLY the summary, nothing else."#, "You are a context summarization assistant. Be detailed and factual, preserving important context.", ); - match ollama.generate(&prompt, Some(system)).await { + match ollama.generate(cx, &prompt, Some(system)).await { Ok(summary) => Some(summary), Err(e) => { log::warn!("Failed to summarize immediate context: {}", e); @@ -1829,7 +1850,7 @@ Return ONLY the summary, nothing else."#, "get_file_tags" => self.tool_get_file_tags(arguments, cx).await, "get_faces_in_photo" => self.tool_get_faces_in_photo(arguments, cx).await, "describe_photo" => { - self.tool_describe_photo(backend.local(), image_base64) + self.tool_describe_photo(backend.local(), image_base64, cx) .await } "reverse_geocode" => self.tool_reverse_geocode(arguments).await, @@ -1880,7 +1901,7 @@ Return ONLY the summary, nothing else."#, &self, args: &serde_json::Value, local: &dyn LlmClient, - _cx: &opentelemetry::Context, + cx: &opentelemetry::Context, ) -> String { let query = match args.get("query").and_then(|v| v.as_str()) { Some(q) => q.to_string(), @@ -1944,7 +1965,10 @@ Return ONLY the summary, nothing else."#, }; let final_results = if rerank_enabled && results.len() > limit { - match self.rerank_with_llm(&query, &results, limit, local).await { + match self + .rerank_with_llm(cx, &query, &results, limit, local) + .await + { Ok(reordered) => reordered, Err(e) => { log::warn!("rerank failed, using vector order: {}", e); @@ -1967,6 +1991,7 @@ Return ONLY the summary, nothing else."#, /// tool_result. On parse failure we fall back to the input order. async fn rerank_with_llm( &self, + cx: &opentelemetry::Context, query: &str, candidates: &[String], limit: usize, @@ -2010,7 +2035,7 @@ Return ONLY the summary, nothing else."#, let started = std::time::Instant::now(); let system = Some("You are a terse relevance ranker. You output only numbers separated by commas."); - let response = local.generate(&prompt, system, None).await?; + let response = local.generate(cx, &prompt, system, None).await?; log::info!( "rerank: finished in {} ms (prompt={} chars)", started.elapsed().as_millis(), @@ -2620,10 +2645,11 @@ Return ONLY the summary, nothing else."#, &self, local: &dyn LlmClient, image_base64: &Option, + cx: &opentelemetry::Context, ) -> String { log::info!("tool_describe_photo: generating visual description"); match image_base64 { - Some(img) => match local.describe_image(img).await { + Some(img) => match local.describe_image(cx, img).await { Ok(desc) => desc, Err(e) => format!("Error describing photo: {}", e), }, @@ -3031,6 +3057,7 @@ Return ONLY the summary, nothing else."#, // configured local backend. let embed_text = format!("{} {}", name, description); let embedding: Option> = match crate::ai::embed_document( + cx, &self.ollama, self.llamacpp.as_deref(), &embed_text, @@ -4277,7 +4304,7 @@ Return ONLY the summary, nothing else."#, // the local backend so non-text work stays off OpenRouter. let inlined_visual_description: Option = if !backend.images_inline { match image_base64.as_deref() { - Some(b64) => match backend.local().describe_image(b64).await { + Some(b64) => match backend.local().describe_image(&insight_cx, b64).await { Ok(desc) => { log::info!("{}: vision describe succeeded ({} chars)", kind, desc.len()); Some(desc) @@ -4397,7 +4424,7 @@ Return ONLY the summary, nothing else."#, let (response, prompt_tokens, eval_tokens) = backend .chat() - .chat_with_tools(messages.clone(), tools.clone()) + .chat_with_tools(&loop_cx, messages.clone(), tools.clone()) .await?; last_prompt_eval_count = prompt_tokens; @@ -4466,7 +4493,7 @@ Return ONLY the summary, nothing else."#, ))); let (final_response, prompt_tokens, eval_tokens) = backend .chat() - .chat_with_tools(messages.clone(), tools.clone()) + .chat_with_tools(&loop_cx, messages.clone(), tools.clone()) .await?; last_prompt_eval_count = prompt_tokens; last_eval_count = eval_tokens; @@ -4499,7 +4526,7 @@ Return ONLY the summary, nothing else."#, } let (resp2, pt2, et2) = backend .chat() - .chat_with_tools(messages.clone(), vec![]) + .chat_with_tools(&loop_cx, messages.clone(), vec![]) .await?; last_prompt_eval_count = pt2; last_eval_count = et2; @@ -4637,6 +4664,7 @@ Return ONLY the summary, nothing else."#, tools: Vec, max_iter: usize, ) -> Result { + let loop_cx = opentelemetry::Context::current(); let mut final_content = String::new(); for iteration in 0..max_iter { @@ -4644,7 +4672,7 @@ Return ONLY the summary, nothing else."#, let (response, _prompt_tokens, _eval_tokens) = backend .chat() - .chat_with_tools(messages.clone(), tools.clone()) + .chat_with_tools(&loop_cx, messages.clone(), tools.clone()) .await?; // Sanitize tool call arguments before pushing back into history. @@ -4686,7 +4714,7 @@ Return ONLY the summary, nothing else."#, "", 0, "", - &opentelemetry::Context::new(), + &loop_cx, ) .await; messages.push(ChatMessage::tool_result(result)); @@ -4710,7 +4738,7 @@ Return ONLY the summary, nothing else."#, )); let (final_response, _, _) = backend .chat() - .chat_with_tools(messages.clone(), tools.clone()) + .chat_with_tools(&loop_cx, messages.clone(), tools.clone()) .await?; // Same bounded tool round as the insight loop: execute any last // tool call, then force a tool-less turn for the actual answer. @@ -4728,14 +4756,14 @@ Return ONLY the summary, nothing else."#, "", 0, "", - &opentelemetry::Context::new(), + &loop_cx, ) .await; messages.push(ChatMessage::tool_result(result)); } let (resp2, _, _) = backend .chat() - .chat_with_tools(messages.clone(), vec![]) + .chat_with_tools(&loop_cx, messages.clone(), vec![]) .await?; final_content = resp2.content; } else { diff --git a/src/ai/llamacpp.rs b/src/ai/llamacpp.rs index 77e7f63..5441b20 100644 --- a/src/ai/llamacpp.rs +++ b/src/ai/llamacpp.rs @@ -20,8 +20,11 @@ // insight_generator / insight_chat. #![allow(dead_code)] -use anyhow::{Context, Result, anyhow, bail}; +use anyhow::Context as _; +use anyhow::{Result, anyhow, bail}; use async_trait::async_trait; +use opentelemetry::trace::{Span, Status, TraceContextExt}; +use opentelemetry::{Context, KeyValue}; use reqwest::Client; use serde::Deserialize; use serde_json::{Value, json}; @@ -486,6 +489,7 @@ impl LlamaCppClient { async fn chat_completion_with_model( &self, model: &str, + mut span: Option<&mut opentelemetry::global::BoxedSpan>, messages: Vec, tools: Vec, ) -> Result<(ChatMessage, Option, Option)> { @@ -517,19 +521,41 @@ impl LlamaCppClient { // Wait for any TTS synthesis to release the GPU before the request // timeout starts (see ai::gpu). let _gpu = crate::ai::gpu::llm_lease().await; - let resp = self - .client - .post(&url) - .json(&Value::Object(body)) - .send() - .await - .with_context(|| format!("POST {} failed", url))?; + let mut request = self.client.post(&url).json(&Value::Object(body)); + if let Some(s) = span.as_mut() { + for (k, v) in crate::otel::traceparent_headers( + &Context::new().with_remote_span_context(s.span_context().clone()), + ) { + request = request.header(k, v); + } + } + let resp = match request.send().await { + Ok(r) => r, + Err(e) => { + if let Some(s) = span.as_mut() { + s.set_status(Status::error("llama-swap request failed")); + } + return Err(e).with_context(|| format!("POST {} failed", url)); + } + }; if !resp.status().is_success() { let status = resp.status(); + if let Some(s) = span.as_mut() { + s.set_attribute(KeyValue::new("http.status_code", status.as_u16() as i64)); + s.set_status(Status::error(format!( + "llama-swap chat request failed: {status}" + ))); + } let body = resp.text().await.unwrap_or_default(); bail!("llama-swap chat request failed: {} — {}", status, body); } + if let Some(s) = span.as_mut() { + s.set_attribute(KeyValue::new( + "http.status_code", + resp.status().as_u16() as i64, + )); + } let parsed: Value = resp.json().await.context("parsing chat response")?; let choice = parsed @@ -562,14 +588,35 @@ impl LlamaCppClient { log_timings(&parsed, prompt_tokens, completion_tokens); + if let Some(s) = span.as_mut() { + if let Some(pt) = prompt_tokens { + s.set_attribute(KeyValue::new("llm.tokens.input", pt as i64)); + } + if let Some(ct) = completion_tokens { + s.set_attribute(KeyValue::new("llm.tokens.output", ct as i64)); + } + s.set_status(Status::Ok); + } + Ok((chat_msg, prompt_tokens, completion_tokens)) } + + /// Start the `llm.request` span for an outgoing llama-swap call. + fn start_request_span( + &self, + cx: &Context, + operation: &str, + model: &str, + ) -> opentelemetry::global::BoxedSpan { + crate::otel::start_llm_request_span(cx, "llamacpp", operation, model) + } } #[async_trait] impl LlmClient for LlamaCppClient { async fn generate( &self, + cx: &Context, prompt: &str, system: Option<&str>, images: Option>, @@ -582,12 +629,15 @@ impl LlmClient for LlamaCppClient { user.images = images; messages.push(user); - let (reply, _, _) = self.chat_with_tools(messages, Vec::new()).await?; + // Delegates to chat_with_tools, which starts the llm.request span + // (operation=chat) — one span per HTTP request. + let (reply, _, _) = self.chat_with_tools(cx, messages, Vec::new()).await?; Ok(reply.content) } async fn chat_with_tools( &self, + cx: &Context, messages: Vec, tools: Vec, ) -> Result<(ChatMessage, Option, Option)> { @@ -597,15 +647,25 @@ impl LlmClient for LlamaCppClient { messages.len(), tools.len() ); - self.chat_completion_with_model(&self.primary_model.clone(), messages, tools) - .await + let mut span = self.start_request_span(cx, "chat", &self.primary_model); + self.chat_completion_with_model( + &self.primary_model.clone(), + Some(&mut span), + messages, + tools, + ) + .await } async fn chat_with_tools_stream( &self, + cx: &Context, messages: Vec, tools: Vec, ) -> Result>> { + // The span outlives this method — it moves into the stream + // generator and is finalized when the response drains. + let mut span = self.start_request_span(cx, "chat_stream", &self.primary_model); let url = format!("{}/chat/completions", self.base_url); let roles: Vec<&str> = messages.iter().map(|m| m.role.as_str()).collect(); log::debug!( @@ -639,19 +699,30 @@ impl LlmClient for LlamaCppClient { // timeout starts (see ai::gpu). The guard is moved into the stream // below so the lease spans the whole generation, not just the send. let gpu = crate::ai::gpu::llm_lease().await; - let resp = self - .client - .post(&url) - .json(&Value::Object(body)) + let mut request = self.client.post(&url).json(&Value::Object(body)); + for (k, v) in crate::otel::traceparent_headers( + &Context::new().with_remote_span_context(span.span_context().clone()), + ) { + request = request.header(k, v); + } + let resp = request .send() .await .with_context(|| format!("POST {} failed", url))?; if !resp.status().is_success() { let status = resp.status(); + span.set_attribute(KeyValue::new("http.status_code", status.as_u16() as i64)); + span.set_status(Status::error(format!( + "llama-swap stream request failed: {status}" + ))); let body = resp.text().await.unwrap_or_default(); bail!("llama-swap stream request failed: {} — {}", status, body); } + span.set_attribute(KeyValue::new( + "http.status_code", + resp.status().as_u16() as i64, + )); let byte_stream = resp.bytes_stream(); let stream = async_stream::stream! { @@ -760,6 +831,13 @@ impl LlmClient for LlamaCppClient { tool_calls, images: None, }; + if let Some(pt) = prompt_tokens { + span.set_attribute(KeyValue::new("llm.tokens.input", pt as i64)); + } + if let Some(ct) = completion_tokens { + span.set_attribute(KeyValue::new("llm.tokens.output", ct as i64)); + } + span.set_status(Status::Ok); yield Ok(LlmStreamEvent::Done { message, prompt_eval_count: prompt_tokens, @@ -770,7 +848,8 @@ impl LlmClient for LlamaCppClient { Ok(Box::pin(stream)) } - async fn generate_embeddings(&self, texts: &[&str]) -> Result>> { + async fn generate_embeddings(&self, cx: &Context, texts: &[&str]) -> Result>> { + let mut span = self.start_request_span(cx, "embeddings", &self.embedding_model); // Deliberately NO GPU lease: the embed slot sits in llama-swap's // always-resident group and never participates in a model swap, so // leasing here would only stall searches behind a queued synthesis. @@ -780,19 +859,33 @@ impl LlmClient for LlamaCppClient { "input": texts, }); - let resp = self - .client - .post(&url) - .json(&body) - .send() - .await - .with_context(|| format!("POST {} failed", url))?; + let mut request = self.client.post(&url).json(&body); + for (k, v) in crate::otel::traceparent_headers( + &Context::new().with_remote_span_context(span.span_context().clone()), + ) { + request = request.header(k, v); + } + let resp = match request.send().await { + Ok(r) => r, + Err(e) => { + span.set_status(Status::error("llama-swap embedding request failed")); + return Err(e).with_context(|| format!("POST {} failed", url)); + } + }; if !resp.status().is_success() { let status = resp.status(); + span.set_attribute(KeyValue::new("http.status_code", status.as_u16() as i64)); + span.set_status(Status::error(format!( + "llama-swap embedding request failed: {status}" + ))); let body = resp.text().await.unwrap_or_default(); bail!("llama-swap embedding request failed: {} — {}", status, body); } + span.set_attribute(KeyValue::new( + "http.status_code", + resp.status().as_u16() as i64, + )); #[derive(Deserialize)] struct EmbedResponse { @@ -803,11 +896,20 @@ impl LlmClient for LlamaCppClient { embedding: Vec, } - let parsed: EmbedResponse = resp.json().await.context("parsing embed response")?; + let parsed: EmbedResponse = match resp.json().await { + Ok(p) => p, + Err(e) => { + span.set_status(Status::error("parsing embed response failed")); + return Err(e).context("parsing embed response"); + } + }; + span.set_status(Status::Ok); Ok(parsed.data.into_iter().map(|i| i.embedding).collect()) } - async fn describe_image(&self, image_base64: &str) -> Result { + async fn describe_image(&self, cx: &Context, image_base64: &str) -> Result { + // Own span with operation=describe_image — routes through the vision slot. + let mut span = self.start_request_span(cx, "describe_image", &self.vision_model); 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."; @@ -823,7 +925,12 @@ impl LlmClient for LlamaCppClient { ]; let (reply, _, _) = self - .chat_completion_with_model(&self.vision_model.clone(), messages, Vec::new()) + .chat_completion_with_model( + &self.vision_model.clone(), + Some(&mut span), + messages, + Vec::new(), + ) .await?; Ok(reply.content) } diff --git a/src/ai/llm_client.rs b/src/ai/llm_client.rs index ff33b54..e598e0e 100644 --- a/src/ai/llm_client.rs +++ b/src/ai/llm_client.rs @@ -1,6 +1,7 @@ use anyhow::Result; use async_trait::async_trait; use futures::stream::BoxStream; +use opentelemetry::Context; use serde::{Deserialize, Serialize}; /// Provider-agnostic surface for LLM backends (Ollama, OpenRouter, …). @@ -10,6 +11,12 @@ use serde::{Deserialize, Serialize}; /// when a provider requires it (OpenAI-compatible APIs do), and `images` /// stays as base64 strings here and is rewritten into content-parts where /// needed. +/// +/// Every method that performs an HTTP call takes the caller's tracing +/// `Context` as its first parameter so the impl can start an `llm.request` +/// span parented under the caller's feature span — the codebase threads +/// contexts explicitly rather than relying on ambient `Context::current()` +/// at deep call sites. // First consumer lands in a later PR (OpenRouter impl + hybrid mode routing). #[allow(dead_code)] #[async_trait] @@ -18,6 +25,7 @@ pub trait LlmClient: Send + Sync { /// base64 images (ignored by providers without vision support). async fn generate( &self, + cx: &Context, prompt: &str, system: Option<&str>, images: Option>, @@ -27,6 +35,7 @@ pub trait LlmClient: Send + Sync { /// (which may contain tool_calls) plus optional prompt/eval token counts. async fn chat_with_tools( &self, + cx: &Context, messages: Vec, tools: Vec, ) -> Result<(ChatMessage, Option, Option)>; @@ -39,16 +48,17 @@ pub trait LlmClient: Send + Sync { /// as one `Done` event. async fn chat_with_tools_stream( &self, + cx: &Context, messages: Vec, tools: Vec, ) -> Result>>; /// Batch embedding generation. Dimensionality is provider/model specific. - async fn generate_embeddings(&self, texts: &[&str]) -> Result>>; + async fn generate_embeddings(&self, cx: &Context, texts: &[&str]) -> Result>>; /// One-shot vision description of an image. Used to convert images into /// plain text for the hybrid-mode conversation flow. - async fn describe_image(&self, image_base64: &str) -> Result; + async fn describe_image(&self, cx: &Context, image_base64: &str) -> Result; /// Enumerate available models with their capabilities. async fn list_models(&self) -> Result>; diff --git a/src/ai/local_llm.rs b/src/ai/local_llm.rs index bf3510c..dd5c350 100644 --- a/src/ai/local_llm.rs +++ b/src/ai/local_llm.rs @@ -47,12 +47,24 @@ impl LocalLlm { /// pick query vs document — retrieval models treat the two sides /// differently and an unmarked embed invites prefix-mismatch bugs. pub async fn embed_query(&self, text: &str) -> Result> { - super::embed_query(&self.ollama, self.llamacpp.as_deref(), text).await + super::embed_query( + &opentelemetry::Context::new(), + &self.ollama, + self.llamacpp.as_deref(), + text, + ) + .await } /// Embed corpus text (applies `EMBED_DOCUMENT_PREFIX`). pub async fn embed_document(&self, text: &str) -> Result> { - super::embed_document(&self.ollama, self.llamacpp.as_deref(), text).await + super::embed_document( + &opentelemetry::Context::new(), + &self.ollama, + self.llamacpp.as_deref(), + text, + ) + .await } /// Single-shot local text generation via the `LLM_BACKEND`-selected @@ -60,14 +72,23 @@ impl LocalLlm { pub async fn generate(&self, prompt: &str, system: Option<&str>) -> Result { if super::local_backend_is_llamacpp() { if let Some(lc) = self.llamacpp.as_deref() { - return ::generate(lc, prompt, system, None).await; + return ::generate( + lc, + &opentelemetry::Context::new(), + prompt, + system, + None, + ) + .await; } anyhow::bail!( "LLM_BACKEND=llamacpp but LlamaCppClient is unconfigured — \ set LLAMA_SWAP_URL or switch to LLM_BACKEND=ollama" ); } - self.ollama.generate(prompt, system).await + self.ollama + .generate(&opentelemetry::Context::new(), prompt, system) + .await } /// Label identifying which backend + model produces embeddings right diff --git a/src/ai/mod.rs b/src/ai/mod.rs index d39645a..68b0153 100644 --- a/src/ai/mod.rs +++ b/src/ai/mod.rs @@ -115,24 +115,26 @@ fn embed_prefix(key: &str) -> String { /// `Instruct: \nQuery: `. Must pair with the document prefix the /// corpus was embedded with or similarity degrades. pub async fn embed_query( + cx: &opentelemetry::Context, ollama: &OllamaClient, llamacpp: Option<&LlamaCppClient>, text: &str, ) -> anyhow::Result> { let prefixed = format!("{}{}", embed_prefix("EMBED_QUERY_PREFIX"), text); - embed_one(ollama, llamacpp, &prefixed).await + embed_one(cx, ollama, llamacpp, &prefixed).await } /// Embed corpus text (the stored side of retrieval). Applies /// `EMBED_DOCUMENT_PREFIX` (default empty; nomic v1.5 wants /// `search_document: `, Qwen3-Embedding wants none). pub async fn embed_document( + cx: &opentelemetry::Context, ollama: &OllamaClient, llamacpp: Option<&LlamaCppClient>, text: &str, ) -> anyhow::Result> { let prefixed = format!("{}{}", embed_prefix("EMBED_DOCUMENT_PREFIX"), text); - embed_one(ollama, llamacpp, &prefixed).await + embed_one(cx, ollama, llamacpp, &prefixed).await } /// Embed a batch of strings via the configured local backend. Routes @@ -140,30 +142,32 @@ pub async fn embed_document( /// configured), else Ollama. See [`local_backend_is_llamacpp`] for the /// rationale on consistency. pub async fn embed_many( + cx: &opentelemetry::Context, ollama: &OllamaClient, llamacpp: Option<&LlamaCppClient>, texts: &[&str], ) -> anyhow::Result>> { if local_backend_is_llamacpp() { if let Some(lc) = llamacpp { - return ::generate_embeddings(lc, texts).await; + return ::generate_embeddings(lc, cx, texts).await; } anyhow::bail!( "LLM_BACKEND=llamacpp but LlamaCppClient is unconfigured — \ set LLAMA_SWAP_URL or switch to LLM_BACKEND=ollama" ); } - ollama.generate_embeddings(texts).await + ollama.generate_embeddings(cx, texts).await } /// Embed one string via the configured local backend. Single-text /// convenience over [`embed_many`]. pub async fn embed_one( + cx: &opentelemetry::Context, ollama: &OllamaClient, llamacpp: Option<&LlamaCppClient>, text: &str, ) -> anyhow::Result> { - let mut vecs = embed_many(ollama, llamacpp, &[text]).await?; + let mut vecs = embed_many(cx, ollama, llamacpp, &[text]).await?; vecs.pop() .ok_or_else(|| anyhow::anyhow!("embedding backend returned no embeddings")) } diff --git a/src/ai/nl_query.rs b/src/ai/nl_query.rs index d709322..b4dabb1 100644 --- a/src/ai/nl_query.rs +++ b/src/ai/nl_query.rs @@ -259,10 +259,13 @@ pub async fn translate_nl_query( nl: &str, tag_vocab: &[(i32, String)], today: chrono::NaiveDate, + cx: &opentelemetry::Context, ) -> Result { let system = build_system_prompt(tag_vocab, today); let messages = vec![ChatMessage::system(system), ChatMessage::user(nl)]; - let (msg, _, _) = client.chat_with_tools(messages, Vec::::new()).await?; + let (msg, _, _) = client + .chat_with_tools(cx, messages, Vec::::new()) + .await?; parse_response(&msg.content, tag_vocab) } diff --git a/src/ai/ollama.rs b/src/ai/ollama.rs index 5316208..f338cfe 100644 --- a/src/ai/ollama.rs +++ b/src/ai/ollama.rs @@ -1,6 +1,9 @@ -use anyhow::{Context, Result}; +use anyhow::Context as _; +use anyhow::Result; use async_trait::async_trait; use chrono::NaiveDate; +use opentelemetry::trace::{Span, Status, TraceContextExt}; +use opentelemetry::{Context, KeyValue}; use reqwest::Client; use serde::{Deserialize, Serialize}; use std::collections::HashMap; @@ -9,6 +12,7 @@ use std::sync::{Arc, Mutex}; use std::time::{Duration, Instant}; use crate::ai::llm_client::{LlmClient, LlmStreamEvent}; +use crate::otel::traceparent_headers; use futures::stream::{BoxStream, StreamExt}; // Re-export shared types so existing `crate::ai::ollama::{...}` imports @@ -365,6 +369,7 @@ impl OllamaClient { async fn try_generate( &self, + mut span: Option<&mut opentelemetry::global::BoxedSpan>, url: &str, model: &str, prompt: &str, @@ -382,15 +387,29 @@ impl OllamaClient { think, }; - let response = self + let mut request_builder = self .client .post(format!("{}/api/generate", url)) - .json(&request) - .send() - .await?; + .json(&request); + if let Some(s) = span.as_mut() { + request_builder = self.inject_traceparent(request_builder, s); + } + let response = match request_builder.send().await { + Ok(r) => r, + Err(e) => { + if let Some(s) = span.as_mut() { + s.set_status(Status::error("Ollama request failed")); + } + return Err(e.into()); + } + }; if !response.status().is_success() { let status = response.status(); + if let Some(s) = span.as_mut() { + s.set_attribute(KeyValue::new("http.status_code", status.as_u16() as i64)); + s.set_status(Status::error(format!("Ollama request failed: {status}"))); + } let error_body = response.text().await.unwrap_or_default(); return Err(anyhow::anyhow!( "Ollama request failed: {} - {}", @@ -398,8 +417,31 @@ impl OllamaClient { error_body )); } + if let Some(s) = span.as_mut() { + s.set_attribute(KeyValue::new( + "http.status_code", + response.status().as_u16() as i64, + )); + } - let result: OllamaResponse = response.json().await?; + let result: OllamaResponse = match response.json().await { + Ok(r) => r, + Err(e) => { + if let Some(s) = span.as_mut() { + s.set_status(Status::error("parsing Ollama response failed")); + } + return Err(e.into()); + } + }; + if let Some(s) = span.as_mut() { + if let Some(pt) = result.prompt_eval_count { + s.set_attribute(KeyValue::new("llm.tokens.input", pt as i64)); + } + if let Some(ct) = result.eval_count { + s.set_attribute(KeyValue::new("llm.tokens.output", ct as i64)); + } + s.set_status(Status::Ok); + } log_chat_metrics( result.prompt_eval_count, result.prompt_eval_duration, @@ -409,32 +451,71 @@ impl OllamaClient { Ok(result.response) } - pub async fn generate(&self, prompt: &str, system: Option<&str>) -> Result { - self.generate_with_images(prompt, system, None).await + /// Attach the W3C traceparent header to an outgoing Ollama request so + /// a server that understands it can parent its own spans onto ours. + fn inject_traceparent( + &self, + builder: reqwest::RequestBuilder, + span: &opentelemetry::global::BoxedSpan, + ) -> reqwest::RequestBuilder { + let mut builder = builder; + for (k, v) in traceparent_headers( + &Context::new().with_remote_span_context(span.span_context().clone()), + ) { + builder = builder.header(k, v); + } + builder + } + + /// Start the `llm.request` span for an outgoing Ollama call. + fn start_request_span( + &self, + cx: &Context, + operation: &str, + model: &str, + ) -> opentelemetry::global::BoxedSpan { + crate::otel::start_llm_request_span(cx, "ollama", operation, model) + } + + pub async fn generate( + &self, + cx: &Context, + prompt: &str, + system: Option<&str>, + ) -> Result { + self.generate_with_images(cx, prompt, system, None).await } #[allow(dead_code)] - pub async fn generate_no_think(&self, prompt: &str, system: Option<&str>) -> Result { - self.generate_with_options(prompt, system, None, Some(false)) + pub async fn generate_no_think( + &self, + cx: &Context, + prompt: &str, + system: Option<&str>, + ) -> Result { + self.generate_with_options(cx, prompt, system, None, Some(false), "generate") .await } pub async fn generate_with_images( &self, + cx: &Context, prompt: &str, system: Option<&str>, images: Option>, ) -> Result { - self.generate_with_options(prompt, system, images, None) + self.generate_with_options(cx, prompt, system, images, None, "generate") .await } async fn generate_with_options( &self, + cx: &Context, prompt: &str, system: Option<&str>, images: Option>, think: Option, + operation: &str, ) -> Result { log::debug!("=== Ollama Request ==="); log::debug!("Primary model: {}", self.primary_model); @@ -447,6 +528,10 @@ impl OllamaClient { } log::debug!("====================="); + // Span for the whole generate attempt; the model attribute + // reflects whichever server actually answered. + let mut span = self.start_request_span(cx, operation, &self.primary_model); + // Try primary server first with primary model log::info!( "Attempting to generate with primary server: {} (model: {})", @@ -455,6 +540,7 @@ impl OllamaClient { ); let primary_result = self .try_generate( + Some(&mut span), &self.primary_url, &self.primary_model, prompt, @@ -483,8 +569,10 @@ impl OllamaClient { fallback_url, fallback_model ); + span.set_attribute(KeyValue::new("llm.model", fallback_model.to_string())); match self .try_generate( + Some(&mut span), fallback_url, fallback_model, prompt, @@ -531,6 +619,7 @@ impl OllamaClient { /// Generate a title for a single photo based on its generated summary pub async fn generate_photo_title( &self, + cx: &Context, summary: &str, custom_system: Option<&str>, ) -> Result { @@ -546,7 +635,7 @@ Capture the key moment or theme. Return ONLY the title, nothing else."#, let system = custom_system.unwrap_or("You are my long term memory assistant. Use only the information provided. Do not invent details."); let title = self - .generate_with_images(&prompt, Some(system), None) + .generate_with_images(cx, &prompt, Some(system), None) .await?; // Models decorate despite "Return ONLY the title": quotes, bold // markers, sometimes a "Title:" label. @@ -563,6 +652,7 @@ Capture the key moment or theme. Return ONLY the title, nothing else."#, /// Generate a summary for a single photo based on its context pub async fn generate_photo_summary( &self, + cx: &Context, date: NaiveDate, location: Option<&str>, contact: Option<&str>, @@ -645,21 +735,32 @@ Analyze the image and use specific details from both the visual content and the let system = custom_system.unwrap_or("You are a memory refreshing assistant who is able to provide insights through analyzing past conversations. Use only the information provided. Do not invent details."); let images = image_base64.map(|img| vec![img]); - self.generate_with_images(&prompt, Some(system), images) + self.generate_with_images(cx, &prompt, Some(system), images) .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: &str) -> Result { + pub async fn generate_photo_description( + &self, + cx: &Context, + 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.to_string()]; let description = self - .generate_with_images(prompt, Some(system), Some(images)) + .generate_with_options( + cx, + prompt, + Some(system), + Some(images), + None, + "describe_image", + ) .await?; Ok(description.trim().to_string()) @@ -672,9 +773,11 @@ Analyze the image and use specific details from both the visual content and the /// iteration of the agent loop. pub async fn chat_with_tools( &self, + cx: &Context, messages: Vec, tools: Vec, ) -> Result<(ChatMessage, Option, Option)> { + let mut span = self.start_request_span(cx, "chat", &self.primary_model); let order = self.attempt_order(); let mut errors: Vec = Vec::new(); @@ -685,8 +788,11 @@ Analyze the image and use specific details from both the visual content and the url, model ); + if *label == "fallback" { + span.set_attribute(KeyValue::new("llm.model", model.to_string())); + } match self - .try_chat_with_tools(url, messages.clone(), tools.clone()) + .try_chat_with_tools(Some(&mut span), url, messages.clone(), tools.clone()) .await { Ok(result) => { @@ -728,18 +834,22 @@ Analyze the image and use specific details from both the visual content and the /// any, live on the final message). pub async fn chat_with_tools_stream( &self, + cx: &Context, messages: Vec, tools: Vec, ) -> Result>> { // Same preference logic as `chat_with_tools`. Only the initial // connection is retried across servers — once the stream begins, - // mid-stream errors propagate to the caller. + // mid-stream errors propagate to the caller. Each attempt gets its + // own `llm.request` span, created inside + // `try_chat_with_tools_stream` so the winning attempt's span moves + // into the stream and is finalized when the response drains. let order = self.attempt_order(); let mut last_err: Option = None; for (label, url, _model) in &order { match self - .try_chat_with_tools_stream(url, messages.clone(), tools.clone()) + .try_chat_with_tools_stream(cx, url, messages.clone(), tools.clone()) .await { Ok(s) => { @@ -759,6 +869,7 @@ Analyze the image and use specific details from both the visual content and the async fn try_chat_with_tools_stream( &self, + cx: &Context, base_url: &str, messages: Vec, tools: Vec, @@ -773,6 +884,11 @@ Analyze the image and use specific details from both the visual content and the }; let options = self.build_options(); + // The span outlives this method — the stream yields until the + // response fully drains, so the owned span moves into the + // generator and is finalized there. + let mut span = self.start_request_span(cx, "chat_stream", model); + let request_body = OllamaChatRequest { model, messages: &messages, @@ -781,16 +897,25 @@ Analyze the image and use specific details from both the visual content and the options, }; - let response = self - .client - .post(&url) - .json(&request_body) - .send() - .await - .with_context(|| format!("Failed to connect to Ollama at {}", url))?; + let response = { + let mut rb = self.client.post(&url).json(&request_body); + rb = self.inject_traceparent(rb, &span); + match rb.send().await { + Ok(r) => r, + Err(e) => { + span.set_status(Status::error("Ollama stream connect failed")); + return Err(e) + .with_context(|| format!("Failed to connect to Ollama at {}", url)); + } + } + }; if !response.status().is_success() { let status = response.status(); + span.set_attribute(KeyValue::new("http.status_code", status.as_u16() as i64)); + span.set_status(Status::error(format!( + "Ollama stream request failed: {status}" + ))); let body = response.text().await.unwrap_or_default(); anyhow::bail!( "Ollama stream request failed with status {}: {}", @@ -798,6 +923,10 @@ Analyze the image and use specific details from both the visual content and the body ); } + span.set_attribute(KeyValue::new( + "http.status_code", + response.status().as_u16() as i64, + )); // Ollama streams NDJSON: each line is a full `OllamaStreamChunk`. // We buffer partial lines across chunks from the byte stream. @@ -879,6 +1008,13 @@ Analyze the image and use specific details from both the visual content and the eval_count, eval_duration, ); + if let Some(pt) = prompt_eval_count { + span.set_attribute(KeyValue::new("llm.tokens.input", pt as i64)); + } + if let Some(ct) = eval_count { + span.set_attribute(KeyValue::new("llm.tokens.output", ct as i64)); + } + span.set_status(Status::Ok); let message = ChatMessage { role, content: accumulated, @@ -897,6 +1033,7 @@ Analyze the image and use specific details from both the visual content and the async fn try_chat_with_tools( &self, + mut span: Option<&mut opentelemetry::global::BoxedSpan>, base_url: &str, messages: Vec, tools: Vec, @@ -924,16 +1061,31 @@ Analyze the image and use specific details from both the visual content and the .unwrap_or_else(|e| format!("", e)); log::debug!("chat_with_tools request body: {}", request_json); - let response = self - .client - .post(&url) - .json(&request_body) - .send() - .await - .with_context(|| format!("Failed to connect to Ollama at {}", url))?; + let response = { + let mut rb = self.client.post(&url).json(&request_body); + if let Some(s) = span.as_mut() { + rb = self.inject_traceparent(rb, s); + } + match rb.send().await { + Ok(r) => r, + Err(e) => { + if let Some(s) = span.as_mut() { + s.set_status(Status::error("Ollama chat connect failed")); + } + return Err(e) + .with_context(|| format!("Failed to connect to Ollama at {}", url)); + } + } + }; if !response.status().is_success() { let status = response.status(); + if let Some(s) = span.as_mut() { + s.set_attribute(KeyValue::new("http.status_code", status.as_u16() as i64)); + s.set_status(Status::error(format!( + "Ollama chat request failed: {status}" + ))); + } let body = response.text().await.unwrap_or_default(); // warn, not error — the outer `chat_with_tools` may recover via // the fallback server. When both fail, the outer layer emits the @@ -950,11 +1102,31 @@ Analyze the image and use specific details from both the visual content and the body ); } + if let Some(s) = span.as_mut() { + s.set_attribute(KeyValue::new( + "http.status_code", + response.status().as_u16() as i64, + )); + } - let chat_response: OllamaChatResponse = response - .json() - .await - .with_context(|| "Failed to parse Ollama chat response")?; + let chat_response: OllamaChatResponse = match response.json().await { + Ok(r) => r, + Err(e) => { + if let Some(s) = span.as_mut() { + s.set_status(Status::error("parsing Ollama chat response failed")); + } + return Err(e).with_context(|| "Failed to parse Ollama chat response"); + } + }; + if let Some(s) = span.as_mut() { + if let Some(pt) = chat_response.prompt_eval_count { + s.set_attribute(KeyValue::new("llm.tokens.input", pt as i64)); + } + if let Some(ct) = chat_response.eval_count { + s.set_attribute(KeyValue::new("llm.tokens.output", ct as i64)); + } + s.set_status(Status::Ok); + } // Log performance counters returned by Ollama. Durations are // reported in nanoseconds; we render ms + tokens/sec for skim-ability @@ -976,8 +1148,8 @@ Analyze the image and use specific details from both the visual content and the /// 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> { - let embeddings = self.generate_embeddings(&[text]).await?; + pub async fn generate_embedding(&self, cx: &Context, text: &str) -> Result> { + let embeddings = self.generate_embeddings(cx, &[text]).await?; embeddings .into_iter() .next() @@ -987,9 +1159,11 @@ Analyze the image and use specific details from both the visual content and the /// Generate embeddings for multiple texts in a single API call (batch mode) /// Returns a vector of 768-dimensional vectors /// This is much more efficient than calling generate_embedding multiple times - pub async fn generate_embeddings(&self, texts: &[&str]) -> Result>> { + pub async fn generate_embeddings(&self, cx: &Context, texts: &[&str]) -> Result>> { let embedding_model = EMBEDDING_MODEL; + let mut span = self.start_request_span(cx, "embeddings", embedding_model); + log::debug!("=== Ollama Batch Embedding Request ==="); log::debug!("Model: {}", embedding_model); log::debug!("Batch size: {} texts", texts.len()); @@ -1003,7 +1177,7 @@ Analyze the image and use specific details from both the visual content and the embedding_model ); let primary_result = self - .try_generate_embeddings(&self.primary_url, embedding_model, texts) + .try_generate_embeddings(Some(&mut span), &self.primary_url, embedding_model, texts) .await; let embeddings = match primary_result { @@ -1026,7 +1200,12 @@ Analyze the image and use specific details from both the visual content and the embedding_model ); match self - .try_generate_embeddings(fallback_url, embedding_model, texts) + .try_generate_embeddings( + Some(&mut span), + fallback_url, + embedding_model, + texts, + ) .await { Ok(embeddings) => { @@ -1073,6 +1252,7 @@ Analyze the image and use specific details from both the visual content and the /// Internal helper to try generating embeddings for multiple texts from a specific server async fn try_generate_embeddings( &self, + mut span: Option<&mut opentelemetry::global::BoxedSpan>, url: &str, model: &str, texts: &[&str], @@ -1082,15 +1262,33 @@ Analyze the image and use specific details from both the visual content and the input: texts.iter().map(|s| s.to_string()).collect(), }; - let response = self - .client - .post(format!("{}/api/embed", url)) - .json(&request) - .send() - .await?; + let response = { + let mut rb = self + .client + .post(format!("{}/api/embed", url)) + .json(&request); + if let Some(s) = span.as_mut() { + rb = self.inject_traceparent(rb, s); + } + match rb.send().await { + Ok(r) => r, + Err(e) => { + if let Some(s) = span.as_mut() { + s.set_status(Status::error("Ollama embedding request failed")); + } + return Err(e.into()); + } + } + }; if !response.status().is_success() { let status = response.status(); + if let Some(s) = span.as_mut() { + s.set_attribute(KeyValue::new("http.status_code", status.as_u16() as i64)); + s.set_status(Status::error(format!( + "Ollama batch embedding request failed: {status}" + ))); + } let error_body = response.text().await.unwrap_or_default(); return Err(anyhow::anyhow!( "Ollama batch embedding request failed: {} - {}", @@ -1098,8 +1296,25 @@ Analyze the image and use specific details from both the visual content and the error_body )); } + if let Some(s) = span.as_mut() { + s.set_attribute(KeyValue::new( + "http.status_code", + response.status().as_u16() as i64, + )); + } - let result: OllamaEmbedResponse = response.json().await?; + let result: OllamaEmbedResponse = match response.json().await { + Ok(r) => r, + Err(e) => { + if let Some(s) = span.as_mut() { + s.set_status(Status::error("parsing Ollama embed response failed")); + } + return Err(e.into()); + } + }; + if let Some(s) = span.as_mut() { + s.set_status(Status::Ok); + } Ok(result.embeddings) } } @@ -1108,35 +1323,38 @@ Analyze the image and use specific details from both the visual content and the impl LlmClient for OllamaClient { async fn generate( &self, + cx: &Context, prompt: &str, system: Option<&str>, images: Option>, ) -> Result { - self.generate_with_images(prompt, system, images).await + self.generate_with_images(cx, prompt, system, images).await } async fn chat_with_tools( &self, + cx: &Context, messages: Vec, tools: Vec, ) -> Result<(ChatMessage, Option, Option)> { - OllamaClient::chat_with_tools(self, messages, tools).await + OllamaClient::chat_with_tools(self, cx, messages, tools).await } async fn chat_with_tools_stream( &self, + cx: &Context, messages: Vec, tools: Vec, ) -> Result>> { - OllamaClient::chat_with_tools_stream(self, messages, tools).await + OllamaClient::chat_with_tools_stream(self, cx, messages, tools).await } - async fn generate_embeddings(&self, texts: &[&str]) -> Result>> { - OllamaClient::generate_embeddings(self, texts).await + async fn generate_embeddings(&self, cx: &Context, texts: &[&str]) -> Result>> { + OllamaClient::generate_embeddings(self, cx, texts).await } - async fn describe_image(&self, image_base64: &str) -> Result { - self.generate_photo_description(image_base64).await + async fn describe_image(&self, cx: &Context, image_base64: &str) -> Result { + self.generate_photo_description(cx, image_base64).await } async fn list_models(&self) -> Result> { diff --git a/src/ai/openrouter.rs b/src/ai/openrouter.rs index 5ec420f..fdc7935 100644 --- a/src/ai/openrouter.rs +++ b/src/ai/openrouter.rs @@ -2,8 +2,11 @@ // the translation helpers directly. #![allow(dead_code)] -use anyhow::{Context, Result, anyhow, bail}; +use anyhow::Context as _; +use anyhow::{Result, anyhow, bail}; use async_trait::async_trait; +use opentelemetry::trace::{Span, Status, TraceContextExt}; +use opentelemetry::{Context, KeyValue}; use reqwest::Client; use serde::Deserialize; use serde_json::{Value, json}; @@ -14,6 +17,7 @@ use std::time::{Duration, Instant}; use crate::ai::llm_client::{ ChatMessage, LlmClient, LlmStreamEvent, ModelCapabilities, Tool, ToolCall, ToolCallFunction, }; + use futures::stream::{BoxStream, StreamExt}; const DEFAULT_BASE_URL: &str = "https://openrouter.ai/api/v1"; @@ -144,6 +148,19 @@ impl OpenRouterClient { b } + /// Start the `llm.request` span for an outgoing call. `operation` is + /// `chat`, `embeddings`, or `describe` — the call shape, not the entry + /// point (a `describe_image` funneled through `chat_with_tools` is + /// still a `describe`). + fn start_request_span( + &self, + cx: &Context, + operation: &str, + model: &str, + ) -> opentelemetry::global::BoxedSpan { + crate::otel::start_llm_request_span(cx, "openrouter", operation, model) + } + /// Translate canonical messages to the OpenAI-compatible wire shape. /// /// Walks in order so it can attach `tool_call_id` to `role=tool` messages @@ -312,6 +329,7 @@ impl OpenRouterClient { impl LlmClient for OpenRouterClient { async fn generate( &self, + cx: &Context, prompt: &str, system: Option<&str>, images: Option>, @@ -324,90 +342,34 @@ impl LlmClient for OpenRouterClient { user.images = images; messages.push(user); - let (reply, _, _) = self.chat_with_tools(messages, Vec::new()).await?; + // Delegates to chat_with_tools, which starts the llm.request span + // (operation=chat) — one span per HTTP request. + let (reply, _, _) = self.chat_with_tools(cx, messages, Vec::new()).await?; Ok(reply.content) } async fn chat_with_tools( &self, + cx: &Context, messages: Vec, tools: Vec, ) -> Result<(ChatMessage, Option, Option)> { - let url = format!("{}/chat/completions", self.base_url); - let mut body = serde_json::Map::new(); - body.insert("model".into(), Value::String(self.primary_model.clone())); - body.insert( - "messages".into(), - Value::Array(Self::messages_to_openai(&messages)), - ); - body.insert("stream".into(), Value::Bool(false)); - if !tools.is_empty() { - body.insert( - "tools".into(), - serde_json::to_value(&tools).context("serializing tools")?, - ); - } - for (k, v) in self.build_options() { - body.insert(k.into(), v); - } - - log::info!( - "OpenRouter chat_with_tools: model={} messages={} tools={}", - self.primary_model, - messages.len(), - tools.len() - ); - - let resp = self - .authed(self.client.post(&url)) - .json(&Value::Object(body)) - .send() + let mut span = self.start_request_span(cx, "chat", &self.primary_model); + self.chat_with_tools_impl(Some(&mut span), messages, tools) .await - .with_context(|| format!("POST {} failed", url))?; - - if !resp.status().is_success() { - let status = resp.status(); - let body = resp.text().await.unwrap_or_default(); - bail!("OpenRouter chat request failed: {} — {}", status, body); - } - - let parsed: Value = resp.json().await.context("parsing chat response")?; - let choice = parsed - .get("choices") - .and_then(|v| v.as_array()) - .and_then(|a| a.first()) - .ok_or_else(|| { - anyhow!( - "response missing choices[0]: {}", - extract_openrouter_error_detail(&parsed) - ) - })?; - let msg = choice.get("message").ok_or_else(|| { - anyhow!( - "choices[0] missing message: {}", - extract_openrouter_error_detail(&parsed) - ) - })?; - let chat_msg = Self::openai_message_to_chat(msg)?; - - let usage = parsed.get("usage"); - let prompt_tokens = usage - .and_then(|u| u.get("prompt_tokens")) - .and_then(|v| v.as_i64()) - .map(|n| n as i32); - let completion_tokens = usage - .and_then(|u| u.get("completion_tokens")) - .and_then(|v| v.as_i64()) - .map(|n| n as i32); - - Ok((chat_msg, prompt_tokens, completion_tokens)) } async fn chat_with_tools_stream( &self, + cx: &Context, messages: Vec, tools: Vec, ) -> Result>> { + // The span outlives this method — the stream yields until the + // response fully drains, so the owned span moves into the + // generator and is finalized there. + let mut span = self.start_request_span(cx, "chat_stream", &self.primary_model); + let url = format!("{}/chat/completions", self.base_url); let mut body = serde_json::Map::new(); body.insert("model".into(), Value::String(self.primary_model.clone())); @@ -432,18 +394,32 @@ impl LlmClient for OpenRouterClient { body.insert(k.into(), v); } - let resp = self + let mut request = self .authed(self.client.post(&url)) - .json(&Value::Object(body)) + .json(&Value::Object(body)); + for (k, v) in crate::otel::traceparent_headers( + &Context::new().with_remote_span_context(span.span_context().clone()), + ) { + request = request.header(k, v); + } + let resp = request .send() .await .with_context(|| format!("POST {} failed", url))?; if !resp.status().is_success() { let status = resp.status(); + span.set_attribute(KeyValue::new("http.status_code", status.as_u16() as i64)); + span.set_status(Status::error(format!( + "OpenRouter stream request failed: {status}" + ))); let body = resp.text().await.unwrap_or_default(); bail!("OpenRouter stream request failed: {} — {}", status, body); } + span.set_attribute(KeyValue::new( + "http.status_code", + resp.status().as_u16() as i64, + )); // OpenAI-compat SSE stream. Each event is `data: \n\n`, with // `data: [DONE]` signalling completion. Tool calls arrive as @@ -467,6 +443,7 @@ impl LlmClient for OpenRouterClient { let chunk = match chunk { Ok(b) => b, Err(e) => { + span.set_status(Status::error(format!("stream read failed: {e}"))); yield Err(anyhow!("stream read failed: {}", e)); return; } @@ -517,6 +494,9 @@ impl LlmClient for OpenRouterClient { .get("completion_tokens") .and_then(|n| n.as_i64()) .map(|n| n as i32); + if let Some(cost) = usage.get("cost").and_then(|c| c.as_f64()) { + span.set_attribute(KeyValue::new("llm.cost_usd", cost)); + } } let Some(choices) = v.get("choices").and_then(|c| c.as_array()) @@ -606,6 +586,13 @@ impl LlmClient for OpenRouterClient { tool_calls, images: None, }; + if let Some(pt) = prompt_tokens { + span.set_attribute(KeyValue::new("llm.tokens.input", pt as i64)); + } + if let Some(ct) = completion_tokens { + span.set_attribute(KeyValue::new("llm.tokens.output", ct as i64)); + } + span.set_status(Status::Ok); yield Ok(LlmStreamEvent::Done { message, prompt_eval_count: prompt_tokens, @@ -616,25 +603,41 @@ impl LlmClient for OpenRouterClient { Ok(Box::pin(stream)) } - async fn generate_embeddings(&self, texts: &[&str]) -> Result>> { + async fn generate_embeddings(&self, cx: &Context, texts: &[&str]) -> Result>> { + let mut span = self.start_request_span(cx, "embeddings", &self.embedding_model); let url = format!("{}/embeddings", self.base_url); let body = json!({ "model": self.embedding_model, "input": texts, }); - let resp = self - .authed(self.client.post(&url)) - .json(&body) + let mut request = self.authed(self.client.post(&url)).json(&body); + for (k, v) in crate::otel::traceparent_headers( + &Context::new().with_remote_span_context(span.span_context().clone()), + ) { + request = request.header(k, v); + } + let resp = request .send() .await - .with_context(|| format!("POST {} failed", url))?; + .with_context(|| format!("POST {} failed", url)) + .inspect_err(|_| { + span.set_status(Status::error("OpenRouter embedding request failed")); + })?; if !resp.status().is_success() { let status = resp.status(); + span.set_attribute(KeyValue::new("http.status_code", status.as_u16() as i64)); + span.set_status(Status::error(format!( + "OpenRouter embedding request failed: {status}" + ))); let body = resp.text().await.unwrap_or_default(); bail!("OpenRouter embedding request failed: {} — {}", status, body); } + span.set_attribute(KeyValue::new( + "http.status_code", + resp.status().as_u16() as i64, + )); #[derive(Deserialize)] struct EmbedResponse { @@ -646,18 +649,28 @@ impl LlmClient for OpenRouterClient { } let parsed: EmbedResponse = resp.json().await.context("parsing embed response")?; + span.set_status(Status::Ok); Ok(parsed.data.into_iter().map(|i| i.embedding).collect()) } - async fn describe_image(&self, image_base64: &str) -> Result { + async fn describe_image(&self, cx: &Context, image_base64: &str) -> Result { + // Own span with operation=describe_image — the funnel into + // chat_with_tools_impl keeps the request on a single span. + let mut span = self.start_request_span(cx, "describe_image", &self.primary_model); let prompt = "Briefly describe what you see in this image in 1-2 sentences. \ Focus on the people, location, and activity."; - self.generate( - prompt, - Some("You are a scene description assistant. Be concise and factual."), - Some(vec![image_base64.to_string()]), - ) - .await + let mut messages: Vec = Vec::new(); + messages.push(ChatMessage::system( + "You are a scene description assistant. Be concise and factual.", + )); + let mut user = ChatMessage::user(prompt); + user.images = Some(vec![image_base64.to_string()]); + messages.push(user); + + let (reply, _, _) = self + .chat_with_tools_impl(Some(&mut span), messages, Vec::new()) + .await?; + Ok(reply.content) } async fn list_models(&self) -> Result> { @@ -711,6 +724,134 @@ impl LlmClient for OpenRouterClient { } } +impl OpenRouterClient { + /// HTTP core of `chat_with_tools`. `span` is the `llm.request` span + /// started by the caller (or a funnelling entry point like + /// `describe_image`) so attributes from the response land on the right + /// operation label. + async fn chat_with_tools_impl( + &self, + mut span: Option<&mut opentelemetry::global::BoxedSpan>, + messages: Vec, + tools: Vec, + ) -> Result<(ChatMessage, Option, Option)> { + let url = format!("{}/chat/completions", self.base_url); + let mut body = serde_json::Map::new(); + body.insert("model".into(), Value::String(self.primary_model.clone())); + body.insert( + "messages".into(), + Value::Array(Self::messages_to_openai(&messages)), + ); + body.insert("stream".into(), Value::Bool(false)); + if !tools.is_empty() { + body.insert( + "tools".into(), + serde_json::to_value(&tools).context("serializing tools")?, + ); + } + for (k, v) in self.build_options() { + body.insert(k.into(), v); + } + + log::info!( + "OpenRouter chat_with_tools: model={} messages={} tools={}", + self.primary_model, + messages.len(), + tools.len() + ); + + let mut request = self + .authed(self.client.post(&url)) + .json(&Value::Object(body)); + if let Some(s) = span.as_mut() { + for (k, v) in crate::otel::traceparent_headers( + &Context::new().with_remote_span_context(s.span_context().clone()), + ) { + request = request.header(k, v); + } + } + let resp = match request.send().await { + Ok(r) => r, + Err(e) => { + if let Some(s) = span.as_mut() { + s.set_status(Status::error("OpenRouter request failed")); + } + return Err(e).with_context(|| format!("POST {} failed", url)); + } + }; + + if !resp.status().is_success() { + let status = resp.status(); + if let Some(s) = span.as_mut() { + s.set_attribute(KeyValue::new("http.status_code", status.as_u16() as i64)); + s.set_status(Status::error(format!( + "OpenRouter chat request failed: {status}" + ))); + } + let body = resp.text().await.unwrap_or_default(); + bail!("OpenRouter chat request failed: {} — {}", status, body); + } + if let Some(s) = span.as_mut() { + s.set_attribute(KeyValue::new( + "http.status_code", + resp.status().as_u16() as i64, + )); + } + + let parsed: Value = match resp.json().await { + Ok(v) => v, + Err(e) => { + if let Some(s) = span.as_mut() { + s.set_status(Status::error("parsing chat response failed")); + } + return Err(e).context("parsing chat response"); + } + }; + let choice = parsed + .get("choices") + .and_then(|v| v.as_array()) + .and_then(|a| a.first()) + .ok_or_else(|| { + anyhow!( + "response missing choices[0]: {}", + extract_openrouter_error_detail(&parsed) + ) + })?; + let msg = choice.get("message").ok_or_else(|| { + anyhow!( + "choices[0] missing message: {}", + extract_openrouter_error_detail(&parsed) + ) + })?; + let chat_msg = Self::openai_message_to_chat(msg)?; + + let usage = parsed.get("usage"); + let prompt_tokens = usage + .and_then(|u| u.get("prompt_tokens")) + .and_then(|v| v.as_i64()) + .map(|n| n as i32); + let completion_tokens = usage + .and_then(|u| u.get("completion_tokens")) + .and_then(|v| v.as_i64()) + .map(|n| n as i32); + // OpenRouter reports per-request USD cost in `usage.cost`. + if let Some(s) = span.as_mut() { + if let Some(pt) = prompt_tokens { + s.set_attribute(KeyValue::new("llm.tokens.input", pt as i64)); + } + if let Some(ct) = completion_tokens { + s.set_attribute(KeyValue::new("llm.tokens.output", ct as i64)); + } + if let Some(cost) = usage.and_then(|u| u.get("cost")).and_then(|c| c.as_f64()) { + s.set_attribute(KeyValue::new("llm.cost_usd", cost)); + } + s.set_status(Status::Ok); + } + + Ok((chat_msg, prompt_tokens, completion_tokens)) + } +} + /// Extract a diagnostic fragment from an OpenRouter response body that /// doesn't match the expected `{choices: [...]}` shape. OpenRouter will /// sometimes return 200 OK with `{"error": {"message": "...", "code": ...}}` diff --git a/src/ai/persona_chat.rs b/src/ai/persona_chat.rs index 596a809..f9ff000 100644 --- a/src/ai/persona_chat.rs +++ b/src/ai/persona_chat.rs @@ -26,7 +26,7 @@ use actix_web::{HttpRequest, HttpResponse, Responder, delete, get, post, web}; use anyhow::{Context, Result, anyhow, bail}; use chrono::Utc; use opentelemetry::KeyValue; -use opentelemetry::trace::{Span, Status, Tracer}; +use opentelemetry::trace::{Span, Status, TraceContextExt, Tracer}; use serde::{Deserialize, Serialize}; use std::collections::HashMap; use std::sync::{Arc, Mutex as StdMutex}; @@ -685,6 +685,7 @@ impl PersonaChatSession { entry_clone.clone(), user_id, library_id, + &mut span, ) .await; match result { @@ -721,6 +722,7 @@ impl PersonaChatSession { entry: Arc, user_id: i32, _library_id: i32, + turn_span: &mut opentelemetry::global::BoxedSpan, ) -> Result<()> { // Per-conversation mutex — serializes this turn against a // concurrent rewind or branch switch on the same transcript. @@ -808,6 +810,12 @@ impl PersonaChatSession { .resolve_backend(backend_kind, &overrides) .await?; let model_used = backend.model().to_string(); + // llm.* attributes on the turn-level span (issue #130), plus the + // parent context for the child llm.request spans. + turn_span.set_attribute(KeyValue::new("llm.backend", backend.kind.as_str())); + turn_span.set_attribute(KeyValue::new("llm.model", model_used.clone())); + let turn_cx = opentelemetry::Context::new() + .with_remote_span_context(turn_span.span_context().clone()); // Build a no-photo tool catalog. `current_gate_opts_for_persona` // already probes each tool's backing table for presence and @@ -874,6 +882,7 @@ impl PersonaChatSession { &persona.persona_id, max_iterations, &entry, + &turn_cx, ) .await .map_err(|e| anyhow!("persona chat loop failed: {e:?}"))?; @@ -942,7 +951,7 @@ impl PersonaChatSession { // list falls back to the persona name while the title is empty. if needs_title { let title = self - .generate_title(&backend, &messages) + .generate_title(&backend, &messages, &turn_cx) .await .unwrap_or_else(|| fallback_title(&messages)); if !title.is_empty() { @@ -1047,6 +1056,7 @@ impl PersonaChatSession { &self, backend: &crate::ai::backend::ResolvedBackend, messages: &[ChatMessage], + cx: &opentelemetry::Context, ) -> Option { let user_message = messages.iter().find(|m| m.role == "user")?; // The last assistant message with actual prose — earlier ones may be @@ -1057,7 +1067,7 @@ impl PersonaChatSession { .find(|m| m.role == "assistant" && !m.content.trim().is_empty())?; let prompt = title_prompt(&user_message.content, &reply.content); - match backend.chat().generate(&prompt, None, None).await { + match backend.chat().generate(cx, &prompt, None, None).await { Ok(raw) => { let title = sanitize_title(&crate::ai::llm_client::strip_think_blocks(&raw)); if title.is_empty() { None } else { Some(title) } diff --git a/src/ai/sms_client.rs b/src/ai/sms_client.rs index d5e175f..02c39f1 100644 --- a/src/ai/sms_client.rs +++ b/src/ai/sms_client.rs @@ -324,6 +324,7 @@ impl SmsApiClient { &self, messages: &[SmsMessage], ollama: &OllamaClient, + cx: &opentelemetry::Context, ) -> Result { if messages.is_empty() { return Ok(String::from("No messages on this day")); @@ -356,6 +357,7 @@ Summary:"#, ollama .generate( + cx, &prompt, // Some("You are a summarizer for the purposes of jogging my memory and highlighting events and situations."), Some("You are the keeper of memories, ingest the context and give me a casual summary of the moment."), diff --git a/src/bin/test_daily_summary.rs b/src/bin/test_daily_summary.rs index d04aa32..f497db5 100644 --- a/src/bin/test_daily_summary.rs +++ b/src/bin/test_daily_summary.rs @@ -220,7 +220,9 @@ async fn main() -> Result<()> { println!("Generating summary..."); - let summary = ollama.generate(&prompt, Some(system_prompt)).await?; + let summary = ollama + .generate(&opentelemetry::Context::new(), &prompt, Some(system_prompt)) + .await?; println!("\n📝 GENERATED SUMMARY:"); println!("─────────────────────────────────────────"); @@ -236,7 +238,9 @@ async fn main() -> Result<()> { ); println!("\nGenerating embedding..."); - let embedding = ollama.generate_embedding(&stripped).await?; + let embedding = ollama + .generate_embedding(&opentelemetry::Context::new(), &stripped) + .await?; println!("✓ Embedding generated ({} dimensions)", embedding.len()); println!("Saving to database..."); diff --git a/src/knowledge.rs b/src/knowledge.rs index 2c6cf6f..0783c36 100644 --- a/src/knowledge.rs +++ b/src/knowledge.rs @@ -859,7 +859,8 @@ async fn synthesize_merge( let ollama = app_state.ollama.clone(); let model_used = ollama.primary_model.clone(); - let proposed = match ollama.generate(&prompt, Some(system)).await { + let cx = opentelemetry::Context::current(); + let proposed = match ollama.generate(&cx, &prompt, Some(system)).await { Ok(out) => { // Strip the framing models reach for even with explicit // "no preamble" guidance: leading "Merged description:" diff --git a/src/otel.rs b/src/otel.rs index 110a2d1..20b319e 100644 --- a/src/otel.rs +++ b/src/otel.rs @@ -2,7 +2,7 @@ use actix_web::HttpRequest; use actix_web::http::header::HeaderMap; use opentelemetry::global::{BoxedSpan, BoxedTracer}; use opentelemetry::propagation::TextMapPropagator; -use opentelemetry::trace::{Span, Status, Tracer}; +use opentelemetry::trace::{Span, SpanBuilder, SpanKind, Status, TraceContextExt, Tracer}; use opentelemetry::{Context, KeyValue, global}; use opentelemetry_appender_log::OpenTelemetryLogBridge; use opentelemetry_otlp::WithExportConfig; @@ -77,11 +77,46 @@ impl<'a> opentelemetry::propagation::Extractor for HeaderExtractor<'a> { self.0.keys().map(|k| k.as_str()).collect() } } +/// Start a client span for a single LLM call. Every provider funnels its +/// LlmClient methods through this so the span shape (name, `SpanKind::Client`, +/// provider / operation / model attributes) is identical across +/// openrouter / llamacpp / ollama. `operation` is one of `chat`, +/// `chat_stream`, `embeddings`, `describe_image`, `generate`. +pub fn start_llm_request_span( + cx: &Context, + provider: &str, + operation: &str, + model: &str, +) -> BoxedSpan { + let builder = SpanBuilder::from_name("llm.request").with_kind(SpanKind::Client); + let mut span = global_tracer().build_with_context(builder, cx); + span.set_attribute(KeyValue::new("llm.provider", provider.to_string())); + span.set_attribute(KeyValue::new("llm.operation", operation.to_string())); + span.set_attribute(KeyValue::new("llm.model", model.to_string())); + span +} + pub fn extract_context_from_request(req: &HttpRequest) -> Context { let propagator = TraceContextPropagator::new(); propagator.extract(&HeaderExtractor(req.headers())) } +/// Build the W3C trace-context headers to attach to an outgoing HTTP +/// request so a downstream service (llama-swap, Ollama) can parent its own +/// spans onto this trace. Returns an empty vec when the context carries no +/// valid span — e.g. debug builds, where the global tracer is a no-op and +/// injecting an all-zero traceparent would produce garbage parent spans. +pub fn traceparent_headers(context: &Context) -> Vec<(String, String)> { + if !context.span().span_context().is_valid() { + return Vec::new(); + } + let mut headers: std::collections::HashMap = std::collections::HashMap::new(); + TraceContextPropagator::new().inject_context(context, &mut headers); + // The propagator writes an empty tracestate header when there is no + // state — drop empty-valued headers rather than sending noise. + headers.into_iter().filter(|(_, v)| !v.is_empty()).collect() +} + pub fn trace_db_call( context: &Context, query_type: &str, @@ -110,3 +145,59 @@ where result } + +#[cfg(test)] +mod traceparent_tests { + use super::*; + use opentelemetry::trace::{ + SpanContext, SpanId, TraceContextExt, TraceFlags, TraceId, TraceState, + }; + + /// A context carrying a span context with known ids — no global + /// provider needed, so tests can't pollute process state. + fn context_with_flags(flags: TraceFlags) -> Context { + let trace_id = TraceId::from_bytes([ + 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, + 0x0f, 0x10, + ]); + let span_id = SpanId::from_bytes([0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08]); + let span_context = SpanContext::new(trace_id, span_id, flags, false, TraceState::default()); + Context::new().with_remote_span_context(span_context) + } + + #[test] + fn traceparent_headers_sampled_context() { + let cx = context_with_flags(TraceFlags::SAMPLED); + let headers = traceparent_headers(&cx); + assert_eq!( + headers, + vec![( + "traceparent".to_string(), + "00-0102030405060708090a0b0c0d0e0f10-0102030405060708-01".to_string(), + )] + ); + } + + #[test] + fn traceparent_headers_not_sampled_context() { + // A valid-but-unsampled context still propagates; the flags byte + // carries the state. + let cx = context_with_flags(TraceFlags::NOT_SAMPLED); + let headers = traceparent_headers(&cx); + assert_eq!( + headers, + vec![( + "traceparent".to_string(), + "00-0102030405060708090a0b0c0d0e0f10-0102030405060708-00".to_string(), + )] + ); + } + + #[test] + fn traceparent_headers_invalid_context_is_empty() { + // No span (debug-build no-op tracer) → no header, never an + // all-zero traceparent. + let cx = Context::new(); + assert!(traceparent_headers(&cx).is_empty()); + } +} diff --git a/src/reels/script.rs b/src/reels/script.rs index 38ef9cc..33b10b7 100644 --- a/src/reels/script.rs +++ b/src/reels/script.rs @@ -281,8 +281,9 @@ pub async fn generate_script( beats: &[PlannedBeat], ) -> Result { let (system, user) = build_script_messages(meta, beats); + let cx = opentelemetry::Context::current(); let raw = client - .generate(&user, Some(&system), None) + .generate(&cx, &user, Some(&system), None) .await .context("LLM script generation failed")?; Ok(parse_script_response(&raw, beats.len())) diff --git a/src/unified_search.rs b/src/unified_search.rs index 0940a92..cba7987 100644 --- a/src/unified_search.rs +++ b/src/unified_search.rs @@ -211,7 +211,7 @@ pub async fn unified_search( log::info!("unified_search: translating with model={}", backend.model()); let today = chrono::Utc::now().date_naive(); - let sq = match translate_nl_query(backend.chat(), &nl, &tag_vocab, today).await { + let sq = match translate_nl_query(backend.chat(), &nl, &tag_vocab, today, &ctx).await { Ok(sq) => sq, Err(e) => { log::warn!("unified_search: translate_nl_query failed: {e:?}"); -- 2.54.0 From 10ed84d7b4f3a9221afd6df430ba2e9aea966848 Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Mon, 28 Sep 2026 10:35:28 -0400 Subject: [PATCH 4/5] Otel: dedupe LLM span error/status + traceparent helpers (#130 follow-up) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the P2 refactoring follow-ups parked on issue #130: - otel::with_traceparent(builder, &SpanContext) — one shared W3C traceparent injector. Replaces the 4-line inline header loop in llamacpp (3x) and openrouter (3x) and removes ollama's private inject_traceparent method (4 call sites now use the shared helper). Takes &SpanContext (not &dyn Span) because the Span trait is not dyn compatible in opentelemetry 0.31. - otel::record_http_failure(span, status, message) and otel::set_http_status(span, status) — collapse the repeated 'set http.status_code attribute + set_status(Status::error)' boilerplate across all three clients (~10 sites). Net -45 lines. The remaining if-let-Some(span.as_mut()) guards (token attributes, parse-failure status) stay inline rather than growing a SpanHandle wrapper type for a pattern that's already readable. --- src/ai/llamacpp.rs | 66 +++++++++++-------------------- src/ai/ollama.rs | 93 ++++++++++++++------------------------------ src/ai/openrouter.rs | 66 +++++++++++-------------------- src/otel.rs | 36 ++++++++++++++++- 4 files changed, 108 insertions(+), 153 deletions(-) diff --git a/src/ai/llamacpp.rs b/src/ai/llamacpp.rs index 5441b20..9d5370e 100644 --- a/src/ai/llamacpp.rs +++ b/src/ai/llamacpp.rs @@ -23,7 +23,7 @@ use anyhow::Context as _; use anyhow::{Result, anyhow, bail}; use async_trait::async_trait; -use opentelemetry::trace::{Span, Status, TraceContextExt}; +use opentelemetry::trace::{Span, Status}; use opentelemetry::{Context, KeyValue}; use reqwest::Client; use serde::Deserialize; @@ -523,11 +523,7 @@ impl LlamaCppClient { let _gpu = crate::ai::gpu::llm_lease().await; let mut request = self.client.post(&url).json(&Value::Object(body)); if let Some(s) = span.as_mut() { - for (k, v) in crate::otel::traceparent_headers( - &Context::new().with_remote_span_context(s.span_context().clone()), - ) { - request = request.header(k, v); - } + request = crate::otel::with_traceparent(request, s.span_context()); } let resp = match request.send().await { Ok(r) => r, @@ -541,21 +537,15 @@ impl LlamaCppClient { if !resp.status().is_success() { let status = resp.status(); - if let Some(s) = span.as_mut() { - s.set_attribute(KeyValue::new("http.status_code", status.as_u16() as i64)); - s.set_status(Status::error(format!( - "llama-swap chat request failed: {status}" - ))); - } + crate::otel::record_http_failure( + span.as_deref_mut(), + status.as_u16(), + &format!("llama-swap chat request failed: {status}"), + ); let body = resp.text().await.unwrap_or_default(); bail!("llama-swap chat request failed: {} — {}", status, body); } - if let Some(s) = span.as_mut() { - s.set_attribute(KeyValue::new( - "http.status_code", - resp.status().as_u16() as i64, - )); - } + crate::otel::set_http_status(span.as_deref_mut(), resp.status().as_u16()); let parsed: Value = resp.json().await.context("parsing chat response")?; let choice = parsed @@ -700,11 +690,7 @@ impl LlmClient for LlamaCppClient { // below so the lease spans the whole generation, not just the send. let gpu = crate::ai::gpu::llm_lease().await; let mut request = self.client.post(&url).json(&Value::Object(body)); - for (k, v) in crate::otel::traceparent_headers( - &Context::new().with_remote_span_context(span.span_context().clone()), - ) { - request = request.header(k, v); - } + request = crate::otel::with_traceparent(request, span.span_context()); let resp = request .send() .await @@ -712,17 +698,15 @@ impl LlmClient for LlamaCppClient { if !resp.status().is_success() { let status = resp.status(); - span.set_attribute(KeyValue::new("http.status_code", status.as_u16() as i64)); - span.set_status(Status::error(format!( - "llama-swap stream request failed: {status}" - ))); + crate::otel::record_http_failure( + Some(&mut span), + status.as_u16(), + &format!("llama-swap stream request failed: {status}"), + ); let body = resp.text().await.unwrap_or_default(); bail!("llama-swap stream request failed: {} — {}", status, body); } - span.set_attribute(KeyValue::new( - "http.status_code", - resp.status().as_u16() as i64, - )); + crate::otel::set_http_status(Some(&mut span), resp.status().as_u16()); let byte_stream = resp.bytes_stream(); let stream = async_stream::stream! { @@ -860,11 +844,7 @@ impl LlmClient for LlamaCppClient { }); let mut request = self.client.post(&url).json(&body); - for (k, v) in crate::otel::traceparent_headers( - &Context::new().with_remote_span_context(span.span_context().clone()), - ) { - request = request.header(k, v); - } + request = crate::otel::with_traceparent(request, span.span_context()); let resp = match request.send().await { Ok(r) => r, Err(e) => { @@ -875,17 +855,15 @@ impl LlmClient for LlamaCppClient { if !resp.status().is_success() { let status = resp.status(); - span.set_attribute(KeyValue::new("http.status_code", status.as_u16() as i64)); - span.set_status(Status::error(format!( - "llama-swap embedding request failed: {status}" - ))); + crate::otel::record_http_failure( + Some(&mut span), + status.as_u16(), + &format!("llama-swap embedding request failed: {status}"), + ); let body = resp.text().await.unwrap_or_default(); bail!("llama-swap embedding request failed: {} — {}", status, body); } - span.set_attribute(KeyValue::new( - "http.status_code", - resp.status().as_u16() as i64, - )); + crate::otel::set_http_status(Some(&mut span), resp.status().as_u16()); #[derive(Deserialize)] struct EmbedResponse { diff --git a/src/ai/ollama.rs b/src/ai/ollama.rs index f338cfe..144fb01 100644 --- a/src/ai/ollama.rs +++ b/src/ai/ollama.rs @@ -2,7 +2,7 @@ use anyhow::Context as _; use anyhow::Result; use async_trait::async_trait; use chrono::NaiveDate; -use opentelemetry::trace::{Span, Status, TraceContextExt}; +use opentelemetry::trace::{Span, Status}; use opentelemetry::{Context, KeyValue}; use reqwest::Client; use serde::{Deserialize, Serialize}; @@ -12,7 +12,6 @@ use std::sync::{Arc, Mutex}; use std::time::{Duration, Instant}; use crate::ai::llm_client::{LlmClient, LlmStreamEvent}; -use crate::otel::traceparent_headers; use futures::stream::{BoxStream, StreamExt}; // Re-export shared types so existing `crate::ai::ollama::{...}` imports @@ -392,7 +391,7 @@ impl OllamaClient { .post(format!("{}/api/generate", url)) .json(&request); if let Some(s) = span.as_mut() { - request_builder = self.inject_traceparent(request_builder, s); + request_builder = crate::otel::with_traceparent(request_builder, s.span_context()); } let response = match request_builder.send().await { Ok(r) => r, @@ -406,10 +405,11 @@ impl OllamaClient { if !response.status().is_success() { let status = response.status(); - if let Some(s) = span.as_mut() { - s.set_attribute(KeyValue::new("http.status_code", status.as_u16() as i64)); - s.set_status(Status::error(format!("Ollama request failed: {status}"))); - } + crate::otel::record_http_failure( + span.as_deref_mut(), + status.as_u16(), + &format!("Ollama request failed: {status}"), + ); let error_body = response.text().await.unwrap_or_default(); return Err(anyhow::anyhow!( "Ollama request failed: {} - {}", @@ -417,12 +417,7 @@ impl OllamaClient { error_body )); } - if let Some(s) = span.as_mut() { - s.set_attribute(KeyValue::new( - "http.status_code", - response.status().as_u16() as i64, - )); - } + crate::otel::set_http_status(span.as_deref_mut(), response.status().as_u16()); let result: OllamaResponse = match response.json().await { Ok(r) => r, @@ -451,22 +446,6 @@ impl OllamaClient { Ok(result.response) } - /// Attach the W3C traceparent header to an outgoing Ollama request so - /// a server that understands it can parent its own spans onto ours. - fn inject_traceparent( - &self, - builder: reqwest::RequestBuilder, - span: &opentelemetry::global::BoxedSpan, - ) -> reqwest::RequestBuilder { - let mut builder = builder; - for (k, v) in traceparent_headers( - &Context::new().with_remote_span_context(span.span_context().clone()), - ) { - builder = builder.header(k, v); - } - builder - } - /// Start the `llm.request` span for an outgoing Ollama call. fn start_request_span( &self, @@ -899,7 +878,7 @@ Analyze the image and use specific details from both the visual content and the let response = { let mut rb = self.client.post(&url).json(&request_body); - rb = self.inject_traceparent(rb, &span); + rb = crate::otel::with_traceparent(rb, span.span_context()); match rb.send().await { Ok(r) => r, Err(e) => { @@ -912,10 +891,11 @@ Analyze the image and use specific details from both the visual content and the if !response.status().is_success() { let status = response.status(); - span.set_attribute(KeyValue::new("http.status_code", status.as_u16() as i64)); - span.set_status(Status::error(format!( - "Ollama stream request failed: {status}" - ))); + crate::otel::record_http_failure( + Some(&mut span), + status.as_u16(), + &format!("Ollama stream request failed: {status}"), + ); let body = response.text().await.unwrap_or_default(); anyhow::bail!( "Ollama stream request failed with status {}: {}", @@ -923,10 +903,7 @@ Analyze the image and use specific details from both the visual content and the body ); } - span.set_attribute(KeyValue::new( - "http.status_code", - response.status().as_u16() as i64, - )); + crate::otel::set_http_status(Some(&mut span), response.status().as_u16()); // Ollama streams NDJSON: each line is a full `OllamaStreamChunk`. // We buffer partial lines across chunks from the byte stream. @@ -1064,7 +1041,7 @@ Analyze the image and use specific details from both the visual content and the let response = { let mut rb = self.client.post(&url).json(&request_body); if let Some(s) = span.as_mut() { - rb = self.inject_traceparent(rb, s); + rb = crate::otel::with_traceparent(rb, s.span_context()); } match rb.send().await { Ok(r) => r, @@ -1080,12 +1057,11 @@ Analyze the image and use specific details from both the visual content and the if !response.status().is_success() { let status = response.status(); - if let Some(s) = span.as_mut() { - s.set_attribute(KeyValue::new("http.status_code", status.as_u16() as i64)); - s.set_status(Status::error(format!( - "Ollama chat request failed: {status}" - ))); - } + crate::otel::record_http_failure( + span.as_deref_mut(), + status.as_u16(), + &format!("Ollama chat request failed: {status}"), + ); let body = response.text().await.unwrap_or_default(); // warn, not error — the outer `chat_with_tools` may recover via // the fallback server. When both fail, the outer layer emits the @@ -1102,12 +1078,7 @@ Analyze the image and use specific details from both the visual content and the body ); } - if let Some(s) = span.as_mut() { - s.set_attribute(KeyValue::new( - "http.status_code", - response.status().as_u16() as i64, - )); - } + crate::otel::set_http_status(span.as_deref_mut(), response.status().as_u16()); let chat_response: OllamaChatResponse = match response.json().await { Ok(r) => r, @@ -1268,7 +1239,7 @@ Analyze the image and use specific details from both the visual content and the .post(format!("{}/api/embed", url)) .json(&request); if let Some(s) = span.as_mut() { - rb = self.inject_traceparent(rb, s); + rb = crate::otel::with_traceparent(rb, s.span_context()); } match rb.send().await { Ok(r) => r, @@ -1283,12 +1254,11 @@ Analyze the image and use specific details from both the visual content and the if !response.status().is_success() { let status = response.status(); - if let Some(s) = span.as_mut() { - s.set_attribute(KeyValue::new("http.status_code", status.as_u16() as i64)); - s.set_status(Status::error(format!( - "Ollama batch embedding request failed: {status}" - ))); - } + crate::otel::record_http_failure( + span.as_deref_mut(), + status.as_u16(), + &format!("Ollama batch embedding request failed: {status}"), + ); let error_body = response.text().await.unwrap_or_default(); return Err(anyhow::anyhow!( "Ollama batch embedding request failed: {} - {}", @@ -1296,12 +1266,7 @@ Analyze the image and use specific details from both the visual content and the error_body )); } - if let Some(s) = span.as_mut() { - s.set_attribute(KeyValue::new( - "http.status_code", - response.status().as_u16() as i64, - )); - } + crate::otel::set_http_status(span.as_deref_mut(), response.status().as_u16()); let result: OllamaEmbedResponse = match response.json().await { Ok(r) => r, diff --git a/src/ai/openrouter.rs b/src/ai/openrouter.rs index fdc7935..2b67755 100644 --- a/src/ai/openrouter.rs +++ b/src/ai/openrouter.rs @@ -5,7 +5,7 @@ use anyhow::Context as _; use anyhow::{Result, anyhow, bail}; use async_trait::async_trait; -use opentelemetry::trace::{Span, Status, TraceContextExt}; +use opentelemetry::trace::{Span, Status}; use opentelemetry::{Context, KeyValue}; use reqwest::Client; use serde::Deserialize; @@ -397,11 +397,7 @@ impl LlmClient for OpenRouterClient { let mut request = self .authed(self.client.post(&url)) .json(&Value::Object(body)); - for (k, v) in crate::otel::traceparent_headers( - &Context::new().with_remote_span_context(span.span_context().clone()), - ) { - request = request.header(k, v); - } + request = crate::otel::with_traceparent(request, span.span_context()); let resp = request .send() .await @@ -409,17 +405,15 @@ impl LlmClient for OpenRouterClient { if !resp.status().is_success() { let status = resp.status(); - span.set_attribute(KeyValue::new("http.status_code", status.as_u16() as i64)); - span.set_status(Status::error(format!( - "OpenRouter stream request failed: {status}" - ))); + crate::otel::record_http_failure( + Some(&mut span), + status.as_u16(), + &format!("OpenRouter stream request failed: {status}"), + ); let body = resp.text().await.unwrap_or_default(); bail!("OpenRouter stream request failed: {} — {}", status, body); } - span.set_attribute(KeyValue::new( - "http.status_code", - resp.status().as_u16() as i64, - )); + crate::otel::set_http_status(Some(&mut span), resp.status().as_u16()); // OpenAI-compat SSE stream. Each event is `data: \n\n`, with // `data: [DONE]` signalling completion. Tool calls arrive as @@ -612,11 +606,7 @@ impl LlmClient for OpenRouterClient { }); let mut request = self.authed(self.client.post(&url)).json(&body); - for (k, v) in crate::otel::traceparent_headers( - &Context::new().with_remote_span_context(span.span_context().clone()), - ) { - request = request.header(k, v); - } + request = crate::otel::with_traceparent(request, span.span_context()); let resp = request .send() .await @@ -627,17 +617,15 @@ impl LlmClient for OpenRouterClient { if !resp.status().is_success() { let status = resp.status(); - span.set_attribute(KeyValue::new("http.status_code", status.as_u16() as i64)); - span.set_status(Status::error(format!( - "OpenRouter embedding request failed: {status}" - ))); + crate::otel::record_http_failure( + Some(&mut span), + status.as_u16(), + &format!("OpenRouter embedding request failed: {status}"), + ); let body = resp.text().await.unwrap_or_default(); bail!("OpenRouter embedding request failed: {} — {}", status, body); } - span.set_attribute(KeyValue::new( - "http.status_code", - resp.status().as_u16() as i64, - )); + crate::otel::set_http_status(Some(&mut span), resp.status().as_u16()); #[derive(Deserialize)] struct EmbedResponse { @@ -764,11 +752,7 @@ impl OpenRouterClient { .authed(self.client.post(&url)) .json(&Value::Object(body)); if let Some(s) = span.as_mut() { - for (k, v) in crate::otel::traceparent_headers( - &Context::new().with_remote_span_context(s.span_context().clone()), - ) { - request = request.header(k, v); - } + request = crate::otel::with_traceparent(request, s.span_context()); } let resp = match request.send().await { Ok(r) => r, @@ -782,21 +766,15 @@ impl OpenRouterClient { if !resp.status().is_success() { let status = resp.status(); - if let Some(s) = span.as_mut() { - s.set_attribute(KeyValue::new("http.status_code", status.as_u16() as i64)); - s.set_status(Status::error(format!( - "OpenRouter chat request failed: {status}" - ))); - } + crate::otel::record_http_failure( + span.as_deref_mut(), + status.as_u16(), + &format!("OpenRouter chat request failed: {status}"), + ); let body = resp.text().await.unwrap_or_default(); bail!("OpenRouter chat request failed: {} — {}", status, body); } - if let Some(s) = span.as_mut() { - s.set_attribute(KeyValue::new( - "http.status_code", - resp.status().as_u16() as i64, - )); - } + crate::otel::set_http_status(span.as_deref_mut(), resp.status().as_u16()); let parsed: Value = match resp.json().await { Ok(v) => v, diff --git a/src/otel.rs b/src/otel.rs index 20b319e..1eebbaf 100644 --- a/src/otel.rs +++ b/src/otel.rs @@ -2,7 +2,9 @@ use actix_web::HttpRequest; use actix_web::http::header::HeaderMap; use opentelemetry::global::{BoxedSpan, BoxedTracer}; use opentelemetry::propagation::TextMapPropagator; -use opentelemetry::trace::{Span, SpanBuilder, SpanKind, Status, TraceContextExt, Tracer}; +use opentelemetry::trace::{ + Span, SpanBuilder, SpanContext, SpanKind, Status, TraceContextExt, Tracer, +}; use opentelemetry::{Context, KeyValue, global}; use opentelemetry_appender_log::OpenTelemetryLogBridge; use opentelemetry_otlp::WithExportConfig; @@ -117,6 +119,38 @@ pub fn traceparent_headers(context: &Context) -> Vec<(String, String)> { headers.into_iter().filter(|(_, v)| !v.is_empty()).collect() } +/// Attach the W3C traceparent header to an outgoing request so a downstream +/// service parents its own spans onto this trace. No-op when the span context +/// is not valid — e.g. debug builds, where the global tracer is a no-op and an +/// all-zero traceparent would produce garbage parent spans. +pub fn with_traceparent( + builder: reqwest::RequestBuilder, + span_context: &SpanContext, +) -> reqwest::RequestBuilder { + let mut b = builder; + for (k, v) in + traceparent_headers(&Context::new().with_remote_span_context(span_context.clone())) + { + b = b.header(k, v); + } + b +} + +/// Record the HTTP status code on the span (success or failure path). +pub fn set_http_status(span: Option<&mut BoxedSpan>, status: u16) { + if let Some(s) = span { + s.set_attribute(KeyValue::new("http.status_code", status as i64)); + } +} + +/// Record an HTTP failure on the span: status code plus an error status. +pub fn record_http_failure(span: Option<&mut BoxedSpan>, status: u16, message: &str) { + if let Some(s) = span { + s.set_attribute(KeyValue::new("http.status_code", status as i64)); + s.set_status(Status::error(message.to_string())); + } +} + pub fn trace_db_call( context: &Context, query_type: &str, -- 2.54.0 From f850c535f1f2b69023604e57b9a83472522f4f9f Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Mon, 28 Sep 2026 21:41:38 -0400 Subject: [PATCH 5/5] Otel: thread request context through streaming chat path (#129 review) The legacy SSE streaming path (chat_turn_stream -> run_streaming_turn -> run_bootstrap/continuation_streaming) built its LLM spans from Context::current(), which is always the empty root context here: the codebase threads OTel contexts explicitly and never attaches one, there is no context-aware tokio layer, and the turn runs on a tokio::spawn. So every llm.request span on that path was an orphaned root span, uncorrelated with the HTTP request. Thread the handler's extracted request context down through chat_turn_stream -> chat_turn_stream_inner -> run_streaming_turn -> run_bootstrap_streaming / run_continuation_streaming, and use it for the describe_image and agentic-loop calls. Mirrors the non-streaming path, which already threads parent_cx -> loop_cx. Also rename the block-scoped db 'cx' in run_streaming_turn to db_cx so it no longer shadows the new parameter. --- src/ai/handlers.rs | 4 +++- src/ai/insight_chat.rs | 27 ++++++++++++++------------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/ai/handlers.rs b/src/ai/handlers.rs index b00c7b2..82a5406 100644 --- a/src/ai/handlers.rs +++ b/src/ai/handlers.rs @@ -1667,10 +1667,12 @@ pub struct ChatSwitchBranchRequest { /// Returns `text/event-stream` with one event per chat stream event. #[post("/insights/chat/stream")] pub async fn chat_stream_handler( + http_request: HttpRequest, claims: Claims, request: web::Json, app_state: web::Data, ) -> HttpResponse { + let parent_context = extract_context_from_request(&http_request); let mut request = request.0; // Stored owner defaults fill what the client omitted (request → pref → // hard default; the hard default still applies downstream). @@ -1716,7 +1718,7 @@ pub async fn chat_stream_handler( }; let service = app_state.insight_chat.clone(); - let events = service.chat_turn_stream(chat_req); + let events = service.chat_turn_stream(chat_req, parent_context); // Map ChatStreamEvent → SSE frame bytes. let sse_stream = futures::stream::StreamExt::map(events, |ev| { diff --git a/src/ai/insight_chat.rs b/src/ai/insight_chat.rs index f4dd3da..b11a1e8 100644 --- a/src/ai/insight_chat.rs +++ b/src/ai/insight_chat.rs @@ -782,10 +782,11 @@ impl InsightChatService { pub fn chat_turn_stream( self: Arc, req: ChatTurnRequest, + cx: opentelemetry::Context, ) -> BoxStream<'static, ChatStreamEvent> { let svc = self; let s = async_stream::stream! { - match svc.chat_turn_stream_inner(req, Ok).await { + match svc.chat_turn_stream_inner(req, cx, Ok).await { Ok(mut rx) => { while let Some(ev) = rx.recv().await { yield ev; @@ -807,6 +808,7 @@ impl InsightChatService { async fn chat_turn_stream_inner( self: Arc, req: ChatTurnRequest, + cx: opentelemetry::Context, _ev_mapper: F, ) -> Result> where @@ -815,7 +817,7 @@ impl InsightChatService { let (tx, rx) = tokio::sync::mpsc::channel::(64); let svc = self.clone(); tokio::spawn(async move { - let result = svc.run_streaming_turn(req, tx.clone()).await; + let result = svc.run_streaming_turn(req, cx, tx.clone()).await; if let Err(e) = result { let _ = tx.send(ChatStreamEvent::Error(format!("{}", e))).await; } @@ -1371,6 +1373,7 @@ impl InsightChatService { async fn run_streaming_turn( self: Arc, req: ChatTurnRequest, + cx: opentelemetry::Context, tx: tokio::sync::mpsc::Sender, ) -> Result<()> { if req.user_message.trim().is_empty() { @@ -1400,17 +1403,17 @@ impl InsightChatService { // library bootstraps cleanly, even when another library has // an insight for the same rel_path. let existing_insight = { - let cx = opentelemetry::Context::new(); + let db_cx = opentelemetry::Context::new(); let mut dao = self.insight_dao.lock().expect("Unable to lock InsightDao"); - dao.get_current_insight_for_library(&cx, req.library_id, &normalized) + dao.get_current_insight_for_library(&db_cx, req.library_id, &normalized) .map_err(|e| anyhow!("failed to load insight: {:?}", e))? }; if req.regenerate || existing_insight.is_none() { - return self.run_bootstrap_streaming(req, normalized, tx).await; + return self.run_bootstrap_streaming(req, normalized, cx, tx).await; } let insight = existing_insight.expect("just checked Some above"); - self.run_continuation_streaming(req, normalized, insight, tx) + self.run_continuation_streaming(req, normalized, cx, insight, tx) .await } @@ -1422,6 +1425,7 @@ impl InsightChatService { &self, req: ChatTurnRequest, normalized: String, + cx: opentelemetry::Context, insight: crate::database::models::PhotoInsight, tx: tokio::sync::mpsc::Sender, ) -> Result<()> { @@ -1519,7 +1523,7 @@ impl InsightChatService { &active_persona, max_iterations, &tx, - &opentelemetry::Context::current(), + &cx, ) .await?; let AgenticLoopOutcome { @@ -1631,6 +1635,7 @@ impl InsightChatService { &self, req: ChatTurnRequest, normalized: String, + cx: opentelemetry::Context, tx: tokio::sync::mpsc::Sender, ) -> Result<()> { let active_persona = req @@ -1687,11 +1692,7 @@ impl InsightChatService { // images_inline backends send images directly to the chat model. let visual_block = if !backend.images_inline { match image_base64.as_deref() { - Some(b64) => match backend - .local() - .describe_image(&opentelemetry::Context::current(), b64) - .await - { + Some(b64) => match backend.local().describe_image(&cx, b64).await { Ok(desc) => { format!("Visual description (from local vision model):\n{}\n", desc) } @@ -1758,7 +1759,7 @@ impl InsightChatService { &active_persona, max_iterations, &tx, - &opentelemetry::Context::current(), + &cx, ) .await?; let AgenticLoopOutcome { -- 2.54.0