fix(dirscan): retain recent runs and clarify restart behavior

[s0up4200]

Dir Scan now keeps only the 10 most recent run records per directory instead of letting run history grow without bounds. New runs prune older rows automatically, and service startup also cleans up legacy history from older installs, so existing databases converge to the same limit without a migration.

I also tightened the user-facing language around stop/start behavior. Dir Scan does not resume from an exact checkpoint; it rechecks the directory, skips finished items based on persisted per-file state, and retries unfinished work. The UI copy and end-user docs now say that plainly so the behavior matches the user’s mental model.

Summary by CodeRabbit

• New Features

  • Directory scan run history now auto-prunes, retaining the last 10 runs per directory.

• Documentation

  • Clarified incremental scan behavior: next run rechecks the directory, skips finished items, and retries unfinished ones.
  • Updated testing/PR guidance to recommend targeted local tests and rely on CI for full test runs.

• UI/UX

  • Improved toast messages, headers and alerts to explain cancellation and progress-reset behavior.

• Bug Fixes

  • Error messages for certain scan injection failures are now more user-friendly.

• Tests

  • Added tests covering per-directory pruning and legacy-history trimming.

[coderabbitai]

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Adds per-directory dir-scan run-history pruning (keep last 10 runs), run-injection validation/marshaling, service startup pruning, tests for pruning behavior and humanized link-plan errors, plus UI and docs text updates describing incremental scan/resume semantics.

Changes

Cohort / File(s)	Summary
Run History & Run-Injection Logic internal/models/dirscan.go	Added ListDirectoryIDs, PruneRunHistory, per-directory pruning helpers/constants, run-injection validation/marshaling, insertRunInjection, and best-effort trim calls after run/injection creation.
Model Tests internal/models/dirscan_run_status_test.go	Added tests verifying per-directory trimming to 10 runs and legacy-row pruning behavior.
Service Startup & Tests internal/services/dirscan/service.go, internal/services/dirscan/cancel_scan_test.go	Added pruneLegacyRunHistory() invoked during Service Start (10s timeout) and test asserting pruning runs on start.
Injection Error Handling & Test internal/services/dirscan/inject.go, internal/services/dirscan/inject_test.go	Humanized link-plan mismatch error helper and updated materializeLinkTree logging; test asserting user-friendly error message on link-plan mismatch.
Docs & Contributor Guidance AGENTS.md, documentation/docs/features/cross-seed/dir-scan.md	Updated testing/CI guidance and dir-scan docs to describe incremental progress, recheck/retry semantics, and that the UI retains the last 10 runs per directory.
UI Text Updates web/src/components/cross-seed/DirScanTab.tsx	Reworded toast, headers, alerts, and settings copy to reflect recheck/retry semantics and last-10-runs retention.
Build Typings web/vite.config.ts	Added /// <reference lib="WebWorker" /> directive for WebWorker typings.
Module File go.mod	Referenced by service change (module metadata touched).

Sequence Diagram(s)

sequenceDiagram
    participant Service as DirScan Service
    participant Store as DirScanStore
    participant DB as Database

    Service->>Service: Start()
    Service->>Store: PruneRunHistory(ctx with 10s timeout)
    Store->>DB: DELETE old runs per directory (ORDER BY started_at DESC, id DESC LIMIT offset)
    DB-->>Store: OK / rows affected
    Store-->>Service: Result / error
    Service-->>Service: Log success or warning, continue Start flow

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• feat(crossseed): add directory scanner for data-based matching #1203 — Related changes to DirScan subsystem; may overlap prior pruning/run-history work.

Suggested labels

backend, database, dirscan, tests, web, bugfix

Poem

🐰 I hopped through runs both old and new,
I trimmed the logs to ten, not few,
Scans will recheck, retry what's stalled,
Old noise gone, neat rows installed.
Hop, prune, and nibble—order's in view! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the main changes: retaining recent run history and clarifying restart behavior documentation, which are the core objectives of the PR.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/dirscan-run-retention

---

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

internal/models/dirscan.go (1)

885-888: Don’t let pruning failures disappear silently.

Both helpers drop DB errors on the floor. If trimming starts failing, the new retention cap quietly stops working and history/injection rows can grow unbounded again with no signal. Please at least surface these failures via logging or an explicit non-fatal error path.

As per coding guidelines, "Prefer explicit error handling over silent failures in Go."

Also applies to: 983-996

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/models/dirscan.go` around lines 885 - 888, The current
trimRunHistoryBestEffort swallows errors from pruneRunHistoryForDirectory;
change the handler to surface failures (log them or return a non-fatal error)
instead of discarding them — e.g., replace the "_ = err" with a descriptive
logging call that includes directoryID and err (use the store's logger field,
e.g. s.logger.Errorf("pruneRunHistoryForDirectory failed for directory %d: %v",
directoryID, err) or fallback to log.Printf) and apply the same change to the
other similar helper mentioned (lines ~983-996) so pruning failures are visible.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@web/src/components/cross-seed/DirScanTab.tsx`:
- Line 339: Add a new Vitest + React Testing Library test file
DirScanTab.test.tsx that renders the DirScanTab component and asserts the
presence of the user-facing strings changed in this PR (including "Scan
canceled. Next run will recheck the directory and continue with unfinished
items." and the other messages at the flagged lines). Mock any external
dependencies used by DirScanTab (e.g., toast from react-hot-toast or similar,
API hooks, and context/providers) and simulate the actions that trigger the
messages (for example invoking the cancel handler that calls the onSuccess toast
in the cancel function referenced in the component). Use screen.getByText /
queryByText assertions to verify each string appears, and ensure the tests run
with Vitest configuration consistent with the repo’s test setup.

---

Nitpick comments:
In `@internal/models/dirscan.go`:
- Around line 885-888: The current trimRunHistoryBestEffort swallows errors from
pruneRunHistoryForDirectory; change the handler to surface failures (log them or
return a non-fatal error) instead of discarding them — e.g., replace the "_ =
err" with a descriptive logging call that includes directoryID and err (use the
store's logger field, e.g. s.logger.Errorf("pruneRunHistoryForDirectory failed
for directory %d: %v", directoryID, err) or fallback to log.Printf) and apply
the same change to the other similar helper mentioned (lines ~983-996) so
pruning failures are visible.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 55ba926d-1300-46d8-9756-59743ef28d45

📥 Commits

Reviewing files that changed from the base of the PR and between 420607e and 74a13fe.

📒 Files selected for processing (7)

• AGENTS.md
• documentation/docs/features/cross-seed/dir-scan.md
• internal/models/dirscan.go
• internal/models/dirscan_run_status_test.go
• internal/services/dirscan/cancel_scan_test.go
• internal/services/dirscan/service.go
• web/src/components/cross-seed/DirScanTab.tsx

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (3)

web/src/components/cross-seed/DirScanTab.test.tsx (1)

229-235: Add an accessible name or test ID to the cancel button.

The test couples to the current icon implementation by selecting via svg.lucide-pause. An icon swap would break the test even if behavior is unchanged. Add an aria-label (e.g., "Cancel scan") or data-testid to the button in DirScanTab.tsx and update the test to use getByRole("button", { name: "Cancel scan" }) or getByTestId, matching the pattern used elsewhere in this test file.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@web/src/components/cross-seed/DirScanTab.test.tsx` around lines 229 - 235,
The test is brittle because it selects the cancel button by an internal SVG
class; update the component DirScanTab (the cancel/pause button element) to
include an accessible identifier (add aria-label="Cancel scan" or
data-testid="cancel-scan-button" on the button) and then update the test in
DirScanTab.test.tsx to find the button via getByRole("button", { name: "Cancel
scan" }) or getByTestId("cancel-scan-button") instead of searching for
svg.lucide-pause; keep the existing fireEvent.click(pauseButton) logic but
replace the pauseButton lookup with the new accessible selector.

web/src/test/setup.ts (1)

9-38: Clear the setup-file mocks after each test.

matchMedia, ResizeObserver, and scrollIntoView are installed once as shared vi.fn() mocks. Without vi.clearAllMocks() in afterEach, their call history leaks between test cases.

Suggested fix

 afterEach(() => {
   cleanup()
+  vi.clearAllMocks()
 })

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@web/src/test/setup.ts` around lines 9 - 38, The test setup installs shared
mocks (window.matchMedia, ResizeObserver via ResizeObserverMock, and
HTMLElement.prototype.scrollIntoView) whose call histories leak between tests;
update the afterEach block (the one that currently calls cleanup()) to also call
vi.clearAllMocks() so all vi.fn() histories are cleared between tests
(optionally vi.restoreAllMocks() if you need to restore implementations),
ensuring matchMedia, ResizeObserverMock, and scrollIntoView have fresh call
state for each test.

internal/models/dirscan.go (1)

720-723: Query only directory IDs for the startup sweep.

This path only needs dir.ID, but ListDirectories pulls and unmarshals full directory rows. A small ID-only query/helper would make the cleanup cheaper and decouple it from unrelated row decode failures.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/models/dirscan.go` around lines 720 - 723, The startup sweep only
needs directory IDs but calls ListDirectories which fetches and unmarshals full
rows; add a lightweight ID-only query (e.g., implement s.ListDirectoryIDs or
ListDirectoriesIDsForPrune that SELECTs only id) and call that from the startup
sweep instead of ListDirectories to avoid unnecessary decoding and coupling to
other fields; update the call site (the code that currently does directories,
err := s.ListDirectories(ctx)) to use the new ID-only helper and adapt the
downstream loop to work with the returned []int64 (or []string as appropriate)
IDs.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@web/vite.config.ts`:
- Line 12: Add the missing TypeScript lib reference by inserting the
triple-slash directive for the WebWorker lib at the top of web/vite.config.ts
(above the existing import of defineConfig) so the file includes /// <reference
lib="WebWorker" />; this ensures web types like Request used by the PWA config
are available despite tsconfig.node.json omitting DOM libs.

---

Nitpick comments:
In `@internal/models/dirscan.go`:
- Around line 720-723: The startup sweep only needs directory IDs but calls
ListDirectories which fetches and unmarshals full rows; add a lightweight
ID-only query (e.g., implement s.ListDirectoryIDs or ListDirectoriesIDsForPrune
that SELECTs only id) and call that from the startup sweep instead of
ListDirectories to avoid unnecessary decoding and coupling to other fields;
update the call site (the code that currently does directories, err :=
s.ListDirectories(ctx)) to use the new ID-only helper and adapt the downstream
loop to work with the returned []int64 (or []string as appropriate) IDs.

In `@web/src/components/cross-seed/DirScanTab.test.tsx`:
- Around line 229-235: The test is brittle because it selects the cancel button
by an internal SVG class; update the component DirScanTab (the cancel/pause
button element) to include an accessible identifier (add aria-label="Cancel
scan" or data-testid="cancel-scan-button" on the button) and then update the
test in DirScanTab.test.tsx to find the button via getByRole("button", { name:
"Cancel scan" }) or getByTestId("cancel-scan-button") instead of searching for
svg.lucide-pause; keep the existing fireEvent.click(pauseButton) logic but
replace the pauseButton lookup with the new accessible selector.

In `@web/src/test/setup.ts`:
- Around line 9-38: The test setup installs shared mocks (window.matchMedia,
ResizeObserver via ResizeObserverMock, and HTMLElement.prototype.scrollIntoView)
whose call histories leak between tests; update the afterEach block (the one
that currently calls cleanup()) to also call vi.clearAllMocks() so all vi.fn()
histories are cleared between tests (optionally vi.restoreAllMocks() if you need
to restore implementations), ensuring matchMedia, ResizeObserverMock, and
scrollIntoView have fresh call state for each test.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: e3f4a687-41ff-4ca9-9081-86d07501fcda

📥 Commits

Reviewing files that changed from the base of the PR and between 74a13fe and 83abff3.

⛔ Files ignored due to path filters (1)

• web/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

📒 Files selected for processing (5)

• internal/models/dirscan.go
• web/package.json
• web/src/components/cross-seed/DirScanTab.test.tsx
• web/src/test/setup.ts
• web/vite.config.ts

[coderabbitai]

🧹 Nitpick comments (2)

internal/services/dirscan/inject_test.go (1)

114-172: Consider making this a small table-driven suite.

The new helper has multiple translation branches plus a generic fallback, but this test locks only one of them. A table-driven test over all outputs would give better coverage with less duplication.

As per coding guidelines, "Prefer table-driven test cases in Go tests".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/services/dirscan/inject_test.go` around lines 114 - 172,
TestInjector_Inject_HumanizesLinkPlanMismatchError only asserts one translation
branch; convert it into a table-driven test that exercises all humanized error
branches plus the generic fallback by iterating test cases with distinct
InjectRequest setups and expected error strings. Refactor the current test into
a TestInjector_Inject_HumanizesLinkPlanMismatchErrors table where each case
provides a name, a modified ParsedTorrent/MatchResult/Searchee (to trigger each
branch), expected error string, then call injector.Inject and assert both
err.Error() and res.ErrorMessage match; keep the same injector creation and
reuse the existing TestInjector_Inject_HumanizesLinkPlanMismatchError logic body
inside the loop, referencing TestInjector_Inject_HumanizesLinkPlanMismatchError
and Injector.Inject to locate the code. Ensure each case has a descriptive name
and use t.Run for subtests.

internal/services/dirscan/inject.go (1)

470-490: Replace string-based error parsing with typed/sentinel errors.

humanizeLinkPlanError depends on exact err.Error() prefixes from pkg/hardlinktree.BuildPlan (lines 106, 120, 165), which currently emits plain errors.New() strings. Any wording change there will silently drop to the generic message. Define typed/sentinel errors in pkg/hardlinktree and use errors.Is/As here to decouple the layers and keep the UI copy stable even if low-level error text changes.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/services/dirscan/inject.go` around lines 470 - 490,
humanizeLinkPlanError currently relies on exact err.Error() prefixes from
pkg/hardlinktree.BuildPlan which is brittle; change the low-level package to
export sentinel errors and/or a typed error (e.g., ErrNoMatchingFile,
ErrNoAvailableMatch, ErrCouldNotMatchFile or a struct type like LinkPlanError
with a File field) and have BuildPlan return those (wrapping context with
fmt.Errorf("...: %w", err) if needed), then update humanizeLinkPlanError to
detect them via errors.Is or errors.As (use the File field from the typed error
to fill the message) instead of parsing err.Error() so the UI messages remain
stable when implementation text changes.

🤖 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/inject_test.go`:
- Around line 114-172: TestInjector_Inject_HumanizesLinkPlanMismatchError only
asserts one translation branch; convert it into a table-driven test that
exercises all humanized error branches plus the generic fallback by iterating
test cases with distinct InjectRequest setups and expected error strings.
Refactor the current test into a
TestInjector_Inject_HumanizesLinkPlanMismatchErrors table where each case
provides a name, a modified ParsedTorrent/MatchResult/Searchee (to trigger each
branch), expected error string, then call injector.Inject and assert both
err.Error() and res.ErrorMessage match; keep the same injector creation and
reuse the existing TestInjector_Inject_HumanizesLinkPlanMismatchError logic body
inside the loop, referencing TestInjector_Inject_HumanizesLinkPlanMismatchError
and Injector.Inject to locate the code. Ensure each case has a descriptive name
and use t.Run for subtests.

In `@internal/services/dirscan/inject.go`:
- Around line 470-490: humanizeLinkPlanError currently relies on exact
err.Error() prefixes from pkg/hardlinktree.BuildPlan which is brittle; change
the low-level package to export sentinel errors and/or a typed error (e.g.,
ErrNoMatchingFile, ErrNoAvailableMatch, ErrCouldNotMatchFile or a struct type
like LinkPlanError with a File field) and have BuildPlan return those (wrapping
context with fmt.Errorf("...: %w", err) if needed), then update
humanizeLinkPlanError to detect them via errors.Is or errors.As (use the File
field from the typed error to fill the message) instead of parsing err.Error()
so the UI messages remain stable when implementation text changes.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: a7d84690-55fb-45c0-a5c2-80ee78d1e5a6

📥 Commits

Reviewing files that changed from the base of the PR and between c0240f1 and 5e1578b.

📒 Files selected for processing (2)

• internal/services/dirscan/inject.go
• internal/services/dirscan/inject_test.go