From a657d13f463578d43b324325a1f2e6938a059124 Mon Sep 17 00:00:00 2001 From: garado Date: Sat, 1 Aug 2026 03:57:46 -0500 Subject: [PATCH 01/20] fix: redesign the really awkward cmd --- README.md | 14 ++-- light_api/light_api/music.py | 108 ++++++++++++++++++++++++----- light_cli_tui/light_cli_tui/cli.py | 58 ++++++++-------- 3 files changed, 128 insertions(+), 52 deletions(-) diff --git a/README.md b/README.md index 5fe0320..a52acee 100644 --- a/README.md +++ b/README.md @@ -78,15 +78,19 @@ After the first login, your auth token will be cached. Tokens are good for 30 da ```sh # Upload tracks -# Overwrite existing matching tracks (match on file title metadata) -light music upload song1 song2 song3 --match-title-by metadata +# Files matching an existing track (by title+artist) are skipped by default +light music upload song1 song2 song3 # Upload tracks -# Overwrite existing matching tracks (match on filename) -light music upload song1 song2 song3 --match-title-by filename +# Delete-and-replace matching existing tracks instead of skipping them +light music upload song1 song2 song3 --replace # Upload tracks -# Don't overwrite existing tracks +# Match by filename instead of metadata tags (for files with missing/broken tags) +light music upload song1 song2 song3 --match-by filename + +# Upload tracks +# Skip duplicate checking entirely, always upload light music upload --allow-duplicates song1 song2 song3 ``` diff --git a/light_api/light_api/music.py b/light_api/light_api/music.py index 22e2fa1..93c6c8d 100644 --- a/light_api/light_api/music.py +++ b/light_api/light_api/music.py @@ -266,11 +266,70 @@ def delete_tracks_by_artist_regex(self, pattern: str) -> None: """ self.delete_tracks_predicate(lambda t: bool(re.match(pattern, t.artist))) + def _track_identity( + self, + file_path: str, + match_by: Literal["metadata", "filename"], + replace: bool = False, + ) -> tuple[str, str | None]: + """Return (title, artist) used to match `file_path` against existing tracks. + + 'filename' mode applies for tracks that were uploaded with no metadata; LightOS + displays those with title=filename, artist='Unknown'. + + If match_by='metadata' but `file_path` has no readable title/artist tags, this + auto-falls back to filename matching for that file - unless replace=True, since + a false-positive filename-only match would delete the wrong track under + --replace. In that case, a missing tag raises instead, requiring the caller to + explicitly pass match_by='filename' to acknowledge the weaker match. + """ + if match_by == "metadata": + tags = File(file_path, easy=True) + title = tags.get("title", [None])[0] if tags else None + artist = tags.get("artist", [None])[0] if tags else None + if title and artist: + return title, artist + + if replace: + raise ValueError( + f"Could not read title/artist metadata from {file_path!r}, and " + "--replace is destructive - pass match_by='filename' to explicitly " + "acknowledge the weaker match instead of silently falling back." + ) + + return os.path.splitext(os.path.basename(file_path))[0], None + + def _find_matching_track(self, title: str, artist: str | None) -> "LightTrack | None": + """Find an existing track matching (title, artist). artist=None matches title only.""" + for t in self._tracks: + if t.title == title and (artist is None or t.artist == artist): + return t + return None + + def find_upload_matches( + self, + files: list[str], + match_by: Literal["metadata", "filename"] = "metadata", + replace: bool = False, + ) -> dict[str, "LightTrack"]: + """Return {file_path: existing LightTrack} for files that match a track already + on the device. Shared by upload_tracks and CLI confirmation prompts, so there's + exactly one definition of "what counts as a duplicate".""" + self._init_tracks() + matches: dict[str, "LightTrack"] = {} + for file_path in files: + title, artist = self._track_identity(file_path, match_by, replace) + match = self._find_matching_track(title, artist) + if match is not None: + matches[file_path] = match + return matches + def upload_tracks( self, files: list[str], allow_duplicates: bool = False, - match_title_by: Literal["metadata", "filename"] = "metadata", + replace: bool = False, + match_by: Literal["metadata", "filename"] = "metadata", convert_flac: bool = True, on_progress: "Callable[[str, int, int], None] | None" = None, ) -> None: @@ -278,28 +337,41 @@ def upload_tracks( Args: files: List of paths to audio files to upload. - allow_duplicates: If False (default), existing tracks with matching titles are - deleted before uploading. If True, duplicates are kept. - match_title_by: How to determine a track's title for duplicate detection. - "metadata" (default) reads the title from the file's ID3/audio tags. - "filename" uses the filename (without extension) as the title. + allow_duplicates: If True, skip duplicate checking entirely and always upload, + potentially creating multiple tracks with the same title/artist. + replace: If True, delete a file's matching existing track (if any) before + uploading it. If False (default), files matching an existing track + are skipped instead, leaving the existing track untouched. + match_by: How to identify a file for duplicate matching. + "metadata" (default) reads title+artist from the file's ID3/audio tags. + Per file, if tags are missing this auto-falls back to filename-only + matching - unless replace=True, in which case it raises instead + (a false-positive filename-only match would delete the wrong track). + "filename" matches on filename-as-title only for every file, for + tracks that were themselves uploaded with no metadata - LightOS + displays those with title=filename and artist="Unknown". """ + if replace and allow_duplicates: + raise ValueError("replace and allow_duplicates are mutually exclusive") + manual_update_cmds = [] + to_upload = files if not allow_duplicates: - titles: list[str] - - if match_title_by == "metadata": - titles = [] - for s in files: - f = File(s, easy=True) - if f is None: - raise ValueError(f"Could not read metadata from {s}") - titles.append(f.get("title", ["Unknown Title"])[0]) - else: - titles = [os.path.splitext(os.path.basename(s))[0] for s in files] + matches = self.find_upload_matches(files, match_by, replace) + to_upload = [] + for file_path in files: + match = matches.get(file_path) + + if match is None: + to_upload.append(file_path) + elif replace: + self.delete_tracks_predicate(lambda t: t is match) + to_upload.append(file_path) + else: + log.info(f"Skipping {file_path!r}: matches existing track {match.title!r}") - self.delete_tracks_by_title(titles) + files = to_upload for file_path in files: log.info(f"Uploading {file_path}") diff --git a/light_cli_tui/light_cli_tui/cli.py b/light_cli_tui/light_cli_tui/cli.py index aae5d84..bd8f0ac 100644 --- a/light_cli_tui/light_cli_tui/cli.py +++ b/light_cli_tui/light_cli_tui/cli.py @@ -235,15 +235,24 @@ def podcasts_delete(light: Light, title): @click.option( "--allow-duplicates", is_flag=True, - help="Skip duplicate checking and always upload.", + help="Skip duplicate checking entirely and always upload.", ) @click.option( - "--match-title-by", + "--replace", + is_flag=True, + help="Delete a file's matching existing track before uploading it, instead of " + "skipping the file (the default).", +) +@click.option( + "--match-by", "-m", type=click.Choice(["filename", "metadata"]), default="metadata", show_default=True, - help="How to match existing tracks when checking for duplicates.", + help="How to identify a file for duplicate matching. 'metadata' matches on " + "title+artist from tags; 'filename' matches on filename-as-title only, for tracks " + "that were themselves uploaded with no metadata (LightOS shows those with " + "title=filename, artist=Unknown).", ) @click.option( "--no-convert-flac", @@ -251,41 +260,31 @@ def podcasts_delete(light: Light, title): default=False, help="Skip FLAC to MP3 conversion (conversion is on by default to preserve metadata).", ) -def music_upload(light: Light, songs, allow_duplicates, match_title_by, no_convert_flac): +def music_upload(light: Light, songs, allow_duplicates, replace, match_by, no_convert_flac): """Upload one or more audio files to your device. - Duplicate detection is on by default - existing tracks with a matching - title will be replaced. Use `--allow-duplicates` to skip this. + Duplicate detection is on by default: files matching an existing track + (by title+artist) are skipped, leaving the existing track untouched. Use + `--replace` to delete-and-replace matches instead, or `--allow-duplicates` + to skip the check entirely. **Example:** `light music upload track1.mp3 track2.mp3` """ + if replace and allow_duplicates: + raise click.UsageError("--replace and --allow-duplicates are mutually exclusive.") + files = list(songs) if not allow_duplicates: - from mutagen._file import File as MutagenFile - import os as _os - - if match_title_by == "metadata": - titles = [] - for s in files: - f = MutagenFile(s, easy=True) - titles.append( - f.get("title", [_os.path.splitext(_os.path.basename(s))[0]])[0] - if f - else _os.path.splitext(_os.path.basename(s))[0] - ) - else: - titles = [_os.path.splitext(_os.path.basename(s))[0] for s in files] - - existing = light.music.get_tracks() - to_overwrite = [t for t in existing if t.title in set(titles)] - - if to_overwrite: - console.print(f"Tracks to overwrite ({len(to_overwrite)}):") - for t in to_overwrite: - console.print(f" {t.artist} — {t.title}") + matches = light.music.find_upload_matches(files, match_by, replace) + + if matches: + verb = "overwrite" if replace else "skip" + console.print(f"Tracks to {verb} ({len(matches)}):") + for file_path, t in matches.items(): + console.print(f" {file_path} -> {t.artist} — {t.title}") if not click.confirm("Proceed?"): return @@ -310,7 +309,8 @@ def on_progress(filename: str, sent: int, total: int) -> None: light.music.upload_tracks( files, allow_duplicates=allow_duplicates, - match_title_by=match_title_by, + replace=replace, + match_by=match_by, convert_flac=not no_convert_flac, on_progress=on_progress, ) From a7410b34f93326f763f109c56add13f7e3691d13 Mon Sep 17 00:00:00 2001 From: garado Date: Sat, 1 Aug 2026 04:02:52 -0500 Subject: [PATCH 02/20] chore: lint --- light_api/light_api/music.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/light_api/light_api/music.py b/light_api/light_api/music.py index 93c6c8d..35bdfb9 100644 --- a/light_api/light_api/music.py +++ b/light_api/light_api/music.py @@ -299,7 +299,7 @@ def _track_identity( return os.path.splitext(os.path.basename(file_path))[0], None - def _find_matching_track(self, title: str, artist: str | None) -> "LightTrack | None": + def _find_matching_track(self, title: str, artist: str | None) -> LightTrack | None: """Find an existing track matching (title, artist). artist=None matches title only.""" for t in self._tracks: if t.title == title and (artist is None or t.artist == artist): @@ -311,12 +311,12 @@ def find_upload_matches( files: list[str], match_by: Literal["metadata", "filename"] = "metadata", replace: bool = False, - ) -> dict[str, "LightTrack"]: + ) -> dict[str, LightTrack]: """Return {file_path: existing LightTrack} for files that match a track already on the device. Shared by upload_tracks and CLI confirmation prompts, so there's exactly one definition of "what counts as a duplicate".""" self._init_tracks() - matches: dict[str, "LightTrack"] = {} + matches: dict[str, LightTrack] = {} for file_path in files: title, artist = self._track_identity(file_path, match_by, replace) match = self._find_matching_track(title, artist) From 13554dec66fa89afe0769d25cba247a0ed3bbe85 Mon Sep 17 00:00:00 2001 From: garado Date: Sat, 1 Aug 2026 04:06:49 -0500 Subject: [PATCH 03/20] fix: fix artist=None accidentally being treated as wildcard --- light_api/light_api/music.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/light_api/light_api/music.py b/light_api/light_api/music.py index 35bdfb9..50472e2 100644 --- a/light_api/light_api/music.py +++ b/light_api/light_api/music.py @@ -299,10 +299,20 @@ def _track_identity( return os.path.splitext(os.path.basename(file_path))[0], None + _UNKNOWN_ARTIST_VALUES = {"", "Unknown"} + def _find_matching_track(self, title: str, artist: str | None) -> LightTrack | None: - """Find an existing track matching (title, artist). artist=None matches title only.""" + """Find an existing track matching (title, artist). + + artist=None matches only untagged tracks (artist is blank or "Unknown"). + """ for t in self._tracks: - if t.title == title and (artist is None or t.artist == artist): + if t.title != title: + continue + if artist is None: + if t.artist in self._UNKNOWN_ARTIST_VALUES: + return t + elif t.artist == artist: return t return None @@ -312,9 +322,7 @@ def find_upload_matches( match_by: Literal["metadata", "filename"] = "metadata", replace: bool = False, ) -> dict[str, LightTrack]: - """Return {file_path: existing LightTrack} for files that match a track already - on the device. Shared by upload_tracks and CLI confirmation prompts, so there's - exactly one definition of "what counts as a duplicate".""" + """Return {file_path: existing LightTrack} for files that match a track already on the device.""" self._init_tracks() matches: dict[str, LightTrack] = {} for file_path in files: From e87adf2392f19b9ffc13ff2a8c0f86daad85210a Mon Sep 17 00:00:00 2001 From: garado Date: Sat, 1 Aug 2026 04:12:04 -0500 Subject: [PATCH 04/20] test: add tests for new light music upload behavior --- tests/test_api.py | 95 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+) diff --git a/tests/test_api.py b/tests/test_api.py index 120d41d..5862b99 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -363,6 +363,101 @@ def test_raises_on_error_response(self, f_devices, f_tools): light.music.get_tracks() +class TestFindMatchingTrack: + def test_ignores_cross_artist_title_collision(self): + light = make_light() + light.music._tracks = [ + LightTrack( + playlist_item_id="1", audio_id="a1", + title="Playing God", artist="Paramore", album="", + ), + ] + + # artist=None (filename-mode / metadata-fallback identity) must not + # wildcard-match a track that has a real, different artist. + assert light.music._find_matching_track("Playing God", None) is None + + # Precise (title, artist) matching still works correctly. + assert light.music._find_matching_track("Playing God", "Paramore") is not None + assert light.music._find_matching_track("Playing God", "Polyphia") is None + + def test_matches_untagged_tracks_by_title_only(self): + """artist=None should still match existing tracks that are themselves + untagged (artist blank or 'Unknown') - what filename-mode targets.""" + light = make_light() + light.music._tracks = [ + LightTrack( + playlist_item_id="2", audio_id="a2", + title="Some Old Rip", artist="Unknown", album="", + ), + ] + + match = light.music._find_matching_track("Some Old Rip", None) + assert match is not None and match.audio_id == "a2" + + +class TestTrackIdentity: + def test_metadata_mode_reads_title_and_artist(self): + light = make_light() + with patch("light_api.music.File", return_value={"title": ["Song"], "artist": ["Artist"]}): + assert light.music._track_identity("song.mp3", "metadata") == ("Song", "Artist") + + def test_metadata_mode_falls_back_to_filename_when_tags_missing(self): + """Default (replace=False): missing tags silently fall back to filename matching.""" + light = make_light() + with patch("light_api.music.File", return_value=None): + title, artist = light.music._track_identity( + "/path/Some Song.mp3", "metadata", replace=False + ) + assert title == "Some Song" + assert artist is None + + def test_metadata_mode_raises_under_replace_when_tags_missing(self): + """--replace is destructive, so a missing-tags fallback must be explicit, not silent.""" + light = make_light() + with patch("light_api.music.File", return_value=None): + with pytest.raises(ValueError, match="match_by='filename'"): + light.music._track_identity("/path/Some Song.mp3", "metadata", replace=True) + + def test_filename_mode_ignores_tags_entirely(self): + light = make_light() + with patch( + "light_api.music.File", + return_value={"title": ["Real Title"], "artist": ["Real Artist"]}, + ): + title, artist = light.music._track_identity("/path/Filename Title.mp3", "filename") + assert title == "Filename Title" + assert artist is None + + +class TestFindUploadMatches: + def test_ignores_cross_artist_title_collision(self): + light = make_light() + light.music._tracks = [ + LightTrack( + playlist_item_id="1", audio_id="a1", + title="Playing God", artist="Paramore", album="", + ), + LightTrack( + playlist_item_id="2", audio_id="a2", + title="New Song", artist="New Artist", album="", + ), + ] + + tags_by_path = { + "playing_god_polyphia.mp3": {"title": ["Playing God"], "artist": ["Polyphia"]}, + "new_song.mp3": {"title": ["New Song"], "artist": ["New Artist"]}, + } + + with patch("light_api.music.File", side_effect=lambda p, easy=True: tags_by_path[p]): + matches = light.music.find_upload_matches( + ["playing_god_polyphia.mp3", "new_song.mp3"], match_by="metadata" + ) + + assert "playing_god_polyphia.mp3" not in matches + assert matches["new_song.mp3"].audio_id == "a2" + + def make_note(overrides: dict | None = None) -> LightNote: data = dict(id="note-1", file_id="file-1", note_type="text", title="old title", updated_at="2026-01-01T00:00:00") if overrides: From d0efba182a2037afd64cc93904bd2fd5670b3789 Mon Sep 17 00:00:00 2001 From: garado Date: Sat, 1 Aug 2026 04:26:24 -0500 Subject: [PATCH 05/20] fix: intercept ValueErrors --- light_api/light_api/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/light_api/light_api/__init__.py b/light_api/light_api/__init__.py index f9097cc..88730c8 100644 --- a/light_api/light_api/__init__.py +++ b/light_api/light_api/__init__.py @@ -20,7 +20,7 @@ def wrapper(*args, **kwargs): ) light.__enter__() return f(light, *args, **kwargs) - except RuntimeError as e: + except (RuntimeError, ValueError) as e: raise click.ClickException(str(e)) return wrapper From 156acf8cb6c35d98a1c184b283b7519352c89efb Mon Sep 17 00:00:00 2001 From: garado Date: Sat, 1 Aug 2026 04:35:59 -0500 Subject: [PATCH 06/20] fix: address review comments --- light_api/light_api/music.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/light_api/light_api/music.py b/light_api/light_api/music.py index 50472e2..ff14063 100644 --- a/light_api/light_api/music.py +++ b/light_api/light_api/music.py @@ -293,8 +293,9 @@ def _track_identity( if replace: raise ValueError( f"Could not read title/artist metadata from {file_path!r}, and " - "--replace is destructive - pass match_by='filename' to explicitly " - "acknowledge the weaker match instead of silently falling back." + "--replace is destructive - pass match_by='filename' " + "(CLI: --match-by filename) to explicitly acknowledge the weaker " + "match instead of silently falling back." ) return os.path.splitext(os.path.basename(file_path))[0], None @@ -367,14 +368,16 @@ def upload_tracks( if not allow_duplicates: matches = self.find_upload_matches(files, match_by, replace) + + if replace and matches: + audio_ids = {t.audio_id for t in matches.values()} + self.delete_tracks_predicate(lambda t: t.audio_id in audio_ids) + to_upload = [] for file_path in files: match = matches.get(file_path) - if match is None: - to_upload.append(file_path) - elif replace: - self.delete_tracks_predicate(lambda t: t is match) + if match is None or replace: to_upload.append(file_path) else: log.info(f"Skipping {file_path!r}: matches existing track {match.title!r}") From 5ae98b4adfd3ef1c8d5d3854ee5401bcda0c0d3b Mon Sep 17 00:00:00 2001 From: garado Date: Sat, 1 Aug 2026 04:50:37 -0500 Subject: [PATCH 07/20] feat: add confirmation on non-matching replace --- light_cli_tui/light_cli_tui/cli.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/light_cli_tui/light_cli_tui/cli.py b/light_cli_tui/light_cli_tui/cli.py index bd8f0ac..14ada6e 100644 --- a/light_cli_tui/light_cli_tui/cli.py +++ b/light_cli_tui/light_cli_tui/cli.py @@ -287,6 +287,10 @@ def music_upload(light: Light, songs, allow_duplicates, replace, match_by, no_co console.print(f" {file_path} -> {t.artist} — {t.title}") if not click.confirm("Proceed?"): return + elif replace: + console.print("[dim]No matching tracks found; uploading all as new.[/dim]") + if not click.confirm("Proceed?"): + return with Progress( TextColumn("[progress.description]{task.description}"), From 731640b1803f72325e767d42dc7cf9cbfa71e83c Mon Sep 17 00:00:00 2001 From: garado Date: Sat, 1 Aug 2026 14:20:51 -0700 Subject: [PATCH 08/20] fix: simplify 'light music upload' behavior --- light_api/light_api/music.py | 85 ++++++++---------------------- light_cli_tui/light_cli_tui/cli.py | 22 ++------ tests/test_api.py | 50 ++++++------------ 3 files changed, 43 insertions(+), 114 deletions(-) diff --git a/light_api/light_api/music.py b/light_api/light_api/music.py index ff14063..19415af 100644 --- a/light_api/light_api/music.py +++ b/light_api/light_api/music.py @@ -7,7 +7,7 @@ import re import tempfile from enum import StrEnum -from typing import Callable, Literal +from typing import Callable from dataclasses import dataclass from mutagen._file import File @@ -266,68 +266,34 @@ def delete_tracks_by_artist_regex(self, pattern: str) -> None: """ self.delete_tracks_predicate(lambda t: bool(re.match(pattern, t.artist))) - def _track_identity( - self, - file_path: str, - match_by: Literal["metadata", "filename"], - replace: bool = False, - ) -> tuple[str, str | None]: - """Return (title, artist) used to match `file_path` against existing tracks. + def _track_identity(self, file_path: str) -> tuple[str, str]: + """Return (title, artist) for file_path, read from its tags where available. - 'filename' mode applies for tracks that were uploaded with no metadata; LightOS - displays those with title=filename, artist='Unknown'. - - If match_by='metadata' but `file_path` has no readable title/artist tags, this - auto-falls back to filename matching for that file - unless replace=True, since - a false-positive filename-only match would delete the wrong track under - --replace. In that case, a missing tag raises instead, requiring the caller to - explicitly pass match_by='filename' to acknowledge the weaker match. - """ - if match_by == "metadata": - tags = File(file_path, easy=True) - title = tags.get("title", [None])[0] if tags else None - artist = tags.get("artist", [None])[0] if tags else None - if title and artist: - return title, artist - - if replace: - raise ValueError( - f"Could not read title/artist metadata from {file_path!r}, and " - "--replace is destructive - pass match_by='filename' " - "(CLI: --match-by filename) to explicitly acknowledge the weaker " - "match instead of silently falling back." - ) - - return os.path.splitext(os.path.basename(file_path))[0], None - - _UNKNOWN_ARTIST_VALUES = {"", "Unknown"} - - def _find_matching_track(self, title: str, artist: str | None) -> LightTrack | None: - """Find an existing track matching (title, artist). - - artist=None matches only untagged tracks (artist is blank or "Unknown"). + Falls back to (filename, "Unknown") for whatever's missing - matching how + LightOS represents a track that was itself uploaded with no metadata. """ + tags = File(file_path, easy=True) + title = (tags.get("title", [None])[0] if tags else None) or os.path.splitext( + os.path.basename(file_path) + )[0] + artist = (tags.get("artist", [None])[0] if tags else None) or "Unknown" + return title, artist + + def _find_matching_track(self, title: str, artist: str) -> LightTrack | None: + """Find an existing track with an exact (title, artist) match.""" for t in self._tracks: - if t.title != title: - continue - if artist is None: - if t.artist in self._UNKNOWN_ARTIST_VALUES: - return t - elif t.artist == artist: + if t.title == title and t.artist == artist: return t return None - def find_upload_matches( - self, - files: list[str], - match_by: Literal["metadata", "filename"] = "metadata", - replace: bool = False, - ) -> dict[str, LightTrack]: - """Return {file_path: existing LightTrack} for files that match a track already on the device.""" + def find_upload_matches(self, files: list[str]) -> dict[str, LightTrack]: + """Return {file_path: existing LightTrack} for files that match a track already + on the device. Shared by upload_tracks and CLI confirmation prompts, so there's + exactly one definition of "what counts as a duplicate".""" self._init_tracks() matches: dict[str, LightTrack] = {} for file_path in files: - title, artist = self._track_identity(file_path, match_by, replace) + title, artist = self._track_identity(file_path) match = self._find_matching_track(title, artist) if match is not None: matches[file_path] = match @@ -338,7 +304,6 @@ def upload_tracks( files: list[str], allow_duplicates: bool = False, replace: bool = False, - match_by: Literal["metadata", "filename"] = "metadata", convert_flac: bool = True, on_progress: "Callable[[str, int, int], None] | None" = None, ) -> None: @@ -351,14 +316,6 @@ def upload_tracks( replace: If True, delete a file's matching existing track (if any) before uploading it. If False (default), files matching an existing track are skipped instead, leaving the existing track untouched. - match_by: How to identify a file for duplicate matching. - "metadata" (default) reads title+artist from the file's ID3/audio tags. - Per file, if tags are missing this auto-falls back to filename-only - matching - unless replace=True, in which case it raises instead - (a false-positive filename-only match would delete the wrong track). - "filename" matches on filename-as-title only for every file, for - tracks that were themselves uploaded with no metadata - LightOS - displays those with title=filename and artist="Unknown". """ if replace and allow_duplicates: raise ValueError("replace and allow_duplicates are mutually exclusive") @@ -367,7 +324,7 @@ def upload_tracks( to_upload = files if not allow_duplicates: - matches = self.find_upload_matches(files, match_by, replace) + matches = self.find_upload_matches(files) if replace and matches: audio_ids = {t.audio_id for t in matches.values()} diff --git a/light_cli_tui/light_cli_tui/cli.py b/light_cli_tui/light_cli_tui/cli.py index 14ada6e..f0dcfd0 100644 --- a/light_cli_tui/light_cli_tui/cli.py +++ b/light_cli_tui/light_cli_tui/cli.py @@ -243,30 +243,19 @@ def podcasts_delete(light: Light, title): help="Delete a file's matching existing track before uploading it, instead of " "skipping the file (the default).", ) -@click.option( - "--match-by", - "-m", - type=click.Choice(["filename", "metadata"]), - default="metadata", - show_default=True, - help="How to identify a file for duplicate matching. 'metadata' matches on " - "title+artist from tags; 'filename' matches on filename-as-title only, for tracks " - "that were themselves uploaded with no metadata (LightOS shows those with " - "title=filename, artist=Unknown).", -) @click.option( "--no-convert-flac", is_flag=True, default=False, help="Skip FLAC to MP3 conversion (conversion is on by default to preserve metadata).", ) -def music_upload(light: Light, songs, allow_duplicates, replace, match_by, no_convert_flac): +def music_upload(light: Light, songs, allow_duplicates, replace, no_convert_flac): """Upload one or more audio files to your device. Duplicate detection is on by default: files matching an existing track - (by title+artist) are skipped, leaving the existing track untouched. Use - `--replace` to delete-and-replace matches instead, or `--allow-duplicates` - to skip the check entirely. + (by title+artist, read from tags) are skipped, leaving the existing track + untouched. Use `--replace` to delete-and-replace matches instead, or + `--allow-duplicates` to skip the check entirely. **Example:** @@ -278,7 +267,7 @@ def music_upload(light: Light, songs, allow_duplicates, replace, match_by, no_co files = list(songs) if not allow_duplicates: - matches = light.music.find_upload_matches(files, match_by, replace) + matches = light.music.find_upload_matches(files) if matches: verb = "overwrite" if replace else "skip" @@ -314,7 +303,6 @@ def on_progress(filename: str, sent: int, total: int) -> None: files, allow_duplicates=allow_duplicates, replace=replace, - match_by=match_by, convert_flac=not no_convert_flac, on_progress=on_progress, ) diff --git a/tests/test_api.py b/tests/test_api.py index 5862b99..388a72c 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -365,6 +365,8 @@ def test_raises_on_error_response(self, f_devices, f_tools): class TestFindMatchingTrack: def test_ignores_cross_artist_title_collision(self): + """Regression test for #18: "Playing God" by Polyphia must not match an + existing "Playing God" by Paramore just because titles collide.""" light = make_light() light.music._tracks = [ LightTrack( @@ -373,17 +375,12 @@ def test_ignores_cross_artist_title_collision(self): ), ] - # artist=None (filename-mode / metadata-fallback identity) must not - # wildcard-match a track that has a real, different artist. - assert light.music._find_matching_track("Playing God", None) is None - - # Precise (title, artist) matching still works correctly. assert light.music._find_matching_track("Playing God", "Paramore") is not None assert light.music._find_matching_track("Playing God", "Polyphia") is None - def test_matches_untagged_tracks_by_title_only(self): - """artist=None should still match existing tracks that are themselves - untagged (artist blank or 'Unknown') - what filename-mode targets.""" + def test_matches_untagged_tracks_by_exact_unknown_artist(self): + """A file with no tags resolves to artist="Unknown" and should exact-match + an existing track that was itself uploaded with no metadata.""" light = make_light() light.music._tracks = [ LightTrack( @@ -392,42 +389,29 @@ def test_matches_untagged_tracks_by_title_only(self): ), ] - match = light.music._find_matching_track("Some Old Rip", None) + match = light.music._find_matching_track("Some Old Rip", "Unknown") assert match is not None and match.audio_id == "a2" class TestTrackIdentity: - def test_metadata_mode_reads_title_and_artist(self): + def test_reads_title_and_artist_from_tags(self): light = make_light() with patch("light_api.music.File", return_value={"title": ["Song"], "artist": ["Artist"]}): - assert light.music._track_identity("song.mp3", "metadata") == ("Song", "Artist") + assert light.music._track_identity("song.mp3") == ("Song", "Artist") - def test_metadata_mode_falls_back_to_filename_when_tags_missing(self): - """Default (replace=False): missing tags silently fall back to filename matching.""" + def test_falls_back_to_filename_and_unknown_when_tags_missing(self): light = make_light() with patch("light_api.music.File", return_value=None): - title, artist = light.music._track_identity( - "/path/Some Song.mp3", "metadata", replace=False - ) + title, artist = light.music._track_identity("/path/Some Song.mp3") assert title == "Some Song" - assert artist is None + assert artist == "Unknown" - def test_metadata_mode_raises_under_replace_when_tags_missing(self): - """--replace is destructive, so a missing-tags fallback must be explicit, not silent.""" + def test_falls_back_per_field_when_only_one_tag_is_missing(self): light = make_light() - with patch("light_api.music.File", return_value=None): - with pytest.raises(ValueError, match="match_by='filename'"): - light.music._track_identity("/path/Some Song.mp3", "metadata", replace=True) - - def test_filename_mode_ignores_tags_entirely(self): - light = make_light() - with patch( - "light_api.music.File", - return_value={"title": ["Real Title"], "artist": ["Real Artist"]}, - ): - title, artist = light.music._track_identity("/path/Filename Title.mp3", "filename") - assert title == "Filename Title" - assert artist is None + with patch("light_api.music.File", return_value={"title": ["Real Title"]}): + title, artist = light.music._track_identity("/path/Filename.mp3") + assert title == "Real Title" + assert artist == "Unknown" class TestFindUploadMatches: @@ -451,7 +435,7 @@ def test_ignores_cross_artist_title_collision(self): with patch("light_api.music.File", side_effect=lambda p, easy=True: tags_by_path[p]): matches = light.music.find_upload_matches( - ["playing_god_polyphia.mp3", "new_song.mp3"], match_by="metadata" + ["playing_god_polyphia.mp3", "new_song.mp3"] ) assert "playing_god_polyphia.mp3" not in matches From 049f687eb6e2b2a6859aa5d0009785a7edf933cf Mon Sep 17 00:00:00 2001 From: garado Date: Sat, 1 Aug 2026 14:21:17 -0700 Subject: [PATCH 09/20] docs: remove outdated example in readme --- README.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/README.md b/README.md index a52acee..16b2a44 100644 --- a/README.md +++ b/README.md @@ -85,10 +85,6 @@ light music upload song1 song2 song3 # Delete-and-replace matching existing tracks instead of skipping them light music upload song1 song2 song3 --replace -# Upload tracks -# Match by filename instead of metadata tags (for files with missing/broken tags) -light music upload song1 song2 song3 --match-by filename - # Upload tracks # Skip duplicate checking entirely, always upload light music upload --allow-duplicates song1 song2 song3 From 88082ff479af16b30364fe24f80f6a09d06ee568 Mon Sep 17 00:00:00 2001 From: garado Date: Sat, 1 Aug 2026 15:06:12 -0700 Subject: [PATCH 10/20] docs: update docstrings for clarity --- light_api/light_api/music.py | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/light_api/light_api/music.py b/light_api/light_api/music.py index 19415af..04de2d4 100644 --- a/light_api/light_api/music.py +++ b/light_api/light_api/music.py @@ -267,10 +267,17 @@ def delete_tracks_by_artist_regex(self, pattern: str) -> None: self.delete_tracks_predicate(lambda t: bool(re.match(pattern, t.artist))) def _track_identity(self, file_path: str) -> tuple[str, str]: - """Return (title, artist) for file_path, read from its tags where available. + """Return (title, artist) for an audio file at file_path. - Falls back to (filename, "Unknown") for whatever's missing - matching how - LightOS represents a track that was itself uploaded with no metadata. + If the track has metadata for a field: use that metadata. + If no metadata for a field:titles will fall back to just 'filename', and + artists will fall back to "Unknown", matching the dashboard's implementation. + + Args: + file_path: File path of audio file to process. + + Returns: + (title, artist) tuple representing that file's content. """ tags = File(file_path, easy=True) title = (tags.get("title", [None])[0] if tags else None) or os.path.splitext( @@ -287,9 +294,15 @@ def _find_matching_track(self, title: str, artist: str) -> LightTrack | None: return None def find_upload_matches(self, files: list[str]) -> dict[str, LightTrack]: - """Return {file_path: existing LightTrack} for files that match a track already - on the device. Shared by upload_tracks and CLI confirmation prompts, so there's - exactly one definition of "what counts as a duplicate".""" + """Given a list of local audio files, find those that already exist on the device and + return the matching existing LightTrack instances. + + Args: + files: A list of paths to audio files. + + Returns: + A {file_path: LightTrack} dict. + """ self._init_tracks() matches: dict[str, LightTrack] = {} for file_path in files: From 8422014ee83aa52462ede7374b0a1595eb49daa6 Mon Sep 17 00:00:00 2001 From: garado Date: Sat, 1 Aug 2026 15:14:21 -0700 Subject: [PATCH 11/20] fix: rename replace->overwrite for clarity --- README.md | 2 +- light_api/light_api/music.py | 18 ++++++++--------- light_cli_tui/light_cli_tui/cli.py | 32 +++++++++++++++++------------- 3 files changed, 28 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index 16b2a44..25813fc 100644 --- a/README.md +++ b/README.md @@ -83,7 +83,7 @@ light music upload song1 song2 song3 # Upload tracks # Delete-and-replace matching existing tracks instead of skipping them -light music upload song1 song2 song3 --replace +light music upload song1 song2 song3 --overwrite # Upload tracks # Skip duplicate checking entirely, always upload diff --git a/light_api/light_api/music.py b/light_api/light_api/music.py index 04de2d4..86a76e3 100644 --- a/light_api/light_api/music.py +++ b/light_api/light_api/music.py @@ -270,7 +270,7 @@ def _track_identity(self, file_path: str) -> tuple[str, str]: """Return (title, artist) for an audio file at file_path. If the track has metadata for a field: use that metadata. - If no metadata for a field:titles will fall back to just 'filename', and + If no metadata for a field: titles will fall back to just 'filename', and artists will fall back to "Unknown", matching the dashboard's implementation. Args: @@ -316,7 +316,7 @@ def upload_tracks( self, files: list[str], allow_duplicates: bool = False, - replace: bool = False, + overwrite: bool = False, convert_flac: bool = True, on_progress: "Callable[[str, int, int], None] | None" = None, ) -> None: @@ -326,12 +326,12 @@ def upload_tracks( files: List of paths to audio files to upload. allow_duplicates: If True, skip duplicate checking entirely and always upload, potentially creating multiple tracks with the same title/artist. - replace: If True, delete a file's matching existing track (if any) before - uploading it. If False (default), files matching an existing track - are skipped instead, leaving the existing track untouched. + overwrite: If True, delete a file's matching existing track (if any) before + uploading it. If False (default), files matching an existing track + are skipped instead, leaving the existing track untouched. """ - if replace and allow_duplicates: - raise ValueError("replace and allow_duplicates are mutually exclusive") + if overwrite and allow_duplicates: + raise ValueError("overwrite and allow_duplicates are mutually exclusive") manual_update_cmds = [] to_upload = files @@ -339,7 +339,7 @@ def upload_tracks( if not allow_duplicates: matches = self.find_upload_matches(files) - if replace and matches: + if overwrite and matches: audio_ids = {t.audio_id for t in matches.values()} self.delete_tracks_predicate(lambda t: t.audio_id in audio_ids) @@ -347,7 +347,7 @@ def upload_tracks( for file_path in files: match = matches.get(file_path) - if match is None or replace: + if match is None or overwrite: to_upload.append(file_path) else: log.info(f"Skipping {file_path!r}: matches existing track {match.title!r}") diff --git a/light_cli_tui/light_cli_tui/cli.py b/light_cli_tui/light_cli_tui/cli.py index f0dcfd0..fb4dffa 100644 --- a/light_cli_tui/light_cli_tui/cli.py +++ b/light_cli_tui/light_cli_tui/cli.py @@ -43,7 +43,9 @@ def invoke(self, ctx: click.Context): raise -@click.group(cls=JsonAwareGroup, context_settings={"help_option_names": ["-h", "--help"]}) +@click.group( + cls=JsonAwareGroup, context_settings={"help_option_names": ["-h", "--help"]} +) @click.version_option(package_name="light-phone-cli-tui", prog_name="light") @click.option("--email", default=None, help="Light account email address.") @click.option("--email-file", default=None, help="Path to file containing email.") @@ -99,9 +101,7 @@ def cli( `--device-id` (mutually exclusive). """ if (phone_number or phone_number_file) and (device_id or device_id_file): - raise click.UsageError( - "--phone-number and --device-id are mutually exclusive." - ) + raise click.UsageError("--phone-number and --device-id are mutually exclusive.") logging.basicConfig(format="%(name)s %(levelname)s %(message)s") logging.getLogger("light").setLevel(log_level.upper()) @@ -238,7 +238,7 @@ def podcasts_delete(light: Light, title): help="Skip duplicate checking entirely and always upload.", ) @click.option( - "--replace", + "--overwrite", is_flag=True, help="Delete a file's matching existing track before uploading it, instead of " "skipping the file (the default).", @@ -249,20 +249,22 @@ def podcasts_delete(light: Light, title): default=False, help="Skip FLAC to MP3 conversion (conversion is on by default to preserve metadata).", ) -def music_upload(light: Light, songs, allow_duplicates, replace, no_convert_flac): +def music_upload(light: Light, songs, allow_duplicates, overwrite, no_convert_flac): """Upload one or more audio files to your device. Duplicate detection is on by default: files matching an existing track - (by title+artist, read from tags) are skipped, leaving the existing track - untouched. Use `--replace` to delete-and-replace matches instead, or + (by title+artist, read from metadata) are skipped, leaving the existing track + untouched. Use `--overwrite` to delete-and-replace matches instead, or `--allow-duplicates` to skip the check entirely. **Example:** `light music upload track1.mp3 track2.mp3` """ - if replace and allow_duplicates: - raise click.UsageError("--replace and --allow-duplicates are mutually exclusive.") + if overwrite and allow_duplicates: + raise click.UsageError( + "--overwrite and --allow-duplicates are mutually exclusive." + ) files = list(songs) @@ -270,13 +272,13 @@ def music_upload(light: Light, songs, allow_duplicates, replace, no_convert_flac matches = light.music.find_upload_matches(files) if matches: - verb = "overwrite" if replace else "skip" + verb = "overwrite" if overwrite else "skip" console.print(f"Tracks to {verb} ({len(matches)}):") for file_path, t in matches.items(): console.print(f" {file_path} -> {t.artist} — {t.title}") if not click.confirm("Proceed?"): return - elif replace: + elif overwrite: console.print("[dim]No matching tracks found; uploading all as new.[/dim]") if not click.confirm("Proceed?"): return @@ -302,7 +304,7 @@ def on_progress(filename: str, sent: int, total: int) -> None: light.music.upload_tracks( files, allow_duplicates=allow_duplicates, - replace=replace, + overwrite=overwrite, convert_flac=not no_convert_flac, on_progress=on_progress, ) @@ -472,7 +474,9 @@ def notes_list(light: Light, show_id=False, content_preview=False): def render_human_readable(): if content_preview: - console.print(f"[dim]Content preview enabled. This might take a while.[/dim]") + console.print( + f"[dim]Content preview enabled. This might take a while.[/dim]" + ) for i, note in enumerate(all_notes, 1): if note.note_type == "audio": From ec7dd928a3f59a11422810e53da71c96e58581df Mon Sep 17 00:00:00 2001 From: garado Date: Sat, 1 Aug 2026 15:29:57 -0700 Subject: [PATCH 12/20] fix: always require confirmation for light music upload, no matter what the params are --- light_cli_tui/light_cli_tui/cli.py | 45 +++++++++++++++--------------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/light_cli_tui/light_cli_tui/cli.py b/light_cli_tui/light_cli_tui/cli.py index fb4dffa..81f5a94 100644 --- a/light_cli_tui/light_cli_tui/cli.py +++ b/light_cli_tui/light_cli_tui/cli.py @@ -235,27 +235,21 @@ def podcasts_delete(light: Light, title): @click.option( "--allow-duplicates", is_flag=True, - help="Skip duplicate checking entirely and always upload.", + help="Allow uploading duplicate tracks.", ) @click.option( "--overwrite", is_flag=True, - help="Delete a file's matching existing track before uploading it, instead of " - "skipping the file (the default).", + help="Overwrite duplicate tracks.", ) @click.option( "--no-convert-flac", is_flag=True, default=False, - help="Skip FLAC to MP3 conversion (conversion is on by default to preserve metadata).", + help="Skip FLAC to MP3 conversion. Conversion is on by default to preserve metadata.", ) def music_upload(light: Light, songs, allow_duplicates, overwrite, no_convert_flac): - """Upload one or more audio files to your device. - - Duplicate detection is on by default: files matching an existing track - (by title+artist, read from metadata) are skipped, leaving the existing track - untouched. Use `--overwrite` to delete-and-replace matches instead, or - `--allow-duplicates` to skip the check entirely. + """Upload audio files to your device. **Example:** @@ -267,21 +261,26 @@ def music_upload(light: Light, songs, allow_duplicates, overwrite, no_convert_fl ) files = list(songs) + matches = light.music.find_upload_matches(files) + new_count = len(files) - len(matches) - if not allow_duplicates: - matches = light.music.find_upload_matches(files) - - if matches: - verb = "overwrite" if overwrite else "skip" - console.print(f"Tracks to {verb} ({len(matches)}):") - for file_path, t in matches.items(): - console.print(f" {file_path} -> {t.artist} — {t.title}") - if not click.confirm("Proceed?"): - return + console.print(f"{new_count} new track{'s' if new_count != 1 else ''} will be added") + if matches: + if allow_duplicates: + verb = "duplicated" elif overwrite: - console.print("[dim]No matching tracks found; uploading all as new.[/dim]") - if not click.confirm("Proceed?"): - return + verb = "overwritten" + else: + verb = "skipped" + console.print( + f"{len(matches)} existing track{'s' if len(matches) != 1 else ''} " + f"will be {verb}:" + ) + for file_path, t in matches.items(): + console.print(f" {file_path} -> {t.artist} — {t.title}") + + if not click.confirm("Proceed?"): + return with Progress( TextColumn("[progress.description]{task.description}"), From ce5ea077c217198776f102bb2da855512da6643b Mon Sep 17 00:00:00 2001 From: garado Date: Sat, 1 Aug 2026 15:32:02 -0700 Subject: [PATCH 13/20] fix: address more review comments --- light_api/light_api/__init__.py | 2 +- light_api/light_api/music.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/light_api/light_api/__init__.py b/light_api/light_api/__init__.py index 88730c8..f9097cc 100644 --- a/light_api/light_api/__init__.py +++ b/light_api/light_api/__init__.py @@ -20,7 +20,7 @@ def wrapper(*args, **kwargs): ) light.__enter__() return f(light, *args, **kwargs) - except (RuntimeError, ValueError) as e: + except RuntimeError as e: raise click.ClickException(str(e)) return wrapper diff --git a/light_api/light_api/music.py b/light_api/light_api/music.py index 86a76e3..0b0a52f 100644 --- a/light_api/light_api/music.py +++ b/light_api/light_api/music.py @@ -350,7 +350,7 @@ def upload_tracks( if match is None or overwrite: to_upload.append(file_path) else: - log.info(f"Skipping {file_path!r}: matches existing track {match.title!r}") + log.info(f"Skipping {file_path!r}: matches existing ({match.title!r}, {match.artist!r})") files = to_upload From 7bc8dc9dbc97f927aab5e6c9cfb1f7422ae8ebcd Mon Sep 17 00:00:00 2001 From: garado Date: Sat, 1 Aug 2026 15:52:13 -0700 Subject: [PATCH 14/20] refactor: abstract matching logic to improve testability of music upload --- light_api/light_api/music.py | 52 +++++++++++++++++++++------------- tests/test_api.py | 55 ++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 19 deletions(-) diff --git a/light_api/light_api/music.py b/light_api/light_api/music.py index 0b0a52f..4e00532 100644 --- a/light_api/light_api/music.py +++ b/light_api/light_api/music.py @@ -312,6 +312,33 @@ def find_upload_matches(self, files: list[str]) -> dict[str, LightTrack]: matches[file_path] = match return matches + def _resolve_upload_plan( + self, files: list[str], allow_duplicates: bool, overwrite: bool + ) -> tuple[list[str], list[LightTrack]]: + """Return the subset of files to upload and the subset of files to overwrite after + applying allow_duplicates/overwrite behavior flags. + + Returns: + Tuple of lists. First item is list[str] of files to upload. Second item is list[LightTrack] + of files to be overwritten. + """ + if allow_duplicates: + return (files, []) + + matches = self.find_upload_matches(files) + to_delete = list({t.audio_id: t for t in matches.values()}.values()) if overwrite else [] + + to_upload = [] + for file_path in files: + match = matches.get(file_path) + + if match is None or overwrite: + to_upload.append(file_path) + else: + log.info(f"Skipping {file_path!r}: matches existing ({match.title!r}, {match.artist!r})") + + return (to_upload, to_delete) + def upload_tracks( self, files: list[str], @@ -334,27 +361,13 @@ def upload_tracks( raise ValueError("overwrite and allow_duplicates are mutually exclusive") manual_update_cmds = [] - to_upload = files - - if not allow_duplicates: - matches = self.find_upload_matches(files) - - if overwrite and matches: - audio_ids = {t.audio_id for t in matches.values()} - self.delete_tracks_predicate(lambda t: t.audio_id in audio_ids) + to_upload, to_delete = self._resolve_upload_plan(files, allow_duplicates, overwrite) - to_upload = [] - for file_path in files: - match = matches.get(file_path) + if to_delete: + audio_ids = {t.audio_id for t in to_delete} + self.delete_tracks_predicate(lambda t: t.audio_id in audio_ids) - if match is None or overwrite: - to_upload.append(file_path) - else: - log.info(f"Skipping {file_path!r}: matches existing ({match.title!r}, {match.artist!r})") - - files = to_upload - - for file_path in files: + for file_path in to_upload: log.info(f"Uploading {file_path}") if not os.path.exists(file_path): @@ -421,6 +434,7 @@ def _chunks(path: str, total: int, filename: str): f"Upload {os.path.basename(upload_path)}: {put_resp.status_code} {put_resp.text}" ) + # (TODO Is this even necessary anymore now that we have FLAC autoconversion) # The Light API has an issue where it won't set title/artist metadata properly # when uploading non-mp3 files. Give user list of commands to patch manually after # upload since files are still processing and a patch immediately after will fail. diff --git a/tests/test_api.py b/tests/test_api.py index 388a72c..c963eb6 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -442,6 +442,61 @@ def test_ignores_cross_artist_title_collision(self): assert matches["new_song.mp3"].audio_id == "a2" +class TestResolveUploadPlan: + """Unit tests for LightMusic._resolve_upload_plan's skip/overwrite/allow_duplicates + filtering. Pure computation - no mocking of delete_tracks_predicate needed.""" + + def _light_with_track(self): + light = make_light() + light.music._tracks = [ + LightTrack( + playlist_item_id="1", audio_id="a1", + title="Song", artist="Artist", album="", + ), + ] + return light + + def test_allow_duplicates_returns_files_unchanged_and_nothing_to_delete(self): + light = self._light_with_track() + + to_upload, to_delete = light.music._resolve_upload_plan( + ["match.mp3", "new.mp3"], allow_duplicates=True, overwrite=False + ) + + assert to_upload == ["match.mp3", "new.mp3"] + assert to_delete == [] + + def test_default_skips_matching_files_without_deleting(self): + light = self._light_with_track() + + tags_by_path = { + "match.mp3": {"title": ["Song"], "artist": ["Artist"]}, + "new.mp3": {"title": ["New Song"], "artist": ["New Artist"]}, + } + with patch("light_api.music.File", side_effect=lambda p, easy=True: tags_by_path[p]): + to_upload, to_delete = light.music._resolve_upload_plan( + ["match.mp3", "new.mp3"], allow_duplicates=False, overwrite=False + ) + + assert to_upload == ["new.mp3"] + assert to_delete == [] + + def test_overwrite_returns_matches_to_delete_and_still_uploads_them(self): + light = self._light_with_track() + + tags_by_path = { + "match.mp3": {"title": ["Song"], "artist": ["Artist"]}, + "new.mp3": {"title": ["New Song"], "artist": ["New Artist"]}, + } + with patch("light_api.music.File", side_effect=lambda p, easy=True: tags_by_path[p]): + to_upload, to_delete = light.music._resolve_upload_plan( + ["match.mp3", "new.mp3"], allow_duplicates=False, overwrite=True + ) + + assert to_upload == ["match.mp3", "new.mp3"] + assert to_delete == [light.music._tracks[0]] + + def make_note(overrides: dict | None = None) -> LightNote: data = dict(id="note-1", file_id="file-1", note_type="text", title="old title", updated_at="2026-01-01T00:00:00") if overrides: From f7afb05b366f059ef402ca455efdba6d1bb62932 Mon Sep 17 00:00:00 2001 From: garado Date: Sat, 1 Aug 2026 16:00:12 -0700 Subject: [PATCH 15/20] fix: address review comments AGAIN --- README.md | 2 +- light_cli_tui/light_cli_tui/cli.py | 23 ++++++++++++----------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 25813fc..cabcba4 100644 --- a/README.md +++ b/README.md @@ -86,7 +86,7 @@ light music upload song1 song2 song3 light music upload song1 song2 song3 --overwrite # Upload tracks -# Skip duplicate checking entirely, always upload +# Allow uploading duplicate tracks light music upload --allow-duplicates song1 song2 song3 ``` diff --git a/light_cli_tui/light_cli_tui/cli.py b/light_cli_tui/light_cli_tui/cli.py index 81f5a94..b708d50 100644 --- a/light_cli_tui/light_cli_tui/cli.py +++ b/light_cli_tui/light_cli_tui/cli.py @@ -262,20 +262,21 @@ def music_upload(light: Light, songs, allow_duplicates, overwrite, no_convert_fl files = list(songs) matches = light.music.find_upload_matches(files) - new_count = len(files) - len(matches) - console.print(f"{new_count} new track{'s' if new_count != 1 else ''} will be added") + skip_count = len(matches) if matches and not overwrite and not allow_duplicates else 0 + upload_count = len(files) - skip_count + + console.print(f"{upload_count} track{'s' if upload_count != 1 else ''} will be uploaded") if matches: - if allow_duplicates: - verb = "duplicated" - elif overwrite: - verb = "overwritten" + if skip_count: + console.print( + f"{skip_count} existing track{'s' if skip_count != 1 else ''} will be skipped:" + ) else: - verb = "skipped" - console.print( - f"{len(matches)} existing track{'s' if len(matches) != 1 else ''} " - f"will be {verb}:" - ) + verb = "duplicated" if allow_duplicates else "overwritten" + console.print( + f"{len(matches)} of these already exist and will be {verb}:" + ) for file_path, t in matches.items(): console.print(f" {file_path} -> {t.artist} — {t.title}") From 7d750802590286dceeb233046c9ab404ee979e58 Mon Sep 17 00:00:00 2001 From: garado Date: Sat, 1 Aug 2026 16:16:20 -0700 Subject: [PATCH 16/20] fix: prevent overwriting existing tracks if their replacement doesn't exist on disk --- light_api/light_api/music.py | 16 +++++++++++----- tests/test_api.py | 28 +++++++++++++++++++++++++++- 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/light_api/light_api/music.py b/light_api/light_api/music.py index 4e00532..0d5b0df 100644 --- a/light_api/light_api/music.py +++ b/light_api/light_api/music.py @@ -361,7 +361,17 @@ def upload_tracks( raise ValueError("overwrite and allow_duplicates are mutually exclusive") manual_update_cmds = [] - to_upload, to_delete = self._resolve_upload_plan(files, allow_duplicates, overwrite) + + existing_files: list[str] = [] + for file_path in files: + if os.path.exists(file_path): + existing_files.append(file_path) + else: + log.warning(f"File not found, skipping: {file_path}") + + to_upload, to_delete = self._resolve_upload_plan( + existing_files, allow_duplicates, overwrite + ) if to_delete: audio_ids = {t.audio_id for t in to_delete} @@ -370,10 +380,6 @@ def upload_tracks( for file_path in to_upload: log.info(f"Uploading {file_path}") - if not os.path.exists(file_path): - log.warning(f"File not found, skipping: {file_path}") - continue - tmp_path = None try: if convert_flac and file_path.lower().endswith(".flac"): diff --git a/tests/test_api.py b/tests/test_api.py index c963eb6..8a8d146 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -5,7 +5,7 @@ import respx import httpx from types import SimpleNamespace -from unittest.mock import patch +from unittest.mock import MagicMock, patch from light_api.client import Light from light_api.notes import LightNote @@ -497,6 +497,32 @@ def test_overwrite_returns_matches_to_delete_and_still_uploads_them(self): assert to_delete == [light.music._tracks[0]] +class TestUploadTracksExcludesMissingFiles: + """overwrite=True must never delete a track whose replacement file doesn't exist on disk.""" + + def test_missing_file_never_reaches_matching_or_deletion(self): + light = make_light() + light.music._tracks = [ + LightTrack( + playlist_item_id="1", audio_id="a1", + title="Song", artist="Artist", album="", + ), + ] + light.music.delete_tracks_predicate = MagicMock() + + # Tags are mocked to guarantee a match *would* occur if File() were ever + # called on this path - but the path doesn't exist, so matching/deletion + # must never even be attempted for it. + with patch( + "light_api.music.File", + return_value={"title": ["Song"], "artist": ["Artist"]}, + ) as mock_file: + light.music.upload_tracks(["/nonexistent/match.mp3"], overwrite=True) + + mock_file.assert_not_called() + light.music.delete_tracks_predicate.assert_not_called() + + def make_note(overrides: dict | None = None) -> LightNote: data = dict(id="note-1", file_id="file-1", note_type="text", title="old title", updated_at="2026-01-01T00:00:00") if overrides: From 65502fdf72182b49277eacc656bb9fca75939adb Mon Sep 17 00:00:00 2001 From: garado Date: Sat, 1 Aug 2026 16:25:32 -0700 Subject: [PATCH 17/20] feat(cli): add --verbose flag to music upload to show full list of tracks instead of just counts --- light_cli_tui/light_cli_tui/cli.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/light_cli_tui/light_cli_tui/cli.py b/light_cli_tui/light_cli_tui/cli.py index b708d50..905bc74 100644 --- a/light_cli_tui/light_cli_tui/cli.py +++ b/light_cli_tui/light_cli_tui/cli.py @@ -228,6 +228,8 @@ def podcasts_delete(light: Light, title): # -- Music commands ------------------------------------------------------------- +_VERBOSE_LIST_THRESHOLD = 20 + @music.command("upload") @with_light @@ -248,7 +250,14 @@ def podcasts_delete(light: Light, title): default=False, help="Skip FLAC to MP3 conversion. Conversion is on by default to preserve metadata.", ) -def music_upload(light: Light, songs, allow_duplicates, overwrite, no_convert_flac): +@click.option( + "--verbose", + "-v", + is_flag=True, + default=False, + help="Show the full list of affected tracks, even for large batches.", +) +def music_upload(light: Light, songs, allow_duplicates, overwrite, no_convert_flac, verbose): """Upload audio files to your device. **Example:** @@ -277,8 +286,12 @@ def music_upload(light: Light, songs, allow_duplicates, overwrite, no_convert_fl console.print( f"{len(matches)} of these already exist and will be {verb}:" ) - for file_path, t in matches.items(): - console.print(f" {file_path} -> {t.artist} — {t.title}") + + if not verbose and len(matches) > _VERBOSE_LIST_THRESHOLD: + console.print("[dim]Use --verbose/-v to show full list.[/dim]") + else: + for file_path, t in matches.items(): + console.print(f" {file_path} -> {t.artist} — {t.title}") if not click.confirm("Proceed?"): return From 5c881003cb08371a54c1c13f54f4fe08172a45d2 Mon Sep 17 00:00:00 2001 From: garado Date: Sat, 1 Aug 2026 16:34:04 -0700 Subject: [PATCH 18/20] fix: fix inconsistent invalid-file handling between api and cli --- light_api/light_api/music.py | 20 +++++++++++++------- light_cli_tui/light_cli_tui/cli.py | 5 ++++- tests/test_api.py | 15 ++++++++++++++- 3 files changed, 31 insertions(+), 9 deletions(-) diff --git a/light_api/light_api/music.py b/light_api/light_api/music.py index 0d5b0df..5323ef8 100644 --- a/light_api/light_api/music.py +++ b/light_api/light_api/music.py @@ -339,6 +339,15 @@ def _resolve_upload_plan( return (to_upload, to_delete) + @staticmethod + def filter_valid_tracks(files: list[str]) -> tuple[list[str], list[str]]: + """Verify validity of audio files, returning (valid, invalid).""" + valid = [] + invalid = [] + for file_path in files: + (valid if os.path.exists(file_path) else invalid).append(file_path) + return valid, invalid + def upload_tracks( self, files: list[str], @@ -362,15 +371,12 @@ def upload_tracks( manual_update_cmds = [] - existing_files: list[str] = [] - for file_path in files: - if os.path.exists(file_path): - existing_files.append(file_path) - else: - log.warning(f"File not found, skipping: {file_path}") + valid_files, invalid_files = self.filter_valid_tracks(files) + for file_path in invalid_files: + log.warning(f"File not found, skipping: {file_path}") to_upload, to_delete = self._resolve_upload_plan( - existing_files, allow_duplicates, overwrite + valid_files, allow_duplicates, overwrite ) if to_delete: diff --git a/light_cli_tui/light_cli_tui/cli.py b/light_cli_tui/light_cli_tui/cli.py index 905bc74..755a61c 100644 --- a/light_cli_tui/light_cli_tui/cli.py +++ b/light_cli_tui/light_cli_tui/cli.py @@ -269,7 +269,10 @@ def music_upload(light: Light, songs, allow_duplicates, overwrite, no_convert_fl "--overwrite and --allow-duplicates are mutually exclusive." ) - files = list(songs) + files, invalid_files = light.music.filter_valid_tracks(list(songs)) + for file_path in invalid_files: + console.print(f"[yellow]File not found, skipping: {file_path}[/yellow]") + matches = light.music.find_upload_matches(files) skip_count = len(matches) if matches and not overwrite and not allow_duplicates else 0 diff --git a/tests/test_api.py b/tests/test_api.py index 8a8d146..6cd86d5 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -9,7 +9,7 @@ from light_api.client import Light from light_api.notes import LightNote -from light_api.music import LightTrack +from light_api.music import LightMusic, LightTrack API = "https://production.lightphonecloud.com" @@ -523,6 +523,19 @@ def test_missing_file_never_reaches_matching_or_deletion(self): light.music.delete_tracks_predicate.assert_not_called() +class TestFilterValidTracks: + def test_splits_existing_and_missing_paths(self, tmp_path): + real_file = tmp_path / "song.mp3" + real_file.write_bytes(b"") + + valid, invalid = LightMusic.filter_valid_tracks( + [str(real_file), "/nonexistent/missing.mp3"] + ) + + assert valid == [str(real_file)] + assert invalid == ["/nonexistent/missing.mp3"] + + def make_note(overrides: dict | None = None) -> LightNote: data = dict(id="note-1", file_id="file-1", note_type="text", title="old title", updated_at="2026-01-01T00:00:00") if overrides: From b6a8870af04e07fd79d082f67fe6b92ccb5b43a1 Mon Sep 17 00:00:00 2001 From: garado Date: Sat, 1 Aug 2026 16:39:36 -0700 Subject: [PATCH 19/20] chore: remove unnecessary manual_upload_cmds --- light_api/light_api/music.py | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/light_api/light_api/music.py b/light_api/light_api/music.py index 5323ef8..f7fa4cb 100644 --- a/light_api/light_api/music.py +++ b/light_api/light_api/music.py @@ -369,8 +369,6 @@ def upload_tracks( if overwrite and allow_duplicates: raise ValueError("overwrite and allow_duplicates are mutually exclusive") - manual_update_cmds = [] - valid_files, invalid_files = self.filter_valid_tracks(files) for file_path in invalid_files: log.warning(f"File not found, skipping: {file_path}") @@ -445,31 +443,12 @@ def _chunks(path: str, total: int, filename: str): raise RuntimeError( f"Upload {os.path.basename(upload_path)}: {put_resp.status_code} {put_resp.text}" ) - - # (TODO Is this even necessary anymore now that we have FLAC autoconversion) - # The Light API has an issue where it won't set title/artist metadata properly - # when uploading non-mp3 files. Give user list of commands to patch manually after - # upload since files are still processing and a patch immediately after will fail. - if content_type != "audio/mpeg": - path = os.path.basename(upload_path) - tags = File(upload_path, easy=True) - title = tags.get("title", ["Unknown"])[0] if tags else "Unknown" - artist = tags.get("artist", ["Unknown"])[0] if tags else "Unknown" - album = tags.get("album", [""])[0] if tags else "" - cmd = f'light music update "{path}" --new-title "{title}" --new-artist "{artist}" --new-album "{album}"' - manual_update_cmds.append(cmd) finally: if tmp_path: os.unlink(tmp_path) log.info("All uploads complete") - if len(manual_update_cmds) > 0: - log.warning( - "Manual metadata fixes needed:\n" - + "\n".join(f" {cmd} ;" for cmd in manual_update_cmds) - ) - def update_track_metadata( self, audio_id: str, From 2f6a4f4e08f68613f576cb2fa5ab117c50daf771 Mon Sep 17 00:00:00 2001 From: garado Date: Sat, 1 Aug 2026 16:42:01 -0700 Subject: [PATCH 20/20] docs: minor docstring clarifications --- light_api/light_api/music.py | 2 +- light_cli_tui/light_cli_tui/cli.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/light_api/light_api/music.py b/light_api/light_api/music.py index f7fa4cb..83ef77f 100644 --- a/light_api/light_api/music.py +++ b/light_api/light_api/music.py @@ -315,7 +315,7 @@ def find_upload_matches(self, files: list[str]) -> dict[str, LightTrack]: def _resolve_upload_plan( self, files: list[str], allow_duplicates: bool, overwrite: bool ) -> tuple[list[str], list[LightTrack]]: - """Return the subset of files to upload and the subset of files to overwrite after + """Return the subset of files to upload and the subset of tracks to overwrite after applying allow_duplicates/overwrite behavior flags. Returns: diff --git a/light_cli_tui/light_cli_tui/cli.py b/light_cli_tui/light_cli_tui/cli.py index 755a61c..715c21d 100644 --- a/light_cli_tui/light_cli_tui/cli.py +++ b/light_cli_tui/light_cli_tui/cli.py @@ -242,7 +242,7 @@ def podcasts_delete(light: Light, title): @click.option( "--overwrite", is_flag=True, - help="Overwrite duplicate tracks.", + help="Overwrite existing matching tracks.", ) @click.option( "--no-convert-flac",