diff --git a/CLAUDE.md b/CLAUDE.md index 69325b0..9552632 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -338,6 +338,15 @@ persistence) → GUI (Qt widgets that read the manager and connect to its signal `device_sync` (so it rides the Syncthing share and both machines agree); keeping it off the `Playlist` keeps it out of the conflict-merge machinery, and **unticking is the only way a playlist comes off the device**. + **The device's filesystem is case-insensitive**, so "RJD2" and "Rjd2" are one + folder there and one artist here: since Round 54 the app's `Library.group()` + keys *both* `albumsByKey` and `artistsByName` case-folded (and fetches the + artist before the album block, holding it — the raw-case lookup afterwards is + what NPE'd on the second track of a two-spelling album and blanked the whole + library), and `plan_andtunes_sync` folds both the collision rule and the + device diff, which had been re-copying every such file over MTP every sync. + `plan.stale` still carries the device's own spelling — that is what + `_delete_stale` unlinks by. Since Round 50 the app exists: `andtunes/app/` is plain Java against the Android framework, built by `andtunes/build.py` (aapt2 → javac → R8 → zipalign → apksigner) — **no Gradle, no Kotlin**, because platform 33 + diff --git a/andtunes/TASKS.md b/andtunes/TASKS.md index 52a8edc..3db06ee 100644 --- a/andtunes/TASKS.md +++ b/andtunes/TASKS.md @@ -7,8 +7,12 @@ A music player for the Rabbit R1 that never scans anything: LinTunes writes Legend: `[ ]` todo · `[~]` in progress · `[x]` done. **Status: phases 1–3 done (2026-09-11, LinTunes rounds 50–51, andTunes -0.2.0).** What's left is the *Parked* list, pulled in as trav finds he needs -it. Built with +0.2.0); 0.2.1 (LinTunes round 54, 2026-09-14) fixed the grouping crash — +`Library.group()` keyed albums case-folded but artists raw-case, so one artist +spelled two ways on one album ("RJD2" / "Rjd2") took the whole library down +with `Couldn't read library.json`. Both maps fold case now, and `Artist` has a +`key` the way `Album` always did.** What's left is the *Parked* list, pulled in +as trav finds he needs it. Built with `python3 andtunes/build.py` — plain Java, no Gradle, nothing downloaded (see `README.md`, *Toolchain*). Phases are andTunes' own; which LinTunes round each lands in is decided when it starts. diff --git a/andtunes/app/src/main/java/me/teafry/andtunes/Library.java b/andtunes/app/src/main/java/me/teafry/andtunes/Library.java index 3b4941b..b5e9bf9 100644 --- a/andtunes/app/src/main/java/me/teafry/andtunes/Library.java +++ b/andtunes/app/src/main/java/me/teafry/andtunes/Library.java @@ -51,7 +51,7 @@ final class Library { } static final class Artist { - String name, sortKey; + String key, name, sortKey; final ArrayList albums = new ArrayList<>(); final ArrayList tracks = new ArrayList<>(); // alphabetical } @@ -168,7 +168,9 @@ final class Library { } checkFormat(format); for (Playlist p : lib.playlists) { - for (Long id : playlistIds.get(p.id)) { + List members = playlistIds.get(p.id); + if (members == null) continue; // two playlists sharing an id + for (Long id : members) { Track t = lib.byId.get(id); if (t != null) p.tracks.add(t); } @@ -257,32 +259,38 @@ final class Library { String who = !t.albumArtist.isEmpty() ? t.albumArtist : !t.artist.isEmpty() ? t.artist : UNKNOWN_ARTIST; String albumName = t.album.isEmpty() ? UNKNOWN_ALBUM : t.album; - String key = who.toLowerCase(Locale.ROOT) + "" + // Both maps fold case, or "RJD2" and "Rjd2" share one album while + // owning two artist entries — and the second track of that album + // skips the block that would have made the artist. Fetched and + // held here, so no lookup below can come back null. + String whoKey = who.toLowerCase(Locale.ROOT); + Artist artist = artistsByName.get(whoKey); + if (artist == null) { + artist = new Artist(); + artist.key = whoKey; + artist.name = who; + artist.sortKey = sortKey(who); + artistsByName.put(whoKey, artist); + artists.add(artist); + } + String key = whoKey + "" + albumName.toLowerCase(Locale.ROOT); Album album = albumsByKey.get(key); if (album == null) { album = new Album(); album.key = key; album.name = albumName; - album.artist = who; + album.artist = artist.name; album.sortKey = sortKey(albumName); albumsByKey.put(key, album); albums.add(album); - Artist artist = artistsByName.get(who); - if (artist == null) { - artist = new Artist(); - artist.name = who; - artist.sortKey = sortKey(who); - artistsByName.put(who, artist); - artists.add(artist); - } artist.albums.add(album); } album.tracks.add(t); if (album.art.isEmpty() && !t.art.isEmpty()) album.art = t.art; if (t.year > 0 && (album.year == 0 || t.year < album.year)) album.year = t.year; t.albumRef = album; - artistsByName.get(who).tracks.add(t); + artist.tracks.add(t); } for (Album a : albums) { Collections.sort(a.tracks, (x, y) -> x.disc != y.disc ? x.disc - y.disc diff --git a/andtunes/app/src/main/java/me/teafry/andtunes/ListActivity.java b/andtunes/app/src/main/java/me/teafry/andtunes/ListActivity.java index a21f7a0..d6ac0fb 100644 --- a/andtunes/app/src/main/java/me/teafry/andtunes/ListActivity.java +++ b/andtunes/app/src/main/java/me/teafry/andtunes/ListActivity.java @@ -301,10 +301,10 @@ public class ListActivity extends BaseActivity { open(PLAYLIST, ((Library.Playlist) row.ref).id); break; case ARTIST_ROW: - open(ARTIST, ((Library.Artist) row.ref).name); + open(ARTIST, ((Library.Artist) row.ref).key); break; case ALL_SONGS: - open(ARTIST_SONGS, ((Library.Artist) row.ref).name); + open(ARTIST_SONGS, ((Library.Artist) row.ref).key); break; case ALBUM_ROW: open(ALBUM, ((Library.Album) row.ref).key); diff --git a/andtunes/build.py b/andtunes/build.py index 5c37394..06fbff1 100644 --- a/andtunes/build.py +++ b/andtunes/build.py @@ -26,8 +26,8 @@ import sys import zipfile from pathlib import Path -VERSION_NAME = "0.2.0" -VERSION_CODE = 2 +VERSION_NAME = "0.2.1" +VERSION_CODE = 3 PACKAGE = "me.teafry.andtunes" MIN_SDK = 26 diff --git a/lintunes/__init__.py b/lintunes/__init__.py index dfe003b..ccdd8f9 100644 --- a/lintunes/__init__.py +++ b/lintunes/__init__.py @@ -1,3 +1,3 @@ """LinTunes — iTunes-style music library manager and player for Linux.""" -__version__ = "0.20.2" +__version__ = "0.20.3" diff --git a/lintunes/android/andTunes.apk b/lintunes/android/andTunes.apk index 5293081..de60aee 100644 Binary files a/lintunes/android/andTunes.apk and b/lintunes/android/andTunes.apk differ diff --git a/lintunes/android/andTunes.json b/lintunes/android/andTunes.json index 4afad13..ebae942 100644 --- a/lintunes/android/andTunes.json +++ b/lintunes/android/andTunes.json @@ -1 +1 @@ -{"version_name": "0.2.0", "version_code": 2} +{"version_name": "0.2.1", "version_code": 3} diff --git a/lintunes/andtunes/sync.py b/lintunes/andtunes/sync.py index b566c9d..68bccce 100644 --- a/lintunes/andtunes/sync.py +++ b/lintunes/andtunes/sync.py @@ -192,9 +192,11 @@ def plan_andtunes_sync(playlists, tracks_by_id, root: Path, device=None, # Same collision rule as the per-playlist sync: every member of a # colliding group gets the [track_id] suffix, so a name never depends on # which playlist happened to be walked first. + # Folded, because the device's filesystem is case-insensitive: two songs + # whose paths differ only in case are one file there, not two. by_rel: dict[str, list[MediaItem]] = {} for item in items.values(): - by_rel.setdefault(item.rel, []).append(item) + by_rel.setdefault(item.rel.lower(), []).append(item) for group in by_rel.values(): if len(group) > 1: for item in group: @@ -231,19 +233,27 @@ def plan_andtunes_sync(playlists, tracks_by_id, root: Path, device=None, expected |= {art_relpath(key) for key in plan.albums} expected.add(MANIFEST_NAME) + # Compared case-folded: the device filesystem is case-insensitive, so a + # song filed under "Toro y Moi" here and "Toro Y Moi" there is the same + # file. Exact-string matching saw it as stale *and* missing and re-copied + # it over MTP every single sync. + on_device_folded = {rel.lower(): size for rel, size in on_device.items()} + expected_folded = {rel.lower() for rel in expected} + for item in plan.items: if item.convert_to: # Its FLAC size is unknown until the worker converts it. - item.device_size = on_device.get(item.rel) + item.device_size = on_device_folded.get(item.rel.lower()) plan.copies.append(item) plan.bytes_to_copy += item.size - elif on_device.get(item.rel) == item.size: + elif on_device_folded.get(item.rel.lower()) == item.size: plan.kept += 1 else: plan.copies.append(item) plan.bytes_to_copy += item.size + # plan.stale keeps the device's own spelling — _delete_stale unlinks by it. for rel, size in sorted(on_device.items()): - if rel not in expected: + if rel.lower() not in expected_folded: plan.stale.append(rel) plan.bytes_freed += size return plan diff --git a/scripts/make_dev_library.py b/scripts/make_dev_library.py index 7fc9bc8..a3685a6 100644 --- a/scripts/make_dev_library.py +++ b/scripts/make_dev_library.py @@ -26,6 +26,9 @@ The edge cases here are not decoration. Each one has bitten something: * a multi-disc release → the "2-04" prefix; * a compilation whose album_artist differs → album grouping and album art keying; +* one artist spelled two ways on one album → the case-folded + ("RJD2" / "Rjd2") grouping in the app and + the folded device diff; * a dangling location → the "skipped" counters; * a 200-character title → the 150-char truncation; * non-ASCII and an emoji → UTF-8 all the way to @@ -152,6 +155,10 @@ EDGE_TRACKS = [ ("Song", "Twin Records", "Doubles", ".mp3", "collision A"), ("Song", "Twin Records", "Doubles", ".mp3", "collision B"), ("Orphan Take", "", "", ".mp3", "no artist, no album"), + ("Iced Lightning", "RJD2", "Since We Last Spoke", ".mp3", + "one artist spelled two ways, sharing an album"), + ("Making Days Longer", "Rjd2", "Since We Last Spoke", ".mp3", + "the other spelling — the pair is the point"), ("わたしの音楽 🎧", "Sakura Denwa", "東京の夜", ".mp3", "non-ASCII + emoji"), ("A Title That Simply Refuses To Stop Going On And On " * 4, "Verbose", "Excess", ".mp3", "200+ characters, hits the 150-char truncation"), diff --git a/tasks-done.md b/tasks-done.md index 955c7b7..3ece031 100644 --- a/tasks-done.md +++ b/tasks-done.md @@ -1,5 +1,47 @@ ## Done +### Round 54 (2026-09-14) — one artist spelled two ways (v0.20.3, andTunes 0.2.1) + +trav synced to the Rabbit, the sync reported success, and andTunes answered +with `No library / Couldn't read library.json`. The sync was fine. Everything +it wrote was on the device and correct — 2,432 media files, 208 covers, a +manifest whose every field type-checks against the parser. The app was the bug, +and it took all 2,439 songs down over fifteen of them. + +- [x] **The diagnosis**, read off the device: a `NullPointerException` on + `Library$Artist.tracks` inside `load` (R8 inlines `group()` into it). + `group()` keyed `albumsByKey` **case-folded** and `artistsByName` + **raw-case**, and only ever built the `Artist` inside + `if (album == null)`. So the second track of an album whose artist is + spelled differently found the album already there, skipped the block that + would have made the artist, and dereferenced the null that came back. + Replaying the algorithm over the real manifest: 15 tracks, six artists + spelled two ways — `RJD2`/`Rjd2`, `Toro Y Moi`/`Toro y Moi`, + `FatBoy Slim`/`Fatboy Slim`, `LOVING`/`Loving`, + `Land Of The Loops`/`Land of the Loops`, + `Salami Rose Joe Louis`/`salami rose joe louis`. It worked on the 11th + because the 12th's sync was the first to carry both spellings of one of + those albums. +- [x] **Both maps fold case now**, and the artist is fetched-or-made *before* + the album block and held in a local, so there is no lookup left in + `group()` that can return null. `Artist` gains a `key` (the folded name), + the way `Album` already had one, and `ListActivity` navigates by it — + `ALBUM_ROW` already passed `album.key`, so artists just stopped being the + exception. `playlistIds.get` in `load()` got the same treatment. +- [x] **The device diff folds case too.** `/sdcard` is case-insensitive, so + those two spellings are one folder there; `plan_andtunes_sync` compared + exact strings and saw every such file as stale **and** missing, deleting + and re-copying it over MTP on every sync forever. The collision rule + folds as well, so two songs whose paths differ only by case get the + `[track_id]` suffix instead of one silently overwriting the other. + `plan.stale` still carries the device's own spelling — that is what + `_delete_stale` unlinks by. +- [x] **The fixture grew the shape** (`scripts/make_dev_library.py`): one + artist spelled two ways on one shared album, next to the other naming + edge cases. `tests/test_round54.py` pins the desktop half (the Java + grouping has no harness — it was verified on the device: 2,439 songs + listed, and RJD2 one row of 12 songs where there had been a crash). + ### Round 52 (2026-09-11) — one fingerprint, six artists (v0.20.1) trav's "Dionne Farris - I Know" kept being identified as Jay-Z, with ID3 tags diff --git a/tests/test_round54.py b/tests/test_round54.py new file mode 100644 index 0000000..6d8e9ce --- /dev/null +++ b/tests/test_round54.py @@ -0,0 +1,160 @@ +"""Round 54: one artist spelled two ways. + +trav's Rabbit showed `No library / Couldn't read library.json` on a sync that +had worked perfectly. The manifest was fine; the app's grouping wasn't. +`Library.group()` keyed albums case-folded but artists raw-case, and only +created the artist inside the `album == null` branch — so the second track of +an album whose artist was spelled with different case ("RJD2" / "Rjd2") found +the album already built, skipped artist creation, and dereferenced a null. +Fifteen tracks across six artists took all 2,439 songs down with them. + +The Java half has no test harness. What's pinned here is the desktop half of +the same root cause: the device filesystem is case-insensitive, so those two +spellings are **one** folder there, and `plan_andtunes_sync`'s exact-string +diff saw every such file as stale *and* missing and re-copied it over MTP on +every sync forever. +""" + +import json + +import pytest + +from lintunes.andtunes.layout import media_relpath +from lintunes.andtunes.sync import AndTunesSyncWorker, plan_andtunes_sync +from lintunes.models import Track +from lintunes.models.playlist import Playlist, PlaylistType + + +@pytest.fixture(autouse=True) +def _private_cache(tmp_path, monkeypatch): + monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path / "cache")) + + +DATA = b"x" * 64 + + +def _track(tid, name, artist, album="Since We Last Spoke", *, tmp_path): + path = tmp_path / "local" / f"{tid}.mp3" + path.parent.mkdir(parents=True, exist_ok=True) + path.write_bytes(DATA) + return Track(track_id=tid, name=name, artist=artist, album=album, + location=str(path), total_time=60_000, kind="MPEG audio file") + + +def _playlist(*tids): + return Playlist(name="Mix", persistent_id="AAAA0001", + playlist_type=PlaylistType.REGULAR, track_ids=list(tids)) + + +def _root(tmp_path): + return tmp_path / "device" / "Music" / "andTunes" + + +def _on_device(root, rel, data=DATA): + path = root / rel + path.parent.mkdir(parents=True, exist_ok=True) + path.write_bytes(data) + return path + + +def _both_spellings(tmp_path): + return { + 1: _track(1, "Iced Lightning", "RJD2", tmp_path=tmp_path), + 2: _track(2, "Making Days Longer", "Rjd2", tmp_path=tmp_path), + } + + +# ---- the two spellings are one folder on the device ---- + +def test_the_two_spellings_ask_for_one_device_folder(tmp_path): + """Different rel paths, but the device can only hold one of the two.""" + tracks = _both_spellings(tmp_path) + assert media_relpath(tracks[1]).startswith("Media/RJD2/") + assert media_relpath(tracks[2]).startswith("Media/Rjd2/") + folders = {media_relpath(t).split("/")[1].lower() for t in tracks.values()} + assert folders == {"rjd2"} + + +def test_a_file_filed_under_the_other_spelling_is_kept_not_recopied(tmp_path): + """The churn: it used to be stale *and* missing, every sync, forever.""" + tracks = _both_spellings(tmp_path) + root = _root(tmp_path) + # What a case-insensitive filesystem actually ends up holding: whichever + # spelling made the folder first wins, and both songs land inside it. + _on_device(root, "Media/RJD2/Since We Last Spoke/Iced Lightning.mp3") + _on_device(root, "Media/RJD2/Since We Last Spoke/Making Days Longer.mp3") + + plan = plan_andtunes_sync([_playlist(1, 2)], tracks, root) + + assert plan.kept == 2 + assert plan.copies == [] + assert plan.stale == [] + assert plan.bytes_to_copy == 0 + + +def test_a_real_change_still_copies(tmp_path): + """Folding the diff must not blind it to a file whose size moved.""" + tracks = _both_spellings(tmp_path) + root = _root(tmp_path) + _on_device(root, "Media/RJD2/Since We Last Spoke/Iced Lightning.mp3") + _on_device(root, "Media/RJD2/Since We Last Spoke/Making Days Longer.mp3", + b"short") + + plan = plan_andtunes_sync([_playlist(1, 2)], tracks, root) + + assert plan.kept == 1 + assert [item.track_id for item in plan.copies] == [2] + + +def test_a_file_under_neither_spelling_is_still_stale(tmp_path): + tracks = _both_spellings(tmp_path) + root = _root(tmp_path) + _on_device(root, "Media/RJD2/Since We Last Spoke/Iced Lightning.mp3") + _on_device(root, "Media/RJD2/Since We Last Spoke/Making Days Longer.mp3") + _on_device(root, "Media/Someone Else/Old Record/Gone.mp3") + + plan = plan_andtunes_sync([_playlist(1, 2)], tracks, root) + + # The device's own spelling, because _delete_stale unlinks by that name. + assert plan.stale == ["Media/Someone Else/Old Record/Gone.mp3"] + + +# ---- names that collide only by case ---- + +def test_two_songs_colliding_only_by_case_both_get_the_id_suffix(tmp_path): + """One file on the device, so one of them would have silently won.""" + tracks = { + 1: _track(1, "Iced Lightning", "RJD2", tmp_path=tmp_path), + 2: _track(2, "Iced Lightning", "Rjd2", tmp_path=tmp_path), + } + plan = plan_andtunes_sync([_playlist(1, 2)], tracks, _root(tmp_path)) + + rels = {item.track_id: item.rel for item in plan.items} + assert rels[1].endswith("Iced Lightning [1].mp3") + assert rels[2].endswith("Iced Lightning [2].mp3") + assert len({rel.lower() for rel in rels.values()}) == 2 + + +# ---- both spellings survive into the manifest ---- + +def test_the_manifest_still_carries_both_tracks(tmp_path): + tracks = _both_spellings(tmp_path) + root = _root(tmp_path) + plan = plan_andtunes_sync([_playlist(1, 2)], tracks, root) + + worker = AndTunesSyncWorker(plan) + out = {} + worker.finished.connect(lambda s: out.setdefault("finished", s)) + worker.failed.connect(lambda m: out.setdefault("failed", m)) + worker._run() + assert "failed" not in out, out.get("failed") + + manifest = json.loads((root / "library.json").read_text(encoding="utf-8")) + by_id = {row["id"]: row for row in manifest["tracks"]} + assert by_id[1]["artist"] == "RJD2" + assert by_id[2]["artist"] == "Rjd2" + # Each keeps its own spelling in the path it asks for; the device resolves + # both to the one folder it actually has. + assert by_id[1]["path"].startswith("Media/RJD2/") + assert by_id[2]["path"].startswith("Media/Rjd2/") + assert manifest["playlists"][0]["tracks"] == [1, 2]