Conversation
❌ 10 blocking issues (10 total)
|
| @@ -225,6 +235,54 @@ def mock_timdex_search_with_hits(total_hits) | |||
| NormalizeTimdexResults.expects(:new).returns(mock_normalizer).at_least_once | |||
| } | ||
| }) | ||
| mock_response.stubs(:data).returns(mock_data) | ||
| mock_response |
Coverage Report for CI Build 26577952935Coverage increased (+0.001%) to 98.318%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
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.
There was a problem hiding this comment.
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_scoringas a valid Feature flag and documentFEATURE_GLOBAL_SCORINGin README and AGENTS.md. - Add
$useGlobalScoring: Booleanto the four TIMDEX GraphQL queries and forward the flag value fromSearchController#query_timdex. - Add controller tests (and a new
build_timdex_mock_responsehelper) 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.
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.
Why are these changes being introduced:
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:
controller to use it when enabled.
Document any side effects to this change:
problems we may need to accept that some results are scored
inconsistently across shards.
Developer
Accessibility
New ENV
Approval beyond code review
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
added technical debt.
Documentation
(not just this pull request message).
Testing