Skip to content

Fixes #89: min-length setting does not work - #90

Open
DevonLobb wants to merge 1 commit into
SIMKL:masterfrom
DevonLobb:fix/min-length
Open

DevonLobb wants to merge 1 commit into
SIMKL:masterfrom
DevonLobb:fix/min-length

Conversation

@DevonLobb

@DevonLobb DevonLobb commented Sep 23, 2026 •

Copy link
Copy Markdown

Scrobbling is skipped if the media length is less than min-length (default 5m).

total_time = int(self.getTotalTime())
total_time_min = int(get_setting("min-length"))
if total_time <= 0 or total_time > total_time_min

total_time is measured in seconds, but total_time_min is measured in minutes, which makes this always true, resulting in every file over 5s (including trailers) triggering a scrobble.

Summary by CodeRabbit

  • Bug Fixes
    • Corrected how minimum-length settings are compared with media duration during playback tracking. The check now uses consistent time units, helping playback tracking apply the configured threshold accurately across media of different lengths.

…aring it total_time, otherwise the default minimum length is 5s instead of 5m.
@coderabbitai

coderabbitai Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: fc9bcfd1-5cfb-4302-ae79-cdcc8450a5ce

📥 Commits

Reviewing files that changed from the base of the PR and between e87ead1 and 03b0199.

📒 Files selected for processing (1)
  • resources/lib/engine.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Playback tracking now converts the configured minimum length from minutes to seconds before comparing it with playback duration in seconds.

Changes

Playback tracking

Layer / File(s) Summary
Convert minimum duration to seconds
resources/lib/engine.py
total_time_min now multiplies the configured min-length value by 60 before the duration comparison.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~5 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 03b01

The minimum-duration conversion matches the supported setting values; no merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: fixing the nonfunctional min-length setting caused by a minutes-to-seconds unit mismatch.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

1 participant