Merge pull request #396 from MITLibraries/use-602

Adds support for global scoring

Author: JPrevost

AGENTS.md

@@ -20,6 +20,7 @@ This file highlights the important, discoverable conventions and workflows an AI
   |------|----------|
   | `FEATURE_BOOLEAN_PICKER` | Allow users to choose AND/OR boolean logic in searches |
   | `FEATURE_GEODATA` | Enable geospatial search (bounding box and radius-based queries); defaults to false |
+  | `FEATURE_GLOBAL_SCORING` | Pass `useGlobalScoring: true` to all TIMDEX GraphQL search queries; defaults to false |
   | `FEATURE_OA_ALWAYS` | Always do OpenAlex lookups when DOI or PMID is detected rather than only when LibKey does not return data |
   | `FEATURE_RECORD_LINK` | Show "View full record" link in search results |
   | `FEATURE_SIMULATE_SEARCH_LATENCY` | Add 1s minimum delay to search results for testing UX behavior |

README.md

@@ -100,6 +100,7 @@ See `Optional Environment Variables` for more information.
 - `FEATURE_GEODATA`: Enables features related to geospatial data discovery. Setting this variable to `true` will trigger geodata
 mode. Note that this is currently intended _only_ for the geodata app and
 may have unexpected consequences if applied to other TIMDEX UI apps.
+- `FEATURE_GLOBAL_SCORING`: Pass `useGlobalScoring: true` to all TIMDEX GraphQL search queries; defaults to false
 - `FEATURE_OA_ALWAYS`: Enables OpenAccess links from OpenAlex whenever they are available, not just when LibKey does not return data. `OPENALEX_EMAIL` must also be set.
 - `FEATURE_RECORD_LINK`: Display the 'View full record' link below each record.
 - `FEATURE_SIMULATE_SEARCH_LATENCY`: DO NOT SET IN PRODUCTION. Set to ensure a minimum of a one second delay in returning search results. Useful to see spinners/loaders. Only introduces delay for results that take less than one second to complete.

app/controllers/search_controller.rb

@@ -191,6 +191,7 @@ def query_timdex(query)
     query[:sourceFilter] = ['DSpace@MIT'] if @active_tab == 'dspace'
     query[:sourceFilter] = ['Research Databases'] if @active_tab == 'databases'
     query[:sourceFilter] = ['OpenGeoMetadata GIS Resources', 'MIT GIS Resources'] if @active_tab == 'geodata'
+    query[:useGlobalScoring] = Feature.enabled?(:global_scoring)
 
     # We generate unique cache keys to avoid naming collisions.
     cache_key = CacheKeyGenerator.call(query)

app/models/feature.rb

@@ -33,8 +33,8 @@
 #
 class Feature
   # List of all valid features in the application
-  VALID_FEATURES = %i[bot_detection geodata boolean_picker oa_always primo_nde_links simulate_search_latency
-                      tab_primo_all tab_timdex_all tab_timdex_alma record_link timdex_fulltext
+  VALID_FEATURES = %i[bot_detection geodata boolean_picker global_scoring oa_always primo_nde_links
+                      simulate_search_latency tab_primo_all tab_timdex_all tab_timdex_alma record_link timdex_fulltext
                       timdex_semantic_search].freeze
 
   # Check if a feature is enabled by name

app/models/timdex_search.rb

@@ -27,6 +27,7 @@ class TimdexSearch < TimdexBase
       $placesFilter: [String!]
       $sourceFilter: [String!]
       $subjectsFilter: [String!]
+      $useGlobalScoring: Boolean
     ) {
       search(
         searchterm: $q
@@ -52,6 +53,7 @@ class TimdexSearch < TimdexBase
         placesFilter: $placesFilter
         sourceFilter: $sourceFilter
         subjectsFilter: $subjectsFilter
+        useGlobalScoring: $useGlobalScoring
       ) {
         hits
         records {
@@ -171,6 +173,7 @@ class TimdexSearch < TimdexBase
       $literaryFormFilter: String
       $sourceFilter: [String!]
       $subjectsFilter: [String!]
+      $useGlobalScoring: Boolean
     ) {
       search(
         searchterm: $q
@@ -184,6 +187,7 @@ class TimdexSearch < TimdexBase
         index: $index
         from: $from
         booleanType: $booleanType
+        useGlobalScoring: $useGlobalScoring
         geobox: {
           minLongitude: $geoboxMinLongitude,
           minLatitude: $geoboxMinLatitude,
@@ -299,6 +303,7 @@ class TimdexSearch < TimdexBase
       $literaryFormFilter: String
       $sourceFilter: [String!]
       $subjectsFilter: [String!]
+      $useGlobalScoring: Boolean
     ) {
       search(
         searchterm: $q
@@ -312,6 +317,7 @@ class TimdexSearch < TimdexBase
         index: $index
         from: $from
         booleanType: $booleanType
+        useGlobalScoring: $useGlobalScoring
         geodistance: {
           distance: $geodistanceDistance,
           latitude: $geodistanceLatitude,
@@ -430,6 +436,7 @@ class TimdexSearch < TimdexBase
       $literaryFormFilter: String
       $sourceFilter: [String!]
       $subjectsFilter: [String!]
+      $useGlobalScoring: Boolean
     ) {
       search(
         searchterm: $q
@@ -462,6 +469,7 @@ class TimdexSearch < TimdexBase
         literaryFormFilter: $literaryFormFilter
         sourceFilter: $sourceFilter
         subjectsFilter: $subjectsFilter
+        useGlobalScoring: $useGlobalScoring
       ) {
         hits
         records {

config/schema/schema.json

@@ -1362,13 +1362,23 @@
                 },
                 {
                   "name": "queryMode",
-                  "description": "Search mode to use. Defaults to \"lexical\". Options include: \"lexical\", \"semantic\"",
+                  "description": "Search mode: \"keyword\" (lexical search), \"semantic\" (vector search), or \"hybrid\" (both)",
                   "type": {
                     "kind": "SCALAR",
                     "name": "String",
                     "ofType": null
                   },
-                  "defaultValue": "\"lexical\""
+                  "defaultValue": "\"keyword\""
+                },
+                {
+                  "name": "useGlobalScoring",
+                  "description": "Calculate relevance scores globally across all shards instead of per-shard. Defaults to false.",
+                  "type": {
+                    "kind": "SCALAR",
+                    "name": "Boolean",
+                    "ofType": null
+                  },
+                  "defaultValue": "false"
                 },
                 {
                   "name": "accessToFilesFilter",
@@ -2863,6 +2873,12 @@
               "isDeprecated": false,
               "deprecationReason": null
             },
+            {
+              "name": "VARIABLE_DEFINITION",
+              "description": "Location adjacent to a variable definition.",
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
             {
               "name": "SCHEMA",
               "description": "Location adjacent to a schema definition.",
@@ -2928,12 +2944,6 @@
               "description": "Location adjacent to an input object field definition.",
               "isDeprecated": false,
               "deprecationReason": null
-            },
-            {
-              "name": "VARIABLE_DEFINITION",
-              "description": "Location adjacent to a variable definition.",
-              "isDeprecated": false,
-              "deprecationReason": null
             }
           ],
           "possibleTypes": null
@@ -3720,4 +3730,4 @@
       ]
     }
   }
-}
+}

test/controllers/search_controller_test.rb

@@ -63,6 +63,10 @@ def mock_primo_search_all_tab
   end
 
   def mock_primo_search_with_hits(total_hits)
+    # Use when the test needs to control the simulated Primo hit count, e.g., pagination
+    # boundary tests or UI behavior that reacts to total result numbers. Always returns
+    # 10 sample documents regardless of `total_hits`; only the reported total varies.
+    # For tests that don't care about hit counts, prefer `mock_primo_search_success`.
     sample_docs = (1..10).map do |i|
       {
         title: "Sample Primo Document Title #{i}",
@@ -86,11 +90,13 @@ def mock_primo_search_with_hits(total_hits)
   end
 
   def mock_timdex_search_success
-    # Mock the TIMDEX GraphQL client to avoid external API calls (single call)
+    # Use for standard single-tab TIMDEX tests that don't inspect the query variables
+    # passed to the GraphQL client. Sets `expects(:query)` without `.with(...)`, so
+    # Mocha only verifies the call count, not the arguments.
     #
-    # NOTE: As with Primo, this helper assumes a single TIMDEX invocation.
-    # Tests exercising the 'all' tab should use `mock_timdex_search_all_tab`
-    # which allows multiple calls.
+    # - For the 'all' tab (multiple TIMDEX calls): use `mock_timdex_search_all_tab`.
+    # - For tests asserting on query variables (e.g., feature flags): use
+    #   `build_timdex_mock_response` and set the expectation with `.with(...)` yourself.
     sample_result = {
       'api' => 'timdex',
       'title' => 'Sample TIMDEX Document Title',
@@ -134,11 +140,10 @@ def mock_timdex_search_success
   end
 
   def mock_timdex_search_all_tab
-    # Mock the TIMDEX GraphQL client for the all tab (multiple calls)
-    #
-    # This helper is intentionally separate from `mock_timdex_search_success`
-    # because the merged-search orchestration can invoke TIMDEX multiple
-    # times. The helper therefore uses `at_least_once` on the expectation.
+    # Use for tests exercising the 'all' tab, where `MergedSearchService` may invoke
+    # TIMDEX multiple times. Relaxes the expectation to `at_least_once` to accommodate
+    # that orchestration. For single-tab TIMDEX tests, prefer `mock_timdex_search_success`
+    # so unexpected extra calls cause a failure rather than passing silently.
     sample_result = {
       'api' => 'timdex',
       'title' => 'Sample TIMDEX Document Title',
@@ -182,6 +187,11 @@ def mock_timdex_search_all_tab
   end
 
   def mock_timdex_search_with_hits(total_hits)
+    # Use when the test needs to control the simulated TIMDEX hit count, e.g., pagination
+    # threshold or no-results-message tests. Always returns 10 sample documents regardless
+    # of `total_hits`; only the reported total varies. Unlike the other TIMDEX helpers,
+    # this one also mocks `NormalizeTimdexResults` explicitly. For tests that don't care
+    # about hit counts, prefer `mock_timdex_search_success`.
     sample_results = (1..10).map do |i|
       {
         'title' => "Sample TIMDEX Document Title #{i}",
@@ -225,6 +235,54 @@ def mock_timdex_search_with_hits(total_hits)
     NormalizeTimdexResults.expects(:new).returns(mock_normalizer).at_least_once
   end
 
+  def build_timdex_mock_response
+    # Use when a test needs to assert on the variables passed to `TimdexBase::Client.query`
+    # (e.g., verifying a feature flag sets the correct query variable). Returns a fully
+    # stubbed response object but intentionally does NOT set any expectation on
+    # `TimdexBase::Client`. The caller is responsible for setting the expectation:
+    #
+    #   mock_response = build_timdex_mock_response
+    #   TimdexBase::Client.expects(:query).with(TimdexSearch::BaseQuery,
+    #     has_entry(:variables, has_entry(:someVar, expected_value))).returns(mock_response)
+    #
+    # The other TIMDEX helpers (`mock_timdex_search_success`, etc.) set the expectation
+    # themselves without `.with(...)`, so they cannot be used when argument assertions
+    # are needed — Mocha would end up with two conflicting expectations on the same method.
+    sample_result = {
+      'api' => 'timdex',
+      'title' => 'Sample TIMDEX Document Title',
+      'timdexRecordId' => 'sample-record-123',
+      'contentType' => ['Article'],
+      'dates' => [{ 'kind' => 'Publication date', 'value' => '2023' }],
+      'contributors' => [{ 'value' => 'Foo Barston', 'kind' => 'Creator' }],
+      'sourceLink' => 'https://example.com/record'
+    }
+
+    mock_response = mock('timdex_response')
+    mock_errors = mock('timdex_errors')
+    mock_errors.stubs(:details).returns({})
+    mock_errors.stubs(:to_h).returns({})
+    mock_response.stubs(:errors).returns(mock_errors)
+
+    mock_data = mock('timdex_data')
+    mock_search = mock('timdex_search')
+    mock_search.stubs(:to_h).returns({
+                                       'hits' => 1,
+                                       'aggregations' => {},
+                                       'records' => [sample_result]
+                                     })
+    mock_data.stubs(:search).returns(mock_search)
+    mock_data.stubs(:to_h).returns({
+                                     'search' => {
+                                       'hits' => 1,
+                                       'aggregations' => {},
+                                       'records' => [sample_result]
+                                     }
+                                   })
+    mock_response.stubs(:data).returns(mock_data)
+    mock_response
+  end
+
   test 'index shows basic search form by default' do
     get '/'
     assert_response :success
@@ -1220,4 +1278,29 @@ def source_filter_count(controller)
     # Should not be redirected to Turnstile (doesn't hit SearchController)
     assert_response :success
   end
+
+  # FEATURE_GLOBAL_SCORING tests
+  test 'timdex query passes useGlobalScoring: false when FEATURE_GLOBAL_SCORING is disabled' do
+    mock_response = build_timdex_mock_response
+    TimdexBase::Client.expects(:query).with(
+      TimdexSearch::BaseQuery,
+      has_entry(:variables, has_entry(:useGlobalScoring, false))
+    ).returns(mock_response)
+
+    get '/results?q=data&tab=timdex'
+    assert_response :success
+  end
+
+  test 'timdex query passes useGlobalScoring: true when FEATURE_GLOBAL_SCORING is enabled' do
+    mock_response = build_timdex_mock_response
+    ClimateControl.modify(FEATURE_GLOBAL_SCORING: 'true') do
+      TimdexBase::Client.expects(:query).with(
+        TimdexSearch::BaseQuery,
+        has_entry(:variables, has_entry(:useGlobalScoring, true))
+      ).returns(mock_response)
+
+      get '/results?q=data&tab=timdex'
+      assert_response :success
+    end
+  end
 end

test/models/feature_test.rb

@@ -4,6 +4,7 @@ class FeatureTest < ActiveSupport::TestCase
   test 'defined features default to false' do
     refute Feature.enabled?(:geodata)
     refute Feature.enabled?(:boolean_picker)
+    refute Feature.enabled?(:global_scoring)
   end
 
   test 'undefined features return false' do