feat(trackers): add Lat-Team (LT) tracker support#238
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI 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 LT tracker support with Spanish audio and subtitle rules, metadata-driven categories, LT-specific release naming, profile registration, and unit tests. ChangesLT tracker support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PreparedMetadata
participant buildUnit3DName
participant buildLTName
participant MediaInfoJSON
PreparedMetadata->>buildUnit3DName: LT tracker metadata
buildUnit3DName->>buildLTName: release name and metadata
buildLTName->>MediaInfoJSON: read optional audio tracks
MediaInfoJSON-->>buildLTName: track language and title fields
buildLTName-->>PreparedMetadata: normalized LT release name
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
internal/trackers/impl/unit3d/lt.go (2)
57-63: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider hoisting the
asianCountriesmap to package level.This 50+ entry map is allocated on every call to
resolveUnit3DLTCategoryID. Moving it to a package-level variable avoids repeated allocation, though the impact is minor since this runs once per upload.♻️ Proposed change
+var ltAsianCountries = map[string]bool{ + "AE": true, "AF": true, "AM": true, "AZ": true, "BD": true, "BH": true, "BN": true, "BT": true, "CN": true, "CY": true, "GE": true, "HK": true, "ID": true, "IL": true, "IN": true, + "IQ": true, "IR": true, "JO": true, "JP": true, "KG": true, "KH": true, "KP": true, "KR": true, "KW": true, "KZ": true, "LA": true, "LB": true, "LK": true, "MM": true, "MN": true, + "MO": true, "MV": true, "MY": true, "NP": true, "OM": true, "PH": true, "PK": true, "PS": true, "QA": true, "SA": true, "SG": true, "SY": true, "TH": true, "TJ": true, "TL": true, + "TM": true, "TR": true, "TW": true, "UZ": true, "VN": true, "YE": true, +} + func resolveUnit3DLTCategoryID(meta api.PreparedMetadata) string {Then replace the inline map with
ltAsianCountriesin the function body.🤖 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 `@internal/trackers/impl/unit3d/lt.go` around lines 57 - 63, Move the asianCountries map out of resolveUnit3DLTCategoryID into a package-level variable named ltAsianCountries, preserving all entries, and replace the function’s inline map reference with that variable.
21-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSimplify redundant default-category logic.
The
else if category != "MOVIE"branch re-assigns"1", which is already the initial value ofcategoryID. This block is a no-op and can be removed for clarity.♻️ Proposed simplification
categoryID := "1" // Default MOVIE if category == "TV" { categoryID = "2" // Default TV - } else if category != "MOVIE" { - categoryID = "1" }🤖 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 `@internal/trackers/impl/unit3d/lt.go` around lines 21 - 26, Remove the redundant else-if branch that reassigns categoryID to "1" in the category selection logic; retain the initial default and the TV override.internal/trackers/impl/unit3d/names.go (2)
506-506: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winHoist regex compilations to package level.
Four
regexp.MustCompilecalls execute insidebuildLTNameon every invocation. These patterns are static and should be compiled once at package level, consistent with Go performance best practices.♻️ Proposed change
+var ( + ltMarkerRegex = regexp.MustCompile(`(?i)\.(19\d\d|20\d\d|3d|480p|576p|720p|1080p|1080i|2160p|4k|mkv|avi|mp4|remux|bluray|blu-ray|web-dl|webdl|web|hdtv|dvdrip|dvd|bd25|bd50|uhd)\b`) + ltAKARegex = regexp.MustCompile(`(?i)[. _-]aka[. _-]`) + ltMultipleDots = regexp.MustCompile(`\.{2,}`) + ltMultipleSpaces = regexp.MustCompile(`\s{2,}`) +)Then replace the inline
regexp.MustCompilecalls with these package-level variables.Also applies to: 530-530, 654-656
🤖 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 `@internal/trackers/impl/unit3d/names.go` at line 506, Move the four static regexp.MustCompile calls from buildLTName and the other referenced functions into package-level variables, then replace each inline compilation with the corresponding shared variable. Preserve the existing regex patterns and matching behavior while ensuring compilation occurs only once during package initialization.
563-573: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider hoisting audio-language maps and keyword slices to package level.
audioLatinoCheck,audioCastilianCheck,latinoKeywords, andcastilianKeywordsare allocated on every call. Since they are static, defining them at package level avoids repeated allocation.♻️ Proposed change
+var ( + ltAudioLatinoCheck = map[string]bool{ + "es-419": true, "es-mx": true, "es-ar": true, "es-cl": true, "es-ve": true, + "es-bo": true, "es-co": true, "es-cr": true, "es-do": true, "es-ec": true, + "es-sv": true, "es-gt": true, "es-hn": true, "es-ni": true, "es-pa": true, + "es-py": true, "es-pe": true, "es-pr": true, "es-uy": true, + } + ltAudioCastilianCheck = map[string]bool{ + "es": true, "es-es": true, + } + ltLatinoKeywords = []string{"latino", "latin america"} + ltCastilianKeywords = []string{"castellano"} +)🤖 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 `@internal/trackers/impl/unit3d/names.go` around lines 563 - 573, Hoist the static audio-language lookup maps and keyword slices from the function containing audioLatinoCheck and audioCastilianCheck to package-level variables, preserving their contents and names. Update the function to reference these package-level definitions so they are not recreated on each call.
🤖 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 `@internal/trackers/impl/unit3d/names.go`:
- Around line 529-533: Prevent title removal when external metadata is
unavailable. In the title-block replacement condition near akaRegex, require
targetTitle to be non-empty before comparing or assigning targetTitleDotted;
retain the existing AKA replacement behavior while leaving titleBlock unchanged
when no target title was resolved.
In `@internal/trackers/impl/unit3d/upload_test.go`:
- Around line 1892-1906: Add an assertion in the “cleans Dual-Audio and Dubbed
and AKA” subtest after calling buildUnit3DName to verify the generated name does
not contain “AKA”, alongside the existing Dual-Audio and Dubbed checks.
- Around line 1982-2013: Update the CAST/SUBS tests to create MediaInfo files
under t.TempDir() instead of os.CreateTemp with manual removal, and ensure files
are closed or written safely. Provide ExternalMetadata containing the expected
target title, then assert the complete generated name preserves “Movie” while
adding the [CAST] or [SUBS] tag before the group suffix; use buildUnit3DName and
the relevant test cases to cover both behaviors.
---
Nitpick comments:
In `@internal/trackers/impl/unit3d/lt.go`:
- Around line 57-63: Move the asianCountries map out of
resolveUnit3DLTCategoryID into a package-level variable named ltAsianCountries,
preserving all entries, and replace the function’s inline map reference with
that variable.
- Around line 21-26: Remove the redundant else-if branch that reassigns
categoryID to "1" in the category selection logic; retain the initial default
and the TV override.
In `@internal/trackers/impl/unit3d/names.go`:
- Line 506: Move the four static regexp.MustCompile calls from buildLTName and
the other referenced functions into package-level variables, then replace each
inline compilation with the corresponding shared variable. Preserve the existing
regex patterns and matching behavior while ensuring compilation occurs only once
during package initialization.
- Around line 563-573: Hoist the static audio-language lookup maps and keyword
slices from the function containing audioLatinoCheck and audioCastilianCheck to
package-level variables, preserving their contents and names. Update the
function to reference these package-level definitions so they are not recreated
on each call.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 30aa37c9-52ac-444e-8eb7-0bacb4ad59c7
📒 Files selected for processing (5)
internal/trackers/impl/unit3d/additional/rules.gointernal/trackers/impl/unit3d/lt.gointernal/trackers/impl/unit3d/names.gointernal/trackers/impl/unit3d/profiles.gointernal/trackers/impl/unit3d/upload_test.go
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Hey, thanks for the PR. I have some refactoring work I wish to do before I pull in other trackers. I shall ensure this gets added before the next release. Thanks. |
2 participants
Summary by CodeRabbit
[CAST]/[SUBS]) based on available media/language information.