[ptr727]

With the Verify option off, Verify() passed and VerifyAndRepair() set Verified and cleared VerifyFailed. Enabling Verify later skipped those files. A metadata repair's VerifyFailed was cleared on the next run, so the repair ran again on every other run.

• VerifyAndRepair() returns with the state unchanged when the Verify option is off, so Verified is set only by a verification that ran. The unreachable option check in Verify() is removed, and the PostDecodeTimestamps clear no longer needs its own option check.
• Verified also kept the idet and closed caption scans from running again. The new ScannedInterlace and ScannedCaptions state flags record a clean scan of the whole file, so each scan runs once per file without a verify. A scan that finds something sets no flag, since DeInterlaced or ClearedCaptions covers that case. A --quickscan caption sample sets no flag, as the interlace scan already skips quickscan.
• Each scan still also skips on Verified, the maintainer's choice in this session. Existing sidecars then cost no full-library rescan on upgrade. A file already Verified still skips a scan enabled later, as it does today.
• Every step that only changes a file's state saves it through the new SaveState(), which skips Refresh(false) with UseSidecarFiles off. Without a sidecar that refresh ran the three media property tools again on unchanged media, to save a state that is never written.
• SidecarFile.Read() clears both scan flags along with Verified on a tool change with SidecarUpdateOnToolChange on.
• ScanStateTests drives both scans with fake tools: a clean scan sets its flag and the next run launches no tool, a failed scan and a quickscan sample set nothing, and a tool change clears the flags. VerifyToolErrorTests covers the Verify-off case, which leaves VerifyFailed and runs no tool.
• ARCHITECTURE.md drops suspect branch S4 and redraws the Closed Captions, Deinterlace, Sidecar Refresh and Verify and Repair diagrams and the state table. HISTORY.md records the change under 3.23.

A sidecar an earlier version wrote with the Verify option off can still hold Verified. Nothing tells it apart from a real verification, so it is not re-verified automatically, and HISTORY.md points at createsidecar to reset it.

Local frontier strict-review passes ran before each push. Every finding was on text this change wrote and is fixed, and the last pass on each head raised none.

The comments label is set for the comment on SaveState().

Fixes #1071

🤖 Generated with Claude Code

[ptr727]

A recorded local strict-review pass covers head 145c98740492f532e7b4b4b3b967c8dc79fde345, 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 2 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: d6f956cc-9f41-4e81-b04f-71e19646e6f6

Commits

Reviewing files that changed from the base of the PR and between 6f38b1c and 00ecf79.

Files selected for processing: 6

• ARCHITECTURE.md
• HISTORY.md
• PlexCleaner/ProcessFile.cs
• PlexCleaner/SidecarFile.cs
• PlexCleanerTests/ScanStateTests.cs
• PlexCleanerTests/VerifyToolErrorTests.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

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

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

PR Summary by Qodo

Set Verified Only After Verification Runs

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

AI Description

• Leave verification state unchanged when Verify is off, preserving failures and allowing later
 verification.
• Cache clean, full-file interlace and caption scans independently; clear those flags on tool
 changes.
• Add regression tests and document the state behavior and legacy-sidecar limitation.

Diagram

graph TD
  F["Media file"] --> O{"Verify enabled?"} -->|yes| V["Run verification"] --> S["Sidecar flags"] --> G{"Full scan needed?"} -->|yes| X["Caption or idet"] -->|clean full scan| S
  O -->|no, unchanged| S
  G -->|no| K["Skip scan"]
  T["Tool change"] -->|clear cached flags| S

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Require scan flags even for Verified files

• ➕ Enabling a scan later would inspect files already marked Verified.
• ➖ Existing verified libraries would incur full-file scans after upgrading.
• ➖ Verification would no longer serve as the existing shortcut for those scans.

Recommendation: Keep the PR's separate scan flags while retaining Verified as a scan shortcut: it fixes repeated scans without forcing an upgrade-wide rescan. The trade-off is that previously Verified files still skip scans enabled later; legacy sidecars incorrectly marked Verified also require an explicit createsidecar reset.

Files changed (6) +260 / -47

Bug fix (2) +27 / -21

ProcessFile.csGate verification state and cache clean full-file scans +20/-17

Gate verification state and cache clean full-file scans

• VerifyAndRepair now returns without changing state when Verify is off. Clean interlace and caption scans persist independent flags; quickscan caption samples, detections, and failed scans do not. Removes redundant Verify-option checks.

PlexCleaner/ProcessFile.cs

SidecarFile.csPersist and invalidate scan-completion flags +7/-4

Persist and invalidate scan-completion flags

• Adds ScannedInterlace and ScannedCaptions state bits. When configured to react to a tool change, sidecar reads clear both flags alongside Verified.

PlexCleaner/SidecarFile.cs

Tests (2) +206 / -5

ScanStateTests.csTest scan caching and invalidation +176/-0

Test scan caching and invalidation

• Adds fake-tool tests showing clean scans persist and skip tools on the next run, while a failed scan and quickscan caption sample leave state unchanged. Also tests clearing cached scan flags on a tool change.

PlexCleanerTests/ScanStateTests.cs

VerifyToolErrorTests.csTest state preservation when Verify is off +30/-5

Test state preservation when Verify is off

• Adjusts the PostDecodeTimestamps expectation so Verify-off processing does not mark a file Verified. Adds a regression test confirming VerifyFailed persists across runs without launching a verification tool.

PlexCleanerTests/VerifyToolErrorTests.cs

Documentation (2) +27 / -21

ARCHITECTURE.mdDocument verification and scan-state transitions +22/-21

Document verification and scan-state transitions

• Updates the sidecar, caption, interlace, and verification diagrams and state table to reflect the new scan flags and Verify-off behavior. Removes the resolved S4 suspect branch.

ARCHITECTURE.md

HISTORY.mdRecord the verification-state fix +5/-0

Record the verification-state fix

• Describes the new scan caching and Verify-off behavior under version 3.23. Notes that createsidecar is needed to reset sidecars incorrectly marked Verified by earlier versions.

HISTORY.md

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

Code Review by Qodo

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

1. Rewritten videos skip fresh scan checks 🐞 Bug ≡ Correctness

Description

FindInterlacedTracks() and FindClosedCaptionTracks() persist clean-scan flags, but later
rewrites retain those flags while updating the sidecar to describe the new media. When re-encoding
or another later processing step changes the video, the next run considers the sidecar current and
skips scans of content the flags never covered.

Code

PlexCleaner/ProcessFile.cs[R1197-1198]

+            _sidecarFile.State |= SidecarFile.StatesType.ScannedInterlace;
+            return Refresh(false);

Evidence

The new flags skip conditional scans. Re-encoding follows both scans in the processing pipeline, and
same-instance sidecar updates preserve state rather than invoking the media-change invalidation in
Read().

PlexCleaner/ProcessFile.cs[1172-1198]
PlexCleaner/ProcessFile.cs[1229-1271]
PlexCleaner/Process.cs[243-260]
PlexCleaner/Process.cs[301-314]
PlexCleaner/ProcessFile.cs[1636-1652]
PlexCleaner/SidecarFile.cs[127-140]
PlexCleaner/SidecarFile.cs[211-244]

Agent prompt

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

## Issue description
Clean-scan flags survive later rewrites of the media they describe, so subsequent runs skip scans of the rewritten video.
## Fix Focus Areas
- PlexCleaner/ProcessFile.cs[1193-1198]
- PlexCleaner/ProcessFile.cs[1265-1271]
- PlexCleaner/ProcessFile.cs[2432-2463]
- PlexCleaner/SidecarFile.cs[211-244]
## Recommended Fix
Clear both clean-scan flags when video content is rewritten, before the refreshed media information and state are saved. Preserve the flags when only scan state is being saved, and test a scan followed by a rewrite and a subsequent processing run.

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

---

2. Clean scans can fail without sidecars ✓ Resolved

Description

Both clean-scan branches now call Refresh(false), which runs all three media-property tools when
UseSidecarFiles is false rather than only recording the in-memory flag. With sidecars disabled, a
successful scan therefore incurs three extra probes and can fail the file if any of those unrelated
probes fails.

Code

PlexCleaner/ProcessFile.cs[R1197-1198]

+            _sidecarFile.State |= SidecarFile.StatesType.ScannedInterlace;
+            return Refresh(false);

Evidence

The two new refresh calls reach Refresh's no-sidecar branch, where GetMediaProps invokes ffprobe,
mkvmerge, and MediaInfo and returns false if any fails.

PlexCleaner/ProcessFile.cs[1193-1198]
PlexCleaner/ProcessFile.cs[1265-1271]
PlexCleaner/ProcessFile.cs[2445-2482]
PlexCleaner/MediaProps.cs[98-129]

Agent prompt

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

## Issue description
Saving a clean-scan flag invokes media-property tools unnecessarily when sidecars are disabled, and their failure turns a successful scan into a failed file.
## Fix Focus Areas
- PlexCleaner/ProcessFile.cs[1193-1198]
- PlexCleaner/ProcessFile.cs[1265-1271]
- PlexCleaner/ProcessFile.cs[2445-2482]
## Recommended Fix
After setting a clean-scan flag, call Refresh(false) only when sidecars are enabled; otherwise return the successful scan result without probing unchanged media again. Cover both scan paths with a sidecar-disabled test.

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

---

3. Video tool upgrades leave scans stale 🐞 Bug ≡ Correctness

Description

ScannedInterlace skips the ffmpeg idet scan, but SidecarFile.IsToolsCurrent() cannot invalidate
that result because it does not compare an ffmpeg version. When ffmpeg changes without one of the
three tracked media-property tools changing, a previously clean sidecar keeps skipping idet despite
SidecarUpdateOnToolChange being enabled.

Code

PlexCleaner/ProcessFile.cs[R1174-1175]

+                State.HasFlag(SidecarFile.StatesType.ScannedInterlace)
+                || State.HasFlag(SidecarFile.StatesType.Verified)

Evidence

Idet is executed by ffmpeg, but the new skip flag is cleared on tool changes only when
IsToolsCurrent detects a version difference; that comparison omits ffmpeg.

PlexCleaner/ProcessFile.cs[1172-1198]
PlexCleaner/FfMpegIdetInfo.cs[63-85]
PlexCleaner/SidecarFile.cs[145-165]
PlexCleaner/SidecarFile.cs[376-438]

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 new interlace cache depends on ffmpeg, but sidecar tool-change detection tracks only other tools, leaving the cache valid after an ffmpeg upgrade.
## Fix Focus Areas
- PlexCleaner/ProcessFile.cs[1172-1178]
- PlexCleaner/SidecarFile.cs[145-165]
- PlexCleaner/SidecarFile.cs[376-438]
- PlexCleaner/SidecarFileJsonSchema.cs[105-126]
## Recommended Fix
Persist and compare the ffmpeg version used for an interlace scan, with migration behavior for existing sidecars. Clear ScannedInterlace when that version changes, and add a test that changes only ffmpeg while leaving the other tool versions unchanged.

ⓘ 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 route each severity your way: inline, summary, both, or drop

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[Copilot]

🟡 Changes recommended

Two critical findings remain: later media rewrites can preserve stale interlace and caption scan flags, causing subsequent runs to skip required scans of the new content.

2 open issues

• Clear interlace scan state after media rewrites · New
• Clear captions scan state after media rewrites · New

What changed in this PR

This pull request ensures verification state is recorded only when verification runs and adds independent persisted state for successful interlace and closed-caption scans.

Changes:

• Preserves verification state when Verify is disabled.
• Adds ScannedInterlace and ScannedCaptions flags with tool-change invalidation.
• Expands regression tests for verification and scan-state behavior.
• Updates architecture documentation and 3.23 release history.

File	Description
PlexCleanerTests/​VerifyToolErrorTests.cs	Tests that disabled verification preserves existing state and launches no tools.
PlexCleanerTests/​ScanStateTests.cs	Tests clean, failed, quickscan, and tool-change scan-state behavior.
PlexCleaner/​SidecarFile.cs	Adds scan flags and clears them when configured tools change.
PlexCleaner/​ProcessFile.cs	Controls verification and scan-state behavior.
HISTORY.md	Records the behavior changes for version 3.23.
ARCHITECTURE.md	Documents the updated verification, scanning, sidecar, and state-flow behavior.

_{Review effort: Lite}

[ptr727]

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

[ptr727]

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

[ptr727]

Answering Qodo's two open comment-only findings, each declined with evidence in its review thread:

1. Rewritten videos skip fresh scan checks

Declined in #1111 (comment). No later rewrite adds embedded captions or interlacing to media whose full-file scan was clean. The metadata checks run on every pass before either flag is read, on the props Refresh(true) re-reads after a rewrite. ARCHITECTURE.md Sidecar Refresh states this as of 00ecf79.

3. Video tool upgrades leave scans stale

Declined in #1111 (comment). FfMpeg and FfProbe are one tool family, installed from one package with one minimum version. The sidecar's FfProbeToolVersion therefore changes with an FfMpeg upgrade, and SidecarFile.Read() clears ScannedInterlace with SidecarUpdateOnToolChange on, which Read_ToolChange_ClearsVerifiedAndScannedFlags covers.