fix(metadata): manual author refresh fetches past the 24 hour cache (#2601)

[vavallee]

Summary

Refresh metadata on an author read the profile and the catalogue through the metadata aggregator's 24 hour cache, so a bio, photo or new book added upstream could stay hidden for up to a day after the user explicitly asked for it. The single author Refresh now asks the providers for current data, writes an answer back into the cache only when it is whole, and the author page waits for the background sync to finish before showing the result. Ordinary reads and background work keep their cache policy. Closes #2601.

Confirmed call path (current main)

Step	Location
POST /author/{id}/refresh	internal/api/authors.go Refresh (was :1491) launches fetchAuthorBooksAsync with discovery: true
Profile	fetchAuthorBooks calls refreshAuthorProfile (was :1846), which calls h.meta.GetAuthor (was :1658)
Catalogue	fetchAuthorBooks calls h.meta.GetAuthorWorksForAuthor (was :1858)
Audible	fetchAuthorBooks calls h.meta.GetAuthorAudiobooks for audiobook and both setups (was :1885)
Profile cache	internal/metadata/aggregator.go GetAuthor returned the author: entry before calling the provider (was :740)
Catalogue cache	aggregator_author_works.go GetAuthorWorksForAuthor returned the merged authorworks-author: entry (was :200), and below it rawPrimaryAuthorWorks returned its own authorworks-raw: entry (was :267)
TTL	NewAggregator builds the cache with newTTLCache(24 * time.Hour) (:46)

The raw works layer matters: bypassing only the merged entry would have re-merged a day old primary list with a fresh supplement.

Implementation notes

• metadata.WithCacheBypass(ctx) / metadata.CacheBypassed(ctx) in internal/metadata/cache_bypass.go. No context key pattern existed in internal/metadata, so this uses the plain unexported struct key the rest of the codebase uses (origBodyCtxKey in internal/api/maxbody.go).
• Honoured by GetAuthor, GetAuthorWorks, GetAuthorWorksUnenriched, GetAuthorWorksForAuthor and GetAuthorAudiobooks. The works lookups consume the flag before nested calls, so per work cover enrichment, editions, book lookups and the ISBN canonicalisation path (aggregator_canonical.go, which calls rawPrimaryAuthorWorks directly) keep their cache, and a refresh of a prolific author does not repeat the ABS import stalls indefinitely on a large folder-backed item (no timeout, no error) and auto-resumes into the same stall #2578 enrichment fan out on every click.
• A refresh does not replace the cache with an answer the aggregator can tell is failed or short. What it can tell depends on the provider (see the note under the table):

Refresh outcome	Cache	What the sync gets
Primary works answer intact	Raw works replaced; enriched authorworks: entry dropped	Fresh works
Primary works answer cut short by a failed call (OpenLibrary later works page, or its search call)	Kept	Cached works plus any new works the short answer carried; cached copy wins for works in both
Primary works provider fails, works cached	Kept	Cached works
Primary works provider fails, nothing cached	Nothing written	The error, as before
Any primary answers an empty works list, works cached (DNB answers a failed author record lookup this way)	Kept	Cached works
Configured supplement (Hardcover) fails, merged catalogue cached	Kept	Cached merged catalogue, since Hardcover drives the compilation prune
Audible fails, list cached	Kept	Cached Audible list
GetAuthor fails	Kept	The error; the stored profile stays

• "Intact" comes from a new openlibrary.Client.GetAuthorWorksForRefresh, which shares its body with GetAuthorWorksSnapshot and reports whether any upstream call failed. It cannot be the snapshot's complete flag: that is also false for any author with more than 200 works in the search index (the search call is capped at 200), so gating the write back on it would stop the refresh working for exactly the prolific authors. Providers without the method cannot report a short answer, so a non empty answer from them gets the trust a cache miss gives it; a list that lost some works to a fault the provider swallows can still replace the cache. An empty one is treated as short while works are cached.
• catalogueSyncOptions.refreshFromProvider carries the intent from the handler into the async sync; fetchAuthorBooks applies WithCacheBypass to the profile, catalogue and Audible lookups only.
• The handler counts running catalogue syncs per author (authorSyncsRunning in internal/api/author_sync_summary.go). Refresh claims the author before answering and returns 409 a refresh for this author is already running while any sync for it runs, matching Refresh all and the imports. GET /author/{id} reports syncInProgress.
• AuthorDetailPage no longer refetches the moment the 202 lands. It polls GET /author/{id} every 2 seconds until syncInProgress clears, for up to 60 seconds per wait, then bumps a reload key so the page's load effect reloads the author and book list with the filters in force at that moment. The button stays on "Refreshing…" meanwhile; moving to another author stops the poll.
• A 409 means some sync of the author is running, and the scheduled, bulk, Refresh all and add syncs all read the cache. The page waits for it, then posts Refresh once more and waits for that one. A second 409 stops there and shows the result.
• A later request deduplication layer (perf(metadata): cache provider searches and deduplicate requests #2615) can tell a bypassed call apart with metadata.CacheBypassed(ctx), which the entry points check before consuming the flag.

Which entry points bypass

Entry point	Bypass	Why
Author page Refresh metadata (POST /author/{id}/refresh)	Yes	One author, explicit user intent
Bulk Refresh on selected authors (BulkHandler.fanOutRefreshes)	No	Fans out concurrently (bulkSearchConcurrency) over an arbitrary selection
Refresh all metadata (POST /authors/refresh-all)	No	Every author in the library; the cache is what keeps a repeat run cheap
Scheduled refresh (Scheduler.refreshMetadata)	No	Background work, every monitored author
Relink upstream, Add author, Add book single work fallback	No	Not a refresh; relink and add resolve an identity the cache has usually not seen
Book level	n/a	There is no book refresh; Rebind already uses GetBookFromProvider, which never read the cache

I kept bulk on the cache rather than bounding it. A bounded bypass would need a new knob and still hit providers harder than today on every repeat run; the per author button covers the case the issue describes. Worth noting: the cache is in memory, so the first Refresh all after a restart is already a cold run.

Rate limits and quotas

The provider clients sit below the aggregator cache, so bypassed calls go through the same guards as a cache miss: Hardcover's shared adaptive throttle and bounded retries (internal/metadata/hardcover/throttle.go) and OpenLibrary's getJSON retry with Retry-After backoff (internal/metadata/openlibrary/client.go). There is no Hardcover daily quota in the codebase to check; the only per day quota is Google Books, which is not on the author refresh path. The 409 guard caps a single author at one refresh sync at a time.

Review fixes

#	Finding	Fix
1	A partial works answer overwrote a complete cached catalogue for a day	refreshPrimaryAuthorWorks caches only an intact answer; otherwise cached works plus new ones
2	A failed Hardcover supplement handed the sync an unpruned list	GetAuthorWorksForAuthor falls back to the cached merged catalogue
3	Audible stayed cached	GetAuthorAudiobooks honours the bypass; the handler passes metaCtx
4	The page showed the state from before the click	Poll syncInProgress, then reload
5	Five clicks started five concurrent syncs	409 while a sync for the author runs
6	Stale enriched authorworks: entry shadowed the refreshed raw works	An intact raw refresh drops it
7	A Refresh during a background sync got the 409, waited, and showed that sync's cached result	On 409 the page waits, then posts Refresh once more; a second 409 does not loop
8	The reload after the wait used the showExcluded captured at click time	The reload goes through the load effect via a reload key
9	Passing ctx instead of metaCtx to GetAuthorAudiobooks left every api test green	New api test with a fake Audible catalogue (Aggregator.WithAudibleCatalogue)
10	"Never worse off" held only for OpenLibrary; DNB can answer an empty list as success	An empty answer over a warm cache keeps the cached works; doc comment, wiki and changelog say what each provider guarantees
11	No test pinned the running sync mark being released on early exits	Counter tests for a works error, a recovered panic, the Calibre relink early return and a jobs group refusal

Checklist

• Tests added or updated
• Doc-update gate cleared: docs/User-Guide-Wiki.md describes what Refresh fetches, the wait, the 409 and the fallbacks; docs/API.md documents the 409 and syncInProgress; godoc on WithCacheBypass and GetAuthorWorksForRefresh
• Changelog fragment changelog.d/2601-refresh-bypasses-cache.md

Test plan

• go build ./... && go vet ./...
• go test ./internal/metadata/... ./internal/api ./internal/scheduler/... -count=1
• go test -race -count=3 on the new metadata, OpenLibrary and API tests
• golangci-lint run (v2.11.4) on internal/metadata/..., internal/api/..., internal/models/...: 0 issues
• cd web && npm run lint (0 errors; no warnings in touched files), npm run typecheck, npx vitest run (929 passed), npm run build
• gofmt -l internal/metadata internal/api prints nothing

New tests:

Test	Pins
TestGetAuthor_CacheBypassRefetchesAndRefreshesCache	Warm cache, bypassed read reaches the provider, next ordinary read serves the fresh value
TestGetAuthor_CacheBypassErrorKeepsCachedEntry	A failed bypassed call keeps the cached author
TestGetAuthorWorksForAuthor_CacheBypassRefetchesAndRefreshesCache	Both the merged and the raw works layers are refetched, supplement too; ordinary reads before stay cached, after serve the refreshed catalogue
TestCacheBypass_LeavesBookCacheAlone	The bypass does not leak into the book cache
TestGetAuthorWorksForAuthor_CacheBypassPartialAnswerKeepsCachedCatalogue	A short answer keeps the complete cache for merged and raw readers, the sync still sees the new work, an intact answer then replaces the cache
TestGetAuthorWorksForAuthor_CacheBypassProviderErrorServesCachedCatalogue	A provider error with works cached gives the sync the cached works; with a cold cache it returns the error
TestGetAuthorWorksForAuthor_CacheBypassSupplementFailureServesCachedCatalogue	Hardcover failing under the bypass hands the sync the pruned cached catalogue
TestGetAuthorWorksUnenriched_ServesRefreshedRawOverStaleEnriched	After a refresh, unenriched and enriched readers see the refreshed works
TestGetAuthorAudiobooks_CacheBypassRefetchesAndKeepsCacheOnError	Audible is refetched under the bypass, written back, and a failure returns the cached list
TestGetAuthorWorksForRefresh_HTTP_IntactOnlyWhenNoCallFailed	OpenLibrary: a failed later page or search call is not intact; a search capped at 200 is intact although the snapshot calls it incomplete
TestAuthorRefresh_ManualRefreshBypassesMetadataCache	Bulk path (RefreshAuthorBooks) stays on the cache; POST /author/{id}/refresh reaches the provider with the bypass, persists the new bio and book, and writes the fresh profile back
TestAuthorRefresh_RefusesSecondRefreshWhileSyncRuns	Second click while a sync runs gets 409 and never reaches the provider; syncInProgress is set while it runs and clears after; a later click starts a sync
TestRefreshMetadata_KeepsMetadataCache	The scheduled refresh stays on the cache and never sets the bypass
AuthorDetailPage: manual refresh (3 vitest cases)	The page waits for the sync and then shows the new book and bio; gives up after 60 seconds; waits on a 409 instead of showing an error
TestGetAuthorWorksForAuthor_CacheBypassEmptyAnswerKeepsCachedWorks	A DNB like primary answering an empty list under the bypass with a warm cache: the cached works go to the sync and stay cached; a cold cache still returns the empty answer
TestAuthorRefresh_ManualRefreshBypassesAudibleCache	Audiobook default: the bulk and Refresh all sync keeps the cached Audible list, the manual Refresh reaches Audible with the bypass and writes the list back. The scheduled refresh only rereads profiles and never asks Audible
TestAuthorRefresh_RunningMarkReleasedOnEarlyExit, TestAuthorRefresh_RunningMarkReleasedWhenJobsRefuse	The running sync mark is released on a works error, a recovered panic, the Calibre relink early return and a jobs group refusal
AuthorDetailPage: manual refresh (3 more vitest cases)	A 409 then a second POST whose result is shown; a second 409 does not loop; switching to Excluded during the wait keeps the excluded rows

Fail before (first commit's tests against the unfixed aggregator and handler):

--- FAIL: TestGetAuthor_CacheBypassRefetchesAndRefreshesCache
    bypassed read = "old bio", want the provider's "new bio": it was served from the cache
--- FAIL: TestGetAuthor_CacheBypassErrorKeepsCachedEntry
    bypassed GetAuthor swallowed the provider error
--- FAIL: TestGetAuthorWorksForAuthor_CacheBypassRefetchesAndRefreshesCache
    primary works calls = 1, want 2: the bypass stopped at the merged entry and reused the raw works cache
--- FAIL: TestAuthorRefresh_ManualRefreshBypassesMetadataCache (5.13s)
    manual refresh never reached the provider: the 24 hour metadata cache answered it (#2601)

Fail before (review fix tests against the first commit):

--- FAIL: TestGetAuthorWorksForAuthor_CacheBypassPartialAnswerKeepsCachedCatalogue
    bypassed catalogue from a partial answer = [Ancillary Justice Translation State], want the cached three plus the new Translation State
--- FAIL: TestGetAuthorWorksForAuthor_CacheBypassProviderErrorServesCachedCatalogue
    bypassed GetAuthorWorksForAuthor with a cached catalogue failed the sync: 503 from upstream
--- FAIL: TestGetAuthorWorksForAuthor_CacheBypassSupplementFailureServesCachedCatalogue
    a failed supplement under the bypass hands the sync [Ancillary Justice Moon Harvest] instead of the pruned cached [Ancillary Justice]
--- FAIL: TestGetAuthorWorksUnenriched_ServesRefreshedRawOverStaleEnriched
    GetAuthorWorksUnenriched after a refresh = [Ancillary Justice], want the 2 refreshed works, not the stale enriched entry
--- FAIL: TestGetAuthorAudiobooks_CacheBypassRefetchesAndKeepsCacheOnError
    bypassed read = [Ancillary Justice], want the new Audible release: it was served from the cache
--- FAIL: TestAuthorRefresh_RefusesSecondRefreshWhileSyncRuns
    second Refresh while the first sync runs = 202, want 409: a second concurrent sync was started
 × waits for the background sync to finish and then shows its result
   Unable to find an accessible element with the role "button" and name "Refreshing…"
 × gives up waiting after a minute and shows whatever the server has
   Unable to find an accessible element with the role "button" and name "Refreshing…"
 × waits on a sync that was already running instead of reporting the 409
   Unable to find an accessible element with the role "heading" and name "Translation State"

Fail before and mutation evidence (second review round):

--- FAIL: TestGetAuthorWorksForAuthor_CacheBypassEmptyAnswerKeepsCachedWorks
    bypassed catalogue from an empty answer = [], want the cached 2 works handed to the sync
--- FAIL: TestAuthorRefresh_ManualRefreshBypassesAudibleCache   (GetAuthorAudiobooks(ctx, ...) mutation)
    after the manual refresh: 1 Audible calls (0 bypassed), want 2 with 1 bypassed: the cached Audible list answered the refresh
 × asks again once a background sync that answered 409 ends, and shows that refresh
   expected "vi.fn()" to be called 2 times, but got 1 times
 × stops after a second 409 instead of looping
   expected "vi.fn()" to be called 2 times, but got 1 times
 × reloads with the status filter picked while it waited
   Unable to find an element with the text: Provenance

The running sync mark tests pass on both sides: the release paths were already right, and the tests keep them that way.

TestGetAuthorWorksForRefresh_HTTP_IntactOnlyWhenNoCallFailed covers a new method, so it has no fail before run; its "search capped by design" case asserts the precondition that the existing snapshot reports that catalogue as incomplete, which is why the snapshot flag could not be used. TestCacheBypass_LeavesBookCacheAlone and TestRefreshMetadata_KeepsMetadataCache are guards and pass on both sides.

Follow-ups (not in this PR)

• Caching and request deduplication #2594 (magrhino's search caching work) will add more aggregator caches. If any of them sits on the author refresh path it should honour WithCacheBypass the same way.

🤖 Generated with Claude Code

https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9

[codecov]

Codecov Report

❌ Patch coverage is 95.59748% with 7 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/metadata/aggregator.go	50.00%	3 Missing ⚠️
internal/metadata/aggregator_author_works.go	95.89%	2 Missing and 1 partial ⚠️
internal/api/author_sync_summary.go	96.15%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[github-actions]

Solid fix, no blocking issues. A few things to consider:

GetAuthorAudiobooks reads but doesn't consume the bypass (aggregator_enrichment.go:194–209).
GetAuthorWorks* all call consumeCacheBypass to strip the flag before nested calls; GetAuthorAudiobooks does not. That's intentional and safe today since the Audible call has no nested aggregator-cache lookups. Just worth noting that if a nested cached lookup is ever added here, the bypass will need to be consumed first, or it will leak into those calls the same way the GetAuthorWorks* entry points prevent.

waitForSync leaks a dangling setTimeout past unmount (AuthorDetailPage.tsx:300–312).
The session.active guard prevents any state write, so there's no observable effect. But the timer still fires 2 s after unmount and the Promise resolves into a no-op. Clean cancellation would be AbortController/clearTimeout. Low priority.

emptyOverCache logic relies on an implicit contract (aggregator_author_works.go:334–338).
An empty answer from a provider without authorWorksRefreshProvider (DNB, etc.) gets intact=true from primaryAuthorWorksForRefresh, then the empty-over-cache guard fires. That's the intended path and the doc comment on authorWorksRefreshProvider explains it, but the dependency between "no GetAuthorWorksForRefresh" → intact=true → "empty-over-cache guard runs" is implicit. A short comment at the guard site pointing back to authorWorksRefreshProvider would make it easier for a future provider addition to get this right.

None of these block the change. The concurrency model (authorSyncsRunning count vs. set, syncClaimed, and the jobs.Go refusal path), the context-key scoping (consumed at entry points, invisible to per-book enrichment), the intact semantics between the OL complete vs. interrupted distinction, and the frontend session.active + reloadKey pattern all look correct. Tests cover the gaps well.

— 🤖 Bindery triage bot (automated). Reply to correct me; a human will see it.