diff --git a/README.md b/README.md index 90a567787..52248a7b8 100644 --- a/README.md +++ b/README.md @@ -120,7 +120,7 @@ Clean-room Go rewrite, modern React UI, MIT-licensed, actively developed. - Library scan matching in five passes: exact ASIN, then a backwards `Author - Title` filename whose folder author the catalogue already knows, then the filename's title, then the book folder's title, and finally series name plus position. Every title pass is one fuzzy comparison that has to agree on the author as well. Honours librarian sort-suffix form (`Title, The`) and series-annotated filenames (`[Mistborn, Book 1]`). - **Library adoption**: whatever the scan could not match waits on the Import page as one row per book (an audiobook folder or disc set is one row), with suggested matches (one click Confirm only for a strong match), one "Add author" decision per missing author, and Undo. Files are registered where they are, never moved. - Author aliases (`RR Haywood` / `R.R. Haywood` / `R R Haywood` merge into one canonical row), and metadata re-bind to correct a wrong match without delete-and-re-add. On a book’s **File → Re-bind** dialog, search configured metadata providers and select an OpenLibrary work or Hardcover book, or enter an exact provider ID and press Enter. Results already in the library are marked and link to the existing book; records belonging to another library book cannot be selected. Each result’s **Links** menu opens its upstream page when available; OpenLibrary editions can be viewed but require a work ID for re-binding. Re-binding updates metadata and series without moving files; author mismatches require confirmation. -- Explicit author-catalogue reconciliation with a selectable preview: remove chosen stale metadata-only Wanted rows after changing provider or metadata profile while always protecting imported books and every row with a tracked file. +- Explicit author-catalogue reconciliation with a selectable preview: remove chosen stale metadata-only Wanted rows after changing provider or metadata profile, review rows kept because evidence is incomplete, and always protect imported books and every row with a tracked file. - Manual metadata editing with field locks: edit a book's title, description, genres, language or release date and the edit survives every refresh. **Fix match** reassigns a file that was attached to the wrong book. See [docs/Metadata-Editing-Wiki.md](docs/Metadata-Editing-Wiki.md). **Search & downloads** diff --git a/changelog.d/2892-reconciliation-indeterminate-rows.md b/changelog.d/2892-reconciliation-indeterminate-rows.md new file mode 100644 index 000000000..4c486daaf --- /dev/null +++ b/changelog.d/2892-reconciliation-indeterminate-rows.md @@ -0,0 +1,2 @@ +### Added +- **Reconcile catalogue now lists the rows it kept for lack of evidence** (#2892): rows kept because provider or profile evidence was incomplete used to show up only as a count. Each one is now listed with its title, provider and the reason, such as language could not be confirmed, so you can review them yourself. They still can never be removed automatically. Thanks kevinatlee. diff --git a/docs/API.md b/docs/API.md index 9e33bbecd..3353bfe50 100644 --- a/docs/API.md +++ b/docs/API.md @@ -140,8 +140,14 @@ The field is absent when the providers match or no primary is configured. Catalogue reconciliation is deliberately separate from refresh. The GET route queries the current primary provider without using its cached author catalogue -and returns `candidates`, a reason-count summary, protection counts, and -`providerComplete`. A partial provider result never treats absence as a reason +and returns `candidates`, `indeterminateRows`, a reason-count summary, +protection counts, and `providerComplete`. Each `indeterminateRows` entry has a +`bookId`, `title`, `metadataProvider`, and display-only `reason`. These rows are +informational: they are kept because evidence is incomplete and are never +deletion candidates. Stable indeterminate reasons are `language_unknown`, +`language_evidence_lookup_failed`, `edition_evidence_unavailable`, +`partial_catalogue`, and `unmatched_cross_provider`. A partial provider result +never treats absence as a reason to remove a row. A complete result can make an absent same-provider row a candidate, but an unmatched row from another provider is kept as indeterminate: provider migration alone is not deletion evidence. Explicit profile rejections diff --git a/docs/User-Guide-Wiki.md b/docs/User-Guide-Wiki.md index db05ec5c6..c57758d79 100644 --- a/docs/User-Guide-Wiki.md +++ b/docs/User-Guide-Wiki.md @@ -834,7 +834,10 @@ an absent row from that same provider may be actionable. A row from a different provider is kept as indeterminate when Bindery cannot correlate it to the current catalogue; changing providers alone is not evidence that a work is obsolete. A correlated work that the metadata profile explicitly rejects can -still be removed for that rejection reason. +still be removed for that rejection reason. The preview lists each +indeterminate row separately with its provider and the incomplete-evidence +reason so it can be reviewed manually; these informational rows have no +selection control and cannot be sent for removal. **Duplicate titles.** The same book often reaches the catalogue twice under slightly different titles — "The Martian" and "Martian", "Dune" and "Dune diff --git a/internal/api/catalogue_reconciliation.go b/internal/api/catalogue_reconciliation.go index 8b19d7ea2..70ce6f332 100644 --- a/internal/api/catalogue_reconciliation.go +++ b/internal/api/catalogue_reconciliation.go @@ -31,6 +31,20 @@ const ( reconcileReasonCatalogueFilter = "catalogue_filter" ) +// CatalogueReconciliationIndeterminateReason identifies why a local row was +// kept because the available provider or profile evidence was not conclusive. +// These reasons are informational only and never make a row eligible for +// deletion. +type CatalogueReconciliationIndeterminateReason string + +const ( + reconcileIndeterminateReasonLanguageUnknown CatalogueReconciliationIndeterminateReason = "language_unknown" + reconcileIndeterminateReasonLanguageLookupFailed CatalogueReconciliationIndeterminateReason = "language_evidence_lookup_failed" + reconcileIndeterminateReasonEditionUnavailable CatalogueReconciliationIndeterminateReason = "edition_evidence_unavailable" + reconcileIndeterminateReasonPartialCatalogue CatalogueReconciliationIndeterminateReason = "partial_catalogue" + reconcileIndeterminateReasonUnmatchedCrossProvider CatalogueReconciliationIndeterminateReason = "unmatched_cross_provider" +) + // CatalogueReconciliationCandidate is a local, metadata-only Wanted row that // the current primary-provider catalogue and metadata profile no longer // accept. It contains no file operation: reconciliation deletes database rows @@ -42,6 +56,17 @@ type CatalogueReconciliationCandidate struct { Reason string `json:"reason"` } +// CatalogueReconciliationIndeterminateRow is a local row that reconciliation +// kept because provider or profile evidence was incomplete. It is deliberately +// separate from CatalogueReconciliationCandidate so it cannot be mistaken for +// a safe deletion target. +type CatalogueReconciliationIndeterminateRow struct { + BookID int64 `json:"bookId"` + Title string `json:"title"` + MetadataProvider string `json:"metadataProvider"` + Reason CatalogueReconciliationIndeterminateReason `json:"reason"` +} + type CatalogueReconciliationSummary struct { Total int `json:"total"` Candidates int `json:"candidates"` @@ -66,15 +91,16 @@ type CatalogueReconciliationApplySummary struct { // only a partial author catalogue; in that case absence is never used as a // deletion reason. type CatalogueReconciliation struct { - AuthorID int64 `json:"authorId"` - AuthorName string `json:"authorName"` - Provider string `json:"provider"` - ProviderComplete bool `json:"providerComplete"` - ProfileName string `json:"profileName"` - Warning string `json:"warning,omitempty"` - Candidates []CatalogueReconciliationCandidate `json:"candidates"` - Summary CatalogueReconciliationSummary `json:"summary"` - Applied *CatalogueReconciliationApplySummary `json:"applied,omitempty"` + AuthorID int64 `json:"authorId"` + AuthorName string `json:"authorName"` + Provider string `json:"provider"` + ProviderComplete bool `json:"providerComplete"` + ProfileName string `json:"profileName"` + Warning string `json:"warning,omitempty"` + Candidates []CatalogueReconciliationCandidate `json:"candidates"` + IndeterminateRows []CatalogueReconciliationIndeterminateRow `json:"indeterminateRows"` + Summary CatalogueReconciliationSummary `json:"summary"` + Applied *CatalogueReconciliationApplySummary `json:"applied,omitempty"` } type applyCatalogueReconciliationRequest struct { @@ -261,24 +287,24 @@ func (h *AuthorHandler) buildCatalogueReconciliation(ctx context.Context, author } acceptedIDs := make(map[string]struct{}, len(works)) - indeterminateIDs := make(map[string]struct{}) + indeterminateIDs := make(map[string]CatalogueReconciliationIndeterminateReason) rejectedIDs := make(map[string]string, len(works)) acceptedTitles := indexer.NewTitleIndex[struct{}]() - indeterminateTitles := indexer.NewTitleIndex[struct{}]() + indeterminateTitles := indexer.NewTitleIndex[CatalogueReconciliationIndeterminateReason]() rejectedTitles := make(map[string]string) normalizedAuthor := strings.ToLower(strings.TrimSpace(author.Name)) for _, work := range works { - reason, indeterminate := reconciliationRejectReason(work, normalizedAuthor, profile, editions[work.ForeignID], languageEvidence, languageEvidenceFailed) + reason, indeterminateReason := reconciliationRejectReason(work, normalizedAuthor, profile, editions[work.ForeignID], languageEvidence, languageEvidenceFailed) if reason == "" { if strings.TrimSpace(work.ForeignID) != "" { acceptedIDs[work.ForeignID] = struct{}{} } acceptedTitles.Add(work.Title, struct{}{}) - if indeterminate { + if indeterminateReason != "" { if strings.TrimSpace(work.ForeignID) != "" { - indeterminateIDs[work.ForeignID] = struct{}{} + indeterminateIDs[work.ForeignID] = indeterminateReason } - indeterminateTitles.Add(work.Title, struct{}{}) + indeterminateTitles.Add(work.Title, indeterminateReason) } continue } @@ -299,12 +325,13 @@ func (h *AuthorHandler) buildCatalogueReconciliation(ctx context.Context, author return CatalogueReconciliation{}, fmt.Errorf("list identifiers for author %d: %w", author.ID, err) } result := CatalogueReconciliation{ - AuthorID: author.ID, - AuthorName: author.Name, - Provider: snapshot.Provider, - ProviderComplete: snapshot.Complete, - ProfileName: profile.name, - Candidates: []CatalogueReconciliationCandidate{}, + AuthorID: author.ID, + AuthorName: author.Name, + Provider: snapshot.Provider, + ProviderComplete: snapshot.Complete, + ProfileName: profile.name, + Candidates: []CatalogueReconciliationCandidate{}, + IndeterminateRows: []CatalogueReconciliationIndeterminateRow{}, Summary: CatalogueReconciliationSummary{ Total: len(localBooks), Reasons: map[string]int{}, @@ -354,15 +381,15 @@ func (h *AuthorHandler) buildCatalogueReconciliation(ctx context.Context, author } if anyReconciliationIDAccepted(ids, acceptedIDs) { result.Summary.Kept++ - if anyReconciliationIDAccepted(ids, indeterminateIDs) { - result.Summary.Indeterminate++ + if indeterminateReason := reconciliationIndeterminateReasonForIDs(ids, indeterminateIDs); indeterminateReason != "" { + result.IndeterminateRows = append(result.IndeterminateRows, catalogueReconciliationIndeterminateRow(book, indeterminateReason)) } continue } if _, accepted := acceptedTitles.Lookup(book.Title); accepted { result.Summary.Kept++ - if _, indeterminate := indeterminateTitles.Lookup(book.Title); indeterminate { - result.Summary.Indeterminate++ + if indeterminateReason, indeterminate := indeterminateTitles.Lookup(book.Title); indeterminate { + result.IndeterminateRows = append(result.IndeterminateRows, catalogueReconciliationIndeterminateRow(book, indeterminateReason)) } continue } @@ -376,12 +403,12 @@ func (h *AuthorHandler) buildCatalogueReconciliation(ctx context.Context, author if reason == "" { if !snapshot.Complete { result.Summary.Kept++ - result.Summary.Indeterminate++ + result.IndeterminateRows = append(result.IndeterminateRows, catalogueReconciliationIndeterminateRow(book, reconcileIndeterminateReasonPartialCatalogue)) continue } if bookProvider(book) != snapshot.Provider { result.Summary.Kept++ - result.Summary.Indeterminate++ + result.IndeterminateRows = append(result.IndeterminateRows, catalogueReconciliationIndeterminateRow(book, reconcileIndeterminateReasonUnmatchedCrossProvider)) continue } reason = reconcileReasonNotInCatalogue @@ -399,45 +426,58 @@ func (h *AuthorHandler) buildCatalogueReconciliation(ctx context.Context, author sort.Slice(result.Candidates, func(i, j int) bool { return strings.ToLower(result.Candidates[i].Title) < strings.ToLower(result.Candidates[j].Title) }) + sort.Slice(result.IndeterminateRows, func(i, j int) bool { + return strings.ToLower(result.IndeterminateRows[i].Title) < strings.ToLower(result.IndeterminateRows[j].Title) + }) result.Summary.Candidates = len(result.Candidates) + result.Summary.Indeterminate = len(result.IndeterminateRows) return result, nil } -func reconciliationRejectReason(work models.Book, normalizedAuthor string, profile reconciliationProfile, evidence editionEvidence, languageEvidence map[string]metadata.AuthorWorkLanguageEvidence, languageEvidenceFailed bool) (string, bool) { +func reconciliationRejectReason(work models.Book, normalizedAuthor string, profile reconciliationProfile, evidence editionEvidence, languageEvidence map[string]metadata.AuthorWorkLanguageEvidence, languageEvidenceFailed bool) (string, CatalogueReconciliationIndeterminateReason) { normalizedTitle := strings.ToLower(strings.TrimSpace(work.Title)) if normalizedTitle == "" || normalizedTitle == normalizedAuthor || work.IsCompilation || metadata.IsUnambiguousBundleTitle(work.Title) { - return reconcileReasonCatalogueFilter, false + return reconcileReasonCatalogueFilter, "" } if len(profile.allowedLangs) > 0 { if languageEvidenceFailed { - return "", true + return "", reconcileIndeterminateReasonLanguageLookupFailed } languageAllowed, indeterminate := authorWorkPassesLanguageFilter(&work, profile.allowedLangs, profile.unknownFail, languageEvidence) if indeterminate { - return "", true + return "", reconcileIndeterminateReasonLanguageUnknown } if !languageAllowed { - return reconcileReasonLanguage, false + return reconcileReasonLanguage, "" } } if profile.skipPartBooks && isPartBookTitle(work.Title) { - return reconcileReasonPartBook, false + return reconcileReasonPartBook, "" } if profile.skipMissingDate && work.ReleaseDate == nil { - return reconcileReasonMissingDate, false + return reconcileReasonMissingDate, "" } if profile.minPages > 0 || profile.skipMissingISBN { if !evidence.known { - return "", true + return "", reconcileIndeterminateReasonEditionUnavailable } if profile.skipMissingISBN && !anyEditionHasISBN(evidence.editions) { - return reconcileReasonISBN, false + return reconcileReasonISBN, "" } if profile.minPages > 0 && !passesMinPagesFilter(evidence.editions, profile.minPages) { - return reconcileReasonPages, false + return reconcileReasonPages, "" } } - return "", false + return "", "" +} + +func catalogueReconciliationIndeterminateRow(book *models.Book, reason CatalogueReconciliationIndeterminateReason) CatalogueReconciliationIndeterminateRow { + return CatalogueReconciliationIndeterminateRow{ + BookID: book.ID, + Title: book.Title, + MetadataProvider: bookProvider(book), + Reason: reason, + } } func anyReconciliationIDAccepted(ids []string, accepted map[string]struct{}) bool { @@ -449,6 +489,15 @@ func anyReconciliationIDAccepted(ids []string, accepted map[string]struct{}) boo return false } +func reconciliationIndeterminateReasonForIDs(ids []string, reasons map[string]CatalogueReconciliationIndeterminateReason) CatalogueReconciliationIndeterminateReason { + for _, id := range ids { + if reason := reasons[id]; reason != "" { + return reason + } + } + return "" +} + func reconciliationReasonForIDs(ids []string, rejected map[string]string) string { for _, id := range ids { if reason := rejected[id]; reason != "" { diff --git a/internal/api/catalogue_reconciliation_test.go b/internal/api/catalogue_reconciliation_test.go index 31a6ae318..f601a785a 100644 --- a/internal/api/catalogue_reconciliation_test.go +++ b/internal/api/catalogue_reconciliation_test.go @@ -115,6 +115,22 @@ func reconciliationRequest(method, target string, authorID int64, body []byte) * return req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx)) } +func assertIndeterminateRow(t *testing.T, got CatalogueReconciliation, book *models.Book, reason CatalogueReconciliationIndeterminateReason) { + t.Helper() + if got.Summary.Indeterminate != len(got.IndeterminateRows) { + t.Fatalf("summary indeterminate = %d, rows = %d", got.Summary.Indeterminate, len(got.IndeterminateRows)) + } + for _, row := range got.IndeterminateRows { + if row.BookID == book.ID { + if row.Title != book.Title || row.MetadataProvider != bookProvider(book) || row.Reason != reason { + t.Fatalf("indeterminate row = %+v, want book=%+v reason=%q", row, book, reason) + } + return + } + } + t.Fatalf("indeterminate rows = %+v, want book %d with reason %q", got.IndeterminateRows, book.ID, reason) +} + func TestPreviewCatalogueReconciliation_ReportsOnlySafeCandidates(t *testing.T) { provider := &stubMetaProvider{name: "hardcover", works: []models.Book{ {ForeignID: "hc:keep", Title: "Keep", Language: "eng", MetadataProvider: "hardcover"}, @@ -124,7 +140,7 @@ func TestPreviewCatalogueReconciliation_ReportsOnlySafeCandidates(t *testing.T) f := newReconciliationFixture(t, provider) keep := f.createBook(t, "hc:keep", "Keep", "hardcover", models.BookStatusWanted) spanish := f.createBook(t, "hc:spanish", "Spanish", "hardcover", models.BookStatusWanted) - f.createBook(t, "OL-stale", "Old OpenLibrary Work", "openlibrary", models.BookStatusWanted) + previousProvider := f.createBook(t, "OL-stale", "Old OpenLibrary Work", "openlibrary", models.BookStatusWanted) missing := f.createBook(t, "hc:missing", "Removed Upstream", "hardcover", models.BookStatusWanted) unknown := f.createBook(t, "hc:unknown", "Unknown", "hardcover", models.BookStatusWanted) imported := f.createBook(t, "hc:owned", "Owned", "hardcover", models.BookStatusImported) @@ -171,6 +187,8 @@ func TestPreviewCatalogueReconciliation_ReportsOnlySafeCandidates(t *testing.T) if got.Summary.Indeterminate != 2 { t.Errorf("indeterminate = %d, want unknown-language and unmatched previous-provider rows", got.Summary.Indeterminate) } + assertIndeterminateRow(t, got, unknown, reconcileIndeterminateReasonLanguageUnknown) + assertIndeterminateRow(t, got, previousProvider, reconcileIndeterminateReasonUnmatchedCrossProvider) if got.Summary.Kept != 3 { t.Errorf("kept = %d, want accepted, unknown-language, and unmatched previous-provider rows (%q and %q included)", got.Summary.Kept, keep.Title, unknown.Title) } @@ -188,8 +206,9 @@ func TestPreviewCatalogueReconciliation_UnmatchedCrossProviderRowsAreIndetermina "Shallow River", "Where’s Molly", } + localBooks := make([]*models.Book, 0, len(titles)) for i, title := range titles { - f.createBook(t, fmt.Sprintf("audible:hdc-%d", i), title, "audible", models.BookStatusWanted) + localBooks = append(localBooks, f.createBook(t, fmt.Sprintf("audible:hdc-%d", i), title, "audible", models.BookStatusWanted)) } preview, err := f.handler.buildCatalogueReconciliation(context.Background(), f.author) @@ -202,6 +221,9 @@ func TestPreviewCatalogueReconciliation_UnmatchedCrossProviderRowsAreIndetermina if preview.Summary.Kept != len(titles) || preview.Summary.Indeterminate != len(titles) { t.Fatalf("summary = %+v, want %d kept and indeterminate rows", preview.Summary, len(titles)) } + for _, book := range localBooks { + assertIndeterminateRow(t, preview, book, reconcileIndeterminateReasonUnmatchedCrossProvider) + } } func TestPreviewCatalogueReconciliation_SameProviderAbsenceRemainsCandidate(t *testing.T) { @@ -257,6 +279,7 @@ func TestApplyCatalogueReconciliation_CannotDeleteUnmatchedCrossProviderRow(t *t if len(preview.Candidates) != 0 || preview.Summary.Kept != 1 || preview.Summary.Indeterminate != 1 { t.Fatalf("preview = %+v, want one indeterminate kept row", preview) } + assertIndeterminateRow(t, preview, local, reconcileIndeterminateReasonUnmatchedCrossProvider) body, err := json.Marshal(applyCatalogueReconciliationRequest{BookIDs: []int64{local.ID}}) if err != nil { @@ -326,7 +349,7 @@ func TestPreviewCatalogueReconciliation_IncompleteLanguageEvidenceIsIndeterminat }, } f := newReconciliationFixture(t, provider) - f.createBook(t, "hc:ambiguous", "Ambiguous", "hardcover", models.BookStatusWanted) + local := f.createBook(t, "hc:ambiguous", "Ambiguous", "hardcover", models.BookStatusWanted) preview, err := f.handler.buildCatalogueReconciliation(context.Background(), f.author) if err != nil { @@ -338,6 +361,7 @@ func TestPreviewCatalogueReconciliation_IncompleteLanguageEvidenceIsIndeterminat if preview.Summary.Kept != 1 || preview.Summary.Indeterminate != 1 { t.Errorf("summary = %+v, want one indeterminate kept row", preview.Summary) } + assertIndeterminateRow(t, preview, local, reconcileIndeterminateReasonLanguageUnknown) } func TestPreviewCatalogueReconciliation_LanguageEvidenceFailureIsIndeterminate(t *testing.T) { @@ -351,7 +375,7 @@ func TestPreviewCatalogueReconciliation_LanguageEvidenceFailureIsIndeterminate(t err: errors.New("language evidence unavailable"), } f := newReconciliationFixture(t, provider) - f.createBook(t, "hc:translated-default", "Translated Default", "hardcover", models.BookStatusWanted) + local := f.createBook(t, "hc:translated-default", "Translated Default", "hardcover", models.BookStatusWanted) preview, err := f.handler.buildCatalogueReconciliation(context.Background(), f.author) if err != nil { @@ -363,6 +387,7 @@ func TestPreviewCatalogueReconciliation_LanguageEvidenceFailureIsIndeterminate(t if preview.Summary.Kept != 1 || preview.Summary.Indeterminate != 1 { t.Errorf("summary = %+v, want one indeterminate kept row", preview.Summary) } + assertIndeterminateRow(t, preview, local, reconcileIndeterminateReasonLanguageLookupFailed) if provider.calls != 1 { t.Errorf("language evidence calls = %d, want 1", provider.calls) } @@ -461,6 +486,7 @@ func TestPreviewCatalogueReconciliation_PartialProviderNeverUsesAbsence(t *testi if got.Summary.Indeterminate != 1 { t.Fatalf("indeterminate = %d, want the missing local row", got.Summary.Indeterminate) } + assertIndeterminateRow(t, got, missing, reconcileIndeterminateReasonPartialCatalogue) if book, _ := f.books.GetByID(context.Background(), missing.ID); book == nil { t.Fatal("preview mutated the database") } @@ -804,9 +830,13 @@ func TestBuildCatalogueReconciliation_UsesConservativeEditionAndIdentityEvidence t.Errorf("candidate %d reason = %q, want %q", candidate.BookID, candidate.Reason, want) } } - if got.Summary.Indeterminate < 4 { + if got.Summary.Indeterminate != 4 { t.Errorf("indeterminate = %d, want no-ID, DNB, edition-error, and unmatched previous-provider rows kept", got.Summary.Indeterminate) } + assertIndeterminateRow(t, got, noID, reconcileIndeterminateReasonEditionUnavailable) + assertIndeterminateRow(t, got, dnb, reconcileIndeterminateReasonEditionUnavailable) + assertIndeterminateRow(t, got, editionUnknown, reconcileIndeterminateReasonEditionUnavailable) + assertIndeterminateRow(t, got, fallbackProvider, reconcileIndeterminateReasonUnmatchedCrossProvider) if got.Summary.ProtectedStatus < 2 { t.Errorf("protected status = %d, want skipped and other-owner rows", got.Summary.ProtectedStatus) } @@ -821,14 +851,14 @@ func TestReconciliationRejectReason_ProfileReasonsAndIndeterminateEvidence(t *te pages100 := 100 pages250 := 250 tests := []struct { - name string - work models.Book - profile reconciliationProfile - editionEvidence editionEvidence - languageEvidence map[string]metadata.AuthorWorkLanguageEvidence - languageEvidenceFailed bool - wantReason string - indeterminate bool + name string + work models.Book + profile reconciliationProfile + editionEvidence editionEvidence + languageEvidence map[string]metadata.AuthorWorkLanguageEvidence + languageEvidenceFailed bool + wantReason string + wantIndeterminateReason CatalogueReconciliationIndeterminateReason }{ {name: "accepted", work: models.Book{Title: "Dune"}}, {name: "empty title", work: models.Book{}, wantReason: reconcileReasonCatalogueFilter}, @@ -841,7 +871,7 @@ func TestReconciliationRejectReason_ProfileReasonsAndIndeterminateEvidence(t *te }, { name: "unknown language kept", work: models.Book{Title: "Unknown"}, - profile: reconciliationProfile{allowedLangs: []string{"eng"}, unknownFail: true}, indeterminate: true, + profile: reconciliationProfile{allowedLangs: []string{"eng"}, unknownFail: true}, wantIndeterminateReason: reconcileIndeterminateReasonLanguageUnknown, }, { name: "allowed edition overrides translated default", @@ -862,7 +892,7 @@ func TestReconciliationRejectReason_ProfileReasonsAndIndeterminateEvidence(t *te { name: "indeterminate evidence protects blank scalar", work: models.Book{ForeignID: "hc:unknown", Title: "Unknown"}, - profile: reconciliationProfile{allowedLangs: []string{"eng"}, unknownFail: true}, indeterminate: true, + profile: reconciliationProfile{allowedLangs: []string{"eng"}, unknownFail: true}, wantIndeterminateReason: reconcileIndeterminateReasonLanguageUnknown, languageEvidence: map[string]metadata.AuthorWorkLanguageEvidence{ "hc:unknown": {State: metadata.AuthorWorkLanguageIndeterminate}, }, @@ -885,11 +915,11 @@ func TestReconciliationRejectReason_ProfileReasonsAndIndeterminateEvidence(t *te wantReason: reconcileReasonLanguage, }, { - name: "failed evidence lookup protects non-allowed scalar", - work: models.Book{ForeignID: "hc:failed", Title: "Failed", Language: "por"}, - profile: reconciliationProfile{allowedLangs: []string{"eng"}, unknownFail: true}, - languageEvidenceFailed: true, - indeterminate: true, + name: "failed evidence lookup protects non-allowed scalar", + work: models.Book{ForeignID: "hc:failed", Title: "Failed", Language: "por"}, + profile: reconciliationProfile{allowedLangs: []string{"eng"}, unknownFail: true}, + languageEvidenceFailed: true, + wantIndeterminateReason: reconcileIndeterminateReasonLanguageLookupFailed, }, { name: "part book", work: models.Book{Title: "The New Turing Omnibus"}, @@ -901,7 +931,7 @@ func TestReconciliationRejectReason_ProfileReasonsAndIndeterminateEvidence(t *te }, { name: "edition lookup unavailable", work: models.Book{Title: "Unresolved"}, - profile: reconciliationProfile{minPages: 200}, indeterminate: true, + profile: reconciliationProfile{minPages: 200}, wantIndeterminateReason: reconcileIndeterminateReasonEditionUnavailable, }, { name: "missing isbn", work: models.Book{Title: "No ISBN"}, @@ -921,9 +951,9 @@ func TestReconciliationRejectReason_ProfileReasonsAndIndeterminateEvidence(t *te } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - reason, indeterminate := reconciliationRejectReason(tt.work, "test author", tt.profile, tt.editionEvidence, tt.languageEvidence, tt.languageEvidenceFailed) - if reason != tt.wantReason || indeterminate != tt.indeterminate { - t.Fatalf("reason=%q indeterminate=%v, want %q/%v", reason, indeterminate, tt.wantReason, tt.indeterminate) + reason, indeterminateReason := reconciliationRejectReason(tt.work, "test author", tt.profile, tt.editionEvidence, tt.languageEvidence, tt.languageEvidenceFailed) + if reason != tt.wantReason || indeterminateReason != tt.wantIndeterminateReason { + t.Fatalf("reason=%q indeterminate=%q, want %q/%q", reason, indeterminateReason, tt.wantReason, tt.wantIndeterminateReason) } }) } diff --git a/web/src/api/authors.ts b/web/src/api/authors.ts index 1872650d6..39b97e26a 100644 --- a/web/src/api/authors.ts +++ b/web/src/api/authors.ts @@ -165,6 +165,20 @@ export interface CatalogueReconciliationCandidate { reason: CatalogueReconciliationReason } +export type CatalogueReconciliationIndeterminateReason = + | 'language_unknown' + | 'language_evidence_lookup_failed' + | 'edition_evidence_unavailable' + | 'partial_catalogue' + | 'unmatched_cross_provider' + +export interface CatalogueReconciliationIndeterminateRow { + bookId: number + title: string + metadataProvider: string + reason: CatalogueReconciliationIndeterminateReason +} + export interface CatalogueReconciliationSummary { total: number candidates: number @@ -186,6 +200,7 @@ export interface CatalogueReconciliation { profileName: string warning?: string candidates: CatalogueReconciliationCandidate[] + indeterminateRows: CatalogueReconciliationIndeterminateRow[] summary: CatalogueReconciliationSummary applied?: { requested: number; deleted: number; skipped: number } } diff --git a/web/src/components/CatalogueReconciliationModal.test.tsx b/web/src/components/CatalogueReconciliationModal.test.tsx index 9e08d196b..a9ebda57b 100644 --- a/web/src/components/CatalogueReconciliationModal.test.tsx +++ b/web/src/components/CatalogueReconciliationModal.test.tsx @@ -1,4 +1,4 @@ -import { fireEvent, render, screen, waitFor } from '@testing-library/react' +import { fireEvent, render, screen, waitFor, within } from '@testing-library/react' import { beforeEach, describe, expect, it, vi } from 'vitest' import { api, CatalogueReconciliation } from '../api/client' import CatalogueReconciliationModal from './CatalogueReconciliationModal' @@ -32,6 +32,10 @@ const preview: CatalogueReconciliation = { { bookId: 11, title: 'Libro', metadataProvider: 'hardcover', reason: 'language_not_allowed' }, { bookId: 12, title: 'Old Work', metadataProvider: 'hardcover', reason: 'not_in_current_catalogue' }, ], + indeterminateRows: [ + { bookId: 21, title: 'Ból za ból', metadataProvider: 'hardcover', reason: 'language_unknown' }, + { bookId: 22, title: 'Legacy Audible Work', metadataProvider: 'audible', reason: 'unmatched_cross_provider' }, + ], summary: { total: 8, candidates: 2, @@ -41,7 +45,7 @@ const preview: CatalogueReconciliation = { protectedImported: 1, protectedStatus: 0, protectedExcluded: 0, - indeterminate: 1, + indeterminate: 2, reasons: { language_not_allowed: 1, not_in_current_catalogue: 1 }, }, } @@ -60,7 +64,16 @@ describe('CatalogueReconciliationModal', () => { expect(screen.getByText('Old Work')).toBeInTheDocument() expect(screen.getByText('Provider: hardcover · Profile: English only')).toBeInTheDocument() expect(screen.getByText(/2 file-bearing, 1 imported, 0 other-status, and 0 excluded/)).toBeInTheDocument() - expect(screen.getByText('1 row(s) were kept because provider or profile evidence was incomplete.')).toBeInTheDocument() + expect(screen.getByText('2 row(s) were kept because provider or profile evidence was incomplete.')).toBeInTheDocument() + expect(screen.getByRole('heading', { name: 'Kept because evidence is incomplete' })).toBeInTheDocument() + const indeterminateRow = screen.getByText('Ból za ból').closest('li') + expect(indeterminateRow).not.toBeNull() + expect(within(indeterminateRow!).getByText('hardcover')).toBeInTheDocument() + expect(within(indeterminateRow!).queryByRole('checkbox')).not.toBeInTheDocument() + expect(screen.getByText('Language could not be confirmed')).toBeInTheDocument() + expect(screen.getByText('Legacy Audible Work')).toBeInTheDocument() + expect(screen.getByText('Could not match this row to the current provider')).toBeInTheDocument() + expect(screen.getAllByRole('checkbox')).toHaveLength(2) expect(screen.getByRole('button', { name: 'Remove 2 stale row(s)' })).toBeInTheDocument() }) @@ -106,7 +119,7 @@ describe('CatalogueReconciliationModal', () => { expect(screen.getByRole('button', { name: 'Remove 0 stale row(s)' })).toBeDisabled() }) - it('does not offer apply when the preview is clean', async () => { + it('shows indeterminate rows without offering a removal action for them', async () => { vi.mocked(api.previewAuthorCatalogueReconciliation).mockResolvedValue({ ...preview, candidates: [], @@ -114,7 +127,10 @@ describe('CatalogueReconciliationModal', () => { }) render( {}} />) - expect(await screen.findByText('No metadata-only Wanted rows need reconciliation.')).toBeInTheDocument() - expect(screen.queryByRole('button', { name: /Remove .* stale/ })).toBeDisabled() + expect(await screen.findByText('No metadata-only Wanted rows are safe to remove.')).toBeInTheDocument() + expect(screen.getByText('Ból za ból')).toBeInTheDocument() + expect(screen.queryByRole('checkbox')).not.toBeInTheDocument() + expect(screen.getByRole('button', { name: 'Remove 0 stale row(s)' })).toBeDisabled() + expect(api.applyAuthorCatalogueReconciliation).not.toHaveBeenCalled() }) }) diff --git a/web/src/components/CatalogueReconciliationModal.tsx b/web/src/components/CatalogueReconciliationModal.tsx index 3a32df9e9..9f03d0112 100644 --- a/web/src/components/CatalogueReconciliationModal.tsx +++ b/web/src/components/CatalogueReconciliationModal.tsx @@ -1,7 +1,12 @@ import { useEffect, useState } from 'react' import { useTranslation } from 'react-i18next' import Alert from './Alert' -import { api, CatalogueReconciliation, CatalogueReconciliationReason } from '../api/client' +import { + api, + CatalogueReconciliation, + CatalogueReconciliationIndeterminateReason, + CatalogueReconciliationReason, +} from '../api/client' import { btn, btnSize } from './buttons' interface Props { @@ -21,6 +26,14 @@ const reasonDefaults: Record = { catalogue_filter: 'Rejected by the catalogue filter', } +const indeterminateReasonDefaults: Record = { + language_unknown: 'Language could not be confirmed', + language_evidence_lookup_failed: 'Language evidence could not be retrieved', + edition_evidence_unavailable: 'Edition evidence could not be confirmed', + partial_catalogue: 'The provider returned only part of its catalogue', + unmatched_cross_provider: 'Could not match this row to the current provider', +} + export default function CatalogueReconciliationModal({ authorId, authorName, onClose, onApplied }: Props) { const { t } = useTranslation() const [result, setResult] = useState(null) @@ -68,6 +81,7 @@ export default function CatalogueReconciliationModal({ authorId, authorName, onC } const candidates = result?.candidates ?? [] + const indeterminateRows = result?.indeterminateRows ?? [] const applied = result?.applied const selectedCount = candidates.reduce( (count, candidate) => count + (selectedBookIds.has(candidate.bookId) ? 1 : 0), @@ -148,49 +162,73 @@ export default function CatalogueReconciliationModal({ authorId, authorName, onC )} - {applied ? ( -
- {t('catalogueReconciliation.applied', { - deleted: applied.deleted, - skipped: applied.skipped, - defaultValue: 'Removed {{deleted}} row(s). {{skipped}} were skipped because they were no longer eligible.', - })} -
- ) : candidates.length === 0 ? ( -

- {t('catalogueReconciliation.clean', 'No metadata-only Wanted rows need reconciliation.')} -

- ) : ( -
-
    - {candidates.map(candidate => ( -
  • -
    • + ))} +
+
+ + )} - )} ) : null} diff --git a/web/src/i18n/locales/en.json b/web/src/i18n/locales/en.json index b649def00..7f19841c2 100644 --- a/web/src/i18n/locales/en.json +++ b/web/src/i18n/locales/en.json @@ -525,7 +525,8 @@ "indeterminate": "{{count}} row(s) were kept because provider or profile evidence was incomplete.", "partialWarning": "The provider returned a partial catalogue. Missing works are protected; only explicit profile rejections can be removed.", "applied": "Removed {{deleted}} row(s). {{skipped}} were skipped because they were no longer eligible.", - "clean": "No metadata-only Wanted rows need reconciliation.", + "noSafeCandidates": "No metadata-only Wanted rows are safe to remove.", + "indeterminateHeading": "Kept because evidence is incomplete", "selectCandidate": "Select {{title}}", "applying": "Rechecking and removing…", "apply": "Remove {{count}} stale row(s)", @@ -537,6 +538,13 @@ "below_minimum_pages": "Below the minimum page count", "missing_isbn": "No edition has an ISBN", "catalogue_filter": "Rejected by the catalogue filter" + }, + "indeterminateReasons": { + "language_unknown": "Language could not be confirmed", + "language_evidence_lookup_failed": "Language evidence could not be retrieved", + "edition_evidence_unavailable": "Edition evidence could not be confirmed", + "partial_catalogue": "The provider returned only part of its catalogue", + "unmatched_cross_provider": "Could not match this row to the current provider" } }, "duplicateCandidates": {