[ptr727]

Summary

Process.ProcessFile() counted the deletion of a sidecar with no matching MKV file as a failure. The file landed in the error count and the results file with FailedOperation set, and the failure path ran DeleteFailedFile(). Deleting a non-MKV file in DeleteNonMkvFile() is the same kind of cleanup and counts as a success.

• Process.ProcessFile() sets result = true when DeleteMismatchedSidecarFile() returns false, the same as for DeleteNonMkvFile().
• The results row reads Result=true, Modified=true, State=FileDeleted, with no FailedOperation.
• ARCHITECTURE.md: the "Per-File Pipeline" diagram routes the orphan sidecar edge to the success node, the prose names both steps as the exceptions, and the S3 entry leaves "Suspect Branches".
• HISTORY.md: a 3.23 line for the change.
• OrphanSidecarTests runs Process.ProcessFile() on an orphan sidecar and on a sidecar with its MKV present. The orphan test fails on the code before this change.

Verification

• dotnet test PlexCleanerTests: 340 passed, 0 failed.
• CSharpier, dotnet format, markdownlint, cspell and the prose gate pass.
• A local strict review pass over the full diff recorded 0 findings.

Fixes #1070

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Orphan sidecars are now treated as successfully cleaned up when deleted, rather than causing the file-processing operation to be reported as failed.
  • Sidecars with a matching MKV file remain in place and are treated as ignored, so valid sidecar files are not removed.

• Documentation

  • Updated the processing guidance and version history to reflect the corrected orphan-sidecar cleanup behavior.

[ptr727]

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

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

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: 3c072388-553d-48ef-a353-7a19db9046f9

Commits

Reviewing files that changed from the base of the PR and between 8ec732f and 456af16.

Files selected for processing: 4

• ARCHITECTURE.md
• HISTORY.md
• PlexCleaner/Process.cs
• PlexCleanerTests/OrphanSidecarTests.cs

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

Walkthrough

When an orphan sidecar is deleted, ProcessFile now records success instead of failure. Tests cover orphan sidecar deletion and the handling of a sidecar with a matching MKV. Documentation describes the updated outcome.

Changes: Orphan sidecar cleanup

Layer / File(s)	Summary
Record orphan sidecar deletion as success PlexCleaner/Process.cs, PlexCleanerTests/OrphanSidecarTests.cs, ARCHITECTURE.md, HISTORY.md	ProcessFile records success when DeleteMismatchedSidecarFile returns false. Tests check that an orphan sidecar is deleted and reported as successful, and that a sidecar with a matching MKV is kept and ignored. The architecture documentation and history describe orphan sidecar deletion as successful cleanup.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: Minimal · up to 456af

Orphan sidecar deletion now counts as successful cleanup, with tests covering orphan and matched sidecars. No actionable merge-blocking risk is identified; merge after normal checks pass.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title "Count an Orphan Sidecar Deletion as a Success" matches the main change. The PR changes Process.ProcessFile so that deleting a sidecar without a matching MKV counts as success, consistent wi…
Linked Issues check		Issue #1070 requires an orphan .PlexCleaner deletion to count as successful cleanup. PlexCleaner/Process.cs now sets result = true when DeleteMismatchedSidecarFile() returns false and exits th…
Out of Scope Changes check		The changes stay within issue #1070. ARCHITECTURE.md documents the corrected success path, HISTORY.md records the behavior change, and OrphanSidecarTests provides regression coverage. These chan…

Full details: Title check

Explanation

The title "Count an Orphan Sidecar Deletion as a Success" matches the main change. The PR changes Process.ProcessFile so that deleting a sidecar without a matching MKV counts as success, consistent with DeleteNonMkvFile. The title is short, specific, and matches the linked issue #1070.

Full details: Linked Issues check

Explanation

Issue #1070 requires an orphan .PlexCleaner deletion to count as successful cleanup. PlexCleaner/Process.cs now sets result = true when DeleteMismatchedSidecarFile() returns false and exits the loop. OrphanSidecarTests verifies deletion, success, modified = true, FileDeleted, and a null failedOperation. The test also verifies that a sidecar with a matching MKV remains ignored.

Full details: Out of Scope Changes check

Explanation

The changes stay within issue #1070. ARCHITECTURE.md documents the corrected success path, HISTORY.md records the behavior change, and OrphanSidecarTests provides regression coverage. These changes directly support the requested behavior.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files. (2 skipped: 2 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• 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

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

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

PR Summary by Qodo

Count orphan sidecar deletion as successful cleanup

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 10-20 Minutes

AI Description

• Treat deleting a sidecar without a matching MKV as success, not a processing failure.
• Add regression tests for orphan deletion and preservation of sidecars with matching MKVs.
• Align the pipeline documentation and release history with the corrected outcome.

Diagram

graph TD
  Input["Sidecar file"] --> Check{"Matching MKV?"} -->|no| Delete["Delete orphan"] --> Success["Successful cleanup"] --> Results["Results row"]
  Check -->|yes| Keep["Keep sidecar"] --> Ignored["Ignored result"]

Loading

High-Level Assessment

Keep the existing deletion step’s return contract and interpret its false return as successful cleanup in Process.ProcessFile(), as the pipeline already does for non-MKV deletion. Changing that contract would require a broader refactor without a clear benefit for this fix.

Files changed (4) +62 / -7

Bug fix (1) +1 / -2

Process.csReturn success after deleting an orphan sidecar +1/-2

Return success after deleting an orphan sidecar

• Treats a false return from DeleteMismatchedSidecarFile as success. This avoids setting FailedOperation and entering the failed-file deletion path while retaining the modified and FileDeleted state.

PlexCleaner/Process.cs

Tests (1) +57 / -0

OrphanSidecarTests.csCover orphan deletion and matching-MKV preservation +57/-0

Cover orphan deletion and matching-MKV preservation

• Adds ProcessFile tests asserting that an orphan sidecar is deleted with a successful result, modified flag, FileDeleted state, and no failed operation. A second test verifies that a sidecar with its MKV present is kept and ignored.

PlexCleanerTests/OrphanSidecarTests.cs

Documentation (2) +4 / -5

ARCHITECTURE.mdDocument orphan deletion as a successful pipeline exit +2/-5

Document orphan deletion as a successful pipeline exit

• Routes the orphan-sidecar branch to the success node, names it as an exception to the usual false-return behavior, and removes the resolved S3 suspect entry.

ARCHITECTURE.md

HISTORY.mdRecord the orphan-sidecar fix in version 3.23 +2/-0

Record the orphan-sidecar fix in version 3.23

• Notes that deleting an orphan sidecar now counts as successful cleanup rather than reporting a failed step.

HISTORY.md

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

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

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

Approval should wait until the required media-dependent Docker/regression runtime validation is completed, since the PR reports only unit and lint checks.

No open issues 🎉

What changed in this PR

This pull request fixes orphan .PlexCleaner sidecar handling so deleting a sidecar without a matching MKV is reported as a successful cleanup rather than a failure.

Changes:

• Treats orphan sidecar deletion as a successful result.
• Adds regression tests for orphan and matching sidecar scenarios.
• Updates the architecture documentation and Version 3.23 history.

File	Description
PlexCleanerTests/​OrphanSidecarTests.cs	Verifies orphan deletion and matching-sidecar behavior, including result state and failure reporting.
PlexCleaner/​Process.cs	Counts orphan sidecar deletion as a successful result.
HISTORY.md	Records the behavior change in Version 3.23.
ARCHITECTURE.md	Documents the corrected success path and removes the resolved suspect branch.

_{Review effort: Lite}

[ptr727]

On the review headline asking for a media-dependent Docker or regression run before approval:

• The changed branch is reached only for a .PlexCleaner file with no matching .mkv (ProcessFile.DeleteMismatchedSidecarFile(), PlexCleaner/ProcessFile.cs:40).
• It ends the file at step P02 in Process.ProcessFile(), before GetMediaProps(), so no media file is opened and no media tool runs on this path.
• A run over the regression media collection or the Docker test media never reaches it, since every input there is a media file, so such a run would not exercise this change.
• OrphanSidecarTests.ProcessFile_OrphanSidecar_DeletesAndSucceeds drives Process.ProcessFile() through the branch end to end and fails on the code before this change. ProcessFile_SidecarWithMkv_IgnoresAndKeeps covers the sibling branch.

OPERATIONS.md "Local Verification" asks for a real-media run because processing behavior depends on the media and the external tools. This branch depends on neither. Whether to run the image test anyway is left to the maintainer at merge.