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<T> 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.
This commit is contained in:
Cameron Cordes committed
1 parent
aa3c58f051
commit
a699c940b8
10 files changed
+863
-85
No files matched your search
@@ -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<String>,
|
||||
backend: &mut Option<String>,
|
||||
persona_id: &mut Option<String>,
|
||||
) {
|
||||
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<ChatTurnHttpRequest>,
|
||||
app_state: web::Data<AppState>,
|
||||
) -> 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<UpsertUserAiPrefs>) -> 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"));
|
||||
}
|
||||
}
|
||||
@@ -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<String>,
|
||||
/// Last/explicit insight + chat model choice (local id or OpenRouter id).
|
||||
/// NULL = current per-request / env default behavior.
|
||||
pub default_model: Option<String>,
|
||||
/// `"local"` | `"hybrid"` — insight/chat backend. NULL = per-request /
|
||||
/// hard default ("local").
|
||||
pub default_backend: Option<String>,
|
||||
/// Active persona for persona chat. NULL = hard default ("default").
|
||||
pub default_persona_id: Option<String>,
|
||||
/// Reel span: `"day"` | `"week"` | `"month"`. NULL = per-request /
|
||||
/// hard default (day).
|
||||
pub reel_span: Option<String>,
|
||||
/// Reel segment cap. NULL = per-request / hard default.
|
||||
pub reel_max_segments: Option<i32>,
|
||||
}
|
||||
|
||||
#[derive(Insertable, Debug, Clone, serde::Deserialize, serde::Serialize)]
|
||||
@@ -589,4 +602,9 @@ pub struct UpsertUserAiPrefs {
|
||||
pub library: Option<String>,
|
||||
pub updated_at: i64,
|
||||
pub display_tz: Option<String>,
|
||||
pub default_model: Option<String>,
|
||||
pub default_backend: Option<String>,
|
||||
pub default_persona_id: Option<String>,
|
||||
pub reel_span: Option<String>,
|
||||
pub reel_max_segments: Option<i32>,
|
||||
}
|
||||
@@ -293,6 +293,11 @@ diesel::table! {
|
||||
library -> Nullable<Text>,
|
||||
updated_at -> BigInt,
|
||||
display_tz -> Nullable<Text>,
|
||||
default_model -> Nullable<Text>,
|
||||
default_backend -> Nullable<Text>,
|
||||
default_persona_id -> Nullable<Text>,
|
||||
reel_span -> Nullable<Text>,
|
||||
reel_max_segments -> Nullable<Integer>,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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());
|
||||
}
|
||||
}
|
||||
@@ -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<Self, Self::Err> {
|
||||
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<MemoriesSpan>,
|
||||
|
||||
+412
-70
@@ -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<String>,
|
||||
pub library: Option<String>,
|
||||
pub display_tz: Option<String>,
|
||||
/// Last/explicit insight + chat model choice. NULL = per-request /
|
||||
/// env default behavior.
|
||||
pub default_model: Option<String>,
|
||||
/// `"local"` | `"hybrid"`. NULL = per-request / hard default.
|
||||
pub default_backend: Option<String>,
|
||||
/// Active persona for persona chat. NULL = hard default ("default").
|
||||
pub default_persona_id: Option<String>,
|
||||
/// Reel span: `"day"` | `"week"` | `"month"`. NULL = per-request /
|
||||
/// hard default (day).
|
||||
pub reel_span: Option<String>,
|
||||
/// Reel segment cap. NULL = per-request / hard default.
|
||||
pub reel_max_segments: Option<i32>,
|
||||
pub updated_at: Option<i64>,
|
||||
}
|
||||
|
||||
@@ -43,41 +60,59 @@ pub struct PrefsResponse {
|
||||
/// explicit null and a missing key apart on `Option<Option<T>>` (both come
|
||||
/// out as outer `None`), so this carries a hand-rolled visitor.
|
||||
#[derive(Clone, Debug, Default)]
|
||||
pub enum TriState {
|
||||
pub enum TriState<T> {
|
||||
#[default]
|
||||
Absent,
|
||||
Null,
|
||||
Val(String),
|
||||
Val(T),
|
||||
}
|
||||
|
||||
impl<'de> Deserialize<'de> for TriState {
|
||||
fn deserialize<D>(deserializer: D) -> Result<TriState, D::Error>
|
||||
impl<'de, T: Deserialize<'de>> Deserialize<'de> for TriState<T> {
|
||||
fn deserialize<D>(deserializer: D) -> Result<TriState<T>, D::Error>
|
||||
where
|
||||
D: serde::Deserializer<'de>,
|
||||
{
|
||||
struct Visitor;
|
||||
impl<'de> serde::de::Visitor<'de> for Visitor {
|
||||
type Value = TriState;
|
||||
struct Visitor<T>(std::marker::PhantomData<T>);
|
||||
impl<'de, T: Deserialize<'de>> serde::de::Visitor<'de> for Visitor<T> {
|
||||
type Value = TriState<T>;
|
||||
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<E: serde::de::Error>(self) -> Result<TriState, E> {
|
||||
// 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<E: serde::de::Error>(self) -> Result<TriState<T>, E> {
|
||||
Ok(TriState::Null)
|
||||
}
|
||||
fn visit_none<E: serde::de::Error>(self) -> Result<TriState, E> {
|
||||
fn visit_none<E: serde::de::Error>(self) -> Result<TriState<T>, E> {
|
||||
Ok(TriState::Null)
|
||||
}
|
||||
fn visit_str<E: serde::de::Error>(self, v: &str) -> Result<TriState, E> {
|
||||
Ok(TriState::Val(v.to_string()))
|
||||
fn visit_some<D2>(self, d: D2) -> Result<TriState<T>, D2::Error>
|
||||
where
|
||||
D2: serde::Deserializer<'de>,
|
||||
{
|
||||
Ok(TriState::Val(T::deserialize(d)?))
|
||||
}
|
||||
fn visit_string<E: serde::de::Error>(self, v: String) -> Result<TriState, E> {
|
||||
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<T, E> with no conversion.
|
||||
fn visit_str<E: serde::de::Error>(self, v: &str) -> Result<TriState<T>, E> {
|
||||
T::deserialize(v.into_deserializer()).map(TriState::Val)
|
||||
}
|
||||
fn visit_string<E: serde::de::Error>(self, v: String) -> Result<TriState<T>, E> {
|
||||
T::deserialize(v.as_str().into_deserializer()).map(TriState::Val)
|
||||
}
|
||||
fn visit_i64<E: serde::de::Error>(self, v: i64) -> Result<TriState<T>, E> {
|
||||
T::deserialize(v.into_deserializer()).map(TriState::Val)
|
||||
}
|
||||
fn visit_u64<E: serde::de::Error>(self, v: u64) -> Result<TriState<T>, 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<String>,
|
||||
#[serde(default)]
|
||||
pub library: TriState,
|
||||
pub library: TriState<String>,
|
||||
#[serde(default)]
|
||||
pub display_tz: TriState,
|
||||
pub display_tz: TriState<String>,
|
||||
#[serde(default)]
|
||||
pub default_model: TriState<String>,
|
||||
#[serde(default)]
|
||||
pub default_backend: TriState<String>,
|
||||
#[serde(default)]
|
||||
pub default_persona_id: TriState<String>,
|
||||
#[serde(default)]
|
||||
pub reel_span: TriState<String>,
|
||||
#[serde(default)]
|
||||
pub reel_max_segments: TriState<i32>,
|
||||
}
|
||||
|
||||
#[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<String> {
|
||||
.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<T: Clone>(
|
||||
field: &TriState<T>,
|
||||
existing: Option<&UserAiPrefs>,
|
||||
keep: impl FnOnce(&UserAiPrefs) -> Option<String>,
|
||||
) -> Option<String> {
|
||||
keep: impl FnOnce(&UserAiPrefs) -> Option<T>,
|
||||
) -> Option<T> {
|
||||
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<String>, Option<String>, Option<String>, 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<String>,
|
||||
library: Option<String>,
|
||||
display_tz: Option<String>,
|
||||
default_model: Option<String>,
|
||||
default_backend: Option<String>,
|
||||
default_persona_id: Option<String>,
|
||||
reel_span: Option<String>,
|
||||
reel_max_segments: Option<i32>,
|
||||
}
|
||||
|
||||
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);
|
||||
}
|
||||
}
|
||||
+174
-15
@@ -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<MemoriesSpan>,
|
||||
req_max_segments: Option<usize>,
|
||||
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<CreateReelRequest>,
|
||||
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<i32>) -> 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(),
|
||||
|
||||
Reference in new issue
Block a user