v0.12.0: the merge report stops reporting things you didn't do

"unplayed", "missed and never skipped" and "top 100 in past year" showed up in
the merge window constantly, and nobody had touched their rules. A live smart
playlist's membership is derived, but it was being persisted — and
recompute_smart_playlist rewrites it (and bumped date_modified) every time a
play count moves. Both machines did that against the same file after every
song, so Syncthing conflicted on a list the next load throws away and rebuilds
anyway. Membership now stays in memory: save_playlist writes track_ids: [] for
a live smart playlist, and the recompute passes touch=False so it marks nothing
dirty and moves no timestamp. live_update=False and unsupported criteria are
unchanged — their track_ids are a snapshot, which is real content. Since
_mark_playlist also dirties the metadata, library_metadata.json stops being
rewritten every song too.

The rest was presentation. Every summary read at the same weight, so someone
resizing a column on the other machine popped and raised the same window as a
21-track reconciliation, described as "Library columns/settings taken from the
most recently edited copy." Summaries now carry a level — WARNING for a merge
that couldn't resolve cleanly or discarded a side, CHANGE for real content,
INFO for cosmetic or derived — and the dialog has a Show: selector that filters
to one level and above. It opens at the highest level in the batch, so nothing
routine steals focus and a blank window can't happen; a later warning always
pulls the view back up to it. The wording says what happened instead of how the
merge works: a metadata merge names the keys that actually differed, and
adopting the other machine's music folder grades CHANGE rather than INFO,
because that one is a setting somebody chose.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y9ZEFi4qNJ39FMiBtiAxy2
This commit is contained in:
2026-08-25 14:53:37 -04:00
co-authored by Claude Opus 5
parent a61cbaa369
commit 521db2f81d
9 changed files with 631 additions and 35 deletions
+17 -1
View File
@@ -64,7 +64,13 @@ persistence) → GUI (Qt widgets that read the manager and connect to its signal
`Playlist.date_modified` — bumped *only* in `Playlist.date_modified` — bumped *only* in
`LibraryManager._set_track_ids` — not by the file's mtime, which moves for `LibraryManager._set_track_ids` — not by the file's mtime, which moves for
cosmetic reasons. `library_manager._reconcile_playlist` uses the same helper cosmetic reasons. `library_manager._reconcile_playlist` uses the same helper
for the live-reload path. for the live-reload path. Since Round 42 every `ConflictSummary` carries a
**`level`** (`WARNING` / `CHANGE` / `INFO`): a merge that only reconciled
column widths, or a smart playlist whose rules are byte-identical on both
sides, is `INFO` and must never read as an edit the user made. The dialog
(`gui/conflict_dialog.py`) shows one level *and above* and opens at the
highest level in the batch, so a routine merge never steals focus but a
blank window is impossible either.
- **`lintunes/storage/play_journal.py`** — why play counts can't conflict. Since - **`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 Round 38 `library.json` holds only a **base** count and each machine owns
@@ -233,6 +239,16 @@ persistence) → GUI (Qt widgets that read the manager and connect to its signal
change; `track_table._on_section_resized` ignores that section, or resizing change; `track_table._on_section_resized` ignores that section, or resizing
the window would rewrite the open playlist's JSON (and hand Syncthing a the window would rewrite the open playlist's JSON (and hand Syncthing a
conflict) purely for a width `apply_settings` overrides on load anyway. conflict) purely for a width `apply_settings` overrides on load anyway.
Round 42 is the same rule one level down: a **live smart playlist's membership
is never persisted** (`Playlist.has_derived_membership` gates it in
`json_storage.save_playlist`, which writes `track_ids: []` +
`derived_membership: true`) and `recompute_smart_playlist` passes
`touch=False` to `_set_track_ids` so it marks nothing dirty and moves no
timestamp. Membership is rebuilt from the criteria on every load
(`recompute_all_smart`), so storing it only bought a conflict per song — one
per finished track, on the same file, from both machines. The exceptions are
`live_update=False` and `unsupported` criteria: their `track_ids` *are* the
content (a snapshot), so they still persist and still count as edits.
- **Qt/Wayland gotchas (GNOME/Mutter):** `QDrag.setPixmap` / `setDragCursor` / - **Qt/Wayland gotchas (GNOME/Mutter):** `QDrag.setPixmap` / `setDragCursor` /
`QCursor.pos()` are unreliable during a drag — `gui/drag_ghost.py` paints its `QCursor.pos()` are unreliable during a drag — `gui/drag_ghost.py` paints its
+1 -1
View File
@@ -1,3 +1,3 @@
"""LinTunes — iTunes-style music library manager and player for Linux.""" """LinTunes — iTunes-style music library manager and player for Linux."""
__version__ = "0.11.1" __version__ = "0.12.0"
+91 -7
View File
@@ -4,6 +4,12 @@ option to open or restore the pre-merge backup.
One dialog per session, not one per merge: Syncthing can deliver conflicts every One dialog per session, not one per merge: Syncthing can deliver conflicts every
few minutes, so ``add_event`` folds each new merge into the open window as a few minutes, so ``add_event`` folds each new merge into the open window as a
timestamped entry instead of stacking another window on the desktop. timestamped entry instead of stacking another window on the desktop.
Entries are graded (see conflict_resolver's WARNING/CHANGE/INFO) and the window
shows one level and above. It opens at whatever the *highest* level in the batch
is, so there is always something to read: a batch of nothing but column-width
merges opens on the details view rather than blank, and a real divergence is
never hidden behind a filter the user left set to something quieter.
""" """
from datetime import datetime from datetime import datetime
from pathlib import Path from pathlib import Path
@@ -11,8 +17,11 @@ from pathlib import Path
from PyQt6.QtCore import QUrl from PyQt6.QtCore import QUrl
from PyQt6.QtGui import QDesktopServices from PyQt6.QtGui import QDesktopServices
from PyQt6.QtWidgets import ( from PyQt6.QtWidgets import (
QDialog, QVBoxLayout, QHBoxLayout, QLabel, QTextEdit, QPushButton, QDialog, QVBoxLayout, QHBoxLayout, QComboBox, QLabel, QTextEdit,
QMessageBox) QPushButton, QMessageBox)
from lintunes.storage.conflict_resolver import (
WARNING, CHANGE, INFO, LEVEL_ORDER, at_least, max_level)
INTRO = ("LinTunes found changes made on more than one machine and merged them " INTRO = ("LinTunes found changes made on more than one machine and merged them "
@@ -20,12 +29,23 @@ INTRO = ("LinTunes found changes made on more than one machine and merged them "
"versions were backed up first — restore them if a merge isn't what " "versions were backed up first — restore them if a merge isn't what "
"you wanted.") "you wanted.")
# Coarsest first, so the combo reads top-down like a volume knob.
LEVEL_CHOICES = [
(WARNING, "Warnings only"),
(CHANGE, "Changes and warnings"),
(INFO, "Everything"),
]
MARKERS = {WARNING: "!", CHANGE: "", INFO: "·"}
class ConflictSummaryDialog(QDialog): class ConflictSummaryDialog(QDialog):
def __init__(self, summaries, manager, parent=None): def __init__(self, summaries, manager, parent=None):
super().__init__(parent) super().__init__(parent)
self._manager = manager self._manager = manager
self._events = [] # newest last: (datetime, list[ConflictSummary]) self._events = [] # newest last: (datetime, list[ConflictSummary])
self._level = INFO
self._user_picked = False
self.setWindowTitle("Synced changes merged") self.setWindowTitle("Synced changes merged")
self.resize(560, 440) self.resize(560, 440)
@@ -35,6 +55,16 @@ class ConflictSummaryDialog(QDialog):
self._intro.setWordWrap(True) self._intro.setWordWrap(True)
layout.addWidget(self._intro) layout.addWidget(self._intro)
filter_row = QHBoxLayout()
filter_row.addWidget(QLabel("Show:"))
self._level_box = QComboBox()
for level, label in LEVEL_CHOICES:
self._level_box.addItem(label, level)
self._level_box.activated.connect(self._on_level_picked)
filter_row.addWidget(self._level_box)
filter_row.addStretch(1)
layout.addLayout(filter_row)
self._body = QTextEdit() self._body = QTextEdit()
self._body.setReadOnly(True) self._body.setReadOnly(True)
layout.addWidget(self._body, 1) layout.addWidget(self._body, 1)
@@ -57,9 +87,49 @@ class ConflictSummaryDialog(QDialog):
# ---- events ---- # ---- events ----
def add_event(self, summaries, when=None): def add_event(self, summaries, when=None) -> bool:
"""Fold another merge into this window as its own timestamped entry.""" """Fold another merge into this window as its own timestamped entry.
self._events.append((when or datetime.now(), list(summaries or [])))
Returns True if the new batch has anything to show at the level now on
screen — the caller uses that to decide whether to raise the window, so
a column resize on the other machine never steals focus."""
summaries = list(summaries or [])
self._events.append((when or datetime.now(), summaries))
self._set_level(self._level_for(summaries))
self._refresh()
return any(at_least(s.level, self._level) for s in summaries)
@property
def level(self) -> str:
return self._level
def _level_for(self, incoming) -> str:
"""Which level to display now that ``incoming`` has arrived.
Untouched by the user, the view follows the most serious thing seen so
far. Once the user has picked a level, their choice stands — except that
a batch more serious than what they are looking at pulls the view back
up to it."""
incoming_max = max_level(incoming)
if not self._user_picked:
return max(
(max_level(s) for _when, s in self._events),
key=lambda lvl: LEVEL_ORDER.get(lvl, 0), default=INFO)
if LEVEL_ORDER.get(incoming_max, 0) > LEVEL_ORDER.get(self._level, 0):
return incoming_max
return self._level
def _set_level(self, level: str):
self._level = level
index = self._level_box.findData(level)
if index >= 0:
self._level_box.blockSignals(True)
self._level_box.setCurrentIndex(index)
self._level_box.blockSignals(False)
def _on_level_picked(self, index: int):
self._user_picked = True
self._level = self._level_box.itemData(index)
self._refresh() self._refresh()
@property @property
@@ -85,16 +155,30 @@ class ConflictSummaryDialog(QDialog):
def _format(self) -> str: def _format(self) -> str:
blocks = [] blocks = []
hidden = 0
shown = 0
multi = len(self._events) > 1 multi = len(self._events) > 1
for when, summaries in reversed(self._events): # newest first for when, summaries in reversed(self._events): # newest first
visible = [s for s in summaries if at_least(s.level, self._level)]
hidden += len(summaries) - len(visible)
shown += len(visible)
if not visible:
continue # no orphan timestamp headers for a filtered-out merge
lines = [] lines = []
if multi: if multi:
lines.append(f"── {when.strftime('%-I:%M %p')} " + "" * 30) lines.append(f"── {when.strftime('%-I:%M %p')} " + "" * 30)
for s in summaries: for s in visible:
lines.append(f" {s.file}") lines.append(f"{MARKERS.get(s.level, '')} {s.file}")
lines.extend(f" {line}" for line in s.lines) lines.extend(f" {line}" for line in s.lines)
lines.append("") lines.append("")
blocks.append("\n".join(lines).rstrip()) blocks.append("\n".join(lines).rstrip())
if not shown:
blocks.append("Nothing at this level.")
if hidden:
entries = "entry" if hidden == 1 else "entries"
blocks.append(f"({hidden} routine {entries} hidden — switch to "
f"“Everything” to see them.)")
return "\n\n".join(blocks) return "\n\n".join(blocks)
# ---- actions ---- # ---- actions ----
+14 -9
View File
@@ -751,22 +751,27 @@ class MainWindow(QMainWindow):
def _on_conflict_resolved(self, summaries): def _on_conflict_resolved(self, summaries):
self.statusBar().showMessage( self.statusBar().showMessage(
f"Merged changes from your other machine " f"Merged changes from your other machine "
f"({len(summaries)} item(s)) — see details", 8000) f"({len(summaries)} item(s))", 8000)
self.show_conflict_summary(summaries) self.show_conflict_summary(summaries)
def show_conflict_summary(self, summaries): def show_conflict_summary(self, summaries):
"""Show (or extend) the one merge window. Syncthing can deliver conflicts """Show (or extend) the one merge window. Syncthing can deliver conflicts
every few minutes; each merge becomes another entry in the open window every few minutes; each merge becomes another entry in the open window
rather than another window.""" rather than another window.
Only raise it when the new batch has something to say at the level on
screen — otherwise the entry is folded in quietly and is there when the
user dials the filter down."""
from lintunes.gui.conflict_dialog import ConflictSummaryDialog from lintunes.gui.conflict_dialog import ConflictSummaryDialog
if self._conflict_dialog is not None: if self._conflict_dialog is not None:
self._conflict_dialog.add_event(summaries) if self._conflict_dialog.add_event(summaries):
else: self._conflict_dialog.raise_()
# Modeless so a merge never blocks playback or what you're doing. return
self._conflict_dialog = ConflictSummaryDialog( # Modeless so a merge never blocks playback or what you're doing.
summaries, self._manager, self) self._conflict_dialog = ConflictSummaryDialog(
self._conflict_dialog.finished.connect(self._on_conflict_dialog_closed) summaries, self._manager, self)
self._conflict_dialog.show() self._conflict_dialog.finished.connect(self._on_conflict_dialog_closed)
self._conflict_dialog.show()
self._conflict_dialog.raise_() self._conflict_dialog.raise_()
def _on_conflict_dialog_closed(self, _result): def _on_conflict_dialog_closed(self, _result):
+14 -6
View File
@@ -317,17 +317,23 @@ class LibraryManager(QObject):
self._set_track_ids(pid, after) self._set_track_ids(pid, after)
self._push_track_ids("Reorder Playlist", pid, before, after) self._push_track_ids("Reorder Playlist", pid, before, after)
def _set_track_ids(self, pid: str, ids: list[int]): def _set_track_ids(self, pid: str, ids: list[int], touch: bool = True):
"""Set a playlist's membership. ``touch=False`` means "this wasn't an
edit": no timestamp, no dirty mark, just the signal — used by the smart
recompute, whose result isn't persisted."""
playlist = self.library.playlists.get(pid) playlist = self.library.playlists.get(pid)
if playlist is None: if playlist is None:
return return
playlist.track_ids = list(ids) playlist.track_ids = list(ids)
# The single funnel for every content change (add / remove / reorder / # The single funnel for every content change (add / remove / reorder /
# undo), and deliberately the *only* place date_modified moves — a # undo), and deliberately the *only* place date_modified moves — a
# column resize must not look like an edit to the merge. See # column resize must not look like an edit to the merge. Nor may a smart
# conflict_resolver._playlist_newer. # recompute: it fires every time a play count moves, and bumping the
playlist.date_modified = _utc_now_iso() # timestamp there made "most recently edited" meaningless for smart
self._mark_playlist(pid) # playlists. See conflict_resolver._playlist_newer.
if touch:
playlist.date_modified = _utc_now_iso()
self._mark_playlist(pid)
self.playlist_content_changed.emit(pid) self.playlist_content_changed.emit(pid)
def _push_track_ids(self, label, pid, before, after): def _push_track_ids(self, label, pid, before, after):
@@ -813,7 +819,9 @@ class LibraryManager(QObject):
ids = smart.evaluate(criteria, self.library.tracks.values(), seed=seed) ids = smart.evaluate(criteria, self.library.tracks.values(), seed=seed)
if ids == playlist.track_ids: if ids == playlist.track_ids:
return False return False
self._set_track_ids(pid, ids) # marks dirty + emits; never undoable # Never undoable. For a derived playlist nothing is written or timestamped
# either — the membership lives in memory and is rebuilt on the next load.
self._set_track_ids(pid, ids, touch=not playlist.has_derived_membership)
return True return True
def recompute_all_smart(self, force: bool = False) -> list[str]: def recompute_all_smart(self, force: bool = False) -> list[str]:
+15
View File
@@ -71,6 +71,21 @@ class Playlist:
def is_smart(self) -> bool: def is_smart(self) -> bool:
return self.playlist_type == PlaylistType.SMART return self.playlist_type == PlaylistType.SMART
@property
def has_derived_membership(self) -> bool:
"""True when track_ids are rebuilt from the criteria on every load, so
they are in-memory state rather than something to persist or merge.
A live smart playlist's membership moves every time a play count does.
Persisting it meant both machines rewrote the same file continuously and
Syncthing conflicted on it once a song — for a list the next load throws
away and recomputes anyway. Non-live and `unsupported` criteria are the
exception: their track_ids *are* the content (a snapshot), so they keep
being stored."""
c = self.smart_criteria
return bool(self.is_smart and c is not None and not c.unsupported
and c.live_update)
def to_dict(self) -> dict: def to_dict(self) -> dict:
d = { d = {
"name": self.name, "name": self.name,
+103 -10
View File
@@ -16,6 +16,25 @@ CONFLICT_PATTERN = re.compile(r"^(.+)\.sync-conflict-(\d{8}-\d{6})-\w+(\.\w+)$")
BACKUPS_TO_KEEP = 10 BACKUPS_TO_KEEP = 10
# How much a merge is worth interrupting for. Most conflicts reconcile something
# nobody typed — a column width, a derived list — and burying a real divergence
# under twenty of those is how the merge window became wallpaper.
WARNING = "warning" # couldn't merge cleanly, or one side was discarded
CHANGE = "change" # real content reconciled: playlist order, track fields
INFO = "info" # cosmetic or derived; nothing you did, nothing you lost
LEVEL_ORDER = {INFO: 0, CHANGE: 1, WARNING: 2}
def max_level(summaries) -> str:
"""The most serious level in a batch (INFO when it's empty)."""
return max((s.level for s in summaries),
key=lambda lvl: LEVEL_ORDER.get(lvl, 0), default=INFO)
def at_least(level: str, minimum: str) -> bool:
return LEVEL_ORDER.get(level, 0) >= LEVEL_ORDER.get(minimum, 0)
@dataclass @dataclass
class ConflictSummary: class ConflictSummary:
"""A human-readable record of one merged Syncthing conflict, surfaced to the """A human-readable record of one merged Syncthing conflict, surfaced to the
@@ -24,6 +43,7 @@ class ConflictSummary:
kind: str # "library" | "playlist" | "metadata" | "other" kind: str # "library" | "playlist" | "metadata" | "other"
lines: list[str] = field(default_factory=list) # what was reconciled lines: list[str] = field(default_factory=list) # what was reconciled
backup_dir: str = "" # <data_dir>/.resolved/<timestamp> backup_dir: str = "" # <data_dir>/.resolved/<timestamp>
level: str = CHANGE # WARNING | CHANGE | INFO
def resolve_conflicts(data_dir: Path) -> list[ConflictSummary]: def resolve_conflicts(data_dir: Path) -> list[ConflictSummary]:
@@ -59,7 +79,8 @@ def resolve_conflicts(data_dir: Path) -> list[ConflictSummary]:
summary = ConflictSummary( summary = ConflictSummary(
str(rel), "other", str(rel), "other",
["Deleted here but edited elsewhere — kept the deletion; " ["Deleted here but edited elsewhere — kept the deletion; "
"the other copy is in the backup."]) "the other copy is in the backup."],
level=WARNING)
elif original_path.name == "library.json": elif original_path.name == "library.json":
summary = _merge_library(original_path, conflict_path) summary = _merge_library(original_path, conflict_path)
elif original_path.name == "library_metadata.json": elif original_path.name == "library_metadata.json":
@@ -74,7 +95,8 @@ def resolve_conflicts(data_dir: Path) -> list[ConflictSummary]:
summary = ConflictSummary( summary = ConflictSummary(
str(rel), "other", str(rel), "other",
[f"Could not merge automatically ({e}); both versions are in the " [f"Could not merge automatically ({e}); both versions are in the "
"backup, current copy left as-is."]) "backup, current copy left as-is."],
level=WARNING)
summary.backup_dir = str(backup_dir) summary.backup_dir = str(backup_dir)
summaries.append(summary) summaries.append(summary)
@@ -159,7 +181,8 @@ def _merge_library(original_path: Path, conflict_path: Path) -> ConflictSummary:
if not lines: if not lines:
lines.append("No differences needed reconciling.") lines.append("No differences needed reconciling.")
lines.extend(examples) lines.extend(examples)
return ConflictSummary("library.json", "library", lines) return ConflictSummary("library.json", "library", lines,
level=CHANGE if (changed or added) else INFO)
# Every field _merge_track_fields reads or writes. If two copies of a track # Every field _merge_track_fields reads or writes. If two copies of a track
@@ -292,9 +315,22 @@ def _merge_playlist(original_path: Path, conflict_path: Path) -> ConflictSummary
if original.get("playlist_type") == "smart" or conflict.get("playlist_type") == "smart": if original.get("playlist_type") == "smart" or conflict.get("playlist_type") == "smart":
if conflict_newer: if conflict_newer:
write_json(original_path, conflict) write_json(original_path, conflict)
# Almost always the rules are identical on both sides and only the
# derived membership drifted apart — which is not something the user did
# and not something the merge decides, since the next recompute rebuilds
# it either way. Saying "rules taken from the most recently edited copy"
# for that read as an edit nobody made.
if original.get("smart_criteria") == conflict.get("smart_criteria"):
return ConflictSummary(
name, "playlist",
["Rules are identical on both machines — only the auto-updated "
"membership differed. Rebuilt from the rules."],
level=INFO)
src = "the other machine" if conflict_newer else "this machine"
return ConflictSummary( return ConflictSummary(
name, "playlist", name, "playlist",
["Smart-playlist rules taken from the most recently edited copy."]) [f"Rules taken from {src} (edited more recently)."],
level=CHANGE)
orig_ids = list(original.get("track_ids", [])) orig_ids = list(original.get("track_ids", []))
conf_ids = list(conflict.get("track_ids", [])) conf_ids = list(conflict.get("track_ids", []))
@@ -318,11 +354,21 @@ def _merge_playlist(original_path: Path, conflict_path: Path) -> ConflictSummary
write_json(original_path, original) write_json(original_path, original)
added = len(set(original["track_ids"]) - orig_set) added = len(set(original["track_ids"]) - orig_set)
if orig_ids == conf_ids:
# The two copies held the same tracks in the same order, so they differed
# only in column widths or sort order — a window resize, not an edit.
# Note this is a test on the *inputs*: a merge whose result happens to
# equal our list still discarded the other machine's ordering, and that
# is a decision worth reporting.
return ConflictSummary(
name, "playlist",
["Only the column layout differed; track order unchanged."],
level=INFO)
lines = [f"Order kept from {order_src} (most recently edited)."] lines = [f"Order kept from {order_src} (most recently edited)."]
if added: if added:
lines.append(f"{added} track(s) that were only in the other copy were kept, " lines.append(f"{added} track(s) that were only in the other copy were kept, "
"back in position (nothing is removed on a merge).") "back in position (nothing is removed on a merge).")
return ConflictSummary(name, "playlist", lines) return ConflictSummary(name, "playlist", lines, level=CHANGE)
def _merge_play_journal(original_path: Path, conflict_path: Path) -> ConflictSummary: def _merge_play_journal(original_path: Path, conflict_path: Path) -> ConflictSummary:
@@ -350,16 +396,39 @@ def _merge_play_journal(original_path: Path, conflict_path: Path) -> ConflictSum
changed += 1 changed += 1
merged[tid] = combined merged[tid] = combined
write_json(original_path, merged) write_json(original_path, merged)
if not changed:
return ConflictSummary(f"plays/{original_path.name}", "plays",
["No differences needed reconciling."], level=INFO)
# A journal has exactly one writer, so reaching this line at all means
# something unexpected happened to the file — worth saying out loud.
return ConflictSummary( return ConflictSummary(
f"plays/{original_path.name}", "plays", f"plays/{original_path.name}", "plays",
[f"{changed} track(s) of play history reconciled (highest total kept)."] [f"{changed} track(s) of play history reconciled (highest total kept).",
if changed else ["No differences needed reconciling."]) "A play journal is only ever written by the machine that owns it, so a "
"conflict here is unusual."],
level=WARNING)
# What library_metadata.json actually holds, in words a person recognizes.
_METADATA_LABELS = {
"library_settings": "column layout",
"music_folder": "music folder",
"music_folder_rel": "music folder",
"music_folder_set_at": "music folder",
"import_date": "import date",
"track_count": "track count",
"playlist_count": "playlist count",
}
_MUSIC_KEYS = ("music_folder", "music_folder_rel", "music_folder_set_at") _MUSIC_KEYS = ("music_folder", "music_folder_rel", "music_folder_set_at")
def _merge_metadata(original_path: Path, conflict_path: Path) -> ConflictSummary: def _merge_metadata(original_path: Path, conflict_path: Path) -> ConflictSummary:
"""Mostly cosmetic, so it grades INFO: this file is the library's column
widths and sort order plus a couple of counts, and someone resizing a column
on the other machine is not news. The music folder is the exception — a
deliberate setting, so adopting the other copy's is a real CHANGE."""
# Read both before _keep_newer overwrites one of them. # Read both before _keep_newer overwrites one of them.
try: try:
before = read_json(original_path) before = read_json(original_path)
@@ -368,8 +437,9 @@ def _merge_metadata(original_path: Path, conflict_path: Path) -> ConflictSummary
before = other = None before = other = None
summary = _keep_newer(original_path, conflict_path) summary = _keep_newer(original_path, conflict_path)
summary.file, summary.kind = "library_metadata.json", "metadata" summary.file, summary.kind, summary.level = (
summary.lines = ["Library columns/settings taken from the most recently edited copy."] "library_metadata.json", "metadata", INFO)
summary.lines = [_describe_metadata_diff(before, other)]
# The whole-file pick above goes by mtime, which moves for cosmetic reasons # The whole-file pick above goes by mtime, which moves for cosmetic reasons
# (a column resize rewrites this file). The music folder is a deliberate # (a column resize rewrites this file). The music folder is a deliberate
@@ -387,9 +457,28 @@ def _merge_metadata(original_path: Path, conflict_path: Path) -> ConflictSummary
write_json(original_path, kept) write_json(original_path, kept)
summary.lines.append( summary.lines.append(
"Music folder taken from the copy that set it most recently.") "Music folder taken from the copy that set it most recently.")
summary.level = CHANGE
return summary return summary
def _describe_metadata_diff(original, conflict) -> str:
"""Name what actually differed. "Library columns/settings taken from the most
recently edited copy" described the mechanism, not the event, and read like
an edit the user had made."""
if original is None or conflict is None:
return "Kept the most recently edited copy."
differing = []
for key in sorted(set(original) | set(conflict)):
if original.get(key) == conflict.get(key):
continue
label = _METADATA_LABELS.get(key, key)
if label not in differing: # the three music_folder keys are one thing
differing.append(label)
if not differing:
return "No differences needed reconciling."
return f"Library {', '.join(differing)} differed; kept the newer copy."
def _newer_stamp(candidate, current) -> bool: def _newer_stamp(candidate, current) -> bool:
if not candidate: if not candidate:
return False return False
@@ -397,12 +486,16 @@ def _newer_stamp(candidate, current) -> bool:
def _keep_newer(original_path: Path, conflict_path: Path) -> ConflictSummary: def _keep_newer(original_path: Path, conflict_path: Path) -> ConflictSummary:
"""Last resort for a file with no merge rule: one side is discarded whole,
so it is always worth flagging."""
took = "this machine" took = "this machine"
if conflict_path.stat().st_mtime > original_path.stat().st_mtime: if conflict_path.stat().st_mtime > original_path.stat().st_mtime:
shutil.copy2(conflict_path, original_path) shutil.copy2(conflict_path, original_path)
took = "the other machine" took = "the other machine"
return ConflictSummary(original_path.name, "other", return ConflictSummary(original_path.name, "other",
[f"Kept the copy from {took} (most recently edited)."]) [f"Kept the copy from {took} (most recently edited); "
"the other version is in the backup."],
level=WARNING)
def _stars(rating) -> str: def _stars(rating) -> str:
+9 -1
View File
@@ -73,7 +73,15 @@ def save_metadata(library: Library, data_dir: Path):
def save_playlist(playlist: Playlist, data_dir: Path): def save_playlist(playlist: Playlist, data_dir: Path):
playlists_dir = data_dir / "playlists" playlists_dir = data_dir / "playlists"
playlists_dir.mkdir(parents=True, exist_ok=True) playlists_dir.mkdir(parents=True, exist_ok=True)
write_json(playlists_dir / f"{playlist.persistent_id}.json", playlist.to_dict()) data = playlist.to_dict()
if playlist.has_derived_membership:
# Membership is a pure function of the criteria and the library, and
# every machine recomputes it on load — storing it only bought a
# Syncthing conflict per song. Same shape as the base-play-count rule in
# save_tracks: to_dict() stays honest, the gate lives at the boundary.
data["track_ids"] = []
data["derived_membership"] = True
write_json(playlists_dir / f"{playlist.persistent_id}.json", data)
def delete_playlist_file(persistent_id: str, data_dir: Path): def delete_playlist_file(persistent_id: str, data_dir: Path):
+367
View File
@@ -0,0 +1,367 @@
"""Round 42: the merge report stops shouting, and smart playlists stop
conflicting.
Two halves of one complaint — the merge window was full of entries for things
nobody did:
* A live smart playlist's membership is derived from its rules, but it was being
*persisted*, and `recompute_smart_playlist` rewrites it (and bumped
`date_modified`) every time a play count moves. Both machines did that
continuously against the same file, so `unplayed` handed Syncthing a conflict
once a song — for a list the next load throws away and recomputes anyway.
* Every summary was presented at the same weight, so a column resize on the
other machine popped and raised the same window as a 21-track reconciliation.
Summaries are now graded WARNING / CHANGE / INFO and the dialog shows one
level and above, opening at the highest level in the batch.
"""
import json
import pytest
from lintunes.library_manager import LibraryManager
from lintunes.models import Library, Playlist, PlaylistType, Track
from lintunes.smart import SmartCriteria, SmartGroup, SmartRule
from lintunes.storage import json_storage, conflict_resolver
from lintunes.storage.conflict_resolver import (
WARNING, CHANGE, INFO, ConflictSummary, at_least, max_level)
def _rock_criteria(**kwargs) -> SmartCriteria:
return SmartCriteria(
root=SmartGroup("all", [SmartRule("genre", "is", "Rock")]), **kwargs)
def _library(*genres) -> Library:
lib = Library()
for i, genre in enumerate(genres, start=1):
lib.tracks[i] = Track(track_id=i, name=f"t{i}", genre=genre)
return lib
def _conflict_name(original: str) -> str:
stem, _, ext = original.rpartition(".")
return f"{stem}.sync-conflict-20240101-120000-ABCDEFG.{ext}"
def _playlist_file(tmp_path, pid="AAAA1111"):
return tmp_path / "playlists" / f"{pid}.json"
# --------------------------------------------------------------------------- #
# Derived membership is never persisted
# --------------------------------------------------------------------------- #
class TestDerivedMembership:
def _saved(self, tmp_path, criteria, track_ids=(1, 2)):
pl = Playlist(name="unplayed", persistent_id="AAAA1111",
playlist_type=PlaylistType.SMART,
track_ids=list(track_ids), smart_criteria=criteria)
lib = Library()
lib.playlists[pl.persistent_id] = pl
json_storage.save_library(lib, tmp_path)
return json.loads(_playlist_file(tmp_path).read_text())
def test_live_smart_playlist_stores_no_track_ids(self, tmp_path):
data = self._saved(tmp_path, _rock_criteria())
assert data["track_ids"] == []
assert data["derived_membership"] is True
def test_non_live_smart_playlist_keeps_its_snapshot(self, tmp_path):
data = self._saved(tmp_path, _rock_criteria(live_update=False))
assert data["track_ids"] == [1, 2]
assert "derived_membership" not in data
def test_unsupported_criteria_keep_the_imported_snapshot(self, tmp_path):
data = self._saved(tmp_path, _rock_criteria(unsupported=True))
assert data["track_ids"] == [1, 2]
def test_regular_playlist_is_untouched(self, tmp_path):
pl = Playlist(name="mix", persistent_id="BBBB2222",
playlist_type=PlaylistType.REGULAR, track_ids=[3, 1])
lib = Library()
lib.playlists[pl.persistent_id] = pl
json_storage.save_library(lib, tmp_path)
data = json.loads(_playlist_file(tmp_path, "BBBB2222").read_text())
assert data["track_ids"] == [3, 1]
def test_membership_survives_a_save_load_roundtrip(self, qapp, tmp_path):
library = _library("Rock", "Jazz", "Rock")
json_storage.save_library(library, tmp_path) # library.json for the reload
mgr = LibraryManager(library, tmp_path)
pl = mgr.create_smart_playlist("rockers", _rock_criteria())
assert pl.track_ids == [1, 3]
mgr.flush()
# Nothing on disk, but a fresh start rebuilds it from the rules.
assert json.loads(
_playlist_file(tmp_path, pl.persistent_id).read_text())["track_ids"] == []
restarted = LibraryManager(json_storage.load_library(tmp_path), tmp_path)
assert restarted.library.playlists[pl.persistent_id].track_ids == []
restarted.recompute_all_smart()
assert restarted.library.playlists[pl.persistent_id].track_ids == [1, 3]
class TestRecomputeIsNotAnEdit:
"""A recompute must leave no trace: no dirty file, no timestamp. Bumping
date_modified there made Round 39's "who edited last" tiebreaker meaningless
for every smart playlist."""
def _manager(self, tmp_path, criteria):
mgr = LibraryManager(_library("Rock", "Jazz"), tmp_path)
pl = mgr.create_smart_playlist("rockers", criteria)
mgr.flush()
mgr._dirty_playlists.clear()
return mgr, pl
def test_derived_recompute_leaves_no_trace(self, qapp, tmp_path):
mgr, pl = self._manager(tmp_path, _rock_criteria())
stamp = pl.date_modified
mgr.library.tracks[2].genre = "Rock" # track 2 now matches
assert mgr.recompute_smart_playlist(pl.persistent_id) is True
assert pl.track_ids == [1, 2] # in memory, recomputed
assert pl.date_modified == stamp # not an edit
assert pl.persistent_id not in mgr._dirty_playlists
def test_derived_recompute_still_announces_itself(self, qapp, tmp_path):
"""No file write, but the views must still redraw."""
mgr, pl = self._manager(tmp_path, _rock_criteria())
seen = []
mgr.playlist_content_changed.connect(seen.append)
mgr.library.tracks[2].genre = "Rock"
mgr.recompute_smart_playlist(pl.persistent_id)
assert seen == [pl.persistent_id]
def test_a_snapshot_playlist_still_persists_its_recompute(self, qapp, tmp_path):
mgr, pl = self._manager(tmp_path, _rock_criteria(live_update=False))
stamp = pl.date_modified
mgr.library.tracks[2].genre = "Rock"
assert mgr.recompute_smart_playlist(pl.persistent_id, force=True) is True
assert pl.date_modified != stamp
assert pl.persistent_id in mgr._dirty_playlists
def test_a_real_edit_still_counts_as_one(self, qapp, tmp_path):
"""The touch=False path must not leak into user edits."""
mgr = LibraryManager(_library("Rock", "Jazz"), tmp_path)
pl = mgr.create_playlist("mix")
mgr.flush()
mgr._dirty_playlists.clear()
stamp = pl.date_modified
mgr.add_tracks_to_playlist(pl.persistent_id, [1])
assert pl.date_modified != stamp
assert pl.persistent_id in mgr._dirty_playlists
# --------------------------------------------------------------------------- #
# Severity
# --------------------------------------------------------------------------- #
class TestLevels:
def test_ordering(self):
assert at_least(WARNING, INFO) and at_least(INFO, INFO)
assert not at_least(INFO, CHANGE)
assert max_level([ConflictSummary("a", "other", level=INFO),
ConflictSummary("b", "other", level=WARNING)]) == WARNING
assert max_level([]) == INFO
def _write_conflict(self, tmp_path, name, payload):
path = tmp_path / name
path.parent.mkdir(parents=True, exist_ok=True)
cfile = path.parent / _conflict_name(path.name)
cfile.write_text(json.dumps(payload))
return cfile
def _smart_pair(self, tmp_path, their_criteria, their_stamp):
pl = Playlist(name="unplayed", persistent_id="AAAA1111",
playlist_type=PlaylistType.SMART,
smart_criteria=_rock_criteria(),
date_modified="2026-08-01T00:00:00")
lib = Library()
lib.playlists[pl.persistent_id] = pl
json_storage.save_library(lib, tmp_path)
theirs = json.loads(_playlist_file(tmp_path).read_text())
theirs["smart_criteria"] = their_criteria.to_dict()
theirs["date_modified"] = their_stamp
theirs["track_ids"] = [7, 8, 9] # their derived membership drifted
self._write_conflict(tmp_path, "playlists/AAAA1111.json", theirs)
return conflict_resolver.resolve_conflicts(tmp_path)
def test_identical_smart_rules_are_info(self, tmp_path):
summaries = self._smart_pair(tmp_path, _rock_criteria(),
"2026-08-09T00:00:00")
assert summaries[0].level == INFO
assert "identical" in summaries[0].lines[0]
def test_genuinely_edited_smart_rules_are_a_change(self, tmp_path):
other = SmartCriteria(
root=SmartGroup("all", [SmartRule("genre", "is", "Jazz")]))
summaries = self._smart_pair(tmp_path, other, "2026-08-09T00:00:00")
assert summaries[0].level == CHANGE
assert "the other machine" in summaries[0].lines[0]
def test_metadata_is_always_info_and_says_what_differed(self, tmp_path):
lib = Library()
lib.music_folder = "/media/muzak"
json_storage.save_library(lib, tmp_path)
theirs = json.loads((tmp_path / "library_metadata.json").read_text())
theirs["library_settings"]["column_widths"] = {"name": 400}
self._write_conflict(tmp_path, "library_metadata.json", theirs)
summaries = conflict_resolver.resolve_conflicts(tmp_path)
assert summaries[0].level == INFO
assert "column layout" in summaries[0].lines[0]
def test_but_adopting_a_music_folder_is_a_change(self, tmp_path):
"""The one thing in this file nobody sets by resizing a window. Round 40
made the music folder win on its own stamp rather than the file's mtime;
when that fires, the merge changed a deliberate setting."""
lib = Library()
lib.music_folder = "/media/muzak"
lib.music_folder_set_at = "2026-08-01T00:00:00"
json_storage.save_library(lib, tmp_path)
theirs = json.loads((tmp_path / "library_metadata.json").read_text())
theirs["music_folder"] = "/media/elsewhere"
theirs["music_folder_rel"] = "../elsewhere"
theirs["music_folder_set_at"] = "2026-08-09T00:00:00"
self._write_conflict(tmp_path, "library_metadata.json", theirs)
summaries = conflict_resolver.resolve_conflicts(tmp_path)
assert summaries[0].level == CHANGE
assert "Music folder" in summaries[0].lines[-1]
assert json.loads((tmp_path / "library_metadata.json").read_text())[
"music_folder"] == "/media/elsewhere"
def test_a_cosmetic_only_playlist_merge_is_info(self, tmp_path):
pl = Playlist(name="mix", persistent_id="BBBB2222",
playlist_type=PlaylistType.REGULAR, track_ids=[1, 2, 3],
date_modified="2026-08-01T00:00:00")
lib = Library()
lib.playlists[pl.persistent_id] = pl
json_storage.save_library(lib, tmp_path)
theirs = json.loads(_playlist_file(tmp_path, "BBBB2222").read_text())
theirs["settings"]["column_widths"] = {"name": 500}
theirs["date_modified"] = "2026-08-09T00:00:00"
self._write_conflict(tmp_path, "playlists/BBBB2222.json", theirs)
summaries = conflict_resolver.resolve_conflicts(tmp_path)
assert summaries[0].level == INFO
assert "column layout" in summaries[0].lines[0]
def test_a_real_order_merge_is_a_change(self, tmp_path):
pl = Playlist(name="mix", persistent_id="BBBB2222",
playlist_type=PlaylistType.REGULAR, track_ids=[1, 2, 3],
date_modified="2026-08-01T00:00:00")
lib = Library()
lib.playlists[pl.persistent_id] = pl
json_storage.save_library(lib, tmp_path)
theirs = json.loads(_playlist_file(tmp_path, "BBBB2222").read_text())
theirs["track_ids"] = [1, 99, 2, 3]
theirs["date_modified"] = "2026-08-09T00:00:00"
self._write_conflict(tmp_path, "playlists/BBBB2222.json", theirs)
summaries = conflict_resolver.resolve_conflicts(tmp_path)
assert summaries[0].level == CHANGE
assert json.loads(
_playlist_file(tmp_path, "BBBB2222").read_text())["track_ids"] == [1, 99, 2, 3]
def test_a_deleted_file_edited_elsewhere_is_a_warning(self, tmp_path):
json_storage.save_library(Library(), tmp_path)
self._write_conflict(tmp_path, "playlists/CCCC3333.json",
{"name": "gone", "persistent_id": "CCCC3333"})
summaries = conflict_resolver.resolve_conflicts(tmp_path)
assert summaries[0].level == WARNING
def test_an_unmergeable_file_is_a_warning(self, tmp_path):
json_storage.save_library(Library(), tmp_path)
(tmp_path / "something.json").write_text("{}")
self._write_conflict(tmp_path, "something.json", {"a": 1})
summaries = conflict_resolver.resolve_conflicts(tmp_path)
assert summaries[0].level == WARNING
def test_a_play_journal_conflict_is_a_warning(self, tmp_path):
json_storage.save_library(Library(), tmp_path)
(tmp_path / "plays").mkdir(exist_ok=True)
(tmp_path / "plays" / "boxA.json").write_text(
json.dumps({"1": {"plays": 2}}))
self._write_conflict(tmp_path, "plays/boxA.json", {"1": {"plays": 5}})
summaries = conflict_resolver.resolve_conflicts(tmp_path)
assert summaries[0].level == WARNING
assert json.loads(
(tmp_path / "plays" / "boxA.json").read_text())["1"]["plays"] == 5
# --------------------------------------------------------------------------- #
# The dialog
# --------------------------------------------------------------------------- #
@pytest.fixture
def dialog_cls():
from lintunes.gui.conflict_dialog import ConflictSummaryDialog
return ConflictSummaryDialog
def _summary(name, level, line="something happened"):
return ConflictSummary(name, "playlist", [line], backup_dir="/tmp/b",
level=level)
class TestConflictDialog:
def test_an_all_info_batch_opens_on_the_info_view(self, qapp, dialog_cls):
dlg = dialog_cls([_summary("library_metadata.json", INFO)], None)
assert dlg.level == INFO
assert "library_metadata.json" in dlg._body.toPlainText()
assert "Nothing at this level" not in dlg._body.toPlainText()
def test_a_warning_batch_hides_the_routine_entries(self, qapp, dialog_cls):
dlg = dialog_cls([_summary("library_metadata.json", INFO),
_summary("plays/boxA.json", WARNING)], None)
body = dlg._body.toPlainText()
assert dlg.level == WARNING
assert "plays/boxA.json" in body
assert "library_metadata.json" not in body
assert "1 routine entry hidden" in body
def test_a_later_warning_pulls_the_view_back_up(self, qapp, dialog_cls):
dlg = dialog_cls([_summary("library_metadata.json", INFO)], None)
assert dlg.add_event([_summary("plays/boxA.json", WARNING)]) is True
assert dlg.level == WARNING
assert "library_metadata.json" not in dlg._body.toPlainText()
def test_routine_events_do_not_ask_to_be_raised(self, qapp, dialog_cls):
dlg = dialog_cls([_summary("mix", CHANGE)], None)
assert dlg.level == CHANGE
assert dlg.add_event([_summary("library_metadata.json", INFO)]) is False
# Still recorded — it's there once the user dials the filter down.
dlg._on_level_picked(dlg._level_box.findData(INFO))
assert "library_metadata.json" in dlg._body.toPlainText()
def test_a_user_choice_survives_a_quieter_batch(self, qapp, dialog_cls):
dlg = dialog_cls([_summary("mix", CHANGE)], None)
dlg._on_level_picked(dlg._level_box.findData(WARNING))
dlg.add_event([_summary("other mix", CHANGE)])
assert dlg.level == WARNING
assert "Nothing at this level" in dlg._body.toPlainText()
def test_filtered_out_events_leave_no_orphan_headers(self, qapp, dialog_cls):
dlg = dialog_cls([_summary("mix", CHANGE)], None)
dlg.add_event([_summary("library_metadata.json", INFO)])
headers = [ln for ln in dlg._body.toPlainText().splitlines()
if ln.startswith("── ")]
assert len(headers) == 1 # only the event that still has content