diff --git a/migrations/2026-04-29-000200_add_is_ignored/down.sql b/migrations/2026-04-29-000200_add_is_ignored/down.sql new file mode 100644 index 0000000..41f7c00 --- /dev/null +++ b/migrations/2026-04-29-000200_add_is_ignored/down.sql @@ -0,0 +1,2 @@ +DROP INDEX IF EXISTS idx_persons_is_ignored; +ALTER TABLE persons DROP COLUMN is_ignored; diff --git a/migrations/2026-04-29-000200_add_is_ignored/up.sql b/migrations/2026-04-29-000200_add_is_ignored/up.sql new file mode 100644 index 0000000..d8fdac9 --- /dev/null +++ b/migrations/2026-04-29-000200_add_is_ignored/up.sql @@ -0,0 +1,20 @@ +-- IGNORE / junk bucket for the face recognition feature. +-- +-- An "Ignored" person is the destination for strangers, faces the user +-- doesn't want tagged, and false detections. It looks like any other +-- person row (so face_detections.person_id stays a clean foreign key) +-- but `is_ignored=1` flags it for special UI treatment: +-- - hidden from the persons list by default +-- - excluded from `find_persons_by_names_ci` so a tag-name match +-- can never auto-bind a real face to the ignore bucket +-- - cluster-suggest already filters by `person_id IS NULL`, so faces +-- bound to an ignored person are naturally excluded from future +-- re-clustering +-- +-- Partial index because the WHERE-clause is small (typically 1 row), +-- and we only ever query for `is_ignored = 1` to find the bucket. + +ALTER TABLE persons ADD COLUMN is_ignored BOOLEAN NOT NULL DEFAULT 0; + +CREATE INDEX idx_persons_is_ignored + ON persons(is_ignored) WHERE is_ignored = 1; diff --git a/src/database/schema.rs b/src/database/schema.rs index 5427ef7..55ad9e5 100644 --- a/src/database/schema.rs +++ b/src/database/schema.rs @@ -160,6 +160,7 @@ diesel::table! { notes -> Nullable, created_at -> BigInt, updated_at -> BigInt, + is_ignored -> Bool, } } diff --git a/src/faces.rs b/src/faces.rs index 769974d..3d97d07 100644 --- a/src/faces.rs +++ b/src/faces.rs @@ -56,6 +56,13 @@ pub struct Person { pub notes: Option, pub created_at: i64, pub updated_at: i64, + /// True for the IGNORE / junk bucket. Hidden from the default + /// persons list, skipped by `find_persons_by_names_ci` (so a tag + /// match can never auto-bind a real face into the ignore bucket), + /// and excluded from cluster suggestions because cluster-suggest + /// already filters by `person_id IS NULL` and ignored faces have + /// a non-null person_id. + pub is_ignored: bool, } #[derive(Insertable, Debug)] @@ -64,6 +71,7 @@ struct InsertPerson { name: String, notes: Option, created_from_tag: bool, + is_ignored: bool, created_at: i64, updated_at: i64, } @@ -184,6 +192,7 @@ pub struct PersonSummary { pub entity_id: Option, pub created_from_tag: bool, pub notes: Option, + pub is_ignored: bool, pub face_count: i64, } @@ -198,6 +207,11 @@ pub struct CreatePersonReq { /// unbridged; set explicitly to wire the person to LLM-extracted facts. #[serde(default)] pub entity_id: Option, + /// True for the IGNORE / junk bucket. The frontend sets this when + /// lazily creating the Ignored person via the dedicated endpoint; + /// hand-rolled callers leave it false. + #[serde(default)] + pub is_ignored: bool, } #[derive(Deserialize, Debug)] @@ -210,6 +224,10 @@ pub struct UpdatePersonReq { pub cover_face_id: Option, #[serde(default)] pub entity_id: Option, + /// Toggle the ignore flag. Mostly used by the UI to "un-ignore" a + /// person that was previously bound to the bucket. + #[serde(default)] + pub is_ignored: Option, } #[derive(Deserialize, Debug)] @@ -370,7 +388,15 @@ pub trait FaceDao: Send + Sync { &mut self, ctx: &opentelemetry::Context, library_id: Option, + include_ignored: bool, ) -> anyhow::Result>; + /// Get the IGNORE/junk bucket, creating it lazily on first call. + /// Idempotent — returns the same row across calls. Single global + /// bucket per database; the frontend never sees the literal name. + fn get_or_create_ignored_person( + &mut self, + ctx: &opentelemetry::Context, + ) -> anyhow::Result; fn update_person( &mut self, ctx: &opentelemetry::Context, @@ -940,6 +966,7 @@ impl FaceDao for SqliteFaceDao { name: req.name.clone(), notes: req.notes.clone(), created_from_tag: from_tag, + is_ignored: req.is_ignored, created_at: now, updated_at: now, }; @@ -967,6 +994,56 @@ impl FaceDao for SqliteFaceDao { }) } + fn get_or_create_ignored_person( + &mut self, + ctx: &opentelemetry::Context, + ) -> anyhow::Result { + // Fast path: there's already an is_ignored row → return it. + // Slow path on first use: create one with a stable display name + // ("Ignored"). Race-safe because the UNIQUE(name COLLATE NOCASE) + // index forces only one ever to exist (we trip and look up). + { + let mut conn = self.connection.lock().expect("face dao lock"); + if let Some(p) = persons::table + .filter(persons::is_ignored.eq(true)) + .order(persons::id.asc()) + .first::(conn.deref_mut()) + .optional() + .with_context(|| "lookup ignored person")? + { + return Ok(p); + } + } + // Drop the lock before delegating to create_person — that + // method takes its own lock. + match self.create_person( + ctx, + &CreatePersonReq { + name: "Ignored".to_string(), + notes: Some( + "Bucket for strangers, false detections, and faces \ + you don't want bound to a real person." + .to_string(), + ), + entity_id: None, + is_ignored: true, + }, + /*from_tag*/ false, + ) { + Ok(p) => Ok(p), + Err(e) if is_unique_violation(&e) => { + // Race: someone else created the row. Re-read. + let mut conn = self.connection.lock().expect("face dao lock"); + persons::table + .filter(persons::is_ignored.eq(true)) + .order(persons::id.asc()) + .first::(conn.deref_mut()) + .with_context(|| "load ignored person after race") + } + Err(e) => Err(e), + } + } + fn get_person( &mut self, ctx: &opentelemetry::Context, @@ -987,6 +1064,7 @@ impl FaceDao for SqliteFaceDao { &mut self, ctx: &opentelemetry::Context, library_id: Option, + include_ignored: bool, ) -> anyhow::Result> { let mut conn = self.connection.lock().expect("face dao lock"); trace_db_call(ctx, "query", "list_persons", |_| { @@ -994,7 +1072,15 @@ impl FaceDao for SqliteFaceDao { // query for face counts. Using a LEFT JOIN + GROUP BY in // Diesel here gets noisy with the optional library filter; a // second roundtrip is cheap and clearer. - let person_rows: Vec = persons::table + let mut person_query = persons::table.into_boxed(); + if !include_ignored { + // Default — hide the IGNORE/junk bucket from the list. + // The frontend asks include_ignored=true explicitly when + // it needs to surface ignored persons (e.g. a "show + // ignored" toggle in the management UI). + person_query = person_query.filter(persons::is_ignored.eq(false)); + } + let person_rows: Vec = person_query .order(persons::name.asc()) .load::(conn.deref_mut()) .with_context(|| "load persons")?; @@ -1045,6 +1131,7 @@ impl FaceDao for SqliteFaceDao { entity_id: p.entity_id, created_from_tag: p.created_from_tag, notes: p.notes, + is_ignored: p.is_ignored, face_count, } }) @@ -1092,6 +1179,12 @@ impl FaceDao for SqliteFaceDao { .execute(conn.deref_mut()) .with_context(|| "update person entity_id")?; } + if let Some(flag) = patch.is_ignored { + diesel::update(persons::table.find(id)) + .set((persons::is_ignored.eq(flag), persons::updated_at.eq(now))) + .execute(conn.deref_mut()) + .with_context(|| "update person is_ignored")?; + } persons::table .find(id) .first::(conn.deref_mut()) @@ -1216,9 +1309,15 @@ impl FaceDao for SqliteFaceDao { let placeholders = std::iter::repeat_n("?", names.len()) .collect::>() .join(","); + // Filter out is_ignored persons so the auto-bind path can + // never target the IGNORE/junk bucket — even if a tag name + // happens to match it (e.g. someone tags photos as "Ignored" + // by hand). Ignore-bucket assignment is an explicit operator + // action through the dedicated endpoint, never a heuristic. let sql = format!( "SELECT id, LOWER(name) AS lower_name FROM persons \ - WHERE LOWER(name) IN ({}) ORDER BY id ASC", + WHERE is_ignored = 0 AND LOWER(name) IN ({}) \ + ORDER BY id ASC", placeholders ); #[derive(QueryableByName)] @@ -1404,6 +1503,10 @@ where web::resource("/persons/bootstrap") .route(web::post().to(bootstrap_persons_handler::)), ) + .service( + web::resource("/persons/ignore-bucket") + .route(web::post().to(ignore_bucket_handler::)), + ) .service( web::resource("/tags/people-bootstrap-candidates") .route(web::get().to(bootstrap_candidates_handler::)), @@ -1731,6 +1834,7 @@ async fn bootstrap_persons_handler( name: trimmed.to_string(), notes: None, entity_id: None, + is_ignored: false, }, /*from_tag*/ true, ) { @@ -1762,6 +1866,17 @@ pub struct LibraryQuery { pub library: Option, } +/// `GET /persons` query: optional library scope, optional include of +/// the IGNORE/junk bucket. The bucket is hidden by default so the +/// management UI shows only "real" persons; the persons-management +/// screen requests it explicitly when it needs to surface ignored. +#[derive(Deserialize)] +pub struct ListPersonsQuery { + pub library: Option, + #[serde(default)] + pub include_ignored: bool, +} + async fn stats_handler( _: Claims, request: HttpRequest, @@ -2047,7 +2162,7 @@ async fn list_persons_handler( _: Claims, request: HttpRequest, app_state: web::Data, - query: web::Query, + query: web::Query, face_dao: web::Data>, ) -> impl Responder { let context = extract_context_from_request(&request); @@ -2059,7 +2174,21 @@ async fn list_persons_handler( .flatten() .map(|l| l.id); let mut dao = face_dao.lock().expect("face dao lock"); - dao.list_persons(&span_context, library_id) + dao.list_persons(&span_context, library_id, query.include_ignored) + .map(|p| HttpResponse::Ok().json(p)) + .into_http_internal_err() +} + +async fn ignore_bucket_handler( + _: Claims, + request: HttpRequest, + face_dao: web::Data>, +) -> impl Responder { + let context = extract_context_from_request(&request); + let span = global_tracer().start_with_context("persons.ignore_bucket", &context); + let span_context = opentelemetry::Context::current_with_span(span); + let mut dao = face_dao.lock().expect("face dao lock"); + dao.get_or_create_ignored_person(&span_context) .map(|p| HttpResponse::Ok().json(p)) .into_http_internal_err() } @@ -2299,6 +2428,7 @@ mod tests { name: "Cameron".into(), notes: None, entity_id: None, + is_ignored: false, }, false, ) @@ -2310,6 +2440,7 @@ mod tests { name: "Cameron".into(), notes: None, entity_id: None, + is_ignored: false, }, false, ) @@ -2457,6 +2588,7 @@ mod tests { name: "Alice".into(), notes: None, entity_id: None, + is_ignored: false, }, false, ) @@ -2468,6 +2600,7 @@ mod tests { name: "Bob".into(), notes: None, entity_id: None, + is_ignored: false, }, false, ) @@ -2508,6 +2641,7 @@ mod tests { name: "Subject".into(), notes: None, entity_id: None, + is_ignored: false, }, false, ) @@ -2580,6 +2714,7 @@ mod tests { name: "Cover".into(), notes: None, entity_id: None, + is_ignored: false, }, false, ) @@ -2648,6 +2783,7 @@ mod tests { name: "Alice".into(), notes: Some("the boss".into()), entity_id: None, + is_ignored: false, }, false, ) @@ -2665,6 +2801,7 @@ mod tests { name: "alice".into(), notes: None, entity_id: None, + is_ignored: false, }, false, ); @@ -2682,6 +2819,7 @@ mod tests { notes: Some("a new note".into()), cover_face_id: None, entity_id: None, + is_ignored: None, }, ) .expect("update"); @@ -2689,11 +2827,48 @@ mod tests { assert!(updated.updated_at >= prev_updated); // List + delete. - let listed = dao.list_persons(&ctx(), None).expect("list"); + let listed = dao.list_persons(&ctx(), None, false).expect("list"); assert_eq!(listed.len(), 1); assert_eq!(listed[0].face_count, 0); assert!(dao.delete_person(&ctx(), p.id, false).expect("delete")); - assert!(dao.list_persons(&ctx(), None).expect("list").is_empty()); + assert!( + dao.list_persons(&ctx(), None, false) + .expect("list") + .is_empty() + ); + } + + #[test] + fn ignore_bucket_idempotent_and_filters_auto_bind() { + // First call creates the bucket; second returns the same row. + // Once it exists, find_persons_by_names_ci must skip it even if + // the search term matches its name — the auto-bind path must + // NEVER target the IGNORE/junk bucket. + let mut dao = fresh_dao(); + let first = dao + .get_or_create_ignored_person(&ctx()) + .expect("create bucket"); + assert!(first.is_ignored); + let second = dao + .get_or_create_ignored_person(&ctx()) + .expect("re-fetch bucket"); + assert_eq!(first.id, second.id, "bucket must be idempotent"); + + // Searching by the bucket's name must return nothing — the + // auto-bind look-up filters is_ignored=true. + let m = dao + .find_persons_by_names_ci(&ctx(), &["ignored".into()]) + .expect("name lookup"); + assert!( + !m.contains_key("ignored"), + "find_persons_by_names_ci must skip the ignore bucket: {m:?}" + ); + + // Default list_persons hides it; include_ignored=true surfaces it. + let visible = dao.list_persons(&ctx(), None, false).expect("list"); + assert!(visible.iter().all(|p| !p.is_ignored)); + let all = dao.list_persons(&ctx(), None, true).expect("list all"); + assert!(all.iter().any(|p| p.is_ignored && p.id == first.id)); } #[test] @@ -2754,6 +2929,7 @@ mod tests { name: "Alice".into(), notes: None, entity_id: None, + is_ignored: false, }, false, ) @@ -2765,6 +2941,7 @@ mod tests { name: "Alyse".into(), notes: Some("dup of alice".into()), entity_id: None, + is_ignored: false, }, false, )