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
1 change: 1 addition & 0 deletions backend/news/124.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
RAG: documents that contributed no context (e.g. Images, which are excluded from chunking) are no longer retrieved by the keyword leg nor cited as sources. @reebalazs
17 changes: 15 additions & 2 deletions backend/src/kitconcept/solr/rag/pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
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.processor import EXCLUDED_PORTAL_TYPES
from kitconcept.solr.rag.config import RETRIEVAL_HYBRID
from kitconcept.solr.rag.config import RRF_K
from kitconcept.solr.rag.config import TOP_K
Expand Down Expand Up @@ -226,6 +227,14 @@ def search_keyword(
f"OR Subject:{term} OR searchwords:({term})^1000) -showinsearch:False"
)
filter_queries = [security_filter, "-is_rag_chunk:true"]
# Mirror the chunking policy: types excluded from chunking (see
# EXCLUDED_PORTAL_TYPES) can never contribute context, so the
# keyword leg must not surface them either - otherwise they end up
# cited as sources with an empty snippet while the answer cannot
# know anything about them.
filter_queries.extend(
f'-portal_type:"{ptype}"' for ptype in sorted(EXCLUDED_PORTAL_TYPES)
)
if extra_filters:
filter_queries.extend(extra_filters)
if path_prefix:
Expand Down Expand Up @@ -319,7 +328,9 @@ def build_sources(
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).
parent. A parent without any context chunk contributed nothing to
the answer and is not cited (e.g. a keyword match on a document
that has no chunks, or a parent past the context cap).
"""
best_chunk: dict[str, dict] = {}
for chunk in context_chunks:
Expand All @@ -332,8 +343,10 @@ def build_sources(

sources = []
for parent_uid in fused_parents:
chunk = best_chunk.get(parent_uid)
if chunk is None:
continue
parent = parents.get(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) :]
Expand Down
5 changes: 4 additions & 1 deletion backend/src/kitconcept/solr/services/solr_utils_extra.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@ def __init__(self, config: dict):

@classmethod
def from_encoded(cls, raw: str):
if raw is not None:
# An empty string (e.g. a bare extra_conditions= URL
# parameter from the results page) means no conditions, like an
# absent parameter - not invalid input worth a log warning.
if raw:
try:
config = json.loads(base64.b64decode(raw))
except (
Expand Down
48 changes: 44 additions & 4 deletions backend/tests/rag/test_pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,13 +186,16 @@ def test_missing_parent_metadata_falls_back_to_chunk(self, conn):
assert sources[0]["title"] == "Vacation policy"
assert sources[0]["@id"] == "http://nohost/plone/vacation-policy"

def test_parent_without_context_chunk_has_empty_snippet(self, conn):
def test_parent_without_context_chunk_is_not_cited(self, conn):
# A parent that contributed no context chunk must not appear
# as a source: the answer cannot know anything about it (the
# observable symptom was an Image cited with an empty snippet
# while the answer denied knowing the image).
with mock.patch.object(
pipeline_module, "fetch_parents", return_value=dict(PARENTS)
):
sources = build_sources(conn, ["uid-a", "uid-b"], CHUNKS[:1])
assert sources[1]["snippet"] == ""
assert sources[1]["title"] == "Cafeteria"
assert [s["UID"] for s in sources] == ["uid-a"]


class TestSearchChunks:
Expand Down Expand Up @@ -256,10 +259,41 @@ def test_keyword_only_parent_joins_sources(self, environment):
"Type": "Page",
"path_string": "/plone/third",
}
with mock.patch.object(pipeline_module, "fetch_parents", return_value=parents):
chunk_c = {
"UID": "uid-c#rag-0",
"parent_uid": "uid-c",
"parent_title": "Third doc",
"chunk_text": "Third doc leading text.",
"path_string": "/plone/third",
}
with (
mock.patch.object(pipeline_module, "fetch_parents", return_value=parents),
mock.patch.object(
pipeline_module, "fetch_leading_chunks", return_value=[chunk_c]
),
):
result = run_rag_search("q", CONFIG, SECURITY_FQ)
assert "uid-c" in [s["UID"] for s in result.sources]

def test_keyword_only_parent_without_chunks_is_not_cited(self, environment):
# A keyword match on a document that has no chunks (e.g. an
# Image indexed before the type was excluded, or a document
# whose embedding failed) contributes nothing to the answer,
# so it must not be cited. fetch_leading_chunks returns [] via
# the environment fixture.
environment["keyword"].return_value = ["uid-a", "uid-c"]
parents = dict(PARENTS)
parents["uid-c"] = {
"UID": "uid-c",
"Title": "a green cat",
"Description": "",
"Type": "Image",
"path_string": "/plone/green-cat.jpg",
}
with mock.patch.object(pipeline_module, "fetch_parents", return_value=parents):
result = run_rag_search("q", CONFIG, SECURITY_FQ)
assert "uid-c" not in [s["UID"] for s in result.sources]

def test_extra_filters_reach_both_legs(self, environment):
run_rag_search(
"q",
Expand Down Expand Up @@ -369,6 +403,12 @@ def test_filters(self, conn):
assert SECURITY_FQ in params["fq"]
assert "-is_rag_chunk:true" in params["fq"]

def test_chunking_excluded_types_are_filtered(self, conn):
# Types excluded from chunking (e.g. Image) can never
# contribute context, so the keyword leg must not rank them.
params = self.fake_search(conn)
assert '-portal_type:"Image"' in params["fq"]

def test_question_is_escaped(self, conn):
conn.search.return_value = mock.Mock()
with mock.patch.object(pipeline_module, "SolrResponse") as solr_response:
Expand Down
16 changes: 16 additions & 0 deletions backend/tests/utils/test_utils_extra_conditions.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,3 +174,19 @@ def test_query_list_string_in_invalid_term_type(self):
obj = SolrExtraConditions(config)
with pytest.raises(BadRequest):
obj.query_list()


class TestFromEncodedEmpty:
def test_empty_string_means_no_conditions(self, caplog):
# A bare extra_conditions= URL parameter (the results page URL
# rewrite produces those) is not invalid input: no conditions,
# and no warning logged.
import logging

with caplog.at_level(logging.WARNING, logger="kitconcept.solr"):
conditions = SolrExtraConditions.from_encoded("")
assert conditions.query_list() == []
assert caplog.records == []

def test_none_means_no_conditions(self):
assert SolrExtraConditions.from_encoded(None).query_list() == []
Loading