Files
vectorize-io__hindsight/hindsight-api-slim/hindsight_api/engine/sql/oracle.py
T
Nicolò Boschi 78d46a7181 fix(recall): honour min_scores.keyword on every text-search backend (#3882) (#3938)
* fix(recall): honour min_scores.keyword on every text-search backend (#3882)

`min_scores.keyword` was a no-op on four of the six text-search backends,
including `native`, the default. A caller asking for `keyword >= 0.30` got
rows scoring 0.2 back.

`bm25_min_score` was added in #1947 as a VectorChord-specific gate: vchord's
`<&>` operator ranks *every* document, so it needed the analogue of native
tsvector's boolean `@@` match gate. Default 0, Oracle got it for symmetry,
behaviour unchanged everywhere else — correct and complete for that purpose.
#2422 then built the public `min_scores.keyword` floor on top of that same
parameter and touched no file under `engine/sql/`. From `retrieval.py` the
wiring looked finished, but only the vchord and Oracle branches ever read the
value; `native`, `pg_textsearch`, `pgroonga` and `pg_search` accepted it and
silently dropped it. An internal gate that defaults to off had been promoted
to a public per-request floor without the backends being re-audited.

- Push the floor into all six backends. pgroonga's `pgroonga_score()` and
  pg_search's `<schema>.score()` are only valid in the target list, and
  re-evaluating native's `ts_rank_cd` or pg_textsearch's `<@>` in WHERE would
  compute the score twice per row (and, for `<@>`, forfeit the index scan the
  ORDER BY relies on), so those four apply it by filtering the ordered LIMIT
  slice from the outside. Every arm orders by score DESC, so that keeps
  exactly the rows an inner predicate would.
- Make the floor inclusive. `min_scores` is documented as inclusive and the
  semantic arm uses `>= min_similarity`, but vchord/Oracle used `>`. The new
  `bm25_score_gate()` helper resolves the overload: `> 0` at the 0.0 default
  (the structural match gate #1947 needed), `>=` once a caller sets a floor,
  which subsumes it. Default behaviour is byte-identical.

The second half of #3882 is a documentation bug. "All inclusive, AND-ed" reads
as a predicate over each returned result, but `semantic` and `keyword` prune
only the arm they name: recall fuses four arms and returns what any of them
surfaced, so a result may carry `null` for a stage that did not surface it,
and graph/temporal results carry neither. That is deliberate — an intersection
would discard the strong single-arm matches hybrid retrieval exists to find.
Only `reranker` and `final` are per-result predicates, and they are what a
caller wanting abstention should use.

Note the two halves interact: once the pushdown is fixed, the union behaviour
can only ever surface as a `null`, never as a below-floor number, because
fusion copies `semantic` only from the semantic arm and `keyword` only from
the BM25 arm. The reporter's `{"keyword": 0.2}` under a 0.30 floor was purely
the pushdown bug. `MinScores`, the `RecallRequest` field, both MCP tool
descriptions and the recall docs now say exactly that.

Tests: `test_bm25_min_score_pushdown.py` asserts the floor reaches the SQL on
all six backends, that it is inclusive, and that the 0.0 default is unchanged
— SQL-shape assertions, so a new backend branch cannot repeat the omission on
a machine with no vchord/pgroonga/pg_search/Oracle available. (`pg_search`
gained a configurable function schema on main while this bug was open and
would have inherited the same gap.) The backend list is hoisted out of
`HindsightConfig.validate()` into `VALID_TEXT_SEARCH_EXTENSIONS` and the test
parametrizes over it, so a sixth backend is covered the moment it becomes
selectable rather than when someone remembers to update a second copy. Plus
DB-level regression tests for the keyword floor and for the per-arm contract.

Closes #3882

Claude-Session: https://claude.ai/code/session_011KDT484YujNcBxHzbfzNbk

* fix(recall): inclusive keyword floor — update the vchord contract test, harden the regression test

CI caught two things the local run could not.

1. `test_db_abstraction.py::test_build_bm25_arm_vchord_honors_custom_min_score`
   asserted `> 2.5`. That is the old exclusive gate this PR deliberately
   replaces, so the assertion is now `>= 2.5` with a comment recording why it
   changed. The test was doing its job; it encoded the behaviour the parameter
   had while it was vchord's internal match gate.

2. `test_keyword_floor_prunes_in_retrieval` asserted `len(kws) >= 2` so the
   floor would discriminate. On the three-fact corpus the keyword arm surfaces
   only one row for "animals", so the guard failed on its own precondition.
   Reworked to assert the contract without depending on corpus rank spread: a
   floor above every observed score must leave nothing keyword-scored (before
   the fix, native returned those rows with their real below-floor scores), and
   a floor at exactly the top score must keep that row (inclusivity, end to
   end). Neither a row count nor a score spread is something to assert on here —
   ranks can tie and the arm may surface a single row.

Also: format the floor with `!r` rather than `:g`. `:g` truncates to six
significant digits, so a caller echoing a `scores.keyword` value back as a floor
could get a literal that rounds up past its own row and silently drops it — the
exact round-trip the new inclusivity assertion exercises.

Claude-Session: https://claude.ai/code/session_011KDT484YujNcBxHzbfzNbk

* test(recall): drop the DB-level keyword-floor test; it asserts an environment property

`test_keyword_floor_prunes_in_retrieval` failed in CI twice, on two different
preconditions, for the same underlying reason: the BM25 arm surfaces nothing for
this module's seeded fixture, so `scores.keyword` is `null` on every result and
there is no floor to exercise. No other test in the repo asserts a non-null
`scores.keyword`, so nothing else depends on that arm surfacing rows here.

`search_vector` is a GENERATED ALWAYS column, so the fixture's raw INSERT does
populate it — the cause is somewhere else in the test configuration and is worth
a separate look, but it is not this fix. (The arm demonstrably works in a real
deployment: the #3882 reporter's own responses carry keyword scores.)

Deleted rather than skipped. The guard for this bug is
`test_bm25_min_score_pushdown.py`, which asserts the floor reaches the SQL on all
six backends deterministically and is what would have caught the original defect;
a DB test that cannot observe a keyword score adds no coverage over it.

Also fixes a vacuous assertion in `test_retrieval_floors_are_per_arm_not_per_result`:
`any(keyword is None)` holds trivially when every keyword is None. It now asserts
that not every result carries a score for both floored arms, which is the union
property the test is named for.

Claude-Session: https://claude.ai/code/session_011KDT484YujNcBxHzbfzNbk
2026-08-31 17:54:20 +02:00

322 lines
12 KiB
Python

"""Oracle 23ai SQL dialect implementation.
Provides Oracle-specific SQL fragments for parameter binding, JSON operators,
vector distance (VECTOR_DISTANCE), full-text search (Oracle Text), and
other non-portable patterns.
"""
from .base import SQLDialect, bm25_score_gate
class OracleDialect(SQLDialect):
"""SQL dialect for Oracle 23ai (python-oracledb)."""
# Characters that need escaping in Oracle Text CONTAINS queries.
_ORACLE_TEXT_SPECIAL = frozenset("&|!{}()[]~*?%-$>")
# Oracle Text reserved words that must be escaped with curly braces
# when used as plain search terms. Full list from Oracle Text docs:
# ABOUT, AND, BT, BTG, BTI, BTP, EQUIV, FUZZY, HASPATH, INPATH,
# MINUS, NEAR, NOT, NT, NTG, NTI, NTP, OR, PT, RT, SQE, SYN,
# TR, TRSYN, TT, WITHIN.
_ORACLE_TEXT_RESERVED = frozenset(
{
"about",
"and",
"bt",
"btg",
"bti",
"btp",
"equiv",
"fuzzy",
"haspath",
"inpath",
"minus",
"near",
"not",
"nt",
"ntg",
"nti",
"ntp",
"or",
"pt",
"rt",
"sqe",
"syn",
"tr",
"trsyn",
"tt",
"within",
}
)
# -- Parameter binding -----------------------------------------------
def param(self, n: int) -> str:
return f":{n}"
# -- Type casting ----------------------------------------------------
def cast(self, param: str, type_name: str) -> str:
# Oracle uses standard CAST syntax
oracle_type = self._map_type(type_name)
return f"CAST({param} AS {oracle_type})"
@staticmethod
def _map_type(pg_type: str) -> str:
"""Map PostgreSQL type names to Oracle equivalents."""
mapping = {
"jsonb": "CLOB", # Oracle stores JSON in CLOB
"json": "CLOB",
"text": "VARCHAR2(4000)",
"text[]": "CLOB", # JSON array
"uuid": "RAW(16)",
"uuid[]": "CLOB", # JSON array
"varchar[]": "CLOB", # JSON array
"float8": "BINARY_DOUBLE",
"float8[]": "CLOB",
"timestamptz": "TIMESTAMP WITH TIME ZONE",
"timestamptz[]": "CLOB",
"vector": "VECTOR",
"vector[]": "CLOB",
"integer": "NUMBER",
"bigint": "NUMBER",
"boolean": "NUMBER(1)",
}
return mapping.get(pg_type, pg_type.upper())
# -- Vector operations -----------------------------------------------
def vector_distance(self, col: str, param: str) -> str:
return f"VECTOR_DISTANCE({col}, {param}, COSINE)"
def vector_similarity(self, col: str, param: str) -> str:
return f"(1 - VECTOR_DISTANCE({col}, {param}, COSINE))"
# -- JSON operations -------------------------------------------------
def json_extract_text(self, col: str, key: str) -> str:
return f"JSON_VALUE({col}, '$.{key}')"
def json_contains(self, col: str, param: str) -> str:
return f"JSON_EXISTS({col}, '$?(@ == {param})')"
def json_merge(self, col: str, param: str) -> str:
return f"JSON_MERGEPATCH({col}, {param})"
# -- Text search -----------------------------------------------------
def text_search_score(self, col: str, query_param: str, *, index_name: str | None = None) -> str:
# Oracle Text: CONTAINS with SCORE
return "SCORE(1)"
def text_search_order(self, col: str, query_param: str, *, index_name: str | None = None) -> str:
return "SCORE(1) DESC"
# -- Fuzzy string matching -------------------------------------------
def similarity(self, col: str, param: str) -> str:
return f"UTL_MATCH.EDIT_DISTANCE_SIMILARITY({col}, {param}) / 100.0"
# -- Upsert ----------------------------------------------------------
def upsert(
self,
table: str,
columns: list[str],
conflict_columns: list[str],
update_columns: list[str],
) -> str:
col_list = ", ".join(columns)
src_cols = ", ".join(f":{i + 1} AS {c}" for i, c in enumerate(columns))
on_clause = " AND ".join(f"t.{c} = s.{c}" for c in conflict_columns)
if not update_columns:
return (
f"MERGE INTO {table} t "
f"USING (SELECT {src_cols} FROM DUAL) s "
f"ON ({on_clause}) "
f"WHEN NOT MATCHED THEN INSERT ({col_list}) "
f"VALUES ({', '.join(f's.{c}' for c in columns)})"
)
updates = ", ".join(f"t.{c} = s.{c}" for c in update_columns)
return (
f"MERGE INTO {table} t "
f"USING (SELECT {src_cols} FROM DUAL) s "
f"ON ({on_clause}) "
f"WHEN MATCHED THEN UPDATE SET {updates} "
f"WHEN NOT MATCHED THEN INSERT ({col_list}) "
f"VALUES ({', '.join(f's.{c}' for c in columns)})"
)
# -- Bulk operations -------------------------------------------------
def bulk_unnest(self, param_types: list[tuple[str, str]]) -> str:
# Oracle: use JSON_TABLE to expand a JSON array into rows
# Caller passes a JSON array as the parameter
columns = []
for i, (param, sql_type) in enumerate(param_types):
oracle_type = self._map_type(sql_type.rstrip("[]"))
columns.append(f"c{i} {oracle_type} PATH '$[{i}]'")
cols_spec = ", ".join(columns)
# Using first param as the JSON array source
first_param = param_types[0][0]
return f"JSON_TABLE({first_param}, '$[*]' COLUMNS ({cols_spec}))"
# -- Pagination ------------------------------------------------------
def limit_offset(self, limit_param: str, offset_param: str) -> str:
return f"OFFSET {offset_param} ROWS FETCH FIRST {limit_param} ROWS ONLY"
# -- RETURNING clause ------------------------------------------------
def returning(self, columns: list[str]) -> str:
# Oracle RETURNING requires INTO clause with output bind variables.
# The backend layer handles the output variable binding.
return f"RETURNING {', '.join(columns)} INTO {', '.join(f':out_{c}' for c in columns)}"
# -- Pattern matching ------------------------------------------------
def ilike(self, col: str, param: str) -> str:
return f"UPPER({col}) LIKE UPPER({param})"
# -- Array operations ------------------------------------------------
def array_any(self, param: str) -> str:
# Oracle: expand JSON array to rows for IN clause
return f"IN (SELECT value FROM JSON_TABLE({param}, '$[*]' COLUMNS (value PATH '$')))"
def array_all(self, param: str) -> str:
return f"NOT IN (SELECT value FROM JSON_TABLE({param}, '$[*]' COLUMNS (value PATH '$')))"
def array_contains(self, col: str, param: str) -> str:
# Oracle: check all elements of param array exist in col JSON array
return (
f"(SELECT COUNT(*) FROM JSON_TABLE({param}, '$[*]' COLUMNS (v PATH '$')) "
f"WHERE JSON_EXISTS({col}, '$[*]?(@ == v)')) = "
f"(SELECT COUNT(*) FROM JSON_TABLE({param}, '$[*]' COLUMNS (v PATH '$')))"
)
# -- Locking ---------------------------------------------------------
def for_update_skip_locked(self) -> str:
return "FOR UPDATE SKIP LOCKED"
# -- UUID generation -------------------------------------------------
def generate_uuid(self) -> str:
return "SYS_GUID()"
# -- Misc ------------------------------------------------------------
def greatest(self, *args: str) -> str:
return f"GREATEST({', '.join(args)})"
def current_timestamp(self) -> str:
return "SYSTIMESTAMP"
def array_agg(self, expr: str) -> str:
return f"JSON_ARRAYAGG({expr})"
# -- Retrieval query arms ----------------------------------------------
def build_semantic_arm(
self,
*,
table: str,
cols: str,
fact_type: str,
embedding_param: str,
bank_id_param: str,
fetch_limit: int,
min_similarity: float,
tags_clause: str = "",
groups_clause: str = "",
extra_where: str = "",
) -> str:
# Oracle 23ai: VECTOR_DISTANCE for cosine, FETCH FIRST for limiting.
# Wrapped in a derived table to work within UNION ALL.
return (
f"SELECT * FROM (SELECT {cols},"
f" 1 - VECTOR_DISTANCE(embedding, {embedding_param}, COSINE) AS similarity,"
f" NULL AS bm25_score,"
f" 'semantic' AS source"
f" FROM {table}"
f" WHERE bank_id = {bank_id_param}"
f" AND fact_type = '{fact_type}'"
f" AND embedding IS NOT NULL"
f" AND (1 - VECTOR_DISTANCE(embedding, {embedding_param}, COSINE)) >= {min_similarity}"
f" {tags_clause}"
f" {groups_clause}"
f" {extra_where}"
f" ORDER BY VECTOR_DISTANCE(embedding, {embedding_param}, COSINE)"
f" FETCH FIRST {fetch_limit} ROWS ONLY) t"
)
def build_bm25_arm(
self,
*,
table: str,
cols: str,
fact_type: str,
bank_id_param: str,
limit_param: str,
text_param: str,
tags_clause: str = "",
groups_clause: str = "",
arm_index: int = 0,
text_search_extension: str = "native",
bm25_language: str = "english",
bm25_min_score: float = 0.0,
pg_search_function_schema: str = "paradedb",
extra_where: str = "",
) -> str:
# Oracle Text: CONTAINS() / SCORE() with the CTXSYS.CONTEXT index.
# Each arm gets a unique SCORE label (10 + arm_index) to avoid
# conflicts within the UNION ALL.
label = 10 + arm_index
return (
f"SELECT * FROM (SELECT {cols},"
f" NULL AS similarity,"
f" SCORE({label}) AS bm25_score,"
f" 'bm25' AS source"
f" FROM {table}"
f" WHERE bank_id = {bank_id_param}"
f" AND fact_type = '{fact_type}'"
# CONTAINS already gates to genuine matches, so at the 0.0 default the
# gate is the structural `> 0`; a caller's `min_scores.keyword` floor
# replaces it with an inclusive `>=`, uniform across backends.
f" AND CONTAINS(text, {text_param}, {label}) {bm25_score_gate(bm25_min_score)}"
f" {tags_clause}"
f" {groups_clause}"
f" {extra_where}"
f" ORDER BY SCORE({label}) DESC"
f" FETCH FIRST {limit_param} ROWS ONLY) t{arm_index}"
)
def prepare_bm25_text(
self,
tokens: list[str],
query_text: str,
*,
text_search_extension: str = "native",
max_query_terms: int | None = None,
) -> str:
# Oracle Text: filter tokens with special chars, escape reserved words
# with curly braces (e.g. "about" → "{about}"), and join with OR.
safe: list[str] = []
for t in tokens:
if any(c in self._ORACLE_TEXT_SPECIAL for c in t):
continue
if t.lower() in self._ORACLE_TEXT_RESERVED:
safe.append(f"{{{t}}}")
else:
safe.append(t)
if safe:
return " OR ".join(safe)
# All tokens were filtered out — escape the original query text as a
# single term so we still attempt a search rather than erroring out.
fallback = query_text.strip() or tokens[0]
return f"{{{fallback}}}"