[kevinatlee]

Summary

Hardcover author works now use profile-specific edition evidence instead of treating the default edition's language as authoritative work-level language. This preserves the strict-profile fix from #2246 / #2241 while preventing translated defaults from hiding legitimate English works. Failed evidence lookups leave the existing language pipeline authoritative, and catalogue reconciliation remains conservative. Closes #2753.

#2247 exposed the regression safely: its preview offered a metadata-only Audible row for removal, but no files were deleted and its apply-time safeguards remain unchanged.

Reproduction and root cause

With Penelope Douglas as Hardcover primary and an English-only profile (eng, unknown = fail), 49 of 78 works were skipped, including Corrupt (por), Hideaway (por), Kill Switch (por), Bully (Spanish), and Nightfall (Spanish).

An existing English Audible Corrupt row (B01CZ0WTEM) was offered by Reconcile catalogue as Rejected by the language filter.

The current path was:

1. Hardcover books has no work-level language.
2. toBook filled Book.Language from default_ebook_edition, then audio.
3. Author sync treated that scalar as definitive.
4. Reconciliation converted the mismatch into language_not_allowed deletion evidence.

A translated default edition therefore classified the whole work as Portuguese or Spanish.

Implementation notes

The primary Hardcover client performs one profile-specific batch query after the author catalogue is fetched. It filters editions by all known ISO 639-1, 639-2/B, and 639-2/T aliases and uses distinct_on: [book_id], returning at most one small row per work.

Evidence	Ingestion	Reconciliation
Allowed-language edition exists	Accept for filtering	Keep
Query completes, no allowed edition exists, and the default supplies known non-allowed evidence	Reject as non-allowed	May report language_not_allowed
Successful query is indeterminate	Fall through to #891 edition sampling, author-majority fallback, scalar language, then unknown-language behavior if still blank	Use the resolved scalar; keep a still-blank result as indeterminate
Evidence query fails	Discard the evidence map and use the existing language pipeline	Keep all affected works as indeterminate; failure is never deletion evidence

Evidence is filter-only. It does not rewrite Book.Language: the user-visible value remains the provider's preferred/default language or a user-locked value. This keeps filtering evidence separate from display metadata and preserves the Book.CanWrite lock protections from #2760 / #2773.

No schema or dependency changes were needed.

API and performance impact

• One additional Hardcover request per restricted-language author catalogue.
• Zero additional requests when the profile allows any language or on the single-work filter-exempt path.
• No per-book request fan-out.
• The request has an explicit GraphQL limit equal to the unique applicable work count, capped by the existing 500-work Hardcover author catalogue bound.
• An input beyond that bound is rejected before any request and falls back conservatively; incomplete evidence cannot manufacture definitive absence.
• The request goes through the existing Hardcover client, throttle, retry, authorization, and SSRF-guarded HTTP path.
• A 78-work regression test asserts exactly one request and limit: 78.

Reconciliation safety

Existing metadata-only Wanted, file-bearing, imported, excluded, ownership, status, apply-time recomputation, and no-filesystem-deletion safeguards are unchanged. Failed evidence lookups cannot become removal evidence. A successful indeterminate result only becomes language_not_allowed when the existing scalar pipeline independently resolves a known non-allowed language.

Checklist

• Commits signed off with git commit -s
• Tests added or updated
• Doc-update gate cleared (docs/User-Guide-Wiki.md)
• Added a changelog fragment under changelog.d/
• Wiki page updated for user-facing behavior
• Rebased onto current upstream main with fix(metadata): keep a locked language through Hardcover edition hydration (#2757) #2760 / fix(metadata): honour field locks on every Hardcover write path (#2767) #2773 / fix(importlist): scope the Hardcover list sync's create decision to the list owner #2774 lock and tenancy changes preserved

Test plan

Passed:

• go test ./internal/models ./internal/metadata/hardcover ./internal/metadata ./internal/bookhydrate -count=1
• Focused internal/api regressions for evidence failure, OpenLibrary edition-sampling for work-level language so the allowed_languages filter catches the OL tail #891 sampling, majority fallback, translated defaults, non-allowed evidence, unknown pass/fail, reconciliation failure/indeterminate behavior, and field locks
• Full tests for internal/api, internal/metadata, and internal/metadata/hardcover with focused coverage
• Changed decision functions at 100% statement coverage; every newly changed production statement executed
• go vet ./...
• golangci-lint run --timeout=5m ./internal/models ./internal/metadata/... ./internal/api (v2.11.4; 0 issues)
• Pinned govulncheck ./... (0 called vulnerabilities)
• gofmt -l .
• git diff --check

Environment / upstream Windows limitations:

• go test ./cmd/... ./internal/... passes all changed packages but retains unrelated Windows/environment failures in cmd/telemetry-server, internal/abs, and internal/importer (path separator, symlink/permission, timing/state, and filesystem semantics).
• Repository-wide golangci-lint run --timeout=5m reaches the unrelated Windows-only internal/importer/samedevice_windows.go: sameDevice is unused; the exact requested changed-package lint scope is clean.
• make is unavailable in this environment.
• Frontend checks were not run because no frontend files changed.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[vavallee]

Thanks for this, and for the detail in #2753. The bug is real and the architecture is the right one: an author catalogue really does need per work language evidence rather than trusting the default edition, and one batched request for it is the right cost. The alias table handling is the best part of the PR (more on that below). Two things block it though, and both make an author look empty rather than mislabelled.

1. A failed evidence lookup empties the whole catalogue under a strict profile.

GetAuthorWorkLanguageEvidence seeds every work as Indeterminate before issuing the query, then returns that fully seeded map alongside the error. The caller at internal/api/authors.go:1881-1886 assigns it regardless of the error and only logs a warning. authorWorkPassesLanguageFilter then takes the default: arm for every work, blanks book.Language, and returns !unknownFail.

A four work author, three eng and one blank, provider returning that map plus an error, profile eng with unknown = fail:

stored = map[]     // 0 of 4 works persisted

On main those three English works are kept. So a token expiry or a single 502 now reads as "this author has no books". Your TestFetchAuthorBooks_LanguageEvidenceFailureRemainsUnknown only covers a Portuguese work, which main skips too, which is why the suite reports this as correct.

Either return nil on error so the caller falls through to the existing behaviour, or have the caller ignore a map it did not successfully obtain.

2. The indeterminate arm discards the majority language fallback and the #891 edition sampling.

Ordering: the evidence query runs at authors.go:1881, FillMissingAuthorWorkLanguages at :1907, and applyAuthorMajorityLanguageFallback at :1925. Whatever those two resolve is then overwritten or blanked by authorWorkPassesLanguageFilter in the candidate loop at :2211.

I reproduced TestFetchAuthorBooks_MajorityLanguageFallbackRescuesUnresolvedWork against an evidence supplying provider, three allowed works and one indeterminate, unknown = fail, and the rescued work was dropped. That is the exact case the test was written to guard; it does not catch it because its stub does not implement the evidence interface.

Either run the evidence query after those two fills, or make the indeterminate arm fall through to the scalar language instead of blanking it.

3. Medium, and I could not verify this one. The new query is the only one in the client without an explicit limit (compare client.go:246, 356, 410, 493, 588). If Hasura applies a default row cap, works past the cap silently become NotAllowed, which is the rejection path rather than the safe one. A zero row response is also indistinguishable from a schema mismatch on code2, code3 or book.slug. Worth a limit plus treating a truncated response as indeterminate.

4. Medium. authors.go:3462 rewrites book.Language to the allowed edition's language, so a work shown as por becomes eng once any English edition exists. Defensible, but it is user visible and neither the PR body nor the wiki edit mentions it.

The alias table, which is the thing I most wanted to check, you got right. hardcoverLanguageName now falls back to models.NormalizeLanguageCode on a miss instead of returning the raw lowercased string, and that does strip subtags, so en-US becomes eng rather than en-us. I ran all forty hardcoverLanguageAliases entries through NormalizeLanguageCode and they agree exactly, which means the duplicate table is now provably redundant and a follow up can delete it outright. That is a nice result and it was not the point of the PR.

Also checked and fine: the central guard is pinned, one request per catalogue as claimed, and the distinct_on column ordering matches the order_by the way Hasura requires.

Tiny one: the changelog fragment has an em dash in it. House style here is no dashes in release notes. Several existing fragments have them too so this is drift rather than something you introduced.

Main has moved under you since you opened this, so a rebase is needed regardless: #2760, #2773 and #2774 all landed, and #2773 touches hardcoverLanguageName's neighbourhood in the same file.

Happy to re review as soon as 1 and 2 are addressed.

[kevinatlee]

Thanks, these were good catches.

Addressed both blockers and the two medium items:

• failed evidence queries are now discarded as evidence, so the existing scalar/OpenLibrary edition-sampling for work-level language so the allowed_languages filter catches the OL tail #891/majority language pipeline remains authoritative on provider failure;
• indeterminate evidence now falls through to already-resolved scalar language instead of blanking it;
• the batched evidence query now carries an explicit limit equal to the unique requested work count (bounded at 500), and over-bound/query-failure paths remain conservative;
• evidence is now filter-only and does not rewrite the user-visible Book.Language; the provider preferred/default value and user locks remain intact after rebasing onto current main;
• cleaned up the changelog fragment style.

Added regressions for the four-work strict-profile failure, evidence-capable majority fallback, #891 edition sampling, indeterminate scalar fallback including unknown pass/fail, explicit bounded query behavior, and reconciliation failure/indeterminate safety. The translated-default/English-edition, Corrupt, and 78-work/one-request regressions remain covered.

The rebase incorporates #2760/#2773/#2774 without dropping their language-lock changes. Focused tests, coverage, vet, the exact requested v2.11.4 lint scope, and the pinned vulnerability scan pass; CI is running on 1bb2a7fb.

Ready for re-review.

[vavallee]

Both blockers fixed, and the way you fixed the second one is better than either option I suggested. Making evidence filter only, so it can rescue a work but never rewrite or blank a language, means a query that silently matches nothing now degrades to exactly what main does rather than to something worse. That also closes my point 4 for free.

What I checked, merged onto current main (it merges clean, builds, vets, and `./internal/api` plus `./internal/metadata/...` pass):

• Blocker 2. Putting the old `default:` arm back fails four tests, including `TestFetchAuthorBooks_MajorityLanguageFallbackRescuesUnresolvedWork`, which is the one that couldn't see it before. That's the evidence I wanted.
• Blocker 1. Returning the seeded map on error fails `TestGetAuthorWorkLanguageEvidence_FailureReturnsNoEvidence`, so the client contract is pinned. Non blocking gap: nothing at the API layer drives a provider that returns a map and an error, so the caller's `languageEvidence = nil` is untested. With both halves reverted `./internal/api` stays green. A stub doing that under unknown = fail with three English works and one blank would pin the exact case from my review. Happy to take it as a follow up.
• The limit. `limit` equal to the unique work count with `distinct_on: [book_id]` is exactly one row per work, so a truncated response can't happen. Nice.