From 633983ed9805b9c3eda60ea7479ce4b132c6835a Mon Sep 17 00:00:00 2001 From: Uyanide Date: Fri, 10 Apr 2026 07:24:29 +0200 Subject: [PATCH] feat: watch mode fetch immediatly on track changes regardless of player status --- README.md | 4 +- requirements.txt | 66 +++++----- src/lrx_cli/watch/session.py | 57 +++------ src/lrx_cli/watch/view/__init__.py | 12 +- src/lrx_cli/watch/view/pipe.py | 10 +- tests/test_watch.py | 196 +++++++++++++++++++++-------- 6 files changed, 206 insertions(+), 139 deletions(-) diff --git a/README.md b/README.md index cec6f9a..98500f6 100644 --- a/README.md +++ b/README.md @@ -143,8 +143,8 @@ socket_path = "" # Unix socket path; defaults to / Clone this repository: ```bash -git clone https://github.com/Uyanide/LRX-CLI.git -cd LRX-CLI +git clone https://github.com/Uyanide/lrx-cli.git +cd lrx-cli ``` Create a virtual environment and install dependencies (for example, using uv): diff --git a/requirements.txt b/requirements.txt index 00ea56d..c9e54a5 100644 --- a/requirements.txt +++ b/requirements.txt @@ -21,9 +21,9 @@ colorama==0.4.6 ; sys_platform == 'win32' \ # via # loguru # pytest -cyclopts==4.10.1 \ - --hash=sha256:35f37257139380a386d9fe4475e1e7c87ca7795765ef4f31abba579fcfcb6ecd \ - --hash=sha256:ad4e4bb90576412d32276b14a76f55d43353753d16217f2c3cd5bdceba7f15a0 +cyclopts==4.10.2 \ + --hash=sha256:a1f2d6f8f7afac9456b48f75a40b36658778ddc9c6d406b520d017ae32c990fe \ + --hash=sha256:d7b950457ef2563596d56331f80cbbbf86a2772535fb8b315c4f03bc7e6127f1 # via lrx-cli dbus-next==0.2.3 \ --hash=sha256:58948f9aff9db08316734c0be2a120f6dc502124d9642f55e90ac82ffb16a18b \ @@ -79,27 +79,23 @@ packaging==26.0 \ --hash=sha256:00243ae351a257117b6a241061796684b084ed1c516a08c48a3f7e147a9d80b4 \ --hash=sha256:b36f1fef9334a5588b4166f8bcd26a14e521f2b55e6b9de3aaa80d3ff7a37529 # via pytest -platformdirs==4.9.4 \ - --hash=sha256:1ec356301b7dc906d83f371c8f487070e99d3ccf9e501686456394622a01a934 \ - --hash=sha256:68a9a4619a666ea6439f2ff250c12a853cd1cbd5158d258bd824a7df6be2f868 +platformdirs==4.9.6 \ + --hash=sha256:3bfa75b0ad0db84096ae777218481852c0ebc6c727b3168c1b9e0118e458cf0a \ + --hash=sha256:e61adb1d5e5cb3441b4b7710bea7e4c12250ca49439228cc1021c00dcfac0917 # via lrx-cli pluggy==1.6.0 \ --hash=sha256:7dcc130b76258d33b90f61b658791dede3486c3e6bfb003ee5c9bfb396dd22f3 \ --hash=sha256:e920276dd6813095e9377c0bc5566d94c932c33b27a3e3945d8389c374dd4746 # via pytest -pygments==2.19.2 \ - --hash=sha256:636cb2477cec7f8952536970bc533bc43743542f70392ae026374600add5b887 \ - --hash=sha256:86540386c03d588bb81d44bc3928634ff26449851e99741617ecb9037ee5ec0b +pygments==2.20.0 \ + --hash=sha256:6757cd03768053ff99f3039c1a36d6c0aa0b263438fcab17520b30a303a82b5f \ + --hash=sha256:81a9e26dd42fd28a23a2d169d86d7ac03b46e2f8b59ed4698fb4785f946d0176 # via # pytest # rich -pytest==9.0.2 \ - --hash=sha256:711ffd45bf766d5264d487b917733b453d917afd2b0ad65223959f59089f875b \ - --hash=sha256:75186651a92bd89611d1d9fc20f0b4345fd827c41ccd5c299a868a05d70edf11 -python-dotenv==1.2.2 \ - --hash=sha256:1d8214789a24de455a8b8bd8ae6fe3c6b69a5e3d64aa8a8e5d68e694bbcb285a \ - --hash=sha256:2c371a91fbd7ba082c2c1dc1f8bf89ca22564a087c2c287cd9b662adde799cf3 - # via lrx-cli +pytest==9.0.3 \ + --hash=sha256:2c5efc453d45394fdd706ade797c0a81091eccd1d6e4bccfcd476e2b8e0ab5d9 \ + --hash=sha256:b86ada508af81d19edeb213c681b1d48246c1a91d304c6c81a427674c17eb91c rich==14.3.3 \ --hash=sha256:793431c1f8619afa7d3b52b2cdec859562b950ea0d4b6b505397612db8d5362d \ --hash=sha256:b8daa0b9e4eef54dd8cf7c86c03713f53241884e814f4e2f5fb342fe520f639b @@ -110,25 +106,25 @@ rich-rst==1.3.2 \ --hash=sha256:a1196fdddf1e364b02ec68a05e8ff8f6914fee10fbca2e6b6735f166bb0da8d4 \ --hash=sha256:a99b4907cbe118cf9d18b0b44de272efa61f15117c61e39ebdc431baf5df722a # via cyclopts -ruff==0.15.8 \ - --hash=sha256:04f79eff02a72db209d47d665ba7ebcad609d8918a134f86cb13dd132159fc89 \ - --hash=sha256:0f29b989a55572fb885b77464cf24af05500806ab4edf9a0fd8977f9759d85b1 \ - --hash=sha256:12e617fc01a95e5821648a6df341d80456bd627bfab8a829f7cfc26a14a4b4a3 \ - --hash=sha256:2033f963c43949d51e6fdccd3946633c6b37c484f5f98c3035f49c27395a8ab8 \ - --hash=sha256:432701303b26416d22ba696c39f2c6f12499b89093b61360abc34bcc9bf07762 \ - --hash=sha256:6ee3ae5c65a42f273f126686353f2e08ff29927b7b7e203b711514370d500de3 \ - --hash=sha256:75e5cd06b1cf3f47a3996cfc999226b19aa92e7cce682dcd62f80d7035f98f49 \ - --hash=sha256:8d9a5b8ea13f26ae90838afc33f91b547e61b794865374f114f349e9036835fb \ - --hash=sha256:995f11f63597ee362130d1d5a327a87cb6f3f5eae3094c620bcc632329a4d26e \ - --hash=sha256:ac51d486bf457cdc985a412fb1801b2dfd1bd8838372fc55de64b1510eff4bec \ - --hash=sha256:bc1f0a51254ba21767bfa9a8b5013ca8149dcf38092e6a9eb704d876de94dc34 \ - --hash=sha256:c2a33a529fb3cbc23a7124b5c6ff121e4d6228029cba374777bd7649cc8598b8 \ - --hash=sha256:c9861eb959edab053c10ad62c278835ee69ca527b6dcd72b47d5c1e5648964f6 \ - --hash=sha256:cbe05adeba76d58162762d6b239c9056f1a15a55bd4b346cfd21e26cd6ad7bc7 \ - --hash=sha256:cf891fa8e3bb430c0e7fac93851a5978fc99c8fa2c053b57b118972866f8e5f2 \ - --hash=sha256:d3e3d0b6ba8dca1b7ef9ab80a28e840a20070c4b62e56d675c24f366ef330570 \ - --hash=sha256:d910ae974b7a06a33a057cb87d2a10792a3b2b3b35e33d2699fdf63ec8f6b17a \ - --hash=sha256:fdce027ada77baa448077ccc6ebb2fa9c3c62fd110d8659d601cf2f475858d94 +ruff==0.15.10 \ + --hash=sha256:0744e31482f8f7d0d10a11fcbf897af272fefdfcb10f5af907b18c2813ff4d5f \ + --hash=sha256:0ee3ef42dab7078bda5ff6a1bcba8539e9857deb447132ad5566a038674540d0 \ + --hash=sha256:136c00ca2f47b0018b073f28cb5c1506642a830ea941a60354b0e8bc8076b151 \ + --hash=sha256:28cb32d53203242d403d819fd6983152489b12e4a3ae44993543d6fe62ab42ed \ + --hash=sha256:51cb8cc943e891ba99989dd92d61e29b1d231e14811db9be6440ecf25d5c1609 \ + --hash=sha256:601d1610a9e1f1c2165a4f561eeaa2e2ea1e97f3287c5aa258d3dab8b57c6188 \ + --hash=sha256:8154d43684e4333360fedd11aaa40b1b08a4e37d8ffa9d95fee6fa5b37b6fab1 \ + --hash=sha256:83e1dd04312997c99ea6965df66a14fb4f03ba978564574ffc68b0d61fd3989e \ + --hash=sha256:8ab88715f3a6deb6bde6c227f3a123410bec7b855c3ae331b4c006189e895cef \ + --hash=sha256:8b80a2f3c9c8a950d6237f2ca12b206bccff626139be9fa005f14feb881a1ae8 \ + --hash=sha256:93cc06a19e5155b4441dd72808fdf84290d84ad8a39ca3b0f994363ade4cebb1 \ + --hash=sha256:a768ff5969b4f44c349d48edf4ab4f91eddb27fd9d77799598e130fb628aa158 \ + --hash=sha256:b0c52744cf9f143a393e284125d2576140b68264a93c6716464e129a3e9adb48 \ + --hash=sha256:b1e7c16ea0ff5a53b7c2df52d947e685973049be1cdfe2b59a9c43601897b22e \ + --hash=sha256:d1f86e67ebfdef88e00faefa1552b5e510e1d35f3be7d423dc7e84e63788c94e \ + --hash=sha256:d4272e87e801e9a27a2e8df7b21011c909d9ddd82f4f3281d269b6ba19789ca5 \ + --hash=sha256:e3e53c588164dc025b671c9df2462429d60357ea91af7e92e9d56c565a9f1b07 \ + --hash=sha256:e59c9bdc056a320fb9ea1700a8d591718b8faf78af065484e801258d3a76bc3f win32-setctime==1.2.0 ; sys_platform == 'win32' \ --hash=sha256:95d644c4e708aba81dc3704a116d8cbc974d70b3bdb8be1d150e36be6e9d1390 \ --hash=sha256:ae1fdf948f5640aae05c511ade119313fb6a30d7eabe25fef9764dca5873c4c0 diff --git a/src/lrx_cli/watch/session.py b/src/lrx_cli/watch/session.py index 18006ad..3e63490 100644 --- a/src/lrx_cli/watch/session.py +++ b/src/lrx_cli/watch/session.py @@ -17,7 +17,7 @@ from ..models import TrackMeta from .control import ControlServer from .fetcher import LyricFetcher from ..config import AppConfig -from .view import BaseOutput, LyricView, WatchState +from .view import BaseOutput, LyricView, WatchState, WatchStatus from .player import ActivePlayerSelector, PlayerMonitor, PlayerTarget from .tracker import PositionTracker @@ -28,14 +28,14 @@ class WatchModel: offset_ms: int active_player: str | None active_track_key: str | None - status: str + status: WatchStatus lyrics: LyricView | None def __init__(self) -> None: self.offset_ms = 0 self.active_player: str | None = None self.active_track_key: str | None = None - self.status: str = "idle" + self.status: WatchStatus = WatchStatus.IDLE self.lyrics: LyricView | None = None def set_lyrics(self, lyrics: LRCData | None) -> None: @@ -56,7 +56,7 @@ class WatchModel: else None ) - if self.status != "ok" or self.lyrics is None: + if self.status != WatchStatus.OK or self.lyrics is None: return ("status", self.status, self.active_player, track_key) at_ms = position_ms + self.offset_ms cursor = self.lyrics.signature_cursor(at_ms) @@ -82,7 +82,7 @@ class WatchViewModel: lyrics=self._model.lyrics, position_ms=position_ms, offset_ms=self._model.offset_ms, - status=self._model.status, # type: ignore[arg-type] + status=self._model.status, ) @@ -207,7 +207,7 @@ class WatchCoordinator: return False if self._model.lyrics is not None: return False - if self._model.status == "fetching": + if self._model.status == WatchStatus.FETCHING: return False logger.info("fetching lyrics for track ({}): {}", reason, track.display_name()) self._fetcher.request(track) @@ -246,7 +246,7 @@ class WatchCoordinator: ) if selected is None: - self._model.status = "idle" + self._model.status = WatchStatus.IDLE self._model.active_track_key = None self._model.set_lyrics(None) self._schedule_emit() @@ -254,7 +254,7 @@ class WatchCoordinator: state = self._player_monitor.players.get(selected) if state is None: - self._model.status = "idle" + self._model.status = WatchStatus.IDLE self._model.active_track_key = None self._model.set_lyrics(None) self._schedule_emit() @@ -284,27 +284,16 @@ class WatchCoordinator: ) ) - if state.status != "Playing": - self._model.status = "paused" - self._schedule_emit() - return - started_fetch = False if track is not None and (player_changed or track_changed): started_fetch = self._request_fetch_for_active_track("track-changed") - elif ( - track is not None - and self._model.lyrics is None - and self._model.status == "paused" - ): - started_fetch = self._request_fetch_for_active_track("resume-playing") if self._model.lyrics is not None: - self._model.status = "ok" + self._model.status = WatchStatus.OK elif started_fetch: - self._model.status = "fetching" - elif self._model.status != "fetching": - self._model.status = "no_lyrics" + self._model.status = WatchStatus.FETCHING + elif self._model.status != WatchStatus.FETCHING: + self._model.status = WatchStatus.NO_LYRICS self._schedule_emit() def _on_seeked(self, bus_name: str, position_ms: int) -> None: @@ -312,24 +301,12 @@ class WatchCoordinator: asyncio.create_task(self._tracker.on_seeked(bus_name, position_ms)) def _on_playback_status(self, bus_name: str, status: str) -> None: - """React to playback status change and tracker sync.""" - if bus_name == self._model.active_player: - if status == "Playing": - started_fetch = self._request_fetch_for_active_track("resume-playing") - if self._model.lyrics is not None: - self._model.status = "ok" - elif started_fetch: - self._model.status = "fetching" - elif self._model.status != "fetching": - self._model.status = "no_lyrics" - else: - self._model.status = "paused" - self._schedule_emit() + """Forward playback status change to position tracker.""" asyncio.create_task(self._tracker.on_playback_status(bus_name, status)) def _on_tracker_tick(self) -> None: """Emit updates from tracker tick only while lyrics are actively rendering.""" - if self._model.status == "ok": + if self._model.status == WatchStatus.OK: self._schedule_emit() def _schedule_emit(self) -> None: @@ -348,13 +325,15 @@ class WatchCoordinator: async def _on_fetching(self) -> None: """Mark model as fetching and emit state.""" - self._model.status = "fetching" + self._model.status = WatchStatus.FETCHING await self._emit_state() async def _on_lyrics_update(self, lyrics: Optional[LRCData]) -> None: """Update model with fetched lyrics and emit state.""" self._model.set_lyrics(lyrics) - self._model.status = "ok" if lyrics is not None else "no_lyrics" + self._model.status = ( + WatchStatus.OK if lyrics is not None else WatchStatus.NO_LYRICS + ) logger.info( "lyrics update result: {}", "found" if lyrics is not None else "not found", diff --git a/src/lrx_cli/watch/view/__init__.py b/src/lrx_cli/watch/view/__init__.py index 699ca1e..c01e3fc 100644 --- a/src/lrx_cli/watch/view/__init__.py +++ b/src/lrx_cli/watch/view/__init__.py @@ -3,12 +3,20 @@ from abc import ABC, abstractmethod from bisect import bisect_right from dataclasses import dataclass -from typing import Literal, Optional +from enum import Enum +from typing import Optional from ...lrc import LRCData, LyricLine from ...models import TrackMeta +class WatchStatus(str, Enum): + IDLE = "idle" + FETCHING = "fetching" + OK = "ok" + NO_LYRICS = "no_lyrics" + + @dataclass(slots=True, frozen=True) class LyricView: """View-ready immutable lyric data projected from one normalized LRC object.""" @@ -70,7 +78,7 @@ class WatchState: lyrics: Optional[LyricView] position_ms: int offset_ms: int - status: Literal["fetching", "ok", "no_lyrics", "paused", "idle"] + status: WatchStatus class BaseOutput(ABC): diff --git a/src/lrx_cli/watch/view/pipe.py b/src/lrx_cli/watch/view/pipe.py index e170bb3..0eb5390 100644 --- a/src/lrx_cli/watch/view/pipe.py +++ b/src/lrx_cli/watch/view/pipe.py @@ -4,7 +4,7 @@ from bisect import bisect_right from dataclasses import dataclass import sys -from . import BaseOutput, WatchState +from . import BaseOutput, WatchState, WatchStatus @dataclass(slots=True) @@ -70,13 +70,11 @@ class PipeOutput(BaseOutput): async def on_state(self, state: WatchState) -> None: """Render and flush one frame for the latest watch state.""" - if state.status == "fetching": + if state.status == WatchStatus.FETCHING: lines = self._render_status("[fetching...]") - elif state.status == "no_lyrics": + elif state.status == WatchStatus.NO_LYRICS: lines = self._render_status("[no lyrics]") - elif state.status == "paused": - lines = self._render_status("[paused]") - elif state.status == "idle": + elif state.status == WatchStatus.IDLE: lines = self._render_status("[idle]") else: lines = self._render_lyrics(state) diff --git a/tests/test_watch.py b/tests/test_watch.py index 8ff9e48..03acf96 100644 --- a/tests/test_watch.py +++ b/tests/test_watch.py @@ -6,7 +6,7 @@ from pathlib import Path from lrx_cli.lrc import LRCData from lrx_cli.models import TrackMeta from lrx_cli.watch.control import ControlClient, ControlServer, parse_delta -from lrx_cli.watch.view import BaseOutput, LyricView, WatchState +from lrx_cli.watch.view import BaseOutput, LyricView, WatchState, WatchStatus from lrx_cli.watch.view.pipe import PipeOutput from lrx_cli.watch.player import ActivePlayerSelector, PlayerState, PlayerTarget from lrx_cli.watch.fetcher import LyricFetcher @@ -299,62 +299,148 @@ def test_pipe_output_repeated_text_uses_correct_timed_occurrence(capsys) -> None assert printed == "B\nX\nC\n" -def test_session_fetches_on_resume_playing_without_lyrics() -> None: +# ── WatchCoordinator state machine ─────────────────────────────────────────── + + +class _CaptureFetcher: + """Records fetch requests without doing real network calls.""" + + def __init__(self) -> None: + self.requested: list[str] = [] + + def request(self, track: TrackMeta) -> None: + self.requested.append(track.display_name()) + + async def stop(self) -> None: + pass + + +def _make_coordinator() -> WatchCoordinator: + class _Manager: + def fetch_for_track(self, *_a, **_kw): + return None + + class _Output(BaseOutput): + async def on_state(self, state: WatchState) -> None: + pass + + session = WatchCoordinator( + _Manager(), # type: ignore + _Output(), + player_hint=None, + config=TEST_CONFIG, + ) + session._tracker = PositionTracker( + lambda _bus: asyncio.sleep(0, result=0), + TEST_CONFIG, + ) + return session + + +BUS = "org.mpris.MediaPlayer2.spotify" + + +def _pstate(status: str = "Playing", title: str = "Song") -> PlayerState: + return PlayerState( + bus_name=BUS, + status=status, + track=TrackMeta(title=title, artist="Artist"), + ) + + +def test_coordinator_fetches_on_initial_player() -> None: async def _run() -> None: - class _Manager: - def fetch_for_track(self, *_args, **_kwargs): - return None + session = _make_coordinator() + fetcher = _CaptureFetcher() + session._fetcher = fetcher # type: ignore[assignment] - class _Output(BaseOutput): - async def on_state(self, state: WatchState) -> None: - return None - - class _Fetcher(LyricFetcher): - def __init__(self): - async def _fetch(_track: TrackMeta): - return None - - async def _on_fetching() -> None: - return None - - async def _on_result(_lyrics) -> None: - return None - - super().__init__( - _fetch, _on_fetching, _on_result, TEST_CONFIG.watch.debounce_ms - ) - self.requested = [] - - def request(self, track: TrackMeta) -> None: - self.requested.append(track.display_name()) - - session = WatchCoordinator( - _Manager(), # type: ignore - _Output(), - player_hint=None, - config=TEST_CONFIG, - ) - fake_fetcher = _Fetcher() - session._fetcher = fake_fetcher - session._tracker = PositionTracker( - lambda _bus: asyncio.sleep(0, result=0), - TEST_CONFIG, - ) - - bus_name = "org.mpris.MediaPlayer2.spotify" - track = TrackMeta(title="Song", artist="Artist") - session._model.active_player = bus_name - session._player_monitor.players = { - bus_name: PlayerState(bus_name=bus_name, status="Playing", track=track) - } - session._model.set_lyrics(None) - session._model.status = "paused" - - session._on_playback_status(bus_name, "Playing") + session._player_monitor.players = {BUS: _pstate("Playing")} + session._on_player_change() await asyncio.sleep(0) - assert fake_fetcher.requested == ["Artist - Song"] - assert session._model.status == "fetching" + assert fetcher.requested == ["Artist - Song"] + assert session._model.status == WatchStatus.FETCHING + + asyncio.run(_run()) + + +def test_coordinator_fetches_while_paused() -> None: + """Fetch is triggered immediately even when player is paused.""" + + async def _run() -> None: + session = _make_coordinator() + fetcher = _CaptureFetcher() + session._fetcher = fetcher # type: ignore[assignment] + + session._player_monitor.players = {BUS: _pstate("Paused")} + session._on_player_change() + await asyncio.sleep(0) + + assert fetcher.requested == ["Artist - Song"] + + asyncio.run(_run()) + + +def test_coordinator_fetches_on_track_change() -> None: + async def _run() -> None: + session = _make_coordinator() + session._model.active_player = BUS + session._model.active_track_key = "Artist - Old Song" + session._model.set_lyrics(LRCData("[00:01.00]old")) + session._model.status = WatchStatus.OK + + fetcher = _CaptureFetcher() + session._fetcher = fetcher # type: ignore[assignment] + + session._player_monitor.players = {BUS: _pstate("Playing", title="New Song")} + session._on_player_change() + await asyncio.sleep(0) + + assert fetcher.requested == ["Artist - New Song"] + assert session._model.lyrics is None # cleared on track change + + asyncio.run(_run()) + + +def test_coordinator_no_refetch_on_calibration_no_lyrics() -> None: + """Calibration with same player/track and no_lyrics must NOT trigger a second fetch.""" + + async def _run() -> None: + session = _make_coordinator() + fetcher = _CaptureFetcher() + session._fetcher = fetcher # type: ignore[assignment] + + session._player_monitor.players = {BUS: _pstate("Playing")} + session._on_player_change() # first call: player appears → fetch + await asyncio.sleep(0) + assert len(fetcher.requested) == 1 + + session._model.status = WatchStatus.NO_LYRICS # simulate fetch returned nothing + + session._on_player_change() # calibration: same player/track + await asyncio.sleep(0) + assert len(fetcher.requested) == 1 # no second fetch + + asyncio.run(_run()) + + +def test_coordinator_no_fetch_when_lyrics_present() -> None: + async def _run() -> None: + session = _make_coordinator() + session._model.active_player = BUS + session._model.active_track_key = "Artist - Song" + session._model.set_lyrics(LRCData("[00:01.00]line")) + session._model.status = WatchStatus.OK + + fetcher = _CaptureFetcher() + session._fetcher = fetcher # type: ignore[assignment] + + session._player_monitor.players = {BUS: _pstate("Playing")} + session._on_player_change() + await asyncio.sleep(0) + + assert fetcher.requested == [] + assert session._model.status == WatchStatus.OK asyncio.run(_run()) @@ -391,7 +477,7 @@ def test_session_emit_state_only_when_lyric_cursor_changes() -> None: bus_name: PlayerState(bus_name=bus_name, status="Playing", track=track) } session._model.set_lyrics(LRCData("[00:01.00]a\n[00:03.00]b")) - session._model.status = "ok" + session._model.status = WatchStatus.OK await session._tracker.set_active_player( bus_name, "Playing", @@ -441,7 +527,7 @@ def test_session_emits_when_crossing_first_timestamp() -> None: bus_name: PlayerState(bus_name=bus_name, status="Playing", track=track) } session._model.set_lyrics(LRCData("[00:02.00]a\n[00:03.00]b")) - session._model.status = "ok" + session._model.status = WatchStatus.OK await session._tracker.set_active_player( bus_name, "Playing",