From aa4ce07dc92be05b05e4c51cafce2bd89db3a8c1 Mon Sep 17 00:00:00 2001 From: Yiorgis Gozadinos Date: Fri, 17 Apr 2026 13:03:29 +0300 Subject: [PATCH] remove filter parameter from analyze skill tool and clean up unused filter helpers --- docs/tools.md | 4 +- haiku_rag_slim/haiku/rag/skills/_tools.py | 16 ++++---- haiku_rag_slim/haiku/rag/tools/__init__.py | 8 +--- haiku_rag_slim/haiku/rag/tools/filters.py | 17 +------- tests/skills/test_analysis.py | 31 +-------------- tests/tools/test_filters.py | 46 ++++------------------ 6 files changed, 21 insertions(+), 101 deletions(-) diff --git a/docs/tools.md b/docs/tools.md index b552a6fe..f6aca748 100644 --- a/docs/tools.md +++ b/docs/tools.md @@ -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. diff --git a/haiku_rag_slim/haiku/rag/skills/_tools.py b/haiku_rag_slim/haiku/rag/skills/_tools.py index c27c8472..e675c563 100644 --- a/haiku_rag_slim/haiku/rag/skills/_tools.py +++ b/haiku_rag_slim/haiku/rag/skills/_tools.py @@ -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( diff --git a/haiku_rag_slim/haiku/rag/tools/__init__.py b/haiku_rag_slim/haiku/rag/tools/__init__.py index def2e88c..384ead52 100644 --- a/haiku_rag_slim/haiku/rag/tools/__init__.py +++ b/haiku_rag_slim/haiku/rag/tools/__init__.py @@ -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", ] diff --git a/haiku_rag_slim/haiku/rag/tools/filters.py b/haiku_rag_slim/haiku/rag/tools/filters.py index b09d14db..66d7f5b9 100644 --- a/haiku_rag_slim/haiku/rag/tools/filters.py +++ b/haiku_rag_slim/haiku/rag/tools/filters.py @@ -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]})" diff --git a/tests/skills/test_analysis.py b/tests/skills/test_analysis.py index f647abc4..0cada1f9 100644 --- a/tests/skills/test_analysis.py +++ b/tests/skills/test_analysis.py @@ -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'" diff --git a/tests/tools/test_filters.py b/tests/tools/test_filters.py index de449dd8..85d02290 100644 --- a/tests/tools/test_filters.py +++ b/tests/tools/test_filters.py @@ -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')"