From 91b0c45dd488e2129d8c646e4392361cdada9ce6 Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Tue, 20 Oct 2026 18:24:08 -0400 Subject: [PATCH] =?UTF-8?q?feat(scanner):=20extractor-version=20(ev)=20mec?= =?UTF-8?q?hanism=20=E2=80=94=20parser=20fixes=20reach=20old=20rows=20with?= =?UTF-8?q?out=20a=20force=20rescan=20(faceted=20plan=20Phase=2010)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Parser fixes (7e4c97f6, genre model v2) never reached rows ingested under an older parser: the sidecar never changed, so every fingerprint said 'unchanged'. The manifest JSONB now carries the reserved 'ev' stamp — the parser version that produced the item's row. Semantics (all pinned by tests): - ABSENT ev decodes as 1 ('v1-or-unknown'); stamping is truthful-only: create paths + post-extraction stores stamp the setting's current value; deploy-day backfill and the extract-failure guard stay UNSET (a row never claims a version its data can't prove — a mid-backfill bump self-heals next poll); repoint reconciliation preserves the stored ev (stat-only refresh re-extracted nothing). - NEW setting scan_extractor_version (seeded + registry + getter, int default 1, category scanner): a manifest whose ev != the setting re-extracts through the SAME change path (full extraction → updateMediaItemWithMetadata → fresh stamped manifest). Sidecar-less books get an entries-less manifest as their ev home (create: stamped; first-sighting: unset). - Catch-up bounded: evReextractBudgetPerScan=100, atomic (watcher pass can overlap a poll scan), reset per ScanFolders — amortized + resumable across polls; no force rescan, no flood. - Multifile books included: the ev gate rides multifileSidecarChanged's shared ladder; ev-driven manifestChanged drives the editorial apply + stamped re-store. Settings API: the unified GET/PUT /api/system/settings now lists the key (registry-driven, auto-surfaces in the admin UI). Test harness: the registry is now Loaded + wired into the SystemSettingsHandler (main.go parity) — settings writes through the API were 503s before. Verified: unit (ev normalization, budget CAS, GetExtractorVersion cache paths, registry clamping); integration state machine — create-stamp ev=1 → equality skips (corruption probe persists) → wiped scan-state backfills UNSET without re-extract → bump re-extracts ONCE (sidecar value restored, ev=2) → stable (second corruption persists); multifile twin green; adjacent suites (Audiobook, Calibre, CrossLibraryMove, SchemaIdempotency) green; live app restart re-scans 1198 files with 0 changed (deploy-day rule), setting listed at /api/system/settings. --- cmd/server/tests/extractor_version_test.go | 182 ++++++++++++++++++ cmd/server/tests/test_helpers_test.go | 14 +- database/schema/schema.sql | 9 +- docs/contract_pins.md | 25 +++ internal/database/settings_registry.go | 7 + internal/services/media_scanner.go | 60 +++++- .../services/media_scanner_audio_multifile.go | 5 + internal/services/media_scanner_ev_test.go | 68 +++++++ internal/services/media_scanner_manifest.go | 92 ++++++++- .../services/media_scanner_settings_test.go | 34 ++++ 10 files changed, 486 insertions(+), 10 deletions(-) create mode 100644 cmd/server/tests/extractor_version_test.go create mode 100644 internal/services/media_scanner_ev_test.go diff --git a/cmd/server/tests/extractor_version_test.go b/cmd/server/tests/extractor_version_test.go new file mode 100644 index 00000000..9816a0c3 --- /dev/null +++ b/cmd/server/tests/extractor_version_test.go @@ -0,0 +1,182 @@ +package main + +// Extractor-version (ev) mechanism, faceted plan Phase 10: parser fixes +// (7e4c97f6, genre model v2) must reach rows ingested under an older +// parser without a force rescan. The manifest's ev stamp is the +// per-item provenance; scan_extractor_version is the current parser's +// claim; a mismatch re-extracts on a later scan, bounded per scan. +// +// State machine pinned here, driven through REAL scans over a temp +// library (the corruption probe makes re-extraction observable: the +// only writer that restores the sidecar values is a re-extraction): +// +// create → stamped ev=1 (current parser, truthful) +// equal → no re-extract (corruption persists) +// wiped → backfill stores ev UNSET, never stamps, never re-extracts +// bump → re-extract ONCE (corruption healed, ev=2) +// equal → stable (second corruption persists) + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "net/http" + "os" + "path/filepath" + "strconv" + "testing" + "time" + + "github.com/jackc/pgx/v5/pgtype" + "github.com/stretchr/testify/require" +) + +// storedManifestEv reads the "ev" stamp from an item's stored manifest +// (0 = absent — the backfill's v1-or-unknown marker). +func storedManifestEv(t *testing.T, setup *TestServerSetup, itemID pgtype.UUID) int { + t.Helper() + row, err := setup.DB.GetMediaScanState(context.Background(), itemID) + require.NoError(t, err, "scan-state row must exist — every book needs an ev home") + var m map[string]any + require.NoError(t, json.Unmarshal(row.Manifest, &m)) + ev, _ := m["ev"].(float64) + return int(ev) +} + +// corruptItemColumn scribbles a scan-owned column on a TEST-CREATED item +// so a later re-extraction is observable (the sidecar rewrites it back). +// The row is test data cascade-deleted with the library — never real rows. +func corruptItemColumn(t *testing.T, setup *TestServerSetup, itemID pgtype.UUID, column, value string) { + t.Helper() + tag, err := setup.DBPool.Exec(context.Background(), + fmt.Sprintf("UPDATE media_items SET %s = $1 WHERE id = $2", column), value, itemID) + require.NoError(t, err) + require.Equal(t, int64(1), tag.RowsAffected(), "corruption must hit exactly the test item") +} + + +func setExtractorVersion(t *testing.T, setup *TestServerSetup, v int) { + t.Helper() + body, _ := json.Marshal(map[string]string{"key": "scan_extractor_version", "value": strconv.Itoa(v)}) + req, _ := http.NewRequest("PUT", setup.Server.URL+"/api/system/settings", bytes.NewBuffer(body)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", "Bearer "+setup.Token) + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + defer func() { _ = resp.Body.Close() }() + require.Equal(t, http.StatusOK, resp.StatusCode, "settings API is the only sanctioned write path") +} + +func TestExtractorVersionReextract(t *testing.T) { + setup := setupTestServer(t) + t.Cleanup(func() { _ = setup.Close() }) + + ctx := context.Background() + admin, _ := moveTestAdminID(t, setup) + libID, folder := createTypedTestLibrary(t, setup, fmt.Sprintf("Test EV Library %d", time.Now().UnixNano()), "ebooks") + t.Cleanup(func() { deleteLibraries(t, setup, libID) }) + + // metadata.opf sidecar: the re-extraction's provenance source. + bookDir := filepath.Join(folder, "EV Probe Author", "EV Probe Book") + require.NoError(t, os.MkdirAll(bookDir, 0o755)) + opf := ` + + + EV Probe Book + EV Probe Author + +` + require.NoError(t, os.WriteFile(filepath.Join(bookDir, "metadata.opf"), []byte(opf), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(bookDir, "EV Probe Book.epub"), []byte("dummy epub content"), 0o644)) + + scanTestFolder(t, setup, admin, folder) + item, found := findAudioItemByPath(t, setup, libID, filepath.ToSlash("EV Probe Author/EV Probe Book/EV Probe Book.epub")) + require.True(t, found, "sidecar book must import") + + // Create stamps the current parser version (truthful). + require.Equal(t, 1, storedManifestEv(t, setup, item.ID), "create must stamp the default ev=1") + + // Equality → skip: corruption persists (nothing re-extracts at ev=1). + // NOTE: `author` is trigger-owned (sync_author_publisher_display + // rewrites it from the authors array on every UPDATE) — corrupt a + // trigger-free column the re-extraction rewrites: title. + corruptItemColumn(t, setup, item.ID, "title", "ZZZ corrupted") + scanTestFolder(t, setup, admin, folder) + item, found = findAudioItemByPath(t, setup, libID, filepath.ToSlash("EV Probe Author/EV Probe Book/EV Probe Book.epub")) + require.True(t, found) + require.Equal(t, "ZZZ corrupted", item.Title, "equal ev must NOT re-extract (corruption persists)") + require.Equal(t, 1, storedManifestEv(t, setup, item.ID)) + + // Wipe the scan state (test-owned row) to simulate a pre-release + // book: backfill re-fingerprints WITHOUT re-extraction and WITHOUT + // stamping — the row never claims a version it can't prove. + _, err := setup.DBPool.Exec(ctx, "DELETE FROM media_scan_state WHERE media_item_id = $1", item.ID) + require.NoError(t, err) + scanTestFolder(t, setup, admin, folder) + require.Equal(t, 0, storedManifestEv(t, setup, item.ID), "backfill must store ev UNSET (v1-or-unknown)") + item, _ = findAudioItemByPath(t, setup, libID, filepath.ToSlash("EV Probe Author/EV Probe Book/EV Probe Book.epub")) + require.Equal(t, "ZZZ corrupted", item.Title, "backfill must NOT re-extract") + + // Bump via the SETTINGS API; the absent ev rides v1 → mismatch → + // re-extract once: the corruption heals and the fresh manifest + // carries ev=2. + setExtractorVersion(t, setup, 2) + t.Cleanup(func() { setExtractorVersion(t, setup, 1) }) + scanTestFolder(t, setup, admin, folder) + item, found = findAudioItemByPath(t, setup, libID, filepath.ToSlash("EV Probe Author/EV Probe Book/EV Probe Book.epub")) + require.True(t, found) + require.Equal(t, "EV Probe Book", item.Title, "ev mismatch must re-extract (sidecar value restored)") + require.Equal(t, 2, storedManifestEv(t, setup, item.ID), "post-extraction store stamps the new ev") + + // Then stable: the second corruption persists at ev=2 == setting 2. + corruptItemColumn(t, setup, item.ID, "title", "ZZZ corrupted again") + scanTestFolder(t, setup, admin, folder) + item, _ = findAudioItemByPath(t, setup, libID, filepath.ToSlash("EV Probe Author/EV Probe Book/EV Probe Book.epub")) + require.Equal(t, "ZZZ corrupted again", item.Title, "equal ev must NOT re-extract again (stable)") + require.Equal(t, 2, storedManifestEv(t, setup, item.ID)) +} + +func TestExtractorVersionMultifile(t *testing.T) { + setup := setupTestServer(t) + t.Cleanup(func() { _ = setup.Close() }) + + admin, _ := moveTestAdminID(t, setup) + libID, folder := audioTestLibrary(t, setup, fmt.Sprintf("Test EV Audio %d", time.Now().UnixNano())) + t.Cleanup(func() { deleteLibraries(t, setup, libID) }) + + // A canonical folder book, sidecar-less: the Phase 6 seed gives it + // an entries-less manifest — now also its ev home. + writeTestTrack(t, filepath.Join(folder, "EV Audio Author", "EV Audio Title", "01 First.mp3")) + writeTestTrack(t, filepath.Join(folder, "EV Audio Author", "EV Audio Title", "02 Second.mp3")) + + scanTestFolder(t, setup, admin, folder) + book, found := findAudioItemByPath(t, setup, libID, filepath.ToSlash("EV Audio Author/EV Audio Title")) + require.True(t, found, "folder book must import") + require.Equal(t, "EV Audio Title", book.Title) + require.Equal(t, 1, storedManifestEv(t, setup, book.ID), "multifile create must stamp the default ev=1") + + // Equality → skip: the multifile ladder's unchanged gate stays shut. + corruptItemColumn(t, setup, book.ID, "title", "ZZZ corrupted") + scanTestFolder(t, setup, admin, folder) + book, _ = findAudioItemByPath(t, setup, libID, filepath.ToSlash("EV Audio Author/EV Audio Title")) + require.Equal(t, "ZZZ corrupted", book.Title, "equal ev must NOT re-apply (corruption persists)") + require.Equal(t, 1, storedManifestEv(t, setup, book.ID)) + + // Bump → the ev gate fires through multifileSidecarChanged → the + // editorial apply re-derives the title from the folder basename. + setExtractorVersion(t, setup, 2) + t.Cleanup(func() { setExtractorVersion(t, setup, 1) }) + scanTestFolder(t, setup, admin, folder) + book, found = findAudioItemByPath(t, setup, libID, filepath.ToSlash("EV Audio Author/EV Audio Title")) + require.True(t, found) + require.Equal(t, "EV Audio Title", book.Title, "ev mismatch must re-extract the folder book") + require.Equal(t, 2, storedManifestEv(t, setup, book.ID), "change-path store stamps the new ev") + + // Then stable. + corruptItemColumn(t, setup, book.ID, "title", "ZZZ corrupted again") + scanTestFolder(t, setup, admin, folder) + book, _ = findAudioItemByPath(t, setup, libID, filepath.ToSlash("EV Audio Author/EV Audio Title")) + require.Equal(t, "ZZZ corrupted again", book.Title, "equal ev must NOT re-apply again (stable)") + require.Equal(t, 2, storedManifestEv(t, setup, book.ID)) +} diff --git a/cmd/server/tests/test_helpers_test.go b/cmd/server/tests/test_helpers_test.go index 7f432696..dd8e120c 100644 --- a/cmd/server/tests/test_helpers_test.go +++ b/cmd/server/tests/test_helpers_test.go @@ -467,6 +467,17 @@ func setupTestServer(t *testing.T) *TestServerSetup { queries := database.New(dbPool) + // Wire the tunable settings registry exactly like main.go: Load + // populates the cache (missing rows fall back to compiled defaults) + // and the SystemSettingsHandler must hold it or every + // /api/system/settings call 503s ("settings registry not + // initialized") — tests that write settings through the API (the + // only sanctioned path) need this wired. + settingsRegistry := database.NewSettingsRegistry(queries) + if err := settingsRegistry.Load(context.Background()); err != nil { + t.Logf("settings registry Load failed (defaults apply): %v", err) + } + // Create login attempt tracker loginAttemptTracker := ratelimit.NewLoginAttemptTracker(5, 15*time.Minute, 5*time.Minute) @@ -493,6 +504,7 @@ func setupTestServer(t *testing.T) *TestServerSetup { analyticsHandler := handlers.NewAnalyticsHandler(queries) queueHandler := handlers.NewQueueHandler(queries, queueProcessor) systemSettingsHandler := handlers.NewSystemSettingsHandler(queries) + systemSettingsHandler.SetSettings(settingsRegistry) processingIssuesHandler := handlers.NewProcessingIssuesHandler(queries) // Create refactored handlers (matching main.go) @@ -571,7 +583,7 @@ func setupTestServer(t *testing.T) *TestServerSetup { // RegisterRoutes dereferences Settings for the auth rate limit; // the main binary wires database.NewSettingsRegistry(queries) — // the test harness must too or route registration nil-panics. - Settings: database.NewSettingsRegistry(queries), + Settings: settingsRegistry, // main.go:246 passes the shared LibraryService here; omitting it // hands the kobo/kepub CFI converter a typed-nil (kobo markup // panic-recovered as 500s in TestProgressWeb_KoboIntegration). diff --git a/database/schema/schema.sql b/database/schema/schema.sql index 4775fe9e..a2f4ed5f 100644 --- a/database/schema/schema.sql +++ b/database/schema/schema.sql @@ -61,6 +61,7 @@ ALTER TABLE system_settings ADD COLUMN IF NOT EXISTS category VARCHAR(40); INSERT INTO system_settings (setting_key, setting_value, description, setting_type, min_value, max_value, requires_restart, category) VALUES ('scan_poll_interval_seconds', '60', 'How often to scan all libraries (seconds)', 'int', '1', '3600', FALSE, 'scanner'), ('auto_scan_enabled', 'true', 'Whether auto-scanning is enabled system-wide', 'bool', NULL, NULL, FALSE, 'scanner'), +('scan_extractor_version', '1', 'Parser version stamped on each book scan manifest; raising it re-extracts metadata from books scanned under an older parser (bounded batch per scan)', 'int', '1', '1000000', FALSE, 'scanner'), ('auto_playlist_collections', 'true', 'Auto-create curated playlist collections from sidecar series marked as reading orders', 'bool', NULL, NULL, FALSE, 'scanner'), ('default_timezone', 'UTC', 'System default timezone', 'string', NULL, NULL, FALSE, 'general'), -- security / auth (live) @@ -1258,8 +1259,12 @@ CREATE INDEX IF NOT EXISTS idx_media_item_formats_sha256 ON media_item_formats(f -- would hide edits landing in the backfill scan's own second), h -- sha256 of the content — present for ALL entries, covers included: -- the repoint reconciliation needs a baseline hash for any stat- --- drifted sidecar. "ev" (extractor version) reserved for parser-fix --- re-extraction. +-- drifted sidecar. "ev" (extractor version, default 1): the parser +-- version that produced the item's row. ABSENT means "v1-or-unknown" +-- — backfill stamps are unset so a row never claims a version its +-- data can't prove; every post-extraction store stamps the setting's +-- current value. A stored ev != scan_extractor_version → re-extract +-- (bounded per scan, resumable across polls). CREATE TABLE IF NOT EXISTS media_scan_state ( media_item_id UUID PRIMARY KEY REFERENCES media_items(id) ON DELETE CASCADE, manifest JSONB diff --git a/docs/contract_pins.md b/docs/contract_pins.md index 6b22ca81..7ee0c91a 100644 --- a/docs/contract_pins.md +++ b/docs/contract_pins.md @@ -342,3 +342,28 @@ cover_image_path, has_conflict, format_group, badge`. - The Phase-7 facet-panel partial stays removed; the all-facets JSON endpoint and its two shapes are unchanged (they serve flat params — the builder's UI does not use them). + +## 2026-10-20 — Scanner Phase 10: extractor version (ev) mechanism +- `media_scan_state.manifest` JSONB gains the reserved `ev` key: + the parser version that produced the item's CURRENT row. ABSENT + means "v1-or-unknown" — decode normalizes it to 1. +- NEW setting `scan_extractor_version` (int, default 1, min 1, + category scanner): the unified `GET/PUT /api/system/settings` list + now includes this key (additive; the list was not a pinned closed + set). Writes stay settings-API-only — direct SQL leaves the registry + cache stale (known gotcha). +- Scanner behavior: a stored manifest whose ev ≠ the setting is + treated as CHANGED → full re-extract + the fresh manifest stamps + the new ev. Stamping is TRUTHFUL-ONLY: create paths and + post-extraction stores stamp; deploy-day BACKFILL and the + extract-failure guard store UNSET (the row never claims a version + its data can't prove — a mid-backfill bump self-heals next poll). + Repoint reconciliation preserves the stored ev (stat-only refresh, + nothing re-extracted). +- Catch-up is BOUNDED per scan pass (`evReextractBudgetPerScan = 100`, + atomic, reset at each ScanFolders): resumable across polls, no + force rescan, no flood. Budget exhaustion leaves ev stale → later + polls resume. +- Deploy-day rule (unchanged, re-verified live): manifests without ev + ride v1; at default setting 1 the first post-release scan re-extracts + NOTHING (0 changed on 1198 files). diff --git a/internal/database/settings_registry.go b/internal/database/settings_registry.go index 0137090a..8983caae 100644 --- a/internal/database/settings_registry.go +++ b/internal/database/settings_registry.go @@ -59,6 +59,7 @@ var SettingDefaults = []SettingDefault{ {Key: "scan_poll_interval_seconds", Value: "60", Type: SettingTypeInt, Min: "1", Max: "3600", Category: "scanner", Group: "Scanning", Description: "How often to scan all libraries (seconds)"}, {Key: "auto_scan_enabled", Value: "true", Type: SettingTypeBool, Category: "scanner", Group: "Scanning", Description: "Whether auto-scanning is enabled system-wide"}, {Key: "auto_playlist_collections", Value: "true", Type: SettingTypeBool, Category: "scanner", Group: "Scanning", Description: "Auto-create curated playlist collections from sidecar series marked as reading orders"}, + {Key: "scan_extractor_version", Value: "1", Type: SettingTypeInt, Min: "1", Max: "1000000", Category: "scanner", Group: "Scanning", Description: "Parser version stamped on each book's scan manifest. Raising it re-extracts metadata from books scanned under an older parser, a bounded batch per scan until all are caught up"}, {Key: "default_timezone", Value: "UTC", Type: SettingTypeString, Category: "general", Group: "System Defaults", Description: "System default timezone"}, {Key: "session_duration_seconds", Value: "604800", Type: SettingTypeInt, Min: "300", Max: "31536000", Category: "security", Group: "Session", Description: "How long a login session stays valid"}, {Key: "password_min_length", Value: "8", Type: SettingTypeInt, Min: "1", Max: "128", Category: "security", Group: "Password Quality", Description: "Minimum password length"}, @@ -195,6 +196,12 @@ func (r *SettingsRegistry) AutoScanEnabled() bool { return r.getBool("auto_scan_ // curated static collections (A2c §2.4). func (r *SettingsRegistry) AutoPlaylistCollections() bool { return r.getBool("auto_playlist_collections") } +// ScanExtractorVersion is the parser version stamped on each book's scan +// manifest. Books whose stored stamp differs re-extract their metadata on +// later scans (bounded per scan) — the mechanism that lets parser fixes +// reach rows ingested under an older parser without a force rescan. +func (r *SettingsRegistry) ScanExtractorVersion() int { return r.getInt("scan_extractor_version") } + // RegistrationEnabled reports whether public signup is allowed. Admin user // creation is never affected — only self-service registration rides this // flag; handlers additionally grant the first-user exception (see diff --git a/internal/services/media_scanner.go b/internal/services/media_scanner.go index 0fa59348..81e70eb6 100644 --- a/internal/services/media_scanner.go +++ b/internal/services/media_scanner.go @@ -148,6 +148,13 @@ type MediaScanner struct { watching atomic.Bool settingsCache *SettingsCache + // evBudgetRemaining is the per-scan extractor-version catch-up + // budget (faceted plan Phase 10): how many ev-mismatch books this + // pass may still re-extract. Reset at the top of ScanFolders; + // atomic because the watcher's scanDirectory pass can overlap a + // poll scan. Zero → ev-mismatch books wait for a later poll. + evBudgetRemaining atomic.Int64 + totalFiles int newItems int errors int @@ -277,6 +284,40 @@ func (s *MediaScanner) GetPollInterval() time.Duration { return time.Duration(seconds) * time.Second } +// GetExtractorVersion is the parser version stamped on each book's +// scan manifest (faceted plan Phase 10). Read through the same 30s-TTL +// settings cache as GetPollInterval — a bump via the SETTINGS API (the +// only sanctioned write path) applies on the next read within TTL. +// Version 1 is the shipped baseline parser; absent/garbage rows ride it. +func (s *MediaScanner) GetExtractorVersion() int { + if cached, ok := s.settingsCache.Get("scan_extractor_version"); ok { + if v, err := strconv.Atoi(cached); err == nil && v >= 1 { + return v + } + return 1 // cached garbage: the registry-clamped default + } + + if s.db == nil { + return 1 + } + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + setting, err := s.db.GetSystemSetting(ctx, "scan_extractor_version") + if err != nil || setting == "" { + return 1 + } + + s.settingsCache.Set("scan_extractor_version", setting) + + v, err := strconv.Atoi(setting) + if err != nil || v < 1 { + return 1 + } + return v +} + func (s *MediaScanner) GetAutoScanEnabled() bool { // Check cache first if cached, ok := s.settingsCache.Get("auto_scan_enabled"); ok { @@ -641,6 +682,11 @@ func (s *MediaScanner) ScanFolders(ctx context.Context) error { s.newItems = 0 s.errors = 0 s.changedItems = 0 + // Fresh parser-version catch-up budget every scan pass (faceted + // plan Phase 10): each pass re-extracts at most + // evReextractBudgetPerScan stale-parser books; the rest resume on + // later polls. + s.evBudgetRemaining.Store(evReextractBudgetPerScan) s.audioBookRoots = make(map[string]bool) // Bulk scan state: per-run reset; one load per distinct library. s.scanStateMu.Lock() @@ -1382,10 +1428,15 @@ func (s *MediaScanner) processMediaFile(ctx context.Context, path string) (bool, // Seed the sidecar manifest at create time (Part 2 §3): the next // scan compares instead of backfilling, so an edit is detected on - // its first post-seed scan. + // its first post-seed scan. Sidecar-less books get an entries-less + // manifest carrying the current ev (Phase 10) — the book was JUST + // extracted by this binary's parser, so the stamp is truthful and + // no re-extraction can be skipped by later bumps. dir := bookSidecarDir(path) if computed := statSidecarManifest(path, dir); computed != nil && len(computed.Entries) > 0 { s.storeManifest(ctx, libraryID, newEntry, hashSidecarManifest(path, dir, computed)) + } else { + s.storeManifest(ctx, libraryID, newEntry, &sidecarManifest{Version: sidecarManifestVersion}) } return true, nil } @@ -1513,7 +1564,12 @@ func (s *MediaScanner) processExistingViaScanState(ctx context.Context, entry *s // reachable today (mergeMetadata never fails) — this is the // regression net for the class of bug the guard exists for. fmt.Printf("Warning: sidecar re-extraction failed for %s: %v — row left unchanged\n", path, extractErr) - s.storeManifest(ctx, libraryID, entry, hashSidecarManifest(path, dir, statSidecarManifest(path, dir))) + // Baseline store, never a stamp: a stamp would record the + // current parser version on a row the current parser never + // successfully processed — for an ev-triggered change that + // would PERMANENTLY skip the fix. Leaving ev stale retries + // on the next poll (budget-bounded). + s.storeManifestBaseline(ctx, libraryID, entry, hashSidecarManifest(path, dir, statSidecarManifest(path, dir))) return false, nil } // Playlist entries ride the FIRST extraction either way — capture diff --git a/internal/services/media_scanner_audio_multifile.go b/internal/services/media_scanner_audio_multifile.go index da00492b..17b16d1a 100644 --- a/internal/services/media_scanner_audio_multifile.go +++ b/internal/services/media_scanner_audio_multifile.go @@ -1053,6 +1053,11 @@ func (s *MediaScanner) seedMultiFileManifest(ctx context.Context, libraryID pgty s.scanStateUpsert(libraryID, newEntry) if computed := statSidecarManifest(representative, dir); len(computed.Entries) > 0 { s.storeManifest(ctx, libraryID, newEntry, hashSidecarManifest(representative, dir, computed)) + } else { + // Sidecar-less book still needs an ev home (faceted plan Phase + // 10): stamp — the book was JUST extracted by the current + // parser, so the stamp is truthful. + s.storeManifest(ctx, libraryID, newEntry, &sidecarManifest{Version: sidecarManifestVersion}) } } diff --git a/internal/services/media_scanner_ev_test.go b/internal/services/media_scanner_ev_test.go new file mode 100644 index 00000000..d4d0209f --- /dev/null +++ b/internal/services/media_scanner_ev_test.go @@ -0,0 +1,68 @@ +package services + +// Unit tests for the extractor-version (ev) mechanism (faceted plan +// Phase 10): the wire normalization (absent = v1-or-unknown = 1) and the +// per-scan catch-up budget. The full scan-state machine (ev mismatch → +// re-extract once → stable) lives in cmd/server/tests +// (TestExtractorVersionReextract) because the store paths need a real DB. + +import ( + "testing" +) + +func TestManifestEvNormalization(t *testing.T) { + if got := manifestEv(nil); got != 1 { + t.Errorf("nil manifest rides v1: got %d, want 1", got) + } + cases := []struct { + raw string + want int + }{ + {`{"v":1}`, 1}, // old shape: ev absent → v1-or-unknown + {`{"v":1,"ev":0}`, 1}, // explicitly unset → same + {`{"v":1,"ev":1}`, 1}, // baseline parser stamp + {`{"v":1,"ev":2}`, 2}, // post-bump stamp + {`{"v":1,"entries":[],"ev":3}`, 3}, // sidecar-less ev home + } + for _, tc := range cases { + m := decodeSidecarManifest([]byte(tc.raw)) + if m == nil { + t.Fatalf("%s must decode", tc.raw) + } + if got := manifestEv(m); got != tc.want { + t.Errorf("manifestEv(%s) = %d, want %d", tc.raw, got, tc.want) + } + } +} + +func TestEvBudgetTake(t *testing.T) { + scanner := &MediaScanner{} + scanner.evBudgetRemaining.Store(2) + if !scanner.evBudgetTake() { + t.Error("budget 2: first take must pass") + } + if !scanner.evBudgetTake() { + t.Error("budget 1: second take must pass") + } + if scanner.evBudgetTake() { + t.Error("budget 0: third take must fail") + } + if got := scanner.evBudgetRemaining.Load(); got != 0 { + t.Errorf("budget must land at exactly 0, got %d", got) + } +} + +func TestEvBudgetTake_FailsFastAtZero(t *testing.T) { + scanner := &MediaScanner{} + scanner.evBudgetRemaining.Store(0) + // Repeated takes at 0 must fail WITHOUT driving the counter negative + // (the CAS loop only fires when cur > 0). + for i := 0; i < 5; i++ { + if scanner.evBudgetTake() { + t.Fatalf("take %d must fail at budget 0", i+1) + } + } + if got := scanner.evBudgetRemaining.Load(); got != 0 { + t.Errorf("budget must stay 0, got %d", got) + } +} diff --git a/internal/services/media_scanner_manifest.go b/internal/services/media_scanner_manifest.go index 3a273c45..8a437c5d 100644 --- a/internal/services/media_scanner_manifest.go +++ b/internal/services/media_scanner_manifest.go @@ -52,6 +52,26 @@ type manifestSidecar struct { type sidecarManifest struct { Version int `json:"v"` Entries []manifestSidecar `json:"entries"` + // Ev is the extractor (parser) version that produced the item's + // CURRENT row. Absent (0) means "v1-or-unknown": backfill and + // extract-failure stores leave it unset so a row never claims a + // version its data can't prove; every store that FOLLOWS a + // successful extraction stamps the setting's current value + // (scan_extractor_version). A stored ev below the setting re-runs + // extraction on a later scan — parser fixes reach old rows without + // a force rescan (faceted plan Phase 10). + Ev int `json:"ev,omitempty"` +} + +// manifestEv normalizes a stored manifest's extractor version: absent +// manifest or unset ev means version 1 (the shipped baseline parser). +// Current-parser versions only ever grow, so "unknown" safely rides the +// oldest live version. +func manifestEv(m *sidecarManifest) int { + if m == nil || m.Ev < 1 { + return 1 + } + return m.Ev } // coverSidecarNames mirrors findSidecarCover's priority list — the @@ -179,7 +199,7 @@ func reconcileSidecarManifest(dir string, stored, computed *sidecarManifest) (fr if !drifted { return nil, false } - fresh = &sidecarManifest{Version: sidecarManifestVersion} + fresh = &sidecarManifest{Version: sidecarManifestVersion, Ev: stored.Ev} // provenance preserved: stat-only reconciliation never re-extracted anything for _, cur := range computed.Entries { old, ok := storedByName[cur.Name] if !ok { @@ -202,8 +222,30 @@ func reconcileSidecarManifest(dir string, stored, computed *sidecarManifest) (fr return fresh, false } +// evReextractBudgetPerScan bounds the parser-version catch-up: at most +// this many ev-triggered re-extractions per scan pass. Each one stamps +// the fresh manifest with the new ev, so progress is resumable — later +// polls pick up where this one stopped, and no single scan floods the +// disk with a full-library re-extraction (faceted plan Phase 10). +const evReextractBudgetPerScan = 100 + // storeManifest persists the manifest and write-throughs the map entry. +// The stored manifest is STAMPED with the setting's current extractor +// version: every caller here follows (or is) a full extraction under +// the current parser, so the stamp is truthful. Backfill and +// extract-failure stores must use storeManifestBaseline instead. func (s *MediaScanner) storeManifest(ctx context.Context, libraryID pgtype.UUID, entry *scanEntry, manifest *sidecarManifest) { + manifest.Ev = s.GetExtractorVersion() + s.storeManifestBaseline(ctx, libraryID, entry, manifest) +} + +// storeManifestBaseline persists a manifest WITHOUT stamping the +// extractor version — for stores that follow NO successful extraction: +// the deploy-day backfill (the row's provenance is unknown; leaving ev +// unset keeps it eligible for a later version bump) and the +// extract-failure guard (stamping would mark parser work as done that +// never ran; leaving it stale retries on the next poll). +func (s *MediaScanner) storeManifestBaseline(ctx context.Context, libraryID pgtype.UUID, entry *scanEntry, manifest *sidecarManifest) { b, err := json.Marshal(manifest) if err != nil { fmt.Printf("Warning: failed to encode sidecar manifest for %s: %v\n", entry.FilePath, err) @@ -220,6 +262,22 @@ func (s *MediaScanner) storeManifest(ctx context.Context, libraryID pgtype.UUID, s.scanStateUpsert(libraryID, entry) } +// evBudgetTake consumes one slot of the per-scan ev catch-up budget. +// Atomic: the poll scan walks sequentially, but the watcher's +// scanDirectory pass can overlap a poll scan (only scanDirectory holds +// scanMutex), so the budget is shared state. +func (s *MediaScanner) evBudgetTake() bool { + for { + cur := s.evBudgetRemaining.Load() + if cur <= 0 { + return false + } + if s.evBudgetRemaining.CompareAndSwap(cur, cur-1) { + return true + } + } +} + // detectSidecarChange fingerprints the book's sidecar directory and // reconciles against the stored manifest. Returns true when the item's // metadata must be re-extracted (a sidecar's CONTENT changed, or @@ -242,16 +300,36 @@ func (s *MediaScanner) detectSidecarChangeWithRepresentative(ctx context.Context computed := statSidecarManifest(representative, dir) stored := decodeSidecarManifest(entry.Manifest) + // Extractor-version gate (faceted plan Phase 10): a manifest stamped + // by an older parser re-extracts even when every sidecar byte is + // unchanged — the fingerprint says "disk same", ev says "parser + // newer". Budget-bounded: over budget → fall through, the stale ev + // stays stored, a later poll resumes. Checked FIRST so books with + // no sidecars (nothing to reconcile) are still caught up. + if stored != nil && manifestEv(stored) != s.GetExtractorVersion() && s.evBudgetTake() { + return true + } + if len(computed.Entries) == 0 { if stored != nil && len(stored.Entries) > 0 { return true // every sidecar was removed — fall back to embedded } - return false // never had sidecars: nothing to detect or store + if stored == nil { + // Never had sidecars: nothing to fingerprint, but the book + // still needs an ev home so future parser bumps see it. A + // first-sighting store follows no extraction → baseline + // (ev unset → eligible for the next bump). + s.storeManifestBaseline(ctx, libraryID, entry, &sidecarManifest{Version: sidecarManifestVersion}) + } + return false } if stored == nil { - // Deploy-day backfill: hash + store WITHOUT re-extraction. - s.storeManifest(ctx, libraryID, entry, hashSidecarManifest(representative, dir, computed)) + // Deploy-day backfill: hash + store WITHOUT re-extraction. The + // row was extracted by an unknown parser version — the stamp is + // deliberately OMITTED (ev unset → "v1-or-unknown") so a later + // scan_extractor_version bump still catches it. + s.storeManifestBaseline(ctx, libraryID, entry, hashSidecarManifest(representative, dir, computed)) return false } @@ -260,7 +338,11 @@ func (s *MediaScanner) detectSidecarChangeWithRepresentative(ctx context.Context return true } if fresh != nil { - s.storeManifest(ctx, libraryID, entry, fresh) + // Repoint reconciliation refreshed stat pairs only — the row was + // NOT re-extracted, so the manifest must keep the row's stored + // ev (a stamp here would claim parser work that never ran; + // possible when the ev budget was exhausted this pass). + s.storeManifestBaseline(ctx, libraryID, entry, fresh) } return false } diff --git a/internal/services/media_scanner_settings_test.go b/internal/services/media_scanner_settings_test.go index 16f29eb5..48537f0c 100644 --- a/internal/services/media_scanner_settings_test.go +++ b/internal/services/media_scanner_settings_test.go @@ -30,3 +30,37 @@ func TestMediaScanner_GetAutoScanEnabled(t *testing.T) { } }) } + +func TestMediaScanner_GetExtractorVersion(t *testing.T) { + t.Run("nil db returns default 1", func(t *testing.T) { + scanner := &MediaScanner{ + db: nil, + settingsCache: NewSettingsCache(30 * time.Second), + } + if v := scanner.GetExtractorVersion(); v != 1 { + t.Errorf("expected 1, got %d", v) + } + }) + + t.Run("cached value wins (scanner read-through path)", func(t *testing.T) { + scanner := &MediaScanner{ + db: nil, + settingsCache: NewSettingsCache(30 * time.Second), + } + scanner.settingsCache.Set("scan_extractor_version", "3") + if v := scanner.GetExtractorVersion(); v != 3 { + t.Errorf("expected 3, got %d", v) + } + }) + + t.Run("cached garbage rides the default", func(t *testing.T) { + scanner := &MediaScanner{ + db: nil, + settingsCache: NewSettingsCache(30 * time.Second), + } + scanner.settingsCache.Set("scan_extractor_version", "zero") + if v := scanner.GetExtractorVersion(); v != 1 { + t.Errorf("expected 1, got %d", v) + } + }) +}