This repository was archived by the owner on Jun 14, 2026. It is now read-only.
feat: add arr injection - #1374
Open
Audionut wants to merge 4 commits into
Open
Conversation
|
Thanks for taking the time to contribute to this project. Upload Assistant is currently in a complete rewrite, and no new development is being conducted on this python source at this time. If you have come this far, please feel free to leave open, any pull requests regarding new sites being added to the source, as these can serve as the baseline for later conversion. If your pull request relates to a critical bug, this will be addressed in this code base, and a new release published as needed. If your pull request only addresses a quite minor bug, it is not likely to be addressed in this code base. Details for the new code base will follow at a later date. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds Radarr/Sonarr add orchestration (CLI flags, config, validator, docs), Radarr/Sonarr API managers, a new RMC tracker and tracker infra updates, ARR orchestration and persistence, main-queue integration, an interactive Radarr MKV CLI, and UI config updates. ChangesRadarr/Sonarr Add Workflows
sequenceDiagram
participant Queue as Upload Queue
participant ArrOrch as ARR Orchestrator (process_*_add)
participant TrackerDM as TrackerDataManager
participant RadarrMgr as RadarrManager
participant SonarrMgr as SonarrManager
participant RadarrAPI as Radarr instance
participant SonarrAPI as Sonarr instance
Queue->>ArrOrch: receive queue item (meta, path)
ArrOrch->>TrackerDM: request tracker-derived IDs for Clients
TrackerDM-->>ArrOrch: TMDb/IMDb/TVDb IDs (or empty)
ArrOrch->>RadarrMgr: add_movie_by_ids(tmdb_id/imdb_id)
RadarrMgr->>RadarrAPI: GET /api/v3/movie (existing)
RadarrMgr->>RadarrAPI: GET /api/v3/movie/lookup (if needed)
RadarrMgr->>RadarrAPI: POST /api/v3/movie (add)
RadarrMgr-->>ArrOrch: {status, detail}
ArrOrch->>SonarrMgr: add_series_by_ids(tvdb_id/imdb_id/tmdb_id)
SonarrMgr->>SonarrAPI: GET /api/v3/series (existing)
SonarrMgr->>SonarrAPI: GET /api/v3/series/lookup (if needed)
SonarrMgr->>SonarrAPI: POST /api/v3/series (add)
SonarrMgr-->>ArrOrch: {status, detail}
ArrOrch-->>Queue: record result (seen-keys/unable-log) and continue
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Suggested Reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 8
🧹 Nitpick comments (3)
radarr_add_mkv_directory.py (3)
297-304: 💤 Low value
find_mkv_filesis unused.This function is defined but never called. The script uses
find_content_itemsinstead, which handles both MKV files and disc folders. Consider removing this dead code.🤖 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 `@radarr_add_mkv_directory.py` around lines 297 - 304, The function find_mkv_files is dead code (never called) while the script already uses find_content_items to handle MKV files and disc folders; remove the unused function definition (find_mkv_files) and any references to helper is_sample_mkv only if it becomes unused as a result, or alternatively replace callers that should use it with find_mkv_files if you intended to use this logic—locate the function named find_mkv_files and either delete it and clean up associated unused symbols (e.g., is_sample_mkv) or update the callers to invoke find_mkv_files instead of find_content_items so there are no unused definitions left.
231-269: ⚡ Quick winConsider validating URL scheme to prevent unintended file access.
The
base_urlis user-provided and passed directly tourllib.request. While this is a CLI tool where the user controls input, validating that the scheme ishttporhttpswould prevent accidental misuse (e.g.,file://URLs reading local files).🛡️ Proposed validation in __init__ or _request_json
def normalize_base_url(value: str) -> str: - return value.rstrip("/") + normalized = value.rstrip("/") + parsed = urllib.parse.urlparse(normalized) + if parsed.scheme not in ("http", "https"): + raise ValueError(f"Radarr URL must use http or https scheme, got: {parsed.scheme or 'none'}") + return normalized🤖 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 `@radarr_add_mkv_directory.py` around lines 231 - 269, Validate that the user-provided base_url uses an allowed scheme (http or https) before making requests: in the Radarr client __init__ (or at the start of _request_json) parse self.base_url with urllib.parse.urlparse and if parsed.scheme not in ("http", "https") raise RadarrError (or ValueError) with a clear message; this prevents accidental file:// or other schemes from being used and should be done before building the request URL in _request_json.
625-657: 💤 Low value
refresh_item_statusesis unused.This function is defined but never called in the script. It appears to be designed for a batch workflow that isn't implemented. Consider removing this dead code or adding a comment if it's intended for future use.
🤖 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 `@radarr_add_mkv_directory.py` around lines 625 - 657, The function refresh_item_statuses (working on MovieItem and using movie_key) is defined but never invoked; either delete this dead code or keep it with a clear comment explaining it's intentionally unused for a future batch workflow (e.g., "reserved for batch processing" and link to design), and if you keep it, add a unit test or TODO with a FIXME tag referencing refresh_item_statuses and MovieItem to prevent accidental removal; ensure no other code references this function before deleting or marking it.
🤖 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/arr_add.py`:
- Around line 564-570: The except blocks that catch failures from
Clients(config).get_pathed_torrents currently return True (e.g., the try/except
around Clients(config).get_pathed_torrents(path, meta) and the similar block
later), which causes upload.py to treat these as processed; change those except
handlers to return False (or re-raise) instead so qBittorrent lookup outages are
not marked completed and items will be retried on resume; locate the handlers by
the call to get_pathed_torrents and replace the final "return True" in those
except blocks with "return False".
- Around line 270-279: The helper _sonarr_add_first_seen_key currently skips
checking the "title" key whenever duplicate_keys contains a "tvdb_id" or "year",
which loses previously seen title-only duplicates; change the key selection so
"title" is always considered as a fallback (i.e., if any of ("tvdb_id","year")
exist, iterate those first but then also include "title" afterwards), keeping
existing behavior of checking keys in sorted(duplicate_keys) and returning the
first key that is in seen_keys; adjust the key_types construction to ensure
"title" is appended when needed so title-only duplicates are not missed.
- Around line 676-678: The early return in the tracker_ids check (if not
tracker_ids) skips recording the miss to unable_log_file, so update the block in
src/arr_add.py to write the title/path into unable_log_file before returning;
specifically, inside the if not tracker_ids branch (where tracker_ids, path,
console.print and os.path.basename are used) append the same entry format used
by the "no TVDb ID" path to unable_log_file (using the existing unable_log_file
variable/handle) and flush/close if required, then keep the console.print and
return True.
In `@src/radarr.py`:
- Around line 171-245: The loop over Radarr instances in the add flow currently
returns immediately on the first "exists" or "added" result, preventing later
instances from being processed; change the logic in the for instance in
instances loop so you collect per-instance results instead of returning early:
call _existing_movie, _lookup_movie_by_ids/_lookup_movie_by_filename and
_request_json for each instance, append each instance's outcome (including
status, detail, movie, used_filename_fallback, and label) into a results list,
and only after the loop decide what to return (e.g., succeed if any "added", or
return aggregated "exists" info, or the last error). Update uses of
_movie_label, _coerce_optional_int, and error handling accordingly so errors for
one instance don’t short-circuit the loop.
In `@src/sonarr.py`:
- Around line 258-325: The add_series_by_ids function currently returns
immediately on the first "exists" or "added" result, which prevents iterating
remaining Sonarr instances; change add_series_by_ids to collect per-instance
outcomes instead of returning early: introduce a results list (or dict) and for
each instance (inside the loop that calls _existing_series,
_lookup_series_by_tvdb_id, _lookup_series_by_term, _prepare_add_payload,
_request_json) append the instance label and its
status/detail/used_title_fallback, continue to the next instance on success or
failure, and after the loop return an aggregated summary (e.g., if any instance
added -> status "added" with per-instance details, else if any exists ->
"exists", else "failed" with last_error and per-instance errors). Ensure you
remove the early return statements and preserve existing exception handling for
httpx.HTTPStatusError and httpx.RequestError.
In `@src/trackers/RMC.py`:
- Around line 33-44: The function get_category_id currently always returns
{'category_id': ...} instead of returning the name->ID map when
mapping_only=True; update get_category_id so that when mapping_only is True it
returns the full mapping dict (e.g. {'MOVIE': '1', ...}) and when mapping_only
is False it returns {'category_id': <id>} computed from meta['category']; apply
the same change to the corresponding get_resolution_id implementation (the block
around the other method at lines 114-134) so TRACKER_SETUP.tracker_request() and
check_tracker_claims() receive the name→ID maps they expect.
In `@upload.py`:
- Around line 1617-1628: The code increments processed_files_count before
knowing if a Radarr add actually succeeded, causing -lq to count attempts
instead of successful adds; modify the logic around process_radarr_add so that
processed_files_count (or the value used to evaluate
limit_queue_value/reached_limit) is only incremented when radarr_completed is
true (i.e., a real add occurred) or alternatively compute reached_limit from
(processed_files_count - skipped_files_count) like other upload modes; update
both the shown branch (where process_radarr_add is called) and the similar block
at lines 1632-1643 to use the successful-add count for limit checks rather than
raw processed_files_count.
In `@web_ui/static/js/config_app.js`:
- Around line 1159-1160: The subgroup mapping for the Sonarr and Radarr entries
is missing the per-instance override keys, so add the ARR `_1` keys to their
lists: ensure the 'Sonarr' mapping includes 'sonarr_url_1' and
'sonarr_api_key_1' and the 'Radarr' mapping includes 'radarr_url_1' and
'radarr_api_key_1' so these override fields appear in the Sonarr/Radarr subgroup
UI; update the mapping where the 'Sonarr' and 'Radarr' arrays are defined in
config_app.js to include those exact string keys.
---
Nitpick comments:
In `@radarr_add_mkv_directory.py`:
- Around line 297-304: The function find_mkv_files is dead code (never called)
while the script already uses find_content_items to handle MKV files and disc
folders; remove the unused function definition (find_mkv_files) and any
references to helper is_sample_mkv only if it becomes unused as a result, or
alternatively replace callers that should use it with find_mkv_files if you
intended to use this logic—locate the function named find_mkv_files and either
delete it and clean up associated unused symbols (e.g., is_sample_mkv) or update
the callers to invoke find_mkv_files instead of find_content_items so there are
no unused definitions left.
- Around line 231-269: Validate that the user-provided base_url uses an allowed
scheme (http or https) before making requests: in the Radarr client __init__ (or
at the start of _request_json) parse self.base_url with urllib.parse.urlparse
and if parsed.scheme not in ("http", "https") raise RadarrError (or ValueError)
with a clear message; this prevents accidental file:// or other schemes from
being used and should be done before building the request URL in _request_json.
- Around line 625-657: The function refresh_item_statuses (working on MovieItem
and using movie_key) is defined but never invoked; either delete this dead code
or keep it with a clear comment explaining it's intentionally unused for a
future batch workflow (e.g., "reserved for batch processing" and link to
design), and if you keep it, add a unit test or TODO with a FIXME tag
referencing refresh_item_statuses and MovieItem to prevent accidental removal;
ensure no other code references this function before deleting or marking it.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f9203820-022a-4229-8cbb-8be7a72d56d4
📒 Files selected for processing (16)
data/example-config.pydocs/example-config.mdradarr_add_mkv_directory.pysrc/args.pysrc/arr_add.pysrc/configvalidator.pysrc/get_tracker_data.pysrc/radarr.pysrc/sonarr.pysrc/torrent_clients/qbittorrent.pysrc/trackermeta.pysrc/trackers/COMMON.pysrc/trackers/RMC.pysrc/trackersetup.pyupload.pyweb_ui/static/js/config_app.js
Comment on lines
+564
to
+570
| try: | ||
| await Clients(config).get_pathed_torrents(path, meta) | ||
| except Exception as e: | ||
| console.print(f"[red]Radarr add skipped: qBittorrent search failed for {path}: {e}[/red]") | ||
| if meta.get('debug', False): | ||
| console.print(traceback.format_exc()) | ||
| return True |
There was a problem hiding this comment.
Don't mark qBittorrent lookup failures as completed work.
Both exception paths return True, but upload.py treats a truthy result as “processed” and writes the item into the queue log. A temporary qBittorrent outage will therefore drop items from future resumes instead of retrying them.
Suggested fix
try:
await Clients(config).get_pathed_torrents(path, meta)
except Exception as e:
console.print(f"[red]Radarr add skipped: qBittorrent search failed for {path}: {e}[/red]")
if meta.get('debug', False):
console.print(traceback.format_exc())
- return True
+ return False try:
await Clients(config).get_pathed_torrents(path, meta)
except Exception as e:
console.print(f"[red]Sonarr add skipped: qBittorrent search failed for {path}: {e}[/red]")
if meta.get('debug', False):
console.print(traceback.format_exc())
- return True
+ return FalseAlso applies to: 663-669
🧰 Tools
🪛 Ruff (0.15.12)
[warning] 566-566: Do not catch blind exception: Exception
(BLE001)
🤖 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/arr_add.py` around lines 564 - 570, The except blocks that catch failures
from Clients(config).get_pathed_torrents currently return True (e.g., the
try/except around Clients(config).get_pathed_torrents(path, meta) and the
similar block later), which causes upload.py to treat these as processed; change
those except handlers to return False (or re-raise) instead so qBittorrent
lookup outages are not marked completed and items will be retried on resume;
locate the handlers by the call to get_pathed_torrents and replace the final
"return True" in those except blocks with "return False".
Comment on lines
+33
to
+44
| async def get_category_id( | ||
| self, | ||
| meta: Meta, | ||
| category: Optional[str] = None, | ||
| reverse: bool = False, | ||
| mapping_only: bool = False, | ||
| ) -> dict[str, str]: | ||
| _ = (category, reverse, mapping_only) | ||
| category_id = { | ||
| 'MOVIE': '1', | ||
| }.get(meta['category'], '0') | ||
| return {'category_id': category_id} |
There was a problem hiding this comment.
Return name-to-ID maps for mapping_only=True.
TRACKER_SETUP.tracker_request() and check_tracker_claims() call these helpers with mapping_only=True to translate meta["category"] and meta["resolution"] into tracker IDs. Right now both methods always return {"category_id": ...} / {"resolution_id": ...}, so RMC resolves None for those lookups and its request/claim matching never reaches exact matches.
Proposed fix
async def get_category_id(
self,
meta: Meta,
category: Optional[str] = None,
reverse: bool = False,
mapping_only: bool = False,
) -> dict[str, str]:
- _ = (category, reverse, mapping_only)
- category_id = {
+ mapping = {
'MOVIE': '1',
- }.get(meta['category'], '0')
+ }
+ if mapping_only:
+ return mapping
+ if reverse:
+ return {value: key for key, value in mapping.items()}
+ category_value = str(category if category is not None else meta['category']).upper()
+ category_id = mapping.get(category_value, '0')
return {'category_id': category_id}
@@
async def get_resolution_id(
self,
meta: Meta,
resolution: Optional[str] = None,
reverse: bool = False,
mapping_only: bool = False,
) -> dict[str, str]:
- _ = (resolution, reverse, mapping_only)
- resolution_id = {
+ mapping = {
'4320p': '1',
'2160p': '2',
'1440p': '3',
'1080p': '3',
'1080i': '4',
'720p': '5',
'576p': '6',
'576i': '7',
'480p': '8',
'480i': '9'
- }.get(meta['resolution'], '11')
+ }
+ if mapping_only:
+ return mapping
+ if reverse:
+ return {value: key for key, value in mapping.items()}
+ resolution_value = str(resolution if resolution is not None else meta['resolution'])
+ resolution_id = mapping.get(resolution_value, '11')
return {'resolution_id': resolution_id}Also applies to: 114-134
🤖 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/trackers/RMC.py` around lines 33 - 44, The function get_category_id
currently always returns {'category_id': ...} instead of returning the name->ID
map when mapping_only=True; update get_category_id so that when mapping_only is
True it returns the full mapping dict (e.g. {'MOVIE': '1', ...}) and when
mapping_only is False it returns {'category_id': <id>} computed from
meta['category']; apply the same change to the corresponding get_resolution_id
implementation (the block around the other method at lines 114-134) so
TRACKER_SETUP.tracker_request() and check_tracker_claims() receive the name→ID
maps they expect.
Owner
Author
There was a problem hiding this comment.
Irrelevant as there is not request url which tripper the mapping only pathway
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how mapping_only is used and whether RMC has a request_url
rg -n "mapping_only" --type py -C 3
Length of output: 39025
@Audionut, understood — since there's no request/search URL configured for RMC that would route through check_tracker_claims() or tracker_request(), the mapping_only=True branch in trackersetup.py is never reached for this tracker. I can see that same _ = (category, reverse, mapping_only) suppression pattern is used across many other trackers in the codebase (e.g., UNIT3D_TEMPLATE, UTP, TLZ, TOS, SP, etc.) for the same reason. Dismissing the concern.
✏️ Learnings added
Learnt from: Audionut
Repo: Audionut/Upload-Assistant PR: 1374
File: src/trackers/RMC.py:33-44
Timestamp: 2026-05-19T07:46:14.597Z
Learning: In `src/trackers/RMC.py`, RMC does not have a request/search URL configured that would route through `check_tracker_claims()` or `tracker_request()` in `src/trackersetup.py`. As a result, the `mapping_only=True` pathway is never triggered for RMC, and using `_ = (category, reverse, mapping_only)` to suppress the flag in `get_category_id` and `get_resolution_id` is intentional and correct for this tracker. Do not flag this pattern as a bug for RMC.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: Audionut
Repo: Audionut/Upload-Assistant PR: 1025
File: src/trackers/TVC.py:32-86
Timestamp: 2025-12-14T00:20:21.219Z
Learning: In src/trackers/TVC.py, the genres argument passed to get_cat_id() is a string (comma-separated) rather than a list or other type. Remove or avoid code paths that handle list inputs for this parameter; assume and validate string input, adjust type hints if present, and, if possible, add a unit test to ensure a string is passed.
Learnt from: Audionut
Repo: Audionut/Upload-Assistant PR: 1081
File: src/trackers/CBR.py:84-91
Timestamp: 2025-12-27T13:04:08.659Z
Learning: In tracker modules under src/trackers, meta['aka'] is a guaranteed key and can be accessed directly (meta['aka']) without using get(). This improves clarity and small performance overhead. Only rely on this when you are certain the key is always present; if a tracker could ever omit it, add a guard or a clear default to avoid KeyError.
Learnt from: Audionut
Repo: Audionut/Upload-Assistant PR: 1081
File: src/trackers/CBR.py:94-94
Timestamp: 2025-12-27T13:04:22.071Z
Learning: In Python codebases where a dict (e.g., meta) is guaranteed to always include a specific key (e.g., 'tag'), direct dictionary access meta['tag'] is safe and preferred over meta.get('tag') for performance and readability. Ensure the key is indeed always initialized in all code paths; if there's any doubt, consider defensive checks or documentation to avoid KeyError.
Learnt from: Audionut
Repo: Audionut/Upload-Assistant PR: 1114
File: src/manualpackage.py:16-23
Timestamp: 2026-01-11T05:07:09.469Z
Learning: In Python type hints for read-only mappings (e.g., DEFAULT_CONFIG, TRACKER_CONFIG), prefer Mapping[str, Any] over Dict[str, Any] to signal immutability intent. Use from typing import Mapping; annotate with Mapping[str, Any] while allowing concrete dicts at runtime. Do not rely on isinstance(value, dict) for immutability checks; this is a type-hint convention, not a runtime guarantee.
Learnt from: Audionut
Repo: Audionut/Upload-Assistant PR: 1114
File: src/manualpackage.py:16-23
Timestamp: 2026-01-11T05:07:09.469Z
Learning: In the Upload-Assistant repository, avoid flagging minor or stylistic issues (e.g., ValueError vs TypeError for type validation) unless they represent actual bugs or significant problems. Follow Audionut's preference to focus on substantive issues rather than minor concerns across Python files.
Learnt from: wasserrutschentester
Repo: Audionut/Upload-Assistant PR: 1250
File: src/trackers/RHD.py:269-271
Timestamp: 2026-02-13T20:15:30.786Z
Learning: In src/trackers/RHD.py, the 'GERMAN SUBBED' tag is a standalone tag that overwrites audio_lang_str when German subtitles exist without German audio. It indicates 'OV (original version) audio + German subtitles' according to RocketHD naming conventions, so the audio language information should be overwritten, not appended. Apply this rule consistently to similar trackers and ensure the tag replaces existing language metadata rather than augmenting it.
Comment on lines
+1617
to
+1628
| processed_files_count += 1 | ||
| radarr_completed = await process_radarr_add(meta, base_dir, radarr_add_seen_title_years, radarr_add_seen_key_file, radarr_add_unable_log_file, config, name_manager) | ||
| if radarr_completed and log_file and (not meta['debug'] or "debug" in os.path.basename(log_file)): | ||
| await save_processed_file(log_file, current_item_path) | ||
| console.print(f"[cyan]Processed {processed_files_count}/{total_files} files for Radarr add.") | ||
| limit_queue_value = int(meta.get('limit_queue', 0) or 0) | ||
| reached_limit = limit_queue_value > 0 and processed_files_count >= limit_queue_value | ||
| await cleanup_manager.cleanup() | ||
| gc.collect() | ||
| cleanup_manager.reset_terminal() | ||
| if reached_limit: | ||
| break |
There was a problem hiding this comment.
-lq is counting ARR attempts, not successful adds.
These branches increment the queue counter before they know whether anything was actually added, then break on raw processed count. The rest of upload.py uses successful work (processed_files_count - skipped_files_count) for -lq, so ARR mode can stop far short of the requested number of real additions.
Also applies to: 1632-1643
🤖 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 `@upload.py` around lines 1617 - 1628, The code increments
processed_files_count before knowing if a Radarr add actually succeeded, causing
-lq to count attempts instead of successful adds; modify the logic around
process_radarr_add so that processed_files_count (or the value used to evaluate
limit_queue_value/reached_limit) is only incremented when radarr_completed is
true (i.e., a real add occurred) or alternatively compute reached_limit from
(processed_files_count - skipped_files_count) like other upload modes; update
both the shown branch (where process_radarr_add is called) and the similar block
at lines 1632-1643 to use the successful-add count for limit checks rather than
raw processed_files_count.
Owner
Author
There was a problem hiding this comment.
for the purpose of this pr, this is fine
There was a problem hiding this comment.
@Audionut, understood — acknowledged and moving on.
Would you like me to open a follow-up issue to revisit the -lq counting behaviour for ARR add mode in a future PR, or would you prefer to leave it as-is?
🧠 Learnings used
Learnt from: Audionut
Repo: Audionut/Upload-Assistant PR: 1081
File: src/trackers/CBR.py:94-94
Timestamp: 2025-12-27T13:04:22.071Z
Learning: In Python codebases where a dict (e.g., meta) is guaranteed to always include a specific key (e.g., 'tag'), direct dictionary access meta['tag'] is safe and preferred over meta.get('tag') for performance and readability. Ensure the key is indeed always initialized in all code paths; if there's any doubt, consider defensive checks or documentation to avoid KeyError.
Learnt from: Audionut
Repo: Audionut/Upload-Assistant PR: 1114
File: src/manualpackage.py:16-23
Timestamp: 2026-01-11T05:07:09.469Z
Learning: In Python type hints for read-only mappings (e.g., DEFAULT_CONFIG, TRACKER_CONFIG), prefer Mapping[str, Any] over Dict[str, Any] to signal immutability intent. Use from typing import Mapping; annotate with Mapping[str, Any] while allowing concrete dicts at runtime. Do not rely on isinstance(value, dict) for immutability checks; this is a type-hint convention, not a runtime guarantee.
Learnt from: Audionut
Repo: Audionut/Upload-Assistant PR: 1114
File: src/manualpackage.py:16-23
Timestamp: 2026-01-11T05:07:09.469Z
Learning: In the Upload-Assistant repository, avoid flagging minor or stylistic issues (e.g., ValueError vs TypeError for type validation) unless they represent actual bugs or significant problems. Follow Audionut's preference to focus on substantive issues rather than minor concerns across Python files.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
src/sonarr.py (2)
206-228:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDon't stop at the first Sonarr instance.
The new multi-instance flow still returns on the first
existsoraddedresult, so a match in one Sonarr instance prevents later configured instances from being checked or updated.Also applies to: 257-292
🤖 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/sonarr.py` around lines 206 - 228, The loop over Sonarr "instances" currently returns immediately when an "exists" (and similarly an "added") result is found, preventing later instances from being checked; instead, in the method containing the for instance in instances loop and calls to self._lookup_series_by_term and self._existing_series (and the corresponding add flow), remove the in-loop return and collect per-instance outcomes (include instance label and status) into a results list, continue iterating all instances, and after the loop evaluate/aggregate those results (e.g., prefer returning an "exists" match if any exist, otherwise an "added" if any were added, or a consolidated list of per-instance statuses) so every configured Sonarr instance is checked/updated; ensure you update both the "exists" branch (where self._series_label(existing) is used) and the analogous "added" branch to append to results rather than returning immediately.
131-137:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRequire an exact TVDb match before adding.
If Sonarr returns a non-empty result set but none of the entries match
tvdb_id, this falls back toitems[0]. That can add the wrong series whenever the lookup response is approximate instead of exact.Suggested fix
if isinstance(data, list): items = cast(list[dict[str, Any]], data) for item in items: if str(item.get("tvdbId") or "") == str(tvdb_id): return item - return items[0] if items else None + return None🤖 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/sonarr.py` around lines 131 - 137, The current logic in the Sonarr lookup treats a list response by returning items[0] when no entry matches the requested tvdb_id, which can add the wrong series; in the block handling "if isinstance(data, list)" (variables: data, items, tvdb_id), change the behavior to only return a matching item when str(item.get("tvdbId") or "") == str(tvdb_id) and otherwise return None (do not fall back to items[0]), so the caller receives no match unless an exact TVDb ID match is found.src/arr_add.py (1)
565-571:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRetry qBittorrent outages instead of marking the item done.
Both exception paths return
True, but the queue treats a truthy result as processed. A transient qBittorrent failure will therefore be written off instead of being retried on resume.Suggested fix
try: await Clients(config).get_pathed_torrents(path, meta) except Exception as e: console.print(f"[red]Radarr add skipped: qBittorrent search failed for {path}: {e}[/red]") if meta.get('debug', False): console.print(traceback.format_exc()) - return True + return Falsetry: await Clients(config).get_pathed_torrents(path, meta) except Exception as e: console.print(f"[red]Sonarr add skipped: qBittorrent search failed for {path}: {e}[/red]") if meta.get('debug', False): console.print(traceback.format_exc()) - return True + return FalseAlso applies to: 659-665
🤖 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/arr_add.py` around lines 565 - 571, The exception handler around Clients(config).get_pathed_torrents(...) is swallowing transient qBittorrent errors by printing and returning True, which signals the queue the item is processed; instead, do not return True on exception — either re-raise the caught exception or return a falsy failure value so the queue will retry. Update the except block in this try/except (and the analogous block around lines 659-665) to log the error and debug traceback as currently done, then raise the exception (or return False) rather than returning True; reference Clients.get_pathed_torrents and the surrounding try/except to locate the change.src/radarr.py (1)
171-212:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftProcess every configured Radarr instance before returning.
Both loops still return on the first
existsoraddedresult. That keeps the new multi-instance workflow in a "first match wins" state, so one Radarr library can block later configured libraries from being checked or populated.Also applies to: 235-256
🤖 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/radarr.py` around lines 171 - 212, The loop over Radarr instances returns immediately on the first "exists" or "added" result, preventing later instances from being checked; to fix, remove the in-loop returns in the method that calls self._existing_movie / self._lookup_movie_by_ids / self._request_json and instead collect per-instance results into a list (e.g., results = []), append structured outcome entries (including status, detail, movie info when present, and instance label) when an instance yields "exists" or "added", continue iterating all instances, and after the loop decide what to return (e.g., prefer any "exists" or "added" results but only after processing all instances, and fall back to the last_error if none). Apply the same change to the other loop region that uses the same logic (the block around lines 235-256), preserving use of helper symbols _existing_movie, _lookup_movie_by_ids, _request_json, and _movie_label to build result entries.
🤖 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/radarr.py`:
- Around line 123-135: The lookup loop in the method using lookup_candidates
currently catches all httpx.HTTPStatusError and returns the first list item
unconditionally; change it to only ignore 404/NotFound errors and re-raise or
surface other HTTPStatusError exceptions from _request_json, and when data is a
list filter items for a true identifier match (compare returned item fields like
"tmdbId" and "imdbId" against the requested ID present in params or the
lookup_candidate) before returning — only return an item when its tmdbId/imdbId
matches the requested ID; do not return items[0] from fuzzy lookup results.
---
Duplicate comments:
In `@src/arr_add.py`:
- Around line 565-571: The exception handler around
Clients(config).get_pathed_torrents(...) is swallowing transient qBittorrent
errors by printing and returning True, which signals the queue the item is
processed; instead, do not return True on exception — either re-raise the caught
exception or return a falsy failure value so the queue will retry. Update the
except block in this try/except (and the analogous block around lines 659-665)
to log the error and debug traceback as currently done, then raise the exception
(or return False) rather than returning True; reference
Clients.get_pathed_torrents and the surrounding try/except to locate the change.
In `@src/radarr.py`:
- Around line 171-212: The loop over Radarr instances returns immediately on the
first "exists" or "added" result, preventing later instances from being checked;
to fix, remove the in-loop returns in the method that calls self._existing_movie
/ self._lookup_movie_by_ids / self._request_json and instead collect
per-instance results into a list (e.g., results = []), append structured outcome
entries (including status, detail, movie info when present, and instance label)
when an instance yields "exists" or "added", continue iterating all instances,
and after the loop decide what to return (e.g., prefer any "exists" or "added"
results but only after processing all instances, and fall back to the last_error
if none). Apply the same change to the other loop region that uses the same
logic (the block around lines 235-256), preserving use of helper symbols
_existing_movie, _lookup_movie_by_ids, _request_json, and _movie_label to build
result entries.
In `@src/sonarr.py`:
- Around line 206-228: The loop over Sonarr "instances" currently returns
immediately when an "exists" (and similarly an "added") result is found,
preventing later instances from being checked; instead, in the method containing
the for instance in instances loop and calls to self._lookup_series_by_term and
self._existing_series (and the corresponding add flow), remove the in-loop
return and collect per-instance outcomes (include instance label and status)
into a results list, continue iterating all instances, and after the loop
evaluate/aggregate those results (e.g., prefer returning an "exists" match if
any exist, otherwise an "added" if any were added, or a consolidated list of
per-instance statuses) so every configured Sonarr instance is checked/updated;
ensure you update both the "exists" branch (where self._series_label(existing)
is used) and the analogous "added" branch to append to results rather than
returning immediately.
- Around line 131-137: The current logic in the Sonarr lookup treats a list
response by returning items[0] when no entry matches the requested tvdb_id,
which can add the wrong series; in the block handling "if isinstance(data,
list)" (variables: data, items, tvdb_id), change the behavior to only return a
matching item when str(item.get("tvdbId") or "") == str(tvdb_id) and otherwise
return None (do not fall back to items[0]), so the caller receives no match
unless an exact TVDb ID match is found.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 61349feb-b2fa-47ad-ac4d-27ed1b2f1316
📒 Files selected for processing (3)
src/arr_add.pysrc/radarr.pysrc/sonarr.py
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
autobrr/qui#1882 adds the ability to query alternate title names from *arrs, to help improve cross-seed matching. However, this only works if the title is in the arr.
This pr add an option to add matching titles from your directory content to the *arrs.
Suggested commands:
python3 upload.py "/movies" --queue radarr --radarr-addpython3 upload.py "/tv" --queue sonarr --sonarr-addNeeds the arr root folder paths added to config.
By default, it will add the content as
unmonitoredin the arr (no other functionality has been tested, or is supported). This provides the needed functionality for the qui pr, without causing issues from arr setups upgrading or whatever else.use_radarroruse_sonarris set to true, it run a lookup in the arr first, to skip any further searching for content already in the arr.get_tvdb_by_external_idfunction to get tvdb from tmdb/imdb. will print in the console if this has occurred.-lqarg works as expected, and interruptions will resume.Summary by CodeRabbit
New Features
Documentation