diff --git a/core/dr-pipeline/src/sidecar.rs b/core/dr-pipeline/src/sidecar.rs index cd616d4..a453988 100644 --- a/core/dr-pipeline/src/sidecar.rs +++ b/core/dr-pipeline/src/sidecar.rs @@ -93,6 +93,9 @@ pub const MAX_RATING: u8 = 5; /// Highest flag code: 0 unflagged, 1 pick, 2 reject. pub const MAX_FLAG: u8 = 2; +/// Highest colour-label code: purple. Mirrors `dr_catalog::rating::label_code`. +pub const MAX_LABEL: u8 = 5; + /// TRACES: FR-CAT-8 | FR-NC-8 /// One image's sidecar: a keyed set of versions. /// @@ -187,6 +190,17 @@ pub struct Version { /// so a value moving between the two stores needs no translation table /// that could drift. pub flag: u8, + /// TRACES: FR-CAT-5 + /// The colour label, `0` for none and `1..=5` red, yellow, green, blue, + /// purple — the catalog's `versions.label` codes, for the reason + /// [`Self::flag`] shares its encoding. + /// + /// Here for the reason the rating is: a label that lived only in the + /// catalog would go with the catalog, and would never reach the + /// photographer's other devices, which learn judgements from this file. + /// A build that predates the key keeps it as an unknown line and writes + /// it back, so an older device passes it on rather than erasing it. + pub label: u8, /// The edit itself: `(op, param) -> value`, non-default values only. pub params: BTreeMap<(String, String), f32>, /// TRACES: FR-DEV-3 | FR-NC-9 @@ -264,6 +278,7 @@ impl Version { // it in the "not yet looked at" state a cull resumes from. rating: 0, flag: 0, + label: 0, params, masks, film, @@ -572,6 +587,10 @@ impl Version { // that never had it. self.rating = merge_judgement(self.rating, remote.rating, remote_wins); self.flag = merge_judgement(self.flag, remote.flag, remote_wins); + // TRACES: FR-CAT-5 + // A label under the same rule: 0 is "none given", so a device that + // never labelled a frame cannot clear another's label. + self.label = merge_judgement(self.label, remote.label, remote_wins); // TRACES: FR-DEV-3f // The film resolves wholesale to the higher revision, like a mask @@ -873,6 +892,9 @@ impl Sidecar { if v.flag > 0 { let _ = writeln!(out, "flag = {}", v.flag); } + if v.label > 0 { + let _ = writeln!(out, "label = {}", v.label); + } // TRACES: FR-DEV-3f // Before the parameters, because it decides what they mean: the // film's exposure slider is a slider on *that stock's* curve. @@ -1068,6 +1090,17 @@ impl Sidecar { Some(value.to_string()); } "flag" => version.flag = value.parse::().unwrap_or(0).min(MAX_FLAG), + // TRACES: FR-CAT-5 + // A code this build does not know reads as none rather than + // being clamped onto purple: a wrong colour is a claim, and + // no colour is only a gap. + "label" => { + version.label = value + .parse::() + .ok() + .filter(|l| *l <= MAX_LABEL) + .unwrap_or(0) + } // TRACES: FR-DEV-8 // Ahead of the `op.param` arm below, which would otherwise try // to read eight numbers as one float and drop the repair with a @@ -2828,6 +2861,37 @@ mod tests { assert_eq!(back.flag, 1); } + #[test] + fn a_label_survives_the_round_trip_and_an_unknown_one_reads_as_none() { + // TRACES: FR-CAT-5 + let mut v = version_of(&EditGraph::default_chain()); + v.label = 3; + let mut sidecar = Sidecar::new(); + sidecar.put(v); + let text = sidecar.to_text(); + assert!(text.contains("label = 3"), "{text}"); + let parsed = Sidecar::parse(&text).expect("valid"); + assert_eq!(parsed.default_version().expect("a version").label, 3); + + let odd = "drsc 1\n\n[version u1]\nname = Default\nrevision = 1\nmodified = 0\n\ + label = 9\n"; + let parsed = Sidecar::parse(odd).expect("valid"); + assert_eq!(parsed.default_version().expect("a version").label, 0); + } + + #[test] + fn an_unlabelled_device_cannot_clear_another_devices_label() { + // TRACES: FR-CAT-5 + let mut local = version_of(&EditGraph::default_chain()); + local.label = 0; + local.revision = 9; + let mut remote = version_of(&EditGraph::default_chain()); + remote.label = 1; + remote.revision = 1; + local.merge(&remote, None); + assert_eq!(local.label, 1); + } + #[test] fn an_unrated_image_writes_no_judgement_lines() { // The non-default rule applied to judgement: a library that has never @@ -2838,6 +2902,7 @@ mod tests { let text = sidecar.to_text(); assert!(!text.contains("rating"), "{text}"); assert!(!text.contains("flag"), "{text}"); + assert!(!text.contains("label"), "{text}"); } #[test] diff --git a/ui/dr-ui/src/library/scan.rs b/ui/dr-ui/src/library/scan.rs index 6127f2c..d1ee2a2 100644 --- a/ui/dr-ui/src/library/scan.rs +++ b/ui/dr-ui/src/library/scan.rs @@ -380,7 +380,14 @@ pub(super) async fn pull_sidecars( continue; }; - match apply_judgement(conn, root_id, path, version.rating, version.flag) { + match apply_judgement( + conn, + root_id, + path, + version.rating, + version.flag, + version.label, + ) { Ok(n) => { applied += n; record_sidecar_read(conn, root_id, path, &entry.validator); @@ -464,6 +471,7 @@ pub(super) fn apply_judgement( sidecar: &str, rating: u8, flag: u8, + label: u8, ) -> Result { let stem = sidecar .rsplit_once('.') @@ -513,14 +521,25 @@ pub(super) fn apply_judgement( // of caution, as `dr_pipeline::sidecar::merge_judgement`. The cost is // the one that rule always carries — clearing a rating does not // propagate. + // + // TRACES: FR-CAT-5 + // The label under the same rule: a sidecar with none leaves this + // device's label alone. let changed = conn .execute( "UPDATE versions SET rating = CASE WHEN ?2 > 0 THEN ?2 ELSE rating END, - flag = CASE WHEN ?3 > 0 THEN ?3 ELSE flag END + flag = CASE WHEN ?3 > 0 THEN ?3 ELSE flag END, + label = CASE WHEN ?4 > 0 THEN ?4 ELSE label END WHERE id = ?1 - AND ((?2 > 0 AND rating <> ?2) OR (?3 > 0 AND flag <> ?3))", - rusqlite::params![version, rating.min(5) as i64, flag.min(2) as i64], + AND ((?2 > 0 AND rating <> ?2) OR (?3 > 0 AND flag <> ?3) + OR (?4 > 0 AND label IS NOT ?4))", + rusqlite::params![ + version, + rating.min(5) as i64, + flag.min(2) as i64, + if label <= 5 { label as i64 } else { 0 } + ], ) .map_err(|e| e.to_string())?; applied += changed; @@ -891,12 +910,27 @@ mod tests { v } + #[test] + fn a_label_from_another_device_reaches_the_catalog_and_none_clears_nothing() { + // TRACES: FR-CAT-5 + let cat = library(&["2026/a.CR2"]); + assert_eq!(apply_judgement(cat.connection(), 1, "2026/a.drsc", 0, 0, 4).unwrap(), 1); + let label = |cat: &Catalog| -> Option { + cat.connection() + .query_row("SELECT label FROM versions WHERE is_default = 1", [], |r| r.get(0)) + .unwrap() + }; + assert_eq!(label(&cat), Some(4)); + apply_judgement(cat.connection(), 1, "2026/a.drsc", 3, 0, 0).unwrap(); + assert_eq!(label(&cat), Some(4), "a sidecar with no label left it alone"); + } + /// The regression, in one line: a rating in a sidecar reaches the grid. /// Before this there was no path by which it could. #[test] fn a_rating_from_another_device_reaches_the_catalog() { let cat = library(&["2026/a.CR2"]); - let n = apply_judgement(cat.connection(), 1, "2026/a.drsc", 4, 1).unwrap(); + let n = apply_judgement(cat.connection(), 1, "2026/a.drsc", 4, 1, 0).unwrap(); assert_eq!(n, 1); assert_eq!(judgements(&cat), vec![("2026/a.CR2".to_string(), 4, 1)]); @@ -908,7 +942,7 @@ mod tests { #[test] fn both_halves_of_a_raw_and_jpeg_pair_are_judged() { let cat = library(&["2026/a.CR2", "2026/a.jpg"]); - let n = apply_judgement(cat.connection(), 1, "2026/a.drsc", 3, 0).unwrap(); + let n = apply_judgement(cat.connection(), 1, "2026/a.drsc", 3, 0, 0).unwrap(); assert_eq!(n, 2); assert_eq!( @@ -926,8 +960,8 @@ mod tests { #[test] fn a_lowered_rating_travels() { let cat = library(&["2026/a.CR2"]); - apply_judgement(cat.connection(), 1, "2026/a.drsc", 4, 0).unwrap(); - apply_judgement(cat.connection(), 1, "2026/a.drsc", 1, 0).unwrap(); + apply_judgement(cat.connection(), 1, "2026/a.drsc", 4, 0, 0).unwrap(); + apply_judgement(cat.connection(), 1, "2026/a.drsc", 1, 0, 0).unwrap(); assert_eq!(judgements(&cat)[0].1, 1); } @@ -937,10 +971,10 @@ mod tests { #[test] fn an_unjudged_sidecar_does_not_erase_a_local_rating() { let cat = library(&["2026/a.CR2"]); - apply_judgement(cat.connection(), 1, "2026/a.drsc", 5, 2).unwrap(); + apply_judgement(cat.connection(), 1, "2026/a.drsc", 5, 2, 0).unwrap(); assert_eq!( - apply_judgement(cat.connection(), 1, "2026/a.drsc", 0, 0).unwrap(), + apply_judgement(cat.connection(), 1, "2026/a.drsc", 0, 0, 0).unwrap(), 0 ); assert_eq!(judgements(&cat), vec![("2026/a.CR2".to_string(), 5, 2)]); @@ -953,11 +987,11 @@ mod tests { fn applying_the_same_judgement_twice_changes_nothing() { let cat = library(&["2026/a.CR2"]); assert_eq!( - apply_judgement(cat.connection(), 1, "2026/a.drsc", 4, 1).unwrap(), + apply_judgement(cat.connection(), 1, "2026/a.drsc", 4, 1, 0).unwrap(), 1 ); assert_eq!( - apply_judgement(cat.connection(), 1, "2026/a.drsc", 4, 1).unwrap(), + apply_judgement(cat.connection(), 1, "2026/a.drsc", 4, 1, 0).unwrap(), 0 ); } @@ -970,7 +1004,7 @@ mod tests { // `_` is LIKE's single-character wildcard, so an unescaped `a_b` stem // would also match `axb`. let cat = library(&["2026/a_b.CR2", "2026/axb.CR2"]); - apply_judgement(cat.connection(), 1, "2026/a_b.drsc", 5, 0).unwrap(); + apply_judgement(cat.connection(), 1, "2026/a_b.drsc", 5, 0, 0).unwrap(); assert_eq!( judgements(&cat), @@ -986,7 +1020,7 @@ mod tests { #[test] fn a_shared_prefix_is_not_a_shared_sidecar() { let cat = library(&["2026/a.CR2", "2026/ab.CR2"]); - apply_judgement(cat.connection(), 1, "2026/a.drsc", 5, 0).unwrap(); + apply_judgement(cat.connection(), 1, "2026/a.drsc", 5, 0, 0).unwrap(); assert_eq!( judgements(&cat), @@ -1004,7 +1038,7 @@ mod tests { fn a_sidecar_with_no_photograph_here_is_not_a_failure() { let cat = library(&["2026/a.CR2"]); assert_eq!( - apply_judgement(cat.connection(), 1, "2026/elsewhere.drsc", 5, 0).unwrap(), + apply_judgement(cat.connection(), 1, "2026/elsewhere.drsc", 5, 0, 0).unwrap(), 0 ); } diff --git a/ui/dr-ui/src/library/sidecar.rs b/ui/dr-ui/src/library/sidecar.rs index cfc8abf..35df69d 100644 --- a/ui/dr-ui/src/library/sidecar.rs +++ b/ui/dr-ui/src/library/sidecar.rs @@ -34,8 +34,8 @@ pub struct SidecarWrite { /// amendment rather than a replacement. #[derive(Debug, Clone)] pub enum Amendment { - /// A star rating and a pick/reject flag — the cull. - Judgement { rating: u8, flag: u8 }, + /// A star rating, a pick/reject flag and a colour label — the cull. + Judgement { rating: u8, flag: u8, label: u8 }, /// TRACES: FR-DEV-6 /// Copied develop settings, applied within `scope`. /// @@ -302,9 +302,14 @@ pub(super) fn amend(base: dr_pipeline::Sidecar, w: &SidecarWrite) -> dr_pipeline // paste must spare, the unknown keys of an operation this build lacks — // survives because it was read from the file and is written back. match &w.amendment { - Amendment::Judgement { rating, flag } => { + Amendment::Judgement { + rating, + flag, + label, + } => { version.rating = *rating; version.flag = *flag; + version.label = *label; } Amendment::Settings { preset, @@ -624,7 +629,11 @@ mod tests { SidecarWrite { image_path: "PhotosRaw/incoming/a.CR2".to_string(), version_uuid: uuid.to_string(), - amendment: Amendment::Judgement { rating, flag: 0 }, + amendment: Amendment::Judgement { + rating, + flag: 0, + label: 0, + }, } } diff --git a/ui/dr-ui/src/library_ui/ratings_keywords.rs b/ui/dr-ui/src/library_ui/ratings_keywords.rs index 29996ce..79a4b98 100644 --- a/ui/dr-ui/src/library_ui/ratings_keywords.rs +++ b/ui/dr-ui/src/library_ui/ratings_keywords.rs @@ -492,7 +492,7 @@ fn collect_sidecar_writes( .collect::>() .join(","); let sql = format!( - "SELECT i.source_ref, v.uuid, v.rating, v.flag + "SELECT i.source_ref, v.uuid, v.rating, v.flag, coalesce(v.label, 0) FROM images i JOIN versions v ON v.image_id = i.id AND v.is_default = 1 WHERE i.id IN ({placeholders})" @@ -512,6 +512,7 @@ fn collect_sidecar_writes( amendment: library::Amendment::Judgement { rating: r.get::<_, i64>(2)? as u8, flag: r.get::<_, i64>(3)? as u8, + label: r.get::<_, i64>(4)?.clamp(0, 5) as u8, }, }) });