fix(poster_renamerr): report only genuine Plex uploads and stop steady-state border churn#356
Merged
Merged
Conversation
…y-state border churn
Audit of the plex apply path (apply_method == "plex") confirmed the
skip-unchanged pipeline is sound, but surfaced four fixes:
- Notifications: the scheduled run notified from the STAGED set and
discarded the uploader's outcome, so skipped/failed posters (and
per-run re-flow retries) appeared in Discord/Notifiarr as uploads
every schedule. The uploader now returns a per-poster "uploaded"
record (action == "updated" only) and the plex path notifies from
that; when nothing genuinely uploaded a one-line heartbeat replaces
the poster list ("No posters were uploaded (nothing changed)." /
"(N failed — check the logs)."). Kometa path unchanged. The
formatter also renders artist/album now, so music-only runs no
longer misreport as "No files were renamed."
- Border churn + /tmp leak: the plex path ran the border pass with
process_all=True, sweeping ALL matched rows. Layer-A-skipped rows
point at prior runs' deleted staging dirs, so every poster was
re-encoded into a recreated stale /tmp/chub_poster_plex_* dir that
nothing cleans (RAM-backed /tmp on Unraid) — pure waste, nothing
uploads from there. New manifest_only flag hard-restricts the plex
border pass to this run's manifest, overriding every full-library
trigger (process_all, holiday reset_all, missing manifest). Kometa
keeps its full-library re-border.
- Wrong-year live upload: the live title-only fallback accepted a
LONE candidate with no year check, so a poster could land on a
same-title sibling of a clearly different year (e.g. a 2018 poster
on the 1978 original) when the cache missed. The single-candidate
branch is now year-guarded like the multi-candidate one; a
candidate with NO year is kept (metadata gap must not reject the
only candidate, mirroring PlexMediaIndex._disambiguate_by_year).
- Border-excluded assets: with borders enabled the border op is the
sole file writer, so an excluded title never got a staged file —
the plex uploader failed "Could not read poster file" on every run
and the kometa destination stayed empty. Excluded assets now get a
crash-safe unbordered copy staged (exclusion means no border, not
no poster).
Also: structural dry-run guard on the hash-equal persist branch, and
a collection analogue of the media skip-column persistence test
(source_file_hash / uploaded_libraries / file_hash / file_mtime /
original_file must survive re-sync or collections would re-upload
every run — previously untested).
|
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 (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughChangesThe PR adds manifest-only border processing with atomic staging for excluded posters, reports only genuine uploads for Plex notifications, expands notification formatting, tightens lone Plex title matching by year, and tests collection cache field preservation. Poster processing and upload reporting
Plex matching and cache preservation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PosterRenamerr
participant BorderReplacerr
participant UploadPosters
participant NotificationFormatter
PosterRenamerr->>BorderReplacerr: Process manifest-only posters
BorderReplacerr-->>PosterRenamerr: Return staged files
PosterRenamerr->>UploadPosters: Upload poster files
UploadPosters-->>PosterRenamerr: Return genuine uploaded records
PosterRenamerr->>NotificationFormatter: Format upload notification
NotificationFormatter-->>PosterRenamerr: Return formatted output
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| def _subset_harness(tmp_path, monkeypatch, exclusion_list=None): | ||
| """Shared setup for manifest-mode runs: two real source posters (ids 1/2), | ||
| a fake DB, notifications stubbed. Returns (br, dest_dir, rows_by_id).""" | ||
| import backend.modules.border_replacerr as border_mod |
Owner
Author
There was a problem hiding this comment.
Declining: the local import backend.modules.border_replacerr as border_mod (monkeypatch target) alongside the top-level from ... import BorderReplacerr is the established pattern in this file — four pre-existing tests do the same. Changing only the new helper would diverge from its siblings; restructuring the file's imports is out of scope here.
![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 (1)
backend/util/plex.py (1)
567-571: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the year-guard comment concise.
This five-line explanation includes rationale that is better represented by the code and tests. Reduce it to one or two navigational lines, such as:
# Reject explicit year mismatches; keep yearless candidates.As per path instructions, comments must be navigational/instructional only (1-2 lines) and avoid why/history essays.
🤖 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 `@backend/util/plex.py` around lines 567 - 571, Shorten the year-guard comment near the lone-candidate logic to one or two navigational lines describing the behavior: reject explicit year mismatches while retaining yearless candidates. Remove the examples, rationale, and references to implementation history or other methods.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 `@backend/util/plex.py`:
- Around line 572-579: Update the year compatibility logic around lone_year and
year_compatible so only genuinely missing year values bypass validation.
Distinguish empty or absent years from non-empty malformed values, and reject or
skip candidates whose explicit candidate year cannot be converted; preserve the
tolerance comparison for valid numeric years.
---
Nitpick comments:
In `@backend/util/plex.py`:
- Around line 567-571: Shorten the year-guard comment near the lone-candidate
logic to one or two navigational lines describing the behavior: reject explicit
year mismatches while retaining yearless candidates. Remove the examples,
rationale, and references to implementation history or other methods.
🪄 Autofix (Beta)
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: e6ab13a0-6b15-406c-93f8-4d208f6c40b5
📒 Files selected for processing (10)
backend/modules/border_replacerr.pybackend/modules/poster_renamerr.pybackend/util/notification_formatting.pybackend/util/plex.pybackend/util/upload_posters.pytests/test_border_replacerr.pytests/test_media_cache_upsert.pytests/test_notification.pytests/test_plex_connect.pytests/test_poster_renamerr.py
Review follow-ups: a non-empty malformed year on the lone title-only candidate now rejects it (only genuinely missing years are treated as a metadata gap), with a test locking it; shortened the year-guard comment; closed the file handle in the excluded-asset staging test.
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
2 participants
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.
Summary
Follow-up to a full audit of the poster_renamerr → upload-to-Plex path (
apply_method == "plex"). The audit confirmed the two-layer skip-unchanged pipeline is sound — a steady-state scheduled run uploads nothing — but surfaced four fixes, all bounded/config-gated:1. Notifications report genuine uploads only (
upload_posters.py,poster_renamerr.py,notification_formatting.py)The scheduled run notified from the staged set and discarded the uploader's outcome (
PosterUploader(...).run()return was unused), so skipped/failed posters — including per-run re-flow retries — appeared in Discord/Notifiarr as uploads on every schedule.uploaded: per-poster records foraction == "updated"only (title/year/season/library/instance).Uploaded to Movies, Movies 4K,Season 02 uploaded to TV); when nothing genuinely uploaded, a one-line heartbeat replaces the poster list:No posters were uploaded (nothing changed).orNo posters were uploaded (N failed — check the logs).job_processor._handle_post_rename_actions).2. Plex border pass restricted to the manifest (
border_replacerr.py,poster_renamerr.py)The plex path ran the border pass with
process_all=True, sweeping all matched rows. Layer-A-skipped rows point at prior runs' deleted staging dirs, so every poster was re-decoded/re-encoded into a recreated stale/tmp/chub_poster_plex_*dir that nothing cleans (RAM-backed /tmp on Unraid) — pure waste; nothing uploads from there. Newmanifest_onlyflag hard-restricts the plex border pass to this run's manifest, overriding every full-library trigger (process_all, holidayreset_all, missing manifest). Kometa keeps its full-library re-border.3. Year-guard on the lone live-search candidate (
plex.py)The live title-only fallback accepted a lone candidate with no year check (only the multi-candidate branch was year-tolerant), so a poster could land on a same-title sibling of a clearly different year (a 2018 poster on the 1978 original) when the cache missed. The single-candidate branch now applies the same ±tolerance; a candidate with NO year is kept (a metadata gap must not reject the only candidate — mirrors
PlexMediaIndex._disambiguate_by_year).4. Border-excluded assets get staged unbordered (
border_replacerr.py)With borders enabled the border op is the sole file writer, so an
exclusion_list/ignore_folderstitle never got a staged file — the plex uploader failed "Could not read poster file" on every run and the kometa destination stayed empty. Excluded assets now get a crash-safe unbordered copy staged (exclusion means no border, not no poster). The manifest branch's duplicated inline exclusion checks were consolidated into_asset_excluded.Also
dry_runguard on the hash-equal persist branch in_sync_single_asset(unreachable today, guarded anyway).source_file_hash/uploaded_libraries/file_hash/file_mtime/original_filemust survive a collections re-sync upsert or every collection would re-upload each run (previously untested; guards theON CONFLICT DO UPDATE SETcolumn list).Behavior changes to note
Test plan
ruff check .clean; fullpytestgreen.empty_textoverride + artist/album rendering, lone-hit year-guard (wrong-year rejected / year-less kept),manifest_onlyoverridesprocess_alle2e, excluded-manifest-asset staged with exact source bytes e2e, collection skip-column persistence across re-sync.Summary by CodeRabbit
New Features
Bug Fixes
Tests