diff --git a/docs/reference/search-api-graphql.md b/docs/reference/search-api-graphql.md index be036f31..2cfc508c 100644 --- a/docs/reference/search-api-graphql.md +++ b/docs/reference/search-api-graphql.md @@ -64,6 +64,28 @@ const gqlSchema = buildGraphQLSchema(searchSchema(DATASET, PERSON), { }); ``` +A `tieBreak` orders the results that the primary sort leaves tied. Sorted by +date, all datasets registered in one go share a date and come back in whatever +order the engine stored them. A reindex can change that order, so a client +paging through the block may see a dataset twice or miss one: + +```ts +types: { + Dataset: { tieBreak: [{ field: 'title', direction: 'asc' }] }, +}, +``` + +The tie-break is appended after the sort that the request and `queryDefaults` +settled on. Terms on a field that is already sorted on are skipped. A query +without a sort keeps the engine’s default order – relevance for a free-text +query, the collection’s default sorting field otherwise. To have ties broken +there too, set a default sort in `queryDefaults`. A facet-only query +(`perPage: 0`) gets no tie-break at all. Clients never see the tie-break in the +`orderBy` input. It may have at most two terms, each on a `sortable` field, +because Typesense sorts on at most three terms and one is the primary sort. +Documents that tie on every term, such as two datasets with the same title, +still come back in storage order. + Shared types (`LanguageString`, the facet buckets, filter inputs and reference types such as a common `Agent`) are created once and reused across root types. diff --git a/packages/search-api-graphql/src/build-schema.ts b/packages/search-api-graphql/src/build-schema.ts index 9dae0ff8..80e8c063 100644 --- a/packages/search-api-graphql/src/build-schema.ts +++ b/packages/search-api-graphql/src/build-schema.ts @@ -30,10 +30,12 @@ import { type SearchQuery, type SearchSchema, type SearchType, + type Sort, } from '@lde/search'; import { AND_KEY, facetableFields, + fieldNamed, filterableFields, labelTargetNameOf, localLookupTypeOf, @@ -84,12 +86,31 @@ export interface SearchTypeOptions { /** Root query field; defaults to the lowercased plural of the type’s `name` * (e.g. `Dataset` → `datasets`). */ readonly queryField?: string; - /** Consumer policy applied to every query of this type (default status, sort, - * tie-breaks). */ + /** Consumer policy applied to every query of this type (default status, + * default sort). */ readonly queryDefaults?: ( query: SearchQuery, context: SearchContext, ) => SearchQuery; + /** + * Sorts that order results the primary sort leaves tied, in precedence + * order – e.g. `[{ field: 'title', direction: 'asc' }]`, so a block of + * datasets sharing a date comes back in the same order on every page. + * + * Appended after the sort the request and {@link queryDefaults} settled on; + * a term on a field already sorted on is skipped. A query with no sort keeps + * the engine’s default order untouched – set a default sort in + * {@link queryDefaults} to have the tie-break follow it. A facet-only query + * (`perPage: 0`) fetches no results to order and gets none. + * + * Deployment policy rather than a client choice, so it adds nothing to the + * `orderBy` input. Each term names a `sortable` field – the only kind an + * engine indexes for sorting – and there are at most two, so a single + * primary sort plus the tie-break stays within Typesense’s cap of three + * terms. A {@link queryDefaults} sort of two terms or more leaves room for + * less; the engine rejects a longer sort per query, naming its terms. + */ + readonly tieBreak?: readonly Sort[]; } export interface BuildGraphQLSchemaOptions { @@ -231,12 +252,28 @@ export function buildGraphQLSchema( [...schema.values()].map((searchType) => [searchType.name, searchType]), ); const rootTypeNames = new Set(rootTypesByName.keys()); - for (const name of Object.keys(options.types ?? {})) { - if (!rootTypeNames.has(name)) { + for (const [name, typeOptions] of Object.entries(options.types ?? {})) { + const rootType = rootTypesByName.get(name); + if (rootType === undefined) { throw new Error( `Options given for type “${name}”, which is not in the search schema.`, ); } + const tieBreak = typeOptions.tieBreak ?? []; + if (tieBreak.length > MAX_TIE_BREAK_TERMS) { + throw new Error( + `Tie-break for type “${name}” may have at most ${MAX_TIE_BREAK_TERMS} terms; got ${tieBreak.length}. It follows a primary sort, and Typesense sorts on 3 terms at most.`, + ); + } + for (const sort of tieBreak) { + // Only a `sortable` field is indexed for sorting, so any other would + // build fine and fail every query. + if (fieldNamed(rootType, sort.field)?.sortable !== true) { + throw new Error( + `Tie-break for type “${name}” sorts on “${sort.field}”, which is not sortable. Declare it on the type with \`sortable: true\`.`, + ); + } + } } const languageString = new GraphQLObjectType({ @@ -1045,9 +1082,12 @@ export function buildGraphQLSchema( // What the client selected decides what each lookup fetches, so the // engine carries the referent fields this query asked for and no more. const resolve = projectionFor(info, searchType, schema); - const finalQuery = typeOptions?.queryDefaults - ? typeOptions.queryDefaults(built, context) - : built; + const finalQuery = withTieBreak( + typeOptions?.queryDefaults + ? typeOptions.queryDefaults(built, context) + : built, + typeOptions?.tieBreak ?? [], + ); // Items + total only; facets are resolved lazily per selected key. const result = await context.engine.search(searchType, { ...finalQuery, @@ -1213,6 +1253,30 @@ function argsToQuery( }; } +/** One primary sort plus the tie-break stays within Typesense’s three terms. */ +const MAX_TIE_BREAK_TERMS = 2; + +/** Append the {@link SearchTypeOptions.tieBreak} terms to a query’s `orderBy`, + * as that option documents. */ +function withTieBreak( + query: SearchQuery, + tieBreak: readonly Sort[], +): SearchQuery { + // An empty `orderBy` leaves the order to the engine’s own default (relevance, + // a default sorting field), which a tie-break alone would replace. + if (query.orderBy.length === 0 || query.limit === 0) { + return query; + } + const sorted = new Set(query.orderBy.map((sort) => sort.field)); + return { + ...query, + orderBy: [ + ...query.orderBy, + ...tieBreak.filter((sort) => !sorted.has(sort.field)), + ], + }; +} + /** * Compile one `Where` input into the flat conjunction of disjunctions the IR * holds, at `on` hops out from the searched type (`[]` at the top level). diff --git a/packages/search-api-graphql/test/build-schema.test.ts b/packages/search-api-graphql/test/build-schema.test.ts index 762da19e..9d3359d3 100644 --- a/packages/search-api-graphql/test/build-schema.test.ts +++ b/packages/search-api-graphql/test/build-schema.test.ts @@ -859,6 +859,132 @@ describe('buildGraphQLSchema', () => { ]); }); + describe('tieBreak', () => { + const tieBreak = [{ field: 'title', direction: 'asc' }] as const; + + async function orderByFor( + source: string, + typeOptions: Parameters[1] = { + types: { [schema.name]: { tieBreak } }, + }, + ) { + const { engine, received } = fakeEngine(canned); + const result = await graphql({ + schema: buildGraphQLSchema( + searchSchema(schema, organization, term), + typeOptions, + ), + source, + contextValue: { engine, acceptLanguage: ['nl'] }, + }); + expect(result.errors).toBeUndefined(); + return received().orderBy; + } + + it('appends the tie-break after the requested sort', async () => { + expect( + await orderByFor( + `{ datasets(orderBy: { field: DATE_POSTED }) { pagination { total } } }`, + ), + ).toEqual([ + { field: 'datePosted', direction: 'desc' }, + { field: 'title', direction: 'asc' }, + ]); + }); + + it('skips a tie-break term on the field already sorted on', async () => { + expect( + await orderByFor( + `{ datasets(orderBy: { field: TITLE }) { pagination { total } } }`, + ), + ).toEqual([{ field: 'title', direction: 'desc' }]); + }); + + it('leaves a query without a sort to the engine’s default order', async () => { + expect( + await orderByFor( + `{ datasets(query: "atlas") { pagination { total } } }`, + ), + ).toEqual([]); + }); + + it('follows an explicit relevance sort', async () => { + expect( + await orderByFor( + `{ datasets(query: "atlas", orderBy: { field: RELEVANCE }) { pagination { total } } }`, + ), + ).toEqual([ + { field: 'relevance', direction: 'desc' }, + { field: 'title', direction: 'asc' }, + ]); + }); + + it('adds no tie-break to a facet-only query', async () => { + expect( + await orderByFor( + `{ datasets(perPage: 0, orderBy: { field: DATE_POSTED }) { pagination { total } } }`, + ), + ).toEqual([{ field: 'datePosted', direction: 'desc' }]); + }); + + it('breaks ties in the sort a queryDefaults policy chose', async () => { + expect( + await orderByFor(`{ datasets { pagination { total } } }`, { + types: { + [schema.name]: { + tieBreak, + queryDefaults: (query) => ({ + ...query, + orderBy: [{ field: 'datePosted', direction: 'desc' }], + }), + }, + }, + }), + ).toEqual([ + { field: 'datePosted', direction: 'desc' }, + { field: 'title', direction: 'asc' }, + ]); + }); + + it('rejects a tie-break on an undeclared field', () => { + expect(() => + buildGraphQLSchema(searchSchema(schema, organization, term), { + types: { + [schema.name]: { tieBreak: [{ field: 'nope', direction: 'asc' }] }, + }, + }), + ).toThrow(/tie-break.*“nope”.*not sortable/i); + }); + + it('rejects a tie-break on a field the engine does not sort', () => { + expect(() => + buildGraphQLSchema(searchSchema(schema, organization, term), { + types: { + [schema.name]: { + tieBreak: [{ field: 'keyword', direction: 'asc' }], + }, + }, + }), + ).toThrow(/tie-break.*“keyword”.*not sortable/i); + }); + + it('rejects a tie-break too long to follow a primary sort', () => { + expect(() => + buildGraphQLSchema(searchSchema(schema, organization, term), { + types: { + [schema.name]: { + tieBreak: [ + { field: 'title', direction: 'asc' }, + { field: 'size', direction: 'asc' }, + { field: 'datePosted', direction: 'asc' }, + ], + }, + }, + }), + ).toThrow(/at most 2 terms; got 3/); + }); + }); + it('derives nullability: required scalar non-null, optional scalar nullable, arrays/booleans non-null', () => { const sdl = printSchema( buildGraphQLSchema( diff --git a/packages/search-api-graphql/vite.config.ts b/packages/search-api-graphql/vite.config.ts index d4ea5d9a..51f6eead 100644 --- a/packages/search-api-graphql/vite.config.ts +++ b/packages/search-api-graphql/vite.config.ts @@ -29,7 +29,7 @@ export default mergeConfig( // Re-anchored for the selection-set projection: its defensive reads // (an unresolvable target, an absent field list) are reachable only // from a direct caller, since GraphQL validates the query first. - branches: 97.05, + branches: 97.16, statements: 100, }, }, diff --git a/packages/search-typesense/src/query-compiler.ts b/packages/search-typesense/src/query-compiler.ts index cdc0bc2f..b15d80d2 100644 --- a/packages/search-typesense/src/query-compiler.ts +++ b/packages/search-typesense/src/query-compiler.ts @@ -125,6 +125,16 @@ export function buildSearchParams( options.schema, ); const filterBy = compileFilterBy(query.where, searchType, options); + // Typesense answers a longer `sort_by` with an error naming no field; say + // which terms, since some came from a policy (a tie-break) the client + // never wrote. + if (query.orderBy.length > MAX_SORT_TERMS) { + throw new Error( + `Typesense sorts on at most ${MAX_SORT_TERMS} sort terms; got ${query.orderBy.length} (${query.orderBy + .map((sort) => sort.field) + .join(', ')}). Shorten the tie-break or the default sort.`, + ); + } const sortBy = query.orderBy .map((sort) => compileSort(sort, searchType, query.locale)) .join(','); @@ -630,6 +640,9 @@ function storedBound( : bound; } +/** Typesense’s cap on `sort_by` terms, `_text_match` included. */ +const MAX_SORT_TERMS = 3; + /** * One `sort_by` term. `relevance` maps to Typesense’s `_text_match`; a localized * text field sorts on its active-locale folded key; any other field (including a diff --git a/packages/search-typesense/test/query-compiler.test.ts b/packages/search-typesense/test/query-compiler.test.ts index 123f6236..3a4f6ef4 100644 --- a/packages/search-typesense/test/query-compiler.test.ts +++ b/packages/search-typesense/test/query-compiler.test.ts @@ -520,6 +520,23 @@ describe('buildSearchParams', () => { ).toBe('title_sort_nl:asc,status_rank:asc'); }); + it('rejects more sort terms than Typesense accepts', () => { + expect(() => + buildSearchParams( + { + ...base, + orderBy: [ + { field: 'datePosted', direction: 'desc' }, + { field: 'title', direction: 'asc' }, + { field: 'size', direction: 'asc' }, + { field: 'status_rank', direction: 'asc' }, + ], + }, + schema, + ), + ).toThrow(/at most 3 sort terms.*datePosted, title, size, status_rank/); + }); + it('pins page to 1 for a facet-only (limit:0) query instead of dividing by zero', () => { const params = buildSearchParams({ ...base, limit: 0 }, schema); expect(params.per_page).toBe(0); diff --git a/packages/search-typesense/test/tie-break.integration.test.ts b/packages/search-typesense/test/tie-break.integration.test.ts new file mode 100644 index 00000000..dbaeedfe --- /dev/null +++ b/packages/search-typesense/test/tie-break.integration.test.ts @@ -0,0 +1,103 @@ +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import type { Client } from 'typesense'; +import { + defineSearchType, + searchSchema, + type SearchQuery, + type Sort, +} from '@lde/search'; +import { Dataset } from '@lde/dataset'; +import { BlueGreenRebuild } from '../src/blue-green-rebuild.js'; +import { createTypesenseSearchEngine } from '../src/search.js'; +import { TypesenseContainer } from './typesense-container.js'; +import { makeRunContext, stream } from './helpers.js'; + +/** + * A tie-break sort, pinned against a real Typesense: a block of documents + * sharing a date comes back in one order on every page, and in the same order + * after a rebuild that inserted them the other way round. Without the + * tie-break, Typesense orders ties by insertion, so a rebuild reshuffles them + * and a client paging through the block sees some twice and misses others. + */ +describe('a tie-break sort', () => { + const container = new TypesenseContainer(); + let client: Client; + + const work = defineSearchType({ + name: 'Work', + class: 'https://schema.org/CreativeWork', + fields: [ + { name: 'title', kind: 'keyword', output: true, sortable: true }, + { name: 'datePosted', kind: 'date', output: true, sortable: true }, + ], + }); + const dataset = new Dataset({ + iri: new URL('http://example.org/dataset/1'), + distributions: [], + }); + const sameDate = 1_750_000_000; + const titles = ['Delta', 'Alfa', 'Echo', 'Charlie', 'Bravo', 'Foxtrot']; + const documents = titles.map((title) => ({ + id: `https://work/${title}`, + title, + datePosted: sameDate, + })); + + const byDate: Sort = { field: 'datePosted', direction: 'desc' }; + const byTitle: Sort = { field: 'title', direction: 'asc' }; + + async function rebuild(inOrder: typeof documents): Promise { + const run = await new BlueGreenRebuild<(typeof documents)[number]>( + client, + work, + ).openRun(makeRunContext([dataset.iri.toString()])); + await run.write(dataset, stream(inOrder)); + await run.commit(); + } + + /** The titles on each page of three, in the order the engine returns. */ + async function pages(orderBy: readonly Sort[]): Promise { + const engine = createTypesenseSearchEngine(client, searchSchema(work)); + const page = async (offset: number) => { + const query: SearchQuery = { + where: [], + orderBy, + limit: 3, + offset, + facets: [], + locale: 'und', + }; + const result = await engine.search(work, query); + return result.hits.map((hit) => String(hit.document.title)); + }; + return [await page(0), await page(3)]; + } + + beforeAll(async () => { + client = await container.start(); + }, 120_000); + + afterAll(async () => { + await container.stop(); + }); + + it('pages a block of equal dates in the same order before and after a rebuild', async () => { + await rebuild(documents); + const before = await pages([byDate, byTitle]); + const untieBefore = await pages([byDate]); + + await rebuild([...documents].reverse()); + const after = await pages([byDate, byTitle]); + const untieAfter = await pages([byDate]); + + const expected = [ + ['Alfa', 'Bravo', 'Charlie'], + ['Delta', 'Echo', 'Foxtrot'], + ]; + expect(before).toEqual(expected); + expect(after).toEqual(expected); + // The control: without the tie-break the rebuild does reshuffle the block, + // so the assertions above are not passing by accident. + expect(untieAfter).not.toEqual(untieBefore); + }); +}); diff --git a/packages/search-typesense/vite.config.ts b/packages/search-typesense/vite.config.ts index 15ebf0ea..69821279 100644 --- a/packages/search-typesense/vite.config.ts +++ b/packages/search-typesense/vite.config.ts @@ -28,7 +28,7 @@ export default mergeConfig( // projection naming what no lookup reaches are unreachable through // the port, since `assertValidQuery` rejects such a query first. // They hold for a direct caller, and are exercised as one. - branches: 95.89, + branches: 95.9, statements: 99.48, }, },