Skip to main content

max / audiofiles

Rewrite legacy key.* tags and keep key migrations off the changelog M034 fixed audio_analysis.musical_key but not tags, which are user-accepted copies of what analysis::suggest proposed at the time, so vaults can still hold 'key.am' and 'key.c-sharpm'. M035 maps the 24 legacy spellings to canonical. They are enumerated rather than pattern-matched: a LIKE rule broad enough to catch 'key.c' would also catch hand-written tags in the key namespace, and mangling one is worse than leaving a stale one. Insert-then-delete because (sample_hash, tag) is the primary key and a sample holding both spellings would collide. Both migrations now suppress the sync changelog. audio_analysis and tags carry sync triggers, so M034 as committed would have enqueued a row per analysed sample on every sync-enabled vault. Volume aside, replicating this is wrong: each device runs the migration and converges on the same value, whereas a peer still on the old detector would receive canonical values and carry on writing the compact spelling, leaving one vault holding both. The toggle is a no-op where sync was never configured, since the sync crate seeds that key, not core. The changelog test carries a control write, otherwise it would pass whether or not the triggers ever fired.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-29 16:21 UTC
Signed with PGP, not checked
Commit: 6dda9c14bd8df4e1225ed6ea9d6181702e30ffe1
Parent: 5a6ee2a
1 file changed, +189 insertions, -6 deletions
M crates/audiofiles-core/src/db.rs +189 -6
@@ -1633,6 +1633,18 @@
1633 1633 -- equivalent today only because no note name contains an 'm', which is not a
1634 1634 -- property worth depending on. Rows already canonical contain a space and are
1635 1635 -- excluded by the WHERE clause, so re-running is a no-op.
1636 + --
1637 + -- Suppressed from the changelog: audio_analysis carries a sync UPDATE trigger,
1638 + -- so without this every analysed sample would enqueue a row on upgrade. Worse
1639 + -- than the volume, replicating it is wrong. Every device runs this migration
1640 + -- itself and converges on the same value, while a peer still on the old code
1641 + -- would receive canonical values and keep writing 'Am' from its own detector,
1642 + -- leaving a vault holding both spellings. The UPDATE is a no-op when the key is
1643 + -- absent, which is the case on any vault where sync was never configured (the
1644 + -- row is seeded by the sync crate, not here), and the trigger's WHEN clause
1645 + -- compares NULL and never fires there either.
1646 + UPDATE sync_state SET value = '1' WHERE key = 'applying_remote';
1647 +
1636 1648 UPDATE audio_analysis
1637 1649 SET musical_key = CASE
1638 1650 WHEN musical_key LIKE '%m'
@@ -1643,6 +1655,67 @@
1643 1655 AND musical_key NOT LIKE '% major'
1644 1656 AND musical_key NOT LIKE '% minor'
1645 1657 AND musical_key != '';
1658 +
1659 + UPDATE sync_state SET value = '0' WHERE key = 'applying_remote';
1660 + ";
1661 +
1662 + const MIGRATION_035: &str = r"
1663 + -- Rewrite key.* tags written from the pre-M034 key spelling.
1664 + --
1665 + -- M034 fixed audio_analysis.musical_key, but tags are user-accepted copies of
1666 + -- what analysis::suggest proposed at the time, so a vault can still hold
1667 + -- 'key.am' and 'key.c-sharpm'. suggest builds the tag as
1668 + -- lowercase -> ' ' to '-' -> '#' to '-sharp', so the compact 'Am' became
1669 + -- 'key.am' and 'C#m' became 'key.c-sharpm', against 'key.a-minor' and
1670 + -- 'key.c-sharp-minor' now.
1671 + --
1672 + -- The 24 legacy spellings are enumerated rather than pattern-matched. A LIKE
1673 + -- rule broad enough to catch 'key.c' would also catch any user tag in the key
1674 + -- namespace, and silently mangling a hand-written tag is worse than leaving a
1675 + -- stale one.
1676 + CREATE TEMP TABLE legacy_key_tags (legacy TEXT PRIMARY KEY, canonical TEXT NOT NULL);
1677 + INSERT INTO legacy_key_tags (legacy, canonical) VALUES
1678 + ('key.c', 'key.c-major'),
1679 + ('key.c-sharp', 'key.c-sharp-major'),
1680 + ('key.d', 'key.d-major'),
1681 + ('key.d-sharp', 'key.d-sharp-major'),
1682 + ('key.e', 'key.e-major'),
1683 + ('key.f', 'key.f-major'),
1684 + ('key.f-sharp', 'key.f-sharp-major'),
1685 + ('key.g', 'key.g-major'),
1686 + ('key.g-sharp', 'key.g-sharp-major'),
1687 + ('key.a', 'key.a-major'),
1688 + ('key.a-sharp', 'key.a-sharp-major'),
1689 + ('key.b', 'key.b-major'),
1690 + ('key.cm', 'key.c-minor'),
1691 + ('key.c-sharpm', 'key.c-sharp-minor'),
1692 + ('key.dm', 'key.d-minor'),
1693 + ('key.d-sharpm', 'key.d-sharp-minor'),
1694 + ('key.em', 'key.e-minor'),
1695 + ('key.fm', 'key.f-minor'),
1696 + ('key.f-sharpm', 'key.f-sharp-minor'),
1697 + ('key.gm', 'key.g-minor'),
1698 + ('key.g-sharpm', 'key.g-sharp-minor'),
1699 + ('key.am', 'key.a-minor'),
1700 + ('key.a-sharpm', 'key.a-sharp-minor'),
1701 + ('key.bm', 'key.b-minor');
1702 +
1703 + -- Suppressed for the same reason as M034; see the note there.
1704 + UPDATE sync_state SET value = '1' WHERE key = 'applying_remote';
1705 +
1706 + -- Insert-then-delete rather than UPDATE: (sample_hash, tag) is the primary key,
1707 + -- and a sample already carrying both spellings would collide. OR IGNORE keeps
1708 + -- the existing canonical row in that case.
1709 + INSERT OR IGNORE INTO tags (sample_hash, tag)
1710 + SELECT t.sample_hash, m.canonical
1711 + FROM tags t
1712 + JOIN legacy_key_tags m ON t.tag = m.legacy;
1713 +
1714 + DELETE FROM tags WHERE tag IN (SELECT legacy FROM legacy_key_tags);
1715 +
1716 + UPDATE sync_state SET value = '0' WHERE key = 'applying_remote';
1717 +
1718 + DROP TABLE legacy_key_tags;
1646 1719 ";
1647 1720
1648 1721 /// Register `hash_row_id(salt, key) -> TEXT` as a deterministic SQLite
@@ -1833,6 +1906,7 @@
1833 1906 MIGRATION_032,
1834 1907 MIGRATION_033,
1835 1908 MIGRATION_034,
1909 + MIGRATION_035,
1836 1910 ];
1837 1911
1838 1912 for (i, sql) in MIGRATIONS.iter().enumerate() {
@@ -2075,6 +2149,115 @@
2075 2149 assert_eq!(after, "A minor");
2076 2150 }
2077 2151
2152 + #[test]
2153 + fn migration_035_rewrites_legacy_key_tags() {
2154 + let db = Database::open_in_memory().unwrap();
2155 + db.conn()
2156 + .execute_batch(
2157 + "INSERT INTO samples
2158 + (hash, original_name, file_extension, file_size, import_date, last_modified)
2159 + VALUES ('a', 'a.wav', 'wav', 1, 0, 0), ('b', 'b.wav', 'wav', 1, 0, 0),
2160 + ('c', 'c.wav', 'wav', 1, 0, 0);
2161 + INSERT INTO tags (sample_hash, tag) VALUES
2162 + ('a', 'key.am'),
2163 + ('a', 'genre.techno'),
2164 + ('b', 'key.c-sharpm'),
2165 + ('b', 'key.f-sharp'),
2166 + -- already migrated, plus its legacy twin: must not collide
2167 + ('c', 'key.a-minor'),
2168 + ('c', 'key.am');",
2169 + )
2170 + .unwrap();
2171 + db.conn().execute_batch(MIGRATION_035).unwrap();
2172 +
2173 + let tags = |hash: &str| -> Vec<String> {
2174 + let mut stmt = db
2175 + .conn()
2176 + .prepare("SELECT tag FROM tags WHERE sample_hash = ?1 ORDER BY tag")
2177 + .unwrap();
2178 + let v: Vec<String> = stmt
2179 + .query_map([hash], |r| r.get(0))
2180 + .unwrap()
2181 + .map(Result::unwrap)
2182 + .collect();
2183 + v
2184 + };
2185 +
2186 + assert_eq!(tags("a"), vec!["genre.techno", "key.a-minor"]);
2187 + assert_eq!(tags("b"), vec!["key.c-sharp-minor", "key.f-sharp-major"]);
2188 + // The collision collapses to the single canonical tag rather than
2189 + // failing the migration on the primary key.
2190 + assert_eq!(tags("c"), vec!["key.a-minor"]);
2191 +
2192 + // Idempotent: canonical tags match no legacy spelling.
2193 + db.conn().execute_batch(MIGRATION_035).unwrap();
2194 + assert_eq!(tags("a"), vec!["genre.techno", "key.a-minor"]);
2195 + }
2196 +
2197 + #[test]
2198 + fn key_migrations_do_not_enqueue_sync_changelog() {
2199 + // Both key migrations normalise a local format that every device fixes
2200 + // for itself. Replicating them would push canonical values to peers
2201 + // still running the old detector, which would keep writing the compact
2202 + // spelling and leave the vault holding both.
2203 + let db = Database::open_in_memory().unwrap();
2204 + db.conn()
2205 + .execute_batch(
2206 + "INSERT INTO sync_state (key, value) VALUES ('applying_remote', '0')
2207 + ON CONFLICT(key) DO UPDATE SET value = '0';
2208 + INSERT INTO samples
2209 + (hash, original_name, file_extension, file_size, import_date, last_modified)
2210 + VALUES ('a', 'a.wav', 'wav', 1, 0, 0);
2211 + INSERT INTO audio_analysis
2212 + (hash, musical_key, duration, sample_rate, channels, analyzed_at)
2213 + VALUES ('a', 'Am', 1.0, 44100, 2, 0);",
2214 + )
2215 + .unwrap();
2216 + let before: i64 = db
2217 + .conn()
2218 + .query_row("SELECT count(*) FROM sync_changelog", [], |r| r.get(0))
2219 + .unwrap();
2220 +
2221 + db.conn().execute_batch(MIGRATION_034).unwrap();
2222 + db.conn().execute_batch(MIGRATION_035).unwrap();
2223 +
2224 + let after: i64 = db
2225 + .conn()
2226 + .query_row("SELECT count(*) FROM sync_changelog", [], |r| r.get(0))
2227 + .unwrap();
2228 + assert_eq!(
2229 + before, after,
2230 + "key migrations must not enqueue changelog rows"
2231 + );
2232 +
2233 + // Control: the same write outside the migration must enqueue, otherwise
2234 + // the assertion above would hold even if the triggers never fired here
2235 + // and would prove nothing.
2236 + db.conn()
2237 + .execute_batch("UPDATE audio_analysis SET musical_key = 'B minor' WHERE hash = 'a';")
2238 + .unwrap();
2239 + let control: i64 = db
2240 + .conn()
2241 + .query_row("SELECT count(*) FROM sync_changelog", [], |r| r.get(0))
2242 + .unwrap();
2243 + assert!(
2244 + control > after,
2245 + "sync trigger never fired, so the suppression assertion is vacuous"
2246 + );
2247 +
2248 + // And the flag is left back where it started, not stuck at '1', which
2249 + // would silently stop capturing every later user edit.
2250 + let flag: String = db
2251 + .conn()
2252 + .query_row(
2253 + "SELECT value FROM sync_state WHERE key = 'applying_remote'",
2254 + [],
2255 + |r| r.get(0),
2256 + )
2257 + .unwrap();
2258 + assert_eq!(flag, "0");
2259 + }
2260 +
2078 2261 #[test]
2079 2262 fn file_db_applies_performance_pragmas() {
2080 2263 let dir = tempfile::tempdir().unwrap();
@@ -2150,7 +2333,7 @@
2150 2333 .conn()
2151 2334 .query_row("PRAGMA user_version", [], |row| row.get(0))
2152 2335 .unwrap();
2153 - assert_eq!(version, 34);
2336 + assert_eq!(version, 35);
2154 2337 }
2155 2338
2156 2339 #[test]
@@ -2161,7 +2344,7 @@
2161 2344 .conn()
2162 2345 .query_row("PRAGMA user_version", [], |row| row.get(0))
2163 2346 .unwrap();
2164 - assert_eq!(version, 34);
2347 + assert_eq!(version, 35);
2165 2348 }
2166 2349
2167 2350 #[test]
@@ -2310,7 +2493,7 @@
2310 2493 .conn()
2311 2494 .query_row("PRAGMA user_version", [], |row| row.get(0))
2312 2495 .unwrap();
2313 - assert_eq!(version, 34);
2496 + assert_eq!(version, 35);
2314 2497 }
2315 2498
2316 2499 /// Simulates the worst-case recovery path: a prior partial migration left
@@ -2354,7 +2537,7 @@
2354 2537 .conn()
2355 2538 .query_row("PRAGMA user_version", [], |row| row.get(0))
2356 2539 .unwrap();
2357 - assert_eq!(version, 34);
2540 + assert_eq!(version, 35);
2358 2541 }
2359 2542
2360 2543 /// M018 contract: the `sync_changelog.row_id` for sensitive tables must
@@ -2584,7 +2767,7 @@
2584 2767 let initial_version: i32 = conn
2585 2768 .query_row("PRAGMA user_version", [], |row| row.get(0))
2586 2769 .unwrap();
2587 - assert_eq!(initial_version, 34);
2770 + assert_eq!(initial_version, 35);
2588 2771
2589 2772 let batch = format!("BEGIN;\n{bad_sql}\nPRAGMA user_version = 999;\nCOMMIT;");
2590 2773 let first_err = conn.execute_batch(&batch).unwrap_err();
@@ -2649,7 +2832,7 @@
2649 2832 .conn()
2650 2833 .query_row("PRAGMA user_version", [], |row| row.get(0))
2651 2834 .unwrap();
2652 - assert_eq!(version, 34);
2835 + assert_eq!(version, 35);
2653 2836 }
2654 2837
2655 2838 #[test]