fix(slskd): year-date early rejects, rejected-source grabs, retry poisoning - #76
Merged
Merged
Conversation
A folder year parsed from the share name became a Jan-1 PublishDate, and Lidarr's EarlyReleaseSpecification permanently rejects any report published more than the early-limit before the album's release date — so every well-named "(2017)" source lost to a mid-2017 album, leaving only year-less (often partial) folders grabbable. Live 2026-08-06: all ten coherent FLAC sources for a single were rejected this way, ~400 rejections/day overall. - AlbumData: only a day-precision date carries publish semantics; year/month synthesis and missing dates report discovery time (age 0). Pre-release protection still works for real dates: a future day-precision date stays early until release. - SlskdItemsParser: tag folder years as year precision. The "(YYYY)" title suffix is unchanged.
MatchedSearchCriteria=false only steered tier-stop counting — the release still reached Lidarr's decision engine and could win the grab once quality or date specs killed the matched sources. Live 2026-08-06: a partial single source the parser had logged as "rejecting partial source" was grabbed twice in one day and could never import (it held two versions of the same track). Automatic searches now drop unmatched albums at parse time (counted in the parse summary as dropped); interactive searches keep emitting everything so the operator can still pick.
Two lifecycle gaps from the 2026-08-06 live audit:
- The pre-import tagger's tag write grows a file (padding/artwork) and
feat-strip renames it, so the basename+size ownership guard could no
longer claim it — the leftover was retained on every delete and, with
batch destinations pinned per album, poisoned every retry of the same
album (import failed at "has unmatched tracks" forever). The tagger now
reports each successful tag's original and final path, and the manager
re-learns the file's identity on the item, so the guard can claim it.
- Restart rehydrates items for downloads slskd still lists (succeeded
transfers persist ~24h) even when Lidarr already resolved them. Replayed
DownloadDirectoryComplete events then post-processed folders that were
legitimately gone ("Folder missing after DownloadDirectoryComplete"),
and the vanished-files check re-failed an already-failed download after
every restart — with the folder guess naming another album's folder when
the single was plucked from an album share. Post-processing and the
vanished-files failure now skip downloads whose history is terminal;
re-attachment itself stays (queue visibility).
Publish-date precision rules (year/none/day), the coherent-source classifier on the live partial-source case, the tagged-file ownership overlay (rename + in-place rewrite), and the terminal-event predicate. 386 tests total.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThe changes refine Soulseek release-date handling and automatic search filtering. They also track tagged file identities for ownership cleanup and prevent repeat processing of terminal downloads. Tests cover metadata, matching, ownership overlays, and terminal events. ChangesSoulseek processing updates
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PreImportTagger
participant SlskdDownloadManager
participant SlskdDownloadItem
PreImportTagger-->>SlskdDownloadManager: Return TaggingResult with final file paths
SlskdDownloadManager->>SlskdDownloadItem: RecordTaggedFile(basename, size)
SlskdDownloadManager->>SlskdDownloadItem: BuildOwnedFileSizes()
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/Sleezer/Core/PostProcessing/PreImportTagger.cs (1)
130-134: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReduce narrative comments to concise gotchas.
Keep each comment to one or two lines. State the required invariant. Remove live-incident history and extended consequence detail.
src/Sleezer/Core/PostProcessing/PreImportTagger.cs#L130-L134: Keep only that successful tagging must report original and final paths for ownership tracking.src/Sleezer/Download/Clients/Soulseek/Models/SlskdDownloadItem.cs#L85-L90: Keep only that tagged files can change basename and size.src/Sleezer/Download/Clients/Soulseek/SlskdDownloadManager.cs#L1104-L1108: Keep only that terminal rehydrated downloads must not start post-processing.src/Sleezer/Download/Clients/Soulseek/SlskdDownloadManager.cs#L1205-L1208: Keep only that ownership identities must be refreshed after tagging.As per path instructions, comments must be navigational or gotcha-only and limited to one or two lines.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Sleezer/Core/PostProcessing/PreImportTagger.cs` around lines 130 - 134, Shorten the comments at src/Sleezer/Core/PostProcessing/PreImportTagger.cs#L130-L134, src/Sleezer/Download/Clients/Soulseek/Models/SlskdDownloadItem.cs#L85-L90, src/Sleezer/Download/Clients/Soulseek/SlskdDownloadManager.cs#L1104-L1108, and src/Sleezer/Download/Clients/Soulseek/SlskdDownloadManager.cs#L1205-L1208 to one or two lines each. Retain only the required gotchas: successful tagging reports original and final paths for ownership tracking; tagged files may change basename and size; terminal rehydrated downloads must not start post-processing; and ownership identities must be refreshed after tagging. Remove incident history and extended consequences.Source: Path instructions
src/Sleezer/Core/Model/AlbumData.cs (1)
84-89: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep added comments to short operational gotchas.
Both comments include historical context and incident narrative. Keep only the invariant needed at each code path.
src/Sleezer/Core/Model/AlbumData.cs#L84-L89: state that only day-precision dates populatePublishDate; year/month dates use discovery time.src/Sleezer/Indexers/Soulseek/SlskdIndexerParser.cs#L162-L168: state that unmatched releases are retained for interactive searches and dropped for automatic searches.As per path instructions, comments must be navigational/gotcha notes limited to 1–2 lines.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Sleezer/Core/Model/AlbumData.cs` around lines 84 - 89, Shorten the comment at src/Sleezer/Core/Model/AlbumData.cs#L84-L89 to state only that day-precision dates populate PublishDate, while year/month-precision dates use discovery time. Also shorten the comment at src/Sleezer/Indexers/Soulseek/SlskdIndexerParser.cs#L162-L168 to state that unmatched releases are retained for interactive searches and dropped for automatic searches; keep each comment to 1–2 lines.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Sleezer/Download/Clients/Soulseek/SlskdDownloadManager.cs`:
- Around line 947-953: The vanished-file handling in the surrounding download
reconciliation logic must use IsTerminalDownloadEvent instead of
IsPoisonedHistoryEvent, so DownloadImportIncomplete remains non-terminal and
completed items with missing folders are failed and retried. Add a regression
test covering DownloadImportIncomplete with a missing completed folder,
asserting it is treated as non-terminal.
---
Nitpick comments:
In `@src/Sleezer/Core/Model/AlbumData.cs`:
- Around line 84-89: Shorten the comment at
src/Sleezer/Core/Model/AlbumData.cs#L84-L89 to state only that day-precision
dates populate PublishDate, while year/month-precision dates use discovery time.
Also shorten the comment at
src/Sleezer/Indexers/Soulseek/SlskdIndexerParser.cs#L162-L168 to state that
unmatched releases are retained for interactive searches and dropped for
automatic searches; keep each comment to 1–2 lines.
In `@src/Sleezer/Core/PostProcessing/PreImportTagger.cs`:
- Around line 130-134: Shorten the comments at
src/Sleezer/Core/PostProcessing/PreImportTagger.cs#L130-L134,
src/Sleezer/Download/Clients/Soulseek/Models/SlskdDownloadItem.cs#L85-L90,
src/Sleezer/Download/Clients/Soulseek/SlskdDownloadManager.cs#L1104-L1108, and
src/Sleezer/Download/Clients/Soulseek/SlskdDownloadManager.cs#L1205-L1208 to one
or two lines each. Retain only the required gotchas: successful tagging reports
original and final paths for ownership tracking; tagged files may change
basename and size; terminal rehydrated downloads must not start post-processing;
and ownership identities must be refreshed after tagging. Remove incident
history and extended consequences.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6fff9f22-4814-4ab0-acfb-7896600cf22c
📒 Files selected for processing (7)
src/Sleezer/Core/Model/AlbumData.cssrc/Sleezer/Core/PostProcessing/PreImportTagger.cssrc/Sleezer/Download/Clients/Soulseek/Models/SlskdDownloadItem.cssrc/Sleezer/Download/Clients/Soulseek/SlskdDownloadManager.cssrc/Sleezer/Indexers/Soulseek/SlskdIndexerParser.cssrc/Sleezer/Indexers/Soulseek/SlskdItemsParser.cstests/Sleezer.Tests/SlskdPublishDateAndOwnershipTests.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Live audit of a stuck single (GLXY – Proposition / Mind Less, two grabs on 2026-08-06, zero imports) traced four interacting defects across search, grab, and retry. Full chain, byte-verified against slskd transfer records and Lidarr history:
PublishDate = 2017-01-01; with an Early Download Limit set on the indexer, Lidarr permanently rejected every report "published" before the album's actual mid-2017 release date. All ten coherent FLAC sources died this way (~400 rejections/day across the library); only year-less folders survived.covers 1/2 matchable tracks — rejecting partial source), butMatchedSearchCriteria=falseonly affected tier-stop counting — the release still reached the decision engine and won. The download held two versions of the same track and could never complete the 2-track single.4. GLXY - Mind Less.flac→02 - Mind Less.flac, +31 KB), so the basename+size ownership guard retained it on every cleanup. With batch destinations pinned per album, the leftover contaminated every retry:Album match 76.7% vs 80%, has unmatched tracks, forever.DownloadDirectoryCompleteevents post-processed long-gone folders and the vanished-files check re-failed an already-failed download — resolving its folder to another album's name when the single had been plucked from an album share.Changes
AlbumData: only day-precision dates carry publish semantics; year/month synthesis and missing dates report discovery time. Pre-release protection still works for real future dates. The(YYYY)title suffix is unchanged.SlskdItemsParser: folder years are tagged year-precision.SlskdIndexerParser: automatic searches drop unmatched albums at parse time (newdropped … unmatchedcounter in the parse summary); interactive searches still emit everything.PreImportTagger:TaggingResultnow reports each successful tag's original and final path.SlskdDownloadItem: post-tag identities overlay the enqueued basename+size map (BuildOwnedFileSizes), so the ownership guard can claim tagged/renamed files; newIsTerminalDownloadEventpredicate.SlskdDownloadManager: records tagged identities after the tag pass; post-processing and the vanished-files failure skip downloads whose history is terminal (re-attachment itself stays, for queue visibility).Verification
Summary by CodeRabbit
Bug Fixes
Search
Metadata