Conversation
… death row logging Fix title_and_year_match rejecting exact year matches due to inverted year != check. The fallback watch data matcher only matched when years differed by exactly 1, causing items with exact year matches to appear unwatched and become deletion candidates. Fix get_or_create_collection crashing when called with no items after Plex auto-removes an empty collection. Return None instead of calling createCollection with an empty items list. Improve death row logging to explain why items were filtered out (thresholds, exclusions, or watch activity since tagging). Refs #227
|
🐳 A Docker image for this PR will be available after the build completes: docker run -e LOG_LEVEL=DEBUG --rm \
-v ./config:/config \
-v ./logs:/config/logs \
ghcr.io/rfsbraz/deleterr:pr-229
|
There was a problem hiding this comment.
Pull request overview
This PR fixes multiple issues in the “death row / leaving_soon” flow by correcting fallback watch-history matching, preventing Plex collection creation crashes when there are no items, and improving the log output when previously-tagged items are no longer eligible for deletion.
Changes:
- Fix
title_and_year_matchso same-year history matches succeed (while keeping ±1-year tolerance) and add regression tests. - Make leaving_soon collection updates resilient to empty item sets by allowing
get_or_create_collectionto returnNoneand clearing existing collections when appropriate. - Enhance death-row logging to include how many items were filtered out and add tests for the new message.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
app/media_cleaner.py |
Pass items into collection creation, handle None return, and clear existing collections when no items remain. |
app/modules/plex.py |
Update get_or_create_collection to accept optional items and skip creation when empty. |
app/deleterr.py |
Add filtered-out count to death-row logging. |
tests/test_media_cleaner_helpers.py |
Add regression tests for title_and_year_match same-year and boundary cases. |
tests/test_leaving_soon.py |
Update existing expectations for new collection-creation signature and add edge-case + logging tests. |
tests/modules/test_plex.py |
Expand Plex module tests for collection creation with/without items. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def get_or_create_collection(self, library: Any, name: str, items: Optional[list] = None) -> Optional[Any]: | ||
| """Get existing collection or create a new one. | ||
|
|
||
| Args: | ||
| library: The Plex library section. | ||
| name: The name of the collection. | ||
| items: Optional list of items to create the collection with. | ||
|
|
||
| Returns: | ||
| The collection object. | ||
| The collection object, or None if it doesn't exist and no items to create with. | ||
| """ |
There was a problem hiding this comment.
PlexMediaServer.get_or_create_collection changes the abstract interface by adding an items kwarg and allowing None returns, but BaseMediaServer.get_or_create_collection still documents/declares a 2-arg method that always returns a collection. This divergence makes the interface contract unclear for other media server implementations and for typed call sites. Update BaseMediaServer (and any other implementations) to match the new signature/semantics, or keep the old signature and handle creation/empty-items behavior internally without changing the interface.
| logger.info( | ||
| f"Found {len(death_row_plex_items)} items in leaving_soon, " | ||
| f"{len(items_to_delete)} still match deletion criteria" | ||
| + (f" ({filtered_out} protected by thresholds, exclusions, or watch activity since tagging)" if filtered_out > 0 else "") |
There was a problem hiding this comment.
filtered_out is computed as the count of death-row items that are not in the intersection with current deletion candidates, but the log message states they are "protected by thresholds, exclusions, or watch activity". In practice this difference can also include items that can’t be mapped back to Radarr/Sonarr candidates (e.g., ID/title mismatches, items no longer present), so the message can be misleading. Consider wording it as "no longer match deletion criteria (e.g., thresholds/exclusions/recent watch activity)" or similar, without attributing a definitive cause.
| + (f" ({filtered_out} protected by thresholds, exclusions, or watch activity since tagging)" if filtered_out > 0 else "") | |
| + ( | |
| f" ({filtered_out} no longer match deletion criteria " | |
| f"(e.g., thresholds, exclusions, or recent watch activity) since tagging)" | |
| if filtered_out > 0 | |
| else "" | |
| ) |
…ature The integration test was asserting the old call signature without the items kwarg, causing CI failure.
|
Summary
year !=check that caused exact year matches to fail. The fallback watch data matcher only matched when years differed by exactly 1, making same-year items appear unwatched and eligible for deletion.get_or_create_collectionnow returnsNoneinstead of crashing when called with no items (happens when Plex auto-removes an empty collection after all its items are deleted).Test plan
test_exact_year_match_should_find_watch_data- red/green verified regressiontest_off_by_one_year_still_matches- existing ±1 tolerance preservedtest_year_off_by_two_rejects- years 2+ apart still rejectedtest_different_title_rejects- different titles rejectedtest_none_year_on_plex_item_rejects/test_none_year_on_history_rejects- None years handledtest_collection_not_created_when_no_items- graceful None returntest_existing_collection_cleared_when_no_items- existing collection clearedtest_death_row_logging_shows_filtered_count- enhanced log messageRefs #227