This repository was archived by the owner on Jun 14, 2026. It is now read-only.
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. |
📝 WalkthroughWalkthroughACM tracker refactored from standalone implementation into UNIT3D subclass with simplified field helpers returning mapping dictionaries, new origin-country validation gate, and integration updates to example config and upload.py language handling. ChangesACM Tracker Refactoring to UNIT3D Subclass
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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: 1
🧹 Nitpick comments (2)
src/trackers/ACM.py (2)
79-79: 💤 Low valueMinor: Use
Metatype alias for consistency.Other methods use
Metabut this one usesdict[str, Any]directly.- async def get_region_id(self, meta: dict[str, Any]) -> dict[str, str]: + async def get_region_id(self, meta: Meta) -> dict[str, str]:🤖 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/ACM.py` at line 79, The get_region_id method signature uses dict[str, Any] instead of the project type alias; change the parameter type to the existing Meta alias for consistency by updating the async def get_region_id(self, meta: Meta) -> dict[str, str] (referencing get_region_id in ACM.py) and ensure any imports or type references align with the Meta alias used by other methods in this module.
120-127: 💤 Low valueSimplify the
akaconditional.The explicit empty-string checks are redundant. Using truthiness is cleaner and idiomatic.
- if aka != "": + if aka: # ugly fix to remove the extra space in the title aka = aka + " " name = name.replace(aka, f" / {original_title} {chr(int('202A', 16))}") - elif aka == "": + else: if meta.get("title") != original_title:🤖 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/ACM.py` around lines 120 - 127, The code uses explicit empty-string checks for aka (if aka != "" / elif aka == ""); change this to a truthy check (if aka: else:) while preserving the existing behavior that appends a space and replaces name with the formatted string using original_title and chr(int('202A',16)), and in the else branch keep the meta title comparison (if meta.get("title") != original_title) and the replacement using meta['title']; update the branches in the same block where variables aka, name, original_title and meta are used.
🤖 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/trackers/ACM.py`:
- Around line 41-61: The get_resolution_id method returns three different dict
shapes depending on flags, breaking type safety and leaving dead/unsafe reverse
behavior; update by splitting responsibilities: create a new method (e.g.,
get_resolution_mapping) that returns mapping_only format dict[str,str] for
callers that need the full map, create a new method (e.g.,
get_resolution_by_meta) that returns the structured {"resolution_id": str} for
normal lookups, and either remove the reverse parameter from get_resolution_id
or replace it with a dedicated get_reverse_mapping method that clearly documents
and handles duplicate-value collisions; update callers (such as callers in
trackers setup) to call the appropriate new method and adjust type annotations
to explicit return types (dict[str,str] vs dict[str,str] with structured key) to
restore type safety.
---
Nitpick comments:
In `@src/trackers/ACM.py`:
- Line 79: The get_region_id method signature uses dict[str, Any] instead of the
project type alias; change the parameter type to the existing Meta alias for
consistency by updating the async def get_region_id(self, meta: Meta) ->
dict[str, str] (referencing get_region_id in ACM.py) and ensure any imports or
type references align with the Meta alias used by other methods in this module.
- Around line 120-127: The code uses explicit empty-string checks for aka (if
aka != "" / elif aka == ""); change this to a truthy check (if aka: else:) while
preserving the existing behavior that appends a space and replaces name with the
formatted string using original_title and chr(int('202A',16)), and in the else
branch keep the meta title comparison (if meta.get("title") != original_title)
and the replacement using meta['title']; update the branches in the same block
where variables aka, name, original_title and meta are used.
🪄 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: 24d8d6ee-29c6-4d80-8f4c-d7806a8273b2
📒 Files selected for processing (3)
data/example-config.pysrc/trackers/ACM.pyupload.py
💤 Files with no reviewable changes (1)
- data/example-config.py
Comment on lines
+41
to
+61
| async def get_resolution_id(self, meta: Meta, resolution: str = "", reverse: bool = False, mapping_only: bool = False) -> dict[str, str]: | ||
| resolution_id = { | ||
| '2160p': '1', | ||
| '1080p': '2', | ||
| '1080i': '2', | ||
| '720p': '3', | ||
| '576p': '4', | ||
| '576i': '4', | ||
| '480p': '5', | ||
| '480i': '5' | ||
| }.get(meta['resolution'], '10') | ||
| return resolution_id | ||
|
|
||
| # ACM rejects uploads with more that 10 keywords | ||
| async def get_keywords(self, meta: dict[str, Any]) -> str: | ||
| keywords: str = str(meta.get('keywords', '')) | ||
| if keywords != '': | ||
| keywords_list = keywords.split(',') | ||
| keywords_list = [keyword.strip() for keyword in keywords_list if " " not in keyword.strip()][:10] | ||
| keywords = ', '.join(keywords_list) | ||
| return keywords | ||
|
|
||
| def get_subtitles(self, meta: dict[str, Any]) -> list[str]: | ||
| sub_lang_map: dict[tuple[str, ...], str] = { | ||
| ("Arabic", "ara", "ar"): 'Ara', | ||
| ("Brazilian Portuguese", "Brazilian", "Portuguese-BR", 'pt-br'): 'Por-BR', | ||
| ("Bulgarian", "bul", "bg"): 'Bul', | ||
| ("Chinese", "chi", "zh", "Chinese (Simplified)", "Chinese (Traditional)"): 'Chi', | ||
| ("Croatian", "hrv", "hr", "scr"): 'Cro', | ||
| ("Czech", "cze", "cz", "cs"): 'Cze', | ||
| ("Danish", "dan", "da"): 'Dan', | ||
| ("Dutch", "dut", "nl"): 'Dut', | ||
| ("English", "eng", "en", "English (CC)", "English - SDH"): 'Eng', | ||
| ("English - Forced", "English (Forced)", "en (Forced)"): 'Eng', | ||
| ("English Intertitles", "English (Intertitles)", "English - Intertitles", "en (Intertitles)"): 'Eng', | ||
| ("Estonian", "est", "et"): 'Est', | ||
| ("Finnish", "fin", "fi"): 'Fin', | ||
| ("French", "fre", "fr"): 'Fre', | ||
| ("German", "ger", "de"): 'Ger', | ||
| ("Greek", "gre", "el"): 'Gre', | ||
| ("Hebrew", "heb", "he"): 'Heb', | ||
| ("Hindi", "hin", "hi"): 'Hin', | ||
| ("Hungarian", "hun", "hu"): 'Hun', | ||
| ("Icelandic", "ice", "is"): 'Ice', | ||
| ("Indonesian", "ind", "id"): 'Ind', | ||
| ("Italian", "ita", "it"): 'Ita', | ||
| ("Japanese", "jpn", "ja"): 'Jpn', | ||
| ("Korean", "kor", "ko"): 'Kor', | ||
| ("Latvian", "lav", "lv"): 'Lav', | ||
| ("Lithuanian", "lit", "lt"): 'Lit', | ||
| ("Norwegian", "nor", "no"): 'Nor', | ||
| ("Persian", "fa", "far"): 'Per', | ||
| ("Polish", "pol", "pl"): 'Pol', | ||
| ("Portuguese", "por", "pt"): 'Por', | ||
| ("Romanian", "rum", "ro"): 'Rom', | ||
| ("Russian", "rus", "ru"): 'Rus', | ||
| ("Serbian", "srp", "sr", "scc"): 'Ser', | ||
| ("Slovak", "slo", "sk"): 'Slo', | ||
| ("Slovenian", "slv", "sl"): 'Slv', | ||
| ("Spanish", "spa", "es"): 'Spa', | ||
| ("Swedish", "swe", "sv"): 'Swe', | ||
| ("Thai", "tha", "th"): 'Tha', | ||
| ("Turkish", "tur", "tr"): 'Tur', | ||
| ("Ukrainian", "ukr", "uk"): 'Ukr', | ||
| ("Vietnamese", "vie", "vi"): 'Vie', | ||
| "2160p": "1", | ||
| "1080p": "2", | ||
| "1080i": "2", | ||
| "720p": "3", | ||
| "576p": "4", | ||
| "576i": "4", | ||
| "480p": "5", | ||
| "480i": "5", | ||
| } | ||
|
|
||
| sub_langs: list[str] = [] | ||
| if meta.get('is_disc', '') != 'BDMV': | ||
| mi = meta['mediainfo'] | ||
| for track in mi['media']['track']: | ||
| if track['@type'] == "Text": | ||
| language = track.get('Language') | ||
| if language == "en": | ||
| if track.get('Forced', "") == "Yes": | ||
| language = "en (Forced)" | ||
| title = track.get('Title', "") | ||
| if isinstance(title, str) and "intertitles" in title.lower(): | ||
| language = "en (Intertitles)" | ||
| for lang, subID in sub_lang_map.items(): | ||
| if language in lang and subID not in sub_langs: | ||
| sub_langs.append(subID) | ||
| if mapping_only: | ||
| return resolution_id | ||
| elif reverse: | ||
| return {v: k for k, v in resolution_id.items()} | ||
| elif resolution: | ||
| return {"resolution_id": resolution_id.get(resolution, "6")} | ||
| else: | ||
| for language in meta['bdinfo']['subtitles']: | ||
| for lang, subID in sub_lang_map.items(): | ||
| if language in lang and subID not in sub_langs: | ||
| sub_langs.append(subID) | ||
|
|
||
| # if sub_langs == []: | ||
| # sub_langs = [44] # No Subtitle | ||
| return sub_langs | ||
|
|
||
| def get_subs_tag(self, subs: list[str]) -> str: | ||
| if subs == []: | ||
| return ' [No subs]' | ||
| elif 'Eng' in subs: | ||
| return '' | ||
| meta_resolution = meta.get("resolution", "") | ||
| resolved_id = resolution_id.get(meta_resolution, "6") | ||
| return {"resolution_id": resolved_id} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how get_resolution_id is called across the codebase to understand usage patterns
rg -n --type=py 'get_resolution_id' -C3Repository: Audionut/Upload-Assistant
Length of output: 8823
🏁 Script executed:
# Check if reverse=True is used anywhere
rg -n 'reverse\s*=\s*True' --type=py | grep -i resolutionRepository: Audionut/Upload-Assistant
Length of output: 51
🏁 Script executed:
# Check other tracker implementations with similar signatures
fd -e py src/trackers/ -exec grep -l 'get_resolution_id' {} \; | head -5 | xargs -I {} sh -c 'echo "=== {} ===" && grep -A 20 "async def get_resolution_id" {}'Repository: Audionut/Upload-Assistant
Length of output: 322
🏁 Script executed:
# Specifically check how UNIT3D.py (line 80) uses the method without parameters
sed -n '75,90p' src/trackers/UNIT3D.pyRepository: Audionut/Upload-Assistant
Length of output: 766
🏁 Script executed:
# Check the structure of actual calls in trackersetup.py
sed -n '510,540p' src/trackersetup.py
sed -n '730,760p' src/trackersetup.pyRepository: Audionut/Upload-Assistant
Length of output: 3800
🏁 Script executed:
# Check other tracker implementations
cat src/trackers/UNIT3D.py | sed -n '259,280p'Repository: Audionut/Upload-Assistant
Length of output: 792
🏁 Script executed:
# Check CBR implementation
cat src/trackers/CBR.py | sed -n '80,100p'Repository: Audionut/Upload-Assistant
Length of output: 693
🏁 Script executed:
# Check if reverse parameter is defined in base class or interface
fd -e py src/ -exec grep -l "get_resolution_id" {} \; | xargs grep -B5 -A15 "def get_resolution_id" | head -80Repository: Audionut/Upload-Assistant
Length of output: 322
Inconsistent return structure across modes affects type safety.
The method returns structurally different dicts depending on the mode, yet the return type is annotated uniformly as dict[str, str]:
mapping_only=True→{"2160p": "1", "1080p": "2", ...}(resolution→id pairs)reverse=True→{"1": "2160p", ...}(id→resolution pairs with data loss from duplicate values like 1080p/1080i→"2")- normal →
{"resolution_id": "2"}(single structured key)
This pattern is repeated across multiple trackers (UNIT3D, CBR). While actual usage in trackersetup.py calls only mapping_only=True (working correctly), the reverse=True parameter is defined but never used, creating dead code with unresolved data-loss semantics.
Consider:
- Splitting into separate methods or using explicit return types (Union) to match actual behavior
- Removing the
reverseparameter if not needed, or documenting its data-loss semantics clearly
🤖 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/ACM.py` around lines 41 - 61, The get_resolution_id method
returns three different dict shapes depending on flags, breaking type safety and
leaving dead/unsafe reverse behavior; update by splitting responsibilities:
create a new method (e.g., get_resolution_mapping) that returns mapping_only
format dict[str,str] for callers that need the full map, create a new method
(e.g., get_resolution_by_meta) that returns the structured {"resolution_id":
str} for normal lookups, and either remove the reverse parameter from
get_resolution_id or replace it with a dedicated get_reverse_mapping method that
clearly documents and handles duplicate-value collisions; update callers (such
as callers in trackers setup) to call the appropriate new method and adjust type
annotations to explicit return types (dict[str,str] vs dict[str,str] with
structured key) to restore type safety.
wastaken7
changed the title
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.
Summary by CodeRabbit
Refactor
Configuration