From d8a5b9bef1a187dd0d46fcb1a8e5e9bde63bfb71 Mon Sep 17 00:00:00 2001 From: Jake Date: Sun, 19 Apr 2026 20:43:56 +0100 Subject: [PATCH] feat(storage): profile + profile_terms CRUD with Default-profile guardrails --- crates/storage/Cargo.toml | 3 + crates/storage/src/database.rs | 408 +++++++++++++++++++++++++++++++++ crates/storage/src/lib.rs | 12 +- 3 files changed, 418 insertions(+), 5 deletions(-) diff --git a/crates/storage/Cargo.toml b/crates/storage/Cargo.toml index 143ed57..58ab465 100644 --- a/crates/storage/Cargo.toml +++ b/crates/storage/Cargo.toml @@ -15,3 +15,6 @@ tokio = { version = "1", features = ["rt", "sync", "macros"] } # Logging log = "0.4" + +# UUIDs for profile + profile_terms ids (v7 random). +uuid = { version = "1", features = ["v4"] } diff --git a/crates/storage/src/database.rs b/crates/storage/src/database.rs index 35b11e7..f8ee851 100644 --- a/crates/storage/src/database.rs +++ b/crates/storage/src/database.rs @@ -537,6 +537,23 @@ pub async fn get_setting(pool: &SqlitePool, key: &str) -> Result> // --- Row types --- +#[derive(Debug, Clone)] +pub struct ProfileRow { + pub id: String, + pub name: String, + pub initial_prompt: String, + pub created_at: String, +} + +#[derive(Debug, Clone)] +pub struct ProfileTermRow { + pub id: String, + pub profile_id: String, + pub term: String, + pub note: String, + pub created_at: String, +} + #[derive(Debug, Clone)] pub struct TranscriptRow { pub id: String, @@ -606,6 +623,25 @@ fn transcript_row_from(r: &sqlx::sqlite::SqliteRow) -> TranscriptRow { } } +fn profile_row_from(r: &sqlx::sqlite::SqliteRow) -> ProfileRow { + ProfileRow { + id: r.get("id"), + name: r.get("name"), + initial_prompt: r.get("initial_prompt"), + created_at: r.get("created_at"), + } +} + +fn profile_term_row_from(r: &sqlx::sqlite::SqliteRow) -> ProfileTermRow { + ProfileTermRow { + id: r.get("id"), + profile_id: r.get("profile_id"), + term: r.get("term"), + note: r.get("note"), + created_at: r.get("created_at"), + } +} + fn task_row_from(r: sqlx::sqlite::SqliteRow) -> TaskRow { TaskRow { id: r.get("id"), @@ -622,6 +658,163 @@ fn task_row_from(r: sqlx::sqlite::SqliteRow) -> TaskRow { } } +// --- Profile CRUD (Phase 2 Task 11) --- +// +// Profiles partition the per-user transcription dictionary so that a single +// database can hold multiple named contexts (work, personal, a client, a +// project). Each profile owns an `initial_prompt` that steers Whisper's +// decoder and a list of `profile_terms` that bias recognition and feed +// post-transcription fix-ups. +// +// The `Default` profile (id = DEFAULT_PROFILE_ID, seeded by migration v6) +// must always exist and must never be renamed. Two layers enforce this: +// 1. The DB triggers `trg_protect_default_profile_delete` / +// `trg_protect_default_profile_rename` from migration v6. +// 2. Rust-layer fail-fast checks below that short-circuit BEFORE the +// query hits sqlite, so UI callers get a friendly KonError::StorageError +// instead of an opaque `SQLITE_CONSTRAINT_TRIGGER` wrapped in text. + +pub async fn list_profiles(pool: &SqlitePool) -> Result> { + let rows = sqlx::query( + "SELECT id, name, initial_prompt, created_at FROM profiles ORDER BY name ASC", + ) + .fetch_all(pool) + .await + .map_err(|e| KonError::StorageError(format!("List profiles failed: {e}")))?; + Ok(rows.iter().map(profile_row_from).collect()) +} + +pub async fn get_profile(pool: &SqlitePool, id: &str) -> Result> { + let row = sqlx::query( + "SELECT id, name, initial_prompt, created_at FROM profiles WHERE id = ?", + ) + .bind(id) + .fetch_optional(pool) + .await + .map_err(|e| KonError::StorageError(format!("Get profile failed: {e}")))?; + Ok(row.as_ref().map(profile_row_from)) +} + +pub async fn create_profile( + pool: &SqlitePool, + name: &str, + initial_prompt: &str, +) -> Result { + let id = uuid::Uuid::new_v4().to_string(); + let row = sqlx::query( + "INSERT INTO profiles (id, name, initial_prompt, created_at) \ + VALUES (?, ?, ?, datetime('now')) \ + RETURNING id, name, initial_prompt, created_at", + ) + .bind(&id) + .bind(name) + .bind(initial_prompt) + .fetch_one(pool) + .await + .map_err(|e| KonError::StorageError(format!("Create profile failed: {e}")))?; + Ok(profile_row_from(&row)) +} + +/// Full replace of `name` and `initial_prompt` for a profile. +/// +/// Fails fast in Rust — before hitting sqlite — for two Default-profile +/// rules that the migration v6 trigger also enforces but returns opaquely: +/// * the Default id must not be renamed; +/// * no other profile may be renamed to "Default" (would collide with +/// the UNIQUE(name) constraint anyway, but we surface a better error). +pub async fn update_profile( + pool: &SqlitePool, + id: &str, + name: &str, + initial_prompt: &str, +) -> Result<()> { + if id == crate::DEFAULT_PROFILE_ID && name != "Default" { + return Err(KonError::StorageError( + "Default profile cannot be renamed".into(), + )); + } + if id != crate::DEFAULT_PROFILE_ID && name == "Default" { + return Err(KonError::StorageError( + "Cannot rename another profile to 'Default'".into(), + )); + } + sqlx::query("UPDATE profiles SET name = ?, initial_prompt = ? WHERE id = ?") + .bind(name) + .bind(initial_prompt) + .bind(id) + .execute(pool) + .await + .map_err(|e| KonError::StorageError(format!("Update profile failed: {e}")))?; + Ok(()) +} + +/// Delete a profile. Rejects the Default profile at the Rust layer so the +/// UI sees a human-readable message instead of a trigger ABORT string. +/// ON DELETE CASCADE on `profile_terms.profile_id` cleans up children. +pub async fn delete_profile(pool: &SqlitePool, id: &str) -> Result<()> { + if id == crate::DEFAULT_PROFILE_ID { + return Err(KonError::StorageError( + "Default profile cannot be deleted".into(), + )); + } + sqlx::query("DELETE FROM profiles WHERE id = ?") + .bind(id) + .execute(pool) + .await + .map_err(|e| KonError::StorageError(format!("Delete profile failed: {e}")))?; + Ok(()) +} + +pub async fn list_profile_terms( + pool: &SqlitePool, + profile_id: &str, +) -> Result> { + let rows = sqlx::query( + "SELECT id, profile_id, term, note, created_at FROM profile_terms \ + WHERE profile_id = ? ORDER BY term ASC", + ) + .bind(profile_id) + .fetch_all(pool) + .await + .map_err(|e| KonError::StorageError(format!("List profile terms failed: {e}")))?; + Ok(rows.iter().map(profile_term_row_from).collect()) +} + +/// UPSERT on UNIQUE(profile_id, term). Mirrors `add_dictionary_entry`'s +/// ON CONFLICT … DO UPDATE pattern so that re-adding an existing term +/// simply refreshes the note and returns the existing row (not a fresh id). +pub async fn add_profile_term( + pool: &SqlitePool, + profile_id: &str, + term: &str, + note: &str, +) -> Result { + let new_id = uuid::Uuid::new_v4().to_string(); + let row = sqlx::query( + "INSERT INTO profile_terms (id, profile_id, term, note, created_at) \ + VALUES (?, ?, ?, ?, datetime('now')) \ + ON CONFLICT(profile_id, term) DO UPDATE SET note = excluded.note \ + RETURNING id, profile_id, term, note, created_at", + ) + .bind(&new_id) + .bind(profile_id) + .bind(term) + .bind(note) + .fetch_one(pool) + .await + .map_err(|e| KonError::StorageError(format!("Add profile term failed: {e}")))?; + Ok(profile_term_row_from(&row)) +} + +pub async fn delete_profile_term(pool: &SqlitePool, id: &str) -> Result<()> { + sqlx::query("DELETE FROM profile_terms WHERE id = ?") + .bind(id) + .execute(pool) + .await + .map_err(|e| KonError::StorageError(format!("Delete profile term failed: {e}")))?; + Ok(()) +} + // --- Error Logging --- /// Log a structured error to the `error_log` table. @@ -949,6 +1142,221 @@ mod tests { assert!(res.is_err(), "update_task must error when id does not exist"); } + // --- Profile CRUD tests (Task 11) --- + + #[tokio::test] + async fn list_profiles_returns_seeded_default() { + // Happy path: a fresh DB has exactly the seeded Default profile. + let pool = test_pool().await; + let profiles = list_profiles(&pool).await.unwrap(); + assert_eq!(profiles.len(), 1, "Default profile must be seeded"); + assert_eq!(profiles[0].id, crate::DEFAULT_PROFILE_ID); + assert_eq!(profiles[0].name, "Default"); + } + + #[tokio::test] + async fn get_profile_happy_path_and_missing() { + let pool = test_pool().await; + let p = get_profile(&pool, crate::DEFAULT_PROFILE_ID).await.unwrap(); + assert!(p.is_some()); + assert_eq!(p.unwrap().name, "Default"); + + // Guardrail: unknown id returns None, not an error. + let missing = get_profile(&pool, "00000000-0000-0000-0000-000000000099") + .await + .unwrap(); + assert!(missing.is_none()); + } + + #[tokio::test] + async fn create_profile_happy_path() { + let pool = test_pool().await; + let row = create_profile(&pool, "Work", "You are a meeting notes assistant.") + .await + .unwrap(); + assert_eq!(row.name, "Work"); + assert_eq!(row.initial_prompt, "You are a meeting notes assistant."); + assert!(!row.id.is_empty()); + assert_ne!(row.id, crate::DEFAULT_PROFILE_ID); + + let profiles = list_profiles(&pool).await.unwrap(); + assert_eq!(profiles.len(), 2, "Default + Work"); + } + + #[tokio::test] + async fn create_profile_rejects_duplicate_name() { + // Guardrail: UNIQUE(name) collision surfaces as StorageError. + let pool = test_pool().await; + create_profile(&pool, "Personal", "").await.unwrap(); + let res = create_profile(&pool, "Personal", "").await; + assert!(res.is_err(), "duplicate name must fail"); + } + + #[tokio::test] + async fn update_profile_happy_path() { + let pool = test_pool().await; + let created = create_profile(&pool, "ClientA", "prompt v1").await.unwrap(); + update_profile(&pool, &created.id, "ClientA Renamed", "prompt v2") + .await + .unwrap(); + let fetched = get_profile(&pool, &created.id).await.unwrap().unwrap(); + assert_eq!(fetched.name, "ClientA Renamed"); + assert_eq!(fetched.initial_prompt, "prompt v2"); + } + + #[tokio::test] + async fn update_profile_rejects_renaming_default() { + // Guardrail: Rust layer rejects renaming the Default id; must not + // reach sqlite trigger. + let pool = test_pool().await; + let res = update_profile(&pool, crate::DEFAULT_PROFILE_ID, "NotDefault", "").await; + let msg = format!("{}", res.expect_err("must reject rename of Default")); + assert!( + msg.contains("Default profile cannot be renamed"), + "got: {msg}" + ); + } + + #[tokio::test] + async fn update_profile_rejects_renaming_other_to_default() { + // Guardrail: another profile cannot adopt the name "Default". + let pool = test_pool().await; + let other = create_profile(&pool, "Temp", "").await.unwrap(); + let res = update_profile(&pool, &other.id, "Default", "").await; + let msg = format!("{}", res.expect_err("must reject adopting 'Default'")); + assert!(msg.contains("Default"), "got: {msg}"); + } + + #[tokio::test] + async fn delete_profile_happy_path() { + let pool = test_pool().await; + let created = create_profile(&pool, "Ephemeral", "").await.unwrap(); + delete_profile(&pool, &created.id).await.unwrap(); + let fetched = get_profile(&pool, &created.id).await.unwrap(); + assert!(fetched.is_none()); + } + + #[tokio::test] + async fn delete_profile_rejects_default() { + let pool = test_pool().await; + let res = delete_profile(&pool, crate::DEFAULT_PROFILE_ID).await; + let msg = format!("{}", res.expect_err("must reject delete of Default")); + assert!( + msg.contains("Default profile cannot be deleted"), + "got: {msg}" + ); + // And Default must still be listed. + let profiles = list_profiles(&pool).await.unwrap(); + assert_eq!(profiles.len(), 1); + } + + #[tokio::test] + async fn list_profile_terms_happy_path() { + let pool = test_pool().await; + let p = create_profile(&pool, "WithTerms", "").await.unwrap(); + add_profile_term(&pool, &p.id, "CORBEL", "company name") + .await + .unwrap(); + add_profile_term(&pool, &p.id, "Wren", "agent name").await.unwrap(); + let terms = list_profile_terms(&pool, &p.id).await.unwrap(); + assert_eq!(terms.len(), 2); + // Sorted alphabetically. + assert_eq!(terms[0].term, "CORBEL"); + assert_eq!(terms[1].term, "Wren"); + } + + #[tokio::test] + async fn list_profile_terms_filters_by_profile() { + // Guardrail: terms from one profile must not leak into another. + let pool = test_pool().await; + let a = create_profile(&pool, "A", "").await.unwrap(); + let b = create_profile(&pool, "B", "").await.unwrap(); + add_profile_term(&pool, &a.id, "alpha", "").await.unwrap(); + add_profile_term(&pool, &b.id, "beta", "").await.unwrap(); + let a_terms = list_profile_terms(&pool, &a.id).await.unwrap(); + let b_terms = list_profile_terms(&pool, &b.id).await.unwrap(); + assert_eq!(a_terms.len(), 1); + assert_eq!(a_terms[0].term, "alpha"); + assert_eq!(b_terms.len(), 1); + assert_eq!(b_terms[0].term, "beta"); + } + + #[tokio::test] + async fn add_profile_term_happy_path() { + let pool = test_pool().await; + let p = create_profile(&pool, "P", "").await.unwrap(); + let row = add_profile_term(&pool, &p.id, "ACME", "client").await.unwrap(); + assert_eq!(row.term, "ACME"); + assert_eq!(row.note, "client"); + assert_eq!(row.profile_id, p.id); + } + + #[tokio::test] + async fn add_profile_term_upsert_on_duplicate_term() { + // Guardrail: UPSERT refreshes note and reuses the existing row id + // rather than violating UNIQUE(profile_id, term). + let pool = test_pool().await; + let p = create_profile(&pool, "P2", "").await.unwrap(); + let first = add_profile_term(&pool, &p.id, "ACME", "first note") + .await + .unwrap(); + let second = add_profile_term(&pool, &p.id, "ACME", "second note") + .await + .unwrap(); + assert_eq!(first.id, second.id, "UPSERT must preserve row id"); + assert_eq!(second.note, "second note"); + let terms = list_profile_terms(&pool, &p.id).await.unwrap(); + assert_eq!(terms.len(), 1, "UPSERT must not create a second row"); + } + + #[tokio::test] + async fn delete_profile_term_happy_path() { + let pool = test_pool().await; + let p = create_profile(&pool, "P3", "").await.unwrap(); + let t = add_profile_term(&pool, &p.id, "Foo", "").await.unwrap(); + delete_profile_term(&pool, &t.id).await.unwrap(); + let remaining = list_profile_terms(&pool, &p.id).await.unwrap(); + assert!(remaining.is_empty()); + } + + #[tokio::test] + async fn delete_profile_term_missing_id_is_noop() { + // Guardrail: deleting a non-existent term must not error; sqlite + // DELETE on zero rows is a success. Verifies we're not hand-rolling + // an existence check that would regress the UI's happy path on + // double-click delete. + let pool = test_pool().await; + let p = create_profile(&pool, "NoopProfile", "").await.unwrap(); + let t = add_profile_term(&pool, &p.id, "real", "").await.unwrap(); + delete_profile_term(&pool, "00000000-dead-beef-0000-000000000000") + .await + .expect("missing id must be a silent no-op"); + // Real term still exists. + let terms = list_profile_terms(&pool, &p.id).await.unwrap(); + assert_eq!(terms.len(), 1); + assert_eq!(terms[0].id, t.id); + } + + #[tokio::test] + async fn delete_profile_cascades_to_terms() { + // Guardrail: deleting a profile must cascade-delete its terms via + // ON DELETE CASCADE (migration v6 FK). + let pool = test_pool().await; + let p = create_profile(&pool, "Doomed", "").await.unwrap(); + add_profile_term(&pool, &p.id, "x", "").await.unwrap(); + add_profile_term(&pool, &p.id, "y", "").await.unwrap(); + delete_profile(&pool, &p.id).await.unwrap(); + // Using a raw query because list_profile_terms would happily + // return [] even if the FK had been forgotten. We want to prove + // the child rows are actually gone from the table. + let count: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM profile_terms WHERE profile_id = ?") + .bind(&p.id) + .fetch_one(&pool) + .await + .unwrap(); + assert_eq!(count, 0, "terms must cascade-delete with their profile"); + } + #[tokio::test] async fn settings_crud_roundtrip() { let pool = test_pool().await; diff --git a/crates/storage/src/lib.rs b/crates/storage/src/lib.rs index 50df542..480a8db 100644 --- a/crates/storage/src/lib.rs +++ b/crates/storage/src/lib.rs @@ -7,12 +7,14 @@ pub mod migrations; pub const DEFAULT_PROFILE_ID: &str = "00000000-0000-0000-0000-000000000001"; pub use database::{ - add_dictionary_entry, complete_subtask_and_check_parent, complete_task, count_transcripts, - delete_dictionary_entry, delete_task, delete_transcript, get_setting, get_task_by_id, uncomplete_task, + add_dictionary_entry, add_profile_term, complete_subtask_and_check_parent, complete_task, + count_transcripts, create_profile, delete_dictionary_entry, delete_profile, + delete_profile_term, delete_task, delete_transcript, get_profile, get_setting, get_task_by_id, get_transcript, init, insert_subtask, insert_task, insert_transcript, list_dictionary, - list_recent_errors, list_subtasks, list_tasks, list_transcripts, list_transcripts_paged, - log_error, search_transcripts, set_setting, update_task, update_transcript, - update_transcript_meta, DictionaryEntry, ErrorLogRow, InsertTranscriptParams, TaskRow, + list_profile_terms, list_profiles, list_recent_errors, list_subtasks, list_tasks, + list_transcripts, list_transcripts_paged, log_error, search_transcripts, set_setting, + uncomplete_task, update_profile, update_task, update_transcript, update_transcript_meta, + DictionaryEntry, ErrorLogRow, InsertTranscriptParams, ProfileRow, ProfileTermRow, TaskRow, TranscriptRow, }; pub use file_storage::{app_data_dir, crashes_dir, database_path, logs_dir, recordings_dir};