code/backend/Cleanuparr.Infrastructure/Features/Arr/WhisparrV2Client.cs (1)

47-61: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Duplicate of RadarrClient.GetQueueDeleteUrlQuery — see the refactor suggestion on RadarrClient.cs.

The implementation here is byte-for-byte identical to RadarrClient's override. Consolidating into ArrClient as described in the RadarrClient.cs comment resolves both.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@code/backend/Cleanuparr.Infrastructure/Features/Arr/WhisparrV2Client.cs`
around lines 47 - 61, This override of GetQueueDeleteUrlQuery in
WhisparrV2Client is identical to RadarrClient.GetQueueDeleteUrlQuery; remove the
duplicate from WhisparrV2Client and implement the shared logic once in the base
ArrClient (e.g., add a protected virtual or concrete GetQueueDeleteUrlQuery
implementation there), then have both WhisparrV2Client and RadarrClient
inherit/override only if behavior diverges; reference the method name
GetQueueDeleteUrlQuery and the classes WhisparrV2Client, RadarrClient, and
ArrClient when making the refactor.

code/backend/Cleanuparr.Infrastructure.Tests/Features/DownloadClient/DelugeServiceTests.cs (1)

416-498: ⚡ Quick win

Same result.ChangeCategory assertion gap as in UTorrentServiceTests.cs.

Add result.ChangeCategory.ShouldBeFalse() to both SlowDownload_MatchesRule_RemovesFromQueue (line 453) and StalledDownload_MatchesRule_RemovesFromQueue (line 497), and a ChangeCategory = true propagation test.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@code/backend/Cleanuparr.Infrastructure.Tests/Features/DownloadClient/DelugeServiceTests.cs`
around lines 416 - 498, The tests SlowDownload_MatchesRule_RemovesFromQueue and
StalledDownload_MatchesRule_RemovesFromQueue are missing assertions for
result.ChangeCategory; add result.ChangeCategory.ShouldBeFalse() to both tests
to mirror UTorrentServiceTests expectations, and add an additional unit test
that sets the rule evaluator to return ChangeCategory = true (e.g., Arrange
EvaluateSlowRulesAsync / EvaluateStallRulesAsync to return (true,
DeleteReason.*, true, true)), call ShouldRemoveFromArrQueueAsync on the SUT, and
assert result.ChangeCategory.ShouldBeTrue() along with the existing DeleteReason
and DeleteFromClient checks so ChangeCategory propagation is covered.

code/backend/Cleanuparr.Infrastructure.Tests/Features/DownloadClient/UTorrentServiceTests.cs (1)

521-613: ⚡ Quick win

result.ChangeCategory is never asserted — ChangeCategory propagation is untested.

SlowDownload_MatchesRule_RemovesFromQueue and StalledDownload_MatchesRule_RemovesFromQueue both verify ShouldRemove, DeleteReason, and DeleteFromClient, but neither asserts the new ChangeCategory field. If the assignment at line 73 of UTorrentServiceQC.cs were accidentally removed, all tests would still pass. The same gap exists in TransmissionServiceTests.cs and DelugeServiceTests.cs.

Consider adding:

1. result.ChangeCategory.ShouldBeFalse() to the existing ChangeCategory = false scenarios.
2. A new scenario where the evaluator returns ChangeCategory = true and the result propagates it correctly.

✅ Suggested additions to existing tests + new test

 result.ShouldRemove.ShouldBeTrue();
 result.DeleteReason.ShouldBe(DeleteReason.SlowSpeed);
 result.DeleteFromClient.ShouldBeTrue();
+result.ChangeCategory.ShouldBeFalse();

 result.ShouldRemove.ShouldBeTrue();
 result.DeleteReason.ShouldBe(DeleteReason.Stalled);
 result.DeleteFromClient.ShouldBeTrue();
+result.ChangeCategory.ShouldBeFalse();

New test inside ShouldRemoveFromArrQueueAsync_SlowAndStalledScenarios:

[Fact]
public async Task SlowDownload_ChangeCategoryRule_PropagatesChangeCategoryFlag()
{
    const string hash = "test-hash";
    var sut = _fixture.CreateSut();

    var torrentItem = new UTorrentItem
    {
        Hash = hash, Name = "Test Torrent", Status = 9, DownloadSpeed = 1000
    };
    var torrentProperties = new UTorrentProperties { Hash = hash, Pex = 1, Trackers = "" };

    _fixture.ClientWrapper.GetTorrentAsync(hash).Returns(torrentItem);
    _fixture.ClientWrapper.GetTorrentPropertiesAsync(hash).Returns(torrentProperties);
    _fixture.ClientWrapper.GetTorrentFilesAsync(hash).Returns(new List<UTorrentFile>
    {
        new UTorrentFile { Name = "file1.mkv", Priority = 1, Index = 0, Size = 1000, Downloaded = 500 }
    });

    // ChangeCategory=true → deleteFromClient must be false (rule gates it)
    _fixture.RuleEvaluator
        .EvaluateSlowRulesAsync(Arg.Any<UTorrentItemWrapper>())
        .Returns((true, DeleteReason.SlowSpeed, false, true));

    var result = await sut.ShouldRemoveFromArrQueueAsync(hash, Array.Empty<string>());

    result.ShouldRemove.ShouldBeTrue();
    result.DeleteReason.ShouldBe(DeleteReason.SlowSpeed);
    result.DeleteFromClient.ShouldBeFalse();
    result.ChangeCategory.ShouldBeTrue();
}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@code/backend/Cleanuparr.Infrastructure.Tests/Features/DownloadClient/UTorrentServiceTests.cs`
around lines 521 - 613, The tests for UTorrentService
(SlowDownload_MatchesRule_RemovesFromQueue and
StalledDownload_MatchesRule_RemovesFromQueue) are missing assertions for the new
ChangeCategory propagation; update those tests to assert
result.ChangeCategory.ShouldBeFalse() (since the RuleEvaluator returns
ChangeCategory=false), and add a new test that stubs
RuleEvaluator.EvaluateSlowRulesAsync (or EvaluateStallRulesAsync) to return
ChangeCategory=true and verifies that ShouldRemove is true, DeleteReason
matches, ChangeCategory is true and DeleteFromClient is false (rule gates
deletion when ChangeCategory=true). Apply the same additional ChangeCategory
assertions and a positive ChangeCategory propagation test pattern to
TransmissionServiceTests and DelugeServiceTests, targeting the
ShouldRemoveFromArrQueueAsync flows and the
EvaluateSlowRulesAsync/EvaluateStallRulesAsync call sites.

code/backend/Cleanuparr.Infrastructure.Tests/Features/DownloadClient/TransmissionServiceTests.cs (1)

617-729: ⚡ Quick win

Same result.ChangeCategory assertion gap as in UTorrentServiceTests.cs.

Add result.ChangeCategory.ShouldBeFalse() to both SlowDownload_MatchesRule_RemovesFromQueue (line 668) and StalledDownload_MatchesRule_RemovesFromQueue (line 727), and a ChangeCategory = true propagation test.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@code/backend/Cleanuparr.Infrastructure.Tests/Features/DownloadClient/TransmissionServiceTests.cs`
around lines 617 - 729, The two TransmissionServiceTests
(SlowDownload_MatchesRule_RemovesFromQueue and
StalledDownload_MatchesRule_RemovesFromQueue) are missing assertions for
ChangeCategory; add result.ChangeCategory.ShouldBeFalse() to both tests after
the other ShouldBe* assertions, and add an additional unit test that stubs
RuleEvaluator.EvaluateSlowRulesAsync (or EvaluateStallRulesAsync) to return
(true, DeleteReason.SlowSpeed, true, true) (or the stalled equivalent) and
asserts that result.ChangeCategory is true so ChangeCategory propagation is
covered; locate the tests by their method names and update assertions
accordingly.

code/backend/Cleanuparr.Infrastructure.Tests/Features/DownloadRemover/QueueItemRemoverTests.cs (1)

426-444: ⚡ Quick win

Add a changeCategory parameter to CreateRemoveRequest and a corresponding forwarding test.

The helper always produces a ChangeCategory = false request (unset bool default), so ChangeCategory = true is never exercised. There is a RemoveQueueItemAsync_PassesCorrectRemoveFromClientFlag theory test but no analogous theory for ChangeCategory.

✅ Suggested additions

 private QueueItemRemoveRequest<SearchItem> CreateRemoveRequest(
     InstanceType instanceType = InstanceType.Sonarr,
     bool removeFromClient = true,
     DeleteReason deleteReason = DeleteReason.Stalled,
-    bool skipSearch = false)
+    bool skipSearch = false,
+    bool changeCategory = false)
 {
     var instance = GetOrCreateArrInstance(instanceType);

     return new QueueItemRemoveRequest<SearchItem>
     {
         Instance = instance,
         SearchItem = new SearchItem { Id = 123 },
         Record = CreateQueueRecord(),
         RemoveFromClient = removeFromClient,
+        ChangeCategory = changeCategory,
         DeleteReason = deleteReason,
         SkipSearch = skipSearch,
         JobRunId = _jobRunId
     };
 }

And add a forwarding theory alongside RemoveQueueItemAsync_PassesCorrectRemoveFromClientFlag:

[Theory]
[InlineData(true)]
[InlineData(false)]
public async Task RemoveQueueItemAsync_PassesCorrectChangeCategoryFlag(bool changeCategory)
{
    var request = CreateRemoveRequest(changeCategory: changeCategory);

    await _queueItemRemover.RemoveQueueItemAsync(request);

    await _arrClient.Received(1).DeleteQueueItemAsync(
        Arg.Any<ArrInstance>(),
        Arg.Any<QueueRecord>(),
        Arg.Any<bool>(),
        Arg.Is<bool>(x => x == changeCategory),
        Arg.Any<DeleteReason>());
}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@code/backend/Cleanuparr.Infrastructure.Tests/Features/DownloadRemover/QueueItemRemoverTests.cs`
around lines 426 - 444, Add a new bool parameter changeCategory (default false)
to CreateRemoveRequest and set the returned QueueItemRemoveRequest's
ChangeCategory property to that value so tests can exercise both true/false;
then add a forwarding theory test
RemoveQueueItemAsync_PassesCorrectChangeCategoryFlag that calls
CreateRemoveRequest(changeCategory: changeCategory), invokes
_queueItemRemover.RemoveQueueItemAsync(request), and verifies
_arrClient.Received(1).DeleteQueueItemAsync(..., Arg.Is<bool>(x => x ==
changeCategory), ...) to assert the flag is forwarded.

code/backend/Cleanuparr.Infrastructure/Features/Arr/LidarrClient.cs (1)

45-59: ⚡ Quick win

GetQueueDeleteUrlQuery is duplicated across all ArrClient subclasses — consolidate into the base.

The body of GetQueueDeleteUrlQuery is byte-for-byte identical in LidarrClient, SonarrClient, and RadarrClient (per the context snippet). Since all *arr apps accept the same query parameters ("All inputs are the same in all apps"), there is no per-client variation. The abstract method in ArrClient should become virtual with this shared body as the default; subclasses that genuinely diverge can still override.

♻️ Suggested change in `ArrClient.cs`

-protected abstract string GetQueueDeleteUrlQuery(bool removeFromClient, bool changeCategory);
+protected virtual string GetQueueDeleteUrlQuery(bool removeFromClient, bool changeCategory)
+{
+    string query = "blocklist=true&skipRedownload=true&";
+
+    if (changeCategory)
+    {
+        query += "changeCategory=true&removeFromClient=false";
+        return query;
+    }
+
+    query += "changeCategory=false";
+    query += removeFromClient ? "&removeFromClient=true" : "&removeFromClient=false";
+
+    return query;
+}

Then remove the identical override from LidarrClient, SonarrClient, and RadarrClient (and any other client with the same body).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@code/backend/Cleanuparr.Infrastructure/Features/Arr/LidarrClient.cs` around
lines 45 - 59, The GetQueueDeleteUrlQuery implementation is identical across
ArrClient subclasses; move the shared body into the base ArrClient by changing
the abstract declaration of GetQueueDeleteUrlQuery to a virtual method that
returns the same query string ("blocklist=true&skipRedownload=true&..." with
changeCategory/removeFromClient logic) so it becomes the default behavior in
ArrClient, then remove the duplicate overrides from LidarrClient, SonarrClient,
RadarrClient (and any other subclass with the same body); keep the method
virtual so subclasses that need different behavior can still override it.

code/backend/Cleanuparr.Infrastructure.Tests/Features/Jobs/QueueCleanerTests.cs (1)

1387-1413: ⚡ Quick win

The first ChangeCategory test's assertion on RemoveFromClient == false is not conclusive.

DownloadCheckResult is constructed without setting DeleteFromClient (it defaults to false). This means RemoveFromClient=false in the assertion would be satisfied even if the ChangeCategory override logic were removed entirely. The test doesn't actually prove that ChangeCategory=true forces RemoveFromClient to false.

Compare this with the second ChangeCategory test (lines 1458–1466), which correctly uses DeleteFromClient = true alongside ChangeCategory = true so the RemoveFromClient == false assertion is only satisfiable due to the override.

♻️ Suggested fix — make the FailedImport path conclusive

Add DeleteFromClient = true to the DownloadCheckResult (or use whatever mechanism controls RemoveFromClient in the failed-import path) so the assertion proves the override:

         .Returns(new DownloadCheckResult
         {
             Found = true,
             ShouldRemove = false,
-            IsPrivate = true
+            IsPrivate = true,
+            DeleteFromClient = true  // ensure RemoveFromClient would be true without ChangeCategory override
         });

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@code/backend/Cleanuparr.Infrastructure.Tests/Features/Jobs/QueueCleanerTests.cs`
around lines 1387 - 1413, The test is inconclusive because the mocked
DownloadCheckResult leaves DeleteFromClient at its default false, so asserting
RemoveFromClient == false doesn't prove ChangeCategory forces the override;
update the mock returned by ShouldRemoveFromArrQueueAsync to set
DeleteFromClient = true on the DownloadCheckResult (in the arrangement before
calling CreateSut/ExecuteAsync) so the assertion on
QueueItemRemoveRequest<SeriesSearchItem> (r => r.ChangeCategory == true &&
r.RemoveFromClient == false && r.DeleteReason == DeleteReason.FailedImport) only
passes if the ChangeCategory override logic actually flips RemoveFromClient to
false.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

code/backend/Cleanuparr.Infrastructure/Features/Arr/ArrClient.cs (1)

162-168: ⚡ Quick win

Add XML docs for the updated public API method.

Line 162 introduces a changed public signature (changeCategory) but this method has no XML documentation (or /// <inheritdoc/>). Please add docs so the new parameter contract is discoverable from implementation as well.

As per coding guidelines, "Add XML documentation comments for public APIs in C#".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@code/backend/Cleanuparr.Infrastructure/Features/Arr/ArrClient.cs` around
lines 162 - 168, Add XML documentation comments for the public method
DeleteQueueItemAsync so the new parameter contract is discoverable: add a
<summary> describing what DeleteQueueItemAsync does, and <param> tags for
arrInstance, record, removeFromClient, changeCategory (explain what true/false
means), and deleteReason; include a <returns> describing the returned Task and
any <exception> tags if the method throws specific exceptions. Ensure the doc
sits immediately above the DeleteQueueItemAsync declaration and either fully
documents the parameters or uses <inheritdoc/> with an explicit <param
name="changeCategory"> entry to cover the new parameter.

code/backend/Cleanuparr.Infrastructure.Tests/Features/DownloadClient/UTorrentServiceTests.cs (1)

568-615: ⚡ Quick win

Add a stalled-rule variant where ChangeCategory = true for symmetry.

You covered this for slow rules, but not for stalled rules in this class. A mirrored test would close the remaining path-level gap.

✅ Suggested test addition

+        [Fact]
+        public async Task StalledDownload_RuleWithChangeCategory_PropagatesChangeCategoryFlag()
+        {
+            const string hash = "test-hash";
+            var sut = _fixture.CreateSut();
+
+            var torrentItem = new UTorrentItem
+            {
+                Hash = hash,
+                Name = "Test Torrent",
+                Status = 9,
+                DownloadSpeed = 0,
+                ETA = 0
+            };
+
+            var torrentProperties = new UTorrentProperties
+            {
+                Hash = hash,
+                Pex = 1,
+                Trackers = ""
+            };
+
+            _fixture.ClientWrapper.GetTorrentAsync(hash).Returns(torrentItem);
+            _fixture.ClientWrapper.GetTorrentPropertiesAsync(hash).Returns(torrentProperties);
+            _fixture.ClientWrapper.GetTorrentFilesAsync(hash).Returns(new List<UTorrentFile>
+            {
+                new UTorrentFile { Name = "file1.mkv", Priority = 1, Index = 0, Size = 1000, Downloaded = 500 }
+            });
+
+            _fixture.RuleEvaluator
+                .EvaluateStallRulesAsync(Arg.Any<UTorrentItemWrapper>())
+                .Returns((true, DeleteReason.Stalled, false, true));
+
+            var result = await sut.ShouldRemoveFromArrQueueAsync(hash, Array.Empty<string>());
+
+            result.ShouldRemove.ShouldBeTrue();
+            result.DeleteReason.ShouldBe(DeleteReason.Stalled);
+            result.DeleteFromClient.ShouldBeFalse();
+            result.ChangeCategory.ShouldBeTrue();
+        }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@code/backend/Cleanuparr.Infrastructure.Tests/Features/DownloadClient/UTorrentServiceTests.cs`
around lines 568 - 615, Add a mirrored unit test in UTorrentServiceTests that
exercises the stalled-rule path where ChangeCategory is true: create a new test
similar to StalledDownload_MatchesRule_RemovesFromQueue but have the mocked
RuleEvaluator.EvaluateStallRulesAsync(Arg.Any<UTorrentItemWrapper>()) return
(true, DeleteReason.Stalled, true, true) and assert result.ShouldRemove == true,
result.DeleteReason == DeleteReason.Stalled, result.DeleteFromClient == true and
result.ChangeCategory == true; ensure the test uses the same setup
(UTorrentItem, UTorrentProperties, UTorrentFile list) and calls
ShouldRemoveFromArrQueueAsync(hash, Array.Empty<string>()) on the SUT.

code/backend/Cleanuparr.Infrastructure.Tests/Features/DownloadClient/QBitServiceTests.cs (1)

836-950: ⚡ Quick win

Missing stall-path equivalent of SlowDownload_RuleWithChangeCategory_PropagatesChangeCategoryFlag.

A StalledDownload_RuleWithChangeCategory_PropagatesChangeCategoryFlag test is absent. Since stall-rule evaluation is a separate code path through EvaluateStallRulesAsync, the ChangeCategory propagation from that branch is not exercised by the new test.

✅ Suggested addition (mirrors the new slow test)

+        [Fact]
+        public async Task StalledDownload_RuleWithChangeCategory_PropagatesChangeCategoryFlag()
+        {
+            // Arrange
+            const string hash = "test-hash";
+            var sut = _fixture.CreateSut();
+
+            var torrentInfo = new TorrentInfo
+            {
+                Hash = hash,
+                Name = "Test Torrent",
+                State = TorrentState.StalledDownload
+            };
+
+            _fixture.ClientWrapper
+                .GetTorrentListAsync(Arg.Is<TorrentListQuery>(q => q.Hashes != null && q.Hashes.Contains(hash)))
+                .Returns(new[] { torrentInfo });
+
+            _fixture.ClientWrapper
+                .GetTorrentTrackersAsync(hash)
+                .Returns(Array.Empty<TorrentTracker>());
+
+            var properties = new TorrentProperties
+            {
+                AdditionalData = new Dictionary<string, JToken>
+                {
+                    { "is_private", JToken.FromObject(false) }
+                }
+            };
+
+            _fixture.ClientWrapper
+                .GetTorrentPropertiesAsync(hash)
+                .Returns(properties);
+
+            _fixture.ClientWrapper
+                .GetTorrentContentsAsync(hash)
+                .Returns(new[]
+                {
+                    new TorrentContent { Index = 0, Priority = TorrentContentPriority.Normal }
+                });
+
+            _fixture.RuleEvaluator
+                .EvaluateStallRulesAsync(Arg.Any<QBitItemWrapper>())
+                .Returns((true, DeleteReason.Stalled, false, true));
+
+            // Act
+            var result = await sut.ShouldRemoveFromArrQueueAsync(hash, Array.Empty<string>());
+
+            // Assert
+            result.ShouldRemove.ShouldBeTrue();
+            result.DeleteReason.ShouldBe(DeleteReason.Stalled);
+            result.DeleteFromClient.ShouldBeFalse();
+            result.ChangeCategory.ShouldBeTrue();
+        }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@code/backend/Cleanuparr.Infrastructure.Tests/Features/DownloadClient/QBitServiceTests.cs`
around lines 836 - 950, Add a test named
StalledDownload_RuleWithChangeCategory_PropagatesChangeCategoryFlag in the
ShouldRemoveFromArrQueueAsync_StalledDownloadScenarios class that mirrors the
SlowDownload_RuleWithChangeCategory_PropagatesChangeCategoryFlag test but sets
TorrentState = TorrentState.StalledDownload and configures
RuleEvaluator.EvaluateStallRulesAsync to return (true, DeleteReason.Stalled,
true, true); call ShouldRemoveFromArrQueueAsync(hash, ...) on the SUT and assert
result.ShouldRemove is true, result.DeleteReason == DeleteReason.Stalled,
result.DeleteFromClient is true and result.ChangeCategory is true so the
stalled-rule branch (EvaluateStallRulesAsync) propagates the ChangeCategory
flag.