diff --git a/ui/dr-ui/src/faces.rs b/ui/dr-ui/src/faces.rs index 25fd06b..029c991 100644 --- a/ui/dr-ui/src/faces.rs +++ b/ui/dr-ui/src/faces.rs @@ -506,21 +506,31 @@ 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. + // Anything the user has ruled on anchors, and there are three ways of + // ruling — only the first of which is obvious. // - // 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. + // A **confirmation** is the plain case. **Setting a group aside** is one + // too, and the faces it covers are only ever suggestions, so anchoring + // confirmations alone let every ignored group scatter into fresh unnamed + // groups that were not ignored, and the strangers came straight back. + // + // And so is **giving a group a name**. That was the omission that did the + // most damage, because it is silent. Naming a cluster does not confirm its + // faces — they stay suggestions — so the next Regroup cut them loose, + // regrouped them into a brand new person, and left the named one holding + // nothing. `prune_empty_unnamed` will not remove it, because it has a name. + // Name the new group the same thing and it happens again. That is how one + // library came to hold sixteen people called Catherine, fourteen of them + // empty, with her faces split across the two that were not. + // + // A name is a judgement about *this group* (FR-CULL-12), exactly as an + // ignore is. Anchoring them all also does one better: a newly indexed face + // that matches a named person now merges *into* them rather than arriving + // as a stranger. let mut confirmed = std::collections::HashMap::new(); for p in faces::people(conn)? { - // Suggestions included exactly when the group was set aside. - for f in faces::for_person(conn, p.id, p.ignored)? { + let ruled_on = p.ignored || !p.name.trim().is_empty(); + for f in faces::for_person(conn, p.id, ruled_on)? { confirmed.insert(f.id, p.id); } } @@ -1204,4 +1214,60 @@ mod tests { assert_eq!(after.len(), 1); assert!(!after[0].ignored); } + + /// Naming a group does not confirm its faces, so before this they were + /// still only suggestions — and the next Regroup cut them loose, built a + /// new person out of them, and left the named one empty. Do that a few + /// times and the rail fills with same-named people holding nothing while + /// the faces sit under whichever one was made last. + #[test] + fn a_named_group_keeps_its_faces_through_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); + let her = people[0].id; + assert_eq!(people[0].suggested_faces, 2); + + // Named, and nothing else — no confirmations, which is what a user who + // types a name and moves on has done. + faces::rename_person(catalog.connection(), her, "Catherine").unwrap(); + + recluster(&catalog, TEST_MODEL, dr_face::DEFAULT_MERGE_PROBABILITY).unwrap(); + + let after = faces::people(catalog.connection()).unwrap(); + assert_eq!( + after.len(), + 1, + "regrouping left a second person behind: {after:?}" + ); + assert_eq!(after[0].id, her); + assert_eq!(after[0].name, "Catherine"); + assert_eq!( + after[0].suggested_faces, 2, + "the named group lost the faces it was named for" + ); + } + + /// And a face found later joins the person it matches rather than arriving + /// as somebody new — the same benefit anchoring gives an ignored group. + #[test] + fn a_new_face_joins_a_named_person_rather_than_starting_a_rival() { + 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 her = faces::people(catalog.connection()).unwrap()[0].id; + faces::rename_person(catalog.connection(), her, "Catherine").unwrap(); + + 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 second Catherine appeared: {after:?}"); + assert_eq!(after[0].suggested_faces, 3); + } }