[ptr727]

Summary

Implements the maintainer decisions recorded on #1076 and #1078.

Repeat conversions (#1076)

• On the automatic process path (conditional), ReEncode() skips a file already carrying ReEncoded, and DeInterlace() skips a file already carrying DeInterlaced. Each logs a warning naming the tracks that still match (Id:Format:Codec).
• The explicit reencode and deinterlace commands (MkvProcess, conditional false) still force the conversion, with the existing "already reencoded/deinterlaced" warning.
• A file replaced by the user is unaffected: a media size or hash change resets the sidecar state to FileModified, which clears both flags.

Bitrate and packet analysis (#1078)

• VerifyBitrate() sets BitrateExceeded when the measurement exceeds MaximumBitrate and clears it when it does not.
• A failed bitrate analysis logs a warning and does not fail the file. A plain non-zero FfProbe exit records no tool error, so the file still verifies and keeps its last measured BitrateExceeded.
• A bitrate analysis that hit a tool error (a launch failure, a cancellation, or a signal) leaves Verified unset, so the next run verifies and measures again.
• A repair whose re-encode tier replaced the media measures the bitrate again after the refresh (SetRepairedState(), shared by the stream repair and the timestamp repair). The setts and remux tiers do not change the stream payload, so they do not re-measure.
• RepairTimestampsAndSetState() checks for a tool error right after the packet analysis and stops before the setts, remux, and re-encode tiers. A plain packet analysis failure logs a warning and falls back to the remux and re-encode tiers as before.

Tests

PlexCleanerTests/ReEncodeBitrateStateTests.cs covers each path with fake tools:

• ReEncode() and DeInterlace() skip only when conditional, and convert again when explicit.
• A measurement sets or clears BitrateExceeded, and the state persists to the next run.
• A failed analysis keeps the state and verifies, and a tool error leaves Verified unset and the next run verifies again.
• A re-encode repair measures again and clears a stale BitrateExceeded, and a tool error in that re-measure leaves Verified unset.
• A remux repair after a bitrate tool error leaves Verified unset.
• A packet analysis tool error runs no repair tier.

PlexCleanerTests/FakeTools.cs gains WriteSampleToolData(directory) and a seeded state parameter on CopyUnverifiedSample() (default None, existing callers unchanged), and the new test class uses both.

The full suite passes locally (dotnet test), and .NET Format is clean.

Deferred

The maintainer deferred the HISTORY.md lines and the removal of the ARCHITECTURE.md "Suspect Branches" entries (S9, S11) and their diagram labels to one final docs pull request. This pull request changes no method name a diagram names, so ArchitectureDiagramTests needs no ARCHITECTURE.md edit.

Pre-existing findings, filed

• SidecarRenameStateTests, ScanStateTests, and VerifyToolErrorTests (design, consolidation): these build the sample tool data fakes or seed the sidecar state by hand rather than through the new FakeTools helpers. Moving them is outside this change's file claim. Filed as Consolidate Fake Tool Fixture Setup in FakeTools #1124.

Labels

Carries the comments label, since the change adds code comments that state a non-obvious why.

Closes on promotion: #1076, #1078

🤖 Generated with Claude Code

[ptr727]

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

[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 37 minutes.

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: b568b15c-d87d-4e6e-a9f1-1b49c1038a48

Commits

Reviewing files that changed from the base of the PR and between 8ce45be and b18a288.

Files selected for processing: 3

• PlexCleaner/ProcessFile.cs
• PlexCleanerTests/FakeTools.cs
• PlexCleanerTests/ReEncodeBitrateStateTests.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

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

PR Summary by Qodo

Skip Repeat Conversions and Keep Bitrate State Current

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Skip repeat conversions during automatic processing while preserving explicit conversion commands.
• Keep bitrate state current after measurement and re-encode repairs; retry verification after tool
 errors.
• Stop timestamp repairs on packet-analysis tool errors and cover these paths with fake-tool tests.

Diagram

graph TD
  A["Process file"] --> B{"Already converted?"} -->|no or explicit| D["Convert tracks"] --> E["Verify bitrate"] --> F["Repair ladder"] --> G["Sidecar state"]
  B -->|automatic repeat| C["Skip conversion"] --> E
  E -->|clean streams| G
  F -->|re-encoded media| E

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Invalidate bitrate on every media replacement

• ➕ Provides a single policy for all payload-changing operations.
• ➖ Requires broader changes to replacement paths and may trigger unnecessary measurements after remuxes.

Recommendation: Keep the targeted re-measurement after re-encode repairs: it addresses stale bitrate state without paying the analysis cost for lossless repair tiers. A centralized invalidation policy would be worthwhile if more payload-changing paths need the same guarantee.

Files changed (2) +458 / -31

Bug fix (1) +83 / -31

ProcessFile.csGuard repeat conversions and reconcile verification state +83/-31

Guard repeat conversions and reconcile verification state

• Conditional re-encode and deinterlace now skip media carrying their respective completion flags, while explicit requests still convert. Bitrate measurements set or clear BitrateExceeded; repair re-encodes trigger another measurement after refresh, and tool errors leave Verified unset for retry. Packet-analysis tool errors now stop timestamp repair before any repair tier runs.

PlexCleaner/ProcessFile.cs

Tests (1) +375 / -0

ReEncodeBitrateStateTests.csExercise conversion guards and persisted bitrate state +375/-0

Exercise conversion guards and persisted bitrate state

• Adds fake-tool tests for conditional versus explicit conversions, bitrate changes across runs, analysis failures, and state after re-encode or remux repair. Tests also assert that a packet-analysis tool error prevents timestamp repair tiers from running.

PlexCleanerTests/ReEncodeBitrateStateTests.cs

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

Code Review by Qodo

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

1. New tests duplicate sample tool fakes ✓ Resolved

Description

WriteSampleToolData() decompresses the stored sample data and writes separate fake-tool scripts
inside ReEncodeBitrateStateTests. This repeats the sample tool setup that belongs in the shared
test helper and leaves this test class responsible for maintaining its own copy.

Code

PlexCleanerTests/ReEncodeBitrateStateTests.cs[R132-139]

+        string sidecarFile = Path.Combine(_directory, "name.PlexCleaner");
+        JsonNode stored = JsonNode.Parse(File.ReadAllText(sidecarFile))!;
+        foreach (string key in new[] { "MediaInfoData", "MkvMergeInfoData", "FfProbeInfoData" })
+        {
+            File.WriteAllText(
+                Path.Combine(_directory, key + ".json"),
+                StringCompression.Decompress(stored[key]!.GetValue<string>())
+            );

Evidence

The new test class directly decompresses the stored MediaInfoData, MkvMergeInfoData, and
FfProbeInfoData values and writes tool scripts, duplicating the sample fake setup that the rule
requires to be centralized.

Centralize sample tool data fakes
PlexCleanerTests/ReEncodeBitrateStateTests.cs[129-155]

Agent prompt

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

## Issue description
`ReEncodeBitrateStateTests` duplicates the sample tool data decompression and fake-writing setup instead of using the shared helper required by the compliance rule.
## Fix Focus Areas
- PlexCleanerTests/ReEncodeBitrateStateTests.cs[129-155]
- PlexCleanerTests/FakeTools.cs[1-38]
## Recommended Fix
Move or reuse the common sample tool data and fake-writing setup through a helper in `PlexCleanerTests/FakeTools.cs`, then replace `WriteSampleToolData()` in `ReEncodeBitrateStateTests` with that shared helper.

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

---

2. New tests reparse state after copying ✓ Resolved

Description

CreateProcessFile() calls CopyUnverifiedSample() without a seeded state and then parses and
rewrites the sidecar solely to set State. Every state-seeded test in this class takes this path,
so the helper's seeded-state contract remains unused and sidecar setup stays duplicated.

Code

PlexCleanerTests/ReEncodeBitrateStateTests.cs[R69-75]

+    private ProcessFile CreateProcessFile(SidecarFile.StatesType state)
+    {
+        string mediaFile = FakeTools.CopyUnverifiedSample(_directory);
+        string sidecarFile = Path.ChangeExtension(mediaFile, ".PlexCleaner");
+        JsonNode sidecar = JsonNode.Parse(File.ReadAllText(sidecarFile))!;
+        sidecar["State"] = (int)state;
+        File.WriteAllText(sidecarFile, sidecar.ToJsonString());

Evidence

The new test helper invokes CopyUnverifiedSample(_directory) and then reads and rewrites the
sidecar solely to set State; the shared copy helper currently always seeds None and has no state
parameter.

Seed sidecar state when copying the unverified sample
PlexCleanerTests/ReEncodeBitrateStateTests.cs[69-75]
PlexCleanerTests/FakeTools.cs[21-35]

Agent prompt

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

## Issue description
`ReEncodeBitrateStateTests` copies the unverified sample and immediately reparses its sidecar only to assign the requested state, while `CopyUnverifiedSample()` should accept a seeded state with a default of `None`.
## Fix Focus Areas
- PlexCleanerTests/ReEncodeBitrateStateTests.cs[69-75]
- PlexCleanerTests/FakeTools.cs[21-35]
## Recommended Fix
Add an optional `SidecarFile.StatesType state = SidecarFile.StatesType.None` parameter to `FakeTools.CopyUnverifiedSample()`, assign that value while creating the copied sidecar, and pass the requested state directly from `CreateProcessFile()` without reparsing or rewriting the sidecar.

ⓘ 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]

🟢 Approval recommended

The review records no active issues as the basis for approval.

No open issues 🎉

What changed in this PR

This PR avoids repeat conversions during automatic processing and keeps bitrate verification state current across verification and repair paths.

Changes:

• Skips already re-encoded or deinterlaced media during conditional processing while preserving explicit command behavior.
• Tracks bitrate-analysis errors, updates BitrateExceeded, and remeasures bitrate after re-encode repairs.
• Adds regression tests for conversion guards, bitrate state transitions, repair behavior, and packet-analysis failures.

File	Summary	Review findings
PlexCleaner/​ProcessFile.cs	Implements conversion guards, bitrate-state management, retry handling, repair-tier behavior, and post-repair measurement.	Three moderate findings remain regarding retryable bitrate tool-error states, re-encode fallback after plain packet-analysis failure, and persistence on the explicit verify path. Two nit findings address comment grammar and reporting all matching interlaced tracks.
PlexCleanerTests/​ReEncodeBitrateStateTests.cs	Adds fake-tool coverage for conditional versus explicit conversions, bitrate persistence, repair remeasurement, and analysis failures.	No final findings supplied.

_{Review effort: Lite}

[ptr727]

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

[ptr727]

Qodo finding "2. New tests reparse state after copying": fixed in b18a288. FakeTools.CopyUnverifiedSample() takes a seeded state (default None), and CreateProcessFile() passes it instead of rewriting the sidecar. Qodo resolved the thread itself on that push. The other test classes that seed state by hand move under #1124.

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

Qodo finding "2. New tests reparse state after copying": fixed in b18a288. FakeTools.CopyUnverifiedSample() takes a seeded state (default None), and CreateProcessFile() passes it instead of rewriting the sidecar. Qodo resolved the thread itself on that push. The other test classes that seed state by hand move under #1124.

Confirmed — finding 2, “New tests reparse state after copying,” is resolved by commit b18a288. The seeded-state helper is now used directly, and the thread was resolved. Moving the remaining hand-seeded state setup under issue #1124 is consistent with that cleanup.