[ptr727]

Adds the test #1151 asks for. It drives ProcessFile.RepairTimestamps() through the lossless setts tier to a successful repair, the one FFmpeg rewrite path that had no fake-tool test of its language restore.

• ReplaceOriginalTests.RepairTimestamps_LosslessRepair_ReplacesOriginalOnlyAfterLanguageRestore runs with MkvPropEdit exiting 0 and 2.
  • FfProbe returns packets with a non-monotonic DTS on the audio stream only, so the lossless tier runs, and the same stream timings for every file.
  • FfMpeg reports the DTS line when verifying any file other than name.tmp14, writes name.tmp14 for the setts command, and returns the same streamhash lines for every file. The re-encode exits 1, and so does HandBrake, so no later tier can replace the original.
  • MkvMerge answers the identify with the sample tracks, and the remux exits 2 unless its name.tmp14 input exists. So a skipped setts rewrite fails the test rather than the remux creating the temp file.
  • MkvPropEdit logs the file it edits together with that file's content. The test asserts the log is exactly name.tmp14 mkvmerge. That shows the edit ran on the MkvMerge output, before the move removed the temp file.
  • On success the original holds the MkvMerge output and the state is Verified | Repaired. When the edit fails, the original is byte-identical, the state is VerifyFailed | RepairFailed, and no name.tmp* file is left behind.
• FakeTools.WritePackets now holds the packet JSON writer that ReEncodeBitrateStateTests.CatPackets used to build inline. Both tests use it, and the JSON CatPackets writes is unchanged.

The test carries comment lines that say why the fakes are built as they are, so this PR carries the comments label.

Verification: the full .NET suite passes (485 tests), and CSharpier and dotnet format style are clean. To check that the test bites, I changed TryLosslessTimestampRepair() four ways. Skipping the SetTimestamps call failed both cases. Dropping the RestoreLanguageTags call failed both cases. Moving it after the File.Move failed both cases. Dropping the File.Move failed the success case. Windows skips the test, as it does every other shell-script fake-tool test.

Pre-existing findings, not filed

• PlexCleaner/ProcessFile.cs TimestampRepairRegressionGate(): test coverage. No test drives the gate's reject path (a streamhash mismatch, or a timing shift beyond SyncToleranceSeconds), so a gate stuck at pass goes unnoticed. This PR covers only the pass path, as Test the Lossless Timestamp Repair Through a Successful Repair #1151 asks.

🤖 Generated with Claude Code

[ptr727]

A recorded local strict-review pass covers head 1e1ea8b7859dc804a96ac46572a601009cf649e2, 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 4 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: f63440c5-8c25-4efe-8624-2222468f028c

Commits

Reviewing files that changed from the base of the PR and between 753f49b and c7b6c99.

Files selected for processing: 3

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

Test successful lossless timestamp repair and language restoration

🧪 Tests 🕐 20-40 Minutes

AI Description

• Cover successful lossless timestamp repair and failed language restoration with fake tools.
• Verify the original is replaced only after restoration succeeds, with state and temporary-file
 assertions.
• Share packet JSON generation between timestamp and bitrate tests.

Diagram

graph TD
  A["Packet fixtures"] --> B["RepairTimestamps"] --> C["Setts and remux"] --> D["Regression gate"] --> E{"Restore tags"} -->|success| F["Replace original"]
  E -->|failure| G["Preserve original"]

Loading

High-Level Assessment

The fake-tool integration test is well suited to checking the real replacement order and persisted state; an isolated mock would not establish both. Sharing the existing packet fixture format avoids duplicating JSON construction.

Files changed (3) +137 / -17

Other (3) +137 / -17

FakeTools.csAdd a shared packet JSON fixture writer +26/-0

Add a shared packet JSON fixture writer

• Adds WritePackets to create FFprobe-compatible packet JSON from codec, stream, DTS, and size tuples. Timestamp and bitrate tests can now use the same fixture format.

PlexCleanerTests/FakeTools.cs

ReEncodeBitrateStateTests.csReuse the shared packet writer for bitrate fixtures +5/-17

Reuse the shared packet writer for bitrate fixtures

• Replaces inline packet JSON construction with FakeTools.WritePackets. The emitted packet data remains unchanged.

PlexCleanerTests/ReEncodeBitrateStateTests.cs

ReplaceOriginalTests.csCover lossless timestamp repair replacement on restore success and failure +106/-0

Cover lossless timestamp repair replacement on restore success and failure

• Adds fake packet, timing, and tool responses that drive RepairTimestamps through the lossless setts tier. Both MkvPropEdit outcomes assert the edited temporary file, return and modified flags, persisted state, original-file contents, and temporary-file cleanup; the shell-script test skips Windows.

PlexCleanerTests/ReplaceOriginalTests.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. A skipped rewrite passes the repair test ✓ Resolved

Description

The test’s setts fake has no invocation assertion, while the sample MkvMerge fake can create
name.tmp14 even when FFmpeg never writes it. If the timestamp rewrite is skipped but the remux
still runs, the fake remux supplies the temporary file and the remaining verification,
language-edit, replacement, and state assertions can all pass.

Code

PlexCleanerTests/ReplaceOriginalTests.cs[240]

+            *setts*) for a; do case $a in *.tmp14) echo ffmpeg > "$a" ;; esac; done ;;

Evidence

TryLosslessTimestampRepair normally writes .tmp14 with FFmpeg before calling Convert.ReMux.
The shared MkvMerge fake instead writes to every *.tmp* argument, including the remux input,
without checking it exists; the new test’s hash, timing, and verification fakes then accept that
fabricated file.

PlexCleaner/ProcessFile.cs[2558-2595]
PlexCleaner/Convert.cs[145-162]
PlexCleanerTests/FakeTools.cs[108-119]
PlexCleanerTests/ReplaceOriginalTests.cs[235-264]

Agent prompt

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

## Issue description
The lossless repair test can pass without running the FFmpeg timestamp rewrite because its MkvMerge fake creates temporary files without requiring an existing input.
## Fix Focus Areas
- PlexCleanerTests/ReplaceOriginalTests.cs[228-264]
- PlexCleanerTests/FakeTools.cs[108-119]
## Recommended Fix
For this test, make the remux fake fail unless its `name.tmp14` input already exists, and assert that the `setts` branch ran. Keep the assertion that language editing occurs on the remuxed temporary file before replacement.

ⓘ 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 show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[Copilot]

🟢 Approval recommended

The review found no active issues, with targeted success and failure coverage for lossless timestamp repair—including restoration, replacement, state updates, cleanup, and original-file preservation—plus reusable packet-fixture generation.

No open issues 🎉

What changed in this PR

Adds fake-tool coverage for the successful lossless timestamp-repair path, verifying language restoration before replacing the original file and preserving the original when restoration fails. It also consolidates packet-fixture generation for reuse across tests.

Changes:

• Adds success and failure cases for RepairTimestamps() through the lossless repair tier.
• Verifies temporary-file handling, state updates, original-file preservation, and cleanup.
• Extracts packet JSON generation into FakeTools.WritePackets().

File	Description
PlexCleanerTests/​ReplaceOriginalTests.cs	Adds lossless timestamp-repair replacement and language-restore coverage.
PlexCleanerTests/​ReEncodeBitrateStateTests.cs	Reuses the shared packet fixture helper.
PlexCleanerTests/​FakeTools.cs	Adds the shared JSON packet fixture writer.

This pull request is ready to be approved.

_{Review effort: Lite}

[ptr727]

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

[ptr727]

Qodo finding 1, "A skipped rewrite passes the repair test": fixed in 960c0e9, with the why comment in c7b6c99. The test now has its own MkvMerge fake that exits 2 unless its name.tmp14 input exists, and writes only the tmp4 and tmp11 outputs. With the SetTimestamps call replaced by a no-op, both cases now fail. The setts invocation needs no separate assertion: name.tmp14 can only exist if the setts arm wrote it, so a passing run proves setts ran.