typing: type mpdstats, thumbnails plugins

[snejus]

Part of #6924.

• This PR is a typing and interface cleanup for beetsplug/mpdstats.py and beetsplug/thumbnails.py. It does not add new features; it makes the code's internal contracts clearer and more explicit.

• In mpdstats, MPD responses and now_playing state now have concrete types, path lookup uses the correct beets path shape, and player event handling is narrowed to known states. This makes the control flow easier to follow and reduces hidden assumptions in the plugin.

• In thumbnails, the code stops passing full Album objects where only album.path or artpath is needed. That separates album metadata from file-path operations, removes ambiguity around optional album.artpath, and makes thumbnail generation steps more direct.

• Tests were updated to match the narrower method signatures and the corrected path handling.

• High-level impact: safer internal APIs, clearer data flow, and lower risk of type-related bugs. The only visible behavior change is that missing album art is now logged as a warning instead of info.

[github-actions]

Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry.

[codecov]

Codecov Report

❌ Patch coverage is 84.31373% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.02%. Comparing base (38f9195) to head (e56f061).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
beetsplug/mpdstats.py	87.50%	7 Missing and 1 partial ⚠️
beetsplug/thumbnails.py	78.94%	5 Missing and 3 partials ⚠️

Additional details and impacted files

@@                       Coverage Diff                        @@
##           type-metadata-source-plugins    #6939      +/-   ##
================================================================
+ Coverage                         75.99%   76.02%   +0.02%     
================================================================
  Files                               164      164              
  Lines                             21585    21615      +30     
  Branches                           3342     3342              
================================================================
+ Hits                              16404    16433      +29     
+ Misses                             4387     4386       -1     
- Partials                            794      796       +2     

Files with missing lines	Coverage Δ
beetsplug/mpdstats.py	66.50% <87.50%> (+5.87%)	⬆️
beetsplug/thumbnails.py	83.00% <78.94%> (-2.03%)	⬇️

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Copilot]

Pull request overview

grug see PR try make typing more clear in two plugins (mpdstats, thumbnails). goal good: make internal contract less spooky and more explicit, and update tests to match new narrower signatures.

Changes:

• thumbnails: pass artpath / album.path instead of whole Album when only path data needed; log missing art as warning.
• mpdstats: add typed MPD response shapes + typed now_playing, narrow state handling to known MPD player states.
• update plugin tests to match new function signatures and path handling.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
beetsplug/thumbnails.py	type annotations + path-only params; choose URI getter and write thumbnails metadata
beetsplug/mpdstats.py	typed MPD client wrapper + typed now-playing state; path query encoding fix
test/plugins/test_thumbnails.py	update calls/asserts to pass artpath / album.path instead of Album object
test/plugins/test_mpdstats.py	remove unknown-status case; update expectations for narrowed state handling

Suppressed comments (3)

beetsplug/mpdstats.py:58

• grug see is_url type say str but body still handle bytes, and tests pass bytes sometimes. make signature match real input so type checker not lie.

def is_url(path: str) -> bool:
    """Try to determine if the path is an URL."""
    if isinstance(path, bytes):  # if it's bytes, then it's a path
        return False
    return path.split("://", 1)[0] in ["http", "https"]

beetsplug/mpdstats.py:159

• grug think MPD currentsong can return dict with no id (or empty dict). entry["id"] blow up KeyError. better be safe and use .get(..., "") so plugin not crash on weird MPD reply.

        self._log.debug("returning: {}", result)
        return result, entry["id"]

beetsplug/mpdstats.py:33

• grug see MPDStatus TypedDict not describe keys code actually use (time, songid). also state should stay required. can model with required base TypedDict plus total=False extension for optional fields.

class MPDStatus(TypedDict):
    id: str
    file: str
    state: Literal["play", "pause", "stop"]

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Suppressed comments (4)

beetsplug/mpdstats.py:87

• The function is annotated as accepting str, but it contains a runtime branch for bytes. This mismatch weakens the value of the annotation and will force type ignores/casts at call sites if bytes are ever passed. Update the signature to accept str | bytes (or refactor away the bytes branch if it’s no longer supported) so the type contract matches the implementation.

def is_url(path: str) -> bool:
    """Try to determine if the path is an URL."""
    if isinstance(path, bytes):  # if it's bytes, then it's a path
        return False

beetsplug/mpdstats.py:315

• run() passes status = self.mpd.status() (now typed as MPDStatus), but the handlers accept JSONDict. This loses the benefit of the new typed MPD contracts and makes it easier to accidentally use fields not present for a given state. Consider typing these parameters as MPDStatus, and (optionally) introducing a narrower type for the "play"/"pause" case (e.g., a TypedDict with required "time"/"songid" as appropriate) to avoid NotRequired fields being treated as always present.

    def on_stop(self, status: JSONDict) -> None:

beetsplug/mpdstats.py:325

• run() passes status = self.mpd.status() (now typed as MPDStatus), but the handlers accept JSONDict. This loses the benefit of the new typed MPD contracts and makes it easier to accidentally use fields not present for a given state. Consider typing these parameters as MPDStatus, and (optionally) introducing a narrower type for the "play"/"pause" case (e.g., a TypedDict with required "time"/"songid" as appropriate) to avoid NotRequired fields being treated as always present.

    def on_pause(self, status: JSONDict) -> None:

beetsplug/mpdstats.py:329

• run() passes status = self.mpd.status() (now typed as MPDStatus), but the handlers accept JSONDict. This loses the benefit of the new typed MPD contracts and makes it easier to accidentally use fields not present for a given state. Consider typing these parameters as MPDStatus, and (optionally) introducing a narrower type for the "play"/"pause" case (e.g., a TypedDict with required "time"/"songid" as appropriate) to avoid NotRequired fields being treated as always present.

    def on_play(self, status: JSONDict) -> None:

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (4)

beetsplug/thumbnails.py:203

• The explicit f.close() is redundant inside a with open(...) context manager and should be removed to avoid confusion and keep the resource-management pattern consistent.

        with open(syspath(outfilename), "w") as f:
            f.write("[Desktop Entry]\n")
            f.write(f"Icon=./{artfile.decode('utf-8')}")
            f.close()

beetsplug/mpdstats.py:378

• This removes the previous fallback for unexpected/unknown MPD state values and will now raise AttributeError (crashing the loop) if MPD ever returns something outside play|pause|stop (e.g., protocol quirks, buggy servers, or unexpected mock/test data). Consider restoring a safe fallback (e.g., handler = getattr(..., None) with a debug log) or explicitly raising a controlled error with context so the plugin fails predictably rather than via AttributeError.

        while True:
            if "player" in events:
                status = self.mpd.status()
                getattr(self, f"on_{status['state']}")(status)

beetsplug/mpdstats.py:58

• The MPDStatus TypedDict marks song and songid as required even though MPD commonly omits these fields when stopped (and/or when the playlist is empty). This can make the type misleading and push callers toward unsafe indexing. Making song/songid NotRequired[...] (and only treating them as required in the play|pause cases) would better match actual response shapes.

    song: str
    songid: str
    # below are only set when status is "play" or "pause"
    time: NotRequired[str]
    elapsed: NotRequired[str]

beetsplug/thumbnails.py:277

• When g_file_get_uri returns NULL, uri_ptr will be a null/None pointer; calling g_free(uri_ptr) here is unnecessary and can be error-prone with ctypes (especially if g_free argtypes are set elsewhere or change later). Since there is nothing to free on the NULL path, remove the g_free call in this branch and just raise the error.

        if not uri_ptr:
            libgio.g_free(uri_ptr)
            raise RuntimeError(
                f"No URI received from the gfile pointer for {displayable_path(path)}"
            )