fix(264): record library-level LastScan after a successful local scan #408

Merged
timothy merged 3 commits from fix/264-library-lastscan into main 2026-07-17 15:00:58 +02:00
3 Commits
Author SHA1 Message Date
timothy 71708633f5 test(264): pin the path-scan-error log; correct testing.md accuracy
Build ErsatzTV Image / CI image pin matches docker/ci (pull_request) Successful in 7s
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 8s
Build ErsatzTV Image / decisions.md append-only (pull_request) Successful in 6s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 8s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 38s
Build ErsatzTV Image / Functional E2E (curl contracts) (pull_request) Successful in 10m49s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 15m6s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 15m9s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Re-review of the previous fix commit (986ccfaf, verdict MERGEABLE)
raised one fair hit and one claim that did not survive checking.

Fair hit — that commit's message asserted "LogError is an extension
method that NSubstitute can't cleanly verify" as the reason for adding
no test. That is FALSE, and this repo disproves it:
ErsatzTV.Tests' ReleaseMemoryHandlerTests.ShouldHaveLogged asserts on
ILogger via ReceivedCalls(), which works precisely because the LogError
extension dispatches to the substituted ILogger.Log. Reusing that idiom
here costs 4 lines, so pin the log the previous commit added. Verified
non-vacuous: asserting a message the handler never logs fails the test.

Not applied — the same review called the row's "1471 tests" misleading
on the grounds that TranscodingTests is [Explicit] and contributes most
of that count. TranscodingTests is indeed [Explicit], but filtering it
out yields exactly 1471, so 1471 is already the runnable count and the
row was accurate. Kept the number; documented TranscodingTests as
[Explicit]/opt-in instead, since it was a genuine omission from a table
that claims to be authoritative.

Also rewrap the verification-gate paragraph the previous commit left
over-long, and name the two always-run projects instead of "both".

Deferred (filed separately): a canceled local scan now logs at ERROR
per path via this log. It mirrors the remote handlers exactly, so
diverging here would be the inconsistency, not the fix.
2026-07-17 14:35:09 +02:00
timothy 986ccfaf6c fix(264): log local path scan errors (review finding) + map Scanner.Tests
Build ErsatzTV Image / CI image pin matches docker/ci (pull_request) Successful in 7s
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 6s
Build ErsatzTV Image / decisions.md append-only (pull_request) Successful in 8s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 8s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 47s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 5m28s
Build ErsatzTV Image / Functional E2E (curl contracts) (pull_request) Successful in 3m29s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 4m3s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Adversarial review (MERGEABLE, no blockers) raised two items worth
folding in rather than deferring:

Medium — ScanLocalLibraryHandler silently swallowed path scan errors,
while the three remote scanners it mirrors all log result.LeftToSeq().
That mattered less when a failed path only skipped the path-level
LastScan, but the previous commit makes a failed path suppress the
library-level scan time too — so the user would see exactly the #264
symptom ("Never scanned") with nothing in the log explaining why. That
is a diagnosis dead-end of the same class as the bug being fixed, so
log it here rather than file a follow-up. No test: the sibling
Synchronize*LibraryByIdHandlerTests don't assert on logging either, and
LogError is an extension method that NSubstitute can't cleanly verify.

Low — docs/testing.md bills itself as the authoritative map of what each
test project covers but omitted ErsatzTV.Scanner.Tests entirely (1471
pre-existing tests). This PR adds a file to that project, so add the row
and include it in the per-PR verification gate.
2026-07-17 14:27:50 +02:00
timothy 5c8dd64acf fix(264): record library-level LastScan after a successful local scan
Build ErsatzTV Image / CI image pin matches docker/ci (pull_request) Successful in 6s
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 7s
Build ErsatzTV Image / decisions.md append-only (pull_request) Successful in 8s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 8s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 40s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 6m10s
Build ErsatzTV Image / Functional E2E (curl contracts) (pull_request) Successful in 5m36s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 11m11s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Local libraries permanently showed "Never scanned" in the SPA libraries
hub regardless of successful scans, even as item counts updated.

Root cause: ScanLocalLibraryHandler wrote only the path-level
LibraryPath.LastScan (which gates the per-path refresh interval) and
never the library-level Library.LastScan. The read API
(GetAllMediaSourcesForApiHandler) populates the hub's scan-time badge
from Library.LastScan, so that value stayed null forever. The three
remote scanners (Jellyfin/Emby/Plex) already set the library-level
value; only the local scanner did not. Both sides predate #202 — the
SPA hub merely made the missing value visible.

Mirror the remote scanners' semantics: record the library-level scan
time only when the scan actually ran and every path that ran succeeded,
so a skipped (unforced, interval not elapsed) or partially-failed scan
does not claim a successful scan time.

Also de-BOM the touched handler per the fix-as-you-touch charset gate
(#311).

fixes #264
2026-07-17 14:18:14 +02:00