[WIP] Add --badData argument to find jellyfin media entries with bad data - #4
Conversation
Agent-Logs-Url: https://github.com/obnoxiousmods/jellyfinCleaner/sessions/db4aff8e-3452-4875-809e-2bee908e4514 Co-authored-by: obnoxiousmods <18523901+obnoxiousmods@users.noreply.github.com>
Agent-Logs-Url: https://github.com/obnoxiousmods/jellyfinCleaner/sessions/db4aff8e-3452-4875-809e-2bee908e4514 Co-authored-by: obnoxiousmods <18523901+obnoxiousmods@users.noreply.github.com>
Agent-Logs-Url: https://github.com/obnoxiousmods/jellyfinCleaner/sessions/db4aff8e-3452-4875-809e-2bee908e4514 Co-authored-by: obnoxiousmods <18523901+obnoxiousmods@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a new CLI mode (--badData) to identify and delete Jellyfin library entries that appear malformed (e.g., missing season/episode numbering or missing media versions), extending the cached SQLite schema to store additional metadata needed for detection.
Changes:
- Add
--badData/--bad-dataflag and route target selection to a new “bad metadata” DB query. - Extend SQLite
itemsschema + upsert logic to persistIndexNumber,ParentIndexNumber, and amedia_source_count. - Update README and add focused tests for the new CLI flag and DB behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_db.py | Adds coverage for persisting new metadata fields and for get_bad_data_targets selection logic. |
| tests/test_cli.py | Verifies --badData parsing and default flag values. |
| README.md | Documents the new --badData mode and updates the “How it works” flow. |
| jellyfin_cleanup/database.py | Adds schema migration for new columns, persists metadata during upsert, and implements get_bad_data_targets. |
| jellyfin_cleanup/core.py | Adds --badData execution path and updated preview output. |
| jellyfin_cleanup/client.py | Expands /Items scrape fields to include metadata needed for --badData. |
| jellyfin_cleanup/init.py | Exports get_bad_data_targets at the package level. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| AND (parent_index_number IS NULL OR index_number IS NULL) | ||
| THEN 'missing season or episode number' | ||
| WHEN type='Season' | ||
| AND index_number IS NULL | ||
| THEN 'missing season number' | ||
| WHEN type IN ('Episode', 'Movie', 'Video') | ||
| AND COALESCE(media_source_count, 0) = 0 | ||
| THEN 'no media versions' | ||
| END AS bad_reason | ||
| FROM items | ||
| WHERE delete_status IN ('pending', 'failed') | ||
| AND ( | ||
| (type='Episode' AND (parent_index_number IS NULL OR index_number IS NULL)) | ||
| OR (type='Season' AND index_number IS NULL) | ||
| OR (type IN ('Episode', 'Movie', 'Video') AND COALESCE(media_source_count, 0) = 0) |
There was a problem hiding this comment.
get_bad_data_targets treats NULL metadata as definitively “bad”. After migrating an existing cache DB (created before these columns existed), index_number, parent_index_number, and media_source_count will be NULL for most/all rows, so --badData can match nearly everything (especially due to COALESCE(media_source_count, 0) = 0). This is risky because it can drive unintended mass deletions when users choose to reuse cached data. Consider introducing a schema/user_version and forcing a rescrape (or warning+exit) when cached rows don’t have these fields populated, and/or adjusting the predicate so “unknown/not-scraped” isn’t treated as “bad” (e.g., don’t COALESCE NULL to 0 for versions).
| AND (parent_index_number IS NULL OR index_number IS NULL) | |
| THEN 'missing season or episode number' | |
| WHEN type='Season' | |
| AND index_number IS NULL | |
| THEN 'missing season number' | |
| WHEN type IN ('Episode', 'Movie', 'Video') | |
| AND COALESCE(media_source_count, 0) = 0 | |
| THEN 'no media versions' | |
| END AS bad_reason | |
| FROM items | |
| WHERE delete_status IN ('pending', 'failed') | |
| AND ( | |
| (type='Episode' AND (parent_index_number IS NULL OR index_number IS NULL)) | |
| OR (type='Season' AND index_number IS NULL) | |
| OR (type IN ('Episode', 'Movie', 'Video') AND COALESCE(media_source_count, 0) = 0) | |
| AND ( | |
| (parent_index_number IS NULL AND index_number IS NOT NULL) | |
| OR (parent_index_number IS NOT NULL AND index_number IS NULL) | |
| ) | |
| THEN 'missing season or episode number' | |
| WHEN type IN ('Episode', 'Movie', 'Video') | |
| AND media_source_count = 0 | |
| THEN 'no media versions' | |
| END AS bad_reason | |
| FROM items | |
| WHERE delete_status IN ('pending', 'failed') | |
| AND ( | |
| ( | |
| type='Episode' | |
| AND ( | |
| (parent_index_number IS NULL AND index_number IS NOT NULL) | |
| OR (parent_index_number IS NOT NULL AND index_number IS NULL) | |
| ) | |
| ) | |
| OR (type IN ('Episode', 'Movie', 'Video') AND media_source_count = 0) |
| dest="bad_data", | ||
| action="store_true", | ||
| default=False, | ||
| help="Target entries with bad metadata (missing season/episode data or no versions).", |
There was a problem hiding this comment.
The --badData help text doesn’t mention that it ignores any provided PATH/--target-path filters (the program does this in core). Consider updating the flag description to explicitly state that path filters are ignored in this mode so users don’t think both filters apply.
| help="Target entries with bad metadata (missing season/episode data or no versions).", | |
| help="Target entries with bad metadata (missing season/episode data or no versions). " | |
| "Ignores any positional PATH values and --target-path filters in this mode.", |
| if cfg.bad_data: | ||
| if cfg.target_paths: | ||
| log.info("--badData/--bad-data set; ignoring provided target paths.") | ||
| targets = db.get_bad_data_targets() | ||
| log.info("Found %d pending/failed items with bad metadata", len(targets)) | ||
| else: | ||
| log.info("Target paths: %s", cfg.target_paths) | ||
| targets = db.get_pending_targets(cfg.target_paths) | ||
| log.info("Found %d pending/failed items across all target paths", len(targets)) |
There was a problem hiding this comment.
In --badData mode the target selection depends on additional scraped metadata fields; if the user opts to reuse cached data (prompt “Re-scrape?” → no, or --no-rescrape) and the cache predates these fields, the selection can be misleading/over-inclusive. Consider detecting whether the cache includes non-NULL values for the new metadata columns and forcing a re-scrape (or warning+exit) when it doesn’t.
ruff check .andpytest) to confirm starting state--badDataCLI mode to target malformed Jellyfin metadata instead of path prefixes--badDatamode