diff --git a/ui/dr-ui/src/faces.rs b/ui/dr-ui/src/faces.rs index b78e31b..25fd06b 100644 --- a/ui/dr-ui/src/faces.rs +++ b/ui/dr-ui/src/faces.rs @@ -505,9 +505,22 @@ pub fn recluster( } // Which faces the user has already ruled on, so they enter as anchors. + // + // Two kinds of ruling, and the second is easy to miss. A *confirmation* is + // the obvious one. But setting a person aside is a ruling too, and the + // faces it covers are only ever suggestions — so anchoring confirmations + // alone left every ignored group's faces loose, and the next Regroup + // scattered them into fresh unnamed groups that were not ignored. The + // strangers came straight back, which is the feature not working at all. + // + // Anchoring them keeps them where the user put them, and does one better: + // a newly indexed face similar to a group that was set aside merges *into* + // it, so a stranger photographed again stays set aside instead of + // reappearing as somebody new. let mut confirmed = std::collections::HashMap::new(); for p in faces::people(conn)? { - for f in faces::for_person(conn, p.id, false)? { + // Suggestions included exactly when the group was set aside. + for f in faces::for_person(conn, p.id, p.ignored)? { confirmed.insert(f.id, p.id); } } @@ -1055,4 +1068,140 @@ mod tests { assert!((rgb[2] - 0.0).abs() < 1e-6); assert!(rgb[3..6].iter().all(|&v| v == 0.0)); } + + // ── setting a group aside has to survive regrouping ─────────────────── + + const TEST_MODEL: &str = "w600k_mbf"; + + /// A catalog holding `n` images and nothing else. + fn catalog_with(n: usize) -> Catalog { + let c = Catalog::in_memory().unwrap(); + let conn = c.connection(); + conn.execute( + "INSERT OR IGNORE INTO roots(id, kind, label) VALUES (1, 'local', 'lib')", + [], + ) + .unwrap(); + for i in 0..n { + conn.execute( + &format!( + "INSERT INTO images(id, root_id, source_ref, added_at) + VALUES ({}, 1, 'IMG_{i}.CR3', 0)", + i + 1 + ), + [], + ) + .unwrap(); + } + c + } + + /// A unit embedding pointing at `identity`, `cosine` of the way there. + fn embedding(identity: usize, cosine: f32) -> Vec { + let mut v = Box::new([0.0_f32; dr_face::EMBEDDING_DIM]); + v[identity * 2] = cosine; + v[identity * 2 + 1] = (1.0 - cosine * cosine).max(0.0).sqrt(); + dr_face::Embedding { + model: ModelId::new(TEST_MODEL.to_string()), + v, + } + .to_f16_bytes() + } + + fn put_face(catalog: &Catalog, image: u64, identity: usize, cosine: f32) { + let f = DetectedFace { + x: 0.1, + y: 0.1, + w: 0.2, + h: 0.2, + landmarks: [(0.0, 0.0); 5], + confidence: 0.9, + embedding: embedding(identity, cosine), + crop_px: 150.0, + model_id: TEST_MODEL.to_string(), + crop: Vec::new(), + }; + faces::record_detections( + catalog.connection(), + ImageId(image), + TEST_MODEL, + 1024, + std::slice::from_ref(&f), + ) + .unwrap(); + } + + /// The bug this pins: "not interested" only ever covered *suggested* faces, + /// and reclustering anchored confirmations alone. So the next Regroup cut + /// the ignored group's faces loose, built fresh unnamed groups out of them, + /// and every stranger the user had dismissed came straight back. + #[test] + fn a_group_set_aside_does_not_come_back_on_the_next_regroup() { + let catalog = catalog_with(3); + put_face(&catalog, 1, 0, 1.0); + put_face(&catalog, 2, 0, 0.99); + + recluster(&catalog, TEST_MODEL, dr_face::DEFAULT_MERGE_PROBABILITY).unwrap(); + let people = faces::people(catalog.connection()).unwrap(); + assert_eq!(people.len(), 1, "the two faces should have grouped"); + let stranger = people[0].id; + assert_eq!(people[0].suggested_faces, 2); + + faces::set_ignored(catalog.connection(), stranger, true).unwrap(); + + recluster(&catalog, TEST_MODEL, dr_face::DEFAULT_MERGE_PROBABILITY).unwrap(); + + let after = faces::people(catalog.connection()).unwrap(); + assert_eq!( + after.len(), + 1, + "regrouping resurrected the group that was set aside: {after:?}" + ); + assert_eq!(after[0].id, stranger); + assert!(after[0].ignored, "the group stopped being set aside"); + assert_eq!( + after[0].suggested_faces, 2, + "the faces left the group they were set aside in" + ); + } + + /// And it holds as the library grows: a stranger photographed again joins + /// the group that was set aside rather than arriving as somebody new. + #[test] + fn a_new_face_joins_the_group_it_matches_even_when_that_group_is_set_aside() { + let catalog = catalog_with(3); + put_face(&catalog, 1, 0, 1.0); + put_face(&catalog, 2, 0, 0.99); + + recluster(&catalog, TEST_MODEL, dr_face::DEFAULT_MERGE_PROBABILITY).unwrap(); + let stranger = faces::people(catalog.connection()).unwrap()[0].id; + faces::set_ignored(catalog.connection(), stranger, true).unwrap(); + + // The same person turns up in a third photograph. + put_face(&catalog, 3, 0, 0.98); + recluster(&catalog, TEST_MODEL, dr_face::DEFAULT_MERGE_PROBABILITY).unwrap(); + + let after = faces::people(catalog.connection()).unwrap(); + assert_eq!(after.len(), 1, "a new face made a second group: {after:?}"); + assert!(after[0].ignored); + assert_eq!(after[0].suggested_faces, 3); + } + + /// The other half of the promise: bringing them back really does. + #[test] + fn bringing_a_group_back_makes_it_ordinary_again() { + let catalog = catalog_with(3); + put_face(&catalog, 1, 0, 1.0); + put_face(&catalog, 2, 0, 0.99); + recluster(&catalog, TEST_MODEL, dr_face::DEFAULT_MERGE_PROBABILITY).unwrap(); + + let id = faces::people(catalog.connection()).unwrap()[0].id; + faces::set_ignored(catalog.connection(), id, true).unwrap(); + faces::set_ignored(catalog.connection(), id, false).unwrap(); + + recluster(&catalog, TEST_MODEL, dr_face::DEFAULT_MERGE_PROBABILITY).unwrap(); + let after = faces::people(catalog.connection()).unwrap(); + assert_eq!(after.len(), 1); + assert!(!after[0].ignored); + } }