Implement natural language search opt-in by jazairi · Pull Request #395 · MITLibraries/timdex-ui

jazairi · 2026-05-21T17:28:28Z

Why these changes are being introduced:

We want users to be able to decide whether to use
hybrid (termed 'natural language search' in the
public UI) or lexical search. USE-577 implemented
the frontend work on this feature; this ticket
implements the controller layer.

Relevant ticket(s):

How this addresses that need:

This adds natural language search opt-in cookie,
which is toggled via a stimulus controller. When
the cookie is set, hybrid search is the default
query mode. When it is not set, the query mode
falls back to lexical search (or other default
mode specified by env).

This behavior can be overridden by supplying a
queryMode param manually. For example, if a user has the NLS cookie set but adds queryMode=semantic to their query string, the search will execute
in semantic mode, not hybrid.

If NLS is not available for a given tab, a
message displays to notify the user of the
limitation.

Side effects of this change:

Tests for this feature are stored in a separate
file. This is different from how we usually do
things, but there are so many conditions to test
that I did not want to pollute the search
controller test file further.

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.

coveralls · 2026-05-21T17:30:12Z

Coverage Report for CI Build 26300577853

Coverage decreased (-0.05%) to 98.322%

Details

Coverage decreased (-0.05%) from the base build.
Patch coverage: 1 uncovered change across 1 file (20 of 21 lines covered, 95.24%).
No coverage regressions found.

Uncovered Changes

File	Changed	Covered	%
app/controllers/static_controller.rb	14	13	92.86%

Coverage Regressions

No coverage regressions found.

Coverage Stats


Relevant Lines:	1430
Covered Lines:	1406
Line Coverage:	98.32%
Coverage Strength:	74.21 hits per line

💛 - Coveralls

qltysh · 2026-05-21T17:30:15Z

❌ 8 blocking issues (10 total)

Tool	Category	Rule	Count
rubocop	Lint	Assignment Branch Condition size for `results` is too high. [<5, 28, 8> 29.55/17]	2	❌
rubocop	Lint	Cyclomatic complexity for `results` is too high. [8/7]	2	❌
rubocop	Lint	Method has too many lines. [21/10]	2	❌
rubocop	Lint	Class has too many lines. [308/100]	1	❌
rubocop	Lint	Perceived complexity for `natural\_language\_search\_optin` is too high. [11/8]	1	❌
qlty	Structure	Function with high complexity (count = 5): results	2

Copilot

Pull request overview

Adds a controller-layer implementation for a “natural language search” opt-in/out preference using a cookie, wiring it to the existing UI toggle and conditionally showing an alert when NLS isn’t applicable to the active tab.

Changes:

Add /natural_language_search_optin endpoint to set/clear the opt-in cookie and redirect back.
Apply the cookie in SearchController#results to default queryMode=hybrid and expose view flags for toggle state + warning display.
Add a Stimulus controller to toggle the preference from the UI and introduce a dedicated integration test file for the behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
test/controllers/natural_language_search_optin_test.rb	Adds integration tests covering cookie setting, redirects, toggle UI state, and warning display.
config/routes.rb	Adds route for the opt-in toggle endpoint.
app/views/search/results.html.erb	Renders the NLS alert only when `@show_nls_warning` is true.
app/views/search/_form.html.erb	Makes the toggle reflect opt-in state and wires click behavior to a Stimulus controller.
app/javascript/controllers/natural_language_search_toggle_controller.js	Implements client-side toggle behavior by hitting the opt-in endpoint and returning to the current page.
app/controllers/static_controller.rb	Adds `natural_language_search_optin` action to set/delete cookie and redirect.
app/controllers/search_controller.rb	Applies cookie-driven default query mode and sets view vars for toggle state + warning logic.

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

+        safe_return_to = nil
+      end
+    end
+    redirect_to(safe_return_to || root_path)


qltysh · 2026-05-21T19:45:41Z

+        safe_return_to = nil
+      end
+    end
+    redirect_to(safe_return_to || root_path)


Found 5 issues:

1. Function with high complexity (count = 9): natural_language_search_optin [qlty:function-complexity]

2. Assignment Branch Condition size for natural_language_search_optin is too high. [<7, 21, 10> 24.29/17] [rubocop:Metrics/AbcSize]

3. Cyclomatic complexity for natural_language_search_optin is too high. [10/7] [rubocop:Metrics/CyclomaticComplexity]

4. Method has too many lines. [19/10] [rubocop:Metrics/MethodLength]

5. Perceived complexity for natural_language_search_optin is too high. [11/8] [rubocop:Metrics/PerceivedComplexity]

Why these changes are being introduced: We want users to be able to decide whether to use hybrid (termed 'natural language search' in the public UI) or lexical search. USE-577 implemented the frontend work on this feature; this ticket implements the controller layer. Relevant ticket(s): - [USE-576](https://mitlibraries.atlassian.net/browse/USE-576) - [USE-577](https://mitlibraries.atlassian.net/browse/USE-577) How this addresses that need: This adds natural language search opt-in cookie, which is toggled via a stimulus controller. When the cookie is set, hybrid search is the default query mode. When it is not set, the query mode falls back to lexical search (or other default mode specified by env). This behavior can be overridden by supplying a `queryMode` param manually. For example, if a user has the NLS cookie set but adds `queryMode=semantic` to their query string, the search will execute in semantic mode, not hybrid. If NLS is not available for a given tab, a message displays to notify the user of the limitation. Side effects of this change: Tests for this feature are stored in a separate file. This is different from how we usually do things, but there are so many conditions to test that I did not want to pollute the search controller test file further.

JPrevost

The only required change is around a line accidentally reintroduced during a rebase.

The other comments are things you can tweak or not based on how they land with you.

Solid feature, I'm excited for others to get to try this soon!

JPrevost · 2026-05-21T20:27:59Z

    # inject session preference for boolean type if it is present
    params[:booleanType] = cookies[:boolean_type] || 'AND'

+    # inject queryMode from cookie if not provided in URL params


Minor nit: this comment feels slightly misleading. We aren't injecting the queryMode from the cookie, we are injecting hybrid if the cookie evaluates to true but no url parameter is passed. It's minor difference but it made me have to read this several times to convince myself what it was doing was how I was interpreting it lol

JPrevost · 2026-05-21T20:29:58Z

  end
+
+  # show_nls_warning? determines if the user should see a warning that results are not using natural language search.
+  # Shows when: user is opted-in AND viewing a Primo-only tab (which only supports keyword search).


Not asking for a change now as I'm pretty sure this is what was asked for, but I'm curious if we should have a partial warning on the all tab like "Some results are not using natural language search" with a link to why. Let's leave this as-is for now though and we can bring this up to the project team.

I agree, fwiw!

JPrevost · 2026-05-21T20:37:57Z

 class ApplicationController < ActionController::Base
  helper Mitlibraries::Theme::Engine.helpers

+  before_action :set_natural_language_search_optin


There really a few aspects of this that I think are fine but we definitely need to confirm are innocuous in geo mode.

I think GeoData is largely unaffected. It will set the cookie, but since GeoData doesn't use queryMode, it won't actually change the behavior. I also confirmed that nothing changes in the UI.

JPrevost · 2026-05-21T20:40:30Z


        <h2 class="results-context" data-matomo-seen="Search, Results Found, Tab: {{getActiveTabName}}"><%= results_summary(@pagination[:hits]) %></h2>
-        <%= render partial: 'search/nls_alert' %>
+        <p class="results-context-description"><%= tab_description %></p>


Change requested:

I think this line got accidentally rebased into place and can be removed. It moved in a commit that just landed earlier today. This branch now shows the tab description twice, once in the new location near the tab as well as in this old location closer to the results.

Oops, good catch! Yes, that rebase was done hastily before review tagging.

JPrevost · 2026-05-21T20:44:42Z

+    assert_redirected_to '/results?q=test'
+  end
+
+  test 'route redirects to root when no referer or return_to' do


Maybe a test for and illegal redirect to an external link goes to root?

JPrevost · 2026-05-21T20:48:26Z

+    assert_select 'aside.nls-alert', count: 0
+  end
+
+  test 'warning does not show on articles tab with no opt-in cookie' do


Minor: these tests sometimes mention an articles tab and sometimes cdi. Consistency might make this easier to follow. I'd go with cdi if you do adjust as that should remain stable even if our labels change

qltysh · 2026-05-22T16:50:54Z

+  # Shows when: user is opted-in AND viewing a Primo-only tab (which only supports keyword search).
+  def show_nls_warning?
+    @natural_language_search_optin && primo_tabs.include?(@active_tab)
+  end


Class has too many lines. [308/100] [rubocop:Metrics/ClassLength]

JPrevost

JPrevost · 2026-05-22T17:16:52Z

    assert_redirected_to root_path
  end

+  test 'external URLs redirect to route' do


Typo: I think you meant root here. Feel free to fixup without a re-review :)

github-advanced-security AI found potential problems May 21, 2026

View reviewed changes

Comment thread app/controllers/static_controller.rb Fixed

qltysh Bot reviewed May 21, 2026

View reviewed changes

mitlib temporarily deployed to timdex-ui-pi-use-576-zhci0u8bj May 21, 2026 17:31 Inactive

jazairi requested a review from Copilot May 21, 2026 17:40

Copilot started reviewing on behalf of jazairi May 21, 2026 17:41 View session

Copilot AI reviewed May 21, 2026

View reviewed changes

Comment thread app/controllers/static_controller.rb Outdated

Comment thread app/controllers/static_controller.rb Outdated

Comment thread app/views/search/_form.html.erb

Comment thread app/views/search/_form.html.erb Outdated

Comment thread app/controllers/search_controller.rb

jazairi force-pushed the use-576 branch from 0c016dc to 31f0847 Compare May 21, 2026 19:28

jazairi temporarily deployed to timdex-ui-pi-use-576-zhci0u8bj May 21, 2026 19:29 Inactive

github-advanced-security AI found potential problems May 21, 2026

View reviewed changes

Comment thread app/controllers/static_controller.rb Fixed

qltysh Bot reviewed May 21, 2026

View reviewed changes

Comment thread app/controllers/search_controller.rb

Comment thread app/controllers/static_controller.rb Outdated

Comment thread app/controllers/static_controller.rb Outdated

jazairi temporarily deployed to timdex-ui-pi-use-576-zhci0u8bj May 21, 2026 19:36 Inactive

github-advanced-security AI found potential problems May 21, 2026

View reviewed changes

Comment thread app/controllers/static_controller.rb

safe_return_to = nil

end

end

redirect_to(safe_return_to || root_path)

qltysh Bot reviewed May 21, 2026

View reviewed changes

Comment thread app/controllers/static_controller.rb Outdated

Comment thread app/controllers/static_controller.rb

Comment thread app/controllers/static_controller.rb Outdated

jazairi temporarily deployed to timdex-ui-pi-use-576-zhci0u8bj May 21, 2026 19:42 Inactive

qltysh Bot reviewed May 21, 2026

View reviewed changes

Comment thread app/controllers/static_controller.rb

jazairi temporarily deployed to timdex-ui-pi-use-576-zhci0u8bj May 21, 2026 19:44 Inactive

qltysh Bot reviewed May 21, 2026

View reviewed changes

jazairi force-pushed the use-576 branch from 50366b6 to 4127aef Compare May 21, 2026 19:51

jazairi temporarily deployed to timdex-ui-pi-use-576-zhci0u8bj May 21, 2026 19:51 Inactive

jazairi force-pushed the use-576 branch from 4127aef to 4a46861 Compare May 21, 2026 19:53

jazairi temporarily deployed to timdex-ui-pi-use-576-zhci0u8bj May 21, 2026 19:53 Inactive

jazairi requested review from JPrevost and matt-bernhardt May 21, 2026 19:59

JPrevost requested changes May 21, 2026

View reviewed changes

Incorporate code review feedback

00efd3d

jazairi temporarily deployed to timdex-ui-pi-use-576-zhci0u8bj May 22, 2026 16:49 Inactive

qltysh Bot reviewed May 22, 2026

View reviewed changes

jazairi requested a review from JPrevost May 22, 2026 16:51

JPrevost approved these changes May 22, 2026

View reviewed changes

jazairi merged commit ea5316f into main May 22, 2026
6 checks passed

Conversation

jazairi commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why these changes are being introduced:

Relevant ticket(s):

How this addresses that need:

Side effects of this change:

Developer

Accessibility

New ENV

Approval beyond code review

Additional context needed to review

Code Reviewer

Code

Documentation

Testing

Uh oh!

Uh oh!

coveralls commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report for CI Build 26300577853

Coverage decreased (-0.05%) to 98.322%

Details

Uncovered Changes

Coverage Regressions

Coverage Stats

💛 - Coveralls

Uh oh!

qltysh Bot commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qltysh Bot May 21, 2026

Choose a reason for hiding this comment

Uh oh!

JPrevost left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

jazairi commented May 21, 2026 •

edited

Loading

coveralls commented May 21, 2026 •

edited

Loading

qltysh Bot commented May 21, 2026 •

edited

Loading