feat(search)!: batch facet searches into a single multi_search#554
Merged
Conversation
- add searchFacets(searchType, queries) to the SearchEngine port: facet-only queries answered positionally, one facet map per query - Typesense adapter sends the whole batch as a single multi_search, compiles every entry facet-only (per_page 0), surfaces failed entries as rejections, and shares one label lookup (or the label cache) across the batch - GraphQL surface: selected facet fields load through a per-request DataLoader that groups them by effective where (skip-own-filter) and dispatches ONE searchFacets call; the unfiltered browse collapses to a single facet query - facet-only queries drop the listing orderBy; a failed batch degrades every selected facet to an empty list, reported per field via onFacetError - extend the port contract suite in @lde/search/testing to cover searchFacets BREAKING CHANGE: SearchEngine implementations must add the searchFacets method.
- searchFacets returns one FacetsOutcome ({ facets } or { error }) per query,
so one failed query no longer discards its siblings' facets: the surface
degrades exactly the failed query's facets and reports each via onFacetError
- the Typesense adapter passes a failed multi_search entry through as an
in-place error naming the query's facet fields, and normalizes orderBy
away alongside limit/offset (facet-only compiles carry no sort)
- fetchLabels now throws on an inline multi_search error entry, engaging the
label degradation path instead of silently missing every label
- a missing outcome (port-contract breach) is reported, not read as empty
- restore the lazy iris thunk so the cached-label path skips collecting them
- record the decision as ADR 5 and amend the port snippet in ADR 3
BREAKING CHANGE: searchFacets returns FacetsOutcome[] instead of FacetMap[].
- one configurable fake (search, export, multi_search; recorded performs and export calls) replaces the five bespoke fakes in parse-response.test.ts - labelLookup() shares the filter_by id-list answering between the fetchLabels and bundled-label-lookup tests - re-anchor coverage thresholds: the helper's defensive guards are uncovered
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix #532
The keyed
DatasetFacetssurface resolved each selected facet through its ownengine.searchcall, so a typical listing page fanned out into 1 listing search + N facet searches. This PR batches the whole facet selection into one engine round-trip, restoring the pre-migration single-multi_searchbehaviour: a typical page now costs the listing search plus one batched facet search (with the label cache on, exactly 2 Typesense round-trips). Recorded as ADR 5.Port (
@lde/search) — breakingSearchEnginegains the batch entry pointsearchFacets(searchType, queries), returning oneFacetsOutcomeper query, positionally:{ facets }or{ error }(anallSettled-style union). A failure of one query is reported in place and never discards its siblings’ facets; the promise rejects only for batch-level failures (foreign type, invalid query, transport). Engines answer every query facet-only — as iflimit: 0and without ordering, whatever the query carries — so hits are never transferred. ExistingSearchEngineimplementations must add the method.@lde/search/testing) covers the new method: foreign-type rejection, per-query validation, positional alignment (including a non-zero-limit query), and the empty batch.Typesense adapter (
@lde/search-typesense)searchFacetscompiles each query with the existingbuildSearchParams(normalized toper_page: 0, nosort_by) and sends the whole batch as onemulti_search.multi_searchentry (reported inline by Typesense) becomes a per-query error outcome naming the affected facet fields, so its siblings’ facets survive;fetchLabelsnow also throws on an inline error entry, engaging the label degradation path (onLabelError, id-only references) instead of silently missing every label.labelCacheTtlMsis set; the label-degradation logic is shared betweensearchandsearchFacets.GraphQL surface (
@lde/search-api-graphql)facet-batch.ts): same-tick loads are collected, grouped by their effectivewhereafter skip-own-filter — facets whose own field is unfiltered share one query, so the unfiltered browse collapses to a single facet query; each own-filtered facet gets its own variant — and dispatched as oneengine.searchFacetscall.onFacetErrorfires only for the fields of the query that actually failed), reference labels, and the public SDL are unchanged. Facet-only queries drop the listingorderBy(nothing to order).dataloader(GraphQL Foundation’s reference batching implementation) instead of a hand-rolled microtask batcher.Deliberately not done
wherefacets on the main listing search viafacet_byfor a 1-round-trip browse — needsGraphQLResolveInfoselection-walking; inside onemulti_searchthe difference is negligible (see ADR 5).