travandClaude Opus 5
521db2f81d
"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
368 lines
16 KiB
Python
368 lines
16 KiB
Python
"""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
|