timothy/ersatztv
1
0
Fork 0
Code Issues 54 Pull Requests 6 Actions Packages Projects Releases 9 Wiki Activity

Local libraries always show "Never scanned" — local scanner sets path-level LastScan but not library.LastScan #264

New Issue
Closed
opened 2026-07-11 17:17:33 +02:00 by timothy · 2 comments
timothy commented 2026-07-11 17:17:33 +02:00
Owner
Copy Link
Copy Source

Severity: Low (display). Pre-existing scanner bug, surfaced by the #202 SPA libraries hub during live E2E.

What's wrong

ErsatzTV.Scanner/Application/MediaSources/Commands/ScanLocalLibraryHandler.cs:145 sets libraryPath.LastScan = DateTime.UtcNow (path-level) after a successful scan but never sets library.LastScan (library-level). The read API GetAllMediaSourcesForApiHandler (ErsatzTV.Application/MediaSources/Queries/GetAllMediaSourcesForApiHandler.cs:39) reads library.LastScan to populate the hub's scan-time / "Never scanned" badge — so every local library permanently shows "Never scanned" in the SPA regardless of successful scans, even as the item count updates correctly.

Remote scanners do set the library-level value (e.g. SynchronizeJellyfinLibraryByIdHandler.cs:92 sets parameters.Library.LastScan = DateTime.UtcNow), so this is local-library-specific and reproduces every scan.

Correction (2026-07-17, from live E2E on PR #408): the "stays null / always shows Never scanned" symptom above is only half right — it was written from reading the code, not the DB. Live state: an unscanned local library's LastScan is DateTime.MinValue, not NULL (a fresh DB seeds 6 of the 7 built-in local libraries with 0001-01-01 00:00:00; only Remote Streams is NULL). Since the SPA branches on truthiness, such a library actually renders "Last scan 12:00 AM", not "Never scanned". The root cause below is unaffected and confirmed — the local scanner never wrote library-level LastScan — and that is what this issue fixes. The residual badge bug is filed separately as #409.

Not a #202 regression

The read API + this scanner both predate #202; #202's SPA hub merely made the missing value visible. Filed as a follow-up, not a gate.

Fix

Set library.LastScan = DateTime.UtcNow on the local library entity after a successful scan in ScanLocalLibraryHandler (mirroring the remote scanners), and persist it. Add a scanner/handler test.

Found via #202 live E2E (PR #262).

Done-when

  • ScanLocalLibraryHandler sets and persists library-level LastScan after a successful local scan, mirroring the remote scanners' IsRight-only semantics
  • Regression test added in ErsatzTV.Scanner.Tests, proven non-vacuous via a negative control (test fails when the fix is inverted)
  • Skip/failure semantics covered: a skipped (unforced, interval not elapsed) scan and a failed path scan must NOT record a library-level scan time
  • Full local test pass green (Scanner.Tests 1471 + ErsatzTV.Tests 1648 + Core.Tests 548)
  • Verified end-to-end that a local library stops reporting "Never scanned" after a successful scan — live run: Library.LastScan 0001-01-01 → 2026-07-17 12:29:55, round-tripped through GET /api/v1/media-sources
  • Adversarial/independent review passed — three cold passes, one per commit; Review-verdict: MERGEABLE @ 71708633
  • Docs updated if any convention changed — no /api/*, route, or convention change; docs/testing.md updated anyway to map ErsatzTV.Scanner.Tests (a gap this PR's new test file exposed)
**Severity: Low (display).** Pre-existing scanner bug, surfaced by the #202 SPA libraries hub during live E2E. ## What's wrong `ErsatzTV.Scanner/Application/MediaSources/Commands/ScanLocalLibraryHandler.cs:145` sets `libraryPath.LastScan = DateTime.UtcNow` (path-level) after a successful scan but **never sets `library.LastScan`** (library-level). The read API `GetAllMediaSourcesForApiHandler` (`ErsatzTV.Application/MediaSources/Queries/GetAllMediaSourcesForApiHandler.cs:39`) reads `library.LastScan` to populate the hub's scan-time / "Never scanned" badge — so **every local library permanently shows "Never scanned"** in the SPA regardless of successful scans, even as the item count updates correctly. Remote scanners do set the library-level value (e.g. `SynchronizeJellyfinLibraryByIdHandler.cs:92` sets `parameters.Library.LastScan = DateTime.UtcNow`), so this is local-library-specific and reproduces every scan. > **Correction (2026-07-17, from live E2E on PR #408):** the "stays null / always shows *Never scanned*" symptom above is only half right — it was written from reading the code, not the DB. Live state: an unscanned local library's `LastScan` is **`DateTime.MinValue`, not `NULL`** (a fresh DB seeds 6 of the 7 built-in local libraries with `0001-01-01 00:00:00`; only Remote Streams is `NULL`). Since the SPA branches on truthiness, such a library actually renders **"Last scan 12:00 AM"**, not "Never scanned". The **root cause below is unaffected and confirmed** — the local scanner never wrote library-level `LastScan` — and that is what this issue fixes. The residual badge bug is filed separately as **#409**. ## Not a #202 regression The read API + this scanner both predate #202; #202's SPA hub merely made the missing value visible. Filed as a follow-up, not a gate. ## Fix Set `library.LastScan = DateTime.UtcNow` on the local library entity after a successful scan in `ScanLocalLibraryHandler` (mirroring the remote scanners), and persist it. Add a scanner/handler test. Found via #202 live E2E (PR #262). ## Done-when - [x] `ScanLocalLibraryHandler` sets and persists library-level `LastScan` after a successful local scan, mirroring the remote scanners' `IsRight`-only semantics - [x] Regression test added in `ErsatzTV.Scanner.Tests`, proven non-vacuous via a negative control (test fails when the fix is inverted) - [x] Skip/failure semantics covered: a skipped (unforced, interval not elapsed) scan and a failed path scan must NOT record a library-level scan time - [x] Full local test pass green (Scanner.Tests 1471 + ErsatzTV.Tests 1648 + Core.Tests 548) - [x] Verified end-to-end that a local library stops reporting "Never scanned" after a successful scan — live run: `Library.LastScan` `0001-01-01` → `2026-07-17 12:29:55`, round-tripped through `GET /api/v1/media-sources` - [x] Adversarial/independent review passed — three cold passes, one per commit; `Review-verdict: MERGEABLE @ 71708633` - [x] Docs updated if any convention changed — no `/api/*`, route, or convention change; `docs/testing.md` updated anyway to map `ErsatzTV.Scanner.Tests` (a gap this PR's new test file exposed)
timothy added the bug label 2026-07-11 17:17:33 +02:00
timothy added the priority: low label 2026-07-11 19:03:45 +02:00
timothy added priority: medium and removed priority: low labels 2026-07-17 00:25:10 +02:00
timothy added the in-progress label 2026-07-17 14:09:17 +02:00
timothy commented 2026-07-17 14:09:32 +02:00
Author
Owner
Copy Link
Copy Source

🔵 Claiming (Claude Code / Opus 4.8 orchestrator session, 2026-07-17).

Live claimability verified from raw Gitea state (not selector prose): OPEN, unclaimed, priority: medium, GET /issues/264/dependencies → [] (no open blocked-by).

How the queue landed here. Tier 1 (open milestones) yielded nothing claimable: the Auto-Tune DetailPanel milestone is an umbrella (#383) whose children are either shipped (#384) or blocked behind in-flight #70 (#385, #386); the Scheduling milestone's #395 is blocked by #381 and #70 is claimed with PR #402 open; #59's milestone is now empty (closed this session — the arc it tracked has shipped). Tier 2 is empty: all five adversarial-reviewer audits (#20–#24) are claimed and carry posted findings deliverables. No open priority: high exists, so the queue falls through to priority: medium, ordered by lowest issue number → #264. (#176 was the strict tier-1 winner but was explicitly deferred by the user this session.)

Selector note (two errors caught, consistent with the standing lore): the cheap selector ranked #383 as winner without noticing it is a container, and reported audits #20–#23 as "unclaimed" when all four have posted deliverables. Re-derived the whole contested tier locally (fetch-all-open + filter on .milestone.title, per-candidate /dependencies) rather than rechecking only the proposed winner.

Plan: verify the diagnosis in the body still holds against current main before trusting it (the issue is 6 days old, and per our standing "recon before building" rule a stated root cause is a hypothesis until re-checked), then mirror the remote scanners' library-level LastScan write in ScanLocalLibraryHandler + add a regression test. Adding a ## Done-when section to the body so the H6/H10 merge gate can derive consent.

🔵 **Claiming** (Claude Code / Opus 4.8 orchestrator session, 2026-07-17). **Live claimability verified from raw Gitea state** (not selector prose): OPEN, unclaimed, `priority: medium`, `GET /issues/264/dependencies` → `[]` (no open `blocked-by`). **How the queue landed here.** Tier 1 (open milestones) yielded nothing claimable: the Auto-Tune DetailPanel milestone is an umbrella (#383) whose children are either shipped (#384) or blocked behind in-flight #70 (#385, #386); the Scheduling milestone's #395 is blocked by #381 and #70 is claimed with PR #402 open; #59's milestone is now empty (closed this session — the arc it tracked has shipped). Tier 2 is empty: all five adversarial-reviewer audits (#20–#24) are claimed **and** carry posted findings deliverables. No open `priority: high` exists, so the queue falls through to `priority: medium`, ordered by lowest issue number → #264. (#176 was the strict tier-1 winner but was explicitly deferred by the user this session.) **Selector note (two errors caught, consistent with the standing lore):** the cheap selector ranked #383 as winner without noticing it is a container, and reported audits #20–#23 as "unclaimed" when all four have posted deliverables. Re-derived the whole contested tier locally (fetch-all-open + filter on `.milestone.title`, per-candidate `/dependencies`) rather than rechecking only the proposed winner. **Plan**: verify the diagnosis in the body still holds against current `main` before trusting it (the issue is 6 days old, and per our standing "recon before building" rule a stated root cause is a hypothesis until re-checked), then mirror the remote scanners' library-level `LastScan` write in `ScanLocalLibraryHandler` + add a regression test. Adding a `## Done-when` section to the body so the H6/H10 merge gate can derive consent.
timothy removed the in-progress label 2026-07-17 15:02:40 +02:00
timothy commented 2026-07-17 15:03:21 +02:00
Author
Owner
Copy Link
Copy Source

Closed — fixed and verified live

Merged as 5ac49e55 (PR #408 → main), CI green at head 71708633; branch deleted.

Root cause (why it existed, not just what changed)

ScanLocalLibraryHandler wrote only the path-level LibraryPath.LastScan — the value that gates the per-path refresh interval — and never the library-level Library.LastScan, which is the value GetAllMediaSourcesForApiHandler projects into the SPA's scan-time badge. Two different fields, same name, different purposes: the one the scanner maintained was not the one the UI reads.

Why only here: the three remote scanners (Jellyfin/Emby/Plex) each set the library-level value on success. The local scanner is the only one that scans per path rather than per library, and the library-level write was simply never added to that loop. I enumerated all four LastScan write sites rather than trusting the cited line — 3 remote (correct) + 1 local (the bug). Both this handler and the read API predate #202; the #202 hub only made the gap visible, so it was never a #202 regression.

What was done

  • ScanLocalLibraryHandler now records library-level LastScan when the scan actually ran and every path that ran succeeded — mirroring the remote scanners' IsRight-only semantics. A skipped (unforced, interval not elapsed) scan records nothing; a partially-failed scan records nothing at library level while successful paths still get their path-level timestamp.
  • Added error logging for failed path scans (review finding): a failed path now suppresses the library-level stamp, so without a log the user would hit the #264 symptom with no explanation — the same diagnosis dead-end this issue is about.
  • 4 tests in ErsatzTV.Scanner.Tests + a logging assertion, all proven non-vacuous by negative control.

Files changed

  • ErsatzTV.Scanner/Application/MediaSources/Commands/ScanLocalLibraryHandler.cs (+ de-BOM'd per the #311 fix-as-you-touch gate)
  • ErsatzTV.Scanner.Tests/Application/MediaSources/ScanLocalLibraryHandlerTests.cs (new)
  • docs/testing.md

Verification

  • Local: Scanner.Tests 1471, ErsatzTV.Tests 1648, Core.Tests 548 — all green.
  • Live E2E (not strictly required — docs/e2e-local.md scopes that to API write-path handlers — but run anyway, since this is an observable-badge bug): seeded a local Shows library, scanned via the real endpoint (HTTP 202, 2 episodes), and confirmed Library.LastScan went 0001-01-01 → 2026-07-17 12:29:55, round-tripping through GET /api/v1/media-sources.
  • Review: three independent cold passes, one per commit (per the #242 re-review-the-fix-commit rule) → MERGEABLE @ 71708633.

The issue's premise was partly wrong (corrected in the body)

Live E2E disproved the stated symptom: an unscanned local library's LastScan is DateTime.MinValue, not NULL — a fresh DB seeds 6 of 7 built-in local libraries with 0001-01-01 and only Remote Streams with NULL. Since the SPA branches on truthiness and MinValue serialises to a truthy string, such a library actually rendered "Last scan 12:00 AM", not "Never scanned". The root cause above was real and is fixed; the symptom description had been written from reading the code rather than the DB.

Follow-ups filed (deferred deliberately, not dropped)

  • #409 — never-scanned libraries render "Last scan 12:00 AM" (MinValue is truthy). The mirror image of this bug; three candidate fix directions. Not folded in here — it's a distinct defect in the read/render path, not the scanner.
  • #410 — a cancelled scan now logs at ERROR across all four scan handlers (ScanCanceled is an ordinary BaseError). Not fixed here because this PR made the local handler consistent with the three remote ones, which have behaved this way all along; a local-only fix would re-split them.

Docs updated

docs/testing.md — added the missing ErsatzTV.Scanner.Tests row (the project has 1471 tests and was absent from a table that claims to be the authoritative map; this PR adding a file to it made the gap concrete) and documented Core/FFmpeg/TranscodingTests as [Explicit]/opt-in. No /api/*, route, or convention change, so no other doc applies.

## Closed — fixed and verified live Merged as **5ac49e55** (PR #408 → `main`), CI green at head `71708633`; branch deleted. ### Root cause (why it existed, not just what changed) `ScanLocalLibraryHandler` wrote **only** the path-level `LibraryPath.LastScan` — the value that gates the per-path refresh interval — and never the **library-level** `Library.LastScan`, which is the value `GetAllMediaSourcesForApiHandler` projects into the SPA's scan-time badge. Two different fields, same name, different purposes: the one the scanner maintained was not the one the UI reads. Why only here: the three remote scanners (Jellyfin/Emby/Plex) each set the library-level value on success. The local scanner is the only one that scans *per path* rather than per library, and the library-level write was simply never added to that loop. I enumerated all four `LastScan` write sites rather than trusting the cited line — 3 remote (correct) + 1 local (the bug). Both this handler and the read API predate #202; the #202 hub only made the gap visible, so it was never a #202 regression. ### What was done - `ScanLocalLibraryHandler` now records library-level `LastScan` when the scan **actually ran** and **every path that ran succeeded** — mirroring the remote scanners' `IsRight`-only semantics. A skipped (unforced, interval not elapsed) scan records nothing; a partially-failed scan records nothing at library level while successful paths still get their path-level timestamp. - Added error logging for failed path scans (review finding): a failed path now suppresses the library-level stamp, so without a log the user would hit the #264 symptom with no explanation — the same diagnosis dead-end this issue is about. - 4 tests in `ErsatzTV.Scanner.Tests` + a logging assertion, all proven non-vacuous by negative control. ### Files changed - `ErsatzTV.Scanner/Application/MediaSources/Commands/ScanLocalLibraryHandler.cs` (+ de-BOM'd per the #311 fix-as-you-touch gate) - `ErsatzTV.Scanner.Tests/Application/MediaSources/ScanLocalLibraryHandlerTests.cs` (new) - `docs/testing.md` ### Verification - Local: Scanner.Tests 1471, ErsatzTV.Tests 1648, Core.Tests 548 — all green. - **Live E2E** (not strictly required — `docs/e2e-local.md` scopes that to API write-path handlers — but run anyway, since this is an observable-badge bug): seeded a local Shows library, scanned via the real endpoint (`HTTP 202`, 2 episodes), and confirmed `Library.LastScan` went `0001-01-01` → `2026-07-17 12:29:55`, round-tripping through `GET /api/v1/media-sources`. - **Review**: three independent cold passes, one per commit (per the #242 re-review-the-fix-commit rule) → `MERGEABLE @ 71708633`. ### The issue's premise was partly wrong (corrected in the body) Live E2E disproved the stated symptom: an unscanned local library's `LastScan` is **`DateTime.MinValue`, not `NULL`** — a fresh DB seeds 6 of 7 built-in local libraries with `0001-01-01` and only Remote Streams with `NULL`. Since the SPA branches on truthiness and MinValue serialises to a truthy string, such a library actually rendered **"Last scan 12:00 AM"**, not "Never scanned". The root cause above was real and is fixed; the symptom description had been written from reading the code rather than the DB. ### Follow-ups filed (deferred deliberately, not dropped) - **#409** — never-scanned libraries render "Last scan 12:00 AM" (MinValue is truthy). The mirror image of this bug; three candidate fix directions. Not folded in here — it's a distinct defect in the read/render path, not the scanner. - **#410** — a *cancelled* scan now logs at ERROR across all four scan handlers (`ScanCanceled` is an ordinary `BaseError`). Not fixed here because this PR made the local handler **consistent** with the three remote ones, which have behaved this way all along; a local-only fix would re-split them. ### Docs updated `docs/testing.md` — added the missing `ErsatzTV.Scanner.Tests` row (the project has 1471 tests and was absent from a table that claims to be the authoritative map; this PR adding a file to it made the gap concrete) and documented `Core/FFmpeg/TranscodingTests` as `[Explicit]`/opt-in. No `/api/*`, route, or convention change, so no other doc applies.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
ad-hoc
One-off / ad-hoc work not tracked by a dedicated issue
 api
REST API / HTTP endpoints
 bug
Something isn't working
 ci-cd
Build, test, deploy pipeline
 content
Channel content / schedules / playlists
 dependencies
Dependency updates (Renovate)
 enhancement
New feature or improvement
 frontend
ChicoryTV React SPA frontend
 in-progress
Claimed by an active session — do not pick up
 jellyfin
Jellyfin tuner / IPTV integration
 parked
Excluded from automatic queue pickup; work only when explicitly selected
 priority: high
priority: low
priority: medium
review
Adversarial review finding
 security
Security / vulnerability fix
No labels bug priority: medium
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
renovate timothy (Timothy)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: timothy/ersatztv#264