Drop the unused per-dataset filter default
No `DatasetSpec` declared `search_filter`, so `resolve_search_filter` and the `--filter ""` clearing rule reconciled the flag against a default that never existed. The flag alone covers the case. An empty clause reaches `ChunkRepository.search`, which already treats it as unfiltered. Rename to `document_filter` throughout, matching `run_capability_question`'s parameter and the metadata key that lands in Logfire. `_stub_spec` merges its overrides, so a test can override a loader instead of rebuilding the whole spec.
This commit is contained in:
parent
9b2ae347d2
commit
a75d89122e
5 changed files with 46 additions and 127 deletions
|
|
@ -1,6 +1,10 @@
|
|||
# Changelog
|
||||
## [Unreleased]
|
||||
|
||||
### Added
|
||||
|
||||
- `evaluations run --filter/-f CLAUSE`: SQL `WHERE` clause over document columns, applied to the retrieval benchmark's searches and to every capability search during QA. Recorded as `document_filter` in experiment metadata.
|
||||
|
||||
### Removed
|
||||
|
||||
- `wix` evaluation dataset and its reference config `evaluations/configs/wix.yaml`.
|
||||
|
|
|
|||
|
|
@ -102,19 +102,7 @@ If the corpora are distinguished by a tag rather than by URI, attach it at inges
|
|||
evaluations run orb_text --skip-db --filter "metadata LIKE '%\"corpus\": \"orb_text\"%'"
|
||||
```
|
||||
|
||||
The clause applies to both benchmark phases — the retrieval benchmark's searches and every search the capability runs during QA — so the two score the same subset. It is recorded as `search_filter` in the run's experiment metadata, so a filtered run is never mistaken for an unfiltered one when comparing results.
|
||||
|
||||
A dataset can declare its own default in its `DatasetSpec`, so runs need no flag:
|
||||
|
||||
```python
|
||||
ORB_TEXT_SPEC = DatasetSpec(
|
||||
key="orb_text",
|
||||
...
|
||||
search_filter="metadata LIKE '%\"corpus\": \"orb_text\"%'",
|
||||
)
|
||||
```
|
||||
|
||||
`--filter` overrides that default; passing an empty string (`--filter ""`) clears it and searches the whole database.
|
||||
The clause applies to both benchmark phases — the retrieval benchmark's searches and every search the capability runs during QA — so the two score the same subset. It is recorded as `document_filter` in the run's experiment metadata, so a filtered run is never mistaken for an unfiltered one when comparing results.
|
||||
|
||||
Filtering affects searches only — a run without `--skip-db` still populates the database with the dataset's full corpus.
|
||||
|
||||
|
|
|
|||
|
|
@ -58,17 +58,6 @@ configure_cli_logging()
|
|||
console = Console()
|
||||
|
||||
|
||||
def resolve_search_filter(spec: DatasetSpec, override: str | None) -> str | None:
|
||||
"""Pick the document filter for a run: `--filter` wins over the dataset's.
|
||||
|
||||
An empty `--filter ""` is honoured as "no filter", so a dataset that
|
||||
declares one can still be run against the whole database.
|
||||
"""
|
||||
if override is None:
|
||||
return spec.search_filter
|
||||
return override or None
|
||||
|
||||
|
||||
def build_experiment_metadata(
|
||||
dataset_key: str,
|
||||
test_cases: int,
|
||||
|
|
@ -76,7 +65,7 @@ def build_experiment_metadata(
|
|||
judge_config: ModelConfig | None = None,
|
||||
target: Target = "rag-capability",
|
||||
capability_config: ModelConfig | None = None,
|
||||
search_filter: str | None = None,
|
||||
document_filter: str | None = None,
|
||||
) -> dict[str, Any]:
|
||||
"""Build experiment metadata for Logfire tracking."""
|
||||
metadata: dict[str, Any] = {
|
||||
|
|
@ -100,7 +89,7 @@ def build_experiment_metadata(
|
|||
"qa_enable_thinking": config.qa.model.enable_thinking,
|
||||
"qa_extra_body": config.qa.model.extra_body,
|
||||
"qa_max_searches": config.qa.max_searches,
|
||||
"search_filter": search_filter,
|
||||
"document_filter": document_filter,
|
||||
}
|
||||
if judge_config is not None:
|
||||
metadata.update(
|
||||
|
|
@ -204,7 +193,7 @@ async def run_retrieval_benchmark(
|
|||
name: str | None = None,
|
||||
db_path: Path | None = None,
|
||||
multimodal_only: bool = False,
|
||||
search_filter: str | None = None,
|
||||
document_filter: str | None = None,
|
||||
) -> dict[str, float] | None:
|
||||
if spec.retrieval_loader is None or spec.retrieval_mapper is None:
|
||||
console.print("Skipping retrieval benchmark; no retrieval config.")
|
||||
|
|
@ -261,7 +250,7 @@ async def run_retrieval_benchmark(
|
|||
|
||||
async def retrieval_target(question: str) -> list[str]:
|
||||
chunks = await rag.search(
|
||||
query=question, limit=5, include_images=False, filter=search_filter
|
||||
query=question, limit=5, include_images=False, filter=document_filter
|
||||
)
|
||||
|
||||
seen = set()
|
||||
|
|
@ -280,7 +269,7 @@ async def run_retrieval_benchmark(
|
|||
dataset_key=spec.key,
|
||||
test_cases=len(cases),
|
||||
config=config,
|
||||
search_filter=search_filter,
|
||||
document_filter=document_filter,
|
||||
)
|
||||
|
||||
report = await dataset.evaluate(
|
||||
|
|
@ -381,7 +370,7 @@ async def run_qa_benchmark(
|
|||
target: Target = "rag-capability",
|
||||
capability_model: ModelConfig | None = None,
|
||||
case_ids: set[str] | None = None,
|
||||
search_filter: str | None = None,
|
||||
document_filter: str | None = None,
|
||||
) -> ReportCaseFailure[str, str, dict[str, str]] | None:
|
||||
corpus = spec.qa_loader()
|
||||
corpus = _filter_qa_corpus(corpus, case_ids)
|
||||
|
|
@ -437,7 +426,7 @@ async def run_qa_benchmark(
|
|||
judge_config=judge_config,
|
||||
target=target,
|
||||
capability_config=capability_config,
|
||||
search_filter=search_filter,
|
||||
document_filter=document_filter,
|
||||
)
|
||||
|
||||
async def _evaluate(answer_fn: Callable[[str], Awaitable[str]]):
|
||||
|
|
@ -459,7 +448,7 @@ async def run_qa_benchmark(
|
|||
config=config,
|
||||
question=question,
|
||||
capability_model=resolved_capability_model,
|
||||
document_filter=search_filter,
|
||||
document_filter=document_filter,
|
||||
)
|
||||
set_eval_attribute("cited_uris", result.cited_uris)
|
||||
set_eval_attribute("cited_chunk_ids", result.cited_chunk_ids)
|
||||
|
|
@ -550,12 +539,10 @@ async def evaluate_dataset(
|
|||
target: Target = "rag-capability",
|
||||
capability_model: ModelConfig | None = None,
|
||||
case_ids: set[str] | None = None,
|
||||
search_filter: str | None = None,
|
||||
document_filter: str | None = None,
|
||||
) -> None:
|
||||
# Resolved once so both phases score the same subset of the database.
|
||||
resolved_filter = resolve_search_filter(spec, search_filter)
|
||||
if resolved_filter is not None:
|
||||
console.print(f"Document filter: {resolved_filter}", style="dim")
|
||||
if document_filter is not None:
|
||||
console.print(f"Document filter: {document_filter}", style="dim")
|
||||
|
||||
if not skip_db:
|
||||
console.print(f"Using dataset: {spec.key}", style="bold magenta")
|
||||
|
|
@ -572,7 +559,7 @@ async def evaluate_dataset(
|
|||
name=name,
|
||||
db_path=db_path,
|
||||
multimodal_only=multimodal_only,
|
||||
search_filter=resolved_filter,
|
||||
document_filter=document_filter,
|
||||
)
|
||||
|
||||
if not skip_qa:
|
||||
|
|
@ -589,7 +576,7 @@ async def evaluate_dataset(
|
|||
target=target,
|
||||
capability_model=capability_model,
|
||||
case_ids=case_ids,
|
||||
search_filter=resolved_filter,
|
||||
document_filter=document_filter,
|
||||
)
|
||||
|
||||
|
||||
|
|
@ -679,7 +666,7 @@ def run(
|
|||
"analysis.model when --target is analysis-capability) from the config."
|
||||
),
|
||||
),
|
||||
search_filter: str | None = typer.Option(
|
||||
document_filter: str | None = typer.Option(
|
||||
None,
|
||||
"--filter",
|
||||
"-f",
|
||||
|
|
@ -687,8 +674,7 @@ def run(
|
|||
"SQL WHERE clause over document columns (id, uri, title, "
|
||||
"created_at, updated_at, metadata) restricting every benchmark "
|
||||
"search, e.g. \"uri LIKE '%arxiv%'\". metadata is stored as a "
|
||||
"string, so match it with LIKE. Overrides the dataset's own "
|
||||
"filter; pass an empty string to search the whole database."
|
||||
"string, so match it with LIKE."
|
||||
),
|
||||
),
|
||||
filter_ids: Path | None = typer.Option(
|
||||
|
|
@ -728,7 +714,7 @@ def run(
|
|||
target=target_value,
|
||||
capability_model=capability_model_config,
|
||||
case_ids=_load_case_ids(filter_ids),
|
||||
search_filter=search_filter,
|
||||
document_filter=document_filter,
|
||||
)
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -46,7 +46,6 @@ class DatasetSpec:
|
|||
retrieval_evaluator: Evaluator | None = None
|
||||
qa_evaluator: Evaluator | None = None
|
||||
document_limit: int | None = None
|
||||
search_filter: str | None = None
|
||||
|
||||
def db_path(self, override_path: Path | None = None) -> Path:
|
||||
"""Get the database path.
|
||||
|
|
|
|||
|
|
@ -17,16 +17,16 @@ from haiku.rag.config.models import AppConfig, ModelConfig
|
|||
|
||||
def _stub_spec(**overrides) -> DatasetSpec:
|
||||
"""A DatasetSpec whose loaders/mappers are inert, for tests that only
|
||||
exercise the surrounding plumbing."""
|
||||
return DatasetSpec(
|
||||
key="test",
|
||||
db_filename="test.lancedb",
|
||||
document_loader=lambda: None, # type: ignore[arg-type] # ty: ignore[invalid-argument-type]
|
||||
document_mapper=lambda doc: None,
|
||||
qa_loader=lambda: [], # type: ignore[arg-type] # ty: ignore[invalid-argument-type]
|
||||
qa_case_builder=lambda idx, doc: None, # type: ignore[arg-type] # ty: ignore[invalid-argument-type]
|
||||
**overrides,
|
||||
)
|
||||
exercise the surrounding plumbing. Any field can be overridden."""
|
||||
fields: dict = {
|
||||
"key": "test",
|
||||
"db_filename": "test.lancedb",
|
||||
"document_loader": lambda: None,
|
||||
"document_mapper": lambda doc: None,
|
||||
"qa_loader": lambda: [],
|
||||
"qa_case_builder": lambda idx, doc: None,
|
||||
}
|
||||
return DatasetSpec(**{**fields, **overrides})
|
||||
|
||||
|
||||
class TestBuildExperimentMetadata:
|
||||
|
|
@ -581,50 +581,24 @@ class TestRetrievalTarget:
|
|||
assert result["map"] == 0.5
|
||||
|
||||
|
||||
class TestResolveSearchFilter:
|
||||
def test_dataset_filter_used_when_no_override(self) -> None:
|
||||
from evaluations.benchmark import resolve_search_filter
|
||||
|
||||
spec = _stub_spec(search_filter="uri LIKE '%arxiv%'")
|
||||
assert resolve_search_filter(spec, None) == "uri LIKE '%arxiv%'"
|
||||
|
||||
def test_override_wins(self) -> None:
|
||||
from evaluations.benchmark import resolve_search_filter
|
||||
|
||||
spec = _stub_spec(search_filter="uri LIKE '%arxiv%'")
|
||||
assert resolve_search_filter(spec, "uri LIKE '%.pdf'") == "uri LIKE '%.pdf'"
|
||||
|
||||
def test_empty_override_clears_dataset_filter(self) -> None:
|
||||
"""`--filter ""` runs a filtered dataset against the whole database."""
|
||||
from evaluations.benchmark import resolve_search_filter
|
||||
|
||||
spec = _stub_spec(search_filter="uri LIKE '%arxiv%'")
|
||||
assert resolve_search_filter(spec, "") is None
|
||||
|
||||
def test_none_when_neither_is_set(self) -> None:
|
||||
from evaluations.benchmark import resolve_search_filter
|
||||
|
||||
assert resolve_search_filter(_stub_spec(), None) is None
|
||||
|
||||
|
||||
class TestSearchFilterThreading:
|
||||
"""The resolved filter must reach both benchmark phases, so retrieval and
|
||||
QA score the same subset of the database."""
|
||||
class TestDocumentFilterThreading:
|
||||
"""The filter must reach both benchmark phases, so retrieval and QA score
|
||||
the same subset of the database."""
|
||||
|
||||
def test_metadata_records_filter(self) -> None:
|
||||
result = build_experiment_metadata(
|
||||
dataset_key="test",
|
||||
test_cases=1,
|
||||
config=AppConfig(),
|
||||
search_filter="uri LIKE '%arxiv%'",
|
||||
document_filter="uri LIKE '%arxiv%'",
|
||||
)
|
||||
assert result["search_filter"] == "uri LIKE '%arxiv%'"
|
||||
assert result["document_filter"] == "uri LIKE '%arxiv%'"
|
||||
|
||||
def test_metadata_filter_is_none_when_unset(self) -> None:
|
||||
result = build_experiment_metadata(
|
||||
dataset_key="test", test_cases=1, config=AppConfig()
|
||||
)
|
||||
assert result["search_filter"] is None
|
||||
assert result["document_filter"] is None
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_retrieval_search_receives_filter(self, tmp_path: Path) -> None:
|
||||
|
|
@ -655,7 +629,7 @@ class TestSearchFilterThreading:
|
|||
spec,
|
||||
AppConfig(),
|
||||
db_path=tmp_path / "test.lancedb",
|
||||
search_filter="uri LIKE '%arxiv%'",
|
||||
document_filter="uri LIKE '%arxiv%'",
|
||||
)
|
||||
|
||||
assert searches[0]["filter"] == "uri LIKE '%arxiv%'"
|
||||
|
|
@ -668,12 +642,8 @@ class TestSearchFilterThreading:
|
|||
from evaluations.evaluators import NumberMatchEvaluator
|
||||
|
||||
# A deterministic evaluator, so no judge model is constructed.
|
||||
spec = DatasetSpec(
|
||||
key="test",
|
||||
db_filename="test.lancedb",
|
||||
document_loader=lambda: None, # type: ignore[arg-type] # ty: ignore[invalid-argument-type]
|
||||
document_mapper=lambda doc: None,
|
||||
qa_loader=lambda: [{"question": "What is X?", "answer": "42"}], # type: ignore[arg-type] # ty: ignore[invalid-argument-type]
|
||||
spec = _stub_spec(
|
||||
qa_loader=lambda: [{"question": "What is X?", "answer": "42"}],
|
||||
qa_case_builder=lambda idx, doc: Case(
|
||||
name=f"case-{idx}",
|
||||
inputs=doc["question"],
|
||||
|
|
@ -691,43 +661,15 @@ class TestSearchFilterThreading:
|
|||
spec,
|
||||
AppConfig(),
|
||||
db_path=tmp_path / "test.lancedb",
|
||||
search_filter="uri LIKE '%arxiv%'",
|
||||
document_filter="uri LIKE '%arxiv%'",
|
||||
)
|
||||
|
||||
mock_run.assert_awaited_once()
|
||||
assert mock_run.call_args[1]["document_filter"] == "uri LIKE '%arxiv%'"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_evaluate_dataset_resolves_once_for_both_phases(self) -> None:
|
||||
"""The dataset's own filter reaches retrieval and QA without a flag."""
|
||||
spec = _stub_spec(search_filter="""metadata LIKE '%"corpus": "orb_text"%'""")
|
||||
|
||||
with (
|
||||
patch(
|
||||
"evaluations.benchmark.run_retrieval_benchmark", new_callable=AsyncMock
|
||||
) as mock_retrieval,
|
||||
patch(
|
||||
"evaluations.benchmark.run_qa_benchmark", new_callable=AsyncMock
|
||||
) as mock_qa,
|
||||
):
|
||||
await evaluate_dataset(
|
||||
spec=spec,
|
||||
config=AppConfig(),
|
||||
skip_db=True,
|
||||
skip_retrieval=False,
|
||||
skip_qa=False,
|
||||
limit=None,
|
||||
name=None,
|
||||
db_path=None,
|
||||
)
|
||||
|
||||
async def test_evaluate_dataset_passes_filter_to_both_phases(self) -> None:
|
||||
expected = """metadata LIKE '%"corpus": "orb_text"%'"""
|
||||
assert mock_retrieval.call_args[1]["search_filter"] == expected
|
||||
assert mock_qa.call_args[1]["search_filter"] == expected
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_evaluate_dataset_override_reaches_both_phases(self) -> None:
|
||||
spec = _stub_spec(search_filter="""metadata LIKE '%"corpus": "orb_text"%'""")
|
||||
|
||||
with (
|
||||
patch(
|
||||
|
|
@ -738,7 +680,7 @@ class TestSearchFilterThreading:
|
|||
) as mock_qa,
|
||||
):
|
||||
await evaluate_dataset(
|
||||
spec=spec,
|
||||
spec=_stub_spec(),
|
||||
config=AppConfig(),
|
||||
skip_db=True,
|
||||
skip_retrieval=False,
|
||||
|
|
@ -746,11 +688,11 @@ class TestSearchFilterThreading:
|
|||
limit=None,
|
||||
name=None,
|
||||
db_path=None,
|
||||
search_filter="title LIKE '%paper%'",
|
||||
document_filter=expected,
|
||||
)
|
||||
|
||||
assert mock_retrieval.call_args[1]["search_filter"] == "title LIKE '%paper%'"
|
||||
assert mock_qa.call_args[1]["search_filter"] == "title LIKE '%paper%'"
|
||||
assert mock_retrieval.call_args[1]["document_filter"] == expected
|
||||
assert mock_qa.call_args[1]["document_filter"] == expected
|
||||
|
||||
|
||||
class TestEvaluateDatasetCaseIds:
|
||||
|
|
|
|||
Loading…
Reference in a new issue