Skip to content

Add filter support to semantic search#968

Open
jazairi wants to merge 1 commit into
mainfrom
use-593
Open

Add filter support to semantic search#968
jazairi wants to merge 1 commit into
mainfrom
use-593

Conversation

@jazairi

@jazairi jazairi commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Why these changes are being introduced:

Semantic searches ignore filters, meaning that
when semantic mode is enabled in the UI, the tabs
do not filter as intended.

Relevant ticket(s):

How this addresses that need:

SemanticQueryBuilder now applies FilterBuilder
filters to semantic queries, making it consistent
with lexical and hybrid search.

Side effects of this change:

We should consider adding regression test coverage in the TIMDEX UI repo.

Developer

  • All new ENV is documented in README
  • All new ENV has been added to Heroku Pipeline, Staging and Prod
  • ANDI or Wave has been run in accordance to
    our guide and
    all issues introduced by these changes have been resolved or opened as new
    issues (link to those issues in the Pull Request details above)
  • Stakeholder approval has been confirmed (or is not needed)

Code Reviewer

  • The commit message is clear and follows our guidelines
    (not just this pull request message)
  • There are appropriate tests covering any new functionality
  • The documentation has been updated or is unnecessary
  • The changes have been verified
  • New dependencies are appropriate or there were no changes

Requires database migrations?

NO

Includes new or updated dependencies?

NO

@qltysh

qltysh Bot commented Jun 12, 2026

Copy link
Copy Markdown

❌ 7 blocking issues (8 total)

Tool Category Rule Count
rubocop Lint Assignment Branch Condition size for combine\_queries is too high. [<9, 18, 16> 25.71/17] 2
rubocop Lint Method has too many lines. [30/10] 2
rubocop Lint Cyclomatic complexity for combine\_queries is too high. [14/7] 1
rubocop Lint Perceived complexity for combine\_queries is too high. [16/8] 1
rubocop Style Use except\(:filter\) instead. 1
qlty Structure Function with high complexity (count = 5): build 1

Comment thread test/models/semantic_query_builder_test.rb Outdated
@jazairi jazairi requested a review from Copilot June 12, 2026 20:29
@mitlib mitlib temporarily deployed to timdex-api-p-use-593-k6818zchr June 12, 2026 20:29 Inactive

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR updates semantic search query construction so UI tab/aggregation filters are applied in semantic mode, aligning behavior with lexical and hybrid search modes.

Changes:

  • Apply FilterBuilder-generated filters to the semantic query returned from the semantic-builder Lambda.
  • Update existing semantic query builder test expectations to include an explicit filter clause.
  • Add new tests covering source_filter, content_type_filter, and the “no filters” case for semantic queries.

Reviewed changes

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

File Description
app/models/semantic_query_builder.rb Adds FilterBuilder filters into the returned semantic bool query.
test/models/semantic_query_builder_test.rb Extends assertions to verify filters are included in semantic queries.

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

Comment thread app/models/semantic_query_builder.rb
Comment thread app/models/semantic_query_builder.rb
@jazairi jazairi temporarily deployed to timdex-api-p-use-593-k6818zchr June 12, 2026 20:37 Inactive
@jazairi jazairi requested a review from Copilot June 12, 2026 20:37
# to avoid redundant filter clauses
semantic_without_filters = if semantic_query.is_a?(Hash) && semantic_query[:bool]
{
bool: semantic_query[:bool].reject { |k, _| k == :filter }

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Use except(:filter) instead. [rubocop:Style/HashExcept]

Comment thread app/models/semantic_query_builder.rb
Comment thread app/models/semantic_query_builder.rb

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comment thread app/models/semantic_query_builder.rb
Comment thread test/models/semantic_query_builder_test.rb Outdated
@jazairi jazairi temporarily deployed to timdex-api-p-use-593-k6818zchr June 12, 2026 20:50 Inactive
@jazairi jazairi requested a review from Copilot June 12, 2026 20:51
@jazairi jazairi temporarily deployed to timdex-api-p-use-593-k6818zchr June 12, 2026 20:51 Inactive
filters = FilterBuilder.new.build(params)
semantic_query[:bool][:filter] = filters

semantic_query

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Found 3 issues:

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


2. Assignment Branch Condition size for build is too high. [<6, 18, 4> 19.39/17] [rubocop:Metrics/AbcSize]


3. Method has too many lines. [14/10] [rubocop:Metrics/MethodLength]

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Why these changes are being introduced:

Semantic searches ignore filters, meaning that
when semantic mode is enabled in the UI, the tabs
do not filter as intended.

Relevant ticket(s):

- [USE-593](https://mitlibraries.atlassian.net/browse/USE-593)

How this addresses that need:

SemanticQueryBuilder now applies FilterBuilder
filters to semantic queries, making it consistent
with lexical and hybrid search.

Side effects of this change:

We should consider adding regression test coverage
in the TIMDEX UI repo.
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.

3 participants