diff --git a/core/downloads/staging.py b/core/downloads/staging.py index f7ca8906..d1d4e3c3 100644 --- a/core/downloads/staging.py +++ b/core/downloads/staging.py @@ -38,8 +38,6 @@ import re from dataclasses import dataclass from typing import Any, Callable -from core.imports.filename import extract_track_number_from_filename - # `shutil` and `SequenceMatcher` are imported inline inside try_staging_match() # to keep the lift byte-identical with the original web_server.py function body. @@ -61,6 +59,25 @@ def _coerce_positive_int(value: Any, default: int = 0) -> int: return coerced if coerced > 0 else default +def _extract_explicit_track_number(filename: str) -> int: + """Extract a track number only when the filename visibly carries one.""" + basename = os.path.splitext(os.path.basename(str(filename or '')))[0].strip() + if not basename: + return 0 + + match = re.match(r"^\d[\-\.](\d{1,2})\s*[\-\.]\s*", basename) + if match: + num = int(match.group(1)) + return num if 1 <= num <= 99 else 0 + + match = re.match(r"^\(?(\d{1,3})\)?\s*[\-\.)\]]\s*", basename) + if match: + num = int(match.group(1)) + return num if 1 <= num <= 999 else 0 + + return 0 + + def _staging_title_variants(title: Any, normalize: Callable[[str], str]) -> list[str]: """Return conservative title variants for release-file matching. @@ -320,12 +337,24 @@ def try_staging_match(task_id, batch_id, track, deps: StagingDeps): file_track_number = ( _coerce_positive_int(best_match.get('track_number'), 0) or - extract_track_number_from_filename(best_match.get('full_path', '')) + _extract_explicit_track_number(best_match.get('full_path', '')) ) - file_disc_number = _coerce_positive_int(best_match.get('disc_number'), 1) + file_disc_number = _coerce_positive_int(best_match.get('disc_number'), 0) if _private_album_bundle_staging: - track_number = file_track_number - disc_number = file_disc_number + track_number = ( + file_track_number or + _coerce_positive_int(track_info.get('track_number'), 0) or + _coerce_positive_int(track_info.get('trackNumber'), 0) or + _coerce_positive_int(getattr(track, 'track_number', 0), 0) or + 1 + ) + disc_number = ( + file_disc_number or + _coerce_positive_int(track_info.get('disc_number'), 0) or + _coerce_positive_int(track_info.get('discNumber'), 0) or + _coerce_positive_int(getattr(track, 'disc_number', 0), 0) or + 1 + ) else: track_number = ( _coerce_positive_int(track_info.get('track_number'), 0) or @@ -337,7 +366,8 @@ def try_staging_match(task_id, batch_id, track, deps: StagingDeps): _coerce_positive_int(track_info.get('disc_number'), 0) or _coerce_positive_int(track_info.get('discNumber'), 0) or _coerce_positive_int(getattr(track, 'disc_number', 0), 0) or - file_disc_number + file_disc_number or + 1 ) track_info['track_number'] = track_number track_info['disc_number'] = disc_number diff --git a/core/downloads/status.py b/core/downloads/status.py index 6e11a763..7f2230bd 100644 --- a/core/downloads/status.py +++ b/core/downloads/status.py @@ -24,6 +24,7 @@ import logging import threading import time from dataclasses import dataclass +from datetime import datetime from typing import Any, Callable, Optional from core.runtime_state import ( @@ -82,6 +83,7 @@ class StatusDeps: download_orchestrator: Any = None run_async: Optional[Callable] = None on_download_completed: Optional[Callable[[str, str, bool], None]] = None + get_persistent_download_history: Optional[Callable[[int], list[dict]]] = None # Streaming sources the engine fallback applies to. Soulseek goes through @@ -536,6 +538,69 @@ _STATUS_PRIORITY = { 'not_found': 6, 'failed': 7, 'cancelled': 8, } +_PERSISTENT_HISTORY_TAIL_LIMIT = 50 + + +def _normalize_identity_part(value: Any) -> str: + return str(value or '').strip().casefold() + + +def _download_identity(title: Any, artist: Any, album: Any) -> tuple[str, str, str]: + return ( + _normalize_identity_part(title), + _normalize_identity_part(artist), + _normalize_identity_part(album), + ) + + +def _history_timestamp(value: Any) -> float: + if not value: + return 0.0 + if isinstance(value, (int, float)): + return float(value) + text = str(value).strip() + if not text: + return 0.0 + try: + # SQLite CURRENT_TIMESTAMP uses "YYYY-MM-DD HH:MM:SS". + return datetime.fromisoformat(text.replace('Z', '+00:00')).timestamp() + except ValueError: + try: + return datetime.strptime(text[:19], '%Y-%m-%d %H:%M:%S').timestamp() + except ValueError: + return 0.0 + + +def _build_history_download_item(entry: dict) -> dict: + history_id = entry.get('id') or entry.get('history_id') or '' + title = entry.get('title') or entry.get('source_track_title') or '' + artist = entry.get('artist_name') or entry.get('source_artist') or '' + album = entry.get('album_name') or '' + created_at = entry.get('created_at') or entry.get('completed_at') or '' + source = entry.get('download_source') or '' + return { + 'task_id': f'history-{history_id}' if history_id else f'history-{title}-{created_at}', + 'title': title, + 'artist': artist, + 'album': album, + 'artwork': entry.get('thumb_url') or '', + 'status': 'completed', + 'progress': 100, + 'error': None, + 'batch_id': '', + 'batch_name': source, + 'batch_source': source, + 'playlist_id': '', + 'track_index': 0, + 'batch_total': 1, + 'timestamp': _history_timestamp(created_at), + 'created_at': created_at, + 'priority': _STATUS_PRIORITY['completed'], + 'quality': entry.get('quality') or '', + 'file_path': entry.get('file_path') or '', + 'is_persistent_history': True, + } + def build_unified_downloads_response(limit: int, deps: StatusDeps) -> dict: """Flat list of every task across batches, sorted active-first then by recency. @@ -543,6 +608,7 @@ def build_unified_downloads_response(limit: int, deps: StatusDeps) -> dict: Powers /api/downloads/all for the centralized Downloads page. """ items = [] + live_identities = set() with tasks_lock: for task_id, task in download_tasks.items(): track_info = task.get('track_info') or {} @@ -589,6 +655,7 @@ def build_unified_downloads_response(limit: int, deps: StatusDeps) -> dict: artwork = images[0].get('url', '') if isinstance(images[0], dict) else str(images[0]) status = task.get('status', 'queued') + live_identities.add(_download_identity(title, artist, album)) # Determine download progress percentage progress = 0 if status == 'completed': @@ -625,8 +692,29 @@ def build_unified_downloads_response(limit: int, deps: StatusDeps) -> dict: 'batch_total': len(batch.get('queue', [])), 'timestamp': task.get('status_change_time', 0), 'priority': _STATUS_PRIORITY.get(status, 9), + 'is_persistent_history': False, }) + if deps.get_persistent_download_history is not None and len(items) < limit: + history_limit = min(limit - len(items), _PERSISTENT_HISTORY_TAIL_LIMIT) + try: + history_entries = deps.get_persistent_download_history(history_limit) or [] + except Exception as exc: + logger.debug("[Downloads] persistent history lookup failed: %s", exc) + history_entries = [] + + appended_history = 0 + for entry in history_entries: + if len(items) >= limit or appended_history >= history_limit: + break + item = _build_history_download_item(entry) + identity = _download_identity(item.get('title'), item.get('artist'), item.get('album')) + if identity in live_identities: + continue + items.append(item) + live_identities.add(identity) + appended_history += 1 + # Sort: active first (by priority), then by timestamp desc within each group items.sort(key=lambda x: (x['priority'], -x['timestamp'])) diff --git a/core/imports/album.py b/core/imports/album.py index db6c7d01..3da27119 100644 --- a/core/imports/album.py +++ b/core/imports/album.py @@ -354,6 +354,10 @@ def build_album_import_context( "images": album.get("images") or ([] if not track_album_image else [{"url": track_album_image}]), "source": source, } + for key in ("format", "country", "status", "label", "disambiguation", "release_group_id"): + value = str(album.get(key) or "").strip() + if value: + normalized_album[key] = value original_search = { "title": normalized_track["name"], diff --git a/core/imports/staging.py b/core/imports/staging.py index 5843d144..748cbb11 100644 --- a/core/imports/staging.py +++ b/core/imports/staging.py @@ -198,6 +198,12 @@ def _normalize_album_result(album: Any, source: str) -> Dict[str, Any]: ).strip() release_date = str(_extract_value(album, "release_date", "releaseDate", default="") or "").strip() album_type = str(_extract_value(album, "album_type", "type", default="album") or "album").strip() or "album" + release_format = str(_extract_value(album, "format", "release_format", default="") or "").strip() + country = str(_extract_value(album, "country", default="") or "").strip() + status = str(_extract_value(album, "status", default="") or "").strip() + label = str(_extract_value(album, "label", default="") or "").strip() + disambiguation = str(_extract_value(album, "disambiguation", default="") or "").strip() + release_group_id = str(_extract_value(album, "release_group_id", "releaseGroupId", default="") or "").strip() total_tracks = _extract_value(album, "total_tracks", "track_count", default=0) if isinstance(total_tracks, (list, tuple, set)): @@ -225,7 +231,7 @@ def _normalize_album_result(album: Any, source: str) -> Dict[str, Any]: else: image_url = _extract_value(first_image, "url", "image_url", "src", default="") - return { + suggestion = { "id": album_id or album_name or "unknown-album", "name": album_name or album_id or "Unknown Album", "artist": artist_name or "Unknown Artist", @@ -235,14 +241,30 @@ def _normalize_album_result(album: Any, source: str) -> Dict[str, Any]: "album_type": album_type, "source": source, } + if release_format: + suggestion["format"] = release_format + if country: + suggestion["country"] = country + if status: + suggestion["status"] = status + if label: + suggestion["label"] = label + if disambiguation: + suggestion["disambiguation"] = disambiguation + if release_group_id: + suggestion["release_group_id"] = release_group_id + return suggestion -def _album_fingerprint(album: Dict[str, Any]) -> Tuple[str, str, str, str]: +def _album_fingerprint(album: Dict[str, Any]) -> Tuple[str, ...]: + if album.get("source") == "musicbrainz" and album.get("id"): + return ("musicbrainz", str(album.get("id", "") or "").strip().casefold()) return ( str(album.get("name", "") or "").strip().casefold(), str(album.get("artist", "") or "").strip().casefold(), str(album.get("release_date", "") or "").strip()[:10].casefold(), str(album.get("album_type", "") or "").strip().casefold(), + str(album.get("total_tracks", "") or "").strip(), ) diff --git a/core/metadata/album_tracks.py b/core/metadata/album_tracks.py index 266134a8..e66be412 100644 --- a/core/metadata/album_tracks.py +++ b/core/metadata/album_tracks.py @@ -264,6 +264,11 @@ def _build_album_info_typed(album_data: Dict[str, Any], album_id: str, if isinstance(first, dict): ctx['image_url'] = first.get('url') or ctx.get('image_url') + for key in ('format', 'country', 'status', 'label', 'disambiguation', 'release_group_id'): + value = album_data.get(key) + if value: + ctx[key] = value + return ctx @@ -327,7 +332,7 @@ def _build_album_info_legacy(album_data: Any, album_id: str, if not image_url: image_url = _extract_lookup_value(album_data, 'image_url', 'thumb_url') - return { + album_info = { 'id': _extract_lookup_value(album_data, 'id', 'album_id', 'collectionId', 'release_id', default=album_id) or album_id, 'name': _extract_lookup_value(album_data, 'name', 'title', default=album_name or album_id) or album_name or album_id, 'artist': resolved_artist_name or '', @@ -345,6 +350,11 @@ def _build_album_info_legacy(album_data: Any, album_id: str, ), 'total_tracks': _extract_lookup_value(album_data, 'total_tracks', 'track_count', default=0) or 0, } + for key in ('format', 'country', 'status', 'label', 'disambiguation', 'release_group_id'): + value = _extract_lookup_value(album_data, key, default='') + if value: + album_info[key] = value + return album_info def _build_album_track_entry(track_item: Any, album_info: Dict[str, Any], source: str) -> Dict[str, Any]: diff --git a/core/musicbrainz_client.py b/core/musicbrainz_client.py index f1d59491..a802373d 100644 --- a/core/musicbrainz_client.py +++ b/core/musicbrainz_client.py @@ -298,6 +298,41 @@ class MusicBrainzClient: logger.error(f"Error browsing release-groups for artist {artist_mbid}: {e}") return [] + @rate_limited + def browse_release_group_releases(self, release_group_mbid: str, + limit: int = 100, + offset: int = 0) -> List[Dict[str, Any]]: + """Browse concrete releases that belong to a release-group. + + Release-groups identify the logical album; releases identify the + actual edition the user may own (country, format, explicit/clean + disambiguation, bonus tracks, track count). Manual import needs the + latter so users can choose the matching tracklist. + """ + try: + params = { + 'release-group': release_group_mbid, + 'fmt': 'json', + 'limit': min(limit, 100), + 'offset': offset, + 'inc': 'artist-credits+media+labels+release-groups', + } + + response = self.session.get( + f"{self.BASE_URL}/release", + params=params, + timeout=10 + ) + response.raise_for_status() + + data = response.json() + releases = data.get('releases', []) + logger.debug(f"Browsed {len(releases)} releases for release-group {release_group_mbid}") + return releases + except Exception as e: + logger.error(f"Error browsing releases for release-group {release_group_mbid}: {e}") + return [] + @rate_limited def search_recordings_by_artist_mbid(self, artist_mbid: str, limit: int = 100) -> List[Dict[str, Any]]: diff --git a/core/musicbrainz_search.py b/core/musicbrainz_search.py index f01292d4..47e95a84 100644 --- a/core/musicbrainz_search.py +++ b/core/musicbrainz_search.py @@ -57,6 +57,12 @@ class Album: album_type: str image_url: Optional[str] = None external_urls: Optional[Dict[str, str]] = None + format: Optional[str] = None + country: Optional[str] = None + status: Optional[str] = None + label: Optional[str] = None + disambiguation: Optional[str] = None + release_group_id: Optional[str] = None def _cover_art_url(mbid: str, scope: str = 'release') -> Optional[str]: @@ -316,8 +322,102 @@ class MusicBrainzSearchClient: album_type=album_type, image_url=image_url, external_urls={'musicbrainz': f'https://musicbrainz.org/release-group/{rg_mbid}'} if rg_mbid else {}, + disambiguation=rg.get('disambiguation') or None, + release_group_id=rg_mbid or None, ) + def _release_total_tracks(self, release: Dict[str, Any]) -> int: + total_tracks = 0 + for medium in release.get('media', []) or []: + try: + total_tracks += int(medium.get('track-count') or 0) + except (TypeError, ValueError): + pass + return total_tracks + + def _release_formats(self, release: Dict[str, Any]) -> str: + formats = [] + for medium in release.get('media', []) or []: + fmt = (medium.get('format') or '').strip() + if fmt and fmt not in formats: + formats.append(fmt) + return ', '.join(formats) + + def _release_label(self, release: Dict[str, Any]) -> str: + for info in release.get('label-info', []) or []: + label = (info.get('label') or {}) if isinstance(info, dict) else {} + name = (label.get('name') or '').strip() + if name: + return name + return '' + + def _release_to_album(self, release: Dict[str, Any], + fallback_artist_name: Optional[str] = None) -> Optional[Album]: + """Project a concrete MusicBrainz release into our Album dataclass.""" + mbid = release.get('id', '') + title = release.get('title', '') or '' + if not title: + return None + + artists = _extract_artist_credit(release.get('artist-credit', [])) + if not artists and fallback_artist_name: + artists = [fallback_artist_name] + + rg = release.get('release-group', {}) or {} + primary_type = rg.get('primary-type', '') or '' + secondary_types = rg.get('secondary-types', []) or [] + album_type = _map_release_type(primary_type, secondary_types) + rg_mbid = rg.get('id', '') or release.get('release-group-id', '') + image_url = self._cached_art(mbid, rg_mbid) + + return Album( + id=mbid, + name=title, + artists=artists if artists else ['Unknown Artist'], + release_date=release.get('date', '') or '', + total_tracks=self._release_total_tracks(release), + album_type=album_type, + image_url=image_url, + external_urls={'musicbrainz': f'https://musicbrainz.org/release/{mbid}'} if mbid else {}, + format=self._release_formats(release) or None, + country=(release.get('country') or '').strip() or None, + status=(release.get('status') or '').strip() or None, + label=self._release_label(release) or None, + disambiguation=(release.get('disambiguation') or '').strip() or None, + release_group_id=rg_mbid or None, + ) + + def _release_variant_key(self, album: Album): + status_rank = 0 if (album.status or '').lower() == 'official' else 1 + date = (album.release_date or '9999-99-99')[:10] or '9999-99-99' + track_rank = album.total_tracks or 9999 + country_rank = 0 if (album.country or '') in ('XW', 'US', 'GB') else 1 + return ( + status_rank, + date, + country_rank, + track_rank, + album.format or '', + album.disambiguation or '', + album.id, + ) + + def _release_group_releases_to_albums(self, rg: Dict[str, Any], artist_name: str, + limit: int) -> List[Album]: + rg_mbid = rg.get('id', '') + if not rg_mbid: + return [] + + releases = self._client.browse_release_group_releases(rg_mbid, limit=max(limit, 25)) + albums = [] + for release in releases: + release.setdefault('release-group', rg) + album = self._release_to_album(release, fallback_artist_name=artist_name) + if album: + albums.append(album) + albums.sort(key=self._release_variant_key) + return albums[:limit] + def search_albums(self, query: str, limit: int = 10) -> List[Album]: """Search MusicBrainz for releases (albums). @@ -400,6 +500,13 @@ class MusicBrainzSearchClient: matched = [rg for rg in rgs if hint_lower in (rg.get('title') or '').lower()] if matched: rgs = matched + expanded = [] + for rg in rgs: + expanded.extend(self._release_group_releases_to_albums(rg, tname, limit)) + if len(expanded) >= limit: + break + if expanded: + return expanded[:limit] else: fallback = self._search_albums_text(title_hint, tname, limit) if fallback: @@ -436,63 +543,24 @@ class MusicBrainzSearchClient: albums = [] for r in results: - mbid = r.get('id', '') - title = r.get('title', '') - if not title: - continue + album = self._release_to_album(r) + if album: + albums.append(album) - artists = _extract_artist_credit(r.get('artist-credit', [])) - release_date = r.get('date', '') or '' - - # Track count from media - total_tracks = 0 - media = r.get('media', []) - for m in media: - total_tracks += m.get('track-count', 0) - - # Release type - rg = r.get('release-group', {}) - primary_type = rg.get('primary-type', '') or '' - secondary_types = rg.get('secondary-types', []) or [] - album_type = _map_release_type(primary_type, secondary_types) - - # Cover art (non-blocking — skip if slow) - rg_mbid = rg.get('id', '') - image_url = self._cached_art(mbid, rg_mbid) - - external_urls = {'musicbrainz': f'https://musicbrainz.org/release/{mbid}'} if mbid else {} - - albums.append(Album( - id=mbid, - name=title, - artists=artists if artists else ['Unknown Artist'], - release_date=release_date, - total_tracks=total_tracks, - album_type=album_type, - image_url=image_url, - external_urls=external_urls, - )) - # Deduplicate: keep best version of each title+artist combo - # (prefer ones with release dates and cover art) - seen = {} - deduped = [] + # Keep distinct MusicBrainz releases. The same title/artist/date + # can represent explicit, clean, regional, format, or bonus-track + # variants with different tracklists, which manual import must let + # the user choose. + seen_ids = set() + unique = [] for album in albums: - key = (album.name.lower().strip(), ', '.join(album.artists).lower().strip()) - if key not in seen: - seen[key] = album - deduped.append(album) - else: - existing = seen[key] - # Prefer: has date > no date, has art > no art - better = False - if not existing.release_date and album.release_date: - better = True - elif not existing.image_url and album.image_url: - better = True - if better: - deduped[deduped.index(existing)] = album - seen[key] = album - return deduped + if album.id and album.id in seen_ids: + continue + if album.id: + seen_ids.add(album.id) + unique.append(album) + unique.sort(key=self._release_variant_key) + return unique[:limit] except Exception as e: logger.warning(f"MusicBrainz album search failed: {e}") return [] @@ -1030,6 +1098,12 @@ class MusicBrainzSearchClient: 'images': images, 'tracks': tracks, 'external_urls': {'musicbrainz': f'https://musicbrainz.org/release/{release_mbid}'}, + 'format': self._release_formats(release), + 'country': release.get('country') or '', + 'status': release.get('status') or '', + 'label': self._release_label(release), + 'disambiguation': release.get('disambiguation') or '', + 'release_group_id': rg_mbid, } def get_artist_albums(self, artist_mbid: str, album_type: str = 'album,single', limit: int = 200) -> List: diff --git a/core/search/sources.py b/core/search/sources.py index e66b9eda..006bf8f0 100644 --- a/core/search/sources.py +++ b/core/search/sources.py @@ -56,6 +56,12 @@ def search_kind(client, query: str, kind: str, source_name: Optional[str] = None "release_date": album.release_date, "total_tracks": album.total_tracks, "album_type": album.album_type, + "format": getattr(album, "format", None), + "country": getattr(album, "country", None), + "status": getattr(album, "status", None), + "label": getattr(album, "label", None), + "disambiguation": getattr(album, "disambiguation", None), + "release_group_id": getattr(album, "release_group_id", None), "external_urls": album.external_urls or {}, }) except Exception as e: diff --git a/core/youtube_client.py b/core/youtube_client.py index df910e59..bd72e674 100644 --- a/core/youtube_client.py +++ b/core/youtube_client.py @@ -154,6 +154,7 @@ class YouTubeClient(DownloadSourcePlugin): # Initialize production matching engine for parity with Soulseek self.matching_engine = MusicMatchingEngine() + logger.info("Initialized production MusicMatchingEngine") # NOTE: deliberately don't call `_check_ffmpeg()` here. That call @@ -216,6 +217,23 @@ class YouTubeClient(DownloadSourcePlugin): # Optional progress callback for UI updates self.progress_callback = None + @staticmethod + def _escape_ytsearch_query(query: str) -> str: + """Escape yt-dlp search terms that begin with a dash. + + YouTube video IDs may start with ``-``. When passed through + ``ytsearchN:``, yt-dlp treats that leading dash as search + syntax unless it is escaped. Preserve already-escaped input so + users who worked around the issue manually keep the same result. + """ + if not isinstance(query, str): + return query + stripped = query.lstrip() + leading_ws_len = len(query) - len(stripped) + if stripped.startswith('-'): + return f"{query[:leading_ws_len]}\\{stripped}" + return query + def is_available(self) -> bool: """ Check if YouTube client is available (yt-dlp installed and ffmpeg available). @@ -698,8 +716,9 @@ class YouTubeClient(DownloadSourcePlugin): if cookies_browser: ydl_opts['cookiesfrombrowser'] = (cookies_browser,) + search_query = self._escape_ytsearch_query(query) with yt_dlp.YoutubeDL(ydl_opts) as ydl: - data = ydl.extract_info(f"ytsearch{max_results}:{query}", download=False) + data = ydl.extract_info(f"ytsearch{max_results}:{search_query}", download=False) if not data or 'entries' not in data: return [] @@ -777,9 +796,10 @@ class YouTubeClient(DownloadSourcePlugin): if cookies_browser: ydl_opts['cookiesfrombrowser'] = (cookies_browser,) + search_query = self._escape_ytsearch_query(query) with yt_dlp.YoutubeDL(ydl_opts) as ydl: # Search YouTube (max 50 results) - search_results = ydl.extract_info(f"ytsearch50:{query}", download=False) + search_results = ydl.extract_info(f"ytsearch50:{search_query}", download=False) if not search_results or 'entries' not in search_results: return [] diff --git a/tests/downloads/test_downloads_staging.py b/tests/downloads/test_downloads_staging.py index 70e86b6b..097c3323 100644 --- a/tests/downloads/test_downloads_staging.py +++ b/tests/downloads/test_downloads_staging.py @@ -357,6 +357,49 @@ def test_private_album_bundle_staging_overrides_default_track_info_number(tmp_pa assert ctx['original_search_result']['filename'] == str(src_file) +def test_private_album_bundle_staging_keeps_task_number_when_file_has_no_number(tmp_path): + """Private release staging must not turn every unnumbered release file into track 1.""" + src_file = tmp_path / 'staging' / 'Katy Perry - Firework.flac' + src_file.parent.mkdir() + src_file.touch() + + def get_batch_field(_batch_id, field): + if field == 'album_bundle_source': + return 'soulseek' + if field == 'album_bundle_private_staging': + return True + return None + + deps = _build_deps( + transfer_path=str(tmp_path / 'transfer'), + staging_files=[ + { + 'full_path': str(src_file), + 'title': 'Firework', + 'artist': 'Katy Perry', + }, + ], + get_batch_field=get_batch_field, + ) + _seed_task('t6d', track_info={ + '_is_explicit_album_download': True, + '_explicit_album_context': {'id': 'alb', 'name': 'Teenage Dream: The Complete Confection'}, + '_explicit_artist_context': {'id': 'art', 'name': 'Katy Perry'}, + 'track_number': 4, + 'disc_number': 1, + }) + + ds.try_staging_match( + 't6d', 'b1', + _Track(name='Firework', artists=['Katy Perry']), + deps, + ) + + ctx = matched_downloads_context['staging_t6d'] + assert ctx['track_info']['track_number'] == 4 + assert ctx['original_search_result']['track_number'] == 4 + + def test_staging_title_match_accepts_feature_suffix_from_release_file(tmp_path): """Album releases can include featured artists in filenames.""" src_file = tmp_path / 'staging' / '05-kendrick_lamar-money_trees_(feat._jay_rock).flac' diff --git a/tests/downloads/test_downloads_status.py b/tests/downloads/test_downloads_status.py index 706f9131..64d41b3d 100644 --- a/tests/downloads/test_downloads_status.py +++ b/tests/downloads/test_downloads_status.py @@ -42,6 +42,7 @@ def _build_deps( cached_transfers=None, download_orchestrator=None, run_async=None, + persistent_history=None, ): submitted = [] @@ -57,6 +58,7 @@ def _build_deps( get_cached_transfer_data=cached_transfers or (lambda: {}), download_orchestrator=download_orchestrator, run_async=run_async, + get_persistent_download_history=persistent_history, ) return deps, submitted @@ -655,3 +657,88 @@ def test_unified_response_respects_limit(): out = st.build_unified_downloads_response(5, deps) assert len(out['downloads']) == 5 assert out['total'] == 20 # total still reflects all + + +def test_unified_response_includes_persistent_download_history(): + deps, _ = _build_deps( + persistent_history=lambda limit: [ + { + 'id': 42, + 'title': 'Persisted Track', + 'artist_name': 'Deezer Artist', + 'album_name': 'Persistent Album', + 'thumb_url': 'http://cover.jpg', + 'download_source': 'Deezer', + 'quality': 'FLAC', + 'created_at': '2026-05-24 12:34:56', + } + ] + ) + + out = st.build_unified_downloads_response(100, deps) + + assert out['total'] == 1 + item = out['downloads'][0] + assert item['task_id'] == 'history-42' + assert item['title'] == 'Persisted Track' + assert item['artist'] == 'Deezer Artist' + assert item['album'] == 'Persistent Album' + assert item['artwork'] == 'http://cover.jpg' + assert item['status'] == 'completed' + assert item['progress'] == 100 + assert item['batch_name'] == 'Deezer' + assert item['is_persistent_history'] is True + + +def test_unified_response_dedupes_history_against_live_task(): + download_tasks['live'] = { + 'track_index': 0, + 'status': 'completed', + 'track_info': { + 'name': 'Same Track', + 'artist': 'Same Artist', + 'album': 'Same Album', + }, + } + deps, _ = _build_deps( + persistent_history=lambda limit: [ + { + 'id': 7, + 'title': 'Same Track', + 'artist_name': 'Same Artist', + 'album_name': 'Same Album', + 'created_at': '2026-05-24 12:34:56', + } + ] + ) + + out = st.build_unified_downloads_response(100, deps) + + assert len(out['downloads']) == 1 + assert out['downloads'][0]['task_id'] == 'live' + assert out['downloads'][0]['is_persistent_history'] is False + + +def test_unified_response_caps_persistent_history_tail(): + requested_limits = [] + + def _history(limit): + requested_limits.append(limit) + return [ + { + 'id': i, + 'title': f'Track {i}', + 'artist_name': 'Artist', + 'album_name': 'Album', + 'created_at': '2026-05-24 12:34:56', + } + for i in range(limit + 10) + ] + + deps, _ = _build_deps(persistent_history=_history) + + out = st.build_unified_downloads_response(300, deps) + + assert requested_limits == [50] + assert len(out['downloads']) == 50 + assert out['total'] == 50 diff --git a/tests/imports/test_import_staging.py b/tests/imports/test_import_staging.py index 56571ede..2405a1f3 100644 --- a/tests/imports/test_import_staging.py +++ b/tests/imports/test_import_staging.py @@ -105,6 +105,55 @@ def test_search_import_albums_falls_back_when_primary_has_no_results(monkeypatch assert spotify_client.calls == [("Album Two", {"limit": 2, "allow_fallback": False})] +def test_search_import_albums_preserves_musicbrainz_release_variants(monkeypatch): + musicbrainz_client = FakeClient([ + SimpleNamespace( + id="rel-clean", + name="Shock Value", + artists=["Timbaland"], + release_date="2007-04-03", + total_tracks=17, + image_url="", + album_type="album", + format="CD", + country="US", + status="Official", + disambiguation="clean", + release_group_id="rg-shock", + ), + SimpleNamespace( + id="rel-explicit", + name="Shock Value", + artists=["Timbaland"], + release_date="2007-04-03", + total_tracks=18, + image_url="", + album_type="album", + format="CD", + country="US", + status="Official", + disambiguation="explicit", + release_group_id="rg-shock", + ), + ]) + + monkeypatch.setattr(import_staging, "get_primary_source", lambda: "musicbrainz") + monkeypatch.setattr(import_staging, "get_source_priority", lambda primary: [primary]) + monkeypatch.setattr(import_staging, "get_client_for_source", lambda source: musicbrainz_client) + monkeypatch.setattr( + import_staging, + "_search_albums_for_source", + lambda source, client, query, limit=5: client.search_albums(query, limit=limit), + ) + + results = import_staging.search_import_albums("Timbaland Shock Value", limit=12) + + assert [result["id"] for result in results] == ["rel-clean", "rel-explicit"] + assert [result["total_tracks"] for result in results] == [17, 18] + assert results[1]["disambiguation"] == "explicit" + assert results[1]["release_group_id"] == "rg-shock" + + def test_search_import_tracks_prefers_primary_source(monkeypatch): deezer_client = FakeClient([ SimpleNamespace( diff --git a/tests/metadata/test_musicbrainz_search.py b/tests/metadata/test_musicbrainz_search.py index cec77648..e5859707 100644 --- a/tests/metadata/test_musicbrainz_search.py +++ b/tests/metadata/test_musicbrainz_search.py @@ -429,6 +429,58 @@ def test_search_albums_text_path_filters_by_score(): assert 'Bad' not in titles +def test_search_albums_text_path_keeps_release_variants(): + client = MusicBrainzSearchClient() + client._client = MagicMock() + client._client.search_release.return_value = [ + {'id': 'rel-clean', 'title': 'Shock Value', 'score': 100, + 'date': '2007-04-03', 'country': 'US', 'status': 'Official', + 'disambiguation': 'clean', + 'media': [{'format': 'CD', 'track-count': 17}], + 'release-group': {'id': 'rg-shock', 'primary-type': 'Album'}, + 'artist-credit': [{'name': 'Timbaland'}]}, + {'id': 'rel-explicit', 'title': 'Shock Value', 'score': 100, + 'date': '2007-04-03', 'country': 'US', 'status': 'Official', + 'disambiguation': 'explicit', + 'media': [{'format': 'CD', 'track-count': 18}], + 'release-group': {'id': 'rg-shock', 'primary-type': 'Album'}, + 'artist-credit': [{'name': 'Timbaland'}]}, + ] + + albums = client.search_albums('Timbaland - Shock Value', limit=10) + + assert [a.id for a in albums] == ['rel-clean', 'rel-explicit'] + assert [a.total_tracks for a in albums] == [17, 18] + assert albums[1].disambiguation == 'explicit' + + +def test_search_albums_title_hint_expands_release_group_to_releases(): + client = MusicBrainzSearchClient() + client._client = MagicMock() + client._client.search_artist.return_value = [_mk_artist('Spiderbait', 'artist-spiderbait', score=100)] + client._client.browse_artist_release_groups.return_value = [ + {'id': 'rg-tonight', 'title': 'Tonight Alright', 'primary-type': 'Album', + 'first-release-date': '2004-03-29', 'secondary-types': []}, + ] + client._client.browse_release_group_releases.return_value = [ + {'id': 'rel-cd', 'title': 'Tonight Alright', 'date': '2004-03-29', + 'country': 'AU', 'status': 'Official', + 'media': [{'format': 'CD', 'track-count': 12}], + 'artist-credit': [{'name': 'Spiderbait'}]}, + {'id': 'rel-vinyl', 'title': 'Tonight Alright', 'date': '2024-07-26', + 'country': 'AU', 'status': 'Official', + 'media': [{'format': '12\" Vinyl', 'track-count': 13}], + 'artist-credit': [{'name': 'Spiderbait'}]}, + ] + + albums = client.search_albums('Spiderbait Tonight Alright', limit=10) + + client._client.browse_release_group_releases.assert_called_once_with('rg-tonight', limit=25) + assert [a.id for a in albums] == ['rel-cd', 'rel-vinyl'] + assert [a.total_tracks for a in albums] == [12, 13] + assert albums[0].format == 'CD' + + # --------------------------------------------------------------------------- # Track search — routing # --------------------------------------------------------------------------- diff --git a/tests/search/test_search_sources.py b/tests/search/test_search_sources.py index 48a93620..f6949027 100644 --- a/tests/search/test_search_sources.py +++ b/tests/search/test_search_sources.py @@ -19,7 +19,9 @@ class _Artist: class _Album: def __init__(self, id_, name, artists=None, image_url=None, release_date=None, - total_tracks=10, album_type='album', external_urls=None): + total_tracks=10, album_type='album', external_urls=None, format=None, + country=None, status=None, label=None, disambiguation=None, + release_group_id=None): self.id = id_ self.name = name self.artists = artists or [] @@ -28,6 +30,12 @@ class _Album: self.total_tracks = total_tracks self.album_type = album_type self.external_urls = external_urls + self.format = format + self.country = country + self.status = status + self.label = label + self.disambiguation = disambiguation + self.release_group_id = release_group_id class _Track: @@ -99,6 +107,27 @@ def test_search_kind_albums_handles_no_artists(): assert result[0]['artist'] == 'Unknown Artist' +def test_search_kind_albums_passthrough_release_metadata(): + client = _Client(albums=[_Album( + 'a1', + 'Variant', + artists=['Artist'], + format='CD', + country='US', + status='Official', + label='Fixture Records', + disambiguation='clean', + release_group_id='rg-1', + )]) + result = sources.search_kind(client, 'v', 'albums') + assert result[0]['format'] == 'CD' + assert result[0]['country'] == 'US' + assert result[0]['status'] == 'Official' + assert result[0]['label'] == 'Fixture Records' + assert result[0]['disambiguation'] == 'clean' + assert result[0]['release_group_id'] == 'rg-1' + + def test_search_kind_tracks_returns_full_shape(): client = _Client(tracks=[_Track('t1', 'Money', artists=['Pink Floyd'], album='DSOTM', duration_ms=383000, image_url='m.jpg', diff --git a/tests/test_import_page_album_lookup_pattern.py b/tests/test_import_page_album_lookup_pattern.py deleted file mode 100644 index 5bf4ab9d..00000000 --- a/tests/test_import_page_album_lookup_pattern.py +++ /dev/null @@ -1,132 +0,0 @@ -"""Pin the import-page album-lookup cache pattern in -``webui/static/stats-automations.js`` — github issue #524 regression -guard at the source-text level. - -Why a structural test instead of a behavioral JS test: - -``stats-automations.js`` is a ~7k-line file with a lot of global state -+ inline DOM rendering. Loading it into a sandboxed Node `vm` context -(the pattern used in `tests/static/test_discover_section_controller.mjs`) -would require stubbing dozens of unrelated dependencies. The file -needs to be modularized before behavioral tests are practical for -arbitrary functions in it. - -Until then, this test fails the suite if the critical pattern from -the #524 fix gets removed: - -1. The album cache (``_albumLookup`` field on ``importPageState``) -2. Card renderers populating the cache before emitting the onclick -3. The match-POST builder reading source/name/artist from the cache - -If anyone deletes the cache, the click handler, or the cache writes, -this test catches it before the regression ships. -""" - -from __future__ import annotations - -import re -from pathlib import Path - -import pytest - - -_REPO_ROOT = Path(__file__).resolve().parents[1] -_SOURCE = _REPO_ROOT / "webui" / "static" / "stats-automations.js" - - -@pytest.fixture(scope="module") -def js_source() -> str: - return _SOURCE.read_text(encoding="utf-8") - - -def test_album_lookup_cache_field_exists_on_state(js_source: str): - """importPageState must have an `_albumLookup` field. Without it, - card renderers have nowhere to stash source/name/artist for the - click handler to read.""" - assert "_albumLookup:" in js_source, ( - "importPageState._albumLookup field missing — the album cache " - "that backs the source-routing fix for issue #524 has been " - "removed. The click handler will fall back to passing only " - "album_id and the backend will silently misroute lookups again." - ) - - -def test_select_album_handler_reads_cache(js_source: str): - """importPageSelectAlbum must read source / name / artist from - the cache and include them in the match POST body. The whole - point of the fix.""" - # Find the function body - match = re.search( - r"async function importPageSelectAlbum\([^)]*\) \{(.*?)^\}", - js_source, re.DOTALL | re.MULTILINE, - ) - assert match, "importPageSelectAlbum function not found" - body = match.group(1) - - # Must read from the lookup cache - assert "_albumLookup[" in body, ( - "importPageSelectAlbum no longer reads from " - "importPageState._albumLookup — match POST will drop source " - "again, see issue #524." - ) - - # Must build a matchBody that includes source + album_name + album_artist - for required_field in ("source:", "album_name:", "album_artist:"): - assert required_field in body, ( - f"matchBody missing required field {required_field!r}. " - "Backend's get_artist_album_tracks needs source to route " - "the lookup to the correct metadata client. Without it, " - "cross-source album_ids fall through to the failure-fallback " - "dict (Unknown Artist / album_id-as-title / 0 tracks). " - "See issue #524 for the original symptom." - ) - - -def test_card_renderer_populates_cache_before_onclick(js_source: str): - """The shared card-renderer ``_renderSuggestionCard`` must write to - ``_albumLookup`` before emitting the onclick — otherwise the click - handler reads an empty cache for newly-displayed albums. - - Originally this test required >=2 cache writes (one per inline - renderer), but the search-results inline render was consolidated - into a single ``_renderSuggestionCard`` call as part of the #681 - fix. The invariant now is: the shared renderer populates the cache, - and every render call site goes through it (no inline duplicates).""" - # 1. The shared renderer must contain the cache write. - match = re.search( - r"function _renderSuggestionCard\([^)]*\) \{(.*?)^\}", - js_source, re.DOTALL | re.MULTILINE, - ) - assert match, "_renderSuggestionCard function not found" - body = match.group(1) - assert re.search(r"_albumLookup\[a\.id\]\s*=\s*\{", body), ( - "_renderSuggestionCard no longer writes to _albumLookup before " - "emitting the onclick — every card rendered through this helper " - "would have an empty cache on click, regressing issue #524." - ) - - # 2. No inline card render allowed outside the shared helper. - # A second `_albumLookup[a.id] = {` write means a caller is - # re-implementing the renderer instead of calling the helper — - # that's exactly the duplication the #524 fix consolidated away. - cache_writes = re.findall(r"_albumLookup\[a\.id\]\s*=\s*\{", js_source) - assert len(cache_writes) == 1, ( - f"Expected exactly 1 _albumLookup write (inside _renderSuggestionCard), " - f"found {len(cache_writes)}. A new inline card-render site has " - "duplicated the cache-write logic — route the new caller through " - "_renderSuggestionCard(a, primarySource) instead." - ) - - -def test_cache_entry_carries_source_field(js_source: str): - """The cache must store `source:` per entry — not just id/name/artist.""" - write_blocks = re.findall( - r"_albumLookup\[a\.id\]\s*=\s*\{[^}]*\}", - js_source, - ) - assert write_blocks, "no _albumLookup writes found" - assert any("source:" in block for block in write_blocks), ( - "_albumLookup cache entries must include `source` — that's the " - "field the click handler forwards to /api/import/album/match " - "to route the lookup to the correct provider." - ) diff --git a/tests/test_youtube_search_dash_query.py b/tests/test_youtube_search_dash_query.py new file mode 100644 index 00000000..a2e0318d --- /dev/null +++ b/tests/test_youtube_search_dash_query.py @@ -0,0 +1,87 @@ +"""Regression tests for YouTube searches whose query starts with ``-``. + +YouTube video IDs can start with a dash. yt-dlp's ``ytsearchN:`` parser +interprets a leading dash as search syntax unless escaped, so manual +searches for those IDs used to fan out into unrelated results. +""" + +from __future__ import annotations + +import asyncio +from types import SimpleNamespace + +from core import youtube_client +from core.youtube_client import YouTubeClient + + +def test_escape_ytsearch_query_handles_leading_dash(): + assert YouTubeClient._escape_ytsearch_query("-4WUHJRhvrM") == r"\-4WUHJRhvrM" + assert YouTubeClient._escape_ytsearch_query(r"\-4WUHJRhvrM") == r"\-4WUHJRhvrM" + assert YouTubeClient._escape_ytsearch_query("Yo-Yo Ma") == "Yo-Yo Ma" + + +def test_search_escapes_leading_dash_before_yt_dlp(monkeypatch): + captured = [] + + class _FakeYoutubeDL: + def __init__(self, opts): + self.opts = opts + + def __enter__(self): + return self + + def __exit__(self, exc_type, exc, tb): + return False + + def extract_info(self, search_query, download=False): + captured.append(search_query) + return {"entries": [{"id": "-4WUHJRhvrM", "title": "Unaccompanied Cello"}]} + + monkeypatch.setattr(youtube_client.yt_dlp, "YoutubeDL", _FakeYoutubeDL) + + client = YouTubeClient.__new__(YouTubeClient) + monkeypatch.setattr(client, "_get_best_audio_format", lambda formats: None) + monkeypatch.setattr( + client, + "_youtube_to_track_result", + lambda entry, best_audio: SimpleNamespace(filename=entry["title"]), + ) + + tracks, albums = asyncio.run(client.search("-4WUHJRhvrM")) + + assert captured == [r"ytsearch50:\-4WUHJRhvrM"] + assert len(tracks) == 1 + assert albums == [] + + +def test_search_videos_escapes_leading_dash_before_yt_dlp(monkeypatch): + captured = [] + + class _FakeYoutubeDL: + def __init__(self, opts): + self.opts = opts + + def __enter__(self): + return self + + def __exit__(self, exc_type, exc, tb): + return False + + def extract_info(self, search_query, download=False): + captured.append(search_query) + return { + "entries": [{ + "id": "-4WUHJRhvrM", + "title": "Unaccompanied Cello", + "duration": 152, + "uploader": "Yo-Yo Ma", + }] + } + + monkeypatch.setattr(youtube_client.yt_dlp, "YoutubeDL", _FakeYoutubeDL) + client = YouTubeClient.__new__(YouTubeClient) + + results = asyncio.run(client.search_videos("-4WUHJRhvrM", max_results=8)) + + assert captured == [r"ytsearch8:\-4WUHJRhvrM"] + assert [r.video_id for r in results] == ["-4WUHJRhvrM"] diff --git a/web_server.py b/web_server.py index f3cad76f..de2344c9 100644 --- a/web_server.py +++ b/web_server.py @@ -1068,6 +1068,13 @@ def _get_batch_max_concurrent(is_album=False, source=None): mode = config_manager.get('download_source.mode', 'soulseek') if mode == 'soulseek': return 1 + if mode == 'hybrid': + hybrid_order = config_manager.get('download_source.hybrid_order', []) or [] + if isinstance(hybrid_order, str): + hybrid_order = [hybrid_order] + first_source = next((str(s).strip().lower() for s in hybrid_order if str(s).strip()), '') + if first_source == 'soulseek': + return 1 return _get_max_concurrent() # --- Session Download Statistics --- @@ -2557,9 +2564,16 @@ def _build_system_stats(): active_downloads = len([batch_id for batch_id, batch_data in download_batches.items() if batch_data.get('phase') == 'downloading']) - # Count finished downloads (completed this session) - use session counter like dashboard.py - with session_stats_lock: - finished_downloads = session_completed_downloads + # Count finished downloads from persistent history so the dashboard + # survives Docker/container restarts and streaming downloads that leave + # the in-memory task tracker after post-processing. + try: + persistent_finished = int(get_database().get_library_history_stats().get('downloads', 0) or 0) + with session_stats_lock: + finished_downloads = max(persistent_finished, session_completed_downloads) + except Exception: + with session_stats_lock: + finished_downloads = session_completed_downloads # Calculate total download speed from active soulseek transfers # Skip the slskd API call entirely when Soulseek is not the active download @@ -16524,6 +16538,8 @@ def _get_staging_file_cache(batch_id): 'title': meta['title'] or '', 'artist': meta['albumartist'] or meta['artist'] or '', 'album': meta['album'] or '', + 'track_number': meta.get('track_number'), + 'disc_number': meta.get('disc_number'), 'extension': ext, }) @@ -16928,6 +16944,11 @@ def _build_status_deps(): download_orchestrator=download_orchestrator, run_async=run_async, on_download_completed=_on_download_completed, + get_persistent_download_history=lambda limit: get_database().get_library_history( + event_type='download', + page=1, + limit=limit, + )[0], ) diff --git a/webui/docs/migration/README.md b/webui/docs/migration/README.md index a7a9d5e3..f54442a1 100644 --- a/webui/docs/migration/README.md +++ b/webui/docs/migration/README.md @@ -16,6 +16,9 @@ This folder is the home for React migration planning work inside `webui`. - cross-route risk assessment - [stats-migration-plan.md](./stats-migration-plan.md) - route-specific migration plan for `stats` +- [import-migration-plan.md](./import-migration-plan.md) + - route-specific migration plan for `import` + - implementation status and follow-up cleanup notes ## Naming Guidance diff --git a/webui/docs/migration/import-migration-plan.md b/webui/docs/migration/import-migration-plan.md new file mode 100644 index 00000000..1ef64f72 --- /dev/null +++ b/webui/docs/migration/import-migration-plan.md @@ -0,0 +1,350 @@ +# WebUI Import Migration Plan + +Snapshot date: 2026-05-24 + +## Status + +- Initial implementation completed on 2026-05-15. +- `import` is now React-owned in the shell route manifest. +- The legacy import page DOM has been removed from `webui/index.html`. +- Legacy import activation has been removed from `webui/static/init.js`. +- A React route subtree now owns import rendering, nested route state, album matching, singles matching, auto-import controls, and the client-side processing queue. +- The old `tab=` URL contract has been replaced by `/import/album`, `/import/singles`, and `/import/auto`, with `/import` redirecting to `/import/album`. +- Route-local workflow state lives in `webui/src/routes/import/-import.store.ts`, which keeps draft matching, selection, and queue state alive while navigating within the import route. +- The old import-page-specific functions have already been removed from `webui/static/stats-automations.js`; any remaining `import` references there belong to the broader automation feature set, not this page migration. +- Backend routes are already grouped around `/api/import/*` and `/api/auto-import/*`. + +## Goal + +- Migrate `import` into a React-owned route without changing the user workflow. +- Preserve manual album matching, singles matching, auto-import review, and the processing queue. +- Keep staging-folder and import-processing behavior backed by the existing API routes. +- Use the completed `issues` and `stats` routes as the structural reference for route slices, API helpers, shell gating, and tests. + +## Why `import` Was The Right Next Route + +- It is the safest remaining route after excluding `help` and `hydrabase`. +- It has real workflows, so it gives the migration program more signal than another mostly-static page. +- The backend API boundary is already clearer than the broad dashboard, library, discover, sync, or settings surfaces. +- The page is important enough to validate mutation, polling, and route-local reducer patterns before larger operational pages. +- It does not need a visual redesign or a new shell abstraction to migrate cleanly. + +## Current Legacy Shape + +Page surface in `webui/index.html`: + +- Header + - import folder path + - file count and total size + - refresh action +- Processing queue + - per-job progress + - partial error display + - clear-finished action +- Tabs + - auto-import + - albums + - singles +- Auto-import tab + - enable toggle + - status text + - confidence and interval settings + - scan-now action + - live scan progress + - result filters + - approve, reject, approve-all, and clear-completed actions +- Albums tab + - auto-group suggestions from staging + - album search + - album result cards + - track matching view + - drag/drop and tap-to-assign overrides + - unmatched file pool + - process album action +- Singles tab + - staging file list + - select all / per-file selection + - per-file track search + - manual match selection + - process selected action + +Legacy JS responsibilities in `webui/static/stats-automations.js`: + +- `initializeImportPage` +- staging fetch and refresh +- tab switching +- auto-import polling +- auto-import mutations +- staging group and suggestion rendering +- album search and match POSTs +- drag/drop assignment state +- singles search and selection state +- client-side processing queue +- sequential album/singles processing requests + +Backend endpoints already available: + +- `GET /api/import/staging/files` +- `GET /api/import/staging/groups` +- `GET /api/import/staging/hints` +- `GET /api/import/staging/suggestions` +- `GET /api/import/search/albums` +- `POST /api/import/album/match` +- `POST /api/import/album/process` +- `GET /api/import/search/tracks` +- `POST /api/import/singles/process` +- `GET /api/auto-import/status` +- `POST /api/auto-import/toggle` +- `GET /api/auto-import/settings` +- `POST /api/auto-import/settings` +- `GET /api/auto-import/results` +- `POST /api/auto-import/approve/:id` +- `POST /api/auto-import/reject/:id` +- `POST /api/auto-import/scan-now` +- `POST /api/auto-import/approve-all` +- `POST /api/auto-import/clear-completed` + +## Implemented Route Slice + +```text +webui/src/routes/import/ + route.tsx + index.tsx + album.tsx + auto.tsx + singles.tsx + -import.types.ts + -import.api.ts + -import.helpers.ts + -import.store.ts + -route.test.tsx + -ui/ + import-page.tsx + album-import-tab.tsx + auto-import-tab.tsx + singles-import-tab.tsx + import-shared.tsx +``` + +## Implemented Route Responsibilities + +`route.tsx` + +- declare `/import` +- gate route through `bridge.isPageAllowed('import')` +- preload shell context +- prefetch the staging-files query without blocking on transient fetch failure + +`index.tsx` + +- redirect `/import` to `/import/album` + +`album.tsx` + +- prefetch album staging groups and suggestions + +`auto.tsx` + +- validate the `autoFilter` search param +- keep route-driven filter changes in the URL while leaving the rest of the workflow state local + +`singles.tsx` + +- mount the singles import tab without extra route-level loader work + +`-import.types.ts` + +- search param schema +- API response types +- staging file, staging group, album result, track result, match, auto-import result, and queue item types + +`-import.api.ts` + +- query options for staging, groups, suggestions, auto-import status, auto-import settings, auto-import results, album search, and track search +- mutation helpers for album match, album process, singles process, auto-import actions, and settings writes +- invalidation helpers for broad route refreshes, staging-only refreshes, and auto-import-only refreshes + +`-import.helpers.ts` + +- byte-size formatting +- album and track display labels +- confidence class/label mapping +- staging match normalization +- auto-import result filtering and counters + +`-import.store.ts` + +- album search state, selected album, auto-group file paths, and match overrides +- selected single-file state and manual matches +- queue job state and queue entry updates +- single-search draft state +- draft state survival across nested route remounts + +`-ui/import-page.tsx` + +- page chrome and nested route navigation +- queue summary and queue-item rendering +- nested route outlet for album, singles, and auto views + +## Search Params + +Use nested route paths for durable, shareable tab state: + +- `/import/album` + - default landing route +- `/import/singles` +- `/import/auto` +- `autoFilter` + - values: `all`, `pending`, `imported`, `failed` + - default: `all` + +Keep these local to React state: + +- album search text +- track search text +- selected album +- match overrides +- selected singles +- processing queue jobs + +Reasoning: + +- The tab choice belongs in the path, which keeps deep links simple and avoids an extra `tab=` query param. +- The auto-import filter is still useful after reloads. +- The matching workflow is ephemeral and should not create fragile URLs with file indexes or local staging paths. + +## Query Model + +Critical route-loader data: + +- `importStagingFilesQueryOptions()` + +Useful prefetch data: + +- `importStagingGroupsQueryOptions()` +- `importStagingSuggestionsQueryOptions()` + +Nested-route data: + +- `autoImportStatusQueryOptions()` +- `autoImportSettingsQueryOptions()` +- `autoImportResultsQueryOptions(autoFilter)` + +Lazy search data: + +- `importAlbumSearchQueryOptions(query)` +- `importTrackSearchQueryOptions(query)` + +Mutation-style actions: + +- album match draft +- process one album track +- process one single file +- toggle auto-import +- save auto-import settings +- scan now +- approve/reject auto-import result +- approve all +- clear completed + +Invalidation rules: + +- Processing album or singles files invalidates staging files, staging groups, staging suggestions, auto-import results, and any route-local queue completion summary. +- Auto-import actions invalidate auto-import status and results. +- Auto-import settings writes invalidate settings and status. +- Refresh invalidates staging files, groups, and suggestions. + +## Incremental Migration Order + +Recommended order: + +1. Add route slice, types, API helpers, reducer, and helper tests. +2. Build the React route shell with header, tabs, and staging summary. +3. Port the Albums tab search and suggestions, but keep processing disabled until match rendering is covered. +4. Port the album matching view, including drag/drop and tap assignment. +5. Port the processing queue and album/singles process mutations. +6. Port the Singles tab. +7. Port the Auto tab and polling behavior. +8. Flip `import` from `legacy` to `react` in the shell route manifest. +9. Remove the legacy `import-page` DOM from `webui/index.html`. +10. Remove import-specific legacy functions from `webui/static/stats-automations.js`. + +This order gives us a visible React page early while delaying the highest-risk file-processing actions until the state model is tested. + +The implemented route keeps the same overall migration shape, but the final URL contract uses nested route paths instead of a `tab=` search param. + +## Testing Sketch + +Unit tests: + +- route path and filter defaults +- staging summary formatting +- auto-import counters +- confidence labels +- reducer assignment behavior +- reducer queue transitions + +API tests: + +- staging files success and error +- staging groups success and error +- album search success and empty result +- track search success and empty result +- album match success and failure +- album process success and partial error +- singles process success and partial error +- auto-import status/results/settings/actions + +Route / component tests: + +- unauthorized users redirect to profile home +- default route redirects to `/import/album` +- `/import/singles` renders the Singles tab +- `/import/auto?autoFilter=pending` renders pending auto-import results +- refresh invalidates staging queries +- album selection opens the match view +- drag/drop and tap assignment update track matches +- processing queue advances and refreshes staging on completion +- client workflow drafts survive page remounts + +Playwright can wait until after route ownership flips. + +## Risks + +- The processing queue is client-side and long-running. +- Auto-import polling must stop when leaving the auto subroute or route. +- File indexes can become stale after staging refreshes. +- Album matching depends on preserving source, album name, and album artist from search results. +- The page currently shares a large legacy module with stats and automations code, so cleanup should be careful and incremental. + +## Decisions To Keep Simple + +- Keep the current visual language. +- Keep the existing backend endpoints. +- Keep the processing queue client-side for the first migration. +- Keep file matching state local to the route. +- Do not extract shared workflow primitives until a second migrated route needs them. + +## Outcome + +- The route now serves as the first React-owned workflow migration. +- The implementation uses nested route paths plus a validated `autoFilter` search param. +- The route uses TanStack Query for staging data, suggestions, auto-import polling, mutations, and invalidation. +- Tests cover shell ownership, nested route state, album match payload preservation, and auto-import rendering. + +## Recommendation + +Treat remaining work as cleanup and hardening rather than route selection. + +Follow-up work should optimize for: + +- shrinking `stats-automations.js` after cutover +- adding E2E coverage around full album and singles processing +- considering route-level code splitting once more large React routes land + +It should not optimize for: + +- redesign +- backend reshaping +- shared queue abstractions +- migrating `automations` at the same time diff --git a/webui/docs/migration/page-migration-overview.md b/webui/docs/migration/page-migration-overview.md index 01c96a57..16808c62 100644 --- a/webui/docs/migration/page-migration-overview.md +++ b/webui/docs/migration/page-migration-overview.md @@ -1,10 +1,10 @@ # WebUI Page Migration Overview -Snapshot date: 2026-05-14 +Snapshot date: 2026-05-15 ## Summary - The shell route manifest now has 18 page ids. -- `issues` and `stats` are now React-owned routes. +- `issues`, `stats`, and `import` are now React-owned routes. - Since the last snapshot, the biggest changes are: - `downloads` was renamed into `search`. - The live queue became `active-downloads`. @@ -78,9 +78,9 @@ Rollups: | --- | --- | --- | --- | --- | --- | | `issues` | React | 2 / 2 / 2 / 2 / 2 | Low | Low | Completed | | `stats` | React | 2 / 2 / 2 / 2 / 2 | Low | Low | Completed | +| `import` | React | 3 / 3 / 3 / 2 / 3 | Medium | Medium | Completed | | `help` | Legacy | 3 / 2 / 1 / 1 / 2 | Low | Low | Wave 1 | | `hydrabase` | Legacy | 2 / 2 / 2 / 2 / 2 | Low | Low | Wave 1 | -| `import` | Legacy | 3 / 3 / 3 / 2 / 3 | Medium | Medium | Wave 1 | | `search` | Legacy | 4 / 4 / 4 / 3 / 4 | High | High | Wave 2 | | `watchlist` | Legacy | 4 / 4 / 4 / 3 / 4 | High | High | Wave 3 | | `wishlist` | Legacy | 4 / 4 / 4 / 3 / 4 | High | High | Wave 3 | @@ -130,11 +130,13 @@ Rollups: - Recommendation: low-risk route with a narrow surface. #### `import` -- Current owner: Legacy. -- Primary files: `webui/index.html`, `webui/static/stats-automations.js`, `webui/static/helper.js`. +- Current owner: React. +- Primary files: `webui/src/routes/import/*`, `webui/src/platform/shell/route-manifest.ts`. - Main surface: staging files, album and singles matching, suggestion cards, processing queue. - Key coupling: settings-derived staging path assumptions and downstream library state. -- Recommendation: still bounded enough for an early wave, though more workflow-heavy than `help` or `hydrabase`. +- Recommendation: completed as the next migration after `stats`. The import subtree now uses nested route paths, with `/import` redirecting to `/import/album` and `autoFilter` remaining in the search string; any remaining `import` references in `webui/static/stats-automations.js` belong to the broader automation feature set, not this page migration. +- Route-local workflow state lives in `webui/src/routes/import/-import.store.ts`, which keeps drafts and queue state alive while moving between album, singles, and auto views. +- Route plan: `webui/docs/migration/import-migration-plan.md`. ### Wave 2: Search split @@ -264,9 +266,10 @@ Rollups: - Waves 6-10 defer the broadest, most coupled, or most orchestration-heavy surfaces until the team has the most leverage. ## Final Recommendation -- Keep `issues` and `stats` as the current React reference implementations, and preserve the explicit bridge contract between React routes and legacy shell behavior. +- Keep `issues`, `stats`, and `import` as the current React reference implementations, and preserve the explicit bridge contract between React routes and legacy shell behavior. - Treat `search`, `watchlist`, `wishlist`, `active-downloads`, and `tools` as the current route ids, and keep `downloads` and `artists` only as compatibility history. -- Migrate the remaining safe legacy routes first: `help`, `hydrabase`, and `import`. +- Migrate the remaining safe legacy routes first: `help` and `hydrabase`. +- `import` has already been migrated and should be treated as the first React-owned workflow route. - During each migration, actively look for small reuse opportunities across route slices and shared UI primitives, but only extract once the overlap is clearly real. -- Use `search` as the next meaningful proving ground now that the download queue has been split out. +- Use `search` as the next larger proving ground after `import`, now that the download queue has been split out. - Avoid pulling `settings`, `sync`, `library`, `artist-detail`, or `automations` forward unless there is a separate product priority strong enough to justify the added regression risk. diff --git a/webui/index.html b/webui/index.html index d590d158..709c9b9e 100644 --- a/webui/index.html +++ b/webui/index.html @@ -2435,10 +2435,6 @@ ← Back - @@ -2466,6 +2462,17 @@ 📻 Artist Radio + + -
@@ -6199,159 +6199,6 @@
- -
-
- -
-
-

Import Music

- -
-
- Import folder: loading... - -
-
- - - - - -
- - - -
- - -
-
-
- - Disabled - -
- - - -
- - - - -
-
-

Enable auto-import to watch your import folder for new music.

-

Drop album folders or single tracks into your import folder and SoulSync will identify, match, and import them automatically.

-
-
-
- - -
- -
-
- -
-
- -
-
- - - - -
- - -
-
-
- - -
-
-
-
Navigate to this page to scan your import folder for audio files.
-
- -
-
-
-
@@ -7174,8 +7021,10 @@
-
diff --git a/webui/package-lock.json b/webui/package-lock.json index 23b81ab1..3e6bf5a6 100644 --- a/webui/package-lock.json +++ b/webui/package-lock.json @@ -15,7 +15,8 @@ "react": "^19.2.5", "react-dom": "^19.2.5", "recharts": "^3.8.1", - "zod": "^4.4.2" + "zod": "^4.4.2", + "zustand": "^5.0.13" }, "devDependencies": { "@playwright/test": "^1.59.1", @@ -5391,6 +5392,35 @@ "funding": { "url": "https://github.com/sponsors/colinhacks" } + }, + "node_modules/zustand": { + "version": "5.0.13", + "resolved": "https://registry.npmjs.org/zustand/-/zustand-5.0.13.tgz", + "integrity": "sha512-efI2tVaVQPqtOh114loML/Z80Y4NP3yc+Ff0fYiZJPauNeWZeIp/bRFD7I9bfmCOYBh/PHxlglQ9+wvlwnPikQ==", + "license": "MIT", + "engines": { + "node": ">=12.20.0" + }, + "peerDependencies": { + "@types/react": ">=18.0.0", + "immer": ">=9.0.6", + "react": ">=18.0.0", + "use-sync-external-store": ">=1.2.0" + }, + "peerDependenciesMeta": { + "@types/react": { + "optional": true + }, + "immer": { + "optional": true + }, + "react": { + "optional": true + }, + "use-sync-external-store": { + "optional": true + } + } } } } diff --git a/webui/package.json b/webui/package.json index 56123f51..ad4cb775 100644 --- a/webui/package.json +++ b/webui/package.json @@ -21,7 +21,8 @@ "react": "^19.2.5", "react-dom": "^19.2.5", "recharts": "^3.8.1", - "zod": "^4.4.2" + "zod": "^4.4.2", + "zustand": "^5.0.13" }, "devDependencies": { "@playwright/test": "^1.59.1", diff --git a/webui/src/components/form/form.module.css b/webui/src/components/form/form.module.css index bf673cac..31eff1a8 100644 --- a/webui/src/components/form/form.module.css +++ b/webui/src/components/form/form.module.css @@ -73,6 +73,7 @@ .select { width: auto; min-width: 130px; + box-sizing: border-box; border: 1px solid rgba(255, 255, 255, 0.1); border-radius: 10px; background: @@ -95,6 +96,7 @@ font: inherit; font-size: 13px; line-height: 1.5; + min-height: 36px; padding: 8px 32px 8px 12px; transition: border-color 0.18s ease, @@ -105,28 +107,204 @@ -webkit-appearance: none; color-scheme: dark; cursor: pointer; + &:hover { + border-color: rgba(255, 255, 255, 0.16); + background: + linear-gradient(180deg, rgba(255, 255, 255, 0.075), rgba(255, 255, 255, 0.045)), + rgba(255, 255, 255, 0.06); + } + + &:focus { + outline: none; + border-color: rgba(var(--accent-light-rgb), 0.55); + background: + linear-gradient(180deg, rgba(255, 255, 255, 0.08), rgba(255, 255, 255, 0.05)), + rgba(255, 255, 255, 0.07); + box-shadow: 0 0 0 3px rgba(var(--accent-light-rgb), 0.12); + } + + &[data-size='sm'] { + min-height: 32px; + padding: 6px 30px 6px 10px; + background-position: + 0 0, + 0 0, + right 10px center; + } + + & option, + & optgroup { + background: #1a1a2e; + color: #fff; + } } -.select:hover { - border-color: rgba(255, 255, 255, 0.16); +.checkbox { + display: inline-flex; + align-items: center; + justify-content: center; + width: 18px; + height: 18px; + flex-shrink: 0; + box-sizing: border-box; + margin: 0; + border: 2px solid rgba(255, 255, 255, 0.2); + border-radius: 4px; + background: transparent; + color: #000; + cursor: pointer; + transition: + border-color 0.18s ease, + background 0.18s ease, + box-shadow 0.18s ease, + transform 0.18s ease; + color-scheme: dark; + &[data-checked] { + border-color: rgb(var(--accent-light-rgb)); + background: rgb(var(--accent-light-rgb)); + } + + &[data-focused] { + outline: none; + box-shadow: 0 0 0 3px rgba(var(--accent-light-rgb), 0.14); + } + + &[data-checked] .checkboxIndicator { + opacity: 1; + } +} + +.checkboxIndicator { + display: flex; + align-items: center; + justify-content: center; + width: 100%; + height: 100%; + opacity: 0; + transition: opacity 0.18s ease; +} + +.checkboxIcon { + color: #000; + font-size: 12px; + font-weight: 700; + line-height: 1; + transform: translateY(-0.5px) scaleX(1.18); +} + +.switch { + position: relative; + display: inline-flex; + align-items: center; + width: 44px; + height: 24px; + flex-shrink: 0; + box-sizing: border-box; + margin: 0; + padding: 0 3px; + border: 1px solid rgba(255, 255, 255, 0.16); + border-radius: 999px; background: - linear-gradient(180deg, rgba(255, 255, 255, 0.075), rgba(255, 255, 255, 0.045)), + linear-gradient(180deg, rgba(255, 255, 255, 0.08), rgba(255, 255, 255, 0.04)), rgba(255, 255, 255, 0.06); + cursor: pointer; + transition: + border-color 0.18s ease, + background 0.18s ease, + box-shadow 0.18s ease, + transform 0.18s ease; + color-scheme: dark; + &[data-checked] { + border-color: rgba(var(--accent-light-rgb), 0.55); + background: + linear-gradient(180deg, rgba(var(--accent-light-rgb), 0.55), rgba(var(--accent-rgb), 0.8)), + rgba(var(--accent-rgb), 0.45); + } + + &[data-focused] { + outline: none; + box-shadow: 0 0 0 3px rgba(var(--accent-light-rgb), 0.14); + } + + &[data-disabled] { + opacity: 0.55; + cursor: not-allowed; + } + + &[data-checked] .switchThumb { + transform: translateX(20px); + } } -.select:focus { - outline: none; - border-color: rgba(var(--accent-light-rgb), 0.55); +.switchThumb { + display: block; + width: 18px; + height: 18px; + border-radius: 50%; + background: #fff; + box-shadow: 0 1px 3px rgba(0, 0, 0, 0.25); + transform: translateX(0); + transition: transform 0.18s ease; +} + +.rangeRoot { + display: inline-flex; + flex: 0 0 160px; + width: 160px; + min-width: 160px; + color-scheme: dark; +} + +.rangeControl { + display: flex; + align-items: center; + width: 100%; + min-height: 28px; + cursor: pointer; +} + +.rangeTrack { + position: relative; + width: 100%; + height: 8px; + border: 1px solid rgba(255, 255, 255, 0.08); + border-radius: 999px; + background: rgba(255, 255, 255, 0.08); + box-shadow: inset 0 1px 1px rgba(255, 255, 255, 0.04); +} + +.rangeIndicator { + height: 100%; + border-radius: inherit; + background: linear-gradient( + 90deg, + rgba(var(--accent-rgb), 0.95), + rgba(var(--accent-light-rgb), 0.95) + ); +} + +.rangeThumb { + width: 16px; + height: 16px; + border: 1px solid rgba(255, 255, 255, 0.24); + border-radius: 50%; background: - linear-gradient(180deg, rgba(255, 255, 255, 0.08), rgba(255, 255, 255, 0.05)), - rgba(255, 255, 255, 0.07); - box-shadow: 0 0 0 3px rgba(var(--accent-light-rgb), 0.12); + radial-gradient(circle at 30% 28%, rgba(255, 255, 255, 0.3), transparent 46%), + rgb(var(--accent-light-rgb)); + box-shadow: 0 2px 8px rgba(0, 0, 0, 0.28); + transition: + transform 0.18s ease, + box-shadow 0.18s ease; } -.select option, -.select optgroup { - background: #1a1a2e; - color: #fff; +.rangeThumb:hover { + transform: scale(1.04); +} + +.rangeThumb:focus-visible { + box-shadow: + 0 0 0 4px rgba(var(--accent-light-rgb), 0.14), + 0 2px 8px rgba(0, 0, 0, 0.28); } .optionCardGroup { @@ -154,25 +332,24 @@ border-color 0.18s ease, box-shadow 0.18s ease, background 0.18s ease; -} + &:hover { + transform: translateY(-1px); + border-color: rgba(255, 255, 255, 0.14); + background: + linear-gradient(180deg, rgba(255, 255, 255, 0.075), rgba(255, 255, 255, 0.04)), + rgba(255, 255, 255, 0.04); + box-shadow: 0 12px 28px rgba(0, 0, 0, 0.22); + } -.optionCard:hover { - transform: translateY(-1px); - border-color: rgba(255, 255, 255, 0.14); - background: - linear-gradient(180deg, rgba(255, 255, 255, 0.075), rgba(255, 255, 255, 0.04)), - rgba(255, 255, 255, 0.04); - box-shadow: 0 12px 28px rgba(0, 0, 0, 0.22); -} - -.optionCardSelected { - border-color: rgba(var(--accent-light-rgb), 0.45); - background: - linear-gradient(180deg, rgba(var(--accent-rgb), 0.18), rgba(255, 255, 255, 0.04)), - rgba(255, 255, 255, 0.05); - box-shadow: - 0 0 0 1px rgba(var(--accent-light-rgb), 0.1), - 0 14px 32px rgba(0, 0, 0, 0.26); + &[data-selected='true'] { + border-color: rgba(var(--accent-light-rgb), 0.45); + background: + linear-gradient(180deg, rgba(var(--accent-rgb), 0.18), rgba(255, 255, 255, 0.04)), + rgba(255, 255, 255, 0.05); + box-shadow: + 0 0 0 1px rgba(var(--accent-light-rgb), 0.1), + 0 14px 32px rgba(0, 0, 0, 0.26); + } } .optionCardIcon { @@ -206,9 +383,22 @@ display: flex; flex-wrap: wrap; gap: 8px; + &[data-size='sm'] { + gap: 6px; + } + + &[data-size='sm'] .optionButton { + min-width: 0; + padding: 6px 12px; + font-size: 12px; + gap: 6px; + } } .optionButton { + display: inline-flex; + align-items: center; + gap: 8px; border: 1px solid rgba(255, 255, 255, 0.1); border-radius: 999px; background: rgba(255, 255, 255, 0.05); @@ -218,24 +408,45 @@ font-size: 13px; font-weight: 600; min-width: 80px; - padding: 10px 14px; + padding: 8px 16px; transition: transform 0.18s ease, border-color 0.18s ease, box-shadow 0.18s ease, background 0.18s ease; -} + &:hover { + transform: translateY(-1px); + border-color: rgba(255, 255, 255, 0.18); + background: rgba(255, 255, 255, 0.08); + } -.optionButton:hover { - transform: translateY(-1px); - border-color: rgba(255, 255, 255, 0.18); - background: rgba(255, 255, 255, 0.08); -} + &[data-variant='ghost'] { + border-color: transparent; + background: transparent; + color: rgba(255, 255, 255, 0.58); + } -.optionButtonSelected { - border-color: rgba(var(--accent-light-rgb), 0.5); - background: rgba(var(--accent-rgb), 0.18); - box-shadow: 0 0 0 1px rgba(var(--accent-light-rgb), 0.08); + &[data-variant='ghost']:hover:not(:disabled) { + transform: none; + border-color: rgba(255, 255, 255, 0.12); + background: rgba(255, 255, 255, 0.06); + color: rgba(255, 255, 255, 0.85); + } + + &[data-selected='true'] { + color: #fff; + border-color: rgba(var(--accent-light-rgb), 0.5); + background: rgba(var(--accent-rgb), 0.18); + box-shadow: 0 0 0 1px rgba(var(--accent-light-rgb), 0.08); + } + + &[data-selected='true']:hover { + border-color: rgba(var(--accent-light-rgb), 0.62); + background: rgba(var(--accent-rgb), 0.26); + box-shadow: + 0 0 0 1px rgba(var(--accent-light-rgb), 0.12), + 0 10px 24px rgba(0, 0, 0, 0.14); + } } .button { @@ -260,24 +471,118 @@ box-shadow 0.18s ease, background 0.18s ease, color 0.18s ease; -} + &:hover:not(:disabled) { + transform: translateY(-1px); + border-color: rgba(255, 255, 255, 0.18); + background: rgba(255, 255, 255, 0.08); + } -.button:hover:not(:disabled) { - transform: translateY(-1px); - border-color: rgba(255, 255, 255, 0.18); - background: rgba(255, 255, 255, 0.08); -} + &:focus-visible { + outline: none; + border-color: rgba(var(--accent-light-rgb), 0.55); + box-shadow: 0 0 0 3px rgba(var(--accent-light-rgb), 0.12); + } -.button:focus-visible { - outline: none; - border-color: rgba(var(--accent-light-rgb), 0.55); - box-shadow: 0 0 0 3px rgba(var(--accent-light-rgb), 0.12); -} + &:disabled { + opacity: 0.55; + cursor: not-allowed; + transform: none; + } -.button:disabled { - opacity: 0.55; - cursor: not-allowed; - transform: none; + &[data-size='sm'] { + min-height: 32px; + padding: 6px 10px; + font-size: 13px; + } + + &[data-size='lg'] { + min-height: 40px; + padding: 10px 20px; + font-size: 14px; + } + + &[data-size='icon'] { + width: var(--button-icon-size, 36px); + height: var(--button-icon-size, 36px); + min-height: var(--button-icon-size, 36px); + min-width: var(--button-icon-size, 36px); + padding: 0; + font-size: var(--button-icon-font-size, 15px); + line-height: 1; + } + + &[data-variant='primary'] { + border-color: rgba(var(--accent-light-rgb), 0.45); + background: rgb(var(--accent-light-rgb)); + color: #000; + } + + &[data-variant='primary']:hover:not(:disabled) { + border-color: rgba(var(--accent-light-rgb), 0.55); + background: rgba(var(--accent-light-rgb), 0.95); + color: #000; + } + + &[data-variant='primary']:focus-visible { + border-color: rgba(var(--accent-light-rgb), 0.8); + box-shadow: 0 0 0 3px rgba(var(--accent-light-rgb), 0.16); + } + + &[data-variant='primary']:disabled { + opacity: 0.62; + background: rgba(var(--accent-light-rgb), 0.45); + color: rgba(0, 0, 0, 0.55); + } + + &[data-variant='primary'] [data-slot='badge'] { + color: #fff; + background: rgba(0, 0, 0, 0.18); + border-color: rgba(0, 0, 0, 0.22); + } + + &[data-variant='secondary'] { + border-color: rgba(255, 255, 255, 0.12); + background: rgba(255, 255, 255, 0.08); + color: #ccc; + } + + &[data-variant='secondary']:hover:not(:disabled) { + border-color: rgba(255, 255, 255, 0.18); + background: rgba(255, 255, 255, 0.14); + color: #fff; + } + + &[data-variant='secondary']:focus-visible { + border-color: rgba(var(--accent-light-rgb), 0.45); + box-shadow: 0 0 0 3px rgba(var(--accent-light-rgb), 0.12); + } + + &[data-variant='secondary']:disabled { + opacity: 0.55; + background: rgba(255, 255, 255, 0.05); + color: rgba(255, 255, 255, 0.45); + } + + &[data-variant='ghost'] { + border-color: transparent; + background: transparent; + color: rgba(255, 255, 255, 0.55); + } + + &[data-variant='ghost']:hover:not(:disabled) { + border-color: rgba(255, 255, 255, 0.14); + background: rgba(255, 255, 255, 0.07); + color: #fff; + } + + &[data-variant='ghost']:focus-visible { + border-color: rgba(var(--accent-light-rgb), 0.42); + box-shadow: 0 0 0 3px rgba(var(--accent-light-rgb), 0.12); + } + + &[data-variant='ghost']:disabled { + opacity: 0.55; + } } .formError { diff --git a/webui/src/components/form/form.test.tsx b/webui/src/components/form/form.test.tsx index 3488a2f6..77b7677b 100644 --- a/webui/src/components/form/form.test.tsx +++ b/webui/src/components/form/form.test.tsx @@ -4,6 +4,7 @@ import { describe, expect, it } from 'vitest'; import { Button, + Checkbox, FormActions, FormError, FormField, @@ -11,7 +12,9 @@ import { OptionButtonGroup, OptionCard, OptionCardGroup, + RangeInput, Select, + Switch, TextArea, TextInput, } from './form'; @@ -22,6 +25,9 @@ function FormDemo() { const [category, setCategory] = useState<'wrong_cover' | 'wrong_metadata'>('wrong_cover'); const [priority, setPriority] = useState<'low' | 'normal' | 'high'>('normal'); const [status, setStatus] = useState('open'); + const [archive, setArchive] = useState(false); + const [enabled, setEnabled] = useState(true); + const [confidence, setConfidence] = useState(90); return (
@@ -90,11 +96,31 @@ function FormDemo() { + + + + + + + + + + + + - + ); @@ -109,9 +135,25 @@ describe('form primitives', () => { expect(screen.getByText('Short summary')).toBeInTheDocument(); expect(screen.getByRole('alert')).toHaveTextContent('Validation failed'); expect(screen.getByLabelText('Status')).toHaveValue('open'); + expect(screen.getByLabelText('Status')).toHaveAttribute('data-size', 'md'); fireEvent.change(screen.getByLabelText('Status'), { target: { value: 'resolved' } }); expect(screen.getByLabelText('Status')).toHaveValue('resolved'); + const archiveCheckbox = screen.getByRole('checkbox', { name: 'Archive' }); + expect(archiveCheckbox).not.toBeChecked(); + fireEvent.click(archiveCheckbox); + expect(archiveCheckbox).toBeChecked(); + + const enabledSwitch = screen.getByRole('switch', { name: 'Enabled' }); + expect(enabledSwitch).toBeChecked(); + fireEvent.click(enabledSwitch); + expect(enabledSwitch).not.toBeChecked(); + + const confidenceSlider = screen.getByLabelText('Confidence', { selector: 'input' }); + expect(confidenceSlider).toHaveValue('90'); + fireEvent.change(confidenceSlider, { target: { value: '75' } }); + expect(confidenceSlider).toHaveValue('75'); + const wrongCover = screen.getByRole('button', { name: /wrong cover/i }); const wrongMetadata = screen.getByRole('button', { name: /wrong metadata/i }); expect(wrongCover).toHaveAttribute('aria-pressed', 'true'); @@ -126,6 +168,32 @@ describe('form primitives', () => { expect(highPriority).toHaveAttribute('aria-pressed', 'true'); expect(screen.getByRole('button', { name: 'Cancel' })).toBeInTheDocument(); - expect(screen.getByRole('button', { name: 'Save' })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Save' })).toHaveAttribute('data-variant', 'primary'); + }); + + it('supports compact option button groups', () => { + const { container } = render( + + All + Pending + , + ); + + expect(container.querySelector('[data-size="sm"]')).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Pending' })).toHaveAttribute( + 'data-variant', + 'ghost', + ); + }); + + it('supports compact select sizing', () => { + render( + , + ); + + expect(screen.getByLabelText('Compact')).toHaveAttribute('data-size', 'sm'); }); }); diff --git a/webui/src/components/form/form.tsx b/webui/src/components/form/form.tsx index 15938435..9e894d43 100644 --- a/webui/src/components/form/form.tsx +++ b/webui/src/components/form/form.tsx @@ -1,10 +1,14 @@ import { Button as BaseButton } from '@base-ui/react/button'; +import { Checkbox as BaseCheckbox } from '@base-ui/react/checkbox'; import { Field } from '@base-ui/react/field'; import { Input as BaseInput } from '@base-ui/react/input'; +import { Slider } from '@base-ui/react/slider'; +import { Switch as BaseSwitch } from '@base-ui/react/switch'; import { Toggle as BaseToggle } from '@base-ui/react/toggle'; import clsx from 'clsx'; import { forwardRef, + type CSSProperties, type ComponentPropsWithoutRef, type ButtonHTMLAttributes, type SelectHTMLAttributes, @@ -73,13 +77,118 @@ export const TextArea = forwardRef(function return