Skip to content

Commit 7468316

Browse files
committed
Address observability and filter issues in hybrid search
This implements some feedback from Copilot during code review.
1 parent fc3e6f5 commit 7468316

4 files changed

Lines changed: 303 additions & 34 deletions

File tree

app/models/hybrid_query_builder.rb

Lines changed: 66 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,31 +2,83 @@ class HybridQueryBuilder
22
def build(params, fulltext: false)
33
query_text = params[:q].to_s.strip
44

5-
# If no query text provided, return a match_all query (consistent with keyword search behavior)
6-
return { match_all: {} } if query_text.blank?
7-
85
lexical_query = LexicalQueryBuilder.new.build(params, fulltext: fulltext)
96

7+
# If no query text provided, return lexical query so filters/other constraints are still applied
8+
return lexical_query if query_text.blank?
9+
1010
begin
1111
semantic_query = SemanticQueryBuilder.new.build(params, fulltext: fulltext)
1212

13-
# Both succeeded - combine them with should clause.
14-
{
15-
bool: {
16-
should: [
17-
normalize_keys(semantic_query),
18-
lexical_query
19-
]
20-
}
21-
}
22-
rescue StandardError
23-
# Semantic builder failed (Lambda error, etc.) - use lexical only.
13+
# Both succeeded - combine them with should clause while preserving filters
14+
combine_queries(semantic_query, lexical_query)
15+
rescue StandardError => e
16+
# Semantic builder failed (Lambda error, etc.) - use lexical only
17+
log_semantic_error(e)
18+
raise unless semantic_fallback_error?(e)
19+
2420
lexical_query
2521
end
2622
end
2723

2824
private
2925

26+
# Combines semantic and lexical queries while preserving non-q filters.
27+
# The q multi_match stays in the lexical branch to allow semantic-only matches.
28+
def combine_queries(semantic_query, lexical_query)
29+
# Extract filters (non-q constraints like title/citation/geo) to apply at top level.
30+
# Do NOT extract must (which contains the q multi_match) - it stays in lexical branch
31+
# so semantic matches can be returned without matching the q query.
32+
lexical_bool = lexical_query.is_a?(Hash) && lexical_query[:bool] ? lexical_query[:bool] : {}
33+
top_level_filters = lexical_bool[:filter] || []
34+
35+
# Keep the full lexical query structure (with q multi_match in must) but remove filters
36+
# so we don't duplicate them in the final query
37+
lexical_search = if lexical_query.is_a?(Hash) && lexical_query[:bool]
38+
{
39+
bool: {
40+
should: lexical_bool[:should] || [],
41+
must: lexical_bool[:must] || []
42+
}.reject { |_, v| v.blank? }
43+
}
44+
else
45+
lexical_query
46+
end
47+
48+
hybrid_bool = {
49+
should: [
50+
normalize_keys(semantic_query),
51+
lexical_search
52+
]
53+
}
54+
55+
# Apply only filters (non-q constraints) at top level so they apply to both branches
56+
hybrid_bool[:filter] = top_level_filters if top_level_filters.present?
57+
58+
# When filters are present, require at least one of the semantic/lexical branches
59+
# to match so the query does not degrade into a filter-only match
60+
hybrid_bool[:minimum_should_match] = 1 if top_level_filters.present?
61+
62+
{
63+
bool: hybrid_bool
64+
}
65+
end
66+
67+
# Logs semantic query builder failures for observability
68+
def log_semantic_error(error)
69+
return unless defined?(Rails) && Rails.respond_to?(:logger) && Rails.logger
70+
71+
Rails.logger.warn(
72+
"HybridQueryBuilder semantic query failed: #{error.class}: #{error.message}"
73+
)
74+
end
75+
76+
# Only fall back to lexical for Lambda/invocation errors from SemanticQueryBuilder.
77+
# All other errors (parsing, validation, unexpected bugs) should be re-raised.
78+
def semantic_fallback_error?(error)
79+
error.is_a?(SemanticQueryBuilder::LambdaError)
80+
end
81+
3082
# Recursively converts all string keys to symbols in hashes and nested structures.
3183
# This ensures consistency since semantic builder returns string keys while lexical uses symbols.
3284
def normalize_keys(value)

app/models/semantic_query_builder.rb

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
class SemanticQueryBuilder
2+
# Dedicated exception for Lambda invocation failures (not parsing/validation errors)
3+
class LambdaError < StandardError; end
4+
25
def build(params, fulltext: false)
36
query_text = params[:q].to_s.strip
47

@@ -13,16 +16,21 @@ def build(params, fulltext: false)
1316

1417
def invoke_semantic_builder(query_text)
1518
payload = { query: query_text }
16-
17-
response = Timdex::LambdaClient.invoke(
18-
function_name: ENV.fetch('TIMDEX_SEMANTIC_BUILDER_FUNCTION_NAME'),
19-
invocation_type: 'RequestResponse',
20-
payload: payload.to_json
21-
)
22-
19+
function_name = ENV.fetch('TIMDEX_SEMANTIC_BUILDER_FUNCTION_NAME')
20+
21+
begin
22+
response = Timdex::LambdaClient.invoke(
23+
function_name: function_name,
24+
invocation_type: 'RequestResponse',
25+
payload: payload.to_json
26+
)
27+
rescue StandardError => e
28+
# Only Lambda invocation errors are wrapped in LambdaError for graceful fallback
29+
raise LambdaError, "Lambda invocation error: #{e.message}", e.backtrace
30+
end
31+
32+
# Parse the response payload - errors here are not Lambda-specific
2333
parse_lambda_payload(response.payload)
24-
rescue StandardError => e
25-
raise "Semantic query builder Lambda error: #{e.message}"
2634
end
2735

2836
def parse_lambda_payload(payload)

0 commit comments

Comments
 (0)