Skip to content

Adds support for global scoring#396

Open
JPrevost wants to merge 3 commits into
mainfrom
use-602
Open

Adds support for global scoring#396
JPrevost wants to merge 3 commits into
mainfrom
use-602

Conversation

@JPrevost
Copy link
Copy Markdown
Member

Why are these changes being introduced:

  • As we moved from an OpenSearch instance where we control the shard
    count to OpenSearch Serverless where we do not, we need a way to more
    consistently score results across shards. This change adds support for
    global scoring (dfs_query_then_fetch under the hood) to allow us to
    opt-in to that behavior when we need it.

Relevant ticket(s):

How does this address that need:

  • This adds a new feature flag for global scoring and updates the search
    controller to use it when enabled.

Document any side effects to this change:

  • This mode is more expensive to run, so if we notice performance
    problems we may need to accept that some results are scored
    inconsistently across shards.

Developer

Accessibility
  • ANDI or WAVE has been run in accordance to our guide.
  • This PR contains no changes to the view layer.
  • New issues flagged by ANDI or WAVE have been resolved.
  • New issues flagged by ANDI or WAVE have been ticketed (link in the Pull Request details above).
  • No new accessibility issues have been flagged.
New ENV
  • All new ENV is documented in README.
  • All new ENV has been added to Heroku Pipeline, Staging and Prod.
  • ENV has not changed.
Approval beyond code review
  • UXWS/stakeholder approval has been confirmed.
  • UXWS/stakeholder review will be completed retroactively.
  • UXWS/stakeholder review is not needed.
Additional context needed to review

E.g., if the PR includes updated dependencies and/or data
migration, or how to confirm the feature is working.

Code Reviewer

Code
  • I have confirmed that the code works as intended.
  • Any CodeClimate issues have been fixed or confirmed as
    added technical debt.
Documentation
  • The commit message is clear and follows our guidelines
    (not just this pull request message).
  • The documentation has been updated or is unnecessary.
  • New dependencies are appropriate or there were no changes.
Testing
  • There are appropriate tests covering any new functionality.
  • No additional test coverage is required.

@qltysh
Copy link
Copy Markdown

qltysh Bot commented May 28, 2026

❌ 10 blocking issues (10 total)

Tool Category Rule Count
rubocop Lint Assignment Branch Condition size for query\_timdex is too high. [<9, 20, 15> 26.57/17] 5
rubocop Lint Class has too many lines. [309/100] 2
rubocop Lint Cyclomatic complexity for query\_timdex is too high. [9/7] 1
rubocop Lint Method has too many lines. [19/10] 1
rubocop Lint Perceived complexity for query\_timdex is too high. [10/8] 1

Comment thread app/models/feature.rb Outdated
@@ -225,6 +235,54 @@ def mock_timdex_search_with_hits(total_hits)
NormalizeTimdexResults.expects(:new).returns(mock_normalizer).at_least_once
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assignment Branch Condition size for mock_timdex_search_with_hits is too high. [<9, 31, 2> 32.34/17] [rubocop:Metrics/AbcSize]

}
})
mock_response.stubs(:data).returns(mock_data)
mock_response
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assignment Branch Condition size for build_timdex_mock_response is too high. [<5, 18, 0> 18.68/17] [rubocop:Metrics/AbcSize]

@coveralls
Copy link
Copy Markdown

coveralls commented May 28, 2026

Coverage Report for CI Build 26577952935

Coverage increased (+0.001%) to 98.318%

Details

  • Coverage increased (+0.001%) from the base build.
  • Patch coverage: 2 of 2 lines across 2 files are fully covered (100%).
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 1427
Covered Lines: 1403
Line Coverage: 98.32%
Coverage Strength: 75.13 hits per line

💛 - Coveralls

@mitlib mitlib temporarily deployed to timdex-ui-pi-use-602-gtjjmyoai May 28, 2026 13:33 Inactive
Why are these changes being introduced:

* As we moved from an OpenSearch instance where we control the shard
  count to OpenSearch Serverless where we do not, we need a way to more
  consistently score results across shards. This change adds support for
  global scoring (dfs_query_then_fetch under the hood) to allow us to
  opt-in to that behavior when we need it.

Relevant ticket(s):

* https://mitlibraries.atlassian.net/browse/USE-602

How does this address that need:

* This adds a new feature flag for global scoring and updates the search
  controller to use it when enabled.

Document any side effects to this change:

* This mode is more expensive to run, so if we notice performance
  problems we may need to accept that some results are scored
  inconsistently across shards.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a FEATURE_GLOBAL_SCORING feature flag that, when enabled, passes useGlobalScoring: true to all TIMDEX GraphQL search queries (BaseQuery, GeoboxQuery, GeodistanceQuery, and the all-query variant), enabling DFS query-then-fetch behavior for more consistent cross-shard scoring on OpenSearch Serverless.

Changes:

  • Register global_scoring as a valid Feature flag and document FEATURE_GLOBAL_SCORING in README and AGENTS.md.
  • Add $useGlobalScoring: Boolean to the four TIMDEX GraphQL queries and forward the flag value from SearchController#query_timdex.
  • Add controller tests (and a new build_timdex_mock_response helper) verifying the flag is propagated, and refresh VCR cassettes.

Reviewed changes

Copilot reviewed 26 out of 36 changed files in this pull request and generated no comments.

Show a summary per file
File Description
app/controllers/search_controller.rb Sets query[:useGlobalScoring] from the feature flag.
app/models/timdex_search.rb Adds useGlobalScoring variable to all four search queries.
app/models/feature.rb Adds global_scoring to VALID_FEATURES.
config/schema/schema.json Updated TIMDEX schema introspection adding useGlobalScoring argument and queryMode description.
test/controllers/search_controller_test.rb New helper + tests asserting the variable is passed for both flag states; added doc comments to existing helpers.
test/models/feature_test.rb Asserts new feature defaults to false.
test/vcr_cassettes/*.yml Re-recorded cassettes to include the new variable.
README.md, AGENTS.md Documents the new env var.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@JPrevost JPrevost requested a review from matt-bernhardt May 28, 2026 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants