fix(crossseed): enforce resolution and language matching with sensible defaults

[s0up4200]

Summary

• Resolution matching now requires equality, with an exception for SD resolutions (480p/576p/SD) matching empty since rls often omits resolution for SD releases
• Language matching now requires equality, with an exception for empty matching ENGLISH since most English releases omit language tags
• Prevents false matches like FRENCH.WEB matching MULTi.1080p.WEB

Fixes #688

Test plan

• Added test cases for issue Cross-seed does not match resolution and language correctly #688 scenarios (resolution/language mismatches)
• Added test cases for SD resolution exception (480p vs empty)
• Added test cases for ENGLISH language exception (empty vs ENGLISH)
• Verified no regressions in full crossseed test suite

Summary by CodeRabbit

• Bug Fixes

  • Release matching now treats unspecified resolutions as matching known SD formats (480p/576p/SD).
  • Language matching now treats missing language as English and relaxes strict mismatches involving English or empty labels.

• Tests

  • Added unit tests covering resolution and language matching scenarios to ensure correct matching behavior.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Loosens release matching in the cross-seed service to treat empty resolutions as matching known SD values and to treat empty language as equivalent to English in specific cases; adds unit tests covering these resolution and language scenarios.

Changes

Cohort / File(s)	Summary
Resolution & language matching logic internal/services/crossseed/matching.go	Relaxed matching: empty resolution can match known SD values (480p/576p/SD) via an isKnownSD check; empty language is treated as English in comparisons via an isEnglishOrEmpty helper. Adjusts resolution/language equality conditions.
Matching test coverage internal/services/crossseed/matching_resolution_language_test.go	New table-driven tests exercising resolution and language matching combinations, including SD exceptions and empty-language-as-English behaviors, asserting expected match outcomes.

Sequence Diagram(s)

(Skipped — changes are localized matching logic and tests, not a multi-component sequential flow.)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• fix(crossseed): compare Site and Sum fields for anime releases #769 — Also modifies releasesMatch in matching.go; adds Site and Sum comparisons that interact with core matching logic.
• fix(crossseed): require strict HDR and Collection matching #799 — Adjusts releasesMatch to change HDR/collection matching rules; structurally similar field-specific matching changes.
• fix(crossseed): improve matching with size validation and relaxed audio checks #845 — Alters releasesMatch to relax audio checks and add size validation; another changeset refining matching heuristics.

Poem

🐰 I sniffed the bits and hopped around,

Empty tags now find their ground.
480 and English, once apart,
Now sometimes match — a gentler art.
Hooray, I twirl with joyful heart! 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically summarizes the main change: enforcing resolution and language matching with sensible defaults in the crossseed service.
Linked Issues check	✅ Passed	The pull request fully implements the requirements from issue #688 by enforcing resolution and language matching with sensible defaults for SD resolutions and English language tags.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to resolving the matching logic issue: updated matching rules in matching.go and added comprehensive test coverage in matching_resolution_language_test.go.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/crossseed-resolution-language-matching

---

📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 207d655 and 4b65eaa.

📒 Files selected for processing (2)

• internal/services/crossseed/matching.go
• internal/services/crossseed/matching_resolution_language_test.go

🚧 Files skipped from review as they are similar to previous changes (2)

• internal/services/crossseed/matching_resolution_language_test.go
• internal/services/crossseed/matching.go

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

internal/services/crossseed/matching.go (1)

236-257: Logic is correct; consider simplifying the control flow.

The SD resolution exception correctly addresses issue #688. However, the empty if-body pattern (lines 253-254) is a minor code smell. Consider inverting the condition for cleaner flow:

🔎 Suggested refactor

 if sourceRes != candidateRes {
-    // rls omits resolution for many SD releases (e.g. "WEB" without "480p"), so
-    // treat an empty resolution as a match only when the other side is clearly SD.
     isKnownSD := func(res string) bool {
         switch strings.ToUpper(strings.TrimSpace(res)) {
         case "480P", "576P", "SD":
             return true
         default:
             return false
         }
     }

-    if (sourceRes == "" && isKnownSD(candidateRes)) || (candidateRes == "" && isKnownSD(sourceRes)) {
-        // allow SD-vs-empty
-    } else {
+    // rls omits resolution for many SD releases (e.g. "WEB" without "480p"), so
+    // treat an empty resolution as a match only when the other side is clearly SD.
+    sdVsEmpty := (sourceRes == "" && isKnownSD(candidateRes)) || (candidateRes == "" && isKnownSD(sourceRes))
+    if !sdVsEmpty {
         return false
     }
 }

internal/services/crossseed/matching_resolution_language_test.go (1)

50-62: Consider adding tests for 576p and "SD" resolution.

The SD exception tests only cover 480p, but the isKnownSD helper also accepts "576P" and "SD". Adding test cases for these would ensure complete coverage of the resolution exception logic.

🔎 Additional test cases

{
    name:          "empty resolution matches 576p (SD exception)",
    sourceName:    "Show.S01E01.WEB-DL-GROUP",
    candidateName: "Show.S01E01.576p.WEB-DL-GROUP",
    wantMatch:     true,
},
{
    name:          "empty resolution matches SD (SD exception)",
    sourceName:    "Show.S01E01.WEB-DL-GROUP",
    candidateName: "Show.S01E01.SD.WEB-DL-GROUP",
    wantMatch:     true,
},

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ace0101 and 207d655.

📒 Files selected for processing (2)

• internal/services/crossseed/matching.go
• internal/services/crossseed/matching_resolution_language_test.go

🧰 Additional context used

🧠 Learnings (2)

📓 Common learnings

Learnt from: s0up4200
Repo: autobrr/qui PR: 641
File: internal/services/crossseed/service.go:209-212
Timestamp: 2025-11-28T20:32:30.126Z
Learning: Repo: autobrr/qui PR: 641
File: internal/services/crossseed/service.go
Learning: The cross-seed recheck-resume worker intentionally runs for the process lifetime and keys pending entries by hash only. This is acceptable under the current constraint that background seeded-search runs operate on a single instance at a time; graceful shutdown and instanceID|hash keying are deferred by design.

Learnt from: s0up4200
Repo: autobrr/qui PR: 641
File: internal/services/crossseed/service.go:2415-2457
Timestamp: 2025-11-28T22:21:20.730Z
Learning: Repo: autobrr/qui PR: 641
File: internal/services/crossseed/service.go
Learning: The determineSavePath function intentionally includes a contentLayout string parameter for future/content-layout branching and API consistency. Its presence is by design even if unused in the current body; do not flag as an issue in reviews.

📚 Learning: 2025-11-28T20:32:30.126Z

Learnt from: s0up4200
Repo: autobrr/qui PR: 641
File: internal/services/crossseed/service.go:209-212
Timestamp: 2025-11-28T20:32:30.126Z
Learning: Repo: autobrr/qui PR: 641
File: internal/services/crossseed/service.go
Learning: The cross-seed recheck-resume worker intentionally runs for the process lifetime and keys pending entries by hash only. This is acceptable under the current constraint that background seeded-search runs operate on a single instance at a time; graceful shutdown and instanceID|hash keying are deferred by design.

Applied to files:

• internal/services/crossseed/matching_resolution_language_test.go

🧬 Code graph analysis (1)

internal/services/crossseed/matching_resolution_language_test.go (2)

pkg/releases/parser.go (1)

• NewDefaultParser (29-31)

pkg/stringutils/normalize.go (1)

• NewDefaultNormalizer (35-37)

🔇 Additional comments (2)

internal/services/crossseed/matching.go (1)

309-322: LGTM — language exception handles the common English-release case correctly.

The logic treating empty language as equivalent to "ENGLISH" addresses the PR objective. The use of joinNormalizedSlice ensures uppercase comparison, so the "ENGLISH" literal match is correct.

internal/services/crossseed/matching_resolution_language_test.go (1)

12-95: Well-structured table-driven tests with good coverage.

The test file correctly validates the resolution and language matching behavior introduced in this PR. The use of parallel execution and clear test case naming is appreciated.