fix(dirscan): queue webhook scans and tighten age filtering#1603
Merged
fix(dirscan): queue webhook scans and tighten age filtering#1603
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughRefactors dirscan to select work per-root, adds webhook queuing/merge/update logic and queued-run cancellation, extends content detection to include audio, updates UI/docs wording for the age-filter and eligible file counts, and introduces work-selection primitives used throughout the scan pipeline. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Service as DirScan Service
participant Store as DirScanStore
participant DB as Database
Client->>Service: StartWebhookScan(directoryPath, scanRoot)
activate Service
Service->>Service: Acquire webhook mutex
Service->>Store: GetActiveRun(directoryID)
activate Store
Store->>DB: Query runs ordered by prioritized status + timestamps (LIMIT 1)
DB-->>Store: activeRun or nil
Store-->>Service: activeRun
deactivate Store
alt Active run exists
Service->>Service: mergedRoot = mergeWebhookScanRoots(dir, activeRun.scan_root, scanRoot)
Service->>Store: UpdateRunScanRoot(activeRun.id, mergedRoot)
else No active run
Service->>Store: GetQueuedRun(directoryID)
activate Store
Store->>DB: Query newest queued run
DB-->>Store: queuedRun or nil
Store-->>Service: queuedRun
deactivate Store
alt Queued run exists
Service->>Service: mergedRoot = mergeWebhookScanRoots(dir, queuedRun.scan_root, scanRoot)
Service->>Store: UpdateRunScanRoot(queuedRun.id, mergedRoot)
else No queued run
Service->>Store: CreateRun(directoryID, scanRoot)
end
end
Service->>Service: Release webhook mutex
deactivate Service
sequenceDiagram
participant Scanner as Scan Phase
participant WorkSel as Work Selection
participant Searcher as Search/Inject Phase
participant Finalizer as Finalize Run
Scanner->>Scanner: Build fileID index & scan directory tree
Scanner-->>WorkSel: ScanResult
activate WorkSel
WorkSel->>WorkSel: Compute cutoff (maxAgeDays + now)
loop For each root in ScanResult
WorkSel->>WorkSel: Build work items for root
loop For each work item
WorkSel->>WorkSel: workItemIsStale(item, cutoff) (check video/audio mtimes)
alt stale
WorkSel->>WorkSel: mark skipped
else fresh
WorkSel->>WorkSel: mark eligible
end
end
end
WorkSel-->>Searcher: scanWorkSelection (eligible items + counts)
deactivate WorkSel
activate Searcher
Searcher->>Searcher: Process eligible work items (search & inject)
Searcher-->>Finalizer: matchesFound, torrentsAdded, filesFound, filesSkipped
deactivate Searcher
activate Finalizer
Finalizer->>Finalizer: finalizeRun(filesFound, filesSkipped, matchesFound, torrentsAdded)
Finalizer->>DB: Update run record
deactivate Finalizer
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/components/cross-seed/DirScanTab.tsx (1)
462-472:⚠️ Potential issue | 🟠 MajorDon’t hide the new counts when everything was skipped.
hasStatsignoresrun.filesSkipped, so a run with0 eligible,N skipped,0 matches, and0 addedshows no file stats in the directory card. That hides the exact age-filter outcome this change is trying to surface.💡 Suggested patch
- const hasStats = run.filesFound > 0 || run.matchesFound > 0 || run.torrentsAdded > 0 + const hasStats = + run.filesFound > 0 || + run.filesSkipped > 0 || + run.matchesFound > 0 || + run.torrentsAdded > 0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/cross-seed/DirScanTab.tsx` around lines 462 - 472, hasStats currently only checks run.filesFound, run.matchesFound, and run.torrentsAdded so runs with only run.filesSkipped > 0 are treated as having no stats; update the condition in DirScanTab (the hasStats declaration) to include run.filesSkipped and also update the rendered stats line (the JSX block that uses RunFilesBadge and the trailing text) to surface the skipped count (e.g., add ", {run.filesSkipped} skipped" or ensure RunFilesBadge reflects skipped) so directory cards show the age-filter skipped count.
🧹 Nitpick comments (2)
internal/services/dirscan/service.go (1)
771-779: Nil-safe sorting, but consider defensive validation.The sort pushes nil roots to the end, and
processRootSearcheesafely handles nil searchees. However, having nil roots in the selection likely indicates an upstream bug.Consider adding a debug log or assertion if a nil root is encountered during iteration at line 800 to help identify such issues during development.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/services/dirscan/service.go` around lines 771 - 779, The sort currently pushes nil entries in workSelection.roots to the end; add a defensive check when iterating over workSelection.roots (the loop that calls processRootSearchee) to detect nil root entries and either log a debug/error via the existing logger or panic/assert in debug builds so upstream creation bugs are caught early; specifically, inspect each element’s root before calling processRootSearchee and emit a descriptive debug log mentioning the index and the surrounding context (or trigger an assertion) when root == nil to aid developers tracing the source of nil roots.internal/services/dirscan/age_filter_test.go (1)
70-86: Consider adding a comment explaining the expected item count.Line 84 asserts
Len(t, items, 3)but doesn't explain why 3 items are expected. Adding a brief comment like// root + 2 episodeswould improve test readability for future maintainers.📝 Suggested documentation
items := buildSearcheeWorkItems(root, NewParser(nil)) + // Expect 3 items: the root searchee + 2 individual episode work items require.Len(t, items, 3)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/services/dirscan/age_filter_test.go` around lines 70 - 86, Add a brief inline comment in TestWorkItemIsStale_KeepsFreshSeasonPack explaining why items length equals 3 (e.g., "// root + 2 episodes") so future readers understand the expectation produced by buildSearcheeWorkItems when called with NewParser(nil); place the comment immediately above or beside the require.Len(t, items, 3) assertion to clarify the source of the three work items.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@documentation/docs/features/cross-seed/dir-scan.md`:
- Around line 174-175: Update the documentation's "response-code table" and any
references to concurrent webhook behavior that still claim a 409 conflict;
change the contract text to reflect that follow-up webhook requests are now
queued and merged rather than rejected, replacing mentions of "409" with the new
behavior (queued/merged) and update any example responses, operator guidance,
and the paragraphs covering the webhook handling (the section previously
claiming 409s) so they describe the queuing/merging semantics and any new
headers or retry guidance operators should use.
In `@internal/models/dirscan.go`:
- Around line 793-805: The ORDER BY in GetActiveRun() and GetQueuedRun() is
non-deterministic when started_at ties; make selection deterministic by adding a
stable tiebreaker (e.g., append ", id DESC" or the primary key column such as
run_id DESC) after started_at DESC in the ORDER BY clause so scanRun() will
always receive a consistent row for concurrent inserts.
In `@internal/services/dirscan/content_detection.go`:
- Around line 16-19: Add the .aob extension to the audioExtensions map so
AUDIO_TS payloads are recognized as audio by filterContentFiles and therefore
subject to the age cutoff enforced by workItemIsStale; update the map literal
named audioExtensions (used by filterContentFiles) to include ".aob": {} so
directories containing .aob files no longer bypass stale-item filtering.
In `@internal/services/dirscan/webhook_queue_test.go`:
- Around line 18-20: The test compares a raw dir string to the cleaned path
returned by mergeWebhookScanRoots, causing OS-dependent failures; update the
assertion to normalize the expected value by running the test's dir through
filepath.Clean (and ensure the test imports path/filepath) so you compare
filepath.Clean(dir) to mergeWebhookScanRoots(dir, "", dir).
---
Outside diff comments:
In `@web/src/components/cross-seed/DirScanTab.tsx`:
- Around line 462-472: hasStats currently only checks run.filesFound,
run.matchesFound, and run.torrentsAdded so runs with only run.filesSkipped > 0
are treated as having no stats; update the condition in DirScanTab (the hasStats
declaration) to include run.filesSkipped and also update the rendered stats line
(the JSX block that uses RunFilesBadge and the trailing text) to surface the
skipped count (e.g., add ", {run.filesSkipped} skipped" or ensure RunFilesBadge
reflects skipped) so directory cards show the age-filter skipped count.
---
Nitpick comments:
In `@internal/services/dirscan/age_filter_test.go`:
- Around line 70-86: Add a brief inline comment in
TestWorkItemIsStale_KeepsFreshSeasonPack explaining why items length equals 3
(e.g., "// root + 2 episodes") so future readers understand the expectation
produced by buildSearcheeWorkItems when called with NewParser(nil); place the
comment immediately above or beside the require.Len(t, items, 3) assertion to
clarify the source of the three work items.
In `@internal/services/dirscan/service.go`:
- Around line 771-779: The sort currently pushes nil entries in
workSelection.roots to the end; add a defensive check when iterating over
workSelection.roots (the loop that calls processRootSearchee) to detect nil root
entries and either log a debug/error via the existing logger or panic/assert in
debug builds so upstream creation bugs are caught early; specifically, inspect
each element’s root before calling processRootSearchee and emit a descriptive
debug log mentioning the index and the surrounding context (or trigger an
assertion) when root == nil to aid developers tracing the source of nil roots.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4eef5c74-9fd7-43bf-8b55-d0367f6ad665
📒 Files selected for processing (10)
documentation/docs/features/cross-seed/dir-scan.mdinternal/models/dirscan.gointernal/models/dirscan_run_status_test.gointernal/services/dirscan/age_filter_test.gointernal/services/dirscan/content_detection.gointernal/services/dirscan/service.gointernal/services/dirscan/webhook_queue.gointernal/services/dirscan/webhook_queue_test.gointernal/services/dirscan/work_selection.goweb/src/components/cross-seed/DirScanTab.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
internal/models/dirscan.go (1)
902-913: Consider returning affected row count for observability.The caller may want to know how many queued runs were canceled (for logging or metrics). Currently, the count is discarded.
💡 Optional: return affected count
-func (s *DirScanStore) CancelQueuedRuns(ctx context.Context, directoryID int) error { - _, err := s.db.ExecContext(ctx, ` +func (s *DirScanStore) CancelQueuedRuns(ctx context.Context, directoryID int) (int64, error) { + res, err := s.db.ExecContext(ctx, ` UPDATE dir_scan_runs SET status = ?, completed_at = CURRENT_TIMESTAMP WHERE directory_id = ? AND status = 'queued' `, DirScanRunStatusCanceled, directoryID) if err != nil { - return fmt.Errorf("cancel queued runs: %w", err) + return 0, fmt.Errorf("cancel queued runs: %w", err) } - return nil + rows, err := res.RowsAffected() + if err != nil { + return 0, fmt.Errorf("rows affected: %w", err) + } + return rows, nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/models/dirscan.go` around lines 902 - 913, The CancelQueuedRuns function currently discards the ExecContext result; update DirScanStore.CancelQueuedRuns to return the number of affected rows (e.g., change signature to return (int64, error) or (int, error)), capture the sql.Result from s.db.ExecContext, call RowsAffected() on it, and return that count along with any error; keep the same SQL and the DirScanRunStatusCanceled constant usage and propagate errors as before (wrap fmt.Errorf("cancel queued runs: %w", err) when ExecContext or RowsAffected() fails).web/src/components/cross-seed/DirScanTab.tsx (1)
469-476: Minor: "skipped" count appears twice when tooltip is visible.
RunFilesBadgealready shows"X discovered, Y skipped"in its tooltip. Line 473 then appends, Y skippedagain to the inline text. This creates redundancy when hovering:
5 eligible(hover) → "10 discovered, 5 skipped"
plus inline:, 5 skipped, 2 matches, 1 added)Consider removing the inline skipped text since the tooltip covers it, or keep both if the intent is to show skipped without requiring hover.
💡 Optional: remove duplicate skipped display
<span className="inline-flex items-center gap-1"> <span className="text-muted-foreground">(</span> <RunFilesBadge run={run} /> <span className="text-muted-foreground"> - {run.filesSkipped > 0 ? `, ${run.filesSkipped} skipped` : ""} - , {run.matchesFound} matches, {run.torrentsAdded} added) + , {run.matchesFound} matches, {run.torrentsAdded} added) </span> </span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/cross-seed/DirScanTab.tsx` around lines 469 - 476, The inline text is duplicating the skipped count already shown by RunFilesBadge's tooltip; update DirScanTab.tsx so the inline span that currently uses run.filesSkipped no longer appends ", {run.filesSkipped} skipped" — keep RunFilesBadge as-is and change the surrounding text (the span containing ", {run.filesSkipped} skipped, {run.matchesFound} matches, {run.torrentsAdded} added") to only include the matches and torrentsAdded (e.g., ", {run.matchesFound} matches, {run.torrentsAdded} added") so skipped is shown only in the RunFilesBadge tooltip.internal/services/dirscan/age_filter_test.go (1)
15-92: Consider converting the threeTestSelectEligibleRootWork_*cases into a table-driven test.The setup/act flow is duplicated and only fixtures/expectations vary. A table will reduce drift and make adding new file-type/age scenarios easier.
♻️ Refactor sketch
+func TestSelectEligibleRootWork(t *testing.T) { + now := time.Date(2026, time.March, 16, 13, 0, 0, 0, time.UTC) + tests := []struct { + name string + scanResult *ScanResult + wantDiscovered int + wantEligible int + wantSkipped int + wantRoots int + }{ + // existing TV / subtitle / AOB scenarios... + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + selection := selectEligibleRootWork(tc.scanResult, nil, NewParser(nil), 3, now) + require.Equal(t, tc.wantDiscovered, selection.discoveredFiles) + require.Equal(t, tc.wantEligible, selection.eligibleFiles) + require.Equal(t, tc.wantSkipped, selection.skippedFiles) + require.Len(t, selection.roots, tc.wantRoots) + }) + } +}As per coding guidelines: "Prefer table-driven test cases in Go backend tests".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/services/dirscan/age_filter_test.go` around lines 15 - 92, Combine the three TestSelectEligibleRootWork_* tests into a single table-driven test that iterates over cases describing the ScanResult fixture, the now/time values, and expected selection fields; for each case construct the ScanResult (using Searchee and ScannedFile entries), call selectEligibleRootWork(..., NewParser(nil), 3, now) and assert selection.discoveredFiles, selection.eligibleFiles, selection.skippedFiles, and selection.roots (and items contents where applicable). Use the existing test names as case keys (e.g., "TVKeepsOnlyFreshEpisodeItems", "IgnoresFreshSubtitleBumps", "TreatsAOBAsAudioContent") to keep intent clear, and remove duplicated setup/act code by reusing the same loop body for assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/services/dirscan/age_filter_test.go`:
- Around line 107-110: The test is brittle because it asserts freshness using
items[0]; instead locate the root work item explicitly (e.g., by matching a
Path, Name, or an IsRoot/Root flag on the work item returned by
buildSearcheeWorkItems) and run the stale check against that found item using
workItemIsStale; update the assertion to require the found item exists
(require.NotNil/require.True for the predicate that finds it) and then
require.False(t, workItemIsStale(rootItem, now.AddDate(0,0,-3))) to ensure the
root-specific behavior is tested regardless of slice ordering.
---
Nitpick comments:
In `@internal/models/dirscan.go`:
- Around line 902-913: The CancelQueuedRuns function currently discards the
ExecContext result; update DirScanStore.CancelQueuedRuns to return the number of
affected rows (e.g., change signature to return (int64, error) or (int, error)),
capture the sql.Result from s.db.ExecContext, call RowsAffected() on it, and
return that count along with any error; keep the same SQL and the
DirScanRunStatusCanceled constant usage and propagate errors as before (wrap
fmt.Errorf("cancel queued runs: %w", err) when ExecContext or RowsAffected()
fails).
In `@internal/services/dirscan/age_filter_test.go`:
- Around line 15-92: Combine the three TestSelectEligibleRootWork_* tests into a
single table-driven test that iterates over cases describing the ScanResult
fixture, the now/time values, and expected selection fields; for each case
construct the ScanResult (using Searchee and ScannedFile entries), call
selectEligibleRootWork(..., NewParser(nil), 3, now) and assert
selection.discoveredFiles, selection.eligibleFiles, selection.skippedFiles, and
selection.roots (and items contents where applicable). Use the existing test
names as case keys (e.g., "TVKeepsOnlyFreshEpisodeItems",
"IgnoresFreshSubtitleBumps", "TreatsAOBAsAudioContent") to keep intent clear,
and remove duplicated setup/act code by reusing the same loop body for
assertions.
In `@web/src/components/cross-seed/DirScanTab.tsx`:
- Around line 469-476: The inline text is duplicating the skipped count already
shown by RunFilesBadge's tooltip; update DirScanTab.tsx so the inline span that
currently uses run.filesSkipped no longer appends ", {run.filesSkipped} skipped"
— keep RunFilesBadge as-is and change the surrounding text (the span containing
", {run.filesSkipped} skipped, {run.matchesFound} matches, {run.torrentsAdded}
added") to only include the matches and torrentsAdded (e.g., ",
{run.matchesFound} matches, {run.torrentsAdded} added") so skipped is shown only
in the RunFilesBadge tooltip.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: fafc636f-8faf-47ba-aec8-41d00e36dcb7
📒 Files selected for processing (6)
documentation/docs/features/cross-seed/dir-scan.mdinternal/models/dirscan.gointernal/models/dirscan_run_status_test.gointernal/services/dirscan/age_filter_test.gointernal/services/dirscan/content_detection.goweb/src/components/cross-seed/DirScanTab.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/models/dirscan_run_status_test.go
- internal/services/dirscan/content_detection.go
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/services/dirscan/age_filter_test.go (1)
15-92: Consider consolidating these test scenarios into a table-driven test.The three test functions (
TVKeepsOnlyFreshEpisodeItems,IgnoresFreshSubtitleBumps,TreatsAOBAsAudioContent) share identical setup and assertion patterns—only the input data and expected counts differ. This repetition is a good candidate for table-driven refactoring, which would make future scenario additions simpler and clearer.Per coding guidelines:
Prefer table-driven test cases in Go backend tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/services/dirscan/age_filter_test.go` around lines 15 - 92, Consolidate the three tests TestSelectEligibleRootWork_TVKeepsOnlyFreshEpisodeItems, TestSelectEligibleRootWork_IgnoresFreshSubtitleBumps, and TestSelectEligibleRootWork_TreatsAOBAsAudioContent into a single table-driven test: create a slice of cases each with a name, input ScanResult (Searchee and ScannedFile list with appropriate ModTime/Size), expected discoveredFiles/eligibleFiles/skippedFiles/roots length and any specific item name expectations; loop over cases using t.Run(case.name, func(t *testing.T) { selection := selectEligibleRootWork(case.scanResult, nil, NewParser(nil), 3, now); assert the expected fields (selection.discoveredFiles, selection.eligibleFiles, selection.skippedFiles, len(selection.roots), len(selection.roots[0].items) and item searchee.Name when applicable) }), preserving the same now/old/fresh time setup per case and keeping use of ScanResult, Searchee, ScannedFile, selectEligibleRootWork and NewParser to locate the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@internal/services/dirscan/age_filter_test.go`:
- Around line 15-92: Consolidate the three tests
TestSelectEligibleRootWork_TVKeepsOnlyFreshEpisodeItems,
TestSelectEligibleRootWork_IgnoresFreshSubtitleBumps, and
TestSelectEligibleRootWork_TreatsAOBAsAudioContent into a single table-driven
test: create a slice of cases each with a name, input ScanResult (Searchee and
ScannedFile list with appropriate ModTime/Size), expected
discoveredFiles/eligibleFiles/skippedFiles/roots length and any specific item
name expectations; loop over cases using t.Run(case.name, func(t *testing.T) {
selection := selectEligibleRootWork(case.scanResult, nil, NewParser(nil), 3,
now); assert the expected fields (selection.discoveredFiles,
selection.eligibleFiles, selection.skippedFiles, len(selection.roots),
len(selection.roots[0].items) and item searchee.Name when applicable) }),
preserving the same now/old/fresh time setup per case and keeping use of
ScanResult, Searchee, ScannedFile, selectEligibleRootWork and NewParser to
locate the code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 33abfeb1-8ddd-479e-a060-8df66a5d73a7
📒 Files selected for processing (1)
internal/services/dirscan/age_filter_test.go
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
internal/services/dirscan/progress.go (1)
191-197: Consider extracting match-type string literals to constants.The
"file_id"and"path"strings are used in multiple places. Constants would provide compile-time typo detection and a single source of truth.♻️ Optional: Extract to package-level constants
+const ( + matchedByFileID = "file_id" + matchedByPath = "path" +) + func lookupTrackedFile(scanned *ScannedFile, idx *trackedFilesIndex) (*models.DirScanFile, string) { if scanned == nil || idx == nil { return nil, "" } if !scanned.FileID.IsZero() { if existing := idx.byFileID[string(scanned.FileID.Bytes())]; existing != nil { - return existing, "file_id" + return existing, matchedByFileID } } if existing := idx.byPath[scanned.Path]; existing != nil { - return existing, "path" + return existing, matchedByPath } return nil, "" }Then update line 248 similarly:
- if alreadySeeding || matchedBy == "file_id" { + if alreadySeeding || matchedBy == matchedByFileID {Also applies to: 248-248
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/services/dirscan/progress.go` around lines 191 - 197, Extract the match-type string literals into package-level constants (e.g. const matchTypeFileID = "file_id" and const matchTypePath = "path") and replace direct uses in the progress logic: change the returns in the checks using idx.byFileID (and scanned.FileID.Bytes()) and idx.byPath (and scanned.Path) to return the corresponding constants; also update the other occurrence referenced near the later return (the one at the similar check around line 248) to use the same constants so all match-type usages share a single source of truth.internal/services/dirscan/work_selection.go (1)
254-289: Document the intentional asymmetry in staleness logic.The function applies different staleness criteria depending on item type:
- TV groups with >1 content file (lines 264-274): Stale if any file is old — a strict check that marks season packs with even one stale episode.
- Non-TV or single-file items (lines 276-288): Stale only if the newest file is old — a lenient check.
This asymmetry appears intentional per the PR objective (per-item granularity for TV), but a brief comment would help future maintainers understand the design choice.
📝 Suggested comment
func workItemIsStale(item searcheeWorkItem, cutoff time.Time) bool { if item.searchee == nil || cutoff.IsZero() { return false } contentFiles := filterContentFiles(item.searchee.Files) if len(contentFiles) == 0 { return false } + // TV groups with multiple content files are marked stale if ANY file is + // older than the cutoff (strict). Non-TV items are stale only if the + // NEWEST content file is older (lenient). if item.tvGroup != nil && len(contentFiles) > 1 {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/services/dirscan/work_selection.go` around lines 254 - 289, Add a short comment inside workItemIsStale explaining the intentional asymmetry: for TV groups with more than one content file (checking item.tvGroup and len(contentFiles) > 1) we mark the work item stale if any file is older than cutoff (per-episode staleness), whereas for non-TV or single-file items we compute the newest file ModTime and only mark stale if that newest time is before cutoff; place this comment near the tvGroup branch (around the loop over contentFiles) and mention the rationale (per-item granularity for TV group packs) so future maintainers understand the design choice.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@internal/services/dirscan/progress.go`:
- Around line 191-197: Extract the match-type string literals into package-level
constants (e.g. const matchTypeFileID = "file_id" and const matchTypePath =
"path") and replace direct uses in the progress logic: change the returns in the
checks using idx.byFileID (and scanned.FileID.Bytes()) and idx.byPath (and
scanned.Path) to return the corresponding constants; also update the other
occurrence referenced near the later return (the one at the similar check around
line 248) to use the same constants so all match-type usages share a single
source of truth.
In `@internal/services/dirscan/work_selection.go`:
- Around line 254-289: Add a short comment inside workItemIsStale explaining the
intentional asymmetry: for TV groups with more than one content file (checking
item.tvGroup and len(contentFiles) > 1) we mark the work item stale if any file
is older than cutoff (per-episode staleness), whereas for non-TV or single-file
items we compute the newest file ModTime and only mark stale if that newest time
is before cutoff; place this comment near the tvGroup branch (around the loop
over contentFiles) and mention the rationale (per-item granularity for TV group
packs) so future maintainers understand the design choice.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d20b3960-dc90-4394-8097-fd98a1a43e9c
📒 Files selected for processing (4)
internal/services/dirscan/age_filter_test.gointernal/services/dirscan/progress.gointernal/services/dirscan/service.gointernal/services/dirscan/work_selection.go
✅ Files skipped from review due to trivial changes (1)
- internal/services/dirscan/service.go
alexlebens
pushed a commit
to alexlebens/infrastructure
that referenced
this pull request
Mar 20, 2026
This PR contains the following updates: | Package | Update | Change | |---|---|---| | [ghcr.io/autobrr/qui](https://github.com/autobrr/qui) | minor | `v1.14.1` → `v1.15.0` | --- >⚠️ **Warning** > > Some dependencies could not be looked up. Check the [Dependency Dashboard](issues/2) for more information. --- ### Release Notes <details> <summary>autobrr/qui (ghcr.io/autobrr/qui)</summary> ### [`v1.15.0`](https://github.com/autobrr/qui/releases/tag/v1.15.0) [Compare Source](autobrr/qui@v1.14.1...v1.15.0) #### Changelog ##### Breaking change CORS is disabled by default; enable by setting QUI\_\_CORS\_ALLOWED\_ORIGINS with explicit origins (http(s)://host\[:port]). See <https://getqui.com/docs/advanced/sso-proxy-cors> ##### New Features - [`93786a2`](autobrr/qui@93786a2): feat(automations): add configurable processing priority/sorting ([#​1235](autobrr/qui#1235)) ([@​Oscariremma](https://github.com/Oscariremma)) - [`45eaf1f`](autobrr/qui@45eaf1f): feat(database): add postgres and sqlite migration CLI ([#​1530](autobrr/qui#1530)) ([@​s0up4200](https://github.com/s0up4200)) - [`430f5d1`](autobrr/qui@430f5d1): feat(torrents): mediaInfo dialog ([#​1537](autobrr/qui#1537)) ([@​s0up4200](https://github.com/s0up4200)) - [`8eb8903`](autobrr/qui@8eb8903): feat(web): Add persistence to unified instance filter in sidebar ([#​1560](autobrr/qui#1560)) ([@​drtaru](https://github.com/drtaru)) - [`7aadde7`](autobrr/qui@7aadde7): feat(web): add path autocomplete to set location dialog ([#​1432](autobrr/qui#1432)) ([@​nitrobass24](https://github.com/nitrobass24)) - [`077f32c`](autobrr/qui@077f32c): feat: add mediainfo api endpoint ([#​1545](autobrr/qui#1545)) ([@​Audionut](https://github.com/Audionut)) - [`99cf695`](autobrr/qui@99cf695): feat: endpoint to trigger directory scans from external tools ([#​1559](autobrr/qui#1559)) ([@​s0up4200](https://github.com/s0up4200)) - [`8956f9b`](autobrr/qui@8956f9b): feat: unify bulk tag editor ([#​1571](autobrr/qui#1571)) ([@​s0up4200](https://github.com/s0up4200)) ##### Bug Fixes - [`552d617`](autobrr/qui@552d617): fix(api): align add torrent OpenAPI field ([#​1617](autobrr/qui#1617)) ([@​s0up4200](https://github.com/s0up4200)) - [`424f7a0`](autobrr/qui@424f7a0): fix(api): restrict CORS to explicit allowlist ([#​1551](autobrr/qui#1551)) ([@​s0up4200](https://github.com/s0up4200)) - [`38991d8`](autobrr/qui@38991d8): fix(auth): allow loopback health probes ([#​1621](autobrr/qui#1621)) ([@​s0up4200](https://github.com/s0up4200)) - [`4ae88c9`](autobrr/qui@4ae88c9): fix(automations): align include-cross-seeds category apply ([#​1517](autobrr/qui#1517)) ([@​s0up4200](https://github.com/s0up4200)) - [`6a127a8`](autobrr/qui@6a127a8): fix(automations): scope skipWithin to only deleted action ([#​1538](autobrr/qui#1538)) ([@​jabloink](https://github.com/jabloink)) - [`c776189`](autobrr/qui@c776189): fix(crossseed): avoid completion timeout misses on non-Gazelle torrents ([#​1536](autobrr/qui#1536)) ([@​s0up4200](https://github.com/s0up4200)) - [`b1338a7`](autobrr/qui@b1338a7): fix(crossseed): handle missing webhook collection tags ([#​1610](autobrr/qui#1610)) ([@​s0up4200](https://github.com/s0up4200)) - [`eacbb68`](autobrr/qui@eacbb68): fix(crossseed): normalize hdr aliases ([#​1572](autobrr/qui#1572)) ([@​s0up4200](https://github.com/s0up4200)) - [`537ad46`](autobrr/qui@537ad46): fix(crossseed): queue completion searches and retry rate-limit waits ([#​1523](autobrr/qui#1523)) ([@​s0up4200](https://github.com/s0up4200)) - [`4fc550f`](autobrr/qui@4fc550f): fix(crossseed): use autobrr indexer ids for webhooks ([#​1614](autobrr/qui#1614)) ([@​s0up4200](https://github.com/s0up4200)) - [`08029ad`](autobrr/qui@08029ad): fix(crossseed): valid partial matches being rejected ([#​1291](autobrr/qui#1291)) ([@​rybertm](https://github.com/rybertm)) - [`77eedd9`](autobrr/qui@77eedd9): fix(database): avoid postgres temp-table statement caching ([#​1581](autobrr/qui#1581)) ([@​s0up4200](https://github.com/s0up4200)) - [`25daa17`](autobrr/qui@25daa17): fix(dirscan): honor canceled queued webhook runs ([#​1612](autobrr/qui#1612)) ([@​s0up4200](https://github.com/s0up4200)) - [`56995f1`](autobrr/qui@56995f1): fix(dirscan): queue webhook scans and tighten age filtering ([#​1603](autobrr/qui#1603)) ([@​s0up4200](https://github.com/s0up4200)) - [`444d07b`](autobrr/qui@444d07b): fix(dirscan): select concrete hardlink base dir ([#​1606](autobrr/qui#1606)) ([@​s0up4200](https://github.com/s0up4200)) - [`c35bea0`](autobrr/qui@c35bea0): fix(instances): improve settings dialog scrolling ([#​1569](autobrr/qui#1569)) ([@​nuxencs](https://github.com/nuxencs)) - [`dc501a0`](autobrr/qui@dc501a0): fix(proxy): reauth qbit passthrough requests ([#​1582](autobrr/qui#1582)) ([@​s0up4200](https://github.com/s0up4200)) - [`7950d1d`](autobrr/qui@7950d1d): fix(proxy): search endpoint handling ([#​1524](autobrr/qui#1524)) ([@​Audionut](https://github.com/Audionut)) - [`1076eea`](autobrr/qui@1076eea): fix(qbit): prune empty managed dirs after delete\_with\_files ([#​1604](autobrr/qui#1604)) ([@​s0up4200](https://github.com/s0up4200)) - [`5a3114b`](autobrr/qui@5a3114b): fix(qbittorrent): stop reboot torrent\_completed spam ([#​1515](autobrr/qui#1515)) ([@​s0up4200](https://github.com/s0up4200)) - [`1d02e6c`](autobrr/qui@1d02e6c): fix(settings): contain settings tab scrolling ([#​1567](autobrr/qui#1567)) ([@​nuxencs](https://github.com/nuxencs)) - [`f5d69f3`](autobrr/qui@f5d69f3): fix(settings): smoother gradient ([#​1570](autobrr/qui#1570)) ([@​nuxencs](https://github.com/nuxencs)) - [`1c0c3bc`](autobrr/qui@1c0c3bc): fix(torrents): copy MediaInfo summary without brackets ([#​1540](autobrr/qui#1540)) ([@​s0up4200](https://github.com/s0up4200)) - [`3ec913a`](autobrr/qui@3ec913a): fix(web): auto-append slash on path autocomplete selection ([#​1431](autobrr/qui#1431)) ([@​nitrobass24](https://github.com/nitrobass24)) - [`aa2f3da`](autobrr/qui@aa2f3da): fix(web): check field.state.value type in AddTorrentDialog ([#​1613](autobrr/qui#1613)) ([@​s0up4200](https://github.com/s0up4200)) - [`1abfc5e`](autobrr/qui@1abfc5e): fix(web): handle SSO proxy redirect to /index.html ([#​1600](autobrr/qui#1600)) ([@​s0up4200](https://github.com/s0up4200)) - [`1991f90`](autobrr/qui@1991f90): fix(web): warn before enabling reannounce ([#​1583](autobrr/qui#1583)) ([@​s0up4200](https://github.com/s0up4200)) ##### Other Changes - [`4069492`](autobrr/qui@4069492): chore(deps): bump the github group with 3 updates ([#​1535](autobrr/qui#1535)) ([@​dependabot](https://github.com/dependabot)\[bot]) - [`a02e9e8`](autobrr/qui@a02e9e8): chore(deps): bump the github group with 7 updates ([#​1558](autobrr/qui#1558)) ([@​dependabot](https://github.com/dependabot)\[bot]) - [`8713667`](autobrr/qui@8713667): chore(deps): bump the golang group with 15 updates ([#​1543](autobrr/qui#1543)) ([@​dependabot](https://github.com/dependabot)\[bot]) - [`420607e`](autobrr/qui@420607e): chore(go,ci): adopt go fix, bump to 1.26, and speed up PR checks ([#​1480](autobrr/qui#1480)) ([@​s0up4200](https://github.com/s0up4200)) - [`0d0df45`](autobrr/qui@0d0df45): docs: add password reset section to CLI commands ([#​1598](autobrr/qui#1598)) ([@​s0up4200](https://github.com/s0up4200)) - [`9ef56a2`](autobrr/qui@9ef56a2): refactor(makefile): windows support ([#​1546](autobrr/qui#1546)) ([@​Audionut](https://github.com/Audionut)) - [`7899cc8`](autobrr/qui@7899cc8): refactor(reflinking): add windows ReFS filesystem support ([#​1576](autobrr/qui#1576)) ([@​Audionut](https://github.com/Audionut)) - [`51d34ab`](autobrr/qui@51d34ab): refactor(releases): share hdr normalization helpers ([#​1586](autobrr/qui#1586)) ([@​s0up4200](https://github.com/s0up4200)) - [`c7f4e3d`](autobrr/qui@c7f4e3d): refactor(web): tighten unified scope navigation ([#​1618](autobrr/qui#1618)) ([@​s0up4200](https://github.com/s0up4200)) - [`4b05177`](autobrr/qui@4b05177): test(handlers): cover tag baseline field requests ([@​s0up4200](https://github.com/s0up4200)) **Full Changelog**: <autobrr/qui@v1.14.1...v1.15.0> #### Docker images - `docker pull ghcr.io/autobrr/qui:v1.15.0` - `docker pull ghcr.io/autobrr/qui:latest` #### What to do next? - Join our [Discord server](https://discord.autobrr.com/qui) Thank you for using qui! </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My41OS4yIiwidXBkYXRlZEluVmVyIjoiNDMuNTkuMiIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiaW1hZ2UiXX0=--> Reviewed-on: https://gitea.alexlebens.dev/alexlebens/infrastructure/pulls/4884 Co-authored-by: Renovate Bot <renovate-bot@alexlebens.net> Co-committed-by: Renovate Bot <renovate-bot@alexlebens.net>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Webhook-triggered directory scans no longer drop later Sonarr or Radarr imports while a scan is already running. qui now keeps one follow-up queued run per directory and merges later webhook paths into it so imported items are not silently missed.
Dir scan age filtering now uses video and audio mtimes only and applies at TV work-item granularity instead of keeping an entire show alive because one file under it is newer. The UI now reports eligible items as the primary count and moves raw discovered and skipped totals into details.
Closes #1602
Summary by CodeRabbit
New Features
Documentation
UI/UX Improvements
Tests