feat(mediainfo): add General rule blocking files without video/audio track in mediainfo#229
Conversation
… the tracker warnings in the cli
📝 WalkthroughWalkthroughAdds a ChangesValidMediaInfoTracks flag and tracker-filtered rule failures
Estimated code review effort: 2 (Simple) | ~15 minutes 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/trackers/rules_test.go (1)
544-575: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd coverage for the AZ-family gate gap.
TestEvaluateRulesModifiedReleaseAcrossFamiliescovers PHD formodified_releasebut no test exercisesinvalid_mediainfo_tracksfor AZ/CZ/PHD, which is why the bypass inrules.go(Lines 55-63) went unnoticed.🤖 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/rules_test.go` around lines 544 - 575, Add test coverage for the AZ-family gate gap by extending TestEvaluateRulesModifiedReleaseAcrossFamilies or adding a new focused test that exercises invalid_mediainfo_tracks for AZ/CZ/PHD. Use EvaluateRules and the existing rule-finding helpers to assert that trackers in the AZ family do not bypass invalid_mediainfo_tracks when ValidMediaInfoTracks is false, and that the failure is present for the relevant tracker family cases. Keep the test structure parallel-friendly and refer to the tracker family names directly so the bypass in rules.go is covered.internal/trackers/rules.go (1)
55-63: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAZ/CZ/PHD trackers bypass the new
invalid_mediainfo_tracksgate.The
switchat Line 56 returns early (return append(failures, evaluateAZFamilyRules(...)...)) before theValidMediaInfoTrackscheck at Lines 60-62 is ever reached. As a result, AZ-family trackers (AZ, CZ, PHD) never get flagged for missing video/audio tracks, unlike every other tracker family. This mirrors the existingmodified_releasecheck pattern (Lines 45-53), which is deliberately evaluated before the family switch specifically to apply universally — the new gate should follow the same pattern.🐛 Proposed fix: move the gate before the family switch
if !skipsModifiedReleaseCheck(name) { if renamed, reason := isRenamedRelease(meta); renamed { addFailure("modified_release", reason) } } + + if !meta.ValidMediaInfoTracks { + addFailure("invalid_mediainfo_tracks", "missing video or audio tracks in MediaInfo") + } switch name { case "AZ", "CZ", "PHD": return append(failures, evaluateAZFamilyRules(name, meta)...) } rules, ok := additional.RulesFor(name) - if !meta.ValidMediaInfoTracks { - addFailure("invalid_mediainfo_tracks", "missing video or audio tracks in MediaInfo") - } // UNIT3D-known trackers without a tracker-specific RuleSet must still reach🤖 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/rules.go` around lines 55 - 63, The AZ/CZ/PHD family in rules.go is returning early from the family switch before the new invalid_mediainfo_tracks gate can run, so those trackers skip the MediaInfo track validation. Move the ValidMediaInfoTracks check so it is evaluated before the switch in the same way as the existing modified_release gate, and keep the family-specific logic in evaluateAZFamilyRules and additional.RulesFor unchanged. Ensure the added failure is applied universally before any early return from the AZ/CZ/PHD branch.
🧹 Nitpick comments (1)
cmd/upbrr/interactive.go (1)
1154-1154: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd test coverage for the new tracker-filtered Rule Failures output.
The filter/sort/print logic for
Rule Failures(Lines 1179-1201) is new and not exercised by any test —TestPrintMetadataPreviewShowsRichReleaseDetailspassesnilfortrackersand never setsTrackerRuleFailures. Consider adding a case that verifies filtering (only candidate trackers shown), sorting, and formatting.Also applies to: 1178-1201
🤖 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 `@cmd/upbrr/interactive.go` at line 1154, Add test coverage for the new tracker-filtered Rule Failures path in printMetadataPreview, since the current TestPrintMetadataPreviewShowsRichReleaseDetails never exercises it. Add a focused case that passes trackers and populates TrackerRuleFailures so you can verify the filtering keeps only matching candidate trackers, the output is sorted as intended, and the printed formatting matches the new Rule Failures behavior.
🤖 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/metadata/media_details.go`:
- Around line 61-66: The MediaInfo validity check is incorrectly marking DVD
releases as invalid when MediaInfoJSONPath has no video/audio tracks, even
though DVDs are handled by a separate VOB MediaInfo path. Update the
ValidMediaInfoTracks logic in media details processing so the false case only
applies when the release is not BDMV and not DVD, using the existing DiscType
check alongside splitMediaInfoTracks and MediaInfoJSONPath to exempt DVDs from
triggering invalid_mediainfo_tracks.
---
Outside diff comments:
In `@internal/trackers/rules_test.go`:
- Around line 544-575: Add test coverage for the AZ-family gate gap by extending
TestEvaluateRulesModifiedReleaseAcrossFamilies or adding a new focused test that
exercises invalid_mediainfo_tracks for AZ/CZ/PHD. Use EvaluateRules and the
existing rule-finding helpers to assert that trackers in the AZ family do not
bypass invalid_mediainfo_tracks when ValidMediaInfoTracks is false, and that the
failure is present for the relevant tracker family cases. Keep the test
structure parallel-friendly and refer to the tracker family names directly so
the bypass in rules.go is covered.
In `@internal/trackers/rules.go`:
- Around line 55-63: The AZ/CZ/PHD family in rules.go is returning early from
the family switch before the new invalid_mediainfo_tracks gate can run, so those
trackers skip the MediaInfo track validation. Move the ValidMediaInfoTracks
check so it is evaluated before the switch in the same way as the existing
modified_release gate, and keep the family-specific logic in
evaluateAZFamilyRules and additional.RulesFor unchanged. Ensure the added
failure is applied universally before any early return from the AZ/CZ/PHD
branch.
---
Nitpick comments:
In `@cmd/upbrr/interactive.go`:
- Line 1154: Add test coverage for the new tracker-filtered Rule Failures path
in printMetadataPreview, since the current
TestPrintMetadataPreviewShowsRichReleaseDetails never exercises it. Add a
focused case that passes trackers and populates TrackerRuleFailures so you can
verify the filtering keeps only matching candidate trackers, the output is
sorted as intended, and the printed formatting matches the new Rule Failures
behavior.
🪄 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: af0dc0a4-ffcf-4981-8e05-e387eaa47770
📒 Files selected for processing (7)
cmd/upbrr/cli_options_test.gocmd/upbrr/interactive.gointernal/metadata/media_details.gointernal/metadata/media_details_test.gointernal/trackers/rules.gointernal/trackers/rules_test.gopkg/api/services.go
Finding: AZ-family trackers bypass the new MediaInfo-track gate
Changes needed after #273 is mergedThis should be reimplemented against #273 rather than rebased mechanically:
|
2 participants
this could happen due to actually broken files or go-mediainfo being unstable autobrr/go-mediainfo#16
additionally broken tracker rules are now displayed in the cli
Summary by CodeRabbit