diff --git a/internal/api/handlers/catalog.go b/internal/api/handlers/catalog.go index 47b03a1b..00dd75d1 100644 --- a/internal/api/handlers/catalog.go +++ b/internal/api/handlers/catalog.go @@ -53,11 +53,24 @@ func (h *CatalogHandler) SetWorkSummaryProvider(provider catalog.WorkSummaryProv } type catalogResponse struct { - Total int `json:"total"` - TotalExact bool `json:"total_exact"` - HasMore bool `json:"has_more"` - Items []itemListResponse `json:"items"` - Snapshot string `json:"snapshot,omitempty"` + Total int `json:"total"` + TotalExact bool `json:"total_exact"` + HasMore bool `json:"has_more"` + Items []itemListResponse `json:"items"` + Snapshot string `json:"snapshot,omitempty"` + SearchDiagnostics *searchDiagnostics `json:"search_diagnostics,omitempty"` +} + +// searchDiagnostics is an additive, per-query observability object emitted on +// /api/v1/catalog only when a relevance-sorted search actually ran through a +// CatalogSearchProvider. mode/semantic_used reflect POST-downgrade reality +// (a hybrid request that fell back to keyword reports mode="keyword", +// semantic_used=false). fallback_reason is omitted when empty. +type searchDiagnostics struct { + Provider string `json:"provider"` + Mode string `json:"mode"` + SemanticUsed bool `json:"semantic_used"` + FallbackReason string `json:"fallback_reason,omitempty"` } type catalogFiltersResponse struct { @@ -215,12 +228,26 @@ func (h *CatalogHandler) writeCatalogResponse(w http.ResponseWriter, result *cat snapshot = result.SnapshotAt.Format(time.RFC3339Nano) } + // A non-empty Provider is the single gate: only the direct-search path sets + // it. Browse / preview / non-relevance-sort q= (which never run a provider) + // and group=work (fresh CatalogResult with empty Provider) all omit it. + var diag *searchDiagnostics + if result.Provider != "" { + diag = &searchDiagnostics{ + Provider: result.Provider, + Mode: result.Mode, + SemanticUsed: result.SemanticUsed, + FallbackReason: result.FallbackReason, + } + } + writeJSON(w, http.StatusOK, catalogResponse{ - Total: result.Total, - TotalExact: result.TotalExact && !groupedByWork, - HasMore: result.HasMore, - Items: items, - Snapshot: snapshot, + Total: result.Total, + TotalExact: result.TotalExact && !groupedByWork, + HasMore: result.HasMore, + Items: items, + Snapshot: snapshot, + SearchDiagnostics: diag, }) } diff --git a/internal/api/handlers/catalog_diagnostics_test.go b/internal/api/handlers/catalog_diagnostics_test.go new file mode 100644 index 00000000..cc686604 --- /dev/null +++ b/internal/api/handlers/catalog_diagnostics_test.go @@ -0,0 +1,119 @@ +package handlers + +import ( + "encoding/json" + "net/http/httptest" + "testing" + + "github.com/Silo-Server/silo-server/internal/catalog" +) + +// decodeCatalogResponse runs writeCatalogResponse against an httptest recorder +// and returns the decoded JSON body as a generic map so individual keys can be +// asserted for presence/absence (search_diagnostics is omitempty). +func decodeCatalogResponse(t *testing.T, result *catalog.CatalogResult, grouped bool) map[string]any { + t.Helper() + rec := httptest.NewRecorder() + // writeCatalogResponse uses no handler state, so a zero-value handler is fine. + (&CatalogHandler{}).writeCatalogResponse(rec, result, []itemListResponse{}, grouped) + + var body map[string]any + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("decoding catalog response: %v\nbody: %s", err, rec.Body.String()) + } + return body +} + +func TestWriteCatalogResponse_DiagnosticsKeywordFallback(t *testing.T) { + body := decodeCatalogResponse(t, &catalog.CatalogResult{ + Total: 3, + TotalExact: true, + HasMore: false, + Provider: catalog.SearchProviderMeilisearch, + Mode: "keyword", + SemanticUsed: false, + FallbackReason: `semantic_not_ready: type "movie" coverage 40% below threshold`, + }, false) + + // Existing keys must stay present and unchanged (byte-stable contract). + for _, key := range []string{"total", "total_exact", "has_more", "items"} { + if _, ok := body[key]; !ok { + t.Fatalf("response missing existing key %q: %v", key, body) + } + } + if body["total"].(float64) != 3 { + t.Fatalf("total = %v, want 3", body["total"]) + } + if body["total_exact"].(bool) != true { + t.Fatalf("total_exact = %v, want true", body["total_exact"]) + } + + diagRaw, ok := body["search_diagnostics"] + if !ok { + t.Fatalf("expected search_diagnostics in response: %v", body) + } + diag := diagRaw.(map[string]any) + if diag["provider"] != catalog.SearchProviderMeilisearch { + t.Fatalf("provider = %v, want %q", diag["provider"], catalog.SearchProviderMeilisearch) + } + if diag["mode"] != "keyword" { + t.Fatalf("mode = %v, want keyword", diag["mode"]) + } + if diag["semantic_used"].(bool) != false { + t.Fatalf("semantic_used = %v, want false", diag["semantic_used"]) + } + if diag["fallback_reason"] != `semantic_not_ready: type "movie" coverage 40% below threshold` { + t.Fatalf("fallback_reason = %v", diag["fallback_reason"]) + } +} + +func TestWriteCatalogResponse_DiagnosticsHybridOmitsFallbackReason(t *testing.T) { + body := decodeCatalogResponse(t, &catalog.CatalogResult{ + Provider: catalog.SearchProviderMeilisearch, + Mode: "hybrid", + SemanticUsed: true, + }, false) + + diagRaw, ok := body["search_diagnostics"] + if !ok { + t.Fatalf("expected search_diagnostics in response: %v", body) + } + diag := diagRaw.(map[string]any) + if diag["semantic_used"].(bool) != true { + t.Fatalf("semantic_used = %v, want true", diag["semantic_used"]) + } + if diag["mode"] != "hybrid" { + t.Fatalf("mode = %v, want hybrid", diag["mode"]) + } + if _, ok := diag["fallback_reason"]; ok { + t.Fatalf("fallback_reason should be omitted when empty: %v", diag) + } +} + +func TestWriteCatalogResponse_NoProviderOmitsDiagnostics(t *testing.T) { + // Browse / preview / non-relevance q= paths never set Provider. + body := decodeCatalogResponse(t, &catalog.CatalogResult{ + Total: 5, + TotalExact: true, + }, false) + + if _, ok := body["search_diagnostics"]; ok { + t.Fatalf("search_diagnostics should be omitted when no provider search ran: %v", body) + } +} + +func TestWriteCatalogResponse_GroupedByWorkOmitsDiagnostics(t *testing.T) { + // group=work builds a fresh CatalogResult with an empty Provider. + body := decodeCatalogResponse(t, &catalog.CatalogResult{ + Total: 2, + TotalExact: true, + }, true) + + if _, ok := body["search_diagnostics"]; ok { + t.Fatalf("grouped response should omit search_diagnostics: %v", body) + } + // Grouped responses force total_exact false regardless of result.TotalExact. + if body["total_exact"].(bool) != false { + t.Fatalf("grouped total_exact = %v, want false", body["total_exact"]) + } +} diff --git a/internal/catalog/catalog_resolver.go b/internal/catalog/catalog_resolver.go index 65ef9ff5..09cb8910 100644 --- a/internal/catalog/catalog_resolver.go +++ b/internal/catalog/catalog_resolver.go @@ -27,6 +27,14 @@ type CatalogResult struct { HasMore bool TotalExact bool SnapshotAt time.Time // pagination fence timestamp + // Provider, Mode, SemanticUsed and FallbackReason are per-query search + // diagnostics. They are only populated on the direct-search path (where a + // CatalogSearchProvider actually ran); browse / preview / grouped paths + // leave them zero-valued so the handler omits search_diagnostics. + Provider string + Mode string + SemanticUsed bool + FallbackReason string } type CatalogFiltersResult struct { @@ -289,10 +297,14 @@ func (r *CatalogResolver) resolveDirectSearchSource(ctx context.Context, req Cat } return &CatalogResult{ - Items: result.Items, - Total: result.Total, - HasMore: result.HasMore, - TotalExact: result.TotalExact, + Items: result.Items, + Total: result.Total, + HasMore: result.HasMore, + TotalExact: result.TotalExact, + Provider: result.Provider, + Mode: result.Mode, + SemanticUsed: result.SemanticUsed, + FallbackReason: result.FallbackReason, }, nil } diff --git a/internal/catalog/catalog_resolver_test.go b/internal/catalog/catalog_resolver_test.go index aae92746..e2291cff 100644 --- a/internal/catalog/catalog_resolver_test.go +++ b/internal/catalog/catalog_resolver_test.go @@ -369,6 +369,100 @@ func newTestResolver(exec previewExecutor) *CatalogResolver { } } +// fakeSearchProvider is a CatalogSearchProvider stub that returns a fixed +// CatalogSearchResult so resolveDirectSearchSource's field plumbing can be +// exercised without a database or live search backend. +type fakeSearchProvider struct { + result *CatalogSearchResult +} + +func (f *fakeSearchProvider) Search(_ context.Context, _ CatalogSearchRequest) (*CatalogSearchResult, error) { + return f.result, nil +} + +// TestResolveDirectSearchSource_PlumbsDiagnostics asserts that the four +// diagnostics fields (Provider, Mode, SemanticUsed, FallbackReason) carried by +// the provider's CatalogSearchResult are copied onto the resolver's +// CatalogResult on the direct-search path. Covers both the downgraded-to-keyword +// case (semantic_not_ready) and the hybrid-survived case. +func TestResolveDirectSearchSource_PlumbsDiagnostics(t *testing.T) { + cases := []struct { + name string + result *CatalogSearchResult + }{ + { + name: "keyword fallback carries reason", + result: &CatalogSearchResult{ + Items: []*models.MediaItem{}, + Provider: SearchProviderMeilisearch, + Mode: "keyword", + SemanticUsed: false, + FallbackReason: `semantic_not_ready: type "movie" coverage 40% below threshold`, + }, + }, + { + name: "hybrid survived", + result: &CatalogSearchResult{ + Items: []*models.MediaItem{}, + Provider: SearchProviderMeilisearch, + Mode: "hybrid", + SemanticUsed: true, + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + resolver := &CatalogResolver{ + searchProvider: &fakeSearchProvider{result: tc.result}, + } + + got, err := resolver.resolveDirectSearchSource( + context.Background(), + CatalogRequest{Source: CatalogSourceQuery, SearchQuery: "dune", Limit: 20}, + AccessFilter{}, + ) + if err != nil { + t.Fatalf("resolveDirectSearchSource error: %v", err) + } + if got.Provider != tc.result.Provider { + t.Fatalf("Provider = %q, want %q", got.Provider, tc.result.Provider) + } + if got.Mode != tc.result.Mode { + t.Fatalf("Mode = %q, want %q", got.Mode, tc.result.Mode) + } + if got.SemanticUsed != tc.result.SemanticUsed { + t.Fatalf("SemanticUsed = %v, want %v", got.SemanticUsed, tc.result.SemanticUsed) + } + if got.FallbackReason != tc.result.FallbackReason { + t.Fatalf("FallbackReason = %q, want %q", got.FallbackReason, tc.result.FallbackReason) + } + }) + } +} + +// TestResolveDirectSearchSource_EarlyEmptyOmitsDiagnostics asserts that the +// early-empty path (no accessible libraries) returns a zero-valued Provider so +// the handler omits search_diagnostics for it. +func TestResolveDirectSearchSource_EarlyEmptyOmitsDiagnostics(t *testing.T) { + resolver := &CatalogResolver{ + searchProvider: &fakeSearchProvider{result: &CatalogSearchResult{Provider: SearchProviderMeilisearch}}, + } + + got, err := resolver.resolveDirectSearchSource( + context.Background(), + CatalogRequest{Source: CatalogSourceQuery, SearchQuery: "dune", Limit: 20}, + // AllowedLibraryIDs empty (non-nil) => effectiveCatalogLibraryIDs early-empties. + AccessFilter{AllowedLibraryIDs: []int{}}, + ) + if err != nil { + t.Fatalf("resolveDirectSearchSource error: %v", err) + } + if got.Provider != "" { + t.Fatalf("early-empty Provider = %q, want empty", got.Provider) + } +} + // TestPreviewQuerySource_NamePrefix_DoesNotFetchAllRows asserts that the // preview path makes a single PreviewPage call with NamePrefix forwarded into // AccessFilter, instead of the previous fetch-all + Go-side filter pattern diff --git a/internal/catalog/search_meilisearch_provider.go b/internal/catalog/search_meilisearch_provider.go index 31cb52fc..9f640433 100644 --- a/internal/catalog/search_meilisearch_provider.go +++ b/internal/catalog/search_meilisearch_provider.go @@ -282,12 +282,21 @@ func (p *MeilisearchSearchProvider) searchMeilisearch(ctx context.Context, req C total = 0 } p.markFallback(semanticFallback) + // Derive Mode/SemanticUsed from the POST-downgrade request: the hybrid + // downgrade above nils baseSearchReq.Hybrid on error, so a hybrid request + // that fell back to keyword correctly reports keyword / semantic_used=false. + mode, semanticUsed := "keyword", false + if baseSearchReq.Hybrid != nil { + mode, semanticUsed = "hybrid", true + } return &CatalogSearchResult{ Items: page, Total: total, HasMore: hasMore, TotalExact: false, Provider: SearchProviderMeilisearch, + Mode: mode, + SemanticUsed: semanticUsed, FallbackReason: semanticFallback, }, nil } diff --git a/internal/catalog/search_provider.go b/internal/catalog/search_provider.go index 75a954d1..be41d124 100644 --- a/internal/catalog/search_provider.go +++ b/internal/catalog/search_provider.go @@ -62,11 +62,18 @@ type CatalogSearchRequest struct { } type CatalogSearchResult struct { - Items []*models.MediaItem - Total int - HasMore bool - TotalExact bool - Provider string + Items []*models.MediaItem + Total int + HasMore bool + TotalExact bool + Provider string + // Mode reports which retrieval path actually served this result — + // "keyword" or "hybrid". For Meilisearch it reflects POST-downgrade + // reality (a hybrid request that fell back to keyword reports "keyword"). + Mode string + // SemanticUsed is true only when a hybrid request was issued AND survived; + // it goes false on any hybrid->keyword downgrade. + SemanticUsed bool FallbackReason string } @@ -121,6 +128,7 @@ func (p *PostgresSearchProvider) Search(ctx context.Context, req CatalogSearchRe HasMore: hasMore, TotalExact: totalExact, Provider: SearchProviderPostgres, + Mode: "keyword", }, nil }