[ptr727]

Summary

Three cleanup-step fixes in ProcessFile, fixed together because they share the cleanup re-run after VerifyAndRepair().

• Metadata errors left by a disabled repair (Fail or Report RepairMetadataErrors When Errors Remain After Repair #1074). RepairMetadataErrors() used to pass silently when the repair a leftover error needs was turned off. It now reports what is left and continues, and the file is not failed:
  • ReMux off: a warning naming the remaining Remove, ReMux and SetLanguage errors.
  • SetIetfLanguageTags off: logged at Information, since that opt-out is deliberate.
  • SetTrackFlags off: a warning.
• Track flags that do not stick (Fail or Report RepairMetadataErrors When Errors Remain After Repair #1074). RepairMetadataFlags() re-checks after setting flags. When they did not stick, it warns once and does not set them again, on this run or a later one. SetFlags keeps its meaning, "flags were set", per the maintainer decision on Fail or Report RepairMetadataErrors When Errors Remain After Repair #1074. A later run reads SetFlags with flag errors still present as flags that did not stick. The cleanup re-run relies on what this run saw instead, so flags that a FfMpeg or HandBrake rewrite drops are still set again. A skipped repeat logs at Information.
• Cover art only MediaInfo reports (Remove Cover Art That Only MediaInfo Reports, and Re-Check the FfProbe Path #1077). RemoveCoverArtMediaInfo() first removes attachments in place with MkvPropEdit. If MediaInfo still reports cover art, it remuxes with MkvMerge only (Convert.ReMux(), whose default input options include --no-attachments). It never uses the FfMpeg fallback, which maps everything. RemoveCoverArt() then re-checks every source, which also covers the FfProbe path after RemoveTags(). When cover art remains, it warns once and sets RemovedCoverArt, so later runs do not remove it again. MkvMerge-reported cover art is still removed by track as before. The first call in a run infers what is left from RemovedCoverArt, the same way the flags fix does. So a file that an earlier version marked RemovedCoverArt and that still shows cover art only in FfProbe or MediaInfo is not retried.
• Sidecar delete failure in DeleteFailedFile() (Set FileDeleted When Only the Sidecar Delete Fails in DeleteFailedFile #1116). The media delete and the sidecar delete have separate try blocks. FileDeleted is set once the media file is gone, and a sidecar delete failure is logged on its own. So the deleted media path is no longer added to FileIgnoreList.

Tests

• New PlexCleanerTests/CleanupStepRepeatTests.cs uses fake tools that log every edit:
  • MediaInfo-only cover art that remains is remuxed once, then not again in the cleanup re-run or on the next run.
  • A MediaInfo-only cover art attachment is removed in place without a remux.
  • Flags that do not stick are set once, then not again in the cleanup re-run or on the next run.
  • With ReMux and SetTrackFlags off, the step continues without editing and without VerifyFailed.
• DeleteFailedFileTests.SidecarDeleteThrows_ReturnsFalse becomes SidecarDeleteThrows_ReturnsTrueAndSetsFileDeleted, to match the new contract.
• The full suite passes locally, along with CSharpier, the build, and dotnet format style --verify-no-changes.

Deferred

HISTORY.md and the ARCHITECTURE.md "Suspect Branches" prose (S7, S10) are deferred by the maintainer to one final docs pull request. ArchitectureDiagramTests passes without a diagram edit, since every method named in the diagrams still exists and the step order is unchanged.

Closes on promotion: #1074, #1077, #1116

Local Review

local-strict-review passes ran over the full diff at each pushed head. The review of the first head led to these changes:

• Cover art that a removal already gave up on is skipped before the ReMux-off error, so turning ReMux off later does not fail the file on every run.
• The skipped-repeat logs moved from Debug to Information.
• One GetRemuxErrors() helper feeds the remux trigger, its re-check, and the report.
• New tests cover the report per option and the cleanup re-run setting flags a rewrite dropped.

Declined, each on evidence:

• No sidecars, so a repeat. With UseSidecarFiles off no state persists, so the MediaInfo-only remux and the warnings repeat every run. Interlace and caption scans repeat the same way, since ScannedInterlace and ScannedCaptions persist only in the sidecar.
• Cover art after a cancelled run. A pipeline rewrite ends in an MkvMerge remux with the default --no-attachments options, the same remux RemoveCoverArtMediaInfo() runs. So a retry of MediaInfo-only cover art left behind that way fails the same way. Old sidecars hold for the same reason.
• Track flags after a cancelled run. The run sets flags, which sets SetFlags, then a rewrite drops them, and the run ends before the cleanup re-run. Later runs then read that as flags that did not stick, and log the skip at Information. The maintainer accepted this window on Fail or Report RepairMetadataErrors When Errors Remain After Repair #1074, matching the RemovedCoverArt decision on Remove Cover Art That Only MediaInfo Reports, and Re-Check the FfProbe Path #1077.
• Test fake duplication. The MkvMerge fake script repeats the one in FakeTools.WriteSampleToolData() with a calls log added, and FakeTools.cs is outside this change's files. The test log-capture sink is a third copy of one that lives in other test files, filed as Share One Log Capture Helper Across Test Classes #1129.

Doc drift, deferred to the final docs pull request:

• ARCHITECTURE.md "Process Command Logic": the RemoveCoverArt() diagram (the ReMux fail edge, D7 and D9 with their S10 notes, and no MediaInfo branch).
• The RepairMetadataErrors() diagram (F1 and F2 with the S7 note, and no report step).
• The prose that a delete that throws counts as not deleted.

Pre-existing findings, filed

• PlexCleaner/TrackProps.cs single State per track: a track needing both a remux and flags reports only the flags while both options are off. Correctness, filed as Report Every Repair a Track Needs When Its State Holds One #1127.
• PlexCleanerTests log-capture sink copies: design, one shared helper, filed as Share One Log Capture Helper Across Test Classes #1129.

Dropped findings

• PlexCleaner/ProcessFile.cs DeleteFailedFile(): a second Error line after LogAndHandle. Style, kept because it names the sidecar path.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 1 minute.

Check out review usage here.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 4ac109b1-cdee-4cb1-96fa-abd8b82e8d63

Commits

Reviewing files that changed from the base of the PR and between e88a830 and 7121ecd.

Files selected for processing: 3

• PlexCleaner/ProcessFile.cs
• PlexCleanerTests/CleanupStepRepeatTests.cs
• PlexCleanerTests/DeleteFailedFileTests.cs

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

[ptr727]

A recorded local strict-review pass covers head 3fac9725f159cc4661d924806e2d4540fff7208d, the content this pull request carries at that commit against develop, and it recorded 0 findings.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Report unrepaired metadata and prevent repeated cleanup edits

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Report metadata errors left by disabled repairs without failing the file.
• Stop retrying ineffective flag and cover-art edits across cleanup passes and later runs.
• Treat media deletion as successful even when its sidecar cannot be deleted.

Diagram

graph TD
  P["Cleanup pass"] --> C["Cover art"] --> T["Edit tools"] --> S[("Sidecar state")]
  P --> M["Metadata repair"] --> T
  P --> D["Failed-file delete"] --> S
  S --> C
  S --> M

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Persist explicit unresolved-issue markers

• ➕ Distinguishes an attempted edit from an edit known not to stick.
• ➖ Requires sidecar schema changes and migration behavior for existing sidecars.

Recommendation: Keep the PR's approach: infer prior ineffective edits from existing sidecar states and current tool results, while tracking the current pass separately. Explicit markers would be clearer but add persistence complexity for these targeted fixes.

Files changed (3) +433 / -21

Bug fix (1) +189 / -19

ProcessFile.csReport residual errors and make cleanup retries outcome-aware +189/-19

Report residual errors and make cleanup retries outcome-aware

• Reports metadata errors left by disabled repairs, rechecks track flags after setting them, and avoids repeating flag edits that did not stick. Adds a MediaInfo-only cover-art path, rechecks all cover-art sources, and avoids retrying art that remains after removal. Separates media and sidecar deletion so a sidecar failure does not reverse the successful media-delete result.

PlexCleaner/ProcessFile.cs

Tests (2) +244 / -2

CleanupStepRepeatTests.csExercise cleanup edits across repeated passes and runs +238/-0

Exercise cleanup edits across repeated passes and runs

• Adds fake-tool tests for persistent MediaInfo cover art, in-place attachment removal, flags that do not stick, and disabled repair options. Call logs verify that ineffective edits are not repeated.

PlexCleanerTests/CleanupStepRepeatTests.cs

DeleteFailedFileTests.csVerify successful media deletion despite sidecar failure +6/-2

Verify successful media deletion despite sidecar failure

• Changes the sidecar-delete failure test to require a successful result, an absent media file, and a FileDeleted state.

PlexCleanerTests/DeleteFailedFileTests.cs

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (1) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Remux needs disappear behind flag repairs 📎 Requirement gap ≡ Correctness

Description

The new unrepaired-error report derives remux and flag needs from the single current
TrackProps.State, while flag detection can overwrite a track's earlier ReMux state. When one
track needs both repairs and both options are disabled, the report sees only SetFlags, so the
remux need is neither reported nor preserved for repair.

Code

PlexCleaner/ProcessFile.cs[R375-377]

+        bool removeErrors = HasMetadataErrors(TrackProps.StateType.Remove);
+        bool remuxErrors = HasMetadataErrors(TrackProps.StateType.ReMux);
+        bool setLanguageErrors = HasMetadataErrors(TrackProps.StateType.SetLanguage);

Evidence

The added report checks remux and flag errors by the current state, but language validation assigns
ReMux and title-based flag validation later assigns SetFlags on the same track, overwriting the
remux state. The existing remux repair then searches by state and cannot find that overwritten need.

Report every repair needed by a track
PlexCleaner/ProcessFile.cs[375-377]
PlexCleaner/ProcessFile.cs[403-406]
PlexCleaner/TrackProps.cs[199-224]
PlexCleaner/TrackProps.cs[773-793]
PlexCleaner/ProcessFile.cs[522-539]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
Issue description
A track can require both remux and flag repairs, but the new report checks only the single current `TrackProps.State`; flag detection can replace `ReMux` with `SetFlags`, causing the remux need to disappear.
Fix Focus Areas
- PlexCleaner/ProcessFile.cs[375-377]
- PlexCleaner/ProcessFile.cs[403-406]
- PlexCleaner/TrackProps.cs[199-224]
- PlexCleaner/TrackProps.cs[773-793]
Recommended Fix
Preserve independent repair requirements for each track instead of relying on one mutually exclusive state. Ensure `RepairMetadataErrors` and `ReportUnrepairedMetadataErrors` can discover and report both remux/language and flag needs when they apply to the same track, and add a regression test for one audio track requiring both repairs with `ReMux` and `SetTrackFlags` disabled.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. The title capitalizes a short conjunction 📘 Rule violation ✧ Quality

Description

The pull request title uses and as an uppercase principal word in `Report Unrepaired Metadata and
Stop Repeating Cleanup Edits`. This breaks the repository's stated Title Case convention for short
conjunctions in the middle of a title and makes title formatting inconsistent.

Code

PlexCleaner/ProcessFile.cs[369]

+        ReportUnrepairedMetadataErrors();

Evidence

The supplied PR title contains the uppercase conjunction and, while the checklist's expected
policy keeps short conjunctions lowercase when they appear in the middle of a title.

Rule 2826333: Pull request titles must follow configurable Title Case style, not checked in code

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
Issue description
The pull request title capitalizes the short conjunction `and` in the middle of the title.
Fix Focus Areas
- PlexCleaner/ProcessFile.cs[369-369]
Recommended Fix
Change the PR title to `Report Unrepaired Metadata and Stop Repeating Cleanup Edits` with the repository's configured title-case treatment for short conjunctions, such as `Report Unrepaired Metadata and Stop Repeating Cleanup Edits` only if the repository explicitly permits capitalized conjunctions; otherwise use the configured lowercase form.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. New flag errors can remain unrepaired ✓ Resolved

Description

RepairMetadataErrors treats a persisted file-wide SetFlags bit plus any current flag error as
proof that the current flag previously failed to stick. If an earlier flag edit succeeded but a
subsequent rewrite drops a flag before this repair pass, RepairMetadataFlags skips the newly
needed edit.

Code

PlexCleaner/ProcessFile.cs[R310-312]

+        _trackFlagsDoNotStick ??=
+            _sidecarFile.State.HasFlag(SidecarFile.StatesType.SetFlags)
+            && HasMetadataErrors(TrackProps.StateType.SetFlags);

Evidence

The sidecar retains the SetFlags bit after an edit, regardless of whether the refreshed metadata
shows that it succeeded. Refreshes preserve that state while replacing tool results, and the new
guard skips the repair call for any later SetFlags error.

PlexCleaner/ProcessFile.cs[309-312]
PlexCleaner/ProcessFile.cs[680-720]
PlexCleaner/ProcessFile.cs[721-738]
PlexCleaner/ProcessFile.cs[2694-2707]
PlexCleaner/TrackProps.cs[773-795]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A historical `SetFlags` bit records that an edit was attempted, not that the current flag error is the same one or that the edit failed. New flag errors after a rewrite can therefore be skipped.
## Fix Focus Areas
- PlexCleaner/ProcessFile.cs[309-312]
- PlexCleaner/ProcessFile.cs[680-688]
- PlexCleaner/ProcessFile.cs[721-738]
## Recommended Fix
Persist non-convergence separately from successful flag edits, with enough track and flag identity to distinguish an already-failed attempt from a newly detected error. Keep the same-run guard for the specific error that did not stick, while allowing newly introduced errors to be repaired.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can keep summaries lean with Findings visible per group, which tucks the rest behind a View link

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[Copilot]

🔵 Needs a closer look

Defer because cleanup paths involving MkvPropEdit, MkvMerge, MediaInfo, sidecar replacement, and FFprobe have only fake-tool unit-test coverage, leaving real-media and external-tool integration behavior unverified.

No open issues 🎉

What changed in this PR

This pull request improves cleanup processing by reporting unrepaired metadata, preventing repeated non-converging edits, handling MediaInfo-only cover art, and preserving deletion state when sidecar cleanup fails.

Changes:

• Reports metadata left behind when repair options are disabled without incorrectly failing the file.
• Tracks persistent flag and cover-art repair failures to avoid repeating edits across cleanup passes and later runs.
• Separates media and sidecar deletion handling and expands regression coverage.
• Minor follow-up: add coverage for FfProbe-only cover art after tag removal.

File	Description
PlexCleanerTests/​DeleteFailedFileTests.cs	Verifies that media deletion succeeds and records FileDeleted when sidecar deletion fails.
PlexCleanerTests/​CleanupStepRepeatTests.cs	Tests repeated flag and cover-art cleanup behavior across passes and subsequent runs.
PlexCleaner/​ProcessFile.cs	Implements metadata reporting, non-repeating cleanup state, MediaInfo cover-art handling, and independent sidecar deletion.

This pull request is ready to be approved.

_{Review effort: Lite}

[ptr727]

A recorded local strict-review pass covers head cc0c7a0cc23e4070e0388039956895f97d70e285, the content this pull request carries at that commit against develop, and it recorded 4 findings.

[ptr727]

Answers to the Qodo review findings:

1. Remux needs disappear behind flag repairs

Deferred to #1127, filed with the pre-existing label and listed in the body under "Pre-existing findings, filed". The cause is the single State that TrackProps holds per track: SetFlagsFromTitle() overwrites a ReMux state set by SetLanguage(). That predates this change, and the fix belongs in TrackProps.cs, which this change does not touch.

2. The title capitalizes a short conjunction

Declined, because the title has no capitalized conjunction. It reads Report Unrepaired Metadata and Stop Repeating Cleanup Edits, with and already lowercase. To check: gh pr view 1128 --repo ptr727/PlexCleaner --json title --jq .title.

3. New flag errors can remain unrepaired

Fixed in 21322b9. SetFlags is now set only when the flags did not stick, and it is cleared when they do. So an earlier successful set that a later rewrite undoes no longer reads as a failure. RepairMetadataErrors_TrackFlagsStick_LeavesSetFlagsUnset and RepairMetadataErrors_RewriteDropsFlagsInRun_SetsThemInCleanupReRun cover it. One residue remains, for a sidecar an older version wrote, and the body lists it under "Declined" with its reason.

[ptr727]

A recorded local strict-review pass covers head 7121ecd1d20a5ccaf64d0dfea2cf7b664734b1b9, the content this pull request carries at that commit against develop, and it recorded 0 findings.

[ptr727]

3. New flag errors can remain unrepaired

A correction to my earlier answer, which said this was fixed in 21322b9. 7121ecd reverts that change by maintainer decision. That decision is recorded on #1074 (2026-10-31): SetFlags keeps its meaning, "flags were set", and the narrow window in this finding is accepted. It matches the RemovedCoverArt decision on #1077.

The window is narrow. Within a run, every FfMpeg or HandBrake rewrite happens after the first RepairMetadataErrors() call. The cleanup re-run at Process.cs (after VerifyAndRepair()) then sets the flags a rewrite dropped, because _trackFlagsDoNotStick reads what this run saw, not the persisted state. RepairMetadataErrors_RewriteDropsFlagsInRun_SetsThemInCleanupReRun covers that. The only case left is a run cancelled or failed between a rewrite and that cleanup re-run. In that case later runs skip the repeat, and each one logs it at Information.