Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions app/graphql/timdex_schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,26 @@ class TimdexSchema < GraphQL::Schema
trace_class(GraphQL::Tracing::LegacyTrace)

query(Types::QueryType)

use GraphQL::Schema::Visibility

# Hide arguments marked as "INTERNAL USE ONLY" from introspection queries
def self.visible?(member, context)
# 1. Detect if the member is one of your internal arguments
if member.respond_to?(:description) && member.description&.include?('INTERNAL USE ONLY')

Comment thread
JPrevost marked this conversation as resolved.
# 2. Extract the root operation fields being requested
selected_fields = context.query&.selected_operation&.selections || []

# 3. Check if any root field is an introspection entrypoint (__schema or __type)
is_introspection = selected_fields.any? do |selection|
selection.respond_to?(:name) && %w[__schema __type].include?(selection.name)
end

# 4. Hide the arguments if GraphiQL/Introspection is sniffing the schema
return false if is_introspection
end
Comment thread
JPrevost marked this conversation as resolved.

super
Comment thread
qltysh[bot] marked this conversation as resolved.

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 = 6): visible? [qlty:function-complexity]


2. Cyclomatic complexity for visible? is too high. [10/7] [rubocop:Metrics/CyclomaticComplexity]


3. Perceived complexity for visible? is too high. [10/8] [rubocop:Metrics/PerceivedComplexity]

end
end
24 changes: 22 additions & 2 deletions app/graphql/types/query_type.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,18 +105,38 @@ def record_id(id:, index:)
argument :subjects_filter, [String], required: false, default_value: nil,
description: 'Filter by subject terms. Use the `contentType` aggregation ' \
'for a list of possible values. Multiple values are ANDed.'

# Internal semantic query tuning parameters (not documented publicly). Must start with `INTERNAL USE ONLY` to be

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Beyond the scope of this PR: do we have docs on what these parameters are and how to tune them?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

# excluded from public documentation.
argument :semantic_must_boost_threshold, Float, required: false, default_value: nil,
description: 'INTERNAL USE ONLY: Semantic query must boost ' \
'threshold (0.0-1.0)'
argument :semantic_drop_boost_threshold, Float, required: false, default_value: nil,
description: 'INTERNAL USE ONLY: Semantic query drop boost ' \
'threshold (0.0-1.0)'
argument :semantic_short_query_max_tokens, Integer, required: false, default_value: nil,
description: 'INTERNAL USE ONLY: Semantic query short ' \
'query max tokens'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there a range we should specify for max tokens?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We don't really know yet, but will hopefully learn over time based on measurements with the NDCG work.

This value works together with semantic_drop_boost_threshold.

Short queries (this value) don't drop any noise. Long queries drop noise based on the value of semantic_drop_boost_threshold.

It all hurts my brain.

end

def search(searchterm:, citation:, contributors:, funding_information:, geodistance:, geobox:, identifiers:,
locations:, subjects:, title:, index:, source:, from:, boolean_type:, fulltext:, per_page: 20,
query_mode: 'keyword', use_global_scoring: false, **filters)
query_mode: 'keyword', use_global_scoring: false, semantic_must_boost_threshold: nil,

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 2 issues:

1. Function with many parameters (count = 22): search [qlty:function-parameters]


2. Avoid parameter lists longer than 5 parameters. [22/5] [rubocop:Metrics/ParameterLists]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I choose to quietly ignore this until it's at least 5x the recommended maximum.

semantic_drop_boost_threshold: nil, semantic_short_query_max_tokens: nil, **filters)
query = construct_query(searchterm, citation, contributors, funding_information, geodistance, geobox, identifiers,
locations, subjects, title, source, boolean_type, filters, per_page, query_mode)

semantic_options = {
must_boost_threshold: semantic_must_boost_threshold,
drop_boost_threshold: semantic_drop_boost_threshold,
short_query_max_tokens: semantic_short_query_max_tokens
}
Comment thread
JPrevost marked this conversation as resolved.

results = Opensearch.new.search(from, query, Timdex::OSClient, highlight: highlight_requested?, index: index,
fulltext: fulltext, query_mode: query_mode,
requested_aggregations: requested_aggregations,
use_global_scoring: use_global_scoring)
use_global_scoring: use_global_scoring,
semantic_options: semantic_options)

response = {}
response[:hits] = results['hits']['total']['value']
Expand Down
4 changes: 2 additions & 2 deletions app/models/hybrid_query_builder.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
class HybridQueryBuilder
def build(params, fulltext: false)
def build(params, fulltext: false, semantic_options: {})
query_text = params[:q].to_s.strip

lexical_query = LexicalQueryBuilder.new.build(params, fulltext: fulltext)
Expand All @@ -8,7 +8,7 @@ def build(params, fulltext: false)
return lexical_query if query_text.blank?

begin
semantic_query = SemanticQueryBuilder.new.build(params, fulltext: fulltext)
semantic_query = SemanticQueryBuilder.new.build(params, fulltext: fulltext, semantic_options: semantic_options)

# Both succeeded - combine them with should clause while preserving filters
combine_queries(semantic_query, lexical_query)
Expand Down
10 changes: 8 additions & 2 deletions app/models/opensearch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@ class Opensearch
MAX_SIZE = 200

def search(from, params, client, highlight: false, index: nil, fulltext: false, query_mode: 'keyword',

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 2 issues:

1. Function with many parameters (count = 10): search [qlty:function-parameters]


2. Avoid parameter lists longer than 5 parameters. [10/5] [rubocop:Metrics/ParameterLists]

requested_aggregations: [], use_global_scoring: false)
requested_aggregations: [], use_global_scoring: false, semantic_options: {})
@params = params
@highlight = highlight
@fulltext = fulltext?(fulltext)
@query_mode = query_mode
@requested_aggregations = requested_aggregations
@semantic_options = semantic_options
index = default_index unless index.present?
search_params = { index:, body: build_query(from) }
search_params[:search_type] = 'dfs_query_then_fetch' if use_global_scoring
Expand Down Expand Up @@ -69,7 +70,12 @@ def query
LexicalQueryBuilder.new
end

builder.build(@params, fulltext: @fulltext)
# Only pass semantic_options to builders that support it (semantic and hybrid)
if @query_mode.in?(%w[semantic hybrid])
builder.build(@params, fulltext: @fulltext, semantic_options: @semantic_options || {})
else
builder.build(@params, fulltext: @fulltext)
end

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Function with high complexity (count = 5): query [qlty:function-complexity]

end

def sort_builder
Expand Down
22 changes: 19 additions & 3 deletions app/models/semantic_query_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ class SemanticQueryBuilder
# Dedicated exception for Lambda invocation failures (not parsing/validation errors)
class LambdaError < StandardError; end

def build(params, fulltext: false)
def build(params, fulltext: false, semantic_options: {})
query_text = params[:q].to_s.strip

# If no query text provided, return a match_all query with filters applied
Expand All @@ -14,7 +14,8 @@ def build(params, fulltext: false)
return { match_all: {} }
end

lambda_response = invoke_semantic_builder(query_text)
lambda_response = invoke_semantic_builder(query_text, semantic_options)

semantic_query = parse_lambda_response(lambda_response)

# Validate the query structure has a bool clause before applying filters
Expand All @@ -31,8 +32,23 @@ def build(params, fulltext: false)

private

def invoke_semantic_builder(query_text)
def invoke_semantic_builder(query_text, semantic_options = {})
payload = { query: query_text }

# Add optional semantic tuning parameters if provided
if semantic_options[:must_boost_threshold].present?
payload[:must_boost_threshold] =
semantic_options[:must_boost_threshold]
end
if semantic_options[:drop_boost_threshold].present?
payload[:drop_boost_threshold] =
semantic_options[:drop_boost_threshold]
end
if semantic_options[:short_query_max_tokens].present?
payload[:short_query_max_tokens] =
semantic_options[:short_query_max_tokens]
end
Comment thread
JPrevost marked this conversation as resolved.

function_name = ENV.fetch('TIMDEX_SEMANTIC_BUILDER_FUNCTION_NAME')

begin
Expand Down
92 changes: 92 additions & 0 deletions test/controllers/graphql_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1224,4 +1224,96 @@ class GraphqlControllerTest < ActionDispatch::IntegrationTest
}' }
assert_equal(200, response.status)
end

test 'graphql introspection hides internal semantic arguments via __schema' do
post '/graphql', params: { query: '{
__schema {
queryType {
fields(includeDeprecated: true) {
name
args {
name
}
}
}
}
}' }
assert_equal(200, response.status)
json = JSON.parse(response.body)

# Find the 'search' field
search_field = json['data']['__schema']['queryType']['fields'].find { |f| f['name'] == 'search' }
assert(search_field, 'search field should exist in schema')

# Get all argument names for the search field
arg_names = search_field['args'].map { |arg| arg['name'] }

# Verify internal semantic arguments are NOT present
assert_not_includes arg_names, 'semanticMustBoostThreshold'
assert_not_includes arg_names, 'semanticDropBoostThreshold'
assert_not_includes arg_names, 'semanticShortQueryMaxTokens'

# Verify other expected arguments ARE present
assert_includes arg_names, 'searchterm'
assert_includes arg_names, 'sourceFilter'
end

test 'graphql introspection hides internal semantic arguments via __type' do
post '/graphql', params: { query: '{
__type(name: "Query") {
fields(includeDeprecated: true) {
name
args {
name
}
}
}
}' }
assert_equal(200, response.status)
json = JSON.parse(response.body)

# Find the 'search' field
search_field = json['data']['__type']['fields'].find { |f| f['name'] == 'search' }
assert(search_field, 'search field should exist in Query type')

# Get all argument names for the search field
arg_names = search_field['args'].map { |arg| arg['name'] }

# Verify internal semantic arguments are NOT present
assert_not_includes arg_names, 'semanticMustBoostThreshold'
assert_not_includes arg_names, 'semanticDropBoostThreshold'
assert_not_includes arg_names, 'semanticShortQueryMaxTokens'

# Verify other expected arguments ARE present
assert_includes arg_names, 'searchterm'
assert_includes arg_names, 'sourceFilter'
end

test 'graphql internal semantic arguments still work in actual queries' do
VCR.use_cassette('opensearch init') do
VCR.use_cassette('graphql search data analytics') do
# Verify that the arguments work when sent in a real query
# (they are just hidden from introspection)
post '/graphql', params: { query: '{
search(
searchterm: "data analytics",
semanticMustBoostThreshold: 0.5,
semanticDropBoostThreshold: 0.2,
semanticShortQueryMaxTokens: 10
) {
records {
title
}
}
}' }
assert_equal(200, response.status)
json = JSON.parse(response.body)

# Verify the query succeeded and returned records
assert_nil json['errors'], "Query should not have errors: #{json['errors']}"
assert_equal('Data analytics and big data',
json['data']['search']['records'].first['title'])
end
end
end
end
Loading