From 9b8065d67cd9f6ee7b2420fd40e28eb2096abb4a Mon Sep 17 00:00:00 2001 From: Sergio Vera Date: Fri, 6 Nov 2026 16:28:56 +0100 Subject: [PATCH 1/5] Fixed contrast, message and 404 handling --- README.md | 2 + internal/datasource/biblioreads/adapter.go | 3 + .../datasource/biblioreads/adapter_test.go | 60 ++++ internal/datasource/biblioreads/client.go | 26 +- .../datasource/biblioreads/client_test.go | 61 ++++ internal/index/bleve.go | 15 + internal/index/bleve_document_enrich.go | 131 +++++++- internal/index/bleve_document_enrich_test.go | 301 +++++++++++++++++- internal/index/bleve_document_write.go | 34 +- internal/index/progress.go | 2 + .../webserver/document_biblioreads_test.go | 73 +++++ .../embedded/js/search-filter-utils.js | 15 +- .../webserver/embedded/translations/de.yml | 6 +- .../webserver/embedded/translations/es.yml | 6 +- .../webserver/embedded/translations/fr.yml | 6 +- .../webserver/embedded/translations/ru.yml | 6 +- .../views/partials/background-progress.html | 15 +- .../js-tests/search-filter-utils.test.mjs | 33 +- main.go | 8 +- 19 files changed, 760 insertions(+), 43 deletions(-) diff --git a/README.md b/README.md index 191b0100..90b41b8d 100644 --- a/README.md +++ b/README.md @@ -98,6 +98,8 @@ coreander --review-source-instances=https://br.example.org,https://br2.example.o Alternatively, set `REVIEW_SOURCE_INSTANCES` to the same comma-separated list. The flag may also be repeated and overrides the environment variable. Instance URLs must use HTTP or HTTPS and serve the BiblioReads API. +BiblioReads can return an "Invalid Query" search error for a temporary upstream failure. Coreander tries the same query on another configured instance, if available; it does not broaden the search in response to an error. Failed lookups do not stop other documents or get marked as completed. Pending lookups are retried automatically after one minute, with exponential backoff up to 15 minutes between failed runs, while respecting instance cooldowns. HTTP 404 responses may also be legitimate: after three lookup attempts returning 404s for a document during the current process, automatic retries for that document stop. A lookup attempt may try multiple configured instances. These documents are reported as unresolved, not as successfully enriched; restart Coreander to retry them. Requests skipped because every instance is in cooldown do not count toward the limit. Other transient failures continue to retry. The progress banner distinguishes pending retries from unresolved lookups and removes the progress bar when only unresolved documents remain. Completed lookups are not repeated; pending lookups from later uploads are also checked every minute after successful runs. + ### Email Some features rely on having an SMTP email service set up, and won't be available otherwise: diff --git a/internal/datasource/biblioreads/adapter.go b/internal/datasource/biblioreads/adapter.go index 96274ad1..28babdc9 100644 --- a/internal/datasource/biblioreads/adapter.go +++ b/internal/datasource/biblioreads/adapter.go @@ -48,6 +48,9 @@ func (*Enricher) NeedsEnrichment(document index.Document) (bool, error) { func (p *Enricher) Enrich(ctx context.Context, idx *index.BleveIndexer, document index.Document) error { _, err := p.enrich(ctx, idx, document) + if errors.Is(err, ErrNotFound) { + return fmt.Errorf("%w: %w", index.ErrEnrichmentNotFound, err) + } return err } diff --git a/internal/datasource/biblioreads/adapter_test.go b/internal/datasource/biblioreads/adapter_test.go index f1758a68..e72cfb8c 100644 --- a/internal/datasource/biblioreads/adapter_test.go +++ b/internal/datasource/biblioreads/adapter_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "image" "reflect" "slices" @@ -339,6 +340,35 @@ func TestEnrichLeavesDocumentsForLaterWhenUnavailable(t *testing.T) { } } +func TestEnrichBoundsNotFoundLookups(t *testing.T) { + source := &fakeSource{err: fmt.Errorf("%w: %w", ErrUnavailable, ErrNotFound)} + idx := newTestIndexer(t, source) + doc := index.Document{ID: testID, Metadata: metadata.Metadata{Title: "El economista camuflado", Authors: []string{"Tim Harford"}}} + if err := index.IndexDocument(idx.documentsIdx, doc.ID, doc); err != nil { + t.Fatal(err) + } + for attempt := range 5 { + err := idx.EnrichDocuments(1) + if attempt < 3 && (!errors.Is(err, ErrNotFound) || !errors.Is(err, index.ErrEnrichmentNotFound)) { + t.Fatalf("not-found classification must reach the retry scheduler: %v", err) + } + if attempt >= 3 && err != nil { + t.Fatalf("exhausted lookup must not be retried automatically: %v", err) + } + } + if queries := source.Queries(); !slices.Equal(queries, []string{ + "El economista camuflado Tim Harford", + "El economista camuflado Tim Harford", + "El economista camuflado Tim Harford", + }) { + t.Fatalf("expected exactly three attempts of the original query: %q", queries) + } + stored, err := idx.DocumentByID(doc.ID) + if err != nil || mustData(t, stored).Enriched { + t.Fatalf("exhausted lookups must remain unresolved: %+v, %v", stored, err) + } +} + func TestNewFileSchedulesEnrichment(t *testing.T) { source := &fakeSource{results: testResults} idx := newTestIndexer(t, source) @@ -875,6 +905,36 @@ func TestTitleFallback(t *testing.T) { } } +func TestUnavailableSearchDoesNotBroadenQuery(t *testing.T) { + const title = "El economista camuflado" + const author = "Tim Harford" + source := &fakeSource{ + byQuery: map[string][]SearchResult{ + title: {{Title: title, Author: author, BookURL: "/book/show/123", Rating: "4.00"}}, + }, + queryErrors: map[string]error{title + " " + author: ErrUnavailable}, + } + idx := newTestIndexer(t, source) + doc := index.Document{ID: testID, Metadata: metadata.Metadata{Title: title, Authors: []string{author}}} + if err := index.IndexDocument(idx.documentsIdx, doc.ID, doc); err != nil { + t.Fatal(err) + } + got, err := idx.Book(context.Background(), doc) + if !errors.Is(err, ErrUnavailable) || got.BookURL != "" { + t.Fatalf("expected unavailable search without a match, got %+v, %v", got, err) + } + stored, err := idx.DocumentByID(doc.ID) + if err != nil { + t.Fatal(err) + } + if mustData(t, stored).Enriched { + t.Fatal("failed search must remain pending") + } + if !slices.Equal(source.Queries(), []string{title + " " + author}) { + t.Fatalf("failed search must not trigger a title-only query, got %q", source.Queries()) + } +} + func TestMatchesTolkienOmnibusSearchResults(t *testing.T) { const title = "El señor de los anillos" const wantURL = "/book/show/37910810-el-se-or-de-los-anillos" diff --git a/internal/datasource/biblioreads/client.go b/internal/datasource/biblioreads/client.go index 21c54b0c..12d2a92d 100644 --- a/internal/datasource/biblioreads/client.go +++ b/internal/datasource/biblioreads/client.go @@ -32,6 +32,9 @@ const ( // non-JSON responses), as opposed to an instance answering with an error of its own. var ErrUnavailable = errors.New("biblioreads: all instances failed") +// ErrNotFound identifies HTTP 404 responses, which may be legitimate or an upstream failure. +var ErrNotFound = errors.New("biblioreads: not found") + type Client struct { instances []string httpClient *http.Client @@ -383,15 +386,34 @@ func (c *Client) postToInstance(ctx context.Context, endpoint string, body []byt // BiblioReads always answers with JSON, even on errors (e.g. a 504 for a book that // doesn't exist), so anything else comes from a broken instance or a proxy in front of it. if !json.Valid(resp.Body) { + reason := fmt.Errorf("status code %d with non-JSON response", resp.StatusCode) + if resp.StatusCode == http.StatusNotFound { + reason = fmt.Errorf("%w: %w", ErrNotFound, reason) + } return nil, &unavailableError{ endpoint: endpoint, retryAfter: httpjson.DefaultRetryAfter, - reason: fmt.Errorf("status code %d with non-JSON response", resp.StatusCode), + reason: reason, } } if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("biblioreads: request to %s failed with status code %d: %s", endpoint, resp.StatusCode, resp.Body) + responseErr := fmt.Errorf("biblioreads: request to %s failed with status code %d: %s", endpoint, resp.StatusCode, resp.Body) + if resp.StatusCode == http.StatusNotFound { + responseErr = fmt.Errorf("%w: %w", ErrNotFound, responseErr) + } + if resp.StatusCode == http.StatusNotFound && strings.HasSuffix(endpoint, searchBooksPath) { + var response struct { + Status string `json:"status"` + } + if err := json.Unmarshal(resp.Body, &response); err == nil && response.Status == "Error - Invalid Query" { + return nil, &unavailableError{ + endpoint: endpoint, retryAfter: httpjson.DefaultRetryAfter, + reason: responseErr, + } + } + } + return nil, responseErr } return resp.Body, nil } diff --git a/internal/datasource/biblioreads/client_test.go b/internal/datasource/biblioreads/client_test.go index 556afdf2..32355282 100644 --- a/internal/datasource/biblioreads/client_test.go +++ b/internal/datasource/biblioreads/client_test.go @@ -393,6 +393,67 @@ func TestErrorStatus(t *testing.T) { } } +func TestInvalidSearchQuery(t *testing.T) { + for _, tt := range []struct { + name string + status int + body string + unavailable bool + }{ + {"invalid query", http.StatusNotFound, `{"status":"Error - Invalid Query","version":"BiblioReads v4.4.0"}`, true}, + {"other not found", http.StatusNotFound, `{"status":"Error - Book not found"}`, false}, + {"other client error", http.StatusBadRequest, `{"status":"Error - Invalid Query"}`, false}, + {"server error", http.StatusServiceUnavailable, `{"status":"Error - Invalid Query"}`, true}, + } { + t.Run(tt.name, func(t *testing.T) { + var hits atomic.Int32 + server := newCountingServer(t, tt.status, tt.body, &hits) + client := newTestClient(biblioreads.WithInstances([]string{server.URL})) + _, err := client.SearchBooks(context.Background(), "El economista camuflado Tim Harford") + if err == nil || errors.Is(err, biblioreads.ErrUnavailable) != tt.unavailable { + t.Fatalf("unexpected search error classification: %v", err) + } + if errors.Is(err, biblioreads.ErrNotFound) != (tt.status == http.StatusNotFound) { + t.Fatalf("HTTP 404 classification must survive failover wrapping: %v", err) + } + }) + } +} + +func TestNonJSONNotFoundClassification(t *testing.T) { + var hits atomic.Int32 + server := newCountingServer(t, http.StatusNotFound, "Not found", &hits) + client := newTestClient(biblioreads.WithInstances([]string{server.URL})) + _, err := client.SearchBooks(context.Background(), "book") + if !errors.Is(err, biblioreads.ErrNotFound) || !errors.Is(err, biblioreads.ErrUnavailable) { + t.Fatalf("non-JSON 404 must retain its HTTP status classification: %v", err) + } +} + +func TestSearchInvalidQueryFailsOverWithoutChangingQuery(t *testing.T) { + var hits atomic.Int32 + fixture := readFixture(t, "search-books.json") + client := newFailoverClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var captured capturedRequest + if !captureRequest(t, w, r, &captured) { + return + } + assertRequest(t, captured, "/api/search/books", map[string]any{ + "queryURL": "https://www.goodreads.com/search?q=El%20economista%20camuflado%20Tim%20Harford", + }) + if hits.Add(1) == 1 { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"status":"Error - Invalid Query","version":"BiblioReads v4.4.0"}`)) + return + } + _, _ = w.Write(fixture) + })) + response, err := client.SearchBooks(context.Background(), "El economista camuflado Tim Harford") + if err != nil || len(response.Result) == 0 || hits.Load() != 2 { + t.Fatalf("expected same-query recovery on another instance, got %+v, %v, requests=%d", response, err, hits.Load()) + } +} + func newCountingServer(t *testing.T, status int, body string, hits *atomic.Int32) *httptest.Server { t.Helper() server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/internal/index/bleve.go b/internal/index/bleve.go index 93e3d513..a613127b 100644 --- a/internal/index/bleve.go +++ b/internal/index/bleve.go @@ -1,6 +1,7 @@ package index import ( + "context" "errors" "log" "path/filepath" @@ -221,6 +222,10 @@ type BleveIndexer struct { documentEnrichers []DocumentEnricher // documentEnrichProgress measures each provider's current run rate, by enricher index. documentEnrichProgress []progressTracker + documentEnrichContext context.Context + documentEnrichCancel context.CancelFunc + documentEnrichStart sync.Once + documentEnrichWG sync.WaitGroup providerProgressMu sync.Mutex providerProgressUntil time.Time providerProgress []providerProgressCount @@ -244,11 +249,14 @@ type progressTracker struct { startNanos atomic.Int64 processed atomic.Uint64 total atomic.Uint64 + failed atomic.Bool + lookups sync.Map // Document ID -> *documentLookupState for provider retry limits. } // begin marks the phase as started, resetting processed to 0 and total to // the given count. func (p *progressTracker) begin(total int) { + p.failed.Store(false) p.startNanos.Store(time.Now().UnixNano()) p.processed.Store(0) p.total.Store(uint64(total)) @@ -323,6 +331,7 @@ func NewBleve(documentsIndex bleve.Index, authorsIndex bleve.Index, fs afero.Fs, textRankEnrichWorkers = 1 } + enrichContext, enrichCancel := context.WithCancel(context.Background()) b := &BleveIndexer{ fs: fs, documentsIdx: documentsIndex, @@ -342,6 +351,8 @@ func NewBleve(documentsIndex bleve.Index, authorsIndex bleve.Index, fs afero.Fs, textRankEnrichJobs: make(chan Document, textRankEnrichWorkers), documentEnrichers: slices.Clone(cfg.DocumentEnrichers), documentEnrichProgress: make([]progressTracker, len(cfg.DocumentEnrichers)), + documentEnrichContext: enrichContext, + documentEnrichCancel: enrichCancel, } for range textRankEnrichWorkers { @@ -562,6 +573,10 @@ func CreateAuthorsMapping() mapping.IndexMapping { // Close closes both indexes func (b *BleveIndexer) Close() error { + if b.documentEnrichCancel != nil { + b.documentEnrichCancel() + } + b.documentEnrichWG.Wait() b.documentsMu.Lock() defer b.documentsMu.Unlock() b.authorsMu.Lock() diff --git a/internal/index/bleve_document_enrich.go b/internal/index/bleve_document_enrich.go index df19a292..35d4356c 100644 --- a/internal/index/bleve_document_enrich.go +++ b/internal/index/bleve_document_enrich.go @@ -6,6 +6,8 @@ import ( "fmt" "log" "sync" + "sync/atomic" + "time" "github.com/blevesearch/bleve/v2" ) @@ -14,6 +16,28 @@ import ( // their own source request limits because batch and upload pools run concurrently. const documentEnrichWorkers = 4 +const maxNotFoundAttempts = 3 + +// ErrEnrichmentNotFound marks an ambiguous provider 404, not a successful no-match lookup. +var ErrEnrichmentNotFound = errors.New("provider lookup returned not found") + +type documentLookupState struct { + mu sync.Mutex + notFoundAttempts atomic.Uint32 +} + +func (p *progressTracker) unresolvedDocuments() map[string]struct{} { + unresolved := make(map[string]struct{}) + p.lookups.Range(func(key, value any) bool { + state := value.(*documentLookupState) + if state.notFoundAttempts.Load() >= maxNotFoundAttempts { + unresolved[key.(string)] = struct{}{} + } + return true + }) + return unresolved +} + // DocumentEnricher owns a provider's lookup policy and persisted payload. type DocumentEnricher interface { // Source is the persisted storage key; Name is shown to users. @@ -57,12 +81,60 @@ func (b *BleveIndexer) documentsNeedingProvider(source string, size int, exclude // EnrichDocuments resumes pending lookups for every configured provider. // A provider failure does not prevent independent providers from running. func (b *BleveIndexer) EnrichDocuments(batchSize int) error { + return b.enrichDocuments(context.Background(), batchSize) +} + +// StartDocumentEnrichment retries pending lookups until the indexer is closed. +func (b *BleveIndexer) StartDocumentEnrichment(batchSize int) error { + if batchSize <= 0 { + return fmt.Errorf("document enrichment batch size must be positive") + } + if len(b.documentEnrichers) == 0 { + return nil + } + b.documentEnrichStart.Do(func() { + b.documentEnrichWG.Go(func() { + b.retryDocumentEnrichment(b.documentEnrichContext, batchSize, time.Minute, 15*time.Minute) + }) + }) + return nil +} + +func (b *BleveIndexer) retryDocumentEnrichment(ctx context.Context, batchSize int, interval, maxDelay time.Duration) { + delay := interval + for ctx.Err() == nil { + err := b.enrichDocuments(ctx, batchSize) + if ctx.Err() != nil { + return + } + if err != nil { + log.Printf("Error enriching documents with provider data; retrying pending lookups in %s: %s", delay, err) + } else { + delay = interval + } + timer := time.NewTimer(delay) + select { + case <-ctx.Done(): + timer.Stop() + return + case <-timer.C: + } + if err != nil { + delay = min(2*delay, maxDelay) + } + } +} + +func (b *BleveIndexer) enrichDocuments(ctx context.Context, batchSize int) error { if batchSize <= 0 { return fmt.Errorf("document enrichment batch size must be positive") } var failures []error for i, provider := range b.documentEnrichers { - if err := b.enrichProvider(provider, b.documentEnrichTracker(i), batchSize); err != nil { + if err := ctx.Err(); err != nil { + return err + } + if err := b.enrichProvider(ctx, provider, b.documentEnrichTracker(i), batchSize); err != nil { failures = append(failures, fmt.Errorf("enriching documents with %s: %w", provider.Source(), err)) } } @@ -76,17 +148,23 @@ func (b *BleveIndexer) documentEnrichTracker(i int) *progressTracker { return &progressTracker{} } -func (b *BleveIndexer) enrichProvider(provider DocumentEnricher, tracker *progressTracker, batchSize int) error { - _, total, err := b.documentsNeedingProvider(provider.Source(), 0, nil) +func (b *BleveIndexer) enrichProvider(ctx context.Context, provider DocumentEnricher, tracker *progressTracker, batchSize int) (resultErr error) { + defer func() { + tracker.failed.Store(resultErr != nil) + tracker.end() + }() + failedDocuments := tracker.unresolvedDocuments() + _, total, err := b.documentsNeedingProvider(provider.Source(), 0, failedDocuments) if err != nil || total == 0 { return err } tracker.begin(int(total)) - defer tracker.end() log.Printf("Enriching %d documents with %s data", total, provider.Source()) var failures []error - failedDocuments := make(map[string]struct{}) for iteration := 0; iteration < int(total)+10; iteration++ { + if err := ctx.Err(); err != nil { + return err + } documents, _, err := b.documentsNeedingProvider(provider.Source(), batchSize, failedDocuments) if err != nil { failures = append(failures, err) @@ -95,7 +173,7 @@ func (b *BleveIndexer) enrichProvider(provider DocumentEnricher, tracker *progre if len(documents) == 0 { return errors.Join(failures...) } - for id, err := range b.enrichProviderBatch(provider, tracker, documents) { + for id, err := range b.enrichProviderBatch(ctx, provider, tracker, documents) { failedDocuments[id] = struct{}{} failures = append(failures, err) } @@ -106,7 +184,7 @@ func (b *BleveIndexer) enrichProvider(provider DocumentEnricher, tracker *progre return fmt.Errorf("document enrichment did not finish; some documents remain pending") } -func (b *BleveIndexer) enrichProviderBatch(provider DocumentEnricher, tracker *progressTracker, documents []Document) map[string]error { +func (b *BleveIndexer) enrichProviderBatch(ctx context.Context, provider DocumentEnricher, tracker *progressTracker, documents []Document) map[string]error { jobs := make(chan Document, len(documents)) for _, document := range documents { jobs <- document @@ -118,10 +196,10 @@ func (b *BleveIndexer) enrichProviderBatch(provider DocumentEnricher, tracker *p for range min(documentEnrichWorkers, len(documents)) { workers.Go(func() { for document := range jobs { - needed, err := provider.NeedsEnrichment(document) - if err == nil && needed { - err = provider.Enrich(context.Background(), b, document) + if ctx.Err() != nil { + return } + err := b.enrichDocument(ctx, provider, tracker, document) if err != nil { failureMu.Lock() failures[document.ID] = fmt.Errorf("enriching document %s: %w", document.ID, err) @@ -135,9 +213,35 @@ func (b *BleveIndexer) enrichProviderBatch(provider DocumentEnricher, tracker *p return failures } +func (b *BleveIndexer) enrichDocument(ctx context.Context, provider DocumentEnricher, tracker *progressTracker, document Document) error { + value, _ := tracker.lookups.LoadOrStore(document.ID, &documentLookupState{}) + state := value.(*documentLookupState) + state.mu.Lock() + defer state.mu.Unlock() + if state.notFoundAttempts.Load() >= maxNotFoundAttempts { + return fmt.Errorf("automatic retry limit reached after %d not-found responses", maxNotFoundAttempts) + } + needed, err := provider.NeedsEnrichment(document) + if err == nil && needed { + err = provider.Enrich(ctx, b, document) + } + if errors.Is(err, ErrEnrichmentNotFound) { + if state.notFoundAttempts.Add(1) == maxNotFoundAttempts { + b.providerProgressVersion.Add(1) + log.Printf("Leaving document %s unresolved for %s after %d not-found responses: %s", + document.ID, provider.Source(), maxNotFoundAttempts, err) + } + } else if err == nil { + if state.notFoundAttempts.Swap(0) > 0 { + b.providerProgressVersion.Add(1) + } + } + return err +} + func (b *BleveIndexer) runDocumentEnrichWorker() { for id := range b.documentEnrichJobs { - for _, provider := range b.documentEnrichers { + for i, provider := range b.documentEnrichers { document, err := b.documentByIndexID(id) if err != nil { log.Printf("Error reading document %s for %s enrichment: %s", id, provider.Source(), err) @@ -146,10 +250,7 @@ func (b *BleveIndexer) runDocumentEnrichWorker() { if document.ID == "" { break } - needed, err := provider.NeedsEnrichment(document) - if err == nil && needed { - err = provider.Enrich(context.Background(), b, document) - } + err = b.enrichDocument(context.Background(), provider, b.documentEnrichTracker(i), document) if err != nil { log.Printf("Error enriching document %s with %s: %s", id, provider.Source(), err) } diff --git a/internal/index/bleve_document_enrich_test.go b/internal/index/bleve_document_enrich_test.go index 69fcc838..79713dad 100644 --- a/internal/index/bleve_document_enrich_test.go +++ b/internal/index/bleve_document_enrich_test.go @@ -23,6 +23,287 @@ type blockingDocumentEnricher struct { release chan struct{} } +type retryDocumentEnricher struct { + testDocumentEnricher + attempts chan time.Time + retries atomic.Int32 +} + +func (p *retryDocumentEnricher) Enrich(ctx context.Context, idx *BleveIndexer, document Document) error { + if document.ID == "retry" { + attempt := p.retries.Add(1) + p.attempts <- time.Now() + if attempt <= 3 { + return errors.New("temporary upstream failure") + } + } + return p.testDocumentEnricher.Enrich(ctx, idx, document) +} + +func TestDocumentEnrichmentAutomaticallyRetriesPendingLookups(t *testing.T) { + provider := &retryDocumentEnricher{ + testDocumentEnricher: testDocumentEnricher{source: "test"}, + attempts: make(chan time.Time, 10), + } + idx, _ := newTestTextRankIndexer(t, Config{DocumentEnrichers: []DocumentEnricher{provider}}) + t.Cleanup(func() { _ = idx.Close() }) + for _, id := range []string{"retry", "successful"} { + if err := IndexDocument(idx.documentsIdx, id, Document{ID: id}); err != nil { + t.Fatal(err) + } + } + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan struct{}) + go func() { + defer close(done) + idx.retryDocumentEnrichment(ctx, 1, 20*time.Millisecond, 40*time.Millisecond) + }() + defer func() { + cancel() + select { + case <-done: + case <-time.After(time.Second): + t.Error("retry worker did not stop on cancellation") + } + }() + var previous time.Time + for attempt := range 4 { + select { + case started := <-provider.attempts: + if attempt > 0 { + minimumDelay := time.Duration(min(attempt, 2)) * 20 * time.Millisecond + if started.Sub(previous) < minimumDelay { + t.Fatalf("retry %d ignored backoff: %s < %s", attempt, started.Sub(previous), minimumDelay) + } + } + previous = started + case <-time.After(2 * time.Second): + t.Fatal("pending lookup was not retried automatically") + } + } + deadline := time.Now().Add(time.Second) + for { + progress, err := idx.BackgroundProgress() + if err != nil { + t.Fatal(err) + } + if len(progress) == 0 { + break + } + if time.Now().After(deadline) { + t.Fatalf("progress did not complete after automatic recovery: %+v", progress) + } + time.Sleep(time.Millisecond) + } + time.Sleep(60 * time.Millisecond) + if provider.retries.Load() != 4 || provider.calls.Load() != 2 { + t.Fatalf("completed lookups were repeated: retries=%d completed=%d", provider.retries.Load(), provider.calls.Load()) + } + idx.documentsMu.Lock() + err := IndexDocument(idx.documentsIdx, "later-upload", Document{ID: "later-upload"}) + idx.documentsMu.Unlock() + if err != nil { + t.Fatal(err) + } + deadline = time.Now().Add(time.Second) + for provider.calls.Load() != 3 { + if time.Now().After(deadline) { + t.Fatal("automatic enrichment did not pick up a later pending document") + } + time.Sleep(time.Millisecond) + } +} + +func TestDocumentEnrichmentBoundsNotFoundRetries(t *testing.T) { + provider := &testDocumentEnricher{ + source: "test", failure: fmt.Errorf("HTTP 404: %w", ErrEnrichmentNotFound), failureID: "unresolved", + } + idx, _ := newTestTextRankIndexer(t, Config{DocumentEnrichers: []DocumentEnricher{provider}}) + t.Cleanup(func() { _ = idx.Close() }) + for _, id := range []string{"unresolved", "successful"} { + if err := IndexDocument(idx.documentsIdx, id, Document{ID: id}); err != nil { + t.Fatal(err) + } + } + for attempt := range 6 { + err := idx.EnrichDocuments(1) + if attempt < maxNotFoundAttempts && !errors.Is(err, ErrEnrichmentNotFound) { + t.Fatalf("attempt %d: expected not-found error, got %v", attempt, err) + } + if attempt >= maxNotFoundAttempts && err != nil { + t.Fatalf("exhausted documents must be excluded from later runs: %v", err) + } + } + if provider.calls.Load() != maxNotFoundAttempts+1 { + t.Fatalf("404s must stop after three attempts without repeating successful work: calls=%d", provider.calls.Load()) + } + progress, err := idx.BackgroundProgress() + if err != nil || len(progress) != 1 || progress[0].Unresolved != 1 || + progress[0].InProgress || progress[0].Failed || progress[0].RemainingTime != 0 || progress[0].Percentage != 50 { + t.Fatalf("expected unresolved warning without active work: %+v, %v", progress, err) + } + document, err := idx.documentByIndexID("unresolved") + if err != nil { + t.Fatal(err) + } + needed, err := provider.NeedsEnrichment(document) + if err != nil || !needed { + t.Fatalf("unresolved lookup must not be stored as successfully enriched: needed=%t, err=%v", needed, err) + } + // Upload and batch callers share the same limit. + if err := idx.enrichDocument(context.Background(), provider, idx.documentEnrichTracker(0), document); err == nil { + t.Fatal("queued work must not bypass the retry limit") + } + if provider.calls.Load() != maxNotFoundAttempts+1 { + t.Fatal("queued work bypassed the retry limit") + } + // A later successful on-demand lookup must remove the unresolved warning. + if err := idx.StoreDocumentData("unresolved", "test", map[string]any{"enriched": true}); err != nil { + t.Fatal(err) + } + progress, err = idx.BackgroundProgress() + if err != nil || len(progress) != 0 { + t.Fatalf("resolved lookup still reported as unresolved: %+v, %v", progress, err) + } +} + +func TestDocumentEnrichmentNotFoundCanRecoverBeforeLimit(t *testing.T) { + provider := &testDocumentEnricher{source: "test", failure: ErrEnrichmentNotFound} + idx, _ := newTestTextRankIndexer(t, Config{DocumentEnrichers: []DocumentEnricher{provider}}) + t.Cleanup(func() { _ = idx.Close() }) + if err := IndexDocument(idx.documentsIdx, "book", Document{ID: "book"}); err != nil { + t.Fatal(err) + } + for range 2 { + if err := idx.EnrichDocuments(1); !errors.Is(err, ErrEnrichmentNotFound) { + t.Fatalf("expected not-found error, got %v", err) + } + } + provider.failure = nil + if err := idx.EnrichDocuments(1); err != nil { + t.Fatal(err) + } + progress, err := idx.BackgroundProgress() + if err != nil || len(progress) != 0 || len(idx.documentEnrichTracker(0).unresolvedDocuments()) != 0 { + t.Fatalf("successful retry must clear pending progress: %+v, %v", progress, err) + } +} + +func TestDocumentEnrichmentOnlyCountsNotFoundResponses(t *testing.T) { + provider := &testDocumentEnricher{source: "test", failure: ErrEnrichmentNotFound} + idx, _ := newTestTextRankIndexer(t, Config{DocumentEnrichers: []DocumentEnricher{provider}}) + t.Cleanup(func() { _ = idx.Close() }) + if err := IndexDocument(idx.documentsIdx, "book", Document{ID: "book"}); err != nil { + t.Fatal(err) + } + if err := idx.EnrichDocuments(1); !errors.Is(err, ErrEnrichmentNotFound) { + t.Fatal(err) + } + unavailable := errors.New("all instances are in cooldown") + provider.failure = unavailable + for range 4 { + if err := idx.EnrichDocuments(1); !errors.Is(err, unavailable) { + t.Fatalf("cooldown must leave lookup retryable: %v", err) + } + } + if len(idx.documentEnrichTracker(0).unresolvedDocuments()) != 0 { + t.Fatal("failures without a 404 exhausted the not-found budget") + } + provider.failure = ErrEnrichmentNotFound + for range 2 { + if err := idx.EnrichDocuments(1); !errors.Is(err, ErrEnrichmentNotFound) { + t.Fatal(err) + } + } + if len(idx.documentEnrichTracker(0).unresolvedDocuments()) != 1 || provider.calls.Load() != 7 { + t.Fatal("lookup must become unresolved only after three actual not-found responses") + } +} + +func TestConcurrentDocumentEnrichmentSharesNotFoundLimit(t *testing.T) { + provider := &testDocumentEnricher{source: "test", failure: ErrEnrichmentNotFound} + idx, _ := newTestTextRankIndexer(t, Config{DocumentEnrichers: []DocumentEnricher{provider}}) + t.Cleanup(func() { _ = idx.Close() }) + documents := make([]Document, 2*documentEnrichWorkers) + for i := range documents { + documents[i] = Document{ID: "book"} + } + idx.enrichProviderBatch(context.Background(), provider, idx.documentEnrichTracker(0), documents) + if provider.calls.Load() != maxNotFoundAttempts { + t.Fatalf("concurrent callers bypassed the limit: %d attempts", provider.calls.Load()) + } +} + +func TestAutomaticDocumentEnrichmentStopsNotFoundRetries(t *testing.T) { + provider := &testDocumentEnricher{source: "test", failure: ErrEnrichmentNotFound} + idx, _ := newTestTextRankIndexer(t, Config{DocumentEnrichers: []DocumentEnricher{provider}}) + t.Cleanup(func() { _ = idx.Close() }) + if err := IndexDocument(idx.documentsIdx, "book", Document{ID: "book"}); err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan struct{}) + go func() { + defer close(done) + idx.retryDocumentEnrichment(ctx, 1, 5*time.Millisecond, 10*time.Millisecond) + }() + defer func() { + cancel() + select { + case <-done: + case <-time.After(time.Second): + t.Error("retry worker did not stop on cancellation") + } + }() + deadline := time.Now().Add(time.Second) + for { + progress, err := idx.BackgroundProgress() + if err != nil { + t.Fatal(err) + } + if len(progress) == 1 && progress[0].Unresolved == 1 { + break + } + if time.Now().After(deadline) { + t.Fatal("automatic retries did not reach the not-found limit") + } + time.Sleep(time.Millisecond) + } + time.Sleep(50 * time.Millisecond) + if provider.calls.Load() != maxNotFoundAttempts { + t.Fatalf("automatic retries exceeded three not-found attempts: %d", provider.calls.Load()) + } +} + +func TestCloseCancelsAutomaticDocumentEnrichment(t *testing.T) { + provider := &blockingDocumentEnricher{started: make(chan struct{}, 1), release: make(chan struct{})} + idx, _ := newTestTextRankIndexer(t, Config{DocumentEnrichers: []DocumentEnricher{provider}}) + if err := IndexDocument(idx.documentsIdx, "book", Document{ID: "book"}); err != nil { + t.Fatal(err) + } + if err := idx.StartDocumentEnrichment(0); err == nil { + t.Fatal("invalid batch size must be rejected") + } + if err := idx.StartDocumentEnrichment(1); err != nil { + t.Fatal(err) + } + select { + case <-provider.started: + case <-time.After(time.Second): + t.Fatal("automatic enrichment did not start") + } + done := make(chan error, 1) + go func() { done <- idx.Close() }() + select { + case err := <-done: + if err != nil { + t.Fatal(err) + } + case <-time.After(time.Second): + t.Fatal("closing the indexer did not cancel its active lookup") + } +} + func (*blockingDocumentEnricher) Source() string { return "blocking" } func (*blockingDocumentEnricher) Name() string { return "Blocking" } @@ -49,8 +330,11 @@ func TestDocumentEnrichmentUsesBoundedConcurrency(t *testing.T) { } idx, _ := newTestTextRankIndexer(t, Config{}) documents := make([]Document, documentCount) + for i := range documents { + documents[i] = Document{ID: fmt.Sprintf("book-%d", i)} + } done := make(chan map[string]error, 1) - go func() { done <- idx.enrichProviderBatch(provider, &progressTracker{}, documents) }() + go func() { done <- idx.enrichProviderBatch(context.Background(), provider, &progressTracker{}, documents) }() defer func() { close(provider.release) if failures := <-done; len(failures) != 0 { @@ -126,7 +410,8 @@ func TestBackgroundProgressTracksPersistedProviderLookups(t *testing.T) { t.Fatal("expected unavailable provider to fail") } progress, err = idx.BackgroundProgress() - if err != nil || len(progress) != 1 || progress[0].Name != "first" || progress[0].Percentage != 66 { + if err != nil || len(progress) != 1 || progress[0].Name != "first" || progress[0].Percentage != 66 || + !progress[0].Failed || progress[0].InProgress || progress[0].RemainingTime != 0 { t.Fatalf("failed lookups must remain visible; successful providers must disappear: %+v, %v", progress, err) } // Progress is reconstructed from stored data, not a running worker counter. @@ -228,6 +513,18 @@ func TestDocumentEnrichmentContinuesAfterDocumentFailure(t *testing.T) { if len(pending) != 1 || pending[0].ID != "book-2" { t.Fatalf("failed document should remain pending for a later retry: %+v", pending) } + progress, err := idx.BackgroundProgress() + if err != nil || len(progress) != 1 || !progress[0].Failed || progress[0].InProgress || progress[0].RemainingTime != 0 { + t.Fatalf("failed run must report stopped progress without an estimate: %+v, %v", progress, err) + } + provider.failure = nil + if err := idx.EnrichDocuments(1); err != nil { + t.Fatal(err) + } + progress, err = idx.BackgroundProgress() + if err != nil || len(progress) != 0 || idx.documentEnrichProgress[0].failed.Load() { + t.Fatalf("successful retry must clear the failed state: %+v, %v", progress, err) + } } func TestDocumentEnrichmentSupportsIndependentProviders(t *testing.T) { diff --git a/internal/index/bleve_document_write.go b/internal/index/bleve_document_write.go index 8e29a4b1..27c1fa2b 100644 --- a/internal/index/bleve_document_write.go +++ b/internal/index/bleve_document_write.go @@ -81,11 +81,19 @@ func (b *BleveIndexer) BackgroundProgress() ([]Progress, error) { } for i, counts := range providers { if counts.enriched < counts.total { + tracker := b.documentEnrichTracker(i) + pending := counts.total - counts.enriched - counts.unresolved + failed := tracker.failed.Load() && pending > 0 + var remaining time.Duration + if !failed { + remaining = tracker.remainingTime(pending) + } progress = append(progress, Progress{ - Kind: ProgressDataSources, InProgress: true, Name: b.documentEnrichers[i].Name(), + Kind: ProgressDataSources, InProgress: pending > 0 && !failed, Failed: failed, + Unresolved: counts.unresolved, Name: b.documentEnrichers[i].Name(), // Do not round pending work up to 100%. Percentage: math.Floor(100 * float64(counts.enriched) / float64(counts.total)), - RemainingTime: b.documentEnrichTracker(i).remainingTime(counts.total - counts.enriched), + RemainingTime: remaining, }) } } @@ -93,7 +101,7 @@ func (b *BleveIndexer) BackgroundProgress() ([]Progress, error) { } type providerProgressCount struct { - total, enriched uint64 + total, enriched, unresolved uint64 } // Cache index queries across page renders and polling clients; timing estimates @@ -112,14 +120,30 @@ func (b *BleveIndexer) providerProgressSnapshot() ([]providerProgressCount, erro return nil, fmt.Errorf("counting documents for enrichment progress: %w", err) } var counts []providerProgressCount - for _, provider := range b.documentEnrichers { + for i, provider := range b.documentEnrichers { query := bleve.NewTermQuery(provider.Source()) query.SetField("EnrichedDataSources") result, err := b.documentsIdx.Search(bleve.NewSearchRequestOptions(query, 0, 0, false)) if err != nil { return nil, fmt.Errorf("reading %s enrichment progress: %w", provider.Source(), err) } - counts = append(counts, providerProgressCount{total: total, enriched: result.Total}) + count := providerProgressCount{total: total, enriched: result.Total} + unresolved := b.documentEnrichTracker(i).unresolvedDocuments() + if len(unresolved) > 0 { + ids := make([]string, 0, len(unresolved)) + for id := range unresolved { + ids = append(ids, id) + } + pending := bleve.NewBooleanQuery() + pending.AddMust(bleve.NewDocIDQuery(ids)) + pending.AddMustNot(query) + result, err := b.documentsIdx.Search(bleve.NewSearchRequestOptions(pending, 0, 0, false)) + if err != nil { + return nil, fmt.Errorf("reading %s unresolved lookups: %w", provider.Source(), err) + } + count.unresolved = result.Total + } + counts = append(counts, count) } b.providerProgress = slices.Clone(counts) b.cachedProviderProgressVersion = version diff --git a/internal/index/progress.go b/internal/index/progress.go index 72948b28..3fce7082 100644 --- a/internal/index/progress.go +++ b/internal/index/progress.go @@ -15,6 +15,8 @@ const ( type Progress struct { Kind ProgressKind InProgress bool + Failed bool + Unresolved uint64 RemainingTime time.Duration Percentage float64 Name string diff --git a/internal/webserver/document_biblioreads_test.go b/internal/webserver/document_biblioreads_test.go index 0993d67d..d8270d02 100644 --- a/internal/webserver/document_biblioreads_test.go +++ b/internal/webserver/document_biblioreads_test.go @@ -110,8 +110,26 @@ func TestReviewEnrichmentProgressBanner(t *testing.T) { reviews.Find(`[role="progressbar"]`).AttrOr("aria-valuenow", "") != "66" { t.Fatalf("incorrect persisted lookup progress: %s", reviews.Text()) } + bar := reviews.Find(".progress-bar") + if !bar.HasClass("bg-info-subtle") || !bar.HasClass("text-body-emphasis") || bar.HasClass("bg-info") { + t.Fatal("review progress must use a theme-aware background with contrasting text") + } } checkPending() + for lang, label := range map[string]string{ + "en": "Retrieving ratings", + "es": "Recuperando valoraciones", + "de": "Bewertungen werden abgerufen", + "fr": "Récupération des évaluations", + "ru": "Получение оценок", + } { + localized := getPage(t, app, "/background-progress?l="+lang) + reviews := localized.Find(`[data-progress-kind="data-sources"]`) + if reviews.Find(`[role="progressbar"]`).AttrOr("aria-label", "") != label || + !strings.Contains(reviews.Find("p").First().Text(), label) { + t.Errorf("%s: review progress must use the localized retrieval label %q", lang, label) + } + } spanish := getPage(t, app, "/background-progress?l=es") if !strings.Contains(spanish.Find("#background-progress").Text(), "La información de valoraciones está incompleta") { t.Fatal("live progress must retain the selected language") @@ -120,6 +138,26 @@ func TestReviewEnrichmentProgressBanner(t *testing.T) { t.Fatal("expected unavailable source to fail") } checkPending() + for lang, label := range map[string]string{ + "en": "Waiting to retry ratings", + "es": "Esperando para reintentar la recuperación de valoraciones", + "de": "Warten auf erneuten Abruf der Bewertungen", + "fr": "En attente d'une nouvelle tentative de récupération des évaluations", + "ru": "Ожидание повторного получения оценок", + } { + localized := getPage(t, app, "/background-progress?l="+lang) + reviews := localized.Find(`[data-progress-kind="data-sources"]`) + if reviews.Find(`[role="progressbar"]`).AttrOr("aria-label", "") != label || + !strings.Contains(reviews.Find("p").First().Text(), label) { + t.Errorf("%s: failed lookups must show pending retries", lang) + } + if lang == "en" && !strings.Contains(reviews.Text(), "Pending lookups will be retried automatically.") { + t.Fatal("failed progress must explain how to retry") + } + if reviews.Find(`[role="progressbar"]`).AttrOr("aria-valuetext", "") != "" { + t.Fatal("stopped retrieval must not display a remaining time estimate") + } + } source.searchErr = nil if err := idx.EnrichDocuments(1); err != nil { t.Fatal(err) @@ -139,6 +177,41 @@ func TestReviewEnrichmentProgressBanner(t *testing.T) { } } +func TestReviewEnrichmentUnresolvedBanner(t *testing.T) { + source := &fakeBookSource{searchErr: biblioreads.ErrNotFound} + app, idx := newBiblioReadsTestApp(t, source) + for range 3 { + if err := idx.EnrichDocuments(1); !errors.Is(err, biblioreads.ErrNotFound) { + t.Fatalf("expected not-found lookup, got %v", err) + } + } + for lang, label := range map[string]string{ + "en": "Some ratings remain unresolved", + "es": "Algunas valoraciones siguen sin resolverse", + "de": "Einige Bewertungen bleiben ungeklärt", + "fr": "Certaines évaluations restent non résolues", + "ru": "Некоторые оценки остаются неполученными", + } { + page := getPage(t, app, "/background-progress?l="+lang) + reviews := page.Find(`[data-progress-kind="data-sources"]`) + if !strings.Contains(reviews.Find("p").First().Text(), label) { + t.Errorf("%s: expected localized unresolved warning: %s", lang, reviews.Text()) + } + if reviews.Find(`[role="progressbar"]`).Length() != 0 { + t.Errorf("%s: unresolved lookups must not leave a stuck progress bar", lang) + } + if strings.Contains(reviews.Text(), "retried automatically") { + t.Errorf("%s: exhausted lookups must not promise further automatic retries", lang) + } + if lang == "en" && !strings.Contains(reviews.Text(), "Rating lookups unresolved after three not-found responses: 1.") { + t.Fatal("unresolved warning must include its count and explain the retry limit") + } + } + if err := idx.EnrichDocuments(1); err != nil { + t.Fatalf("exhausted lookup should not block later runs: %v", err) + } +} + func newDataSourceProvidersTestApp(t *testing.T, source *fakeBookSource, provider document.ReviewProvider, dataSourceLinkProvider authorcontroller.DataSourceLinkProvider) (*fiber.App, *index.BleveIndexer) { t.Helper() db := infrastructure.Connect(":memory:", 250) diff --git a/internal/webserver/embedded/js/search-filter-utils.js b/internal/webserver/embedded/js/search-filter-utils.js index cf94a027..2793c7d1 100644 --- a/internal/webserver/embedded/js/search-filter-utils.js +++ b/internal/webserver/embedded/js/search-filter-utils.js @@ -197,15 +197,20 @@ export function initDateControls(searchFilters, searchFiltersForm) { } export function initClearRangeControls(searchFilters) { + function updateClearButtons() { + searchFilters.querySelectorAll('.clear-range-control').forEach(clearLink => { + const group = clearLink.closest('.input-group') + if (group) updateRangeClearButtonState(group) + }) + } + + searchFilters.addEventListener('input', updateClearButtons) + searchFilters.addEventListener('change', updateClearButtons) + searchFilters.querySelectorAll('.clear-range-control').forEach(clearLink => { const group = clearLink.closest('.input-group') if (!group) return - group.querySelectorAll('input, select').forEach(input => { - input.addEventListener('input', () => updateRangeClearButtonState(group)) - input.addEventListener('change', () => updateRangeClearButtonState(group)) - }) - clearLink.addEventListener('click', (e) => { e.preventDefault() resetRangeGroup(group) diff --git a/internal/webserver/embedded/translations/de.yml b/internal/webserver/embedded/translations/de.yml index 5e32fd4b..4796de4f 100644 --- a/internal/webserver/embedded/translations/de.yml +++ b/internal/webserver/embedded/translations/de.yml @@ -426,6 +426,10 @@ "No score": "Keine Bewertung" "Not scored": "Nicht bewertet" "%s ratings": "%s Bewertungen" -"Review information progress": "Fortschritt der Bewertungsinformationen" +"Retrieving ratings": "Bewertungen werden abgerufen" +"Waiting to retry ratings": "Warten auf erneuten Abruf der Bewertungen" +"Some ratings remain unresolved": "Einige Bewertungen bleiben ungeklärt" +"Rating lookups unresolved after three not-found responses: %d. Restart Coreander to retry them.": "Ungeklärte Bewertungsabfragen nach drei Nicht-gefunden-Antworten: %d. Starte Coreander neu, um sie erneut zu versuchen." +"Some ratings could not be retrieved. Pending lookups will be retried automatically.": "Einige Bewertungen konnten nicht abgerufen werden. Ausstehende Abfragen werden automatisch wiederholt." "Review information is incomplete. Score filters may omit documents whose ratings have not been retrieved.": "Bewertungsinformationen sind unvollständig. Bewertungsfilter können Dokumente auslassen, deren Bewertungen noch nicht abgerufen wurden." "Unable to retrieve background progress.": "Der Fortschritt der Hintergrundaufgaben konnte nicht abgerufen werden." diff --git a/internal/webserver/embedded/translations/es.yml b/internal/webserver/embedded/translations/es.yml index bcfea15c..dbc453a4 100644 --- a/internal/webserver/embedded/translations/es.yml +++ b/internal/webserver/embedded/translations/es.yml @@ -422,6 +422,10 @@ "No score": "Sin puntuación" "Not scored": "Sin puntuar" "%s ratings": "%s valoraciones" -"Review information progress": "Progreso de la información de valoraciones" +"Retrieving ratings": "Recuperando valoraciones" +"Waiting to retry ratings": "Esperando para reintentar la recuperación de valoraciones" +"Some ratings remain unresolved": "Algunas valoraciones siguen sin resolverse" +"Rating lookups unresolved after three not-found responses: %d. Restart Coreander to retry them.": "Búsquedas de valoraciones sin resolver tras tres respuestas de no encontrado: %d. Reinicia Coreander para reintentarlas." +"Some ratings could not be retrieved. Pending lookups will be retried automatically.": "No se pudieron recuperar algunas valoraciones. Las búsquedas pendientes se reintentarán automáticamente." "Review information is incomplete. Score filters may omit documents whose ratings have not been retrieved.": "La información de valoraciones está incompleta. Los filtros de puntuación pueden omitir documentos cuyas valoraciones aún no se han obtenido." "Unable to retrieve background progress.": "No se pudo obtener el progreso de las tareas en segundo plano." diff --git a/internal/webserver/embedded/translations/fr.yml b/internal/webserver/embedded/translations/fr.yml index 1346ad3f..3203229e 100644 --- a/internal/webserver/embedded/translations/fr.yml +++ b/internal/webserver/embedded/translations/fr.yml @@ -424,6 +424,10 @@ "No score": "Aucune note" "Not scored": "Non noté" "%s ratings": "%s notes" -"Review information progress": "Progression des informations d'évaluation" +"Retrieving ratings": "Récupération des évaluations" +"Waiting to retry ratings": "En attente d'une nouvelle tentative de récupération des évaluations" +"Some ratings remain unresolved": "Certaines évaluations restent non résolues" +"Rating lookups unresolved after three not-found responses: %d. Restart Coreander to retry them.": "Recherches d'évaluations non résolues après trois réponses introuvables : %d. Redémarrez Coreander pour les réessayer." +"Some ratings could not be retrieved. Pending lookups will be retried automatically.": "Certaines évaluations n'ont pas pu être récupérées. Les recherches en attente seront réessayées automatiquement." "Review information is incomplete. Score filters may omit documents whose ratings have not been retrieved.": "Les informations d'évaluation sont incomplètes. Les filtres de note peuvent omettre les documents dont les évaluations n'ont pas encore été récupérées." "Unable to retrieve background progress.": "Impossible de récupérer la progression des tâches en arrière-plan." diff --git a/internal/webserver/embedded/translations/ru.yml b/internal/webserver/embedded/translations/ru.yml index 7fd89c59..4425a1e0 100644 --- a/internal/webserver/embedded/translations/ru.yml +++ b/internal/webserver/embedded/translations/ru.yml @@ -418,6 +418,10 @@ "No score": "Нет оценки" "Not scored": "Не оценено" "%s ratings": "Оценок: %s" -"Review information progress": "Ход получения информации об оценках" +"Retrieving ratings": "Получение оценок" +"Waiting to retry ratings": "Ожидание повторного получения оценок" +"Some ratings remain unresolved": "Некоторые оценки остаются неполученными" +"Rating lookups unresolved after three not-found responses: %d. Restart Coreander to retry them.": "Неразрешённые запросы оценок после трёх ответов «не найдено»: %d. Перезапустите Coreander, чтобы повторить их." +"Some ratings could not be retrieved. Pending lookups will be retried automatically.": "Не удалось получить некоторые оценки. Оставшиеся запросы будут повторены автоматически." "Review information is incomplete. Score filters may omit documents whose ratings have not been retrieved.": "Информация об оценках неполная. Фильтры по оценкам могут пропускать документы, оценки которых ещё не получены." "Unable to retrieve background progress.": "Не удалось получить состояние фоновых задач." diff --git a/internal/webserver/embedded/views/partials/background-progress.html b/internal/webserver/embedded/views/partials/background-progress.html index 45ee5747..f045c849 100644 --- a/internal/webserver/embedded/views/partials/background-progress.html +++ b/internal/webserver/embedded/views/partials/background-progress.html @@ -11,15 +11,26 @@ {{if eq .Kind "authors"}}{{$label = "Author information progress"}} {{else if eq .Kind "textrank"}}{{$label = "Related content analysis progress"}} {{else if eq .Kind "pruning"}}{{$label = "Keyword clean-up progress"}} - {{else if eq .Kind "data-sources"}}{{$label = "Review information progress"}} + {{else if eq .Kind "data-sources"}} + {{$label = "Retrieving ratings"}} + {{if .Failed}}{{$label = "Waiting to retry ratings"}}{{end}} + {{if and (gt .Unresolved 0) (not .InProgress) (not .Failed)}}{{$label = "Some ratings remain unresolved"}}{{end}} {{end}}

{{t $.Lang $label}}{{if .Name}} ({{.Name}}){{end}}

{{$remaining := ""}}{{if gt .RemainingTime 0}}{{$remaining = t $.Lang "%s left" (remainingTime .RemainingTime)}}{{end}} + {{if or .InProgress .Failed}}
-
{{.Percentage}}%{{if $remaining}} ({{$remaining}}){{end}}
+
{{.Percentage}}%{{if $remaining}} ({{$remaining}}){{end}}
+ {{end}} {{if eq .Kind "data-sources"}} + {{if gt .Unresolved 0}} +

{{t $.Lang "Rating lookups unresolved after three not-found responses: %d. Restart Coreander to retry them." .Unresolved}}

+ {{end}} + {{if .Failed}} +

{{t $.Lang "Some ratings could not be retrieved. Pending lookups will be retried automatically."}}

+ {{end}}

{{t $.Lang "Review information is incomplete. Score filters may omit documents whose ratings have not been retrieved."}}

{{else if eq .Kind "authors"}}

{{t $.Lang "Retrieving author information. Author pages may be incomplete, and author filters may not work yet."}}

diff --git a/internal/webserver/js-tests/search-filter-utils.test.mjs b/internal/webserver/js-tests/search-filter-utils.test.mjs index 85063ed5..67d7ec32 100644 --- a/internal/webserver/js-tests/search-filter-utils.test.mjs +++ b/internal/webserver/js-tests/search-filter-utils.test.mjs @@ -20,9 +20,13 @@ for (const tag of ['input', 'select']) { : selector.split(', ').includes(tag) ? controls[0] : null, } button.closest = () => group - const filters = { - querySelectorAll: selector => selector === '.clear-range-control' ? [button] : [], - } + const filters = new EventTarget() + filters.querySelectorAll = selector => selector === '.clear-range-control' ? [button] : [] + controls.forEach(control => { + for (const type of ['input', 'change']) { + control.addEventListener(type, () => filters.dispatchEvent(new Event(type))) + } + }) const reset = initClearRangeControls(filters) assert.equal(button.disabled, true) @@ -54,3 +58,26 @@ for (const tag of ['input', 'select']) { assert.equal(button.disabled, true) }) } + +test('score range clear button refreshes after form values are synchronized', () => { + const controls = [ + { value: '', tagName: 'SELECT' }, + { value: '', tagName: 'SELECT' }, + ] + const button = new EventTarget() + const group = { + querySelectorAll: () => controls, + querySelector: () => button, + } + button.closest = () => group + const filters = new EventTarget() + filters.querySelectorAll = () => [button] + initClearRangeControls(filters) + assert.equal(button.disabled, true) + + for (const value of ['0', '3', '5', '']) { + controls[1].value = value + filters.dispatchEvent(new Event('input')) + assert.equal(button.disabled, value === '') + } +}) diff --git a/main.go b/main.go index ffaffb42..4858abe5 100644 --- a/main.go +++ b/main.go @@ -241,11 +241,9 @@ func startIndex(idx *index.BleveIndexer, batchSize int, libPath string, indexWor log.Printf("Indexing finished, took %d seconds", int(dur.Seconds())) // Network-bound, so it runs alongside the CPU-bound TextRank enrichment. - go func() { - if err := idx.EnrichDocuments(batchSize); err != nil { - log.Printf("Error enriching documents with provider data: %s", err) - } - }() + if err := idx.StartDocumentEnrichment(batchSize); err != nil { + log.Printf("Error starting document enrichment: %s", err) + } if err := idx.EnrichTextRankKeywords(batchSize, indexWorkers); err != nil { log.Printf("Error enriching documents with TextRank keywords: %s", err) From c11bded3bee43da5b564965d06ac278ba8eecdc0 Mon Sep 17 00:00:00 2001 From: Sergio Vera Date: Fri, 6 Nov 2026 16:38:10 +0100 Subject: [PATCH 2/5] Simplify --- README.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/README.md b/README.md index 90b41b8d..191b0100 100644 --- a/README.md +++ b/README.md @@ -98,8 +98,6 @@ coreander --review-source-instances=https://br.example.org,https://br2.example.o Alternatively, set `REVIEW_SOURCE_INSTANCES` to the same comma-separated list. The flag may also be repeated and overrides the environment variable. Instance URLs must use HTTP or HTTPS and serve the BiblioReads API. -BiblioReads can return an "Invalid Query" search error for a temporary upstream failure. Coreander tries the same query on another configured instance, if available; it does not broaden the search in response to an error. Failed lookups do not stop other documents or get marked as completed. Pending lookups are retried automatically after one minute, with exponential backoff up to 15 minutes between failed runs, while respecting instance cooldowns. HTTP 404 responses may also be legitimate: after three lookup attempts returning 404s for a document during the current process, automatic retries for that document stop. A lookup attempt may try multiple configured instances. These documents are reported as unresolved, not as successfully enriched; restart Coreander to retry them. Requests skipped because every instance is in cooldown do not count toward the limit. Other transient failures continue to retry. The progress banner distinguishes pending retries from unresolved lookups and removes the progress bar when only unresolved documents remain. Completed lookups are not repeated; pending lookups from later uploads are also checked every minute after successful runs. - ### Email Some features rely on having an SMTP email service set up, and won't be available otherwise: From 14d5c80be5a46a2ed4d4bd2085dbe379c36b7b37 Mon Sep 17 00:00:00 2001 From: Sergio Vera Date: Fri, 6 Nov 2026 16:38:17 +0100 Subject: [PATCH 3/5] Simplify --- internal/datasource/biblioreads/adapter.go | 5 +- .../datasource/biblioreads/adapter_test.go | 30 -- internal/datasource/biblioreads/client.go | 55 ++-- .../datasource/biblioreads/client_test.go | 103 +++++-- internal/index/bleve.go | 13 - internal/index/bleve_document_enrich.go | 126 +------- internal/index/bleve_document_enrich_test.go | 283 +----------------- internal/index/bleve_document_write.go | 30 +- internal/index/progress.go | 1 - .../webserver/document_biblioreads_test.go | 57 +--- .../webserver/embedded/translations/de.yml | 4 +- .../webserver/embedded/translations/es.yml | 4 +- .../webserver/embedded/translations/fr.yml | 4 +- .../webserver/embedded/translations/ru.yml | 4 +- .../views/partials/background-progress.html | 10 +- main.go | 8 +- 16 files changed, 152 insertions(+), 585 deletions(-) diff --git a/internal/datasource/biblioreads/adapter.go b/internal/datasource/biblioreads/adapter.go index 28babdc9..7315e3aa 100644 --- a/internal/datasource/biblioreads/adapter.go +++ b/internal/datasource/biblioreads/adapter.go @@ -14,7 +14,7 @@ const ( // maxConcurrentLookups bounds lookups across every enrichment caller, so they do not pile up // waiting on the client. Client request spacing and cooldowns protect each instance. maxConcurrentLookups = 4 - requestTimeout = 2 * time.Minute + requestTimeout = 3 * time.Minute ) type SearchSource interface { @@ -48,9 +48,6 @@ func (*Enricher) NeedsEnrichment(document index.Document) (bool, error) { func (p *Enricher) Enrich(ctx context.Context, idx *index.BleveIndexer, document index.Document) error { _, err := p.enrich(ctx, idx, document) - if errors.Is(err, ErrNotFound) { - return fmt.Errorf("%w: %w", index.ErrEnrichmentNotFound, err) - } return err } diff --git a/internal/datasource/biblioreads/adapter_test.go b/internal/datasource/biblioreads/adapter_test.go index e72cfb8c..ef7fb4a9 100644 --- a/internal/datasource/biblioreads/adapter_test.go +++ b/internal/datasource/biblioreads/adapter_test.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "errors" - "fmt" "image" "reflect" "slices" @@ -340,35 +339,6 @@ func TestEnrichLeavesDocumentsForLaterWhenUnavailable(t *testing.T) { } } -func TestEnrichBoundsNotFoundLookups(t *testing.T) { - source := &fakeSource{err: fmt.Errorf("%w: %w", ErrUnavailable, ErrNotFound)} - idx := newTestIndexer(t, source) - doc := index.Document{ID: testID, Metadata: metadata.Metadata{Title: "El economista camuflado", Authors: []string{"Tim Harford"}}} - if err := index.IndexDocument(idx.documentsIdx, doc.ID, doc); err != nil { - t.Fatal(err) - } - for attempt := range 5 { - err := idx.EnrichDocuments(1) - if attempt < 3 && (!errors.Is(err, ErrNotFound) || !errors.Is(err, index.ErrEnrichmentNotFound)) { - t.Fatalf("not-found classification must reach the retry scheduler: %v", err) - } - if attempt >= 3 && err != nil { - t.Fatalf("exhausted lookup must not be retried automatically: %v", err) - } - } - if queries := source.Queries(); !slices.Equal(queries, []string{ - "El economista camuflado Tim Harford", - "El economista camuflado Tim Harford", - "El economista camuflado Tim Harford", - }) { - t.Fatalf("expected exactly three attempts of the original query: %q", queries) - } - stored, err := idx.DocumentByID(doc.ID) - if err != nil || mustData(t, stored).Enriched { - t.Fatalf("exhausted lookups must remain unresolved: %+v, %v", stored, err) - } -} - func TestNewFileSchedulesEnrichment(t *testing.T) { source := &fakeSource{results: testResults} idx := newTestIndexer(t, source) diff --git a/internal/datasource/biblioreads/client.go b/internal/datasource/biblioreads/client.go index 12d2a92d..db15e3dc 100644 --- a/internal/datasource/biblioreads/client.go +++ b/internal/datasource/biblioreads/client.go @@ -32,9 +32,6 @@ const ( // non-JSON responses), as opposed to an instance answering with an error of its own. var ErrUnavailable = errors.New("biblioreads: all instances failed") -// ErrNotFound identifies HTTP 404 responses, which may be legitimate or an upstream failure. -var ErrNotFound = errors.New("biblioreads: not found") - type Client struct { instances []string httpClient *http.Client @@ -92,7 +89,8 @@ func WithCacheTTL(ttl time.Duration) Option { // instances ready to accept a request and randomizing ties. Requests to each // instance are spaced at least one second apart by default, including failover // attempts. Unavailable instances are skipped until their Retry-After time, or -// httpjson.DefaultRetryAfter, has passed. Successful responses are cached in memory for +// httpjson.DefaultRetryAfter, has passed. Book searches wait if all instances are cooling down. +// Successful responses are cached in memory for // DefaultCacheTTL unless changed with WithCacheTTL. No endpoints are configured // by default; callers must supply them with WithInstances. func NewClient(opts ...Option) *Client { @@ -134,7 +132,7 @@ type reviewsPayload struct { LanguageCode string `json:"languageCode,omitempty"` } -// SearchBooks searches Goodreads books matching query. +// SearchBooks searches Goodreads books matching query, with at most three attempts on retryable errors. func (c *Client) SearchBooks(ctx context.Context, query string) (SearchBooksResponse, error) { var res SearchBooksResponse payload := queryURLPayload{ @@ -215,8 +213,15 @@ func (c *Client) post(ctx context.Context, path string, payload, dest any) error var errs []error var incompleteResponse []byte tried := make(map[string]bool) - for len(tried) < len(c.instances) { - instance, err := c.acquireInstance(ctx, tried) + attempts := len(c.instances) + if path == searchBooksPath { + attempts = 3 + } + for range attempts { + if len(tried) == len(c.instances) { + clear(tried) + } + instance, err := c.acquireInstance(ctx, tried, path == searchBooksPath) if err != nil { if ctx.Err() != nil { return ctx.Err() @@ -313,7 +318,7 @@ func supportsReviewLanguage(version string) bool { // acquireInstance rechecks availability after waiting, so cancellation does not // reserve a future slot and concurrent requests cannot take the same slot. -func (c *Client) acquireInstance(ctx context.Context, tried map[string]bool) (string, error) { +func (c *Client) acquireInstance(ctx context.Context, tried map[string]bool, waitForCooldown bool) (string, error) { for { if err := ctx.Err(); err != nil { return "", err @@ -321,19 +326,27 @@ func (c *Client) acquireInstance(ctx context.Context, tried map[string]bool) (st c.mu.Lock() start := rand.IntN(len(c.instances)) instance := "" + var readyAt time.Time now := time.Now() for i := range c.instances { candidate := c.instances[(start+i)%len(c.instances)] - if !tried[candidate] && !now.Before(c.cooldownUntil[candidate]) && - (instance == "" || c.nextRequest[candidate].Before(c.nextRequest[instance])) { + if tried[candidate] || (!waitForCooldown && now.Before(c.cooldownUntil[candidate])) { + continue + } + ready := c.nextRequest[candidate] + if c.cooldownUntil[candidate].After(ready) { + ready = c.cooldownUntil[candidate] + } + if instance == "" || ready.Before(readyAt) { instance = candidate + readyAt = ready } } if instance == "" { c.mu.Unlock() return "", fmt.Errorf("%w: no untried instance is outside its cooldown", ErrUnavailable) } - wait := time.Until(c.nextRequest[instance]) + wait := time.Until(readyAt) if wait <= 0 { c.nextRequest[instance] = time.Now().Add(c.requestInterval) c.mu.Unlock() @@ -386,31 +399,19 @@ func (c *Client) postToInstance(ctx context.Context, endpoint string, body []byt // BiblioReads always answers with JSON, even on errors (e.g. a 504 for a book that // doesn't exist), so anything else comes from a broken instance or a proxy in front of it. if !json.Valid(resp.Body) { - reason := fmt.Errorf("status code %d with non-JSON response", resp.StatusCode) - if resp.StatusCode == http.StatusNotFound { - reason = fmt.Errorf("%w: %w", ErrNotFound, reason) - } return nil, &unavailableError{ endpoint: endpoint, retryAfter: httpjson.DefaultRetryAfter, - reason: reason, + reason: fmt.Errorf("status code %d with non-JSON response", resp.StatusCode), } } if resp.StatusCode != http.StatusOK { responseErr := fmt.Errorf("biblioreads: request to %s failed with status code %d: %s", endpoint, resp.StatusCode, resp.Body) - if resp.StatusCode == http.StatusNotFound { - responseErr = fmt.Errorf("%w: %w", ErrNotFound, responseErr) - } if resp.StatusCode == http.StatusNotFound && strings.HasSuffix(endpoint, searchBooksPath) { - var response struct { - Status string `json:"status"` - } - if err := json.Unmarshal(resp.Body, &response); err == nil && response.Status == "Error - Invalid Query" { - return nil, &unavailableError{ - endpoint: endpoint, retryAfter: httpjson.DefaultRetryAfter, - reason: responseErr, - } + return nil, &unavailableError{ + endpoint: endpoint, retryAfter: httpjson.ParseRetryAfter(resp.Header.Get("Retry-After")), + reason: responseErr, } } return nil, responseErr diff --git a/internal/datasource/biblioreads/client_test.go b/internal/datasource/biblioreads/client_test.go index 32355282..4cc975b1 100644 --- a/internal/datasource/biblioreads/client_test.go +++ b/internal/datasource/biblioreads/client_test.go @@ -4,12 +4,15 @@ import ( "context" "encoding/json" "errors" + "io" "net/http" "net/http/httptest" "os" "reflect" + "strings" "sync/atomic" "testing" + "testing/synctest" "time" "github.com/svera/coreander/v5/internal/datasource/biblioreads" @@ -393,41 +396,95 @@ func TestErrorStatus(t *testing.T) { } } -func TestInvalidSearchQuery(t *testing.T) { +type searchTransport func(*http.Request) (*http.Response, error) + +func (transport searchTransport) RoundTrip(request *http.Request) (*http.Response, error) { + return transport(request) +} + +func TestSearchRetries(t *testing.T) { + fixture := string(readFixture(t, "search-books.json")) for _, tt := range []struct { name string status int body string unavailable bool + attempts int + recover bool }{ - {"invalid query", http.StatusNotFound, `{"status":"Error - Invalid Query","version":"BiblioReads v4.4.0"}`, true}, - {"other not found", http.StatusNotFound, `{"status":"Error - Book not found"}`, false}, - {"other client error", http.StatusBadRequest, `{"status":"Error - Invalid Query"}`, false}, - {"server error", http.StatusServiceUnavailable, `{"status":"Error - Invalid Query"}`, true}, + {"invalid query", http.StatusNotFound, `{"status":"Error - Invalid Query"}`, true, 3, false}, + {"legitimate not found", http.StatusNotFound, `{"status":"Error - Book not found"}`, true, 3, false}, + {"non-JSON not found", http.StatusNotFound, "Not found", true, 3, false}, + {"other client error", http.StatusBadRequest, `{"status":"Error - Invalid Query"}`, false, 1, false}, + {"server error", http.StatusServiceUnavailable, `{"status":"Error - Invalid Query"}`, true, 3, false}, + {"rate limit", http.StatusTooManyRequests, `{}`, true, 3, false}, + {"recovery", http.StatusNotFound, `{"status":"Error - Invalid Query"}`, false, 3, true}, + {"empty success", http.StatusOK, `{"status":"Received","result":[]}`, false, 1, false}, } { t.Run(tt.name, func(t *testing.T) { - var hits atomic.Int32 - server := newCountingServer(t, tt.status, tt.body, &hits) - client := newTestClient(biblioreads.WithInstances([]string{server.URL})) - _, err := client.SearchBooks(context.Background(), "El economista camuflado Tim Harford") - if err == nil || errors.Is(err, biblioreads.ErrUnavailable) != tt.unavailable { - t.Fatalf("unexpected search error classification: %v", err) - } - if errors.Is(err, biblioreads.ErrNotFound) != (tt.status == http.StatusNotFound) { - t.Fatalf("HTTP 404 classification must survive failover wrapping: %v", err) - } + synctest.Test(t, func(t *testing.T) { + attempts := 0 + client := newTestClient(biblioreads.WithInstances([]string{"https://example.test"}), + biblioreads.WithHTTPClient(&http.Client{Transport: searchTransport(func(r *http.Request) (*http.Response, error) { + var captured capturedRequest + if !captureRequest(t, httptest.NewRecorder(), r, &captured) { + t.Fatal("invalid search request") + } + assertRequest(t, captured, "/api/search/books", map[string]any{ + "queryURL": "https://www.goodreads.com/search?q=El%20economista%20camuflado%20Tim%20Harford", + }) + attempts++ + status, body := tt.status, tt.body + if tt.recover && attempts == 3 { + status, body = http.StatusOK, fixture + } + header := make(http.Header) + if status == http.StatusTooManyRequests { + header.Set("Retry-After", "1") + } + return &http.Response{StatusCode: status, Header: header, Body: io.NopCloser(strings.NewReader(body))}, nil + })})) + start := time.Now() + response, err := client.SearchBooks(context.Background(), "El economista camuflado Tim Harford") + wantError := !tt.recover && tt.status != http.StatusOK + if (err != nil) != wantError || errors.Is(err, biblioreads.ErrUnavailable) != tt.unavailable { + t.Fatalf("unexpected search error classification: %v", err) + } + if attempts != tt.attempts { + t.Fatalf("expected %d attempts, got %d", tt.attempts, attempts) + } + cooldown := time.Minute + if tt.status == http.StatusTooManyRequests { + cooldown = time.Second + } + if attempts == 3 && time.Since(start) != 2*cooldown { + t.Fatalf("retries must respect the instance cooldown: %s", time.Since(start)) + } + if tt.recover && len(response.Result) == 0 { + t.Fatal("successful retry must return its results") + } + }) }) } } -func TestNonJSONNotFoundClassification(t *testing.T) { - var hits atomic.Int32 - server := newCountingServer(t, http.StatusNotFound, "Not found", &hits) - client := newTestClient(biblioreads.WithInstances([]string{server.URL})) - _, err := client.SearchBooks(context.Background(), "book") - if !errors.Is(err, biblioreads.ErrNotFound) || !errors.Is(err, biblioreads.ErrUnavailable) { - t.Fatalf("non-JSON 404 must retain its HTTP status classification: %v", err) - } +func TestSearchRetryWaitHonorsCancellation(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + attempts := 0 + client := newTestClient(biblioreads.WithInstances([]string{"https://example.test"}), + biblioreads.WithHTTPClient(&http.Client{Transport: searchTransport(func(*http.Request) (*http.Response, error) { + attempts++ + return &http.Response{StatusCode: http.StatusNotFound, Header: make(http.Header), + Body: io.NopCloser(strings.NewReader(`{"status":"Error - Invalid Query"}`))}, nil + })})) + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + start := time.Now() + _, err := client.SearchBooks(ctx, "book") + if !errors.Is(err, context.DeadlineExceeded) || attempts != 1 || time.Since(start) != 10*time.Second { + t.Fatalf("cancellation must interrupt cooldown: attempts=%d, elapsed=%s, err=%v", attempts, time.Since(start), err) + } + }) } func TestSearchInvalidQueryFailsOverWithoutChangingQuery(t *testing.T) { diff --git a/internal/index/bleve.go b/internal/index/bleve.go index a613127b..0841a673 100644 --- a/internal/index/bleve.go +++ b/internal/index/bleve.go @@ -1,7 +1,6 @@ package index import ( - "context" "errors" "log" "path/filepath" @@ -222,10 +221,6 @@ type BleveIndexer struct { documentEnrichers []DocumentEnricher // documentEnrichProgress measures each provider's current run rate, by enricher index. documentEnrichProgress []progressTracker - documentEnrichContext context.Context - documentEnrichCancel context.CancelFunc - documentEnrichStart sync.Once - documentEnrichWG sync.WaitGroup providerProgressMu sync.Mutex providerProgressUntil time.Time providerProgress []providerProgressCount @@ -250,7 +245,6 @@ type progressTracker struct { processed atomic.Uint64 total atomic.Uint64 failed atomic.Bool - lookups sync.Map // Document ID -> *documentLookupState for provider retry limits. } // begin marks the phase as started, resetting processed to 0 and total to @@ -331,7 +325,6 @@ func NewBleve(documentsIndex bleve.Index, authorsIndex bleve.Index, fs afero.Fs, textRankEnrichWorkers = 1 } - enrichContext, enrichCancel := context.WithCancel(context.Background()) b := &BleveIndexer{ fs: fs, documentsIdx: documentsIndex, @@ -351,8 +344,6 @@ func NewBleve(documentsIndex bleve.Index, authorsIndex bleve.Index, fs afero.Fs, textRankEnrichJobs: make(chan Document, textRankEnrichWorkers), documentEnrichers: slices.Clone(cfg.DocumentEnrichers), documentEnrichProgress: make([]progressTracker, len(cfg.DocumentEnrichers)), - documentEnrichContext: enrichContext, - documentEnrichCancel: enrichCancel, } for range textRankEnrichWorkers { @@ -573,10 +564,6 @@ func CreateAuthorsMapping() mapping.IndexMapping { // Close closes both indexes func (b *BleveIndexer) Close() error { - if b.documentEnrichCancel != nil { - b.documentEnrichCancel() - } - b.documentEnrichWG.Wait() b.documentsMu.Lock() defer b.documentsMu.Unlock() b.authorsMu.Lock() diff --git a/internal/index/bleve_document_enrich.go b/internal/index/bleve_document_enrich.go index 35d4356c..b048c44d 100644 --- a/internal/index/bleve_document_enrich.go +++ b/internal/index/bleve_document_enrich.go @@ -6,8 +6,6 @@ import ( "fmt" "log" "sync" - "sync/atomic" - "time" "github.com/blevesearch/bleve/v2" ) @@ -16,28 +14,6 @@ import ( // their own source request limits because batch and upload pools run concurrently. const documentEnrichWorkers = 4 -const maxNotFoundAttempts = 3 - -// ErrEnrichmentNotFound marks an ambiguous provider 404, not a successful no-match lookup. -var ErrEnrichmentNotFound = errors.New("provider lookup returned not found") - -type documentLookupState struct { - mu sync.Mutex - notFoundAttempts atomic.Uint32 -} - -func (p *progressTracker) unresolvedDocuments() map[string]struct{} { - unresolved := make(map[string]struct{}) - p.lookups.Range(func(key, value any) bool { - state := value.(*documentLookupState) - if state.notFoundAttempts.Load() >= maxNotFoundAttempts { - unresolved[key.(string)] = struct{}{} - } - return true - }) - return unresolved -} - // DocumentEnricher owns a provider's lookup policy and persisted payload. type DocumentEnricher interface { // Source is the persisted storage key; Name is shown to users. @@ -81,60 +57,12 @@ func (b *BleveIndexer) documentsNeedingProvider(source string, size int, exclude // EnrichDocuments resumes pending lookups for every configured provider. // A provider failure does not prevent independent providers from running. func (b *BleveIndexer) EnrichDocuments(batchSize int) error { - return b.enrichDocuments(context.Background(), batchSize) -} - -// StartDocumentEnrichment retries pending lookups until the indexer is closed. -func (b *BleveIndexer) StartDocumentEnrichment(batchSize int) error { - if batchSize <= 0 { - return fmt.Errorf("document enrichment batch size must be positive") - } - if len(b.documentEnrichers) == 0 { - return nil - } - b.documentEnrichStart.Do(func() { - b.documentEnrichWG.Go(func() { - b.retryDocumentEnrichment(b.documentEnrichContext, batchSize, time.Minute, 15*time.Minute) - }) - }) - return nil -} - -func (b *BleveIndexer) retryDocumentEnrichment(ctx context.Context, batchSize int, interval, maxDelay time.Duration) { - delay := interval - for ctx.Err() == nil { - err := b.enrichDocuments(ctx, batchSize) - if ctx.Err() != nil { - return - } - if err != nil { - log.Printf("Error enriching documents with provider data; retrying pending lookups in %s: %s", delay, err) - } else { - delay = interval - } - timer := time.NewTimer(delay) - select { - case <-ctx.Done(): - timer.Stop() - return - case <-timer.C: - } - if err != nil { - delay = min(2*delay, maxDelay) - } - } -} - -func (b *BleveIndexer) enrichDocuments(ctx context.Context, batchSize int) error { if batchSize <= 0 { return fmt.Errorf("document enrichment batch size must be positive") } var failures []error for i, provider := range b.documentEnrichers { - if err := ctx.Err(); err != nil { - return err - } - if err := b.enrichProvider(ctx, provider, b.documentEnrichTracker(i), batchSize); err != nil { + if err := b.enrichProvider(provider, b.documentEnrichTracker(i), batchSize); err != nil { failures = append(failures, fmt.Errorf("enriching documents with %s: %w", provider.Source(), err)) } } @@ -148,23 +76,20 @@ func (b *BleveIndexer) documentEnrichTracker(i int) *progressTracker { return &progressTracker{} } -func (b *BleveIndexer) enrichProvider(ctx context.Context, provider DocumentEnricher, tracker *progressTracker, batchSize int) (resultErr error) { +func (b *BleveIndexer) enrichProvider(provider DocumentEnricher, tracker *progressTracker, batchSize int) (resultErr error) { defer func() { tracker.failed.Store(resultErr != nil) tracker.end() }() - failedDocuments := tracker.unresolvedDocuments() - _, total, err := b.documentsNeedingProvider(provider.Source(), 0, failedDocuments) + _, total, err := b.documentsNeedingProvider(provider.Source(), 0, nil) if err != nil || total == 0 { return err } tracker.begin(int(total)) log.Printf("Enriching %d documents with %s data", total, provider.Source()) var failures []error + failedDocuments := make(map[string]struct{}) for iteration := 0; iteration < int(total)+10; iteration++ { - if err := ctx.Err(); err != nil { - return err - } documents, _, err := b.documentsNeedingProvider(provider.Source(), batchSize, failedDocuments) if err != nil { failures = append(failures, err) @@ -173,7 +98,7 @@ func (b *BleveIndexer) enrichProvider(ctx context.Context, provider DocumentEnri if len(documents) == 0 { return errors.Join(failures...) } - for id, err := range b.enrichProviderBatch(ctx, provider, tracker, documents) { + for id, err := range b.enrichProviderBatch(provider, tracker, documents) { failedDocuments[id] = struct{}{} failures = append(failures, err) } @@ -184,7 +109,7 @@ func (b *BleveIndexer) enrichProvider(ctx context.Context, provider DocumentEnri return fmt.Errorf("document enrichment did not finish; some documents remain pending") } -func (b *BleveIndexer) enrichProviderBatch(ctx context.Context, provider DocumentEnricher, tracker *progressTracker, documents []Document) map[string]error { +func (b *BleveIndexer) enrichProviderBatch(provider DocumentEnricher, tracker *progressTracker, documents []Document) map[string]error { jobs := make(chan Document, len(documents)) for _, document := range documents { jobs <- document @@ -196,10 +121,10 @@ func (b *BleveIndexer) enrichProviderBatch(ctx context.Context, provider Documen for range min(documentEnrichWorkers, len(documents)) { workers.Go(func() { for document := range jobs { - if ctx.Err() != nil { - return + needed, err := provider.NeedsEnrichment(document) + if err == nil && needed { + err = provider.Enrich(context.Background(), b, document) } - err := b.enrichDocument(ctx, provider, tracker, document) if err != nil { failureMu.Lock() failures[document.ID] = fmt.Errorf("enriching document %s: %w", document.ID, err) @@ -213,35 +138,9 @@ func (b *BleveIndexer) enrichProviderBatch(ctx context.Context, provider Documen return failures } -func (b *BleveIndexer) enrichDocument(ctx context.Context, provider DocumentEnricher, tracker *progressTracker, document Document) error { - value, _ := tracker.lookups.LoadOrStore(document.ID, &documentLookupState{}) - state := value.(*documentLookupState) - state.mu.Lock() - defer state.mu.Unlock() - if state.notFoundAttempts.Load() >= maxNotFoundAttempts { - return fmt.Errorf("automatic retry limit reached after %d not-found responses", maxNotFoundAttempts) - } - needed, err := provider.NeedsEnrichment(document) - if err == nil && needed { - err = provider.Enrich(ctx, b, document) - } - if errors.Is(err, ErrEnrichmentNotFound) { - if state.notFoundAttempts.Add(1) == maxNotFoundAttempts { - b.providerProgressVersion.Add(1) - log.Printf("Leaving document %s unresolved for %s after %d not-found responses: %s", - document.ID, provider.Source(), maxNotFoundAttempts, err) - } - } else if err == nil { - if state.notFoundAttempts.Swap(0) > 0 { - b.providerProgressVersion.Add(1) - } - } - return err -} - func (b *BleveIndexer) runDocumentEnrichWorker() { for id := range b.documentEnrichJobs { - for i, provider := range b.documentEnrichers { + for _, provider := range b.documentEnrichers { document, err := b.documentByIndexID(id) if err != nil { log.Printf("Error reading document %s for %s enrichment: %s", id, provider.Source(), err) @@ -250,7 +149,10 @@ func (b *BleveIndexer) runDocumentEnrichWorker() { if document.ID == "" { break } - err = b.enrichDocument(context.Background(), provider, b.documentEnrichTracker(i), document) + needed, err := provider.NeedsEnrichment(document) + if err == nil && needed { + err = provider.Enrich(context.Background(), b, document) + } if err != nil { log.Printf("Error enriching document %s with %s: %s", id, provider.Source(), err) } diff --git a/internal/index/bleve_document_enrich_test.go b/internal/index/bleve_document_enrich_test.go index 79713dad..2f1c91ac 100644 --- a/internal/index/bleve_document_enrich_test.go +++ b/internal/index/bleve_document_enrich_test.go @@ -23,287 +23,6 @@ type blockingDocumentEnricher struct { release chan struct{} } -type retryDocumentEnricher struct { - testDocumentEnricher - attempts chan time.Time - retries atomic.Int32 -} - -func (p *retryDocumentEnricher) Enrich(ctx context.Context, idx *BleveIndexer, document Document) error { - if document.ID == "retry" { - attempt := p.retries.Add(1) - p.attempts <- time.Now() - if attempt <= 3 { - return errors.New("temporary upstream failure") - } - } - return p.testDocumentEnricher.Enrich(ctx, idx, document) -} - -func TestDocumentEnrichmentAutomaticallyRetriesPendingLookups(t *testing.T) { - provider := &retryDocumentEnricher{ - testDocumentEnricher: testDocumentEnricher{source: "test"}, - attempts: make(chan time.Time, 10), - } - idx, _ := newTestTextRankIndexer(t, Config{DocumentEnrichers: []DocumentEnricher{provider}}) - t.Cleanup(func() { _ = idx.Close() }) - for _, id := range []string{"retry", "successful"} { - if err := IndexDocument(idx.documentsIdx, id, Document{ID: id}); err != nil { - t.Fatal(err) - } - } - ctx, cancel := context.WithCancel(context.Background()) - done := make(chan struct{}) - go func() { - defer close(done) - idx.retryDocumentEnrichment(ctx, 1, 20*time.Millisecond, 40*time.Millisecond) - }() - defer func() { - cancel() - select { - case <-done: - case <-time.After(time.Second): - t.Error("retry worker did not stop on cancellation") - } - }() - var previous time.Time - for attempt := range 4 { - select { - case started := <-provider.attempts: - if attempt > 0 { - minimumDelay := time.Duration(min(attempt, 2)) * 20 * time.Millisecond - if started.Sub(previous) < minimumDelay { - t.Fatalf("retry %d ignored backoff: %s < %s", attempt, started.Sub(previous), minimumDelay) - } - } - previous = started - case <-time.After(2 * time.Second): - t.Fatal("pending lookup was not retried automatically") - } - } - deadline := time.Now().Add(time.Second) - for { - progress, err := idx.BackgroundProgress() - if err != nil { - t.Fatal(err) - } - if len(progress) == 0 { - break - } - if time.Now().After(deadline) { - t.Fatalf("progress did not complete after automatic recovery: %+v", progress) - } - time.Sleep(time.Millisecond) - } - time.Sleep(60 * time.Millisecond) - if provider.retries.Load() != 4 || provider.calls.Load() != 2 { - t.Fatalf("completed lookups were repeated: retries=%d completed=%d", provider.retries.Load(), provider.calls.Load()) - } - idx.documentsMu.Lock() - err := IndexDocument(idx.documentsIdx, "later-upload", Document{ID: "later-upload"}) - idx.documentsMu.Unlock() - if err != nil { - t.Fatal(err) - } - deadline = time.Now().Add(time.Second) - for provider.calls.Load() != 3 { - if time.Now().After(deadline) { - t.Fatal("automatic enrichment did not pick up a later pending document") - } - time.Sleep(time.Millisecond) - } -} - -func TestDocumentEnrichmentBoundsNotFoundRetries(t *testing.T) { - provider := &testDocumentEnricher{ - source: "test", failure: fmt.Errorf("HTTP 404: %w", ErrEnrichmentNotFound), failureID: "unresolved", - } - idx, _ := newTestTextRankIndexer(t, Config{DocumentEnrichers: []DocumentEnricher{provider}}) - t.Cleanup(func() { _ = idx.Close() }) - for _, id := range []string{"unresolved", "successful"} { - if err := IndexDocument(idx.documentsIdx, id, Document{ID: id}); err != nil { - t.Fatal(err) - } - } - for attempt := range 6 { - err := idx.EnrichDocuments(1) - if attempt < maxNotFoundAttempts && !errors.Is(err, ErrEnrichmentNotFound) { - t.Fatalf("attempt %d: expected not-found error, got %v", attempt, err) - } - if attempt >= maxNotFoundAttempts && err != nil { - t.Fatalf("exhausted documents must be excluded from later runs: %v", err) - } - } - if provider.calls.Load() != maxNotFoundAttempts+1 { - t.Fatalf("404s must stop after three attempts without repeating successful work: calls=%d", provider.calls.Load()) - } - progress, err := idx.BackgroundProgress() - if err != nil || len(progress) != 1 || progress[0].Unresolved != 1 || - progress[0].InProgress || progress[0].Failed || progress[0].RemainingTime != 0 || progress[0].Percentage != 50 { - t.Fatalf("expected unresolved warning without active work: %+v, %v", progress, err) - } - document, err := idx.documentByIndexID("unresolved") - if err != nil { - t.Fatal(err) - } - needed, err := provider.NeedsEnrichment(document) - if err != nil || !needed { - t.Fatalf("unresolved lookup must not be stored as successfully enriched: needed=%t, err=%v", needed, err) - } - // Upload and batch callers share the same limit. - if err := idx.enrichDocument(context.Background(), provider, idx.documentEnrichTracker(0), document); err == nil { - t.Fatal("queued work must not bypass the retry limit") - } - if provider.calls.Load() != maxNotFoundAttempts+1 { - t.Fatal("queued work bypassed the retry limit") - } - // A later successful on-demand lookup must remove the unresolved warning. - if err := idx.StoreDocumentData("unresolved", "test", map[string]any{"enriched": true}); err != nil { - t.Fatal(err) - } - progress, err = idx.BackgroundProgress() - if err != nil || len(progress) != 0 { - t.Fatalf("resolved lookup still reported as unresolved: %+v, %v", progress, err) - } -} - -func TestDocumentEnrichmentNotFoundCanRecoverBeforeLimit(t *testing.T) { - provider := &testDocumentEnricher{source: "test", failure: ErrEnrichmentNotFound} - idx, _ := newTestTextRankIndexer(t, Config{DocumentEnrichers: []DocumentEnricher{provider}}) - t.Cleanup(func() { _ = idx.Close() }) - if err := IndexDocument(idx.documentsIdx, "book", Document{ID: "book"}); err != nil { - t.Fatal(err) - } - for range 2 { - if err := idx.EnrichDocuments(1); !errors.Is(err, ErrEnrichmentNotFound) { - t.Fatalf("expected not-found error, got %v", err) - } - } - provider.failure = nil - if err := idx.EnrichDocuments(1); err != nil { - t.Fatal(err) - } - progress, err := idx.BackgroundProgress() - if err != nil || len(progress) != 0 || len(idx.documentEnrichTracker(0).unresolvedDocuments()) != 0 { - t.Fatalf("successful retry must clear pending progress: %+v, %v", progress, err) - } -} - -func TestDocumentEnrichmentOnlyCountsNotFoundResponses(t *testing.T) { - provider := &testDocumentEnricher{source: "test", failure: ErrEnrichmentNotFound} - idx, _ := newTestTextRankIndexer(t, Config{DocumentEnrichers: []DocumentEnricher{provider}}) - t.Cleanup(func() { _ = idx.Close() }) - if err := IndexDocument(idx.documentsIdx, "book", Document{ID: "book"}); err != nil { - t.Fatal(err) - } - if err := idx.EnrichDocuments(1); !errors.Is(err, ErrEnrichmentNotFound) { - t.Fatal(err) - } - unavailable := errors.New("all instances are in cooldown") - provider.failure = unavailable - for range 4 { - if err := idx.EnrichDocuments(1); !errors.Is(err, unavailable) { - t.Fatalf("cooldown must leave lookup retryable: %v", err) - } - } - if len(idx.documentEnrichTracker(0).unresolvedDocuments()) != 0 { - t.Fatal("failures without a 404 exhausted the not-found budget") - } - provider.failure = ErrEnrichmentNotFound - for range 2 { - if err := idx.EnrichDocuments(1); !errors.Is(err, ErrEnrichmentNotFound) { - t.Fatal(err) - } - } - if len(idx.documentEnrichTracker(0).unresolvedDocuments()) != 1 || provider.calls.Load() != 7 { - t.Fatal("lookup must become unresolved only after three actual not-found responses") - } -} - -func TestConcurrentDocumentEnrichmentSharesNotFoundLimit(t *testing.T) { - provider := &testDocumentEnricher{source: "test", failure: ErrEnrichmentNotFound} - idx, _ := newTestTextRankIndexer(t, Config{DocumentEnrichers: []DocumentEnricher{provider}}) - t.Cleanup(func() { _ = idx.Close() }) - documents := make([]Document, 2*documentEnrichWorkers) - for i := range documents { - documents[i] = Document{ID: "book"} - } - idx.enrichProviderBatch(context.Background(), provider, idx.documentEnrichTracker(0), documents) - if provider.calls.Load() != maxNotFoundAttempts { - t.Fatalf("concurrent callers bypassed the limit: %d attempts", provider.calls.Load()) - } -} - -func TestAutomaticDocumentEnrichmentStopsNotFoundRetries(t *testing.T) { - provider := &testDocumentEnricher{source: "test", failure: ErrEnrichmentNotFound} - idx, _ := newTestTextRankIndexer(t, Config{DocumentEnrichers: []DocumentEnricher{provider}}) - t.Cleanup(func() { _ = idx.Close() }) - if err := IndexDocument(idx.documentsIdx, "book", Document{ID: "book"}); err != nil { - t.Fatal(err) - } - ctx, cancel := context.WithCancel(context.Background()) - done := make(chan struct{}) - go func() { - defer close(done) - idx.retryDocumentEnrichment(ctx, 1, 5*time.Millisecond, 10*time.Millisecond) - }() - defer func() { - cancel() - select { - case <-done: - case <-time.After(time.Second): - t.Error("retry worker did not stop on cancellation") - } - }() - deadline := time.Now().Add(time.Second) - for { - progress, err := idx.BackgroundProgress() - if err != nil { - t.Fatal(err) - } - if len(progress) == 1 && progress[0].Unresolved == 1 { - break - } - if time.Now().After(deadline) { - t.Fatal("automatic retries did not reach the not-found limit") - } - time.Sleep(time.Millisecond) - } - time.Sleep(50 * time.Millisecond) - if provider.calls.Load() != maxNotFoundAttempts { - t.Fatalf("automatic retries exceeded three not-found attempts: %d", provider.calls.Load()) - } -} - -func TestCloseCancelsAutomaticDocumentEnrichment(t *testing.T) { - provider := &blockingDocumentEnricher{started: make(chan struct{}, 1), release: make(chan struct{})} - idx, _ := newTestTextRankIndexer(t, Config{DocumentEnrichers: []DocumentEnricher{provider}}) - if err := IndexDocument(idx.documentsIdx, "book", Document{ID: "book"}); err != nil { - t.Fatal(err) - } - if err := idx.StartDocumentEnrichment(0); err == nil { - t.Fatal("invalid batch size must be rejected") - } - if err := idx.StartDocumentEnrichment(1); err != nil { - t.Fatal(err) - } - select { - case <-provider.started: - case <-time.After(time.Second): - t.Fatal("automatic enrichment did not start") - } - done := make(chan error, 1) - go func() { done <- idx.Close() }() - select { - case err := <-done: - if err != nil { - t.Fatal(err) - } - case <-time.After(time.Second): - t.Fatal("closing the indexer did not cancel its active lookup") - } -} - func (*blockingDocumentEnricher) Source() string { return "blocking" } func (*blockingDocumentEnricher) Name() string { return "Blocking" } @@ -334,7 +53,7 @@ func TestDocumentEnrichmentUsesBoundedConcurrency(t *testing.T) { documents[i] = Document{ID: fmt.Sprintf("book-%d", i)} } done := make(chan map[string]error, 1) - go func() { done <- idx.enrichProviderBatch(context.Background(), provider, &progressTracker{}, documents) }() + go func() { done <- idx.enrichProviderBatch(provider, &progressTracker{}, documents) }() defer func() { close(provider.release) if failures := <-done; len(failures) != 0 { diff --git a/internal/index/bleve_document_write.go b/internal/index/bleve_document_write.go index 27c1fa2b..4fe69d8f 100644 --- a/internal/index/bleve_document_write.go +++ b/internal/index/bleve_document_write.go @@ -82,15 +82,13 @@ func (b *BleveIndexer) BackgroundProgress() ([]Progress, error) { for i, counts := range providers { if counts.enriched < counts.total { tracker := b.documentEnrichTracker(i) - pending := counts.total - counts.enriched - counts.unresolved - failed := tracker.failed.Load() && pending > 0 + failed := tracker.failed.Load() var remaining time.Duration if !failed { - remaining = tracker.remainingTime(pending) + remaining = tracker.remainingTime(counts.total - counts.enriched) } progress = append(progress, Progress{ - Kind: ProgressDataSources, InProgress: pending > 0 && !failed, Failed: failed, - Unresolved: counts.unresolved, Name: b.documentEnrichers[i].Name(), + Kind: ProgressDataSources, InProgress: !failed, Failed: failed, Name: b.documentEnrichers[i].Name(), // Do not round pending work up to 100%. Percentage: math.Floor(100 * float64(counts.enriched) / float64(counts.total)), RemainingTime: remaining, @@ -101,7 +99,7 @@ func (b *BleveIndexer) BackgroundProgress() ([]Progress, error) { } type providerProgressCount struct { - total, enriched, unresolved uint64 + total, enriched uint64 } // Cache index queries across page renders and polling clients; timing estimates @@ -120,30 +118,14 @@ func (b *BleveIndexer) providerProgressSnapshot() ([]providerProgressCount, erro return nil, fmt.Errorf("counting documents for enrichment progress: %w", err) } var counts []providerProgressCount - for i, provider := range b.documentEnrichers { + for _, provider := range b.documentEnrichers { query := bleve.NewTermQuery(provider.Source()) query.SetField("EnrichedDataSources") result, err := b.documentsIdx.Search(bleve.NewSearchRequestOptions(query, 0, 0, false)) if err != nil { return nil, fmt.Errorf("reading %s enrichment progress: %w", provider.Source(), err) } - count := providerProgressCount{total: total, enriched: result.Total} - unresolved := b.documentEnrichTracker(i).unresolvedDocuments() - if len(unresolved) > 0 { - ids := make([]string, 0, len(unresolved)) - for id := range unresolved { - ids = append(ids, id) - } - pending := bleve.NewBooleanQuery() - pending.AddMust(bleve.NewDocIDQuery(ids)) - pending.AddMustNot(query) - result, err := b.documentsIdx.Search(bleve.NewSearchRequestOptions(pending, 0, 0, false)) - if err != nil { - return nil, fmt.Errorf("reading %s unresolved lookups: %w", provider.Source(), err) - } - count.unresolved = result.Total - } - counts = append(counts, count) + counts = append(counts, providerProgressCount{total: total, enriched: result.Total}) } b.providerProgress = slices.Clone(counts) b.cachedProviderProgressVersion = version diff --git a/internal/index/progress.go b/internal/index/progress.go index 3fce7082..ca6550e6 100644 --- a/internal/index/progress.go +++ b/internal/index/progress.go @@ -16,7 +16,6 @@ type Progress struct { Kind ProgressKind InProgress bool Failed bool - Unresolved uint64 RemainingTime time.Duration Percentage float64 Name string diff --git a/internal/webserver/document_biblioreads_test.go b/internal/webserver/document_biblioreads_test.go index d8270d02..76228003 100644 --- a/internal/webserver/document_biblioreads_test.go +++ b/internal/webserver/document_biblioreads_test.go @@ -137,25 +137,23 @@ func TestReviewEnrichmentProgressBanner(t *testing.T) { if err := idx.EnrichDocuments(1); err == nil { t.Fatal("expected unavailable source to fail") } - checkPending() for lang, label := range map[string]string{ - "en": "Waiting to retry ratings", - "es": "Esperando para reintentar la recuperación de valoraciones", - "de": "Warten auf erneuten Abruf der Bewertungen", - "fr": "En attente d'une nouvelle tentative de récupération des évaluations", - "ru": "Ожидание повторного получения оценок", + "en": "Some ratings remain unresolved", + "es": "Algunas valoraciones siguen sin resolverse", + "de": "Einige Bewertungen bleiben ungeklärt", + "fr": "Certaines évaluations restent non résolues", + "ru": "Некоторые оценки остаются неполученными", } { localized := getPage(t, app, "/background-progress?l="+lang) reviews := localized.Find(`[data-progress-kind="data-sources"]`) - if reviews.Find(`[role="progressbar"]`).AttrOr("aria-label", "") != label || - !strings.Contains(reviews.Find("p").First().Text(), label) { - t.Errorf("%s: failed lookups must show pending retries", lang) + if !strings.Contains(reviews.Find("p").First().Text(), label) { + t.Errorf("%s: failed lookups must show incomplete results", lang) } - if lang == "en" && !strings.Contains(reviews.Text(), "Pending lookups will be retried automatically.") { + if lang == "en" && !strings.Contains(reviews.Text(), "Restart Coreander to retry pending lookups.") { t.Fatal("failed progress must explain how to retry") } - if reviews.Find(`[role="progressbar"]`).AttrOr("aria-valuetext", "") != "" { - t.Fatal("stopped retrieval must not display a remaining time estimate") + if reviews.Find(`[role="progressbar"]`).Length() != 0 { + t.Fatal("finished retrieval must not leave a stuck progress bar") } } source.searchErr = nil @@ -177,41 +175,6 @@ func TestReviewEnrichmentProgressBanner(t *testing.T) { } } -func TestReviewEnrichmentUnresolvedBanner(t *testing.T) { - source := &fakeBookSource{searchErr: biblioreads.ErrNotFound} - app, idx := newBiblioReadsTestApp(t, source) - for range 3 { - if err := idx.EnrichDocuments(1); !errors.Is(err, biblioreads.ErrNotFound) { - t.Fatalf("expected not-found lookup, got %v", err) - } - } - for lang, label := range map[string]string{ - "en": "Some ratings remain unresolved", - "es": "Algunas valoraciones siguen sin resolverse", - "de": "Einige Bewertungen bleiben ungeklärt", - "fr": "Certaines évaluations restent non résolues", - "ru": "Некоторые оценки остаются неполученными", - } { - page := getPage(t, app, "/background-progress?l="+lang) - reviews := page.Find(`[data-progress-kind="data-sources"]`) - if !strings.Contains(reviews.Find("p").First().Text(), label) { - t.Errorf("%s: expected localized unresolved warning: %s", lang, reviews.Text()) - } - if reviews.Find(`[role="progressbar"]`).Length() != 0 { - t.Errorf("%s: unresolved lookups must not leave a stuck progress bar", lang) - } - if strings.Contains(reviews.Text(), "retried automatically") { - t.Errorf("%s: exhausted lookups must not promise further automatic retries", lang) - } - if lang == "en" && !strings.Contains(reviews.Text(), "Rating lookups unresolved after three not-found responses: 1.") { - t.Fatal("unresolved warning must include its count and explain the retry limit") - } - } - if err := idx.EnrichDocuments(1); err != nil { - t.Fatalf("exhausted lookup should not block later runs: %v", err) - } -} - func newDataSourceProvidersTestApp(t *testing.T, source *fakeBookSource, provider document.ReviewProvider, dataSourceLinkProvider authorcontroller.DataSourceLinkProvider) (*fiber.App, *index.BleveIndexer) { t.Helper() db := infrastructure.Connect(":memory:", 250) diff --git a/internal/webserver/embedded/translations/de.yml b/internal/webserver/embedded/translations/de.yml index 4796de4f..df64fd46 100644 --- a/internal/webserver/embedded/translations/de.yml +++ b/internal/webserver/embedded/translations/de.yml @@ -427,9 +427,7 @@ "Not scored": "Nicht bewertet" "%s ratings": "%s Bewertungen" "Retrieving ratings": "Bewertungen werden abgerufen" -"Waiting to retry ratings": "Warten auf erneuten Abruf der Bewertungen" "Some ratings remain unresolved": "Einige Bewertungen bleiben ungeklärt" -"Rating lookups unresolved after three not-found responses: %d. Restart Coreander to retry them.": "Ungeklärte Bewertungsabfragen nach drei Nicht-gefunden-Antworten: %d. Starte Coreander neu, um sie erneut zu versuchen." -"Some ratings could not be retrieved. Pending lookups will be retried automatically.": "Einige Bewertungen konnten nicht abgerufen werden. Ausstehende Abfragen werden automatisch wiederholt." +"Some ratings could not be retrieved. Restart Coreander to retry pending lookups.": "Einige Bewertungen konnten nicht abgerufen werden. Starte Coreander neu, um ausstehende Abfragen zu wiederholen." "Review information is incomplete. Score filters may omit documents whose ratings have not been retrieved.": "Bewertungsinformationen sind unvollständig. Bewertungsfilter können Dokumente auslassen, deren Bewertungen noch nicht abgerufen wurden." "Unable to retrieve background progress.": "Der Fortschritt der Hintergrundaufgaben konnte nicht abgerufen werden." diff --git a/internal/webserver/embedded/translations/es.yml b/internal/webserver/embedded/translations/es.yml index dbc453a4..43a80b84 100644 --- a/internal/webserver/embedded/translations/es.yml +++ b/internal/webserver/embedded/translations/es.yml @@ -423,9 +423,7 @@ "Not scored": "Sin puntuar" "%s ratings": "%s valoraciones" "Retrieving ratings": "Recuperando valoraciones" -"Waiting to retry ratings": "Esperando para reintentar la recuperación de valoraciones" "Some ratings remain unresolved": "Algunas valoraciones siguen sin resolverse" -"Rating lookups unresolved after three not-found responses: %d. Restart Coreander to retry them.": "Búsquedas de valoraciones sin resolver tras tres respuestas de no encontrado: %d. Reinicia Coreander para reintentarlas." -"Some ratings could not be retrieved. Pending lookups will be retried automatically.": "No se pudieron recuperar algunas valoraciones. Las búsquedas pendientes se reintentarán automáticamente." +"Some ratings could not be retrieved. Restart Coreander to retry pending lookups.": "No se pudieron recuperar algunas valoraciones. Reinicia Coreander para reintentar las búsquedas pendientes." "Review information is incomplete. Score filters may omit documents whose ratings have not been retrieved.": "La información de valoraciones está incompleta. Los filtros de puntuación pueden omitir documentos cuyas valoraciones aún no se han obtenido." "Unable to retrieve background progress.": "No se pudo obtener el progreso de las tareas en segundo plano." diff --git a/internal/webserver/embedded/translations/fr.yml b/internal/webserver/embedded/translations/fr.yml index 3203229e..475acf52 100644 --- a/internal/webserver/embedded/translations/fr.yml +++ b/internal/webserver/embedded/translations/fr.yml @@ -425,9 +425,7 @@ "Not scored": "Non noté" "%s ratings": "%s notes" "Retrieving ratings": "Récupération des évaluations" -"Waiting to retry ratings": "En attente d'une nouvelle tentative de récupération des évaluations" "Some ratings remain unresolved": "Certaines évaluations restent non résolues" -"Rating lookups unresolved after three not-found responses: %d. Restart Coreander to retry them.": "Recherches d'évaluations non résolues après trois réponses introuvables : %d. Redémarrez Coreander pour les réessayer." -"Some ratings could not be retrieved. Pending lookups will be retried automatically.": "Certaines évaluations n'ont pas pu être récupérées. Les recherches en attente seront réessayées automatiquement." +"Some ratings could not be retrieved. Restart Coreander to retry pending lookups.": "Certaines évaluations n'ont pas pu être récupérées. Redémarrez Coreander pour réessayer les recherches en attente." "Review information is incomplete. Score filters may omit documents whose ratings have not been retrieved.": "Les informations d'évaluation sont incomplètes. Les filtres de note peuvent omettre les documents dont les évaluations n'ont pas encore été récupérées." "Unable to retrieve background progress.": "Impossible de récupérer la progression des tâches en arrière-plan." diff --git a/internal/webserver/embedded/translations/ru.yml b/internal/webserver/embedded/translations/ru.yml index 4425a1e0..63c14f1f 100644 --- a/internal/webserver/embedded/translations/ru.yml +++ b/internal/webserver/embedded/translations/ru.yml @@ -419,9 +419,7 @@ "Not scored": "Не оценено" "%s ratings": "Оценок: %s" "Retrieving ratings": "Получение оценок" -"Waiting to retry ratings": "Ожидание повторного получения оценок" "Some ratings remain unresolved": "Некоторые оценки остаются неполученными" -"Rating lookups unresolved after three not-found responses: %d. Restart Coreander to retry them.": "Неразрешённые запросы оценок после трёх ответов «не найдено»: %d. Перезапустите Coreander, чтобы повторить их." -"Some ratings could not be retrieved. Pending lookups will be retried automatically.": "Не удалось получить некоторые оценки. Оставшиеся запросы будут повторены автоматически." +"Some ratings could not be retrieved. Restart Coreander to retry pending lookups.": "Не удалось получить некоторые оценки. Перезапустите Coreander, чтобы повторить оставшиеся запросы." "Review information is incomplete. Score filters may omit documents whose ratings have not been retrieved.": "Информация об оценках неполная. Фильтры по оценкам могут пропускать документы, оценки которых ещё не получены." "Unable to retrieve background progress.": "Не удалось получить состояние фоновых задач." diff --git a/internal/webserver/embedded/views/partials/background-progress.html b/internal/webserver/embedded/views/partials/background-progress.html index f045c849..ba70e8f5 100644 --- a/internal/webserver/embedded/views/partials/background-progress.html +++ b/internal/webserver/embedded/views/partials/background-progress.html @@ -13,23 +13,19 @@ {{else if eq .Kind "pruning"}}{{$label = "Keyword clean-up progress"}} {{else if eq .Kind "data-sources"}} {{$label = "Retrieving ratings"}} - {{if .Failed}}{{$label = "Waiting to retry ratings"}}{{end}} - {{if and (gt .Unresolved 0) (not .InProgress) (not .Failed)}}{{$label = "Some ratings remain unresolved"}}{{end}} + {{if .Failed}}{{$label = "Some ratings remain unresolved"}}{{end}} {{end}}

{{t $.Lang $label}}{{if .Name}} ({{.Name}}){{end}}

{{$remaining := ""}}{{if gt .RemainingTime 0}}{{$remaining = t $.Lang "%s left" (remainingTime .RemainingTime)}}{{end}} - {{if or .InProgress .Failed}} + {{if .InProgress}}
{{.Percentage}}%{{if $remaining}} ({{$remaining}}){{end}}
{{end}} {{if eq .Kind "data-sources"}} - {{if gt .Unresolved 0}} -

{{t $.Lang "Rating lookups unresolved after three not-found responses: %d. Restart Coreander to retry them." .Unresolved}}

- {{end}} {{if .Failed}} -

{{t $.Lang "Some ratings could not be retrieved. Pending lookups will be retried automatically."}}

+

{{t $.Lang "Some ratings could not be retrieved. Restart Coreander to retry pending lookups."}}

{{end}}

{{t $.Lang "Review information is incomplete. Score filters may omit documents whose ratings have not been retrieved."}}

{{else if eq .Kind "authors"}} diff --git a/main.go b/main.go index 4858abe5..ffaffb42 100644 --- a/main.go +++ b/main.go @@ -241,9 +241,11 @@ func startIndex(idx *index.BleveIndexer, batchSize int, libPath string, indexWor log.Printf("Indexing finished, took %d seconds", int(dur.Seconds())) // Network-bound, so it runs alongside the CPU-bound TextRank enrichment. - if err := idx.StartDocumentEnrichment(batchSize); err != nil { - log.Printf("Error starting document enrichment: %s", err) - } + go func() { + if err := idx.EnrichDocuments(batchSize); err != nil { + log.Printf("Error enriching documents with provider data: %s", err) + } + }() if err := idx.EnrichTextRankKeywords(batchSize, indexWorkers); err != nil { log.Printf("Error enriching documents with TextRank keywords: %s", err) From 487e80b2e8cca0f839a212cfbe6b614f457e30bd Mon Sep 17 00:00:00 2001 From: Sergio Vera Date: Fri, 6 Nov 2026 16:46:26 +0100 Subject: [PATCH 4/5] Remove message --- .../webserver/document_biblioreads_test.go | 35 +++++++++++-------- .../webserver/embedded/translations/de.yml | 2 -- .../webserver/embedded/translations/es.yml | 2 -- .../webserver/embedded/translations/fr.yml | 2 -- .../webserver/embedded/translations/ru.yml | 2 -- .../views/partials/background-progress.html | 4 --- internal/webserver/webserver.go | 8 ++++- 7 files changed, 27 insertions(+), 28 deletions(-) diff --git a/internal/webserver/document_biblioreads_test.go b/internal/webserver/document_biblioreads_test.go index 76228003..1adb77af 100644 --- a/internal/webserver/document_biblioreads_test.go +++ b/internal/webserver/document_biblioreads_test.go @@ -137,23 +137,13 @@ func TestReviewEnrichmentProgressBanner(t *testing.T) { if err := idx.EnrichDocuments(1); err == nil { t.Fatal("expected unavailable source to fail") } - for lang, label := range map[string]string{ - "en": "Some ratings remain unresolved", - "es": "Algunas valoraciones siguen sin resolverse", - "de": "Einige Bewertungen bleiben ungeklärt", - "fr": "Certaines évaluations restent non résolues", - "ru": "Некоторые оценки остаются неполученными", - } { + for _, lang := range []string{"en", "es", "de", "fr", "ru"} { localized := getPage(t, app, "/background-progress?l="+lang) - reviews := localized.Find(`[data-progress-kind="data-sources"]`) - if !strings.Contains(reviews.Find("p").First().Text(), label) { - t.Errorf("%s: failed lookups must show incomplete results", lang) + if localized.Find(`[data-progress-kind="data-sources"]`).Length() != 0 { + t.Errorf("%s: failed ratings retrieval must not display a message or progress bar", lang) } - if lang == "en" && !strings.Contains(reviews.Text(), "Restart Coreander to retry pending lookups.") { - t.Fatal("failed progress must explain how to retry") - } - if reviews.Find(`[role="progressbar"]`).Length() != 0 { - t.Fatal("finished retrieval must not leave a stuck progress bar") + if localized.Find(`[data-progress-kind="documents"]`).Length() != 1 { + t.Errorf("%s: hiding failed ratings must preserve active indexing progress", lang) } } source.searchErr = nil @@ -175,6 +165,21 @@ func TestReviewEnrichmentProgressBanner(t *testing.T) { } } +func TestFailedReviewEnrichmentLeavesNoAlert(t *testing.T) { + app, idx := newBiblioReadsTestApp(t, &fakeBookSource{searchErr: biblioreads.ErrUnavailable}) + if err := idx.EnrichDocuments(1); err == nil { + t.Fatal("expected unavailable source to fail") + } + page := getPage(t, app, "/background-progress?l=en") + banner := page.Find("#background-progress") + if banner.Find(".alert").Length() != 0 || strings.TrimSpace(banner.Text()) != "" { + t.Fatal("failed ratings retrieval must not leave an empty alert or end-user message") + } + if banner.AttrOr("hx-trigger", "") != "every 5s" { + t.Fatal("progress polling must remain available for future tasks") + } +} + func newDataSourceProvidersTestApp(t *testing.T, source *fakeBookSource, provider document.ReviewProvider, dataSourceLinkProvider authorcontroller.DataSourceLinkProvider) (*fiber.App, *index.BleveIndexer) { t.Helper() db := infrastructure.Connect(":memory:", 250) diff --git a/internal/webserver/embedded/translations/de.yml b/internal/webserver/embedded/translations/de.yml index df64fd46..fafb347e 100644 --- a/internal/webserver/embedded/translations/de.yml +++ b/internal/webserver/embedded/translations/de.yml @@ -427,7 +427,5 @@ "Not scored": "Nicht bewertet" "%s ratings": "%s Bewertungen" "Retrieving ratings": "Bewertungen werden abgerufen" -"Some ratings remain unresolved": "Einige Bewertungen bleiben ungeklärt" -"Some ratings could not be retrieved. Restart Coreander to retry pending lookups.": "Einige Bewertungen konnten nicht abgerufen werden. Starte Coreander neu, um ausstehende Abfragen zu wiederholen." "Review information is incomplete. Score filters may omit documents whose ratings have not been retrieved.": "Bewertungsinformationen sind unvollständig. Bewertungsfilter können Dokumente auslassen, deren Bewertungen noch nicht abgerufen wurden." "Unable to retrieve background progress.": "Der Fortschritt der Hintergrundaufgaben konnte nicht abgerufen werden." diff --git a/internal/webserver/embedded/translations/es.yml b/internal/webserver/embedded/translations/es.yml index 43a80b84..dbadb263 100644 --- a/internal/webserver/embedded/translations/es.yml +++ b/internal/webserver/embedded/translations/es.yml @@ -423,7 +423,5 @@ "Not scored": "Sin puntuar" "%s ratings": "%s valoraciones" "Retrieving ratings": "Recuperando valoraciones" -"Some ratings remain unresolved": "Algunas valoraciones siguen sin resolverse" -"Some ratings could not be retrieved. Restart Coreander to retry pending lookups.": "No se pudieron recuperar algunas valoraciones. Reinicia Coreander para reintentar las búsquedas pendientes." "Review information is incomplete. Score filters may omit documents whose ratings have not been retrieved.": "La información de valoraciones está incompleta. Los filtros de puntuación pueden omitir documentos cuyas valoraciones aún no se han obtenido." "Unable to retrieve background progress.": "No se pudo obtener el progreso de las tareas en segundo plano." diff --git a/internal/webserver/embedded/translations/fr.yml b/internal/webserver/embedded/translations/fr.yml index 475acf52..21280b21 100644 --- a/internal/webserver/embedded/translations/fr.yml +++ b/internal/webserver/embedded/translations/fr.yml @@ -425,7 +425,5 @@ "Not scored": "Non noté" "%s ratings": "%s notes" "Retrieving ratings": "Récupération des évaluations" -"Some ratings remain unresolved": "Certaines évaluations restent non résolues" -"Some ratings could not be retrieved. Restart Coreander to retry pending lookups.": "Certaines évaluations n'ont pas pu être récupérées. Redémarrez Coreander pour réessayer les recherches en attente." "Review information is incomplete. Score filters may omit documents whose ratings have not been retrieved.": "Les informations d'évaluation sont incomplètes. Les filtres de note peuvent omettre les documents dont les évaluations n'ont pas encore été récupérées." "Unable to retrieve background progress.": "Impossible de récupérer la progression des tâches en arrière-plan." diff --git a/internal/webserver/embedded/translations/ru.yml b/internal/webserver/embedded/translations/ru.yml index 63c14f1f..edf04d04 100644 --- a/internal/webserver/embedded/translations/ru.yml +++ b/internal/webserver/embedded/translations/ru.yml @@ -419,7 +419,5 @@ "Not scored": "Не оценено" "%s ratings": "Оценок: %s" "Retrieving ratings": "Получение оценок" -"Some ratings remain unresolved": "Некоторые оценки остаются неполученными" -"Some ratings could not be retrieved. Restart Coreander to retry pending lookups.": "Не удалось получить некоторые оценки. Перезапустите Coreander, чтобы повторить оставшиеся запросы." "Review information is incomplete. Score filters may omit documents whose ratings have not been retrieved.": "Информация об оценках неполная. Фильтры по оценкам могут пропускать документы, оценки которых ещё не получены." "Unable to retrieve background progress.": "Не удалось получить состояние фоновых задач." diff --git a/internal/webserver/embedded/views/partials/background-progress.html b/internal/webserver/embedded/views/partials/background-progress.html index ba70e8f5..6de5c90d 100644 --- a/internal/webserver/embedded/views/partials/background-progress.html +++ b/internal/webserver/embedded/views/partials/background-progress.html @@ -13,7 +13,6 @@ {{else if eq .Kind "pruning"}}{{$label = "Keyword clean-up progress"}} {{else if eq .Kind "data-sources"}} {{$label = "Retrieving ratings"}} - {{if .Failed}}{{$label = "Some ratings remain unresolved"}}{{end}} {{end}}

{{t $.Lang $label}}{{if .Name}} ({{.Name}}){{end}}

@@ -24,9 +23,6 @@
{{end}} {{if eq .Kind "data-sources"}} - {{if .Failed}} -

{{t $.Lang "Some ratings could not be retrieved. Restart Coreander to retry pending lookups."}}

- {{end}}

{{t $.Lang "Review information is incomplete. Score filters may omit documents whose ratings have not been retrieved."}}

{{else if eq .Kind "authors"}}

{{t $.Lang "Retrieving author information. Author pages may be incomplete, and author filters may not work yet."}}

diff --git a/internal/webserver/webserver.go b/internal/webserver/webserver.go index 6939d14b..cdbcc1f1 100644 --- a/internal/webserver/webserver.go +++ b/internal/webserver/webserver.go @@ -144,7 +144,13 @@ func New(cfg Config, controllers Controllers, sender Sender, idx IndexInfo, user if err != nil { log.Printf("Error reading background progress: %s", err) } - return fiber.Map{"Tasks": tasks, "Error": err != nil} + var visibleTasks []index.Progress + for _, task := range tasks { + if !task.Failed { + visibleTasks = append(visibleTasks, task) + } + } + return fiber.Map{"Tasks": visibleTasks, "Error": err != nil} }) // UploadDocumentMaxSize <= 0 means "unlimited", but fiber has no such From bb24b81c6a10ee1079ca5bf21486cd9022538f8c Mon Sep 17 00:00:00 2001 From: Sergio Vera Date: Fri, 6 Nov 2026 16:48:18 +0100 Subject: [PATCH 5/5] Remove flag --- internal/index/bleve_document_enrich_test.go | 9 ++++----- internal/index/bleve_document_write.go | 10 ++++------ internal/index/progress.go | 1 - internal/webserver/webserver.go | 8 +------- 4 files changed, 9 insertions(+), 19 deletions(-) diff --git a/internal/index/bleve_document_enrich_test.go b/internal/index/bleve_document_enrich_test.go index 2f1c91ac..a8f242e6 100644 --- a/internal/index/bleve_document_enrich_test.go +++ b/internal/index/bleve_document_enrich_test.go @@ -129,9 +129,8 @@ func TestBackgroundProgressTracksPersistedProviderLookups(t *testing.T) { t.Fatal("expected unavailable provider to fail") } progress, err = idx.BackgroundProgress() - if err != nil || len(progress) != 1 || progress[0].Name != "first" || progress[0].Percentage != 66 || - !progress[0].Failed || progress[0].InProgress || progress[0].RemainingTime != 0 { - t.Fatalf("failed lookups must remain visible; successful providers must disappear: %+v, %v", progress, err) + if err != nil || len(progress) != 0 { + t.Fatalf("finished providers must disappear, including failed lookups: %+v, %v", progress, err) } // Progress is reconstructed from stored data, not a running worker counter. resumed := &BleveIndexer{documentsIdx: idx.documentsIdx, documentEnrichers: []DocumentEnricher{first}} @@ -233,8 +232,8 @@ func TestDocumentEnrichmentContinuesAfterDocumentFailure(t *testing.T) { t.Fatalf("failed document should remain pending for a later retry: %+v", pending) } progress, err := idx.BackgroundProgress() - if err != nil || len(progress) != 1 || !progress[0].Failed || progress[0].InProgress || progress[0].RemainingTime != 0 { - t.Fatalf("failed run must report stopped progress without an estimate: %+v, %v", progress, err) + if err != nil || len(progress) != 0 { + t.Fatalf("failed run must not report active progress: %+v, %v", progress, err) } provider.failure = nil if err := idx.EnrichDocuments(1); err != nil { diff --git a/internal/index/bleve_document_write.go b/internal/index/bleve_document_write.go index 4fe69d8f..b3d12c63 100644 --- a/internal/index/bleve_document_write.go +++ b/internal/index/bleve_document_write.go @@ -82,16 +82,14 @@ func (b *BleveIndexer) BackgroundProgress() ([]Progress, error) { for i, counts := range providers { if counts.enriched < counts.total { tracker := b.documentEnrichTracker(i) - failed := tracker.failed.Load() - var remaining time.Duration - if !failed { - remaining = tracker.remainingTime(counts.total - counts.enriched) + if tracker.failed.Load() { + continue } progress = append(progress, Progress{ - Kind: ProgressDataSources, InProgress: !failed, Failed: failed, Name: b.documentEnrichers[i].Name(), + Kind: ProgressDataSources, InProgress: true, Name: b.documentEnrichers[i].Name(), // Do not round pending work up to 100%. Percentage: math.Floor(100 * float64(counts.enriched) / float64(counts.total)), - RemainingTime: remaining, + RemainingTime: tracker.remainingTime(counts.total - counts.enriched), }) } } diff --git a/internal/index/progress.go b/internal/index/progress.go index ca6550e6..72948b28 100644 --- a/internal/index/progress.go +++ b/internal/index/progress.go @@ -15,7 +15,6 @@ const ( type Progress struct { Kind ProgressKind InProgress bool - Failed bool RemainingTime time.Duration Percentage float64 Name string diff --git a/internal/webserver/webserver.go b/internal/webserver/webserver.go index cdbcc1f1..6939d14b 100644 --- a/internal/webserver/webserver.go +++ b/internal/webserver/webserver.go @@ -144,13 +144,7 @@ func New(cfg Config, controllers Controllers, sender Sender, idx IndexInfo, user if err != nil { log.Printf("Error reading background progress: %s", err) } - var visibleTasks []index.Progress - for _, task := range tasks { - if !task.Failed { - visibleTasks = append(visibleTasks, task) - } - } - return fiber.Map{"Tasks": visibleTasks, "Error": err != nil} + return fiber.Map{"Tasks": tasks, "Error": err != nil} }) // UploadDocumentMaxSize <= 0 means "unlimited", but fiber has no such