v0.9.0: play counts that can't conflict
The merge windows kept coming because finishing a track rewrote all 15 MB of library.json. Both machines did that, so Syncthing saw two edits to one big file between syncs and produced a conflict file roughly once per song — and the merge then took max() of the two counts, discarding whichever side had played less. The .resolved/ backups showed the last nine library.json merges were ~98% play counts plus exactly one real edit, with the same ~45 tracks disagreeing every time and the count only creeping down over a day. library.json now holds only a base count. Each machine owns plays/<machine-id>.json with its own per-track totals, and the effective count is base + the sum of every journal. Only the owner writes its journal, so play data can't conflict; the journal stores 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. Two places carry the whole hazard, and both are pinned by tests: save_tracks writes journal.base_fields(), never the Track's effective count, and PlayJournal.load must be given a base-valued library — which is why reload_from_disk folds the journals onto the disk copy before reconciling. On the real 21k library: three plays write 83 bytes and leave library.json untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M8Mzze7shr5pZoEgKQU5NW
This commit is contained in:
Claude Opus 5
@@ -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/<persistent_id>.json` per playlist. Writes are atomic (`*.json.tmp`
|
||||
the data dir: `library.json` (all tracks), `library_metadata.json`, one
|
||||
`playlists/<persistent_id>.json` per playlist, and one
|
||||
`plays/<machine-id>.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/<machine-id>.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
|
||||
|
||||
@@ -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/<machine-id>.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/<machine-id>.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
|
||||
|
||||
@@ -1,3 +1,3 @@
|
||||
"""LinTunes — iTunes-style music library manager and player for Linux."""
|
||||
|
||||
__version__ = "0.8.0"
|
||||
__version__ = "0.9.0"
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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 = {
|
||||
|
||||
@@ -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 ``<data_dir>/plays/<machine-id>.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
|
||||
@@ -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:
|
||||
|
||||
@@ -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/<machine-id>.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.
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user