Skip to content

fix(arr): use authoritative library paths for provider reconciliation - #138

Merged
moderniselife merged 1 commit into
moderniselife:developfrom
samtruman:pr/tmdb-arr-path-reconciliation
Oct 18, 2026
Merged

moderniselife merged 1 commit into
moderniselife:developfrom
samtruman:pr/tmdb-arr-path-reconciliation

Conversation

@samtruman

@samtruman samtruman commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • resolve the existing Radarr/Sonarr entity before creating a provider-backed symlink;
  • match by title, original title, alternate titles and year, then fall back to Arr's own metadata lookup and TMDb identity;
  • place the symlink below the authoritative library path already stored by Arr instead of inventing a parallel title folder;
  • preserve the original release filename;
  • fail closed on ambiguous/missing identities and on Arr paths outside the configured symlink-library root.

Scope

This branch is rebuilt from the current develop and is limited to the provider-reconciliation flow already present upstream. It contains one commit and changes only:

  • src/services/providerReconciliation.ts
  • tests/e2e/provider-reconciliation-three-inputs.test.ts
  • docs/provider-reconciliation.md

It does not contain Media Manager, generated dist/ output, deployment files, Docker changes, workflows, screenshots or fork-specific artifacts.

Validation

  • Typecheck: passed
  • Full unit/e2e/regression suite: 162 passed, 0 failed
  • Focused provider-reconciliation E2E: 8 passed
  • git diff --check: passed

@moderniselife moderniselife left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @samtruman, another really useful one! Matching existing Radarr/Sonarr entries by TMDb ID before falling back to title matching should make reconciliation a lot more reliable, and centralising the canonical Movies/TV path construction is a nice cleanup.

One thing needs sorting before this can be reviewed properly: the branch looks like it was cut from your fork's integration branch rather than from develop, so the PR currently carries 171 commits and 257 files (+28k lines). That includes dist/ build output, artifacts/media-manager-ux-live/ screenshots, several planning/assessment markdown files, Dockerfile and docker-compose changes, and edits to .github/workflows/ (including a new upstream-update-alert.yml). It also has merge conflicts with develop.

Could you rebase or cherry-pick just the TMDb path-alignment commits onto the current develop, so the diff only contains the Organizer/Arr reconciliation changes and their tests? You mentioned the TypeScript pre-commit check didn't finish, so it'd be great to confirm the type check passes once it's rebased too.

I've passed it to Joe for a look in the meantime. Thanks again for all the work you've been putting into SchroDrive!

@moderniselife moderniselife added the needs-joe Needs a decision from the maintainer label Oct 15, 2026
@moderniselife

Copy link
Copy Markdown
Owner

Thanks for the new commits, @samtruman! The Dolby Vision ranking and season-pack fixes look like useful improvements. Just a heads-up that the branch still carries the full fork history (now 176 commits / 258 files, including dist/, Docker and .github/workflows changes) and still conflicts with develop, so the earlier review still applies. When you get a chance, a branch with only the TMDb/Arr path commits rebased onto develop would make this much easier to review, and the other fixes could come as their own small PRs. Thanks again for all your work on SchroDrive!

@moderniselife

moderniselife commented Oct 15, 2026 •

Copy link
Copy Markdown
Owner

Another thing,

I would just like to check if you are ready for this PR to come in or if you are still working on it (I can see you adding commits still)

If you are ready - I would ask you resolve the conflicts first and I'll merge it in after that.

If you arent ready - that's okay, I will add the not ready badge to the PR so I don't merge it in.

@moderniselife

Copy link
Copy Markdown
Owner

If the 176 commits are intentional, we can work together to get those squashed down into a more sensible count for a PR and cleaning up what gets pulled in (IE git ignore the dist and artifacts folders)

@samtruman
samtruman force-pushed the pr/tmdb-arr-path-reconciliation branch from 92e1032 to 6fc7cbf Compare October 16, 2026 09:35
@samtruman samtruman changed the title fix(arr): align library paths through TMDb identity fix(arr): use authoritative library paths for provider reconciliation Oct 16, 2026
@samtruman

Copy link
Copy Markdown
Contributor Author

Thanks for flagging this, and apologies for the noisy branch.

My fork contains a separate Media Manager subsystem that is not currently part of upstream SchroDrive. I mistakenly reused my production integration branch as the head of this PR. As a result, subsequent Media Manager work — including technical and Dolby Vision ranking, ffprobe comparison evidence, season-pack review logic, aggregate review UI, and deployment artifacts — was unintentionally included in #138.

Those changes cannot be reviewed independently in their current form because they depend on the Media Manager subsystem. They should not have been mixed into an upstream PR concerning the existing provider-reconciliation flow.

I have now rebuilt #138 from the current upstream develop. It contains one commit and three files, limited to functionality already present upstream. It also incorporates the relevant fixes discovered after the original submission:

  • matching against Arr title, original title, alternate titles, and year;
  • fallback through Arr’s own metadata lookup and TMDb identity;
  • use of the authoritative Radarr/Sonarr library path;
  • padded season directories;
  • rejection of paths outside the configured symlink-library root.

The previous integrated branch has been preserved only as an archive in my PR fork, so no work has been lost, but it is no longer part of this PR.

Going forward, any functionality that is genuinely applicable upstream will be proposed separately, based directly on upstream develop, and without Media Manager, deployment, generated artifacts, or fork-specific dependencies.

The cleaned branch is ready for review:

  • typecheck passed;
  • complete unit/e2e/regression suite: 162/162 passed;
  • focused provider-reconciliation E2E suite: 8/8 passed;
  • git diff --check passed.

@moderniselife moderniselife left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for rebuilding this, @samtruman, and for the clear write-up! This is now exactly the kind of PR that's easy to review: one commit, three files, based on current develop, no conflicts, and test/guard/GitGuardian are all green.

The change itself reads well. Resolving the existing Radarr/Sonarr entity first and placing the link under Arr's own stored path is a much more robust approach than building a parallel title folder, and matching on original/alternate titles plus the TMDb fallback via Arr's lookup should cut down on misses. Failing closed on ambiguous matches and on paths outside the symlink library is a nice safety touch, and B4-B6 cover the new paths well.

Two small, non-blocking notes for awareness. First, the library-root check compares Arr's stored path directly against symlinkLibraryPath, so it assumes Arr and SchröDrive see that library at the same path. Setups where the containers mount it at different paths will now have items deferred to Review rather than linked. That's the safe outcome, but it might be worth a line in the docs. Second, season folders change from Season 1 to Season 01 (matching Sonarr's default), and shows without a parsed season now link into the series root. Both seem sensible, just flagging them as behaviour changes.

This looks good to me, and it addresses the earlier review. I've passed it to Joe for a final look.

@moderniselife
moderniselife merged commit a30011a into moderniselife:develop Oct 18, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-joe Needs a decision from the maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants