Fix extraction limits and strategy

cmd/root.go

@@ -17,7 +17,7 @@ import (
 )
 
 const (
-	version               = "0.8.0"
+	version               = "0.8.1"
 	defaultConfigFilename = "config"
 	envPrefix             = "OPENSERP"
 )

core/server_extract.go

@@ -180,6 +180,10 @@ func (s *Server) enrichEnvelopeWithExtraction(ctx context.Context, env *Envelope
 	if limit > len(env.Results) {
 		limit = len(env.Results)
 	}
+	candidateLimit := limit + 3
+	if candidateLimit > len(env.Results) {
+		candidateLimit = len(env.Results)
+	}
 
 	// Per-fetch timeouts bound a single URL; this aggregate deadline bounds the
 	// whole batch so a few slow/hanging targets can't stretch the search request
@@ -187,9 +191,46 @@ func (s *Server) enrichEnvelopeWithExtraction(ctx context.Context, env *Envelope
 	// Config.BatchTimeout) rather than a separate knob. When it fires, in-flight
 	// fetches are cancelled and any not yet started record a timeout error instead
 	// of a result — never a 500.
-	ctx, cancel := context.WithTimeout(ctx, cfg.BatchTimeout(limit))
+	ctx, cancel := context.WithTimeout(ctx, cfg.BatchTimeout(candidateLimit))
 	defer cancel()
 
+	extractOne := func(idx int) {
+		// Skip the fetch entirely if the batch budget is already spent.
+		if err := ctx.Err(); err != nil {
+			env.Results[idx].Extracted = &ExtractedContent{Error: sanitizeExtractError(err)}
+			return
+		}
+		req := extractpkg.ExtractRequest{
+			URL:      env.Results[idx].URL,
+			Mode:     extractpkg.Mode(q.ExtractMode),
+			ProxyURL: q.ProxyURL,
+			LangCode: q.LangCode,
+			Timeout:  cfg.Timeout,
+			MaxBytes: cfg.MaxBytes,
+			MinRunes: q.ExtractMinRunes,
+		}
+		result, err := extractor.Extract(ctx, req)
+		if err != nil {
+			env.Results[idx].Extracted = &ExtractedContent{Error: sanitizeExtractError(err)}
+			return
+		}
+		content := result.Markdown
+		if contentFormat == "text" {
+			content = result.Text
+		}
+		if !extractedContentLooksUseful(content) {
+			env.Results[idx].Extracted = &ExtractedContent{Error: "empty extracted content"}
+			return
+		}
+		env.Results[idx].Extracted = &ExtractedContent{
+			Title:     result.Title,
+			Format:    contentFormat,
+			Content:   content,
+			ModeUsed:  result.Meta.ModeUsed,
+			FetchedAt: result.Meta.FetchedAt,
+		}
+	}
+
 	sem := make(chan struct{}, cfg.MaxConcurrent)
 	var wg sync.WaitGroup
 	for i := 0; i < limit; i++ {
@@ -201,39 +242,43 @@ func (s *Server) enrichEnvelopeWithExtraction(ctx context.Context, env *Envelope
 		go func(idx int) {
 			defer wg.Done()
 			defer func() { <-sem }()
-			// Skip the fetch entirely if the batch budget is already spent.
-			if err := ctx.Err(); err != nil {
-				env.Results[idx].Extracted = &ExtractedContent{Error: sanitizeExtractError(err)}
-				return
-			}
-			req := extractpkg.ExtractRequest{
-				URL:      env.Results[idx].URL,
-				Mode:     extractpkg.Mode(q.ExtractMode),
-				ProxyURL: q.ProxyURL,
-				LangCode: q.LangCode,
-				Timeout:  cfg.Timeout,
-				MaxBytes: cfg.MaxBytes,
-				MinRunes: q.ExtractMinRunes,
-			}
-			result, err := extractor.Extract(ctx, req)
-			if err != nil {
-				env.Results[idx].Extracted = &ExtractedContent{Error: sanitizeExtractError(err)}
-				return
-			}
-			content := result.Markdown
-			if contentFormat == "text" {
-				content = result.Text
-			}
-			env.Results[idx].Extracted = &ExtractedContent{
-				Title:     result.Title,
-				Format:    contentFormat,
-				Content:   content,
-				ModeUsed:  result.Meta.ModeUsed,
-				FetchedAt: result.Meta.FetchedAt,
-			}
+			extractOne(idx)
 		}(i)
 	}
 	wg.Wait()
+
+	successes := extractedSuccessCount(env.Results[:limit])
+	for i := limit; successes < limit && i < candidateLimit; i++ {
+		if strings.TrimSpace(env.Results[i].URL) == "" {
+			continue
+		}
+		extractOne(i)
+		if extractedResultSucceeded(env.Results[i]) {
+			successes++
+		}
+	}
+}
+
+const minUsefulExtractRunes = 80
+
+func extractedContentLooksUseful(content string) bool {
+	return len([]rune(strings.TrimSpace(content))) >= minUsefulExtractRunes
+}
+
+func extractedSuccessCount(results []Result) int {
+	count := 0
+	for _, result := range results {
+		if extractedResultSucceeded(result) {
+			count++
+		}
+	}
+	return count
+}
+
+func extractedResultSucceeded(result Result) bool {
+	return result.Extracted != nil &&
+		result.Extracted.Error == "" &&
+		extractedContentLooksUseful(result.Extracted.Content)
 }
 
 func sendExtractResult(c *fiber.Ctx, format string, result *extractpkg.ExtractResult) error {

core/server_extract_test.go

@@ -0,0 +1,61 @@
+package core
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+	"time"
+
+	extractpkg "github.com/karust/openserp/extract"
+)
+
+func TestEnrichEnvelopeWithExtractionRetriesThinAndFailedCandidates(t *testing.T) {
+	target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch r.URL.Path {
+		case "/thin":
+			w.WriteHeader(http.StatusOK)
+			_, _ = w.Write([]byte(`<html><head><title>tripadvisor.com</title></head><body>tripadvisor.com</body></html>`))
+		case "/blocked":
+			w.WriteHeader(http.StatusBadGateway)
+		case "/useful":
+			w.WriteHeader(http.StatusOK)
+			_, _ = w.Write([]byte(`<html><body><article><h1>Useful page</h1><p>This useful page has enough body text to count as extracted content and should be selected after earlier candidates fail.</p></article></body></html>`))
+		default:
+			w.WriteHeader(http.StatusNotFound)
+		}
+	}))
+	defer target.Close()
+
+	opts := DefaultServerOptions()
+	opts.Extract = extractpkg.Config{
+		Enabled:       true,
+		DefaultMode:   string(extractpkg.ModeFast),
+		Timeout:       time.Second,
+		MaxBytes:      256 * 1024,
+		MaxConcurrent: 2,
+	}
+	s := &Server{opts: opts}
+	env := &Envelope{Results: []Result{
+		{URL: target.URL + "/thin"},
+		{URL: target.URL + "/blocked"},
+		{URL: target.URL + "/useful"},
+	}}
+	q := Query{Extract: true, ExtractTop: 1, ExtractMode: string(extractpkg.ModeFast)}
+
+	s.enrichEnvelopeWithExtraction(context.Background(), env, q, "json")
+
+	if env.Results[0].Extracted == nil || env.Results[0].Extracted.Error != "empty extracted content" {
+		t.Fatalf("first candidate extracted = %+v, want empty-content error", env.Results[0].Extracted)
+	}
+	if env.Results[1].Extracted == nil || env.Results[1].Extracted.Error == "" {
+		t.Fatalf("second candidate extracted = %+v, want failure error", env.Results[1].Extracted)
+	}
+	if env.Results[2].Extracted == nil || env.Results[2].Extracted.Error != "" {
+		t.Fatalf("third candidate extracted = %+v, want successful retry", env.Results[2].Extracted)
+	}
+	if !strings.Contains(env.Results[2].Extracted.Content, "Useful page") {
+		t.Fatalf("third candidate content = %q", env.Results[2].Extracted.Content)
+	}
+}