remove filter parameter from analyze skill tool and clean up unused filter helpers
This commit is contained in:
parent
2a0e89bbe7
commit
aa4ce07dc9
6 changed files with 21 additions and 101 deletions
|
|
@ -63,6 +63,4 @@ docs = create_document_toolset(config)
|
|||
|
||||
`haiku.rag.tools.filters` provides utilities for building SQL filters:
|
||||
|
||||
- **`build_document_filter(document_name)`** — Builds a LIKE filter matching against both `uri` and `title`, case-insensitive. Also matches without spaces (e.g., "TB MED 593" matches "tbmed593").
|
||||
- **`build_multi_document_filter(document_names)`** — Combines multiple document name filters with OR logic.
|
||||
- **`combine_filters(filter1, filter2)`** — Combines two filters with AND logic. Returns `None` if both are `None`.
|
||||
- **`build_multi_document_filter(document_names)`** — Combines multiple document name filters with OR logic. Matches against both `uri` and `title`, case-insensitive.
|
||||
|
|
|
|||
|
|
@ -8,7 +8,6 @@ from haiku.rag.agents.research.models import Citation
|
|||
from haiku.rag.config.models import AppConfig
|
||||
from haiku.rag.store.models.chunk import SearchResult
|
||||
from haiku.rag.tools.document import DocumentInfo
|
||||
from haiku.rag.tools.filters import combine_filters
|
||||
from haiku.rag.tools.qa import QAHistoryEntry
|
||||
from haiku.skills.state import SkillRunDeps
|
||||
|
||||
|
|
@ -210,13 +209,15 @@ async def skill_analyze(
|
|||
config: AppConfig,
|
||||
question: str,
|
||||
document: str | None = None,
|
||||
filter: str | None = None,
|
||||
document_filter: str | None = None,
|
||||
) -> tuple[str, str, str | None]:
|
||||
from haiku.rag.client import HaikuRAG
|
||||
|
||||
async with HaikuRAG(db_path, config=config, read_only=True) as rag:
|
||||
documents = [document] if document else None
|
||||
result = await rag.analyze(question, documents=documents, filter=filter)
|
||||
result = await rag.analyze(
|
||||
question, documents=documents, filter=document_filter
|
||||
)
|
||||
output = result.answer
|
||||
if result.program:
|
||||
output += f"\n\nProgram:\n{result.program}"
|
||||
|
|
@ -465,7 +466,6 @@ def create_skill_tools(
|
|||
ctx: RunContext[SkillRunDeps],
|
||||
question: str,
|
||||
document: str | None = None,
|
||||
filter: str | None = None,
|
||||
) -> str:
|
||||
"""Answer complex analytical questions using code execution.
|
||||
|
||||
|
|
@ -475,13 +475,15 @@ def create_skill_tools(
|
|||
Args:
|
||||
question: The question to answer.
|
||||
document: Optional document ID or title to pre-load for analysis.
|
||||
filter: Optional SQL WHERE clause to filter documents.
|
||||
"""
|
||||
state = _get_state(ctx, state_type)
|
||||
state_filter = state.document_filter if state else None
|
||||
effective_filter = combine_filters(state_filter, filter)
|
||||
output, answer, program = await skill_analyze(
|
||||
db_path, config, question, document=document, filter=effective_filter
|
||||
db_path,
|
||||
config,
|
||||
question,
|
||||
document=document,
|
||||
document_filter=state_filter,
|
||||
)
|
||||
if state:
|
||||
state.analyses.append(
|
||||
|
|
|
|||
|
|
@ -1,10 +1,6 @@
|
|||
from haiku.rag.tools.context import RAGDeps
|
||||
from haiku.rag.tools.document import create_document_toolset
|
||||
from haiku.rag.tools.filters import (
|
||||
build_document_filter,
|
||||
build_multi_document_filter,
|
||||
combine_filters,
|
||||
)
|
||||
from haiku.rag.tools.filters import build_multi_document_filter
|
||||
from haiku.rag.tools.qa import PRIOR_ANSWER_RELEVANCE_THRESHOLD, QAHistoryEntry
|
||||
from haiku.rag.tools.search import create_search_toolset
|
||||
|
||||
|
|
@ -12,9 +8,7 @@ __all__ = [
|
|||
"PRIOR_ANSWER_RELEVANCE_THRESHOLD",
|
||||
"QAHistoryEntry",
|
||||
"RAGDeps",
|
||||
"build_document_filter",
|
||||
"build_multi_document_filter",
|
||||
"combine_filters",
|
||||
"create_document_toolset",
|
||||
"create_search_toolset",
|
||||
]
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
def build_document_filter(document_name: str) -> str:
|
||||
def _build_document_filter(document_name: str) -> str:
|
||||
"""Build SQL filter for document name matching.
|
||||
|
||||
Matches against both uri and title fields, case-insensitive.
|
||||
|
|
@ -19,20 +19,7 @@ def build_multi_document_filter(document_names: list[str]) -> str | None:
|
|||
"""
|
||||
if not document_names:
|
||||
return None
|
||||
filters = [build_document_filter(name) for name in document_names]
|
||||
filters = [_build_document_filter(name) for name in document_names]
|
||||
if len(filters) == 1:
|
||||
return filters[0]
|
||||
return " OR ".join(f"({f})" for f in filters)
|
||||
|
||||
|
||||
def combine_filters(filter1: str | None, filter2: str | None) -> str | None:
|
||||
"""Combine two SQL filters with AND logic.
|
||||
|
||||
Returns None if both filters are None.
|
||||
"""
|
||||
filters = [f for f in [filter1, filter2] if f]
|
||||
if not filters:
|
||||
return None
|
||||
if len(filters) == 1:
|
||||
return filters[0]
|
||||
return f"({filters[0]}) AND ({filters[1]})"
|
||||
|
|
|
|||
|
|
@ -175,34 +175,7 @@ class TestAnalyzeTool:
|
|||
await analyze(ctx, question="How many documents?")
|
||||
assert captured_kwargs.get("filter") == "title = 'AI Overview'"
|
||||
|
||||
async def test_analyze_combines_state_filter_with_explicit_filter(
|
||||
self, rag_db, monkeypatch
|
||||
):
|
||||
from haiku.rag.skills.analysis import AnalysisState, create_skill
|
||||
|
||||
captured_kwargs = {}
|
||||
|
||||
async def mock_analyze(self, question, **kwargs):
|
||||
captured_kwargs.update(kwargs)
|
||||
return AnalysisResult(answer="Result", program="code()")
|
||||
|
||||
monkeypatch.setattr(HaikuRAG, "analyze", mock_analyze)
|
||||
|
||||
skill = create_skill(db_path=rag_db)
|
||||
analyze = _get_tool(skill, "analyze")
|
||||
state = AnalysisState(document_filter="title = 'AI Overview'")
|
||||
ctx = _make_ctx(state)
|
||||
await analyze(
|
||||
ctx,
|
||||
question="Count pages",
|
||||
filter="uri LIKE '%test%'",
|
||||
)
|
||||
result_filter = captured_kwargs["filter"]
|
||||
assert isinstance(result_filter, str)
|
||||
assert "title = 'AI Overview'" in result_filter
|
||||
assert "uri LIKE '%test%'" in result_filter
|
||||
|
||||
async def test_analyze_with_document_and_filter(self, rag_db, monkeypatch):
|
||||
async def test_analyze_with_document(self, rag_db, monkeypatch):
|
||||
from haiku.rag.skills.analysis import create_skill
|
||||
|
||||
captured_kwargs = {}
|
||||
|
|
@ -220,7 +193,5 @@ class TestAnalyzeTool:
|
|||
ctx,
|
||||
question="Count pages",
|
||||
document="AI Overview",
|
||||
filter="title = 'AI Overview'",
|
||||
)
|
||||
assert captured_kwargs.get("documents") == ["AI Overview"]
|
||||
assert captured_kwargs.get("filter") == "title = 'AI Overview'"
|
||||
|
|
|
|||
|
|
@ -1,21 +1,16 @@
|
|||
from haiku.rag.tools.filters import (
|
||||
build_document_filter,
|
||||
build_multi_document_filter,
|
||||
combine_filters,
|
||||
)
|
||||
from haiku.rag.tools.filters import _build_document_filter, build_multi_document_filter
|
||||
|
||||
|
||||
def test_build_document_filter_simple():
|
||||
"""Test build_document_filter with simple name."""
|
||||
result = build_document_filter("mytest")
|
||||
"""Test _build_document_filter with simple name."""
|
||||
result = _build_document_filter("mytest")
|
||||
assert "LOWER(uri) LIKE LOWER('%mytest%')" in result
|
||||
assert "LOWER(title) LIKE LOWER('%mytest%')" in result
|
||||
|
||||
|
||||
def test_build_document_filter_with_spaces():
|
||||
"""Test build_document_filter handles spaces correctly."""
|
||||
result = build_document_filter("TB MED 593")
|
||||
# Should include both the original (with spaces) and without spaces
|
||||
"""Test _build_document_filter handles spaces correctly."""
|
||||
result = _build_document_filter("TB MED 593")
|
||||
assert "LOWER(uri) LIKE LOWER('%TB MED 593%')" in result
|
||||
assert "LOWER(uri) LIKE LOWER('%TBMED593%')" in result
|
||||
assert "LOWER(title) LIKE LOWER('%TB MED 593%')" in result
|
||||
|
|
@ -23,9 +18,8 @@ def test_build_document_filter_with_spaces():
|
|||
|
||||
|
||||
def test_build_document_filter_escapes_quotes():
|
||||
"""Test build_document_filter escapes single quotes."""
|
||||
result = build_document_filter("O'Reilly")
|
||||
# Single quotes should be doubled for SQL escaping
|
||||
"""Test _build_document_filter escapes single quotes."""
|
||||
result = _build_document_filter("O'Reilly")
|
||||
assert "O''Reilly" in result
|
||||
|
||||
|
||||
|
|
@ -41,7 +35,6 @@ def test_build_multi_document_filter_single():
|
|||
assert result is not None
|
||||
assert "LOWER(uri) LIKE LOWER('%mytest%')" in result
|
||||
assert "LOWER(title) LIKE LOWER('%mytest%')" in result
|
||||
# Single document should not have extra wrapping parentheses
|
||||
assert " OR (" not in result
|
||||
|
||||
|
||||
|
|
@ -49,31 +42,6 @@ def test_build_multi_document_filter_multiple():
|
|||
"""Test build_multi_document_filter with multiple documents."""
|
||||
result = build_multi_document_filter(["doc1", "doc2"])
|
||||
assert result is not None
|
||||
# Should have OR-combined filters
|
||||
assert "doc1" in result
|
||||
assert "doc2" in result
|
||||
assert " OR (" in result
|
||||
|
||||
|
||||
def test_combine_filters_both_none():
|
||||
"""Test combine_filters with both None."""
|
||||
result = combine_filters(None, None)
|
||||
assert result is None
|
||||
|
||||
|
||||
def test_combine_filters_first_only():
|
||||
"""Test combine_filters with only first filter."""
|
||||
result = combine_filters("uri = 'test'", None)
|
||||
assert result == "uri = 'test'"
|
||||
|
||||
|
||||
def test_combine_filters_second_only():
|
||||
"""Test combine_filters with only second filter."""
|
||||
result = combine_filters(None, "title = 'doc'")
|
||||
assert result == "title = 'doc'"
|
||||
|
||||
|
||||
def test_combine_filters_both():
|
||||
"""Test combine_filters combines with AND."""
|
||||
result = combine_filters("uri = 'test'", "title = 'doc'")
|
||||
assert result == "(uri = 'test') AND (title = 'doc')"
|
||||
|
|
|
|||
Loading…
Reference in a new issue