From 1b8b7998a2375192c1640e66fee346cea2890a6c Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Sat, 29 Aug 2026 09:48:32 +0200 Subject: [PATCH] Let a name hold a group together, the way a confirmation does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sixteen people called Catherine, fourteen of them holding no faces at all, and her actual photographs split across the two that did. That is not a sync fault; it is this function, and it has been quietly doing it on every Regroup. Naming a cluster does not confirm its faces. They stay *suggestions* — and only confirmed faces anchored here, so the next pass cut them loose, regrouped them into a brand new person, and left the named one holding nothing. `prune_empty_unnamed` will not clean that up, because it has a name. Name the new group the same thing, and it happens again. Repeat over a few sessions and you have sixteen of her. A name is a judgement about *this group*, of exactly the kind FR-CULL-12 says travels and inference does not — the same argument that already anchors a group the user set aside. So all three kinds of ruling anchor now: confirmed, ignored, and named. It also fixes the quieter half of the same fault. A face indexed later that matches a named person now merges *into* them, rather than arriving as a rival group the user has to name all over again. Two tests, and the first fails without the change — it reports "Catherine" finishing the pass with zero faces while a fresh unnamed person holds the two she was named for. This does not retro-fit an existing library: the fourteen empty Catherines stay until they are merged by hand, and the two holding faces are separate identities that only the user can say are one person. What it stops is making more. Co-Authored-By: Claude Opus 5 (1M context) --- ui/dr-ui/src/faces.rs | 90 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 78 insertions(+), 12 deletions(-) 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); + } }