mirror of
https://github.com/infiniflow/ragflow.git
synced 2026-11-11 05:40:52 +08:00
fix(api): intersect metadata filter results with the caller's doc scope (#19353)
Fix #19352 — metadata filter now intersects with document_ids (was extending/widening with duplicates). Mirrors Go's constrainDocIDs()
This commit is contained in:
1 parent
9ec20a97e9
commit
fa2467d33f
4 files changed
+121
-12
No files matched your search
@@ -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"]
|
||||
|
||||
|
||||
@@ -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
|
||||
}
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -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
|
||||
Reference in new issue
Block a user