feat(search)!: replace labelOnly with a lookup strategy naming its target - #739
Open
ddeboer wants to merge 12 commits into
Open
feat(search)!: replace labelOnly with a lookup strategy naming its target#739ddeboer wants to merge 12 commits into
ddeboer wants to merge 12 commits into
Conversation
ddeboer
marked this pull request as ready for review
August 14, 2026 12:14
ddeboer
force-pushed
the
feat/search-lookup-strategy
branch
2 times, most recently
from
August 14, 2026 13:34
f3718b9 to
354e912
Compare
…ction Sets the axis the three reference strategies sit on, replaces labelOnly with lookup, and puts the per-query projection in the decision rather than deferring it: without one there is no bound on how deep a nested lookup resolves.
…rget - ReferenceStrategy is a union per strategy: idOnly carries nothing, lookup names its target Root Type, inline keeps its reference typeName - labelSource is restricted to idOnly, where it labels facet buckets; labelSourceNameOf reads target or labelSource as one - SearchQuery gains an optional resolve projection, validated for this type's own level; deeper levels are the engine's, which has the schema BREAKING CHANGE: strategy 'labelOnly' is now 'lookup', taking 'target' instead of 'labelSource' plus 'ref.typeName'.
- labelSourceNameOf across lookup, idOnly and a reference resolving none - labelSource rejected on a lookup; a lookup target that serves no labels - projection validated at this type's level: unknown and non-lookup fields
validateQuery and assertValidQuery take the schema, so each projection level is checked against the target the level above names – the guard's promise held a hole exactly where the newest feature is. Both production call sites are schema-bound already. BREAKING CHANGE: validateQuery and assertValidQuery take the SearchSchema as a third argument.
…NameOf One reading for a lookup's target and an idOnly's labelSource, so the adapter never branches on strategy to find a collection. Fixtures that declared a typeName and a labelSource for the same type now name it once; the emitted collection schema is unchanged.
The reference that resolved no labels becomes idOnly; the rest name the target they already declared twice.
…ields - a lookup's emitted type derives from its target and carries that root type's output fields, the rule inline already followed; its id is non-null, since a lookup resolves a document by IRI - a target's own references are registered in turn, so a cycle between two targets terminates on the memo rather than recursing - an output idOnly reference must name its emitted type: it is the one strategy that derives no name, and graphql-js failed cryptically on it BREAKING CHANGE: a labelOnly reference becomes a lookup naming its target, and is served with the target's output fields rather than a resolved label alone.
Rebasing onto the IRI-typed filters made an idOnly reference a bare IRI, which leaves no reference emitting an id-plus-label object: a lookup carries its target's fields, an inline reference its Reference Type's. So labelKeyOf and the label-word agreement check go, and with them the missing-ref-type-name rule – an idOnly reference names no emitted type because it emits none. Fixtures that declared a labelOnly reference to a type nothing indexes were serving a label no engine could fill; they are bare IRIs now, or lookups where the target really is a root type.
…llection resolveProjection walks a query's projection level by level: the IRIs of each level are deduped across the whole page, grouped by the collection they live in and fetched in one batched multi_search with include_fields holding exactly what the level asked for. One round-trip per level, not per document, and a level that cannot be fetched degrades its references to bare ids rather than failing the search. The referents reconstruct through the same path a hit does, read through the target's own declaration, so a consumer cannot tell a projected referent from an inline one. The facet path is untouched: a bucket carries one label and stays on the cacheable label lookup.
- the strategy table, the reference prose and three examples described the removed labelOnly; copied today they threw - a section on projecting what a lookup carries, since what it fetches is named per query rather than declared - CONTEXT.md gains Target and Reference Projection, and marks labelOnly as a word to avoid - corrects a JSDoc claim that an output idOnly reference must name a type: it surfaces as a bare IRI, so there is no type to name
A lookup derives its emitted name from the target it already names, and an idOnly reference is served as a bare IRI, so neither can be nameless.
ddeboer
force-pushed
the
feat/search-lookup-strategy
branch
from
August 14, 2026 13:44
354e912 to
da71d41
Compare
Every GraphQL client injects __typename into each selection set, and it was carried into the projection verbatim: the port's guard then reported it as an unknown field of the target and threw, failing the whole search for any query selecting a lookup. The projection now asks only for what the target declares as output. Two more, from the same review: - one lookup selected twice (two fragments spreading it) merged its deeper levels shallowly, so the second selection replaced the first and a field the client asked for was never fetched - a lookup declaring no target passed validation, since the type-name rule had been narrowed to inline alone
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.
A
labelOnlyreference carried{ id, label }and nothing else, so any otherfield of the referent cost a second query keyed off the id just received. The
round-trip that would carry those fields is already made: the label lookup
dedupes the page's referenced IRIs, groups them by collection, fires one batched
multi_search– then keeps the label and discards the document.labelOnlyis replaced bylookup, which names its target once and carriesthat target's fields. Design and rejected alternatives are in
ADR 20;
it differs from the proposal in this issue in one way, argued there: no
per-reference
fieldslist, because two references on one target withdifferent lists would both want to be
‹Target›Referencewith different shapes.What is fetched is named per query instead.
The model
Three strategies on one axis – where a reference's fields come from:
idOnlyIRIlookupinlinetargetnames the Root Type once – the collection its fields and labels areread from, and the name its emitted type derives from. It replaces
labelSourceand
ref.typeName, which for every reallabelOnlywere the same typedeclared twice.
labelSourcesurvives only onidOnly, which emits no type butstill labels its facet buckets.
What a query asks for
SearchQuerygains aresolveprojection, and the GraphQL surface builds itfrom the client's selection set:
→
resolve: { dataset: { fields: ['license'], resolve: { publisher: {…} } } }The Typesense adapter answers that with one batched round-trip per level:
each level's IRIs are deduped across the whole page, grouped by collection, and
fetched with
include_fieldsholding exactly what the level named. A level thatcannot be fetched degrades its references to bare ids rather than failing the
search. Referents reconstruct through the same path a hit does, read through the
target's own declaration, so a consumer cannot tell a projected referent from an
inline one.
Omitted, a lookup carries its target's label alone – what every reference
carried before. Facet buckets are untouched: a bucket is a value, a count and
one label, so it stays on the cacheable label lookup.
validateQuery/assertValidQuerynow take the schema and check a projection atevery level – each level's fields belong to the target the level above names,
which only the schema resolves.
Consequences worth reviewing
idOnlya bareIRI (feat(search)!: type a filter by what its field keys on #728) and
labelOnlygone,labelKeyOfand the label-word agreementcheck added in feat(search): let a search type name its label field #729 have no subject left, and are removed.
labelOnlyreference to a type nothing indexes was serving a label noengine could fill. The merged model cannot express that – a label needs a
collection, so it needs a root target. Such fixtures became bare IRIs, which
the SDL snapshot shows.
missing-ref-type-nameis narrowed toinlinealone: a lookup derives itsname from
target, and anidOnlyemits no type to name.branches that went.
Rebased through #741, so it carries #728, #719 and #740.
Fix #731