From a437363bd6763fdb390ad64eec5d2e8d29ffdc37 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Sun, 20 Sep 2026 13:00:59 +0200 Subject: [PATCH] Schema V20: put the mis-spelled run markers right MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The markers the previous commit stops writing are already in the catalogs — 2 on the desktop, 429 on the tablet — and in the shards both have exchanged. Renaming them to the faces' own id with a fresh time is what makes the export send each image again, under an entry newer than the empty one `held_model` would otherwise pick. Where the old write had inserted its marker beside the right one, the wrong one goes and the right one is refreshed for the same reason: its entry in the shards is older than the empty one. Images V14 left with faces and no marker are not touched. That state is the quality pass's cue, and the fixed write marks them correctly when it reaches them. Checked against copies of both real catalogs: the desktop renames 2, the tablet deletes 429, both in under 200 ms. --- core/dr-catalog/src/schema.rs | 175 +++++++++++++++++++++++++++++++++- 1 file changed, 174 insertions(+), 1 deletion(-) diff --git a/core/dr-catalog/src/schema.rs b/core/dr-catalog/src/schema.rs index e78d407..7fbe885 100644 --- a/core/dr-catalog/src/schema.rs +++ b/core/dr-catalog/src/schema.rs @@ -15,7 +15,7 @@ use rusqlite::Connection; use crate::error::CatalogError; /// Schema version this build writes and understands. -pub const SCHEMA_VERSION: i64 = 19; +pub const SCHEMA_VERSION: i64 = 20; /// Apply migrations up to [`SCHEMA_VERSION`]. /// @@ -188,9 +188,98 @@ pub fn migrate(conn: &Connection) -> Result { tx.commit()?; } + if from < 20 { + let tx = conn.unchecked_transaction()?; + v20_markers_name_the_detector_that_found_the_faces(&tx)?; + tx.pragma_update(None, "user_version", 20)?; + tx.commit()?; + } + Ok(from) } +// V20 -- TRACES: FR-CAT-7 +// +// Run markers that named the wrong detector, put right. +// +// `faces::record_updates` -- the write behind the quality, eye and crop +// passes -- re-marked an image under the pipeline the pass ran as, while +// the faces it had updated kept the id of the detector that found them. +// A marker of `scrfd_10g+w600k_mbf` over faces spelled `w600k_mbf` reads, +// to every consumer, as the thorough detector having examined the image: +// the upgrade repair skips it, and `face_shard::export_to_shards` selects +// its faces by the marker's id, finds none, and tells every other device +// that the thorough detector found nothing there. The desktop's shard index +// held 54 such entries over photographs with named faces, and the tablet's +// eye pass over faces it had adopted from the desktop had made 430 more. +// +// The write is fixed to keep the marker under the faces' own id. This puts +// the markers already written right, with a fresh time so the export sends +// each image again under an entry newer than the empty one -- which is what +// `held_model` orders by. Where the right marker is still there beside the +// wrong one (the old write inserted rather than replaced), the wrong one +// goes and the right one is refreshed for the same reason: its entry in +// the shards is older than the empty one, and a device that has neither +// would take the empty one. An image V14 left with faces and no marker at +// all is not touched: that state is the quality pass's cue, and the fixed +// write marks it correctly when the pass reaches it. +// +// Restated in Rust rather than SQL because the embedder half of a pipeline +// id is `faces::embedder_sql`, which this must agree with. +fn v20_markers_name_the_detector_that_found_the_faces(tx: &Connection) -> Result<(), CatalogError> { + let fi = crate::faces::embedder_sql("face_index.model_id"); + let f = crate::faces::embedder_sql("f.model_id"); + // A marker is wrong when the image holds faces of its embedder under + // another id. First the wrong ones that sit beside a right one -- the + // update below would collide with it -- then the rest are renamed. + let wrong = format!( + "EXISTS (SELECT 1 FROM faces f + WHERE f.image_id = face_index.image_id + AND {f} = {fi} + AND f.model_id != face_index.model_id)" + ); + let found_by = format!( + "(SELECT MIN(f.model_id) FROM faces f + WHERE f.image_id = face_index.image_id AND {f} = {fi})" + ); + let now = crate::faces::now_secs(); + tx.execute( + &format!( + "UPDATE face_index + SET indexed_at = ?1 + WHERE model_id = {found_by} + AND EXISTS (SELECT 1 FROM face_index w + WHERE w.image_id = face_index.image_id + AND w.model_id != face_index.model_id + AND {} = {fi})", + crate::faces::embedder_sql("w.model_id") + ), + [now], + )?; + tx.execute( + &format!( + "DELETE FROM face_index + WHERE {wrong} + AND EXISTS (SELECT 1 FROM face_index o + WHERE o.image_id = face_index.image_id + AND o.model_id = {found_by})" + ), + [], + )?; + tx.execute( + &format!( + "UPDATE face_index + SET model_id = {found_by}, + faces_found = (SELECT COUNT(*) FROM faces f + WHERE f.image_id = face_index.image_id AND {f} = {fi}), + indexed_at = ?1 + WHERE {wrong}" + ), + [now], + )?; + Ok(()) +} + /// The seven columns V16 adds to `faces`, in the order the readers name them. /// /// Named once because three places have to agree on them: this migration, @@ -1854,6 +1943,90 @@ mod tests { assert_eq!(faces, 2); } + #[test] + fn v20_renames_markers_to_the_detector_that_found_the_faces() { + let c = mem(); + c.pragma_update(None, "user_version", 0).unwrap(); + migrate(&c).unwrap(); + c.execute( + "INSERT INTO roots(id, kind, label) VALUES (1, 'local', 'test')", + [], + ) + .unwrap(); + c.execute( + "INSERT INTO images(id, root_id, source_ref, added_at) + VALUES (1,1,'a',0),(2,1,'b',0),(3,1,'c',0),(4,1,'d',0),(5,1,'e',0)", + [], + ) + .unwrap(); + // 1: the desktop's case -- old faces, re-marked as thorough. + // 2: the tablet's case -- adopted thorough faces, re-marked int8, + // and the right marker still beside it (refreshed, so it is + // exported again over the empty entry). + // 3: right already. 4: examined and empty. 5: V14's state, faces + // and no marker. + for (image, model) in [ + (1, "scrfd_10g+w600k_mbf"), + (2, "scrfd_10g_i8+w600k_mbf"), + (2, "scrfd_10g+w600k_mbf"), + (3, "scrfd_10g+w600k_mbf"), + (4, "scrfd_10g+w600k_mbf"), + ] { + c.execute( + "INSERT INTO face_index(image_id, model_id, indexed_at, faces_found, source_edge) + VALUES (?1, ?2, 100, 0, 6000)", + rusqlite::params![image, model], + ) + .unwrap(); + } + for (image, model) in [ + (1, "w600k_mbf"), + (1, "w600k_mbf"), + (2, "scrfd_10g+w600k_mbf"), + (3, "scrfd_10g+w600k_mbf"), + (5, "w600k_mbf"), + ] { + c.execute( + "INSERT INTO faces + (image_id, x, y, w, h, landmarks, detector_confidence, embedding, + crop_px, model_id, detected_at) + VALUES (?1, 0.1, 0.1, 0.2, 0.2, X'00', 0.9, X'00', 180.0, ?2, 0)", + rusqlite::params![image, model], + ) + .unwrap(); + } + c.pragma_update(None, "user_version", 19).unwrap(); + + migrate(&c).unwrap(); + + let markers: Vec<(i64, String, i64, bool)> = c + .prepare( + "SELECT image_id, model_id, faces_found, indexed_at > 100 + FROM face_index ORDER BY image_id, model_id", + ) + .unwrap() + .query_map([], |r| Ok((r.get(0)?, r.get(1)?, r.get(2)?, r.get(3)?))) + .unwrap() + .map(Result::unwrap) + .collect(); + assert_eq!( + markers, + vec![ + (1, "w600k_mbf".to_string(), 2, true), + (2, "scrfd_10g+w600k_mbf".to_string(), 0, true), + (3, "scrfd_10g+w600k_mbf".to_string(), 0, false), + (4, "scrfd_10g+w600k_mbf".to_string(), 0, false), + ] + ); + // Re-enterable: nothing left to rename. + c.pragma_update(None, "user_version", 19).unwrap(); + migrate(&c).unwrap(); + let n: i64 = c + .query_row("SELECT count(*) FROM face_index", [], |r| r.get(0)) + .unwrap(); + assert_eq!(n, 4); + } + #[test] fn job_uniqueness_coalesces_rather_than_duplicating() { let c = mem();