Skip to content

typing: add missing types to beets.importer#6711

Open
snejus wants to merge 6 commits into
masterfrom
add-missing-types-to-importer
Open

typing: add missing types to beets.importer#6711
snejus wants to merge 6 commits into
masterfrom
add-missing-types-to-importer

Conversation

@snejus
Copy link
Copy Markdown
Member

@snejus snejus commented Jun 5, 2026

  • This change tightens typing across the importer pipeline, mainly in beets.importer.session, beets.importer.stages, beets.importer.tasks, beets.importer.state, and beets.util.pipeline.

  • Architecturally, it makes the importer's core flow more explicit:

    • session methods now declare clear input/output types
    • pipeline stage message types are named and propagated through Pipeline
    • task classes expose more precise return values and argument types
    • plugin event dispatch for import_task_created now has a typed contract
  • A small but important part of the change is switching importer config handling to work with confuse views more safely, using .set(...) and .get(...) instead of replacing values directly. This keeps config mutation aligned with the config system instead of treating it like a plain dict.

  • There are also a few correctness-oriented fixes uncovered while adding types, such as:

    • keeping byte/string path handling consistent in importer task cleanup and archive extraction
    • making nullable values explicit before use
    • returning None in places that already behaved that way implicitly
  • High-level impact: no feature change to importer behavior is intended. The main benefit is a clearer, more strongly typed importer architecture that is easier to reason about, safer to refactor, and better at catching edge cases during development.

Copilot AI review requested due to automatic review settings June 5, 2026 17:20
@snejus snejus requested a review from a team as a code owner June 5, 2026 17:20
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 5, 2026

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.

@snejus snejus requested a review from semohr June 5, 2026 17:21
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 5, 2026

Codecov Report

❌ Patch coverage is 95.65217% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.58%. Comparing base (d179dbb) to head (9ffe742).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
beets/importer/tasks.py 92.53% 4 Missing and 1 partial ⚠️
beets/util/pipeline.py 95.23% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6711   +/-   ##
=======================================
  Coverage   75.57%   75.58%           
=======================================
  Files         162      162           
  Lines       20806    20815    +9     
  Branches     3292     3291    -1     
=======================================
+ Hits        15725    15733    +8     
- Misses       4306     4307    +1     
  Partials      775      775           
Files with missing lines Coverage Δ
beets/dbcore/db.py 94.06% <100.00%> (-0.03%) ⬇️
beets/importer/session.py 94.32% <100.00%> (-0.04%) ⬇️
beets/importer/stages.py 88.62% <100.00%> (-0.14%) ⬇️
beets/importer/state.py 91.37% <100.00%> (+0.15%) ⬆️
beets/plugins.py 88.84% <100.00%> (ø)
beetsplug/convert.py 74.70% <100.00%> (-0.08%) ⬇️
beetsplug/fetchart.py 74.07% <100.00%> (ø)
beets/util/pipeline.py 91.96% <95.23%> (-0.34%) ⬇️
beets/importer/tasks.py 91.32% <92.53%> (+0.18%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

grug see PR try make importer more typed. goal good: less guess, more clear pipe flow. but grug also see one bug where plugin return shape now break old contract.

Changes:

  • add many type hints across importer tasks/session/stages, and pipeline typing
  • make importer config mutation use confuse .set(...) / .get(...)
  • small cleanup in plugins event typing and some path bytes handling

Reviewed changes

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

Show a summary per file
File Description
beetsplug/fetchart.py guard prune call when candidate path maybe None
beetsplug/convert.py small pipeline call simplification
beets/util/pipeline.py add typing + generic Pipeline shape + adjust parallel run structure
beets/plugins.py add overload for typed send() event contract
beets/importer/tasks.py add types, safer config access, adjust archive extract bytes handling, and change import_task_created dispatch path
beets/importer/state.py add types for ImportState and context manager methods
beets/importer/stages.py add stage message type aliases and stage function return types
beets/importer/session.py add types + switch config mutation to confuse .set(...) / .get(...)
beets/dbcore/db.py make LazyConvertDict implement Mapping for typing
.git-blame-ignore-revs ignore rev for this typing-only-ish change set

Comment thread beets/importer/tasks.py
Comment thread beets/plugins.py
Comment thread beets/dbcore/db.py
Comment thread beets/importer/tasks.py
@snejus snejus force-pushed the add-missing-types-to-importer branch from 83eece5 to 16998ab Compare June 5, 2026 23:12
@snejus snejus changed the base branch from master to return-keysview-from-keys-methods June 5, 2026 23:12
@snejus snejus force-pushed the add-missing-types-to-importer branch from 16998ab to 2de70c5 Compare June 6, 2026 00:29
@snejus snejus force-pushed the return-keysview-from-keys-methods branch from ff1c9e7 to b19cc73 Compare June 6, 2026 05:53
@snejus snejus force-pushed the add-missing-types-to-importer branch from 2de70c5 to dbe9a86 Compare June 6, 2026 05:56
Base automatically changed from return-keysview-from-keys-methods to master June 6, 2026 09:09
@snejus snejus force-pushed the add-missing-types-to-importer branch 2 times, most recently from 35acaac to b148c94 Compare June 6, 2026 09:23
snejus added 6 commits June 6, 2026 10:31
Previously, we had
`iconfig: dict[str, confuse.Subview] = dict(config)`

Since this was just a dictionary, explicit assignments like
`iconfig["copy"] = False` replaced `Subview` with the value:

```
iconfig
{
  'autotag': <Subview: import.autotag>,
  'bell': <Subview: import.bell>,
  'copy': False,
  ...
}
```

Thus, keys overriden by `set_config` lost their `Subview` type, where
methods like `.get` were lost. This ultimately resulted in the following
type:

`ImportSession.config: dict[str, confuse.Subview | str | bool]`
This branch never gets run, and it would cause a TypeError if it did.
dirs is always provided as a list of bytes.
@snejus snejus force-pushed the add-missing-types-to-importer branch from b148c94 to 9ffe742 Compare June 6, 2026 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants