Renaming artist/album moves the file: music folder stays organized

Editing artist/album_artist/album now relocates the file to its new
<music_folder>/Music/Artist/Album/ home (folders created as needed,
"Unknown Artist" fallback via the same organized_destination the
importer uses), deliberately relaxing the old never-move rule:

- LibraryManager.organize_root() is the shared definition of the
  managed tree; MainWindow._music_import_dir delegates to it.
- Only files already inside the organize root are managed; external
  files get their tags edited but are never pulled into the tree.
- The move rides the existing undo Command maps as a "location" field:
  Ctrl+Z moves the file back, redo re-moves, both best-effort (a failed
  physical move drops "location" so memory tracks the real file).
- A failed move applies the edit, keeps the old path, and reports via
  the new file_move_failed signal (status bar), mirroring
  tag_write_failed. Collisions get the importer's " 1" suffix.
- Directories left empty are pruned up to (never including) the root.
- conflict_resolver._merge_track_fields now carries "location" with
  the newest-date_modified rule, so a rename-move on one machine
  propagates through both the sync-conflict merge and the live
  reload_from_disk reconcile on the other.
- The on-disk basename is kept verbatim (no re-sanitizing), so files
  named on macOS aren't gratuitously renamed by a tag edit.

Same-filesystem rename keeps the player's open fd valid, so moving the
currently-playing track is safe.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

lintunes/gui/main_window.py

@@ -110,6 +110,9 @@ class MainWindow(QMainWindow):
         manager.tag_write_failed.connect(
             lambda name, err: self.statusBar().showMessage(
                 f"Couldn't write tags to “{name}”: {err}", 8000))
+        manager.file_move_failed.connect(
+            lambda name, err: self.statusBar().showMessage(
+                f"Couldn't move the file for “{name}”: {err}", 8000))
         manager.library_reloaded.connect(self._on_library_reloaded)
         manager.conflict_resolved.connect(self._on_conflict_resolved)
         prefs.changed.connect(self._on_prefs_changed)
@@ -305,7 +308,9 @@ class MainWindow(QMainWindow):
     # ---- file import ----
 
     def _music_import_dir(self) -> Path:
-        return Path(self._manager.library.music_folder) / "Music"
+        # Fall back to ./Music when no music folder is set (fresh library),
+        # matching the pre-organize_root behavior.
+        return self._manager.organize_root() or Path("Music")
 
     def _open_files_dialog(self):
         paths, _filter = QFileDialog.getOpenFileNames(

lintunes/library_manager.py

@@ -8,6 +8,7 @@ from PyQt6.QtCore import QObject, QTimer, pyqtSignal
 log = logging.getLogger(__name__)
 
 from lintunes import smart, tagging
+from lintunes.importers.file_importer import organized_destination, unique_path
 from lintunes.models import Library, Playlist, PlaylistType
 from lintunes.storage import json_storage
 from lintunes.undo import Command, UndoStack
@@ -36,6 +37,7 @@ class LibraryManager(QObject):
     library_reloaded = pyqtSignal()            # library re-read from disk (sync)
     conflict_resolved = pyqtSignal(list)        # list[ConflictSummary] just merged
     tag_write_failed = pyqtSignal(str, str)     # track name, error text
+    file_move_failed = pyqtSignal(str, str)     # track name, error text
 
     def __init__(self, library: Library, data_dir: Path, parent=None):
         super().__init__(parent)
@@ -374,6 +376,10 @@ class LibraryManager(QObject):
             # File write failed: leave the library untouched so memory and file
             # don't diverge, and don't record an un-undoable phantom edit.
             return
+        new_loc = self._maybe_move_file(track, changed)
+        if new_loc is not None:
+            old = {**old, "location": track.location}
+            changed = {**changed, "location": new_loc}
         self._apply_track_fields(track_id, changed)
         self.undo_stack.push(Command(
             "Edit Info",
@@ -398,6 +404,10 @@ class LibraryManager(QObject):
             if not changed or not self._write_track_tags(track, changed):
                 continue
             old = {k: getattr(track, k) for k in changed}
+            new_loc = self._maybe_move_file(track, changed)
+            if new_loc is not None:
+                old = {**old, "location": track.location}
+                changed = {**changed, "location": new_loc}
             self._apply_track_fields(track_id, changed)
             changes.append((track_id, changed, old))
         if not changes:
@@ -441,8 +451,93 @@ class LibraryManager(QObject):
         track = self.library.tracks.get(track_id)
         if track:
             self._write_track_tags(track, field_map)  # best-effort
+            field_map = self._revert_file_move(track, field_map)
         self._apply_track_fields(track_id, field_map)
 
+    def _revert_file_move(self, track, field_map: dict) -> dict:
+        """Undo/redo of an edit that moved the file: move it to the map's
+        location, best-effort. If the physical move fails, drop ``location``
+        from the map so memory keeps pointing at where the file really is."""
+        target = field_map.get("location")
+        if target is None or target == track.location:
+            return field_map
+        src = Path(track.location)
+        try:
+            if not src.is_file():
+                raise OSError(f"missing source file {src}")
+            actual = self._move_file(src, Path(target))
+            return {**field_map, "location": str(actual)}
+        except OSError as e:
+            log.warning("Could not move %s back to %s: %s", src, target, e)
+            self.file_move_failed.emit(track.name or str(src), str(e))
+            return {k: v for k, v in field_map.items() if k != "location"}
+
+    # ---- keeping the music folder organized ----
+
+    _ORGANIZE_FIELDS = frozenset({"artist", "album_artist", "album"})
+
+    def organize_root(self) -> Path | None:
+        """Root of the organized music tree (<music_folder>/Music) — the only
+        place imports copy into and rename-moves manage."""
+        if not self.library.music_folder:
+            return None
+        return Path(self.library.music_folder) / "Music"
+
+    def _maybe_move_file(self, track, changed: dict) -> str | None:
+        """If an artist/album_artist/album edit changes where the file belongs
+        in the organized Artist/Album tree, move it there (creating folders as
+        needed) and return the new absolute path, else None.
+
+        Only files already inside the organize root are managed — an external
+        file the user never imported must not get pulled into the synced tree
+        by a tag edit. A failed move is reported via ``file_move_failed`` and
+        the edit proceeds with the old location (the tags are already written,
+        so the library stays consistent, just unorganized)."""
+        if not (self._ORGANIZE_FIELDS & changed.keys()) or not track.location:
+            return None
+        root = self.organize_root()
+        src = Path(track.location)
+        if root is None or root not in src.parents or not src.is_file():
+            return None
+        merged = {key: changed.get(key, getattr(track, key))
+                  for key in self._ORGANIZE_FIELDS}
+        # Keep the exact on-disk basename: it's already valid here, and
+        # re-sanitizing could gratuitously rename files that came from macOS.
+        dest = organized_destination(root, merged, src.name).with_name(src.name)
+        if dest == src:
+            return None
+        try:
+            return str(self._move_file(src, dest))
+        except OSError as e:
+            log.warning("Could not move %s -> %s: %s", src, dest, e)
+            self.file_move_failed.emit(track.name or str(src), str(e))
+            return None
+
+    def _move_file(self, src: Path, dest: Path) -> Path:
+        """Physically relocate a music file and prune the directories it left
+        empty. Same-filesystem rename, so this is safe even while the player
+        has the file open (the fd stays valid; the path is re-read at next
+        play). Returns the actual destination (collision-suffixed if taken)."""
+        dest = unique_path(dest)
+        dest.parent.mkdir(parents=True, exist_ok=True)
+        src.rename(dest)
+        root = self.organize_root()
+        if root is not None:
+            self._prune_empty_dirs(src.parent, root)
+        return dest
+
+    @staticmethod
+    def _prune_empty_dirs(start: Path, root: Path):
+        """Remove now-empty directories from ``start`` upward, strictly below
+        ``root`` (the organize root itself is never touched)."""
+        current = start
+        while root in current.parents:
+            try:
+                current.rmdir()  # raises OSError if not empty
+            except OSError:
+                return
+            current = current.parent
+
     def record_play(self, track_id: int):
         track = self.library.tracks.get(track_id)
         if not track:

lintunes/storage/conflict_resolver.py

@@ -178,10 +178,15 @@ def _merge_track_fields(orig: dict, conflict: dict) -> list[str]:
         if conflict.get("rating") != orig.get("rating") and "rating" in conflict:
             notes.append(f"rating {_stars(orig.get('rating'))} → "
                          f"{_stars(conflict.get('rating'))} (newer)")
+        # location rides along: renaming artist/album moves the file (and bumps
+        # date_modified), so the machine with the newest edit also knows where
+        # the file now lives. Both sides are relative here (raw JSON) and both
+        # absolute in the reload_from_disk reconcile — always like-with-like.
         for field_name in ("rating", "name", "artist", "album_artist", "album",
                            "genre", "composer", "grouping", "comments",
                            "sort_name", "sort_artist", "sort_album_artist",
-                           "sort_album", "sort_composer", "date_modified"):
+                           "sort_album", "sort_composer", "location",
+                           "date_modified"):
             if field_name in conflict:
                 orig[field_name] = conflict[field_name]
     return notes

tests/test_round18.py

@@ -6,10 +6,17 @@
    folder organized (Artist/Album, Unknown Artist fallback).
 """
 
+import shutil
+from pathlib import Path
+
+import pytest
+
 from PyQt6.QtCore import QObject, pyqtSignal
 from PyQt6.QtGui import QColor
 
 from lintunes import theme
+from lintunes.library_manager import LibraryManager
+from lintunes.models import Library, Track
 from lintunes.preferences import Preferences
 from lintunes.gui.visualizer import (
     VisualizerWidget, MODE_ON, MODE_DIM, MODE_OFF)
@@ -106,8 +113,6 @@ class TestVisualizerGrayColor:
 # --------------------------------------------------------------------------
 
 def _seeded_manager(tmp_path, track_ids=(1, 2, 3)):
-    from lintunes.models import Library, Track
-    from lintunes.library_manager import LibraryManager
     tracks = {tid: Track(track_id=tid, name=f"T{tid}") for tid in track_ids}
     return LibraryManager(Library(tracks=tracks), tmp_path / "data")
 
@@ -171,3 +176,191 @@ class TestFileDropPosition:
 
         assert received == [(["/x.mp3"], playlist.persistent_id, 2),
                             (["/y.mp3"], playlist.persistent_id, None)]
+
+
+# --------------------------------------------------------------------------
+# 3. Renaming artist/album moves the file (keep the music folder organized)
+# --------------------------------------------------------------------------
+
+@pytest.fixture
+def organized(qapp, tmp_path, mp3_file):
+    """A library whose music folder holds one organized track:
+    <share>/Music/Old Artist/Old Album/test.mp3."""
+    share = tmp_path / "share"
+    src = share / "Music" / "Old Artist" / "Old Album" / "test.mp3"
+    src.parent.mkdir(parents=True)
+    shutil.copy2(mp3_file, src)
+    track = Track(track_id=1, name="Song", artist="Old Artist",
+                  album="Old Album", location=str(src))
+    library = Library(tracks={1: track}, music_folder=str(share))
+    manager = LibraryManager(library, share / "lintunes")
+    return manager, track, share
+
+
+class TestRenameMovesFile:
+    def test_artist_edit_moves_file(self, organized):
+        manager, track, share = organized
+        old_path = Path(track.location)
+
+        manager.edit_track_fields(1, {"artist": "New Artist"})
+
+        expected = share / "Music" / "New Artist" / "Old Album" / "test.mp3"
+        assert Path(track.location) == expected
+        assert expected.is_file()
+        assert not old_path.exists()
+        assert track.artist == "New Artist"
+
+    def test_album_edit_moves_file(self, organized):
+        manager, track, share = organized
+
+        manager.edit_track_fields(1, {"album": "New Album"})
+
+        expected = share / "Music" / "Old Artist" / "New Album" / "test.mp3"
+        assert Path(track.location) == expected
+        assert expected.is_file()
+
+    def test_album_artist_shields_artist_edit(self, organized):
+        manager, track, share = organized
+        track.album_artist = "Main Act"
+        dest = share / "Music" / "Main Act" / "Old Album" / "test.mp3"
+        dest.parent.mkdir(parents=True)
+        Path(track.location).rename(dest)
+        track.location = str(dest)
+
+        # Folder is keyed on album_artist, so an artist edit changes nothing.
+        manager.edit_track_fields(1, {"artist": "Feat Guy"})
+
+        assert Path(track.location) == dest
+        assert dest.is_file()
+        assert track.artist == "Feat Guy"
+
+    def test_empty_dirs_pruned_but_not_root(self, organized):
+        manager, track, share = organized
+
+        manager.edit_track_fields(1, {"artist": "New Artist",
+                                      "album": "New Album"})
+
+        assert not (share / "Music" / "Old Artist").exists()  # pruned
+        assert (share / "Music").is_dir()                     # root survives
+
+    def test_outside_root_file_untouched(self, organized, mp3_file):
+        manager, _track, share = organized
+        outside = Path(mp3_file)
+        external = Track(track_id=2, name="Ext", artist="Someone",
+                         location=str(outside))
+        manager.library.tracks[2] = external
+
+        manager.edit_track_fields(2, {"artist": "Someone Else"})
+
+        assert Path(external.location) == outside
+        assert outside.is_file()
+        assert external.artist == "Someone Else"
+
+    def test_undo_moves_back_redo_removes(self, organized):
+        manager, track, share = organized
+        old_path = Path(track.location)
+
+        manager.edit_track_fields(1, {"artist": "New Artist"})
+        new_path = Path(track.location)
+
+        manager.undo_stack.undo()
+        assert Path(track.location) == old_path
+        assert old_path.is_file() and not new_path.exists()
+        assert track.artist == "Old Artist"
+
+        manager.undo_stack.redo()
+        assert Path(track.location) == new_path
+        assert new_path.is_file() and not old_path.exists()
+        assert track.artist == "New Artist"
+
+    def test_collision_gets_suffix(self, organized):
+        manager, track, share = organized
+        taken = share / "Music" / "New Artist" / "Old Album" / "test.mp3"
+        taken.parent.mkdir(parents=True)
+        taken.write_bytes(b"other file already here")
+
+        manager.edit_track_fields(1, {"artist": "New Artist"})
+
+        assert Path(track.location) == taken.with_stem("test 1")
+        assert Path(track.location).is_file()
+        assert taken.read_bytes() == b"other file already here"
+
+    def test_move_failure_keeps_edit_and_old_location(self, organized,
+                                                      monkeypatch):
+        manager, track, share = organized
+        old_path = Path(track.location)
+        failures = []
+        manager.file_move_failed.connect(
+            lambda name, err: failures.append((name, err)))
+        monkeypatch.setattr(
+            LibraryManager, "_move_file",
+            lambda self, src, dest: (_ for _ in ()).throw(OSError("nope")))
+
+        manager.edit_track_fields(1, {"artist": "New Artist"})
+
+        assert track.artist == "New Artist"        # edit applied
+        assert Path(track.location) == old_path    # location kept
+        assert old_path.is_file()
+        assert failures and failures[0][0] == "Song"
+
+    def test_bulk_album_rename_moves_all_and_one_undo(self, organized,
+                                                      mp3_file):
+        manager, track, share = organized
+        second_src = share / "Music" / "Old Artist" / "Old Album" / "two.mp3"
+        shutil.copy2(mp3_file, second_src)
+        second = Track(track_id=2, name="Two", artist="Old Artist",
+                       album="Old Album", location=str(second_src))
+        manager.library.tracks[2] = second
+
+        manager.edit_tracks_fields([1, 2], {"album": "Remaster"})
+
+        new_dir = share / "Music" / "Old Artist" / "Remaster"
+        assert Path(track.location).parent == new_dir
+        assert Path(second.location).parent == new_dir
+        assert Path(track.location).is_file()
+        assert Path(second.location).is_file()
+
+        manager.undo_stack.undo()  # ONE undo restores both
+        old_dir = share / "Music" / "Old Artist" / "Old Album"
+        assert Path(track.location).parent == old_dir
+        assert Path(second.location).parent == old_dir
+        assert Path(track.location).is_file()
+        assert Path(second.location).is_file()
+        assert not new_dir.exists()  # pruned again
+
+
+class TestLocationSync:
+    def test_merge_track_fields_newest_wins_location(self):
+        from lintunes.storage.conflict_resolver import _merge_track_fields
+        orig = {"location": "Music/A/B/x.mp3", "date_modified": "2026-01-01"}
+        newer = {"location": "Music/C/B/x.mp3", "date_modified": "2026-06-01"}
+        _merge_track_fields(orig, newer)
+        assert orig["location"] == "Music/C/B/x.mp3"
+
+        older = {"location": "Music/Z/B/x.mp3", "date_modified": "2025-01-01"}
+        _merge_track_fields(orig, older)
+        assert orig["location"] == "Music/C/B/x.mp3"  # older copy loses
+
+    def test_reload_from_disk_adopts_external_move(self, organized):
+        from lintunes.storage import json_storage
+        manager, track, share = organized
+        data_dir = manager.data_dir
+        json_storage.save_library(manager.library, data_dir)
+
+        # Another machine renamed the artist: file moved + library.json
+        # rewritten with a newer date_modified and the new relative path.
+        new_loc = share / "Music" / "Elsewhere" / "Old Album" / "test.mp3"
+        new_loc.parent.mkdir(parents=True)
+        Path(track.location).rename(new_loc)
+        from lintunes.storage.json_storage import read_json, write_json
+        disk = read_json(data_dir / "library.json")
+        entry = disk["1"]
+        entry["artist"] = "Elsewhere"
+        entry["location"] = "../Music/Elsewhere/Old Album/test.mp3"
+        entry["date_modified"] = "2099-01-01T00:00:00"
+        write_json(data_dir / "library.json", disk)
+
+        manager.reload_from_disk()
+
+        assert Path(track.location) == new_loc
+        assert track.artist == "Elsewhere"