Forget the face runs made on proxies too small to see a face
The floor added in the previous commit stops this happening again; it does nothing about the 1,824 images in the reference library that already carry a face_index row written against a proxy of 1024 or less. Those rows are why the damage is permanent rather than merely past. The work list is "images with no row for this model", so an image examined against a 1024px proxy -- 0.078 faces per image, nine in ten finding nothing -- is indistinguishable from one examined properly, and no later pass will ever offer it to the detector again. V12 deletes exactly those markers, and nothing else. The faces those runs did find stay in place and keep drawing the People screen until a better pass replaces them, and record_detections re-attaches the user's confirmed names across that replacement by box overlap, so a library somebody has spent an evening naming does not lose that evening. The cost is a re-fetch of the affected images. Deleting the marker rather than teaching the work-list query to select on source_edge, which was the other option and is worse. A standing `source_edge < floor` predicate never lets go: an image whose largest embedded preview is genuinely smaller than the floor would be re-fetched on every sweep for ever, because the next pass cannot do any better than the last one did. A one-off deletion gives each affected image exactly one more attempt through the good path and then lets the ordinary "has a row" rule settle it. The threshold is written out in the SQL instead of referring to dr_face::MIN_DETECT_EDGE. A migration has to keep meaning what it meant when it ran; binding it to a constant someone may raise later would quietly change what an old catalog gets migrated to. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -15,7 +15,7 @@ use rusqlite::Connection;
|
|||||||
use crate::error::CatalogError;
|
use crate::error::CatalogError;
|
||||||
|
|
||||||
/// Schema version this build writes and understands.
|
/// Schema version this build writes and understands.
|
||||||
pub const SCHEMA_VERSION: i64 = 11;
|
pub const SCHEMA_VERSION: i64 = 12;
|
||||||
|
|
||||||
/// Apply migrations up to [`SCHEMA_VERSION`].
|
/// Apply migrations up to [`SCHEMA_VERSION`].
|
||||||
///
|
///
|
||||||
@@ -105,6 +105,13 @@ pub fn migrate(conn: &Connection) -> Result<i64, CatalogError> {
|
|||||||
tx.commit()?;
|
tx.commit()?;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if from < 12 {
|
||||||
|
let tx = conn.unchecked_transaction()?;
|
||||||
|
tx.execute_batch(V12)?;
|
||||||
|
tx.pragma_update(None, "user_version", 12)?;
|
||||||
|
tx.commit()?;
|
||||||
|
}
|
||||||
|
|
||||||
Ok(from)
|
Ok(from)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -476,6 +483,32 @@ CREATE TABLE burst_expanded (
|
|||||||
);
|
);
|
||||||
"#;
|
"#;
|
||||||
|
|
||||||
|
const V12: &str = r#"
|
||||||
|
-- TRACES: FR-CULL-8
|
||||||
|
-- Forget the runs that were made against a proxy too small to find a face on.
|
||||||
|
--
|
||||||
|
-- Detection used to accept any proxy, and one of the two sweeps detected on
|
||||||
|
-- the stored 1024px tier. On the reference library that produced 0.078 faces
|
||||||
|
-- per image against 1.82 for the same photographs at 2048 or better -- and
|
||||||
|
-- every one of those runs left a `face_index` row behind saying the image had
|
||||||
|
-- been examined. That row is what makes the damage permanent: the work list is
|
||||||
|
-- "images with no row", so a photograph examined badly is indistinguishable
|
||||||
|
-- from one examined well, and is never offered to a later pass.
|
||||||
|
--
|
||||||
|
-- Deleting the marker is the whole repair, and it is deliberately not a
|
||||||
|
-- deletion of anything else. The `faces` rows those runs found stay exactly
|
||||||
|
-- where they are and keep drawing the People screen until a better pass
|
||||||
|
-- replaces them, and `record_detections` carries the user's confirmed names
|
||||||
|
-- across that replacement by box overlap. So this costs a re-fetch of the
|
||||||
|
-- affected images and loses no work the user has done.
|
||||||
|
--
|
||||||
|
-- The threshold is written out rather than taken from `dr_face::MIN_DETECT_EDGE`
|
||||||
|
-- on purpose. A migration has to keep meaning what it meant on the day it ran;
|
||||||
|
-- binding it to a constant someone may raise later would silently change what
|
||||||
|
-- an old catalog gets migrated to.
|
||||||
|
DELETE FROM face_index WHERE source_edge <= 1024;
|
||||||
|
"#;
|
||||||
|
|
||||||
const V9: &str = r#"
|
const V9: &str = r#"
|
||||||
-- TRACES: FR-CULL-8
|
-- TRACES: FR-CULL-8
|
||||||
-- A record that face detection has *run* on an image, distinct from what it
|
-- A record that face detection has *run* on an image, distinct from what it
|
||||||
@@ -1268,6 +1301,50 @@ mod tests {
|
|||||||
assert_eq!(n, 0, "images must not outlive their root");
|
assert_eq!(n, 0, "images must not outlive their root");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn v12_forgets_runs_made_on_a_proxy_too_small_to_see_a_face() {
|
||||||
|
let c = mem();
|
||||||
|
// Migrate to 11, then seed the state V12 exists to repair: markers
|
||||||
|
// written at the 1024 store tier beside ones written on a real
|
||||||
|
// preview.
|
||||||
|
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)",
|
||||||
|
[],
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
for (image, edge) in [(1, 896), (2, 1024), (3, 1025), (4, 2560)] {
|
||||||
|
c.execute(
|
||||||
|
"INSERT INTO face_index(image_id, model_id, indexed_at, faces_found, source_edge)
|
||||||
|
VALUES (?1, 'm', 0, 0, ?2)",
|
||||||
|
rusqlite::params![image, edge],
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
}
|
||||||
|
c.pragma_update(None, "user_version", 11).unwrap();
|
||||||
|
|
||||||
|
migrate(&c).unwrap();
|
||||||
|
|
||||||
|
let kept: Vec<i64> = c
|
||||||
|
.prepare("SELECT image_id FROM face_index ORDER BY image_id")
|
||||||
|
.unwrap()
|
||||||
|
.query_map([], |r| r.get(0))
|
||||||
|
.unwrap()
|
||||||
|
.map(Result::unwrap)
|
||||||
|
.collect();
|
||||||
|
// 1024 goes: it is exactly ThumbSize::Large, the tier that produced
|
||||||
|
// the bad runs. 1025 stays, or the floor and the repair disagree
|
||||||
|
// about the same boundary.
|
||||||
|
assert_eq!(kept, vec![3, 4]);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn job_uniqueness_coalesces_rather_than_duplicating() {
|
fn job_uniqueness_coalesces_rather_than_duplicating() {
|
||||||
let c = mem();
|
let c = mem();
|
||||||
|
|||||||
+59
-59
File diff suppressed because one or more lines are too long
Reference in New Issue
Block a user