diff --git a/CLAUDE.md b/CLAUDE.md index 23ed4c5..21fae77 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -52,12 +52,27 @@ persistence) → GUI (Qt widgets that read the manager and connect to its signal hex `persistent_id` (folder containment via `parent_persistent_id`). - **`lintunes/storage/json_storage.py`** — the library is **multiple files** in - the data dir: `library.json` (all tracks), `library_metadata.json`, and one - `playlists/.json` per playlist. Writes are atomic (`*.json.tmp` + the data dir: `library.json` (all tracks), `library_metadata.json`, one + `playlists/.json` per playlist, and one + `plays/.json` per machine. Writes are atomic (`*.json.tmp` → rename). **`storage/conflict_resolver.py`** merges Syncthing `*.sync-conflict-*` files on startup: play counts take the max, edited fields take the newest, playlist membership takes the union. +- **`lintunes/storage/play_journal.py`** — why play counts can't conflict. Since + Round 38 `library.json` holds only a **base** count and each machine owns + `plays/.json` with *its own* per-track totals; effective count = + base + the sum of every journal. Only the owner ever writes its journal, so + two machines never touch the same file — and finishing a track no longer + rewrites 15 MB, which is what handed Syncthing a conflict once per song. + Totals, not an append log, so there is no compaction step to double-count in. + `PlayJournal.load()` must be handed a library whose tracks still carry **base** + counts (the fresh load at startup, the `disk` copy inside `reload_from_disk`) — + folding an already-folded library promotes the effective count to base. + Mirror-image rule in `json_storage.save_tracks()`: it writes + `journal.base_fields(tid)`, never the `Track`'s effective count. Those two + places are the whole hazard; `tests/test_round38.py` pins both. + - **`lintunes/library_manager.py`** — `LibraryManager(QObject)` owns the `Library`, is the single funnel for all mutations, and persists them **debounced** (3 s) with per-area dirty tracking (a play-count bump rewrites diff --git a/TASKS.md b/TASKS.md index 16a3936..5d551ed 100644 --- a/TASKS.md +++ b/TASKS.md @@ -14,10 +14,13 @@ When a round closes, move its finished items to `tasks-done.md`. Folder + web-mix export, audio.js/jQuery dropped for a dependency-free player, lossless-only conversion. Round 36 below is still open. -## Round 36 — the merge rework (planned, Round 35 covered the speed half) +## Round 38 (2026-08-20) — merge windows + play journals: done, see tasks-done.md + +## Round 36 — the merge rework (the playlist half is still open) The three symptoms in Round 35 shared a root cause; that round fixed the -performance half. This is the correctness half. +performance half. Round 38 took the play-count and dialog items; the two +playlist items below are what's left. - [ ] **Playlist merges lose position.** `_merge_playlist` is a 2-way union with no common ancestor: it takes one side's order wholesale and *appends* the @@ -31,13 +34,10 @@ performance half. This is the correctness half. So "most recently edited" often means "most recently resized". Give `Playlist` a `date_modified` bumped only in `_set_track_ids`, and merge on that. -- [ ] **`max()` play counts discard concurrent plays.** Base 100, one machine - plays 5 (105), the other plays 3 (103) → merge keeps 105 and those 3 are - gone. Per-machine `plays/.json` journal (machine id kept - outside the synced dir): only its owner ever writes it, so play data can - never conflict, effective count = base + sum of journals, and `library.json` - stops being rewritten every 3 s during playback — which is what generates - the conflicts in the first place. +- [x] **`max()` play counts discard concurrent plays.** Done as described — + per-machine `plays/.json` journals, machine id in the config + dir. Three plays on the real library now write 83 bytes instead of 15 MB, + and `library.json` isn't touched at all. - [x] **Quiet the merge dialog.** One dialog per session: each merge is folded in as a timestamped entry (newest first) instead of opening another window. Fifteen had stacked up. Kept the window for every merge rather diff --git a/lintunes/__init__.py b/lintunes/__init__.py index 87127ca..517314e 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.8.0" +__version__ = "0.9.0" diff --git a/lintunes/library_manager.py b/lintunes/library_manager.py index 7174cfd..ad32e6d 100644 --- a/lintunes/library_manager.py +++ b/lintunes/library_manager.py @@ -12,6 +12,7 @@ from lintunes.importers.file_importer import organized_destination, unique_path from lintunes.models import Library, Playlist, PlaylistType from lintunes.perf import timed from lintunes.storage import json_storage +from lintunes.storage.play_journal import PlayJournal from lintunes.undo import Command, UndoStack @@ -53,6 +54,13 @@ class LibraryManager(QObject): self._dirty_playlists: set[str] = set() self._deleted_playlists: set[str] = set() + # Play counts live in per-machine journals, not library.json — see + # storage/play_journal. Folding them in here means the rest of the app + # keeps reading one plain track.play_count. + self.play_journal = PlayJournal() + self.play_journal.load(self.data_dir, self.library) + self._dirty_journal = False + self._save_timer = QTimer(self) self._save_timer.setSingleShot(True) self._save_timer.setInterval(SAVE_DEBOUNCE_MS) @@ -680,9 +688,14 @@ class LibraryManager(QObject): track = self.library.tracks.get(track_id) if not track: return + now = _utc_now_iso() track.play_count += 1 - track.play_date_utc = _utc_now_iso() - self._dirty_tracks = True + track.play_date_utc = now + # Deliberately does NOT mark library.json dirty: a play only moves this + # machine's journal, so finishing a track no longer rewrites 15 MB (and + # no longer hands Syncthing a conflict once per song). + self.play_journal.record_play(track_id, now) + self._dirty_journal = True self._schedule_save() self.track_updated.emit(track_id) self._touch_smart({"play_count", "play_date_utc"}) @@ -691,9 +704,11 @@ class LibraryManager(QObject): track = self.library.tracks.get(track_id) if not track: return + now = _utc_now_iso() track.skip_count += 1 - track.skip_date = _utc_now_iso() - self._dirty_tracks = True + track.skip_date = now + self.play_journal.record_skip(track_id, now) + self._dirty_journal = True self._schedule_save() self.track_updated.emit(track_id) self._touch_smart({"skip_count", "skip_date"}) @@ -801,10 +816,16 @@ class LibraryManager(QObject): def flush(self): self._save_timer.stop() + if self._dirty_journal: + self.play_journal.save(self.data_dir) + self._dirty_journal = False + self._record_sig(self.data_dir / "plays" + / f"{self.play_journal.machine}.json") if self._dirty_tracks: with timed("flush library.json (%d tracks)", len(self.library.tracks)): - json_storage.save_tracks(self.library, self.data_dir) + json_storage.save_tracks(self.library, self.data_dir, + self.play_journal) self._dirty_tracks = False self._record_sig(self.data_dir / "library.json") for pid in list(self._dirty_playlists): @@ -831,6 +852,9 @@ class LibraryManager(QObject): playlists_dir = self.data_dir / "playlists" if playlists_dir.exists(): files.extend(sorted(playlists_dir.glob("*.json"))) + plays_dir = self.data_dir / "plays" + if plays_dir.exists(): + files.extend(sorted(plays_dir.glob("*.json"))) return files @staticmethod @@ -877,6 +901,11 @@ class LibraryManager(QObject): or self._dirty_metadata) dirty_pids = set(self._dirty_playlists) disk = json_storage.load_library(self.data_dir) + # Fold the journals onto the disk copy *before* reconciling. The tracks + # in memory already carry effective counts, so without this the merge + # would compare effective against base and the max() would win by an + # amount equal to this machine's journal. + self.play_journal.load(self.data_dir, disk) for tid, disk_track in disk.tracks.items(): mem_track = self.library.tracks.get(tid) diff --git a/lintunes/storage/conflict_resolver.py b/lintunes/storage/conflict_resolver.py index a468867..481785d 100644 --- a/lintunes/storage/conflict_resolver.py +++ b/lintunes/storage/conflict_resolver.py @@ -66,6 +66,8 @@ def resolve_conflicts(data_dir: Path) -> list[ConflictSummary]: summary = _merge_metadata(original_path, conflict_path) elif original_path.parent.name == "playlists": summary = _merge_playlist(original_path, conflict_path) + elif original_path.parent.name == "plays": + summary = _merge_play_journal(original_path, conflict_path) else: summary = _keep_newer(original_path, conflict_path) except Exception as e: # never let a bad file abort startup/live reload @@ -264,6 +266,37 @@ def _merge_playlist(original_path: Path, conflict_path: Path) -> ConflictSummary return ConflictSummary(name, "playlist", lines) +def _merge_play_journal(original_path: Path, conflict_path: Path) -> ConflictSummary: + """Insurance only. A journal is owned by one machine, so it should never + conflict — but if a filesystem or clock oddity produces one, falling through + to _keep_newer would throw away a machine's entire play history. Take the + higher total per track instead.""" + original = read_json(original_path) + incoming = read_json(conflict_path) + merged = dict(original) + changed = 0 + for tid, entry in incoming.items(): + mine = merged.get(tid) + if mine is None: + merged[tid] = entry + changed += 1 + continue + combined = dict(mine) + for count in ("plays", "skips"): + combined[count] = max(mine.get(count, 0), entry.get(count, 0)) + for stamp in ("last_played", "last_skipped"): + a, b = mine.get(stamp), entry.get(stamp) + combined[stamp] = a if b is None else (b if a is None else max(a, b)) + if combined != mine: + changed += 1 + merged[tid] = combined + write_json(original_path, merged) + return ConflictSummary( + f"plays/{original_path.name}", "plays", + [f"{changed} track(s) of play history reconciled (highest total kept)."] + if changed else ["No differences needed reconciling."]) + + def _merge_metadata(original_path: Path, conflict_path: Path) -> ConflictSummary: summary = _keep_newer(original_path, conflict_path) summary.file, summary.kind = "library_metadata.json", "metadata" diff --git a/lintunes/storage/json_storage.py b/lintunes/storage/json_storage.py index 4540552..c5bf9a6 100644 --- a/lintunes/storage/json_storage.py +++ b/lintunes/storage/json_storage.py @@ -22,7 +22,7 @@ def save_library(library: Library, data_dir: Path): playlist_file.unlink() -def save_tracks(library: Library, data_dir: Path): +def save_tracks(library: Library, data_dir: Path, journal=None): data_dir.mkdir(parents=True, exist_ok=True) tracks_dict = {} for tid, track in library.tracks.items(): @@ -31,10 +31,26 @@ def save_tracks(library: Library, data_dir: Path): # Store paths relative to the data dir so the library is portable # across machines that sync the folder to different mount points. data["location"] = to_relative(data["location"], data_dir) + if journal is not None: + # A Track carries the *effective* play count (base plus every + # machine's journal). library.json must get the base back — writing + # the effective value here would fold this machine's plays into the + # base and count them twice on the next load. See play_journal. + _write_base_play_fields(data, journal.base_fields(tid)) tracks_dict[str(tid)] = data write_json(data_dir / "library.json", tracks_dict) +def _write_base_play_fields(data: dict, base: dict): + """Put the base play fields back into a to_dict() result, keeping to_dict's + rule that a default-valued field is omitted rather than stored.""" + for field, value in base.items(): + if value in (0, None, ""): + data.pop(field, None) + else: + data[field] = value + + def save_metadata(library: Library, data_dir: Path): data_dir.mkdir(parents=True, exist_ok=True) metadata = { diff --git a/lintunes/storage/play_journal.py b/lintunes/storage/play_journal.py new file mode 100644 index 0000000..8df72f5 --- /dev/null +++ b/lintunes/storage/play_journal.py @@ -0,0 +1,191 @@ +"""Per-machine play journals — the reason play counts can no longer conflict. + +``library.json`` used to carry the live play count, so finishing a track +rewrote all 15 MB of it. Both machines did that, so Syncthing saw two edits to +one big file between syncs and produced a conflict file roughly once per song. +The merge then took ``max()`` of the two counts, silently discarding whichever +side had played less. + +Now ``library.json`` holds only a **base** count, and each machine additionally +owns exactly one file ``/plays/.json`` recording *its own* +per-track totals. Effective count = base + the sum of every journal. + +Three properties fall out of that: + +* **No conflict is possible.** A journal file is only ever written by the + machine that owns it, so two machines never touch the same file. +* **No compaction step.** The journal stores running totals, not an append log, + so it is bounded by "tracks this machine has played" and never needs folding + back into the base — which is what would risk double-counting. +* **Version skew is safe.** A machine still on older code keeps bumping its own + base in ``library.json``; we never write base, so the sum stays correct. When + it updates, its already-counted plays stay frozen in base and it starts its + own journal. + +The machine id lives in the config dir, deliberately *outside* the synced data +dir — a synced id would make two machines share one journal. +""" +import uuid +from pathlib import Path + +from lintunes.config import config_path +from lintunes.storage.json_storage import read_json, write_json + + +JOURNAL_DIR = "plays" + +# The four Track fields a journal contributes to. Everything else about a track +# still lives in library.json. +PLAY_FIELDS = ("play_count", "skip_count", "play_date_utc", "skip_date") + + +def machine_id() -> str: + """This machine's stable id, created on first use. Kept next to config.json + (not in the data dir) so Syncthing never copies it to the other machine.""" + path = config_path().parent / "machine_id" + try: + text = path.read_text(encoding="utf-8").strip() + if text: + return text + except OSError: + pass + new_id = uuid.uuid4().hex[:12] + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(new_id + "\n", encoding="utf-8") + return new_id + + +def _newer(a, b): + """The later of two ISO-8601 stamps, either of which may be None.""" + if a is None: + return b + if b is None: + return a + return a if a >= b else b + + +class PlayJournal: + """The base counts from library.json plus every machine's journal. + + ``load()`` snapshots the base off the freshly loaded Tracks and then folds + the journals in, so the rest of the app keeps reading one plain + ``track.play_count``. Only the save path needs to know the difference — + see ``base_fields``. + """ + + def __init__(self, machine=None): + self.machine = machine or machine_id() + self._base = {} # track_id -> {field: value} as stored in library.json + self._own = {} # track_id -> {"plays", "skips", "last_played", "last_skipped"} + self._others = {} # machine id -> same shape as _own + self.dirty = False + + # ---- loading ---- + + def load(self, data_dir: Path, library): + """Read every journal in the data dir and fold them into ``library``. + + ``library`` must be one whose tracks still hold the **base** counts — + the freshly loaded library at startup, or the disk copy during a reload. + Passing an already-folded library would promote the effective count to + base and double-count it. + """ + self._base = { + tid: {f: getattr(track, f) for f in PLAY_FIELDS} + for tid, track in library.tracks.items() + } + # Our own journal is only ever written by us, so unsaved bumps in memory + # are always more current than the file — never re-read over them. + keep_own = self._own if self.dirty else None + self._own = {} + self._others = {} + journal_dir = Path(data_dir) / JOURNAL_DIR + if journal_dir.is_dir(): + for path in sorted(journal_dir.glob("*.json")): + if path.name.endswith(".tmp"): + continue + try: + entries = read_json(path) + except (ValueError, OSError): + continue # a half-synced journal must never abort startup + entries = {int(k): v for k, v in entries.items() + if isinstance(v, dict)} + if path.stem == self.machine: + self._own = entries + else: + self._others[path.stem] = entries + if keep_own is not None: + self._own = keep_own + else: + self.dirty = False + self.apply(library) + + def apply(self, library): + """Set every track's play fields to base + the sum of all journals.""" + for tid, track in library.tracks.items(): + added = sum(e[tid].get("plays", 0) + for e in self._all_journals() if tid in e) + skipped_n = sum(e[tid].get("skips", 0) + for e in self._all_journals() if tid in e) + base = self._base.get(tid) + if base is None: + # A track added after the snapshot. Its current count already + # includes anything recorded into the journal since, so back + # that out rather than baking it into the base. + base = {f: getattr(track, f) for f in PLAY_FIELDS} + base["play_count"] = (base["play_count"] or 0) - added + base["skip_count"] = (base["skip_count"] or 0) - skipped_n + self._base[tid] = base + plays = (base["play_count"] or 0) + added + skips = (base["skip_count"] or 0) + skipped_n + played = base["play_date_utc"] + skipped = base["skip_date"] + for entries in self._all_journals(): + e = entries.get(tid) + if not e: + continue + played = _newer(played, e.get("last_played")) + skipped = _newer(skipped, e.get("last_skipped")) + track.play_count = plays + track.skip_count = skips + track.play_date_utc = played + track.skip_date = skipped + + def _all_journals(self): + yield self._own + yield from self._others.values() + + # ---- recording ---- + + def record_play(self, track_id: int, when: str): + entry = self._own.setdefault(track_id, {}) + entry["plays"] = entry.get("plays", 0) + 1 + entry["last_played"] = when + self.dirty = True + + def record_skip(self, track_id: int, when: str): + entry = self._own.setdefault(track_id, {}) + entry["skips"] = entry.get("skips", 0) + 1 + entry["last_skipped"] = when + self.dirty = True + + # ---- saving ---- + + def base_fields(self, track_id: int) -> dict: + """What library.json should hold for this track. Writing the *effective* + count here instead would double-count it on the next load — this is the + one place in the change where that can go wrong.""" + base = self._base.get(track_id) + if base is None: # never seen at load time — nothing to preserve + return {"play_count": 0, "skip_count": 0, + "play_date_utc": None, "skip_date": None} + return dict(base) + + def save(self, data_dir: Path): + """Write this machine's journal only. Nothing else ever writes this file, + so it cannot conflict.""" + journal_dir = Path(data_dir) / JOURNAL_DIR + journal_dir.mkdir(parents=True, exist_ok=True) + write_json(journal_dir / f"{self.machine}.json", + {str(k): v for k, v in sorted(self._own.items())}) + self.dirty = False diff --git a/lintunes/sync_watcher.py b/lintunes/sync_watcher.py index 559de92..87a0b4e 100644 --- a/lintunes/sync_watcher.py +++ b/lintunes/sync_watcher.py @@ -34,11 +34,15 @@ class LibrarySyncWatcher(QObject): the old inode, so we re-add paths after every event.""" wanted = [self._data_dir, self._data_dir / "playlists", + self._data_dir / "plays", self._data_dir / "library.json", self._data_dir / "library_metadata.json"] playlists_dir = self._data_dir / "playlists" if playlists_dir.exists(): wanted.extend(playlists_dir.glob("*.json")) + plays_dir = self._data_dir / "plays" + if plays_dir.exists(): + wanted.extend(plays_dir.glob("*.json")) already = set(self._watcher.files()) | set(self._watcher.directories()) to_add = [str(p) for p in wanted if p.exists() and str(p) not in already] if to_add: diff --git a/tasks-done.md b/tasks-done.md index a69985d..10a4136 100644 --- a/tasks-done.md +++ b/tasks-done.md @@ -1,5 +1,45 @@ ## Done +### Round 38 (2026-08-20) — One merge window, and play counts that can't conflict (v0.9.0) + +Fifteen "Synced changes merged" windows were stacked on the desktop. Two +independent bugs, and the `.resolved/` backups said which mattered: every one of +the last nine `library.json` merges was ~98% play counts (43-47 of ~45 changed +tracks) plus exactly one real edit — the same single rating each time. The same +~45 tracks disagreed in *every* merge and the count only crept down (48 → 44 over +a day), so `max()` had been quietly discarding plays for at least that long. + +- [x] **One window, not one per merge.** `show_conflict_summary` built a fresh + `ConflictSummaryDialog` each time and only reassigned the attribute — the + old dialog stayed a live child of the window. Since + `check_for_external_changes` runs `resolve_conflicts` on every Syncthing + watch tick, that was one window per merge for the life of the process. The + dialog is now a session log: `add_event()` folds each merge in as its own + timestamped entry (newest first), the restore button names the merge it + would undo, and `finished` clears MainWindow's reference so a close lets + the next merge open a fresh one. +- [x] **Per-machine play journals.** `library.json` now holds only a base count; + each machine owns `plays/.json` with its own per-track totals, + and the effective count is base + the sum of all journals. Only the owner + writes its journal, so play data can't conflict; totals rather than an + append log, so there's no compaction step to double-count in; and a machine + still on older code keeps bumping its own base, which stays additive with + our journal, so the version-skew window is safe too. On the real 21k + library: three plays wrote **83 bytes** and left `library.json` untouched, + where before each one rewrote 15 MB. +- [x] **The two hazards, pinned by tests.** `save_tracks` must write the base and + `PlayJournal.load` must be handed a base-valued library — the second is why + `reload_from_disk` folds the journals onto the `disk` copy *before* + reconciling, or the max() against the effective in-memory count would win + by exactly this machine's journal. +- [x] Journal conflicts merge by highest total rather than newest-wins. They + shouldn't be possible; if a filesystem oddity makes one, falling through to + `_keep_newer` would discard a machine's whole history. + +Deliberately not done: lossless merges still open the window rather than +demoting to a status-bar line. With journals landing, a merge stops being +routine — the ones left are real edits, and worth seeing. + ### Round 37 (2026-08-19) — Export a playlist: a folder, or a whole website (v0.8.0) `File → Export Playlist…`, and the same item on a playlist's right-click menu. diff --git a/tests/test_round38.py b/tests/test_round38.py index 57da872..3f5369c 100644 --- a/tests/test_round38.py +++ b/tests/test_round38.py @@ -99,3 +99,213 @@ class TestWindowReuse: host.show_conflict_summary([_summary()]) assert host._conflict_dialog is not None assert host._conflict_dialog is not first + + +# -------------------------------------------------------------------------- +# Per-machine play journals +# -------------------------------------------------------------------------- + +from lintunes.models import Library, Track # noqa: E402 +from lintunes.storage import json_storage # noqa: E402 +from lintunes.storage.play_journal import PlayJournal # noqa: E402 + + +@pytest.fixture +def data_dir(tmp_path, monkeypatch): + """A data dir with a two-track library, and a machine id kept out of the + real ~/.config.""" + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "config")) + d = tmp_path / "data" + library = Library(tracks={ + 1: Track(track_id=1, name="Thunder Peel", play_count=102), + 2: Track(track_id=2, name="Black Blood", play_count=42), + }) + json_storage.save_library(library, d) + return d + + +def _base_on_disk(data_dir, tid=1, field="play_count"): + return json_storage.read_json(data_dir / "library.json")[str(tid)].get(field, 0) + + +def _write_journal(data_dir, machine, entries): + (data_dir / "plays").mkdir(parents=True, exist_ok=True) + json_storage.write_json(data_dir / "plays" / f"{machine}.json", entries) + + +class TestPlayJournal: + def test_machine_id_is_stable_and_outside_the_data_dir(self, data_dir, tmp_path): + from lintunes.storage.play_journal import machine_id + first = machine_id() + assert first and machine_id() == first + # A synced id would make both machines share one journal. + assert not (data_dir / "machine_id").exists() + assert (tmp_path / "config" / "lintunes" / "machine_id").exists() + + def test_effective_count_is_base_plus_every_journal(self, data_dir): + _write_journal(data_dir, "machine-a", {"1": {"plays": 5, "last_played": "2026-08-20T10:00:00"}}) + _write_journal(data_dir, "machine-b", {"1": {"plays": 3, "last_played": "2026-08-20T12:00:00"}}) + + library = json_storage.load_library(data_dir) + PlayJournal(machine="machine-a").load(data_dir, library) + + # This is the case max() used to throw away: 102 + 5 + 3, not 107. + assert library.tracks[1].play_count == 110 + assert library.tracks[1].play_date_utc == "2026-08-20T12:00:00" + assert library.tracks[2].play_count == 42 # untouched + + def test_a_play_moves_the_journal_and_leaves_library_json_alone(self, data_dir): + library = json_storage.load_library(data_dir) + journal = PlayJournal(machine="machine-a") + journal.load(data_dir, library) + + before = (data_dir / "library.json").read_bytes() + journal.record_play(1, "2026-08-20T18:42:00") + journal.save(data_dir) + + assert (data_dir / "library.json").read_bytes() == before + assert json_storage.read_json(data_dir / "plays" / "machine-a.json") == { + "1": {"plays": 1, "last_played": "2026-08-20T18:42:00"}} + + def test_save_tracks_writes_the_base_not_the_effective_count(self, data_dir): + """The one sharp edge: writing the folded total back into library.json + would fold this machine's plays into the base and count them twice.""" + _write_journal(data_dir, "machine-a", {"1": {"plays": 5}}) + library = json_storage.load_library(data_dir) + journal = PlayJournal(machine="machine-a") + journal.load(data_dir, library) + assert library.tracks[1].play_count == 107 + + json_storage.save_tracks(library, data_dir, journal) + assert _base_on_disk(data_dir) == 102 + + # And a second round trip must not drift either. + again = json_storage.load_library(data_dir) + PlayJournal(machine="machine-a").load(data_dir, again) + assert again.tracks[1].play_count == 107 + + def test_zero_base_is_omitted_from_library_json(self, data_dir): + """to_dict() omits default-valued fields; the base rewrite must too, or + 21k tracks each grow a redundant "play_count": 0.""" + library = json_storage.load_library(data_dir) + library.tracks[3] = Track(track_id=3, name="New Song") + journal = PlayJournal(machine="machine-a") + journal.load(data_dir, library) + journal.record_play(3, "2026-08-20T18:42:00") + + json_storage.save_tracks(library, data_dir, journal) + assert "play_count" not in json_storage.read_json( + data_dir / "library.json")["3"] + + def test_other_machine_on_old_code_does_not_double_count(self, data_dir): + """While the other machine still bumps its base in library.json, our + journal stays additive on top — no double count, no loss.""" + _write_journal(data_dir, "machine-a", {"1": {"plays": 5}}) + library = json_storage.load_library(data_dir) + PlayJournal(machine="machine-a").load(data_dir, library) + assert library.tracks[1].play_count == 107 + + # The other machine, still on old code, syncs in a base bumped by 3. + raw = json_storage.read_json(data_dir / "library.json") + raw["1"]["play_count"] = 105 + json_storage.write_json(data_dir / "library.json", raw) + + library = json_storage.load_library(data_dir) + PlayJournal(machine="machine-a").load(data_dir, library) + assert library.tracks[1].play_count == 110 + + def test_unsaved_plays_survive_a_journal_reread(self, data_dir): + """A reload re-reads every journal; ours is only ever written by us, so + in-memory bumps must not be read over.""" + library = json_storage.load_library(data_dir) + journal = PlayJournal(machine="machine-a") + journal.load(data_dir, library) + journal.record_play(1, "2026-08-20T18:42:00") + + fresh = json_storage.load_library(data_dir) + journal.load(data_dir, fresh) # what reload_from_disk does + assert fresh.tracks[1].play_count == 103 + assert journal.dirty + + +class TestManagerRoundTrip: + """End to end through LibraryManager, which is where a base/effective mix-up + would actually cost trav play counts.""" + + @pytest.fixture + def manager(self, data_dir, qapp): + from lintunes.library_manager import LibraryManager + return LibraryManager(json_storage.load_library(data_dir), data_dir) + + def test_playing_does_not_rewrite_library_json(self, manager, data_dir): + before = (data_dir / "library.json").read_bytes() + manager.record_play(1) + manager.flush() + + assert (data_dir / "library.json").read_bytes() == before + assert manager.library.tracks[1].play_count == 103 + + def test_a_play_survives_a_restart_exactly_once(self, manager, data_dir): + manager.record_play(1) + manager.flush() + + from lintunes.library_manager import LibraryManager + restarted = LibraryManager(json_storage.load_library(data_dir), data_dir) + assert restarted.library.tracks[1].play_count == 103 + assert _base_on_disk(data_dir) == 102 + + def test_an_edit_after_a_play_still_writes_the_base(self, manager, data_dir): + """The dangerous sequence: a play inflates the in-memory count, then an + unrelated edit flushes the whole library.json.""" + manager.record_play(1) + manager.add_track(Track(track_id=9, name="Sev Beni Beni")) + manager.flush() + + assert _base_on_disk(data_dir) == 102 + assert manager.library.tracks[1].play_count == 103 + + from lintunes.library_manager import LibraryManager + restarted = LibraryManager(json_storage.load_library(data_dir), data_dir) + assert restarted.library.tracks[1].play_count == 103 + assert 9 in restarted.library.tracks + + def test_reload_does_not_inflate_the_count(self, manager, data_dir): + """reload_from_disk reconciles in-memory (effective) against disk (base); + folding the journals onto the disk copy first is what keeps max() honest.""" + manager.record_play(1) + manager.flush() + manager.reload_from_disk() + assert manager.library.tracks[1].play_count == 103 + + manager.reload_from_disk() + manager.flush() + assert manager.library.tracks[1].play_count == 103 + assert _base_on_disk(data_dir) == 102 + + def test_the_other_machines_plays_arrive_on_reload(self, manager, data_dir): + _write_journal(data_dir, "machine-other", {"1": {"plays": 7}}) + manager.reload_from_disk() + assert manager.library.tracks[1].play_count == 109 + + +class TestJournalConflictInsurance: + """A journal has one writer, so it should never conflict. If one ever does, + falling through to newest-wins would discard a machine's whole history.""" + + def test_conflicting_journals_merge_by_highest_total(self, data_dir): + from lintunes.storage import conflict_resolver + _write_journal(data_dir, "machine-a", {"1": {"plays": 9, "last_played": "2026-08-20T10:00:00"}, + "2": {"plays": 4}}) + json_storage.write_json( + data_dir / "plays" / "machine-a.sync-conflict-20260820-184200-ABCDEFG.json", + {"1": {"plays": 3, "last_played": "2026-08-20T18:00:00"}, + "3": {"plays": 6}}) + + summaries = conflict_resolver.resolve_conflicts(data_dir) + assert [s.kind for s in summaries] == ["plays"] + + merged = json_storage.read_json(data_dir / "plays" / "machine-a.json") + assert merged["1"]["plays"] == 9 # not 3 + assert merged["1"]["last_played"] == "2026-08-20T18:00:00" + assert merged["2"]["plays"] == 4 # kept + assert merged["3"]["plays"] == 6 # gained