From 0ecff7eeb9f5f6e5645c9032cd42288822b6f7da Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 8 Oct 2026 17:01:42 -0400 Subject: [PATCH] Core: tag names fold every letter, as the server does (#5385) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SQLite's lower() folds ASCII only, so a device could hold "Café" and "CAFÉ" as two tags while the server held one. derive::fold (to_lowercase) is now the one comparison. It is registered as the deterministic SQL function fold() on every connection (schema::migrate), and find_or_create, the rename clash, the pull clash and the unique index all use it. derive's push_unique used eq_ignore_ascii_case and now folds the same way. v14 merges pairs a device already holds before rebuilding the index. The older row survives, as in rename_label. Memberships move with via_tag kept, affected notes go dirty, and the merged-away row is a pending delete. It is written out rather than calling store::merge_labels so the migration doesn't depend on store code that later versions may change. rusqlite gains its "functions" feature (operator-approved, 2026-10-08). grammar.json gains "#café and #CAFÉ" -> ["café"], which all three implementations run. Co-Authored-By: Claude Opus 5.5 --- core/Cargo.toml | 4 +- core/src/local/derive.rs | 15 +++- core/src/local/schema.rs | 178 ++++++++++++++++++++++++++++++++++++- core/src/local/store.rs | 16 +++- core/src/sync/pull.rs | 6 +- core/testdata/grammar.json | 1 + 6 files changed, 210 insertions(+), 10 deletions(-) diff --git a/core/Cargo.toml b/core/Cargo.toml index a5a02ff..a39dfd8 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -11,7 +11,9 @@ serde_json = { workspace = true } log = { workspace = true } # Local-first store (M10.4): bundled = compile SQLite in, so there's no system # libsqlite dependency to vary across the AppImage / native / Windows / Android builds. -rusqlite = { version = "0.32", features = ["bundled"] } +# functions = the `fold` SQL function tag names are compared and indexed by (#5385): +# SQLite's own lower() folds ASCII only, and the server folds every letter. +rusqlite = { version = "0.32", features = ["bundled", "functions"] } uuid = { version = "1", features = ["v4"] } # RFC3339 timestamps for created_at/updated_at/remind_at (Date.parse-able on the JS side). chrono = { version = "0.4", default-features = false, features = ["clock"] } diff --git a/core/src/local/derive.rs b/core/src/local/derive.rs index 40410d7..2a40f2e 100644 --- a/core/src/local/derive.rs +++ b/core/src/local/derive.rs @@ -205,10 +205,10 @@ pub fn lift_standalone_tags(body: &str) -> (Vec, Vec, String) { // A tag that ALSO appears in prose stays derived: the prose copy still backs it, // so deleting that copy should still detach the label. - let inline_lower: Vec = inline.iter().map(|n| n.to_lowercase()).collect(); + let inline_folded: Vec = inline.iter().map(|n| fold(n)).collect(); let standalone = standalone .into_iter() - .filter(|n| !inline_lower.contains(&n.to_lowercase())) + .filter(|n| !inline_folded.contains(&fold(n))) .collect(); (standalone, inline, lifted) } @@ -217,8 +217,17 @@ fn is_tag_char(c: char) -> bool { c.is_alphanumeric() || c == '_' || c == '-' } +/// How two tag names compare: equal once every letter is lowercased, accented ones +/// included, so "Café" and "CAFÉ" are one tag. The server's `labeling.named` folds +/// the same way (Postgres `lower`, Python `str.lower`), and the store's `fold` SQL +/// function is this (#5385). +pub(crate) fn fold(name: &str) -> String { + name.to_lowercase() +} + fn push_unique(out: &mut Vec, candidate: &str) { - if !out.iter().any(|x| x.eq_ignore_ascii_case(candidate)) { + let folded = fold(candidate); + if !out.iter().any(|x| fold(x) == folded) { out.push(candidate.to_string()); } } diff --git a/core/src/local/schema.rs b/core/src/local/schema.rs index 1b7a246..9dc2990 100644 --- a/core/src/local/schema.rs +++ b/core/src/local/schema.rs @@ -7,6 +7,7 @@ //! Migrations are gated on `PRAGMA user_version`; a change is a new entry at the end of //! [`STEPS`]. +use rusqlite::functions::FunctionFlags; use rusqlite::{params, Connection, OptionalExtension}; use crate::local::derive; @@ -331,6 +332,62 @@ const SCHEMA_V13: &str = r#" DROP TABLE saved_filters; "#; +// v14 (#5385): tag names compare by `fold`, which lowercases every letter. Until now +// the index and the queries used SQLite's lower(), which folds ASCII only, so "Café" +// and "CAFÉ" could be two tags on a device while the server held one. +// +// Pairs that already exist are merged before the index can be rebuilt. THE OLDER ROW +// SURVIVES, as in `store::rename_label`, so every device holding the same pair picks +// the same one. The merge is written out here rather than calling +// `store::merge_labels`: a migration replays on an old database before every later +// step, and store code is free to start reading a column a later step adds. What it +// does is that merge's: memberships move to the survivor (keeping `via_tag`, since +// the body's `#CAFÉ` now derives the survivor), the affected notes are marked dirty so +// their label sets reach the server, and the merged-away row is a pending delete. +fn migrate_v14(conn: &Connection) -> rusqlite::Result<()> { + let mut groups: Vec<(String, Vec)> = Vec::new(); + { + let mut stmt = + conn.prepare("SELECT id, name FROM labels ORDER BY created_at ASC, id ASC")?; + let mut rows = stmt.query([])?; + while let Some(row) = rows.next()? { + let id: String = row.get(0)?; + let key = derive::fold(&row.get::<_, String>(1)?); + match groups.iter_mut().find(|(k, _)| *k == key) { + Some((_, ids)) => ids.push(id), + None => groups.push((key, vec![id])), + } + } + } + + for (_, ids) in groups.into_iter().filter(|(_, ids)| ids.len() > 1) { + let survivor = &ids[0]; + for doomed in &ids[1..] { + conn.execute( + "INSERT OR IGNORE INTO note_labels (note_id, label_id, via_tag) + SELECT note_id, ?2, via_tag FROM note_labels WHERE label_id = ?1", + params![doomed, survivor], + )?; + conn.execute( + "UPDATE notes SET dirty = 1 + WHERE id IN (SELECT note_id FROM note_labels WHERE label_id = ?1)", + [doomed], + )?; + conn.execute( + "INSERT OR REPLACE INTO pending_deletes (entity, id, deleted_at) + VALUES ('label', ?1, strftime('%Y-%m-%dT%H:%M:%fZ', 'now'))", + [doomed], + )?; + conn.execute("DELETE FROM labels WHERE id = ?1", [doomed])?; + } + } + + conn.execute_batch( + "DROP INDEX idx_labels_name; + CREATE UNIQUE INDEX idx_labels_name ON labels (fold(name));", + ) +} + /// One schema version's change: SQL, or code for a change SQL alone can't make. enum Step { Sql(&'static str), @@ -353,10 +410,16 @@ const STEPS: &[Step] = &[ Step::Sql(SCHEMA_V11), Step::Sql(SCHEMA_V12), Step::Sql(SCHEMA_V13), + Step::Code(migrate_v14), ]; /// Bring the database up to the latest schema. Idempotent. +/// +/// Also gives the connection the `fold` function first. The label index is built on +/// it, so a connection without it can't write a label at all, and every connection +/// the core opens comes through here. pub fn migrate(conn: &Connection) -> rusqlite::Result<()> { + register_fold(conn)?; conn.execute_batch("PRAGMA foreign_keys = ON;")?; let version: i64 = conn.query_row("PRAGMA user_version", [], |r| r.get(0))?; for (to, step) in (1..).zip(STEPS) { @@ -372,6 +435,19 @@ pub fn migrate(conn: &Connection) -> rusqlite::Result<()> { Ok(()) } +/// `fold(name)`: [`derive::fold`] in SQL. Deterministic, so an index may use it, and +/// innocuous (no side effects), so the schema may name it. +fn register_fold(conn: &Connection) -> rusqlite::Result<()> { + conn.create_scalar_function( + "fold", + 1, + FunctionFlags::SQLITE_UTF8 + | FunctionFlags::SQLITE_DETERMINISTIC + | FunctionFlags::SQLITE_INNOCUOUS, + |ctx| Ok(ctx.get::>(0)?.map(|s| derive::fold(&s))), + ) +} + #[cfg(test)] mod tests { use super::*; @@ -483,7 +559,7 @@ mod tests { let version: i64 = conn .query_row("PRAGMA user_version", [], |r| r.get(0)) .expect("version"); - assert_eq!(version, 13); + assert_eq!(version, 14); } /// Every attachment that predates v10 came down the feed, so it is already on the @@ -550,4 +626,104 @@ mod tests { .expect("count"); assert_eq!(tables, 0); } + + /// A database at v13: the last schema whose label index folded ASCII only. + fn v13_db() -> Connection { + let conn = Connection::open_in_memory().expect("open"); + conn.execute_batch("PRAGMA foreign_keys = ON;").expect("fk"); + for step in &STEPS[..13] { + match step { + Step::Sql(sql) => conn.execute_batch(sql).expect("batch"), + Step::Code(apply) => apply(&conn).expect("step"), + } + } + conn.execute_batch("PRAGMA user_version = 13;") + .expect("v13"); + conn + } + + fn add_label(conn: &Connection, id: &str, name: &str, created: &str) { + conn.execute( + "INSERT INTO labels (id, name, created_at, updated_at, dirty) + VALUES (?1, ?2, ?3, ?3, 0)", + params![id, name, created], + ) + .expect("label"); + } + + fn tag(conn: &Connection, note: &str, label: &str, via_tag: bool) { + conn.execute( + "INSERT INTO note_labels (note_id, label_id, via_tag) VALUES (?1, ?2, ?3)", + params![note, label, via_tag], + ) + .expect("membership"); + } + + /// One string per row, as `sql` builds it. + fn rows(conn: &Connection, sql: &str) -> Vec { + conn.prepare(sql) + .expect("prepare") + .query_map([], |r| r.get(0)) + .expect("query") + .collect::>() + .expect("rows") + } + + /// Before v14 the index was on lower(name), which leaves É alone, so a device + /// could hold both of these. The older one survives and gains the other's notes. + #[test] + fn v14_merges_tags_that_differ_only_in_an_accented_letter() { + let conn = v13_db(); + add_label(&conn, "old", "Café", "2026-01-01T00:00:00.000Z"); + add_label(&conn, "new", "CAFÉ", "2026-02-01T00:00:00.000Z"); + add_label(&conn, "other", "Tea", "2026-03-01T00:00:00.000Z"); + add_note(&conn, "derived", "#CAFÉ"); + add_note(&conn, "both", "both"); + add_note(&conn, "untouched", "tea"); + conn.execute("UPDATE notes SET dirty = 0", []) + .expect("clean"); + tag(&conn, "derived", "new", true); + tag(&conn, "both", "old", false); + tag(&conn, "both", "new", false); + tag(&conn, "untouched", "other", false); + + migrate(&conn).expect("migrate"); + + assert_eq!( + rows(&conn, "SELECT id || ' ' || name FROM labels ORDER BY id"), + ["old Café", "other Tea"] + ); + assert_eq!( + rows( + &conn, + "SELECT note_id || ' ' || label_id || ' ' || via_tag FROM note_labels ORDER BY note_id" + ), + // `derived` stays derived: the body's #CAFÉ now names the survivor. + ["both old 0", "derived old 1", "untouched other 0"] + ); + assert_eq!( + rows(&conn, "SELECT id FROM notes WHERE dirty = 1 ORDER BY id"), + ["both", "derived"] + ); + assert_eq!( + rows( + &conn, + "SELECT id FROM pending_deletes WHERE entity = 'label'" + ), + ["new"] + ); + } + + /// The rebuilt index is what keeps the pair from coming back. + #[test] + fn after_v14_a_name_differing_only_in_an_accented_letter_is_refused() { + let conn = Connection::open_in_memory().expect("open"); + migrate(&conn).expect("migrate"); + add_label(&conn, "a", "Café", "2026-01-01T00:00:00.000Z"); + let twin = conn.execute( + "INSERT INTO labels (id, name, created_at, updated_at) VALUES ('b', 'CAFÉ', 'x', 'x')", + [], + ); + assert!(twin.is_err(), "the index let CAFÉ in beside Café"); + } } diff --git a/core/src/local/store.rs b/core/src/local/store.rs index 77e8068..02f3aa8 100644 --- a/core/src/local/store.rs +++ b/core/src/local/store.rs @@ -248,7 +248,7 @@ fn touch(conn: &Connection, id: &str) -> rusqlite::Result<()> { fn find_or_create_label(conn: &Connection, name: &str) -> rusqlite::Result { let existing: Option = conn .query_row( - "SELECT id FROM labels WHERE lower(name) = lower(?1)", + "SELECT id FROM labels WHERE fold(name) = fold(?1)", [name], |r| r.get(0), ) @@ -1039,7 +1039,7 @@ pub fn create_label(conn: &Connection, name: &str) -> rusqlite::Result