CenterForDigitalHumanities / CenterForDigitalHumanities/rerum_server_nodejs
`/search` merge sorts on a field that does not exist and drops documents whose `_id` is an embedded object
- Dominant language
- JavaScript
- Stars
- 3
- Forks
- 6
- Avg merge
- 1h 25m
- Merged PRs (30d)
- 3
Description
## Summary
`mergeSearchResults()` (`controllers/search.js:32`) combines the results of the two Atlas Search branches. It has two independent one-line bugs:
```js
const id = result._id?.$oid || result._id?.toString() // :37 — the drop bug
...
return merged.sort((a, b) => (b.score || 0) - (a.score || 0)) // :45 — the ranking bug
```
- **Ranking never happens.** The pipelines write the score to `__rerum.score` (`controllers/search.js:210,215`). No document has a top-level `score`, so every comparison is `0 - 0`, the sort is a no-op, and results come back as "every IIIF 3.0 hit, then every IIIF 2.1 hit" regardless of relevance. The JSDoc on six handlers promises results "sorted by relevance score (highest first)".
- **Matching documents are discarded.** `({}).toString()` is `"[object Object]"`. Every legacy v0 document whose `_id` is an embedded object keys to that same string, the first is kept, and the rest are silently dropped before paging begins.
Both fixes are one line each. They belong in the minimum set for honest `/search` paging, because **no amount of pagination work makes a `/search` walk complete while the endpoint drops matches on every page.** They are deliberately separated from the Atlas restructuring in #309 so they are not held hostage to it.
## Why this matters
### Documents are unreachable at any `skip`
Measured against the two branch pipelines on the dev collection at full depth:
| Term | `presi3` raw | `presi2` raw | Cross-index overlap | Embedded-object `_id` | Merged | **Dropped** |
|------|--------------|--------------|---------------------|-----------------------|--------|-------------|
| `line` | 205 | 3802 | 0 | 183 | 3825 | **182** |
| `text` | 2301 | 46 | 0 | 4 | 2344 | **3** |
| `manuscript` | 0 | 68 | 0 | 19 | 50 | **18** |
| `page` | 63 | 3 | 0 | 0 | 66 | 0 |
Confirmed end to end through the endpoint, not only against the pipelines. Walking `/search` for `line` at `limit=500` until it returns an empty page reaches exactly **3825** records against 4007 raw matches. The 182 missing documents are not on a later page; they are on no page.
**Pages under-fill before results are exhausted**, which is the same trap as clamped `limit`:
```text
POST /v1/api/search?limit=100&skip=0 {"searchText":"manuscript"}
-> 50 documents (100 requested, 68 actually match)
```
A client that stops on a short page reports 50 of 68 and believes it is done.
Note that cross-index overlap is **zero** for every term tried. A document is written in either IIIF 3.0 or IIIF 2.1 shape and the two indexes cover disjoint field paths. The deduplication is not doing the job it was written for; it is doing a different, harmful one.
### The best match is buried
The results of a single `/search?limit=500` request, read back via `.__rerum.score`. A correctly sorted list never increases:
| Deployment | Term | Returned | Score increases at | Values at the increase | Best score | Rank of best |
|------------|------|----------|--------------------|------------------------|------------|--------------|
| prod | `line` | 500 | index 5 | 1.690 → 83.928 | 83.928 | 6th |
| prod | `text` | 24 | index 2 | 2.452 → 6.958 | 6.958 | 3rd |
| prod | `page` | 298 | index 5 | 1.843 → 3.971 | 3.971 | 6th |
| dev, devstore | `line` | 500 | index 205 | 1.958 → 12.013 | 12.013 | 206th |
In every case there is exactly one increase and it sits exactly at the branch boundary. On production, the single best match for `line` — scoring 84 — ranks behind five documents scoring under 2. On dev the best match for `line` ranks 206th, so a client showing "top 10 results" never sees it and a client paging at 100 finds it on page 3. `/search/phrase` shows the identical boundary.
Any deployment where one vocabulary dominates the corpus buries the other vocabulary's matches, however well they score.
## Evidence
Verified 2026-09-02, re-run twice 2026-09-03, read-only, against `localhost:3001` (same collection as `devstore.rerum.io`), `devstore.rerum.io`, and `store.rerum.io`, and directly against the `presi3AnnotationText` / `presi2AnnotationText` indexes to isolate the merge step. Full detail in [the detailed report on #306](https://github.com/CenterForDigitalHumanities/rerum_server_nodejs/issues/306#issuecomment-5529634333).
Confirmed at the source: every returned document has `__rerum.score` populated and top-level `score` `undefined`, so the comparator is `0` for every pair.
Confirmed by grouping raw branch output by the merge's own key: for `manuscript`, 68 raw results yield 50 distinct keys, and the one colliding key — `"[object Object]"` — has 19 members, each a different document with a different `_id`, all `@type: oa:Annotation` from the v0 import.
The dev collection holds **6427** documents whose `_id` is an embedded object, the serialized Java `ObjectId` shape left by the v0 import from `165.134.105.29/annotationstore`. Production currently has none (`{"_id.inc":{"$exists":true}}` returns nothing on `store.rerum.io`), so the drop is not observable there today. It is a defect in the code path regardless: any `_id` that is neither an `ObjectId` nor a string is lost the same way, and dev is where clients are developed against.
## Affected lines
| File | Line | Current |
|------|------|---------|
| `controllers/search.js` | 37 | Dedup key collapses embedded-object `_id` to `"[object Object]"` — **the drop bug** |
| `controllers/search.js` | 45 | Sorts on top-level `score`, which is never set — **the ranking bug** |
| `controllers/search.js` | 25, 230, 238, 300, 386, 464, 557 | JSDoc promising results "sorted by relevance score" |
| `controllers/search.js` | 638, 656 | `searchAlikes()` hand-rolls the same shape |
## Proposed change
Two one-line changes in `mergeSearchResults()`:
- **Sort on `__rerum.score`**, the field the pipelines actually write. This makes cross-index ordering real without touching the Atlas index layout.
- **Derive the dedup key from something unique for every `_id` shape.** `JSON.stringify(result._id)` is the smallest correct option; the document's `@id` / `id` is an alternative. `toString()` is wrong for any non-primitive.
Then reconcile the JSDoc. After the sort fix the "sorted by relevance score" promise becomes true, so it stays — but it should say the ordering is a merge of two independently scored branches, since scores from two different indexes are not strictly comparable even once they are sorted together. That caveat goes away under #309 and not before.
## Notes
- **Breaking.** Result order changes for every `/search` client, and membership changes on dev as the dropped documents reappear. The diffs are trivial; the behavior change is not. Land on dev first.
- Cross-branch score comparison is an improvement over no ordering at all, but it is not the same as a single-index score. This is a real fix for a real bug, not a substitute for #309.
- In the minimum set for honest `/search` paging, alongside #302. Independent of both — no dependencies, can start immediately.
- Apply to the unmounted `searchFuzzily`, `searchWildly`, and `searchAlikes` as well, so the bugs are not carried forward when those get routes.
- Worth capturing before-and-after result ordering on a handful of representative queries, so the shift is understood rather than discovered by a client.
## Acceptance criteria
- [ ] `/search` results are ordered by descending relevance score across both vocabularies, verified by asserting the returned score sequence never increases
- [ ] Every document matched by the search is reachable by paging: a full walk of `line` on the dev collection reaches 4007 records, not 3825
- [ ] `manuscript` at `limit=100` returns 68 documents, not 50
- [ ] A regression test feeds `mergeSearchResults()` documents with `ObjectId`, string, and embedded-object `_id` values and asserts none are dropped
- [ ] The JSDoc promise of relevance ordering is true, and states the cross-branch caveat
- [ ] `/search` and `/search/phrase` both covered, and the unmounted search variants updated to match
- [ ] Sequential paged results still equal a single larger request, in the same order
- [ ] Before-and-after ordering on representative queries is posted on the issue before cutover
Contributor guide
Assessment
This issue has not been assessed yet.