From 51dba8d4910618cc5815bd3d24d59e26d4ef786b Mon Sep 17 00:00:00 2001 From: DepengWang <2818245+DepengWang@users.noreply.github.com> Date: Fri, 2 Oct 2026 23:12:16 +0800 Subject: [PATCH] fix(sync): stop treating list position and hit counts as conflicts Encrypted sync compared whole records, and every history, dictionary and correction record carried sortIndex: its position in the exporting device's list. History and corrections insert at the front, so each new dictation renumbered every older row on that device. Two devices that had both been used since the last sync, an unequal number of times, therefore disagreed with each other and with the baseline on every shared history row, and the user was asked to pick a side for each one even though the text was identical. Dictionary hit counters did the same per word. The merge now compares what a record says. sortIndex (history, dictionary, corrections) and dictionary hits are left out of the comparison; when both sides hold a dictionary entry the higher hit count is kept, including when a conflict is resolved by choice. Collection order is derived from the records instead of from a device's list: history and corrections newest first, dictionary manual entries newest first ahead of learned entries oldest first, which is how the stores insert. sortIndex stays on the wire as a dense index so older clients keep restoring in order; the incoming value only breaks ties, so rows without a usable createdAt keep their relative order. Real conflicts are unchanged: the same record edited on both sides, and delete against modify, still ask the user. Co-Authored-By: Claude Opus 5.5 --- .../src/cloud_sync_e2ee_documents/merge.rs | 70 ++++++- .../src/cloud_sync_e2ee_documents/tests.rs | 182 ++++++++++++++++++ .../src/cloud_sync_e2ee_documents/validate.rs | 43 +++-- 3 files changed, 277 insertions(+), 18 deletions(-) diff --git a/openless-all/app/crates/openless-core/src/cloud_sync_e2ee_documents/merge.rs b/openless-all/app/crates/openless-core/src/cloud_sync_e2ee_documents/merge.rs index bdc8ac062..3b16379f6 100644 --- a/openless-all/app/crates/openless-core/src/cloud_sync_e2ee_documents/merge.rs +++ b/openless-all/app/crates/openless-core/src/cloud_sync_e2ee_documents/merge.rs @@ -4,6 +4,7 @@ use std::collections::{BTreeMap, BTreeSet}; use std::fmt; use serde::{Deserialize, Serialize}; +use serde_json::Value; use sha2::{Digest, Sha256}; use crate::cloud_sync_e2ee_protocol::types::{ @@ -96,10 +97,11 @@ impl MergePreview { } for (id, (key, local, remote)) in std::mem::take(&mut self.unresolved) { let chosen = match selected[&id] { - ConflictSide::Local => local, - ConflictSide::Remote => remote, + ConflictSide::Local => local.clone(), + ConflictSide::Remote => remote.clone(), }; - if let Some(unit) = chosen { + if let Some(mut unit) = chosen { + keep_highest_hits(&mut unit, [local.as_ref(), remote.as_ref()]); self.accepted.insert(key, unit); } } @@ -204,7 +206,8 @@ pub fn diff_sync_documents( None }; if let Some(chosen) = chosen { - if let Some(unit) = chosen { + if let Some(mut unit) = chosen { + keep_highest_hits(&mut unit, [local_unit, remote_unit]); preview.accepted.insert(key, unit); } continue; @@ -332,6 +335,63 @@ fn unit_key(key: &DocumentKey) -> DocumentKey { } } +/// Fields that record how one device listed or used a record, not what the record says. +/// They never make two copies differ: validation re-derives `sortIndex` from the record +/// itself, and `keep_highest_hits` reconciles the dictionary hit counter. Comparing them +/// would turn ordinary use on two devices into a conflict on every shared record. +fn usage_fields(kind: DocumentKind) -> &'static [&'static str] { + match kind { + DocumentKind::Dictionary => &["sortIndex", "hits"], + DocumentKind::Corrections | DocumentKind::History => &["sortIndex"], + _ => &[], + } +} + +fn same_content(left: &LogicalDocument, right: &LogicalDocument) -> bool { + let fields = usage_fields(left.kind); + if fields.is_empty() { + return left == right; + } + let content = |doc: &LogicalDocument| { + let mut value = doc.value.clone(); + if let Some(object) = value.as_object_mut() { + for field in fields { + object.remove(*field); + } + } + value + }; + left.id == right.id + && left.kind == right.kind + && left.schema_version == right.schema_version + && content(left) == content(right) +} + +/// A hit counter only grows, on whichever device used the word, so the larger copy is the +/// better record of use. Neither side's count is a change the user has to pick between. +fn keep_highest_hits(unit: &mut Unit, sides: [Option<&Unit>; 2]) { + for (key, entry) in unit.iter_mut() { + let Entry::Live(doc) = entry else { continue }; + if doc.kind != DocumentKind::Dictionary { + continue; + } + let hits = |doc: &LogicalDocument| doc.value.get("hits").and_then(Value::as_u64); + let highest = sides + .iter() + .flatten() + .filter_map(|side| match side.get(key) { + Some(Entry::Live(other)) => hits(other), + _ => None, + }) + .max(); + if let Some(highest) = highest.filter(|highest| hits(doc) < Some(*highest)) { + if let Some(object) = doc.value.as_object_mut() { + object.insert("hits".into(), Value::from(highest)); + } + } + } +} + fn equivalent(left: Option<&Unit>, right: Option<&Unit>) -> bool { match (left, right) { (None, None) => true, @@ -339,7 +399,7 @@ fn equivalent(left: Option<&Unit>, right: Option<&Unit>) -> bool { left.iter() .all(|(key, value)| match (value, right.get(key)) { (Entry::Deleted(_), Some(Entry::Deleted(_))) => true, - (_, Some(other)) => value == other, + (Entry::Live(doc), Some(Entry::Live(other))) => same_content(doc, other), _ => false, }) } diff --git a/openless-all/app/crates/openless-core/src/cloud_sync_e2ee_documents/tests.rs b/openless-all/app/crates/openless-core/src/cloud_sync_e2ee_documents/tests.rs index c62a51ed8..4a9125739 100644 --- a/openless-all/app/crates/openless-core/src/cloud_sync_e2ee_documents/tests.rs +++ b/openless-all/app/crates/openless-core/src/cloud_sync_e2ee_documents/tests.rs @@ -603,3 +603,185 @@ fn collection_order_is_preserved_independently_of_stable_id_sorting() { 1 ); } + +fn history_row(id: &str, created_at: &str, text: &str) -> SecretJson { + SecretJson::new( + json!({"id":id,"createdAt":created_at,"source":"quick_note","rawTranscript":text,"finalText":text,"mode":"raw","insertStatus":"notRequested","hasAudioRecording":false}), + ) +} + +/// Newest first, the way the history store keeps its list. +fn history_set(rows: &[(&str, &str, &str)]) -> ValidatedSyncDocuments { + let mut data = snapshot(); + data.history = rows + .iter() + .map(|(id, created_at, text)| history_row(id, created_at, text)) + .collect(); + export_snapshot(data).unwrap().documents +} + +fn dictionary_set(rows: Vec) -> ValidatedSyncDocuments { + let mut data = snapshot(); + data.dictionary = rows.into_iter().map(SecretJson::new).collect(); + export_snapshot(data).unwrap().documents +} + +fn ids_in_order(set: &ValidatedSyncDocuments, kind: DocumentKind) -> Vec { + let mut rows: Vec<_> = set + .documents() + .documents + .iter() + .filter(|doc| doc.kind == kind) + .collect(); + rows.sort_by_key(|doc| doc.value["sortIndex"].as_u64().unwrap()); + rows.into_iter().map(|doc| doc.id.clone()).collect() +} + +#[test] +fn dictating_on_two_devices_between_syncs_is_not_a_conflict() { + let shared = [ + ("old-2", "2026-09-26T09:00:00Z", "second"), + ("old-1", "2026-09-26T08:00:00Z", "first"), + ]; + let base = history_set(&shared); + // Each new row lands at the front of its own device's list, pushing every shared row + // down by a different amount on the two devices. + let local = history_set(&[ + ("a-2", "2026-09-27T12:00:00Z", "a later"), + ("a-1", "2026-09-27T10:00:00Z", "a earlier"), + shared[0], + shared[1], + ]); + let remote = history_set(&[("b-1", "2026-09-27T11:00:00Z", "b"), shared[0], shared[1]]); + + let preview = diff_sync_documents(Some(&base), &local, &remote).unwrap(); + assert!(preview.conflicts().is_empty()); + let merged = preview.resolve(&[]).unwrap(); + // Both devices' rows are kept, interleaved by when they were dictated. + assert_eq!( + ids_in_order(&merged, DocumentKind::History), + ["a-2", "b-1", "a-1", "old-2", "old-1"] + ); + + // The other device reaches the same result from its side. + let mirrored = diff_sync_documents(Some(&base), &remote, &local) + .unwrap() + .resolve(&[]) + .unwrap(); + assert_eq!( + ids_in_order(&mirrored, DocumentKind::History), + ids_in_order(&merged, DocumentKind::History) + ); +} + +#[test] +fn records_that_differ_only_in_list_position_merge_without_a_baseline() { + let local = history_set(&[ + ("only-local", "2026-09-27T10:00:00Z", "local"), + ("shared", "2026-09-26T08:00:00Z", "same text"), + ]); + let remote = history_set(&[("shared", "2026-09-26T08:00:00Z", "same text")]); + let preview = diff_sync_documents(None, &local, &remote).unwrap(); + assert!(preview.conflicts().is_empty()); +} + +#[test] +fn editing_the_same_record_on_both_devices_is_still_a_conflict() { + let base = history_set(&[("shared", "2026-09-26T08:00:00Z", "original")]); + let local = history_set(&[("shared", "2026-09-26T08:00:00Z", "edited here")]); + let remote = history_set(&[ + ("new-remote", "2026-09-27T10:00:00Z", "unrelated"), + ("shared", "2026-09-26T08:00:00Z", "edited there"), + ]); + let preview = diff_sync_documents(Some(&base), &local, &remote).unwrap(); + assert_eq!(preview.conflicts().len(), 1); + assert_eq!(preview.conflicts()[0].kind, DocumentKind::History); + assert_eq!(preview.conflicts()[0].reason, ConflictReason::BothModified); +} + +#[test] +fn dictionary_hits_keep_the_highest_count_instead_of_conflicting() { + let word = |hits: u64, phrase: &str| json!({"id":"w1","phrase":phrase,"note":null,"enabled":true,"hits":hits,"createdAt":"2026-09-26T00:00:00Z"}); + let word_of = |set: &ValidatedSyncDocuments| { + set.documents() + .documents + .iter() + .find(|doc| doc.kind == DocumentKind::Dictionary) + .unwrap() + .value + .clone() + }; + let base = dictionary_set(vec![word(1, "OpenLess")]); + + // Used on both devices, edited on neither. + let preview = diff_sync_documents( + Some(&base), + &dictionary_set(vec![word(4, "OpenLess")]), + &dictionary_set(vec![word(6, "OpenLess")]), + ) + .unwrap(); + assert!(preview.conflicts().is_empty()); + assert_eq!(word_of(&preview.resolve(&[]).unwrap())["hits"], 6); + + // Renamed on the other device while this one kept using it: the rename wins, and + // this device's higher count is not thrown away with its old spelling. + let preview = diff_sync_documents( + Some(&base), + &dictionary_set(vec![word(9, "OpenLess")]), + &dictionary_set(vec![word(2, "OpenLess IME")]), + ) + .unwrap(); + assert!(preview.conflicts().is_empty()); + let merged = word_of(&preview.resolve(&[]).unwrap()); + assert_eq!(merged["phrase"], "OpenLess IME"); + assert_eq!(merged["hits"], 9); + + // Renamed differently on both devices is a real conflict; either choice keeps the + // highest count. + let local = dictionary_set(vec![word(9, "Open Less")]); + let remote = dictionary_set(vec![word(2, "OpenLess IME")]); + let preview = diff_sync_documents(Some(&base), &local, &remote).unwrap(); + assert_eq!(preview.conflicts().len(), 1); + let choice = ConflictChoice { + conflict_id: preview.conflicts()[0].conflict_id.clone(), + side: ConflictSide::Remote, + }; + let merged = word_of(&preview.resolve(&[choice]).unwrap()); + assert_eq!(merged["phrase"], "OpenLess IME"); + assert_eq!(merged["hits"], 9); +} + +#[test] +fn collection_order_follows_the_records_not_a_device_list_position() { + let learned = crate::shared_types::LEARNED_VOCAB_NOTE; + let entry = |id: &str, note: Option<&str>, created_at: &str| json!({"id":id,"phrase":id,"note":note,"enabled":true,"hits":0,"createdAt":created_at}); + // Deliberately listed in the wrong order. + let set = dictionary_set(vec![ + entry("learned-new", Some(learned), "2026-09-28T00:00:00Z"), + entry("manual-old", None, "2026-09-26T00:00:00Z"), + entry("undated", None, ""), + entry("learned-old", Some(learned), "2026-09-27T00:00:00Z"), + entry("manual-new", None, "2026-09-29T00:00:00+08:00"), + ]); + // Manual entries newest first, then learned entries oldest first — how the store adds + // them. Offsets are compared as instants, and undated rows close their group. + assert_eq!( + ids_in_order(&set, DocumentKind::Dictionary), + [ + "manual-new", + "manual-old", + "undated", + "learned-old", + "learned-new" + ] + ); + + let history = history_set(&[ + ("morning", "2026-09-27T08:00:00Z", "m"), + ("evening", "2026-09-27T20:00:00Z", "e"), + ]); + assert_eq!( + ids_in_order(&history, DocumentKind::History), + ["evening", "morning"] + ); +} diff --git a/openless-all/app/crates/openless-core/src/cloud_sync_e2ee_documents/validate.rs b/openless-all/app/crates/openless-core/src/cloud_sync_e2ee_documents/validate.rs index 0f51ec367..8650bf80e 100644 --- a/openless-all/app/crates/openless-core/src/cloud_sync_e2ee_documents/validate.rs +++ b/openless-all/app/crates/openless-core/src/cloud_sync_e2ee_documents/validate.rs @@ -724,6 +724,31 @@ pub(crate) fn uuid_v4(value: &str) -> DocumentResult<()> { Ok(()) } +/// Where a record belongs in its collection, derived from what the record says instead of +/// from one device's list position — a position shifts on every device with each new row, +/// so two devices in ordinary use could never agree on it. +/// +/// Mirrors how the stores themselves insert: history and corrections newest first; the +/// dictionary keeps manual entries (newest first) ahead of learned ones (oldest first). +/// Rows without a usable `createdAt` sort last in their group. +fn collection_rank(kind: DocumentKind, value: &Value) -> (bool, bool, i64) { + let learned = kind == DocumentKind::Dictionary + && value.get("note").and_then(Value::as_str) + == Some(crate::shared_types::LEARNED_VOCAB_NOTE); + let created = value + .get("createdAt") + .and_then(Value::as_str) + .and_then(|text| chrono::DateTime::parse_from_rfc3339(text).ok()) + .map(|time| time.timestamp_micros()); + match created { + Some(micros) if learned => (learned, false, micros), + Some(micros) => (learned, false, micros.saturating_neg()), + None => (learned, true, 0), + } +} + +/// `sortIndex` stays on the wire as a dense index so older clients keep restoring in order, +/// but it is rewritten here from [`collection_rank`]; the incoming value only breaks ties. fn normalize_collection_order(set: &mut DocumentSet) -> DocumentResult<()> { for kind in [ DocumentKind::Dictionary, @@ -746,24 +771,16 @@ fn normalize_collection_order(set: &mut DocumentSet) -> DocumentResult<()> { return Err(DocumentError::InvalidDocument); } } - indices.sort_by(|left, right| { - let left = &set.documents[*left]; - let right = &set.documents[*right]; + indices.sort_by_cached_key(|index| { + let doc = &set.documents[*index]; ( - left.value + collection_rank(kind, &doc.value), + doc.value .get("sortIndex") .and_then(Value::as_u64) .unwrap_or(u64::MAX), - &left.id, + doc.id.clone(), ) - .cmp(&( - right - .value - .get("sortIndex") - .and_then(Value::as_u64) - .unwrap_or(u64::MAX), - &right.id, - )) }); for (order, index) in indices.into_iter().enumerate() { set.documents[index]