Merge master: pluggable storage, and a name that anchors
Conflicts were docs/traceability.md alone, and it is generated — so it was regenerated rather than hand-merged. dr-face was untouched on the other side; ui/dr-ui/src/faces.rs and identity_ui.rs auto-merged, the first around recluster's anchoring and the second around load_faces. Worth recording because the two branches met on the same problem from different ends. Master's "Let a name hold a group together" is the fix for the sixteen Catherines — fourteen of them empty — that this branch found while measuring the library and reported without fixing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
+78
-12
@@ -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);
|
||||
}
|
||||
}
|
||||
@@ -1178,4 +1188,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);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user