Skip to content

fix(os-search): honor track_total_hits and _score sort on the OpenSearch read path (#36501)#36557

Merged
fabrizzio-dotCMS merged 1 commit into
mainfrom
issue-36501-os-search-readpath
Jul 20, 2026
Merged

fix(os-search): honor track_total_hits and _score sort on the OpenSearch read path (#36501)#36557
fabrizzio-dotCMS merged 1 commit into
mainfrom
issue-36501-os-search-readpath

Conversation

@fabrizzio-dotCMS

@fabrizzio-dotCMS fabrizzio-dotCMS commented Jul 13, 2026 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Member

Proposed Changes

Two OpenSearch read-path gaps found running the integration suite under ES→OS migration phase 2 (reads from OpenSearch). Both have a working ES reference implementation. Contributes to #36501.

  • track_total_hits — OSIndexProperty.TRACK_TOTAL_HITS had no ES fallback key, so an existing ES_TRACK_TOTAL_HITS config was ignored under OS reads and OS always returned the full accurate count. Added the ES_TRACK_TOTAL_HITS fallback (same dual-key pattern as USE_FILTERS_FOR_SEARCHING).
  • _score sort — ContentFactoryIndexOperationsOS.addSorting, for sortBy=score, added only the secondary field sort and never a primary _score sort, so results ordered by moddate and hit[0] was not the highest-scoring doc. Added the _score DESC primary sort (mirrors ContentFactoryIndexOperationsES.addSorting; API matches OSContentletScrollImpl).

Verification

Phase 2, isolated env: ESContentFactoryImplTest 42/0/0 (incl. Test_TrackHits_SearchCount and testScore, previously failing). Core compiles.

Refs #36501, #36320.

This PR fixes: #36501

@fabrizzio-dotCMS @claude
fix(os-search): honor track_total_hits and _score sort on the OpenSea…
9fbddcc 
…rch read path (#36501)

Two OS read-path gaps surfaced running the integration suite under migration
phase 2 (reads from OpenSearch); both have a working ES reference implementation.

- track_total_hits: OSIndexProperty.TRACK_TOTAL_HITS had no ES fallback key, so an
  existing ES_TRACK_TOTAL_HITS configuration was ignored under OS reads and OS always
  reported the full accurate count. Add the "ES_TRACK_TOTAL_HITS" fallback, matching
  the dual-key pattern used by USE_FILTERS_FOR_SEARCHING. (Fixes ESContentFactoryImplTest
  .Test_TrackHits_SearchCount under phase 2 — a gap tracked in #36320.)

- _score sort: ContentFactoryIndexOperationsOS.addSorting, for sortBy=score, added only
  the secondary field sort and never a primary _score sort, so results were ordered by
  moddate and hit[0] was not the highest-scoring document. Add the _score DESC primary
  sort before the secondary field, mirroring ContentFactoryIndexOperationsES.addSorting.
  (Fixes ESContentFactoryImplTest.testScore under phase 2.)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot mentioned this pull request Jul 13, 2026
Open
3 tasks
@github-actions github-actions Bot added the Area : Backend PR changes Java/Maven backend code label Jul 13, 2026
@claude

claude Bot commented Jul 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Claude finished @fabrizzio-dotCMS's task in 2m 20s —— View job

Rollback-Safety Analysis — PR #36557

  • Read docs/core/ROLLBACK_UNSAFE_CATEGORIES.md
  • Get full PR diff (ContentFactoryIndexOperationsOS.java, OSIndexProperty.java)
  • Evaluate diff against every unsafe category
  • Post verdict / apply label

Verdict: ✅ Safe to rollback — label AI: Safe To Rollback applied.

Why: Both changes are pure query-construction logic evaluated at search time, not storage/schema/mapping changes:

  • OSIndexProperty.java — adds an ES_TRACK_TOTAL_HITS fallback key to the existing TRACK_TOTAL_HITS enum entry (OS_TRACK_TOTAL_HITS, null → OS_TRACK_TOTAL_HITS, "ES_TRACK_TOTAL_HITS"). This only changes which config key is consulted when building a search request; no data, index mapping, or DB schema is touched. N-1 simply ignores the new fallback and keeps its old behavior.
  • ContentFactoryIndexOperationsOS.java — adds a _score DESC primary sort() clause to the OpenSearch SearchRequestBuilder for sortBy=score queries. This affects only the ordering of search results returned to the caller for that one request; it doesn't alter the ES/OS index mapping, stored documents, DB rows, or any REST/GraphQL response shape.

No category in the reference doc applies: no runonce migration task, no putMapping()/mapping change, no contentlet_as_json version bump, no DROP/RENAME, no PK change, no new field type, no storage-provider change, no VTL viewtool contract change, and no REST/GraphQL/OSGi public contract change — this is internal, request-scoped search behavior. Rolling back to N-1 restores the prior (already-existing, previously-working per the PR's own description) behavior with no data or index incompatibility.

@mergify

mergify Bot commented Jul 20, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@fabrizzio-dotCMS
fabrizzio-dotCMS added this pull request to the merge queue Jul 20, 2026
Merged via the queue into main with commit f34f527 Jul 20, 2026
63 checks passed
@fabrizzio-dotCMS
fabrizzio-dotCMS deleted the issue-36501-os-search-readpath branch July 20, 2026 22:02
@fabrizzio-dotCMS fabrizzio-dotCMS mentioned this pull request Jul 20, 2026
Merged
5 tasks
dudemin pushed a commit to dudemin/core that referenced this pull request Jul 24, 2026
@fabrizzio-dotCMS @claude
fix(opensearch): honor sort=score in OS non-scroll read path (dotCMS#…
aec4ca2 
…36494) (dotCMS#36643)

## Proposed Changes

Fixes dotCMS#36494.

During the ES→OpenSearch migration, content queries sorting by relevance
(`sort=score`) silently lost their ordering once reads switched to
OpenSearch (Phase 2+): they came back in default `moddate desc` order
instead of `_score` order. HTTP 200, plausible-looking hit set, wrong
order — the kind of regression count-based parity checks can't catch.
Root cause: `ContentFactoryIndexOperationsOS.addSorting`'s
`sortBy.startsWith("score")` branch added only the secondary sort and
never the primary `_score` sort.

> **Note (post-rebase):** the core `_score`-sort fix has since landed on
`main` via dotCMS#36501/dotCMS#36557 (`fix(os-search): honor track_total_hits and
_score sort on the OpenSearch read path`), which added the same
primary/secondary sort to this branch. After merging `main`, the **net
contribution of this PR is now**:
> 1. **`unmappedType(Date)` hardening** on the secondary sort, and
> 2. the **regression tests** for `sort=score` (which `main` does not
have).

### What this PR still changes

**Production — `ContentFactoryIndexOperationsOS.addSorting`:** add
`unmappedType(Date)` to the secondary field sort in the `score` branch,
matching the scroll path (`OSContentletScrollImpl`). Without it,
`sort=score <field>` fails on indices where the secondary field is not
mapped, instead of degrading gracefully. Behavior for all other sort
inputs is unchanged.

### Tests

Added to `OpenSearchUpgradeSuite`
(`ContentFactoryIndexOperationsOSIntegrationTest`) — new coverage not
present on `main`:
- `test_searchHits_sortByScore_shouldOrderByRelevanceNotModdate` —
indexes docs so relevance order is the exact reverse of moddate order;
asserts scores come back descending and the most-relevant doc is first
(guards against the original regression).
-
`test_searchHits_sortByScore_secondaryOverride_shouldApplySecondarySort`
— equal-score docs, `sort=score moddate asc`; asserts the secondary sort
breaks the tie.

### Acceptance Criteria

- [x] `addSorting` adds the primary `_score` (desc) sort in the `score`
branch, secondary sort after it (now on `main` via dotCMS#36501/dotCMS#36557;
preserved through the merge).
- [x] Secondary sort uses `unmappedType(Date)` so `sort=score <field>`
does not fail on indices missing that field.
- [x] `sort=score` returns relevance ordering in Phase 2/3 (suite
coverage).
- [x] `sort=score <field> <dir>` secondary-sort override behaves
correctly (suite coverage).
- [x] Scroll path (`OSContentletScrollImpl`) unchanged (already
correct).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ihoffmann-dot ihoffmann-dot ihoffmann-dot approved these changes

Assignees

No one assigned

Labels

AI: Safe To Rollback Area : Backend PR changes Java/Maven backend code OpenSearch

Projects

dotCMS - Product Planning
Status: No status

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

bug(index): product fixes & findings from phased (ES→OS) integration runs

2 participants

@fabrizzio-dotCMS @ihoffmann-dot