Merge pull request #92 from kitconcept/kid/460/hybrid-rrf-tests

RAG hybrid retrieval: BM25 + knn with RRF, integration test, prompt fix (#79)

Author: reekitconcept

IMPLEMENTATION-79.md

@@ -306,17 +306,62 @@ repository's estimates).
 
 **Step P1 — Hybrid RRF + tests (2d), first follow-up:**
 
-- [ ] **P1.1 Hybrid retrieval (1d)**: BM25 (reusing the existing
+- [x] **P1.1 Hybrid retrieval (1d)**: BM25 (reusing the existing
       `SolrSearch` query building) + knn as two Solr requests, client-side
       Reciprocal Rank Fusion (k=60) in `@rag-search`; designed to be swapped
       for Solr's native RRF combiner when 9.11/10.1 ships. Hybrid was
       originally MVP scope (empirical signal that pure vector is not enough
       on intranet content); the pure-knn MVP results should confirm the
       deferral was acceptable — if not, this item moves up.
-- [ ] **P1.2 Test pass (1d)**: unit tests (chunker, LLM client mocked) + one
+- [x] **P1.2 Test pass (1d)**: unit tests (chunker, LLM client mocked) + one
       integration test against the docker-compose stack with deterministic
       mock endpoints (CI needs no GPU/model).
 
+### Implementation notes — Step P1 (design decisions made on the way)
+
+- **Fusion happens at the parent-document level.** Chunks are invisible
+  to keyword search by design (stored, not indexed), so BM25 ranks
+  parent documents, while the knn chunk hits are collapsed to their
+  parents. Client-side RRF (`k=60`) fuses the two rankings; ties
+  resolve toward the vector ranking.
+- **The keyword scoring expression is an exact copy of the `@solr`
+  main query** — same fields, same boosts (decision after review):
+  - we DO want the same fields with the same weighting, including
+    `Subject`;
+  - we DO want `searchwords^1000` (the editorial keyword-pinning
+    mechanism) and the `-showinsearch:False` exclusion — both must
+    behave identically in the AI search (important);
+  - `id^0.75` is included; whether id matching is useful for natural
+    language questions may be revisited later (note);
+  - `text_prefix`/`text_suffix^0.75` are likely unneeded for full NL
+    questions (they serve terse/partial-word queries) but are included
+    for exact parity since their low boosts don't disturb the ranking;
+    may be revisited (note).
+  A pinning unit test spells out every clause, so the copy cannot
+  drift silently. NOT inherited (deliberately): facet/search-tab
+  conditions, highlighting, spellcheck, pagination — request-driven
+  UI machinery without meaning for the RAG query.
+- **Shared query builder refactoring deferred** (overflow list): for
+  now the expression is copied and the production `@solr` service is
+  left untouched; the later refactoring extracts the common core with
+  the acceptance criteria: (a) new unit tests for the factored-out
+  parts, (b) the pre-existing solr service tests pass unchanged after
+  the refactoring, (c) the RAG route gets its own tests modeled on the
+  solr service test examples.
+- **Keyword-only parents contribute context.** For a parent that only
+  the keyword ranking surfaced, the leading chunks are fetched from
+  Solr so its text reaches the model — otherwise a document found by
+  keyword search could appear as a source without being able to
+  influence the answer.
+- **Retrieval mode override** `KITCONCEPT_SOLR_RAG_RETRIEVAL=knn`
+  (default `hybrid`) exists solely for the pure-vector vs. hybrid
+  comparison in the evaluation on the real corpus; not a supported
+  setting.
+- The unit-test part of P1.2 had already been delivered inline with
+  Steps 1 and 3; the new piece is the integration test with a
+  deterministic mock LLM (topic-axis unit vectors, canned answer)
+  against the real docker Solr — CI-runnable with no network/model.
+
 **AI presentation via tabs configuration** (decided with the team
 2026-07-23, replaces the earlier "AI search" toggle; to be elaborated
 and implemented after the UX has been seen live in kitconcept.intranet):
@@ -343,6 +388,7 @@ remains):
 | Full configuration surface: registry records for model names, topK, chunk size, prompt override | 0.5d |
 | Acceptance test flow + full CI wiring (after the search-UI integration, so acceptance tests target the real UI) | 1d |
 | Separate Solr core/ports for tests vs. local dev site (see known issue 1) | 0.5d |
+| Extract a shared keyword-query builder used by both `@solr` and the RAG pipeline (criteria: unit tests for the shared part, old solr tests green, RAG route tests modeled on the solr test examples) | 0.5d |
 
 **Later roadmap** (tracked, not scheduled): full evaluation harness
 (Recall@k/MRR/nDCG as CI regression gate, RAGAS faithfulness/relevancy with

SPECIFICATION-79.md

@@ -331,6 +331,31 @@ Resolved open point:
    yet be produced manually, e2e tests follow when the editor
    settles.
 
+Remaining open point:
+
+1. **Facet/tab conditions and the AI search** — requirements question,
+   **to be discussed with Dante and Timo in the review**. The classic
+   search presents tabbed results per content type with facet conditions
+   in selected tabs (e.g. the person search). Two distinct readings for
+   the AI search:
+   - *Facets as navigation over the results:* does **not** transfer — the
+     source list is a small top-K evidence set justifying the answer, not
+     an exhaustive listing to narrow down; filtering the evidence away
+     would undermine the fact-checking contract.
+   - *Facets/tabs as scope constraints on the question* (ask within a
+     tab or facet selection, e.g. a department in the person search):
+     transfers well and the architecture already supports it — the knn
+     query composes any filter query as a pre-filter (as security, path
+     and language do today). Adding tab/facet parameters to
+     `@rag-search` is a bounded, additive change reusing the classic
+     search's condition builders. Caveat: chunks do not denormalize
+     `portal_type` or the facet fields, so scoped retrieval needs either
+     those fields on the chunks (schema addition + reindex) or filtering
+     at the parent-collapse step — a real design decision.
+   Proposed MVP stance: whole-intranet scope (matches the single search
+   box); scoped RAG as a follow-up once the modal UX (external search
+   modal project) defines what scoping looks like.
+
 ## 9. References
 
 Research summaries behind the recommendations (full reports in the planning

backend/news/79.feature.3

@@ -0,0 +1 @@
+Hybrid retrieval for the RAG search: parent-level Reciprocal Rank Fusion of the knn chunk ranking with the classic keyword query (an exact copy of the @solr main query incl. searchwords boost and showinsearch exclusion); prompt context labeled by document title instead of leaking bracketed reference numbers. @reebalazs

backend/src/kitconcept/solr/rag/config.py

@@ -53,6 +53,16 @@
 # generation prompt).
 TOP_K = 5
 
+# Reciprocal Rank Fusion constant for hybrid retrieval (the standard
+# value from Cormack et al. 2009; Solr's native RRF uses it as well).
+RRF_K = 60
+
+# Retrieval mode: "hybrid" (BM25 + vector, RRF-fused; the default) or
+# "knn" (pure vector). The env override exists for the evaluation on
+# the real corpus (compare the two modes); not a supported setting.
+RETRIEVAL_HYBRID = "hybrid"
+RETRIEVAL_KNN = "knn"
+
 REGISTRY_ENABLED_KEY = "kitconcept.solr.rag_enabled"
 
 # Endpoint paths, relative to the server root URL. The defaults match
@@ -70,6 +80,7 @@
 ENV_CHAT_MODEL = "KITCONCEPT_SOLR_LLM_CHAT_MODEL"
 ENV_EMBED_PATH = "KITCONCEPT_SOLR_LLM_EMBED_PATH"
 ENV_CHAT_PATH = "KITCONCEPT_SOLR_LLM_CHAT_PATH"
+ENV_RETRIEVAL = "KITCONCEPT_SOLR_RAG_RETRIEVAL"
 
 
 @dataclass(frozen=True)
@@ -84,6 +95,7 @@ class RagConfig:
     chat_path: str = DEFAULT_CHAT_PATH
     embed_timeout: float = EMBED_TIMEOUT
     chat_timeout: float = CHAT_TIMEOUT
+    retrieval: str = RETRIEVAL_HYBRID
 
 
 def rag_enabled() -> bool:
@@ -119,6 +131,7 @@ def get_rag_config() -> RagConfig | None:
         chat_model=os.environ.get(ENV_CHAT_MODEL, "").strip() or DEFAULT_CHAT_MODEL,
         embed_path=os.environ.get(ENV_EMBED_PATH, "").strip() or DEFAULT_EMBED_PATH,
         chat_path=os.environ.get(ENV_CHAT_PATH, "").strip() or DEFAULT_CHAT_PATH,
+        retrieval=os.environ.get(ENV_RETRIEVAL, "").strip() or RETRIEVAL_HYBRID,
     )
 
 

backend/src/kitconcept/solr/rag/pipeline.py

@@ -1,15 +1,28 @@
 """The RAG query pipeline: question -> retrieved chunks -> answer.
 
-Implements the single-turn RAG search (SPECIFICATION-79.md §4):
+Implements the single-turn RAG search (SPECIFICATION-79.md §4) with
+hybrid retrieval:
 
 1. embed the user's question (``search_query:`` prefix),
 2. retrieve the top chunks via a ``{!knn}`` query — the existing
    security/path/language filter queries compose with the vector query
    as HNSW pre-filters, so permission trimming works unchanged,
-3. collapse the chunk hits to their parent documents (the sources),
-4. generate the answer with the general-purpose model, prompted with
-   the matched chunk texts (chunk-level context, decision 9) and
-   constrained to the provided context.
+3. retrieve the top parent documents via the classic keyword (BM25)
+   query and fuse both rankings with client-side Reciprocal Rank
+   Fusion — hybrid is the industry default because keyword and vector
+   search have complementary failure modes (exact names/codes vs.
+   paraphrase). Solr's native RRF lands in 9.11/10.1; the client-side
+   fusion is drop-in replaceable by it,
+4. assemble the context chunks for the fused parent ranking (chunks
+   from the knn hits; fetched from Solr for keyword-only parents),
+5. generate the answer with the general-purpose model, prompted with
+   the chunk texts (chunk-level context, decision 9) and constrained
+   to the provided context.
+
+Fusion happens at the *parent document* level: chunks are invisible to
+keyword search by design (their text is stored but not indexed), so
+BM25 ranks parents, while the chunk hits of the knn side are collapsed
+to their parents (parent-document retrieval).
 
 The pipeline is independent of the REST service so it can be tested
 with a faked Solr connection and LLM client, and reused (e.g. by a
@@ -24,10 +37,14 @@
 from kitconcept.solr.rag.client import LLMClient
 from kitconcept.solr.rag.client import LLMClientError
 from kitconcept.solr.rag.config import RagConfig
+from kitconcept.solr.rag.config import RETRIEVAL_HYBRID
+from kitconcept.solr.rag.config import RRF_K
 from kitconcept.solr.rag.config import TOP_K
 from kitconcept.solr.rag.prompt import build_prompt
 from kitconcept.solr.rag.prompt import strip_thinking
 from kitconcept.solr.rag.prompt import SYSTEM_PROMPT
+from kitconcept.solr.services.solr_utils import escape
+from kitconcept.solr.services.solr_utils import replace_reserved
 from plone import api
 from zope.component import queryUtility
 
@@ -90,18 +107,28 @@ def run_rag_search(
         )
     try:
         chunks = search_chunks(conn, vector, security_filter, path_prefix, lang)
-        sources = collapse_sources(conn, chunks) if chunks else []
+        knn_parents = parent_ranking(chunks)
+        if config.retrieval == RETRIEVAL_HYBRID:
+            keyword_parents = search_keyword(
+                conn, question, security_filter, path_prefix, lang
+            )
+            fused_parents = rrf_fuse([knn_parents, keyword_parents])
+        else:
+            fused_parents = knn_parents
+        fused_parents = fused_parents[:TOP_K]
+        if not fused_parents:
+            # No matching (visible) content: not an error - the answer is
+            # that there is no answer.
+            return RagResult()
+        context_chunks = assemble_context(conn, chunks, fused_parents)
+        sources = build_sources(conn, fused_parents, context_chunks)
     except (SolrConnectionException, OSError) as e:
         # collective.solr raises raw socket errors (e.g.
         # ConnectionRefusedError) when the Solr server is down
         logger.warning("rag-search: Solr unavailable: %s", e)
         return RagResult.failure(ERROR_SOLR_UNAVAILABLE, str(e))
-    if not chunks:
-        # No matching (visible) content: not an error - the answer is
-        # that there is no answer.
-        return RagResult()
 
-    prompt = build_prompt(question, chunks)
+    prompt = build_prompt(question, context_chunks)
     try:
         answer = client.chat(prompt, system=SYSTEM_PROMPT)
     except LLMClientError as e:
@@ -148,35 +175,150 @@ def search_chunks(
         response.close()
 
 
-def collapse_sources(conn, chunks: list[dict]) -> list[dict]:
-    """Parent documents of the matched chunks, in rank order.
+def search_keyword(
+    conn,
+    question: str,
+    security_filter: str,
+    path_prefix: str | None = None,
+    lang: str | None = None,
+) -> list[str]:
+    """Top-K parent documents for the classic keyword (BM25) query.
+
+    The scoring expression is an exact copy of the ``@solr`` main
+    query (``SolrSearch._base_query``): same fields, same boosts —
+    including ``searchwords^1000`` (the editorial "pin a document for
+    a keyword" mechanism) and the ``-showinsearch:False`` exclusion,
+    both of which must behave identically in the AI search. Notes:
 
-    Parent-document retrieval: retrieval matches chunks, but the user
-    sees the parent documents as the sources. Parent metadata is
-    fetched from Solr in one query and merged with a snippet from the
-    best-ranked chunk of each parent.
+    - ``id^0.75``: kept for parity; whether id matching makes sense
+      for natural language questions may be revisited.
+    - ``text_prefix``/``text_suffix^0.75``: likely unneeded for full
+      NL questions (they serve terse/partial-word queries), but
+      included for exact parity since their low boosts don't disturb
+      the ranking; may be revisited.
+
+    Not inherited (deliberately): facet/search-tab conditions,
+    highlighting, spellcheck, pagination — request-driven UI machinery
+    of the classic search page that has no meaning here and does not
+    affect the ranking. Extracting a shared query builder so the copy
+    cannot drift is a planned refactoring (see the overflow list).
+
+    Chunks are excluded — they carry no indexed text anyway.
     """
+    term = f"({escape(replace_reserved(question))})"
+    query = (
+        f"+(Title:{term}^5 OR Description:{term}^2 OR id:{term}^0.75 "
+        f"OR text_prefix:{term}^0.75 OR text_suffix:{term}^0.75 "
+        f"OR default:{term} OR body_text:{term} OR SearchableText:{term} "
+        f"OR Subject:{term} OR searchwords:({term})^1000) -showinsearch:False"
+    )
+    filter_queries = [security_filter, "-is_rag_chunk:true"]
+    if path_prefix:
+        portal_path = "/".join(api.portal.get().getPhysicalPath())
+        prefix = portal_path + path_prefix.rstrip("/")
+        filter_queries.append(f'path_parents:"{prefix}"')
+    if lang:
+        filter_queries.append(f"Language:({lang} OR any)")
+    response = conn.search(
+        q=query,
+        fq=filter_queries,
+        fl="UID",
+        rows=TOP_K,
+    )
+    try:
+        results = SolrResponse(response).results()
+    finally:
+        response.close()
+    return [flare["UID"] for flare in results]
+
+
+def parent_ranking(chunks: list[dict]) -> list[str]:
+    """Parent UIDs of the chunk hits, deduplicated, in rank order."""
     order: list[str] = []
-    best_chunk: dict[str, dict] = {}
+    seen = set()
     for chunk in chunks:
         parent_uid = chunk.get("parent_uid")
-        if not parent_uid:
-            continue
-        if parent_uid not in best_chunk:
+        if parent_uid and parent_uid not in seen:
+            seen.add(parent_uid)
             order.append(parent_uid)
-            best_chunk[parent_uid] = chunk
-    if not order:
-        return []
-    parents = fetch_parents(conn, order)
+    return order
+
+
+def rrf_fuse(rankings: list[list[str]], k: int = RRF_K) -> list[str]:
+    """Reciprocal Rank Fusion of ranked UID lists.
+
+    ``score(d) = sum over rankings of 1 / (k + rank(d))`` — the
+    standard fusion that needs no score normalization (Cormack et al.
+    2009). Ties keep the order of the first ranking.
+    """
+    scores: dict[str, float] = {}
+    for ranking in rankings:
+        for index, uid in enumerate(ranking):
+            scores[uid] = scores.get(uid, 0.0) + 1.0 / (k + index + 1)
+    return sorted(scores, key=lambda uid: -scores[uid])
+
+
+def assemble_context(
+    conn, knn_chunks: list[dict], fused_parents: list[str]
+) -> list[dict]:
+    """Context chunks for the fused parent ranking, capped at TOP_K.
+
+    Chunks retrieved by the knn query are used as-is; for parents that
+    only the keyword ranking surfaced, the leading chunks are fetched
+    from Solr — their text must reach the model, otherwise a document
+    found by keyword search could not contribute to the answer.
+    """
+    by_parent: dict[str, list[dict]] = {}
+    for chunk in knn_chunks:
+        by_parent.setdefault(chunk.get("parent_uid"), []).append(chunk)
+    context: list[dict] = []
+    for parent_uid in fused_parents:
+        if parent_uid in by_parent:
+            context.extend(by_parent[parent_uid])
+        else:
+            context.extend(fetch_leading_chunks(conn, parent_uid))
+        if len(context) >= TOP_K:
+            break
+    return context[:TOP_K]
+
+
+def fetch_leading_chunks(conn, parent_uid: str, limit: int = 2) -> list[dict]:
+    """The first chunks of a document (for keyword-only parents)."""
+    response = conn.search(
+        q=f'+parent_uid:"{parent_uid}" +is_rag_chunk:true',
+        sort="chunk_index asc",
+        fl=CHUNK_FIELD_LIST,
+        rows=limit,
+    )
+    try:
+        return list(SolrResponse(response).results())
+    finally:
+        response.close()
+
+
+def build_sources(
+    conn, fused_parents: list[str], context_chunks: list[dict]
+) -> list[dict]:
+    """Source documents in fused rank order.
+
+    Parent-document retrieval: the user sees the parent documents as
+    the sources. Parent metadata is fetched from Solr in one query and
+    merged with a snippet from the best-ranked context chunk of each
+    parent (empty when a parent contributed no context).
+    """
+    best_chunk: dict[str, dict] = {}
+    for chunk in context_chunks:
+        best_chunk.setdefault(chunk.get("parent_uid"), chunk)
+    parents = fetch_parents(conn, fused_parents)
 
     portal = api.portal.get()
     portal_path = "/".join(portal.getPhysicalPath())
     portal_url = portal.absolute_url()
 
     sources = []
-    for parent_uid in order:
+    for parent_uid in fused_parents:
         parent = parents.get(parent_uid, {})
-        chunk = best_chunk[parent_uid]
+        chunk = best_chunk.get(parent_uid, {})
         path_string = parent.get("path_string") or chunk.get("path_string", "")
         url = (
             portal_url + path_string[len(portal_path) :]