diff --git a/common/metadata_utils.py b/common/metadata_utils.py index 9d76e486dd..27cd3797c8 100644 --- a/common/metadata_utils.py +++ b/common/metadata_utils.py @@ -209,10 +209,37 @@ async def apply_meta_data_filter( """Run conditions through ES/Infinity push-down when possible, in-memory otherwise.""" return filter_doc_ids_by_metadata(kb_ids or [], conditions, logic, _get_metas) + def _constrain(filtered: list[str]) -> list[str]: + """Intersect metadata-filter hits with the caller-scoped base doc ids. + + ``base_doc_ids`` is the retrieval scope the caller already narrowed to + (explicit ``document_ids``, a dialog-scoped doc set, ...). The metadata + filter must *narrow* that scope, not widen it: a doc outside the base + scope, or inside it but not matching the filter, must not be searched. + Mirrors ``constrainDocIDs`` in internal/service/metadata_filter.go. + """ + filtered = dedupe_list(filtered) + base_count = len(base_doc_ids or []) + if not base_doc_ids: + constrained = filtered + elif not filtered: + constrained = [] + else: + allowed = set(filtered) + constrained = dedupe_list([doc_id for doc_id in base_doc_ids if doc_id in allowed]) + logging.debug( + "Metadata filter scope constraint: base_count=%d, filter_hit_count=%d, constrained_count=%d, removed_all_filter_hits=%s", + base_count, + len(filtered), + len(constrained), + bool(filtered) and bool(base_doc_ids) and not constrained, + ) + return constrained + if method == "auto": filters: dict = await gen_meta_filter(chat_mdl, _get_metas(), question) logging.debug(f"Metadata filter(auto) generated: {filters}") - doc_ids.extend(_run_metadata_filter(filters["conditions"], filters.get("logic", "and"))) + doc_ids = _constrain(_run_metadata_filter(filters["conditions"], filters.get("logic", "and"))) if not doc_ids: return None elif method == "semi_auto": @@ -234,7 +261,7 @@ async def apply_meta_data_filter( if filtered_metas: filters: dict = await gen_meta_filter(chat_mdl, filtered_metas, question, constraints=constraints) logging.debug(f"Metadata filter(semi_auto) generated: {filters}") - doc_ids.extend(_run_metadata_filter(filters["conditions"], filters.get("logic", "and"))) + doc_ids = _constrain(_run_metadata_filter(filters["conditions"], filters.get("logic", "and"))) if not doc_ids: return None elif method == "manual": @@ -242,7 +269,7 @@ async def apply_meta_data_filter( if manual_value_resolver: filters = [manual_value_resolver(flt) for flt in filters] logging.debug(f"Metadata filter(manual): {filters}") - doc_ids.extend(_run_metadata_filter(filters, meta_data_filter.get("logic", "and"))) + doc_ids = _constrain(_run_metadata_filter(filters, meta_data_filter.get("logic", "and"))) if filters and not doc_ids: doc_ids = ["-999"] diff --git a/internal/service/dataset/search.go b/internal/service/dataset/search.go index 486b209015..237c19ae65 100644 --- a/internal/service/dataset/search.go +++ b/internal/service/dataset/search.go @@ -241,15 +241,8 @@ func (d *DatasetService) SearchDatasets(ctx context.Context, req *service.Search common.Warn("Failed to get flatted metadata, using empty metadata for filter", zap.Error(err)) flattedMeta = make(common.MetaData) } - if hasMetadataCondition { - filteredDocIDs, _ := service.ApplyMetaDataFilter(ctx, metadataFilter, flattedMeta, question, chatModelForFilter, documentIDs, datasetIDs) - docIDs = filteredDocIDs - } else { - filteredDocIDs, filterReturnedEmpty := service.ApplyMetaDataFilter(ctx, metadataFilter, flattedMeta, question, chatModelForFilter, nil, datasetIDs) - if !filterReturnedEmpty { - docIDs = append(docIDs, filteredDocIDs...) - } - } + filteredDocIDs, filterReturnedEmpty := service.ApplyMetaDataFilter(ctx, metadataFilter, flattedMeta, question, chatModelForFilter, documentIDs, datasetIDs) + docIDs = selectMetadataFilteredDocIDs(docIDs, filteredDocIDs, hasMetadataCondition, filterReturnedEmpty) } // Apply cross_languages and keyword extraction @@ -365,3 +358,10 @@ func (d *DatasetService) SearchDatasets(ctx context.Context, req *service.Search Total: retrievalResult.Total, }, nil } + +func selectMetadataFilteredDocIDs(currentDocIDs, filteredDocIDs []string, hasMetadataCondition, filterReturnedEmpty bool) []string { + if hasMetadataCondition || !filterReturnedEmpty { + return filteredDocIDs + } + return currentDocIDs +} diff --git a/internal/service/dataset/search_test.go b/internal/service/dataset/search_test.go index 08e48c0284..0b4790bc11 100644 --- a/internal/service/dataset/search_test.go +++ b/internal/service/dataset/search_test.go @@ -59,3 +59,32 @@ func TestSearchDatasetRequestToSearchDatasetsRequest(t *testing.T) { t.Fatalf("converted request did not preserve include_knowledge_compilation: %#v", converted) } } + +func TestSelectMetadataFilteredDocIDs(t *testing.T) { + tests := []struct { + name string + current []string + filtered []string + hasMetadataCondition bool + filterReturnedEmpty bool + want []string + }{ + {name: "metadata filter narrows explicit document ids", current: []string{"doc-1"}, filtered: []string{"doc-1"}, want: []string{"doc-1"}}, + {name: "metadata filter cannot widen explicit document ids", current: []string{"doc-1"}, filtered: []string{}, want: []string{}}, + {name: "empty generated filter keeps explicit document ids", current: []string{"doc-1"}, filtered: nil, filterReturnedEmpty: true, want: []string{"doc-1"}}, + {name: "metadata condition uses definitive empty result", current: []string{"doc-1"}, filtered: nil, hasMetadataCondition: true, filterReturnedEmpty: true, want: nil}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := selectMetadataFilteredDocIDs(tt.current, tt.filtered, tt.hasMetadataCondition, tt.filterReturnedEmpty) + if len(got) != len(tt.want) { + t.Fatalf("got %v, want %v", got, tt.want) + } + for i := range got { + if got[i] != tt.want[i] { + t.Fatalf("got %v, want %v", got, tt.want) + } + } + }) + } +} diff --git a/test/unit_test/common/test_metadata_filter_operators.py b/test/unit_test/common/test_metadata_filter_operators.py index dea5e3ef90..f3613027b1 100644 --- a/test/unit_test/common/test_metadata_filter_operators.py +++ b/test/unit_test/common/test_metadata_filter_operators.py @@ -192,3 +192,56 @@ def test_greater_than_unaffected_by_prior_dict_entry_coercing_the_query_value(): filters = [{"key": "score", "op": ">", "value": "5"}] assert meta_filter(metas, filters) == ["doc2"] + + +class TestApplyMetaDataFilterBaseScope: + """apply_meta_data_filter must narrow the caller-scoped base doc ids, + never widen them (parity with Go constrainDocIDs).""" + + @staticmethod + def _run(filter_def, metas, base): + import asyncio + import sys + import types + from unittest.mock import patch + + stub = types.ModuleType("rag.prompts.generator") + + async def gen_meta_filter(*a, **k): # pragma: no cover - manual mode never calls the LLM + raise RuntimeError("unexpected LLM call in manual mode") + + stub.gen_meta_filter = gen_meta_filter + + from common.metadata_utils import apply_meta_data_filter + + with patch.dict(sys.modules, {"rag.prompts.generator": stub}): + return asyncio.run(apply_meta_data_filter(filter_def, metas, base_doc_ids=base, kb_ids=None)) + + def test_filter_excludes_base_doc_that_does_not_match(self): + flt = {"method": "manual", "logic": "and", "manual": [{"key": "color", "op": "=", "value": "red"}]} + metas = {"color": {"red": ["docB"], "blue": ["docA"]}} + assert self._run(flt, metas, ["docA"]) == ["-999"] + + def test_filter_intersects_with_base_scope(self): + flt = {"method": "manual", "logic": "and", "manual": [{"key": "color", "op": "=", "value": "red"}]} + metas = {"color": {"red": ["docA", "docB"]}} + assert self._run(flt, metas, ["docA"]) == ["docA"] + + def test_empty_base_keeps_filter_hits_deduped(self): + flt = {"method": "manual", "logic": "and", "manual": [{"key": "color", "op": "=", "value": "red"}]} + metas = {"color": {"red": ["docA", "docA", "docB"]}} + result = self._run(flt, metas, []) + assert set(result) == {"docA", "docB"} + assert len(result) == 2 + + def test_duplicate_base_doc_ids_are_deduped(self): + flt = {"method": "manual", "logic": "and", "manual": [{"key": "color", "op": "=", "value": "red"}]} + metas = {"color": {"red": ["docA"]}} + assert self._run(flt, metas, ["docA", "docA"]) == ["docA"] + + def test_scope_constraint_logs_counts_and_full_removal(self, caplog): + flt = {"method": "manual", "logic": "and", "manual": [{"key": "color", "op": "=", "value": "red"}]} + metas = {"color": {"red": ["docB"]}} + with caplog.at_level("DEBUG"): + assert self._run(flt, metas, ["docA"]) == ["-999"] + assert "base_count=1, filter_hit_count=1, constrained_count=0, removed_all_filter_hits=True" in caplog.text