Conversation
thehabes
added this pull request to stack #315
September 18, 2026 16:18
…of unexpected 500s
thehabes
marked this pull request as ready for review
September 18, 2026 19:14
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.
Summary
Resolves #309.
Resolves #313.
Part of #306. #308 is the only sub-issue still open.
Stacked on #312. Merge that first.
/searchand/search/phrasenow run one Atlas Search pipeline against one index, and paging happens in the database. They carryrel="next"the same way/querydoes after #312./queryand every search endpoint stop serving objects whose_idis not a string. AsearchTextthat isn't a non-empty string, oroptionsthat aren't a JSON object, are refused with a400before they reach Atlas.What changed
presi3AnnotationTextandpresi2AnnotationText, in twoaggregate()calls run throughPromise.allannotationText, which covers both vocabularies, in one callmergeSearchResults()keyed on_id, collapsing every embedded-object_idonto"[object Object]"limit + skiprecords, then Node merged and sliced them$skipand$limitin the pipeline, plus one extra record to learn whether another page existsrel="next"_id/query./searchserved theObjectIdones, and by accident kept only one embedded-object_idper responselimit.catch(() => [])into an empty half of the resultsnext(err)searchTextoroptions200 []400before the database is called. See Input validationoptionsqueryandpath, so they could replace the search text or the fields searchedLatency
{"searchText":"a"}atlimit=100on the dev cluster, median of three runs, measured 2026-09-18. The old two-index fan-out was replayed next to the new pipeline in the same script:skipahad 1218 servable matches on dev when this was measured, so the new pipeline's pages are empty fromskip=5000on. The old code still returned records atskip=5000because it served the legacy objects. Atlas still walks matches up toskip, but Node no longer receives, holds, or sorts them, so an empty page past the end costs about as much as a page near the front.The pipeline
searchPipelineFor()is shared by every search operator:$search$matchon a string_id$addFieldsfor__rerum.score$skip$limit: limit + 1serveSearchPage()trims the extra record. It sets the JSON-LD headers before callingsetNextPageLink(), becauseconfigureLDHeadersFor()replacesLinkandsetNextPageLink()appends to it. The unmountedsearchFuzzily,searchWildlyandsearchAlikesuse the same two helpers, so they will page the same way if they are ever mounted.Two
shouldclauses carried over from the old IIIF 3.0 query could never match anything, so they are removed. One wasembeddedDocumentonitems.annotations.items, and the other wasembeddedDocumentonannotations. AnembeddedDocumentclause scoped to an outerembeddedDocumentspath can't see the fields of the one nested inside it, and it matches nothing without raising an error. The Manifest text the first clause was meant for is already reached through theitemsclause. With both removed, all 16 sampled responses on dev stayed byte-identical, scores included.Input validation
Without the
.catch(() => []), an Atlas rejection now reaches the client. Atlas reports a bad operator option asMongoServerErrorcode 8 (UnknownError), which can't be told apart from a server fault. So the shape of the request is checked before the pipeline runs:searchTextmust be a non-empty string.options, when sent, must be a JSON object.readSearchOptions()refuses a string, an array, a number or a boolean.nullcounts as not sent. The unmountedsearchFuzzilyandsearchWildlyuse the same check.Each of these is a
400, and neither reaches the database.Anything else Atlas rejects is a
500that carries Atlas's message. That is intended. It includes:{"slop":"x"}or{"fuzzy":{"maxEdits":9}}./searchsearchTextof more than 64 distinct words. The one query searches 16 paths, and 16 × 65 clauses exceed Atlas'smaxClauseCountof 1024. The old IIIF 3.0 query searched 12 paths and the IIIF 2.1 query 8, so up to 85 words were searched in full. From 86 to 128 words the IIIF 3.0 half failed silently. Above 128 the search answered200 []./search/phraseisn't affected./querycontrollers/crud.jswraps the client's filter as{ $and: [props, { _id: { $type: "string" } }] }. It doesn't merge into it, so a client's own_idcondition still applies. The winning plan is still anIXSCANon_id, with the samekeysExaminedwith and without the clause.Documentation
public/API.html:rel="next", with a paged-search example that follows it.$sorttiebreaker. See Out of scope.controllers/search.jswas corrected to match.Pagination-Limitis lower than the requestedlim.openapi/contracts/core-provider.openapi.yaml:/api/searchand/api/search/phrasedeclare theNextPageLinkheader. All three paths state their result ordering and the non-string_idexclusion. The exclusion is documented there, not inAPI.html.Tests
routes/__tests__/search.test.jsreplaces the two-index dedup and merge tests with tests of the one pipeline. They cover:annotationText, searching the IIIF 3.0 and IIIF 2.1 fields.$skipand$limit: limit + 1in the pipeline, and the string_id$matchplaced before$limit.400s, none of which reach the database.searchTextor the paths.rel="next"walks that serve every record once, including past theskipmaximum.routes/__tests__/query.test.jschecks that the$andwrapper keeps a client's own_idcondition.npm testpasses 274 of 274.Before deploying
annotationTextmust exist and bequeryableon the target cluster. A missing Atlas Search index doesn't raise an error. It matches nothing, so every search would answer200 []and leave nothing in the logs. Dev has the index. Production has not been confirmed._id. After this change,manuscriptreturns 0 records instead of 50.linegoes from 4007 matches to 272 servable ones. Read-only probes ofstore.rerum.ioforline,textandloremreturned no records with a legacy@id, so production looks unaffected.presi3AnnotationTextandpresi2AnnotationTextare no longer queried, and can be dropped once this is live.Out of scope
annotationTextdefinition still lives only in Atlas. When it gets checked in, its top-levelitems.annotations.itemsmapping can be dropped, because no query can reach it.limitorskipparameters and noPagination-*headers.$sorttiebreaker, because one would give up the latency win. On dev, followingrel="next"foraat page sizes 7, 100 and 500 returned the same 1218 records in the same order each time, with no repeats, across tie groups of up to 237 equal scores./queryso paging depth is unbounded and cost is flat #303: cursor-based paging stays deferred.