Fold a photograph's rival default versions back into one

Picking the newer of two default versions stopped the wrong edit being shown,
but it did not close the split: the losing version stayed in the file, and a
device holding disjoint work — a crop made here, an exposure change made there
— still contributed only one of the two.

Worse, the next write made it larger. `amend` looks its version up by uuid,
neither of the two was ours, so the miss minted a *third* `default = 1` block
and the file grew one rival per device per photograph.

`Sidecar::fuse_default_versions` folds them down. The version with the highest
`(revision, modified)` is the accumulator and every other default is merged
into it as the remote, which is what makes the fold order-independent —
`Version::merge` raises its own revision to `max + 1` as it goes, so merging a
chain in ascending order stops being ascending after the first step and a third
device would be dropped. Contested values resolve to the winner, disjoint keys
survive from both sides because the merge is key-wise, and ratings come across
under `merge_judgement`, so a device that never judged the frame cannot erase
one that did.

The result is a function of the file's bytes alone, so two devices that fuse
independently reach the same document and converge instead of overwriting each
other.

Called wherever a sidecar is parsed:

- `amend`, with the write's own uuid, so the fold lands on the identity this
  device is about to use and the lookup below it hits instead of missing.
- `spawn_sidecar_fetch`, so opening a photograph shows everything done to it
  rather than whichever half won.
- `drain_one`, because `merge_into` reconciles by uuid and would otherwise
  publish the split rather than resolve it.
- `presets::load_local` and `save_local` — a local sidecar's folder may be
  synced by something else entirely, and gets the same split.

A file with one default under the expected uuid comes back byte-identical, so
this costs nothing on the ordinary write and no sidecar is uploaded merely for
having been read.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-09-05 16:13:53 +02:00
co-authored by Claude Opus 5
parent 98fcf8e98e
commit 601c984894
4 changed files with 432 additions and 37 deletions
+243 -2
View File
@@ -624,8 +624,8 @@ impl Sidecar {
/// This used to answer with the first `is_default` in map order, and map
/// order is *uuid* order. That is only ever right when exactly one version
/// claims to be the default, and two devices editing one photograph
/// routinely produce two that do, because each mints its own uuid for the
/// same photograph. With two, the answer became a per-photograph
/// routinely produce two that do — see [`Self::fuse_default_versions`] for
/// how they come to exist. With two, the answer became a per-photograph
/// coin toss decided by which random uuid happened to sort lower, and the
/// losing device's afternoon of work simply did not appear.
///
@@ -646,6 +646,101 @@ impl Sidecar {
.or_else(|| self.versions.values().next())
}
/// TRACES: FR-NC-8 | FR-NC-9
/// Collapse every version claiming to be the default onto one identity.
///
/// # The failure this repairs
///
/// A version's uuid is the identity a cross-device merge keys on, and for
/// a long time each device minted its own at random for the same
/// photograph. Two devices editing one frame therefore wrote two
/// `[version]` blocks, both marked `default = 1`, and
/// [`Version::merge`] — which is perfectly capable of combining them —
/// was never handed the pair, because it matches on uuid and the uuids
/// never matched. The two edits simply accumulated, and whichever one
/// [`Self::default_version`] happened to pick was the only one anybody
/// saw.
///
/// Minting the uuid deterministically stops new splits. It does nothing
/// for the files already written, which is what this is for.
///
/// # Why folding into the winner rather than merging in sequence
///
/// [`Version::merge`] raises its own `revision` to `max + 1` as it goes,
/// so merging a chain in ascending order stops being ascending after the
/// first step and the third version would lose to the accumulator. Folding
/// *into* the highest `(revision, modified)` avoids the question: every
/// other version is merged in as the remote and loses every contested
/// value, while the disjoint case — a key only it holds — is taken
/// regardless of who wins, which is the whole point of a key-wise merge.
/// Ratings come across under [`merge_judgement`], so a device that never
/// judged the frame cannot erase one that did.
///
/// The result is the same on every device, because it is a function of the
/// file's contents alone. Two devices that fuse independently and upload
/// converge rather than ping-ponging.
///
/// # `canonical`
///
/// The uuid the result should carry — the caller's derived identity for
/// this photograph (`dr_catalog::rating::derived_version_uuid`). Passing
/// `None` folds onto the lexicographically smallest of the defaults, which
/// is still device-independent and is the rule
/// `dr_catalog::keywords::fuse_duplicates` already uses for the same shape
/// of problem.
///
/// A rename happens even when there is nothing to fuse: a lone default
/// still sitting under a randomly minted uuid has to move to the derived
/// one, or the next write finds no match and adds a *third* version.
///
/// Returns the uuid the default now carries, or `None` if there was no
/// default version to fuse.
pub fn fuse_default_versions(&mut self, canonical: Option<&str>) -> Option<String> {
let mut defaults: Vec<String> = self
.versions
.values()
.filter(|v| v.is_default)
.map(|v| v.uuid.clone())
.collect();
defaults.sort();
let winner_uuid = self
.versions
.values()
.filter(|v| v.is_default)
.max_by_key(|v| (v.revision, v.modified))
.map(|v| v.uuid.clone())?;
// A canonical uuid that already names a *different*, non-default
// version is a virtual copy (FR-CAT-12), and renaming onto it would
// delete it. Vanishingly unlikely — a derived uuid is structurally
// distinct from a randomly minted one — but silently destroying a
// named edit is precisely the loss this module exists to prevent, so
// the rename is declined rather than risked.
let target = match canonical {
Some(c) if !defaults.iter().any(|u| u == c) && self.versions.contains_key(c) => {
log::warn!(
"not renaming the default version onto {c}: another version already has it"
);
winner_uuid.clone()
}
Some(c) => c.to_string(),
None => defaults
.first()
.cloned()
.unwrap_or_else(|| winner_uuid.clone()),
};
let mut winner = self.versions.remove(&winner_uuid)?;
for uuid in defaults.iter().filter(|u| **u != winner_uuid) {
if let Some(loser) = self.versions.remove(uuid) {
winner.merge(&loser, None);
}
}
winner.uuid = target.clone();
self.versions.insert(target.clone(), winner);
Some(target)
}
/// Insert or replace a version.
pub fn put(&mut self, version: Version) {
self.versions.insert(version.uuid.clone(), version);
@@ -2457,4 +2552,150 @@ mod split_defaults {
]);
assert_eq!(s.default_version().unwrap().rating, 3);
}
/// Fusing is what makes the two converge rather than merely choosing
/// between them: a parameter only the *losing* version holds survives,
/// because the merge is key-wise.
#[test]
fn fusing_keeps_the_disjoint_work_of_both_devices() {
let mut old = default_version("0545c20a", 1, 100, 4);
old.params
.insert(("exposure".into(), "exposure".into()), 0.75);
let mut new = default_version("679fe872", 2, 200, 0);
new.params
.insert(("saturation".into(), "saturation".into()), 0.5);
let mut s = with(vec![old, new]);
let uuid = s.fuse_default_versions(None).unwrap();
assert_eq!(s.versions.len(), 1, "the split must be closed");
let v = &s.versions[&uuid];
assert_eq!(
v.params.get(&("exposure".into(), "exposure".into())),
Some(&0.75),
"the older device's exposure was dropped"
);
assert_eq!(
v.params.get(&("saturation".into(), "saturation".into())),
Some(&0.5),
"the newer device's saturation was dropped"
);
// A judgement the loser holds and the winner does not is adopted, for
// the reason `merge_judgement` gives: a zero is "never judged".
assert_eq!(v.rating, 4);
}
/// A contested value resolves to the higher revision, not to whichever
/// uuid sorted first.
#[test]
fn fusing_resolves_a_contested_value_by_revision() {
let mut old = default_version("aaaa", 9, 100, 0);
old.params
.insert(("exposure".into(), "exposure".into()), 0.75);
let mut new = default_version("bbbb", 1, 999_999, 0);
new.params
.insert(("exposure".into(), "exposure".into()), -2.0);
let mut s = with(vec![old, new]);
let uuid = s.fuse_default_versions(None).unwrap();
assert_eq!(
s.versions[&uuid]
.params
.get(&("exposure".into(), "exposure".into())),
Some(&0.75),
"the higher revision must win"
);
}
/// Every device must reach the same file from the same input, or two of
/// them fuse and upload forever without converging.
#[test]
fn fusing_is_the_same_on_every_device() {
let versions = vec![
default_version("cccc", 1, 100, 2),
default_version("aaaa", 3, 300, 0),
default_version("bbbb", 2, 200, 0),
];
let mut one = with(versions.clone());
// The other device parsed the same file, so it holds the same set in a
// different insertion order.
let mut two = with(versions.into_iter().rev().collect());
one.fuse_default_versions(None);
two.fuse_default_versions(None);
assert_eq!(one.to_text(), two.to_text());
assert_eq!(one.versions.len(), 1);
}
/// Three or more versions must all fold in. The accumulator's revision
/// climbs as it merges, so folding a chain in ascending order would drop
/// everything after the second — which is why the fold goes into the
/// winner instead.
#[test]
fn a_third_device_is_not_lost_to_the_accumulators_rising_revision() {
let mut a = default_version("aaaa", 1, 100, 0);
a.params.insert(("a".into(), "a".into()), 1.0);
let mut b = default_version("bbbb", 2, 200, 0);
b.params.insert(("b".into(), "b".into()), 2.0);
let mut c = default_version("cccc", 3, 300, 0);
c.params.insert(("c".into(), "c".into()), 3.0);
let mut s = with(vec![a, b, c]);
let uuid = s.fuse_default_versions(None).unwrap();
let v = &s.versions[&uuid];
for key in ["a", "b", "c"] {
assert!(
v.params.contains_key(&(key.to_string(), key.to_string())),
"{key} was dropped by the fold"
);
}
}
/// The rename half. A lone default under a randomly minted uuid has to
/// move onto the derived one, or the next write matches nothing and adds a
/// third version instead of amending this one.
#[test]
fn a_lone_default_is_renamed_onto_the_canonical_uuid() {
let mut s = with(vec![default_version("random-v4", 1, 100, 3)]);
let uuid = s.fuse_default_versions(Some("derived")).unwrap();
assert_eq!(uuid, "derived");
assert_eq!(s.versions.len(), 1);
assert_eq!(s.versions["derived"].rating, 3, "the edit went with it");
}
/// A virtual copy (FR-CAT-12) that happens to hold the canonical uuid must
/// not be overwritten by the rename.
#[test]
fn a_named_version_is_not_destroyed_by_the_rename() {
let mut copy = default_version("derived", 1, 100, 5);
copy.is_default = false;
copy.name = "For print".to_string();
let mut s = with(vec![default_version("random-v4", 1, 100, 3), copy]);
let uuid = s.fuse_default_versions(Some("derived")).unwrap();
assert_eq!(uuid, "random-v4", "the rename must be declined");
assert_eq!(s.versions.len(), 2);
assert_eq!(s.versions["derived"].name, "For print");
}
/// Fusing a file that never needed it must not rewrite it — a sidecar that
/// changes on every read is a sidecar that uploads on every read.
#[test]
fn fusing_a_healthy_sidecar_changes_nothing() {
let mut s = with(vec![default_version("only", 4, 400, 2)]);
let before = s.to_text();
s.fuse_default_versions(None);
assert_eq!(s.to_text(), before);
}
/// Nothing to fuse is not a failure — most photographs have never been
/// edited on any device.
#[test]
fn an_empty_sidecar_has_no_default_to_fuse() {
let mut s = Sidecar::new();
assert_eq!(s.fuse_default_versions(Some("derived")), None);
assert!(s.versions.is_empty());
}
}
+31 -31
View File
File diff suppressed because one or more lines are too long
+142 -3
View File
@@ -744,9 +744,23 @@ pub enum SidecarMessage {
fn amend(base: dr_pipeline::Sidecar, w: &SidecarWrite) -> dr_pipeline::Sidecar {
let mut sidecar = base;
// TRACES: FR-NC-8 | FR-NC-9
// Close any split this file already carries, *before* looking for our own
// version, and onto the uuid this write is about to use.
//
// Every device used to mint its own uuid for the same photograph, so a
// frame edited on two of them holds two `default = 1` blocks and the
// lookup below misses both — adding a third rather than amending either.
// Fusing first folds them into one under `w.version_uuid`, which turns the
// miss into a hit and makes this an amendment of the other device's work
// instead of a rival to it.
//
// Idempotent: a file with one default and the right uuid is returned
// byte-identical, so this costs nothing on the ordinary write.
sidecar.fuse_default_versions(Some(&w.version_uuid));
// Amend the version this write belongs to, creating it if the file did
// not have one. The uuid comes from the catalog, so the same photograph
// keeps one identity across devices (FR-NC-8).
// not have one — a photograph nobody has edited anywhere.
let mut version = sidecar
.versions
.get(&w.version_uuid)
@@ -1020,6 +1034,18 @@ async fn drain_one(
}
}
// TRACES: FR-NC-8 | FR-NC-9
// `merge_into` reconciles version by version *by uuid*, so two devices'
// independently minted defaults pass straight through it and both land in
// what is about to be uploaded. Fusing here is what stops the outbox from
// publishing the split rather than resolving it.
//
// No canonical uuid: this entry may have been queued by a build that had
// not derived one yet, and the smallest uuid is device-independent, which
// is all convergence needs. The next write from either device moves it
// onto the derived identity.
local.fuse_default_versions(None);
backend
.put(&path, local.to_text().into_bytes(), None)
.await
@@ -2010,7 +2036,19 @@ pub fn spawn_sidecar_fetch(
let text = String::from_utf8_lossy(&bytes).into_owned();
let parsed = match dr_pipeline::Sidecar::parse(&text) {
Ok(s) => Some(s),
Ok(mut s) => {
// TRACES: FR-NC-8 | FR-NC-9
// Opening a photograph must show everything that has been
// done to it, not whichever of two split default versions
// happens to win. Fused in memory with no canonical uuid
// to impose — this is a read, and the write path is where
// the identity is decided.
//
// The fused document is what gets cached below, so the
// next offline open sees the union too.
s.fuse_default_versions(None);
Some(s)
}
Err(e) => {
log::warn!("sidecar at {} is unreadable ({e})", path.as_str());
None
@@ -6821,3 +6859,104 @@ mod tests {
assert_eq!(total_images_scoped(&catalog, None, &f).unwrap(), 3);
}
}
/// TRACES: FR-NC-8 | FR-NC-9
/// What a write does to a sidecar another device has already edited.
#[cfg(test)]
mod amending_across_devices {
use super::*;
fn judgement(uuid: &str, rating: u8) -> SidecarWrite {
SidecarWrite {
image_path: "PhotosRaw/incoming/a.CR2".to_string(),
version_uuid: uuid.to_string(),
amendment: Amendment::Judgement { rating, flag: 0 },
}
}
fn version(uuid: &str, revision: u64, modified: i64, rating: u8) -> dr_pipeline::Version {
dr_pipeline::sidecar::Version {
uuid: uuid.to_string(),
name: "Default".to_string(),
is_default: true,
revision,
modified,
rating,
..Default::default()
}
}
/// The regression. A sidecar already holding two independently minted
/// defaults used to gain a *third* on the next write, because the lookup
/// is by uuid and neither of the two was ours.
#[test]
fn a_write_onto_a_split_sidecar_does_not_add_a_third_version() {
let mut base = dr_pipeline::Sidecar::new();
base.put(version("tablet-uuid", 1, 100, 4));
base.put(version("laptop-uuid", 2, 200, 1));
let out = amend(base, &judgement("derived-uuid", 5));
assert_eq!(out.versions.len(), 1, "the split must be closed, not grown");
assert!(out.versions.contains_key("derived-uuid"));
assert_eq!(out.versions["derived-uuid"].rating, 5);
}
/// The other device's work has to survive the fold, or closing the split
/// would be the same data loss by a different route.
#[test]
fn the_other_devices_edit_survives_the_write() {
let mut theirs = version("tablet-uuid", 3, 300, 4);
theirs
.params
.insert(("exposure".into(), "exposure".into()), 0.75);
let mut base = dr_pipeline::Sidecar::new();
base.put(theirs);
// Ours is a rating, which touches no parameter at all.
let out = amend(base, &judgement("derived-uuid", 2));
let v = &out.versions["derived-uuid"];
assert_eq!(
v.params.get(&("exposure".into(), "exposure".into())),
Some(&0.75),
"the tablet's exposure was dropped by our rating"
);
assert_eq!(v.rating, 2, "and our own judgement did not land");
}
/// A sidecar this device has already written must not be disturbed: the
/// ordinary case is one default under the right uuid, and fusing it has to
/// be a no-op beyond the amendment itself.
#[test]
fn the_ordinary_write_is_unaffected() {
let mut base = dr_pipeline::Sidecar::new();
base.put(version("derived-uuid", 7, 700, 3));
let out = amend(base, &judgement("derived-uuid", 5));
assert_eq!(out.versions.len(), 1);
assert_eq!(out.versions["derived-uuid"].rating, 5);
assert_eq!(
out.versions["derived-uuid"].revision, 8,
"one bump for one edit"
);
}
/// A virtual copy is not a rival default and must be left where it is.
#[test]
fn a_named_version_is_not_folded_into_the_default() {
let mut copy = version("for-print", 4, 400, 5);
copy.is_default = false;
copy.name = "For print".to_string();
let mut base = dr_pipeline::Sidecar::new();
base.put(version("tablet-uuid", 1, 100, 4));
base.put(copy);
let out = amend(base, &judgement("derived-uuid", 2));
assert_eq!(out.versions.len(), 2);
assert_eq!(out.versions["for-print"].name, "For print");
assert_eq!(out.versions["for-print"].rating, 5);
}
}
+16 -1
View File
@@ -284,7 +284,16 @@ pub fn load_local(image: &Path) -> Option<Sidecar> {
let path = local_sidecar_path(image);
let text = std::fs::read_to_string(&path).ok()?;
match Sidecar::parse(&text) {
Ok(s) => Some(s),
Ok(mut s) => {
// TRACES: FR-NC-8 | FR-NC-9
// A local sidecar is only local to *this* app. The folder it sits
// in may well be synced by something else — a Nextcloud desktop
// client, a git-annex, a memory card carried between machines — so
// it can hold the same two-default split a library sidecar does,
// and opening the photograph must show both halves.
s.fuse_default_versions(None);
Some(s)
}
Err(e) => {
log::warn!("sidecar at {} is unreadable ({e})", path.display());
None
@@ -323,6 +332,12 @@ pub fn save_local(
})?,
};
// TRACES: FR-NC-8 | FR-NC-9
// Fold any split down before choosing which version to amend, so a save
// resolves the two rather than writing into one of them and leaving the
// other to be picked next time.
sidecar.fuse_default_versions(None);
// A local file has no catalog behind it to supply a version identity, so
// the file's own default version is used and one is created if absent.
// Deterministic rather than random: reopening the same photograph must