-
Notifications
You must be signed in to change notification settings - Fork 6
Add support for semantic search #954
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -45,8 +45,8 @@ The test recording process is as follows: | |||||||||||||||||||||||
| - Set the following values to whatever cluster you want to connect to in `.env` (Note: `.env` is preferred over `.env.test` because it is already in our `.gitignore` and will work together with `.env.test`.) | ||||||||||||||||||||||||
| - OPENSEARCH_URL | ||||||||||||||||||||||||
| - AWS_OPENSEARCH=true | ||||||||||||||||||||||||
| - AWS_OPENSEARCH_ACCESS_KEY_ID | ||||||||||||||||||||||||
| - AWS_OPENSEARCH_SECRET_ACCESS_KEY | ||||||||||||||||||||||||
| - AWS_ACCESS_KEY_ID | ||||||||||||||||||||||||
| - AWS_SECRET_ACCESS_KEY | ||||||||||||||||||||||||
| - AWS_REGION | ||||||||||||||||||||||||
| - Delete any cassette you want to regenerate (for new tests, you can skip this). If you are making a graphql test, nest your cassette inside the `opensearch_init` cassette. | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
@@ -158,14 +158,16 @@ locally. | |||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ## Production required Environment Variables | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| - `AWS_OPENSEARCH`: boolean. Set to true to enable AWSv4 Signing | ||||||||||||||||||||||||
| - `AWS_OPENSEARCH_ACCESS_KEY_ID` | ||||||||||||||||||||||||
| - `AWS_OPENSEARCH_SECRET_ACCESS_KEY` | ||||||||||||||||||||||||
| - `AWS_REGION` | ||||||||||||||||||||||||
| - `AWS_ACCESS_KEY_ID`: AWS credentials for OpenSearch and Lambda | ||||||||||||||||||||||||
| - `AWS_SECRET_ACCESS_KEY`: AWS credentials for OpenSearch and Lambda | ||||||||||||||||||||||||
|
Comment on lines
+161
to
+162
|
||||||||||||||||||||||||
| - `AWS_ACCESS_KEY_ID`: AWS credentials for OpenSearch and Lambda | |
| - `AWS_SECRET_ACCESS_KEY`: AWS credentials for OpenSearch and Lambda | |
| - `AWS_ACCESS_KEY_ID`: AWS access key for OpenSearch and Lambda. Required when using explicit static or temporary | |
| credentials. When running with an IAM role or metadata-based credentials (e.g., EC2/ECS/Lambda role), this is normally | |
| resolved automatically and should not be set. | |
| - `AWS_SECRET_ACCESS_KEY`: AWS secret access key for OpenSearch and Lambda. Required when using explicit static or | |
| temporary credentials. When running with an IAM role or metadata-based credentials, this is normally resolved | |
| automatically and should not be set. | |
| - `AWS_SESSION_TOKEN`: AWS session token for temporary (STS) credentials. Required when using temporary credentials | |
| issued by STS. When running with long-lived access keys or with an IAM role / metadata-based credentials, this is | |
| typically not set. |
Copilot
AI
Apr 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code now supports session-based AWS credentials via AWS_SESSION_TOKEN (see OpenSearch initializer), but this ENV isn’t documented here. Add AWS_SESSION_TOKEN (optional) to the environment variable list and clarify it’s required when using temporary credentials.
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -66,6 +66,8 @@ def record_id(id:, index:) | |||
| argument :boolean_type, String, required: false, default_value: 'OR', | ||||
| description: 'How to join multiword queries. Defaults to "OR" which means any ' \ | ||||
| 'of the words much match. Options include: "OR", "AND"' | ||||
| argument :query_mode, String, required: false, default_value: 'keyword', | ||||
| description: 'Search mode, either "keyword" or "semantic"' | ||||
|
|
||||
| # applied filters | ||||
| argument :access_to_files_filter, [String], | ||||
|
|
@@ -103,11 +105,11 @@ def record_id(id:, index:) | |||
| end | ||||
|
|
||||
| def search(searchterm:, citation:, contributors:, funding_information:, geodistance:, geobox:, identifiers:, | ||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||
| locations:, subjects:, title:, index:, source:, from:, boolean_type:, fulltext:, per_page: 20, **filters) | ||||
| locations:, subjects:, title:, index:, source:, from:, boolean_type:, fulltext:, per_page: 20, query_mode: 'keyword', **filters) | ||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||
| query = construct_query(searchterm, citation, contributors, funding_information, geodistance, geobox, identifiers, | ||||
| locations, subjects, title, source, boolean_type, filters, per_page) | ||||
| locations, subjects, title, source, boolean_type, filters, per_page, query_mode) | ||||
|
|
||||
| results = Opensearch.new.search(from, query, Timdex::OSClient, highlight_requested?, index, fulltext) | ||||
| results = Opensearch.new.search(from, query, Timdex::OSClient, highlight_requested?, index, fulltext, query_mode) | ||||
|
|
||||
| response = {} | ||||
| response[:hits] = results['hits']['total']['value'] | ||||
|
|
@@ -135,9 +137,10 @@ def inject_hits_fields_into_source(hits) | |||
| end | ||||
|
|
||||
| def construct_query(searchterm, citation, contributors, funding_information, geodistance, geobox, identifiers, | ||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||
| locations, subjects, title, source, boolean_type, filters, per_page) | ||||
| locations, subjects, title, source, boolean_type, filters, per_page, query_mode = 'keyword') | ||||
| query = {} | ||||
| query[:q] = searchterm | ||||
| query[:query_mode] = query_mode | ||||
|
||||
| query[:query_mode] = query_mode |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,13 @@ | ||
| # rubocop:disable Metrics/ClassLength | ||
| # rubocop:disable Metrics/MethodLength | ||
| class Opensearch | ||
| SIZE = 20 | ||
| MAX_SIZE = 200 | ||
|
|
||
| def search(from, params, client, highlight = false, index = nil, fulltext = false) | ||
| def search(from, params, client, highlight = false, index = nil, fulltext = false, query_mode = 'keyword') | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Found 4 issues: |
||
| @params = params | ||
| @highlight = highlight | ||
| @fulltext = fulltext?(fulltext) | ||
| @query_mode = query_mode | ||
| index = default_index unless index.present? | ||
| client.search(index:, | ||
| body: build_query(from)) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
@@ -54,8 +54,14 @@ def build_query(from) | |
|
|
||
| # Build the query portion of the elasticsearch json | ||
| def query | ||
| @query_strategy ||= LexicalQueryBuilder.new | ||
| @query_strategy.build(@params, @fulltext) | ||
| builder = case @query_mode | ||
| when 'semantic' | ||
| SemanticQueryBuilder.new | ||
| else | ||
| LexicalQueryBuilder.new | ||
| end | ||
|
|
||
| builder.build(@params, @fulltext) | ||
| end | ||
|
|
||
| def sort_builder | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| require 'aws-sdk-lambda' | ||
|
qltysh[bot] marked this conversation as resolved.
|
||
|
|
||
| class SemanticQueryBuilder | ||
| def build(params, _fulltext = false) | ||
|
qltysh[bot] marked this conversation as resolved.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| query_text = params[:q].to_s.strip | ||
|
|
||
| # If no query text provided, return a match_all query (consistent with keyword search behavior) | ||
| return { match_all: {} } if query_text.blank? | ||
|
|
||
| lambda_response = invoke_semantic_builder(query_text) | ||
| parse_lambda_response(lambda_response) | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def invoke_semantic_builder(query_text) | ||
| client_options = { | ||
| region: ENV.fetch('AWS_REGION', 'us-east-1'), | ||
| access_key_id: ENV.fetch('AWS_ACCESS_KEY_ID', nil), | ||
| secret_access_key: ENV.fetch('AWS_SECRET_ACCESS_KEY', nil) | ||
| } | ||
|
|
||
| client = Aws::Lambda::Client.new(client_options) | ||
| payload = { query: query_text } | ||
|
|
||
| response = client.invoke( | ||
| function_name: ENV.fetch('TIMDEX_SEMANTIC_BUILDER_FUNCTION_NAME', nil), | ||
| invocation_type: 'RequestResponse', | ||
| payload: payload.to_json | ||
| ) | ||
|
qltysh[bot] marked this conversation as resolved.
|
||
|
|
||
| parse_lambda_payload(response.payload) | ||
| rescue StandardError => e | ||
| raise "Semantic query builder Lambda error: #{e.message}" | ||
|
qltysh[bot] marked this conversation as resolved.
qltysh[bot] marked this conversation as resolved.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| end | ||
|
|
||
| def parse_lambda_payload(payload) | ||
| # AWS Lambda response payload can be an IO-like object (e.g., StringIO) or a string | ||
| payload_str = if payload.respond_to?(:read) | ||
| payload.read | ||
| else | ||
| payload.to_s | ||
| end | ||
| JSON.parse(payload_str) | ||
| rescue JSON::ParserError => e | ||
| raise "Invalid JSON response from semantic query builder: #{e.message}" | ||
| end | ||
|
|
||
| def parse_lambda_response(lambda_response) | ||
| # Lambda returns: { "query": { "bool": { "should": [...] } } } | ||
| # We extract and return just the inner query object | ||
| raise "Invalid semantic query builder response: missing 'query' key" unless lambda_response.key?('query') | ||
|
|
||
| query = lambda_response['query'] | ||
| raise 'Invalid semantic query builder response: query must be a Hash' unless query.is_a?(Hash) | ||
|
|
||
| query | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,20 +15,20 @@ def os_client | |
| def aws_os_client | ||
| OpenSearch::Client.new log: ENV.fetch('OPENSEARCH_LOG', false), url: ENV['OPENSEARCH_URL'] do |config| | ||
| # personal keys use expiring credentials with tokens | ||
| if ENV.fetch('AWS_OPENSEARCH_SESSION_TOKEN', false) | ||
| if ENV['AWS_SESSION_TOKEN'].present? | ||
| config.request :aws_sigv4, | ||
| service: 'es', | ||
| region: ENV['AWS_REGION'], | ||
|
Comment on lines
17
to
21
|
||
| access_key_id: ENV['AWS_OPENSEARCH_ACCESS_KEY_ID'], | ||
| secret_access_key: ENV['AWS_OPENSEARCH_SECRET_ACCESS_KEY'], | ||
| session_token: ENV['AWS_OPENSEARCH_SESSION_TOKEN'] | ||
| access_key_id: ENV['AWS_ACCESS_KEY_ID'], | ||
| secret_access_key: ENV['AWS_SECRET_ACCESS_KEY'], | ||
| session_token: ENV['AWS_SESSION_TOKEN'] | ||
| # application keys don't use tokens | ||
| else | ||
| config.request :aws_sigv4, | ||
| service: 'es', | ||
| region: ENV['AWS_REGION'], | ||
| access_key_id: ENV['AWS_OPENSEARCH_ACCESS_KEY_ID'], | ||
| secret_access_key: ENV['AWS_OPENSEARCH_SECRET_ACCESS_KEY'] | ||
| access_key_id: ENV['AWS_ACCESS_KEY_ID'], | ||
| secret_access_key: ENV['AWS_SECRET_ACCESS_KEY'] | ||
| end | ||
| end | ||
| end | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.